[PATCH] D55350: [WebAssembly] clang-format/clang-tidy AsmParser (NFC)

Heejin Ahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 6 16:38:02 PST 2018


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


================
Comment at: lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp:446
+      return expect(AsmToken::EndOfStatement, "EOL");
+
     } else if (DirectiveID.getString() == ".local") {
----------------
sbc100 wrote:
> aheejin wrote:
> > sbc100 wrote:
> > > Adding a newline here before a closing brace seems strange.
> > > 
> > Then in case code is structured like 
> > ```
> > if (...) {
> >   ...
> > else if (...) {
> >   ...
> > } else if (...) {
> >   ...
> > }
> > ...
> > ```
> > 
> > And body of each `if` and `else if` is long enough so I'd like to add newlines, where should I add them?
> For me that block structure it enough to break it up visually.  
> 
> To my eye adding newlines at the top of bottom of a block looks strange so I would leave it as is.
> 
> In this particular case since we return at the end of each block you could remove the else's completely and make it into a sequence of "if (xxx) { return .. }", but I'm not sure that would help with read-ability in this case.  
That sounds good. I changed them into `if`s and added newlines after. LLVM coding standards say [[ https://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return | don't use else after a return ]] anyway.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55350





More information about the llvm-commits mailing list