Lunderberg commented on code in PR #13267:
URL: https://github.com/apache/tvm/pull/13267#discussion_r1013504662


##########
src/tir/transforms/narrow_datatype.cc:
##########
@@ -187,6 +187,11 @@ class DataTypeVisitor final : public StmtExprVisitor {
   arith::ConstIntBoundAnalyzer::BoundMapType bound_;
 };
 
+#if __clang__

Review Comment:
   (A little bit late to the discussion, but I hope this helps.)
   
   I looked a bit into the warning, and it looks like part of it is because the 
`DataTypeLegalizer` defines the `VisitStmt_` as public, whereas the 
`StmtExprMutator` defines them as protected.  So the other instances of this 
pattern are overriding a protected method, which doesn't trigger the warning.
   
   As example code. the warning is trying to avoid cases like the third one 
below, where the static type causes an unexpected overload set when calling a 
method on the derived type.
   
   ```c++
   class Base {
   public:
     virtual func(int x) {}
     virtual func(double x) {}
   };
   
   class Derived {
   public:
     virtual func(double x) {}
   };
   
   int main() {
     // Static type is base, uses Base's vtable.
     std::unique_ptr<Base> base = std::make_unique<Base>();
     base->func(5); // Calls Base::func(int)
   
     // Static type is base, uses Derived's vtable.
     std::unique_ptr<Base> derived = std::make_unique<Dervied>();
     derived->func(5); // Calls Base::func(int)
   
     // Static type is Derived, uses Derived's vtable.  Overload
     // resolution selects `Derived::func(double)` because the overload
     // resolution set is based on the static type, and doesn't include
     // `Base::func(int)`.
     std::unique_ptr<Derived> derived = std::make_unique<Dervied>();
     derived->func(5); // Calls Derived::func(double)
   }
   ```



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to