[clang] [CLANG-CL] ignores wpadded (PR #130182)

[Theo de Magalhaes via cfe-commits]

=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes 
Message-ID:
In-Reply-To: 


https://github.com/therealcoochieman updated 
https://github.com/llvm/llvm-project/pull/130182

>From 3a9bc908a791669c82f288ff745664411bf285ac Mon Sep 17 00:00:00 2001
From: Theo de Magalhaes 
Date: Thu, 6 Mar 2025 22:26:37 +0100
Subject: [PATCH 1/7] chore(clang-format): formatted
 clang/lib/AST/RecordLayoutBuilder.cpp

---
 clang/lib/AST/RecordLayoutBuilder.cpp | 199 --
 1 file changed, 94 insertions(+), 105 deletions(-)

diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp 
b/clang/lib/AST/RecordLayoutBuilder.cpp
index b8600e6a344a4..695771f67868d 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -43,7 +43,7 @@ struct BaseSubobjectInfo {
   bool IsVirtual;
 
   /// Bases - Information about the base subobjects.
-  SmallVector Bases;
+  SmallVector Bases;
 
   /// PrimaryVirtualBaseInfo - Holds the base info for the primary virtual base
   /// of this base info (if one exists).
@@ -77,8 +77,7 @@ struct ExternalLayout {
   /// Get the offset of the given field. The external source must provide
   /// entries for all fields in the record.
   uint64_t getExternalFieldOffset(const FieldDecl *FD) {
-assert(FieldOffsets.count(FD) &&
-   "Field does not have an external offset");
+assert(FieldOffsets.count(FD) && "Field does not have an external offset");
 return FieldOffsets[FD];
   }
 
@@ -167,16 +166,15 @@ class EmptySubobjectMap {
   CharUnits SizeOfLargestEmptySubobject;
 
   EmptySubobjectMap(const ASTContext &Context, const CXXRecordDecl *Class)
-  : Context(Context), CharWidth(Context.getCharWidth()), Class(Class) {
-  ComputeEmptySubobjectSizes();
+  : Context(Context), CharWidth(Context.getCharWidth()), Class(Class) {
+ComputeEmptySubobjectSizes();
   }
 
   /// CanPlaceBaseAtOffset - Return whether the given base class can be placed
   /// at the given offset.
   /// Returns false if placing the record will result in two components
   /// (direct or indirect) of the same type having the same offset.
-  bool CanPlaceBaseAtOffset(const BaseSubobjectInfo *Info,
-CharUnits Offset);
+  bool CanPlaceBaseAtOffset(const BaseSubobjectInfo *Info, CharUnits Offset);
 
   /// CanPlaceFieldAtOffset - Return whether a field can be placed at the given
   /// offset.
@@ -227,9 +225,8 @@ void EmptySubobjectMap::ComputeEmptySubobjectSizes() {
   }
 }
 
-bool
-EmptySubobjectMap::CanPlaceSubobjectAtOffset(const CXXRecordDecl *RD,
- CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceSubobjectAtOffset(const CXXRecordDecl *RD,
+  CharUnits Offset) const {
   // We only need to check empty bases.
   if (!RD->isEmpty())
 return true;
@@ -265,9 +262,8 @@ void EmptySubobjectMap::AddSubobjectAtOffset(const 
CXXRecordDecl *RD,
 MaxEmptyClassOffset = Offset;
 }
 
-bool
-EmptySubobjectMap::CanPlaceBaseSubobjectAtOffset(const BaseSubobjectInfo *Info,
- CharUnits Offset) {
+bool EmptySubobjectMap::CanPlaceBaseSubobjectAtOffset(
+const BaseSubobjectInfo *Info, CharUnits Offset) {
   // We don't have to keep looking past the maximum offset that's known to
   // contain an empty class.
   if (!AnyEmptySubobjectsBeyondOffset(Offset))
@@ -368,10 +364,9 @@ bool EmptySubobjectMap::CanPlaceBaseAtOffset(const 
BaseSubobjectInfo *Info,
   return true;
 }
 
-bool
-EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const CXXRecordDecl *RD,
-  const CXXRecordDecl *Class,
-  CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(
+const CXXRecordDecl *RD, const CXXRecordDecl *Class,
+CharUnits Offset) const {
   // We don't have to keep looking past the maximum offset that's known to
   // contain an empty class.
   if (!AnyEmptySubobjectsBeyondOffset(Offset))
@@ -418,9 +413,8 @@ EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const 
CXXRecordDecl *RD,
   return true;
 }
 
-bool
-EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const FieldDecl *FD,
-  CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const FieldDecl *FD,
+   CharUnits Offset) const 
{
   // We don't have to keep looking past the maximum offset that's known to
   // contain an empty class.
   if (!AnyEmptySubobjectsBeyondOffset(Offset))
@@ -560,7 +554,7 @@ void EmptySubobjectMap::UpdateEmptyFieldSubobjects(
   }
 }
 
-typedef llvm::SmallPtrSet ClassSetTy;
+typedef llvm::SmallPtrSet ClassSetTy;
 
 class ItaniumRecordLayoutBuilder {
 protected:
@@ -712,15 +706,13 @@ class ItaniumRecordLa

[clang] [CLANG-CL] ignores wpadded (PR #130182)

[Theo de Magalhaes via cfe-commits]

=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes 
Message-ID:
In-Reply-To: 


https://github.com/therealcoochieman updated 
https://github.com/llvm/llvm-project/pull/130182

>From 3a9bc908a791669c82f288ff745664411bf285ac Mon Sep 17 00:00:00 2001
From: Theo de Magalhaes 
Date: Thu, 6 Mar 2025 22:26:37 +0100
Subject: [PATCH 1/8] chore(clang-format): formatted
 clang/lib/AST/RecordLayoutBuilder.cpp

---
 clang/lib/AST/RecordLayoutBuilder.cpp | 199 --
 1 file changed, 94 insertions(+), 105 deletions(-)

diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp 
b/clang/lib/AST/RecordLayoutBuilder.cpp
index b8600e6a344a4..695771f67868d 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -43,7 +43,7 @@ struct BaseSubobjectInfo {
   bool IsVirtual;
 
   /// Bases - Information about the base subobjects.
-  SmallVector Bases;
+  SmallVector Bases;
 
   /// PrimaryVirtualBaseInfo - Holds the base info for the primary virtual base
   /// of this base info (if one exists).
@@ -77,8 +77,7 @@ struct ExternalLayout {
   /// Get the offset of the given field. The external source must provide
   /// entries for all fields in the record.
   uint64_t getExternalFieldOffset(const FieldDecl *FD) {
-assert(FieldOffsets.count(FD) &&
-   "Field does not have an external offset");
+assert(FieldOffsets.count(FD) && "Field does not have an external offset");
 return FieldOffsets[FD];
   }
 
@@ -167,16 +166,15 @@ class EmptySubobjectMap {
   CharUnits SizeOfLargestEmptySubobject;
 
   EmptySubobjectMap(const ASTContext &Context, const CXXRecordDecl *Class)
-  : Context(Context), CharWidth(Context.getCharWidth()), Class(Class) {
-  ComputeEmptySubobjectSizes();
+  : Context(Context), CharWidth(Context.getCharWidth()), Class(Class) {
+ComputeEmptySubobjectSizes();
   }
 
   /// CanPlaceBaseAtOffset - Return whether the given base class can be placed
   /// at the given offset.
   /// Returns false if placing the record will result in two components
   /// (direct or indirect) of the same type having the same offset.
-  bool CanPlaceBaseAtOffset(const BaseSubobjectInfo *Info,
-CharUnits Offset);
+  bool CanPlaceBaseAtOffset(const BaseSubobjectInfo *Info, CharUnits Offset);
 
   /// CanPlaceFieldAtOffset - Return whether a field can be placed at the given
   /// offset.
@@ -227,9 +225,8 @@ void EmptySubobjectMap::ComputeEmptySubobjectSizes() {
   }
 }
 
-bool
-EmptySubobjectMap::CanPlaceSubobjectAtOffset(const CXXRecordDecl *RD,
- CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceSubobjectAtOffset(const CXXRecordDecl *RD,
+  CharUnits Offset) const {
   // We only need to check empty bases.
   if (!RD->isEmpty())
 return true;
@@ -265,9 +262,8 @@ void EmptySubobjectMap::AddSubobjectAtOffset(const 
CXXRecordDecl *RD,
 MaxEmptyClassOffset = Offset;
 }
 
-bool
-EmptySubobjectMap::CanPlaceBaseSubobjectAtOffset(const BaseSubobjectInfo *Info,
- CharUnits Offset) {
+bool EmptySubobjectMap::CanPlaceBaseSubobjectAtOffset(
+const BaseSubobjectInfo *Info, CharUnits Offset) {
   // We don't have to keep looking past the maximum offset that's known to
   // contain an empty class.
   if (!AnyEmptySubobjectsBeyondOffset(Offset))
@@ -368,10 +364,9 @@ bool EmptySubobjectMap::CanPlaceBaseAtOffset(const 
BaseSubobjectInfo *Info,
   return true;
 }
 
-bool
-EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const CXXRecordDecl *RD,
-  const CXXRecordDecl *Class,
-  CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(
+const CXXRecordDecl *RD, const CXXRecordDecl *Class,
+CharUnits Offset) const {
   // We don't have to keep looking past the maximum offset that's known to
   // contain an empty class.
   if (!AnyEmptySubobjectsBeyondOffset(Offset))
@@ -418,9 +413,8 @@ EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const 
CXXRecordDecl *RD,
   return true;
 }
 
-bool
-EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const FieldDecl *FD,
-  CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const FieldDecl *FD,
+   CharUnits Offset) const 
{
   // We don't have to keep looking past the maximum offset that's known to
   // contain an empty class.
   if (!AnyEmptySubobjectsBeyondOffset(Offset))
@@ -560,7 +554,7 @@ void EmptySubobjectMap::UpdateEmptyFieldSubobjects(
   }
 }
 
-typedef llvm::SmallPtrSet ClassSetTy;
+typedef llvm::SmallPtrSet ClassSetTy;
 
 class ItaniumRecordLayoutBuilder {
 protected:
@@ -712,15 

[clang] [CLANG-CL] ignores wpadded (PR #130182)

[Theo de Magalhaes via cfe-commits]

=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes 
Message-ID:
In-Reply-To: 


https://github.com/therealcoochieman updated 
https://github.com/llvm/llvm-project/pull/130182

>From 09572af03c07966925a3456a7d51ad9b3abb750b Mon Sep 17 00:00:00 2001
From: Theo de Magalhaes 
Date: Thu, 6 Mar 2025 22:26:37 +0100
Subject: [PATCH 01/11] chore(clang-format): formatted
 clang/lib/AST/RecordLayoutBuilder.cpp

---
 clang/lib/AST/RecordLayoutBuilder.cpp | 199 --
 1 file changed, 94 insertions(+), 105 deletions(-)

diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp 
b/clang/lib/AST/RecordLayoutBuilder.cpp
index 3e756ab9b9bfe..9740c8e4cdd65 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -43,7 +43,7 @@ struct BaseSubobjectInfo {
   bool IsVirtual;
 
   /// Bases - Information about the base subobjects.
-  SmallVector Bases;
+  SmallVector Bases;
 
   /// PrimaryVirtualBaseInfo - Holds the base info for the primary virtual base
   /// of this base info (if one exists).
@@ -77,8 +77,7 @@ struct ExternalLayout {
   /// Get the offset of the given field. The external source must provide
   /// entries for all fields in the record.
   uint64_t getExternalFieldOffset(const FieldDecl *FD) {
-assert(FieldOffsets.count(FD) &&
-   "Field does not have an external offset");
+assert(FieldOffsets.count(FD) && "Field does not have an external offset");
 return FieldOffsets[FD];
   }
 
@@ -167,16 +166,15 @@ class EmptySubobjectMap {
   CharUnits SizeOfLargestEmptySubobject;
 
   EmptySubobjectMap(const ASTContext &Context, const CXXRecordDecl *Class)
-  : Context(Context), CharWidth(Context.getCharWidth()), Class(Class) {
-  ComputeEmptySubobjectSizes();
+  : Context(Context), CharWidth(Context.getCharWidth()), Class(Class) {
+ComputeEmptySubobjectSizes();
   }
 
   /// CanPlaceBaseAtOffset - Return whether the given base class can be placed
   /// at the given offset.
   /// Returns false if placing the record will result in two components
   /// (direct or indirect) of the same type having the same offset.
-  bool CanPlaceBaseAtOffset(const BaseSubobjectInfo *Info,
-CharUnits Offset);
+  bool CanPlaceBaseAtOffset(const BaseSubobjectInfo *Info, CharUnits Offset);
 
   /// CanPlaceFieldAtOffset - Return whether a field can be placed at the given
   /// offset.
@@ -227,9 +225,8 @@ void EmptySubobjectMap::ComputeEmptySubobjectSizes() {
   }
 }
 
-bool
-EmptySubobjectMap::CanPlaceSubobjectAtOffset(const CXXRecordDecl *RD,
- CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceSubobjectAtOffset(const CXXRecordDecl *RD,
+  CharUnits Offset) const {
   // We only need to check empty bases.
   if (!RD->isEmpty())
 return true;
@@ -265,9 +262,8 @@ void EmptySubobjectMap::AddSubobjectAtOffset(const 
CXXRecordDecl *RD,
 MaxEmptyClassOffset = Offset;
 }
 
-bool
-EmptySubobjectMap::CanPlaceBaseSubobjectAtOffset(const BaseSubobjectInfo *Info,
- CharUnits Offset) {
+bool EmptySubobjectMap::CanPlaceBaseSubobjectAtOffset(
+const BaseSubobjectInfo *Info, CharUnits Offset) {
   // We don't have to keep looking past the maximum offset that's known to
   // contain an empty class.
   if (!AnyEmptySubobjectsBeyondOffset(Offset))
@@ -368,10 +364,9 @@ bool EmptySubobjectMap::CanPlaceBaseAtOffset(const 
BaseSubobjectInfo *Info,
   return true;
 }
 
-bool
-EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const CXXRecordDecl *RD,
-  const CXXRecordDecl *Class,
-  CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(
+const CXXRecordDecl *RD, const CXXRecordDecl *Class,
+CharUnits Offset) const {
   // We don't have to keep looking past the maximum offset that's known to
   // contain an empty class.
   if (!AnyEmptySubobjectsBeyondOffset(Offset))
@@ -418,9 +413,8 @@ EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const 
CXXRecordDecl *RD,
   return true;
 }
 
-bool
-EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const FieldDecl *FD,
-  CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const FieldDecl *FD,
+   CharUnits Offset) const 
{
   // We don't have to keep looking past the maximum offset that's known to
   // contain an empty class.
   if (!AnyEmptySubobjectsBeyondOffset(Offset))
@@ -560,7 +554,7 @@ void EmptySubobjectMap::UpdateEmptyFieldSubobjects(
   }
 }
 
-typedef llvm::SmallPtrSet ClassSetTy;
+t

[clang] [CLANG-CL] ignores wpadded (PR #130182)

[Theo de Magalhaes via cfe-commits]

=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes 
Message-ID:
In-Reply-To: 


https://github.com/therealcoochieman updated 
https://github.com/llvm/llvm-project/pull/130182

>From 3a9bc908a791669c82f288ff745664411bf285ac Mon Sep 17 00:00:00 2001
From: Theo de Magalhaes 
Date: Thu, 6 Mar 2025 22:26:37 +0100
Subject: [PATCH 1/9] chore(clang-format): formatted
 clang/lib/AST/RecordLayoutBuilder.cpp

---
 clang/lib/AST/RecordLayoutBuilder.cpp | 199 --
 1 file changed, 94 insertions(+), 105 deletions(-)

diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp 
b/clang/lib/AST/RecordLayoutBuilder.cpp
index b8600e6a344a4..695771f67868d 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -43,7 +43,7 @@ struct BaseSubobjectInfo {
   bool IsVirtual;
 
   /// Bases - Information about the base subobjects.
-  SmallVector Bases;
+  SmallVector Bases;
 
   /// PrimaryVirtualBaseInfo - Holds the base info for the primary virtual base
   /// of this base info (if one exists).
@@ -77,8 +77,7 @@ struct ExternalLayout {
   /// Get the offset of the given field. The external source must provide
   /// entries for all fields in the record.
   uint64_t getExternalFieldOffset(const FieldDecl *FD) {
-assert(FieldOffsets.count(FD) &&
-   "Field does not have an external offset");
+assert(FieldOffsets.count(FD) && "Field does not have an external offset");
 return FieldOffsets[FD];
   }
 
@@ -167,16 +166,15 @@ class EmptySubobjectMap {
   CharUnits SizeOfLargestEmptySubobject;
 
   EmptySubobjectMap(const ASTContext &Context, const CXXRecordDecl *Class)
-  : Context(Context), CharWidth(Context.getCharWidth()), Class(Class) {
-  ComputeEmptySubobjectSizes();
+  : Context(Context), CharWidth(Context.getCharWidth()), Class(Class) {
+ComputeEmptySubobjectSizes();
   }
 
   /// CanPlaceBaseAtOffset - Return whether the given base class can be placed
   /// at the given offset.
   /// Returns false if placing the record will result in two components
   /// (direct or indirect) of the same type having the same offset.
-  bool CanPlaceBaseAtOffset(const BaseSubobjectInfo *Info,
-CharUnits Offset);
+  bool CanPlaceBaseAtOffset(const BaseSubobjectInfo *Info, CharUnits Offset);
 
   /// CanPlaceFieldAtOffset - Return whether a field can be placed at the given
   /// offset.
@@ -227,9 +225,8 @@ void EmptySubobjectMap::ComputeEmptySubobjectSizes() {
   }
 }
 
-bool
-EmptySubobjectMap::CanPlaceSubobjectAtOffset(const CXXRecordDecl *RD,
- CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceSubobjectAtOffset(const CXXRecordDecl *RD,
+  CharUnits Offset) const {
   // We only need to check empty bases.
   if (!RD->isEmpty())
 return true;
@@ -265,9 +262,8 @@ void EmptySubobjectMap::AddSubobjectAtOffset(const 
CXXRecordDecl *RD,
 MaxEmptyClassOffset = Offset;
 }
 
-bool
-EmptySubobjectMap::CanPlaceBaseSubobjectAtOffset(const BaseSubobjectInfo *Info,
- CharUnits Offset) {
+bool EmptySubobjectMap::CanPlaceBaseSubobjectAtOffset(
+const BaseSubobjectInfo *Info, CharUnits Offset) {
   // We don't have to keep looking past the maximum offset that's known to
   // contain an empty class.
   if (!AnyEmptySubobjectsBeyondOffset(Offset))
@@ -368,10 +364,9 @@ bool EmptySubobjectMap::CanPlaceBaseAtOffset(const 
BaseSubobjectInfo *Info,
   return true;
 }
 
-bool
-EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const CXXRecordDecl *RD,
-  const CXXRecordDecl *Class,
-  CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(
+const CXXRecordDecl *RD, const CXXRecordDecl *Class,
+CharUnits Offset) const {
   // We don't have to keep looking past the maximum offset that's known to
   // contain an empty class.
   if (!AnyEmptySubobjectsBeyondOffset(Offset))
@@ -418,9 +413,8 @@ EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const 
CXXRecordDecl *RD,
   return true;
 }
 
-bool
-EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const FieldDecl *FD,
-  CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const FieldDecl *FD,
+   CharUnits Offset) const 
{
   // We don't have to keep looking past the maximum offset that's known to
   // contain an empty class.
   if (!AnyEmptySubobjectsBeyondOffset(Offset))
@@ -560,7 +554,7 @@ void EmptySubobjectMap::UpdateEmptyFieldSubobjects(
   }
 }
 
-typedef llvm::SmallPtrSet ClassSetTy;
+typedef llvm::SmallPtrSet ClassSetTy;
 
 class ItaniumRecordLayoutB

[clang] [CLANG-CL] ignores wpadded (PR #130182)

[Theo de Magalhaes via cfe-commits]

=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes 
Message-ID:
In-Reply-To: 



@@ -0,0 +1,12 @@
+// RUN: %clang_cl -Wpadded -Wno-msvc-not-found -fsyntax-only -- %s 2>&1 | 
FileCheck -check-prefix=WARN %s

therealcoochieman wrote:

Hello @efriedma-quic, I hope you don’t mind the ping.
While waiting for your response, I’ve added more bitfield tests.
I’ve also fixed #131647 in a different branch. Once these tests are validated, 
I’ll rebase this PR onto that branch and merge the two test files.

Would you like me to notify you directly when the next PR is ready for review?
Thank you.

https://github.com/llvm/llvm-project/pull/130182
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CLANG-CL] ignores Wpadded (PR #134426)

[Theo de Magalhaes via cfe-commits]

=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,Theo de
 Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes 
Message-ID:
In-Reply-To: 


https://github.com/theomagellan updated 
https://github.com/llvm/llvm-project/pull/134426

>From 09572af03c07966925a3456a7d51ad9b3abb750b Mon Sep 17 00:00:00 2001
From: Theo de Magalhaes 
Date: Thu, 6 Mar 2025 22:26:37 +0100
Subject: [PATCH 01/15] chore(clang-format): formatted
 clang/lib/AST/RecordLayoutBuilder.cpp

---
 clang/lib/AST/RecordLayoutBuilder.cpp | 199 --
 1 file changed, 94 insertions(+), 105 deletions(-)

diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp 
b/clang/lib/AST/RecordLayoutBuilder.cpp
index 3e756ab9b9bfe..9740c8e4cdd65 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -43,7 +43,7 @@ struct BaseSubobjectInfo {
   bool IsVirtual;
 
   /// Bases - Information about the base subobjects.
-  SmallVector Bases;
+  SmallVector Bases;
 
   /// PrimaryVirtualBaseInfo - Holds the base info for the primary virtual base
   /// of this base info (if one exists).
@@ -77,8 +77,7 @@ struct ExternalLayout {
   /// Get the offset of the given field. The external source must provide
   /// entries for all fields in the record.
   uint64_t getExternalFieldOffset(const FieldDecl *FD) {
-assert(FieldOffsets.count(FD) &&
-   "Field does not have an external offset");
+assert(FieldOffsets.count(FD) && "Field does not have an external offset");
 return FieldOffsets[FD];
   }
 
@@ -167,16 +166,15 @@ class EmptySubobjectMap {
   CharUnits SizeOfLargestEmptySubobject;
 
   EmptySubobjectMap(const ASTContext &Context, const CXXRecordDecl *Class)
-  : Context(Context), CharWidth(Context.getCharWidth()), Class(Class) {
-  ComputeEmptySubobjectSizes();
+  : Context(Context), CharWidth(Context.getCharWidth()), Class(Class) {
+ComputeEmptySubobjectSizes();
   }
 
   /// CanPlaceBaseAtOffset - Return whether the given base class can be placed
   /// at the given offset.
   /// Returns false if placing the record will result in two components
   /// (direct or indirect) of the same type having the same offset.
-  bool CanPlaceBaseAtOffset(const BaseSubobjectInfo *Info,
-CharUnits Offset);
+  bool CanPlaceBaseAtOffset(const BaseSubobjectInfo *Info, CharUnits Offset);
 
   /// CanPlaceFieldAtOffset - Return whether a field can be placed at the given
   /// offset.
@@ -227,9 +225,8 @@ void EmptySubobjectMap::ComputeEmptySubobjectSizes() {
   }
 }
 
-bool
-EmptySubobjectMap::CanPlaceSubobjectAtOffset(const CXXRecordDecl *RD,
- CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceSubobjectAtOffset(const CXXRecordDecl *RD,
+  CharUnits Offset) const {
   // We only need to check empty bases.
   if (!RD->isEmpty())
 return true;
@@ -265,9 +262,8 @@ void EmptySubobjectMap::AddSubobjectAtOffset(const 
CXXRecordDecl *RD,
 MaxEmptyClassOffset = Offset;
 }
 
-bool
-EmptySubobjectMap::CanPlaceBaseSubobjectAtOffset(const BaseSubobjectInfo *Info,
- CharUnits Offset) {
+bool EmptySubobjectMap::CanPlaceBaseSubobjectAtOffset(
+const BaseSubobjectInfo *Info, CharUnits Offset) {
   // We don't have to keep looking past the maximum offset that's known to
   // contain an empty class.
   if (!AnyEmptySubobjectsBeyondOffset(Offset))
@@ -368,10 +364,9 @@ bool EmptySubobjectMap::CanPlaceBaseAtOffset(const 
BaseSubobjectInfo *Info,
   return true;
 }
 
-bool
-EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const CXXRecordDecl *RD,
-  const CXXRecordDecl *Class,
-  CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(
+const CXXRecordDecl *RD, const CXXRecordDecl *Class,
+CharUnits Offset) const {
   // We don't have to keep looking past the maximum offset that's known to
   // contain an empty class.
   if (!AnyEmptySubobjectsBeyondOffset(Offset))
@@ -418,9 +413,8 @@ EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const 
CXXRecordDecl *RD,
   return true;
 }
 
-bool
-EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const FieldDecl *FD,
-  CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const FieldDecl *FD,
+   CharUnits Offset) const 
{
   // We don't have to keep looking past the maximum offset that's known to
   // contain an empty class.
   if (!AnyEmptySubobjectsBeyondOffset(Offset))
@@ -560,7 +5

[clang] [CLANG-CL] ignores Wpadded (PR #134426)

[Theo de Magalhaes via cfe-commits]

=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,Theo de
 Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes 
Message-ID:
In-Reply-To: 



@@ -3231,6 +3259,22 @@ void MicrosoftRecordLayoutBuilder::finalizeLayout(const 
RecordDecl *RD) {
 Size = Context.toCharUnitsFromBits(External.Size);
 if (External.Align)
   Alignment = Context.toCharUnitsFromBits(External.Align);
+return;
+  }
+  unsigned CharBitNum = Context.getTargetInfo().getCharWidth();
+  uint64_t DataSizeInBits = Context.toBits(DataSize);

theomagellan wrote:

You were right, using DataSize did create issues while using certain alignment 
attributes. I added some in the test file.
Now, 1 byte is artificially added to `UnpaddedSizeInBits` when `Size` is zero 
before aligning the empty struct to its alignment. This allows to keep the 
checks as they used to be (using Size instead of DataSize) and also takes care 
of the empty struct case.

Thank you for catching that case!

https://github.com/llvm/llvm-project/pull/134426
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CLANG-CL] ignores wpadded (PR #130182)

[Theo de Magalhaes via cfe-commits]

=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes 
Message-ID:
In-Reply-To: 


https://github.com/therealcoochieman updated 
https://github.com/llvm/llvm-project/pull/130182

>From 3a9bc908a791669c82f288ff745664411bf285ac Mon Sep 17 00:00:00 2001
From: Theo de Magalhaes 
Date: Thu, 6 Mar 2025 22:26:37 +0100
Subject: [PATCH 01/11] chore(clang-format): formatted
 clang/lib/AST/RecordLayoutBuilder.cpp

---
 clang/lib/AST/RecordLayoutBuilder.cpp | 199 --
 1 file changed, 94 insertions(+), 105 deletions(-)

diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp 
b/clang/lib/AST/RecordLayoutBuilder.cpp
index b8600e6a344a4..695771f67868d 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -43,7 +43,7 @@ struct BaseSubobjectInfo {
   bool IsVirtual;
 
   /// Bases - Information about the base subobjects.
-  SmallVector Bases;
+  SmallVector Bases;
 
   /// PrimaryVirtualBaseInfo - Holds the base info for the primary virtual base
   /// of this base info (if one exists).
@@ -77,8 +77,7 @@ struct ExternalLayout {
   /// Get the offset of the given field. The external source must provide
   /// entries for all fields in the record.
   uint64_t getExternalFieldOffset(const FieldDecl *FD) {
-assert(FieldOffsets.count(FD) &&
-   "Field does not have an external offset");
+assert(FieldOffsets.count(FD) && "Field does not have an external offset");
 return FieldOffsets[FD];
   }
 
@@ -167,16 +166,15 @@ class EmptySubobjectMap {
   CharUnits SizeOfLargestEmptySubobject;
 
   EmptySubobjectMap(const ASTContext &Context, const CXXRecordDecl *Class)
-  : Context(Context), CharWidth(Context.getCharWidth()), Class(Class) {
-  ComputeEmptySubobjectSizes();
+  : Context(Context), CharWidth(Context.getCharWidth()), Class(Class) {
+ComputeEmptySubobjectSizes();
   }
 
   /// CanPlaceBaseAtOffset - Return whether the given base class can be placed
   /// at the given offset.
   /// Returns false if placing the record will result in two components
   /// (direct or indirect) of the same type having the same offset.
-  bool CanPlaceBaseAtOffset(const BaseSubobjectInfo *Info,
-CharUnits Offset);
+  bool CanPlaceBaseAtOffset(const BaseSubobjectInfo *Info, CharUnits Offset);
 
   /// CanPlaceFieldAtOffset - Return whether a field can be placed at the given
   /// offset.
@@ -227,9 +225,8 @@ void EmptySubobjectMap::ComputeEmptySubobjectSizes() {
   }
 }
 
-bool
-EmptySubobjectMap::CanPlaceSubobjectAtOffset(const CXXRecordDecl *RD,
- CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceSubobjectAtOffset(const CXXRecordDecl *RD,
+  CharUnits Offset) const {
   // We only need to check empty bases.
   if (!RD->isEmpty())
 return true;
@@ -265,9 +262,8 @@ void EmptySubobjectMap::AddSubobjectAtOffset(const 
CXXRecordDecl *RD,
 MaxEmptyClassOffset = Offset;
 }
 
-bool
-EmptySubobjectMap::CanPlaceBaseSubobjectAtOffset(const BaseSubobjectInfo *Info,
- CharUnits Offset) {
+bool EmptySubobjectMap::CanPlaceBaseSubobjectAtOffset(
+const BaseSubobjectInfo *Info, CharUnits Offset) {
   // We don't have to keep looking past the maximum offset that's known to
   // contain an empty class.
   if (!AnyEmptySubobjectsBeyondOffset(Offset))
@@ -368,10 +364,9 @@ bool EmptySubobjectMap::CanPlaceBaseAtOffset(const 
BaseSubobjectInfo *Info,
   return true;
 }
 
-bool
-EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const CXXRecordDecl *RD,
-  const CXXRecordDecl *Class,
-  CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(
+const CXXRecordDecl *RD, const CXXRecordDecl *Class,
+CharUnits Offset) const {
   // We don't have to keep looking past the maximum offset that's known to
   // contain an empty class.
   if (!AnyEmptySubobjectsBeyondOffset(Offset))
@@ -418,9 +413,8 @@ EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const 
CXXRecordDecl *RD,
   return true;
 }
 
-bool
-EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const FieldDecl *FD,
-  CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const FieldDecl *FD,
+   CharUnits Offset) const 
{
   // We don't have to keep looking past the maximum offset that's known to
   // contain an empty class.
   if (!AnyEmptySubobjectsBeyondOffset(Offset))
@@ -560,7 +554,7 @@ void EmptySubobjectMap::UpdateEmptyFieldSubobjects(
   }
 }
 
-typedef llvm::SmallPtrSet ClassSetTy;
+t

[clang] [CLANG][MS-STRUCT] bitfield padding warning presents padding to exact bit count (PR #136062)

[Theo de Magalhaes via cfe-commits]

https://github.com/theomagellan created 
https://github.com/llvm/llvm-project/pull/136062

Aims to fix #131647.

>From 842f0fbed0043ad0aa1679d8a30bc13d64eb25cf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Th=C3=A9o=20de=20Magalhaes?= 
Date: Thu, 17 Apr 2025 01:45:12 +0200
Subject: [PATCH] fix(ms_struct): bitfield padding warning presents padding to
 exact bit count

---
 clang/lib/AST/RecordLayoutBuilder.cpp   |  4 +++-
 clang/test/SemaCXX/windows-Wpadded-bitfield.cpp | 15 +++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp 
b/clang/lib/AST/RecordLayoutBuilder.cpp
index ea353f88a8aec..ca08e186f4ff2 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1538,6 +1538,7 @@ void ItaniumRecordLayoutBuilder::LayoutBitField(const 
FieldDecl *D) {
   uint64_t StorageUnitSize = FieldInfo.Width;
   unsigned FieldAlign = FieldInfo.Align;
   bool AlignIsRequired = FieldInfo.isAlignRequired();
+  unsigned char PaddingInLastUnit = 0;
 
   // UnfilledBitsInLastUnit is the difference between the end of the
   // last allocated bitfield (i.e. the first bit offset available for
@@ -1610,6 +1611,7 @@ void ItaniumRecordLayoutBuilder::LayoutBitField(const 
FieldDecl *D) {
   if (!LastBitfieldStorageUnitSize && !FieldSize)
 FieldAlign = 1;
 
+  PaddingInLastUnit = UnfilledBitsInLastUnit;
   UnfilledBitsInLastUnit = 0;
   LastBitfieldStorageUnitSize = 0;
 }
@@ -1706,7 +1708,7 @@ void ItaniumRecordLayoutBuilder::LayoutBitField(const 
FieldDecl *D) {
   // For purposes of diagnostics, we're going to simultaneously
   // compute the field offsets that we would have used if we weren't
   // adding any alignment padding or if the field weren't packed.
-  uint64_t UnpaddedFieldOffset = FieldOffset;
+  uint64_t UnpaddedFieldOffset = FieldOffset - PaddingInLastUnit;
   uint64_t UnpackedFieldOffset = FieldOffset;
 
   // Check if we need to add padding to fit the bitfield within an
diff --git a/clang/test/SemaCXX/windows-Wpadded-bitfield.cpp 
b/clang/test/SemaCXX/windows-Wpadded-bitfield.cpp
index ee5a57124eca5..0b88b4c170617 100644
--- a/clang/test/SemaCXX/windows-Wpadded-bitfield.cpp
+++ b/clang/test/SemaCXX/windows-Wpadded-bitfield.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-windows-msvc -fsyntax-only -verify -Wpadded 
%s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -verify -Wpadded %s
 
 struct __attribute__((ms_struct)) BitfieldStruct { // expected-warning 
{{padding size of 'BitfieldStruct' with 3 bytes to alignment boundary}}
   char c : 1;
@@ -24,9 +25,23 @@ struct __attribute__((ms_struct)) DifferentUnitSizeBitfield 
{ // expected-warnin
   char i; // expected-warning {{padding struct 'DifferentUnitSizeBitfield' 
with 31 bits to align 'i'}}
 };
 
+struct __attribute__((ms_struct)) Foo { // expected-warning {{padding size of 
'Foo' with 63 bits to alignment boundary}}
+  long long x;
+  char a : 1;
+  long long b : 1; // expected-warning {{padding struct 'Foo' with 63 bits to 
align 'b'}}
+};
+
+struct __attribute__((ms_struct)) SameUnitSizeMultiple { // expected-warning 
{{padding size of 'SameUnitSizeMultiple' with 2 bits to alignment boundary}}
+  char c : 1;
+  char cc : 2;
+  char ccc : 3;
+};
+
 int main() {
   BitfieldStruct b;
   SevenBitfieldStruct s;
   SameUnitSizeBitfield su;
   DifferentUnitSizeBitfield du;
+  Foo f;
+  SameUnitSizeMultiple susm;
 }

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CLANG][MS-STRUCT] bitfield padding warning presents padding to exact bit count (PR #136062)

[Theo de Magalhaes via cfe-commits]

https://github.com/theomagellan converted_to_draft 
https://github.com/llvm/llvm-project/pull/136062
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CLANG][MS-STRUCT] bitfield padding warning presents padding to exact bit count (PR #136062)

[Theo de Magalhaes via cfe-commits]

https://github.com/theomagellan ready_for_review 
https://github.com/llvm/llvm-project/pull/136062
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CLANG][MS-STRUCT] bitfield padding warning presents padding to exact bit count (PR #136062)

[Theo de Magalhaes via cfe-commits]

theomagellan wrote:

Hello @efriedma-quic, hope you don’t mind the ping.

This patch addresses the padded bitfield warning, which was reporting the wrong 
number of padded bits when using `ms_struct` and two consecutive bit-fields 
under the Itanium ABI.

I originally planned to merge `SemaCXX/windows-Wpadded.cc` and 
`SemaCXX/windows-Wpadded-bitfield.cc` into a single test file, but after some 
consideration, I think keeping them separate makes the coverage clearer. What 
do you think?

Thank you!

https://github.com/llvm/llvm-project/pull/136062
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CLANG][MS-STRUCT] bitfield padding warning presents padding to exact bit count (PR #136062)

[Theo de Magalhaes via cfe-commits]

https://github.com/theomagellan updated 
https://github.com/llvm/llvm-project/pull/136062

>From 842f0fbed0043ad0aa1679d8a30bc13d64eb25cf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Th=C3=A9o=20de=20Magalhaes?= 
Date: Thu, 17 Apr 2025 01:45:12 +0200
Subject: [PATCH] fix(ms_struct): bitfield padding warning presents padding to
 exact bit count

---
 clang/lib/AST/RecordLayoutBuilder.cpp   |  4 +++-
 clang/test/SemaCXX/windows-Wpadded-bitfield.cpp | 15 +++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp 
b/clang/lib/AST/RecordLayoutBuilder.cpp
index ea353f88a8aec..ca08e186f4ff2 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1538,6 +1538,7 @@ void ItaniumRecordLayoutBuilder::LayoutBitField(const 
FieldDecl *D) {
   uint64_t StorageUnitSize = FieldInfo.Width;
   unsigned FieldAlign = FieldInfo.Align;
   bool AlignIsRequired = FieldInfo.isAlignRequired();
+  unsigned char PaddingInLastUnit = 0;
 
   // UnfilledBitsInLastUnit is the difference between the end of the
   // last allocated bitfield (i.e. the first bit offset available for
@@ -1610,6 +1611,7 @@ void ItaniumRecordLayoutBuilder::LayoutBitField(const 
FieldDecl *D) {
   if (!LastBitfieldStorageUnitSize && !FieldSize)
 FieldAlign = 1;
 
+  PaddingInLastUnit = UnfilledBitsInLastUnit;
   UnfilledBitsInLastUnit = 0;
   LastBitfieldStorageUnitSize = 0;
 }
@@ -1706,7 +1708,7 @@ void ItaniumRecordLayoutBuilder::LayoutBitField(const 
FieldDecl *D) {
   // For purposes of diagnostics, we're going to simultaneously
   // compute the field offsets that we would have used if we weren't
   // adding any alignment padding or if the field weren't packed.
-  uint64_t UnpaddedFieldOffset = FieldOffset;
+  uint64_t UnpaddedFieldOffset = FieldOffset - PaddingInLastUnit;
   uint64_t UnpackedFieldOffset = FieldOffset;
 
   // Check if we need to add padding to fit the bitfield within an
diff --git a/clang/test/SemaCXX/windows-Wpadded-bitfield.cpp 
b/clang/test/SemaCXX/windows-Wpadded-bitfield.cpp
index ee5a57124eca5..0b88b4c170617 100644
--- a/clang/test/SemaCXX/windows-Wpadded-bitfield.cpp
+++ b/clang/test/SemaCXX/windows-Wpadded-bitfield.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-windows-msvc -fsyntax-only -verify -Wpadded 
%s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -verify -Wpadded %s
 
 struct __attribute__((ms_struct)) BitfieldStruct { // expected-warning 
{{padding size of 'BitfieldStruct' with 3 bytes to alignment boundary}}
   char c : 1;
@@ -24,9 +25,23 @@ struct __attribute__((ms_struct)) DifferentUnitSizeBitfield 
{ // expected-warnin
   char i; // expected-warning {{padding struct 'DifferentUnitSizeBitfield' 
with 31 bits to align 'i'}}
 };
 
+struct __attribute__((ms_struct)) Foo { // expected-warning {{padding size of 
'Foo' with 63 bits to alignment boundary}}
+  long long x;
+  char a : 1;
+  long long b : 1; // expected-warning {{padding struct 'Foo' with 63 bits to 
align 'b'}}
+};
+
+struct __attribute__((ms_struct)) SameUnitSizeMultiple { // expected-warning 
{{padding size of 'SameUnitSizeMultiple' with 2 bits to alignment boundary}}
+  char c : 1;
+  char cc : 2;
+  char ccc : 3;
+};
+
 int main() {
   BitfieldStruct b;
   SevenBitfieldStruct s;
   SameUnitSizeBitfield su;
   DifferentUnitSizeBitfield du;
+  Foo f;
+  SameUnitSizeMultiple susm;
 }

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CLANG][MS-STRUCT] bitfield padding warning presents padding to exact bit count (PR #136062)

[Theo de Magalhaes via cfe-commits]

=?utf-8?q?Théo?= de Magalhaes 
Message-ID:
In-Reply-To: 


https://github.com/theomagellan updated 
https://github.com/llvm/llvm-project/pull/136062

>From 842f0fbed0043ad0aa1679d8a30bc13d64eb25cf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Th=C3=A9o=20de=20Magalhaes?= 
Date: Thu, 17 Apr 2025 01:45:12 +0200
Subject: [PATCH 1/2] fix(ms_struct): bitfield padding warning presents padding
 to exact bit count

---
 clang/lib/AST/RecordLayoutBuilder.cpp   |  4 +++-
 clang/test/SemaCXX/windows-Wpadded-bitfield.cpp | 15 +++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp 
b/clang/lib/AST/RecordLayoutBuilder.cpp
index ea353f88a8aec..ca08e186f4ff2 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1538,6 +1538,7 @@ void ItaniumRecordLayoutBuilder::LayoutBitField(const 
FieldDecl *D) {
   uint64_t StorageUnitSize = FieldInfo.Width;
   unsigned FieldAlign = FieldInfo.Align;
   bool AlignIsRequired = FieldInfo.isAlignRequired();
+  unsigned char PaddingInLastUnit = 0;
 
   // UnfilledBitsInLastUnit is the difference between the end of the
   // last allocated bitfield (i.e. the first bit offset available for
@@ -1610,6 +1611,7 @@ void ItaniumRecordLayoutBuilder::LayoutBitField(const 
FieldDecl *D) {
   if (!LastBitfieldStorageUnitSize && !FieldSize)
 FieldAlign = 1;
 
+  PaddingInLastUnit = UnfilledBitsInLastUnit;
   UnfilledBitsInLastUnit = 0;
   LastBitfieldStorageUnitSize = 0;
 }
@@ -1706,7 +1708,7 @@ void ItaniumRecordLayoutBuilder::LayoutBitField(const 
FieldDecl *D) {
   // For purposes of diagnostics, we're going to simultaneously
   // compute the field offsets that we would have used if we weren't
   // adding any alignment padding or if the field weren't packed.
-  uint64_t UnpaddedFieldOffset = FieldOffset;
+  uint64_t UnpaddedFieldOffset = FieldOffset - PaddingInLastUnit;
   uint64_t UnpackedFieldOffset = FieldOffset;
 
   // Check if we need to add padding to fit the bitfield within an
diff --git a/clang/test/SemaCXX/windows-Wpadded-bitfield.cpp 
b/clang/test/SemaCXX/windows-Wpadded-bitfield.cpp
index ee5a57124eca5..0b88b4c170617 100644
--- a/clang/test/SemaCXX/windows-Wpadded-bitfield.cpp
+++ b/clang/test/SemaCXX/windows-Wpadded-bitfield.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-windows-msvc -fsyntax-only -verify -Wpadded 
%s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -verify -Wpadded %s
 
 struct __attribute__((ms_struct)) BitfieldStruct { // expected-warning 
{{padding size of 'BitfieldStruct' with 3 bytes to alignment boundary}}
   char c : 1;
@@ -24,9 +25,23 @@ struct __attribute__((ms_struct)) DifferentUnitSizeBitfield 
{ // expected-warnin
   char i; // expected-warning {{padding struct 'DifferentUnitSizeBitfield' 
with 31 bits to align 'i'}}
 };
 
+struct __attribute__((ms_struct)) Foo { // expected-warning {{padding size of 
'Foo' with 63 bits to alignment boundary}}
+  long long x;
+  char a : 1;
+  long long b : 1; // expected-warning {{padding struct 'Foo' with 63 bits to 
align 'b'}}
+};
+
+struct __attribute__((ms_struct)) SameUnitSizeMultiple { // expected-warning 
{{padding size of 'SameUnitSizeMultiple' with 2 bits to alignment boundary}}
+  char c : 1;
+  char cc : 2;
+  char ccc : 3;
+};
+
 int main() {
   BitfieldStruct b;
   SevenBitfieldStruct s;
   SameUnitSizeBitfield su;
   DifferentUnitSizeBitfield du;
+  Foo f;
+  SameUnitSizeMultiple susm;
 }

>From 7d003a1d49867bd7e0ea25f597d03944b1e1ef7b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Th=C3=A9o=20de=20Magalhaes?= 
Date: Tue, 22 Apr 2025 00:06:39 +0200
Subject: [PATCH 2/2] fix(bitfield-test): changed struct name to describe test
 case

---
 clang/test/SemaCXX/windows-Wpadded-bitfield.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/clang/test/SemaCXX/windows-Wpadded-bitfield.cpp 
b/clang/test/SemaCXX/windows-Wpadded-bitfield.cpp
index 0b88b4c170617..28917ffaadb6f 100644
--- a/clang/test/SemaCXX/windows-Wpadded-bitfield.cpp
+++ b/clang/test/SemaCXX/windows-Wpadded-bitfield.cpp
@@ -25,10 +25,10 @@ struct __attribute__((ms_struct)) DifferentUnitSizeBitfield 
{ // expected-warnin
   char i; // expected-warning {{padding struct 'DifferentUnitSizeBitfield' 
with 31 bits to align 'i'}}
 };
 
-struct __attribute__((ms_struct)) Foo { // expected-warning {{padding size of 
'Foo' with 63 bits to alignment boundary}}
+struct __attribute__((ms_struct)) BitfieldBigPadding { // expected-warning 
{{padding size of 'BitfieldBigPadding' with 63 bits to alignment boundary}}
   long long x;
   char a : 1;
-  long long b : 1; // expected-warning {{padding struct 'Foo' with 63 bits to 
align 'b'}}
+  long long b : 1; // expected-warning {{padding struct 'BitfieldBigPadding' 
with 63 bits to align 'b'}}
 };
 
 struct __attribute__((ms_struct)) SameUnitSizeMultiple { // expected-warning 
{{padding size of 'SameUnitSizeMultiple' with 2 bits to alignment boun

[clang] [CLANG][MS-STRUCT] bitfield padding warning presents padding to exact bit count (PR #136062)

[Theo de Magalhaes via cfe-commits]

=?utf-8?q?Théo?= de Magalhaes 
Message-ID:
In-Reply-To: 


theomagellan wrote:

It doesn't affect non-ms_struct definitions! The new `PaddingInLastUnit` 
variable only gets a non-zero value when the following condition is met:
`IsMsStruct && (LastBitfieldStorageUnitSize != StorageUnitSize ||
UnfilledBitsInLastUnit < FieldSize)`
Otherwise, it remains zero and doesn't impact any calculations.

I could still add tests for that but I am not sure what they would look like as 
so far the tests only make sure both ABIs (MSVC and Itanium under ms_struct) 
produce the correct padding warnings.
I'm not entirely sure what new cases you'd like to see covered. 
Do you have something specific in mind?

I gave the `Foo` structure a more descriptive name, thank you.

https://github.com/llvm/llvm-project/pull/136062
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CLANG-CL] ignores wpadded (PR #130182)

[Theo de Magalhaes via cfe-commits]

https://github.com/therealcoochieman updated 
https://github.com/llvm/llvm-project/pull/130182

>From 3a9bc908a791669c82f288ff745664411bf285ac Mon Sep 17 00:00:00 2001
From: Theo de Magalhaes 
Date: Thu, 6 Mar 2025 22:26:37 +0100
Subject: [PATCH 1/3] chore(clang-format): formatted
 clang/lib/AST/RecordLayoutBuilder.cpp

---
 clang/lib/AST/RecordLayoutBuilder.cpp | 199 --
 1 file changed, 94 insertions(+), 105 deletions(-)

diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp 
b/clang/lib/AST/RecordLayoutBuilder.cpp
index b8600e6a344a4..695771f67868d 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -43,7 +43,7 @@ struct BaseSubobjectInfo {
   bool IsVirtual;
 
   /// Bases - Information about the base subobjects.
-  SmallVector Bases;
+  SmallVector Bases;
 
   /// PrimaryVirtualBaseInfo - Holds the base info for the primary virtual base
   /// of this base info (if one exists).
@@ -77,8 +77,7 @@ struct ExternalLayout {
   /// Get the offset of the given field. The external source must provide
   /// entries for all fields in the record.
   uint64_t getExternalFieldOffset(const FieldDecl *FD) {
-assert(FieldOffsets.count(FD) &&
-   "Field does not have an external offset");
+assert(FieldOffsets.count(FD) && "Field does not have an external offset");
 return FieldOffsets[FD];
   }
 
@@ -167,16 +166,15 @@ class EmptySubobjectMap {
   CharUnits SizeOfLargestEmptySubobject;
 
   EmptySubobjectMap(const ASTContext &Context, const CXXRecordDecl *Class)
-  : Context(Context), CharWidth(Context.getCharWidth()), Class(Class) {
-  ComputeEmptySubobjectSizes();
+  : Context(Context), CharWidth(Context.getCharWidth()), Class(Class) {
+ComputeEmptySubobjectSizes();
   }
 
   /// CanPlaceBaseAtOffset - Return whether the given base class can be placed
   /// at the given offset.
   /// Returns false if placing the record will result in two components
   /// (direct or indirect) of the same type having the same offset.
-  bool CanPlaceBaseAtOffset(const BaseSubobjectInfo *Info,
-CharUnits Offset);
+  bool CanPlaceBaseAtOffset(const BaseSubobjectInfo *Info, CharUnits Offset);
 
   /// CanPlaceFieldAtOffset - Return whether a field can be placed at the given
   /// offset.
@@ -227,9 +225,8 @@ void EmptySubobjectMap::ComputeEmptySubobjectSizes() {
   }
 }
 
-bool
-EmptySubobjectMap::CanPlaceSubobjectAtOffset(const CXXRecordDecl *RD,
- CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceSubobjectAtOffset(const CXXRecordDecl *RD,
+  CharUnits Offset) const {
   // We only need to check empty bases.
   if (!RD->isEmpty())
 return true;
@@ -265,9 +262,8 @@ void EmptySubobjectMap::AddSubobjectAtOffset(const 
CXXRecordDecl *RD,
 MaxEmptyClassOffset = Offset;
 }
 
-bool
-EmptySubobjectMap::CanPlaceBaseSubobjectAtOffset(const BaseSubobjectInfo *Info,
- CharUnits Offset) {
+bool EmptySubobjectMap::CanPlaceBaseSubobjectAtOffset(
+const BaseSubobjectInfo *Info, CharUnits Offset) {
   // We don't have to keep looking past the maximum offset that's known to
   // contain an empty class.
   if (!AnyEmptySubobjectsBeyondOffset(Offset))
@@ -368,10 +364,9 @@ bool EmptySubobjectMap::CanPlaceBaseAtOffset(const 
BaseSubobjectInfo *Info,
   return true;
 }
 
-bool
-EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const CXXRecordDecl *RD,
-  const CXXRecordDecl *Class,
-  CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(
+const CXXRecordDecl *RD, const CXXRecordDecl *Class,
+CharUnits Offset) const {
   // We don't have to keep looking past the maximum offset that's known to
   // contain an empty class.
   if (!AnyEmptySubobjectsBeyondOffset(Offset))
@@ -418,9 +413,8 @@ EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const 
CXXRecordDecl *RD,
   return true;
 }
 
-bool
-EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const FieldDecl *FD,
-  CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const FieldDecl *FD,
+   CharUnits Offset) const 
{
   // We don't have to keep looking past the maximum offset that's known to
   // contain an empty class.
   if (!AnyEmptySubobjectsBeyondOffset(Offset))
@@ -560,7 +554,7 @@ void EmptySubobjectMap::UpdateEmptyFieldSubobjects(
   }
 }
 
-typedef llvm::SmallPtrSet ClassSetTy;
+typedef llvm::SmallPtrSet ClassSetTy;
 
 class ItaniumRecordLayoutBuilder {
 protected:
@@ -712,15 +706,13 @@ class ItaniumRecordLayoutBuilder {
   bool FieldPacked, const FieldDecl *D);
   void LayoutBitField(const FieldDecl *D);
 
-  TargetCXXABI getCXXABI() 

[clang] [CLANG-CL] ignores wpadded (PR #130182)

[Theo de Magalhaes via cfe-commits]

therealcoochieman wrote:

> Please make sure to keep the formatting of the file as it was (you can format 
> the changes _you_ made to the file with `git clang-format`.
> 
> 
> 
> Thanks!

Hello, I misread the guidelines and thought I had to make a first clang-format 
commit before editing the file. I reverted the commit.
Thank you for the reactive heads up!

https://github.com/llvm/llvm-project/pull/130182
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CLANG-CL] ignores wpadded (PR #130182)

[Theo de Magalhaes via cfe-commits]

https://github.com/therealcoochieman created 
https://github.com/llvm/llvm-project/pull/130182

Aims to fix #61702.

I encountered an interesting behavior during my testing. It seems the Itanium 
Record Layout Builder can give out Wpadded-bitfield warnings with the wrong 
amount of padding when mimicking the Microsoft struct layout.

For the following struct:
```cpp
struct __attribute__((ms_struct)) Foo {
  long long x;
  char y : 1;
  long long yy : 1;
};
```

My understanding is we would expect `yy` to be padded by 63 bits, but the 
warning says there are 7 bytes of padding.
Which one is correct? I am not sure this question is within the scope of this 
PR but I am still shooting my shot here. I can always create an issue if needed.

>From 3a9bc908a791669c82f288ff745664411bf285ac Mon Sep 17 00:00:00 2001
From: Theo de Magalhaes 
Date: Thu, 6 Mar 2025 22:26:37 +0100
Subject: [PATCH 1/2] chore(clang-format): formatted
 clang/lib/AST/RecordLayoutBuilder.cpp

---
 clang/lib/AST/RecordLayoutBuilder.cpp | 199 --
 1 file changed, 94 insertions(+), 105 deletions(-)

diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp 
b/clang/lib/AST/RecordLayoutBuilder.cpp
index b8600e6a344a4..695771f67868d 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -43,7 +43,7 @@ struct BaseSubobjectInfo {
   bool IsVirtual;
 
   /// Bases - Information about the base subobjects.
-  SmallVector Bases;
+  SmallVector Bases;
 
   /// PrimaryVirtualBaseInfo - Holds the base info for the primary virtual base
   /// of this base info (if one exists).
@@ -77,8 +77,7 @@ struct ExternalLayout {
   /// Get the offset of the given field. The external source must provide
   /// entries for all fields in the record.
   uint64_t getExternalFieldOffset(const FieldDecl *FD) {
-assert(FieldOffsets.count(FD) &&
-   "Field does not have an external offset");
+assert(FieldOffsets.count(FD) && "Field does not have an external offset");
 return FieldOffsets[FD];
   }
 
@@ -167,16 +166,15 @@ class EmptySubobjectMap {
   CharUnits SizeOfLargestEmptySubobject;
 
   EmptySubobjectMap(const ASTContext &Context, const CXXRecordDecl *Class)
-  : Context(Context), CharWidth(Context.getCharWidth()), Class(Class) {
-  ComputeEmptySubobjectSizes();
+  : Context(Context), CharWidth(Context.getCharWidth()), Class(Class) {
+ComputeEmptySubobjectSizes();
   }
 
   /// CanPlaceBaseAtOffset - Return whether the given base class can be placed
   /// at the given offset.
   /// Returns false if placing the record will result in two components
   /// (direct or indirect) of the same type having the same offset.
-  bool CanPlaceBaseAtOffset(const BaseSubobjectInfo *Info,
-CharUnits Offset);
+  bool CanPlaceBaseAtOffset(const BaseSubobjectInfo *Info, CharUnits Offset);
 
   /// CanPlaceFieldAtOffset - Return whether a field can be placed at the given
   /// offset.
@@ -227,9 +225,8 @@ void EmptySubobjectMap::ComputeEmptySubobjectSizes() {
   }
 }
 
-bool
-EmptySubobjectMap::CanPlaceSubobjectAtOffset(const CXXRecordDecl *RD,
- CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceSubobjectAtOffset(const CXXRecordDecl *RD,
+  CharUnits Offset) const {
   // We only need to check empty bases.
   if (!RD->isEmpty())
 return true;
@@ -265,9 +262,8 @@ void EmptySubobjectMap::AddSubobjectAtOffset(const 
CXXRecordDecl *RD,
 MaxEmptyClassOffset = Offset;
 }
 
-bool
-EmptySubobjectMap::CanPlaceBaseSubobjectAtOffset(const BaseSubobjectInfo *Info,
- CharUnits Offset) {
+bool EmptySubobjectMap::CanPlaceBaseSubobjectAtOffset(
+const BaseSubobjectInfo *Info, CharUnits Offset) {
   // We don't have to keep looking past the maximum offset that's known to
   // contain an empty class.
   if (!AnyEmptySubobjectsBeyondOffset(Offset))
@@ -368,10 +364,9 @@ bool EmptySubobjectMap::CanPlaceBaseAtOffset(const 
BaseSubobjectInfo *Info,
   return true;
 }
 
-bool
-EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const CXXRecordDecl *RD,
-  const CXXRecordDecl *Class,
-  CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(
+const CXXRecordDecl *RD, const CXXRecordDecl *Class,
+CharUnits Offset) const {
   // We don't have to keep looking past the maximum offset that's known to
   // contain an empty class.
   if (!AnyEmptySubobjectsBeyondOffset(Offset))
@@ -418,9 +413,8 @@ EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const 
CXXRecordDecl *RD,
   return true;
 }
 
-bool
-EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const FieldDecl *FD,
-  CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const FieldDecl 

[clang] [CLANG-CL] ignores Wpadded (PR #134426)

[Theo de Magalhaes via cfe-commits]

=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,Theo de
 Magalhaes 
Message-ID: 
In-Reply-To:


https://github.com/therealcoochieman created 
https://github.com/llvm/llvm-project/pull/134426

Implements the Wpadded diagnostic for clang-cl.
#61702

>From 09572af03c07966925a3456a7d51ad9b3abb750b Mon Sep 17 00:00:00 2001
From: Theo de Magalhaes 
Date: Thu, 6 Mar 2025 22:26:37 +0100
Subject: [PATCH 01/12] chore(clang-format): formatted
 clang/lib/AST/RecordLayoutBuilder.cpp

---
 clang/lib/AST/RecordLayoutBuilder.cpp | 199 --
 1 file changed, 94 insertions(+), 105 deletions(-)

diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp 
b/clang/lib/AST/RecordLayoutBuilder.cpp
index 3e756ab9b9bfe..9740c8e4cdd65 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -43,7 +43,7 @@ struct BaseSubobjectInfo {
   bool IsVirtual;
 
   /// Bases - Information about the base subobjects.
-  SmallVector Bases;
+  SmallVector Bases;
 
   /// PrimaryVirtualBaseInfo - Holds the base info for the primary virtual base
   /// of this base info (if one exists).
@@ -77,8 +77,7 @@ struct ExternalLayout {
   /// Get the offset of the given field. The external source must provide
   /// entries for all fields in the record.
   uint64_t getExternalFieldOffset(const FieldDecl *FD) {
-assert(FieldOffsets.count(FD) &&
-   "Field does not have an external offset");
+assert(FieldOffsets.count(FD) && "Field does not have an external offset");
 return FieldOffsets[FD];
   }
 
@@ -167,16 +166,15 @@ class EmptySubobjectMap {
   CharUnits SizeOfLargestEmptySubobject;
 
   EmptySubobjectMap(const ASTContext &Context, const CXXRecordDecl *Class)
-  : Context(Context), CharWidth(Context.getCharWidth()), Class(Class) {
-  ComputeEmptySubobjectSizes();
+  : Context(Context), CharWidth(Context.getCharWidth()), Class(Class) {
+ComputeEmptySubobjectSizes();
   }
 
   /// CanPlaceBaseAtOffset - Return whether the given base class can be placed
   /// at the given offset.
   /// Returns false if placing the record will result in two components
   /// (direct or indirect) of the same type having the same offset.
-  bool CanPlaceBaseAtOffset(const BaseSubobjectInfo *Info,
-CharUnits Offset);
+  bool CanPlaceBaseAtOffset(const BaseSubobjectInfo *Info, CharUnits Offset);
 
   /// CanPlaceFieldAtOffset - Return whether a field can be placed at the given
   /// offset.
@@ -227,9 +225,8 @@ void EmptySubobjectMap::ComputeEmptySubobjectSizes() {
   }
 }
 
-bool
-EmptySubobjectMap::CanPlaceSubobjectAtOffset(const CXXRecordDecl *RD,
- CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceSubobjectAtOffset(const CXXRecordDecl *RD,
+  CharUnits Offset) const {
   // We only need to check empty bases.
   if (!RD->isEmpty())
 return true;
@@ -265,9 +262,8 @@ void EmptySubobjectMap::AddSubobjectAtOffset(const 
CXXRecordDecl *RD,
 MaxEmptyClassOffset = Offset;
 }
 
-bool
-EmptySubobjectMap::CanPlaceBaseSubobjectAtOffset(const BaseSubobjectInfo *Info,
- CharUnits Offset) {
+bool EmptySubobjectMap::CanPlaceBaseSubobjectAtOffset(
+const BaseSubobjectInfo *Info, CharUnits Offset) {
   // We don't have to keep looking past the maximum offset that's known to
   // contain an empty class.
   if (!AnyEmptySubobjectsBeyondOffset(Offset))
@@ -368,10 +364,9 @@ bool EmptySubobjectMap::CanPlaceBaseAtOffset(const 
BaseSubobjectInfo *Info,
   return true;
 }
 
-bool
-EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const CXXRecordDecl *RD,
-  const CXXRecordDecl *Class,
-  CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(
+const CXXRecordDecl *RD, const CXXRecordDecl *Class,
+CharUnits Offset) const {
   // We don't have to keep looking past the maximum offset that's known to
   // contain an empty class.
   if (!AnyEmptySubobjectsBeyondOffset(Offset))
@@ -418,9 +413,8 @@ EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const 
CXXRecordDecl *RD,
   return true;
 }
 
-bool
-EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const FieldDecl *FD,
-  CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const FieldDecl *FD,
+   CharUnits Offset) const 
{
   // We don't have to keep looking past the maximum offset that's known to
   // contain an empty class.
   if (!AnyEmptySubobjectsBeyondOffset(Offset))
@@ -560,7 +554,7 @@ void EmptySubobjectMap::Upd

[clang] [CLANG-CL] ignores Wpadded (PR #134426)

[Theo de Magalhaes via cfe-commits]

=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,Theo de
 Magalhaes 
Message-ID:
In-Reply-To: 


therealcoochieman wrote:

Fixes the issue in #130182 where the variable `RemainingBitsInField` was used 
uninitialized, leading to undefined behavior in certain cases.
Although I wasn't able to reproduce the undefined behavior using Clang 18.1.8, 
I validated the fix with Valgrind while running the windows-Wpadded.cpp test.

@mikaelholmen @asb, could you please confirm if this resolves the issue on your 
end?
Thanks for your patience, and I hope the pin isn't too disruptive.

https://github.com/llvm/llvm-project/pull/134426
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CLANG-CL] ignores wpadded (PR #130182)

[Theo de Magalhaes via cfe-commits]

=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes 
Message-ID:
In-Reply-To: 



@@ -3004,6 +3010,15 @@ void MicrosoftRecordLayoutBuilder::layoutField(const 
FieldDecl *FD) {
   } else {
 FieldOffset = Size.alignTo(Info.Alignment);
   }
+
+  uint64_t UnpaddedFielddOffsetInBits =
+  Context.toBits(DataSize) - RemainingBitsInField;

therealcoochieman wrote:

Hello, this is also my main suspicion. I will fix this later today.

Thank you for your thoroughness, I did not think to check with other compilers 
or do a valgrind check. Sorry for that.

https://github.com/llvm/llvm-project/pull/130182
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CLANG-CL] ignores wpadded (PR #130182)

[Theo de Magalhaes via cfe-commits]

=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes 
Message-ID:
In-Reply-To: 



@@ -0,0 +1,12 @@
+// RUN: %clang_cl -Wpadded -Wno-msvc-not-found -fsyntax-only -- %s 2>&1 | 
FileCheck -check-prefix=WARN %s

therealcoochieman wrote:

I have merged every test inside the same file 
(`clang/test/SemaCXX/windows-Wpadded.cpp`) except for the one dealing with 
Bitfields as it cannot be tested with both target triples. This test is in a 
separate file.

Thank you for your patience. 

https://github.com/llvm/llvm-project/pull/130182
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CLANG-CL] ignores Wpadded (PR #134426)

[Theo de Magalhaes via cfe-commits]

=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,Theo de
 Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes 
Message-ID:
In-Reply-To: 


https://github.com/therealcoochieman updated 
https://github.com/llvm/llvm-project/pull/134426

>From 09572af03c07966925a3456a7d51ad9b3abb750b Mon Sep 17 00:00:00 2001
From: Theo de Magalhaes 
Date: Thu, 6 Mar 2025 22:26:37 +0100
Subject: [PATCH 01/13] chore(clang-format): formatted
 clang/lib/AST/RecordLayoutBuilder.cpp

---
 clang/lib/AST/RecordLayoutBuilder.cpp | 199 --
 1 file changed, 94 insertions(+), 105 deletions(-)

diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp 
b/clang/lib/AST/RecordLayoutBuilder.cpp
index 3e756ab9b9bfe..9740c8e4cdd65 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -43,7 +43,7 @@ struct BaseSubobjectInfo {
   bool IsVirtual;
 
   /// Bases - Information about the base subobjects.
-  SmallVector Bases;
+  SmallVector Bases;
 
   /// PrimaryVirtualBaseInfo - Holds the base info for the primary virtual base
   /// of this base info (if one exists).
@@ -77,8 +77,7 @@ struct ExternalLayout {
   /// Get the offset of the given field. The external source must provide
   /// entries for all fields in the record.
   uint64_t getExternalFieldOffset(const FieldDecl *FD) {
-assert(FieldOffsets.count(FD) &&
-   "Field does not have an external offset");
+assert(FieldOffsets.count(FD) && "Field does not have an external offset");
 return FieldOffsets[FD];
   }
 
@@ -167,16 +166,15 @@ class EmptySubobjectMap {
   CharUnits SizeOfLargestEmptySubobject;
 
   EmptySubobjectMap(const ASTContext &Context, const CXXRecordDecl *Class)
-  : Context(Context), CharWidth(Context.getCharWidth()), Class(Class) {
-  ComputeEmptySubobjectSizes();
+  : Context(Context), CharWidth(Context.getCharWidth()), Class(Class) {
+ComputeEmptySubobjectSizes();
   }
 
   /// CanPlaceBaseAtOffset - Return whether the given base class can be placed
   /// at the given offset.
   /// Returns false if placing the record will result in two components
   /// (direct or indirect) of the same type having the same offset.
-  bool CanPlaceBaseAtOffset(const BaseSubobjectInfo *Info,
-CharUnits Offset);
+  bool CanPlaceBaseAtOffset(const BaseSubobjectInfo *Info, CharUnits Offset);
 
   /// CanPlaceFieldAtOffset - Return whether a field can be placed at the given
   /// offset.
@@ -227,9 +225,8 @@ void EmptySubobjectMap::ComputeEmptySubobjectSizes() {
   }
 }
 
-bool
-EmptySubobjectMap::CanPlaceSubobjectAtOffset(const CXXRecordDecl *RD,
- CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceSubobjectAtOffset(const CXXRecordDecl *RD,
+  CharUnits Offset) const {
   // We only need to check empty bases.
   if (!RD->isEmpty())
 return true;
@@ -265,9 +262,8 @@ void EmptySubobjectMap::AddSubobjectAtOffset(const 
CXXRecordDecl *RD,
 MaxEmptyClassOffset = Offset;
 }
 
-bool
-EmptySubobjectMap::CanPlaceBaseSubobjectAtOffset(const BaseSubobjectInfo *Info,
- CharUnits Offset) {
+bool EmptySubobjectMap::CanPlaceBaseSubobjectAtOffset(
+const BaseSubobjectInfo *Info, CharUnits Offset) {
   // We don't have to keep looking past the maximum offset that's known to
   // contain an empty class.
   if (!AnyEmptySubobjectsBeyondOffset(Offset))
@@ -368,10 +364,9 @@ bool EmptySubobjectMap::CanPlaceBaseAtOffset(const 
BaseSubobjectInfo *Info,
   return true;
 }
 
-bool
-EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const CXXRecordDecl *RD,
-  const CXXRecordDecl *Class,
-  CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(
+const CXXRecordDecl *RD, const CXXRecordDecl *Class,
+CharUnits Offset) const {
   // We don't have to keep looking past the maximum offset that's known to
   // contain an empty class.
   if (!AnyEmptySubobjectsBeyondOffset(Offset))
@@ -418,9 +413,8 @@ EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const 
CXXRecordDecl *RD,
   return true;
 }
 
-bool
-EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const FieldDecl *FD,
-  CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const FieldDecl *FD,
+   CharUnits Offset) const 
{
   // We don't have to keep looking past the maximum offset that's known to
   // contain an empty class.
   if (!AnyEmptySubobjectsBeyondOffset(Offset))
@@ -560,7 +554,7 @@ void EmptySubobjectMap::UpdateEmptyFieldSubobjects(

[clang] [CLANG-CL] ignores Wpadded (PR #134426)

[Theo de Magalhaes via cfe-commits]

=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,Theo de
 Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes ,
=?utf-8?q?Théo?= De Magalhaes 
Message-ID:
In-Reply-To: 


https://github.com/therealcoochieman updated 
https://github.com/llvm/llvm-project/pull/134426

>From 09572af03c07966925a3456a7d51ad9b3abb750b Mon Sep 17 00:00:00 2001
From: Theo de Magalhaes 
Date: Thu, 6 Mar 2025 22:26:37 +0100
Subject: [PATCH 01/14] chore(clang-format): formatted
 clang/lib/AST/RecordLayoutBuilder.cpp

---
 clang/lib/AST/RecordLayoutBuilder.cpp | 199 --
 1 file changed, 94 insertions(+), 105 deletions(-)

diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp 
b/clang/lib/AST/RecordLayoutBuilder.cpp
index 3e756ab9b9bfe..9740c8e4cdd65 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -43,7 +43,7 @@ struct BaseSubobjectInfo {
   bool IsVirtual;
 
   /// Bases - Information about the base subobjects.
-  SmallVector Bases;
+  SmallVector Bases;
 
   /// PrimaryVirtualBaseInfo - Holds the base info for the primary virtual base
   /// of this base info (if one exists).
@@ -77,8 +77,7 @@ struct ExternalLayout {
   /// Get the offset of the given field. The external source must provide
   /// entries for all fields in the record.
   uint64_t getExternalFieldOffset(const FieldDecl *FD) {
-assert(FieldOffsets.count(FD) &&
-   "Field does not have an external offset");
+assert(FieldOffsets.count(FD) && "Field does not have an external offset");
 return FieldOffsets[FD];
   }
 
@@ -167,16 +166,15 @@ class EmptySubobjectMap {
   CharUnits SizeOfLargestEmptySubobject;
 
   EmptySubobjectMap(const ASTContext &Context, const CXXRecordDecl *Class)
-  : Context(Context), CharWidth(Context.getCharWidth()), Class(Class) {
-  ComputeEmptySubobjectSizes();
+  : Context(Context), CharWidth(Context.getCharWidth()), Class(Class) {
+ComputeEmptySubobjectSizes();
   }
 
   /// CanPlaceBaseAtOffset - Return whether the given base class can be placed
   /// at the given offset.
   /// Returns false if placing the record will result in two components
   /// (direct or indirect) of the same type having the same offset.
-  bool CanPlaceBaseAtOffset(const BaseSubobjectInfo *Info,
-CharUnits Offset);
+  bool CanPlaceBaseAtOffset(const BaseSubobjectInfo *Info, CharUnits Offset);
 
   /// CanPlaceFieldAtOffset - Return whether a field can be placed at the given
   /// offset.
@@ -227,9 +225,8 @@ void EmptySubobjectMap::ComputeEmptySubobjectSizes() {
   }
 }
 
-bool
-EmptySubobjectMap::CanPlaceSubobjectAtOffset(const CXXRecordDecl *RD,
- CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceSubobjectAtOffset(const CXXRecordDecl *RD,
+  CharUnits Offset) const {
   // We only need to check empty bases.
   if (!RD->isEmpty())
 return true;
@@ -265,9 +262,8 @@ void EmptySubobjectMap::AddSubobjectAtOffset(const 
CXXRecordDecl *RD,
 MaxEmptyClassOffset = Offset;
 }
 
-bool
-EmptySubobjectMap::CanPlaceBaseSubobjectAtOffset(const BaseSubobjectInfo *Info,
- CharUnits Offset) {
+bool EmptySubobjectMap::CanPlaceBaseSubobjectAtOffset(
+const BaseSubobjectInfo *Info, CharUnits Offset) {
   // We don't have to keep looking past the maximum offset that's known to
   // contain an empty class.
   if (!AnyEmptySubobjectsBeyondOffset(Offset))
@@ -368,10 +364,9 @@ bool EmptySubobjectMap::CanPlaceBaseAtOffset(const 
BaseSubobjectInfo *Info,
   return true;
 }
 
-bool
-EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const CXXRecordDecl *RD,
-  const CXXRecordDecl *Class,
-  CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(
+const CXXRecordDecl *RD, const CXXRecordDecl *Class,
+CharUnits Offset) const {
   // We don't have to keep looking past the maximum offset that's known to
   // contain an empty class.
   if (!AnyEmptySubobjectsBeyondOffset(Offset))
@@ -418,9 +413,8 @@ EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const 
CXXRecordDecl *RD,
   return true;
 }
 
-bool
-EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const FieldDecl *FD,
-  CharUnits Offset) const {
+bool EmptySubobjectMap::CanPlaceFieldSubobjectAtOffset(const FieldDecl *FD,
+   CharUnits Offset) const 
{
   // We don't have to keep looking past the maximum offset that's known to
   // contain an empty class.
   if (!AnyEmptySubobjectsBeyondOffset(Offset))
@@ -560,7 +554,7 @@ void EmptySubobject