Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
440bx opened this issue Mar 8, 2024 · 7 comments
Open

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

440bx opened this issue Mar 8, 2024 · 7 comments
Labels
A-doc Area: Documentation C-enhancement Category: Enhancement of existing features P-low Priority: Low

Comments

@440bx
Copy link

440bx commented Mar 8, 2024

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.

@flobernd flobernd added the C-enhancement Category: Enhancement of existing features label Mar 8, 2024
@ZehMatt
Copy link
Contributor

ZehMatt commented Mar 8, 2024

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
Copy link
Author

440bx commented Mar 8, 2024

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
Copy link
Member

athre0z commented Mar 8, 2024

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.

@athre0z athre0z added A-doc Area: Documentation P-low Priority: Low labels Mar 8, 2024
@flobernd
Copy link
Member

flobernd commented Mar 8, 2024

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

@440bx
Copy link
Author

440bx commented Mar 8, 2024

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
Copy link
Contributor

mappzor commented Mar 8, 2024

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
Copy link
Author

440bx commented Mar 9, 2024

Closed due to no interest

@440bx 440bx closed this as completed Mar 9, 2024
@flobernd flobernd reopened this Mar 11, 2024
@flobernd flobernd changed the title Add Source code Annotation Language (SAL) to function parameters Add [in,out] hints to Doxygen parameter strings Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-doc Area: Documentation C-enhancement Category: Enhancement of existing features P-low Priority: Low
Projects
None yet
Development

No branches or pull requests

5 participants