Is it correct to ignore warning infos when compiling TVM?

Development
1

Hi experts,

I met too many compile warning infos when integrating TVM into my project. I find TVM compiling doesn’t open -Wextra switch. If I open it, it also generates many warning infos. Such as:

/codes/tvm/include/tvm/operation.h:56:7: warning: base class ‘class tvm::ir::FunctionBaseNode’ has accessible non-virtual destructor [-Wnon-virtual-dtor]
 class OperationNode : public ir::FunctionBaseNode {

/codes/tvm/include/tvm/runtime/object.h:650:10: warning: type qualifiers ignored on function return type [-Wignored-qualifiers]
   static const uint32_t _GetOrAllocRuntimeTypeIndex()  {

/codes/tvm/src/runtime/module.cc:99:48: warning: unused parameter ‘file_name’ [-Wunused-parameter]
 void ModuleNode::SaveToFile(const std::string& file_name,

As I know, some of warnings are suggested to be fixed, such as -Wnon-virtual-dtor. It may generate errors when integrating TVM into other project, which has strict CMAKE_CXX_FLAGS.

So should we fix these warnings and open as many as warning infos possible?

2

While it is true that in most cases we need to fix the virtual destructor for classes. It is OK to ignore -Wnon-virtual-dtor in the case of TVM, because the object class are constructed via make_object, which will directly record the correct destructor(so virtual destructor is not necessary). There is a stricter check virtual deletion with virtual destructor which the current class passes.

We can consider fix some of these warning though if we think it is important to support -Wextra

3

should fix most of the issues except for the unused-parameters. Note that there are certain virtual-dtor report in llvm as well. These warnings, along with most of the TVM ones are false alarms, so the fix is just act extra caution.

4

Thank you and your code is helpful.

5

I think I have some concern adding empty virtual destructors.

First, as @tqchen mentioned, it is okay not to use virtual destructors, because objects are constructed using make_object which figures the correct destructors automatically.

Second, will this bring redundant vtables in those classes?

6

We are not adding v-destructor to all objects, but only the objects that has virtual functions. Most of the IR nodes do not, and as a result won’t have vtable. For the objects that have virtual functions, the cost will be one additional entry in the existing vtable.

7

Then it sounds good. Thanks!