-
-
Notifications
You must be signed in to change notification settings - Fork 740
Alpha renaming, yay #3394
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
Alpha renaming, yay #3394
Conversation
/** | ||
Performs $(LUCKY alpha renaming) of all occurrences of `From` into `To`, in one | ||
or more types `T`. This is an advanced type manipulation necessary e.g. for | ||
replacing the placeholder type `This` in $(XREF variant, Algebraic). |
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.
Example? Returns: ?
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.
Will add Returns:. The unittest underneath is the example.
Issue 9608 is about template introspection. |
In general I would recommend not mixing different type of changes in the same commit. This single commit contains both style changes for comments, replacement of It's easier to review if these are separate commits or even separate pull requests. |
@jacob-carlborg Renaming types was a primary motivator for 9608. Since then there are new traits such as TemplateOf etc. But since there's also dlang/dmd#3515 I'll leave it alone. |
@jacob-carlborg yah, variant.d is kinda difficult to review. Let me make a pass trying to break down stuff. |
7a304c1
to
20ee7b7
Compare
Looks a lot less cluttered now with the ddoc macro change as a separate commit. |
ping |
static if (functionLinkage!fun == "C") | ||
{ | ||
result ~= "extern(C) "; | ||
} |
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.
What about other linkages?
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.
Isn't this more succinctly written as:
result ~= "extern(" ~ functionLinkage!fun ~ ") ";
Or even:
result ~= "extern(%s) ".format(functionLinkage!fun);
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.
nice
ok, linkage added |
ping |
Auto-merge toggled on |
This pull request introduced a regression: |
result ~= "ref "; | ||
} | ||
result ~= (ReplaceType!(X, Y, ReturnType!fun)).stringof; | ||
static if (is(fun == delegate)) |
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.
There is no way to express storage classes such as ref, out, lazy as parts of types.
Which has always been an annoying limitation.
You could alias the type first (alias RT = ReplaceType!(X, Y, ReturnType!fun);) and then use "ref RT"
.
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.
Yes, and I used the method in #3761.
This pull request introduced a regression: |
Now Algebraic can be used for creating self-referenced types. Refer to https://issues.dlang.org/show_bug.cgi?id=9608 for more motivation (probably this ought to close it).