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

Macro generating redundant source files #86

Open
paulsgcross opened this issue Mar 2, 2023 · 6 comments
Open

Macro generating redundant source files #86

paulsgcross opened this issue Mar 2, 2023 · 6 comments

Comments

@paulsgcross
Copy link

paulsgcross commented Mar 2, 2023

Due to the numbering scheme that json2object employs, it would seem that redundant source files are being generated. For example, for a JsonParser that parses an object such as MyObject, it is possible (after enough builds) for the macro to generate JsonParser_0 and JsonParser_2 that both perform the exact same role. This means, after enough compilations, we can have as many as hundreds of redundant parsers. While performing a complete rebuild circumvents this, it is not ideal and could also lead to source code conflicts if we choose to only rebuild.

My suggestion would be to name the parsers not by integer, but by the type they operate on (i.e. JsonParser_MyObject), while the macro can throw in a quick "if type exists" check using the object name to establish if it needs to recreate the type or not.

@elnabo
Copy link
Owner

elnabo commented Mar 2, 2023

Hello, my memory is a bit fuzzy about this but I think your suggestion was the original design.

However that way, the parser were never regenerated by the compilation server even in case of changes.

I agree that the current system is suboptimal, but I don't see an elegant solution that doesn't come with worse drawbacks

@paulsgcross
Copy link
Author

Ah, I had wondered if it was the original design -- thanks for clearing that up.

Out of curiosity, do you remember what it was in the original design that caused issues? I know that Context.registerModuleDependency can be used to force re-typing if specific files change.

@elnabo
Copy link
Owner

elnabo commented Mar 3, 2023

I think the problem was that once we defined a type, if we tried to define an other one with the same name, we would often have error like "Type name JsonParser_1 is redefined from module JsonParser_1" in the compilation server.

@paulsgcross
Copy link
Author

When defining the type via the macro, did you also make sure to include the package path in it's name?

For example you transform path.to.MyObject into JsonParser_path_to_MyObject. I have done this before and it's a sure-fire way to prevent naming conflicts like the one you mentioned above. Have I understood correctly?

@elnabo
Copy link
Owner

elnabo commented Mar 6, 2023

Yes we did do that and it caused other problem due to name length I think.

But the conflict I mentioned should appears as long you as try to define twice a class with the same name in the same compilation server.

@paulsgcross
Copy link
Author

My second thought, if name length is an issue, would be to hash the object name and path into something more managable. Happy to fork off this repo and give some ideas a go and see how it plays out.

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