Re: [PATCH] Make language setting tests more robust

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2017-02-01 10:17, Luis Machado wrote:
> This patch robustifies the setting of the language in gdb.  Right now 
> most of
> the tests assume "set language" is a silent command and use 
> gdb_test_no_output
> for testing, but it may output a warning stating the language the user 
> wants
> to select does not match the frame's language.  When this happens we 
> have a
> failure. This is also the case with "show language", where it outputs 
> the
> language currently-selected but may also output said warning.
> 
> One case of the warning being displayed happens when one has debug 
> information
> for glibc, which may cause GDB to identify the frame as having an "asm"
> language. Therefore setting it to something else will get GDB's 
> attention.
> 
> This patch addresses the problem by creating a function in lib/gdb.exp 
> to
> set the language. That function will also handle potential warnings and 
> check
> to make sure the language was properly selected.
> 
> Also, i noticed most of the languages have their own 
> set_lang_<language> proc,
> and they are all the same.  Therefore i've removed those and switched 
> to using
> only the new set_language proc.
> 
> I tried to confirm why set_lang_<language> was replicated, but my 
> conclusion
> was that it was just the way the code worked and people just 
> copy/pasted from
> an existing language .exp file.
> 
> Overall i see no regressions with Ubuntu 16.04 x86-64, though the 
> number of
> tests changed slightly due to the way set_language works.  No 
> additional
> failures nonetheless.

I have just looked at the stuff in lib/, it looks good to me, that's a 
nice cleanup.  A few comments below.

> diff --git a/gdb/testsuite/lib/d-support.exp 
> b/gdb/testsuite/lib/d-support.exp
> index ed568d3..100fb63 100644
> --- a/gdb/testsuite/lib/d-support.exp
> +++ b/gdb/testsuite/lib/d-support.exp
> @@ -16,17 +16,6 @@
>  # Auxiliary function to set the language to D.
>  # The result is 1 (true) for success, 0 (false) for failure.

Remove this comment.

> -proc set_lang_d {} {
> -    if [gdb_test_no_output "set language d"] {
> -	return 0
> -    }
> -    if [gdb_test "show language" ".* source language is \"d\"." \
> -	   "set language to \"d\""] {
> -	return 0
> -    }
> -    return 1
> -}
> -
>  # D version of runto_main.
> 
>  proc d_runto_main { } {
> diff --git a/gdb/testsuite/lib/fortran.exp 
> b/gdb/testsuite/lib/fortran.exp
> index 867e4fc..2f4820b 100644
> --- a/gdb/testsuite/lib/fortran.exp
> +++ b/gdb/testsuite/lib/fortran.exp
> @@ -18,17 +18,6 @@
>  # Auxiliary function to set the language to fortran.
>  # The result is 1 (true) for success, 0 (false) for failure.

Comment.

> -proc set_lang_fortran {} {
> -    if [gdb_test_no_output "set language fortran"] {
> -	return 0
> -    }
> -    if [gdb_test "show language" ".* source language is \"fortran\"." 
> \
> -	   "set language to \"fortran\""] {
> -	return 0
> -    }
> -    return 1
> -}
> -
>  proc fortran_int4 {} {
>      if {[test_compiler_info {gcc-4-[012]-*}]} {
>  	return "int4"
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 48bd7ee..327bce8 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -6018,6 +6018,47 @@ proc multi_line_input { args } {
>      return [join $args "\n"]
>  }
> 
> +# Set the language and handle possible warnings output by GDB if we 
> select a
> +# language that differs from the current frame's language.
> +#
> +# The first argument is the language to set.
> +# The second argument is an optional message to be output by the test.
> +
> +proc set_language { args } {

Instead of "args", it might be clearer to use a default argument for the 
second arg:

   proc set_language { language { message "" } } {

And lower:

   if { $message == "" } {
     set message $command
   }

Not sure about the exact TCL syntax, but I think you get the point :).

> +    global gdb_prompt
> +
> +    set language [lindex $args 0]
> +    set command "set language $language"
> +    set lang_pattern [string_to_regexp $language]
> +
> +    # Do we have an optional user-provided test message?
> +    if {[llength $args] > 1} {
> +	set message [lindex $args 1]
> +    } else {
> +	set message $command
> +    }
> +
> +    # Switch to the user-selected language.
> +    gdb_test_multiple $command $message {
> +	-re "Undefined item: \"$lang_pattern\"\.\[\r\n\]+$gdb_prompt" {
> +	  fail $message
> +	  return 0
> +	}
> +	-re "Warning: the current language does not match this
> frame.\[\r\n\]+$gdb_prompt $" {
> +	}
> +	-re "$gdb_prompt $" {}
> +    }

Do you want to add a "pass" in there somewhere, or we keep it silent if 
it succeeds?

> +
> +    # Verify the language has been set correctly.  GDB may output a 
> warning
> +    # stating the user-provided language doesn't match the frame's 
> language.
> +    # We ignore that warning for testing purposes.
> +    if {$language != "auto"} {
> +	if [gdb_test "show language" ".* source language is
> \"$lang_pattern\".*" $message] {
> +	    return 0
> +	}
> +    }

In the else, you could verify that it matches 'The current source 
language is "auto; currently .*"'.

> +    return 1
> +}
> 
>  # Always load compatibility stuff.
>  load_lib future.exp
> diff --git a/gdb/testsuite/lib/go.exp b/gdb/testsuite/lib/go.exp
> index c29b7c1..cb98bdc 100644
> --- a/gdb/testsuite/lib/go.exp
> +++ b/gdb/testsuite/lib/go.exp
> @@ -19,17 +19,6 @@
>  # Auxiliary function to set the language to Go.
>  # The result is 1 (true) for success, 0 (false) for failure.

Comment.

> -proc set_lang_go {} {
> -    if [gdb_test_no_output "set language go"] {
> -	return 0
> -    }
> -    if [gdb_test "show language" ".* source language is \"go\"." \
> -	   "set language to \"go\""] {
> -	return 0
> -    }
> -    return 1
> -}
> -
>  # Go version of runto_main.
> 
>  proc go_runto_main { } {
> diff --git a/gdb/testsuite/lib/objc.exp b/gdb/testsuite/lib/objc.exp
> index 7189f03..9891758 100644
> --- a/gdb/testsuite/lib/objc.exp
> +++ b/gdb/testsuite/lib/objc.exp
> @@ -17,14 +17,3 @@
> 
>  # Auxiliary function to set the language to fortran.
>  # The result is 1 (true) for success, 0 (false) for failure.

Comment.  Also, note that it mentions fortran :).  Actually, you could 
probably just delete that file.

> -
> -proc set_lang_objc {} {
> -    if [gdb_test_no_output "set language objective-c"] {
> -	return 0
> -    }
> -    if [gdb_test "show language" ".* source language is 
> \"objective-c\"." \
> -	   "set language to \"objective-c\""] {
> -	return 0
> -    }
> -    return 1
> -}
> diff --git a/gdb/testsuite/lib/pascal.exp 
> b/gdb/testsuite/lib/pascal.exp
> index a0562c3..f0eb36b 100644
> --- a/gdb/testsuite/lib/pascal.exp
> +++ b/gdb/testsuite/lib/pascal.exp
> @@ -167,17 +167,3 @@ proc gdb_compile_pascal {source destfile type 
> options} {
>          return "Pascal compilation failed."
>      }
>  }
> -
> -# Auxiliary function to set the language to pascal.
> -# The result is 1 (true) for success, 0 (false) for failure.
> -
> -proc set_lang_pascal {} {
> -    if [gdb_test_no_output "set language pascal"] {
> -	return 0
> -    }
> -    if [gdb_test "show language" ".* source language is \"pascal\"." \
> -	   "set language to \"pascal\""] {
> -	return 0
> -    }
> -    return 1
> -}
> diff --git a/gdb/testsuite/lib/rust-support.exp
> b/gdb/testsuite/lib/rust-support.exp
> index 2a12cca..a80d60b 100644
> --- a/gdb/testsuite/lib/rust-support.exp
> +++ b/gdb/testsuite/lib/rust-support.exp
> @@ -15,16 +15,6 @@
> 
>  # Auxiliary function to set the language to Rust.
>  # The result is 1 (true) for success, 0 (false) for failure.

Comment.

> -proc set_lang_rust {} {
> -    if [gdb_test_no_output "set language rust"] {
> -	return 0
> -    }
> -    if [gdb_test "show language" ".* source language is \"rust\"." \
> -	   "set language to \"rust\""] {
> -	return 0
> -    }
> -    return 1
> -}
> 
>  proc gdb_compile_rust {sources dest options} {
>      if {[llength $sources] > 1} {