llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.01k stars 11.95k forks source link

Invalid iterator substitution in `modernize-use-ranges` fix-it #110223

Open srpgilles opened 1 month ago

srpgilles commented 1 month ago

If I run clang-tidy v19.1.0 from Homebrew on macOS / XCode 16 over this small snippet:

#include <algorithm>
#include <iostream>
#include <vector>

int main([[maybe_unused]] int argc, [[maybe_unused]] char** argv)
{
    std::vector<int> indexes { 1, 2, 7, 8, 4, 5 }; 
    std::sort(indexes.begin(), indexes.end());
    auto logical_end = std::unique(indexes.begin(), indexes.end());

    if (logical_end != indexes.end())
        std::cout << "There were duplicates\n";

    return EXIT_SUCCESS;
}

I get the following code:

#include <algorithm>
#include <iostream>
#include <vector>

int main([[maybe_unused]] int argc, [[maybe_unused]] char** argv)
{
    std::vector<int> indexes { 1, 2, 7, 8, 4, 5 };
    std::ranges::sort(indexes);
    auto logical_end = std::ranges::unique(indexes);

    if (logical_end != indexes.end())
        std::cout << "There were duplicates\n";

    return EXIT_SUCCESS;
}

which is incorrect as std::ranges::unique returns a std::ranges::in_out_result (I got the same mistake with std::ranges::copy).

Replacing the generated:

auto logical_end = std::ranges::unique(indexes);

by:

auto [_, logical_end] = std::ranges::unique(indexes);

would fix it.

At any rate, thanks a lot for this extremely useful new diagnostic: I upgraded my whole codebase in less than 30 minutes 🙂

chrchr-github commented 1 month ago

Just fyi, the proposed fix should be auto [logical_end, _] = std::ranges::unique(indexes); Returns {ret, last}, where ret is a past-the-end iterator for the new end of the range. https://en.cppreference.com/w/cpp/algorithm/ranges/unique

srpgilles commented 1 month ago

Many thanks for pointing this out!

As a matter of fact I did the homework for std::ranges::copy, where it is really the second on that should be taken, and I assumed - wrongly it appears! - that it would be the same kind of object returned.

llvmbot commented 1 month ago

@llvm/issue-subscribers-clang-tidy

Author: None (srpgilles)

If I run clang-tidy v19.1.0 from Homebrew on macOS / XCode 16 over this small snippet: ```c++ #include <algorithm> #include <iostream> #include <vector> int main([[maybe_unused]] int argc, [[maybe_unused]] char** argv) { std::vector<int> indexes { 1, 2, 7, 8, 4, 5 }; std::sort(indexes.begin(), indexes.end()); auto logical_end = std::unique(indexes.begin(), indexes.end()); if (logical_end != indexes.end()) std::cout << "There were duplicates\n"; return EXIT_SUCCESS; } ``` I get the following code: ```c++ #include <algorithm> #include <iostream> #include <vector> int main([[maybe_unused]] int argc, [[maybe_unused]] char** argv) { std::vector<int> indexes { 1, 2, 7, 8, 4, 5 }; std::ranges::sort(indexes); auto logical_end = std::ranges::unique(indexes); if (logical_end != indexes.end()) std::cout << "There were duplicates\n"; return EXIT_SUCCESS; } ``` which is incorrect as `std::ranges::unique` returns a `std::ranges::in_out_result` (I got the same mistake with `std::ranges::copy`). Replacing the generated: ```c++ auto logical_end = std::ranges::unique(indexes); ``` by: ```c++ auto [_, logical_end] = std::ranges::unique(indexes); ``` would fix it. At any rate, thanks a lot for this extremely useful new diagnostic: I upgraded my whole codebase in less than 30 minutes 🙂
llvmbot commented 1 month ago

@llvm/issue-subscribers-bug

Author: None (srpgilles)

If I run clang-tidy v19.1.0 from Homebrew on macOS / XCode 16 over this small snippet: ```c++ #include <algorithm> #include <iostream> #include <vector> int main([[maybe_unused]] int argc, [[maybe_unused]] char** argv) { std::vector<int> indexes { 1, 2, 7, 8, 4, 5 }; std::sort(indexes.begin(), indexes.end()); auto logical_end = std::unique(indexes.begin(), indexes.end()); if (logical_end != indexes.end()) std::cout << "There were duplicates\n"; return EXIT_SUCCESS; } ``` I get the following code: ```c++ #include <algorithm> #include <iostream> #include <vector> int main([[maybe_unused]] int argc, [[maybe_unused]] char** argv) { std::vector<int> indexes { 1, 2, 7, 8, 4, 5 }; std::ranges::sort(indexes); auto logical_end = std::ranges::unique(indexes); if (logical_end != indexes.end()) std::cout << "There were duplicates\n"; return EXIT_SUCCESS; } ``` which is incorrect as `std::ranges::unique` returns a `std::ranges::in_out_result` (I got the same mistake with `std::ranges::copy`). Replacing the generated: ```c++ auto logical_end = std::ranges::unique(indexes); ``` by: ```c++ auto [_, logical_end] = std::ranges::unique(indexes); ``` would fix it. At any rate, thanks a lot for this extremely useful new diagnostic: I upgraded my whole codebase in less than 30 minutes 🙂