dgoldman marked an inline comment as done.
dgoldman 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
----------------
rsmith wrote:
> 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.
>
Will submit a follow up patch to handle this, I think I'll likely rewrite the
ambiguity handling to remove the `AmbiguousTypoExprs` bit.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62648/new/
https://reviews.llvm.org/D62648
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
