r227251 - Fix a think-o in handling ambiguous corrections for a TypoExpr.

[Hans Wennborg]

On Tue, Jan 27, 2015 at 2:39 PM, Kaelyn Takata <rikka at google.com> wrote:
> 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.

Merged in r227266.

Thanks,
Hans


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