Incorrect formatting for class and struct members

[saloniig]

Output of Haiku-format is converting tabs into spaces :

private:
	BString fTitle;
	BBitmap* fPrimaryIcon;
	bool fSelected;

whereas the expected output is :

private:

	BString		fTitle;
	BBitmap*	fPrimaryIcon;
	bool		fSelected;

[pulkomandy]

To be precise, the expected style from the guidelines is defined by this example (note: this does not look correct on github because they don't use 4 columns per tab character. refer to the Haiku website or a properly configured editor for the correct look):

class Foo : public Bar {
public:
								Foo(int32);
	virtual						~Foo();

	virtual void				SomeFunction(SomeType* argument);

	// indent long argument lists by a tab:
	virtual const char*		 FunctionLotsOfArguments(const char* name,
									const char* path, const char* user);

private:
			int32			   _PrivateMethod();
	static  int32			   _SomeStaticPrivateMethod();

// Redundant private: to separate the fields from the methods:
private:
	volatile int32				fMember;
		const char*				fPointerMember;
};

The basic idea is:

indented 1 tab: qualifiers (volatile, virtual, static)
indented 3 tabs: type of fields and return type of functions
indented 8 tabs: function or field name

This normally allows to keep all types nicely aligned (unless you have something complicated like a "static volatile") and generally keep all function and field names also aligned (unless you have a complicated return type for one of them)

In some cases we move the function and field names to a further tab stop (9 or 10) to keep things aligned even when there are long return types. Sometimes we move them back to the left when return types are short, but function or field names are very long.

It's possible we will need to decide on something stricter for clang-format because the current style may be hard to write code for. But at least we should make sure that we keep using tabs and try to keep things aligned.

[owenca]

This is not something that should be handled by clang-format/haiku-format. You may have to use clang-format off if you want to keep the indentations.

[pulkomandy]

We use this style everywhere in Haiku class definitions, so if we just turn clang-format off in all these places, it makes the tool a bit useless.

clang-format already does something similar with aligning declarations, why couldn't it do it for field and method definitions?

[owenca]

To keep clang-format light and make it run reasonably fast, its creators only used the lexer of clang cfe. Without the AST, clang-format relies heavily on heaustics when guessing the semantics of the tokens. I don't think Haiku's alignment requirements/preferences within class declarations can be easily handled by clang-format.

[saloniig]

The code for this issue can be found here :
saloniig/llvm-project@2892ce0

[owenca]

Can you create a PR at https://github.com/owenca/llvm-project/tree/haiku-format? Please also include the test case(s) you used. Thanks!

[nielx]

@owenca could you rename this ticket to 'Incorrect formatting for class and struct members'?

[owenca]

The code for this issue can be found here : saloniig/llvm-project@2892ce0

I've just tried it on a snippet from the example above:

Not only is the alignment off and FunctionLotsOfArguments indented with a mixture of tabs and spaces, the function arguments are also wrapped at the wrong place and indented to the wrong column.

Even worse, it crashed on a format test:

[ RUN      ] FormatTest.HandlesUTF8BOM
0  FormatTests              0x0000000102b8cab4 llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 52
1  FormatTests              0x0000000102b8c130 llvm::sys::RunSignalHandlers() + 64
2  FormatTests              0x0000000102b8d27c SignalHandler(int) + 312
3  libsystem_platform.dylib 0x0000000181fb1a24 _sigtramp + 56
4  FormatTests              0x0000000102c0fe7c clang::format::UnwrappedLineFormatter::formatHaikuIdentifier(llvm::SmallVectorImpl<clang::format::AnnotatedLine*> const&, clang::SourceManager const&, bool, int, bool, unsigned int, unsigned int, unsigned int) + 168
5  FormatTests              0x0000000102bf0f50 clang::format::(anonymous namespace)::IdentifierFormatter::analyze(clang::format::TokenAnnotator&, llvm::SmallVectorImpl<clang::format::AnnotatedLine*>&, clang::format::FormatTokenLexer&) + 1272
6  FormatTests              0x0000000102c00740 clang::format::TokenAnalyzer::process() + 780
7  FormatTests              0x0000000102bf0924 std::__1::__function::__func<clang::format::internal::reformat(clang::format::FormatStyle const&, llvm::StringRef, llvm::ArrayRef<clang::tooling::Range>, unsigned int, unsigned int, unsigned int, llvm::StringRef, clang::format::FormattingAttemptStatus*)::$_10, std::__1::allocator<clang::format::internal::reformat(clang::format::FormatStyle const&, llvm::StringRef, llvm::ArrayRef<clang::tooling::Range>, unsigned int, unsigned int, unsigned int, llvm::StringRef, clang::format::FormattingAttemptStatus*)::$_10>, std::__1::pair<clang::tooling::Replacements, unsigned int> (clang::format::Environment const&)>::operator()(clang::format::Environment const&) + 88
8  FormatTests              0x0000000102bdf0a8 clang::format::internal::reformat(clang::format::FormatStyle const&, llvm::StringRef, llvm::ArrayRef<clang::tooling::Range>, unsigned int, unsigned int, unsigned int, llvm::StringRef, clang::format::FormattingAttemptStatus*) + 1060
9  FormatTests              0x0000000102bdf504 clang::format::reformat(clang::format::FormatStyle const&, llvm::StringRef, llvm::ArrayRef<clang::tooling::Range>, llvm::StringRef, clang::format::FormattingAttemptStatus*) + 52
10 FormatTests              0x00000001029de9f8 clang::format::(anonymous namespace)::FormatTest::format(llvm::StringRef, clang::format::FormatStyle const&, clang::format::(anonymous namespace)::FormatTest::StatusCheck) + 144
11 FormatTests              0x0000000102a9eea4 clang::format::(anonymous namespace)::FormatTest_HandlesUTF8BOM_Test::TestBody() + 72
12 FormatTests              0x0000000102b944f4 testing::Test::Run() + 524
13 FormatTests              0x0000000102b95144 testing::TestInfo::Run() + 472
14 FormatTests              0x0000000102b95934 testing::TestCase::Run() + 264
15 FormatTests              0x0000000102b9c4f4 testing::internal::UnitTestImpl::RunAllTests() + 1060
16 FormatTests              0x0000000102b9c05c testing::UnitTest::Run() + 192
17 FormatTests              0x0000000102b8d660 main + 132
18 dyld                     0x0000000181c010e0 start + 2360
Bus error: 10