[review] [gdb/testsuite] Compile ada with -lgnarl_pic and -lgnat_pic if required
Luis Machado (Code Review)
gerrit@gnutoolchain-gerrit.osci.io
Mon Oct 21 20:52:00 GMT 2019
Luis Machado has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/32
......................................................................
Patch Set 1:
(4 comments)
At first the change looked a bit too complicated for what it does. But i realized that, if we don't want to keep compiling things incorrectly over and over again, caching the setting is the only way to do it. And that makes the code a bit larger.
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/32/1/gdb/testsuite/lib/ada.exp
File gdb/testsuite/lib/ada.exp:
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/32/1/gdb/testsuite/lib/ada.exp@16
PS1, Line 16: # Call target_compile with SOURCE DEST TYPE and OPTIONS as argument,
: # after having temporarily changed the current working directory to
: # BUILDDIR.
The location of this comment needs to be adjusted, since it is now out of place.
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/32/1/gdb/testsuite/lib/ada.exp@20
PS1, Line 20: proc gdb_simple_compile_ada {name code {type object} {compile_flags {}} {object obj}} {
Some documentation for this function would be nice. It seems to be a helper function that returns true/false for whether a compilation succeeded or not?
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/32/1/gdb/testsuite/lib/ada.exp@55
PS1, Line 55: gdb_caching_proc gdb_ada_needs_libs_pic_suffix {
It would be nice to have some documentation here explaining that we're caching true/false for whether we need to use the special workaround compilation flags.
https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/32/1/gdb/testsuite/lib/ada.exp@131
PS1, Line 131: # Compile some Ada code.
I think this is now a helper function? Should we adjust its documentation a bit?
My understanding is that gdb_compile_ada_1 now only silently tests a compilation and doesn't actually gdb_compile_test it?
More information about the Gdb-patches
mailing list