[PATCH] D57500: [WebAssembly] clang-tidy (NFC)

Thomas Lively via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 31 11:40:54 PST 2019


tlively added inline comments.


================
Comment at: lib/Target/WebAssembly/CMakeLists.txt:48
   WebAssemblyRuntimeLibcallSignatures.cpp
-  WebAssemblySelectionDAGInfo.cpp
   WebAssemblySetP2AlignOperands.cpp
----------------
sbc100 wrote:
> What is this?
I actually put something useful in this file in https://reviews.llvm.org/D57498, so please revert this particular change.


================
Comment at: lib/Target/WebAssembly/WebAssemblyFastISel.cpp:97
+      Offset = NewOffset;
     }
     int64_t getOffset() const { return Offset; }
----------------
sbc100 wrote:
> Is this actually better?  Perhaps revert this?
FWIW, I'm not a fan to adding underscores to create new identifiers.


================
Comment at: lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:326
+  MachineBasicBlock *FalseMBB = F->CreateMachineBasicBlock(LLVMBB);
+  MachineBasicBlock *DoneMBB = F->CreateMachineBasicBlock(LLVMBB);
 
----------------
sbc100 wrote:
> Revert?  This seem unrelated.
I think clang tidy just doesn't like identifiers that are all caps with underscores.


================
Comment at: lib/Target/WebAssembly/WebAssemblyISelLowering.h:104
+  SDValue lowerAccessVectorElement(SDValue Op, SelectionDAG &DAG) const;
+  SDValue lowerShift(SDValue Op, SelectionDAG &DAG) const;
 };
----------------
sbc100 wrote:
> Perhaps lets stick with the ISel convention of calling everything "LowerXX" for now.
> 
> Better to match the convention than be locally style conformant I think
+1


================
Comment at: tools/llvm-objdump/WasmDump.cpp:51
   Result.append(FmtBuf.begin(), FmtBuf.end());
-  return std::error_code();
+  return {};
 }
----------------
This should be reverted if we revert the other similar changes.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57500





More information about the llvm-commits mailing list