- User Since
- Mar 14 2013, 3:16 PM (231 w, 5 d)
Wed, Aug 16
Mon, Aug 14
Fri, Aug 11
I've commit in r310727.
I've commit as r310707.
Wed, Aug 9
LGTM, good catch!
Tue, Aug 8
I've commit in r310388
Aside from a minor naming nit, LGTM!
Can you generate the updated patch with more context (https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface)?
Aside from a coding style nit and the unanswered question that hopefully @tstellar can help answer, this LGTM. I'll wait to accept until we figure out the answer for Linux, however.
Wed, Aug 2
This looks reasonable to me, once it's been formatted. Let's give @rsmith a few days to comment before committing, though.
Fri, Jul 28
Thu, Jul 27
This looks reasonable to me, but you should wait for @rsmith to sign off before committing.
Tue, Jul 25
How does this check differ from the -Wdelete-non-virtual-dtor warning class that comes out of the frontend?
Jul 20 2017
LGTM with a small testing nit.
Jul 18 2017
A few small nits, but otherwise LGTM
Jul 15 2017
LGTM with a small commenting nit.
Has GCC picked the same name for their language standard? I want to make sure we're consistent there (and think this is the correct name).
Jul 14 2017
Committed in r308038.
You've explained how you are accomplishing this but not why. I don't think Clang typically keeps erroneous AST nodes in the tree. What kind of problem is this intended to solve?
Jul 13 2017
This looks reasonable to me, but I am not overly familiar with this part of the toolchain. You should wait for another LG before committing.
Jul 12 2017
Jul 11 2017
LGTM, thank you!
Jul 10 2017
Jul 5 2017
Jul 3 2017
Jun 30 2017
Jun 28 2017
Jun 26 2017
Jun 22 2017
Aside from a few small nits with the test cases, this LGTM! You should hold off on committing for a day or two in case Richard or others have comments on the patch.
Jun 21 2017
Jun 16 2017
A few more nits, but this feels like it's getting close to ready (at least, to me).
This is generally looking good to me, with a few small nits. @rsmith, do you have thought on this?
Jun 15 2017
Jun 14 2017
Committed in r305432. You should consider reaching out to Chris Lattner for svn access!
Jun 13 2017
Adding Richard to the review for some wider perspective than just mine on the overall design.
Jun 8 2017
Aside from one minor nit, LGTM!
Jun 7 2017
Aside from one minor nit, LGTM
Aside from a minor suggestion, LGTM!
Jun 5 2017
This check is an interesting one. The rules around what is signal safe are changing for C++17 to be a bit more lenient than what the rules are for C++14. CERT's rule is written against C++14, and so the current behavior matches the rule wording. However, the *intent* of the rule is to ensure that only signal-safe functionality is used from a signal handler, and so from that perspective, I can imagine a user compiling for C++17 to want the relaxed rules to still comply with CERT's wording. What do you think?