Ticket #323 (closed defect: fixed)

Opened 5 years ago

Last modified 4 years ago

Internal error for dynamic argument

Reported by: Charles Owned by: Chuck
Priority: major Milestone:
Component: Cobra Compiler Version: 0.9.4
Keywords: Cc:


The program below works in Cobra 0.9.3, but not 0.9.4:

class X

    def foo(c as char)

    def foo(s as String)

    def foo(obj as Object)

    def main
        x = 'aoeu' to dynamic

changeset:2966 from ticket:189 caused the change

Commenting out this code in Expr.cobra (around line 752) will allow the above program to compile and run without error:

                            if winner inherits BoxMember # .no-warnings.
                                # to-do: 2012-07-01 CE: the following just isn't ready
                                if not in ['format', 'invoke', 'split', 'sort']  # to-do: hack
                                    params = winner.params
                                    if _checkParamsCount(winner, args, params)
                                        if needInferOutArgs, _inferOutArgs(args, params, false) 
                                        _checkArgsAssignable(winner, args, params)


overload-and-dyn-arg.patch Download (3.8 KB) - added by hopscc 5 years ago.

Change History

Changed 5 years ago by Charles

  • version changed from 0.9.3 to 0.9.4

Changed 5 years ago by hopscc

  • owner set to hopscc
  • status changed from new to assigned

The cause is actually in _checkArgsAssignable where the contextType field is updated after determining the arg is assignable to the parameterType (about line 940-ish).

The root cause is a poor choice for the best overloaded method (and the use of a dynamic arg).

Though to be fair it would be an edge case optimisation on work on ticket:130/ticket:321 to choose the correct overload in the first case.

Program Source code corrections include changing the order of the overloaded methods OR explicitly casting the arg to disambiguate the overload needed.

The compiler code prior to the current setting contextType for this case compiled so it would be reasonable to fallback to that for this case (Overloaded method with dynamic arg(s))
but a more general fix may be to just determine that the combination is inherently
error prone to auto resolve and emit a warning...
A subsequent failure would then indicate that some more code hinting ( per the warning) is needed.

I have a fix (as per the first option above) just waiting for tests..

Changed 5 years ago by hopscc

Changed 5 years ago by hopscc

  • owner changed from hopscc to Chuck

Patch to not update contextType on BoxMembers that are both overloads and have args with dynamic types.
Above failing program as new test

Changed 5 years ago by Charles

Regarding the patch, we'll see if this sticks or not:

                if defnIsOverload and arg.type.nonNil inherits DynamicType 
                        arg.contextType = param.type 

The whole point of setting the .contextType is that most of the time it is necessary. We might get away without it though if we switch to C# dynamic.

Changed 4 years ago by hopscc

prior to changeset:2966 it wasnt passing through that code at all (and presumably wasnt setting contextType for any overloads) so its probably not necessary for them..
With the above its setting them for overloads except those that have dynamic args

Changed 4 years ago by Charles

  • status changed from assigned to closed
  • resolution set to fixed
Note: See TracTickets for help on using tickets.