[PATCH] D119138: [clang-format] Further improve support for requires expressions

Arthur O'Dwyer via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 11 13:39:19 PST 2022


Quuxplusone added inline comments.


================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2798-2802
+    // This one is really tricky, since most tokens doesn't have a type yet.
+    // The & or && can be binary operators, then we have a requires
+    // expression. But they also can be r-value overload indicators, then
+    // we have a trailing requires clause. We try to detect the latter and
+    // default to the expressions.
----------------
s/doesn't/don't/
IIUC you're talking about, like, `void member() && requires (` — is that right?
It might help the reader to give an example snippet right here. (OTOH, it might be "obvious," I don't know. I'm not in the target audience for this code.)
...Ah, I see you give a snippet on line 2837 that's basically what I mean; I just first felt the need for that snippet all the way up //here//.


================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2835-2841
+      // FIXME: We need an annotation on the paren to really know if it is a
+      // function call:
+      // ... foo() && requires ...
+      // or a declaration:
+      // void foo() && requires ...
+      // there is no distinction possible right now. We go for the latter,
+      // because it's more likely to appear in code.
----------------
I think it's weird that your heuristic parses backward rather than forward. I would think that the next token //after// the `requires` keyword tells you what it is with pretty high probability:
`requires requires` — it's a clause
`requires identifier` — it's a clause
`requires {` — it's an expression
`requires (` — unclear, apply further heuristics

Or are those heuristics already present in trunk, and this PR is just dealing with the "unclear" case?


================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2882
+      if (BeforeLastParenContent->isSimpleTypeSpecifier()) {
+        // Definetly function delcaration.
+        return false;
----------------



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119138/new/

https://reviews.llvm.org/D119138



More information about the cfe-commits mailing list