[TableGen][Docs] Accept "code" as a Type #124902
Conversation
Previously the Type production did not include "code", which was only
accepted in one place in the grammar:
BodyItem: (`Type` | "code") `TokIdentifier` ["=" `Value`] ";"
However the parser implementation accepts "code" as a Type with only one
place where it is *not* allowed, corresponding to this production:
SimpleValue9: `BangOperator` ["<" `Type` ">"] "(" `ValueListNE` ")"
This patch changes the production for Type to include "code", thereby
fixing most occurrences of Type in the grammar, and documents the
restriction for BangOperator Types in the text instead of codifying it
in the grammar.
|
@llvm/pr-subscribers-tablegen Author: Jay Foad (jayfoad) ChangesPreviously the Type production did not include "code", which was only BodyItem: ( However the parser implementation accepts "code" as a Type with only one SimpleValue9: This patch changes the production for Type to include "code", thereby Full diff: https://github.com/llvm/llvm-project/pull/124902.diff 1 Files Affected:
diff --git a/llvm/docs/TableGen/ProgRef.rst b/llvm/docs/TableGen/ProgRef.rst
index cfe61382658ec4..f26056475162fd 100644
--- a/llvm/docs/TableGen/ProgRef.rst
+++ b/llvm/docs/TableGen/ProgRef.rst
@@ -268,7 +268,7 @@ high-level types (e.g., ``dag``). This flexibility allows you to describe a
wide range of records conveniently and compactly.
.. productionlist::
- Type: "bit" | "int" | "string" | "dag"
+ Type: "bit" | "int" | "string" | "dag" | "code"
:| "bits" "<" `TokInteger` ">"
:| "list" "<" `Type` ">"
:| `ClassID`
@@ -285,6 +285,10 @@ wide range of records conveniently and compactly.
The ``string`` type represents an ordered sequence of characters of arbitrary
length.
+``code``
+ The keyword ``code`` is an alias for ``string`` which may be used to
+ indicate string values that are code.
+
``bits<``\ *n*\ ``>``
The ``bits`` type is a fixed-sized integer of arbitrary length *n* that
is treated as separate bits. These bits can be accessed individually.
@@ -498,6 +502,8 @@ arguments, producing a value for that bang operator. The ``!cond`` operator
takes a list of pairs of arguments separated by colons. See `Appendix A:
Bang Operators`_ for a description of each bang operator.
+The `Type` is only accepted for certain bang operators, and must not be
+``code``.
Suffixed values
---------------
@@ -670,7 +676,7 @@ arguments.
.. productionlist::
Body: ";" | "{" `BodyItem`* "}"
- BodyItem: (`Type` | "code") `TokIdentifier` ["=" `Value`] ";"
+ BodyItem: `Type` `TokIdentifier` ["=" `Value`] ";"
:| "let" `TokIdentifier` ["{" `RangeList` "}"] "=" `Value` ";"
:| "defvar" `TokIdentifier` "=" `Value` ";"
:| `Assert`
@@ -678,8 +684,7 @@ arguments.
A field definition in the body specifies a field to be included in the class
or record. If no initial value is specified, then the field's value is
uninitialized. The type must be specified; TableGen will not infer it from
-the value. The keyword ``code`` may be used to emphasize that the field
-has a string value that is code.
+the value.
The ``let`` form is used to reset a field to a new value. This can be done
for fields defined directly in the body or fields inherited from parent
|
|
This patch undoes some of the documentation changes made in https://reviews.llvm.org/D92269 "[TableGen] Eliminate the 'code' type". I don't understand the reason for those changes. |
TBH I don't see the need for this restriction. Why don't we just allow e.g. |
|
We decided to eliminate the code type because it is identical to string. I thought I had eliminated it completely, but apparently not. Is it used in any source files? |
Yes, quite a few, e.g.: |
|
Ah, so that's why I didn't eliminate it. All I really remember is deciding with Chris to eliminate/deprecate it. It seems like a perfectly good synonym to me. |
Is that an endorsement of this patch? Incidentally I would appreciate any comments on my other recent TableGen patches. Many of them just improve the documentation of the grammar. |
|
I endorse this patch. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/12731 Here is the relevant piece of the build log for the reference |
4 participants
Previously the Type production did not include "code", which was only
accepted in one place in the grammar:
BodyItem: (
Type| "code")TokIdentifier["="Value] ";"However the parser implementation accepts "code" as a Type with only one
place where it is not allowed, corresponding to this production:
SimpleValue9:
BangOperator["<"Type">"] "("ValueListNE")"This patch changes the production for Type to include "code", thereby
fixing most occurrences of Type in the grammar, and documents the
restriction for BangOperator Types in the text instead of codifying it
in the grammar.