AcoSail/llvm-project
8
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

inline stmt attribute diagnosing in templates

Browse Source 
D146089's author discovered that our diagnostics for always/no inline
would null-dereference when used in a template. He fixed that by
skipping in the dependent case.

This patch makes sure we diagnose these after a template instantiation.
It also adds infrastructure for other statement attributes to add
checking/transformation.

Differential Revision: https://reviews.llvm.org/D146323
This commit is contained in:
Erich Keane 2023-03-17 09:54:39 -07:00
parent b33f5e7ed3
commit 514e4359a5
7 changed files with 220 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
clang/docs/ReleaseNotes.rst
View File

@ -181,6 +181,8 @@ Improvements to Clang's diagnostics
- Clang now avoids duplicate warnings on unreachable ``[[fallthrough]];`` statements
previously issued from ``-Wunreachable-code`` and ``-Wunreachable-code-fallthrough``
by prioritizing ``-Wunreachable-code-fallthrough``.
- Clang now correctly diagnoses statement attributes ``[[clang::always_inine]]`` and
``[[clang::noinline]]`` when used on a statement with dependent call expressions.
Bug Fixes in This Version
-------------------------

5
clang/include/clang/Sema/Sema.h
View File

@ -4715,6 +4715,11 @@ public:
void CheckAlignasUnderalignment(Decl *D);
bool CheckNoInlineAttr(const Stmt *OrigSt, const Stmt *CurSt,
const AttributeCommonInfo &A);
bool CheckAlwaysInlineAttr(const Stmt *OrigSt, const Stmt *CurSt,
const AttributeCommonInfo &A);
/// Adjust the calling convention of a method to be the ABI default if it
/// wasn't specified explicitly. This handles method types formed from
/// function type typedefs and typename template arguments.

80
clang/lib/Sema/SemaStmtAttr.cpp
View File

@ -215,6 +215,59 @@ static Attr *handleNoMergeAttr(Sema &S, Stmt *St, const ParsedAttr &A,
return ::new (S.Context) NoMergeAttr(S.Context, A);
}
template <typename OtherAttr, int DiagIdx>
static bool CheckStmtInlineAttr(Sema &SemaRef, const Stmt *OrigSt,
const Stmt *CurSt,
const AttributeCommonInfo &A) {
CallExprFinder OrigCEF(SemaRef, OrigSt);
CallExprFinder CEF(SemaRef, CurSt);
// If the call expressions lists are equal in size, we can skip
// previously emitted diagnostics. However, if the statement has a pack
// expansion, we have no way of telling which CallExpr is the instantiated
// version of the other. In this case, we will end up re-diagnosing in the
// instantiation.
// ie: [[clang::always_inline]] non_dependent(), (other_call<Pack>()...)
// will diagnose nondependent again.
bool CanSuppressDiag =
OrigSt && CEF.getCallExprs().size() == OrigCEF.getCallExprs().size();
if (!CEF.foundCallExpr()) {
return SemaRef.Diag(CurSt->getBeginLoc(),
diag::warn_attribute_ignored_no_calls_in_stmt)
<< A;
}
for (auto Tup :
llvm::zip_longest(OrigCEF.getCallExprs(), CEF.getCallExprs())) {
// If the original call expression already had a callee, we already
// diagnosed this, so skip it here. We can't skip if there isn't a 1:1
// relationship between the two lists of call expressions.
if (!CanSuppressDiag || !(*std::get<0>(Tup))->getCalleeDecl()) {
const Decl *Callee = (*std::get<1>(Tup))->getCalleeDecl();
if (Callee &&
(Callee->hasAttr<OtherAttr>() || Callee->hasAttr<FlattenAttr>())) {
SemaRef.Diag(CurSt->getBeginLoc(),
diag::warn_function_stmt_attribute_precedence)
<< A << (Callee->hasAttr<OtherAttr>() ? DiagIdx : 1);
SemaRef.Diag(Callee->getBeginLoc(), diag::note_conflicting_attribute);
}
}
}
return false;
}
bool Sema::CheckNoInlineAttr(const Stmt *OrigSt, const Stmt *CurSt,
const AttributeCommonInfo &A) {
return CheckStmtInlineAttr<AlwaysInlineAttr, 0>(*this, OrigSt, CurSt, A);
}
bool Sema::CheckAlwaysInlineAttr(const Stmt *OrigSt, const Stmt *CurSt,
const AttributeCommonInfo &A) {
return CheckStmtInlineAttr<NoInlineAttr, 2>(*this, OrigSt, CurSt, A);
}
static Attr *handleNoInlineAttr(Sema &S, Stmt *St, const ParsedAttr &A,
SourceRange Range) {
NoInlineAttr NIA(S.Context, A);
@ -224,20 +277,8 @@ static Attr *handleNoInlineAttr(Sema &S, Stmt *St, const ParsedAttr &A,
return nullptr;
}
CallExprFinder CEF(S, St);
if (!CEF.foundCallExpr()) {
S.Diag(St->getBeginLoc(), diag::warn_attribute_ignored_no_calls_in_stmt)
<< A;
if (S.CheckNoInlineAttr(/*OrigSt=*/nullptr, St, A))
return nullptr;
}
for (const auto *CallExpr : CEF.getCallExprs()) {
const Decl *Decl = CallExpr->getCalleeDecl();
if (Decl &&
(Decl->hasAttr<AlwaysInlineAttr>() || Decl->hasAttr<FlattenAttr>()))
S.Diag(St->getBeginLoc(), diag::warn_function_stmt_attribute_precedence)
<< A << (Decl->hasAttr<AlwaysInlineAttr>() ? 0 : 1);
}
return ::new (S.Context) NoInlineAttr(S.Context, A);
}
@ -251,19 +292,8 @@ static Attr *handleAlwaysInlineAttr(Sema &S, Stmt *St, const ParsedAttr &A,
return nullptr;
}
CallExprFinder CEF(S, St);
if (!CEF.foundCallExpr()) {
S.Diag(St->getBeginLoc(), diag::warn_attribute_ignored_no_calls_in_stmt)
<< A;
if (S.CheckAlwaysInlineAttr(/*OrigSt=*/nullptr, St, A))
return nullptr;
}
for (const auto *CallExpr : CEF.getCallExprs()) {
const Decl *Decl = CallExpr->getCalleeDecl();
if (Decl && (Decl->hasAttr<NoInlineAttr>() || Decl->hasAttr<FlattenAttr>()))
S.Diag(St->getBeginLoc(), diag::warn_function_stmt_attribute_precedence)
<< A << (Decl->hasAttr<NoInlineAttr>() ? 2 : 1);
}
return ::new (S.Context) AlwaysInlineAttr(S.Context, A);
}

20
clang/lib/Sema/SemaTemplateInstantiate.cpp
View File

@ -1272,6 +1272,12 @@ namespace {
bool AllowInjectedClassName = false);
const LoopHintAttr *TransformLoopHintAttr(const LoopHintAttr *LH);
const NoInlineAttr *TransformStmtNoInlineAttr(const Stmt *OrigS,
const Stmt *InstS,
const NoInlineAttr *A);
const AlwaysInlineAttr *
TransformStmtAlwaysInlineAttr(const Stmt *OrigS, const Stmt *InstS,
const AlwaysInlineAttr *A);
ExprResult TransformPredefinedExpr(PredefinedExpr *E);
ExprResult TransformDeclRefExpr(DeclRefExpr *E);
@ -1767,6 +1773,20 @@ TemplateInstantiator::TransformLoopHintAttr(const LoopHintAttr *LH) {
return LoopHintAttr::CreateImplicit(getSema().Context, LH->getOption(),
LH->getState(), TransformedExpr, *LH);
}
const NoInlineAttr *TemplateInstantiator::TransformStmtNoInlineAttr(
const Stmt *OrigS, const Stmt *InstS, const NoInlineAttr *A) {
if (!A || getSema().CheckNoInlineAttr(OrigS, InstS, *A))
return nullptr;
return A;
}
const AlwaysInlineAttr *TemplateInstantiator::TransformStmtAlwaysInlineAttr(
const Stmt *OrigS, const Stmt *InstS, const AlwaysInlineAttr *A) {
if (!A || getSema().CheckAlwaysInlineAttr(OrigS, InstS, *A))
return nullptr;
return A;
}
ExprResult TemplateInstantiator::transformNonTypeTemplateParmRef(
Decl *AssociatedDecl, const NonTypeTemplateParmDecl *parm,

74
clang/lib/Sema/TreeTransform.h
View File

@ -377,22 +377,43 @@ public:
/// By default, this routine transforms a statement by delegating to the
/// appropriate TransformXXXAttr function to transform a specific kind
/// of attribute. Subclasses may override this function to transform
/// attributed statements using some other mechanism.
/// attributed statements/types using some other mechanism.
///
/// \returns the transformed attribute
const Attr *TransformAttr(const Attr *S);
/// Transform the specified attribute.
///
/// Subclasses should override the transformation of attributes with a pragma
/// spelling to transform expressions stored within the attribute.
///
/// \returns the transformed attribute.
#define ATTR(X)
#define PRAGMA_SPELLING_ATTR(X) \
// Transform the given statement attribute.
//
// Delegates to the appropriate TransformXXXAttr function to transform a
// specific kind of statement attribute. Unlike the non-statement taking
// version of this, this implements all attributes, not just pragmas.
const Attr *TransformStmtAttr(const Stmt *OrigS, const Stmt *InstS,
const Attr *A);
// Transform the specified attribute.
//
// Subclasses should override the transformation of attributes with a pragma
// spelling to transform expressions stored within the attribute.
//
// \returns the transformed attribute.
#define ATTR(X) \
const X##Attr *Transform##X##Attr(const X##Attr *R) { return R; }
#include "clang/Basic/AttrList.inc"
// Transform the specified attribute.
//
// Subclasses should override the transformation of attributes to do
// transformation and checking of statement attributes. By default, this
// delegates to the non-statement taking version.
//
// \returns the transformed attribute.
#define ATTR(X) \
const X##Attr *TransformStmt##X##Attr(const Stmt *, const Stmt *, \
const X##Attr *A) { \
return getDerived().Transform##X##Attr(A); \
}
#include "clang/Basic/AttrList.inc"
/// Transform the given expression.
///
/// By default, this routine transforms an expression by delegating to the
@ -7551,9 +7572,8 @@ const Attr *TreeTransform<Derived>::TransformAttr(const Attr *R) {
return R;
switch (R->getKind()) {
// Transform attributes with a pragma spelling by calling TransformXXXAttr.
#define ATTR(X)
#define PRAGMA_SPELLING_ATTR(X) \
// Transform attributes by calling TransformXXXAttr.
#define ATTR(X) \
case attr::X: \
return getDerived().Transform##X##Attr(cast<X##Attr>(R));
#include "clang/Basic/AttrList.inc"
@ -7562,25 +7582,45 @@ const Attr *TreeTransform<Derived>::TransformAttr(const Attr *R) {
}
}
template <typename Derived>
const Attr *TreeTransform<Derived>::TransformStmtAttr(const Stmt *OrigS,
const Stmt *InstS,
const Attr *R) {
if (!R)
return R;
switch (R->getKind()) {
// Transform attributes by calling TransformStmtXXXAttr.
#define ATTR(X) \
case attr::X: \
return getDerived().TransformStmt##X##Attr(OrigS, InstS, cast<X##Attr>(R));
#include "clang/Basic/AttrList.inc"
default:
return R;
}
return TransformAttr(R);
}
template <typename Derived>
StmtResult
TreeTransform<Derived>::TransformAttributedStmt(AttributedStmt *S,
StmtDiscardKind SDK) {
StmtResult SubStmt = getDerived().TransformStmt(S->getSubStmt(), SDK);
if (SubStmt.isInvalid())
return StmtError();
bool AttrsChanged = false;
SmallVector<const Attr *, 1> Attrs;
// Visit attributes and keep track if any are transformed.
for (const auto *I : S->getAttrs()) {
const Attr *R = getDerived().TransformAttr(I);
const Attr *R =
getDerived().TransformStmtAttr(S->getSubStmt(), SubStmt.get(), I);
AttrsChanged |= (I != R);
if (R)
Attrs.push_back(R);
}
StmtResult SubStmt = getDerived().TransformStmt(S->getSubStmt(), SDK);
if (SubStmt.isInvalid())
return StmtError();
if (SubStmt.get() == S->getSubStmt() && !AttrsChanged)
return S;

46
clang/test/Sema/attr-alwaysinline.cpp
View File

@ -3,8 +3,9 @@
int bar();
[[gnu::always_inline]] void always_inline_fn(void) {}
// expected-note@+1{{conflicting attribute is here}}
[[gnu::flatten]] void flatten_fn(void) {}
// expected-note@+1{{conflicting attribute is here}}
[[gnu::noinline]] void noinline_fn(void) {}
void foo() {
@ -32,9 +33,44 @@ int foo(int x) {
[[clang::always_inline]] return foo<D-1>(x + 1);
}
// FIXME: This should warn that always_inline statement attribute has higher
// precedence than the noinline function attribute.
template<int D>
[[gnu::noinline]]
int dependent(int x){ return x + D;} // #DEP
[[gnu::noinline]]
int non_dependent(int x){return x;} // #NO_DEP
template<int D> [[gnu::noinline]]
int bar(int x) {
[[clang::always_inline]] return bar<D-1>(x + 1);
int baz(int x) { // #BAZ
// expected-warning@+2{{statement attribute 'always_inline' has higher precedence than function attribute 'noinline'}}
// expected-note@#NO_DEP{{conflicting attribute is here}}
[[clang::always_inline]] non_dependent(x);
if constexpr (D>0) {
// expected-warning@+6{{statement attribute 'always_inline' has higher precedence than function attribute 'noinline'}}
// expected-note@#NO_DEP{{conflicting attribute is here}}
// expected-warning@+4 3{{statement attribute 'always_inline' has higher precedence than function attribute 'noinline'}}
// expected-note@#BAZ 3{{conflicting attribute is here}}
// expected-note@#BAZ_INST 3{{in instantiation}}
// expected-note@+1 3{{in instantiation}}
[[clang::always_inline]] return non_dependent(x), baz<D-1>(x + 1);
}
return x;
}
// We can't suppress if there is a variadic involved.
template<int ... D>
int variadic_baz(int x) {
// Diagnoses NO_DEP 2x, once during phase 1, the second during instantiation.
// Dianoses DEP 3x, once per variadic expansion.
// expected-warning@+5 2{{statement attribute 'always_inline' has higher precedence than function attribute 'noinline'}}
// expected-note@#NO_DEP 2{{conflicting attribute is here}}
// expected-warning@+3 3{{statement attribute 'always_inline' has higher precedence than function attribute 'noinline'}}
// expected-note@#DEP 3{{conflicting attribute is here}}
// expected-note@#VARIADIC_INST{{in instantiation}}
[[clang::always_inline]] return non_dependent(x) + (dependent<D>(x) + ...);
}
void use() {
baz<3>(0); // #BAZ_INST
variadic_baz<0, 1, 2>(0); // #VARIADIC_INST
}

45
clang/test/Sema/attr-noinline.cpp
View File

@ -2,9 +2,10 @@
int bar();
// expected-note@+1{{conflicting attribute is here}}
[[gnu::always_inline]] void always_inline_fn(void) { }
// expected-note@+1{{conflicting attribute is here}}
[[gnu::flatten]] void flatten_fn(void) { }
[[gnu::noinline]] void noinline_fn(void) { }
void foo() {
@ -32,9 +33,43 @@ int foo(int x) {
[[clang::noinline]] return foo<D-1>(x + 1);
}
// FIXME: This should warn that noinline statement attribute has higher
// precedence than the always_inline function attribute.
template<int D>
[[clang::always_inline]]
int dependent(int x){ return x + D;} // #DEP
[[clang::always_inline]]
int non_dependent(int x){return x;} // #NO_DEP
template<int D> [[clang::always_inline]]
int bar(int x) {
[[clang::noinline]] return bar<D-1>(x + 1);
int baz(int x) { // #BAZ
// expected-warning@+2{{statement attribute 'noinline' has higher precedence than function attribute 'always_inline'}}
// expected-note@#NO_DEP{{conflicting attribute is here}}
[[clang::noinline]] non_dependent(x);
if constexpr (D>0) {
// expected-warning@+6{{statement attribute 'noinline' has higher precedence than function attribute 'always_inline'}}
// expected-note@#NO_DEP{{conflicting attribute is here}}
// expected-warning@+4 3{{statement attribute 'noinline' has higher precedence than function attribute 'always_inline'}}
// expected-note@#BAZ 3{{conflicting attribute is here}}
// expected-note@#BAZ_INST 3{{in instantiation}}
// expected-note@+1 3{{in instantiation}}
[[clang::noinline]] return non_dependent(x), baz<D-1>(x + 1);
}
return x;
}
// We can't suppress if there is a variadic involved.
template<int ... D>
int variadic_baz(int x) {
// Diagnoses NO_DEP 2x, once during phase 1, the second during instantiation.
// Dianoses DEP 3x, once per variadic expansion.
// expected-warning@+5 2{{statement attribute 'noinline' has higher precedence than function attribute 'always_inline'}}
// expected-note@#NO_DEP 2{{conflicting attribute is here}}
// expected-warning@+3 3{{statement attribute 'noinline' has higher precedence than function attribute 'always_inline'}}
// expected-note@#DEP 3{{conflicting attribute is here}}
// expected-note@#VARIADIC_INST{{in instantiation}}
[[clang::noinline]] return non_dependent(x) + (dependent<D>(x) + ...);
}
void use() {
baz<3>(0); // #BAZ_INST
variadic_baz<0, 1, 2>(0); // #VARIADIC_INST
}