OpenAADL / ocarina

AADL model processor: mappings to code (C, Ada); Petri Nets; scheduling tools (MAST, Cheddar); WCET; REAL
http://www.openaadl.org
Other
64 stars 29 forks source link

Support of nested lists in properties #282

Closed nvcyc closed 3 years ago

nvcyc commented 3 years ago

OCARINA VERSION:

root@c226fb0da43f:~/opt/ocarina-build# ocarina --version
Ocarina v2017.1-490-ga5cc118 (Working Copy from ra5cc1182)
Copyright (c) 2003-2009 Telecom ParisTech, 2010-2021 ESA & ISAE
Build date: Jan 14 2021 20:54:26

HOST MACHINE and OPERATING SYSTEM:

root@c226fb0da43f:~/opt/ocarina-build# uname -a
Linux c226fb0da43f 5.4.0-56-generic #62~18.04.1-Ubuntu SMP Tue Nov 24 10:07:50 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux

COMPILER VERSION

root@c226fb0da43f:~/opt/ocarina-build# gnatls -v

GNATLS 7.5.0
Copyright (C) 1997-2017, Free Software Foundation, Inc.

Source Search Path:
   <Current_Directory>
   /usr/lib/gcc/x86_64-linux-gnu/7/adainclude

Object Search Path:
   <Current_Directory>
   /usr/lib/gcc/x86_64-linux-gnu/7/adalib

Project Search Path:
   <Current_Directory>
   /usr/x86_64-linux-gnu/lib/gnat
   /usr/x86_64-linux-gnu/share/gpr
   /usr/share/gpr
   /usr/lib/gnat
root@c226fb0da43f:~/opt/ocarina-build# gcc --version
gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0
Copyright (C) 2017 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

DESCRIPTION:

I'm aware of the issue #22 being fixed long time ago, but this issue is related to more general property types. Specifically, in the example presented in next section, an assignment in the implementation for the property list of list of aadlinteger cannot be parsed correctly by Ocarina. The same issue goes to 2 or more layers of nested lists.

REPEAT BY:

A simple two-file test can reproduce this issue:

The first file named Test_List_of_List_Properties.aadl:

property set Test_List_of_List_Properties is
    a_list_of_list_property: list of list of aadlinteger applies to (system);
end Test_List_of_List_Properties;

and the second file named Test_List_of_List.aadl:

package Test_List_of_List
public
    with Test_List_of_List_Properties;

    system main
    end main;

    system implementation main.impl
        properties
            Test_List_of_List_Properties::a_list_of_list_property => ((1,2), (3,4));
    end main.impl;

end Test_List_of_List;

Then, with the following command: ocarina -aadlv2 -r Test_List_of_List::main.impl Test_List_of_List_Properties.aadl Test_List_of_List.aadl we'll get the following message with a failed compilation:

Test_List_of_List.aadl:10:72: parsing Boolean_Or_Record_Term, unexpected integer value [1]
Test_List_of_List.aadl:11:05: parsing Property_Association or Contained_Property_Association, token ';' is expected, found keyword 'end'
Test_List_of_List.aadl:9:09: parsing Properties, list is empty
Test_List_of_List.aadl:11:18: parsing Package_Specification, Empty packages are not allowed
Test_List_of_List.aadl:13:01: parsing AADL_Declaration, unexpected keyword 'end'
Cannot parse AADL specifications

SAMPLE FIX/WORKAROUND:

By looking at the source code, it seems that it was planned to support nested lists in general with the introduced variable Multiplicity. Yet, it appears to be completed and touches in every part of the compiler code (e.g., frontend, parser, model, analyzer) might be required.

yoogx commented 3 years ago

Indeed, and this is very low priority given the amount of work. Why do you need this?

nvcyc commented 3 years ago

In my own use case, I declared a property to store a list of supported instruction/command sequences which are represented by lists of integers.

yoogx commented 3 years ago

I understand, but this would not be processed by Ocarina, unless you plan writing your own extension. I can guide you in implementing this feature, but I cannot commit on it. I will give it a try next week

nvcyc commented 3 years ago

Thanks for your hint and I understand that the custom property sets have to be handled outside of the Ocarina core.

It will be great to have this feature implemented, either by you or me with your guidance. Either way, I've started with fixing the property definition in the pull request #283 if this helps. Some feedback for how this feature should be implemented or your plan will be greatly appreciated!

nvcyc commented 3 years ago

Thanks for reviewing the pull request #283. Before proceeding with further implementation, I'd like to get some feedback from you.

As the first step towards supporting nested lists in parsing a property value, let me start with my code tracking for where the fix should start so that we are in the same page.

A property value is parsed in the function P_Property_Value https://github.com/OpenAADL/ocarina/blob/5582e59fea10a212732c89c3a4e95a1879cd8bd4/src/frontends/aadl/ocarina-fe_aadl-parser-properties.adb#L72

that invokes the function P_Items_List in the case that a list value is found:

https://github.com/OpenAADL/ocarina/blob/5582e59fea10a212732c89c3a4e95a1879cd8bd4/src/frontends/aadl/ocarina-fe_aadl-parser-properties.adb#L106-L113

The corresponding P_Items_List function is implemented in

https://github.com/OpenAADL/ocarina/blob/5582e59fea10a212732c89c3a4e95a1879cd8bd4/src/frontends/aadl/ocarina-fe_aadl-parser.adb#L347-L353

which in effect maintains a loop and invokes the function P_Property_Expression to parse each of the items in the list.

From the implementation of the function P_Property_Expression https://github.com/OpenAADL/ocarina/blob/5582e59fea10a212732c89c3a4e95a1879cd8bd4/src/frontends/aadl/ocarina-fe_aadl-parser-properties-values.adb#L2236

we can see that it does not handle the case when an expression is a nested list.

Now going back to our goal, it seems that a reasonable place for handling the nested lists in the first place is in the function P_Property_Value. Ideally, we can recursively invoke P_Property_Value until we hit the list in the last layer that will be parsed by P_Items_List as usual. In the end, in the case of nested lists, we will have a node holding a set of list nodes (that hold nested list nodes if exist) as its items (i.e., nested K_Property_List_Value).

However, the current interface design may not work (for example, we can't append a K_Property_List_Value node to a list) and thus something fundamental needs to be revised as well. Do you think this is the right direction to go, or do you have a more favorable way to do so (such as assigning something other than K_Property_List_Value for the nested lists)?

And of course the rest of the compiler will have to be revised accordingly based on how we are going to implement it.

yoogx commented 3 years ago

I think you are going in th right direction

Note that in the lexer packages, you have a function P_ for each BNF rule. One is missing for

property_list_value ::=
( [ ( property_list_value | property_expression )
{ , ( property_list_value | property_expression ) }* ] )

and it should probably replace the code around https://github.com/OpenAADL/ocarina/blob/5582e59fea10a212732c89c3a4e95a1879cd8bd4/src/frontends/aadl/ocarina-fe_aadl-parser-properties.adb#L106-L113

For the moment, it seems this file may not faithfully follow this portion of the BNF.

I am not sure I understand your point in "we can't append a K_Property_List_Value node to a list) and thus something fundamental needs to be revised as well.", can you elaborate?

If you mean this

  interface Property_List_Value : List_Id { };
  //  interface Property_List_Value : Node_Id {
  //  List_Id Property_Values;
  //};

Then yes, the commented code should probably be reintroduced. Turning Property_List_Value into a Node_Id is required to build lists of lists

Note this comes from a first attempt at this 8 years (!) ago. See https://github.com/OpenAADL/ocarina/commit/0158a248c6b61fa56bdad274a8bfdd02df7551ab

yoogx commented 3 years ago

This issue is considered solved.