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

Add isAggegregateType and isInstantiationOf to phobos.sys.traits. #8981

Merged
merged 1 commit into from Apr 18, 2024

Conversation

jmdavis
Copy link
Member

@jmdavis jmdavis commented Apr 17, 2024

isAggregateType is the same as isAggregateType from std.traits but with tweaked documentation and examples.

isInstantiationOf is the equivalent of std.traits' isInstanceOf. The documentation and tests have been updated, and an overload for partial instantiation has been added.

The reason for the name change is that "instance" is not normally used with templates (instantiation is typically considered to be the correct term). Rather, instance is normally used to indicate that a value is an instance of a particular type. So, using isInstanceOf to check whether a type is an instantiation of a particular template seems like a misuse of the term and like it could easily cause confusion. The downside of course is that the new name is longer and harder to type, but while it's a trait that is necessary in some situations, IMHO, it's not needed frequently enough for the longer name to be a problem - particularly when it's a clearer name.

I did try to simplify isInstantationOf's implementation so that it didn't need an alias overload, but I failed, because apparently, when typeof is used on the instantiation of a function template, the fact that it's a template instantation is lost. So, unfortunately, we're forced to operate on the function's symbol rather than its type to detect whether it's an instantation of a particular template. The documentation has been updated to include that information.

I also tried to then make the alias overload not need a helper template so that fewer template instantiations would be needed, but that didn't work either, because the alias overload needs a template specialization to work, and I couldn't find a way to write an is expression that would have the same effect. So maybe, someone can improve the implementation later if they can figure that out, but since it's the same implementation as std.traits, we're not any worse off. And the overload which operates on aggregate types probably sees a lot more use anyway.

@jmdavis jmdavis added the Phobos 3 The PR is for Phobos V3. label Apr 17, 2024
@dlang-bot
Copy link
Contributor

Thanks for your pull request, @jmdavis!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + phobos#8981"

isAggregateType is the same as isAggregateType from std.traits but with
tweaked documentation and examples.

isInstantiationOf is the equivalent of std.traits' isInstanceOf. The
documentation and tests have been updated, and an overload for partial
instantiation has been added.

The reason for the name change is that "instance" is not normally used
with templates (instantiation is typically considered to be the correct
term). Rather, instance is normally used to indicate that a value is an
instance of a particular type. So, using isInstanceOf to check whether a
type is an instantiation of a particular template seems like a misuse of
the term and like it could easily cause confusion. The downside of
course is that the new name is longer and harder to type, but while it's
a trait that is necessary in some situations, IMHO, it's not needed
frequently enough for the longer name to be a problem - particularly
when it's a clearer name.

I did try to simplify isInstantationOf's implementation so that it
didn't need an alias overload, but I failed, because apparently, when
typeof is used on the instantiation of a function template, the fact
that it's a template instantation is lost. So, unfortunately, we're
forced to operate on the function's symbol rather than its type to
detect whether it's an instantation of a particular template. The
documentation has been updated to include that information.

I also tried to then make the alias overload not need a helper template
so that fewer template instantiations would be needed, but that didn't
work either, because the alias overload needs a template specialization
to work, and I couldn't find a way to write an is expression that would
have the same effect. So maybe, someone can improve the implementation
later if they can figure that out, but since it's the same
implementation as std.traits, we're not any worse off. And the overload
which operates on aggregate types probably sees a lot more use anyway.
Copy link
Contributor

@LightBender LightBender left a comment

Choose a reason for hiding this comment

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

I like the name change.

@LightBender LightBender merged commit 5d6eacc into dlang:master Apr 18, 2024
10 checks passed
@jmdavis jmdavis deleted the pv3_traits_instantiationof branch April 19, 2024 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Phobos 3 The PR is for Phobos V3.
Projects
None yet
4 participants