Crystal: Compiler ignores call to undefined method after a NoReturn call

Created on 15 May 2018  路  10Comments  路  Source: crystal-lang/crystal

def defined_method : NoReturn
  raise "no"
end

undefined_method(defined_method)

The above example compiles and runs despite there being a call to an undefined method in the body. This is annoying when undefined_method is actually intended to be a macro call and can lead to renaming a macro definition, but not renaming a usage silently succeeding where it should fail to compile.

compiler

Most helpful comment

@SlayerShadow it's impossible to compile a function which isn't called in crystal. That's a limitation of the typing algorithm (nothing to do with llvm, gcc, linker, etc). That's never going to change. See #4402. And send can't be implemented for the exact same reason.

What i'm thinking is essentially an enhancement: at least look up the method names to see if they exist in scope, even if we can't type them.

All 10 comments

Even more reduced: foo raise "bar"
Which compiles and gives the exception (https://carc.in/#/r/42r0)

@RX14, @bew, why do you think that it is a bug? Ruby has the same behaviour because code launches from right to left.

Just by nature, Crystal compiler can optomize your code to this:
defined_method
(or raise bar in @bew's example) because it knows that left part will never run.

Try this:

class Some
  def initialize
    @foo = 3
    @bar = "xxx"

    @foo + @bar
  end
end

p :success

This code compiles correctly and even launches. But if we change last line to Some.new, we will take an error.

The undefined method is reproducible if we leave some "door" to make launch undefined_method() action possible:

def defined_method : Void
  if ARGV.size > 0
    raise "no"
  end
end

undefined_method(defined_method)

Always shows Error in app/temp.cr:7: undefined method 'undefined_method'. Point - we do not describe else behaviour. Again, if we add else and put raise there, we catch the raise's exception.

Whether it's a bug or a feature request is moot: I explained why this behaviour is undesirable above.

Using return fails at least:

def foo
  bar return # Syntax error: void value expression
end

foo

@RX14, I did not say that it is a bug or a feature, i just asked "why".

My research led to the hypothesis that LLVM, or GCC, or linker can remove unused functions due to optimization reasons (source 1, source 2). It is nice and it is bad.

  • SlayerShadow: I wrote a big code. Let's try to run an empty page without any logic - just classes initialization and see it has no bugs. Crystal! Compile!
  • Crystal: Yes! It is working code! Compiled successfully.
  • SlayerShadow: Really?.. Well, now let's initialize a little class' instance to check if my world launches (added SuperWorld.new). Crystal! Compile again!
  • Crystal: BOOM! Argument Error! Instance variable should be UInt8 but input was Int32: here, here, here, here. Additionally here and here. Also here, here and here. ~_Another over 9k little errors._~
  • SlayerShadow: o_O

Maybe it is better to at least notify user that there is unused code like -Wunused.
If it can be fixed somehow else (maybe look-up table through whole app to search and point used and unused functions), it can move Crystal to one step for create a send action which could call any method.

@SlayerShadow it's impossible to compile a function which isn't called in crystal. That's a limitation of the typing algorithm (nothing to do with llvm, gcc, linker, etc). That's never going to change. See #4402. And send can't be implemented for the exact same reason.

What i'm thinking is essentially an enhancement: at least look up the method names to see if they exist in scope, even if we can't type them.

@RX14, thank you for detailed answer.

It's a feature. If the type is NoReturn, the call can't be made, so no overload will ever match it. Maybe we can check if at least one method exists with that name, and give an error if not. I personally don't find this a problem at all. Why is it a problem?

@asterite I explained up above (and I agree on your method of fixing it): the undefined_method was actually previously a macro and was therefore used and expanded. Then you rename the macro definition and the compiler silently ignores the macro call (assuming it's a method call) because of the NoReturn.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

lgphp picture lgphp  路  3Comments

pbrusco picture pbrusco  路  3Comments

oprypin picture oprypin  路  3Comments

jhass picture jhass  路  3Comments

lbguilherme picture lbguilherme  路  3Comments