This is an archive of the discontinued LLVM Phabricator instance.

[clang-format] Add PPIndentWidth option
ClosedPublic

Authored by gergap on May 27 2021, 2:01 PM.

Details

Summary

This allows to set a different indent width for preprocessor statements.

Example:

#ifdef __linux_
# define FOO
#endif

int main(void)
{
    return 0;
}

Diff Detail

Event Timeline

gergap created this revision.May 27 2021, 2:01 PM
gergap requested review of this revision.May 27 2021, 2:01 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 27 2021, 2:01 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
gergap edited reviewers, added: rsmith; removed: Richard.May 27 2021, 2:06 PM
gergap updated this revision to Diff 348453.May 28 2021, 12:20 AM

fixing test issues

HazardyKnusperkeks requested changes to this revision.May 29 2021, 8:59 AM

Love it!

But this will result in unexpected (one might say breaking) behaviour, if someone set IndentWidth to a different value than his base style and update clang-format without adding a setting for PPIndentWidth. So in my opinion it should have a different default value, which always picks up IndentWidth until PPIndentWidth is explicitly set. Either some form of optional, or for me -1 is also fine, but I know others are more reluctant to use -1.

clang/unittests/Format/FormatTest.cpp
3460

How about the other IndentPPDirectives values?

This revision now requires changes to proceed.May 29 2021, 8:59 AM
HazardyKnusperkeks added a project: Restricted Project.
HazardyKnusperkeks added inline comments.
clang/include/clang/Format/Format.h
2343

I prefer alphabetical sorting, I know there are some entries which aren't sorted.

MyDeveloperDay added inline comments.May 30 2021, 4:25 AM
clang/unittests/Format/FormatTest.cpp
3469

can you add a test

#ifdef X
   void foo() {
       ...
   }
#endif

its unclear if PPIndentWidth affects code in #ifdef or just # preprocessor instructions

gergap marked 3 inline comments as done.May 31 2021, 2:24 AM

I fixed the review findings and changed the behavior to be better backwards compatible by using PPIndentWidth=-1 by default.
This value defaults to IndentWidth now.

clang/unittests/Format/FormatTest.cpp
3469

It does not affect the code. It works in the same way as IndentWidth.
I updated the test cases now to verify this.

gergap updated this revision to Diff 348753.May 31 2021, 2:24 AM
gergap marked an inline comment as done.

fix review findings

Looks good to me, I would just change the wording a bit. Could you please also add a entry in the ReleaseNotes.rst?

clang/include/clang/Format/Format.h
2670

Not only backwards compatibility, but you can always choose to match those two.

This revision is now accepted and ready to land.May 31 2021, 5:47 AM
gergap updated this revision to Diff 348794.May 31 2021, 6:56 AM

changed wording as requested and updated release notes

@HazardyKnusperkeks could you please land this for me?

gergap updated this revision to Diff 348955.Jun 1 2021, 6:04 AM

trigger build again

clang/include/clang/Format/Format.h
2681

I've replaced this with int, because signed results in an exception from dump_format_style.py.

This revision was automatically updated to reflect the committed changes.