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

Support nested lists in property values #284

Merged
merged 5 commits into from
Feb 5, 2021

Conversation

nvcyc
Copy link
Contributor

@nvcyc nvcyc commented Feb 2, 2021

This pull request enables the support for the use of nested lists in property values.
As suggested in Issue #282, the implementation is based on the idea to change Property_List_Value from List_Id to Node_Id.

A few things to note and check when reviewing the pull request are:

  1. To minimize the impact on the existing code base, I keep Multi_Value and Expanded_Multi_Value as List_Id so that the existing handling for the property values with just a single-layer list is not impacted. The second and above layer nested lists, if existed, will be stored as K_Property_List_Value nodes in their previous layer's list nodes as intended.

  2. The support for nested lists is also implemented in the aadl backend which benefits the test (/tests/github/issue_282/test.aadl) for this pull request.

  3. The output for the test tests/real-annexes-execution/test_real_exec_02.aadl is revised based on my understanding that the correction is necessary and the difference is due to the change for the structure of Property_List_Value. It would be appreciated if you can verify this test output in particular.

@codecov
Copy link

codecov bot commented Feb 2, 2021

Codecov Report

Merging #284 (a768dfa) into master (5582e59) will not change coverage.
The diff coverage is 91.78%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #284   +/-   ##
=======================================
  Coverage   70.53%   70.53%           
=======================================
  Files         369      369           
  Lines       93428    93428           
=======================================
  Hits        65902    65902           
  Misses      27526    27526           
Impacted Files Coverage Δ
src/core/model/ocarina-builder-aadl-properties.adb 93.59% <ø> (ø)
src/core/instance/ocarina-instances-properties.adb 75.13% <37.50%> (ø)
...nstance/ocarina-instances-processor-properties.adb 66.47% <90.00%> (ø)
...rc/backends/aadl_pp/ocarina-be_aadl-properties.adb 95.60% <100.00%> (ø)
src/core/model/ocarina-analyzer-aadl-semantics.adb 89.34% <100.00%> (ø)
src/core/model/ocarina-processor-properties.adb 89.04% <100.00%> (ø)
.../ocarina-me_aadl-aadl_tree-entities-properties.adb 58.89% <100.00%> (ø)
...ontends/aadl/ocarina-fe_aadl-parser-properties.adb 79.96% <100.00%> (ø)
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5582e59...402cf43. Read the comment docs.

@yoogx yoogx merged commit 0091900 into OpenAADL:master Feb 5, 2021
@yoogx
Copy link
Contributor

yoogx commented Feb 5, 2021

Thanks for the patch.

@yoogx
Copy link
Contributor

yoogx commented Feb 5, 2021

Can you test again? For some reasons the first automated build was successful, but now there is a failure that I cannot explain. Thanks

@nvcyc
Copy link
Contributor Author

nvcyc commented Feb 5, 2021

I can reproduce the failure when using the build script with --scenario=github:

git clone https://github.com/OpenAADL/ocarina-build.git
cd ocarina-build
./build_ocarina.sh --scenario=github 

However, if I do as what I did previously then the tests can pass:

git clone https://github.com/OpenAADL/ocarina-build.git
cd ocarina-build
git clone https://github.com/OpenAADL/ocarina.git
./build_ocarina.sh --configure --build --force -t

I'm testing with a few more configurations to see where the problem might be.

@yoogx
Copy link
Contributor

yoogx commented Feb 6, 2021

Thanks. As a general rule, please compile with --enable-debug
This adds more checks including style checks

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.

2 participants