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

[Jeremy Morse via Phabricator via llvm-commits]

jmorse accepted this revision.
jmorse added a comment.
This revision is now accepted and ready to land.

LGTM, with a couple of nits, plus Orlandos comments should be addressed.

The precompiled binary test is great, that should get us going in the right direction.



================
Comment at: debuginfo-tests/dexter/dex/tools/test/Tool.py:144-145
+
+        for new_source_file in new_source_files:
+              self.context.options.source_files.append(new_source_file)
 
----------------
Mega nit: `self.context.options.source_files.extend(list(new_source_files))` is slightly more concise.


================
Comment at: debuginfo-tests/dexter/feature_tests/commands/penalty/dex_declare_file.cpp:16
+
+// DexDeclareFile('not_dex_declare_file.cpp')
+// DexExpectWatchValue('result', 0, on_line='return')
----------------
Major nit, but I think this should be a filename that absolutely screams that it doesn't exist; thus something like "this_file_should_not_exist.cpp". that avoids the reader spending a second wondering whether the "not" prefix is significant.

(This might be because my default name for anything, when moved, is not_$originalname).


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

https://reviews.llvm.org/D99651