ilya-biryukov updated this revision to Diff 442633.
ilya-biryukov added a comment.

- Always call PushToScopeChains. This is the right behavior after we stopped 
calling makeMergedDefinitionVisible


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128921

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/Modules/merge-concepts-cxx-modules.cpp
  clang/test/Modules/merge-concepts.cpp

Index: clang/test/Modules/merge-concepts.cpp
===================================================================
--- /dev/null
+++ clang/test/Modules/merge-concepts.cpp
@@ -0,0 +1,83 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -xc++ -std=c++20 -fmodules -fmodule-name=library \
+// RUN:     -emit-module %t/modules.map \
+// RUN:     -o %t/module.pcm
+//
+//
+// RUN: %clang_cc1 -xc++ -std=c++20 -fmodules -fmodule-file=%t/module.pcm  \
+// RUN:     -fmodule-map-file=%t/modules.map \
+// RUN:     -fsyntax-only -verify %t/use.cpp
+//
+//--- use.cpp
+
+#include "concepts.h"
+#include "conflicting.h"
+#include "format.h"
+
+template <class T> void foo()
+  requires same_as<T, T>
+{}
+ConflictingConcept auto x = 10; // expected-error {{reference to 'ConflictingConcept' is ambiguous}}
+                                // expected-note@* 2 {{candidate}}
+
+//--- modules.map
+module "library" {
+	export *
+	module "concepts" {
+		export *
+		header "concepts.h"
+	}
+	module "format" {
+		export *
+		header "format.h"
+	}
+	module "conflicting" {
+		export *
+		header "conflicting.h"
+	}
+}
+
+//--- concepts.h
+#ifndef SAMEAS_CONCEPTS_H_
+#define SAMEAS_CONCEPTS_H_
+
+#include "same_as.h"
+
+template <class T>
+concept ConflictingConcept = true;
+
+#endif // SAMEAS_CONCEPTS_H
+
+//--- same_as.h
+#ifndef SAME_AS_H
+#define SAME_AS_H
+
+template <class T, class U>
+concept same_as = __is_same(T, U);
+
+#endif // SAME_AS_H
+
+//--- format.h
+#ifndef FORMAT_H
+#define FORMAT_H
+
+#include "concepts.h"
+#include "same_as.h"
+
+template <class T> void foo()
+  requires same_as<T, int>
+{}
+
+#endif // FORMAT_H
+
+//--- conflicting.h
+#ifndef CONFLICTING_H
+#define CONFLICTING_H
+
+template <class T, class U = int>
+concept ConflictingConcept = true;
+
+#endif // CONFLICTING_H
Index: clang/test/Modules/merge-concepts-cxx-modules.cpp
===================================================================
--- /dev/null
+++ clang/test/Modules/merge-concepts-cxx-modules.cpp
@@ -0,0 +1,46 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/same_as.cppm -o %t/same_as.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -fprebuilt-module-path=%t %t/concepts.cppm -o %t/concepts.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -fprebuilt-module-path=%t %t/format.cppm -o %t/format.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/conflicting.cppm -o %t/conflicting.pcm
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/Use.cppm -fsyntax-only -verify
+
+//--- same_as.cppm
+export module same_as;
+export template <class T, class U>
+concept same_as = __is_same(T, U);
+
+//--- concepts.cppm
+export module concepts;
+export import same_as;
+
+export template <class T>
+concept ConflictingConcept = true;
+
+//--- format.cppm
+
+export module format;
+export import concepts;
+export import same_as;
+
+export template <class T> void foo()
+  requires same_as<T, int>
+{}
+
+//--- conflicting.cppm
+export module conflicting;
+export template <class T, class U = int>
+concept ConflictingConcept = true;
+
+//--- Use.cppm
+import format;
+import conflicting;
+
+template <class T> void foo()
+  requires same_as<T, T>
+{}
+ConflictingConcept auto x = 10; // expected-error {{reference to 'ConflictingConcept' is ambiguous}}
+                                // expected-note@* 2 {{candidate}}
Index: clang/lib/Sema/SemaTemplate.cpp
===================================================================
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -8656,23 +8656,48 @@
   // Check for conflicting previous declaration.
   DeclarationNameInfo NameInfo(NewDecl->getDeclName(), NameLoc);
   LookupResult Previous(*this, NameInfo, LookupOrdinaryName,
-                        ForVisibleRedeclaration);
+                        forRedeclarationInCurContext());
   LookupName(Previous, S);
-
   FilterLookupForScope(Previous, DC, S, /*ConsiderLinkage=*/false,
                        /*AllowInlineNamespace*/false);
-  if (!Previous.empty()) {
-    auto *Old = Previous.getRepresentativeDecl();
-    Diag(NameLoc, isa<ConceptDecl>(Old) ? diag::err_redefinition :
-         diag::err_redefinition_different_kind) << NewDecl->getDeclName();
-    Diag(Old->getLocation(), diag::note_previous_definition);
-  }
+  CheckConceptRedefinition(NewDecl, Previous);
 
   ActOnDocumentableDecl(NewDecl);
   PushOnScopeChains(NewDecl, S);
   return NewDecl;
 }
 
+void Sema::CheckConceptRedefinition(ConceptDecl *NewDecl,
+                                    LookupResult &Previous) {
+  if (Previous.empty())
+    return;
+  // Check if there is a concept declaration we can merge with.
+  auto *PrevDef =
+      Previous.isSingleResult() ? Previous.getAsSingle<ConceptDecl>() : nullptr;
+  if (PrevDef && !hasReachableDefinition(PrevDef) &&
+      Context.isSameEntity(NewDecl, PrevDef)) {
+    Context.setPrimaryMergedDecl(NewDecl, PrevDef);
+    return;
+  }
+  // Filter out non-visible declaration to avoid spurious redefinition errors.
+  auto F = Previous.makeFilter();
+  while (F.hasNext()) {
+    if (!isReachable(F.next())) {
+      F.erase();
+    }
+  }
+  F.done();
+  // We report redefinition if the lookup is not empty after filters.
+  if (Previous.empty())
+    return;
+  auto *Old = Previous.getRepresentativeDecl();
+  Diag(NewDecl->getLocation(), isa<ConceptDecl>(Old)
+                                   ? diag::err_redefinition
+                                   : diag::err_redefinition_different_kind)
+      << NewDecl->getDeclName();
+  Diag(Old->getLocation(), diag::note_previous_definition);
+}
+
 /// \brief Strips various properties off an implicit instantiation
 /// that has just been explicitly specialized.
 static void StripImplicitInstantiation(NamedDecl *D) {
Index: clang/lib/AST/ASTContext.cpp
===================================================================
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -6524,6 +6524,7 @@
   // and patterns match.
   if (const auto *TemplateX = dyn_cast<TemplateDecl>(X)) {
     const auto *TemplateY = cast<TemplateDecl>(Y);
+    // FIXME: for C++20 concepts, check their requirements are the same.
     return isSameEntity(TemplateX->getTemplatedDecl(),
                         TemplateY->getTemplatedDecl()) &&
            isSameTemplateParameterList(TemplateX->getTemplateParameters(),
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -8247,6 +8247,8 @@
       Scope *S, MultiTemplateParamsArg TemplateParameterLists,
       IdentifierInfo *Name, SourceLocation NameLoc, Expr *ConstraintExpr);
 
+  void CheckConceptRedefinition(ConceptDecl *NewDecl, LookupResult &Previous);
+
   RequiresExprBodyDecl *
   ActOnStartRequiresExpr(SourceLocation RequiresKWLoc,
                          ArrayRef<ParmVarDecl *> LocalParameters,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to