Skip to content
This repository has been archived by the owner on Aug 24, 2022. It is now read-only.

JSReplacement and reflection #850

Open
iskiselev opened this issue Sep 11, 2015 · 14 comments
Open

JSReplacement and reflection #850

iskiselev opened this issue Sep 11, 2015 · 14 comments

Comments

@iskiselev
Copy link
Member

Currently using of JSReplacement affect reflection, as member with JSReplacement is not translated.
I suggest in any translate JSReplacement method with same body as passed in JSReplacement attribute.

@kg
Copy link
Member

kg commented Sep 15, 2015

The replacement can't be a method body, though, because it's a textual replacement macro.

Not sure what we should do about this...

@kg kg added the Integration label Sep 15, 2015
@iskiselev
Copy link
Member Author

Really, JSReplacement currently do two things:

  1. Provide macro to use instead of method.
  2. Suppress marked method from output.

I'd like split it a little bit. JSReplacement still will replace method usage, but will not suppress marked method generation (but it still not be called in most cases, except reflection).
Suppress marked method from output should do some other meta attribute or some flag inside JSReplacement.

@kg
Copy link
Member

kg commented Sep 15, 2015

I'm OK with making JSReplacement not strip the method from output, as long as it doesn't break anything. Seems like a good idea.

The behavioral gap between normal invocation/reflection invocation will suck, though.

@iskiselev
Copy link
Member Author

That's why I originally suggested put same macro inside MethodBody that was marked with JSReplacement. So, body of method marked with JSReplacement included in translated results will be just same if you use it through reflection.

@kg
Copy link
Member

kg commented Sep 15, 2015

Well, there are cases where it wouldn't work, but maybe that's an acceptable compromise.

@iskiselev
Copy link
Member Author

On other side, right now we use JSReplace mostly to change invocation format if we cannot use standard - such as calling string/array/numeric methods.
Here, really, you may even prefer preserve original method body and just use some hacks to call it.
So, it really good candidate to be spitted into two separate attributes.

@kg
Copy link
Member

kg commented Sep 15, 2015

I think you're right. We want a split where one of them is 'replace method body (and inline optimistically)' and the other one is a macro replacement of sorts.

@iskiselev
Copy link
Member Author

@kg, what do you think about JSSuppressOutput meta attribute?

Right now we have:
JSIgnore - will affect both method output and method invocation
JSExternal - affect only method output, but don't allow fully suppress method from output, only method body
JSSuppressTypeDeclaration - will write type alias. Probably should be renamed to something more descriptive.

We still have no any attribute, that will affect only method output and will just mute it for some particular class/method/etc.

If JSReplacement will not suppress output, there may be some places where we want mark method additionally with JSSuppressOutput attribute.

@iskiselev
Copy link
Member Author

Looks like we should do changes proposed in this issue to implement ToString for char (#938)

iskiselev added a commit to sq/JSIL.Meta that referenced this issue Dec 22, 2015
@iskiselev
Copy link
Member Author

Probably we need to review meta attributes (and corresponding MemberInfo/TypeInfo flags) that affect on member proxy mapping, member output and member invocation translation.

I'll try to collect here tasks that should affect on our decision.

Meta attributes that help us identify member as proxy for another member
JSProxy with target defined as list of Types or list of Strings.
JSReplaceConstructor
JSExtraStaticConstructor

Meta attributes that affect how member is translated
now: each member in class marked as JSProxy use to replace method content.
proposal: use additional attribute to use method content, don't use it by default unless special property on type JSProxy specified.
topics o investigate: how works inner classes if parent class marked with JSProxy

JSExternal

member effect
class writes MakeExternalType
property writes ExternalProperty, suppress property methods
field suppress field declaration, ToCheck: how it affect default constructor?
method writes ExternalMethod

Topics o investigate: how it JSExternal affect default field value, what will happen with inner classes if parent class marked with JSExternal.

JSIgnore
Suppress type/member output
Topics o investigate: what will happen with inner classes if parent class marked with JSIgnore.

JSIgnore
Suppress type/member output
Topics o investigate: what will happen with inner classes if parent class marked with JSIgnore.

JSReplacement
Suppress method output
Proposal: don't affect member translation
Topics o investigate: how it works with non-method members and types

JSStubOnly
Each method in class will be replaces with ExternalMethod.
Topics o investigate: how it works with non-method members and inner types.

JSNeverStub
???

JSChangeName
Topics o investigate: how it works? how it affect reflection?

JSChangeToStaticMethod
???

JSExtraStaticConstructor
???

Meta attributes that affect how member invocation/usage is translated:
JSIgnore
usage will be translated as JSIL.IgnoredMember/JSIL.IgnoredType
Topics o investigate: how it affect type usage as attribute, method argument, generic argument. Looks like type reference still could be preserved in some memoised expressions. How it affects inner types?

JSReplacement
Replace member reference/invocation with provided value.
Topics o investigate: how it works with non-method members and types?

JSChangeName
use changed member name to invoke

JSChangeToStaticMethod
???

JSRuntimeDispatch
don't use method signature for invocation. How it affect properties?

JSAlwaysAccessAsProperty
don't use get_/set_ methods, use property instead. How it works with indexers?

Additional questions:
How they should work with DCE?
EDITED: how should attributes be combined?

@kg
Copy link
Member

kg commented Dec 23, 2015

Off the top of my head:

JSNeverStub

This forces the method to be translated even if you're not translating the module. I... forget what this is for, but it solved some sort of problem. Property accessors, maybe?

JSChangeToStaticMethod

This... doesn't seem to do anything anymore? I think it did once, the comment above the definition explains what it should've done. Weird, we should remove it.

JSExtraStaticConstructor

This lets you introduce additional static constructors on the type that run... after the original one, I think. This is intended to let you initialize additional global state with the type so that your externals/proxies will work. .cctor2, .cctor3, etc. I'm not sure whether this is used anywhere anymore. Without this feature you would have to replace the static constructor, which is usually not what you want.

JSRuntimeDispatch
don't use method signature for invocation. How it affect properties?

It should do the right thing, but I'm not sure whether there is a test.

JSAlwaysAccessAsProperty
don't use get_/set_ methods, use property instead. How it works with indexers?

Great question, never tested it. For indexers it should produce obj[index] in the JS, but I'm not sure whether it does.

Things here are complex enough that I'm wondering whether it would be worth it to do a full overhaul of all the Meta attributes so that we have a smaller set of better-defined attributes that have arguments to fine-tune behavior. We could map all the old attributes to the new ones, such that there's an old-Meta that's compatible with old code.

@iskiselev
Copy link
Member Author

Great, really rethinking ad re-implementing meta-attributes from scratch was what I'd thought about.

Will try to create some proposal that will limit number of attributes/combinations and attribute targets for each attribute in such way, that it still be able do anything that is used in Proxies now, but have minimal or no duplication of functions.

@iskiselev
Copy link
Member Author

Do we really need JSChangeName?

@kg
Copy link
Member

kg commented Dec 23, 2015

It really only exists for stuff like toString, and I think we can just solve that by putting an appropriate method on System.Object. The other case where it's needed is if you want to manually resolve some sort of name collision with JS. But it's possible all of those can be addressed by JSIL having a sufficiently large set of reserved words...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants