[PATCH] D59118: creduce script for clang crashes

[Amy Huang via Phabricator via cfe-commits]

akhuang added inline comments.


================
Comment at: clang/utils/creduce-clang-crash.py:43
+  # Get crash output
+  p = subprocess.Popen(build_script,
+                       stdout=subprocess.PIPE,
----------------
george.burgess.iv wrote:
> nit: can replace with `subprocess.check_output` unless we explicitly want to ignore the return value (in which case, we should probably still call `wait()` anyway?)
`check_output()` raises an error when the return code is nonzero, which it is in this case. I think `communicate()` calls `wait()`, though.


================
Comment at: clang/utils/creduce-clang-crash.py:49
+  output = ['#!/bin/bash']
+  output.append('%s --crash %s >& t.log || exit 1' % (llvm_not, cmd))
+
----------------
george.burgess.iv wrote:
> please `pipes.quote(llvm_not)` and `pipes.quote(cmd)`
`cmd` is already quoted, since it's read out of another file


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

https://reviews.llvm.org/D59118