public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Dave Korn <dave.korn.cygwin@gmail.com>
To: Kai Tietz <ktietz70@googlemail.com>
Cc: Binutils <binutils@sourceware.org>,
	 Dave Korn <dave.korn.cygwin@googlemail.com>
Subject: Re: [patch gas/testsuite SEH x64]: Some initial tests about SEH pseudo-operators
Date: Sun, 12 Sep 2010 16:35:00 -0000	[thread overview]
Message-ID: <4C8D0666.6060603@gmail.com> (raw)
In-Reply-To: <AANLkTi=Azz9dVJeiy8P6f3+YN6uxXiZ_KJcpomQ0+Upd@mail.gmail.com>

On 12/09/2010 16:39, Kai Tietz wrote:
> 2010/9/12 Dave Korn <dave.korn.cygwin:
>> On 12/09/2010 10:43, Kai Tietz wrote:

>>> this patch adds some x64 SEH related tests to gas' testsuite.

>>>           * /gas/pe/pe_seh.exp: New.

>>  Please let's not multiply expect scripts unnecessarily.  I can't think of
>> any reason not to just tag the "if ([istarget "x86_64-*-mingw*"])" clause onto
>> the end of the existing gas/pe/pe.exp, 

> Hmm, I think it is worth having here a separate .exp script. SEH is
> present for other PE-COFF targets, too. And so tests of features
> should be grouped IMHO.

  That is a non-sequitur.  Yes, SEH is present for other PE-COFF targets too,
but they could run the tests just the same regardless of whether those tests
are in the same .exp file or a separate one.

> What make you think that it is better to have just on giant .exp files
> containing everything unsorted?

  There's no need for it to be "unsorted"; tests within the file can still be
arranged into logical groups, and the whole thing formatted nicely.  Nor will
it be "giant"; there's only a few lines of tests in there already, so adding a
handful more won't make it giant.  (If we had hundreds of tests in there,
you'd have a point, but that's not going to happen any time in the foreseeable
future, so let's cross that bridge /if/ we come to it!)

  My advice is based on Alan's advice to me in an earlier thread(*):

> Yes, this was to avoid proliferation of .exp files.  More
> .exp files means slightly slower testsuite runs, for all targets.
> There isn't really any reason to put simple run_dump_test style tests
> in separate files.  You can select targets, set as and ld flags
> etc. all in their .d files.  I think the ideal is one main .exp file
> per directory to handle all the simple tests, with other .exp files as
> necessary for more complex tests, but it's not terribly important.

  Your new .exp file has 38 lines, of which all but 5 are an exact duplicate
of the contents of the existing pe.exp.  Redundancy is bad, and so is
redundancy!  This is why I can't see any value in having them in a separate file.

    cheers,
      DaveK
-- 
(*) - http://sourceware.org/ml/binutils/2009-05/msg00263.html

  reply	other threads:[~2010-09-12 16:35 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-12  9:44 Kai Tietz
2010-09-12 15:10 ` Dave Korn
2010-09-12 15:39   ` Kai Tietz
2010-09-12 16:35     ` Dave Korn [this message]
2010-09-13  7:15       ` Kai Tietz

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=4C8D0666.6060603@gmail.com \
    --to=dave.korn.cygwin@gmail.com \
    --cc=binutils@sourceware.org \
    --cc=dave.korn.cygwin@googlemail.com \
    --cc=ktietz70@googlemail.com \
    /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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).