John Snow <js...@redhat.com> writes: > On 12/17/20 6:09 AM, Markus Armbruster wrote: >> John Snow <js...@redhat.com> writes: >> >>> On 12/16/20 5:42 AM, Markus Armbruster wrote: >>>> John Snow <js...@redhat.com> writes: >>>> >>>>> Instead of using None as the built-in module filename, use an empty >>>>> string instead. >>>> >>>> PATCH 05's changes the module name of the special system module for >>>> built-in stuff from None to './builtin'. The other system modules are >>>> named like './FOO': './init' and './emit' currently. >>>> >>>> This one changes the module *filename* from None to "", and not just for >>>> the builtin module, but for *all* system modules. Your commit message >>>> claims only "the built-in module", which is not true as far as I can >>>> tell. >>>> >>> >>> Is this true? ... "./init" and "./emit" are defined only in the >>> generators, outside of the schema entirely. They don't even have >>> "QAPISchemaModule" objects associated with them. >>> >>> I changed: >>> >>> self._make_module(None) # built-ins >>> >>> >>> to >>> >>> self._make_module(QAPISourceInfo.builtin().fname) # built-ins >>> >>> >>> >>> that should be precisely only "the" built-in module, shouldn't it? the >>> other "system" modules don't even pass through _make_module. >> >> You're right. >> >> The schema IR has only user-defined modules and the built-ins module. >> Each module has a name. We use the source file name for the >> user-defined modules. The built-ins module has none, so we use None. >> >> The QAPISchemaModularCVisitor generates "modular" output. It has >> per-module data, keyed by module name. It supports user-defined and >> system modules. We set them up as follows. The user-defined modules >> are exactly the schema IR's (same name). The system modules are the >> schema IR's built-ins module (same name) plus whatever the user of >> QAPISchemaModularCVisitor adds (convention: name starts with ./, so it >> can't clash). >> >> Why let generators add system modules that don't exist in the schema IR? >> It's a reasonably clean way to generate stuff that doesn't fit elsewhere >> into separate .[ch] files. >> >> PATCH 05 changes the name of the built-ins module from None to >> './builtins' in the QAPISchemaModularCVisitor only. Correct? >> > > That's right. That was a useful change all by itself, and winds up being > useful for the genc/genh typing. > >> This patch changes the name of the built-ins module from None to "" in >> the schema IR only. Correct? >> > > As far as I know, yes. ("Schema IR -- Internal Registry?")
Internal representation (the stuff in schema.py). Compiler jargon, sorry. >>>> Should we use the opportunity to make the filename match the module >>>> name? >>>> >>> >>> If that's something you want to have happen, it can be done, yes. >> >> Having different "module names" for the built-ins module in different >> places could be confusing. >> > > Yes, but we technically didn't use the generator-only names in the > Schema before, so I didn't want to assume we'd want them here. We can't have them there, because the things they name don't exist there. What we can have is a consistent naming convention that leads to same things having the same names in both places. >> The issue appears in PATCH 05. I'm fine with the two module names >> diverging temporarily in this series. I'd prefer them to be the same at >> the end. >> > > OK, no problem. I'll try to make this nicer and unify things just a > pinch more. > > --js