diff mbox

C++ PATCH to -Winvalid-offsetof

Message ID 540EEB91.3070905@redhat.com
State New
Headers show

Commit Message

Jason Merrill Sept. 9, 2014, 11:59 a.m. UTC
-Winvalid-offsetof was introduced before __builtin_offsetof, and the 
comments both in the code and the testsuite note that even though 
offsetof on a non-standard-layout type is undefined, the usual expansion 
seems to be well-defined.  So -Winvalid-offsetof should move to 
offsetof-specific code, and we can upgrade the warning to a pedwarn.

Tested x86_64-pc-linux-gnu, applying to trunk.
diff mbox

Patch

commit faa1550a6c8075c4190143a48d92f6f52e746356
Author: Jason Merrill <jason@redhat.com>
Date:   Mon Sep 8 13:21:41 2014 -0400

    	* typeck.c (build_class_member_access_expr): Move
    	-Winvalid-offsetof code...
    	* semantics.c (finish_offsetof): ...here.
    	* parser.c (cp_parser_builtin_offsetof): Remember the location of
    	the type argument.
    	* pt.c (tsubst_copy_and_build) [OFFSETOF_EXPR]: Preserve it.

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 05fa86a..204ed42 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -8577,6 +8577,7 @@  cp_parser_builtin_offsetof (cp_parser *parser)
   /* Consume the opening `('.  */
   cp_parser_require (parser, CPP_OPEN_PAREN, RT_OPEN_PAREN);
   /* Parse the type-id.  */
+  location_t loc = cp_lexer_peek_token (parser->lexer)->location;
   type = cp_parser_type_id (parser);
   /* Look for the `,'.  */
   cp_parser_require (parser, CPP_COMMA, RT_COMMA);
@@ -8630,6 +8631,9 @@  cp_parser_builtin_offsetof (cp_parser *parser)
     }
 
  success:
+  if (CAN_HAVE_LOCATION_P (expr))
+    SET_EXPR_LOCATION (expr, 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)
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 38093ec..b778644 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -15440,7 +15440,12 @@  tsubst_copy_and_build (tree t,
       }
 
     case OFFSETOF_EXPR:
-      RETURN (finish_offsetof (RECUR (TREE_OPERAND (t, 0))));
+      {
+	tree expr = RECUR (TREE_OPERAND (t, 0));
+	if (CAN_HAVE_LOCATION_P (expr))
+	  SET_EXPR_LOCATION (expr, EXPR_LOCATION (TREE_OPERAND (t, 0)));
+	RETURN (finish_offsetof (expr));
+      }
 
     case TRAIT_EXPR:
       {
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 776fa26..cd43384 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -3816,6 +3816,7 @@  finish_bases (tree type, bool direct)
 tree
 finish_offsetof (tree expr)
 {
+  location_t loc = EXPR_LOCATION (expr);
   if (TREE_CODE (expr) == PSEUDO_DTOR_EXPR)
     {
       error ("cannot apply %<offsetof%> to destructor %<~%T%>",
@@ -3846,6 +3847,13 @@  finish_offsetof (tree expr)
       tree object = TREE_OPERAND (expr, 0);
       if (!complete_type_or_else (TREE_TYPE (object), object))
 	return error_mark_node;
+      if (warn_invalid_offsetof
+	  && CLASS_TYPE_P (TREE_TYPE (object))
+	  && CLASSTYPE_NON_STD_LAYOUT (TREE_TYPE (object))
+	  && cp_unevaluated_operand == 0)
+	pedwarn (loc, OPT_Winvalid_offsetof,
+		 "offsetof within non-standard-layout type %qT is undefined",
+		 TREE_TYPE (object));
     }
   return fold_offsetof (expr);
 }
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 05fd48e..aa82f1c 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -2359,10 +2359,8 @@  build_class_member_access_expr (tree object, tree member,
 	    {
 	      if (complain & tf_error)
 		{
-		  error ("invalid access to non-static data member %qD of "
-			 "NULL object",
-			 member);
-		  error ("(perhaps the %<offsetof%> macro was used incorrectly)");
+		  error ("invalid access to non-static data member %qD in "
+			 "virtual base of NULL object", member);
 		}
 	      return error_mark_node;
 	    }
@@ -2375,27 +2373,6 @@  build_class_member_access_expr (tree object, tree member,
 	  gcc_assert (object != error_mark_node);
 	}
 
-      /* Complain about other invalid uses of offsetof, even though they will
-	 give the right answer.  Note that we complain whether or not they
-	 actually used the offsetof macro, since there's no way to know at this
-	 point.  So we just give a warning, instead of a pedwarn.  */
-      /* Do not produce this warning for base class field references, because
-	 we know for a fact that didn't come from offsetof.  This does occur
-	 in various testsuite cases where a null object is passed where a
-	 vtable access is required.  */
-      if (null_object_p && warn_invalid_offsetof
-	  && CLASSTYPE_NON_STD_LAYOUT (object_type)
-	  && !DECL_FIELD_IS_BASE (member)
-	  && cp_unevaluated_operand == 0
-	  && (complain & tf_warning))
-	{
-	  warning (OPT_Winvalid_offsetof, 
-                   "invalid access to non-static data member %qD "
-                   " of NULL object", member);
-	  warning (OPT_Winvalid_offsetof, 
-                   "(perhaps the %<offsetof%> macro was used incorrectly)");
-	}
-
       /* If MEMBER is from an anonymous aggregate, we have converted
 	 OBJECT so that it refers to the class containing the
 	 anonymous union.  Generate a reference to the anonymous union
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 55e6d56..a4a345d 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -5097,12 +5097,10 @@  warnings produced by @option{-Winline} to appear or disappear.
 @opindex Wno-invalid-offsetof
 @opindex Winvalid-offsetof
 Suppress warnings from applying the @samp{offsetof} macro to a non-POD
-type.  According to the 1998 ISO C++ standard, applying @samp{offsetof}
-to a non-POD type is undefined.  In existing C++ implementations,
-however, @samp{offsetof} typically gives meaningful results even when
-applied to certain kinds of non-POD types (such as a simple
-@samp{struct} that fails to be a POD type only by virtue of having a
-constructor).  This flag is for users who are aware that they are
+type.  According to the 2014 ISO C++ standard, applying @samp{offsetof}
+to a non-standard-layout type is undefined.  In existing C++ implementations,
+however, @samp{offsetof} typically gives meaningful results.
+This flag is for users who are aware that they are
 writing nonportable code and who have deliberately chosen to ignore the
 warning about it.
 
diff --git a/gcc/testsuite/g++.dg/abi/offsetof.C b/gcc/testsuite/g++.dg/abi/offsetof.C
index d6a53e6..5a66511 100644
--- a/gcc/testsuite/g++.dg/abi/offsetof.C
+++ b/gcc/testsuite/g++.dg/abi/offsetof.C
@@ -4,7 +4,6 @@ 
 // implementation thereof.
 
 // Yes, this is bad, naughty, evil code.  But it seems to be well-formed.
-// So we'll just warn.
 
 // { dg-do run }
 
@@ -18,5 +17,5 @@  struct C: public B { };
 
 int main ()
 {
-  return ((__SIZE_TYPE__) &((C*)0)->i) != sizeof(void*); // { dg-warning "offsetof|invalid" "" }
+  return ((__SIZE_TYPE__) &((C*)0)->i) != sizeof(void*);
 }
diff --git a/gcc/testsuite/g++.dg/other/offsetof3.C b/gcc/testsuite/g++.dg/other/offsetof3.C
index 5946c81..8d98242 100644
--- a/gcc/testsuite/g++.dg/other/offsetof3.C
+++ b/gcc/testsuite/g++.dg/other/offsetof3.C
@@ -1,7 +1,6 @@ 
-/* Verify that offsetof warns if given a non-standard-layout class */
+/* Verify that offsetof complains if given a non-standard-layout class.  */
 /* Copyright (C) 2003 Free Software Foundation, Inc. */
 /* Contributed by Matt Austern <austern@apple.com> 15 May 2003 */
-/* { dg-do compile } */
 
 struct X
 {
@@ -13,5 +12,4 @@  protected:
 typedef X* pX;
 typedef __SIZE_TYPE__ size_t;
 
-size_t yoff = size_t(&(pX(0)->y)); /* { dg-warning "invalid access" "" } */
-/* { dg-warning "macro was used incorrectly" "macro" { target *-*-* } 16 } */
+size_t yoff = __builtin_offsetof (X, y); /* { dg-error "35:non-standard-layout" } */
diff --git a/gcc/testsuite/g++.dg/other/offsetof5.C b/gcc/testsuite/g++.dg/other/offsetof5.C
index b53b06f..86b1448 100644
--- a/gcc/testsuite/g++.dg/other/offsetof5.C
+++ b/gcc/testsuite/g++.dg/other/offsetof5.C
@@ -9,14 +9,14 @@  struct A
   int &i;
 };
 
-int j = offsetof (A, i);		// { dg-warning "invalid access|offsetof" }
+int j = offsetof (A, i);		// { dg-error "offsetof" }
 
 template <typename T>
 struct S
 {
   T h;
   T &i;
-  static const int j = offsetof (S, i);	// { dg-warning "invalid access|offsetof" }
+  static const int j = offsetof (S, i);	// { dg-error "offsetof" }
 };
 
 int k = S<int>::j;			// { dg-message "required from here" }