owenpan added inline comments.

================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2769
 
-  if (Style.isCSharp()) {
     do {
----------------
owenpan wrote:
> curdeius wrote:
> > owenpan wrote:
> > > From 
> > > https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements,
> > >  it's unclear whether the braces should be removed when a block has a 
> > > do-while loop as its single statement. Should I put the braces back and 
> > > add handling of do-while for `RemoveBracesLLVM`?
> > It's indeed not clear, I remember having paused over this code for a moment.
> > I believe that the best course of action is to grep for both cases in the 
> > codebase and see what's more frequent (and whether there's some sort of 
> > consensus).
> Good idea. I don't think there's any consensus specifically about this but 
> will ask on discourse.llvm.org. To err on the conservative side, I will 
> manually add the braces back for now.
See D126512.


================
Comment at: clang/lib/Format/WhitespaceManager.cpp:231-239
+      if (Change.Tok->is(TT_LineComment) || !Change.IsInsideToken) {
         LastBlockComment = &Change;
-      else {
-        if ((Change.StartOfBlockComment = LastBlockComment))
+      } else {
+        if ((Change.StartOfBlockComment = LastBlockComment)) {
           Change.IndentationOffset =
               Change.StartOfTokenColumn -
               Change.StartOfBlockComment->StartOfTokenColumn;
----------------
owenpan wrote:
> curdeius wrote:
> > owenpan wrote:
> > > It would be nice to remove the braces of the `else` here. See 
> > > https://github.com/llvm/llvm-project/issues/55663.
> > +1. Would you fix it manually here then and fix it later?
> I will leave it as is and use it as a test case for later. Implementing this 
> would add more complexity to `RemoveBracesLLVM`, which is complex enough 
> already. I will look into it in a few months as I will be away for the summer.
See D127260.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126157

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to