-
Notifications
You must be signed in to change notification settings - Fork 1
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
Ignoring declarations (updated 8/27) #43
Comments
Since
gives back
(and an error :-( ), I guess we should output the same.
Yes, and also to parse the header for possible variable declarations, i.e. we want
to give the same matrix instead of an error. |
Working on variable declarations, and I am running into a new issue. Specifically I need to understand the role of this https://github.com/seiller/pymwp/blob/0c07edf4061b8136669cea53ec6751c2b6cc94a9/pymwp/analysis.py#L59 This index is used as the degree of index is initially 0. Currently there are two cases that cause this index to increment by 1:
Constant and unary do not cause index to increment; we detect the variable, add it to matrix, but do not increment index. The index of this program is 2:
Not incrementing the index causes the following problem with this program:
index = 0 --> intertools.product generates no choices --> analysis concludes "infinite" based on no valid choices remain after eval (there were 0 to being with) ---> error b/c delta graph should catch all infinite programs. It is not really infinite, the problem is the index is never incremented. Maybe in the infinity check could say: if there are some variables, index > 0, and still no valid choices then it is infinity; but constrain it on index > 0? |
Please review PR #50 if it makes sense why I added it? It is a very small patch but I believe necessary to resolve the primary issue. I will merge it to continue on the main issue. |
Regarding "we should output the same" for these two programs: int y1; // m // (from AST) this is a declaration
y1 = 0; // 0 // is this 0 here correct? compose using C rule (x) to get m x 0 = 0 --> final matrix: int y1 = 0; // m // (from AST) this is a declaration (with value 0) declaration --> 1 x 1 identity matrix --> final matrix: No composition in the second example, so final result is the identity matrix. Maybe if it is declaration + initialization it needs to be broken down and composed? |
That's the number of choices we have to make, I believe. When there is an assignment, we must (essentially) decide if the value was obtained using E1 / E2 / E3 / E4. For that example,
There is no choice, as 0 does not relate to the input / there is no choice for the derivation (cf. also #49 ). So it's "normal" that this index remains at 0.
Well, that's once again related to #49 : we made the choice of saying "if your variable is erased, its coefficient should become 0". It is not completely clear (at least to me) if we can do that while maintaining the results, but I believe this is the choice we took thus far. |
I have limited the scope of this issue in the PR to only include declarations leaving the question about initialization open to discussion in #49. This means we will get different result for these two programs: int y1;
y1 = 0; int y1 = 0; because the first is analyzed as "declaration" then "assignment of constant" |
Analyzing simple program like this yields empty result because we skip declaration statements and currently we pick up variables from when they are being used. This issue is to discuss the analysis output for such simple program.
https://github.com/seiller/pymwp/blob/252b5e3f9e159ab17990d0e16f65dd14187a1adc/c_files/basics/assign_value.c#L6-L8
We could display:
but it requires adding some logic to handle declaration nodes in the AST, so that the presence of the variable is discovered and then to run the analysis to generate the matrix.
-or- we display empty output which is not very user-friendly.
-or- we still skip declaration nodes in the AST, and then if the analysis discovers no variables, we can say some alternative output message to indicate "there was nothing to analyze" (I am putting this in quotes because it needs better wording to be clear in meaning).
Comments?
The text was updated successfully, but these errors were encountered: