Download Reference Manual
The Developer's Library for D
About Wiki Forums Source Search Contact

Ticket #1333 (new defect)

Opened 16 years ago

Last modified 16 years ago

D2.0: object not const correct

Reported by: thaven Assigned to: keinfarbton
Priority: normal Milestone: 2.0
Component: Tango Version: D2.0 branch
Keywords: Cc: schveiguy

Description

This is exactly the same as http://d.puremagic.com/issues/show_bug.cgi?id=1824 . Of course this should ripple on to all modules where those methods get overridden.

Call it 'the great const cleanup phase 2' ;-)

Attachments

genobj.diff (6.5 kB) - added by thaven on 10/22/08 07:40:30.
Updated partial patch

Change History

10/19/08 21:12:47 changed by keinfarbton

I would, for now, not do that. Even phobos is not that const correct.

However the patch would be incomplete, because much more classes in tango will need adjustments too, i think.

(follow-up: ↓ 3 ) 10/19/08 21:22:45 changed by kris

Yeah, I agree with keinfarbton.

Using "const bool" returns is overkill, IMO. I would expect Tango to make liberal use of the in keyword instead of all that const(type) verbosity, for backward compatibility with D1 and for readability.

(in reply to: ↑ 2 ) 10/20/08 06:28:59 changed by thaven

@kris: please note that the added const before the return type makes it a const function, so it doesn't make the return type 'const bool' for opEquals, but it specifies that the method won't change the object it applies to.

So, this certainly isn't overkill, it is needed to compare const(Object) instances! (If opEquals isn't declared 'const' you may not call it on a const(Object)!) That is probably why it is marked as a critical bug for Phobos.

For the function parameters: i agree it makes sense to make it ' in type ' instead of ' const(type) '. Looks somewhat cleaner.

And I know the patch is incomplete, it is just the very 'basic part' of the patch. As I already said, each override of any of those methods needs to be updated. Of course I am most willing to work on towards a complete patch for this.

10/20/08 14:03:36 changed by schveiguy

  • cc set to schveiguy.

I sort of agree with thaven.

Although I would prefer the trailing const syntax like C++:

bool opEquals(Object o) const

Putting the const first is not as readable.

However, there is a compiler bug which is a blocker for this:

http://d.puremagic.com/issues/show_bug.cgi?id=1645

So even if you declare that opEquals is const, an object can simply override it with a non-const function, and the whole const protection is lost.

I found this bug by trying to do exactly what you asked for. And I recompiled to see all the places I needed to update objects. I was surprised when none showed up!

I'd put this request on hold until at least that bug gets fixed.

And RE: using 'in' instead of 'const'. I believe there is no notation for marking 'this' as in for D1, so AFAIK, there's no way to make a D1-compatible version.

10/20/08 15:58:44 changed by kris

yeah, you're right thaven - don't know what I was thinking.

At this time Tango will need to be D1 compatible because we cannot maintain two versions of the library (along with all the dependent libraries). I suggest petitioning Walter to do the right thing for D1 compatibility.

BTW, it seems to me D2 has lost much of that simplicity that made D1 so attractive.

10/20/08 20:02:25 changed by thaven

Unfortunately it is not possible to have const and D1 compatibility at the same time... And D1 is just cripple because it lacks a way to specify immutability... As a programmer you need to be able to enforce the rules you create. And yes... D2 loses quite a lot of simplicity, and sometimes that feels frustrating, but when thinking about it, for me the conclusion is always that it's all about avoiding bugs. The rules of D2 are strict, sometimes unconfortable, but very effective.

But I must note: I am such a guy who always says that what should not be done, should also be impossible or at least difficult to do. D2 enables me to really do things that way, so most of the time the compiler will stop me when I make some stupid mistake...

As a runtime library, Tango must fully follow the rules of D2, because if it doesn't, it would not expose the full power of D2. For example, when the DMD bug pointed out by schveiguy is fixed and Tango's implementation of Object is still as it is, one cannot override opEquals with a 'const' version, so it becomes impossible to do things the right way!

Having said that, I'd like to add that Tango will need to move on to D2 some day, and Tango for D2 can't be compatible with D1, that's simply impossible... So please let's do what is needed in the D2 branch, it'll need to be done anyway... But still, what schveiguy proposes (wait for the fix of that DMD bug) makes sense. But I'd say: don't wait much longer.

10/20/08 20:30:01 changed by kris

  • priority changed from critical to normal.

D1 could easily support the syntactic elements of D2, and then ignore them. That's why I suggest you petition Walter instead of placing the onus on Tango to choose one or the other

10/21/08 06:19:46 changed by thaven

Apart from being a severe pollution of D1, such would introduce confusion about the meaning of the 'const' keyword when used as a storage class.

@schveiguy: that C++ like syntax, is it currently supported by D2? I couldn't find it in the docs...

10/21/08 13:57:31 changed by schveiguy

Yes, it is supported. Look at http://www.digitalmars.com/d/2.0/const3.html search for Invariant Member Functions, then look at the second code block.

10/22/08 07:40:30 changed by thaven

  • attachment genobj.diff added.

Updated partial patch