[PATCH] D21000: [CFLAA] Cleaned up StratifiedAttrs handling

Jia Chen via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 6 15:12:10 PDT 2016


grievejia added a comment.

In http://reviews.llvm.org/D21000#450028, @george.burgess.iv wrote:

> - This needs tests. Ideally at least one for each logical change that's being made


This is my mistake. I have no excuse for it :(

> - We generally request that people try to make reviews as targeted as possible. So, if you could please split patches like these out into N different reviews in the future, that would be great!


OK I'll try to do that in the future.

There were two reasons why I did this giant patch: (1) I thought they are all related to StratifiedAttrs and share a common theme. (2) I haven't figure out how to work on patches that depends on each other, so I tend to use giant patch to minimize inter-patch dependency. Say I break this patch into two pieces P0 and http://reviews.llvm.org/P1, where http://reviews.llvm.org/P1 depends on P0. If for some reasons P0 get changed before it is committed in-tree, is there a clean way I can propagate the changes to http://reviews.llvm.org/P1 as well?

> One is `noteAttribute`, the other is `noteAttributes`, so I don't think overload resolution should be kicking in at all here. :)


No wonder!

Then the problem could have been solved in a far less intrusive manner. Well, I'll just leave the changes as-is because I'm too lazy to manually revert all those changes...


http://reviews.llvm.org/D21000





More information about the llvm-commits mailing list