Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: Joel Brobecker <brobecker@adacore.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [RFC/commmit] [testsuite/Ada] stop using project files when building test programs
Date: Wed, 23 Dec 2015 15:21:00 -0000	[thread overview]
Message-ID: <567ABC0C.3040204@redhat.com> (raw)
In-Reply-To: <20151223144303.GC18676@adacore.com>

On 12/23/2015 02:43 PM, Joel Brobecker wrote:
>> > Humpf, that's a fair number of reasons showing that assuming 8.6
>> > may not be reasonable. Bouh...
>> > 
>>> > > How hard would it be to avoid try/finally?  Wouldn't you just have to
>>> > > use catch instead?
>> > 
>> > I don't think it would be very hard. I think catch will work, but
>> > will be a little more convoluted. I'll give it a try...
> Still, for a complete dummy like myself, that took nearly 45 minutes,
> just to make sure I understand TCL semantics better, and find a machine
> that has TCL 8.5...
> 

Thanks for doing this.

> I would appreciate a review of the attached patch, because TCL and
> I are not regular friends.

:-)  Looks generally good to me.  A couple minor comments below.



> --- a/gdb/testsuite/gdb.ada/cond_lang.exp
> +++ b/gdb/testsuite/gdb.ada/cond_lang.exp
> @@ -39,7 +39,7 @@ gdb_test "show lang" \
>  # current language mode is auto, and the breakpoint is inside Ada code.
>  set bp_location [gdb_get_line_number "STOP" ${testdir}/mixed.adb]
>  gdb_test "break mixed.adb:${bp_location} if light = green" \
> -         "Breakpoint \[0-9\]* at .*: file .*/mixed.adb, line \[0-9\]*\\."
> +         "Breakpoint \[0-9\]* at .*: file (.*/)?mixed.adb, line \[0-9\]*\\."

Isn't that the same as just:

 -         "Breakpoint \[0-9\]* at .*: file .*/mixed.adb, line \[0-9\]*\\."
 +         "Breakpoint \[0-9\]* at .*: file .*mixed.adb, line \[0-9\]*\\."

?

>  gdb_test "continue" \
> -    "Breakpoint .*, pck\\.call_me \\(w=(w@entry=)?50\\) at .*/pck.adb:.*" \
> +    "Breakpoint .*, pck\\.call_me \\(w=(w@entry=)?50\\) at (.*)?/pck.adb:.*" \
>      "continue to call_me"

Likewise, I think this is a no-op.  Did you mean to put the / inside the
parens like in the other case?  If so I'd suggest:

 -    "Breakpoint .*, pck\\.call_me \\(w=(w@entry=)?50\\) at .*/pck.adb:.*" \
 +    "Breakpoint .*, pck\\.call_me \\(w=(w@entry=)?50\\) at .*pck.adb:.*" \

>  
>  # And just to make sure, we also verify that the parameter value
> diff --git a/gdb/testsuite/lib/ada.exp b/gdb/testsuite/lib/ada.exp
> index 6a1e192..28284ff 100644
> --- a/gdb/testsuite/lib/ada.exp
> +++ b/gdb/testsuite/lib/ada.exp
> @@ -13,6 +13,26 @@
>  # You should have received a copy of the GNU General Public License
>  # along with this program.  If not, see <http://www.gnu.org/licenses/>.
>  
> +# Call target_compile with SOURCE DEST TYPE and OPTIONS as argument,
> +# after having temporarily changed the current working directory to
> +# BUILDDIR.
> +#
> +# FIXME: brobecke/2015-12-23: We can get rid of this function entirely
> +# when we are able to migrate to TCL 8.6, which added support for
> +# try { ... } finally {...} constructs.  Despite being released on
> +# December 2012, that version is still far from being widely used
> +# (in particular in the more exotic systems of the GCC compile farm).

Not sure this FIXME comment here turns out useful going forward.  We have
many other instances of catch in the harness.  And, IMO, having a wrapper
function looks nicer than open coding try/finally at the caller.  Once we
rely on 8.6 and thus try/finally, I would think we'd keep the wrapper
function, but reimplement its body with try/finally instead.

Thanks,
Pedro Alves


  reply	other threads:[~2015-12-23 15:21 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-22 15:33 Joel Brobecker
2015-12-23 13:21 ` Pedro Alves
2015-12-23 13:35   ` Joel Brobecker
2015-12-23 14:43     ` Joel Brobecker
2015-12-23 15:21       ` Pedro Alves [this message]
2015-12-23 15:30         ` Pedro Alves
2015-12-24  5:30           ` Joel Brobecker

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=567ABC0C.3040204@redhat.com \
    --to=palves@redhat.com \
    --cc=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    /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