Paolo Bonzini <pbonz...@redhat.com> writes:
> 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.
I quoted the generated qapi-visit*.c as an example. There may
handwritten instances, too.
>> 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.
Looks worth exploring!
