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

[Emily Bellows]

You’re right, it doesn't. I hadn’t run into RecursiveASTVisitor yet, I will rewrite it to use that and to test for that in the test file.

Thanks for your help!

> On Dec 5, 2014, at 9:57 AM, Gabor Kozar <kozargabor at gmail.com> wrote:
> 
> 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)
>  
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev