[clang] Add the check of membership in decltype for the issue #58674

[lime]

Summary

D137531 had once fixed the issue. However, it caused a crash during compiling
llvm/unittests/IR/PatternMatch.cpp in stage-2. The reason is the predicator
isDerivedFrom does not consider independent types if the derived type is
dependent.

This patch improves D137531 by adding an option to make isDerivedFrom consider
independent types.

[erichkeane]

1 nit, otherwise I don't have concerns.

clang/lib/Sema/SemaExpr.cpp

2717

[shafik]

FYI, I was looking at similar issues over the weekend and put up this PR: https://reviews.llvm.org/D142490

[lime]

Updated as suggested.

clang/docs/ReleaseNotes.rst

 Major New Features
 ------------------
 
 Bug Fixes
 ---------
 - Fix crash on invalid code when looking up a destructor in a templated class
   inside a namespace. This fixes
   `Issue 59446 <https://github.com/llvm/llvm-project/issues/59446>`_.
+- Fix an issue about ``decltype`` in the members of class templates derived from
+  templates with related parameters. This fixes
+  `Issue 58674 <https://github.com/llvm/llvm-project/issues/58674>`_
 
 Improvements to Clang's diagnostics
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Non-comprehensive list of changes in this release
 -------------------------------------------------
 
 New Compiler Flags

clang/include/clang/AST/DeclCXX.h

   /// This routine only determines whether this class is derived from \p Base,
   /// but does not account for factors that may make a Derived -> Base class
   /// ill-formed, such as private/protected inheritance or multiple, ambiguous
   /// base class subobjects.
   ///
   /// \param Base the base class we are searching for.
   ///
   /// \returns true if this class is derived from Base, false otherwise.
-  bool isDerivedFrom(const CXXRecordDecl *Base) const;
+  bool isDerivedFrom(const CXXRecordDecl *Base,
+                     bool LookupIndependent = false) const;
 
   /// Determine whether this class is derived from the type \p Base.
   ///
   /// This routine only determines whether this class is derived from \p Base,
   /// but does not account for factors that may make a Derived -> Base class
   /// ill-formed, such as private/protected inheritance or multiple, ambiguous
   /// base class subobjects.
   ///
   /// \param Base the base class we are searching for.
   ///
   /// \param Paths will contain the paths taken from the current class to the
   /// given \p Base class.
   ///
   /// \returns true if this class is derived from \p Base, false otherwise.
   ///
   /// \todo add a separate parameter to configure IsDerivedFrom, rather than
   /// tangling input and output in \p Paths
-  bool isDerivedFrom(const CXXRecordDecl *Base, CXXBasePaths &Paths) const;
+  bool isDerivedFrom(const CXXRecordDecl *Base, CXXBasePaths &Paths,
+                     bool LookupIndependent = false) const;
 
   /// Determine whether this class is virtually derived from
   /// the class \p Base.
   ///
   /// This routine only determines whether this class is virtually
   /// derived from \p Base, but does not account for factors that may
   /// make a Derived -> Base class ill-formed, such as
   /// private/protected inheritance or multiple, ambiguous base class

clang/lib/AST/CXXInheritance.cpp

void CXXBasePaths::swap(CXXBasePaths &Other) {
   ClassSubobjects.swap(Other.ClassSubobjects);
   VisitedDependentRecords.swap(Other.VisitedDependentRecords);
   std::swap(FindAmbiguities, Other.FindAmbiguities);
   std::swap(RecordPaths, Other.RecordPaths);
   std::swap(DetectVirtual, Other.DetectVirtual);
   std::swap(DetectedVirtual, Other.DetectedVirtual);
 }
 
-bool CXXRecordDecl::isDerivedFrom(const CXXRecordDecl *Base) const {
+bool CXXRecordDecl::isDerivedFrom(const CXXRecordDecl *Base,
+                                  bool LookupIndependent) const {
   CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/false,
                      /*DetectVirtual=*/false);
-  return isDerivedFrom(Base, Paths);
+  return isDerivedFrom(Base, Paths, LookupIndependent);
 }
 
 bool CXXRecordDecl::isDerivedFrom(const CXXRecordDecl *Base,
-                                  CXXBasePaths &Paths) const {
+                                  CXXBasePaths &Paths,
+                                  bool LookupIndependent) const {
   if (getCanonicalDecl() == Base->getCanonicalDecl())
     return false;
 
   Paths.setOrigin(const_cast<CXXRecordDecl*>(this));
 
   const CXXRecordDecl *BaseDecl = Base->getCanonicalDecl();
   return lookupInBases(
       [BaseDecl](const CXXBaseSpecifier *Specifier, CXXBasePath &Path) {
-        return FindBaseClass(Specifier, Path, BaseDecl);
+        return Specifier->getType()->getAsRecordDecl() &&
+               FindBaseClass(Specifier, Path, BaseDecl);
       },
-      Paths);
+      Paths, LookupIndependent);
 }
 
 bool CXXRecordDecl::isVirtuallyDerivedFrom(const CXXRecordDecl *Base) const {
   if (!getNumVBases())
     return false;
 
   CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/false,
                      /*DetectVirtual=*/false);

clang/lib/Sema/SemaExpr.cpp

Sema::ActOnIdExpression(Scope *S, CXXScopeSpec &SS,
   //   Otherwise, if E1.E2 refers to a non-static member function. . .
   //   [t]he expression can be used only as the left-hand operand of a
   //   member function call.
   //
   // There are other safeguards against such uses, but it's important
   // to get this right here so that we don't end up making a
   // spuriously dependent expression if we're inside a dependent
   // instance method.
+  //
+  // We also don't need to do this if R resolved to a member in another
+  // class, which can happen in an unevaluated operand:
+  //
+  // C++ [expr.prim.id]p3.3:
+  //   If that id-expression denotes a non-static data member and it
+  //   appears in an unevaluated operand.
   if (!R.empty() && (*R.begin())->isCXXClassMember()) {
-    bool MightBeImplicitMember;
-    if (!IsAddressOfOperand)
-      MightBeImplicitMember = true;
-    else if (!SS.isEmpty())
-      MightBeImplicitMember = false;
-    else if (R.isOverloadedResult())
+    bool MightBeImplicitMember = true, CheckField = true;
+    if (IsAddressOfOperand) {
+      MightBeImplicitMember = SS.isEmpty() && !R.isOverloadedResult();
+      CheckField = !R.isUnresolvableResult();
+    }
+    if (MightBeImplicitMember && CheckField) {
+      if (R.isSingleResult() &&
+          isa<FieldDecl, IndirectFieldDecl, MSPropertyDecl>(R.getFoundDecl())) {
+        auto Class = cast<CXXRecordDecl>((*R.begin())->getDeclContext());
+        for (auto Curr = S->getLookupEntity(); Curr && !Curr->isFileContext();
+             Curr = Curr->getParent()) {
+          if (auto ThisClass = dyn_cast_if_present<CXXRecordDecl>(Curr)) {
+            if ((MightBeImplicitMember =
+                     ThisClass->Equals(Class) ||
+                     ThisClass->isDerivedFrom(Class,
+                                              /*LookupIndependent=*/true)))
+              break;
+          }
+        }
+      } else if (IsAddressOfOperand)
         MightBeImplicitMember = false;
-    else if (R.isUnresolvableResult())
-      MightBeImplicitMember = true;
-    else
-      MightBeImplicitMember = isa<FieldDecl>(R.getFoundDecl()) ||
-                              isa<IndirectFieldDecl>(R.getFoundDecl()) ||
-                              isa<MSPropertyDecl>(R.getFoundDecl());
+    }
 
     if (MightBeImplicitMember)
       return BuildPossibleImplicitMemberExpr(SS, TemplateKWLoc,
                                              R, TemplateArgs, S);
   }
 
   if (TemplateArgs || TemplateKWLoc.isValid()) {
 

clang/test/SemaCXX/decltype.cpp

namespace D5789 {
   void foo(decltype(T(LP1{ .p1 = { "foo" }, .p1.x[1] = 'x'}))) {} // okay
 
   // expected-warning@+3 {{initializer partially overrides}}
   // expected-note@+2 {{previous initialization}}
   // expected-error@+1 {{redefinition of 'foo'}}
   template<class T> void foo(decltype(T(LP1{ .p1 = g1, .p1.x[1] = 'x' }))) {}
 }
 
+namespace GH58674 {
+  struct Foo {
+    float value_;
+    struct nested {
+      float value_;
+    };
+  };
+
+  template <typename T>
+  struct TemplateFoo {
+    float value_;
+  };
+
+  float bar;
+
+  template <typename T>
+  struct Animal{};
+
+  template <typename T>
+  class Cat : Animal<T> {
+    using okay = decltype(Foo::value_);
+    using also_okay = decltype(bar);
+    using okay2 = decltype(Foo::nested::value_);
+    using okay3 = decltype(TemplateFoo<T>::value_);
+  public:
+    void meow() {
+      using okay = decltype(Foo::value_);
+      using also_okay = decltype(bar);
+      using okay2 = decltype(Foo::nested::value_);
+      using okay3 = decltype(TemplateFoo<T>::value_);
+    }
+  };
+
+  void baz() {
+      Cat<void>{}.meow();
+  }
+}
+
 template<typename>
 class conditional {
 };
 
 // FIXME: The diagnostics here are produced twice.
 void foo(conditional<decltype((1),int>) {  // expected-note 2 {{to match this '('}} expected-error {{expected ')'}} expected-note 2{{to match this '<'}}
 } // expected-error {{expected function body after function declarator}} expected-error 2 {{expected '>'}} expected-error {{expected ')'}}