[C++] Deprecate -ffriend-injection

Message ID 34da4f83-10e6-7644-d8d8-be55abf1cb85@acm.org
State New
Headers show
Series
  • [C++] Deprecate -ffriend-injection
Related show

Commit Message

Nathan Sidwell Feb. 16, 2018, 3:44 p.m.
This patch deprecates the -ffriend-injection option.
1) issue a deprecated warning, if the option is set.
2) if we do inject a visible friend, issue a warning.  This warning is 
not inhibitable, like the for-scope behaviour.  (Notice we never 
injected visible friend classes, only functions).
3) Update the 'backwards compatibility' section to mention this.

I noticed that at the point we warn about the flag, input_location is 
BUILTINS_LOCATION.  There isn't a known fixed location for 
<command-line> or <main-input-file>.  I think using UNKNOWN_LOCATION, 
which generates 'cc1plus: diag...', is less confusing than '<builtins>: 
diag...'.

'Now that there is a definitive ISO standard C++, G++ has a 
specification to adhere to.'

We now have several :)

nathan
-- 
Nathan Sidwell

Comments

Sandra Loosemore Feb. 16, 2018, 6:52 p.m. | #1
On 02/16/2018 08:44 AM, Nathan Sidwell wrote:
> Index: doc/extend.texi

> ===================================================================

> --- doc/extend.texi	(revision 257739)

> +++ doc/extend.texi	(working copy)

> @@ -23881,11 +23881,23 @@ deprecated.   @xref{Deprecated Features}

>  

>  @table @code

>  @item For scope

> -If a variable is declared at for scope, it used to remain in scope until

> -the end of the scope that contained the for statement (rather than just

> -within the for scope).  G++ retains this, but issues a warning, if such a

> +If a variable is declared at for scope, it used to remain in scope

> +until the end of the scope that contained the for statement (rather

> +than just within the for scope).  The deprecated

> +@option{-fno-for-scope} option enables this non-standard behaviour.

> +Without the option, G++ retains this, but issues a warning, if such a

>  variable is accessed outside the for scope.

>  

> +The behaviour is deprecated, only available with @option{-std=c++98}

> +@option{-std=gnu++98} languages and you must use the

> +@option{-fpermissive} option to enable it.  The behaviour will be

> +removed.

> +

> +@item Friend Injection

> +The @option{-ffriend-injection} option makes injected friends visible

> +to regular name lookup, unlike standard C++.  This option is

> +deprecated and will be removed.

> +

>  @item Implicit C language

>  Old C system header files did not contain an @code{extern "C" @{@dots{}@}}

>  scope to set the language.  On such systems, all header files are


The GCC documentation conventions say to use American spellings, not 
British....  so in the patch hunk please

s/behaviour/behavior/g

-Sandra the nit-picky
Jason Merrill Feb. 16, 2018, 6:54 p.m. | #2
My dejagnu doesn't like "cc1plus" as a line number:

ERROR: g++.old-deja/g++.jason/scoping15.C  -std=gnu++98: expected
integer but got "cc1plus:" for " dg-warning 25 "ffriend-injection.* is
deprecated" "" { target *-*-* } cc1plus: "

On Fri, Feb 16, 2018 at 10:44 AM, Nathan Sidwell <nathan@acm.org> wrote:
> This patch deprecates the -ffriend-injection option.

> 1) issue a deprecated warning, if the option is set.

> 2) if we do inject a visible friend, issue a warning.  This warning is not

> inhibitable, like the for-scope behaviour.  (Notice we never injected

> visible friend classes, only functions).

> 3) Update the 'backwards compatibility' section to mention this.

>

> I noticed that at the point we warn about the flag, input_location is

> BUILTINS_LOCATION.  There isn't a known fixed location for <command-line> or

> <main-input-file>.  I think using UNKNOWN_LOCATION, which generates

> 'cc1plus: diag...', is less confusing than '<builtins>: diag...'.

>

> 'Now that there is a definitive ISO standard C++, G++ has a specification to

> adhere to.'

>

> We now have several :)

>

> nathan

> --

> Nathan Sidwell
Jakub Jelinek Feb. 16, 2018, 6:57 p.m. | #3
On Fri, Feb 16, 2018 at 01:54:15PM -0500, Jason Merrill wrote:
> My dejagnu doesn't like "cc1plus" as a line number:

> 

> ERROR: g++.old-deja/g++.jason/scoping15.C  -std=gnu++98: expected

> integer but got "cc1plus:" for " dg-warning 25 "ffriend-injection.* is

> deprecated" "" { target *-*-* } cc1plus: "


Usually 0 is used in that spot for lineno of UNKNOWN_LOCATION.

	Jakub
Nathan Sidwell Feb. 16, 2018, 7:06 p.m. | #4
On 02/16/2018 01:52 PM, Sandra Loosemore wrote:

> The GCC documentation conventions say to use American spellings, not 

> British....  so in the patch hunk please

> 

> s/behaviour/behavior/g


American imperialist :)

nathan
-- 
Nathan Sidwell
2018-02-16  Nathan Sidwell  <nathan@acm.org>

	* doc/extend.texi (Backwards Compatibility): Americanize 'behaviour'.

Index: gcc/doc/extend.texi
===================================================================
--- gcc/doc/extend.texi	(revision 257742)
+++ gcc/doc/extend.texi	(working copy)
@@ -23884,13 +23884,13 @@ deprecated.   @xref{Deprecated Features}
 If a variable is declared at for scope, it used to remain in scope
 until the end of the scope that contained the for statement (rather
 than just within the for scope).  The deprecated
-@option{-fno-for-scope} option enables this non-standard behaviour.
+@option{-fno-for-scope} option enables this non-standard behavior.
 Without the option, G++ retains this, but issues a warning, if such a
 variable is accessed outside the for scope.
 
-The behaviour is deprecated, only available with @option{-std=c++98}
+The behavior is deprecated, only available with @option{-std=c++98}
 @option{-std=gnu++98} languages and you must use the
-@option{-fpermissive} option to enable it.  The behaviour will be
+@option{-fpermissive} option to enable it.  The behavior will be
 removed.
 
 @item Friend Injection
Nathan Sidwell Feb. 16, 2018, 7:32 p.m. | #5
On 02/16/2018 01:54 PM, Jason Merrill wrote:
> My dejagnu doesn't like "cc1plus" as a line number:

> 

> ERROR: g++.old-deja/g++.jason/scoping15.C  -std=gnu++98: expected

> integer but got "cc1plus:" for " dg-warning 25 "ffriend-injection.* is

> deprecated" "" { target *-*-* } cc1plus: "


I think I got distracted halfway though editing that.  Fixed.

nathan
-- 
Nathan Sidwell
2018-02-16  Nathan Sidwell  <nathan@acm.org>

	* g++.old-deja/g++.jason/scoping15.C: Fix dg-warning.

Index: g++.old-deja/g++.jason/scoping15.C
===================================================================
--- g++.old-deja/g++.jason/scoping15.C	(revision 257742)
+++ g++.old-deja/g++.jason/scoping15.C	(working copy)
@@ -13,7 +13,7 @@ public:
 
 class FComplex {
 public:
-  friend  float    imag(const FComplex& a); // { dg-warning "is visible"
+  friend  float    imag(const FComplex& a); // { dg-warning "is visible" }
 };
 
 void
@@ -22,4 +22,4 @@ scnrm2(FComplex cx[])
   int imag;
   ::imag( cx[0] );
 }
-// { dg-warning "ffriend-injection.* is deprecated" "" { target *-*-* } cc1plus: }
+// { dg-warning "ffriend-injection.* is deprecated" "cc1plus:" { target *-*-* } 0 }

Patch

2018-02-16  Nathan Sidwell  <nathan@acm.org>

	Deprecate -ffriend-injection.
	* decl.c (cxx_init_decl_processing): Emit warning on option.
	* name-lookup.c (do_pushdecl): Emit warning if we push a visible
	friend.

	* doc/extend.texi (Backwards Compatibility): Mention friend
	injection.  Note for-scope is deprecated.
	* doc/invoke.texi (-ffriend-injection): Deprecate.

	* g++.old-deja/g++.jason/scoping15.C: Expect warnings.
	* g++.old-deja/g++.mike/net43.C: Likewise.

Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 257739)
+++ cp/decl.c	(working copy)
@@ -4091,8 +4091,14 @@  cxx_init_decl_processing (void)
   pop_namespace ();
 
   flag_noexcept_type = (cxx_dialect >= cxx17);
+  /* There's no fixed location for <command-line>, the current
+     location is <builtins>, which is somewhat confusing.  */
   if (!flag_new_for_scope)
-    warning (OPT_Wdeprecated, "%<-fno-for-scope%> is deprecated");
+    warning_at (UNKNOWN_LOCATION, OPT_Wdeprecated,
+		"%<-fno-for-scope%> is deprecated");
+  if (flag_friend_injection)
+    warning_at (UNKNOWN_LOCATION, OPT_Wdeprecated,
+		"%<-ffriend-injection%> is deprecated");
 
   c_common_nodes_and_builtins ();
 
Index: cp/name-lookup.c
===================================================================
--- cp/name-lookup.c	(revision 257739)
+++ cp/name-lookup.c	(working copy)
@@ -3071,6 +3071,7 @@  do_pushdecl (tree decl, bool is_friend)
 	old = OVL_CHAIN (old);
 
       check_template_shadow (decl);
+      bool visible_injection = false;
 
       if (DECL_DECLARES_FUNCTION_P (decl))
 	{
@@ -3091,6 +3092,8 @@  do_pushdecl (tree decl, bool is_friend)
 	      if (!flag_friend_injection)
 		/* Hide it from ordinary lookup.  */
 		DECL_ANTICIPATED (decl) = DECL_HIDDEN_FRIEND_P (decl) = true;
+	      else
+		visible_injection = true;
 	    }
 	}
 
@@ -3142,6 +3145,9 @@  do_pushdecl (tree decl, bool is_friend)
 	}
       else if (VAR_P (decl))
 	maybe_register_incomplete_var (decl);
+      else if (visible_injection)
+	warning (0, "injected friend %qD is visible"
+		" due to %<-ffriend-injection%>", decl);
 
       if ((VAR_P (decl) || TREE_CODE (decl) == FUNCTION_DECL)
 	  && DECL_EXTERN_C_P (decl))
Index: doc/extend.texi
===================================================================
--- doc/extend.texi	(revision 257739)
+++ doc/extend.texi	(working copy)
@@ -23881,11 +23881,23 @@  deprecated.   @xref{Deprecated Features}
 
 @table @code
 @item For scope
-If a variable is declared at for scope, it used to remain in scope until
-the end of the scope that contained the for statement (rather than just
-within the for scope).  G++ retains this, but issues a warning, if such a
+If a variable is declared at for scope, it used to remain in scope
+until the end of the scope that contained the for statement (rather
+than just within the for scope).  The deprecated
+@option{-fno-for-scope} option enables this non-standard behaviour.
+Without the option, G++ retains this, but issues a warning, if such a
 variable is accessed outside the for scope.
 
+The behaviour is deprecated, only available with @option{-std=c++98}
+@option{-std=gnu++98} languages and you must use the
+@option{-fpermissive} option to enable it.  The behaviour will be
+removed.
+
+@item Friend Injection
+The @option{-ffriend-injection} option makes injected friends visible
+to regular name lookup, unlike standard C++.  This option is
+deprecated and will be removed.
+
 @item Implicit C language
 Old C system header files did not contain an @code{extern "C" @{@dots{}@}}
 scope to set the language.  On such systems, all header files are
Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi	(revision 257739)
+++ doc/invoke.texi	(working copy)
@@ -2451,8 +2451,7 @@  However, in ISO C++ a friend function th
 in an enclosing scope can only be found using argument dependent
 lookup.  GCC defaults to the standard behavior.
 
-This option is for compatibility, and may be removed in a future
-release of G++.
+This option is deprecated and will be removed.
 
 @item -fno-elide-constructors
 @opindex fno-elide-constructors
Index: testsuite/g++.old-deja/g++.jason/scoping15.C
===================================================================
--- testsuite/g++.old-deja/g++.jason/scoping15.C	(revision 257739)
+++ testsuite/g++.old-deja/g++.jason/scoping15.C	(working copy)
@@ -3,14 +3,17 @@ 
 // Bug: g++ ignores the :: qualification and dies trying to treat an integer
 // variable as a list of functions.
 
+class DComplex;
+double imag (const DComplex&);
+
 class DComplex {
 public:
-  friend  double   imag(const DComplex& a);
+  friend  double   imag(const DComplex& a); // Not injected, no warning
 };
 
 class FComplex {
 public:
-  friend  float    imag(const FComplex& a);
+  friend  float    imag(const FComplex& a); // { dg-warning "is visible"
 };
 
 void
@@ -19,3 +22,4 @@  scnrm2(FComplex cx[])
   int imag;
   ::imag( cx[0] );
 }
+// { dg-warning "ffriend-injection.* is deprecated" "" { target *-*-* } cc1plus: }
Index: testsuite/g++.old-deja/g++.mike/net43.C
===================================================================
--- testsuite/g++.old-deja/g++.mike/net43.C	(revision 257739)
+++ testsuite/g++.old-deja/g++.mike/net43.C	(working copy)
@@ -1,9 +1,9 @@ 
 // { dg-do assemble  }
-// { dg-options "-ffriend-injection" }
+// { dg-options "-ffriend-injection -Wno-deprecated" }
 
 class foo {
  public:
-   friend int operator ^(const foo&, const foo&);
+  friend int operator ^(const foo&, const foo&); // { dg-message "is visible" }
 };
 
 int main ()