[debuginfo-tests] 723a8ae - [dexter] Change line label reference syntax to enable label-relative offsets (1/2)

via llvm-commits llvm-commits at lists.llvm.org
Fri May 21 00:59:20 PDT 2021


Author: OCHyams
Date: 2021-05-21T08:58:58+01:00
New Revision: 723a8ae5dab25b329c6aebc25564b3630e8e7d17

URL: https://github.com/llvm/llvm-project/commit/723a8ae5dab25b329c6aebc25564b3630e8e7d17
DIFF: https://github.com/llvm/llvm-project/commit/723a8ae5dab25b329c6aebc25564b3630e8e7d17.diff

LOG: [dexter] Change line label reference syntax to enable label-relative offsets (1/2)

This patch changes how line labels are resolved in order to enable
label-relative offsets to be used in commands. This is a breaking change in
dexter. Instead of using label references directly as argument values, labels
will instead be referenced through a function `ref(str)`.

    // No way to use offsets currently.
    Currently: DexExpectWatchValue('x', '1', on_line='labled_line')
    Patched:   DexExpectWatchValue('x', '1', on_line=ref('labled_line'))
    Patched:   DexExpectWatchValue('x', '1', on_line=ref('labled_line') + 3)

A dexter command is "parsed" by finding the whole command and sending it off to
`eval`. This change adds a function called `ref` to the `eval` globals map that
simply looks up the name and returns an int. If the line name hasn't been
defined, or a name is defined more than once, an error is reported (see
err_bad_label_ref.cpp and err_duplicate_label.cpp). Label offsets can be
achieved by simply writing the desired expression.

The rationale behind removing the existing label referencing mechanic is for
consistency and to simplify the code required to make labels work.

I've separated the update to llvm's dexter tests into another patch for ease of
review here (D101148). Here is a small python script which can be used to
update tests to use the new syntax:
https://gist.github.com/OCHyams/8255efe7757cac266440ed2ba55f1442

If it helps anyone using dexter on downstream tests we can come up with a
deprecation plan instead out outright removing the existing syntax.

Reviewed By: jmorse

Differential Revision: https://reviews.llvm.org/D101147

Added: 
    debuginfo-tests/dexter/feature_tests/subtools/test/err_bad_label_ref.cpp
    debuginfo-tests/dexter/feature_tests/subtools/test/err_duplicate_label.cpp
    debuginfo-tests/dexter/feature_tests/subtools/test/label_offset.cpp

Modified: 
    debuginfo-tests/dexter/Commands.md
    debuginfo-tests/dexter/dex/command/ParseCommand.py
    debuginfo-tests/dexter/dex/command/commands/DexExpectProgramState.py
    debuginfo-tests/dexter/dex/command/commands/DexExpectWatchBase.py
    debuginfo-tests/dexter/dex/command/commands/DexLimitSteps.py

Removed: 
    


################################################################################
diff  --git a/debuginfo-tests/dexter/Commands.md b/debuginfo-tests/dexter/Commands.md
index 27d6993c37bac..f61418bbdeedd 100644
--- a/debuginfo-tests/dexter/Commands.md
+++ b/debuginfo-tests/dexter/Commands.md
@@ -217,8 +217,13 @@ large test cases that would normally take much longer to complete.
 
 ### Description
 Name the line this command is found on or 'on_line' if it is provided. Line
-names can be referenced by other commands expecting line number arguments.
-For example, `DexExpectWatchValues(..., on_line='my_line_name')`.
+names can be converted to line numbers with the `ref(str)` function. For
+example, `DexExpectWatchValues(..., on_line=ref('my_line_name'))`. Use
+arithmetic operators to get offsets from labels:
+
+    DexExpectWatchValues(..., on_line=ref('my_line_name') + 3)
+    DexExpectWatchValues(..., on_line=ref('my_line_name') - 5)
+
 
 ### Heuristic
 This command does not contribute to the heuristic score.

diff  --git a/debuginfo-tests/dexter/dex/command/ParseCommand.py b/debuginfo-tests/dexter/dex/command/ParseCommand.py
index 8246ea9e3cf97..c9908ef4b399f 100644
--- a/debuginfo-tests/dexter/dex/command/ParseCommand.py
+++ b/debuginfo-tests/dexter/dex/command/ParseCommand.py
@@ -69,7 +69,7 @@ def _merge_subcommands(command_name: str, valid_commands: dict) -> dict:
     return valid_commands
 
 
-def _build_command(command_type, raw_text: str, path: str, lineno: str) -> CommandBase:
+def _build_command(command_type, labels, raw_text: str, path: str, lineno: str) -> CommandBase:
     """Build a command object from raw text.
 
     This function will call eval().
@@ -80,8 +80,18 @@ def _build_command(command_type, raw_text: str, path: str, lineno: str) -> Comma
     Returns:
         A dexter command object.
     """
+    def label_to_line(label_name: str) -> int:
+        line = labels.get(label_name, None)
+        if line != None:
+            return line
+        raise format_unresolved_label_err(label_name, raw_text, path, lineno)
+
     valid_commands = _merge_subcommands(
-        command_type.get_name(), { command_type.get_name(): command_type })
+        command_type.get_name(), {
+            'ref': label_to_line,
+            command_type.get_name(): command_type,
+        })
+
     # pylint: disable=eval-used
     command = eval(raw_text, valid_commands)
     # pylint: enable=eval-used
@@ -91,27 +101,6 @@ def _build_command(command_type, raw_text: str, path: str, lineno: str) -> Comma
     return command
 
 
-def resolve_labels(command: CommandBase, commands: dict):
-    """Attempt to resolve any labels in command"""
-    dex_labels = commands['DexLabel']
-    command_label_args = command.get_label_args()
-    for command_arg in command_label_args:
-        for dex_label in list(dex_labels.values()):
-            if (os.path.samefile(dex_label.path, command.path) and
-                dex_label.eval() == command_arg):
-                command.resolve_label(dex_label.get_as_pair())
-    # labels for command should be resolved by this point.
-    if command.has_labels():
-        syntax_error = SyntaxError()
-        syntax_error.filename = command.path
-        syntax_error.lineno = command.lineno
-        syntax_error.offset = 0
-        syntax_error.msg = 'Unresolved labels'
-        for label in command.get_label_args():
-            syntax_error.msg += ' \'' + label + '\''
-        raise syntax_error
-
-
 def _search_line_for_cmd_start(line: str, start: int, valid_commands: dict) -> int:
     """Scan `line` for a string matching any key in `valid_commands`.
 
@@ -176,6 +165,16 @@ def get_column(self):
         return self.char + 1
 
 
+def format_unresolved_label_err(label: str, src: str, filename: str, lineno) -> CommandParseError:
+    err = CommandParseError()
+    err.src = src
+    err.caret = '' # Don't bother trying to point to the bad label.
+    err.filename = filename
+    err.lineno = lineno
+    err.info = f'Unresolved label: \'{label}\''
+    return err
+
+
 def format_parse_err(msg: str, path: str, lines: list, point: TextPoint) -> CommandParseError:
     err = CommandParseError()
     err.filename = path
@@ -193,10 +192,27 @@ def skip_horizontal_whitespace(line, point):
             return
 
 
+def add_line_label(labels, label, cmd_path, cmd_lineno):
+    # Enforce unique line labels.
+    if label.eval() in labels:
+        err = CommandParseError()
+        err.info = f'Found duplicate line label: \'{label.eval()}\''
+        err.lineno = cmd_lineno
+        err.filename = cmd_path
+        err.src = label.raw_text
+        # Don't both trying to point to it since we're only printing the raw
+        # command, which isn't much text.
+        err.caret = ''
+        raise err
+    labels[label.eval()] = label.get_line()
+
+
 def _find_all_commands_in_file(path, file_lines, valid_commands):
+    labels = {} # dict of {name: line}.
     commands = defaultdict(dict)
     paren_balance = 0
     region_start = TextPoint(0, 0)
+
     for region_start.line in range(len(file_lines)):
         line = file_lines[region_start.line]
         region_start.char = 0
@@ -222,7 +238,7 @@ def _find_all_commands_in_file(path, file_lines, valid_commands):
             end, paren_balance = _search_line_for_cmd_end(line, region_start.char, paren_balance)
             # Add this text blob to the command.
             cmd_text_list.append(line[region_start.char:end])
-            # Move parse ptr to end of line or parens
+            # Move parse ptr to end of line or parens.
             region_start.char = end
 
             # If the parens are unbalanced start reading the next line in an attempt
@@ -235,6 +251,7 @@ def _find_all_commands_in_file(path, file_lines, valid_commands):
             try:
                 command = _build_command(
                     valid_commands[command_name],
+                    labels,
                     raw_text,
                     path,
                     cmd_point.get_lineno(),
@@ -252,7 +269,8 @@ def _find_all_commands_in_file(path, file_lines, valid_commands):
                 err_point.char += len(command_name)
                 raise format_parse_err(str(e), path, file_lines, err_point)
             else:
-                resolve_labels(command, commands)
+                if type(command) is DexLabel:
+                    add_line_label(labels, command, path, cmd_point.get_lineno())
                 assert (path, cmd_point) not in commands[command_name], (
                     command_name, commands[command_name])
                 commands[command_name][path, cmd_point] = command

diff  --git a/debuginfo-tests/dexter/dex/command/commands/DexExpectProgramState.py b/debuginfo-tests/dexter/dex/command/commands/DexExpectProgramState.py
index 78335838a65f0..26f97b61590ea 100644
--- a/debuginfo-tests/dexter/dex/command/commands/DexExpectProgramState.py
+++ b/debuginfo-tests/dexter/dex/command/commands/DexExpectProgramState.py
@@ -66,18 +66,3 @@ def eval(self, step_collection: DextIR) -> bool:
                 self.encounters.append(step.step_index)
 
         return self.times < 0 < len(self.encounters) or len(self.encounters) == self.times
-
-    def has_labels(self):
-        return len(self.get_label_args()) > 0
-
-    def get_label_args(self):
-        return [frame.location.lineno
-                    for frame in self.expected_program_state.frames
-                        if frame.location and
-                        isinstance(frame.location.lineno, str)]
-
-    def resolve_label(self, label_line__pair):
-        label, line = label_line__pair
-        for frame in self.expected_program_state.frames:
-            if frame.location and frame.location.lineno == label:
-                frame.location.lineno = line

diff  --git a/debuginfo-tests/dexter/dex/command/commands/DexExpectWatchBase.py b/debuginfo-tests/dexter/dex/command/commands/DexExpectWatchBase.py
index 24ef4ccede8bb..e892f01619ca8 100644
--- a/debuginfo-tests/dexter/dex/command/commands/DexExpectWatchBase.py
+++ b/debuginfo-tests/dexter/dex/command/commands/DexExpectWatchBase.py
@@ -82,22 +82,6 @@ def missing_values(self):
     def encountered_values(self):
         return sorted(list(set(self.values) - self._missing_values))
 
-
-    def resolve_label(self, label_line_pair):
-        # from_line and to_line could have the same label.
-        label, lineno = label_line_pair
-        if self._to_line == label:
-            self._to_line = lineno
-        if self._from_line == label:
-            self._from_line = lineno
-
-    def has_labels(self):
-        return len(self.get_label_args()) > 0
-
-    def get_label_args(self):
-        return [label for label in (self._from_line, self._to_line)
-                      if isinstance(label, str)]
-
     @abc.abstractmethod
     def _get_expected_field(self, watch):
         """Return a field from watch that this ExpectWatch command is checking.

diff  --git a/debuginfo-tests/dexter/dex/command/commands/DexLimitSteps.py b/debuginfo-tests/dexter/dex/command/commands/DexLimitSteps.py
index 09eb05007cc65..5090ff40ccb4e 100644
--- a/debuginfo-tests/dexter/dex/command/commands/DexLimitSteps.py
+++ b/debuginfo-tests/dexter/dex/command/commands/DexLimitSteps.py
@@ -32,22 +32,6 @@ def __init__(self, *args, **kwargs):
                 ', '.join(kwargs)))
         super(DexLimitSteps, self).__init__()
 
-    def resolve_label(self, label_line_pair):
-        label, lineno = label_line_pair
-        if isinstance(self.from_line, str):
-            if self.from_line == label:
-                self.from_line = lineno
-        if isinstance(self.to_line, str):
-            if self.to_line == label:
-                self.to_line = lineno
-
-    def has_labels(self):
-        return len(self.get_label_args()) > 0
-
-    def get_label_args(self):
-        return [label for label in (self.from_line, self.to_line)
-                      if isinstance(label, str)]
-
     def eval(self):
         raise NotImplementedError('DexLimitSteps commands cannot be evaled.')
 

diff  --git a/debuginfo-tests/dexter/feature_tests/subtools/test/err_bad_label_ref.cpp b/debuginfo-tests/dexter/feature_tests/subtools/test/err_bad_label_ref.cpp
new file mode 100644
index 0000000000000..424edaa121a99
--- /dev/null
+++ b/debuginfo-tests/dexter/feature_tests/subtools/test/err_bad_label_ref.cpp
@@ -0,0 +1,14 @@
+// Purpose:
+//      Check that referencing an undefined label gives a useful error message.
+//
+// RUN: not %dexter_regression_test -v -- %s | FileCheck %s --match-full-lines
+//
+// CHECK: parser error:{{.*}}err_bad_label_ref.cpp(14): Unresolved label: 'label_does_not_exist'
+// CHECK-NEXT: {{Dex}}ExpectWatchValue('result', '0', on_line=ref('label_does_not_exist'))
+
+int main() {
+    int result = 0;
+    return result;
+}
+
+// DexExpectWatchValue('result', '0', on_line=ref('label_does_not_exist'))

diff  --git a/debuginfo-tests/dexter/feature_tests/subtools/test/err_duplicate_label.cpp b/debuginfo-tests/dexter/feature_tests/subtools/test/err_duplicate_label.cpp
new file mode 100644
index 0000000000000..7da69203ea38f
--- /dev/null
+++ b/debuginfo-tests/dexter/feature_tests/subtools/test/err_duplicate_label.cpp
@@ -0,0 +1,12 @@
+// Purpose:
+//      Check that defining duplicate labels gives a useful error message.
+//
+// RUN: not %dexter_regression_test -v -- %s | FileCheck %s --match-full-lines
+//
+// CHECK: parser error:{{.*}}err_duplicate_label.cpp(11): Found duplicate line label: 'oops'
+// CHECK-NEXT: {{Dex}}Label('oops')
+
+int main() {
+    int result = 0; // DexLabel('oops')
+    return result;  // DexLabel('oops')
+}

diff  --git a/debuginfo-tests/dexter/feature_tests/subtools/test/label_offset.cpp b/debuginfo-tests/dexter/feature_tests/subtools/test/label_offset.cpp
new file mode 100644
index 0000000000000..81d4ca47b1144
--- /dev/null
+++ b/debuginfo-tests/dexter/feature_tests/subtools/test/label_offset.cpp
@@ -0,0 +1,24 @@
+// Purpose:
+//      Check that we can use label-relative line numbers.
+//
+// RUN: %dexter_regression_test -v -- %s | FileCheck %s
+//
+// CHECK: label_offset.cpp: (1.0000)
+
+int main() {  // DexLabel('main')
+    int var = 0;
+    var = var;
+    return 0;
+}
+
+/*
+DexExpectWatchValue('var', '0', from_line=ref('main')+2, to_line=ref('main')+3)
+DexExpectProgramState({
+    'frames': [
+        {
+            'location': { 'lineno': ref('main')+2 },
+            'watches': { 'var': '0' }
+        }
+    ]
+})
+*/


        


More information about the llvm-commits mailing list