Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dart2js and VM differ in error on assignment to a static const #13964

Closed
fsc8000 opened this issue Oct 10, 2013 · 7 comments
Closed

dart2js and VM differ in error on assignment to a static const #13964

fsc8000 opened this issue Oct 10, 2013 · 7 comments
Assignees
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). closed-not-planned Closed as we don't intend to take action on the reported issue P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@fsc8000
Copy link
Contributor

fsc8000 commented Oct 10, 2013

const int XX = ZZ + 42;

main() {
  XX = 123;
}

On dart2js I get a NoSuchMethodError, the VM produces a "not a valid compile-time constant" compile error. I'm not sure what the correct thing would be, but in any case both should report the same error.

@kasperl
Copy link

kasperl commented Oct 10, 2013

Hmm. Johnni and I just looked at this and it is a little bit tricky. If you do this:

const int XX = ZZ + 42;
final int YY = XX;

main() {
  YY = 123;
}

then the VM gives you a runtime error (cannot call non-existing setter), so clearly it doesn't care about the right hand side of the YY definition at that point. It's not quite clear what the most consistent thing is here. Let's make this a language issue and see if we can agree on something.


Removed Priority-Unassigned, Area-Dart2JS labels.
Added Priority-Medium, Area-Language labels.

@fsc8000
Copy link
Contributor Author

fsc8000 commented Oct 11, 2013

btw: The same difference is behavior exists when trying to assign to a const where the RHS has a circular dependency in initialization:

const X = Z;
const Y = X + 1;
const Z = Y + 1;

main() {
  Z = 123;
}

@DartBot
Copy link

DartBot commented Oct 11, 2013

This comment was originally written by @mhausner


To shed a bit of light on the difference: when the VM parses the identifier, it does not yet know whether it will be used on the right- or left-hand side of an expression. It first treats the identifier as right-hand side, and while doing so, runs the initializer of the const value which produces the error messages you see. It would be better if the VM delayed this to a point where it knows whether the getter or setter is referenced. That's a bigger change we'll have to postpone until after 1.0.

@gbracha
Copy link
Contributor

gbracha commented Aug 27, 2014

The declaration of XX is defined a s compile time error in the spec. However, we allow compilation to happen late, so one need not always compile the declaration of XX. If you call the (non-existent) setter for XX before compiling XX, you get a no such method.

I would think that dart2js is in a position to compile all the code, used or not, and reliably give a compilation error, but the VM might not be able to do so.


Set owner to @gbracha.
Added Accepted label.

@fsc8000 fsc8000 added Type-Defect area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). labels Aug 27, 2014
@kevmoo kevmoo added P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) and removed accepted labels Feb 29, 2016
@lrhn
Copy link
Member

lrhn commented Dec 15, 2016

Just to summarize: The only "problem" here is that the VM evaluates the RHS of the const declaration even if nobody is reading it. It is a compile-time error that the RHS isn't a valid compile-time constant, the only surprise is when (or that) it's reported.

The assignment should fail too, since a const declaration doesn't introduce a setter, and it does fail if the const declaration is valid.

@munificent
Copy link
Member

We're not planning to clarify this in the language spec. Since the code already has a compile-time error, the runtime behavior is undefined.

@munificent munificent added the closed-not-planned Closed as we don't intend to take action on the reported issue label Dec 16, 2016
@lrhn
Copy link
Member

lrhn commented Jan 2, 2017

When the VM made compile-time errors catchable (instead of killing the isolate), there was a guide to when a compile-time error could reasonably be thrown.

For const declarations with compile-time errors, the compile-time error should happen in the non-constant code that refers to the constant declaration. That is:

const x = arglebargle;  // Not valid constant!
foo() {
  return x + 2;
}

The compile time error for x should happen (at the latest) when you call foo, and possibly earlier when compiling something that refers to foo (because inlining).

I think it's slightly confusing that the VM throws a compilation error in this case because main doesn't refer to XX. It refers to XX=, which is not the const declaration.
If you had a program like:

const X = someError;
void set X(v) { print(v); }
main() {
  X = "Hello world!";
}

then (you are certifiable insane, but) the program should not touch the X constant.

(Currently it doesn't since implementations fail to allow setters next to constant declarations).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). closed-not-planned Closed as we don't intend to take action on the reported issue P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

7 participants