Refactor modules support #4321
Merged
Refactor modules support #4321
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Arthapz
force-pushed
the
improve-modules-support
branch
2 times, most recently
from
274c4d2
to
19055f4
Compare
|
Now that the code is splitted, i will simplify things, i plan to handle external modules differently, currently external project and std module are built a lot of times when different target use them i will change the package support as the paper which the current implementation has not been accepted, no need to bother with "standard" mecanism, we can design our module metadata format |
|
@waruqi Should i maintain compatibility with older toolchains for modules ? or can i remove support of old msvc / gcc / clang ? i personnally think we should remove experimental / old stuff and only support first compiler version where modules are not experimental because users that want to use modules, use updated compiler anyway |
We can remove them, but for low version compilers, I need to provide a detailed hint to tell the user to upgrade the compiler to the specified version. And we can only remove very old experimental stuff. |
waruqi
reviewed
Oct 27, 2023
|
(i took 2 weeks away from code, i'll continue next monday) |
|
i think i'm done with the refactoring |
Please wait a few days, I've been very busy lately and haven't had much time to review it carefully. I will take a look at it in the next few days. |
|
no problem :) |
waruqi
reviewed
Dec 2, 2023
waruqi
reviewed
Dec 2, 2023
waruqi
reviewed
Dec 2, 2023
waruqi
reviewed
Dec 2, 2023
waruqi
reviewed
Dec 2, 2023
waruqi
reviewed
Dec 2, 2023
waruqi
reviewed
Dec 2, 2023
waruqi
reviewed
Dec 2, 2023
|
There seems to be some issues with incremental compilation gccruki@d40dc62cc957:/mnt/xmake/tests/projects/c++/modules/user_headerunit$ xmake
[ 9%]: populating.headerunit.map user_headerunit
[ 27%]: flushing.headerunit.map user_headerunit
[ 36%]: populating.module.map user_headerunit
[ 63%]: flushing.module.map user_headerunit
[100%]: build ok, spent 0.489s
ruki@d40dc62cc957:/mnt/xmake/tests/projects/c++/modules/user_headerunit$ xmake
[ 9%]: populating.headerunit.map user_headerunit
[ 27%]: flushing.headerunit.map user_headerunit
[ 36%]: populating.module.map user_headerunit
[ 63%]: flushing.module.map user_headerunit
[100%]: build ok, spent 0.417s
ruki@d40dc62cc957:/mnt/xmake/tests/projects/c++/modules/user_headerunit$ cd ../dependence
ruki@d40dc62cc957:/mnt/xmake/tests/projects/c++/modules/dependence$ xmake
[ 7%]: populating.module.map dependence
[ 53%]: flushing.module.map dependence
[100%]: build ok, spent 0.551s
ruki@d40dc62cc957:/mnt/xmake/tests/projects/c++/modules/dependence$ xmake
[ 7%]: populating.module.map dependence
[ 53%]: flushing.module.map dependence
[100%]: build ok, spent 0.437sclangruki@d40dc62cc957:/mnt/xmake/tests/projects/c++/modules/dependence$ xmake -r
[ 0%]: generating.module.deps src/hello_impl.cpp
[ 0%]: generating.module.deps src/main.cpp
[ 0%]: generating.module.deps src/mod_impl.cpp
[ 0%]: generating.module.deps src/hello.mpp
[ 0%]: generating.module.deps src/mod.mpp
[ 8%]: compiling.module.release hello
[ 8%]: compiling.module.release mod
[ 50%]: flushing.module.map dependence
[ 58%]: compiling.release src/hello_impl.cpp
[ 58%]: compiling.release src/main.cpp
[ 58%]: compiling.release src/mod_impl.cpp
xm[ 83%]: linking.release dependence
ake[100%]: build ok, spent 2.11s
ruki@d40dc62cc957:/mnt/xmake/tests/projects/c++/modules/dependence$ xmake
[ 50%]: flushing.module.map dependence
[100%]: build ok, spent 0.379s |
with gcc ? it doesn't build modified file ? |
|
Now it always compile all for clang. ruki@d40dc62cc957:/mnt/xmake/tests/projects/c++/modules/dependence$ xmake -r
[ 0%]: generating.module.deps src/hello_impl.cpp
[ 0%]: generating.module.deps src/main.cpp
[ 0%]: generating.module.deps src/mod_impl.cpp
[ 0%]: generating.module.deps src/hello.mpp
[ 0%]: generating.module.deps src/mod.mpp
[ 8%]: compiling.module.release mod
[ 8%]: compiling.module.release hello
[ 50%]: flushing.module.map dependence
[ 58%]: compiling.release src/hello_impl.cpp
[ 58%]: compiling.release src/main.cpp
[ 58%]: compiling.release src/mod_impl.cpp
[ 83%]: linking.release dependence
[100%]: build ok, spent 2.505s
ruki@d40dc62cc957:/mnt/xmake/tests/projects/c++/modules/dependence$ xmake
[ 50%]: flushing.module.map dependence
[ 58%]: compiling.release src/hello_impl.cpp
[ 66%]: compiling.release src/mod_impl.cpp
[ 83%]: linking.release dependence
[100%]: build ok, spent 1.382s
ruki@d40dc62cc957:/mnt/xmake/tests/projects/c++/modules/dependence$ xmake
[ 50%]: flushing.module.map dependence
[ 58%]: compiling.release src/hello_impl.cpp
[ 66%]: compiling.release src/mod_impl.cpp
[ 83%]: linking.release dependence
[100%]: build ok, spent 1.276sAnd does not compile files for gcc. but always show compile progress info. ruki@d40dc62cc957:/mnt/xmake/tests/projects/c++/modules/dependence$ xmake
[ 0%]: generating.module.deps src/hello_impl.cpp
[ 0%]: generating.module.deps src/main.cpp
[ 0%]: generating.module.deps src/mod_impl.cpp
[ 0%]: generating.module.deps src/hello.mpp
[ 0%]: generating.module.deps src/mod.mpp
[ 7%]: populating.module.map dependence
[ 53%]: flushing.module.map dependence
[ 61%]: compiling.release src/hello_impl.cpp
[ 61%]: compiling.release src/main.cpp
[ 61%]: compiling.release src/mod_impl.cpp
[ 84%]: linking.release dependence
[100%]: build ok, spent 2.893s
ruki@d40dc62cc957:/mnt/xmake/tests/projects/c++/modules/dependence$ xmake
[ 7%]: populating.module.map dependence
[ 53%]: flushing.module.map dependence
[100%]: build ok, spent 0.486s |
|
It works for gcc now, but does not work for clang. ruki@d40dc62cc957:/mnt/xmake/tests/projects/c++/modules/dependence$ rm -rf build/
ruki@d40dc62cc957:/mnt/xmake/tests/projects/c++/modules/dependence$ xmake
[ 0%]: generating.module.deps src/hello_impl.cpp
[ 0%]: generating.module.deps src/main.cpp
[ 0%]: generating.module.deps src/mod_impl.cpp
[ 0%]: generating.module.deps src/hello.mpp
[ 0%]: generating.module.deps src/mod.mpp
[ 8%]: compiling.module.release mod
[ 8%]: compiling.module.release hello
[ 50%]: flushing.module.map dependence
[ 58%]: compiling.release src/hello_impl.cpp
[ 58%]: compiling.release src/main.cpp
[ 58%]: compiling.release src/mod_impl.cpp
[ 83%]: linking.release dependence
[100%]: build ok, spent 2.427s
ruki@d40dc62cc957:/mnt/xmake/tests/projects/c++/modules/dependence$ xmake
[ 54%]: compiling.release src/hello_impl.cpp
[ 81%]: linking.release dependence
[100%]: build ok, spent 1.229s
|
|
And it does not work for msvc. D:\projects\personal\xmake\tests\projects\c++\modules\headerunits_person>xmake -r
checking for platform ... windows
checking for architecture ... x64
checking for Microsoft Visual Studio (x64) version ... 2022
[ 0%]: generating.module.deps src\test.cpp
[ 0%]: generating.module.deps src\Person.mpp
[ 10%]: compiling.headerunit.release string
[ 10%]: compiling.headerunit.release iostream
[ 30%]: flushing.headerunit.map headerunits_person
[ 40%]: compiling.module.release person
[ 60%]: flushing.module.map headerunits_person
[ 70%]: compiling.release src\test.cpp
[ 80%]: linking.release headerunits_person.exe
[100%]: build ok, spent 3.985s
D:\projects\personal\xmake\tests\projects\c++\modules\headerunits_person>xmake
[ 33%]: flushing.headerunit.map headerunits_person
[ 66%]: compiling.release src\test.cpp
[ 77%]: linking.release headerunits_person.exe
[100%]: build ok, spent 0.781s
D:\projects\personal\xmake\tests\projects\c++\modules\headerunits_person>cd ..
D:\projects\personal\xmake\tests\projects\c++\modules>de dependence
D:\projects\personal\xmake\tests\projects\c++\modules>cd dependence
D:\projects\personal\xmake\tests\projects\c++\modules\dependence>xmake -r
checking for platform ... windows
checking for architecture ... x64
checking for Microsoft Visual Studio (x64) version ... 2022
[ 0%]: generating.module.deps src\hello_impl.cpp
[ 0%]: generating.module.deps src\main.cpp
[ 0%]: generating.module.deps src\mod_impl.cpp
[ 0%]: generating.module.deps src\hello.mpp
[ 0%]: generating.module.deps src\mod.mpp
[ 8%]: compiling.module.release hello
[ 8%]: compiling.module.release mod
[ 50%]: flushing.module.map dependence
[ 58%]: compiling.release src\hello_impl.cpp
[ 58%]: compiling.release src\main.cpp
[ 58%]: compiling.release src\mod_impl.cpp
[ 83%]: linking.release dependence.exe
[100%]: build ok, spent 3.203s
D:\projects\personal\xmake\tests\projects\c++\modules\dependence>xmake
[ 54%]: compiling.release src\hello_impl.cpp
[ 63%]: compiling.release src\mod_impl.cpp
[ 81%]: linking.release dependence.exe
[100%]: build ok, spent 1.235s |
|
i found some problem when building my engine, so it's not ready |
Arthapz
force-pushed
the
improve-modules-support
branch
from
f9eda05
to
0998780
Compare
waruqi
reviewed
Jan 31, 2024
waruqi
reviewed
Jan 31, 2024
waruqi
reviewed
Feb 1, 2024
| end | ||
|
|
||
| return mapper, table.keys(mapper) | ||
| end |
There was a problem hiding this comment.
-- get or create a target module mapper
function get_target_module_mapper(target)
local memcache = compiler_support.memcache()
local mapper = memcache:get2(target:name(), "module_mapper")
if not mapper then
mapper = {}
memcache:set2(target:name(), "module_mapper", mapper)
end
local mapper_keys = memcache:get2(target:name(), "module_mapper_keys")
if not mapper_keys then
mapper_keys = {}
for _, item in pairs(mapper) do
mapper_keys[item.key] = item
end
memcache:set2(target:name(), "module_mapper_keys", mapper_keys)
end
return mapper, mapper_keys
end
-- flush target module mapper keys
function flush_target_module_mapper_keys(target)
local memcache = compiler_support.memcache()
memcache:set2(target:name(), "module_mapper_keys", nil)
end
waruqi
reviewed
Feb 1, 2024
|
|
||
| function _is_duplicated_headerunit(target, key) | ||
| local mapper, mapper_keys = get_target_module_mapper(target) | ||
| for _, mapped_key in ipairs(mapper_keys) do |
There was a problem hiding this comment.
no, we should remove the whole for-loop.
function _is_duplicated_headerunit(target, key)
local _, mapper_keys = get_target_module_mapper(target)
return mapper_keys[key]
end
waruqi
reviewed
Feb 1, 2024
| -- add a module to target mapper | ||
| function add_module_to_target_mapper(target, name, sourcefile, bmifile, opt) | ||
| local mapper = get_target_module_mapper(target) | ||
| mapper[name] = {name = name, key = name, bmi = bmifile, sourcefile = sourcefile, opt = opt} |
There was a problem hiding this comment.
flush_target_module_mapper_keys(target)
waruqi
reviewed
Feb 1, 2024
| mapper[headerunit.name] = {name = headerunit.name, key = key, aliasof = deduplicated.name, headerunit = headerunit} | ||
| else | ||
| mapper[headerunit.name] = {name = headerunit.name, key = key, headerunit = headerunit, bmi = bmifile} | ||
| end |
There was a problem hiding this comment.
flush_target_module_mapper_keys(target)
These problems still exist. |
i fixed the "nil" as modulename for "stdmodules/GCC" but i'm not sure what you want for "clang on ubuntu/macos", clang doesn't support std module currently so it should fail like this (i have a PR ready for support of experimental std module stuff for clang but it'll come after this PR) for "hello/clang on macOS" i tried with xmake without my PR and it doesn't work too, i think something changed in apple clang |
nil has been fixed, thanks. |
clang 15 does not support it? |
i think apple disabled module support or something because i can't get it to work, and on llvm discord they don't know why it doesn't work simple command line tests like or doesn't work anymore, but worked before |
|
I don't know, clang 14 works fine on linix, but clang15/macos does not. |
I don't know, clang 14 works fine on linux, but clang 15/macos does not. |
and macOS CI work too ... |
|
on llvm discord |
Arthapz
force-pushed
the
improve-modules-support
branch
from
1922e9a
to
b1005e8
Compare
|
i got clang support to use compinst:compile, but now for msvc i need a way to have compinst:compile omit -Fo flag |
|
i got the confirmation that C++20 modules are disabled on apple clang on llvm discord https://discord.com/channels/636084430946959380/636808521371090956/1202656509834825819
|
Why omit it? Can you show me the whole compile command and flags? |
so we need to add [ 0%]: <hello> generating.module.deps src/main.cpp
checking for /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDef
ault.xctoolchain/usr/bin/clang-scan-deps ... no
checking for /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDef
ault.xctoolchain/usr/bin/clang-scan-deps ... no
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolch
ain/usr/bin/clang -Qunused-arguments -target x86_64-apple-macos14.0 -isysro
ot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Dev
eloper/SDKs/MacOSX14.0.sdk -fvisibility=hidden -fvisibility-inlines-hidden
-O3 -fno-delayed-template-parsing -fcxx-modules -Xclang -DNDEBUG -E -x c++
src/main.cpp -o build/.gens/hello/macosx/x86_64/release/rules/bmi/cache/mod
ules/2d9b7c0b/main.cpp.i
[ 0%]: <hello> generating.module.deps src/hello.mpp
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolch
ain/usr/bin/clang -Qunused-arguments -target x86_64-apple-macos14.0 -isysro
ot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Dev
eloper/SDKs/MacOSX14.0.sdk -fvisibility=hidden -fvisibility-inlines-hidden
-O3 -fno-delayed-template-parsing -fcxx-modules -Xclang -DNDEBUG -E -x c++
src/hello.mpp -o build/.gens/hello/macosx/x86_64/release/rules/bmi/cache/mo
dules/2d9b7c0b/hello.mpp.i
[ 16%]: <hello> compiling.module.release hello
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolch
ain/usr/bin/clang -Qunused-arguments -target x86_64-apple-macos14.0 -isysro
ot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Dev
eloper/SDKs/MacOSX14.0.sdk -fvisibility=hidden -fvisibility-inlines-hidden
-O3 -std=c++20 -fmodules-ts -fno-delayed-template-parsing -fcxx-modules -Xc
lang -fmodules-local-submodule-visibility -DNDEBUG -x c++-module --precompi
le -o build/.gens/hello/macosx/x86_64/release/rules/bmi/cache/modules/2d9b7
c0b/hello.pcm -c src/hello.mpp
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolch
ain/usr/bin/clang -Qunused-arguments -target x86_64-apple-macos14.0 -isysro
ot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Dev
eloper/SDKs/MacOSX14.0.sdk -fvisibility=hidden -fvisibility-inlines-hidden
-O3 -std=c++20 -fmodules-ts -fno-delayed-template-parsing -fcxx-modules -Xc
lang -fmodules-local-submodule-visibility -DNDEBUG -x c++-module --precompi
le -o build/.gens/hello/macosx/x86_64/release/rules/bmi/cache/modules/2d9b7
c0b/hello.pcm -c src/hello.mpp
src/hello.mpp:1:7: error: expected a module name after 'module'
module;
^
1 error generated. |
|
I'm going to merge this patch into the modules branch so that I can make some improvements. If you have new improvements, you can continue to submit them to the modules branch. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is a little refactoring of C++20 module support
i'll divide the refactoring in multiple PR (there is little to no modification to the code, only splitting in multiple interfaces,
simplification / modification will come in an other PR)
this one is to setup an easier dev environment because the current C++20 module implementation begin to be big, it's easier to work on something a little more structured (draft because i didn't refactor msvc yet)