[PATCH] D59725: Additions to creduce script

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

In D59725#1443990 <https://reviews.llvm.org/D59725#1443990>, @george.burgess.iv wrote:

> Only a few more nits on my side, and this LGTM. WDYT, arichardson?


LGTM with the minor tempfile changes.



================
Comment at: clang/utils/creduce-clang-crash.py:201
+    # Instead of modifying the filename in the test file, just run the command
+    empty_file = tempfile.NamedTemporaryFile()
+    is_interesting = self.check_expected_output(filename=empty_file.name)
----------------
`with tempfile.NamedTemporaryFile() as empty_file:`? Definitely works with python 3 and 2.7 docs say `This file-like object can be used in a with statement, just like a normal file.`.


================
Comment at: clang/utils/creduce-clang-crash.py:210
+    # use delete=False in case the tmpfile flag causes problems when copying
+    tmpfile = tempfile.NamedTemporaryFile(delete=False)
+
----------------
Use a with statement?


================
Comment at: clang/utils/creduce-clang-crash.py:212
+
+    cmd = self.get_crash_cmd() + ['-E', '-P']
+    try:
----------------
Some crash messages might include the line numbers, do you think it makes sense to fall back to running with -E but without -P and also checking that? I do it in my script but I'm not sure preprocessing saves that much time since creduce will try to remove those statements early.


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

https://reviews.llvm.org/D59725