This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB 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: [RFC/commmit] [testsuite/Ada] stop using project files when building test programs


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


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