Adjust the constant expression rules for lists, maps, and conditional expressions#4313
Conversation
|
Responded to the comments, PTAL. |
|
I made the change I suggested, such that collection literals follow the same approach as all other constructs. |
lrhn
left a comment
There was a problem hiding this comment.
Add entry in changelog saying why this change was made.
|
Just discussed this with @leafpetersen who noticed that the rules are redundant in certain ways: Constant object expressions and constant collection literals already have a set of rules in the sections specific to those constructs, and everything in the 'Constants' section was redundant. So they now just say, essentially, "a constant object expression is a potentially constant and constant expression" and similarly for the other forms. This preserves the property that the set of constant expressions is a subset of the set of potentially constant expressions. We can (and probably should) add a rule that if |
|
@lrhn, PTAL, the latest changes are non-trivial. |
|
The updated specification of what it takes for a conditional expression to be constant is the already implemented behavior. An example: The current implementation of the analyzer as well as the CFE (Dart SDK 3.8.0-250.0.dev and Flutter SDK 3.31.0-1.0.pre.363) reject the following as an error: var v = 'World';
void main() {
const c = 2 > 1 ? 'Hello' : v; // Error!
}This clearly shows that the current wording of the rule about constant conditional expressions is not implemented, the actual implementation is more strict (it requires the unused branch to be a constant expression, and the spec does not). Checking visitConditionalExpression of the relevant visitor, we can see that the implementation in the analyzer checks exactly the condition which is added by this PR (that is, the new behavior is already the implemented behavior in the analyzer). @chloestefantsova found the reason why the same error is reported by the CFE, and it turns out to be implemented in a very early phase of the compilation (in the BodyBuilder) where it is checked that every expression in a constant context is potentially constant. In both cases, this shows that the tools implement the approach which is specified in this PR (namely: a conditional expression is not constant if it is not potentially constant), and hence there is no breaking change, and not even an implementation effort. |
See #4311 for background information.
This PR changes the rule about constant expressions of the form
b ? e1 : e2such that it is required that every constant expression of this form is also a potentially constant expression. The point is that this maintains the subset relationship "every constant expression is also a potentially constant expression", which is otherwise maintained in all cases. Also, this implies that the checks that are applied to conditional expressions are slightly stronger than they have been so far.The PR also changes the rules about constant collection literals and constant object expressions (
const C()): They used to have a special part about being a potentially constant expression, plus some constraints on elements/arguments. However, this is already specified in the sections about collection literals / constant object expressions, so there's no need to repeat that (incompletely) in the 'Constants' section. So this PR simplifies those rules to eliminate the redundancy.Based on the discussion in issue 4311 we might want to add a rule along the lines of "an expression that contains a formal parameter of an enclosing constructor declaration is not constant". This PR hasn't done it, but it could be added.
We need to consider breakage:
The rule about conditional expressions is in principle breaking, because it adds an extra constraint on some constant expressions. However, I suspect that no code is actually broken because the requirements for being a constant expression would also imply the requirements for being a potentially constant expression.
The simplification of the rules about collection literals are non-breaking because everything which has been deleted is already specified as requirements in other sections.