Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deque#dup and #clone return the wrong type if subclassed #9874

Closed
HertzDevil opened this issue Nov 3, 2020 · 2 comments
Closed

Deque#dup and #clone return the wrong type if subclassed #9874

HertzDevil opened this issue Nov 3, 2020 · 2 comments
Labels
good first issue This is an issue suited for newcomers to become aquianted with working on the codebase. kind:bug topic:stdlib:collection

Comments

@HertzDevil
Copy link
Contributor

HertzDevil commented Nov 3, 2020

I searched the 0.35.1 standard library docs and found that Deque also suffers from this same issue as Array (#9753) and Hash (#9754):

class Deque(T)
  def dup
    Deque(T).new(size) { |i| self[i].as(T) }
  end

  def clone
    Deque(T).new(size) { |i| self[i].clone.as(T) }
  end
end

The remaining types without a covariant #clone are Regex, String, and value types. They aren't meant to be subclassed so Deque is all that's left.

For #dup there is also OpenSSL::Digest, but likewise I believe that class shouldn't be subclassed.

@HertzDevil HertzDevil changed the title Deque#clone returns the wrong type if subclassed Deque#dup and #clone return the wrong type if subclassed Nov 3, 2020
@straight-shoota straight-shoota added good first issue This is an issue suited for newcomers to become aquianted with working on the codebase. kind:bug topic:stdlib:collection labels Nov 3, 2020
@vlazar
Copy link
Contributor

vlazar commented Nov 22, 2020

Probably both #9874 and #9753 shouldn't have community:newcomer labels.

I wanted to fix Deque#dup and Deque#clone how I did it in #9871 but it looks even worse than an issue appeared in #9753 (comment)

$ git diff
diff --git a/spec/std/deque_spec.cr b/spec/std/deque_spec.cr
index ea6d73052..2e2595294 100644
--- a/spec/std/deque_spec.cr
+++ b/spec/std/deque_spec.cr
@@ -36,6 +36,9 @@ end

 private alias RecursiveDeque = Deque(RecursiveDeque)

+private class DequeSubclass(T) < Deque(T)
+end
+
 describe "Deque" do
   describe "implementation" do
     it "works the same as array" do

ERROR:

$ bin/crystal spec spec/std/deque_spec.cr
Using compiled compiler at .build/crystal
Module validation failed: Function return type does not match operand type of return inst!
  ret i32* %1, !dbg !46
 %"Deque(/Users/vlazar/pet/PR/crystal/spec/std/deque_spec.cr::RecursiveDeque)"* (Exception)
  from src/llvm/module.cr:80:9 in 'verify'
  from src/compiler/crystal/codegen/codegen.cr:369:9 in 'finish'
  from src/compiler/crystal/codegen/codegen.cr:69:7 in 'codegen'
  from src/compiler/crystal/codegen/codegen.cr:65:5 in 'codegen:debug:single_module'
  from src/compiler/crystal/progress_tracker.cr:22:7 in 'codegen'
  from src/compiler/crystal/compiler.cr:172:16 in 'compile'
  from src/compiler/crystal/command/spec.cr:76:14 in 'spec'
  from src/compiler/crystal/command.cr:105:7 in 'run'
  from src/compiler/crystal/command.cr:49:5 in 'run'
  from src/compiler/crystal/command.cr:48:3 in 'run'
  from src/compiler/crystal.cr:11:1 in '__crystal_main'
  from src/crystal/main.cr:105:5 in 'main_user_code'
  from src/crystal/main.cr:91:7 in 'main'
  from src/crystal/main.cr:114:3 in 'main'
Error: you've found a bug in the Crystal compiler. Please open an issue, including source code that will allow us to reproduce the bug: https://github.com/crystal-lang/crystal/issues

@asterite
Copy link
Member

Inheritance of generics types is broken. Please don't use it until it's fixed. And that's also a reason there's no need to change dup or clone, because they won't work well with generic inheritance

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue This is an issue suited for newcomers to become aquianted with working on the codebase. kind:bug topic:stdlib:collection
Projects
None yet
Development

No branches or pull requests

4 participants