courbet added a comment.

In http://reviews.llvm.org/D18649#391001, @alexfh wrote:

> In http://reviews.llvm.org/D18649#390862, @courbet wrote:
>
> > In http://reviews.llvm.org/D18649#389363, @alexfh wrote:
> >
> > > Thank you for working on the new clang-tidy check!
> > >
> > > We usually recommend authors to run their checks on a large code base to 
> > > ensure it doesn't crash and doesn't generate obvious false positives. It 
> > > would be nice, if you could provide a quick summary of such a run (total 
> > > number of hits, number of what seems to be a false positive in a sample 
> > > of ~100).
> >
> >
> > The tool generated 20k positives on our codebase. On a sample of 100, there 
> > are:
> >
> > - 8 instances of the same exact code structure that's just wrong: const 
> > string var = FLAGS_some_flag + "some_sufix";
> > - 8 false positives.
> > - 84 possible issues. (interestingly 6 of these are from premature use of 
> > variations of "extern char* empty_string;"
> >
> >   The false positives fall into 3 categories:
> > - 3 variations of: ``` extern int i; static const int* pi = &i;  // diag ```
>
>
> Should we warn at all when only an address of a global variable is used?


We could, but it would allow stuff like:

  extern const int i;
  const char c = *(const char*)&i;

No strong opinion here.

> 

> 

> > // Then pi is dereferenced later, once i is intialized. 

> 

> >  Public example of this: 
> > https://github.com/python-git/python/blob/py3k/Objects/dictobject.c#L2027

> 

> > 

> 

> > 2. 3 variations of: ``` // .h class A { static const int i = 42; }; // .cc 
> > int A::i;  // diag ```

> 

> 

> Looks like we have all information to fix this kind of a false positive.

> 

> > 

> 

> > 

> 

> > 3. 2 variations of: ``` // .h class A { static int i; static int j; }; // 
> > .cc int A::i = 0; int A::j = i;  // diag ```

> 

> 

> ditto


It turns out (2) and (3) are just variations of the same - (2) was just:

  // .h
  class A {
    static const int i = 0;
    static const int j = i;
  };
  // .cc
  int A::i;
  int A::j;

Which has *exactly* the same semantics as (3).

I could not find a way to go from a decl to its (possible) definition. I was 
thinking of matching all global definitions that are not declarations and then 
making sure that the current matches are not referring to one of those 
definitions (in syntactic order because of [3.6.2.3]). Does this  sound like a 
reasonable idea ?


http://reviews.llvm.org/D18649



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

Reply via email to