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

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D93654#2468695 <https://reviews.llvm.org/D93654#2468695>, @dblaikie wrote:

> In D93654#2468692 <https://reviews.llvm.org/D93654#2468692>, @dblaikie wrote:
>
>> In D93654#2468663 <https://reviews.llvm.org/D93654#2468663>, @Paul-C-Anagnostopoulos wrote:
>>
>>> I will update this Phabricator revision later today and then let it cook for a day or two before pushing it.
>>
>> As mentioned elsewhere - reviews don't generally have timelines like this. If it's sent for review, it should only be committed once approved. ( https://llvm.org/docs/CodeReview.html#code-review-workflow ) There are a few cases where that doesn't tend to hold as much, but that's the general rule.
>
> Admittedly it does get a bit weird when a patch gets approved, and then has significant changes after that approval. (I'd probably have read @lattner's first comment/approval as "good to commit as-is, if ArrayRef was considered and not used for some reason", though once the change then went to ArrayRef and subsequent call-site cleanups, probably worth coming back around for a new approval)

Because sometimes even the best of ideas turns out looking weird/problematic in practice.


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

https://reviews.llvm.org/D93654