[PATCH] D62648: [Sema][Typo] Fix assertion failure for expressions with multiple typos

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

I don't think you need to change the `TreeTranform` base class to support this; `TreeTransform::TransformExpr` is an extension point which you can override from `TransformTypos` to inject the custom logic you need. But I also don't think this `TreeTransform::TransformExpr` approach works in general anyway, as noted below.



================
Comment at: lib/Sema/TreeTransform.h:3485-3489
+  // If the transformed Expr is valid, check if it's a TypoExpr so we can keep
+  // track of them. Otherwise, if the transform result is invalid, clear any
+  // TypoExprs that might have been created recursively (TODO: verify that
+  // this can happen in practice here instead of via an error trap).
+  if (Res.isUsable()) {
----------------
It's not safe to assume that all created `TypoExpr`s will be produced by `TransformExpr`. We might (for example) produce a new `TypoExpr` for the callee expression while transforming a `CallExpr`, and you won't catch that here.

It would seem cleaner and more robust to me to collect the typos produced during typo correction from within `Sema::createDelayedTypo`. (Eg, set some state on `Sema` for the duration of typo correction, and use that state to communicate any created typos back from `createDelayedTypo` to typo correction.)


Repository:
  rC Clang

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

https://reviews.llvm.org/D62648