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

whisperity added a comment. Bump. 🙂 I would prefer this check to be able to go into Clang-Tidy 13 if it's okay, together with the other check I implemented. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D20689/new/ https://reviews.llvm.org/D20689

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

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp:140 + diag(InvocationParm->getLocation(), + "consider changing the %0st parameter of %1 from %2 to 'const %3 &'", + DiagnosticIDs::Note)

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

whisperity added a comment. I think this is becoming okay and looks safe enough. I can't really grasp what `HasCheckedMoveSet` means, and how it synergises with storing `CallExpr`s so maybe a better name should be added. Do you mean `AlreadyCheckedMoves`? When is it possible that the same `Call

[PATCH] D112334: [clang-tidy] Suppress readability-static-access-through-instance for CUDA built-in variables

whisperity added a comment. In D112334#3084533 , @aaron.ballman wrote: > In D112334#3081213 , @carlosgalvezp > wrote: > >> Btw, regarding this `CHECK-MESSAGES-NOT`, how does it work? I can't find it >> in `chec

[PATCH] D112334: [clang-tidy] Suppress readability-static-access-through-instance for CUDA built-in variables

whisperity closed this revision. whisperity added a comment. @carlosgalvezp Force pushes are prevented by the serverside integration so we couldn't force push even if we wanted to. What one could do is to revert the patch and land it again (or land an empty patch that has the association line i

[PATCH] D46081: [analyzer] Expand conversion check to check more expressions for overflow and underflow

whisperity added reviewers: martong, steakhal. whisperity added a subscriber: balazske. whisperity added a comment. Herald added subscribers: manas, ASDenysPetrov, donat.nagy. Pinging my people here, perhaps you being more knowledgeable in the analyser can say if this patch is salvageable in any

[PATCH] D112596: [clang-tidy] Correct typo in bugprone-easily-swappable-parameters

whisperity added a comment. 100% typo. 😂 Aww 'chute... Bunch of rebases. Yeah I didn't add extensive tests for simply the configuration, only for the configurations working. Did you check the documentation file? It should mention the default values for the checker configuration options. Does it

[PATCH] D112596: [clang-tidy] Correct typo in bugprone-easily-swappable-parameters

whisperity accepted this revision. whisperity added a comment. This revision is now accepted and ready to land. Perfect! Have you obtained commit rights already, or should I go ahead and commit this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D11

[PATCH] D112409: [clang-tidy] Add check 'cert-err33-c'.

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp:45-50 +// The following functions are +// deliberately excluded because they can +// be called with NULL argument and in +// this case the check is not applicable: +// mblen, mbrlen

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

whisperity added reviewers: aaron.ballman, xazax.hun. whisperity added a comment. Herald added a subscriber: rnkovacs. Why `readability-`, if the intent is to make users move to a newer API? @xazax.hun I think you did something similar wrt. `empty()`, right? Could you take a look at this? ===

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

whisperity added a comment. Okay. I think I am convinced. And removing a bogus automated fix is always a positive change! Comment at: clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp:144 + << (InvocationParm->getFunctionScopeIndex() + 1) + << Re

[PATCH] D112730: [clang-tidy] Add AUTOSAR module

whisperity added a comment. "C++14 Guidelines"? So it's always and definitely C++14 specific? What is the licencing approach of this guideline? Looking up with searchers seems to turn up a few PDFs which say `--- AUTOSAR CONFIDENTIAL ---`, yet they are up and out on the official-looking website

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

whisperity added a comment. In D108893#3111674 , @steakhal wrote: > It seems like the checker is documented as `readability-data-pointer` but in > the tests it reports issues under the `readability-container-data-pointer` > name. > Shouldn't they be the

[PATCH] D108893: clang-tidy: introduce readability-containter-data-pointer check

whisperity added a comment. In D108893#3118389 , @whisperity wrote: > In D108893#3111674 , @steakhal > wrote: > >> It seems like the checker is documented as `readability-data-pointer` but in >> the tests it re

[PATCH] D75041: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with mixability because of implicit conversions

whisperity updated this revision to Diff 327743. whisperity retitled this revision from "[clang-tidy] Approximate implicit conversion issues in 'experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type'" to "[clang-tidy] Extend 'bugprone-easily-swappable-parameters' with mixabil

[PATCH] D75041: [clang-tidy] Extend 'bugprone-easily-swappable-parameters' with mixability because of implicit conversions

whisperity updated this revision to Diff 327761. whisperity added a comment. After reading through the diff as rendered by Phabricateur, I realised I left in a few crappy debug prints and commented-out values that are not needed in the end. This is fixed now. CHANGES SINCE LAST ACTION https:

[PATCH] D69560: [clang-tidy] Add 'bugprone-easily-swappable-parameters' check

whisperity added a comment. @aaron.ballman I've reworked and uploaded the final addition to this line, the crown jewel, the very reason this checker was developed (D75041 ). So, AFAIK, the "let's rewrite the whole thing with better code quality" work can be cons

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

whisperity updated this revision to Diff 329666. whisperity edited the summary of this revision. whisperity added reviewers: hokein, njames93. whisperity added a comment. - Massively refactored and modernised the implementation - Removed spaghetti code related to the check options and made their s

[PATCH] D98635: [libtooling][clang-tidy] Fix diagnostics not handlings SourceRange highlights

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

[PATCH] D98635: [libtooling][clang-tidy] Fix diagnostics not handlings SourceRange highlights

whisperity updated this revision to Diff 330687. whisperity added a comment. **NFC** I originally wrote a Release Notes entry about this, which I forgot to commit before I generated the patch file I uploaded, this is fixed now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D98635: [libtooling][clang-tidy] Fix diagnostics not respecting and highlighting fed SourceRanges

whisperity added a comment. In D98635#2626455 , @aaron.ballman wrote: > It looks like there are some unit test failures from the change: > https://buildkite.com/llvm-project/premerge-checks/builds/29754#a9b04ab5-e326-4ff1-beea-773f21a33349 Strange becau

[PATCH] D98635: [libtooling][clang-tidy] Fix diagnostics not respecting and highlighting fed SourceRanges

whisperity added a comment. In D98635#2626464 , @whisperity wrote: > Strange because I specifically ran both `check-clang` **and** > `check-clang-tools` locally, but will look into this. Turns out there is a `check-clang-extra-unit` too, and it looks lik

[PATCH] D98635: [libtooling][clang-tidy] Fix diagnostics not respecting and highlighting fed SourceRanges

whisperity updated this revision to Diff 330737. whisperity edited the summary of this revision. whisperity added a subscriber: compositeprimes. whisperity added a comment. Fixed a test file that I originally missed to align with the changes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/

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

whisperity accepted this revision. whisperity added a comment. This revision is now accepted and ready to land. The fix is concise, thank you for observing and untangling the crash! As the check was originally introduced in the ongoing release, we can go without a release notes entry. ===

[PATCH] D64454: [clang-tidy] Adding static analyzer check to list of clang-tidy checks

whisperity added a comment. In D64454#3124423 , @carlosgalvezp wrote: > I was a bit confused as well about the ClangSA support in clang-tidy. What's > the current status? Is clang-tidy running *all* checks from StaticAnalyzer? If your package is built w

[PATCH] D113863: [clang-tidy] Make `readability-container-data-pointer` more robust

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp:20-23 +static const char ContainerExprName[] = "container-expr"; +static const char DerefContainerExprName[] = "deref-container-expr"; +static const char AddrofCo

[PATCH] D113848: [clang-tidy][NFC] Refactor ClangTidyDiagnosticConsumer files

whisperity added a comment. What is the motivation behind the change? Can we be sure that these classes that were moved out weren't details to the context itself, and are understandable, usable, and can be relied upon without the context object? In D113848#3130542

[PATCH] D113847: [clang-tidy][NFC] Simplify ClangTidyStats

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h:50 struct ClangTidyStats { - ClangTidyStats() - : ErrorsDisplayed(0), ErrorsIgnoredCheckFilter(0), ErrorsIgnoredNOLINT(0), -ErrorsIgnoredNonUserCode(0), Errors

[PATCH] D113201: [clang-tidy] Fix a crash in modernize-loop-convert around conversion operators

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp:155-167 +/// Look through conversion/copy constructors and operators to find the explicit /// initialization expression, returning it is found. /// /// The main idea is t

[PATCH] D113148: Add new clang-tidy check for string_view(nullptr)

whisperity added a comment. In D113148#3109190 , @CJ-Johnson wrote: > In D113148#3108993 , @aaron.ballman > wrote: > >> Generally speaking, we prefer to improve the existing checks. I think >> `bugprone-string-

[PATCH] D111100: enable plugins for clang-tidy

whisperity added a comment. Wait... Is the diff //really// supposed to be this small? You didn't include code... that one header takes care of **everything**? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D00/new/ https://reviews.llvm.org/D

[PATCH] D113422: [clang-tidy][NFC] Move CachedGlobList to GlobList.h

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/GlobList.h:50-52 +/// A \p GlobList that caches search results, so that search is performed only +/// once for the same query. +class CachedGlobList { If `CachedGlobList` //is-a// `GlobLis

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

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h:27 SourceLocation LastReplacementEnd; - SourceRange LastTagDeclRange; + std::map LastTagDeclRanges; + Considering the pointer is just a number that hashes we

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

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp:49 + + // Find containment checks which use `count` + Finder->addMatcher(implicitCastExpr(hasImplicitDestinationType(booleanType()), Same comment

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

whisperity added a comment. In D112646#3104236 , @avogelsgesang wrote: > So I guess the name would have to be `container-count-begin-end-contains` or > similar... which would be a bit much in my opinion Yeah, that would be too much indeed. However, co

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

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp:105-108 + const auto *PositiveCheck = Result.Nodes.getNodeAs("positive"); + const auto *NegativeCheck = Result.Nodes.getNodeAs("negative"); + bool Negated = Negat

[PATCH] D114105: [clang-tidy] Ignore narrowing conversions in case of bitfields

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:87 + // Reading a small unsigned bitfield member will be wrapped by an implicit + // cast to 'int' triggering this checker. But this is known to be safe b

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

[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 dropLeadingUnderscores(Strin

[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()); + `EXPECT_TRUE(Errors[0]

[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] 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 = Result.Nodes.getNodeAs(

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

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

[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?". Other

[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 (`decl(hasDescend

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

[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 `clang-tools-extra/clang-tidy/bugprone/EasilySwappab

[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); - Right->Profile(Ri

[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 &operator=(int) { return *this; } +}; This is a no-warn due to the parameter being

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

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

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

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

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

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

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

[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);`. T

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

[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&id=494265#toc

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

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

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

[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] 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] 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 ``readability-identifier

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

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

[PATCH] D76541: [clang-tidy][docs][NFC] Check group doc line was missing from darwin & linuxkernel

whisperity created this revision. whisperity added a reviewer: aaron.ballman. whisperity added a project: clang-tools-extra. Herald added subscribers: cfe-commits, martong, gamesh411, Szelethus, arphaman, dkrupp, rnkovacs, xazax.hun. Herald added a project: clang. Two directories for Tidy checks

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

whisperity added a comment. @aaron.ballman I've gone over LLVM (and a few other projects). Some general observations: - Length of `2` **is vile**. I understand that the C++CG rule says even lengths of 2 should be matched, but that is industrially infeasible unless one introduces such a rule in

[PATCH] D76541: [clang-tidy][NFC] Add missing check group docs and order entries

whisperity updated this revision to Diff 251839. whisperity retitled this revision from "[clang-tidy][docs][NFC] Check group doc line was missing from darwin & linuxkernel" to "[clang-tidy][NFC] Add missing check group docs and order entries". whisperity edited the summary of this revision. whisp

[PATCH] D76545: [clang-tidy] Add a new check group 'experimental-'

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

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

whisperity updated this revision to Diff 251841. whisperity retitled this revision from "[clang-tidy] Add 'cppcoreguidelines-avoid-adjacent-arguments-of-same-type' check" to "[clang-tidy] Add 'experimental-cppcoreguidelines-avoid-adjacent-arguments-of-the-same-type' check". whisperity edited th

[PATCH] D76545: [clang-tidy] Add a new check group 'experimental-'

whisperity added a comment. In D76545#1935603 , @aaron.ballman wrote: > I think we want to keep the experimental checks grouped to their parent > module rather than being in a module of their own. For this, wouldn't the fact that telling Tidy to enable

[PATCH] D76541: [clang-tidy][NFC] Add missing check group docs and order entries

whisperity added a comment. In D76541#1935163 , @njames93 wrote: > Forgive me if I'm wrong, but these kinds of changes typically don't require a > review. Ah, pardon me. This is the first time I'm touching the insides of Tidy. Repository: rG LLVM Gi

[PATCH] D76541: [clang-tidy][NFC] Add missing check group docs and order entries

This revision was automatically updated to reflect the committed changes. Closed by commit rGdf5fa4873976: [clang-tidy][NFC] Add missing check group docs and order entries (authored by whisperity). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76541/

[PATCH] D76545: [clang-tidy] Add a new check group 'experimental-'

whisperity added a comment. @aaron.ballman @njames93 Should I write up a pitch mail to cfe-dev about this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76545/new/ https://reviews.llvm.org/D76545 ___ c

[PATCH] D75041: [WIP][clang-tidy] Approximate implicit conversion issues for the 'cppcoreguidelines-avoid-adjacent-arguments-of-same-type' check

whisperity created this revision. whisperity added a project: clang-tools-extra. Herald added subscribers: cfe-commits, martong, gamesh411, Szelethus, dkrupp, rnkovacs, kbarton, xazax.hun, nemanjai. Herald added a project: clang. whisperity added a parent revision: D69560: [clang-tidy] Add 'cppco

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

whisperity marked an inline comment as done. whisperity added a comment. I'd rather not call them //false positive// because the definition of `false positive` is ugly and mushy with regards to this check. This check attempts to enforce an interface rule, whether you(r project) wants to adhere t

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

whisperity added a comment. In D74463#1888270 , @aaron.ballman wrote: > I am also concerned about the false positives from this check because I don't > think there's going to be an easy heuristic for determining whether two > identifiers are "related" t

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

whisperity added a comment. In D69560#1890026 , @aaron.ballman wrote: > In D69560#1889925 , @vingeldal wrote: > > > Doesn't clang-tidy exclude warnings from system headers by default though? > > > Third-party SDKs

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

whisperity added a comment. In D69560#1889925 , @vingeldal wrote: > I think this value very well may strike a good balance in that compromise but > I still think it is a clear deviation from the guideline as specified. > IMO all deviations from the guide

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

whisperity added a comment. @aaron.ballman @vingeldal I'll go over the findings (as I've run this on LLVM and various other projects, a few examples are uploaded in my first comment as HTML renders of the bug report!) soon, but I want to wait until a research paper I have based on this topic ge

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

whisperity added a comment. In D69560#1891167 , @aaron.ballman wrote: > Btw, we should update the terminology in the patch to use `parameter` instead > of `argument` (parameters are what the function declares, arguments are what > are passed in at the c

[PATCH] D75041: [WIP][clang-tidy] Approximate implicit conversion issues for the 'cppcoreguidelines-avoid-adjacent-arguments-of-same-type' check

whisperity added a reviewer: aaron.ballman. whisperity added subscribers: vingeldal, zporky. whisperity added a comment. **WIP** because there's a lot of debug stuff that should be cleared out from here. This is a crude patch. It works fancy, but the code is crude. Repository: rG LLVM Github

[PATCH] D75171: [Analyzer] Fix for incorrect use of container and iterator checkers

whisperity added a comment. Do you have access to the `Driver` somehow? Instead of a `cerr` (-ish) output, something that is formatted like a "true" error diagnostic (or warning), such as `"no such file or directory"` would be much better, I fear this `[ERROR]` will simply be flooded away with

[PATCH] D75171: [Analyzer] Fix for incorrect use of container and iterator checkers

whisperity added a comment. Also, //errors// should conceptually break the operation at hand, so this thing should be a warning instead? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75171/new/ https://reviews.llvm.org/D75171 __

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

whisperity updated this revision to Diff 227254. whisperity added a comment. - Removed //`This check`// from the documentation comment of the check's class too. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69560/new/ https://reviews.llvm.org/D69560 Files: clang-tools-extra/clang-t

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

whisperity marked 11 inline comments as done. whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AdjacentArgumentsOfSameTypeCheck.cpp:488 +void AdjacentArgumentsOfSameTypeCheck::registerMatchers(MatchFinder *Finder) { + if (!getLangOpts

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

whisperity updated this revision to Diff 227253. whisperity added a comment. - Fix some comments, formatting and documentation - Organised test files to be in the same directory as others on the Monorepo structure - Helper functions moved from `namespace (anonymous)` to `static`. - Added test for

[PATCH] D71001: [clang-tidy] New check: bugprone-misplaced-pointer-arithmetic-in-alloc

whisperity added a comment. While I can't pitch in with actual findings of this check, @MyDeveloperDay, you're right in many aspects, including those specific examples //not// firing. But an example that actually fires this check indicates a very specific **undefined behaviour** case. So if suc

[PATCH] D71001: [clang-tidy] New check: bugprone-misplaced-pointer-arithmetic-in-alloc

whisperity added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/MisplacedPointerArithmeticInAllocCheck.cpp:81-82 + diag(PtrArith->getBeginLoc(), + "pointer arithmetic is applied to the result of %0() instead of its " + "argument") + << Func-

[PATCH] D71199: [clang-tidy] New check readability-prefer-initialization-list

whisperity added a comment. Can you refresh my memory on whether a rule for "if init expr is constant, initialise in class body instead" exists for init list members? If so, this will be a funny "two pass needed to fix" kind of check. Comment at: clang-tools-extra/clang-tidy

<    1   2   3   4   5   6   >