llvm / llvm/llvm-project

clang-include-cleaner: function template used via a using-declaration — header removed but not re-added

Open
#206,635 1 comment 0 reactions 0 assignees View on GitHub
clang-include-cleaner
Dominant language
LLVM
Stars
40.5k
Forks
18.7k
PR merge metrics
PR metrics pending

Description

When a function template is brought into scope with a using-declaration and then called, clang-include-cleaner tells me to remove the header the template actually comes from, without suggesting the header I should add instead. Following the suggestion breaks the build.

It seems specific to function templates. A plain (non-template) function, a class template, and a variable template in the same situation all behave correctly.

```cpp
// a.h
namespace ns { template void foo(T) {} }

// b.h
#include "a.h"

// main.cc
#include "b.h"
using ::ns::foo;

void g() { foo(0); }
```

Running `clang-include-cleaner --print=changes main.cc -- -std=c++20`, we get

**Actual**: `- "b.h"`
**Expected:** `- "b.h"` and `+ "a.h"`

By contrast, if we swap the template for `void foo(int){}` and call `foo(0)`, include-cleaner correctly reports `- "b.h" / + "a.h"`.

I'm not familiar with this codebase, but with the help of Claude, I poked around the implementation.

The decision seems to be made in `ASTWalker::VisitUsingDecl`: https://github.com/llvm/llvm-project/blob/df108f91d5b851e4a7012579d0fbb1c3fed8a995/clang-tools-extra/include-cleaner/lib/WalkAST.cpp#L236-L238

For functions, a using-decl's reference to the target is only treated as an explicit use if the target looks used; otherwise it's downgraded to Ambiguous. The problem is that for a function template, calling `foo(0)` marks the *implicitly-instantiated specialization* as used, not the primary FunctionTemplateDecl that the using-decl points at — so isUsed()/isReferenced() on TD are both false, and the reference is wrongly marked Ambiguous.

One possible fix is to also consult the function template's specializations when deciding whether it's used:

```cpp
auto IsUsed = TD->isUsed() || TD->isReferenced() || !TD->getAsFunction();
// A call to a function template named through the using-decl marks the
// (implicitly instantiated) specialization as used, not the primary
// FunctionTemplateDecl. Consult the specializations so a used function
// template isn't mistaken for an unused transitively-visible overload.
if (!IsUsed)
if (const auto *FTD = llvm::dyn_cast(TD))
IsUsed = llvm::any_of(FTD->specializations(), [](const auto *Spec) {
return Spec->isUsed() || Spec->isReferenced();
});
```

Tests seems to pass, and this does the right thing on my case. I would be glad to open a PR with this plus a regression test if it looks like the right direction.

Contributor guide

Open the contributing guide

Research direction

Reproduce the issue with clang-include-cleaner --print=changes main.cc -- -std=c++20, then inspect ASTWalker::VisitUsingDecl in clang-tools-extra/include-cleaner/lib/WalkAST.cpp. Add a regression test covering a function template brought into scope by a using-declaration, and verify the output removes b.h and adds a.h.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
compilers
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.