Open
Conversation
This is the first part of a refactoring for Runtime Capabailities Analysis (RCA) internals. This is meant to simplify the internals by removing some elements that are no longer used, simplifying some parts, and preparing for later renaming and feature work. In particular, this includes: - Removal of the `ValueKind` struct that differentiated between elements and arrays. - Use `RuntimeFeatureFlags::UseOfDynamicallySizedArray` more consistently to identify places where arrays are dynamic. - Propagate `RuntimeFeatureFlags` via increased use of `ComputeKind` struct. - Simplify array related `ApplicationInstance` entries to drop distinction between dynamic and static content. - Update RCA tests for changes in structure. Note that for the test updates, the `value_kind` field changes to `runtime_kind` but the actual content of the field (static vs dynamic) and importantly the `runtime_features` do NOT change, showing that the refactor did not affect the behavior of these tests. The one exception is a new test case in arrays that shows an indirect call to the `Length` intrinsic with a dynamically sized array now correctly reflects the output as a dynamic integer, which was a pre-existing subtle propagation bug fixed during refactor.
Collaborator
Author
|
We've never had an issue filed for this refactor, but I remember discussing it before during the initial development of RCA. The closest I could find after some digging through old PR comments in this one from May 2024: #1391 (comment). Given the other updates to RCA I'm interested in making, it was finally time to do this simplification to avoid propagated the pending refactor even further into the future! |
This file contains hidden or 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
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.
This is the first part of a refactoring for Runtime Capabailities Analysis (RCA) internals. This is meant to simplify the internals by removing some elements that are no longer used, simplifying some parts, and preparing for later renaming and feature work. In particular, this includes:
ValueKindstruct that differentiated between elements and arrays, rename the exisitingRuntimeKindtoValueKindRuntimeFeatureFlags::UseOfDynamicallySizedArraymore consistently to identify places where arrays are dynamic.RuntimeFeatureFlagsvia increased use ofComputeKindstruct.ApplicationInstanceentries to drop distinction between dynamic and static content.Note that for the test updates, the actual content of the field (static vs dynamic) and importantly the
runtime_featuresdo NOT change, showing that the refactor did not affect the behavior of these tests. The one exception is a new test case in arrays that shows an indirect call to theLengthintrinsic with a dynamically sized array now correctly reflects the output as a dynamic integer, which was a pre-existing subtle propagation bug fixed during refactor.