[M extension] variable category generalization #217
Conversation
|
Ok, I don't think I can go much further in this PR. |
| iv.Mast.input_category | ||
| |> function | ||
| | Some s -> s | ||
| | None -> assert false |
There was a problem hiding this comment.
Here you should maybe put an error message in case someone enters an attribute not in the list of strings you accept above?
There was a problem hiding this comment.
So the issue here would be that none of the expected categories appears, not that there is an unexpected one. I definitely would add a clean error message here, but this file in its current state ought to fall into oblivion as it's mostly adhoc, hardcoded, necessary mess that works only because tailored for the current M codebase.
| type var_type = Input | Computed | ||
|
|
||
| type var_category_decl = { | ||
| var_type : var_type; |
There was a problem hiding this comment.
I don't really understand the meaning of this struct. Does this correspond in the syntax to a declaration of a new variable category, or a new variable ? If this was the declaration of a new variable category I would just have expected a string here (déclaration catégorie XXX or déclaration attribut XXX).
There was a problem hiding this comment.
Oh I see the declaration is of the sort déclaration catégorie XXX saisie attributs YYY, ZZZ
There was a problem hiding this comment.
Yes, the expected attributes are simply listed while the actual category is the set of declared keywords before. It reads as "variables declared with keywords that includes this set are expected to have exactly these attributes."
|
|
||
| let computed_category = "calculee" | ||
|
|
||
| let base_category = "base" |
There was a problem hiding this comment.
Cant' these be put in a attributs.m file and not hardcoded in the compiler?
There was a problem hiding this comment.
This is only to minimize occurrences of hardcoded strings in various places of the code. Because the compiler still needs to identify them for lack of generality. This ought to disappear once every specificities like this are declared away in the user code instead of in the compiler.
| "No variable categories defined. No check will be performed."; | ||
| categories | ||
|
|
||
| let check_var_category (categories : Mast.var_category_decl Pos.marked list) |
There was a problem hiding this comment.
The reason why you relaxed the requirements for category checking with regards to the attributes is that you don't want to modify the current M code that does not respect strict rules (always the same attributes for each categories) ?
There was a problem hiding this comment.
Yes, although I am not sure of what relaxing you meant exactly.
There was a problem hiding this comment.
In the behavior introduced by this PR, if you declare a variable of category X with all the expected attributes for X AND SOME MORE, then it's OK to have more attributes, right?
There was a problem hiding this comment.
It's ok to have more category keywords, not more attributes. Say a category X Y, a variable of category X Y Z would fit, but a warning would be raised if the sets of attribute are not the same.
And in fact it should be an error, since those attributes will eventually end up as field of a C struct in the generated code. We can't allow variables that would need more fields than declared.
| var_type; | ||
| var_category = c; | ||
| var_attributes = attr; | ||
| }, |
Implementing M extension suggestion of @david-michel1.
WIP, as the generalization implies to let go of some structure (and hypothesis on the M code), making parts of the compiler unstable with hardcoded values.
Reviews are appreciated nonetheless, to point out leverage for robustness or missed deadcode.