[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

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

[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

[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

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

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

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

[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

[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

[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

[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

[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

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

[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

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

[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

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

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

[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 , const Point3D ) {} +// NO-WARN: Distinct types. aaron.ballman wrote:

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

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] 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. Release Notes entry of current development tree reverted in rG94850918274c20c15c6071dc52314df51ed2a357

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

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

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

<    1   2   3   4   5   6   >