https://github.com/Endilll updated https://github.com/llvm/llvm-project/pull/69104
>From 976aa5c8f3d936a15e7123069a49d97ad3bf7a05 Mon Sep 17 00:00:00 2001 From: Vlad Serebrennikov <serebrennikov.vladis...@gmail.com> Date: Sun, 15 Oct 2023 13:14:55 +0300 Subject: [PATCH 01/14] [clang] Add clang::debug_info_type attribute --- clang/include/clang/Basic/Attr.td | 11 ++++++++ clang/include/clang/Basic/AttrDocs.td | 12 +++++++++ .../clang/Basic/DiagnosticSemaKinds.td | 2 ++ clang/lib/CodeGen/CGDebugInfo.cpp | 2 ++ clang/lib/Sema/SemaDeclAttr.cpp | 26 +++++++++++++++++++ .../CodeGen/debug-info-debug-info-type.cpp | 14 ++++++++++ 6 files changed, 67 insertions(+) create mode 100644 clang/test/CodeGen/debug-info-debug-info-type.cpp diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 5c9eb7b8a981037..024421c0583c019 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -107,6 +107,10 @@ def NonBitField : SubsetSubject<Field, [{!S->isBitField()}], "non-bit-field non-static data members">; +def BitField : SubsetSubject<Field, + [{S->isBitField()}], + "bit-field non-static data members">; + def NonStaticCXXMethod : SubsetSubject<CXXMethod, [{!S->isStatic()}], "non-static member functions">; @@ -4264,3 +4268,10 @@ def CountedBy : InheritableAttr { void setCountedByFieldLoc(SourceRange Loc) { CountedByFieldLoc = Loc; } }]; } + +def DebugInfoType: InheritableAttr { + let Spellings = [Clang<"debug_info_type">]; + let Subjects = SubjectList<[BitField], ErrorDiag>; + let Args = [TypeArgument<"Type", 1>]; + let Documentation = [DebugInfoTypeDocumentation]; +} diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 9f9991bdae36155..6cceba1e0e0ad01 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -7219,6 +7219,18 @@ its underlying representation to be a WebAssembly ``funcref``. }]; } +def DebugInfoTypeDocumentation : Documentation { + let Category = DocCatField; + let Content = [{ +This attribute allows to alter type of a bitfield in debug information. +Such a need might arise when bitfield is intended to store an enumeration value, +but has to be specified as having enumeration's underlying type, in order to +facilitate compiler optimizations. But this also causes underlying type to be +emitted in debug information, making it hard for debuggers to map bitfield's +value back to enumeration. This attribute helps with this. + }]; +} + def CleanupDocs : Documentation { let Category = DocCatType; let Content = [{ diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index e85cd4d1a1ddc0d..b5c73494df367a6 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -3153,6 +3153,8 @@ def err_invalid_branch_protection_spec : Error< "invalid or misplaced branch protection specification '%0'">; def warn_unsupported_branch_protection_spec : Warning< "unsupported branch protection specification '%0'">, InGroup<BranchProtection>; +def warn_attribute_underlying_type_mismatch : Warning< + "underlying type %0 of enumeration %1 doesn't match bitfield type %2">; def warn_unsupported_target_attribute : Warning<"%select{unsupported|duplicate|unknown}0%select{| CPU|" diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index c73a63e12f03aab..85aedd87b21d41e 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -1497,6 +1497,8 @@ CGDebugInfo::createBitFieldType(const FieldDecl *BitFieldDecl, llvm::DIScope *RecordTy, const RecordDecl *RD) { StringRef Name = BitFieldDecl->getName(); QualType Ty = BitFieldDecl->getType(); + if (BitFieldDecl->hasAttr<DebugInfoTypeAttr>()) + Ty = BitFieldDecl->getAttr<DebugInfoTypeAttr>()->getType(); SourceLocation Loc = BitFieldDecl->getLocation(); llvm::DIFile *VUnit = getOrCreateFile(Loc); llvm::DIType *DebugType = getOrCreateType(Ty, VUnit); diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index feb02cad9080e3e..8d58968b7f985c8 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -5910,6 +5910,28 @@ static void handleBuiltinAliasAttr(Sema &S, Decl *D, D->addAttr(::new (S.Context) BuiltinAliasAttr(S.Context, AL, Ident)); } +static void handleDebugInfoTypeAttr(Sema &S, Decl *D, const ParsedAttr &AL) { + if (!AL.hasParsedType()) { + S.Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments) << AL << 1; + return; + } + + TypeSourceInfo *ParmTSI = nullptr; + QualType type = S.GetTypeFromParser(AL.getTypeArg(), &ParmTSI); + assert(ParmTSI && "no type source info for attribute argument"); + + if (type->isEnumeralType()) { + QualType BitfieldType = llvm::cast<FieldDecl>(D)->getType(); + QualType EnumUnderlyingType = type->getAs<EnumType>()->getDecl()->getIntegerType(); + if (EnumUnderlyingType != BitfieldType) { + S.Diag(AL.getLoc(), diag::warn_attribute_underlying_type_mismatch) << EnumUnderlyingType << type << BitfieldType; + return; + } + } + + D->addAttr(::new (S.Context) DebugInfoTypeAttr(S.Context, AL, ParmTSI)); +} + //===----------------------------------------------------------------------===// // Checker-specific attribute handlers. //===----------------------------------------------------------------------===// @@ -9629,6 +9651,10 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL, handleBuiltinAliasAttr(S, D, AL); break; + case ParsedAttr::AT_DebugInfoType: + handleDebugInfoTypeAttr(S, D, AL); + break; + case ParsedAttr::AT_UsingIfExists: handleSimpleAttribute<UsingIfExistsAttr>(S, D, AL); break; diff --git a/clang/test/CodeGen/debug-info-debug-info-type.cpp b/clang/test/CodeGen/debug-info-debug-info-type.cpp new file mode 100644 index 000000000000000..6104ce7463ef918 --- /dev/null +++ b/clang/test/CodeGen/debug-info-debug-info-type.cpp @@ -0,0 +1,14 @@ +// RUN: %clang -target x86_64-linux -g -S -emit-llvm -o - %s | FileCheck %s +// RUN: %clang_cc1 -verify -DMISMATCH %s + +struct A { + enum E : unsigned {}; + [[clang::debug_info_type(E)]] unsigned b : 2; +#ifdef MISMATCH + [[clang::debug_info_type(E)]] int b2 : 2; + // expected-warning@-1 {{underlying type 'unsigned int' of enumeration 'E' doesn't match bitfield type 'int'}} +#endif +} a; + +// CHECK-DAG: [[ENUM:![0-9]+]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: "E"{{.*}} +// CHECK-DAG: !DIDerivedType(tag: DW_TAG_member, name: "b",{{.*}} baseType: [[ENUM]] \ No newline at end of file >From 4330b70c2fde70010f1caabbf6562b61c701ef7e Mon Sep 17 00:00:00 2001 From: Vlad Serebrennikov <serebrennikov.vladis...@gmail.com> Date: Sun, 15 Oct 2023 13:34:09 +0300 Subject: [PATCH 02/14] Add newline to the test --- clang/test/CodeGen/debug-info-debug-info-type.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/test/CodeGen/debug-info-debug-info-type.cpp b/clang/test/CodeGen/debug-info-debug-info-type.cpp index 6104ce7463ef918..4b60d1b64be239f 100644 --- a/clang/test/CodeGen/debug-info-debug-info-type.cpp +++ b/clang/test/CodeGen/debug-info-debug-info-type.cpp @@ -11,4 +11,4 @@ struct A { } a; // CHECK-DAG: [[ENUM:![0-9]+]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: "E"{{.*}} -// CHECK-DAG: !DIDerivedType(tag: DW_TAG_member, name: "b",{{.*}} baseType: [[ENUM]] \ No newline at end of file +// CHECK-DAG: !DIDerivedType(tag: DW_TAG_member, name: "b",{{.*}} baseType: [[ENUM]] >From faf8238ae2a6362214e8176e4b9b7225fa7c8bd1 Mon Sep 17 00:00:00 2001 From: Vlad Serebrennikov <serebrennikov.vladis...@gmail.com> Date: Sun, 15 Oct 2023 13:55:13 +0300 Subject: [PATCH 03/14] Run clang-format --- clang/lib/Sema/SemaDeclAttr.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 8d58968b7f985c8..294a73d2abf2d29 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -5922,9 +5922,11 @@ static void handleDebugInfoTypeAttr(Sema &S, Decl *D, const ParsedAttr &AL) { if (type->isEnumeralType()) { QualType BitfieldType = llvm::cast<FieldDecl>(D)->getType(); - QualType EnumUnderlyingType = type->getAs<EnumType>()->getDecl()->getIntegerType(); + QualType EnumUnderlyingType = + type->getAs<EnumType>()->getDecl()->getIntegerType(); if (EnumUnderlyingType != BitfieldType) { - S.Diag(AL.getLoc(), diag::warn_attribute_underlying_type_mismatch) << EnumUnderlyingType << type << BitfieldType; + S.Diag(AL.getLoc(), diag::warn_attribute_underlying_type_mismatch) + << EnumUnderlyingType << type << BitfieldType; return; } } >From fc1d920323bfa533805f67441b6cfb2917bc3c6a Mon Sep 17 00:00:00 2001 From: Vlad Serebrennikov <serebrennikov.vladis...@gmail.com> Date: Thu, 19 Oct 2023 12:36:00 +0300 Subject: [PATCH 04/14] Address feedback from reviewers - Rename the attribute to `preferred_type` - Add a new diagnostic that checks that `preferred_type(bool)` is attached to bit-field of width 1 - Split CodeGen and Sema tests, add more tests - Apply all suggestions --- clang/include/clang/Basic/Attr.td | 8 +++--- clang/include/clang/Basic/AttrDocs.td | 27 ++++++++++++++----- clang/include/clang/Basic/DiagnosticGroups.td | 1 + .../clang/Basic/DiagnosticSemaKinds.td | 6 ++++- clang/lib/CodeGen/CGDebugInfo.cpp | 4 +-- clang/lib/Sema/SemaDeclAttr.cpp | 26 +++++++++++------- ...type.cpp => debug-info-preferred-type.cpp} | 7 +---- clang/test/Sema/attr-preferred-type.cpp | 20 ++++++++++++++ 8 files changed, 70 insertions(+), 29 deletions(-) rename clang/test/CodeGen/{debug-info-debug-info-type.cpp => debug-info-preferred-type.cpp} (52%) create mode 100644 clang/test/Sema/attr-preferred-type.cpp diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 024421c0583c019..867a31495cbc34a 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -109,7 +109,7 @@ def NonBitField : SubsetSubject<Field, def BitField : SubsetSubject<Field, [{S->isBitField()}], - "bit-field non-static data members">; + "bit-field data members">; def NonStaticCXXMethod : SubsetSubject<CXXMethod, [{!S->isStatic()}], @@ -4269,9 +4269,9 @@ def CountedBy : InheritableAttr { }]; } -def DebugInfoType: InheritableAttr { - let Spellings = [Clang<"debug_info_type">]; +def PreferredType: InheritableAttr { + let Spellings = [Clang<"preferred_type">]; let Subjects = SubjectList<[BitField], ErrorDiag>; let Args = [TypeArgument<"Type", 1>]; - let Documentation = [DebugInfoTypeDocumentation]; + let Documentation = [PreferredTypeDocumentation]; } diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 6cceba1e0e0ad01..aea316f0db9acee 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -7219,15 +7219,28 @@ its underlying representation to be a WebAssembly ``funcref``. }]; } -def DebugInfoTypeDocumentation : Documentation { +def PreferredTypeDocumentation : Documentation { let Category = DocCatField; let Content = [{ -This attribute allows to alter type of a bitfield in debug information. -Such a need might arise when bitfield is intended to store an enumeration value, -but has to be specified as having enumeration's underlying type, in order to -facilitate compiler optimizations. But this also causes underlying type to be -emitted in debug information, making it hard for debuggers to map bitfield's -value back to enumeration. This attribute helps with this. +This attribute allows adjusting the type of a bit-field in debug information. +This can be helpful when a bit-field is intended to store an enumeration value, +but has to be specified as having the enumeration's underlying type in order to +facilitate compiler optimizations or bit-field packing behavior. Normally, the +underlying type is what is emitted in debug information, which can make it hard +for debuggers to know to map a bit-field's value back to a particular enumeration. + +.. code-block:: c++ + + enum Colors { Red, Green, Blue }; + + struct S { + [[clang::preferred_type(Colors)]] unsigned ColorVal : 2; + [[clang::preferred_type(bool)]] unsigned UseAlternateColorSpace : 1; + } s = { Green, false }; + +Without the attribute, a debugger is likely to display the value `1` for `ColorVal` +and `0` for `UseAlternateColorSpace`. With the attribute, the debugger may now +display `Green` and `false` instead. }]; } diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 0b09c002191848a..381a674736b316e 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -54,6 +54,7 @@ def BitFieldConstantConversion : DiagGroup<"bitfield-constant-conversion", [SingleBitBitFieldConstantConversion]>; def BitFieldEnumConversion : DiagGroup<"bitfield-enum-conversion">; def BitFieldWidth : DiagGroup<"bitfield-width">; +def BitFieldType : DiagGroup<"bitfield-type">; def CompoundTokenSplitByMacro : DiagGroup<"compound-token-split-by-macro">; def CompoundTokenSplitBySpace : DiagGroup<"compound-token-split-by-space">; def CompoundTokenSplit : DiagGroup<"compound-token-split", diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index b5c73494df367a6..ffbd264f190f119 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -3154,7 +3154,11 @@ def err_invalid_branch_protection_spec : Error< def warn_unsupported_branch_protection_spec : Warning< "unsupported branch protection specification '%0'">, InGroup<BranchProtection>; def warn_attribute_underlying_type_mismatch : Warning< - "underlying type %0 of enumeration %1 doesn't match bitfield type %2">; + "underlying type %0 of enumeration %1 doesn't match bit-field type %2">, + InGroup<BitFieldType>; +def warn_attribute_bool_bitfield_width : Warning< + "bit-field that holds a boolean value should have width of 1 instead of %0">, + InGroup<BitFieldWidth>; def warn_unsupported_target_attribute : Warning<"%select{unsupported|duplicate|unknown}0%select{| CPU|" diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index 85aedd87b21d41e..e6a751c4e297ab6 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -1497,8 +1497,8 @@ CGDebugInfo::createBitFieldType(const FieldDecl *BitFieldDecl, llvm::DIScope *RecordTy, const RecordDecl *RD) { StringRef Name = BitFieldDecl->getName(); QualType Ty = BitFieldDecl->getType(); - if (BitFieldDecl->hasAttr<DebugInfoTypeAttr>()) - Ty = BitFieldDecl->getAttr<DebugInfoTypeAttr>()->getType(); + if (BitFieldDecl->hasAttr<PreferredTypeAttr>()) + Ty = BitFieldDecl->getAttr<PreferredTypeAttr>()->getType(); SourceLocation Loc = BitFieldDecl->getLocation(); llvm::DIFile *VUnit = getOrCreateFile(Loc); llvm::DIType *DebugType = getOrCreateType(Ty, VUnit); diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 294a73d2abf2d29..55c570a4e10b861 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -5910,28 +5910,36 @@ static void handleBuiltinAliasAttr(Sema &S, Decl *D, D->addAttr(::new (S.Context) BuiltinAliasAttr(S.Context, AL, Ident)); } -static void handleDebugInfoTypeAttr(Sema &S, Decl *D, const ParsedAttr &AL) { +static void handlePreferredTypeAttr(Sema &S, Decl *D, const ParsedAttr &AL) { if (!AL.hasParsedType()) { S.Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments) << AL << 1; return; } TypeSourceInfo *ParmTSI = nullptr; - QualType type = S.GetTypeFromParser(AL.getTypeArg(), &ParmTSI); + QualType QT = S.GetTypeFromParser(AL.getTypeArg(), &ParmTSI); assert(ParmTSI && "no type source info for attribute argument"); - if (type->isEnumeralType()) { - QualType BitfieldType = llvm::cast<FieldDecl>(D)->getType(); + if (QT->isEnumeralType()) { + QualType BitfieldType = cast<FieldDecl>(D)->getType()->getCanonicalTypeUnqualified(); QualType EnumUnderlyingType = - type->getAs<EnumType>()->getDecl()->getIntegerType(); + QT->getAs<EnumType>()->getDecl()->getIntegerType()->getCanonicalTypeUnqualified(); if (EnumUnderlyingType != BitfieldType) { S.Diag(AL.getLoc(), diag::warn_attribute_underlying_type_mismatch) - << EnumUnderlyingType << type << BitfieldType; + << EnumUnderlyingType << QT << BitfieldType; + return; + } + } + else if (QT->isBooleanType()) { + unsigned BitfieldWidth = cast<FieldDecl>(D)->getBitWidthValue(S.getASTContext()); + if (BitfieldWidth != 1) { + S.Diag(cast<FieldDecl>(D)->getBitWidth()->getExprLoc(), + diag::warn_attribute_bool_bitfield_width) << BitfieldWidth; return; } } - D->addAttr(::new (S.Context) DebugInfoTypeAttr(S.Context, AL, ParmTSI)); + D->addAttr(::new (S.Context) PreferredTypeAttr(S.Context, AL, ParmTSI)); } //===----------------------------------------------------------------------===// @@ -9653,8 +9661,8 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL, handleBuiltinAliasAttr(S, D, AL); break; - case ParsedAttr::AT_DebugInfoType: - handleDebugInfoTypeAttr(S, D, AL); + case ParsedAttr::AT_PreferredType: + handlePreferredTypeAttr(S, D, AL); break; case ParsedAttr::AT_UsingIfExists: diff --git a/clang/test/CodeGen/debug-info-debug-info-type.cpp b/clang/test/CodeGen/debug-info-preferred-type.cpp similarity index 52% rename from clang/test/CodeGen/debug-info-debug-info-type.cpp rename to clang/test/CodeGen/debug-info-preferred-type.cpp index 4b60d1b64be239f..6406657a18e925c 100644 --- a/clang/test/CodeGen/debug-info-debug-info-type.cpp +++ b/clang/test/CodeGen/debug-info-preferred-type.cpp @@ -1,13 +1,8 @@ // RUN: %clang -target x86_64-linux -g -S -emit-llvm -o - %s | FileCheck %s -// RUN: %clang_cc1 -verify -DMISMATCH %s struct A { enum E : unsigned {}; - [[clang::debug_info_type(E)]] unsigned b : 2; -#ifdef MISMATCH - [[clang::debug_info_type(E)]] int b2 : 2; - // expected-warning@-1 {{underlying type 'unsigned int' of enumeration 'E' doesn't match bitfield type 'int'}} -#endif + [[clang::preferred_type(E)]] unsigned b : 2; } a; // CHECK-DAG: [[ENUM:![0-9]+]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: "E"{{.*}} diff --git a/clang/test/Sema/attr-preferred-type.cpp b/clang/test/Sema/attr-preferred-type.cpp new file mode 100644 index 000000000000000..95373af46cc33fb --- /dev/null +++ b/clang/test/Sema/attr-preferred-type.cpp @@ -0,0 +1,20 @@ +// RUN: %clang_cc1 -verify %s + +struct A { + enum E : unsigned {}; + [[clang::preferred_type(E)]] unsigned b : 2; + [[clang::preferred_type(E)]] int b2 : 2; + // expected-warning@-1 {{underlying type 'unsigned int' of enumeration 'E' doesn't match bit-field type 'int'}} + [[clang::preferred_type(E)]] const unsigned b3 : 2; + [[clang::preferred_type(bool)]] unsigned b4 : 1; + [[clang::preferred_type(bool)]] unsigned b5 : 2; + // expected-warning@-1 {{bit-field that holds a boolean value should have width of 1 instead of 2}} + [[clang::preferred_type()]] unsigned b6 : 2; + // expected-error@-1 {{'preferred_type' attribute takes one argument}} + [[clang::preferred_type]] unsigned b7 : 2; + // expected-error@-1 {{'preferred_type' attribute takes one argument}} + [[clang::preferred_type(E, int)]] unsigned b8 : 2; + // expected-error@-1 {{expected ')'}} + // expected-error@-2 {{expected ','}} + // expected-warning@-3 {{unknown attribute 'int' ignored}} +}; \ No newline at end of file >From 06bd08b936781a6561e105f91e7c7cc009e4627f Mon Sep 17 00:00:00 2001 From: Vlad Serebrennikov <serebrennikov.vladis...@gmail.com> Date: Thu, 19 Oct 2023 12:49:53 +0300 Subject: [PATCH 05/14] Run clang-format --- clang/lib/Sema/SemaDeclAttr.cpp | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 55c570a4e10b861..a232ba4827668d9 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -5921,20 +5921,24 @@ static void handlePreferredTypeAttr(Sema &S, Decl *D, const ParsedAttr &AL) { assert(ParmTSI && "no type source info for attribute argument"); if (QT->isEnumeralType()) { - QualType BitfieldType = cast<FieldDecl>(D)->getType()->getCanonicalTypeUnqualified(); - QualType EnumUnderlyingType = - QT->getAs<EnumType>()->getDecl()->getIntegerType()->getCanonicalTypeUnqualified(); + QualType BitfieldType = + cast<FieldDecl>(D)->getType()->getCanonicalTypeUnqualified(); + QualType EnumUnderlyingType = QT->getAs<EnumType>() + ->getDecl() + ->getIntegerType() + ->getCanonicalTypeUnqualified(); if (EnumUnderlyingType != BitfieldType) { S.Diag(AL.getLoc(), diag::warn_attribute_underlying_type_mismatch) << EnumUnderlyingType << QT << BitfieldType; return; } - } - else if (QT->isBooleanType()) { - unsigned BitfieldWidth = cast<FieldDecl>(D)->getBitWidthValue(S.getASTContext()); + } else if (QT->isBooleanType()) { + unsigned BitfieldWidth = + cast<FieldDecl>(D)->getBitWidthValue(S.getASTContext()); if (BitfieldWidth != 1) { S.Diag(cast<FieldDecl>(D)->getBitWidth()->getExprLoc(), - diag::warn_attribute_bool_bitfield_width) << BitfieldWidth; + diag::warn_attribute_bool_bitfield_width) + << BitfieldWidth; return; } } >From 9bd1652661acfbe9d724835eacc853aeb4f6102a Mon Sep 17 00:00:00 2001 From: Vlad Serebrennikov <serebrennikov.vladis...@gmail.com> Date: Thu, 19 Oct 2023 20:46:28 +0300 Subject: [PATCH 06/14] Fix location for underlying type mismatch diagnostic --- clang/lib/Sema/SemaDeclAttr.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index a232ba4827668d9..0c95ac5bc2bc0f2 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -5928,7 +5928,8 @@ static void handlePreferredTypeAttr(Sema &S, Decl *D, const ParsedAttr &AL) { ->getIntegerType() ->getCanonicalTypeUnqualified(); if (EnumUnderlyingType != BitfieldType) { - S.Diag(AL.getLoc(), diag::warn_attribute_underlying_type_mismatch) + S.Diag(ParmTSI->getTypeLoc().getBeginLoc(), + diag::warn_attribute_underlying_type_mismatch) << EnumUnderlyingType << QT << BitfieldType; return; } >From b9f66ef9e1a5b78646c60e3efafc2687b8af8192 Mon Sep 17 00:00:00 2001 From: Vlad Serebrennikov <serebrennikov.vladis...@gmail.com> Date: Thu, 19 Oct 2023 21:33:00 +0300 Subject: [PATCH 07/14] Ignore sign-ness when comparing bitfield type against enum underlying type --- clang/lib/Sema/SemaDeclAttr.cpp | 9 ++++++++- clang/test/Sema/attr-preferred-type.cpp | 4 ++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 0c95ac5bc2bc0f2..8c0ae9d8461d79c 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -5921,13 +5921,20 @@ static void handlePreferredTypeAttr(Sema &S, Decl *D, const ParsedAttr &AL) { assert(ParmTSI && "no type source info for attribute argument"); if (QT->isEnumeralType()) { + auto IsCorrespondingType = [&](QualType LHS, QualType RHS) { + assert(LHS != RHS); + if (LHS->isSignedIntegerType()) + return LHS == S.getASTContext().getCorrespondingSignedType(RHS); + return LHS == S.getASTContext().getCorrespondingUnsignedType(RHS); + }; QualType BitfieldType = cast<FieldDecl>(D)->getType()->getCanonicalTypeUnqualified(); QualType EnumUnderlyingType = QT->getAs<EnumType>() ->getDecl() ->getIntegerType() ->getCanonicalTypeUnqualified(); - if (EnumUnderlyingType != BitfieldType) { + if (EnumUnderlyingType != BitfieldType && + !IsCorrespondingType(EnumUnderlyingType, BitfieldType)) { S.Diag(ParmTSI->getTypeLoc().getBeginLoc(), diag::warn_attribute_underlying_type_mismatch) << EnumUnderlyingType << QT << BitfieldType; diff --git a/clang/test/Sema/attr-preferred-type.cpp b/clang/test/Sema/attr-preferred-type.cpp index 95373af46cc33fb..7961070d32ae640 100644 --- a/clang/test/Sema/attr-preferred-type.cpp +++ b/clang/test/Sema/attr-preferred-type.cpp @@ -2,10 +2,10 @@ struct A { enum E : unsigned {}; + enum E2 : int {}; [[clang::preferred_type(E)]] unsigned b : 2; [[clang::preferred_type(E)]] int b2 : 2; - // expected-warning@-1 {{underlying type 'unsigned int' of enumeration 'E' doesn't match bit-field type 'int'}} - [[clang::preferred_type(E)]] const unsigned b3 : 2; + [[clang::preferred_type(E2)]] const unsigned b3 : 2; [[clang::preferred_type(bool)]] unsigned b4 : 1; [[clang::preferred_type(bool)]] unsigned b5 : 2; // expected-warning@-1 {{bit-field that holds a boolean value should have width of 1 instead of 2}} >From f926eeab32160411a5354009b6c296e628125f9c Mon Sep 17 00:00:00 2001 From: Vlad Serebrennikov <serebrennikov.vladis...@gmail.com> Date: Thu, 19 Oct 2023 21:51:09 +0300 Subject: [PATCH 08/14] Add missing newline --- clang/test/Sema/attr-preferred-type.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/test/Sema/attr-preferred-type.cpp b/clang/test/Sema/attr-preferred-type.cpp index 7961070d32ae640..1d98aa7c32f96c8 100644 --- a/clang/test/Sema/attr-preferred-type.cpp +++ b/clang/test/Sema/attr-preferred-type.cpp @@ -17,4 +17,4 @@ struct A { // expected-error@-1 {{expected ')'}} // expected-error@-2 {{expected ','}} // expected-warning@-3 {{unknown attribute 'int' ignored}} -}; \ No newline at end of file +}; >From 84a8ea7cbe1311b0c62290014e3271b5743b8ed2 Mon Sep 17 00:00:00 2001 From: Vlad Serebrennikov <serebrennikov.vladis...@gmail.com> Date: Fri, 20 Oct 2023 19:00:19 +0300 Subject: [PATCH 09/14] Ensure that type argument is emitted in debug info --- clang/lib/Sema/SemaDeclAttr.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 8c0ae9d8461d79c..5e550f7e1d5f47b 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -5918,6 +5918,7 @@ static void handlePreferredTypeAttr(Sema &S, Decl *D, const ParsedAttr &AL) { TypeSourceInfo *ParmTSI = nullptr; QualType QT = S.GetTypeFromParser(AL.getTypeArg(), &ParmTSI); + QT->getAsTagDecl()->setCompleteDefinitionRequired(); assert(ParmTSI && "no type source info for attribute argument"); if (QT->isEnumeralType()) { >From 85129ee19a4fe43f2f3ef037876840c951f2a768 Mon Sep 17 00:00:00 2001 From: Vlad Serebrennikov <serebrennikov.vladis...@gmail.com> Date: Fri, 20 Oct 2023 22:38:11 +0300 Subject: [PATCH 10/14] Require type argument to be a complete type --- clang/lib/Sema/SemaDeclAttr.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 5e550f7e1d5f47b..aa6858ccb891be4 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -5918,8 +5918,8 @@ static void handlePreferredTypeAttr(Sema &S, Decl *D, const ParsedAttr &AL) { TypeSourceInfo *ParmTSI = nullptr; QualType QT = S.GetTypeFromParser(AL.getTypeArg(), &ParmTSI); - QT->getAsTagDecl()->setCompleteDefinitionRequired(); assert(ParmTSI && "no type source info for attribute argument"); + S.RequireCompleteType(ParmTSI->getTypeLoc().getBeginLoc(), QT, diag::err_incomplete_type); if (QT->isEnumeralType()) { auto IsCorrespondingType = [&](QualType LHS, QualType RHS) { >From 436b966cf0862d771e3c28b48924d88b9f78f0fc Mon Sep 17 00:00:00 2001 From: Vlad Serebrennikov <serebrennikov.vladis...@gmail.com> Date: Fri, 20 Oct 2023 22:48:00 +0300 Subject: [PATCH 11/14] Remove diagnostic for bool bif-fields --- clang/include/clang/Basic/DiagnosticSemaKinds.td | 3 --- clang/lib/Sema/SemaDeclAttr.cpp | 9 --------- clang/test/Sema/attr-preferred-type.cpp | 1 - 3 files changed, 13 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index ffbd264f190f119..24e001dadc38a25 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -3156,9 +3156,6 @@ def warn_unsupported_branch_protection_spec : Warning< def warn_attribute_underlying_type_mismatch : Warning< "underlying type %0 of enumeration %1 doesn't match bit-field type %2">, InGroup<BitFieldType>; -def warn_attribute_bool_bitfield_width : Warning< - "bit-field that holds a boolean value should have width of 1 instead of %0">, - InGroup<BitFieldWidth>; def warn_unsupported_target_attribute : Warning<"%select{unsupported|duplicate|unknown}0%select{| CPU|" diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index aa6858ccb891be4..7d8a46340cd9df4 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -5941,15 +5941,6 @@ static void handlePreferredTypeAttr(Sema &S, Decl *D, const ParsedAttr &AL) { << EnumUnderlyingType << QT << BitfieldType; return; } - } else if (QT->isBooleanType()) { - unsigned BitfieldWidth = - cast<FieldDecl>(D)->getBitWidthValue(S.getASTContext()); - if (BitfieldWidth != 1) { - S.Diag(cast<FieldDecl>(D)->getBitWidth()->getExprLoc(), - diag::warn_attribute_bool_bitfield_width) - << BitfieldWidth; - return; - } } D->addAttr(::new (S.Context) PreferredTypeAttr(S.Context, AL, ParmTSI)); diff --git a/clang/test/Sema/attr-preferred-type.cpp b/clang/test/Sema/attr-preferred-type.cpp index 1d98aa7c32f96c8..dfa7e636c21e94b 100644 --- a/clang/test/Sema/attr-preferred-type.cpp +++ b/clang/test/Sema/attr-preferred-type.cpp @@ -8,7 +8,6 @@ struct A { [[clang::preferred_type(E2)]] const unsigned b3 : 2; [[clang::preferred_type(bool)]] unsigned b4 : 1; [[clang::preferred_type(bool)]] unsigned b5 : 2; - // expected-warning@-1 {{bit-field that holds a boolean value should have width of 1 instead of 2}} [[clang::preferred_type()]] unsigned b6 : 2; // expected-error@-1 {{'preferred_type' attribute takes one argument}} [[clang::preferred_type]] unsigned b7 : 2; >From 300da8b8da522e482169c232d4cf449430787a2a Mon Sep 17 00:00:00 2001 From: Vlad Serebrennikov <serebrennikov.vladis...@gmail.com> Date: Fri, 20 Oct 2023 22:54:57 +0300 Subject: [PATCH 12/14] Fix formatting --- clang/lib/Sema/SemaDeclAttr.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 7d8a46340cd9df4..4aac9139c20e855 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -5919,7 +5919,8 @@ static void handlePreferredTypeAttr(Sema &S, Decl *D, const ParsedAttr &AL) { TypeSourceInfo *ParmTSI = nullptr; QualType QT = S.GetTypeFromParser(AL.getTypeArg(), &ParmTSI); assert(ParmTSI && "no type source info for attribute argument"); - S.RequireCompleteType(ParmTSI->getTypeLoc().getBeginLoc(), QT, diag::err_incomplete_type); + S.RequireCompleteType(ParmTSI->getTypeLoc().getBeginLoc(), QT, + diag::err_incomplete_type); if (QT->isEnumeralType()) { auto IsCorrespondingType = [&](QualType LHS, QualType RHS) { >From 32975c2c199bcf9b0a743cd0c29c68182c0a9c14 Mon Sep 17 00:00:00 2001 From: Vlad Serebrennikov <serebrennikov.vladis...@gmail.com> Date: Mon, 23 Oct 2023 19:36:52 +0300 Subject: [PATCH 13/14] Add a release note --- clang/docs/ReleaseNotes.rst | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index be7c8bf247f7af5..01a28944cfeb88d 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -227,6 +227,24 @@ Attribute Changes in Clang supports but that are never the result of default argument promotion, such as ``float``. (`#59824: <https://github.com/llvm/llvm-project/issues/59824>`_) +- Clang now supports ``[[clang::preferred_type(type-name)]]`` as an attribute + which can be applied to a bit-field. This attribute helps to map a bit-field + back to a particular type that may be better-suited to representing the bit- + field but cannot be used for other reasons and will impact the debug + information generated for the bit-field. This is most useful when mapping a + bit-field of basic integer type back to a ``bool`` or an enumeration type, + e.g., + + .. code-block:: c++ + + enum E { Apple, Orange, Pear }; + struct S { + [[clang::preferred_type(E)]] unsigned FruitKind : 2; + }; + + When viewing ``S::FruitKind`` in a debugger, it will behave as if the member + was declared as type ``E`` rather than ``unsigned``. + Improvements to Clang's diagnostics ----------------------------------- - Clang constexpr evaluator now prints template arguments when displaying >From b2bda5aa4697e568991eae40fd4818478ac61cff Mon Sep 17 00:00:00 2001 From: Vlad Serebrennikov <serebrennikov.vladis...@gmail.com> Date: Mon, 23 Oct 2023 19:47:09 +0300 Subject: [PATCH 14/14] Update the documentation using Aaron's suggestions --- clang/include/clang/Basic/AttrDocs.td | 44 +++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index aea316f0db9acee..3327182b54bc759 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -7238,9 +7238,47 @@ for debuggers to know to map a bit-field's value back to a particular enumeratio [[clang::preferred_type(bool)]] unsigned UseAlternateColorSpace : 1; } s = { Green, false }; -Without the attribute, a debugger is likely to display the value `1` for `ColorVal` -and `0` for `UseAlternateColorSpace`. With the attribute, the debugger may now -display `Green` and `false` instead. +Without the attribute, a debugger is likely to display the value ``1`` for ``ColorVal`` +and ``0`` for ``UseAlternateColorSpace``. With the attribute, the debugger may now +display ``Green`` and ``false`` instead. + +This can be used to map a bit-field to an arbitrary type that isn't integral +or an enumeration type. For example: + +.. code-block:: c++ + + struct A { + short a1; + short a2; + }; + + struct B { + [[clang::preferred_type(A)]] unsigned b1 : 32 = 0x000F'000C; + }; + +will associate the type ``A`` with the ``b1`` bit-field and is intended to display +something like this in the debugger: + +.. code-block:: text + + Process 2755547 stopped + * thread #1, name = 'test-preferred-', stop reason = step in + frame #0: 0x0000555555555148 test-preferred-type`main at test.cxx:13:14 + 10 int main() + 11 { + 12 B b; + -> 13 return b.b1; + 14 } + (lldb) v -T + (B) b = { + (A:32) b1 = { + (short) a1 = 12 + (short) a2 = 15 + } + } + +Note that debuggers may not be able to handle more complex mappings, and so +this usage is debugger-dependent. }]; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits