[PATCH] D80925: Fix compiler crash when trying to parse alignment argument as a constant expression.

Mon Jun 1 12:59:57 PDT 2020

rjmccall added a comment.

In D80925#2066915 <https://reviews.llvm.org/D80925#2066915>, @ABataev wrote:

> In D80925#2066728 <https://reviews.llvm.org/D80925#2066728>, @rjmccall wrote:
>
> > Narrowly this seems to fix the immediate problem, but I feel like we're in trouble if tentative parsing is changing the semantic context in ways that persist.  In particular, I'm concerned that we could end up tentatively parsing an ODR use as part of an expression and then completely discarding it, causing Sema to think that there's an ODR use later because it never sees an L2R conversion (because the expression is not actually used).  Probably the most architectural thing would be for tentative expression parsing to push a possibly-unevaluated context, and then when we claim an expression annotation token we can do the retroactive work necessary to make it an expression in the proper context.  We already have most of the logic to support that because of C99 `sizeof`, which is usually not evaluated but can be in the narrow circumstance of a VLA.
> >
> > If we do decide to solve this more narrowly, then we should audit our use of the tentative-parsing queries to make sure that we're pushing contexts consistently, and we should leave comments in places like this to make sure that maintainers understand the subtleties.
>
>
> So, you suggest to not create annot_primary_expr during tentative parsing and revert parsing completely, right?

Not creating the annotation doesn't help if we're still making Sema calls.  Also, I assume we're making the annotation token intentionally, probably to avoid re-doing the lookup.  But I do think we could recognize that we're doing this, push an unevaluated context in tentative parsing, and then call `TransformToPotentiallyEvaluated` when we see the token in expression parsing.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80925/new/

https://reviews.llvm.org/D80925