Repository navigation
enable DefaultArguments functions with mismatch signature - #4518
Conversation
There was a problem hiding this comment.
Here i no need to be increased? I'd suggest to cleanup this foreach to make it more readable, for example,
int i = 0;
foreach (var param in descriptor.Parameters)
{
i++;
...
}
There was a problem hiding this comment.
Here we compare the type, and if type mismatch, then may choose using default argument. What happen if a function is overloaded? For example,
Foo(Point p1 = default point 2, Point p2 = default point 1);
Foo(Line l1, Line l2);
And we open a .dyn whose signature is Foo@Line,Line, will it be changed to Foo@Point,Point and use default argument?
There was a problem hiding this comment.
In case the new parameter is added in the beginning, for example
old param: double
new param: Point,double
then when comparing Point and double, we shouldn't increase i
There was a problem hiding this comment.
If the signature in the file is Foo@Line,Line then the descriptor MangledName is also Foo@Line,Line so the condition is evaluated as false and this step is skipped. The function will still be Foo@Line,Line not Foo@Point,Point.
|
Thanks @nguyen-binh-minh, I have tested your code. It works with my new Excel node. |
|
Please add a test cases for LGTM. Thanks! |
|
updated with 2 more tests |
enable DefaultArguments functions with mismatch signature
Purpose
Foo@int,intshould be migrated toFoo@int,int,boolinstead ofFoo@double,double).Reviewers
@ke-yu PTAL
FYIs
@aparajit-pratap