On 4/1/22 15:11, Markus Armbruster wrote:
If it can do really serious interprocedural analysis, it _might_ be able
to see through the visitor constructor and know that the "value = *obj"
is not initialized (e.g. "all callers of object_property_set use an
input visitor"). I doubt that honestly, but a man can dream.
I'm wary of arguments based on "a sufficiently smart compiler can"...
Absolutely.
Because it communicates what the caller expects: "I have left this
uninitialized because I expect my "v" argument to be the kind of visitor
that fills it in". It's this argument that gives me the confidence
needed to shut up Coverity's false positives.
Embedding the visitor type in the signature makes it impossible not to
pass it, unlike e.g. an assertion in every getter or setter.
I think we got two kinds of code calling visitor methods:
1. Code for use with one kind of visitor only
We get to pass a literal argument to the additional parameter you
propose.
2. Code for use with arbitrary visitors (such as qapi-visit*.c)
We need to pass v->type, where @v is the existing visitor argument.
Except we can't: struct Visitor and VisitorType are private, defined
in <visitor-impl.h>. Easy enough to work around, but has a distinct
"this design is falling apart" smell, at least to me.
Hmm, maybe that's a feature though. If we only need v->type in .c files
for the generated visit_type_* functions, then it's not a huge deal that
they will have to include <visitor-impl.h>. All callers outside
generated type visitors (which includes for example QMP command
marshaling), instead, would _have_ to pass visitor type constants and
make it clear what direction the visit is going.
Note that "intent explicit in every method call" is sufficient, but not
necessary for "intent is locally explicit, which lets us dismiss false
positives with confidence". We could do "every function that calls
methods". Like checking a precondition. We already have
visit_is_input(). We could have visit_is_output().
The sane way to make output intent explicit is of course passing the
thing by value rather than by reference. To get that, we could generate
even more code. So, if the amount of code we currently generate isn't
disgusting enough, ...
Yeah, that would be ugly. Or, we could generate the same code plus some
static inline wrappers that take a
struct InputVisitor {
Visitor dont_use_me_it_hurts;
}
struct OutputVisitor {
Visitor dont_use_me_it_hurts;
}
That would be zero-cost abstraction at runtime.
Paolo
