-
-
Notifications
You must be signed in to change notification settings - Fork 739
Implementation of fullyQualifiedTypename template that operates on raw t... #863
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
Conversation
* static assert(fullyQualifiedTypename!(const MyStruct[]) == "const(mymodule.MyStruct[])"); | ||
* --- | ||
*/ | ||
template fullyQualifiedTypename(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.
TypeName
, not Typename
.
This seems OK to me, but I won't claim to be an expert on D's metaprogramming. Can anyone else chime in? |
Need to make the auto tester run this one. |
import std.conv; | ||
|
||
enum fullyQualifiedTypeNameImpl = chain!( | ||
fullyQualifiedTypeNameImpl!(typeof(T.init[0]), qualifiers) ~ "["~to!string(T.length)~"]" |
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.
const(int)[3]
and const(int[3])
should be merged to const(int[3])
, because it is compatible with compiler's output.
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.
I double checked this and that is actually how it works now. Simple comparison chart:
const(int)[3]
typeid: const(int[3])
fullyQualifiedTypeName: const(int[3])
const(int[3])
typeid: const(int[3])
fullyQualifiedTypeName: const(int[3])
const(int[])
typeid: const(const(int)[])
fullyQualifiedTypeName: const(int[])
So actually difference is for dynamic slices and as far as I understand const it is typeid output here which is redundant.
Can you merge |
@9rnsr Is it possible? fullyQualifiedName accepts symbols (alias) and fullyQualifiedTypeName - plain types. What could have been common declaration for merged one? Thanks for your extensive comments, I was sure there will be a lot of stupid mistakes. I ll adress them closer to this weekend. |
As far as I see, fullyQualifiedTypeName just works for the built-in types (for the aggregate types, you simply forward it to fullyQualifiedName). Next, template overloads for the alias/type parameter, it works as follows. struct S {}
template T() {}
template Test(alias A) { enum Test = 1; }
template Test(T) { enum Test = 2; }
static assert(Test!S == 1); // aggregate symbol matches alias parameter
static assert(Test!T == 1); // ditto
static assert(Test!string == 2); // built-in types matches type parameter So, I think you can simply add an overloaded template as like: template fullyQualifiedName(alias T) { ... }
template fullyQualifiedName(T) { .. } // your new implementation |
Ah, so not exactly merge into one template but overload with same name? Sure, will do. |
@@ -254,11 +254,20 @@ template fullyQualifiedName(alias T) | |||
|
|||
version(unittest) | |||
{ | |||
struct Outer | |||
// Used for both fullyQualifiedName and fullyQualifiedTypeName unittests | |||
struct QualifiedNameTests |
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.
make private
Rebased, most comments adressed. |
|
||
ref const(Inner[string]) func( ref Inner var1, lazy scope string var2 ); | ||
shared(const(Inner[string])[]) data; | ||
const Inner delegate(double, string) deleg; |
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.
Remove trailing spaces.
Remove trailing redundant spaces on the whole. |
Good work, @Dicebot ! Except function pointer type formatting, looks good to me. |
My humble proposals:
unittest
{
alias fqn = fullyQualifiedName;
enum Inn = "std.traits.QualifiedNameTests.Inner";
with (QualifiedNameTests)
{
static assert(fqn!(string) == "immutable(char)[]");
static assert(fqn!(Inner) == Inn);
static assert(fqn!(typeof(array)) == Inn~"[]");
static assert(fqn!(typeof(sarray)) == Inn~"[16]");
static assert(fqn!(typeof(aarray)) == Inn~"["~Inn~"]");
static assert(fqn!(ReturnType!func) == "const("~Inn~"[immutable(char)[]])");
static assert(fqn!(typeof(func)) == "const("~Inn~"[immutable(char)[]])("~Inn~", immutable(char)[])");
static assert(fqn!(typeof(data)) == "shared(const("~Inn~"[immutable(char)[]])[])");
static assert(fqn!(typeof(deleg)) == "const("~Inn~" delegate(double, immutable(char)[]))");
static assert(fqn!(typeof(funcPtr)) == Inn~"(double, immutable(char)[])*");
}
} |
else static if (isSomeFunction!T) | ||
{ | ||
static if (isDelegate!T) | ||
enum format_str = "%s delegate(%s)"; |
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.
delegate and function may have some attributes. Furthermore, delegate may have qualifier as the function type.
For example:
shared(immutable(Inner) delegate(double, string) const shared nothrow) dg;
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.
@shoo I have experimented a bit with this and explored spec more and still not sure what to do about this. writeln(typeid(typeof(dg))) just ignores almost anything so is hardly a reference and I wonder if shared and nothrow are part of a type, as those are function attributes and generally belong to symbol.
Also current dmd prohibits me from declaring delegate variable of such type: "Error: const/immutable/shared/inout attributes are only valid for non-static member functions", sounds like a bug?
tl;dr: implementing is not a problem but I need someone to guide me here on theoretical language-related side.
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 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.
With git head, directly declaring qualified delegate type (e.g. void delegate() const
) is allowed. It is consistent with a member function void foo() const
.
So, adding qualifier to delegate type is necessary.
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.
And, TypeInfo.toString() sometimes returns a bad representation of the type. The lack of qualifier for delegate types is one of them.
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.
Hm, it seems that type qualifiers and storage classes / function attributes are frequently mixed in dlang.org docs.
Am I understanding it right that in @shoo example const/shared are type qualifiers and should be stringified and nothrow is storage class? Using http://dlang.org/declaration.html as a reference currently.
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.
Using http://dlang.org/declaration.html as a reference currently.
Oh, I found a documentation bug in website.
BasicType2:
...
delegate Parameters FunctionAttributesopt
FunctionAttributes:
FunctionAttribute
FunctionAttribute FunctionAttributes
FunctionAttribute:
nothrow
pure
Property
Property:
@ PropertyIdentifier
In current spec, delegate type declaration cannot have postfix qualifiers.
I think BasicType2
should be:
BasicType2:
...
delegate Parameters MemberFunctionAttributeopt
MemberFunctionAttributes:
MemberFunctionAttribute
MemberFunctionAttribute MemberFunctionAttributes
MemberFunctionAttribute:
const
immutable
inout
shared
FunctionAttribute
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.
OK, posted pull request.
dlang/dlang.org#218
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 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.
No. Delegate type qualifier cannot test directly. You should extract function type from it, then test const
.
void main() {
alias void delegate() const DG;
alias const(void delegate() const) CDG;
static assert(!is( DG == const));
static assert( is(CDG == const));
static if (is(DG F == delegate)) // extract function type from DG
{
// F is a "const function type"
pragma(msg, F); // const void()
static assert(is(F == const)); // pass
}
}
Sorry guys, i am currently in a hospital after apoplexy, forced to pause this. Will response as soon as be back on my feet. |
Take care of yourself. |
Rebased, squashed, implemented @9rnsr proposals. |
static assert(fqn!(typeof(func)) == xformat("const(%s[string])(%s, string)", inner_name, inner_name)); | ||
static assert(fqn!(typeof(data)) == xformat("shared(const(%s[string])[])", inner_name)); | ||
static assert(fqn!(typeof(deleg)) == xformat("const(%s delegate(double, string))", inner_name)); | ||
static assert(fqn!(typeof(funcPtr)) == xformat("%s function(double, string)", inner_name)); |
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.
Two weeks ago, the implementation of std.string.format
is changed to xformat
's. (by merging #939). So currently, you can just use format
in here.
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.
Finally! :)
I am recovering more slowly than I hoped and that took some time but feels like all "interesting" cases for function type stringification were addressed. May be missed some corner cases, unit test proposals are welcome. But so far it is done, awaiting for next comment portion :) |
Noticed test suite issues, made a quick fix. I am unsure regarding @Property applicability here though - those utility functions do not really look like ones, but stuff staticMap assumes its first parameter is either plain template or @Property function. Are those fine or shall I change staticMap to accept template functions with zero arguments? |
Excepting just one comment, looks good to me. Nice job, @Dicebot !
It is an enhancement for |
It is done, but blocked by of ParameterStorageClassTuple, which seems unaware of inout. Made relevant comment here: |
…w types contrary to fullyQualifiedName which is tied to aliases to symbols
1) Added formatting for: * Attributes * Parameter storage classes * Variadic arguments * Function type qualifiers for delegates * Function linkage 2) Unit tests were re-arranged and improved a bit 3) xformat -> format
…xclusive qualifier combination in output
…t stack anymore, turned one of inner templates to CTFE function
@9rnsr Thanks for fast fix. Another problem: I have found that any addition of qualifiers break compatibility between two fullyQualifiedName templates for user-defined symbols. Consider this:
I can't imagine any workaround for this now. Unit test is failing currently because of this problem, I pushed last changes anyway in case any additional comments may arise. |
You can make one entry template with tuple template parameter.
|
…to match new hierarchy
@9rnsr Thanks. Your sample was not generic enough but provided me with insight of more elegant solution via is(T) :) |
import std.conv; | ||
|
||
enum fullyQualifiedNameImplForTypes = chain!( | ||
format("%s[%s]", fullyQualifiedNameImplForTypes!(typeof(T.init[0]), qualifiers), to!string(T.length)) |
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.
Calling to!string
is not need anymore.
Reviewed and commented. |
All comments have been addressed. |
Looks good to me! Let's wait the auto-tester result. |
OK, now the auto-tester result is all green. |
Implementation of fullyQualifiedTypename template that operates on raw t...
Merged. Thanks for your long work, @Dicebot ! |
else static if (variadic == Variadic.typesafe) | ||
enum variadicStr = " ..."; | ||
else | ||
static assert(0, "New variadic style has been added, please update fullyQualifiedName implementation"); |
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.
Why not simply use a final switch here?
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.
An artifact of old template-only non-CTFE approach that did not catch my attention in time.
...ypes contrary to fullyQualifiedName which is tied to aliases to symbols.
Original need in such function came from vibe.d rest module I have been working on lately to improve reliability of automatic REST client generation. I.e. module A needs to generate a class that implements methods of interface defined in module b. Method may return symbol typed with nested hierarchy and that is it.
After some tweaking template looked generic enough so that it may be a fit to phobos so here am I :)
Destroy (c)