[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGee572129ae15: [C++20] [Modules] Use - as the separator of partitions when searching (authored by ChuanqiXu). Repository: rG LLVM Github Monorepo

[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

ChuanqiXu updated this revision to Diff 419324. ChuanqiXu added a comment. Rebase CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120874/new/ https://reviews.llvm.org/D120874 Files: clang/lib/Lex/HeaderSearch.cpp clang/test/Modules/search-partitions.cpp Index:

[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

ChuanqiXu updated this revision to Diff 419322. ChuanqiXu added a comment. Herald added a subscriber: dexonsmith. Rebase CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120874/new/ https://reviews.llvm.org/D120874 Files: clang/include/clang/Basic/Module.h

[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

iains accepted this revision. iains added a comment. This revision is now accepted and ready to land. In D120874#3416130 , @arames wrote: > I was was asked to chime in to assess whether this patch could be a problem > for > the prebuilt-implicit clang

[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

arames added a comment. I was was asked to chime in to assess whether this patch could be a problem for the prebuilt-implicit clang modules workflow. No problem here. With prebuilt modules, the output file name has to be specified manually. So the mapping does not change the existing requirement

[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

ChuanqiXu added a comment. In D120874#3406954 , @vsapsai wrote: > By the way, tried a module name with a colon and ModuleMapParser doesn't > accept it > > include/module.modulemap:1:12: error: skipping stray token > module Test:Colon { >

[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

vsapsai added a comment. By the way, tried a module name with a colon and ModuleMapParser doesn't accept it include/module.modulemap:1:12: error: skipping stray token module Test:Colon { ^ include/module.modulemap:1:13: error: expected '{' to start module 'Test' module

[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

vsapsai added a comment. In D120874#3404420 , @ChuanqiXu wrote: > @vsapsai @dblaikie I want to make sure if this one would affect clang > modules. Or simply, would the name of clang modules contain `:`? (`:` is the > separator of C++20 modules

[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

ChuanqiXu added a comment. @vsapsai @dblaikie I want to make sure if this one would affect clang modules. Or simply, would the name of clang modules contain `:`? (`:` is the separator of C++20 modules partitions.) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120874/new/

[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

iains added a comment. I have no comment the patch itself, that seems fine, I even agree that this could be convenient :-) However, I am concerned that this now means that '-fprebuilt-module-path' has meaning for both clang modules and c++20 modules - which is something we agree is generally

[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

ChuanqiXu added a comment. @iains ping~ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120874/new/ https://reviews.llvm.org/D120874 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

ChuanqiXu marked an inline comment as done. ChuanqiXu added a comment. @iains ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120874/new/ https://reviews.llvm.org/D120874 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

ChuanqiXu marked an inline comment as done. ChuanqiXu added a comment. In D120874#3358870 , @tschuett wrote: > Stupid question: this works with Windows as well? And the `BTW` sounds odd. > gcc also decided to use a dash as the separator. It should work

[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

ChuanqiXu updated this revision to Diff 412928. ChuanqiXu added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120874/new/ https://reviews.llvm.org/D120874 Files: clang/lib/Lex/HeaderSearch.cpp clang/test/Modules/search-partitions.cpp Index:

[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

tschuett added inline comments. Comment at: clang/lib/Lex/HeaderSearch.cpp:199 + // The separator of C++20 modules partitions (':') is not good for file + // systems, here we choose '-' by default since it is not a valid + // character of C++ indentifiers. So we

[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

tschuett added a comment. Stupid question: this works with Windows as well? And the `BTW` sounds odd. gcc also decided to use a dash as the separator. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120874/new/ https://reviews.llvm.org/D120874

[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

ChuanqiXu added inline comments. Comment at: clang/test/Modules/search-partitions.cppm:20 +//--- partition1.cpp +export module A : Part1; + iains wrote: > ChuanqiXu wrote: > > @MyDeveloperDay hi, I remember the support for partitions in clang-format > > is done

[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

ChuanqiXu updated this revision to Diff 412899. ChuanqiXu added a comment. Format tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120874/new/ https://reviews.llvm.org/D120874 Files: clang/lib/Lex/HeaderSearch.cpp clang/test/Modules/search-partitions.cpp Index:

[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

iains added inline comments. Comment at: clang/test/Modules/search-partitions.cppm:20 +//--- partition1.cpp +export module A : Part1; + ChuanqiXu wrote: > @MyDeveloperDay hi, I remember the support for partitions in clang-format is > done in

[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

ChuanqiXu added a subscriber: MyDeveloperDay. ChuanqiXu added inline comments. Comment at: clang/test/Modules/search-partitions.cppm:20 +//--- partition1.cpp +export module A : Part1; + @MyDeveloperDay hi, I remember the support for partitions in clang-format is

[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

ChuanqiXu created this revision. ChuanqiXu added reviewers: urnathan, iains. ChuanqiXu added a project: clang. Herald added a project: All. ChuanqiXu requested review of this revision. Herald added a subscriber: cfe-commits. It is simpler to search for module unit by `-fprebuilt-module-path`