[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 ___ 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)
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] 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/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)
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] 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 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)
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] [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-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)
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] [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 places with different ways of configuring it. An inconsistency is that `readability-uppercase-literal-suffix` only handles `l` and `u` by default whereas the check here also handles `f`. So if code is modified by this it will add `F` to the code which might not be desired. So you also need to configure which suffices to make uppercase here. https://github.com/llvm/llvm-project/pull/102831 ___ 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)
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] `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 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] 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 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)
@@ -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][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/shared_ptr/make_shared#Notes this appears to be something behaving very differently and less optimal/safe. https://github.com/llvm/llvm-project/pull/118120 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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/llvm/llvm-project/pull/125420 ___ 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 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 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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++
+
+if (map.count(key) && map[key] < threshold) {
+ // do stuff
+}
+
+In this example, we would check if the key is present in the map,
+and then do a second lookup to actually load the value.
+We could refactor the code into this, to use a single lookup:
+
+.. code-block:: c++
+
+if (auto it = map.find(key); it != map.end() && it->second < threshold) {
+ // do stuff
+}
+
+In this example, we do three lookups while calculating, caching and then
+using the result of some expensive computation:
+
+.. code-block:: c++
+
+if (!cache.contains(key)) {
+cache[key] = computeExpensiveValue();
+}
+use(cache[key]);
+
+We could refactor this code using ``try_emplace`` to fill up the cache entry
+if wasn't present, and just use it if we already computed the value.
+This way we would only have a single unavoidable lookup:
+
+.. code-block:: c++
+
+auto [cacheSlot, inserted] cache.try_emplace(key);
+if (inserted) {
+cacheSlot->second = computeExpensiveValue();
+}
+use(cacheSlot->second);
+
+
+What is a "lookup"?
+---
+
+All container operations that walk the internal structure of the container
+should be considered as "lookups".
+This means that checking if an element is present or inserting an element
+is also considered as a "lookup".
+
+For example, ``contains``, ``count`` but even the ``operator[]``
+should be considered as "lookups".
+
+Technically ``insert``, ``emplace`` or ``try_emplace`` are also lookups,
+even if due to limitations, they are not recognized as such.
+
+Lookups inside macros are ignored, thus not considered as "lookups".
+For example:
+
+.. code-block:: c++
+
+assert(map.count(key) == 0); // Not considered as a "lookup".
+
+Options
+---
+
+.. option:: ContainerNameRegex
+
+ The regular expression matching the type of the container objects.
+ This is matched in a case insensitive manner.
+ Default is ``set|map``.
firewave wrote:
That would also match `{flat_|unordered_}{multi}{set|map}` from the standard
which appear to have similar interfaces which should be fine. But maybe it
should be limited to the `std` namespace so it does not apply to any
implementation.
And it would also match an object like `class my_mmap`.
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-commits
[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-commits
[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++
+
+if (map.count(key) && map[key] < threshold) {
+ // do stuff
+}
+
+In this example, we would check if the key is present in the map,
+and then do a second lookup to actually load the value.
+We could refactor the code into this, to use a single lookup:
+
+.. code-block:: c++
+
+if (auto it = map.find(key); it != map.end() && it->second < threshold) {
+ // do stuff
+}
+
+In this example, we do three lookups while calculating, caching and then
+using the result of some expensive computation:
+
+.. code-block:: c++
+
+if (!cache.contains(key)) {
+cache[key] = computeExpensiveValue();
+}
+use(cache[key]);
+
+We could refactor this code using ``try_emplace`` to fill up the cache entry
+if wasn't present, and just use it if we already computed the value.
+This way we would only have a single unavoidable lookup:
+
+.. code-block:: c++
+
+auto [cacheSlot, inserted] cache.try_emplace(key);
+if (inserted) {
+cacheSlot->second = computeExpensiveValue();
+}
+use(cacheSlot->second);
+
+
+What is a "lookup"?
+---
+
+All container operations that walk the internal structure of the container
+should be considered as "lookups".
+This means that checking if an element is present or inserting an element
+is also considered as a "lookup".
+
+For example, ``contains``, ``count`` but even the ``operator[]``
+should be considered as "lookups".
+
+Technically ``insert``, ``emplace`` or ``try_emplace`` are also lookups,
+even if due to limitations, they are not recognized as such.
+
+Lookups inside macros are ignored, thus not considered as "lookups".
+For example:
+
+.. code-block:: c++
+
+assert(map.count(key) == 0); // Not considered as a "lookup".
+
+Options
+---
+
+.. option:: ContainerNameRegex
+
+ The regular expression matching the type of the container objects.
+ This is matched in a case insensitive manner.
+ Default is ``set|map``.
firewave wrote:
Maybe at least make it `set$|map$` so it only matches at the end of the string
instead of random occurrences.
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-commits
[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++
+
+if (map.count(key) && map[key] < threshold) {
+ // do stuff
+}
+
+In this example, we would check if the key is present in the map,
+and then do a second lookup to actually load the value.
+We could refactor the code into this, to use a single lookup:
+
+.. code-block:: c++
+
+if (auto it = map.find(key); it != map.end() && it->second < threshold) {
+ // do stuff
+}
+
+In this example, we do three lookups while calculating, caching and then
+using the result of some expensive computation:
+
+.. code-block:: c++
+
+if (!cache.contains(key)) {
+cache[key] = computeExpensiveValue();
+}
+use(cache[key]);
+
+We could refactor this code using ``try_emplace`` to fill up the cache entry
+if wasn't present, and just use it if we already computed the value.
+This way we would only have a single unavoidable lookup:
+
+.. code-block:: c++
+
+auto [cacheSlot, inserted] cache.try_emplace(key);
+if (inserted) {
+cacheSlot->second = computeExpensiveValue();
+}
+use(cacheSlot->second);
+
+
+What is a "lookup"?
+---
+
+All container operations that walk the internal structure of the container
+should be considered as "lookups".
+This means that checking if an element is present or inserting an element
+is also considered as a "lookup".
+
+For example, ``contains``, ``count`` but even the ``operator[]``
+should be considered as "lookups".
+
+Technically ``insert``, ``emplace`` or ``try_emplace`` are also lookups,
+even if due to limitations, they are not recognized as such.
+
+Lookups inside macros are ignored, thus not considered as "lookups".
+For example:
+
+.. code-block:: c++
+
+assert(map.count(key) == 0); // Not considered as a "lookup".
+
+Options
+---
+
+.. option:: ContainerNameRegex
+
+ The regular expression matching the type of the container objects.
+ This is matched in a case insensitive manner.
+ Default is ``set|map``.
firewave wrote:
But it would match `class UseTest` or `class Settings`.
> In that case it wouldnt match `llvm::SetVector`.
Maybe such classes should be explicitly be specified.
I am not a fan of including classes which a regular users does not come across
i.e. `llvm::`. Even including ``boost::` specific stuff feels like a stretch to
me. But let's not get too off-topic and wait for other people to chime in.
Making it so broad might even affect the performance but let's wait for the
numbers first. I will build it locally an give it a spin with the next few
days. (hopefully)
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-commits
[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.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 detects violations such as the use +of an illegally cast pointer, or misuse of a union. + +The violations TypeSanitizer catches may cause the compiler to emit incorrect code. + +Typical slowdown introduced by TypeSanitizer is about **4x** [[CHECK THIS]]. Typical memory overhead introduced by TypeSanitizer is about **9x**. firewave wrote: It also seems to have a sizable compile-time overhead. https://github.com/llvm/llvm-project/pull/123595 ___ 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 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, when ``unsigned char (uint8_t)`` or ``signed char (int8_t)`` is used, it +is more likely a number than a character. However, when it is passed directly to +``std::ostream``'s ``operator<<``, resulting in character-based output instead +of numeric value. This often contradicts the developer's intent to print +integer values. + +.. code-block:: c++ + +uint8_t v = 9; +std::cout << v; // output '\t' instead of '9' + +It could be fixed as + +.. code-block:: c++ + +std::cout << static_cast(v); firewave wrote: It is "magic" but I think it might make sense to make sure that the warning is not triggered by it. https://github.com/llvm/llvm-project/pull/127720 ___ 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 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: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "UnintendedCharOstreamOutputCheck.h"
+#include "clang/AST/Type.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+
+// check if the type is unsigned char or signed char
+AST_MATCHER(Type, isNumericChar) {
+ const auto *BT = dyn_cast(&Node);
+ if (BT == nullptr)
+return false;
+ const BuiltinType::Kind K = BT->getKind();
+ return K == BuiltinType::UChar || K == BuiltinType::SChar;
+}
+
+// check if the type is char
+AST_MATCHER(Type, isChar) {
+ const auto *BT = dyn_cast(&Node);
+ if (BT == nullptr)
+return false;
+ const BuiltinType::Kind K = BT->getKind();
+ return K == BuiltinType::Char_U || K == BuiltinType::Char_S;
+}
+
+} // namespace
+
+void UnintendedCharOstreamOutputCheck::registerMatchers(MatchFinder *Finder) {
+ auto BasicOstream =
+ cxxRecordDecl(hasName("::std::basic_ostream"),
+// only basic_ostream has overload operator<<
+// with char / unsigned char / signed char
+classTemplateSpecializationDecl(
+hasTemplateArgument(0, refersToType(isChar();
+ Finder->addMatcher(
+ cxxOperatorCallExpr(
+ hasOverloadedOperatorName("<<"),
+ hasLHS(hasType(hasUnqualifiedDesugaredType(
+ recordType(hasDeclaration(cxxRecordDecl(
+ anyOf(BasicOstream, isDerivedFrom(BasicOstream,
+ hasRHS(hasType(hasUnqualifiedDesugaredType(isNumericChar()
+ .bind("x"),
+ this);
+}
+
+void UnintendedCharOstreamOutputCheck::check(
+const MatchFinder::MatchResult &Result) {
+ const auto *Call = Result.Nodes.getNodeAs("x");
+ const Expr *Value = Call->getArg(1);
+ diag(Call->getOperatorLoc(),
+ "%0 passed to 'operator<<' outputs as character instead of integer. "
+ "cast to 'unsigned' to print numeric value or cast to 'char' to print "
+ "as character")
+ << Value->getType() << Value->getSourceRange();
firewave wrote:
Given the range only suggesting `int` should be safe. Adding `unsigned` in case
the underlying type would make it more clearer though.
As in other cases I think it should be coding guideline agnostic and suggest
the least intrusive (e.g. `std::int32_t` requires the `` include). But
let's not start that discussion again...
https://github.com/llvm/llvm-project/pull/127720
___
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 `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-correctness` (and incremental runs) - it should not duplicate any of that logic. And shouldn't global variables also be declared `static` or in an anonymous namespace? A similar use I came across multiples times is STL containers being constructed for each. That is something which can not be covered with this check https://github.com/llvm/llvm-project/pull/131455 ___ 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 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 templated type and should probably be
omitted in that case:
```
#include
#include
template
void func(const T& data)
{
std::ostringstream ostr;
ostr << data;
}
void f()
{
func((char)0);
func((int8_t)0);
func("");
}
```
```
:8:10: warning: 'signed char' passed to 'operator<<' outputs as
character instead of integer. cast to 'unsigned int' to print numeric value or
cast to 'char' to print as character [bugprone-unintended-char-ostream-output]
8 | ostr << data;
| ^
| static_cast(data)
```
https://godbolt.org/z/93axK1E4M
https://github.com/llvm/llvm-project/pull/127720
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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/listinfo/cfe-commits
[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 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
