[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:38 + whileStmt(anyOf( + has(declStmt(containsDeclaration( + 0, varDecl( aaron.ballman wrote: > This seems like a fair amo

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h:21 -typedef llvm::SmallSet HeaderFileExtensionsSet; +using HeaderFileExtensionsSet = ::llvm::SmallSet; hintonda wrote: > aaron.ballman wrote: > > C

[PATCH] D60101: [Sema] Fix a use-after-deallocate of a ParsedAttr

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from some minor nits. Thanks for this! Comment at: clang/include/clang/Sema/ParsedAttr.h:896 + void takeOneFrom(ParsedAttributes &attrs, ParsedAtt

[PATCH] D60123: [AST] Forbid copy/move of statements/types.

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/D60123/new/ https://reviews.llvm.org/D60123 ___

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 🔍

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp:112 + << Message->getMethodDecl() + << FixItHint::CreateReplacement(Message->getSourceRange(), + StringRef("[super init]")); --

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:18 + +AST_MATCHER(Expr, isMacroID) { return Node.getExprLoc().isMacroID(); } +} // namespace ast_matchers hintonda wrote: > @aaron.ballman: This ma

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:145 + +diag(MatchedDecl->getBeginLoc(), "use dyn_cast_or_null") +<< FixItHint::CreateReplacement(SourceRange(MatchedDecl->getBeginLoc(), ---

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

aaron.ballman added inline comments. Comment at: include/clang/Basic/Attr.td:293 bit Negated = negated; + string CustomCode = customCode; } I think the type here should be `code` instead of `string` since the user is passing in code snippets, no?

[PATCH] D60151: [clang-tidy] Add using SmallSet to LLVM.h to fix bug in typedef in llvm checkers.

aaron.ballman requested changes to this revision. aaron.ballman added inline comments. This revision now requires changes to proceed. Comment at: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h:21 -typedef llvm::SmallSet HeaderFileExtensionsSet; +using HeaderFile

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

aaron.ballman added inline comments. Comment at: include/clang/Basic/AttrDocs.td:1116 +def ObjCClassStubDocs : Documentation { +let Category = DocCatFunction; +let Content = [{ aaron.ballman wrote: > slavapestov wrote: > > aaron.ballman wrote: > > > This

[PATCH] D60151: [clang-tidy] Add using SmallSet to LLVM.h to fix bug in typedef in llvm checkers.

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h:21 -typedef llvm::SmallSet HeaderFileExtensionsSet; +using HeaderFileExtensionsSet = SmallSet; hintonda wrote: > aaron.ballman wrote: > > I do not

[PATCH] D60151: [clang-tidy] Add using SmallSet to LLVM.h to fix bug in typedef in llvm checkers.

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h:21 -typedef llvm::SmallSet HeaderFileExtensionsSet; +using HeaderFileExtensionsSet = SmallSet; alexfh wrote: > hintonda wrote: > > aaron.ballman wr

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:145 + +diag(MatchedDecl->getBeginLoc(), "use dyn_cast_or_null") +<< FixItHint::CreateReplacement(SourceRange(MatchedDecl->getBeginLoc(), ---

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:18 + +AST_MATCHER(Expr, isMacroID) { return Node.getExprLoc().isMacroID(); } +} // namespace ast_matchers hintonda wrote: > aaron.ballman wrote: > >

[PATCH] D60455: [SYCL] Add support for SYCL device attributes

aaron.ballman added a reviewer: aaron.ballman. aaron.ballman added a comment. These attributes are being parsed and silently ignored -- are there semantics expected for the attributes? Comment at: clang/include/clang/Basic/Attr.td:1000 +def SYCLDevice : InheritableAttr { + le

[PATCH] D60151: [clang-tidy] Rename llvm checkers to llvm-project

aaron.ballman added a comment. In D60151#1454802 , @hintonda wrote: > In D60151#1454741 , @alexfh wrote: > > > In D60151#1454105 , @hintonda > > wrote: > > > > > - Rename ll

[PATCH] D60139: [clang-tidy] Add bugprone-placement-new-target-type-mismatch check

aaron.ballman added a comment. In D60139#1461269 , @DennisL wrote: > In D60139#1460233 , @JonasToth wrote: > > > Hey Dennis, > > > > my 2cents on the check. I think it is very good to have! Did you check > > coding

[PATCH] D60542: Add support for attributes on @implementations in Objective-C

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM aside from a question. Comment at: clang/test/FixIt/fixit-pragma-attribute.cpp:19-20 #pragma clang attribute push (__attribute__((annotate("subRuleContradictions"))), apply_to = any(variable, variable(

[PATCH] D60544: Support objc_nonlazy_class attribute on Objective-C implementations

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, modulo the `__has_feature` question. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60544/new/ https://reviews.llvm.org/D60544 __

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/llvm/PreferIsaOrDynCastInConditionalsCheck.cpp:125 +diag(MatchedDecl->getBeginLoc(), + "isa_and_nonnull<> is cleaner and more efficient") +<< FixItHint::CreateReplacement(SourceRang

[PATCH] D60455: [SYCL] Add support for SYCL device attributes

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1000 +def SYCLDevice : InheritableAttr { + let Spellings = [GNU<"sycl_device">]; + let Subjects = SubjectList<[Function, Var]>; keryell wrote: > Fznamznon wrote: > > aaron.ballma

[PATCH] D60543: [clang] Add matcher for subclasses of Objective-C interfaces 🔍

aaron.ballman added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:1479 +/// \endcode +AST_MATCHER_P(ObjCInterfaceDecl, isSubclassOfInterface, + internal::Matcher, stephanemoore wrote: > I am still uncertain about the nami

[PATCH] D55409: [clang-tidy] check for using declarations not in an anonymous namespace when there exists one

aaron.ballman added inline comments. Comment at: clang-tidy/abseil/AnonymousEnclosedAliasesCheck.cpp:41 +// to the vector containing all candidate using declarations. +if (AnonymousNamespaceDecl) { + diag(MatchedUsingDecl->getLocation(), Nay

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

aaron.ballman added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst:10 + +This check corresponds to the CERT C++ Coding Standard rule +`OOP54-CPP. Gracefully handle self-copy assignment Eugene.Zelenko

[PATCH] D55411: [clang-tidy] check for flagging using declarations not in the inner most namespace

aaron.ballman added inline comments. Comment at: clang-tidy/abseil/SafelyScopedCheck.cpp:37 + diag(MatchedDecl->getLocation(), + "using declaration %0 not declared in the innermost namespace.") + << MatchedDecl; You should remove the full stop at the

[PATCH] D60542: Add support for attributes on @implementations in Objective-C

aaron.ballman added inline comments. Comment at: clang/test/FixIt/fixit-pragma-attribute.cpp:19-20 #pragma clang attribute push (__attribute__((annotate("subRuleContradictions"))), apply_to = any(variable, variable(is_parameter), function(is_member), variable(is_global))) -//

[PATCH] D60455: [SYCL] Add support for SYCL device attributes

aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. In D60455#1464324 , @Fznamznon wrote: > Applied comments from @aaron.ballman and @keryell > > - Introduced a C++11 and C2x style

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a nit and the ultimate name for the `isa_and_nonnull<>` API. Comment at: clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp:27-28 CheckFa

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_checker'

aaron.ballman added a comment. No opposition from me regarding `llvm_check`, though I think `llvm_tidy` might also be a reasonable option. I'm a bit less keen on `llvm_checker` as the name, but not strongly opposed. Comment at: clang-tools-extra/clang-tidy/add_new_check.py:38

[PATCH] D60543: [clang] Add matcher for subclasses of Objective-C interfaces 🔍

aaron.ballman added reviewers: jordan_rose, rjmccall. aaron.ballman added a comment. Adding some ObjC experts in case they'd like to weigh in on the name `isDerivedFrom` in this context. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:1479 +/// \endcode +AST_MATCHER_

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp:130-131 "bugprone-throw-keyword-missing"); +CheckFactories.registerCheck( +"bugprone-too-small-loop-variable"); CheckFactories.registerCheck( --

[PATCH] D60749: [Test] Remove obsolete test.

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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

aaron.ballman added a comment. In D59802#1467430 , @hintonda wrote: > In D59802#1467363 , @aaron.ballman > wrote: > > > LGTM aside from a nit and the ultimate name for the `isa_and_nonnull<>` API. > > > Thanks. I'

[PATCH] D60455: [SYCL] Add support for SYCL device attributes

aaron.ballman added a comment. In D60455#1468714 , @bader wrote: > In D60455#1468386 , @Fznamznon wrote: > > > > Ok, my question is whether you are planning to duplicate the same logic > > > as for OpenCL kernel wh

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 🔍

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! You can either land this now and refactor after the AST matcher lands, or you can wait until the AST matcher lands and land this patch after -- your call. Repository: rG

[PATCH] D60543: [clang] Add matcher for subclasses of Objective-C interfaces 🔍

aaron.ballman added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:1479 +/// \endcode +AST_MATCHER_P(ObjCInterfaceDecl, isSubclassOfInterface, + internal::Matcher, jordan_rose wrote: > aaron.ballman wrote: > > stephanemoor

[PATCH] D60543: [clang] Add matcher for subclasses of Objective-C interfaces 🔍

aaron.ballman added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:1479 +/// \endcode +AST_MATCHER_P(ObjCInterfaceDecl, isSubclassOfInterface, + internal::Matcher, jordan_rose wrote: > aaron.ballman wrote: > > jordan_rose

[PATCH] D60543: [clang] Add matcher for subclasses of Objective-C interfaces 🔍

aaron.ballman added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:1479 +/// \endcode +AST_MATCHER_P(ObjCInterfaceDecl, isSubclassOfInterface, + internal::Matcher, jordan_rose wrote: > aaron.ballman wrote: > > jordan_rose

[PATCH] D60872: Add new warning knob for unknown attribute namespaces

aaron.ballman marked 7 inline comments as done. aaron.ballman added inline comments. Comment at: lib/Sema/SemaDeclAttr.cpp:8623 + } else if (A.hasScope()) { +#define ATTR(A) +#define ATTR_NAMESPACE(A) .Case(#A, false) dblaikie wrote: > dblaikie wrote: > > Not su

[PATCH] D60872: Add new warning knob for unknown attribute namespaces

aaron.ballman updated this revision to Diff 195790. aaron.ballman marked 3 inline comments as done. aaron.ballman added a comment. Updated based on review feedback. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60872/new/ https://reviews.llvm.org/D60872 Files: include/clang/Basic/Dia

[PATCH] D60910: [WIP] Dumping the AST to JSON

aaron.ballman created this revision. aaron.ballman added reviewers: rsmith, riccibruno, steveire, dblaikie. Herald added a subscriber: mgorny. This is a work in progress patch that adds the ability to specify an AST dump format on the command line. By default, we continue to dump the AST with its

[PATCH] D60872: Add new warning knob for unknown attribute namespaces

aaron.ballman marked 2 inline comments as done. aaron.ballman added a comment. In D60872#1473205 , @rsmith wrote: > Hmm. So there are a few different situations where we might meet an unknown > attribute (I'm sure I missed some): > > 1. The attribute had

[PATCH] D60910: [WIP] Dumping the AST to JSON

aaron.ballman marked 2 inline comments as done. aaron.ballman added a comment. In D60910#1473441 , @riccibruno wrote: > A few comments/questions: > > 1. How stable is the format going to be, and how much state is going to be > exposed ? I don't expect i

[PATCH] D60956: [Sema] Fix the lookup for a declaration conflicting with an enumerator (bogus use of LookupResult::getAsSingle)

aaron.ballman added inline comments. Comment at: lib/Sema/SemaDecl.cpp:16607 + // Check for other kinds of shadowing not already handled. + if (PrevDecl && isa(PrevDecl->getUnderlyingDecl()) && + !TheEnumDecl->isScoped()) Is the change to `PrevDecl->getUnd

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

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/RedundantAccessSpecifiersCheck.cpp:35-36 + for (DeclContext::specific_decl_iterator + AS(MatchedDecl->decls_begin()), + ASEnd(MatchedDecl->decls_end()); + AS != ASEnd

[PATCH] D60956: [Sema] Fix the lookup for a declaration conflicting with an enumerator (bogus use of LookupResult::getAsSingle)

aaron.ballman added inline comments. Comment at: lib/Sema/SemaDecl.cpp:16607 + // Check for other kinds of shadowing not already handled. + if (PrevDecl && isa(PrevDecl->getUnderlyingDecl()) && + !TheEnumDecl->isScoped()) riccibruno wrote: > aaron.ballman

[PATCH] D55411: [clang-tidy] check for flagging using declarations not in the inner most namespace

aaron.ballman added inline comments. Comment at: clang-tidy/abseil/SafelyScopedCheck.cpp:22 + // The target using declaration is either: + // 1. not in any namespace declaration, or + // 2. in some namespace declaration but not in the innermost layer aaron.bal

[PATCH] D59963: [clang-tidy] Add a module for the Linux kernel.

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp:39 + if (MatchedCallExpr && + MatchedCallExpr->hasUnusedResultAttr(*Result.Context)) { +diag(MatchedCallExpr->getExprLoc(), "Unused result from error functio

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

aaron.ballman marked an inline comment as done. aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp:34-36 + const auto HasNoSelfCheck = cxxMethodDecl(unless(hasDescendant( + binaryOperator(anyOf(hasOperato

[PATCH] D59802: [clang-tidy] Add new checker: llvm-prefer-isa-or-dyn-cast-in-conditionals

aaron.ballman added a comment. In D59802#1474300 , @hintonda wrote: > @aaron.ballman, I just ran it over llvm/lib, including all in-tree headers, > and it seems to work fine. However, it did miss this one: > > - if (V && isa(V) && (EntInst = cast(V)) &&

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp:54 + recordType(hasDeclaration(classTemplateSpecializationDecl( + hasAnyName("std::shared_ptr", "std::unique_ptr", "std::weak_ptr", +

[PATCH] D59963: [clang-tidy] Add a module for the Linux kernel.

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp:24 + "ERR_CAST", "PTR_ERR_OR_ZERO")); + auto NonCheckingStmts = stmt(anyOf(compoundStmt(), labelStmt())); + Finder->addMatcher(

[PATCH] D61097: [Sema] Emit warning for visibility attribute on internal-linkage declaration

aaron.ballman added a reviewer: aaron.ballman. aaron.ballman added inline comments. Comment at: lib/Sema/SemaDeclAttr.cpp:2619 + // Visibility attributes have no effect on symbols with internal linkage. + if (auto ND = dyn_cast(D)) { +if (!ND->isExternallyVisible()) { -

[PATCH] D60910: [WIP] Dumping the AST to JSON

aaron.ballman updated this revision to Diff 196645. aaron.ballman added a comment. Herald added a subscriber: jdoerfert. Rebased to master + small amount of additional functionality. Since it seems this is non-controversial, I will probably commit after next week unless reviewers bring up concer

[PATCH] D60910: [WIP] Dumping the AST to JSON

aaron.ballman marked 3 inline comments as done. aaron.ballman added inline comments. Comment at: include/clang/AST/JSONNodeDumper.h:73 + Indentation.clear(); + OS << "\n" << Indentation << "}\n"; + TopLevel = true; hintonda wrote: > Just curious, s

[PATCH] D61370: Add a C2x mode and allow attributes in it

aaron.ballman created this revision. aaron.ballman added reviewers: rsmith, dblaikie. WG14 has a working draft for C2x (WG14 N2346) and we've begun voting new features into it, so I think it's time for use to expose a C2x flag and use it. The new features we adopted this week that Clang already

[PATCH] D61097: [Sema] Emit warning for visibility attribute on internal-linkage declaration

aaron.ballman added inline comments. Comment at: lib/Sema/SemaDeclAttr.cpp:2621 +if (!ND->isExternallyVisible()) { + S.Diag(AL.getRange().getBegin(), diag::warn_attribute_ignored) << AL; + return; scott.linder wrote: > aaron.ballman wrote: > > I thi

[PATCH] D61209: [clang-tidy] Fix readability-redundant-smartptr-get for MSVC STL

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a commenting nit. Comment at: clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.cpp:46 + // Make sure we are not missing th

[PATCH] D61260: [clang-tidy] Extend bugprone-sizeof-expression to check sizeof(pointers to structures)

aaron.ballman added a comment. Aside from the testing request, this LGTM. Comment at: test/clang-tidy/bugprone-sizeof-expression.cpp:231 + sum += sizeof(MyStruct*); + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate + sum +

[PATCH] D61239: [libclang] Allow field offset lookups in types with incomplete arrays.

aaron.ballman added inline comments. Comment at: test/Index/print-type-size.c:22 + +// RUN: c-index-test -test-print-type-size %s | FileCheck %s +// CHECK: FieldDecl=size:2:9 (Definition) [type=int] [typekind=Int] [sizeof=4] [alignof=4] [offsetof=0] This should

[PATCH] D59520: [WebAssembly] Address review comments on r352930

aaron.ballman added inline comments. Comment at: include/clang/Basic/DiagnosticIDs.h:39 DIAG_SIZE_CROSSTU = 100, - DIAG_SIZE_SEMA = 3500, + DIAG_SIZE_SEMA = 3504, DIAG_SIZE_ANALYSIS = 100, I think this should

[PATCH] D61288: [Diagnostics] Implemented support for -Wswitch-default

aaron.ballman added a comment. Do we have evidence of false positive vs true positive rates for this catching actual problems in real world code? I realize that this is for GCC compatibility, but because it's default-off, I'm wondering where the utilities lies. Comment at: i

[PATCH] D61239: [libclang] Allow field offset lookups in types with incomplete arrays.

aaron.ballman added a comment. In D61239#1485994 , @JornVernee wrote: > - holding of on adding helper method until hearing back. My rationale for wanting a helper method is because we already have two places were incompleteness is important but has spec

[PATCH] D61288: [Diagnostics] Implemented support for -Wswitch-default

aaron.ballman added a comment. In D61288#1486006 , @xbolva00 wrote: > Some coding guidelines may require switch to have always default label. Even > if devs know that default is not reachable, they can add default: abort(); or > assert to increase safety

[PATCH] D61288: [Diagnostics] Implemented support for -Wswitch-default

aaron.ballman added a comment. In D61288#1486034 , @xbolva00 wrote: > I am not familiar with clang-tidy’s codebase and I personally prefer good > compiler warnings than dependency on another tool (clang-tidy). I mean the > cases when diagnostic check is

[PATCH] D61239: [libclang] Allow field offset lookups in types with incomplete arrays.

aaron.ballman added inline comments. Comment at: test/Index/print-type-size.c:6 + // CHECK: FieldDecl=size:4:7 (Definition) [type=int] [typekind=Int] [sizeof=4] [alignof=4] [offsetof=0] + void* data[]; + // CHECK: FieldDecl=data:6:9 (Definition) [type=void *[]] [typekind=Inc

[PATCH] D61097: [Sema] Emit warning for visibility attribute on internal-linkage declaration

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

[PATCH] D61239: [libclang] Allow field offset lookups in types with incomplete arrays.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, thank you! That's strange that clang-format would remove the newline from the end of the file. I don't get that behavior when I try it, so it makes me wonder what's differe

[PATCH] D61239: [libclang] Allow field offset lookups in types with incomplete arrays.

aaron.ballman added a comment. In D61239#1486692 , @JornVernee wrote: > In D61239#1486634 , @aaron.ballman > wrote: > > > LGTM, thank you! That's strange that clang-format would remove the newline > > from the end

[PATCH] D61444: Do not warn on switches over enums that do not use [[maybe_unused]] enumerators

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/D61444/new/ https://reviews.llvm.org/D61444 ___

[PATCH] D61422: [clang-tidy] Extend bugprone-sizeof-expression check to detect sizeof misuse in pointer arithmetic

aaron.ballman added a comment. Out of curiosity, have you run this over any large code bases to see what the false positive and true positive rate is? Comment at: clang-tidy/bugprone/SizeofExpressionCheck.cpp:217 + // Detect sizeof in pointer aritmetic like: N * sizeof(S) ==

[PATCH] D61480: Added an AST matcher for declarations that are in the `std` namespace

aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. You should also update Registry.cpp to add this to clang-query and you should also regenerate the documentation by running clang\docs\tools\dump_ast_matchers.py. ===

[PATCH] D61480: Added an AST matcher for declarations that are in the `std` namespace

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/D61480/new/ https://reviews.llvm.org/D61480

[PATCH] D61422: [clang-tidy] Extend bugprone-sizeof-expression check to detect sizeof misuse in pointer arithmetic

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D61422#1489159 , @baloghadamsoftware wrote: > In D61422#1488081 , @aaron.ballman > wrote: >

[PATCH] D55125: [clang-tidy] Fix a false positive in misc-redundant-expression check

aaron.ballman added inline comments. Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:135-137 +const UnaryExprOrTypeTraitExpr *LeftUnaryExpr = +cast(Left); +const UnaryExprOrTypeTraitExpr *RightUnaryExpr = `const auto *` since you already

[PATCH] D61552: [clang] Adapt ASTMatcher to explicit(bool) specifier

aaron.ballman requested changes to this revision. aaron.ballman added a reviewer: aaron.ballman. aaron.ballman added a comment. You should also regenerate the documentation by running clang/docs/tools/dump_ast_matchers.py. Comment at: clang/include/clang/ASTMatchers/ASTMatcher

[PATCH] D61508: [clang-tidy] misc-header-guard : a simple version of llvm-header-guard

aaron.ballman added a comment. In D61508#1491570 , @JonasToth wrote: > Hi trixirt and thanks for the patch! > > I would rather like to generalize the llvm check to allow different styles > and then alias the general version with different configurations.

[PATCH] D61260: [clang-tidy] Extend bugprone-sizeof-expression to check sizeof(pointers to structures)

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/D61260/new/ https://reviews.llvm.org/D61260 ___ cfe-commits mailing lis

[PATCH] D33841: [clang-tidy] redundant keyword check

aaron.ballman added inline comments. Comment at: test/clang-tidy/readability-redundant-extern.cpp:37 + +void another_file_scope(int _extern); koldaniel wrote: > aaron.ballman wrote: > > More tests that I figured out: > > ``` > > namespace { > > extern void f(); /

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

aaron.ballman added a comment. In D60507#1491095 , @ztamas wrote: > Ping. > Is it good to go or is there anything else I need to include in this patch? Sorry for the delay, I was gone for WG14 meetings last week, which made reviewing more involved patc

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

aaron.ballman added inline comments. Comment at: clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:85-87 +if (TL.getQualifierLoc()) + if (!TraverseNestedNameSpecifierLoc(TL.getQualifierLoc())) +return false; You can combine these. ===

[PATCH] D33841: [clang-tidy] redundant keyword check

aaron.ballman added inline comments. Comment at: test/clang-tidy/readability-redundant-extern.cpp:37 + +void another_file_scope(int _extern); koldaniel wrote: > aaron.ballman wrote: > > koldaniel wrote: > > > aaron.ballman wrote: > > > > More tests that I figured

[PATCH] D61350: [clang-tidy] New check calling out uses of +new in Objective-C code

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/google/AvoidNSObjectNewCheck.cpp:73 + // of -init. + const StringRef Receiver = + getReceiverString(Expr->getReceiverRange(), SM, LangOpts); We don't typically make local value t

[PATCH] D60956: [Sema] Fix the lookup for a declaration conflicting with an enumerator

aaron.ballman added a comment. I found a few nits, but generally think this LG. However, I think @rsmith should sign off on it just in case I've misinterpreted something along the way. Comment at: lib/Sema/SemaDecl.cpp:16537 +// instead would cause us to miss using-decla

[PATCH] D61239: [libclang] Allow field offset lookups in types with incomplete arrays.

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

[PATCH] D60910: [WIP] Dumping the AST to JSON

aaron.ballman added inline comments. Comment at: include/clang/AST/DeclBase.h:1139 + void dump(raw_ostream &Out, bool Deserialize = false, +ASTDumpOutputFormat OutputFormat = ADOF_Default) const; steveire wrote: > I think we've talked about this be

[PATCH] D60910: [WIP] Dumping the AST to JSON

aaron.ballman updated this revision to Diff 198477. aaron.ballman marked 14 inline comments as done. aaron.ballman added a comment. Updating based on review feedback. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60910/new/ https://reviews.llvm.org/D60910 Files: include/clang/AST/AST

[PATCH] D61642: [clang-tidy] Do not show incorrect fix in modernize-make-unique

aaron.ballman marked an inline comment as done. aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp:302 if (const auto *ImplicitCE = dyn_cast(Arg->IgnoreImplicit())) { if (ImplicitCE->isStdIn

[PATCH] D61566: Fix for bug 41747: AST Printer doesn't print nested name specifier for out of scope record definitions

aaron.ballman added inline comments. Comment at: clang/lib/AST/DeclPrinter.cpp:958 + +if (auto *Q = D->getQualifier()) + Q->print(Out, Policy); Don't use `auto` here as the type is not explicitly spelled out in the initialization. If possible, `const`-q

[PATCH] D61370: Add a C2x mode and allow attributes in it

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

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

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/D55793/new/ https://reviews.llvm.org/D55793 ___ cfe-commits mailing lis

[PATCH] D61350: [clang-tidy] New check calling out uses of +new in Objective-C code

aaron.ballman accepted this revision. aaron.ballman marked an inline comment as done. aaron.ballman added a comment. LGTM modulo the comments from @gribozavr. Comment at: clang-tools-extra/clang-tidy/google/AvoidNSObjectNewCheck.cpp:76-77 + // Some classes should use standard

[PATCH] D61642: [clang-tidy] Do not show incorrect fix in modernize-make-unique

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/D61642/new/ https://reviews.llvm.org/D61642

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

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Aside from a formatting issue, this LGTM, thank you! Comment at: clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:93 +DeclarationName Name = S->getNameIn

[PATCH] D60629: [clang-tidy] Change the namespace for llvm checkers from 'llvm' to 'llvm_check'

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. This LGTM, but you may want to wait a day or so to see if @alexfh has any remaining concerns. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D52967: Extend shelf-life by 70 years

aaron.ballman closed this revision. aaron.ballman added a comment. In D52967#1494969 , @bmwiedemann wrote: > Can someone please merge this change? I commit the change in r360254, thank you for the patch! Repository: rC Clang CHANGES SINCE LAST ACTIO

[PATCH] D61566: Fix for bug 41747: AST Printer doesn't print nested name specifier for out of scope record definitions

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

[PATCH] D24892: [clang-tidy] Add option "LiteralInitializers" to cppcoreguidelines-pro-type-member-init

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp:361 + case BuiltinType::Float: +return " = 0.0F"; + case BuiltinType::Double: We may not have

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

aaron.ballman added a comment. In D56160#1495847 , @bernhardmgruber wrote: > @aaron.ballman and @JonasToth: Thank you for the patience and all the > feedback! It means a great deal to me to have a patch accepted here! You're very welcome! Do you need o

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

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/D60507/new/ https://reviews.llvm.org/D60507

<    6   7   8   9   10   11   12   13   14   15   >