aaron.ballman (Aaron Ballman)
User

Projects

User does not belong to any projects.

User Details

User Since
Mar 14 2013, 3:16 PM (288 w, 5 d)

Recent Activity

Yesterday

aaron.ballman added inline comments to D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length.
Tue, Sep 25, 1:32 PM · Restricted Project

Mon, Sep 24

aaron.ballman added a comment to D52398: Thread safety analysis: Unwrap __builtin_expect in getTrylockCallExpr.

No problem. Thanks for reviewing! I'm terribly sorry to be bombarding the two of you with so many review requests lately, and I hope it'll soon be over.

Mon, Sep 24, 4:14 PM
aaron.ballman added a comment to D52395: Thread safety analysis: Require exclusive lock for passing by non-const reference.

Unlike checking const qualifiers on member functions, there are probably not many false positives here: if a function takes a non-const reference, it will in almost all cases modify the object that we passed it.

Mon, Sep 24, 3:14 PM
aaron.ballman accepted D52398: Thread safety analysis: Unwrap __builtin_expect in getTrylockCallExpr.

LGTM, but you should give @delesley a chance to weigh in before you commit.

Mon, Sep 24, 2:03 PM
aaron.ballman added inline comments to D52281: [clang-tidy] Add modernize check to use std::invoke in generic code.
Mon, Sep 24, 1:59 PM · Restricted Project
aaron.ballman accepted D52136: [clang-tidy] Add modernize-concat-nested-namespaces check.

Aside from the local const qualification stuff and some minor wordsmithing of the documentation, this LGTM.

Mon, Sep 24, 1:56 PM · Restricted Project
aaron.ballman updated the diff for D52421: [Sema] Diagnose parameter names that shadow inherited field names.

Updated based on review feedback.

Mon, Sep 24, 10:37 AM
aaron.ballman added inline comments to D52421: [Sema] Diagnose parameter names that shadow inherited field names.
Mon, Sep 24, 10:37 AM
aaron.ballman created D52421: [Sema] Diagnose parameter names that shadow inherited field names.
Mon, Sep 24, 7:52 AM

Sat, Sep 22

aaron.ballman created D52400: Improve -Wshadow warnings with enumerators.
Sat, Sep 22, 8:58 PM

Fri, Sep 21

aaron.ballman added inline comments to D52136: [clang-tidy] Add modernize-concat-nested-namespaces check.
Fri, Sep 21, 5:46 PM · Restricted Project
aaron.ballman created D52384: [Sema] Fix redeclaration contexts for enumerators in C.
Fri, Sep 21, 5:18 PM
aaron.ballman added a comment to D52339: Support enums with a fixed underlying type in all language modes.

From the last line in the paper, it seems that C++ compatibility is a goal of the paper (or at least a consideration). We should probably think about this if/when the final wording gets accepted though.

Fri, Sep 21, 10:44 AM
aaron.ballman added inline comments to D52281: [clang-tidy] Add modernize check to use std::invoke in generic code.
Fri, Sep 21, 10:26 AM · Restricted Project
aaron.ballman added a comment to D52339: Support enums with a fixed underlying type in all language modes.
In D52339#1242118, @jfb wrote:
In D52339#1242086, @jfb wrote:

I think we should consider proposing this to the C committee. @aaron.ballman I can help Erik write the paper, would you be able to present it? Too tight for the upcoming meeting, but I'm guessing we have *plenty* of time for ++C17.

It's already been proposed to WG14 and is currently on the SD-3 list of features to consider for C2x. See http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2008.pdf. I know Clive and am happy to point him towards this patch (when it lands) as demonstration of industry desire for the feature, in case he needs to provide updated papers.

Wonderful! Does this match he proposed C2x semantics? Once voted in we'll want to change this from just an extension to also be a -std=c2x thing, better have them match now.

Fri, Sep 21, 9:55 AM
aaron.ballman added a comment to D52339: Support enums with a fixed underlying type in all language modes.
In D52339#1242086, @jfb wrote:

I think we should consider proposing this to the C committee. @aaron.ballman I can help Erik write the paper, would you be able to present it? Too tight for the upcoming meeting, but I'm guessing we have *plenty* of time for ++C17.

Fri, Sep 21, 9:33 AM
aaron.ballman requested changes to D52339: Support enums with a fixed underlying type in all language modes.

Whoops, that acceptance was accidental. Pretending I want changes just to make it clear in Phab. :-)

Fri, Sep 21, 7:59 AM
aaron.ballman accepted D52339: Support enums with a fixed underlying type in all language modes.

I really like this idea in general, thank you for proposing this patch!

Fri, Sep 21, 7:58 AM

Thu, Sep 20

aaron.ballman added inline comments to D52281: [clang-tidy] Add modernize check to use std::invoke in generic code.
Thu, Sep 20, 5:06 PM · Restricted Project
aaron.ballman added inline comments to D52136: [clang-tidy] Add modernize-concat-nested-namespaces check.
Thu, Sep 20, 2:13 PM · Restricted Project
aaron.ballman accepted D50214: Add inherited attributes before parsed attributes..

LGTM!

Thu, Sep 20, 1:48 PM
aaron.ballman accepted D52280: Don't trim non-alphanumeric characters in 'file not found' errors for include directives..

LGTM!

Thu, Sep 20, 8:58 AM

Tue, Sep 18

aaron.ballman added a reviewer for D52200: Thread safety analysis: Handle ObjCIvarRefExpr in SExprBuilder::translate: rjmccall.

Adding a reviewer who knows more about ObjC than I do.

Tue, Sep 18, 5:13 AM
aaron.ballman accepted D52179: [clang-tidy] Replace redundant checks with an assert()..

LGTM!

Tue, Sep 18, 5:07 AM · Restricted Project

Mon, Sep 17

aaron.ballman added inline comments to D52179: [clang-tidy] Replace redundant checks with an assert()..
Mon, Sep 17, 1:20 PM · Restricted Project
aaron.ballman accepted D52184: Remove dead function user_cache_directory().

LGTM!

Mon, Sep 17, 1:15 PM
aaron.ballman accepted D52141: Thread safety analysis: Run more tests with capability attributes [NFC].

LGTM, with a minor formatting nit.

Mon, Sep 17, 7:58 AM
aaron.ballman added inline comments to D51789: [clang] Add the exclude_from_explicit_instantiation attribute.
Mon, Sep 17, 7:24 AM
aaron.ballman accepted D52157: [ASTMatchers] Let isArrow also support UnresolvedMemberExpr, CXXDependentScopeMemberExpr.

LGTM, with a request for the documentation.

Mon, Sep 17, 7:12 AM
aaron.ballman accepted D52135: [Clang-Tidy: modernize] Fix for modernize-redundant-void-arg: complains about variable cast to void.

LGTM with the nits corrected (there are still some formatting concerns).

Mon, Sep 17, 4:52 AM · Restricted Project

Tue, Sep 11

aaron.ballman accepted D51880: [ASTMatchers] add three matchers for dependent expressions.

LGTM!

Tue, Sep 11, 8:51 AM
aaron.ballman added a comment to D51880: [ASTMatchers] add three matchers for dependent expressions.

Missing tests and changes to Registry.cpp for dynamic matchers.

Also, do you want to add isInstantiationDependent() at the same time, given the relationship with the other two matchers?

Do you mean a matcher that does return Node.isValueDependent() || Node.isTypeDependent() or hasAncestor(expr(anyOf(isValueDependent(), isTypeDependent())))?

Tue, Sep 11, 5:19 AM

Mon, Sep 10

aaron.ballman added a comment to D51880: [ASTMatchers] add three matchers for dependent expressions.

Missing tests and changes to Registry.cpp for dynamic matchers.

Mon, Sep 10, 1:25 PM
aaron.ballman accepted D51853: Merge two attribute diagnostics into one.

LGTM!

Mon, Sep 10, 1:20 PM

Thu, Sep 6

aaron.ballman added a comment to D51192: Fix reported range of partial token replacement.

Thanks.

The arc tool already inserted Differential Revision: into my git commit, but that's not what I wonder about. I'm looking for something I can insert into my git commit so that the bug will automatically be closed when I commit (and so that the bug will get a link to this PR when the PR is created). Does such a thing exist here?

Thu, Sep 6, 1:11 PM
aaron.ballman accepted D51192: Fix reported range of partial token replacement.

As far as I know, no existing clang-tidy checks are affected. I discovered this while implementing a custom check. See https://bugs.llvm.org/show_bug.cgi?id=38678

Thu, Sep 6, 12:59 PM
aaron.ballman added a comment to D51192: Fix reported range of partial token replacement.

I'd probably pipe the diagnostic output to a temporary file that gets FileChecked with strict whitespace to see if the underlines from the diagnostic match the expected locations. We do this in a few Clang tests, like SemaCXX\struct-class-redecl.cpp or Misc\wrong-encoding.c.

Doesn't this require building-in a new check to clang-tidy which exists only for the purpose of the test? Otherwise how would a test similar to SemaCXX\struct-class-redecl.cpp work? What would be in the RUN line?

Thu, Sep 6, 12:46 PM
aaron.ballman added a comment to D51192: Fix reported range of partial token replacement.

How? This is 'private' code. I don't think it's worth changing that or creating a test with a huge amount of infrastructure in order to test this indirectly.

Thu, Sep 6, 12:09 PM
aaron.ballman added a reviewer for D51650: Implement target_clones multiversioning: rsmith.

Thank you for this! I have some cursory review comments, and possibly more later.

Thu, Sep 6, 10:36 AM
aaron.ballman accepted D51697: [Sema] Clean up some __builtin_*_chk diagnostics.

LGTM

Thu, Sep 6, 9:41 AM

Tue, Sep 4

aaron.ballman accepted D51507: Allow all supportable attributes to be used with #pragma clang attribute..

Thank you for pointing out that the new form of type attributes aren't automatically opted in from this change. With that (and the two problematic attributes opted out), this LGTM.

Tue, Sep 4, 1:33 PM
aaron.ballman closed D50610: Disable -Wnoexcept-type wholesale.

I've commit in r341361, thank you for the patch!

Tue, Sep 4, 5:07 AM

Fri, Aug 31

aaron.ballman added inline comments to D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space.
Fri, Aug 31, 5:32 AM · Restricted Project
aaron.ballman added inline comments to D51507: Allow all supportable attributes to be used with #pragma clang attribute..
Fri, Aug 31, 5:23 AM
aaron.ballman added a comment to D51192: Fix reported range of partial token replacement.

I think the fix generally looks good, but can you please add some test coverage for the change?

Fri, Aug 31, 5:16 AM
aaron.ballman added a comment to D51333: Diagnose likely typos in include statements.

Instead of guessing whether the corrected filename would be valid, why not strip off the leading and trailing non-alphanumeric characters, look up the resulting filename, and find out? If we did that, then not only could we be a lot more confident that we'd found the file that was intended, but we could also recover from the error by including the trimmed filename.

Fri, Aug 31, 5:05 AM

Thu, Aug 30

aaron.ballman accepted D51473: Improve attribute documentation to list which spellings are used in which syntaxes..

LGTM with a commenting nit. Thank you for this, I like this exposition better!

Thu, Aug 30, 11:10 AM
aaron.ballman added a reviewer for D51333: Diagnose likely typos in include statements: rsmith.
Thu, Aug 30, 10:42 AM
aaron.ballman added a comment to D46313: [clang] Add WriteOnly attribute.

I'd like to see some C++ tests that check reference behavior (and perhaps more esoteric pointerish non-pointer types like member function pointers).

Thu, Aug 30, 10:27 AM

Wed, Aug 29

aaron.ballman closed D51132: [clang-tidy] abseil-redundant-strcat-calls-check .

I've commit in r340918. Thank you for the patch!

Wed, Aug 29, 4:30 AM · Restricted Project
aaron.ballman added a comment to D51061: [clang-tidy] abseil-str-cat-append.

@aaron.ballman when you get the chance could you take another look at this and commit if it is ready? My internship ends rather soon and this is a tad time sensitive and I don't have commit access. Thank you for your time!

Wed, Aug 29, 4:21 AM · Restricted Project

Tue, Aug 28

aaron.ballman added inline comments to D51333: Diagnose likely typos in include statements.
Tue, Aug 28, 12:12 PM
aaron.ballman added a comment to D51132: [clang-tidy] abseil-redundant-strcat-calls-check .

@aaron.ballman when you get the chance could you take another look at this and commit if it is ready? My internship ends rather soon and this is a tad time sensitive. Thank you for your time!

Tue, Aug 28, 11:58 AM · Restricted Project

Mon, Aug 27

aaron.ballman added inline comments to D48714: [clang-tidy] fix PR37913, templated exception factory diagnosed correctly.
Mon, Aug 27, 5:47 AM
aaron.ballman added a comment to D51260: Extract runCommandsInFile method.

I'm not opposed to the changes here, but I am wondering what the benefits are to splitting this off into its own function?

Mon, Aug 27, 4:33 AM

Aug 24 2018

aaron.ballman accepted D51132: [clang-tidy] abseil-redundant-strcat-calls-check .

Aside from the formatting and one small documentation nit, LGTM!

Aug 24 2018, 10:27 AM · Restricted Project
aaron.ballman accepted D51061: [clang-tidy] abseil-str-cat-append.

LGTM!

Aug 24 2018, 10:24 AM · Restricted Project
aaron.ballman added inline comments to D51061: [clang-tidy] abseil-str-cat-append.
Aug 24 2018, 9:48 AM · Restricted Project
aaron.ballman added inline comments to D51132: [clang-tidy] abseil-redundant-strcat-calls-check .
Aug 24 2018, 9:45 AM · Restricted Project
aaron.ballman added a comment to D51187: [RFC] Thread safety analysis: Track status of scoped capability.

I'm not certain how I feel about this as it seems to be removing functionality users may be relying on that I think is still technically correct. Can you describe more about why you think this should change?

Aug 24 2018, 8:33 AM
aaron.ballman added inline comments to D51061: [clang-tidy] abseil-str-cat-append.
Aug 24 2018, 8:13 AM · Restricted Project
aaron.ballman added inline comments to D51132: [clang-tidy] abseil-redundant-strcat-calls-check .
Aug 24 2018, 7:24 AM · Restricted Project

Aug 23 2018

aaron.ballman accepted D50617: [ASTMatchers] Let hasObjectExpression also support UnresolvedMemberExpr, CXXDependentScopeMemberExpr.

LGTM!

Aug 23 2018, 6:37 AM

Aug 21 2018

aaron.ballman added a comment to D50994: Add a new flag and attributes to control static destructor registration .

Some minor nits that can be handled post-commit.

Aug 21 2018, 10:29 AM

Aug 14 2018

aaron.ballman accepted D49511: [Sema/Attribute] Check for noderef attribute.

A few more minor nits to be cleared up, but otherwise LGTM. You should wait for @rsmith to sign off before committing in case he has further comments, however.

Aug 14 2018, 12:48 PM · Restricted Project
aaron.ballman added inline comments to D50214: Add inherited attributes before parsed attributes..
Aug 14 2018, 12:34 PM

Aug 13 2018

aaron.ballman added inline comments to D49511: [Sema/Attribute] Check for noderef attribute.
Aug 13 2018, 1:11 PM · Restricted Project
aaron.ballman added inline comments to D49511: [Sema/Attribute] Check for noderef attribute.
Aug 13 2018, 11:37 AM · Restricted Project
aaron.ballman accepted D50647: [Sema] fix -Wfloat-conversion test case..

LGTM, though you don't need review for fixing bots.

Aug 13 2018, 10:51 AM
aaron.ballman added inline comments to D50580: [clang-tidy] Abseil: no namespace check.
Aug 13 2018, 4:54 AM · Restricted Project

Aug 12 2018

aaron.ballman accepted D50605: [ASTMatchers] Let hasAnyArgument also support CXXUnresolvedConstructExpr.

LGTM!

Aug 12 2018, 2:57 PM
aaron.ballman added inline comments to D50605: [ASTMatchers] Let hasAnyArgument also support CXXUnresolvedConstructExpr.
Aug 12 2018, 2:20 PM
aaron.ballman accepted D49910: [clang-tidy] Recognize [[clang::reinitializes]] attribute in bugprone-use-after-move.

LGTM! An attribute really is the right way to go for this sort of thing, and it sounds like you've got a base of users looking for the functionality.

Aug 12 2018, 8:35 AM · Restricted Project
aaron.ballman edited reviewers for D50214: Add inherited attributes before parsed attributes., added: rsmith; removed: nicholas, dlj, echristo.

In general, I think this is the right way to go. I've added @rsmith to the reviewers because I'm curious what his thoughts are on this approach, though.

Aug 12 2018, 8:26 AM
aaron.ballman accepted D50526: Model type attributes as regular Attrs.

LGTM! Thank you for this fantastic work!

Aug 12 2018, 8:00 AM · Restricted Project
aaron.ballman accepted D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis.

LGTM!

Aug 12 2018, 7:40 AM
aaron.ballman closed D49114: [clang-tidy] Add a check for "magic numbers".

Thank you for the patch and great discussion! I've commit in r339516.

Aug 12 2018, 7:36 AM
aaron.ballman added inline comments to D50542: [clang-tidy] Add abseil-no-internal-deps check.
Aug 12 2018, 7:21 AM · Restricted Project
aaron.ballman added inline comments to D50580: [clang-tidy] Abseil: no namespace check.
Aug 12 2018, 7:08 AM · Restricted Project
aaron.ballman accepted D49114: [clang-tidy] Add a check for "magic numbers".

Alex has had plenty of time to respond, so I'm fine handling any concerns he has post-commit. Do you need me to commit this on your behalf?

Aug 12 2018, 7:01 AM
aaron.ballman requested changes to D50605: [ASTMatchers] Let hasAnyArgument also support CXXUnresolvedConstructExpr.

Please be sure to regenerate the AST matcher documentation as well, as I'd expect to see some documentation changes from this.

Aug 12 2018, 6:58 AM
aaron.ballman accepted D50606: [ASTMatchers] Add matchers unresolvedMemberExpr, cxxDependentScopeMemberExpr.

LGTM, but please split out the unrelated changes into their own commit.

Aug 12 2018, 6:55 AM

Aug 10 2018

aaron.ballman added inline comments to D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis.
Aug 10 2018, 1:18 PM
aaron.ballman added a comment to D49511: [Sema/Attribute] Check for noderef attribute.

Oops, I hit "Send" too soon. I was going to say that you should also keep an eye on D50526 because that may impact this patch (or vice versa if this one lands first).

Aug 10 2018, 12:08 PM · Restricted Project
aaron.ballman added inline comments to D49511: [Sema/Attribute] Check for noderef attribute.
Aug 10 2018, 12:07 PM · Restricted Project
aaron.ballman requested changes to D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis.
Aug 10 2018, 11:53 AM
aaron.ballman added a comment to D50526: Model type attributes as regular Attrs.

Thank you for working on this -- the approach you've taken looks good, and I mostly have little nits here and there. I think this is a great refactoring overall though -- much needed!

Aug 10 2018, 11:48 AM · Restricted Project
aaron.ballman closed D49885: Thread safety analysis: Allow relockable scopes.

Committed in r339456; if @delesley has concerns with the commit, they can be addressed post-commit. Thank you for the patch!

Aug 10 2018, 10:34 AM
aaron.ballman closed D50055: Update the coding standard about NFC changes and whitespace.

Thank you for the review and discussion -- I've commit in r339455.

Aug 10 2018, 10:27 AM

Aug 6 2018

aaron.ballman added a comment to D45444: [clang-tidy] implement new check for const-correctness.

The functionality is looking good, aside from a few small nits remaining. However, I'm wondering how this should integrate with other const-correctness efforts like readability-non-const-parameter? Also, I'm wondering how this check performs over a large code base like LLVM -- how chatty are the diagnostics, and how bad is the false positive rate (roughly)?

Aug 6 2018, 6:04 AM

Aug 5 2018

aaron.ballman added a comment to D45444: [clang-tidy] implement new check for const-correctness.

I'm sorry for the delay in reviewing this; I'm not certain how it fell off my radar for so long!

Aug 5 2018, 7:47 AM
aaron.ballman added a comment to D49114: [clang-tidy] Add a check for "magic numbers".

I think this is close. If @alexfh doesn't chime in on the open question in the next few days, I would say the check is ready to go in and we can address the open question in follow-up commits.

Aug 5 2018, 7:32 AM

Aug 3 2018

aaron.ballman closed D50110: Handle shared release attributes correctly.

For now I think I'm done fixing things in the thread safety analysis, but if I see myself contributing more in the longer term, I will definitely try to obtain commit access.

Aug 3 2018, 12:38 PM
aaron.ballman added a comment to D49910: [clang-tidy] Recognize [[clang::reinitializes]] attribute in bugprone-use-after-move.

Are you going to propose adding this attribute to libc++, or is this expected to only work with UDTs?

I don't have any experience contributing to libc++, but I think this would make sense.

The check currently hard-codes various member functions of classes in the "std" namespace that do reinitializations; I'm not sure though if those can be removed after the attribute has been added to libc++. We'd would also presumably have to add the attribute to libstdc++ -- does it accept Clang-only attributes? And what is the story for people using clang-tidy with MSVC projects? (I have to admit I'm very hazy on how that works...)

Aug 3 2018, 11:26 AM · Restricted Project
aaron.ballman added a comment to D50110: Handle shared release attributes correctly.

Thanks for the review! Could you commit for me again?

Aug 3 2018, 11:17 AM
aaron.ballman accepted D50110: Handle shared release attributes correctly.

LGTM!

Aug 3 2018, 7:19 AM
aaron.ballman added a reviewer for D49910: [clang-tidy] Recognize [[clang::reinitializes]] attribute in bugprone-use-after-move: rsmith.

Are you going to propose adding this attribute to libc++, or is this expected to only work with UDTs?

Aug 3 2018, 7:17 AM · Restricted Project

Aug 2 2018

aaron.ballman added inline comments to D50110: Handle shared release attributes correctly.
Aug 2 2018, 1:34 PM
aaron.ballman accepted D48100: Append new attributes to the end of an AttributeList..

I am still okay with this, and agree that the ordering issues are a separate thing to tackle.

Aug 2 2018, 1:29 PM
aaron.ballman added inline comments to D46313: [clang] Add WriteOnly attribute.
Aug 2 2018, 1:20 PM