Bug 10150 – Prefix method 'this' qualifiers should be just ignored anytime

By fixing issue 9199, git head rejects all meaningless method qualifiers for non-method functions. class C1 { const static void fc() {} // NG in both 2.062 and git head immutable static void fi() {} // NG in both 2.062 and git head shared static void fs() {} // OK in 2.063 but NG in git head inout static void fw() {} // OK in 2.063 but NG in git head } class C2 { static void fc() const {} // NG in both 2.062 and git head static void fi() immutable {} // NG in both 2.062 and git head static void fs() shared {} // OK in 2.063 but NG in git head static void fw() inout {} // OK in 2.063 but NG in git head } const void fc1() {} // NG in both 2.062 and git head immutable void fi1() {} // NG in both 2.062 and git head shared void fs1() {} // OK in 2.063 but NG in git head inout void fw1() {} // OK in 2.063 but NG in git head void fc2() const {} // NG in both 2.062 and git head void fi2() immutable {} // NG in both 2.062 and git head void fs2() shared {} // OK in 2.063 but NG in git head void fw2() inout {} // OK in 2.063 but NG in git head --- But this behavior is restrictive and now breaking exist code. Dlanguage allows to rewrite prefix storage class to label syntax and scope syntax. const void foo() {} // previous const: void foo() {} // label const { void foo() {} } // scope Making errors for the functions enclosed by attribute would be inconvenient. So, I'd like to propose that: 1. Module level functions and class/struct scope static functions just ignore prefix method 'this' qualifiers (const, immutable, shared, and inout). 2. But specifying them at postfix position should be rejected. The rule is consistent and make writing code more handy.

Commits pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/958abf2c48333cc0f800dff51da435338458b751 fix Issue 10150 - Prefix method 'this' qualifiers should be just ignored anytime https://github.com/D-Programming-Language/dmd/commit/091cb6eea52eec998f60d8e6be6b54413d173b3b Merge pull request #2071 from 9rnsr/fix10150 [enh][REG2.063a] Issue 10150 - Prefix method 'this' qualifiers should be just ignored anytime

Commit pushed to 2.063 at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/7c3ed3e585e829c7bf97ca8f78b6c7d76c98b0ba Merge pull request #2071 from 9rnsr/fix10150 [enh][REG2.063a] Issue 10150 - Prefix method 'this' qualifiers should be just ignored anytime

I disagree with this change, this is extremely dangerous behavior when interfacing with C. Take a look at the following: ----- extern(C) const int *foo(); void main() { *foo() = 1; // compiles } ----- The proper definition should have been: ----- extern(C) const(int)* foo(); void main() { *foo() = 1; // fails } ----- It is very easy to make this mistake, it should not be silent, at least not for module-scoped functions. Alternatively as a compromise I suggest we at least add this check for extern(C) functions, because this is where this problem can occur very frequently.

Another alternative is to make the compiler smarter, and only disallow const where it's only applied to one function, for example: ----- const int * foo(); // disallowed const { int * foo(); // ok int * bar(); // ok } const: int * foo(); // ok ----- This would be for the sake of convenience, to avoid breaking existing code.

I think "const T foo" should be parsed as "const(T) foo" where T is not void and be an error otherwise. (I was almost certain that former was how it has behaved before!) This is a very natural style to type in, especially for those that come from C/C++. Silent no-op is probably most dangerous decision possible.

I actually see no rationale behind this change because enhancement descriptions mentions only function returning "void".

(In reply to comment #6) > This is a very natural style to type in, especially for those that come from > C/C++. Well not entirely, since 'const void *' in D and C++ mean different things, const(void*) vs const(void)*. > Silent no-op is probably most dangerous decision possible. Yep, at least for non-grouped declarations like I've mentioned.

(In reply to comment #6) > I think "const T foo" should be parsed as "const(T) foo" where T is not void > and be an error otherwise. (I was almost certain that former was how it has > behaved before!) I think that would be a horrible decision, because "const T foo" *means* something for member functions. Your proposed change would have a different parse meaning depending on if the function is member or not: const T foo(); (1) struct S { const T foo(); (2) } Method (1) would return a const(T). But method (2) would return a T. As much of a surprise as it is for C++ new comers, D's "const" syntax is simply different (and better). The word "const" all by itself is a function attribute, and that is all it should ever be. The compiler should *help* catch such erroneous usage, not silently ignore it. Also, having a different behavior depending on prefix/postfix makes no sense to me. Both are *equally* wrong, and arguably, "prefix qualifiers" have *higher* chances of being actual mistakes. The evidence of this has already been demonstrated in this thread, and is brought up *regularly* in learn (and even then, only when it actually creates problems). So we brake some code? Yes. Code that was most probably silently wrong to begin with. This is a "good move". The error is verbose and explicit, and the fix is trivial. Let's keep making D safe by default please (!) Let's do everyone a favor and put a definitive *end* to this mess.

I want both to go to return type. Prefix qualifier -> return type. Postfix qualifier -> function type. If qualifier is not applicable to return type, it falls through to function type (as far as I understand the grammar anyway). Simple and obvious. The very idea that prefix qualifier has something to do with function type is confusing and error-prone. And this change makes it much worse.

(In reply to comment #10) > I want both to go to return type. Prefix qualifier -> return type. Postfix > qualifier -> function type. Typ? "Both", then you state two different things. > If qualifier is not applicable to return type, it > falls through to function type (as far as I understand the grammar anyway). AFAIK, anything returnable can be qualified with const, so there would be no fall through case. > Simple and obvious. > > The very idea that prefix qualifier has something to do with function type is > confusing and error-prone. And this change makes it much worse. What about: -------- struct S { const nothrow pure T foo(); nothrow pure const T bar(); } -------- What does (should) foo return? What about bar? The current rules are obvious: stand alone attribute => Function. Simple and obvious. There is no reasons for "const" to get special treatment.

(In reply to comment #4) > I disagree with this change, this is extremely dangerous behavior when > interfacing with C. Take a look at the following: > > ----- > extern(C) const int *foo(); > > void main() > { > *foo() = 1; // compiles > } > ----- > > The proper definition should have been: > > ----- > extern(C) const(int)* foo(); > > void main() > { > *foo() = 1; // fails > } > ----- > > It is very easy to make this mistake, it should not be silent, at least not for > module-scoped functions. > > Alternatively as a compromise I suggest we at least add this check for > extern(C) functions, because this is where this problem can occur very > frequently.

(In reply to comment #12) > (In reply to comment #4) > > I disagree with this change, this is extremely dangerous behavior when > > interfacing with C. Take a look at the following: > > > > [...] Sorry for the empty post, my finger slipped on "commit". I just wanted to add that when C code is copy pasted and *compiles* in D, then the rules say it *must* create the same results during runtime: This is not a problem here, because it *should* still produce the same actual results. However, it is largely *expected* that the C code will have the same meaning and/or semantic, in the one in D. Given that this is not the case, then things should be changed to either mean the same thing (IMO bad idea), or emit a compile time error. The middle ground is an extremely dangerous place to stand.

(In reply to comment #11) > Typ? "Both", then you state two different things. Both free function and method. > AFAIK, anything returnable can be qualified with const, so there would be no > fall through case. I don't know if const(void) is valid, but at least pure is applicable to return type only when it is one of function types so it will fall through. I am not speaking only about const, all attributes should behave consistently. > What about: > > -------- > struct S > { > const nothrow pure > T foo(); > > nothrow pure const > T bar(); > } > -------- > > What does (should) foo return? What about bar? both return const(T) (at the very least, I have no idea what T is). When I am speaking about postfix qualifier I mean "T bar() const pure nothrow" > The current rules are obvious: stand alone attribute => Function. > Simple and obvious. > There is no reasons for "const" to get special treatment. No, they are more like "stand alone attribute -> function or no-op, you'd better be careful!". And breaks similarity with C/C++ principle. And makes your code needlessly "Lisp-y". Behaviour has been very confusing all the time but at least it slapped me into face every time I tried to type stuff in a more natural syntax. Not any more.

(In reply to comment #14) > > What about: > > > > -------- > > struct S > > { > > const nothrow pure > > T foo(); > > > > nothrow pure const > > T bar(); > > } > > -------- > > > > What does (should) foo return? What about bar? > > both return const(T) (at the very least, I have no idea what T is). When I am > speaking about postfix qualifier I mean "T bar() const pure nothrow" Well, such a change would break existing code that is correct and respects the spec. I'm not sure changing the semantics of such code would be acceptable. D allows attributes to be placed either prefix or postfix. Forcing "const" to only work right hand side is a C++ holdover. There should be no difference where you place it. If anything, I find it weird when I find code that is written like: "pure @property T foo() const" Why place the const there? Put it on the left, that's where you placed everything else... > > The current rules are obvious: stand alone attribute => Function. > > Simple and obvious. > > There is no reasons for "const" to get special treatment. > > No, they are more like "stand alone attribute -> function or no-op, you'd > better be careful!". And breaks similarity with C/C++ principle. I agree both are problem. I happen to think the better solution is that no-op becomes explicit compile error. This is both safe, and doesn't add special positional casing. > And makes your > code needlessly "Lisp-y". Don't know lisp. > Behaviour has been very confusing all the time but at least it slapped me into > face every time I tried to type stuff in a more natural syntax. Not any more. I agree we should be slapped in the face every time we type something wrong. Currently, being slapped in the face for wrong postfix is good. Also being slapped for wrong prefix would be better. The solution of saying "prefix applies to return value", would be a big interface change. Even where we to agree it is the right decision, I don't think it could acceptable change.

(In reply to comment #15) > The solution of saying "prefix applies to return value", would be a big > interface change. Even where we to agree it is the right decision, I don't > think it could acceptable change. You have asked what it my opinion about "how it should behave", not about "what can be released" ;) I don't expect this to change, it is a design mistake with roots too ancient to shake it. But I at least want it to remind me every time about it, not fail silently. This pull request was a disaster. Actually, paying even more attention to initial enhancement description I see no valid use case. "const: void foo() {}" - this should be an error, nothing convenient about letting it go. P.S. By "Lisp-y" I have meant excessive () bracket usage.

I opened issue 12967 to improve compiler behavior.