Fix cloning inheritance

[IndrajeetPatil]

Closes #247 Closes #165

Example

library(R6)

Creature <- R6Class("Creature", cloneable = TRUE)

Sheep <- R6Class("Sheep", inherit = Creature, cloneable = TRUE)
sheep <- Sheep$new()
class(sheep$clone())
#> [1] "Sheep"    "Creature" "R6"

Human <- R6Class("Human", inherit = Creature, cloneable = FALSE)
human <- Human$new()
human$clone()
#> Error in eval(expr, envir, enclos): attempt to apply non-function

Creature <- R6Class("Creature", cloneable = FALSE)

Sheep <- R6Class("Sheep", inherit = Creature, cloneable = TRUE)
sheep <- Sheep$new()
#> Subclass wants to allow cloning, but superclass has turned it off. Therefore, cloning will also be turned off for subclass.
class(sheep$clone())
#> Error in eval(expr, envir, enclos): attempt to apply non-function

Human <- R6Class("Human", inherit = Creature, cloneable = FALSE)
human <- Human$new()
human$clone()
#> Error in eval(expr, envir, enclos): attempt to apply non-function

^{Created on 2022-10-27 with reprex v2.0.2}

[wch]

If I understand correctly, this will change the default behavior so that if someone creates a subclass and does not explicitly specify a value for cloneable, then:

• Previously, the subclass would not be cloneable.
• With this change, the subclass would be cloneable.

Am I understanding this correctly?

[IndrajeetPatil]

That's correct.

If cloneable is not specified for superclass, then the default behaviour remains the same.

library(R6)

Creature <- R6Class("Creature")
Sheep <- R6Class("Sheep", inherit = Creature)
Sheep$new()
#> <Sheep>
#>   Inherits from: <Creature>
#>   Public:
#>     clone: function (deep = FALSE)

But, if it is set to FALSE, then the default behaviour changes with this PR. Now, the subclass would override superclass setting because there is a mismatch.

Creature <- R6Class("Creature", cloneable = FALSE)
Sheep <- R6Class("Sheep", inherit = Creature)
Sheep$new()
#> Sub and superclass have different cloneable properties. Subclass property will override superclass property.
#> <Sheep>
#>   Inherits from: <Creature>
#>   Public:
#>     clone: function (deep = FALSE)

^{Created on 2022-10-26 with reprex v2.0.2}

I'd run a revdepcheck to see how many deps are affected by this, but {R6} has many deps, and I don't think my desktop can handle it.

I am open to suggestions on how to let the subclass override subclass cloning property when there is a mismatch, but in a backward-compatible manner.

[wch]

Some thoughts:

• When the superclass has cloneable=F, it is dangerous for a subclass to have cloneable=T. The option cloneable=FALSE must be set explicitly, and I'd expect that the author of the superclass would do this for a good reason -- for example, the object may represent some external resource, or it may contain reference objects.
• When the superclass has cloneable=T, it can make sense for a subclass to have cloneable=FALSE.
• It would be better if the subclass inherited the cloneable property by default, instead of defaulting to TRUE. I'm not sure of the best way to do this right offhand. Maybe with some sort of sentinel value.

[IndrajeetPatil]

@wch I've addressed the first two points.

• When super has turned off cloning, cloning will always be off for subclasses, irrespective of what cloneable is set to.
• If super allow cloning, then the cloneable property set for sub will be respected.

library(R6)

# superclass non-cloneable  ---------------------

Creature <- R6Class("Creature", cloneable = FALSE)

Sheep <- R6Class("Sheep", inherit = Creature, cloneable = TRUE)
sheep <- Sheep$new()
#> Subclass wants to allow cloning, but superclass has turned it off. Therefore, cloning will also be turned off for subclass.
class(sheep$clone())
#> Error in eval(expr, envir, enclos): attempt to apply non-function

Human <- R6Class("Human", inherit = Creature, cloneable = FALSE)
human <- Human$new()
human$clone()
#> Error in eval(expr, envir, enclos): attempt to apply non-function

# superclass cloneable  ---------------------

Creature <- R6Class("Creature", cloneable = TRUE)

Sheep <- R6Class("Sheep", inherit = Creature, cloneable = TRUE)
sheep <- Sheep$new()
class(sheep$clone())
#> [1] "Sheep"    "Creature" "R6"

Human <- R6Class("Human", inherit = Creature, cloneable = FALSE)
human <- Human$new()
human$clone()
#> Error in eval(expr, envir, enclos): attempt to apply non-function

^{Created on 2022-10-27 with reprex v2.0.2}

Addressing the third point in this PR feels like expanding its scope. I am also hesitant to work on it because I am not sure what the expected API change looks like (should default be NULL, FALSE, NA, etc.?), or what impact it might have.

[wch]

I think that the way you addressed the first two points actually solves the third point.