Re: Review Request 119564: Add define to re-enable Qt functionality we depend on.
On Aug. 2, 2014, 9:04 a.m., Alex Merry wrote: I would rather change the code. The Qt behaviour was changed for a reason, to prevent accidental use of dangerous behaviour, and I'm not too keen on undoing that move for all software that uses KDECompilerSettings. Sune Vuorela wrote: agreed. Kevin Funk wrote: Yep. I'm also *strongly* in favor of adjusting the code instead of enabling the define. In fact, I thought I've fixed all of KF5. (It isn't?). There are some compile errors in code *using* KF5, which I'm trying to port ASAP. Axel Rasmussen wrote: Would we prefer just using `static_cast` like QExplicitlySharedDataPointer used to do, or shall we use `dynamic_cast` and check for `NULL` results to be extra safe? I am willing to write up a patch that fixes this issue in KService over the next day or two. Alex Merry wrote: I'd expect most of the cases to be suitable for `static_cast` (if you can guarantee what it will be from the context). If in doubt, state your uncertainty in the review request description so that reviewers know to take extra care. Note: KService has already been fixed two weeks ago -- see https://git.reviewboard.kde.org/r/119241/ - Kevin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119564/#review63665 --- On Aug. 1, 2014, 4:06 p.m., Axel Rasmussen wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119564/ --- (Updated Aug. 1, 2014, 4:06 p.m.) Review request for Build System, Extra Cmake Modules and KDE Frameworks. Bugs: 337472 http://bugs.kde.org/show_bug.cgi?id=337472 Repository: extra-cmake-modules Description --- Upstream Qt commit e112c2e altered the way QExplicitlySharedDataPointer behaves by default, such that it no longer uses a `static_cast` to cast compatible pointers. A nontrivial amount of KDE code (I encountered the build error in KService, although there are other users) depends on this functionality, so this commit adds a define to the build system which re-enables the code in Qt. Diffs - kde-modules/KDECompilerSettings.cmake fdc930e Diff: https://git.reviewboard.kde.org/r/119564/diff/ Testing --- I applied this patch, attempted to build KService, and the build succeeded, rather than exhibiting the compile errors found in the log provided on the bug report. Thanks, Axel Rasmussen ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119564: Add define to re-enable Qt functionality we depend on.
On Aug. 2, 2014, 9:04 a.m., Alex Merry wrote: I would rather change the code. The Qt behaviour was changed for a reason, to prevent accidental use of dangerous behaviour, and I'm not too keen on undoing that move for all software that uses KDECompilerSettings. agreed. - Sune --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119564/#review63665 --- On Aug. 1, 2014, 4:06 p.m., Axel Rasmussen wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119564/ --- (Updated Aug. 1, 2014, 4:06 p.m.) Review request for Build System, Extra Cmake Modules and KDE Frameworks. Bugs: 337472 http://bugs.kde.org/show_bug.cgi?id=337472 Repository: extra-cmake-modules Description --- Upstream Qt commit e112c2e altered the way QExplicitlySharedDataPointer behaves by default, such that it no longer uses a `static_cast` to cast compatible pointers. A nontrivial amount of KDE code (I encountered the build error in KService, although there are other users) depends on this functionality, so this commit adds a define to the build system which re-enables the code in Qt. Diffs - kde-modules/KDECompilerSettings.cmake fdc930e Diff: https://git.reviewboard.kde.org/r/119564/diff/ Testing --- I applied this patch, attempted to build KService, and the build succeeded, rather than exhibiting the compile errors found in the log provided on the bug report. Thanks, Axel Rasmussen ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119564: Add define to re-enable Qt functionality we depend on.
On Aug. 2, 2014, 9:04 a.m., Alex Merry wrote: I would rather change the code. The Qt behaviour was changed for a reason, to prevent accidental use of dangerous behaviour, and I'm not too keen on undoing that move for all software that uses KDECompilerSettings. Sune Vuorela wrote: agreed. Yep. I'm also *strongly* in favor of adjusting the code instead of enabling the define. In fact, I thought I've fixed all of KF5. (It isn't?). There are some compile errors in code *using* KF5, which I'm trying to port ASAP. - Kevin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119564/#review63665 --- On Aug. 1, 2014, 4:06 p.m., Axel Rasmussen wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119564/ --- (Updated Aug. 1, 2014, 4:06 p.m.) Review request for Build System, Extra Cmake Modules and KDE Frameworks. Bugs: 337472 http://bugs.kde.org/show_bug.cgi?id=337472 Repository: extra-cmake-modules Description --- Upstream Qt commit e112c2e altered the way QExplicitlySharedDataPointer behaves by default, such that it no longer uses a `static_cast` to cast compatible pointers. A nontrivial amount of KDE code (I encountered the build error in KService, although there are other users) depends on this functionality, so this commit adds a define to the build system which re-enables the code in Qt. Diffs - kde-modules/KDECompilerSettings.cmake fdc930e Diff: https://git.reviewboard.kde.org/r/119564/diff/ Testing --- I applied this patch, attempted to build KService, and the build succeeded, rather than exhibiting the compile errors found in the log provided on the bug report. Thanks, Axel Rasmussen ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119564: Add define to re-enable Qt functionality we depend on.
On Aug. 2, 2014, 9:04 a.m., Alex Merry wrote: I would rather change the code. The Qt behaviour was changed for a reason, to prevent accidental use of dangerous behaviour, and I'm not too keen on undoing that move for all software that uses KDECompilerSettings. Sune Vuorela wrote: agreed. Kevin Funk wrote: Yep. I'm also *strongly* in favor of adjusting the code instead of enabling the define. In fact, I thought I've fixed all of KF5. (It isn't?). There are some compile errors in code *using* KF5, which I'm trying to port ASAP. Would we prefer just using `static_cast` like QExplicitlySharedDataPointer used to do, or shall we use `dynamic_cast` and check for `NULL` results to be extra safe? I am willing to write up a patch that fixes this issue in KService over the next day or two. - Axel --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119564/#review63665 --- On Aug. 1, 2014, 4:06 p.m., Axel Rasmussen wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119564/ --- (Updated Aug. 1, 2014, 4:06 p.m.) Review request for Build System, Extra Cmake Modules and KDE Frameworks. Bugs: 337472 http://bugs.kde.org/show_bug.cgi?id=337472 Repository: extra-cmake-modules Description --- Upstream Qt commit e112c2e altered the way QExplicitlySharedDataPointer behaves by default, such that it no longer uses a `static_cast` to cast compatible pointers. A nontrivial amount of KDE code (I encountered the build error in KService, although there are other users) depends on this functionality, so this commit adds a define to the build system which re-enables the code in Qt. Diffs - kde-modules/KDECompilerSettings.cmake fdc930e Diff: https://git.reviewboard.kde.org/r/119564/diff/ Testing --- I applied this patch, attempted to build KService, and the build succeeded, rather than exhibiting the compile errors found in the log provided on the bug report. Thanks, Axel Rasmussen ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119564: Add define to re-enable Qt functionality we depend on.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119564/#review63665 --- I would rather change the code. The Qt behaviour was changed for a reason, to prevent accidental use of dangerous behaviour, and I'm not too keen on undoing that move for all software that uses KDECompilerSettings. - Alex Merry On Aug. 1, 2014, 4:06 p.m., Axel Rasmussen wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119564/ --- (Updated Aug. 1, 2014, 4:06 p.m.) Review request for Build System, Extra Cmake Modules and KDE Frameworks. Bugs: 337472 http://bugs.kde.org/show_bug.cgi?id=337472 Repository: extra-cmake-modules Description --- Upstream Qt commit e112c2e altered the way QExplicitlySharedDataPointer behaves by default, such that it no longer uses a `static_cast` to cast compatible pointers. A nontrivial amount of KDE code (I encountered the build error in KService, although there are other users) depends on this functionality, so this commit adds a define to the build system which re-enables the code in Qt. Diffs - kde-modules/KDECompilerSettings.cmake fdc930e Diff: https://git.reviewboard.kde.org/r/119564/diff/ Testing --- I applied this patch, attempted to build KService, and the build succeeded, rather than exhibiting the compile errors found in the log provided on the bug report. Thanks, Axel Rasmussen ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 119564: Add define to re-enable Qt functionality we depend on.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119564/ --- Review request for Build System, Extra Cmake Modules and KDE Frameworks. Bugs: 337472 http://bugs.kde.org/show_bug.cgi?id=337472 Repository: extra-cmake-modules Description --- Upstream Qt commit e112c2e altered the way QExplicitlySharedDataPointer behaves by default, such that it no longer uses a `static_cast` to cast compatible pointers. A nontrivial amount of KDE code (I encountered the build error in KService, although there are other users) depends on this functionality, so this commit adds a define to the build system which re-enables the code in Qt. Diffs - kde-modules/KDECompilerSettings.cmake fdc930e Diff: https://git.reviewboard.kde.org/r/119564/diff/ Testing --- I applied this patch, attempted to build KService, and the build succeeded, rather than exhibiting the compile errors found in the log provided on the bug report. Thanks, Axel Rasmussen ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
