[PATCH] D51211: [Sema] Emit -Wformat properly for bitfield promotions.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM then, thank you for the fix! Would you like me to commit this one for you? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D51211/new/ https://reviews.llvm.org/D51211

[PATCH] D55784: [clang-tidy] Update abseil-duration-comparison to handle macro arguments

aaron.ballman added inline comments. Comment at: clang-tidy/abseil/DurationComparisonCheck.cpp:25 +static bool IsValidMacro(const MatchFinder::MatchResult &Result, + const Expr *expr) { + if (!expr->getBeginLoc().isMacroID()) `expr` doesn

[PATCH] D55792: Allow direct navigation to static analysis checker documentation through SARIF exports

aaron.ballman marked 3 inline comments as done. aaron.ballman added a comment. In D55792#1334291 , @Szelethus wrote: > Thank you so much for working on this! The lack of a proper documentation is > indeed a weak points of the project. Yeah, the more we

[PATCH] D51211: [Sema] Emit -Wformat properly for bitfield promotions.

aaron.ballman closed this revision. aaron.ballman added a comment. I've commit as r349497, thank you! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D51211/new/ https://reviews.llvm.org/D51211 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D55792: Allow direct navigation to static analysis checker documentation through SARIF exports

aaron.ballman added a comment. In D55792#1334644 , @Szelethus wrote: > Global nit: I guess having both DESC and DOCS as variable/macro arg names is > confusing. Can we instead use DOCSURI/DocsUri? Sure, I'll make that change. > In D55792#1334339

[PATCH] D55784: [clang-tidy] Update abseil-duration-comparison to handle macro arguments

aaron.ballman added inline comments. Comment at: test/clang-tidy/abseil-duration-comparison.cpp:131 + // We should still transform the expression inside this macro invocation +#define VALUE_IF(v, e) v ? (e) : 0 + int a = VALUE_IF(1, 5 > absl::ToDoubleSeconds(d1)); -

[PATCH] D55737: [Utils] Attempt to fix clang.natvis following "[AST] Various optimizations in DeclarationName(Table)"

aaron.ballman added a comment. There was a bit more work to be done here, but this got me started. I commit extra fixes on top of your work in r349547 since I was able to test the behavior locally. Thank you for the help! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403)

aaron.ballman added a comment. In D55793#1335243 , @m4tx wrote: > In D55793#1333723 , @riccibruno > wrote: > > > In D55793#1333691 , @m4tx wrote: > > > > > Don't use `CXXRec

[PATCH] D55628: Add support for "labels" on push/pop directives in #pragma clang attribute

aaron.ballman added a comment. In D55628#1335841 , @erik.pilkington wrote: > After looking through some users of `#pragma clang attribute`, I don't think > that the begin/end solution is what we want here. Some users of this > attribute write macros tha

[PATCH] D55784: [clang-tidy] Update abseil-duration-comparison to handle macro arguments

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D55784#1334929 , @hwright wrote: > @aaron.ballman I am both grateful and sad that I don't possess the same macro > creativity as you d

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

aaron.ballman added inline comments. Comment at: clang/include/clang/AST/Decl.h:1357 + bool isARCPseudoStrong() const { return VarDeclBits.ARCPseudoStrong; } + void setARCPseudoStrong(bool ps) { VarDeclBits.ARCPseudoStrong = ps; } Can you update `ps` to be `P

[PATCH] D55552: [Sema] Better static assert diagnostics for expressions involving temporaries.

aaron.ballman added inline comments. Comment at: lib/AST/StmtPrinter.cpp:78 const PrintingPolicy &Policy, unsigned Indentation = 0, -StringRef NL = "\n", -const ASTContext *Context = nullptr) +StringRef NL = "\n", c

[PATCH] D55552: [Sema] Better static assert diagnostics for expressions involving temporaries.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, but you should wait a few days before committing in case @Quuxplusone has comments. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D2/new

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

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6117 + // to ensure that the variable is 'const' so that we can error on + // modification, which can otherwise overrelease. + VD->setType(Ty.withConst()); erik.pilkington wrote:

[PATCH] D55628: Add support for "labels" on push/pop directives in #pragma clang attribute

aaron.ballman accepted this revision. aaron.ballman marked an inline comment as done. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: clang/lib/Parse/ParsePragma.cpp:3161 + if (!Tok.is(tok::period)) { +PP.Diag(Tok.g

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

aaron.ballman added inline comments. Comment at: include/clang/Basic/AttrDocs.td:2673 + +Check `Software pipelining`, `Modulo scheduling` to get more details on optimisation. + "Check" -- check where? Perhaps "Search for" but I still don't think that's very sat

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

aaron.ballman added inline comments. Comment at: include/clang/Basic/AttrDocs.td:2673 + +Check `Software pipelining`, `Modulo scheduling` to get more details on optimisation. + alexey.lapshin wrote: > aaron.ballman wrote: > > "Check" -- check where? Perhaps "Sea

[PATCH] D55792: Allow direct navigation to static analysis checker documentation through SARIF exports

aaron.ballman added a comment. In D55792#1334886 , @Szelethus wrote: > This is awesome. Please wait for either @NoQ or @george.karpenkov to have the > final say. > > Edit: Maybe couple days I guess. I've been digging through these files quite > a lot in

[PATCH] D55792: Allow direct navigation to static analysis checker documentation through SARIF exports

aaron.ballman closed this revision. aaron.ballman added a comment. Thanks for the reviews! I've commit in r349812. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55792/new/ https://reviews.llvm.org/D55792 ___ cfe-commits mailing list cfe-com

[PATCH] D55949: Correctly handle function pointers returning a type marked nodiscard

aaron.ballman created this revision. aaron.ballman added reviewers: rsmith, Quuxplusone, erik.pilkington. When a function returns a type and that type was declared `[[nodiscard]]`, we diagnose any unused results from that call as though the function were marked nodiscard. The same behavior shoul

[PATCH] D55955: Properly diagnose [[nodiscard]] on the body of a range-based for loop

aaron.ballman created this revision. aaron.ballman added reviewers: rsmith, Quuxplusone, erik.pilkington. If the range-based for loop function body is not a compound statement, we would fail to diagnose discarded results from the statement. This only impacted range-based for loops because other

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

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6117 + // to ensure that the variable is 'const' so that we can error on + // modification, which can otherwise overrelease. + VD->setType(Ty.withConst()); erik.pilkington wrote:

[PATCH] D55955: Properly diagnose [[nodiscard]] on the body of a range-based for loop

aaron.ballman marked 3 inline comments as done. aaron.ballman added inline comments. Comment at: lib/Sema/SemaStmt.cpp:2846 diag::warn_empty_range_based_for_body); + DiagnoseUnusedExprResult(B); rsmith wrote: > rsmith wrote: > > While

[PATCH] D55949: Correctly handle function pointers returning a type marked nodiscard

aaron.ballman updated this revision to Diff 179318. aaron.ballman marked 3 inline comments as done. aaron.ballman added a comment. Updated based on review feedback. Moved `FunctionDecl::hasUnusedResultAttr()` and `FunctionDecl::getUnusedResultAttr()` to `CallExpr` to reduce logic duplication on

[PATCH] D55932: [Sema] Simplfy static_assert diagnostic code.

aaron.ballman added inline comments. Comment at: test/SemaCXX/static-assert.cpp:130 static_assert(std::is_same()), int>::value, "message"); -// expected-error@-1{{static_assert failed due to requirement 'std::is_same, int>::value' "message"}} +// expected-error@-1{{static_asse

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

aaron.ballman added inline comments. Comment at: include/clang/Basic/AttrDocs.td:2650 +Software Pipelining optimization is a technique used to optimize loops by + utilizing instructions level parallelism. It reorders loop instructions to + overlap iterations. As the result

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

aaron.ballman added inline comments. Comment at: clang/test/SemaObjC/externally-retained.m:14 + + EXT_RET int (^d)() = ^{return 0;}; + EXT_RET ObjCTy *e = 0; erik.pilkington wrote: > aaron.ballman wrote: > > Should this be useful for function pointer parameters

[PATCH] D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403)

aaron.ballman added inline comments. Comment at: clang-tidy/readability/DuplicatedAccessSpecifiersCheck.cpp:51 + diag(ASDecl->getLocation(), "duplicated access specifier") + << MatchedDecl + << FixItHint::CreateRemoval(ASDecl->getSourceRange()); ---

[PATCH] D56012: [clang-tidy] Be more liberal about literal zeroes in abseil checks

aaron.ballman added inline comments. Comment at: clang-tidy/abseil/DurationRewriter.cpp:110 + ast_matchers::internal::Matcher, InnerMatcher) { + return (N < Node.getNumInits() && + InnerMatcher.matches(*Node.getInit(N)->IgnoreParenImpCasts(), Finder, -

[PATCH] D55949: Correctly handle function pointers returning a type marked nodiscard

aaron.ballman requested review of this revision. aaron.ballman added a comment. There's enough churn based on the review feedback that this should probably have a second round of review just to be sure. Comment at: lib/AST/Expr.cpp:2281-2286 + // If there is no FunctionDe

[PATCH] D56012: [clang-tidy] Be more liberal about literal zeroes in abseil checks

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56012/new/ https://reviews.llvm.org/D56012 ___ cfe-commits mailing lis

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

aaron.ballman added a reviewer: tonic. aaron.ballman added a subscriber: tonic. aaron.ballman added a comment. Adding @tonic as a reviewer, who may have suggestions on how to improve the documentation wording. Comment at: include/clang/Basic/AttrDocs.td:2655 + dependence viol

[PATCH] D55955: Properly diagnose [[nodiscard]] on the body of a range-based for loop

aaron.ballman updated this revision to Diff 179475. aaron.ballman marked 3 inline comments as done. aaron.ballman added a comment. Updated based on review feedback. The lookahead wasn't too annoying to thread through, but I did run into surprises in TreeTransform where I can't use the lookahead

[PATCH] D55955: Properly diagnose [[nodiscard]] on the body of a range-based for loop

aaron.ballman marked 3 inline comments as done. aaron.ballman added inline comments. Comment at: include/clang/Sema/Sema.h:5337 ExprResult ActOnFinishFullExpr(Expr *Expr, SourceLocation CC, - bool DiscardedValue = false, +

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

aaron.ballman added a comment. The docs are getting much closer -- just a few grammatical things left to fix, I believe. Comment at: include/clang/Basic/AttrDocs.td:2582 specified for the subsequent loop. The directive allows vectorization, -interleaving, and unrolling to be

[PATCH] D56090: Add a matcher for members of an initializer list expression

aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. In D56090#1341023 , @hwright wrote: > @lebedev.ri Where do the appropriate tests live? (I couldn't find an obvious > subdirect

[PATCH] D54408: [ASTMatchers] Add matchers available through casting to derived

aaron.ballman added inline comments. Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:651 +llvm::Optional> +getNodeConstructorType(MatcherCtor targetCtor) { + const auto &ctors = RegistryData->nodeConstructors(); targetCtor -> TargetCtor Comme

[PATCH] D55932: [Sema] Simplfy static_assert diagnostic code.

aaron.ballman added inline comments. Comment at: test/SemaCXX/static-assert.cpp:130 static_assert(std::is_same()), int>::value, "message"); -// expected-error@-1{{static_assert failed due to requirement 'std::is_same, int>::value' "message"}} +// expected-error@-1{{static_asse

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

aaron.ballman added inline comments. Comment at: include/clang/Basic/AttrDocs.td:2582 specified for the subsequent loop. The directive allows vectorization, -interleaving, and unrolling to be enabled or disabled. Vector width as well -as interleave and unrolling count can be man

[PATCH] D56160: [clang-tidy] modernize-use-trailing-return check

aaron.ballman added inline comments. Comment at: test/clang-tidy/modernize-use-trailing-return.cpp:2 +// RUN: %check_clang_tidy %s modernize-use-trailing-return %t -- -- --std=c++14 + +namespace std { lebedev.ri wrote: > Missing test coverage: > * macros > * is t

[PATCH] D55932: [Sema] Simplfy static_assert diagnostic code.

aaron.ballman added inline comments. Comment at: test/SemaCXX/static-assert.cpp:130 static_assert(std::is_same()), int>::value, "message"); -// expected-error@-1{{static_assert failed due to requirement 'std::is_same, int>::value' "message"}} +// expected-error@-1{{static_asse

[PATCH] D55955: Properly diagnose [[nodiscard]] on the body of a range-based for loop

aaron.ballman updated this revision to Diff 179920. aaron.ballman marked an inline comment as done. aaron.ballman added a comment. Updated based on review feedback. The vast majority of the changes are from the removal of a default argument to `ActOnFullExpr()`. CHANGES SINCE LAST ACTION http

[PATCH] D55955: Properly diagnose [[nodiscard]] on the body of a range-based for loop

aaron.ballman marked 2 inline comments as done. aaron.ballman added inline comments. Comment at: include/clang/Sema/Sema.h:5337 ExprResult ActOnFinishFullExpr(Expr *Expr, SourceLocation CC, - bool DiscardedValue = false, +

[PATCH] D55949: Correctly handle function pointers returning a type marked nodiscard

aaron.ballman added a comment. Ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55949/new/ https://reviews.llvm.org/D55949 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55949: Correctly handle function pointers returning a type marked nodiscard

aaron.ballman closed this revision. aaron.ballman added a comment. Committed in r350317. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55949/new/ https://reviews.llvm.org/D55949 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:/

[PATCH] D55932: [Sema] Simplfy static_assert diagnostic code.

aaron.ballman added inline comments. Comment at: test/SemaCXX/static-assert.cpp:130 static_assert(std::is_same()), int>::value, "message"); -// expected-error@-1{{static_assert failed due to requirement 'std::is_same, int>::value' "message"}} +// expected-error@-1{{static_asse

[PATCH] D56090: Add a matcher for members of an initializer list expression

aaron.ballman added a subscriber: klimek. aaron.ballman added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:3527 + return N < Node.getNumInits() && + InnerMatcher.matches(*Node.getInit(N)->IgnoreParenImpCasts(), Finder, +

[PATCH] D56090: Add a matcher for members of an initializer list expression

aaron.ballman added a reviewer: klimek. aaron.ballman added a comment. Adding Manuel for the `hasArg()` questions. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56090/new/ https://reviews.llvm.org/D56090 ___ cfe-commits mailing list cfe-com

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

aaron.ballman added inline comments. Comment at: test/Sema/pragma-pipeline.cpp:3 + +#pragma clang loop pipeline(disable) /* expected-error {{expected unqualified-id}} */ +int main() { I think this error is pretty unfortunate -- it doesn't really help the user to

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: test/Sema/pragma-pipeline.cpp:3 + +#pragma clang loop pipeline(disable) /* expected-error {{expected unqualified-id}} */ +int main() { --

[PATCH] D55955: Properly diagnose [[nodiscard]] on the body of a range-based for loop

aaron.ballman marked 3 inline comments as done. aaron.ballman added inline comments. Comment at: test/Parser/switch-recovery.cpp:108 expected-error {{no member named 'x' in the global namespace; did you mean simply 'x'?}} \ - expected-warning 2 {{expr

[PATCH] D55337: NFC: Move dumpDeclRef to NodeDumper

aaron.ballman added inline comments. Comment at: include/clang/AST/TextNodeDumper.h:28 const comments::FullComment *> { + TextTreeStructure &TreeStructure; raw_ostream &OS; steveire wrote: > aaron.ballman wrote: > >

[PATCH] D55955: Properly diagnose [[nodiscard]] on the body of a range-based for loop

aaron.ballman closed this revision. aaron.ballman added a comment. Committed with minor modifications for the range-based for loop changes in r350404. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55955/new/ https://reviews.llvm.org/D55955 ___

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

aaron.ballman closed this revision. aaron.ballman added a comment. I commit this in r350414, thank you for the patch! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55710/new/ https://reviews.llvm.org/D55710 ___ cfe-commits mailing list cfe-

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

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a few minor nits/questions. Comment at: clang/docs/AutomaticReferenceCounting.rst:1747 +contrast with ``__unsafe_unretained``, an externally-ret

[PATCH] D56090: Add a matcher for members of an initializer list expression

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Thanks, both! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56090/new/ https://reviews.llvm.org/D56090 ___ cfe-commi

[PATCH] D56303: [clang-tidy] Handle case/default statements when simplifying boolean expressions

aaron.ballman added inline comments. Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:511 + StringRef Id) { + // binding to the returnStmt matched is pointless because we can't guarantee + // anything about the

[PATCH] D56303: [clang-tidy] Handle case/default statements when simplifying boolean expressions

aaron.ballman added inline comments. Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:386 - bool BoolValue = Bool->getValue(); + const bool BoolValue = Bool->getValue(); LegalizeAdulthood wrote: > JonasToth wrote: > > `const` on values is unc

[PATCH] D56354: [AST] Replace asserts with if() in SourceLocation accessors

aaron.ballman added a comment. 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 o

[PATCH] D55212: Handle alloc_size attribute on function pointers

aaron.ballman added reviewers: erik.pilkington, dexonsmith. aaron.ballman added a comment. In D55212#1347994 , @arichardson wrote: > I don't see an easy way of fixing the pragma clang attribute support for > this. > Would it be okay to commit this chang

[PATCH] D56395: [ASTMatchers] Improve assert message for broken parent map.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56395/new/ https://reviews.llvm.org/D56395 ___

[PATCH] D56354: [AST] Replace asserts with if() in SourceLocation accessors

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a formatting nit. Comment at: include/clang/AST/DeclarationName.h:735 + Name.getNameKind() != DeclarationName::CXXConversionFunctionNa

[PATCH] D56405: Split -Wdelete-non-virtual-dtor into two groups

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56405/new/ https://reviews.llvm.org/D56405 ___

[PATCH] D56405: Split -Wdelete-non-virtual-dtor into two groups

aaron.ballman added inline comments. Comment at: include/clang/Basic/DiagnosticGroups.td:108-109 def DeleteNonVirtualDtor : DiagGroup<"delete-non-virtual-dtor">; +def DeleteAbstractNonVirtualDtor : DiagGroup<"delete-abstract-non-virtual-dtor", +

[PATCH] D56405: Split -Wdelete-non-virtual-dtor into two groups

aaron.ballman added inline comments. Comment at: include/clang/Basic/DiagnosticGroups.td:108-109 def DeleteNonVirtualDtor : DiagGroup<"delete-non-virtual-dtor">; +def DeleteAbstractNonVirtualDtor : DiagGroup<"delete-abstract-non-virtual-dtor", +

[PATCH] D56407: Implement the TreeStructure interface through the TextNodeDumper

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. I think this is a good approach to solving the problem, so let's go this route. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56407/new/ https://

[PATCH] D56415: NFC: Port QueryParser to StringRef

aaron.ballman added inline comments. Comment at: clang-query/QueryParser.cpp:36 + if (Line.front() == '#') { +Line = {}; return StringRef(); steveire wrote: > kristina wrote: > > I don't think this is the best way of handling it, in fact I'm pretty > >

[PATCH] D55337: NFC: Move dumpDeclRef to NodeDumper

aaron.ballman added inline comments. Comment at: include/clang/AST/TextNodeDumper.h:28 const comments::FullComment *> { + TextTreeStructure &TreeStructure; raw_ostream &OS; steveire wrote: > steveire wrote: > > stev

[PATCH] D56415: NFC: Port QueryParser to StringRef

aaron.ballman added inline comments. Comment at: clang-query/QueryParser.cpp:33 + if (Line.empty()) +return StringRef(Line.begin(), 0); steveire wrote: > aaron.ballman wrote: > > Why not just return `Line`? > It is the pre-existing behavior to return a zer

[PATCH] D56415: NFC: Port QueryParser to StringRef

aaron.ballman added inline comments. Comment at: clang-query/QueryParser.cpp:33 + if (Line.empty()) +return StringRef(Line.begin(), 0); steveire wrote: > steveire wrote: > > aaron.ballman wrote: > > > steveire wrote: > > > > aaron.ballman wrote: > > > > >

[PATCH] D56415: NFC: Port QueryParser to StringRef

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from the nits pointed out; you can fix and commit without another round of review. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D55337: NFC: Move dumpDeclRef to NodeDumper

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Once D56407 lands, the rebased changes here LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55337/new/ http

[PATCH] D56444: [AST] RecursiveASTVisitor visits lambda classes when implicit visitation is on.

aaron.ballman added a comment. In D56444#1350849 , @JonasToth wrote: > In D56444#1350746 , @sammccall wrote: > > > @klimek: would it be better to preserve the odd behavior of the > > `functionDecl()` matcher, and a

[PATCH] D56444: [AST] RecursiveASTVisitor visits lambda classes when implicit visitation is on.

aaron.ballman added a comment. In D56444#1351067 , @klimek wrote: > In D56444#1351056 , @aaron.ballman > wrote: > > > In D56444#1350849 , @JonasToth > > wrote: > > > > > In

[PATCH] D56444: [AST] RecursiveASTVisitor visits lambda classes when implicit visitation is on.

aaron.ballman added a comment. In D56444#1351096 , @sammccall wrote: > In D56444#1351056 , @aaron.ballman > wrote: > > > Given that, I kind of think we should have functionDecl() match only > > functions, and give

[PATCH] D56444: [AST] RecursiveASTVisitor visits lambda classes when implicit visitation is on.

aaron.ballman added a comment. In D56444#1351127 , @steveire wrote: > 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 fo

[PATCH] D55488: Add utility for dumping a label with child nodes

aaron.ballman added a comment. @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. Comment at: include/clang/AST/TextNodeDumper.h:43 + /// Add a child of the current no

[PATCH] D56444: [AST] RecursiveASTVisitor visits lambda classes when implicit visitation is on.

aaron.ballman added a comment. Can you add a test that checks that a lambda declared within another lambda also traverses as expected? In D56444#1351145 , @sammccall wrote: > In D56444#1351128 , @aaron.ballman >

[PATCH] D55488: Add utility for dumping a label with child nodes

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Aside from cosmetic nits, this LGTM. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55488/new/ https://reviews.llvm.org/D55488

[PATCH] D55492: Implement Attr dumping in terms of visitors

aaron.ballman added inline comments. Comment at: include/clang/AST/TextNodeDumper.h:187 + +// Implements Visit methods for Attrs +#include "clang/AST/AttrTextNodeDump.inc" Add a full stop to the end of the comment. Comment at: lib/AST/ASTDumper

[PATCH] D55491: Implement TemplateArgument dumping in terms of Visitor

aaron.ballman added inline comments. Comment at: include/clang/AST/TemplateArgumentVisitor.h:23 + +template struct make_ref { using type = T &; }; +template struct make_const_ref { using type = const T &; }; `std::add_lvalue_reference<>` already does this, so d

[PATCH] D56405: Split -Wdelete-non-virtual-dtor into two groups

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56405/new/ https://reviews.llvm.org/D56405 ___ cfe-commits mailing lis

[PATCH] D55491: Implement TemplateArgument dumping in terms of Visitor

aaron.ballman added inline comments. Comment at: lib/AST/ASTDumper.cpp:449-450 +void VisitPackTemplateArgument(const TemplateArgument &TA) { + for (TemplateArgument::pack_iterator I = TA.pack_begin(), + E = TA.pack_end(); +

[PATCH] D55646: [ASTImporter] Make ODR diagnostics warning by default

aaron.ballman added a comment. 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

[PATCH] D56444: [AST] RecursiveASTVisitor visits lambda classes when implicit visitation is on.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM with one minor testing request (conditional on the test passing, of course). Comment at: unittests/Tooling/RecursiveASTVisitorTests/LambdaExpr.cpp:68 }

[PATCH] D56552: [clang-tidy] update FunctionSizeCheck for D56444

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56552/new/ https://reviews.llvm.org/D56552 _

[PATCH] D54450: Get the correct range of tokens for preprocessor conditions

aaron.ballman closed this revision. aaron.ballman added a comment. I've commit in r350891 (clang part) and r350892 (clang-tools-extra part). If @rsmith has concerns, we can fix or revert when raised. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54450/new/ https://reviews.llvm.org/D544

[PATCH] D67140: [analyzer][NFC] Fix inconsistent references to checkers as "checks"

aaron.ballman added a comment. In D67140#1664406 , @gribozavr wrote: > In D67140#1664106 , @NoQ wrote: > > > In D67140#1659982 , @gribozavr > > wrote: > > > > > We should ta

[PATCH] D67368: [NFCI]Create CommonAttributeInfo Type as base type of *Attr and ParsedAttr.

aaron.ballman added a comment. Thank you for doing all this work -- in general, this is looking great and really cleans things up! Comment at: clang/include/clang/Basic/Attr.td:2266 let Spellings = [CXX11<"", "maybe_unused", 201603>, GCC<"unused">, - C2x<"

[PATCH] D66856: [Sema] Suppress -Wformat diagnostics for bool types when printed using %hhd

aaron.ballman added a comment. In D66856#1650803 , @aaron.ballman wrote: > 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

[PATCH] D52524: Add -Wpoison-system-directories warning

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. 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. ===

[PATCH] D67368: [NFCI]Create CommonAttributeInfo Type as base type of *Attr and ParsedAttr.

aaron.ballman accepted this revision. aaron.ballman marked an inline comment as done. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Thank you for this refactoring -- it was large, but I think the results from it are really good. Comment

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

aaron.ballman added a comment. Mostly LGTM but a few nits. Comment at: clang/include/clang/Basic/DiagnosticGroups.td:65 +def ObjCSignedCharBoolImplicitIntConversion : + DiagGroup<"objc-signed-char-bool-implicit-int-conversion">; +def ImplicitIntConversion : DiagGroup<"implicit

[PATCH] D65917: [clang-tidy] Added check for the Google style guide's category method naming rule.

aaron.ballman added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst:41 + +.. option:: ExpectedPrefixes + stephanemoore wrote: > This option seems to describe a list of class prefixes that are

[PATCH] D67159: [clang] New __attribute__((__clang_arm_mve_alias)).

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:596 } +def ArmMveAlias : InheritableAttr, TargetSpecificAttr { + let Spellings = [Clang<"__clang_arm_mve_alias">]; Add a newline before the attribute definition for some separati

[PATCH] D64671: [clang-tidy] New check: misc-init-local-variables

aaron.ballman added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-init-variables.rst:40 + +Options +--- This is still missing the documentation for `MathHeader` and its default value. CHANGES SINCE LAST ACTION ht

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

aaron.ballman added a comment. Thank you for working on this check! In D67545#1672106 , @balazske wrote: > C++17 makes things more difficult because the align is probably handled by > `operator new`, probably not, depending on the defined allocation func

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp:53 +CheckFactories.registerCheck( +"cert-default-operator-new"); CheckFactories.registerCheck( balazske wrote: > aaron.ballman wrote: > > balazs

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

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clang/test/Sema/objc-bool-constant-conversion-fixit.m:37 + + b = 1 ? 2 : 3; + // CHECK: b = 1 ? 2 ? YES : NO : 3 ? YES : NO; ---

[PATCH] D65917: [clang-tidy] Added check for the Google style guide's category method naming rule.

aaron.ballman added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst:41 + +.. option:: ExpectedPrefixes + stephanemoore wrote: > aaron.ballman wrote: > > stephanemoore wrote: > > > This option

<    12   13   14   15   16   17   18   19   20   21   >