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

[UNIFEX] Allow addition to the make_env sandbox imports #907

Open
FatigueDev opened this issue Nov 14, 2024 · 6 comments
Open

[UNIFEX] Allow addition to the make_env sandbox imports #907

FatigueDev opened this issue Nov 14, 2024 · 6 comments

Comments

@FatigueDev
Copy link

In Unifex, when you create a spec.exs file you are unable to load compiled modules. While in normal use-cases this is fine, because you're building the module yourself with only a couple of type definitions or function specs. However, when you start writing a NIF with a lot of requirements / type definitions, you run into the issue of extreme bloat and workarounds.

Take for example Raylib. It has a litany of required structs which are required to define function specs, so the best way to go about it would be to create a "Raylib.Types.ex" file which imports Unifex.Specs.DSL and calling Raylib.Types.get_types(__unifex_config)which just adds a metric tonne of type definitions, or common function specs.
exVector2, exVector3, exMatrix, exAudio, exAudioThing1, exAudioThing2- About a hundred.

Currently this is not allowed and you need to

module Exray.Textures.Image.Loader
interface [NIF, CNode]
callback :load
callback :upgrade
state_type "State"

Code.require_file("./c_src/exray/exray_types.exs")
unifex_config__ = Exray.Unifex.Types.get_types(unifex_config__) # Prepends types to the unifex_config__

# Without the above two lines, we would need to copy / paste the contents of exray_type's get_types function.
# Which would be absolutely soul destroying and unmaintainable.
# Without the above two lines, this wouldn't compile at all
spec do_function(pos_1 :: exVector2, color :: exColor) :: :ok

However it would be a lot better if we could do something akin to the following instead

exray.shared.defaultspec.ex

defmodule Exray.Shared.DefaultSpec do
  import Unifex.Specs.DSL
  def on_include(unifex_config__) do
    interface [NIF, CNode]
    callback :load
    callback :upgrade
    state_type "State"
    
    unifex_config__
  end
end

exray.shared.defaulttypes.ex

defmodule Exray.Shared.DefaultTypes do
  import Unifex.Specs.DSL
  def on_include(unifex_config__) do
    type exVector2 :: Exray.Structs.Vector2{x :: int, y :: int}
    # ... another 500 gorillion typedefs ...
    
    unifex_config__
  end
end

shared.spec.exs

module Exray.Shape.Draw
includes [
  Exray.Shared.DefaultSpec,
  Exray.Shared.Types
]

# Pseudo ->
spec draw_shape_at_position(model :: exModel, pos :: exVector2, transform :: exMatrix) :: exColor
# /Pseudo

What would be even better than simply having a 'stamp' function is that if modules that import a certain Unifex.Specs.Shared module are actually compiled separately and are #include'd into files generated by the specfile, because as it stands now using this method to import the types based off of a Code.require_file call actually flags the code generator to not define the types in the C++, or give us any option to enable it. This leads to being forced to stamp in the same types over and over, repeating ourselves and doing our best to guard our headers when we are nest-requiring different modules with different levels of "I need Vector", then "I need Image, which has Vector" then in the worst case, Fonts which have glyphs which have rects, vectors, colors and images- Which in turn have textures which hold state.

So in essence, if there can be a way to create Shared file of some kind, or lacking that being able to easily stamp from compiled .EX files I'd be very grateful; because right now I need to download Raylib for each compartmentalized module, define every type and at it leaves the load time extremely lackluster due to the amount that needs to be compiled, when it should be compiled once and included in existing specs.

You can refer to Exray for my implementation; the structure of c_src should make a lot of sense when you realize that every sub-directory is actually just a 'piece' of the Raylib.h file, cut apart for a good dev experience.

https://github.com/FatigueDev/exray

@varsill
Copy link
Contributor

varsill commented Nov 15, 2024

Hello @FatigueDev !
Thanks for reflecting on your problem that comprehensively and providing a solution suggestions.
Indeed I don't think that currently there is any way to share types definitions other than the one with Code.require.
The mechanism with on_include sounds as a good idea.
One thing is though not 100% clear to me - is it necessary to allow for sharing all arbitrary pieces of Unifex's DSL code? (what benefits come with including Exray.Shared.DefaultSpec in your example?)
Aren't the types the only parts that are worth sharing among different modules?
I am slightly worried that including :callback definition etc. might turn out to be slightly confusing and that it would be better for the user to define all the callbacks, state_type and interfaces explicitly for each native module.
CC @mat-hek

@FatigueDev
Copy link
Author

In this hypothetical, being able to include Exray.Shared.Spec would also read that the module is Exray.Shared.DefaultSpec, which would in turn append into the file we generate something like #include "path_to_generated". For the :on_load callback, in some cases this is actually important to keep the same, or in my extremely specific case- Say I define it in the specfile, but it is defined in a completely separate module cpp. This allows me to be able to quickly stamp functions in place that I know aren't going to change, for example the logger. When calling std::cout from a specific module that hasn't got the function defined, it has weird effects on the output or can sometimes crash; so it's almost better to 'pretend' the module defines it but have it actually be defined somewhere else.

The only thing that is of great importance is the ability to include predefined types; I understand the worry in regards to stamping everything out of a module definition, however it is up to the user to define what is included. They can omit it, if they so choose; or create a module that returns a function to automatically generate module tags, or types, or perform some sort of file IO skimming to spit out new spec.exs files based on text in a given header file. That's all hypothetical, but the idea is there.

Breaking it down into it's components, there's a couple of things that could be done to prevent the need for a DefaultSpec situation. There could be a MIX environment variable to match on if we should always require a defined callback. The behaviour being it makes sure to add it to the generated output and if it's not defined, it'll error at compile-time which is a good indicator you're missing the function definition. I think that state_type, at least in my (again) extremely specific use case is a stand in for the lack of support for void* in specs. Perhaps it could require an elixir reference type, like returned by state?

I'm rambling on, which I love to do, but one last thing I would enjoy is simply being able to append my own module to the sandbox. It could let me, depending on compile-time flags, enable or disable the exposure of functions based on the target platform. I can probably actually already do that, but it'd be neat if I could wrap it up in a nice module I can just hand to a spec.exs. But yeah if you want an absolutely insane take on how to use Unifex I do recommend you check the source of Exray, it's a trip.

@varsill
Copy link
Contributor

varsill commented Nov 20, 2024

Hello! I got your point, thanks for describing.
I initially thought of moving types definition to a separate header file, that would be included to all the the modules, that have includes in their .spec.exs configuration file. That would be a relatively simple change which should solve most of the problems you described (namely - the ones with repeating the types definition). It wouldn't allow you to reuse your DefaultSpec, though.
As of a more comprehensive solution, I think we might also consider using the already existing mechanism of Bundlex's deps - you can take a look how it works here: https://github.com/membraneframework/shmex/blob/41f0367850536354ca025724335495f3db4651ba/bundlex.exs#L30
The idea would be to allow using of the types defined in the spec.exs of the dependency in the dependant module.
That way we would have a single "dependencies importing" mechanism, but on the other hand we would need to couple Unifex and Bundlex quite tightly.

I'm not sure when our team will have the capacity to address that issue, as we are currently focused on some other priorities.

@FatigueDev
Copy link
Author

FatigueDev commented Nov 21, 2024

I'll fork and give it a squizz, if I come up with anything of note I'll stop back in to comment. Because I've noticed that when you use the deps method you described, Unifex tends to miss or redefine functions and types depending on if it's included in a spec or not. For example, given this spec:

Project1/c_src/testing/my_types.spec.exs

module(MyTypes)
interface([NIF, CNode])

type image :: %SomeModule.Image{x: int, y: int}

# No specs

Project2 - Has Project1's output as dep

module(MyMainProject)

# This will default to using `image` as a default instead of `struct`.
spec do_thing(my_image :: image) :: :ok

I think it may be a case of just adding another type called 'implicit' which just stamps a typedef struct #structname#, perhaps.

# Just pretend image exists for the spec.
type image :: implicit

# image is defined, but not initialized; that's handled elsewhere :)
spec get_image() :: image

The only thing that is stamped in is the struct definition which would be fine if we were using it in this spec, but we don't plan to. Of course, Project1 has it's own bundlex config just to spit it out. When we hand it to Project2 using the Unifex preprocessor, it may consider deps and it redefines core functions like unifex_load_nif. What I mean by 'may consider' is actually that since the generator bricks at function redefines, I can't really check to see if it's doing things properly until break. This isn't exactly 'intended use case', so I'm not surprised things are breaking while I work on game lib bindings. I'll see what I can come up with; thank you for taking the time to respond

@varsill
Copy link
Contributor

varsill commented Nov 22, 2024

I'll fork and give it a squizz, if I come up with anything of note I'll stop back in to comment.

Thanks, that would be great!

Concerning the types redefinition, your proposed solution with implicit types seems valid, but I think it could be solved on the bundlex/unifex level, without adding any new instruction to .spec.exs (maybe we could recognize the types defined in the dependant module and stamp them with typedef struct #structname# automatically?)
If this proves to be impossible, I believe that if we were to add some new instruction to the .spec.exs files, let it better be some "import" statement (just like the includes instruction you proposed) rather than "implicit" type inference.

@FatigueDev
Copy link
Author

The blocker I ran into here specifically is that each .spec.exs is expected to be it's own standalone environment. While this would be fine if the application didn't have interconnected dependencies, what this actually means is that even when you don't add anything into the file it'll generate all of the boilerplate that Unifex requires. This causes every definition in the NIF to double up, causing a compile time error.

Fixing this would require a spec-time function call which would store_config something like :no_boilerplate, then during code generation disabling as much as humanly possible from the spec. This would, in theory, allow us to scan the specs value where we assign user_structs and append each type from the current spec's dependencies.

I think there just has to be more control in the .spec.exs about what the specfile should generate, or rather what it shouldn't with a list of atoms we can use to say something like

exclude_generation_for([
  :main,
  :type_definitions,
  :function_definitions,
  :unifex,
  :shmex,
  :state_functions,
  :all_the_other_stuff_i_forgot
])

Where during the Common, NIF and CNode Generators it just has a simple

if excludes_generation?(from_exclude_generation_for_list), do: # what was already there #

Makes me wish there was a return function in a lot of ways, but overall I think it's just.. Nah. I can't get it to compile and I spent two days on just this one problem with a lot of different ways. A use_dep(Module) in the .spec.exs, precompile-time dep check and addition but that created boilerplate which caused type double-ups.. It's a tricky situation with lots of solutions and I think an ok way to go about it might just be wrapping the generated files in a namespace with the name of the spec, add heaps of #ifndef #define #endif's.. But that's not tested and may not even work.

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

No branches or pull requests

2 participants