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

2022-07-13 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. 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 this option at all because in: https://eel.is/c++draft/module#reach-2 note2 says "[Note 2: It is advisable to avoid depending on

[PATCH] D129045: [C++20][Modules] Update handling of implicit inlines [P1779R3]

2022-07-15 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. reopening to post the patch I plan to re-land. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129045/new/ https://reviews.llvm.org/D129045 __

[PATCH] D129045: [C++20][Modules] Update handling of implicit inlines [P1779R3]

2022-07-15 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 444965. iains added a comment. rebased, fixed interaction with clang modules. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129045/new/ https://reviews.llvm.org/D129045 Files: clang/lib/AST/TextNodeDumper.cpp

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

2022-07-15 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. reopening to post the patch I intend to land. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126189/new/ https://reviews.llvm.org/D126189

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

2022-07-15 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 444989. iains added a comment. rebased, fixed the interaction with clang module map modules. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126189/new/ https://reviews.llvm.org/D126189 Files: clang/include/clan

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

2022-07-18 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 445530. iains marked 5 inline comments as done. iains added a comment. rebased, addressed review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129174/new/ https://reviews.llvm.org/D129174 Files: clan

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

2022-07-18 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. sorry got a little delayed in responding to this (sorting out some testing problems) Comment at: clang/lib/Sema/SemaLookup.cpp:3859 +// C++20 [basic.lookup.argdep] p4.3 .. are exported +Module *FM = D->getOwningModule(); +

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

2022-07-19 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D126189#3662123 , @h-vetinari wrote: > Is it realistic for this to land before LLVM 15 branches? Would be great! that is the intention - I just got side-tracked with trying to replicate the test fails that got the commit rever

[PATCH] D130138: [modules] Replace `-Wauto-import` with `-Rmodule-include-translation`.

2022-07-20 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a subscriber: MaskRay. iains added a comment. makes sense to me... I guess the name looks long at first, but it's specific (I find that easy-to-remember flag names are more important than short-to-type ones, but maybe that's just me) - I wonder if @MaskRay has any comments on the fl

[PATCH] D130138: [modules] Replace `-Wauto-import` with `-Rmodule-include-translation`.

2022-07-20 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. In D130138#3667172 , @MaskRay wrote: > In D130138#3664913 , @iains wrote: > >> makes sense to me... >> >> I gue

[PATCH] D129045: [C++20][Modules] Update handling of implicit inlines [P1779R3]

2022-07-21 Thread Iain Sandoe via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG97af17c5cae6: re-land [C++20][Modules] Update handling of implicit inlines [P1779R3] (authored by iains). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D12904

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

2022-07-22 Thread Iain Sandoe via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGafda39a566d9: re-land [C++20][Modules] Build module static initializers per P1874R1. (authored by iains). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D12618

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

2022-07-23 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 447046. iains added a comment. rebased, addressed review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129174/new/ https://reviews.llvm.org/D129174 Files: clang/include/clang/Sema/Overload.h clang/l

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

2022-07-23 Thread Iain Sandoe via Phabricator via cfe-commits
iains marked 2 inline comments as done. iains added a comment. it would be good to get this landed before llvm-15 branches.. Comment at: clang/lib/Sema/SemaLookup.cpp:3864-3878 + for (auto *E : AssociatedClasses) { +// and have the same innermost en

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

2022-07-23 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 447062. iains added a comment. rebased, retested. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128328/new/ https://reviews.llvm.org/D128328 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/incl

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

2022-07-23 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 447094. iains added a comment. rebase, update testcases for upstream changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126694/new/ https://reviews.llvm.org/D126694 Files: clang/include/clang/AST/DeclBase.

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

2022-07-24 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D129138#3670708 , @ChuanqiXu wrote: > @iains I'm going to land this in next Monday if all the dependent patch > landed. Do you feel good with this? I am not sure if we will get p1815 part 1 landed (effectively today) - and, of

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

2022-07-25 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 rG25558a1bfd79: [C++20][Modules] Update ADL to handle basic.lookup.argdep p4 [P1815R2 part 1] (authored by iains). Changed prior to commit: https://

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

2022-05-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. sorry for being slow, concentrating on module initialisers! this LGTM (but we can still probably make the visibility code cleaner - perhaps after we get the main functionality installed). Repo

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

2022-05-23 Thread Iain Sandoe via Phabricator via cfe-commits
iains created this revision. Herald added a project: All. iains added reviewers: urnathan, Bigcheese, ChuanqiXu, jansvoboda11. iains added a subscriber: clang-modules. iains edited the summary of this revision. iains edited the summary of this revision. iains edited the summary of this revision. ia

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

2022-10-18 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. once again we are getting off topic for this patch :) We (compiler engineers) will have to cater for multiple models used by different build systems. SG15 might give guidance/recommendations, but in the end the standard's normative text is not likely to make a 'discover

[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#3868830 , @dblaikie wrote: > I'm OK with sticking with the existing `-fmodule-file` if that works for > everyone. Yeah, it's short and ambiguous in a space with many concepts of > what a "module file" is, but also the f

[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#3869520 , @tahonermann wrote: >> In a pre-scanned world, the build system does know the info for each source >> file (published and dependent modules) [which ought to dispel some of the >> concerns raised about not kno

[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

[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#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] 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] 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] D128328: [C++20][Modules] Improve handing of Private Module Fragment diagnostics.

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

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

2022-08-15 Thread Iain Sandoe via Phabricator via cfe-commits
iains marked an inline comment as done. iains 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

[PATCH] D128981: [C++20][Modules] Implement include translation.

2022-08-17 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D128981#3728070 , @jansvoboda11 wrote: > Hi @iains, upstream Clang crashes on the attached test case due to an > assertion failure. Git bisect pointed me to this commit. Can you please take > a look? Previously, the test would

[PATCH] D128981: [C++20][Modules] Implement include translation.

2022-08-19 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D128981#3728503 , @iains wrote: > In D128981#3728070 , @jansvoboda11 > wrote: > >> Hi @iains, upstream Clang crashes on the attached test case due to an >> assertion failure. Git bisect

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

2022-08-20 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 454205. iains marked an inline comment as done. iains added a comment. rebased, addressed remaing comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128328/new/ https://reviews.llvm.org/D128328 Files: cla

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

2022-08-20 Thread Iain Sandoe via Phabricator via cfe-commits
iains marked 2 inline comments as done. iains added inline comments. Comment at: clang/lib/Sema/Sema.cpp:1262 +if (auto *FDD = FD->getDefinition()) { + DefInPMF = FDD->getOwningModule()->isPrivateModule(); + if (!DefInPMF) ChuanqiXu wrot

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

2022-08-21 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 rGfee36cda: [C++20][Modules] Improve handing of Private Module Fragment diagnostics. (authored by iains). Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

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

2022-09-24 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 462657. iains added a comment. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. rebased and reworked. The version here has now been tested to consume all of the libc++ headers including those in experimental and ext. Repository: rG LL

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

2022-09-24 Thread Iain Sandoe via Phabricator via cfe-commits
iains created this revision. Herald added a project: All. iains added a reviewer: urnathan. iains added a subscriber: clang-modules. iains published this revision for review. Herald added a project: clang. Herald added a subscriber: cfe-commits. For the Itanium ABI, we emit an initializer for each

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

2022-09-24 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 462689. iains marked 3 inline comments as done. iains edited the summary of this revision. iains added a comment. address review comments, minor amendments to description. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

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

2022-09-24 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. thanks for the review. Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:690-695 + // If we have an empty initializer then we do not want to create a guard var. + // 'Empty' needs only to apply to init functions that we call directly, calls + // to imported

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

2022-09-26 Thread Iain Sandoe via Phabricator via cfe-commits
iains planned changes to this revision. iains added a comment. So, in light of these comments; - It seems that there is one case where I //can// elide the guard - where the init is completely empty (including of calls to imported module inits) - otherwise, I need to investigate what is needed fo

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

2022-10-01 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 464502. iains edited the summary of this revision. iains added a comment. rebased and updated to elide the guard only for the case of no inits _and_ no imports. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D13458

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

2022-10-10 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. @ChuanqiXu BTW, I think we (unfortunately) have some overlap in work in progress here; I have implemented a similar scheme to GCC's where C/BMIs can be produced "on demand" when the FE encounters the module keywords. The idea was to make it possible to have the same com

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

2022-10-10 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D134267#3846218 , @ChuanqiXu wrote: > In D134267#3846153 , @iains wrote: > >> @ChuanqiXu BTW, I think we (unfortunately) have some overlap in work in >> progress here; >> >> I have imple

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

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

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

2022-10-10 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. FAOD: I think this should not be about "who's patches" - but about the compilation model and command lines that we want to end up with. I am concerned that with the current scheme (with or without the change in this review) we could end up with three process launches per

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

2022-10-10 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D134267#3848539 , @ChuanqiXu wrote: > In D134267#3847399 , @iains wrote: > >> >> I am concerned that with the current scheme (with or without the change in >> this review) we could e

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

2022-10-11 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D134267#3848745 , @ChuanqiXu wrote: >> FAOD: I think this should not be about "who's patches" - but about the >> compilation model and command lines that we want to end up with. > > BTW, in the current context, when I say "my" a

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

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

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

2022-10-11 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D134267#3848862 , @ChuanqiXu wrote: > In D134267#3848793 , @iains wrote: > >> In D134267#3848745 , @ChuanqiXu >> wrote: >> FAOD: I think th

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

2022-10-11 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D134267#3848883 , @ChuanqiXu wrote: > In D134267#3848876 , @iains wrote: > >> If we are going to **//require//** the serialisation/deserialisation for >> correctness then IMO we should h

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

2022-10-11 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D134267#3848974 , @ChuanqiXu wrote: > In D134267#3848958 , @iains wrote: > >> In D134267#3848883 , @ChuanqiXu >> wrote: >> >>> In D134267#384887

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

2022-10-11 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. (trying not derail this discussion further) - Yes, the alternate solution proposed for the "Hello World" case would work with the module mapper interface. However, the initial discussion was simply about being able to produce both the .PCM and .O artefacts from one invoc

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

2022-10-11 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D134267#3849712 , @dblaikie wrote: > In D134267#3849673 , @iains wrote: > >> >> - I do not think this patch fully addresses the issue of harmonising GCC and >> clang's command lines

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

2022-10-12 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. To avoid more tangential discussion here - I've opened https://discourse.llvm.org/t/generating-pcm-module-interfaces-and-regular-object-files-from-the-same-compiler-invocation/65918 ... it would be great if the major stakeholders here especially @rsmith could comment on

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

2022-10-13 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D134267#3854980 , @ChuanqiXu wrote: > In D134267#3852136 , @boris wrote: > >>> For example, my experimental support for P1689 >>> is at: [...]. The imple

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

2022-10-17 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D134267#3861615 , @ChuanqiXu wrote: > Address @tschuett's opinion: > > - Use new introduced -fc++-module-cache-path= instead of -fc++-module-path to > avoid many logics about modules cache in clang modules. > - Use new introduce

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

2022-10-17 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D134267#3863938 , @ChuanqiXu wrote: >>> Although modular code is user-facing - BMIs are an implementational detail, >>> right? >> >> but I don't think BMIs are an implementation detail, anymore than object >> files are - users

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

2022-10-17 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. I am also OK with doing this in two steps (first in the driver with this patch and then by updating the FE to allow the two outputs from one invocation - my draft patch series). BTW: I did mean to ask before .,, did you consider this (existing) command syntax? `-fmodule

[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] 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] 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 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 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] 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] 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] 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] 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-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] 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] 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] 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] 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-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-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 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-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-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-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-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-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] 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] 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-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 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 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 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] 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 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] 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] 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] D126694: [C++20][Modules] Implementation of GMF decl elision.

2022-06-12 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 436245. iains edited the summary of this revision. iains added a comment. this is a rework of the implementation. it now passes all test-cases except one - which is one of the cases added by D113545 ; that test-case now seems t

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

2022-06-12 Thread Iain Sandoe via Phabricator via cfe-commits
iains marked 3 inline comments as done. iains added a comment. @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) - but we still need to treat them as non-reachable

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

2022-06-13 Thread Iain Sandoe via Phabricator via cfe-commits
iains created this revision. Herald added a project: All. iains added reviewers: urnathan, ChuanqiXu. iains published this revision for review. iains added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. this is really a bug fix, rather than a new feature - the cu

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

2022-06-13 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 436378. iains marked 2 inline comments as done. iains added a comment. rebased, fixed the fail for explicitly-specialized-template.cpp. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126694/new/ https://reviews.ll

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

2022-06-13 Thread Iain Sandoe via Phabricator via cfe-commits
iains marked 3 inline comments as done. iains added a comment. In D126694#3576853 , @ChuanqiXu wrote: > In D126694#3576602 , @iains wrote: > >> @ChuanqiXu - I changed the module ownership name to "ModuleDiscardabl

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

2022-06-13 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 436487. iains added a comment. rebased. Amended to include a global CTOR entry for each module kind. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126189/new/ https://reviews.llvm.org/D126189 Files: clang/incl

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

2022-06-13 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. @urnathan - I believe that this now implements the same scheme as you have done for GCC (less any of the optimisations). In particular, we now emit global CTOR entries for module initializers, even though these should really be called explicitly, but as was discussed on th

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

2022-06-21 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. @ChuanqiXu - have you tried the test example from 10.6 ex1 - I did the impl as below, but the behaviour does not seem to be quite correct? any thoughts? 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 w

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

2022-06-21 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. as suspected, we also have a missing diagnostic in 10.7 ex1 // RUN: rm -rf %t // RUN: split-file %s %t // RUN: cd %t // RUN: %clang_cc1 -std=c++20 -emit-module-interface std-10-7-ex1-M-A.cpp \ // RUN: -o M-A.pcm // RUN: %clang_cc1 -std=c++20 -emit-module-i

<    1   2   3   4   5   >