For a bit of an experiment I've been trying to compile LLVM & Clang with -Weverything (disabling any errors that seem like more noise/less interesting). One warning I've recently hit a few instances of is -Wweak-vtable which is, in fact, an explicitly documented LLVM coding standard ( http://llvm.org/docs/CodingStandards.html#ll_virtual_anch ). Some instances of this have been easy to fix (see attached patch - if it looks good to anyone I'll check that in) but a particular set of them have been a little more problematic. If you take a look at CommandLine.h/CommandLine.cpp you'll see some code that basically amounts to this: header: template<class DataType> class basic_parser { virtual ~basic_parser() {} }; __extension__ extern template class basic_parser<bool>; implementation: template class basic_parser<bool>; (both lines are wrapped in a macro (Compiler.h:77-88) & are no-ops in non-GNUC compilers (where the __extension__ extern is not available)) Adding in a virtual anchor function with an out-of-line (but still template) definition (either in the header or the cpp file) does not remove the warning. So the question is - is there any way to anchor these explicit instantiations? Should the warning (& possibly even the underlying implementation/codegen) be fixed to not flag this particular case of the GNUC extension - since these vtables should be able to be anchored (with the addition of such an out of line definition - either in the header or cpp file (though in this case I don't think it should be necessary in the header - since only these explicit instantiations of basic_parser are used))? Is there a portable way to address the warning? If not, should the warning just be silent, or have a separate group/warning for this case so the actionable warning can remain while this one can be disabled? Thanks, - David -------------- next part -------------- A non-text attachment was scrubbed... Name: anchors.diff Type: text/x-diff Size: 3491 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20111129/4d91afe0/attachment.diff>
On Nov 29, 2011, at 12:26 AM, David Blaikie wrote:> For a bit of an experiment I've been trying to compile LLVM & Clang > with -Weverything (disabling any errors that seem like more noise/less > interesting). One warning I've recently hit a few instances of is > -Wweak-vtable which is, in fact, an explicitly documented LLVM coding > standard ( http://llvm.org/docs/CodingStandards.html#ll_virtual_anch > ). Some instances of this have been easy to fix (see attached patch - > if it looks good to anyone I'll check that in) but a particular set of > them have been a little more problematic.Nice, please commit your patch. I don't know about explicit instantiations though, maybe a C++ guru on the clang list knows. -Chris> > If you take a look at CommandLine.h/CommandLine.cpp you'll see some > code that basically amounts to this: > > header: > > template<class DataType> > class basic_parser { > virtual ~basic_parser() {} > }; > > __extension__ extern template class basic_parser<bool>; > > implementation: > > template class basic_parser<bool>; > > (both lines are wrapped in a macro (Compiler.h:77-88) & are no-ops in > non-GNUC compilers (where the __extension__ extern is not available)) > > Adding in a virtual anchor function with an out-of-line (but still > template) definition (either in the header or the cpp file) does not > remove the warning. > > So the question is - is there any way to anchor these explicit > instantiations? Should the warning (& possibly even the underlying > implementation/codegen) be fixed to not flag this particular case of > the GNUC extension - since these vtables should be able to be anchored > (with the addition of such an out of line definition - either in the > header or cpp file (though in this case I don't think it should be > necessary in the header - since only these explicit instantiations of > basic_parser are used))? Is there a portable way to address the > warning? If not, should the warning just be silent, or have a separate > group/warning for this case so the actionable warning can remain while > this one can be disabled? > > Thanks, > - David > <anchors.diff>_______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
On Wed, Nov 30, 2011 at 10:42 PM, Chris Lattner <clattner at apple.com> wrote:> On Nov 29, 2011, at 12:26 AM, David Blaikie wrote: >> For a bit of an experiment I've been trying to compile LLVM & Clang >> with -Weverything (disabling any errors that seem like more noise/less >> interesting). One warning I've recently hit a few instances of is >> -Wweak-vtable which is, in fact, an explicitly documented LLVM coding >> standard ( http://llvm.org/docs/CodingStandards.html#ll_virtual_anch >> ). Some instances of this have been easy to fix (see attached patch - >> if it looks good to anyone I'll check that in) but a particular set of >> them have been a little more problematic. > > Nice, please commit your patch. I don't know about explicit instantiations though, maybe a C++ guru on the clang list knows.Thanks Chris, committed as r145578. I don't suppose you'll mind some similar commits as I encounter them? (there's also some legitimate unreachable code warnings I'd be happy to fix as I find them, things like: --- a/lib/Support/CommandLine.cpp +++ b/lib/Support/CommandLine.cpp @@ -294,10 +294,7 @@ static inline bool ProvideOption(Option *Handler, StringRef ArgName, break; default: - errs() << ProgramName - << ": Bad ValueMask flag! CommandLine usage error:" - << Handler->getValueExpectedFlag() << "\n"; - llvm_unreachable(0); + llvm_unreachable("Bad ValueMask flag!"); } and --- a/lib/Support/APInt.cpp +++ b/lib/Support/APInt.cpp @@ -1440,16 +1440,14 @@ APInt APInt::sqrt() const { APInt nextSquare((x_old + 1) * (x_old +1)); if (this->ult(square)) return x_old; - else if (this->ule(nextSquare)) { + if (this->ule(nextSquare)) { APInt midpoint((nextSquare - square).udiv(two)); APInt offset(*this - square); if (offset.ult(midpoint)) return x_old; - else - return x_old + 1; - } else - llvm_unreachable("Error in APInt::sqrt computation"); - return x_old + 1; + return x_old + 1; + } + llvm_unreachable("Error in APInt::sqrt computation"); } (a return after an llvm_unreachable - muddied up with a bit of syntactic tidyup)) & I'll take the detailed discussion about -Wweak-vtables for template instantiations to cfe-dev & see what they have to say. - David
Apparently Analagous Threads
- [LLVMdev] anchoring explicit template instantiations
- [LLVMdev] anchoring explicit template instantiations
- [LLVMdev] anchoring explicit template instantiations
- [LLVMdev] anchoring explicit template instantiations
- [LLVMdev] anchoring explicit template instantiations