[cfe-dev] Working on my first patch for the static analyzer, few questions.

[Gabor Kozar]

Hi,

This probably belongs to cfe-commits instead.

Regarding FindMethodCallAnywhere(): would this find method calls that
are not direct children of the initializer Expr? For example:

> struct Moo { explicit Moo(int n) {} };
>
> int global_func(int x) { return x; }
>
> struct Foo : Moo { Foo() : Moo(global_func(member_func())) {}
>
> int member_func() { return 42; } };

If not, consider using a RecursiveASTVisitor instead. Regardless of
whether it currently works, you should test for this in your test file.

Otherwise, it's looking good to me, good job!

I also think that this would definitely qualify to become a
compiler warning.

---
Best regards,

Gábor 'ShdNx' Kozár http://gaborkozar.me



On Thu, Dec 4, 2014, at 01:30, Emily Bellows wrote:
> Hi everyone,
>
> I’ve been writing one of the potential checkers from the
> clang-analyzer website. It detects calls to member functions before
> all base classes are initialized in a constructor initializer list,
> which the standard says is undefined behavior (C++11 12.6.2.p13.)
>
> My checker finds these cases fine, it runs perfectly on the examples
> given from the standard, and I ran it on the LLVM codebase and it even
> found three instances of this (calls to allocHungoffUses in
> lib/IR/Instructions.cpp if you’re curious, lines 89, 196, and 3579.)
>
> Questions,
>
> 1. Does this seem like it might be better as a compiler warning? Right
>    now the checker only uses checkASTDecl, and so is not path
>    sensitive. It could be made path sensitive in the future to detect
>    code indirectly using ’this’, however.
>
> 2. Does the ReportBug method look sensible? Given the check is not
>    path sensitive, I couldn’t quite figure out what the
>    PathDiagnosticLocation class is supposed to represent, or whether
>    or not I’m using it correctly because most of the constructors take
>    an ExplodedNode. The error messages my checker gives out looks
>    reasonable to me, so it works, but I’m not sure if it’s idiomatic
>    usage of the reporting API.
>
> Example error:
>
> /Users/emilybellows/Workspace/llvm/lib/IR/Instructions.cpp:89:17:
> warning: Method called before all bases were initialized
> allocHungoffUses(PN.getNumOperands()), PN.getNumOperands()),
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> 3. Who is the code owner of this part of clang for when I end up
>    submitting the patch. Is the owner Chris Lattner by default? And
>    does this patch look ready to submit to cfe-commits?
>
> I welcome any other comments too, this is my first time working with
> either LLVM or Clang, and my first time contributing code to an open
> source project.
>
> Thanks, Emily
>
>
>
> _________________________________________________
> cfe-dev mailing list cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev Email had 1
> attachment:


>  * newchecker.patch 6k (application/octet-stream)

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20141205/dc84321f/attachment.html>