Review Request 121263: Prevent API abuse of calling setters on temporary objects.
Dominik Haumann
dhaumann at kde.org
Sat Dec 13 13:34:08 UTC 2014
> On Nov. 27, 2014, 7:29 vorm., Christoph Cullmann wrote:
> > I actually would prefer no such hack in the public headers.
> > If that is just to make porting easier, you can use that locally as a patch until the kdevplatform code is cleaned up.
>
> Milian Wolff wrote:
> I still don't get why you think this is a hack, or why it would be bad to have it in public headers. Any consumer of your API could shoot yourself in the foot...
>
> Christoph Cullmann wrote:
> I must rephrase: I think, without any guard define, this is not even source compatible (even if the use might be in most cases an error).
> And with a guard define, this is a hack, as you need to turn it on, which most people won't do at all.
> It might have been a good idea to add to the API in KF 5.0, but as we missed that, its now too late, or?
@Milian: is it correct, that this is/was only an issue in KDevelop?
- Dominik
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121263/#review70996
-----------------------------------------------------------
On Nov. 27, 2014, 1:15 vorm., Milian Wolff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121263/
> -----------------------------------------------------------
>
> (Updated Nov. 27, 2014, 1:15 vorm.)
>
>
> Review request for KDE Frameworks, Christoph Cullmann, Dominik Haumann, and Kevin Funk.
>
>
> Repository: ktexteditor
>
>
> Description
> -------
>
> In KDevelop we currently hit this often since our old class
> previously returned a reference for the start/end getters of range
> and cursor. With the help of C++11 ref qualifiers, we can detect that
> and let the compiler give the user an error message:
>
> error: cannot initialize object parameter of type 'KTextEditor::Cursor'
> with an expression of type 'KTextEditor::Cursor'
> documentRange().start().setColumn(42);
> ^~~~~~~~~~~~~~~~~~~~~~~
>
> Yes, the error message is pretty bad. But better than nothing? We
> could also mark the && overloads of these functions as explictily
> deleted, which would slightly improve the error message...
>
>
> Diffs
> -----
>
> src/include/CMakeLists.txt 94b8e79e2f2b273ec344a963ba6ac81ec5a481c6
> src/include/ktexteditor/cursor.h 4ebe38fc1bffb2dad02150884fd225fe3ca9e193
> src/include/ktexteditor/global.h PRE-CREATION
> src/include/ktexteditor/range.h 1a2fc5b200c70364c3d99223e43a2ad6179055de
>
> Diff: https://git.reviewboard.kde.org/r/121263/diff/
>
>
> Testing
> -------
>
> with the fixes to kdev's codebase, all of ktexteditor, kate and kdev* builds fine.
>
>
> Thanks,
>
> Milian Wolff
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20141213/92a6fa2e/attachment.html>
More information about the Kde-frameworks-devel
mailing list