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

Added support for atomic vectors #1245

Conversation

Slartibartfass2
Copy link
Collaborator

@Slartibartfass2 Slartibartfass2 commented Jan 17, 2025

Closes #1142
Closes #1144

What changed:

  • indices now also have an index, i.e., the position in the container + the name if available
  • indices are defined for atomic vectors: c(1, 2, 3, 4)
  • unnamed list arguments are now supported (earlier only named arguments)
  • list and vector definitions are processed on the way up of the fold, which makes resolving non-primitive arguments and flattened vectors less complex
  • access using brackets are now supported: [[1]] and [1]
  • container assignment is now supported container1 <- container2 (indices are copied)
  • added plenty of tests (including missing tests for previous list support) (currently 625 a1f9d365)
    • tests whether indices are defined at all: using assertContainerIndicesDefinition assertion function
    • adding tests for newly added features
    • instead of copying the test cases for each container and access type, we now use parameterization for both dataflow and slicing tests
      • this enables defining one test scenario which maps to 21 test cases (7 container cases and 3 engine cases)
    • the 7 container cases are as follows:
      • 2x: single and double bracket access
        • vector
        • list with named arguments
        • list with unnamed arguments
      • list with dollar access and named arguments

@Slartibartfass2 Slartibartfass2 self-assigned this Jan 17, 2025
@Slartibartfass2 Slartibartfass2 force-pushed the 1142-add-support-for-atomic-vectors-field-sensitive-pointer-analysis branch 3 times, most recently from e0f55e2 to 199b3f0 Compare January 21, 2025 15:29
@Slartibartfass2 Slartibartfass2 force-pushed the 1142-add-support-for-atomic-vectors-field-sensitive-pointer-analysis branch 3 times, most recently from bbe8c41 to b4f8b0a Compare January 25, 2025 19:35
@Slartibartfass2 Slartibartfass2 force-pushed the 1142-add-support-for-atomic-vectors-field-sensitive-pointer-analysis branch 2 times, most recently from 8e7099c to 92932de Compare February 11, 2025 10:16
@Slartibartfass2 Slartibartfass2 force-pushed the 1142-add-support-for-atomic-vectors-field-sensitive-pointer-analysis branch from 7701ce9 to 6fc92b6 Compare February 24, 2025 11:41
@Slartibartfass2 Slartibartfass2 force-pushed the 1142-add-support-for-atomic-vectors-field-sensitive-pointer-analysis branch from a1f9d36 to e18c129 Compare March 3, 2025 14:02
This reduces the indentation of the whole pointer analysis block
and the argument filtering before can be excluded too.
While lexemes are great to destinguish indices from named arguments in lists, for vectors we need to also store the index.
Furthermore, this is important for index based access on lists or vectors.
Index information for named arguments will be added separately.
These will be required later for the index based access on lists.
For an expression `c(1, 2, c(3, 4))` the nested vector would be flattened so that it equals `c(1, 2, 3, 4)`.
For this to work, we need to fetch the indices for the nested vector and add them to the existing indices.
In a case where there are values after a nested index we need to rewrite the indices so that they are in correct order.
E.g. for `c(1, c(2, 3), 4)` the argument with value '4' has initially the index 3, after it has been flattened,
it has to be 4. This is done by merging both lists with new indices similar to the merging part of MergeSort.
With this test suite we can test whether the indices of a vector are defined correctly.
Previously we only could check this by accessing the indices and testing the reads edges.
This eliminates this step and lets us test this unit separately.
Previously only numbers were recognized as values for a vector definition.
Now we can use the same method for the index based access, which works in the same way.
- removed 'dataflow' prefix, because the tests are in the `test/functionality/dataflow` directory, which makes the prefix obsolete
- added the type of access to the test files: list-access -> list-name-based-access to differentiate other types of access
On the way down, we don't have all information e.g. about nested containers, which makes it inevitable to resolve them on the way up.
Previously, for a vector definition nested containers were handled on the way up, while primitves were handled on the way down.
This lead to a rather complex merging logic. Handling now everything on the way up let's us iterate the arguments once and define the
indices for the container at the same time. While adding the resolving of unnamed list arguments, I changed this too for list definitions.
These functions are now used for all container logic.
This enabled the pointer analysis for assignments like the following:
```r
a <- c(1, 2)
a[[1]] <- 3
a[2] <- 4
```

This does not include access with a variable or a range or anything that accessed more than one index.
This way, we can test the same behavior for vectors and lists (with named and unnamed arguments).
This enables it e.g. to query the second access operation in the same line, which was previously
only possible using the line:column format.
This enables passing the indices of one container to another.
Example:
```r
a <- c(1, 2)
b <- a # indices [1, 2] are passed to b
print(b) # definition of a is also in slice
```
Now all edge types can be created using queries.
This way, we can extend the test suite and test more scenarios in less code/time.
@Slartibartfass2 Slartibartfass2 force-pushed the 1142-add-support-for-atomic-vectors-field-sensitive-pointer-analysis branch from fef4e56 to 938b340 Compare March 6, 2025 15:41
@Slartibartfass2 Slartibartfass2 marked this pull request as ready for review March 10, 2025 10:26
@EagleoutIce EagleoutIce merged commit 4fcfee3 into main Mar 10, 2025
17 checks passed
@EagleoutIce EagleoutIce deleted the 1142-add-support-for-atomic-vectors-field-sensitive-pointer-analysis branch March 10, 2025 17:01
@EagleoutIce
Copy link
Member

This pull request is included in v2.2.12 (see Release v2.2.12 (Vector Support, Improved Graphic Support, Eval of Strings)).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants