[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:117-118 +FunctionCallCollector Collector{[&CalledFunctions](const CallExpr *CE) { + if (isa(CE->getCalleeDecl())) +CalledFunctions.push_back(CE); +}}; --

[PATCH] D88645: [Annotation] Allows annotation to carry some additional constant arguments.

aaron.ballman added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2338 + Exprs = Exprs.drop_front(); + if (Exprs.size() == 0) +return llvm::ConstantPointerNull::get(Int8PtrTy); `Exprs.empty()` would be more concise. Com

[PATCH] D69560: [clang-tidy] Add 'experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type' check

aaron.ballman added a comment. In D69560#2292505 , @whisperity wrote: > I'd like to resurrect the discussion about this. Unfortunately, the concept > of `experimental-` group (in D76545 ) has > fallen silent, too. We will

[PATCH] D88088: [clang] improve accuracy of ExprMutAnalyzer

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM though I may have spotted a potential improvement (it can be handled in a follow-up if you want). Comment at: clang/lib/Analysis/ExprMutationAnalyzer.cpp:61 + // below. + auto const ConditionalOperator

[PATCH] D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt:37 + clangLex + clangSerialization clangTidy Why do serialization and lex need to be pulled in? Comment at: clang-tools-extra/c

[PATCH] D87279: [clang] Fix handling of physical registers in inline assembly operands.

aaron.ballman added inline comments. Comment at: clang/lib/CodeGen/CGStmt.cpp:2095 +if (!GCCReg.empty() && !PhysRegOutputs.insert(GCCReg).second) + CGM.Error(S.getAsmLoc(), "Multiple outputs to hard register: " + GCCReg); + Diagnostics in Clang are typic

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

aaron.ballman added a comment. In D88833#2319118 , @fiesh wrote: > @aaron.ballman valid point. It would seem natural to diagnose both since the > user is voluntarily causing the decay? I'm on the fence. If the argument to the function is one that's defi

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

aaron.ballman added a comment. In D88833#2319799 , @lebedev.ri wrote: > I think the current behavior should instead be configurable, with a way to > opt-in into weaker guarantees. I think that's a good idea, thank you! Repository: rG LLVM Github Mono

[PATCH] D88314: Added llvm-string-referencing check

aaron.ballman added a comment. Thank you for working on this check! Have you run the check over LLVM to see how much it reports? One of the concerns I have with this is that it's not always appropriate to replace a `const std::string&` with an `llvm::StringRef` and so I'm wondering what the "fa

[PATCH] D87279: [clang] Fix handling of physical registers in inline assembly operands.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. This LGTM, but I'm not super well-versed with asm statements to begin with. You should wait a bit for other reviewers to weigh in before landing. CHANGES SINCE LAST ACTION htt

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:117-118 +FunctionCallCollector Collector{[&CalledFunctions](const CallExpr *CE) { + if (isa(CE->getCalleeDecl())) +CalledFunctions.push_back(CE); +}}; --

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:117-118 +FunctionCallCollector Collector{[&CalledFunctions](const CallExpr *CE) { + if (isa(CE->getCalleeDecl())) +CalledFunctions.push_back(CE); +}}; --

[PATCH] D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp:46 + +void ConstCorrectnessCheck::registerMatchers(MatchFinder *Finder) { + const auto ConstType = hasType(isConstQualified()); JonasToth w

[PATCH] D88314: Added llvm-string-referencing check

aaron.ballman added a comment. It looks like you generated a diff from a previous diff instead of trunk -- can you regenerate the diff against trunk so that reviewers can see the full content of the changes? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D88645: [Annotation] Allows annotation to carry some additional constant arguments.

aaron.ballman added a comment. In D88645#2321980 , @jdoerfert wrote: > (Drive By: This is cool) I didn't say this before, but yeah, I agree -- this is really quite neat, thank you for working on it! Comment at: clang/include/clang/Sem

[PATCH] D88314: Added llvm-string-referencing check

aaron.ballman added a comment. In D88314#2322870 , @bogser01 wrote: > @aaron.ballman Thank you for picking up this review! Running the check over > the entire LLVM causes ~74K warnings across 430 files. As to the false > positive rate it's tricky to meas

[PATCH] D67422: [analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis.

aaron.ballman added inline comments. Comment at: clang/lib/Analysis/SarifPathDiagnosticConsumer.cpp:307 .Case(FULLNAME, HELPTEXT) #include "clang/StaticAnalyzer/Checkers/Checkers.inc" #undef CHECKER Szelethus wrote: > NoQ wrote: > > Szelethus wrote: > > > No

[PATCH] D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp:46 + +void ConstCorrectnessCheck::registerMatchers(MatchFinder *Finder) { + const auto ConstType = hasType(isConstQualified()); JonasToth w

[PATCH] D89210: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in SwitchStmt

aaron.ballman added a comment. Thank you for the continued work on this feature! I'd like to better understand the behavior of fallthrough labels because I think there are two conceptual models we could follow. When a label falls through to another label, should both labels be treated as having

[PATCH] D89212: PR47663: Warn if an entity becomes weak after its first use.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from some very small nits (feel free to ignore any that don't make sense to you). Comment at: clang/lib/AST/DeclBase.cpp:622 +AvailabilityResult A

[PATCH] D89210: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in SwitchStmt

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a documentation request, but you may want to wait a few days before committing in case Richard or John have opinions. In D89210#2330466

[PATCH] D89651: [clang-tidy] Add bugprone-suspicious-memory-comparison check

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

[PATCH] D91872: [libTooling] Update Transformer's `node` combinator to include the trailing semicolon for decls.

aaron.ballman added a comment. Drive-by question from the peanut gallery, sorry if this is an ignorant one -- not all declarations have a trailing semicolon; is that handled properly? e.g., `int x;` has a trailing semicolon but `int x, y;` only has a trailing semicolon for one of the two declar

[PATCH] D91872: [libTooling] Update Transformer's `node` combinator to include the trailing semicolon for decls.

aaron.ballman added a comment. In D91872#2408321 , @ymandel wrote: > In D91872#2408278 , @aaron.ballman > wrote: > >> Drive-by question from the peanut gallery, sorry if this is an ignorant one >> -- not all decla

[PATCH] D91828: [Sema/Attribute] Ignore noderef attribute in unevaluated context

aaron.ballman added inline comments. Comment at: clang/test/Frontend/noderef.c:71 + x = sizeof(s->a + (s->b)); // ok + // Nested struct access Can you add tests for the weird situations where the expression actually is evaluated? e.g., ``` struct S { virtu

[PATCH] D91565: Guard init_priority attribute within libc++

aaron.ballman added inline comments. Comment at: clang/test/SemaCXX/init-priority-attr.cpp:26 Two foo __attribute__((init_priority(101))) ( 5, 6 ); +#if defined(__MVS__) + #if defined(SYSTEM) Rather than using the preprocessor, you can assign a prefix to be chec

[PATCH] D91565: Guard init_priority attribute within libc++

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM, thank you for the fixes! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91565/new/ https://reviews.llvm.org/D91565 ___ cfe-commit

[PATCH] D89743: Support Attr in DynTypedNode and ASTMatchers.

aaron.ballman added a comment. In D89743#2406017 , @sammccall wrote: > In D89743#2363201 , @aaron.ballman > wrote: > >> In D89743#2360258 , @sammccall >> wrote: >> >>> (whi

[PATCH] D91828: [Sema/Attribute] Ignore noderef attribute in unevaluated context

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

[PATCH] D91828: [Sema/Attribute] Ignore noderef attribute in unevaluated context

aaron.ballman added a comment. In D91828#2411203 , @thejh wrote: > @aaron.ballman Can you land it for me? I don't have commit access. Happy to do so -- are you okay with "Jann Horn " for author attribution? Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D91828: [Sema/Attribute] Ignore noderef attribute in unevaluated context

aaron.ballman added a comment. In D91828#2411210 , @thejh wrote: > In D91828#2411207 , @aaron.ballman > wrote: > >> In D91828#2411203 , @thejh wrote: >> >>> @aaron.ballman C

[PATCH] D89743: Support Attr in DynTypedNode and ASTMatchers.

aaron.ballman edited subscribers, added: ymandel; removed: aaron.ballman. aaron.ballman added a comment. In D89743#2409115 , @sammccall wrote: > In D89743#2409001 , @sammccall wrote: > >> We didn't talk about overlo

[PATCH] D91917: Update mode used in traverse() examples

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

[PATCH] D91916: Remove automatic traversal from forEach matcher

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

[PATCH] D91918: Remove the IgnoreImplicitCastsAndParentheses traversal kind

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a documentation request, thank you for this cleanup! Comment at: clang/docs/ReleaseNotes.rst:237 +- The TK_IgnoreImplicitCastsAndParentheses t

[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:616-619 + if (CRD->isStruct() && !isHungarianNotationOptionEnabled("TreatStructAsClass", + HNOption.Gen

[PATCH] D91915: [clang-tidy] Fix RenamerClangTidy checks trying to emit a fix that isnt a valid identifier

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

[PATCH] D90282: [clang-tidy] Add IgnoreShortNames config to identifier naming checks

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, but please wait for @njames93 in case they have additional feedback. Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h:64 +

[PATCH] D91055: [clang-tidy] Introduce misc No Integer To Pointer Cast check

aaron.ballman added a comment. In D91055#2410447 , @lebedev.ri wrote: > Ping. > If there's no pushback within next 24h i will commit this and wait for > post-commit review (if any). > Thanks. FWIW, that's not the way we usually do things, so please don't

[PATCH] D91898: [attributes] Add a facility for defining and enforcing a Trusted Computing Base.

aaron.ballman added reviewers: aaronpuchert, delesley. aaron.ballman added a comment. This feels an awful lot like a set of attributes we already have -- can capability attributes be used for this instead? https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#basic-concepts-capabilities The doc

[PATCH] D90188: Add support for attribute 'using_if_exists'

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:442 +// Make the selection of the recovery decl deterministic. +(RealRes)->getLocation().getRawEncoding() < IIDecl->getLocation().getRawEncoding())

[PATCH] D91898: [attributes] Add a facility for defining and enforcing a Trusted Computing Base.

aaron.ballman added a comment. In D91898#2413004 , @NoQ wrote: >> if the TCB-based functions are a small subset of the code then this >> attribute is better > > Yes, that's exactly the case. We want most functions to be untrusted by > default but also ha

[PATCH] D91975: [clang-tidy] cppcoreguidelines Narrowing Conversions Check: detect narrowing conversions involving typedefs

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from some small nits, thank you for the fix! Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-narrowing-conversions.cpp:346

[PATCH] D90282: [clang-tidy] Support IgnoredRegexp configuration to selectively suppress identifier naming checks

aaron.ballman added a comment. In D90282#2415189 , @smhc wrote: > There's a build failure from this merge, looks like a typo: > > diff --git > a/clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst > b/clang-tools-extra/docs/clang-t

[PATCH] D92355: [clang] add a `swift_async_name` attribute

aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. There's no information in either the attribute definition in Attr.td or in the documentation as to what subject this attribute applies to. Comment at

[PATCH] D92354: [clang] add a new `swift_attr` attribute

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:2153 +def SwiftAttr : InheritableAttr { + let Spellings = [Clang<"swift_attr">]; + let Args = [StringArgument<"Attribute">]; The other swift attributes use a GNU spelling and don

[PATCH] D92001: [ubsan] Fix crash on __builtin_assume_aligned

aaron.ballman added inline comments. Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:2524 // defined. - if (Ty->getPointeeType().isVolatileQualified()) + if (!Ty->getPointeeType().isNull() && Ty->getPointeeType().isVolatileQualified()) return; lebede

[PATCH] D92577: Don't reject tag declarations in for loop clause-1

aaron.ballman created this revision. aaron.ballman added reviewers: rsmith, rjmccall. aaron.ballman requested review of this revision. We currently reject this valid C construct by claiming it declares a non-local variable: `for (struct { int i; } s={0}; s.i != 0; s.i--) ;` The problem is that w

[PATCH] D92439: [CLANG] Fix missing error for use of 128-bit integer inside SPIR64 device code.

aaron.ballman accepted this revision. aaron.ballman added a comment. In D92439#2431256 , @jdoerfert wrote: > In D92439#2429815 , @jyu2 wrote: > >> In D92439#2429511 , @jdoerf

[PATCH] D92495: [clang] Add a new nullability annotation for swift async: _Nullable_result

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:3516 +``_Nullable_result`` pointer can be ``nil``, just like ``_Nullable``. Where this +attribute differs from ``_Nullable`` is when its used on a parameter to a +completion handler in a Swift

[PATCH] D91885: [clang-tidy] Add support for diagnostics with no location

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.h:183 + + /// Adds a diagnostic to report errors in the checks configuration. + DiagnosticBuilder checks -> check's Comment at: clang-tools-extra/cla

[PATCH] D89743: Support Attr in DynTypedNode and ASTMatchers.

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM aside from a minor testing request. Comment at: clang/lib/AST/ASTTypeTraits.cpp:138 +return ASTNodeKind(NKI_##A##Attr); +#include "clang/Basic/AttrList.inc" + } ymandel wrote: > aaro

[PATCH] D92384: [AST][NFC] Silence GCC warning about broken strict aliasing rules

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

[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Thank you for the new functionality! You may want to wait a bit before landing to see if @njames93 has any remaining comments. Repository: rG LLVM Github Monorepo CHANG

[PATCH] D92108: Fix inconsistent availability attribute message string literal check.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. I think this is a good change but would point out that the diagnostic message is pretty confusing as-is. `u8"a"` is a string literal, so saying that a string literal is expected

[PATCH] D91630: [Parse] Add parsing support for C++ attributes on using-declarations

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a few minor NFC changes and a documentation request. Thank you for this! Comment at: clang/docs/LanguageExtensions.rst:623 +C++11 Attributes on

[PATCH] D90188: Add support for attribute 'using_if_exists'

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, thank you! Comment at: clang/lib/Sema/SemaDeclCXX.cpp:11681 + if (isa(Target) != + (NonTag && isa(NonTag))) { +if (!NonTa

[PATCH] D92495: [clang] Add a new nullability annotation for swift async: _Nullable_result

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM aside from the testing requests. Comment at: clang/lib/Basic/IdentifierTable.cpp:718-719 + case NullabilityKind::NullableResult: +assert(!isContextSensitive && + "_Nullable_result isn't sup

[PATCH] D92652: [clang-tidy][docs] Update check options with boolean values instead of non-zero/0/1

aaron.ballman added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst:60 - If copy to the destination array can overflow [1] and - ``AreSafeFunctionsAvailable`` is set to ``Yes``, ``y`` or non-zero and it is + ``AreS

[PATCH] D92006: Refactoring the attrubute plugin example to fit the new API

aaron.ballman added inline comments. Comment at: clang/examples/Attribute/Attribute.cpp:26 ExampleAttrInfo() { -// Can take an optional string argument (the check that the argument -// actually is a string happens in handleDeclAttribute). -OptArgs = 1; +// Can

[PATCH] D91495: [clang-tidy] false-positive for bugprone-redundant-branch-condition in case of passed-by-ref params

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/RedundantBranchConditionCheck.cpp:39 + + // Check if PrevS < Mut < NextS return MutS && I think this comment should be hoisted above to update the comment on line 32. ==

[PATCH] D91789: [clang-tidy] find/fix unneeded trailing semicolons in macros

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/linuxkernel/ExtraSemiCheck.cpp:49 + +void ExtraSemiCheck::registerMatchers(MatchFinder *Finder) { + if (FixerKind == ESFK_Switch) { Something that's not for you to solve, but for us to

[PATCH] D92652: [clang-tidy][docs] Update check options with boolean values instead of non-zero/0/1

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM, thank you for this cleanup! Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst:60 - If copy to the destination array can overflow [1] and - ``AreSafeFunctions

[PATCH] D91885: [clang-tidy] Add support for diagnostics with no location

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/ClangTidyDiagnosticConsumer.cpp:276 + // Never ignore these. +} else if (!Context.isCheckEnabled(Error.D

[PATCH] D92355: [clang] add a `swift_async_name` attribute

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

[PATCH] D92354: [clang] add a new `swift_attr` 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/D92354/new/ https://reviews.llvm.org/D92354 __

[PATCH] D92671: Add diagnostic for for-range-declaration being specificed with thread_local

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! In D92671#2434154 , @shafik wrote: > The problem with this approach is given: > > for (static thread_local int a : A()) {} > > it

[PATCH] D92742: [clang] Add support for attribute 'swift_async'

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM aside from a minor nit. Comment at: clang/include/clang/Basic/AttrDocs.td:4367 +``actuallyAsync:callThisAsync`` wouldn't have been imported as ``async`` if not +for ``swift_async`` because it doesn't matc

[PATCH] D68410: [AttrDocs] document always_inline

aaron.ballman added a comment. LGTM aside from the minor wording nit from @ojeda. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68410/new/ https://reviews.llvm.org/D68410 ___ cfe-commits mailing list cfe

[PATCH] D91893: [clang-tidy] performance-unnecessary-copy-initialization: Prevent false positives when dependent variable is modified.

aaron.ballman added a comment. I have some quick drive-by comments but still have to think about the test cases a bit more. Comment at: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp:74 +// the BlockStmt. It does this by checking the following: +//

[PATCH] D89490: Introduce __attribute__((darwin_abi))

aaron.ballman added a reviewer: rjmccall. aaron.ballman added a subscriber: rjmccall. aaron.ballman added a comment. You should add some frontend Sema tests for the attribute. The usual tests are: correct usage without diagnostics (as both a declaration attribute and a type attribute), applying

[PATCH] D91055: [clang-tidy] Introduce misc No Integer To Pointer Cast check

aaron.ballman added a comment. There are a couple of questions from the previous iteration of the review that aren't answered yet: - the diagnostic wording doesn't actually say what's wrong with the code or how to fix it; how should users silence the diagnostic if they've found a case where it

[PATCH] D91055: [clang-tidy] Introduce misc No Integer To Pointer Cast check

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/NoIntToPtrCheck.cpp:26 + const auto *MatchedCast = Result.Nodes.getNodeAs("x"); + diag(MatchedCast->getBeginLoc(), "avoid integer to pointer casts"); +} lebedev.ri wrote: > aaron

[PATCH] D90275: [clang][IR] Add support for leaf attribute

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; gulfem wrote: > gulfem wrote: > > gulfem wrote: > > > aaron

[PATCH] D92756: [clang-tidy] Support all YAML supported spellings for bools in CheckOptions.

aaron.ballman added a comment. Is there a way to add test coverage for this to demonstrate that we handle what YAML considers a Boolean and that we don't allow oddities? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92756/new/ https://reviews.llvm

[PATCH] D92600: [ASTImporter] Add support for importing GenericSelectionExpr AST nodes.

aaron.ballman added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2366 +/// Matches C11 _Generic expression. +extern const internal::VariadicDynCastAllOfMatcher +genericSelectionExpr; Do we have a use case for adding this as an AS

[PATCH] D90275: [clang][IR] Add support for leaf attribute

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1435 + let Spellings = [GCC<"leaf">]; + let Subjects = SubjectList<[Function]>; + let Documentation = [Undocumented]; gulfem wrote: > aaron.ballman wrote: > > gulfem wrote: > >

[PATCH] D88645: [Annotation] Allows annotation to carry some additional constant arguments.

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3696-3697 + +if (E->isValueDependent() || E->isTypeDependent()) + continue; + Should this move up above the point where we're checking whether the expression has an array t

[PATCH] D87449: [clang-tidy] Add new check for SEI CERT rule SIG30-C

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/SignalHandlerCheck.cpp:131 +} +/*FunctionCallCollector Collector{[&CalledFunctions](const CallExpr *CE) { + if (isa(CE->getCalleeDecl())) Remove commented-out code?

[PATCH] D89332: [clang-tidy] performance-unnecessary-copy-initialization: Always allow std::function to be copied.

aaron.ballman added a comment. In D89332#2338319 , @njames93 wrote: > Come to think of it, this is a pretty illogical way to solve this problem, > just append `::std::function` to the AllowedTypes vector in > `registerMatchers` and be do with it. Will re

[PATCH] D89212: PR47663: Warn if an entity becomes weak after its first use.

aaron.ballman added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:6435-6436 +Attr *WeakA = nullptr; +for (Attr *A : VD->getAttrs()) { + if (!isa(A)) +continue; rsmith wrote: > aaron.ballman wrote: > > Ah, it's too bad that `Decl::s

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

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

[PATCH] D89490: Introduce __attribute__((darwin_abi))

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:1481 +def DarwinABI : DeclOrTypeAttr { + let Spellings = [GCC<"darwin_abi">]; +// let Subjects = [Function, ObjCMethod]; I suspect this should be using the `Clang` spelling as I

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

aaron.ballman closed this revision. aaron.ballman added a comment. In D74299#2338772 , @AlexanderLanin wrote: > Could someone commit this as I cannot? Thanks a lot! > Alexander Lanin > > Thanks for review @aaron.ballman! Happy to do so! I've commit on y

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

aaron.ballman added a comment. In D74299#2338823 , @aaron.ballman wrote: > In D74299#2338772 , @AlexanderLanin > wrote: > >> Could someone commit this as I cannot? Thanks a lot! >> Alexander Lanin >> >> Thanks fo

[PATCH] D89407: [clang-tidy] Add scoped enum constants to identifier naming check

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:410 + if (const auto *EnumConst = dyn_cast(D)) { +

[PATCH] D72218: [clang-tidy] new altera kernel name restriction check

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D72218#2334787 , @ffrankies wrote: > Addressed comments by @aaron.ballman regarding the diagnostic warning. > > I tried to add a test c

[PATCH] D89649: Fix __has_unique_object_representations with no_unique_address

aaron.ballman added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:2608 +continue; + else if (!Context.hasUniqueObjectRepresentations(Field->getType())) +return llvm::None; You can drop the `else` after this sorta-return if you'd l

[PATCH] D85697: [clang-tidy] Add cppcoreguidelines-prefer-scoped-enums

aaron.ballman added a comment. I tend to be very skeptical of the value of checks that basically throw out entire usable chunks of the base language because the check is effectively impossible to apply to an existing code base. Have you run the check over large C++ code bases to see just how of

[PATCH] D89743: Support Attr in DynTypedNode and ASTMatchers.

aaron.ballman added a reviewer: aaron.ballman. aaron.ballman added a comment. This is *awesome*, thank you so much for working on it! One question I have is: as it stands, this is not a particularly useful matcher because it can only be used to say "yup, there's an attribute" -- are you planning

[PATCH] D89743: Support Attr in DynTypedNode and ASTMatchers.

aaron.ballman added a comment. In D89743#2341900 , @sammccall wrote: > In D89743#2341779 , @aaron.ballman > wrote: > >> This is *awesome*, thank you so much for working on it! > > Thanks for the comments - haven't

[PATCH] D85697: [clang-tidy] Add cppcoreguidelines-prefer-scoped-enums

aaron.ballman added a comment. In D85697#2340258 , @njames93 wrote: > In D85697#2339055 , @aaron.ballman > wrote: > >> I tend to be very skeptical of the value of checks that basically throw out >> entire usable c

[PATCH] D89490: Introduce __attribute__((darwin_abi))

aaron.ballman added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:238 + if (D->hasAttr()) +return IsDarwin ? CC_C : CC_AArch64Darwin; + mstorsjo wrote: > aaron.ballman wrote: > > Can you help me understand this change a bit better? If the declara

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

aaron.ballman added a comment. In D74299#2339994 , @AlexanderLanin wrote: > I cannot reproduce the problem and there is just not enough info to go on. Ugh, I hate when that happens. :-( > Is there any way to get anything in addition? e.g. run the same t

[PATCH] D89332: [clang-tidy] performance-unnecessary-copy-initialization: Always allow std::function to be copied.

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM with an extra testing request (that should hopefully just work for you). Comment at: clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp:445 + int i = Orig(); +} -

[PATCH] D85697: [clang-tidy] Add cppcoreguidelines-prefer-scoped-enums

aaron.ballman added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-prefer-scoped-enums.cpp:165 + +enum class OpaqueScopedEnumWithFixedUnderlyingType : unsigned; janosbenjaminantal wrote: > aaron.ballman wrote: > > njame

[PATCH] D88645: [Annotation] Allows annotation to carry some additional constant arguments.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a request for a comment to be added. Thank you! Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3706 + +if (!Result || !Notes.empty()) { +

[PATCH] D89651: [clang-tidy] Add bugprone-suspicious-memory-comparison check

aaron.ballman added a comment. In D89651#2342367 , @gbencze wrote: > In D89651#2338266 , @njames93 wrote: > >> Should point out there is already a check for cert-oop57-cpp, added in >> D72488

[PATCH] D72553: [clang-tidy] Add performance-prefer-preincrement check

aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. In D72553#1815497 , @njames93 wrote: > could this do with an alias into performance? If it was 1997, I'd say yes, but I am not

[PATCH] D89899: [CodeGen] Implement [[likely]] and [[unlikely]] for the iteration statements

aaron.ballman added a comment. Thank you for this! Comment at: clang/include/clang/Basic/AttrDocs.td:1773 -At the moment the attributes only have effect when used in an ``if``, ``else``, -or ``switch`` statement. +Below some usage examples likelihood attributes and their effe

<    38   39   40   41   42   43   44   45   46   47   >