[libcxx-commits] [PATCH] D86765: [clang] Don't emit "no member" diagnostic if the lookup fails on an invalid record decl.

[Sam McCall via Phabricator via libcxx-commits]

sammccall accepted this revision.
sammccall added a comment.

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



================
Comment at: clang/lib/Sema/SemaExpr.cpp:2584
+    // Don't diagnose problems with invalid record decl, the no_member
+    // diagnostic is likely bogus, e.g. if a base specificer is invalid, the
+    // derived class is invalid, and has no base class attached, lookup of base
----------------
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?


================
Comment at: clang/lib/Sema/SemaExpr.cpp:2586
+    // derived class is invalid, and has no base class attached, lookup of base
+    // class members will fails.
+    if (const auto *CD = dyn_cast<CXXRecordDecl>(DC))
----------------
nit: will fails -> will fail


================
Comment at: clang/lib/Sema/SemaExpr.cpp:2588
+    if (const auto *CD = dyn_cast<CXXRecordDecl>(DC))
+      if (CD->isInvalidDecl())
+        return ExprError();
----------------
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86765/new/

https://reviews.llvm.org/D86765