[PATCH] D92674: [TableGen] Cache the vectors of records returned by getAllDerivedDefinitions().

[Paul C. Anagnostopoulos via Phabricator via llvm-commits]

Paul-C-Anagnostopoulos added inline comments.


================
Comment at: llvm/lib/TableGen/Record.cpp:2600
+// the same vectors multiple times.
 std::vector<Record *> RecordKeeper::getAllDerivedDefinitions(
+    StringRef ClassName) const {
----------------
craig.topper wrote:
> Paul-C-Anagnostopoulos wrote:
> > dblaikie wrote:
> > > Now that the result is cached, perhaps this function could return by const ref instead of by value, to avoid unnecessary copies of the resulting vector?
> > The vector must be copied, because there is nothing stopping the caller from modifying it.
> > 
> > Do you think it's worth the effort to establish the rule that it cannot be modified and then investigate every call to see what's going on?
> > 
> If the caller needs to modify, the caller can make a copy explicitly.  I doubt most callers modify it. I think if you just change it to return const std::vector<Record*> & any code that does 
> 
> ```
> std::vector<Record*> Foo = getAllDerivedDefinitions
> ```
> 
> will still compile. A copy will just be made by the assignment. This should cover most callers I would guess. Then we can through and individually change the callers to use "const std::vector &" to remove the copies.
> 
> 
That sounds like a good plan. 

Some day I will get used to all the copying. I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92674