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