[PATCH] D94661: [clang-format] [PR19056] Add support for access modifiers indentation

[Jakub Budiský via Phabricator via cfe-commits]

Budovi added a comment.

In D94661#2497744 <https://reviews.llvm.org/D94661#2497744>, @HazardyKnusperkeks wrote:

> I would add a test where you have a member before the first access modifier.

The very first unit test has no modifiers, the third one has a member before a modifier in the nested class C.

In D94661#2497744 <https://reviews.llvm.org/D94661#2497744>, @HazardyKnusperkeks wrote:

> Also this is not exactly what they wanted in the bug, as far as I can see members of structs or classes with no access modifier at all should only be indented once.

I don't think there is a consensus about how this should actually work, see e. g. https://bugs.llvm.org/show_bug.cgi?id=19056#c11 .

I see qtcreator's style a bit more relevant than some astyle configuration of a specific project. I also find it personally ugly, can't use it myself and thus would have to make it configurable. In that case, this change could be viewed as a "possible stepping stone", even though it would probably require a major overhaul of the logic.

The previous patch, if I read it correctly, implements this detail in the same way as I did.



================
Comment at: clang/unittests/Format/FormatTest.cpp:17882-17908
+  {
+    const char *Expected = "struct S {\n"
+                           "  private:\n"
+                           "    class C {\n"
+                           "        int j;\n"
+                           "\n"
+                           "      public:\n"
----------------
When you re-create this test using `verifyFormat`, you get a weird results, i.e. if you keep the newlines before access modifiers, the test fails because it thinks they should not be there, but if you remove them, test fails again, because the formatter includes them instead.

It's a good question whether it is a side-effect of `test::messUp` or a different bug, I'm not sure.


================
Comment at: clang/unittests/Format/FormatTest.cpp:17914-17918
+  verifyFormat("enum class E {\n"
+               "  A,\n"
+               "  B\n"
+               "};\n",
+               Style);
----------------
This unit test currently fails, I believe it is a non-related bug in the formatter. Nevertheless I wanted to add a check that enums (non-records) are preserved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94661