rsmith 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);
----------------
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).



================
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
----------------
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.


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

Reply via email to