[PATCH] D90892: [AIX][FE] Support constructor/destructor attribute

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. I think this generally seems reasonable, but I'm far from an AIX expert so you should wait a few days in case other reviewers have feedback. Comment at: clang/

[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] D91035: [NFC, Refactor] Convert FunctionDefinitionKind from DeclSpech.h to a scoped enum

aaron.ballman added inline comments. Comment at: clang/include/clang/Sema/DeclSpec.h:1837 /// Actually a FunctionDefinitionKind. - unsigned FunctionDefinition : 2; + FunctionDefinitionKind FunctionDefinition : 2; faisalv wrote: > aaron.ballman wrote: > > f

[PATCH] D91037: [clang-tidy] Fix crash in bugprone-redundant-branch-condition on ExprWithCleanups

aaron.ballman added a comment. In D91037#2387377 , @njames93 wrote: > Taking a step back, rather than worrying about if its an `ExprWithCleanups`. > Shouldn't we just get the condition removing all implicit nodes. > > const Expr* Cond = InnerIf->getCond

[PATCH] D91037: [clang-tidy] Fix crash in bugprone-redundant-branch-condition on ExprWithCleanups

aaron.ballman added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp:1092 + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'isSet' [bugprone-redundant-branch-condition] + // CHECK-FIXES: {

[PATCH] D91047: Add a call super attribute plugin example

aaron.ballman added a comment. I have more review to do on this but have to run for a while and wanted to get you this feedback sooner rather than later. Comment at: clang/docs/ClangPlugins.rst:119 + +Attribute plugin to mark a virtual method as `call_super`, subclasses must

[PATCH] D91037: [clang-tidy] Fix crash in bugprone-redundant-branch-condition on ExprWithCleanups

aaron.ballman added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp:1092 + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'isSet' [bugprone-redundant-branch-condition] + // CHECK-FIXES: {

[PATCH] D91311: Add new 'preferred_name' attribute.

aaron.ballman added a comment. I really like this attribute, thank you for working on this! Comment at: clang/include/clang/Basic/Attr.td:2367 +def PreferredName : InheritableAttr { + let Spellings = [Clang<"preferred_name">]; + let Subjects = SubjectList<[ClassTmpl]>; --

[PATCH] D91184: [clang-tidy] Merge options inplace instead of copying

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

[PATCH] D90042: [clang-tidy] performance-unnecessary-copy-initialization: Check for const reference arguments that are replaced template parameter type.

aaron.ballman added a comment. In D90042#2390203 , @flx wrote: > Thanks for the suggestion, I had never hear of creduce! Glad to have introduced you to it -- it's a great tool! > After a bit of trial an error I seem to have found a more minimal example:

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

aaron.ballman added a comment. In D90282#2391005 , @njames93 wrote: > Should this be a NamingStyle option instead. > `{key: readability-identifier-naming.ParameterShortSizeThreshold, value: 2}` > WDYT? I think that makes a lot of sense -- I can imagine wa

[PATCH] D91037: [clang-tidy] Fix crash in bugprone-redundant-branch-condition on ExprWithCleanups

aaron.ballman added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp:1092 + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'isSet' [bugprone-redundant-branch-condition] + // CHECK-FIXES: {

[PATCH] D91037: [clang-tidy] Fix crash in bugprone-redundant-branch-condition on ExprWithCleanups

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/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp:1092 + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: re

[PATCH] D90984: Update matchers to be traverse-aware

aaron.ballman added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3115 + + if (!Finder->isTraversalAsIs() && (*MatchIt)->isImplicit()) +return false; If the traversal is not `AsIs`, that doesn't mean it's `IgnoreUnlessSpelledInS

[PATCH] D91311: Add new 'preferred_name' attribute.

aaron.ballman accepted this revision. aaron.ballman added a comment. The attribute parts LGTM, but I did have a question about the libc++ parts. Comment at: clang/include/clang/Basic/Attr.td:2367 +def PreferredName : InheritableAttr { + let Spellings = [Clang<"preferred_name">

[PATCH] D91047: Add a call super attribute plugin example

aaron.ballman added inline comments. Comment at: clang/docs/ClangPlugins.rst:117 +Defining CallSuperAttr +=== + psionic12 wrote: > aaron.ballman wrote: > > aaron.ballman wrote: > > > The number of underlines here looks off -- can you verify it's c

[PATCH] D91035: [NFC, Refactor] Convert FunctionDefinitionKind from DeclSpech.h to a scoped enum

aaron.ballman added inline comments. Comment at: clang/include/clang/Sema/DeclSpec.h:1762 }; +using FDK = FunctionDefinitionKind; rsmith wrote: > I don't think it's OK to have an initialism like this in the `clang` > namespace scope -- generally-speaking, the

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

aaron.ballman added subscribers: jeroen.dobbelaere, jdoerfert, hfinkel. aaron.ballman added a comment. In D91055#2382356 , @lebedev.ri wrote: > CC'ing @rsmith regarding the suggestion/question of also having a clang > diagnostic for this. I'm not Richard

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

aaron.ballman added a comment. In D90282#2393019 , @smhc wrote: > In D90282#2391360 , @aaron.ballman > wrote: > >> In D90282#2391005 , @njames93 wrote: >> >>> Should this be

[PATCH] D91506: [NFC, Refactor] Convert TypeSpecifiersPipe from Specifiers.h to a scoped enum (tiny)

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Would you like me to commit this one on your behalf or would you like to request your own commit access (https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access)

[PATCH] D91506: [NFC, Refactor] Convert TypeSpecifiersPipe from Specifiers.h to a scoped enum (tiny)

aaron.ballman added a comment. In D91506#2397158 , @tschuett wrote: > I am happy, if you could commit this for me. I am still learning. I'm happy to do so -- how would you like me to attribute the patch? Is `Thorsten ` what you'd like me to use? Reposi

[PATCH] D91506: [NFC, Refactor] Convert TypeSpecifiersPipe from Specifiers.h to a scoped enum (tiny)

aaron.ballman closed this revision. aaron.ballman added a comment. In D91506#2397181 , @tschuett wrote: > Yes. Thank you! You're welcome! I've commit on your behalf in a6ac2b32fbab9679c8f2fa97a3b1123e3a9654c8

[PATCH] D91498: [NFC, Refactor] Convert TypeSpecifierSign from Specifiers.h to a scoped enum

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! I'll land on your behalf momentarily. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91498/new/ https://reviews.llvm.org/D914

[PATCH] D91498: [NFC, Refactor] Convert TypeSpecifierSign from Specifiers.h to a scoped enum

aaron.ballman added a comment. In D91498#2397187 , @aaron.ballman wrote: > LGTM! I'll land on your behalf momentarily. I lied. The patch doesn't apply cleanly for me -- can you rebase and upload a new diff? Repository: rG LLVM Github Monorepo CHANG

[PATCH] D91498: [NFC, Refactor] Convert TypeSpecifierSign from Specifiers.h to a scoped enum

aaron.ballman closed this revision. aaron.ballman added a comment. In D91498#2397251 , @tschuett wrote: > rebased Thanks! I've commit on your behalf in 7c6412e0ccf5e00a9f59c5805df9df6ff6720ed2

[PATCH] D91311: Add new 'preferred_name' attribute.

aaron.ballman added inline comments. Comment at: libcxx/include/regex:2520 +_LIBCPP_PREFERRED_NAME(wregex) +basic_regex { rsmith wrote: > Quuxplusone wrote: > > Why does this attribute go on the class template? Shouldn't it be an > > attribute on the ty

[PATCH] D91047: Add a call super attribute plugin example

aaron.ballman added inline comments. Comment at: clang/docs/ClangPlugins.rst:117 +Defining CallSuperAttr +=== + psionic12 wrote: > aaron.ballman wrote: > > psionic12 wrote: > > > aaron.ballman wrote: > > > > aaron.ballman wrote: > > > > > The numb

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

aaron.ballman added a comment. In D90188#2394361 , @erik.pilkington wrote: > Add support for C++11-style attributes on using-declarations. FWIW, it'd be a bit easier if those changes were split off into their own patch, as they're orthogonal to `using_i

[PATCH] D90984: Update matchers to be traverse-aware

aaron.ballman added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3115 + + if (!Finder->isTraversalAsIs() && (*MatchIt)->isImplicit()) +return false; steveire wrote: > aaron.ballman wrote: > > If the traversal is not `AsIs`, that

[PATCH] D90042: [clang-tidy] performance-unnecessary-copy-initialization: Check for const reference arguments that are replaced template parameter type.

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

[PATCH] D90982: Ignore implicit nodes in IgnoreUnlessSpelledInSource mode

aaron.ballman added a comment. Thank you for this, I think it's mostly looking good! Comment at: clang/include/clang/AST/ASTNodeTraverser.h:88 void Visit(const Decl *D) { +if (Traversal != TK_AsIs && D->isImplicit()) + return; Similar to the feedbac

[PATCH] D91592: [ASTMatchers] Fix typo for hasAnyOverloadedOperatorName

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? If so, is `Keishi Hattori ` the correct attribution you'd like me to use? Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D91543: [clang-tidy] Improving bugprone-sizeof-expr check.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a testing request. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-sizeof-expression.cpp:301 + sum += sizeof(PtrArray) / sizeo

[PATCH] D90851: [clang-tidy] Extending bugprone-signal-handler with POSIX functions.

aaron.ballman added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/system-header-posix-api.h:1 -//===--- signal.h - Stub header for tests ---*- C++ -*-===// +//===--- system-header-posix-api.h - Stub header for tests -

[PATCH] D91311: Add new 'preferred_name' attribute.

aaron.ballman added a comment. In D91311#2398526 , @rsmith wrote: > In D91311#2398144 , @ldionne wrote: > >> I think that the fact we need to re declare everything shows how the >> ergonomics would be better if we

[PATCH] D90984: Update matchers to be traverse-aware

aaron.ballman added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:4083 +return false; + return InnerMatcher.matches(*Arg->IgnoreParenImpCasts(), Finder, Builder); } This probably shouldn't compile given that there's no declarati

[PATCH] D90984: Update matchers to be traverse-aware

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

[PATCH] D90982: Ignore implicit nodes in IgnoreUnlessSpelledInSource mode

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, thank you! Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:164 auto *LambdaNode = dyn_cast_or_null(StmtNode); - if (LambdaNode && !Finder

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

aaron.ballman added a comment. Can you mention this change in the release notes? Also, should we document it explicitly in the Language Extensions documentation, or do you think this is too tiny of a feature to warrant that? Comment at: clang/include/clang/Parse/Parser.h:2648

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

aaron.ballman added inline comments. Comment at: clang/test/Parser/cxx0x-attributes.cpp:134 -[[]] using ns::i; // expected-error {{an attribute list cannot appear here}} +[[]] using ns::i; [[unknown]] using namespace ns; // expected-warning {{unknown attribute 'unknown' ignor

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

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.cpp:76 + "values|the members of the object}1 " + "manually") +<< PointeeQualifiedType << (PointeeType->isRecordType() ? 1 :

[PATCH] D91543: [clang-tidy] Improving bugprone-sizeof-expr check.

aaron.ballman added a comment. In D91543#2400051 , @balazske wrote: > This checker has multiple weaknesses. There are more cases when the warnings > should not appear (probably if the argument of `sizeof` is a template > parameter), or more than one warn

[PATCH] D91485: [clang-tidy] ElseAfterReturn check wont suggest fixes if preprocessor branches are involved

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp:118 static void removeElseAndBrackets(DiagnosticBuilder &Diag, ASTContext &Context, - const Stmt *Else, SourceLocation ElseLoc) { +

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

aaron.ballman added a comment. Thank you for working on this! Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp:10 +bool tryToExtinguish(bool&); +bool tryToExtinguishByVal(bool &); void tryPutFireOut(); Did you mea

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

aaron.ballman added a comment. In D91630#2400731 , @rsmith wrote: > Do `[[deprecated]]` and `[[maybe_unused]]` now work for > //using-declarator//s? If so, a test for that would be useful. I think the answer is yes and no, respectively (at least in terms

[PATCH] D91592: [ASTMatchers] Fix typo for hasAnyOverloadedOperatorName

aaron.ballman closed this revision. aaron.ballman added a comment. Thank you for the fix, I've commit on your behalf in 871fe71f2951cb19421a2b47ddb54ed6b3c8cba2 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D91485: [clang-tidy] ElseAfterReturn check wont suggest fixes if preprocessor branches are involved

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from the testing request. Comment at: clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return.cpp:313 +#endif +} n

[PATCH] D91015: [clang-tidy] Extend bugprone-string-constructor-check to std::string_view.

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

[PATCH] D91543: [clang-tidy] Improving bugprone-sizeof-expr check.

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-sizeof-expression.cpp:317 + sum = sizeof(PtrArray) / sizeof(PtrArray1[0]); + // There is no warning for 'sizeof(T*)/sizeof(Q)' case. +

[PATCH] D91047: Add a call super attribute plugin example

aaron.ballman added inline comments. Comment at: clang/docs/ClangPlugins.rst:116 +Defining CallSuperAttr +== psionic12 wrote: > After a whole day's research of `Sphinx`, I figured out that > `ClangPlugins.rst` is the "proto-type" of > htt

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

aaron.ballman added inline comments. Comment at: clang/include/clang/Basic/Attr.td:384 +def ExcludeTarget : TargetSpec { + let CustomCode = [{ !Target.getTriple().isOSzOS() }]; This is not a very descriptive name -- if this is only supposed to be used for `in

[PATCH] D91485: [clang-tidy] ElseAfterReturn check wont suggest fixes if preprocessor branches are involved

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Comment at: clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return-pp-no-crash.cpp:1 +// RUN: clang-tidy %s -checks=-*,readability-else-after-return -- -std=c++11 + Yo

[PATCH] D91047: Add a call super attribute plugin example

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 plugin example! Do you need me to commit on your behalf? If so, are you okay with `Yafei Liu ` for attribution? Repository: rG LLVM Github Monorep

[PATCH] D91639: Add documentation illustrating use of IgnoreUnlessSpelledInSource

aaron.ballman added a comment. Thank you for the documentation effort! Comment at: clang/docs/LibASTMatchersReference.html:100 + +Traverse Mode + I think this section should go above the Node Matchers section in the document so that Node Matchers stays directl

[PATCH] D91009: [clang-tidy] Include std::basic_string_view in readability-redundant-string-init.

aaron.ballman added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp:25 +struct basic_string_view { + using size_type = unsigned; + nit: `using size_type = decltype(sizeof(0));` Com

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

aaron.ballman added a comment. In D90282#2395956 , @smhc wrote: > I updated it to use XXShortSizeThreshold. > > But I was thinking.. perhaps a more generic approach could be > "xxIgnoreRegex", this would allow you to do things such as : > > VariableIgnore

[PATCH] D75229: [clang-tidy] Add signal-in-multithreaded-program check

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SignalInMultithreadedProgramCheck.cpp:57 +const MatchFinder::MatchResult &Result) { + bool IsPosix = PP->isMacroDefined("_POSIX_C_SOURCE") || + Result.Context->getTargetIn

[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:237 +std::string Val = HNOption.General.lookup(Opt.first); +if (Val.empty()) { + HNOption.General.insert({Opt.first, Opt.second.str()}); -

[PATCH] D91639: Add documentation illustrating use of IgnoreUnlessSpelledInSource

aaron.ballman added inline comments. Comment at: clang/docs/LibASTMatchersReference.html:91-94 +This mode is hard to use correctly and +requires more development iteration because it means +that the user must write AST Matchers to explicitly traverse or ignore nodes +which are no

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

aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. The request in D90180 was to not add a check for each kind of problematic semicolon situation but to instead make a single check that

[PATCH] D91639: Add documentation illustrating use of IgnoreUnlessSpelledInSource

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

[PATCH] D91047: Add a call super attribute plugin example

aaron.ballman closed this revision. aaron.ballman added a comment. In D91047#2407091 , @psionic12 wrote: > @aaron.ballman That would be nice if your could help, and `Yafei Liu > ` is okay. Thank you for the new example, I've commit on your behalf in 2ce

[PATCH] D91009: [clang-tidy] Include std::basic_string_view in readability-redundant-string-init.

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

[PATCH] D91035: [NFC, Refactor] Convert FunctionDefinitionKind from DeclSpech.h to a scoped enum

aaron.ballman added inline comments. Comment at: clang/include/clang/Sema/DeclSpec.h:1756 /// a function. -enum FunctionDefinitionKind { - FDK_Declaration, - FDK_Definition, - FDK_Defaulted, - FDK_Deleted +enum class FunctionDefinitionKind : unsigned char { + Declaration, -

[PATCH] D88445: Use "default member initializer" instead of "in-class initializer" for diagnostics

aaron.ballman created this revision. aaron.ballman added a reviewer: rsmith. aaron.ballman requested review of this revision. This changes some diagnostics to use terminology from the standard rather than invented terminology, which improves consistency with other diagnostics as well. There are

[PATCH] D26418: [clang-tidy] Add '-suppress-checks-filter' option to suppress diagnostics from certain files

aaron.ballman added a comment. While I agree that there's an issue here that needs to be solved, I don't think this patch will be merged as-is -- there are technical issues brought up by @alexfh that have not been addressed, and I share the opinion that we should extend existing suppression mec

[PATCH] D88445: Use "default member initializer" instead of "in-class initializer" for diagnostics

aaron.ballman updated this revision to Diff 294947. aaron.ballman added a comment. Updating based on review feedback. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88445/new/ https://reviews.llvm.org/D88445 Files: clang/include/clang/Basic/DiagnosticParseKinds.td clang/include/clan

[PATCH] D88445: Use "default member initializer" instead of "in-class initializer" for diagnostics

aaron.ballman updated this revision to Diff 294951. aaron.ballman added a comment. Missed a test case. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88445/new/ https://reviews.llvm.org/D88445 Files: clang/include/clang/Basic/DiagnosticParseKinds.td clang/include/clang/Basic/Diagnos

[PATCH] D88445: Use "default member initializer" instead of "in-class initializer" for diagnostics

aaron.ballman closed this revision. aaron.ballman added a comment. Thank you for the review! I've commit in 15fbae8ac303d8601ea95418d4818cb50d0765e1 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88445/new/ https://r

[PATCH] D88333: Better diagnostics for anonymous bit-fields with attributes or an initializer

aaron.ballman closed this revision. aaron.ballman marked 2 inline comments as done. aaron.ballman added a comment. Thank you for the reviews, I've committed in 538762fef0b662048be2a261ebc12da249efa977 Comme

[PATCH] D88363: [CodeGen] Improve likelihood attribute branch weights

aaron.ballman added inline comments. Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:1622 EmitBranchOnBoolExpr(CondOp->getCond(), LHSBlock, RHSBlock, - getProfileCount(CondOp), Weights); + getProfileCount(CondOp), Stmt::LH_No

[PATCH] D88526: [clang][Sema] Fix PR47676: Handle dependent AltiVec C-style cast

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

[PATCH] D88275: [ASTMatchers] Add matcher `hasParentIgnoringImplicit`.

aaron.ballman added a comment. In D88275#2303283 , @ymandel wrote: >> I'm not concerned about the basic idea behind the proposed matcher, I'm only >> worried we're making AST matching more confusing by having two different >> ways of inconsistently accom

[PATCH] D87962: [clang] Change the multi-character character constants from extension to implementation-defined.

aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. In D87962#2305375 , @nomanous wrote: > In D87962#2298021 , @aaron.ballman > wrote: > >

[PATCH] D88491: [ASTContext] Use AllowCXX in all merge*Type methods, strip references

aaron.ballman added a comment. Do you have test cases for this change? I didn't see any relevant ones in D88384 . Comment at: clang/lib/AST/ASTContext.cpp:9174 bool ASTContext::typesAreBlockPointerCompatible(QualType LHS, QualType RHS) { re

[PATCH] D88666: DirectoryWatcher: add an implementation for Windows

aaron.ballman added a comment. I wonder if we should unit test this functionality by having some tests that create and remove files that are watched. I'm not 100% convinced that is a great idea, but not having test coverage for the change is also not really a great idea either. Thoughts welcome

[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:33 + + for (const FunctionDecl *D : Node.redecls()) +if (D->getASTContext().getSourceManager().isInSystemHeader( I'm not certain I understand why we're

[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:33 + + for (const FunctionDecl *D : Node.redecls()) +if (D->getASTContext().getSourceManager().isInSystemHeader( balazske wrote: > aaron.ballman wrote:

[PATCH] D87962: [clang] Change the multi-character character constants from extension to implementation-defined.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D87962#2307824 , @rsmith wrote: > In D87962#2306043 , @aaron.ballman > wrote: > >> That doesn

[PATCH] D88700: [clang-tidy] modernize-use-trailing-return-type fix #44206

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

[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:33 + + for (const FunctionDecl *D : Node.redecls()) +if (D->getASTContext().getSourceManager().isInSystemHeader( balazske wrote: > aaron.ballman wrote:

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

aaron.ballman added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp:25 +// RUN: {key: readability-identifier-naming.FunctionCase , value: CamelCase }, \ +// RUN: {key: readabili

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

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.cpp:90 +Check.diag(SM.getLocForStartOfFile(SM.getMainFileID()), + "Naming your OpenCL kernel source file 'kernel.cl', 'Verilog.cl'" +

[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:27 + // Find a possible redeclaration in system header. + for (const FunctionDecl *D : FD->redecls()) +if (FD->getASTContext().getSourceManager().isInSystemHeader(

[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:41 +static bool isAllowedSystemCall(const FunctionDecl *FD) { + if (!FD->getIdentifier()) +return true; balazske wrote: > aaron.ballman wrote: > > A fun

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:93-96 +// Criteria only uses three bits, so uint8_t is used as an underlying type. +// We could make C a bitfield, but then we would not save an

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

aaron.ballman added a comment. In D36836#2308981 , @lattner wrote: > Hi all, > > The LLVM foundation board discussed this offline -- it looks like the > contributor is a bit confused about the LLVM project policies towards > copyright, license, patent st

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:367-368 + bool TraverseStmt(Stmt *Node) { +if (!Node) + return Base::TraverseStmt(Node); + lebedev.ri wrote: > aaron.ballman

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM, thank you for your patience with the process. Comment at: clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp:381 +// FIXME: "each method in a recursion cycle" Increment is

[PATCH] D88363: [CodeGen] Improve likelihood attribute branch weights

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM modulo comment nits. Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:1622 EmitBranchOnBoolExpr(CondOp->getCond(), LHSBlock, RHSBlock, - getProfileCount(CondOp), Weights); +

[PATCH] D88700: [clang-tidy] modernize-use-trailing-return-type fix #44206

aaron.ballman added a comment. In D88700#2309431 , @bernhardmgruber wrote: > Thank you for the quick review! Can you please commit it for me as well? > Thank you! Happy to! I've commit on your behalf in 07028cd5dbb8417fb41121a7e75290fab00f65fc

[PATCH] D88700: [clang-tidy] modernize-use-trailing-return-type fix #44206

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:72 +TL.getAs().getTypePtr()->getDecl()->getName())) + return false; default: bernhardmgruber wrote: > There is

[PATCH] D88700: [clang-tidy] modernize-use-trailing-return-type fix #44206

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:72 +TL.getAs().getTypePtr()->getDecl()->getName())) + return false; default: bernhardmgruber wrote: > aaron.bal

[PATCH] D88831: [clang-tidy] Remove obsolete checker google-runtime-references

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM to remove rather than move into `readability` unless someone has a good argument as to why it belongs there. I don't think this pattern makes code more readable in general as it recommends using a more verbose syntax (poin

[PATCH] D87962: [clang] Change the multi-character character constants from extension to implementation-defined.

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Do you need someone to commit on your behalf? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87962/new/ https://reviews.llvm.org/D87962 ___ cfe-commits mailing list cfe

[PATCH] D88831: [clang-tidy] Remove obsolete checker google-runtime-references

aaron.ballman added a comment. One request though: can you add a release note about the removal with a brief explanation of why it was removed? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88831/new/ https://reviews.llvm.org/D88831 _

[PATCH] D87962: [clang] Change the multi-character character constants from extension to implementation-defined.

aaron.ballman closed this revision. aaron.ballman added a comment. In D87962#2313363 , @nomanous wrote: > In D87962#2312617 , @aaron.ballman > wrote: > >> LGTM! Do you need someone to commit on your behalf? > > Yes

[PATCH] D88275: [ASTMatchers] Add matcher `hasParentIgnoringImplicit`.

aaron.ballman added a comment. In D88275#2313342 , @ymandel wrote: > In D88275#2305989 , @aaron.ballman > wrote: > >> In D88275#2303283 , @ymandel wrote: >> I'm not con

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

aaron.ballman added a comment. Despite my earlier happiness with the patch, it's now a bit unclear to me from the C++ Core Guideline wording what the right behavior here actually is. The bounds profile is trying to ensure you don't access out-of-range elements of a container and the decay part

[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); +}}; --

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