[prev in list] [next in list] [prev in thread] [next in thread]
List: kwin
Subject: Re: Review Request: Make KWin compile with C++11
From: Martin_Gräßlin <kde () martin-graesslin ! com>
Date: 2012-04-26 19:40:24
Message-ID: 20120426194024.17844.64188 () vidsolbach ! de
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
> On April 26, 2012, 10:37 a.m., Thomas Lübking wrote:
> > > For some casts like the double to float in the compositor
> > > it would be more useful to use float values in the first
> > > place.
> >
> > +1
> >
> > actually it should still compile unless you set "-Werror=narrowing"?
> >
> > Aside this, let's come to what i consider the most braindead part of c++11:
> > // snip
> > int i[2] = {1.0, 2.0}; // stupid therefore not allowed
> > int j = {1.0}; // who would do so? still not allowed
> > int k = 1.2; // ok, but that's fine?????
> >
> > tenthousands LOC later:
> > i[0] = 1.2; i[1] = 2.1; // but hey, why not? - this is ok.
> > i[1000] = 123.456; // still just fine...
> > // /snip
> >
> > => let's add many static casts in and notably the new uniform init (yes, i know \
> > it's supposed to become the general and only one and it's pretty cool, but this \
> > narrowing protection is just half-baked - and nasty ;-) /rant
I did not get "-Werror=narrowing" to work, but maybe I'm just too stupid to adjust \
cmake ;-)
I'll probably discard this review and add changes for compositor and xrender, so that \
we only need the casts for the X stuff.
- Martin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104728/#review12922
-----------------------------------------------------------
On April 25, 2012, 7:56 p.m., Martin Gräßlin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104728/
> -----------------------------------------------------------
>
> (Updated April 25, 2012, 7:56 p.m.)
>
>
> Review request for kwin.
>
>
> Description
> -------
>
> Enabled c++0x support in gcc4.6 and run into some issues...
>
> There are some "narrowing conversion" errors which are
> no longer allowed with C++11. Use explicit casts to
> smaller datatype if it's correct in that case.
>
> For some casts like the double to float in the compositor
> it would be more useful to use float values in the first
> place.
>
>
> Diffs
> -----
>
> kwin/client.cpp 017d4292133dfe2d66f6348dff10573990c972db
> kwin/lanczosfilter.cpp 010acc2ae55eb5ffa5fefce9ec4eb146e6536cde
> kwin/overlaywindow.cpp 528dade5df85df907d8ecdc60fd43917c8de8964
> kwin/scene_opengl.cpp 43d30ad55a0f5e5aa7f52cafe9945bb53b99c84e
> kwin/scene_xrender.cpp 265825cb82e8c7959f0ac8cf82623d3a3537dcfc
>
> Diff: http://git.reviewboard.kde.org/r/104728/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Martin Gräßlin
>
>
[Attachment #5 (text/html)]
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 \
solid;"> <tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/104728/">http://git.reviewboard.kde.org/r/104728/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <p style="margin-top: 0;">On April 26th, 2012, 10:37 a.m., <b>Thomas \
Lübking</b> wrote:</p> <blockquote style="margin-left: 1em; border-left: 2px solid \
#d0d0d0; padding-left: 10px;"> <pre style="white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">> For some casts like the double to float in the compositor > it \
would be more useful to use float values in the first > place.
+1
actually it should still compile unless you set "-Werror=narrowing"?
Aside this, let's come to what i consider the most braindead part of c++11:
// snip
int i[2] = {1.0, 2.0}; // stupid therefore not allowed
int j = {1.0}; // who would do so? still not allowed
int k = 1.2; // ok, but that's fine?????
tenthousands LOC later:
i[0] = 1.2; i[1] = 2.1; // but hey, why not? - this is ok.
i[1000] = 123.456; // still just fine...
// /snip
=> let's add many static casts in and notably the new uniform init (yes, i \
know it's supposed to become the general and only one and it's pretty cool, \
but this narrowing protection is just half-baked - and nasty ;-) /rant</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I did not get \
"-Werror=narrowing" to work, but maybe I'm just too stupid to adjust \
cmake ;-)
I'll probably discard this review and add changes for compositor and xrender, so \
that we only need the casts for the X stuff.</pre> <br />
<p>- Martin</p>
<br />
<p>On April 25th, 2012, 7:56 p.m., Martin Gräßlin wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black \
solid;"> <tr>
<td>
<div>Review request for kwin.</div>
<div>By Martin Gräßlin.</div>
<p style="color: grey;"><i>Updated April 25, 2012, 7:56 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">Enabled c++0x support in gcc4.6 and run into some issues...
There are some "narrowing conversion" errors which are
no longer allowed with C++11. Use explicit casts to
smaller datatype if it's correct in that case.
For some casts like the double to float in the compositor
it would be more useful to use float values in the first
place.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kwin/client.cpp <span style="color: \
grey">(017d4292133dfe2d66f6348dff10573990c972db)</span></li>
<li>kwin/lanczosfilter.cpp <span style="color: \
grey">(010acc2ae55eb5ffa5fefce9ec4eb146e6536cde)</span></li>
<li>kwin/overlaywindow.cpp <span style="color: \
grey">(528dade5df85df907d8ecdc60fd43917c8de8964)</span></li>
<li>kwin/scene_opengl.cpp <span style="color: \
grey">(43d30ad55a0f5e5aa7f52cafe9945bb53b99c84e)</span></li>
<li>kwin/scene_xrender.cpp <span style="color: \
grey">(265825cb82e8c7959f0ac8cf82623d3a3537dcfc)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/104728/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
_______________________________________________
kwin mailing list
kwin@kde.org
https://mail.kde.org/mailman/listinfo/kwin
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic