[PATCH] D64799: [Sema] Emit diagnostics for uncorrected delayed typos at the end of TU

Richard Smith - zygoloid via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 29 16:05:15 PDT 2019


rsmith added a comment.

In D64799#1605211 <https://reviews.llvm.org/D64799#1605211>, @ilya-biryukov wrote:

> @rsmith, emitting the typos on pop expression evaluation context resulted in a bunch of failures when trying to transform the resulting errors,  see a typical stacktrace below.
>  It seems we can actually pop expression evaluation contexts between producing and correcting a typo expression.


I think I see the problem: if we have expression `E1` as a subexpression of `E2`, but `E1` has its own expression evaluation context (eg, maybe it's in a `sizeof` expression or similar), we'll now diagnose `E1`'s typos when we leave that context. That by itself seems fine, but then when we run typo correction for `E2`, we rediscover the typo from `E1` and are surprised to find that the correction information has been discarded but the `TypoExpr` is still reachable in the AST. It seems like we could handle that by tracking the already-corrected `TypoExpr`s and teaching `TransformTypos` to substitute in the known correction in this case rather than trying to pick a correction for itself.

> Would you be ok with landing the workaround as is? Alternatively, any ideas on how we can avoid this problem without extending the scope of the patch too much?

As above, I think we can probably fix this without changing too much by tracking which typos we've already resolved rather than throwing away the information about them. Failing that, I can live with this landing as-is, but delaying these diagnostics to the end of the file is going to result in a bad user experience whenever it happens -- and I suspect it'll start happening significantly more than the assert currently fails, because we'll no longer have this assertion forcing people to call `CorrectDelayedTyposInExpr` when discarding an expression.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64799





More information about the cfe-commits mailing list