[cfe-commits] r155580 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/Sema/SemaExprMember.cpp test/FixIt/fixit.cpp test/SemaCXX/arrow-operator.cpp

[Douglas Gregor]

On Apr 25, 2012, at 12:49 PM, Kaelyn Uhrain <rikka at google.com> wrote:

> Author: rikka
> Date: Wed Apr 25 14:49:54 2012
> New Revision: 155580
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=155580&view=rev
> Log:
> Add an error message with fixit hint for changing '.' to '->'.
> 
> This is mainly for attempting to recover in cases where a class provides
> a custom operator-> and a '.' was accidentally used instead of '->' when
> accessing a member of the object returned by the current object's
> operator->.
> 
> Modified:
>    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>    cfe/trunk/include/clang/Sema/Sema.h
>    cfe/trunk/lib/Sema/SemaExprMember.cpp
>    cfe/trunk/test/FixIt/fixit.cpp
>    cfe/trunk/test/SemaCXX/arrow-operator.cpp
> 
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=155580&r1=155579&r2=155580&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Apr 25 14:49:54 2012
> @@ -3641,6 +3641,8 @@
> 
> def err_typecheck_incomplete_tag : Error<"incomplete definition of type %0">;
> def err_no_member : Error<"no member named %0 in %1">;
> +def err_no_member_overloaded_arrow : Error<
> +  "no member named %0 in %1; did you mean to use '->' instead of '.'?">;
> 
> def err_member_not_yet_instantiated : Error<
>   "no member %0 in %1; it has not yet been instantiated">;
> 
> Modified: cfe/trunk/include/clang/Sema/Sema.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=155580&r1=155579&r2=155580&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Sema/Sema.h (original)
> +++ cfe/trunk/include/clang/Sema/Sema.h Wed Apr 25 14:49:54 2012
> @@ -2703,6 +2703,18 @@
>                                 const DeclarationNameInfo &NameInfo,
>                                 const TemplateArgumentListInfo *TemplateArgs);
> 
> +  // This struct is for use by ActOnMemberAccess to allow
> +  // BuildMemberReferenceExpr to be able to reinvoke ActOnMemberAccess after
> +  // changing the access operator from a '.' to a '->' (to see if that is the
> +  // change needed to fix an error about an unknown member, e.g. when the class
> +  // defines a custom operator->).
> +  struct ActOnMemberAccessExtraArgs {
> +    Scope *S;
> +    UnqualifiedId &Id;
> +    Decl *ObjCImpDecl;
> +    bool HasTrailingLParen;
> +  };
> +
>   ExprResult BuildMemberReferenceExpr(Expr *Base, QualType BaseType,
>                                       SourceLocation OpLoc, bool IsArrow,
>                                       const CXXScopeSpec &SS,
> @@ -2710,7 +2722,8 @@
>                                       NamedDecl *FirstQualifierInScope,
>                                       LookupResult &R,
>                                  const TemplateArgumentListInfo *TemplateArgs,
> -                                      bool SuppressQualifierCheck = false);
> +                                      bool SuppressQualifierCheck = false,
> +                                     ActOnMemberAccessExtraArgs *ExtraArgs = 0);
> 
>   ExprResult PerformMemberExprBaseConversion(Expr *Base, bool IsArrow);
>   ExprResult LookupMemberExpr(LookupResult &R, ExprResult &Base,
> 
> Modified: cfe/trunk/lib/Sema/SemaExprMember.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprMember.cpp?rev=155580&r1=155579&r2=155580&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaExprMember.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExprMember.cpp Wed Apr 25 14:49:54 2012
> @@ -813,8 +813,9 @@
>                                SourceLocation TemplateKWLoc,
>                                NamedDecl *FirstQualifierInScope,
>                                LookupResult &R,
> -                         const TemplateArgumentListInfo *TemplateArgs,
> -                               bool SuppressQualifierCheck) {
> +                               const TemplateArgumentListInfo *TemplateArgs,
> +                               bool SuppressQualifierCheck,
> +                               ActOnMemberAccessExtraArgs *ExtraArgs) {
>   QualType BaseType = BaseExprType;
>   if (IsArrow) {
>     assert(BaseType->isPointerType());
> @@ -835,6 +836,32 @@
>                        ? computeDeclContext(SS, false)
>                        : BaseType->getAs<RecordType>()->getDecl());
> 
> +    if (ExtraArgs) {
> +      ExprResult RetryExpr;
> +      if (!IsArrow && BaseExpr) {
> +        SFINAETrap Trap(*this);

I think you want

	SFINAETrap Trap(*this, true);

to SFINAE'ify access checking as well.

> +        ParsedType ObjectType;
> +        bool MayBePseudoDestructor = false;
> +        RetryExpr = ActOnStartCXXMemberReference(getCurScope(), BaseExpr,
> +                                                 OpLoc, tok::arrow, ObjectType,
> +                                                 MayBePseudoDestructor);
> +        if (RetryExpr.isUsable() && !Trap.hasErrorOccurred()) {
> +          CXXScopeSpec TempSS(SS);
> +          RetryExpr = ActOnMemberAccessExpr(
> +              ExtraArgs->S, RetryExpr.get(), OpLoc, tok::arrow, TempSS,
> +              TemplateKWLoc, ExtraArgs->Id, ExtraArgs->ObjCImpDecl,
> +              ExtraArgs->HasTrailingLParen);
> +        }

It makes me a bit nervous to be recursing like this, because it would be very easy for a later refactor to introduce a "->" -> to "." replacement and get into an infinite loop. I'd rather that we at least pass down a bit that says "we're recovering; don't try to recover further".

Thanks for working on this!

	- Doug