Aaron Ballman via llvm-dev
2021-Mar-02 18:00 UTC
[llvm-dev] False Negatives Wunused-function in clang found by gcc
On Tue, Mar 2, 2021 at 12:03 PM David Blaikie <dblaikie at gmail.com> wrote:> > Any reason it shouldn't be a subgroup of -Wunused-function? I guess maybe because people already had codebases that were -Wunused-function clean, and we wouldn't want to regress that/start warning on new things/force them to have to add -Wno-unused-member-function to unbreak their builds? > > (I guess there's some history here? Like perhaps -Wunused-function was initially implemented without member function support, and it was added later & so added under a separate flag to make rollout easier? But I think it'd probably still be OK for it to be under a subgroup - people could opt out of it initially without losing all their existing -Wunused-function-cleanliness, and opt in if they ever decided to do the extra cleanup)Here is the commit that introduced the feature (under a different flag that was later renamed to -Wunused-member-function): https://github.com/llvm/llvm-project/commit/cad715fb9bfd50370fa22ca62634048dcc85598b but I was not able to locate the discussion about the patch from the list archives. It looks like the big reason it's not under -Wunused is because the LLVM codebase isn't ready to handle it, but given that 11 years have passed, I feel like it would be reasonable to revisit the warning groups. Logically, I would expect -Wunused-function to include -Wunused-member-function and I would expect -Wunused to include them both. If LLVM is not able to handle that change, we can alter our CMake files. ~Aaron> > On Tue, Mar 2, 2021 at 6:05 AM Aaron Ballman <aaron at aaronballman.com> wrote: >> >> On Mon, Mar 1, 2021 at 8:22 PM David Blaikie <dblaikie at gmail.com> wrote: >> > >> > Not sure where the regression might've leaked in. But does seem to be something to do with anonymous namespace support and member functions: https://godbolt.org/z/zz8WP1 >> >> I don't think there's a regression here, unless I'm missing something. >> -Wunused-member-function is what diagnoses this >> (https://godbolt.org/z/MxP4h6) but it is not a subgroup of either >> -Wunused-function or -Wunused >> (https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/DiagnosticGroups.td#L839). >> Perhaps we should explicitly enable it? >> >> ~Aaron >> >> > >> > >> > >> > On Mon, Mar 1, 2021 at 5:12 PM Luke Benes via llvm-dev <llvm-dev at lists.llvm.org> wrote: >> >> >> >> >> >> By building llvm-project with gcc 7 and 10, I found multiple Wunused-function warnings that clang 11/12 missed. >> >> >> >> All of these warnings were recently introduced, and the authors fixed many of them. Any ideas why clang misses so many of this category? And why the ones missed are recent changes to the llvm codebase? >> >> >> >> -Luke >> >> >> >> >> >> =========>> >> >> >> After dab953c8e44a2 with gcc 10.2.1, I see the following new warnings: >> >> >> >> [ 65%] Building CXX object lib/Linker/CMakeFiles/LLVMLinker.dir/IRMover.cpp.o >> >> /llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:1600:1: warning: ‘std::pair<llvm::Value*, llvm::Value*> {anonymous}::DataFlowSanitizer::getShadowOriginAddress(llvm::Value*, llvm::Align, llvm::Instruction*)’ defined but not used [-Wunused-function] >> >> 1600 | DataFlowSanitizer::getShadowOriginAddress(Value *Addr, Align InstAlignment, >> >> | ^~~~~~~~~~~~~~~~~ >> >> /llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:1510:6: warning: ‘void {anonymous}::DFSanFunction::setOrigin(llvm::Instruction*, llvm::Value*)’ defined but not used [-Wunused-function] >> >> 1510 | void DFSanFunction::setOrigin(Instruction *I, Value *Origin) { >> >> | ^~~~~~~~~~~~~ >> >> /llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:1476:8: warning: ‘llvm::Value* {anonymous}::DFSanFunction::getOrigin(llvm::Value*)’ defined but not used [-Wunused-function] >> >> 1476 | Value *DFSanFunction::getOrigin(Value *V) { >> >> | ^~~~~~~~~~~~~ >> >> /llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:1469:8: warning: ‘llvm::Value* {anonymous}::DFSanFunction::getRetvalOriginTLS()’ defined but not used [-Wunused-function] >> >> 1469 | Value *DFSanFunction::getRetvalOriginTLS() { return DFS.RetvalOriginTLS; } >> >> | ^~~~~~~~~~~~~ >> >> >> >> FIX: https://github.com/llvm/llvm-project/commit/9524632fa2bf8c894e4aac27f03631a1915d84d3 >> >> >> >> =========>> >> >> >> After bc8e262afe83, with gcc 10.1 but not clang 11, I'm seeing: >> >> >> >> [ 21%] Building CXX object lib/MC/MCParser/CMakeFiles/LLVMMCParser.dir/MasmParser.cpp.o >> >> /llvm/lib/MC/MCParser/MasmParser.cpp:3836:6: warning: ‘bool {anonymous}::MasmParser::emitStructValue(const {anonymous}::StructInfo&)’ defined but not used [-Wunused-function] >> >> 3836 | bool MasmParser::emitStructValue(const StructInfo &Structure) { >> >> | ^~~~~~~~~~ >> >> >> >> FIX: https://reviews.llvm.org/D83898 >> >> >> >> =========>> >> >> >> In 931a68f26b9a3, I'm seeing the following warning with gcc7 and gcc10: >> >> [ 70%] Building CXX object lib/CodeGen/CMakeFiles/LLVMCodeGen.dir/RegAllocPBQP.cpp.o >> >> /llvm/lib/CodeGen/RegAllocFast.cpp:384:6: warning: ‘bool {anonymous}::RegAllocFast::verifyRegStateMapping(const {anonymous}::RegAllocFast::LiveReg&) const’ defined but not used [-Wunused-function] >> >> bool RegAllocFast::verifyRegStateMapping(const LiveReg &LR) const { >> >> ^~~~~~~~~~~~ >> >> >> >> FIX: 0671a4c5087d40450603d9d26cf239f1a8b1367e >> >> >> >> _______________________________________________ >> >> LLVM Developers mailing list >> >> llvm-dev at lists.llvm.org >> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
David Blaikie via llvm-dev
2021-Mar-02 18:06 UTC
[llvm-dev] False Negatives Wunused-function in clang found by gcc
Yep, +1 to that. On Tue, Mar 2, 2021 at 10:00 AM Aaron Ballman <aaron at aaronballman.com> wrote:> On Tue, Mar 2, 2021 at 12:03 PM David Blaikie <dblaikie at gmail.com> wrote: > > > > Any reason it shouldn't be a subgroup of -Wunused-function? I guess > maybe because people already had codebases that were -Wunused-function > clean, and we wouldn't want to regress that/start warning on new > things/force them to have to add -Wno-unused-member-function to unbreak > their builds? > > > > (I guess there's some history here? Like perhaps -Wunused-function was > initially implemented without member function support, and it was added > later & so added under a separate flag to make rollout easier? But I think > it'd probably still be OK for it to be under a subgroup - people could opt > out of it initially without losing all their existing > -Wunused-function-cleanliness, and opt in if they ever decided to do the > extra cleanup) > > Here is the commit that introduced the feature (under a different flag > that was later renamed to -Wunused-member-function): > > https://github.com/llvm/llvm-project/commit/cad715fb9bfd50370fa22ca62634048dcc85598b > but I was not able to locate the discussion about the patch from the > list archives. > > It looks like the big reason it's not under -Wunused is because the > LLVM codebase isn't ready to handle it, but given that 11 years have > passed, I feel like it would be reasonable to revisit the warning > groups. Logically, I would expect -Wunused-function to include > -Wunused-member-function and I would expect -Wunused to include them > both. If LLVM is not able to handle that change, we can alter our > CMake files. > > ~Aaron > > > > > On Tue, Mar 2, 2021 at 6:05 AM Aaron Ballman <aaron at aaronballman.com> > wrote: > >> > >> On Mon, Mar 1, 2021 at 8:22 PM David Blaikie <dblaikie at gmail.com> > wrote: > >> > > >> > Not sure where the regression might've leaked in. But does seem to be > something to do with anonymous namespace support and member functions: > https://godbolt.org/z/zz8WP1 > >> > >> I don't think there's a regression here, unless I'm missing something. > >> -Wunused-member-function is what diagnoses this > >> (https://godbolt.org/z/MxP4h6) but it is not a subgroup of either > >> -Wunused-function or -Wunused > >> ( > https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/DiagnosticGroups.td#L839 > ). > >> Perhaps we should explicitly enable it? > >> > >> ~Aaron > >> > >> > > >> > > >> > > >> > On Mon, Mar 1, 2021 at 5:12 PM Luke Benes via llvm-dev < > llvm-dev at lists.llvm.org> wrote: > >> >> > >> >> > >> >> By building llvm-project with gcc 7 and 10, I found multiple > Wunused-function warnings that clang 11/12 missed. > >> >> > >> >> All of these warnings were recently introduced, and the authors > fixed many of them. Any ideas why clang misses so many of this category? > And why the ones missed are recent changes to the llvm codebase? > >> >> > >> >> -Luke > >> >> > >> >> > >> >> =========> >> >> > >> >> After dab953c8e44a2 with gcc 10.2.1, I see the following new > warnings: > >> >> > >> >> [ 65%] Building CXX object > lib/Linker/CMakeFiles/LLVMLinker.dir/IRMover.cpp.o > >> >> /llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:1600:1: > warning: ‘std::pair<llvm::Value*, llvm::Value*> > {anonymous}::DataFlowSanitizer::getShadowOriginAddress(llvm::Value*, > llvm::Align, llvm::Instruction*)’ defined but not used [-Wunused-function] > >> >> 1600 | DataFlowSanitizer::getShadowOriginAddress(Value *Addr, Align > InstAlignment, > >> >> | ^~~~~~~~~~~~~~~~~ > >> >> /llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:1510:6: > warning: ‘void {anonymous}::DFSanFunction::setOrigin(llvm::Instruction*, > llvm::Value*)’ defined but not used [-Wunused-function] > >> >> 1510 | void DFSanFunction::setOrigin(Instruction *I, Value *Origin) > { > >> >> | ^~~~~~~~~~~~~ > >> >> /llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:1476:8: > warning: ‘llvm::Value* {anonymous}::DFSanFunction::getOrigin(llvm::Value*)’ > defined but not used [-Wunused-function] > >> >> 1476 | Value *DFSanFunction::getOrigin(Value *V) { > >> >> | ^~~~~~~~~~~~~ > >> >> /llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:1469:8: > warning: ‘llvm::Value* {anonymous}::DFSanFunction::getRetvalOriginTLS()’ > defined but not used [-Wunused-function] > >> >> 1469 | Value *DFSanFunction::getRetvalOriginTLS() { return > DFS.RetvalOriginTLS; } > >> >> | ^~~~~~~~~~~~~ > >> >> > >> >> FIX: > https://github.com/llvm/llvm-project/commit/9524632fa2bf8c894e4aac27f03631a1915d84d3 > >> >> > >> >> =========> >> >> > >> >> After bc8e262afe83, with gcc 10.1 but not clang 11, I'm seeing: > >> >> > >> >> [ 21%] Building CXX object > lib/MC/MCParser/CMakeFiles/LLVMMCParser.dir/MasmParser.cpp.o > >> >> /llvm/lib/MC/MCParser/MasmParser.cpp:3836:6: warning: ‘bool > {anonymous}::MasmParser::emitStructValue(const {anonymous}::StructInfo&)’ > defined but not used [-Wunused-function] > >> >> 3836 | bool MasmParser::emitStructValue(const StructInfo > &Structure) { > >> >> | ^~~~~~~~~~ > >> >> > >> >> FIX: https://reviews.llvm.org/D83898 > >> >> > >> >> =========> >> >> > >> >> In 931a68f26b9a3, I'm seeing the following warning with gcc7 and > gcc10: > >> >> [ 70%] Building CXX object > lib/CodeGen/CMakeFiles/LLVMCodeGen.dir/RegAllocPBQP.cpp.o > >> >> /llvm/lib/CodeGen/RegAllocFast.cpp:384:6: warning: ‘bool > {anonymous}::RegAllocFast::verifyRegStateMapping(const > {anonymous}::RegAllocFast::LiveReg&) const’ defined but not used > [-Wunused-function] > >> >> bool RegAllocFast::verifyRegStateMapping(const LiveReg &LR) const { > >> >> ^~~~~~~~~~~~ > >> >> > >> >> FIX: 0671a4c5087d40450603d9d26cf239f1a8b1367e > >> >> > >> >> _______________________________________________ > >> >> LLVM Developers mailing list > >> >> llvm-dev at lists.llvm.org > >> >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210302/b395c9d6/attachment.html>