Skip to content

Conversation

Nonchalant
Copy link
Contributor

Xcode (and possibly other IDEs) provide a UI over -D flags where you provide the flag name and -D is prepended for you. Users might reasonably add a redundant -D themselves, leading to the invalid flag -D-DFOO getting passed into the compiler. The driver could detect this and raise a specific error message alerting the user to their mistake.

Resolves SR-7040.

@Nonchalant Nonchalant force-pushed the sr_7040_redundant_D branch from 20d650d to 3fca891 Compare March 3, 2018 04:02
Copy link
Contributor

@jrose-apple jrose-apple left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking this on! I have a suggestion for the phrasing of the message, but the implementation looks good and I'm glad it worked so cleanly.

@@ -209,6 +209,10 @@ ERROR(symbol_in_ir_not_in_tbd,none,
"symbol '%0' (%1) is in generated IR file, but not in TBD file",
(StringRef, StringRef))

ERROR(redundant_prefix_compilation_flag,none,
"did you provide a redundant '-D' in your build settings? (got '-D%0')",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We usually try to say what went wrong first and then offer a suggestion, so maybe something more like "invalid argument '-D%0'; did you…"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! I will try to fix message!! ✨

@jrose-apple
Copy link
Contributor

Looks great!

@swift-ci Please smoke test

@jrose-apple jrose-apple self-assigned this Mar 5, 2018
@rintaro
Copy link
Member

rintaro commented Mar 6, 2018

Disabled failed test in #15005

@rintaro
Copy link
Member

rintaro commented Mar 6, 2018

@swift-ci Please smoke test

@jrose-apple jrose-apple merged commit be4c2b9 into swiftlang:master Mar 6, 2018
@Nonchalant Nonchalant deleted the sr_7040_redundant_D branch May 17, 2019 08:45
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