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

Template/Generic Compilation #51

Closed
JohnDog3112 opened this issue Oct 1, 2024 · 4 comments · Fixed by #53
Closed

Template/Generic Compilation #51

JohnDog3112 opened this issue Oct 1, 2024 · 4 comments · Fixed by #53
Assignees
Labels
help wanted Extra attention is needed

Comments

@JohnDog3112
Copy link
Collaborator

I mentioned this over the phone, however, I found a temporary solution and would like some input on why it was necessary.

The problem stems from a generic LinkedList implementation I made:

template<typename T>
class LinkedListRoot {
   public:
   LinkedList<T>* root = NULL;
   LinkedListRoot();
   ~LinkedListRoot();
   void addNode(T val);

   private:
   LinkedList<T>* tail = NULL;
};

When this was implemented in pong-menu.h and pong-menu.cpp (where it was used), everything worked fine. However, when I was setting up a new class for rendering centered text I moved it to extra.h and extra.cpp so it could be used among multiple files. When I did this, I started getting the following linking errors:

wasm-ld: error: out/pong-menu.o: undefined symbol: LinkedListRoot<MenuButton>::LinkedListRoot()
wasm-ld: error: out/pong-menu.o: undefined symbol: LinkedListRoot<MenuButton>::addNode(MenuButton)
wasm-ld: error: out/pong-menu.o: undefined symbol: LinkedListRoot<MenuButton>::addNode(MenuButton)
wasm-ld: error: out/pong-menu.o: undefined symbol: LinkedListRoot<MenuButton>::addNode(MenuButton)
wasm-ld: error: out/entry-point.o: undefined symbol: LinkedListRoot<MenuButton>::~LinkedListRoot()

I found that moving the MenuButton struct into extra.h and adding:

template class LinkedListRoot<MenuButton>;

to extra.cpp fixed it.

I was wondering if you knew what might be causing the explicit declaration to be necessary and if there's any other way I could fix this?

@JohnDog3112 JohnDog3112 added the help wanted Extra attention is needed label Oct 1, 2024
@JohnDog3112
Copy link
Collaborator Author

I figured out that putting all of the function definitions in the header fixed the issue. I'm still not quite sure why it's necessary, but it's a better solution than needing to define every variant of the class in extra.cpp.

@twiddlingbits
Copy link
Owner

I had answered this in a different issue. #14. Here is what I found:

However, I keep getting linking issues with it. Do you know what might be causing these?

chatGPT says: https://chatgpt.com/share/66f9d97d-481c-800a-9372-c31f27465483

Here is what I wrote before I aske chatGPT, but I haven't really used templates in c++ (i do use them in typescript):

If you show me the code (commit it, or provide a link, or whatever), i can take a look.
But I will say that in the past whenever I have tried to put actual code in a .h file it has never ended well and I always end up putting it in a .cpp file (and the class detentions in a .h file, like canvas.cpp). When it's in the .h, then you end up with name collision (same name compiled into multiple .o). I think you can work around it with appropriate use of inline, but that seems to be very fragile and confusing. I gave up getting it to work the one time i tried recently. You could make the code static, which might avoid the collision. But then you still have multiple copies of the same code, which doesn't seem clean, and there are probably other issues.

@twiddlingbits
Copy link
Owner

i sent you an email with an intro to don, who can explain this in detail.

@twiddlingbits
Copy link
Owner

please close this if you think its fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants