[llvm] fbec83c - [llvm][TableGen][Jupyter] Add configurable default reset behaviour
David Spickett via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 4 03:16:49 PDT 2023
Author: David Spickett
Date: 2023-08-04T11:16:43+01:00
New Revision: fbec83cbe3ffcb53586f365651c6e208be3d1880
URL: https://github.com/llvm/llvm-project/commit/fbec83cbe3ffcb53586f365651c6e208be3d1880
DIFF: https://github.com/llvm/llvm-project/commit/fbec83cbe3ffcb53586f365651c6e208be3d1880.diff
LOG: [llvm][TableGen][Jupyter] Add configurable default reset behaviour
Often you are doing one of 2 things:
* Building a larger example from many small cells.
* Showing many small isolated examples.
The default so far has followed iPython, where cells are connected
by default (in its case, the interpreter state backing them sticks
around).
This change adds a new magic `%config` where you can change the setting
`cellreset` to change that behaviour (this is currently the only setting).
Also added is a `%noreset` magic so that along with `%reset` you can
override the default for one particular cell.
The default is equivalent to `%config cellreset off`. If you then
wanted to reset in a cell, you can just do %reset to override it.
(this is what the current notebooks do)
If you instead do `%config cellreset on`, cells always reset and
you can choose not to using `%noreset`.
The setting is named `cellreset` not `reset` to differentiate it
a bit more from the one off command `reset`.
The demo notebook has been updated with examples of this in action.
Reviewed By: awarzynski
Differential Revision: https://reviews.llvm.org/D149055
Added:
Modified:
llvm/utils/TableGen/jupyter/LLVM_TableGen.ipynb
llvm/utils/TableGen/jupyter/LLVM_TableGen.md
llvm/utils/TableGen/jupyter/tablegen_kernel/kernel.py
Removed:
################################################################################
diff --git a/llvm/utils/TableGen/jupyter/LLVM_TableGen.ipynb b/llvm/utils/TableGen/jupyter/LLVM_TableGen.ipynb
index 058d9a5d382391..cef0f88ba8f3d1 100644
--- a/llvm/utils/TableGen/jupyter/LLVM_TableGen.ipynb
+++ b/llvm/utils/TableGen/jupyter/LLVM_TableGen.ipynb
@@ -169,11 +169,187 @@
"def other_thing : Stuff {}"
]
},
+ {
+ "cell_type": "markdown",
+ "id": "8f120cee",
+ "metadata": {},
+ "source": [
+ "You can also configure the default reset behaviour using the `%config` magic."
+ ]
+ },
+ {
+ "cell_type": "code",
+ "execution_count": 6,
+ "id": "7c356853",
+ "metadata": {},
+ "outputs": [
+ {
+ "name": "stdout",
+ "output_type": "stream",
+ "text": [
+ "------------- Classes -----------------\n",
+ "class Thing {\n",
+ "}\n",
+ "------------- Defs -----------------\n"
+ ]
+ }
+ ],
+ "source": [
+ "%config cellreset on\n",
+ "class Thing {}"
+ ]
+ },
+ {
+ "cell_type": "code",
+ "execution_count": 7,
+ "id": "c5399401",
+ "metadata": {},
+ "outputs": [
+ {
+ "name": "stderr",
+ "output_type": "stream",
+ "text": [
+ "<stdin>:2:13: error: Couldn't find class 'Thing'\n",
+ "def AThing: Thing {}\n",
+ " ^\n"
+ ]
+ }
+ ],
+ "source": [
+ "// The cache is reset here so this is an error.\n",
+ "def AThing: Thing {}"
+ ]
+ },
+ {
+ "cell_type": "markdown",
+ "id": "53220700",
+ "metadata": {},
+ "source": [
+ "The default value is `off`, meaning cells are connected. If you want to override the default for one cell only, use the `%reset` or `%noreset` magic. These always override the default."
+ ]
+ },
+ {
+ "cell_type": "code",
+ "execution_count": 8,
+ "id": "50a865ea",
+ "metadata": {},
+ "outputs": [
+ {
+ "name": "stdout",
+ "output_type": "stream",
+ "text": [
+ "------------- Classes -----------------\n",
+ "class Thing {\n",
+ "}\n",
+ "------------- Defs -----------------\n"
+ ]
+ }
+ ],
+ "source": [
+ "class Thing {}"
+ ]
+ },
+ {
+ "cell_type": "code",
+ "execution_count": 9,
+ "id": "3c079649",
+ "metadata": {},
+ "outputs": [
+ {
+ "name": "stdout",
+ "output_type": "stream",
+ "text": [
+ "------------- Classes -----------------\n",
+ "class Thing {\n",
+ "}\n",
+ "------------- Defs -----------------\n",
+ "def AThing {\t// Thing\n",
+ "}\n"
+ ]
+ }
+ ],
+ "source": [
+ "%noreset\n",
+ "// This works because of the noreset above.\n",
+ "def AThing: Thing {}"
+ ]
+ },
+ {
+ "cell_type": "code",
+ "execution_count": 10,
+ "id": "75f3c51c",
+ "metadata": {},
+ "outputs": [
+ {
+ "name": "stderr",
+ "output_type": "stream",
+ "text": [
+ "<stdin>:2:19: error: Couldn't find class 'Thing'\n",
+ "def AnotherThing: Thing {}\n",
+ " ^\n"
+ ]
+ }
+ ],
+ "source": [
+ "// This does not because we're not changing the default.\n",
+ "def AnotherThing: Thing {}"
+ ]
+ },
+ {
+ "cell_type": "code",
+ "execution_count": 11,
+ "id": "9abe502e",
+ "metadata": {},
+ "outputs": [
+ {
+ "name": "stdout",
+ "output_type": "stream",
+ "text": [
+ "------------- Classes -----------------\n",
+ "------------- Defs -----------------\n"
+ ]
+ }
+ ],
+ "source": [
+ "%config cellreset off\n",
+ "%reset\n",
+ "// Here we have an empty cache and default reset behaviour."
+ ]
+ },
+ {
+ "cell_type": "markdown",
+ "id": "0de42c3c",
+ "metadata": {},
+ "source": [
+ "It is not valid to have `%reset` and `%noreset` in the same cell."
+ ]
+ },
+ {
+ "cell_type": "code",
+ "execution_count": 12,
+ "id": "a763daa4",
+ "metadata": {},
+ "outputs": [
+ {
+ "name": "stderr",
+ "output_type": "stream",
+ "text": [
+ "%reset and %noreset in the same cell is not allowed. Use only one, or neither."
+ ]
+ }
+ ],
+ "source": [
+ "%reset\n",
+ "%noreset"
+ ]
+ },
{
"cell_type": "markdown",
"id": "f6d4613b",
"metadata": {},
"source": [
+ "Consider setting `cellreset` to the majority usecase for your notebook. For example a tutorial building a large example across many cells will likely want it `off`. One with many standalone examples, `on`.\n",
+ "\n",
"There is a \"magic\" directive `%args` that you can use to send command line arguments to `llvm-tblgen`.\n",
"\n",
"For example, here we have some code that shows a warning."
@@ -181,7 +357,7 @@
},
{
"cell_type": "code",
- "execution_count": 6,
+ "execution_count": 13,
"id": "2586893b",
"metadata": {},
"outputs": [
@@ -212,7 +388,7 @@
},
{
"cell_type": "code",
- "execution_count": 7,
+ "execution_count": 14,
"id": "ae078bc4",
"metadata": {},
"outputs": [
@@ -242,7 +418,7 @@
},
{
"cell_type": "code",
- "execution_count": 8,
+ "execution_count": 15,
"id": "9634170c",
"metadata": {},
"outputs": [
@@ -264,7 +440,7 @@
},
{
"cell_type": "code",
- "execution_count": 9,
+ "execution_count": 16,
"id": "fa15b542",
"metadata": {},
"outputs": [
@@ -293,7 +469,7 @@
},
{
"cell_type": "code",
- "execution_count": 10,
+ "execution_count": 17,
"id": "2ac46c7a",
"metadata": {},
"outputs": [
diff --git a/llvm/utils/TableGen/jupyter/LLVM_TableGen.md b/llvm/utils/TableGen/jupyter/LLVM_TableGen.md
index a8a6f7cfb98959..aa5068f6c18001 100644
--- a/llvm/utils/TableGen/jupyter/LLVM_TableGen.md
+++ b/llvm/utils/TableGen/jupyter/LLVM_TableGen.md
@@ -77,6 +77,93 @@ def other_thing : Stuff {}
^
+You can also configure the default reset behaviour using the `%config` magic.
+
+
+```tablegen
+%config cellreset on
+class Thing {}
+```
+
+ ------------- Classes -----------------
+ class Thing {
+ }
+ ------------- Defs -----------------
+
+
+
+```tablegen
+// The cache is reset here so this is an error.
+def AThing: Thing {}
+```
+
+ <stdin>:2:13: error: Couldn't find class 'Thing'
+ def AThing: Thing {}
+ ^
+
+
+The default value is `off`, meaning cells are connected. If you want to override the default for one cell only, use the `%reset` or `%noreset` magic. These always override the default.
+
+
+```tablegen
+class Thing {}
+```
+
+ ------------- Classes -----------------
+ class Thing {
+ }
+ ------------- Defs -----------------
+
+
+
+```tablegen
+%noreset
+// This works because of the noreset above.
+def AThing: Thing {}
+```
+
+ ------------- Classes -----------------
+ class Thing {
+ }
+ ------------- Defs -----------------
+ def AThing { // Thing
+ }
+
+
+
+```tablegen
+// This does not because we're not changing the default.
+def AnotherThing: Thing {}
+```
+
+ <stdin>:2:19: error: Couldn't find class 'Thing'
+ def AnotherThing: Thing {}
+ ^
+
+
+
+```tablegen
+%config cellreset off
+%reset
+// Here we have an empty cache and default reset behaviour.
+```
+
+ ------------- Classes -----------------
+ ------------- Defs -----------------
+
+
+It is not valid to have `%reset` and `%noreset` in the same cell.
+
+
+```tablegen
+%reset
+%noreset
+```
+
+ %reset and %noreset in the same cell is not allowed. Use only one, or neither.
+
+Consider setting `cellreset` to the majority usecase for your notebook. For example a tutorial building a large example across many cells will likely want it `off`. One with many standalone examples, `on`.
+
There is a "magic" directive `%args` that you can use to send command line arguments to `llvm-tblgen`.
For example, here we have some code that shows a warning.
diff --git a/llvm/utils/TableGen/jupyter/tablegen_kernel/kernel.py b/llvm/utils/TableGen/jupyter/tablegen_kernel/kernel.py
index e1a05d936c8925..f6d3297bd84d8b 100644
--- a/llvm/utils/TableGen/jupyter/tablegen_kernel/kernel.py
+++ b/llvm/utils/TableGen/jupyter/tablegen_kernel/kernel.py
@@ -11,20 +11,34 @@
__version__ = "0.0.1"
+class TableGenKernelException(Exception):
+ pass
+
+
class TableGenKernel(Kernel):
"""Kernel using llvm-tblgen inside jupyter.
All input is treated as TableGen unless the first non whitespace character
is "%" in which case it is a "magic" line.
- The supported magic is:
- * %args - to set the arguments passed to llvm-tblgen.
- * %reset - to reset the cached code and magic state.
+ The supported cell magic is:
+ * %args - to set the arguments passed to llvm-tblgen.
+ * %reset - to reset the cached code and magic state.
+ * %noreset - to not reset the cached code and magic state
+ (useful when you have changed the default to always
+ reset the cache).
These are "cell magic" meaning it applies to the whole cell. Therefore
it must be the first line, or part of a run of magic lines starting
from the first line.
+ The following are global magic (that applies to all cells going
+ forward):
+ * %config - to change the behaviour of the kernel overall, including
+ changing defaults for things like resets.
+
+ Global magic must be written in the same way as cell magic.
+
```tablgen
%args
%reset
@@ -59,6 +73,9 @@ def __init__(self, **kwargs):
self._previous_code = ""
# The most recent set of magic since the last reset.
self._previous_magic = {}
+ # The default cache reset behaviour. True means do not cache anything
+ # between cells.
+ self._cell_reset = False
@property
def banner(self):
@@ -82,6 +99,61 @@ def get_executable(self):
return self._executable
+ def parse_config_magic(self, config):
+ """Config should be a list of parameters given to the %config command.
+ We allow only one setting per %config line and that setting can only
+ have one value.
+
+ Assuming the parameters are valid, update the kernel's setting with
+ the new value.
+
+ If there is an error, raise a TableGenKernelException.
+
+ >>> k.parse_config_magic([])
+ Traceback (most recent call last):
+ ...
+ TableGenKernelException: Incorrect number of parameters to %config. Expected %config <setting> <value>.
+ >>> k._cell_reset
+ False
+ >>> k.parse_config_magic(["a", "b", "c"])
+ Traceback (most recent call last):
+ ...
+ TableGenKernelException: Incorrect number of parameters to %config. Expected %config <setting> <value>.
+ >>> k.parse_config_magic(["notasetting", "..."])
+ Traceback (most recent call last):
+ ...
+ TableGenKernelException: Unknown kernel setting "notasetting". Possible settings are: "cellreset".
+ >>> k.parse_config_magic(["cellreset", "food"])
+ Traceback (most recent call last):
+ ...
+ TableGenKernelException: Invalid value for setting "cellreset", expected "on" or "off".
+ >>> k.parse_config_magic(["cellreset", "on"])
+ >>> k._cell_reset
+ True
+ >>> k.parse_config_magic(["cellreset", "off"])
+ >>> k._cell_reset
+ False
+ """
+ if len(config) != 2:
+ raise TableGenKernelException(
+ "Incorrect number of parameters to %config. Expected %config <setting> <value>."
+ )
+
+ name, value = config
+ if name != "cellreset":
+ raise TableGenKernelException(
+ 'Unknown kernel setting "{}". '
+ 'Possible settings are: "cellreset".'.format(name)
+ )
+
+ try:
+ self._cell_reset = {"on": True, "off": False}[value.lower()]
+ except KeyError:
+ raise TableGenKernelException(
+ 'Invalid value for setting "{}", '
+ 'expected "on" or "off".'.format(name)
+ )
+
def get_magic(self, code):
"""Given a block of code remove the magic lines from it.
Returns a tuple of newline joined code lines and a dictionary of magic.
@@ -128,6 +200,51 @@ def get_magic(self, code):
return "\n".join(code_lines), magic
+ def should_reset(self, magic):
+ """Return true if we should reset the cache, based on the default
+ setting and the current cell's magic %reset and/or %noreset.
+
+ >>> k._cell_reset = False
+ >>> k.should_reset({})
+ False
+ >>> k.should_reset({'reset': [], 'noreset': []})
+ Traceback (most recent call last):
+ ...
+ TableGenKernelException: %reset and %noreset in the same cell is not allowed. Use only one, or neither.
+ >>> k.should_reset({'reset': []})
+ True
+ >>> k.should_reset({'noreset': []})
+ False
+ >>> k._cell_reset = True
+ >>> k.should_reset({})
+ True
+ >>> k.should_reset({'reset': [], 'noreset': []})
+ Traceback (most recent call last):
+ ...
+ TableGenKernelException: %reset and %noreset in the same cell is not allowed. Use only one, or neither.
+ >>> k.should_reset({'reset': []})
+ True
+ >>> k.should_reset({'noreset': []})
+ False
+ """
+ # Cell reset is the default unless told otherwise.
+ should_reset = self._cell_reset
+ # Magic reset commands always win if present.
+ reset = magic.get("reset") is not None
+ noreset = magic.get("noreset") is not None
+
+ if reset and not noreset:
+ should_reset = True
+ elif noreset and not reset:
+ should_reset = False
+ elif noreset and reset:
+ raise TableGenKernelException(
+ "%reset and %noreset in the same cell is not allowed. Use only one, or neither."
+ )
+ # else neither are set so use the default.
+
+ return should_reset
+
def get_code_and_args(self, new_code):
"""Get the code that do_execute should use, taking into account
the code from any cached cells.
@@ -149,8 +266,12 @@ def get_code_and_args(self, new_code):
"""
new_code, new_magic = self.get_magic(new_code)
- # Only a reset in the newest cell actually does a reset.
- if new_magic.get("reset") is not None:
+ # Update kernel configuration first, if needed.
+ config_magic = new_magic.get("config")
+ if config_magic is not None:
+ self.parse_config_magic(config_magic)
+
+ if self.should_reset(new_magic):
self._previous_code = new_code
self._previous_magic = new_magic
else:
@@ -182,7 +303,10 @@ def do_execute(
self, code, silent, store_history=True, user_expressions=None, allow_stdin=False
):
"""Execute user code using llvm-tblgen binary."""
- all_code, args = self.get_code_and_args(code)
+ try:
+ all_code, args = self.get_code_and_args(code)
+ except TableGenKernelException as e:
+ return self.send_stderr(str(e))
# If we cannot find llvm-tblgen, propogate the error to the notebook.
# (in case the user is not able to see the output from the Jupyter server)
More information about the llvm-commits
mailing list