This is in preparation for the -Wunused-but-set-variable warning.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Several variables I have just removed outright, including
llvm/lib/Target/X86/X86FloatingPoint.cpp: STDeadDefs
lldb/source/Plugins/ABI/PowerPC/ABISysV_ppc.cpp: fp_bytes
lldb/source/Interpreter/CommandInterpreter.cpp: actual_cmd_name_len
Someone familiar with this code should probably look at these and determine whether these variables not being used could have lead to buggy behavior.
Sure, all sounds good - if you can, please reach out to the authors of any of the semantics changing changes (the ones related to the Changed values in transformations) to see if they could add missing test coverage.
llvm/lib/Target/AArch64/GISel/AArch64PostSelectOptimize.cpp | ||
---|---|---|
183 | Might want to CC/respond to whoever wrote this to see if someone missed test coverage for this code. | |
llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp | ||
1227–1230 | Maybe in a separate patch would be good to switch this around to: if (!LoLoop.Preheader) return Changed; LoLoop.Start = SearchForStart(LoLoop.Preheader); | |
1230 | Also might be worth reaching out to authors to check that this change is intended & possibly tested. |
ABISysV_ppc.cpp feels a bit like this should be used to calculate some offset, but I don't think that's for this review to figure out what is wrong there. The other LLDB changes LGTM.
lldb/source/Interpreter/CommandInterpreter.cpp | ||
---|---|---|
3104 | Was already checked in unused and I don't see what we would need that for, LGTM. | |
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.cpp | ||
342 | This code was apparently rendered obsolete by 89870cebcb0c9996f49336a21e72b80511f37341 so LGTM |
llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp | ||
---|---|---|
1230 | Ah, yes this should be fine. The Changed value would only be used by the pipeline manager and this pass is the last transform in the Arm pipeline. |
I also heard via email from Amara Emerson that the change is fine for similar reasons.
Was already checked in unused and I don't see what we would need that for, LGTM.