Resolve a Layering Violation in libBasic #61014
Conversation
include/swift/Parse/ParseVersion.h
Outdated
| @@ -0,0 +1,45 @@ | |||
| //===--- VersionParser.h - Parser Swift Version Numbers ---------*- C++ -*-===// | |||
There was a problem hiding this comment.
This doesn't match the name of the file. VersionParser.h seems more in line with the rest of the tree.
| Version getCurrentCompilerVersion(); | ||
| } // namespace version | ||
|
|
||
| class VersionParser final { |
There was a problem hiding this comment.
Is there a reason to make this a class and add static methods rather than a namespace and free functions for parseCompilerVersionString and parseVersionString?
There was a problem hiding this comment.
I need to friend Version so I can poke at the component vector.
There was a problem hiding this comment.
Ah, I see. That probably deserves a comment.
|
@swift-ci please smoke test |
CodaFi
changed the title
CodaFi
force-pushed
the
version-militude
branch
2 times, most recently
from
376fef9 to
3c9645f
Compare
|
@swift-ci smoke test |
There was a problem hiding this comment.
friends are gross, but meh, I'll allow it.
It looks like you should be able to parse out the numbers and then use the Version constructor to emit the version once you've computed everything instead of needing to manipulate the components list directly. Then we can drop the friend.
I merged the other patch to get clean builds.
If you want to, feel free to remove the circular link edge. If not, I can do it later.
https://github.com/apple/swift/blob/4b7db51477d08f0c20d57d11a9e7b2b2f7ae281f/lib/Basic/CMakeLists.txt#L101-L103
Basic should not be allowed to link Parse, yet it was doing so to allow Version to provide a constructor that would conveniently parse a StringRef. This entrypoint also emitted diagnostics, so it pulled in libAST. Sink the version parser entrypoint down into Parse where it belongs and point all the clients to the right place.
This reverts commit 9010813.
CodaFi
force-pushed
the
version-militude
branch
from
3c9645f to
a71b970
Compare
|
@swift-ci smoke test |
This is set on a per-file basis and was not forwarded when swiftlang#61014 was integrated. This is a short-term fix for this issue. I don't see a reason this version number should be applied to individual files instead of being passed as an argument to every translation unit. rdar://102731783
Basic should not be allowed to link Parse, yet it was doing so to allow Version to provide a constructor that would conveniently parse a StringRef. This entrypoint also emitted diagnostics, so it pulled in libAST.
Sink the version parser entrypoint down into Parse where it belongs and point all the clients to the right place.