Check cell properties for restricted attributes #3982
Assignees
Labels
Comments
|
We should separate out a class for attributes that can appear on configuration declarations, and do the check for these early enough to catch the sentences that are generated from these declarations. |
|
Does the |
rv-jenkins
pushed a commit
that referenced
this issue
Feb 13, 2024
When we originally added `group(_)` to replace unstructured attributes, we treated it as syntactic sugar where `[group(foo1,...,fooN)]` expands internally into `[foo1,...,fooN]`. However, on a second look, this expansion is unnecessary - `group(_)` is only relevant in the `Context` object used by `KILtoKORE` which maps tags in syntax blocks to sets of corresponding `Production`s. In this PR, we then - Remove all the `ProcessGroupAttributes` logic and instead just parse `group(_)` when we build the `Context`. - Remove the special-cased `KeyType.UserGroup` - (Opinionated) Remove the mention of `group(_)` from the "unrecognized attributes" error message. - The hint was useful when we first disallowed unstructured attributes. However, now that `group(_)` and the whitelist are more established, it feels a bit odd to assume any unrecognized attribute is an attempt to create a group. As a side effect of this change, - `syntax` declarations can now only refer to productions by their klabel or group. Previously, any attribute was permitted, and the priority or associativity would apply to everything with that attribute. - Cell properties will no longer expand `<cell group="foo"> </cell>` - This should be disallowed entirely, but see #3982 - We no longer error if a group conflicts with a built-in attribute - Before, this error was required lest `[group(function)]` be treated the same as `[function]`
|
The Otherwise, attributes get automatically added before |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The attribute infrastructure fails to check cell properties for restricted attributes, allowing nonsense such as
The text was updated successfully, but these errors were encountered: