-
Notifications
You must be signed in to change notification settings - Fork 239
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Emit anonymous functions as
mutable
lambdas
This example now works: main: () = { v: std::vector = ( 1, 2, 3, 4, 5 ); // Definite last use of v => move-capture v into f's closure f := :() -> forward _ = v$; // Now we can access the vector captured inside f()... f().push_back(6); for f() do(e) std::cout << e; // prints 123456 }
- Loading branch information
Showing
17 changed files
with
40 additions
and
34 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
6 changes: 3 additions & 3 deletions
6
...ion-tests/test-results/clang-12/pure2-bugfix-for-non-local-function-expression.cpp.output
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,10 @@ | ||
pure2-bugfix-for-non-local-function-expression.cpp2:5:34: error: lambda expression in an unevaluated operand | ||
template<typename T> concept v = []() -> bool { return true; }(); | ||
template<typename T> concept v = []() mutable -> bool { return true; }(); | ||
^ | ||
pure2-bugfix-for-non-local-function-expression.cpp2:7:41: error: lambda expression in an unevaluated operand | ||
using u = std::type_identity_t<decltype([]() -> void{})>; | ||
using u = std::type_identity_t<decltype([]() mutable -> void{})>; | ||
^ | ||
pure2-bugfix-for-non-local-function-expression.cpp2:9:47: error: lambda expression in an unevaluated operand | ||
class t: public std::type_identity_t<decltype([]() -> void{})> { | ||
class t: public std::type_identity_t<decltype([]() mutable -> void{})> { | ||
^ | ||
3 errors generated. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
"8B20:1424" | ||
"8B21:1401" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
4bd0c04
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't updated to use this commit, but I think it might break me.
Function expressions that capture are no longer const-callable.
Even if they don't take advantage of being
mutable
(https://cpp2.godbolt.org/z/a5KW7nM1r):Reasonable code that didn't care about (im)mutability will stop working.
Maybe the use case should be left to explicit
this
parameters (https://en.cppreference.com/w/cpp/compiler_support):4bd0c04
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For a long time, I had been wondering how you could conciliate
Cpp2
this
parameter vs. Cpp1 explicit object parameter, specially for lambdas.For lambdas, it turns out that in Cpp2,
this
is illegal.In principle, but due to #281, a reference capture is emitted.
Otherwise, the only legal way to use
this
is in a capture.That means
this
is free real state in a function expression.So we can have
:() 0;
work as before this commit,:(this) 0;
also work, and:(inout this) 0;
emit amutable
lambda.And we can use
this
to refer to the explicit object parameter, if the compiler supports it, with these macros:We only diagnose the use of
this
, not the parameter, so we can lower the above as follows::() 0;
[]() -> auto { return 0; }
[]() -> auto { return 0; }
[]() -> auto { return 0; }
:(this) 0;
[](CPP2_THIS_PARAMETER(auto const&)) -> auto { return 0; }
[](this auto const& cpp2_this) -> auto { return 0; }
[]() -> auto { return 0; }
:(this) this();
[](CPP2_THIS_PARAMETER(auto const&)) -> auto { return CPP2_THIS(); }
[](this auto const& cpp2_this) -> auto { return cpp2_this(); }
[]() -> auto { return /* error lambda */(); }
:(inout this) 0;
[](CPP2_THIS_PARAMETER(auto &)) CPP2_MUTABLE -> auto { return 0; }
[](this auto & cpp2_this) -> auto { return 0; }
[]() mutable -> auto { return 0; }
:(inout this) this();
[](CPP2_THIS_PARAMETER(auto &)) CPP2_MUTABLE -> auto { return CPP2_THIS(); }
[](this auto & cpp2_this) -> auto { return cpp2_this(); }
[]() mutable -> auto { return /* error lambda */(); }
The only thing we lose to Cpp1 explicit object parameter is the explicit naming.
4bd0c04
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And being able to specify the type of
this
.You can lift that restriction for the function expression object parameter.
4bd0c04
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an example that broke (from #797 (comment)):
4bd0c04
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additionally, we can emit function expressions without a
this
parameter and no captures asstatic
.static operator()
4bd0c04
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I always found it curious that "
this
is explicit" seems to not apply to function expressions.Where does
:() v$
sits in this regard?I'd err on the safe side of Cpp1 semantics that there really is a closure object.
So with this commit, we're making the implicit
this
parameter default toinout
,which isn't consistent with the default of
in
parameters.See also #30.
4bd0c04
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4bd0c04#commitcomment-133345681 is the exception where "
this
is explicit" needn't apply.4bd0c04
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! While working on merging #506 I noticed that branch didn't have this commit, and then I saw this thread.
I've given this thought, and I think what you want is a C++17
constexpr
lambda -- and we already have the spellingf:() == { /*body*/ }
for named functions we want to beconstexpr
, and I should support that for lambdas as well since they're the same as any function except for the name, so:() == { /*body*/ }
should work and I now think it's just an oversight that I didn't think of that case until your note.Why not
this
? Briefly: Even though Cpp1 lambdas happen to be specified in terms of a rewrite to a functor, that's not inherently the only way to think about them... my conceptual model of captures is actually closer to them being additional local variables that are "static" (persistent across calls), and local variables are non-const by default. Note also that if Cpp2 did expose that as a(this)
parameter, for consistency Cpp2 would then also need to support more things that don't currently work, such asthis.obj
syntax in the body to work as a synonym for capturedobj
.Again, thanks!
4bd0c04
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a really surprising interpretation,
since I'm all I'm talking about is the Cpp1
mutable
and the Cpp2this
parameter of function expressions.Function expression bodies are already implicitly
constexpr
thanks to the Cpp1 semantics (see also #714 (reply in thread)).4bd0c04
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[edited to expand discussion]
Yes, Cpp1 lambda functions are implicitly
constexpr
, if they happen to satisfy theconstexpr
requirements (which continue to be relaxed in each edition of the standard). As of C++23 and P2242, nearly all non-constexpr
code is allowed as long as the function isn't actually invoked in aconstexpr
context, exceptstatic
andthread_local
which is fine. For example, in C++23 aconstexpr
lambda can have astd::set
local variable, and I just checked Clang and GCC and they support that. So sayingconstexpr
shouldn't be a significant restriction on lambdas.My observation was mainly that:
constexpr
implies "no side effects, the function's output depends on the arguments supplied to its parameters only," which I think is what we'd want aconst
lambda to mean because that implies no mutation of the stored state.==
is what Cpp2 uses to lower to Cpp1constexpr
functions anyway, and it should work consistently for an unnamed function too and emitconstexpr
on the generated Cpp1 lambda... and once I fix it so==
does that, we have the "notmutable
" syntax you were looking for.So correcting the
==
inconsistency also seemed to solve the problem of not having a way to say "notmutable
" at the same time. That was my thought anyway.You mentioned above you had an example that didn't work with lowering to a
mutable
lambda. Can you confirm whether that example would work with aconstexpr
lambda? (Probably the simplest thing is to compile it with cppfront and just manually changemutable
toconstexpr
in the.cpp
and see what the Cpp1 compiler thinks?)4bd0c04
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes more sense.
That can be left to Cpp1 with 4bd0c04#commitcomment-133309652.
In fact, the only thing I use
this
for there is to call it.Because the only way to reference a capture is with
captured_expression$
.That would be any capturing lambda called through a
const
variable (https://cpp2.godbolt.org/z/jWorf3Eq1):If using
==
removes themutable
, it would naturally go back to working.4bd0c04
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, it doesn't work (https://cpp2.godbolt.org/z/EoY1n65TE):
4bd0c04
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right,
:() args$
is still shorthand for:() -> _ = args$
. All functions do default to non-constexpr.But this now works with the commit I just pushed:
Thanks again for the feedback!
4bd0c04
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MSVC also rejects: https://cpp1.godbolt.org/z/dPdczbaTj.
See also llvm/llvm-project#66262 (comment).
4bd0c04
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, MSVC doesn't support P2242 yet.
I think the right thing for now is to put a comment in the code that in the current design path we're trying out,
constexpr
is the eventual target for all==
functions including anonymous functions (with the intent that it declare 'depends only on the arguments'), but until we want to take a dependency on C++23 (P2242) we will emit it asconst
/whitespace instead.4bd0c04
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clang does, and it still fails.
P2242 doesn't lift the restriction on
constexpr
functions returning literal types.Neither does P2280, which GCC now implements.
4bd0c04
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, thanks... so for now
==
is the way to say "always the same result when invoked with the same arguments" (i.e., not mutable) but I've commented out theconstexpr
part for the future. Fixed here: c80f12aIs that better?
4bd0c04
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, that works.
Too bad about losing the terse syntax when Cpp1
mutable
won't work.4bd0c04
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this another place in the language where we are losing "constant by default"?
While I agree with Herb's mental model of captured variables being just static local variables and local variables are mutable by default so lambdas ought to be mutable as in a comment above-
But the problem with lambdas is that you can pass a local variable as an
in
parameter and it works fine but you can't do the same with mutable lambdas that captures. #868Maybe terse lambdas could be made
constexpr
and un-terse syntax could emitmutable
lambdas. This would work but seem inconsistent, other solutions are welcome.4bd0c04
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4bd0c04
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sometimes, the tail of a reply includes a copy of the whole discussion, which makes scrolling a tough job.
Maybe you can configure your email client to snip that when replying to GitHub.
4bd0c04
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've found that people often edit their comments quite significantly, and GitHub doesn't email you when that happens.