This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: [PATCH 2/2] LD/testsuite: Expand STB_GNU_UNIQUE test coverage


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]