[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

[Kaelyn Uhrain]

On Mon, Apr 30, 2012 at 5:18 PM, Douglas Gregor <dgregor at apple.com> wrote:

>
> 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.
>

Good point. Added this in r155871 (screwed it up in r155870 because I
managed to read the email wrong and saw a "false" instead of a "true").


> > +        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".
>

As long as ActOnMemberAccessExpr doesn't pass false for IsArrow when
calling BuildMemberReferenceExpr when ActOnMemberAccessExpr was called with
tok::arrow as the OpKind, it won't recurse more than once. I think the
introduction of a "->" to "." replacement within ActOnMemberAccessExpr
would be time enough to introduce yet another flag to indicate whether the
recovery has already been attempted (this wouldn't be the first place where
introducing a particular kind of replacement for recovering from errors
would introduce an infinite loop).


> Thanks for working on this!
>

You're welcome! :)

Cheers,
Kaelyn
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120430/5ea22c18/attachment.html>