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

Reduce reliance on C++ types in the public API #121

Closed
LoganDark opened this issue Nov 4, 2021 · 13 comments · Fixed by #216
Closed

Reduce reliance on C++ types in the public API #121

LoganDark opened this issue Nov 4, 2021 · 13 comments · Fixed by #216
Labels
enhancement New feature or request

Comments

@LoganDark
Copy link
Contributor

I understand how Luau evolved to contain so much C++, especially as Roblox uses C++ internally for the game engine and many other things.

Whatever is used for the implementation, C++ in the header files specifically raises lots of portability concerns. For example, most languages are simply unable to process C++ templates in any capacity, let alone classes.

I'm trying to create bindings to Luau so that I can use it from Rust, but bindgen famously cannot understand templates at all, and I don't imagine other languages have any solutions either. Furthermore, it's not even possible to use any templated functions or classes because templates require expansion before use.

This problem mostly attributes itself to the 500+ uses of STL types in the header files, specifically AST, Compiler and Analysis, which is pretty much 3/4ths of the project at this point.

  • Analysis is the worst offender, using STL types at least 400 times;
  • AST is next, with slightly over 100 uses;
  • Compiler uses STL types around 30 times;
  • VM uses it 0 times

Using any module other than VM (compiler especially, compiler is quite important) from another programming language is currently extremely difficult, manual, and error-prone with the current API surface. STL types are generally magical and cannot be understood by any language that is not C++, so it's usually best to keep them as implementation details, not exposed in the public API as they are now. Automated tools cannot digest them so all FFI must be done manually by a human which can introduce subtle memory bugs and undefined behavior just like that.

Luau is already in its situation with template types everywhere. Is it even possible to solve this now?

@LoganDark LoganDark added the enhancement New feature or request label Nov 4, 2021
@zeux
Copy link
Collaborator

zeux commented Nov 4, 2021

We do have plans to add a C++-free interface to compile code. I agree it's important, it would allow using Luau compiler + VM across a C or C-like boundary. This could be done with an extra header like CompilerC.h that exports an extra C-friendly interface, still implemented in Compiler.cpp. PR welcome ;)

I don't think it's realistic for us to do this for AST or Analysis.

@LoganDark
Copy link
Contributor Author

LoganDark commented Nov 4, 2021

This could be done with an extra header like CompilerC.h that exports an extra C-friendly interface

It's a bit weird that Luau uses .h for C++ files. Everything outside of VM should probably be .hpp and then there could just be Compiler.h for the pure C version.

Of course it could also continue with the convention that all-lowercase headers are pure C and CamelCase headers are C++, because that seems to be a thing here as well.

Nevermind - forgot Windows was stupid - and has a case-insensitive filesystem. Scratch that idea.

PR welcome ;)

I'll look into it

I don't think it's realistic for us to do this for AST or Analysis.

It's not realistic for you to do it for anything, all things considered. Roblox as a company doesn't really have any C-only products to use it in :)

@zeux
Copy link
Collaborator

zeux commented Nov 4, 2021

Of course it could also continue with the convention that all-lowercase headers are pure C and CamelCase headers are C++, because that seems to be a thing here as well.

We don't really have any pure C headers; in general in Roblox codebase ".h" is used as "C/C++ header" without differentiation. This is a pretty common style in C++ projects as well, although some projects indeed use .hpp.

@LoganDark
Copy link
Contributor Author

LoganDark commented Nov 4, 2021

It's a bit involved to define a C version of the compiler interface without pulling in tons of C++ stuff. For example, the error gives (and/or takes) a Location, which is a C++ class, and in order to get access you have to import Luau/Common.h which is also full of C++ stuff, so I'd have to redefine it as a struct and have duplicated types.

Is that something you'd be willing to accept, if I had to redefine some C++ classes as C structs? They would stay C++ classes in other parts of the codebase, but the C-only header file would have its own struct version.

@zeux
Copy link
Collaborator

zeux commented Nov 4, 2021

I think you only need to expose Luau::compile(), that's what we were planning to do. It takes source code and produces a bytecode blob - this bytecode blob will encode an error if the error happens, which will then trigger the error at runtime when you use luau_load. So the error location is effectively embedded into the resulting blob.

@LoganDark
Copy link
Contributor Author

LoganDark commented Nov 4, 2021

I think you only need to expose Luau::compile(), that's what we were planning to do. It takes source code and produces a bytecode blob - this bytecode blob will encode an error if the error happens, which will then trigger the error at runtime when you use luau_load. So the error location is effectively embedded into the resulting blob.

What about all the options structs and the bytecode encoder? I can expose functions to create/free a bytecode encoder and you'd just have to pass the pointer to compile - would that be okay?

@zeux
Copy link
Collaborator

zeux commented Nov 4, 2021

You can just skip the bytecode encoder. I forgot about the options - these are good to preserve. I think the headers with options are self-sufficient and don't rely on STL?

@LoganDark
Copy link
Contributor Author

LoganDark commented Nov 4, 2021

I think the headers with options are self-sufficient and don't rely on STL?

CompileOptions are provided in the Compiler header itself, so I will have to redefine it, possibly as lua(u)_CompileOptions. But then we'll have two identical definitions, so one of them would have to go. Most likely the one in the C++ header file, which could import the C header file to use its CompileOptions instead.

@andyfriesen
Copy link
Collaborator

If you wanted to build an interface to the type checker in another language, it wouldn't be worth it to reflect the whole interface over. You probably just need a handful of things:

  • A way to subclass Luau::FileResolver and Luau::ConfigResolver
  • A way to allocate a Luau::Frontend instance and invoke its check, lint, and markDirty methods.
  • Maybe an accessor to unpack Luau::TypeErrorData? This would admittedly be pretty awkward. It depends on whether Luau::toString(const Luau::TypeError&) is sufficient for your needs.

If you need more than that, you'll probably have to build it out in C++, but you should still be able to provide a narrow interface to that.

@zeux
Copy link
Collaborator

zeux commented Nov 6, 2021

I think the real action item here is an FFI-friendly interface for the compiler; this will let projects that use other languages or projects that don't use STL includes for policy reasons to still use compiler + VM. Everything else is probably best left outside of scope of this project, at least for now - you can always wrap Frontend or type introspection.

Also worth noting that right now we plan to be ABI- and source-compatible within reason in the C-like API surface, but don't really have any sort of compat guarantees for C++ surface as we'd like to keep an ability to refactor at will. This is currently not documented, which we can fix after we add C-friendly compiler interface, and will result in a guarantee that loosely matches LLVM - LLVM has a C interface that is stable but only exposes a subset of LLVM functionality, and to build a fully featured toolchain you likely need to use the C++ interface. Similarly for Luau, the C interface is going to be sufficient for basic integration, but for something fully featured that includes type checking, autocomplete, et al, you really will need to work with the C++ interface, at least for now.

We'll keep this issue open and will close it when a compiler interface arrives and the ABI/source compat gets documented in README.md

@LoganDark
Copy link
Contributor Author

@zeux If you're interested, I've already implemented a pure C interface to the compiler that doesn't raise exceptions, but manages to still communicate all the information from the exception(s) that would have been raised when there is an error. It does this is through a tagged union, which can represent a successful compilation, any amount of parse errors, or a compile error.

It is quite complex, though, and more allocations than I'd like are required to shuffle blocks of data around. Perhaps Luau could adopt something like that? (That repository is licensed under GPLv3, but I still have full rights to the code and can offer it or a modified version of it to you under MIT.)

@zeux zeux closed this as completed in #216 Nov 19, 2021
zeux added a commit that referenced this issue Nov 19, 2021
- Improve error recovery during type checking
- Initial (not fully complete) implementation for singleton types (RFC RFC: Singleton types #37)
- Implement a C-friendly interface for compiler (luacode.h)
- Remove C++ features from lua.h (removed default arguments from luau_load and lua_pushcfunction)
- Fix lua_breakpoint behavior when enabled=false
- Implement coroutine.close (RFC RFC: coroutine.close #88)

Note, this introduces small breaking changes in lua.h:

- luau_load env argument is now required, pass an extra 0
- lua_pushcfunction now must be called with 3 arguments; if you were calling it with 2 arguments, pass an extra NULL; if you were calling it with 4, use lua_pushcclosure.

These changes are necessary to make sure lua.h can be used from pure C - the future release will make it possible by adding an option to luaconf.h to change function name mangling to be C-compatible. We don't anticipate breaking the FFI interface in the future, but this change was necessary to restore C compatibility.

Closes #121
Fixes #213
@LoganDark
Copy link
Contributor Author

The luacode.h version of the compiler loses a ton of information, so I'll keep using my glue code. Thanks though.

@zeux
Copy link
Collaborator

zeux commented Nov 19, 2021

It's obviously up to you as to what you use; luacode.h should be sufficient for most use cases that involve pure code compilation, e.g. it's sufficient for Roblox.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging a pull request may close this issue.

3 participants