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

ChuanqiXu added inline comments. Comment at: clang/lib/Sema/Sema.cpp:1262 +if (auto *FDD = FD->getDefinition()) { + DefInPMF = FDD->getOwningModule()->isPrivateModule(); + if (!DefInPMF) iains wrote: > ChuanqiXu wrote: > > nit: > (I adde

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

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

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.

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.

ChuanqiXu accepted this revision. ChuanqiXu added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Sema/Sema.h:2264 + /// be checked at the end of the TU. + llvm::SmallPtrSet PendingInlineFuncDecls; + I'm curious

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

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

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.

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

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

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

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 module fragment">; Chu

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

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

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

iains added a comment. the revised diagnostics look like this: ` error: {un-}exported inline function not defined before the private module fragment` with ` note: private module fragment begins here` pointing to the start of the PMF If there is no PMF then we just say: ` error: {un-}exported

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

iains updated this revision to Diff 442942. iains marked 3 inline comments as done. iains added a comment. rebased, reworked - to follow the changes proposed by core - to make the diagnostics follow that and a compromise for the proposed revision before the core amendment. Repository: rG LLV

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

iains planned changes to this revision. iains added a comment. In D128328#3611194 , @ChuanqiXu wrote: > From the discussion, it looks like the 'export' part is not necessary here > and we don't need to care about linkage in this revision. Indeed. > In

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

ChuanqiXu added a comment. From the discussion, it looks like the 'export' part is not necessary here and we don't need to care about linkage in this revision. In D128328#3609827 , @vsapsai wrote: > Sorry for changing my mind. I've thought about the err

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

vsapsai added a comment. Sorry for changing my mind. I've thought about the errors more and especially about the case mentioned by Chuanqi export module A; [export] inline void func(); I'm afraid it can complicate the implementation but we can achieve some consistency with errors like e

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

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

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

iains added a comment. In D128328#3603980 , @ChuanqiXu wrote: > >>> Also, **if** [module.private.frag]p2.1 is changed into: >>> the point by which the definition of an [exported] inline function or variable is required >>> >>> The test abov

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

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

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

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

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

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

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

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

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

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

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

iains added a comment. In D128328#3602646 , @iains wrote: > In D128328#3601080 , @ChuanqiXu > wrote: > >> It looks like we need to handle inline variable as well to match the >> intention. > > can you construct

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

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

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

vsapsai added a comment. From the perspective of handling `err_export_inline_not_defined` error as a developer what about the following option? export inline void fn_e(); // note: function 'fn_e' exported as 'inline' here // ... module :private; void fn_e() {} // error: definition of

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

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

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

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

iains marked 2 inline comments as done. iains added a comment. In D128328#3601080 , @ChuanqiXu wrote: > It looks like we need to handle inline variable as well to match the > intention. can you construct a test-case, where this would apply and which is

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

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

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

iains created this revision. Herald added a project: All. iains added reviewers: urnathan, ChuanqiXu. iains added a subscriber: clang-modules. iains published this revision for review. Herald added a project: clang. Herald added a subscriber: cfe-commits. This adds a check for exported inline func