Eric Blake <ebl...@redhat.com> writes: > On 02/16/2016 10:35 AM, Daniel P. Berrange wrote: >> In my LUKS encryption series, I have a discriminated union for >> storing options for different encryption formats. See qapi/crypto.json >> in this file: >> >> https://lists.gnu.org/archive/html/qemu-devel/2016-02/msg03187.html >> >> You'll notice I have the 'prefix' line for the enum commented out. When >> I uncomment this, I discovered that the discriminated union visitor does >> not deal with prefixes. >> > >> Apply that and then try to build and it'll fail with: >> >> qapi-visit.c: In function ‘visit_type_QDemo’: >> qapi-visit.c:7596:10: error: ‘Q_DEMO_TYPE_FOO’ undeclared (first use in this >> function) >> case Q_DEMO_TYPE_FOO: >> ^ >> qapi-visit.c:7596:10: note: each undeclared identifier is reported only once >> for each function it appears in >> qapi-visit.c:7599:10: error: ‘Q_DEMO_TYPE_BAR’ undeclared (first use in this >> function) >> case Q_DEMO_TYPE_BAR: >> ^ >> >> The issue is that we used the 'QDEMO_TYPE' custom prefix for generating the >> enum, but we didn't use the prefix in the union visitor. > > Should be a quick fix, if we want to keep prefixes. I'll go ahead and > post it, at least for discussion purposes. > >> >> I know we had had previous discussions with Markus strongly wanting to kill >> off the support for enum prefixes. So before I waste time trying to fix >> this union visitor code to handle prefixes, I figure we should decide if >> we actually want to fix it, or go with Markus' plan to kill custom prefixes >> on enums. > > I'm still on the fence which way to go; we've definitely improved the > code base so that inadvertent collisions due to odd heuristics are less > likely to occur, but every special case we have to carry (custom prefix > being one of them) results in more code to maintain and test.
Yes. As so many things, QAPI started out relatively simple & stupid, if saddled with a certain amount of accidental complexity due to hasty design and implementation. Then features got bolted on left and right. That's normal; useful software expands to fill available space. We've been working hard on reducing the accidental complexity. I believe we should also try to control the complexity due to features. Features need to earn their keep. >> Per previous discussions, I think the ability to have custom prefixes is >> quite desirable, to get more natural enum constant names. At the end of >> the day though, the default enum naming is far from the worst bit of >> QEMU, so I'm not ultimately too bothered either way. We either make >> custom enum prefixes work everything they need to, or remove them. Keeping an unloved feature half-broken while we deliberate whether to keep it is the worst of all worlds. Since the fix is a one-liner (plus tests), let's just apply it to unblock Dan as soon as possible.
