Re: [PATCH] C++ API database

["Martin Liška" <mliska@suse.cz>]

On 10/18/22 13:01, Ulrich Drepper via Gcc-patches wrote:
> On Tue, Oct 18, 2022 at 2:56 AM Jason Merrill <jason@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