Page MenuHomePhabricator

aaron.ballman (Aaron Ballman)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Yesterday

aaron.ballman added inline comments to D55447: [Sema] Fix Modified Type in address_space AttributedType.
Tue, Jan 15, 1:14 PM · Restricted Project
aaron.ballman added inline comments to D56663: [MSP430] Improve support of 'interrupt' attribute.
Tue, Jan 15, 1:00 PM
aaron.ballman added inline comments to D55447: [Sema] Fix Modified Type in address_space AttributedType.
Tue, Jan 15, 1:00 PM · Restricted Project
aaron.ballman accepted D55483: Introduce the callback attribute and emit !callback metadata.

LGTM aside from some minor nits.

Tue, Jan 15, 12:53 PM
aaron.ballman added a reviewer for D56663: [MSP430] Improve support of 'interrupt' attribute: aaron.ballman.
Tue, Jan 15, 12:38 PM
aaron.ballman added inline comments to D56663: [MSP430] Improve support of 'interrupt' attribute.
Tue, Jan 15, 12:37 PM
aaron.ballman accepted D56657: [clang-tidy] bugprone-string-constructor: Catch string from nullptr..

LGTM aside from a nit and a test case request.

Tue, Jan 15, 12:33 PM · Restricted Project
aaron.ballman added inline comments to D56532: [clang-tidy] Add the abseil-duration-conversion-cast check.
Tue, Jan 15, 12:23 PM · Restricted Project
aaron.ballman added a comment to D56555: Add Attribute to define nonlazy objc classes.

In terms of implementation, the attribute code looks pretty close (just a few nits). However, someone more familiar with Obj-C should chime in with whether the attribute semantics make sense and whether the documentation will be sufficiently clear for an Objective-C user.

Tue, Jan 15, 12:06 PM
aaron.ballman accepted D55083: Re-arrange content in FunctionDecl dump.

LGTM aside from some nit cleanup.

Tue, Jan 15, 11:57 AM
aaron.ballman accepted D55394: Re-order type param children of ObjC nodes.

This is the AST dumper. This is not a user feature.

Tue, Jan 15, 11:22 AM
aaron.ballman accepted D56709: Implement BlockDecl::Capture dump in terms of visitors.

LGTM!

Tue, Jan 15, 11:11 AM
aaron.ballman accepted D56708: NFC: Implement OMPClause dump in terms of visitors.

LGTM aside from a nit.

Tue, Jan 15, 11:05 AM
aaron.ballman accepted D56707: Implement CXXCtorInitializer dump in terms of Visitor.

LGTM!

Tue, Jan 15, 10:59 AM

Mon, Jan 14

aaron.ballman accepted D56642: NFC: Move dump of type nodes to NodeDumper.

LGTM aside from auto nits.

Mon, Jan 14, 2:28 PM
aaron.ballman added a comment to D55394: Re-order type param children of ObjC nodes.

I don't really have an opinion about this, sorry.

Mon, Jan 14, 2:25 PM
aaron.ballman added a comment to D56555: Add Attribute to define nonlazy objc classes.

It's still missing the tests in SemaObjC -- you can use clang\test\SemaObjC\attr-root-class.m as an example of what I'm talking about.

Mon, Jan 14, 9:21 AM
aaron.ballman added a reviewer for D55394: Re-order type param children of ObjC nodes: rjmccall.

Adding @rjmccall as an Obj-C expert to see if he has opinions on the output changes. Also, pinging @rsmith in case he'd like to weigh in.

Mon, Jan 14, 9:11 AM
aaron.ballman added inline comments to D55483: Introduce the callback attribute and emit !callback metadata.
Mon, Jan 14, 7:04 AM
aaron.ballman added inline comments to D56642: NFC: Move dump of type nodes to NodeDumper.
Mon, Jan 14, 6:43 AM
aaron.ballman added a comment to D56642: NFC: Move dump of type nodes to NodeDumper.

Mostly looks good (a few minor nits), but I did have a design question about whether we should prefer calling a Visit method for type hierarchies instead of reimplementing the functionality.

Mon, Jan 14, 6:26 AM
aaron.ballman accepted D56643: NFC: Move Decl node handling to TextNodeDumper.

LGTM aside from some minor nits.

Mon, Jan 14, 5:58 AM
aaron.ballman accepted D56641: NFC: Move dumping of QualType node to TextNodeDumper.

LGTM!

Mon, Jan 14, 5:49 AM
aaron.ballman accepted D56640: NFC: Canonicalize handling of TypeLocInfo.

LGTM!

Mon, Jan 14, 5:46 AM
aaron.ballman accepted D56639: NFC: Move Type Visit implementation to TextNodeDumper.

LGTM aside from some minor nits.

Mon, Jan 14, 5:42 AM

Fri, Jan 11

aaron.ballman accepted D55491: Implement TemplateArgument dumping in terms of Visitor.

Aside from nits that aren't complete yet, LGTM.

Fri, Jan 11, 6:10 PM
aaron.ballman accepted D56622: [Algorithm] Add make_const_ref corresponding to make_const_ptr.

LGTM!

Fri, Jan 11, 6:07 PM
aaron.ballman added inline comments to D55483: Introduce the callback attribute and emit !callback metadata.
Fri, Jan 11, 7:02 AM
aaron.ballman requested changes to D56555: Add Attribute to define nonlazy objc classes.

The patch is missing SemaObjC tests that ensure the attribute only appertains to the expected subjects, accepts no args, etc.

Fri, Jan 11, 5:41 AM
aaron.ballman accepted D56292: [attributes] Extend os_returns_(not_?)_retained attributes to parameters..

LGTM aside from some minor nits.

Fri, Jan 11, 5:35 AM
aaron.ballman accepted D55492: Implement Attr dumping in terms of visitors.

LGTM!

Fri, Jan 11, 5:23 AM

Thu, Jan 10

aaron.ballman closed D54450: Get the correct range of tokens for preprocessor conditions.

I've commit in r350891 (clang part) and r350892 (clang-tools-extra part). If @rsmith has concerns, we can fix or revert when raised.

Thu, Jan 10, 1:27 PM
aaron.ballman added inline comments to D56473: Allow 'static' storage specifier on an out-of-line member function template declaration in MSVCCompat mode.
Thu, Jan 10, 12:19 PM
aaron.ballman accepted D56552: [clang-tidy] update FunctionSizeCheck for D56444.

LGTM!

Thu, Jan 10, 10:36 AM
aaron.ballman accepted D56444: [AST] RecursiveASTVisitor visits lambda classes when implicit visitation is on..

LGTM with one minor testing request (conditional on the test passing, of course).

Thu, Jan 10, 10:30 AM
aaron.ballman added a comment to D55646: [ASTImporter] Make ODR diagnostics warning by default.

In general, I agree that it doesn't make sense for ODR violations (with false positives) to trigger error diagnostics while performing AST merging, so downgrading this to warnings is a good idea. However, I do worry about impacting modules and C compatibility with the changes. I'm curious what @rsmith thinks the answer should be here. AFAIK, modules do not *require* ODR violations checks (I believe that's still ill-formed, no diagnostic required), but confirmation would be good.

Thu, Jan 10, 10:30 AM
aaron.ballman added inline comments to D55491: Implement TemplateArgument dumping in terms of Visitor.
Thu, Jan 10, 9:51 AM
aaron.ballman accepted D56405: Split -Wdelete-non-virtual-dtor into two groups.

LGTM!

Thu, Jan 10, 8:03 AM
aaron.ballman added inline comments to D55491: Implement TemplateArgument dumping in terms of Visitor.
Thu, Jan 10, 7:58 AM
aaron.ballman added inline comments to D55492: Implement Attr dumping in terms of visitors.
Thu, Jan 10, 7:41 AM
aaron.ballman accepted D55488: Add utility for dumping a label with child nodes.

Aside from cosmetic nits, this LGTM.

Thu, Jan 10, 7:17 AM
aaron.ballman accepted D56533: [DOCS] it it => it.

LGTM!

Thu, Jan 10, 5:09 AM

Wed, Jan 9

aaron.ballman added a comment to D56444: [AST] RecursiveASTVisitor visits lambda classes when implicit visitation is on..

Can you add a test that checks that a lambda declared within another lambda also traverses as expected?

Wed, Jan 9, 7:50 AM
aaron.ballman added a comment to D55488: Add utility for dumping a label with child nodes.

@rsmith -- do you have opinions on the new format for how we display the child node? I think this is a more clear approach, but a second opinion would be nice.

Wed, Jan 9, 7:21 AM
aaron.ballman added a comment to D56444: [AST] RecursiveASTVisitor visits lambda classes when implicit visitation is on..

I suggest not trying to make any such drastic changes for 8.0, try to fix the bug in a minimal way if possible, and have a more considered approach to the future of AST Matchers for after the release.

Wed, Jan 9, 7:05 AM
aaron.ballman added a comment to D56444: [AST] RecursiveASTVisitor visits lambda classes when implicit visitation is on..

Given that, I kind of think we should have functionDecl() match only functions, and give users some other way to match the semantic declarations in a consistent manner. Alternatively, we could decide semantics are what we want to match (because it's what the AST encodes) and instead we give users a way to request to only match syntax.

I believe matching the implied semantic nodes is how closer to how matchers behave in general (corresponding to the fact that the ASTMatcher RecursiveASTVisitor sets shouldVisitImplicitCode to true). e.g.

Wed, Jan 9, 7:02 AM
aaron.ballman added a comment to D56444: [AST] RecursiveASTVisitor visits lambda classes when implicit visitation is on..

@klimek: would it be better to preserve the odd behavior of the functionDecl() matcher, and add a new functionOrLambdaDecl()? It seems too surprising to me.

We can change the clang-tidy check as well. I think lambdas should be considered functionDecls, shouldnt they? WDYT @aaron.ballman ?

I think it depends on whether we want the AST matchers to match what the user wrote (syntax) or how the program behaves (semantics). If the user writes a lambda, they did not syntactically write a function declaration and so it makes sense for functionDecl() to not match. However, the semantics of a lambda are that they act as a functor (in effect) which includes a class declaration, a function declaration for the function call operator, conversion operators, etc and so it does make sense for users to want to match those semantic components. Given that, I kind of think we should have functionDecl() match only functions, and give users some other way to match the semantic declarations in a consistent manner. Alternatively, we could decide semantics are what we want to match (because it's what the AST encodes) and instead we give users a way to request to only match syntax.

The problem is that while I agree it would be nice if we matched either exactly the semantic or syntactic form of the code, the reality is that we match whatever the AST represents when we visit all nodes. Instead o f promising folks something that we can't hold (because nobody has time to fully check which matchers will match which form), I think telling people that we match the AST is the more honest and less surprising result in the end.

Wed, Jan 9, 6:56 AM
aaron.ballman added a comment to D56444: [AST] RecursiveASTVisitor visits lambda classes when implicit visitation is on..

@klimek: would it be better to preserve the odd behavior of the functionDecl() matcher, and add a new functionOrLambdaDecl()? It seems too surprising to me.

We can change the clang-tidy check as well. I think lambdas should be considered functionDecls, shouldnt they? WDYT @aaron.ballman ?

Wed, Jan 9, 6:30 AM

Tue, Jan 8

aaron.ballman accepted D55337: NFC: Move dumpDeclRef to NodeDumper.

Once D56407 lands, the rebased changes here LGTM

Tue, Jan 8, 10:32 AM
aaron.ballman accepted D56415: NFC: Port QueryParser to StringRef.

LGTM aside from the nits pointed out; you can fix and commit without another round of review.

Tue, Jan 8, 9:09 AM
aaron.ballman added inline comments to D56415: NFC: Port QueryParser to StringRef.
Tue, Jan 8, 8:56 AM
aaron.ballman added inline comments to D56415: NFC: Port QueryParser to StringRef.
Tue, Jan 8, 8:46 AM
aaron.ballman added inline comments to D55337: NFC: Move dumpDeclRef to NodeDumper.
Tue, Jan 8, 8:29 AM
aaron.ballman added inline comments to D56415: NFC: Port QueryParser to StringRef.
Tue, Jan 8, 8:05 AM
aaron.ballman accepted D56407: Implement the TreeStructure interface through the TextNodeDumper.

I think this is a good approach to solving the problem, so let's go this route.

Tue, Jan 8, 7:15 AM
aaron.ballman added inline comments to D56405: Split -Wdelete-non-virtual-dtor into two groups.
Tue, Jan 8, 6:32 AM
aaron.ballman added inline comments to D56292: [attributes] Extend os_returns_(not_?)_retained attributes to parameters..
Tue, Jan 8, 6:20 AM
aaron.ballman added inline comments to D54909: [clang][slh] add Clang attr no_speculative_load_hardening.
Tue, Jan 8, 5:50 AM

Mon, Jan 7

aaron.ballman added inline comments to D56405: Split -Wdelete-non-virtual-dtor into two groups.
Mon, Jan 7, 4:59 PM
aaron.ballman accepted D56405: Split -Wdelete-non-virtual-dtor into two groups.

LGTM!

Mon, Jan 7, 2:00 PM
aaron.ballman accepted D56354: [AST] Replace asserts with if() in SourceLocation accessors.

LGTM aside from a formatting nit.

Mon, Jan 7, 1:11 PM
aaron.ballman accepted D56395: [ASTMatchers] Improve assert message for broken parent map..

LGTM!

Mon, Jan 7, 10:10 AM
aaron.ballman added reviewers for D55212: Handle alloc_size attribute on function pointers: erik.pilkington, dexonsmith.

I don't see an easy way of fixing the pragma clang attribute support for this.
Would it be okay to commit this change anyway?

Mon, Jan 7, 7:59 AM
aaron.ballman added a comment to D56354: [AST] Replace asserts with if() in SourceLocation accessors.

The downside to this change is that callers now have to check the return values for validity where they didn't previously because the assertion covered it. However, I think that's probably reasonable in these cases since they're mostly returning source locations or ranges.

Mon, Jan 7, 5:55 AM

Sun, Jan 6

aaron.ballman added inline comments to D56303: [clang-tidy] Handle case/default statements when simplifying boolean expressions.
Sun, Jan 6, 7:43 AM · Restricted Project

Sat, Jan 5

aaron.ballman added inline comments to D56303: [clang-tidy] Handle case/default statements when simplifying boolean expressions.
Sat, Jan 5, 10:43 AM · Restricted Project
aaron.ballman added a comment to D56301: Enhance MakeSmartPtrCheck for work with class hierarchies.

@hokein Is there any policy about the question "Which changes can clang-tidy check suggest?" . Should clang-tidy check suggest only "safe" fixes and warn only in "safe" cases?

Sat, Jan 5, 10:35 AM
aaron.ballman added inline comments to D56292: [attributes] Extend os_returns_(not_?)_retained attributes to parameters..
Sat, Jan 5, 10:27 AM
aaron.ballman accepted D56090: Add a matcher for members of an initializer list expression.

LGTM! Thanks, both!

Sat, Jan 5, 7:14 AM · Restricted Project

Fri, Jan 4

aaron.ballman accepted D55865: [ObjC] Add a new attribute to opt-out of implicit callee retain/release in ARC.

LGTM aside from a few minor nits/questions.

Fri, Jan 4, 9:58 AM
aaron.ballman closed D55710: add pragmas to control Software Pipelining optimisation.

I commit this in r350414, thank you for the patch!

Fri, Jan 4, 9:28 AM
aaron.ballman closed D55955: Properly diagnose [[nodiscard]] on the body of a range-based for loop.

Committed with minor modifications for the range-based for loop changes in r350404.

Fri, Jan 4, 9:02 AM
aaron.ballman added inline comments to D55337: NFC: Move dumpDeclRef to NodeDumper.
Fri, Jan 4, 8:33 AM
aaron.ballman added inline comments to D55955: Properly diagnose [[nodiscard]] on the body of a range-based for loop.
Fri, Jan 4, 8:07 AM

Thu, Jan 3

aaron.ballman accepted D55710: add pragmas to control Software Pipelining optimisation.

LGTM!

Thu, Jan 3, 12:18 PM
aaron.ballman added inline comments to D55710: add pragmas to control Software Pipelining optimisation.
Thu, Jan 3, 9:47 AM
aaron.ballman added a reviewer for D56090: Add a matcher for members of an initializer list expression: klimek.

Adding Manuel for the hasArg() questions.

Thu, Jan 3, 9:43 AM · Restricted Project
aaron.ballman updated subscribers of D56090: Add a matcher for members of an initializer list expression.
Thu, Jan 3, 9:42 AM · Restricted Project
aaron.ballman added inline comments to D55932: [Sema] Simplfy static_assert diagnostic code..
Thu, Jan 3, 9:10 AM
aaron.ballman closed D55949: Correctly handle function pointers returning a type marked nodiscard.

Committed in r350317.

Thu, Jan 3, 6:29 AM

Wed, Jan 2

aaron.ballman added a comment to D55949: Correctly handle function pointers returning a type marked nodiscard.

Ping

Wed, Jan 2, 12:24 PM
aaron.ballman added inline comments to D55955: Properly diagnose [[nodiscard]] on the body of a range-based for loop.
Wed, Jan 2, 12:24 PM
aaron.ballman updated the diff for D55955: Properly diagnose [[nodiscard]] on the body of a range-based for loop.

Updated based on review feedback. The vast majority of the changes are from the removal of a default argument to ActOnFullExpr().

Wed, Jan 2, 12:20 PM
aaron.ballman added inline comments to D55932: [Sema] Simplfy static_assert diagnostic code..
Wed, Jan 2, 8:32 AM
aaron.ballman added inline comments to D56160: [clang-tidy] modernize-use-trailing-return check.
Wed, Jan 2, 8:09 AM · Restricted Project
aaron.ballman added inline comments to D55710: add pragmas to control Software Pipelining optimisation.
Wed, Jan 2, 8:03 AM
aaron.ballman added inline comments to D55932: [Sema] Simplfy static_assert diagnostic code..
Wed, Jan 2, 7:48 AM
aaron.ballman accepted D56186: Fix MSVC PointerUnion visualization.

This looks great to me (aside from a minor commenting nit). Thank you for the fix!

Wed, Jan 2, 6:58 AM

Mon, Dec 31

aaron.ballman added inline comments to D54408: [ASTMatchers] Add matchers available through casting to derived.
Mon, Dec 31, 8:13 AM
aaron.ballman requested changes to D56090: Add a matcher for members of an initializer list expression.

@lebedev.ri Where do the appropriate tests live? (I couldn't find an obvious subdirectory in test/)

Mon, Dec 31, 7:12 AM · Restricted Project
aaron.ballman added a comment to D55710: add pragmas to control Software Pipelining optimisation.

The docs are getting much closer -- just a few grammatical things left to fix, I believe.

Mon, Dec 31, 7:06 AM

Mon, Dec 24

aaron.ballman added inline comments to D55955: Properly diagnose [[nodiscard]] on the body of a range-based for loop.
Mon, Dec 24, 11:06 AM
aaron.ballman updated the diff for D55955: Properly diagnose [[nodiscard]] on the body of a range-based for loop.

Updated based on review feedback.

Mon, Dec 24, 9:42 AM

Sun, Dec 23

aaron.ballman added a reviewer for D55710: add pragmas to control Software Pipelining optimisation: tonic.

Adding @tonic as a reviewer, who may have suggestions on how to improve the documentation wording.

Sun, Dec 23, 7:02 AM

Fri, Dec 21

aaron.ballman accepted D56012: [clang-tidy] Be more liberal about literal zeroes in abseil checks.

LGTM!

Fri, Dec 21, 1:06 PM · Restricted Project
aaron.ballman requested review of D55949: Correctly handle function pointers returning a type marked nodiscard.

There's enough churn based on the review feedback that this should probably have a second round of review just to be sure.

Fri, Dec 21, 12:57 PM
aaron.ballman added inline comments to D56012: [clang-tidy] Be more liberal about literal zeroes in abseil checks.
Fri, Dec 21, 12:08 PM · Restricted Project
aaron.ballman added inline comments to D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403).
Fri, Dec 21, 11:57 AM · Restricted Project
aaron.ballman added inline comments to D55865: [ObjC] Add a new attribute to opt-out of implicit callee retain/release in ARC.
Fri, Dec 21, 11:52 AM
aaron.ballman added inline comments to D55710: add pragmas to control Software Pipelining optimisation.
Fri, Dec 21, 11:43 AM