-
Notifications
You must be signed in to change notification settings - Fork 4
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
auto return type for accessors #13
Conversation
Codecov Report
@@ Coverage Diff @@
## master #13 +/- ##
==========================================
+ Coverage 68.29% 71.42% +3.13%
==========================================
Files 1 2 +1
Lines 123 126 +3
==========================================
+ Hits 84 90 +6
+ Misses 39 36 -3
Continue to review full report at Codecov.
|
test/main.d
Outdated
} | ||
|
||
// Issue #11: https://github.com/funkwerk/accessors/issues/11 | ||
pure nothrow @safe @nogc unittest |
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.
Attributes should be listed in alphabetical ordering, e.g. const @nogc nothrow pure @safe
(the ordering should ignore the leading @
).
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.
fixed.
src/accessors.d
Outdated
"{ inout(%s)[] result = null; result ~= this.%s; return result; }", | ||
visibility, valueType, accessorName, valueType, name); | ||
return format("%s final @property auto %s() inout {" | ||
~ "import std.traits;" |
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.
Local imports should specify the symbols being imported to avoid hiding local symbols.
src/accessors.d
Outdated
return format("%s final @property auto %s() inout {" | ||
~ "import std.traits;" | ||
~ "inout(ForeachType!(typeof(this.%s)))[] result = null;" | ||
~ "return result ~ this.%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.
Is there a more explicit way to copy the array?
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'm aware only of .dup
.
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.
try .dup
; otherwise, try [] ~ ...
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.
.dup:
src/accessors.d-mixin-666-mixin-668(668,58): Error: template object.dup cannot deduce function from argument types !()(inout(const(X)[])), candidates are:
/usr/include/dmd/druntime/import/object.d(1943,6): object.dup(T : V[K], K, V)(T aa)
/usr/include/dmd/druntime/import/object.d(1979,6): object.dup(T : V[K], K, V)(T* aa)
/usr/include/dmd/druntime/import/object.d(3737,16): object.dup(T)(T[] a) if (!is(const(T) : T))
/usr/include/dmd/druntime/import/object.d(3753,15): object.dup(T)(const(T)[] a) if (is(const(T) : T))
src/accessors.d(677,9): Error: static assert is(typeof(y) == const(X)[]) is false
src/accessors.d
Outdated
return format("%s final @property auto %s() inout {" | ||
~ "import std.traits;" | ||
~ "inout(ForeachType!(typeof(this.%s)))[] result = null;" | ||
~ "return result ~ this.%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.
try .dup
; otherwise, try [] ~ ...
ping @duselbaer |
Doesn't solve it for @Write |
This is a suggestion to fix #11.
Instead of using a string as the accessor return type, it may be better to use
auto
, so the compiler can infer the return type based on the type of the appropriate class/struct member.Marking the property as const/inout seems to be enough for the compiler to infer the return type with the correct mutability.