[clang] Revert "[clang][dataflow] Correctly handle `InitListExpr` of union type." (PR #82856)

2024-02-26 Thread via cfe-commits

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


[clang] Revert "[clang][dataflow] Correctly handle `InitListExpr` of union type." (PR #82856)

2024-02-26 Thread via cfe-commits

martinboehme wrote:

I see you haven't merged this revert yet.

Alternatively, here's a fix-forward, if you're prepared to go with that:

https://github.com/llvm/llvm-project/pull/82986

Otherwise, do of course merge the revert, and I'll reland with the fix-forward 
incorporated.

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


[clang] Revert "[clang][dataflow] Correctly handle `InitListExpr` of union type." (PR #82856)

2024-02-26 Thread via cfe-commits

martinboehme wrote:

Sorry about the breakage. Will re-land soon with a test for this case and a fix.

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


[clang] Revert "[clang][dataflow] Correctly handle `InitListExpr` of union type." (PR #82856)

2024-02-25 Thread Yitzhak Mandelbaum via cfe-commits

https://github.com/ymand approved this pull request.


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


[clang] Revert "[clang][dataflow] Correctly handle `InitListExpr` of union type." (PR #82856)

2024-02-23 Thread Samira Bazuzi via cfe-commits

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


[clang] Revert "[clang][dataflow] Correctly handle `InitListExpr` of union type." (PR #82856)

2024-02-23 Thread via cfe-commits

github-actions[bot] wrote:

⚠️ We detected that you are using a GitHub private e-mail address to contribute 
to the repo.
  Please turn off [Keep my email addresses 
private](https://github.com/settings/emails) setting in your account.
  See [LLVM 
Discourse](https://discourse.llvm.org/t/hidden-emails-on-github-should-we-do-something-about-it)
 for more information.


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


[clang] Revert "[clang][dataflow] Correctly handle `InitListExpr` of union type." (PR #82856)

2024-02-23 Thread Samira Bazuzi via cfe-commits

bazuzi wrote:

@ymand Can you merge this?

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


[clang] Revert "[clang][dataflow] Correctly handle `InitListExpr` of union type." (PR #82856)

2024-02-23 Thread via cfe-commits

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: Samira Bazuzi (bazuzi)


Changes

Reverts llvm/llvm-project#82348, which caused crashes when analyzing 
empty InitListExprs for unions.

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


5 Files Affected:

- (modified) clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h 
(+3-6) 
- (modified) clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp (+4-14) 
- (modified) clang/lib/Analysis/FlowSensitive/Transfer.cpp (+11-14) 
- (modified) clang/unittests/Analysis/FlowSensitive/TestingSupport.h (-19) 
- (modified) clang/unittests/Analysis/FlowSensitive/TransferTest.cpp (+2-12) 


``diff
diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h 
b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index b3dc940705f870..0aecc749bf415c 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -753,12 +753,9 @@ RecordStorageLocation *getImplicitObjectLocation(const 
CXXMemberCallExpr ,
 RecordStorageLocation *getBaseObjectLocation(const MemberExpr ,
  const Environment );
 
-/// Returns the fields of a `RecordDecl` that are initialized by an
-/// `InitListExpr`, in the order in which they appear in
-/// `InitListExpr::inits()`.
-/// `Init->getType()` must be a record type.
-std::vector
-getFieldsForInitListExpr(const InitListExpr *InitList);
+/// Returns the fields of `RD` that are initialized by an `InitListExpr`, in 
the
+/// order in which they appear in `InitListExpr::inits()`.
+std::vector getFieldsForInitListExpr(const RecordDecl *RD);
 
 /// Associates a new `RecordValue` with `Loc` and returns the new value.
 RecordValue (RecordStorageLocation , Environment );
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 0cfc26ea952cda..d487944ce92111 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -361,8 +361,8 @@ getFieldsGlobalsAndFuncs(const Stmt , FieldSet ,
 if (const auto *FD = dyn_cast(VD))
   Fields.insert(FD);
   } else if (auto *InitList = dyn_cast()) {
-if (InitList->getType()->isRecordType())
-  for (const auto *FD : getFieldsForInitListExpr(InitList))
+if (RecordDecl *RD = InitList->getType()->getAsRecordDecl())
+  for (const auto *FD : getFieldsForInitListExpr(RD))
 Fields.insert(FD);
   }
 }
@@ -1104,22 +1104,12 @@ RecordStorageLocation *getBaseObjectLocation(const 
MemberExpr ,
   return Env.get(*Base);
 }
 
-std::vector
-getFieldsForInitListExpr(const InitListExpr *InitList) {
-  const RecordDecl *RD = InitList->getType()->getAsRecordDecl();
-  assert(RD != nullptr);
-
-  std::vector Fields;
-
-  if (InitList->getType()->isUnionType()) {
-Fields.push_back(InitList->getInitializedFieldInUnion());
-return Fields;
-  }
-
+std::vector getFieldsForInitListExpr(const RecordDecl *RD) {
   // Unnamed bitfields are only used for padding and do not appear in
   // `InitListExpr`'s inits. However, those fields do appear in `RecordDecl`'s
   // field list, and we thus need to remove them before mapping inits to
   // fields to avoid mapping inits to the wrongs fields.
+  std::vector Fields;
   llvm::copy_if(
   RD->fields(), std::back_inserter(Fields),
   [](const FieldDecl *Field) { return !Field->isUnnamedBitfield(); });
diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp 
b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index cd1f04e53cff68..fe13e919bddcd8 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -663,7 +663,14 @@ class TransferVisitor : public 
ConstStmtVisitor {
   void VisitInitListExpr(const InitListExpr *S) {
 QualType Type = S->getType();
 
-if (!Type->isRecordType()) {
+if (Type->isUnionType()) {
+  // FIXME: Initialize unions properly.
+  if (auto *Val = Env.createValue(Type))
+Env.setValue(*S, *Val);
+  return;
+}
+
+if (!Type->isStructureOrClassType()) {
   // Until array initialization is implemented, we don't need to care about
   // cases where `getNumInits() > 1`.
   if (S->getNumInits() == 1)
@@ -681,9 +688,10 @@ class TransferVisitor : public 
ConstStmtVisitor {
 llvm::DenseMap FieldLocs;
 
 // This only contains the direct fields for the given type.
-std::vector FieldsForInit = getFieldsForInitListExpr(S);
+std::vector FieldsForInit =
+getFieldsForInitListExpr(Type->getAsRecordDecl());
 
-// `S->inits()` contains all the initializer expressions, including the
+// `S->inits()` contains all the initializer epressions, including the
 // ones for direct base classes.
 auto Inits = S->inits();
 size_t InitIdx = 0;
@@ -723,17 +731,6 @@ class 

[clang] Revert "[clang][dataflow] Correctly handle `InitListExpr` of union type." (PR #82856)

2024-02-23 Thread via cfe-commits

llvmbot wrote:




@llvm/pr-subscribers-clang-analysis

Author: Samira Bazuzi (bazuzi)


Changes

Reverts llvm/llvm-project#82348, which caused crashes when analyzing 
empty InitListExprs for unions.

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


5 Files Affected:

- (modified) clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h 
(+3-6) 
- (modified) clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp (+4-14) 
- (modified) clang/lib/Analysis/FlowSensitive/Transfer.cpp (+11-14) 
- (modified) clang/unittests/Analysis/FlowSensitive/TestingSupport.h (-19) 
- (modified) clang/unittests/Analysis/FlowSensitive/TransferTest.cpp (+2-12) 


``diff
diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h 
b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index b3dc940705f870..0aecc749bf415c 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -753,12 +753,9 @@ RecordStorageLocation *getImplicitObjectLocation(const 
CXXMemberCallExpr ,
 RecordStorageLocation *getBaseObjectLocation(const MemberExpr ,
  const Environment );
 
-/// Returns the fields of a `RecordDecl` that are initialized by an
-/// `InitListExpr`, in the order in which they appear in
-/// `InitListExpr::inits()`.
-/// `Init->getType()` must be a record type.
-std::vector
-getFieldsForInitListExpr(const InitListExpr *InitList);
+/// Returns the fields of `RD` that are initialized by an `InitListExpr`, in 
the
+/// order in which they appear in `InitListExpr::inits()`.
+std::vector getFieldsForInitListExpr(const RecordDecl *RD);
 
 /// Associates a new `RecordValue` with `Loc` and returns the new value.
 RecordValue (RecordStorageLocation , Environment );
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 0cfc26ea952cda..d487944ce92111 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -361,8 +361,8 @@ getFieldsGlobalsAndFuncs(const Stmt , FieldSet ,
 if (const auto *FD = dyn_cast(VD))
   Fields.insert(FD);
   } else if (auto *InitList = dyn_cast()) {
-if (InitList->getType()->isRecordType())
-  for (const auto *FD : getFieldsForInitListExpr(InitList))
+if (RecordDecl *RD = InitList->getType()->getAsRecordDecl())
+  for (const auto *FD : getFieldsForInitListExpr(RD))
 Fields.insert(FD);
   }
 }
@@ -1104,22 +1104,12 @@ RecordStorageLocation *getBaseObjectLocation(const 
MemberExpr ,
   return Env.get(*Base);
 }
 
-std::vector
-getFieldsForInitListExpr(const InitListExpr *InitList) {
-  const RecordDecl *RD = InitList->getType()->getAsRecordDecl();
-  assert(RD != nullptr);
-
-  std::vector Fields;
-
-  if (InitList->getType()->isUnionType()) {
-Fields.push_back(InitList->getInitializedFieldInUnion());
-return Fields;
-  }
-
+std::vector getFieldsForInitListExpr(const RecordDecl *RD) {
   // Unnamed bitfields are only used for padding and do not appear in
   // `InitListExpr`'s inits. However, those fields do appear in `RecordDecl`'s
   // field list, and we thus need to remove them before mapping inits to
   // fields to avoid mapping inits to the wrongs fields.
+  std::vector Fields;
   llvm::copy_if(
   RD->fields(), std::back_inserter(Fields),
   [](const FieldDecl *Field) { return !Field->isUnnamedBitfield(); });
diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp 
b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index cd1f04e53cff68..fe13e919bddcd8 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -663,7 +663,14 @@ class TransferVisitor : public 
ConstStmtVisitor {
   void VisitInitListExpr(const InitListExpr *S) {
 QualType Type = S->getType();
 
-if (!Type->isRecordType()) {
+if (Type->isUnionType()) {
+  // FIXME: Initialize unions properly.
+  if (auto *Val = Env.createValue(Type))
+Env.setValue(*S, *Val);
+  return;
+}
+
+if (!Type->isStructureOrClassType()) {
   // Until array initialization is implemented, we don't need to care about
   // cases where `getNumInits() > 1`.
   if (S->getNumInits() == 1)
@@ -681,9 +688,10 @@ class TransferVisitor : public 
ConstStmtVisitor {
 llvm::DenseMap FieldLocs;
 
 // This only contains the direct fields for the given type.
-std::vector FieldsForInit = getFieldsForInitListExpr(S);
+std::vector FieldsForInit =
+getFieldsForInitListExpr(Type->getAsRecordDecl());
 
-// `S->inits()` contains all the initializer expressions, including the
+// `S->inits()` contains all the initializer epressions, including the
 // ones for direct base classes.
 auto Inits = S->inits();
 size_t InitIdx = 0;
@@ -723,17 +731,6 

[clang] Revert "[clang][dataflow] Correctly handle `InitListExpr` of union type." (PR #82856)

2024-02-23 Thread Samira Bazuzi via cfe-commits

https://github.com/bazuzi created 
https://github.com/llvm/llvm-project/pull/82856

Reverts llvm/llvm-project#82348, which caused crashes when analyzing empty 
InitListExprs for unions.

>From ae17f89f38d56c2aa9feaa1ebfc7a641b41dc068 Mon Sep 17 00:00:00 2001
From: Samira Bazuzi 
Date: Fri, 23 Feb 2024 20:12:33 -0500
Subject: [PATCH] =?UTF-8?q?Revert=20"[clang][dataflow]=20Correctly=20handl?=
 =?UTF-8?q?e=20`InitListExpr`=20of=20union=20type.=20(#82=E2=80=A6"?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This reverts commit 4725993f1a812c86b9ad79d229a015d0216ff550.
---
 .../FlowSensitive/DataflowEnvironment.h   |  9 +++
 .../FlowSensitive/DataflowEnvironment.cpp | 18 +++--
 clang/lib/Analysis/FlowSensitive/Transfer.cpp | 25 ---
 .../Analysis/FlowSensitive/TestingSupport.h   | 19 --
 .../Analysis/FlowSensitive/TransferTest.cpp   | 14 ++-
 5 files changed, 20 insertions(+), 65 deletions(-)

diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h 
b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index b3dc940705f870..0aecc749bf415c 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -753,12 +753,9 @@ RecordStorageLocation *getImplicitObjectLocation(const 
CXXMemberCallExpr ,
 RecordStorageLocation *getBaseObjectLocation(const MemberExpr ,
  const Environment );
 
-/// Returns the fields of a `RecordDecl` that are initialized by an
-/// `InitListExpr`, in the order in which they appear in
-/// `InitListExpr::inits()`.
-/// `Init->getType()` must be a record type.
-std::vector
-getFieldsForInitListExpr(const InitListExpr *InitList);
+/// Returns the fields of `RD` that are initialized by an `InitListExpr`, in 
the
+/// order in which they appear in `InitListExpr::inits()`.
+std::vector getFieldsForInitListExpr(const RecordDecl *RD);
 
 /// Associates a new `RecordValue` with `Loc` and returns the new value.
 RecordValue (RecordStorageLocation , Environment );
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 0cfc26ea952cda..d487944ce92111 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -361,8 +361,8 @@ getFieldsGlobalsAndFuncs(const Stmt , FieldSet ,
 if (const auto *FD = dyn_cast(VD))
   Fields.insert(FD);
   } else if (auto *InitList = dyn_cast()) {
-if (InitList->getType()->isRecordType())
-  for (const auto *FD : getFieldsForInitListExpr(InitList))
+if (RecordDecl *RD = InitList->getType()->getAsRecordDecl())
+  for (const auto *FD : getFieldsForInitListExpr(RD))
 Fields.insert(FD);
   }
 }
@@ -1104,22 +1104,12 @@ RecordStorageLocation *getBaseObjectLocation(const 
MemberExpr ,
   return Env.get(*Base);
 }
 
-std::vector
-getFieldsForInitListExpr(const InitListExpr *InitList) {
-  const RecordDecl *RD = InitList->getType()->getAsRecordDecl();
-  assert(RD != nullptr);
-
-  std::vector Fields;
-
-  if (InitList->getType()->isUnionType()) {
-Fields.push_back(InitList->getInitializedFieldInUnion());
-return Fields;
-  }
-
+std::vector getFieldsForInitListExpr(const RecordDecl *RD) {
   // Unnamed bitfields are only used for padding and do not appear in
   // `InitListExpr`'s inits. However, those fields do appear in `RecordDecl`'s
   // field list, and we thus need to remove them before mapping inits to
   // fields to avoid mapping inits to the wrongs fields.
+  std::vector Fields;
   llvm::copy_if(
   RD->fields(), std::back_inserter(Fields),
   [](const FieldDecl *Field) { return !Field->isUnnamedBitfield(); });
diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp 
b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index cd1f04e53cff68..fe13e919bddcd8 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -663,7 +663,14 @@ class TransferVisitor : public 
ConstStmtVisitor {
   void VisitInitListExpr(const InitListExpr *S) {
 QualType Type = S->getType();
 
-if (!Type->isRecordType()) {
+if (Type->isUnionType()) {
+  // FIXME: Initialize unions properly.
+  if (auto *Val = Env.createValue(Type))
+Env.setValue(*S, *Val);
+  return;
+}
+
+if (!Type->isStructureOrClassType()) {
   // Until array initialization is implemented, we don't need to care about
   // cases where `getNumInits() > 1`.
   if (S->getNumInits() == 1)
@@ -681,9 +688,10 @@ class TransferVisitor : public 
ConstStmtVisitor {
 llvm::DenseMap FieldLocs;
 
 // This only contains the direct fields for the given type.
-std::vector FieldsForInit = getFieldsForInitListExpr(S);
+std::vector FieldsForInit =
+