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

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

[PATCH] D85319: [analyzer] Get info from the LLVM IR for precision

whisperity added subscribers: dcoughlin, rjmccall, rsmith. whisperity added a comment. What will happen with the ability to analyse a translation unit whose target was not part of `LLVM_TARGETS_TO_BUILD` of the current `clang` binary? Will it break, because the binary lacks the information on ho

[PATCH] D85528: [analyzer] Fix cast evaluation on scoped enums in ExprEngine

whisperity added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:98-104 +const auto IsIntegralOrUnscopedCompleteEnumerationType = [](QualType Ty) { + const Type *CanonicalType = Ty.getCanonicalType().getTypePtr(); +

[PATCH] D85528: [analyzer] Fix cast evaluation on scoped enums in ExprEngine

whisperity added inline comments. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:98-104 +const auto IsIntegralOrUnscopedCompleteEnumerationType = [](QualType Ty) { + const Type *CanonicalType = Ty.getCanonicalType().getTypePtr(); +

[PATCH] D84520: [Analyzer] Improve invalid dereference bug reporting in DereferenceChecker.

whisperity added a comment. In D84520#2206077 , @balazske wrote: > Do the null pointer and invalid pointer dereference belong to the same > checker, that is called //NullDereference//? I think both SA and Tidy auto-appends the checker's name into the emi

[PATCH] D85528: [analyzer] Fix cast evaluation on scoped enums in ExprEngine

whisperity added inline comments. Comment at: clang/test/Analysis/z3-refute-enum-crash.cpp:5 +// +// Requires z3 only for refutation. Works with both constraint managers. + You can have multiple `RUN` lines in the same test file, which will result in the tests p

[PATCH] D85752: [Analyzer] Store the pointed/referenced type for dynamic casts

whisperity added a comment. @baloghadamsoftware Maybe there is a typo in the summary of the patch > then `&B` //is a// `&B` Shouldn't this be "&A is a &B"? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85752/new/ https://reviews.llvm.org/D85752

[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

whisperity added a comment. In D72705#2210255 , @balazske wrote: > More results in CodeChecker: emacs_errorreturn >

[PATCH] D85728: [Analyzer] Support for the new variadic isa<> and isa_and_not_null<> in CastValueChecker

whisperity added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:289 + for (QualType CastToTy: CastToTyVec) { +if (CastFromTy->isPointerType()) + CastToTy = C.getASTContext().getPointerType(CastToTy); NoQ wrote: > Hmm

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

whisperity added a comment. In D81272#2218050 , @aaron.ballman wrote: > Thanks to the new info, I think the check basically LGTM. Can you add some > negative tests and documentation wording to make it clear that the check > doesn't currently handle all

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

whisperity added a comment. In D81272#2218175 , @aaron.ballman wrote: > While I agree with your observation about data and flow sensitivity, I > approved on the belief that the check as-is provides enough utility to > warrant adding it as-is. If someone

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

whisperity requested changes to this revision. whisperity added a comment. This revision now requires changes to proceed. (Maybe this will make Phab not show "✅ Accepted"...) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75665/new/ https://reviews.

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

whisperity added a comment. Please check the summary of the patch, it seems to contain old information as well. Comment at: clang/docs/analyzer/user-docs/CrossTranslationUnit.rst:210-212 +Preferably the same compilation database should be used when generating the external def

[PATCH] D79330: [Analyzer][VLASizeChecker] Check for VLA size overflow.

whisperity added inline comments. Comment at: clang/test/Analysis/vla-overflow.c:8 + if (x == BIGINDEX) { +// We expect here that size_t is a 64 bit value. +// Size of this array should be the first to overflow. While it's generally true nowadays, instea

[PATCH] D79330: [Analyzer][VLASizeChecker] Check for VLA size overflow.

whisperity added a comment. In D79330#2035850 , @balazske wrote: > I was looking at CERT ARR32-C > > "Ensure size arguments for

[PATCH] D91000: [clang-tidy] Add bugprone-unsafe-functions checker.

whisperity added a comment. Generally LGTM. Please revisit the documentation and let's fix the style, and then we can land. In D91000#3197851 , @aaron.ballman wrote: > In terms of whether we should expose the check in C++: I think we shouldn't. > [...]

[PATCH] D118370: [clang-tidy] bugprone-signal-handler: Message improvement and code refactoring.

whisperity accepted this revision. whisperity added a comment. This revision is now accepted and ready to land. Alright, I think this is good to go. I like that it makes it clear that the called function is coming from some external source (system header or otherwise). Repository: rG LLVM Gi

[PATCH] D118996: [clang-tidy] Support C++14 in bugprone-signal-handler.

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp:153 +diag(HandlerLambda->getBeginLoc(), + "lambda function is not allowed as signal handler (until C++17)") +<< HandlerLambda->getSourceRange();

[PATCH] D121387: [analyzer] ClangSA should tablegen doc urls refering to the main doc page

whisperity added a comment. (Side note: you should avoid the list-expansion syntax in URLs because browsers do not understand them and result in links that are not leading anywhere.) I like this. We should continue getting to the point where the documentation is having a uniform layout. CHANG

[PATCH] D118996: [clang-tidy] Support C++14 in bugprone-signal-handler.

whisperity accepted this revision. whisperity added a comment. Alright, I think we should have this in and let C++17 things to be future work. Please see the inline comment, but otherwise this should be good enough. Can always improve in a future version. 😉 LLVM 15 has branched off, so this sho

[PATCH] D131319: [clang-tidy] Update llvm-prefer-isa-or-dyn-cast-in-conditionals with new syntax

whisperity added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/llvm/prefer-isa-or-dyn-cast-in-conditionals.cpp:12-13 template bool isa(Y *); template Will the tests pass properly once the fixes are applied, even though the repla

[PATCH] D131319: [clang-tidy] Update llvm-prefer-isa-or-dyn-cast-in-conditionals with new syntax

whisperity removed a reviewer: bzcheeseman. whisperity added a comment. This revision now requires review to proceed. In D131319#3708667 , @bzcheeseman wrote: > This is great, thank you for doing this! I'm not a competent reviewer for the > actual clang

[PATCH] D20689: [clang-tidy] Add 'readability-suspicious-call-argument' check

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG73e4b5cfa8ea: [clang-tidy] Add 'readability-suspicious-call-argume

[PATCH] D106361: [clang-tidy] Fix crash and handle AttributedTypes in 'bugprone-easily-swappable-parameters'

whisperity created this revision. whisperity added reviewers: aaron.ballman, vabridgers. whisperity added a project: clang-tools-extra. Herald added subscribers: martong, gamesh411, Szelethus, rnkovacs, xazax.hun. whisperity requested review of this revision. Herald added a subscriber: cfe-commits.

[PATCH] D106361: [clang-tidy] Fix crash and handle AttributedTypes in 'bugprone-easily-swappable-parameters'

whisperity updated this revision to Diff 360269. whisperity marked 4 inline comments as done. whisperity added a comment. - Remove orthogonal/half-baked changes. - Fix a mistake in the logic changing only a temporary state. - Add a missing //FIXME//. Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D106361: [clang-tidy] Fix crash and handle AttributedTypes in 'bugprone-easily-swappable-parameters'

whisperity added a comment. In D106361#2890458 , @aaron.ballman wrote: > The rename from `CommonType` to `CoreType` could probably be done as a > separate NFC to make the review a bit easier, if we decide that's a good > terminology switch. I'm not cer

[PATCH] D106361: [clang-tidy] Fix crash and handle AttributedTypes in 'bugprone-easily-swappable-parameters'

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:476-477 + +QualType NewCoreType = CoreType; +NewCoreType.addFastQualifiers(Quals.getFastQualifiers()); +NewCoreType.getQualifiers().addQualifiers(Qu

[PATCH] D106431: [clang-tidy] Fix cppcoreguidelines-init-variables with enum judgement

whisperity added a comment. Is this the right decision to make, conceptually? It will leave the variable uninitialised still, and reading such an uninit variable is still an issue, even if it is an enum. Could we consider the alternative of warning the user about the uninitialized variable, jus

[PATCH] D106361: [clang-tidy] Fix crash and handle AttributedTypes in 'bugprone-easily-swappable-parameters'

whisperity updated this revision to Diff 360435. whisperity edited the summary of this revision. whisperity added a comment. **NFC** Fix bad phrasing in comments (e.g. still mentioning //core type// and having a misleading negated sentence.) Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D106442: [clang-tidy] Improve "common type" diagnostic output in 'bugprone-easily-swappable-parameters'

whisperity created this revision. whisperity added a reviewer: aaron.ballman. whisperity added a project: clang-tools-extra. Herald added subscribers: martong, gamesh411, Szelethus, dkrupp, rnkovacs, xazax.hun. whisperity requested review of this revision. Herald added a subscriber: cfe-commits.

[PATCH] D106361: [clang-tidy] Fix crash and handle AttributedTypes in 'bugprone-easily-swappable-parameters'

This revision was automatically updated to reflect the committed changes. Closed by commit rG473eff1c3057: [clang-tidy] Fix crash and handle AttributedType in 'bugprone-easily-swappable… (authored by whisperity). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D106431: [clang-tidy] Fix cppcoreguidelines-init-variables with enum judgement

whisperity added a comment. The problem with enums is that translating //zero// (0, 0.0, nullptr, etc...) to the enum case is not always apparent. A warning **should** always be given. And //if// you can find a zero member in the enum, we can report an automated suggestion for that. To give an

[PATCH] D106431: [clang-tidy] Fix cppcoreguidelines-init-variables with enum judgement

whisperity added a comment. In D106431#2896334 , @aaron.ballman wrote: > In D106431#2896206 , @whisperity > wrote: > >> The problem with enums is that translating //zero// (0, 0.0, nullptr, >> etc...) to the en

[PATCH] D106431: [clang-tidy] Fix cppcoreguidelines-init-variables with enum judgement

whisperity added a comment. In D106431#2896441 , @aaron.ballman wrote: > However, I don't recall how clang-tidy interacts with fix-its on notes off > the top of my head, so I'm making an assumption that clang-tidy's automatic > fixit applying mode hand

[PATCH] D106442: [clang-tidy] Improve "common type" diagnostic output in 'bugprone-easily-swappable-parameters'

This revision was automatically updated to reflect the committed changes. Closed by commit rG8b0cc4a65dd4: [clang-tidy] Improve "common type" diagnostic output in 'bugprone-easily… (authored by whisperity). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D106431: [clang-tidy] Fix cppcoreguidelines-init-variables with enum judgement

whisperity added a comment. In D106431#2907002 , @Sockke wrote: > Any thoughts? : ) First, let's first fix that we should still warn for the uninitialised `enum` case, without a FixIt. That's the issue at hand, right now, Clang-Tidy generates, as you

[PATCH] D106431: [clang-tidy] Fix cppcoreguidelines-init-variables by removing the enum FixIt, and add support for initialization check of scoped enum.

whisperity added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:156 + + ⁣Added support for initialization check of the scoped enum + Unterminated sequence. Also, where is this feature implemented? CHANGES SINCE LAST ACTION https://revie

[PATCH] D106946: [clang-tidy] Fix crash on "reference to array" parameters in 'bugprone-easily-swappable-parameters'

whisperity created this revision. whisperity added a reviewer: aaron.ballman. whisperity added a project: clang-tools-extra. Herald added subscribers: martong, gamesh411, Szelethus, dkrupp, rnkovacs, xazax.hun. whisperity requested review of this revision. Herald added a subscriber: cfe-commits.

[PATCH] D106946: [clang-tidy] Fix crash on "reference to array" parameters in 'bugprone-easily-swappable-parameters'

whisperity updated this revision to Diff 362333. whisperity added a comment. Add some more test cases! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106946/new/ https://reviews.llvm.org/D106946 Files: clang-tools-extra/clang-tidy/bugprone/Easily

[PATCH] D106946: [clang-tidy] Fix crash on "reference to array" parameters in 'bugprone-easily-swappable-parameters'

whisperity updated this revision to Diff 362334. whisperity added a comment. **NFC** //Typo-fix//: Remove empty line from end of diff. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106946/new/ https://reviews.llvm.org/D106946 Files: clang-tools-

[PATCH] D106431: [clang-tidy] Fix cppcoreguidelines-init-variables by removing the enum FixIt, and add support for initialization check of scoped enum.

whisperity added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:156 + + ⁣Added support for initialization check of the scoped enum + whisperity wrote: > Unterminated sequence. Also, where is this feature implemented? @Sockke Sorry if I'm ul

[PATCH] D106946: [clang-tidy] Fix crash on "reference to array" parameters in 'bugprone-easily-swappable-parameters'

whisperity added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.cpp:46 + +void crefToArrayTypedefBoth1(const Point2D &VecDescartes, const Point3D &VecThreeD) {} +// NO-WARN: Distinct types.

[PATCH] D106946: [clang-tidy] Fix crash on "reference to array" parameters in 'bugprone-easily-swappable-parameters'

whisperity updated this revision to Diff 362340. whisperity added a comment. Add a test case where the referenced arrays are deduced from template parameters. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106946/new/ https://reviews.llvm.org/D1069

[PATCH] D106946: [clang-tidy] Fix crash on "reference to array" parameters in 'bugprone-easily-swappable-parameters'

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG21832121e112: [clang-tidy] Fix crash on "reference-to-array" parameters in 'bugprone-easily… (authored by whisperity). Repository: rG LLVM Github

[PATCH] D106431: [clang-tidy] Fix cppcoreguidelines-init-variables by removing the enum FixIt, and add support for initialization check of scoped enum.

whisperity added a comment. Alright, thanks! So it does work that way, I didn't know. I think this is okay from my end. You can mark the inline discussion done, by the way. 🙂 Let's see if @aaron.ballman has any comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106431/new/ https:

[PATCH] D106431: [clang-tidy] Fix cppcoreguidelines-init-variables by removing the enum FixIt, and add support for initialization check of scoped enum.

whisperity added a comment. If we are tinkering with the code post-acceptance, could you please also change `const char *` to `StringRef`? They are the same thing essentially, but accessing the length is (AFIAK) //O(1)// and there isn't a //concerning// `strlen` call in the code. CHANGES SINC

[PATCH] D106431: [clang-tidy] Fix cppcoreguidelines-init-variables by removing the enum FixIt, and add support for initialization check of scoped enum.

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp:85 + if (TypePtr->isEnumeralType()) +InitializationString = ""; + else if (TypePtr->isIntegerType()) Also, if we're using //Optional//, this

[PATCH] D106431: [clang-tidy] Fix cppcoreguidelines-init-variables by removing the enum FixIt, and add support for initialization check of scoped enum.

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp:85 + if (TypePtr->isEnumeralType()) +InitializationString = ""; + else if (TypePtr->isIntegerType()) whisperity wrote: > Also, if we're usin

[PATCH] D89380: [clang-tidy] Fix for cppcoreguidelines-prefer-member-initializer to handle classes declared in macros

whisperity added a comment. In D89380#2913664 , @martong wrote: > @baloghadamsoftware Ping. Bug 47778 is marked //FIXED// by D97132 . CHANGES SINCE LAST ACTION https://revi

[PATCH] D121214: [clang-tidy][docs][NFC] Add alias cert-mem51-cpp to bugprone-shared-ptr-array-mismatch

whisperity added a comment. I have some concerns about this. While it is now clear to me that the //partial//ness refers to partial coverage of the guideline rule, it is indeed very, very partial. **MEM51-CPP** as of now lists **9** cases of non-compliant examples, from which `std::shared_ptr =

[PATCH] D121214: [clang-tidy][docs][NFC] Add alias cert-mem51-cpp to bugprone-shared-ptr-array-mismatch

whisperity added a comment. In D121214#3369871 , @steakhal wrote: > Drop the alias-related changes and preserve the note in the > `bugprone-shared-ptr-array-mismatch.rst` stating this relationship with the > cert rule? > If we do it like that, then this

[PATCH] D118370: [clang-tidy] bugprone-signal-handler: Message improvement and code refactoring.

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp:159-160 Itr != ItrE; ++Itr) { const auto *CallF = dyn_cast((*Itr)->getDecl()); -if (CallF && !isFunctionAsyncSafe(CallF)) { - assert(Itr.getPathLength(

[PATCH] D123992: [clang-tidy] Fix crash on calls to overloaded operators in llvmlibc-callee-namespace

whisperity created this revision. whisperity added reviewers: aaron.ballman, njames93, PaulkaToast. whisperity added a project: clang-tools-extra. Herald added subscribers: carlosgalvezp, martong, gamesh411, Szelethus, dkrupp, rnkovacs, xazax.hun. Herald added a project: All. whisperity requested

[PATCH] D114292: [clang-tidy] Fix `altera-struct-pack-align` check for empty structs

whisperity accepted this revision. whisperity added a comment. This revision is now accepted and ready to land. Herald added a project: All. @aaron.ballman Alright, I think this can go. The `ReleaseNotes.rst` needs a rebase anyway. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114292/ne

[PATCH] D123992: [clang-tidy] Fix crash on calls to overloaded operators in llvmlibc-callee-namespace

whisperity updated this revision to Diff 423595. whisperity added a comment. **[NFC]** Fix the test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123992/new/ https://reviews.llvm.org/D123992 Files: clang-tools-extra/clang-tidy/llvmlibc/CalleeNa

[PATCH] D123992: [clang-tidy] Fix crash on calls to overloaded operators in llvmlibc-callee-namespace

This revision was automatically updated to reflect the committed changes. Closed by commit rGf4834815f439: [clang-tidy] Fix crash on calls to overloaded operators in `llvmlibc-callee… (authored by whisperity). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D118996: [clang-tidy] Support C++14 in bugprone-signal-handler.

whisperity added a comment. After adding improvements to the documentation, I think this will be good to go, and thank you! Perhaps just for a safety measure you could run it on a few projects (LLVM itself?) to ensure we didn't miss a case where it might magically crash, but I wonder how many s

[PATCH] D123352: [analyzer] Add FixItHint to `nullability.NullReturnedFromNonnull` and `nullability.NullableReturnedFromNonnull`

whisperity added a comment. Regarding FixIts... FixIts are implemented in the "Diagnostic" library, which is non-specific to neither Clang-Tidy nor Sema whatsoever, they use the same infrastructure under the hood. Why the apply logic in CSA might do the same FixIt multiple times is beyond me, b

[PATCH] D123065: [clang-tidy] support --load in clang-tidy-diff.py/run-clang-tidy.py

whisperity added a comment. In D123065#3474107 , @bernhardmgruber wrote: > Ping. > > Can someone merge this please for me? Thank you! Yes, sure, just please specify what `user.name` and `user.email` would you like to be associated with the commit's aut

[PATCH] D123065: [clang-tidy] support --load in clang-tidy-diff.py/run-clang-tidy.py

whisperity added a comment. D'oh! I made a mistake when copying the URL and accidentally associated the commit with D12306 instead of D123065 ... Anyhow, this was committed in rGb1f1688e90b22dedc829f5abc9a912f18c034fbc

[PATCH] D124447: [clang-tidy] Add infrastructure support for running on project-level information

whisperity added a comment. In D124447#3493446 , @aaron.ballman wrote: > precommit CI is showing a fair amount of failures that I believe are related > to your patch. It was me who helped @bahramib to rebase and split a much larger and more convoluted

[PATCH] D124447: [clang-tidy] Add infrastructure support for running on project-level information

whisperity added a comment. In D124447#3645197 , @sammccall wrote: > In D124447#3608747 , @aaron.ballman > wrote: > >> In general, I think this is looking pretty close to good. Whether clang-tidy >> should get t

[PATCH] D124447: [clang-tidy] Add infrastructure support for running on project-level information

whisperity added a comment. In D124447#3651074 , @whisperity wrote: > I will attempt to write a concise response to this today (and tomorrow)! > Sorry, I was away travelling to and doing post-action bureaucracy of > conferences the past few weeks. @aa

[PATCH] D125383: [ASTMatchers][clang-tidy][NFC] Hoist 'forEachTemplateArgument' matcher into the core library

whisperity created this revision. whisperity added reviewers: aaron.ballman, klimek, hokein, njames93. whisperity added projects: clang, clang-tools-extra. Herald added subscribers: carlosgalvezp, gamesh411, Szelethus, dkrupp, rnkovacs, xazax.hun. Herald added a project: All. whisperity requested

[PATCH] D124446: [clang-tidy] Add the misc-discarded-return-value check

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/DiscardedReturnValueCheck.cpp:96-99 +AST_MATCHER_P(StaticAssertDecl, hasAssertExpr, + ast_matchers::internal::Matcher, InnerMatcher) { + return InnerMatcher.matches(*Node.getAssertExpr(

[PATCH] D124447: [clang-tidy] Add infrastructure support for running on project-level information

whisperity added a comment. In D124447#3493996 , @whisperity wrote: > In D124447#3493446 , @aaron.ballman > wrote: > >> precommit CI is showing a fair amount of failures that I believe are related >> to your pa

[PATCH] D91000: [clang-tidy] Add bugprone-unsafe-functions checker.

whisperity added a comment. Just one question if you could try this out for me: what happens if you run `clang-tidy a.c b.c` (two TUs in the invocation) where **one of them** (preferably the later one, i.e. **`b.c`**) does //NOT// have Annex K enabled? I believe the cached `IsAnnexKAvailable` (

[PATCH] D118996: [clang-tidy] Support C++14 in bugprone-signal-handler.

whisperity added a comment. @njames93 What do you think about the current approach? It will under-approximate the problem-inducing node set but at least cover what we know about C++ now. (@balazske please mark //"Done"// the answered discussion threads.) Comment at: clang-to

[PATCH] D124447: [clang-tidy] Add infrastructure support for running on project-level information

whisperity added a comment. In D124447#3506536 , @whisperity wrote: > In D124447#3493996 , @whisperity > wrote: > >> In D124447#3493446 , >> @aaron.ballman wrote: >> >>

[PATCH] D125209: [clang-tidy] modernize-deprecated-headers check should respect extern "C" blocks

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp:70-73 +auto ExternCBlockBegin = +SM.getDecomposedExpansionLoc(LinkSpecDecl->getBeginLoc()); +auto ExternCBlockEnd = +SM.getDecomposedExpansionL

[PATCH] D125209: [clang-tidy] modernize-deprecated-headers check should respect extern "C" blocks

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp:87-88 const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { -PP->addPPCallbacks( -::std::make_unique(*this, getLangOpts())); +

[PATCH] D125209: [clang-tidy] modernize-deprecated-headers check should respect extern "C" blocks

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp:135 {"float.h", "cfloat"}, {"limits.h", "climits"}, {"locale.h", "clocale"}, steakhal wrote: > whisperity wrote

[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

whisperity added inline comments. Comment at: clang/include/clang/AST/ASTImporterSharedState.h:83 + + void setNewDecl(Decl *ToD) { NewDecls.insert(ToD); } }; (The naming of this function feels a bit odd. `markAsNewDecl` or just `markNewDecl`?) ==

[PATCH] D125209: [clang-tidy] modernize-deprecated-headers check should respect extern "C" blocks

whisperity added a comment. LGTM. Some typos inline. Also I think this is a significant enough bug-fix that it warrants an entry in the Release Notes under `Changes to existing checks`. Also, the tests are failing, but it fails on the formatting of the test file (???), and not the actual test i

[PATCH] D125209: [clang-tidy] modernize-deprecated-headers check should respect extern "C" blocks

whisperity added a comment. (Also: this is a fix to an issue of your own finding and not something that was reported on Bugzilla? Just to make sure we cross-reference and close tickets.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125209/new/ ht

[PATCH] D125383: [ASTMatchers][clang-tidy][NFC] Hoist 'forEachTemplateArgument' matcher into the core library

whisperity added a comment. In D125383#3508960 , @aaron.ballman wrote: > Do you expect to use this matcher in a new check in the immediate future? D124446 would like to use it. Repository: rG LLVM Github Monorepo

[PATCH] D125383: [ASTMatchers][clang-tidy][NFC] Hoist 'forEachTemplateArgument' matcher into the core library

whisperity updated this revision to Diff 429161. whisperity added a comment. - Added to `ASTMatchers/Registry.cpp` - Updated with release notes CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125383/new/ https://reviews.llvm.org/D125383 Files: clang-tools-extra/clang-tidy/misc/UnusedUs

[PATCH] D125383: [ASTMatchers][clang-tidy][NFC] Hoist 'forEachTemplateArgument' matcher into the core library

whisperity added a comment. In D125383#3509084 , @aaron.ballman wrote: > Then I think the only thing missing here are updates to > https://github.com/llvm/llvm-project/blob/main/clang/lib/ASTMatchers/Dynamic/Registry.cpp. Could you please check if I di

[PATCH] D125383: [ASTMatchers][clang-tidy][NFC] Hoist 'forEachTemplateArgument' matcher into the core library

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9add949557d2: [ASTMatchers][clang-tidy][NFC] Hoist `forEachTemplateArgument` matcher into the… (authored by whisperity). Changed prior to commit:

[PATCH] D124446: [clang-tidy] Add the misc-discarded-return-value check

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/DiscardedReturnValueCheck.cpp:83-86 +void DiscardedReturnValueCheck::onStartOfTranslationUnit() { + ConsumedCalls.clear(); + CallMap.clear(); +} aaron.ballman wrote: > Is this code

[PATCH] D124446: [clang-tidy] Add the misc-discarded-return-value check

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/DiscardedReturnValueCheck.cpp:181 + + static const auto Decltype = decltypeType(hasUnderlyingExpr(Call)); + static const auto TemplateArg = whisperity wrote: > whisperity wrote: > >

[PATCH] D125771: [clang-tidy] Add a useful note about -std=c++11-or-later

whisperity added a comment. In D125771#3519606 , @LegalizeAdulthood wrote: > I thought there wasn't any support for validating fixits applied to header > files? It is not specifically about the fixits, but diagnostics as a whole. It was not clear that

[PATCH] D125769: [clang-tidy] Introduce the WarnIntoHeaders option to modernize-deprecated-headers

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.h:63 std::vector IncludesToBeProcessed; + bool WarnIntoHeaders; }; LegalizeAdulthood wrote: > 1) How is this different from the clang-tidy option that

[PATCH] D124446: [clang-tidy] Add the misc-discarded-return-value check

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/DiscardedReturnValueCheck.cpp:181 + + static const auto Decltype = decltypeType(hasUnderlyingExpr(Call)); + static const auto TemplateArg = aaron.ballman wrote: > aaron.ballman wrot

[PATCH] D124446: [clang-tidy] Add the misc-discarded-return-value check

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/DiscardedReturnValueCheck.cpp:181 + + static const auto Decltype = decltypeType(hasUnderlyingExpr(Call)); + static const auto TemplateArg = whisperity wrote: > aaron.ballman wrote:

[PATCH] D124446: [clang-tidy] Add the misc-discarded-return-value check

whisperity added a comment. In D124446#3521619 , @whisperity wrote: > @aaron.ballman [...] I think I can put in a measurement job [...] I've rerun the experiment **on another computer** (compared to the results shown in the inline comments) from scratc

[PATCH] D66333: [analyzer] NonNullParamChecker and CStringChecker parameter number in checker message

whisperity accepted this revision. whisperity added a comment. @Szelethus Nope. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66333/new/ https://reviews.llvm.org/D66333 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D34512: Add preliminary Cross Translation Unit support library

whisperity added a comment. Apart from those in the in-line comments, I have a question: how safe is this library to `Release` builds? I know this is only a submodule dependency for the "real deal" in https://reviews.llvm.org/D30691, but I have seen some asserts that "imported function should a

[PATCH] D34512: Add preliminary Cross Translation Unit support library

whisperity added inline comments. Comment at: include/clang/CrossTU/CrossTranslationUnit.h:42 +/// Note that this class also implements caching. +class CrossTranslationUnit { +public: xazax.hun wrote: > whisperity wrote: > > Does the name of this class make sense

[PATCH] D71963: clang-tidy doc: Add the severities description

whisperity added a comment. In D71963#1798199 , @vingeldal wrote: > - Style: things that are handled by clang-format rather than clang-tidy. This is not true, for two reasons. The shorter answer: In case it was true, the "severity category" `STYLE` woul

[PATCH] D71963: clang-tidy doc: Add the severities description

whisperity added a comment. @vingeldal Apologies, in that case. However, I would still claim that `style` (as a //potential// severity) has its purpose for Tidy checkers, not just for `clang-format`. In D71963#1798871 , @vingeldal wrote: > If severity

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

whisperity planned changes to this revision. whisperity added a comment. There are a few really minor bug fixes, test additions, documentation update, etc. coming along soon, but I've some more pressing matters. However, please feel free to review the patch as-is! CHANGES SINCE LAST ACTION h

[PATCH] D87527: [ASTMatchers] Fix `hasBody` for the descendants of `FunctionDecl`

whisperity added a comment. What about http://clang.llvm.org/doxygen/classclang_1_1CXXConversionDecl.html ? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87527/new/ https://reviews.llvm.org/D87527 ___ cf

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

whisperity added a comment. Minor inline comments. Could you please add a test case where the constructor is defined out of line? So far in every test, the constructor //body// was inside the class. Something like: struct S { int i, j; S(); }; S::S() { i = 1; j = 2;

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

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp:43 +static bool isLiteral(const Expr *E) { + return isa(E) || + isa(E) || baloghadamsoftware wrote: > aaron.ballman wrote: > >

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

whisperity added a comment. I'd like to resurrect the discussion about this. Unfortunately, the concept of `experimental-` group (in D76545 ) has fallen silent, too. We will present the results of this analysis soon at IEEE SCAM (Source Code Analysis and Manipul

[PATCH] D84176: [CMake][CTE] Add "check-clang-extra-..." targets to test only a particular Clang extra tool

whisperity added a comment. Ping. Can this go in? Still using this on a local fork, and still feels nice to be able to specify just a single tool. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84176/new/ https://reviews.llvm.org/D84176 ___ c

[PATCH] D84176: [CMake][CTE] Add "check-clang-extra-..." targets to test only a particular Clang extra tool

This revision was automatically updated to reflect the committed changes. Closed by commit rG9d2ef5e74eea: [CMake][CTE] Add "check-clang-extra-..." targets to test only a particular… (authored by whisperity). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes

whisperity added a comment. Some grammatical fixes and suggestions, inline. I might have absolutely butchered 80-col in the suggestions (thanks Phab for not showing any indication of line length...), so make sure you manually reformat the document before going forward! Comme

[PATCH] D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes

whisperity added inline comments. Comment at: clang/docs/ReleaseNotes.rst:439 - ... Static Analyzer ... here. Comment at: clang/docs/ReleaseNotes.rst:498 .. _release-notes-ubsan: @Szelethus Speaking of labels in the de

<    1   2   3   4   5   6   >