[PATCH] D128921: [Sema] Merge C++20 concept definitions from different modules in same TU

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 30 11:28:34 PDT 2022

ilya-biryukov added a comment.

I haven't seen your reply before posting my initial comments. Many thanks for a very quick turnaroud on this!
Will wait for module folks to approve.

Comment at: clang/lib/Sema/SemaTemplate.cpp:8674
     auto *Old = Previous.getRepresentativeDecl();
     Diag(NameLoc, isa<ConceptDecl>(Old) ? diag::err_redefinition :
          diag::err_redefinition_different_kind) << NewDecl->getDeclName();
ilya-biryukov wrote:
> Not sure if reporting the redefinition if `isSameEntity` returns false is the right approach here.
> This leads to errors on definitions of something like:
> ```
> // foo.h
> template <class T> concept A = false;
> // bar.h
> template <class T, class U> concept A = true;
> #include "foo.h"
> #include "bar.h" // <-- redefinition error shown here
> ```
> The alternative is the "ambiguous reference" error on the use of the concept. I will check what happens for variables and typedefs in that case and follow the same pattern.
Clang tends to do "ambiguous reference" instead of the behavior we had in this patch.
I have updated it to be consistent with what typedefs and other symbols produce.

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list