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

2024-06-05 Thread Oliver Stöneberg via cfe-commits

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 suffice for now.

https://github.com/llvm/llvm-project/pull/94362
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-06-04 Thread Oliver Stöneberg via cfe-commits

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/listinfo/cfe-commits


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

2024-05-02 Thread Oliver Stöneberg via cfe-commits


@@ -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 helps to better remove dead code. In addition, it gives
+the compiler more information and can help compiler make more aggressive
+optimizations.
+

firewave wrote:

Did some research but did not find the case I remembered (it might have 
variable templates though):
- anonymous namespaces do not have external linkage until C++11: 
https://en.cppreference.com/w/cpp/language/namespace#Unnamed_namespaces / 
https://en.cppreference.com/w/cpp/language/storage_duration#Linkage
- `static` variable templates do not have internal linkage until C++14: 
https://en.cppreference.com/w/cpp/language/storage_duration#Linkage

Interesting tidbit that enumerations have external linkage.

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)

2024-05-02 Thread Oliver Stöneberg via cfe-commits


@@ -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 helps to better remove dead code. In addition, it gives
+the compiler more information and can help compiler make more aggressive
+optimizations.
+

firewave wrote:

AFAIK `static` doesn't prevent ODR violations (unfortunately I do not have 
specifics - it is just from experience). You need to use anonymous namespaces 
for that.
Also Clang has a bug where you might still encounter issues although the 
symbols should be internal (resulting in a crash) #75275.

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)

2024-05-02 Thread Oliver Stöneberg via cfe-commits

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] cppcheck: pass NodeKinds by const reference (PR #87273)

2024-04-01 Thread Oliver Stöneberg via cfe-commits

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 list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-03-08 Thread Oliver Stöneberg via cfe-commits

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


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

2024-03-08 Thread Oliver Stöneberg via cfe-commits

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-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2024-03-07 Thread Oliver Stöneberg via cfe-commits

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.

https://github.com/llvm/llvm-project/pull/84333
___
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 new modernize-string-find-startswith check (PR #72385)

2023-11-15 Thread Oliver Stöneberg via cfe-commits

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 the suggested use of 
`contains()` in?

https://github.com/llvm/llvm-project/pull/72385
___
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 new modernize-string-find-startswith check (PR #72385)

2023-11-15 Thread Oliver Stöneberg via cfe-commits

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/llvm-project/pull/72385
___
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 new modernize-string-find-startswith check (PR #72385)

2023-11-15 Thread Oliver Stöneberg via cfe-commits

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

https://github.com/llvm/llvm-project/pull/72385
___
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)

2023-11-12 Thread Oliver Stöneberg via cfe-commits

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
___
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)

2023-11-12 Thread Oliver Stöneberg via cfe-commits

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