Review Request: parenthesis and not null checking

Jaime Torres Amate jtamate at gmail.com
Sun Nov 11 09:03:17 GMT 2012



> On Nov. 10, 2012, 10:21 p.m., Rolf Eike Beer wrote:
> > A simple way to verify if this is correct is check if you and the compiler agree on the code. Run "make -n" in the build tree to get the full compiler command line, then insert a "-S" (assuming you are using gcc) and change the -o to point to a temporary file. This will output the assembler code. Do this with and without your modifications and look if the result is still the same.

yes, I've done it. They match.

bool kk1()
{
 int a = 1;
 int b = 2;
 return (!a ^ b);
}

bool kk2()
{
 int a = 1;
 int b = 2;
 return (!(a ^ b));
}

bool kk3()
{
 int a = 1;
 int b = 2;
 return (!(a) ^ b);
}

kk1 and kk3 produce the same assembler output.

But the problem is if the author really wanted the parenthesis that way or not.
for example, (!a) ^ b  (what gcc produces) vs. !(a ^ b) (what the author wanted?)


- Jaime Torres


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107213/#review21785
-----------------------------------------------------------


On Nov. 5, 2012, 3:13 p.m., Jaime Torres Amate wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107213/
> -----------------------------------------------------------
> 
> (Updated Nov. 5, 2012, 3:13 p.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Description
> -------
> 
> place some parenthesis around ! operators, with less priority than ^ operators.
> place some parenthesis around = inside conditions 
> check for n not being null before using it
> simplify if (a==true) return true else return false;
> 
> 
> Diffs
> -----
> 
>   khtml/khtml_caret.cpp 45fd90c 
>   khtml/rendering/render_inline.cpp acfc1e4 
>   khtml/rendering/render_object.cpp f37627c 
>   solid/solid/backends/wmi/wmiopticaldisc.cpp c6e390f 
> 
> Diff: http://git.reviewboard.kde.org/r/107213/diff/
> 
> 
> Testing
> -------
> 
> I've been using this code several weeks.
> 
> 
> Thanks,
> 
> Jaime Torres Amate
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20121111/471cac45/attachment.htm>


More information about the kde-core-devel mailing list