Timon Gehr , dans le message (digitalmars.D:144133), a écrit : > On 09/07/2011 11:43 PM, Christophe wrote: >> >> Ok, don't drop this feature then. But the API is too complicated. I >> think you can keep the same functionality with an API that is much >> easier to understand: >> - Make RegionAllocatorStack private. This is implementation detail no >> one has to know about. >> - Create a method for RegionAllocator that is called something like >> "invalidatingCopy", which is the same as creating a new regionAllocator >> with the same regionAllocatorStack. Tell about the fact that the >> invalidatingCopy make the calls to the RegionAllocator being copied >> throw until all instances of the invalidatingCopy go out of scope. >> - Make a function to call "invalidatingCopy" on the default thread-local >> RegionAllocator, to replace newRegionAllocator, but I insist, please use >> a different name since it is confusing: it makes you think the allocator >> is breand new, whereas it reuses a common stack. > > It is a brand new allocator which operates on an existing stack. Well, then a simple copy of a RegionAllocator is also brand new, isn't it ? As is a copy of a File, etc.
>> - Make a function to create a brand new allocator with its own stack, >> and specifies that the previous function may be more efficient. >> >> With this, I think everybody will understand easily what is an >> invalidatingCopy of your allocator. You can explain in allocate, >> free and other methods that you check for the existence of an >> invalidatingCopy without having to tell anything about >> sharing RegionAllocatorStack with other RegionAllocators. >> > > Actually, I think the current semantics are a lot less confusing than > the invalidatingCopy proposal: > > alloc2 = alloc1.invalidatingCopy(); > alloc3 = alloc1.invalidatingCopy(); > > now alloc2 is invalid, even though you called the invalidatingCopies on > alloc1 only. To get why this is the case, the programmer has to > understand that there is an underlying stack structure, confusingly > hidden away. Therefore, hiding the RegionAllocStack is a leaky > abstraction and does not help. You are right, this is a problem. The documentation of invalidatingCopy should tell that previously created invalidatingCopies are also invalidated, and that they should be destroyed in the right order. The documentation could (and should) tell about the fact that an internal stack is shared between invalidating copy, that is not the point. This is the same explanations that what you have to explain carefully in dsimcha's API. The point is that you gave an explicit name to invalidatingCopy. With this API, a user can start to use a RegionAllocator easily, without knowing about the possibility of the powerful but dangerous idea sharing stacks. But then, this user see or type this method called invalidatingCopy, the "invalidating" in the name sounds dangerous and rings an alarm bell, and he reads carefully the documentation of invalidatingCopy. All information about stack sharing is gathered in the documentation of invalidatingCopy. Now, the user may remember having read in the documentation of allocate, free, etc, that these methods could throw an exception if an invalidatingCopy had been created, but having also decided not to use this dangeroursly looking invalidatingCopy. When he reads the documentation of allocate again, he now understand quite well what is an invalidating copy. With dsimcha's API, the user want a RegionAllocator, and types newRegionAllocator. When he wants a second RegionAllocator, he calls newRegionAllocator again, but still think he can use the first allocator. Nothing let him think calling this method twice was so dangerous. The error message make him look at the documentation of RegionAllocator.free (if it is the method that threw). He looks at that, gets confused, does not manage to understand that "an instance of RegionAllocator using the same RegionAllocatorStack" is something different than a simple copy of the RegionAllocator structure. He tries to read a bit further but rails at the documentation being spread between the introduction of the package, RegionAllocator, RegionAllocatorStack, and newRegionAllocator, and swears he will not use RegionAllocator again... I, as a "new user", understood how RegionAllocator worked only because I wanted to help in the review process by trying to find out what went wrong. And I really thought simple copies of a RegionAllocator would invalidate each other because they used the same RegionAllocatorStack, until I went down in the code to see how the correctRegionIndex field worked. > It even hurts, because with it you cannot > pass around a RegionAllocStack without a RegionAlloc operating on it. How is this a problem ? If it really is, you could still make RegionAllocatorStack available, but just further down in the documentation, so the user do not have to read it all if he do not want to use it. -- Christophe
