This is an archive of the discontinued LLVM Phabricator instance.

Make simple requirements starting with requires ill-formed in in requirement body
ClosedPublic

Authored by cor3ntin on Jul 18 2021, 11:43 PM.

Details

Summary

This patch implements P2092

Simple requirements in requirement body shall not start with
requires.
A warning was already in place so we just turn this warning into
an error.

In addition, we add tests to make sure typename is optional
in requirement-parameter-list as per the same paper.

Diff Detail

Event Timeline

cor3ntin requested review of this revision.Jul 18 2021, 11:43 PM
cor3ntin created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptJul 18 2021, 11:43 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
cor3ntin updated this revision to Diff 359676.Jul 18 2021, 11:54 PM

Fix cxx_status

cor3ntin updated this revision to Diff 359824.Jul 19 2021, 9:36 AM

Formatting

cjdb accepted this revision.Jul 23 2021, 3:47 PM

LGTM upon inspection. There aren't any cases in libc++ that I'm aware of that will break because of this, but I think it's a good practice to try and bootstrap the library for certainty.

Please also provide a more descriptive Git commit subject (I don't know what P2092 is at a glance).

This revision is now accepted and ready to land.Jul 23 2021, 3:47 PM
cor3ntin retitled this revision from Implement P2092 to Make simple requirements starting with `requires` ill-formed in in requirement body.Jul 23 2021, 4:22 PM
cor3ntin edited the summary of this revision. (Show Details)
cor3ntin retitled this revision from Make simple requirements starting with `requires` ill-formed in in requirement body to Make simple requirements starting with requires ill-formed in in requirement body.
Quuxplusone requested changes to this revision.Jul 23 2021, 4:58 PM
Quuxplusone added a subscriber: Quuxplusone.
Quuxplusone added inline comments.
clang/include/clang/Basic/DiagnosticParseKinds.td
810

Typo here.

clang/test/Parser/cxx2a-concepts-requires-expr.cpp
148–158

s/int/T/ otherwise you miss the point of p2092's "Down with typename!" changes.

152–153

s/int/T/ otherwise you miss the point of p2092's "Down with typename!" changes.

clang/www/cxx_status.html
946–948

This should say "partial," unless you have already implemented p2092's "Down with typename!" changes.

This revision now requires changes to proceed.Jul 23 2021, 4:58 PM
cor3ntin updated this revision to Diff 361416.Jul 24 2021, 12:55 AM
  • Fix typos
  • remove the tests about typenames (don't pass yet)
  • Mark as partial until P2092 is implemented
cor3ntin added inline comments.Jul 24 2021, 12:57 AM
clang/test/Parser/cxx2a-concepts-requires-expr.cpp
152–153

Haha, you are right, thanks.
No wonder it passed...
I've elected to remove these tests for now, until P2092 is implemented (https://reviews.llvm.org/D53847)

@Quuxplusone Are you happy with the fix? Thanks

This revision is now accepted and ready to land.Aug 2 2021, 2:16 PM
aaron.ballman closed this revision.Aug 3 2021, 4:43 AM

I've commit on your behalf in 977bdf6f44edabb857bdff9ca249aa6eccb98e96, thank you!