aaron.ballman (Aaron Ballman)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Sat, Feb 17

aaron.ballman added inline comments to D43392: [clang-tidy] Add Fuchsia checker for visibility attributes.
Sat, Feb 17, 7:02 AM · Restricted Project

Fri, Feb 16

aaron.ballman added inline comments to D43392: [clang-tidy] Add Fuchsia checker for visibility attributes.
Fri, Feb 16, 1:22 PM · Restricted Project
aaron.ballman added inline comments to D43248: [Attr] Fix printing of parameter indices in attributes.
Fri, Feb 16, 10:32 AM
aaron.ballman added inline comments to D43248: [Attr] Fix printing of parameter indices in attributes.
Fri, Feb 16, 9:56 AM
aaron.ballman added inline comments to D43392: [clang-tidy] Add Fuchsia checker for visibility attributes.
Fri, Feb 16, 9:37 AM · Restricted Project
aaron.ballman accepted D43359: Clean up 'target' attribute diagnostics.

Aside from a minor wording nit, LGTM!

Fri, Feb 16, 9:25 AM
aaron.ballman added inline comments to D43359: Clean up 'target' attribute diagnostics.
Fri, Feb 16, 8:42 AM
aaron.ballman added inline comments to D42730: [clang-tidy]] Add check for use of types/classes/functions from <functional> header which are deprecated and removed in C++17.
Fri, Feb 16, 7:38 AM · Restricted Project
aaron.ballman added inline comments to D41880: Adding nocf_check attribute for cf-protection fine tuning .
Fri, Feb 16, 7:34 AM
aaron.ballman added inline comments to D43248: [Attr] Fix printing of parameter indices in attributes.
Fri, Feb 16, 7:23 AM
aaron.ballman added inline comments to D43352: Add support for __declspec(code_seg("segname")).
Fri, Feb 16, 7:03 AM
aaron.ballman added inline comments to D41880: Adding nocf_check attribute for cf-protection fine tuning .
Fri, Feb 16, 6:18 AM
aaron.ballman added inline comments to D43359: Clean up 'target' attribute diagnostics.
Fri, Feb 16, 5:52 AM

Thu, Feb 15

aaron.ballman added inline comments to D43248: [Attr] Fix printing of parameter indices in attributes.
Thu, Feb 15, 10:16 AM
aaron.ballman added inline comments to D43248: [Attr] Fix printing of parameter indices in attributes.
Thu, Feb 15, 9:50 AM
aaron.ballman added inline comments to D41880: Adding nocf_check attribute for cf-protection fine tuning .
Thu, Feb 15, 8:50 AM
aaron.ballman added inline comments to D41880: Adding nocf_check attribute for cf-protection fine tuning .
Thu, Feb 15, 5:57 AM

Wed, Feb 14

aaron.ballman accepted D43321: Improve documentation for attribute artificial.

LGTM, thanks!

Wed, Feb 14, 3:00 PM
aaron.ballman added a comment to D41880: Adding nocf_check attribute for cf-protection fine tuning .

One thing I notice is that GCC trunk requires passing the -fcf-protection flag in order to enable the attribute. Do we wish to do the same?

Wed, Feb 14, 5:37 AM
aaron.ballman accepted D43120: [clang-tidy] New checker for exceptions that are created but not thrown.

LGTM, thank you!

Wed, Feb 14, 5:10 AM · Restricted Project

Tue, Feb 13

aaron.ballman added a reviewer for D41880: Adding nocf_check attribute for cf-protection fine tuning : aaron.ballman.
Tue, Feb 13, 12:41 PM
aaron.ballman added inline comments to D42624: [clang-tidy] Add a utility Matcher to match the next statement within a statement sequence.
Tue, Feb 13, 10:18 AM · Restricted Project
aaron.ballman requested changes to D43120: [clang-tidy] New checker for exceptions that are created but not thrown.

I apologize for not noticing this important detail earlier -- I think this check should live under bugprone rather than misc. Other than where it lives (and the related naming issues), I think this is looking good!

Tue, Feb 13, 9:58 AM · Restricted Project

Mon, Feb 12

aaron.ballman added a comment to D43120: [clang-tidy] New checker for exceptions that are created but not thrown.

I also came up with this problem:

 RegularException funcReturningExceptioniTest(int i) {
   return RegularException();
 }
 
 void returnedValueTest() {
   funcReturningExceptioniTest(3); //Should this emit a warning?
}

I'm not sure whether it'd be a good idea to warn about these cases. Unused return values can be found by many other means, and I'm afraid the standard library is filled with these cases.

Mon, Feb 12, 10:28 AM · Restricted Project
aaron.ballman closed D41553: Support parsing double square-bracket attributes in ObjC.

Thanks for the detailed review! I've commit in r324890.

Mon, Feb 12, 5:40 AM

Sun, Feb 11

aaron.ballman updated the diff for D41553: Support parsing double square-bracket attributes in ObjC.

Added the remaining ObjC, NS, and CF attributes. I think that's the last of the Apple-related attributes (I plan to continue to work on the other attributes as well, but in separate patches). @rjmccall, do you see any remaining issues?

Sun, Feb 11, 7:54 AM
aaron.ballman accepted D43095: Make attribute-target on a Definition-after-use update the LLVM attributes.

This looks reasonable to me, but you should wait a few days to commit in case someone more into CodeGen has comments.

Sun, Feb 11, 6:06 AM
aaron.ballman added inline comments to D42730: [clang-tidy]] Add check for use of types/classes/functions from <functional> header which are deprecated and removed in C++17.
Sun, Feb 11, 5:59 AM · Restricted Project

Fri, Feb 9

aaron.ballman added inline comments to D43120: [clang-tidy] New checker for exceptions that are created but not thrown.
Fri, Feb 9, 10:22 AM · Restricted Project
aaron.ballman added a comment to D43120: [clang-tidy] New checker for exceptions that are created but not thrown.

One concern I have is with RAII objects with "exception" in the name. You may already properly handle this, but I'd like to see a test case like:

struct ExceptionRAII {
  ExceptionRAII() {}
  ~ExceptionRAII() {}
};
Fri, Feb 9, 7:35 AM · Restricted Project

Wed, Feb 7

aaron.ballman accepted D42887: [Driver] Add option to manually control discarding value names in LLVM IR..

LGTM!

Wed, Feb 7, 10:35 AM

Tue, Feb 6

aaron.ballman added inline comments to D42730: [clang-tidy]] Add check for use of types/classes/functions from <functional> header which are deprecated and removed in C++17.
Tue, Feb 6, 6:49 AM · Restricted Project
aaron.ballman requested changes to D42730: [clang-tidy]] Add check for use of types/classes/functions from <functional> header which are deprecated and removed in C++17.

A few comments were not applied, and I'd like a more descriptive name for the check (especially if we plan to generalize this).

Tue, Feb 6, 6:25 AM · Restricted Project
aaron.ballman added a comment to D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers.

It sounds to me like the check is working as designed and that the user code was already broken, but it happened to work at runtime because the stars aligned properly for the user. Am I misunderstanding something?

Tue, Feb 6, 4:59 AM
aaron.ballman accepted D42561: [PR36008] Avoid -Wsign-compare warning for enum constants in typeof expressions.

LGTM, aside from a minor commenting nit.

Tue, Feb 6, 4:49 AM · Restricted Project

Sun, Feb 4

aaron.ballman added inline comments to D42887: [Driver] Add option to manually control discarding value names in LLVM IR..
Sun, Feb 4, 7:32 AM

Sat, Feb 3

aaron.ballman accepted D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions.

This LGTM with a few minor nits to fix.

Sat, Feb 3, 6:59 AM · Restricted Project

Fri, Feb 2

aaron.ballman accepted D42829: Emit label names according to -discard-value-names..

LGTM with a small formatting nit in the test.

Fri, Feb 2, 7:19 AM
aaron.ballman added inline comments to D42730: [clang-tidy]] Add check for use of types/classes/functions from <functional> header which are deprecated and removed in C++17.
Fri, Feb 2, 7:17 AM · Restricted Project
aaron.ballman added a comment to D41655: [clang-tidy] New check bugprone-unused-return-value.

The checker reports 7 warnings on LLVM + Clang code bases, all on std::unique_ptr::release:

lib/Bitcode/Reader/BitReader.cpp:114:3

  • release() called on moved-from unique_ptr
  • no harm, just unnecessary
Fri, Feb 2, 6:34 AM · Restricted Project

Thu, Feb 1

aaron.ballman added inline comments to D42730: [clang-tidy]] Add check for use of types/classes/functions from <functional> header which are deprecated and removed in C++17.
Thu, Feb 1, 9:31 AM · Restricted Project

Wed, Jan 31

aaron.ballman added inline comments to D42730: [clang-tidy]] Add check for use of types/classes/functions from <functional> header which are deprecated and removed in C++17.
Wed, Jan 31, 1:55 PM · Restricted Project
aaron.ballman added inline comments to D42730: [clang-tidy]] Add check for use of types/classes/functions from <functional> header which are deprecated and removed in C++17.
Wed, Jan 31, 9:44 AM · Restricted Project
aaron.ballman added inline comments to D42730: [clang-tidy]] Add check for use of types/classes/functions from <functional> header which are deprecated and removed in C++17.
Wed, Jan 31, 8:12 AM · Restricted Project
aaron.ballman added inline comments to D42730: [clang-tidy]] Add check for use of types/classes/functions from <functional> header which are deprecated and removed in C++17.
Wed, Jan 31, 6:06 AM · Restricted Project

Mon, Jan 29

aaron.ballman added inline comments to D41553: Support parsing double square-bracket attributes in ObjC.
Mon, Jan 29, 1:18 PM
aaron.ballman updated the diff for D41553: Support parsing double square-bracket attributes in ObjC.

Updates based on review feedback.

Mon, Jan 29, 1:09 PM
aaron.ballman added inline comments to D42624: [clang-tidy] Add a utility Matcher to match the next statement within a statement sequence.
Mon, Jan 29, 5:28 AM · Restricted Project
aaron.ballman added inline comments to D42623: [clang-tidy] Add a Lexer util to get the source text of a statement.
Mon, Jan 29, 5:12 AM · Restricted Project
aaron.ballman added inline comments to D41720: [clang-tidy] Add a -show-color flag..
Mon, Jan 29, 5:07 AM · Restricted Project

Fri, Jan 26

aaron.ballman added inline comments to D42561: [PR36008] Avoid -Wsign-compare warning for enum constants in typeof expressions.
Fri, Jan 26, 8:03 AM · Restricted Project

Thu, Jan 25

aaron.ballman requested changes to D42520: Implement __attribute__((import_module("foo"))).

You're missing a lot of test cases for the semantic checks of the attribute in Clang. There should be a Sema test that ensures things like the attribute subject, its argument count and types, etc.

Thu, Jan 25, 5:35 AM

Wed, Jan 24

aaron.ballman added inline comments to D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions.
Wed, Jan 24, 11:17 AM · Restricted Project

Tue, Jan 23

aaron.ballman accepted D42426: [clang-tidy] Handle bitfields in cppcoreguidelines-pro-type-member-init if using C++2a.

LGTM!

Tue, Jan 23, 8:56 AM
aaron.ballman accepted D42413: [clang-tidy] Handle bitfields in modernize-use-default-member-init if using C++2a.

LGTM!

Tue, Jan 23, 7:30 AM
aaron.ballman added inline comments to D42413: [clang-tidy] Handle bitfields in modernize-use-default-member-init if using C++2a.
Tue, Jan 23, 6:06 AM

Mon, Jan 22

aaron.ballman added inline comments to D40787: [clang-tidy] Replace the usage of std::uncaught_exception with std::uncaught_exceptions.
Mon, Jan 22, 12:09 PM · Restricted Project
aaron.ballman accepted D42273: Add hasTrailingReturn AST matcher.

LGTM!

Mon, Jan 22, 11:52 AM
aaron.ballman accepted D42213: [ASTMatchers] [NFC] Fix code examples.

LGTM, thank you! Do you need me to commit on your behalf?

Mon, Jan 22, 11:52 AM

Sat, Jan 20

aaron.ballman added a comment to D42213: [ASTMatchers] [NFC] Fix code examples.

Great, all that remains is for you to regenerate the documentation file and upload that with the next patch.

Sat, Jan 20, 4:55 AM
aaron.ballman added inline comments to D42273: Add hasTrailingReturn AST matcher.
Sat, Jan 20, 4:55 AM

Jan 19 2018

aaron.ballman added a comment to D42213: [ASTMatchers] [NFC] Fix code examples.

Yes, I was asking about this because the results seemed to be wrong. I should have used parameterCountIs() to reduce the confusion :) I'll leave them untouched.

Jan 19 2018, 2:03 PM
aaron.ballman added a comment to D42213: [ASTMatchers] [NFC] Fix code examples.

I am also not sure about this function: line 3548

/// \brief Matches \c FunctionDecls and \c FunctionProtoTypes that have a
/// specific parameter count.
///
/// Given
/// \code
///   void f(int i) {}
///   void g(int i, int j) {}
///   void h(int i, int j);
///   void j(int i);
///   void k(int x, int y, int z, ...);
/// \endcode
/// functionDecl(parameterCountIs(2))
///   matches void g(int i, int j) {}
/// functionProtoType(parameterCountIs(2))
///   matches void h(int i, int j)
/// functionProtoType(parameterCountIs(3))
///   matches void k(int x, int y, int z, ...);
AST_POLYMORPHIC_MATCHER_P(parameterCountIs,
                          AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl,
                                                          FunctionProtoType),
}

Both functionDecl and functionProtoType match these functions as long as the parameter count is matched.

% echo 'match functionDecl()'|clang-query =(printf 'void f(){}') -- -xc++
% echo 'match functionProtoType()'|clang-query =(printf 'void f(){}') -- -xc++
Jan 19 2018, 12:09 PM
aaron.ballman accepted D42273: Add hasTrailingReturn AST matcher.

LGTM with a minor commenting/documentation nit.

Jan 19 2018, 11:56 AM
aaron.ballman added a comment to D42213: [ASTMatchers] [NFC] Fix code examples.

The documentation needs to be regenerated for this patch. One thing that seems to be inconsistent is with the "what gets matched" messages is that sometimes it includes extra adornments like curly braces and other times it does not. It might be good to pick a style and try to be more consistent with it.

Do I need to do anything to re-generate the doc and check it into this revision? If so, can you show me the Doxygen generation instruction?

Jan 19 2018, 11:26 AM
aaron.ballman added inline comments to D42273: Add hasTrailingReturn AST matcher.
Jan 19 2018, 5:53 AM
aaron.ballman added a comment to D42213: [ASTMatchers] [NFC] Fix code examples.

The documentation needs to be regenerated for this patch. One thing that seems to be inconsistent is with the "what gets matched" messages is that sometimes it includes extra adornments like curly braces and other times it does not. It might be good to pick a style and try to be more consistent with it.

Jan 19 2018, 5:51 AM
aaron.ballman accepted D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance.

LGTM aside from a few small nits.

Jan 19 2018, 5:43 AM · Restricted Project

Jan 18 2018

aaron.ballman added reviewers for D42234: [ADT] Add PropagateConst from LFTS2: dblaikie, echristo.
Jan 18 2018, 10:33 AM
aaron.ballman added a comment to D42234: [ADT] Add PropagateConst from LFTS2.

What is the use case within LLVM and Clang, if you have a specific use in mind?

Jan 18 2018, 10:32 AM

Jan 17 2018

aaron.ballman accepted D42116: [clang-tidy] Adding Fuchsia checker for trailing returns.

LGTM! If you felt so inclined, I would say those two AST matchers would both be reasonable candidates to add to ASTMatchers.h if you wanted to do that in a follow-up patch.

Jan 17 2018, 12:03 PM · Restricted Project
aaron.ballman added a comment to D41455: [ASTMatchers] Add isNoReturn() match narrower for FunctionDeclarations.

Great, looks good to commit. Thanks, Roman!

Jan 17 2018, 11:25 AM · Restricted Project
aaron.ballman added inline comments to D42116: [clang-tidy] Adding Fuchsia checker for trailing returns.
Jan 17 2018, 11:25 AM · Restricted Project
aaron.ballman requested changes to D42185: [ASTMatcher] Add isScoped matcher for enumDecl..

This needs to be added to the dynamic matcher registry and also should not be commit until the documentation is generated for it.

Jan 17 2018, 8:32 AM
aaron.ballman added inline comments to D42116: [clang-tidy] Adding Fuchsia checker for trailing returns.
Jan 17 2018, 7:35 AM · Restricted Project
aaron.ballman accepted D41921: [Parse] Forward brace locations to TypeConstructExpr.

LGTM!

Jan 17 2018, 6:59 AM

Jan 16 2018

aaron.ballman added a comment to D42116: [clang-tidy] Adding Fuchsia checker for trailing returns.

Can you give some background on what problem the coding standard is trying to avoid by banning this? For instance, if trailing return types are bad, are deduced return types similarly bad, or are those fine?

The main concern is that of readability. Deduced return types can help readability, so those are okay (within reason).

Jan 16 2018, 2:10 PM · Restricted Project
aaron.ballman accepted D42120: [clang-tidy] Fixing Fuchsia overloaded operator warning message.

LGTM!

Jan 16 2018, 11:01 AM · Restricted Project
aaron.ballman added a comment to D42116: [clang-tidy] Adding Fuchsia checker for trailing returns.

Can you give some background on what problem the coding standard is trying to avoid by banning this? For instance, if trailing return types are bad, are deduced return types similarly bad, or are those fine?

Jan 16 2018, 10:53 AM · Restricted Project
aaron.ballman accepted D41815: [clang-tidy] implement check for goto.

LGTM aside from a minor formatting nit.

Jan 16 2018, 8:47 AM

Jan 15 2018

aaron.ballman added a comment to D41648: [clang-tidy] implement cppcoreguidelines macro rules.

My gut reaction is still that this check is going to be too chatty on real world code bases to be of value beyond compliance with the C++ Core Guidelines, but that's also sufficient enough reason to have the check in the first place. Perhaps a reasonable approach would be to put in the heuristics you think make the most sense, then run the check over some large real world C++ code bases to see how many diagnostics are generated. That will also help you to see code patterns to modify your heuristic until you're happy with the results. Then we can debate whether there's more left to do from the data.

Jan 15 2018, 12:32 PM
aaron.ballman added inline comments to D41815: [clang-tidy] implement check for goto.
Jan 15 2018, 12:23 PM
aaron.ballman accepted D39454: [clang-tidy] Misc redundant expressions checker updated for confused operators.

LGTM!

Jan 15 2018, 12:11 PM

Jan 13 2018

aaron.ballman added inline comments to D41553: Support parsing double square-bracket attributes in ObjC.
Jan 13 2018, 9:07 AM
aaron.ballman added a comment to D41921: [Parse] Forward brace locations to TypeConstructExpr.

On the whole, this approach looks reasonable to me. Just a few minor nits for clarity.

Jan 13 2018, 8:39 AM
aaron.ballman added inline comments to D41963: [clang-tidy] Adding Fuchsia checker for thread local storage..
Jan 13 2018, 8:27 AM · Restricted Project
aaron.ballman accepted D42005: [docs] Use monospace for PCH option flags.

LGTM, thanks!

Jan 13 2018, 8:23 AM
aaron.ballman accepted D41998: [clang-tidy] Expand readability-redundant-smartptr-get to understand implicit converions to bool in more contexts..

Aside from a minor formatting nit, LGTM!

Jan 13 2018, 8:22 AM
aaron.ballman added a comment to D41648: [clang-tidy] implement cppcoreguidelines macro rules.

Yes, and I'm saying that the guidelines aren't useful for real code bases because they restrict more than is reasonable. So while I think the check is largely implementing what the guidelines recommend, I think that some of these scenarios should be brought back to the guideline authors to weigh in on before we expose the check to users.

I see. Are u willing to identify some of the exceptions with me or don't you have enough time? I would like to propose the first list here. My opinion is, that we should leave some useful macros (e.g. Ensure from guidelines) left for manual inspection and to disable the check for these specific macros.

Jan 13 2018, 8:20 AM
aaron.ballman added inline comments to D41655: [clang-tidy] New check bugprone-unused-return-value.
Jan 13 2018, 8:03 AM · Restricted Project
aaron.ballman added inline comments to D41815: [clang-tidy] implement check for goto.
Jan 13 2018, 7:55 AM

Jan 12 2018

aaron.ballman added a comment to D41897: Fixing a crash in Sema..

Thank you!

Jan 12 2018, 8:07 AM
aaron.ballman accepted D41897: Fixing a crash in Sema..

LGTM, thank you!

Jan 12 2018, 7:58 AM
aaron.ballman added inline comments to D41897: Fixing a crash in Sema..
Jan 12 2018, 7:28 AM

Jan 11 2018

aaron.ballman added inline comments to D40580: [clang-tidy] Adding Fuchsia checker for multiple inheritance.
Jan 11 2018, 12:48 PM · Restricted Project
aaron.ballman accepted D41933: handle scoped_lockable objects being returned by value in C++17.

LGTM, but you should wait for a bit to see if DeLesley has concerns.

Jan 11 2018, 12:48 PM
aaron.ballman added a comment to D41815: [clang-tidy] implement check for goto.

I enhanced the check to do more:

  • check that jumps will only be forward. AFAIK that is required in all sensefull usecases of goto, is it?
  • additionally check for gotos in nested loops. These are not diagnosed if the jump is forward implementing the exception in the core guidelines.

    With these modifications the check can be used to enforce the rule in the CoreGuidelines and the goto part of 6.3.1 Ensure that the label(s) for a jump statement or a switch condition appear later, in the same or an enclosing block for the HICPP module.
Jan 11 2018, 11:33 AM
aaron.ballman added inline comments to D41897: Fixing a crash in Sema..
Jan 11 2018, 11:15 AM
aaron.ballman added a comment to D41933: handle scoped_lockable objects being returned by value in C++17.

I think this approach is a reasonable approximation, but @delesley has the final word.

Jan 11 2018, 11:13 AM
aaron.ballman accepted D41546: [clang-tidy] Adding Fuchsia checker for statically constructed objects.

Aside from a minor formatting nit that I missed, LGTM!

Jan 11 2018, 11:06 AM · Restricted Project