Skip to content

Make it explicit we are currently using a signed char - #13375

Merged
ktf merged 2 commits into
AliceO2Group:devfrom
ktf:fix-signed-char
Aug 13, 2024
Merged

Make it explicit we are currently using a signed char#13375
ktf merged 2 commits into
AliceO2Group:devfrom
ktf:fix-signed-char

Conversation

@ktf

@ktf ktf commented Aug 13, 2024

Copy link
Copy Markdown
Member

On x86 Linux char is assumed to be signed, however that's an implementation detail.

This makes sure we are explicit about the sign-ness.

@github-actions

Copy link
Copy Markdown
Contributor

REQUEST FOR PRODUCTION RELEASES:
To request your PR to be included in production software, please add the corresponding labels called "async-" to your PR. Add the labels directly (if you have the permissions) or add a comment of the form (note that labels are separated by a ",")

+async-label <label1>, <label2>, !<label3> ...

This will add <label1> and <label2> and removes <label3>.

The following labels are available
async-2023-pbpb-apass3
async-2023-pbpb-apass4
async-2023-pp-apass4
async-2024-pp-apass1
async-2022-pp-apass7
async-2024-pp-cpass0

@ktf

ktf commented Aug 13, 2024

Copy link
Copy Markdown
Member Author

@davidrohr this fixes one of the two tests on ARM.

davidrohr
davidrohr previously approved these changes Aug 13, 2024

@davidrohr davidrohr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

makes sense, but the error during the RTC test is genuine :(.

In a few places the code checks for the value being positive, which
of course is always true in case char is unsigned.
@ktf

ktf commented Aug 13, 2024

Copy link
Copy Markdown
Member Author

Added the specialization for the printout.

Comment thread GPU/GPUTracking/utils/qconfig_helpers.h Outdated
@AliceO2Group AliceO2Group deleted a comment from alibuild Aug 13, 2024
@alibuild

Copy link
Copy Markdown
Collaborator

Error while checking build/O2/fullCI for c731b2a at 2024-08-13 16:18:

## sw/BUILD/O2-latest/log
c++: error: unrecognized command-line option '--rtlib=compiler-rt'
c++: error: unrecognized command-line option '--rtlib=compiler-rt'


## sw/BUILD/o2checkcode-latest/log
--
========== List of errors found ==========
++ GRERR=0
++ grep -v clang-diagnostic-error error-log.txt
++ grep ' error:'
/sw/SOURCES/O2/13375-slc8_x86-64/0/GPU/GPUTracking/Global/GPUChainITS.cxx:40:14: error: use '= default' to define a trivial destructor [modernize-use-equals-default]
/sw/SOURCES/O2/13375-slc8_x86-64/0/GPU/GPUTracking/Interface/GPUO2Interface.cxx:49:17: error: use '= default' to define a trivial default constructor [modernize-use-equals-default]
++ [[ 0 == 0 ]]
++ exit 1
--

Full log here.

@ktf
ktf merged commit 8cd5556 into AliceO2Group:dev Aug 13, 2024
@ktf
ktf deleted the fix-signed-char branch August 13, 2024 21:23
@ktf

ktf commented Aug 13, 2024

Copy link
Copy Markdown
Member Author

Verified to fix one of the two ARM tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants