January 13, 2022

On Thursday, 13 January 2022 at 17:18:47 UTC, Dukc wrote:

>

On Monday, 10 January 2022 at 13:48:42 UTC, Mike Parker wrote:

>

Feedback Thread

This is the feedback thread for the first round of Community Review of DIP 1042, "ProtoObject".

I still think [snip]

Oh, I forgot my overall opinion. I guess it's positive. Yes, even now you can make your own object to accomplish mostly the same and yes, afterthought additions like this do make the object tree to look ugly. But the real value of this DIP is that we're going to have a standard "better object" instead of a dozen different ones in DUB packages.

Another opinion would have been to make a new object that inherits the old one, instead of vice-versa. But this option makes it possible to statically reject opCmp invocations, as opposed to throwing an exception at runtime. And we also get to kill the mandatory monitor without needing to break code.

January 13, 2022
Based on RazvanN's reply to Elronnd (quoted for context):

> I think that the fundamental idea here is that when the library owner
> switches from Object to ProtoObject that is a breaking change that should
> be advertised, because he is changing the API. The library owner should
> either release a new major version or provide the same utilies as Object.
> The idea here is that you do not know what your users are doing; even without
> library X, the user can simply call toHash because Object used to have it.
If this change leads each of my dependencies to make breaking changes then its backwards compatibility certainly isn't the "no breaking changes" claimed by the DIP. This effect on users of third-party libraries should be called out in the DIP's Breaking Changes and Deprecations section.

(I'm replying to the original post here because my original reply violated Feedback rules; sorry about that).
January 14, 2022
On Thursday, 13 January 2022 at 17:57:24 UTC, David Gileadi wrote:
> Based on RazvanN's reply to Elronnd (quoted for context):
>
>> I think that the fundamental idea here is that when the library owner
>> switches from Object to ProtoObject that is a breaking change that should
>> be advertised, because he is changing the API. The library owner should
>> either release a new major version or provide the same utilies as Object.
>> The idea here is that you do not know what your users are doing; even without
>> library X, the user can simply call toHash because Object used to have it.
> If this change leads each of my dependencies to make breaking changes then its backwards compatibility certainly isn't the "no breaking changes" claimed by the DIP. This effect on users of third-party libraries should be called out in the DIP's Breaking Changes and Deprecations section.
>
> (I'm replying to the original post here because my original reply violated Feedback rules; sorry about that).

I think that this case is being exaggerated here. I find it hard to believe
that people write libraries where they simply type a parameter as being
Object. Most library code is templated which means that the received types
can be either class, struct or basic types. As such, I would expect that
folks would at least test for the existence of the likes of toHash, opCmp etc.
before using them.

My expectation is that, in most cases, updating to ProtoObject will not incur any breaking changes, however, there might be a few situations where users (arguably, wrongfully) were using the builtin object methods of class instances from library X. In this scenario, you will end up with a compile error in user code after upgrading. I would argue that this is for the best if the library owner did not intend to expose those facilities.

January 14, 2022
On Thursday, 13 January 2022 at 17:57:24 UTC, David Gileadi wrote:
> Based on RazvanN's reply to Elronnd (quoted for context):
>

> If this change leads each of my dependencies to make breaking changes then its backwards compatibility certainly isn't the "no breaking changes" claimed by the DIP. This effect on users of third-party libraries should be called out in the DIP's Breaking Changes and Deprecations section.
>
> (I'm replying to the original post here because my original reply violated Feedback rules; sorry about that).

I think that this breaking change, although possible, has minimal
chances of happening. The reason I believe this is that libraries
typically write templated code to be able to deal with classes,
structs and templated types. As a consequence, you cannot blindly
call the likes of opCmp, toHash etc.

Even if this scenario is met (which I think it is highly unprobable),
the failure will be clear and arguably it would be justifiable because
the defaults for opCmp/toHash simply return the address of the class.
January 14, 2022
> At best, class factory registration should be opt-in because only a small number of classes (none in the standard library) require the feature

If making factory opt in is the best choice (which it is), why does the DIP not do this?
January 15, 2022
This old bug came up overnight:

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

ponce said: "Hopefully fixed in ProtoObject! ;)"

Dr. Alexandrescu said in reply: "Yah, the DIP should definitely amended to include ~this() on the list of functions to look at. Assigning to Razvan so we don't forget about this."


Of course, ProtoObject wouldn't fix this anyway - just like with the other attributes, they have *nothing to do with Object* so no amount of changes there would actually fix the problem - but now we have yet another thing this DIP was supposed to fix that was completely unaddressed.

This DIP must be rejected. There is no hope to salvage it; it is completely broken from top to bottom.
January 15, 2022

The DIP is jumping into details without acknowledging that the class concept is a high level construct related to OO methodology. The design should be rooted in a rationale based on specific OO modelling principles (maybe read conference proceeding for ideas) and usability.

Since it is a high level concept you essentially should strive to make it easy on the programmer and put the burden of optimization on the compiler. This must be discussed. For instance, an implementation can remove the mutex if it isnt used. It can even do so in separate compilation by putting the mutex at a negative offset. If you need more details, feel free to ask.

All other OO languages in the Simula family use «object» as the root (if they provide one). Deviating from this norm will cause confusion and create friction and annoyance among people who have an OO background. The motivation for deviating from this is not convincing.

It is desirable to have one root, yet, you propose 3 roots? This needs more motivation. D needs to be made simpler, not more convoluted.

I am not getting into the specifics of interfaces, but as implemented they are to be avoided as they cost 8 bytes per instanced object per interface. So it is better to adorn the root object with abstract virtuals which are free. If you want to force interfaces on developers then you need to provide an interface mechanism that is usable (the current one is too expensive).

The list of popular languages at the end does not seem motivated. Why only these commercial languages and why listing them? By not referencing more academic OO resources the reader will get the impression that you did not do a proper survey and that the DIP as written is more of an early sketch than a final design.

January 15, 2022

One more thing: a root object redesign must take into account adding a reference count and discuss how that can be merged with a monitor mutex into a single field.

Not covering this would be a serious omission given the desire to provide an alternative to GC ownership.

January 25, 2022

On Monday, 10 January 2022 at 13:48:42 UTC, Mike Parker wrote:

>

The review period will end at 11:59 PM ET on January 24, or when I make a post declaring it complete. At that point, this thread will be considered closed and any subsequent feedback may be ignored at the DIP author's discretion.

This round of review is now complete. Thanks to everyone who left feedback.

1 2
Next ›   Last »