Confirming.

*** Bug 604726 has been marked as a duplicate of this bug. ***

Created attachment 150252 [details] [review]
Check the default value to be contant

This patch simply enforces the default value to be a constant expression. Non-constants would be unnecessarily complicated to implement and inconsistent in the C API.

If someone needs for example:

  f (string param1, string param2 = param1) {
  }

That can be easily achieved like this, possibly with an empty string instead of null:

  f (string param1, string? param2 = null) {
    if (param2 == null) param2 = param1;
  }

No need for non-constant expressions whatsoever (in case someone has a valid use case where this is not doable, feel free to prove me wrong :)).

This breaks bootstrapping. Non-constant default arguments are used.

Ah, that didn't occur to me. Will fix.

*** Bug 589604 has been marked as a duplicate of this bug. ***

Invalid C code is no longer generated.

error: Cannot assign to construct-only properties, use Object (property: value) constructor chain up
        this.str = inputString;

The error report is about invalid c-code being generated, that cannot be compiled by a c-compiler. The current error is from the vala-compiler and seems correct for the code.
So this is resolved.

Simpler test case:

void foo (string a, string b = a) {
}

void main () {
	foo("a");
}

OK,for the initial test case the vala compiler gives a error and does no longer generates invalid c-code.

The simpler test case does still demonstrate the problem it' even worse:

void foo_str (string a, string b = a) {
}

void foo_int (int a, int b = a) {
}

void main () {
    foo_str("a");
    foo_int(1);
}


Translates to:

void _vala_main (void) {
	const gchar* _tmp0_;
	gint _tmp1_;
	_tmp0_ = a;
	foo_str ("a", _tmp0_);
	_tmp1_ = a;
	foo_int (1, _tmp1_);
}

It looks related to: Bug 566863 - vala semantic analyser turns abstract syntax tree into abstract syntax DAG 


If I'm correct, in the example the expression could be copied since it is a constant free of side effects. However what if this is a non constant expression?

void foo (int a, int b = a) {
}

void main () {		
    foo(Rand.next_int());
}

Transform to vala code

void main () {		
    var tmp0 = Rand.next_int();
    var tmp1 = tmp0;
    foo(tmp0, tmp1);
}

Or c-code:

	int tmp0 = g_random_int();
	int tmp1 = tmp0;
	foo(tmp0, tmp1);

Still a problem in 0.35.

I don't see a problem here.

void foo (sting a, string b = null) {
   string sb = b;
   if (sb == null) sb = a;
  /* Do your stuff */
}

Then is not necessary to pass a non-constant, non-known value, at declaration time.

This should be solved in your method implementation. While in C API there is no way to define default values, this makes no sense to add this to Vala directly, but implementing parameter values handling, this will help C users.

I'm closing this bug. Feel free to file a new one as an enhancement and target for 2.0 if you wish.

(In reply to Daniel Espinosa from comment #12)

Sorry, I fundamentally disagree here. In my opinion this is a showstopper for 1.0:

For _every_ bit of code that is valid Vala code (as defined by 'value -C case.vala' does not emit an error, but happily creates a C file out of it) and which later bombs when the C compiler tries to compile it, we have a serious semantic breach.

To the user it looks as Vala is to blame here, and even worse, it emits an error in a language the user did not chose to work with in the first place.

Our goal should be that the user does not have to care whether the Vala compiler is standalone or a C-transpiler.

Thus, I strongly believe that the _minimum_ goal for 1.0 needs to fix every single place where Vala creates invalid C code.

I agree with you. Thanks for reopen and 

We have different types of bugs, where users produce compilable code:

A) Incorrect GObject mechanisms applications, in any area related to construct, signals, properties an so or programming errors (including Cush code assumptions not directly supported in Vala yet). They can fixable by the user using code practices, like code conventions and tips.

This kind of things can be documented, as a limitation, in order to help users to avoid this kind of errors.

This bugs can be targeted to 1.2, I think, because Vala can help users by emitting warnings or errors, when possible, but are not a barrier to use Vala in a safe way.

B) Valid Vala code and valid GObject features application, fixable only in Vala.

These should be fixed before 1.0.

This is important, in order to advance and focus resources in critical issues.

Vala is not around the corner, then there is no hurry. Just is important to clarify users how to use Vala with its limitations.

(In reply to Daniel Espinosa from comment #14)
> Vala is not around the corner, then there is no hurry. Just is important to
> clarify users how to use Vala with its limitations.

Vala 0.36 is due 22 March 2017 with GNOME 3.24 ( see https://wiki.gnome.org/ThreePointTwentythree/ for the release schedule ). It is useful to have the 1.0 target milestone now in bugzilla, but the focus at present should be on getting good patches in for 0.36. Once 0.36 is out then we can start discussing what is needed for any possible 1.0 release. That discussion should be on IRC #vala and through the mailing list. I think it is OK to start tagging bugs as 1.0, so we can get some idea of the criteria we can use, but the criteria may change after we've had the community discussion.

(In reply to Al Thomas from comment #15)
> (In reply to Daniel Espinosa from comment #14)
> > Vala is not around the corner, then there is no hurry. Just is important to
> > clarify users how to use Vala with its limitations.
> 
> Vala 0.36 is due 22 March 2017 with GNOME 3.24 ( see
> https://wiki.gnome.org/ThreePointTwentythree/ for the release schedule ). It
> is useful to have the 1.0 target milestone now in bugzilla, but the focus at
> present should be on getting good patches in for 0.36. Once 0.36 is out then
> we can start discussing what is needed for any possible 1.0 release. That
> discussion should be on IRC #vala and through the mailing list. I think it
> is OK to start tagging bugs as 1.0, so we can get some idea of the criteria
> we can use, but the criteria may change after we've had the community
> discussion.

I agree.

I mean 1.0, I know 0.36 is around the corner and any patch should take carefully.

Moving this discussion to mailing list.

-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/vala/issues/8.