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

exclude const generic arguments from generic_types #627

Merged
merged 1 commit into from
May 30, 2023

Conversation

BKSalman
Copy link
Contributor

resolves #626

nvm got help from @aumetra and solved it :)

@BKSalman BKSalman mentioned this pull request May 26, 2023
2 tasks
Copy link
Owner

@juhaku juhaku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering whether it actually should work with const as well if so, then the fix would be more evolved. Yet at the moment it is not working with consts so I guess it is just fine to ignore them though. But most likely users with a struct having generic const variable it would still fail but with a different error than now.

However that can be another PR later on. I'll create an issue to the kanban board to track this if I ever get to test the cons support.

@juhaku juhaku merged commit f8c6d07 into juhaku:master May 30, 2023
@BKSalman
Copy link
Contributor Author

Just wondering whether it actually should work with const as well if so, then the fix would be more evolved. Yet at the moment it is not working with consts so I guess it is just fine to ignore them though. But most likely users with a struct having generic const variable it would still fail but with a different error than now.

However that can be another PR later on. I'll create an issue to the kanban board to track this if I ever get to test the cons support.

sounds good :)

I was actually trying to somehow change the code to make it also support consts (not just filter it out), but I didn't even know what the idea of it would be, or where it can be used in the generated docs

@aumetra
Copy link

aumetra commented May 30, 2023

or where it can be used in the generated docs

Yeah, whatever can be done with const-generics (at the moment) seems somewhat irrelevant to OpenAPI docs (I could also just not be aware of use-cases).

But most likely users with a struct having generic const variable it would still fail but with a different error than now.

Do you mean with a constant associated type? Something like Foo<BAR = 2>? These will receive the same errors presumably, since they fall under the branch GenericArgument::AssocConst, which is covered by utoipa to fail with the same error.

If you mean something like Foo<2>, it would fall under the same branch (GenericArgument::Const) that was patched with this PR and would be ignored.

@juhaku
Copy link
Owner

juhaku commented May 30, 2023

Do you mean with a constant associated type? Something like Foo<BAR = 2>? These will receive the same errors presumably, since they fall under the branch GenericArgument::AssocConst, which is covered by utoipa to fail with the same error.

If you mean something like Foo<2>, it would fall under the same branch (GenericArgument::Const) that was patched with this PR and would be ignored.

Either of those, frankly I haven't looked in to the details how or what could be done with a const generic type. For utoipa OpenAPI docs it indeed does not make much sense and even a generic ToSchema type should be used with #[aliases] that tell the actual types of the generic arg anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

procmacro incompatible with const generics
3 participants