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

Analyzer: Short-circuited const code has false negatives AND positives #36219

Open
MichaelRFairhurst opened this Issue Mar 14, 2019 · 3 comments

Comments

Projects
None yet
4 participants
@MichaelRFairhurst
Copy link
Contributor

MichaelRFairhurst commented Mar 14, 2019

void main() {                                                                    
  dynamic nonConst;                                                              
  const dynamic nil = null;                                                      
  const Object constValue = const Object();                                      
                                                                                 
  const falsePositive1 = true ? constValue : nil + 1;                            
  const falsePositive2 = false ? nil + 1 : constValue;                           
  const falsePositive3 = constValue ?? nil + 1;                                  
                                                                                 
  const falseNegative1 = true || nonConst;                                       
  const falseNegative2 = false && nonConst;                                      
                                                                                 
  print(falsePositive1);                                                         
  print(falsePositive2);                                                         
  print(falsePositive3);                                                         
  print(falseNegative1);                                                         
  print(falseNegative2);                                                         
}               

dartanalyzer reports:

  error • Evaluation of this constant expression throws an exception at lib/test.dart:6:46 • const_eval_throws_exception
  error • Evaluation of this constant expression throws an exception at lib/test.dart:7:34 • const_eval_throws_exception
  error • Evaluation of this constant expression throws an exception at lib/test.dart:8:40 • const_eval_throws_exception

These are false negatives. The spec regarding short-circuiting operations specifies that these expressions only require the unexecuted code to be potentially-const expressions, which does not require them to be evaluated.

There are also two false negatives, because falseNegative and falseNegative2 depend on non-const expressions, but no error is reported.

CFE reports:

lib/test.dart:10:34: Error: Not a constant expression.
  const falseNegative1 = true || nonConst;
                                 ^^^^^^^^
lib/test.dart:11:35: Error: Not a constant expression.
  const falseNegative2 = false && nonConst;
                                  ^^^^^^^^

The issue is that we rely on constant evaluator to report references to non-const concepts like local variables etc. However, if we run the constant evaluator we get the runtime errors that should not be reported. Constant verifier is probably where the potentially-const expression validation should occur, however, it does not do that type of validation (and if it did, it would be duplicating work).

@MichaelRFairhurst MichaelRFairhurst changed the title Analyzer: Short-circuited code has false negatives AND positives Analyzer: Short-circuited const code has false negatives AND positives Mar 14, 2019

@bwilkerson

This comment has been minimized.

Copy link
Member

bwilkerson commented Mar 14, 2019

Related to #36192.

@leafpetersen

This comment has been minimized.

Copy link
Member

leafpetersen commented Mar 14, 2019

Just to be clear, I think the false positives here are only false positives with the new constant semantics that have not yet shipped. dart2js reports errors there as well.

@lrhn

This comment has been minimized.

Copy link
Member

lrhn commented Mar 14, 2019

Agree, the false-positives should still be erors until we release the update constants. Since some of these constant changes were implemented already during the Dart 2.0 phase, we are in a slightly shaky position until every platform catches up.

The false negatives are bugs. They are, always have been, and always will be (being optimisitic here) compile-time errors. It's likely a regression from partially implementing the new short-circuiting const specification, and not getting all the details right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.