Wiki

Ticket #189 (closed enhancement: fixed)

Opened 8 years ago

Last modified 5 years ago

When method calls have incorrect number of arguments or types, give the method sigs

Reported by: Chuck Owned by: Chuck
Priority: medium Milestone:
Component: Cobra Compiler Version: 0.8.0
Keywords: Cc:

Description

Suppose you get this error:

error: The method "invoke" is expecting 8 arguments, but 3 are being supplied in this call.

It would be productive if Cobra displayed the method signature.

Don't forget about overloads.

Don't forget errors can occur because of typing rather than wrong number.

In addition to "error" and "warning", perhaps we need a 3rd kind of message: "info"

Attachments

tkt-189.patch Download (8.5 KB) - added by hopscc 5 years ago.
tkt-189-augment-and-cleanup.patch Download (11.2 KB) - added by hopscc 5 years ago.

Change History

Changed 5 years ago by hopscc

Changed 5 years ago by hopscc

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

Display method/indexer paramlist and arglist signature in error msg.
Cleanup msg gen (count and typing) to a single place.
tests checking new detail info.

'expecting ... but got' error msgs not applicable to overloads.
These seem to be trapped in C#

Changed 5 years ago by hopscc

I started wondering why my simple tests on overloaded wrongly typed/arglist-length calls werent being picked up by the cobra compiler ( but are trapped in C# compiler).

Theres some commented code in Expr.cobra line 775 marked

#to-do: 2012-07-01 CE: the following just isn't ready
if winner inherits BoxMember # .no-warnings.
...

which does the arglist checking etc on overloads so thats why...
I uncommented it and rebuilt and ran my tests and it gives the paramlist and arglist detail info (as above for non overloaded methods) so thats cool.

Ran it on the full test suite and it failed at one place - building the compiler due to 4 uncaught mismatches (presumably in overloads) in calls using nilable and not in args vs params.

I fixed those in the compiler and all tests in test suite then passed ( or rather all that passed previous to these fixes going in still passed - theres still the 4 failures - 3 on partial class tests and one on implicit reference/dll loading).

The second patch file above is tkt-189 patches (as in first patch file) plus uncommenting overload testing PLUS cleanup for nilable arglists vs non nilable paramlists.

Changed 5 years ago by hopscc

Changed 5 years ago by hopscc

even if the overloaded method checks are not reasserted the cleanup up calls
in the compiler should be ...

Changed 5 years ago by Charles

  • status changed from assigned to closed
  • resolution set to fixed

I have applied the patch in changeset:2966, but the reason you didn't get many failures is because of this line:

if winner.name not in ['format', 'invoke', 'split', 'sort']

And those just happen to be the names of std lib methods with overloads that aren't handled. Imagine using a 3rd party library with its own overloads. We'll need to be prepared to fix these problems as they are reported.

Of course, we could start earlier by eliminating that if guard and fixing the problems we encounter. I'll fill out a new ticket.

Changed 5 years ago by Charles

New ticket:321 tracks the remaining issues with overloads.

Changed 5 years ago by nerdzero

Did the comment: "the following just isn't ready" refer to this being a little too aggressive in regards to nil-checking. It seems with this patch that some of my existing code (the addin) no longer compiles.

Here's an example program that used to compile before this patch:

class NilTesting
    def main
        s as String? = "hola"
        if s == nil
            return
        .printMsg(s, 1)

    def printMsg(msg as String, times as int)
        for i in times
            print msg

    def printMsg(msg as String)
        print msg

Now, I get:

test.cobra(6): error: Argument 1 of method "printMsg" expects a non-nilable 
type (String), but the call is supplying a nilable type (String?). The 
declaration is "printMsg(msg as String, times as int)", the call is supplying 
an arglist of types "(String?, int)".

This is not an argument to remove the patch as clearly this was just a byproduct of the way overload resolution used to work. Just pointing out that this requires a lot of unnecessary "foo(bar to !)" now.

Changed 5 years ago by hopscc

Arglist checking on any overloaded methods was totally suppressed, now its not.
The "following just isnt ready" I took to refer to how well the overload resolution worked in finding a matching method ( in conjunction with subsequent arg checking.

Arguably any code that calls methods having a non-nilable param with a corres nilable arg is wrong type-wise. (cf int param called with string arg - non-match)
The code should either explicitly cast to nilability (as you note - though its not unnecessary) or change the method to have a nilable param (and perhaps check for nil at runtime).

Remember nilability is intended to be a (static) typed ( type-like) check rather than a runtime one...

Note: See TracTickets for help on using tickets.