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

Sam Clegg via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 6 15:09:48 PST 2018


sbc100 added inline comments.


================
Comment at: lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp:446
+      return expect(AsmToken::EndOfStatement, "EOL");
+
     } else if (DirectiveID.getString() == ".local") {
----------------
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.  


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