On 10/18/22 13:01, Ulrich Drepper via Gcc-patches wrote: > On Tue, Oct 18, 2022 at 2:56 AM Jason Merrill <ja...@redhat.com> wrote: > >> That makes sense; the file could say something to that effect. > > > I did that. > > > >> Or the >> CSV file could live in the library directory, or a third directory. > > > The reasoning is that with the file living in the cp subdir we don't have > the situation that the compiler sources depend on something that lives > elsewhere. Unless I miss something, there are no such outside dependencies > and one can build the compiler without the libstdc++ subdir to exist. > > > > >> And >> maybe separate the two generators; it seems like the code shared between them >> is pretty small. >> > > The shared code as-is is small, but so is the whole script. > > Unless someone feels strongly about it I'd prefer not to split the file. > It is far easier to forget to make adequate changes in multiple places. > There can be changes to the format going forward and even yet another > consumer of that information. Keeping more than one (or two, if you count > the CSV file) places is sync is asking for trouble. > > > The CSV file could use a header row documenting the fields (as well as > >> the documentation in the script). >> > > I duplicated the information from the script in the CSV file itself. > > > >>> +# This is the file that depends in the generated header file. >> >> s/in/on/ >> > > Done. > > > There is one additional issue, though, which will likely not hit anyone > here but theoretically is handled in the current version of the build > infrastructure. > > The Makefile contains the provision outside of maintainer mode to rebuild > the generated files by removing the generated file first. This will > require the tools (gperf) to be available but that's deemed OK. > > With this CSV file as the source of the information we have a two-step > derivation of the generated file, though: > > CSV -> .gperf -> .h > > The trick used to avoid the rebuild of the .h file today is to not have a > prerequisite for the rule. This doesn't work for the two-step process. If > the .h and.gperf files are removed there would be no information about the > dependency of the .h generation on the .gperf file. > > This is of course not a new problem in general. GNU make has for a long > time support for order-only prerequisites. With those one can exactly > express what is needed: > > ifeq ($(ENABLE_MAINTAINER_RULES), true) > FOO.h: FOO.gperf > else > FOO.h: | FOO.gperf > endif > gperf ... > > ifeq ($(ENABLE_MAINTAINER_RULES), true) > FOO.gperf: CSV > else > FOO.gperf: | CSV > endif > python ... > > The question here is whether there is a reason not to depend on GNU make > features (and whatever other make implemented this extension). > > > The alternative would be to not expose the .gperf file at all, remove it > from git and remove it after the .h file was created. This gets us back to > a one-step process which the unclean make trick can handle. > > I haven't found an obvious place where tool dependencies are recorded so I > have no idea what the assumptions are. > > Any comment on this? > > I have a patch with the order-only rule ready to send out.
Hi. Can you please fix the coding style of the newly added Python scripts? I documented what's expected from Python scripts here: https://gcc.gnu.org/git/?p=gcc-wwwdocs.git;a=commitdiff;h=ece15e0758bdbb6938942a9f91f6342e66fe443a But unfortunately, the change hasn't reached yet: https://gcc.gnu.org/codingconventions.html The issues I see: gcc/cp/gen-cxxapi-file.py:34:80: E501 line too long (96 > 79 characters) gcc/cp/gen-cxxapi-file.py:35:80: E501 line too long (103 > 79 characters) gcc/cp/gen-cxxapi-file.py:61:1: W293 blank line contains whitespace gcc/cp/gen-cxxapi-file.py:63:1: W293 blank line contains whitespace gcc/cp/gen-cxxapi-file.py:71:11: E225 missing whitespace around operator gcc/cp/gen-cxxapi-file.py:72:19: E225 missing whitespace around operator gcc/cp/gen-cxxapi-file.py:77:27: E225 missing whitespace around operator gcc/cp/gen-cxxapi-file.py:84:31: E225 missing whitespace around operator gcc/cp/gen-cxxapi-file.py:90:1: E302 expected 2 blank lines, found 1 gcc/cp/gen-cxxapi-file.py:132:11: E225 missing whitespace around operator gcc/cp/gen-cxxapi-file.py:133:19: E225 missing whitespace around operator gcc/cp/gen-cxxapi-file.py:137:27: E225 missing whitespace around operator gcc/cp/gen-cxxapi-file.py:141:31: E225 missing whitespace around operator gcc/cp/gen-cxxapi-file.py:144:1: E302 expected 2 blank lines, found 1 gcc/cp/gen-cxxapi-file.py:150:1: E305 expected 2 blank lines after class or function definition, found 1 gcc/cp/gen-cxxapi-file.py:155:1: E302 expected 2 blank lines, found 1 gcc/cp/gen-cxxapi-file.py:159:1: E302 expected 2 blank lines, found 1 gcc/cp/gen-cxxapi-file.py:167:8: E713 test for membership should be 'not in' gcc/cp/gen-cxxapi-file.py:173:80: E501 line too long (91 > 79 characters) gcc/cp/gen-cxxapi-file.py:178:80: E501 line too long (81 > 79 characters) gcc/cp/gen-cxxapi-file.py:181:1: E305 expected 2 blank lines after class or function definition, found 1 Note that we may want to relax the E501 issues to something like 120 chars: see: ./maintainer-scripts/setup.cfg Thanks, Martin
