Skip tests requiring "alignof (void)" when compiling using clang

Message ID 1593529380-8689-1-git-send-email-gbenson@redhat.com
State New
Headers show
Series
  • Skip tests requiring "alignof (void)" when compiling using clang
Related show

Commit Message

Shahab Vahedi via Gdb-patches June 30, 2020, 3:03 p.m.
Hi all,

Clang fails to compile the generated output of gdb.cp/align.exp with
the following error: invalid application of 'alignof' to an incomplete
type 'void'.  This patch adds preprocessor conditionals to the
generated output, to avoid the offending code, and causes the tests
that require it to be skipped.

Checked on Fedora 31 x86_64, GCC and clang.  Ok to commit?

Cheers,
Gary

--
gdb/testsuite/ChangeLog:

	* gdb.cp/align.exp: Skip tests requiring "alignof (void)"
	when compiling using clang.
---
 gdb/testsuite/ChangeLog        | 5 +++++
 gdb/testsuite/gdb.cp/align.exp | 9 +++++++--
 2 files changed, 12 insertions(+), 2 deletions(-)

-- 
1.8.3.1

Comments

Shahab Vahedi via Gdb-patches June 30, 2020, 5:10 p.m. | #1
Hi,

On 6/30/20 12:03 PM, Gary Benson via Gdb-patches wrote:
> Hi all,

> 

> Clang fails to compile the generated output of gdb.cp/align.exp with

> the following error: invalid application of 'alignof' to an incomplete

> type 'void'.  This patch adds preprocessor conditionals to the

> generated output, to avoid the offending code, and causes the tests

> that require it to be skipped.

> 

> Checked on Fedora 31 x86_64, GCC and clang.  Ok to commit?

> 

> Cheers,

> Gary

> 

> --

> gdb/testsuite/ChangeLog:

> 

> 	* gdb.cp/align.exp: Skip tests requiring "alignof (void)"

> 	when compiling using clang.

> ---

>   gdb/testsuite/ChangeLog        | 5 +++++

>   gdb/testsuite/gdb.cp/align.exp | 9 +++++++--

>   2 files changed, 12 insertions(+), 2 deletions(-)

> 

> diff --git a/gdb/testsuite/gdb.cp/align.exp b/gdb/testsuite/gdb.cp/align.exp

> index 0905a27..65ffb3b 100644

> --- a/gdb/testsuite/gdb.cp/align.exp

> +++ b/gdb/testsuite/gdb.cp/align.exp

> @@ -80,7 +80,9 @@ puts $outfile {

>   

>       unsigned a_int3 = alignof (int[3]);

>   

> +#if !defined(__clang__)

>       unsigned a_void = alignof (void);

> +#endif

>   

>       struct base { char c; };

>       struct derived : public virtual base { int i; };

> @@ -170,5 +172,8 @@ foreach type $typelist {

>   

>   set expected [get_integer_valueof a_int3 0]

>   gdb_test "print alignof(int\[3\])" " = $expected"

> -set expected [get_integer_valueof a_void 0]

> -gdb_test "print alignof(void)" " = $expected"

> +

> +if ![test_compiler_info clang*] {

> +    set expected [get_integer_valueof a_void 0]

> +    gdb_test "print alignof(void)" " = $expected"

> +}

> 


Instead of adding such pre-processing blocks, should we fix the code 
instead, to be more portable across compilers?
Pedro Alves July 2, 2020, 8:49 p.m. | #2
On 6/30/20 4:03 PM, Gary Benson via Gdb-patches wrote:
> Hi all,

> 

> Clang fails to compile the generated output of gdb.cp/align.exp with

> the following error: invalid application of 'alignof' to an incomplete

> type 'void'.  This patch adds preprocessor conditionals to the

> generated output, to avoid the offending code, and causes the tests

> that require it to be skipped.

> 

> Checked on Fedora 31 x86_64, GCC and clang.  Ok to commit?

> 

> Cheers,

> Gary

> 

> --

> gdb/testsuite/ChangeLog:

> 

> 	* gdb.cp/align.exp: Skip tests requiring "alignof (void)"

> 	when compiling using clang.

> ---

>  gdb/testsuite/ChangeLog        | 5 +++++

>  gdb/testsuite/gdb.cp/align.exp | 9 +++++++--

>  2 files changed, 12 insertions(+), 2 deletions(-)

> 

> diff --git a/gdb/testsuite/gdb.cp/align.exp b/gdb/testsuite/gdb.cp/align.exp

> index 0905a27..65ffb3b 100644

> --- a/gdb/testsuite/gdb.cp/align.exp

> +++ b/gdb/testsuite/gdb.cp/align.exp

> @@ -80,7 +80,9 @@ puts $outfile {

>  

>      unsigned a_int3 = alignof (int[3]);

>  

> +#if !defined(__clang__)

>      unsigned a_void = alignof (void);

> +#endif


Space before parens:

  #if !defined (__clang__)

Or:

  #ifndef __clang__

>  

>      struct base { char c; };

>      struct derived : public virtual base { int i; };

> @@ -170,5 +172,8 @@ foreach type $typelist {

>  

>  set expected [get_integer_valueof a_int3 0]

>  gdb_test "print alignof(int\[3\])" " = $expected"

> -set expected [get_integer_valueof a_void 0]

> -gdb_test "print alignof(void)" " = $expected"

> +

> +if ![test_compiler_info clang*] {

> +    set expected [get_integer_valueof a_void 0]

> +    gdb_test "print alignof(void)" " = $expected"

> +}


I think this should still test GDB's support.  And a
comment would be helpful.  Like:

# As an extension, GCC allows void pointer arithmetic, with
# sizeof(void) and alignof(void) both 1.  GDB supports GCC's
# extension.  Clang does not.
if ![test_compiler_info clang*] {
    set expected [get_integer_valueof a_void 0]
    gdb_test "print alignof(void)" " = $expected"
} else {
    gdb_test "print alignof(void)" " = 1"
}

OK with those changes, and the commit log adjusted accordingly.

Pedro Alves
Pedro Alves July 2, 2020, 8:50 p.m. | #3
On 6/30/20 6:10 PM, Luis Machado via Gdb-patches wrote:

> Instead of adding such pre-processing blocks, should we fix the code instead, to be more portable across compilers?


Not in this case -- we're testing that GDB supports the GCC extension too.
Simon Marchi July 2, 2020, 8:52 p.m. | #4
On 2020-07-02 4:49 p.m., Pedro Alves wrote:
> I think this should still test GDB's support.  And a

> comment would be helpful.  Like:

> 

> # As an extension, GCC allows void pointer arithmetic, with

> # sizeof(void) and alignof(void) both 1.  GDB supports GCC's

> # extension.  Clang does not.

> if ![test_compiler_info clang*] {

>     set expected [get_integer_valueof a_void 0]

>     gdb_test "print alignof(void)" " = $expected"

> } else {

>     gdb_test "print alignof(void)" " = 1"

> }


Indeed.  Otherwise, let's say that clang gain this feature, it would remain untested
(when testing with clang) and nobody would think of coming here to update the test
case.  Here, if clang gains the feature, then I suppose it would generate a FAIL,
which would prompt us to update the test case.

Simon
Pedro Alves July 2, 2020, 9:08 p.m. | #5
On 7/2/20 9:52 PM, Simon Marchi wrote:
> On 2020-07-02 4:49 p.m., Pedro Alves wrote:

>> I think this should still test GDB's support.  And a

>> comment would be helpful.  Like:

>>

>> # As an extension, GCC allows void pointer arithmetic, with

>> # sizeof(void) and alignof(void) both 1.  GDB supports GCC's

>> # extension.  Clang does not.

>> if ![test_compiler_info clang*] {

>>     set expected [get_integer_valueof a_void 0]

>>     gdb_test "print alignof(void)" " = $expected"

>> } else {

>>     gdb_test "print alignof(void)" " = 1"

>> }

> 

> Indeed.  Otherwise, let's say that clang gain this feature, it would remain untested

> (when testing with clang) and nobody would think of coming here to update the test

> case.  Here, if clang gains the feature, then I suppose it would generate a FAIL,

> which would prompt us to update the test case.


Actually, if Clang gains the feature, the test would still pass.
GDB's alignof support is builtin, does not depend on what the compiler
outputs.

Patch

diff --git a/gdb/testsuite/gdb.cp/align.exp b/gdb/testsuite/gdb.cp/align.exp
index 0905a27..65ffb3b 100644
--- a/gdb/testsuite/gdb.cp/align.exp
+++ b/gdb/testsuite/gdb.cp/align.exp
@@ -80,7 +80,9 @@  puts $outfile {
 
     unsigned a_int3 = alignof (int[3]);
 
+#if !defined(__clang__)
     unsigned a_void = alignof (void);
+#endif
 
     struct base { char c; };
     struct derived : public virtual base { int i; };
@@ -170,5 +172,8 @@  foreach type $typelist {
 
 set expected [get_integer_valueof a_int3 0]
 gdb_test "print alignof(int\[3\])" " = $expected"
-set expected [get_integer_valueof a_void 0]
-gdb_test "print alignof(void)" " = $expected"
+
+if ![test_compiler_info clang*] {
+    set expected [get_integer_valueof a_void 0]
+    gdb_test "print alignof(void)" " = $expected"
+}