- User Since
- Mar 14 2013, 3:16 PM (278 w, 6 d)
LGTM aside from a minor nit.
LGTM as well
Aside from an assert than can be removed, this LGTM on the attribute side of things.
Adding Richard to see if he agrees with the direction taken.
Mon, Jul 16
LGTM aside from a style nit (you can take it or leave it).
LGTM with a formatting nit.
Sun, Jul 15
Sat, Jul 14
Fri, Jul 13
LGTM, though it's your call on the const_cast stuff whether you want to revert or keep it.
LGTM! Thank you for this!
I believe this would be non-comforming.
Thu, Jul 12
LGTM! Thank you for doing this work, Erich!
Mon, Jul 9
Fri, Jul 6
LGTM! Do you need me to commit on your behalf, or do you have commit privileges?
LGTM, thank you!
Thu, Jul 5
This is acceptable because Darwin guarantees that, despite the watchOS ABI differences, sizeof(ptrdiff_t) == sizeof(NS[U]Integer)
Wed, Jul 4
This mostly looks good, but the name of the matcher is inconsistent.
Tue, Jul 3
Aside from a new round of minor doc nits, I think this is looking good.
This still LGTM; do you need someone to commit on your behalf?
Mon, Jul 2
Sat, Jun 30
Wow, this is an amazing start to a very large refactoring! Thank you for tackling this. I've made a few review comments but haven't gotten through the entire patch yet. The parts that I've seen all look reasonably sane thus far.
Fri, Jun 29
Once this goes in, you can also update TrailingReturnTypeCheck.cpp to remove its local instance of this type matcher.
Thu, Jun 28
LGTM with a small documentation nit.
Wed, Jun 27
LGTM aside from some documentation nits.
Mon, Jun 25
Fri, Jun 22
Wed, Jun 20
Do you also want to update AttrDocs.td with some similar information?
Tue, Jun 19
Jun 19 2018
LGTM, thank you for this!
I've committed in r335048; if @rsmith has concerns, they can be addressed post commit.
Jun 15 2018
LGTM aside from some minor nits.
Jun 14 2018
Thank you for working on this odd detail of attributes!
Jun 13 2018
I had to revert due to failing tests. The revert was done in r334606 and this is an example of a failing bot: http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/31500
I've commit in r334604 with one minor change -- I added an include for <cctype> to the unit test so that std::isspace() would compile properly on all platforms.
Jun 12 2018
Updating based on review feedback.
Jun 9 2018
Jun 8 2018
Jun 7 2018
Jun 6 2018
Jun 5 2018
LGTM as well, for what little it's worth. :-)
LGTM aside from a nit with one of the tests. Once you've updated the patch and verified that check-clang passes all tests, I can commit for you next week when I'm back from meetings (unless someone else gets to it before me).
Aside from a minor commenting nit, this LGTM.
Jun 4 2018
Jun 1 2018
May 31 2018
Committed in r333653.