Re: [PATCH,v2] Make language setting tests more robust

[Pedro Alves <palves@redhat.com>]

On 02/01/2017 08:21 PM, Luis Machado wrote:

> 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.

Sorry to be a naysayer, but I'm not sure about this.  :-(  I recall fixing bugs
related to gdb printing that warning when it really should not, or causing
regressions locally due to breaking that logic, with the testsuite helping
notice them.  I worry that this would be hiding such bugs going forward.
Can you name the tests that where you saw the problem you mention?  Many of the tests
you're touching are changing the language after starting gdb with no binary
loaded, even.  Those definitely should not ever warn.  There are a few even that
have "no prompt when changing language"-like messages.

> 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.

One small advantage I see is that set_lang_<language> is a tcl symbol, so
if you typo it, you'll get a tcl error.  Kind of like avoiding sprinkling
magic constants in C.  You could keep the wrapper procs but define them
in terms of set_language, like:

proc set_lang_objc {} {
   set_language "objective-c"
}

But I won't insist.

> +# 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 be set.
> +#
> +# The second argument is an optional message to be output by the test.  If
> +# the message is empty, the command to set the language will be used instead.
> +
> +proc set_language { language { message "" } } {
> +    global gdb_prompt
> +
> +    set command "set language $language"
> +    set lang_pattern [string_to_regexp $language]
> +
> +    if { $message == "" } {
> +	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 $" {}
> +    }

Writing this as:

    set command_re [string_to_regexp $command]

    # Switch to the user-selected language.
    gdb_test_multiple $command $message {
	-re "Warning: the current language does not match this frame.\[\r\n\]+$gdb_prompt $" {
	}
	-re "^$command_re\r\n$gdb_prompt $" {
        }
	-re "$gdb_prompt" {
	  fail $message
	  return 0
	}
    }

Would be more robust in case the "Undefined item" output ever changes.

Thanks,
Pedro Alves