mboehme created this revision.
Herald added a reviewer: aaron.ballman.
Herald added a project: All.
mboehme requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Clang has allowed this so far, transferring the attributes to the declaration's
type instead, as would be done for GNU syntax attributes. However, the C++
standard is clear that attributes in certain positions appertain to the
declaration, so we shouldn't allow type attributes in these positions.
For backwards compatibility, we only warn on non-declaration attributes that we
have historically allowed to be put on declarations. We only produce errors for
new non-declaration attributes.
Depends On D111548 <https://reviews.llvm.org/D111548>
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D124081
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Parse/Parser.h
clang/include/clang/Sema/ParsedAttr.h
clang/lib/Parse/ParseDecl.cpp
clang/lib/Parse/ParseDeclCXX.cpp
clang/lib/Parse/ParseExprCXX.cpp
clang/lib/Parse/ParseStmt.cpp
clang/lib/Parse/ParseTemplate.cpp
clang/lib/Parse/Parser.cpp
clang/test/Sema/annotate-type.c
clang/test/SemaCXX/annotate-type.cpp
clang/utils/TableGen/ClangAttrEmitter.cpp
Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===================================================================
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -4220,6 +4220,9 @@
OS << " /*IsStmt=*/";
OS << (Attr.isSubClassOf("StmtAttr") || Attr.isSubClassOf("DeclOrStmtAttr"))
<< ",\n";
+ OS << " /*IsDecl=*/";
+ OS << (!Attr.isSubClassOf("TypeAttr") && !Attr.isSubClassOf("StmtAttr"))
+ << ",\n";
OS << " /*IsKnownToGCC=*/";
OS << IsKnownToGCC(Attr) << ",\n";
OS << " /*IsSupportedByPragmaAttribute=*/";
Index: clang/test/SemaCXX/annotate-type.cpp
===================================================================
--- clang/test/SemaCXX/annotate-type.cpp
+++ clang/test/SemaCXX/annotate-type.cpp
@@ -2,10 +2,7 @@
struct S1 {
void f() [[clang::annotate_type("foo")]];
- // FIXME: We would want to prohibit the attribute in the following location.
- // However, Clang currently generally doesn't prohibit type-only C++11
- // attributes on declarations. This should be fixed more generally.
- [[clang::annotate_type("foo")]] void g();
+ [[clang::annotate_type("foo")]] void g(); // expected-error {{'annotate_type' attribute cannot be applied to a declaration}}
};
template <typename T1, typename T2> struct is_same {
@@ -50,10 +47,8 @@
// More error cases: Prohibit adding the attribute to declarations.
// Different declarations hit different code paths, so they need separate tests.
-// FIXME: Clang currently generally doesn't prohibit type-only C++11
-// attributes on declarations.
-namespace [[clang::annotate_type("foo")]] my_namespace {}
-struct [[clang::annotate_type("foo")]] S3{};
+namespace [[clang::annotate_type("foo")]] my_namespace {} // expected-error {{'annotate_type' attribute cannot be applied to a declaration}}
+struct [[clang::annotate_type("foo")]] S3{}; // expected-error {{'annotate_type' attribute cannot be applied to a declaration}}
void f4() {
- for ([[clang::annotate_type("foo")]] int i = 0; i < 42; ++i) {}
+ for ([[clang::annotate_type("foo")]] int i = 0; i < 42; ++i) {} // expected-error {{'annotate_type' attribute cannot be applied to a declaration}}
}
Index: clang/test/Sema/annotate-type.c
===================================================================
--- clang/test/Sema/annotate-type.c
+++ clang/test/Sema/annotate-type.c
@@ -17,10 +17,7 @@
int *__attribute__((annotate_type("bar"))) y2; // expected-warning {{unknown attribute 'annotate_type' ignored}}
// Various error cases
- // FIXME: We would want to prohibit the attribute in the following location.
- // However, Clang currently generally doesn't prohibit type-only C++11
- // attributes on declarations. This should be fixed more generally.
- [[clang::annotate_type("bar")]] int *z1;
+ [[clang::annotate_type("bar")]] int *z1; // expected-error {{'annotate_type' attribute cannot be applied to a declaration}}
int *z2 [[clang::annotate_type("bar")]]; // expected-error {{'annotate_type' attribute cannot be applied to a declaration}}
[[clang::annotate_type("bar")]]; // expected-error {{'annotate_type' attribute cannot be applied to a statement}}
int *[[clang::annotate_type(1)]] z3; // expected-error {{'annotate_type' attribute requires a string}}
@@ -33,7 +30,5 @@
}
// More error cases: Prohibit adding the attribute to declarations.
// Different declarations hit different code paths, so they need separate tests.
-// FIXME: Clang currently generally doesn't prohibit type-only C++11
-// attributes on declarations.
-[[clang::annotate_type("bar")]] int *global;
-void g([[clang::annotate_type("bar")]] int);
+[[clang::annotate_type("bar")]] int *global; // expected-error {{'annotate_type' attribute cannot be applied to a declaration}}
+void g([[clang::annotate_type("bar")]] int); // expected-error {{'annotate_type' attribute cannot be applied to a declaration}}
Index: clang/lib/Parse/Parser.cpp
===================================================================
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -1115,6 +1115,7 @@
return Actions.ConvertDeclToDeclGroup(TheDecl);
}
+ DiagnoseCXX11NonDeclAttrs(attrs);
DS.takeAttributesFrom(attrs);
// ObjC2 allows prefix attributes on class interfaces and protocols.
Index: clang/lib/Parse/ParseTemplate.cpp
===================================================================
--- clang/lib/Parse/ParseTemplate.cpp
+++ clang/lib/Parse/ParseTemplate.cpp
@@ -241,8 +241,10 @@
// Move the attributes from the prefix into the DS.
if (TemplateInfo.Kind == ParsedTemplateInfo::ExplicitInstantiation)
ProhibitAttributes(prefixAttrs);
- else
+ else {
+ DiagnoseCXX11NonDeclAttrs(prefixAttrs);
DS.takeAttributesFrom(prefixAttrs);
+ }
// Parse the declarator.
ParsingDeclarator DeclaratorInfo(*this, DS, (DeclaratorContext)Context);
Index: clang/lib/Parse/ParseStmt.cpp
===================================================================
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -2584,6 +2584,7 @@
MaybeParseCXX11Attributes(Attributes);
DeclSpec DS(AttrFactory);
+ DiagnoseCXX11NonDeclAttrs(Attributes);
DS.takeAttributesFrom(Attributes);
if (ParseCXXTypeSpecifierSeq(DS))
Index: clang/lib/Parse/ParseExprCXX.cpp
===================================================================
--- clang/lib/Parse/ParseExprCXX.cpp
+++ clang/lib/Parse/ParseExprCXX.cpp
@@ -2079,6 +2079,7 @@
// type-specifier-seq
DeclSpec DS(AttrFactory);
+ DiagnoseCXX11NonDeclAttrs(attrs);
DS.takeAttributesFrom(attrs);
ParseSpecifierQualifierList(DS, AS_none, DeclSpecContext::DSC_condition);
Index: clang/lib/Parse/ParseDeclCXX.cpp
===================================================================
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -10,7 +10,6 @@
//
//===----------------------------------------------------------------------===//
-#include "clang/Parse/Parser.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/DeclTemplate.h"
#include "clang/AST/PrettyDeclStackTrace.h"
@@ -19,10 +18,12 @@
#include "clang/Basic/OperatorKinds.h"
#include "clang/Basic/TargetInfo.h"
#include "clang/Parse/ParseDiagnostic.h"
+#include "clang/Parse/Parser.h"
#include "clang/Parse/RAIIObjectsForParser.h"
#include "clang/Sema/DeclSpec.h"
#include "clang/Sema/ParsedTemplate.h"
#include "clang/Sema/Scope.h"
+#include "clang/Sema/SemaDiagnostic.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/Support/TimeProfiler.h"
@@ -213,6 +214,8 @@
Diag(FirstNestedInlineLoc, diag::ext_inline_nested_namespace_definition);
}
+ DiagnoseCXX11NonDeclAttrs(attrs);
+
// If we're still good, complain about inline namespaces in non-C++0x now.
if (InlineLoc.isValid())
Diag(InlineLoc, getLangOpts().CPlusPlus11 ?
@@ -2004,6 +2007,8 @@
TParams =
MultiTemplateParamsArg(&(*TemplateParams)[0], TemplateParams->size());
+ DiagnoseCXX11NonDeclAttrs(attrs);
+
stripTypeAttributesOffDeclSpec(attrs, DS, TUK);
// Declaration or definition of a class type
@@ -2726,6 +2731,7 @@
// decl-specifier-seq:
// Parse the common declaration-specifiers piece.
ParsingDeclSpec DS(*this, TemplateDiags);
+ DiagnoseCXX11NonDeclAttrs(attrs);
DS.takeAttributesFrom(attrs);
if (MalformedTypeSpec)
DS.SetTypeSpecError();
Index: clang/lib/Parse/ParseDecl.cpp
===================================================================
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -10,8 +10,6 @@
//
//===----------------------------------------------------------------------===//
-#include "clang/Parse/Parser.h"
-#include "clang/Parse/RAIIObjectsForParser.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/DeclTemplate.h"
#include "clang/AST/PrettyDeclStackTrace.h"
@@ -20,6 +18,8 @@
#include "clang/Basic/CharInfo.h"
#include "clang/Basic/TargetInfo.h"
#include "clang/Parse/ParseDiagnostic.h"
+#include "clang/Parse/Parser.h"
+#include "clang/Parse/RAIIObjectsForParser.h"
#include "clang/Sema/Lookup.h"
#include "clang/Sema/ParsedTemplate.h"
#include "clang/Sema/Scope.h"
@@ -28,6 +28,7 @@
#include "llvm/ADT/SmallSet.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringSwitch.h"
+#include <unordered_set>
using namespace clang;
@@ -1716,6 +1717,61 @@
}
}
+void Parser::DiagnoseCXX11NonDeclAttrs(ParsedAttributes &Attrs) {
+ for (ParsedAttr &PA : Attrs) {
+ if (!(PA.isCXX11Attribute() || PA.isC2xAttribute()))
+ continue;
+
+ if (PA.getInfo().IsDecl)
+ continue;
+
+ AttributeCommonInfo::Kind kind = PA.getKind();
+ if (kind == ParsedAttr::UnknownAttribute)
+ continue;
+
+ // Using unordered_set because we don't have DenseMapInfo for
+ // ParsedAttr::Kind.
+ static std::unordered_set<ParsedAttr::Kind> WarningOnlyAttrs = {
+ ParsedAttr::AT_AddressSpace,
+ ParsedAttr::AT_CmseNSCall,
+ ParsedAttr::AT_OpenCLPrivateAddressSpace,
+ ParsedAttr::AT_OpenCLGlobalAddressSpace,
+ ParsedAttr::AT_OpenCLGlobalDeviceAddressSpace,
+ ParsedAttr::AT_OpenCLGlobalHostAddressSpace,
+ ParsedAttr::AT_OpenCLLocalAddressSpace,
+ ParsedAttr::AT_OpenCLConstantAddressSpace,
+ ParsedAttr::AT_OpenCLGenericAddressSpace,
+ ParsedAttr::AT_NeonPolyVectorType,
+ ParsedAttr::AT_NeonVectorType,
+ ParsedAttr::AT_ArmSveVectorBits,
+ ParsedAttr::AT_ArmMveStrictPolymorphism,
+ ParsedAttr::AT_BTFTypeTag,
+ ParsedAttr::AT_TypeNonNull,
+ ParsedAttr::AT_TypeNullable,
+ ParsedAttr::AT_TypeNullableResult,
+ ParsedAttr::AT_TypeNullUnspecified,
+ ParsedAttr::AT_ObjCInertUnsafeUnretained,
+ ParsedAttr::AT_Regparm,
+ ParsedAttr::AT_NoDeref,
+ ParsedAttr::AT_ObjCGC,
+ ParsedAttr::AT_VectorSize,
+ ParsedAttr::AT_MatrixType,
+ ParsedAttr::AT_Ptr32,
+ ParsedAttr::AT_Ptr64,
+ ParsedAttr::AT_SPtr,
+ ParsedAttr::AT_UPtr,
+ };
+
+ if (WarningOnlyAttrs.count(kind)) {
+ // FIXME: Emit a warning and change all affected tests to expect this
+ // warning.
+ } else {
+ Diag(PA.getLoc(), diag::err_type_attribute_invalid_on_decl) << PA;
+ PA.setInvalid();
+ }
+ }
+}
+
// Usually, `__attribute__((attrib)) class Foo {} var` means that attribute
// applies to var, not the type Foo.
// As an exception to the rule, __declspec(align(...)) before the
@@ -1863,6 +1919,7 @@
if (DeclSpecStart)
DS.SetRangeStart(*DeclSpecStart);
+ DiagnoseCXX11NonDeclAttrs(Attrs);
DS.takeAttributesFrom(Attrs);
return ParseDeclGroup(DS, Context, &DeclEnd, FRI);
}
@@ -4314,6 +4371,7 @@
// Parse leading attributes.
ParsedAttributesWithRange Attrs(AttrFactory);
MaybeParseCXX11Attributes(Attrs);
+ DiagnoseCXX11NonDeclAttrs(Attrs);
DS.takeAttributesFrom(Attrs);
// Parse the common specifier-qualifiers-list piece.
@@ -6977,6 +7035,8 @@
// too much hassle.
DS.takeAttributesFrom(FirstArgAttrs);
+ DiagnoseCXX11NonDeclAttrs(DS.getAttributes());
+
ParseDeclarationSpecifiers(DS);
Index: clang/include/clang/Sema/ParsedAttr.h
===================================================================
--- clang/include/clang/Sema/ParsedAttr.h
+++ clang/include/clang/Sema/ParsedAttr.h
@@ -61,6 +61,8 @@
unsigned IsType : 1;
/// True if this attribute applies to statements.
unsigned IsStmt : 1;
+ /// True if this attribute applies to declarations.
+ unsigned IsDecl : 1;
/// True if this attribute has any spellings that are known to gcc.
unsigned IsKnownToGCC : 1;
/// True if this attribute is supported by #pragma clang attribute.
@@ -79,20 +81,23 @@
AttributeCommonInfo::NoSemaHandlerAttribute)
: AttrKind(AttrKind), NumArgs(0), OptArgs(0), NumArgMembers(0),
HasCustomParsing(0), AcceptsExprPack(0), IsTargetSpecific(0), IsType(0),
- IsStmt(0), IsKnownToGCC(0), IsSupportedByPragmaAttribute(0) {}
+ IsStmt(0), IsDecl(0), IsKnownToGCC(0), IsSupportedByPragmaAttribute(0) {
+ }
constexpr ParsedAttrInfo(AttributeCommonInfo::Kind AttrKind, unsigned NumArgs,
unsigned OptArgs, unsigned NumArgMembers,
unsigned HasCustomParsing, unsigned AcceptsExprPack,
unsigned IsTargetSpecific, unsigned IsType,
- unsigned IsStmt, unsigned IsKnownToGCC,
+ unsigned IsStmt, unsigned IsDecl,
+ unsigned IsKnownToGCC,
unsigned IsSupportedByPragmaAttribute,
ArrayRef<Spelling> Spellings,
ArrayRef<const char *> ArgNames)
: AttrKind(AttrKind), NumArgs(NumArgs), OptArgs(OptArgs),
NumArgMembers(NumArgMembers), HasCustomParsing(HasCustomParsing),
AcceptsExprPack(AcceptsExprPack), IsTargetSpecific(IsTargetSpecific),
- IsType(IsType), IsStmt(IsStmt), IsKnownToGCC(IsKnownToGCC),
+ IsType(IsType), IsStmt(IsStmt), IsDecl(IsDecl),
+ IsKnownToGCC(IsKnownToGCC),
IsSupportedByPragmaAttribute(IsSupportedByPragmaAttribute),
Spellings(Spellings), ArgNames(ArgNames) {}
Index: clang/include/clang/Parse/Parser.h
===================================================================
--- clang/include/clang/Parse/Parser.h
+++ clang/include/clang/Parse/Parser.h
@@ -2654,6 +2654,13 @@
/// clang accepts as an extension.
void DiagnoseCXX11AttributeExtension(ParsedAttributesWithRange &Attrs);
+ /// Emit diagnostics for C++11 and C2x attributes that aren't declaration
+ /// attributes.
+ /// Historically, we've allowed some pure type attributes to be applied to
+ /// declarations. For these, we emit only a warning. For all others, we emit
+ /// an error.
+ void DiagnoseCXX11NonDeclAttrs(ParsedAttributes &Attrs);
+
/// Parses syntax-generic attribute arguments for attributes which are
/// known to the implementation, and adds them to the given ParsedAttributes
/// list with the given attribute syntax. Returns the number of arguments
Index: clang/include/clang/Basic/Attr.td
===================================================================
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -2553,7 +2553,7 @@
let Documentation = [SwiftAsyncErrorDocs];
}
-def Suppress : StmtAttr {
+def Suppress : DeclOrStmtAttr {
let Spellings = [CXX11<"gsl", "suppress">];
let Args = [VariadicStringArgument<"DiagnosticIdentifiers">];
let Documentation = [SuppressDocs];
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
