On 3/15/22 15:50, Kevin Wolf wrote:
I'm not sure what the C++ lock guards offer that our current lock guards
don't? Passing down lock guards makes sense to me, but why can't you do
that with QemuLockable?

Passing a QemuLockable alone doesn't ensure that the lock has been taken. I guess we could build a QemuLockGuard on top that ensures that.

(Hm, or can the C++ version somehow check at
compile time that it's the _right_ lock that is held rather than just
any lock? It didn't look like it at the first sight.)

It's not related to lock guards but clang++ has thread-safety analysis that checks for the right lock. It is in theory there for C as well but it's mostly a joke. Ironically, the only good use that I've seen of it is Marc-André's never committed work to check coroutine_fn markers at compile-time[1].

But I do see the benefit of a compiler checked CoroutineFn<> return type
compared to the coroutine_fn markers we have today. On the other hand...

Also, once C++ is available people will start submitting C++ patches
simply because they are more comfortable with C++ (especially
one-time/infrequent contributors).

...using C++ in coroutine code means that all of the block layer would
suddenly become C++ and would be most affected by this effect. I'm not
sure if that's something I would like to see, at least until there is a
clear tree-wide policy (that preferably limits it to a subset that I
understand).

You are the maintainer of the block layer, so you would have the biggest impact and obviously have input in such a policy. I obviously don't have a policy ready[2], but I do have some goals:

1) whenever C++ is used to build an API for use in C++ source files:

        Those source files and their debugging experience should look
        like C, and not any more "foreign" than       the "QEMU C" dialect
        that we already have.

        Rationale: we might have funky code in headers, but
        "QEMU_WITH_LOCK_GUARD(x) { }" is understandable; the same would
        hold for "CoroutineFn<void>", "co_await fn();" or if hwaddr
        internally used operator overloading.

2) whenever C and C++ provide two different implementations of an API, for use in C and C++ source files respectively:

        If the same API is implemented in different ways for C and C++,
        the semantics should be equivalent, and covered by both
        C and C++ testcases.

        Rationale: this should be a rare occasion, but there are
        features such as _Generic that are C only, so it should be
        catered for; QemuLockable came out already in this proof of
        concept.  The implementation is completely different for C
        and C++, but it works the same (as proved to some extent
        by testcases).

        Looking again at the hwaddr example, it would be okay to forbid
        "hwaddr - hwaddr" subtraction in C++ code.  But it would not be
        okay to have it return a signed integer in C++, while it
        returns an unsigned integer in C.  Could there be a C++-only
        method "hwaddr.diff(hwaddr)" that does return a signed integer?
        That would have to be discussed, right now my opinion is
        "probably not" but see the next point too.

3) whenever C++ features (templates, classes, etc.) are used in .cc files:

        The features should have a clear benefit over equivalent
        C-only code (e.g. using the preprocessor instead of templates)
        and if applicable they should publish a C API[3] that can be
        consumed outside a particular subsystem.

        If in doubt, err on the side of moderation, i.e. less C++ and
        fewer C++ features.

        Example: I can imagine that in some cases the semantic
        replacement abilities provided by templates will for example
        improve compile-time checking and reduce code duplication
        compared to gcc.  However, QEMU already has a wide range of
        data structures and auxiliary APIs for C, and those should be
        used for C++ code to avoid splitting the code base.

        In turn, this should limit the usage of other C++ features.
        For example, constructors (as opposed to just being able to
        zero-initialize memory) can be more of a pain than a benefit
        if you can't use "new", "delete" or "std::vector<>".  Likewise,
        it's unlikely that we'll have reasons to use anonymous
        namespaces instead of "static", because that's mostly useful
        for classes defined in .cc files and their member functions[5].
        And VMState will prevent the usage of "private:", thus lowering
        the benefit from methods.

Would this kind of formulation be enough? Maybe, maybe not, but either way I don't really believe in prohibiting features by policy. The QEMU community and especially its maintainers won't change overnight if C++ is allowed in the codebase; but they should be allowed to evolve their opinions as their experience grows. If some day the block layer maintainers want to extend coroutines to variable arguments rather than a single void *, they shouldn't be forbidden from doing so just because (I guess) it entails using variadic templates. They should show that there is a benefit, though.

In any case, Stefan's suggestion to look into what other projects did, especially GCC/GDB is a good one:

- https://gcc.gnu.org/wiki/CppConventions probably is a good start[4], but we would probably cut on the "explicitly permitted" parts. https://gcc.gnu.org/codingrationale.html also focuses a lot on things that QEMU would likely not use.

- the discussion at https://gcc.gnu.org/legacy-ml/gcc/2010-05/msg00705.html has some quotes that capture interesting the point. It's possible that I have subconsciously introduced it in my two messages on the topic:

        The goal is not to reimplement GCC from the ground up using
        modern OO/C++ techniques.  The goal is simply to permit
        ourselves to use C++ features where appropriate in the codebase.
        ---
        The right subset is dependent on context.  We're not in a
        safety-critical environment, we have a large existing C
        codebase, and we have a developer team made up of experienced
        C programmers, not all of whom are used to programming in C++.
        So, we need to take those factors into account.
        ---
        Switching to C++ should never be excuse to bring more more
        brittle codes or more obscurities.  Rather, it should be
        opportunity to write simpler and better code.

Also of interest is the replies to less enthusiastic developers starting at https://gcc.gnu.org/legacy-ml/gcc/2010-05/msg00744.html at https://gcc.gnu.org/legacy-ml/gcc/2010-06/msg00174.html.

- Richard might have some experience here, and his insight is valuable.

Paolo

[1] https://lists.gnu.org/archive/html/qemu-devel/2017-07/msg00793.html

[2] in fact, even if we decided to go with C++ coroutines, it would take quite some time to split the coroutine/non-coroutine cases, which is a prerequisite for that, so we'd have a lot of time to develop a policy. Once the policy is there, there will be even more time before a C++-using patch that does something useful gets submitted and approved.

[3] or at least C-like, for example qemu_coroutine_create technically is a C++-only API because it takes a CoroutineFn<void>(*)(void *) argument. But it *looks* like C.

[4] lol, there's even a remark fom me in there, but I stopped working on GCC about at the time that the document was being written.

[5] https://stackoverflow.com/questions/154469/unnamed-anonymous-namespaces-vs-static-functions

Reply via email to