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

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 22 12:14:29 PST 2020


dblaikie added a comment.

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)


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

https://reviews.llvm.org/D93654



More information about the llvm-commits mailing list