Skip to content

[5.1] Upgrade PCMacro/PlaygroundTransform to support module/file IDs #24811

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 29, 2019
Merged

[5.1] Upgrade PCMacro/PlaygroundTransform to support module/file IDs #24811

merged 1 commit into from
May 29, 2019

Conversation

ChrisBrough
Copy link
Contributor

This change PCMacro and PlaygroundTransform to return an a moduleID and
fileID in addition to the source location information. The Frontend has
been changed to run PCMacro and PlaygroundTransform on all input files
instead of the main file only.

The tests have been updated to conform to these changes with an addition
of module and file ID specific tests. The Playgrounds related tests were
adjusted to make a module out of the stub interface files since those
files should not have PCMacro and PlaygroundTransform applied to them.

rdar://problem/50821146

@ChrisBrough ChrisBrough self-assigned this May 15, 2019
@ChrisBrough
Copy link
Contributor Author

@swift-ci Please test

@ChrisBrough ChrisBrough mentioned this pull request May 15, 2019
Merged
@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - ea587add92431f1c1e786c8e972e500675784d46

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - ea587add92431f1c1e786c8e972e500675784d46

@ChrisBrough
Copy link
Contributor Author

ChrisBrough commented May 16, 2019 •
edited
Loading

  • Update the following tests to compile a separate module for supporting files
Failing Tests (6):
    Swift(macosx-x86_64) :: PlaygroundTransform/implicit_return_var_intliteral.swift
    Swift(macosx-x86_64) :: PlaygroundTransform/implicit_return_subscript_chain.swift
    Swift(macosx-x86_64) :: PlaygroundTransform/implicit_return_init_nil.swift
    Swift(macosx-x86_64) :: PlaygroundTransform/implicit_return_func_stringliteral.swift
    Swift(macosx-x86_64) :: PlaygroundTransform/implicit_return_func_is.swift
    Swift(macosx-x86_64) :: PlaygroundTransform/implicit_return_func_binaryoperation_args.swift

@ChrisBrough ChrisBrough force-pushed the cbrough/50821146-review branch from ea587ad to eaec764 Compare May 17, 2019 17:02
@swiftlang swiftlang deleted a comment from swift-ci May 17, 2019
@swiftlang swiftlang deleted a comment from swift-ci May 17, 2019
@ChrisBrough
Copy link
Contributor Author

@swift-ci Please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - eaec764f71722069591647424ec288f8f67e56ed

@rintaro
Copy link
Member

rintaro commented May 17, 2019

CI issue has been fixed.
@swift-ci Please test OS X platform

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - eaec764f71722069591647424ec288f8f67e56ed

@ChrisBrough
Copy link
Contributor Author

Please test with the following PR:
apple/swift-lldb#1586

@swift-ci Please test OS X platform

jrose-apple
jrose-apple reviewed May 20, 2019
View reviewed changes
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.

Looks reasonable, but this should go to the master branch first, and then get cherry-picked to the release branch.

lib/Sema/InstrumenterSupport.cpp Outdated

// Setup File identifier
StringRef filePath = TypeCheckDC->getParentSourceFile()->getFilename();
std::string fileName = llvm::sys::path::stem(filePath).str();
Copy link
Contributor

Choose a reason for hiding this comment

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

Here's a better example of not needing to copy, I guess.

Copy link
Contributor

Choose a reason for hiding this comment

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

Still doing an extra copy! It's not critical but…

@ChrisBrough ChrisBrough force-pushed the cbrough/50821146-review branch 2 times, most recently from 8944dbc to 1b1fcc6 Compare May 22, 2019 18:21
@ChrisBrough ChrisBrough changed the base branch from swift-5.1-branch to master May 22, 2019 18:21
@ChrisBrough
Copy link
Contributor Author

Please test with the following PR:
apple/swift-lldb#1586

@swift-ci Please test

@ChrisBrough ChrisBrough requested a review from jrose-apple May 22, 2019 22:51
jrose-apple
jrose-apple approved these changes May 24, 2019
View reviewed changes
lib/Sema/InstrumenterSupport.cpp Outdated

// Setup File identifier
StringRef filePath = TypeCheckDC->getParentSourceFile()->getFilename();
std::string fileName = llvm::sys::path::stem(filePath).str();
Copy link
Contributor

Choose a reason for hiding this comment

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

Still doing an extra copy! It's not critical but…

@ChrisBrough
[5.1] Upgrade PCMacro/PlaygroundTransform to support module/file IDs
87e8627 
This change PCMacro and PlaygroundTransform to return an a moduleID and
fileID in addition to the source location information. The Frontend has
been changed to run PCMacro and PlaygroundTransform on all input files
instead of the main file only.

The tests have been updated to conform to these changes with an addition
of module and file ID specific tests. The Playgrounds related tests were
adjusted to make a module out of the stub interface files since those
files should not have PCMacro and PlaygroundTransform applied to them.

rdar://problem/50821146
@ChrisBrough ChrisBrough force-pushed the cbrough/50821146-review branch from 1b1fcc6 to 87e8627 Compare May 28, 2019 21:21
@ChrisBrough
Copy link
Contributor Author

Please test with the following PR:
apple/swift-lldb#1586

@swift-ci Please test

@swiftlang swiftlang deleted a comment from swift-ci May 29, 2019
@swiftlang swiftlang deleted a comment from swift-ci May 29, 2019
@ChrisBrough ChrisBrough merged commit b75f9ee into swiftlang:master May 29, 2019
@ChrisBrough ChrisBrough mentioned this pull request May 29, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jrose-apple jrose-apple jrose-apple approved these changes

@cwakamo cwakamo Awaiting requested review from cwakamo

Assignees

@ChrisBrough ChrisBrough

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ChrisBrough @swift-ci @rintaro @jrose-apple