Check if MemberExpr arguments are type-dependent.

[mattd]

Summary

Fixes: PR28290

When checking an argument list, arguments from the templated class instance, were
returning 'is dependent' based on the 'this' pointer. In that case, arguments were
being marked dependent, and name lookup was delayed until template instantiation. This
had the side-effect of -fdelayed-template-parsing in certain cases
(attached test case), when that should not have been the case, especially
since that flag was never passed.

According to the standard the referenced member's type needs
to be checked:

[temp.dep.expr]p5:
A class member access expression (5.2.5) is type-dependent if the expression
refers to a member of the current instantiation and the type of the referenced
member is dependent, or the class member access expression refers to a member
of an unknown specialization.

This change decides if the argument belongs to a MemberExpr. If so, then the
type of the expression is checked if it is dependent or not.

[erik.pilkington]

Hi, thanks for working on this!
Why don't we just set the TypeDependent bit to false when building the underlying MemberExpr here? Based on the section of the standard you quoted that node shouldn't be considered type-dependent, but it is by clang. I think this patch is just hiding that underlying problem by fishing out the real type-dependence after constructing the MemberExpr when really that information should be included in the MemberExpr to begin with.
Thanks,
Erik

[mattd]

Thanks for the comments Eli! I agree, it makes sense to flag certain expressions to be type dependent at the earliest time of compilation. Clang already does this, as the Value/Type dependence is established upon Expr construction from MemberExpr construction.

As we have discussed, in the case of MemberExpr, Clang bases dependence on the 'this' pointer, which can be seen in the MemberExpr constructor. And from the attached test case, this is decision is not correct. From what we noted, member access expressions can base their dependence on the type of the member.

However, after trying to propagate this change sooner rather than later (in the MemberExpr ctor), I realized that I might have been changing language semantics. One thing, in particular, is how we handle address-of expressions. Clang predicates them, internally, on type-dependence. From that perspective, we will never try to bind a address-of expression within a template definition, if the address-of is for a member variable. The bind will only occur during template instantiation. Another case we have to be aware of are MemberExpr where the member is a CXXMethodExpr, which would be dependent given the implicit 'this' parameter; that's correct, but I don't want to break that either.

[efriedma]

The part which seems to be throwing you off is "or the class member access expression refers to a member of an unknown specialization". A "member of an unknown specialization" is defined in [temp.dep.type] in the standard.

lib/AST/Expr.cpp

bool Expr::isImplicitCXXThis() const {
   }
   
   if (const CXXThisExpr *This = dyn_cast<CXXThisExpr>(E))
     return This->isImplicit();
   
   return false;
 }
 
+/// \brief Returns true if the expression is a MemberExpr
+/// with a dependent type.
+static bool isDependentMember(const Expr *E)
+{
+  if (const auto ME = dyn_cast_or_null<MemberExpr>(E))
+    if (const auto Member = ME->getMemberDecl())
+      return Member->getType()->isDependentType();
+
+  return false;
+}
+
 /// hasAnyTypeDependentArguments - Determines if any of the expressions
 /// in Exprs is type-dependent.
 bool Expr::hasAnyTypeDependentArguments(ArrayRef<Expr *> Exprs) {
-  for (unsigned I = 0; I < Exprs.size(); ++I)
-    if (Exprs[I]->isTypeDependent())
+  for (const auto E : Exprs) {
+    if (isa<MemberExpr>(E)) {
+      if (isDependentMember(E))
+        return true;
+    } else if (E->isTypeDependent())
       return true;
+  }
 
   return false;
 }
 
 bool Expr::isConstantInitializer(ASTContext &Ctx, bool IsForRef,
                                  const Expr **Culprit) const {
   // This function is attempting whether an expression is an initializer
   // which can be evaluated at compile-time. It very closely parallels

test/CXX/class.access/class.access.dcl/p1.cpp

 #if __cplusplus <= 199711L
     // expected-warning@-2 {{access declarations are deprecated; use using declarations instead}}
 #else
     // expected-error@-4 {{ISO C++11 does not allow access declarations; use using declarations instead}}
 #endif
   };
 }
 
-struct Opaque0 {};
+struct Opaque0 {}; // expected-note-re 1-2 {{candidate constructor {{.*}}}}
 
 namespace test1 {
   struct A {
     typedef int type;
     struct hiding {}; // expected-note {{previous use is here}}
     Opaque0 hiding;
     union { double union_member; };
     enum tagname { enumerator };
void test3() {
       char array[sizeof(union_member) == sizeof(double) ? 1 : -1];
     }
 
     void test4() {
       enum tagname _ = enumerator;
     }
 
     void test5() {
-      Opaque0 _ = hiding;
+      Opaque0 _ = hiding; // expected-error {{no viable conversion from 'int' to 'Opaque0'}}
     }
   };
 }
 
 namespace test3 {
   struct hiding {};
 
   template <class T> struct A {

test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp

namespace test0 {
   class Test0 {
     using NonClass::type; // expected-error {{not a class}}
     using NonClass::hiding; // expected-error {{not a class}}
     using NonClass::union_member; // expected-error {{not a class}}
     using NonClass::enumerator; // expected-error {{not a class}}
   };
 }
 
-struct Opaque0 {};
+struct Opaque0 {}; // expected-note-re 1-2{{candidate constructor {{.*}}}}
 
 namespace test1 {
   struct A {
     typedef int type;
     struct hiding {}; // expected-note {{previous use is here}}
     Opaque0 hiding;
     union { double union_member; };
     enum tagname { enumerator };
void test3() {
       char array[sizeof(union_member) == sizeof(double) ? 1 : -1];
     }
 
     void test4() {
       enum tagname _ = enumerator;
     }
 
     void test5() {
-      Opaque0 _ = hiding;
+      Opaque0 _ = hiding; // expected-error {{no viable conversion from 'int' to 'Opaque0'}}
     }
   };
 }
 
 namespace test3 {
   struct hiding {};
 
   template <class T> struct A {

test/SemaTemplate/template-with-invalid-decl.cpp

+// RUN: %clang_cc1 -verify %s
+
+class UBS;
+
+template <bool bCS> struct FIBH 
+{
+  FIBH() { GDN(BS); } // expected-error {{use of undeclared identifier 'GDN'}}
+  class UBS* BS;
+};
+
+extern void foo()
+{
+  FIBH<false> IBH;
+}
+
+extern void GDN(UBS* BS);