tqchen commented on a change in pull request #6456:
URL: https://github.com/apache/incubator-tvm/pull/6456#discussion_r487307521
##########
File path: src/tir/transforms/lower_custom_datatypes.cc
##########
@@ -97,7 +97,7 @@ class CustomDatatypesLowerer : public StmtExprMutator {
return expr;
}
-#define DEFINE_MUTATE__(OP, NodeName)
\
+#define DEFINE_MUTATE(OP, NodeName)
\
Review comment:
What is the warning does this macro leads to? Seems the original code is
fine as it is(with underscores as part of the macro)
##########
File path: include/tvm/node/container.h
##########
@@ -1105,7 +1105,7 @@ class DenseMapNode : public MapNode {
friend class MapNode;
};
-#define _TVM_DISPATCH_MAP(base, var, body) \
Review comment:
Perhaps underscore in the macro is fine. unless there is a explict
reason not to do so
##########
File path: include/tvm/auto_scheduler/cost_model.h
##########
@@ -70,6 +70,11 @@ class CostModelNode : public Object {
LOG(FATAL) << "Not implemented";
}
+ /*!
+ * \brief Default virtual destructor
+ */
+ virtual ~CostModelNode() {}
Review comment:
The default v-destructor is not necessary for the object, because they
are created by make_object, and the destructor is stored there.
The `Wdelete-non-virtual-dtor` already covers that case and will create a
warning if an object is created using other methods (e.g. new) but deleted via
virtual destructor. Right now the code base is free of that warning.
##########
File path: include/tvm/auto_scheduler/cost_model.h
##########
@@ -70,6 +70,11 @@ class CostModelNode : public Object {
LOG(FATAL) << "Not implemented";
}
+ /*!
+ * \brief Default virtual destructor
+ */
+ virtual ~CostModelNode() {}
Review comment:
The default v-destructor is not necessary for a sub-class of object,
because they are created by make_object, and the destructor is stored there.
https://github.com/apache/incubator-tvm/blob/master/include/tvm/runtime/memory.h#L74
The `Wdelete-non-virtual-dtor` already covers that case and will create a
warning if an object is created using other methods (e.g. new) but deleted via
virtual destructor. Right now the code base is free of that warning.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
