zyantific / zydis

Fast and lightweight x86/x86-64 disassembler and code generation library
https://zydis.re
MIT License
3.47k stars 438 forks source link

Add [in,out] hints to Doxygen parameter strings #492

Open 440bx opened 8 months ago

440bx commented 8 months ago

Hello,

It would be useful, particularly to a Zydis newcomer, to have the function parameters be annotated using MS SAL conventions, e.g, in, inout, etc

Initially the annotations could be limited to exported functions only since those are the only ones most programmers (Zydis users) will use.

ZehMatt commented 8 months ago

That is not really cross platform so other compilers other than MSVC won't understand this. The documentation is pretty good in my opinion and I find that those annotations make it actually really hard to read.

440bx commented 8 months ago

I can't argue about it not being "cross compiler", it definitely isn't.

That said, I find the SAL gives instant additional information about a parameter (is it in, inout, out, etc) which not even the documentation gives at this time. The parameter disposition is a very nice thing to know.

I'd say that if the C definitions themselves cannot be annotated due to cross platform concerns (which are valid) then it would be useful to see the parameter dispositions stated in the documentation.

Just for the record, I don't find the presence of SAL makes the function definitions any harder or easier to read but, that's a moot point now.

athre0z commented 8 months ago

We use const to annotate pure input parameters. The only thing that SAL would provide on top of this is the ability to distinguish pure output parameters from in/out parameters. Using the MS defines is a complete deal-breaker for the reasons that @ZehMatt outlined previously; if anything we could consider pursuing the documentation based approach. Doxygen supports this via the @param[in,out] syntax. I'm not opposed to this and would accept a PR that adds these for all pointer arguments in the public interface, but I consider it low priority and won't do that work myself.

flobernd commented 8 months ago

Didn't know Doxygen even support that! That would be way better than the MS defines, or even custom ones.

440bx commented 8 months ago

As someone who is currently learning how to use Zydis, having the parameter dispositions documented would be helpful.

I understand it's not a high priority thing but, for a new user, it would be helpful.

If I manage to get really good at using Zydis, I might offer to do it. (please don't hold your breath)

mappzor commented 8 months ago

const + proper parameter naming should be enough to deduce what's input/output.

I find that those annotations make it actually really hard to read.

As someone who goes through Microsoft's header quite a lot I feel the same way about this. It's a lot of unnecessary clutter in most situations. In Zydis we also have quite restrictive maximum line length limitation that would get in the way as well. Doxygen solution seems way better.

440bx commented 8 months ago

Closed due to no interest