[PATCH] D133341: [C++] [Coroutines] Prefer aligned (de)allocation for coroutines - implement the option2 of P2014R0

2022-09-15 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments. Comment at: clang/test/SemaCXX/coroutine-alloc-4.cpp:49 +void return_value(int) {} +void *operator new(std::size_t, std::align_val_t) noexcept; +void *operator new(std::size_t) noexcept; ychen wrote: > ChuanqiXu wrote:

[PATCH] D134105: [Docs] Add a link that refers to C++ standard modules in Clang modules doc

2022-09-17 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. Thanks. I wanted to do this. I suggest to do the change in later sections. Since the document talks more than the clang modules extension. It also describes some general module properties and semantics. It is a pretty document and from my point of view, it shouldn be

[PATCH] D134105: [Docs] Add a link that refers to C++ standard modules in Clang modules doc

2022-09-18 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu accepted this revision. ChuanqiXu added inline comments. This revision is now accepted and ready to land. Comment at: clang/docs/Modules.rst:107-108 + +.. note:: + This pages mainly talks about the Clang modules extension and describes some general

[PATCH] D134249: [modules] Fix error "malformed or corrupted AST file: 'SourceLocation remap refers to unknown module...'".

2022-09-19 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu accepted this revision. ChuanqiXu added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Serialization/ModuleManager.cpp:284-286 for (ModuleIterator victim = First; victim != Last; ++victim) { Modules.erase(victim->File);

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

2022-09-20 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu created this revision. ChuanqiXu added reviewers: iains, dblaikie, Bigcheese, vsapsai. ChuanqiXu added projects: clang, clang-modules. Herald added a project: All. ChuanqiXu requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. This patch tries to make clang

[PATCH] D134269: [docs] Document the one-phase style compilation to c++ standard modules

2022-09-20 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu created this revision. ChuanqiXu added reviewers: iains, dblaikie, ruoso, ben.boeckel. ChuanqiXu added a project: clang-modules. Herald added a project: All. ChuanqiXu requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This is the fo

[PATCH] D134269: [docs] Document the one-phase style compilation to c++ standard modules

2022-09-20 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 461782. ChuanqiXu marked 2 inline comments as done. ChuanqiXu added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134269/new/ https://reviews.llvm.org/D134269 Files: clang/docs/StandardCPlusPlusModules.rst Index: c

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

2022-09-21 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. Tested with our internal workloads and things look fine. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126907/new/ https://reviews.llvm.org/D126907 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lis

[PATCH] D133341: [C++] [Coroutines] Prefer aligned (de)allocation for coroutines - implement the option2 of P2014R0

2022-09-21 Thread Chuanqi Xu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG327141fb1d8c: [C++] [Coroutines] Prefer aligned (de)allocation for coroutines - (authored by ChuanqiXu). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133341

[PATCH] D131858: [clang] Track the templated entity in type substitution.

2022-09-23 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments. Comment at: clang/lib/Serialization/ASTWriterDecl.cpp:623-626 + VisitDeclaratorDecl(D); + Record.AddDeclarationNameLoc(D->DNLoc, D->getDeclName()); + Record.push_back(D->getIdentifierNamespace()); + ChuanqiXu wrote: > mizvekov

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

2022-09-25 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 462700. ChuanqiXu added a comment. - Add ReleaseNotes. - Refine the behavior about partitions. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134267/new/ https://reviews.llvm.org/D134267 Files: clang/docs/ReleaseNotes.rst clang/include/clang/B

[PATCH] D134269: [docs] Document the one-phase style compilation to c++ standard modules

2022-09-25 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 462701. ChuanqiXu added a comment. Refine the behavior about partitions. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134269/new/ https://reviews.llvm.org/D134269 Files: clang/docs/StandardCPlusPlusModules.rst Index: clang/docs/StandardCPlusP

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

2022-10-07 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a reviewer: rsmith. ChuanqiXu added a comment. In D134267#3815453 , @dblaikie wrote: >> This also tries to fix the problem I raised a year ago: >> https://discourse.llvm.org/t/make-command-line-support-for-c-20-module-uniform-with-gcc/591

[PATCH] D135472: [ODRHash] Hash attributes on declarations.

2022-10-07 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. I guess the reason why we didn't check odr violation for attributes is that the attributes was not a part of declarations in C++ before. But it is odd to skip the check for attributes from the perspective of users. So I think this one should be good. The only concern

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

2022-10-08 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D134267#3844605 , @ChuanqiXu wrote: > In D134267#3815453 , @dblaikie > wrote: > >>> This also tries to fix the problem I raised a year ago: >>> https://discourse.llvm.org/t/make-com

[PATCH] D134269: [docs] Document the one-phase style compilation to c++ standard modules

2022-10-09 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D134269#3813568 , @tschuett wrote: > Thanks for doing this! The traditional Clang Header modules call this > implicit and explicit modules. > > Feel free to ignore, but I found this awesome article about implicit and > expl

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

2022-10-10 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D134267#3846153 , @iains wrote: > @ChuanqiXu BTW, I think we (unfortunately) have some overlap in work in > progress here; > > I have implemented a similar scheme to GCC's where C/BMIs can be produced "on > demand" when the

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

2022-10-10 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D134267#3846264 , @iains wrote: > In D134267#3846218 , @ChuanqiXu > wrote: > >> In D134267#3846153 , @iains wrote: >> >>> @ChuanqiXu BTW, I

[PATCH] D131858: [clang] Track the templated entity in type substitution.

2022-10-10 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments. Comment at: clang/lib/Serialization/ASTWriterDecl.cpp:623-626 + VisitDeclaratorDecl(D); + Record.AddDeclarationNameLoc(D->DNLoc, D->getDeclName()); + Record.push_back(D->getIdentifierNamespace()); + mizvekov wrote: > ChuanqiXu

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

2022-10-10 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D134267#3847399 , @iains wrote: > FAOD: I think this should not be about "who's patches" - but about the > compilation model and command lines that we want to end up with. Yeah, of course. > I am concerned that with the cu

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

2022-10-11 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. > FAOD: I think this should not be about "who's patches" - but about the > compilation model and command lines that we want to end up with. BTW, in the current context, when I say "my" and "your", I just want to refer the corresponding things. There is no means to off

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

2022-10-11 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D134267#3848793 , @iains wrote: > In D134267#3848745 , @ChuanqiXu > wrote: > >>> FAOD: I think this should not be about "who's patches" - but about the >>> compilation model and com

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

2022-10-11 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D134267#3848822 , @tschuett wrote: > Clang header modules started with implicit builds and module caches. Apple is > moving too explicit builds. There are all kinds of problems with implicit > module builds. I believe that

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

2022-10-11 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D134267#3848876 , @iains wrote: > If we are going to **//require//** the serialisation/deserialisation for > correctness then IMO we should have an error for an incorrect compilation. Of course. > I still feel that deseria

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

2022-10-11 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D134267#3848958 , @iains wrote: > In D134267#3848883 , @ChuanqiXu > wrote: > >> In D134267#3848876 , @iains wrote: >> >>> If we are going to

[PATCH] D135472: [ODRHash] Hash attributes on declarations.

2022-10-11 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. > In the near future I was planning to add various Objective-C and Swift > attributes. For other attributes I don't know which are high-value. I > definitely want to check attributes affecting the memory layout (alignment, > packing) and believe I've addressed them (t

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

2022-10-11 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a reviewer: tschuett. ChuanqiXu added a comment. In D134267#3849629 , @dblaikie wrote: > To the original point I was making about implicit modules (which might've > been my own confusion due to the term being brought up in D134269 >

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

2022-10-12 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D134267#3852047 , @boris wrote: > Just to add a few points on the module mapper discussion: overall, the > current view is that there are two ways to handle C++20 modules from the > build system's POV: (1) pre-scan the code

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

2022-10-13 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D134267#3852136 , @boris wrote: >> For example, my experimental support for P1689 >> is at: [...]. The implementation looks >> relatively trivial to me. The simplicity is pretty important. >

[PATCH] D135472: [ODRHash] Hash attributes on declarations.

2022-10-13 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments. Comment at: clang/lib/AST/ODRHash.cpp:479-480 + + llvm::copy_if(D->attrs(), std::back_inserter(HashableAttrs), +[](const Attr *A) { return !A->isImplicit(); }); +} aaron.ballman wrote: > ChuanqiXu wrote: > > vsaps

[PATCH] D135472: [ODRHash] Hash attributes on declarations.

2022-10-13 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu accepted this revision. ChuanqiXu added a comment. This revision is now accepted and ready to land. LGTM basically. Our discussion is mainly about the future work in Attr.td and not this patch. So I think they are not blocking issues. Comment at: clang/lib/AST/ODRDia

[PATCH] D131858: [clang] Track the templated entity in type substitution.

2022-10-13 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu accepted this revision. ChuanqiXu added a comment. The codes look good and also the tests pass. So LGTM. Thanks! Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:1082-1094 +auto merge = [this, &Redecl, FD](auto &&F) { + auto *Existing = cast_or_null(Red

[PATCH] D135472: [ODRHash] Hash attributes on declarations.

2022-10-14 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments. Comment at: clang/lib/AST/ODRHash.cpp:479-480 + + llvm::copy_if(D->attrs(), std::back_inserter(HashableAttrs), +[](const Attr *A) { return !A->isImplicit(); }); +} aaron.ballman wrote: > ChuanqiXu wrote: > > aaron

[PATCH] D132352: Introduce noread_thread_id to address the thread identification problem in coroutines

2022-10-16 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. We prefer https://reviews.llvm.org/D135550 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132352/new/ https://reviews.llvm.org/D132352 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D132352: Introduce noread_thread_id to address the thread identification problem in coroutines

2022-10-16 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. We prefer https://reviews.llvm.org/D135550 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132352/new/ https://reviews.llvm.org/D132352 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D132434: Add noread_thread_id attribute to intrinsics

2022-10-16 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu abandoned this revision. ChuanqiXu added a comment. We prefer https://reviews.llvm.org/D135550 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132434/new/ https://reviews.llvm.org/D132434 ___ cfe

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

2022-10-17 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 468138. ChuanqiXu added a comment. Address @tschuett's opinion: - Use new introduced `-fc++-module-cache-path=` instead of `-fc++-module-path` to avoid many logics about modules cache in clang modules. - Use new introduced `-fmodule-bmi-output=` instead of

[PATCH] D134269: [docs] Document the one-phase style compilation to c++ standard modules

2022-10-17 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 468139. ChuanqiXu added a comment. Update to reflect the change in D134267 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134269/new/ https://reviews.llvm.org/D134269 Files: clang/docs/StandardCPlusPlusModules.

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

2022-10-17 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. @dblaikie @iains @tschuett @boris There are many opinions in this page. They are about problems in the higher level. They are pretty important and helpful indeed. I'm really appreciated it. But most of them are not related to this diff itself and I don't think they a

[PATCH] D135772: Stop evaluating trailing requires clause after overload resolution

2022-10-17 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. LGTM basically. Comment at: clang/lib/Sema/SemaExpr.cpp:6748 + SourceLocation ConstraintFailLoc = NakedFn->getBeginLoc(); + It looks like `ConstraintFailLoc` is not used? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1357

[PATCH] D135772: Stop evaluating trailing requires clause after overload resolution

2022-10-17 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu accepted this revision. ChuanqiXu added a comment. This revision is now accepted and ready to land. LGTM then. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135772/new/ https://reviews.llvm.org/D135772 ___ cfe-commits mailing list c

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

2022-10-17 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added subscribers: ruoso, ben.boeckel. ChuanqiXu added a comment. >> Although modular code is user-facing - BMIs are an implementational detail, >> right? > > but I don't think BMIs are an implementation detail, anymore than object > files are - users should/will be as aware of BMIs as

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

2022-10-17 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. > Yes, all of this stuff is important (and, yes, we are drifting into other > [related] topics) - we already have a mechanism for providing function bodies > to optimisation [LTO] - I do not think we should want to make module > interfaces larger than necessary to dup

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

2022-10-18 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D134267#3864248 , @iains wrote: > I am also OK with doing this in two steps (first in the driver with this > patch and then by updating the FE to allow the two outputs from one > invocation - my draft patch series). For yo

[PATCH] D126694: [C++20][Modules] Implementation of GMF decl elision.

2022-10-18 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. I haven't looked into the details. But I feel the feature worth a sentence in ReleaseNotes. (Maybe in the potential-breaking-change section). Also I think it'll be better to have an option `-fenable-discarding-unreachable-decls-in-gmf` to control the behavior. It'll

[PATCH] D140261: [C++20][Modules] Do not allow non-inline external definitions in header units.

2022-12-18 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu accepted this revision. ChuanqiXu added a comment. This revision is now accepted and ready to land. LGTM generally. Comment at: clang/lib/Sema/SemaDecl.cpp:12957-12958 + // units. + if (getLangOpts().CPlusPlus20 && getLangOpts().CPlusPlusModules && + !ModuleSco

[PATCH] D137058: [Driver] [Modules] Support -fmodule-output (1/2)

2022-12-18 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments. Comment at: clang/lib/Driver/Driver.cpp:5541-5545 +if (Arg *FinalOutput = C.getArgs().getLastArg(options::OPT_o)) + TempPath = FinalOutput->getValue(); +else + TempPath = BaseInput; + tahonermann wrote: > Chuanqi

[PATCH] D140261: [C++20][Modules] Do not allow non-inline external definitions in header units.

2022-12-19 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:12957-12958 + // units. + if (getLangOpts().CPlusPlus20 && getLangOpts().CPlusPlusModules && + !ModuleScopes.empty() && ModuleScopes.back().Module->isHeaderUnit()) { +if (VDecl->getFormalLinkage()

[PATCH] D140261: [C++20][Modules] Do not allow non-inline external definitions in header units.

2022-12-19 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D140261#4004542 , @iains wrote: > OK so this is what I plan to land assuming testing goes OK. > I suspect that this might cause some user code to flag errors - there are > quite a number of ODR violations "in the wild". I f

[PATCH] D140261: [C++20][Modules] Do not allow non-inline external definitions in header units.

2022-12-19 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. > So it's not a change that breaks existing code - because no-one has Header > Units in existing code. We can't assume this. Since we already said people can try to use header units in clang15. We can't assume there is nobody using header units just because they don'

[PATCH] D139168: [C++20] [Modules] [ClangScanDeps] Enable to print make-style dependency file within P1689 format (4/4)

2022-12-21 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. @Bigcheese @jansvoboda11 gentle ping~ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139168/new/ https://reviews.llvm.org/D139168 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin

[PATCH] D137534: [C++20] [Modules] [ClangScanDeps] Allow clang-scan-deps to without specified compilation database in P1689 (3/4)

2022-12-22 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. > Why is it necessary to add new command-line flags for this? Can't the input > and output be inherited from the specified Clang command line? Would such > command line make sense? CMake wants to query the dependency information for a single file from time to time du

[PATCH] D137534: [C++20] [Modules] [ClangScanDeps] Allow clang-scan-deps to without specified compilation database in P1689 (3/4)

2022-12-22 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 485022. ChuanqiXu added a comment. Address comments: fix a typo. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137534/new/ https://reviews.llvm.org/D137534 Files: clang/include/clang/Tooling/CompilationDatabase.h clang/lib/Tooling/Compilation

[PATCH] D137534: [C++20] [Modules] [ClangScanDeps] Allow clang-scan-deps to without specified compilation database in P1689 (3/4)

2022-12-22 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. > My GCC patch names it -fdepfile-output= instead of --p1689-targeted-output, > gets --p1689-targeted-file-name from the regular command line (it looks like > a preprocessor command overall) and the --p1689-makeformat-output flag is > handled by the normal -MT and -MF

[PATCH] D140055: [ODRHash] Detect mismatches in anonymous `RecordDecl`.

2022-12-25 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu accepted this revision. ChuanqiXu added a comment. This revision is now accepted and ready to land. LGTM since it shouldn't be bad to call `getMostRecentDecl()` multiple times. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140055/new/ htt

[PATCH] D140073: [ODRHash] Hash `ObjCInterfaceDecl` and diagnose discovered mismatches.

2022-12-25 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu accepted this revision. ChuanqiXu added a comment. This revision is now accepted and ready to land. Although I'm not familiar with ObjC, the code looks good. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140073/new/ https://reviews.llvm.o

[PATCH] D137058: [Driver] [Modules] Support -fmodule-output (1/2)

2022-12-27 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 485457. ChuanqiXu added a comment. Address comments: Reject the command line if it specifies -fmodule-output and multiple arch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137058/new/ https://reviews.llvm.org/D137058 Files: clang/include/cla

[PATCH] D140927: [C++20][Modules] Fix named module import diagnostics.

2023-01-03 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. [module.import]p1 says: > In a module unit, all module-import-declarations and export-declarations > exporting module-import-declarations shall appear before all other > declarations in the declaration-seq of the translation-unit and of the > private-module-fragment

[PATCH] D137058: [Driver] [Modules] Support -fmodule-output (1/2)

2023-01-03 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 486132. ChuanqiXu added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137058/new/ https://reviews.llvm.org/D137058 Files: clang/include/clang/Basic/DiagnosticDriverKinds.td clang/include/clang/Driver/Options.td

[PATCH] D137534: [C++20] [Modules] [ClangScanDeps] Allow clang-scan-deps to without specified compilation database in P1689 (3/4)

2023-01-03 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. > I'm getting confused by the semantics of --p1689-targeted-output=. In D137527 > , it's used to populate > CompilerCommand::Output which is supposed to be the main compiler output (.o > file). Is that correct? If that's not the case

[PATCH] D137534: [C++20] [Modules] [ClangScanDeps] Allow clang-scan-deps to without specified compilation database in P1689 (3/4)

2023-01-04 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 486195. ChuanqiXu added a comment. Address comments from @jansvoboda11: use the style of `clang-scan-deps -format=p1689 -- -std=c++20 -o ` instead of `clang-scan-deps -format=p1689 --p1689-targeted-file-name= --p1689-targeted-output= -- -std=c++20`.

[PATCH] D137534: [C++20] [Modules] [ClangScanDeps] Allow clang-scan-deps to without specified compilation database in P1689 (3/4)

2023-01-04 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments. Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:571-581 + + // FIXME: createInvocation will drop the `-o` option since it requires + // `-fsyntax-only`. So here we try to parse the output file manually. + auto CommandLineIter = +

[PATCH] D139168: [C++20] [Modules] [ClangScanDeps] Enable to print make-style dependency file within P1689 format (4/4)

2023-01-04 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 486196. ChuanqiXu added a comment. Update since https://reviews.llvm.org/D137534 changes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139168/new/ https://reviews.llvm.org/D139168 Files: clang/include/clang/Tooling/DependencyScanning/Dependenc

[PATCH] D139168: [C++20] [Modules] [ClangScanDeps] Enable to print make-style dependency file within P1689 format (4/4)

2023-01-04 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. Currently we will detect `-MF` in the command line and we will write the make-format dependency output to the corresponding file once we find `-MF`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139168/new/ https://reviews.llvm.org/D139168 __

[PATCH] D137534: [C++20] [Modules] [ClangScanDeps] Allow clang-scan-deps to without specified compilation database in P1689 (3/4)

2023-01-04 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments. Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:571-581 + + // FIXME: createInvocation will drop the `-o` option since it requires + // `-fsyntax-only`. So here we try to parse the output file manually. + auto CommandLineIter = +

[PATCH] D140927: [C++20][Modules] Fix named module import diagnostics.

2023-01-04 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. From the discussion in https://github.com/llvm/llvm-project/issues/59688, I feel you are correct. But let's wait for we get a clarification. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140927/new/ https://reviews.llvm

[PATCH] D140261: [C++20][Modules] Do not allow non-inline external definitions in header units.

2023-01-04 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu accepted this revision. ChuanqiXu added a comment. Yeah, it looks good to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140261/new/ https://reviews.llvm.org/D140261 ___ cfe-commits mailing

[PATCH] D137534: [C++20] [Modules] [ClangScanDeps] Allow clang-scan-deps to without specified compilation database in P1689 (3/4)

2023-01-04 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. > Note that multi-arch builds are an open question (just like multi-arch .pcm > files are I believe); the format could support it as two reports on > provides/reqs, but the merged object file complicates things. BTW, we decide to forbid multiple-arch .pcm files in ht

[PATCH] D139168: [C++20] [Modules] [ClangScanDeps] Enable to print make-style dependency file within P1689 format (4/4)

2023-01-04 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D139168#4026799 , @ben.boeckel wrote: > In D139168#4025277 , @ChuanqiXu > wrote: > >> Currently we will detect `-MF` in the command line and we will write the >> make-format depend

[PATCH] D139168: [C++20] [Modules] [ClangScanDeps] Enable to print make-style dependency file within P1689 format (4/4)

2023-01-04 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 486472. ChuanqiXu added a comment. Add the test to generate make-style dependency file with compilation database. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139168/new/ https://reviews.llvm.org/D139168 Files: clang/include/clang/Tooling/Depe

[PATCH] D139168: [C++20] [Modules] [ClangScanDeps] Enable to print make-style dependency file within P1689 format (4/4)

2023-01-04 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. @ben.boeckel I updated the test with compilation db. BTW, I am curious that according to https://reviews.llvm.org/D137534, cmake may not use clang-scan-deps with compilation db for P1689 . So this is a pure review comment and it is not

[PATCH] D140927: [C++20][Modules] Fix named module import diagnostics.

2023-01-05 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu accepted this revision. ChuanqiXu added a comment. This revision is now accepted and ready to land. LGTM. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140927/new/ https://reviews.llvm.org/D140927

[PATCH] D137058: [Driver] [Modules] Support -fmodule-output (1/2)

2023-01-05 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. @dblaikie would you like to take a look at this? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137058/new/ https://reviews.llvm.org/D137058 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.

[PATCH] D139168: [C++20] [Modules] [ClangScanDeps] Enable to print make-style dependency file within P1689 format (4/4)

2023-01-05 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. > IMO, this is not suitable; there must be *one* depfile for Ninja to work > (otherwise build tools will need to manually collate all of this into one for > ninja. Yeah, but the producer of the compilation database is able to create a compilation db with the same `-M

[PATCH] D139168: [C++20] [Modules] [ClangScanDeps] Enable to print make-style dependency file within P1689 format (4/4)

2023-01-05 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. >> In what mode will clang output #include information without erroring about >> missing .pcm files for import statements? -E -fdirectives-only perhaps? This >> further exacerbates the need for "fake" command lines and costs on platforms >> with expensive process exec

[PATCH] D137534: [C++20] [Modules] [ClangScanDeps] Allow clang-scan-deps to without specified compilation database in P1689 (3/4)

2023-01-08 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. gentle ping @jansvoboda11 . sorry for the a little bit hurried ping since LLVM-16 is going to be branched in the 24th of January. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137534/new/ https://reviews.llvm.org/D137534

[PATCH] D137058: [Driver] [Modules] Support -fmodule-output (1/2)

2023-01-08 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D137058#4033825 , @h-vetinari wrote: > Without undue haste, I just wanted to comment from the peanut gallery that it > would be amazing if the patches that are necessary for CMake + Clang to use > C++ modules would make th

[PATCH] D139168: [C++20] [Modules] [ClangScanDeps] Enable to print make-style dependency file within P1689 format (4/4)

2023-01-08 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. > Hi, just wanted to say that i added support of these patch to XMake (i built > a LLVM with your 4 patches) and it work pretty well :) (except for header > units) @Arthapz Great to hear that! It is pretty important for us to know there are more build systems which a

[PATCH] D141023: [C++20] [Modules] Make placement allocation functions always acceptable

2023-01-08 Thread Chuanqi Xu via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGee1be282241b: [C++20] [Modules] Make placement allocation functions always visible (authored by ChuanqiXu). Herald added a project: clang. Herald add

[PATCH] D139168: [C++20] [Modules] [ClangScanDeps] Enable to print make-style dependency file within P1689 format (4/4)

2023-01-08 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. > FWIW, the same with should happen with -MT. Yeah, it should be. Since the patch reuses the previous logics. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139168/new/ https://reviews.llvm.org/D139168 ___ cfe-commi

[PATCH] D140867: [C++20] [Modules] Don't generate global ctors/dtors for variables which are available externally

2023-01-08 Thread Chuanqi Xu via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG08f957808e5f: [C++20] [Modules] Don't generate global ctors/dtors for variables which areā€¦ (authored by ChuanqiXu). Herald added a project: clang. He

[PATCH] D140867: [C++20] [Modules] Don't generate global ctors/dtors for variables which are available externally

2023-01-09 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D140867#4036239 , @Jake-Egan wrote: > Hi, this test is failing on AIX, could you take a look please? > https://lab.llvm.org/buildbot/#/builders/214/builds/5242/steps/6/logs/FAIL__Clang__pr59765-modules-global-ctor-dtor_cppm

[PATCH] D137534: [C++20] [Modules] [ClangScanDeps] Allow clang-scan-deps to without specified compilation database in P1689 (3/4)

2023-01-09 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 487692. ChuanqiXu added a comment. Address comments: - Add more comments in the test. - Avoid ad-hoc parsing `-o` option manually. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137534/new/ https://reviews.llvm.org/D137534 Files: clang/test/Cla

[PATCH] D137534: [C++20] [Modules] [ClangScanDeps] Allow clang-scan-deps to without specified compilation database in P1689 (3/4)

2023-01-09 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu marked 3 inline comments as done. ChuanqiXu added a comment. > I originally investigated (and ended up lost) for something like GCC where > P1689 information is extracted via -E > -fdep-file=p1689.json -fdep-output=module.o -fdep-format=p1689 (which is

[PATCH] D139168: [C++20] [Modules] [ClangScanDeps] Enable to print make-style dependency file within P1689 format (4/4)

2023-01-09 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D139168#4036850 , @tahonermann wrote: >> Header units have even more need to be involved in the build graph than >> named units. ODR violations and cache invalidation problems await anyone >> just winging it on header unit

[PATCH] D141450: [Clang][cc1] Add -fno-modules-local-submodule-visibility to override the default

2023-01-10 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu accepted this revision. ChuanqiXu added a comment. This revision is now accepted and ready to land. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141450/new/ https://reviews.llvm.org/D141450

[PATCH] D137058: [Driver] [Modules] Support -fmodule-output (1/2)

2023-01-10 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 488057. ChuanqiXu added a comment. Address comments: merge the conditions. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137058/new/ https://reviews.llvm.org/D137058 Files: clang/include/clang/Basic/DiagnosticDriverKinds.td clang/include/clan

[PATCH] D137058: [Driver] [Modules] Support -fmodule-output (1/2)

2023-01-10 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu marked an inline comment as done. ChuanqiXu added inline comments. Comment at: clang/lib/Driver/Driver.cpp:5736 + C.getArgs().hasArg(options::OPT_fmodule_output) && + C.getArgs().hasArg(options::OPT_o)) { +SmallString<128> OutputPath; dbla

[PATCH] D140867: [C++20] [Modules] Don't generate global ctors/dtors for variables which are available externally

2023-01-10 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D140867#4040590 , @Jake-Egan wrote: > In D140867#4038388 , @ChuanqiXu > wrote: > >> In D140867#4036239 , @Jake-Egan >> wrote: >> >>> Hi, th

[PATCH] D140867: [C++20] [Modules] Don't generate global ctors/dtors for variables which are available externally

2023-01-10 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D140867#4042351 , @Jake-Egan wrote: >> Would you like to take a double look? > > Yes, it still fails on the bot and on my local machine. I've added an XFAIL > to the test for now just to get the AIX bot green. Weird. Maybe

[PATCH] D137059: [Driver] [Modules] Introduce -fsave-std-c++-module-file= to specify the path of the module file (2/2)

2022-11-02 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D137059#3899339 , @ben.boeckel wrote: > There is another motivating factor for 1-phase: the build graph is far > simpler. With 2-phase, CMake will have to write out rules to perform: > > - source -> .bmi > - .bmi -> .withbm

[PATCH] D134267: [C++] [Modules] Support one phase compilation model for named modules

2022-11-02 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D134267#3902564 , @ben.boeckel wrote: > I tried applying this patch to your MyP1689 branch (fixing conflicts with > attempts there), but it seems that something isn't being plumbed properly: > > clang-15: error: unknown a

[PATCH] D137058: [Driver] [Modules] Support -fsave-std-c++-module-file (1/2)

2022-11-03 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. > so I can appreciate the -fsave-std-c++-module-file name here, but it does > sound a bit verbose? My thought is the option is mainly used by build systems so that the users wouldn't type it frequently. In this case, I feel it might not be bad to make it a little bit

[PATCH] D137514: [clang-tidy] add check for capturing lambda coroutines

2022-11-06 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. This is helpful. I feel it is OK to add the warning in the clang too. Are you interested in that? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137514/new/ https://reviews.llvm.org/D137514 __

[PATCH] D137534: [C++20] [Modules] [ClangScanDeps] Support to query separate file for P1689

2022-11-06 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu created this revision. ChuanqiXu added reviewers: Bigcheese, jansvoboda11, dblaikie, iains, ben.boeckel. Herald added a project: All. ChuanqiXu requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This functionality is required by CMa

[PATCH] D137514: [clang-tidy] add check for capturing lambda coroutines

2022-11-07 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D137514#3912626 , @dotnwat wrote: > In D137514#3911046 , @ChuanqiXu > wrote: > >> This is helpful. I feel it is OK to add the warning in the clang too. Are >> you interested in that

[PATCH] D137609: [C++20] [Modules] Remove unmaintained header modules

2022-11-07 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu created this revision. ChuanqiXu added reviewers: rsmith, iains, dblaikie, Bigcheese. ChuanqiXu added a project: clang-modules. Herald added a project: All. ChuanqiXu requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Currently there

[PATCH] D137609: [C++20] [Modules] Remove unmaintained header modules

2022-11-07 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 473884. ChuanqiXu added a comment. Remove other logics. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137609/new/ https://reviews.llvm.org/D137609 Files: clang/include/clang/Frontend/FrontendActions.h clang/include/clang/Frontend/FrontendOpti

[PATCH] D137609: [C++20] [Modules] Remove unmaintained header modules

2022-11-08 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a reviewer: bruno. ChuanqiXu added a comment. In D137609#3915632 , @iains wrote: > I think that (if this change is approved) there will be also some > simplifications possible in the driver (since the mode that produces a > wrapper hea

[PATCH] D137693: [NFC] [C++20] [Modules] [clangd] Add test for code completion for C++20 Named Modules

2022-11-09 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu created this revision. ChuanqiXu added reviewers: nridge, sammccall. ChuanqiXu added a project: clang-tools-extra. Herald added subscribers: kadircet, arphaman. Herald added a project: All. ChuanqiXu requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-b

<    2   3   4   5   6   7   8   9   10   11   >