Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More nullability changes #178

Merged
merged 1 commit into from
Apr 26, 2020
Merged

Conversation

smaillet
Copy link
Member

  • Adjusted general philosophy for handling nulls, there are many cases where properties of an object are pulled from an LLVM IR operand. That technically is nullable, however in the context of that type and property it should never be null. In such cases the '!' operator is applied. Thus any such, totally unexpected cases can still trigger a fatal null ref exception but the code is simpler and more efficient by not always checking for null when it isn't ever supposed to be null. (The only way it can be is bug in the LLVM internals or a bug in the .NET bindings in this library)
  • Reversed course on Debug metadata types nullability and null object pattern. Fundamentally LLVM itself is designed where DIScope, DIFile and DIType are allowed null in most cases (and in fact null for each has a meaning. [i.e. DIType == null => void]) Fighting against that complicates the code base and makes using the API harder. The biggest problem is in mapping a handle back to a concrete type. Since handles are equivalent to a pointer to the base type it's impossible to determine what the derived type is supposed to be when the pointer is null. There are a lot of places in the code (especially operands) that deal in collections of the base type and there's no way to determine which "null object" is supposed to be used. Thus it is simpler and cleaner to accept the design of the underlying LLVM library and formally declare these as nullable, so that analysis tooling can warn consumers appropriately. There are cases where a null is not appropriate (like creating an array, where void makes no sense as the element type)
  • Significant re-work of the operand handling. The previous approach of using an operand container interface was just plain confusing and hard to follow. (Multiple indexers in play depending on explicit interface vs not etc... made it hard to read and follow - the new approach uses more specific classes that implement a common interface and that supports slicing via the new C#8 ranges so creating slices for properties is much simpler.

* Adjusted general philosophy for handling nulls, there are many cases where properties of an object are pulled from an LLVM IR operand. That technically is nullable, however in the context of that type and property it should never be null. In such cases the '!' operator is applied. Thus any such, totally unexpected cases can still trigger a fatal null ref exception but the code is simpler and more efficient by not always checking for null when it isn't ever supposed to be null. (The only way it can be is  bug in the LLVM internals or a bug in the .NET bindings in this library)
* Reversed course on Debug metadata types nullability and null object pattern. Fundamentally LLVM itself is designed where DIScope, DIFile and DIType are allowed null in most cases (and in fact null for each has a meaning. [i.e. DIType == null => void]) Fighting against that complicates the code base and makes using the API harder. The biggest problem is in mapping a handle back to a concrete type. Since handles are equivalent to a pointer to the base type it's impossible to determine what the derived type is supposed to be when the pointer is null. There are a lot of places in the code (especially operands) that deal in collections of the base type and there's no way to determine which "null object" is supposed to be used. Thus it is simpler and cleaner to accept the design of the underlying LLVM library and formally declare these as nullable, so that analysis tooling can warn consumers appropriately. There are cases where a null is not appropriate (like creating an array, where void makes no sense as the element type)
* Significant re-work of the operand handling. The previous approach of using an operand container interface was just plain confusing and hard to follow. (Multiple indexers in play depending on explicit interface vs not etc... made it hard to read and follow - the new approach uses more specific classes that implement a common interface and that supports slicing via the new C#8 ranges so creating slices for properties is much simpler.
@smaillet smaillet merged commit 4eb1ef2 into UbiquityDotNET:develop Apr 26, 2020
@smaillet smaillet deleted the RelaxNullability branch April 26, 2020 23:25
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.

1 participant