[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

2022-10-13 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. @erichkeane I was looking at Daisy meta programming shenanigans and found this crashes https://cute.godbolt.org/z/6fWoEheKc - seems related to your work, probably? It used to work in Clang 15. I thought you might want to know! Repository: rG LLVM Github Monorepo CH

[PATCH] D124351: [Clang][WIP] Implement Change scope of lambda trailing-return-type - Take 2

2022-12-18 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 483812. cor3ntin added a comment. Handle late constraints checking and fully implement P2579 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124351/new/ https://reviews.llvm.or

[PATCH] D124351: [Clang][WIP] Implement Change scope of lambda trailing-return-type - Take 2

2022-12-18 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 483813. cor3ntin added a comment. Update release notes and commit message Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124351/new/ https://reviews.llvm.org/D124351 Files: clang/docs/ReleaseNotes.rst clan

[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

2022-12-18 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 483816. cor3ntin added a comment. Remove unused variables Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124351/new/ https://reviews.llvm.org/D124351 Files: clang/docs/ReleaseNotes.rst clang/include/clang/

[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

2022-12-18 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 483817. cor3ntin added a comment. revert WS only change, removed unused `DelayedCapture` type Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124351/new/ https://reviews.llvm.org/D124351 Files: clang/docs/Rel

[PATCH] D137531: [clang] Add the check of membership in decltype for the issue #58674

2022-12-18 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. In D137531#4003723 , @lime wrote: > Rebase and ping! I think looks good, but I want to make sure @erichkeane finds his comments resolved, and he is in vacation until January. CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

2022-12-18 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 483820. cor3ntin added a comment. Remove the lookahead parsing code code which is no longer necessary Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124351/new/ https://reviews.llvm.org/D124351 Files: clang/

[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 i

[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-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 member

[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 = [] { test_body{};

[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 c

[PATCH] D140554: [C++20] Determine the dependency of unevaluated lambdas more accurately

2023-01-03 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. @shafik I'm not sure either it's the right direction but I don't think it makes the situation any worse. And given I still don't really know what a proper fix would look like (except that it would probably be pretty involved), I think we might has well do that as an im

[PATCH] D140828: [WIP][C++] Implement "Deducing this" (P0847R7)

2023-01-03 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/include/clang/AST/Expr.h:1447 + bool Set, const ASTContext &Context) { +DeclRefExprBits.CapturedByCopyInLambdaWithExplicitObjectParameter = Set; +setDependence(computeDependence(this, Context)); erich

[PATCH] D140828: [WIP][C++] Implement "Deducing this" (P0847R7)

2023-01-03 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/lib/AST/MicrosoftMangle.cpp:2560 IsInLambda = true; -if (MD->isInstance()) +if (MD->isImplicitObjectMemberFunction()) HasThisQuals = true; erichkeane wrote: > cor3ntin wrote: > > erichkeane w

[PATCH] D132877: Downgrade the UAX31 diagnostic to a warning which defaults to an error

2023-01-03 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. @aaron.ballman I don't think this patch is no longer necessary given that we merged the math profile Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132877/new/ https://reviews.llvm.org/D132877

[PATCH] D139889: [Clang] Fix a crash when encountering an ill-formed delimited UCN.

2023-01-03 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. @tahonermann gentle ping :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139889/new/ https://reviews.llvm.org/D139889 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D139889: [Clang] Fix a crash when encountering an ill-formed delimited UCN.

2023-01-03 Thread Corentin Jabot via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG0d6b26b4d3e3: [Clang] Fix a crash when encountering an ill-formed delimited UCN. (authored by cor3ntin). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139889

[PATCH] D139889: [Clang] Fix a crash when encountering an ill-formed delimited UCN.

2023-01-03 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. @tahonermann Thanks :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139889/new/ https://reviews.llvm.org/D139889 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:/

[PATCH] D140711: [clang] Fix unused variable warning in SemaConcept.cpp

2023-01-03 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. nit: I think that's a cleaner fix :) Comment at: clang/lib/Sema/SemaConcept.cpp:1348 bool &Result) { if (const auto *FD1 = dyn_cast(D1)) { auto IsExpectedEntity = [](const FunctionDecl *FD) {

[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 resu

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

2023-01-04 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. @aaron.ballman gentle ping. I hope your feedback was addressed to your satisfaction :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137244/new/ https://reviews.llvm.org/D137244 __

[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 ma

[PATCH] D140984: [clang] Correct -frewrite-includes generation of line control directives with mixed EOL forms.

2023-01-04 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. Thanks Tom I think this looks reasonable but given the subtlety a comment in the code may be useful + a release note to indicate the fixed issue Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140984/new/ https://reviews.l

[PATCH] D139586: [Clang][C++23] Lifetime extension in range-based for loops

2023-01-04 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. In D139586#4000309 , @hubert.reinterpretcast wrote: > At least on the surface, it looks like there will be a lot of trouble to deal > with default arguments: > > struct A { A(); ~A(); int x[3]; }; > int (&f(const A & = 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 DoMarkVarDec

[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 unev

[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 clang/include/clang/AST/ExprCXX.h

[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 ma

[PATCH] D139837: [Clang] Implements CTAD for aggregates P1816R0 and P2082R1

2023-01-05 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. Sorry for not getting back to you earlier. It mostly looks reasonable to me. what do you think @erichkeane ? Comment at: clang/lib/Sema/SemaInit.cpp:10338 +Context.getRValueReferenceType(ElementTypes[i]); + else if (isa

[PATCH] D139837: [Clang] Implements CTAD for aggregates P1816R0 and P2082R1

2023-01-05 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/include/clang/AST/DeclCXX.h:1953-1959 bool isCopyDeductionCandidate() const { -return FunctionDeclBits.IsCopyDeductionCandidate; +return getDeductionCandidateKind() == DeductionCandidateKind::Copy; + } + + bool isAggr

[PATCH] D139837: [Clang] Implements CTAD for aggregates P1816R0 and P2082R1

2023-01-05 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a subscriber: hubert.reinterpretcast. cor3ntin added a comment. + @hubert.reinterpretcast In case i missed something conformance wise. Speaking of conformance, your implementation does not appear to gate the feature on C++20, which it should, so you should add a test that it doesn

[PATCH] D140554: [C++20] Determine the dependency of unevaluated lambdas more accurately

2023-01-06 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin accepted this revision. cor3ntin added a comment. This revision is now accepted and ready to land. Thanks. I think this looks good! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140554/new/ https://reviews.llvm.org/D140554 ___ cfe-co

[PATCH] D139837: [Clang] Implements CTAD for aggregates P1816R0 and P2082R1

2023-01-06 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:13 +#include "clang/AST/APValue.h" #include "clang/AST/ASTContext.h" Do we actually need that? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[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&id=487151#toc Reposito

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

2023-01-09 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/lib/AST/DeclCXX.cpp:3232 +VarDecl *ValueDecl::getPotentiallyDecomposedVarDecl() { + assert((isa(this) || isa(this)) && + "expected a VarDecl or a BindingDecl"); aaron.ballman wrote: > That's future tech,

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

2023-01-09 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 487452. cor3ntin added a comment. Address Aaron's comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137244/new/ https://reviews.llvm.org/D137244 Files: clang/include/clang/AST/Decl.h clang/include/cl

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

2023-01-09 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/lib/AST/DeclCXX.cpp:3232 +VarDecl *ValueDecl::getPotentiallyDecomposedVarDecl() { + assert((isa(this) || isa(this)) && + "expected a VarDecl or a BindingDecl"); aaron.ballman wrote: > cor3ntin wrote: > >

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

2023-01-09 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 487453. cor3ntin added a comment. Rewrite assert in DeclCXX.cpp Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137244/new/ https://reviews.llvm.org/D137244 Files: clang/include/clang/AST/Decl.h clang/inclu

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

2023-01-09 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:125 +diag::err_param_default_argument_references_local) + << D->getDeclName() << DefaultArg->getSourceRange(); } aaron.ballman wrote: > aaron.ballman wro

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

2023-01-09 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 487464. cor3ntin added a comment. Symplify printing a NamedDecl in moved code per Aaron's comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137244/new/ https://reviews.llvm.org/D137244 Files: clang/incl

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

2023-01-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 rGee2056e7: [Clang] Correctly capture bindings in dependent lambdas. (authored by cor3ntin). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

2023-01-09 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 487539. cor3ntin marked 24 inline comments as done. cor3ntin added a comment. - Address Aaron's comments (except renaming ActOnLambdaIntroducer as we pounder on it) - Delete an aditional outdated comment Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D137172: [Clang] Implement CWG2358 Explicit capture of value

2022-11-01 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 472386. cor3ntin added a comment. Fix tests + address aaron's feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137172/new/ https://reviews.llvm.org/D137172 Files: clang/docs/ReleaseNotes.rst clang/l

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

2022-11-02 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin created this revision. Herald added a project: All. cor3ntin requested review of this revision. Herald added a reviewer: jdoerfert. Herald added subscribers: cfe-commits, sstefan1. Herald added a project: clang. Structured bindings were not properly marked odr-used and therefore captured

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

2022-11-02 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 472581. cor3ntin added a comment. Forgot to clang-format! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137244/new/ https://reviews.llvm.org/D137244 Files: clang/include/clang/AST/Decl.h clang/include/cla

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

2022-11-02 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. (No changelog, this fixes an unreleased feature) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137244/new/ https://reviews.llvm.org/D137244 ___ cfe-commits mailing list cfe-comm

[PATCH] D137172: [Clang] Implement CWG2358 Explicit capture of value

2022-11-02 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 472601. cor3ntin added a comment. Properly handle structured bindings in init capture Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137172/new/ https://reviews.llvm.org/D137172 Files: clang/docs/ReleaseNote

[PATCH] D137172: [Clang] Implement CWG2358 Explicit capture of value

2022-11-02 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:107-111 +if (const auto *BD = dyn_cast(Decl)) + VD = dyn_cast_if_present(BD->getDecomposedDecl()); +else + VD = dyn_cast(Decl); +if (VD) { Note that https://reviews.l

[PATCH] D137172: [Clang] Implement CWG2358 Explicit capture of value

2022-11-02 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 472637. cor3ntin added a comment. Fix tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137172/new/ https://reviews.llvm.org/D137172 Files: clang/docs/ReleaseNotes.rst clang/lib/Sema/SemaDeclCXX.cpp c

[PATCH] D129531: [clang][C++20] P0960R3: Allow initializing aggregates from a parenthesized list of values

2022-11-02 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. I think this is basically good to go, but I'll let other people give it a look too Comment at: clang/lib/AST/ExprConstant.cpp:9987-9990 +} else { + llvm_unreachable( + "Expression is neither an init list nor a C++ paren list"); +

[PATCH] D137172: [Clang] Implement CWG2358 Explicit capture of value

2022-11-02 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:107-111 +if (const auto *BD = dyn_cast(Decl)) + VD = dyn_cast_if_present(BD->getDecomposedDecl()); +else + VD = dyn_cast(Decl); +if (VD) { aaron.ballman wrote: > cor3

[PATCH] D137172: [Clang] Implement CWG2358 Explicit capture of value

2022-11-03 Thread Corentin Jabot via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG742970920b7a: [Clang] Implement CWG2358 Explicit capture of value (authored by cor3ntin). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137172/new/ https://

[PATCH] D136554: Implement CWG2631

2022-11-03 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 472912. cor3ntin added a comment. Add tests for consteval calls in init captures in default parameters. Clang behavior deviates from the wording of CWG2631 as it is not clear how we can implement the intended behavior in that corner case without introducin

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

2022-11-03 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 472920. cor3ntin added a comment. Use the `getPotentiallyDecomposedVarDecl` everywhere it makes sense. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137244/new/ https://reviews.llvm.org/D137244 Files: clang

[PATCH] D136554: Implement CWG2631

2022-11-03 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 472922. cor3ntin added a comment. Tweak comment in source_location tests with a suggestion Aaron made offline. 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-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 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 `check_la

[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 clang/

[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 || + Ctx.IsCheckingDefaultArgumentOrInitia

[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 RecursiveAS

[PATCH] D136554: Implement CWG2631

2022-11-03 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 https://reviews.llvm.org/D136554/ne

[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 https://reviews.llvm.org/D

[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 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 SIN

[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 issu

[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 http

[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 https://

[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 clang/include/clang/AST/ExprCXX

[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 https://reviews.llvm.org/D

[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 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 S

[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 cfe-c

[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/ https://reviews.llv

[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 cl

[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-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] D137531: [clang] Fix the GitHub issue #58674

2022-11-07 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. Thanks for working on this. I agree with Aaron, can you make the commit message clearer as to what is fixed, and mention in clang/docs/ReleaseNotes that the issue is fixed. Comment at: clang/lib/Sema/SemaExpr.cpp:2698-2704 + if (!SS.isEmpty()) +

[PATCH] D129531: [clang][C++20] P0960R3 and P1975R0: Allow initializing aggregates from a parenthesized list of values

2022-11-07 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. There is a C++ standard meeting this week so some folks won't be able to review it. Feel free to re-ping next week if nobody else replies. Because it's a fairly big change and I've been known to miss glaring things, I don't feel comfortable to approve it. But it looks g

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

2022-11-08 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. In D137244#3915633 , @aaron.ballman wrote: > Thanks for this! Can you also add a release note for the changes? Thanks for the Review! As mentioned this fixes an unreleased feature, hence why I don't think a release note entry

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

2022-11-08 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/lib/AST/DeclCXX.cpp:3234-3237 + if (auto *Var = llvm::dyn_cast(this)) +return Var; + if (auto *BD = llvm::dyn_cast(this)) +return llvm::dyn_cast(BD->getDecomposedDecl()); aaron.ballman wrote: > No need t

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

2022-11-08 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 474080. cor3ntin marked 4 inline comments as done. cor3ntin added a comment. Address Aaron's comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137244/new/ https://reviews.llvm.org/D137244 Files: clang/

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

2022-11-11 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 474770. cor3ntin added a comment. Address Shafik's comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137244/new/ https://reviews.llvm.org/D137244 Files: clang/include/clang/AST/Decl.h clang/include/cl

[PATCH] D142401: [Clang] Fix a crash when recursively callig a default member initializer.

2023-01-26 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. In D142401#4074959 , @shafik wrote: > I could not find any tests that test for this warning. I'm unable to trigger the warning at all. I feel i should but just using that function resolves the crash, probably because it allocat

[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

2023-01-30 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 493328. cor3ntin added a comment. - rebase - rename ActOnLambdaIntroducer - Fix a crash in `transformedLocalDecl` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124351/new/ https://reviews.llvm.org/D124351 Fil

[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

2023-01-31 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 rGd708a186b6a9: [Clang] Implement Change scope of lambda trailing-return-type (authored by cor3ntin). Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

2023-01-31 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. @aaron.ballman Thanks for the review! I hope we won't find further issue with the paper this time :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124351/new/ https://reviews.llvm.org/D124351

[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

2023-02-01 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. In D124351#4093840 , @nikic wrote: > FYI this causes a minor compile-time regression (around 0.35% on 7zip at > `O0`): > http://llvm-compile-time-tracker.com/compare.php?from=cd173cbd7cca69c29df42cd4b42e60433435c29b&to=d708a186

[PATCH] D143053: [C++20] [Modules] Pop Expression Evaluation Context when we skip its body during parsing

2023-02-01 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/lib/Parse/Parser.cpp:1415 +// +// FIXME: It looks not easy to balance PushExpressionEvaluationContext() +// and PopExpressionEvaluationContext(). shafik wrote: > It does seem a bit ad-hoc could we use

[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

2023-02-01 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. In D124351#4097224 , @aaron.ballman wrote: > In D124351#4097192 , @dblaikie > wrote: > >> In D124351#4097110 , @cor3ntin >> wrote: >> >>> In D

[PATCH] D143109: [Sema] Push a LambdaScopeInfo before calling SubstDefaultArgument

2023-02-01 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. In D143109#4097775 , @erichkeane wrote: > @cor3ntin : Mind taking a look here? You're my lambda expert these days :) I did something similar to check the requires clause of a lambda and it does technically fix the issue, i do

[PATCH] D143109: [Sema] Push a LambdaScopeInfo before calling SubstDefaultArgument

2023-02-01 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. In D143109#4097850 , @ahatanak wrote: > Doesn't `Sema::FunctionScopeRAII` pop the lambda scope when it goes out of > scope? No, sadly lambdas are handled differently > https://github.com/llvm/llvm-project/blob/main/clang/inclu

[PATCH] D143099: [clang][lex] Expose findBeginningOfLine()

2023-02-01 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin accepted this revision. cor3ntin added a comment. This revision is now accepted and ready to land. LGTM except the duplicated comment Comment at: clang/lib/Lex/Lexer.cpp:493-494 /// Returns the pointer that points to the beginning of line that contains /// the give

[PATCH] D142717: [clang] Mark CWG2165 as N/A

2023-02-01 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin accepted this revision. cor3ntin added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142717/new/ https://reviews.llvm.org/D142717 ___

[PATCH] D140614: [C++20] Mark lambdas in lambda specifiers as dependent if necessary

2023-02-02 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/test/SemaCXX/lambda-unevaluated.cpp:127 +auto foo(int t) { + int(*f)(int) = [](auto t) -> decltype([=] { return t; } ()) { return t; }; + return f; I thought unevaluated lambdas could not have captures. that mig

[PATCH] D143053: [C++20] [Modules] Pop Expression Evaluation Context when we skip its body during parsing

2023-02-02 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin accepted this revision. cor3ntin added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Parse/Parser.cpp:1415 +// +// FIXME: It looks not easy to balance PushExpressionEvaluationContext() +// and PopExpressionEvaluationC

[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

2023-02-02 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. In D124351#4101611 , @rupprecht wrote: > Hi, me again :) > > I ran into an interesting build breakage from this, I can't tell if it's a > legitimate breakage based on reading P2036R3 and P2579R0 (mostly I'm not a > language law

[PATCH] D143109: [Sema] Push a LambdaScopeInfo before calling SubstDefaultArgument

2023-02-04 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. My question is, why do we need to mess up with scopes in that way outside of parsing (there are only a couple places where we do that at the moment, and they are dummy scopes which only exist to balance some push and pop, afaict they serve no other purpose). As i said

[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

2023-01-13 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/lib/Parse/ParseExprCXX.cpp:1293 Actions.PushLambdaScope(); + Actions.ActOnLambdaIntroducer(Intro, getCurScope()); aaron.ballman wrote: > Typically, we call an `ActOn` method after having parsed the construct

[PATCH] D142401: [Clang] Fix a crash when recursively callig a default member initializer.

2023-01-23 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin created this revision. Herald added a project: All. cor3ntin requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This fixes a regression introduced by ca61961380 , t

<    2   3   4   5   6   7   8   9   10   11   >