Review Request 121263: Prevent API abuse of calling setters on temporary objects.

Christoph Cullmann cullmann at kde.org
Mon Dec 8 19:41:39 UTC 2014



> On Nov. 27, 2014, 7:29 a.m., 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...

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?


- Christoph


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121263/#review70996
-----------------------------------------------------------


On Nov. 27, 2014, 1:15 a.m., 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 a.m.)
> 
> 
> 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/20141208/1b93831a/attachment.html>


More information about the Kde-frameworks-devel mailing list