Skip to content

Conversation

@davidungar
Copy link
Contributor

A small change to keep printPath from crashes when nodes are integrated from swiftmodules, when we start doing cross-module dependencies.

@davidungar
Copy link
Contributor Author

@swift-ci please test

// swiftmodules won't be in the map
graph.inputDependencySourceMap.contains(key: $0)
? "\(node.key) in \(graph.inputDependencySourceMap[$0].file.basename)"
: "\(node.key)"
Copy link
Contributor

Choose a reason for hiding this comment

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

if let won't be one line but it will prevent need to query the map twice

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting... I always thought that ?: used short-circuit evaluation. I'll have to try that out. Thanks for taking a look.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, according to a hasty experiment, I think that ?: does short-circuit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But, now I see what you mean. Sorry--there's an odd aspect to this map; unlike normal maps, it crashes if subscripted with an absent key. We plan to fix that in a later PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At least this isn't in code that normally runs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And thanks!

@davidungar davidungar merged commit b9f8032 into swiftlang:main Feb 13, 2021
@davidungar davidungar deleted the print-path-fix branch February 16, 2021 18:10
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