[PATCH] D99651: [Dexter] Implement DexDeclareFile, a new Dexter command

[Orlando Cazalet-Hyams via Phabricator via llvm-commits]

Orlando added a comment.

LGTM with a few tiny nits, but please wait for a +1 from someone else too.



================
Comment at: debuginfo-tests/dexter/Commands.md:229
+### Description
+Changes the path attribute of all commands from this point in the test onwards.
+The new path holds until the end of the test file or until a new DexDeclareFile
----------------
`Changes` -> `Change` (or maybe `Set`) fits the existing style of this document better imo, but this isn't very important.


================
Comment at: debuginfo-tests/dexter/dex/command/ParseCommand.py:264
+                        cmd_path = os.path.join(source_dir, cmd_path)
+                    declared_files.add(cmd_path)
                 resolve_labels(command, commands)
----------------
Should `cmd_path` get normalized with `os.path.normcase` here since the user can spell the path name in a few different ways on windows? Unhelpfully I can't remember exactly why it was, but when tinkering with this patch, at one point I did have to make that change.

Does everything work if the `DexDeclareFile` commands `use\\annOying/PathSpelling` (i.e. a mix of case and slashes that doesn't match the "canonical" spelling)? Maybe this deserves a test.


================
Comment at: debuginfo-tests/dexter/dex/tools/TestToolBase.py:103
 
+        # test files contain dexter commands.
+        options.test_files = []
----------------
Tiny nit: comments should probably follow the llvm style (comments added here don't start with capitals but should).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99651/new/

https://reviews.llvm.org/D99651