Skip to content

enabled and fixed modernize-use-auto clang-tidy warnings#6969

Merged
firewave merged 4 commits intodanmar:mainfrom
firewave:auto_it
Jan 1, 2025
Merged

enabled and fixed modernize-use-auto clang-tidy warnings#6969
firewave merged 4 commits intodanmar:mainfrom
firewave:auto_it

Conversation

@firewave
Copy link
Copy Markdown
Collaborator

No description provided.

@firewave firewave force-pushed the auto_it branch 2 times, most recently from c9b94be to ef37224 Compare October 30, 2024 21:50
@firewave
Copy link
Copy Markdown
Collaborator Author

/home/runner/work/cppcheck/cppcheck/lib/utils.h:412:16: error: returning a constant reference parameter may cause use-after-free when the parameter is constructed from a temporary [bugprone-return-const-ref-from-parameter,-warnings-as-errors]
  412 |         return t;
      |                ^
#include <set>
#include <type_traits>

namespace utils {
    template<class T>
    constexpr typename std::add_const<T>::type & as_const(T& t) noexcept
    {
        return t;
    }
}

struct C
{
    const int* f() const
    {
        const auto it = utils::as_const(v).find(0);
        if (it == v.cend())
            return nullptr;
        return &(*it);
    }

    std::set<int> v;
};

https://godbolt.org/z/91qxYYxcz

If you remove the const from the method the warning goes away.

This happens because we try as_const() something that is already const. That is intentional though.

It seems like this might just be a false positive since everything seems to work fine and the CI is happy otherwise.

@firewave
Copy link
Copy Markdown
Collaborator Author

These changes exposed a unreadVariable false negative: https://trac.cppcheck.net/ticket/13303.

@firewave
Copy link
Copy Markdown
Collaborator Author

This is ready for review but keeping a draft so it does not interfere with the ValueFlow revert.

@firewave firewave marked this pull request as ready for review December 14, 2024 13:25
Copy link
Copy Markdown
Owner

@danmar danmar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please review if we can use range for loops in some of these loops.

fd_set rfds;
FD_ZERO(&rfds);
for (std::list<int>::const_iterator rp = rpipes.cbegin(); rp != rpipes.cend(); ++rp)
for (auto rp = rpipes.cbegin(); rp != rpipes.cend(); ++rp)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be a range for loop right?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It probably can - but so do others. modernize-loop-convert is currently disabled and that should report by it. Now that we have made more cleanups I will give that another spin.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR enabling that warning is coming up after this.

@firewave firewave marked this pull request as draft December 20, 2024 23:47
@firewave firewave changed the title use auto for iterators enabled and fixed modernize-use-auto clang-tidy warnings Dec 20, 2024
@firewave firewave marked this pull request as ready for review December 20, 2024 23:52
@firewave firewave merged commit e228821 into danmar:main Jan 1, 2025
@firewave firewave deleted the auto_it branch January 1, 2025 16:38
firewave added a commit to firewave/cppcheck that referenced this pull request Apr 2, 2026
firewave added a commit to firewave/cppcheck that referenced this pull request Apr 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants