sociomantic-tsunami / dmxnet

D bindings and wrapper library for the MXNet deep learning library
Boost Software License 1.0
14 stars 11 forks source link

Use enum for manifest constants #100

Closed jens-mueller-sociomantic closed 5 years ago

jens-mueller-sociomantic commented 5 years ago

This change replaces all uses of static immutable with enum where the use of manifest constant won't cause an allocation. Specifically, this is the case when defining manifest constants using non-array literals (which includes string literals).

When converting to D2-only (see 6b81096) D1 const was replaced by static immutable. In all these cases D1 const served as a manifest constant. In D2 enum is used to define manifest constants. This is preferred except when done for array literals accessed more than once when running. Because in this case each use of the manifest constant will cause an allocation which is often not wanted. This does not apply to string literals which are treated specially. Hence, using an enum for a string literal carries no risk of allocation when used and is usually fine. Using enum with this in mind enables replacing static immutable which is clearer in intent and avoids any possible need for static memory to hold the immutable data.

jens-mueller-sociomantic commented 5 years ago

I'm going to improve the commit message. Any more feedback?

jens-mueller-sociomantic commented 5 years ago

Updated commit message and rebased.

joseph-wakeling-sociomantic commented 5 years ago

Your commit message refers to non-array literals, can you note the string literal case as well, since technically a string could be interpreted as an array?

jens-mueller-sociomantic commented 5 years ago

Your commit message refers to non-array literals, can you note the string literal case as well, since technically a string could be interpreted as an array?

You mean making clearer that string literals are non-array literals? I used the term literal for that reason. Because strings are arrays but string literals are not array literals. But I see the potential for confusion.

jens-mueller-sociomantic commented 5 years ago

Rephrase commit message and fixed indentation.