Improve Swift support #13120
Open
Improve Swift support #13120
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I.e. types that are not FileOrString.
Essential when a module.modulemap is generated, or any generated header files referenced by a modulemap.
In order to translate “-Wl” into “-Xlinker”.
To avoid warnings about duplicate “-l” flags.
Newer toolchains support it.
dcbaker
requested changes
Apr 22, 2024
| for arg in dep.get_link_args(): | ||
| if arg.startswith("-Wl,"): | ||
| for flag in arg[4:].split(","): | ||
| result += ["-Xlinker", flag] |
There was a problem hiding this comment.
there's self.LINKER_PREFIX that should be used here instead of open coding.
| @@ -49,6 +50,16 @@ def get_werror_args(self) -> T.List[str]: | |||
| def get_dependency_gen_args(self, outtarget: str, outfile: str) -> T.List[str]: | |||
| return ['-emit-dependencies'] | |||
|
|
|||
| def get_dependency_link_args(self, dep: 'Dependency') -> T.List[str]: | |||
| result = [] | |||
Comment on lines
+57
to
+58
| for flag in arg[4:].split(","): | ||
| result += ["-Xlinker", flag] |
There was a problem hiding this comment.
This isn't right. But calling .split() here flag becomes a List[str], that means that result ends up with ['-Xlinker', ['-linkarg']], which is not a string.
| seen_libs.add(arg) | ||
| else: | ||
| deduped_args.append(arg) | ||
| return deduped_args |
There was a problem hiding this comment.
This seems to be the sort of thing that CompileArgs exists for, is it possible to use that for this purose?
Comment on lines
+1224
to
+1225
| outhandle, outfile = tempfile.mkstemp('.out') | ||
| os.close(outhandle) |
There was a problem hiding this comment.
Is it not possible to use NamedTemporaryFile here, to avoid the manual cleanup?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
(I have two additional fixes that I will submit later, as they depend on APIs added in #12994.)