Page MenuHomePhabricator

[clang-format] Add options for indenting preprocessor directives
AbandonedPublic

Authored by curdeius on Mar 24 2017, 8:31 AM.

Details

Reviewers
djasper
Summary

This concerns bug #17362 (https://bugs.llvm.org//show_bug.cgi?id=17362).

2 styles for indenting are added: before and after hash.
3 methods:

  • None (default): equivalent to current behaviour of clang-format
  • All: indents all directives
  • Inner: recognizes include guards and ignores them during indentation

Indentation level uses IndentWidth style parameter.

For instance, and indented code with include guards:

#ifndef INCLUDE_GUARD
#define INCLUDE_GUARD
#define A // not indented with Inner method
#ifdef B
# undef B
#else
# define C
#endif
#endif // INCLUDE_GUARD

I've tried to make the detection of include guards as correct as possible, but if you know of any edge cases that are not taken into account, I'm all ears.

Event Timeline

curdeius created this revision.Mar 24 2017, 8:31 AM
curdeius updated this revision to Diff 92955.Mar 24 2017, 9:06 AM

Fix omitted dereference.

djasper edited edge metadata.Mar 27 2017, 10:47 PM

Thank you for working on this. Unfortunately, this is done at the wrong level:

  • You are using a separate pass, which means that as soon as the preprocessor directives exceed the column limit, they won't be wrapped correctly.
  • You aren't using Clang's Lexer to separate PP directives into tokens, which might work in the short term, but seems really fragile and a maintenance headache.

The approach to do this properly would be to:

  • Extend UnwrappedLineParser to properly report the level of preprocessor directives. My guess is that it might suffice to initialize Parser.Line->Level with PPBranchLevel in the constructor of ScopedLineState.
  • Once the level is reported correctly, UnwrappedLineFormatter's formatFirstToken function can be changed to add the spaces required for that level either before or after the "#" for preprocessor directives.

Happy to help more if you are up for taking this on.

Thanks Daniel.

I'll give it a try using the proposed approach and update you soon(ish).

curdeius abandoned this revision.Sep 7 2017, 7:33 AM

Superseded by rL312125.