Skip to content

Analyzer should issue a "mixed mode unsoundness" warning (or hint) in certain scenarios #43452

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
stereotype441 opened this issue Sep 16, 2020 · 3 comments
Labels
legacy-area-analyzer Use area-devexp instead. NNBD Issues related to NNBD Release P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug

Comments

@stereotype441
Copy link
Member

Due to dart-lang/language#1143, we are changing the front end so that inserts a synthetic throw if certain "impossible" situations arise due to the inherent unsoundness of mixed mode. For most of these scenarios, there should be a corresponding analyzer warning (or hint) to alert users to the situation and point them to documentation of what to do about it.

(Note that some or all of these scenarios may already be covered by analyzer hints, but I believe that in a lot of cases the hint is too general, e.g. a "dead code" hint. We want to have one or more hints that are specific to the following scenarios so that we can link to the appropriate documentation.)

The scenarios the analyzer should warn (or hint) about are as follows:

  • Flow analysis believes it is impossible for an is test to produce a false result (or an is! test to produce a true result). This can be identified by FlowAnalysis.isExpression_end returning false.
    • Note that the user can eliminate the warning by removing the check, or suppress it by casting the type of the operand of is to dynamic.
  • Flow analysis believes it is impossible for an == test to produce a true result (or a != test to produce a false result). This can be identified by FlowAnaylsis.equalityOp_end returning false. This will only occur when one side of the comparison has type Null, so in practice this probably means one side of the comparison is the literal null.
    • Note that the user can eliminate the warning by removing the check, or suppress it by casting the type of the non-null operand to dynamic.
  • Flow analysis believes that it is impossible for the right hand side of a ?? or ??= operation to be reached. This can be identified by FlowAnalysis.ifNullExpression_rightBegin returning false.
    • Note that for a ?? expression, that the user can eliminate the warning by replacing the expression with its left hand side, or suppress it by casting the left hand side to a nullable type.
    • For a ??= expression, the user can eliminate the warning by removing the assignment, or suppress it by replacing it with its desugared equivalent and using a cast to dynamic (e.g. replacing x ??= y; with if ((x as dynamic) == null) x = y;).
  • Flow analysis believes that it is impossible for the target of a ?. or ?.. to be null. This can be identified by FlowAnalysis.nullAwareAccess_rightBegin returning false.
    • Note that the user can eliminate the warning by removing the ?, or suppress it by casting the target to a nullable type.

There is one other scenario in which the front end inserts a synthetic throw: after evaluation of an expression having type Never. However, I don't think this should have a special analyzer warning beyond the usual "dead code" hint, because it crops up too frequently in legitimate code (e.g. calling fail in a test). Also I believe it's unlikely to be a problem in practice.

I plan to follow up with a language test that expects an analyzer warning in each of these cases. If we decide that we want a hint instead, we'll have to work with @munificent to adapt the language test infrastructure to be able to test for hints.

@franklinyow
Copy link
Contributor

@leafpetersen Please triage

@leafpetersen leafpetersen added the type-enhancement A request for a change that isn't a bug label Sep 17, 2020
@franklinyow
Copy link
Contributor

Move this work out of Beta EPIC

@srawlins srawlins added the P2 A bug or feature request we're likely to work on label Nov 2, 2020
@srawlins
Copy link
Member

Mixed mode is no longer supported.

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. NNBD Issues related to NNBD Release P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants