[PATCH] D59118: creduce script for clang crashes
Nico Weber via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 11 18:25:47 PDT 2019
thakis added a comment.
(A few python style comments; feel free to ignore, and feel free to land no matter what you do with the comments.)
Comment at: clang/utils/creduce-clang-crash.py:1
+# A tool that calls C-Reduce to create a minimal reproducer for clang crashes
> george.burgess.iv wrote:
> > nit: please specify a python version here, since the world is steadily making `python` == `python3` (if `pipes.quote` is working, I assume you'll want `#!/usr/bin/env python2`)
> Please don't do that, there is no python2.exe or python3.exe on Windows. `#!/usr/bin/env python` is the simplest thing that could work.
There's no python2 on mac either. `#!/usr/bin/env python` is the correct first line for executable python scripts.
Comment at: clang/utils/creduce-clang-crash.py:29
+ return exe_file
+ return None
There's `distutils.spawn.find_executable()` which does this for you.
Comment at: clang/utils/creduce-clang-crash.py:68
+ parser = ArgumentParser(description='Runs C-Reduce on the input file')
+ parser.add_argument('build_script', type=str, nargs=1,
It's a somewhat common pattern to put a module docstring at the top of the file instead of a comment which explains usage, and then say `description=__doc__` here; see llvm/utils/gn/build/symlink_or_copy.py for an example.
Comment at: clang/utils/creduce-clang-crash.py:90
+ print(("ERROR: input file '%s' does not exist") % build_script)
It's a bit more common to say `return 1` here and do `sys.exit(main())` at the bottom.
CHANGES SINCE LAST ACTION
More information about the cfe-commits