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

Remove blob #8931

Merged
merged 3 commits into from Apr 4, 2021
Merged

Remove blob #8931

merged 3 commits into from Apr 4, 2021

Conversation

LordAro
Copy link
Member

@LordAro LordAro commented Apr 2, 2021

Motivation / Problem

Unfixable GCC warnings when linking with LTO due to "optimised" code mixing heap & stack objects (it can't ever do this, but GCC warns all the same)
attempt to free a non-heap object 'hdrEmpty' [-Wfree-nonheap-object]

Description

Blob/CBlobT/CStrA is some weird code (mostly used by YAPF desync dump output) with auto-lifetime managed container objects.
Luckily, the STL exists and we can use std::string/std::vector for the same purposes.

Limitations

This is basically a 1-1 translation and the code is still real ugly.
Converting "advanced" printf commands (0x%04X) to iostream-based code is real ugly. std::format would make it nicer, but that requires C++20 :)

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

  • The bug fix is important enough to be backported? (label: 'backport requested')
  • This PR affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
    • ai_changelog.hpp, gs_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')

src/misc/dbg_helpers.h Outdated Show resolved Hide resolved
CStrA out;
out.Format("0x%04X (%d, %d)", tile, TileX(tile), TileY(tile));
return out.Transfer();
std::stringstream ss;
Copy link
Member

@frosch123 frosch123 Apr 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

personally I always do "char buf[32]" and "seprintf" in such cases.
But now you already put in the effort to find those weird stream modifiers :p

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Depends how much we care about the extra overhead of sstream/iomanip includes

@LordAro LordAro merged commit e8a94dc into OpenTTD:master Apr 4, 2021
@LordAro LordAro deleted the remove-blob branch April 4, 2021 07:01
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.

None yet

4 participants