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
next prev parent 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).