Add prefix to problematic stb_truetype equal() func#905
Add prefix to problematic stb_truetype equal() func#905RobLoach merged 1 commit intoImmediate-Mode-UI:masterfrom
Conversation
|
Is the lack of prefix causing any symbol conflicts in your code? I didn't found anything in Nuklear where this would be the case. Shouldn't this be reported to STB upstream first? I feel a bit uneasy that you're trying to patch it here. Someone may overwrite your changes when bumping this file. |
If you take a look at git history of that file (
I'm afraid your patch will meet the same fate ;/ |
It's conflicting with my code not Nuklear or it would have been discovered much immediately. However, whether as stb_truetype alone, or as part of Nuklear, having a function named "equal" in a C library, especially a single header library, is problematic. Sure it's declared static and I could put it in a separate TU, but I tend to prefer unity builds, (or at least unity-ish builds), so that doesn't help. And honestly, I'm surprised someone even made a function so small that's used literally twice.
Theoretically, but stb_truetype hasn't been updated in almost 5 years so I wouldn't hold my breath for a new release. Even if there were an update eventually, unless it's a significant fix/improvement there's no real need to update our local copy, and if it is worth it to update, and the people involved in updating forgot to keep the change, no big deal. Remember, no one forces developers to update Nuklear (and most developers using it seriously make local changes to libraries like Nuklear anyway), meaning if/when stb_truetype get's updated and this eventually possibly gets overwritten, they won't even notice even if it would affect them because they're not grabbing the latest version immediately just because it exists because updating means they have to re-apply all their changes again and handle any conflicts. Anyway, long story short, I think it's worth changing, whether prefixing or eliminating it entirely. |
There was a problem hiding this comment.
I just entered an upstream issue for this: nothings/stb#1898
I personally don't mind this being patched here. It's not a big deal.
|
Yes, this is absolutely a bug that we will fix upstream as well. |
RobLoach
left a comment
There was a problem hiding this comment.
equals(AWESOME);
// => true
I don't know how this function has lasted so long in such a widely used library but we are on the latest version of stb_truetype and it's still there.
It's such a small function and only used twice so I'm also fine with just eliminating it entirely but I figure just changing the name keeps it closer to upstream.