jpakkane marked an inline comment as done.
jpakkane added inline comments.

================
Comment at: clang-tools-extra/clang-tidy/misc/InitLocalVariablesCheck.cpp:21
+  Finder->addMatcher(
+      varDecl(unless(hasInitializer(anything()))).bind("vardecl"), this);
+}
----------------
alexfh wrote:
> jpakkane wrote:
> > alexfh wrote:
> > > jpakkane wrote:
> > > > alexfh wrote:
> > > > > I believe, this should skip matches within template instantiations. 
> > > > > Consider this code:
> > > > > ```
> > > > > template<typename T>
> > > > > void f(T) { T t; }
> > > > > void g() {
> > > > >     f(0);
> > > > >     f(0.0);
> > > > > }
> > > > > ```
> > > > > 
> > > > > What will the fix  be?
> > > > I tested with the following function:
> > > > 
> > > > 
> > > > ```
> > > > template<typename T>
> > > > void template_test_function() {
> > > >   T t;
> > > >   int uninitialized;
> > > > }
> > > > ```
> > > > 
> > > > Currently it warns on the "uninitialized" variable regardless of 
> > > > whether the template is instantiated or not. If you call it with an int 
> > > > type, it will warn about variable t being uninitialized. If you call it 
> > > > with a, say, struct type, there is no warnings. Is this a reasonable 
> > > > approach?
> > > And what happens, if there are multiple instantiations of the same 
> > > template, each of them requiring a different fix? Can you try the check 
> > > with my example above (and maybe also add `f("");`inside `g()`). I 
> > > believe, the check will produce multiple warnings with conflicting fixes 
> > > (and each of them will be wrong, btw).
> > Interestingly it does warn about it, but only once, even if you have two 
> > different template specializations.
> > 
> > I tried to suppress this warning when the type being instantiated is a 
> > template argument type but no matter what I tried I could not get this to 
> > work. Is there a way to get this information from the MatchedDecl object or 
> > does one need to do something more complicated like going up the AST until 
> > a function definition is found and checking if it is a template 
> > specialization (presumably with TemplatedKind)? Any help would be 
> > appreciated.
> If there are multiple warnings with the same message at the same location 
> (clang-tidy/ClangTidyDiagnosticConsumer.cpp:745), they will be deduplicated. 
> Thus, a random fix will probably be suggested. The proper way to filter out 
> matches in template instantiations is to add 
> `unless(isInTemplateInstantiation())` to the matcher.
I tried to make this work but I just could not combine statement and 
declaration matching in a reliable way. Matching a statement that is not in a 
template declaration can be done, as well as matching a declaration without 
intial value, but combining those two into one is hard. After trying many, many 
things the best I could come up with was this:

```
declStmt(containsDeclaration(0, 
varDecl(unless(hasInitializer(anything()))).bind("vardecl"))), this)
```

The problem is that `containsDeclaration` takes an integer denoting how manyth 
declaration should be processed. Manually adding matchers for, say, 0, 1, 2, 3 
and 4 works and does the right thing but fails if anyone has an uninitialized 
variable in the sixth location, things will silently fail.

The weird thing is that if you do the matching this way, you don't need to 
filter out things with `unless(isInTemplateInstantiation())`. Maybe statements 
are handled differently from declarations?


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64671/new/

https://reviews.llvm.org/D64671



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

Reply via email to