iains marked 2 inline comments as done.
iains added inline comments.
================
Comment at: clang/lib/Sema/SemaOverload.cpp:6406
+ if (Function->getFormalLinkage() <= Linkage::InternalLinkage &&
+ getLangOpts().CPlusPlus20 && MF != getCurrentModule()) {
+ Candidate.Viable = false;
----------------
ChuanqiXu wrote:
> 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.
> 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/lib/Sema/SemaOverload.cpp:6406
+ if (Function->getFormalLinkage() <= Linkage::InternalLinkage &&
+ getLangOpts().CPlusPlus20 && MF != getCurrentModule()) {
+ Candidate.Viable = false;
----------------
iains wrote:
> ChuanqiXu wrote:
> > 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.
> > 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.
>
>
> The current implementation may reject following two cases:
> ```
> module;
> static void foo(int); // Ignore header
> ...
Actually, I have a note to check on the global module case, since we have
special rules that allow merging of items there,
> 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.
otherwise, agreed, these cases should be ok - I guess we need a second test
case with lookups that should succeed.
================
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"
----------------
ChuanqiXu wrote:
> 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.
well, it is related to the paper mentioned, but yes we can make this separate.
================
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;
----------------
ChuanqiXu wrote:
> This looks more consistent with the example.
This changes the example so that M is directly included in the implementation
rather than transitively (the example does specifically use a different module
name for the implementation)
I am not sure what you mean by "more consistent"
(the example will fail to reject some of the lookups with this change).
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
