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
