From 365904085e27b49d402e389d5c454781eca4f3f5 Mon Sep 17 00:00:00 2001 From: Giuseppe D'Angelo Date: Sat, 30 Jul 2022 13:16:01 +0200 Subject: [PATCH] QFile: make constructors taking a path explicit This is a level A SIC, as it breaks QFile f = "/some/path"; In general, it's not a good idea to have this implicit conversion. A QFile is not a representation of a path, so the conversion should be explicit. I am going to keep the current semantics (implicit conversion) up to and including Qt 6.8 (LTS). Starting from 6.9, the constructor will be unconditionally explicit. This is deliberate, and done in order to make users fix their code while staying in Qt 6, rather than encountering this issue (and countless many more) if and when they upgrade from Qt 6 to Qt 7. In the meanwhile, users can opt-in to the new semantics by defining a macro. [ChangeLog][QtCore][QFile] The QFile constructors that take a path are going to become unconditionally `explicit` in Qt 6.9. Code like `QFile f = "/path";` will need to be ported to equivalent one (e.g. `QFile f{"/path/"}`). This has been done in order to prevent a category of mistakes when passing strings or paths to functions that actually take a QFile. Users can opt-in to this change even before Qt 6.9 by defining the QT_EXPLICIT_QFILE_CONSTRUCTION_FROM_PATH macro before including any Qt header. Change-Id: I065a09b9ce5d24c352664df0d48776545f6a0d8e Reviewed-by: Thiago Macieira --- cmake/QtInternalTargets.cmake | 1 + src/corelib/io/qfile.cpp | 11 +++++++++++ src/corelib/io/qfile.h | 13 ++++++++++--- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/cmake/QtInternalTargets.cmake b/cmake/QtInternalTargets.cmake index 21bd645639a..dc45b57eecd 100644 --- a/cmake/QtInternalTargets.cmake +++ b/cmake/QtInternalTargets.cmake @@ -152,6 +152,7 @@ target_link_libraries(PlatformToolInternal INTERFACE PlatformAppInternal) qt_internal_add_global_definition(QT_NO_JAVA_STYLE_ITERATORS) qt_internal_add_global_definition(QT_NO_NARROWING_CONVERSIONS_IN_CONNECT) +qt_internal_add_global_definition(QT_EXPLICIT_QFILE_CONSTRUCTION_FROM_PATH) if(WARNINGS_ARE_ERRORS) qt_internal_set_warnings_are_errors_flags(PlatformModuleInternal INTERFACE) diff --git a/src/corelib/io/qfile.cpp b/src/corelib/io/qfile.cpp index b1115ad15c0..bb4a4640174 100644 --- a/src/corelib/io/qfile.cpp +++ b/src/corelib/io/qfile.cpp @@ -226,6 +226,15 @@ QFile::QFile(QObject *parent) } /*! Constructs a new file object to represent the file with the given \a name. + +//! [qfile-explicit-constructor-note] + \note In versions up to and including Qt 6.8, this constructor is + implicit, for backward compatibility. Starting from Qt 6.9 this + constructor is unconditionally \c{explicit}. Users can force this + constructor to be \c{explicit} even in earlier versions of Qt by + defining the \c{QT_EXPLICIT_QFILE_CONSTRUCTION_FROM_PATH} macro + before including any Qt header. +//! [qfile-explicit-constructor-note] */ QFile::QFile(const QString &name) : QFileDevice(*new QFilePrivate, nullptr) @@ -1145,6 +1154,8 @@ qint64 QFile::size() const \since 6.0 Constructs a new file object to represent the file with the given \a name. + + \include qfile.cpp qfile-explicit-constructor-note */ /*! \fn QFile::QFile(const std::filesystem::path &name, QObject *parent) diff --git a/src/corelib/io/qfile.h b/src/corelib/io/qfile.h index 25f63d01c84..3ac6795495c 100644 --- a/src/corelib/io/qfile.h +++ b/src/corelib/io/qfile.h @@ -54,6 +54,13 @@ using ForceFilesystemPath = typename std::enable_if_t= QT_VERSION_CHECK(6, 9, 0)) || defined(QT_EXPLICIT_QFILE_CONSTRUCTION_FROM_PATH) +# define QFILE_MAYBE_EXPLICIT explicit +#else +# define QFILE_MAYBE_EXPLICIT Q_IMPLICIT +#endif + class Q_CORE_EXPORT QFile : public QFileDevice { #ifndef QT_NO_QOBJECT @@ -63,12 +70,12 @@ class Q_CORE_EXPORT QFile : public QFileDevice public: QFile(); - QFile(const QString &name); + QFILE_MAYBE_EXPLICIT QFile(const QString &name); #ifdef Q_QDOC - QFile(const std::filesystem::path &name); + QFILE_MAYBE_EXPLICIT QFile(const std::filesystem::path &name); #elif QT_CONFIG(cxx17_filesystem) template = 0> - QFile(const T &name) : QFile(QtPrivate::fromFilesystemPath(name)) + QFILE_MAYBE_EXPLICIT QFile(const T &name) : QFile(QtPrivate::fromFilesystemPath(name)) { } #endif // QT_CONFIG(cxx17_filesystem)