diff mbox series

[C++] Deprecate ARM-era for scopes

Message ID e576547c-09c5-0e98-9ea1-a43305205b82@acm.org
State New
Headers show
Series [C++] Deprecate ARM-era for scopes | expand

Commit Message

Nathan Sidwell Jan. 23, 2018, 12:18 p.m. UTC 
As discussed (https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01778.html) 
this patch deprecates the ARM-era for scope.

a) in c++98 mode with -fpermissive, there's now a deprecation note when 
we fix up something like
    for (int i = ...) {}
    ... i ... // out of scope use of i

b) -fno-for-scope gives a deprecated warning

I noticed that the flag showed signs of being tri-valued at some point, 
but it is no longer (I suspect at some point we only attempted #a if 
neither sense of -ffor-scope was given).  Cleaned that up.

Applying to trunk.

nathan

Comments

Mike Stump Jan. 23, 2018, 6:27 p.m. UTC | #1 
On Jan 23, 2018, at 4:18 AM, Nathan Sidwell <nathan@acm.org> wrote:
> 
> As discussed (https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01778.html) this patch deprecates the ARM-era for scope.

The code gives:

  if you use %<-fpermissive%> G++ will accept your code

I think we should no longer recommend a deprecated feature without at least stating it is deprecated in the hint?  Not too important, as if they actually use it, the compiler will spit out:

  this flexibility is deprecated and will be removed

if someone tried to use it, but thought I might mention it.
Nathan Sidwell Jan. 23, 2018, 7:02 p.m. UTC | #2 
On 01/23/2018 01:27 PM, Mike Stump wrote:
> On Jan 23, 2018, at 4:18 AM, Nathan Sidwell <nathan@acm.org> wrote:
>>
>> As discussed (https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01778.html) this patch deprecates the ARM-era for scope.
> 
> The code gives:
> 
>    if you use %<-fpermissive%> G++ will accept your code
> 
> I think we should no longer recommend a deprecated feature without at least stating it is deprecated in the hint?  Not too important, as if they actually use it, the compiler will spit out:
> 
>    this flexibility is deprecated and will be removed
> 
> if someone tried to use it, but thought I might mention it.

yeah, I hemmed and hawed about that, but given how bitrotted those 
diagnostics had become punted.

nathan
diff mbox series

Patch

2018-01-23  Nathan Sidwell  <nathan@acm.org>

	gcc/cp/
	Deprecate ARM-era for scope handling
	* decl.c (poplevel): Flag_new_for_scope is a boolean-like.
	(cxx_init_decl_processing): Deprecate flag_new_for_scope being
	cleared.
	* name-lookup.c (check_for_out_of_scope_variable): Deprecate and
	cleanup handling.
	* semantics.c (begin_for_scope): Flag_new_for_scope is
	boolean-like.
	(finish_for_stmt, begin_range_for_stmt): Likewise.

	gcc/
	* doc/invoke.texi (ffor-scope): Deprecate.

	gcc/cp/
	* g++.dg/cpp0x/range-for10.C: Adjust.
	* g++.dg/ext/forscope1.C: Adjust.
	* g++.dg/ext/forscope2.C: Adjust.
	* g++.dg/template/for1.C: Adjust.

Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 256951)
+++ cp/decl.c	(working copy)
@@ -644,7 +644,7 @@  poplevel (int keep, int reverse, int fun
      in a init statement were in scope after the for-statement ended.
      We only use the new rules if flag_new_for_scope is nonzero.  */
   leaving_for_scope
-    = current_binding_level->kind == sk_for && flag_new_for_scope == 1;
+    = current_binding_level->kind == sk_for && flag_new_for_scope;
 
   /* Before we remove the declarations first check for unused variables.  */
   if ((warn_unused_variable || warn_unused_but_set_variable)
@@ -4094,6 +4094,8 @@  cxx_init_decl_processing (void)
   pop_namespace ();
 
   flag_noexcept_type = (cxx_dialect >= cxx17);
+  if (!flag_new_for_scope)
+    warning (OPT_Wdeprecated, "%<-fno-for-scope%> is deprecated");
 
   c_common_nodes_and_builtins ();
 
Index: cp/name-lookup.c
===================================================================
--- cp/name-lookup.c	(revision 256951)
+++ cp/name-lookup.c	(working copy)
@@ -3231,7 +3231,9 @@  push_local_binding (tree id, tree decl,
    standard.  If so, issue an error message.  If name lookup would
    work in both cases, but return a different result, this function
    returns the result of ANSI/ISO lookup.  Otherwise, it returns
-   DECL.  */
+   DECL.
+
+   FIXME: Scheduled for removal after GCC-8 is done.  */
 
 tree
 check_for_out_of_scope_variable (tree decl)
@@ -3252,16 +3254,16 @@  check_for_out_of_scope_variable (tree de
     shadowed = find_namespace_value (current_namespace, DECL_NAME (decl));
   if (shadowed)
     {
-      if (!DECL_ERROR_REPORTED (decl))
+      if (!DECL_ERROR_REPORTED (decl)
+	  && flag_permissive
+	  && warning (0, "name lookup of %qD changed", DECL_NAME (decl)))
 	{
-	  warning (0, "name lookup of %qD changed", DECL_NAME (decl));
-	  warning_at (DECL_SOURCE_LOCATION (shadowed), 0,
-		      "  matches this %qD under ISO standard rules",
-		      shadowed);
-	  warning_at (DECL_SOURCE_LOCATION (decl), 0,
-		      "  matches this %qD under old rules", decl);
-	  DECL_ERROR_REPORTED (decl) = 1;
+	  inform (DECL_SOURCE_LOCATION (shadowed),
+		  "matches this %qD under ISO standard rules", shadowed);
+	  inform (DECL_SOURCE_LOCATION (decl),
+		  "  matches this %qD under old rules", decl);
 	}
+      DECL_ERROR_REPORTED (decl) = 1;
       return shadowed;
     }
 
@@ -3279,26 +3281,25 @@  check_for_out_of_scope_variable (tree de
     {
       error ("name lookup of %qD changed for ISO %<for%> scoping",
 	     DECL_NAME (decl));
-      error ("  cannot use obsolete binding at %q+D because "
-	     "it has a destructor", decl);
+      inform (DECL_SOURCE_LOCATION (decl),
+	      "cannot use obsolete binding %qD because it has a destructor",
+	      decl);
       return error_mark_node;
     }
   else
     {
-      permerror (input_location, "name lookup of %qD changed for ISO %<for%> scoping",
+      permerror (input_location,
+		 "name lookup of %qD changed for ISO %<for%> scoping",
 	         DECL_NAME (decl));
       if (flag_permissive)
-        permerror (DECL_SOURCE_LOCATION (decl),
-		   "  using obsolete binding at %qD", decl);
-      else
-	{
-	  static bool hint;
-	  if (!hint)
-	    {
-	      inform (input_location, "(if you use %<-fpermissive%> G++ will accept your code)");
-	      hint = true;
-	    }
-	}
+        inform (DECL_SOURCE_LOCATION (decl),
+		"using obsolete binding %qD", decl);
+      static bool hint;
+      if (!hint)
+	inform (input_location, flag_permissive
+		? "this flexibility is deprecated and will be removed"
+		: "if you use %<-fpermissive%> G++ will accept your code");
+      hint = true;
     }
 
   return decl;
Index: cp/semantics.c
===================================================================
--- cp/semantics.c	(revision 256951)
+++ cp/semantics.c	(working copy)
@@ -931,7 +931,7 @@  tree
 begin_for_scope (tree *init)
 {
   tree scope = NULL_TREE;
-  if (flag_new_for_scope > 0)
+  if (flag_new_for_scope)
     scope = do_pushlevel (sk_for);
 
   if (processing_template_decl)
@@ -956,7 +956,7 @@  begin_for_stmt (tree scope, tree init)
 
   if (scope == NULL_TREE)
     {
-      gcc_assert (!init || !(flag_new_for_scope > 0));
+      gcc_assert (!init || !flag_new_for_scope);
       if (!init)
 	scope = begin_for_scope (&init);
     }
@@ -1053,7 +1053,7 @@  finish_for_stmt (tree for_stmt)
     FOR_BODY (for_stmt) = do_poplevel (FOR_BODY (for_stmt));
 
   /* Pop the scope for the body of the loop.  */
-  if (flag_new_for_scope > 0)
+  if (flag_new_for_scope)
     {
       tree scope;
       tree *scope_ptr = (TREE_CODE (for_stmt) == RANGE_FOR_STMT
@@ -1082,7 +1082,7 @@  begin_range_for_stmt (tree scope, tree i
 
   if (scope == NULL_TREE)
     {
-      gcc_assert (!init || !(flag_new_for_scope > 0));
+      gcc_assert (!init || !flag_new_for_scope);
       if (!init)
 	scope = begin_for_scope (&init);
     }
Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi	(revision 256951)
+++ doc/invoke.texi	(working copy)
@@ -2467,9 +2467,8 @@  a @i{for-init-statement} extends to the
 as was the case in old versions of G++, and other (traditional)
 implementations of C++.
 
-If neither flag is given, the default is to follow the standard,
-but to allow and give a warning for old-style code that would
-otherwise be invalid, or have different behavior.
+This option is deprecated and the associated non-standard
+functionality will be removed.
 
 @item -fno-gnu-keywords
 @opindex fno-gnu-keywords
Index: testsuite/g++.dg/cpp0x/range-for10.C
===================================================================
--- testsuite/g++.dg/cpp0x/range-for10.C	(revision 256951)
+++ testsuite/g++.dg/cpp0x/range-for10.C	(working copy)
@@ -1,6 +1,6 @@ 
 // PR c++/47388
 // { dg-do compile { target c++11 } }
-// { dg-options "-fno-for-scope" }
+// { dg-options "-fno-for-scope -Wno-deprecated" }
 
 template <int>
 void
Index: testsuite/g++.dg/ext/forscope1.C
===================================================================
--- testsuite/g++.dg/ext/forscope1.C	(revision 256951)
+++ testsuite/g++.dg/ext/forscope1.C	(working copy)
@@ -1,5 +1,5 @@ 
 // { dg-do compile }
-// { dg-options -fno-for-scope }
+// { dg-options "-fno-for-scope -Wno-deprecated" }
 
 // Copyright (C) 2001 Free Software Foundation, Inc.
 // Contributed by Nathan Sidwell 4 Sept 2001 <nathan@codesourcery.com>
Index: testsuite/g++.dg/ext/forscope2.C
===================================================================
--- testsuite/g++.dg/ext/forscope2.C	(revision 256951)
+++ testsuite/g++.dg/ext/forscope2.C	(working copy)
@@ -16,7 +16,7 @@  struct A
 
 void Go( )
 {
-  for (int i = 1;;)	// { dg-warning "using obsolete binding" }
+  for (int i = 1;;)	// { dg-message "using obsolete binding" }
     {
       switch (1) {
       default: {}
Index: testsuite/g++.dg/template/for1.C
===================================================================
--- testsuite/g++.dg/template/for1.C	(revision 256951)
+++ testsuite/g++.dg/template/for1.C	(working copy)
@@ -1,6 +1,6 @@ 
 // PR c++/47388
 // { dg-do compile }
-// { dg-options "-fno-for-scope" }
+// { dg-options "-fno-for-scope -Wno-deprecated" }
 
 template <int>
 void