Skip to content

Rewrite MachOCASWriter to be a derived class of MachObjectWriter #9111

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

Conversation

rastogishubham
Copy link

This patch rewrites MachOCASWriter to be a derived class of MachObjectWriter which fixes a lot of the issues we see from the changes made to MCAssembler. This also fixes the issue of the MCMachoStreamer only having calls to the getWriter() function that returns a MachOCASWriter.

It also reverts all the changes made to MCObjectWriter for rdar://133264719 and rdar://133001453

(It is a cherry-pick of PR: #9086)

A significant amount of changes were made upstream to MCAssembler and
MachObjectWriter that broke MCCAS. To fix them, we moved a bunch of code
from MachObjectWriter to MCObjectWriter as a band-aid fix, but those are
full of layering violations, this patch reverts those changes in favor
of a correct fix in the next commit which involves rewriting
MachOCASWriter to be a derived class if MachObjectWriter.

(cherry picked from commit 91e9561)
@rastogishubham
Copy link
Author

@swift-ci please test

@rastogishubham
Copy link
Author

@swift-ci please test llvm

@rastogishubham rastogishubham force-pushed the MachOCASWriterRewriteStable branch from cc01163 to 49eed84 Compare August 14, 2024 21:30
@rastogishubham
Copy link
Author

@swift-ci please test

@rastogishubham
Copy link
Author

@swift-ci please test llvm

This patch rewrites MachOCASWriter to be a derived class of
MachObjectWriter which fixes a lot of the issues we see from the changes
made to MCAssembler. This also fixes the issue of the MCMachoStreamer
only having calls to the getWriter() function that returns a
MachOCASWriter.

(cherry picked from commit 895cd5a)
@rastogishubham rastogishubham force-pushed the MachOCASWriterRewriteStable branch from 49eed84 to 61aec53 Compare August 14, 2024 21:48
@rastogishubham
Copy link
Author

@swift-ci please test

@rastogishubham
Copy link
Author

@swift-ci please test llvm

@rastogishubham
Copy link
Author

@swift-ci please test

@rastogishubham
Copy link
Author

@swift-ci please test llvm

@compnerd
Copy link
Member

The Linux build failed the following test:

Failed Tests (1):
  Clang :: Modules/pch-in-module-units.cppm

@compnerd
Copy link
Member

@swift-ci please test Windows platform

@rastogishubham
Copy link
Author

@compnerd the linux platform test break is not because of this patch, look at
https://ci.swift.org/job/pr-apple-llvm-project-llvm-linux/346/

********************
Failed Tests (1):
  Clang :: Modules/pch-in-module-units.cppm

@compnerd
Copy link
Member

compnerd commented Aug 15, 2024

@rastogishubham yeah, I agree. The Windows build hung but got further than without the change. I think that this is fine to merge personally.

@compnerd
Copy link
Member

The windows build is failing on swift-driver, far after the LLVM build, I think that this is ready to merge.

@rastogishubham rastogishubham merged commit 8f9beda into swiftlang:stable/20240723 Aug 16, 2024
0 of 5 checks passed
@rastogishubham rastogishubham deleted the MachOCASWriterRewriteStable branch August 16, 2024 01:11
@rastogishubham
Copy link
Author

@compnerd thanks for the feedback!

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.

2 participants