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

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

rsmith added inline comments.


================
Comment at: lib/Sema/SemaExprCXX.cpp:7762-7787
     // Ensure none of the TypoExprs have multiple typo correction candidates
     // with the same edit length that pass all the checks and filters.
     // TODO: Properly handle various permutations of possible corrections when
     // there is more than one potentially ambiguous typo correction.
     // Also, disable typo correction while attempting the transform when
     // handling potentially ambiguous typo corrections as any new TypoExprs will
     // have been introduced by the application of one of the correction
----------------
dgoldman wrote:
> rsmith wrote:
> > What happens if an ambiguous `TypoExpr` is created as a result of one of the outer level transformations?
> > 
> > In that case, I think that we will try alternatives for that `TypoExpr` here, but that `TypoExpr` is not in the expression we're transforming (it was created within `RecursiveTransformLoop` and isn't part of `E`), so we're just redoing the same transformation we already did but with typo-correction disabled. This means that the transform will fail (because we hit a typo and can't correct it), so we'll accept the original set of corrections despite them being ambiguous.
> So what do you recommend here? Checking for the non-ambiguity in `RecursiveTransformLoop` itself?
Suggestion: in the recursive transform, if we find a potential ambiguity, check whether it's actually ambiguous before returning. If it is ambiguous, fail out of the entire typo-correction process: one of our tied-for-best candidates was ambiguous, so we deem the overall typo-correction process to be ambiguous. And if not, then carry on as in your current patch.



Repository:
  rC Clang

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

https://reviews.llvm.org/D62648