Module validation failed: inlinable function call in a function with debug info must have a !dbg location

[BigBoyBarney]

Hi!

The following minimal example compiles and runs with crystal run, but results in a compiler error with crystal run --release

alias Elements = Array(String)
CONSTANT = [] of Elements

macro bug(elements)
{%
  CONSTANT << elements.map &.downcase
%}
end

bug ["One", "Two", "Three"]
bug ["Four", "Five", "Six"]
puts CONSTANT

The error

Module validation failed: inlinable function call in a function with debug info must have a !dbg location
  %0 = call ptr @"*Array(String)@Array(T)::unsafe_build<Int32>:Array(String)"(i32 713, i32 3)
inlinable function call in a function with debug info must have a !dbg location
  %5 = call ptr @"*Pointer(String)@Pointer(T)#[]=<Int32, String>:String"(ptr %4, i32 0, ptr @"'ONE'")
inlinable function call in a function with debug info must have a !dbg location
  %7 = call ptr @"*Pointer(String)@Pointer(T)#[]=<Int32, String>:String"(ptr %6, i32 1, ptr @"'TWO'")
inlinable function call in a function with debug info must have a !dbg location
  %9 = call ptr @"*Pointer(String)@Pointer(T)#[]=<Int32, String>:String"(ptr %8, i32 2, ptr @"'THREE'")
inlinable function call in a function with debug info must have a !dbg location
  %11 = call ptr @"*Array(String)@Array(T)::unsafe_build<Int32>:Array(String)"(i32 713, i32 3)
inlinable function call in a function with debug info must have a !dbg location
  %16 = call ptr @"*Pointer(String)@Pointer(T)#[]=<Int32, String>:String"(ptr %15, i32 0, ptr @"'FOUR'")
inlinable function call in a function with debug info must have a !dbg location
  %18 = call ptr @"*Pointer(String)@Pointer(T)#[]=<Int32, String>:String"(ptr %17, i32 1, ptr @"'FIVE'")
inlinable function call in a function with debug info must have a !dbg location
  %20 = call ptr @"*Pointer(String)@Pointer(T)#[]=<Int32, String>:String"(ptr %19, i32 2, ptr @"'SIX'")

Removing .map &.downcase compiles correctly with --release. Passing --no-debug fixes it as well, obviously :rofl:

Relevant information:

Crystal 1.14.0 [dacd97b] (2024-10-09)

LLVM: 19.1.0 Default target: x86_64-redhat-linux-gnu

Fedora Linux 41

Edit:

Interestingly, if I'm not adding the elements to an array, it works.

macro bug(elements)
{%
  puts elements.map &.downcase
%}
end

bug ["One", "Two", "Three"]
bug ["Four", "Five", "Six"]

[straight-shoota]

Looks like the cause for this error is that the downcased StringLiterals have no location. Adding at(name_loc) to the result value of StringLiteral#downcase fixes that. But it's also obvious that similar methods have exactly the same issue. All #interpret implementations receive the name_loc parameter. But none of them uses it to attach a location to the result value. And only one single method (TypeDef#name) attaches a location at all.

[straight-shoota]

Macro methods which return "fresh" values should have the location of the call that created them. Like in this example, the downased string should have it's location point to the call to StringLiteral#downcase.

Ones that act as getters, returning already existing values (may be duplicates, though), the location should be that of the original value (like in TypeDef#name).

The former is probably much more common, so we could consider adding a catch-all in Interpreter#visit(call) which assigns the call's location to the return value. Existing locations should not be overridden, of course. So implementations of #interpret could still assign a different location. We'll need to go through the methods to check which ones need that. But the catch-all would remove the need to add .at(name_loc) to most of the implementations.

[straight-shoota]

I suppose it's not really feasible to assume that every AST node will always have an associated location. Ideally that would be the case, but unless there's a hard rule to enforce that, it cannot be expected at 100%.

Under that premise the compiler (or LLVM) should probably be able to gracefully handle missing location information. This affects only AST nodes that can lead to function calls, which is the case for ArrayLiteral like in the OP. The compiler must not crash on that. I'd even say that missing debug location should not prevent compilation. It does not seem to be really relevant to the integrity of the program that everything has a location. 🤷 I'm not sure about the reasoning why this is an error in LLVM.

Here are a couple of ideas:

1. Detect missing debug locations in the compiler and raise an error on our end. This is probably not very useful because this will always be a compiler bug and the user could not fix it.
2. Make sure to always have debug location on nodes that may produce calls. This seems to be hard to verify, so it seems like a weak goal.
3. Attach bogus locations to inlinable function calls. This doesn't seem to be ideal, but it would ensure compilation succeeds. And the compiler is probably able to pick a location that is at least somewhat relevant to the call.
4. Mark function calls without debug location as non-inlinable. This doesn't seem like a really good choice because while missing location information would not hinder compilation, it would affect performance characteristics.

Unrelated to that, we should add location information to more AST nodes. I'll implement the suggestion from https://github.com/crystal-lang/crystal/issues/15202#issuecomment-2482924310.

Here is the relevant code in LLVM: https://github.com/llvm/llvm-project/blob/aa746495affb3ab94daafcbe09bfb229dd27429f/llvm/lib/IR/Verifier.cpp#L3787-L3799

[straight-shoota]

I've submitted a fix for the immediate problem with the code in the OP: #15213 This does not affect the deeper issue that we can potentially pass calls with missing location data to LLVM.