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

Helper function to convert tokens to passing_styles. #198

Merged
merged 2 commits into from
Jan 9, 2023
Merged

Helper function to convert tokens to passing_styles. #198

merged 2 commits into from
Jan 9, 2023

Conversation

jbatez
Copy link
Contributor

@jbatez jbatez commented Jan 5, 2023

Also use is_literal() in an appropriate place.

I considered giving expressions starting with passing styles other than out, move, or forward explicit error messages (e.g. f(in x) isn't allowed at this time), but a simple implementation would break expressions like i is (in(10,30)). We could come up with some rules to detect when it looks like you're trying to actually use it as a passing style, but I wanted to ask some questions first.

What are your current thoughts on explicit passing styles at call sites? Are out, move, and forward just the first you've implemented here, or were they explicitly singled out? Personally, as a user, if I know I can use some of them, I'd expect the ability to use all of them (with maybe the exception of in).

inout in-particular seems like important information I'd like to see at call sites. Looking at D0708, you give f2(out myb) or f2(mutable myb) or f2(&myb) as options, but why not f2(inout myb)? That seems like the most most intuitive syntax to me.

copy at call sites could be useful too. You could be saying "hey, no matter what the function signature says, take a copy of this value. I don't want you to change it and/or I don't want to accidentally change it out from under you".

Also, are you thinking of them as keywords or identifiers with special meaning? If they're real keywords, detecting their misuse would be much easier.

Also use is_literal() in an appropriate place.
@hsutter
Copy link
Owner

hsutter commented Jan 9, 2023

@jbatez Thanks Jo, I'll look at this PR now...

Re your questions: I'm currently supporting out, move, and forward arguments only. I'm thinking about copy or inout arguments in the future, but I don't plan to implement them now because I'm still thinking them through, and they probably require more work. Quick examples: For copy we probably want to avoid doing an extra second copy unconditionally when we might be calling a copy-declared parameter. For inout (or mut) we probably want to know that we're calling an inout- or possibly move-declared parameter. But we don't know those things unless we implement more name lookup and possibly overload resolution (neither of which cppfront has to do now) which is a big job I want to defer until later.

@JohelEGP
Copy link
Contributor

For copy we probably want to avoid doing an extra second copy unconditionally when we might be calling a copy-declared parameter.

As I understand it, the function parameter is initialized from the argument, which in this case would be a prvalue. Guaranteed copy elision should mean no extra copy. Clang and GCC seem to agree: https://compiler-explorer.com/z/Gr63qhY44.

@JohelEGP
Copy link
Contributor

Now, copy is necessary to require that an in class type is passed by value.

@JohelEGP
Copy link
Contributor

JohelEGP commented May 1, 2023

For copy we probably want to avoid doing an extra second copy unconditionally when we might be calling a copy-declared parameter.

As I understand it, the function parameter is initialized from the argument, which in this case would be a prvalue. Guaranteed copy elision should mean no extra copy. Clang and GCC seem to agree: https://compiler-explorer.com/z/Gr63qhY44.

An extra copy would happen if the cv unqualified type of the argument and of the (type referenced by the) parameter are different.

Personally, as a user, if I know I can use some of them, I'd expect the ability to use all of them (with maybe the exception of in).

in x would be an useful shorthand for std::as_const(x).
Although in does more when applied to a parameter.

@hsutter
Copy link
Owner

hsutter commented May 21, 2023

in x would be an useful shorthand for std::as_const(x).
Although in does more when applied to a parameter.

This issue is merged, but I don't want to lose this comment -- @JohelEGP could you please open a new issue on expanding the allowed argument qualifiers to include in (with this rationale) and inout (linking to the other issues or comment threads where that has come up, which I can't find right now but you may remember) and maybe copy (to explicitly require a copy at the call site)? Then we have them all in one place... it seems like we're getting a critical mass of examples where most of them should be enabled on the argument side, and with understanding of what they should mean.

Thanks!

@JohelEGP
Copy link
Contributor

Sure. I'll try to gather up those.

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.

3 participants