This is an archive of the discontinued LLVM Phabricator instance.

[pstl] Indent preprocessor directives as part of the clang-format rules
ClosedPublic

Authored by ldionne on Mar 25 2019, 7:29 AM.

Details

Summary

Indenting preprocessor directives provides a significant gain in
readability. We do it for normal if statements, and it makes sense
to do it for preprocessor ifs too.

Diff Detail

Repository
rL LLVM

Event Timeline

ldionne created this revision.Mar 25 2019, 7:29 AM
ldionne changed the repository for this revision from rCXX libc++ to rPSTL pstl.Mar 25 2019, 7:30 AM

a significant gain in readability

To tell the truth, I guess it is subjectively... and rather it is a question of habit IMHO...

Or this style is accepted for libcxx and libcstd and we have to do it for making code consistent ? If yes, I have no objections.

a significant gain in readability

To tell the truth, I guess it is subjectively... and rather it is a question of habit IMHO...

Why do we indent normal if statements? This isn't true for all libraries, but a base library like the standard library tends to have so much preprocessor-level logic that things become really hard to grasp when things are not indented properly. Again, to be clear, it's not about the cuteness of indenting inside a single #if, it's about how to spot matching #endifs when deeply nested.

Or this style is accepted for libcxx and libcstd and we have to do it for making code consistent ? If yes, I have no objections.

libc++ doesn't do this consistently and this has been a source of confusion in the past, in __config where there are many nested #ifs.

I'm in favour of this.

libc++ doesn't do this consistently and this has been a source of confusion in the past, in __config where there are many nested #ifs.

Seems like something we could/should add to libcxx as well then. Might even be acceptable to do in an en masse change there.

rodgert accepted this revision.Mar 29 2019, 1:25 PM

I am also in favor of this.

@dexonsmith - libstdc++ also indents it's conditional definitions

Is this something clang-format can mechanically apply?

This revision is now accepted and ready to land.Mar 29 2019, 1:25 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptApr 1 2019, 8:21 AM