Improve and fix static types and their ID and string functions #2756
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…s/accesses, and intersection type
…nd reference type, so it matches sema
dsainati1
approved these changes
Sep 1, 2023
Base automatically changed from
bastian/account-type
to
feature/stable-cadence
September 1, 2023 17:28
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## feature/stable-cadence #2756 +/- ##
==========================================================
+ Coverage 79.60% 79.63% +0.02%
==========================================================
Files 337 337
Lines 80575 80462 -113
==========================================================
- Hits 64145 64077 -68
+ Misses 14125 14086 -39
+ Partials 2305 2299 -6
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
Cadence Benchstat comparisonThis branch with compared with the base branch onflow:feature/stable-cadence commit 978dc9d Collapsed results for better readability
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
SupunS
approved these changes
Sep 1, 2023
turbolent
added
the
Storage Breaking Change
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Depends on #2648
Description
So far, both
sema.Typeandcadence.TypehadIDfunctions, which returned the type ID. With #2750, alsointerpreter.StaticTypegot anIDfunction.Most of these functions re-implemented the ID generation, which is hard to maintain and error prone (inconsistencies). In some cases the ID generation was mixed with the string (human-readable) representation, and the string formatting was re-implemented as both metered and unmetered variants.
Reduce the potential for inconsistencies by implementing the ID generation in a central place, as
sema.Format*TypeIDfunctions, and use them in theIDfunctions of all three Go types representing types.#2754 had previously already started to sort entitlement sets.
We also need to sort type IDs in intersection type IDs.
Avoid code duplication in string formatting functions, by delegating unmetered functions (
String) to metered ones (MeteredString). Also fix memory metering along the way (in some cases memory was allocated before it was metered).Ensure that the type ID generation for entitlement sets in entitlement set authorizations and for intersection types sorts the type IDs. Keep the user-defined order in string formatting functions (e.g.
StringandQualifiedString). Avoid allocating multiple ordered maps when generating the sorted type IDs.Add tests for the ID and string functions of authorizations, reference types, and intersection types, on all three levels (
sema,interpreter, andcadence).Also:
interpreter.StaticTypes to be pointer types, as they might get nested and could potentially lead to lots of data being copied. This wasn't really necessary, but noticed a bit late, but maybe still a good improvement.assertwhere it makes sense, fix argument order (expected, actual), avoid repeated casts, etc.masterbranchFiles changedin the Github PR explorer