Issue
This issue got started while experimenting on a class derived from std::tuple
in order to reuse tuple's comparison operators.
The code below is supposed to remove duplicates from lists of 2D or 3D coordinates.
In the beginning of the main function, lines 49 and 50, there are two vectors v2
and v3
initialized with initializer lists. I suspect that there is something wrong in the initialization but can not figure out what.
g++ produces wrong results with optimization levels {0, 1, 2} and correct results with optimization level 3.
clang++ produces wrong results with optimization levels {1, 2, 3} and correct results with optimization level 0.
Question: Are the wrong results due to compiler bugs or is there some undefined behavior in my code?
The compiler versions running on Ubuntu 20.10 are:
g++ (Ubuntu 10.2.0-13ubuntu1) 10.2.0
Ubuntu clang version 11.0.0-2
The correct output is:
Before duplicate removal:
(1,2) (2,3) (1,2) (4,5) (1,2)
(1,2,3) (4,5,6) (1,2,3) (7,8,9) (1,2,3)
After duplicate removal:
(1,2) (2,3) (4,5)
(1,2,3) (4,5,6) (7,8,9)
The incorrect output is:
Before duplicate removal:
(1074266112,0) (32764,-976103536) (1075314688,0) (32764,-976103488) (1074266112,0)
(1,2,3) (4,5,6) (1,2,3) (7,8,9) (1,2,3)
After duplicate removal:
(1074266112,0) (32764,-976103536) (32764,-976103488)
(1,2,3) (4,5,6) (7,8,9)
Examples of compilation commands:
g++ point.cc -std=c++17 -O3
clang++ point.cc -std=c++17 -O0
The code (point.cc):
#include <iostream>
#include <vector>
#include <type_traits>
#include <tuple>
#include <utility>
#include <functional>
#include <algorithm>
template<class T, class W = std::reference_wrapper<T>>
struct Ref : W
{
using W::W;
Ref& operator=(const T& x) noexcept
{
this->get() = x;
return *this;
}
};
template<class T, class P = std::tuple<T,T>>
struct Point2D : P
{
using P::P;
// Make aliases for the tuple elements
Ref<T> x = std::get<0>(*this);
Ref<T> y = std::get<1>(*this);
};
template<class T, class P = std::tuple<T,T,T>>
struct Point3D : P
{
using P::P;
// Make aliases for the tuple elements
Ref<T> x = std::get<0>(*this);
Ref<T> y = std::get<1>(*this);
Ref<T> z = std::get<2>(*this);
};
template <class V>
void removeDuplicates(V& v)
{
std::sort(v.begin(), v.end());
auto last = std::unique(v.begin(), v.end());
v.erase(last, v.end());
}
int main() {
std::vector<Point2D<int>> v2 { {1,2}, {2,3}, {1,2}, {4,5}, {1,2} };
std::vector<Point3D<double>> v3 { {1,2,3}, {4,5,6}, {1,2,3}, {7,8,9}, {1,2,3} };
std::cout << "Before duplicate removal:\n";
for (auto p : v2)
std::cout << "(" << p.x << "," << p.y << ") ";
std::cout << "\n";
for (auto p : v3)
std::cout << "(" << p.x << "," << p.y << "," << p.z << ") ";
std::cout << "\n";
removeDuplicates(v2);
removeDuplicates(v3);
std::cout << "After duplicate removal:\n";
for (auto p : v2)
std::cout << "(" << p.x << "," << p.y << ") ";
std::cout << "\n";
for (auto p : v3)
std::cout << "(" << p.x << "," << p.y << "," << p.z << ") ";
std::cout << "\n";
}
Solution
The problem is with copy ctor. Since user provided copy ctor is absent, compiler default is generated; this copies references from sources that are no more valid. You need copy/move ctors that explicitly initialize those references:
Point2D::Point2D(Point2D const& p)
: P{p}
, x{get<0>(*this)}
, y{get<1>(*this)}
{};
The move ctor can as well be defined if needed. Similar argument holds for the Point3D
too. Your code currently triggers UB in that regard.
=============================================
PS: Do not miss the assignment:
auto& PointXD:: operator (PointXD& p){
P&{*this}=p;
return *this;//skip the refs; they're good
};
Again the move-assignment can be defined similarly.
Answered By - Red.Wave Answer Checked By - Senaida (WPSolving Volunteer)