[2/3] GNAT/testsuite: Pass the `ada' option to target compilation

Message ID alpine.DEB.2.20.1905142118020.18422@tpp.hgst.com
State New
Headers show
Series
  • GNAT test suite fixes for build sysroot
Related show

Commit Message

Maciej W. Rozycki May 14, 2019, 9:48 p.m.
Pass the `ada' option to DejaGNU's `target_compile' procedure, which by 
default calls `default_target_compile', so that it arranges for an Ada 
compilation rather the default of C.  We set the compiler to `gnatmake' 
manually here, so that part of the logic in `default_target_compile' is 
not used, but it affects other settings, such as the use of `adaflags'.

	gcc/testsuite/
	* lib/gnat.exp (gnat_target_compile): Pass the `ada' option to 
	`target_compile'.
---
Hi,

 Unfortunately I have exhausted the limit of changes I can make to GCC
without my WDC copyright paperwork sorted with FSF.  OK to apply once that
has been completed?

  Maciej
---
 gcc/testsuite/lib/gnat.exp |    2 ++
 1 file changed, 2 insertions(+)

gcc-test-gnat-options-ada.diff

Comments

Jacob Bachmeyer May 15, 2019, 11:12 p.m. | #1
Maciej W. Rozycki wrote:
> [...]

> ---

>  gcc/testsuite/lib/gnat.exp |    2 ++

>  1 file changed, 2 insertions(+)

>

> gcc-test-gnat-options-ada.diff

> Index: gcc/gcc/testsuite/lib/gnat.exp

> ===================================================================

> --- gcc.orig/gcc/testsuite/lib/gnat.exp

> +++ gcc/gcc/testsuite/lib/gnat.exp

> @@ -167,6 +167,8 @@ proc gnat_target_compile { source dest t

>  	set options [concat "additional_flags=$TOOL_OPTIONS" $options]

>      }

>  

> +    set options [concat "{ada}" $options]

> +

>      return [target_compile $source $dest $type $options]

>  }

>   

Your Tcl syntax looks suspicious to me.  Is there a reason for "ada" to 
be in both double quotes and braces?

Perhaps {lappend options ada} might be simpler?  Is placing ada at the 
beginning of the list important?

-- Jacob
Maciej W. Rozycki May 16, 2019, 12:38 p.m. | #2
On Wed, 15 May 2019, Jacob Bachmeyer wrote:

> > Index: gcc/gcc/testsuite/lib/gnat.exp

> > ===================================================================

> > --- gcc.orig/gcc/testsuite/lib/gnat.exp

> > +++ gcc/gcc/testsuite/lib/gnat.exp

> > @@ -167,6 +167,8 @@ proc gnat_target_compile { source dest t

> >  	set options [concat "additional_flags=$TOOL_OPTIONS" $options]

> >      }

> >  

> > +    set options [concat "{ada}" $options]

> > +

> >      return [target_compile $source $dest $type $options]

> >  }

> >   

> Your Tcl syntax looks suspicious to me.  Is there a reason for "ada" to 

> be in both double quotes and braces?


 Most existing `options' elements are lists, as shown by:

clone_output "options: $options\n"

placed at the top of `default_target_compile' (leading paths stripped 
here):

options: {ada} {additional_flags=-fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers -fdiagnostics-color=never --sysroot=.../sysroot} {additional_flags=-gnatJ -c -u} {compiler=.../gcc/gnatmake --GCC=.../gcc/xgcc --GNATBIND=.../gcc/gnatbind --GNATLINK=.../gcc/gnatlink -cargs -B.../gcc -largs --GCC=.../gcc/xgcc\ -B.../gcc\ -march=rv64imafdc\ -mabi=lp64d -margs --RTS=.../riscv64-linux-gnu/lib64/lp64d/libada -q -f} timeout=300

so I did this for consistency, although in reality it doesn't matter, not 
at least for `default_target_compile', and either approach would work.

 We are not consistent here in `gnat_target_compile' anyway, as you can 
see from the two existing `concat' invocations, and also the `timeout=300' 
element.

> Perhaps {lappend options ada} might be simpler?  Is placing ada at the 

> beginning of the list important?


 It can't be last because we override the default compiler otherwise
selected by this option in `default_target_compile', and then options 
passed in may override it further.  Overall I felt it to be safer if we 
placed the compiler type selection first rather than somewhere in the 
middle.

 I hope it clears your concerns.

  Maciej
Jacob Bachmeyer May 16, 2019, 10:57 p.m. | #3
Maciej W. Rozycki wrote:
> On Wed, 15 May 2019, Jacob Bachmeyer wrote:

> [...]

>  We are not consistent here in `gnat_target_compile' anyway, as you can 

> see from the two existing `concat' invocations, and also the `timeout=300' 

> element.

>   


That is the GCC testsuite rather than DejaGnu itself, so it is less of a 
concern to me.

>> Perhaps {lappend options ada} might be simpler?  Is placing ada at the 

>> beginning of the list important?

>>     

>

>  It can't be last because we override the default compiler otherwise

> selected by this option in `default_target_compile', and then options 

> passed in may override it further.  Overall I felt it to be safer if we 

> placed the compiler type selection first rather than somewhere in the 

> middle.

>   


This is probably a bug in DejaGnu, (those options should set defaults 
rather than override whatever else has been given) but you will still 
need to work around it for the installed base.

>  I hope it clears your concerns.

>   


As far as the patch to GCC goes, I am not worried.

-- Jacob

Patch

Index: gcc/gcc/testsuite/lib/gnat.exp
===================================================================
--- gcc.orig/gcc/testsuite/lib/gnat.exp
+++ gcc/gcc/testsuite/lib/gnat.exp
@@ -167,6 +167,8 @@  proc gnat_target_compile { source dest t
 	set options [concat "additional_flags=$TOOL_OPTIONS" $options]
     }
 
+    set options [concat "{ada}" $options]
+
     return [target_compile $source $dest $type $options]
 }