[PATCH] D34238: clang-format: Do not binpack initialization lists

Francois Ferrand via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 15 07:30:58 PDT 2017


Typz added a comment.

This patch is probably not complete, though it works fine in all situations I could think of: nested initializers, "short" statement (properly merged), column layout is still performed when needed...

  static int types[] = {
      SourcePrivate::registerTypes(),
      registerEnum<MediaItemType>(),
      registerEnum<MediaItemFlag>(),
  };
  static int types[] = {registerTypes(),
                        registerEnum<MediaItemType>(),
                        registerEnum<MediaItemFlag>()};
  static int types[]{
      SourcePrivate::registerTypes(),
      registerEnum<MediaItemType>(),
      registerEnum<MediaItemFlag>(),
  };
  static int types[]{SourcePrivate::registerTypes(),
                     registerEnum<MediaItemType>(),
                     registerEnum<MediaItemFlag>()};
  static int types[]{0, 1, 2};
  static int types[] = {0, 1, 2};
  static int types[] = {aaaaaaaaaaa,  bbbbbbbbbb,   cccccccccccc,
                        dddddddddddd, eeeeeeeeeee,  fffffffffff,
                        gggggggggg,   hhhhhhhhhhhh, iiiiiiiiii};
  static int types[] = {
      aaaaaaaaaaa, bbbbbbbbbb, cccccccccccc, dddddddddddd, eeeeeeeeeee,
      fffffffffff, gggggggggg, hhhhhhhhhhhh, iiiiiiiiii,
  };
  std::map<int, std::string> x = {
      {0, "foo fjakfjaklf kljj"},
      {1, "bar fjakfjaklf kljj"},
      {2, "stuff fjakfjaklf kljj"},
  };

I have seen 2 problems so far, but they do not seem to be related:

- indent is performed using `continuationIndentWidth`, while I think `IndentWidth` may be more appropriate
- When force-wrapping due to comma, all the content may be merged afterwards, leading to some strange layout. In this case, I think it should either not merge the content at all, or merge the braces as well: static int types[] = { 0, 1, 2 };

but I would need some feedback:

- Would this be an interesting improvement?
- Should this come with an option, or should this be the new behavior?
- Any case in particular you think will break?
- Technically, is this the right approach, or should I introduce a new TokenType and try to determine it in TokenAnnotator? This may help factorize some code from TokenAnnotator (which adds the forced line breaks) and ContinuationIndenter (which prevents BinPacking) and possibly UnwrappedLineFormatter (to fix the issue where the items get merged but not the braces)



================
Comment at: lib/Format/ContinuationIndenter.cpp:1007
     AvoidBinPacking =
-        (Current.is(TT_ArrayInitializerLSquare) && EndsInComma) ||
-        Current.is(TT_DictLiteral) ||
+        (/*Current.is(TT_ArrayInitializerLSquare) && */EndsInComma) ||
+        IsAfterAssignment || Current.is(TT_DictLiteral) ||
----------------
I don't really understand what this "TT_ArrayInitializerLSquare" case is...


https://reviews.llvm.org/D34238





More information about the cfe-commits mailing list