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

[cling] Emit const variables only once #13614

Merged
merged 1 commit into from
Oct 3, 2023

Conversation

hahnjo
Copy link
Member

@hahnjo hahnjo commented Sep 6, 2023

Otherwise they are emitted as internal and we get double-construction and -destruction on the same memory address due to the way we promote internal declarations in KeepLocalGVPass.

According to upstream tests, the de-duplication doesn't work on Windows (yet), but I think this problem is severe enough to fix it on the other platforms sooner rather than later.

Fixes #13429

@hahnjo hahnjo added the in:Cling label Sep 6, 2023
@hahnjo hahnjo self-assigned this Sep 6, 2023
@phsft-bot

This comment was marked as outdated.

@phsft-bot

This comment was marked as outdated.

@phsft-bot

This comment was marked as resolved.

@phsft-bot

This comment was marked as resolved.

@phsft-bot

This comment was marked as resolved.

@phsft-bot

This comment was marked as resolved.

@github-actions
Copy link

github-actions bot commented Sep 6, 2023

Test Results

       11 files         11 suites   2d 8h 54m 2s ⏱️
  2 483 tests   2 481 ✔️ 0 💤 2
26 228 runs  26 225 ✔️ 0 💤 3

For more details on these failures, see this check.

Results for commit f0130ce.

♻️ This comment has been updated with latest results.

@phsft-bot

This comment was marked as outdated.

@phsft-bot

This comment was marked as outdated.

Otherwise they are emitted as internal and we get double-construction
and -destruction on the same memory address due to the way we promote
interal declarations in KeepLocalGVPass.

According to upstream tests, the de-duplication doesn't work on Windows
(yet), but I think this problem is severe enough to fix it on the other
platforms sooner rather than later.

Fixes root-project#13429
@phsft-bot
Copy link
Collaborator

Starting build on ROOT-performance-centos8-multicore/soversion, ROOT-ubuntu2204/nortcxxmod, ROOT-ubuntu2004/python3, mac11/noimt, mac12arm/cxx20, windows10/default
How to customize builds

@hahnjo hahnjo marked this pull request as ready for review October 3, 2023 12:23
@hahnjo hahnjo merged commit 2dc5786 into root-project:master Oct 3, 2023
10 of 16 checks passed
@hahnjo hahnjo deleted the cling-const branch October 3, 2023 15:03
@makortel
Copy link

makortel commented Oct 3, 2023

Can I ask if this fix is planned to be backported (say to 6.28)?

@hahnjo
Copy link
Member Author

hahnjo commented Oct 3, 2023

Can I ask if this fix is planned to be backported (say to 6.28)?

#13795 😃 🚀 (edit: I was already working on the backport, even though now the timeline of (public) events looks rather weird)

@phsft-bot
Copy link
Collaborator

Build failed on ROOT-ubuntu2004/python3.
Running on root-ubuntu-2004-3.cern.ch:/home/sftnight/build/workspace/root-pullrequests-build
See console output.

Errors:

  • [2023-10-03T16:00:17.119Z] CMake Error at /home/sftnight/build/workspace/root-pullrequests-build/rootspi/jenkins/root-build.cmake:1126 (message):

maksgraczyk pushed a commit to maksgraczyk/root that referenced this pull request Jan 12, 2024
Otherwise they are emitted as internal and we get double-construction
and -destruction on the same memory address due to the way we promote
internal declarations in KeepLocalGVPass.

According to upstream tests, the de-duplication doesn't work on Windows
(yet), but I think this problem is severe enough to fix it on the other
platforms sooner rather than later.

Fixes root-project#13429
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TROOT::EndOfProcessCleanups fails when using TCMalloc on different destructors
4 participants