qbittorrent / qBittorrent

qBittorrent BitTorrent client
https://www.qbittorrent.org
Other
26.1k stars 3.84k forks source link

qpair copy assignment operator is implicitly deleted. #3231

Closed Jichao closed 9 years ago

Jichao commented 9 years ago

environment: boost 1.58.0 mac os x yosemite. Qt5.4.2/5.4/clang_64.

step to reproduce:

  1. qt creator open the project.
  2. build the project.
  3. error message: qpair<torrenthandle*const,...> copy assignment operator is implicitly deleted becasuse first is const type.(cannot rember the exactly message).

temp solution: remove all the const qualifier for torrenthandle.

glassez commented 9 years ago

cannot rember the exactly message

For this purpose there is a Copy-Paste feature. Without details your message is useless.

Jichao commented 9 years ago

you really mean it? the step to reproduce. the error message which point the reason.the way to solve the problem is useless?

glassez commented 9 years ago

the step to reproduce.

I (and some other developers) go through these steps several times a day. In addition, each PR must be compiled by Travis CI before it is merged. Your problem is clearly specific. It is therefore required to assess the circumstances in which it occurs.

the error message which point the reason

This is no error message.This is just some fragment of the error message that you remembered. Perhaps you think this part is the most important, but developers need more: error code, file name, line number, etc.

the way to solve the problem is useless?

This is about as useful as opening the door using a battering RAM. You, of course, opens the door, but better still to try to find the keys.

Jichao commented 9 years ago

ok,i think i'd better dig out the root cause and come up a pull request before report an issue.

Chocobo1 commented 9 years ago

The truth is there are relatively less osx devs here. Most devs are working on linux/windows, so it's hard to find out what's going wrong on your platform. If you have free time and willing to contribute, I heard that Travis CI supports osx compilation, maybe you could help by adding osx in our Travis configuration.

Jichao commented 9 years ago

Actually I'm able to dig out a bit on my Windows 8.

qpair copy assignment operator is implicitly deleted becasuse first is const type. That means the const member in the class prevent the generation of the implicitly copy assignment opeartor.

In mainwindow.cpp, There is a data member QList<QPair<BitTorrent::TorrentHandle *const, QString> > unauthenticated_trackers; for class MainWindow.

QPair<BitTorrent::TorrentHandle *const, QString> is stored in the QList container, which resemble std::list, requires CopyAssignable. But QPair<BitTorrent::TorrentHandle *const, QString> does not satisfied this requirement.

In qlist.h, the implementation of the << or push_back depends on node_construt,

template <typename T>
Q_INLINE_TEMPLATE void QList<T>::node_construct(Node *n, const T &t)
{
    if (QTypeInfo<T>::isLarge || QTypeInfo<T>::isStatic) n->v = new T(t);
    else if (QTypeInfo<T>::isComplex) new (n) T(t);
#if (defined(__GNUC__) || defined(__INTEL_COMPILER) || defined(__IBMCPP__)) && !defined(__OPTIMIZE__)
    // This violates pointer aliasing rules, but it is known to be safe (and silent)
    // in unoptimized GCC builds (-fno-strict-aliasing). The other compilers which
    // set the same define are assumed to be safe.
    else *reinterpret_cast<T*>(n) = t;
#else
    // This is always safe, but penaltizes unoptimized builds a lot.
    else ::memcpy(n, &t, sizeof(T));
#endif
}

*reinterpret_cast<T*>(n) = t called the non-exist copy assignment operator, which leads to the compilation error.

If I am right, I'm curious why the current version of source code could pass compliation on the other platform.

glassez commented 9 years ago

temp solution: remove all the const qualifier for torrenthandle.

real solution: remove the const qualifier for torrenthandle in such places only.

glassez commented 9 years ago

If I am right, I'm curious why the current version of source code could pass compliation on the other platform.

I suspect the fact that you're using clang.

Jichao commented 9 years ago

Ok, I could not access any Mac environment right now. But I'll post the full complication log when I am home ASAP. Actually if you request, I could record the whole compilication progress and upload it to youtube.

Here is evidence I am able to come up on Window.

#include <iostream>

template <typename T1, typename T2>
struct pair
{
    T1 a;
    T2 b;
    pair(const T1& a, const T2& b) {
        this->a = a;
        this->b = b;
    }
};

int main()
{
    // test for const point variable
    int foo = 1;
    int bar = 22;
    int* const constptr = &foo;
    //constptr = &bar; cannot assign value for const variable
    int* ptr = &bar;

    // test for default copy assignment 
    pair<int* const, int*> val(constptr, ptr);
    pair<int* const, int*> val2(constptr, ptr);
    val = val2;
    return 0;
}

Compiling the above sample, let's check the erorr message generated by cl/gcc/clang.

cl

gcc

clang

Is it clear enough?

glassez commented 9 years ago
#include <QPair>
#include <QList>

int main()
{
    // test for const point variable
    int foo = 1;
    int bar = 22;
    int* const constptr = &foo;
    int* ptr = &bar;

    QPair<int* const, int*> val(constptr, ptr);
    QPair<int* const, int*> val2(constptr, ptr);
    val = val2; // this doesn't work!

    QList<QPair<int* const, int*> > list;
    list << val << val2; // but this works!

    return 0;
}

Apparently, when we use this template as a parameter to another, not all compilers remove the assignment operator (only clang, most likely). The solution is to remove the const qualifier where TorrentHandle is the template parameter (but not in other places!). @Jichao Do you want to do this? If not, I'll fix it soon.

Jichao commented 9 years ago

At least on Windows all the compiler (gcc/clang/cl) remove the default copy assignment operator. Actually, any c++11 standard compiler should remove the copy assignment operator accordding to c++11 $12.8.24

A defaulted copy/move assignment operator for class X is defined as deleted if X has: — a variant member with a non-trivial corresponding assignment operator and X is a union-like class, or — a non-static data member of const non-class type (or array thereof), or — a non-static data member of reference type, or — a non-static data member of class type M (or array thereof) that cannot be copied/moved because overload resolution (13.3), as applied to M’s corresponding assignment operator, results in an ambiguity or a function that is deleted or inaccessible from the defaulted assignment operator, or

I have tested you example on Windows with Qt5.4.2\5.4\msvc2013 32bit, keke

    template <typename TT1, typename TT2>
    QPair &operator=(const QPair<TT1, TT2> &p)
    { first = p.first; second = p.second; return *this; }

This template copy assignment does not suit the type. However, the default assignment is still missing.

Anyway, I'll not report any issue util I have a pull rest or I'm exactly sure the root cause, or I could explain all details, so help youself.

Chocobo1 commented 9 years ago

why the current version of source code could pass compliation on the other platform.

Pardon me if wrong, maybe by default we compile with optimizations on, i.e. __OPTIMIZE__ is defined, hence ::memcpy(n, &t, sizeof(T));

glassez commented 9 years ago

I have tested you example on Windows with Qt5.4.2\5.4\msvc2013 32bit

It compiles without errors if you comment this line:

val = val2; // this doesn't work!

And so I wrote that it doesn't work.

Jichao commented 9 years ago

@glassez Its my mistake.

@Chocobo1 Finally, I understood the node_construct, but could not get clang compile. Here is my compilation log on mac:

Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang++ -c -pipe -g -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk -std=c++11 -stdlib=libc++ -mmacosx-version-min=10.7 -Wall -W -fPIC -DWITH_GEOIP_EMBEDDED -DBOOST_NO_CXX11_RVALUE_REFERENCES -DVERSION_MAJOR=3 -DVERSION_MINOR=3 -DVERSION_BUGFIX=0 -DVERSION_BUILD=0 -DVERSION=\"v3.3.0alpha\" -DQT_NO_CAST_TO_ASCII -DQT_USE_FAST_CONCATENATION -DQT_USE_FAST_OPERATOR_PLUS -DBOOST_FILESYSTEM_VERSION=2 -DQT_WIDGETS_LIB -DQT_GUI_LIB -DQT_XML_LIB -DQT_NETWORK_LIB -DQT_CORE_LIB -I../../qBittorrent/src -I. -I/usr/local/include -I../../qBittorrent/src -I../../qBittorrent/src/app -I../../qBittorrent/src/app/qtsingleapplication -I../../qBittorrent/src/gui -I../../qBittorrent/src/gui/lineedit/src -I../../qBittorrent/src/gui/properties -I../../qBittorrent/src/gui/rss -I../../qBittorrent/src/gui/geoip -I../../qBittorrent/src/gui/powermanagement -I../../qBittorrent/src/searchengine -I../../../app/Qt5.4.2/5.4/clang_64/lib/QtWidgets.framework/Versions/5/Headers -I../../../app/Qt5.4.2/5.4/clang_64/lib/QtGui.framework/Versions/5/Headers -I../../../app/Qt5.4.2/5.4/clang_64/lib/QtXml.framework/Versions/5/Headers -I../../../app/Qt5.4.2/5.4/clang_64/lib/QtNetwork.framework/Versions/5/Headers -I../../../app/Qt5.4.2/5.4/clang_64/lib/QtCore.framework/Versions/5/Headers -I. -I/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk/System/Library/Frameworks/OpenGL.framework/Versions/A/Headers -I/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk/System/Library/Frameworks/AGL.framework/Headers -I. -I../../../app/Qt5.4.2/5.4/clang_64/mkspecs/macx-clang -F/Users/jcyangzh/app/Qt5.4.2/5.4/clang_64/lib -o mainwindow.o ../../qBittorrent/src/gui/mainwindow.cpp In file included from ../../qBittorrent/src/gui/mainwindow.cpp:55: In file included from ../../qBittorrent/src/gui/addnewtorrentdialog.h:39: In file included from ../../qBittorrent/src/core/bittorrent/torrentinfo.h:33: In file included from /usr/local/include/libtorrent/torrent_info.hpp:43: In file included from /usr/local/include/boost/optional.hpp:15: /usr/local/include/boost/optional/optional.hpp:1254:53: warning: unused parameter 'out' [-Wunused-parameter] operator<<(std::basic_ostream<CharType, CharTrait>& out, optional_detail::optional_tag const& v) ^ /usr/local/include/boost/optional/optional.hpp:1254:95: warning: unused parameter 'v' [-Wunused-parameter] operator<<(std::basic_ostream<CharType, CharTrait>& out, optional_detail::optional_tag const& v) ^ ../../qBittorrent/src/gui/mainwindow.cpp:892:26: warning: unused variable 'yesBtn' [-Wunused-variable] QPushButton _yesBtn = confirmBox.addButton(tr("&Yes"), QMessageBox::YesRole); ^ In file included from ../../qBittorrent/src/gui/mainwindow.cpp:37: In file included from ../../../app/Qt5.4.2/5.4/clang_64/lib/QtWidgets.framework/Versions/5/Headers/QFileDialog:1: In file included from ../../../app/Qt5.4.2/5.4/clang_64/lib/QtWidgets.framework/Versions/5/Headers/qfiledialog.h:37: In file included from ../../../app/Qt5.4.2/5.4/clang_64/lib/QtCore.framework/Versions/5/Headers/qdir.h:38: In file included from /Users/jcyangzh/app/Qt5.4.2/5.4/clang_64/lib/QtCore.framework/Headers/qfileinfo.h:37: In file included from ../../../app/Qt5.4.2/5.4/clang_64/lib/QtCore.framework/Versions/5/Headers/qfile.h:37: In file included from /Users/jcyangzh/app/Qt5.4.2/5.4/clang_64/lib/QtCore.framework/Headers/qfiledevice.h:37: In file included from /Users/jcyangzh/app/Qt5.4.2/5.4/clang_64/lib/QtCore.framework/Headers/qiodevice.h:39: In file included from ../../../app/Qt5.4.2/5.4/clang_64/lib/QtCore.framework/Versions/5/Headers/qobject.h:43: ../../../app/Qt5.4.2/5.4/clang_64/lib/QtCore.framework/Versions/5/Headers/qlist.h:378:35: error: object of type 'QPair<BitTorrent::TorrentHandle const, QString>' cannot be assigned because its copy assignment operator is implicitly deleted else reinterpretcast<T>(n) = t; ^ ../../../app/Qt5.4.2/5.4/clang_64/lib/QtCore.framework/Versions/5/Headers/qlist.h:521:13: note: in instantiation of member function 'QList<QPair<BitTorrent::TorrentHandle const, QString> >::node_construct' requested here node_construct(n, t); ^ ../../../app/Qt5.4.2/5.4/clang_64/lib/QtCore.framework/Versions/5/Headers/qlist.h:331:7: note: in instantiation of member function 'QList<QPair<BitTorrent::TorrentHandle const, QString> >::append' requested here { append(t); return this; } ^ ../../qBittorrent/src/gui/mainwindow.cpp:1160:34: note: in instantiation of member function 'QList<QPair<BitTorrent::TorrentHandle const, QString> >::operator<<' requested here unauthenticated_trackers << tracker; ^ ../../../app/Qt5.4.2/5.4/clang_64/lib/QtCore.framework/Versions/5/Headers/qpair.h:67:8: note: copy assignment operator of 'QPair<BitTorrent::TorrentHandle const, QString>' is implicitly deleted because field 'first' is of const-qualified type 'BitTorrent::TorrentHandle const' T1 first; ^ 3 warnings and 1 error generated.

The key is QPair<BitTorrent::TorrentHandle* const, Qstring> should be isStatic. In node_constuct, compiler should choose the first case and use the default copy ctor rather than entering the third case else *reinterpret_cast<T*>(n) = t;.

I haved added the following lines in mainwindow.cpp

    qDebug() << QTypeInfo<BitTorrent::TorrentHandle*const>::isStatic;
    qDebug() << QTypeInfo<QPair<BitTorrent::TorrentHandle*const,QString>>::isStatic;

and comment out

    if (unauthenticated_trackers.indexOf(tracker) < 0)
        unauthenticated_trackers << tracker;

The debug output shows right value 1 1.

In conclusion, it should be a clang bug, current version of source code is right.