tqchen commented on a change in pull request #9533:
URL: https://github.com/apache/tvm/pull/9533#discussion_r752560404



##########
File path: src/relay/ir/expr.cc
##########
@@ -71,6 +71,29 @@ Tuple::Tuple(tvm::Array<relay::Expr> fields, Span span) {
   data_ = std::move(n);
 }
 
+Tuple Tuple::WithFields(Optional<Array<Expr>> opt_fields, Optional<Span> 
opt_span) {
+  Array<Expr> fields = opt_fields.value_or(get()->fields);

Review comment:
       To elaborate, `x.WithFields` in the current case might mutate x if it is 
the only copy. 
   
   ```c++
   y = x.WithFields(..., ...)
   // if x is the only copy, it will change x itself. Which may not be the 
intention of the API.
   ```
   
   instead if we do 
   ```c++
   y = WithFields(std::move(x), .., ...)_
   ```
   and x is the only copy, we know for certainly that x will not be used in the 
future and it is safe to re-use the memory space
   




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