Re: [C++ PATCH] Fix -Winvalid-offsetof warning (PR c++/68727)

[Jason Merrill]

OK.

On Tue, Jan 24, 2017 at 5:43 PM, Jakub Jelinek  wrote:
> Hi!
>
> r215070 moved -Winvalid-offsetof pedwarn from build_class_member_access_expr
> (where it warned even about offsetof-like hand-written constructs) to
> finish_offsetof.  That is fine, the problem is that while
> build_class_member_access_expr saw the original first argument to
> __builtin_offsetof (the object type), finish_offsetof can see a different
> type and the first argument of COMPONENT_REF is a result of various folding
> (e.g. build_base_path (prematurely?) folds trees etc.), so it would be too
> hard to reliably discover the original type.
> Now, if the type finish_offsetof sees is e.g. a std layout type,
> while the original type is not, we don't emit a warning that we should.
> Instead of trying to discover the original type from the folded expression,
> the following patch passes the type (well, a cast of null pointer to
> pointer to that type, as that is something we have handy) to finish_offsetof
> and for template instantiation saves it in a new OFFSETOF_EXPR argument.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2017-01-24  Jakub Jelinek  
>
> PR c++/68727
> * cp-tree.def (OFFSETOF_EXPR): Bump number of operands to 2.
> * cp-tree.h (finish_offsetof): Add OBJECT_PTR argument.
> * parser.c (cp_parser_builtin_offsetof): Pass result of
> build_static_cast of null_pointer_node to finish_offsetof.
> * semantics.c (finish_offsetof): Add OBJECT_PTR argument, use
> it for -Winvalid-offsetof pedwarn instead of trying to guess
> original offsetof type from EXPR.  Save OBJECT_PTR as a new
> second operand to OFFSETOF_EXPR.
> * pt.c (tsubst_copy_and_build) : Adjust
> finish_offsetof caller, pass the second operand of OFFSETOF_EXPR
> as OBJECT_PTR.
>
> * g++.dg/other/offsetof8.C: Add expected error.
> * g++.dg/other/offsetof9.C: New test.
>
> --- gcc/cp/cp-tree.def.jj   2017-01-10 08:12:46.0 +0100
> +++ gcc/cp/cp-tree.def  2017-01-24 14:27:00.907820968 +0100
> @@ -333,7 +333,7 @@ DEFTREECODE (EXPR_STMT, "expr_stmt", tcc
>  DEFTREECODE (TAG_DEFN, "tag_defn", tcc_expression, 0)
>
>  /* Represents an 'offsetof' expression during template expansion.  */
> -DEFTREECODE (OFFSETOF_EXPR, "offsetof_expr", tcc_expression, 1)
> +DEFTREECODE (OFFSETOF_EXPR, "offsetof_expr", tcc_expression, 2)
>
>  /* Represents an '__builtin_addressof' expression during template
> expansion.  This is similar to ADDR_EXPR, but it doesn't invoke
> --- gcc/cp/cp-tree.h.jj 2017-01-21 02:26:07.0 +0100
> +++ gcc/cp/cp-tree.h2017-01-24 14:14:51.132548587 +0100
> @@ -6485,7 +6485,7 @@ extern tree finish_underlying_type
>  extern tree calculate_bases (tree);
>  extern tree finish_bases(tree, bool);
>  extern tree calculate_direct_bases  (tree);
> -extern tree finish_offsetof(tree, location_t);
> +extern tree finish_offsetof(tree, tree, location_t);
>  extern void finish_decl_cleanup(tree, tree);
>  extern void finish_eh_cleanup  (tree);
>  extern void emit_associated_thunks (tree);
> --- gcc/cp/parser.c.jj  2017-01-21 02:26:06.0 +0100
> +++ gcc/cp/parser.c 2017-01-24 14:18:22.482716966 +0100
> @@ -9498,11 +9498,12 @@ cp_parser_builtin_offsetof (cp_parser *p
>token = cp_lexer_peek_token (parser->lexer);
>
>/* Build the (type *)null that begins the traditional offsetof macro.  */
> -  expr = build_static_cast (build_pointer_type (type), null_pointer_node,
> -tf_warning_or_error);
> +  tree object_ptr
> += build_static_cast (build_pointer_type (type), null_pointer_node,
> +tf_warning_or_error);
>
>/* Parse the offsetof-member-designator.  We begin as if we saw "expr->".  
> */
> -  expr = cp_parser_postfix_dot_deref_expression (parser, CPP_DEREF, expr,
> +  expr = cp_parser_postfix_dot_deref_expression (parser, CPP_DEREF, 
> object_ptr,
>  true, , 
> token->location);
>while (true)
>  {
> @@ -9554,7 +9555,7 @@ cp_parser_builtin_offsetof (cp_parser *p
>loc = make_location (loc, start_loc, finish_loc);
>/* The result will be an INTEGER_CST, so we need to explicitly
>   preserve the location.  */
> -  expr = cp_expr (finish_offsetof (expr, loc), loc);
> +  expr = cp_expr (finish_offsetof (object_ptr, expr, loc), loc);
>
>   failure:
>parser->integral_constant_expression_p = save_ice_p;
> --- gcc/cp/semantics.c.jj   2017-01-21 02:26:06.0 +0100
> +++ gcc/cp/semantics.c  2017-01-24 14:30:05.024371882 +0100
> @@ -3995,13 +3995,13 @@ finish_bases (tree type, bool direct)
> fold_offsetof.  */
>
>  tree
> -finish_offsetof (tree expr, location_t loc)
> +finish_offsetof 

[C++ PATCH] Fix -Winvalid-offsetof warning (PR c++/68727)

[Jakub Jelinek]

Hi!

r215070 moved -Winvalid-offsetof pedwarn from build_class_member_access_expr
(where it warned even about offsetof-like hand-written constructs) to
finish_offsetof.  That is fine, the problem is that while
build_class_member_access_expr saw the original first argument to
__builtin_offsetof (the object type), finish_offsetof can see a different
type and the first argument of COMPONENT_REF is a result of various folding
(e.g. build_base_path (prematurely?) folds trees etc.), so it would be too
hard to reliably discover the original type.
Now, if the type finish_offsetof sees is e.g. a std layout type,
while the original type is not, we don't emit a warning that we should.
Instead of trying to discover the original type from the folded expression,
the following patch passes the type (well, a cast of null pointer to
pointer to that type, as that is something we have handy) to finish_offsetof
and for template instantiation saves it in a new OFFSETOF_EXPR argument.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2017-01-24  Jakub Jelinek  

PR c++/68727
* cp-tree.def (OFFSETOF_EXPR): Bump number of operands to 2.
* cp-tree.h (finish_offsetof): Add OBJECT_PTR argument.
* parser.c (cp_parser_builtin_offsetof): Pass result of
build_static_cast of null_pointer_node to finish_offsetof.
* semantics.c (finish_offsetof): Add OBJECT_PTR argument, use
it for -Winvalid-offsetof pedwarn instead of trying to guess
original offsetof type from EXPR.  Save OBJECT_PTR as a new
second operand to OFFSETOF_EXPR.
* pt.c (tsubst_copy_and_build) : Adjust
finish_offsetof caller, pass the second operand of OFFSETOF_EXPR
as OBJECT_PTR.

* g++.dg/other/offsetof8.C: Add expected error.
* g++.dg/other/offsetof9.C: New test.

--- gcc/cp/cp-tree.def.jj   2017-01-10 08:12:46.0 +0100
+++ gcc/cp/cp-tree.def  2017-01-24 14:27:00.907820968 +0100
@@ -333,7 +333,7 @@ DEFTREECODE (EXPR_STMT, "expr_stmt", tcc
 DEFTREECODE (TAG_DEFN, "tag_defn", tcc_expression, 0)
 
 /* Represents an 'offsetof' expression during template expansion.  */
-DEFTREECODE (OFFSETOF_EXPR, "offsetof_expr", tcc_expression, 1)
+DEFTREECODE (OFFSETOF_EXPR, "offsetof_expr", tcc_expression, 2)
 
 /* Represents an '__builtin_addressof' expression during template
expansion.  This is similar to ADDR_EXPR, but it doesn't invoke
--- gcc/cp/cp-tree.h.jj 2017-01-21 02:26:07.0 +0100
+++ gcc/cp/cp-tree.h2017-01-24 14:14:51.132548587 +0100
@@ -6485,7 +6485,7 @@ extern tree finish_underlying_type
 extern tree calculate_bases (tree);
 extern tree finish_bases(tree, bool);
 extern tree calculate_direct_bases  (tree);
-extern tree finish_offsetof(tree, location_t);
+extern tree finish_offsetof(tree, tree, location_t);
 extern void finish_decl_cleanup(tree, tree);
 extern void finish_eh_cleanup  (tree);
 extern void emit_associated_thunks (tree);
--- gcc/cp/parser.c.jj  2017-01-21 02:26:06.0 +0100
+++ gcc/cp/parser.c 2017-01-24 14:18:22.482716966 +0100
@@ -9498,11 +9498,12 @@ cp_parser_builtin_offsetof (cp_parser *p
   token = cp_lexer_peek_token (parser->lexer);
 
   /* Build the (type *)null that begins the traditional offsetof macro.  */
-  expr = build_static_cast (build_pointer_type (type), null_pointer_node,
-tf_warning_or_error);
+  tree object_ptr
+= build_static_cast (build_pointer_type (type), null_pointer_node,
+tf_warning_or_error);
 
   /* Parse the offsetof-member-designator.  We begin as if we saw "expr->".  */
-  expr = cp_parser_postfix_dot_deref_expression (parser, CPP_DEREF, expr,
+  expr = cp_parser_postfix_dot_deref_expression (parser, CPP_DEREF, object_ptr,
 true, , token->location);
   while (true)
 {
@@ -9554,7 +9555,7 @@ cp_parser_builtin_offsetof (cp_parser *p
   loc = make_location (loc, start_loc, finish_loc);
   /* The result will be an INTEGER_CST, so we need to explicitly
  preserve the location.  */
-  expr = cp_expr (finish_offsetof (expr, loc), loc);
+  expr = cp_expr (finish_offsetof (object_ptr, expr, loc), loc);
 
  failure:
   parser->integral_constant_expression_p = save_ice_p;
--- gcc/cp/semantics.c.jj   2017-01-21 02:26:06.0 +0100
+++ gcc/cp/semantics.c  2017-01-24 14:30:05.024371882 +0100
@@ -3995,13 +3995,13 @@ finish_bases (tree type, bool direct)
fold_offsetof.  */
 
 tree
-finish_offsetof (tree expr, location_t loc)
+finish_offsetof (tree object_ptr, tree expr, location_t loc)
 {
   /* If we're processing a template, we can't finish the semantics yet.
  Otherwise we can fold the entire expression now.  */
   if (processing_template_decl)
 {
-  expr = build1 (OFFSETOF_EXPR,