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

2022-06-08 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D126694#3565417 , @ChuanqiXu wrote: > In D126694#3564629 , @iains wrote: > >> the first failure is like this: >> >> x-h.h: >> struct A { >> friend int operator+(const A &lhs, cons

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

2022-06-07 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D126694#3564629 , @iains wrote: > the first failure is like this: > > x-h.h: > struct A { > friend int operator+(const A &lhs, const A &rhs) { > return 0; > } > }; > > X.cpp: > module; > #include

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

2022-06-07 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. the first failure is like this: x-h.h: struct A { friend int operator+(const A &lhs, const A &rhs) { return 0; } }; X.cpp: module; #include "x-h.h" export module X; export using ::A; This does not mark *anything* in the GMF (x-h.h) as 'u

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

2022-06-07 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. again, thanks for review - but please do not spend any effort on style points yet - the debug and dump stuff is intentionally present this is "for comment on the approach" i.e. what is important is to establish that this is a reasonable approach. As of now we have the fo

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

2022-06-07 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 434834. iains marked 8 inline comments as done. iains added a comment. rebased and removed dependency on p1874 initializer patch. Some tidying - added 104. ex2 testcase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

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

2022-06-07 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D126694#3561257 , @iains wrote: > so standard 10.4 ex 2 gives the expected output with this patch stack Cool, and I think we should add that one as a test if it works. Tests are always good. --- BTW, I guess we could remo

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

2022-06-06 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. so standard 10.4 ex 2 gives the expected output with this patch stack note that there's a query to core about whether that example is fully correct (but that does not affect the skeleton of these implements - only the detail of what is included / excluded in the `markReac

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

2022-06-06 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 434563. iains added a comment. reworked to use D113545 as a parent revision which resolves the issue of needing to make module ownership 'visible' to satisfy lookup. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

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

2022-06-06 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. In D126694#3559818 , @ChuanqiXu wrote: > Given it touches ModuleOwnershipKind too and some codes looks similar to me > (I haven't look into the it yet), I want to know if it is possible to make > D113545

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

2022-06-06 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. on the serialisation-front; maybe it would be better just to stream the 3 bits of the ownership and then we only need to check on deserialisation that the owning module exists for anything other than !owned. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

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

2022-06-06 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. Given it touches ModuleOwnershipKind too and some codes looks similar to me (I haven't look into the it yet), I want to know if it is possible to make D113545 a parent revision of this one? I feel like reachability might be more impo

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

2022-06-06 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment. @chuanqiXu, thanks for reviewing - but it seems I need to find the right direction before dealing with the details. @rsmith - So here is a revised strategy - it is incomplete, and the code contains debug - so I am posting only for a review of the direction taken.

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

2022-06-06 Thread Iain Sandoe via Phabricator via cfe-commits
iains updated this revision to Diff 434417. iains added a comment. Herald added a reviewer: aaron.ballman. this is a re-write - please see additional comments on the revised direction. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126694/new/ https

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

2022-05-31 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/lib/Sema/Sema.cpp:1150 + continue; +if (!D->isUsed(false) && !D->isReferenced()) + WorkList.push_back(D); rsmith wrote: > ChuanqiXu wrote: > > Should we check for `D->isUsed()` simply? It l

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

2022-05-31 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/lib/Sema/Sema.cpp:1150 + continue; +if (!D->isUsed(false) && !D->isReferenced()) + WorkList.push_back(D); ChuanqiXu wrote: > Should we check for `D->isUsed()` simply? It looks more straight

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

2022-05-31 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments. Comment at: clang/test/CXX/basic/basic.scope/basic.scope.namespace/p2.cpp:32 void test_early() { - in_header = 1; // expected-error {{missing '#include "foo.h"'; 'in_header' must be declared before it is used}} - // expected-note@*{{not visibl

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

2022-05-31 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments. Comment at: clang/lib/Sema/Sema.cpp:1130 DiagnoseUseOfUnimplementedSelectors(); +// For C++20 modules, we are permitted to elide decls in the Global I prefer to wrap this logic to a function to make it easier to read.

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

2022-05-31 Thread Iain Sandoe via Phabricator via cfe-commits
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. The basis for removal of GMF decls is that