[PATCH] D136554: Implement CWG2631

2023-01-17 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment. In D136554#4059723 , @aeubanks wrote: > one more regression bisected to this: > https://bugs.chromium.org/p/chromium/issues/detail?id=1408177 > `incomplete type 'blink::ResourceClient' used in type trait expression` > I'll try

[PATCH] D136554: Implement CWG2631

2023-01-17 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment. one more regression bisected to this: https://bugs.chromium.org/p/chromium/issues/detail?id=1408177 `incomplete type 'blink::ResourceClient' used in type trait expression` I'll try to come up with a smaller repro Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D136554: Implement CWG2631

2023-01-16 Thread Chris Bowler via Phabricator via cfe-commits
cebowleratibm added a comment. In D136554#4056862 , @cor3ntin wrote: > Yes please! However the warning looks correct to me in that case. A > constructs x which constructs A etc. https://github.com/llvm/llvm-project/issues/60082 Please set the

Re: [PATCH] D136554: Implement CWG2631

2023-01-16 Thread Corentin via cfe-commits
Yes please! However the warning looks correct to me in that case. A constructs x which constructs A etc. On Mon, Jan 16, 2023, 18:00 Chris Bowler via Phabricator < revi...@reviews.llvm.org> wrote: > cebowleratibm added a comment. > > I've reduced a regression on: > > commit

[PATCH] D136554: Implement CWG2631

2023-01-16 Thread Chris Bowler via Phabricator via cfe-commits
cebowleratibm added a comment. I've reduced a regression on: commit ca619613801233ef2def8c3cc7d311d5ed0033cb (HEAD, refs/bisect/bad) Author: Corentin Jabot Date: Sun Oct 23 17:32:58 2022 +0200 template int f(T) {

[PATCH] D136554: Implement CWG2631

2023-01-08 Thread Corentin Jabot 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 rGca6196138012: Implement CWG2631 (authored by cor3ntin). Changed prior to commit: https://reviews.llvm.org/D136554?vs=486510=487151#toc

[PATCH] D136554: Implement CWG2631

2023-01-05 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. Unless something else pops up, I'll try to land that this week end. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136554/new/ https://reviews.llvm.org/D136554 ___ cfe-commits

[PATCH] D136554: Implement CWG2631

2023-01-05 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 486510. cor3ntin added a comment. formatting Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136554/new/ https://reviews.llvm.org/D136554 Files: clang/docs/ReleaseNotes.rst

[PATCH] D136554: Implement CWG2631

2023-01-05 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 486508. cor3ntin added a comment. I've convinced myself that the assertition case should be removed. Before this patch, CXXDefaultInitExpr where never visited by UsedDeclVisitor. After this patch, when doing that visitation, a variable can appear in an

[PATCH] D136554: Implement CWG2631

2023-01-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D136554#4021754 , @cor3ntin wrote: > In D136554#4020387 , @MaskRay wrote: > >> @rupprecht You may consider contributing some interesting tests (which work >> before this patch) in a

[PATCH] D136554: Implement CWG2631

2023-01-04 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. reduced to this lovely thing template struct d {}; template struct f : d<__is_constructible(e)> { using type = bool; }; template ::type> int *l; int m; struct p { int o = m; p() {} }; int* i(l); SemaExpr.cpp:19857: void

[PATCH] D136554: Implement CWG2631

2023-01-04 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. I found a new issue doing a bootstrap build, I'm currently reducing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136554/new/ https://reviews.llvm.org/D136554 ___ cfe-commits

[PATCH] D136554: Implement CWG2631

2023-01-04 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. In D136554#4024719 , @rupprecht wrote: > In D136554#4024628 , @rupprecht > wrote: > >> I still see one behavior change (actually it was there before, but I missed >> it in the test

[PATCH] D136554: Implement CWG2631

2023-01-03 Thread Jordan Rupprecht via Phabricator via cfe-commits
rupprecht added a comment. In D136554#4024628 , @rupprecht wrote: > I still see one behavior change (actually it was there before, but I missed > it in the test results), but as far as I can tell, it's a good one? If I > reduce it too much, I get the

[PATCH] D136554: Implement CWG2631

2023-01-03 Thread Jordan Rupprecht via Phabricator via cfe-commits
rupprecht accepted this revision. rupprecht added a comment. I still see one behavior change (actually it was there before, but I missed it in the test results), but as far as I can tell, it's a good one? If I reduce it too much, I get the warning with the baseline toolchain, so it's not

[PATCH] D136554: Implement CWG2631

2023-01-02 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. In D136554#4020387 , @MaskRay wrote: > @rupprecht You may consider contributing some interesting tests (which work > before this patch) in a separate patch:) Well, all the failures reported have been reduced and added as test

[PATCH] D136554: Implement CWG2631

2022-12-30 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. @rupprecht You may consider contributing some interesting tests in a separate patch:) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136554/new/ https://reviews.llvm.org/D136554

[PATCH] D136554: Implement CWG2631

2022-12-30 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 485703. cor3ntin added a comment. Finding the issue took about all day. I reduced your example to template int templated_func() { return 0; } struct test_body { int mem = templated_func(); }; struct S { int a = [] {

[PATCH] D136554: Implement CWG2631

2022-12-29 Thread Jordan Rupprecht via Phabricator via cfe-commits
rupprecht added a comment. I threw this at the "test everything" test (some millions of targets) and it found only one breakage, so this is very very close. Without further ado, here is this silly looking thing: File `blah.h`: #include #include template struct Base { virtual

[PATCH] D136554: Implement CWG2631

2022-12-29 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 485605. cor3ntin added a comment. Another fun one. I really apreciate your help on this. The constructor of `Wrapper` causes the constructor of `Option` to be defined, but not yet used (as default parameters are not odr used) - so none of the default

[PATCH] D136554: Implement CWG2631

2022-12-29 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D136554#4019135 , @rupprecht wrote: > [...] > The undefined symbols before are all provided by libc++, so those are fine. > After, the new undefined symbol for the lambda cannot be resolved. Depending > on how the linker is

[PATCH] D136554: Implement CWG2631

2022-12-28 Thread Jordan Rupprecht via Phabricator via cfe-commits
rupprecht added a comment. I'm not sure what to make of the new failure when I try it out this time. Given a source like this: #include struct Options { std::function identity = [](bool x) -> bool { return x; }; }; struct Wrapper { explicit Wrapper(const Options& options

[PATCH] D136554: Implement CWG2631

2022-12-28 Thread Jordan Rupprecht via Phabricator via cfe-commits
rupprecht added a comment. In D136554#4018870 , @rupprecht wrote: > All good now! The latest revision of this patch doesn't seem to break > anything, unless I ran our tests wrong. From my perspective this is OK to > reland now. ... and yep, I was

[PATCH] D136554: Implement CWG2631

2022-12-28 Thread Jordan Rupprecht via Phabricator via cfe-commits
rupprecht added a comment. All good now! The latest revision of this patch doesn't seem to break anything, unless I ran our tests wrong. From my perspective this is OK to reland now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136554/new/

[PATCH] D136554: Implement CWG2631

2022-12-23 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 485067. cor3ntin added a comment. I hope we will get there... It further reduces to consteval void immediate(){}; struct h { int g = 0; int blah = (immediate(), g); }; struct k { h j{}; }_; The issue was that nested expressions are

[PATCH] D136554: Implement CWG2631

2022-12-22 Thread Jordan Rupprecht via Phabricator via cfe-commits
rupprecht added a comment. In D136554#4013463 , @rupprecht wrote: > Glad the test case made sense to you, it was convoluted to me :) > > Still seeing one more error, and it's not modules-related so I might be able > to get it reduced today. Generally,

[PATCH] D136554: Implement CWG2631

2022-12-22 Thread Jordan Rupprecht via Phabricator via cfe-commits
rupprecht added a comment. Glad the test case made sense to you, it was convoluted to me :) Still seeing one more error, and it's not modules-related so I might be able to get it reduced today. Generally, it looks like this: struct Inner { Foo& foo; const std::unique_ptr<...> x =

[PATCH] D136554: Implement CWG2631

2022-12-22 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 484802. cor3ntin added a comment. Thanks @rupprecht As all good bugs, this was very confusing up until the moment it became obvious. I did not make the initializer a full expression during parsing, (but on use), as I thought that was unecessary. But then

[PATCH] D136554: Implement CWG2631

2022-12-20 Thread Jordan Rupprecht via Phabricator via cfe-commits
rupprecht added a comment. Sorry for the delay, I was out on vacation for a bit. I have a repro for this new issue now: F25778542: repro.tar.gz $ CLANG=~/dev/clang ./repro.sh ++ dirname /tmp/repro/repro.sh + DIR=/tmp/repro + : /tmp/D136554 + rm -rf

[PATCH] D136554: Implement CWG2631

2022-12-16 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. In D136554#4000363 , @rupprecht wrote: > I applied this version of the patch and the crash is now gone  > > However, now I get this inexplicable error -- I'm not entirely sure it's > related, maybe I'm holding it wrong: > >

[PATCH] D136554: Implement CWG2631

2022-12-15 Thread Jordan Rupprecht via Phabricator via cfe-commits
rupprecht added a comment. I applied this version of the patch and the crash is now gone  However, now I get this inexplicable error -- I'm not entirely sure it's related, maybe I'm holding it wrong: In module '': foo.h$line:$num: error: 'foo::FooClass' has different definitions in

[PATCH] D136554: Implement CWG2631

2022-12-15 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 483133. cor3ntin added a comment. Properly transform the this pointer in member initializers that need to be rewritten. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136554/new/

[PATCH] D136554: Implement CWG2631

2022-12-15 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. In D136554#3996643 , @rupprecht wrote: > Here's a well-formed reproducer: Thanks a lot. We were transforming the initializer in a context where the this pointer was not set up properly, so if it was referring to this,

[PATCH] D136554: Implement CWG2631

2022-12-14 Thread Jordan Rupprecht via Phabricator via cfe-commits
rupprecht added a comment. Here's a well-formed reproducer: struct MyStringView { MyStringView(const char *); }; struct SourceLocation { static SourceLocation current(const char * = __builtin_FILE(), unsigned int = __builtin_LINE()); };

[PATCH] D136554: Implement CWG2631

2022-12-14 Thread Jordan Rupprecht via Phabricator via cfe-commits
rupprecht added a comment. Actually, that assertion failure is pre-existing. However, this is newly failing in a no-asserts clang, so I wonder if something about this patch is just surfacing an existing bug in clang. Anyway, I hope to have a better repro by EOD. Repository: rG LLVM Github

[PATCH] D136554: Implement CWG2631

2022-12-14 Thread Jordan Rupprecht via Phabricator via cfe-commits
rupprecht added a comment. Ugh, I left cvise running overnight and forgot to include the validity check by building with a previous clang, so my reduction is invalid. I'm going to run it again, but here's the invalid crasher in the meantime: struct SourceLocation { static SourceLocation

[PATCH] D136554: Implement CWG2631

2022-12-14 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. In D136554#3993791 , @rupprecht wrote: > Looks like the latest reland still has some issue remaining. With asserts > enabled, I get: `assert.h assertion failed at > clang/include/clang/AST/Type.h:752 in const

[PATCH] D136554: Implement CWG2631

2022-12-13 Thread Jordan Rupprecht via Phabricator via cfe-commits
rupprecht added a comment. Looks like the latest reland still has some issue remaining. With asserts enabled, I get: `assert.h assertion failed at clang/include/clang/AST/Type.h:752 in const ExtQualsTypeCommonBase *clang::QualType::getCommonPtr() const: !isNull() && "Cannot retrieve a NULL

[PATCH] D136554: Implement CWG2631

2022-12-13 Thread Corentin Jabot 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 rGf1f1b60c7ba6: Implement CWG2631 (authored by cor3ntin). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D136554: Implement CWG2631

2022-12-13 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 482377. cor3ntin added a comment. remove llvm::None Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136554/new/ https://reviews.llvm.org/D136554 Files: clang/docs/ReleaseNotes.rst

[PATCH] D136554: Implement CWG2631

2022-12-12 Thread Dan McGregor via Phabricator via cfe-commits
dankm added inline comments. Comment at: clang/include/clang/Sema/Sema.h:9688 +} +return llvm::None; + } llvm::None has been deprecated in favour of std::nullopt. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D136554: Implement CWG2631

2022-12-12 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment. I'm fine with relanding and seeing if anything else pops up Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136554/new/ https://reviews.llvm.org/D136554 ___ cfe-commits mailing

[PATCH] D136554: Implement CWG2631

2022-12-12 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. @aeubanks I just completed a compilation of all of chrome including tests. let me know if you want to run more tests on your side and thanks again for reporting these issues. I think we are getting there. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D136554: Implement CWG2631

2022-12-12 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 482051. cor3ntin added a comment. When a CXXDefaultInitExpr calls a Constructor, make sure that the fields default initialized by that constructor are also marked odr used. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D136554: Implement CWG2631

2022-12-11 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. In D136554#3987122 , @aeubanks wrote: > no, I got > > $ ninja -C out/MyClang/ content_unittests > ld.lld: error: undefined symbol: > mojo::Receiver

[PATCH] D136554: Implement CWG2631

2022-12-11 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment. no, I got $ ninja -C out/MyClang/ content_unittests ld.lld: error: undefined symbol: mojo::Receiver>::Receiver(content::mojom::TestInterfaceForDefer*) >>> referenced by mojo_binder_policy_applier_unittest.cc:90

[PATCH] D136554: Implement CWG2631

2022-12-11 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. In D136554#3986779 , @cor3ntin wrote: > In D136554#3986713 , @aeubanks > wrote: > >> yes, it was chrome >> I went ahead and tried the latest patch, it successfully compiles the file >>

[PATCH] D136554: Implement CWG2631

2022-12-11 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. In D136554#3986713 , @aeubanks wrote: > yes, it was chrome > I went ahead and tried the latest patch, it successfully compiles the file > that crashed before. I built all of chrome, and now I'm getting one last > linker error,

[PATCH] D136554: Implement CWG2631

2022-12-10 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment. yes, it was chrome I went ahead and tried the latest patch, it successfully compiles the file that crashed before. I built all of chrome, and now I'm getting one last linker error, I'll try to get some more info about that Repository: rG LLVM Github Monorepo

[PATCH] D136554: Implement CWG2631

2022-12-10 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. @aeubanks I think i got it. Were you having this issue with an open source project by any chance? Maybe i could try it locally before commiting Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136554/new/

[PATCH] D136554: Implement CWG2631

2022-12-10 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 481850. cor3ntin added a comment. - Make sure cleanups expressions are created in the correct evaluation context. This simplifies and reduce the numbers of nested evaluation contexts we create, notably to make sure the creation of the initialier and

[PATCH] D136554: Implement CWG2631

2022-12-09 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. Thanks! Cleaned up version: struct string { constexpr string(const char*) {}; constexpr ~string(); }; struct AutocompleteParsingResult; struct optional { template constexpr optional(U &&) {} }; struct AutocompleteParsingResult {

[PATCH] D136554: Implement CWG2631

2022-12-09 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment. a really bad reduced repro which also has random syntax errors: using string = struct basic_string { constexpr basic_string(char *) {} constexpr ~basic_string( } struct AutocompleteParsingResult struct optional { template constexpr

[PATCH] D136554: Implement CWG2631

2022-12-09 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks reopened this revision. aeubanks added a comment. This revision is now accepted and ready to land. I've bisected another crash to this change. Reverting, and currently creducing a repro. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D136554: Implement CWG2631

2022-12-09 Thread Corentin Jabot 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 rGc9a6713b4788: Implement CWG2631 (authored by cor3ntin). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D136554: Implement CWG2631

2022-12-08 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 481411. cor3ntin added a comment. Correctly visit array filer when marking default member init as ODR-used Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136554/new/ https://reviews.llvm.org/D136554 Files:

[PATCH] D136554: Implement CWG2631

2022-12-08 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin reopened this revision. cor3ntin added a comment. This revision is now accepted and ready to land. In D136554#3982161 , @aeubanks wrote: > the following now produces a link error: > > $ cat /tmp/a.cc > #include > #include > > struct

[PATCH] D136554: Implement CWG2631

2022-12-08 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment. the following now produces a link error: $ cat /tmp/a.cc #include #include struct S { std::map a; }; using T = std::array; class C { T t{}; }; int main() { C c; } $ ./build/rel/bin/clang++ -o /dev/null

[PATCH] D136554: Implement CWG2631

2022-12-08 Thread Corentin Jabot via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGa96a6ed83230: Implement CWG2631 (authored by cor3ntin). Changed prior to commit: https://reviews.llvm.org/D136554?vs=479395=481194#toc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D136554: Implement CWG2631

2022-12-07 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. The new changes LGTM as well and the bots seem to be happy with things. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136554/new/ https://reviews.llvm.org/D136554

[PATCH] D136554: Implement CWG2631

2022-12-01 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. @aaron.ballman if you don't scream, I'll probably try again this week end. I did multiple stage 2 full builds to be sure this time. The fix is pretty small but the investigation took a while... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D136554: Implement CWG2631

2022-12-01 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 479395. cor3ntin added a comment. - Member initializers wwere not marked odr used properly - Fixing that unearthed an other bug: consteval calls in a nested member initializers were not properly detected if the member was list initialized to its default

[PATCH] D136554: Implement CWG2631

2022-11-30 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin reopened this revision. cor3ntin added a comment. This revision is now accepted and ready to land. There is still an ODR issue which caused linkers errors I think I managed to reduce the issue to template struct function_ref { function_ref(auto) {} }; struct S {

[PATCH] D136554: Implement CWG2631

2022-11-30 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. Thanks Aaron. I hope it sticks this time! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136554/new/ https://reviews.llvm.org/D136554 ___ cfe-commits mailing list

[PATCH] D136554: Implement CWG2631

2022-11-30 Thread Corentin Jabot via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. cor3ntin marked 2 inline comments as done. Closed by commit rG26fa17ed2914: Implement CWG2631 (authored by cor3ntin). Changed prior to commit:

[PATCH] D136554: Implement CWG2631

2022-11-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Comment at: clang/include/clang/Sema/Sema.h:9654-9655 +return true; + if (Ctx.isConstantEvaluated() || Ctx.isImmediateFunctionContext() || + Ctx.isUnevaluated()) +return

[PATCH] D136554: Implement CWG2631

2022-11-29 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/include/clang/Sema/Sema.h:9654-9655 +return true; + if (Ctx.isConstantEvaluated() || Ctx.isImmediateFunctionContext() || + Ctx.isUnevaluated()) +return false; cor3ntin wrote: >

[PATCH] D136554: Implement CWG2631

2022-11-28 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 478328. cor3ntin added a comment. Update DR status Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136554/new/ https://reviews.llvm.org/D136554 Files: clang/docs/ReleaseNotes.rst

[PATCH] D136554: Implement CWG2631

2022-11-28 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/include/clang/AST/ExprCXX.h:1274 + *getTrailingObjects() = RewrittenExpr; setDependence(computeDependence(this)); } aaron.ballman wrote: > Do we need to update `computeDependence()` to account for

[PATCH] D136554: Implement CWG2631

2022-11-28 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 478216. cor3ntin added a comment. Remove extra ; Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136554/new/ https://reviews.llvm.org/D136554 Files: clang/docs/ReleaseNotes.rst

[PATCH] D136554: Implement CWG2631

2022-11-28 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added reviewers: usaxena95, erichkeane, hubert.reinterpretcast. aaron.ballman added a comment. The changes generally LGTM, but I'd appreciate a second set of eyes on the changes just to double-check we're implementing the resolution properly. Comment at:

[PATCH] D136554: Implement CWG2631

2022-11-21 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 477003. cor3ntin added a comment. Properly mark default members initializers as ODR-used. default members initializers were marked as maybe odr-used but never actually odr-used. We fix that by checking that an initializer consitutes a full expression at

[PATCH] D136554: Implement CWG2631

2022-11-05 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. reduced to struct a { } constexpr b; class c { public: c(a); }; class d { d() {} c e = b; } Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136554/new/ https://reviews.llvm.org/D136554

[PATCH] D136554: Implement CWG2631

2022-11-04 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. In D136554#3909488 , @dblaikie wrote: > fwiw, @rsmith came up with a crasher reproducer from this patch here: > > template struct F { > template F(const U&) {} > }; > > struct A { > static constexpr auto x =

[PATCH] D136554: Implement CWG2631

2022-11-04 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. Looks like it's distinct from the other reported crash, this one crashes with: clang-16: /usr/local/google/home/blaikie/dev/llvm/src/clang/lib/CodeGen/CGExpr.cpp:2811: clang::CodeGen::LValue clang::CodeGen::CodeGenFunction::EmitDeclRefLValue(const

[PATCH] D136554: Implement CWG2631

2022-11-04 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added subscribers: rsmith, dblaikie. dblaikie added a comment. fwiw, @rsmith came up with a crasher reproducer from this patch here: template struct F { template F(const U&) {} }; struct A { static constexpr auto x = [] {}; F f = x; }; void f(A a = A())

[PATCH] D136554: Implement CWG2631

2022-11-04 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 473341. cor3ntin added a comment. Reopening with subsequent changes merged. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136554/new/ https://reviews.llvm.org/D136554 Files: clang/docs/ReleaseNotes.rst

[PATCH] D136554: Implement CWG2631

2022-11-04 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. @abrachet Reverted, sorry again @aaron.ballman @shafik feel free to investigate the issue, i won't have the opportunity over the next two weeks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136554/new/

[PATCH] D136554: Implement CWG2631

2022-11-04 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. Reverting now, just re-running the tests to be sure Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136554/new/ https://reviews.llvm.org/D136554 ___ cfe-commits mailing list

[PATCH] D136554: Implement CWG2631

2022-11-04 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. Fascinating. Looking more at the error it seems slightly different than what we fixed this morning. I'll revert soon when I'm at a computer. Feel free to do it earlier if it's urgent for you Sorry for the inconvenience. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D136554: Implement CWG2631

2022-11-04 Thread Alex Brachet via Phabricator via cfe-commits
abrachet added a comment. In D136554#3909198 , @cor3ntin wrote: > In D136554#3909196 , @abrachet > wrote: > >> Hi, we're hitting an assertion failure after this patch >> >>

[PATCH] D136554: Implement CWG2631

2022-11-04 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. In D136554#3909196 , @abrachet wrote: > Hi, we're hitting an assertion failure after this patch > > llvm-project/clang/lib/Sema/SemaDecl.cpp:15589: Decl > *clang::Sema::ActOnFinishFunctionBody(Decl *, Stmt *, bool): Assertion

[PATCH] D136554: Implement CWG2631

2022-11-04 Thread Alex Brachet via Phabricator via cfe-commits
abrachet added a comment. Hi, we're hitting an assertion failure after this patch llvm-project/clang/lib/Sema/SemaDecl.cpp:15589: Decl *clang::Sema::ActOnFinishFunctionBody(Decl *, Stmt *, bool): Assertion `MaybeODRUseExprs.empty() && "Leftover expressions for odr-use checking"' failed.

[PATCH] D136554: Implement CWG2631

2022-11-04 Thread Corentin Jabot 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 rG7acfe3629479: Implement CWG2631 (authored by cor3ntin). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D136554: Implement CWG2631

2022-11-04 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 473213. cor3ntin added a comment. clamg-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136554/new/ https://reviews.llvm.org/D136554 Files: clang/docs/ReleaseNotes.rst

[PATCH] D136554: Implement CWG2631

2022-11-04 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 473202. cor3ntin added a comment. Cleaner fix: Only use MaybeCreateExprWithCleanups for member inits. For default arguments, this is already handled in ConvertParamDefaultArgument. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D136554: Implement CWG2631

2022-11-04 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 473198. cor3ntin added a comment. Make sure cleanup expressions are correctly added to rewritten initializers (I'm assuming this did not manifest in C++20 mode because of copy elision) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D136554: Implement CWG2631

2022-11-04 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. In D136554#3907761 , @royjacobson wrote: > In D136554#3907661 , @cor3ntin > wrote: > >> I pushed a tentative fix for the bot failure, >> alas I'm completely unable to reproduce the

[PATCH] D136554: Implement CWG2631

2022-11-04 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added a comment. In D136554#3907661 , @cor3ntin wrote: > I pushed a tentative fix for the bot failure, > alas I'm completely unable to reproduce the issue locally, even > though it does not appear to be architecture dependant afaict. It

[PATCH] D136554: Implement CWG2631

2022-11-04 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 473177. cor3ntin added a comment. I pushed a tentative fix for the bot failure, alas I'm completely unable to reproduce the issue locally, even though it does not appear to be architecture dependant afaict. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D136554: Implement CWG2631

2022-11-04 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. The bots are unhappy, I'll investigate later https://lab.llvm.org/buildbot/#/builders/139/builds/30564 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136554/new/ https://reviews.llvm.org/D136554

[PATCH] D136554: Implement CWG2631

2022-11-04 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. As discussed with Shafik and Aaron, I'm landing that now so there is some buffer to handle unforeseen issues as i won't be available next week and the one after. Further comments are of course welcomed. If we don't find any issues we can land

[PATCH] D136554: Implement CWG2631

2022-11-04 Thread Corentin Jabot via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. cor3ntin marked an inline comment as not done. Closed by commit rGbf1e235695a7: Implement CWG2631 (authored by cor3ntin). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D136554: Implement CWG2631

2022-11-03 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin marked an inline comment as done and an inline comment as not done. cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:5918 +if (const FunctionDecl *FD = E->getDirectCallee()) + HasImmediateCalls |= FD->isConsteval(); +return

[PATCH] D136554: Implement CWG2631

2022-11-03 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin marked 8 inline comments as done. cor3ntin added inline comments. Comment at: clang/include/clang/Sema/Sema.h:9602-9604 +return Ctx.Context == + ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed || +

[PATCH] D136554: Implement CWG2631

2022-11-03 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 473006. cor3ntin added a comment. Add parentheses to make check clearer Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136554/new/ https://reviews.llvm.org/D136554 Files: clang/docs/ReleaseNotes.rst

[PATCH] D136554: Implement CWG2631

2022-11-03 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 473002. cor3ntin added a comment. - rename `/IsCheckingDefaultArgumentOrInitializer/IsCurrentlyCheckingDefaultArgumentOrInitializer` following offline discussion with Aaron - Address Shafik comments - Revert WS change - add comment explaining the

[PATCH] D136554: Implement CWG2631

2022-11-03 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. Thanks for the review Shafik, I'll address the other comments later. Comment at: clang/test/SemaCXX/cxx2a-consteval-default-params.cpp:10 +{ +return undefined(); // expected-error {{not a constant expression}} \ +

[PATCH] D136554: Implement CWG2631

2022-11-03 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added inline comments. Comment at: clang/include/clang/Sema/Sema.h:9602-9604 +return Ctx.Context == + ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed || + Ctx.IsCheckingDefaultArgumentOrInitializer; aaron.ballman wrote:

[PATCH] D136554: Implement CWG2631

2022-11-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: clang/docs/ReleaseNotes.rst:782-783 operator. -- ``clang_Cursor_getNumTemplateArguments``, ``clang_Cursor_getTemplateArgumentKind``, -

  1   2   >