Re: [PATCH 01/11] gdb/testsuite: ignore Non-C-typedefs for gdb.cp/class2.exp

[Andrew Burgess <aburgess@redhat.com>]

Bruno Larsen <blarsen@redhat.com> writes:

> On 26/10/2022 14:02, Andrew Burgess wrote:
>> Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:
>>
>>> When attempting to test gdb.cp/class2.exp using clang, it fails to
>>> prepare with the following error:
>>>
>>> Executing on host: clang++    -fdiagnostics-color=never -Wno-unknown-warning-option  -c -g -o /home/blarsen/Documents/fsf_build/gdb/testsuite/outputs/gdb.cp/class2/class20.o /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc    (timeout = 300)
>>> builtin_spawn -ignore SIGHUP clang++ -fdiagnostics-color=never -Wno-unknown-warning-option -c -g -o /home/blarsen/Documents/fsf_build/gdb/testsuite/outputs/gdb.cp/class2/class20.o /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc
>>> /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc:53:14: warning: anonymous non-C-compatible type given name for linkage purposes by typedef declaration; add a tag name here [-Wnon-c-typedef-for-linkage]
>>> typedef class {
>>>               ^
>>>                Dbase
>>> /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc:54:2: note: type is not C-compatible due to this member declaration
>>>   public:
>>>   ^~~~~~~
>>> /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc:58:3: note: type is given name 'Dbase' for linkage purposes by this typedef declaration
>>> } Dbase;
>>>    ^
>>> 1 warning generated.
>>> gdb compile failed, /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc:53:14: warning: anonymous non-C-compatible type given name for linkage purposes by typedef declaration; add a tag name here [-Wnon-c-typedef-for-linkage]
>>> typedef class {
>>>               ^
>>>                Dbase
>>> /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc:54:2: note: type is not C-compatible due to this member declaration
>>>   public:
>>>   ^~~~~~~
>>> /home/blarsen/Documents/fsf_build/gdb/testsuite/../../../binutils-gdb/gdb/testsuite/gdb.cp/class2.cc:58:3: note: type is given name 'Dbase' for linkage purposes by this typedef declaration
>>> } Dbase;
>>>    ^
>>> 1 warning generated.
>>> UNTESTED: gdb.cp/class2.exp: failed to prepare
>>>
>>> This can be silenced by adding -Wno-non-c-typedef-for-linkage. The test
>>> shows no failures with this change.
>>> ---
>>>   gdb/testsuite/gdb.cp/class2.exp | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/gdb/testsuite/gdb.cp/class2.exp b/gdb/testsuite/gdb.cp/class2.exp
>>> index 32f9dfc18a5..e1e4ad0fae6 100644
>>> --- a/gdb/testsuite/gdb.cp/class2.exp
>>> +++ b/gdb/testsuite/gdb.cp/class2.exp
>>> @@ -18,7 +18,7 @@ if { [skip_cplus_tests] } { return }
>>>   
>>>   standard_testfile .cc
>>>   
>>> -if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} {
>>> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++ additional_flags=-Wno-non-c-typedef-for-linkage}]} {
>>>       return -1
>> I worry about just adding an extra flag like this.  GCC is happy enough
>> as it ignores unknown arguments, but some other compiler might be
>> unhappy.
>>
>> Additionally, with earlier versions of clang++ (I tested 9.0.1) the new
>> flag isn't recognised, and actually causes the build to fail.
> Good call. This flag seems to have been added by clang-11, so I'll check 
> for that version.
>>
>> I wonder if we should look to something like gdb.cp/pr9167.exp for how
>> to solve this?  In that script we do this:
>>
>>    set flags [list debug c++]
>>    if { [test_compiler_info gcc-*] && [gcc_major_version] >= 10 } {
>>        # Work around PR gcc/101452.
>>        lappend flags additional_flags=-fno-eliminate-unused-debug-types
>>    }
>>
>> this might be a better model for how to add the flag.
>
> Looking at gcc_major_version, this looks too verbose. gcc_major_version 
> already checks for the given compiler, returning -1 if the compiler is 
> not the expected one, so that code from gdb.cp/pr9167.exp could be 
> shortened to:
>
> if  { [ gcc_major_version ] >= 10 } {
>      ....
> }

I guess you actually need:

 [ gcc_major_version "gcc-*" c++ ]

in order to check the C++ compiler.

Thanks,
Andrew

>
> What do you think?
>
> (I also think the function should be renamed, but that is a patch for 
> another day).
>
> Cheers,
> Bruno
>
>>
>> Thanks,
>> Andrew
>>