<div dir="ltr">I'd like to nominate this for the release branch, as it fixes partially-incorrect and confusing typo-correction diagnostics that weren't an issue in 3.5.</div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jan 27, 2015 at 2:01 PM, Kaelyn Takata <span dir="ltr"><<a href="mailto:rikka@google.com" target="_blank">rikka@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rikka<br>
Date: Tue Jan 27 16:01:39 2015<br>
New Revision: 227251<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=227251&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=227251&view=rev</a><br>
Log:<br>
Fix a think-o in handling ambiguous corrections for a TypoExpr.<br>
<br>
Under certain circumstances, the identifier mentioned in the diagnostic<br>
won't match the intended correction even though the replacement<br>
expression and the note pointing to the decl are both correct.<br>
Basically, the TreeTransform assumes the TypoExpr's Consumer points to<br>
the correct TypoCorrection, but the handling of typos that appear to be<br>
ambiguous from the point of view of TransformTypoExpr would cause that<br>
assumption to be violated by altering the Consumer's correction stream.<br>
This fix allows the Consumer's correction stream to be reset to the<br>
right TypoCorrection after successfully resolving the percieved ambiguity.<br>
<br>
Included is a fix to suppress correcting the RHS of an assignment to the<br>
LHS of that assignment for non-C++ code, to prevent a regression in<br>
test/SemaObjC/provisional-ivar-lookup.m.<br>
<br>
This fixes PR22297.<br>
<br>
Modified:<br>
cfe/trunk/include/clang/Sema/SemaInternal.h<br>
cfe/trunk/lib/Sema/SemaExpr.cpp<br>
cfe/trunk/lib/Sema/SemaExprCXX.cpp<br>
cfe/trunk/test/SemaCXX/typo-correction-delayed.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Sema/SemaInternal.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/SemaInternal.h?rev=227251&r1=227250&r2=227251&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/SemaInternal.h?rev=227251&r1=227250&r2=227251&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Sema/SemaInternal.h (original)<br>
+++ cfe/trunk/include/clang/Sema/SemaInternal.h Tue Jan 27 16:01:39 2015<br>
@@ -101,7 +101,7 @@ public:<br>
DeclContext *MemberContext,<br>
bool EnteringContext)<br>
: Typo(TypoName.getName().getAsIdentifierInfo()), CurrentTCIndex(0),<br>
- SemaRef(SemaRef), S(S),<br>
+ SavedTCIndex(0), SemaRef(SemaRef), S(S),<br>
SS(SS ? llvm::make_unique<CXXScopeSpec>(*SS) : nullptr),<br>
CorrectionValidator(std::move(CCC)), MemberContext(MemberContext),<br>
Result(SemaRef, TypoName, LookupKind),<br>
@@ -187,6 +187,17 @@ public:<br>
CurrentTCIndex >= ValidatedCorrections.size();<br>
}<br>
<br>
+ /// \brief Save the current position in the correction stream (overwriting any<br>
+ /// previously saved position).<br>
+ void saveCurrentPosition() {<br>
+ SavedTCIndex = CurrentTCIndex;<br>
+ }<br>
+<br>
+ /// \brief Restore the saved position in the correction stream.<br>
+ void restoreSavedPosition() {<br>
+ CurrentTCIndex = SavedTCIndex;<br>
+ }<br>
+<br>
ASTContext &getContext() const { return SemaRef.Context; }<br>
const LookupResult &getLookupResult() const { return Result; }<br>
<br>
@@ -267,6 +278,7 @@ private:<br>
<br>
SmallVector<TypoCorrection, 4> ValidatedCorrections;<br>
size_t CurrentTCIndex;<br>
+ size_t SavedTCIndex;<br>
<br>
Sema &SemaRef;<br>
Scope *S;<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=227251&r1=227250&r2=227251&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=227251&r1=227250&r2=227251&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Jan 27 16:01:39 2015<br>
@@ -9459,6 +9459,18 @@ static void checkObjCPointerIntrospectio<br>
}<br>
}<br>
<br>
+static NamedDecl *getDeclFromExpr(Expr *E) {<br>
+ if (!E)<br>
+ return nullptr;<br>
+ if (auto *DRE = dyn_cast<DeclRefExpr>(E))<br>
+ return DRE->getDecl();<br>
+ if (auto *ME = dyn_cast<MemberExpr>(E))<br>
+ return ME->getMemberDecl();<br>
+ if (auto *IRE = dyn_cast<ObjCIvarRefExpr>(E))<br>
+ return IRE->getDecl();<br>
+ return nullptr;<br>
+}<br>
+<br>
/// CreateBuiltinBinOp - Creates a new built-in binary operation with<br>
/// operator @p Opc at location @c TokLoc. This routine only supports<br>
/// built-in operations; ActOnBinOp handles overloaded operators.<br>
@@ -9496,7 +9508,13 @@ ExprResult Sema::CreateBuiltinBinOp(Sour<br>
// doesn't handle dependent types properly, so make sure any TypoExprs have<br>
// been dealt with before checking the operands.<br>
LHS = CorrectDelayedTyposInExpr(LHSExpr);<br>
- RHS = CorrectDelayedTyposInExpr(RHSExpr);<br>
+ RHS = CorrectDelayedTyposInExpr(RHSExpr, [Opc, LHS](Expr *E) {<br>
+ if (Opc != BO_Assign)<br>
+ return ExprResult(E);<br>
+ // Avoid correcting the RHS to the same Expr as the LHS.<br>
+ Decl *D = getDeclFromExpr(E);<br>
+ return (D && D == getDeclFromExpr(LHS.get())) ? ExprError() : E;<br>
+ });<br>
if (!LHS.isUsable() || !RHS.isUsable())<br>
return ExprError();<br>
}<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=227251&r1=227250&r2=227251&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=227251&r1=227250&r2=227251&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Tue Jan 27 16:01:39 2015<br>
@@ -6165,15 +6165,18 @@ public:<br>
while (!AmbiguousTypoExprs.empty()) {<br>
auto TE = AmbiguousTypoExprs.back();<br>
auto Cached = TransformCache[TE];<br>
- AmbiguousTypoExprs.pop_back();<br>
+ auto &State = SemaRef.getTypoExprState(TE);<br>
+ State.Consumer->saveCurrentPosition();<br>
TransformCache.erase(TE);<br>
if (!TryTransform(E).isInvalid()) {<br>
- SemaRef.getTypoExprState(TE).Consumer->resetCorrectionStream();<br>
+ State.Consumer->resetCorrectionStream();<br>
TransformCache.erase(TE);<br>
Res = ExprError();<br>
break;<br>
- } else<br>
- TransformCache[TE] = Cached;<br>
+ }<br>
+ AmbiguousTypoExprs.remove(TE);<br>
+ State.Consumer->restoreSavedPosition();<br>
+ TransformCache[TE] = Cached;<br>
}<br>
<br>
// Ensure that all of the TypoExprs within the current Expr have been found.<br>
<br>
Modified: cfe/trunk/test/SemaCXX/typo-correction-delayed.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/typo-correction-delayed.cpp?rev=227251&r1=227250&r2=227251&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/typo-correction-delayed.cpp?rev=227251&r1=227250&r2=227251&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/SemaCXX/typo-correction-delayed.cpp (original)<br>
+++ cfe/trunk/test/SemaCXX/typo-correction-delayed.cpp Tue Jan 27 16:01:39 2015<br>
@@ -175,3 +175,13 @@ namespace PR22250 {<br>
// expected-error@+1 {{expected ';' after top level declarator}}<br>
int getenv_s(size_t *y, char(&z)) {}<br>
}<br>
+<br>
+namespace PR22297 {<br>
+double pow(double x, double y);<br>
+struct TimeTicks {<br>
+ static void Now(); // expected-note {{'Now' declared here}}<br>
+};<br>
+void f() {<br>
+ TimeTicks::now(); // expected-error {{no member named 'now' in 'PR22297::TimeTicks'; did you mean 'Now'?}}<br>
+}<br>
+}<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>