[PATCH] D126691: ASTContext: Provide a query for module initializer contents.

2023-10-03 Thread Iain Sandoe via Phabricator via cfe-commits
iains abandoned this revision. iains added a comment. although this was approved, we did not need to use it to implement the dependent changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126691/new/ https://reviews.llvm.org/D126691 ___

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

2023-07-04 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D126694#4470297 , @ChuanqiXu wrote: >> Yes, that was the decision at the last time we looked - because removing >> decls would degrade this - if we have new information that changes our >> preferred design, then fine. > > I rem

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

2023-07-03 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D126694#4470261 , @ChuanqiXu wrote: >> That is clearly a big motivation - I will ask the folks we were talking to >> at WG21 if that is their priority - or maybe they care about language >> isolation etc. > > Yeah, I know the f

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

2023-07-03 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D126694#4470250 , @ChuanqiXu wrote: > BTW, in my experience for talking about modules to users, they mainly/mostly > care about the compilation performance. And I can't image how many people > would like to use modules if they

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

2023-07-03 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D126694#4470139 , @ChuanqiXu wrote: > Now I think the feature may be important for the performance of modules. And > I feel we should work on the ASTWriter side instead of ASTReader side. Since > the size of BMIs is a problem n

[PATCH] D145965: [C++20][Modules] Fix incorrect visibilities in implementation units.

2023-07-01 Thread Iain Sandoe via Phabricator via cfe-commits
iains planned changes to this revision. iains added a comment. changes are needed to address review comments - but this revision is needed as a parent to other work (so might need to be rebased from time to time) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D145965: [C++20][Modules] Fix incorrect visibilities in implementation units.

2023-07-01 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 536524. iains added a comment. rebased, fixed some format issues; again to support p1815 work only. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145965/new/ https://reviews.llvm.org/D145965 Files: clang/inclu

[PATCH] D145965: [C++20][Modules] Fix incorrect visibilities in implementation units.

2023-06-30 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 536457. iains added a comment. just rebased to support p1815 work Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145965/new/ https://reviews.llvm.org/D145965 Files: clang/include/clang/Basic/DiagnosticSemaKinds

[PATCH] D153542: [C++20][Modules] Implement P2615R1 exported specialization diagnostics.

2023-06-26 Thread Iain Sandoe via Phabricator via cfe-commits
iains planned changes to this revision. iains added a comment. need to re-check that the intention of the paper is covered (since we currently treat bare 'export' and 'export {}' in a similar manner). Comment at: clang/lib/Sema/SemaModule.cpp:848-849 +if (auto *FD = dyn_ca

[PATCH] D153542: [C++20][Modules] Implement P2615R1 exported specialization diagnostics.

2023-06-26 Thread Iain Sandoe via Phabricator via cfe-commits
iains added inline comments. Comment at: clang/lib/Sema/SemaModule.cpp:848-849 +if (auto *FD = dyn_cast(D)) { + if (FD->getTemplateSpecializationKind() == TSK_ExplicitSpecialization) +BadExport = true; +} else if (auto *VD = dyn_cast(D)) { Ch

[PATCH] D152746: [C++20][Modules] Complete implementation of module.import p7.

2023-06-25 Thread Iain Sandoe via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGb37233a253f3: [C++20][Modules] Complete implementation of module.import p7. (authored by iains). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152746/new/ h

[PATCH] D152946: [C++20][Modules] Implement P2615R1 revised export diagnostics.

2023-06-24 Thread Iain Sandoe via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. iains marked an inline comment as done. Closed by commit rGe5c7904fa0bf: [C++20][Modules] Implement P2615R1 revised export diagnostics. (authored by iains). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D152746: [C++20][Modules] Complete implementation of module.import p7.

2023-06-22 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 533656. iains added a comment. rebased and fixed some formatting. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152746/new/ https://reviews.llvm.org/D152746 Files: clang/include/clang/Basic/Module.h clang/li

[PATCH] D153542: [C++20][Modules] Implement P2615R1 exported specialization diagnostics.

2023-06-22 Thread Iain Sandoe via Phabricator via cfe-commits
iains created this revision. Herald added a subscriber: ChuanqiXu. Herald added a project: All. iains added a reviewer: ChuanqiXu. iains added subscribers: clang-modules, h-vetinari. iains published this revision for review. iains added a comment. Herald added a project: clang. Herald added a subsc

[PATCH] D152946: [C++20][Modules] Implement P2615R1 revised export diagnostics.

2023-06-22 Thread Iain Sandoe via Phabricator via cfe-commits
iains marked 4 inline comments as done. iains added a comment. not sure why the debian CI is reported clang-format errors; I am not seeing them here. Comment at: clang/lib/Sema/SemaModule.cpp:824-827 + bool AllUnnamed = true; + for (auto *D : DC->decls()) +AllUnnamed &=

[PATCH] D152946: [C++20][Modules] Implement P2615R1 revised export diagnostics.

2023-06-22 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 533554. iains added a comment. rebased, addressed review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152946/new/ https://reviews.llvm.org/D152946 Files: clang/include/clang/Basic/DiagnosticSemaKind

[PATCH] D152946: [C++20][Modules] Implement P2615R1 revised export diagnostics.

2023-06-21 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D152946#4431974 , @h-vetinari wrote: > Mark P2615 as implemented in > https://github.com/llvm/llvm-project/blame/main/clang/www/cxx_status.html? In D152946#4431974

[PATCH] D145965: [C++20][Modules] Fix incorrect visibilities in implementation units.

2023-06-19 Thread Iain Sandoe via Phabricator via cfe-commits
iains marked an inline comment as done. iains added a comment. In D145965#4431997 , @ChuanqiXu wrote: >> It is a case that we have supported; the user puts in a use of a decl but >> forgets to import the module exporting it (I agree it is not _exactly_ a

[PATCH] D145965: [C++20][Modules] Fix incorrect visibilities in implementation units.

2023-06-19 Thread Iain Sandoe via Phabricator via cfe-commits
iains marked 3 inline comments as done. iains added a comment. I will look at the rest of the comments once back in the office. In D145965#4431929 , @ChuanqiXu wrote: > In D145965#4431888 , @iains wrote: > >> In

[PATCH] D145965: [C++20][Modules] Fix incorrect visibilities in implementation units.

2023-06-19 Thread Iain Sandoe via Phabricator via cfe-commits
iains marked 2 inline comments as done. iains added a comment. In D145965#4431846 , @ChuanqiXu wrote: >> if we do not adjust the typo fixes, we will regress diagnostics. > > What the kind of diagnostics will be regressed? I mean, it looks weird to me > t

[PATCH] D152946: [C++20][Modules] Implement P2615R1 revised export diagnostics.

2023-06-14 Thread Iain Sandoe via Phabricator via cfe-commits
iains created this revision. Herald added a subscriber: ChuanqiXu. Herald added a project: All. iains added a reviewer: ChuanqiXu. iains added a subscriber: clang-modules. iains edited the summary of this revision. iains published this revision for review. iains added a comment. Herald added a proj

[PATCH] D145965: [C++20][Modules] Fix incorrect visibilities in implementation units.

2023-06-12 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 530786. iains added a comment. rebased, added testcase for Issue 61601. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145965/new/ https://reviews.llvm.org/D145965 Files: clang/include/clang/Basic/DiagnosticSem

[PATCH] D152746: [C++20][Modules] Complete implementation of module.import p7.

2023-06-12 Thread Iain Sandoe via Phabricator via cfe-commits
iains created this revision. Herald added a subscriber: ChuanqiXu. Herald added a project: All. iains edited the summary of this revision. iains added a reviewer: ChuanqiXu. iains added a subscriber: clang-modules. iains published this revision for review. iains added a comment. Herald added a proj

[PATCH] D145965: [C++20][Modules] Fix incorrect visibilities in implementation units.

2023-06-09 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. about the comments that the patch seems to do multiple things; I do not think we can fix the lookup without touching the typo-fixes since it uses the same underlying machinery - if we do not adjust the typo fixes, we will regress diagnostics. Repository: rG LLVM Gith

[PATCH] D145965: [C++20][Modules] Fix incorrect visibilities in implementation units.

2023-06-09 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 529852. iains marked 2 inline comments as done. iains added a comment. rebased and adjusted for upstream changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145965/new/ https://reviews.llvm.org/D145965 Files:

[PATCH] D145965: [C++20][Modules] Fix incorrect visibilities in implementation units.

2023-04-06 Thread Iain Sandoe via Phabricator via cfe-commits
iains marked 2 inline comments as done. iains added inline comments. Comment at: clang/include/clang/Sema/Sema.h:2356 + /// Determine whether the module M is part of the current named module. + bool isPartOfCurrentNamedModule(const Module *M) const { +if (!M || M->isGlobalM

[PATCH] D145965: [C++20][Modules] Fix incorrect visibilities in implementation units.

2023-04-06 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. in the end, we have to deal with the following cases: 1. decls in the same TU (including GMF and PMF) 2. decls in the current named module (excluding GMF) 3. decls on an instantiation path (which can include internal-linkage entities) 4. decls that are needed by an exported

[PATCH] D145965: [C++20][Modules] Fix incorrect visibilities in implementation units.

2023-04-06 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 511327. iains added a comment. rebased, addressed review comments (patch still needs refactoring) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145965/new/ https://reviews.llvm.org/D145965 Files: clang/include

[PATCH] D145965: [C++20][Modules] Fix incorrect visibilities in implementation units.

2023-04-02 Thread Iain Sandoe via Phabricator via cfe-commits
iains marked 2 inline comments as done. iains added inline comments. Comment at: clang/lib/Sema/SemaLookup.cpp:2082 +Module *DeclModule = SemaRef.getOwningModule(D); +if (DeclModule && !DeclModule->isModuleMapModule() && +!SemaRef.isModuleUnitOfCurrentTU(DeclModul

[PATCH] D145965: [C++20][Modules] Fix incorrect visibilities in implementation units.

2023-04-02 Thread Iain Sandoe via Phabricator via cfe-commits
iains marked 2 inline comments as done. iains added a comment. I updated this because I am going to push the latest version of the `P1815` patch and that depends on the lookup changes. Comment at: clang/lib/Sema/SemaLookup.cpp:2082 +Module *DeclModule = SemaRef.getOwningMo

[PATCH] D145965: [C++20][Modules] Fix incorrect visibilities in implementation units.

2023-04-02 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 510388. iains added a comment. rebased Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145965/new/ https://reviews.llvm.org/D145965 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/include/clang/S

[PATCH] D126959: [C++20][Modules] Introduce an implementation module.

2023-03-28 Thread Iain Sandoe via Phabricator via cfe-commits
iains marked an inline comment as done. iains added inline comments. Comment at: clang/lib/Lex/ModuleMap.cpp:935 + // with any legal user-defined module name). + StringRef IName = ".ImplementationUnit"; + assert(!Modules[IName] && "multiple implementation units?"); ---

[PATCH] D126959: [C++20][Modules] Introduce an implementation module.

2023-03-28 Thread Iain Sandoe via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG6e4f870a21e3: re-land [C++20][Modules] Introduce an implementation module. (authored by iains). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126959/new/ ht

[PATCH] D126959: [C++20][Modules] Introduce an implementation module.

2023-03-28 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 509014. iains added a comment. rebased, retested. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126959/new/ https://reviews.llvm.org/D126959 Files: clang/include/clang/Basic/Module.h clang/include/clang/Lex/

[PATCH] D146986: Downgrade reserved module identifier error into a warning

2023-03-28 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. +1 for this as an interim solution. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146986/new/ https://reviews.llvm.org/D146986 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D126959: [C++20][Modules] Introduce an implementation module.

2023-03-27 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. It took me a while to get my local macOS based devt. environment to reproduce the problem - but it was as expected; the implementation module was unowned and nothing was deleting it. unless CI throws up anything untoward, I plan to re-land this tomorrow (since there is

[PATCH] D126959: [C++20][Modules] Introduce an implementation module.

2023-03-27 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 508662. iains added a comment. rebased, reworked to have the module owned. The implementation module needs to be owned by the mpodul map so that it is released when done. Reworked the comments and assert to check the main file presence. Repository: rG LLVM

[PATCH] D126959: [C++20][Modules] Introduce an implementation module.

2023-03-27 Thread Iain Sandoe via Phabricator via cfe-commits
iains reopened this revision. iains added a comment. This revision is now accepted and ready to land. I had a hunch that the issue was the non-ownership of the implementation module. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126959/new/ https:/

[PATCH] D126959: [C++20][Modules] Introduce an implementation module.

2023-03-26 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D126959#4222637 , @bjope wrote: > This seem to case problems when building with asan enabled > (LLVM_USE_SANITIZER='Address'): investigating... do you need the patch reverted? Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D145965: [C++20][Modules] Fix incorrect visibilities in implementation units.

2023-03-24 Thread Iain Sandoe via Phabricator via cfe-commits
iains planned changes to this revision. iains added a comment. although this patch does handle the cases needed, it really needs refactoring. posting here since we are both working in this area and this might be useful input. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D145965: [C++20][Modules] Fix incorrect visibilities in implementation units.

2023-03-24 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 507977. iains edited the summary of this revision. iains added a comment. rebased, split the changes to module and private linkage out, this needs to be refactored to collect the test functions into either module.h and / or sema.h Repository: rG LLVM Github

[PATCH] D126959: [C++20][Modules] Introduce an implementation module.

2023-03-23 Thread Iain Sandoe via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. iains marked an inline comment as done. Closed by commit rGc6e9823724ef: [C++20][Modules] Introduce an implementation module. (authored by iains). Repository: rG LLV

[PATCH] D126959: [C++20][Modules] Introduce an implementation module.

2023-03-22 Thread Iain Sandoe via Phabricator via cfe-commits
iains marked an inline comment as done. iains added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:1672-1677 + // A module implementation unit has visibility of the decls in its implicitly + // imported interface. + if (getLangOpts().CPlusPlusModules && NewM && OldM

[PATCH] D126959: [C++20][Modules] Introduce an implementation module.

2023-03-22 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 507535. iains marked 12 inline comments as done. iains edited the summary of this revision. iains added a comment. rebased, addressed review comments, and amended the description. the patch has been repurposed to provide a mechanism to track implementation TUs

[PATCH] D145886: [C++2x][Modules] Amend module purview constant linkage [P2788R0].

2023-03-19 Thread Iain Sandoe 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 rG62a16d5e2069: [C++2x][Modules] Amend module purview constant linkage [P2788R0]. (authored by iains). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D145886: [C++2x][Modules] Amend module purview constant linkage [P2788R0].

2023-03-19 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 506380. iains added a comment. rebased. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145886/new/ https://reviews.llvm.org/D145886 Files: clang/lib/AST/Decl.cpp clang/test/CXX/module/basic/basic.def.odr/p4.c

[PATCH] D126959: [C++20][Modules] Introduce an implementation module.

2023-03-17 Thread Iain Sandoe via Phabricator via cfe-commits
iains marked 10 inline comments as done. iains added a comment. General comments (at least my opinion). 1. One intention of this patch is to make Implementation Module Units regular (i.e. they should behave the same as any other module unit). So I would tend to avoid changes that make this bac

[PATCH] D126959: [C++20][Modules] Introduce an implementation module.

2023-03-16 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. This was originally created (and @ChuanqiXu approved) for the work on module initialisers. I did not apply it then, since it was possible to determine the correct state for the initialisers without it. However, now we are trying to handle some of the details of different

[PATCH] D126959: [C++20][Modules] Introduce an implementation module.

2023-03-16 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 505759. iains added a comment. rebased, and reworked for changes in main. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126959/new/ https://reviews.llvm.org/D126959 Files: clang/include/clang/Basic/Module.h

[PATCH] D144844: [C++20] [Modules] Offer -fno-import-inter-module-function-defs to avoid duplicated compilation in modules

2023-03-15 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D144844#4195633 , @ChuanqiXu wrote: >> However, "performance" also includes compilation speed in the 'no >> optimisation, debug' case - that is also considered very important. So, >> perhaps, the short-term approach should be (

[PATCH] D144844: [C++20] [Modules] Offer -fno-import-inter-module-function-defs to avoid duplicated compilation in modules

2023-03-15 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D144844#4195316 , @ChuanqiXu wrote: > Got your points. Let's postpone this one. > > But I want to emphasize that this patch (and the thin PCM) will decrease the > performance. While LTO can save the regression, LTO is not widely

[PATCH] D145965: [C++20][Modules] Fix incorrect visibilities in implementation units.

2023-03-14 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D145965#4192051 , @iains wrote: > > I was thinking at one stage to add an 'Implementation' module Kind, but at > the moment I do not think it is worth extending the size of the ModuleKind > enum bit field for this (since we

[PATCH] D144844: [C++20] [Modules] Offer -fno-import-inter-module-function-defs to avoid duplicated compilation in modules

2023-03-14 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D144844#4193568 , @dblaikie wrote: > Seem to recall @iains and others were concerned about the number of modules > flags - this one I'd be inclined to suggest we shouldn't add if possible. If > one way or the other is generally

[PATCH] D145965: [C++20][Modules] Fix incorrect visibilities in implementation units.

2023-03-14 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D145965#4192072 , @ChuanqiXu wrote: > In D145965#4192051 , @iains wrote: > >> >> >> >> >> The checks for internal-linkage symbols and the improvements to diagnostics >> do not depend

[PATCH] D145965: [C++20][Modules] Fix incorrect visibilities in implementation units.

2023-03-14 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D145965#4191973 , @ChuanqiXu wrote: > Yeah, it is indeed a problem that `Sema::isModuleUnitOfCurrentTU` doesn't > work for implementation units. And I have been thinking about this for a > while. My thought is that the root cau

[PATCH] D145965: [C++20][Modules] Fix incorrect visibilities in implementation units.

2023-03-14 Thread Iain Sandoe via Phabricator via cfe-commits
iains created this revision. Herald added a subscriber: ChuanqiXu. Herald added a project: All. iains added a reviewer: ChuanqiXu. iains added a comment. iains published this revision for review. Herald added a project: clang. Herald added a subscriber: cfe-commits. notes: a) I need to update the

[PATCH] D145886: [C++2x][Modules] Amend module purview constant linkage [P2788R0].

2023-03-13 Thread Iain Sandoe via Phabricator via cfe-commits
iains created this revision. Herald added a project: All. iains added a reviewer: ChuanqiXu. iains updated this revision to Diff 504524. iains added a comment. iains updated this revision to Diff 504591. iains added a reviewer: rsmith. iains published this revision for review. Herald added a projec

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

2023-02-02 Thread Iain Sandoe 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 rGcdd44e2c8554: [C++20][Modules] Handle template declarations in header units. (authored by iains). Repository: rG LLVM Github Monorepo CHANGES SIN

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

2023-01-31 Thread Iain Sandoe via Phabricator via cfe-commits
iains 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::Delete

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

2023-01-31 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 493546. iains marked 2 inline comments as done. iains added a comment. rebased, added tests for instantiated variable/function templates. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142704/new/ https://reviews.

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

2023-01-30 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. 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 forward? Comment at: clang/lib/Sema/SemaDecl.cpp:15265 FD->getForma

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

2023-01-30 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. 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 forward? Comment at: clang/lib/Sema/SemaDecl.cpp:15265 FD->getForma

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

2023-01-29 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. AFAICT the failing test is unrelated to this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142704/new/ https://reviews.llvm.org/D142704 ___ cfe-commits mailing list cfe-comm

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

2023-01-28 Thread Iain Sandoe via Phabricator via cfe-commits
iains marked an inline comment as done. iains added a comment. In D142704#4088217 , @Arthapz wrote: > tried the patch and it seems to work with libstdc++ but not with libc++ > > > > rm -rf .xmake build; xmake f --toolchain=clang > --cxxflags="-stdli

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

2023-01-28 Thread Iain Sandoe via Phabricator via cfe-commits
iains marked an inline comment as done. iains added a comment. in my local testing, I was able to consume all libc++ headers individually. Comment at: clang/lib/Sema/SemaDecl.cpp:15265 FD->getFormalLinkage() == Linkage::ExternalLinkage && - !FD->isInvalidDecl() && B

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

2023-01-28 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 493014. iains added a comment. rebased, and revised to handle variable templates and instantiations. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142704/new/ https://reviews.llvm.org/D142704 Files: clang/lib/

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

2023-01-27 Thread Iain Sandoe via Phabricator via cfe-commits
iains planned changes to this revision. iains added a comment. this is necessary, but not sufficient (I need to make additions) .. no need to review yet. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142704/new/ https://reviews.llvm.org/D142704

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

2023-01-27 Thread Iain Sandoe via Phabricator via cfe-commits
iains created this revision. Herald added a project: All. iains added reviewers: dblaikie, ChuanqiXu. iains published this revision for review. iains added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. @dblaikie - I suspect that this would be useful on the llvm-

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

2023-01-22 Thread Iain Sandoe 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 rG53a1314ed1b5: [C++20][Modules] Fix named module import diagnostics. (authored by iains). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

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

2023-01-21 Thread Iain Sandoe 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 rGff70e22f08d9: [C++20][Modules] Handle defaulted and deleted functions in header units. (authored by iains). Repository: rG LLVM Github Monorepo C

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

2023-01-18 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 490383. iains added a comment. rebased. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141908/new/ https://reviews.llvm.org/D141908 Files: clang/lib/Sema/SemaDecl.cpp clang/test/CXX/module/module.import/p6.cp

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

2023-01-18 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 490099. iains added a comment. rebased Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140927/new/ https://reviews.llvm.org/D140927 Files: clang/include/clang/Sema/Sema.h clang/lib/Parse/Parser.cpp clang/lib

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

2023-01-18 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D141908#4061451 , @ChuanqiXu wrote: >> 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 : ) (I added the FIXME) Hav

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

2023-01-18 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 490079. iains marked 5 inline comments as done. iains added a comment. address review commments, add an assert and a FIXME. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141908/new/ https://reviews.llvm.org/D1419

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

2023-01-18 Thread Iain Sandoe via Phabricator via cfe-commits
iains marked 2 inline comments as done. iains added a comment. In D141908#4061409 , @ChuanqiXu wrote: > 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

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

2023-01-18 Thread Iain Sandoe via Phabricator via cfe-commits
iains 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 && current

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

2023-01-18 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 490062. iains added a comment. rebased, address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141908/new/ https://reviews.llvm.org/D141908 Files: clang/lib/Sema/SemaDecl.cpp clang/test/CXX/mo

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

2023-01-17 Thread Iain Sandoe via Phabricator via cfe-commits
iains created this revision. Herald added a project: All. iains added a reviewer: ChuanqiXu. iains published this revision for review. Herald added a project: clang. Herald added a subscriber: cfe-commits. Address part of https://github.com/llvm/llvm-project/issues/60079. Deleted and Defaulted fu

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

2023-01-08 Thread Iain Sandoe via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG335668b11643: [C++20][Modules] Do not allow non-inline external definitions in header units. (authored by iains). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

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

2023-01-04 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. how does that look? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140261/new/ https://reviews.llvm.org/D140261 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists

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

2023-01-04 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 486202. iains added a comment. rebase, added release notes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140261/new/ https://reviews.llvm.org/D140261 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Ba

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

2023-01-04 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D140927#4024648 , @ChuanqiXu wrote: > [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 i

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

2023-01-03 Thread Iain Sandoe via Phabricator via cfe-commits
iains created this revision. Herald added a subscriber: ChuanqiXu. Herald added a project: All. iains added reviewers: ChuanqiXu, dblaikie. iains added a subscriber: clang-modules. iains published this revision for review. Herald added a project: clang. Herald added a subscriber: cfe-commits. We h

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

2022-12-19 Thread Iain Sandoe via Phabricator via cfe-commits
iains marked an inline comment as done. iains added a comment. In D140261#4006653 , @ChuanqiXu wrote: > In D140261#4004542 , @iains wrote: > >> OK so this is what I plan to land assuming testing goes OK. >> I susp

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

2022-12-19 Thread Iain Sandoe via Phabricator via cfe-commits
iains marked 2 inline comments as done. iains added a comment. 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". Comment at: clang/lib/Sema/SemaD

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

2022-12-19 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 483909. iains added a comment. rebased, addressed comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140261/new/ https://reviews.llvm.org/D140261 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td

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

2022-12-19 Thread Iain Sandoe via Phabricator via cfe-commits
iains 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] D134589: [C++20][Modules] Elide unused guard variables in Itanium ABI module initializers.

2022-12-18 Thread Iain Sandoe via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGbd7f4c561f5e: [C++20][Modules] Elide unused guard variables in Itanium ABI moduleā€¦ (authored by iains). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134589/

[PATCH] D134589: [C++20][Modules] Elide unused guard variables in Itanium ABI module initializers.

2022-12-17 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 483770. iains added a comment. rebased, amended a comment as suggested Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134589/new/ https://reviews.llvm.org/D134589 Files: clang/lib/CodeGen/CGDeclCXX.cpp clang/

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

2022-12-17 Thread Iain Sandoe via Phabricator via cfe-commits
iains created this revision. Herald added a project: All. iains added a reviewer: ChuanqiXu. iains added a subscriber: clang-modules. iains published this revision for review. iains added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. this came up during discussi

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

2022-12-09 Thread Iain Sandoe via Phabricator via cfe-commits
iains accepted this revision. iains added a comment. This revision is now accepted and ready to land. this LGTM ( but please wait for an ack from @dblaikie ) and again thanks for patience in seeing this through. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137059/new/ https://reviews.

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

2022-12-09 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. thanks all for the patience on this one - and for the collaborative discussion - I do think the outcome is going to be an easier to remember option name ;) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137058/new/ https://reviews.llvm.org/D137058 ___

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

2022-12-05 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D137059#3973016 , @ben.boeckel wrote: > In D137059#3934448 , @dblaikie > wrote: > >> I'm still curious what about the details of other compilers - I think from >> the sounds of it, @ia

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

2022-11-08 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. 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 header for multiple command-line headers is different from the mode where multiple command line headers would each produce a si

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

2022-11-03 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D137058#3906579 , @dblaikie wrote: > I realize I got this jumbled up and the thread about "why do we need to name > things" is meant to be over in D137059 > (sorry @ben.boeckel :/ I know this

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

2022-11-01 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D137059#3898482 , @ChuanqiXu wrote: > In D137059#3898463 , @iains wrote: > >> In D137059#3898239 , @ChuanqiXu >> wrote: >> >>> In D137059#389666

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

2022-11-01 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D137059#3898239 , @ChuanqiXu wrote: > In D137059#3896661 , @dblaikie > wrote: > >> Could you link to the email/discourse discussion about supporting this mode >> (I think you've linked

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

2022-10-19 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D134267#3870064 , @ChuanqiXu wrote: > I grepped `options.td` and got (incomplete) list for options to take a output > name: > > # -o and its alias > -o > -object_file_name= > --output= > > /Fa (windows for assembly

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

2022-10-19 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D134267#3869678 , @dblaikie wrote: > In D134267#3869643 , @iains wrote: > >> In D134267#3869614 , @dblaikie >> wrote: >> >>> In D134267#3869162

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

2022-10-19 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D134267#3869614 , @dblaikie wrote: > In D134267#3869162 , @iains wrote: > >> In D134267#3868830 , @dblaikie >> wrote: >> >>> I'm OK with stickin

  1   2   3   4   5   >