Ignore lambdas and other anonymous symbols.

[GoogleCodeExporter]

The "use" of the call operator of anonymous symbols (e.g., lambdas) is 
recorded, though there can occasionally be contexts where we convert this full 
use into a forward declaration use. I haven't quite figured out a test case for 
why this happens yet, but suffice to say if a found symbol has no name, then it 
couldn't ever be forward declared, so just strip it out of the analysis.

A test case is probably required.

Original issue reported on code.google.com by SmSpil...@gmail.com on 6 Oct 2014 at 1:17

Attachments:

• ignore_anonymous_symbols.patch

[GoogleCodeExporter]

+  // (B1) Discard uses of symbols which have no name.
+  if (use->symbol_name().empty()) {
+     VERRS(6) << "Ignoring use of anonymous symbol at "
+              << use->PrintableUseLoc();
+     use->set_ignore_use();
+     return;
+  }

I don't think this will work -- ReportIncludeFileUse (which is used e.g. for 
pragma: export) records a use without a symbol.

I don't like this design as-is, but I doubt your patch will pass the current 
set of tests.

Original comment by kim.gras...@gmail.com on 6 Oct 2014 at 6:56

[GoogleCodeExporter]

It passes the current set of tests as is (asides from badinc, which is known to 
be broken on OS X).

Can we also add a DeclCanBeForwardDeclared () check as well? That should cause 
this not to operate on includes added to symbol_uses_  in ReportIncludeFileUse.

Unfortunately, I haven't been able to come up with a test case for this. The 
project I'm using this on is open source and I can post a somewhat trimmed down 
version of that in case you're interested in looking into this.

Original comment by SmSpil...@gmail.com on 7 Oct 2014 at 1:15

[GoogleCodeExporter]

Okay, so I've been able to come up with a somewhat reduced test case for this. 
It still involves the use of <functional>, which doesn't make debugging any 
easier, but its better than nothing I suppose.

This patch passes all the tests (except test_badinc, which is already broken on 
OS X).

Let me know if you need any further clarification. This is basically the last 
thing that needs to be fixed before we can enable IWYU in my project's build 
tooling by default :).

Original comment by SmSpil...@gmail.com on 15 Oct 2014 at 7:13

Attachments:

• ignore_anonymous_symbols.patch

[GoogleCodeExporter]

Nice, thanks! I'll take a look at your patches as soon as I'm able (might be a 
few days)

Original comment by kim.gras...@gmail.com on 15 Oct 2014 at 7:29

[GoogleCodeExporter]

Issue 160 has been merged into this issue.

Original comment by kim.gras...@gmail.com on 24 Oct 2014 at 4:54

[GoogleCodeExporter]

I'm looking at this now, and I have a smaller test case without std::function;

--
template<class F>
int eval(F f, int arg) {
  return f(arg);
}

int add(int value, int addend) {
  return eval([&](int v) { return v + addend; }, value);
}
--

However, I don't really understand why IWYU feels compelled to require a 
forward decl here... If I do this:

--
template<class F>
int eval(F f, int arg) {
  return f(arg);
}

int add_5(int value) {
  return value + 5;
}

int add(int value, int addend) {
  return eval(add_5, value);
}
--

it should require a forward decl for add_5 by the same reasoning, but it 
doesn't.

So I suspect the lambda throws IWYU off somewhere, and that in turn leads to 
the attempt to track the anonymous symbol.

I'll keep digging.

Original comment by kim.gras...@gmail.com on 24 Oct 2014 at 10:12

[GoogleCodeExporter]

Thank you so much, I'm planning on integrating include-what-you-use into my CI 
builds and this was the only (small) niggle getting in the way so far. I'll let 
you know the results once everything is all in place.

By the way - I noticed the other day that RecursiveASTVisitor has special 
methods to traverse lambdas 
(http://clang.llvm.org/doxygen/classclang_1_1RecursiveASTVisitor.html#ae03e9cdad
c00ef84802bd0386d8e8f18). Might be useful.

Original comment by SmSpil...@gmail.com on 25 Oct 2014 at 10:36

[GoogleCodeExporter]

Please try this patch, it's essentially a small change in 
DeclCanBeForwardDeclared that appears to solve this problem nicely.

This passes your test case with std::function as well, but it'd be nice to know 
if it fixes your original problem.

I wonder if maybe we should ignore lambdas when collecting declarations. Since 
they're always inline, they're always used, so it could be useless to analyze 
them. But I think this is a good first step that seems to solve your problem.

Original comment by kim.gras...@gmail.com on 26 Oct 2014 at 10:27

Attachments:

• issue161.diff

[GoogleCodeExporter]

Change looks good to me.

Original comment by vsap...@gmail.com on 29 Oct 2014 at 5:27

[GoogleCodeExporter]

Yup, that patch fixes my case. Thanks so much for looking into it!

I've found a few other interesting things in the meantime, so I might be 
posting some more patches (probably just to include maps).

Original comment by SmSpil...@gmail.com on 30 Oct 2014 at 2:51

[GoogleCodeExporter]

Thanks, r590!

Original comment by kim.gras...@gmail.com on 1 Nov 2014 at 8:23

• Changed state: Fixed