[WIP] fix for property setter called with argument name

[smoothdeveloper]

Related issue: #16034

Instead of internal error (which our test infrastructure cannot assert against), it now gives:

error FS0035: This construct is deprecated: The unnamed arguments do not form a prefix of the arguments of the method called

I never encountered this message, if anyone can clarify this is intended behaviour.

sample code:

typeT()=memberx.indexed1with get (a1:obj)=
printfn $"T().indexed1 {a1} !\t%03i{i}"1andset(a1:obj)(value:int)=
printfn $"T().indexed1 {a1} <- {value} !\t%03i{i}"lett= T()
t.indexed1(a1="ok")<-1

Just checking for now that it doesn't break CI.

[smoothdeveloper]

The change breaks nothing, but I doubt there is enough coverage on indexed properties setters.

My opinion in favour of supporting the construct (using normal suggestion process):

• reaching symmetry with getter
• currently surface the "named arguments coming only after ordinal ones" rule, which is impossible for the special case of value / RHS in indexed property setter case

In meantime, it remains a minor hindrance which has a work around: set_indexed1(...named arguments)

@dsyme would you mind sharing:

• some context about the "unnamed arguments do not form a prefix of the arguments of the method called" deprecation
• your current opinion about enabling it for this use case, and the other use cases (if any) it should remain the same

[vzarytovskii]

@smoothdeveloper Is it still WIP?

[smoothdeveloper]

@vzarytovskii, let me put all my trust in isIndexerSetter being accurate, and my change only preventing the internal exception, then yes, I think this can be merged.

I'll add more coverage on the behaviour in #16035.

[psfinaki]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[edgarfgp]

Out of curiosity. Will this also for multiple indexes ?

typeT()=memberx.indexed1with get (a1:obj)=
printfn $"T().indexed1 {a1} !\t%03i{i}"1andset(a1:obj,a2:obj)(value:int)=
printfn $"T().indexed1 {a1} <- {value} !\t%03i{i}"lett= T()
t.indexed1(a1="ok", a2 ="")<-1

[smoothdeveloper]

@edgarfgp it doesn't fix the crash for this particular case 😞, nor for adjusting the getter to have both a1 & a2.

So it is sadly not a complete fix, I need to look more into it, I think it is related to tupled arguments.

Thanks for sharing the sample.

[edgarfgp]

@edgarfgp it doesn't fix the crash for this particular case 😞, nor for adjusting the getter to have both a1 & a2.

So it is sadly not a complete fix, I need to look more into it, I think it is related to tupled arguments.

Thanks for sharing the sample.

I'm having similar problems in #16023 when there is an Indexer property with multiple indexes. It seems that current logic does not consider a setter as part of the property

[smoothdeveloper]

The crash occurs in place where we check for parameter supporting [<ParamArray>].

Can someone confirm to me that the only place an indexed setter can accept param array is the last before value:

set_indexed(a1, a2, [<ParamArray>]a3, value) which conflicts a bit with the [<ParamArray>] should be last, but seems legit still.

What is odd is that there is an assumption that the "possibleParamArg" is going to be among the unnamedCalledArgs, but this can't hold for sure: it is perfectly possible to call the indexed setter by naming all the arguments but value.

Going to thinker more, will take some time before I'm confident of not just fixing the simplest case :)

[smoothdeveloper]

Closing, same reason as #16035