[PATCH] D88832: [TableGen] Add new getAllDerivedDefinitionsTwo function to RecordKeeper

Jessica Clarke via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 5 07:37:01 PDT 2020


jrtc27 added a comment.

In D88832#2311859 <https://reviews.llvm.org/D88832#2311859>, @Paul-C-Anagnostopoulos wrote:

> Is there a way to get full context selectively? I suspect you want full context only on the last two files.

There's no harm in including the full context for everything given it's hidden by default. But no, if you wanted to do that you'd have to run `git show -U10000000 <commit> <file> [file ..]` and replace just the bit of the diff that corresponds to those files.

> I thought about naming it in a more scalable way. I also thought about simply overloading the existing getAllDerivedDefinitions. I decided not to because there is exactly one backend that looks for definitions with two specific parents. I really can't conceive that anyone would ever want to do three, though, of course, I could be wrong.
>
> If people think it's important to allow an arbitrary number of parent classes, I think I will overload getAllDerivedDefinitions with one that takes an array.

Well, (a) we can't predict the future; (b) if it's possible then maybe people will find ways to use it and make their code cleaner; (c) it allows you to implement the existing single-class function in terms of the general one, rather than having two copies of almost the same code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88832



More information about the llvm-commits mailing list