[PATCH] D9600: Add scan-build python implementation

[Laszlo Nagy via cfe-commits]

rizsotto.mailinglist marked 3 inline comments as done.
rizsotto.mailinglist added a comment.

updated comments before upload a new diff.


================
Comment at: tools/scan-build-py/MANIFEST.in:1
@@ +1,2 @@
+include README.md
+include *.txt
----------------
dcoughlin wrote:
> rizsotto.mailinglist wrote:
> > dcoughlin wrote:
> > > How about this one? Is it needed in clang trunk?
> > in order to make this code a standalone python tool tool, we need this file. (see llvm/utils/lit directory for example.)
> Can you explain why this is needed? We didn't need one for the perl scan-build nor for SATestBuild.py/SATestAdd.py?
> 
> A difference between lit and scan-build is that scan-build is not a standalone tool. Are you envisioning users installing scan-build without installing clang?
yes, did set it up already. see https://pypi.python.org/pypi/scan-build 

================
Comment at: tools/scan-build-py/libscanbuild/driver.py:67
@@ +66,3 @@
+    except Exception:
+        logging.exception("Something unexpected had happened.")
+        return 127
----------------
jroelofs wrote:
> rizsotto.mailinglist wrote:
> > dcoughlin wrote:
> > > I think this error message can be improved. Perhaps "Unexpected error running intercept-build"?
> > this line is printed as:
> > 
> >   intercept-build: ERROR: Something unexpected had happened.
> >   (and the stack-trace)
> > 
> > because the logging formating. so, 'intercept-build' and 'error' will be part of the message anyway.
> Is there a pythonic way of doing llvm crash handlers? I.e. the "here's the steps to reproduce this, a stack trace, and a bug report url" things that clang spits out.
this crash/exception is not a clang crash. it's more like this program's fault. clang crash reports are recorded already in crash report files (and linked into the html report file).

================
Comment at: tools/scan-build-py/libscanbuild/intercept.py:98
@@ +97,3 @@
+
+    if args.override_compiler or not ear_library_path:
+        environment.update({
----------------
dcoughlin wrote:
> rizsotto.mailinglist wrote:
> > dcoughlin wrote:
> > > What is the purpose of setting CC/CXX environmental variables here? Doesn't libear intercept calls to the compiler? Why is intercept-cc/cxx needed?
> > documentation added. this branch is used when preload does not work. (eg.: windows builds or failed to compile `libear` for unknown reasons.)
> Is there an indication to the user that preload didn't work? Do you think there should be one? (Or is this an implementation detail that users do not need to be aware of?)
i think the user shall be notified when he requests it (emit debug level log now). but otherwise shall be hidden how this thing is working internally. (plenty of documentation explain internals in case if user want to know more.)


http://reviews.llvm.org/D9600