[PATCH] D84924: [clang-tidy][WIP] Added command line option `fix-notes`

aaron.ballman added a comment. In D84924#2421116 , @njames93 wrote: > Fix documentation. > > Ping?? Ah, sorry, this fell to the bottom of my review list because the title still says [WIP] and so I thought there was more work being done on it. You may wan

[PATCH] D91898: [attributes] Add a facility for defining and enforcing a Trusted Computing Base.

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3593 +def EnforceTCBLeaf : InheritableAttr { + let Spellings = [Clang<"enforce_tcb_leaf">]; + let Subjects = SubjectList<[Function]>; Are these two attributes mutually exclusive,

[PATCH] D91495: [clang-tidy] false-positive for bugprone-redundant-branch-condition in case of passed-by-ref params

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, thank you for the fix! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91495/new/ https://reviews.llvm.org/D91495 ___ cf

[PATCH] D91893: [clang-tidy] performance-unnecessary-copy-initialization: Prevent false positives when dependent variable is modified.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Thanks for verifying! Aside from the minor nit with `isa<>`, this LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91893/new/ ht

[PATCH] D91979: [Clang][Attr] Introduce the `assume` function attribute

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:742 "the argument to %0 has side effects that will be discarded">, - InGroup>; + InGroup; +def warn_assume_attribute_string_unknown : Warning< aaron.ballman wro

[PATCH] D92800: [Clang] Make nomerge attribute a function attribute as well as a statement attribute.

aaron.ballman added a comment. In general, I think this is reasonable -- there's a bit more work to be done before it's ready to land, but this is heading in the right direction. Comment at: clang/include/clang/Basic/Attr.td:559 +/// A attribute is either a declaration attribu

[PATCH] D93164: [AST] Add generator for source location introspection

aaron.ballman added a comment. Do I understand correctly that the workflow is to use the new dumping tool to generate the needed JSON file that then gets used as input to generate_cxx_src_locs.py which creates NodeLocationIntrospection.cpp/.h that then gets used by clang-query (eventually)? So

[PATCH] D91979: [Clang][Attr] Introduce the `assume` function attribute

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM (found one very minor late nit in a cmake file though). Thank you for the discussion on this! Comment at: clang/lib/Sema/CMakeLists.txt:3 FrontendOpenM

[PATCH] D93324: Avoid isImplicit call on object which could be nullptr

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Good catch, LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93324/new/ https://reviews.llvm.org/D93324 __

[PATCH] D92756: [clang-tidy] Support all YAML supported spellings for bools in CheckOptions.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92756/new/ https://reviews.llvm.org/D92756 __

[PATCH] D92920: [clang-tidy] Add a diagnostic callback to parseConfiguration

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp:400 +DiagCallback Handler) { + llvm::yaml::Input Input(Config, nullptr, Handler ? diagHandlerImpl : nullptr, + &Handler); -

[PATCH] D72241: [clang-tidy] new altera single work item barrier check

aaron.ballman added a comment. In D72241#2449426 , @ffrankies wrote: > @aaron.ballman Thank you! If there are no further comments, could you please > commit this on my behalf? My GitHub username is ffrankies > . I'd be happ

[PATCH] D93102: [Clang][Sema] Detect section type conflicts between functions and variables

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3048 D->addAttr(NewAttr); +if (auto FD = dyn_cast(D)) + if (auto SA = dyn_cast(NewAttr)) Does this need to be limited to function declarations? It seems to me that this

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:5095 +Clang implements a check for ``called_once`` parameters, +``-Wcalled-once-parameter``. It is on by default and finds the following +violations: Comme

[PATCH] D92920: [clang-tidy] Add a diagnostic callback to parseConfiguration

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LG! Comment at: clang-tools-extra/clang-tidy/ClangTidyOptions.cpp:400 +DiagCallback Handler) { + llvm::yaml::Input Input(Config, nu

[PATCH] D92800: [Clang] Make nomerge attribute a function attribute as well as a statement attribute.

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1321 let Documentation = [NoMergeDocs]; + let Subjects = SubjectList<[Function], ErrorDiag>; + let SimpleHandler = 1; Related to my comments in ClangAttrEmitter.cpp, I think y

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

aaron.ballman added a comment. Is the attribute considered to be a property of the parameter or a property of the function the parameter is declared in? e.g., void someOtherFunc(void (^cb)(void)) { if (something()) cb(); } void barWithCallback(void (^callback)(void) __attribut

[PATCH] D93402: [clang-tidy] prevent readability-identifier-naming from triggering on implicit VarDecls in coroutines

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from some small nits, thank you for the fix! Do you need me to commit on your behalf? (If so, are you okay with using `Emma Blink ` for patch attribution?)

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

aaron.ballman added a comment. In D92039#2458042 , @vsavchenko wrote: > That was a tough choice whether we should do it in the analyzer or not. > The analyzer check would've been easier in terms of existing infrastructure, > path sensitivity, and IPA. It

[PATCH] D93102: [Clang][Sema] Detect section type conflicts between functions and variables

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a minor nit. Thank you for the patch! Do you need me to commit on your behalf? If so, are you okay with `Tomas Matheson ` for patch attribution? ==

[PATCH] D93110: [analyzer] Implement a first version of suppressions via attributes

aaron.ballman added a comment. In D93110#2448587 , @vsavchenko wrote: > Right now, I reused existing `suppress` attribute. However I did it in a > rather unconventional manner. I allowed 0 arguments in one spelling and >1 > in another, which seems odd.

[PATCH] D93402: [clang-tidy] prevent readability-identifier-naming from triggering on implicit VarDecls in coroutines

aaron.ballman closed this revision. aaron.ballman added a comment. In D93402#2458591 , @0x1eaf wrote: >> Do you need me to commit on your behalf? > > Yes, I don't have commit access. The attribution line looks good to me. Thank > you! 🙂 I've commit on yo

[PATCH] D93325: Add srcloc output to clang-query

aaron.ballman added a comment. Thank you for this, it looks really useful! Comment at: clang-tools-extra/clang-query/Query.cpp:94 +void dumpLocations(llvm::raw_ostream &OS, ast_type_traits::DynTypedNode node, + ASTContext &Ctx, const DiagnosticsEngine &Diags

[PATCH] D92577: Don't reject tag declarations in for loop clause-1

aaron.ballman added a comment. In D92577#2445714 , @aaron.ballman wrote: > Ping Ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92577/new/ https://reviews.llvm.org/D92577 ___ cfe-commits mailing

[PATCH] D92800: [Clang] Make nomerge attribute a function attribute as well as a statement attribute.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Thank you for this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92800/new/ https://reviews.llvm.org/D92800 __

[PATCH] D92577: Don't reject tag declarations in for loop clause-1

aaron.ballman updated this revision to Diff 312494. aaron.ballman added a comment. Updating based on review feedback. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92577/new/ https://reviews.llvm.org/D92577 Files: clang/lib/Sema/SemaStmt.cpp clang/test/Sema/for.c Index: clang/tes

[PATCH] D92577: Don't reject tag declarations in for loop clause-1

aaron.ballman added a comment. In D92577#2458903 , @rjmccall wrote: > Please test that there's actually an object declared and that it's not *just* > a tag declaration. Good catch, I've corrected this and added some more test coverage. CHANGES SINCE LA

[PATCH] D93102: [Clang][Sema] Detect section type conflicts between functions and variables

aaron.ballman closed this revision. aaron.ballman added a comment. In D93102#2460034 , @tmatheson wrote: > In D93102#2458433 , @aaron.ballman > wrote: > >> LGTM aside from a minor nit. Thank you for the patch! Do y

[PATCH] D92577: Don't reject tag declarations in for loop clause-1

aaron.ballman updated this revision to Diff 312613. aaron.ballman added a comment. Updating based on review feedback. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92577/new/ https://reviews.llvm.org/D92577 Files: clang/lib/Sema/SemaStmt.cpp clang/test/Sema/for.c Index: clang/tes

[PATCH] D92577: Don't reject tag declarations in for loop clause-1

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:1846 +(*Iter)->setInvalidDecl(); + } } rjmccall wrote: > You can have multiple tag declarations because of complex declarators or > type-specifiers, e.g. `

[PATCH] D72241: [clang-tidy] new altera single work item barrier check

aaron.ballman closed this revision. aaron.ballman added a comment. In D72241#2461921 , @ffrankies wrote: > @aaron.ballman hmm, that is strange. I've rebased the patch and updated the > diff, let me know if this one doesn't work either or there's something

[PATCH] D92577: Don't reject tag declarations in for loop clause-1

aaron.ballman closed this revision. aaron.ballman added a comment. Thank you for the review! I've commit in 2d2498ec6c42b12eae873257e6ddcefe8348 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92577/new/ https://r

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

aaron.ballman added a comment. In D92039#2462255 , @vsavchenko wrote: > In D92039#2458392 , @aaron.ballman > wrote: > >> > > > >> There have been efforts to add cross-TU support to the static analyzer, so >> I'm

[PATCH] D92039: [-Wcalled-once-parameter] Introduce 'called_once' attribute

aaron.ballman added a comment. In D92039#2462995 , @vsavchenko wrote: > In D92039#2462889 , @aaron.ballman > wrote: > >> Your test cases suggest that this is not inter-procedurally checked; it >> seems to require

[PATCH] D89031: [SVE] Add support to vectorize_width loop pragma for scalable vectors

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3356 EnumArgument<"State", "LoopHintState", - ["enable", "disable", "numeric", "assume_safety", "full"], - ["Enable", "Disable",

[PATCH] D92006: Refactoring the attribute plugin example to fit the new API

aaron.ballman closed this revision. aaron.ballman added a comment. In D92006#2465239 , @psionic12 wrote: > Hi, could anyone help to commit this? I'm happy to do so, sorry about not asking earlier. I've commit on your behalf in b2ba6867eac10874bd279c73963

[PATCH] D92006: Refactoring the attribute plugin example to fit the new API

aaron.ballman added a comment. In D92006#2466114 , @DavidSpickett wrote: > The last update didn't update the expected errors. I have done so in > 9a93f95fce91fb4616cee0f307b564b253789282 >

[PATCH] D89031: [SVE] Add support to vectorize_width loop pragma for scalable vectors

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3356 EnumArgument<"State", "LoopHintState", - ["enable", "disable", "numeric", "assume_safety", "full"], - ["Enable", "Disable",

[PATCH] D93596: [ASTMatchers] Traverse-ignore range-for implementation details

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93596/new/ https://reviews.llvm.org/D93596 __

[PATCH] D84924: [clang-tidy] Added command line option `fix-notes`

aaron.ballman added a comment. In D84924#2465745 , @njames93 wrote: > In D84924#2446075 , @aaron.ballman > wrote: > >> In D84924#2184132 , @njames93 wrote: >> >>> This is ve

[PATCH] D84924: [clang-tidy] Added command line option `fix-notes`

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h:231 +/// If \p AnyFix is true and there is no FixIt attached to the Message, +/// returns the first FixIt attached to any notes in the message. +/// If no FixIt is found, r

[PATCH] D91302: Handle template instantiations better in clang-tidy check

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp:26-28 + // The first argument of an overloaded member operator is the implicit object + // argument of the method which should not be matched against a paramet

[PATCH] D91303: Simplify implementation of container-size-empty

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp:56 - hasEitherOperand(ignoringImpCasts( - anyOf(integerLiteral(equals(1)), -

[PATCH] D80623: WIP: Add an API to simplify setting TraversalKind in clang-tidy matchers

aaron.ballman added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchFinder.h:118 +/// behavior of clang-tidy. +virtual llvm::Optional +getCheckTraversalKind() const; steveire wrote: > sammccall wrote: > > I don't really get why th

[PATCH] D93110: [analyzer] Implement a first version of suppressions via attributes

aaron.ballman added a comment. In D93110#2460375 , @vsavchenko wrote: > In D93110#2458613 , @aaron.ballman > wrote: > >> Have you explored how this attribute will work with clang frontend >> diagnostics or clang-t

[PATCH] D93630: [Attr] Apply GNU-style attributes to expression statements

aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. Thank you for working on this, it's a problem that's annoyed me for a while but I've never found a satisfactory solution to. Unfortunately, it causes us to reject valid

[PATCH] D91302: Handle template instantiations better in clang-tidy check

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp:56 +AST_MATCHER(Expr, usedInBooleanContext) { + std::string exprName = "__booleanContextExpr"; + auto result = Comment at:

[PATCH] D93630: [Attr] Apply GNU-style attributes to expression statements

aaron.ballman added a comment. In D93630#2468168 , @vsavchenko wrote: > @aaron.ballman I totally agree, but I also would like to understand. > `__attribute__` is a GNU extension, right? Correct. > Then why does it affect the grammar of C? I always thou

[PATCH] D80623: WIP: Add an API to simplify setting TraversalKind in clang-tidy matchers

aaron.ballman added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchFinder.h:118 +/// behavior of clang-tidy. +virtual llvm::Optional +getCheckTraversalKind() const; steveire wrote: > aaron.ballman wrote: > > steveire wrote: > > >

[PATCH] D91302: Handle template instantiations better in clang-tidy check

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91302/new/ https://reviews.llvm.org/D91302 ___ cfe-commits mailing list cfe-commit

[PATCH] D80623: WIP: Add an API to simplify setting TraversalKind in clang-tidy matchers

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! I'm assuming that @sammccall may be busy given the holiday season, so I think it's fine to land this as-is if you don't hear from him in the next day. If he has additional

[PATCH] D91303: [clang-tidy] readability-container-size-empty: simplify implementation

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Comment at: clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp:101 - ignoringImpCasts(stringLiteral(hasSize(0))), - ignoringImpCasts(cxxBindTemporaryExpr(has(DefaultConstruct

[PATCH] D93630: [Attr] Apply GNU-style attributes to expression statements

aaron.ballman added a comment. In D93630#2468241 , @vsavchenko wrote: > In D93630#2468197 , @aaron.ballman > wrote: > >> However, taking a step back -- what attributes would need this functionality >> (and couldn'

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. I think this LGTM aside from a few minor nits. Thank you for working on this! Comment at: clang-tools-extra/docs/clang-tidy/checks/cert-sig30-c.rst:12-14 +This

[PATCH] D86559: [Sema, CodeGen] Allow [[likely]] and [[unlikely]] on labels

aaron.ballman added a comment. In D86559#2371581 , @Mordante wrote: > In D86559#2371058 , @aaron.ballman > wrote: > >> In D86559#2369317 , @Mordante wrote: >> >>> Then me tr

[PATCH] D90188: Add support for attribute 'using_if_exists'

aaron.ballman added inline comments. Comment at: clang/include/clang/AST/DeclCXX.h:3801 +/// error. +class UnresolvedUsingIfExistsDecl final : public NamedDecl { + UnresolvedUsingIfExistsDecl(DeclContext *DC, SourceLocation Loc, erik.pilkington wrote: > aaron.ba

[PATCH] D90188: Add support for attribute 'using_if_exists'

aaron.ballman added inline comments. Comment at: clang/include/clang/AST/DeclCXX.h:3801 +/// error. +class UnresolvedUsingIfExistsDecl final : public NamedDecl { + UnresolvedUsingIfExistsDecl(DeclContext *DC, SourceLocation Loc, erik.pilkington wrote: > aaron.ba

[PATCH] D90221: Include more attribute details when dumping AST in JSON

aaron.ballman added a comment. In D90221#2358469 , @aronsky wrote: > In D90221#2357060 , @aaron.ballman > wrote: > >> In D90221#2356110 , @aronsky wrote: >> >>> In D90221#

[PATCH] D90188: Add support for attribute 'using_if_exists'

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:5273 + namespace empty_namespace {}; + using empty_namespace::does_not_exist __attribute__((using_if_exists)); // no error! + Mordante wrote: > erik.pilkington wrote: > > aa

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

aaron.ballman added a comment. In D90180#2374839 , @nickdesaulniers wrote: > In D90180#2357247 , @aaron.ballman > wrote: > >> When you pass `-fix` to clang-tidy, it will apply fix-its from the compiler >> as well

[PATCH] D90275: [clang][IR] Add support for leaf attribute

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; gulfem wrote: > aaron.ballman wrote: > > gulfem wrote: > >

[PATCH] D90832: [clang-tidy] Extend IdentifierNamingCheck per file config

aaron.ballman added a comment. Is there a way to test this functionality out? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90832/new/ https://reviews.llvm.org/D90832 ___ cfe-commits mailing list cfe-com

[PATCH] D90832: [clang-tidy] Extend IdentifierNamingCheck per file config

aaron.ballman added a comment. In D90832#2376009 , @njames93 wrote: > In D90832#2375934 , @aaron.ballman > wrote: > >> Is there a way to test this functionality out? > > There's already tests for loading the indivi

[PATCH] D90832: [clang-tidy] Extend IdentifierNamingCheck per file config

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from some tiny nits! Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:172 SmallString<64> StyleString; + auto Styl

[PATCH] D90767: Add new matchers for dependent names in templates

aaron.ballman added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2838 +/// Matches template-dependent, but known, member names +/// Missing full stop at the end of the comment. Comment at: clang/include/clang/AST

[PATCH] D90275: [clang][IR] Add support for leaf attribute

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; gulfem wrote: > aaron.ballman wrote: > > gulfem wrote: > >

[PATCH] D90763: Traverse-ignore explicit template instantiations

aaron.ballman added inline comments. Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:503 TraversalKind Traversal, BindKind Bind) { +auto ScopedTraversal = TraversingTemplateInstantiationNotSpelledInSource; + Please spell this t

[PATCH] D90891: [clang] ns_error_domain attribute also supports CFString typed variables

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90891/new/ https://reviews.llvm.org/D90891 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/

[PATCH] D90763: Traverse-ignore explicit template instantiations

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a tiny commenting request. Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:506 +if (const auto *CTSD = Node.get()) { + auto SK = C

[PATCH] D90767: Add new matchers for dependent names in templates

aaron.ballman added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2859 +AST_MATCHER_P(CXXDependentScopeMemberExpr, hasMemberName, std::string, N) { + return Node.getMember().getAsString() == N; +} steveire wrote: > aaron.ballman wrot

[PATCH] D90892: [AIX][FE] Support constructor/destructor attribute

aaron.ballman added inline comments. Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:276 // extern "C" int atexit(void (*f)(void)); - assert(cast(dtorStub)->getFunctionType() == - llvm::FunctionType::get(CGM.VoidTy, false) && + llvm::PointerType *Expected = + ll

[PATCH] D90767: Add new matchers for dependent names in templates

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2859 +AST_MATCHER_P(CXXDependentScopeMemberExpr, hasMemberName, std::string, N) { + return Node

[PATCH] D90221: Include more attribute details when dumping AST in JSON

aaron.ballman added a comment. In D90221#2374344 , @aronsky wrote: >> It seems surprising to me that you'd have to add those declarations; I think >> that's a layering violation. There's something somewhat strange going on >> here that I think is related

[PATCH] D90180: [clang-tidy] find/fix unneeded semicolon after switch

aaron.ballman added a comment. In D90180#2379803 , @nickdesaulniers wrote: > In D90180#2375878 , @aaron.ballman > wrote: > >> In D90180#2374839 , >> @nickdesaulniers wrote

[PATCH] D90992: [clang-tidy] Use vfs::FileSystem when getting config

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a nit with the test. Comment at: clang-tools-extra/unittests/clang-tidy/OptionsProviderTest.cpp:68 +} // namespace clang \ No newline at end of

[PATCH] D90983: Change algorithms to return iterators

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90983/new/ https://reviews.llvm.org/D90983 __

[PATCH] D90984: Update matchers to be traverse-aware

aaron.ballman added a comment. I think this change should come with a warning in the release notes because it silently changes the behavior of existing matchers in a way that may be surprising to users. I think the behavior is correct in that these constructs are all implicit nodes that aren't

[PATCH] D91035: [NFC, Refactor] Convert FunctionDefinitionKind from DeclSpech.h to a scoped enum

aaron.ballman added inline comments. Comment at: clang/include/clang/Sema/DeclSpec.h:1837 /// Actually a FunctionDefinitionKind. - unsigned FunctionDefinition : 2; + FunctionDefinitionKind FunctionDefinition : 2; I think we need to keep this as `unsigned`

[PATCH] D91011: [NFC, Refactor] Rename the (scoped) enum DeclaratorContext's enumerators to avoid redundancy

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91011/new/ https://reviews.llvm.org/D91011 __

[PATCH] D72218: [clang-tidy] new altera kernel name restriction check

aaron.ballman added a comment. In D72218#2381149 , @ffrankies wrote: > Moved test files `KERNEL.cl`, `VHDL.cl` and `vERILOG.cl` to the `uppercase` > subdirectory to prevent filename clashes in some environments. > > This is in response to the buildbot fai

[PATCH] D91015: [clang-tidy] Extend bugprone-string-constructor-check to std::string_view.

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp:64 void StringConstructorCheck::registerMatchers(MatchFinder *Finder) { + const auto hasStringCtorName = + hasAnyNameStdString(removeNamespaces(StringNames));

[PATCH] D90221: Include more attribute details when dumping AST in JSON

aaron.ballman added a subscriber: Tyker. aaron.ballman added a comment. In D90221#2381325 , @aronsky wrote: > In D90221#2379793 , @aaron.ballman > wrote: > >> I think the issue is that ASTNodeTraverser is visiting

[PATCH] D91047: Add a call super attribute plugin example

aaron.ballman added inline comments. Comment at: clang/docs/ClangPlugins.rst:117 +Defining CallSuperAttr +=== + The number of underlines here looks off -- can you verify it's correct? Comment at: clang/docs/ClangPlugins.rst:119

[PATCH] D91035: [NFC, Refactor] Convert FunctionDefinitionKind from DeclSpech.h to a scoped enum

aaron.ballman added inline comments. Comment at: clang/include/clang/Sema/DeclSpec.h:1837 /// Actually a FunctionDefinitionKind. - unsigned FunctionDefinition : 2; + FunctionDefinitionKind FunctionDefinition : 2; faisalv wrote: > aaron.ballman wrote: > > I

[PATCH] D91144: Add utility for testing if we're matching nodes AsIs

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, thanks! Comment at: clang/lib/ASTMatchers/ASTMatchersInternal.cpp:195 +bool ASTMatchFinder::isTraversalAsIs() const { + return getASTContext().getParentM

[PATCH] D90984: Update matchers to be traverse-aware

aaron.ballman added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3115 + + if (Finder->getASTContext().getParentMapContext().getTraversalKind() != + TK_AsIs && steveire wrote: > aaron.ballman wrote: > > Given how often this

[PATCH] D91033: [clang-tidy][NFC] Tweak GlobList to iterate backwards

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM with a request to add a comment. Comment at: clang-tools-extra/clang-tidy/GlobList.cpp:56 bool GlobList::contains(StringRef S) { - bool Contains = false;

[PATCH] D91033: [clang-tidy][NFC] Tweak GlobList to iterate backwards

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/GlobList.cpp:56 bool GlobList::contains(StringRef S) { - bool Contains = false; - for (const GlobListItem &Item : Items) { + for (const GlobListItem &Item : llvm::reverse(Items)) { if (Item.Reg

[PATCH] D91037: [clang-tidy] Fix crash in bugprone-redundant-branch-condition on ExprWithCleanups

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp:85 // remove the inner `if`. - const auto *BinOpCond = dyn_cast(InnerIf->getCond()); + const auto *BinOpCond = dyn_cast_or_null(InnerIf->getCond()); + i

[PATCH] D90851: [clang-tidy] Extending bugprone-signal-handler with POSIX functions.

aaron.ballman added a comment. One thing that's not clear to me in the patch is what the behavior should be when the user specifies that they want the POSIX set but they're compiling for a non-POSIX system like Windows. Do you have thoughts on that situation? Comment at: clan

[PATCH] D90275: [clang][IR] Add support for leaf attribute

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; gulfem wrote: > aaron.ballman wrote: > > gulfem wrote: > >

[PATCH] D77493: [clang-tidy] Add do-not-refer-atomic-twice check

aaron.ballman added a comment. In D77493#1962465 , @njames93 wrote: > In my mind this check is definitely in the realm of the static analyser, > clang-tidy just isn't designed for this. +1, I think this probably should be handled in the static analyzer i

[PATCH] D90851: [clang-tidy] Extending bugprone-signal-handler with POSIX functions.

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h:25 public: + enum class AsyncSafeFunctionSetType { Minimal, POSIX }; + balazske wrote: > aaron.ballman wrote: > > I don't think this needs to be public?

[PATCH] D90892: [AIX][FE] Support constructor/destructor attribute

aaron.ballman added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.h:1482 + void AddGlobalDtor(llvm::Function *Dtor, int Priority = 65535, + bool IsDtorAttrFunc = false); Xiangling_L wrote: > aaron.ballman wrote: > > There's a

[PATCH] D91037: [clang-tidy] Fix crash in bugprone-redundant-branch-condition on ExprWithCleanups

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp:85 // remove the inner `if`. - const auto *BinOpCond = dyn_cast(InnerIf->getCond()); + const auto *BinOpCond = dyn_cast_or_null(InnerIf->getCond()); + i

[PATCH] D90892: [AIX][FE] Support constructor/destructor attribute

aaron.ballman added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.h:1482 + void AddGlobalDtor(llvm::Function *Dtor, int Priority = 65535, + bool IsDtorAttrFunc = false); Xiangling_L wrote: > aaron.ballman wrote: > > Xiangling

[PATCH] D91144: Add utility for testing if we're matching nodes AsIs

aaron.ballman added inline comments. Comment at: clang/lib/ASTMatchers/ASTMatchersInternal.cpp:195 +bool ASTMatchFinder::isTraversalAsIs() const { + return getASTContext().getParentMapContext().getTraversalKind() == TK_AsIs; +} steveire wrote: > aaron.ballman wr

[PATCH] D91037: [clang-tidy] Fix crash in bugprone-redundant-branch-condition on ExprWithCleanups

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp:85 // remove the inner `if`. - const auto *BinOpCond = dyn_cast(InnerIf->getCond()); + const auto *BinOpCond = dyn_cast_or_null(InnerIf->getCond()); + i

[PATCH] D75229: [clang-tidy] Add signal-in-multithreaded-program check

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SignalInMultithreadedProgramCheck.cpp:41 + hasDeclaration(functionDecl(hasAnyListedName(ThreadList, +hasDescendant(varDecl(hasType(recordDecl(hasName("std::

[PATCH] D91037: [clang-tidy] Fix crash in bugprone-redundant-branch-condition on ExprWithCleanups

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp:145 + const auto *LeftDRE = dyn_cast(CondOp->getLHS()->IgnoreParenImpCasts()); The old code used to assert that `CondOp` was a

<    36   37   38   39   40   41   42   43   44   45   >