[PATCH] D131258: [Sema] Merge variable template specializations

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu accepted this revision.
ChuanqiXu added a comment.
This revision is now accepted and ready to land.

LGTM then.



================
Comment at: clang/test/Modules/merge-var-template-spec-cxx-modules.cpp:18-23
+template <class T> constexpr T zero = 0; // expected-error-re {{declaration{{.*}}in the global module follows declaration in module var_def}}
+                                         // expected-note@* {{previous}}
+template <> constexpr Int zero<Int> = {0}; // expected-error-re {{declaration{{.*}}in the global module follows declaration in module var_def}}
+                                           // expected-note@* {{previous}}
+template <class T> constexpr T* zero<T*> = nullptr; // expected-error-re {{declaration{{.*}}in the global module follows declaration in module var_def}}
+                                                    // expected-note@* {{previous}}
----------------
ilya-biryukov wrote:
> ChuanqiXu wrote:
> > BTW, these errors are not expected by me since they are not in a named modules and they are in different TU with var_def.cppm.
> > 
> I am not sure about this one, but my understanding of the standard was that no two definitions are allowed if any is attached to a named module.
> Am I reading the standard wrong here?
> The relevant bits from my perspective are in [[ https://eel.is/c++draft/basic.def.odr#14.3 | {basic.def.odr}p14.3 ]]:
> ```
> For any definable item D with definitions in multiple translation units,
> ...
> - if the definitions in different translation units do not satisfy the following requirements,
> the program is ill-formed; a diagnostic is required only if the definable item is attached to a named module and a prior definition is reachable at the point where a later definition occurs. 
> ...
>   - Each such definition shall not be attached to a named module ([module.unit]).
> ```
> The diagnostic seems to be required by the standard when looking at a declaration in the named module and this example is the other way around here.
> However, I don't see why we can't provide a diagnostic in that case too.
Oh, you are right. My bad. Sorry for wasting time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131258