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

Emit warning on generic function/class export #981

Closed

Conversation

9oelM
Copy link

@9oelM 9oelM commented Nov 30, 2019

Without knowledge that exporting a generic function/class is not possible (yes.. I was dumb..), I wasted my time figuring things out on my own.

When I googled and finally found #570, I wanted to improve this.

  • Emit warning on generic function/class export
  • Add tests accordingly
  • Adjust runTest function to consume stdout

Would resolve #570

@9oelM
Copy link
Author

9oelM commented Nov 30, 2019

@MaxGraey @dcodeIO Actually, I'm not still sure if we should emit an error or a warning.

@9oelM 9oelM force-pushed the emit-warning-on-generic-export branch 2 times, most recently from 4064d87 to af4d261 Compare November 30, 2019 16:38
@9oelM 9oelM force-pushed the emit-warning-on-generic-export branch from af4d261 to 76b7f99 Compare November 30, 2019 16:40
@dcodeIO
Copy link
Member

dcodeIO commented Nov 30, 2019

The way this currently works is that if one does

export class Foo<T> {
  bar(a: T): T {
    return a;
  }
}

while it cannot be automatically compiled due to missing context, if there is

var foo = new Foo<i32>();
foo.bar(1);

somewhere in the code one will still get a Foo<i32>#bar export. As such, exporting generics is limited to what's actually reachable from other code and thus present in the binary.

@MaxGraey
Copy link
Member

@9oelM Yes, I guess warning will be more prefer

@9oelM
Copy link
Author

9oelM commented Dec 1, 2019

@dcodeIO
I'm sorry, but I haven't fully understood what you said. Could you elaborate a bit more so that I could fix the PR?

@dcodeIO
Copy link
Member

dcodeIO commented Dec 1, 2019

What I'm trying to get at is that exporting a generic function or class does actually work iff there is a concrete usage of the function or class within the module. Only if there is no concrete usage, there won't be a respective export. Still uncertain how to handle this properly, but perhaps we should attempt to detect generic exports that don't have any concrete usage and only then emit a warning?

For an example of what I mean, compile

export class Foo<T> {
  bar(a: T): T {
    return a;
  }
}
var foo = new Foo<i32>();
foo.bar(1);

with

asc thefile.ts --runtime none -t

and you'll notice that there is an export named Foo<i32>#bar representing the member function bar of Foo because it is used inside of the module with a concrete type parameter of i32.

 (export "Foo<i32>#bar" (func $index/Foo<i32>#bar))
 (func $index/Foo<i32>#bar (param $0 i32) (param $1 i32) (result i32)
  local.get $1
 )

@9oelM
Copy link
Author

9oelM commented Dec 5, 2019

@dcodeIO I get what you mean now. For now, I think it would be a good idea to just emit a warning if there is no concrete usage inside the module itself.

@dcodeIO
Copy link
Member

dcodeIO commented Dec 7, 2019

One strategy to implement this could be to compile the module normally, inspecting module-level exports right before finishing compilation, emitting the warning if the respective FunctionPrototype or ClassPrototype has no compiled .instances. Somewhat similar to what Compiler#ensureModuleExports does, but as a kind of check.

@9oelM
Copy link
Author

9oelM commented Dec 10, 2019

@dcodeIO Will try that.

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.

Explicitly restrict exporting generic functions for entry file
3 participants