| Message ID | 2B0DD900-561C-44FB-B3BC-E629E10EF0D1@sandoe-acoustics.co.uk |
|---|---|
| State | New |
| Headers | show |
On Sat, 4 Sep 2010, IainS wrote: > What do you think of these as common functionality to be collected up? 1. Functions to append either copied or new arguments to the new array of arguments without needing an error-prone calculation of the exact size of that array. Only Fortran has this at present, but it would clearly be good for all drivers. 2. Description of how the mapping from file extensions to languages differs from the default (for example, g++ compiles .c files as C++). Should be able to go in a datastructure. Consider having some structure provided by the drivers that is used by lookup_compiler to override its defaults rather than inserting -x options on the command line. 3. Logic for when the language's libraries need to be appended and for putting them in the right sequence (see what gfortranspec.c does about arranging the sequence -lgfortran -lm). It might be better for this to go not in lang_specific_driver but later, in gcc.c after it's determined the language for each input file; that way, lang_specific_driver wouldn't need to look at file suffixes and track -x options to work out whether a particular input file is a source file for its own language. 4. Related to that, logic identifying which option (OPT_ value) if any causes the static version of which library to be linked in, and controlling whether shared libgcc is the default for a particular driver or not. 5. There may be extra or different libraries for static linking or profiling; see what g++spec.c does. > 1/ determining that the action is PCH > (and ensuring that no link commands are put into the driver line -- these > cause collect2 to be invoked which barfs because there is no linking to do). Yes, I suppose so. > 3/ consider that the input files might be supplied from a file list and that > should be considered before quitting with a "no files, not prepared to ... " @files are expanded as the very first stage in processing argv, so lang_specific_driver does not need to consider them at all.
On 4 September 2010 16:05, Joseph S. Myers <joseph@codesourcery.com> wrote: > On Sat, 4 Sep 2010, IainS wrote: > >> What do you think of these as common functionality to be collected up? > > 1. Functions to append either copied or new arguments to the new array of > arguments without needing an error-prone calculation of the exact size of > that array. Only Fortran has this at present, but it would clearly be > good for all drivers. > > 2. Description of how the mapping from file extensions to languages > differs from the default (for example, g++ compiles .c files as C++). > Should be able to go in a datastructure. Consider having some structure > provided by the drivers that is used by lookup_compiler to override its > defaults rather than inserting -x options on the command line. > > 3. Logic for when the language's libraries need to be appended and for > putting them in the right sequence (see what gfortranspec.c does about > arranging the sequence -lgfortran -lm). It might be better for this to go > not in lang_specific_driver but later, in gcc.c after it's determined the > language for each input file; that way, lang_specific_driver wouldn't need > to look at file suffixes and track -x options to work out whether a > particular input file is a source file for its own language. > > 4. Related to that, logic identifying which option (OPT_ value) if any > causes the static version of which library to be linked in, and > controlling whether shared libgcc is the default for a particular driver > or not. > > 5. There may be extra or different libraries for static linking or > profiling; see what g++spec.c does. > >> 1/ determining that the action is PCH >> (and ensuring that no link commands are put into the driver line -- these >> cause collect2 to be invoked which barfs because there is no linking to do). > > Yes, I suppose so. > >> 3/ consider that the input files might be supplied from a file list and that >> should be considered before quitting with a "no files, not prepared to ... " > > @files are expanded as the very first stage in processing argv, so > lang_specific_driver does not need to consider them at all. > > -- > Joseph S. Myers > joseph@codesourcery.com > I played around there and i think i have a good idea of what it could look like now i'll aim to clean it up and send a patch around tomorrow night since i would need feedback on what it looks like. --Phil
Hey all I've built up a set of functions in making compiler drivers much simpler now i would like some feedback on the patch it gives objc a working compiler driver now and thanks to IainS for your Make-lang.in to fix my compiler driver name and installing routines. The routines follow the idea of the Fortran front-end in logic but its much simpler in adding argument i think, once you do the initial check it sets up plenty of variables then you tell it what your runtime library is and if you need the math lib and if its a specific lib or not. Then you transform the arguments into a new array and once that's done you are free to do what you will. I was unsure on where i should be sticking these routines so i just created a file along side the objc driver (driver-utils.c) and put the code in there, i had some problems moving things into gcc.c then i got a little confused in what i was doing since it didn't seem to really like my extern'd variables when the static lib was create it complained about missing variable declarations which the driver was going to declare and i got a bit messed up so i just started clean and hope some of you guys could shed some light on where things might take place better and hopefuly some more feedback on this not sure if its quite what you all had in mind or not... :) --Phil
On Wed, 8 Sep 2010, Philip Herron wrote: > I was unsure on where i should be sticking these routines so i just > created a file along side the objc driver (driver-utils.c) and put the > code in there, i had some problems moving things into gcc.c then i got > a little confused in what i was doing since it didn't seem to really > like my extern'd variables when the static lib was create it > complained about missing variable declarations which the driver was > going to declare and i got a bit messed up so i just started clean and That is far too vague a description to be able to help at all. Shared code for drivers can go in gcc.c with extern declarations in gcc.h. > hope some of you guys could shed some light on where things might take > place better and hopefuly some more feedback on this not sure if its > quite what you all had in mind or not... :) I don't want to see a patch that starts by implementing a new driver in terms of new functions. I want to see a patch that creates new functions and switches existing drivers to using them, with *no change in the behavior of those existing drivers except for changes that you carefully explain and justify in the body of the message*. Also, please keep to the GNU Coding Standards from the start; comments and code following standard formatting, comments on functions detailing the semantics of all parameters and the return type, etc.; otherwise the patch is not effectively reviewable. The "no change in behavior" part means starting small and being very careful with each incremental patch, and thinking carefully and clearly about exactly what the existing code in each driver does so you can understand and explain exactly what is changed as a consequence of moving to unified infrastructure.
On 8 September 2010 12:06, Joseph S. Myers <joseph@codesourcery.com> wrote: > On Wed, 8 Sep 2010, Philip Herron wrote: > >> I was unsure on where i should be sticking these routines so i just >> created a file along side the objc driver (driver-utils.c) and put the >> code in there, i had some problems moving things into gcc.c then i got >> a little confused in what i was doing since it didn't seem to really >> like my extern'd variables when the static lib was create it >> complained about missing variable declarations which the driver was >> going to declare and i got a bit messed up so i just started clean and > > That is far too vague a description to be able to help at all. Shared > code for drivers can go in gcc.c with extern declarations in gcc.h. > >> hope some of you guys could shed some light on where things might take >> place better and hopefuly some more feedback on this not sure if its >> quite what you all had in mind or not... :) > > I don't want to see a patch that starts by implementing a new driver in > terms of new functions. I want to see a patch that creates new functions > and switches existing drivers to using them, with *no change in the > behavior of those existing drivers except for changes that you carefully > explain and justify in the body of the message*. Also, please keep to the > GNU Coding Standards from the start; comments and code following standard > formatting, comments on functions detailing the semantics of all > parameters and the return type, etc.; otherwise the patch is not > effectively reviewable. > > The "no change in behavior" part means starting small and being very > careful with each incremental patch, and thinking carefully and clearly > about exactly what the existing code in each driver does so you can > understand and explain exactly what is changed as a consequence of moving > to unified infrastructure. > I'm not quite sure does that mean you looked at it or not? And does that mean you want a patch that starts implementing/changing the existing compiler drivers to use these new shared functions? I just put these shared functions in a separate file along side the objc front-end for a first pass since i would have liked to get some feedback on the functions them selves since i would be the C/C++ front-end might have more complicated switches etc going on which i may not handle correctly, but i can look into that now, as well as move the shared code into gcc.c. I think i need to fix some things in it anyways i'll try and document the functions etc with some examples etc along side the next patch + fix the formatting.
On Wed, 8 Sep 2010, Philip Herron wrote: > I'm not quite sure does that mean you looked at it or not? And does I began looking at it but rapidly found it wasn't in a state that was worth my while looking at. When sending patches to the lists, the patches should be aimed primarily at a human reader - what's important is making it clear to people what the patch does and why, and the details of the code itself should be secondary; readability is the most important attribute of almost all code that anyone is ever going to have to read or maintain in future. I look first at: * The patch write-up in the body of the message. For changes to complicated code with subtle effects on semantics in existing corner cases, I often find it takes several hours' analysis before starting work on the patch to identify these cases, and a few hours more to write up the details of the patch (having kept notes as I went along of issues found during implementation); there are plenty of examples in my option handling patch series, e.g. <http://gcc.gnu.org/ml/gcc-patches/2010-08/msg01399.html>. * The changes to the documentation. This sort of thing often may not make any changes that would require adjusting user documentation - making the write-up increasingly important. * The new testcases or changes to testcases. The sort of corner cases such a patch affects may be hard to cover in the testsuite - making it increasingly important to detail them in the write-up. * The internal datastructures and interfaces and comments in the patch that provide a high-level overview of the design being used. At the point where I saw new functions without their parameters explained, and comments not formatted in the standard way, I stopped looking at the patch. Only after this point - if everything above is satisfactory - do the details of the code become relevant. Code extracts showing the datastructures and function prototypes with full detailed comments are much more useful than function bodies without explanation. > that mean you want a patch that starts implementing/changing the > existing compiler drivers to use these new shared functions? Exactly. Whenever you find too many corner cases of behavior might be affected, that's a sign that you need smaller incremental patches - possibly having a patch that explicitly fixes a corner case first, before a patch introducing the use of new infrastructure without changing the behavior on that case. The minimal step might be a new interface for appending new or modified options to an array (that automatically resizes) - with no changes at all to what options get appended in what conditions, just the removal of code in the various drivers that tries to work out exactly how many options it will end up adding, in order to allocate space, separately from adding those options.
==== as for factoring: What do you think of these as common functionality to be collected up? 1/ determining that the action is PCH (and ensuring that no link commands are put into the driver line -- these cause collect2 to be invoked which barfs because there is no linking to do). [g++ fails on this as of now] 2/ the repeated sequence of actions for a 'static-xxxx' flag: NOTE: when encountering a "static-xxxx" flag - please do not remove this (or , specifically, ensure that it is replaced when linking) so that targets that use spec substitution (i.e. those that do not support Bstatic/Bdynamic) for static libs can see it. [g++ fails on this too, at present, fortran has it right] 3/ consider that the input files might be supplied from a file list and that should be considered before quitting with a "no files, not prepared to ... " Phil, your spec file works for me on i686-darwin9 (for NeXT and gnu- runtimes) with the modified Makefile fragment below. cheers, Iain Index: gcc/objc/Make-lang.in =================================================================== --- gcc/objc/Make-lang.in (revision 163837) +++ gcc/objc/Make-lang.in (working copy) @@ -39,13 +39,14 @@ # # Define the names for selecting Objective-C in LANGUAGES. objc: cc1obj$(exeext) +.phony: objc -# Tell GNU make to ignore these if they exist. -.PHONY: objc - -# Use maximal warnings for this front end. +# Use strict warnings for this front end. objc-warn = $(STRICT_WARN) +OBJC_INSTALL_NAME := $(shell echo objc|sed '$(program_transform_name)') +OBJC_TARGET_INSTALL_NAME := $(target_noncanonical)-$(shell echo objc| sed '$(program_transform_name)') + # Language-specific object files for Objective C. OBJC_OBJS = objc/objc-lang.o objc/objc-act.o