We're currently implementing typed throws, and as a part of it, we want to serialize over a Type across the type-check system. As the parameter has to be not nil we're resolving to the throwsType to getNeverType(). But we're having many issues with that, because that is always null so the serialization crashes.
Is this null intended? I think it shouldn't, as it is a type in the stdlib, an enum, that conforms to Error, so we can safely check our fallback type in case the function doesn't throw.
I don't fully understand the issue. Are you saying that serialization is receiving a null throwsType when it shouldn't? Or are you saying that you serialize a non-null type but during deserialization you see null?
If it's the former, then something somewhere in-between is dropping the type being thrown. Function types are created in a bunch of places in the compiler. If you added a breakpoint in one of the factory methods (which centralize the allocation logic) like FunctionType::get, you could inspect the function types that are being created and look at the call-stack.
If it's the latter, maybe you didn't update ModuleFormat.h properly, it has a using FunctionTypeLayout = BCRecordLayout<. Right now, it just has BCFixed<1>, // throws?, that would need to change to store a type.
Yeah, I updated the ModuleFormat.h correctly adding a TypeID to represent the non-null throwsType. But the point is that when the function doesn't throw (which is represented by a boolean) the ThrowsType which is a Type, can't be null as far as I have seen. So we're using the Never type to fill the gap when a function doesn't throw.
The point of all above is that we use the function getNeverType() in many places (there're many ways to create a method that can throw) so when serializing we use that function a lot and, in many of the places that method returns null which is silently assigned into ThrowsType and then, the crash.
Are you using TypeIDField to store the type? Also, are you serializing/deserializing a Type or a TypeRepr? (your implementation seems to be using a TypeRepr)?
add a ThrowsDecl : Decl class to Decl.h, with variables (and getters/setters etc):
SourceLoc throwsLoc
Optional<Type> throwsType.
Optional<SourceLoc> throwsTypeLocation
Add the createX factory methods for serialization and stuff to ThrowsDecl.
Add an optional ThrowsDecl to AbstractFunctionDecl with serialization support.
Then we'd have a serializable component inside AbstractFunctionDecl that we could type-check without having to push the Never type around. The type-checking rule is that the ThrowsType has to conform to Error which might be a little problematic when the standard library is not available, so we would have to work through that.
Does that make sense or am I missing something in the numbered list above?
I see one issue with ThrowsDecl being different for CanFunctions (using CanType) and regular functions (using Type). Also updating the Module to reflect those changes might be hard to do, i'm not quite sure how to do it or if the idea is correct.
Depending on the ASTContext, the stdlib module is loaded or not, but also you can get into the point that calling loadStdlibModule is no possible, so we cannot lookup for types from the stdlib. And hence don't get the Never type or any of the known stdlib types.
Cc @suyashsrijan have you any idea why we can't load that module from an ASTContext that hasn't load the types yet?
If the standard library module isn’t loaded, perhaps it could be an error to use typed throws? Because if it isn’t loaded, you can’t use lots of other things, like Error.
(if you look at the codebase, we have diagnostics and assertions for broken modules/conformances or missing types)
IMO it should be an error. I don’t think we have precedent for synthesising types and declarations from standard library if it cannot be loaded. It’s just an error if it isn’t available.
Many tests (almost all of them) doesn't seem to load the Swift stdlib, I don't know if that's because we're using a stdlib compiled with a different module kind (but not increased the number to generate the errors) and that step is necessary when the serialization is changed, to increase the number and try to compile the whole stdlib or it is supposed to work even if you haven't compiled the stdlib with the new serialization?
I think you should increment SWIFTMODULE_VERSION_MINOR. Any changes to the serialization format should be accompanied with a corresponding bump of the version number.
Is the failure to load the Never type happening during type checking or during serialization/deserialization?
That is deliberate in order to reduce coupling between the stdlib and the test cases (and more generally, reduce the dependencies of test cases on things outside the test case itself).
In IRGen, SILGen and SIL tests, avoid using platform-dependent implementation details of the standard library (unless doing so is point of the test).
Unless testing the standard library, avoid using arbitrary standard library types and APIs, even if it is very convenient for you to do so in your tests. Using the more common APIs like Array subscript or + on IntXX is acceptable. This is important because you can't rely on the full standard library being available. The long-term plan is to introduce a mock, minimal standard library that only has a very basic set of APIs.
During typechecking. Most of the ASTContext instances don't have the stdlib loaded, but this may have to be with what @typesanitizer said that some tests are meant to be stdlib independent. Very inconvinient if we're using Never and Error as both paths of throwing methods...