reimplement std.traits.ParameterStorageClassTuple()#5626
reimplement std.traits.ParameterStorageClassTuple()#5626dlang-bot merged 4 commits intodlang:masterfrom
Conversation
|
Thanks for your pull request, @rainers! 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. |
The project tester now tests the new v0.8.0 vibe.d release, we should test this with v0.7.31, but otherwise this is great :) |
std/traits.d
Outdated
| ParameterStorageClass extractParameterStorageClassFlags(Attribs...)() | ||
| { | ||
| auto result = ParameterStorageClass.none; | ||
| foreach (attrib; Attribs) |
There was a problem hiding this comment.
it disables the loop-unrolling.
currently the switch will be duplicated Attribs.length times.
There was a problem hiding this comment.
Ok I need to reclarify - why is this a bad thing? Isn't this even faster due to the saved conditional jump and better pipelining?
There was a problem hiding this comment.
this code is always executed at ctfe.
at ctfe it's strictly the less instructions you have, the better.
There was a problem hiding this comment.
The brackets don't work, I get errors building the unittests:
std\traits.d(1092): Error: variable std.traits.extractParameterStorageClassFlags!().extractParameterStorageClassFlags.attrib variables cannot be of type void
std\traits.d(1092): Error: expression __r3585[__key3586] is void and has no value
std\traits.d(1058): Error: template instance std.traits.ParameterStorageClassTuple!(inout Inner(inout(Inner))).StorageClass!0LU error instantiating
std\traits.d(635): instantiated from here: ParameterStorageClassTuple!(inout Inner(inout(Inner)))
std\traits.d(792): instantiated from here: parametersTypeString!(inout Inner(inout(Inner)))
std\traits.d(472): instantiated from here: fqnType!(inout Inner(inout(Inner)), false, false, false, false)
std\traits.d(860): instantiated from here: fqn!(inout Inner(inout(Inner)))
std\traits.d(860): while evaluating: static assert(fqn!(inout Inner(inout(Inner))) == format("inout(%s(inout(%s)))", "std.t
raits.QualifiedNameTests.Inner", "std.traits.QualifiedNameTests.Inner"))
There was a problem hiding this comment.
put a static if (Attribs.length) if front of the foreach
then brackets should work
There was a problem hiding this comment.
Yeah, it works with static if (Attribs.length). Updated.
Tested. Builds fine. |
std/traits.d
Outdated
| Params: | ||
| func = function symbol or type of function, delegate, or pointer to function | ||
| Returns: | ||
| a tuple of ParameterStorageClass bits |
There was a problem hiding this comment.
Nit: Common style is to start with upper case here (it's not enforced yet).
Thanks.
Updated. BTW: there is a "little demangler" at the top of the file that seems unused now. Should we ditch it? |
I think so, but let's better do this in a different PR. I'm impatient as well and I want to see your great work merged rather sooner than later! It's been in the queue way too long. |
I was already removing it when having the same thought ;-) |
std/traits.d
Outdated
| * Returns: | ||
| * ParameterStorageClass bits | ||
| */ | ||
| ParameterStorageClass extractParameterStorageClassFlags(Attribs...)() |
There was a problem hiding this comment.
transform this into template extractParameterStorageClassFlags(Attribs...)
enum extractParameterStorageClassFlags = () {
// the old function body
} ();
this will force the template to resolve to literal rather then a symbol.
There was a problem hiding this comment.
Done. Interesting trick to avoid generating code into the object file for functions only executed at compile time. This should also avoid the coverage tests complaining.
|
@CyberShadow I suspect you are aware of this, but just in case: the documentation tester currently seems to complain about "Pull request SHA mismatch" on every phobos pull request I looked at. Anything we need to fix? |
|
Merging this now as it has been approved before. |
Due to our current CI problem here, I will create a new PR to be sure that there aren't any new failures popping up in Jenkins. |
|
Okay sorry - seems that didn't help:
But at least restarting twice seems to work :/ edit: And FYI the first commit built fine |
|
Jenkins failures are due to dlang/druntime#1879 (comment). |
5 participants
reboot #5427 by @WalterBright as it is required for the new mangling in dlang/dmd#6998
Maybe something else changes, but locally I cannot reproduce an error with
toDelegate!(void function(FreeListRef!(shared(int), true), int delegate(int, int) pure nothrow @nogc @safe, int, int) nothrow @nogc @system)as in the previous attempts build logs.