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

ChuanqiXu added inline comments. Comment at: clang-tools-extra/clangd/test/completion-modules2.test:13 +export module A; +void printA() {} + It looks redundant at the first sight. I know we can reduce the size by the following trick: ``` #ifdef USE_EXPORT expor

[PATCH] D137724: [CMake] Warn when the version is older than 3.20.0.

ChuanqiXu accepted this revision. ChuanqiXu added a comment. LGTM. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137724/new/ https://reviews.llvm.org/D137724 ___ cfe-commits mailing list cfe-comm

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

ChuanqiXu updated this revision to Diff 474445. ChuanqiXu added a comment. Add dependency to clangd tests to make sure they can find the corresponding rules. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137693/new/ https://reviews.llvm.org/D137693 Files: clang-tools-extra/clangd/te

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

ChuanqiXu updated this revision to Diff 474462. ChuanqiXu added a comment. Set the path to clang correctly. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137693/new/ https://reviews.llvm.org/D137693 Files: clang-tools-extra/clangd/test/CMakeLists.txt clang-tools-extra/clangd/test/c

[PATCH] D137770: [docs] Introduce clangd part in StandardCPlusPlusModules

ChuanqiXu created this revision. ChuanqiXu added reviewers: sammccall, nridge. Herald added subscribers: kadircet, arphaman. Herald added a project: All. ChuanqiXu requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang. Intro

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

ChuanqiXu added a comment. The failed tests in windows says: > Failed to find compilation database for > \\C:\ws\w9\llvm-project\premerge-checks\build\tools\clang\tools\extra\clangd\test\Output\completion-modules.test.tmp\Use.cpp CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137693/new

[PATCH] D137770: [docs] Introduce clangd part in StandardCPlusPlusModules

ChuanqiXu planned changes to this revision. ChuanqiXu added a comment. @sammccall Thanks for you professional and valuable input. It is very helpful. From the perspective of clangd, it looks not good indeed that the user can't get a stable support for modules. > This situation is not at all obv

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

ChuanqiXu planned changes to this revision. ChuanqiXu added a comment. Thanks for the explanation. I got your point. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137693/new/ https://reviews.llvm.org/D137693 ___ cfe-commits mailing list cfe-c

[PATCH] D137787: [CodeGen] Relax assertion on generating destructor call

ChuanqiXu added a comment. I think it might be necessary/better to add a test for this. Otherwise, it looks not good to change this. > This fixes the assertion for a downstream case in ROOT/Cling with the > involvement of modules. If anyone has ideas how to test this, please let me > know...

[PATCH] D130327: [ODRHash] Detect duplicate `ObjCProtocolDecl` ODR mismatches during parsing.

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 change here looks good generally. Comment at: clang/include/clang/Sema/Sema.h:10091-10096 ObjCProtocolDecl *ActOnStartProtoc

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

ChuanqiXu added a comment. @dblaikie @ben.boeckel gentle ping~ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137058/new/ https://reviews.llvm.org/D137058 ___ cfe-commits mailing list cfe-commits@lists.ll

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

ChuanqiXu added a comment. @ben.boeckel gentle ping~ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137059/new/ https://reviews.llvm.org/D137059 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

[PATCH] D137787: [CodeGen] Relax assertion on generating destructor call

ChuanqiXu added a comment. In D137787#3921673 , @Hahnfeld wrote: > I've trimmed the failing code down to > > #include > #include > #include > > template > struct SO { > void a() { > struct SI { > std::vector v; > }

[PATCH] D137787: [CodeGen] Relax assertion on generating destructor call

ChuanqiXu added a comment. In D137787#3923540 , @Hahnfeld wrote: >> Is it the reproducer? > > No, as I wrote: > >> Sadly this works fine in standalone Clang... Sorry, I don't understand well. Could you rewrite the reproducer in the style I wrote? And in

[PATCH] D137787: [CodeGen] Relax assertion on generating destructor call

ChuanqiXu added a comment. In D137787#3924341 , @Hahnfeld wrote: >> Sorry, I don't understand well. Could you rewrite the reproducer in the >> style I wrote? And in what cases it works fine? > > Okay, let me try to restate: I can reproduce the problem in

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

ChuanqiXu added a comment. @bruno ping~ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137609/new/ https://reviews.llvm.org/D137609 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

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

ChuanqiXu updated this revision to Diff 475663. 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/Driver/Options.td clang/lib/Driver/Driver.cpp clang/test/Driver/save-

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

ChuanqiXu marked 2 inline comments as done. ChuanqiXu added inline comments. Comment at: clang/lib/Driver/Driver.cpp:5541-5542 + // + // FIXME: Do we need to handle the case that the calculated output is + // conflicting with the specified output file or the input file? + if

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

ChuanqiXu updated this revision to Diff 475664. ChuanqiXu added a comment. Use tests with `-###` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137059/new/ https://reviews.llvm.org/D137059 Files: clang/include/clang/Driver/Options.td clang/lib/Driver/Driver.cpp clang/test/Driver/s

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

ChuanqiXu updated this revision to Diff 475719. ChuanqiXu retitled this revision from "[C++20] [Modules] [ClangScanDeps] Support to query separate file for P1689 (3/3)" to "[C++20] [Modules] [ClangScanDeps] Allow clang-scan-deps to without specified compilation database in P1689". ChuanqiXu edit

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

ChuanqiXu added a comment. @dblaikie gentle ping~ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137059/new/ https://reviews.llvm.org/D137059 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listin

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

ChuanqiXu added a comment. Thanks for every one to review this! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137609/new/ https://reviews.llvm.org/D137609 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG4a7be42d922a: [C++20] [Modules] Remove unmaintained Header Module (authored by ChuanqiXu). Herald added a subscriber: MaskRay. Changed prior to commit: https://reviews.llvm.org/D137609?vs=473884&id=4763

[PATCH] D117087: [C++20] [Coroutines] Implement return value optimization for get_return_object

ChuanqiXu added a comment. In D117087#4082454 , @alkis wrote: > Does this change the semantics of `get_return_value()` such that it is > eagerly converted to R the return value of the coro? AFAIU before this change > one could return T convertible to R

[PATCH] D142925: [Clang] Improve error message for violations of -fmodules-decluse.

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/D142925/new/ https://reviews.llvm.org/D142925

[PATCH] D142077: [Clang][SemaCXX][Coroutines] Fix misleading diagnostics with -Wunsequenced

ChuanqiXu accepted this revision. ChuanqiXu added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Sema/SemaChecking.cpp:15186 + if (ChildExpr == CSE->getOperand()) +// Do not recurse over a CoroutineSuspendExpr's operand. +

[PATCH] D142704: [C++20][Modules] Handle template declarations in header units.

ChuanqiXu added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:15265 FD->getFormalLinkage() == Linkage::ExternalLinkage && - !FD->isInvalidDecl() && BodyKind != FnBodyKind::Delete && + !FD->isInvalidDecl() && !IsFnTemplate && BodyKind != FnBodyKind::Del

[PATCH] D142704: [C++20][Modules] Handle template declarations in header units.

ChuanqiXu added a comment. In D142704#4092724 , @iains wrote: > I think we need to find a way to proceed - because this causes a regression > on the llvm-16 branch, and that should be resolved soon, if possible. > What is your suggestion for a way forwar

[PATCH] D142704: [C++20][Modules] Handle template declarations in header units.

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/D142704/new/ https://reviews.llvm.org/D142704

[PATCH] D143053: [C++20] [Modules] Pop Expression Evaluation Context when we skip its body during parsing

ChuanqiXu created this revision. ChuanqiXu added reviewers: erichkeane, royjacobson, shafik, clang-language-wg. ChuanqiXu added a project: clang. Herald added a project: All. ChuanqiXu requested review of this revision. Herald added a subscriber: cfe-commits. Close https://github.com/llvm/llvm-pro

[PATCH] D143053: [C++20] [Modules] Pop Expression Evaluation Context when we skip its body during parsing

ChuanqiXu added a reviewer: cor3ntin. ChuanqiXu added inline comments. Comment at: clang/lib/Parse/Parser.cpp:1415 +// +// FIXME: It looks not easy to balance PushExpressionEvaluationContext() +// and PopExpressionEvaluationContext(). cor3ntin wrote:

[PATCH] D142384: [C++20] Fix a crash with modules.

ChuanqiXu added a comment. It should be good to prevent crashes. But it looks not good that it doesn't have a test. Do you have plans to add a test case for this soon? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142384/new/ https://reviews.llvm.

[PATCH] D143053: [C++20] [Modules] Pop Expression Evaluation Context when we skip its body during parsing

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG10d85dc75468: [C++20] [Modules] Pop Expression Evaluation Context when we skip its body… (authored by ChuanqiXu). Repository: rG LLVM Github Monor

[PATCH] D141992: [NFC] [Serialization] Add static assert for the size of the decls to mention developers to remember to touch the serializer after them modified the field of decls

ChuanqiXu updated this revision to Diff 494511. ChuanqiXu added a comment. This is only part of the original revision, which contains platform independent assertion only. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141992/new/ https://reviews.llvm.org/D141992 Files: clang/include/

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

ChuanqiXu added a comment. In D140867#4043763 , @Jake-Egan wrote: > In D140867#4042405 , @ChuanqiXu > wrote: > >> In D140867#4042351 , @Jake-Egan >> wrote: >> Would

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

ChuanqiXu updated this revision to Diff 488455. ChuanqiXu added a comment. Address comments: - Extract the logic to compute the output path of `-fmodule-output` to a reusable function. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137058/new/ https://reviews.llvm.org/D137058 Files:

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

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] D141572: [C++] [Coroutines] Deprecates the '-fcoroutines-ts' flag

ChuanqiXu created this revision. ChuanqiXu added reviewers: aaron.ballman, ldionne. ChuanqiXu added a project: clang-language-wg. Herald added a project: All. ChuanqiXu requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. According to th

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

ChuanqiXu updated this revision to Diff 488836. ChuanqiXu marked an inline comment as done. ChuanqiXu added a comment. Address comments: fix a typo. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137058/new/ https://reviews.llvm.org/D137058 Files: clang/include/clang/Basic/DiagnosticD

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

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; h-ve

[PATCH] D141572: [C++] [Coroutines] Deprecates the '-fcoroutines-ts' flag

ChuanqiXu updated this revision to Diff 488873. ChuanqiXu added a comment. Herald added a subscriber: arichardson. After failed to reproduce the error locally, try to follow the instructions from @philnik to see if the CI-Bot will break. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141

[PATCH] D141572: [C++] [Coroutines] Deprecates the '-fcoroutines-ts' flag

ChuanqiXu added a comment. @philnik when I run `ninja libcxx-generate-files` locally, it says `ninja: error: unknown target 'libcxx-generate-files'`. The following off is my configuring command: cmake -G Ninja -S llvm -B build_libcxx_modules -DLLVM_ENABLE_PROJECTS="clang" -DLLVM_ENABLE_RUNT

[PATCH] D141572: [C++] [Coroutines] Deprecates the '-fcoroutines-ts' flag

ChuanqiXu updated this revision to Diff 489412. ChuanqiXu added a comment. Update the header tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141572/new/ https://reviews.llvm.org/D141572 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/DiagnosticDriverKinds.td cla

[PATCH] D141572: [C++] [Coroutines] Deprecates the '-fcoroutines-ts' flag

ChuanqiXu added a comment. In D141572#4050758 , @philnik wrote: > In D141572#4050492 , @ChuanqiXu > wrote: > >> @philnik when I run `ninja libcxx-generate-files` locally, it says `ninja: >> error: unknown target

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

ChuanqiXu added a comment. In D137058#4050188 , @dblaikie wrote: > I really don't think this is the right thing to do - the Split DWARF code, > for instance, has support for GPU bundling that's missing in the module file > naming code, which seems likel

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

ChuanqiXu marked an inline comment as done. ChuanqiXu added a comment. Oh, I forgot to submit the inline comments. My bad. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137534/new/ https://reviews.llvm.org/D137534 ___ cfe-commits mailing list

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

ChuanqiXu added a subscriber: h-vetinari. ChuanqiXu added a comment. @jansvoboda11 gentle ping~ (although I feel it may be hard to land this before the branching since I'll take a vacation in 19th until February, @h-vetinari said it should be OK to backport these patches to branched 16.x.) CHA

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

ChuanqiXu marked an inline comment as done. ChuanqiXu added a comment. In D137534#4055286 , @h-vetinari wrote: > In D137534#4055283 , @ChuanqiXu > wrote: > >> @h-vetinari said it should be OK to backport these p

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf89327e28bc1: [Driver] [Modules] Support -fmodule-output (1/2) (authored by ChuanqiXu). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

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

ChuanqiXu added a comment. In D137534#4055323 , @h-vetinari wrote: >> Sorry for the misleading and thanks for the quick clarification. So it looks >> like the status quo is a little bit worse than I imaged... > > I opened a thread on discourse for more

[PATCH] D141572: [C++] [Coroutines] Deprecates the '-fcoroutines-ts' flag

ChuanqiXu added a reviewer: philnik. ChuanqiXu added a comment. Now it looks like the failure are not related to the patch. Although there are some clang-format related failures, the related changes are generated automatically and I roughly remember libcxx don't use libcxx. @philnik would you l

[PATCH] D137059: [Driver] [C++20] [Modules] Support -fmodule-output= (2/2)

ChuanqiXu updated this revision to Diff 489426. ChuanqiXu added a comment. Update since the dependent one changes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137059/new/ https://reviews.llvm.org/D137059 Files: clang/include/clang/Driver/Options.td clang/lib/Driver/Driver.cpp c

[PATCH] D137059: [Driver] [C++20] [Modules] Support -fmodule-output= (2/2)

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3e9e8d6ef442: [Driver] [C++20] [Modules] Support -fmodule-output= (2/2) (authored by ChuanqiXu). Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D140001: [Driver] [C++20] [Modules] Don't emit unused-argument warning for ` -fmodule-output=` and `-fmodule-output`

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGfcb6afb508c4: [Driver] [C++20] [Modules] Don't emit unused-argument warning for '-fmodule… (authored by ChuanqiXu). Herald added a project: clang. He

[PATCH] D141572: [C++] [Coroutines] Deprecates the '-fcoroutines-ts' flag

ChuanqiXu updated this revision to Diff 489437. ChuanqiXu added a comment. Move the release note to `Deprecated Compiler Flags` section. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141572/new/ https://reviews.llvm.org/D141572 Files: clang/docs/ReleaseNotes.rst clang/include/clang

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

ChuanqiXu added a comment. In D137058#4056647 , @Jake-Egan wrote: > Hi, this new test fails on AIX > https://lab.llvm.org/buildbot/#/builders/214/builds/5351/steps/6/logs/FAIL__Clang__module-output_cppm > Could you take a look? I added `// REQUIRES: x86

[PATCH] D131938: [C++20] [Coroutines] Disable to take the address of labels in coroutines

ChuanqiXu accepted this revision. ChuanqiXu added a comment. This revision is now accepted and ready to land. Oh, sorry, I forgot this. Given D132084 is not updated recently, I think we can merge this first. And it should be fine to remove this one after patche

[PATCH] D141572: [C++] [Coroutines] Deprecates the '-fcoroutines-ts' flag

ChuanqiXu updated this revision to Diff 489665. ChuanqiXu added a comment. Address comments and refine the wording from "use `-std=c++20`" to "use `-std=c++20` or higher". CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141572/new/ https://reviews.llvm.org/D141572 Files: clang/docs/Re

[PATCH] D131938: [C++20] [Coroutines] Disable to take the address of labels in coroutines

This revision was automatically updated to reflect the committed changes. Closed by commit rGcc526e346deb: [C++20] [Coroutines] Disable to take the address of labels in coroutines (authored by ChuanqiXu). Changed prior to commit: https://reviews.llvm.org/D131938?vs=452925&id=489677#toc Reposit

[PATCH] D71734: [ODRHash] Hash `RecordDecl` and diagnose discovered mismatches.

ChuanqiXu added inline comments. Comment at: clang/lib/AST/Decl.cpp:4881-4882 + // For RecordDecl the ODRHash is stored in the remaining 26 + // bit of RecordDeclBits, adjust the hash to accomodate. + setODRHash(Hash.CalculateHash() >> 6); + return RecordDeclBits.ODRHash; ---

[PATCH] D141803: [Clang] Reject in-class defaulting of previously declared comparison operators

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/D141803/new/ https://reviews.llvm.org/D141803

[PATCH] D141905: [C++20] [Modules] Only diagnose the non-inline external variable definitions in header units

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb3eb004ca78f: [C++20] [Modules] Only diagnose the non-inline external variable (authored by ChuanqiXu). Herald added a project: clang. Herald added a

[PATCH] D141908: [C++20][Modules] Handle defaulted and deleted functions in header units.

ChuanqiXu added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:15258 + // units. Deleted and Defaulted functions are implicitly inline (but the + // inline state is not set at this point, so check the BodyKind explicitly). if (getLangOpts().CPlusPlusModules && cur

[PATCH] D141572: [C++] [Coroutines] Deprecates the '-fcoroutines-ts' flag

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf90074274673: [C++] [Coroutines] Deprecates the '-fcoroutines-ts' flag (authored by ChuanqiXu). Herald added a project: libc++. Herald added a subscr

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

ChuanqiXu added a comment. In D137058#4058836 , @Jake-Egan wrote: > In D137058#4057424 , @ChuanqiXu > wrote: > >> In D137058#4056647 , @Jake-Egan >> wrote: >> >>> Hi, th

[PATCH] D71734: [ODRHash] Hash `RecordDecl` and diagnose discovered mismatches.

ChuanqiXu accepted this revision. ChuanqiXu added a comment. This revision is now accepted and ready to land. LGTM generally. It'd better to mention this in the `Potentially Breaking Changes` section of ReleaseNotes. Comment at: clang/lib/AST/Decl.cpp:4714 setIsRandomized(f

[PATCH] D141908: [C++20][Modules] Handle defaulted and deleted functions in header units.

ChuanqiXu accepted this revision. ChuanqiXu added a comment. This revision is now accepted and ready to land. LGTM basically. I still feel we need a FIXME there. But I don't want to block this for this reason especially we need to land this before the branch. Comment at: clang

[PATCH] D141908: [C++20][Modules] Handle defaulted and deleted functions in header units.

ChuanqiXu added a comment. > Well.. we have time for another iteration, I am going to take a vacation for the Chinese New Year since tomorrow to February. So I am a little bit hurried : ) Comment at: clang/lib/Sema/SemaDecl.cpp:15258 + // units. Deleted and Defaulted functi

[PATCH] D141992: [NFC] [Serialization] Add static assert for the size of the decls to mention developers to remember to touch the serializer after them modified the field of decls

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc79635cce845: [NFC] [Serialization] Add static assert for the size of the decls to (authored by ChuanqiXu). Herald added a project: clang. Herald add

[PATCH] D141992: [NFC] [Serialization] Add static assert for the size of the decls to mention developers to remember to touch the serializer after them modified the field of decls

ChuanqiXu added inline comments. Comment at: clang/lib/Serialization/ASTWriterDecl.cpp:2313 void ASTDeclWriter::VisitOMPDeclareMapperDecl(OMPDeclareMapperDecl *D) { + static_assert(sizeof(OMPDeclareMapperDecl) == 120, +"You need to update the serializer after yo

[PATCH] D141992: [NFC] [Serialization] Add static assert for the size of the decls to mention developers to remember to touch the serializer after them modified the field of decls

ChuanqiXu added a comment. It passes on the previous windows CI. It looks like the assertion for `sizeof(...)` is indeed not a good idea. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141992/new/ https://reviews.llvm.org/D141992 _

[PATCH] D127471: [Coroutines] Convert coroutine.presplit to enum attr

ChuanqiXu added a comment. @rjmccall Does this look good to you? I am afraid to break swift's codes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127471/new/ https://reviews.llvm.org/D127471 ___ cfe-com

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

ChuanqiXu added a comment. In D126694#3576602 , @iains wrote: > @ChuanqiXu - I changed the module ownership name to "ModuleDiscardable" - > because, while we are permitted to discard these, we might choose not to (to > give your better diagnostics) - bu

[PATCH] D113545: [C++20] [Module] Support reachable definition initially/partially

ChuanqiXu added a comment. @iains Given @rsmith is too busy, would you like to review this one? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113545/new/ https://reviews.llvm.org/D113545 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D127624: [C++20][Modules] Allow for redeclarations in partitions.

ChuanqiXu accepted this revision. ChuanqiXu added a comment. This revision is now accepted and ready to land. LGTM. Yeah, it might be painful to compare string all the time. Let's do it when refactoring. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D127471: [Coroutines] Convert coroutine.presplit to enum attr

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG735e6c40b5e9: [Coroutines] Convert coroutine.presplit to enum attr (authored by ChuanqiXu). Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

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

ChuanqiXu added a comment. In D126694#3577815 , @iains wrote: > In D126694#3576853 , @ChuanqiXu > wrote: > >> In D126694#3576602 , @iains wrote: >> >>> @ChuanqiXu - I cha

[PATCH] D127471: [Coroutines] Convert coroutine.presplit to enum attr

ChuanqiXu added a comment. In D127471#3580581 , @akuegel wrote: > It seems this change triggers a warning: > > https://buildkite.com/llvm-project/upstream-bazel/builds/31190#018160e2-0b96-4254-8986-f039b7f2e0a1 > > llvm-project/llvm/lib/Transforms/Utils/C

[PATCH] D125291: Introduce @llvm.threadlocal.address intrinsic to access TLS variable

ChuanqiXu added a comment. In D125291#3581064 , @nikic wrote: > In D125291#3548671 , @jyknight > wrote: > >> Anyhow -- I think the prototype I'm fiddling with is also along the path to >> the ideal long-term sta

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

ChuanqiXu added a comment. I finally found some time to look at the crash. Although I haven't get an idea, I found it crash at the following one too: template concept Constraint = true; template class completion_handler_async_result { public: template static void init

[PATCH] D125936: [Sema] Relax an assertion in BuildStmtExpr

ChuanqiXu added a comment. In D125936#3587542 , @ahatanak wrote: > Adding more people to get more eyes on the changes I made to > SemaCoroutine.cpp. The change to SemaCoroutine.cpp might be good. But I feel like it is irreverent to the revision. If you

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

ChuanqiXu added a comment. From what I can see, the crash reason would be the mismatch depth and the setting of MultiLevelTemplateArgumentList. In `::CheckConstraintSatisfaction`, the depth of `RawCompletionToken ` is 0, while the depth of corresponding MultiLevelTemplateArgumentList is 2. So t

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

ChuanqiXu added a comment. In D126907#3588708 , @erichkeane wrote: > In D126907#3588417 , @ChuanqiXu > wrote: > >> From what I can see, the crash reason would be the mismatch depth and the >> setting of MultiLe

[PATCH] D113545: [C++20] [Module] Support reachable definition initially/partially

ChuanqiXu added a comment. @rsmith @iains gentle ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113545/new/ https://reviews.llvm.org/D113545 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/li

[PATCH] D118034: [C++20] [Modules] Don't complain about duplicated default template argument across modules

ChuanqiXu added a comment. @rsmith @iains ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118034/new/ https://reviews.llvm.org/D118034 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

ChuanqiXu added a comment. Great progress! In D126907#3599835 , @erichkeane wrote: > Note that the failure comes down to: > > template concept C = T::f(); > template class P> struct S1{}; > template struct X{}; > S1 s11; > > and requires the -fr

[PATCH] D125291: Introduce @llvm.threadlocal.address intrinsic to access TLS variable

ChuanqiXu added a comment. @jyknight How do you think about the status now? I want to fix the thread local problem for coroutines in clang15 since the problem have been found for years... CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125291/new/ https://reviews.llvm.org/D125291

[PATCH] D113545: [C++20] [Module] Support reachable definition initially/partially

ChuanqiXu updated this revision to Diff 438929. ChuanqiXu added a comment. Add test from https://eel.is/c++draft/module.context#example-1 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113545/new/ https://reviews.llvm.org/D113545 Files: clang/include/clang/AST/DeclBase.h clang/inclu

[PATCH] D113545: [C++20] [Module] Support reachable definition initially/partially

ChuanqiXu added a comment. In D113545#3598412 , @iains wrote: > As for review, I can try to pick up the "nits" but not sure that I know the > instantiation sub=system too well, so it would be better if @rsmith could > cast an eye over those parts. Yeah

[PATCH] D113545: [C++20] [Module] Support reachable definition initially/partially

ChuanqiXu updated this revision to Diff 438930. ChuanqiXu added a comment. Update comments for the test of [module.context]p7. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113545/new/ https://reviews.llvm.org/D113545 Files: clang/include/clang/AST/DeclBase.h clang/include/clang/Ba

[PATCH] D113545: [C++20] [Module] Support reachable definition initially/partially

ChuanqiXu added a comment. In D113545#3601031 , @iains wrote: > I think it is helpful to collect the standard examples into one place (i.e. > test/Modules) and name them for the standard version (i.e. cxx20-N-M-exO.cpp) > .. because > > - the details of

[PATCH] D113545: [C++20] [Module] Support reachable definition initially/partially

ChuanqiXu added a comment. In D113545#3601069 , @iains wrote: > In D113545#3601050 , @ChuanqiXu > wrote: > >> In D113545#3601031 , @iains wrote: >> >>> I think it is help

[PATCH] D128328: [C++20][Modules] Improve handing of Private Module Fragment diagnostics.

ChuanqiXu added a comment. It looks like we need to handle inline variable as well to match the intention. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11155 +def err_export_inline_not_defined : Error< + "exported inline functions must be defined within the mod

[PATCH] D128328: [C++20][Modules] Improve handing of Private Module Fragment diagnostics.

ChuanqiXu added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11155 +def err_export_inline_not_defined : Error< + "exported inline functions must be defined within the module purview" + " and before any private module fragment">;

[PATCH] D128328: [C++20][Modules] Improve handing of Private Module Fragment diagnostics.

ChuanqiXu added a comment. In D128328#3603781 , @vsapsai wrote: > From the perspective of handling `err_export_inline_not_defined` error as a > developer what about the following option? > > export inline void fn_e(); // note: function 'fn_e' exported

[PATCH] D128328: [C++20][Modules] Improve handing of Private Module Fragment diagnostics.

ChuanqiXu added a comment. In D128328#3603940 , @iains wrote: > In D128328#3602646 , @iains wrote: > >> In D128328#3601080 , @ChuanqiXu >> wrote: >> >>> It looks like we

[PATCH] D128328: [C++20][Modules] Improve handing of Private Module Fragment diagnostics.

ChuanqiXu added a comment. In D128328#3603945 , @iains wrote: > In D128328#3603942 , @ChuanqiXu > wrote: > >> In D128328#3603940 , @iains wrote: >> >>> In D128328#3602646

[PATCH] D128328: [C++20][Modules] Improve handing of Private Module Fragment diagnostics.

ChuanqiXu added a comment. In D128328#3603967 , @iains wrote: > In D128328#3603953 , @ChuanqiXu > wrote: > >> In D128328#3603945 , @iains wrote: >> >>> In D128328#3603942

[PATCH] D128328: [C++20][Modules] Improve handing of Private Module Fragment diagnostics.

ChuanqiXu added a comment. In D128328#3604110 , @iains wrote: > In D128328#3603980 , @ChuanqiXu > wrote: > >> > > > Also, **if** [module.private.frag]p2.1 is changed into: > the point by which the

[PATCH] D126187: [C++20] [Coroutines] Conform the updates for CWG issue 2585

ChuanqiXu updated this revision to Diff 431861. ChuanqiXu added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126187/new/ https://reviews.llvm.org/D126187 Files: clang/docs/ReleaseNotes.rst clang/lib/Sema/SemaCoroutine.cpp clang/test/CodeGenCoroutines

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