[PATCH] D29124: [ARM] GlobalISel: Fix stack-use-after-scope bug.

[Renato Golin via Phabricator via llvm-commits]

rengolin added a comment.

In https://reviews.llvm.org/D29124#656228, @bkramer wrote:

> It's covered by existing tests. ASAN catches it with -fsanitize-address-use-after-scope, which is not on by default yet afaik.


Then say that on the description. What introduced the bug, how did you find out, etc. It helps a lot to know where things came from, not just what they are.

>> OTOH, why push a public review if the review was done internally anyway?
> 
> So other people have a chance to chime in?

Chime in? It was approved seconds after posting and committed one minute after approved. This was clearly an internal process made public for no reason.

> I'd argue that this change is below the triviality threshold for post-commit review though.

I'd argue the same thing, but once you posted for review, you *have* to let it linger. This is in the developer policy and it should be followed.

People from the same company, working together, approving on the same minute is really not the way we should be going.

Worst case scenario (you posted by mistake), ping people on email/IRC to expedite the approval. On an obvious patch like this, anyone would have approved instantly.

cheers,
--renato


Repository:
  rL LLVM

https://reviews.llvm.org/D29124