[PATCH] D146712: [clang-tidy] Add portability-non-portable-integer-constant check

whisperity requested changes to this revision. whisperity added a comment. Earlier (IIRC in March) we did an internal test of the check and the following results were obtained. The results are kinda //weird//, to say the least. Numbers are //after// CodeChecker has done a serverside uniqueing

[PATCH] D140562: [clang][ASTImporter] Improve import of InjectedClassNameType.

whisperity added a subscriber: vabridgers. whisperity added a comment. @vabridgers Please take a look at this, as per off-list discussion, in relation to D142822 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D140562: [clang][ASTImporter] Improve import of InjectedClassNameType.

whisperity added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:4627 } else { -Type *newType = - new (*this, TypeAlignment) InjectedClassNameType(Decl, TST); +Type *newType = new (*this, TypeAlignment) InjectedClassNameType(Decl, TST);

[PATCH] D142822: [clang] ASTImporter: Fix importing of va_list types and declarations

whisperity added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cert/dcl58-cpp.cpp:5-6 + +// On arm and arch64 architectures, va_list is within the std namespace. +// D136886 exposed an issue in the cert-dcl58-cpp checker where +// implicit namespaces

[PATCH] D142822: [clang] ASTImporter: Fix importing of va_list types and declarations

whisperity added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cert/dcl58-cpp.cpp:3 +// RUN: %check_clang_tidy -std=c++17-or-later %s cert-dcl58-cpp %t -- -- -I %clang_tidy_headers -target aarch64 +// RUN: %check_clang_tidy -std=c++17-or-later %s

[PATCH] D142822: [clang] ASTImporter: Fix importing of va_list types and declarations

whisperity added subscribers: rsmith, whisperity. whisperity added a comment. In D142822#4095896 , @DavidSpickett wrote: > > /home/david.spickett/llvm-project/clang/test/CodeGenCXX/cxx20-module-std-subst-2b.cpp:14:18: > error: reference to 'std' is

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

whisperity added a comment. In D91000#4099773 , @dyung wrote: > To XFAIL the test, grep for lines with "XFAIL" and "ps4" and you should find > some examples. It was recently changed how it worked lately. Thank you, yes I found an example. I went with

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

whisperity added a comment. In D91000#4099669 , @dyung wrote: > -DLLVM_DEFAULT_TARGET_TRIPLE=x86_64-scei-ps4 > > Hopefully this can help you to reproduce the problem. Ah, thank you very much. And indeed, giving `--target=x86_64-scei-ps4` to Clang-Tidy

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

whisperity added a subscriber: dyung. whisperity added a comment. Ping @dyung, it looks like you're the owner of the relevant build-bot. I can't find any information what `x86_64-sie` is... Meanwhile, it seems my attempt at fix is not actually fixing anything, I'm trying to figure out how to

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

whisperity added a comment. Alright, right now, I have no meaningful idea why this failure appears , locally I'm trying every test command as they appear in the test, and all the tests

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

whisperity added a comment. Oddly enough, one of the buildbots caught a not matching test that was working locally... on it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91000/new/ https://reviews.llvm.org/D91000

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

This revision was automatically updated to reflect the committed changes. Closed by commit rGf27c8ac83e7c: [clang-tidy] Add the `bugprone-unsafe-functions` check (authored by futogergely, committed by whisperity). Changed prior to commit: https://reviews.llvm.org/D91000?vs=485774=494265#toc

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

whisperity accepted this revision. whisperity added a comment. This revision is now accepted and ready to land. Alright, I've done a full reanalysis after a rebase. Overhead is not meaningfully measurable, even for complex TUs such as LLVM. The check is viable in C++ code as it finds cases

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

whisperity added a comment. Alright, the tests came back from the CI positively and there were no crashes on like 15 projects (about 50:50 C and C++). There are some nice results: - In LLVM, in `LexPPMacroExpansion.cpp` `ExpandBuiltinMacro()` there is a hit for a call `Result = asctime(TM);`.

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

whisperity added a comment. Tentative **LGTM**, will still need to run the "usual test projects" in the CI for confirmation. What's missing from this patch are the `.rst` files for the CERT-specific aliases of the check, but those are trivial and we can "add them in post" anyway. CHANGES

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

whisperity added a comment. In D91000#4020891 , @futogergely wrote: > What we could do is: > > 1. add a new checker option to decide if we suggest replacements from AnnexK. > We could avoid registering matchers this way, but I don't really like this, >

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

whisperity added a comment. In D91000#3861942 , @aaron.ballman wrote: > My concern with that approach was that we pay the full expense of doing the > matches only get get into the `check()` function to bail out on all the Annex > K functions, but now

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

whisperity added a comment. In D91000#3770071 , @whisperity wrote: > In D91000#3197851 , @aaron.ballman > wrote: > >> In terms of whether we should expose the check in C++: I think we shouldn't. >> [...] >> >> I

[PATCH] D133119: [clang-tidy] Add checker 'bugprone-suspicious-realloc-usage'.

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SuspiciousReallocUsageCheck.cpp:89-90 +return; + diag(Call->getBeginLoc(), + "memory block passed to 'realloc' may leak if 'realloc' fails"); +} whisperity wrote: > I

[PATCH] D133119: [clang-tidy] Add checker 'bugprone-suspicious-realloc-usage'.

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SuspiciousReallocUsageCheck.cpp:89-90 +return; + diag(Call->getBeginLoc(), + "memory block passed to 'realloc' may leak if 'realloc' fails"); +} I have some

[PATCH] D133119: [clang-tidy] Add checker 'bugprone-suspicious-realloc-usage'.

whisperity added a comment. I agree with @martong that `$ = realloc($, ...)` is indicative of something going wrong, even if the developer has saved the original value somewhere. Are we trying to catch this with Tidy specifically for this reason (instead of CSA's stdlib checker, or some

[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] 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

[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

[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

[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.

[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

[PATCH] D112916: [clang-tidy] Confusable identifiers detection

whisperity accepted this revision. whisperity added a comment. This revision is now accepted and ready to land. LGTM. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112916/new/ https://reviews.llvm.org/D112916 ___ cfe-commits mailing

[PATCH] D126891: [clang-tidy] The check should ignore final classes

whisperity added a comment. (Please ensure a more appropriate commit message that actually mentions the check when committing, @steakhal.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126891/new/ https://reviews.llvm.org/D126891

[PATCH] D112916: [clang-tidy] Confusable identifiers detection

whisperity requested changes to this revision. whisperity added a comment. This revision now requires changes to proceed. Because of the stability issues related to `getName()`-like constructs I'm putting a temporary ❌ on this (so it doesn't show up as faux accept). However, I have to emphasise

[PATCH] D112916: [clang-tidy] Confusable identifiers detection

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/Homoglyph.cpp:32-35 +/** + * Build a skeleton out of the Original identifier, following the algorithm + * described in http://www.unicode.org/reports/tr39/#def-skeleton + */ `/*` ->

[PATCH] D126247: [clang-tidy][doc] Document readability-indentifier-naming resolution order and examples

whisperity added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst:2750-2752 +where an individual identifier can fall into several classifications. Below +is a list of the classifications supported by

[PATCH] D126891: [clang-tidy] The check should ignore final classes

whisperity accepted this revision. whisperity added a comment. This revision is now accepted and ready to land. Thank you! Comment at: clang-tools-extra/docs/ReleaseNotes.rst:170 + ` involving + ``final`` classes. The check will not diagnose ``final`` marked classes, since

[PATCH] D127599: [clang] small speed improvement of Sema::AddArgumentDependentLookupCandidates

whisperity added a comment. The context of the diff is missing. Please re-run the diff making with `-U999`. Comment at: clang/include/clang/Sema/Lookup.h:817 + bool empty(void) { return Decls.empty(); } + using iterator = `(void)`? LLVM is

[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

[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 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

[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

[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

[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] 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

[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] 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

[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:

[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] 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/

[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

[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 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

[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 , 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:70-73 +auto ExternCBlockBegin = +SM.getDecomposedExpansionLoc(LinkSpecDecl->getBeginLoc()); +auto ExternCBlockEnd = +

[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] 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:

[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] 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

[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

[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] 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

[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] 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

[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,

[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

[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

[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:

[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

[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] 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.

[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] 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

[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)) { -

[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

[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. Is "partial aliasing" a new notion? Or... is the alias not partial, but the coverage? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121214/new/ https://reviews.llvm.org/D121214

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

whisperity added a reviewer: whisperity. whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp:142-149 Finder->addMatcher( callExpr(callee(SignalFunction), hasArgument(1, HandlerExpr))

[PATCH] D120555: [clang-tidy] Fix `readability-suspicious-call-argument` crash for arguments without name-like identifier

whisperity added a comment. In D120555#3345707 , @njames93 wrote: > If you backport, the release notes change on trunk should then be reverted. Release Notes entry of current development tree reverted in rG94850918274c20c15c6071dc52314df51ed2a357

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

whisperity added a reviewer: whisperity. whisperity added a comment. Herald added a subscriber: rnkovacs. This generally looks good, and thank you! I have a few minor comments (mostly presentation and documentation). Comment at:

[PATCH] D120555: [clang-tidy] Fix `readability-suspicious-call-argument` crash for arguments without name-like identifier

whisperity added a comment. In D120555#3345707 , @njames93 wrote: > If you backport, the release notes change on trunk should then be reverted. Indeed, this is not the first time I mess this up... Ugh. I'll wait a little bit for that to make sure the

[PATCH] D120555: [clang-tidy] Fix `readability-suspicious-call-argument` crash for arguments without name-like identifier

whisperity added a comment. Thanks! I'll re-run the tests on top of **14.0** and do the backport too, soon.  Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120555/new/ https://reviews.llvm.org/D120555

[PATCH] D120555: [clang-tidy] Fix `readability-suspicious-call-argument` crash for arguments without name-like identifier

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG416e689ecda6: [clang-tidy] Fix `readability-suspicious-call-argument` crash for arguments… (authored by whisperity). Repository: rG LLVM Github

[PATCH] D120555: [clang-tidy] Fix `readability-suspicious-call-argument` crash for arguments without name-like identifier

whisperity updated this revision to Diff 411413. whisperity added a comment. Added //Release notes// entry. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120555/new/ https://reviews.llvm.org/D120555 Files:

[PATCH] D120555: [clang-tidy] Fix `readability-suspicious-call-argument` crash for arguments without name-like identifier

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

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

whisperity added a reviewer: whisperity. whisperity added a comment. Herald added a subscriber: rnkovacs. Bump! Thanks @futogergely for picking up @ktomi996's work, and @steakhal for the help given during the implementation process! In D91000#3231417 ,

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

This revision was automatically updated to reflect the committed changes. Closed by commit rG3696c70e67d9: [clang-tidy] Add `readability-container-contains` check (authored by avogelsgesang, committed by whisperity). Changed prior to commit:

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

whisperity accepted this revision. whisperity added a comment. In D112646#3265670 , @whisperity wrote: > Please specify what name and e-mail address you'd like to have the Git commit > author attributed with! Nevermind, I forgot that you commented

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

whisperity added a comment. In D112646#3251025 , @avogelsgesang wrote: > @xazax.hun Can you please merge this to `main` on my behalf? (I don't have > commit rights) Hey! Sorry for my absence, I'm terribly busy with doing stuffs  I'll check the patch

[PATCH] D91000: [clang-tidy] Add cert-msc24-c checker.

whisperity added a comment. Should/does this work in C++ mode for `std::whatever`? Comment at: clang-tools-extra/clang-tidy/cert/ObsolescentFunctionsCheck.cpp:48 + // Matching functions with safe replacements in annex K. + auto FunctionNamesWithAnnexKReplacementMatcher =

[PATCH] D114197: [clang-tidy] Fix false positives involving type aliases in `misc-unconventional-assign-operator` check

whisperity added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator.cpp:151 + using Alias3 = TemplateTypeAlias; + Alias3 =(int) { return *this; } +}; This is a no-warn due to the parameter being a

[PATCH] D114622: [clang-tidy][analyzer] Fix false-positive in IdenticalExprChecker and misc-redundant-expression

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp:54-55 const NestedNameSpecifier *Right) { - llvm::FoldingSetNodeID LeftID, RightID; - Left->Profile(LeftID); -

[PATCH] D114622: [clang-tidy][analyzer] Fix false-positive in IdenticalExprChecker and misc-redundant-expression

whisperity added a comment. I haven't looked at the patch in detail yet, but I know I've run into the issue this aims to fix when developing a Tidy check! There is a `// NOLINTNEXTLINE(misc-redundant-expression)` directive in the source file

[PATCH] D114602: [clang-tidy] Improve documentation of bugprone-unhandled-exception-at-new [NFC]

whisperity added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-exception-at-new.rst:8-10 Calls to ``new`` can throw exceptions of type ``std::bad_alloc`` that should be handled by the code. Alternatively, the nonthrowing form of

[PATCH] D113804: [clang-tidy] Fix behavior of `modernize-use-using` with nested structs/unions

whisperity added a comment. `hasParent()` is the //direct// upwards traversal, right? I remember some discussion about matchers like that being potentially costly. But I can't find any description of this, so I guess it's fine, rewriting the matcher to have the opposite logic

[PATCH] D113518: [clang][Sema] Create delegating constructors even in templates

whisperity added a comment. In D113518#3155040 , @carlosgalvezp wrote: > Thanks for the finding and sorry for delaying the review, I didn't know that > :( Do you know if there's an option for signaling "LGTM but I want someone > else to review?".

[PATCH] D113518: [clang][Sema] Create delegating constructors even in templates

whisperity edited reviewers, added: Quuxplusone; removed: carlosgalvezp, whisperity. whisperity added subscribers: carlosgalvezp, whisperity. whisperity added a comment. This revision now requires review to proceed. (@carlosgalvezp Removing you from reviewers so the patch shows up as not yet

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

whisperity requested changes to this revision. whisperity added a comment. This revision now requires changes to proceed. I believe this is worth a note in the `ReleaseNotes.rst` file. People who might have disabled the check specifically for the reason outlined in the PR would get to know it's

[PATCH] D107450: [clang-tidy] Fix wrong FixIt in performance-move-const-arg

whisperity added a comment. This generally looks fine for me, but I'd rather have other people who are more knowledgeable about the standard's intricacies looked at it too. AFAIK Aaron is busy this week and the next (?) due to C++ Committee meetings, or something like that. And I bet after

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

whisperity added a comment. Thank you, this is getting much better! Comment at: clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp:105-108 + const auto *PositiveCheck = Result.Nodes.getNodeAs("positive"); + const auto *NegativeCheck =

[PATCH] D113558: [clang-tidy] Fix cppcoreguidelines-virtual-base-class-destructor in macros

whisperity accepted this revision. whisperity added a comment. I'm not sure if it is a good idea to merge things on a Friday, but I am comfortable with this, let's get the check crash less. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113558/new/ https://reviews.llvm.org/D113558

[PATCH] D114254: [libtooling][clang-tidy] Fix crashing on rendering invalid SourceRanges

whisperity added inline comments. Comment at: clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp:102 + EXPECT_EQ("invalid->invalid", Errors[0].Message.Message); + EXPECT_EQ(0ul, Errors[0].Message.Ranges.size()); +

[PATCH] D114149: [clang-tidy] Fix pr48613: "llvm-header-guard uses a reserved identifier"

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp:42-46 +/// Remove all '_' characters at the beginning of the identifier. Only reserved +/// identifiers are allowed to start with these. +static StringRef

[PATCH] D113251: [analyzer][doc] Add user documenation for taint analysis

whisperity added inline comments. Comment at: clang/docs/analyzer/checkers.rst:2341-2342 + +Default propagations defined by `GenericTaintChecker`: +``atoi``, ``atol``, ``atoll``, ``fgetc``, ``fgetln``, ``fgets``, ``fscanf``, ``sscanf``, ``getc``, ``getc_unlocked``,

  1   2   3   4   5   6   >