Re: hasElaborateCopyConstructor bug?
On Sunday, June 2, 2019 1:29:22 PM MDT Jonathan M Davis via Digitalmars-d-
learn wrote:
> On Sunday, June 2, 2019 8:32:16 AM MDT Paul Backus via Digitalmars-d-learn
> wrote:
> > On Sunday, 2 June 2019 at 06:59:02 UTC, Jonathan M Davis wrote:
> > > Almost certainly, hasElaborateCopyConstructor should be updated
> > > to test for both postblit constructors and copy constructors,
> > > since its purpose is to test for whether the type has a
> > > user-defined copying function [...] Whether
> > > hasElaborateCopyConstructor was the best name is debatable, but
> > > it _does_ involve "elaborate" copying, and copy constructors
> > > weren't actually in the language at the time. The documentation
> > > is wonderfully confusing though in that it talks about copy
> > > constructors and then says that a copy constructor is
> > > introduced by defining this(this) for a struct. So, it
> > > basically calls a postblit constructor a copy constructor.
> >
> > I've made the mistake in the past of trying to use
> > hasElaborateCopyConstructor to test for the presence of
> > __xpostblit, and I'm sure I'm not the only one. The name is quite
> > misleading--even more so now that D has real copy constructors.
> >
> > If std.v2 ever materializes, we'll have an opportunity to fix
> > papercuts like this. Until then, my preferred workaround is to
> > use a renaming import:
> >
> > import std.traits: hasNontrivialCopy =
> > hasElaborateCopyConstructor;
>
> Why is it a mistake to use hasElaborateCopyConstructor to test for
> __xpostblit? Because you're trying to test for __xpostblit for some
> purpose other than determining whether the type is blittable? I'm not
> sure what other reason there would be to test for __xpostblit though.
> Either way, hasElaborateCopyConstructor currently checks for exactly that
> (with the addition that it checks whether a static array has elements
> with __xpostblit):
>
> template hasElaborateCopyConstructor(S)
> {
> static if (__traits(isStaticArray, S) && S.length)
> {
> enum bool hasElaborateCopyConstructor =
> hasElaborateCopyConstructor! (typeof(S.init[0]));
> }
> else static if (is(S == struct))
> {
> enum hasElaborateCopyConstructor = __traits(hasMember, S,
> "__xpostblit");
> }
> else
> {
> enum bool hasElaborateCopyConstructor = false;
> }
> }
>
> My point was that given the purpose of hasElaborateCopyConstructor,
> updating it to test for both a postblit constructor and copy constructor
> would be appropriate. In fact, the fact that it hasn't been means that
> the introduction of copy constructors has broken existing code (or at
> least that such code won't interact correctly with structs that have copy
> constructors). There should be no need to rename the trait, just update
> it, and whether it uses the name "elaborate" or "non-trivial" is pretty
> much irrelevant. Personally, I probably would have chosen hasNonTrivial
> over hasElaborate, but they mean the same thing, and we have
> hasElaborateDestructor for the corresponding test for destructors and
> hasElaborateAssign for the corresponding test for assignment. It really
> doesn't make sense to change the name at this point.
It looks like Manu already reported a bug on this:
https://issues.dlang.org/show_bug.cgi?id=19902
- Jonathan M Davis
Re: hasElaborateCopyConstructor bug?
On Sunday, June 2, 2019 8:32:16 AM MDT Paul Backus via Digitalmars-d-learn
wrote:
> On Sunday, 2 June 2019 at 06:59:02 UTC, Jonathan M Davis wrote:
> > Almost certainly, hasElaborateCopyConstructor should be updated
> > to test for both postblit constructors and copy constructors,
> > since its purpose is to test for whether the type has a
> > user-defined copying function [...] Whether
> > hasElaborateCopyConstructor was the best name is debatable, but
> > it _does_ involve "elaborate" copying, and copy constructors
> > weren't actually in the language at the time. The documentation
> > is wonderfully confusing though in that it talks about copy
> > constructors and then says that a copy constructor is
> > introduced by defining this(this) for a struct. So, it
> > basically calls a postblit constructor a copy constructor.
>
> I've made the mistake in the past of trying to use
> hasElaborateCopyConstructor to test for the presence of
> __xpostblit, and I'm sure I'm not the only one. The name is quite
> misleading--even more so now that D has real copy constructors.
>
> If std.v2 ever materializes, we'll have an opportunity to fix
> papercuts like this. Until then, my preferred workaround is to
> use a renaming import:
>
> import std.traits: hasNontrivialCopy =
> hasElaborateCopyConstructor;
Why is it a mistake to use hasElaborateCopyConstructor to test for
__xpostblit? Because you're trying to test for __xpostblit for some purpose
other than determining whether the type is blittable? I'm not sure what
other reason there would be to test for __xpostblit though. Either way,
hasElaborateCopyConstructor currently checks for exactly that (with the
addition that it checks whether a static array has elements with
__xpostblit):
template hasElaborateCopyConstructor(S)
{
static if (__traits(isStaticArray, S) && S.length)
{
enum bool hasElaborateCopyConstructor = hasElaborateCopyConstructor!
(typeof(S.init[0]));
}
else static if (is(S == struct))
{
enum hasElaborateCopyConstructor = __traits(hasMember, S,
"__xpostblit");
}
else
{
enum bool hasElaborateCopyConstructor = false;
}
}
My point was that given the purpose of hasElaborateCopyConstructor, updating
it to test for both a postblit constructor and copy constructor would be
appropriate. In fact, the fact that it hasn't been means that the
introduction of copy constructors has broken existing code (or at least that
such code won't interact correctly with structs that have copy
constructors). There should be no need to rename the trait, just update it,
and whether it uses the name "elaborate" or "non-trivial" is pretty much
irrelevant. Personally, I probably would have chosen hasNonTrivial over
hasElaborate, but they mean the same thing, and we have
hasElaborateDestructor for the corresponding test for destructors and
hasElaborateAssign for the corresponding test for assignment. It really
doesn't make sense to change the name at this point.
- Jonathan M Davis
Re: hasElaborateCopyConstructor bug?
On Sunday, 2 June 2019 at 14:44:47 UTC, H. S. Teoh wrote: On Sun, Jun 02, 2019 at 02:32:16PM +, Paul Backus via Digitalmars-d-learn wrote: [...] If std.v2 ever materializes, we'll have an opportunity to fix papercuts like this. Until then, my preferred workaround is to use a renaming import: import std.traits: hasNontrivialCopy = hasElaborateCopyConstructor; Couldn't we just rename hasElaborateCopyConstructor to hasNontrivialCopy and leave a deprecated alias from the former to the latter? (Or perhaps without the deprecation, but the documentation would use the new name and hopefully new code would, too.) T My impression was that a pure name change would be unlikely to pass review (see for example https://github.com/dlang/phobos/pull/6227). But perhaps it's worth submitting the PR anyway just to see what happens.
Re: hasElaborateCopyConstructor bug?
On Sun, Jun 02, 2019 at 02:32:16PM +, Paul Backus via Digitalmars-d-learn wrote: [...] > If std.v2 ever materializes, we'll have an opportunity to fix > papercuts like this. Until then, my preferred workaround is to use a > renaming import: > > import std.traits: hasNontrivialCopy = hasElaborateCopyConstructor; Couldn't we just rename hasElaborateCopyConstructor to hasNontrivialCopy and leave a deprecated alias from the former to the latter? (Or perhaps without the deprecation, but the documentation would use the new name and hopefully new code would, too.) T -- Unix was not designed to stop people from doing stupid things, because that would also stop them from doing clever things. -- Doug Gwyn
Re: hasElaborateCopyConstructor bug?
On Sunday, 2 June 2019 at 06:59:02 UTC, Jonathan M Davis wrote: Almost certainly, hasElaborateCopyConstructor should be updated to test for both postblit constructors and copy constructors, since its purpose is to test for whether the type has a user-defined copying function [...] Whether hasElaborateCopyConstructor was the best name is debatable, but it _does_ involve "elaborate" copying, and copy constructors weren't actually in the language at the time. The documentation is wonderfully confusing though in that it talks about copy constructors and then says that a copy constructor is introduced by defining this(this) for a struct. So, it basically calls a postblit constructor a copy constructor. I've made the mistake in the past of trying to use hasElaborateCopyConstructor to test for the presence of __xpostblit, and I'm sure I'm not the only one. The name is quite misleading--even more so now that D has real copy constructors. If std.v2 ever materializes, we'll have an opportunity to fix papercuts like this. Until then, my preferred workaround is to use a renaming import: import std.traits: hasNontrivialCopy = hasElaborateCopyConstructor;
Re: hasElaborateCopyConstructor bug?
On Saturday, June 1, 2019 5:29:08 PM MDT SrMordred via Digitalmars-d-learn wrote: > On Saturday, 1 June 2019 at 21:39:33 UTC, SImen Kjærås wrote: > > On Saturday, 1 June 2019 at 21:05:32 UTC, SrMordred wrote: > > > > hasElaborateCopyConstructor checks if the type defines a > > postblit[0]. > > Yes, I know this. > > But since dmd 2.086 we have copy ctors: > https://dlang.org/changelog/2.086.0.html#copy_constructor > > And its seem logical that if I want a trait that check if copy > ctors exists I will use this name 'hasElaborateCopyConstructor' > > So it looks like a naming issue for me. > Unless postblits will be eventually replaced by copy ctors. Effectively, postblit constructors are being replaced by copy constructors. Ideally, no new code would be written with postblit constructors, and all existing postblit constructors would eventually be replaced with copy constructors. However, because of how long postblit constructors have existed in the language, Walter has no interest in actually deprecating them until he actually has to (so potentially never) in order to avoid code breakage. Almost certainly, hasElaborateCopyConstructor should be updated to test for both postblit constructors and copy constructors, since its purpose is to test for whether the type has a user-defined copying function (be it explicitly or because the type contains a type with a user-defined copying function) - and thus whether copying the type involves anything other than simply blitting. Historically, the user-defined copying function has been the postblit constructor, but whether it's a postblit constructor or a copy constructor is pretty much irrelevant to the purpose of the trait. It does make _some_ sense that it's not hasPostblit or hasPostblitConstructor, because that could easily be misconstrued as being whether it explicitly has a user-defined postblit constructor, which isn't what it tests. If a type has a postblit constructor in it directly, it has the member __postblit and the member __xpostblit (where __postblit is the explicitly declared postblit constructor and __xpostblit is what calls the postblit constructor). However, if the type does not directly declare a postblit constructor but it has a member that does declare one (directly or indirectly), then it will just have __xpostblit, which will in turn deal with copying each member correctly. So, calling the trait hasPostblit could easily have given the wrong impression. Whether hasElaborateCopyConstructor was the best name is debatable, but it _does_ involve "elaborate" copying, and copy constructors weren't actually in the language at the time. The documentation is wonderfully confusing though in that it talks about copy constructors and then says that a copy constructor is introduced by defining this(this) for a struct. So, it basically calls a postblit constructor a copy constructor. Regardless, as long as changing hasElaborateCopyConstructor to test for both the postblit constructor and the copy constructor isn't likely to break anything (and I don't _think_ that it will, but I'd have to think about it more to say for sure), then it should just be updated to take copy constructors into account. And if we ever do reach the point that we actually fully get rid of postblit constructors, then hasElaborateCopyConstructor can be updated to not test for postblit constructors any longer. - Jonathan M Davis
Re: hasElaborateCopyConstructor bug?
On Sunday, 2 June 2019 at 04:02:08 UTC, Paul Backus wrote: On Saturday, 1 June 2019 at 23:29:08 UTC, SrMordred wrote: On Saturday, 1 June 2019 at 21:39:33 UTC, SImen Kjærås wrote: On Saturday, 1 June 2019 at 21:05:32 UTC, SrMordred wrote: Haven't tested it extensively, so use at your own risk, but it should work. Nice, thanks! Atm i'm using __traits(compiles, instance.__ctor(other) ) but your solution may be the right way of doing it :)
Re: hasElaborateCopyConstructor bug?
On Saturday, 1 June 2019 at 23:29:08 UTC, SrMordred wrote:
On Saturday, 1 June 2019 at 21:39:33 UTC, SImen Kjærås wrote:
On Saturday, 1 June 2019 at 21:05:32 UTC, SrMordred wrote:
hasElaborateCopyConstructor checks if the type defines a
postblit[0].
Yes, I know this.
But since dmd 2.086 we have copy ctors:
https://dlang.org/changelog/2.086.0.html#copy_constructor
And its seem logical that if I want a trait that check if copy
ctors exists I will use this name 'hasElaborateCopyConstructor'
So it looks like a naming issue for me.
Unless postblits will be eventually replaced by copy ctors.
Here's something I came up with to check for new-style copy
constructors:
import std.traits;
import std.meta;
template hasNewCopyConstructor(T)
{
static if (hasMember!(T, "__ctor")) {
enum hasCopyConstructor = anySatisfy!(
isNewCopyConstructor,
__traits(getOverloads, T, "__ctor")
);
} else {
enum hasNewCopyConstructor = false;
}
}
enum isNewCopyConstructor(alias ctor) =
is(Unqual!(Parameters!ctor[0]) == __traits(parent, ctor))
&& (ParameterStorageClassTuple!ctor[0] &
ParameterStorageClass.ref_);
Haven't tested it extensively, so use at your own risk, but it
should work.
Re: hasElaborateCopyConstructor bug?
On Saturday, 1 June 2019 at 21:39:33 UTC, SImen Kjærås wrote: On Saturday, 1 June 2019 at 21:05:32 UTC, SrMordred wrote: hasElaborateCopyConstructor checks if the type defines a postblit[0]. Yes, I know this. But since dmd 2.086 we have copy ctors: https://dlang.org/changelog/2.086.0.html#copy_constructor And its seem logical that if I want a trait that check if copy ctors exists I will use this name 'hasElaborateCopyConstructor' So it looks like a naming issue for me. Unless postblits will be eventually replaced by copy ctors.
Re: hasElaborateCopyConstructor bug?
On Saturday, 1 June 2019 at 21:05:32 UTC, SrMordred wrote:
import std.traits;
struct T { this(ref return scope T other){} }
pragma(msg, hasElaborateCopyConstructor!T); //false
hasElaborateCopyConstructor checks if the type defines a
postblit[0]. That is, a function called this(this). this(T) is
just a regular constructor, and as such does not qualify. From
the documentation[1]:
Elaborate copy constructors are introduced by defining
this(this) for a struct.
Postblits are called after assignment, on the instance being
assigned to, and has no access to the source of the assignment.
If you need access to the source, a ref constructor like yours
will work.
--
Simen
[0]: https://dlang.org/spec/struct.html#struct-postblit
[1]:
https://dlang.org/library/std/traits/has_elaborate_copy_constructor.html
