ChuanqiXu added inline comments.
================
Comment at: clang/include/clang/Sema/Overload.h:800
+ /// This candidate was not viable because it has internal linkage and is
+ /// from a different module than the use.
+ ovl_fail_module_mismatched,
----------------
================
Comment at: clang/lib/Sema/SemaOverload.cpp:6403
+ // Functions with internal linkage are only viable in the same module.
+ if (auto *MF = Function->getOwningModule()) {
----------------
================
Comment at: clang/lib/Sema/SemaOverload.cpp:6406
+ if (Function->getFormalLinkage() <= Linkage::InternalLinkage &&
+ getLangOpts().CPlusPlus20 && MF != getCurrentModule()) {
+ Candidate.Viable = false;
----------------
The current implementation may reject following two cases:
```
module;
static void foo(int); // Ignore header
...
export module A;
void bar() { foo(5); } // Should it be invalid?
```
and
```
export module A;
static void foo(int);
...
module :private;
void bar() { foo(5); } // Should it be invalid?
```
I mean in the above examples, the function of `foo(int)` is defined in the same
TU but the call to it might be rejected.
================
Comment at: clang/test/CXX/basic/basic.link/p10-ex2.cpp:10-35
+//--- decls.h
+int f(); // #1, attached to the global module
+int g(); // #2, attached to the global module
+
+//--- M.cpp
+module;
+#include "decls.h"
----------------
I feel like the test is irrelevant with the revision, isn't it? If yes, I think
we could land this as a separate NFC patch.
================
Comment at:
clang/test/CXX/basic/basic.lookup/basic.lookup.argdep/p5-ex2.cpp:36-41
+//--- Q.cpp
+export module Q;
+
+//--- Q-impl.cpp
+module Q;
+import N;
----------------
This looks more consistent with the example.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D129174/new/
https://reviews.llvm.org/D129174
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
