Modified CondExprNode generate_evaluation_code to resolve gh-5555

[oleksandr-pavlyk]

Only wrap conditional statement in parenthesis if it does not start with ( or does not end with ) already.

This change resolves gh-5555 and its duplicate gh-5584 which I filed.

@da-woods Please let me know if you'd like tests to be added.

[da-woods]

I'm never hugely keen on inspecting strings of generated code, but I could believe this is the best solution for this problem.

Testing: We do now have some tests that match regexes in the generated code. I think we also have some tests where we make C warnings an error. Personally I'm inclined not to do either here. The main reason is that the warnings came from the cython.array utility code so they aren't a particularly clean or minimal thing to test.

One code-style comment, but other than that I'm happy. I'll leave it a day or two for other comments though.

[scoder]

I'm never hugely keen on inspecting strings of generated code

Me neither.

One thing that seems to work for me is to wrap the condition in a no-op macro:

if (__Pyx_IsCTrue((...))) {

emits no warning even if I just #define __Pyx_IsCTrue(x) x.

Obfuscates the C code, though.

[scoder]

This change also works for me:

diff --git a/Cython/Compiler/ExprNodes.py b/Cython/Compiler/ExprNodes.py
index 94c8be17d..e64cec695 100644
--- a/Cython/Compiler/ExprNodes.py
+++ b/Cython/Compiler/ExprNodes.py
@@ -12800,7 +12800,7 @@ class CondExprNode(ExprNode):
         return self.true_val.is_ephemeral() or self.false_val.is_ephemeral()
 
     def analyse_types(self, env):
-        self.test = self.test.analyse_types(env).coerce_to_boolean(env)
+        self.test = self.test.analyse_temp_boolean_expression(env)
         self.true_val = self.true_val.analyse_types(env)
         self.false_val = self.false_val.analyse_types(env)
         return self.analyse_result_type(env)

It aligns conditional expressions with what we do for if-statements. It uses an additional temporary for the condition result, but that's a cheap cost, I'd say.

[scoder]

I've applied this change to the master branch.

[da-woods]

Slightly irrelevant now, but I'm not sure how robust the string editing version would be. e.g. dereferencing and calling a function pointer:

(*fptr)(arg)

I'm not sure if Cython generates that exact code, but it'd have been worth investigating. (Missed this on my first look)