Skip to content
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

reimplement std.traits.ParameterStorageClassTuple() #5427

Merged
merged 1 commit into from Jun 10, 2017

Conversation

WalterBright
Copy link
Member

Reimplement to remove dependency on demangler.

Blocked by dlang/dmd#6829

* to avoid breaking existing code.
*/
if (result == (ParameterStorageClass.ref_ | ParameterStorageClass.return_))
result = ParameterStorageClass.return_;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really a concern as the result is passed via bit flags?
Isn't a much bigger concern that ParameterStorageClassTuple will then for this subset return an invalid/unexpected result and thus potentially break much more code in the future?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is used extensively by many organizations, and breaking legacy behavior is beyond the scope of this PR.

@ghost
Copy link

ghost commented May 30, 2017

The autotester errors show well why __traits should bypass the protection !

Copy link
Member

@andralex andralex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nicely done

std/traits.d Outdated
}
else
{
static assert(0, func[0].stringof ~ "is not a function");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space before "is not a..."

@WalterBright
Copy link
Member Author

Auto-merge toggled on

@wilzbach
Copy link
Member

wilzbach commented Jun 11, 2017

Hmm it seems like Vibe.d 0.7.31-rc.2 doesn't like this change:

> dub test -c core --combined --compiler=$HOME/dlang/dmd/generated/linux/debug/64/dmd --config=libevent -v
Using direct -l... flags for :libcrypto.so.1.0.0, :libssl.so.1.0.0.
/home/seb/dlang/dmd/generated/linux/debug/64/dmd -c -of.dub/build/vibe-d-test-libevent-unittest-linux.posix-x86_64-dmd_2075-4AF1581E5169A759B9CBFBDD452E80B0/vibe-d-test-libevent.o -debug -g -unittest -w -version=VibeCustomMain -version=VibeLibeventDriver -version=Have_vibe_d -version=Have_libevent -version=Have_openssl -version=Have_vibe_d_utils -version=Have_vibe_d_data -version=Have_vibe_d_core -Isource/ -I../../../.dub/packages/libevent-2.0.2_2.0.16/libevent -I../../../.dub/packages/openssl-1.1.5_1.0.1g/openssl ../../../../../tmp/dub_test_root-689601ec-138a-48ad-93f3-f1e98910070a.d source/vibe/d.d source/vibe/vibe.d source/vibe/core/args.d source/vibe/core/concurrency.d source/vibe/core/connectionpool.d source/vibe/core/core.d source/vibe/core/driver.d source/vibe/core/drivers/libasync.d source/vibe/core/drivers/libevent2.d source/vibe/core/drivers/libevent2_tcp.d source/vibe/core/drivers/native.d source/vibe/core/drivers/threadedfile.d source/vibe/core/drivers/timerqueue.d source/vibe/core/drivers/utils.d source/vibe/core/drivers/win32.d source/vibe/core/drivers/winrt.d source/vibe/core/file.d source/vibe/core/log.d source/vibe/core/net.d source/vibe/core/stream.d source/vibe/core/sync.d source/vibe/core/task.d source/vibe/crypto/cryptorand.d source/vibe/crypto/passwordhash.d source/vibe/inet/message.d source/vibe/inet/mimetypes.d source/vibe/inet/path.d source/vibe/inet/url.d source/vibe/inet/urltransfer.d source/vibe/inet/webform.d source/vibe/stream/base64.d source/vibe/stream/botan.d source/vibe/stream/counting.d source/vibe/stream/memory.d source/vibe/stream/multicast.d source/vibe/stream/openssl.d source/vibe/stream/operations.d source/vibe/stream/stdio.d source/vibe/stream/taskpipe.d source/vibe/stream/tls.d source/vibe/stream/wrapper.d source/vibe/stream/zlib.d source/vibe/textfilter/html.d source/vibe/textfilter/markdown.d source/vibe/textfilter/urlencode.d source/vibe/data/bson.d source/vibe/data/json.d source/vibe/data/serialization.d source/vibe/internal/meta/all.d source/vibe/internal/meta/codegen.d source/vibe/internal/meta/funcattr.d source/vibe/internal/meta/traits.d source/vibe/internal/meta/typetuple.d source/vibe/internal/meta/uda.d source/vibe/internal/rangeutil.d source/vibe/internal/win32.d source/vibe/utils/array.d source/vibe/utils/dictionarylist.d source/vibe/utils/hashmap.d source/vibe/utils/memory.d source/vibe/utils/string.d source/vibe/utils/validation.d source/vibe/appmain.d -vcolumns
/home/seb/dlang/dmd/generated/linux/debug/64/../../../../../phobos/std/traits.d(1046,60): Error: parameter index must be in range 0..3 not 3LU
/home/seb/dlang/dmd/generated/linux/debug/64/../../../../../phobos/std/traits.d(1047,25): Error: template instance std.traits.ParameterStorageClassTuple!(void function(FreeListRef!(shared(int), true), int delegate(int, int) pure nothrow @nogc @safe, int, int) nothrow @nogc @system).StorageClass!3LU error instantiating
/home/seb/dlang/dmd/generated/linux/debug/64/../../../../../phobos/std/traits.d(1047,25):        instantiated from here: StorageClass!2LU
/home/seb/dlang/dmd/generated/linux/debug/64/../../../../../phobos/std/traits.d(1047,25):        instantiated from here: StorageClass!1LU
/home/seb/dlang/dmd/generated/linux/debug/64/../../../../../phobos/std/traits.d(1052,44):        instantiated from here: StorageClass!0LU
/home/seb/dlang/dmd/generated/linux/debug/64/../../../../../phobos/std/typecons.d(4273,22):        ... (4 instantiations, -v to show) ...
source/vibe/core/concurrency.d(1146,34):        instantiated from here: toDelegate!(void function(FreeListRef!(shared(int), true), int delegate(int, int) pure nothrow @nogc @safe, int, int) nothrow @nogc @system)
source/vibe/core/concurrency.d(1187,15):        instantiated from here: async!(int delegate(int, int) pure nothrow @nogc @safe, int, int)
/home/seb/dlang/dmd/generated/linux/debug/64/../../../../../phobos/std/functional.d(1224,13): Error: CTFE failed because of previous errors in generateFunction
source/vibe/core/concurrency.d(1131,47): Error: function vibe.core.concurrency.async!(int delegate(int, int) pure nothrow @nogc @safe, int, int).async no return exp; or assert(0); at end of function
FAIL .dub/build/vibe-d-test-libevent-unittest-linux.posix-x86_64-dmd_2075-4AF1581E5169A759B9CBFBDD452E80B0/ vibe-d-test-libevent executable

Error on Jenkins.

@andralex
Copy link
Member

Hmm why was it then accepted? I don't think I pressed the red button for this. @WalterBright I'll post a revert to see whether it fixes the matter.

@CyberShadow
Copy link
Member

CyberShadow commented Jun 11, 2017

Hmm why was it then accepted?

We disabled Jenkins as a required CI because it was broken while @MartinNowak was unavailable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants