Re: [PATCH v2 04/16] gdb/testsuite: move getting_compiler_info to front of gdb_compile

[Andrew Burgess <aburgess@redhat.com>]

Nils-Christian Kempke via Gdb-patches <gdb-patches@sourceware.org>
writes:

> The procedure gdb_compile queries its options via
>
>    [lsearch -exact $options getting_compiler_info]
>
> to check whether or not it was called in with the option
> getting_compiler_info.  If it was called with this option it would
> preprocess some test input to try and figure out the actual compiler
> version of the compiler used.  While doing this we cannot again try to
> figure out the current compiler version via the 'getting_compiler_info'
> option as this would cause infinite recursion.  As some parts of the
> procedure do recursively test for the compiler version to e.g. set
> certain flags, at several places gdb_compile there are checks for the
> getting_compiler_info option needed.
>
> In the procedure, there was already a variable 'getting_compiler_info'
> which was set to the result of the 'lsearch' query and used instead of
> again and again looking for getting_compiler_info in the procedure
> options.  But, this variable was actually set too late within the code.
> This lead to a mixture of querying 'getting_compiler_info' or
> doing an lserach on the options passed to the procedure.
>
> I found this inconsistent and instead moved the variable
> getting_compiler_info to the front of the procedure.  It is set to true
> or false depending on whether or not the argument is found in the
> procedure's options (just as before) and queried instead of doing an
> lsearch on the procedure options in the rest of the procedure.

LGTM.

Thanks,
Andrew

> ---
>  gdb/testsuite/lib/gdb.exp | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 6fbf74442c..381367ee6c 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -4412,6 +4412,13 @@ proc gdb_compile {source dest type options} {
>  
>      set outdir [file dirname $dest]
>  
> +    # If this is set, calling test_compiler_info will cause recursion.
> +    if { [lsearch -exact $options getting_compiler_info] == -1 } {
> +	set getting_compiler_info false
> +    } else {
> +	set getting_compiler_info true
> +    }
> +
>      # Add platform-specific options if a shared library was specified using
>      # "shlib=librarypath" in OPTIONS.
>      set new_options {}
> @@ -4428,7 +4435,7 @@ proc gdb_compile {source dest type options} {
>      # default, unless you pass -Wno-unknown-warning-option as well.
>      # We do that here, so that individual testcases don't have to
>      # worry about it.
> -    if {[lsearch -exact $options getting_compiler_info] == -1
> +    if {!$getting_compiler_info
>  	&& [lsearch -exact $options rust] == -1
>  	&& [lsearch -exact $options ada] == -1
>  	&& [lsearch -exact $options f90] == -1
> @@ -4439,7 +4446,7 @@ proc gdb_compile {source dest type options} {
>  
>      # Treating .c input files as C++ is deprecated in Clang, so
>      # explicitly force C++ language.
> -    if { [lsearch -exact $options getting_compiler_info] == -1
> +    if { !$getting_compiler_info
>  	 && [lsearch -exact $options c++] != -1
>  	 && [string match *.c $source] != 0 } {
>  
> @@ -4460,7 +4467,7 @@ proc gdb_compile {source dest type options} {
>      # Place (and look for) Fortran `.mod` files in the output
>      # directory for this specific test.  For Intel compilers the -J
>      # option is not supported so instead use the -module flag.
> -    if { [lsearch -exact $options f90] != -1 } {
> +    if { !$getting_compiler_info && [lsearch -exact $options f90] != -1 } {
>  	# Fortran compile.
>  	set mod_path [standard_output_file ""]
>  	if [test_compiler_info "gcc-*"] {
> @@ -4473,7 +4480,6 @@ proc gdb_compile {source dest type options} {
>  
>      set shlib_found 0
>      set shlib_load 0
> -    set getting_compiler_info 0
>      foreach opt $options {
>          if {[regexp {^shlib=(.*)} $opt dummy_var shlib_name]
>  	    && $type == "executable"} {
> @@ -4505,8 +4511,9 @@ proc gdb_compile {source dest type options} {
>  	} elseif { $opt == "shlib_load" && $type == "executable" } {
>  	    set shlib_load 1
>  	} elseif { $opt == "getting_compiler_info" } {
> -	    # If this is set, calling test_compiler_info will cause recursion.
> -	    set getting_compiler_info 1
> +	    # Ignore this setting here as it has been handled earlier in this
> +	    # procedure.  Do not append it to new_options as this will cause
> +	    # recursion.
>          } elseif {[regexp "^text_segment=(.*)" $opt dummy_var addr]} {
>              if { [linker_supports_Ttext_segment_flag] } {
>                  # For GNU ld.
> @@ -4529,7 +4536,7 @@ proc gdb_compile {source dest type options} {
>      # DWARF line numbering.
>      # See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88432
>      # This option defaults to on for Debian/Ubuntu.
> -    if { $getting_compiler_info == 0
> +    if { !$getting_compiler_info
>  	 && [test_compiler_info {gcc-*-*}]
>  	 && !([test_compiler_info {gcc-[0-3]-*}]
>  	      || [test_compiler_info {gcc-4-0-*}])
> -- 
> 2.25.1
>
> Intel Deutschland GmbH
> Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
> Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
> Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
> Chairperson of the Supervisory Board: Nicole Lau
> Registered Office: Munich
> Commercial Register: Amtsgericht Muenchen HRB 186928