Hi all,

This came up on StackOverflow recently:

'this' is rejected by clang in static member functions:

error: 'this' cannot be used in a static member function declaration
  static auto f() -> decltype(this);
                              ^

However, what the standard actually says is that 'this' is not allowed
except in non-static member functions (and some more bits not relevant
here). Declarations that look like member functions but aren't, not even
static ones, shouldn't allow 'this' either.

  typedef auto f() -> decltype(this);

Looking to see what clang implements, I was very surprised. There is
already perfectly functional code that rejects 'this' in 'friend'
functions, and that code is not reused for 'static' functions (and
'typedef's): instead, the checks have effectively been rewritten for
'static'.

I'm wondering, why is that? If I do attempt to re-use the existing code,
as in the attached patch, then the only changes in the test results are
actually correct, as references to non-static data members are permitted
even in 'static' functions, so long as they appear in unevaluated
expressions. There is even a FIXME comment about this in the test. But
I'm sure the current checks have been added for a good reason. Am I
overlooking some important details that are not covered by the testsuite?

Cheers,
Harald van Dijk
Reject 'this' in typedefs, allow implicit 'this'

clang initially allows 'this' in all member declarationss, and later
re-checks static member function declarations to end up rejecting them.
These re-checks end up rejecting more than intended, as implicit 'this'
references are permitted in unevaluated expressions. At the same time,
they don't end up rejecting enough, as they are only called for static
member functions, even though 'this' is equally invalid in typedefs.

By using the same checks that already exist for 'friend' functions also
when 'static' or 'typedef' is seen, and removing the re-checks, we only
give an error when we really need to.
---
 include/clang/Basic/DiagnosticSemaKinds.td         |   3 -
 include/clang/Sema/Sema.h                          |  16 ---
 lib/Parse/ParseDecl.cpp                            |   5 +-
 lib/Sema/SemaDecl.cpp                              |   7 -
 lib/Sema/SemaDeclCXX.cpp                           | 143 ---------------------
 .../CXX/expr/expr.prim/expr.prim.general/p3-0x.cpp |  14 +-
 6 files changed, 11 insertions(+), 177 deletions(-)

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 18b6fa2..8e6b77d 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4652,9 +4652,6 @@ def note_logical_not_silence_with_parens : Note<
 
 def err_invalid_this_use : Error<
   "invalid use of 'this' outside of a non-static member function">;
-def err_this_static_member_func : Error<
-  "'this' cannot be%select{| implicitly}0 used in a static member function "
-  "declaration">;
 def err_invalid_member_use_in_static_method : Error<
   "invalid use of member %0 in static member function">;
 def err_invalid_qualified_function_type : Error<
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 40f86e9..2837115 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -3974,22 +3974,6 @@ public:
   /// special member function.
   bool isImplicitlyDeleted(FunctionDecl *FD);
 
-  /// \brief Check whether 'this' shows up in the type of a static member
-  /// function after the (naturally empty) cv-qualifier-seq would be.
-  ///
-  /// \returns true if an error occurred.
-  bool checkThisInStaticMemberFunctionType(CXXMethodDecl *Method);
-
-  /// \brief Whether this' shows up in the exception specification of a static
-  /// member function.
-  bool checkThisInStaticMemberFunctionExceptionSpec(CXXMethodDecl *Method);
-
-  /// \brief Check whether 'this' shows up in the attributes of the given
-  /// static member function.
-  ///
-  /// \returns true if an error occurred.
-  bool checkThisInStaticMemberFunctionAttributes(CXXMethodDecl *Method);
-
   /// MaybeBindToTemporary - If the passed in expression has a record type with
   /// a non-trivial destructor, this will return CXXBindTemporaryExpr. Otherwise
   /// it simply returns the passed in expression.
diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp
index 9835e24..e4e6b76 100644
--- a/lib/Parse/ParseDecl.cpp
+++ b/lib/Parse/ParseDecl.cpp
@@ -5193,11 +5193,12 @@ void Parser::ParseFunctionDeclarator(Declarator &D,
       //   "pointer to cv-qualifier-seq X" between the optional cv-qualifer-seq
       //   and the end of the function-definition, member-declarator, or
       //   declarator.
-      // FIXME: currently, "static" case isn't handled correctly.
       bool IsCXX11MemberFunction =
         getLangOpts().CPlusPlus11 &&
         (D.getContext() == Declarator::MemberContext
-         ? !D.getDeclSpec().isFriendSpecified()
+         ? !D.getDeclSpec().isFriendSpecified() &&
+           D.getDeclSpec().getStorageClassSpec() != DeclSpec::SCS_typedef &&
+           D.getDeclSpec().getStorageClassSpec() != DeclSpec::SCS_static
          : D.getContext() == Declarator::FileContext &&
            D.getCXXScopeSpec().isValid() &&
            Actions.CurContext->isRecord());
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 222211d..d4dbede 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -7657,9 +7657,6 @@ bool Sema::CheckFunctionDeclaration(Scope *S, FunctionDecl *NewFD,
           }
         }
       }
-      
-      if (Method->isStatic())
-        checkThisInStaticMemberFunctionType(Method);
     }
 
     // Extra checking for C++ overloaded operators (C++ [over.oper]).
@@ -9971,10 +9968,6 @@ void Sema::ActOnFinishDelayedAttribute(Scope *S, Decl *D,
   if (TemplateDecl *TD = dyn_cast<TemplateDecl>(D))
     D = TD->getTemplatedDecl();
   ProcessDeclAttributeList(S, D, Attrs.getList());  
-  
-  if (CXXMethodDecl *Method = dyn_cast_or_null<CXXMethodDecl>(D))
-    if (Method->isStatic())
-      checkThisInStaticMemberFunctionAttributes(Method);
 }
 
 
diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index 820f57f..8eacb86 100644
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -12604,149 +12604,6 @@ void Sema::CheckDelegatingCtorCycles() {
     (*CI)->setInvalidDecl();
 }
 
-namespace {
-  /// \brief AST visitor that finds references to the 'this' expression.
-  class FindCXXThisExpr : public RecursiveASTVisitor<FindCXXThisExpr> {
-    Sema &S;
-    
-  public:
-    explicit FindCXXThisExpr(Sema &S) : S(S) { }
-    
-    bool VisitCXXThisExpr(CXXThisExpr *E) {
-      S.Diag(E->getLocation(), diag::err_this_static_member_func)
-        << E->isImplicit();
-      return false;
-    }
-  };
-}
-
-bool Sema::checkThisInStaticMemberFunctionType(CXXMethodDecl *Method) {
-  TypeSourceInfo *TSInfo = Method->getTypeSourceInfo();
-  if (!TSInfo)
-    return false;
-  
-  TypeLoc TL = TSInfo->getTypeLoc();
-  FunctionProtoTypeLoc ProtoTL = TL.getAs<FunctionProtoTypeLoc>();
-  if (!ProtoTL)
-    return false;
-  
-  // C++11 [expr.prim.general]p3:
-  //   [The expression this] shall not appear before the optional 
-  //   cv-qualifier-seq and it shall not appear within the declaration of a 
-  //   static member function (although its type and value category are defined
-  //   within a static member function as they are within a non-static member
-  //   function). [ Note: this is because declaration matching does not occur
-  //  until the complete declarator is known. - end note ]
-  const FunctionProtoType *Proto = ProtoTL.getTypePtr();
-  FindCXXThisExpr Finder(*this);
-  
-  // If the return type came after the cv-qualifier-seq, check it now.
-  if (Proto->hasTrailingReturn() &&
-      !Finder.TraverseTypeLoc(ProtoTL.getResultLoc()))
-    return true;
-
-  // Check the exception specification.
-  if (checkThisInStaticMemberFunctionExceptionSpec(Method))
-    return true;
-  
-  return checkThisInStaticMemberFunctionAttributes(Method);
-}
-
-bool Sema::checkThisInStaticMemberFunctionExceptionSpec(CXXMethodDecl *Method) {
-  TypeSourceInfo *TSInfo = Method->getTypeSourceInfo();
-  if (!TSInfo)
-    return false;
-  
-  TypeLoc TL = TSInfo->getTypeLoc();
-  FunctionProtoTypeLoc ProtoTL = TL.getAs<FunctionProtoTypeLoc>();
-  if (!ProtoTL)
-    return false;
-  
-  const FunctionProtoType *Proto = ProtoTL.getTypePtr();
-  FindCXXThisExpr Finder(*this);
-
-  switch (Proto->getExceptionSpecType()) {
-  case EST_Uninstantiated:
-  case EST_Unevaluated:
-  case EST_BasicNoexcept:
-  case EST_DynamicNone:
-  case EST_MSAny:
-  case EST_None:
-    break;
-    
-  case EST_ComputedNoexcept:
-    if (!Finder.TraverseStmt(Proto->getNoexceptExpr()))
-      return true;
-    
-  case EST_Dynamic:
-    for (FunctionProtoType::exception_iterator E = Proto->exception_begin(),
-         EEnd = Proto->exception_end();
-         E != EEnd; ++E) {
-      if (!Finder.TraverseType(*E))
-        return true;
-    }
-    break;
-  }
-
-  return false;
-}
-
-bool Sema::checkThisInStaticMemberFunctionAttributes(CXXMethodDecl *Method) {
-  FindCXXThisExpr Finder(*this);
-
-  // Check attributes.
-  for (Decl::attr_iterator A = Method->attr_begin(), AEnd = Method->attr_end();
-       A != AEnd; ++A) {
-    // FIXME: This should be emitted by tblgen.
-    Expr *Arg = 0;
-    ArrayRef<Expr *> Args;
-    if (GuardedByAttr *G = dyn_cast<GuardedByAttr>(*A))
-      Arg = G->getArg();
-    else if (PtGuardedByAttr *G = dyn_cast<PtGuardedByAttr>(*A))
-      Arg = G->getArg();
-    else if (AcquiredAfterAttr *AA = dyn_cast<AcquiredAfterAttr>(*A))
-      Args = ArrayRef<Expr *>(AA->args_begin(), AA->args_size());
-    else if (AcquiredBeforeAttr *AB = dyn_cast<AcquiredBeforeAttr>(*A))
-      Args = ArrayRef<Expr *>(AB->args_begin(), AB->args_size());
-    else if (ExclusiveLockFunctionAttr *ELF 
-               = dyn_cast<ExclusiveLockFunctionAttr>(*A))
-      Args = ArrayRef<Expr *>(ELF->args_begin(), ELF->args_size());
-    else if (SharedLockFunctionAttr *SLF 
-               = dyn_cast<SharedLockFunctionAttr>(*A))
-      Args = ArrayRef<Expr *>(SLF->args_begin(), SLF->args_size());
-    else if (ExclusiveTrylockFunctionAttr *ETLF
-               = dyn_cast<ExclusiveTrylockFunctionAttr>(*A)) {
-      Arg = ETLF->getSuccessValue();
-      Args = ArrayRef<Expr *>(ETLF->args_begin(), ETLF->args_size());
-    } else if (SharedTrylockFunctionAttr *STLF
-                 = dyn_cast<SharedTrylockFunctionAttr>(*A)) {
-      Arg = STLF->getSuccessValue();
-      Args = ArrayRef<Expr *>(STLF->args_begin(), STLF->args_size());
-    } else if (UnlockFunctionAttr *UF = dyn_cast<UnlockFunctionAttr>(*A))
-      Args = ArrayRef<Expr *>(UF->args_begin(), UF->args_size());
-    else if (LockReturnedAttr *LR = dyn_cast<LockReturnedAttr>(*A))
-      Arg = LR->getArg();
-    else if (LocksExcludedAttr *LE = dyn_cast<LocksExcludedAttr>(*A))
-      Args = ArrayRef<Expr *>(LE->args_begin(), LE->args_size());
-    else if (ExclusiveLocksRequiredAttr *ELR 
-               = dyn_cast<ExclusiveLocksRequiredAttr>(*A))
-      Args = ArrayRef<Expr *>(ELR->args_begin(), ELR->args_size());
-    else if (SharedLocksRequiredAttr *SLR 
-               = dyn_cast<SharedLocksRequiredAttr>(*A))
-      Args = ArrayRef<Expr *>(SLR->args_begin(), SLR->args_size());
-
-    if (Arg && !Finder.TraverseStmt(Arg))
-      return true;
-    
-    for (unsigned I = 0, N = Args.size(); I != N; ++I) {
-      if (!Finder.TraverseStmt(Args[I]))
-        return true;
-    }
-  }
-  
-  return false;
-}
-
 void
 Sema::checkExceptionSpecification(ExceptionSpecificationType EST,
                                   ArrayRef<ParsedType> DynamicExceptions,
diff --git a/test/CXX/expr/expr.prim/expr.prim.general/p3-0x.cpp b/test/CXX/expr/expr.prim/expr.prim.general/p3-0x.cpp
index fd90482..892acfb 100644
--- a/test/CXX/expr/expr.prim/expr.prim.general/p3-0x.cpp
+++ b/test/CXX/expr/expr.prim/expr.prim.general/p3-0x.cpp
@@ -105,19 +105,21 @@ namespace PR15290 {
   }
 }
 
-namespace Static {
+namespace StaticTypedef {
   struct X1 {
     int m;
-    // FIXME: This should be accepted.
-    static auto f() -> decltype(m); // expected-error{{'this' cannot be implicitly used in a static member function declaration}}
-    static auto g() -> decltype(this->m); // expected-error{{'this' cannot be used in a static member function declaration}}
+    static auto f() -> decltype(m);
+    static auto g() -> decltype(this->m); // expected-error{{invalid use of 'this' outside of a non-static member function}}
 
     static int h();
     
-    static int i() noexcept(noexcept(m + 2)); // expected-error{{'this' cannot be implicitly used in a static member function declaration}}
+    static int i() noexcept(noexcept(m + 2));
+
+    typedef auto j() -> decltype(m);
+    typedef auto k() -> decltype(this->m); // expected-error{{invalid use of 'this' outside of a non-static member function}}
   };
 
-  auto X1::h() -> decltype(m) { return 0; } // expected-error{{'this' cannot be implicitly used in a static member function declaration}}
+  auto X1::h() -> decltype(m) { return 0; }
 
   template<typename T>
   struct X2 {
-- 
1.8.5.2

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to