Skip to content
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

Fix clang submodules #1982

Merged
merged 9 commits into from
Jan 19, 2022
Merged

Fix clang submodules #1982

merged 9 commits into from
Jan 19, 2022

Conversation

aacirino
Copy link
Contributor

I made this pull request due to build failures with submodules and clang 13. I've tested it with the project xmake/tests/projects/c++/modules/submodules and it runs as expected.

local moduleinfo = data.moduleinfo
moduledeps = moduledeps or {}
moduledeps[moduleinfo.name] = moduleinfo
if data.moduleinfo then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and

local moduleinfo = data.moduleinfo
if moduleinfo then
                moduledeps = moduledeps or {}
                moduledeps[moduleinfo.name] = moduleinfo
end

@@ -100,6 +100,14 @@ function build_with_batchjobs(target, batchjobs, sourcebatch, opt)
opt2.objectfile = modulefiles[i]
opt2.dependfile = target:dependfile(opt2.objectfile)
opt2.sourcekind = assert(sourcebatch.sourcekind, "%s: sourcekind not found!", sourcefile)
if moduledep["deps"] then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use moduledep.deps moduledep.name

if found == nil then
target:add("cxxflags", "-fmodule-file=" .. modulefile, {force = true})
end
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why? The code below will add it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

count = count + 1
if count == sourcefiles_total then
target:add("cxxflags", modulesflag, "-fmodules-cache-path=" .. cachedir, {force = true})
-- FIXME It is invalid for the module implementation unit
--target:add("cxxflags", "-fimplicit-modules", "-fimplicit-module-maps", "-fprebuilt-module-path=" .. cachedir, {force = true})
for _, modulefile in ipairs(modulefiles) do
target:add("cxxflags", "-fmodule-file=" .. modulefile, {force = true})
end

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code above doesn't add the pcm file to the compilation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've turned on -fimplicit-modules and module cache, clang should find submodules without explicitly setting -fmodule-file.

The root cause of it is that the module files for cache/module1.pcm are not named correctly, in case of submodules, it should be cache/main_module.module1.pcm so that clang can find them correctly.

I have tested your demo project and confirmed that it can be fixed successfully.

@waruqi
Copy link
Member

waruqi commented Jan 18, 2022

I can't reproduce it, It work fine here

xmake f --toolchain=clang-13 -cvD;xmake -r  
checking for platform ... linux
checking for architecture ... x86_64
checking for clang-13 ... /usr/bin/clang-13
configure
{
    ndk_stdcxx = true
    ccache = true
    arch = x86_64
    mode = release
    buildir = build
    toolchain = clang-13
    clean = true
    plat = linux
    host = linux
    kind = static
}
[ 14%]: ccache compiling.release src/math.math1.mpp
[ 14%]: ccache compiling.release src/math.math2.mpp
[ 42%]: ccache compiling.release src/math.mpp
[ 57%]: ccache compiling.release src/main.cpp
[ 71%]: linking.release math
[100%]: build ok!

Can you give a reproducible example? or provide your error output?

@aacirino
Copy link
Contributor Author

This is the complete flow that led to the error and the build with my patches

[~/git/modfix]$ tree
.
├── build
│   └── linux
│       └── x86_64
│           └── release
├── p.lua
├── src
│   ├── main.cpp
│   ├── main_module.mpp
│   ├── module1.cpp
│   ├── module1.mpp
│   ├── module2.cpp
│   └── module2.mpp
└── xmake.lua

5 directories, 8 files
[~/git/modfix]$ cat src/main.cpp
import main_module;

int main(int argc, char** argv) {
  hello1();
  hello2();
  return 0;
}
[~/git/modfix]$ cat src/main_module.mpp 
export module main_module;

export import main_module.module1;
export import main_module.module2;

export void hello() {
  hello1();
  hello2();
}
[~/git/modfix]$ cat src/module1.mpp    
export module main_module.module1;

export void hello1(void);
[~/git/modfix]$ cat src/module1.cpp
module main_module.module1;

import <iostream>;

void hello1(void) {
  std::cout << "Hello One\n";
}
[~/git/modfix]$ cat src/module2.mpp
export module main_module.module2;

export void hello2(void);
[~/git/modfix]$ cat src/module2.cpp
module main_module.module2;

import <iostream>;

void hello2(void) {
  std::cout << "Hello Two\n";
}
[~/git/modfix]$ cat xmake.lua      
add_rules("mode.debug", "mode.release")

target("modfix")
    set_kind("binary")
    add_files("src/*.cpp", "src/*.mpp")
    add_includedirs(
      "/usr/local/include",
      "$(shell brew --prefix)/include",
      "$(shell brew --prefix)/opt/llvm/include"
    )
    add_cxxflags(
      "-march=native",
      "-std=c++20",
      "-isystem $LLVM_DIR/include/c++/v1/include",
      "-stdlib=libc++"
    )
    add_ldflags(
      "-fuse-ld=mold"
    )
    add_links("c++")
target_end()
[~/git/modfix]$ which xmake
/home/linuxbrew/.linuxbrew/bin/xmake
[~/git/modfix]$ xmake clean && clear && xmake -b -v -j 6
generating.moduledeps src/module1.mpp
generating.moduledeps src/module2.mpp
generating.moduledeps src/main_module.mpp
[ 11%]: ccache compiling.release src/module1.mpp
/home/linuxbrew/.linuxbrew/bin/ccache /home/linuxbrew/.linuxbrew/opt/llvm/bin/clang -c -Qunused-arguments -m64 -fvisibility=hidden -fvisibility-inlines-hidden -O3 -I/usr/local/include -I/home/linuxbrew/.linuxbrew/include -I/home/linuxbrew/.linuxbrew/opt/llvm/include -march=native -std=c++20 -isystem $LLVM_DIR/include/c++/v1/include -stdlib=libc++ -DNDEBUG -fmodules -fimplicit-modules -fimplicit-module-maps -fprebuilt-module-path=build/.gens/modfix/linux/x86_64/release/rules/modules/cache --precompile -x c++-module -fmodules-cache-path=build/.gens/modfix/linux/x86_64/release/rules/modules/cache -o build/.gens/modfix/linux/x86_64/release/rules/modules/cache/module1.pcm src/module1.mpp
[ 11%]: ccache compiling.release src/module2.mpp
/home/linuxbrew/.linuxbrew/bin/ccache /home/linuxbrew/.linuxbrew/opt/llvm/bin/clang -c -Qunused-arguments -m64 -fvisibility=hidden -fvisibility-inlines-hidden -O3 -I/usr/local/include -I/home/linuxbrew/.linuxbrew/include -I/home/linuxbrew/.linuxbrew/opt/llvm/include -march=native -std=c++20 -isystem $LLVM_DIR/include/c++/v1/include -stdlib=libc++ -DNDEBUG -fmodules -fimplicit-modules -fimplicit-module-maps -fprebuilt-module-path=build/.gens/modfix/linux/x86_64/release/rules/modules/cache --precompile -x c++-module -fmodules-cache-path=build/.gens/modfix/linux/x86_64/release/rules/modules/cache -o build/.gens/modfix/linux/x86_64/release/rules/modules/cache/module2.pcm src/module2.mpp
/home/linuxbrew/.linuxbrew/bin/ccache /home/linuxbrew/.linuxbrew/opt/llvm/bin/clang -c -Qunused-arguments -m64 -fvisibility=hidden -fvisibility-inlines-hidden -O3 -I/usr/local/include -I/home/linuxbrew/.linuxbrew/include -I/home/linuxbrew/.linuxbrew/opt/llvm/include -march=native -std=c++20 -isystem $LLVM_DIR/include/c++/v1/include -stdlib=libc++ -DNDEBUG -fmodules -fmodules-cache-path=build/.gens/modfix/linux/x86_64/release/rules/modules/cache -fimplicit-modules -fimplicit-module-maps -fprebuilt-module-path=build/.gens/modfix/linux/x86_64/release/rules/modules/cache -o build/.objs/modfix/linux/x86_64/release/src/module1.mpp.o build/.gens/modfix/linux/x86_64/release/rules/modules/cache/module1.pcm
/home/linuxbrew/.linuxbrew/bin/ccache /home/linuxbrew/.linuxbrew/opt/llvm/bin/clang -c -Qunused-arguments -m64 -fvisibility=hidden -fvisibility-inlines-hidden -O3 -I/usr/local/include -I/home/linuxbrew/.linuxbrew/include -I/home/linuxbrew/.linuxbrew/opt/llvm/include -march=native -std=c++20 -isystem $LLVM_DIR/include/c++/v1/include -stdlib=libc++ -DNDEBUG -fmodules -fmodules-cache-path=build/.gens/modfix/linux/x86_64/release/rules/modules/cache -fimplicit-modules -fimplicit-module-maps -fprebuilt-module-path=build/.gens/modfix/linux/x86_64/release/rules/modules/cache -o build/.objs/modfix/linux/x86_64/release/src/module2.mpp.o build/.gens/modfix/linux/x86_64/release/rules/modules/cache/module2.pcm
[ 33%]: ccache compiling.release src/main_module.mpp
/home/linuxbrew/.linuxbrew/bin/ccache /home/linuxbrew/.linuxbrew/opt/llvm/bin/clang -c -Qunused-arguments -m64 -fvisibility=hidden -fvisibility-inlines-hidden -O3 -I/usr/local/include -I/home/linuxbrew/.linuxbrew/include -I/home/linuxbrew/.linuxbrew/opt/llvm/include -march=native -std=c++20 -isystem $LLVM_DIR/include/c++/v1/include -stdlib=libc++ -DNDEBUG -fmodules -fimplicit-modules -fimplicit-module-maps -fprebuilt-module-path=build/.gens/modfix/linux/x86_64/release/rules/modules/cache --precompile -x c++-module -fmodules-cache-path=build/.gens/modfix/linux/x86_64/release/rules/modules/cache -o build/.gens/modfix/linux/x86_64/release/rules/modules/cache/main_module.pcm src/main_module.mpp
error: src/main_module.mpp:3:15: fatal error: module 'main_module.module1' not found
export import main_module.module1;
       ~~~~~~~^~~~~~~~~~~
1 error generated.
[~/git/modfix]$ /usr/local/bin/xmake.new clean && /usr/local/bin/xmake.new -b -j 6
[ 11%]: ccache compiling.release src/module1.mpp
[ 11%]: ccache compiling.release src/module2.mpp
[ 33%]: ccache compiling.release src/main_module.mpp
[ 44%]: ccache compiling.release src/module2.cpp
[ 44%]: ccache compiling.release src/module1.cpp
[ 44%]: ccache compiling.release src/main.cpp
[ 77%]: linking.release modfix
[100%]: build ok!
[~/git/modfix]$ /usr/local/bin/xmake.new r                                        
Hello One
Hello Two

@waruqi
Copy link
Member

waruqi commented Jan 18, 2022

can you upload your demo project?

@aacirino
Copy link
Contributor Author

aacirino commented Jan 18, 2022 via email

@waruqi
Copy link
Member

waruqi commented Jan 18, 2022

thanks, I will look at it in these days

@waruqi
Copy link
Member

waruqi commented Jan 19, 2022

The zip file is attached modfix.zip https://drive.google.com/file/d/1nSYd-79nsJKSkJQaiAg7TzvQ5fuQdx1_/view?usp=drivesdk Em ter., 18 de jan. de 2022 10:14, ruki @.> escreveu:

can you upload your demo project? — Reply to this email directly, view it on GitHub <#1982 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARARENZQEEA3HBCJHGOO5LUWVRS3ANCNFSM5MFMM3MA . You are receiving this because you authored the thread.Message ID: @.
>

I cannot downlload it, can you upload it to github issues attachment?

image

@waruqi
Copy link
Member

waruqi commented Jan 19, 2022

@waruqi waruqi merged commit 093fcdb into xmake-io:dev Jan 19, 2022
@aacirino
Copy link
Contributor Author

Here follows the test project
modfix.zip

@waruqi
Copy link
Member

waruqi commented Jan 19, 2022

Does it work for this patch now?

@aacirino
Copy link
Contributor Author

Yes, I merged the latest master commits and it is working fine. Thanx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants