[PATCH] D80603: add isAtPosition narrowing matcher for parmVarDecl

aaron.ballman added a reviewer: aaron.ballman. aaron.ballman added a comment. Does the `hasParameter()` matcher not already do this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80603/new/ https://reviews.llvm.org/D80603 ___

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

aaron.ballman added a comment. In D69764#2056104 , @rsmith wrote: > I'm uncomfortable about `clang-format` performing this transformation at all. > Generally, clang-format only makes changes that are guaranteed to preserve > the meaning of the source pro

[PATCH] D80603: add isAtPosition narrowing matcher for parmVarDecl

aaron.ballman added a comment. In D80603#2057970 , @oontvoo wrote: > In D80603#2057014 , @aaron.ballman > wrote: > > > Does the `hasParameter()` matcher not already do this? > > > Yes, it does. But we'd like it to

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

aaron.ballman added a comment. In D69764#2058590 , @rsmith wrote: > In D69764#2058334 , @MyDeveloperDay > wrote: > > > @rsmith, Thank you for your comments, I don't agree, but thank you anyway. > > > > I will conti

[PATCH] D80603: add isAtPosition narrowing matcher for parmVarDecl

aaron.ballman added a comment. In D80603#2059122 , @gribozavr2 wrote: > In D80603#2058019 , @aaron.ballman > wrote: > > > I'm sorry if I'm being dense, but `hasParameter` traverses to the > > `ParmVarDecl`, so I'

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

aaron.ballman added a comment. In D80753#2061565 , @njames93 wrote: > It may be worth verifying that the fix-its are identical too, multiple > versions of a check could be running with differing options resulting in > different fix-its generated, in that

[PATCH] D80836: Support GCC [[gnu::attributes]] in C2x mode

aaron.ballman created this revision. aaron.ballman added reviewers: rsmith, erichkeane, dblaikie. Herald added a subscriber: krytarowski. GCC 10.1 introduced support for the `[[]]` style spelling of attributes in C mode. Similar to how GCC supports `__attribute__((foo))` as `[[gnu::foo]]` in C++

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

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp:106-107 +void PreferMemberInitializerCheck::registerMatchers(MatchFinder *Finder) { + if (!getLangOpts().CPlusPlus) +return; + T

[PATCH] D79437: [clang-tidy] Add fsetpos argument checker

aaron.ballman added a comment. In D79437#2052704 , @DerWaschbar wrote: > In D79437#2052109 , @aaron.ballman > wrote: > > > Have you considered writing a static analyzer check so you can do data and > > control flo

[PATCH] D77572: [clang-tidy] add new check readability-use-anyofallof

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, though please wait a bit for @njames93 to speak up if they still have concerns. Comment at: clang-tools-extra/clang-tidy/readability/UseAnyOfAllOfCheck.c

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

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D54408#2056251 , @steveire wrote: > @aaron.ballman I think we agreed in Belfast in November (after the most > recent comment) to get

[PATCH] D80631: [clang-tidy] RenamerClangTidyChecks ignore builtin and command line macros

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

[PATCH] D79437: [clang-tidy] Add fsetpos argument checker

aaron.ballman added a comment. btw, if you're interested in exploring a static analyzer solution for this, here's a recent review of an analyzer feature that's in the same general problem space: https://reviews.llvm.org/D80018 Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION h

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

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, but please wait for a few days in case @rsmith has further concerns. Comment at: clang/lib/Sema/SemaInit.cpp:8182 !ToPtrType->getPointeeTyp

[PATCH] D80836: Support GCC [[gnu::attributes]] in C2x mode

aaron.ballman updated this revision to Diff 267466. aaron.ballman marked an inline comment as done. aaron.ballman added a comment. Updated to remove the KnownToGCC tablegen bit. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80836/new/ https://reviews.llvm.org/D80836 Files: clang/incl

[PATCH] D80836: Support GCC [[gnu::attributes]] in C2x mode

aaron.ballman added inline comments. Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:87 Ret.emplace_back("CXX11", std::string(Name), "gnu", true); + if (Spelling->getValueAsBit("AllowInC")) +Ret.emplace_back("C2x", std::string(Name), "gnu", true); --

[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:21 +// FIXME: Add any more builtin variadics that shouldn't trigger this +static constexpr StringRef AllowedVariadics[] = { How would we know

[PATCH] D80879: clang-tidy and clang-query wont crash with invalid command line options

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, but can you add test coverage for this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80879/new/ https://reviews.llvm.org/D8

[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:21 +// FIXME: Add any more builtin variadics that shouldn't trigger this +static constexpr StringRef AllowedVariadics[] = { njames93 wrote: >

[PATCH] D80836: Support GCC [[gnu::attributes]] in C2x mode

aaron.ballman marked 3 inline comments as done. aaron.ballman added inline comments. Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:65 -bool Unset; -K = Spelling.getValueAsBitOrUnset("KnownToGCC", Unset); } erichkeane wrote: > aaron.ballman wrot

[PATCH] D80836: Support GCC [[gnu::attributes]] in C2x mode

aaron.ballman updated this revision to Diff 267603. aaron.ballman added a comment. New and improved with proper member initialization in all constructors. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80836/new/ https://reviews.llvm.org/D80836 Files: clang/include/clang/Basic/Attr.td

[PATCH] D80836: Support GCC [[gnu::attributes]] in C2x mode

aaron.ballman marked an inline comment as done. aaron.ballman added inline comments. Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:65 -bool Unset; -K = Spelling.getValueAsBitOrUnset("KnownToGCC", Unset); } erichkeane wrote: > Doesn't this resul

[PATCH] D80836: Support GCC [[gnu::attributes]] in C2x mode

aaron.ballman updated this revision to Diff 267604. aaron.ballman marked an inline comment as done. aaron.ballman added a comment. Whoops, *now* I'm properly setting the bit from the record. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80836/new/ https://reviews.llvm.org/D80836 Files:

[PATCH] D80836: Support GCC [[gnu::attributes]] in C2x mode

aaron.ballman added a comment. In D80836#2064181 , @erichkeane wrote: > 1 more question, how did you arrive at the 'not supported in C' list? Did > you find some GCC docs for that (if so, please put in the commit message)? > Or did you just try them al

[PATCH] D80836: Support GCC [[gnu::attributes]] in C2x mode

aaron.ballman updated this revision to Diff 267606. aaron.ballman added a comment. Sorry for the back and forth, this Monday morning is not easy it seems. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80836/new/ https://reviews.llvm.org/D80836 Files: clang/include/clang/Basic/Attr.td

[PATCH] D80836: Support GCC [[gnu::attributes]] in C2x mode

aaron.ballman closed this revision. aaron.ballman added a comment. Thank you for the review, I've commit in 522934da1f0c78c1de1a80d4ba14204a11f5afa8 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80836/new/ https://r

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

aaron.ballman added a comment. My experience with clang-tidy has been that template instantiations are a double-edged sword. The instantiation is the only place at which you have sufficient information to perform many kinds of analyses, but it's also often not plausible to modify the templated

[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:21 +// FIXME: Add any more builtin variadics that shouldn't trigger this +static constexpr StringRef AllowedVariadics[] = { njames93 wrote: >

[PATCH] D80877: [clang-tidy] Added MacroDefiniton docs for readability-identifier-naming

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! We can add the `ObjCIVar` docs in a future patch if someone has the expertise there. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[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", I think we may want to keep this one as it's

[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:40 +// "__builtin_va_start", +// "__builtin_stdarg_start", +"__builtin_assume_aligned", // Documented as variadic to support overloading --

[PATCH] D81087: Replaced C++2a with C++20 in clang-tools-extra

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

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

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM aside from a small nit. Comment at: clang/lib/Sema/SemaType.cpp:7698 +llvm::APSInt &Result) { + Expr *AttrExpr = static_cast(Attr.getArgAsExpr(0)); + if (AttrExpr->is

[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/AST/Type.h:1928 + /// Determines if this is vector-length sized typed (VLST), i.e. a + /// sizeless type with the 'arm_sve_vector_bits(N)' attribute applied. ... this is a vector-length-size

[PATCH] D83174: Teach AttachPreviousImpl to inherit MSInheritanceAttr attribute

aaron.ballman added a comment. In D83174#2155143 , @gargvaibhav64 wrote: > The tests weren't failing for me. So, we are possibly missing test coverage. > I have made the change now. Thanks for making the change. > Also, I would like to add that the cu

[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; davidstone wrote: > aaron.ballman wrote: > > riccibruno wrote: > > > riccibruno wrote: > > > > davidstone wrote: > >

[PATCH] D84005: Introduce ns_error_domain attribute.

aaron.ballman added a comment. FWIW, this is *really* hard to review because it's not a diff against the trunk and so it's not immediately clear what the actual changes are. The change is missing all of its test coverage. Comment at: clang/include/clang/Basic/Attr.td:1860 +de

[PATCH] D84005: Introduce ns_error_domain attribute.

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9450 +def err_nserrordomain_not_tagdecl : Error< + "ns_error_domain attribute only valid on " + "%select{enums, structs, and unions|enums, structs, unions, and classes}0">; ---

[PATCH] D84005: Introduce ns_error_domain attribute.

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1860 +def NSErrorDomain : Attr { + let Spellings = [GNU<"ns_error_domain">]; + let Args = [IdentifierArgument<"ErrorDomain">]; gribozavr2 wrote: > aaron.ballman wrote: > > MForst

[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: > aaron.ballman wrote: > > c-rhodes wrote: > > > c-rhodes wrote: > > > > aaron.ballman wrote: >

[PATCH] D83174: Teach AttachPreviousImpl to inherit MSInheritanceAttr attribute

aaron.ballman added reviewers: aaron.ballman, riccibruno, martong. aaron.ballman added a comment. Herald added a subscriber: rnkovacs. In D83174#2156454 , @v.g.vassilev wrote: > >> Also, I would like to add that the current test present in this diff does

[PATCH] D83901: [clang] Disable a few formatting options for test/

aaron.ballman accepted this revision. aaron.ballman added reviewers: rsmith, dblaikie. aaron.ballman added a comment. This revision is now accepted and ready to land. I think this is a good improvement, but you should wait for a few days in case other reviewers have feedback. Repository: rG L

[PATCH] D83680: [clang-tidy] Refactor IncludeInserter

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/IncludeInserter.cpp:55 IncludeSorter &IncludeInserter::getOrCreate(FileID FileID) { + assert(SourceMgr && "SourceMgr shouldn't be null"); // std::unique_ptr is cheap to construct, so force a

[PATCH] D83680: [clang-tidy] Refactor 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/D83680/new/ https://reviews.llvm.org/D83680

[PATCH] D84005: Introduce ns_error_domain attribute.

aaron.ballman added a comment. It's a bit odd that this attribute has an AST node created for it but nothing is using that AST node elsewhere in the project. Are there other patches expected for making use of this attribute? Comment at: clang/include/clang/Basic/AttrDocs.td:3

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

aaron.ballman added a comment. I like the way the code is shaping up with the slightly altered design, nice! Comment at: clang/lib/AST/ASTContext.cpp:1872 +unsigned getSveVectorWidth(const Type *T) { + // Get the vector size from the 'arm_sve_vector_bits' attribute via the -

[PATCH] D84213: [clang-tools-extra] Disable -Wsuggest-override for unittests/

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

[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] D84048: DR2303: Prefer 'nearer' base classes during template deduction.

aaron.ballman removed a reviewer: aaron.ballman. aaron.ballman added a comment. While this looks reasonable to me, I don't feel comfortable signing off on it because Templates Are Complicated (switching myself to a subscriber instead). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84048

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

aaron.ballman added a comment. LGTM, though I have a possible code simplification if you think it's an improvement. Comment at: clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp:78 + + if (const auto *DeclRef = Result.Nodes.getNodeAs("expr")) { +

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

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. The attribute bits LGTM, thanks! Comment at: clang/lib/AST/ASTContext.cpp:1887 + +unsigned getSvePredWidth(const Type *T) { return getSveVectorWidth(T) / 8; } +

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

aaron.ballman added a comment. I reviewed the changes in the patch and they seem reasonable, but this patch is hard to have high confidence in because you need to audit all the places where the default behavior silently changed and no changes were made in the patch. I'm assuming that the code c

[PATCH] D84414: [RISCV] Support Shadow Call Stack

aaron.ballman added inline comments. Comment at: clang/test/CodeGen/shadowcallstack-attr.c:8 +// RUN: %clang_cc1 -triple riscv32-linux-gnu -emit-llvm -o - %s -fsanitize=shadow-call-stack | FileCheck -check-prefix=UNBLACKLISTED %s + Now might be a good opportun

[PATCH] D84005: Introduce ns_error_domain attribute.

aaron.ballman added a comment. In D84005#2171962 , @MForster wrote: > In D84005#2162433 , @aaron.ballman > wrote: > > > It's a bit odd that this attribute has an AST node created for it but > > nothing is using th

[PATCH] D83174: Teach AttachPreviousImpl to inherit MSInheritanceAttr attribute

aaron.ballman added a comment. In D83174#2174294 , @gargvaibhav64 wrote: > Hi everyone, are there any more changes required on this review? FWIW, I'm holding my LG until there's a test case which covers the changes. CHANGES SINCE LAST ACTION https:/

[PATCH] D84591: [clang-tidy][NFC] Replace comment by private method

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

[PATCH] D84591: [clang-tidy][NFC] Replace comment by private method

aaron.ballman added a comment. In D84591#2174442 , @hanneskaeufler wrote: > In D84591#2174437 , @aaron.ballman > wrote: > > > LGTM! > > > Awesome, thanks! Do you need someone to commit on your behalf? If so, can

[PATCH] D84005: Introduce ns_error_domain attribute.

aaron.ballman added a comment. In D84005#2172747 , @doug.gregor wrote: > In D84005#2171982 , @MForster wrote: > > > @milseman, @doug.gregor, could you please help with the open questions on > > this review? > > > >

[PATCH] D84591: [clang-tidy][NFC] Replace comment by private method

aaron.ballman closed this revision. aaron.ballman added a comment. In D84591#2174473 , @hanneskaeufler wrote: > In D84591#2174447 , @aaron.ballman > wrote: > > > In D84591#2174442

[PATCH] D84005: Introduce ns_error_domain attribute.

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5328 + if (!IdentLoc || !IdentLoc->Ident) { +// Try to locate the argument directly +SourceLocation Loc = AL.getLoc(); Comments should be grammatical including punctuation (

[PATCH] D84005: Introduce ns_error_domain attribute.

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5350 + + D->addAttr(::new (S.Context) + NSErrorDomainAttr(S.Context, AL, IdentLoc->Ident)); MForster wrote: > aaron.ballman wrote: > > Shouldn't we also be validatin

[PATCH] D84656: [clang] Pass the NamedDecl* instead of the DeclarationName into many diagnostics.

aaron.ballman accepted this revision. aaron.ballman added a comment. The changes LGTM but it seems like there may be some formatting issues with the patch (or the lint tool is acting up). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84656/new/ ht

[PATCH] D84658: [clang] Overload NamedDecl::printName to provide a user-friendly name for unnamed entities

aaron.ballman added a comment. I think these changes are going in the right direction (and I agree with the feedback from Erich). Thank you for looking into this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84658/new/ https://reviews.llvm.org/D8

[PATCH] D84603: Thread safety analysis: More consistent warning message

aaron.ballman added inline comments. Comment at: clang/lib/Analysis/ThreadSafety.cpp:1644 if (!LDat) { - Analyzer->Handler.handleMutexNotHeld("", D, POK, Cp.toString(), - LK_Shared, Loc); + Analyzer->Handler.handleMutexNotH

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

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1465 + let Spellings = [Clang<"msp430_builtin">]; + let Documentation = [Undocumented]; +} No new, undocumented attributes, please. Or is this attribute not expected to be used

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

aaron.ballman added a comment. In D84602#2176584 , @rjmccall wrote: > I don't have a problem with introducing a new convention even if it's only > used for "builtin" functions. To be clear, I also don't have a problem with it, but if users aren't suppos

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

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1465 + let Spellings = [Clang<"msp430_builtin">]; + let Documentation = [Undocumented]; +} atrosinenko wrote: > aaron.ballman wrote: > > No new, undocumented attributes, please.

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

aaron.ballman added a comment. In D84602#2178328 , @atrosinenko wrote: > In D84602#2176592 , @aaron.ballman > wrote: > >> To be clear, I also don't have a problem with it, but if users aren't >> supposed to be wri

[PATCH] D82898: [clang-tidy] Handled insertion only fixits when determining conflicts.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM with a few style nits. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:629 Priority = std::make_tuple(Begin, Type, -End,

[PATCH] D84658: [clang] Overload NamedDecl::printName to provide a user-friendly name for unnamed entities

aaron.ballman added inline comments. Comment at: clang/include/clang/AST/PrettyPrinter.h:78 + char getOpenDelimiter() const { return MSVCFormatting ? '`' : '('; } + char getCloseDelimiter() const { return MSVCFormatting ? '\'' : ')'; } These names a bit too g

[PATCH] D84202: [clang][noderef] Enable -Wnoderef warning for CXX C-style casts

aaron.ballman added a comment. > users can still disable the warning on line granularity with pragmas. This makes me a bit uncomfortable because those pragmas extremely ugly (and not easily portable). Also, this will break code for users who were previously doing something that was explicitly a

[PATCH] D84202: [clang][noderef] Enable -Wnoderef warning for CXX C-style casts

aaron.ballman added a comment. In D84202#2179718 , @leonardchan wrote: > In D84202#2179618 , @aaron.ballman > wrote: > >>> users can still disable the warning on line granularity with pragmas. >> >> This makes me a

[PATCH] D82898: [clang-tidy] Handled insertion only fixits when determining conflicts.

aaron.ballman accepted this revision. aaron.ballman added a comment. LG! Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:703 + Apply[Event.ErrorId] = false; +continue; + case Event::ET_Insert: njames93 wrote: > aar

[PATCH] D84658: [clang] Overload NamedDecl::printName to provide a user-friendly name for unnamed entities

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

[PATCH] D84678: [clang] False line number in a function definition with "void" parameter

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaType.cpp:5112 if (FTI.NumParams != 1 || FTI.isVariadic) { - S.Diag(DeclType.Loc, diag::err_void_only_param); + S.Diag(FTI.Params[i].IdentLoc, diag::err_void_only_param);

[PATCH] D84678: [clang] False line number in a function definition with "void" parameter

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM with a small nit about the tests, though I'm still surprised `IdentLoc` is valid even when there's no identifier present. :-) Comment at: clang/test/Sema/

[PATCH] D84005: Introduce ns_error_domain attribute.

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM aside from an almost NFC simplification. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:5350 + + D->addAttr(::new (S.Context) + NSErrorDomainAttr(S.Context, AL, IdentLoc->Ident)); --

[PATCH] D70265: [clang-tidy] Add CppCoreGuidelines I.2 "Avoid non-const global variables" check

aaron.ballman added a comment. In D70265#1954925 , @vingeldal wrote: > After looking more closely at the code I think the issue is within > hasLocalStorage() which is called in hasGlobalStorage(). My expectation would > be that anything inside of functio

[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] D77085: [clang-tidy] Added support for validating configuration options

aaron.ballman added a comment. Thank you for working on this, I think it's going to be a very useful interface! Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp:25 + llvm::raw_svector_ostream Output(Buffer); + Output << "warning: Option not found '" << OptionName <

[PATCH] D77199: [clang-tidy] Address false positive in modernize-use-default-member-init

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/test/clang-tidy/checkers/modernize-use-default-member-init.cpp:436 + +struct PR45363 { + // Ensure

[PATCH] D76990: [clang-tidy]: fix false positive of cert-oop54-cpp check.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM with a testing request. I agree that the Clang AST is a bit surprising, but not so surprising that I could definitely call it a bug. Comment at: clang-to

[PATCH] D76818: [clang-tidy] Add check llvmlibc-implementation-in-namespace.

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/llvmlibc/ImplementationInNamespaceCheck.cpp:21 + Finder->addMatcher( + decl(hasParent(translationUnitDecl()), unless(linkageSpecDecl())) + .bind("child_of_translation_unit"), ---

[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] D77085: [clang-tidy] Added support for validating configuration options

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp:25 + llvm::raw_svector_ostream Output(Buffer); + Output << "warning: Option not found '" << OptionName << "'\n"; + return std::string(Buffer); aaron.ballman wrote:

[PATCH] D77532: Fix a typo in clang/lib/Frontend/FrontendAction.cpp

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Do you need someone to commit on your behalf? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77532/new/ https://reviews.llvm.

[PATCH] D77499: [ASTMatchers] Matchers that take enumerations args provide hints with invalid arguments

aaron.ballman added a reviewer: aaron.ballman. aaron.ballman added a comment. Thank you for working on this, I think it's very useful functionality! I'd appreciate seeing some unit test coverage for these changes. Comment at: clang/lib/ASTMatchers/Dynamic/CMakeLists.txt:17

[PATCH] D77532: Fix a typo in clang/lib/Frontend/FrontendAction.cpp

aaron.ballman closed this revision. aaron.ballman added a comment. Thank you for the patch! I've committed on your behalf in 2aa593be5486a9c5d3092647f2576273f84f3159 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D76818: [clang-tidy] Add check llvmlibc-implementation-in-namespace.

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/llvmlibc/ImplementationInNamespaceCheck.cpp:21 + Finder->addMatcher( + decl(hasParent(translationUnitDecl(

[PATCH] D77499: [ASTMatchers] Matchers that take enumerations args provide hints with invalid arguments

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

[PATCH] D77586: Allow parameter names to be elided in a function definition in C

aaron.ballman created this revision. aaron.ballman added reviewers: rsmith, rjmccall, erik.pilkington. Herald added a subscriber: dexonsmith. WG14 has adopted N2480 (http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2480.pdf) into C2x at the meetings last week, allowing parameter names of a funct

[PATCH] D71142: [Sema] Validate large bitfields

aaron.ballman added a comment. In D71142#1967315 , @xbolva00 wrote: > Any comments? > > @rsmith @aaron.ballman There are outstanding review comments, so I'm waiting for the patch author to address those, unless I've missed a question somewhere? Reposi

[PATCH] D77085: [clang-tidy] Added support for validating configuration options

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM with a few minor nits. Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp:117 + llvm::Expected ValueOr = get(LocalName); + if (ValueOr) { +r

[PATCH] D76606: [clang-tidy] Change checks that take enum configurations to use a new access method.

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

[PATCH] D77586: Allow parameter names to be elided in a function definition in C

aaron.ballman closed this revision. aaron.ballman marked an inline comment as done. aaron.ballman added a comment. I've committed in 86b5eabfeab164dcb680f6690e7718e3d21ceeb5 Comment at: clang/include/clang/B

[PATCH] D77701: [Sema] refactor static functions into private methods NFC

aaron.ballman added a reviewer: rsmith. aaron.ballman added a comment. OTOH, this is reasonable, NFC, and I tend to agree about it being a code smell. OTOH, this makes parsing Sema.h that much slower and adds even more text for us to wade through in that header file. Given that the approaches ar

[PATCH] D77680: [clang-tidy] misc-unused-parameters: Don't remove parameter from lambda

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM as a temporary workaround until we find a better solution. (I just noticed that we don't need a similar fix for blocks because this check doesn't seem to support blocks: htt

[PATCH] D77571: [clang-tidy] Add check to find calls to NSInvocation methods under ARC that don't have proper object argument lifetimes.

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM aside from the file names concern. Comment at: clang-tools-extra/clang-tidy/objc/CMakeLists.txt:8 MissingHashCheck.cpp + NsinvocationArgumentLifetimeCheck.cpp ObjCTidyModule.cpp I

[PATCH] D77701: [Sema] refactor static functions into private methods NFC

aaron.ballman added a comment. In D77701#1969687 , @nickdesaulniers wrote: > In D77701#1969132 , @aaron.ballman > wrote: > > > OTOH, this is reasonable, NFC, and I tend to agree about it being a code > > smell. >

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

aaron.ballman added a comment. In D75844#1951915 , @tbaeder wrote: > Sorry for taking so long but it seems like I've went down a rabbit hole a > bit. My previous patch sets the range in `parseGNUAttributes()` > unconditionally, but that seems to trigger

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