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

Incorrect "T.cast is useless" error on T.bind with rescue / ensure blocks #6323

Open
alto-rlk opened this issue Aug 31, 2022 · 4 comments
Open
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@alto-rlk
Copy link
Contributor

Input

→ View on sorbet.run

# typed: strict
module M
  extend T::Sig

  sig {void}
  def good
    T.bind(self, C)
  end

  sig {void}
  def bad
    T.bind(self, C)
  rescue => e
  end

end

class C
  include M
end

Observed output

editor.rb:12: T.cast is useless because C is already a subtype of C https://srb.help/7015
    12 |    T.bind(self, C)
            ^^^^^^^^^^^^^^^
  Got C originating from:
    editor.rb:12:
    12 |    T.bind(self, C)
            ^^^^^^^^^^^^^^^
  Autocorrect: Use `-a` to autocorrect
    editor.rb:12: Replace with self
    12 |    T.bind(self, C)
            ^^^^^^^^^^^^^^^
Errors: 1

Runtime behavior: this version exercises the methods and runs with no errors.

Here's an additional sorbet.run example covering a few more cases.

Expected behavior

No errors.

@Morriar
Copy link
Collaborator

Morriar commented Aug 31, 2022

This usage of T.bind is strange. What would happen if I included the module in another class?

class D
  include M
end

I think what you're looking for here is requires_ancestor:

# typed: strict
module M
  extend T::Sig
  extend T::Helpers

  requires_ancestor { C }
end

class C
  include M
end

This also have the benefit of preventing M being included in something that is not a C.

@alto-rlk
Copy link
Contributor Author

Thanks! I'd agree that requires_ancestor is the better solution for this use case in general, but it is still listed as experimental.

FWIW, I don't believe this bug is specific/related to M being a module.

@jez
Copy link
Collaborator

jez commented Aug 31, 2022

This is a perfectly valid use of bind, and this error should probably not reported. Not sure at a glance what the best solution is, but there are two options:

  • silence the useless cast error for bind (not great, as that would cause sorbet to report some errors that it likely should)
  • figure out why this extra cast instruction is getting into the method body:

<self>: C = cast(<castTemp>$6: C, C); in bb4

https://sorbet.run/?arg=--print=cfg-text#%23%20typed%3A%20strict%0Amodule%20M%0A%20%20extend%20T%3A%3ASig%0A%0A%20%20%23%20sig%20%7Bvoid%7D%0A%20%20%23%20def%20good%0A%20%20%23%20%20%20T.bind%28self%2C%20C%29%0A%20%20%23%20end%0A%0A%20%20sig%20%7Bvoid%7D%0A%20%20def%20bad%0A%20%20%20%20T.bind%28self%2C%20C%29%0A%20%20rescue%20%3D%3E%20e%0A%20%20end%0Aend%0A%0Aclass%20C%0A%20%20include%20M%0Aend

method ::M#bad {

bb0[rubyRegionId=0, firstDead=-1]():
    <self>: M = cast(<self>: NilClass, M);
    <magic>$7: T.class_of(<Magic>) = alias <C <Magic>>
    <exceptionValue>$3: T.untyped = <get-current-exception>
    <self>: C = cast(<castTemp>$6: NilClass, C);
    <returnMethodTemp>$2: C = <self>
    <exceptionValue>$3 -> (T.untyped ? bb3 : bb4)

# backedges
# - bb6(rubyRegionId=3)
# - bb9(rubyRegionId=0)
bb1[rubyRegionId=0, firstDead=-1]():
    <unconditional> -> bb1

# backedges
# - bb0(rubyRegionId=0)
# - bb4(rubyRegionId=1)
bb3[rubyRegionId=2, firstDead=-1](<returnMethodTemp>$2: C, <exceptionValue>$3: T.untyped, <magic>$7: T.class_of(<Magic>)):
    e: T.untyped = <exceptionValue>$3
    <cfgAlias>$10: T.class_of(StandardError) = alias <C StandardError>
    <isaCheckTemp>$11: T.untyped = e: T.untyped.is_a?(<cfgAlias>$10: T.class_of(StandardError))
    <isaCheckTemp>$11 -> (T.untyped ? bb7 : bb8)

# backedges
# - bb0(rubyRegionId=0)
bb4[rubyRegionId=1, firstDead=-1](<self>: C, <magic>$7: T.class_of(<Magic>)):
    <cfgAlias>$5: T.class_of(C) = alias <C C>
    keep_for_ide$4: T.class_of(C) = <cfgAlias>$5
    keep_for_ide$4: T.untyped = <keep-alive> keep_for_ide$4
    <castTemp>$6: C = <self>
    <self>: C = cast(<castTemp>$6: C, C);
    <returnMethodTemp>$2: C = <self>
    <exceptionValue>$3: T.untyped = <get-current-exception>
    <exceptionValue>$3 -> (T.untyped ? bb3 : bb5)

# backedges
# - bb4(rubyRegionId=1)
bb5[rubyRegionId=4, firstDead=-1](<returnMethodTemp>$2: C):
    <unconditional> -> bb6

# backedges
# - bb5(rubyRegionId=4)
# - bb7(rubyRegionId=2)
# - bb8(rubyRegionId=2)
bb6[rubyRegionId=3, firstDead=-1](<returnMethodTemp>$2: C, <gotoDeadTemp>$12: T.nilable(TrueClass)):
    <gotoDeadTemp>$12 -> (T.nilable(TrueClass) ? bb1 : bb9)

# backedges
# - bb3(rubyRegionId=2)
bb7[rubyRegionId=2, firstDead=-1](<returnMethodTemp>$2: C, <magic>$7: T.class_of(<Magic>)):
    <exceptionValue>$3: NilClass = nil
    <keepForCfgTemp>$8: Sorbet::Private::Static::Void = <magic>$7: T.class_of(<Magic>).<keep-for-cfg>(<exceptionValue>$3: NilClass)
    <unconditional> -> bb6

# backedges
# - bb3(rubyRegionId=2)
bb8[rubyRegionId=2, firstDead=-1](<returnMethodTemp>$2: C):
    <gotoDeadTemp>$12: TrueClass = true
    <unconditional> -> bb6

# backedges
# - bb6(rubyRegionId=3)
bb9[rubyRegionId=0, firstDead=1](<returnMethodTemp>$2: C):
    <finalReturn>: T.noreturn = return <returnMethodTemp>$2: C
    <unconditional> -> bb1

}

Anyways, thanks for the report. I'm not sure when I'll have time to get to this.

@jez jez closed this as completed Aug 31, 2022
@jez jez reopened this Aug 31, 2022
@jez
Copy link
Collaborator

jez commented Aug 31, 2022

(whoops wrong button 😅 sorry about the accidental close)

@jez jez added bug Something isn't working good first issue Good for newcomers labels Aug 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants