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

fix: Fix missing includes #5295

Merged
merged 1 commit into from
Apr 6, 2023

Conversation

OlegGirko
Copy link

The <stdexcept> include is needed for std::runtime_error definition.
The <cstdint> include is needed for uint8_t and uint32_t definition.

Issue being fixed or feature implemented

Compilation failure with GCC 13.
GCC 13 is more strict about missing includes that were included indirectly by previous versions of GCC.

What was done?

Added missing includes.

How Has This Been Tested?

Successful compilation on Fedora 38 with GCC 13. All tests passed successfully.

Breaking Changes

None.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation

@OlegGirko OlegGirko changed the title Fix missing includes fix: Fix missing includes Apr 5, 2023
@OlegGirko OlegGirko force-pushed the fix_missing_includes branch from 212d235 to 9d01937 Compare April 5, 2023 21:20
@UdjinM6 UdjinM6 added this to the 19 milestone Apr 5, 2023
The <stdexcept> include is needed for std::runtime_error definition.
The <cstdint> include is needed for uint8_t and uint32_t definition.

Signed-off-by: Oleg Girko <[email protected]>
@OlegGirko OlegGirko force-pushed the fix_missing_includes branch from 9d01937 to 89f9c28 Compare April 5, 2023 21:24
@OlegGirko
Copy link
Author

Rebased on updated develop branch.

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

LGTM, utACK

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

utACK for squash merge

@PastaPastaPasta PastaPastaPasta merged commit 9b74c70 into dashpay:develop Apr 6, 2023
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Apr 6, 2023
The `<stdexcept>` include is needed for `std::runtime_error` definition.
The `<cstdint>` include is needed for `uint8_t` and `uint32_t`
definition.

## Issue being fixed or feature implemented
Compilation failure with GCC 13.
GCC 13 is more strict about missing includes that were included
indirectly by previous versions of GCC.


## What was done?
Added missing includes.

## How Has This Been Tested?
Successful compilation on Fedora 38 with GCC 13. All tests passed
successfully.


## Breaking Changes
None.


## Checklist:
<!--- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation

Signed-off-by: Oleg Girko <[email protected]>
Co-authored-by: Oleg Girko <[email protected]>
@OlegGirko OlegGirko deleted the fix_missing_includes branch April 6, 2023 12:53
PastaPastaPasta pushed a commit that referenced this pull request May 26, 2023
…acktraces (#5375)

## Description

Pull request was inspired by the need to debug lock problems when
working on #5352.

As far as I'm aware, only macOS has `-Werror=thread-safety` as part of
its default `CXXFLAGS` despite the capability being present on Linux as
well. This PR introduces thread safety checks for that into our thread
sanitizer build.

Additionally, since we're using Clang, something that on first glimpse,
appears to be something that `stacktraces.cpp` isn't happy with, due to
`-Wl,-wrap` being available only on GCC, that no longer seems to be the
case, since the version of Clang with comes with `focal`, its `lld`
_does_ have support for `-wrap` (see [man page for `lld` on
`focal`](https://manpages.ubuntu.com/manpages/focal/en/man1/lld.1.html)).

The current `stable` version of Clang/LLVM is 15, at the time of this
pull request (see https://apt.llvm.org/) but `focal` ships with an older
version, requiring us to use the official LLVM APT repository. I feel we
should be testing with recent compilers alongside the ones shipped by
LTS distributions.

Certain bugs are only made apparent when testing on rolling release
distros or distros that have faster update cycles, like Fedora (see
#5295 for an illustration of that),
which ship with more recent compilers. Until we overhaul our CI systems
to test using those distros directly (our current infrastructure is
centered around using a "development image" with an LTS distro as the
base), this is the best we can do.

A similar pull request testing against the latest GCC stable will be
welcome as that is currently outside the scope of this PR as the changes
made were to make sure that builds were operating as expected on
Clang/LLVM 15.

---------

Co-authored-by: UdjinM6 <[email protected]>
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