two fixups for the arrayop implementation#1903
Conversation
MartinNowak
commented
Aug 17, 2017
MartinNowak
commented
- add support for mixed vector types
- fix argument index in initScalarVecs
|
Thanks for your pull request, @MartinNowak! Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. |
MartinNowak
force-pushed
the
fixup_arrayops
branch
from
2ed3040 to
cdc09da
Compare
| } | ||
| else static if (isBinaryOp(arg)) | ||
| { | ||
| stack[$ - 2] = "(cast(T)(" ~ stack[$ - 2] ~ " " ~ arg ~ " " ~ stack[$ - 1] ~ "))"; |
There was a problem hiding this comment.
The existing arrayop behavior only casts the whole RHS before assignment, but not intermediate expressions. This is fixed here.
MartinNowak
force-pushed
the
fixup_arrayops
branch
from
cdc09da to
d346497
Compare
src/core/internal/arrayop.d
Outdated
| { | ||
| enum not(Args...) = !tmlp!Args; | ||
| } | ||
| /// find index for which pred!haystack[index] is true or return -1 |
There was a problem hiding this comment.
Please use Ddoc style comments, with Params: and Returns: sections. It's not just for readability, it's so tools can be used on the documentation.
There was a problem hiding this comment.
Done.
Note that this already was a ddoc comment and also it's just a private internal helper template with only 5 lines of implementation.
| } | ||
| } | ||
|
|
||
| // std.meta.staticMap |
There was a problem hiding this comment.
Looking at the documentation for std.meta.staticMap, all it says is:
Evaluates to $(D AliasSeq!(F!(T[0]), F!(T[1]), ..., F!(T[$ - 1]))).
but that is not what this template body does.
There was a problem hiding this comment.
It's exactly what this template does, in fact I've copied the implementation.
We still need to rename TypeTuple to AliasSeq in this module though.
| else static if (isBinaryOp(arg)) | ||
| { | ||
| stack[$ - 2] = "(cast(T)(" ~ stack[$ - 2] ~ " " ~ arg ~ " " ~ stack[$ - 1] ~ "))"; | ||
| stack[$ - 2] = "(" ~ stack[$ - 2] ~ " " ~ arg ~ " " ~ stack[$ - 1] ~ ")"; |
There was a problem hiding this comment.
Any way this and the following red lines can be turned to green?
There was a problem hiding this comment.
No, it's CTFE only functions as most things in this module.
MartinNowak
force-pushed
the
fixup_arrayops
branch
from
d346497 to
9bd81da
Compare
|
Done, thanks for the review @WalterBright. |
MartinNowak
force-pushed
the
fixup_arrayops
branch
from
9bd81da to
823f87f
Compare
3 participants
