[PATCH] D46951: [clang-tidy] misc-unused-parameters - retain old behavior under StrictMode

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM aside from some documentation nits. Comment at: docs/clang-tidy/checks/misc-unused-parameters.rst:6 -Finds unused parameters and fixes them, so that `-Wunused-parameter` can be -turned on. +Finds unused

[PATCH] D48617: [Builtins][Attributes][X86] Tag all X86 builtins with their required vector width. Add a min_vector_width function attribute and tag all x86 instrinsics with it.

aaron.ballman added a reviewer: aaron.ballman. aaron.ballman added inline comments. Comment at: include/clang/Basic/Attr.td:1949 + let Args = [UnsignedArgument<"VectorWidth">]; + let Subjects = SubjectList<[Function], ErrorDiag>; + let Documentation = [Undocumented]; -

[PATCH] D44143: [clang-tidy] Create properly seeded random generator check

aaron.ballman added inline comments. Comment at: clang-tidy/cert/ProperlySeededRandomGeneratorCheck.cpp:24 +: ClangTidyCheck(Name, Context), + RawDisallowedSeedTypes(Options.get("DisallowedSeedTypes", "")) { + StringRef(RawDisallowedSeedTypes).split(DisallowedSeedTypes,

[PATCH] D48717: [clang-tidy] fix PR36489 - respect deduced pointer types from auto as well

aaron.ballman requested changes to this revision. aaron.ballman added inline comments. This revision now requires changes to proceed. Comment at: test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic-pr36489.cpp:4 + +// Fix PR36489 and detect auto-deduced value correct

[PATCH] D48717: [clang-tidy] fix PR36489 - respect deduced pointer types from auto as well

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48717 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D44143: [clang-tidy] Create properly seeded random generator check

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM with a small documentation nit. Comment at: docs/clang-tidy/checks/cert-msc51-cpp.rst:39 + + A comma-separated list of the type names which are disallowe

[PATCH] D48412: [RISCV] Add support for interrupt attribute

aaron.ballman added inline comments. Comment at: lib/Sema/SemaDeclAttr.cpp:5305 + + if (hasFunctionProto(D) && getFunctionOrMethodNumParams(D) != 0) { +S.Diag(D->getLocation(), diag::warn_riscv_interrupt_attribute) << 0; I would have assumed this would be: `

[PATCH] D48734: [Sema] Consider all format_arg attributes.

aaron.ballman added inline comments. Comment at: lib/Sema/SemaChecking.cpp:5523 + StringLiteralCheckType CommonResult; + for (const FormatArgAttr *FA : ND->specific_attrs()) { const Expr *Arg = CE->getArg(FA->getFormatIdx().getASTIndex()); You

[PATCH] D48759: [ASTMatchers] add matcher for decltypeType and its underlyingType

aaron.ballman added a comment. Once this goes in, you can also update TrailingReturnTypeCheck.cpp to remove its local instance of this type matcher. Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:137 REGISTER_MATCHER(autoType); + REGISTER_MATCHER(decltypeType); REGIST

[PATCH] D48717: [clang-tidy] fix PR36489 - respect deduced pointer types from auto as well

aaron.ballman added a comment. In https://reviews.llvm.org/D48717#1147644, @JonasToth wrote: > - fix decltype deduction with new matcher > > I had to introduce a new matcher in clang for `DecltypeType` to reach its > underlying type. It would be better to have `hasType` resolve all these > is

[PATCH] D48412: [RISCV] Add support for interrupt attribute

aaron.ballman added inline comments. Comment at: lib/Sema/SemaDeclAttr.cpp:5305 + + if (hasFunctionProto(D) && getFunctionOrMethodNumParams(D) != 0) { +S.Diag(D->getLocation(), diag::warn_riscv_interrupt_attribute) << 0; apazos wrote: > aaron.ballman wrote:

[PATCH] D48617: [Builtins][Attributes][X86] Tag all X86 builtins with their required vector width. Add a min_vector_width function attribute and tag all x86 instrinsics with it.

aaron.ballman added inline comments. Comment at: test/Sema/attr-min-vector-width.c:8 + +void f3(void) __attribute__((__min_vector_width__(128), __min_vector_width__(256))); /* expected-warning {{attribute '__min_vector_width__' is already applied with different parameters}} */

[PATCH] D48617: [Builtins][Attributes][X86] Tag all X86 builtins with their required vector width. Add a min_vector_width function attribute and tag all x86 instrinsics with it.

aaron.ballman added inline comments. Comment at: include/clang/Basic/Attr.td:1949 + let Args = [UnsignedArgument<"VectorWidth">]; + let Subjects = SubjectList<[Function], ErrorDiag>; + let Documentation = [Undocumented]; craig.topper wrote: > aaron.ballman wro

[PATCH] D48617: [Builtins][Attributes][X86] Tag all X86 builtins with their required vector width. Add a min_vector_width function attribute and tag all x86 instrinsics with it.

aaron.ballman added inline comments. Comment at: include/clang/Basic/Attr.td:1949 + let Args = [UnsignedArgument<"VectorWidth">]; + let Subjects = SubjectList<[Function], ErrorDiag>; + let Documentation = [Undocumented]; chandlerc wrote: > aaron.ballman wrote:

[PATCH] D42682: [clang-tidy] Add io-functions-misused checker

aaron.ballman added inline comments. Comment at: clang-tidy/bugprone/BugproneTidyModule.cpp:84 +CheckFactories.registerCheck( +"bugprone-io-functions-misused"); CheckFactories.registerCheck( This name reads a bit awkwardly because usually "misuse

[PATCH] D44143: [clang-tidy] Create properly seeded random generator check

aaron.ballman added a comment. This still LGTM; do you need someone to commit on your behalf? https://reviews.llvm.org/D44143 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48734: [Sema] Consider all format_arg attributes.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rC Clang https://reviews.llvm.org/D48734 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.

[PATCH] D48617: [Builtins][Attributes][X86] Tag all X86 builtins with their required vector width. Add a min_vector_width function attribute and tag all x86 instrinsics with it.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Aside from a new round of minor doc nits, I think this is looking good. One remaining question I have is whether the attribute should diagnose an argument for a width that's not

[PATCH] D44143: [clang-tidy] Create properly seeded random generator check

aaron.ballman closed this revision. aaron.ballman added a comment. In https://reviews.llvm.org/D44143#1152550, @boga95 wrote: > How can I commit it? You need to have obtained commit access first. I went ahead and committed on your behalf in r336301. Thank you for the patch!

[PATCH] D48852: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %tu/%td on Darwin

aaron.ballman added a comment. > This is acceptable because Darwin guarantees that, despite the watchOS ABI > differences, sizeof(ptrdiff_t) == sizeof(NS[U]Integer) Can you describe these ABI differences please? Also, does Darwin guarantee that alignof(ptrdiff_t) == alignof(NS[U]Integer)? Rep

[PATCH] D48852: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %tu/%td on Darwin

aaron.ballman added a comment. In https://reviews.llvm.org/D48852#1153598, @arphaman wrote: > In https://reviews.llvm.org/D48852#1153415, @aaron.ballman wrote: > > > > This is acceptable because Darwin guarantees that, despite the watchOS > > > ABI differences, sizeof(ptrdiff_t) == sizeof(NS[U]I

[PATCH] D48412: [RISCV] Add support for interrupt attribute

aaron.ballman added inline comments. Comment at: test/Sema/riscv-interrupt-attr.c:23 + // expected-note {{repeated RISC-V 'interrupt' attribute is here}} +__attribute__((interrupt("user"))) void foo8() {} +__attribute__((inter

[PATCH] D48412: [RISCV] Add support for interrupt attribute

aaron.ballman added inline comments. Comment at: test/Sema/riscv-interrupt-attr.c:23 + // expected-note {{repeated RISC-V 'interrupt' attribute is here}} +__attribute__((interrupt("user"))) void foo8() {} +__attribute__((inter

[PATCH] D48412: [RISCV] Add support for interrupt attribute

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, or do you have commit privileges? https://reviews.llvm.org/D48412 ___ cfe-commits ma

[PATCH] D48845: [Sema] Add fixit for unused lambda captures

aaron.ballman added inline comments. Comment at: include/clang/Sema/DeclSpec.h:2552-2553 ParsedType InitCaptureType; +SourceLocation LocStart; +SourceLocation LocEnd; + How does `LocStart` relate to the existing source location `Loc`? I think this s

[PATCH] D49112: [Sema] Implement -Wmemset-transposed-args

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:662 +def note_suspicious_sizeof_memset_silence : Note< + "%select{parenthesize the third argument|cast the second argument to 'int'}0 to silence">; + erik.pilkingt

[PATCH] D49275: Thread safety: Run tests with both lock and capability attributes

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: rC Clang https://reviews.llvm.org/D49275 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D49045: PR15730/PR16986 Allow dependently typed vector_size types.

aaron.ballman added inline comments. Comment at: include/clang/AST/ASTContext.h:1327-1329 + QualType getDependentVectorType(QualType VectorType, Expr *SizeExpr, + SourceLocation AttrLoc, + VectorType::Ve

[PATCH] D49045: PR15730/PR16986 Allow dependently typed vector_size types.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, though it's your call on the const_cast stuff whether you want to revert or keep it. Comment at: include/clang/AST/Type.h:3105 +public: + Expr *getSizeE

[PATCH] D48845: [Sema] Add fixit for unused lambda captures

aaron.ballman added inline comments. Comment at: include/clang/Sema/DeclSpec.h:2552-2553 ParsedType InitCaptureType; +SourceLocation LocStart; +SourceLocation LocEnd; + aaron.ballman wrote: > How does `LocStart` relate to the existing source location

[PATCH] D49275: Thread safety: Run tests with both lock and capability attributes

aaron.ballman closed this revision. aaron.ballman added a comment. In https://reviews.llvm.org/D49275#1162204, @aaronpuchert wrote: > Thanks for the review. Could you commit this for me (`Aaron Puchert > `)? Happy to do so. I've commit in r337125. Thank you for the patch! Repository: rC Cl

[PATCH] D48845: [Sema] Add fixit for unused lambda captures

aaron.ballman accepted this revision. aaron.ballman added a comment. Aside from a small nit in the comments, LGTM. Comment at: include/clang/Sema/Sema.h:5608 + /// diagnostic is emitted. + bool DiagnoseUnusedLambdaCapture(const SourceRange CaptureRange, +

[PATCH] D49356: [clang-tidy: modernize] Fix modernize-use-equals-default with {} brackets list initialization: patch

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM with a formatting nit. Comment at: clang-tidy/modernize/UseEqualsDefaultCheck.cpp:100 AccessToFieldInParam, +

[PATCH] D49421: [CodeComplete] Fix accessibilty of protected members from base class.

aaron.ballman added a reviewer: rsmith. aaron.ballman added a comment. Adding Richard to see if he agrees with the direction taken. Comment at: lib/Sema/SemaAccess.cpp:1871-1873 +// The access should be AS_none as we don't know how the member was +// accessed - `Accesse

[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning

aaron.ballman added inline comments. Comment at: include/clang/AST/Decl.h:2212-2213 + bool isCpuDispatchMultiVersion() const; + bool isCpuSpecificMultiVersion() const; + aaron.ballman wrote: > Pedantic nit: CPU instead of Cpu? Thoughts on `isCPUDispatchMultiV

[PATCH] D49112: [Sema] Implement -Wmemset-transposed-args

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:7975-7976 +static void CheckMemsetSizeof(Sema &S, unsigned BId, const CallExpr *Call) { + if (BId != Builtin::BImemset) +return; + erik.pilkington wrote: > aaron.ballman wrote:

[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning

aaron.ballman added inline comments. Comment at: include/clang/Basic/Attr.td:851 + let Spellings = [Clang<"cpu_specific">]; + let Args = [VariadicIdentifierArgument<"Cpus">]; + let Subjects = SubjectList<[Function]>; erichkeane wrote: > aaron.ballman wrote: >

[PATCH] D49421: [CodeComplete] Fix accessibilty of protected members from base class.

aaron.ballman added inline comments. Comment at: lib/Sema/SemaAccess.cpp:1871-1873 +// The access should be AS_none as we don't know how the member was +// accessed - `AccessedEntity::getAccess` describes what access was used to +// access an entity.

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

aaron.ballman added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:23 + +const char DefaultIgnoredValues[] = "0;1;2;10;100;"; + Why 2, 10, and 100? Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:30-32 + for

[PATCH] D47474: Implement cpu_dispatch/cpu_specific Multiversioning

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Aside from an `assert` than can be removed, this LGTM on the attribute side of things. Comment at: lib/CodeGen/CodeGenModule.cpp:2446 + const auto *FD = cast(

[PATCH] D49114: [clang-tidy] Add a check for "magic numbers"

aaron.ballman added inline comments. Comment at: clang-tidy/readability/MagicNumbersCheck.cpp:23 + +const char DefaultIgnoredValues[] = "0;1;2;10;100;"; + lebedev.ri wrote: > aaron.ballman wrote: > > Why 2, 10, and 100? > These really should be a config option. T

[PATCH] D67159: [clang] New __attribute__((__clang_arm_mve_alias)).

aaron.ballman added inline comments. Comment at: clang/lib/AST/Decl.cpp:3107 +if (!ArmMveAliasValid(BuiltinID, getIdentifier()->getName())) { + getASTContext().getDiagnostics().Report( +getLocation(), diag::err_attribute_arm_mve_alias); simon_tat

[PATCH] D68377: [Builtins] Teach Clang about memccpy

aaron.ballman added inline comments. Comment at: include/clang/Basic/Builtins.def:483 BUILTIN(__builtin_memcpy, "v*v*vC*z", "nF") +BUILTIN(__builtin_memccpy, "v*v*vC*iz", "nF") BUILTIN(__builtin_memmove, "v*v*vC*z", "nF") xbolva00 wrote: > aaron.ballman wrote:

[PATCH] D67775: [Sema] Split out -Wformat-type-confusion from -Wformat-pedantic

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, thank you for this! Comment at: clang/test/Sema/format-strings-pedantic.c:1 -// RUN: %clang_cc1 -fsyntax-only -verify -Wformat -Wformat-pedantic -isystem

[PATCH] D68377: [Builtins] Teach Clang about memccpy

aaron.ballman added inline comments. Comment at: include/clang/Basic/Builtins.def:983 // POSIX string.h +LIBBUILTIN(memccpy, "v*v*vC*iz", "f", "string.h", ALL_GNU_LANGUAGES) LIBBUILTIN(stpcpy, "c*c*cC*", "f", "string.h", ALL_GNU_LANGUAGES) xbolva00

[PATCH] D68377: [Builtins] Teach Clang about memccpy

aaron.ballman added inline comments. Comment at: include/clang/Basic/Builtins.def:983 // POSIX string.h +LIBBUILTIN(memccpy, "v*v*vC*iz", "f", "string.h", ALL_GNU_LANGUAGES) LIBBUILTIN(stpcpy, "c*c*cC*", "f", "string.h", ALL_GNU_LANGUAGES) xbolva00

[PATCH] D68377: [Builtins] Teach Clang about memccpy

aaron.ballman added inline comments. Comment at: include/clang/Basic/Builtins.def:483 BUILTIN(__builtin_memcpy, "v*v*vC*z", "nF") +BUILTIN(__builtin_memccpy, "v*v*vC*iz", "nF") BUILTIN(__builtin_memmove, "v*v*vC*z", "nF") xbolva00 wrote: > aaron.ballman wrote:

[PATCH] D68640: Try to get readability-deleted-default.cpp to pass on Windows.

aaron.ballman added a comment. In D68640#1699563 , @gribozavr wrote: > It looks to me that a better fix is to fix the checker to not emit this > warning in MS compatibility mode. +1 > I'm OK with "fixing" the test like this, however, it should come wit

[PATCH] D68640: Try to get readability-deleted-default.cpp to pass on Windows.

aaron.ballman added a comment. In D68640#1699605 , @thakis wrote: > In D68640#1699563 , @gribozavr wrote: > > > It looks to me that a better fix is to fix the checker to not emit this > > warning in MS compatibilit

[PATCH] D68640: Try to get readability-deleted-default.cpp to pass on Windows.

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

[PATCH] D68694: Do not show "use of a signed integer operand with a binary bitwise operator" for positive integer operands

aaron.ballman added a reviewer: aaron.ballman. aaron.ballman added a comment. > In my opinion this check should be disabled in case of integer literals, > since there are a lot of existing code (even in system libraries) where user > can do nothing, e.g.: I think that this check should behave h

[PATCH] D68581: Include leading attributes in DeclStmt's SourceRange

aaron.ballman added a comment. I think this seems like reasonable behavior (for instance, we include the location of a storage class specifier already), but I am curious if @rsmith agrees. > Is there a good way to write a test for this? Yes, you can put an AST dumping test into the test\AST di

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

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

[PATCH] D68539: [clang-tidy] fix for readability-identifier-naming incorrectly fixes variables which become keywords

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:865 + IdentifierTable &Idents = Decl->getASTContext().Idents; + auto CheckNewIdentifier = Idents.find(Fixup); Can this be `const Ident

[PATCH] D67159: [clang] New __attribute__((__clang_arm_mve_alias)).

aaron.ballman marked an inline comment as done. aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/DiagnosticASTKinds.td:539 "packed attribute is unnecessary for %0">, InGroup, DefaultIgnore; + } Spurious newline? Co

[PATCH] D68581: Include leading attributes in DeclStmt's SourceRange

aaron.ballman added inline comments. Comment at: clang/test/AST/sourceranges.cpp:147 + +// CHECK-1Z: NamespaceDecl {{.*}} attributed_decl +namespace attributed_decl { I think these CHECK lines should just be a normal check instead of a C++17 check. CHANGES SIN

[PATCH] D67159: [clang] New __attribute__((__clang_arm_mve_alias)).

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

[PATCH] D68824: Fix __builtin_assume_aligned with too large values.

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:6067-6070 +// Alignment calculations can wrap around if it's greater than 2**28. +unsigned MaximumAlignment = +Context.getTargetInfo().getTriple().isOSBinFormatCOFF() ? 8192 +

[PATCH] D68913: Adds fixit hints to the Wrange-loop-analysis

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaStmt.cpp:2759 SemaRef.Context.getLValueReferenceType(E->getType().withConst()); -SemaRef.Diag(VD->getBeginLoc(), diag::note_use_type_or_non_reference) -<< NonReferenceType << NewReferenceType

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

aaron.ballman added a comment. In D55125#1704397 , @dkrupp wrote: > @aaron.ballman could you please commit? > I don't have commit access. Thx. I'm happy to do so, but the patch does not apply cleanly to trunk. Can you rebase? (Sorry for the delayed on

[PATCH] D68694: [clang-tidy] hicpp-signed-bitwise: Do not show "use of a signed integer operand with a binary bitwise operator" for positive integer operands

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM aside from a small nit. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:174 +- The :doc:`hicpp-signed-bitwise + ` now supports `IgnorePositiveIntegerLiterals` + option. `IgnorePosi

[PATCH] D68028: [clang] Add no_builtin attribute

aaron.ballman added a comment. In D68028#1707931 , @gchatelet wrote: > A gentle ping @aaron.ballman Sorry for the delay, I was traveling for last week. Comment at: clang/include/clang/Basic/AttrDocs.td:4402 +The ``__attribute__((no_bu

[PATCH] D68581: Include leading attributes in DeclStmt's SourceRange

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, assuming the requested test changes are as successful as I expect they will be (doesn't need additional review if the tests pass). Comment at: clang/test

[PATCH] D68028: [clang] Add no_builtin attribute

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:9508-9510 + // FIXME: We should really be doing this in SemaDeclAttr.cpp::handleNoBuiltin + // but there is a bug with FunctionDecl::isThisDeclarationADefinition() which + // always returns false befo

[PATCH] D68028: [clang] Add no_builtin attribute

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:9508-9510 + // FIXME: We should really be doing this in SemaDeclAttr.cpp::handleNoBuiltin + // but there is a bug with FunctionDecl::isThisDeclarationADefinition() which + // always returns false befo

[PATCH] D68028: [clang] Add no_builtin attribute

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:3416 + +def NoBuiltin : InheritableAttr { + let Spellings = [Clang<"no_builtin">]; I think we do not want this to be an inheritable attribute, but just `Attr` instead, because th

[PATCH] D68377: [Builtins] Teach Clang about memccpy

aaron.ballman added a comment. In D68377#1698399 , @xbolva00 wrote: > Current solution does not work > /home/xbolva00/LLVM/llvm/tools/clang/include/clang/Basic/Builtins.h:50:34: > error: redefinition of ‘BImemccpy’ > > #define BUILTIN(ID, TYPE, ATTRS)

[PATCH] D68028: [clang] Add no_builtin attribute

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:4407 +It accepts one or more strings corresponding to the name of the builtin +(e.g. "memcpy", "memset") or "*" which disables all builtins at once. + This mention of `*` is no

[PATCH] D66564: [clang-tidy] new performance struct pack align check

aaron.ballman added a comment. I sort of understand why this was asked to be put into `performance`, but I'm not convinced that's the right place to put it. Performance could be degraded by packing structures on some architectures depending on how the objects are accessed. I worry that people w

[PATCH] D68539: [clang-tidy] fix for readability-identifier-naming incorrectly fixes variables which become keywords

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-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:869-873 +if (Ident->isKeyword(getLangOpts()

[PATCH] D67545: [clang-tidy] Added DefaultOperatorNewCheck.

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM aside from a minor issue. Comment at: clang-tools-extra/clang-tidy/cert/DefaultOperatorNewAlignmentCheck.cpp:30-31 + // Check not applicable in C++17 or newer. + if (getLangOpts().CPlusPlus17 || getLan

[PATCH] D68913: Adds fixit hints to the Wrange-loop-analysis

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

[PATCH] D69225: Sema: Fixes a crash with a templated destructor

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

[PATCH] D69181: [clang-tidy] Adding misc-signal-terminated-thread check

aaron.ballman added a comment. Thank you for this! I don't think that the check should live in `misc`. It should have an alias in the `CERT` module, but maybe we want to have this live in a `posix` module? If not, this should probably live in `bugprone`. Comment at: clang-too

[PATCH] D69292: Proposal to add -Wtautological-compare to -Wall

aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. I agree with the changes and want to see this go in, but it needs a test case. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69292/new/ https://reviews.llvm.or

[PATCH] D69238: Fix clang-tidy readability-redundant-string-init for c++17/c++2a

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp:75 +// and if found, extend the SourceRange to start at 'Name'. +std::pair +CtorSourceRange(const MatchFinder::MatchResult &Result, You can dro

[PATCH] D68028: [clang] Add no_builtin attribute

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

[PATCH] D69388: [clang-tidy] Fix modernize-use-nodiscard check for classes marked as [[nodiscard]]

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Thank you for the patch, and welcome! LGTM aside from a minor nit. Do you need someone to commit this on your behalf? Comment at: clang-tidy/modernize/UseNodis

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

aaron.ballman added a comment. In D69145#1720254 , @poelmanc wrote: > What do @malcolm.parsons, @alexfh, @hokein, @aaron.ballman, @lebedev.ri think > of @mgehre's suggestion to enable `IgnoreBaseInCopyConstructors` as the > default setting, so gcc users

[PATCH] D69292: Proposal to add -Wtautological-compare to -Wall

aaron.ballman added a comment. In D69292#1722162 , @thakis wrote: > This is imho basic enough that it doesn't need a test. A test for this > doesn't add any value that I can see. The value comes from having an explicit test to demonstrate the behavior i

[PATCH] D69414: [clang-tidy] Regenerate clang-tidy check list 📋

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

[PATCH] D68539: [clang-tidy] fix for readability-identifier-naming incorrectly fixes variables which become keywords

aaron.ballman added a comment. Do you need someone to commit this on your behalf? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68539/new/ https://reviews.llvm.org/D68539 ___ cfe-commits mailing list c

[PATCH] D68912: Adds -Wrange-loop-analysis to -Wall

aaron.ballman added a comment. Does this analysis require CFG support or have a high false positive rate? We typically keep those out of `-Wall` because of performance concerns, so I am wondering if the diagnostic was kept out of `-Wall` for a reason. CHANGES SINCE LAST ACTION https://review

[PATCH] D68377: [Builtins] Teach Clang about memccpy

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a minor testing nit. Comment at: clang/test/CodeGen/memccpy-libcall.c:13 +// CHECK: attributes #2 = { nobuiltin } \ No newline at end of file -

[PATCH] D69478: [Sema] Allow warnStackExhausted to show more info

aaron.ballman added a comment. I sort of feel like we don't want to go this route because we want to avoid calling `runWithSufficientStackSpace` whenever possible, but I am not strongly opposed to this patch. It should be combined with the usage of the new functionality, however, so that it get

[PATCH] D68912: Adds -Wrange-loop-analysis to -Wall

aaron.ballman added a comment. In D68912#1723691 , @xbolva00 wrote: > >> Does this analysis require CFG support > > https://reviews.llvm.org/D69292 also requires CFG support. Yes, it does. > Generally, is CFG support ok for -Wall if the warning has few

[PATCH] D69292: Proposal to add -Wtautological-compare to -Wall

aaron.ballman added a comment. In D69292#1723720 , @xbolva00 wrote: > Just use > > // RUN: %clang_cc1 -fsyntax-only -verify -Wall %s > // RUN: %clang_cc1 -fsyntax-only -verify -Wtautological-bitwise-compare %s > > > in > > clang/test/SemaCXX/warn-bitwi

[PATCH] D69435: [clang-tidy] New checker performance-trivially-destructible-check

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/performance/TriviallyDestructibleCheck.cpp:27 + for (FieldDecl *Field : RecordDecl->fields()) { +const QualType FieldType = Field->getType(); +if (FieldType->isDependentType() || -

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

aaron.ballman added a comment. In D69764#2050226 , @steveire wrote: > I like the approach of using clang-format to implement this. It's much faster > than a `clang-tidy` approach. > > The broader C++ community has already chosen `East`/`West` and it has

[PATCH] D80371: [clang-tidy] Fix potential assert in use-noexcept check

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, but please add a test case for the changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80371/new/ https://reviews.llvm.or

[PATCH] D73037: Add a way to set traversal mode in clang-query

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

[PATCH] D66564: [clang-tidy] new altera struct pack align check

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Aside from a minor nit with the matcher, this LGTM! Comment at: clang-tools-extra/clang-tidy/altera/StructPackAlignCheck.cpp:50-53 + // Do not trigger on templ

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

aaron.ballman added a comment. In D77572#1981871 , @mgehre wrote: > Thanks for the comments so far. > I'm a bit lost now. Which changes that cannot wait for the next PR do you > see necessary to get this check merged? I'd be curious to know what @njame

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

aaron.ballman added a comment. Thank you for working on this check, I think it's useful functionality. One concern I have, though, is that it's not flow-sensitive and should probably be implemented as a clang static analyzer check instead of a clang-tidy check. For instance, consider these thre

[PATCH] D80371: [clang-tidy] Fix potential assert in use-noexcept check

aaron.ballman added a comment. In D80371#2052454 , @njames93 wrote: > In D80371#2052069 , @aaron.ballman > wrote: > > > LGTM, but please add a test case for the changes. > > > As this fix is preventing a crash in e

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

aaron.ballman added inline comments. Comment at: clang/lib/AST/Decl.cpp:3169-3175 } else { -if (!getIdentifier()) +const auto *Attr = getAttr(); + +if (!Attr) return 0; +BuiltinID = Attr->getID(); I think this is a bit more clear: ```

[PATCH] D80499: Remove obsolete ignore*() matcher uses

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

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

aaron.ballman added inline comments. Comment at: clang/lib/Format/EastWestConstFixer.cpp:139 + return (Tok->isSimpleTypeSpecifier() || + Tok->isOneOf(tok::kw_volatile, tok::kw_auto)); +} Do you need to look for `restrict` here as well as `volatile`? =

[PATCH] D80547: [clang-format] Fix an ObjC regression introduced with new [[likely]][[unlikely]] support in if/else clauses

aaron.ballman added a comment. In D80547#2054763 , @MyDeveloperDay wrote: > Handle more complex nested ObjC calls > Rename function to tryParseSimpleAttributes (not supporting full capability > as yet) > Use RAII object to reset the TokenPosition > ut

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

aaron.ballman added a comment. In D69764#2055716 , @MyDeveloperDay wrote: > I really do appreciate the reviews and the comments especially regarding > east/west/left/right/wrong ;-), I know there won't be 100% agreement, but I > think most people who ev

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