diff mbox

[C++] PR 53158

Message ID 4FA8132B.2020904@oracle.com
State New
Headers show

Commit Message

Paolo Carlini May 7, 2012, 6:23 p.m. UTC 
Hi,

On 05/07/2012 08:07 PM, Manuel López-Ibáñez wrote:
> On 7 May 2012 19:40, Paolo Carlini<paolo.carlini@oracle.com>  wrote:
>> On 05/07/2012 07:18 PM, Paolo Carlini wrote:
>>
>> a.cc:5:12: error: could not convert ‘foo()’ from ‘void’ to ‘bool’
>>    if (foo())
>>
>> Is this the error message we want to see for the former testcase, or we want
>> something slightly different?
> If "foo()" is printed with %qE, and now that we have caret
> diagnostics, I would expect the message to be:
>
> error: could not convert expression from 'void' to 'bool'
A basic fix seems reasonably easy (modulo the caret in the wrong place, 
I guess we have to pass down more locations): instead of just 
unconditionally forwarding to c_common_truthvalue_conversion from 
cp_truthvalue_conversion, which seems a bad idea, given this comment 
preceding the former:

/* Prepare expr to be an argument of a TRUTH_NOT_EXPR,
    or for an `if' or `while' statement or ?..: exp.  It should already
    have been validated to be of suitable type; otherwise, a bad
    diagnostic may result.

note in particular the last two lines ;) we could tell apart CALL_EXPRs 
and pass those through condition_conversion (tree expr). Seems ok also 
from the optimization point of view, becase CALL_EXPRs are not handled 
by c_common_truthvalue_conversion, and just forwarded to 
cp_build_binary_op to build an "appropriate" (incorrect in this specific 
case) NE_EXPR.

Something like the quick hack below gives:

a.cc:5:20: error: could not convert ‘foo()’ from ‘void’ to ‘bool’
    if (foo() && a < b)

Paolo.

//////////////////////

CLEANUP_POINT_EXPR.  */

Comments

Paolo Carlini May 7, 2012, 6:35 p.m. UTC | #1 
On 05/07/2012 08:23 PM, Paolo Carlini wrote:
> Index: typeck.c
> ===================================================================
> --- typeck.c    (revision 187249)
> +++ typeck.c    (working copy)
> @@ -4782,7 +4782,11 @@ cp_truthvalue_conversion (tree expr)
>        return ret;
>      }
>    else
> -    return c_common_truthvalue_conversion (input_location, expr);
> +    {
> +      if (TREE_CODE (expr) == CALL_EXPR)
> +       return condition_conversion (expr);
> +      return c_common_truthvalue_conversion (input_location, expr);
> +    }
>  }
>
>  /* Just like cp_truthvalue_conversion, but we want a 
> CLEANUP_POINT_EXPR.  */
Well, not exactly like this because it recurses forever when the code is 
fine ;) but I think you get the idea, we don't want to unconditionally 
use c_common_truthvalue_conversion and we want to stop *before* trying 
to synthesize a NE_EXPR.

About the error message proper, the locations are currently pretty wrong 
for these code paths - is always too advanced, at the end of the 
expression, I'm afraid fixing that will require touching quite a few 
lines of code.

Paolo.
Jason Merrill May 8, 2012, 2:10 a.m. UTC | #2 
How do we even get to calling cp_truthvalue_conversion?

Jason
diff mbox

Patch

Index: typeck.c
===================================================================
--- typeck.c    (revision 187249)
+++ typeck.c    (working copy)
@@ -4782,7 +4782,11 @@  cp_truthvalue_conversion (tree expr)
        return ret;
      }
    else
-    return c_common_truthvalue_conversion (input_location, expr);
+    {
+      if (TREE_CODE (expr) == CALL_EXPR)
+       return condition_conversion (expr);
+      return c_common_truthvalue_conversion (input_location, expr);
+    }
  }

  /* Just like cp_truthvalue_conversion, but we want a