[PATCH] D118921: [Format] Don't derive pointers right based on space before method ref-qualifiers

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton accepted this revision.
benhamilton added a comment.
This revision is now accepted and ready to land.

Got it, thanks.



================
Comment at: clang/lib/Format/Format.cpp:1949
           continue;
+        // Don't treat space in `void foo() &&` as evidence.
+        if (const auto *Prev = Tok->getPreviousNonComment()) {
----------------
sammccall wrote:
> benhamilton wrote:
> > Do we also need to worry about `T && foo();` style declarations?
> > 
> A space before && in that case *should* trigger right-alignment.
> 
> Are you concerned the exclusion here might fire?
No, just wanted to make sure it should trigger right-alignment (which is what this implements, of course.)



================
Comment at: clang/lib/Format/Format.cpp:1951
+        if (const auto *Prev = Tok->getPreviousNonComment()) {
+          if (Prev->is(tok::r_paren) && Prev->MatchingParen)
+            if (const auto *Func = Prev->MatchingParen->getPreviousNonComment())
----------------
sammccall wrote:
> benhamilton wrote:
> > If this is parsing a general function declaration, I think it's not safe to assume that the token immediately before the ref-spec is the close-paren.
> > 
> > It looks like `Prev` can be a few other things, including at least:
> > 
> > * `tok::kw_const`
> > * `tok::kw_volatile`
> > * `tok::kw_throw`
> > * `tok::kw_noexcept`
> > 
> > and probably C++ attribute(s) as well:
> > 
> > > noptr-declarator ( parameter-list ) cv(optional) ref(optional) except(optional) attr(optional)	
> > 
> > * https://en.cppreference.com/w/cpp/language/function
> > * https://en.cppreference.com/w/cpp/language/declarations
> > 
> > I *think* they can also be in any order, so the ref-spec could come at the very end after cv-qualifier, an expection specification, and C++ attributes.
> > 
> const/volatile: a space between const and & does indicate right alignment. (And clang-format emits a space under PAS_Right)
> 
> throw etc: these are not possible, there's no "in any order". Here's the grammar: http://eel.is/c++draft/dcl.decl.general#nt:parameters-and-qualifiers
Ah, good to know the grammar is more strict about ordering.


================
Comment at: clang/unittests/Format/FormatTest.cpp:9689-9692
+  // There's always a space between the function and its trailing qualifiers.
+  // This isn't evidence for PAS_Right (or for PAS_Left).
+  std::string Prefix = "void a() &;\n"
+                       "void b() &;\n";
----------------
Worth a test with `&&`?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118921