aaron.ballman (Aaron Ballman)
User

Projects

User does not belong to any projects.
User Since
Mar 14 2013, 3:16 PM (210 w, 5 d)

Recent Activity

Yesterday

aaron.ballman accepted D28671: [ASTMatchers] add typeAliasTemplateDecl matcher..

LGTM!

Tue, Mar 28, 5:49 AM
aaron.ballman added a reviewer for D28671: [ASTMatchers] add typeAliasTemplateDecl matcher.: aaron.ballman.
Tue, Mar 28, 5:21 AM
aaron.ballman added inline comments to D31370: [clang-tidy] Prototype to check for exception specification.
Tue, Mar 28, 5:03 AM
aaron.ballman added a comment to D31413: [libc++] Use __attribute__((init_priority(101))) to ensure streams get initialized early.

I'm not certain of a good way to test it, but I have a question about the value you picked for init_priority. My understanding of the values starting from 101 is that 1-100 are reserved for implementation use. Is that understanding correct? If so, you may want to pick a value below 100 to ensure there's not an arms race with the user. I believe this may require some alteration to SemaDeclAttr.cpp to not diagnose when this is coming from a system header. Otherwise, what's to stop the user from having something marked constructor(101) that attempts to use a stream, but can't because they're not initialized yet?

Tue, Mar 28, 4:57 AM

Mon, Mar 27

aaron.ballman added a comment to D31153: Add the ability to use the children() range API in a const-correct manner.

As I mentioned to Craig Topper recently on another review, generally when implementing const and non-const overloads the non-const is implemented in terms of the const overload (& const_casts away const on the result). This ensures no UB if the const overload is called on a truly const object. Even if there aren't any truly const Stmts today, I'd still prefer the code be written so as not to assume they couldn't exist.

Mon, Mar 27, 10:20 AM
aaron.ballman added inline comments to D31308: [clang-tidy] new check readability-no-alternative-tokens.
Mon, Mar 27, 9:56 AM
aaron.ballman added inline comments to D18914: [clang-tidy] new readability-redundant-inline.
Mon, Mar 27, 8:36 AM
aaron.ballman added inline comments to D31308: [clang-tidy] new check readability-no-alternative-tokens.
Mon, Mar 27, 7:45 AM
aaron.ballman accepted D31166: Encapsulate FPOptions and use it consistently.

LGTM!

Mon, Mar 27, 7:25 AM
aaron.ballman added a comment to D31153: Add the ability to use the children() range API in a const-correct manner.

Because I expect this to be fairly uncontroversial, I'll give it a week in case folks have comments, then commit (and will handle anything in post-commit review).

Mon, Mar 27, 5:25 AM

Sun, Mar 26

aaron.ballman added inline comments to D29599: Clang Changes for alloc_align.
Sun, Mar 26, 7:40 AM
aaron.ballman accepted D24886: Add [[clang::suppress(rule, ...)]] attribute.

LGTM!

Sun, Mar 26, 7:20 AM
aaron.ballman added inline comments to D30383: [clang-tidy] aliases for safety module from section 8 to 12 done.
Sun, Mar 26, 7:18 AM

Sat, Mar 25

aaron.ballman added inline comments to D24892: [clang-tidy] Add option "LiteralInitializers" to cppcoreguidelines-pro-type-member-init.
Sat, Mar 25, 3:15 PM

Thu, Mar 23

aaron.ballman accepted D31245: Correct class-template deprecation behavior-REDUX.

Being better about const, I prefer the strictest capture, so unless we have a policy different, I'll leave it.

Thu, Mar 23, 11:19 AM
aaron.ballman added inline comments to D31245: Correct class-template deprecation behavior-REDUX.
Thu, Mar 23, 11:06 AM
aaron.ballman added inline comments to D31166: Encapsulate FPOptions and use it consistently.
Thu, Mar 23, 10:38 AM
aaron.ballman added a comment to D31276: Add #pragma clang fp.

Thanks very much, Aaron! It would be great if you could also look at the three patches that add support for the generation of the new FMF 'contract' for -ffp-contract=fast. I've added them in the dependencies of this revision. (D31168 is not really a prerequisite but I am planning to commit the patches in this order.)

Thu, Mar 23, 10:26 AM
aaron.ballman added inline comments to D31245: Correct class-template deprecation behavior-REDUX.
Thu, Mar 23, 10:24 AM
aaron.ballman accepted D31276: Add #pragma clang fp.

LGTM!

Thu, Mar 23, 10:12 AM
aaron.ballman added inline comments to D31245: Correct class-template deprecation behavior-REDUX.
Thu, Mar 23, 9:42 AM
aaron.ballman added inline comments to D31245: Correct class-template deprecation behavior-REDUX.
Thu, Mar 23, 9:32 AM
aaron.ballman added inline comments to D31276: Add #pragma clang fp.
Thu, Mar 23, 9:07 AM
aaron.ballman added inline comments to D30547: [clang-tidy] Forwarding reference overload in constructors.
Thu, Mar 23, 5:58 AM · Restricted Project

Wed, Mar 22

aaron.ballman added inline comments to D30547: [clang-tidy] Forwarding reference overload in constructors.
Wed, Mar 22, 7:17 AM · Restricted Project
aaron.ballman added inline comments to D30547: [clang-tidy] Forwarding reference overload in constructors.
Wed, Mar 22, 7:01 AM · Restricted Project
aaron.ballman added inline comments to D30547: [clang-tidy] Forwarding reference overload in constructors.
Wed, Mar 22, 6:35 AM · Restricted Project

Tue, Mar 21

aaron.ballman requested changes to D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros.
Tue, Mar 21, 6:23 PM
aaron.ballman reopened D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros.

This change was reverted in r298470. The use of the <assert.h> include is a problem because this is not a clang-supplied header file. Also, there's a (good) question about what array decay is happening in the assert(false) test case.

Tue, Mar 21, 6:23 PM
aaron.ballman closed D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros.

I've commit in r298421.

Tue, Mar 21, 12:13 PM
aaron.ballman updated the diff for D31153: Add the ability to use the children() range API in a const-correct manner.

Missed a case for UnaryExprOrTypeTraitExpr.

Tue, Mar 21, 12:05 PM
aaron.ballman added a comment to D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros.

Hi @aaron.ballman,

Thanks for the review!

I don't have rights to push this. Who should I contact to push the change in my behalf?

Tue, Mar 21, 11:13 AM
aaron.ballman added a comment to D30009: Add support for '#pragma clang attribute'.

I would be ok with that. We could merge apply_to and apply_only_to into a single apply_to matching rule set specifier (it would behave like apply_only_to).

Tue, Mar 21, 10:01 AM
aaron.ballman accepted D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros.

LGTM!

Tue, Mar 21, 9:56 AM
aaron.ballman added inline comments to D31179: Objective-C categories should support attributes.
Tue, Mar 21, 9:42 AM

Mon, Mar 20

aaron.ballman created D31153: Add the ability to use the children() range API in a const-correct manner.
Mon, Mar 20, 3:30 PM
aaron.ballman added inline comments to D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros.
Mon, Mar 20, 3:15 PM
aaron.ballman added a comment to D30009: Add support for '#pragma clang attribute'.

Looking over the most recent version, I'm happy with the general semantics of push with apply_only_to. I'm not sure I see the point of apply_to: it doesn't allow the user to do anything that can't be done with apply_only_to. Also, if the apply_to list for an attribute ever changes, it becomes impossible to write code which supports both versions of the compiler.

Mon, Mar 20, 2:55 PM
aaron.ballman added inline comments to D30383: [clang-tidy] aliases for safety module from section 8 to 12 done.
Mon, Mar 20, 2:30 PM
aaron.ballman added a comment to D30746: [clang-tidy] new check for throw expressions in destructors.

@aaron.ballman i asked on the mailing list and we had a little discussion for a general noexcept check. Most were the opinion that flow analysis should be enough.

I don't currently see why even flow-sensitive analysis is required -- the cases I can think of can be done on the AST, but I'm sure there's something I'm missing, and I'm curious what it is...

Mon, Mar 20, 1:46 PM

Sun, Mar 19

aaron.ballman edited reviewers for D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros, added: aaron.ballman; removed: cfe-commits.
Sun, Mar 19, 12:21 PM
aaron.ballman added inline comments to D30383: [clang-tidy] aliases for safety module from section 8 to 12 done.
Sun, Mar 19, 10:36 AM
aaron.ballman closed D31128: Rename the safety module to be hicpp.

Commit in r298229. If we want to reword the hicpp-no-assembler diagnostic, we can do so in a follow-up patch.

Sun, Mar 19, 10:36 AM
aaron.ballman added inline comments to D31128: Rename the safety module to be hicpp.
Sun, Mar 19, 10:05 AM
aaron.ballman added inline comments to D31128: Rename the safety module to be hicpp.
Sun, Mar 19, 9:42 AM
aaron.ballman created D31128: Rename the safety module to be hicpp.
Sun, Mar 19, 9:35 AM
aaron.ballman added inline comments to D30383: [clang-tidy] aliases for safety module from section 8 to 12 done.
Sun, Mar 19, 8:58 AM
aaron.ballman added a comment to D24886: Add [[clang::suppress(rule, ...)]] attribute.

I'd also like to see some tests in Misc that confirm the attribute is being attached to the appropriate AST nodes for declarations, statements, and at namespace scope.

Sun, Mar 19, 8:45 AM
aaron.ballman added inline comments to D30383: [clang-tidy] aliases for safety module from section 8 to 12 done.
Sun, Mar 19, 6:43 AM

Sat, Mar 18

aaron.ballman added a comment to D30009: Add support for '#pragma clang attribute'.

I really like the way this feature is shaping up! I apologize for how long it took to review the code (I was out for work for two weeks and this is a pretty extensive patch). Thank you for the continued efforts on this.

Sat, Mar 18, 10:22 AM
aaron.ballman accepted D30766: Add support for attribute "enum_extensibility".

Aside from the request for a FIXME and a decision from the author on error vs warning, the code LGTM. Feel free to make a decision and commit without further review.

Sat, Mar 18, 8:31 AM

Thu, Mar 16

aaron.ballman added a comment to D30896: [Clang-tidy] add check misc-prefer-switch-for-enums.

I've played around with a few heuristics but it's still far too contentious to have this check on by default and have it warn in places I want warnings. Where should it go?

Thu, Mar 16, 11:09 AM · Restricted Project

Wed, Mar 15

aaron.ballman closed D30854: [ASTMatchers] Add ObjC matchers for fundamental decls.

I've commit in r297882, thanks!

Wed, Mar 15, 1:26 PM
aaron.ballman accepted D30854: [ASTMatchers] Add ObjC matchers for fundamental decls.

LGTM! I'll go ahead and commit for you.

Wed, Mar 15, 12:36 PM
aaron.ballman added inline comments to D30854: [ASTMatchers] Add ObjC matchers for fundamental decls.
Wed, Mar 15, 10:51 AM
aaron.ballman added inline comments to D30854: [ASTMatchers] Add ObjC matchers for fundamental decls.
Wed, Mar 15, 9:10 AM
aaron.ballman added a comment to D30896: [Clang-tidy] add check misc-prefer-switch-for-enums.

I've run the check on clang. It's noisy (690 warnings). The (few) cases I've manually inspected look like firm candidates for replacing with switches. Not my call though.

Wed, Mar 15, 8:20 AM · Restricted Project

Tue, Mar 14

aaron.ballman added inline comments to D30854: [ASTMatchers] Add ObjC matchers for fundamental decls.
Tue, Mar 14, 10:26 AM
aaron.ballman added inline comments to D30854: [ASTMatchers] Add ObjC matchers for fundamental decls.
Tue, Mar 14, 9:58 AM
aaron.ballman accepted D30945: Fix mis-spelled enum.

LGTM, thank you!

Tue, Mar 14, 9:32 AM
aaron.ballman added a comment to D30383: [clang-tidy] aliases for safety module from section 8 to 12 done.

@aaron.ballman any news for PRQA on naming this module? Seems a shame to block good work.

Tue, Mar 14, 5:27 AM
aaron.ballman added a comment to D30896: [Clang-tidy] add check misc-prefer-switch-for-enums.

@aaron.ballman

re: "perhaps the check could be relaxed to only consider cases where you have multiple if/else if clauses".

Sadly this does not satisfy my use case where most of the time a single enum value is used.

enum class animal_kind { herbivore, carnivore };
if(k == animal_kind:: carnivore) { ... }

I now add omnivore and play 'hunt-the-if' knowing anything I miss will be a bug.

Tue, Mar 14, 5:17 AM · Restricted Project

Mon, Mar 13

aaron.ballman added a comment to D30896: [Clang-tidy] add check misc-prefer-switch-for-enums.

I'm curious what kind of results you get when running this over a large code base. There are definitely times when using == or != for a specific value is *way* cleaner than using a switch statement, and I worry about this being so chatty that it needs to be disabled by default.

Mon, Mar 13, 3:28 PM · Restricted Project
aaron.ballman added inline comments to D30766: Add support for attribute "enum_extensibility".
Mon, Mar 13, 3:22 PM
aaron.ballman added inline comments to D19201: [clang-tidy] misc-throw-with-noexcept.
Mon, Mar 13, 3:09 PM
aaron.ballman closed D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check.

I've commit in r297671, thanks!

Mon, Mar 13, 2:51 PM
aaron.ballman added a comment to D30736: [Support] Add support for getting file system permissions on Windows and implement sys::fs::set/getPermissions to work with them.

Can someone more familiar with UNIX permissions review the new changes before we land them?

Mon, Mar 13, 9:36 AM
aaron.ballman added a comment to D30736: [Support] Add support for getting file system permissions on Windows and implement sys::fs::set/getPermissions to work with them.

LGTM as well; I've commit in r297617, thank you!

Mon, Mar 13, 5:37 AM
aaron.ballman closed D30736: [Support] Add support for getting file system permissions on Windows and implement sys::fs::set/getPermissions to work with them.

LGTM as well; I've commit in r297617, thank you!

Mon, Mar 13, 5:29 AM
aaron.ballman accepted D30884: When diagnosing taking address of packed members skip __unaligned-qualified expressions.

LGTM!

Mon, Mar 13, 5:11 AM
aaron.ballman added inline comments to D30547: [clang-tidy] Forwarding reference overload in constructors.
Mon, Mar 13, 4:47 AM · Restricted Project

Sun, Mar 12

aaron.ballman closed D26800: [Sema] Make __attribute__((notnull)) inheritable through function parameters.

Committed in r297592, thank you!

Sun, Mar 12, 3:42 PM
aaron.ballman accepted D26800: [Sema] Make __attribute__((notnull)) inheritable through function parameters.
Sun, Mar 12, 3:42 PM
aaron.ballman accepted D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check.

LGTM!

Sun, Mar 12, 3:30 PM
aaron.ballman added a comment to D30746: [clang-tidy] new check for throw expressions in destructors.

that is true as well.
lets see what the static analysis brings. maybe its easy to achieve something there.

is there a check for noexcept already?

Sun, Mar 12, 12:21 PM
aaron.ballman added a comment to D30746: [clang-tidy] new check for throw expressions in destructors.

yeah you are right.
i will look for some codebases, maybe some interesting results there.

trying to save this one :D
keeping the throw check and see if every function call not in a try statement is noexcept?

Sun, Mar 12, 11:05 AM
aaron.ballman added a comment to D30746: [clang-tidy] new check for throw expressions in destructors.

I agree with the static analysis. Iam not capable so atm, since i have no clue how to write sensefull static analysis :) But i would like to learn it.

Sun, Mar 12, 10:58 AM
aaron.ballman added inline comments to D30547: [clang-tidy] Forwarding reference overload in constructors.
Sun, Mar 12, 10:48 AM · Restricted Project
aaron.ballman added a comment to D30746: [clang-tidy] new check for throw expressions in destructors.

@aaron.ballman I would like to make this more valuable and mor applicable to other rulesets as well.
Do you think it would be enough to do the same bad logic in deallocation, without deep analysis of the exceptions that can occur and if they are caught some where?

Sun, Mar 12, 10:30 AM
aaron.ballman added a comment to D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle..

Any updates on this?

Sun, Mar 12, 10:15 AM
aaron.ballman added a reviewer for D30854: [ASTMatchers] Add ObjC matchers for fundamental decls: aaron.ballman.
Sun, Mar 12, 6:35 AM
aaron.ballman added inline comments to D30854: [ASTMatchers] Add ObjC matchers for fundamental decls.
Sun, Mar 12, 6:35 AM

Wed, Mar 8

aaron.ballman added a comment to D30736: [Support] Add support for getting file system permissions on Windows and implement sys::fs::set/getPermissions to work with them.

My biggest concern with this is that it doesn't really set the file permissions on Windows, it just sets the file *attributes*. It's perfectly cromulent for a file to have the read-only attribute set but still not have read permissions for the current user, or to not have the read-only attribute set and still not be writable for the current user, for instance. Is that something we should be caring about?

To be fair, to actually implement this as setting the permissions would be a huge effort and involve messing with ACLs and all kinds of other nonsense that just seems completely unnecessary, especially when the only real motivation is to test the behavior of read-only files (which would probably double as a reasonably effective test for the case where you don't have actual permissions to write the file either).

Wed, Mar 8, 9:17 PM
aaron.ballman added a comment to D30736: [Support] Add support for getting file system permissions on Windows and implement sys::fs::set/getPermissions to work with them.

My biggest concern with this is that it doesn't really set the file permissions on Windows, it just sets the file *attributes*. It's perfectly cromulent for a file to have the read-only attribute set but still not have read permissions for the current user, or to not have the read-only attribute set and still not be writable for the current user, for instance. Is that something we should be caring about?

Wed, Mar 8, 8:59 PM
aaron.ballman added a comment to D30746: [clang-tidy] new check for throw expressions in destructors.

This check is also somewhat similar to DCL57-CPP from the CERT C++ secure coding guidelines (https://www.securecoding.cert.org/confluence/display/cplusplus/DCL57-CPP.+Do+not+let+exceptions+escape+from+destructors+or+deallocation+functions). If you're interested in extending this check to cover deallocation functions, which have the same concerns as destructors, then it could also be added to the CERT module as well.

Wed, Mar 8, 8:50 PM
aaron.ballman added inline comments to D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check.
Wed, Mar 8, 8:29 PM
aaron.ballman requested changes to D30547: [clang-tidy] Forwarding reference overload in constructors.
Wed, Mar 8, 8:19 PM · Restricted Project

Sat, Mar 4

aaron.ballman requested changes to D30610: [clang-tidy] Added options to cppcoreguidelines-special-member-functions check.

Missing documentation changes for the new options.

Sat, Mar 4, 12:32 PM
aaron.ballman accepted D30592: [clang-tidy] Fix diag message for catch-by-value.

LGTM, thank you!

Sat, Mar 4, 11:43 AM

Fri, Mar 3

aaron.ballman added inline comments to D30383: [clang-tidy] aliases for safety module from section 8 to 12 done.
Fri, Mar 3, 3:53 PM
aaron.ballman requested changes to D30592: [clang-tidy] Fix diag message for catch-by-value.

This change is missing test cases.

Fri, Mar 3, 3:48 PM
aaron.ballman removed a reviewer for D30592: [clang-tidy] Fix diag message for catch-by-value: cfe-commits.
Fri, Mar 3, 3:47 PM

Thu, Mar 2

aaron.ballman added inline comments to D30547: [clang-tidy] Forwarding reference overload in constructors.
Thu, Mar 2, 10:31 PM · Restricted Project

Wed, Mar 1

aaron.ballman added a comment to D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle..

Out of curiosity, have you run this over libc++ or libstdc++ test suites involving std::random_shuffle? If so, were the results acceptable?

Wed, Mar 1, 12:11 PM
aaron.ballman added inline comments to D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle..
Wed, Mar 1, 11:51 AM
aaron.ballman added inline comments to D30383: [clang-tidy] aliases for safety module from section 8 to 12 done.
Wed, Mar 1, 11:48 AM
aaron.ballman added inline comments to D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle..
Wed, Mar 1, 11:21 AM
aaron.ballman added inline comments to D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle..
Wed, Mar 1, 10:45 AM
aaron.ballman added inline comments to D30383: [clang-tidy] aliases for safety module from section 8 to 12 done.
Wed, Mar 1, 10:09 AM

Tue, Feb 28

aaron.ballman added inline comments to D30383: [clang-tidy] aliases for safety module from section 8 to 12 done.
Tue, Feb 28, 3:30 PM