Szelethus accepted this revision.
Szelethus added a comment.
This revision is now accepted and ready to land.

In D79072#2025120 <https://reviews.llvm.org/D79072#2025120>, @martong wrote:

> I'd split this patch into two as well.
>
> 1. [NFC] Refactoring parts
> 2. The actual extra additions about sizeof and typedef. WDYT?
>
>   Other than that looks good.


I wouldn't change this now, but for the future, changes like splitting 
functions into `checkVLA` and `checkVLASize` should deserve their own revision, 
because the actual new logic would only take a few lines, and a few minutes to 
review. The biggest pain point is that both this, and the followup patch change 
the same code and it makes it difficult to know whats the new functionality and 
what is just reorganization.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79072



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to