[clang-format][c++2b] support removal of the space between auto and {} in P0849R8

[MyDeveloperDay]

Summary

Looks like the work of D113393: [c++2b] Implement P0849R8 auto(x) requires manual clang-formatting intervention.

Removal of the space between auto and {}

@lichray would this help?

[• Quuxplusone]

clang/unittests/Format/FormatTest.cpp

22574

Looks like a good improvement to me!
However, note that auto(), auto{}, auto({}), auto{{}} are all invalid expressions; and it would be good to test with something inside the braces anyway. Please replace with e.g.

verifyFormat("foo(auto(1))");
verifyFormat("foo(auto{1})");
verifyFormat("foo(new auto(1))");
verifyFormat("foo(new auto{1})");
verifyFormat("decltype(auto(1)) x;");
verifyFormat("decltype(auto{1}) x;");
verifyFormat("auto(x);");
verifyFormat("auto{x};");
verifyFormat("auto{x} = y;");
verifyFormat("auto(x) = y;");  // actually a declaration, but this is clearly the user's own fault
verifyFormat("integral auto(x) = y;");  // actually a declaration, but this is clearly the user's own fault
verifyFormat("auto(*p)() = f;");  // actually a declaration; TODO FIXME

[MyDeveloperDay]

Additional use cases

[• Quuxplusone]

Personally I'd remove the tests that are invalid expressions; but I don't know what's the usual practice. Ship it AFAIC!

[MyDeveloperDay]

Remove invalid testcases

[• arthur.j.odwyer]

Sorry, I meant remove the first four cases that are invalid C++, e.g.
auto{}.
Personally I'd keep the "valid but arguably misformatted" cases (which I
had suggested earlier but you just removed).
How that's clearer now.

[MyDeveloperDay]

Sorry my mistake, the 1st 4 I want to keep because they related to the unit tests for D113393: [c++2b] Implement P0849R8 auto(x)

foo(auto());   // expected-error {{initializer for functional-style cast to 'auto' is empty}}
foo(auto{});   // expected-error {{initializer for functional-style cast to 'auto' is empty}}
foo(auto({})); // expected-error {{cannot deduce actual type for 'auto' from parenthesized initializer list}}
foo(auto{{}}); // expected-error {{cannot deduce actual type for 'auto' from nested initializer list}}

[MyDeveloperDay]

Revert the last change but comment that the first 4 tests are from the unit tests of D113393 which are expected invalid (I wouldn't want the unit tests from formatting incorrectly)

[HazardyKnusperkeks]

Looks good. :)

In D113826#3129229, @Quuxplusone wrote:

Personally I'd remove the tests that are invalid expressions; but I don't know what's the usual practice. Ship it AFAIC!

It's nice if even invalid code gets formatted, because some of us have formatting on typing, and the then invalid code may become valid very quickly and it would be nice if the code does not jump around. :)

[lichray]

Thanks for the patch! It works for me, both for the auto{x} and new auto{x} cases. Some corner cases left:

void f() { T(&a)->n = 1; }
void f() { T{&a}->n = 1; }     // xxx
void g() { auto(&a)->n = 0; }
void h() { auto{&a}->n = 0; }  // xxx

The /// xxx cases are formatted as

void f() {
  T { &a } -> n = 1;
}

and

void h() {
  auto{ &a } -> n = 0;
}

But this may not worth a fix (for now).

[• Quuxplusone]

In D113826#3129383, @lichray wrote:

void f() {
  T { &a } -> n = 1;
}

and

void h() {
  auto{ &a } -> n = 0;
}

But this may not worth a fix (for now).

Kibitzing: I don't think those cases matter to real code. But that spacing is so bizarre that it might be worth looking into anyway. Like, what does clang-format think auto{&n} even is? I can't think of any construct at all in C++ where we'd want to format it as T { anything or auto{ anything. I wonder if this means clang-format would misformat T{1, 2} as T { 1, 2 }! Basically the only reason to leave a space in the middle of identifier { is if you're concerned about cutesy macros like IF_CONSTEVAL {, but in those cases you should be looking for there to be a newline after the { anyway.

[HazardyKnusperkeks]

In D113826#3129383, @lichray wrote:

Thanks for the patch! It works for me, both for the auto{x} and new auto{x} cases. Some corner cases left:

void f() { T(&a)->n = 1; }
void f() { T{&a}->n = 1; }     // xxx
void g() { auto(&a)->n = 0; }
void h() { auto{&a}->n = 0; }  // xxx

The /// xxx cases are formatted as

void f() {
  T { &a } -> n = 1;
}

and

void h() {
  auto{ &a } -> n = 0;
}

But this may not worth a fix (for now).

I'm sure someone will look into it, but this should not be handled by this change.

[MyDeveloperDay]

minor comment and unit test change before committing

clang/lib/Format/TokenAnnotator.cpp

if (Left.is(tok::l_paren) && Left.Previous &&
         isKeywordWithCondition(*Left.Previous))
       return true;
     if (Right.is(tok::r_paren) && Right.MatchingParen &&
         Right.MatchingParen->Previous &&
         isKeywordWithCondition(*Right.MatchingParen->Previous))
       return true;
   }
 
+  // auto{x}
+  if (Left.is(tok::kw_auto) && Right.isOneOf(tok::l_paren, tok::l_brace))
+    return false;
+
   // requires clause Concept1<T> && Concept2<T>
   if (Left.is(TT_ConstraintJunctions) && Right.is(tok::identifier))
     return true;
 
   if (Left.is(tok::l_paren) || Right.is(tok::r_paren))
     return (Right.is(TT_CastRParen) ||
             (Left.MatchingParen && Left.MatchingParen->is(TT_CastRParen)))
                ? Style.SpacesInCStyleCastParentheses

clang/unittests/Format/FormatTest.cpp

constexpr StringRef Code =
       "check if everything is working.\n"
       "  const std::string SmallString = \"Hello World\";\n"
       "  // Small line comment\n"
       "  return String.size() > SmallString.size();\n"
       "}";
   EXPECT_EQ(Code, format(Code, Style));
 }
 
+TEST_F(FormatTest, FormatDecayCopy) {
+  // error cases from unit tests
+  verifyFormat("foo(auto())");
+  verifyFormat("foo(auto{})");
+  verifyFormat("foo(auto({}))");
+  verifyFormat("foo(auto{{}})");
+
+  verifyFormat("foo(auto(1))");
+  verifyFormat("foo(auto{1})");
+  verifyFormat("foo(new auto(1))");
+  verifyFormat("foo(new auto{1})");
+  verifyFormat("decltype(auto(1)) x;");
+  verifyFormat("decltype(auto{1}) x;");
+  verifyFormat("auto(x);");
+  verifyFormat("auto{x};");
+  verifyFormat("auto{x} = y;");
+  verifyFormat("auto(x) = y;"); // actually a declaration, but this is clearly
+                                // the user's own fault
+  verifyFormat("integral auto(x) = y;"); // actually a declaration, but this is
+                                         // clearly the user's own fault
+  verifyFormat("auto(*p)() = f;");       // actually a declaration; TODO FIXME
+}
+
 } // namespace
 } // namespace format
 } // namespace clang