Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: Andrew Burgess <aburgess@broadcom.com>
Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Subject: Re: [PATCH] Call get_compiler_info directly from test_compiler_info if needed.
Date: Tue, 10 Dec 2013 15:49:00 -0000	[thread overview]
Message-ID: <52A737EE.80800@redhat.com> (raw)
In-Reply-To: <52A7303B.2080901@broadcom.com>

On 12/10/2013 03:16 PM, Andrew Burgess wrote:
> On 10/12/2013 3:00 PM, Pedro Alves wrote:
>> On 12/10/2013 02:43 PM, Andrew Burgess wrote:
>>>  proc test_compiler_info { {compiler ""} } {
>>>      global compiler_info
>>>  
>>> +    # Check that compiler_info has been initialised
>>> +    if [string match "" $compiler_info] {
>>> +	if [get_compiler_info] {
>>> +	    warning "Could not get compiler info"
>>> +	    return -1
>>> +	}
>>> +    }
>>
>> What about get_compiler_info's $arg?
> 
> In the cases where I removed calls to get_compiler_info no arg was being
> passed anyway, my assumption then is that in most cases the "default"
> result of get_compiler_info is fine.
> 
> If you really want to pass some args to get_compiler_info you can still
> do that in a separate call, and I left in place (in gdb.exp) a call that
> does just this.
> 
> The code in test_compiler_info will not overwrite an existing
> compiler_info value, so if you've taken care to call get_compiler_info
> yourself then all should still work as expected.

Yes, but, if the interface will be "you don't need to call
get_compiler_info yourself, except ...", is there any gain in
doing this here?  The current rule is "you need to call
get_compiler_info yourself, period.", which seems easier to
explain.  E.g., here:

> @@ -2027,10 +2027,6 @@ gdb_caching_proc skip_altivec_tests {
>
>      # Make sure we have a compiler that understands altivec.
>      set compile_flags {debug nowarnings}
> -    if [get_compiler_info] {
> -       warning "Could not get compiler info"
> -       return 1
> -    }
>      if [test_compiler_info gcc*] {
>          set compile_flags "$compile_flags additional_flags=-maltivec"
>      } elseif [test_compiler_info xlc*] {

Before, the removed get_compiler_info would always overwrite
whatever was in the compiler_info before.  Now, if the previous
call to get_compiler_info happened to get an $arg,
test_compiler_info will now reuse the wrong compiler_info.

It seems to me that it should be get_compiler_info that
caches its results, not test_compiler_info, taking into
account $arg.  And then, if we want to get rid of the
need to call get_compiler_info before test_compiler_info,
then test_compiler_info should have likewise an $arg parameter
that gets passed down to the get_compiler_info call.

-- 
Pedro Alves


  reply	other threads:[~2013-12-10 15:49 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-10 14:43 Andrew Burgess
2013-12-10 15:07 ` Pedro Alves
2013-12-10 15:16   ` Andrew Burgess
2013-12-10 15:49     ` Pedro Alves [this message]
2013-12-10 16:10       ` Call get_compiler_info before test_compiler_info. (was: Re: [PATCH] Call get_compiler_info directly from test_compiler_info if needed.) Andrew Burgess
2013-12-10 16:26         ` Call get_compiler_info before test_compiler_info Pedro Alves
2013-12-10 17:10           ` Andrew Burgess

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52A737EE.80800@redhat.com \
    --to=palves@redhat.com \
    --cc=aburgess@broadcom.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox