This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
RE: [PATCH 2/2] LD/testsuite: Expand STB_GNU_UNIQUE test coverage
- From: Matthew Fortune <Matthew dot Fortune at imgtec dot com>
- To: Maciej Rozycki <Maciej dot Rozycki at imgtec dot com>
- Cc: "binutils at sourceware dot org" <binutils at sourceware dot org>
- Date: Wed, 4 May 2016 15:55:43 +0000
- Subject: RE: [PATCH 2/2] LD/testsuite: Expand STB_GNU_UNIQUE test coverage
- Authentication-results: sourceware.org; auth=none
- References: <alpine dot DEB dot 2 dot 00 dot 1605032355130 dot 6794 at tp dot orcam dot me dot uk> <alpine dot DEB dot 2 dot 00 dot 1605040010060 dot 6794 at tp dot orcam dot me dot uk> <6D39441BF12EF246A7ABCE6654B023537E3F7E91 at hhmail02 dot hh dot imgtec dot org> <alpine dot DEB dot 2 dot 00 dot 1605041509120 dot 6794 at tp dot orcam dot me dot uk>
Maciej Rozycki <Maciej.Rozycki@imgtec.com> writes:
> On Wed, 4 May 2016, Matthew Fortune wrote:
>
> > > +# Exclude some more targets; feel free to include your favorite one
> > > +# if you like. The MSP430 and Visium targets set the ELF header's
> > > +# OSABI field to ELFOSABI_STANDALONE and cannot support
> STB_GNU_UNIQUE.
> > > +if { !([istarget "*-*-elf*"]
> > > + && ![istarget "msp430-*-*"]
> > > + && ![istarget "visium-*-*"])
> > > + && ![istarget *-*-nacl*]
> > > + && ![istarget *-*-linux*]
> > > + && ![istarget *-*-gnu*] } {
> > > verbose "UNIQUE tests not run - target does not support UNIQUE"
> > > return
> > > }
> >
> > Quite subjective but I found the new condition hard to read; the
> > following seems to match the comment more naturally to me:
> >
> > if { (![istarget "*-*-elf*"]
> > || [istarget "msp430-*-*"]
> > || [istarget "visium-*-*"])
> > && ![istarget *-*-nacl*]
> > && ![istarget *-*-linux*]
> > && ![istarget *-*-gnu*] } {
>
> I decided keeping all the conditions negated at the outer level was
> more consistent and natural actually. I won't mind updating this
> statement to something like:
>
> if { !([istarget "*-*-elf*"]
> && !([istarget "msp430-*-*"] || [istarget "visium-*-*"]))
> && ![istarget *-*-nacl*]
> && ![istarget *-*-linux*]
> && ![istarget *-*-gnu*] } {
>
> though, or maybe even make another De Morgan transformation and have a
> single negation at the then outermost level. Keeping the `*-*-elf*'
> exceptions on a single line might improve readability, although it'll be
> lost with the first addition of another exception.
Given you had already thought about it then I'd go with your original.
Matthew