You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by "Uwe Schindler (JIRA)" <ji...@apache.org> on 2009/10/21 10:46:59 UTC
[jira] Created: (LUCENE-2000) Use covariant clone() return types
Use covariant clone() return types
----------------------------------
Key: LUCENE-2000
URL: https://issues.apache.org/jira/browse/LUCENE-2000
Project: Lucene - Java
Issue Type: Task
Affects Versions: 3.0
Reporter: Uwe Schindler
Attachments: LUCENE-2000-clone_covariance.patch
*Paul Cown wrote in LUCENE-1257:*
OK, thought I'd jump in and help out here with one of my Java 5 favourites. Haven't seen anyone discuss this, and don't believe any of the patches address this, so thought I'd throw a patch out there (against SVN HEAD @ revision 827821) which uses Java 5 covariant return types for (almost) all of the Object#clone() implementations in core.
i.e. this:
public Object clone() {
changes to:
public SpanNotQuery clone() {
which lets us get rid of a whole bunch of now-unnecessary casts, so e.g.
if (clone == null) clone = (SpanNotQuery) this.clone();
becomes
if (clone == null) clone = this.clone();
Almost everything has been done and all downcasts removed, in core, with the exception of
Some SpanQuery stuff, where it's assumed that it's safe to cast the clone() of a SpanQuery to a SpanQuery - this can't be made covariant without declaring "abstract SpanQuery clone()" in SpanQuery itself, which breaks those SpanQuerys that don't declare their own clone()
Some IndexReaders, e.g. DirectoryReader - we can't be more specific than changing .clone() to return IndexReader, because it returns the result of IndexReader.clone(boolean). We could use covariant types for THAT, which would work fine, but that didn't follow the pattern of the others so that could be a later commit.
Two changes were also made in contrib/, where not making the changes would have broken code by trying to widen IndexInput#clone() back out to returning Object, which is not permitted. contrib/ was otherwise left untouched.
Let me know what you think, or if you have any other questions.
[ Show » ] Paul Cowan added a comment - 21/Oct/09 03:01 AM OK, thought I'd jump in and help out here with one of my Java 5 favourites. Haven't seen anyone discuss this, and don't believe any of the patches address this, so thought I'd throw a patch out there (against SVN HEAD @ revision 827821) which uses Java 5 covariant return types for (almost) all of the Object#clone() implementations in core. i.e. this: public Object clone() { changes to: public SpanNotQuery clone() { which lets us get rid of a whole bunch of now-unnecessary casts, so e.g. if (clone == null) clone = (SpanNotQuery) this.clone(); becomes if (clone == null) clone = this.clone(); Almost everything has been done and all downcasts removed, in core, with the exception of
Some SpanQuery stuff, where it's assumed that it's safe to cast the clone() of a SpanQuery to a SpanQuery - this can't be made covariant without declaring "abstract SpanQuery clone()" in SpanQuery itself, which breaks those SpanQuerys that don't declare their own clone()
Some IndexReaders, e.g. DirectoryReader - we can't be more specific than changing .clone() to return IndexReader, because it returns the result of IndexReader.clone(boolean). We could use covariant types for THAT, which would work fine, but that didn't follow the pattern of the others so that could be a later commit.
Two changes were also made in contrib/, where not making the changes would have broken code by trying to widen IndexInput#clone() back out to returning Object, which is not permitted. contrib/ was otherwise left untouched. Let me know what you think, or if you have any other questions.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-2000) Use covariant clone() return types
Posted by "Uwe Schindler (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-2000?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12768141#action_12768141 ]
Uwe Schindler commented on LUCENE-2000:
---------------------------------------
I moved this to an extra issue, because there is some discussion needed.
I am strongly against this for various reasons:
- Java 5 itsself does not override clone() with covariant return type (nowhere!). So e.g. String.clone() always returns jl.Object.
- This is because of backwards problems (which are not easy to explain) -- it has something to do, if a subclass compiled against Java 1.4 version of Lucene overrides clone and calls super.clone(). Because of this, the JDK does not provide String.clone() retrurning String. javac does its best to prevent problems here, but for APIs that need to be backwards compatible, it should return Object as always.
- Covariant clone return types need, that *all* subclasses of a class, that originally implemented a covariant clone() also override it covariant to be consistent. And because of this you have consistency problems (see your IndexReader problem). This is not possible for backwards compatibility. Because of this, covariant clone should only be done for internal classes (package-private, private) or final classes. Another example of this problem is AttributeImpl which defines an abstract clone method. Subclasses would need to override this covariant clone() method. Custom Attributes compiled against Lucene 2.9 would fail to do this -> MethodNotFoundException (I tried it out, it breaks)
Because of all this problems, I prefer to always cast the return value of clone(). This is not unsafe (and because of this you get no unchecked warning), because you always know how to cast the clone result. By the way: You still have to always clone() the super.clone() call, so you do not get any pros of using covariant return types.
I do not want to start a flame war here, but we should not do this.
> Use covariant clone() return types
> ----------------------------------
>
> Key: LUCENE-2000
> URL: https://issues.apache.org/jira/browse/LUCENE-2000
> Project: Lucene - Java
> Issue Type: Task
> Affects Versions: 3.0
> Reporter: Uwe Schindler
> Priority: Minor
> Attachments: LUCENE-2000-clone_covariance.patch
>
>
> *Paul Cowan wrote in LUCENE-1257:*
> OK, thought I'd jump in and help out here with one of my Java 5 favourites. Haven't seen anyone discuss this, and don't believe any of the patches address this, so thought I'd throw a patch out there (against SVN HEAD @ revision 827821) which uses Java 5 covariant return types for (almost) all of the Object#clone() implementations in core.
> i.e. this:
> public Object clone() {
> changes to:
> public SpanNotQuery clone() {
> which lets us get rid of a whole bunch of now-unnecessary casts, so e.g.
> if (clone == null) clone = (SpanNotQuery) this.clone();
> becomes
> if (clone == null) clone = this.clone();
> Almost everything has been done and all downcasts removed, in core, with the exception of
> Some SpanQuery stuff, where it's assumed that it's safe to cast the clone() of a SpanQuery to a SpanQuery - this can't be made covariant without declaring "abstract SpanQuery clone()" in SpanQuery itself, which breaks those SpanQuerys that don't declare their own clone()
> Some IndexReaders, e.g. DirectoryReader - we can't be more specific than changing .clone() to return IndexReader, because it returns the result of IndexReader.clone(boolean). We could use covariant types for THAT, which would work fine, but that didn't follow the pattern of the others so that could be a later commit.
> Two changes were also made in contrib/, where not making the changes would have broken code by trying to widen IndexInput#clone() back out to returning Object, which is not permitted. contrib/ was otherwise left untouched.
> Let me know what you think, or if you have any other questions.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Issue Comment Edited: (LUCENE-2000) Use covariant clone()
return types
Posted by "Uwe Schindler (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-2000?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12768141#action_12768141 ]
Uwe Schindler edited comment on LUCENE-2000 at 10/21/09 9:14 AM:
-----------------------------------------------------------------
I moved this to an extra issue, because there is some discussion needed.
I am strongly against this for various reasons:
- Java 5 itsself does not override clone() with covariant return type (nowhere!). So e.g. String.clone() always returns jl.Object.
- This is because of backwards problems (which are not easy to explain) -- it has something to do, if a subclass compiled against Java 1.4 version of Lucene overrides clone and calls super.clone(). Because of this, the JDK does not provide String.clone() retrurning String. javac does its best to prevent problems here, but for APIs that need to be backwards compatible, it should return Object as always.
- Covariant clone return types need, that *all* subclasses of a class, that originally implemented a covariant clone() also override it covariant to be consistent. And because of this you have consistency problems (see your IndexReader problem). This is not possible for backwards compatibility. Because of this, covariant clone should only be done for internal classes (package-private, private) or final classes. Another example of this problem is AttributeImpl which defines a clone() method. Subclasses would need to override this covariant clone() method. Custom Attributes compiled against Lucene 2.9 would fail to do this -> MethodNotFoundException (I tried it out, it breaks)
Because of all this problems, I prefer to always cast the return value of clone(). This is not unsafe (and because of this you get no unchecked warning), because you always know how to cast the clone result. By the way: You still have to always clone() the super.clone() call, so you do not get any pros of using covariant return types.
I do not want to start a flame war here, but we should not do this.
was (Author: thetaphi):
I moved this to an extra issue, because there is some discussion needed.
I am strongly against this for various reasons:
- Java 5 itsself does not override clone() with covariant return type (nowhere!). So e.g. String.clone() always returns jl.Object.
- This is because of backwards problems (which are not easy to explain) -- it has something to do, if a subclass compiled against Java 1.4 version of Lucene overrides clone and calls super.clone(). Because of this, the JDK does not provide String.clone() retrurning String. javac does its best to prevent problems here, but for APIs that need to be backwards compatible, it should return Object as always.
- Covariant clone return types need, that *all* subclasses of a class, that originally implemented a covariant clone() also override it covariant to be consistent. And because of this you have consistency problems (see your IndexReader problem). This is not possible for backwards compatibility. Because of this, covariant clone should only be done for internal classes (package-private, private) or final classes. Another example of this problem is AttributeImpl which defines an abstract clone method. Subclasses would need to override this covariant clone() method. Custom Attributes compiled against Lucene 2.9 would fail to do this -> MethodNotFoundException (I tried it out, it breaks)
Because of all this problems, I prefer to always cast the return value of clone(). This is not unsafe (and because of this you get no unchecked warning), because you always know how to cast the clone result. By the way: You still have to always clone() the super.clone() call, so you do not get any pros of using covariant return types.
I do not want to start a flame war here, but we should not do this.
> Use covariant clone() return types
> ----------------------------------
>
> Key: LUCENE-2000
> URL: https://issues.apache.org/jira/browse/LUCENE-2000
> Project: Lucene - Java
> Issue Type: Task
> Affects Versions: 3.0
> Reporter: Uwe Schindler
> Priority: Minor
> Attachments: LUCENE-2000-clone_covariance.patch
>
>
> *Paul Cowan wrote in LUCENE-1257:*
> OK, thought I'd jump in and help out here with one of my Java 5 favourites. Haven't seen anyone discuss this, and don't believe any of the patches address this, so thought I'd throw a patch out there (against SVN HEAD @ revision 827821) which uses Java 5 covariant return types for (almost) all of the Object#clone() implementations in core.
> i.e. this:
> public Object clone() {
> changes to:
> public SpanNotQuery clone() {
> which lets us get rid of a whole bunch of now-unnecessary casts, so e.g.
> if (clone == null) clone = (SpanNotQuery) this.clone();
> becomes
> if (clone == null) clone = this.clone();
> Almost everything has been done and all downcasts removed, in core, with the exception of
> Some SpanQuery stuff, where it's assumed that it's safe to cast the clone() of a SpanQuery to a SpanQuery - this can't be made covariant without declaring "abstract SpanQuery clone()" in SpanQuery itself, which breaks those SpanQuerys that don't declare their own clone()
> Some IndexReaders, e.g. DirectoryReader - we can't be more specific than changing .clone() to return IndexReader, because it returns the result of IndexReader.clone(boolean). We could use covariant types for THAT, which would work fine, but that didn't follow the pattern of the others so that could be a later commit.
> Two changes were also made in contrib/, where not making the changes would have broken code by trying to widen IndexInput#clone() back out to returning Object, which is not permitted. contrib/ was otherwise left untouched.
> Let me know what you think, or if you have any other questions.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Commented: (LUCENE-2000) Use covariant clone() return types
Posted by "Earwin Burrfoot (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-2000?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12844130#action_12844130 ]
Earwin Burrfoot commented on LUCENE-2000:
-----------------------------------------
I believe we should do this at our next "we're breaking backcompat" release.
Any compile errors that could bring to clients are fixed like, exceptionally easy. And the code comes out cleaner -> we have way more callees of clone() than overrides.
> Use covariant clone() return types
> ----------------------------------
>
> Key: LUCENE-2000
> URL: https://issues.apache.org/jira/browse/LUCENE-2000
> Project: Lucene - Java
> Issue Type: Task
> Affects Versions: 3.0
> Reporter: Uwe Schindler
> Priority: Minor
> Attachments: LUCENE-2000-clone_covariance.patch
>
>
> *Paul Cowan wrote in LUCENE-1257:*
> OK, thought I'd jump in and help out here with one of my Java 5 favourites. Haven't seen anyone discuss this, and don't believe any of the patches address this, so thought I'd throw a patch out there (against SVN HEAD @ revision 827821) which uses Java 5 covariant return types for (almost) all of the Object#clone() implementations in core.
> i.e. this:
> public Object clone() {
> changes to:
> public SpanNotQuery clone() {
> which lets us get rid of a whole bunch of now-unnecessary casts, so e.g.
> if (clone == null) clone = (SpanNotQuery) this.clone();
> becomes
> if (clone == null) clone = this.clone();
> Almost everything has been done and all downcasts removed, in core, with the exception of
> Some SpanQuery stuff, where it's assumed that it's safe to cast the clone() of a SpanQuery to a SpanQuery - this can't be made covariant without declaring "abstract SpanQuery clone()" in SpanQuery itself, which breaks those SpanQuerys that don't declare their own clone()
> Some IndexReaders, e.g. DirectoryReader - we can't be more specific than changing .clone() to return IndexReader, because it returns the result of IndexReader.clone(boolean). We could use covariant types for THAT, which would work fine, but that didn't follow the pattern of the others so that could be a later commit.
> Two changes were also made in contrib/, where not making the changes would have broken code by trying to widen IndexInput#clone() back out to returning Object, which is not permitted. contrib/ was otherwise left untouched.
> Let me know what you think, or if you have any other questions.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Updated: (LUCENE-2000) Use covariant clone() return types
Posted by "Uwe Schindler (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-2000?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Uwe Schindler updated LUCENE-2000:
----------------------------------
Description:
*Paul Cown wrote in LUCENE-1257:*
OK, thought I'd jump in and help out here with one of my Java 5 favourites. Haven't seen anyone discuss this, and don't believe any of the patches address this, so thought I'd throw a patch out there (against SVN HEAD @ revision 827821) which uses Java 5 covariant return types for (almost) all of the Object#clone() implementations in core.
i.e. this:
public Object clone() {
changes to:
public SpanNotQuery clone() {
which lets us get rid of a whole bunch of now-unnecessary casts, so e.g.
if (clone == null) clone = (SpanNotQuery) this.clone();
becomes
if (clone == null) clone = this.clone();
Almost everything has been done and all downcasts removed, in core, with the exception of
Some SpanQuery stuff, where it's assumed that it's safe to cast the clone() of a SpanQuery to a SpanQuery - this can't be made covariant without declaring "abstract SpanQuery clone()" in SpanQuery itself, which breaks those SpanQuerys that don't declare their own clone()
Some IndexReaders, e.g. DirectoryReader - we can't be more specific than changing .clone() to return IndexReader, because it returns the result of IndexReader.clone(boolean). We could use covariant types for THAT, which would work fine, but that didn't follow the pattern of the others so that could be a later commit.
Two changes were also made in contrib/, where not making the changes would have broken code by trying to widen IndexInput#clone() back out to returning Object, which is not permitted. contrib/ was otherwise left untouched.
Let me know what you think, or if you have any other questions.
was:
*Paul Cown wrote in LUCENE-1257:*
OK, thought I'd jump in and help out here with one of my Java 5 favourites. Haven't seen anyone discuss this, and don't believe any of the patches address this, so thought I'd throw a patch out there (against SVN HEAD @ revision 827821) which uses Java 5 covariant return types for (almost) all of the Object#clone() implementations in core.
i.e. this:
public Object clone() {
changes to:
public SpanNotQuery clone() {
which lets us get rid of a whole bunch of now-unnecessary casts, so e.g.
if (clone == null) clone = (SpanNotQuery) this.clone();
becomes
if (clone == null) clone = this.clone();
Almost everything has been done and all downcasts removed, in core, with the exception of
Some SpanQuery stuff, where it's assumed that it's safe to cast the clone() of a SpanQuery to a SpanQuery - this can't be made covariant without declaring "abstract SpanQuery clone()" in SpanQuery itself, which breaks those SpanQuerys that don't declare their own clone()
Some IndexReaders, e.g. DirectoryReader - we can't be more specific than changing .clone() to return IndexReader, because it returns the result of IndexReader.clone(boolean). We could use covariant types for THAT, which would work fine, but that didn't follow the pattern of the others so that could be a later commit.
Two changes were also made in contrib/, where not making the changes would have broken code by trying to widen IndexInput#clone() back out to returning Object, which is not permitted. contrib/ was otherwise left untouched.
Let me know what you think, or if you have any other questions.
[ Show » ] Paul Cowan added a comment - 21/Oct/09 03:01 AM OK, thought I'd jump in and help out here with one of my Java 5 favourites. Haven't seen anyone discuss this, and don't believe any of the patches address this, so thought I'd throw a patch out there (against SVN HEAD @ revision 827821) which uses Java 5 covariant return types for (almost) all of the Object#clone() implementations in core. i.e. this: public Object clone() { changes to: public SpanNotQuery clone() { which lets us get rid of a whole bunch of now-unnecessary casts, so e.g. if (clone == null) clone = (SpanNotQuery) this.clone(); becomes if (clone == null) clone = this.clone(); Almost everything has been done and all downcasts removed, in core, with the exception of
Some SpanQuery stuff, where it's assumed that it's safe to cast the clone() of a SpanQuery to a SpanQuery - this can't be made covariant without declaring "abstract SpanQuery clone()" in SpanQuery itself, which breaks those SpanQuerys that don't declare their own clone()
Some IndexReaders, e.g. DirectoryReader - we can't be more specific than changing .clone() to return IndexReader, because it returns the result of IndexReader.clone(boolean). We could use covariant types for THAT, which would work fine, but that didn't follow the pattern of the others so that could be a later commit.
Two changes were also made in contrib/, where not making the changes would have broken code by trying to widen IndexInput#clone() back out to returning Object, which is not permitted. contrib/ was otherwise left untouched. Let me know what you think, or if you have any other questions.
> Use covariant clone() return types
> ----------------------------------
>
> Key: LUCENE-2000
> URL: https://issues.apache.org/jira/browse/LUCENE-2000
> Project: Lucene - Java
> Issue Type: Task
> Affects Versions: 3.0
> Reporter: Uwe Schindler
> Attachments: LUCENE-2000-clone_covariance.patch
>
>
> *Paul Cown wrote in LUCENE-1257:*
> OK, thought I'd jump in and help out here with one of my Java 5 favourites. Haven't seen anyone discuss this, and don't believe any of the patches address this, so thought I'd throw a patch out there (against SVN HEAD @ revision 827821) which uses Java 5 covariant return types for (almost) all of the Object#clone() implementations in core.
> i.e. this:
> public Object clone() {
> changes to:
> public SpanNotQuery clone() {
> which lets us get rid of a whole bunch of now-unnecessary casts, so e.g.
> if (clone == null) clone = (SpanNotQuery) this.clone();
> becomes
> if (clone == null) clone = this.clone();
> Almost everything has been done and all downcasts removed, in core, with the exception of
> Some SpanQuery stuff, where it's assumed that it's safe to cast the clone() of a SpanQuery to a SpanQuery - this can't be made covariant without declaring "abstract SpanQuery clone()" in SpanQuery itself, which breaks those SpanQuerys that don't declare their own clone()
> Some IndexReaders, e.g. DirectoryReader - we can't be more specific than changing .clone() to return IndexReader, because it returns the result of IndexReader.clone(boolean). We could use covariant types for THAT, which would work fine, but that didn't follow the pattern of the others so that could be a later commit.
> Two changes were also made in contrib/, where not making the changes would have broken code by trying to widen IndexInput#clone() back out to returning Object, which is not permitted. contrib/ was otherwise left untouched.
> Let me know what you think, or if you have any other questions.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Updated: (LUCENE-2000) Use covariant clone() return types
Posted by "Uwe Schindler (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-2000?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Uwe Schindler updated LUCENE-2000:
----------------------------------
Description:
*Paul Cowan wrote in LUCENE-1257:*
OK, thought I'd jump in and help out here with one of my Java 5 favourites. Haven't seen anyone discuss this, and don't believe any of the patches address this, so thought I'd throw a patch out there (against SVN HEAD @ revision 827821) which uses Java 5 covariant return types for (almost) all of the Object#clone() implementations in core.
i.e. this:
public Object clone() {
changes to:
public SpanNotQuery clone() {
which lets us get rid of a whole bunch of now-unnecessary casts, so e.g.
if (clone == null) clone = (SpanNotQuery) this.clone();
becomes
if (clone == null) clone = this.clone();
Almost everything has been done and all downcasts removed, in core, with the exception of
Some SpanQuery stuff, where it's assumed that it's safe to cast the clone() of a SpanQuery to a SpanQuery - this can't be made covariant without declaring "abstract SpanQuery clone()" in SpanQuery itself, which breaks those SpanQuerys that don't declare their own clone()
Some IndexReaders, e.g. DirectoryReader - we can't be more specific than changing .clone() to return IndexReader, because it returns the result of IndexReader.clone(boolean). We could use covariant types for THAT, which would work fine, but that didn't follow the pattern of the others so that could be a later commit.
Two changes were also made in contrib/, where not making the changes would have broken code by trying to widen IndexInput#clone() back out to returning Object, which is not permitted. contrib/ was otherwise left untouched.
Let me know what you think, or if you have any other questions.
was:
*Paul Cown wrote in LUCENE-1257:*
OK, thought I'd jump in and help out here with one of my Java 5 favourites. Haven't seen anyone discuss this, and don't believe any of the patches address this, so thought I'd throw a patch out there (against SVN HEAD @ revision 827821) which uses Java 5 covariant return types for (almost) all of the Object#clone() implementations in core.
i.e. this:
public Object clone() {
changes to:
public SpanNotQuery clone() {
which lets us get rid of a whole bunch of now-unnecessary casts, so e.g.
if (clone == null) clone = (SpanNotQuery) this.clone();
becomes
if (clone == null) clone = this.clone();
Almost everything has been done and all downcasts removed, in core, with the exception of
Some SpanQuery stuff, where it's assumed that it's safe to cast the clone() of a SpanQuery to a SpanQuery - this can't be made covariant without declaring "abstract SpanQuery clone()" in SpanQuery itself, which breaks those SpanQuerys that don't declare their own clone()
Some IndexReaders, e.g. DirectoryReader - we can't be more specific than changing .clone() to return IndexReader, because it returns the result of IndexReader.clone(boolean). We could use covariant types for THAT, which would work fine, but that didn't follow the pattern of the others so that could be a later commit.
Two changes were also made in contrib/, where not making the changes would have broken code by trying to widen IndexInput#clone() back out to returning Object, which is not permitted. contrib/ was otherwise left untouched.
Let me know what you think, or if you have any other questions.
Priority: Minor (was: Major)
> Use covariant clone() return types
> ----------------------------------
>
> Key: LUCENE-2000
> URL: https://issues.apache.org/jira/browse/LUCENE-2000
> Project: Lucene - Java
> Issue Type: Task
> Affects Versions: 3.0
> Reporter: Uwe Schindler
> Priority: Minor
> Attachments: LUCENE-2000-clone_covariance.patch
>
>
> *Paul Cowan wrote in LUCENE-1257:*
> OK, thought I'd jump in and help out here with one of my Java 5 favourites. Haven't seen anyone discuss this, and don't believe any of the patches address this, so thought I'd throw a patch out there (against SVN HEAD @ revision 827821) which uses Java 5 covariant return types for (almost) all of the Object#clone() implementations in core.
> i.e. this:
> public Object clone() {
> changes to:
> public SpanNotQuery clone() {
> which lets us get rid of a whole bunch of now-unnecessary casts, so e.g.
> if (clone == null) clone = (SpanNotQuery) this.clone();
> becomes
> if (clone == null) clone = this.clone();
> Almost everything has been done and all downcasts removed, in core, with the exception of
> Some SpanQuery stuff, where it's assumed that it's safe to cast the clone() of a SpanQuery to a SpanQuery - this can't be made covariant without declaring "abstract SpanQuery clone()" in SpanQuery itself, which breaks those SpanQuerys that don't declare their own clone()
> Some IndexReaders, e.g. DirectoryReader - we can't be more specific than changing .clone() to return IndexReader, because it returns the result of IndexReader.clone(boolean). We could use covariant types for THAT, which would work fine, but that didn't follow the pattern of the others so that could be a later commit.
> Two changes were also made in contrib/, where not making the changes would have broken code by trying to widen IndexInput#clone() back out to returning Object, which is not permitted. contrib/ was otherwise left untouched.
> Let me know what you think, or if you have any other questions.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org
[jira] Updated: (LUCENE-2000) Use covariant clone() return types
Posted by "Uwe Schindler (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/LUCENE-2000?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Uwe Schindler updated LUCENE-2000:
----------------------------------
Attachment: LUCENE-2000-clone_covariance.patch
> Use covariant clone() return types
> ----------------------------------
>
> Key: LUCENE-2000
> URL: https://issues.apache.org/jira/browse/LUCENE-2000
> Project: Lucene - Java
> Issue Type: Task
> Affects Versions: 3.0
> Reporter: Uwe Schindler
> Attachments: LUCENE-2000-clone_covariance.patch
>
>
> *Paul Cown wrote in LUCENE-1257:*
> OK, thought I'd jump in and help out here with one of my Java 5 favourites. Haven't seen anyone discuss this, and don't believe any of the patches address this, so thought I'd throw a patch out there (against SVN HEAD @ revision 827821) which uses Java 5 covariant return types for (almost) all of the Object#clone() implementations in core.
> i.e. this:
> public Object clone() {
> changes to:
> public SpanNotQuery clone() {
> which lets us get rid of a whole bunch of now-unnecessary casts, so e.g.
> if (clone == null) clone = (SpanNotQuery) this.clone();
> becomes
> if (clone == null) clone = this.clone();
> Almost everything has been done and all downcasts removed, in core, with the exception of
> Some SpanQuery stuff, where it's assumed that it's safe to cast the clone() of a SpanQuery to a SpanQuery - this can't be made covariant without declaring "abstract SpanQuery clone()" in SpanQuery itself, which breaks those SpanQuerys that don't declare their own clone()
> Some IndexReaders, e.g. DirectoryReader - we can't be more specific than changing .clone() to return IndexReader, because it returns the result of IndexReader.clone(boolean). We could use covariant types for THAT, which would work fine, but that didn't follow the pattern of the others so that could be a later commit.
> Two changes were also made in contrib/, where not making the changes would have broken code by trying to widen IndexInput#clone() back out to returning Object, which is not permitted. contrib/ was otherwise left untouched.
> Let me know what you think, or if you have any other questions.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: java-dev-help@lucene.apache.org