Wiki

Ticket #151 (closed defect: fixed)

Opened 9 years ago

Last modified 5 years ago

incomplete support for vari nilable parameters

Reported by: jonathandavid Owned by: Chuck
Priority: medium Milestone:
Component: Cobra Compiler Version: 0.8.0
Keywords: nilable, vari Cc: Chuck

Description

The following code does not behave as expected:

class Test

	def takesVari(x as vari String) is shared
		for y in x
			print y

	def takesArray(x as String[]) is shared
		for y in x
			print y
		
	def main is shared
		
		.takesVari(@['hola', nil])  # compiles fine, no runtime assertion 

		.takesArray(@['hola', nil])  # does not even compile

The call to "takesVari" should have caused a compilation error, just like the [] version does.

Cobra accepts the syntax fun(x as vari String?), but the resulting code behaves as if the "?" wasn't there at all.

Attachments

vari-typed-params.patch Download (6.6 KB) - added by hopscc 5 years ago.

Change History

Changed 5 years ago by hopscc

No code to check typing/nilability on vari params.

Changed 5 years ago by hopscc

Changed 5 years ago by hopscc

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

Add assignability checking for vari args to params.
Support both arglist and a correctly typed array args.

Changed 5 years ago by Charles

Some feedback on the coding in the patch:

(1)

		require param.type inherits VariType 
		variType = param.type.innerType to !

The local var is named variType but the "param.type" is actually the vari-type as shown in the contract. A better name for that local var is innerType:

		require param.type inherits VariType 
		innerType = param.type.innerType to !

(2)

		if typeName.startsWith('vari Object') or typeName.startsWith('vari dynamic')  # always assignable

It is unusual to check for types based on string matches and even more rare to use .startsWith. I changed to:

		if innerType == .typeProvider.objectType or innerType == .typeProvider.dynamicType, return
		if innerType == .typeProvider.arrayType(.typeProvider.objectType), return

Maybe you started with that as well and switched because the above code didn't work right away. DynamicType didn't support == comparisons as it should have, but now does.

(3)
600-vari-nilable-pars.cobra had two error directives in it, but was placed in the 800-warnings/ directory.

Changed 5 years ago by Charles

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

Patch applied in changeset:2951

Changed 5 years ago by hopscc

varitype = Type of the vari Type
- inner type is artifact of varitype being a Wrapped Type
- The method is checking only against variTypes so variType = Type of the vari Thing.

I changed the checks cos the String form was clearer as to what was being checked against ( vs a cascade of Types and wrappers)..
The other wasnt working

You'll probably want to add a check for a dynamic array type as well..

Note: See TracTickets for help on using tickets.