george.burgess.iv added a comment.

Just a few style nits for you, and this LGTM. I assume rnk and 
serge-sans-paille are content, so I'm happy to check this in for you once these 
are addressed.

Thanks!



================
Comment at: clang/utils/creduce-clang-crash.py:64
   crash_output, _ = p.communicate()
+  for msg in expected_output:
+    if msg not in crash_output:
----------------
nit: can be simplified to `return all(msg not in crash_output for msg in 
expected_output)`


================
Comment at: clang/utils/creduce-clang-crash.py:116
+  with open(os.devnull, 'w') as devnull:
+    p = subprocess.Popen(testfile, stdout=devnull)
+    p.communicate()
----------------
nit: looks like you can use `returncode = subprocess.call(testfile, 
stdout=devnull)` here


================
Comment at: clang/utils/creduce-clang-crash.py:124
+  with open(os.devnull, 'w') as devnull:
+    p = subprocess.Popen([testfile, empty_file], stdout=devnull)
+    p.communicate()
----------------
same `subprocess.call` nit


================
Comment at: clang/utils/creduce-clang-crash.py:243
 
+  #FIXME: reduce the clang crash command
+
----------------
nit: please add a space: `# FIXME`


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

https://reviews.llvm.org/D59440



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to