[clang-format] Preprocessor directives lead to exessive indentation and other issues

[Flamefire]

Given the following (reduced) .clangformat style file:

---
Language:        Cpp
AllowShortFunctionsOnASingleLine: Inline
BasedOnStyle:    'LLVM'
IndentPPDirectives: AfterHash
IndentWidth:     4
NamespaceIndentation: Inner
CompactNamespaces: True
...

I am reformatting the following C++ file:

#ifdef TRIGGER_CLANGFORMAT_15+
#    pragma suppress warning
#endif
namespace a { namespace b { namespace c {

    class Foo {
#ifdef TRIGGER_CLANGFORMAT_14
#    pragma suppress warning
#endif
        std::string bar_;

      public:
        const std::string &bar() const { return bar_; }
    };

}}} // namespace a::b::c

As you can see no changes are expected. However clang-format-15 up to 17 indent the whole class by an additional 4 spaces.
clang-format-14 does not do that but it breaks the member function into separate lines although AllowShortFunctionsOnASingleLine is set.

A bit more debugging:

• Removing IndentPPDirectives or the preprocessor directives fixes the behavior
• The value of IndentPPDirectives does not matter, i.e. Before/AfterHash both produce the bug (None is the same as removal, i.e. works)
• The bug on clang-format 15+ is triggered by the first preprocessor directive, the one on 14 by the 2nd. No effect of the first on 14 or 2nd on 15+ (i.e. that works) and the names/content seem to be irrelevant.
• The value of PPIndentWidth does not matter
• When disabling CompactNamespaces the 3 namespace declarations are put on separate lines but namespace c is already indented despite NamespaceIndentation: Inner (happens consistently on version 14-17) so might be a related issue

[llvmbot]

@llvm/issue-subscribers-clang-format

[rymiel]

So it turns out this has nothing to do with PP directives, check this out:

namespace a { namespace b { namespace c {

    class Foo {
        std::string bar_;

      public:
        const std::string &bar() const { return bar_; }
    };

}}} // namespace a::b::c

int main() {
if (true)
    return 0;
};

however

int foo() {
    return 0;
}

namespace a { namespace b { namespace c {

        class Foo {
            std::string bar_;

          public:
            const std::string &bar() const { return bar_; }
        };

}}} // namespace a::b::c

int main() {
    if (true)
        return 0;
};

It turns out all that is required is something "indentable" before the namespace.
But even then, the existence of the namespaces completely destroy all further indentation beyond the namespaces. (in the first example). This is because it sets entries to 0 in IndentForLevel, which are global to the program, not to just the scope. It also only sets that entry to 0 if something doesn't already exist, which is why an indentation before it fixes it.
It appears the CompactNamespaces has lots of flaws, that haven't been caught ever since the 6 years it was added...

[rymiel]

Patch: https://reviews.llvm.org/D144296

[Flamefire]

@rymiel Thanks for figuring this out! While at it can you check the "inner namespace indentation" issue:

namespace a {
namespace b {
    namespace c {

        class Foo {
        };

    }
}
}

That is without the compact namespaces feature

[rymiel]

That is working as intended? b is not indented since it inside of a top level namespace, but c is indented because it is within an inner namespace.

Changing that would be a feature request for a new option

[Flamefire]

That is working as intended? b is not indented since it inside of a top level namespace, but c is indented because it is within an inner namespace.

I see, this is because I have 3 levels of namespaces. I checked https://clang.llvm.org/docs/ClangFormatStyleOptions.html#namespaceindentation and it has this example:

namespace out {
int i;
namespace in {
  int i;
}
}

So I expected "Inner" to mean "InnerMost". So maybe that part of the documentation can be improved to have 3 nested namespaces as the example.