rishka_virtual_machine and others could benefit from a bit more C++ seasoning, IMO.

[robertlipe]

rishka/src/rishka_vm.cpp

Line 968 in 6a92ab6

for(uint8_t i = 0; i < 32; i++)

If rishka_virtual_machine were a class had a constructor or init style class that initialized it to a guaranteed sane state, you might be able to save yourself some maintenance/synchronization grief.

The zeroing of registers[], for example, could probably reduce to a memset, probably inlined. Then if you were targeting, say, RV32E that had half as many RV registers, you wouldn't need to special-case that '32', the ctor would just set the appropriate registers.

This is very nice code, but don't let your structures be afraid to blossome out into classes with all the convenience and performance that goes with that. In fact, elsewhere in the code, it shows that you're not afraid of C++..., so don't be afraid to let your C++-flag fly freely!

I'd default running to false, zero-initialize registers[] (and provide accessors for things like debuggers or viewers...), ensure that stream*'s life cycle is always appropriate, etc.

You don't have to include something from every chapter of https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines in order just to prove you're worthy (you've produced a meaningful base of code - you're clearly worthy!) but don't be afraid to let the C++ bits poke through. and take advantage of th safety and performance that's available to you. Let rishka_syscall (and the rest of instructions*) be class enums for better type safety and so your own debugging code can offer more introspective views, etc.

It's clean and nifty code, but it looks like C code without the type safety and promises allowed by "Modern C++".

(And if you're tired of my opinionated bullshit, feel free to justcan all of these suggestions... :-) )

[nthnn]

Hey, so this took a while. I think this would do it. I used C++ class construct instead of that salted C you previously mentioned on the other post. I was also able to hide some utility helper functions inside the RishkaVM class.

Moreover, C++ template functions T getPointerParam<T>() and T getParam<T>() are declared in src/rishka_vm.h. It looks much better and readable as used in src/rishka_syscalls.cpp in fetching parameters and pointer parameters on each system call handlers.

I'm still working on making the src/rishka_syscalls.h much more C++ flavored source files.

you've produced a meaningful base of code - you're clearly worthy!

Also, thanks for this. With this, I'll make sure to make this base of code much more meaningful.

... if you're tired of my opinionated bullshit

Well, as for me, I actually believe these are good opinions. Suggestions like this improved the codebase a lot and enabled me to re-evaluate my coding styles. Thanks again!

[robertlipe]

Looks much better, IMO.

Can the syscalls be members of RishkaVM? Then

int rishka_syscall_io_available(RishkaVM* vm) {
return vm->getStream()->available();
}

becomes

int rishka_syscall_io_available() {
return getStream()->available();
}

...because you have the implicit |this|.

Maybe that doesn't play out. I'm half asleep. :-)

[nthnn]

I've already made several changes to src/rishka_syscalls.h and src/rishka_syscalls.cpp. Although, I admit it might not be most suitable construct.