2010/8/25 Basile Starynkevitch <bas...@starynkevitch.net>: > I am not at all sure we would be able to sent them separately, in such a > way that gengtype still works correctly with each patch applied one by > one.
It's hard to tell without looking at your patch, but your cleanups do sound independent. Also you can define the order of the patches that suits you best, for example, start with get_output_file_with_visibility (that sounds like a really local change, although I could be wrong), and then build on that. Maybe I could have some suggestions, but then I need a readable patch (maybe you can just re-diff it with whitespace ignored, that would do for reading I guess). > And our previous gengtype patch (only adding program arguments to > prepare the persistent state work) was very small but have not being > reviewed till acceptance. So I am not sure that small patches are > enough! Sadly independence & smallness of the patch is more or less necessary but not necessarily sufficient condition for the review. I am afraid you must accept this. > Should we try to improve GTY documentation? Apparently ptr_alias is not > documented in gty.texi. By all means, please do. > Besides, testing any gengtype change is very expensive for us (& > probably for others), because we have to regenerate all the gt*.[ch] > generated files & rebuild the entire compiler at each test. Even with > the help of ccache, that takes a lot of time. Can you elaborate? I think it's a matter of "make s-gtype" or from a clean tree: configure, make all-build-libiberty configure-stage1-gcc && cd gcc && make s-gtype and then you can compare the output with a known good copy, in fact, as far as GCC patch testing is concerned, gengtype probably has it easiest if the output does not change. Save the rebuildings, bootstraps & testsuites for the final test before submission. > A question for gengtype experts: what exactly determines the set of > generated gt*.[ch] files? We have the fuzzy impression that even some > ada specific files are generated while ada was not configured but we may > be wrong. make s-gtype-input is your friend. Also look at GTFILES in Makefile. Basically the output set is determined by the input set which in turn is the GTFILES variable. The frontends are pulled in by @all_gtfiles@ part of it which is defined by looping through all the language frontend directories in gcc/configure.ac and I didn't really figure out if it was intended to be affected by --enable-languages or not. However, --enable-languages=c resulted in gtyp-input.list containing all the frontends, so that's how the things are right now. There is also GTFILES_LANG_H and ALL_GTFILES_H in Makefile.in which you can ignore for your purposes. > Laurynas, Ian, others, could you please explain in a few words how you > believe the various languages (cp, ada, objc, ...) work inside gengtype? > Obviously, the bitmask is representing a set of languages, but neither > Jérémie nor me Basile grasped all the details... I am not sure to > understand what exact parts of gengtype is using the [cp] & [ada] ... > tags in gtype-input.list, These tags are recognized by read_input_line which returns true upon encountering one to read_input_list, look at the lines 425-440. Upon reading the tag gengtype knows what language assign the file names that follow it until the next [language name] tag. IMHO this is quite vital for correct language bitmap handling or what will replace it. > and I am not sure to grasp what > TYPE_LANG_STRUCT & TYPE_PARAM_STRUCT are exactly for, even if I do > guess a bit. TYPE_LANG_STRUCT is for structs that have a definition per frontend, struct lang_decl and lang_type are the canonical examples, which are defined by all the rontends and handled naively would result in duplicate name clashes. TYPE_PARAM_STRUCT is orthogonal to the frontends, it's for parameterized structs (GTY options param and param_is), since parameterized structs need separate gengtype representations for each parameterization. > Does the set of configure-d languages matter to gengtype? > If yes, how? I guess no. -- Laurynas
