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

Recent Activity

Sat, Jul 20

aaron.ballman added inline comments to D64883: Add new warning -Walloca for use of builtin alloca function.
Sat, Jul 20, 6:45 AM · Restricted Project
aaron.ballman committed rG7017a6d3a3f4: Mark P1301R4 in C++2a as being SVN instead. (authored by aaron.ballman).
Mark P1301R4 in C++2a as being SVN instead.
Sat, Jul 20, 1:59 AM
aaron.ballman committed rG1358af27c090: We support P1301R4 in C++2a as of r366626. (authored by aaron.ballman).
We support P1301R4 in C++2a as of r366626.
Sat, Jul 20, 1:26 AM
aaron.ballman committed rG3bef014e7d79: Implement P1301R4, which allows specifying an optional message on the… (authored by aaron.ballman).
Implement P1301R4, which allows specifying an optional message on the…
Sat, Jul 20, 12:57 AM

Fri, Jul 19

aaron.ballman accepted D64780: Disallow most calling convention attributes on PS4..

LGTM aside from a nit. You should give other reviewers a chance to sign off in case they have additional comments.

Fri, Jul 19, 8:02 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D64838: [Attr] Support _attribute__ ((fallthrough)).
void foo() {
  __attribute__((address_space(0))) *x;
  *y;
}

If the attributes are parsed then function body looks like this to the parser:

{
  *x; //this one has attributes now
  *y;
{

The first line should be a valid declaration and the second like should be a dereference of an uninitialized variable. If the attributes token is discarded before parsing the rest of the line the only way to differentiate these is by looking at the attributes added to them.

An alternative may be parse the attributes list and immediately try to parse as a declaration then if that parsing fails attempt to parse as something else. Although this approach also has the scary implication of things that are supposed to be declarations getting reparsed as something entirely different.

Fri, Jul 19, 7:16 AM · Restricted Project
aaron.ballman added inline comments to D64914: Implement P1771.
Fri, Jul 19, 5:34 AM · Restricted Project
aaron.ballman added inline comments to D64914: Implement P1771.
Fri, Jul 19, 5:09 AM · Restricted Project
aaron.ballman accepted D64098: [NFC][clang] Refactor getCompilationPhases step 1: Move list of phases into Types.def table..

LGTM!

Fri, Jul 19, 12:08 AM · Restricted Project

Thu, Jul 18

aaron.ballman added a comment to D64838: [Attr] Support _attribute__ ((fallthrough)).

The main problem that we have is that the __attribute__ token always causes the parser to read the line as a declaration. Then the declaration parser handles reading the attributes list.

This case demonstrates the problem:

void foo() {
  __attribute__((address_space(0))) *x;
}

If the attribute token is read beforehand this code just becomes *x and is parsed as a dereference of an undefined variable when it should actually be a declaration.

Maybe the best solution is to pull the attributes parsing out to ParseStatementOrDeclaration then pass those attributes through to following functions.
When the determination is made whether a line is a declaration or a statement the attributes list can be looked at to determine if the attributes are statement or declaration attributes and continue accordingly. Maybe throwing a warning if mixing of declaration and statement attributes occur.

Does that sound like a good solution?

Please see the discussion in https://reviews.llvm.org/D63299#inline-564887 -- if I understand your approach properly, this approach leads to spooky action at a distance, where the name of the attribute dictates whether something is a statement or a declaration. I'm not comfortable with that. There's no reason we could not have an attribute that is both a statement attribute and a declaration attribute, and how would *that* parse?

I think this requires some deeper surgery in the parsing logic so that attributes do not impact whether something is treated as a declaration or a statement. The parser should parse attributes first, keep them around for a while, and attach them to either the decl or the statement at a later point.

Any idea how to fix the problem in the code sample I gave? The addition of the attributes token causes code to be read as a declaration and without the token it is read as a unary operator.

Thu, Jul 18, 11:52 AM · Restricted Project
aaron.ballman added inline comments to D64600: [ObjC] Add an attribute that "clamps" signed char BOOLs to {0,1}.
Thu, Jul 18, 11:37 AM · Restricted Project
aaron.ballman added inline comments to D64098: [NFC][clang] Refactor getCompilationPhases step 1: Move list of phases into Types.def table..
Thu, Jul 18, 11:22 AM · Restricted Project
aaron.ballman added a comment to D64838: [Attr] Support _attribute__ ((fallthrough)).

The main problem that we have is that the __attribute__ token always causes the parser to read the line as a declaration. Then the declaration parser handles reading the attributes list.

This case demonstrates the problem:

void foo() {
  __attribute__((address_space(0))) *x;
}

If the attribute token is read beforehand this code just becomes *x and is parsed as a dereference of an undefined variable when it should actually be a declaration.

Maybe the best solution is to pull the attributes parsing out to ParseStatementOrDeclaration then pass those attributes through to following functions.
When the determination is made whether a line is a declaration or a statement the attributes list can be looked at to determine if the attributes are statement or declaration attributes and continue accordingly. Maybe throwing a warning if mixing of declaration and statement attributes occur.

Does that sound like a good solution?

Thu, Jul 18, 11:09 AM · Restricted Project
aaron.ballman added a comment to D64914: Implement P1771.

There seems to be some separable concerns in this patch. I'd rather real with warn_unused_result, const, and pure attributes separately only because those are under the gnu attribute namespace and I'd prefer to match GNU's semantics for those attributes (or at least understand better where the inconsistencies are).

Thu, Jul 18, 8:57 AM · Restricted Project
aaron.ballman added inline comments to D61989: [clang-tidy] enable modernize-concat-nested-namespaces on header files.
Thu, Jul 18, 6:19 AM · Restricted Project
aaron.ballman added inline comments to D64098: [NFC][clang] Refactor getCompilationPhases step 1: Move list of phases into Types.def table..
Thu, Jul 18, 12:01 AM · Restricted Project

Wed, Jul 17

aaron.ballman added inline comments to D64098: [NFC][clang] Refactor getCompilationPhases step 1: Move list of phases into Types.def table..
Wed, Jul 17, 11:57 AM · Restricted Project
aaron.ballman added inline comments to D64780: Disallow most calling convention attributes on PS4..
Wed, Jul 17, 11:02 AM · Restricted Project, Restricted Project

Tue, Jul 16

aaron.ballman accepted D61749: [clang-tidy] initial version of readability-convert-member-functions-to-static.

LGTM aside from a tiny nit.

Tue, Jul 16, 2:31 AM · Restricted Project, Restricted Project, Restricted Project
aaron.ballman added a comment to D64454: [clang-tidy] Adding static analyzer check to list of clang-tidy checks.

I think this looks reasonable to me, though I am still not certain if the relative path in the python script will work with both the svn in-tree directory layout as well as the git monorepo layout (which I'm far less familiar with).

Tue, Jul 16, 1:38 AM · Restricted Project

Fri, Jul 12

aaron.ballman added inline comments to D64600: [ObjC] Add an attribute that "clamps" signed char BOOLs to {0,1}.
Fri, Jul 12, 10:44 AM · Restricted Project
aaron.ballman committed rG352f0a22b8e8: Dump actual line numbers when dumping the AST to JSON. (authored by aaron.ballman).
Dump actual line numbers when dumping the AST to JSON.
Fri, Jul 12, 9:55 AM
aaron.ballman added inline comments to D64632: [clang-format] Don't detect call to ObjC class method as C++11 attribute specifier.
Fri, Jul 12, 8:47 AM · Restricted Project, Restricted Project
aaron.ballman committed rG4d08f899e39b: Dump floating-point values as strings when dumping to JSON. (authored by aaron.ballman).
Dump floating-point values as strings when dumping to JSON.
Fri, Jul 12, 7:00 AM

Thu, Jul 11

aaron.ballman added inline comments to D64380: Add 'require_designated_init' and 'required' attribute to clang.
Thu, Jul 11, 2:03 PM · Restricted Project
aaron.ballman added inline comments to D63954: Add lifetime categories attributes.
Thu, Jul 11, 1:51 PM · Restricted Project
aaron.ballman added inline comments to D64454: [clang-tidy] Adding static analyzer check to list of clang-tidy checks.
Thu, Jul 11, 1:51 PM · Restricted Project
aaron.ballman added inline comments to D64380: Add 'require_designated_init' and 'required' attribute to clang.
Thu, Jul 11, 8:57 AM · Restricted Project
aaron.ballman added inline comments to D64454: [clang-tidy] Adding static analyzer check to list of clang-tidy checks.
Thu, Jul 11, 8:40 AM · Restricted Project
aaron.ballman accepted D64543: [Docs] Add standardized header links to analyzer doc.

LGTM! Do you need someone to commit on your behalf?

Thu, Jul 11, 8:32 AM · Restricted Project, Restricted Project

Wed, Jul 10

aaron.ballman added a comment to D63139: [Diagnostics] Implement -Wswitch-unreachable.

I agree that tools shouldn't be forced to deal with invalid AST that looks like valid AST. To me that means finding ways to preserve information that (1) don't badly violate invariants and (2) are easily discoverable as invalid.

Wed, Jul 10, 2:05 PM · Restricted Project
aaron.ballman added a comment to D63192: [Diagnostics] Implement -Wswitch-default.

I see. I should really check clang-tidy codebase :)

Wed, Jul 10, 10:58 AM · Restricted Project
aaron.ballman added a comment to D63192: [Diagnostics] Implement -Wswitch-default.

I'm wary of adding this as an on-by-default warning, despite its presence in MSVC (and despite submitting a similar patch years ago to implement the same functionality). Machine-generated code runs into this one frequently (we've been bitten by it numerous times with our tablegen code, especially in the backends), but it does not seem likely to occur in hand-written code very often.

Wed, Jul 10, 10:26 AM · Restricted Project
aaron.ballman added a comment to D64454: [clang-tidy] Adding static analyzer check to list of clang-tidy checks.

Just thinking aloud!

We were tinkering with the idea of a checker creator script similar to what clang-tidy has, that could help on this potentially.

Is generating the rst code with Tblgen a feasable approach? At first glance, it sounds like a nightmare to implement, and I wonder whether the maintainers of sphinx buildbots would need to adjust to that.

Wed, Jul 10, 9:58 AM · Restricted Project
aaron.ballman added a comment to D63139: [Diagnostics] Implement -Wswitch-unreachable.

Are the CaseStmts being dropped in C++ because the expression overflows? I agree that that's bad AST behavior; we should strive to generate AST whenever we can, even if it's not valid.

Wed, Jul 10, 7:46 AM · Restricted Project
aaron.ballman added reviewers for D64454: [clang-tidy] Adding static analyzer check to list of clang-tidy checks: NoQ, Szelethus.

I'm worried that this will continue to drift out of sync with the static analyzer checks unless we find some way to automate it. I wonder if we could write a script to generate these .rst files and somehow fit it into the workflow for adding new static analyzer checks to re-run the script to produce new files (or modified files) as needed? @NoQ and @Szelethus may have ideas.

Wed, Jul 10, 7:25 AM · Restricted Project
aaron.ballman added inline comments to D64380: Add 'require_designated_init' and 'required' attribute to clang.
Wed, Jul 10, 7:22 AM · Restricted Project
aaron.ballman added inline comments to D61749: [clang-tidy] initial version of readability-convert-member-functions-to-static.
Wed, Jul 10, 6:08 AM · Restricted Project, Restricted Project, Restricted Project
aaron.ballman added inline comments to D63954: Add lifetime categories attributes.
Wed, Jul 10, 5:54 AM · Restricted Project

Tue, Jul 9

aaron.ballman added inline comments to D62829: [clang-tidy] Check for dynamically initialized statics in headers..
Tue, Jul 9, 9:03 AM · Restricted Project, Restricted Project
aaron.ballman committed rGb1e511bf5a4c: Ignore trailing NullStmts in StmtExprs for GCC compatibility. (authored by aaron.ballman).
Ignore trailing NullStmts in StmtExprs for GCC compatibility.
Tue, Jul 9, 8:03 AM
aaron.ballman closed D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility.

I've commit on your behalf in r365498. Thank you for the patch!

Tue, Jul 9, 8:02 AM

Mon, Jul 8

aaron.ballman accepted D64326: Retire VS2015 Support.

LGTM, but you should wait for others to have a chance to discuss before committing. You can always do the Clang docs in a separate follow-up patch (I know the changes are trivial, I just wanted to make sure they were all taken care of).

Mon, Jul 8, 6:43 AM · Restricted Project
aaron.ballman added a comment to D64326: Retire VS2015 Support.

I think this patch should also update our user-facing documentation at the same time, such as https://clang.llvm.org/get_started.html, https://llvm.org/docs/GettingStarted.html, and https://llvm.org/docs/GettingStartedVS.html.

Mon, Jul 8, 6:19 AM · Restricted Project

Sat, Jul 6

aaron.ballman added a comment to D63845: [WIP] Create a clang attribute that lets users specify LLVM attributes.

I think we should postpone a detailed discussion until we are ready to send an RFC on this. Nevertheless, I inlined some responses below.

What's the target use-case here? What can't be solved with normal attributes?

With "normal" you mean something like __attribute__((noescape))? We don't have them for all attributes, I doubt we want to but I might be wrong.

That is precisely the question.
What is the motivation for exposing such LLVM-specific low-level unstable implementation detail?

I would disagree to the unstable part. Keeping LLVM-IR backward compatibility is always a priority and changing the meaning of an existing LLVM-IR attribute will break various things already.
Why would you think it is unstable?

Because, to date, we've never made blanket any stability guarantees about LLVM attributes from within the context of Clang. Instead, LLVM attributes are introduced into Clang on a case by case basis when they've been determined to be stable enough to warrant exposing. This extra layer also gives us more flexibility in translating frontend attributes into backend attributes.

You still assume LLVM attributes are not stable while I tried to argue they are, they have to be already for backward compatibility, at least target agnostic ones.

Sat, Jul 6, 9:15 AM · Restricted Project, Restricted Project

Fri, Jul 5

aaron.ballman added a comment to D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403).

@JonasToth thanks for asking! Yes, since I do not have commit access, I need someone to do the commit for me.

Fri, Jul 5, 8:04 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility.

clang-format the patch

Thanks! Do you need someone to commit on your behalf?

You are very welcome; thank you both for your comments!

I do need someone to commit on my behalf :)

Fri, Jul 5, 8:04 AM

Thu, Jul 4

aaron.ballman added a comment to D63845: [WIP] Create a clang attribute that lets users specify LLVM attributes.

What's the target use-case here? What can't be solved with normal attributes?

With "normal" you mean something like __attribute__((noescape))? We don't have them for all attributes, I doubt we want to but I might be wrong.

That is precisely the question.
What is the motivation for exposing such LLVM-specific low-level unstable implementation detail?

I would disagree to the unstable part. Keeping LLVM-IR backward compatibility is always a priority and changing the meaning of an existing LLVM-IR attribute will break various things already.
Why would you think it is unstable?

Thu, Jul 4, 12:29 PM · Restricted Project, Restricted Project

Wed, Jul 3

aaron.ballman added inline comments to D61749: [clang-tidy] initial version of readability-convert-member-functions-to-static.
Wed, Jul 3, 8:05 AM · Restricted Project, Restricted Project, Restricted Project

Mon, Jul 1

aaron.ballman added inline comments to D63954: Add lifetime categories attributes.
Mon, Jul 1, 9:08 AM · Restricted Project
aaron.ballman added inline comments to D60455: [SYCL] Implement SYCL device code outlining.
Mon, Jul 1, 8:15 AM · Restricted Project
aaron.ballman added a comment to D63954: Add lifetime categories attributes.

We explicitly allow to add an annotation after
the definition of the class to allow adding annotations
to external source from by the user, e.g.

#include <vector>

namespace std {
template<typename T, typename Alloc>
class [[gsl::Owner(T)]] vector;
}

Wait, does that even work? What if the vector was declared in an inline namespace in the header? Seems like a strange recommendation for users. Is there some reason you can't just add these annotations to standard libraries? I doubt libcxx would have a problem with getting better warnings on their types.

Mon, Jul 1, 7:08 AM · Restricted Project
aaron.ballman added inline comments to D62829: [clang-tidy] Check for dynamically initialized statics in headers..
Mon, Jul 1, 6:08 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D63845: [WIP] Create a clang attribute that lets users specify LLVM attributes.

Just to preface these comments -- this is work in progress and not intended to go in as is / without discussion (but happily is indeed inviting discussion).

Mon, Jul 1, 5:58 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D63082: [Diagnostics] Added support for -Wint-in-bool-context.

I wanted to follow GCC’s behaviour -Wall but then dicussion moved to “tautological compare group” - I was fine with that too..
I can again revert it to -Wall...
If this should be off by default even on -Wall, then all work was useless..

Mon, Jul 1, 5:39 AM · Restricted Project

Thu, Jun 27

aaron.ballman added inline comments to D63276: [AST] Add FunctionDecl::getParametersSourceRange().
Thu, Jun 27, 5:02 AM · Restricted Project
aaron.ballman added a comment to D63082: [Diagnostics] Added support for -Wint-in-bool-context.

In terms of the code, I think this is ready to go. However, I'm still not happy with '*' in boolean context as a diagnostic. Perhaps appending something about the resulting value always being true|false would help most of these diagnostics be more obvious. blah in boolean context will always evaluate to true|false optionally followed by ; did you mean yada? would make it more obvious what is wrong with the code.

Thu, Jun 27, 4:55 AM · Restricted Project
aaron.ballman added a reviewer for D63845: [WIP] Create a clang attribute that lets users specify LLVM attributes: aaron.ballman.

What's the target use-case here? What can't be solved with normal attributes?
I wonder if this should go to cfe+llvm -dev lists first, it's kinda intrusive.
I also wonder if all these should cause a clang diagnostic, at least under -Wall.
How is versioning expected to be handled? New attribute vs old clang, and vice versa.

Thu, Jun 27, 4:46 AM · Restricted Project, Restricted Project

Tue, Jun 25

aaron.ballman added inline comments to D62977: [clang-tidy]: Google: new check 'google-upgrade-googletest-case'.
Tue, Jun 25, 5:09 AM · Restricted Project, Restricted Project, Restricted Project

Mon, Jun 24

aaron.ballman added inline comments to D60455: [SYCL] Implement SYCL device code outlining.
Mon, Jun 24, 1:28 PM · Restricted Project
aaron.ballman committed rGa612e34c1473: Augment location information when dumping the AST to JSON. (authored by aaron.ballman).
Augment location information when dumping the AST to JSON.
Mon, Jun 24, 1:08 PM
aaron.ballman added a reviewer for D63276: [AST] Add FunctionDecl::getParametersSourceRange(): aaron.ballman.
Mon, Jun 24, 11:27 AM · Restricted Project

Jun 21 2019

aaron.ballman added inline comments to D63082: [Diagnostics] Added support for -Wint-in-bool-context.
Jun 21 2019, 12:58 PM · Restricted Project
aaron.ballman added inline comments to D63082: [Diagnostics] Added support for -Wint-in-bool-context.
Jun 21 2019, 12:29 PM · Restricted Project
aaron.ballman added inline comments to D63423: [Diagnostics] Diagnose misused xor as pow.
Jun 21 2019, 10:31 AM · Restricted Project
aaron.ballman committed rG4c9def4a51ac: Ensure that top-level QualType objects also have a "kind" field when dumping… (authored by aaron.ballman).
Ensure that top-level QualType objects also have a "kind" field when dumping…
Jun 21 2019, 10:14 AM
aaron.ballman committed rG104b12980ccf: Print more type node information when dumping the AST to JSON. (authored by aaron.ballman).
Print more type node information when dumping the AST to JSON.
Jun 21 2019, 9:04 AM
aaron.ballman committed rG60294f9d3547: Add an automated note to files produced by gen_ast_dump_json_test.py. (authored by aaron.ballman).
Add an automated note to files produced by gen_ast_dump_json_test.py.
Jun 21 2019, 7:35 AM
aaron.ballman committed rGc07cfce23ad2: Print information about various type nodes when dumping the AST to JSON. (authored by aaron.ballman).
Print information about various type nodes when dumping the AST to JSON.
Jun 21 2019, 6:20 AM

Jun 20 2019

aaron.ballman committed rG75e23f8523bb: Print information about various ObjC expression nodes when dumping the AST to… (authored by aaron.ballman).
Print information about various ObjC expression nodes when dumping the AST to…
Jun 20 2019, 2:43 PM
aaron.ballman committed rG7f1b223a5a28: Print additional information about @encode expressions when dumping the AST to… (authored by aaron.ballman).
Print additional information about @encode expressions when dumping the AST to…
Jun 20 2019, 12:09 PM
aaron.ballman committed rG7dbb3a8fac6a: Print additional information on dependent scopes when dumping the AST to JSON. (authored by aaron.ballman).
Print additional information on dependent scopes when dumping the AST to JSON.
Jun 20 2019, 11:55 AM
aaron.ballman committed rG1fffe8d6eede: Dump more information about expressions involving temporaries when dumping the… (authored by aaron.ballman).
Dump more information about expressions involving temporaries when dumping the…
Jun 20 2019, 9:22 AM
aaron.ballman committed rG0ac17bef251e: Removing a helper function that was trivial to inline into its only use; NFC. (authored by aaron.ballman).
Removing a helper function that was trivial to inline into its only use; NFC.
Jun 20 2019, 8:09 AM
aaron.ballman committed rG20fe9e54525c: Add test cases for explicit casts when dumping the AST to JSON; NFC. (authored by aaron.ballman).
Add test cases for explicit casts when dumping the AST to JSON; NFC.
Jun 20 2019, 8:03 AM
aaron.ballman committed rGd91b1edf7b5d: Dump more information about construct expressions (resolved and unresolved)… (authored by aaron.ballman).
Dump more information about construct expressions (resolved and unresolved)…
Jun 20 2019, 6:19 AM

Jun 19 2019

aaron.ballman committed rG07e6da933049: Print whether a generic selection expression is result dependent when dumping… (authored by aaron.ballman).
Print whether a generic selection expression is result dependent when dumping…
Jun 19 2019, 1:15 PM
aaron.ballman committed rG709a769cdd4e: Print out the union field being initialized by an InitListExpr when dumping the… (authored by aaron.ballman).
Print out the union field being initialized by an InitListExpr when dumping the…
Jun 19 2019, 12:39 PM
aaron.ballman committed rG91f7265759cf: Dump the value calculated by a constant expression when dumping the AST to JSON. (authored by aaron.ballman).
Dump the value calculated by a constant expression when dumping the AST to JSON.
Jun 19 2019, 12:12 PM
aaron.ballman committed rG5f84ebe8dec2: Switching this test to use output generated by script; NFC. (authored by aaron.ballman).
Switching this test to use output generated by script; NFC.
Jun 19 2019, 12:07 PM
aaron.ballman accepted D63535: [clang][AST] ASTNameGenerator: A refactoring of CodegenNameGeneratorImpl (NFC)..

LGTM with a few nits.

Jun 19 2019, 11:37 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D63423: [Diagnostics] Diagnose misused xor as pow.

Perhaps the author can run the check over a large corpus of code to see whether fps come up in practice? (The presence of fps will suggest to not warn in macros, but the absence of fps won't tell us too much.)

Sorry, I have no time nor spare computers to check big codebases. As @jfb said, let's start with a conservative approach and possibly improve it in the future.

Jun 19 2019, 11:33 AM · Restricted Project
aaron.ballman added a comment to D63423: [Diagnostics] Diagnose misused xor as pow.
In D63423#1550713, @jfb wrote:
In D63423#1550697, @jfb wrote:

What I meant with macros was that I don't think we should warn on:

#define LEGIT(a, b) ({ work work work; a ^ b; work work work; })

LEGIT(10, 5);

If the constants are inline in the macros then sure, warn there. Basically: if you literally wrote CONSTANT ^ CONSTANT and it looks fishy, let's warn. If token-pasting wrote it for you and it looks fishy, let's not warn.

So you would not want to see this code diagnosed?

#define POW(x, y) (x ^ y)

void f() {
  int two_to_the_derp = POW(2, 16);
}

I'm not certain I understand the rationale for why we would not want to diagnose such a construct. Do we have reason to expect a lot of false positives?

If you wrote CONSTANT_2_OR_10 ^ CONSTANT I'm pretty sure you messed up. Macros are opaque, and I'm not sure you clearly messed up anymore. Let's take the clear win, see what falls out, and only expand our reach if we have reason to do so. Without trying on huge codebases I don't know whether we'll get a lot of false positive. I'd rather be conservative and not cause headache.

Jun 19 2019, 11:16 AM · Restricted Project
aaron.ballman committed rG22a5a61674c5: Add test cases for dumping record definition data to JSON; NFC. (authored by aaron.ballman).
Add test cases for dumping record definition data to JSON; NFC.
Jun 19 2019, 10:47 AM
aaron.ballman added a comment to D63423: [Diagnostics] Diagnose misused xor as pow.
In D63423#1550697, @jfb wrote:

What I meant with macros was that I don't think we should warn on:

#define LEGIT(a, b) ({ work work work; a ^ b; work work work; })

LEGIT(10, 5);

If the constants are inline in the macros then sure, warn there. Basically: if you literally wrote CONSTANT ^ CONSTANT and it looks fishy, let's warn. If token-pasting wrote it for you and it looks fishy, let's not warn.

Jun 19 2019, 10:39 AM · Restricted Project
aaron.ballman added inline comments to D63535: [clang][AST] ASTNameGenerator: A refactoring of CodegenNameGeneratorImpl (NFC)..
Jun 19 2019, 10:10 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D63423: [Diagnostics] Diagnose misused xor as pow.

... But I go thru codesearch and this is just one case I think.

I think we should warn in macros... In codesearch case ,they are many more true positives, then false negatives.

#define MAX_T_U32 ((2^32)-1)

Ouch.

Jun 19 2019, 9:49 AM · Restricted Project
aaron.ballman added a comment to D63423: [Diagnostics] Diagnose misused xor as pow.

We will be noisy in this case

#define IRQ_CHINT4_LEVEL        (11 ^ 7)
#define IRQ_CHINT3_LEVEL        (10 ^ 7)
#define IRQ_CHINT2_LEVEL        (9 ^ 7)
#define IRQ_CHINT1_LEVEL        (8 ^ 7)
Jun 19 2019, 9:40 AM · Restricted Project
aaron.ballman added inline comments to D61552: [clang] Adapt ASTMatcher to explicit(bool) specifier.
Jun 19 2019, 9:10 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D63423: [Diagnostics] Diagnose misused xor as pow.

The only remaining question is, as your said, whether or not to diagnose xors in macros. @regerh @jfb ?

IMO it's better to not warn about xors in macros-- I like the current tradeoffs.

Jun 19 2019, 9:08 AM · Restricted Project
aaron.ballman added inline comments to D63423: [Diagnostics] Diagnose misused xor as pow.
Jun 19 2019, 9:08 AM · Restricted Project
aaron.ballman added a comment to D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility.

clang-format the patch

Jun 19 2019, 8:58 AM
aaron.ballman accepted D61552: [clang] Adapt ASTMatcher to explicit(bool) specifier.

LGTM aside from a documentation nit.

Jun 19 2019, 8:52 AM · Restricted Project, Restricted Project
aaron.ballman accepted D63451: P0840R2: support for [[no_unique_address]] attribute.

The attribute parts LGTM! You can change the TargetItaniumCXXABI part in a follow-up commit if you'd prefer.

Jun 19 2019, 8:38 AM · Restricted Project, Restricted Project
aaron.ballman closed D63490: Script for generating AST JSON dump test cases.

Committed in r363820; we'll look into integrating the script into lit as you suggested for a possible follow-up.

Jun 19 2019, 8:23 AM
aaron.ballman committed rG1ad10137c9b5: Add a script to help generate expected test output for dumping the AST to JSON. (authored by aaron.ballman).
Add a script to help generate expected test output for dumping the AST to JSON.
Jun 19 2019, 8:23 AM
aaron.ballman committed rG7556615a9d0b: Change the way we output templates for JSON AST dumping and dump information… (authored by aaron.ballman).
Change the way we output templates for JSON AST dumping and dump information…
Jun 19 2019, 8:22 AM
aaron.ballman accepted D62952: [analyzer] SARIF: Add EOF newline; replace diff_sarif.

LGTM!

Jun 19 2019, 8:07 AM · Restricted Project, Restricted Project
aaron.ballman added a comment to D61552: [clang] Adapt ASTMatcher to explicit(bool) specifier.

fixed requested changes.

Are you getting errors from running it, or just incorrect output?

the issue happens to me even on master so i suppose the input is correct.
here is the error report:

[tyker@tyker tools]$ ./dump_ast_matchers.py 
*** Unparsable: " #ifndef LLVM_CLANG_ASTMATCHERS_ASTMATCHERS_H #define LLVM_CLANG_ASTMATCHERS_ASTMATCHERS_H " ***
*** Unparsable: " #include "clang/AST/ASTContext.h" #include "clang/AST/ASTTypeTraits.h" #include "clang/AST/Attr.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclFriend.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/DeclTemplate.h" #include "clang/AST/Expr.h" #include "clang/AST/ExprCXX.h" #include "clang/AST/ExprObjC.h" #include "clang/AST/NestedNameSpecifier.h" #include "clang/AST/OpenMPClause.h" #include "clang/AST/OperationKinds.h" #include "clang/AST/Stmt.h" #include "clang/AST/StmtCXX.h" #include "clang/AST/StmtObjC.h" #include "clang/AST/StmtOpenMP.h" #include "clang/AST/TemplateBase.h" #include "clang/AST/TemplateName.h" #include "clang/AST/Type.h" #include "clang/AST/TypeLoc.h" #include "clang/ASTMatchers/ASTMatchersInternal.h" #include "clang/ASTMatchers/ASTMatchersMacros.h" #include "clang/Basic/AttrKinds.h" #include "clang/Basic/ExceptionSpecificationType.h" #include "clang/Basic/IdentifierTable.h" #include "clang/Basic/LLVM.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/Specifiers.h" #include "clang/Basic/TypeTraits.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Casting.h" #include "llvm/Support/Compiler.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/Regex.h" #include <cassert> #include <cstddef> #include <iterator> #include <limits> #include <string> #include <utility> #include <vector> " ***
*** Unparsable: " namespace clang {" ***
*** Unparsable: " using DeclarationMatcher = internal::Matcher<Decl>;" ***
*** Unparsable: " using StatementMatcher = internal::Matcher<Stmt>;" ***
*** Unparsable: " using TypeMatcher = internal::Matcher<QualType>;" ***
*** Unparsable: " using TypeLocMatcher = internal::Matcher<TypeLoc>;" ***
*** Unparsable: " using NestedNameSpecifierMatcher = internal::Matcher<NestedNameSpecifier>;" ***
*** Unparsable: " using NestedNameSpecifierLocMatcher = internal::Matcher<NestedNameSpecifierLoc>;" ***
*** Unparsable: " using CXXCtorInitializerMatcher = internal::Matcher<CXXCtorInitializer>;" ***
Probing https://clang.llvm.org/doxygen/classclang_1_1Decl.html...

after this the script keep probing with no issues.
i tested using python 2.7 and python 3.7 the same error occurs

Jun 19 2019, 6:12 AM · Restricted Project, Restricted Project

Jun 18 2019

aaron.ballman added inline comments to D60455: [SYCL] Implement SYCL device code outlining.
Jun 18 2019, 3:15 PM · Restricted Project