[PATCH] D61716: [libclang] Expose AtomicType

aaron.ballman added a comment. I'm sorry about the delay in review, this did fall off my radar. Thank you for bringing it to my attention! Looks mostly good to me, just a few small nits. Comment at: clang/include/clang-c/Index.h:3953 +/** + * Gets the type contained by this a

[PATCH] D77503: [ASTMatchers] Fixed CastKind being parsed incorrectly for dynamic matchers

aaron.ballman added a comment. Can you also add some unit tests to the dynamic matcher tests? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77503/new/ https://reviews.llvm.org/D77503 ___ cfe-commits ma

[PATCH] D77791: [ASTMatchers] Add support for dynamic matching of ofKind narrowing matcher

aaron.ballman added a comment. Thank you for this! It basically LG, but can you also add a dynamic matcher unit test for this functionality? Comment at: clang/lib/ASTMatchers/Dynamic/Marshallers.cpp:102-106 + static constexpr llvm::StringRef Allowed[] = { + "UETT_SizeOf"

[PATCH] D77791: [ASTMatchers] Add support for dynamic matching of ofKind narrowing matcher

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, thank you! Comment at: clang/lib/ASTMatchers/Dynamic/Marshallers.cpp:102-106 + static constexpr llvm::StringRef Allowed[] = { + "UETT_SizeOf",

[PATCH] D77503: [ASTMatchers] Fixed CastKind being parsed incorrectly for dynamic matchers

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

[PATCH] D77836: [Attribute] Fix noderef attribute false-negatives

aaron.ballman added inline comments. Comment at: clang/test/Frontend/noderef.c:211 + +int *implicit_cast(NODEREF int *x) { + return (int *)x; // expected-warning{{casting to dereferenceable pointer removes 'noderef' attribute}} The name of the function is a bit

[PATCH] D77831: [clang-tidy] Convert config options that are bools to use the bool overload of get(GlobalOrLocal)?

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

[PATCH] D77461: [WIP][clang-tidy] Remove false positive in AvoidNonConstGlobalVariables

aaron.ballman added a comment. In D77461#1977503 , @vingeldal wrote: > In D77461#1963166 , @lebedev.ri > wrote: > > > https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Ri-global > > is i

[PATCH] D76083: [clang-tidy] Expand the list of functions in bugprone-unused-return-value

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp:98 + "::access;" + "::bind;" + "::connect;" aar

[PATCH] D77461: [WIP][clang-tidy] Remove false positive in AvoidNonConstGlobalVariables

aaron.ballman added a comment. In D77461#1977687 , @vingeldal wrote: > In D77461#1977639 , @aaron.ballman > wrote: > > > In D77461#1977503 , @vingeldal > > wrote: > > > > >

[PATCH] D78068: [www] Turn 'Clang 10' boxes green in C++ status pages to reflect release

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

[PATCH] D77461: [WIP][clang-tidy] Remove false positive in AvoidNonConstGlobalVariables

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp:50 +// variables in a function. +if (!Variable->isLocalVarDecl()) { + diag(Variable->getLocation(), "variable %0 is non-const and gl

[PATCH] D76083: [clang-tidy] Expand the list of functions in bugprone-unused-return-value

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp:98 + "::access;" + "::bind;" + "::connect;" sam

[PATCH] D77461: [WIP][clang-tidy] Remove false positive in AvoidNonConstGlobalVariables

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp:50 +// variables in a function. +if (!Variable->isLocalVarDecl()) { + diag(Variable->getLocation(), "variable %0 is non-const and gl

[PATCH] D78068: [www] Turn 'Clang 10' boxes green in C++ status pages to reflect release

aaron.ballman added inline comments. Comment at: clang/www/cxx_dr_status.html:3 "http://www.w3.org/TR/html4/strict.dtd";> hubert.reinterpretcast wrote: > hubert.reinterpretcast wrote: > > rsmith wrote: > > > Please heed this comment, and check in a

[PATCH] D77461: [WIP][clang-tidy] Remove false positive in AvoidNonConstGlobalVariables

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

[PATCH] D77461: [WIP][clang-tidy] Remove false positive in AvoidNonConstGlobalVariables

aaron.ballman closed this revision. aaron.ballman added a comment. I've commit on your behalf in b2d8c89ea48beb83e0392b1f00c9eafa33c09ca8 , thank you for the patch! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D61716: [libclang] Expose AtomicType

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Do you need me to commit on your behalf? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61716/new/ https://reviews.llvm.org/D61716 __

[PATCH] D80887: [clang-tidy] ignore builtin varargs from pro-type-vararg-check

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp:39 +"__builtin_classify_type", +// "__builtin_va_start", +// "__builtin_stdarg_start", njames93 wrote: > aaron.ballman wrote: > > I

[PATCH] D80887: [clang-tidy] ignore builtin varargs from pro-type-vararg-check

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

[PATCH] D80961: WIP: Ignore template instantiations if not in AsIs mode

aaron.ballman added a comment. In D80961#2073049 , @klimek wrote: > Without jumping into the discussion whether it should be the default, I think > we should be able to control template instantiation visitation separately > from other implicit nodes. >

[PATCH] D80896: [clang-tidy][misc-redundant-expression] Support for CXXFoldExpr

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, but please wait a bit in case @njames93 has any final thoughts. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80896/new/ https://reviews.llvm.org/D80896 ___

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

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

[PATCH] D74299: [clang-tidy] extend tests of run-clang-tidy

aaron.ballman added a comment. Generally looks good to me, but I had a question about test coverage. Comment at: clang-tools-extra/test/clang-tidy/infrastructure/run-clang-tidy_config-file.cpp:1 -// RUN: %run_clang_tidy --help // RUN: rm -rf %t Is there a rea

[PATCH] D81336: [clang-tidy] simplify-bool-expr ignores template instantiations

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp:425 Finder->addMatcher( - ifStmt(isExpansionInMainFile(), hasCondition(cxxBoolLiteral(equals(Value)).bind(BooleanId))) Th

[PATCH] D79912: Assignment and Inc/Dec operators wouldn't register as a mutation when Implicit Paren Casts were present

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

[PATCH] D81336: [clang-tidy] simplify-bool-expr ignores template instantiations

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp:425 Finder->addMatcher( - ifStmt(isExpansionInMainFile(), hasCondition(cxxBoolLiteral(equals(Value)).bind(BooleanId))) nj

[PATCH] D81336: [clang-tidy] simplify-bool-expr ignores template instantiations

aaron.ballman added a subscriber: LegalizeAdulthood. aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp:425 Finder->addMatcher( - ifStmt(isExpansionInMainFile(), hasCondition(cxxBoolLiteral

[PATCH] D80961: Ignore template instantiations if not in AsIs mode

aaron.ballman added a comment. In D80961#2079044 , @klimek wrote: > In D80961#2076242 , @aaron.ballman > wrote: > > > In D80961#2073049 , @klimek wrote: > > > > > Without ju

[PATCH] D81332: Thread safety analysis: Support deferring locks

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Should we update the public docs for this change as well? Specifically, I am wondering if we should update https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#mutexheader

[PATCH] D81352: Thread safety analysis: Add note for double unlock

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a nit. Comment at: clang/include/clang/Analysis/Analyses/ThreadSafety.h:111 /// \param Loc -- The SourceLocation of the Unlock + /// \param

[PATCH] D80944: Add begin source location for the attributed statement created from PragmaLoopHint decorated loop

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM aside from a minor nit. Comment at: clang/lib/Parse/ParseStmt.cpp:2189 + + assert(!Attrs.Range.getBegin().isValid()); + Attrs.Range.setBegin(StartLoc); You can use `isInvalid()` here ra

[PATCH] D81552: [ASTMatchers] Added hasDirectBase and hasClass Matchers

aaron.ballman added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2888-2890 +/// class Foo; +/// class Bar : Foo {}; +/// class Baz : Bar {}; It seems like these aren't really part of the example? Comment at:

[PATCH] D81552: [ASTMatchers] Added hasDirectBase and hasClass Matchers

aaron.ballman added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2888-2890 +/// class Foo; +/// class Bar : Foo {}; +/// class Baz : Bar {}; njames93 wrote: > aaron.ballman wrote: > > It seems like these aren't really part of t

[PATCH] D81455: [clang][NFC] Generate the {Type,ArrayType,UnaryExprOrType,Expression}Traits enumerations from TokenKinds.def...

aaron.ballman added inline comments. Comment at: clang/lib/Basic/ExpressionTraits.cpp:29-34 + assert(T <= ET_Last && "invalid enum value!"); + return ExpressionTraitNames[T]; +} + +const char *clang::getTraitSpelling(ExpressionTrait T) { + assert(T <= ET_Last && "invalid enum

[PATCH] D81455: [clang][NFC] Generate the {Type,ArrayType,UnaryExprOrType,Expression}Traits enumerations from TokenKinds.def...

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, your choice on hiding in TokenKinds.def. Comment at: clang/lib/Basic/ExpressionTraits.cpp:29-34 + assert(T <= ET_Last && "invalid enum value!"); + retur

[PATCH] D81552: [ASTMatchers] Added hasDirectBase and hasClass Matchers

aaron.ballman added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3537 AST_POLYMORPHIC_MATCHER_P_OVERLOAD( -hasType, -AST_POLYMORPHIC_SUPPORTED_TYPES(Expr, FriendDecl, ValueDecl, -CXXBaseSpecifier), +ha

[PATCH] D81552: [ASTMatchers] Added hasDirectBase and hasClass Matchers

aaron.ballman added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3553 +/// \endcode +AST_MATCHER_P(CXXBaseSpecifier, hasClass, internal::Matcher, + InnerMatcher) { njames93 wrote: > jkorous wrote: > > aaron.ballman wrote

[PATCH] D81336: [clang-tidy] simplify-bool-expr ignores template instantiations

aaron.ballman added a comment. In D81336#2090708 , @LegalizeAdulthood wrote: > Yeah, the restriction to the header file is a bug. It was a misconception on > my part when I originally wrote the matcher. Excellent, thank you for weighing in! I'm fine w

[PATCH] D81552: [ASTMatchers] Added hasDirectBase and hasClass Matchers

aaron.ballman added reviewers: sammccall, dblaikie. aaron.ballman added subscribers: sammccall, dblaikie. aaron.ballman added a comment. Pinging @klimek , @sammccall , and @dblaikie to see if there are some opinions about overloading `hasName` (and possibly other naming related questions). ===

[PATCH] D79285: [clang-tidy] Add diagnostics level to YAML output

aaron.ballman added inline comments. Comment at: clang/include/clang/Tooling/DiagnosticsYaml.h:109 + static void enumeration(IO &IO, clang::tooling::Diagnostic::Level &Value) { +IO.enumCase(Value, "Warning", clang::tooling::Diagnostic::Warning); +IO.enumCase(Value, "Erro

[PATCH] D81786: [clang][utils] Modify make-ast-dump-check.sh to generate AST serialization dump tests

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D81786#2091781 , @riccibruno wrote: > Summarising a quick discussion with @lebedev.ri on IRC yesterday: this is > not necessarily the

[PATCH] D81543: [CodeGen][TLS] Set TLS Model for __tls_guard as well.

aaron.ballman added a comment. Changes generally LGTM, but this is not my area of expertise. Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2622 Guard->setThreadLocal(true); +Guard->setThreadLocalMode(CGM.GetDefaultLLVMTLSModel()); Do we need a simi

[PATCH] D81787: [clang] Fix the serialization of LambdaExpr and the bogus mutation in LambdaExpr::getBody

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM with some nits. Comment at: clang/include/clang/AST/ExprCXX.h:2012-2017 + Stmt *getBody() const { return getStoredStmts()[capture_size()]; } + + /// Retr

[PATCH] D79285: [clang-tidy] Add diagnostics level to YAML output

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/Tooling/DiagnosticsYaml.h:109 + static void enumeration(IO &IO, clang::tooling::Diagnostic::Level &Value) { +IO.enumC

[PATCH] D81336: [clang-tidy] simplify-bool-expr ignores template instantiations

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

[PATCH] D81543: [CodeGen][TLS] Set TLS Model for __tls_guard as well.

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

[PATCH] D81552: [ASTMatchers] Added hasDirectBase and hasClass Matchers

aaron.ballman added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3553 +/// \endcode +AST_MATCHER_P(CXXBaseSpecifier, hasClass, internal::Matcher, + InnerMatcher) { sammccall wrote: > aaron.ballman wrote: > > jkorous wrot

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

aaron.ballman added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-assignment.cpp:29 +public: + Simple2() : n (0) { +x = 0.0; baloghadamsoftware wrote: > aaron.ballman wrote: > > By my rea

[PATCH] D75844: [clang] Set begin loc on GNU attribute parsed attrs

aaron.ballman added a comment. In D75844#2076260 , @tbaeder wrote: > I'm looking at this again and I am not sure how it is meant to work. For > example in `Parser::parseClassSpecifier` in `ParseDeclCXX.cpp`. > Here `attrs` is a local variable of type `Pa

[PATCH] D81953: [clang-tidy] warnings-as-error no longer exits with ErrorCount

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM unless @jroelofs has a reason why the code was originally written that way, but can you add test coverage for it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D81953: [clang-tidy] warnings-as-error no longer exits with ErrorCount

aaron.ballman added a comment. In D81953#2096635 , @njames93 wrote: > In D81953#2096388 , @aaron.ballman > wrote: > > > LGTM unless @jroelofs has a reason why the code was originally written that > > way, but can

[PATCH] D82004: [clang-tidy][NFC] Remove the double look-up on IncludeInserter

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

[PATCH] D82940: [ASTReader][ASTWriter][PCH][Modules] Fix (de)serialization of SwitchCases

aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. In D82940#2125005 , @riccibruno wrote: > In D82940#2124955 , @aaron.ballman > wrote: >

[PATCH] D82706: [ASTMatchers] Enhanced support for matchers taking Regex arguments

aaron.ballman added inline comments. Comment at: clang/lib/ASTMatchers/Dynamic/Marshallers.cpp:124 +llvm::Optional getRegexFlag(llvm::StringRef Flag) { + for (auto &StringFlag : RegexMap) { +if (Flag == StringFlag.first) `const auto &` (same below) ===

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

aaron.ballman added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-assignment.cpp:29 +public: + Simple2() : n (0) { +x = 0.0; baloghadamsoftware wrote: > aaron.ballman wrote: > > baloghada

[PATCH] D82921: Removed a RecursiveASTVisitor feature to visit operator kinds with different methods

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM, I think the complexity argument is reasonable (and I appreciate the information showing this also reduces binary size at the same time). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D82924: [clang-tidy] fix cppcoreguidelines-init-variables with catch variables

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

[PATCH] D82940: [ASTReader][ASTWriter][PCH][Modules] Fix (de)serialization of SwitchCases

aaron.ballman added a comment. In D82940#2125183 , @riccibruno wrote: > > I'm not certain I understand, but I'm removing my LG while we figure it > > out. Can you expound a bit further? > > Take a look at the AST dump of a small modification of the reduce

[PATCH] D83009: [clang][Serialization] Don't duplicate the body of LambdaExpr during deserialization

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

[PATCH] D82880: Fix PR35677: UB on __int128_t or __uint128_t template parameters.

aaron.ballman added inline comments. Comment at: clang/lib/AST/StmtPrinter.cpp:1159 + case BuiltinType::UInt128: +OS << "Ui128"; +break; riccibruno wrote: > riccibruno wrote: > > davidstone wrote: > > > riccibruno wrote: > > > > `i128` and `Ui128` are no

[PATCH] D82706: [ASTMatchers] Enhanced support for matchers taking Regex arguments

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

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

aaron.ballman added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-member-initializer-assignment.cpp:29 +public: + Simple2() : n (0) { +x = 0.0; baloghadamsoftware wrote: > aaron.ballman wrote: > > baloghada

[PATCH] D83076: Revert AST Matchers default to AsIs mode

aaron.ballman accepted this revision. aaron.ballman added a comment. LG, but should this also update the release notes to remove the mention about the new default? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83076/new/ https://reviews.llvm.org/D

[PATCH] D82904: [clang-tidy] Warn pointer captured in async block

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp:82 "bugprone-bool-pointer-implicit-conversion"); -CheckFactories.registerCheck( -"bugprone-branch-clone"); +CheckFactories.registerCheck("bugpron

[PATCH] D83183: [clang] Rework how and when APValues are dumped

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: clang/test/Import/switch-stmt/test.cpp:8 // CHECK-NEXT: ConstantExpr +// CHECK-NEXT: value: Int 1 // CHECK-NEXT: IntegerLiteral

[PATCH] D83183: [clang] Rework how and when APValues are dumped

aaron.ballman added a comment. Do none of the JSON tests break from this change? Comment at: clang/include/clang/AST/TextNodeDumper.h:163 + raw_ostream &getOS() { return OS; } + This is a pretty strange public method; any way to limit its visibility? =

[PATCH] D82425: [SemaCXX] Fix false positive of -Wuninitialized-const-reference in empty function body.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM despite my enduring sadness that Boost elected to use this totally bizarre approach to silencing unused variable diagnostics when far better-supported options have existed f

[PATCH] D82904: [clang-tidy] Warn pointer captured in async block

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp:82 "bugprone-bool-pointer-implicit-conversion"); -CheckFactories.registerCheck( -"bugprone-branch-clone"); +CheckFactories.registerCheck("bugpron

[PATCH] D80301: [yaml][clang-tidy] Fix multiline YAML serialization

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, this seems like a pretty clean solution. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80301/new/ https://reviews.ll

[PATCH] D82904: [clang-tidy] Warn pointer captured in async block

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM with a testing request. Comment at: clang-tools-extra/clang-tidy/bugprone/NoEscapeCheck.cpp:34 + const BlockDecl *EscapingBlockDecl = MatchedEscapingBlock

[PATCH] D81552: [ASTMatchers] Added hasDirectBase Matcher

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM with a whitespace nit. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2895 + BaseSpecMatcher) { + + return Node.hasDefinition() &&

[PATCH] D82278: Fix traversal over CXXConstructExpr in Syntactic mode

aaron.ballman added reviewers: rsmith, sammccall. aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp:80 auto Matches = - match(expr(hasDescendant(typeLoc().bind("t"))), *E, *Result.Context); + match(traverse(TK_

[PATCH] D82904: [clang-tidy] Warn pointer captured in async block

aaron.ballman closed this revision. aaron.ballman added a comment. In D82904#2136686 , @ellis wrote: > Hey @aaron.ballman, thanks for accepting my diff! Would you mind landing my > diff since I don't have commit access yet? Gladly -- I've committed on y

[PATCH] D82904: [clang-tidy] Warn pointer captured in async block

aaron.ballman added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-no-escape.rst:13 + + .. code-block:: objc +void foo(__attribute__((noescape)) int *p) { ellis wrote: > It looks like I triggered a build failure for the clang-

[PATCH] D83402: ParsedAttrInfo: Change spelling to use StringRef instead of const char*

aaron.ballman added a comment. I think the change is safe to make, but I'm slightly uncomfortable with it because storing a `StringRef` is a somewhat questionable practice given that it's non-owning and it has converting constructors from dangerous types like `std::string`. Repository: rG L

[PATCH] D83502: Change behavior with zero-sized static array extents

aaron.ballman created this revision. aaron.ballman added reviewers: rsmith, echristo, dblaikie, rjmccall. Currently, Clang diagnoses this code by default: `void f(int a[static 0]);` saying that "static has no effect on zero-length arrays" and this diagnostic is accurate for our implementation.

[PATCH] D83502: Change behavior with zero-sized static array extents

aaron.ballman marked an inline comment as done. aaron.ballman added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:2515 +} else { + AI->addAttr(llvm::Attribute::NonNull); +} rjmccall wrote: > Isn't the o

[PATCH] D83174: Teach AttachPreviousImpl to inherit MSInheritanceAttr attribute

aaron.ballman added a comment. In D83174#2137133 , @rsmith wrote: > @aaron.ballman We will need to do something like this in general, but I'm not > sure it's going to be as easy as just inheriting the attribute in the general > case. What do you think?

[PATCH] D83502: Change behavior with zero-sized static array extents

aaron.ballman added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:2515 +} else { + AI->addAttr(llvm::Attribute::NonNull); +} rjmccall wrote: > aaron.ballman wrote: > > rjmccall wrote: > > > Isn't the ol

[PATCH] D83502: Change behavior with zero-sized static array extents

aaron.ballman updated this revision to Diff 277120. aaron.ballman marked an inline comment as done. aaron.ballman added a comment. Updated based on review feedback. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83502/new/ https://reviews.llvm.org/D83502 Files: clang/include/clang/Bas

[PATCH] D83502: Change behavior with zero-sized static array extents

aaron.ballman closed this revision. aaron.ballman marked an inline comment as done. aaron.ballman added a comment. Thanks for the review, I've gone ahead and committed in 006c49d890da633d1ce502117fc2a49863cd65b7 ===

[PATCH] D80514: [clang-tidy] modernize-use-trailing-return-type support for C++20 concepts and decltype

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:293 +StringRef File = SM.getBufferData(Loc.first); +const char *TokenBegin = File.data() + Loc.second; +Lexer Lexer(SM.getLocForStartOfFile(Loc.firs

[PATCH] D83223: [clang-tidy] Header guard check can skip past license comment

aaron.ballman added a comment. > // This is not identified as a license comment as the > // block is followed by code. > void foo(); FWIW: https://github.com/GrammaTech/gtirb-pprinter/blob/master/include/gtirb_pprinter/AttPrettyPrinter.hpp or https://github.com/GrammaTech/gtirb/blob/maste

[PATCH] D83223: [clang-tidy] Header guard check can skip past license comment

aaron.ballman added a comment. In D83223#2147247 , @njames93 wrote: > In D83223#2147072 , @aaron.ballman > wrote: > > > > // This is not identified as a license comment as the > > > // block is followed by code

[PATCH] D83616: [clang] Add 'override' to virtual function overrides generated by ClangAttrEmitter

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

[PATCH] D83550: [PATCH 1/4][Sema][AArch64] Add parsing support for arm_sve_vector_bits attribute

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1538 + let Args = [IntArgument<"NumBits">]; + let Documentation = [Undocumented]; +} No new, undocumented attributes, please. Comment at: clang/include/clang/B

[PATCH] D83551: [PATCH 2/4][Sema][AArch64] Add semantics for arm_sve_vector_bits attribute

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1541 +def ArmSveVectorBits128 : TypeAttr { + let Spellings = []; sdesmalen wrote: > nit: Can you add a comment saying why these are undocumented (and have no > spellings) Also,

[PATCH] D83053: [clang-tidy] OptionsView::store specialized on bool

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

[PATCH] D83174: Teach AttachPreviousImpl to inherit MSInheritanceAttr attribute

aaron.ballman added inline comments. Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:3544 +const auto *IA = +dyn_cast(Previous->getAttr()); + aaron.ballman wrote: > Don't do `hasAttr` followed by `getAttr` (that duplicates work); also, you > do

[PATCH] D83551: [PATCH 2/4][Sema][AArch64] Add semantics for arm_sve_vector_bits attribute

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1541 +def ArmSveVectorBits128 : TypeAttr { + let Spellings = []; c-rhodes wrote: > sdesmalen wrote: > > aaron.ballman wrote: > > > sdesmalen wrote: > > > > nit: Can you add a co

[PATCH] D83550: [PATCH 1/4][Sema][AArch64] Add parsing support for arm_sve_vector_bits attribute

aaron.ballman added inline comments. Comment at: clang/test/Sema/arm-feature-sve-bits-macro.c:24 +#define __ARM_FEATURE_SVE_BITS N +typedef svint8_t macro_non_int_size2 __attribute__((arm_sve_vector_bits(N))); // expected-error {{'__ARM_FEATURE_SVE_BITS' must expand to an intege

[PATCH] D83681: [clang] Provide a more specific diagnostic for a misplaced lambda capture-default.

aaron.ballman added inline comments. Comment at: clang/lib/Parse/ParseExprCXX.cpp:937 + return Invalid([&] { +Diag(Tok.getLocation(), diag::err_capture_default_first); + }); Would it make sense to provide a fix-it to move the capture default to

[PATCH] D83681: [clang] Provide a more specific diagnostic for a misplaced lambda capture-default.

aaron.ballman added a comment. Thanks for the additional tests. This LGTM, though I still wonder about the fix-it (not certain if you saw the comment). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83681/new/ https://reviews.llvm.org/D83681 ___

[PATCH] D83681: [clang] Provide a more specific diagnostic for a misplaced lambda capture-default.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: clang/lib/Parse/ParseExprCXX.cpp:937 + return Invalid([&] { +Diag(Tok.getLocation(), diag::err_capture_default_first); + });

[PATCH] D80753: [clang-tidy] remove duplicate fixes of alias checkers

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a few small nits, but please wait a bit for @njames93 in case they have final thoughts. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagn

[PATCH] D79755: Implement constexpr BinaryOperator for vector types

aaron.ballman added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:2699 +RHSValue.getInt(), Result); + assert(LHSValue.getKind() == APValue::Float && "SHhuld be no other options"); + return handleLogicalOpForVector(LHSValue.getFl

[PATCH] D82059: [clang-tidy] RenamerClangTidy group redecls into 1 warning.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:383 +{ + const auto *Canonical = cast(Decl->getCanonicalDecl()); -

[PATCH] D79755: Implement constexpr BinaryOperator for vector types

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

[PATCH] D82279: Handle invalid types in the nullPointerConstant AST matcher

aaron.ballman created this revision. aaron.ballman added reviewers: rsmith, steveire, klimek, gribozavr. Currently, using the nullPointerConstant AST matcher can lead to assertions in situations where a node to be matched does not have a valid type associated with it, such as a ParenListExpr. Th

<    16   17   18   19   20   21   22   23   24   25   >