diff mbox

[gdb/testsuite] XFAIL under Clang tests using label debug info

Message ID CAENS6EsnBvJpjmxyoFv-iLe=_nn3qJP=qj3at-kXPwYBDLnWGQ@mail.gmail.com
State New
Headers show

Commit Message

David Blaikie Sept. 5, 2019, 10:12 p.m. UTC 
Clang now supports labels - so I'd like to essentially revert this patch.

Is this OK?


On Thu, Apr 24, 2014 at 8:23 PM David Blaikie <dblaikie@gmail.com> wrote:
>
> On Wed, Apr 23, 2014 at 3:03 PM, Doug Evans <dje@google.com> wrote:
> > David Blaikie writes:
> >  > Clang doesn't emit debug info for labels (Clang PR14500). XFAIL a
> >  > bunch of GDB tests that rely on debug info for labels.
> >  >
> >  > For some reason gdb.linespec/ls-expr.exp gathered all tests into a
> >  > dictionary and then ran them. This made it hard to XFAIL just the
> >  > right tests. I refactored this to execute the tests directly, removing
> >  > the dictionary so I could XFAIL the right tests. Is there a reason it
> >  > would've been written that way? Does my patch break it in some way?
> >  > commit c438cb16b63292e415330f289616c4e4ecece63c
> >  > Author: David Blaikie <dblaikie@gmail.com>
> >  > Date:   Sun Apr 13 11:42:02 2014 -0700
> >  >
> >  >     XFAIL under Clang tests using labels
> >  >
> >  >     gdb/testsuite/
> >  >      * gdb.base/label.exp: XFAIL label related tests under Clang.
> >  >      * gdb.cp/cplabel.exp: Ditto.
> >  >      * gdb.linespec/ls-errs.exp: Refactor tests to execute directly
> >  >      and XFAIL under Clang those using labels.
> >
> > LGTM
> >
> >  > diff --git gdb/testsuite/ChangeLog gdb/testsuite/ChangeLog
> >  > index 730c116..b04b940 100644
> >  > --- gdb/testsuite/ChangeLog
> >  > +++ gdb/testsuite/ChangeLog
> >  > @@ -1,3 +1,9 @@
> >  > +2014-04-12  David Blaikie  <dblaikie@gmail.com>
> >  > +
> >  > +        * gdb.base/label.exp: XFAIL label related tests under Clang.
> >  > +    * gdb.cp/cplabel.exp: Ditto.
> >  > +    * gdb.linespec/ls-errs.exp: Refactor tests to execute directly and XFAIL under Clang those using labels.
> >  > +
> >
> > Nit: space vs tabs.  Just use tabs.
> > Plus line is longer than 80 chars.
>
> Thanks for the catches - fixed those up and committed in
> c2e827ad5340fcf1735df6c77cb0311e56b985ef.
>
> Also refactored some of the xfails along the lines of what Pedro
> suggested in the one test case that had several similar failures
> (gdb.base/label.exp). If/when we fix this in Clang it might be worth
> refactoring into a common function (though I'm personally not very
> vested in keeping the test suite usable with anything other than ToT
> Clang - perhaps others are).

Comments

Doug Evans via gdb-patches Sept. 6, 2019, 12:52 a.m. UTC | #1 
SGTM

On Thu, Sep 5, 2019 at 3:12 PM David Blaikie <dblaikie@gmail.com> wrote:

> Clang now supports labels - so I'd like to essentially revert this patch.
>
> Is this OK?
>
>
> On Thu, Apr 24, 2014 at 8:23 PM David Blaikie <dblaikie@gmail.com> wrote:
> >
> > On Wed, Apr 23, 2014 at 3:03 PM, Doug Evans <dje@google.com> wrote:
> > > David Blaikie writes:
> > >  > Clang doesn't emit debug info for labels (Clang PR14500). XFAIL a
> > >  > bunch of GDB tests that rely on debug info for labels.
> > >  >
> > >  > For some reason gdb.linespec/ls-expr.exp gathered all tests into a
> > >  > dictionary and then ran them. This made it hard to XFAIL just the
> > >  > right tests. I refactored this to execute the tests directly,
> removing
> > >  > the dictionary so I could XFAIL the right tests. Is there a reason
> it
> > >  > would've been written that way? Does my patch break it in some way?
> > >  > commit c438cb16b63292e415330f289616c4e4ecece63c
> > >  > Author: David Blaikie <dblaikie@gmail.com>
> > >  > Date:   Sun Apr 13 11:42:02 2014 -0700
> > >  >
> > >  >     XFAIL under Clang tests using labels
> > >  >
> > >  >     gdb/testsuite/
> > >  >      * gdb.base/label.exp: XFAIL label related tests under Clang.
> > >  >      * gdb.cp/cplabel.exp: Ditto.
> > >  >      * gdb.linespec/ls-errs.exp: Refactor tests to execute directly
> > >  >      and XFAIL under Clang those using labels.
> > >
> > > LGTM
> > >
> > >  > diff --git gdb/testsuite/ChangeLog gdb/testsuite/ChangeLog
> > >  > index 730c116..b04b940 100644
> > >  > --- gdb/testsuite/ChangeLog
> > >  > +++ gdb/testsuite/ChangeLog
> > >  > @@ -1,3 +1,9 @@
> > >  > +2014-04-12  David Blaikie  <dblaikie@gmail.com>
> > >  > +
> > >  > +        * gdb.base/label.exp: XFAIL label related tests under
> Clang.
> > >  > +    * gdb.cp/cplabel.exp: Ditto.
> > >  > +    * gdb.linespec/ls-errs.exp: Refactor tests to execute directly
> and XFAIL under Clang those using labels.
> > >  > +
> > >
> > > Nit: space vs tabs.  Just use tabs.
> > > Plus line is longer than 80 chars.
> >
> > Thanks for the catches - fixed those up and committed in
> > c2e827ad5340fcf1735df6c77cb0311e56b985ef.
> >
> > Also refactored some of the xfails along the lines of what Pedro
> > suggested in the one test case that had several similar failures
> > (gdb.base/label.exp). If/when we fix this in Clang it might be worth
> > refactoring into a common function (though I'm personally not very
> > vested in keeping the test suite usable with anything other than ToT
> > Clang - perhaps others are).
>
Doug Evans via gdb-patches Sept. 6, 2019, 2:47 a.m. UTC | #2 
On Thu, Sep 5, 2019 at 5:12 PM David Blaikie <dblaikie@gmail.com> wrote:
>
> Clang now supports labels - so I'd like to essentially revert this patch.

Out of curiosity, when did clang get support for this?

Christian

> Is this OK?
>
>
> On Thu, Apr 24, 2014 at 8:23 PM David Blaikie <dblaikie@gmail.com> wrote:
> >
> > On Wed, Apr 23, 2014 at 3:03 PM, Doug Evans <dje@google.com> wrote:
> > > David Blaikie writes:
> > >  > Clang doesn't emit debug info for labels (Clang PR14500). XFAIL a
> > >  > bunch of GDB tests that rely on debug info for labels.
> > >  >
> > >  > For some reason gdb.linespec/ls-expr.exp gathered all tests into a
> > >  > dictionary and then ran them. This made it hard to XFAIL just the
> > >  > right tests. I refactored this to execute the tests directly, removing
> > >  > the dictionary so I could XFAIL the right tests. Is there a reason it
> > >  > would've been written that way? Does my patch break it in some way?
> > >  > commit c438cb16b63292e415330f289616c4e4ecece63c
> > >  > Author: David Blaikie <dblaikie@gmail.com>
> > >  > Date:   Sun Apr 13 11:42:02 2014 -0700
> > >  >
> > >  >     XFAIL under Clang tests using labels
> > >  >
> > >  >     gdb/testsuite/
> > >  >      * gdb.base/label.exp: XFAIL label related tests under Clang.
> > >  >      * gdb.cp/cplabel.exp: Ditto.
> > >  >      * gdb.linespec/ls-errs.exp: Refactor tests to execute directly
> > >  >      and XFAIL under Clang those using labels.
> > >
> > > LGTM
> > >
> > >  > diff --git gdb/testsuite/ChangeLog gdb/testsuite/ChangeLog
> > >  > index 730c116..b04b940 100644
> > >  > --- gdb/testsuite/ChangeLog
> > >  > +++ gdb/testsuite/ChangeLog
> > >  > @@ -1,3 +1,9 @@
> > >  > +2014-04-12  David Blaikie  <dblaikie@gmail.com>
> > >  > +
> > >  > +        * gdb.base/label.exp: XFAIL label related tests under Clang.
> > >  > +    * gdb.cp/cplabel.exp: Ditto.
> > >  > +    * gdb.linespec/ls-errs.exp: Refactor tests to execute directly and XFAIL under Clang those using labels.
> > >  > +
> > >
> > > Nit: space vs tabs.  Just use tabs.
> > > Plus line is longer than 80 chars.
> >
> > Thanks for the catches - fixed those up and committed in
> > c2e827ad5340fcf1735df6c77cb0311e56b985ef.
> >
> > Also refactored some of the xfails along the lines of what Pedro
> > suggested in the one test case that had several similar failures
> > (gdb.base/label.exp). If/when we fix this in Clang it might be worth
> > refactoring into a common function (though I'm personally not very
> > vested in keeping the test suite usable with anything other than ToT
> > Clang - perhaps others are).
Eric Christopher Sept. 6, 2019, 4:17 a.m. UTC | #3 
https://bugs.llvm.org/show_bug.cgi?id=14500#c3 has the relevant information
:)

On Thu, Sep 5, 2019, 7:48 PM Christian Biesinger via gdb-patches <
gdb-patches@sourceware.org> wrote:

> On Thu, Sep 5, 2019 at 5:12 PM David Blaikie <dblaikie@gmail.com> wrote:
> >
> > Clang now supports labels - so I'd like to essentially revert this patch.
>
> Out of curiosity, when did clang get support for this?
>
> Christian
>
> > Is this OK?
> >
> >
> > On Thu, Apr 24, 2014 at 8:23 PM David Blaikie <dblaikie@gmail.com>
> wrote:
> > >
> > > On Wed, Apr 23, 2014 at 3:03 PM, Doug Evans <dje@google.com> wrote:
> > > > David Blaikie writes:
> > > >  > Clang doesn't emit debug info for labels (Clang PR14500). XFAIL a
> > > >  > bunch of GDB tests that rely on debug info for labels.
> > > >  >
> > > >  > For some reason gdb.linespec/ls-expr.exp gathered all tests into a
> > > >  > dictionary and then ran them. This made it hard to XFAIL just the
> > > >  > right tests. I refactored this to execute the tests directly,
> removing
> > > >  > the dictionary so I could XFAIL the right tests. Is there a
> reason it
> > > >  > would've been written that way? Does my patch break it in some
> way?
> > > >  > commit c438cb16b63292e415330f289616c4e4ecece63c
> > > >  > Author: David Blaikie <dblaikie@gmail.com>
> > > >  > Date:   Sun Apr 13 11:42:02 2014 -0700
> > > >  >
> > > >  >     XFAIL under Clang tests using labels
> > > >  >
> > > >  >     gdb/testsuite/
> > > >  >      * gdb.base/label.exp: XFAIL label related tests under Clang.
> > > >  >      * gdb.cp/cplabel.exp: Ditto.
> > > >  >      * gdb.linespec/ls-errs.exp: Refactor tests to execute
> directly
> > > >  >      and XFAIL under Clang those using labels.
> > > >
> > > > LGTM
> > > >
> > > >  > diff --git gdb/testsuite/ChangeLog gdb/testsuite/ChangeLog
> > > >  > index 730c116..b04b940 100644
> > > >  > --- gdb/testsuite/ChangeLog
> > > >  > +++ gdb/testsuite/ChangeLog
> > > >  > @@ -1,3 +1,9 @@
> > > >  > +2014-04-12  David Blaikie  <dblaikie@gmail.com>
> > > >  > +
> > > >  > +        * gdb.base/label.exp: XFAIL label related tests under
> Clang.
> > > >  > +    * gdb.cp/cplabel.exp: Ditto.
> > > >  > +    * gdb.linespec/ls-errs.exp: Refactor tests to execute
> directly and XFAIL under Clang those using labels.
> > > >  > +
> > > >
> > > > Nit: space vs tabs.  Just use tabs.
> > > > Plus line is longer than 80 chars.
> > >
> > > Thanks for the catches - fixed those up and committed in
> > > c2e827ad5340fcf1735df6c77cb0311e56b985ef.
> > >
> > > Also refactored some of the xfails along the lines of what Pedro
> > > suggested in the one test case that had several similar failures
> > > (gdb.base/label.exp). If/when we fix this in Clang it might be worth
> > > refactoring into a common function (though I'm personally not very
> > > vested in keeping the test suite usable with anything other than ToT
> > > Clang - perhaps others are).
>
Doug Evans via gdb-patches Sept. 6, 2019, 5:25 p.m. UTC | #4 
Thanks! So I think that requires clang 10?

Christian

On Thu, Sep 5, 2019 at 11:17 PM Eric Christopher <echristo@gmail.com> wrote:

> https://bugs.llvm.org/show_bug.cgi?id=14500#c3 has the relevant
> information :)
>
> On Thu, Sep 5, 2019, 7:48 PM Christian Biesinger via gdb-patches <
> gdb-patches@sourceware.org> wrote:
>
>> On Thu, Sep 5, 2019 at 5:12 PM David Blaikie <dblaikie@gmail.com> wrote:
>> >
>> > Clang now supports labels - so I'd like to essentially revert this
>> patch.
>>
>> Out of curiosity, when did clang get support for this?
>>
>> Christian
>>
>> > Is this OK?
>> >
>> >
>> > On Thu, Apr 24, 2014 at 8:23 PM David Blaikie <dblaikie@gmail.com>
>> wrote:
>> > >
>> > > On Wed, Apr 23, 2014 at 3:03 PM, Doug Evans <dje@google.com> wrote:
>> > > > David Blaikie writes:
>> > > >  > Clang doesn't emit debug info for labels (Clang PR14500). XFAIL a
>> > > >  > bunch of GDB tests that rely on debug info for labels.
>> > > >  >
>> > > >  > For some reason gdb.linespec/ls-expr.exp gathered all tests into
>> a
>> > > >  > dictionary and then ran them. This made it hard to XFAIL just the
>> > > >  > right tests. I refactored this to execute the tests directly,
>> removing
>> > > >  > the dictionary so I could XFAIL the right tests. Is there a
>> reason it
>> > > >  > would've been written that way? Does my patch break it in some
>> way?
>> > > >  > commit c438cb16b63292e415330f289616c4e4ecece63c
>> > > >  > Author: David Blaikie <dblaikie@gmail.com>
>> > > >  > Date:   Sun Apr 13 11:42:02 2014 -0700
>> > > >  >
>> > > >  >     XFAIL under Clang tests using labels
>> > > >  >
>> > > >  >     gdb/testsuite/
>> > > >  >      * gdb.base/label.exp: XFAIL label related tests under Clang.
>> > > >  >      * gdb.cp/cplabel.exp: Ditto.
>> > > >  >      * gdb.linespec/ls-errs.exp: Refactor tests to execute
>> directly
>> > > >  >      and XFAIL under Clang those using labels.
>> > > >
>> > > > LGTM
>> > > >
>> > > >  > diff --git gdb/testsuite/ChangeLog gdb/testsuite/ChangeLog
>> > > >  > index 730c116..b04b940 100644
>> > > >  > --- gdb/testsuite/ChangeLog
>> > > >  > +++ gdb/testsuite/ChangeLog
>> > > >  > @@ -1,3 +1,9 @@
>> > > >  > +2014-04-12  David Blaikie  <dblaikie@gmail.com>
>> > > >  > +
>> > > >  > +        * gdb.base/label.exp: XFAIL label related tests under
>> Clang.
>> > > >  > +    * gdb.cp/cplabel.exp: Ditto.
>> > > >  > +    * gdb.linespec/ls-errs.exp: Refactor tests to execute
>> directly and XFAIL under Clang those using labels.
>> > > >  > +
>> > > >
>> > > > Nit: space vs tabs.  Just use tabs.
>> > > > Plus line is longer than 80 chars.
>> > >
>> > > Thanks for the catches - fixed those up and committed in
>> > > c2e827ad5340fcf1735df6c77cb0311e56b985ef.
>> > >
>> > > Also refactored some of the xfails along the lines of what Pedro
>> > > suggested in the one test case that had several similar failures
>> > > (gdb.base/label.exp). If/when we fix this in Clang it might be worth
>> > > refactoring into a common function (though I'm personally not very
>> > > vested in keeping the test suite usable with anything other than ToT
>> > > Clang - perhaps others are).
>>
>
David Blaikie Sept. 6, 2019, 8:23 p.m. UTC | #5 
On Fri, Sep 6, 2019 at 10:26 AM Christian Biesinger
<cbiesinger@google.com> wrote:
>
> Thanks! So I think that requires clang 10?

The fix was made last year (July 24, 2018) - so I think that means it
should be in Clang 7.0, though I haven't tested specifically.

>
> Christian
>
> On Thu, Sep 5, 2019 at 11:17 PM Eric Christopher <echristo@gmail.com> wrote:
>>
>> https://bugs.llvm.org/show_bug.cgi?id=14500#c3 has the relevant information :)

Thanks Eric!

>>
>> On Thu, Sep 5, 2019, 7:48 PM Christian Biesinger via gdb-patches <gdb-patches@sourceware.org> wrote:
>>>
>>> On Thu, Sep 5, 2019 at 5:12 PM David Blaikie <dblaikie@gmail.com> wrote:
>>> >
>>> > Clang now supports labels - so I'd like to essentially revert this patch.
>>>
>>> Out of curiosity, when did clang get support for this?
>>>
>>> Christian
>>>
>>> > Is this OK?
>>> >
>>> >
>>> > On Thu, Apr 24, 2014 at 8:23 PM David Blaikie <dblaikie@gmail.com> wrote:
>>> > >
>>> > > On Wed, Apr 23, 2014 at 3:03 PM, Doug Evans <dje@google.com> wrote:
>>> > > > David Blaikie writes:
>>> > > >  > Clang doesn't emit debug info for labels (Clang PR14500). XFAIL a
>>> > > >  > bunch of GDB tests that rely on debug info for labels.
>>> > > >  >
>>> > > >  > For some reason gdb.linespec/ls-expr.exp gathered all tests into a
>>> > > >  > dictionary and then ran them. This made it hard to XFAIL just the
>>> > > >  > right tests. I refactored this to execute the tests directly, removing
>>> > > >  > the dictionary so I could XFAIL the right tests. Is there a reason it
>>> > > >  > would've been written that way? Does my patch break it in some way?
>>> > > >  > commit c438cb16b63292e415330f289616c4e4ecece63c
>>> > > >  > Author: David Blaikie <dblaikie@gmail.com>
>>> > > >  > Date:   Sun Apr 13 11:42:02 2014 -0700
>>> > > >  >
>>> > > >  >     XFAIL under Clang tests using labels
>>> > > >  >
>>> > > >  >     gdb/testsuite/
>>> > > >  >      * gdb.base/label.exp: XFAIL label related tests under Clang.
>>> > > >  >      * gdb.cp/cplabel.exp: Ditto.
>>> > > >  >      * gdb.linespec/ls-errs.exp: Refactor tests to execute directly
>>> > > >  >      and XFAIL under Clang those using labels.
>>> > > >
>>> > > > LGTM
>>> > > >
>>> > > >  > diff --git gdb/testsuite/ChangeLog gdb/testsuite/ChangeLog
>>> > > >  > index 730c116..b04b940 100644
>>> > > >  > --- gdb/testsuite/ChangeLog
>>> > > >  > +++ gdb/testsuite/ChangeLog
>>> > > >  > @@ -1,3 +1,9 @@
>>> > > >  > +2014-04-12  David Blaikie  <dblaikie@gmail.com>
>>> > > >  > +
>>> > > >  > +        * gdb.base/label.exp: XFAIL label related tests under Clang.
>>> > > >  > +    * gdb.cp/cplabel.exp: Ditto.
>>> > > >  > +    * gdb.linespec/ls-errs.exp: Refactor tests to execute directly and XFAIL under Clang those using labels.
>>> > > >  > +
>>> > > >
>>> > > > Nit: space vs tabs.  Just use tabs.
>>> > > > Plus line is longer than 80 chars.
>>> > >
>>> > > Thanks for the catches - fixed those up and committed in
>>> > > c2e827ad5340fcf1735df6c77cb0311e56b985ef.
>>> > >
>>> > > Also refactored some of the xfails along the lines of what Pedro
>>> > > suggested in the one test case that had several similar failures
>>> > > (gdb.base/label.exp). If/when we fix this in Clang it might be worth
>>> > > refactoring into a common function (though I'm personally not very
>>> > > vested in keeping the test suite usable with anything other than ToT
>>> > > Clang - perhaps others are).
Doug Evans via gdb-patches Sept. 6, 2019, 8:24 p.m. UTC | #6 
On Fri, Sep 6, 2019 at 3:23 PM David Blaikie <dblaikie@gmail.com> wrote:

> On Fri, Sep 6, 2019 at 10:26 AM Christian Biesinger
> <cbiesinger@google.com> wrote:
> >
> > Thanks! So I think that requires clang 10?
>
> The fix was made last year (July 24, 2018) - so I think that means it
> should be in Clang 7.0, though I haven't tested specifically.
>

Reading is hard :( Thanks. That certainly seems long enough ago that
removing this is reasonable (but I'm not an approver)

Christian


>
> >
> > Christian
> >
> > On Thu, Sep 5, 2019 at 11:17 PM Eric Christopher <echristo@gmail.com>
> wrote:
> >>
> >> https://bugs.llvm.org/show_bug.cgi?id=14500#c3 has the relevant
> information :)
>
> Thanks Eric!
>
> >>
> >> On Thu, Sep 5, 2019, 7:48 PM Christian Biesinger via gdb-patches <
> gdb-patches@sourceware.org> wrote:
> >>>
> >>> On Thu, Sep 5, 2019 at 5:12 PM David Blaikie <dblaikie@gmail.com>
> wrote:
> >>> >
> >>> > Clang now supports labels - so I'd like to essentially revert this
> patch.
> >>>
> >>> Out of curiosity, when did clang get support for this?
> >>>
> >>> Christian
> >>>
> >>> > Is this OK?
> >>> >
> >>> >
> >>> > On Thu, Apr 24, 2014 at 8:23 PM David Blaikie <dblaikie@gmail.com>
> wrote:
> >>> > >
> >>> > > On Wed, Apr 23, 2014 at 3:03 PM, Doug Evans <dje@google.com>
> wrote:
> >>> > > > David Blaikie writes:
> >>> > > >  > Clang doesn't emit debug info for labels (Clang PR14500).
> XFAIL a
> >>> > > >  > bunch of GDB tests that rely on debug info for labels.
> >>> > > >  >
> >>> > > >  > For some reason gdb.linespec/ls-expr.exp gathered all tests
> into a
> >>> > > >  > dictionary and then ran them. This made it hard to XFAIL just
> the
> >>> > > >  > right tests. I refactored this to execute the tests directly,
> removing
> >>> > > >  > the dictionary so I could XFAIL the right tests. Is there a
> reason it
> >>> > > >  > would've been written that way? Does my patch break it in
> some way?
> >>> > > >  > commit c438cb16b63292e415330f289616c4e4ecece63c
> >>> > > >  > Author: David Blaikie <dblaikie@gmail.com>
> >>> > > >  > Date:   Sun Apr 13 11:42:02 2014 -0700
> >>> > > >  >
> >>> > > >  >     XFAIL under Clang tests using labels
> >>> > > >  >
> >>> > > >  >     gdb/testsuite/
> >>> > > >  >      * gdb.base/label.exp: XFAIL label related tests under
> Clang.
> >>> > > >  >      * gdb.cp/cplabel.exp: Ditto.
> >>> > > >  >      * gdb.linespec/ls-errs.exp: Refactor tests to execute
> directly
> >>> > > >  >      and XFAIL under Clang those using labels.
> >>> > > >
> >>> > > > LGTM
> >>> > > >
> >>> > > >  > diff --git gdb/testsuite/ChangeLog gdb/testsuite/ChangeLog
> >>> > > >  > index 730c116..b04b940 100644
> >>> > > >  > --- gdb/testsuite/ChangeLog
> >>> > > >  > +++ gdb/testsuite/ChangeLog
> >>> > > >  > @@ -1,3 +1,9 @@
> >>> > > >  > +2014-04-12  David Blaikie  <dblaikie@gmail.com>
> >>> > > >  > +
> >>> > > >  > +        * gdb.base/label.exp: XFAIL label related tests
> under Clang.
> >>> > > >  > +    * gdb.cp/cplabel.exp: Ditto.
> >>> > > >  > +    * gdb.linespec/ls-errs.exp: Refactor tests to execute
> directly and XFAIL under Clang those using labels.
> >>> > > >  > +
> >>> > > >
> >>> > > > Nit: space vs tabs.  Just use tabs.
> >>> > > > Plus line is longer than 80 chars.
> >>> > >
> >>> > > Thanks for the catches - fixed those up and committed in
> >>> > > c2e827ad5340fcf1735df6c77cb0311e56b985ef.
> >>> > >
> >>> > > Also refactored some of the xfails along the lines of what Pedro
> >>> > > suggested in the one test case that had several similar failures
> >>> > > (gdb.base/label.exp). If/when we fix this in Clang it might be
> worth
> >>> > > refactoring into a common function (though I'm personally not very
> >>> > > vested in keeping the test suite usable with anything other than
> ToT
> >>> > > Clang - perhaps others are).
>
David Blaikie Sept. 6, 2019, 9:18 p.m. UTC | #7 
Thanks!
Committed as 736b0f76188c7a4d497a5e2255b78af909393afe

On Thu, Sep 5, 2019 at 5:53 PM Doug Evans <dje@google.com> wrote:
>
> SGTM
>
> On Thu, Sep 5, 2019 at 3:12 PM David Blaikie <dblaikie@gmail.com> wrote:
>>
>> Clang now supports labels - so I'd like to essentially revert this patch.
>>
>> Is this OK?
>>
>>
>> On Thu, Apr 24, 2014 at 8:23 PM David Blaikie <dblaikie@gmail.com> wrote:
>> >
>> > On Wed, Apr 23, 2014 at 3:03 PM, Doug Evans <dje@google.com> wrote:
>> > > David Blaikie writes:
>> > >  > Clang doesn't emit debug info for labels (Clang PR14500). XFAIL a
>> > >  > bunch of GDB tests that rely on debug info for labels.
>> > >  >
>> > >  > For some reason gdb.linespec/ls-expr.exp gathered all tests into a
>> > >  > dictionary and then ran them. This made it hard to XFAIL just the
>> > >  > right tests. I refactored this to execute the tests directly, removing
>> > >  > the dictionary so I could XFAIL the right tests. Is there a reason it
>> > >  > would've been written that way? Does my patch break it in some way?
>> > >  > commit c438cb16b63292e415330f289616c4e4ecece63c
>> > >  > Author: David Blaikie <dblaikie@gmail.com>
>> > >  > Date:   Sun Apr 13 11:42:02 2014 -0700
>> > >  >
>> > >  >     XFAIL under Clang tests using labels
>> > >  >
>> > >  >     gdb/testsuite/
>> > >  >      * gdb.base/label.exp: XFAIL label related tests under Clang.
>> > >  >      * gdb.cp/cplabel.exp: Ditto.
>> > >  >      * gdb.linespec/ls-errs.exp: Refactor tests to execute directly
>> > >  >      and XFAIL under Clang those using labels.
>> > >
>> > > LGTM
>> > >
>> > >  > diff --git gdb/testsuite/ChangeLog gdb/testsuite/ChangeLog
>> > >  > index 730c116..b04b940 100644
>> > >  > --- gdb/testsuite/ChangeLog
>> > >  > +++ gdb/testsuite/ChangeLog
>> > >  > @@ -1,3 +1,9 @@
>> > >  > +2014-04-12  David Blaikie  <dblaikie@gmail.com>
>> > >  > +
>> > >  > +        * gdb.base/label.exp: XFAIL label related tests under Clang.
>> > >  > +    * gdb.cp/cplabel.exp: Ditto.
>> > >  > +    * gdb.linespec/ls-errs.exp: Refactor tests to execute directly and XFAIL under Clang those using labels.
>> > >  > +
>> > >
>> > > Nit: space vs tabs.  Just use tabs.
>> > > Plus line is longer than 80 chars.
>> >
>> > Thanks for the catches - fixed those up and committed in
>> > c2e827ad5340fcf1735df6c77cb0311e56b985ef.
>> >
>> > Also refactored some of the xfails along the lines of what Pedro
>> > suggested in the one test case that had several similar failures
>> > (gdb.base/label.exp). If/when we fix this in Clang it might be worth
>> > refactoring into a common function (though I'm personally not very
>> > vested in keeping the test suite usable with anything other than ToT
>> > Clang - perhaps others are).
diff mbox

Patch

commit 78c67ab1f234b91aa3016ee29966ba141fd5b5b1
Author: David Blaikie <dblaikie@gmail.com>
Date:   Thu Sep 5 15:01:25 2019 -0700

    un-XFAIL under Clang tests using labels
    
    gdb/testsuite/
            * gdb.base/label.exp: un-XFAIL label related tests under Clang.
            * gdb.cp/cplabel.exp: Ditto.
            * gdb.linespec/ls-errs.exp: Ditto.

diff --git gdb/testsuite/ChangeLog gdb/testsuite/ChangeLog
index 46c7b5f927..1d1b706bb2 100644
--- gdb/testsuite/ChangeLog
+++ gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@ 
+2014-04-24  David Blaikie  <dblaikie@gmail.com>
+
+	* gdb.base/label.exp: un-XFAIL label related tests under Clang.
+	* gdb.cp/cplabel.exp: Ditto.
+	* gdb.linespec/ls-errs.exp: Ditto.
+
 2019-09-04  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
 
 	* gdb.base/info-var.exp: Test info variables without running
diff --git gdb/testsuite/gdb.base/label.exp gdb/testsuite/gdb.base/label.exp
index bce90cb60e..ec4851970e 100644
--- gdb/testsuite/gdb.base/label.exp
+++ gdb/testsuite/gdb.base/label.exp
@@ -29,34 +29,24 @@  if {![runto_main]} {
   return -1
 }
 
-set has_pr_14500_fixed 1
-if {[test_compiler_info {clang-*-*}]} {
-  set has_pr_14500_fixed 0
-}
-
-if {!$has_pr_14500_fixed} {  setup_xfail clang/14500 *-*-* }
 gdb_test "break here" \
   "Breakpoint.*at.*" \
   "breakpoint here"
 
-if {!$has_pr_14500_fixed} {  setup_xfail clang/14500 *-*-* }
 gdb_test "break main:there" \
   "Breakpoint.*at.*" \
   "breakpoint there"
 
-if {!$has_pr_14500_fixed} {  setup_xfail clang/14500 *-*-* }
 gdb_test "cont" \
   "Breakpoint 3,.*" \
   "continue to 'there'"
 
-if {!$has_pr_14500_fixed} {  setup_xfail clang/14500 *-*-* }
 gdb_test "cont" \
   "Breakpoint 2,.*" \
   "continue to 'here'"
 
 rerun_to_main
 
-if {!$has_pr_14500_fixed} {  setup_xfail clang/14500 *-*-* }
 gdb_test "cont" \
   "Breakpoint 3,.*" \
   "continue to 'there' after re-run"
diff --git gdb/testsuite/gdb.cp/cplabel.exp gdb/testsuite/gdb.cp/cplabel.exp
index 26818cb1a2..93febd6925 100644
--- gdb/testsuite/gdb.cp/cplabel.exp
+++ gdb/testsuite/gdb.cp/cplabel.exp
@@ -34,7 +34,6 @@  set labels {"to_the_top" "get_out_of_here"}
 foreach m $methods {
     foreach l $labels {
 	set line [gdb_get_line_number "$m:$l"]
-	if {[test_compiler_info {clang-*-*}]} { setup_xfail clang/14500 *-*-* }
 	gdb_test "break foo::$m:$l" \
 	    "Breakpoint $decimal at $hex: file .*$srcfile, line $line\."
     }
diff --git gdb/testsuite/gdb.linespec/ls-errs.exp gdb/testsuite/gdb.linespec/ls-errs.exp
index 92a858ce26..f031c461cb 100644
--- gdb/testsuite/gdb.linespec/ls-errs.exp
+++ gdb/testsuite/gdb.linespec/ls-errs.exp
@@ -231,9 +231,6 @@  proc do_test {lang} {
 
     foreach x $spaces {
 	test_break "main${x}there" invalid_label "there" "main"
-	if {[test_compiler_info {clang-*-*}]} {
-	    setup_xfail clang/14500 *-*-*
-	}
 	test_break "main:here${x}" unexpected "end of input"
     }