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

add GCC unreachability intrinstic on false branch of assert #2020

Merged
merged 1 commit into from
Oct 21, 2022

Conversation

scarf005
Copy link
Member

Summary

SUMMARY: Bugfixes "add GCC unreachability intrinstic on false branch of assert"

Purpose of change

/home/scarf/repo/cata/Cataclysm/src/vpart_position.h:52:20: error: ‘*(const vpart_position*)((char*)&<unnamed> + offsetof(vehicle_part_iterator<vehicle_part_range>,vehicle_part_iterator<vehicle_part_range>::vp_.cata::optional<vpart_reference>::<unnamed>)).vpart_position::part_index_’ may be used uninitialized [-Werror=maybe-uninitialized]
   52 |             return part_index_;
      |                    ^~~~~~~~~~~

Describe the solution

  • redefine assert to use __builtin_unreachable() on GCC
  • only applied on vpart_range.h since BN does not have cata_assert.h

Describe alternatives you've considered

  • cherry-pick cata_assert.h

Testing

Compiles on GCC

@joveeater
Copy link
Collaborator

This shouldn't be needed. Assertions should work like this anyway. DDA needed to do this but only because they use their own implementation of assertions so they had to mimic this behavior.

The error message that you gave, i.e. vpart_position.h line 52, doesn't have an assertion in that function so it seems very weird that this fixes that problem. More importantly this could trip people up. It means including a seemingly innocuous vehicle parts header causes assertions to behave differently and not get checked even on the debug versions.

Redefining assert should be a last resort. I would try giving the property a default value first. Hopefully that'll shut gcc up, if not I'd just go with the pragmas until they sort out their junk warnings.

@scarf005 scarf005 force-pushed the fix-gcc-unreachable branch from 4a24020 to 3f85450 Compare October 14, 2022 00:08
@Coolthulhu Coolthulhu self-assigned this Oct 15, 2022
@Coolthulhu Coolthulhu merged commit a4bdfa9 into cataclysmbnteam:upload Oct 21, 2022
@scarf005 scarf005 deleted the fix-gcc-unreachable branch October 21, 2022 14:41
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