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

clang: fix Any linker error with multiple compilers #16855

Merged
merged 1 commit into from
Apr 22, 2023
Merged

Conversation

filnet
Copy link
Contributor

@filnet filnet commented Apr 21, 2023

reverts this change: llvm/llvm-project@95b27b2

fixes the Rust build, see #16847

@filnet
Copy link
Contributor Author

filnet commented Apr 21, 2023

Please consider merging. New Rust version is dependent on this new version of LLVM.

@lazka
Copy link
Member

lazka commented Apr 22, 2023

Can we change it to the new workaround? Or would you prefer to do that later?

@filnet
Copy link
Contributor Author

filnet commented Apr 22, 2023

Can we change it to the new workaround? Or would you prefer to do that later?

I'll adapt the patch to the new workaround.

@filnet filnet changed the title clang: revert llvm msvc workaround clang: fix Any linker error with multiple compilers Apr 22, 2023
@filnet
Copy link
Contributor Author

filnet commented Apr 22, 2023

C'est parti...

@lazka lazka merged commit 7dee807 into msys2:master Apr 22, 2023
@mati865
Copy link
Collaborator

mati865 commented Apr 23, 2023

I think this patch shouldn't be merged for LLVM based subsystems but since it's merged already we can keep probably keep it (unless something regress).

@filnet
Copy link
Contributor Author

filnet commented Apr 23, 2023

I think this patch shouldn't be merged for LLVM based subsystems but since it's merged already we can keep probably keep it (unless something regress).

Not sure I understand but the patch should land upstream sometimes in the future.

EDIT: I the reason is that the change from 0 to 1 will break things, then it should not according to the llvm maintainers.
The 0 or 1 is apparently irrelevant.

@mati865
Copy link
Collaborator

mati865 commented Apr 23, 2023

Oh, it's straight from the phabricator. Sorry, missed that bit.
So let's just keep an eye on it and see if sticks or gets reverted.

@filnet
Copy link
Contributor Author

filnet commented Apr 23, 2023

It is not yet merged : https://reviews.llvm.org/D148953

@filnet
Copy link
Contributor Author

filnet commented Apr 24, 2023

The fix has been merged upstream.

For sake of completeness, an alternative solution mentioned in the fix discussion:

One could stand on the point that if objects from different compilers are linked, the user needs to supply -f[no-]zero-initialized-in-bss to make sure they behave the same.

We should keep that in mind should a similar issue popup.

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