maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[clangd] IncludeCleaner: Do not require forward declarations of RecordDecls when definition is available 

This makes IncludeCleaner more useful in the presense of a large number of
forward declarations. If the definition is already in the Translation Unit and
visible to the Main File, forward declarations have no effect.

The original patch D112707 was split in two: D114864 and this one.

Reviewed By: kadircet

Differential Revision: https://reviews.llvm.org/D114949
This commit is contained in:
Kirill Bobyrev 2021-12-03 09:36:49 +01:00
parent 4a5086dce3
commit bab7a30ab6
2 changed files with 26 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
clang-tools-extra/clangd/IncludeCleaner.cpp
View File

@ -122,16 +122,20 @@ private:
void add(const Decl *D) {
if (!D || !isNew(D->getCanonicalDecl()))
return;
// If we see a declaration in the mainfile, skip all non-definition decls.
// We only do this for classes because forward declarations are common and
// we don't want to include every header that forward-declares the symbol
// because they're often unrelated.
// Special case RecordDecls, as it is common for them to be forward
// declared multiple times. The most common cases are:
// - Definition available in TU, only mark that one as usage. The rest is
// likely to be unnecessary. This might result in false positives when an
// internal definition is visible.
// - There's a forward declaration in the main file, no need for other
// redecls.
if (const auto *RD = llvm::dyn_cast<RecordDecl>(D)) {
if (SM.isInMainFile(RD->getMostRecentDecl()->getBeginLoc())) {
if (const auto *Definition = RD->getMostRecentDecl()->getDefinition())
Result.insert(Definition->getLocation());
if (const auto *Definition = RD->getDefinition()) {
Result.insert(Definition->getLocation());
return;
}
if (SM.isInMainFile(RD->getMostRecentDecl()->getLocation()))
return;
}
for (const Decl *Redecl : D->redecls())
Result.insert(Redecl->getLocation());

23
clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
View File

@ -39,10 +39,10 @@ TEST(IncludeCleaner, ReferencedLocations) {
"class ^X;",
"X *y;",
},
// FIXME(kirillbobyrev): When definition is available, we don't need to
// mark forward declarations as used.
// When definition is available, we don't need to mark forward
// declarations as used.
{
"class ^X {}; class ^X;",
"class ^X {}; class X;",
"X *y;",
},
// We already have forward declaration in the main file, the other
@ -51,17 +51,24 @@ TEST(IncludeCleaner, ReferencedLocations) {
"class ^X {}; class X;",
"class X; X *y;",
},
// Nested class definition can occur outside of the parent class
// definition. Bar declaration should be visible to its definition but
// it will always be because we will mark Foo definition as used.
{
"class ^Foo { class Bar; };",
"class Foo::Bar {};",
},
// TypedefType and UsingDecls
{
"using ^Integer = int;",
"Integer x;",
},
{
"namespace ns { struct ^X; struct ^X {}; }",
"using ns::X;",
"namespace ns { void ^foo(); void ^foo() {} }",
"using ns::foo;",
},
{
"namespace ns { struct X; struct X {}; }",
"namespace ns { void foo(); void foo() {}; }",
"using namespace ns;",
},
{
@ -88,8 +95,8 @@ TEST(IncludeCleaner, ReferencedLocations) {
},
// Redecls
{
"class ^X; class ^X{}; class ^X;",
"X *y;",
"void ^foo(); void ^foo() {} void ^foo();",
"void bar() { foo(); }",
},
// Constructor
{