You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2021/03/05 00:58:56 UTC

[GitHub] [tvm] tkonolige opened a new pull request #7593: [FIX] Fix clang12 warnings

tkonolige opened a new pull request #7593:
URL: https://github.com/apache/tvm/pull/7593


   These fixes include
   - Using 0 or NULL instead of nullptr
   - Marking overridden virtual functions with `override`
   - Adding error handling and a missing else statement to a string check
   
   @junrushao1994 @jroesch @areusch @rohanmukh 


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [tvm] tqchen commented on a change in pull request #7593: [FIX] Fix clang12 warnings

Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7593:
URL: https://github.com/apache/tvm/pull/7593#discussion_r588000196



##########
File path: src/runtime/contrib/json/json_runtime.h
##########
@@ -98,7 +98,7 @@ class JSONRuntimeBase : public ModuleNode {
     }
   }
 
-  virtual void SaveToBinary(dmlc::Stream* stream) {
+  void SaveToBinary(dmlc::Stream* stream) override {

Review comment:
       Should be final 




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [tvm] tqchen commented on a change in pull request #7593: [FIX] Fix clang12 warnings

Posted by GitBox <gi...@apache.org>.
tqchen commented on a change in pull request #7593:
URL: https://github.com/apache/tvm/pull/7593#discussion_r588887990



##########
File path: src/runtime/contrib/json/json_runtime.h
##########
@@ -55,7 +55,7 @@ class JSONRuntimeBase : public ModuleNode {
     LoadGraph(graph_json_);
   }
 
-  const char* type_key() const { return "json"; }
+  const char* type_key() const override { return "json"; }

Review comment:
       should also be final as well, do it to other places that overrides




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [tvm] tkonolige commented on a change in pull request #7593: [FIX] Fix clang12 warnings

Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #7593:
URL: https://github.com/apache/tvm/pull/7593#discussion_r588505069



##########
File path: src/runtime/contrib/json/json_runtime.h
##########
@@ -98,7 +98,7 @@ class JSONRuntimeBase : public ModuleNode {
     }
   }
 
-  virtual void SaveToBinary(dmlc::Stream* stream) {
+  void SaveToBinary(dmlc::Stream* stream) override {

Review comment:
       done




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [tvm] junrushao1994 merged pull request #7593: [FIX] Fix clang12 warnings

Posted by GitBox <gi...@apache.org>.
junrushao1994 merged pull request #7593:
URL: https://github.com/apache/tvm/pull/7593


   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [tvm] tkonolige commented on a change in pull request #7593: [FIX] Fix clang12 warnings

Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #7593:
URL: https://github.com/apache/tvm/pull/7593#discussion_r589649796



##########
File path: src/runtime/contrib/json/json_runtime.h
##########
@@ -55,7 +55,7 @@ class JSONRuntimeBase : public ModuleNode {
     LoadGraph(graph_json_);
   }
 
-  const char* type_key() const { return "json"; }
+  const char* type_key() const override { return "json"; }

Review comment:
       done




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [tvm] tkonolige commented on a change in pull request #7593: [FIX] Fix clang12 warnings

Posted by GitBox <gi...@apache.org>.
tkonolige commented on a change in pull request #7593:
URL: https://github.com/apache/tvm/pull/7593#discussion_r589792957



##########
File path: src/runtime/contrib/json/json_runtime.h
##########
@@ -55,7 +55,7 @@ class JSONRuntimeBase : public ModuleNode {
     LoadGraph(graph_json_);
   }
 
-  const char* type_key() const { return "json"; }
+  const char* type_key() const override { return "json"; }

Review comment:
       Actually, arm compute lib inherits from this class. I'm leaving everything as override.




----------------------------------------------------------------
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:
users@infra.apache.org