Issue 1824 - Object not const correct #72

Merged
merged 6 commits into from Jul 9, 2012

Projects

None yet

5 participants

@9rnsr
D Programming Language member

http://d.puremagic.com/issues/show_bug.cgi?id=1824
dlang/dmd#387
#72
dlang/phobos#262

  1. Change Object.opEquals, opCmp, toHash and toString to const member functions.
  2. Change using TypeInfo objects to const.
@yebblies
D Programming Language member

You're missing a const in a couple of opApply signatures: http://yebblies.com/results/dmd_387_9rnsr_constApply_druntimebuild.txt

@jmdavis
D Programming Language member

Personally, I'd argue that all of those consts should go after the functions that they're on. Putting them in front risks confusion, since then it looks like they're marking the return types and not the functions.

@jmdavis
D Programming Language member

Another thing to consider (though it shouldn't be dealt with in this pull request regardless) is whether some of Object's functions should be pure as well. I'm not sure if we can for sure or not, but their lack of purity could be a problem. If opEquals isn't pure, can objects be compared for equality within a pure function? And not being to use toString in a pure function could be really annoying (though as long as format isn't pure, there's no way that toString could be). To some extent, the problem is solved by the fact that you can override Object's functions with versions with stricter attributes, so as long as you do that and specifically use the derived type, then you should be okay. But if it's actually reasonably to require that functions such as toString be pure, then we should do it. The question though is whether that's reasonable or not. I should probably bring it up in the newsgroup. Regardless, it's not something that should be dealt with in this pull request.

@jmdavis
D Programming Language member

Another thing, we need to have non-const versions of Object's const functions, so that those who want caching and the like in those functions can have it with non-const objects.

And again, I would strongly argue that const should go after the functions and not before. It's bad enough that it's even legal D to put it in front. It's not something that we should be doing.

@9rnsr
D Programming Language member

Hmm, I had not focused so much that ambiguity, but also I can agree to your point.
OK. I'll fix it after a while.

@9rnsr
D Programming Language member

Change prefix const to suffix.
And add feature for opEquals migration. See discussion in dlang/phobos#262

@9rnsr
D Programming Language member

Updated for interface and typedef'ed class/interface comparison for fixing issue 4088.

@andralex
D Programming Language member

rebase? I'll also pull the Phobos request.

@9rnsr
D Programming Language member

Rebased.

@jmdavis
D Programming Language member

According to the pull tester, these changes are currently failing.

@andralex andralex was assigned Jul 8, 2012
@andralex
D Programming Language member

Yah, @9rnsr - this seems to be failing, and is out of sync again. Could you please take a look? I'll be back on this in a week if not earlier. Thanks!

@9rnsr
D Programming Language member

OK. Rebased and fixed for git head.

@LightBender

@9rnsr, this needs another rebase, we've been doing merges all day and something got changed in object_.d

@9rnsr
D Programming Language member

Thanks for the info. Re-rebasing is done.

@andralex andralex merged commit d39bd49 into dlang:master Jul 9, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment