FieldMetadata.jl icon indicating copy to clipboard operation
FieldMetadata.jl copied to clipboard

`@bounds` with no bounds defined gives error

Open kongdd opened this issue 2 months ago • 5 comments

# script in test.jl 
using Parameters
import FieldMetadata: @bounds, bounds, @units, units

@bounds @units @with_kw mutable struct Muskingum{FT}
  x::FT = 0.35 | (0.01, 0.5) | "-"

  dt::FT = 1.0
  C0::FT = FT(NaN)
  C1::FT = FT(NaN)
  C2::FT = FT(NaN)
end

x = Muskingum{Float64}()
@show bounds(x)
@show units(x)
(miniforge3) PS Z:\GitHub\jl-pkgs\FieldMetadata.jl> julia test.jl
ERROR: LoadError: type Float64 has no field head
Stacktrace:
 [1] getproperty(x::Float64, f::Symbol)
   @ Base .\Base.jl:49
 [2] parseblock!(block::Expr, exprs::Vector{Expr}, method::Symbol, typ::Symbol, checktyp::Type)
   @ FieldMetadata C:\Users\hydro\.julia\packages\FieldMetadata\oeQwS\src\FieldMetadata.jl:164
 [3] (::FieldMetadata.var"#5#7"{Symbol, DataType, Vector{Expr}})(block::Expr)
   @ FieldMetadata C:\Users\hydro\.julia\packages\FieldMetadata\oeQwS\src\FieldMetadata.jl:121
 [4] firsthead(f::FieldMetadata.var"#5#7"{Symbol, DataType, Vector{Expr}}, ex::Expr, sym::Symbol)
   @ FieldMetadata C:\Users\hydro\.julia\packages\FieldMetadata\oeQwS\src\FieldMetadata.jl:243
 [5] firsthead(f::FieldMetadata.var"#5#7"{Symbol, DataType, Vector{Expr}}, ex::Expr, sym::Symbol) (repeats 3 times)
   @ FieldMetadata C:\Users\hydro\.julia\packages\FieldMetadata\oeQwS\src\FieldMetadata.jl:247
 [6] funcs_from_unknown(expr::Expr, name::Symbol, checktyp::Type; update::Bool)
   @ FieldMetadata C:\Users\hydro\.julia\packages\FieldMetadata\oeQwS\src\FieldMetadata.jl:120
 [7] funcs_from_unknown(expr::Expr, name::Symbol, checktyp::Type)
   @ FieldMetadata C:\Users\hydro\.julia\packages\FieldMetadata\oeQwS\src\FieldMetadata.jl:102
 [8] var"@bounds"(__source__::LineNumberNode, __module__::Module, expr::Any)
   @ FieldMetadata C:\Users\hydro\.julia\packages\FieldMetadata\oeQwS\src\FieldMetadata.jl:46
in expression starting at Z:\GitHub\jl-pkgs\FieldMetadata.jl\test.jl:11
in expression starting at Z:\GitHub\jl-pkgs\FieldMetadata.jl\test.jl:11

kongdd avatar Sep 29 '25 12:09 kongdd

https://github.com/rafaqz/FieldMetadata.jl/blob/e2203ae52946f1cf49ba602675fa034752e80d6c/src/FieldMetadata.jl#L161-L166

This issue can be solved by adding a field check

    key = getkey(fn)
    # Then make sure its a call to |
    expr = line.args[2]
    !hasfield(typeof(expr), :head) && continue
    if expr.head == :call && expr.args[1] == :(|)
        process_equals_line!(exprs, line, expr, key, method, typ, checktyp)
    end

The above case:

using Parameters
using Pkg
Pkg.activate(".")
import FieldMetadata: @metadata
@metadata bounds nothing
@metadata units "-" String

@bounds @units @with_kw mutable struct Muskingum{FT}
  x::FT = 0.35 | (0.01, 0.5) | "-"

  dt::FT = 1.0
  C0::FT = FT(NaN)
  C1::FT = FT(NaN)
  C2::FT = FT(NaN)
end

x = Muskingum{Float64}()
@show bounds(x)
@show units(x)
> julia test.jl
  Activating project at `Z:\GitHub\jl-pkgs\FieldMetadata.jl`
bounds(x) = ((0.01, 0.5), nothing, nothing, nothing, nothing)
units(x) = ("-", "-", "-", "-", "-")

kongdd avatar Sep 29 '25 12:09 kongdd

But there are five lines can not be passed tests even with the original version.

https://github.com/kongdd/FieldMetadata.jl/actions/runs/18096859843/job/51489801526

using FieldMetadata, Parameters, Test, Markdown, REPL

abstract type AbstractTest end

import FieldMetadata: @description, description, MetadataError

@description mutable struct Described{P}
  a::Int | "an Int"
  b | "an untyped field"
  c::P | "a parametric field"
  "An inner constructor should work, even with a docstring"
  Described(a, b, c) = begin
    new{typeof(c)}(a, b, c)
  end
  function Described(a, b)
    new{Nothing}(a, b, nothing)
  end
end

# @test length(methods(Described).ms) == 2
# @test length(methods(description).ms) == 12

d = Described(1, 1.0, nothing)

## The following not work 
@inferred description(d, :a)
@inferred description(d, :c)
@inferred description(typeof(d), :b)
@inferred description(Described, :a)
@inferred description(Described, :c)
> julia .\debug.jl
ERROR: LoadError: return type String does not match inferred return type Any
Stacktrace:
 [1] error(s::String)
   @ Base .\error.jl:35
 [2] top-level scope
   @ Z:\GitHub\jl-pkgs\FieldMetadata.jl\debug.jl:26
in expression starting at Z:\GitHub\jl-pkgs\FieldMetadata.jl\debug.jl:26

kongdd avatar Sep 29 '25 12:09 kongdd

Ah so we have lost type stability. Likely that's from changes to the compiler over time.

rafaqz avatar Sep 30 '25 07:09 rafaqz

By strictly restricting the type of the returned variable, this problem can be solved. See details in https://github.com/rafaqz/FieldMetadata.jl/pull/21/commits/1ec0886dc35762c36a1c90e53244388b7302fd5f. Then all tests are passed

        @inline $name(x, key) = ($default::$checktyp)
        @inline $name(x::Type, key::Type) = ($default::$checktyp)
        @inline $name(::X, key::Symbol) where X = ($name(X, Val{key})::$checktyp)
        @inline $name(::X, key::Type) where X = ($name(X, key)::$checktyp)
        @inline $name(::Type{X}, key::Symbol) where X = ($name(X, Val{key})::$checktyp)

kongdd avatar Sep 30 '25 07:09 kongdd

Thanks for the fix. I'm wondering if this will break anything.

rafaqz avatar Sep 30 '25 07:09 rafaqz