Skip to content

Conversation

@CodaFi
Copy link
Contributor

@CodaFi CodaFi commented Nov 11, 2020

This leaves FrontendTool devoid of everything except the high-level entrypoints to functionality in the rest of the compiler stack. Hopefully we can make a later pass and drop a bunch of unused includes...

@CodaFi
Copy link
Contributor Author

CodaFi commented Nov 11, 2020

@swift-ci smoke test

Copy link
Contributor

@davidungar davidungar 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 like the constification. There were one or two places where a const reference could maybe be used.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just a thought: would it work to use a const reference here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would. But if the goal is to prevent reassignment, encapsulation is your best bet.

@CodaFi
Copy link
Contributor Author

CodaFi commented Nov 12, 2020

@swift-ci smoke test Linux

T *const does not prevent logically non-const accesses to the underlying data, it merely indicates that the pointer value itself is const. This modifier can be cast off by a copy, so it's not generally what you want here. Switch to const T * instead.
@CodaFi
Copy link
Contributor Author

CodaFi commented Nov 12, 2020

@swift-ci smoke test

@CodaFi
Copy link
Contributor Author

CodaFi commented Nov 12, 2020

@swift-ci smoke test macOS

@CodaFi
Copy link
Contributor Author

CodaFi commented Nov 12, 2020

@CodaFi CodaFi merged commit 7b3130b into swiftlang:main Nov 12, 2020
@CodaFi CodaFi deleted the frontend-loader branch November 12, 2020 22:48
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.

2 participants