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.

Sign up for GitHub

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 spec for the @disable new(); allocator relic #2846

Closed
wants to merge 1 commit into from
Closed

add spec for the @disable new(); allocator relic #2846

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 0 additions & 51 deletions deprecate.dd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ $(SPEC_S Deprecated Features,
$(THEAD Feature, Spec, Dep, Error, Gone)
$(TROW $(DEPLINK body keyword), 2.075,  ,  ,  )
$(TROW $(DEPLINK Hexstring literals), 2.079, 2.079, 2.086,  )
$(TROW $(DEPLINK Class allocators and deallocators),  , 2.080, 2.087,  )
Copy link
Member

Choose a reason for hiding this comment

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

Just need to update the gone column, no need to remove the documented deprecation.

$(TROW $(DEPLINK Implicit comparison of different enums), 2.075, 2.075, 2.081,  )
$(TROW $(DEPLINK Implicit string concatenation), 2.072, 2.072, 2.081,  )
$(TROW $(DEPLINK Using the result of a comma expression), 2.072, 2.072, 2.079,  )
Expand Down Expand Up @@ -116,56 +115,6 @@ $(H4 Rationale)
$(P Hexstrings are used so seldom that they don't warrant a language feature.
)

$(H3 $(DEPNAME Class allocators and deallocators))
$(P D classes can have members customizing the (de)allocation strategy.
---
class Foo
{
new(uint size, ...)
{
return malloc(size);
}

delete(void* obj)
{
free(obj);
}
}

Foo foo = new(...) Foo();
delete foo;
---
)
$(H4 Corrective Action)
$(P Move the (de)allocation strategy out of the class
---
class Foo
{
}

T make(T, Args...)(auto ref Args args) if (is(T == Foo))
{
enum size = __traits(classInstanceSize, T);
void* mem = malloc(size);
scope (failure) free(mem);
return mem !is null ? emplace!T(mem[0..size], args) : null;
}

void dispose(T)(T obj)
{
auto mem = cast(void*) obj;
scope (exit) free(mem);
destroy(obj);
}

Foo foo = make!Foo();
if (foo !is null) dispose(foo);
---
)
$(H4 Rationale)
$(P Classes should not be responsible for their own (de)allocation strategy.
)
Copy link
Member

Choose a reason for hiding this comment

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

No this needs to be kept. The entire point of this document is to provide historic rationals.


$(H3 $(DEPNAME Implicit comparison of different enums))
$(P Comparison of different enumerated type was allowed:
---
Expand Down
127 changes: 0 additions & 127 deletions spec/class.dd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,6 @@ $(UL
$(LI $(GLINK SharedStaticDestructor)s)
$(LI $(RELATIVE_LINK2 invariants, Class Invariants))
$(LI $(DDSUBLINK spec/unittest, unittest, Unit Tests))
$(LI $(RELATIVE_LINK2 allocators, Class Allocators))
$(LI $(RELATIVE_LINK2 deallocators, Class Deallocators))
$(LI $(RELATIVE_LINK2 alias-this, Alias This))
)
)
Expand Down Expand Up @@ -1004,131 +1002,6 @@ $(GNAME ClassInvariant):
)
)


$(H2 $(LNAME2 allocators, Class Allocators))
$(B Note): Class allocators are deprecated in D2.
$(GRAMMAR
$(GNAME Allocator):
$(D new) $(GLINK2 function, Parameters) $(D ;)
$(D new) $(GLINK2 function, Parameters) $(GLINK2 function, FunctionBody)
)

$(P A class member function of the form:)

------
new(size_t size)
{
...
}
------

is called a class allocator.
The class allocator can have any number of parameters, provided
the first one is of type `size_t`.
Any number can be defined for a class, the correct one is
determined by the usual function overloading rules.
When a new expression:

------
new Foo;
------

is executed, and Foo is a class that has
an allocator, the allocator is called with the first argument
set to the size in bytes of the memory to be allocated for the
instance.
The allocator must allocate the memory and return it as a
$(D void*).
If the allocator fails, it must not return a $(D null), but
must throw an exception.
If there is more than one parameter to the allocator, the
additional arguments are specified within parentheses after
the $(D new) in the $(I NewExpression):

------
class Foo
{
this(string a) { ... }

new(size_t size, int x, int y)
{
...
}
}

...

new(1,2) Foo(a); // calls new(Foo.sizeof,1,2)
------

$(P Derived classes inherit any allocator from their base class,
if one is not specified.
)

$(P The class allocator is not called if the instance is created
on the stack.
)

$(P See also
$(LINK2 https://wiki.dlang.org/Memory_Management#Explicit_Class_Instance_Allocation,
Explicit Class Instance Allocation).
)

$(H2 $(LNAME2 deallocators, Class Deallocators))
$(B Note): Class deallocators and the delete operator are deprecated in D2.
Use the $(D destroy) function to finalize an object by calling its destructor.
The memory of the object is $(B not) immediately deallocated, instead the GC
will collect the memory of the object at an undetermined point after finalization:

------
class Foo { int x; this() { x = 1; } }
Foo foo = new Foo;
destroy(foo);
assert(foo.x == int.init); // object is still accessible
------

$(GRAMMAR
$(GNAME Deallocator):
$(D delete) $(GLINK2 function, Parameters) $(D ;)
$(D delete) $(GLINK2 function, Parameters) $(GLINK2 function, FunctionBody)
)

$(P A class member function of the form:)

------
delete(void *p)
{
...
}
------

is called a class deallocator.
The deallocator must have exactly one parameter of type $(D void*).
Only one can be specified for a class.
When a delete expression:

------
delete f;
------

is executed, and f is a reference to a class instance that has a deallocator,
the deallocator is called with a pointer to the class instance after the
destructor (if any) for the class is called. It is the responsibility of the
deallocator to free the memory.

$(P Derived classes inherit any deallocator from their base class,
if one is not specified.
)

$(P The class allocator is not called if the instance is created
on the stack.
)

$(P See also
$(LINK2 https://wiki.dlang.org/Memory_Management#Explicit_Class_Instance_Allocation,
Explicit Class Instance Allocation).
)

$(H2 $(LEGACY_LNAME2 AliasThis, alias-this, Alias This))

$(GRAMMAR
Expand Down
24 changes: 22 additions & 2 deletions spec/module.dd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ $(GNAME DeclDef):
$(GLINK2 class, Constructor)
$(GLINK2 class, Destructor)
$(GLINK2 struct, Postblit)
$(GLINK2 class, Allocator)
$(GLINK2 class, Deallocator)
$(GLINK2 class, ClassInvariant)
$(GLINK2 struct, StructInvariant)
$(GLINK2 unittest, UnitTest)
Expand All @@ -37,6 +35,7 @@ $(GNAME DeclDef):
$(GLINK2 template-mixin, TemplateMixinDeclaration)
$(GLINK2 template-mixin, TemplateMixin)
$(GLINK MixinDeclaration)
$(GLINK AllocatorRelic)
$(D ;)
)

Expand Down Expand Up @@ -634,6 +633,27 @@ $(GNAME MixinDeclaration):
$(GLINK DeclDefs), and is compiled as such.
)

$(H2 $(LEGACY_LNAME2 AllocatorRelic, allocator-relic, Allocator Relic))
Copy link
Member

Choose a reason for hiding this comment

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

I don't see there any need to call it a relic, either in the grammar or documentation, unless @disable new() itself is up for deprecation.

Copy link
Author

Choose a reason for hiding this comment

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

Sorry but I really like this rule name. from the Cambridge dictionnary

relic
 noun
  /ˈrelik/
something left from a past time

That matches perfectly to what it is.

Copy link
Member

Choose a reason for hiding this comment

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

By calling it a relic, to me this is implicitly saying "don't use", or at the very least discourages use (why would you use a relic feature in your codebase?)

Copy link
Author

Choose a reason for hiding this comment

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

A relic is not something you drop. It's cultural inheritage. It's something you keep because it tells something about history and culture.

Copy link
Member

Choose a reason for hiding this comment

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

A relic conjures up that it is an artefact, vestige, or an archaism in the language. But as far as I can tell, there is no reason to tell people that they should not use @disable new().

Copy link
Author

Choose a reason for hiding this comment

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

Suggest me another rule name. Discussing about semantics will lead to nothing, although, to be clear, I really liked my proposal.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it's a cultural/language thing but I would read "relic" (in the context of software docs) as a soft deprecation.
(Read as "it exists but we don't dare to remove it").

Maybe use a neutral name based on the actual effect of @disable new? Smth. like Disabling dynamic allocation, ...

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I'm sorry, but I agree that relic is a terrible name for the reasons stated above.

Why not name it what it actually is. A few examples: "NewClassDisabler", "ClassAllocationOptOut", "DefaultClassAllocationRule".

Copy link
Member

@ibuclaw ibuclaw Aug 19, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Suggest me another rule name. Discussing about semantics will lead to nothing, although, to be clear, I really liked my proposal.

I think the following:

  1. Use LEGACY_LNAME2 as you currently are.
  2. Keep the grammar rule name as Allocator.
  3. While the spec shouldn't be used to sell features, try to keep to strictly saying what the feature is, rather allude to what it was.

For instance (re-using some of the existing documentation):

A class member function of the form

@disable new();

is used to explicitly disallow the new operator being used to allocate an object instance
of its type. For example, to enforce the usage of std.allocators.make() on classes or
encourage stack allocation for structs.

I assume that @disable new() is permitted for structs too?


$(GRAMMAR
$(GNAME AllocatorRelic):
$(D new) $(D $(LPAREN)) $(D $(RPAREN)) $(D ;)
)

$(P This is a relic of the old D1-style class and struct custom $(D new) operator.
Its sole purpose is to be prefixed with $(D @disable) so that $(D new) operator
is not permited for the aggregate that contains the allocator relic, for example
Copy link
Member

Choose a reason for hiding this comment

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

permitted

Copy link
Author

@ghost ghost Aug 18, 2020 •
edited by ghost
Loading

Choose a reason for hiding this comment

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

"NewClassDisabler" does not work. @disable is verified during semantics checks, it is still grammatically correct to have new(); without the attribute.

Copy link
Author

Choose a reason for hiding this comment

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

let's go for NewDeclaration then.

to enforce the usage of $(D std.allocators.make()) on classes or encourage
stack allocation for structs.)

---------
class NewNotAllowed
{
@disable new();
}

with (new NewNotAllowed()) {} // Error: the `new` operator is disabled for type `main.NewNotAllowed`
---------

$(H2 $(LEGACY_LNAME2 PackageModule, package-module, Package Module))

Expand Down