[clang] Don't emit "no member" diagnostic if the lookup fails on an invalid record decl.

[hokein]

Summary

The "no member" diagnostic is likely bogus.

[sammccall]

This seems like fixing a fairly specific case of a general problem: if the base class is invalid I would expect to see similar problems in other places (e.g. unqualified lookup from inside the class).
But it fixes a diagnostic the libc++ folks think is important...

clang/lib/Sema/SemaExpr.cpp

2585

I found the wording of this example a bit hard to follow.

What about "e.g. if a class is invalid because it's derived from an invalid base class, then missing members were likely supposed to be inherited".

I'm actually not convinced this is merely an example - is there any other reason for invalidity other than invalid base class where this logic applies?

2587

nit: will fails -> will fail

2589

this is a fairly general check, but the reasoning really only applies to invalid bases.
If there's an easy way to check for that specifically instead, that'd be nice, but fine if not.

[hokein]

Fix a comment.

clang/lib/Sema/SemaExpr.cpp

2585

I'm actually not convinced this is merely an example - is there any other reason for invalidity other than invalid base class where this logic applies?

I suppose there are other examples, but failed to find one.
Yeah, there are other reason that can cause an invalid RecordDecl (e.g. an invalid field), but it doesn't run into this code path. This code seems to be applied during template instantiation, I think it should be fine.

2589

we don't record that the base specifier is not attached due to the invalidity, we could introduce a bit to do that, not sure it is worth for this particular case, might be worth to reconsider if we see large regressions (currently, we don't have any regressions in existing tests).

clang/lib/Sema/SemaExpr.cpp

ExprResult Sema::BuildQualifiedDeclarationNameExpr(
   if (R.isAmbiguous())
     return ExprError();
 
   if (R.getResultKind() == LookupResult::NotFoundInCurrentInstantiation)
     return BuildDependentDeclRefExpr(SS, /*TemplateKWLoc=*/SourceLocation(),
                                      NameInfo, /*TemplateArgs=*/nullptr);
 
   if (R.empty()) {
+    // Don't diagnose problems with invalid record decl, the secondary no_member
+    // diagnostic during template instantiation is likely bogus, e.g. if a class
+    // is invalid because it's derived from an invalid base class, then missing
+    // members were likely supposed to be inherited.
+    if (const auto *CD = dyn_cast<CXXRecordDecl>(DC))
+      if (CD->isInvalidDecl())
+        return ExprError();
     Diag(NameInfo.getLoc(), diag::err_no_member)
       << NameInfo.getName() << DC << SS.getRange();
     return ExprError();
   }
 
   if (const TypeDecl *TD = R.getAsSingle<TypeDecl>()) {
     // Diagnose a missing typename if this resolved unambiguously to a type in
     // a dependent context.  If we can recover with a type, downgrade this to

clang/test/SemaCXX/access-base-class.cpp

namespace T7 {
   class B : public A {};
   class C : private B { 
     void f(C *c) {
       A* a = c; // okay
     }
   };
 }
 
+namespace T8 {
+template <int>
+struct flag {
+  static constexpr bool value = true;
+};
+
+template <class T>
+struct trait : flag<sizeof(T)> {};
+
+template <class T, bool Inferred = trait<T>::value>
+struct a {};
+
+template <class T>
+class b {
+  a<T> x;
+  using U = a<T>;
+};
+
+template <int>
+struct Impossible {
+  static_assert(false, ""); // expected-error {{static_assert failed}}
+};
+
+// verify "no member named 'value'" bogus diagnostic is not emitted.
+trait<b<Impossible<0>>>::value;
+} // namespace T8

libcxx/test/libcxx/utilities/memory/util.smartptr/util.smartptr.shared/function_type_default_deleter.fail.cpp

 int main(int, char**) {
   {
     SPtr<0> s; // OK
     SPtr<1> s1(nullptr); // OK
     SPtr<2> s2(getFn<2>(), Deleter{}); // OK
     SPtr<3> s3(nullptr, Deleter{}); // OK
   }
   // expected-error-re@memory:* 2 {{static_assert failed{{.*}} "default_delete cannot be instantiated for function types"}}
-  // FIXME: suppress this bogus diagnostic, see https://reviews.llvm.org/D86685.
-  // expected-error@memory:* 0+ {{no member named 'value' in}}
   {
     SPtr<4> s4(getFn<4>()); // expected-note {{requested here}}
     SPtr<5> s5(getFn<5>(), std::default_delete<FnType<5>>{}); // expected-note {{requested here}}
   }
 
   return 0;
 }