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 user defined attributes / manual overload resolution #71

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

ivancb
Copy link

@ivancb ivancb commented Aug 16, 2014

Adds support for the following attributes on class methods:

  • NoScript - Prevents the method from being registered.
  • ScriptRename(newname) - Changes the method's registration name to newname (so if you had A::foo @ScriptRename(bar) it'd be registered as A::bar).
  • ScriptAffix(affix, isSuffix) - Changes the method's registration name to include a prefix or suffix.

Utility

  • Allows binding properties and other overloaded functions correctly, if the user specifies alternate function names.
  • Allows the user to hide internal APIs (useful in games for example, where it might be beneficial to have access to renderer stats in Lua but without including the rest of the renderer's functions) with reduced boilerplate.

Issues
This still doesn't solve the base problem of registering overloaded functions and ideally pushMethod and pushOverloadedMethod would be written with a more generic pushMethod as their base.

Example

class A
{
    int _x = 10;

    int x() @property
    {
        return _x;
    }

    void x(int nx) @property @ScriptAffix("Set")
    {
        _x = nx;
    }

    void sayhello() @ScriptRename("hello")
    {
        writeln("hello");
    }

    void internalfunc() @NoScript
    {
        // ...
    }
}

A a = new A();
lua["a"] = a;

// In lua code
print(a:x()) // 10
a:Setx(2)
print(a:x()) // 2
// a:sayhello()  -- error, doesn't exist
a:hello() // "hello"
// a:internalfunc() -- error, doesn't exist

IvanC added 3 commits August 16, 2014 11:34
Modifies class registration, adding support for the usage of UDAs that
define alternate method names.
Also adds a NoScript attribute that prevents the method from being
registered.
DMD Issue 11678
@TurkeyMan
Copy link
Contributor

I've been toying with the idea of supporting overloads transparently. The function call shim would need to be enhanced to try and match the args that were received from the Lua call against the set of overloads, and call through to the appropriate overload if a match is found.
I'm also pondering the same issue in relation to default args.

@aubade
Copy link
Contributor

aubade commented Aug 17, 2014

Given that Lua's and D's type systems don't overlap all that well. I'm going to personally vote for manual overload resolution, at least by default.

@ivancb
Copy link
Author

ivancb commented Aug 18, 2014

I thought about doing automatic overload resolution but couldn't think of a way to do it without a runtime cost. You could handle overloads either in Lua or D but either way you'd have to check the parameter types and count in order to call the appropriate function.

I've fixed the unit tests added by this PR and attempted to fix some previously existing vararg related unit test failures (albeit without much success, since I can't reproduce them locally).

@TurkeyMan
Copy link
Contributor

Yeah, I think I'd do the parameter checks on the D side, it should be a little bit faster.
I do realise there would be some cost involved, but like, we're using Lua here, we can't really be too worried about performance...?

@aubade: My pull to make struct's raw userdata make the type system overlap much better, and I'm about to commit comprehensive support for enum's.

@@ -24,6 +24,22 @@ import luad.c.all;

import luad.stack;

// User defined attributes that can be placed on functions in order to change the binding behavior
// Prevents this function from being registered
enum NoScript = "NoScript";
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would use an empty struct here instead of a string. If say, PyD decided to use exactly the same method, there wouldn't be any way for the user to discriminate between Lua and Python scripts when marshalling the given method.

Also, clever uses of strings as attributes in other programs could lead to difficult-to-find bugs when used with LuaD:

alias strings = TypeTuple!("foo", "NoScript", "bar"); // happens to contain the string "NoScript"

void foo() @(strings) // This function is now NoScript for LuaD
{
}

Using instances of structs is just more hygienic in general.

@JakobOvrum
Copy link
Owner

I posted some implementation concerns in line comments.

I like the Lua-agnostic attribute names and I think the @NoScript and @ScriptRename functionality is sound, but I wonder if @ScriptAffix is too specific. It doesn't seem to buy much over @ScriptRename.

It's not entirely clear whether attribute identifiers should be capitalized or not. I'm leaning towards non-capitalization, because as discussed in the original UDA thread, it allows for the most consistency when using various types of UDAs, especially when mixing functions and constructors:

enum E { a, b, c }
enum myAttribute = E.a;
E myAttribute() { return E.a; }
struct MyAttribute {} // :(

As for overloading, I see no reason why LuaD should not do automatic overload resolution. Manual overload resolution is not a practical solution when the code to marshall is in third party code, or standard library code.

I also think the type checking needed will be extremely fast; checking Lua types is fast, and so is comparing Lua strings (required when a function is overloaded on userdata types). When overload sets are simple, overload resolution code will also be simple, and even for complex overload sets, I think branch prediction will help greatly.

@TurkeyMan
Copy link
Contributor

The point of @ScriptAffix is for use across a scope:

@scriptPrefix("MyLib_"):
  int x;
  float y;
  void func(int z);

All of them will be prefixed appropriately, where rename wouldn't work.

@TurkeyMan
Copy link
Contributor

Also, I feel 'Script' in the attribute name is pretty superfluous. Why should it be there?
And why would 'Script' be better than 'Lua'? This is a very specific library.
Personally, I would just let the module system do what it's made to do, and remove the cruft. Make the attributes one word.

@JakobOvrum
Copy link
Owner

Also, I feel 'Script' in the attribute name is pretty superfluous. Why should it be there?
And why would 'Script' be better than 'Lua'? This is a very specific library.

It opens for generalization/cooperation later. Many programs support scripting in multiple languages. Having a common set of attributes like this between say, LuaD and PyD, seems beneficial to me.

Personally, I would just let the module system do what it's made to do, and remove the cruft.

I agree, but there are cases where it's not sufficient. For example, LuaD rejects functions with UTF16 or UTF32 parameter types. I don't know if this is the right solution for that particular problem, though.

I guess the benefit is mostly for hiding certain functionality from scripts but not from D code without having to make a wrapper type just for the script-side, and for renaming symbols to match Lua conventions on the Lua side, while keeping the D style on the D side.

edit:

I realise now you're referring to the naming of the attributes. I was talking about the functionality of the attributes themselves versus say, using private instead of @NoScript or using automatic overload resolution instead of @ScriptRename.

@TurkeyMan
Copy link
Contributor

It opens for generalization/cooperation later. Many programs support scripting in multiple languages. Having a common set of attributes like this between say, LuaD and PyD, seems beneficial to me.

I see no reason why the binding to lua or python would be identical. Scripting languages are so wildly different, why would you expect such a clean mapping that a generic attribution would work? Is there some evidence to draw from?

I realise now [...]

Indeed, I'm all good with @noscript, that one's a no-brainer. I think @rename (and friends) may be useful, even in the presence of proper overload handling.

I was referring to the need to write 'Script' in the attribute name. It's not like we're worried about name collisions. It's just cruft. I'd say make them one word attributes (except @noscript, which already has precedent in @nogc).

- Shortened script UDAs and made them lowercase.
- Split ScriptAffix into prefix and suffix.
- Moved UDA logic to separate functions.
- Cleaned up pushOverloadedMethod.
@ivancb
Copy link
Author

ivancb commented Aug 18, 2014

Thanks for the feedback, I've just updated with the proposed changes, including splitting @ScriptAffix into @Prefix and @suffix.

@JakobOvrum
Copy link
Owner

I see no reason why the binding to lua or python would be identical. Scripting languages are so wildly different, why would you expect such a clean mapping that a generic attribution would work? Is there some evidence to draw from?

I think it should be pretty obvious. Pushing entire types and modules automatically makes sense for both Lua and Python, and in both cases it's useful to be able to hide certain symbols from the script.

All of them will be prefixed appropriately, where rename wouldn't work.

I just think they're too specific; how about adding a replacement scheme to @rename instead:

@rename("MyLib_$"):
int foo();
int bar();

Ideally it would also support capitalization so foo can be renamed to getFoo etc.

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