Page MenuHomePhabricator

[lldb] Enable -Wmisleading-indentation
ClosedPublic

Authored by kastiglione on May 7 2021, 2:22 PM.

Details

Summary

Enable -Wmisleading-indentation to balance with the LLVM style of optional parentheses.

Diff Detail

Event Timeline

kastiglione created this revision.May 7 2021, 2:22 PM
kastiglione requested review of this revision.May 7 2021, 2:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 7 2021, 2:22 PM

might as well enable it for LLVM more generally? (though probably lldb specifically, and lots of llvm more generally, would need cleanup before this is enabled? Have you checked if lldb or other parts of llvm build cleanly with this warning enabled?)

I think some people are already building with this warning + -Werror so this should be fine (Apparently that list of people includes me and Stella as I fixed a warning that broke the build in https://reviews.llvm.org/D99694#2671667 )

@dblaikie I have checked lldb, it builds cleanly. In the downstream swift branch of lldb there were a couple issues.

I'll check llvm.

Enable for llvm.

Herald added a project: Restricted Project. · View Herald TranscriptMay 7 2021, 3:42 PM
kastiglione accepted this revision.May 7 2021, 3:42 PM
kastiglione added a reviewer: dblaikie.
This revision is now accepted and ready to land.May 7 2021, 3:42 PM
kastiglione resigned from this revision.May 7 2021, 3:43 PM
This revision now requires review to proceed.May 7 2021, 3:43 PM

Didn't mean to self-accept this, so I resigned as reviewer.

dblaikie accepted this revision.May 7 2021, 3:50 PM

Sounds good to me if - if the build (of ~everything in the monorepo) is clean - and do please keep an eye on the buildbots when this is committed as there's likely to be cleanup here and there still required.

This revision is now accepted and ready to land.May 7 2021, 3:50 PM
This revision was automatically updated to reflect the committed changes.

Will keep an eye on buildbots, thanks.

In both gcc and clang, -Wmisleading-indentation is part of -Wall as far as I can tell: https://godbolt.org/z/Msaz6Kb6T

In which build configuration does this change have an effect?

@thakis thanks for pointing that out. I had tried -Wall using the clang included with the latest version of Xcode, and that does not enable -Wmisleading-indentation. I see from the compiler explorer that recent clang versions do include misleading indentation diagnostics with -Wall. It seems worth keeping in until an Xcode release includes it with -Wall.

I remember some noise due to this option (albeit on GCC), because it stops tracking indentation when the number of lines is too big, as it is for files including TableGen'ed inc files. Not sure if there is a way to improve it.

@thakis thanks for pointing that out. I had tried -Wall using the clang included with the latest version of Xcode, and that does not enable -Wmisleading-indentation. I see from the compiler explorer that recent clang versions do include misleading indentation diagnostics with -Wall. It seems worth keeping in until an Xcode release includes it with -Wall.

Thanks for pointing this out, I didn't know Xcode's clang has a different -Wall set than normal clang. I agree it makes sense to opt in to this then.