Page MenuHomePhabricator

[clang-format] Fixup #include guard indents after parseFile()
ClosedPublic

Authored by mzeren-vmw on Jan 14 2018, 7:29 AM.

Details

Summary

When a preprocessor indent closes after the last line of normal code we do not
correctly fixup include guard indents. For example:

#ifndef HEADER_H
#define HEADER_H
#if 1
int i;
#  define A 0
#endif
#endif

incorrectly reformats to:

#ifndef HEADER_H
#define HEADER_H
#if 1
int i;
#    define A 0
#  endif
#endif

To resolve this issue we must fixup levels after parseFile(). Delaying
the fixup introduces a new state, so consolidate include guard search
state into an enum.

Diff Detail

Repository
rL LLVM

Event Timeline

mzeren-vmw created this revision.Jan 14 2018, 7:29 AM

rebase, ping.

Looks good! Just some comments for the include guard states would be helpful.

lib/Format/UnwrappedLineParser.h
259 ↗(On Diff #132320)

Please put a short comment explaining each of these states.

mzeren-vmw added inline comments.Feb 1 2018, 6:08 AM
lib/Format/UnwrappedLineParser.cpp
244 ↗(On Diff #132320)

Hm. From self review, I think this should be:

IncludeGuard = Style.IndentPPDirectives == FormatStyle::PPDIS_None ? IG_Rejected : IG_Inited;
736 ↗(On Diff #132320)

technically I could drop the braces opened on this line. Would you like me to do that?

mzeren-vmw updated this revision to Diff 132394.Feb 1 2018, 7:20 AM
  • Add comments to IncludeGuardState.
  • Fix re-initialization of IncludeGuard in UnwrappedLineParser::reset.
  • Remove unnecessary block after if.
  • Rebase
krasimir accepted this revision.Feb 5 2018, 1:36 AM

Looks good! Thank you!

lib/Format/UnwrappedLineParser.cpp
244 ↗(On Diff #132320)

Makes sense.

736 ↗(On Diff #132320)

I like the braces.

This revision is now accepted and ready to land.Feb 5 2018, 1:36 AM
This revision was automatically updated to reflect the committed changes.