Cast to int format error (Format function no longer accepts enums)

[shewitt-au]

This is a "fix" for this issue (Format function no longer accepts enums). I use quotes because I don't have a deep understanding of how this stuff works. This is the smallest change to PatternRef I've come up with that fixes the issue. Hope it helps.

[shewitt-au]

I'm a bit disappointed with myself on this one. I wasted too much time trying to solve the problem analytically instead of brute-forcing it. I hope my fix doesn't negate the point of PatternRef.

[WerWolv]

That does indeed make the pattern refs basically useless.
The reason I introduced these reference patterns was that format functions basically had to clone enormous pattern trees on each invocation which made complex patterns cause ImHex's UI to freeze up for a long time.
I think the "right" solution would be to basically make all the functions in the Pattern class virtual and have the PatternRef class overload and forward each of these calls to the referenced pattern. I was hoping we could avoid that but I obviously broke things by not doing so

[shewitt-au]

I had a feeling the fix would defeat the point.

[shewitt-au]

I would appreciate a more elaborate explanation of the need for cloning if you’ve got the time at some point.

[shewitt-au]

I can see why you would endeavor to avoid the "right" solution.

[shewitt-au]

PatternRef was ambitious:)

[WerWolv]

The issue is basically that the evaluator works with std::shared_ptr to represent objects. Now, when we call pattern->getFormattedValue(), we need to pass the current pattern back to the evaluator so it can be accessed as a function argument. But the this pointer is of course not a shared_ptr. It's just a regular pointer. And we can't turn it into a shared_ptr because that would cause a double free. So the only option is to clone it (or shallow clone it which is what I attempted to do with the PatternRef type).
I'm open for better ideas

[shewitt-au]

Thanks for the clarification. It’s bed time now, actually well past bed time, but I’m going to contemplate this dilemma.

[shewitt-au]

I've been looking through the code. I'm not sure when you mean when you say, "But the this pointer is of course not a shared_ptr". Is the fundamental problem caused by a cycle of shared_ptrs?

[WerWolv]

Here's one such issue:

[[nodiscard]] std::string toString() override {
    u128 value = this->getValue().toUnsigned();
    return Pattern::callUserFormatFunc(PatternRef::create(this), true).value_or(getEnumName(this->getTypeName(), value, m_enumValues));
}

The Pattern::callUserFormatFunc takes in a Token::Literal which is the thing used everywhere in the evaluator to pass values around. When passing around a Pattern, you pass it a std::shared_ptr<Pattern>.
Here though we want to pass the current pattern to it using the this pointer. But that is a Pattern* that is owned by some other shared_ptr somewhere. So we can't wrap it up into a shared_ptr again because we don't own it.

This is what we really want to do:

[[nodiscard]] std::string toString() override {
    u128 value = this->getValue().toUnsigned();
    return Pattern::callUserFormatFunc(this, true).value_or(getEnumName(this->getTypeName(), value, m_enumValues));
}

This is how we did it before:

[[nodiscard]] std::string toString() override {
    u128 value = this->getValue().toUnsigned();
    return Pattern::callUserFormatFunc(this->clone(), true).value_or(getEnumName(this->getTypeName(), value, m_enumValues));
}

[shewitt-au]

Yeah, that a bit of a bind alright. What should be simple is made hard by the abstractions that are in place.

[shewitt-au]

No good