[PATCH] D63062: [clang-format] Added New Style Rule: BitFieldDeclarationsOnePerLine

Manikishan Ghantasala via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 15 12:23:40 PDT 2019


Manikishan marked an inline comment as done.
Manikishan added inline comments.


================
Comment at: lib/Format/TokenAnnotator.cpp:2921
+      Right.is(tok::identifier) && (Right.Next->is(tok::colon)))
+      return true;
   if (Right.NewlinesBefore > 1 && Style.MaxEmptyLinesToKeep > 0)
----------------
MyDeveloperDay wrote:
> mgorny wrote:
> > Misindent.
> This code appears 3 times (does it need to appear 3 times?), do we need some sort of 
> 
> ```
> bool isBitField(FormatToken)
> {
>     ...
> }
> ```
> 
> Should a bit field check for the existence of a number after the colon? I can't think of other C++ constructs that appear as
> 
> ```
> comma identifier colon
> ```
> 
> but given that clang-format is used for ObjC,ProtoBuf,Java,JavaScript,C# I'm pretty sure something odd is going to happen with JavaScript named parameters, to be honest I think this is going to cause the following to get reformatted 
> 
> MyFunctionCall({ xPosition: 20**, yPosition: 50,** width: 100, height: 5, drawingNow: true });
> 
> 
> ```
> MyFunctionCall({ xPosition: 20**, yPosition: 50,**
>                             width: 100, 
>                             height: 5, 
>                             drawingNow: true });
> ```
> 
> or something like that
Yes,  you are correct this patch is failing many Javascript Tests. Will refactor the implementation.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63062/new/

https://reviews.llvm.org/D63062





More information about the cfe-commits mailing list