Strict @property syntax compliance (-property)

[dnadlinger]

Makes library and unit tests build with -property and adds the switch to the default build to avoid regressions. Requires dlang/druntime#97, which has been merged in the meantime.

The pull request also contains a few other commits with minor cleanup changes I made while going through the source.

Tested on OS X x86, Linux x86_64 and Windows x86.

[shoo]

I think that user-defined init is a property.

[dnadlinger]

@shoo: Are you referring to TypeInfo.init() from the druntime pull request? If so, the situation is a bit more complex than it seems at first: Yes it should be a @property from a semantic point of view, but its name is a very unfortunate choice, since it returns the initializer (i.e., a byte array) for the type it describes, not for TypeInfo itself. Thus, just annotating it with the property attribute produces a name clash with the built-in .init property which refers to a default-initialized instance of the type info class itself.

[shoo]

@klickverbot: Yes, I commented to druntime's one (Sorry).
IMHO, even if TypeInfo.init had such a problem if I think, after all init should be a property. It brings me into question that init does not return TypeInfo oneself, but it is a different problem.

[dnadlinger]

@shoo: The problem is that making init() a property without changing its name would mask the built-in .init property with a completely different meaning, leaving you no way to actually access the zero-initialized instance of the TypeInfo class itself (this was also discussed back when #39 was merged). In this pull request, I went for the simple route and just fixed the local symptoms so that Phobos can be built with -property (which was my original goal, so that in turn user code can be built with it). What would be your suggestion for resolving the issue?

[shoo]

I did not know that it had been already discussed...

My opinion:
It is impossible to distinguish whether obj.init is build-in property or not if -property switch is not used.
In addition, T.init may not always return an initialized value of T. ( e.g. @disable static @property typeof(this) init(); is defined. / This shows that you must not hope that T.init is always an initial value of T.)
Therefore, it does not have any problem to make .init @Property.

On the other hand however, this problem must be corrected with a big breaking change sometime.
It is a clear design fault that TypeInfo.init does not return TypeInfo.
The function of this purpose should prepare separately.

[dnadlinger]

@shoo: »It is a clear design fault that TypeInfo.init does not return TypeInfo.« I don't know if this is clear to you anyway and I'm just misunderstanding you, but no, the design fault is not that TypeInfo.init returns void[] but that it is named init in the first place - the array of bytes it returns does not represent a TypeInfo instance, but an instance of the described type.

Besides, I don't think I agree with your point that T.init might not always be an initialized value of T – the language spec explicitly says so. Everything else is probably an accepts-invalid bug or at least a gross abuse of the spec.

[shoo]

@klickverbot:
User Defined Class and Struct Properties is written in the language spec, too. The priority does not exist... A further argument is a problem of likes and dislikes.

[dnadlinger]

@shoo: I don't think »the priority does not exist« – the spec lists .init in the »Properties for All Types« table for a reason, and thus it can be assumed that it has said meaning, well, for all types (which Phobos btw does in quite a few places). Everything else should be disallowed, as it violates the spec in this point. Or the spec changed, obviously, but would there be even a single argument for allowing it? By the way, the druntime pull request has been merged in the meantime, so if you want to continue this discussion, you might want to post to the ML/NG.

[shoo]

I do not deny your opinion. I can catch a meaning like that. However, it is possible as follows to catch a meaning.
.init property is had in all types and returns an initial value of the type. It may be a user definition.
I do not know which is more right in two opinions. But, it is not the main intent of this argument. Therefore, I said that it was the problem of likes and dislikes.
It is important for this argument that the feature is property or not.

[shoo]

I think that isFoo is not property.
If the name begins with a verb, it is not a property.

[dnadlinger]

I don't think a strict »if the name contains a verb, it's not a property« rule works well here – the »is« prefix in »isFoo« does not represent any change to the object state (like »make«, »do«, etc. would), but is a commonly used naming convention for boolean fields (»Is it alive?«).

[shoo]

This may be the problem of likes and dislikes, too.
Because I have a confused idea, I itemize it:

• For example, Range.empty does not have "is". This naming lacks in consistency...?
• I feel the old custom that names getter-property getFoo in this.
• Is not isFoo an action to refer to an object for a state?

[dnadlinger]

isAlive was clearly intended to be a property (the doc comment even explicitly mentions it), but @property annotations simply didn't exist in the pre-1.0 days when the code was written.

Using an »is« prefix for boolean fields is a fairly wide-spread naming convention, obviously with its own pros and cons – see e.g. Code Complete. It is used in quite a few places all over Phobos, also in conjunction with @property, not only free »predicate methods« (e.g. isEnd, isLeapYear, isDir, isSymlink, …), and, yes, the names in Phobos are not consistent in that regard. Of course, these choices could be discussed, but I think here in this pull request is neither the right place nor time to do so.

[andralex]

This disagreement is a reflection of what I feared when opposing @Property: everybody will have a slightly different opinion about exactly what's a property and what's not. To add to the quarrel, here's mine: If it doesn't change the object and always returns the same thing for equal objects, it's a property. Otherwise, it's not.

[shoo]

I reviewed it as possible in detail. I judge that there is not the fatal problem. After having put some periods of grace, I'll merge it.

[dnadlinger]

どうもありがとうございました (Did I get that right? I am still struggling with the very basics of Japanese) – reviewing this mess of tiny changes must've been very tedious.

[andralex]

Thanks for this work, both of you. David, I should note that although this is clearly valuable, your time would be much more gainfully spent on the Thrift bindings. Thanks!

[dnadlinger]

@andralex: Actually, the Thrift bindings were the very reason for these changes (which I had in the pipe for quite some time now) – I wanted to make sure that they don't fall apart when someone tries to use -property, and the easiest way was to make them compile with it, which necessitated quite a part of the commits (because of templates being instantiated using -property) ;) The Thrift test suite is all green now, btw, expect a mail soon.