-
Notifications
You must be signed in to change notification settings - Fork 21
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
Added support for requires() constraints on methods that are not directly templated #86
Conversation
pytest output:
|
struct Payload | ||
{ | ||
constexpr Payload(T v) | ||
requires(std::is_pod_v<T>) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The requires statement gets completely discarded here, whereas in other contexts the requires is retained as a raw Value? Even though we aren't doing anything with the data at the moment, this is inconsistent and could be a problem if we ever decide to do something with the data.
It seems that the best way to deal with this would be to move raw_requires_post
into Function instead of being in TemplateDecl? Even though that would be a breaking change, since this is mostly unparsed anyways I don't think it's that big of a deal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not entirely discarded, right? It ends up as the raw_requires_post
on a TemplateDecl
with no template args, which was the sanest way I could think to represent it at a glance.
That said, it probably does make sense to move raw_requires_post
to Function
. I actually looked at doing that first before I realized what the parsing issue was. I've only taken a cursory dive into the parser but it seemed initially like it might be a much bigger/far-reaching change to parsing.
Now that I've spent more time in there (working on #87), It might be doable right around where trailing return types are parsed in _parse_fn_end
and _parse_method_end
. Would it be preferable if I put a peek in those spots looking for requires
, parse it, and store the result on Function
?
If so, just let me know, and I'll take a run at it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It isn't discarded immediately, but eventually the TemplateDecl isn't used so it is discarded. If it wasn't discarded, then the requires(std::is_pod_v<T>)
would be in the testcase's ParsedData somewhere, and it isn't.
When I was looking at this I noticed a simpler way to do this, so I'm going to close this in favor of doing it that way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... which was exactly what you suggested, ha
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh sweet, thank you!
Closed in favor of #89, please take a look and let me know if that works for you |
Currently,
cxxheaderparser
can only parserequires()
constraints on methods which are also templated. However, under C++20, requires statements can also exist on un-templated methods in a templated class as well, this is usually used to replacestd::enable_if<>
function toggling.