www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - hasElaborateCopyConstructor bug?

reply SrMordred <patric.dexheimer gmail.com> writes:
import std.traits;

struct T { this(ref return scope T other){} }
pragma(msg,  hasElaborateCopyConstructor!T); //false
Jun 01 2019
parent reply SImen =?UTF-8?B?S2rDpnLDpXM=?= <simen.kjaras gmail.com> writes:
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
Jun 01 2019
parent reply SrMordred <patric.dexheimer gmail.com> writes:
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.
Jun 01 2019
next sibling parent reply Paul Backus <snarwin gmail.com> writes:
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.
Jun 01 2019
parent SrMordred <patric.dexheimer gmail.com> writes:
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 :)
Jun 01 2019
prev sibling parent reply Jonathan M Davis <newsgroup.d jmdavisprog.com> writes:
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
Jun 01 2019
parent reply Paul Backus <snarwin gmail.com> writes:
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;
Jun 02 2019
next sibling parent reply "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Sun, Jun 02, 2019 at 02:32:16PM +0000, 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
Jun 02 2019
parent Paul Backus <snarwin gmail.com> writes:
On Sunday, 2 June 2019 at 14:44:47 UTC, H. S. Teoh wrote:
 On Sun, Jun 02, 2019 at 02:32:16PM +0000, 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.
Jun 02 2019
prev sibling next sibling parent Jonathan M Davis <newsgroup.d jmdavisprog.com> writes:
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
Jun 02 2019
prev sibling parent Jonathan M Davis <newsgroup.d jmdavisprog.com> writes:
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
Jun 02 2019