[C++] PR 92804 [10 Regression] ICE trying to use concept as a nested-name-specifier

Message ID a8e4ade9-e5e6-5c82-8aae-96fb259788cc@oracle.com
State New
Headers show
Series
  • [C++] PR 92804 [10 Regression] ICE trying to use concept as a nested-name-specifier
Related show

Commit Message

Paolo Carlini Jan. 22, 2020, 3:22 p.m.
Hi,

in this simple issue we either wrongly talked about variable template-id 
in c++17 mode or ICEd in c++2a. I think we simply want to handle 
concept-ids first, both as represented in c++17 mode and as represented 
in c++2a mode. Tested x86_64-linux.

Thanks, Paolo.

///////////////////////
Fix "PR c++/92804 ICE trying to use concept as a nested-name-specifier"

A rather simple ICE where we failed to properly check for concept-ids
uses in nested-name-specifiers.

Tested x86_64-linux.

       /cp
       PR c++/92804
       * parser.c (cp_parser_nested_name_specifier_opt): Properly
       diagnose concept-ids.

       /testsuite
       PR c++/92804
       * g++.dg/concepts/pr92804.C: New.

Comments

Jason Merrill Jan. 22, 2020, 4:27 p.m. | #1
On 1/22/20 10:22 AM, Paolo Carlini wrote:
> Hi,

> 

> in this simple issue we either wrongly talked about variable template-id 

> in c++17 mode or ICEd in c++2a. I think we simply want to handle 

> concept-ids first, both as represented in c++17 mode and as represented 

> in c++2a mode. Tested x86_64-linux.


What happens if you try to use a function template/function concept name?

Jason
Paolo Carlini Jan. 22, 2020, 8:31 p.m. | #2
Hi,

On 22/01/20 17:27, Jason Merrill wrote:
> On 1/22/20 10:22 AM, Paolo Carlini wrote:

>> Hi,

>>

>> in this simple issue we either wrongly talked about variable 

>> template-id in c++17 mode or ICEd in c++2a. I think we simply want to 

>> handle concept-ids first, both as represented in c++17 mode and as 

>> represented in c++2a mode. Tested x86_64-linux.

> What happens if you try to use a function template/function concept name?


AFAICS no ICEs, no regressions but indeed we can do better, tell 
concepts from function templates. The below does that and passes testing 
but I'm not sure if the wording is optimal, whether we always want to 
talk about concept-id.

Thanks! Paolo.

///////////////////////
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index caafbefda8e..85a4ea5be82 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -6467,16 +6467,27 @@ cp_parser_nested_name_specifier_opt (cp_parser *parser,
 		      tree fns = get_fns (tid);
 		      if (OVL_SINGLE_P (fns))
 			tmpl = OVL_FIRST (fns);
-		      error_at (token->location, "function template-id %qD "
-				"in nested-name-specifier", tid);
+		      if (function_concept_p (fns))
+			error_at (token->location, "concept-id %qD "
+				  "in nested-name-specifier", tid);
+		      else
+			error_at (token->location, "function template-id "
+				  "%qD in nested-name-specifier", tid);
 		    }
 		  else
 		    {
-		      /* Variable template.  */
 		      tmpl = TREE_OPERAND (tid, 0);
-		      gcc_assert (variable_template_p (tmpl));
-		      error_at (token->location, "variable template-id %qD "
-				"in nested-name-specifier", tid);
+		      if (variable_concept_p (tmpl)
+			  || standard_concept_p (tmpl))
+			error_at (token->location, "concept-id %qD "
+				  "in nested-name-specifier", tid);
+		      else
+			{
+			  /* Variable template.  */
+			  gcc_assert (variable_template_p (tmpl));
+			  error_at (token->location, "variable template-id "
+				    "%qD in nested-name-specifier", tid);
+			}
 		    }
 		  if (tmpl)
 		    inform (DECL_SOURCE_LOCATION (tmpl),
diff --git a/gcc/testsuite/g++.dg/concepts/pr92804-1.C b/gcc/testsuite/g++.dg/concepts/pr92804-1.C
new file mode 100644
index 00000000000..cc21426bb9e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr92804-1.C
@@ -0,0 +1,19 @@
+// { dg-do compile { target c++17 } }
+// { dg-options "-fconcepts" }
+
+template<typename T>
+concept foo = true;  // { dg-message "declared here" }
+
+template<typename T>
+void bar(T t)
+{
+  if constexpr (foo<T>::value)  // { dg-error "17:concept-id .foo<T>. in nested-name-specifier" }
+  // { dg-error "expected|value" "" { target c++17 } .-1 }
+  {
+  }
+}
+
+int main()
+{
+  bar(1);
+}
diff --git a/gcc/testsuite/g++.dg/concepts/pr92804-2.C b/gcc/testsuite/g++.dg/concepts/pr92804-2.C
new file mode 100644
index 00000000000..98ec4ae598e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr92804-2.C
@@ -0,0 +1,19 @@
+// { dg-do compile { target c++17 } }
+// { dg-options "-w -fconcepts" }
+
+template<typename T>
+concept bool foo() { return true; };  // { dg-message "declared here" }
+
+template<typename T>
+void bar(T t)
+{
+  if constexpr (foo<T>::value)  // { dg-error "17:concept-id .foo<T>. in nested-name-specifier" }
+  // { dg-error "expected|value" "" { target *-*-* } .-1 }
+  {
+  }
+}
+
+int main()
+{
+  bar(1);
+}
Jason Merrill Jan. 22, 2020, 9:32 p.m. | #3
On 1/22/20 3:31 PM, Paolo Carlini wrote:
> Hi,

> 

> On 22/01/20 17:27, Jason Merrill wrote:

>> On 1/22/20 10:22 AM, Paolo Carlini wrote:

>>> Hi,

>>>

>>> in this simple issue we either wrongly talked about variable 

>>> template-id in c++17 mode or ICEd in c++2a. I think we simply want to 

>>> handle concept-ids first, both as represented in c++17 mode and as 

>>> represented in c++2a mode. Tested x86_64-linux.

>> What happens if you try to use a function template/function concept name?

> 

> AFAICS no ICEs, no regressions but indeed we can do better, tell 

> concepts from function templates. The below does that and passes testing 

> but I'm not sure if the wording is optimal, whether we always want to 

> talk about concept-id.


I think it's fine either way.

> +// { dg-do compile { target c++17 } }

> +// { dg-options "-w -fconcepts" }


Does it work to use -fconcepts-ts instead of -w -fconcepts?

Jason
Paolo Carlini Jan. 23, 2020, 9:31 a.m. | #4
Hi,

On 22/01/20 22:32, Jason Merrill wrote:
> On 1/22/20 3:31 PM, Paolo Carlini wrote:

>> Hi,

>>

>> On 22/01/20 17:27, Jason Merrill wrote:

>>> On 1/22/20 10:22 AM, Paolo Carlini wrote:

>>>> Hi,

>>>>

>>>> in this simple issue we either wrongly talked about variable 

>>>> template-id in c++17 mode or ICEd in c++2a. I think we simply want 

>>>> to handle concept-ids first, both as represented in c++17 mode and 

>>>> as represented in c++2a mode. Tested x86_64-linux.

>>> What happens if you try to use a function template/function concept 

>>> name?

>>

>> AFAICS no ICEs, no regressions but indeed we can do better, tell 

>> concepts from function templates. The below does that and passes 

>> testing but I'm not sure if the wording is optimal, whether we always 

>> want to talk about concept-id.

>

> I think it's fine either way.

>

>> +// { dg-do compile { target c++17 } }

>> +// { dg-options "-w -fconcepts" }

>

> Does it work to use -fconcepts-ts instead of -w -fconcepts?


Sure, it does. Attached what I tested.

Thanks, Paolo.

//////////////////
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index caafbefda8e..85a4ea5be82 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -6467,16 +6467,27 @@ cp_parser_nested_name_specifier_opt (cp_parser *parser,
 		      tree fns = get_fns (tid);
 		      if (OVL_SINGLE_P (fns))
 			tmpl = OVL_FIRST (fns);
-		      error_at (token->location, "function template-id %qD "
-				"in nested-name-specifier", tid);
+		      if (function_concept_p (fns))
+			error_at (token->location, "concept-id %qD "
+				  "in nested-name-specifier", tid);
+		      else
+			error_at (token->location, "function template-id "
+				  "%qD in nested-name-specifier", tid);
 		    }
 		  else
 		    {
-		      /* Variable template.  */
 		      tmpl = TREE_OPERAND (tid, 0);
-		      gcc_assert (variable_template_p (tmpl));
-		      error_at (token->location, "variable template-id %qD "
-				"in nested-name-specifier", tid);
+		      if (variable_concept_p (tmpl)
+			  || standard_concept_p (tmpl))
+			error_at (token->location, "concept-id %qD "
+				  "in nested-name-specifier", tid);
+		      else
+			{
+			  /* Variable template.  */
+			  gcc_assert (variable_template_p (tmpl));
+			  error_at (token->location, "variable template-id "
+				    "%qD in nested-name-specifier", tid);
+			}
 		    }
 		  if (tmpl)
 		    inform (DECL_SOURCE_LOCATION (tmpl),
diff --git a/gcc/testsuite/g++.dg/concepts/pr92804-1.C b/gcc/testsuite/g++.dg/concepts/pr92804-1.C
new file mode 100644
index 00000000000..cc21426bb9e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr92804-1.C
@@ -0,0 +1,19 @@
+// { dg-do compile { target c++17 } }
+// { dg-options "-fconcepts" }
+
+template<typename T>
+concept foo = true;  // { dg-message "declared here" }
+
+template<typename T>
+void bar(T t)
+{
+  if constexpr (foo<T>::value)  // { dg-error "17:concept-id .foo<T>. in nested-name-specifier" }
+  // { dg-error "expected|value" "" { target c++17 } .-1 }
+  {
+  }
+}
+
+int main()
+{
+  bar(1);
+}
diff --git a/gcc/testsuite/g++.dg/concepts/pr92804-2.C b/gcc/testsuite/g++.dg/concepts/pr92804-2.C
new file mode 100644
index 00000000000..32a15543310
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr92804-2.C
@@ -0,0 +1,19 @@
+// { dg-do compile { target c++17 } }
+// { dg-options "-fconcepts-ts" }
+
+template<typename T>
+concept bool foo() { return true; };  // { dg-message "declared here" }
+
+template<typename T>
+void bar(T t)
+{
+  if constexpr (foo<T>::value)  // { dg-error "17:concept-id .foo<T>. in nested-name-specifier" }
+  // { dg-error "expected|value" "" { target *-*-* } .-1 }
+  {
+  }
+}
+
+int main()
+{
+  bar(1);
+}
Jason Merrill Jan. 23, 2020, 6 p.m. | #5
On 1/23/20 4:31 AM, Paolo Carlini wrote:
> Hi,

> 

> On 22/01/20 22:32, Jason Merrill wrote:

>> On 1/22/20 3:31 PM, Paolo Carlini wrote:

>>> Hi,

>>>

>>> On 22/01/20 17:27, Jason Merrill wrote:

>>>> On 1/22/20 10:22 AM, Paolo Carlini wrote:

>>>>> Hi,

>>>>>

>>>>> in this simple issue we either wrongly talked about variable 

>>>>> template-id in c++17 mode or ICEd in c++2a. I think we simply want 

>>>>> to handle concept-ids first, both as represented in c++17 mode and 

>>>>> as represented in c++2a mode. Tested x86_64-linux.

>>>> What happens if you try to use a function template/function concept 

>>>> name?

>>>

>>> AFAICS no ICEs, no regressions but indeed we can do better, tell 

>>> concepts from function templates. The below does that and passes 

>>> testing but I'm not sure if the wording is optimal, whether we always 

>>> want to talk about concept-id.

>>

>> I think it's fine either way.

>>

>>> +// { dg-do compile { target c++17 } }

>>> +// { dg-options "-w -fconcepts" }

>>

>> Does it work to use -fconcepts-ts instead of -w -fconcepts?

> 

> Sure, it does. Attached what I tested.


OK.

Patch

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index caafbefda8e..fe490d4a69f 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -6472,11 +6472,18 @@  cp_parser_nested_name_specifier_opt (cp_parser *parser,
 		    }
 		  else
 		    {
-		      /* Variable template.  */
 		      tmpl = TREE_OPERAND (tid, 0);
-		      gcc_assert (variable_template_p (tmpl));
-		      error_at (token->location, "variable template-id %qD "
-				"in nested-name-specifier", tid);
+		      if (variable_concept_p (tmpl)
+			  || standard_concept_p (tmpl))
+			error_at (token->location, "concept-id %qD "
+				  "in nested-name-specifier", tid);
+		      else
+			{
+			  /* Variable template.  */
+			  gcc_assert (variable_template_p (tmpl));
+			  error_at (token->location, "variable template-id "
+				    "%qD in nested-name-specifier", tid);
+			}
 		    }
 		  if (tmpl)
 		    inform (DECL_SOURCE_LOCATION (tmpl),
diff --git a/gcc/testsuite/g++.dg/concepts/pr92804.C b/gcc/testsuite/g++.dg/concepts/pr92804.C
new file mode 100644
index 00000000000..cc21426bb9e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr92804.C
@@ -0,0 +1,19 @@ 
+// { dg-do compile { target c++17 } }
+// { dg-options "-fconcepts" }
+
+template<typename T>
+concept foo = true;  // { dg-message "declared here" }
+
+template<typename T>
+void bar(T t)
+{
+  if constexpr (foo<T>::value)  // { dg-error "17:concept-id .foo<T>. in nested-name-specifier" }
+  // { dg-error "expected|value" "" { target c++17 } .-1 }
+  {
+  }
+}
+
+int main()
+{
+  bar(1);
+}