[pushed] c++: parse trivial DMI immediately [PR96645]

Message ID 20220331014312.2080968-1-jason@redhat.com
State Committed
Commit 0ce8154f1c72e6d701bff969a007938e2f986369
Headers
Series [pushed] c++: parse trivial DMI immediately [PR96645] |

Commit Message

Jason Merrill March 31, 2022, 1:43 a.m. UTC
  The recent change to reject __is_constructible for nested classes with DMI
is breaking some code loudly that was previously only silently broken.
Let's allow simple cases by immediately parsing DMI that do no name lookup;
then being in complete class scope makes no difference.

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

	PR c++/96645

gcc/cp/ChangeLog:

	* parser.cc (cp_parser_early_parsing_nsdmi): New.
	(cp_parser_member_declaration): Call it.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp0x/nsdmi10.C: Now OK.
	* g++.dg/ext/is_constructible3.C: Likewise.
	* g++.dg/ext/is_constructible7.C: Likewise.
---
 gcc/cp/parser.cc                             | 28 +++++++++++++++++++-
 gcc/testsuite/g++.dg/cpp0x/nsdmi10.C         |  4 +--
 gcc/testsuite/g++.dg/ext/is_constructible3.C |  2 +-
 gcc/testsuite/g++.dg/ext/is_constructible7.C |  3 +--
 4 files changed, 31 insertions(+), 6 deletions(-)


base-commit: 150ab50f7449cf5b496bbe6e5c60cb1adb2e2d6c
  

Comments

Patrick Palka March 31, 2022, 12:50 p.m. UTC | #1
On Wed, 30 Mar 2022, Jason Merrill via Gcc-patches wrote:

> The recent change to reject __is_constructible for nested classes with DMI
> is breaking some code loudly that was previously only silently broken.
> Let's allow simple cases by immediately parsing DMI that do no name lookup;
> then being in complete class scope makes no difference.

Not sure if this is a problem in practice but it seems the initializer
processing step of cp_parser_late_parse_one_default_arg may involve name
lookup even if the parse itself didn't:

  struct A {
    struct B {
      const A &a = {};
    };
  };

We used to accept this (very contrived example), but now reject with

  error: invalid use of incomplete type ‘const struct A’

> 
> Tested x86_64-pc-linux-gnu, applying to trunk.
> 
> 	PR c++/96645
> 
> gcc/cp/ChangeLog:
> 
> 	* parser.cc (cp_parser_early_parsing_nsdmi): New.
> 	(cp_parser_member_declaration): Call it.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp0x/nsdmi10.C: Now OK.
> 	* g++.dg/ext/is_constructible3.C: Likewise.
> 	* g++.dg/ext/is_constructible7.C: Likewise.
> ---
>  gcc/cp/parser.cc                             | 28 +++++++++++++++++++-
>  gcc/testsuite/g++.dg/cpp0x/nsdmi10.C         |  4 +--
>  gcc/testsuite/g++.dg/ext/is_constructible3.C |  2 +-
>  gcc/testsuite/g++.dg/ext/is_constructible7.C |  3 +--
>  4 files changed, 31 insertions(+), 6 deletions(-)
> 
> diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> index 7e1c777364e..63c8af1c722 100644
> --- a/gcc/cp/parser.cc
> +++ b/gcc/cp/parser.cc
> @@ -2701,6 +2701,8 @@ static tree cp_parser_late_parse_one_default_arg
>    (cp_parser *, tree, tree, tree);
>  static void cp_parser_late_parsing_nsdmi
>    (cp_parser *, tree);
> +static bool cp_parser_early_parsing_nsdmi
> +  (cp_parser *, tree);
>  static void cp_parser_late_parsing_default_args
>    (cp_parser *, tree);
>  static tree cp_parser_sizeof_operand
> @@ -27478,7 +27480,8 @@ cp_parser_member_declaration (cp_parser* parser)
>  	      if (DECL_DECLARES_FUNCTION_P (decl))
>  		cp_parser_save_default_args (parser, STRIP_TEMPLATE (decl));
>  	      else if (TREE_CODE (decl) == FIELD_DECL
> -		       && DECL_INITIAL (decl))
> +		       && DECL_INITIAL (decl)
> +		       && !cp_parser_early_parsing_nsdmi (parser, decl))
>  		/* Add DECL to the queue of NSDMI to be parsed later.  */
>  		vec_safe_push (unparsed_nsdmis, decl);
>  	    }
> @@ -32292,6 +32295,29 @@ cp_parser_late_parsing_nsdmi (cp_parser *parser, tree field)
>    DECL_INITIAL (field) = def;
>  }
>  
> +/* If the DEFERRED_PARSE for FIELD is safe to parse immediately, do so.
> +   Returns true if deferred parsing is no longer needed.  */
> +
> +static bool
> +cp_parser_early_parsing_nsdmi (cp_parser *parser, tree field)
> +{
> +  tree init = DECL_INITIAL (field);
> +  if (TREE_CODE (init) != DEFERRED_PARSE)
> +    return true;
> +
> +  cp_token_cache *tokens = DEFPARSE_TOKENS (init);
> +  for (cp_token *p = tokens->first; p != tokens->last; ++p)
> +    if (p->type == CPP_NAME
> +	|| p->keyword == RID_THIS
> +	|| p->keyword == RID_OPERATOR)
> +      /* There's a name to look up or 'this', give up.  */
> +      return false;
> +
> +  /* It's trivial, parse now.  */
> +  cp_parser_late_parsing_nsdmi (parser, field);
> +  return true;
> +}
> +
>  /* FN is a FUNCTION_DECL which may contains a parameter with an
>     unparsed DEFERRED_PARSE.  Parse the default args now.  This function
>     assumes that the current scope is the scope in which the default
> diff --git a/gcc/testsuite/g++.dg/cpp0x/nsdmi10.C b/gcc/testsuite/g++.dg/cpp0x/nsdmi10.C
> index d8588b7f29e..a965f7bc333 100644
> --- a/gcc/testsuite/g++.dg/cpp0x/nsdmi10.C
> +++ b/gcc/testsuite/g++.dg/cpp0x/nsdmi10.C
> @@ -6,7 +6,7 @@ struct A1 {
>      int y1 = 1;
>    };
>  
> -  A1(const B1& opts = B1()) {}  // { dg-error "default member initializer" }
> +  A1(const B1& opts = B1()) {}
>  };
>  
>  struct A2 {
> @@ -14,5 +14,5 @@ struct A2 {
>      int x2, y2 = 1;
>    };
>  
> -  A2(const B2& opts = B2()) {}  // { dg-error "default member initializer" }
> +  A2(const B2& opts = B2()) {}
>  };
> diff --git a/gcc/testsuite/g++.dg/ext/is_constructible3.C b/gcc/testsuite/g++.dg/ext/is_constructible3.C
> index 305751d28e2..c7c58746cd0 100644
> --- a/gcc/testsuite/g++.dg/ext/is_constructible3.C
> +++ b/gcc/testsuite/g++.dg/ext/is_constructible3.C
> @@ -8,7 +8,7 @@ struct A {
>      B() = default;
>    };
>  
> -  static constexpr bool v = __is_constructible (B); // { dg-error "member initializer" }
> +  static constexpr bool v = __is_constructible (B);
>  
>  };
>  
> diff --git a/gcc/testsuite/g++.dg/ext/is_constructible7.C b/gcc/testsuite/g++.dg/ext/is_constructible7.C
> index 76a63bba5d0..013a1df03c6 100644
> --- a/gcc/testsuite/g++.dg/ext/is_constructible7.C
> +++ b/gcc/testsuite/g++.dg/ext/is_constructible7.C
> @@ -12,7 +12,7 @@ using true_type = bool_constant<true>;
>  
>  template<typename T>
>  struct is_default_constructible
> -  : bool_constant<__is_constructible(T)> // { dg-error "default member init" }
> +  : bool_constant<__is_constructible(T)>
>  { };
>  
>  void testVarStruct()
> @@ -22,7 +22,6 @@ void testVarStruct()
>        int number = 5; // compiles, if remove initialization
>      };
>  
> -    // { dg-prune-output "could not convert" }
>      is_default_constructible<A>::type t = true_type{};
>    };
>  }
> 
> base-commit: 150ab50f7449cf5b496bbe6e5c60cb1adb2e2d6c
> -- 
> 2.27.0
> 
>
  

Patch

diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 7e1c777364e..63c8af1c722 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -2701,6 +2701,8 @@  static tree cp_parser_late_parse_one_default_arg
   (cp_parser *, tree, tree, tree);
 static void cp_parser_late_parsing_nsdmi
   (cp_parser *, tree);
+static bool cp_parser_early_parsing_nsdmi
+  (cp_parser *, tree);
 static void cp_parser_late_parsing_default_args
   (cp_parser *, tree);
 static tree cp_parser_sizeof_operand
@@ -27478,7 +27480,8 @@  cp_parser_member_declaration (cp_parser* parser)
 	      if (DECL_DECLARES_FUNCTION_P (decl))
 		cp_parser_save_default_args (parser, STRIP_TEMPLATE (decl));
 	      else if (TREE_CODE (decl) == FIELD_DECL
-		       && DECL_INITIAL (decl))
+		       && DECL_INITIAL (decl)
+		       && !cp_parser_early_parsing_nsdmi (parser, decl))
 		/* Add DECL to the queue of NSDMI to be parsed later.  */
 		vec_safe_push (unparsed_nsdmis, decl);
 	    }
@@ -32292,6 +32295,29 @@  cp_parser_late_parsing_nsdmi (cp_parser *parser, tree field)
   DECL_INITIAL (field) = def;
 }
 
+/* If the DEFERRED_PARSE for FIELD is safe to parse immediately, do so.
+   Returns true if deferred parsing is no longer needed.  */
+
+static bool
+cp_parser_early_parsing_nsdmi (cp_parser *parser, tree field)
+{
+  tree init = DECL_INITIAL (field);
+  if (TREE_CODE (init) != DEFERRED_PARSE)
+    return true;
+
+  cp_token_cache *tokens = DEFPARSE_TOKENS (init);
+  for (cp_token *p = tokens->first; p != tokens->last; ++p)
+    if (p->type == CPP_NAME
+	|| p->keyword == RID_THIS
+	|| p->keyword == RID_OPERATOR)
+      /* There's a name to look up or 'this', give up.  */
+      return false;
+
+  /* It's trivial, parse now.  */
+  cp_parser_late_parsing_nsdmi (parser, field);
+  return true;
+}
+
 /* FN is a FUNCTION_DECL which may contains a parameter with an
    unparsed DEFERRED_PARSE.  Parse the default args now.  This function
    assumes that the current scope is the scope in which the default
diff --git a/gcc/testsuite/g++.dg/cpp0x/nsdmi10.C b/gcc/testsuite/g++.dg/cpp0x/nsdmi10.C
index d8588b7f29e..a965f7bc333 100644
--- a/gcc/testsuite/g++.dg/cpp0x/nsdmi10.C
+++ b/gcc/testsuite/g++.dg/cpp0x/nsdmi10.C
@@ -6,7 +6,7 @@  struct A1 {
     int y1 = 1;
   };
 
-  A1(const B1& opts = B1()) {}  // { dg-error "default member initializer" }
+  A1(const B1& opts = B1()) {}
 };
 
 struct A2 {
@@ -14,5 +14,5 @@  struct A2 {
     int x2, y2 = 1;
   };
 
-  A2(const B2& opts = B2()) {}  // { dg-error "default member initializer" }
+  A2(const B2& opts = B2()) {}
 };
diff --git a/gcc/testsuite/g++.dg/ext/is_constructible3.C b/gcc/testsuite/g++.dg/ext/is_constructible3.C
index 305751d28e2..c7c58746cd0 100644
--- a/gcc/testsuite/g++.dg/ext/is_constructible3.C
+++ b/gcc/testsuite/g++.dg/ext/is_constructible3.C
@@ -8,7 +8,7 @@  struct A {
     B() = default;
   };
 
-  static constexpr bool v = __is_constructible (B); // { dg-error "member initializer" }
+  static constexpr bool v = __is_constructible (B);
 
 };
 
diff --git a/gcc/testsuite/g++.dg/ext/is_constructible7.C b/gcc/testsuite/g++.dg/ext/is_constructible7.C
index 76a63bba5d0..013a1df03c6 100644
--- a/gcc/testsuite/g++.dg/ext/is_constructible7.C
+++ b/gcc/testsuite/g++.dg/ext/is_constructible7.C
@@ -12,7 +12,7 @@  using true_type = bool_constant<true>;
 
 template<typename T>
 struct is_default_constructible
-  : bool_constant<__is_constructible(T)> // { dg-error "default member init" }
+  : bool_constant<__is_constructible(T)>
 { };
 
 void testVarStruct()
@@ -22,7 +22,6 @@  void testVarStruct()
       int number = 5; // compiles, if remove initialization
     };
 
-    // { dg-prune-output "could not convert" }
     is_default_constructible<A>::type t = true_type{};
   };
 }