summaryrefslogtreecommitdiff
path: root/compilerplugins
diff options
context:
space:
mode:
authorStephan Bergmann <sbergman@redhat.com>2019-11-16 21:00:53 +0100
committerStephan Bergmann <sbergman@redhat.com>2019-11-17 00:28:17 +0100
commit314f15bff08b76bf96acf99141776ef64d2f1355 (patch)
tree842f7b109d9c4a57fa47fc5089f5818b2610368b /compilerplugins
parent46920005f74edcb70acfb8dd1a0ffb9553e5c2b2 (diff)
Extend loplugin:external to warn about enums
To mitigate the dangers of silently breaking ADL when moving enums into unnamed namespaces (see the commit message of 206b5b2661be37efdff3c6aedb6f248c4636be79 "New loplugin:external"), note all functions that are affected. (The plan is to extend loplugin:external further to also warn about classes and class templates, and the code to identify affected functions already takes that into account, so some parts of that code are not actually relevant for enums.) But it appears that none of the functions that are actually affected by the changes in this commit relied on being found through ADL, so no adaptions were necessary for them. (clang::DeclContext::collectAllContexts is non-const, which recursively means that External's Visit... functions must take non-const Decl*. Which required compilerplugins/clang/sharedvisitor/analyzer.cxx to be generalized to support such Visit... functions with non-const Decl* parameters.) Change-Id: Ia215291402bf850d43defdab3cff4db5b270d1bd Reviewed-on: https://gerrit.libreoffice.org/83001 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
Diffstat (limited to 'compilerplugins')
-rw-r--r--compilerplugins/clang/external.cxx239
-rw-r--r--compilerplugins/clang/sharedvisitor/analyzer.cxx25
-rw-r--r--compilerplugins/clang/test/external.cxx61
3 files changed, 316 insertions, 9 deletions
diff --git a/compilerplugins/clang/external.cxx b/compilerplugins/clang/external.cxx
index 2b6b689ea315..e4a6b0df6e42 100644
--- a/compilerplugins/clang/external.cxx
+++ b/compilerplugins/clang/external.cxx
@@ -10,6 +10,9 @@
#include <algorithm>
#include <cassert>
+#include <iterator>
+#include <list>
+#include <set>
#include "clang/Sema/SemaDiagnostic.h"
@@ -45,6 +48,79 @@ bool isInjectedFunction(FunctionDecl const* decl)
return true;
}
+// Whether type1 mentions type2 (in a way relevant for argument-dependent name lookup):
+bool mentions(QualType type1, QualType type2)
+{
+ auto t1 = type1;
+ for (;;)
+ {
+ if (auto const t2 = t1->getAs<ReferenceType>())
+ {
+ t1 = t2->getPointeeType();
+ }
+ else if (auto const t3 = t1->getAs<PointerType>())
+ {
+ t1 = t3->getPointeeType();
+ }
+ else if (auto const t4 = t1->getAsArrayTypeUnsafe())
+ {
+ t1 = t4->getElementType();
+ }
+ else
+ {
+ break;
+ }
+ }
+ if (t1.getCanonicalType().getTypePtr() == type2.getTypePtr())
+ {
+ return true;
+ }
+ if (auto const t2 = t1->getAs<TemplateSpecializationType>())
+ {
+ for (auto a = t2->begin(); a != t2->end(); ++a)
+ {
+ if (a->getKind() != TemplateArgument::Type)
+ {
+ continue;
+ }
+ if (mentions(a->getAsType(), type2))
+ {
+ return true;
+ }
+ }
+ auto const t3 = t2->desugar();
+ if (t3.getTypePtr() == t2)
+ {
+ return false;
+ }
+ return mentions(t3, type2);
+ }
+ if (auto const t2 = t1->getAs<FunctionProtoType>())
+ {
+ if (mentions(t2->getReturnType(), type2))
+ {
+ return true;
+ }
+ for (auto t3 = t2->param_type_begin(); t3 != t2->param_type_end(); ++t3)
+ {
+ if (mentions(*t3, type2))
+ {
+ return true;
+ }
+ }
+ return false;
+ }
+ if (auto const t2 = t1->getAs<MemberPointerType>())
+ {
+ if (t2->getClass()->getUnqualifiedDesugaredType() == type2.getTypePtr())
+ {
+ return true;
+ }
+ return mentions(t2->getPointeeType(), type2);
+ }
+ return false;
+}
+
class External : public loplugin::FilteringPlugin<External>
{
public:
@@ -55,10 +131,11 @@ public:
void run() override { TraverseDecl(compiler.getASTContext().getTranslationUnitDecl()); }
- bool VisitTagDecl(TagDecl const* decl)
+ bool VisitTagDecl(TagDecl* decl)
{
/*TODO:*/
- return true; // in general, moving classes or enumerations into an unnamed namespace can break ADL
+ if (!isa<EnumDecl>(decl))
+ return true; // in general, moving classes into an unnamed namespace can break ADL
if (isa<ClassTemplateSpecializationDecl>(decl))
{
return true;
@@ -102,7 +179,7 @@ public:
return handleDeclaration(decl);
}
- bool VisitFunctionDecl(FunctionDecl const* decl)
+ bool VisitFunctionDecl(FunctionDecl* decl)
{
if (isa<CXXMethodDecl>(decl))
{
@@ -166,7 +243,7 @@ public:
return handleDeclaration(decl);
}
- bool VisitVarDecl(VarDecl const* decl)
+ bool VisitVarDecl(VarDecl* decl)
{
if (decl->isStaticDataMember())
{
@@ -187,7 +264,7 @@ public:
return handleDeclaration(decl);
}
- bool VisitClassTemplateDecl(ClassTemplateDecl const* decl)
+ bool VisitClassTemplateDecl(ClassTemplateDecl* decl)
{
/*TODO:*/
return true; // in general, moving classes or enumerations into an unnamed namespace can break ADL
@@ -202,7 +279,7 @@ public:
return handleDeclaration(decl);
}
- bool VisitFunctionTemplateDecl(FunctionTemplateDecl const* decl)
+ bool VisitFunctionTemplateDecl(FunctionTemplateDecl* decl)
{
if (!decl->isThisDeclarationADefinition())
{
@@ -219,7 +296,7 @@ public:
return handleDeclaration(decl);
}
- bool VisitVarTemplateDecl(VarTemplateDecl const* decl)
+ bool VisitVarTemplateDecl(VarTemplateDecl* decl)
{
if (!decl->isThisDeclarationADefinition())
{
@@ -243,7 +320,149 @@ private:
}
}
- bool handleDeclaration(NamedDecl const* decl)
+ void computeAffectedTypes(Decl const* decl, std::vector<QualType>* affected)
+ {
+ assert(affected != nullptr);
+ if (auto const d = dyn_cast<EnumDecl>(decl))
+ {
+ affected->push_back(compiler.getASTContext().getEnumType(d));
+ }
+ else
+ {
+ CXXRecordDecl const* rec;
+ if (auto const d = dyn_cast<ClassTemplateDecl>(decl))
+ {
+ rec = d->getTemplatedDecl();
+ }
+ else
+ {
+ rec = cast<CXXRecordDecl>(decl);
+ }
+ affected->push_back(compiler.getASTContext().getRecordType(rec));
+ for (auto d = rec->decls_begin(); d != rec->decls_end(); ++d)
+ {
+ if (*d != (*d)->getCanonicalDecl())
+ {
+ continue;
+ }
+ if (isa<TagDecl>(*d) || isa<ClassTemplateDecl>(*d))
+ {
+ if (auto const d1 = dyn_cast<RecordDecl>(*d))
+ {
+ if (d1->isInjectedClassName())
+ {
+ continue;
+ }
+ }
+ computeAffectedTypes(*d, affected);
+ }
+ }
+ }
+ }
+
+ void reportAssociatingFunctions(std::vector<QualType> const& affected, Decl* decl)
+ {
+ auto c = decl->getDeclContext();
+ while (isa<LinkageSpecDecl>(c) || c->isInlineNamespace())
+ {
+ c = c->getParent();
+ }
+ assert(c->isTranslationUnit() || c->isNamespace());
+ SmallVector<DeclContext*, 2> parts;
+ c->collectAllContexts(parts);
+ std::list<DeclContext const*> ctxs;
+ std::copy(parts.begin(), parts.end(),
+ std::back_insert_iterator<std::list<DeclContext const*>>(ctxs));
+ if (auto const d = dyn_cast<CXXRecordDecl>(decl))
+ {
+ // To find friend functions declared in the class:
+ ctxs.push_back(d);
+ }
+ std::set<FunctionDecl const*> fdecls; // to report every function just once
+ for (auto ctx = ctxs.begin(); ctx != ctxs.end(); ++ctx)
+ {
+ for (auto i = (*ctx)->decls_begin(); i != (*ctx)->decls_end(); ++i)
+ {
+ auto d = *i;
+ if (auto const d1 = dyn_cast<LinkageSpecDecl>(d))
+ {
+ ctxs.push_back(d1);
+ continue;
+ }
+ if (auto const d1 = dyn_cast<NamespaceDecl>(d))
+ {
+ if (d1->isInline())
+ {
+ ctxs.push_back(d1);
+ }
+ continue;
+ }
+ if (auto const d1 = dyn_cast<FriendDecl>(d))
+ {
+ d = d1->getFriendDecl();
+ }
+ FunctionDecl const* f;
+ if (auto const d1 = dyn_cast<FunctionTemplateDecl>(d))
+ {
+ f = d1->getTemplatedDecl();
+ }
+ else
+ {
+ f = dyn_cast<FunctionDecl>(d);
+ if (f == nullptr)
+ {
+ continue;
+ }
+ }
+ if (!fdecls.insert(f->getCanonicalDecl()).second)
+ {
+ continue;
+ }
+ if (isa<CXXMethodDecl>(f))
+ {
+ continue;
+ }
+ for (auto const t : affected)
+ {
+ auto const tc = t.getCanonicalType();
+ for (auto p = f->param_begin(); p != f->param_end(); ++p)
+ {
+ if (mentions((*p)->getType(), tc))
+ {
+ report(DiagnosticsEngine::Note,
+ "a %select{function|function template|function template "
+ "specialization}0 associating %1 is declared here",
+ f->getLocation())
+ << (f->isFunctionTemplateSpecialization()
+ ? 2
+ : f->getDescribedFunctionTemplate() != nullptr ? 1 : 0)
+ << t << f->getSourceRange();
+ for (auto f1 = f->redecls_begin(); f1 != f->redecls_end(); ++f1)
+ {
+ if (*f1 == f)
+ {
+ continue;
+ }
+ report(DiagnosticsEngine::Note, "another declaration is here",
+ f1->getLocation())
+ << f1->getSourceRange();
+ }
+ break;
+ }
+ }
+ }
+ }
+ }
+ }
+
+ void reportAssociatingFunctions(Decl* decl)
+ {
+ std::vector<QualType> affected; // enum/class/class template + recursively affected members
+ computeAffectedTypes(decl, &affected);
+ reportAssociatingFunctions(affected, decl);
+ }
+
+ bool handleDeclaration(NamedDecl* decl)
{
if (ignoreLocation(decl))
{
@@ -364,6 +583,10 @@ private:
{
reportSpecializations(d->specializations());
}
+ if (isa<TagDecl>(decl) || isa<ClassTemplateDecl>(decl))
+ {
+ reportAssociatingFunctions(decl);
+ }
return true;
}
};
diff --git a/compilerplugins/clang/sharedvisitor/analyzer.cxx b/compilerplugins/clang/sharedvisitor/analyzer.cxx
index d0f5d4e7a8a7..745064647c81 100644
--- a/compilerplugins/clang/sharedvisitor/analyzer.cxx
+++ b/compilerplugins/clang/sharedvisitor/analyzer.cxx
@@ -14,6 +14,7 @@
#include "clang/Tooling/Tooling.h"
#include "llvm/ADT/StringExtras.h"
+#include <cassert>
#include <cstddef>
#include <cstring>
#include <iostream>
@@ -55,6 +56,8 @@ class CheckFileVisitor
: public RecursiveASTVisitor< CheckFileVisitor >
{
public:
+ void setContext(ASTContext const& context) { context_ = &context; }
+
bool VisitCXXRecordDecl(CXXRecordDecl *Declaration);
bool TraverseNamespaceDecl(NamespaceDecl * decl)
@@ -67,6 +70,21 @@ public:
}
return RecursiveASTVisitor<CheckFileVisitor>::TraverseNamespaceDecl(decl);
}
+
+private:
+ ASTContext const* context_ = nullptr;
+
+ QualType unqualifyPointeeType(QualType type)
+ {
+ assert(context_ != nullptr);
+ if (auto const t = type->getAs<PointerType>())
+ {
+ return context_->getQualifiedType(
+ context_->getPointerType(t->getPointeeType().getUnqualifiedType()),
+ type.getQualifiers());
+ }
+ return type;
+ }
};
static bool inheritsPluginClassCheck( const Decl* decl )
@@ -116,7 +134,8 @@ bool CheckFileVisitor::VisitCXXRecordDecl( CXXRecordDecl* decl )
cout << "VisitFunctionStart" << endl;
cout << "VisitFunctionName:" << method->getName().str() << endl;
cout << "VisitFunctionArgument:"
- << method->getParamDecl( 0 )->getTypeSourceInfo()->getType().getAsString()
+ << unqualifyPointeeType(
+ method->getParamDecl( 0 )->getTypeSourceInfo()->getType()).getAsString()
<< endl;
cout << "VisitFunctionEnd" << endl;
}
@@ -185,6 +204,10 @@ class FindNamedClassConsumer
: public ASTConsumer
{
public:
+ void Initialize(ASTContext& context) override
+ {
+ visitor.setContext(context);
+ }
virtual void HandleTranslationUnit(ASTContext& context) override
{
visitor.TraverseDecl( context.getTranslationUnitDecl());
diff --git a/compilerplugins/clang/test/external.cxx b/compilerplugins/clang/test/external.cxx
index ff996f54a467..28b7c6df01b7 100644
--- a/compilerplugins/clang/test/external.cxx
+++ b/compilerplugins/clang/test/external.cxx
@@ -7,6 +7,10 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/
+#include <sal/config.h>
+
+#include <vector>
+
// expected-error@+1 {{externally available entity 'n1' is not previously declared in an included file (if it is only used in this translation unit, make it static or put it in an unnamed namespace; otherwise, provide a declaration of it in an included file) [loplugin:external]}}
int n1 = 0;
// expected-note@+1 {{another declaration is here [loplugin:external]}}
@@ -42,11 +46,68 @@ static void g()
// expected-note@+1 {{another declaration is here [loplugin:external]}}
void f2();
+namespace N
+{
+inline namespace I1
+{
+extern "C++" {
+// expected-note@+1 {{another declaration is here [loplugin:external]}}
+enum E : int;
+
+// expected-error@+1 {{externally available entity 'E' is not previously declared in an included file (if it is only used in this translation unit, put it in an unnamed namespace; otherwise, provide a declaration of it in an included file) [loplugin:external]}}
+enum E : int
+{
+};
+}
+}
+
+// expected-note@+1 {{a function associating 'N::I1::E' is declared here [loplugin:external]}}
+static void g(std::vector<E>)
+{
+ // expected-note@+1 {{another declaration is here [loplugin:external]}}
+ void f(E const*);
+}
+
+// expected-note@+1 {{a function associating 'N::I1::E' is declared here [loplugin:external]}}
+void f(E const*);
+
+extern "C++" {
+// expected-note@+1 {{a function associating 'N::I1::E' is declared here [loplugin:external]}}
+void fc(E const*);
+}
+
+struct S1
+{
+ struct S2;
+ // No note about associating function; injected friend function not found by ADL:
+ friend void f2(E const*);
+};
+
+inline namespace I2
+{
+// expected-note@+1 {{a function associating 'N::I1::E' is declared here [loplugin:external]}}
+void f3(E);
+
+inline namespace I3
+{
+// expected-note@+1 {{a function associating 'N::I1::E' is declared here [loplugin:external]}}
+void f4(E);
+}
+}
+}
+
+struct N::S1::S2
+{
+ // expected-note@+1 {{another declaration is here [loplugin:external]}}
+ friend void f(E const*);
+};
+
int main()
{
(void)n2;
(void)n3;
g();
+ (void)&N::g;
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */