[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