[PATCH] D119409: [C++20] [Modules] Remain variable's definition in module interface

ChuanqiXu added inline comments. Comment at: clang/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp:5 // CHECK-DAG: @extern_var_exported = external {{(dso_local )?}}global -// CHECK-DAG: @inline_var_exported = linkonce_odr {{(dso_local )?}}global +// CHECK-DAG: @inline_var_

[PATCH] D119409: [C++20] [Modules] Remain variable's definition in module interface

ChuanqiXu added inline comments. Comment at: clang/test/CodeGenCXX/static-variable-in-module.cpp:2-8 +// RUN: mkdir %t +// RUN: echo "struct S { S(); };" >> %t/foo.h +// RUN: echo "static S s = S();" >> %t/foo.h +// RUN: %clang -std=c++20 -I%t %s -S -emit-llvm -o - | FileCheck %s

[PATCH] D119778: [clang] Add a note "deducing return type for 'foo'"

ChuanqiXu added a comment. I am curious about the behavior if we removed the guard `if (OrigResultType.getBeginLoc().isValid())`. Comment at: clang/lib/Sema/SemaStmt.cpp:3804-3807 +if (DAR != DAR_Succeeded) { + if (OrigResultType.getBeginLoc().isValid()) +Diag

[PATCH] D119409: [C++20] [Modules] Remain variable's definition in module interface

ChuanqiXu updated this revision to Diff 408694. ChuanqiXu added a comment. Update tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119409/new/ https://reviews.llvm.org/D119409 Files: clang/lib/Serialization/ASTWriterDecl.cpp clang/test/CXX/modules-ts/basic/basic.def.odr/p4/modu

[PATCH] D119409: [C++20] [Modules] Remain variable's definition in module interface

ChuanqiXu added inline comments. Comment at: clang/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp:5 // CHECK-DAG: @extern_var_exported = external {{(dso_local )?}}global -// CHECK-DAG: @inline_var_exported = linkonce_odr {{(dso_local )?}}global +// CHECK-DAG: @inline_var_

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

ChuanqiXu updated this revision to Diff 408761. ChuanqiXu added a comment. Update test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117087/new/ https://reviews.llvm.org/D117087 Files: clang/include/clang/AST/StmtCXX.h clang/lib/AST/StmtCXX.cpp clang/lib/CodeGen/CGCoroutine.cpp

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

ChuanqiXu added inline comments. Comment at: clang/lib/CodeGen/CGCoroutine.cpp:650 - if (Stmt *Ret = S.getReturnStmt()) + if (Stmt *Ret = S.getReturnStmt()) { +// Since we already emitted the return value above, so we shouldn't junparser wrote: > can we j

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

ChuanqiXu added inline comments. Comment at: clang/lib/CodeGen/CGCoroutine.cpp:654 +cast(Ret)->setRetValue(nullptr); EmitStmt(Ret); + } junparser wrote: > I mean, remove the if statements here since the retuen expr is null. We couldn't. Since we need em

[PATCH] D119772: [clang] [NFC] More exhaustive tests for deducing void return types

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

[PATCH] D119823: [Modules] Add module structure output to -module-file-info.

ChuanqiXu added inline comments. Comment at: clang/test/Modules/module-file-info-cxx20.cpp:26 + +#if TU == 1 + According to [[ http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p1857r3.html | P1857R3 ]], it might not be good to add macro declarative befor

[PATCH] D118893: [C++20][Modules] Track valid import state.

ChuanqiXu added inline comments. Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1543 +def err_import_not_allowed_here : Error< + "imports must be contiguous and immediately follow the module declaration">; +def err_import_in_global_fragment : Error< --

[PATCH] D118893: [C++20][Modules] Track valid import state.

ChuanqiXu added a comment. BTW, I guess it would be helpful to add the index in the series in the title. Like `[C++20][Modules] Track valid import state (1/8)`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118893/new/ https://reviews.llvm.org/D11

[PATCH] D112094: Add support for floating-point option `ffp-eval-method` and for `pragma clang fp eval_method`.

ChuanqiXu added a comment. Herald added a subscriber: pcwang-thead. It looks this landed without being approved: https://github.com/llvm/llvm-project/commit/4bafe65c2b2f1ce745894a509a6d80c87fb1c335. It broke the test. If we need to revert it, we need to revert https://github.com/llvm/llvm-proje

[PATCH] D118586: [C++20][Modules] Initial handling for module partitions.

ChuanqiXu added a comment. Oh, I did similar thing in our downstream version. I planned to show it in next few weeks. Now it looks I could only give up on it... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118586/new/ https://reviews.llvm.org/D11

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

This revision was automatically updated to reflect the committed changes. Closed by commit rGd30ca5e2e23f: [C++20] [Coroutines] Implement return value optimization for get_return_object (authored by ChuanqiXu). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D119409: [C++20] [Modules] Remain variable's definition in module interface

ChuanqiXu added inline comments. Comment at: clang/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp:5 // CHECK-DAG: @extern_var_exported = external {{(dso_local )?}}global -// CHECK-DAG: @inline_var_exported = linkonce_odr {{(dso_local )?}}global +// CHECK-DAG: @inline_var_

[PATCH] D119409: [C++20] [Modules] Remain variable's definition in module interface

ChuanqiXu added inline comments. Comment at: clang/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp:5 // CHECK-DAG: @extern_var_exported = external {{(dso_local )?}}global -// CHECK-DAG: @inline_var_exported = linkonce_odr {{(dso_local )?}}global +// CHECK-DAG: @inline_var_

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

ChuanqiXu updated this revision to Diff 409157. ChuanqiXu added a comment. Address comment to check if the duplicated default template arguments are the same and add tests to address the negative cases. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118034/new/ https://reviews.llvm.org/

[PATCH] D119409: [C++20] [Modules] Remain variable's definition in module interface

ChuanqiXu added inline comments. Comment at: clang/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp:5 // CHECK-DAG: @extern_var_exported = external {{(dso_local )?}}global -// CHECK-DAG: @inline_var_exported = linkonce_odr {{(dso_local )?}}global +// CHECK-DAG: @inline_var_

[PATCH] D119823: [Modules] Add module structure output to -module-file-info.

ChuanqiXu added inline comments. Comment at: clang/test/Modules/module-file-info-cxx20.cpp:26 + +#if TU == 1 + iains wrote: > ChuanqiXu wrote: > > According to [[ > > http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p1857r3.html | > > P1857R3 ]], it migh

[PATCH] D119823: [Modules] Add module structure output to -module-file-info.

ChuanqiXu accepted this revision. ChuanqiXu added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clang/test/Modules/module-file-info-cxx20.cpp:26 + +#if TU == 1 + urnathan wrote: > iains wrote: > > urnathan wrote: > > > urnathan w

[PATCH] D118893: [C++20][Modules][1/8] Track valid import state.

ChuanqiXu added inline comments. Comment at: clang/include/clang/Sema/Sema.h:2955-2960 +FirstDecl, +GlobalFragment, +ImportAllowed, +ImportFinished, +PrivateFragment, +NotACXX20Module I guess it is worth to add comment for these fields. Al

[PATCH] D119409: [C++20] [Modules] Remain variable's definition in module interface

ChuanqiXu added inline comments. Comment at: clang/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp:5 // CHECK-DAG: @extern_var_exported = external {{(dso_local )?}}global -// CHECK-DAG: @inline_var_exported = linkonce_odr {{(dso_local )?}}global +// CHECK-DAG: @inline_var_

[PATCH] D119409: [C++20] [Modules] Remain variable's definition in module interface

ChuanqiXu updated this revision to Diff 409503. ChuanqiXu added a comment. Handle static variable only. Don't touch inline variable now. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119409/new/ https://reviews.llvm.org/D119409 Files: clang/lib/Serialization/ASTWriterDecl.cpp clang

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

ChuanqiXu updated this revision to Diff 409507. ChuanqiXu added a comment. Use ODRHash to simplify code. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118034/new/ https://reviews.llvm.org/D118034 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Sema/SemaTemplate.cp

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

ChuanqiXu added inline comments. Comment at: clang/lib/Sema/SemaTemplate.cpp:2658-2682 +Expr::EvalResult EVRX, EVRY; +if (!DefaultArgumentX->EvaluateAsConstantExpr(EVRX, C) || +!DefaultArgumentY->EvaluateAsConstantExpr(EVRY, C)) + return false; + +APValue

[PATCH] D114714: [C++20][Modules][2/8] Add enumerations for partition modules and stream them.

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

[PATCH] D118586: [C++20][Modules][3/8] Initial handling for module partitions.

ChuanqiXu added a comment. I think we should add 2 more tests at least: Import a partition a module purview but we failed to find one. And test reflects the successful case. Comment at: clang/include/clang/Sema/Sema.h:2989 /// \param ImportLoc The location of the 'import' k

[PATCH] D118893: [C++20][Modules][1/8] Track valid import state.

ChuanqiXu accepted this revision. ChuanqiXu added a comment. LGTM. Comment at: clang/lib/Sema/SemaModule.cpp:144 GlobalModuleFragment = ModuleScopes.back().Module; // In C++20, the module-declaration must be the first declaration if there iains wrote:

[PATCH] D118586: [C++20][Modules][3/8] Initial handling for module partitions.

ChuanqiXu added inline comments. Comment at: clang/lib/Sema/SemaModule.cpp:177 + if (IsPartition) { +ModuleName += ":"; +ModuleName += stringFromPath(Partition); I chose '-' in my implementation since here ModuleName refers to the name in filesystem, ri

[PATCH] D118586: [C++20][Modules][3/8] Initial handling for module partitions.

ChuanqiXu added inline comments. Comment at: clang/test/Modules/cxx20-partition-diagnostics-a.cpp:3-5 +// RUN: %clang_cc1 -std=c++20 -S -D TU=1 -x c++ %s -o /dev/null -verify + +// RUN: %clang_cc1 -std=c++20 -S -D TU=2 -x c++ %s -o /dev/null -verify We could use

[PATCH] D118587: [C++20][Modules][4/8] Handle generation of partition implementation CMIs.

ChuanqiXu added inline comments. Comment at: clang/test/Modules/cxx20-10-1-ex1.cpp:7-8 + +// RUN: %clang_cc1 -std=c++20 -emit-module-interface -D TU=1 -x c++ %s \ +// RUN: -o %t/A_Internals.pcm + In my implementation, I replace ':' to '-' in pcm filename. I do s

[PATCH] D118588: [C++20][Modules]5/8] Diagnose wrong import/export for partition CMIs.

ChuanqiXu accepted this revision. ChuanqiXu added a comment. LGTM with the name about partition CMI addressed (It is raised in previous patch) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118588/new/ https://reviews.llvm.org/D118588

[PATCH] D118586: [C++20][Modules][3/8] Initial handling for module partitions.

ChuanqiXu added inline comments. Comment at: clang/lib/Sema/SemaModule.cpp:367-368 + // owning named module. + size_t P = NamedMod->Name.find_first_of(":"); + ModuleName = NamedMod->Name.substr(0, P + 1); +} else { ChuanqiXu wrote: > I would li

[PATCH] D118589: [C++20][Modules][6/8] Record direct module imports.

ChuanqiXu added a comment. hmmm, I don't find this is used in following patch. Oversight? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118589/new/ https://reviews.llvm.org/D118589 ___ cfe-commits mailin

[PATCH] D118598: [C++20][Modules][7/8] Find the primary interface name for a module.

ChuanqiXu added a comment. My implementation is StringRef getPrimaryModuleName() const { if (Check Kind) return Name; return StringRef(Name).split('-').first; } Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118598/new/ ht

[PATCH] D118599: [C++20][Modules][8/8] Amend module visibility rules for partitions.

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

[PATCH] D118589: [C++20][Modules][6/8] Record direct module imports.

ChuanqiXu accepted this revision. ChuanqiXu added a comment. This revision is now accepted and ready to land. Sorry, I took an oversight. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118589/new/ https://reviews.llvm.org/D118589 __

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

ChuanqiXu added inline comments. Comment at: clang/lib/Sema/SemaTemplate.cpp:2658-2682 +Expr::EvalResult EVRX, EVRY; +if (!DefaultArgumentX->EvaluateAsConstantExpr(EVRX, C) || +!DefaultArgumentY->EvaluateAsConstantExpr(EVRY, C)) + return false; + +APValue

[PATCH] D119544: Deferred Concept Instantiation Implementation

ChuanqiXu added inline comments. Comment at: clang/test/SemaTemplate/concepts.cpp:391 + + CausesFriendConstraint CFC; + FriendFunc(CFC, 1); erichkeane wrote: > erichkeane wrote: > > A bunch of the tests below this all fail. > See these two tests, which fail by

[PATCH] D119544: Deferred Concept Instantiation Implementation

ChuanqiXu added a comment. In D119544#3472537 , @erichkeane wrote: > Alright, a touch more: It looks like the looping through the `CurContext` at > line 6084 finds the wrong thing in evaluating this `CXXMethodDecl`! The > "broken" version has its Cur

[PATCH] D124149: [NFC] follow up code cleanup after D123837

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

[PATCH] D124504: Remove --no-opaque-pointers in test/cxx2a-thread-local-constinit.cpp

ChuanqiXu created this revision. ChuanqiXu added a reviewer: nikic. ChuanqiXu added a project: clang. Herald added a project: All. ChuanqiXu requested review of this revision. Herald added a subscriber: cfe-commits. The option `--no-opaque-pointers` in test/cxx2a-thread-local-constinit.cpp blocks

[PATCH] D124504: Remove --no-opaque-pointers in test/cxx2a-thread-local-constinit.cpp

ChuanqiXu added a comment. In D124504#3476537 , @nikic wrote: > How does it block your work? Tests currently still use `-no-opaque-pointers` > to avoid breaking this mode. I think it's generally okay to drop the option, > but I'm a bit unclear about the

[PATCH] D124504: Remove --no-opaque-pointers in test/cxx2a-thread-local-constinit.cpp

ChuanqiXu abandoned this revision. ChuanqiXu added a comment. In D124504#3476585 , @nikic wrote: > In D124504#3476562 , @ChuanqiXu > wrote: > >> In D124504#3476537 , @nik

[PATCH] D119544: Deferred Concept Instantiation Implementation

ChuanqiXu added a comment. Oh, I'm busy with coroutines these days. Thanks for reminding me for this. LGTM to me basically. Only comments for style left. Comment at: clang/lib/Sema/SemaConcept.cpp:70 +assert((!LHS.isInvalid() && !RHS.isInvalid()) && "not good expressions?"

[PATCH] D124149: [NFC] follow up code cleanup after D123837

ChuanqiXu added a comment. @iains ping! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124149/new/ https://reviews.llvm.org/D124149 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.ll

[PATCH] D128690: [ODRHash diagnostics] Preparation to minimize subsequent diffs. NFC.

ChuanqiXu accepted this revision. ChuanqiXu added a comment. This revision is now accepted and ready to land. LGTM basically. Comment at: clang/lib/Serialization/ASTReader.cpp:10621-10626 // Compute the hash of the method as if it has no body. -auto ComputeCXXM

[PATCH] D129174: [C++20][Modules] Update ADL to handle basic.lookup.argdep p4 [P1815R2 part 1]

ChuanqiXu added inline comments. Comment at: clang/lib/Sema/SemaLookup.cpp:3859 +// C++20 [basic.lookup.argdep] p4.3 .. are exported +Module *FM = D->getOwningModule(); +// .. are attached to a named module M, do not appear in the -

[PATCH] D129068: [AST] Accept identical TypeConstraint referring to other template parameters.

ChuanqiXu updated this revision to Diff 443548. ChuanqiXu added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129068/new/ https://reviews.llvm.org/D129068 Files: clang/include/clang/AST/ASTContext.h clang/lib/AST/ASTContext.cpp clang/test/Modules/conc

[PATCH] D129068: [AST] Accept identical TypeConstraint referring to other template parameters.

ChuanqiXu added a comment. In D129068#3640249 , @vsapsai wrote: > Thanks for the changes, they look good! While I was looking how we handle > "requires" constraints in other cases, I've noticed that they are > suspiciously similar. That's why I offer th

[PATCH] D129138: [clang] [docs] Update the changes of C++20 Modules in clang15

ChuanqiXu updated this revision to Diff 443549. ChuanqiXu marked an inline comment as done. ChuanqiXu added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129138/new/ https://reviews.llvm.org/D129138 Files: clang/docs/ReleaseNotes.rst clang/www/cxx_statu

[PATCH] D129138: [clang] [docs] Update the changes of C++20 Modules in clang15

ChuanqiXu added inline comments. Comment at: clang/www/cxx_status.html:1183 https://wg21.link/p1874r1";>P1874R1 -Partial +Clang 15 avogelsgesang wrote: > should this be `class="unreleased"` instead of `class="full"`? At least this

[PATCH] D129530: [coroutine] add nomerge function attribute to `llvm.coro.save`

ChuanqiXu added a comment. This looks good to me basically. I would love to see an example to test the two coro.save wouldn't be merged. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129530/new/ https://reviews.llvm.org/D129530 __

[PATCH] D129530: [coroutine] add nomerge function attribute to `llvm.coro.save`

ChuanqiXu accepted this revision. ChuanqiXu added a comment. This revision is now accepted and ready to land. In D129530#3644091 , @ychen wrote: > In D129530#3644074 , @ChuanqiXu > wrote: > >> This looks good to

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

ChuanqiXu added a comment. In D126907#3642530 , @erichkeane wrote: > This version passes check-runtimes, so libc++ is fine, and it passes > everything I have available. @ChuanqiXu : Would you be able to do 1 more run > over this to make sure it won't

[PATCH] D129104: [Modules] Add ODR Check for concepts

ChuanqiXu added a comment. @erichkeane @ilya-biryukov gentle ping~ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129104/new/ https://reviews.llvm.org/D129104 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-b

[PATCH] D129068: [AST] Accept identical TypeConstraint referring to other template parameters.

ChuanqiXu updated this revision to Diff 443823. ChuanqiXu added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129068/new/ https://reviews.llvm.org/D129068 Files: clang/include/clang/AST/ASTContext.h clang/lib/AST/ASTContext.cpp clang/test/Modules/conc

[PATCH] D129068: [AST] Accept identical TypeConstraint referring to other template parameters.

ChuanqiXu marked 2 inline comments as done. ChuanqiXu added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:6254 return false; if (TXTC->hasExplicitTemplateArgs() != TYTC->hasExplicitTemplateArgs()) return false; ilya-biryukov w

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

ChuanqiXu added a comment. In D126907#3645198 , @erichkeane wrote: > In D126907#3644127 , @ChuanqiXu > wrote: > >> In D126907#3642530 , @erichkeane >> wrote: >> >>> Thi

[PATCH] D129104: [Modules] Add ODR Check for concepts

ChuanqiXu added a comment. Thanks for reviewing and noting! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129104/new/ https://reviews.llvm.org/D129104 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mail

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

ChuanqiXu added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:963-964 + Entity(Entity), + EvaluatingAConstraint(EvaluatingConstraint || +!SemaRef.CurContext->isDependentContext()) {} er

[PATCH] D129104: [Modules] Add ODR Check for concepts

ChuanqiXu updated this revision to Diff 443957. ChuanqiXu added a comment. Add ReleaseNotes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129104/new/ https://reviews.llvm.org/D129104 Files: clang/docs/ReleaseNotes.rst clang/lib/AST/ASTContext.cpp clang/test/Modules/concept_diffe

[PATCH] D129104: [Modules] Add ODR Check for concepts

ChuanqiXu updated this revision to Diff 443958. ChuanqiXu added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129104/new/ https://reviews.llvm.org/D129104 Files: clang/docs/ReleaseNotes.rst clang/lib/AST/ASTContext.cpp clang/test/Modules/concept_diffe

[PATCH] D129104: [Modules] Add ODR Check for concepts

ChuanqiXu marked 2 inline comments as done. ChuanqiXu added inline comments. Comment at: clang/docs/ReleaseNotes.rst:187 +- Clang now would check ODR violations when merging concepts from different modules. + Note that it may break existing codes and the behavior is intended. +

[PATCH] D129068: [AST] Accept identical TypeConstraint referring to other template parameters.

ChuanqiXu marked an inline comment as done. ChuanqiXu added a comment. Thanks for reviewing! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129068/new/ https://reviews.llvm.org/D129068 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D129104: [Modules] Add ODR Check for concepts

ChuanqiXu updated this revision to Diff 443961. ChuanqiXu marked an inline comment as done. ChuanqiXu added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129104/new/ https://reviews.llvm.org/D129104 Files: clang/docs/ReleaseNotes.rst clang/lib/AST/ASTCo

[PATCH] D129104: [Modules] Add ODR Check for concepts

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG4b95a5a77253: [Modules] Add ODR Check for concepts (authored by ChuanqiXu). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D129068: [AST] Accept identical TypeConstraint referring to other template parameters.

This revision was automatically updated to reflect the committed changes. Closed by commit rGf6b0ae144ed8: [AST] Accept identical TypeConstraint referring to other template (authored by ChuanqiXu). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129068

[PATCH] D128974: [RFC] [AST] [Modules] Handle full cases of DefaultArgStorage::setInherited

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5791bcf9db0a: [AST] [Modules] Handle full cases of DefaultArgStorage::setInherited (authored by ChuanqiXu). Repository: rG LLVM Github Monorepo C

[PATCH] D128690: [ODRHash diagnostics] Preparation to minimize subsequent diffs. NFC.

ChuanqiXu added inline comments. Comment at: clang/lib/Serialization/ASTReader.cpp:10621-10626 // Compute the hash of the method as if it has no body. -auto ComputeCXXMethodODRHash = [&Hash](const CXXMethodDecl *D) { - Hash.clear(); - Hash.AddFu

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

ChuanqiXu added a reviewer: ychen. ChuanqiXu added a comment. @jyknight @nikic @rjmccall @efriedma @nhaehnle gentle ping~ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125291/new/ https://reviews.llvm.org/D125291 ___ cfe-commits mailing list

[PATCH] D127187: [C++20] [Modules] Implement AllAdditionalTUReachable

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

[PATCH] D127187: [C++20] [Modules] Implement AllAdditionalTUReachable

ChuanqiXu abandoned this revision. ChuanqiXu added a comment. In D127187#3650824 , @iains wrote: > Hi @ChuanqiXu, > I have no comment on the technical content of the patch (it looks reasonable > to me). > > However, I wonder if we should be supplying th

[PATCH] D129748: [Modules] Disable preferred_name attribute in C++20 Modules

ChuanqiXu created this revision. ChuanqiXu added reviewers: rsmith, vsapsai, ilya-biryukov, erichkeane, iains, v.g.vassilev. ChuanqiXu added a project: clang-modules. Herald added a reviewer: aaron.ballman. Herald added a project: All. ChuanqiXu requested review of this revision. Herald added a pr

[PATCH] D129748: [Modules] Disable preferred_name attribute in C++20 Modules

ChuanqiXu added a comment. Or is it better to add a special option `-fno-preferred-name` to ignore it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129748/new/ https://reviews.llvm.org/D129748 ___ cfe-c

[PATCH] D128921: [Sema] Merge C++20 concept definitions from different modules in same TU

ChuanqiXu accepted this revision. ChuanqiXu added a comment. This revision is now accepted and ready to land. LGTM if the comments about diagnostic get addressed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128921/new/ https://reviews.llvm.org/D1

[PATCH] D129748: [Modules] Disable preferred_name attribute in C++20 Modules

ChuanqiXu added a comment. In D129748#3651771 , @erichkeane wrote: > I guess I don't have a good idea why this attribute would cause ODR issues? > It would seem that if it appeared in 2 different TUs that we could just > 'pick' whichever we wanted, ri

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

ChuanqiXu updated this revision to Diff 444880. ChuanqiXu marked 3 inline comments as done. ChuanqiXu added a comment. Address comments and remove verifier due to the automatic merging for constant expression I mentioned above. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125291/new/

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

ChuanqiXu marked an inline comment as done. ChuanqiXu added a comment. In D125291#3651640 , @nhaehnle wrote: > I can't judge the Clang changes. Now the clang part is moved to D129833 . > On the LLVM side, can you also a

[PATCH] D129748: [Modules] Disable preferred_name attribute in C++20 Modules

ChuanqiXu added a comment. In D129748#3654918 , @aaron.ballman wrote: > For example, I would be IBOutletCollection, OwnerAttr, PointerAttr, and > TypeTagForDatatypeAttr all behave the same way as they all take a type > argument. I don't think we want t

[PATCH] D129174: [C++20][Modules] Update ADL to handle basic.lookup.argdep p4 [P1815R2 part 1]

ChuanqiXu accepted this revision. ChuanqiXu added a comment. This revision is now accepted and ready to land. LGTM basically if the following comments addressed. Comment at: clang/lib/Sema/SemaLookup.cpp:3864-3878 + for (auto *E : AssociatedClasses) { +

[PATCH] D129748: [Modules] Disable preferred_name attribute in C++20 Modules

ChuanqiXu added a comment. In D129748#3660650 , @tahonermann wrote: > I neglected to explicitly mention in conjunction with my last comment, but > @ChuanqiXu, can you check to see if we are indeed serializing class template > specializations "too early

[PATCH] D129748: [Modules] Disable preferred_name attribute in C++20 Modules

ChuanqiXu added a comment. In D129748#3660684 , @aaron.ballman wrote: > The argument type is `TypeArgument`, not `TypdefType`. The guts of the > reading code for the preferred name attribute *should* be: Oh, My bad. I didn't mention it in the above res

[PATCH] D129748: [Modules] Disable preferred_name attribute in C++20 Modules

ChuanqiXu added a comment. In D129748#3662498 , @erichkeane wrote: > I'll note that I don't "like" the idea, so much as I see this patch as an > 'improvement' to unblock efforts with additional value, though not much of > one. I suspect as Aaron does

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

ChuanqiXu added a comment. In D126907#3665900 , @erichkeane wrote: > The more I look at this... the more I question my approach. I now am > definitely sure we won't make Clang15, and hope that I can figure something > better out for 16 :/ I feel like

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

ChuanqiXu added a comment. I feel like we could land this sooner to avoid any unimagined failures. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126189/new/ https://reviews.llvm.org/D126189 ___ cfe-commi

[PATCH] D129748: [C++20] [Modules] Warn for the use of preferred_name in modules

ChuanqiXu updated this revision to Diff 446404. ChuanqiXu retitled this revision from "[Modules] Disable preferred_name attribute in C++20 Modules" to "[C++20] [Modules] Warn for the use of preferred_name in modules". ChuanqiXu edited the summary of this revision. ChuanqiXu added a comment. Don'

[PATCH] D129748: [C++20] [Modules] Warn for the use of preferred_name in modules

ChuanqiXu added a comment. In D129748#3666511 , @tahonermann wrote: > I can imagine several ways in which things may be going wrong. Perhaps > (de)serialization is switching to a canonical declaration when it should not > be (e.g., reading/writing a de

[PATCH] D129748: [C++20] [Modules] Warn for the use of preferred_name in modules

ChuanqiXu added a comment. @erichkeane @aaron.ballman @tahonermann I get another idea. We could tell the user honestly that we couldn't handle it (temporarily). We could still modularize the STL by a trick: https://github.com/ChuanqiXu9/stdmodules/blob/e81f4e9e74f96021f2e45c48f44da93e806c4524/M

[PATCH] D128490: [ODRHash diagnostics] Transform method `ASTReader::diagnoseOdrViolations` into a class `ODRDiagsEmitter`. NFC.

ChuanqiXu added a comment. In D128490#3670441 , @vsapsai wrote: > Sorry for the huge amount of changes in ASTReader.cpp but most of them are > indentation changes, so I hope it's not too bad. I've tried to minimize the > changes unrelated to introducing

[PATCH] D128695: [ODRHash diagnostics] Move `ODRDiagsEmitter` to libAST in separate files. NFC.

ChuanqiXu added a comment. It looks like we lack a patch to use it in parser, right? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128695/new/ https://reviews.llvm.org/D128695 ___ cfe-commits mailing lis

[PATCH] D129138: [clang] [docs] Update the changes of C++20 Modules in clang15

ChuanqiXu added a comment. @iains I'm going to land this in next Monday if all the dependent patch landed. Do you feel good with this? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129138/new/ https://reviews.llvm.org/D129138 ___ cfe-commits

[PATCH] D130331: [C++20] [Modules] Disable preferred_name when writing a C++20 Module interface

ChuanqiXu created this revision. ChuanqiXu added reviewers: erichkeane, tahonermann, aaron.ballman. 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 an

[PATCH] D129748: [C++20] [Modules] Warn for the use of preferred_name in modules

ChuanqiXu added a comment. In D129748#3668446 , @erichkeane wrote: > In D129748#3667932 , @ChuanqiXu > wrote: > >> @erichkeane @aaron.ballman @tahonermann I get another idea. We could tell >> the user honestly

[PATCH] D130331: [C++20] [Modules] Disable preferred_name when writing a C++20 Module interface

ChuanqiXu added inline comments. Comment at: clang/lib/Serialization/ASTWriter.cpp:4353 + // https://github.com/llvm/llvm-project/issues/56490 for example. + if (!A || (isa(A) && Writer->isWritingNamedModules())) return Record.push_back(0); The `Writer->is

[PATCH] D129174: [C++20][Modules] Update ADL to handle basic.lookup.argdep p4 [P1815R2 part 1]

ChuanqiXu added inline comments. Comment at: clang/lib/Sema/SemaLookup.cpp:3864-3878 + for (auto *E : AssociatedClasses) { +// and have the same innermost enclosing non-inline namespace +// scope as a declaration of an associated entit

[PATCH] D130331: [C++20] [Modules] Disable preferred_name when writing a C++20 Module interface

ChuanqiXu updated this revision to Diff 447185. ChuanqiXu added a comment. Add more tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130331/new/ https://reviews.llvm.org/D130331 Files: clang/include/clang/Serialization/ASTWriter.h clang/lib/Serialization/ASTReaderDecl.cpp cla

[PATCH] D130331: [C++20] [Modules] Disable preferred_name when writing a C++20 Module interface

ChuanqiXu added inline comments. Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:2925-2926 for (unsigned I = 0, E = readInt(); I != E; ++I) -Attrs.push_back(readAttr()); +if (auto *Attr = readAttr()) + Attrs.push_back(Attr); } tahonermann w

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

ChuanqiXu added inline comments. Comment at: clang/include/clang/Sema/Scope.h:213-215 + /// A single NRVO candidate variable in this scope, or nullptr if the + /// candidate is not available/allowed in this scope. + Optional NRVO; Now NRVO has three states, No

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