Expand the definition of potentially constant expression.

Expand the definition of potentially constant expression.

The current definition requires the expression to be valid and
compile-time constant if constructor parameters are considered
compile-time constants with types appropriate for their context.

This is necessary but not sufficient, so this CL adds the further
requirement that the expression must also be a valid (not necessarily
constant) expession if the parameters are considered non-constant
variables.

This rules out the case:

    const C(x) : y = const [x];

where the current specification would, technically, allow it as a
potentially constant expression and therefore be a valid const
constructor. In practice it would never work if the constructor is
used with "new" instead of "const".

This is not really a specification *change* - existing implementations has this restriction anyway.

Addressed the concern of #24970.
BUG= http://dartbug.com/24970
R=eernst@google.com, gbracha@google.com

Committed: https://github.com/dart-lang/sdk/commit/2a0cc74db17c3eb9883582975c8ce5165bdd8dfd

[Lasse Reichstein Nielsen, 2015-11-19 15:30:13]

Description was changed from

==========
Expand the definition of potentially constant expression.

The current definition requires the expression to be valid and
compile-time constant if constructor parameters are considered
compile-time constants with types appropriate for their context.

This is necessary but not sufficient, so this CL adds the further
requirement that the expression must also be a valid (not necessarily
constant) expession if the parameters are considered non-constant
variables.

This rules out the case:

    const C(x) : y = const [x];

where the current specification would, technically, allow it as a
potentially constant expression and therefore be a valid const
constructor. In practice it would never work if the constructor is
used with "new" instead of "const".

Addressed the concern of #24970.
BUG= http://dartbug.com/24970
==========

to

==========
Expand the definition of potentially constant expression.

The current definition requires the expression to be valid and
compile-time constant if constructor parameters are considered
compile-time constants with types appropriate for their context.

This is necessary but not sufficient, so this CL adds the further
requirement that the expression must also be a valid (not necessarily
constant) expession if the parameters are considered non-constant
variables.

This rules out the case:

    const C(x) : y = const [x];

where the current specification would, technically, allow it as a
potentially constant expression and therefore be a valid const
constructor. In practice it would never work if the constructor is
used with "new" instead of "const".

This is not really a specification *change* - existing implementations has this
restriction anyway.

Addressed the concern of #24970.
BUG= http://dartbug.com/24970
==========

[Lasse Reichstein Nielsen, 2015-11-19 15:30:13]

lrn@google.com changed reviewers:
+ gbracha@google.com

[eernst, 2015-11-20 10:03:18]

eernst@google.com changed reviewers:
+ eernst@google.com

[eernst, 2015-11-20 10:03:18]

Tried to clarify the issue for myself, hoping that this would bring a couple of
relevant things to the surface.

https://codereview.chromium.org/1465473002/diff/1/docs/language/dartLangSpec.tex
File docs/language/dartLangSpec.tex (right):

https://codereview.chromium.org/1465473002/diff/1/docs/language/dartLangSpec....
docs/language/dartLangSpec.tex:1550: A {\em potentially constant expression} is
an expression $e$ that would be a valid constant expression if all formal
parameters of $e$'s immediately enclosing constant constructor were treated as
compile-time constants that were guaranteed to evaluate to an integer, boolean
or string value as required by their immediately enclosing superexpression,
<em>and</em> where $e$ is also a valid expression if all the formal parameters
are treated as non-constant variables.
So, basically, you're saying that (1) is more important than (2), where

  (1) it is possible to use `new` with a `const` constructor
  (2) `const` constructors can create `const` values in field initializers and
superinitializers using arguments

If we have (2) then (1) does not always hold, and if we insist that (1) must
always hold then (2) must be prevented (which is what your added words will
achieve).

An alternative that should be considered is to have (1) in all the cases where
(2) hasn't been applied. This would, presumably, be achieved by leaving the
words in this paragraph unchanged (and maybe adding commentary to explain the
situation). This would be rather pragmatic, and it would allow for more
programs, some of which would presumably make sense. ;-)

On the other hand, the more strict approach that you propose would emphasize the
separation of interface and implementation, because it avoids the situation
where an initializer determines whether or not a given application of the
declared entity (the constructor) is admissible.

In practice, it would be an unpleasant surprise if a propagating change of
`const` to `new` suddenly stops because a certain constructor does not admit
`new`. I'm not sure whether this is worse than the converse (which is very
well-known): A propagating change from `new` to `const` is very likely to fail,
because there is no suitable `const` constructor in one of the expressions where
you wish to make that change.

Yet another option would be to say that `const`ness is a first class concept,
and some `as_const_as_the_outermost_expression` keyword could be used to specify
that nested expression which should be `const` when used in a `const` object
creation, and `new` otherwise.

[Lasse Reichstein Nielsen, 2015-11-20 10:37:55]

https://codereview.chromium.org/1465473002/diff/1/docs/language/dartLangSpec.tex
File docs/language/dartLangSpec.tex (right):

https://codereview.chromium.org/1465473002/diff/1/docs/language/dartLangSpec....
docs/language/dartLangSpec.tex:1550: A {\em potentially constant expression} is
an expression $e$ that would be a valid constant expression if all formal
parameters of $e$'s immediately enclosing constant constructor were treated as
compile-time constants that were guaranteed to evaluate to an integer, boolean
or string value as required by their immediately enclosing superexpression,
<em>and</em> where $e$ is also a valid expression if all the formal parameters
are treated as non-constant variables.
I'm saying that (1) is what we have been implementing so far, and what the
specification says, and (2) has never worked or been expected to work - except
for string interpolations which can satisfy both, but which the VM doesn't allow
anyway. So yes, we have prioritized (1) over (2), and it has "always" been that
way.

The alternative you describe, which I wouldn't mind at all since it allows more
valid constructors, gives us constructors that are *const only*.

It's not allowed by leaving the the current specification unchanged. A const
expression that is not actually compile-time constant is a compile-time error.
If you write:
   const C(x) : x = const D(x);
and this would be a "const only" constructor (since using it with `new` would
not be valid) then there is no semantics for it in the spce. There is no text
saying that `new C(42)` could be invalid. It would fall between the cracks of
the current spec, and not have a specified semantics if it isn't a compile-time
error.

For the last sentence: Oh yeah! const/new/auto construction. Much easier if
const/new is optional in a potentially const scope :)

FWIW, the spec basically treats const constructors as two different constructors
- one used at compile-time and one used at runtime (for example String.+
canonicalizes at compile time, but not necessarily at runtime, which means that
`const C(x, y) : z = x + y;` may need to distinguish between `const C("a","b")`
and `new C("a","b")`.
The VM only has one constructor implementation which is the cause of some of
their issues with allowing non-constants in const constructor calls.

[Lasse Reichstein Nielsen, 2015-11-20 16:36:04]

On 2015/11/20 at 10:37:55, Lasse Reichstein Nielsen wrote:

> FWIW, the spec basically treats const constructors as two different
constructors - one used at compile-time and one used at runtime (for example
String.+ canonicalizes at compile time, but not necessarily at runtime, which
means that `const C(x, y) : z = x + y;` may need to distinguish between `const
C("a","b")` and `new C("a","b")`.
> The VM only has one constructor implementation which is the cause of some of
their issues with allowing non-constants in const constructor calls.

Or, to be precise: Strings are not necessarily canonicalized, but "identical"
used on compile time constants must work like == on strings. In practice that's
implementable by canonicalizing constant expressions, but the VM doesn't do
that. It only canonicalizes strings that are used as compile time constants.
(But now we a diverging).

[eernst, 2015-11-20 16:57:06]

On 2015/11/20 16:36:04, Lasse Reichstein Nielsen wrote:
> On 2015/11/20 at 10:37:55, Lasse Reichstein Nielsen wrote:
> 
> > FWIW, the spec basically treats const constructors as two different
> constructors - one used at compile-time and one used at runtime (for example
> String.+ canonicalizes at compile time, but not necessarily at runtime, which
> means that `const C(x, y) : z = x + y;` may need to distinguish between `const
> C("a","b")` and `new C("a","b")`.
> > The VM only has one constructor implementation which is the cause of some of
> their issues with allowing non-constants in const constructor calls.
> 
> Or, to be precise: Strings are not necessarily canonicalized, but "identical"
> used on compile time constants must work like == on strings. In practice
that's
> implementable by canonicalizing constant expressions, but the VM doesn't do
> that. It only canonicalizes strings that are used as compile time constants.
> (But now we a diverging).

I suspect that this is developing into something that should be a DEP rather
than a directly proposed spec update, but it is certainly useful to
discuss whether the current spec does allow for (1) only, or maybe also (2),
even if the implementations don't.

[Lasse Reichstein Nielsen, 2016-01-14 12:22:28]

On 2015/11/20 16:57:06, eernst wrote:
> I suspect that this is developing into something that should be a DEP rather
> than a directly proposed spec update, but it is certainly useful to
> discuss whether the current spec does allow for (1) only, or maybe also (2),
> even if the implementations don't.

I'm not actively developing this in any direction. 

The spec change above is still the only thing I propose to do, and only because
it will close a hole in the spec that seemingly allows things that no
implementation allows, and which is (probably) not intended.

We can always discuss what we could have instead, but that's a much bigger
change - this is a spec-only change to make it actually match how everybody is
reading it.

[eernst, 2016-01-14 15:10:41]

On 2016/01/14 12:22:28, Lasse Reichstein Nielsen wrote:
> On 2015/11/20 16:57:06, eernst wrote:
> > I suspect that this is developing into something that should be a DEP rather
> > than a directly proposed spec update, but it is certainly useful to
> > discuss whether the current spec does allow for (1) only, or maybe also (2),
> > even if the implementations don't.
> 
> I'm not actively developing this in any direction. 
> 
> The spec change above is still the only thing I propose to do, and only
because
> it will close a hole in the spec that seemingly allows things that no
> implementation allows, and which is (probably) not intended.
> 
> We can always discuss what we could have instead, but that's a much bigger
> change - this is a spec-only change to make it actually match how everybody is
> reading it.

OK. This means that the spec will be adjusted to say more precisely what
the implementations do, and this is a clarification rather than a change,
and that should not be controversial or harmful. Then we can always return
to the issue whether there should be support for const-only constructors.

LGTM.

Gilad?

[gbracha, 2016-01-14 21:17:43]

lgtm

[Lasse Reichstein Nielsen, 2016-01-20 12:21:42]

Description was changed from

==========
Expand the definition of potentially constant expression.

The current definition requires the expression to be valid and
compile-time constant if constructor parameters are considered
compile-time constants with types appropriate for their context.

This is necessary but not sufficient, so this CL adds the further
requirement that the expression must also be a valid (not necessarily
constant) expession if the parameters are considered non-constant
variables.

This rules out the case:

    const C(x) : y = const [x];

where the current specification would, technically, allow it as a
potentially constant expression and therefore be a valid const
constructor. In practice it would never work if the constructor is
used with "new" instead of "const".

This is not really a specification *change* - existing implementations has this
restriction anyway.

Addressed the concern of #24970.
BUG= http://dartbug.com/24970
==========

to

==========
Expand the definition of potentially constant expression.

The current definition requires the expression to be valid and
compile-time constant if constructor parameters are considered
compile-time constants with types appropriate for their context.

This is necessary but not sufficient, so this CL adds the further
requirement that the expression must also be a valid (not necessarily
constant) expession if the parameters are considered non-constant
variables.

This rules out the case:

    const C(x) : y = const [x];

where the current specification would, technically, allow it as a
potentially constant expression and therefore be a valid const
constructor. In practice it would never work if the constructor is
used with "new" instead of "const".

This is not really a specification *change* - existing implementations has this
restriction anyway.

Addressed the concern of #24970.
BUG= http://dartbug.com/24970
R=eernst@google.com, gbracha@google.com

Committed:
https://github.com/dart-lang/sdk/commit/2a0cc74db17c3eb9883582975c8ce5165bdd8dfd
==========

[Lasse Reichstein Nielsen, 2016-01-20 12:21:44]

Committed patchset #1 (id:1) manually as
2a0cc74db17c3eb9883582975c8ce5165bdd8dfd (presubmit successful).