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.

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

Add support for native cs type parameter constraints #8311

Conversation

kLabz
Copy link
Contributor

@kLabz kLabz commented May 20, 2019

Allow one to define native C# constraints from haxe.Constraints.Constructible or cs.Constraints.*. This is useful when using C# APIs using such constraints for inputs; without it we need "ugly" workarounds.

Supported native constraints:

  • where T : struct with cs.Constraints.CsStruct
  • where T : class with cs.Constraints.CsClass
  • where T : unmanaged with cs.Constraints.CsUnmanaged
  • where T : new() with haxe.Constraints.Constructible<Void->Void>

For example, this:

import cs.Constraints;

@:nativeGen
class Main<T:CsStruct> {
	public function new() {}
}

Will result in:

public class Main<T> where T : struct {
	public Main() {
	}
}

See Microsoft's documentation on the subject for reference.

@Gama11
Copy link
Member

Gama11 commented May 20, 2019

Is this a case where @:coreType would make sense to avoid the abstract needing a (Dynamic) underlying type?

@kLabz
Copy link
Contributor Author

kLabz commented May 20, 2019

Could be; although from Dynamic (or something nicer) would still be needed.

@skial skial mentioned this pull request May 20, 2019
1 task
@RealyUniqueName
Copy link
Member

Add a test, please.

@kLabz kLabz changed the title Add support for native cs type parameter constraints [WIP] Add support for native cs type parameter constraints May 21, 2019
@kLabz kLabz changed the title [WIP] Add support for native cs type parameter constraints Add support for native cs type parameter constraints May 21, 2019
@Gama11
Copy link
Member

Gama11 commented May 21, 2019

cs_ver should probably be added to define.ml so it shows up in haxe --help-defines?

@kLabz
Copy link
Contributor Author

kLabz commented May 21, 2019

Notes on tests:

  • TestCs class may seem redundant but is actually required to make sure the generated where constraints are present in the generated C# code so that we can use C# APIs requiring them.

  • Both static functions and classes are tested in Main.hx, again it seems to be redundant but it makes sure native constraints work in both places

  • Unsupported combinations as documented on MSDN are tested in incompatible-combinations-fail.hxml. This includes the undocumented incompatibility between class and struct (both have to be the very first constraint).

@kLabz
Copy link
Contributor Author

kLabz commented May 21, 2019

Should add

	{
		"name": "CsVer",
		"define": "cs_ver",
		"doc": "The C# version to target",
		"platforms": ["cs"]
	},

in #8195 if this is merged.

Edit: #8195 has been merged, and this PR has been updated.

@kLabz
Copy link
Contributor Author

kLabz commented May 22, 2019

@Gama11

Is this a case where @:coreType would make sense to avoid the abstract needing a (Dynamic) underlying type?

Should I do this? I used haxe.Constraints as a model for cs.Constraints, but I can change it (and maybe update haxe.Constraints too?).

@Gama11
Copy link
Member

Gama11 commented May 22, 2019

I'm not sure, is there a reason why haxe.Constraints types are defined the way they are @Simn?

@kLabz kLabz force-pushed the feature/native-cs-type-parameter-constraints branch 3 times, most recently from 8875bd1 to 8499157 Compare May 24, 2019 07:38
@kLabz kLabz force-pushed the feature/native-cs-type-parameter-constraints branch from 8499157 to eb5a6f8 Compare May 29, 2019 12:33
@kLabz
Copy link
Contributor Author

kLabz commented May 29, 2019

simn
@:coreType typically has a specific native representation which is handled by the target generator

Rudy G
Would you say then that @:coreType would make more sense in cs.Constraints since they generate native c# type parameter constraints?
(and not necessarily in haxe.Constraints)

simn
Yes if these typed are picked up by the generator you can use @:coreType
Abstracts over Dynamic are for when the run-time representation is not specified.

I updated cs.Constraints to use @:coreType (and not haxe.Constraints) and rebased development again, waiting for CI ~

@Simn Simn merged commit 3e90e13 into HaxeFoundation:development May 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants