[PATCH] D30487: ClangFormat - Add option to break before inheritance separation operator in class declaration

Daniel Jasper via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Mar 2 08:13:44 PST 2017


djasper added inline comments.


================
Comment at: include/clang/Format/Format.h:309
+  /// inheritance.
+  bool BreakInhertianceBeforeColonAndComma;
+  
----------------
Hm. I am still not sure about this flag and it's name. Fundamentally, this is currently controlling two different things:
- Whether to wrap before or after colon and comma
- Whether or not to force wraps for multiple inheritance

That's going to get us in trouble if at some point people also want other combinations of these two things. I have to alternative suggestions:
- Add a single flag (e.g. InheritanceListWrapping) with multiple enum values describing the
  various choices.
- Don't add a flag at all, but instead break inheritance lists exactly like constructor initializers.
  I *think* that should solve all the know use cases for now, is easier to configure for users
  and we can cross the bridge of naming additional flags when we have to.


================
Comment at: lib/Format/TokenAnnotator.cpp:2400
+// Returns 'true' if there is an TT_InheritanceComma in the current token list.
+static bool hasMultipleInheritance(const FormatToken &Tok) {
+  for (const FormatToken* nextTok = Tok.Next; nextTok; nextTok = nextTok->Next)
----------------
I don't think you need this. If you set MustBreakBefore to true for the InheritanceCommas and set NoLineBreak to true when adding the InheritanceColon on the same line, then clang-format will already do the right thing.

Fundamentally, this seems to be identical to how we wrap constructor initializer lists in Mozilla style. So I think we should also implement this the same way (if not even reusing the same implementation).


Repository:
  rL LLVM

https://reviews.llvm.org/D30487





More information about the cfe-commits mailing list