[clang-tools-extra] [clang-tidy] Improved readability-bool-conversion to be more consistent when using parentheses (PR #72068)

firewave wrote: Another PR for this was opened a few hours ago: #71848. https://github.com/llvm/llvm-project/pull/72068 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Improved readability-bool-conversion to be more consistent when using parentheses (PR #72068)

firewave wrote: > Hi @firewave, I think you are referencing a different issue. If I test #71852 > with PR #72050 I do not get the expected behavior. Of course you are right. I missed there being two different issues. Sorry about the noise. https://github.com/llvm/llvm-project/pull/72068 _

[clang-tools-extra] [clang-tidy] Add new modernize-string-find-startswith check (PR #72385)

firewave wrote: I wonder if this should also detect the `str.compare("marker", 0, 6) == 0` pattern. There is possibly some kind of pattern involving `std::equal()` as well. Could as well be a different check though. Not sure if it would have a performance impact to use `starts_with()` instead

[clang-tools-extra] [clang-tidy] Add new modernize-string-find-startswith check (PR #72385)

firewave wrote: > would be to support also endswith in same check +1 On a side note: I will be looking into the related patterns and their performance soon as I am getting very strange code/performance when they are used outside of a benchmark - especially with Clang. https://github.com/llvm

[clang-tools-extra] [clang-tidy] Add new modernize-string-find-startswith check (PR #72385)

firewave wrote: > Any thoughts on open-ended check name instead? `modernize-string-find-affix` > (affix = prefix | suffix)? `modernize-string-startswith-endswith` is the first what popped into my head but it would not have been my first choice. Would this also be the check you would implement

[clang] cppcheck: pass NodeKinds by const reference (PR #87273)

firewave wrote: Depending on the calling code the fix might actually be the introduction of `std::move()`. This is a known issue upstream: https://trac.cppcheck.net/ticket/12384. https://github.com/llvm/llvm-project/pull/87273 ___ cfe-commits mailing

[clang-tools-extra] [clang-tidy]avoid bugprone-unused-return-value false positive for function with the same prefix as the default argument (PR #84333)

firewave wrote: I think this might also require documentation changes. The documentation is also a bit misleading in terms of the defaults: https://clang.llvm.org/extra/clang-tidy/checks/bugprone/unused-return-value.html. I add issues detecting a custom function as it required the ``::` prefix

[clang-tools-extra] [run-clang-tidy.py] Add option to ignore source files from compilation database (PR #82416)

firewave wrote: > Why can't we make "filter" use a full regex that supports negative > expressions instead? How do you do that? I thought `llvm::RegEx` doesn't support negative expressions. https://github.com/llvm/llvm-project/pull/82416 ___ cfe-com

[clang-tools-extra] [clang-tidy]avoid bugprone-unused-return-value false positive for assignment operator overloading (PR #84489)

firewave wrote: Maybe add `+=` to the tests as well? I have also seen it reported with that. https://github.com/llvm/llvm-project/pull/84489 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

[clang-tools-extra] [clang-tidy] use upper case letters for bool conversion suffix (PR #102831)

firewave wrote: I do not think this logic should be added to a check and should stay in a single place as it is right now. > A bit of nitpick, but would it make sense to have some consistency with > `readability-identifier-naming`? As pointed out here this logic now exists in three different

[clang-tools-extra] [clang-tidy] `doesNotMutateObject`: Handle calls to member functions … (PR #94362)

firewave wrote: Could this also be applied for #69577? (please also mind the tickets referenced in it) https://github.com/llvm/llvm-project/pull/94362 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listi

[clang-tools-extra] [clang-tidy] `doesNotMutateObject`: Handle calls to member functions … (PR #94362)

firewave wrote: Thanks for looking into this. > So unfortunately this change won't improve > `performance-unnecessary-value-param`. > > I can have a look at unifying both in a subsequent PR. Simply adding comments to the tickets in question, so the information is not lost to time, would suff

[clang-tools-extra] [clang-tidy] new check readability-mark-static (PR #90830)

firewave wrote: Isn't this already covered by `-Wmissing-prototypes`? https://github.com/llvm/llvm-project/pull/90830 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] new check readability-mark-static (PR #90830)

@@ -0,0 +1,26 @@ +.. title:: clang-tidy - readability-mark-static + +readability-mark-static +=== + +Detects variable and function can be marked as static. + +Static functions and variables are scoped to a single file. Marking functions +and variables as static

[clang-tools-extra] [clang-tidy] new check readability-mark-static (PR #90830)

@@ -0,0 +1,26 @@ +.. title:: clang-tidy - readability-mark-static + +readability-mark-static +=== + +Detects variable and function can be marked as static. + +Static functions and variables are scoped to a single file. Marking functions +and variables as static

[clang-tools-extra] [clang-tidy][NFC] optimize unused using decls performance (PR #110200)

firewave wrote: This might help with #72300. https://github.com/llvm/llvm-project/pull/110200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Add modernize-cleanup-static-cast check (PR #118033)

firewave wrote: How is this different from `readability-redundant-casting`? https://github.com/llvm/llvm-project/pull/118033 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-tidy] Add modernize-make-direct check (PR #118120)

firewave wrote: This will conflict with `modernize-make-shared` and `modernize-make-unique`. I also very sure having `new` any modern C++ code is very much to be avoided. Having no insight on the differences of the inner workings - but just based on https://en.cppreference.com/w/cpp/memory/sha

[clang] [analyzer] Add time-trace scopes for high-level analyzer steps (PR #125508)

firewave wrote: Does this also work when CSA is called through clang-tidy? The analyzer profiling data in missing in the `--enable-check-profile` output - see #73673? https://github.com/llvm/llvm-project/pull/125508 ___ cfe-commits mailing list cfe-co

[clang-tools-extra] [clang-tidy] Add performance-redundant-lookup check (PR #125420)

firewave wrote: I ran it on the Cppcheck codebase and there were several cases I would consider false positives as well as some which are really awkward to fix (in some cases leading to potentially unnecessary lookups). I will provide some examples in the next few days. https://github.com/llv

[clang-tools-extra] [clang-tidy] Add performance-redundant-lookup check (PR #125420)

firewave wrote: Performance looks good. Everything under 5% is acceptable. The top three entries should probably get tickets so they get looked into. The amount seems too much for what it actually provides. https://github.com/llvm/llvm-project/pull/125420 __

[clang] [analyzer] Add time-trace scopes for high-level analyzer steps (PR #125508)

firewave wrote: > It should be relatively easy to add, but I don't want to extend the scope of > this PR. Thanks for the explanation. Yes, please don't extend the scope of the change. https://github.com/llvm/llvm-project/pull/125508 ___ cfe-commits m

[clang-tools-extra] [clang-tidy] Add performance-redundant-lookup check (PR #125420)

firewave wrote: I think we should agree on an initial implementation to be landed and open separate tickets and not a single meta one for additional patterns to detect. https://github.com/llvm/llvm-project/pull/125420 ___ cfe-commits mailing list cfe-

[clang-tools-extra] [clang-tidy] Add performance-redundant-lookup check (PR #125420)

@@ -0,0 +1,147 @@ +.. title:: clang-tidy - performance-redundant-lookup + +performance-redundant-lookup + + +This check warns about potential redundant container lookup operations within +a function. + +Examples + + +.. code-block:: c++ + +i

[clang-tools-extra] [clang-tidy] Add performance-redundant-lookup check (PR #125420)

firewave wrote: Could you please run this with `--enable-check-profile` to see how heavy it is? https://github.com/llvm/llvm-project/pull/125420 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe

[clang-tools-extra] [clang-tidy] Add performance-redundant-lookup check (PR #125420)

@@ -0,0 +1,147 @@ +.. title:: clang-tidy - performance-redundant-lookup + +performance-redundant-lookup + + +This check warns about potential redundant container lookup operations within +a function. + +Examples + + +.. code-block:: c++ + +i

[clang-tools-extra] [clang-tidy] Add performance-redundant-lookup check (PR #125420)

@@ -0,0 +1,147 @@ +.. title:: clang-tidy - performance-redundant-lookup + +performance-redundant-lookup + + +This check warns about potential redundant container lookup operations within +a function. + +Examples + + +.. code-block:: c++ + +i

[clang-tools-extra] [clang-tidy]detecting conversion directly by `make_unique` and `make_shared` in bugprone-optional-value-conversion (PR #119371)

firewave wrote: Can the logic for implementing this also be used to address https://github.com/llvm/llvm-project/issues/86447#issuecomment-2016943524? https://github.com/llvm/llvm-project/pull/119371 ___ cfe-commits mailing list cfe-commits@lists.llvm

[clang] [TySan] Add initial documentation for Type Sanitizer (PR #123595)

@@ -0,0 +1,153 @@ + +TypeSanitizer + + +.. contents:: + :local: + +Introduction + + +TypeSanitizer is a detector for strict type aliasing violations. It consists of a compiler +instrumentation module and a run-time library. The tool d

[clang-tools-extra] [clang-tidy] Add new check bugprone-unintended-char-ostream-output (PR #127720)

@@ -0,0 +1,30 @@ +.. title:: clang-tidy - bugprone-unintended-char-ostream-output + +bugprone-unintended-char-ostream-output +=== + +Finds unintended character output from ``unsigned char`` and ``signed char`` to an +``ostream``. + +Normally, w

[clang-tools-extra] [clang-tidy] Add new check bugprone-unintended-char-ostream-output (PR #127720)

@@ -0,0 +1,69 @@ +//===--- UnintendedCharOstreamOutputCheck.cpp - clang-tidy ===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apa

[clang-tools-extra] [clang-tidy] Add new `construct-reusable-objects-once` check (PR #131455)

firewave wrote: Nice. It looks like it does not consider that static initializations within functions are only thread-safe starting with C++11. Also this may obviously only be applied to read-only objects. That would require an existing `const` object or usage in tandem with `misc-const-corr

[clang-tools-extra] [clang-tidy] Add new check bugprone-unintended-char-ostream-output (PR #127720)

firewave wrote: Some thoughts based on the warnings I am seeing in actual code. --- Another way to fix this could be using `std::to_string()`. --- I am seeing this with an enum type which might be valid, working code but have not looked into it yet. --- The fix-it is problematic if it is a

[clang-tools-extra] [NFC] Remove dead code detected by code sanitizer. (PR #134385)

firewave wrote: `LStrl` and `RStrl` are unused now and can also be dropped. https://github.com/llvm/llvm-project/pull/134385 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [Clang-tidy][NFC] Remove dead code detected by code sanitizer. (PR #134385)

firewave wrote: Based on the message it seems the "sanitizer" was Coverity. Is my assumption correct? https://github.com/llvm/llvm-project/pull/134385 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listi

[clang-tools-extra] [clang-tidy] Add new check bugprone-unintended-char-ostream-output (PR #127720)

firewave wrote: > I am seeing this with an enum type which might be valid, working code but > have not looked into it yet. The warnings in question were correct. It exposed a behavior change introduced by fixing `performance-enum-size`. https://github.com/llvm/llvm-project/pull/127720 ___