-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[clang][Modules] Import then include fails to merge inline lambdas #102721
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
@llvm/issue-subscribers-clang-modules Author: David Stone (davidstone)
Given the following valid translation units:
module;
inline auto x = []{};
export module a; module;
import a;
inline auto x = []{};
export module b; clang erroneously rejects with b.cpp:5:13: error: redeclaration of 'x' with a different type: '(lambda at /app/b.cpp:5:17)' vs '(lambda at /app/a.cpp:3:17)'
5 | inline auto x = []{};
| ^
a.cpp:3:13: note: previous definition is here
3 | inline auto x = []{};
| ^
1 error generated. See it live: https://godbolt.org/z/WxW47zW4v This is the reduced version of the following reproducer: module;
#include <compare>
export module a; module;
import a;
#include <compare>
export module b; in which clang with libc++ erroneously rejects with In file included from /app/b.cpp:4:
In file included from /opt/compiler-explorer/clang-assertions-trunk-20240809/bin/../include/c++/v1/compare:156:
/opt/compiler-explorer/clang-assertions-trunk-20240809/bin/../include/c++/v1/__compare/synth_three_way.h:28:45: error: redeclaration of '__synth_three_way' with a different type: 'const std::__1::(lambda at /opt/compiler-explorer/clang-assertions-trunk-20240809/bin/../include/c++/v1/__compare/synth_three_way.h:28:65)' vs 'const std::__1::(lambda at /opt/compiler-explorer/clang-assertions-trunk-20240809/bin/../include/c++/v1/__compare/synth_three_way.h:28:65)'
28 | _LIBCPP_HIDE_FROM_ABI inline constexpr auto __synth_three_way = []<class _Tp, class _Up>(const _Tp& __t, const _Up& __u)
| ^
/opt/compiler-explorer/clang-assertions-trunk-20240809/bin/../include/c++/v1/__compare/synth_three_way.h:28:45: note: previous definition is here
28 | _LIBCPP_HIDE_FROM_ABI inline constexpr auto __synth_three_way = []<class _Tp, class _Up>(const _Tp& __t, const _Up& __u)
| ^
1 error generated. See it live: https://godbolt.org/z/4dK717sr4 Looks like the library side of it was reintroduced by d043e4c @H-G-Hristov, @Zingam but I suspect that the clang side of it was never fully fixed. #57222 @ChuanqiXu9 shows a unit test for two modules that just include a header with an inline lambda, but this case of import module that included, followed by include appears to have slipped through. |
Close llvm#102721 Previously we tried to merge lambdas between TUs. But we may still meet problems if we import and then include the headers containing the same lambda. The solution in this patch is, when we assigned a lambda numbering in a sema, try to see if there is already a same lambda imported. If yes, try to merge them.
Close #102721 Generally, the type of merged decls will be reused in ASTContext. But for lambda, in the import and then include case, we can't decide its previous decl in the imported modules so that we can't assign the previous decl before creating the type for it. Since we can't decide its numbering before creating it. So we have to assign the previous decl and the canonical type for it after creating it, which is unusual and slightly hack.
Given the following valid translation units:
clang erroneously rejects with
See it live: https://godbolt.org/z/WxW47zW4v
This is the reduced version of the following reproducer:
in which clang with libc++ erroneously rejects with
See it live: https://godbolt.org/z/4dK717sr4
Looks like the library side of it was reintroduced by d043e4c @H-G-Hristov, @Zingam but I suspect that the clang side of it was never fully fixed. #57222 @ChuanqiXu9 shows a unit test for two modules that just include a header with an inline lambda, but this case of import module that included, followed by include appears to have slipped through.
The text was updated successfully, but these errors were encountered: