https://github.com/Endilll edited
https://github.com/llvm/llvm-project/pull/69104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -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.get
@@ -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
@@ -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.get
erichkeane wrote:
>I meant warning on something like https://godbolt.org/z/Ma17xjjc5. That
>doesn't seem like it should be that hard.
Yeah, that one wouldn't be too hard (and isn't a pattern I've ever seen TBH),
but doesn't solve the problem at hand, which is that it is very common to need
to
@@ -5910,6 +5910,30 @@ 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.get
@@ -5910,6 +5910,30 @@ 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.get
Endilll wrote:
>>@erichkeane I guess you'd be pretty happy if our enums were declared the
>> following way:
>Not really? That doesn't gain us the 'we must change where this is stored'
>situation like a preferred_type attribute would/could. We MIGHT be able to get
>away with a comment on t
erichkeane wrote:
> > > @erichkeane I guess you'd be pretty happy if our enums were declared the
> > > following way:
>
> > Not really? That doesn't gain us the 'we must change where this is stored'
> > situation like a preferred_type attribute would/could. We MIGHT be able to
> > get away wi
Endilll wrote:
Taking the example above, I think it would have to look the following way to
fully complement a check for bit-field width in `preferred_type`:
```cpp
enum StoredNameKind : unsigned _BitInt(3) {
StoredIdentifier = 0,
StoredObjCZeroArgSelector = Selector::ZeroArg,
Store
erichkeane wrote:
Ah, found it: https://godbolt.org/z/59sc87Y3Y
See how on the assignment to a bitfield we check to make sure the largest value
of the enum will fit in the bitfield? I'm saying I want us to do this EARLIER,
on declaration with this attribute.
https://github.com/llvm/llvm-proj
Endilll wrote:
> See how on the assignment to a bitfield we check to make sure the largest
> value of the enum will fit in the bitfield? I'm saying I want us to do this
> EARLIER, on declaration with this attribute.
I totally do. Thank you for pointing out to `-Wbitfield-enum-conversion`! But
erichkeane wrote:
> > See how on the assignment to a bitfield we check to make sure the largest
> > value of the enum will fit in the bitfield? I'm saying I want us to do this
> > EARLIER, on declaration with this attribute.
>
> I totally do. Thank you for pointing out to `-Wbitfield-enum-conv
Endilll wrote:
> when does someone have an enum value that they don't intend to ever be in the
> enum
In my previous comments
(https://github.com/llvm/llvm-project/pull/69104#issuecomment-1771167758,
https://github.com/llvm/llvm-project/pull/69104#issuecomment-1771204043) I
provided an examp
erichkeane wrote:
>In my previous comments
>(https://github.com/llvm/llvm-project/pull/69104#issuecomment-1771167758,
>https://github.com/llvm/llvm-project/pull/69104#issuecomment-1771204043) I
>provided an example of StoredNameKind enum that has enumerators 0 through 8 (9
>total, 4 bits to s
Endilll wrote:
> which brings up an additional concern/thing to deal with for the non_storable
> attribute (that is, do we apply it to assignments? how about
> exact-assignments like you've done there?).
Conservative approach would be to issue diagnostics based on `non_storable`
only when LHS
erichkeane wrote:
> > which brings up an additional concern/thing to deal with for the
> > non_storable attribute (that is, do we apply it to assignments? how about
> > exact-assignments like you've done there?).
>
> Conservative approach would be to issue diagnostics based on `non_storable`
AaronBallman wrote:
> Taking the example above, I think it would have to look the following way to
> fully complement a check for bit-field width in `preferred_type`:
>
> ```c++
> enum StoredNameKind : unsigned _BitInt(3) {
> ... snip ...
>```
> I think we can robustly diagnose new enumerator
erichkeane wrote:
>>I'm still thinking my way through a non_storable attribute, but on its face,
>>it seems like it could be overkill. I suspect (but haven't measured!) that
>>there is way more code out there that maps enumerations to bit-fields that
>>expect all members of the enumeration to
Endilll wrote:
> There's some danger here. _BitInt is a C23 feature as are enumerations with a
> fixed underlying type. Enumerations with a fixed underlying type explicitly
> disallow using a bit-precise integer type as the underlying type. See C23
> 6.7.2.2p4, which says in part, "For all the
AaronBallman wrote:
> > There's some danger here. _BitInt is a C23 feature as are enumerations with
> > a fixed underlying type. Enumerations with a fixed underlying type
> > explicitly disallow using a bit-precise integer type as the underlying
> > type. See C23 6.7.2.2p4, which says in part,
erichkeane wrote:
> > > There's some danger here. _BitInt is a C23 feature as are enumerations
> > > with a fixed underlying type. Enumerations with a fixed underlying type
> > > explicitly disallow using a bit-precise integer type as the underlying
> > > type. See C23 6.7.2.2p4, which says in
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
Date: Sun, 15 Oct 2023 13:14:55 +0300
Subject: [PATCH 1/6] [clang] Add clang::debug_info_type attribute
---
clang
@@ -5910,6 +5910,30 @@ 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.get
@@ -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.get
erichkeane wrote:
> I filed an issue for this, automatically mentioned above.
Fixed!
https://github.com/llvm/llvm-project/pull/69104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
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
Date: Sun, 15 Oct 2023 13:14:55 +0300
Subject: [PATCH 1/7] [clang] Add clang::debug_info_type attribute
---
clang
@@ -5910,6 +5910,30 @@ 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.get
@@ -5910,6 +5910,30 @@ 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.get
Endilll wrote:
I'm ignoring signed/unsigned mismatch as @erichkeane and @AaronBallman
suggested. The only outstanding aspect is the following diagnostic I added
today and haven't received feedback on:
```cpp
[[clang::preferred_type(bool)]] unsigned b4 : 1;
[[clang::preferred_type(bool)]] un
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
Date: Sun, 15 Oct 2023 13:14:55 +0300
Subject: [PATCH 1/8] [clang] Add clang::debug_info_type attribute
---
clang
erichkeane wrote:
> I'm ignoring signed/unsigned mismatch as @erichkeane and @AaronBallman
> suggested. The only outstanding aspect is the following diagnostic I added
> today and haven't received feedback on:
>
> ```c++
> [[clang::preferred_type(bool)]] unsigned b4 : 1;
> [[clang::preferr
Endilll wrote:
> While I think that warning is accurate, I somewhat question the value of the
> 'bool' as working on this type
I'm not sure what you mean by "working" here, but I'd like to highlight that we
have hundreds of single-bit bit-fields across Clang that would benefit from
`[[clang::
AaronBallman wrote:
> > While I think that warning is accurate, I somewhat question the value of
> > the 'bool' as working on this type
>
> I'm not sure what you mean by "working" here, but I'd like to highlight that
> we have hundreds of single-bit bit-fields across Clang that would benefit
erichkeane wrote:
>As I mentioned in
>https://github.com/llvm/llvm-project/pull/69104#discussion_r1365269451, I'm
>not putting any restrictions on type parameter of the attribute, which makes
>even more sense for more generic preferred_type.
>But I'm confused by the fact you are raising this
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
Date: Sun, 15 Oct 2023 13:14:55 +0300
Subject: [PATCH 1/9] [clang] Add clang::debug_info_type attribute
---
clang
@@ -5910,6 +5910,51 @@ static void handleBuiltinAliasAttr(Sema &S, Decl *D,
D->addAttr(::new (S.Context) BuiltinAliasAttr(S.Context, AL, Ident));
}
+static void handlePreferredTypeAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+ if (!AL.hasParsedType()) {
+S.Diag(AL.get
Endilll wrote:
> However, will this actually work in practice in the debugger? If not, perhaps
> we should limit to just integer and enumeration types for now, leaving the
> extension for the future.
I composed an example of that:
```cpp
struct A {
short a1;
short a2;
};
struct B {
[[cl
dwblaikie wrote:
> ```c++
> struct A {
> short a1;
> short a2;
> };
>
> struct B {
> [[clang::preferred_type(A)]] unsigned b1 : 32 = 0x000F'000C;
> };
>
> int main()
> {
> B b;
> return b.b1;
> }
> ```
An example where the layout doesn't match the normal struct layout might be
m
AaronBallman wrote:
> > I wonder if we should treat one-bit bit-fields as if they were bool
> > automatically (e.g., create this attribute implicitly in that case). How
> > often do we expect to see one-bit bit-fields that are arithmetic? I'm sure
> > it happens (to multiply against -1, 0, or
@@ -3153,6 +3153,12 @@ 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;
+def warn_attribute_und
@@ -3153,6 +3153,12 @@ 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;
+def warn_attribute_und
@@ -3153,6 +3153,12 @@ 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;
+def warn_attribute_und
@@ -3153,6 +3153,12 @@ 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;
+def warn_attribute_und
@@ -3153,6 +3153,12 @@ 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;
+def warn_attribute_und
@@ -5910,6 +5910,51 @@ static void handleBuiltinAliasAttr(Sema &S, Decl *D,
D->addAttr(::new (S.Context) BuiltinAliasAttr(S.Context, AL, Ident));
}
+static void handlePreferredTypeAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+ if (!AL.hasParsedType()) {
+S.Diag(AL.get
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
Date: Sun, 15 Oct 2023 13:14:55 +0300
Subject: [PATCH 01/10] [clang] Add clang::debug_info_type attribute
---
cla
@@ -5910,6 +5910,51 @@ static void handleBuiltinAliasAttr(Sema &S, Decl *D,
D->addAttr(::new (S.Context) BuiltinAliasAttr(S.Context, AL, Ident));
}
+static void handlePreferredTypeAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+ if (!AL.hasParsedType()) {
+S.Diag(AL.get
erichkeane wrote:
> > However, will this actually work in practice in the debugger? If not,
> > perhaps we should limit to just integer and enumeration types for now,
> > leaving the extension for the future.
>
> I composed an example of that:
>
> ```c++
> struct A {
> short a1;
> short a
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
Date: Sun, 15 Oct 2023 13:14:55 +0300
Subject: [PATCH 01/11] [clang] Add clang::debug_info_type attribute
---
cla
@@ -3153,6 +3153,12 @@ 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;
+def warn_attribute_und
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
Date: Sun, 15 Oct 2023 13:14:55 +0300
Subject: [PATCH 01/12] [clang] Add clang::debug_info_type attribute
---
cla
Endilll wrote:
> but I also see "you got what you asked for!" as being a reasonable defense to
> that.
That's my thinking indeed, and the reason why I opposed to Aaron's proposal to
implicitly mark 1-bit bit-fields as `preferred_type(bool)`.
https://github.com/llvm/llvm-project/pull/69104
___
@@ -5910,6 +5910,30 @@ 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.get
https://github.com/AaronBallman edited
https://github.com/llvm/llvm-project/pull/69104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -7219,6 +7219,31 @@ its underlying representation to be a WebAssembly
``funcref``.
}];
}
+def PreferredTypeDocumentation : Documentation {
+ let Category = DocCatField;
+ let Content = [{
+This attribute allows adjusting the type of a bit-field in debug information.
+T
@@ -7219,6 +7219,31 @@ its underlying representation to be a WebAssembly
``funcref``.
}];
}
+def PreferredTypeDocumentation : Documentation {
+ let Category = DocCatField;
+ let Content = [{
+This attribute allows adjusting the type of a bit-field in debug information.
+T
https://github.com/AaronBallman commented:
The changes should also come with a release note, but I'm generally happy with
the code. I did have some suggestions for changes to documentation though.
https://github.com/llvm/llvm-project/pull/69104
___
cf
AaronBallman wrote:
> The changes should also come with a release note, but I'm generally happy
> with the code. I did have some suggestions for changes to documentation
> though.
Suggestion for the release note would be something along these lines:
```
- Clang now supports ``[[clang::preferre
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
Date: Sun, 15 Oct 2023 13:14:55 +0300
Subject: [PATCH 01/14] [clang] Add clang::debug_info_type attribute
---
cla
@@ -7219,6 +7219,31 @@ its underlying representation to be a WebAssembly
``funcref``.
}];
}
+def PreferredTypeDocumentation : Documentation {
+ let Category = DocCatField;
+ let Content = [{
+This attribute allows adjusting the type of a bit-field in debug information.
+T
@@ -7219,6 +7219,31 @@ its underlying representation to be a WebAssembly
``funcref``.
}];
}
+def PreferredTypeDocumentation : Documentation {
+ let Category = DocCatField;
+ let Content = [{
+This attribute allows adjusting the type of a bit-field in debug information.
+T
Endilll wrote:
Thank you @AaronBallman for writing even more documentation!
https://github.com/llvm/llvm-project/pull/69104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/AaronBallman approved this pull request.
These changes LGTM, but please give @erichkeane a chance to chime in given his
previous feedback.
https://github.com/llvm/llvm-project/pull/69104
___
cfe-commits mailing list
cfe-commits@list
https://github.com/erichkeane approved this pull request.
LGTM, i think we're in an acceptable way forward, particularly on diagnostics.
https://github.com/llvm/llvm-project/pull/69104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lis
https://github.com/Endilll closed
https://github.com/llvm/llvm-project/pull/69104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
66 matches
Mail list logo
