[PATCH] D93654: [TableGen] Change getAllDerivedDefinitions() to return an ArrayRef

Paul C. Anagnostopoulos via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 23 14:05:54 PST 2020


Paul-C-Anagnostopoulos added inline comments.


================
Comment at: mlir/tools/mlir-tblgen/OpInterfacesGen.cpp:83-84
+
+  InterfaceGenerator(const std::vector<llvm::Record *> &defs, raw_ostream &os)
+      : defs(defs), os(os) {} // Copy the defs vector.
 
----------------
dblaikie wrote:
> This change looks incorrect to me - what was the motivation to change the parameter from rvalue ref? 
> (though it might, regardless of this change, be better written as pass-by-value - then callers with temporaries (like those passing the result of getAllOpInterfaceDefinitions) or explicit std::move for the parameter would be benefit from being able to move into the InterfaceGenerator - and callers that want to keep their own copy of the vector could do so as well - https://abseil.io/tips/117 discusses the general idea in more detail)
The code would not compile as it was. I will investigate the best way to do this.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93654/new/

https://reviews.llvm.org/D93654



More information about the llvm-commits mailing list