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

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/TableGen/Record.cpp:2600
+// the same vectors multiple times.
 std::vector<Record *> RecordKeeper::getAllDerivedDefinitions(
+    StringRef ClassName) const {
----------------
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.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92674