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

Kaelyn Takata rikka at google.com
Tue Jan 27 14:39:47 PST 2015


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.

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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150127/91d79990/attachment.html>


More information about the cfe-commits mailing list