Skip to content

Macro redefinitions and overloading order #12078

@HertzDevil

Description

@HertzDevil

Macros, like defs, can be overloaded by their signatures. The compiler never reorders macros, but it does detect redefinitions and replace existing macros, as defined by Crystal::Macro#overrides?:

def overrides?(other : Macro)
# If they have different number of arguments, splat index or presence of
# double splat, no override.
if args.size != other.args.size ||
splat_index != other.splat_index ||
!!double_splat != !!other.double_splat
return false
end
self_named_args = self.required_named_arguments
other_named_args = other.required_named_arguments
# If both don't have named arguments, override.
return true if !self_named_args && !other_named_args
# If one has required named args and the other doesn't, no override.
return false unless self_named_args && other_named_args
self_names = self_named_args.map(&.external_name)
other_names = other_named_args.map(&.external_name)
# If different named arguments names, no override.
return false unless self_names == other_names
true
end

It says two macros are redefinitions if and only if:

  • their numbers of positional parameters agree;
  • either both macros have a single splat parameter, or neither macro has one;
  • their required named parameters have the same names;
  • either both macros have a double splat parameter, or neither macro has one.

These conditions are insufficient, so bugs similar to defs can occur. For example default arguments are completely ignored:

macro foo(x = 0); end
macro foo(x); end # considered a redefinition

foo # Error: wrong number of arguments for macro 'foo' (given 0, expected 1)
macro foo(*, x = 0); end
macro foo(*, y = 1); end # considered a redefinition

foo(x: 2) # Error: no parameter named 'x'

And there are no checks for the bare single splat:

macro foo(x, *y, z); end
macro foo(x, *, z); end # considered a redefinition

foo(1, 2, z: 3) # Error: wrong number of arguments for macro 'foo' (given 2, expected 1)

I suggest that macros should follow the same rules in the corresponding fix for defs. The actual checks are probably not much different from what we have right now because there is no need to check for restrictions, although the possibility remains. But that new algorithm also specifies an overload order, which isn't currently imposed:

# if these were defs, the second overload would come first,
# whether or not `-Dpreview_overload_order` is in effect
macro foo(*x); 1; end
macro foo(x, y); 2; end

foo(3, 4) # => 1

A major difference between macros and defs is that the former must be fully defined prior to being called, because top-level macro expansion occurs in the same phase as macro definition. This means there shouldn't be any problems if we reorder macros there even if we don't have the macro counterpart of #11840.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions