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

implement std.traits.isInstanceOf for non-types #4739

Merged
merged 1 commit into from
Aug 30, 2016

Conversation

aG0aep6G
Copy link
Contributor

Sparked by @lodo1995 showing me in a Learn thread that TemplateOf supports this.

@dlang-bot
Copy link
Contributor

@aG0aep6G, thanks for your PR! By analyzing the annotation information on this pull request, we identified @dsimcha, @9rnsr and @sinfu to be potential reviewers. @dsimcha: The PR was automatically assigned to you, please reassign it if you were identified mistakenly.

(The DLang Bot is under development. If you experience any issues, please open an issue at its repo.)

static assert(isInstanceOf!(Foo, Foo!int));
static assert(!isInstanceOf!(Foo, Bar!int));
static assert(!isInstanceOf!(Foo, int));
static assert(isInstanceOf!(Doo, Doo!int));
static assert(isInstanceOf!(ABC, ABC!1));
static assert(!__traits(compiles, isInstanceOf!(Foo, Foo)));
static assert(!isInstanceOf!(Foo, Foo));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This might need some attention. I don't see a reason why isInstanceOf!(Foo, Foo) should fail compilation, but since this was specifically stated here, maybe there is a reason.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it's because the original implementation did not handle things different from types as second parameter, and Foo is not a type, so it wouldn't compile.

@lodo1995
Copy link
Contributor

The autotester fail seems unrelated to the changes made here: problems with a file descriptors, probably due to the test machine being short of disk space.

@wilzbach
Copy link
Member

The autotester fail seems unrelated to the changes made here: problems with a file descriptors, probably due to the test machine being short of disk space.

FYI you can deprecate such builds by pressing on the "deprecate" link

enum impl(alias T : S!Args, Args...) = true;
enum impl(alias T) = false;
enum isInstanceOf = impl!T;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would have written this as

enum bool isInstanceOf(alias S, alias T : S!Args, Args...) = true;
enum bool isInstanceOf(alias S, alias T) = false;

But: https://issues.dlang.org/show_bug.cgi?id=16403

Copy link
Member

Choose a reason for hiding this comment

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

noice

@lodo1995
Copy link
Contributor

FYI you can deprecate such builds by pressing on the "deprecate" link

Well, until two minutes ago I didn't even know that I can log in the autotester... Thank you!

@wilzbach
Copy link
Member

Well, until two minutes ago I didn't even know that I can log in the autotester... Thank you!

You might also be interested in reading #4585 then ;-)
(sorry for the off topic chat)

@burner
Copy link
Member

burner commented Aug 25, 2016

  1. The order to T and S is wrong. It is like if(5 == x). think UFCS
  2. This is just a very long wrapper around a language feature. I'm not sure if that has a place in phobos @andralex ?

@dnadlinger
Copy link
Member

dnadlinger commented Aug 25, 2016

@burner: The template already exists, and there is no UFCS for templates anyway.

@burner
Copy link
Member

burner commented Aug 25, 2016

whoops my bad. Sorry for the noise.

still reads funny though

@andralex
Copy link
Member

cool thx

@andralex andralex merged commit 4983386 into dlang:master Aug 30, 2016
@aG0aep6G aG0aep6G deleted the isInstanceOf-for-non-types branch August 31, 2016 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants