Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[js] optional params + overloads #7794

Open
ncannasse opened this issue Feb 14, 2019 · 7 comments · May be fixed by #11947
Open

[js] optional params + overloads #7794

ncannasse opened this issue Feb 14, 2019 · 7 comments · May be fixed by #11947
Labels
feature-overload feature-type-inference test-needed
Milestone
Later

Comments

@ncannasse
Copy link
Member

The following used to work but no longer since #7776

	var current : js.html.audio.AudioBufferSourceNode = null;
	var ctx : js.html.audio.AudioContext = null;
	current.connect(ctx.destination); // overload resolution failed

Ping @Simn (if we can fix this in compiler) and @haxiomic (FYI)
@ncannasse ncannasse added this to the Release 4.0 milestone Feb 14, 2019
@haxiomic
Copy link
Member

Signature for connect() is:

@:overload( function( destination : AudioNode, output : Int = 0, input : Int = 0 ) : AudioNode {} )
function connect( destination : AudioParam, output : Int = 0 ) : Void;

@Simn Simn self-assigned this Feb 14, 2019
@Simn
Copy link
Member

Simn commented Feb 14, 2019

That suggests that the "optional" inference of arguments in overloads doesn't work.

@ncannasse
Copy link
Member Author

Most likely default values are discarded in overload signatures.

@Simn
Copy link
Member

Simn commented Feb 14, 2019

We don't need the default value as long as the type is correct.

@Simn
Copy link
Member

Simn commented Feb 14, 2019

I guess #7754 was mistaken for a display-specific bug when it is actually a general problem.

@Simn
Copy link
Member

Simn commented Feb 14, 2019

Never mind, that one uses an explicit ? which makes the difference.

Simn added a commit that referenced this issue Feb 14, 2019
@Simn
[typer] consider overloaded arguments optional if there's a default v…
e314c81 
…alue

see #7794
@Simn Simn removed the regression label Feb 14, 2019
@Simn
Copy link
Member

Simn commented Feb 14, 2019

Should be fixed. Keeping this open because I'd like to add a test for it somehow.

@Simn Simn modified the milestones: Release 4.0, Release 4.1 Feb 27, 2019
@Simn Simn removed their assignment Jul 1, 2019
@Simn Simn modified the milestones: Release 4.1, Release 4.2 Feb 19, 2020
@RealyUniqueName RealyUniqueName modified the milestones: Release 4.2, Backlog Dec 14, 2020
@Simn Simn modified the milestones: Backlog, Later Mar 24, 2023
kLabz added a commit that referenced this issue Jan 27, 2025
@kLabz
[tests] Add test for #7794
e066b4e
@kLabz kLabz linked a pull request Jan 27, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature-overload feature-type-inference test-needed
Projects
None yet
Milestone
Later
Development

Successfully merging a pull request may close this issue.

[tests] Add test for #7794 HaxeFoundation/haxe
5 participants
@Simn @ncannasse @Gama11 @RealyUniqueName @haxiomic