mgehre created this revision.
mgehre added reviewers: klimek, aaron.ballman, rsmith.
mgehre added a subscriber: cfe-commits.

A MemberExpr is non-dependent if
1) it is a member of the current instantiation
2) the member is non-dependent

We check 1) by asserting that the base of the
MemberExpr is a CXXThisExpr. In addition,
the parent of the member needs to be the current
class or a base of it. (It can happen that the member
decl resolves to a member of an outer class, and later
on Sema prints an error about it. In this case, we keep
it type-dependent, and the error will only appear if it is
ODR-used.)

This changes makes clang reject certain source code that it previously accepted,
namely invalid use of members in functions that are never specialized
or ODR-used. The changes to the tests show examples of this.

https://reviews.llvm.org/D22476

Files:
  lib/AST/Expr.cpp
  lib/Sema/SemaOverload.cpp
  test/Analysis/stack-addr-ps.cpp
  test/CXX/class.access/class.access.dcl/p1.cpp
  test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
  test/CXX/drs/dr3xx.cpp
  test/SemaTemplate/dependent-names.cpp
  test/SemaTemplate/enum-argument.cpp
  test/SemaTemplate/member-access-expr.cpp

Index: test/SemaTemplate/member-access-expr.cpp
===================================================================
--- test/SemaTemplate/member-access-expr.cpp
+++ test/SemaTemplate/member-access-expr.cpp
@@ -154,9 +154,7 @@
   template <class T> class Derived : public Base<T> {
     A *field;
     void get(B **ptr) {
-      // It's okay if at some point we figure out how to diagnose this
-      // at instantiation time.
-      *ptr = field;
+      *ptr = field; // expected-error{{assigning to 'test6::B *' from incompatible type 'test6::A *'}}
     }
   };
 }
Index: test/SemaTemplate/enum-argument.cpp
===================================================================
--- test/SemaTemplate/enum-argument.cpp
+++ test/SemaTemplate/enum-argument.cpp
@@ -31,7 +31,7 @@
     unsigned long long bitfield : e0;
 
     void f(int j) {
-      bitfield + j;
+      (void)(bitfield + j);
     }
   };
 }
Index: test/SemaTemplate/dependent-names.cpp
===================================================================
--- test/SemaTemplate/dependent-names.cpp
+++ test/SemaTemplate/dependent-names.cpp
@@ -274,7 +274,7 @@
       int e[10];
     };
     void g() {
-      S<int>().f(); // expected-note {{here}}
+      S<int>().f();
     }
   }
 
Index: test/CXX/drs/dr3xx.cpp
===================================================================
--- test/CXX/drs/dr3xx.cpp
+++ test/CXX/drs/dr3xx.cpp
@@ -1167,8 +1167,8 @@
 namespace dr390 { // dr390: yes
   template<typename T>
   struct A {
-    A() { f(); } // expected-warning {{call to pure virt}}
-    virtual void f() = 0; // expected-note {{here}}
+    A() { f(); } // expected-warning {{call to pure virt}} expected-warning {{call to pure virt}}
+    virtual void f() = 0; // expected-note {{here}} expected-note {{here}}
     virtual ~A() = 0;
   };
   template<typename T> A<T>::~A() { T::error; } // expected-error {{cannot be used prior to}}
Index: test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
===================================================================
--- test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
+++ test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp
@@ -29,7 +29,10 @@
   };
 }
 
-struct Opaque0 {};
+struct Opaque0 {}; // expected-note {{no known conversion}}
+#if __cplusplus >= 201103L
+//  expected-note@-2 {{no known conversion}}
+#endif
 
 namespace test1 {
   struct A {
@@ -112,7 +115,7 @@
     }
 
     void test5() {
-      Opaque0 _ = hiding;
+      Opaque0 _ = hiding; // expected-error {{no viable conversion from 'int' to 'Opaque0'}}
     }
   };
 }
Index: test/CXX/class.access/class.access.dcl/p1.cpp
===================================================================
--- test/CXX/class.access/class.access.dcl/p1.cpp
+++ test/CXX/class.access/class.access.dcl/p1.cpp
@@ -56,7 +56,10 @@
   };
 }
 
-struct Opaque0 {};
+struct Opaque0 {}; // expected-note {{candidate constructor}}
+#if __cplusplus >= 201103L
+// expected-note@-2 {{candidate constructor}}
+#endif
 
 namespace test1 {
   struct A {
@@ -196,7 +199,7 @@
     }
 
     void test5() {
-      Opaque0 _ = hiding;
+      Opaque0 _ = hiding; // expected-error {{no viable conversion from 'int' to 'Opaque0'}}
     }
   };
 }
Index: test/Analysis/stack-addr-ps.cpp
===================================================================
--- test/Analysis/stack-addr-ps.cpp
+++ test/Analysis/stack-addr-ps.cpp
@@ -84,8 +84,8 @@
 struct TS {
   int *get();
   int *m() {
-    int *&x = get();
-    return x;
+    int * const &x = get(); // expected-note {{binding reference variable 'x' here}}
+    return x; // expected-warning {{returning address of local temporary object}}
   }
 };
 
Index: lib/Sema/SemaOverload.cpp
===================================================================
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -4881,7 +4881,7 @@
     assert(FromClassification.isLValue());
   }
 
-  assert(FromType->isRecordType());
+  assert(FromType->isRecordType() || FromType->getTypeClass() == Type::InjectedClassName);
 
   // C++0x [over.match.funcs]p4:
   //   For non-static member functions, the type of the implicit object
Index: lib/AST/Expr.cpp
===================================================================
--- lib/AST/Expr.cpp
+++ lib/AST/Expr.cpp
@@ -1421,17 +1421,40 @@
   MemberExpr *E = new (Mem)
       MemberExpr(base, isarrow, OperatorLoc, memberdecl, nameinfo, ty, vk, ok);
 
+
+  if (E->isTypeDependent() && isa<CXXThisExpr>(base)) {
+    bool MemberOfCurrentInstantiation = true;
+
+    auto *BaseClass = base->getType()->getPointeeType()->getAsCXXRecordDecl();
+    assert(BaseClass && "Type of CXXThisExpr must be a pointer to CXXRecordDecl");
+
+    if (auto *Method = dyn_cast<CXXMethodDecl>(memberdecl)) {
+      for (auto *EnableIf : Method->specific_attrs<EnableIfAttr>()) {
+        if (EnableIf->getCond()->isTypeDependent()) {
+          MemberOfCurrentInstantiation = false;
+          break;
+        }
+      }
+
+      const auto *Parent = Method->getParent();
+      if (BaseClass != Parent && !BaseClass->isDerivedFrom(Parent))
+        MemberOfCurrentInstantiation = false;
+
+    } else if (auto *Field = dyn_cast<FieldDecl>(memberdecl)) {
+      const auto *Parent = dyn_cast<CXXRecordDecl>(Field->getParent());
+      if (!Parent || (BaseClass != Parent && !BaseClass->isDerivedFrom(Parent)))
+        MemberOfCurrentInstantiation = false;
+    } else {
+      MemberOfCurrentInstantiation = false;
+    }
+
+    // A member of the current instantiation is only type dependent if
+    // the field is type dependent. (14.6.2.1 [temp.dep.type])
+    if (MemberOfCurrentInstantiation && !memberdecl->getType()->isDependentType())
+      E->setTypeDependent(false);
+  }
+
   if (hasQualOrFound) {
-    // FIXME: Wrong. We should be looking at the member declaration we found.
-    if (QualifierLoc && QualifierLoc.getNestedNameSpecifier()->isDependent()) {
-      E->setValueDependent(true);
-      E->setTypeDependent(true);
-      E->setInstantiationDependent(true);
-    } 
-    else if (QualifierLoc && 
-             QualifierLoc.getNestedNameSpecifier()->isInstantiationDependent()) 
-      E->setInstantiationDependent(true);
-    
     E->HasQualifierOrFoundDecl = true;
 
     MemberExprNameQualifier *NQ =
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to