[PATCH] D82223: [clang-tidy] Implement storeOptions for checks missing it.

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

[PATCH] D82188: [clang-tidy] Reworked enum options handling(again)

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. I like the direction of this, thank you! LGTM with a small suggestion, but you should wait a few days in case one of the other reviewers has comments. Comment

[PATCH] D82162: [clang-tidy] RenamerClangTidy wont emit fixes in scratch space

aaron.ballman added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming.cpp:570 + +// We don't want a warning here as the call to this in Foo is in a scratch +// buffer so its fix-it wouldn't be applied, resulting in invalid code

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

aaron.ballman marked an inline comment as done. aaron.ballman added inline comments. Comment at: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp:2618 + )"; + EXPECT_TRUE(matches(kTest, expr(nullPointerConstant(; } steveire wrote: > While this test

[PATCH] D82162: [clang-tidy] RenamerClangTidy wont emit fixes in scratch space

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

[PATCH] D82281: [clang-tidy] llvm-twine-local ignores parameters

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

[PATCH] D81769: [clang-tidy] Repair various issues with modernize-avoid-bind

aaron.ballman added a comment. In D81769#2106574 , @jaafar wrote: > "Ping". I hope this can be considered for 10.0.1. If nothing else I think > reviewing the test cases has a lot of value - there are some real issues with > the current checks and fixits.

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

aaron.ballman closed this revision. aaron.ballman added a comment. Thanks for the review! Committed in 8a9311940a26372dab6706edfd07288667394cfe CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82279/new/ https://reviews

[PATCH] D81769: [clang-tidy] Repair various issues with modernize-avoid-bind

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Aside from a minor nit, this LGTM Comment at: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp:414-415 + for (const clang::Decl *D : RecordDecl->decls

[PATCH] D82446: [clang] Fix duplicate warning

aaron.ballman added inline comments. Comment at: clang/lib/Parse/ParseExprCXX.cpp:2273 ParseSpecifierQualifierList(DS, AS_none, DeclSpecContext::DSC_type_specifier); - DS.Finish(Actions, Actions.getASTContext().getPrintingPolicy()); return false; This cha

[PATCH] D81769: [clang-tidy] Repair various issues with modernize-avoid-bind

aaron.ballman added a comment. In D81769#2109936 , @jaafar wrote: > One more simplification from Aaron. Thanks! No problem! Do you need someone to commit on your behalf, btw? If so, let me know what name and email address you would like me to attribute.

[PATCH] D82446: [clang] Fix duplicate warning

aaron.ballman added inline comments. Comment at: clang/lib/Parse/ParseExprCXX.cpp:2273 ParseSpecifierQualifierList(DS, AS_none, DeclSpecContext::DSC_type_specifier); - DS.Finish(Actions, Actions.getASTContext().getPrintingPolicy()); return false; kamleshb

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

aaron.ballman added a comment. Personally, I think this behavior is a bit mysterious given that there are *much* better ways to silence unused variable warnings (like casting to `void`) that have been well-supported by compilers for a long time and I'd prefer not to penalize every call expressi

[PATCH] D81769: [clang-tidy] Repair various issues with modernize-avoid-bind

aaron.ballman closed this revision. aaron.ballman added a comment. Thank you for the patch! I've commit on your behalf in 95a3550dc89a0d424d90e2c0ad30d9ecfa9422cf CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81769/n

[PATCH] D82661: [clang-tidy][NFC] Remove unnecessary includes throughout clang-tidy header files

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM, though I think this should be landed in two commits (one for the headers, one for the tests). Comment at: clang-tools-extra/test/clang-tidy/checkers/google-module.cpp:4 +// CHECK-DAG: {{- key: *google-

[PATCH] D82711: [clang-tidy] Fix hicpp-named-paramater

aaron.ballman added a comment. I do not understand the justification here -- `IdentifierNamingCheck` and `NamedParameterCheck` are two different checks. How do you know that this swap continues to honor the HIC++ coding standard? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D82720: [clang-tidy] performance-faster-string-find string-view

aaron.ballman added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/performance-faster-string-find.rst:27 + ``::std::basic_string`` and ``::std::basic_string_view`` are considered. + The list of methods to consired is fixed. I'm not

[PATCH] D82711: [clang-tidy] Fix hicpp-named-paramater

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D82711#2119714 , @njames93 wrote: > In D82711#2119651 , @aaron.ballman > wrote: > > > I do no

[PATCH] D82720: [clang-tidy] performance-faster-string-find string-view

aaron.ballman added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/performance-faster-string-find.rst:27 + ``::std::basic_string`` and ``::std::basic_string_view`` are considered. + The list of methods to consired is fixed. njames93

[PATCH] D82815: [clang-tidy] Sanity checks in ClangTidyTest header.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM (the harbormaster failure looks unrelated to your changes from what I can tell) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D82823: canonicalize macOS 10.16 availability to macOS 11 while preserving uses of if @available macOS 10.16

aaron.ballman added inline comments. Comment at: clang/include/clang/AST/ExprObjC.h:1719-1721 + VersionTuple getVersion() { return VersionToCheck.Version; } + VersionTuple getVersionAsWritten() { return VersionToCheck.SourceVersion; } Any reason these functio

[PATCH] D82720: [clang-tidy] performance-faster-string-find string-view

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

[PATCH] D82824: [clang-tidy] Added option to readability-else-after-return

aaron.ballman added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-else-after-return.rst:67 + + When `true`, Emit a warning for cases where the check can't output a + Fix-It. These can occur with declarations inside the ``else`` branch tha

[PATCH] D82825: [clang-tidy] Added alias llvm-else-after-return.

aaron.ballman added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/llvm-else-after-return.rst:3 +.. meta:: + :http-equiv=refresh: 5;URL=readability-else-after-return.html + Hmmm, this means users have five seconds to read the text below

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

aaron.ballman added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchersInternal.h:1952 +std::shared_ptr createAndVerifyRegex(StringRef Regex, + llvm::Regex::RegexFlags Flags, Is there a reas

[PATCH] D82824: [clang-tidy] Added option to readability-else-after-return

aaron.ballman added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-else-after-return.rst:74 + + When `true`, The check will attempt to refactor a variable defined inside + the condition of the ``if`` statement that is used in the ``else`` b

[PATCH] D82825: [clang-tidy] Added alias llvm-else-after-return.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Other than the documentation and settling on the option name in the parent revision, I think this LGTM (the changes will be mechanical and can be done without further review once

[PATCH] D82824: [clang-tidy] Added option to readability-else-after-return

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp:193 if (checkConditionVarUsageInElse(If) != nullptr) { +if (!WarnOnConditionVariables) + return; Would it make sense to hoist this into

[PATCH] D82824: [clang-tidy] Added option to readability-else-after-return

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp:193 if (checkConditionVarUsageInElse(If) != nullptr) { +if (!WarnOnConditionVariables) + return; njames93 wrote: > aaron.ballman wrote:

[PATCH] D82825: [clang-tidy] Added alias llvm-else-after-return.

aaron.ballman added a comment. In D82825#2123125 , @njames93 wrote: > In D82825#2122789 , @aaron.ballman > wrote: > > > Other than the documentation and settling on the option name in the parent > > revision, I th

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

aaron.ballman added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchersInternal.h:1952 +std::shared_ptr createAndVerifyRegex(StringRef Regex, + llvm::Regex::RegexFlags Flags, njames93 wrote:

[PATCH] D82824: [clang-tidy] Added option to readability-else-after-return

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM once the documentation nit is fixed up. Comment at: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp:193 if (checkConditionVarUsageInE

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

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! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82940/new/ https://reviews.llvm.org/D82940 _

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

aaron.ballman added inline comments. Comment at: clang/test/Analysis/cfg.cpp:570 } +// CHECK-LABEL: void vla_simple(int x) Can you also add tests for: ``` int vla_unevaluated(int x) { // Evaluates the ++x sizeof(int[++x]); // Does not evaluate the ++x

[PATCH] D61716: [libclang] Expose AtomicType

aaron.ballman closed this revision. aaron.ballman added a comment. I've commit on your behalf in 38ca7b11db2d22e0fdfbff3f19276f9796f747d3 , thank you for the patch! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61716/

[PATCH] D46317: [clang-tidy] New check bugprone-map-subscript-operator-lookup

aaron.ballman added a comment. In D46317#1988261 , @lebedev.ri wrote: > IMHO we should proceed with this patch. > There's been several patches recently that seem to not care about > false-positive rate, I don't think that's accurate. We still care abou

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

aaron.ballman added inline comments. Comment at: clang/test/Analysis/cfg.cpp:570 } +// CHECK-LABEL: void vla_simple(int x) NoQ wrote: > balazske wrote: > > balazske wrote: > > > aaron.ballman wrote: > > > > Can you also add tests for: > > > > ``` > > > > int v

[PATCH] D78567: C++2a -> C++20 in some identifiers; NFC

aaron.ballman created this revision. aaron.ballman added reviewers: rsmith, dblaikie, echristo. Herald added subscribers: martong, kbarton, nemanjai. This does not hit all of the C++2a -> C++20 changes, but gets all of the ones related to the language options and keywords. https://reviews.llvm.

[PATCH] D78567: C++2a -> C++20 in some identifiers; NFC

aaron.ballman closed this revision. aaron.ballman added a comment. Thanks! I've committed in 6a30894391ca671bab16c505eff30c7819bd8e8e . If I get a spare moment, I may do this dance again with diagnostic identifers as well. CH

[PATCH] D78693: Make "#pragma clang attribute" support uninitialized attribute.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: clang/test/CodeGenCXX/trivial-auto-var-init-attribute.cpp:38-50 +// UNINIT-LABEL: test_pragma_attribute_uninitialized_f2( +// UNINIT: al

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

aaron.ballman added inline comments. Comment at: clang/test/Analysis/cfg.cpp:570 } +// CHECK-LABEL: void vla_simple(int x) balazske wrote: > aaron.ballman wrote: > > NoQ wrote: > > > balazske wrote: > > > > balazske wrote: > > > > > aaron.ballman wrote: > > >

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM with a minor nit, thank you! Comment at: clang/lib/Analysis/CFG.cpp:2855 + VA = FindVA(VA->getElementType().getTypePtr())) { + if (CFGBlock *newBlock = addStmt(VA->getSizeExpr())) +La

[PATCH] D77809: [Analyzer] Include typedef statements in CFG build.

aaron.ballman added inline comments. Comment at: clang/lib/Analysis/CFG.cpp:2855 + VA = FindVA(VA->getElementType().getTypePtr())) { + if (CFGBlock *newBlock = addStmt(VA->getSizeExpr())) +LastBlock = newBlock; balazske wrote: > aaron.ballman

[PATCH] D78777: [AST] Use PrintingPolicy for format string diagnosis

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

[PATCH] D77979: [clang-tidy] modernize-use-using: Fix broken fixit with InjectedClassName

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/include/clang/AST/PrettyPrinter.h:248 + /// Whether to print an InjectedClassNameType with template argumen

[PATCH] D78508: [Clang] Allow long as size_t printf argument on 32-bit Windows platforms.

aaron.ballman added a comment. Personally, I don't consider those to be useless casts. They appropriately ensure that the types match between the arguments and the format specifiers, rather than relying on underlying details of what SIZE_T expands to for a given target. I would expect these to

[PATCH] D73052: [clang-tidy] RenamerClangTidy now renames dependent member expr when the member can be resolved

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:186-188 + NameLookup(const NamedDecl *ND) : Data(ND, false) {} + NameLookup(llvm::NoneType) : Data(nullptr, true) {} + NameLookup(std::nullptr_t) : Data(nullptr, false

[PATCH] D78223: [clang-tidy] performance-range-for-copy only for copy.

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM aside from some small nits Comment at: clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp:68 +// loop index because it's less clear than copying the temporary. +static bool isCopyConstructedFr

[PATCH] D78890: [clang-tidy] Add check callee-namespace.

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:22-23 +const DeclContext *getOutermostNamespace(const DeclContext *Decl) { + if (Decl->isTranslationUnit()) +return Decl; + if (Decl->getParent() && Decl->getPare

[PATCH] D79012: [AST] Fix a crash on a dependent vector_size attribute

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

[PATCH] D78890: [clang-tidy] Add check callee-namespace.

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. Comment at: clang-tools-extra/clang-tidy/llvmlibc/CalleeNamespaceCheck.cpp:52-53 + + diag(FuncDecl->getLocation(), + "cu

[PATCH] D78904: [clang-tidy] extend bugprone-signed-char-misuse check with array subscript case.

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. Comment at: clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp:106 + + // Catch array subscripts with singed char

[PATCH] D79121: Add nomerge function attribute to clang

aaron.ballman added a reviewer: aaron.ballman. aaron.ballman added a comment. This should also have Sema tests ensuring that it only applies to the correct subjects, accepts no arguments, etc. Comment at: clang/include/clang/Basic/Attr.td:1799 + let Spellings = [Clang<"nomerg

[PATCH] D79121: Add nomerge function attribute to clang

aaron.ballman added a comment. This is missing Sema tests ensuring that the attribute only appertains to the correct subject, does not accept arguments, etc. Comment at: clang/include/clang/Basic/AttrDocs.td:356 + let Content = [{ +Calls to functions marked `nomerge` will not

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6898 case ParsedAttr::AT_Constructor: -handleConstructorAttr(S, D, AL); +if (S.Context.getTargetInfo().getTriple().isOSAIX()) + S.Diag(AL.getLoc(), diag::warn_attribute_type_not_suppo

[PATCH] D79094: [SemaObjC] Warn on visibility attributes on an @implementation

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. This seems reasonable to me. I agree that this feels like it's something that should be written on the declaration in the header, not just magically appear on the definition, but

[PATCH] D79113: Revert "Remove false positive in AvoidNonConstGlobalVariables."

aaron.ballman added a comment. I only see a comment on the issue, but not that the issue was resolved in favor of that comment (unless I've missed something). If the issue gets resolved in the direction that comment is going, then the revert makes sense (though we should add an explicit test ca

[PATCH] D79292: [sema] NFC Unable to build Sema library with MSVC Debug target due to missing /bigobj

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

[PATCH] D84005: Introduce ns_error_domain attribute.

aaron.ballman added inline comments. Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:332 << "()->getName() : \"\") << \""; + else if (type == "VarDecl *") +OS << "\" << get" << getUpperName() << "()->getName() << \""; I think this c

[PATCH] D84814: [clang-tidy] readability-identifier-naming checks configs for included files

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. This looks reasonable to me. Comment at: clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/global-style-disabled/header.h:4 +void

[PATCH] D83717: [clang-tidy] Add check fo SEI CERT item ENV32-C

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:33 +bool isExitFunction(StringRef FnName) { + return FnName == "_Exit" || FnName == "exit" || FnName == "quick_exit"; +} Because this rule applies in C++ as w

[PATCH] D83188: [clang-tidy] bugprone-bool-pointer-implicit-conversion doesn't handle members

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Re-accepting the changes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83188/new/ https://reviews.llvm.org/D83188 ___ cfe-c

[PATCH] D83188: [clang-tidy] bugprone-bool-pointer-implicit-conversion doesn't handle members

aaron.ballman closed this revision. aaron.ballman added a comment. Thank you for the patch, I've gone ahead and commit on your behalf in a44161692ae879068d4086a7e568a348800ba01d . CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D85033: [clang] Provide a better pretty-printed name for unnamed parameters, lambda classes and lambda captures.

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. Comment at: clang/lib/AST/Decl.cpp:2130 + // the pretty-printed name of the capture instead. + if (isa(DD) && + maybePrintFi

[PATCH] D83717: [clang-tidy] Add check fo SEI CERT item ENV32-C

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:33 +bool isExitFunction(StringRef FnName) { + return FnName == "_Exit" || FnName == "exit" || FnName == "quick_exit"; +} njames93 wrote: > whisperity wrote: >

[PATCH] D83717: [clang-tidy] Add check fo SEI CERT item ENV32-C

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/ExitHandlerCheck.cpp:64 +/// argument. +void ExitHandlerCheck::registerMatchers(MatchFinder *Finder) { + const auto IsRegisterFunction = whisperity wrote: > aaron.ballman wrote: >

[PATCH] D85450: Support the standards-based dates for __has_c_attribute

aaron.ballman created this revision. aaron.ballman added a reviewer: rsmith. Herald added a subscriber: krytarowski. aaron.ballman requested review of this revision. WG14 N2481 was adopted with minor modifications at this week's WG14 meetings. The only modification to the paper was to correct the

[PATCH] D85545: [Diagnostics] Diagnose missing comma in string array initialization

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3003 +def warn_concatenated_literal_array_init : Warning< + "concatenated literal in a string array initialization - " + "possibly missing a comma">, How about: `s

[PATCH] D85536: [clang] Add a matcher for template template parameters.

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

[PATCH] D85193: [clang] Do not use an invalid expression to update the initializer.

aaron.ballman added a comment. Should the `StructuredIndex` still be incremented even in the case of an error, as done around line 1589 and 1647? Do we need a similar change around line 2405? I sort of wonder if the correct change is to make `UpdateStructuredListElement()` resilient to being pa

[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:430 + AT->getKeyword() == AutoTypeKeyword::Auto && + !hasAnyNestedLocalQualifiers(F->getDeclaredReturnType())) +return; aaron.bal

[PATCH] D85545: [Diagnostics] Diagnose missing comma in string array initialization

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a nit, but you should give other reviewers a chance to comment if they have additional feedback. Comment at: clang/lib/Sema/SemaExpr.cpp:6911 +

[PATCH] D85611: Improve dynamic AST matching diagnostics for conversion errors

aaron.ballman created this revision. aaron.ballman added reviewers: steveire, sbenza, klimek, sammccall. aaron.ballman requested review of this revision. Currently, when marshaling a dynamic AST matchers, we check for the type and value validity of matcher arguments at the same time for some matc

[PATCH] D85612: [Sema] Use proper integral cast for an enumerate with a fixed bool type

aaron.ballman added inline comments. Comment at: clang/test/AST/ast-dump-enum-bool.cpp:1 +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -ast-dump=json %s | FileCheck %s + riccibruno wrote: > Why a (pretty unreadable) json test? There is also `m

[PATCH] D84636: [RFC] Make the default LibCall implementations from compiler-rt builtins library more customizable

aaron.ballman added inline comments. Comment at: compiler-rt/lib/builtins/arm-libcall-overrides.h:8 +//===--===// + +#define AEABI_RTABI __attribute__((__pcs__("aapcs"))) It looks like you're mis

[PATCH] D84602: [MSP430] Expose msp430_builtin calling convention to C code

aaron.ballman added a reviewer: echristo. aaron.ballman added a subscriber: echristo. aaron.ballman added a comment. Adding @echristo for the debugging info question. In D84602#2200274 , @atrosinenko wrote: > I suspect there might be some terminology clas

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

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D80514#2206019 , @bernhardmgruber wrote: > Btw: what is the general rule for Phabricator reviews here when to tick the > `Done` check

[PATCH] D84005: Introduce ns_error_domain attribute.

aaron.ballman accepted this revision. aaron.ballman added a comment. Continues to LGTM with your changes, but I did spot one tiny nit (no further reviews needed from me). Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5346 + if (!isNSStringType(VD->getType(), S.Context)) { +

[PATCH] D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt

aaron.ballman added a comment. Thank you for starting the implementation work on these attributes! Comment at: clang/include/clang/Basic/Attr.td:1288 +def Likely : StmtAttr { + let Spellings = [CXX11<"", "likely", 201803>]; + let Documentation = [LikelihoodDocs];

[PATCH] D85193: [clang] Do not use an invalid expression to update the initializer.

aaron.ballman added a comment. In D85193#2207052 , @ArcsinX wrote: > In D85193#2204685 , @aaron.ballman > wrote: > >> Should the `StructuredIndex` still be incremented even in the case of an >> error, as done arou

[PATCH] D81272: [clang-tidy] New check `misc-redundant-condition`

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp:46 +CheckFactories.registerCheck( +"misc-redundant-condition"); CheckFactories.registerCheck( I think this check should probably live in the `bu

[PATCH] D85193: [clang] Do not use an invalid expression to update the initializer.

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:3088 // No structured initializer list to update if (!StructuredList) return; I would move the check up to here as the only time we should get a null expression is if somethi

[PATCH] D85193: [clang] Do not use an invalid expression to update the initializer.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: clang/lib/Sema/SemaInit.cpp:3088 // No structured initializer list to update if (!StructuredList) return; ArcsinX

[PATCH] D85193: [clang] Check `expr` inside `InitListChecker::UpdateStructuredListElement()`

aaron.ballman added a comment. I have a suggestion for the added comment, but I also forgot to ask, do you need someone to commit on your behalf? Comment at: clang/lib/Sema/SemaInit.cpp:3094 +// This initializer overwrites a previous initializer. +// Emit warning if `e

[PATCH] D83224: [clangd] Move clang-tidy check modifications into ClangdServer

aaron.ballman added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:115 +// either due to crashes or false positives. +const char *getClangTidyBlacklist() { + static const std::string FalsePositives = njames93 wrote: > Return by StringRef?

[PATCH] D81272: [clang-tidy] New check `misc-redundant-condition`

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/RedundantConditionCheck.cpp:73 + // If the variable has an alias then it can be changed by that alias as well. + // FIXME: Track pointers and references. + if (hasPtrOrReferenceInFunc(Func, Cond

[PATCH] D85193: [clang] Check `expr` inside `InitListChecker::UpdateStructuredListElement()`

aaron.ballman accepted this revision. aaron.ballman added a comment. In D85193#2213384 , @ArcsinX wrote: > In D85193#2212976 , @aaron.ballman > wrote: > >> I have a suggestion for the added comment, > > Fixed comme

[PATCH] D83224: [clangd] Move clang-tidy check modifications into ClangdServer

aaron.ballman added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:115 +// either due to crashes or false positives. +const char *getClangTidyBlacklist() { + static const std::string FalsePositives = kadircet wrote: > aaron.ballman wrote:

[PATCH] D81272: [clang-tidy] New check `misc-redundant-condition`

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp:46 +CheckFactories.registerCheck( +"misc-redundant-condition"); CheckFactories.registerCheck( baloghadamsoftware wrote: > aaron.ballman wrote: >

[PATCH] D83224: [clangd] Move clang-tidy check modifications into ClangdServer

aaron.ballman added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:120 + // clangd doesn't replay those when using a preamble. + "-llvm-header-guard"); + static const std::string CrashingChecks = ---

[PATCH] D85450: Support the standards-based dates for __has_c_attribute

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. I'm accepting these changes on my own authority as code owner; if there are issues, I'll deal with them post-commit. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85450/

[PATCH] D85450: Support the standards-based dates for __has_c_attribute

aaron.ballman closed this revision. aaron.ballman added a comment. Changes committed in 9936b96d5333af4e6dff55025943366bb5f07272 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85450/new/ https://reviews.llvm.org/D8545

[PATCH] D81272: [clang-tidy] New check `misc-redundant-condition`

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp:46 +CheckFactories.registerCheck( +"misc-redundant-condition"); CheckFactories.registerCheck( baloghadamsoftware wrote: > aaron.ballman wrote: >

[PATCH] D83224: [clangd] Move clang-tidy check modifications into ClangdServer

aaron.ballman added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:120 + // clangd doesn't replay those when using a preamble. + "-llvm-header-guard"); + static const std::string CrashingChecks = ---

[PATCH] D83224: [clangd] Move clang-tidy check modifications into ClangdServer

aaron.ballman added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:120 + // clangd doesn't replay those when using a preamble. + "-llvm-header-guard"); + static const std::string CrashingChecks = ---

[PATCH] D81272: [clang-tidy] New check `misc-redundant-condition`

aaron.ballman added a comment. Poking @alexfh for more opinions about check similarity. Comment at: clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp:46 +CheckFactories.registerCheck( +"misc-redundant-condition"); CheckFactories.registerCheck( --

[PATCH] D83224: [clangd] Move clang-tidy check modifications into ClangdServer

aaron.ballman added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:115 +// either due to crashes or false positives. +const char *getClangTidyBlacklist() { + static const std::string FalsePositives = kadircet wrote: > sammccall wrote: > >

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

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D71199#2215719 , @baloghadamsoftware wrote: > Since no better idea cam to my mind, in this version I check whether > `modernize-use-d

[PATCH] D81272: [clang-tidy] New check `bugprone-redundant-branch-condition`

aaron.ballman added a comment. Thanks to the new info, I think the check basically LGTM. Can you add some negative tests and documentation wording to make it clear that the check doesn't currently handle all logically equivalent predicates, like: if (foo) { } else { if (!foo) { }

[PATCH] D85611: Improve dynamic AST matching diagnostics for conversion errors

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

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