Skip to content

Fix for issue #9967 - ParameterIdentifierTuple broken for setters #1378

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

Merged
merged 1 commit into from
Jul 23, 2013

Conversation

yazd
Copy link
Contributor

@yazd yazd commented Jun 29, 2013

Fixes http://d.puremagic.com/issues/show_bug.cgi?id=9967

ParameterIdentifierTuple broken for setters

@yazd
Copy link
Contributor Author

yazd commented Jul 5, 2013

A regression due to commit 877e7bf fixing issue http://d.puremagic.com/issues/show_bug.cgi?id=3646 is causing this pull to fail. I will file a regression bug shortly.

dlang/dmd#1102

Edit: filed a regression bug http://d.puremagic.com/issues/show_bug.cgi?id=10548

@9rnsr
Copy link
Contributor

9rnsr commented Jul 6, 2013

Currently there's no way to get parameter identifiers from property functions.
You can see the parameters name by pragma(msg, typeof(&prop)), but cannot take them by alias.

@property void prop(int x) {}
pragma(msg, typeof(&prop));   // prints "(int x)"
alias FP = typeof(&prop);
pragma(msg, FP); // prints "(int)" [!]

Right now this is expected behavior. But at the same time this makes fixing bug 9967 impossible.
So, I'm proposing a compiler enhancement (bugzilla issue and PR)

@yazd
Copy link
Contributor Author

yazd commented Jul 6, 2013

I think that the enhancement is very useful as it serves the purpose without hacks/workarounds. I'm all with it.

However, disregarding this particular issue, I'm not sure whether it is wise to make an alias that is not truly completely aliasing the original symbol. I would say that this is unexpected behaviour.

@9rnsr
Copy link
Contributor

9rnsr commented Jul 7, 2013

I reconsidered about semantics, and then concluded that we can fix 10548 without any problems. Thank you for the good pointing out.

@yazd
Copy link
Contributor Author

yazd commented Jul 7, 2013

Thanks for your great efforts and rapid response.

@yazd
Copy link
Contributor Author

yazd commented Jul 19, 2013

This is all green.
To ease review, the problem with setters was that typeof returned the returned type of the function and so it wouldn't match the first branch. And the second branch which used FunctionTypeOf deliberately silenced the parameter names so as delegates and function pointers are not tied with parameter names.
I changed the first branch to do the check using FunctionTypeOf and then filter out delegates and function pointers to have the intended behaviour.

template Get(size_t i)
{
enum Get = "";
static if (!isFunctionPointer!(func[0]) && !isDelegate!(func[0]))
Copy link
Contributor

Choose a reason for hiding this comment

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

func is 1-length tuple, so you can write like as isFunctionPointer!func and isDelegate!func.

@9rnsr
Copy link
Contributor

9rnsr commented Jul 21, 2013

LGTM, except one nitpick.

@yazd
Copy link
Contributor Author

yazd commented Jul 22, 2013

Updated.

9rnsr added a commit that referenced this pull request Jul 23, 2013
Fix for issue #9967 - ParameterIdentifierTuple broken for setters
@9rnsr 9rnsr merged commit 8c2f4c5 into dlang:master Jul 23, 2013
@9rnsr
Copy link
Contributor

9rnsr commented Jul 23, 2013

Thanks!

@yazd
Copy link
Contributor Author

yazd commented Jul 23, 2013

Thank you!

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.

2 participants