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 (340 w, 3 d)

Recent Activity

Fri, Sep 20

aaron.ballman accepted D67737: [clang-tidy] Add check for classes missing -hash ⚠️.

LGTM!

Fri, Sep 20, 5:41 AM · Restricted Project, Restricted Project
aaron.ballman accepted D67825: [AST] Extract Decl::printNestedNameSpecifier helper from Decl::printQualifiedName.

LGTM!

Fri, Sep 20, 5:28 AM · Restricted Project
aaron.ballman added a reviewer for D67825: [AST] Extract Decl::printNestedNameSpecifier helper from Decl::printQualifiedName: aaron.ballman.
Fri, Sep 20, 4:51 AM · Restricted Project

Thu, Sep 19

aaron.ballman committed rGefb9e45d6bc5: Revert r372325 - Reverting r372323 because it broke color tests on Linux. (authored by aaron.ballman).
Revert r372325 - Reverting r372323 because it broke color tests on Linux.
Thu, Sep 19, 8:12 AM
aaron.ballman added inline comments to D67737: [clang-tidy] Add check for classes missing -hash ⚠️.
Thu, Sep 19, 7:44 AM · Restricted Project, Restricted Project
aaron.ballman added inline comments to D63640: [clang] Improve Serialization/Imporing of APValues.
Thu, Sep 19, 7:09 AM · Restricted Project
aaron.ballman committed rGed9104c3f878: Reverting r372323 because it broke color tests on Linux. (authored by aaron.ballman).
Reverting r372323 because it broke color tests on Linux.
Thu, Sep 19, 7:00 AM
aaron.ballman committed rG3c3602aefa59: Remove an unsafe member variable that wasn't needed; NFC. (authored by aaron.ballman).
Remove an unsafe member variable that wasn't needed; NFC.
Thu, Sep 19, 6:56 AM

Wed, Sep 18

aaron.ballman accepted D66856: [Sema] Suppress -Wformat diagnostics for bool types when printed using %hhd.

LGTM, thank you for this!

Wed, Sep 18, 11:23 AM · Restricted Project, Restricted Project
aaron.ballman added inline comments to D67663: [Compiler] Fix LLVM_NODISCARD for GCC.
Wed, Sep 18, 8:32 AM
aaron.ballman added inline comments to D67545: [clang-tidy] Added DefaultOperatorNewCheck..
Wed, Sep 18, 8:16 AM · Restricted Project
aaron.ballman added inline comments to D67545: [clang-tidy] Added DefaultOperatorNewCheck..
Wed, Sep 18, 6:32 AM · Restricted Project
aaron.ballman added inline comments to D67545: [clang-tidy] Added DefaultOperatorNewCheck..
Wed, Sep 18, 6:30 AM · Restricted Project
aaron.ballman added inline comments to D67545: [clang-tidy] Added DefaultOperatorNewCheck..
Wed, Sep 18, 6:18 AM · Restricted Project
aaron.ballman added a comment to D52281: [clang-tidy] Add modernize check to use std::invoke in generic code.

The review says you abandoned it -- was that accidental?

Wed, Sep 18, 5:55 AM · Restricted Project
aaron.ballman added inline comments to D67545: [clang-tidy] Added DefaultOperatorNewCheck..
Wed, Sep 18, 5:49 AM · Restricted Project
aaron.ballman added inline comments to D65917: [clang-tidy] Added check for the Google style guide's category method naming rule..
Wed, Sep 18, 5:07 AM · Restricted Project, Restricted Project
aaron.ballman added inline comments to D67663: [Compiler] Fix LLVM_NODISCARD for GCC.
Wed, Sep 18, 5:07 AM
aaron.ballman added inline comments to D67663: [Compiler] Fix LLVM_NODISCARD for GCC.
Wed, Sep 18, 4:49 AM

Tue, Sep 17

aaron.ballman accepted D67559: [Sema] Split of versions of -Wimplicit-{float,int}-conversion for Objective-C BOOL.

LGTM.

Tue, Sep 17, 11:21 AM · Restricted Project, Restricted Project
aaron.ballman added inline comments to D67545: [clang-tidy] Added DefaultOperatorNewCheck..
Tue, Sep 17, 8:05 AM · Restricted Project
aaron.ballman added a comment to D67545: [clang-tidy] Added DefaultOperatorNewCheck..

Thank you for working on this check!

Tue, Sep 17, 7:10 AM · Restricted Project
aaron.ballman added inline comments to D64671: [clang-tidy] New check: misc-init-local-variables.
Tue, Sep 17, 6:25 AM · Restricted Project
aaron.ballman added inline comments to D67159: [clang] New __attribute__((__clang_arm_mve_alias))..
Tue, Sep 17, 6:25 AM · Restricted Project
aaron.ballman added inline comments to D65917: [clang-tidy] Added check for the Google style guide's category method naming rule..
Tue, Sep 17, 4:34 AM · Restricted Project, Restricted Project

Fri, Sep 13

aaron.ballman added a comment to D67559: [Sema] Split of versions of -Wimplicit-{float,int}-conversion for Objective-C BOOL.

Mostly LGTM but a few nits.

Fri, Sep 13, 1:32 PM · Restricted Project, Restricted Project
aaron.ballman accepted D67368: [NFCI]Create CommonAttributeInfo Type as base type of *Attr and ParsedAttr..

LGTM! Thank you for this refactoring -- it was large, but I think the results from it are really good.

Fri, Sep 13, 9:00 AM · Restricted Project

Wed, Sep 11

aaron.ballman accepted D52524: Add -Wpoison-system-directories warning.

Aside from a minor nit, this LGTM. However, I'm not the most familiar with how cross-compiling works in the first place, so I may be the wrong one to approve this.

Wed, Sep 11, 12:14 PM · Restricted Project
aaron.ballman added a comment to D66856: [Sema] Suppress -Wformat diagnostics for bool types when printed using %hhd.

To me, actual problems at runtime belong in -Wformat and logical inconsistencies that don't cause runtime problems belong in -Wformat-pedantic. However, I think it's a defect that the C standard has no clearly UB-free way to print a _Bool value. I will bring this up on the reflectors to ask if we can clarify the standard here.

Wed, Sep 11, 11:48 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D67368: [NFCI]Create CommonAttributeInfo Type as base type of *Attr and ParsedAttr..

Thank you for doing all this work -- in general, this is looking great and really cleans things up!

Wed, Sep 11, 11:24 AM · Restricted Project

Tue, Sep 10

aaron.ballman added a comment to D67140: [analyzer][NFC] Fix inconsistent references to checkers as "checks".
In D67140#1664106, @NoQ wrote:

We should take a page from desktop software here. If the messages were in a separate file, there would be a lot of people capable of mass-editing them. When messages are hardcoded in the tool code, navigating and editing them requires more skill, and definitely a lot more jumping around.

In the Static Analyzer there's often an explosive amount of dynamically generated messages that are going to be pretty hard to stuff into a tablegen pattern. Say, you can probably turn this into "%0 %1 %2 with a %3 retain count into an out parameter %4%5" but would it really help?

Unfortunately, that message is already not following best practices. One should not be passing snippets in natural language into substitutions. Every character that is a natural language construct should be in the message string. The only allowed substitutions are types, names, numbers and such. We already support %select in message strings, but there are frameworks that are more flexible -- we can port features from them into our message strings as needed.

Tue, Sep 10, 9:56 AM · Restricted Project, Restricted Project

Sat, Sep 7

aaron.ballman committed rGc4450437ec91: Fixes an assertion while instantiating a template with an incomplete typo… (authored by aaron.ballman).
Fixes an assertion while instantiating a template with an incomplete typo…
Sat, Sep 7, 1:14 PM
aaron.ballman accepted D65917: [clang-tidy] Added check for the Google style guide's category method naming rule..

LGTM!

Sat, Sep 7, 1:14 PM · Restricted Project, Restricted Project
aaron.ballman closed D64644: [Sema] Fixes an assertion failure while instantiation a template with an incomplete typo corrected type.

Thanks for the review.
Could you commit this patch? I don't have commit access.

Sat, Sep 7, 1:13 PM · Restricted Project

Fri, Sep 6

aaron.ballman added inline comments to D65917: [clang-tidy] Added check for the Google style guide's category method naming rule..
Fri, Sep 6, 5:21 AM · Restricted Project, Restricted Project
aaron.ballman accepted D64644: [Sema] Fixes an assertion failure while instantiation a template with an incomplete typo corrected type.

LGTM!

Fri, Sep 6, 5:05 AM · Restricted Project
aaron.ballman added a comment to D64671: [clang-tidy] New check: misc-init-local-variables.

Aside from the missing documentation bit, I think this LG.

Fri, Sep 6, 5:02 AM · Restricted Project
aaron.ballman added a comment to D67140: [analyzer][NFC] Fix inconsistent references to checkers as "checks".
In D67140#1659907, @NoQ wrote:

If we're okay with the inconsistency but still feel like giving ourselves more work, adding proper punctuation to Static Analyzer diagnostics would at least make them grammatically correct. :-D

I'd love us some punctuation! Unfortunately the last time a person who actually speaks English committed actively to the Static Analyzer was, like, a couple of years ago at least (:

I guess there's a joke waiting to be made about a couple Hungarians and Russians discussing how to write proper English :^)

Fri, Sep 6, 5:00 AM · Restricted Project, Restricted Project

Thu, Sep 5

aaron.ballman added a comment to D67140: [analyzer][NFC] Fix inconsistent references to checkers as "checks".
In D67140#1659831, @NoQ wrote:

I don't think it's a requirement (so long as the diagnostics are clear about the issue being diagnosed, I'm happy enough), but I think it's good for a tool to be self-consistent in its messaging. It's jarring that one part of the compiler emits diagnostics one way and another emits them a totally different way. It may be less of an impact for people who don't see the output from both at the same time, but that happens to be the use case I have.

Unfortunately i think at this point many clients who tried to integrate multiple tools resorted to "Automatic Message Recapitalization" (c). For that reason we probably can harmlessly decapitalize Static Analyzer messages. I suspect that it won't really change anything either, because most tools will still be afraid of accidental inconsistencies in the compiler.

Thu, Sep 5, 2:52 PM · Restricted Project, Restricted Project
aaron.ballman added a comment to D67140: [analyzer][NFC] Fix inconsistent references to checkers as "checks".

Ah, good to know! That reduces my concern, but doesn't negate it. AFAIK, we haven't changed the interface such that it requires code changes rather than just a recompile in recent history, so this is a bit novel.

I think API changes happen all the time. At Google, we are integrating upstream LLVM and Clang changes into our internal codebase daily. We have a lot of internal ClangTidy checkers. Fixing up all our internal code to keep with upstream changes is a full time job for one engineer (but it is a rotation).

I think this sort of backs up the point I was making.

Sorry, but in the previous comment you were saying that "we haven't changed the interface such that it requires code changes". So which is it, for you?

Thu, Sep 5, 2:08 PM · Restricted Project, Restricted Project
aaron.ballman added a comment to D54408: [ASTMatchers] Add matchers available through casting to derived.

@aaron.ballman
I think the auto usage improves and simplifies the code, however, as I would really like to see this code landed, I would be ok to help Stephen to finish this work and replace auto by the "old" way. Do you think we can have this approved if we make the changes mentioned earlier?

Thu, Sep 5, 7:39 AM · Restricted Project
aaron.ballman added a comment to D67140: [analyzer][NFC] Fix inconsistent references to checkers as "checks".

Ah, good to know! That reduces my concern, but doesn't negate it. AFAIK, we haven't changed the interface such that it requires code changes rather than just a recompile in recent history, so this is a bit novel.

I think API changes happen all the time. At Google, we are integrating upstream LLVM and Clang changes into our internal codebase daily. We have a lot of internal ClangTidy checkers. Fixing up all our internal code to keep with upstream changes is a full time job for one engineer (but it is a rotation).

Thu, Sep 5, 7:35 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D66919: Warn about zero-parameter K&R definitions in -Wstrict-prototypes.

We do have numerous warnings that are default errors, you can look for DefaultError in the diagnostic .td files to see the uses.

Thanks, I hadn't seen that before. It seems that most of these warnings are for extensions, but one comes pretty close to what @dexonsmith has suggested:

def warn_cannot_pass_non_pod_arg_to_vararg : Warning<
  "cannot pass object of %select{non-POD|non-trivial}0 type %1 through variadic"
  " %select{function|block|method|constructor}2; call will abort at runtime">,
  InGroup<NonPODVarargs>, DefaultError;

The standard explicitly says in C11 6.5.2.2p8: “the number and types of arguments are not compared with those of the parameters in a function definition that does not include a function prototype declarator”, but perhaps this just means a programmer can't rely on such a check?

Thu, Sep 5, 7:24 AM · Restricted Project
aaron.ballman added a comment to D67159: [clang] New __attribute__((__clang_arm_mve_alias))..

Come to think of it, it would also not be too hard to constrain it to only be usable for a particular subset of builtins, and perhaps even only with a particular set of alias names for them. (I could easily derive all that information from the same Tablegen that arm_mve.h itself is made from.)

Thu, Sep 5, 6:45 AM · Restricted Project
aaron.ballman added inline comments to D65917: [clang-tidy] Added check for the Google style guide's category method naming rule..
Thu, Sep 5, 5:03 AM · Restricted Project, Restricted Project

Wed, Sep 4

aaron.ballman added inline comments to D66397: [Diagnostics] Improve -Wxor-used-as-pow.
Wed, Sep 4, 3:14 PM · Restricted Project
aaron.ballman added inline comments to D64671: [clang-tidy] New check: misc-init-local-variables.
Wed, Sep 4, 3:09 PM · Restricted Project
aaron.ballman added a comment to D67159: [clang] New __attribute__((__clang_arm_mve_alias))..

I share in the discomfort expressed for this attribute, but I don't have a better solution in mind just yet, so I'm giving some other review feedback in the meantime.

Wed, Sep 4, 2:58 PM · Restricted Project
aaron.ballman added inline comments to D66397: [Diagnostics] Improve -Wxor-used-as-pow.
Wed, Sep 4, 2:51 PM · Restricted Project
aaron.ballman added inline comments to D65917: [clang-tidy] Added check for the Google style guide's category method naming rule..
Wed, Sep 4, 2:41 PM · Restricted Project, Restricted Project
aaron.ballman added a comment to D67140: [analyzer][NFC] Fix inconsistent references to checkers as "checks".
In D67140#1656831, @NoQ wrote:

Honestly, i'm much more worried about message capitalization :)

Likewise. I wish the static analyzer would follow the usual conventions followed by clang and clang-tidy. ;-)

I have the opposite opinion -- I wish that ClangTidy used complete sentences, and multiple sentences if it makes sense. The sentence fragments are too brief to explain complex and nuanced topics that ClangTidy communicates about. ClangTidy often plays the role of a developer education tool. It is not a guard like a compiler; developers can totally ignore ClangTidy if they disagree with the message. The better we can explain the problem, the more likely it is the developer will act on the message. I believe static analysis tools would be better off if we could write multiple sentences in the diagnostic.

Even for compiler messages, a sentence fragment is sometimes too concise.

Wed, Sep 4, 2:24 PM · Restricted Project, Restricted Project
aaron.ballman added a comment to D66919: Warn about zero-parameter K&R definitions in -Wstrict-prototypes.

I went back to read notes from when we deployed -Wstrict-prototypes (which we have had on-by-default for our users for a couple of years, since it catches lots of -fblocks-related bugs). I was mainly objecting because I had (mis)remembered trying and backing out the specific behaviour you're adding. On the contrary, our notes say that we might want to strengthen the diagnostic as you're doing.

Thank you for having a look at the notes, that's good to hear.

Your users are Xcode/Apple Clang users? @aaron.ballman suggested to turn the warning on per default, and if Apple has that deployed already, it might be another argument in favor of doing so in vanilla Clang as well.

Wed, Sep 4, 2:14 PM · Restricted Project
aaron.ballman added inline comments to D66397: [Diagnostics] Improve -Wxor-used-as-pow.
Wed, Sep 4, 2:10 PM · Restricted Project
aaron.ballman added inline comments to D64644: [Sema] Fixes an assertion failure while instantiation a template with an incomplete typo corrected type.
Wed, Sep 4, 2:08 PM · Restricted Project
aaron.ballman committed rG5cd5d56eedf9: Diagnose _Atomic as a C11 extension. (authored by aaron.ballman).
Diagnose _Atomic as a C11 extension.
Wed, Sep 4, 2:02 PM
aaron.ballman added a comment to D67023: Diagnose use of ATOMIC_VAR_INIT.
In D67023#1656453, @jfb wrote:
In D67023#1654070, @jfb wrote:

I refer you to http://wg21.link/p0883
I don’t think this diagnostic should be added to clang until p0883 is fully implemented, even just for C. Otherwise we leave users with no portable way to do the right thing without diagnostic.

P0883 has not been adopted, that I can tell (it has strong support, but isn't [expected to be] a part of C++2a)?

I think it'll make it in as an NB comment.

Also, this functionality is implemented by libc++ and libstdc++, so I'm not certain what you mean by "until p0883 is fully implemented" or why that paper would be implemented "even just for C". Are you suggesting to gate this deprecation for C functionality based on what C++ standard library implementations are doing?

Yes, because atomics are already hard enough to use correctly between C and C++.

Wed, Sep 4, 1:56 PM
aaron.ballman added a comment to D67140: [analyzer][NFC] Fix inconsistent references to checkers as "checks".
In D67140#1656831, @NoQ wrote:

Honestly, i'm much more worried about message capitalization :)

Wed, Sep 4, 1:38 PM · Restricted Project, Restricted Project
aaron.ballman committed rG40e3760472ea: Generate parent context id from Decl* instead of DeclContext*. (authored by aaron.ballman).
Generate parent context id from Decl* instead of DeclContext*.
Wed, Sep 4, 1:31 PM
aaron.ballman closed D67057: [AST][JSON] Generate parent context id from Decl* instead of DeclContext*.

Fixed nit, this is ready to land IMO.

Wed, Sep 4, 1:28 PM · Restricted Project
aaron.ballman accepted D67084: [clang-tidy] Fix bugprone-argument-comment bug: negative literal number is not checked..

LGTM!

Wed, Sep 4, 7:04 AM · Restricted Project, Restricted Project, Restricted Project
aaron.ballman accepted D67057: [AST][JSON] Generate parent context id from Decl* instead of DeclContext*.

LGTM aside from a small nit. Thank you for this!

Wed, Sep 4, 6:15 AM · Restricted Project

Tue, Sep 3

aaron.ballman added inline comments to D67084: [clang-tidy] Fix bugprone-argument-comment bug: negative literal number is not checked..
Tue, Sep 3, 1:02 PM · Restricted Project, Restricted Project, Restricted Project
aaron.ballman added a comment to D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour.

One fear I have with this is in expansions of the offsetof macro, where it is a common implementation strategy to cast a null pointer to be of the correct type when calculating member offsets. Do you think you will be able to distinguish between null pointer additions that the user wrote directly (which is UB) as opposed to null pointer additions that come from the implementation (which is not UB)?

Can you show a snippet on godbolt?

Tue, Sep 3, 12:15 PM · Restricted Project, Restricted Project, Restricted Project
aaron.ballman added a comment to D67122: [UBSan][clang][compiler-rt] Applying non-zero offset to nullptr is undefined behaviour.

One fear I have with this is in expansions of the offsetof macro, where it is a common implementation strategy to cast a null pointer to be of the correct type when calculating member offsets. Do you think you will be able to distinguish between null pointer additions that the user wrote directly (which is UB) as opposed to null pointer additions that come from the implementation (which is not UB)?

Tue, Sep 3, 11:57 AM · Restricted Project, Restricted Project, Restricted Project
aaron.ballman added inline comments to D64671: [clang-tidy] New check: misc-init-local-variables.
Tue, Sep 3, 9:51 AM · Restricted Project

Mon, Sep 2

aaron.ballman added a comment to D67023: Diagnose use of ATOMIC_VAR_INIT.
In D67023#1654070, @jfb wrote:

I refer you to http://wg21.link/p0883
I don’t think this diagnostic should be added to clang until p0883 is fully implemented, even just for C. Otherwise we leave users with no portable way to do the right thing without diagnostic.

Mon, Sep 2, 6:26 AM

Sun, Sep 1

aaron.ballman added a comment to D67023: Diagnose use of ATOMIC_VAR_INIT.
In D67023#1653425, @jfb wrote:

Is atomic initialization now correct in all modes (including C++) without this macro?

Sun, Sep 1, 9:24 AM

Fri, Aug 30

aaron.ballman added inline comments to D67023: Diagnose use of ATOMIC_VAR_INIT.
Fri, Aug 30, 3:47 PM
aaron.ballman created D67023: Diagnose use of ATOMIC_VAR_INIT.
Fri, Aug 30, 3:04 PM
aaron.ballman added a comment to D66397: [Diagnostics] Improve -Wxor-used-as-pow.

tbh, I would appreciate if you would leave the definition of diagnoseXorMisusedAsPow() where it is and add a forward declare of the function earlier in the file.

I uploaded the patch almost 2 weeks ago and nobody complained so I thought it is okay. Uploaded, fixed.

Fri, Aug 30, 12:17 PM · Restricted Project
aaron.ballman added inline comments to D66397: [Diagnostics] Improve -Wxor-used-as-pow.
Fri, Aug 30, 12:17 PM · Restricted Project
aaron.ballman accepted D66505: Make add_new_check.py's insertion of registerCheck<> match the sort order.

LGTM, thank you for working on this!

Fri, Aug 30, 12:03 PM · Restricted Project, Restricted Project
aaron.ballman accepted D65919: [clang-tidy] Add llvm-prefer-register-over-unsigned check and apply it to LLVM.

LGTM aside from a nit with auto.

Fri, Aug 30, 4:56 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D66505: Make add_new_check.py's insertion of registerCheck<> match the sort order.

Generally LG to me as well

Fri, Aug 30, 4:56 AM · Restricted Project, Restricted Project

Thu, Aug 29

aaron.ballman added inline comments to D64671: [clang-tidy] New check: misc-init-local-variables.
Thu, Aug 29, 1:12 PM · Restricted Project
aaron.ballman committed rG1755617214e8: Avoid crash when dumping NULL Type as JSON. (authored by aaron.ballman).
Avoid crash when dumping NULL Type as JSON.
Thu, Aug 29, 1:00 PM
aaron.ballman closed D66850: [AST][JSON] Avoid crash when dumping NULL Type as JSON.
Thu, Aug 29, 12:59 PM · Restricted Project
aaron.ballman added a comment to D66850: [AST][JSON] Avoid crash when dumping NULL Type as JSON.

@aaron.ballman
I was able to run the tests, I think this is good to go.
Can you help me get it landed?

Thu, Aug 29, 12:59 PM · Restricted Project
aaron.ballman added a comment to D66919: Warn about zero-parameter K&R definitions in -Wstrict-prototypes.

This could cause a lot of churn in existing projects (especially with static void foo()), without giving Clang any new information. I'm wary of this.

Those projects likely aren't aware they're using prototypeless functions, which are trivial to call incorrectly. I suspect this diagnostic will find real bugs in code.

To be clear, my understanding is that -Wstrict-prototypes already warns on non-prototype declarations like this:

void foo();

we just don't warn on non-prototype defining declarations, where the meaning is unambiguous:

void foo() {}
Thu, Aug 29, 10:13 AM · Restricted Project
aaron.ballman added a comment to D66856: [Sema] Suppress -Wformat diagnostics for bool types when printed using %hhd.

I'm wondering whether this should even warn pedantically. There are no format specifiers that apply directly to a _Bool datatype, so the user is left making a choice as to what specifier fits best. I think hhd and hhu are both equally reasonable types, as are the d and u groups directly -- I don't think we should warn on either of those specifiers with a _Bool argument. I could maybe see a case for pedantically diagnosing h, l, and ll prefixes with _Bool because that seems a bit type-confused to me. c as a specifier seems weird (given that false values will potentially terminate the string suddenly depending on terminal behavior, IIRC), but lc seems like type confusion again.

Hmm... on second though I think the l and ll prefixes are worth -Wformat proper, since that can result in printing a half-initialized integer when the argument ends up on the stack (we generate movl $0,(%rsp) for the first stack argument).

Thu, Aug 29, 6:02 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D66919: Warn about zero-parameter K&R definitions in -Wstrict-prototypes.

This could cause a lot of churn in existing projects (especially with static void foo()), without giving Clang any new information. I'm wary of this.

Thu, Aug 29, 5:52 AM · Restricted Project
aaron.ballman added a reviewer for D66919: Warn about zero-parameter K&R definitions in -Wstrict-prototypes: aaron.ballman.
Thu, Aug 29, 5:52 AM · Restricted Project

Wed, Aug 28

aaron.ballman accepted D66850: [AST][JSON] Avoid crash when dumping NULL Type as JSON.

LGTM, but missing a test case.

LGTM. I'm not an expert in JSON, but may be it makes sense to move the change a line earlier before creation of pointer representation?

I would prefer it remains where it is -- having the 0x0 in the output for a null pointer is a good thing because it conveys more information than a totally empty Type object. We're accidentally inconsistent about this currently (Decl prints 0x0 but Stmt gives an empty object).

I don't disagree, but I would argue that "0x0" is a rather poor choice, since now the consumer has to treat the "id" field as an opaque value, except when it's "0x0". A better choice would be to use JavaScript null to represent null pointers.

Wed, Aug 28, 1:57 PM · Restricted Project
aaron.ballman added a comment to D65919: [clang-tidy] Add llvm-prefer-register-over-unsigned check and apply it to LLVM.

Aside from the few remaining nits, I think this is almost ready to go.

Wed, Aug 28, 11:49 AM · Restricted Project, Restricted Project
aaron.ballman added inline comments to D52524: Add -Wpoison-system-directories warning.
Wed, Aug 28, 11:44 AM · Restricted Project
aaron.ballman added a comment to D66397: [Diagnostics] Improve -Wxor-used-as-pow.

Adding @rsmith to see if we can make forward progress on this patch again.

On the other side, I don't want to waste Richard's time since I dont want to extend (variables and macros are controversal topic anyway) it more now. I promised to @jfb to handle (2 ^ 64) - 1 as follow up patch and the promised patch is here..

Wed, Aug 28, 11:38 AM · Restricted Project
aaron.ballman added a comment to D66856: [Sema] Suppress -Wformat diagnostics for bool types when printed using %hhd.

I'm wondering whether this should even warn pedantically. There are no format specifiers that apply directly to a _Bool datatype, so the user is left making a choice as to what specifier fits best. I think hhd and hhu are both equally reasonable types, as are the d and u groups directly -- I don't think we should warn on either of those specifiers with a _Bool argument. I could maybe see a case for pedantically diagnosing h, l, and ll prefixes with _Bool because that seems a bit type-confused to me. c as a specifier seems weird (given that false values will potentially terminate the string suddenly depending on terminal behavior, IIRC), but lc seems like type confusion again.

Wed, Aug 28, 6:11 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D66850: [AST][JSON] Avoid crash when dumping NULL Type as JSON.

LGTM, but missing a test case.

Wed, Aug 28, 5:15 AM · Restricted Project

Tue, Aug 27

aaron.ballman committed rG27e66bf71095: Diagnose _Bool as a C99 extension. (authored by aaron.ballman).
Diagnose _Bool as a C99 extension.
Tue, Aug 27, 1:40 PM
aaron.ballman committed rG9fac4a5d3522: Diagnose both _Complex and _Imaginary as C99 extensions. (authored by aaron.ballman).
Diagnose both _Complex and _Imaginary as C99 extensions.
Tue, Aug 27, 12:21 PM
aaron.ballman added a comment to D64671: [clang-tidy] New check: misc-init-local-variables.

I used stdint to replicate a real world use case as I'd imagine those types would match this search quite heavily.

Tue, Aug 27, 11:43 AM · Restricted Project
aaron.ballman added a reviewer for D66397: [Diagnostics] Improve -Wxor-used-as-pow: rsmith.

Adding @rsmith to see if we can make forward progress on this patch again.

Tue, Aug 27, 11:31 AM · Restricted Project
aaron.ballman committed rG1d935220565e: Replace some custom C11 extension warnings with the generic warning. (authored by aaron.ballman).
Replace some custom C11 extension warnings with the generic warning.
Tue, Aug 27, 7:42 AM
aaron.ballman committed rG99178faf59da: Quote the token being diagnosed for C11 extensions. (authored by aaron.ballman).
Quote the token being diagnosed for C11 extensions.
Tue, Aug 27, 6:53 AM
aaron.ballman committed rG21b18966643f: Speculatively fix the build bots after r370052. (authored by aaron.ballman).
Speculatively fix the build bots after r370052.
Tue, Aug 27, 6:50 AM
aaron.ballman committed rG0299dbd2ae89: Implement codegen for MSVC unions with reference members. (authored by aaron.ballman).
Implement codegen for MSVC unions with reference members.
Tue, Aug 27, 5:44 AM
aaron.ballman closed D62571: Implement codegen for MSVC unions with reference members.

Rebased onto master, clang format the patch.

Merge conflict resolve by having the bitcast of the field reference happening after recording access index.

Tue, Aug 27, 5:44 AM · Restricted Project
aaron.ballman added a comment to D66651: Annotate return values of allocation functions with dereferenceable_or_null.

I'm not an LLVM person, but I just wanted to double-check: is this correct even in the face of situations where malloc() and friends return a non-null pointer that is *not* dereferenceable? e.g., when someone calls malloc(0) with certain libc implementations?

Tue, Aug 27, 4:58 AM · Restricted Project

Mon, Aug 26

aaron.ballman committed rG72797ba072c0: Updating a test case that was missed in r369957. (authored by aaron.ballman).
Updating a test case that was missed in r369957.
Mon, Aug 26, 2:00 PM
aaron.ballman added inline comments to rG33d563e59ed9: Reword the C11 extension diagnostic..
Mon, Aug 26, 1:56 PM