[GitHub] [incubator-tvm] tqchen commented on a change in pull request #4886: [WIP][POC]First pass a defining at non-recursive Graph Vistor and Rewriter

[GitBox <gi...@apache.org>]

tqchen commented on a change in pull request #4886: [WIP][POC]First pass a defining at non-recursive Graph Vistor and Rewriter
URL: https://github.com/apache/incubator-tvm/pull/4886#discussion_r401213597
 
 

 ##########
 File path: include/tvm/relay/expr_functor.h
 ##########
 @@ -196,7 +196,7 @@ class ExprMutator
    * \brief Mutate is alias for VisitExpr
    * \return expr.
    */
-  Expr Mutate(const Expr& expr) {
+  virtual Expr Mutate(const Expr& expr) {
 
 Review comment:
   I still think it is better to not subclass Mutate, instead, override VisitExpr in the ScopeMutator, which calls into DispatchVisitExpr that does the dispatching.
   
   This way we do not have to change a lot of calls of VisitExpr into Mutate in the subclasses, which can be confusing(for user to think about which one to call).

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


With regards,
Apache Git Services