Skip to content

cast-to-int-format-error-v3#169

Closed
shewitt-au wants to merge 1 commit intoWerWolv:masterfrom
shewitt-au:cast-to-int-format-error-v3
Closed

cast-to-int-format-error-v3#169
shewitt-au wants to merge 1 commit intoWerWolv:masterfrom
shewitt-au:cast-to-int-format-error-v3

Conversation

@shewitt-au
Copy link
Contributor

@shewitt-au shewitt-au commented May 25, 2025

Another attempt at a fix for Cast to int format error (Format function no longer accepts enums) #166

The previous attempt is here.

Both methods work, but this one is more in the spirit of how shared_ptrs are intended to be used and smells less of hack.

This one uses enable_shared_from_this.

@shewitt-au shewitt-au marked this pull request as ready for review May 25, 2025 07:52
@WerWolv
Copy link
Owner

WerWolv commented May 25, 2025

We'll need more changes than that sadly. Apparently this only works when called directly from another std::shared_ptr instance
image

@shewitt-au
Copy link
Contributor Author

shewitt-au commented May 25, 2025

Ah. That's what I was worried about when I implemented v2. Adding v3 was to demonstrate the option. Although I did get excited when, for me, it worked. This error suggests that the Pattern Pattern::get_shared was called on is not managed by a shared_ptr. I'm not suprised, but I have not seen these issues on this end.

@WerWolv
Copy link
Owner

WerWolv commented May 25, 2025

Correction, I was using them wrong. The way you did it actually works!

@shewitt-au
Copy link
Contributor Author

shewitt-au commented May 25, 2025

Ah, that's a relief. Are all Patterns owned by shared_ptrs? This is important. If this is not the case you risk UB, although from what I've seen you get bad_weak_ptr.

@WerWolv
Copy link
Owner

WerWolv commented May 25, 2025

Cloning currently returns a unique_ptr so there it wouldn't work. But I believe that's actually the only thing that does so we should be able to switch that over.
Other than that, Patterns are only produced by the evaluator runtime so that shouldn't be an issue

@shewitt-au
Copy link
Contributor Author

shewitt-au commented May 25, 2025

This doesn't mean you can't have raw pointer to Patterns, just that it's lifetime is managed by one (or more obviously).

@paxcut
Copy link
Collaborator

paxcut commented May 25, 2025

I havent followed all the posts but I am wondering if the problem of recursion with templated patterns created by cloning could be revisited as well.

@shewitt-au
Copy link
Contributor Author

I'm not aware of this issue. So far I've been fixing problems that I encounter myself.

@WerWolv
Copy link
Owner

WerWolv commented May 25, 2025

Yeah I did not understand how std::enable_shared_from_this worked at all, the things I said originally were just wrong.
It works completely fine as long as the pointer is originally owned by a shared_ptr. You can use the functions with raw pointers and references too as long as that object comes from a shared_ptr

I pushed my own version of the fixes now, very closely related to yours so thank you very much for the pointers <3

@WerWolv WerWolv closed this May 25, 2025
@WerWolv
Copy link
Owner

WerWolv commented May 25, 2025

Oh yeah the templates issue could be solved like this as well potentially. I'll look into it

@shewitt-au
Copy link
Contributor Author

No worries.

@paxcut
Copy link
Collaborator

paxcut commented May 25, 2025

That would be awesome. being able to use recursion with templates makes some patterns so much easier to write. currently having to use parent and copying variables is doable but it can be hard to keep track of the information flow.

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.

3 participants