[clang] Refactor ASTContext::getDeclAlign() (NFC) (PR #72977)

2023-11-30 Thread Jonas Paulsson via cfe-commits 

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

[clang] Refactor ASTContext::getDeclAlign() (NFC) (PR #72977)

2023-11-29 Thread Eli Friedman via cfe-commits 

https://github.com/efriedma-quic approved this pull request.

LGTM

See 
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements
 for the rules on use/don't use braces.  A single multi-line statement is sort 
of on the edge; I tend to prefer braces in that case, but you can go either way.

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

[clang] Refactor ASTContext::getDeclAlign() (NFC) (PR #72977)

2023-11-28 Thread Jonas Paulsson via cfe-commits 

JonPsson1 wrote:

Eli, are you supposed to press some button before I merge this?

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

[clang] Refactor ASTContext::getDeclAlign() (NFC) (PR #72977)

2023-11-22 Thread Jonas Paulsson via cfe-commits 

https://github.com/JonPsson1 updated 
https://github.com/llvm/llvm-project/pull/72977

>From 000dcadc0fd118df643e3f2ecbe5fcbb2f8eaab0 Mon Sep 17 00:00:00 2001
From: Jonas Paulsson 
Date: Tue, 21 Nov 2023 12:10:03 +0100
Subject: [PATCH 1/4] Refactor ASTContext::getDeclAlign() (NFC)

---
 clang/lib/AST/ASTContext.cpp | 33 +
 1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 1c893d008cb49f31..d08b9072c0e62985 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1627,28 +1627,21 @@ const llvm::fltSemantics 
::getFloatTypeSemantics(QualType T) const {
 CharUnits ASTContext::getDeclAlign(const Decl *D, bool ForAlignof) const {
   unsigned Align = Target->getCharWidth();
 
-  bool UseAlignAttrOnly = false;
-  if (unsigned AlignFromAttr = D->getMaxAlignment()) {
+  const unsigned AlignFromAttr = D->getMaxAlignment();
+  if (AlignFromAttr)
 Align = AlignFromAttr;
 
-// __attribute__((aligned)) can increase or decrease alignment
-// *except* on a struct or struct member, where it only increases
-// alignment unless 'packed' is also specified.
-//
-// It is an error for alignas to decrease alignment, so we can
-// ignore that possibility;  Sema should diagnose it.
-if (isa(D)) {
-  UseAlignAttrOnly = D->hasAttr() ||
-cast(D)->getParent()->hasAttr();
-} else {
-  UseAlignAttrOnly = true;
-}
-  }
-  else if (isa(D))
-  UseAlignAttrOnly =
-D->hasAttr() ||
-cast(D)->getParent()->hasAttr();
-
+  // __attribute__((aligned)) can increase or decrease alignment
+  // *except* on a struct or struct member, where it only increases
+  // alignment unless 'packed' is also specified.
+  //
+  // It is an error for alignas to decrease alignment, so we can
+  // ignore that possibility;  Sema should diagnose it.
+  bool IsPackedField = isa(D) &&
+   (D->hasAttr() ||
+
cast(D)->getParent()->hasAttr());
+  bool UseAlignAttrOnly =
+isa(D) ? IsPackedField : AlignFromAttr;
   // If we're using the align attribute only, just ignore everything
   // else about the declaration and its type.
   if (UseAlignAttrOnly) {

>From c79760665730a0f2eb70fb86a9d1a7667033c25d Mon Sep 17 00:00:00 2001
From: Jonas Paulsson 
Date: Tue, 21 Nov 2023 12:31:16 +0100
Subject: [PATCH 2/4] Reformat

---
 clang/lib/AST/ASTContext.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index d08b9072c0e62985..50bb24631c118f4b 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1640,8 +1640,7 @@ CharUnits ASTContext::getDeclAlign(const Decl *D, bool 
ForAlignof) const {
   bool IsPackedField = isa(D) &&
(D->hasAttr() ||
 
cast(D)->getParent()->hasAttr());
-  bool UseAlignAttrOnly =
-isa(D) ? IsPackedField : AlignFromAttr;
+  bool UseAlignAttrOnly = isa(D) ? IsPackedField : AlignFromAttr;
   // If we're using the align attribute only, just ignore everything
   // else about the declaration and its type.
   if (UseAlignAttrOnly) {

>From b688dc79c00b86bccc695507f77868e1721e64a0 Mon Sep 17 00:00:00 2001
From: Jonas Paulsson 
Date: Wed, 22 Nov 2023 11:42:35 +0100
Subject: [PATCH 3/4] Use Eli's version instead

---
 clang/lib/AST/ASTContext.cpp | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 50bb24631c118f4b..454f07b4303e0632 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1637,10 +1637,12 @@ CharUnits ASTContext::getDeclAlign(const Decl *D, bool 
ForAlignof) const {
   //
   // It is an error for alignas to decrease alignment, so we can
   // ignore that possibility;  Sema should diagnose it.
-  bool IsPackedField = isa(D) &&
-   (D->hasAttr() ||
-
cast(D)->getParent()->hasAttr());
-  bool UseAlignAttrOnly = isa(D) ? IsPackedField : AlignFromAttr;
+  bool UseAlignAttrOnly;
+  if (FieldDecl *FD = dyn_cast(D))
+UseAlignAttrOnly =
+FD->hasAttr() || FD->getParent()->hasAttr();
+  else
+UseAlignAttrOnly = AlignFromAttr != 0;
   // If we're using the align attribute only, just ignore everything
   // else about the declaration and its type.
   if (UseAlignAttrOnly) {

>From 8078141fd797b40baf1e097a8d48a6294abf49c7 Mon Sep 17 00:00:00 2001
From: Jonas Paulsson 
Date: Wed, 22 Nov 2023 12:35:45 +0100
Subject: [PATCH 4/4] Add needed const

---
 clang/lib/AST/ASTContext.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 454f07b4303e0632..3d9331c6a859acfa 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1638,7 +1638,7 @@ CharUnits ASTContext::getDeclAlign(const

[clang] Refactor ASTContext::getDeclAlign() (NFC) (PR #72977)

2023-11-22 Thread Jonas Paulsson via cfe-commits 


@@ -1627,28 +1627,20 @@ const llvm::fltSemantics 
::getFloatTypeSemantics(QualType T) const {
 CharUnits ASTContext::getDeclAlign(const Decl *D, bool ForAlignof) const {
   unsigned Align = Target->getCharWidth();
 
-  bool UseAlignAttrOnly = false;
-  if (unsigned AlignFromAttr = D->getMaxAlignment()) {
+  const unsigned AlignFromAttr = D->getMaxAlignment();
+  if (AlignFromAttr)
 Align = AlignFromAttr;
 
-// __attribute__((aligned)) can increase or decrease alignment
-// *except* on a struct or struct member, where it only increases
-// alignment unless 'packed' is also specified.
-//
-// It is an error for alignas to decrease alignment, so we can
-// ignore that possibility;  Sema should diagnose it.
-if (isa(D)) {
-  UseAlignAttrOnly = D->hasAttr() ||
-cast(D)->getParent()->hasAttr();
-} else {
-  UseAlignAttrOnly = true;
-}
-  }
-  else if (isa(D))
-  UseAlignAttrOnly =
-D->hasAttr() ||
-cast(D)->getParent()->hasAttr();
-
+  // __attribute__((aligned)) can increase or decrease alignment
+  // *except* on a struct or struct member, where it only increases
+  // alignment unless 'packed' is also specified.
+  //
+  // It is an error for alignas to decrease alignment, so we can
+  // ignore that possibility;  Sema should diagnose it.
+  bool IsPackedField = isa(D) &&
+   (D->hasAttr() ||
+
cast(D)->getParent()->hasAttr());
+  bool UseAlignAttrOnly = isa(D) ? IsPackedField : AlignFromAttr;

JonPsson1 wrote:

That works for me as well: main problem to me with the original code was the 
duplicated handling of a FieldDecl.

I took your suggestion, but removed the braces, which seem superfluous? Or are 
they required by the coding standard?

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

[clang] Refactor ASTContext::getDeclAlign() (NFC) (PR #72977)

2023-11-22 Thread Jonas Paulsson via cfe-commits 

https://github.com/JonPsson1 updated 
https://github.com/llvm/llvm-project/pull/72977

>From 000dcadc0fd118df643e3f2ecbe5fcbb2f8eaab0 Mon Sep 17 00:00:00 2001
From: Jonas Paulsson 
Date: Tue, 21 Nov 2023 12:10:03 +0100
Subject: [PATCH 1/3] Refactor ASTContext::getDeclAlign() (NFC)

---
 clang/lib/AST/ASTContext.cpp | 33 +
 1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 1c893d008cb49f31..d08b9072c0e62985 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1627,28 +1627,21 @@ const llvm::fltSemantics 
::getFloatTypeSemantics(QualType T) const {
 CharUnits ASTContext::getDeclAlign(const Decl *D, bool ForAlignof) const {
   unsigned Align = Target->getCharWidth();
 
-  bool UseAlignAttrOnly = false;
-  if (unsigned AlignFromAttr = D->getMaxAlignment()) {
+  const unsigned AlignFromAttr = D->getMaxAlignment();
+  if (AlignFromAttr)
 Align = AlignFromAttr;
 
-// __attribute__((aligned)) can increase or decrease alignment
-// *except* on a struct or struct member, where it only increases
-// alignment unless 'packed' is also specified.
-//
-// It is an error for alignas to decrease alignment, so we can
-// ignore that possibility;  Sema should diagnose it.
-if (isa(D)) {
-  UseAlignAttrOnly = D->hasAttr() ||
-cast(D)->getParent()->hasAttr();
-} else {
-  UseAlignAttrOnly = true;
-}
-  }
-  else if (isa(D))
-  UseAlignAttrOnly =
-D->hasAttr() ||
-cast(D)->getParent()->hasAttr();
-
+  // __attribute__((aligned)) can increase or decrease alignment
+  // *except* on a struct or struct member, where it only increases
+  // alignment unless 'packed' is also specified.
+  //
+  // It is an error for alignas to decrease alignment, so we can
+  // ignore that possibility;  Sema should diagnose it.
+  bool IsPackedField = isa(D) &&
+   (D->hasAttr() ||
+
cast(D)->getParent()->hasAttr());
+  bool UseAlignAttrOnly =
+isa(D) ? IsPackedField : AlignFromAttr;
   // If we're using the align attribute only, just ignore everything
   // else about the declaration and its type.
   if (UseAlignAttrOnly) {

>From c79760665730a0f2eb70fb86a9d1a7667033c25d Mon Sep 17 00:00:00 2001
From: Jonas Paulsson 
Date: Tue, 21 Nov 2023 12:31:16 +0100
Subject: [PATCH 2/3] Reformat

---
 clang/lib/AST/ASTContext.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index d08b9072c0e62985..50bb24631c118f4b 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1640,8 +1640,7 @@ CharUnits ASTContext::getDeclAlign(const Decl *D, bool 
ForAlignof) const {
   bool IsPackedField = isa(D) &&
(D->hasAttr() ||
 
cast(D)->getParent()->hasAttr());
-  bool UseAlignAttrOnly =
-isa(D) ? IsPackedField : AlignFromAttr;
+  bool UseAlignAttrOnly = isa(D) ? IsPackedField : AlignFromAttr;
   // If we're using the align attribute only, just ignore everything
   // else about the declaration and its type.
   if (UseAlignAttrOnly) {

>From b688dc79c00b86bccc695507f77868e1721e64a0 Mon Sep 17 00:00:00 2001
From: Jonas Paulsson 
Date: Wed, 22 Nov 2023 11:42:35 +0100
Subject: [PATCH 3/3] Use Eli's version instead

---
 clang/lib/AST/ASTContext.cpp | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 50bb24631c118f4b..454f07b4303e0632 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1637,10 +1637,12 @@ CharUnits ASTContext::getDeclAlign(const Decl *D, bool 
ForAlignof) const {
   //
   // It is an error for alignas to decrease alignment, so we can
   // ignore that possibility;  Sema should diagnose it.
-  bool IsPackedField = isa(D) &&
-   (D->hasAttr() ||
-
cast(D)->getParent()->hasAttr());
-  bool UseAlignAttrOnly = isa(D) ? IsPackedField : AlignFromAttr;
+  bool UseAlignAttrOnly;
+  if (FieldDecl *FD = dyn_cast(D))
+UseAlignAttrOnly =
+FD->hasAttr() || FD->getParent()->hasAttr();
+  else
+UseAlignAttrOnly = AlignFromAttr != 0;
   // If we're using the align attribute only, just ignore everything
   // else about the declaration and its type.
   if (UseAlignAttrOnly) {

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

[clang] Refactor ASTContext::getDeclAlign() (NFC) (PR #72977)

2023-11-21 Thread Eli Friedman via cfe-commits 


@@ -1627,28 +1627,20 @@ const llvm::fltSemantics 
::getFloatTypeSemantics(QualType T) const {
 CharUnits ASTContext::getDeclAlign(const Decl *D, bool ForAlignof) const {
   unsigned Align = Target->getCharWidth();
 
-  bool UseAlignAttrOnly = false;
-  if (unsigned AlignFromAttr = D->getMaxAlignment()) {
+  const unsigned AlignFromAttr = D->getMaxAlignment();
+  if (AlignFromAttr)
 Align = AlignFromAttr;
 
-// __attribute__((aligned)) can increase or decrease alignment
-// *except* on a struct or struct member, where it only increases
-// alignment unless 'packed' is also specified.
-//
-// It is an error for alignas to decrease alignment, so we can
-// ignore that possibility;  Sema should diagnose it.
-if (isa(D)) {
-  UseAlignAttrOnly = D->hasAttr() ||
-cast(D)->getParent()->hasAttr();
-} else {
-  UseAlignAttrOnly = true;
-}
-  }
-  else if (isa(D))
-  UseAlignAttrOnly =
-D->hasAttr() ||
-cast(D)->getParent()->hasAttr();
-
+  // __attribute__((aligned)) can increase or decrease alignment
+  // *except* on a struct or struct member, where it only increases
+  // alignment unless 'packed' is also specified.
+  //
+  // It is an error for alignas to decrease alignment, so we can
+  // ignore that possibility;  Sema should diagnose it.
+  bool IsPackedField = isa(D) &&
+   (D->hasAttr() ||
+
cast(D)->getParent()->hasAttr());
+  bool UseAlignAttrOnly = isa(D) ? IsPackedField : AlignFromAttr;

efriedma-quic wrote:

That's a lot of `isa<>` and `cast<>`. I think I'd prefer something more like:

```
bool UseAlignAttrOnly;
if (FieldDecl *FD = dyn_cast(D)) {
  UseAlignAttrOnly = FD->hasAttr() ||
 FD->getParent()->hasAttr();
} else {
  UseAlignAttrOnly = AlignFromAttr != 0;
}
```

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

[clang] Refactor ASTContext::getDeclAlign() (NFC) (PR #72977)

2023-11-21 Thread Jonas Paulsson via cfe-commits 

https://github.com/JonPsson1 updated 
https://github.com/llvm/llvm-project/pull/72977

>From 90938183b35284cff65d100f3cb5284b816f28cc Mon Sep 17 00:00:00 2001
From: Jonas Paulsson 
Date: Tue, 21 Nov 2023 12:10:03 +0100
Subject: [PATCH 1/2] Refactor ASTContext::getDeclAlign() (NFC)

---
 clang/lib/AST/ASTContext.cpp | 33 +
 1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 1c893d008cb49f3..d08b9072c0e6298 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1627,28 +1627,21 @@ const llvm::fltSemantics 
::getFloatTypeSemantics(QualType T) const {
 CharUnits ASTContext::getDeclAlign(const Decl *D, bool ForAlignof) const {
   unsigned Align = Target->getCharWidth();
 
-  bool UseAlignAttrOnly = false;
-  if (unsigned AlignFromAttr = D->getMaxAlignment()) {
+  const unsigned AlignFromAttr = D->getMaxAlignment();
+  if (AlignFromAttr)
 Align = AlignFromAttr;
 
-// __attribute__((aligned)) can increase or decrease alignment
-// *except* on a struct or struct member, where it only increases
-// alignment unless 'packed' is also specified.
-//
-// It is an error for alignas to decrease alignment, so we can
-// ignore that possibility;  Sema should diagnose it.
-if (isa(D)) {
-  UseAlignAttrOnly = D->hasAttr() ||
-cast(D)->getParent()->hasAttr();
-} else {
-  UseAlignAttrOnly = true;
-}
-  }
-  else if (isa(D))
-  UseAlignAttrOnly =
-D->hasAttr() ||
-cast(D)->getParent()->hasAttr();
-
+  // __attribute__((aligned)) can increase or decrease alignment
+  // *except* on a struct or struct member, where it only increases
+  // alignment unless 'packed' is also specified.
+  //
+  // It is an error for alignas to decrease alignment, so we can
+  // ignore that possibility;  Sema should diagnose it.
+  bool IsPackedField = isa(D) &&
+   (D->hasAttr() ||
+
cast(D)->getParent()->hasAttr());
+  bool UseAlignAttrOnly =
+isa(D) ? IsPackedField : AlignFromAttr;
   // If we're using the align attribute only, just ignore everything
   // else about the declaration and its type.
   if (UseAlignAttrOnly) {

>From 3a2b09bbdf533df9797f5d40ae1e027852400560 Mon Sep 17 00:00:00 2001
From: Jonas Paulsson 
Date: Tue, 21 Nov 2023 12:31:16 +0100
Subject: [PATCH 2/2] Reformat

---
 clang/lib/AST/ASTContext.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index d08b9072c0e6298..50bb24631c118f4 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1640,8 +1640,7 @@ CharUnits ASTContext::getDeclAlign(const Decl *D, bool 
ForAlignof) const {
   bool IsPackedField = isa(D) &&
(D->hasAttr() ||
 
cast(D)->getParent()->hasAttr());
-  bool UseAlignAttrOnly =
-isa(D) ? IsPackedField : AlignFromAttr;
+  bool UseAlignAttrOnly = isa(D) ? IsPackedField : AlignFromAttr;
   // If we're using the align attribute only, just ignore everything
   // else about the declaration and its type.
   if (UseAlignAttrOnly) {

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

[clang] Refactor ASTContext::getDeclAlign() (NFC) (PR #72977)

2023-11-21 Thread via cfe-commits 

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff 7f740be4acddd8acf46796229c46117b735a9be8 
90938183b35284cff65d100f3cb5284b816f28cc -- clang/lib/AST/ASTContext.cpp
``





View the diff from clang-format here.


``diff
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index d08b9072c0..50bb24631c 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1640,8 +1640,7 @@ CharUnits ASTContext::getDeclAlign(const Decl *D, bool 
ForAlignof) const {
   bool IsPackedField = isa(D) &&
(D->hasAttr() ||
 
cast(D)->getParent()->hasAttr());
-  bool UseAlignAttrOnly =
-isa(D) ? IsPackedField : AlignFromAttr;
+  bool UseAlignAttrOnly = isa(D) ? IsPackedField : AlignFromAttr;
   // If we're using the align attribute only, just ignore everything
   // else about the declaration and its type.
   if (UseAlignAttrOnly) {

``




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

[clang] Refactor ASTContext::getDeclAlign() (NFC) (PR #72977)

2023-11-21 Thread via cfe-commits 

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: Jonas Paulsson (JonPsson1)


Changes

@efriedma-quic
@rjmccall

While working on this (with the other patch: 72886), I found this refactoring 
at the top of the function which I like. What do you think?


---
Full diff: https://github.com/llvm/llvm-project/pull/72977.diff


1 Files Affected:

- (modified) clang/lib/AST/ASTContext.cpp (+13-20) 


``diff
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 1c893d008cb49f3..d08b9072c0e6298 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1627,28 +1627,21 @@ const llvm::fltSemantics 
::getFloatTypeSemantics(QualType T) const {
 CharUnits ASTContext::getDeclAlign(const Decl *D, bool ForAlignof) const {
   unsigned Align = Target->getCharWidth();
 
-  bool UseAlignAttrOnly = false;
-  if (unsigned AlignFromAttr = D->getMaxAlignment()) {
+  const unsigned AlignFromAttr = D->getMaxAlignment();
+  if (AlignFromAttr)
 Align = AlignFromAttr;
 
-// __attribute__((aligned)) can increase or decrease alignment
-// *except* on a struct or struct member, where it only increases
-// alignment unless 'packed' is also specified.
-//
-// It is an error for alignas to decrease alignment, so we can
-// ignore that possibility;  Sema should diagnose it.
-if (isa(D)) {
-  UseAlignAttrOnly = D->hasAttr() ||
-cast(D)->getParent()->hasAttr();
-} else {
-  UseAlignAttrOnly = true;
-}
-  }
-  else if (isa(D))
-  UseAlignAttrOnly =
-D->hasAttr() ||
-cast(D)->getParent()->hasAttr();
-
+  // __attribute__((aligned)) can increase or decrease alignment
+  // *except* on a struct or struct member, where it only increases
+  // alignment unless 'packed' is also specified.
+  //
+  // It is an error for alignas to decrease alignment, so we can
+  // ignore that possibility;  Sema should diagnose it.
+  bool IsPackedField = isa(D) &&
+   (D->hasAttr() ||
+
cast(D)->getParent()->hasAttr());
+  bool UseAlignAttrOnly =
+isa(D) ? IsPackedField : AlignFromAttr;
   // If we're using the align attribute only, just ignore everything
   // else about the declaration and its type.
   if (UseAlignAttrOnly) {

``




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

[clang] Refactor ASTContext::getDeclAlign() (NFC) (PR #72977)

2023-11-21 Thread Jonas Paulsson via cfe-commits 

https://github.com/JonPsson1 created 
https://github.com/llvm/llvm-project/pull/72977

@efriedma-quic
@rjmccall

While working on this (with the other patch: 72886), I found this refactoring 
at the top of the function which I like. What do you think?


>From 90938183b35284cff65d100f3cb5284b816f28cc Mon Sep 17 00:00:00 2001
From: Jonas Paulsson 
Date: Tue, 21 Nov 2023 12:10:03 +0100
Subject: [PATCH] Refactor ASTContext::getDeclAlign() (NFC)

---
 clang/lib/AST/ASTContext.cpp | 33 +
 1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 1c893d008cb49f3..d08b9072c0e6298 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1627,28 +1627,21 @@ const llvm::fltSemantics 
::getFloatTypeSemantics(QualType T) const {
 CharUnits ASTContext::getDeclAlign(const Decl *D, bool ForAlignof) const {
   unsigned Align = Target->getCharWidth();
 
-  bool UseAlignAttrOnly = false;
-  if (unsigned AlignFromAttr = D->getMaxAlignment()) {
+  const unsigned AlignFromAttr = D->getMaxAlignment();
+  if (AlignFromAttr)
 Align = AlignFromAttr;
 
-// __attribute__((aligned)) can increase or decrease alignment
-// *except* on a struct or struct member, where it only increases
-// alignment unless 'packed' is also specified.
-//
-// It is an error for alignas to decrease alignment, so we can
-// ignore that possibility;  Sema should diagnose it.
-if (isa(D)) {
-  UseAlignAttrOnly = D->hasAttr() ||
-cast(D)->getParent()->hasAttr();
-} else {
-  UseAlignAttrOnly = true;
-}
-  }
-  else if (isa(D))
-  UseAlignAttrOnly =
-D->hasAttr() ||
-cast(D)->getParent()->hasAttr();
-
+  // __attribute__((aligned)) can increase or decrease alignment
+  // *except* on a struct or struct member, where it only increases
+  // alignment unless 'packed' is also specified.
+  //
+  // It is an error for alignas to decrease alignment, so we can
+  // ignore that possibility;  Sema should diagnose it.
+  bool IsPackedField = isa(D) &&
+   (D->hasAttr() ||
+
cast(D)->getParent()->hasAttr());
+  bool UseAlignAttrOnly =
+isa(D) ? IsPackedField : AlignFromAttr;
   // If we're using the align attribute only, just ignore everything
   // else about the declaration and its type.
   if (UseAlignAttrOnly) {

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