SR-6717 Builtin types are always be printed with Builtin prefix #27732
Conversation
lib/AST/ASTPrinter.cpp
Outdated
| @@ -3529,6 +3526,9 @@ class TypePrinter : public TypeVisitor<TypePrinter> { | |||
|
|
|||
| ModuleDecl *M = D->getDeclContext()->getParentModule(); | |||
|
|
|||
| if (!Options.FullyQualifiedTypesIfAmbiguous && !M->isBuiltinModule()) | |||
There was a problem hiding this comment.
I think this is equivalent today, but we could go even further by always returning true if M->isBuiltinModule() and then testing Options.FullyQualifiedTypesIfAmbiguous after that.
There was a problem hiding this comment.
No problem, I've separated the two checks
test/type/builtin_types.swift
Outdated
| // RUN: %target-typecheck-verify-swift | ||
|
|
||
| let x = 0 | ||
| _ = x._value == x // expected-error {{referencing operator function '==' on 'BinaryInteger' requires that 'Builtin.Int64' conform to 'BinaryInteger'}} |
There was a problem hiding this comment.
So, the comment about tests in the Jira was about not relying on the implementation details of Int (the _value property). Instead, you can make some other type in this file that uses Builtin.Int64 (or whatever) as a stored property by adding -parse-stdlib to the test invocation. (You'll then need to import Swift to still get access to the normal standard library.)
There was a problem hiding this comment.
Something like this one that I've pushed now?
|
@swift-ci Please test |
|
Build failed |
Ah, that makes sense: a builtin type doesn't have any of the initialization logic that a normal type does. You can sidestep this problem by using a function that takes a Builtin.Int64 instead. |
|
Build failed |
|
Ok I’m lost, I don’t know how to use all the builtin stuff and I can’t figure how to write the test :( |
|
There's no real documentation on it, so that's not surprising! I was thinking of something like this: import Swift
func test(_ builtin: Builtin.Int64) {
let x = 0
_ = x == builtin // expected-error {{operator function '==' requires that 'Builtin.Int64' conform to 'BinaryInteger'}}
} |
|
Oh yes, I was rethinking on it during work, and it struck me that I was only suppose to trigger an error not generate a valid Builtin value to use in the current check. Sorry for the trouble |
JGiola
force-pushed
the
SR-6717
branch
2 times, most recently
from
84f7d7e to
396254a
Compare
|
Ok I’ve updated the test |
|
@swift-ci Please test |
|
Build failed |
|
Build failed |
|
Looks like it worked! …but other tests need to be updated for this change as well. Mind getting those too? |
|
I’m on it |
|
Ah…what about the other failures? https://ci.swift.org/job/swift-PR-Linux/16520/consoleFull You should be able to run the tests locally as well, either by using build-script or one of the lit.py invocations in docs/Testing.md. |
|
Ops, my bad. I haven’t seen the SILOptimizer one in the log. I’ve fixed them, and I will running the full suite tonight for catching other failures if there are any. |
ASTPrinter now ignore the Options.FullyQualifiedTypesIfAmbiguous value if the containing module is the builtin one.
|
Ok now, at least on a mac, all tests pass! |
|
@swift-ci please test |
|
Build failed |
|
Build failed |
|
Thanks, @JGiola! |
ASTPrinter now ignore the Options.FullyQualifiedTypesIfAmbiguous value if the containing module is the builtin one.
As stated in the bug report, I've implemented the test in the description to validate the fix but if someone has a more self-contained example I will swap it in no time.
Resolves SR-6717.