Skip to content

vm: field access shouldn't be compile-time constant #11206

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

Closed
DartBot opened this issue Jun 11, 2013 · 8 comments
Closed

vm: field access shouldn't be compile-time constant #11206

DartBot opened this issue Jun 11, 2013 · 8 comments
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. closed-duplicate Closed in favor of an existing report

Comments

@DartBot
Copy link

DartBot commented Jun 11, 2013

This issue was originally filed by [email protected]


Dart SDK version 0.5.16.0_r23799

Consider the following program which runs without issue in the VM:

///////////////////////////////////////////////////
class Colors {
  final String background = 'transparent';
  const Colors();
}

class Resources {
  final String style =
"""
.foo {
  background-color: ${const Colors().background};
}
""";
  const Resources();
}

main() {
  var res = const Resources();
  print('${res.style}');
}
///////////////////////////////////////////////////

Running dart2js on the above program yields:

test.dart:11:23: Error: not a compile-time constant
  background-color: ${const Colors().background};

                      ^^^^^^^^^^^^^^^^^^^^^^^^^
Error: Compilation failed.

@kasperl
Copy link

kasperl commented Jun 12, 2013

At first glance this actually appears to be a bug in the VM. Accessing a field from a compile-time constant isn't specified to be a compile-time constant; see

http://www.dartlang.org/docs/spec/latest/dart-language-specification.html#id.hzs87hup8wb


Added Area-Dart2JS, Triaged labels.

@kasperl
Copy link

kasperl commented Jun 12, 2013

cc @sgmitrovic.
Removed Area-Dart2JS label.
Added Area-VM label.
Changed the title to: "vm: field access shouldn't be compile-time constant".

@DartBot
Copy link
Author

DartBot commented Jun 12, 2013

This comment was originally written by [email protected]


But isn't that field access inside of an interpolated expression that evaluates to a string?

"A literal string where any interpolated expression is a compile-time constant that evaluates to a numeric, string or boolean value or to null. It would be tempting to allow string interpolation where the interpolated value is any compile-time constant. However, this would require running the toString() method for constant objects, which could contain arbitrary code."

@iposva-google
Copy link
Contributor

As far as I understand the stricter checking Srdjan is working on will not find this issue, correct? We still need to ensure that we do not execute non-constant expressions in const constructors.


Set owner to @mhausner.
Added Accepted label.

@peter-ahe-google
Copy link
Contributor

Ross,

The specification is saying: "A constant expression is one of the following: ... A literal string where any interpolated expression is a compile-time constant that evaluates to a numeric, string or boolean value or to null. ..."

Let's take a look at your string literal:

"""
.foo {
  background-color: ${const Colors().background};
}
"""

Are all the interpolated expressions compile-time constants? That question boils down to is this a compile-time constant:

const Colors().background

That problem is that it isn't a compile-time constant (a property access is not legal in a constant expression). const Colors() is a compile-time constant (but it doesn't evaluate to "a numeric, string or boolean value or to null").

Hope this helps clarify the rules.

Cheers,
Peter

@DartBot
Copy link
Author

DartBot commented Jun 12, 2013

This comment was originally written by [email protected]


Thanks Peter, I think I get it now :)

@DartBot
Copy link
Author

DartBot commented Jun 12, 2013

This comment was originally written by @mhausner


That is correct, Srdjan's change is not going to catch this problem. The VM knows that the field access is not a compile time constant, but it is not performing the checks in the field initializer expression in class Resources. That's because we only have one constructor which can be called through new (in which case the field initializer does not have to be a constant) and through const, in which case the initializer has to be a constant.

This is essentially a dup of issue #392.

@DartBot
Copy link
Author

DartBot commented Jun 13, 2013

This comment was originally written by @mhausner


Closing this. It's covered with issue #392.


Added Duplicate label.
Marked as being merged into #392.

@DartBot DartBot added Type-Defect area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. closed-duplicate Closed in favor of an existing report labels Jun 13, 2013
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. closed-duplicate Closed in favor of an existing report
Projects
None yet
Development

No branches or pull requests

4 participants