Skip to content

Conversation

@cltnschlosser
Copy link
Contributor

Fixes: Driver/help.swift

Also replaces deprecated createSymlink calls with localFileSystem.createSymbolicLink

@cltnschlosser cltnschlosser requested a review from owenv November 11, 2020 07:58
Copy link
Contributor

@owenv owenv left a comment

Choose a reason for hiding this comment

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

Thanks!

@DougGregor
Copy link
Member

@swift-ci test

Copy link
Contributor

@owenv owenv left a comment

Choose a reason for hiding this comment

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

LGTM!

@cltnschlosser
Copy link
Contributor Author

@swift-ci test

1 similar comment
@owenv
Copy link
Contributor

owenv commented Nov 14, 2020

@swift-ci test

@cltnschlosser cltnschlosser force-pushed the cs_swiftHelpIntegrationTests branch from e694866 to c761312 Compare November 14, 2020 23:04
@cltnschlosser
Copy link
Contributor Author

@owenv -emit-module-doc-path, -index-ignore-stdlib, -emit-module-source-info are (seemingly incorrectly) marked .noDriver. Those are just the ones caught by the unit tests. I'll see if I can do a scan for any other .noDriver options that are used in the driver. Best solution is to update the options in c++ driver?

@cltnschlosser
Copy link
Contributor Author

Okay, found 3 categories of .noDriver options that are used.

Definitely checked for and used:

.emitDependenciesPath
.emitModuleDocPath
.emitModuleDoc
.emitModuleSourceInfo
.enableCxxInterop

Technically supported, sets the compiler output type to pch, but I don't know if it actually works:

.emitPch

Erased when parsedOptions.hasArgument(.indexFile) && parsedOptions.hasArgument(.dumpAst), but never actually passed through or used, so maybe that code can be removed:

.indexIgnoreStdlib
.indexSystemModules

@cltnschlosser
Copy link
Contributor Author

Okay, actually I have to do a bit more digging here. C++ driver claims these are unsupported, so swift-driver probably should too <unknown>:0: error: unknown argument: '-emit-pch'.

This happens for all these parameters. First I'm going to find where this check happens in the c++ driver, then I'm going to look at the unit tests we have that are using these parameters.

@cltnschlosser
Copy link
Contributor Author

Okay, I've updated the driver to emit an unknown argument error for .noDriver args (this matches c++ driver). Also made a few changes where the swift-driver was expecting these arguments. Including unit tests that used them.

Re-requesting reviews since the diff has changed quite a bit.

Also I need someone to kick off CI :)

@DougGregor
Copy link
Member

@swift-ci please test

Copy link
Contributor

@owenv owenv left a comment

Choose a reason for hiding this comment

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

LGTM! I'm surprised this many options were affected, the C++ driver definitely has some dead code that's checking for these...

@cltnschlosser cltnschlosser merged commit 8f65c26 into main Nov 15, 2020
@cltnschlosser cltnschlosser deleted the cs_swiftHelpIntegrationTests branch November 15, 2020 07:20
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.

5 participants