dgoldman marked 2 inline comments as done.
dgoldman added inline comments.
================
Comment at: lib/Sema/SemaExprCXX.cpp:7713-7714
+ // Add the newly created typos to the TypoExprs list, even if they
+ // failed to apply. This allows them to be reaped although they won't
+ // emit any diagnostic.
+ SavedTypoExprs.set_union(TypoExprs);
----------------
rsmith wrote:
> What prevents diagnostics from being emitted for `TypoExpr`s we create for an
> outer transform that we end up discarding? Eg, in:
>
> ```
> struct Y {};
> struct Z { int value; };
> struct A {
> Y get_me_a_Y();
> };
> struct B {
> Z get_me_a_Z();
> };
> A make_an_A();
> B make_a_B();
> int f() {
> return make_an_E().get_me_a_Z().value;
> }
> ```
>
> I'm concerned that we will:
> * try correcting `make_me_an_E` (`TypoExpr` `T0`) to `make_me_an_A` (because
> that correction has the minimum edit distance) and discover that there is no
> `get_me_a_Z` member, creating a new `TypoExpr` `T1`, and recurse:
> * try correcting `get_me_a_Z` (`TypoExpr` `T1`) to `get_me_a_Y` and
> discover that there is no `value` member
> * no correction works: bail out of recursive step
> * try correcting `make_me_an_E` (`TypoExpr` `T0`) to `make_me_a_B` and
> succeed
>
> But now we still have `T1` in our list of `TypoExpr`s, and will presumably
> diagnose it (and take action below if it turned out to be ambiguous etc).
>
Fixed by discarding the failed `TypoExpr`s
================
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:
> 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?
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
