[v2] C++: add type checking for static local vector variable in template

Message ID 20210917075859.4147-1-pc.wang@linux.alibaba.com
State Superseded
Headers
Series [v2] C++: add type checking for static local vector variable in template |

Commit Message

pc.wang Sept. 17, 2021, 7:58 a.m. UTC
  This patch moves verify_type_context from start_decl_1 to cp_finish_decl
and adds type checking for static local vector variable in C++ template.

2021-08-06  wangpc  <pc.wang@linux.alibaba.com>

gcc/cp/ChangeLog

        * decl.c (start_decl_1): Remove verify_type_context.
	        (cp_finish_decl): Add more type checking.

gcc/testsuite/ChangeLog

        * g++.target/aarch64/sve/static-var-in-template.C: New test.
  

Comments

Jason Merrill Sept. 17, 2021, 1:34 p.m. UTC | #1
On 9/17/21 03:58, wangpc wrote:
> This patch moves verify_type_context from start_decl_1 to cp_finish_decl
> and adds type checking for static local vector variable in C++ template.

How have you tested this patch?
https://gcc.gnu.org/contribute.html#testing

> 2021-08-06  wangpc  <pc.wang@linux.alibaba.com>
> 
> gcc/cp/ChangeLog
> 
>          * decl.c (start_decl_1): Remove verify_type_context.
> 	        (cp_finish_decl): Add more type checking.
> 
> gcc/testsuite/ChangeLog
> 
>          * g++.target/aarch64/sve/static-var-in-template.C: New test.
> 
> diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
> index 90111e4c786..d411963896a 100644
> --- a/gcc/cp/decl.c
> +++ b/gcc/cp/decl.c
> @@ -5491,13 +5491,6 @@ start_decl_1 (tree decl, bool initialized)
>         cp_apply_type_quals_to_decl (cp_type_quals (type), decl);
>       }
>   
> -  if (is_global_var (decl))
> -    {
> -      type_context_kind context = (DECL_THREAD_LOCAL_P (decl)
> -				   ? TCTX_THREAD_STORAGE
> -				   : TCTX_STATIC_STORAGE);
> -      verify_type_context (input_location, context, TREE_TYPE (decl));
> -    }
>     if (initialized)
>       /* Is it valid for this decl to have an initializer at all?  */
>       {
> @@ -7520,6 +7513,22 @@ cp_finish_decl (tree decl, tree init, bool init_const_expr_p,
>         && DECL_INITIALIZED_IN_CLASS_P (decl))
>       check_static_variable_definition (decl, type);
>   
> +  if (!processing_template_decl && VAR_P (decl))
> +    {
> +      if (is_global_var (decl))
> +	{
> +	  type_context_kind context = (DECL_THREAD_LOCAL_P (decl)
> +				      ? TCTX_THREAD_STORAGE
> +				      : TCTX_STATIC_STORAGE);
> +	  verify_type_context (input_location, context, TREE_TYPE (decl));
> +	}
> +
> +      if (DECL_FUNCTION_SCOPE_P (decl)
> +	  && TREE_STATIC (decl))
> +	verify_type_context (DECL_SOURCE_LOCATION (decl),
> +			     TCTX_STATIC_STORAGE, type);

This is redundant; is_global_var is true for a local static.  Which 
makes the name confusing, but that's the intended behavior.

> +    }
> +
>     if (init && TREE_CODE (decl) == FUNCTION_DECL)
>       {
>         tree clone;
> diff --git a/gcc/testsuite/g++.target/aarch64/sve/static-var-in-template.C b/gcc/testsuite/g++.target/aarch64/sve/static-var-in-template.C
> new file mode 100644
> index 00000000000..c2395d18d50
> --- /dev/null
> +++ b/gcc/testsuite/g++.target/aarch64/sve/static-var-in-template.C
> @@ -0,0 +1,17 @@
> +/* { dg-do compile } */
> +
> +#include <arm_sve.h>
> +
> +template <int N>
> +void f()
> +{
> +    static svbool_t pg = svwhilelt_b64(0, N);
> +}
> +
> +int main(int argc, char **argv)
> +{
> +    f<2>();
> +    return 0;
> +}
> +
> +/* { dg-error {SVE type 'svbool_t' does not have a fixed size} } */
>
  
pc.wang Sept. 17, 2021, 2:30 p.m. UTC | #2
I have tested this patch on AArch64 and RISCV by running testsuites, the 
diagnostic message seems to be right.

While one thing that should be noted is that error message will be 
reported twice as below:

static-template.cpp: In instantiation of 'void f1() [with int a = 2]':
static-template.cpp:29:11:   required from here
static-template.cpp:11:24: error: RVV type 'vuint16m1_t' does not have a fixed size
    11 |     static vuint16m1_t v = vmv_v_x_u16m1(a, gvl);
       |                        ^
static-template.cpp:11:24: error: RVV type 'vuint16m1_t' does not have a fixed size

I haven't figured it out, or is it a normal behavior?

On 2021/9/17 21:34, Jason Merrill wrote:
> On 9/17/21 03:58, wangpc wrote:
>> This patch moves verify_type_context from start_decl_1 to cp_finish_decl
>> and adds type checking for static local vector variable in C++ template.
>
> How have you tested this patch?
> https://gcc.gnu.org/contribute.html#testing
>
>> 2021-08-06  wangpc <pc.wang@linux.alibaba.com>
>>
>> gcc/cp/ChangeLog
>>
>>          * decl.c (start_decl_1): Remove verify_type_context.
>>             (cp_finish_decl): Add more type checking.
>>
>> gcc/testsuite/ChangeLog
>>
>>          * g++.target/aarch64/sve/static-var-in-template.C: New test.
>>
>> diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
>> index 90111e4c786..d411963896a 100644
>> --- a/gcc/cp/decl.c
>> +++ b/gcc/cp/decl.c
>> @@ -5491,13 +5491,6 @@ start_decl_1 (tree decl, bool initialized)
>>         cp_apply_type_quals_to_decl (cp_type_quals (type), decl);
>>       }
>>   -  if (is_global_var (decl))
>> -    {
>> -      type_context_kind context = (DECL_THREAD_LOCAL_P (decl)
>> -                   ? TCTX_THREAD_STORAGE
>> -                   : TCTX_STATIC_STORAGE);
>> -      verify_type_context (input_location, context, TREE_TYPE (decl));
>> -    }
>>     if (initialized)
>>       /* Is it valid for this decl to have an initializer at all?  */
>>       {
>> @@ -7520,6 +7513,22 @@ cp_finish_decl (tree decl, tree init, bool 
>> init_const_expr_p,
>>         && DECL_INITIALIZED_IN_CLASS_P (decl))
>>       check_static_variable_definition (decl, type);
>>   +  if (!processing_template_decl && VAR_P (decl))
>> +    {
>> +      if (is_global_var (decl))
>> +    {
>> +      type_context_kind context = (DECL_THREAD_LOCAL_P (decl)
>> +                      ? TCTX_THREAD_STORAGE
>> +                      : TCTX_STATIC_STORAGE);
>> +      verify_type_context (input_location, context, TREE_TYPE (decl));
>> +    }
>> +
>> +      if (DECL_FUNCTION_SCOPE_P (decl)
>> +      && TREE_STATIC (decl))
>> +    verify_type_context (DECL_SOURCE_LOCATION (decl),
>> +                 TCTX_STATIC_STORAGE, type);
>
> This is redundant; is_global_var is true for a local static. Which 
> makes the name confusing, but that's the intended behavior.
>
>> +    }
>> +
>>     if (init && TREE_CODE (decl) == FUNCTION_DECL)
>>       {
>>         tree clone;
>> diff --git 
>> a/gcc/testsuite/g++.target/aarch64/sve/static-var-in-template.C 
>> b/gcc/testsuite/g++.target/aarch64/sve/static-var-in-template.C
>> new file mode 100644
>> index 00000000000..c2395d18d50
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.target/aarch64/sve/static-var-in-template.C
>> @@ -0,0 +1,17 @@
>> +/* { dg-do compile } */
>> +
>> +#include <arm_sve.h>
>> +
>> +template <int N>
>> +void f()
>> +{
>> +    static svbool_t pg = svwhilelt_b64(0, N);
>> +}
>> +
>> +int main(int argc, char **argv)
>> +{
>> +    f<2>();
>> +    return 0;
>> +}
>> +
>> +/* { dg-error {SVE type 'svbool_t' does not have a fixed size} } */
>>
  
pc.wang Sept. 17, 2021, 4:20 p.m. UTC | #3
OK I know, it's because the redundant code will check a declaration twice.

On 2021/9/17 22:30, wangpc wrote:
>
> I have tested this patch on AArch64 and RISCV by running testsuites, 
> the diagnostic message seems to be right.
>
> While one thing that should be noted is that error message will be 
> reported twice as below:
>
> static-template.cpp: In instantiation of 'void f1() [with int a = 2]':
> static-template.cpp:29:11:   required from here
> static-template.cpp:11:24: error: RVV type 'vuint16m1_t' does not have a fixed size
>     11 |     static vuint16m1_t v = vmv_v_x_u16m1(a, gvl);
>        |                        ^
> static-template.cpp:11:24: error: RVV type 'vuint16m1_t' does not have a fixed size
>
> I haven't figured it out, or is it a normal behavior?
>
> On 2021/9/17 21:34, Jason Merrill wrote:
>> On 9/17/21 03:58, wangpc wrote:
>>> This patch moves verify_type_context from start_decl_1 to 
>>> cp_finish_decl
>>> and adds type checking for static local vector variable in C++ 
>>> template.
>>
>> How have you tested this patch?
>> https://gcc.gnu.org/contribute.html#testing
>>
>>> 2021-08-06  wangpc <pc.wang@linux.alibaba.com>
>>>
>>> gcc/cp/ChangeLog
>>>
>>>          * decl.c (start_decl_1): Remove verify_type_context.
>>>             (cp_finish_decl): Add more type checking.
>>>
>>> gcc/testsuite/ChangeLog
>>>
>>>          * g++.target/aarch64/sve/static-var-in-template.C: New test.
>>>
>>> diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
>>> index 90111e4c786..d411963896a 100644
>>> --- a/gcc/cp/decl.c
>>> +++ b/gcc/cp/decl.c
>>> @@ -5491,13 +5491,6 @@ start_decl_1 (tree decl, bool initialized)
>>>         cp_apply_type_quals_to_decl (cp_type_quals (type), decl);
>>>       }
>>>   -  if (is_global_var (decl))
>>> -    {
>>> -      type_context_kind context = (DECL_THREAD_LOCAL_P (decl)
>>> -                   ? TCTX_THREAD_STORAGE
>>> -                   : TCTX_STATIC_STORAGE);
>>> -      verify_type_context (input_location, context, TREE_TYPE (decl));
>>> -    }
>>>     if (initialized)
>>>       /* Is it valid for this decl to have an initializer at all?  */
>>>       {
>>> @@ -7520,6 +7513,22 @@ cp_finish_decl (tree decl, tree init, bool 
>>> init_const_expr_p,
>>>         && DECL_INITIALIZED_IN_CLASS_P (decl))
>>>       check_static_variable_definition (decl, type);
>>>   +  if (!processing_template_decl && VAR_P (decl))
>>> +    {
>>> +      if (is_global_var (decl))
>>> +    {
>>> +      type_context_kind context = (DECL_THREAD_LOCAL_P (decl)
>>> +                      ? TCTX_THREAD_STORAGE
>>> +                      : TCTX_STATIC_STORAGE);
>>> +      verify_type_context (input_location, context, TREE_TYPE (decl));
>>> +    }
>>> +
>>> +      if (DECL_FUNCTION_SCOPE_P (decl)
>>> +      && TREE_STATIC (decl))
>>> +    verify_type_context (DECL_SOURCE_LOCATION (decl),
>>> +                 TCTX_STATIC_STORAGE, type);
>>
>> This is redundant; is_global_var is true for a local static. Which 
>> makes the name confusing, but that's the intended behavior.
>>
>>> +    }
>>> +
>>>     if (init && TREE_CODE (decl) == FUNCTION_DECL)
>>>       {
>>>         tree clone;
>>> diff --git 
>>> a/gcc/testsuite/g++.target/aarch64/sve/static-var-in-template.C 
>>> b/gcc/testsuite/g++.target/aarch64/sve/static-var-in-template.C
>>> new file mode 100644
>>> index 00000000000..c2395d18d50
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.target/aarch64/sve/static-var-in-template.C
>>> @@ -0,0 +1,17 @@
>>> +/* { dg-do compile } */
>>> +
>>> +#include <arm_sve.h>
>>> +
>>> +template <int N>
>>> +void f()
>>> +{
>>> +    static svbool_t pg = svwhilelt_b64(0, N);
>>> +}
>>> +
>>> +int main(int argc, char **argv)
>>> +{
>>> +    f<2>();
>>> +    return 0;
>>> +}
>>> +
>>> +/* { dg-error {SVE type 'svbool_t' does not have a fixed size} } */
>>>
  

Patch

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 90111e4c786..d411963896a 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -5491,13 +5491,6 @@  start_decl_1 (tree decl, bool initialized)
       cp_apply_type_quals_to_decl (cp_type_quals (type), decl);
     }
 
-  if (is_global_var (decl))
-    {
-      type_context_kind context = (DECL_THREAD_LOCAL_P (decl)
-				   ? TCTX_THREAD_STORAGE
-				   : TCTX_STATIC_STORAGE);
-      verify_type_context (input_location, context, TREE_TYPE (decl));
-    }
   if (initialized)
     /* Is it valid for this decl to have an initializer at all?  */
     {
@@ -7520,6 +7513,22 @@  cp_finish_decl (tree decl, tree init, bool init_const_expr_p,
       && DECL_INITIALIZED_IN_CLASS_P (decl))
     check_static_variable_definition (decl, type);
 
+  if (!processing_template_decl && VAR_P (decl))
+    {
+      if (is_global_var (decl))
+	{
+	  type_context_kind context = (DECL_THREAD_LOCAL_P (decl)
+				      ? TCTX_THREAD_STORAGE
+				      : TCTX_STATIC_STORAGE);
+	  verify_type_context (input_location, context, TREE_TYPE (decl));
+	}
+
+      if (DECL_FUNCTION_SCOPE_P (decl)
+	  && TREE_STATIC (decl))
+	verify_type_context (DECL_SOURCE_LOCATION (decl),
+			     TCTX_STATIC_STORAGE, type);
+    }
+
   if (init && TREE_CODE (decl) == FUNCTION_DECL)
     {
       tree clone;
diff --git a/gcc/testsuite/g++.target/aarch64/sve/static-var-in-template.C b/gcc/testsuite/g++.target/aarch64/sve/static-var-in-template.C
new file mode 100644
index 00000000000..c2395d18d50
--- /dev/null
+++ b/gcc/testsuite/g++.target/aarch64/sve/static-var-in-template.C
@@ -0,0 +1,17 @@ 
+/* { dg-do compile } */
+
+#include <arm_sve.h>
+
+template <int N>
+void f()
+{
+    static svbool_t pg = svwhilelt_b64(0, N);
+}
+
+int main(int argc, char **argv)
+{
+    f<2>();
+    return 0;
+}
+
+/* { dg-error {SVE type 'svbool_t' does not have a fixed size} } */