https://github.com/AaronBallman closed
https://github.com/llvm/llvm-project/pull/84068
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -1,12 +1,19 @@
// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wenum-compare
-Wno-unused-comparison %s
// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wenum-compare
-Wno-unused-comparison %s
+// In C enumerators (i.e enumeration constants) have type int (until C23). In
o
@@ -1,12 +1,19 @@
// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wenum-compare
-Wno-unused-comparison %s
// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wenum-compare
-Wno-unused-comparison %s
+// In C enumerators (i.e enumeration constants) have type int (until C23). In
o
https://github.com/Kupa-Martin updated
https://github.com/llvm/llvm-project/pull/84068
>From cc6a1329761a8fa5ae0b9f51ef8b7f839d5edff2 Mon Sep 17 00:00:00 2001
From: 44-2-Kupa-Martin
Date: Tue, 5 Mar 2024 17:21:02 -0300
Subject: [PATCH] [Clang][Sema] Fix type of enumerators in incomplete
enumer
https://github.com/AaronBallman approved this pull request.
Aside from some formatting issues, this LGTM. Once you push up a fix for the
formatting, I'll land. Thank you!
https://github.com/llvm/llvm-project/pull/84068
___
cfe-commits mailing list
cfe
@@ -1,12 +1,19 @@
// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wenum-compare
-Wno-unused-comparison %s
// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wenum-compare
-Wno-unused-comparison %s
+// In C enumerators (i.e enumeration constants) have type int (until C23). In
o
@@ -1,12 +1,19 @@
// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wenum-compare
-Wno-unused-comparison %s
// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wenum-compare
-Wno-unused-comparison %s
+// In C enumerators (i.e enumeration constants) have type int (until C23). In
o
https://github.com/AaronBallman edited
https://github.com/llvm/llvm-project/pull/84068
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Kupa-Martin wrote:
> > @shafik We dont have a dedicated cpp test for this. I can add one if you
> > want, but clang/test/Sema/warn-compare-enum-types-mismatch.c runs clang
> > both on C and C++ mode, so I didnt think it necessary.
>
> I think we just a bug that demonstrates this issue: #84712
https://github.com/Kupa-Martin edited
https://github.com/llvm/llvm-project/pull/84068
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/Kupa-Martin updated
https://github.com/llvm/llvm-project/pull/84068
>From 57538a2e2b9446cf692d11386b3560225eb86ce4 Mon Sep 17 00:00:00 2001
From: 44-2-Kupa-Martin
Date: Tue, 5 Mar 2024 17:21:02 -0300
Subject: [PATCH] [Clang][Sema] Fix type of enumerators in incomplete
enumer
Sirraide wrote:
Regarding #84712: From what I can tell, the behaviour of cases such as
```c++
enum e1 { a };
enum e2 { b = a };
```
has changed between C++11 and C++14 (at least the wording is different starting
with C++14; there may be some other section that states the same for C++11, but
I c
shafik wrote:
> @shafik We dont have a dedicated cpp test for this. I can add one if you
> want, but clang/test/Sema/warn-compare-enum-types-mismatch.c runs clang both
> on C and C++ mode, so I didnt think it necessary.
I think we just a bug that demonstrates this issue:
https://github.com/ll
Kupa-Martin wrote:
@shafik We dont have a dedicated cpp test for this. I can add one if you want,
but clang/test/Sema/warn-compare-enum-types-mismatch.c runs clang both on C and
C++ mode, so I didnt think it necessary.
https://github.com/llvm/llvm-project/pull/84068
___
@@ -6,7 +6,9 @@ typedef enum EnumA {
} EnumA;
enum EnumB {
- B
+ B,
shafik wrote:
I think what I was asking, was do we have an equivalent C++ test that verifies
in a `.cpp` file that we also do not obtain a diagnostic for this.
https://github.com/llvm/llv
shafik wrote:
> So I believe:
>
> ```
> typedef enum EnumA {
> A
> } EnumA;
>
> enum EnumB {
> B,
> B1 = 1,
> B2 = A == B1
> };
> ```
>
> is not an enum compare warning in C++ because `B1` doesn't have an
> enumeration type due to the enumeration not being fully-defined, and is not
>
@@ -6,7 +6,9 @@ typedef enum EnumA {
} EnumA;
enum EnumB {
- B
+ B,
Kupa-Martin wrote:
done
https://github.com/llvm/llvm-project/pull/84068
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.ll
@@ -264,11 +264,14 @@ namespace {
}
QualType Expr::getEnumCoercedType(const ASTContext &Ctx) const {
- if (isa(this->getType()))
-return this->getType();
- else if (const auto *ECD = this->getEnumConstantDecl())
-return Ctx.getTypeDeclType(cast(ECD->getDeclContext())
https://github.com/Kupa-Martin updated
https://github.com/llvm/llvm-project/pull/84068
>From 4bbd5cf7eb1eeeaba8eed2ac4aba76cdbbcc671f Mon Sep 17 00:00:00 2001
From: 44-2-Kupa-Martin
Date: Tue, 5 Mar 2024 17:21:02 -0300
Subject: [PATCH] [Clang][Sema] Fix type of enumerators in incomplete
enumer
Kupa-Martin wrote:
> not an enum compare warning in C++ because `B1` doesn't have an enumeration
> type due to the enumeration not being fully-defined, and is not an enum
> compare warning in C because `A` has type `int` (due to p15) and `B1` has
> type `int` (due to p12).
Yes, you are correc
@@ -6,7 +6,9 @@ typedef enum EnumA {
} EnumA;
enum EnumB {
- B
+ B,
AaronBallman wrote:
It might be good to capture some of what I wrote in the review summary as a
comment here so it's clear why this is correct in both C and C++.
https://github.com/llvm/l
@@ -264,11 +264,14 @@ namespace {
}
QualType Expr::getEnumCoercedType(const ASTContext &Ctx) const {
- if (isa(this->getType()))
-return this->getType();
- else if (const auto *ECD = this->getEnumConstantDecl())
-return Ctx.getTypeDeclType(cast(ECD->getDeclContext())
https://github.com/AaronBallman edited
https://github.com/llvm/llvm-project/pull/84068
___
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.
Hmm, the rules are different between C and C++, but I think we may be okay. In
C++:
https://eel.is/c++draft/enum#dcl.enum-5.sentence-6
Following the closing brace of an enum-specifier, each enumerator has the type
of its enumeration.
Kupa-Martin wrote:
> Should we also have a C++ test for this fix?
clang/test/Sema/warn-compare-enum-types-mismatch.c should cover both C and C++.
Or do you mean some other kind of test?
https://github.com/llvm/llvm-project/pull/84068
___
cfe-commits
https://github.com/shafik commented:
Should we also have a C++ test for this fix?
https://github.com/llvm/llvm-project/pull/84068
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
erichkeane wrote:
> > > Also, this probably needs a release note.
> >
> >
> > If you want I'll add one but this bug has been on main no longer than a
> > week, so I didnt think it would be necessary.
>
> I see. Yeah, I don’t think we really need one if the bug was introduced and
> fixed in t
Sirraide wrote:
> > Also, this probably needs a release note.
>
> If you want I'll add one but this bug has been on main no longer than a week,
> so I didnt think it would be necessary.
I see. Yeah, I don’t think we really need one if the bug was introduced and
fixed in the same version.
htt
@@ -264,10 +264,13 @@ namespace {
}
QualType Expr::getEnumCoercedType(const ASTContext &Ctx) const {
- if (isa(this->getType()))
+ if (isa(this->getType())) {
return this->getType();
- else if (const auto *ECD = this->getEnumConstantDecl())
-return Ctx.getTypeDeclT
https://github.com/Kupa-Martin updated
https://github.com/llvm/llvm-project/pull/84068
>From 92c0ebc1fa419404dfa8c2497248cc35c2644c5b Mon Sep 17 00:00:00 2001
From: 44-2-Kupa-Martin
Date: Tue, 5 Mar 2024 17:21:02 -0300
Subject: [PATCH] [Clang][Sema] Fix type of enumerators in incomplete
enumer
Kupa-Martin wrote:
> Also, this probably needs a release note.
If you want I'll add one but this bug has been on main no longer than a week,
so I didnt think it would be necessary.
https://github.com/llvm/llvm-project/pull/84068
___
cfe-commits maili
Sirraide wrote:
Also, this probably needs a release note.
https://github.com/llvm/llvm-project/pull/84068
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -264,10 +264,13 @@ namespace {
}
QualType Expr::getEnumCoercedType(const ASTContext &Ctx) const {
- if (isa(this->getType()))
+ if (isa(this->getType())) {
return this->getType();
- else if (const auto *ECD = this->getEnumConstantDecl())
-return Ctx.getTypeDeclT
Kupa-Martin wrote:
@erichkeane I cant add the previous reviewers myself, could you do it for me
please?
https://github.com/llvm/llvm-project/pull/84068
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/list
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: None (Kupa-Martin)
Changes
Enumerators dont have the type of their enumeration before the closing brace.
In these cases Expr::getEnumCoercedType() incorrectly returned the enumeration
type.
Introduced in PR #81418
---
Full diff: https:/
https://github.com/Kupa-Martin created
https://github.com/llvm/llvm-project/pull/84068
Enumerators dont have the type of their enumeration before the closing brace.
In these cases Expr::getEnumCoercedType() incorrectly returned the enumeration
type.
Introduced in PR #81418
>From bc9f93713a07
36 matches
Mail list logo
