Skip to content

Conversation

@ahoppen
Copy link
Member

@ahoppen ahoppen commented May 17, 2024

The existing ad-hoc logic was not quite correct because it didn’t eg. remove -MT/depfile because it assumed that -MT was followed by a space. It also didn’t take into account that serialize-diagnostics can be spelled with a single dash or two dashes.

Create a CompilerCommandLineOption type that forces decisions to be made about the dash spelling and argument styles, which should help avoid problems like this in the future.

@ahoppen ahoppen requested review from bnbarham and hamishknight May 17, 2024 15:46
@ahoppen ahoppen requested a review from benlangmuir as a code owner May 17, 2024 15:46
@ahoppen
Copy link
Member Author

ahoppen commented May 17, 2024

@swift-ci Please test

@ahoppen ahoppen mentioned this pull request May 17, 2024
The existing ad-hoc logic was not quite correct because it didn’t eg. remove `-MT/depfile` because it assumed that `-MT` was followed by a space. It also didn’t take into account that `serialize-diagnostics` can be spelled with a single dash or two dashes.

Create a `CompilerCommandLineOption` type that forces decisions to be made about the dash spelling and argument styles, which should help avoid problems like this in the future.
@ahoppen ahoppen force-pushed the command-line-sanitization branch from cd33fbf to 5bad2c5 Compare May 17, 2024 21:16
@ahoppen
Copy link
Member Author

ahoppen commented May 17, 2024

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented May 17, 2024

@swift-ci Please test Windows

2 similar comments
@ahoppen
Copy link
Member Author

ahoppen commented May 17, 2024

@swift-ci Please test Windows

@ahoppen
Copy link
Member Author

ahoppen commented May 20, 2024

@swift-ci Please test Windows

@ahoppen ahoppen merged commit 0da0527 into swiftlang:main May 20, 2024
@ahoppen ahoppen deleted the command-line-sanitization branch May 20, 2024 20:40
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