This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: G++ PATCH: static_cast rejecting valid casts


Jason Merrill wrote:
> Your patch is more general than necessary; the passage you quote only
> applies to single-level pointer conversions.  Indeed, the only conversions
> that static_cast can perform between two multi-level pointers are also
> standard conversions.
Yes, I don't think the patch lets through any casts which it shouldn't,
but which of the following diagnostics would you prefer for
	static_cast <void ***> ((void const ***)expr)
static_cast from `const void ***' to `void ***'
or
static_cast from `const void ***' to `void ***' casts away constness

I think the latter is more informative. As we're informing of const
problems in some cases, user's will probably expect us to be
diagnosing all cases where it is a constness problem which invalidates
the cast. casts_away_constness carefully implements that check -- we
should make use of it. However, if you'd prefer the former, the first
attached patch just turns `T cv * cv' into `T *'.

The current behaviour with this example is to fail in the
can_convert_arg test (because it's not a standard conversion), and
then succeed in the TYPE_PTROB_P tests, as both types are
pointers to object. As they're not pointers to aggr, we don't find
a conversion. We never get to try the reversed conversion sequence
in the final part of the test.

You still need to apply this cv removal to the arguments of the
can_convert_arg call to deal with the case of casting a
const derived * to non-const base *.  In that case the cast is
invalid, but you'd like to say the reason is the loss of constness,
rather than the lack of relationship between the two classes (because
they are related, of course).

There are some conversions which fail in both directions due to
constness problems i.e `const void **const *' to/from
`void *const**', and it'd be nice to say it's a const problem for those
too. I just noticed that my original patch didn't get this case, as
only stripping the qualifiers off the source type fails the final
qualification conversion condition of [4.4]/4, hence both source and
target types need the qualifiers stripping in the call to
can_convert_arg.

I attach two patches, both minor variants of the original patch.
The first just turns `T cv * cv' into `T *' with
strip_first_pointer_quals. This, I beleive, is the minimum to
acheive conformance in static_cast. The second adds an additional
call to strip_all_pointer_quals in the can_convert_arg call to
catch the (const invalid) multi-level pointer conversions. This
gives more informative diagnostics by distinguishing when it is
const correctness which is invalidiating the cast.

The previous test case still passes with either patch.

Let me know what you think.

nathan

-- 
Dr Nathan Sidwell :: Computer Science Department :: Bristol University
Never hand someone a gun unless you are sure where they will point it
nathan@acm.org  http://www.cs.bris.ac.uk/~nathan/  nathan@cs.bris.ac.uk
1999-12-16  Nathan Sidwell  <nathan@acm.org>

	* typeck.c (strip_first_pointer_quals): New static function.
	(build_static_cast): Use it. Don't use at_least_as_qualified_p.

Index: cp/typeck.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/cp/typeck.c,v
retrieving revision 1.229
diff -c -3 -p -r1.229 typeck.c
*** typeck.c	1999/12/15 12:39:02	1.229
--- typeck.c	1999/12/16 10:37:02
*************** static int comp_cv_target_types PROTO((t
*** 65,70 ****
--- 65,71 ----
  static void casts_away_constness_r PROTO((tree *, tree *));
  static int casts_away_constness PROTO ((tree, tree));
  static void maybe_warn_about_returning_address_of_local PROTO ((tree));
+ static tree strip_first_pointer_quals PROTO ((tree));
  
  /* Return the target type of TYPE, which means return T for:
     T*, T&, T[], T (...), and otherwise, just T.  */
*************** build_static_cast (type, expr)
*** 5182,5195 ****
    /* FIXME handle casting to array type.  */
  
    ok = 0;
!   if (can_convert_arg (type, intype, expr))
      ok = 1;
    else if (TYPE_PTROB_P (type) && TYPE_PTROB_P (intype))
      {
        tree binfo;
        if (IS_AGGR_TYPE (TREE_TYPE (type)) && IS_AGGR_TYPE (TREE_TYPE (intype))
- 	  && at_least_as_qualified_p (TREE_TYPE (type),
- 				      TREE_TYPE (intype))
  	  && (binfo = get_binfo (TREE_TYPE (intype), TREE_TYPE (type), 0))
  	  && ! TREE_VIA_VIRTUAL (binfo))
  	ok = 1;
--- 5183,5194 ----
    /* FIXME handle casting to array type.  */
  
    ok = 0;
!   if (can_convert_arg (type, strip_first_pointer_quals (intype), expr))
      ok = 1;
    else if (TYPE_PTROB_P (type) && TYPE_PTROB_P (intype))
      {
        tree binfo;
        if (IS_AGGR_TYPE (TREE_TYPE (type)) && IS_AGGR_TYPE (TREE_TYPE (intype))
  	  && (binfo = get_binfo (TREE_TYPE (intype), TREE_TYPE (type), 0))
  	  && ! TREE_VIA_VIRTUAL (binfo))
  	ok = 1;
*************** build_static_cast (type, expr)
*** 5198,5205 ****
      {
        if (same_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (TREE_TYPE (type))),
  		       TYPE_MAIN_VARIANT (TREE_TYPE (TREE_TYPE (intype))))
- 	  && at_least_as_qualified_p (TREE_TYPE (TREE_TYPE (type)),
- 				      TREE_TYPE (TREE_TYPE (intype)))
  	  && (binfo = get_binfo (TYPE_OFFSET_BASETYPE (TREE_TYPE (type)),
  				 TYPE_OFFSET_BASETYPE (TREE_TYPE (intype)), 0))
  	  && ! TREE_VIA_VIRTUAL (binfo))
--- 5197,5202 ----
*************** build_static_cast (type, expr)
*** 5208,5220 ****
    else if (TREE_CODE (intype) != BOOLEAN_TYPE
  	   && TREE_CODE (type) != ARRAY_TYPE
  	   && TREE_CODE (type) != FUNCTION_TYPE
! 	   && can_convert (intype, type))
      ok = 1;
  
    /* [expr.static.cast]
  
       The static_cast operator shall not be used to cast away
!      constnes.  */
    if (ok && casts_away_constness (intype, type))
      {
        cp_error ("static_cast from `%T' to `%T' casts away constness",
--- 5205,5217 ----
    else if (TREE_CODE (intype) != BOOLEAN_TYPE
  	   && TREE_CODE (type) != ARRAY_TYPE
  	   && TREE_CODE (type) != FUNCTION_TYPE
! 	   && can_convert (intype, strip_first_pointer_quals (type)))
      ok = 1;
  
    /* [expr.static.cast]
  
       The static_cast operator shall not be used to cast away
!      constness.  */
    if (ok && casts_away_constness (intype, type))
      {
        cp_error ("static_cast from `%T' to `%T' casts away constness",
*************** casts_away_constness (t1, t2)
*** 7172,7175 ****
--- 7169,7186 ----
      return 1;
  
    return 0;
+ }
+ 
+ /* Remove the outermost cv qualifiers of TYPE.
+    When TYPE is a pointer type, remove the cv qualifiers from the
+    pointed to type as well.  */
+ 
+ static tree
+ strip_first_pointer_quals (type)
+      tree type;
+ {
+   if (TREE_CODE (type) == POINTER_TYPE)
+     return build_pointer_type (strip_top_quals (TREE_TYPE (type)));
+   else
+     return strip_top_quals (type);
  }
1999-12-16  Nathan Sidwell  <nathan@acm.org>

	* typeck.c (strip_all_pointer_quals): New static function.
	(build_static_cast): Use it. Don't use at_least_as_qualified_p.

Index: cp/typeck.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/cp/typeck.c,v
retrieving revision 1.229
diff -c -3 -p -r1.229 typeck.c
*** typeck.c	1999/12/15 12:39:02	1.229
--- typeck.c	1999/12/16 11:24:59
*************** static int comp_cv_target_types PROTO((t
*** 65,70 ****
--- 65,71 ----
  static void casts_away_constness_r PROTO((tree *, tree *));
  static int casts_away_constness PROTO ((tree, tree));
  static void maybe_warn_about_returning_address_of_local PROTO ((tree));
+ static tree strip_all_pointer_quals PROTO ((tree));
  
  /* Return the target type of TYPE, which means return T for:
     T*, T&, T[], T (...), and otherwise, just T.  */
*************** build_static_cast (type, expr)
*** 5182,5195 ****
    /* FIXME handle casting to array type.  */
  
    ok = 0;
!   if (can_convert_arg (type, intype, expr))
      ok = 1;
    else if (TYPE_PTROB_P (type) && TYPE_PTROB_P (intype))
      {
        tree binfo;
        if (IS_AGGR_TYPE (TREE_TYPE (type)) && IS_AGGR_TYPE (TREE_TYPE (intype))
- 	  && at_least_as_qualified_p (TREE_TYPE (type),
- 				      TREE_TYPE (intype))
  	  && (binfo = get_binfo (TREE_TYPE (intype), TREE_TYPE (type), 0))
  	  && ! TREE_VIA_VIRTUAL (binfo))
  	ok = 1;
--- 5183,5195 ----
    /* FIXME handle casting to array type.  */
  
    ok = 0;
!   if (can_convert_arg (strip_all_pointer_quals (type),
!                        strip_all_pointer_quals (intype), expr))
      ok = 1;
    else if (TYPE_PTROB_P (type) && TYPE_PTROB_P (intype))
      {
        tree binfo;
        if (IS_AGGR_TYPE (TREE_TYPE (type)) && IS_AGGR_TYPE (TREE_TYPE (intype))
  	  && (binfo = get_binfo (TREE_TYPE (intype), TREE_TYPE (type), 0))
  	  && ! TREE_VIA_VIRTUAL (binfo))
  	ok = 1;
*************** build_static_cast (type, expr)
*** 5198,5205 ****
      {
        if (same_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (TREE_TYPE (type))),
  		       TYPE_MAIN_VARIANT (TREE_TYPE (TREE_TYPE (intype))))
- 	  && at_least_as_qualified_p (TREE_TYPE (TREE_TYPE (type)),
- 				      TREE_TYPE (TREE_TYPE (intype)))
  	  && (binfo = get_binfo (TYPE_OFFSET_BASETYPE (TREE_TYPE (type)),
  				 TYPE_OFFSET_BASETYPE (TREE_TYPE (intype)), 0))
  	  && ! TREE_VIA_VIRTUAL (binfo))
--- 5198,5203 ----
*************** build_static_cast (type, expr)
*** 5208,5220 ****
    else if (TREE_CODE (intype) != BOOLEAN_TYPE
  	   && TREE_CODE (type) != ARRAY_TYPE
  	   && TREE_CODE (type) != FUNCTION_TYPE
! 	   && can_convert (intype, type))
      ok = 1;
  
    /* [expr.static.cast]
  
       The static_cast operator shall not be used to cast away
!      constnes.  */
    if (ok && casts_away_constness (intype, type))
      {
        cp_error ("static_cast from `%T' to `%T' casts away constness",
--- 5206,5218 ----
    else if (TREE_CODE (intype) != BOOLEAN_TYPE
  	   && TREE_CODE (type) != ARRAY_TYPE
  	   && TREE_CODE (type) != FUNCTION_TYPE
! 	   && can_convert (intype, strip_all_pointer_quals (type)))
      ok = 1;
  
    /* [expr.static.cast]
  
       The static_cast operator shall not be used to cast away
!      constness.  */
    if (ok && casts_away_constness (intype, type))
      {
        cp_error ("static_cast from `%T' to `%T' casts away constness",
*************** casts_away_constness (t1, t2)
*** 7172,7175 ****
--- 7170,7187 ----
      return 1;
  
    return 0;
+ }
+ 
+ /* Returns TYPE with its cv qualifiers removed
+    TYPE is T cv* .. *cv where T is not a pointer type,
+    returns T * .. *  */
+ 
+ static tree
+ strip_all_pointer_quals (type)
+      tree type;
+ {
+   if (TREE_CODE (type) == POINTER_TYPE)
+     return build_pointer_type (strip_all_pointer_quals (TREE_TYPE (type)));
+   else
+     return strip_top_quals (type);
  }

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]