[C++] Don't filter using declaration when we perform qualified look up

[ChuanqiXu]

Summary

Close https://github.com/llvm/llvm-project/issues/62174

And this was originally a try to close https://github.com/llvm/llvm-project/issues/62158.

I don't feel this is the correct fix. I just think it is not bad as an ad-hoc patch. And let's discuss things in the higher-level in the above GitHub issue link.

[ChuanqiXu]

clang/lib/Sema/SemaDecl.cpp

1824–1826

This is the ad-hoc change. Look at the following comment.

6375–6378

I can't find the wording in the existing spec (at least not in current [dcl.meaning]). But if p8.cpp will be accepted unexpectedly if I remove RemoveUsingDecls(Previous); completely. It would look good to me if we can remove RemoveUsingDecls(Previous); completely.

clang/test/CXX/special/class.inhctor/p8.cpp

32–33

I don't understand why this should be rejected. I tried to read P0136R1 but I failed to find the related things and I don't find a related wording in the current spec.

[ChuanqiXu]

@rsmith gentle ping~

[ChuanqiXu]

Code cleanup and gentle ping~. Given this is small and innocent, I'd like to land this in 2 weeks if no comments come in.

[erichkeane]

Give folks another 48 hrs to disagree, but this seems fine to me/consistent with other comments.

clang/lib/Sema/SemaDecl.cpp

1824

So this name isn't really what you're doing.

[ChuanqiXu]

Address comments and thank you for reviewing this!

clang/lib/Sema/SemaDecl.cpp

bool Sema::IsRedefinitionInModule(const NamedDecl *New,
   // Then New and Old lives in the same TU if their share one same module unit.
   if (NewM)
     NewM = NewM->getTopLevelModule();
   if (OldM)
     OldM = OldM->getTopLevelModule();
   return OldM == NewM;
 }
 
-static bool isUsingDecl(NamedDecl *D) {
+static bool isUsingDeclAtClassScope(NamedDecl *D) {
+  if (D->getDeclContext()->isFileContext())
+    return false;
+
   return isa<UsingShadowDecl>(D) ||
          isa<UnresolvedUsingTypenameDecl>(D) ||
          isa<UnresolvedUsingValueDecl>(D);
 }
 
-/// Removes using shadow declarations from the lookup results.
+/// Removes using shadow declarations at class scope from the lookup results.
 static void RemoveUsingDecls(LookupResult &R) {
   LookupResult::Filter F = R.makeFilter();
   while (F.hasNext())
-    if (isUsingDecl(F.next()))
+    if (isUsingDeclAtClassScope(F.next()))
       F.erase();
 
   F.done();
 }
 
 /// Check for this common pattern:
 /// @code
 /// class S {
if (!D.getCXXScopeSpec().isSet()) {
     //   };
     //
     //   void X::f(int) { } // ill-formed
     //
     // In this case, Previous will point to the overload set
     // containing the two f's declared in X, but neither of them
     // matches.
 
-    // C++ [dcl.meaning]p1:
-    //   [...] the member shall not merely have been introduced by a
-    //   using-declaration in the scope of the class or namespace nominated by
-    //   the nested-name-specifier of the declarator-id.
+    // FIXME: Why do we need to remove using decls here?
     RemoveUsingDecls(Previous);
   }
 
   if (Previous.isSingleResult() &&
       Previous.getFoundDecl()->isTemplateParameter()) {
     // Maybe we will complain about the shadowed template parameter.
     if (!D.isInvalidType())
       DiagnoseTemplateParameterShadow(D.getIdentifierLoc(),

clang/test/CXX/special/class.inhctor/p8.cpp

struct C {
   template<typename T> constexpr C(T t) : v(t) {}
   int v;
 };
 struct D : C {
   using C::C;
 };
 static_assert(D(123).v == 123, "");
 
+// FIMXE: Why should the following line be rejected?
 template<typename T> constexpr D::D(T t) : C(t) {} // expected-error {{does not match any declaration in 'D'}}

clang/test/Modules/pr62158.cppm

+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/lib.cppm -o %t/lib.pcm
+// RUN: %clang_cc1 -std=c++20 %t/main.cpp -fmodule-file=lib=%t/lib.pcm \
+// RUN:     -verify -fsyntax-only
+
+//--- header.h
+namespace lib::inline __1 {
+template <class>
+inline constexpr bool test = false;
+template <class>
+constexpr bool func() {
+    return false;
+}
+inline constexpr bool non_templ = true;
+} // namespace lib
+
+//--- lib.cppm
+module;
+#include "header.h"
+export module lib;
+
+export namespace lib {
+    using lib::test;
+    using lib::func;
+    using lib::non_templ;
+} // namespace lib
+
+//--- main.cpp
+// expected-no-diagnostics
+import lib;
+
+struct foo {};
+
+template <>
+inline constexpr bool lib::test<foo> = true;
+
+template <>
+constexpr bool lib::func<foo>() {
+    return true;
+}
+
+static_assert(lib::test<foo>);
+static_assert(lib::func<foo>());

clang/test/SemaCXX/pr62174.cpp

+// RUN: %clang_cc1 -std=c++20 %s -fsyntax-only -verify
+// expected-no-diagnostics
+namespace lib {
+    namespace impl {
+        template <class>
+        inline constexpr bool test = false;
+    }
+    using impl::test;
+}
+
+struct foo {};
+
+template <>
+inline constexpr bool lib::test<foo> = true;
+
+static_assert(lib::test<foo>);