Skip to content

can't assert "== null" in an initializer assert (but != null works) #30288

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
Hixie opened this issue Jul 28, 2017 · 9 comments
Closed

can't assert "== null" in an initializer assert (but != null works) #30288

Hixie opened this issue Jul 28, 2017 · 9 comments
Labels
legacy-area-analyzer Use area-devexp instead. 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

@Hixie
Copy link
Contributor

Hixie commented Jul 28, 2017

class A {
  const A(this.a, this.b) : assert(a == null || b == null);
  final Object a;
  final Object b;
}

...results in:

  error • In constant expressions, operands of this operator must be of type 'bool', 'num', 'String' or 'null' at test.dart:2:36 • const_eval_type_bool_num_string
  error • In constant expressions, operands of this operator must be of type 'bool', 'num', 'String' or 'null' at test.dart:2:49 • const_eval_type_bool_num_string

...but this works fine:

class A {
  const A(this.a, this.b) : assert(!((a != null) && (b != null)));
  final Object a;
  final Object b;
}
@lrhn lrhn added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Jul 30, 2017
@a-siva
Copy link
Contributor

a-siva commented Jul 31, 2017

Are you using the --assert_initializer option. I tried this on TOT command line dart VM and it seems to work :
class A {
const A(this.a, this.b) : assert(a == null || b == null);
final Object a;
final Object b;
}

main()
{
A a = new A(null, null);
print(a);
}

sivalinuxmach[sdk]>out/ReleaseX64/dart --checked --assert_initializer /tmp/junk.dart
Instance of 'A'

@Hixie
Copy link
Contributor Author

Hixie commented Jul 31, 2017

Sorry, I should have been explicit. This is an analyzer failure, not a VM failure.

@a-siva a-siva added legacy-area-analyzer Use area-devexp instead. and removed area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. labels Jul 31, 2017
@a-siva
Copy link
Contributor

a-siva commented Jul 31, 2017

Thanks somebody marked it under area-vm, will change accordingly.

Hixie pushed a commit to flutter/flutter that referenced this issue Aug 1, 2017
* PopupMenuButton: create IconButton if child is Icon

Otherwise the resulting button has an abnormally small and rectangular
area. With multiple PopupMenuButton(child: Icon) they get squished
together in the AppBar.

* Add separate icon argument to PopupMenuButton

* Fix style issues and tweak dartdocs

* Add tests for icon argument to PopupMenuButton

* Group icon tests and fix broken test, analyzer warnings

* Test that the correct custom icon is present

* Apply De Morgan's to work around dart analyzer bug

see: dart-lang/sdk#30288
@bwilkerson
Copy link
Member

@lrhn I have a question about the specification, and I'm hoping you can answer it.

In section 16.1 it says:

An expression of one of the forms e1 == e2 or e1 != e2 where e1 and e2 are constant expressions that evaluate to a numeric, string or boolean value or to null.

Given the code above, I would expect analyzer to figure out that a has type Object and to create an error. It looks to me like the bug is that we're not generating an error in the != case.

Is that your interpretation, or am I wrong here?

@bwilkerson bwilkerson added the P2 A bug or feature request we're likely to work on label Aug 25, 2017
@lrhn
Copy link
Member

lrhn commented Aug 25, 2017

Constant evaluation is all about objects, not static types. That's why the VM has been able to implement it without having a static type system.

The potentially constant vs actually compile-time constant distinction is a little tricky in the spec.

The expression a == null is a valid potentially constant expression because it would be a valid compile-time constant expression if a was a constant variable with a value that was an int, bool or string (section 10.6.3). That requirement is a static syntactic constraint on the potentially constant expression, it's independent of the value that the variable will have when the constructor is invoked -
because that can vary anyway.

When you do evaluate the const constructor call, you evaluate the expression as a compile-time constant expression as if the parameters were const variables with the provided values.
In this case, a and b both evaluate to the value null, so the equality expressions are valid compile-time constant expressions by the rule you quoted (all operands are null).

So, it's a valid potentially constant expression and evaluation as compile-time constant expression succeeds. It would succeed no matter which arguments were passed.

It would not succeed if the arguments were non-int/string/bool/null values, because then the expression a == null would not be compile-time constant, because of 16.1.

To get around that, you can use identical(a, null) instead because that's a valid compile-time constant expression for all constant values of a (the point before == in section 16.1).

(And yes, the != case should be equivalent to the == case).

@eernstg
Copy link
Member

eernstg commented Nov 3, 2017

PS: As of f11f2ed, the spec has been modified to lift the non-int/string/bool/null restriction. Here's the updated constant expression case:

\item An expression of one of the forms  \code{$e_1$ == $e_2$} or  \code{$e_1$ != $e_2$}
where $e_1$ and $e_2$ are constant expressions, and either both evaluate to a numeric,
string or boolean value, or at least one of $e_1$ or $e_2$ evaluates to \NULL{}.

So the tools should allow it, plain == as well as !=.

@Hixie
Copy link
Contributor Author

Hixie commented Nov 3, 2017

See also #29278 which is about allowing enums in these expressions.

@dnfield
Copy link
Contributor

dnfield commented Jun 11, 2018

Tooling is allowing identical but not == e.g.

Fails:

class Test2 {
  const Test2({this.opt1, this.opt2}) : assert(opt1 == null || opt2 == null);
  final Object opt1;
  final Object opt2;
}

Passes:

class Test2 {
  const Test2({this.opt1, this.opt2}) : assert(identical(opt1, null) ||  identical(opt2, null));
  final Object opt1;
  final Object opt2;
}

@eernstg
Copy link
Member

eernstg commented Jun 11, 2018

Apparently, the implementation of this feature was never requested explicitly. I just created #33408 to make that happen. I'm closing this because the 'area-language' part was resolved already in Oct 2017 with commit f11f2ed.

@eernstg eernstg closed this as completed Jun 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
legacy-area-analyzer Use area-devexp instead. 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

6 participants