Page MenuHomePhabricator

[C++20] Support for lambdas in unevaluated context
ClosedPublic

Authored by cor3ntin on May 30 2021, 5:40 AM.

Details

Summary

Partially implement P0315R4.

This patch allow lambda in unevaluated context.
It does not implement temp.deduct/9.

This is a rewrite of https://reviews.llvm.org/D66481 that did
not see progress over the past 2 years.

Diff Detail

Event Timeline

cor3ntin requested review of this revision.May 30 2021, 5:40 AM
cor3ntin created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptMay 30 2021, 5:40 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
cor3ntin updated this revision to Diff 348693.May 30 2021, 5:44 AM

Code formatting

sjanel added a subscriber: sjanel.May 30 2021, 12:56 PM
cjdb requested changes to this revision.May 30 2021, 2:57 PM

Thanks for working on this, it'll be a huge win for testing in libc++, methinks!

Could we get a few more tests please? I'd like to know what happens when we put a lambda directly inside:

  • a SFINAE expression
  • a noexcept specifier
  • a sizeof operator

Similarly for lambdas inside decltype for each of the above. It'd also be good to have lambdas that aren't called as well.

This revision now requires changes to proceed.May 30 2021, 2:57 PM
cor3ntin updated this revision to Diff 348759.May 31 2021, 3:23 AM

Add more tests

cjdb accepted this revision.May 31 2021, 9:40 AM

LGTM, but I'm neither a Clang nor core expert (and I'm not a maintainer), so please wait for others' approval.

clang/test/CXX/expr/expr.prim/expr.prim.lambda/p2-unevaluated.cpp
9

Nit: please move this under line 3.

22

Nit: please move this under line 3.

This revision is now accepted and ready to land.May 31 2021, 9:40 AM
cor3ntin updated this revision to Diff 348828.May 31 2021, 9:44 AM

Cleanup tests

Thanks for working on this!

It does not implement temp.deduct/9.

Is there a reason why?

clang/lib/Sema/SemaConcept.cpp
44
clang/test/CXX/expr/expr.prim/expr.prim.lambda/p2-unevaluated.cpp
1

This test doesn't really have anything to do with [expr.prim.lambda]p2 as it appears in C++20 and later, it's more testing the words that used to live in p2 in C++17 and earlier no longer apply.

I think the functionality in the test is good, but should either be moved to the correct file based on what property is being tested or should be moved out of the CXX directory and into SemaCXX (or a combination of both). I'd recommend going through the examples presented in the wording of https://wg21.link/p0315r4 and make a test for each.

4

I'd appreciate a test that shows you can also use a lambda that is not immediately invoked.

20

I think it's important to have some tests that show two lambdas are never equivalent (http://eel.is/c++draft/temp.over.link#5.sentence-4). Given the intent specified in the standard, I think a super useful case would be:

template <typename Ty, typename Uy>
auto func() -> decltype(Ty{}.operator()(Uy{})) { return 1; }

int main() {
  auto lambda = [](auto i){ return i; };
  func<decltype(lambda), int>();
}

I *think* that will cause the lambda to be mangled as part of the trailing return type.

Other tests would be typical type equivalence tests (like using std::is_same).

26

Can you add the newline to the end of the file?

cor3ntin updated this revision to Diff 349000.Jun 1 2021, 9:58 AM

Typos, add more tests

cor3ntin updated this revision to Diff 349002.Jun 1 2021, 10:05 AM

Move tests to SemaCXX

cjdb added a comment.Jun 16 2021, 10:10 PM

When should we expect this to land?

cor3ntin marked an inline comment as done.Jun 16 2021, 10:41 PM

When should we expect this to land?

Thanks for the nudge, Chris!
Please let me know if I can help move this forward :)

aaron.ballman accepted this revision.Jun 21 2021, 9:39 AM

I think this is incremental progress that LGTM, but let's wait a few days in case @rsmith has concerns before landing.

aaron.ballman closed this revision.Jun 28 2021, 6:02 AM

Thank you for the patch! I've committed in 22aa3680eaccb9b77ca224711c4da3a354aa2d45.