[PATCH] D133659: [Clang] P1169R4: static operator()

This revision was automatically updated to reflect the committed changes. Closed by commit rG6523814c4e38: [Clang] P1169R4: static operator() (authored by royjacobson). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133659/new/ https://reviews.llvm.o

[PATCH] D133659: [Clang] P1169R4: static operator()

royjacobson added a comment. Thanks for the patient review and the discussion! @aaron.ballman @cor3ntin Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133659/new/ https://reviews.llvm.org/D133659 ___ cfe-

[PATCH] D133659: [Clang] P1169R4: static operator()

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, thank you for all the work on this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133659/new/ https://reviews.llvm.org/D1336

[PATCH] D133659: [Clang] P1169R4: static operator()

royjacobson added a comment. In D133659#3824472 , @aaron.ballman wrote: > Double-checking my understanding of the overload resolution changes: we added > a new conversion sequence, but we don't expect that conversion sequence to > cause a change in ove

[PATCH] D133659: [Clang] P1169R4: static operator()

royjacobson updated this revision to Diff 463978. royjacobson marked 7 inline comments as done. royjacobson added a comment. Address CR comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133659/new/ https://reviews.llvm.org/D133659 Files: cl

[PATCH] D133659: [Clang] P1169R4: static operator()

cor3ntin added inline comments. Comment at: clang/lib/Parse/ParseExprCXX.cpp:1265 + if (Intro.hasLambdaCapture()) +P.Diag(StaticLoc, diag::err_static_lambda_captures); +} aaron.ballman wrote: > aaron.ballman wrote: > > royjacobson wrote: > > > cor3ntin wrote

[PATCH] D133659: [Clang] P1169R4: static operator()

aaron.ballman added a comment. Double-checking my understanding of the overload resolution changes: we added a new conversion sequence, but we don't expect that conversion sequence to cause a change in overload resolution in practice? (I'm wondering if there's test coverage we should be adding

[PATCH] D133659: [Clang] P1169R4: static operator()

royjacobson updated this revision to Diff 463926. royjacobson added a comment. Rebase (main was broken) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133659/new/ https://reviews.llvm.org/D133659 Files: clang/docs/ReleaseNotes.rst clang/include

[PATCH] D133659: [Clang] P1169R4: static operator()

cor3ntin added a comment. @aaron.ballman Can you give one last pass at this? The new conversions rules is not something I feel confident blessing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133659/new/ https://reviews.llvm.org/D133659

[PATCH] D133659: [Clang] P1169R4: static operator()

royjacobson updated this revision to Diff 463920. royjacobson added a comment. Rebase and address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133659/new/ https://reviews.llvm.org/D133659 Files: clang/docs/ReleaseNotes.rst clang/inc

[PATCH] D133659: [Clang] P1169R4: static operator()

cor3ntin added a comment. I only left a few comments because I'm not familiar enough with that part of the code to be certain the changes are correct but overall it looks fine to me. Comment at: clang/include/clang/Sema/Overload.h:524-526 +/// StaticObjectArgumentConversio

[PATCH] D133659: [Clang] P1169R4: static operator()

royjacobson added a comment. GCC have implemented this yesterday , and they also apply the overload resolution change retroactively. So I've done it as well and downgraded the errors to ExtWarns. I'm not su

[PATCH] D133659: [Clang] P1169R4: static operator()

royjacobson updated this revision to Diff 463346. royjacobson added a comment. - Fix overload resolution and apply it retroactively - Allow static lambdas and static operator() as extensions - Define the feature macros in earlier versions and fix the tests Repository: rG LLVM Github Monorepo

[PATCH] D133659: [Clang] P1169R4: static operator()

royjacobson planned changes to this revision. royjacobson added a comment. I think I didn't get the overload resolution changes quite right, so this will probably change a bit. Comment at: clang/lib/Frontend/InitPreprocessor.cpp:698 Builder.defineMacro("__cpp_multidimensi

[PATCH] D133659: [Clang] P1169R4: static operator()

royjacobson updated this revision to Diff 462667. royjacobson marked 7 inline comments as done. royjacobson added a comment. Apply small suggestions by Aaron Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133659/new/ https://reviews.llvm.org/D133659

[PATCH] D133659: [Clang] P1169R4: static operator()

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1037-1041 +def err_static_mutable_lambda : Error< + "lambda cannot be both mutable and static">; +def err_static_lambda_captures : Error< + "a static lambda cannot have any captur

[PATCH] D133659: [Clang] P1169R4: static operator()

cor3ntin added inline comments. Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1037-1041 +def err_static_mutable_lambda : Error< + "lambda cannot be both mutable and static">; +def err_static_lambda_captures : Error< + "a static lambda cannot have any captures">;

[PATCH] D133659: [Clang] P1169R4: static operator()

royjacobson marked an inline comment as not done. royjacobson added inline comments. Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1037-1041 +def err_static_mutable_lambda : Error< + "lambda cannot be both mutable and static">; +def err_static_lambda_captures : E

[PATCH] D133659: [Clang] P1169R4: static operator()

royjacobson updated this revision to Diff 462438. royjacobson marked 8 inline comments as done. royjacobson added a comment. Small doc change Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133659/new/ https://reviews.llvm.org/D133659 Files: clang

[PATCH] D133659: [Clang] P1169R4: static operator()

royjacobson updated this revision to Diff 462437. royjacobson added a comment. Apply suggestions from Aaron and rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133659/new/ https://reviews.llvm.org/D133659 Files: clang/docs/ReleaseNotes.rst

[PATCH] D133659: [Clang] P1169R4: static operator()

aaron.ballman added a reviewer: hubert.reinterpretcast. aaron.ballman added a subscriber: hubert.reinterpretcast. aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1037-1041 +def err_static_mutable_lambda : Error< + "lambda cannot

[PATCH] D133659: [Clang] P1169R4: static operator()

royjacobson added inline comments. Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1037-1041 +def err_static_mutable_lambda : Error< + "lambda cannot be both mutable and static">; +def err_static_lambda_captures : Error< + "a static lambda cannot have any captures

[PATCH] D133659: [Clang] P1169R4: static operator()

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1037-1041 +def err_static_mutable_lambda : Error< + "lambda cannot be both mutable and static">; +def err_static_lambda_captures : Error< + "a static lambda cannot have any captur

[PATCH] D133659: [Clang] P1169R4: static operator()

royjacobson added inline comments. Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1037-1041 +def err_static_mutable_lambda : Error< + "lambda cannot be both mutable and static">; +def err_static_lambda_captures : Error< + "a static lambda cannot have any captures

[PATCH] D133659: [Clang] P1169R4: static operator()

aaron.ballman added reviewers: aaron.ballman, erichkeane, shafik, cor3ntin, lichray. aaron.ballman added a comment. In D133659#3808095 , @royjacobson wrote: > Thanks everyone! So if no one else has comments I'm planning to merge this > tomorrow. FWIW,

[PATCH] D133659: [Clang] P1169R4: static operator()

royjacobson added a comment. Thanks everyone! So if no one else has comments I'm planning to merge this tomorrow. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133659/new/ https://reviews.llvm.org/D133659 _

[PATCH] D133659: [Clang] P1169R4: static operator()

erichkeane added a comment. In D133659#3806315 , @royjacobson wrote: > Does anyone feel comfortable with looking at the CodeGen tests? They're > checking that we aren't passing a `this` argument. > > @erichkeane @shafik @lichray The codegen tests look

[PATCH] D133659: [Clang] P1169R4: static operator()

royjacobson added a comment. Does anyone feel comfortable with looking at the CodeGen tests? They're checking that we aren't passing a `this` argument. @erichkeane @shafik @lichray Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133659/new/ https:/

[PATCH] D133659: [Clang] P1169R4: static operator()

royjacobson updated this revision to Diff 461026. royjacobson added a comment. Added tests for 'this' lambda captures and for the pre-cxx2b compatability warnings. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133659/new/ https://reviews.llvm.org/

[PATCH] D133659: [Clang] P1169R4: static operator()

cor3ntin added a subscriber: erichkeane. cor3ntin added a comment. Beside a missing test, this LGTM I'd like someone else (@erichkeane maybe) to review the codegen tests. Comment at: clang/test/Parser/cxx2b-lambdas.cpp:60 + auto SC5 = [&y = x]() static {}; // expected-error {{

[PATCH] D133659: [Clang] P1169R4: static operator()

lichray added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133659/new/ https://reviews.llvm.org/D133659 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi

[PATCH] D133659: [Clang] P1169R4: static operator()

royjacobson updated this revision to Diff 460155. royjacobson added a comment. Add tests required by reviewerd. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133659/new/ https://reviews.llvm.org/D133659 Files: clang/docs/ReleaseNotes.rst clang

[PATCH] D133659: [Clang] P1169R4: static operator()

shafik added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:15934 + // or a reference to an enumeration. + // Note: Before C++23, a member function also has to not be static. if (CXXMethodDecl *MethodDecl = dyn_cast(FnDecl)) { =

[PATCH] D133659: [Clang] P1169R4: static operator()

lichray added inline comments. Comment at: clang/test/CodeGenCXX/cxx2b-static-call-operator.cpp:5 +struct Functor { + static int operator()(int x, int y) { +return x + y; I want to see some tests that diagnose `extern operator()`. Comment

[PATCH] D133659: [Clang] P1169R4: static operator()

royjacobson updated this revision to Diff 459525. royjacobson marked an inline comment as done. royjacobson added a comment. Add note in diagnostic. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133659/new/ https://reviews.llvm.org/D133659 Files:

[PATCH] D133659: [Clang] P1169R4: static operator()

royjacobson marked 6 inline comments as done. royjacobson added inline comments. Comment at: clang/lib/Parse/ParseExprCXX.cpp:1265 + if (Intro.hasLambdaCapture()) +P.Diag(StaticLoc, diag::err_static_lambda_captures); +} cor3ntin wrote: > We might want to add

[PATCH] D133659: [Clang] P1169R4: static operator()

royjacobson updated this revision to Diff 459414. royjacobson added a comment. format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133659/new/ https://reviews.llvm.org/D133659 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/Diagn

[PATCH] D133659: [Clang] P1169R4: static operator()

royjacobson updated this revision to Diff 459413. royjacobson added a comment. Fix test after warning text change Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133659/new/ https://reviews.llvm.org/D133659 Files: clang/docs/ReleaseNotes.rst cla

[PATCH] D133659: [Clang] P1169R4: static operator()

royjacobson updated this revision to Diff 459387. royjacobson marked an inline comment as done. royjacobson added a comment. Fix conversion to function pointer, add more tests and apply Corentin's wording. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D133659: [Clang] P1169R4: static operator()

royjacobson marked 2 inline comments as done. royjacobson added a comment. Wow, thanks for the quick review! In D133659#3783008 , @cor3ntin wrote: > Thanks a lot for working on that! > > I think this is the right direction but i would like to see more te

[PATCH] D133659: [Clang] P1169R4: static operator()

cor3ntin added a comment. Thanks a lot for working on that! I think this is the right direction but i would like to see more tests. Notably: - converting a lambda with a static operator to a pointer - tests in dependant contexts - classes having both static and non-static `operator()` . especial

[PATCH] D133659: [Clang] P1169R4: static operator()

royjacobson updated this revision to Diff 459361. royjacobson edited the summary of this revision. royjacobson added a comment. Fix codegen test on windows, rebase on main. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133659/new/ https://reviews.l

[PATCH] D133659: [Clang] P1169R4: static operator()

royjacobson created this revision. Herald added a project: All. royjacobson updated this revision to Diff 459334. royjacobson added a comment. royjacobson updated this revision to Diff 459349. royjacobson updated this revision to Diff 459355. royjacobson retitled this revision from "[Clang] Impleme