[PATCH] D57104: [AST] Pack GenericSelectionExpr

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

In D57104#1370025 <https://reviews.llvm.org/D57104#1370025>, @aaron.ballman wrote:

> In D57104#1369985 <https://reviews.llvm.org/D57104#1369985>, @steveire wrote:
>
> > There's definitely a better possible ordering in two commits:
> >
> > 1. Introduce `::Create` and port to it
> > 2. Use trailing objects, taking advantage of the fact that `::Create` exists.
> >
> >   That would make it clear in the future to other people because both commits would be cleaner, both commit messages would say what the commit does, and neither commit would have the noise of the other change.
> >
> >   Not splitting this commit makes it less reviewable to people who are not around today.
>
>
> I would find reviewing that more confusing because the two reviews are inextricably linked. There's no need for a `Create()` method without the other changes, and the other changes require a `Create()` method. I get what you mean about keeping reviews concise, but at some point you can split the reviews so much that it becomes really difficult to perform a sensible review because you get too much information in isolation.


+1 You can cut a commit into arbitrarily small pieces. As a silly example I could inherit from `llvm::TrailingObjects` in one commit.
Then remove the `Stmt **` in another one and so on.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57104