date:20220726

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

2022-07-26 Thread Roman Rusyaev via Phabricator via cfe-commits

rusyaev-roman added inline comments.



Comment at: clang/lib/Sema/Scope.cpp:152-154
+  // Consider the variable as NRVO candidate if the return slot is available
+  // for it in the current scope, or if it can be available in outer scopes.
+  NRVO = CanBePutInReturnSlot ? VD : nullptr;

ChuanqiXu wrote:
> rusyaev-roman wrote:
> > rusyaev-roman wrote:
> > > rusyaev-roman wrote:
> > > > ChuanqiXu wrote:
> > > > > rusyaev-roman wrote:
> > > > > > ChuanqiXu wrote:
> > > > > > > rusyaev-roman wrote:
> > > > > > > > ChuanqiXu wrote:
> > > > > > > > > rusyaev-roman wrote:
> > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > rusyaev-roman wrote:
> > > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > > rusyaev-roman wrote:
> > > > > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > > > > What if NRVO contains a value already? It is 
> > > > > > > > > > > > > > > possible due to the value of NRVO could be set by 
> > > > > > > > > > > > > > > its children.
> > > > > > > > > > > > > > Actually this is intention. If the parent has 
> > > > > > > > > > > > > > already NRVO candidate, then it should be 
> > > > > > > > > > > > > > invalidated (or not). Let's consider the following 
> > > > > > > > > > > > > > examples:
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > ```
> > > > > > > > > > > > > > X foo(bool b) {
> > > > > > > > > > > > > >X x;
> > > > > > > > > > > > > >X y;
> > > > > > > > > > > > > >if (b)
> > > > > > > > > > > > > >   return x;
> > > > > > > > > > > > > >else
> > > > > > > > > > > > > >   return y; // when we process this return 
> > > > > > > > > > > > > > statement, the parent has already NRVO and it will 
> > > > > > > > > > > > > > be invalidated (this is correct behavior)
> > > > > > > > > > > > > > }
> > > > > > > > > > > > > > ```
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > ```
> > > > > > > > > > > > > > X foo(bool b) {
> > > > > > > > > > > > > >X x;
> > > > > > > > > > > > > >if (b)
> > > > > > > > > > > > > >   return x;
> > > > > > > > > > > > > >
> > > > > > > > > > > > > >X y;
> > > > > > > > > > > > > >// when we process this return statement, the 
> > > > > > > > > > > > > > parent has already NRVO and it WON't be invalidated
> > > > > > > > > > > > > >//  (this is correct behavior), because a return 
> > > > > > > > > > > > > > slot will be available for it
> > > > > > > > > > > > > >return y;
> > > > > > > > > > > > > > }
> > > > > > > > > > > > > > ```
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > ```
> > > > > > > > > > > > > > X foo(bool b) {
> > > > > > > > > > > > > >X x;
> > > > > > > > > > > > > >if (b)
> > > > > > > > > > > > > >   return x;
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > >// when we process this return statement, the 
> > > > > > > > > > > > > > parent has already NRVO and it WON't be invalidated 
> > > > > > > > > > > > > > (this is correct behavior)
> > > > > > > > > > > > > >return x;
> > > > > > > > > > > > > > }
> > > > > > > > > > > > > > ```
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > ```
> > > > > > > > > > > > > > X foo(bool b, X x) {
> > > > > > > > > > > > > >X y;
> > > > > > > > > > > > > >
> > > > > > > > > > > > > >if (b)
> > > > > > > > > > > > > >   return x;
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > >// when we process this return statement, the 
> > > > > > > > > > > > > > parent contains nullptr (invalid candidate) and it 
> > > > > > > > > > > > > > will be invalidated (this is correct behavior)
> > > > > > > > > > > > > >return y;
> > > > > > > > > > > > > > }
> > > > > > > > > > > > > > ```
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > ```
> > > > > > > > > > > > > > X foo(bool b, X x) {
> > > > > > > > > > > > > >if (b)
> > > > > > > > > > > > > >   return x;
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > >X y;
> > > > > > > > > > > > > >// when we process this return statement, the 
> > > > > > > > > > > > > > parent contains nullptr (invalid candidate) and it 
> > > > > > > > > > > > > > WON't be invalidated (this is correct behavior)
> > > > > > > > > > > > > >return y;
> > > > > > > > > > > > > > }
> > > > > > > > > > > > > > ```
> > > > > > > > > > > > > Oh, I see. Tricky. I don't find invalid cases now. 
> > > > > > > > > > > > > But I recommend to comment that the children would 
> > > > > > > > > > > > > maintain the `ReturnSlots` of their parents. (This is 
> > > > > > > > > > > > > anti-intuition)
> > > > > > > > > > > > > 
> > > > > > > > > > > > > Have you tested any larger projects? Like libc++, 
> > > > > > > > > > > > > libstdc++ or something like folly. I feel we need to 
> > > > > > > > > > > > > do such tests to avoid we get anything wrong.
> > > > > > > > > > > > I've already added a comment at the beginning of 
> > > > > > > >

[PATCH] D130522: [clang][dataflow] Fix SAT solver crashes on `X ^ X` and `X v X`

2022-07-26 Thread Dmitri Gribenko via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3281138aad80: [clang][dataflow] Fix SAT solver crashes on `X 
^ X` and `X v X` (authored by gribozavr).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130522/new/

https://reviews.llvm.org/D130522

Files:
  clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp
  clang/unittests/Analysis/FlowSensitive/SolverTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/SolverTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/SolverTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/SolverTest.cpp
@@ -120,7 +120,7 @@
   expectUnsatisfiable(solve({NotXAndY, XAndY}));
 }
 
-TEST(SolverTest, DisjunctionSameVars) {
+TEST(SolverTest, DisjunctionSameVarWithNegation) {
   ConstraintContext Ctx;
   auto X = Ctx.atom();
   auto NotX = Ctx.neg(X);
@@ -130,6 +130,15 @@
   expectSatisfiable(solve({XOrNotX}), _);
 }
 
+TEST(SolverTest, DisjunctionSameVar) {
+  ConstraintContext Ctx;
+  auto X = Ctx.atom();
+  auto XOrX = Ctx.disj(X, X);
+
+  // X v X
+  expectSatisfiable(solve({XOrX}), _);
+}
+
 TEST(SolverTest, ConjunctionSameVarsConflict) {
   ConstraintContext Ctx;
   auto X = Ctx.atom();
@@ -140,6 +149,15 @@
   expectUnsatisfiable(solve({XAndNotX}));
 }
 
+TEST(SolverTest, ConjunctionSameVar) {
+  ConstraintContext Ctx;
+  auto X = Ctx.atom();
+  auto XAndX = Ctx.conj(X, X);
+
+  // X ^ X
+  expectSatisfiable(solve({XAndX}), _);
+}
+
 TEST(SolverTest, PureVar) {
   ConstraintContext Ctx;
   auto X = Ctx.atom();
Index: clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp
===
--- clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp
+++ clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp
@@ -263,30 +263,52 @@
   const Variable LeftSubVar = GetVar(>getLeftSubValue());
   const Variable RightSubVar = GetVar(>getRightSubValue());
 
-  // `X <=> (A ^ B)` is equivalent to `(!X v A) ^ (!X v B) ^ (X v !A v !B)`
-  // which is already in conjunctive normal form. Below we add each of the
-  // conjuncts of the latter expression to the result.
-  Formula.addClause(negLit(Var), posLit(LeftSubVar));
-  Formula.addClause(negLit(Var), posLit(RightSubVar));
-  Formula.addClause(posLit(Var), negLit(LeftSubVar), negLit(RightSubVar));
-
-  // Visit the sub-values of `Val`.
-  UnprocessedSubVals.push(>getLeftSubValue());
-  UnprocessedSubVals.push(>getRightSubValue());
+  if (LeftSubVar == RightSubVar) {
+// `X <=> (A ^ A)` is equivalent to `(!X v A) ^ (X v !A)` which is
+// already in conjunctive normal form. Below we add each of the
+// conjuncts of the latter expression to the result.
+Formula.addClause(negLit(Var), posLit(LeftSubVar));
+Formula.addClause(posLit(Var), negLit(LeftSubVar));
+
+// Visit a sub-value of `Val` (pick any, they are identical).
+UnprocessedSubVals.push(>getLeftSubValue());
+  } else {
+// `X <=> (A ^ B)` is equivalent to `(!X v A) ^ (!X v B) ^ (X v !A v !B)`
+// which is already in conjunctive normal form. Below we add each of the
+// conjuncts of the latter expression to the result.
+Formula.addClause(negLit(Var), posLit(LeftSubVar));
+Formula.addClause(negLit(Var), posLit(RightSubVar));
+Formula.addClause(posLit(Var), negLit(LeftSubVar), negLit(RightSubVar));
+
+// Visit the sub-values of `Val`.
+UnprocessedSubVals.push(>getLeftSubValue());
+UnprocessedSubVals.push(>getRightSubValue());
+  }
 } else if (auto *D = dyn_cast(Val)) {
   const Variable LeftSubVar = GetVar(>getLeftSubValue());
   const Variable RightSubVar = GetVar(>getRightSubValue());
 
-  // `X <=> (A v B)` is equivalent to `(!X v A v B) ^ (X v !A) ^ (X v !B)`
-  // which is already in conjunctive normal form. Below we add each of the
-  // conjuncts of the latter expression to the result.
-  Formula.addClause(negLit(Var), posLit(LeftSubVar), posLit(RightSubVar));
-  Formula.addClause(posLit(Var), negLit(LeftSubVar));
-  Formula.addClause(posLit(Var), negLit(RightSubVar));
+  if (LeftSubVar == RightSubVar) {
+// `X <=> (A v A)` is equivalent to `(!X v A) ^ (X v !A)` which is
+// already in conjunctive normal form. Below we add each of the
+// conjuncts of the latter expression to the result.
+Formula.addClause(negLit(Var), posLit(LeftSubVar));
+Formula.addClause(posLit(Var), negLit(LeftSubVar));
 
-  // Visit the sub-values of `Val`.
-  UnprocessedSubVals.push(>getLeftSubValue());
-  UnprocessedSubVals.push(>getRightSubValue());
+// Visit a sub-value of `Val` (pick any, they are identical).
+

[clang] 3281138 - [clang][dataflow] Fix SAT solver crashes on `X ^ X` and `X v X`

2022-07-26 Thread Dmitri Gribenko via cfe-commits


Author: Dmitri Gribenko
Date: 2022-07-26T10:26:44+02:00
New Revision: 3281138aad80fcefc7f266c7e3b2e359d5dbc8da

URL: 
https://github.com/llvm/llvm-project/commit/3281138aad80fcefc7f266c7e3b2e359d5dbc8da
DIFF: 
https://github.com/llvm/llvm-project/commit/3281138aad80fcefc7f266c7e3b2e359d5dbc8da.diff

LOG: [clang][dataflow] Fix SAT solver crashes on `X ^ X` and `X v X`

BooleanFormula::addClause has an invariant that a clause has no duplicated
literals. When the solver was desugaring a formula into CNF clauses, it
could construct a clause with such duplicated literals in two cases.

Reviewed By: sgatev, ymandel, xazax.hun

Differential Revision: https://reviews.llvm.org/D130522

Added: 


Modified: 
clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp
clang/unittests/Analysis/FlowSensitive/SolverTest.cpp

Removed: 




diff  --git a/clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp 
b/clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp
index 6a3948bd1fea0..b081543ac1333 100644
--- a/clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp
+++ b/clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp
@@ -263,30 +263,52 @@ BooleanFormula buildBooleanFormula(const 
llvm::DenseSet ) {
   const Variable LeftSubVar = GetVar(>getLeftSubValue());
   const Variable RightSubVar = GetVar(>getRightSubValue());
 
-  // `X <=> (A ^ B)` is equivalent to `(!X v A) ^ (!X v B) ^ (X v !A v !B)`
-  // which is already in conjunctive normal form. Below we add each of the
-  // conjuncts of the latter expression to the result.
-  Formula.addClause(negLit(Var), posLit(LeftSubVar));
-  Formula.addClause(negLit(Var), posLit(RightSubVar));
-  Formula.addClause(posLit(Var), negLit(LeftSubVar), negLit(RightSubVar));
-
-  // Visit the sub-values of `Val`.
-  UnprocessedSubVals.push(>getLeftSubValue());
-  UnprocessedSubVals.push(>getRightSubValue());
+  if (LeftSubVar == RightSubVar) {
+// `X <=> (A ^ A)` is equivalent to `(!X v A) ^ (X v !A)` which is
+// already in conjunctive normal form. Below we add each of the
+// conjuncts of the latter expression to the result.
+Formula.addClause(negLit(Var), posLit(LeftSubVar));
+Formula.addClause(posLit(Var), negLit(LeftSubVar));
+
+// Visit a sub-value of `Val` (pick any, they are identical).
+UnprocessedSubVals.push(>getLeftSubValue());
+  } else {
+// `X <=> (A ^ B)` is equivalent to `(!X v A) ^ (!X v B) ^ (X v !A v 
!B)`
+// which is already in conjunctive normal form. Below we add each of 
the
+// conjuncts of the latter expression to the result.
+Formula.addClause(negLit(Var), posLit(LeftSubVar));
+Formula.addClause(negLit(Var), posLit(RightSubVar));
+Formula.addClause(posLit(Var), negLit(LeftSubVar), 
negLit(RightSubVar));
+
+// Visit the sub-values of `Val`.
+UnprocessedSubVals.push(>getLeftSubValue());
+UnprocessedSubVals.push(>getRightSubValue());
+  }
 } else if (auto *D = dyn_cast(Val)) {
   const Variable LeftSubVar = GetVar(>getLeftSubValue());
   const Variable RightSubVar = GetVar(>getRightSubValue());
 
-  // `X <=> (A v B)` is equivalent to `(!X v A v B) ^ (X v !A) ^ (X v !B)`
-  // which is already in conjunctive normal form. Below we add each of the
-  // conjuncts of the latter expression to the result.
-  Formula.addClause(negLit(Var), posLit(LeftSubVar), posLit(RightSubVar));
-  Formula.addClause(posLit(Var), negLit(LeftSubVar));
-  Formula.addClause(posLit(Var), negLit(RightSubVar));
+  if (LeftSubVar == RightSubVar) {
+// `X <=> (A v A)` is equivalent to `(!X v A) ^ (X v !A)` which is
+// already in conjunctive normal form. Below we add each of the
+// conjuncts of the latter expression to the result.
+Formula.addClause(negLit(Var), posLit(LeftSubVar));
+Formula.addClause(posLit(Var), negLit(LeftSubVar));
 
-  // Visit the sub-values of `Val`.
-  UnprocessedSubVals.push(>getLeftSubValue());
-  UnprocessedSubVals.push(>getRightSubValue());
+// Visit a sub-value of `Val` (pick any, they are identical).
+UnprocessedSubVals.push(>getLeftSubValue());
+  } else {
+// `X <=> (A v B)` is equivalent to `(!X v A v B) ^ (X v !A) ^ (X v 
!B)`
+// which is already in conjunctive normal form. Below we add each of 
the
+// conjuncts of the latter expression to the result.
+Formula.addClause(negLit(Var), posLit(LeftSubVar), 
posLit(RightSubVar));
+Formula.addClause(posLit(Var), negLit(LeftSubVar));
+Formula.addClause(posLit(Var), negLit(RightSubVar));
+
+// Visit the sub-values of `Val`.
+UnprocessedSubVals.push(>getLeftSubValue());
+UnprocessedSubVals.push(>getRightSubValue());
+  }
 } else

[PATCH] D128837: [analyzer] Structured binding to tuple-like types

2022-07-26 Thread Domján Dániel via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa618d5e0dd5d: [analyzer] Structured binding to tuple-like 
types (authored by isuckatcs).
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128837/new/

https://reviews.llvm.org/D128837

Files:
  clang/lib/Analysis/CFG.cpp
  clang/lib/Analysis/LiveVariables.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/test/Analysis/live-bindings-test.cpp
  clang/test/Analysis/uninit-structured-binding-tuple.cpp

Index: clang/test/Analysis/uninit-structured-binding-tuple.cpp
===
--- /dev/null
+++ clang/test/Analysis/uninit-structured-binding-tuple.cpp
@@ -0,0 +1,580 @@
+// RUN: %clang_analyze_cc1 -Wno-ignored-reference-qualifiers -analyzer-checker=core,debug.ExprInspection -std=c++17 -verify %s
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+void clang_analyzer_eval(bool);
+
+namespace std {
+template 
+struct tuple_size {
+};
+
+template 
+struct tuple_element {
+};
+
+// The std::pair in our system header simulator is not tuple-like, so a tuple-like mock is created here
+template 
+struct mock_pair {
+  T1 first;
+  T2 second;
+};
+template 
+struct tuple_size> {
+  static const std::size_t value = 2;
+};
+
+template 
+struct tuple_element<0, mock_pair> {
+  using type = T1;
+};
+
+template 
+struct tuple_element<1, mock_pair> {
+  using type = T2;
+};
+
+template 
+using tuple_element_t = typename tuple_element::type;
+
+template 
+constexpr std::tuple_element_t> &
+get(std::mock_pair ) noexcept {
+  if (I == 0)
+return p.first;
+  else
+return p.second;
+}
+
+template 
+constexpr const std::tuple_element_t> &
+get(const std::mock_pair ) noexcept {
+  if (I == 0)
+return p.first;
+  else
+return p.second;
+}
+
+template 
+constexpr std::tuple_element_t> &&
+get(std::mock_pair &) noexcept {
+
+  if (I == 0)
+return static_cast> &&>(p.first);
+  else
+return static_cast> &&>(p.second);
+}
+
+template 
+constexpr const std::tuple_element_t> &&
+get(const std::mock_pair &) noexcept {
+  if (I == 0)
+return static_cast> &&>(p.first);
+  else
+return static_cast> &&>(p.second);
+}
+
+} // namespace std
+// A utility that generates a tuple-like struct with 2 fields
+//  of the same type. The fields are 'first' and 'second'
+#define GENERATE_TUPLE_LIKE_STRUCT(name, element_type) \
+  struct name {\
+element_type first;\
+element_type second;   \
+  };   \
+   \
+  namespace std {  \
+  template <>  \
+  struct tuple_size {\
+static const std::size_t value = 2;\
+  };   \
+   \
+  template  \
+  struct tuple_element {  \
+using type = element_type; \
+  };   \
+  }
+
+void non_user_defined_by_value(void) {
+  std::mock_pair p = {1, 2};
+
+  auto [u, v] = p;
+
+  clang_analyzer_eval(u == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(v == 2); // expected-warning{{TRUE}}
+
+  int x = u;
+  u = 10;
+  int y = u;
+
+  clang_analyzer_eval(x == 1);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(u == 10); // expected-warning{{TRUE}}
+
+  clang_analyzer_eval(y == 10);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(p.first == 1); // expected-warning{{TRUE}}
+
+  p.first = 5;
+
+  clang_analyzer_eval(u == 10); // expected-warning{{TRUE}}
+}
+
+void non_user_defined_by_lref(void) {
+  std::mock_pair p = {1, 2};
+
+  auto &[u, v] = p;
+
+  int x = u;
+  u = 10;
+  int y = u;
+
+  clang_analyzer_eval(x == 1);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(u == 10); // expected-warning{{TRUE}}
+
+  clang_analyzer_eval(y == 10);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(p.first == 10); // expected-warning{{TRUE}}
+
+  clang_analyzer_eval(v == 2);// expected-warning{{TRUE}}
+  clang_analyzer_eval(p.second == 2); // expected-warning{{TRUE}}
+
+  p.first = 5;
+
+  clang_analyzer_eval(u == 5); // expected-warning{{TRUE}}
+}
+
+void non_user_defined_by_rref(void) {
+  std::mock_pair p = {1, 2};
+
+  auto &&[u, v] = p;
+
+  int x = u;
+  u = 10;
+  int y = u;
+
+  clang_analyzer_eval(x == 1);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(u == 10); // expected-warning{{TRUE}}
+
+  clang_analyzer_eval(y == 10);   // expected-warning{{TRUE}}
+  clang_analyzer_eval(p.first == 10); //

[clang] a618d5e - [analyzer] Structured binding to tuple-like types

2022-07-26 Thread via cfe-commits


Author: isuckatcs
Date: 2022-07-26T10:24:29+02:00
New Revision: a618d5e0dd5d6fee5d73e823dbf8301663be0b4f

URL: 
https://github.com/llvm/llvm-project/commit/a618d5e0dd5d6fee5d73e823dbf8301663be0b4f
DIFF: 
https://github.com/llvm/llvm-project/commit/a618d5e0dd5d6fee5d73e823dbf8301663be0b4f.diff

LOG: [analyzer] Structured binding to tuple-like types

Introducing support for creating structured binding
to tuple-like types.

Differential Revision: https://reviews.llvm.org/D128837

Added: 
clang/test/Analysis/uninit-structured-binding-tuple.cpp

Modified: 
clang/lib/Analysis/CFG.cpp
clang/lib/Analysis/LiveVariables.cpp
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
clang/test/Analysis/live-bindings-test.cpp

Removed: 




diff  --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index 3937fffcff5a..84178ff488a5 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -2932,6 +2932,20 @@ CFGBlock *CFGBuilder::VisitDeclSubExpr(DeclStmt *DS) {
 }
   }
 
+  // If we bind to a tuple-like type, we iterate over the HoldingVars, and
+  // create a DeclStmt for each of them.
+  if (const auto *DD = dyn_cast(VD)) {
+for (auto BD : llvm::reverse(DD->bindings())) {
+  if (auto *VD = BD->getHoldingVar()) {
+DeclGroupRef DG(VD);
+DeclStmt *DSNew =
+new (Context) DeclStmt(DG, VD->getLocation(), GetEndLoc(VD));
+cfg->addSyntheticDeclStmt(DSNew, DS);
+Block = VisitDeclSubExpr(DSNew);
+  }
+}
+  }
+
   autoCreateBlock();
   appendStmt(Block, DS);
 

diff  --git a/clang/lib/Analysis/LiveVariables.cpp 
b/clang/lib/Analysis/LiveVariables.cpp
index 6c601c290c92..ff7f3ebe28f8 100644
--- a/clang/lib/Analysis/LiveVariables.cpp
+++ b/clang/lib/Analysis/LiveVariables.cpp
@@ -72,6 +72,11 @@ bool LiveVariables::LivenessValues::isLive(const VarDecl *D) 
const {
 bool alive = false;
 for (const BindingDecl *BD : DD->bindings())
   alive |= liveBindings.contains(BD);
+
+// Note: the only known case this condition is necessary, is when a bindig
+// to a tuple-like structure is created. The HoldingVar initializers have a
+// DeclRefExpr to the DecompositionDecl.
+alive |= liveDecls.contains(DD);
 return alive;
   }
   return liveDecls.contains(D);
@@ -343,8 +348,12 @@ void TransferFunctions::VisitBinaryOperator(BinaryOperator 
*B) {
 
   if (const BindingDecl* BD = dyn_cast(D)) {
 Killed = !BD->getType()->isReferenceType();
-if (Killed)
+if (Killed) {
+  if (const auto *HV = BD->getHoldingVar())
+val.liveDecls = LV.DSetFact.remove(val.liveDecls, HV);
+
   val.liveBindings = LV.BSetFact.remove(val.liveBindings, BD);
+}
   } else if (const auto *VD = dyn_cast(D)) {
 Killed = writeShouldKill(VD);
 if (Killed)
@@ -371,8 +380,12 @@ void TransferFunctions::VisitDeclRefExpr(DeclRefExpr *DR) {
   const Decl* D = DR->getDecl();
   bool InAssignment = LV.inAssignment[DR];
   if (const auto *BD = dyn_cast(D)) {
-if (!InAssignment)
+if (!InAssignment) {
+  if (const auto *HV = BD->getHoldingVar())
+val.liveDecls = LV.DSetFact.add(val.liveDecls, HV);
+
   val.liveBindings = LV.BSetFact.add(val.liveBindings, BD);
+}
   } else if (const auto *VD = dyn_cast(D)) {
 if (!InAssignment && !isAlwaysAlive(VD))
   val.liveDecls = LV.DSetFact.add(val.liveDecls, VD);
@@ -382,8 +395,16 @@ void TransferFunctions::VisitDeclRefExpr(DeclRefExpr *DR) {
 void TransferFunctions::VisitDeclStmt(DeclStmt *DS) {
   for (const auto *DI : DS->decls()) {
 if (const auto *DD = dyn_cast(DI)) {
-  for (const auto *BD : DD->bindings())
+  for (const auto *BD : DD->bindings()) {
+if (const auto *HV = BD->getHoldingVar())
+  val.liveDecls = LV.DSetFact.remove(val.liveDecls, HV);
+
 val.liveBindings = LV.BSetFact.remove(val.liveBindings, BD);
+  }
+
+  // When a bindig to a tuple-like structure is created, the HoldingVar
+  // initializers have a DeclRefExpr to the DecompositionDecl.
+  val.liveDecls = LV.DSetFact.remove(val.liveDecls, DD);
 } else if (const auto *VD = dyn_cast(DI)) {
   if (!isAlwaysAlive(VD))
 val.liveDecls = LV.DSetFact.remove(val.liveDecls, VD);

diff  --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp 
b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index bb2fdc83c8f8..936d4ed7c89b 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -2788,7 +2788,10 @@ void ExprEngine::VisitCommonDeclRefExpr(const Expr *Ex, 
const NamedDecl *D,
 
 SVal Base = state->getLValue(DD, LCtx);
 if (DD->getType()->isReferenceType()) {
-  Base = state->getSVal(Base.getAsRegion());
+  if (const MemRegion *R = Base.getAsRegion())
+Base = state->getSVal(R);
+  else
+Base = UnknownVal();

[PATCH] D130522: [clang][dataflow] Fix SAT solver crashes on `X ^ X` and `X v X`

2022-07-26 Thread Dmitri Gribenko via Phabricator via cfe-commits

gribozavr updated this revision to Diff 447609.
gribozavr added a comment.

Actually visit a subexpression


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130522/new/

https://reviews.llvm.org/D130522

Files:
  clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp
  clang/unittests/Analysis/FlowSensitive/SolverTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/SolverTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/SolverTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/SolverTest.cpp
@@ -120,7 +120,7 @@
   expectUnsatisfiable(solve({NotXAndY, XAndY}));
 }
 
-TEST(SolverTest, DisjunctionSameVars) {
+TEST(SolverTest, DisjunctionSameVarWithNegation) {
   ConstraintContext Ctx;
   auto X = Ctx.atom();
   auto NotX = Ctx.neg(X);
@@ -130,6 +130,15 @@
   expectSatisfiable(solve({XOrNotX}), _);
 }
 
+TEST(SolverTest, DisjunctionSameVar) {
+  ConstraintContext Ctx;
+  auto X = Ctx.atom();
+  auto XOrX = Ctx.disj(X, X);
+
+  // X v X
+  expectSatisfiable(solve({XOrX}), _);
+}
+
 TEST(SolverTest, ConjunctionSameVarsConflict) {
   ConstraintContext Ctx;
   auto X = Ctx.atom();
@@ -140,6 +149,15 @@
   expectUnsatisfiable(solve({XAndNotX}));
 }
 
+TEST(SolverTest, ConjunctionSameVar) {
+  ConstraintContext Ctx;
+  auto X = Ctx.atom();
+  auto XAndX = Ctx.conj(X, X);
+
+  // X ^ X
+  expectSatisfiable(solve({XAndX}), _);
+}
+
 TEST(SolverTest, PureVar) {
   ConstraintContext Ctx;
   auto X = Ctx.atom();
Index: clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp
===
--- clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp
+++ clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp
@@ -263,30 +263,52 @@
   const Variable LeftSubVar = GetVar(>getLeftSubValue());
   const Variable RightSubVar = GetVar(>getRightSubValue());
 
-  // `X <=> (A ^ B)` is equivalent to `(!X v A) ^ (!X v B) ^ (X v !A v !B)`
-  // which is already in conjunctive normal form. Below we add each of the
-  // conjuncts of the latter expression to the result.
-  Formula.addClause(negLit(Var), posLit(LeftSubVar));
-  Formula.addClause(negLit(Var), posLit(RightSubVar));
-  Formula.addClause(posLit(Var), negLit(LeftSubVar), negLit(RightSubVar));
-
-  // Visit the sub-values of `Val`.
-  UnprocessedSubVals.push(>getLeftSubValue());
-  UnprocessedSubVals.push(>getRightSubValue());
+  if (LeftSubVar == RightSubVar) {
+// `X <=> (A ^ A)` is equivalent to `(!X v A) ^ (X v !A)` which is
+// already in conjunctive normal form. Below we add each of the
+// conjuncts of the latter expression to the result.
+Formula.addClause(negLit(Var), posLit(LeftSubVar));
+Formula.addClause(posLit(Var), negLit(LeftSubVar));
+
+// Visit a sub-value of `Val` (pick any, they are identical).
+UnprocessedSubVals.push(>getLeftSubValue());
+  } else {
+// `X <=> (A ^ B)` is equivalent to `(!X v A) ^ (!X v B) ^ (X v !A v !B)`
+// which is already in conjunctive normal form. Below we add each of the
+// conjuncts of the latter expression to the result.
+Formula.addClause(negLit(Var), posLit(LeftSubVar));
+Formula.addClause(negLit(Var), posLit(RightSubVar));
+Formula.addClause(posLit(Var), negLit(LeftSubVar), negLit(RightSubVar));
+
+// Visit the sub-values of `Val`.
+UnprocessedSubVals.push(>getLeftSubValue());
+UnprocessedSubVals.push(>getRightSubValue());
+  }
 } else if (auto *D = dyn_cast(Val)) {
   const Variable LeftSubVar = GetVar(>getLeftSubValue());
   const Variable RightSubVar = GetVar(>getRightSubValue());
 
-  // `X <=> (A v B)` is equivalent to `(!X v A v B) ^ (X v !A) ^ (X v !B)`
-  // which is already in conjunctive normal form. Below we add each of the
-  // conjuncts of the latter expression to the result.
-  Formula.addClause(negLit(Var), posLit(LeftSubVar), posLit(RightSubVar));
-  Formula.addClause(posLit(Var), negLit(LeftSubVar));
-  Formula.addClause(posLit(Var), negLit(RightSubVar));
+  if (LeftSubVar == RightSubVar) {
+// `X <=> (A v A)` is equivalent to `(!X v A) ^ (X v !A)` which is
+// already in conjunctive normal form. Below we add each of the
+// conjuncts of the latter expression to the result.
+Formula.addClause(negLit(Var), posLit(LeftSubVar));
+Formula.addClause(posLit(Var), negLit(LeftSubVar));
 
-  // Visit the sub-values of `Val`.
-  UnprocessedSubVals.push(>getLeftSubValue());
-  UnprocessedSubVals.push(>getRightSubValue());
+// Visit a sub-value of `Val` (pick any, they are identical).
+UnprocessedSubVals.push(>getLeftSubValue());
+  } else {
+// `X <=> (A v B)` is equivalent to `(!X v A v B) ^ (X v !A) ^ (X v !B)`
+// which is already

[PATCH] D130145: [AArch64] Simplify BTI/PAC-RET module flags

2022-07-26 Thread Daniel Kiss via Phabricator via cfe-commits

danielkiss accepted this revision.
danielkiss added a comment.
This revision is now accepted and ready to land.

AutoUpgrade is added due to similar issue at a user.
LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130145/new/

https://reviews.llvm.org/D130145

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

[PATCH] D126745: [RISCV][Clang] Support policy functions for vmerge, vfmerge and vcompress.

2022-07-26 Thread Nuno Lopes via Phabricator via cfe-commits

nlopes added a comment.

While at it, could you switch those UndefValue with PoisonValue if possible?  
Thank you!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126745/new/

https://reviews.llvm.org/D126745

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

[PATCH] D126748: [RISCV][Clang] Support policy functions for Vector Reduction Instructions.

2022-07-26 Thread Nuno Lopes via Phabricator via cfe-commits

nlopes added a comment.

While at it, could you switch those UndefValue with PoisonValue if possible?  
Thank you!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126748/new/

https://reviews.llvm.org/D126748

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

[PATCH] D126750: [RISCV][Clang] Support policy function for all vector segment load.

2022-07-26 Thread Nuno Lopes via Phabricator via cfe-commits

nlopes added a comment.

While at it, could you switch those UndefValue with PoisonValue if possible?  
Thank you!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126750/new/

https://reviews.llvm.org/D126750

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

[PATCH] D126750: [RISCV][Clang] Support policy function for all vector segment load.

2022-07-26 Thread Kito Cheng via Phabricator via cfe-commits

kito-cheng accepted this revision.
kito-cheng added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: nlopes.

LGTM, and verified with internal testsuite :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126750/new/

https://reviews.llvm.org/D126750

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

[PATCH] D126748: [RISCV][Clang] Support policy functions for Vector Reduction Instructions.

2022-07-26 Thread Kito Cheng via Phabricator via cfe-commits

kito-cheng accepted this revision.
kito-cheng added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: nlopes.

LGTM, and verified with internal testsuite :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126748/new/

https://reviews.llvm.org/D126748

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

[PATCH] D126745: [RISCV][Clang] Support policy functions for vmerge, vfmerge and vcompress.

2022-07-26 Thread Kito Cheng via Phabricator via cfe-commits

kito-cheng accepted this revision.
kito-cheng added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: nlopes.

LGTM, and verified with internal testsuite :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126745/new/

https://reviews.llvm.org/D126745

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

[PATCH] D126744: [RISCV][Clang] Support policy functions for vneg, vnot, vncvt, vwcvt, vwcvtu, vfabs and vfneg.

2022-07-26 Thread Kito Cheng via Phabricator via cfe-commits

kito-cheng accepted this revision.
kito-cheng added a comment.
This revision is now accepted and ready to land.

LGTM, and verified with internal testsuite :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126744/new/

https://reviews.llvm.org/D126744

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

[PATCH] D111617: [RISCV] Lazily add RVV C intrinsics.

2022-07-26 Thread Kito Cheng via Phabricator via cfe-commits

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
kito-cheng marked an inline comment as done.
Closed by commit rG7a5cb15ea6fa: [RISCV] Lazily add RVV C intrinsics. (authored 
by kito-cheng).

Changed prior to commit:
  https://reviews.llvm.org/D111617?vs=444188=447601#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111617/new/

https://reviews.llvm.org/D111617

Files:
  clang/include/clang/Basic/CMakeLists.txt
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/RISCVIntrinsicManager.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Support/RISCVVIntrinsicUtils.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/CMakeLists.txt
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaRISCVVectorLookup.cpp
  clang/lib/Support/RISCVVIntrinsicUtils.cpp
  clang/test/Sema/riscv-bad-intrinsic-pragma.c
  clang/test/Sema/riscv-intrinsic-pragma.c
  clang/utils/TableGen/RISCVVEmitter.cpp
  clang/utils/TableGen/TableGen.cpp
  clang/utils/TableGen/TableGenBackends.h

Index: clang/utils/TableGen/TableGenBackends.h
===
--- clang/utils/TableGen/TableGenBackends.h
+++ clang/utils/TableGen/TableGenBackends.h
@@ -110,6 +110,7 @@
 void EmitRVVHeader(llvm::RecordKeeper , llvm::raw_ostream );
 void EmitRVVBuiltins(llvm::RecordKeeper , llvm::raw_ostream );
 void EmitRVVBuiltinCG(llvm::RecordKeeper , llvm::raw_ostream );
+void EmitRVVBuiltinSema(llvm::RecordKeeper , llvm::raw_ostream );
 
 void EmitCdeHeader(llvm::RecordKeeper , llvm::raw_ostream );
 void EmitCdeBuiltinDef(llvm::RecordKeeper , llvm::raw_ostream );
Index: clang/utils/TableGen/TableGen.cpp
===
--- clang/utils/TableGen/TableGen.cpp
+++ clang/utils/TableGen/TableGen.cpp
@@ -88,6 +88,7 @@
   GenRISCVVectorHeader,
   GenRISCVVectorBuiltins,
   GenRISCVVectorBuiltinCG,
+  GenRISCVVectorBuiltinSema,
   GenAttrDocs,
   GenDiagDocs,
   GenOptDocs,
@@ -243,6 +244,8 @@
"Generate riscv_vector_builtins.inc for clang"),
 clEnumValN(GenRISCVVectorBuiltinCG, "gen-riscv-vector-builtin-codegen",
"Generate riscv_vector_builtin_cg.inc for clang"),
+clEnumValN(GenRISCVVectorBuiltinSema, "gen-riscv-vector-builtin-sema",
+   "Generate riscv_vector_builtin_sema.inc for clang"),
 clEnumValN(GenAttrDocs, "gen-attr-docs",
"Generate attribute documentation"),
 clEnumValN(GenDiagDocs, "gen-diag-docs",
@@ -458,6 +461,9 @@
   case GenRISCVVectorBuiltinCG:
 EmitRVVBuiltinCG(Records, OS);
 break;
+  case GenRISCVVectorBuiltinSema:
+EmitRVVBuiltinSema(Records, OS);
+break;
   case GenAttrDocs:
 EmitClangAttrDocs(Records, OS);
 break;
Index: clang/utils/TableGen/RISCVVEmitter.cpp
===
--- clang/utils/TableGen/RISCVVEmitter.cpp
+++ clang/utils/TableGen/RISCVVEmitter.cpp
@@ -20,6 +20,7 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringSet.h"
+#include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
@@ -29,6 +30,59 @@
 using namespace clang::RISCV;
 
 namespace {
+struct SemaRecord {
+  // Intrinsic name, e.g. vadd_vv
+  std::string Name;
+
+  // Overloaded intrinsic name, could be empty if can be computed from Name
+  // e.g. vadd
+  std::string OverloadedName;
+
+  // Supported type, mask of BasicType.
+  unsigned TypeRangeMask;
+
+  // Supported LMUL.
+  unsigned Log2LMULMask;
+
+  // Required extensions for this intrinsic.
+  unsigned RequiredExtensions;
+
+  // Prototype for this intrinsic.
+  SmallVector Prototype;
+
+  // Prototype for masked intrinsic.
+  SmallVector MaskedPrototype;
+
+  // Suffix of intrinsic name.
+  SmallVector Suffix;
+
+  // Suffix of overloaded intrinsic name.
+  SmallVector OverloadedSuffix;
+
+  // Number of field, large than 1 if it's segment load/store.
+  unsigned NF;
+};
+
+// Compressed function signature table.
+class SemaSignatureTable {
+private:
+  std::vector SignatureTable;
+
+  void insert(ArrayRef Signature);
+
+public:
+  static constexpr unsigned INVALID_INDEX = ~0U;
+
+  // Create compressed signature table from SemaRecords.
+  void init(ArrayRef SemaRecords);
+
+  // Query the Signature, return INVALID_INDEX if not found.
+  unsigned getIndex(ArrayRef Signature);
+
+  /// Print signature table in RVVHeader Record to \p OS
+  void print(raw_ostream );
+};
+
 class RVVEmitter {
 private:
   RecordKeeper 
@@ -45,22 +99,22 @@
   /// Emit all the information needed to map builtin -> LLVM IR intrinsic.
   void

[clang] 7a5cb15 - [RISCV] Lazily add RVV C intrinsics.

2022-07-26 Thread Kito Cheng via cfe-commits


Author: Kito Cheng
Date: 2022-07-26T15:47:47+08:00
New Revision: 7a5cb15ea6facd82756adafae76d60f36a0b60fd

URL: 
https://github.com/llvm/llvm-project/commit/7a5cb15ea6facd82756adafae76d60f36a0b60fd
DIFF: 
https://github.com/llvm/llvm-project/commit/7a5cb15ea6facd82756adafae76d60f36a0b60fd.diff

LOG: [RISCV] Lazily add RVV C intrinsics.

Leverage the method OpenCL uses that adds C intrinsics when the lookup
failed. There is no need to define C intrinsics in the header file any
more. It could help to avoid the large header file to speed up the
compilation of RVV source code. Besides that, only the C intrinsics used
by the users will be added into the declaration table.

This patch is based on https://reviews.llvm.org/D103228 and inspired by
OpenCL implementation.

### Experimental Results

 TL;DR:

- Binary size of clang increase ~200k, which is +0.07%  for debug build and 
+0.13% for release build.
- Single file compilation speed up ~33x for debug build and ~8.5x for release 
build
- Regression time reduce ~10% (`ninja check-all`, enable all targets)

 Header size change
```
   |  size | LoC |
--
Before | 4,434,725 |  69,749 |
After  | 6,140 | 162 |
```

 Single File Compilation Time
Testcase:
```
#include 

vint32m1_t test_vadd_vv_vfloat32m1_t(vint32m1_t op1, vint32m1_t op2, size_t vl) 
{
  return vadd(op1, op2, vl);
}
```
# Debug build:
Before:
```
real0m19.352s
user0m19.252s
sys 0m0.092s
```

After:
```
real0m0.576s
user0m0.552s
sys 0m0.024s
```

~33x speed up for debug build

# Release build:
Before:
```
real0m0.773s
user0m0.741s
sys 0m0.032s
```

After:
```
real0m0.092s
user0m0.080s
sys 0m0.012s
```

~8.5x speed up for release build

 Regression time
Note: the failed case is `tools/llvm-debuginfod-find/debuginfod.test` which is 
unrelated to this patch.

# Debug build
Before:
```
Testing Time: 1358.38s
  Skipped  :11
  Unsupported  :   446
  Passed   : 75767
  Expectedly Failed:   190
  Failed   : 1
```
After
```
Testing Time: 1220.29s
  Skipped  :11
  Unsupported  :   446
  Passed   : 75767
  Expectedly Failed:   190
  Failed   : 1
```
# Release build
Before:
```
Testing Time: 381.98s
  Skipped  :12
  Unsupported  :  1407
  Passed   : 74765
  Expectedly Failed:   176
  Failed   : 1
```
After:
```
Testing Time: 346.25s
  Skipped  :12
  Unsupported  :  1407
  Passed   : 74765
  Expectedly Failed:   176
  Failed   : 1
```

 Binary size of clang

# Debug build
Before
```
   textdata bss dec hex filename
335261851   12726004 552812 348540667   14c64efb
bin/clang
```
After
```
   textdata bss dec hex filename
335442803   12798708 552940 348794451   14ca2e53
bin/clang
```
+253K, +0.07% code size

# Release build
Before
```
   textdata bss dec hex filename
144123975   8374648  483140 152981763   91e5103 bin/clang
```
After
```
   textdata bss dec hex filename
144255762   8447296  483268 153186326   9217016 bin/clang
```
+204K, +0.13%

Authored-by: Kito Cheng 
Co-Authored-by: Hsiangkai Wang 

Reviewed By: khchen, aaron.ballman

Differential Revision: https://reviews.llvm.org/D111617

Added: 
clang/include/clang/Sema/RISCVIntrinsicManager.h
clang/lib/Sema/SemaRISCVVectorLookup.cpp
clang/test/Sema/riscv-bad-intrinsic-pragma.c
clang/test/Sema/riscv-intrinsic-pragma.c

Modified: 
clang/include/clang/Basic/CMakeLists.txt
clang/include/clang/Basic/TokenKinds.def
clang/include/clang/Parse/Parser.h
clang/include/clang/Sema/Sema.h
clang/include/clang/Support/RISCVVIntrinsicUtils.h
clang/lib/Parse/ParsePragma.cpp
clang/lib/Sema/CMakeLists.txt
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaLookup.cpp
clang/lib/Support/RISCVVIntrinsicUtils.cpp
clang/utils/TableGen/RISCVVEmitter.cpp
clang/utils/TableGen/TableGen.cpp
clang/utils/TableGen/TableGenBackends.h

Removed: 




diff  --git a/clang/include/clang/Basic/CMakeLists.txt 
b/clang/include/clang/Basic/CMakeLists.txt
index 8cd891385a483..b930842ae8cfd 100644
--- a/clang/include/clang/Basic/CMakeLists.txt
+++ b/clang/include/clang/Basic/CMakeLists.txt
@@ -90,3 +90,6 @@ clang_tablegen(riscv_vector_builtins.inc 
-gen-riscv-vector-builtins
 clang_tablegen(riscv_vector_builtin_cg.inc -gen-riscv-vector-builtin-codegen
   SOURCE riscv_vector.td
   TARGET ClangRISCVVectorBuiltinCG)
+clang_tablegen(riscv_vector_builtin_sema.inc -gen-riscv-vector-builtin-sema
+  SOURCE riscv_vector.td
+  TARGET ClangRISCVVectorBuiltinSema)

diff  --git a/clang/include/clang/Basic/TokenKinds.def 
b/clang/include/clang/Basic/TokenKinds.def
index

[PATCH] D130519: [clang][dataflow] Add explicit "AST" nodes for implications and iff

2022-07-26 Thread Stanislav Gatev via Phabricator via cfe-commits

sgatev accepted this revision.
sgatev added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/Value.h:40-41
 
 // Synthetic boolean values are either atomic values or composites that
 // represent conjunctions, disjunctions, and negations.
 AtomicBool,





Comment at: clang/lib/Analysis/FlowSensitive/DebugSupport.cpp:110
+  auto R = debugString(BV.getRightSubValue(), Depth + 1);
+  S = formatv("(=\n{0}\n{1})", L, R);
+  break;

I think `<=>` would be more natural.



Comment at: clang/unittests/Analysis/FlowSensitive/SolverTest.cpp:201
+
+  expectUnsatisfiable(solve({NotEquivalent}));
+}

Let's add a label: `!((X <=> Y) <=> ((X ^ Y) v (!X ^ !Y)))`



Comment at: clang/unittests/Analysis/FlowSensitive/SolverTest.cpp:303
+
+  expectUnsatisfiable(solve({NotEquivalent}));
+}

Let's add a label: `!((X => Y) <=> (!X v Y))`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130519/new/

https://reviews.llvm.org/D130519

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

[PATCH] D130553: [clang][lld][cmake] Simplify header dirs

2022-07-26 Thread John Ericson via Phabricator via cfe-commits

Ericson2314 created this revision.
Ericson2314 added reviewers: beanz, sebastian-ne, mstorsjo.
Herald added a subscriber: mgorny.
Herald added a project: All.
Ericson2314 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

We don't need to recompute the list LLVMConfig.cmake provides us.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130553

Files:
  clang/CMakeLists.txt
  lld/CMakeLists.txt


Index: lld/CMakeLists.txt
===
--- lld/CMakeLists.txt
+++ lld/CMakeLists.txt
@@ -70,13 +70,15 @@
   if (NOT LLVM_CONFIG_FOUND)
 # Pull values from LLVMConfig.cmake.  We can drop this once the llvm-config
 # path is removed.
-set(MAIN_INCLUDE_DIR "${LLVM_INCLUDE_DIR}")
+set(INCLUDE_DIRS ${LLVM_INCLUDE_DIRS})
 set(LLVM_OBJ_DIR "${LLVM_BINARY_DIR}")
 # N.B. this is just a default value, the CACHE PATHs below can be 
overridden.
 set(MAIN_SRC_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../llvm")
+  else()
+set(INCLUDE_DIRS "${LLVM_BINARY_DIR}/include" "${MAIN_INCLUDE_DIR}")
   endif()
 
-  set(LLVM_MAIN_INCLUDE_DIR "${MAIN_INCLUDE_DIR}" CACHE PATH "Path to 
llvm/include")
+  set(LLVM_INCLUDE_DIRS ${INCLUDE_DIRS} CACHE PATH "Path to llvm/include and 
any other header dirs needed")
   set(LLVM_BINARY_DIR "${LLVM_OBJ_ROOT}" CACHE PATH "Path to LLVM build tree")
   set(LLVM_MAIN_SRC_DIR "${MAIN_SRC_DIR}" CACHE PATH "Path to LLVM source 
tree")
 
@@ -95,7 +97,7 @@
 
   set(PACKAGE_VERSION "${LLVM_PACKAGE_VERSION}")
 
-  include_directories("${LLVM_BINARY_DIR}/include" ${LLVM_INCLUDE_DIRS})
+  include_directories(${LLVM_INCLUDE_DIRS})
   link_directories(${LLVM_LIBRARY_DIRS})
 
   if(LLVM_INCLUDE_TESTS)
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -78,15 +78,17 @@
   if (NOT LLVM_CONFIG_FOUND)
 # Pull values from LLVMConfig.cmake.  We can drop this once the llvm-config
 # path is removed.
-set(MAIN_INCLUDE_DIR "${LLVM_INCLUDE_DIR}")
+set(INCLUDE_DIRS ${LLVM_INCLUDE_DIRS})
 set(LLVM_OBJ_DIR "${LLVM_BINARY_DIR}")
 # N.B. this is just a default value, the CACHE PATHs below can be 
overriden.
 set(MAIN_SRC_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../llvm")
 set(TOOLS_BINARY_DIR "${LLVM_TOOLS_BINARY_DIR}")
 set(LIBRARY_DIR "${LLVM_LIBRARY_DIR}")
+  else()
+set(INCLUDE_DIRS "${LLVM_BINARY_DIR}/include" "${MAIN_INCLUDE_DIR}")
   endif()
 
-  set(LLVM_MAIN_INCLUDE_DIR "${MAIN_INCLUDE_DIR}" CACHE PATH "Path to 
llvm/include")
+  set(LLVM_INCLUDE_DIRS ${INCLUDE_DIRS} CACHE PATH "Path to llvm/include and 
any other header dirs needed")
   set(LLVM_BINARY_DIR "${LLVM_OBJ_ROOT}" CACHE PATH "Path to LLVM build tree")
   set(LLVM_MAIN_SRC_DIR "${MAIN_SRC_DIR}" CACHE PATH "Path to LLVM source 
tree")
   set(LLVM_TOOLS_BINARY_DIR "${TOOLS_BINARY_DIR}" CACHE PATH "Path to 
llvm/bin")
@@ -128,7 +130,7 @@
 set(LLVM_INCLUDE_TESTS ON)
   endif()
 
-  include_directories("${LLVM_BINARY_DIR}/include" "${LLVM_MAIN_INCLUDE_DIR}")
+  include_directories(${LLVM_INCLUDE_DIRS})
   link_directories("${LLVM_LIBRARY_DIR}")
 
   set( CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin )


Index: lld/CMakeLists.txt
===
--- lld/CMakeLists.txt
+++ lld/CMakeLists.txt
@@ -70,13 +70,15 @@
   if (NOT LLVM_CONFIG_FOUND)
 # Pull values from LLVMConfig.cmake.  We can drop this once the llvm-config
 # path is removed.
-set(MAIN_INCLUDE_DIR "${LLVM_INCLUDE_DIR}")
+set(INCLUDE_DIRS ${LLVM_INCLUDE_DIRS})
 set(LLVM_OBJ_DIR "${LLVM_BINARY_DIR}")
 # N.B. this is just a default value, the CACHE PATHs below can be overridden.
 set(MAIN_SRC_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../llvm")
+  else()
+set(INCLUDE_DIRS "${LLVM_BINARY_DIR}/include" "${MAIN_INCLUDE_DIR}")
   endif()
 
-  set(LLVM_MAIN_INCLUDE_DIR "${MAIN_INCLUDE_DIR}" CACHE PATH "Path to llvm/include")
+  set(LLVM_INCLUDE_DIRS ${INCLUDE_DIRS} CACHE PATH "Path to llvm/include and any other header dirs needed")
   set(LLVM_BINARY_DIR "${LLVM_OBJ_ROOT}" CACHE PATH "Path to LLVM build tree")
   set(LLVM_MAIN_SRC_DIR "${MAIN_SRC_DIR}" CACHE PATH "Path to LLVM source tree")
 
@@ -95,7 +97,7 @@
 
   set(PACKAGE_VERSION "${LLVM_PACKAGE_VERSION}")
 
-  include_directories("${LLVM_BINARY_DIR}/include" ${LLVM_INCLUDE_DIRS})
+  include_directories(${LLVM_INCLUDE_DIRS})
   link_directories(${LLVM_LIBRARY_DIRS})
 
   if(LLVM_INCLUDE_TESTS)
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -78,15 +78,17 @@
   if (NOT LLVM_CONFIG_FOUND)
 # Pull values from LLVMConfig.cmake.  We can drop this once the llvm-config
 # path is removed.
-set(MAIN_INCLUDE_DIR "${LLVM_INCLUDE_DIR}")
+set(INCLUDE_DIRS ${LLVM_INCLUDE_DIRS})

[PATCH] D129967: [analyzer] Lambda capture non-POD type array

2022-07-26 Thread Domján Dániel via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG996b092c5e17: [analyzer] Lambda capture non-POD type array 
(authored by isuckatcs).
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129967/new/

https://reviews.llvm.org/D129967

Files:
  clang/include/clang/Analysis/CFG.h
  clang/include/clang/Analysis/ConstructionContext.h
  clang/lib/Analysis/CFG.cpp
  clang/lib/Analysis/ConstructionContext.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/test/Analysis/array-init-loop.cpp
  clang/test/Analysis/lambdas.cpp

Index: clang/test/Analysis/lambdas.cpp
===
--- clang/test/Analysis/lambdas.cpp
+++ clang/test/Analysis/lambdas.cpp
@@ -400,7 +400,7 @@
 // CHECK: [B1]
 // CHECK:   1: x
 // CHECK:   2: [B1.1] (ImplicitCastExpr, NoOp, const struct X)
-// CHECK:   3: [B1.2] (CXXConstructExpr, struct X)
+// CHECK:   3: [B1.2] (CXXConstructExpr[B1.4]+0, struct X)
 // CHECK:   4: [x] {
 // CHECK:}
 // CHECK:   5: (void)[B1.4] (CStyleCastExpr, ToVoid, void)
@@ -408,4 +408,3 @@
 // CHECK:   Succs (1): B0
 // CHECK: [B0 (EXIT)]
 // CHECK:   Preds (1): B1
-
Index: clang/test/Analysis/array-init-loop.cpp
===
--- clang/test/Analysis/array-init-loop.cpp
+++ clang/test/Analysis/array-init-loop.cpp
@@ -160,6 +160,11 @@
   int i;
 };
 
+// The duplicate is required to emit a warning at 2 different places. 
+struct S3_duplicate {
+  int i;
+};
+
 void array_uninit_non_pod() {
   S3 arr[1];
 
@@ -170,24 +175,23 @@
   S2::c = 0;
   S2 arr[4];
 
-  // FIXME: These should be TRUE, but we fail to capture the array properly.
   auto l = [arr] { return arr[0].i; }();
-  clang_analyzer_eval(l == 2); // expected-warning{{TRUE}} // expected-warning{{FALSE}}
+  clang_analyzer_eval(l == 2); // expected-warning{{TRUE}}
 
   l = [arr] { return arr[1].i; }();
-  clang_analyzer_eval(l == 3); // expected-warning{{TRUE}} // expected-warning{{FALSE}}
+  clang_analyzer_eval(l == 3); // expected-warning{{TRUE}}
 
   l = [arr] { return arr[2].i; }();
-  clang_analyzer_eval(l == 4); // expected-warning{{TRUE}} // expected-warning{{FALSE}}
+  clang_analyzer_eval(l == 4); // expected-warning{{TRUE}}
 
   l = [arr] { return arr[3].i; }();
-  clang_analyzer_eval(l == 5); // expected-warning{{TRUE}} // expected-warning{{FALSE}}
+  clang_analyzer_eval(l == 5); // expected-warning{{TRUE}}
 }
 
 void lambda_uninit_non_pod() {
-  S3 arr[4];
+  S3_duplicate arr[4];
 
-  int l = [arr] { return arr[3].i; }();
+  int l = [arr] { return arr[3].i; }(); // expected-warning@164{{ in implicit constructor is garbage or undefined }}
 }
 
 // If this struct is being copy/move constructed by the implicit ctors, ArrayInitLoopExpr
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -290,6 +290,23 @@
 
   return loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(E, LCtx));
 }
+case ConstructionContext::LambdaCaptureKind: {
+  CallOpts.IsTemporaryCtorOrDtor = true;
+
+  const auto *LCC = cast(CC);
+
+  SVal Base = loc::MemRegionVal(
+  MRMgr.getCXXTempObjectRegion(LCC->getInitializer(), LCtx));
+
+  const auto *CE = dyn_cast_or_null(E);
+  if (getIndexOfElementToConstruct(State, CE, LCtx)) {
+CallOpts.IsArrayCtorOrDtor = true;
+Base = State->getLValue(E->getType(), svalBuilder.makeArrayIndex(Idx),
+Base);
+  }
+
+  return Base;
+}
 case ConstructionContext::ArgumentKind: {
   // Arguments are technically temporaries.
   CallOpts.IsTemporaryCtorOrDtor = true;
@@ -450,6 +467,17 @@
 
   return State;
 }
+case ConstructionContext::LambdaCaptureKind: {
+  const auto *LCC = cast(CC);
+
+  // If we capture and array, we want to store the super region, not a
+  // sub-region.
+  if (const auto *EL = dyn_cast_or_null(V.getAsRegion()))
+V = loc::MemRegionVal(EL->getSuperRegion());
+
+  return addObjectUnderConstruction(
+  State, {LCC->getLambdaExpr(), LCC->getIndex()}, LCtx, V);
+}
 case ConstructionContext::ArgumentKind: {
   const auto *ACC = cast(CC);
   if (const auto *BTE = ACC->getCXXBindTemporaryExpr())
@@ -1105,19 +1133,40 @@
 
   // If we created a new MemRegion for the lambda, we should explicitly bind
   // the captures.
+  unsigned Idx = 0;
   CXXRecordDecl::field_iterator CurField = LE->getLambdaClass()->field_begin();
   for (LambdaExpr::const_capture_init_iterator i = LE->capture_init_begin(),
e = LE->capture_init_end();
-   i != e; ++i, ++CurField) {
+   i

[clang] 996b092 - [analyzer] Lambda capture non-POD type array

2022-07-26 Thread via cfe-commits


Author: isuckatcs
Date: 2022-07-26T09:40:25+02:00
New Revision: 996b092c5e170786572e925345e502e5376feaee

URL: 
https://github.com/llvm/llvm-project/commit/996b092c5e170786572e925345e502e5376feaee
DIFF: 
https://github.com/llvm/llvm-project/commit/996b092c5e170786572e925345e502e5376feaee.diff

LOG: [analyzer] Lambda capture non-POD type array

This patch introduces a new `ConstructionContext` for
lambda capture. This `ConstructionContext` allows the
analyzer to construct the captured object directly into
it's final region, and makes it possible to capture
non-POD arrays.

Differential Revision: https://reviews.llvm.org/D129967

Added: 


Modified: 
clang/include/clang/Analysis/CFG.h
clang/include/clang/Analysis/ConstructionContext.h
clang/lib/Analysis/CFG.cpp
clang/lib/Analysis/ConstructionContext.cpp
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
clang/test/Analysis/array-init-loop.cpp
clang/test/Analysis/lambdas.cpp

Removed: 




diff  --git a/clang/include/clang/Analysis/CFG.h 
b/clang/include/clang/Analysis/CFG.h
index d8e7e1e43d81..4f16a6361950 100644
--- a/clang/include/clang/Analysis/CFG.h
+++ b/clang/include/clang/Analysis/CFG.h
@@ -202,7 +202,8 @@ class CFGCXXRecordTypedCall : public CFGStmt {
  isa(C) ||
  isa(C) ||
  isa(C) ||
- isa(C)));
+ isa(C) ||
+ isa(C)));
 Data2.setPointer(const_cast(C));
   }
 

diff  --git a/clang/include/clang/Analysis/ConstructionContext.h 
b/clang/include/clang/Analysis/ConstructionContext.h
index 7ff1a007f8ee..67a091199b91 100644
--- a/clang/include/clang/Analysis/ConstructionContext.h
+++ b/clang/include/clang/Analysis/ConstructionContext.h
@@ -36,13 +36,14 @@ class ConstructionContextItem {
 ElidedDestructorKind,
 ElidableConstructorKind,
 ArgumentKind,
-STATEMENT_WITH_INDEX_KIND_BEGIN=ArgumentKind,
-STATEMENT_WITH_INDEX_KIND_END=ArgumentKind,
+LambdaCaptureKind,
+STATEMENT_WITH_INDEX_KIND_BEGIN = ArgumentKind,
+STATEMENT_WITH_INDEX_KIND_END = LambdaCaptureKind,
 STATEMENT_KIND_BEGIN = VariableKind,
-STATEMENT_KIND_END = ArgumentKind,
+STATEMENT_KIND_END = LambdaCaptureKind,
 InitializerKind,
-INITIALIZER_KIND_BEGIN=InitializerKind,
-INITIALIZER_KIND_END=InitializerKind
+INITIALIZER_KIND_BEGIN = InitializerKind,
+INITIALIZER_KIND_END = InitializerKind
   };
 
   LLVM_DUMP_METHOD static StringRef getKindAsString(ItemKind K) {
@@ -55,6 +56,8 @@ class ConstructionContextItem {
   case ElidedDestructorKind:return "elide destructor";
   case ElidableConstructorKind: return "elide constructor";
   case ArgumentKind:return "construct into argument";
+  case LambdaCaptureKind:
+return "construct into lambda captured variable";
   case InitializerKind: return "construct into member variable";
 };
 llvm_unreachable("Unknown ItemKind");
@@ -72,7 +75,7 @@ class ConstructionContextItem {
 
   bool hasIndex() const {
 return Kind >= STATEMENT_WITH_INDEX_KIND_BEGIN &&
-   Kind >= STATEMENT_WITH_INDEX_KIND_END;
+   Kind <= STATEMENT_WITH_INDEX_KIND_END;
   }
 
   bool hasInitializer() const {
@@ -127,6 +130,9 @@ class ConstructionContextItem {
   ConstructionContextItem(const CXXCtorInitializer *Init)
   : Data(Init), Kind(InitializerKind), Index(0) {}
 
+  ConstructionContextItem(const LambdaExpr *LE, unsigned Index)
+  : Data(LE), Kind(LambdaCaptureKind), Index(Index) {}
+
   ItemKind getKind() const { return Kind; }
 
   LLVM_DUMP_METHOD StringRef getKindAsString() const {
@@ -254,7 +260,8 @@ class ConstructionContext {
 CXX17ElidedCopyReturnedValueKind,
 RETURNED_VALUE_BEGIN = SimpleReturnedValueKind,
 RETURNED_VALUE_END = CXX17ElidedCopyReturnedValueKind,
-ArgumentKind
+ArgumentKind,
+LambdaCaptureKind
   };
 
 protected:
@@ -674,6 +681,42 @@ class ArgumentConstructionContext : public 
ConstructionContext {
   }
 };
 
+class LambdaCaptureConstructionContext : public ConstructionContext {
+  // The lambda of which the initializer we capture.
+  const LambdaExpr *LE;
+
+  // Index of the captured element in the captured list.
+  unsigned Index;
+
+  friend class ConstructionContext; // Allows to create<>() itself.
+
+  explicit LambdaCaptureConstructionContext(const LambdaExpr *LE,
+unsigned Index)
+  : ConstructionContext(LambdaCaptureKind), LE(LE), Index(Index) {}
+
+public:
+  const LambdaExpr *getLambdaExpr() const { return LE; }
+  unsigned getIndex() const { return Index; }
+
+  const Expr *getInitializer() const {
+return *(LE->capture_init_begin() + Index);
+  }
+
+  const FieldDecl *getFieldDecl() const {
+auto It = LE->getLambdaClass()->field_begin();
+std::advance(It,

[PATCH] D111617: [RISCV] Lazily add RVV C intrinsics.

2022-07-26 Thread Kito Cheng via Phabricator via cfe-commits

kito-cheng marked 9 inline comments as done.
kito-cheng added a comment.

@aaron.ballman thanks for your review!




Comment at: clang/include/clang/Support/RISCVVIntrinsicUtils.h:400
+  // Number of fields, greater than 1 if it's segment load/store.
+  uint8_t NF;
+};

aaron.ballman wrote:
> 
That the term used in RVV spec, so I keep this as `NF` :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111617/new/

https://reviews.llvm.org/D111617

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

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

2022-07-26 Thread Chuanqi Xu via Phabricator via cfe-commits

ChuanqiXu added inline comments.



Comment at: clang/lib/Sema/Scope.cpp:152-154
+  // Consider the variable as NRVO candidate if the return slot is available
+  // for it in the current scope, or if it can be available in outer scopes.
+  NRVO = CanBePutInReturnSlot ? VD : nullptr;

rusyaev-roman wrote:
> rusyaev-roman wrote:
> > rusyaev-roman wrote:
> > > ChuanqiXu wrote:
> > > > rusyaev-roman wrote:
> > > > > ChuanqiXu wrote:
> > > > > > rusyaev-roman wrote:
> > > > > > > ChuanqiXu wrote:
> > > > > > > > rusyaev-roman wrote:
> > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > rusyaev-roman wrote:
> > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > rusyaev-roman wrote:
> > > > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > > > What if NRVO contains a value already? It is 
> > > > > > > > > > > > > > possible due to the value of NRVO could be set by 
> > > > > > > > > > > > > > its children.
> > > > > > > > > > > > > Actually this is intention. If the parent has already 
> > > > > > > > > > > > > NRVO candidate, then it should be invalidated (or 
> > > > > > > > > > > > > not). Let's consider the following examples:
> > > > > > > > > > > > > 
> > > > > > > > > > > > > 
> > > > > > > > > > > > > ```
> > > > > > > > > > > > > X foo(bool b) {
> > > > > > > > > > > > >X x;
> > > > > > > > > > > > >X y;
> > > > > > > > > > > > >if (b)
> > > > > > > > > > > > >   return x;
> > > > > > > > > > > > >else
> > > > > > > > > > > > >   return y; // when we process this return 
> > > > > > > > > > > > > statement, the parent has already NRVO and it will be 
> > > > > > > > > > > > > invalidated (this is correct behavior)
> > > > > > > > > > > > > }
> > > > > > > > > > > > > ```
> > > > > > > > > > > > > 
> > > > > > > > > > > > > ```
> > > > > > > > > > > > > X foo(bool b) {
> > > > > > > > > > > > >X x;
> > > > > > > > > > > > >if (b)
> > > > > > > > > > > > >   return x;
> > > > > > > > > > > > >
> > > > > > > > > > > > >X y;
> > > > > > > > > > > > >// when we process this return statement, the 
> > > > > > > > > > > > > parent has already NRVO and it WON't be invalidated
> > > > > > > > > > > > >//  (this is correct behavior), because a return 
> > > > > > > > > > > > > slot will be available for it
> > > > > > > > > > > > >return y;
> > > > > > > > > > > > > }
> > > > > > > > > > > > > ```
> > > > > > > > > > > > > 
> > > > > > > > > > > > > ```
> > > > > > > > > > > > > X foo(bool b) {
> > > > > > > > > > > > >X x;
> > > > > > > > > > > > >if (b)
> > > > > > > > > > > > >   return x;
> > > > > > > > > > > > > 
> > > > > > > > > > > > >// when we process this return statement, the 
> > > > > > > > > > > > > parent has already NRVO and it WON't be invalidated 
> > > > > > > > > > > > > (this is correct behavior)
> > > > > > > > > > > > >return x;
> > > > > > > > > > > > > }
> > > > > > > > > > > > > ```
> > > > > > > > > > > > > 
> > > > > > > > > > > > > ```
> > > > > > > > > > > > > X foo(bool b, X x) {
> > > > > > > > > > > > >X y;
> > > > > > > > > > > > >
> > > > > > > > > > > > >if (b)
> > > > > > > > > > > > >   return x;
> > > > > > > > > > > > > 
> > > > > > > > > > > > >// when we process this return statement, the 
> > > > > > > > > > > > > parent contains nullptr (invalid candidate) and it 
> > > > > > > > > > > > > will be invalidated (this is correct behavior)
> > > > > > > > > > > > >return y;
> > > > > > > > > > > > > }
> > > > > > > > > > > > > ```
> > > > > > > > > > > > > 
> > > > > > > > > > > > > ```
> > > > > > > > > > > > > X foo(bool b, X x) {
> > > > > > > > > > > > >if (b)
> > > > > > > > > > > > >   return x;
> > > > > > > > > > > > > 
> > > > > > > > > > > > >X y;
> > > > > > > > > > > > >// when we process this return statement, the 
> > > > > > > > > > > > > parent contains nullptr (invalid candidate) and it 
> > > > > > > > > > > > > WON't be invalidated (this is correct behavior)
> > > > > > > > > > > > >return y;
> > > > > > > > > > > > > }
> > > > > > > > > > > > > ```
> > > > > > > > > > > > Oh, I see. Tricky. I don't find invalid cases now. But 
> > > > > > > > > > > > I recommend to comment that the children would maintain 
> > > > > > > > > > > > the `ReturnSlots` of their parents. (This is 
> > > > > > > > > > > > anti-intuition)
> > > > > > > > > > > > 
> > > > > > > > > > > > Have you tested any larger projects? Like libc++, 
> > > > > > > > > > > > libstdc++ or something like folly. I feel we need to do 
> > > > > > > > > > > > such tests to avoid we get anything wrong.
> > > > > > > > > > > I've already added a comment at the beginning of 
> > > > > > > > > > > `updateNRVOCandidate` function where this point is 
> > > > > > > > > > > mentioned: 
> > > > > > > > > > > ```
> > > > > > > > > > > //  ... Therefore, we need to clear return slots for 
> > > > > > > > > > > other
> > > > > > > > > >

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

2022-07-26 Thread Roman Rusyaev via Phabricator via cfe-commits

rusyaev-roman added inline comments.



Comment at: clang/lib/Sema/Scope.cpp:152-154
+  // Consider the variable as NRVO candidate if the return slot is available
+  // for it in the current scope, or if it can be available in outer scopes.
+  NRVO = CanBePutInReturnSlot ? VD : nullptr;

rusyaev-roman wrote:
> rusyaev-roman wrote:
> > ChuanqiXu wrote:
> > > rusyaev-roman wrote:
> > > > ChuanqiXu wrote:
> > > > > rusyaev-roman wrote:
> > > > > > ChuanqiXu wrote:
> > > > > > > rusyaev-roman wrote:
> > > > > > > > ChuanqiXu wrote:
> > > > > > > > > rusyaev-roman wrote:
> > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > rusyaev-roman wrote:
> > > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > > What if NRVO contains a value already? It is possible 
> > > > > > > > > > > > > due to the value of NRVO could be set by its children.
> > > > > > > > > > > > Actually this is intention. If the parent has already 
> > > > > > > > > > > > NRVO candidate, then it should be invalidated (or not). 
> > > > > > > > > > > > Let's consider the following examples:
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > ```
> > > > > > > > > > > > X foo(bool b) {
> > > > > > > > > > > >X x;
> > > > > > > > > > > >X y;
> > > > > > > > > > > >if (b)
> > > > > > > > > > > >   return x;
> > > > > > > > > > > >else
> > > > > > > > > > > >   return y; // when we process this return 
> > > > > > > > > > > > statement, the parent has already NRVO and it will be 
> > > > > > > > > > > > invalidated (this is correct behavior)
> > > > > > > > > > > > }
> > > > > > > > > > > > ```
> > > > > > > > > > > > 
> > > > > > > > > > > > ```
> > > > > > > > > > > > X foo(bool b) {
> > > > > > > > > > > >X x;
> > > > > > > > > > > >if (b)
> > > > > > > > > > > >   return x;
> > > > > > > > > > > >
> > > > > > > > > > > >X y;
> > > > > > > > > > > >// when we process this return statement, the parent 
> > > > > > > > > > > > has already NRVO and it WON't be invalidated
> > > > > > > > > > > >//  (this is correct behavior), because a return 
> > > > > > > > > > > > slot will be available for it
> > > > > > > > > > > >return y;
> > > > > > > > > > > > }
> > > > > > > > > > > > ```
> > > > > > > > > > > > 
> > > > > > > > > > > > ```
> > > > > > > > > > > > X foo(bool b) {
> > > > > > > > > > > >X x;
> > > > > > > > > > > >if (b)
> > > > > > > > > > > >   return x;
> > > > > > > > > > > > 
> > > > > > > > > > > >// when we process this return statement, the parent 
> > > > > > > > > > > > has already NRVO and it WON't be invalidated (this is 
> > > > > > > > > > > > correct behavior)
> > > > > > > > > > > >return x;
> > > > > > > > > > > > }
> > > > > > > > > > > > ```
> > > > > > > > > > > > 
> > > > > > > > > > > > ```
> > > > > > > > > > > > X foo(bool b, X x) {
> > > > > > > > > > > >X y;
> > > > > > > > > > > >
> > > > > > > > > > > >if (b)
> > > > > > > > > > > >   return x;
> > > > > > > > > > > > 
> > > > > > > > > > > >// when we process this return statement, the parent 
> > > > > > > > > > > > contains nullptr (invalid candidate) and it will be 
> > > > > > > > > > > > invalidated (this is correct behavior)
> > > > > > > > > > > >return y;
> > > > > > > > > > > > }
> > > > > > > > > > > > ```
> > > > > > > > > > > > 
> > > > > > > > > > > > ```
> > > > > > > > > > > > X foo(bool b, X x) {
> > > > > > > > > > > >if (b)
> > > > > > > > > > > >   return x;
> > > > > > > > > > > > 
> > > > > > > > > > > >X y;
> > > > > > > > > > > >// when we process this return statement, the parent 
> > > > > > > > > > > > contains nullptr (invalid candidate) and it WON't be 
> > > > > > > > > > > > invalidated (this is correct behavior)
> > > > > > > > > > > >return y;
> > > > > > > > > > > > }
> > > > > > > > > > > > ```
> > > > > > > > > > > Oh, I see. Tricky. I don't find invalid cases now. But I 
> > > > > > > > > > > recommend to comment that the children would maintain the 
> > > > > > > > > > > `ReturnSlots` of their parents. (This is anti-intuition)
> > > > > > > > > > > 
> > > > > > > > > > > Have you tested any larger projects? Like libc++, 
> > > > > > > > > > > libstdc++ or something like folly. I feel we need to do 
> > > > > > > > > > > such tests to avoid we get anything wrong.
> > > > > > > > > > I've already added a comment at the beginning of 
> > > > > > > > > > `updateNRVOCandidate` function where this point is 
> > > > > > > > > > mentioned: 
> > > > > > > > > > ```
> > > > > > > > > > //  ... Therefore, we need to clear return slots for 
> > > > > > > > > > other
> > > > > > > > > > //  variables defined before the current return 
> > > > > > > > > > statement in the current
> > > > > > > > > > //  scope and in outer scopes.
> > > > > > > > > > ```
> > > > > > > > > > If it's not enough, please let me know.
> > > > > > > > > > 
> > > > > >

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

2022-07-26 Thread Roman Rusyaev via Phabricator via cfe-commits

rusyaev-roman added inline comments.



Comment at: clang/lib/Sema/Scope.cpp:152-154
+  // Consider the variable as NRVO candidate if the return slot is available
+  // for it in the current scope, or if it can be available in outer scopes.
+  NRVO = CanBePutInReturnSlot ? VD : nullptr;

rusyaev-roman wrote:
> ChuanqiXu wrote:
> > rusyaev-roman wrote:
> > > ChuanqiXu wrote:
> > > > rusyaev-roman wrote:
> > > > > ChuanqiXu wrote:
> > > > > > rusyaev-roman wrote:
> > > > > > > ChuanqiXu wrote:
> > > > > > > > rusyaev-roman wrote:
> > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > rusyaev-roman wrote:
> > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > What if NRVO contains a value already? It is possible 
> > > > > > > > > > > > due to the value of NRVO could be set by its children.
> > > > > > > > > > > Actually this is intention. If the parent has already 
> > > > > > > > > > > NRVO candidate, then it should be invalidated (or not). 
> > > > > > > > > > > Let's consider the following examples:
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > ```
> > > > > > > > > > > X foo(bool b) {
> > > > > > > > > > >X x;
> > > > > > > > > > >X y;
> > > > > > > > > > >if (b)
> > > > > > > > > > >   return x;
> > > > > > > > > > >else
> > > > > > > > > > >   return y; // when we process this return statement, 
> > > > > > > > > > > the parent has already NRVO and it will be invalidated 
> > > > > > > > > > > (this is correct behavior)
> > > > > > > > > > > }
> > > > > > > > > > > ```
> > > > > > > > > > > 
> > > > > > > > > > > ```
> > > > > > > > > > > X foo(bool b) {
> > > > > > > > > > >X x;
> > > > > > > > > > >if (b)
> > > > > > > > > > >   return x;
> > > > > > > > > > >
> > > > > > > > > > >X y;
> > > > > > > > > > >// when we process this return statement, the parent 
> > > > > > > > > > > has already NRVO and it WON't be invalidated
> > > > > > > > > > >//  (this is correct behavior), because a return slot 
> > > > > > > > > > > will be available for it
> > > > > > > > > > >return y;
> > > > > > > > > > > }
> > > > > > > > > > > ```
> > > > > > > > > > > 
> > > > > > > > > > > ```
> > > > > > > > > > > X foo(bool b) {
> > > > > > > > > > >X x;
> > > > > > > > > > >if (b)
> > > > > > > > > > >   return x;
> > > > > > > > > > > 
> > > > > > > > > > >// when we process this return statement, the parent 
> > > > > > > > > > > has already NRVO and it WON't be invalidated (this is 
> > > > > > > > > > > correct behavior)
> > > > > > > > > > >return x;
> > > > > > > > > > > }
> > > > > > > > > > > ```
> > > > > > > > > > > 
> > > > > > > > > > > ```
> > > > > > > > > > > X foo(bool b, X x) {
> > > > > > > > > > >X y;
> > > > > > > > > > >
> > > > > > > > > > >if (b)
> > > > > > > > > > >   return x;
> > > > > > > > > > > 
> > > > > > > > > > >// when we process this return statement, the parent 
> > > > > > > > > > > contains nullptr (invalid candidate) and it will be 
> > > > > > > > > > > invalidated (this is correct behavior)
> > > > > > > > > > >return y;
> > > > > > > > > > > }
> > > > > > > > > > > ```
> > > > > > > > > > > 
> > > > > > > > > > > ```
> > > > > > > > > > > X foo(bool b, X x) {
> > > > > > > > > > >if (b)
> > > > > > > > > > >   return x;
> > > > > > > > > > > 
> > > > > > > > > > >X y;
> > > > > > > > > > >// when we process this return statement, the parent 
> > > > > > > > > > > contains nullptr (invalid candidate) and it WON't be 
> > > > > > > > > > > invalidated (this is correct behavior)
> > > > > > > > > > >return y;
> > > > > > > > > > > }
> > > > > > > > > > > ```
> > > > > > > > > > Oh, I see. Tricky. I don't find invalid cases now. But I 
> > > > > > > > > > recommend to comment that the children would maintain the 
> > > > > > > > > > `ReturnSlots` of their parents. (This is anti-intuition)
> > > > > > > > > > 
> > > > > > > > > > Have you tested any larger projects? Like libc++, libstdc++ 
> > > > > > > > > > or something like folly. I feel we need to do such tests to 
> > > > > > > > > > avoid we get anything wrong.
> > > > > > > > > I've already added a comment at the beginning of 
> > > > > > > > > `updateNRVOCandidate` function where this point is mentioned: 
> > > > > > > > > ```
> > > > > > > > > //  ... Therefore, we need to clear return slots for other
> > > > > > > > > //  variables defined before the current return statement 
> > > > > > > > > in the current
> > > > > > > > > //  scope and in outer scopes.
> > > > > > > > > ```
> > > > > > > > > If it's not enough, please let me know.
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > > Have you tested any larger projects?
> > > > > > > > > 
> > > > > > > > > Yes, I've built the `clang` itself and `compiler-rt` project. 
> > > > > > > > > Then I've checked them to run 'check-all' (on built clang and 
> > > > > > > > >

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

2022-07-26 Thread Roman Rusyaev via Phabricator via cfe-commits

rusyaev-roman added inline comments.



Comment at: clang/lib/Sema/Scope.cpp:152-154
+  // Consider the variable as NRVO candidate if the return slot is available
+  // for it in the current scope, or if it can be available in outer scopes.
+  NRVO = CanBePutInReturnSlot ? VD : nullptr;

ChuanqiXu wrote:
> rusyaev-roman wrote:
> > ChuanqiXu wrote:
> > > rusyaev-roman wrote:
> > > > ChuanqiXu wrote:
> > > > > rusyaev-roman wrote:
> > > > > > ChuanqiXu wrote:
> > > > > > > rusyaev-roman wrote:
> > > > > > > > ChuanqiXu wrote:
> > > > > > > > > rusyaev-roman wrote:
> > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > What if NRVO contains a value already? It is possible due 
> > > > > > > > > > > to the value of NRVO could be set by its children.
> > > > > > > > > > Actually this is intention. If the parent has already NRVO 
> > > > > > > > > > candidate, then it should be invalidated (or not). Let's 
> > > > > > > > > > consider the following examples:
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > ```
> > > > > > > > > > X foo(bool b) {
> > > > > > > > > >X x;
> > > > > > > > > >X y;
> > > > > > > > > >if (b)
> > > > > > > > > >   return x;
> > > > > > > > > >else
> > > > > > > > > >   return y; // when we process this return statement, 
> > > > > > > > > > the parent has already NRVO and it will be invalidated 
> > > > > > > > > > (this is correct behavior)
> > > > > > > > > > }
> > > > > > > > > > ```
> > > > > > > > > > 
> > > > > > > > > > ```
> > > > > > > > > > X foo(bool b) {
> > > > > > > > > >X x;
> > > > > > > > > >if (b)
> > > > > > > > > >   return x;
> > > > > > > > > >
> > > > > > > > > >X y;
> > > > > > > > > >// when we process this return statement, the parent has 
> > > > > > > > > > already NRVO and it WON't be invalidated
> > > > > > > > > >//  (this is correct behavior), because a return slot 
> > > > > > > > > > will be available for it
> > > > > > > > > >return y;
> > > > > > > > > > }
> > > > > > > > > > ```
> > > > > > > > > > 
> > > > > > > > > > ```
> > > > > > > > > > X foo(bool b) {
> > > > > > > > > >X x;
> > > > > > > > > >if (b)
> > > > > > > > > >   return x;
> > > > > > > > > > 
> > > > > > > > > >// when we process this return statement, the parent has 
> > > > > > > > > > already NRVO and it WON't be invalidated (this is correct 
> > > > > > > > > > behavior)
> > > > > > > > > >return x;
> > > > > > > > > > }
> > > > > > > > > > ```
> > > > > > > > > > 
> > > > > > > > > > ```
> > > > > > > > > > X foo(bool b, X x) {
> > > > > > > > > >X y;
> > > > > > > > > >
> > > > > > > > > >if (b)
> > > > > > > > > >   return x;
> > > > > > > > > > 
> > > > > > > > > >// when we process this return statement, the parent 
> > > > > > > > > > contains nullptr (invalid candidate) and it will be 
> > > > > > > > > > invalidated (this is correct behavior)
> > > > > > > > > >return y;
> > > > > > > > > > }
> > > > > > > > > > ```
> > > > > > > > > > 
> > > > > > > > > > ```
> > > > > > > > > > X foo(bool b, X x) {
> > > > > > > > > >if (b)
> > > > > > > > > >   return x;
> > > > > > > > > > 
> > > > > > > > > >X y;
> > > > > > > > > >// when we process this return statement, the parent 
> > > > > > > > > > contains nullptr (invalid candidate) and it WON't be 
> > > > > > > > > > invalidated (this is correct behavior)
> > > > > > > > > >return y;
> > > > > > > > > > }
> > > > > > > > > > ```
> > > > > > > > > Oh, I see. Tricky. I don't find invalid cases now. But I 
> > > > > > > > > recommend to comment that the children would maintain the 
> > > > > > > > > `ReturnSlots` of their parents. (This is anti-intuition)
> > > > > > > > > 
> > > > > > > > > Have you tested any larger projects? Like libc++, libstdc++ 
> > > > > > > > > or something like folly. I feel we need to do such tests to 
> > > > > > > > > avoid we get anything wrong.
> > > > > > > > I've already added a comment at the beginning of 
> > > > > > > > `updateNRVOCandidate` function where this point is mentioned: 
> > > > > > > > ```
> > > > > > > > //  ... Therefore, we need to clear return slots for other
> > > > > > > > //  variables defined before the current return statement 
> > > > > > > > in the current
> > > > > > > > //  scope and in outer scopes.
> > > > > > > > ```
> > > > > > > > If it's not enough, please let me know.
> > > > > > > > 
> > > > > > > > 
> > > > > > > > > Have you tested any larger projects?
> > > > > > > > 
> > > > > > > > Yes, I've built the `clang` itself and `compiler-rt` project. 
> > > > > > > > Then I've checked them to run 'check-all' (on built clang and 
> > > > > > > > compiler-rt). Everything  works.
> > > > > > > Great! Clang should be large enough.
> > > > > > Thanks a lot for the careful review!
> > > > > > 
> > > > > > @ChuanqiXu  , could you land this patch please?
> > > > > > 
> > > > > > Many thanks

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

2022-07-26 Thread Roman Rusyaev via Phabricator via cfe-commits

rusyaev-roman added a comment.

@ChuanqiXu , the release notes were updated. Could you check and merge please?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119792/new/

https://reviews.llvm.org/D119792

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

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

2022-07-26 Thread Chuanqi Xu via Phabricator via cfe-commits

ChuanqiXu added inline comments.



Comment at: clang/lib/Sema/Scope.cpp:152-154
+  // Consider the variable as NRVO candidate if the return slot is available
+  // for it in the current scope, or if it can be available in outer scopes.
+  NRVO = CanBePutInReturnSlot ? VD : nullptr;

rusyaev-roman wrote:
> ChuanqiXu wrote:
> > rusyaev-roman wrote:
> > > ChuanqiXu wrote:
> > > > rusyaev-roman wrote:
> > > > > ChuanqiXu wrote:
> > > > > > rusyaev-roman wrote:
> > > > > > > ChuanqiXu wrote:
> > > > > > > > rusyaev-roman wrote:
> > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > What if NRVO contains a value already? It is possible due 
> > > > > > > > > > to the value of NRVO could be set by its children.
> > > > > > > > > Actually this is intention. If the parent has already NRVO 
> > > > > > > > > candidate, then it should be invalidated (or not). Let's 
> > > > > > > > > consider the following examples:
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > ```
> > > > > > > > > X foo(bool b) {
> > > > > > > > >X x;
> > > > > > > > >X y;
> > > > > > > > >if (b)
> > > > > > > > >   return x;
> > > > > > > > >else
> > > > > > > > >   return y; // when we process this return statement, the 
> > > > > > > > > parent has already NRVO and it will be invalidated (this is 
> > > > > > > > > correct behavior)
> > > > > > > > > }
> > > > > > > > > ```
> > > > > > > > > 
> > > > > > > > > ```
> > > > > > > > > X foo(bool b) {
> > > > > > > > >X x;
> > > > > > > > >if (b)
> > > > > > > > >   return x;
> > > > > > > > >
> > > > > > > > >X y;
> > > > > > > > >// when we process this return statement, the parent has 
> > > > > > > > > already NRVO and it WON't be invalidated
> > > > > > > > >//  (this is correct behavior), because a return slot will 
> > > > > > > > > be available for it
> > > > > > > > >return y;
> > > > > > > > > }
> > > > > > > > > ```
> > > > > > > > > 
> > > > > > > > > ```
> > > > > > > > > X foo(bool b) {
> > > > > > > > >X x;
> > > > > > > > >if (b)
> > > > > > > > >   return x;
> > > > > > > > > 
> > > > > > > > >// when we process this return statement, the parent has 
> > > > > > > > > already NRVO and it WON't be invalidated (this is correct 
> > > > > > > > > behavior)
> > > > > > > > >return x;
> > > > > > > > > }
> > > > > > > > > ```
> > > > > > > > > 
> > > > > > > > > ```
> > > > > > > > > X foo(bool b, X x) {
> > > > > > > > >X y;
> > > > > > > > >
> > > > > > > > >if (b)
> > > > > > > > >   return x;
> > > > > > > > > 
> > > > > > > > >// when we process this return statement, the parent 
> > > > > > > > > contains nullptr (invalid candidate) and it will be 
> > > > > > > > > invalidated (this is correct behavior)
> > > > > > > > >return y;
> > > > > > > > > }
> > > > > > > > > ```
> > > > > > > > > 
> > > > > > > > > ```
> > > > > > > > > X foo(bool b, X x) {
> > > > > > > > >if (b)
> > > > > > > > >   return x;
> > > > > > > > > 
> > > > > > > > >X y;
> > > > > > > > >// when we process this return statement, the parent 
> > > > > > > > > contains nullptr (invalid candidate) and it WON't be 
> > > > > > > > > invalidated (this is correct behavior)
> > > > > > > > >return y;
> > > > > > > > > }
> > > > > > > > > ```
> > > > > > > > Oh, I see. Tricky. I don't find invalid cases now. But I 
> > > > > > > > recommend to comment that the children would maintain the 
> > > > > > > > `ReturnSlots` of their parents. (This is anti-intuition)
> > > > > > > > 
> > > > > > > > Have you tested any larger projects? Like libc++, libstdc++ or 
> > > > > > > > something like folly. I feel we need to do such tests to avoid 
> > > > > > > > we get anything wrong.
> > > > > > > I've already added a comment at the beginning of 
> > > > > > > `updateNRVOCandidate` function where this point is mentioned: 
> > > > > > > ```
> > > > > > > //  ... Therefore, we need to clear return slots for other
> > > > > > > //  variables defined before the current return statement in 
> > > > > > > the current
> > > > > > > //  scope and in outer scopes.
> > > > > > > ```
> > > > > > > If it's not enough, please let me know.
> > > > > > > 
> > > > > > > 
> > > > > > > > Have you tested any larger projects?
> > > > > > > 
> > > > > > > Yes, I've built the `clang` itself and `compiler-rt` project. 
> > > > > > > Then I've checked them to run 'check-all' (on built clang and 
> > > > > > > compiler-rt). Everything  works.
> > > > > > Great! Clang should be large enough.
> > > > > Thanks a lot for the careful review!
> > > > > 
> > > > > @ChuanqiXu  , could you land this patch please?
> > > > > 
> > > > > Many thanks to @Izaron for the original implementation.
> > > > Sure. What's your prefer Name and Mail address?
> > > Thanks!
> > > 
> > > Roman Rusyaev 
> > Oh, I forgot you need edit the ReleaseNotes at clang/docs/ReleaseNotes.rst
> I'm going to add a

[PATCH] D129496: [analyzer] ArrayInitLoopExpr with array of non-POD type.

2022-07-26 Thread Domján Dániel via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8a13326d184c: [analyzer] ArrayInitLoopExpr with array of 
non-POD type (authored by isuckatcs).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129496/new/

https://reviews.llvm.org/D129496

Files:
  clang/include/clang/Analysis/ConstructionContext.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  clang/lib/Analysis/CFG.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
  clang/test/Analysis/array-init-loop.cpp
  clang/test/Analysis/ctor.mm
  clang/test/Analysis/uninit-structured-binding-array.cpp

Index: clang/test/Analysis/uninit-structured-binding-array.cpp
===
--- clang/test/Analysis/uninit-structured-binding-array.cpp
+++ clang/test/Analysis/uninit-structured-binding-array.cpp
@@ -292,3 +292,97 @@
   clang_analyzer_eval(e == 5); // expected-warning{{UNKNOWN}}
   clang_analyzer_eval(f == 6); // expected-warning{{UNKNOWN}}
 }
+
+struct S {
+  int a = 1;
+  int b = 2;
+};
+
+void non_pod_val(void) {
+  S arr[2];
+
+  auto [x, y] = arr;
+
+  clang_analyzer_eval(x.a == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(x.b == 2); // expected-warning{{TRUE}}
+
+  clang_analyzer_eval(y.a == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y.b == 2); // expected-warning{{TRUE}}
+}
+
+void non_pod_val_syntax_2(void) {
+  S arr[2];
+
+  auto [x, y](arr);
+
+  clang_analyzer_eval(x.a == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(x.b == 2); // expected-warning{{TRUE}}
+
+  clang_analyzer_eval(y.a == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y.b == 2); // expected-warning{{TRUE}}
+}
+
+void non_pod_lref(void) {
+  S arr[2];
+
+  auto &[x, y] = arr;
+
+  clang_analyzer_eval(x.a == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(x.b == 2); // expected-warning{{TRUE}}
+
+  clang_analyzer_eval(y.a == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y.b == 2); // expected-warning{{TRUE}}
+}
+
+void non_pod_rref(void) {
+  S arr[2];
+
+  auto &&[x, y] = arr;
+
+  clang_analyzer_eval(x.a == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(x.b == 2); // expected-warning{{TRUE}}
+
+  clang_analyzer_eval(y.a == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y.b == 2); // expected-warning{{TRUE}}
+}
+
+struct SUD {
+  inline static int c = 0;
+
+  int a = 1;
+  int b = 2;
+
+  SUD() { ++c; };
+
+  SUD(const SUD ) {
+a = copy.a + 1;
+b = copy.b + 1;
+  }
+};
+
+void non_pod_user_defined_val(void) {
+  SUD arr[2];
+
+  auto [x, y] = arr;
+
+  clang_analyzer_eval(x.a == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(x.b == 3); // expected-warning{{TRUE}}
+
+  clang_analyzer_eval(y.a == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y.b == 3); // expected-warning{{TRUE}}
+}
+
+void non_pod_user_defined_val_syntax_2(void) {
+  SUD::c = 0;
+  SUD arr[2];
+
+  auto [x, y](arr);
+
+  clang_analyzer_eval(SUD::c == 2); // expected-warning{{TRUE}}
+
+  clang_analyzer_eval(x.a == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(x.b == 3); // expected-warning{{TRUE}}
+
+  clang_analyzer_eval(y.a == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y.b == 3); // expected-warning{{TRUE}}
+}
Index: clang/test/Analysis/ctor.mm
===
--- clang/test/Analysis/ctor.mm
+++ clang/test/Analysis/ctor.mm
@@ -439,16 +439,16 @@
 
 NonPOD b = a;
 
-clang_analyzer_eval(b.values[0].x == 1); // expected-warning{{UNKNOWN}}
-clang_analyzer_eval(b.values[1].x == 2); // expected-warning{{UNKNOWN}}
-clang_analyzer_eval(b.values[2].x == 3); // expected-warning{{UNKNOWN}}
+clang_analyzer_eval(b.values[0].x == 1); // expected-warning{{TRUE}}
+clang_analyzer_eval(b.values[1].x == 2); // expected-warning{{TRUE}}
+clang_analyzer_eval(b.values[2].x == 3); // expected-warning{{TRUE}}
 
 NonPOD c;
 c = b;
 
-clang_analyzer_eval(c.values[0].x == 1); // expected-warning{{UNKNOWN}}
-clang_analyzer_eval(c.values[1].x == 2); // expected-warning{{UNKNOWN}}
-clang_analyzer_eval(c.values[2].x == 3); // expected-warning{{UNKNOWN}}
+clang_analyzer_eval(c.values[0].x == 1); // expected-warning{{TRUE}}
+clang_analyzer_eval(c.values[1].x == 2); // expected-warning{{TRUE}}
+clang_analyzer_eval(c.values[2].x == 3); // expected-warning{{TRUE}}
   }
 
   struct NestedNonPOD {
@@ -502,16 +502,16 @@
 
 NonPODDefaulted b = a;
 
-clang_analyzer_eval(b.values[0].x == 1); // expected-warning{{UNKNOWN}}
-clang_analyzer_eval(b.values[1].x == 2); // expected-warning{{UNKNOWN}}
-

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

2022-07-26 Thread Roman Rusyaev via Phabricator via cfe-commits

rusyaev-roman added inline comments.



Comment at: clang/lib/Sema/Scope.cpp:152-154
+  // Consider the variable as NRVO candidate if the return slot is available
+  // for it in the current scope, or if it can be available in outer scopes.
+  NRVO = CanBePutInReturnSlot ? VD : nullptr;

ChuanqiXu wrote:
> rusyaev-roman wrote:
> > ChuanqiXu wrote:
> > > rusyaev-roman wrote:
> > > > ChuanqiXu wrote:
> > > > > rusyaev-roman wrote:
> > > > > > ChuanqiXu wrote:
> > > > > > > rusyaev-roman wrote:
> > > > > > > > ChuanqiXu wrote:
> > > > > > > > > What if NRVO contains a value already? It is possible due to 
> > > > > > > > > the value of NRVO could be set by its children.
> > > > > > > > Actually this is intention. If the parent has already NRVO 
> > > > > > > > candidate, then it should be invalidated (or not). Let's 
> > > > > > > > consider the following examples:
> > > > > > > > 
> > > > > > > > 
> > > > > > > > ```
> > > > > > > > X foo(bool b) {
> > > > > > > >X x;
> > > > > > > >X y;
> > > > > > > >if (b)
> > > > > > > >   return x;
> > > > > > > >else
> > > > > > > >   return y; // when we process this return statement, the 
> > > > > > > > parent has already NRVO and it will be invalidated (this is 
> > > > > > > > correct behavior)
> > > > > > > > }
> > > > > > > > ```
> > > > > > > > 
> > > > > > > > ```
> > > > > > > > X foo(bool b) {
> > > > > > > >X x;
> > > > > > > >if (b)
> > > > > > > >   return x;
> > > > > > > >
> > > > > > > >X y;
> > > > > > > >// when we process this return statement, the parent has 
> > > > > > > > already NRVO and it WON't be invalidated
> > > > > > > >//  (this is correct behavior), because a return slot will 
> > > > > > > > be available for it
> > > > > > > >return y;
> > > > > > > > }
> > > > > > > > ```
> > > > > > > > 
> > > > > > > > ```
> > > > > > > > X foo(bool b) {
> > > > > > > >X x;
> > > > > > > >if (b)
> > > > > > > >   return x;
> > > > > > > > 
> > > > > > > >// when we process this return statement, the parent has 
> > > > > > > > already NRVO and it WON't be invalidated (this is correct 
> > > > > > > > behavior)
> > > > > > > >return x;
> > > > > > > > }
> > > > > > > > ```
> > > > > > > > 
> > > > > > > > ```
> > > > > > > > X foo(bool b, X x) {
> > > > > > > >X y;
> > > > > > > >
> > > > > > > >if (b)
> > > > > > > >   return x;
> > > > > > > > 
> > > > > > > >// when we process this return statement, the parent 
> > > > > > > > contains nullptr (invalid candidate) and it will be invalidated 
> > > > > > > > (this is correct behavior)
> > > > > > > >return y;
> > > > > > > > }
> > > > > > > > ```
> > > > > > > > 
> > > > > > > > ```
> > > > > > > > X foo(bool b, X x) {
> > > > > > > >if (b)
> > > > > > > >   return x;
> > > > > > > > 
> > > > > > > >X y;
> > > > > > > >// when we process this return statement, the parent 
> > > > > > > > contains nullptr (invalid candidate) and it WON't be 
> > > > > > > > invalidated (this is correct behavior)
> > > > > > > >return y;
> > > > > > > > }
> > > > > > > > ```
> > > > > > > Oh, I see. Tricky. I don't find invalid cases now. But I 
> > > > > > > recommend to comment that the children would maintain the 
> > > > > > > `ReturnSlots` of their parents. (This is anti-intuition)
> > > > > > > 
> > > > > > > Have you tested any larger projects? Like libc++, libstdc++ or 
> > > > > > > something like folly. I feel we need to do such tests to avoid we 
> > > > > > > get anything wrong.
> > > > > > I've already added a comment at the beginning of 
> > > > > > `updateNRVOCandidate` function where this point is mentioned: 
> > > > > > ```
> > > > > > //  ... Therefore, we need to clear return slots for other
> > > > > > //  variables defined before the current return statement in 
> > > > > > the current
> > > > > > //  scope and in outer scopes.
> > > > > > ```
> > > > > > If it's not enough, please let me know.
> > > > > > 
> > > > > > 
> > > > > > > Have you tested any larger projects?
> > > > > > 
> > > > > > Yes, I've built the `clang` itself and `compiler-rt` project. Then 
> > > > > > I've checked them to run 'check-all' (on built clang and 
> > > > > > compiler-rt). Everything  works.
> > > > > Great! Clang should be large enough.
> > > > Thanks a lot for the careful review!
> > > > 
> > > > @ChuanqiXu  , could you land this patch please?
> > > > 
> > > > Many thanks to @Izaron for the original implementation.
> > > Sure. What's your prefer Name and Mail address?
> > Thanks!
> > 
> > Roman Rusyaev 
> Oh, I forgot you need edit the ReleaseNotes at clang/docs/ReleaseNotes.rst
I'm going to add a description in `C++ Language Changes in Clang` paragraph.

It will look like:
```
- Improved ``copy elision` optimization. It's possible to apply ``NRVO`` for an 
object if at the moment when
  any return statement of this object is executed, the ``return

[clang] 8a13326 - [analyzer] ArrayInitLoopExpr with array of non-POD type

2022-07-26 Thread via cfe-commits


Author: isuckatcs
Date: 2022-07-26T09:07:22+02:00
New Revision: 8a13326d184c2829a51d7c00f467ded06412f858

URL: 
https://github.com/llvm/llvm-project/commit/8a13326d184c2829a51d7c00f467ded06412f858
DIFF: 
https://github.com/llvm/llvm-project/commit/8a13326d184c2829a51d7c00f467ded06412f858.diff

LOG: [analyzer] ArrayInitLoopExpr with array of non-POD type

This patch introduces the evaluation of ArrayInitLoopExpr
in case of structured bindings and implicit copy/move
constructor. The idea is to call the copy constructor for
every element in the array. The parameter of the copy
constructor is also manually selected, as it is not a part
of the CFG.

Differential Revision: https://reviews.llvm.org/D129496

Added: 


Modified: 
clang/include/clang/Analysis/ConstructionContext.h
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
clang/lib/Analysis/CFG.cpp
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
clang/test/Analysis/array-init-loop.cpp
clang/test/Analysis/ctor.mm
clang/test/Analysis/uninit-structured-binding-array.cpp

Removed: 




diff  --git a/clang/include/clang/Analysis/ConstructionContext.h 
b/clang/include/clang/Analysis/ConstructionContext.h
index a437160e07789..7ff1a007f8eed 100644
--- a/clang/include/clang/Analysis/ConstructionContext.h
+++ b/clang/include/clang/Analysis/ConstructionContext.h
@@ -298,6 +298,11 @@ class ConstructionContext {
const ConstructionContextLayer *TopLayer);
 
   Kind getKind() const { return K; }
+
+  virtual const ArrayInitLoopExpr *getArrayInitLoop() const { return nullptr; }
+
+  // Only declared to silence -Wnon-virtual-dtor warnings.
+  virtual ~ConstructionContext() = default;
 };
 
 /// An abstract base class for local variable constructors.
@@ -314,6 +319,12 @@ class VariableConstructionContext : public 
ConstructionContext {
 public:
   const DeclStmt *getDeclStmt() const { return DS; }
 
+  const ArrayInitLoopExpr *getArrayInitLoop() const override {
+const auto *Var = cast(DS->getSingleDecl());
+
+return dyn_cast(Var->getInit());
+  }
+
   static bool classof(const ConstructionContext *CC) {
 return CC->getKind() >= VARIABLE_BEGIN &&
CC->getKind() <= VARIABLE_END;
@@ -381,6 +392,10 @@ class ConstructorInitializerConstructionContext : public 
ConstructionContext {
 public:
   const CXXCtorInitializer *getCXXCtorInitializer() const { return I; }
 
+  const ArrayInitLoopExpr *getArrayInitLoop() const override {
+return dyn_cast(I->getInit());
+  }
+
   static bool classof(const ConstructionContext *CC) {
 return CC->getKind() >= INITIALIZER_BEGIN &&
CC->getKind() <= INITIALIZER_END;

diff  --git 
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index 116a5970c341f..8773e171369fd 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -622,6 +622,11 @@ class ExprEngine {
   getIndexOfElementToConstruct(ProgramStateRef State, const CXXConstructExpr 
*E,
const LocationContext *LCtx);
 
+  /// Retreives the size of the array in the pending ArrayInitLoopExpr.
+  static Optional getPendingInitLoop(ProgramStateRef State,
+   const CXXConstructExpr *E,
+   const LocationContext *LCtx);
+
   /// By looking at a certain item that may be potentially part of an object's
   /// ConstructionContext, retrieve such object's location. A particular
   /// statement can be transparently passed as \p Item in most cases.
@@ -816,7 +821,9 @@ class ExprEngine {
 
   /// Checks whether our policies allow us to inline a non-POD type array
   /// construction.
-  bool shouldInlineArrayConstruction(const ArrayType *Type);
+  bool shouldInlineArrayConstruction(const ProgramStateRef State,
+ const CXXConstructExpr *CE,
+ const LocationContext *LCtx);
 
   /// Checks whether we construct an array of non-POD type, and decides if the
   /// constructor should be inkoved once again.
@@ -916,6 +923,16 @@ class ExprEngine {
   const CXXConstructExpr *E,
   const LocationContext *LCtx);
 
+  /// Sets the size of the array in a pending ArrayInitLoopExpr.
+  static ProgramStateRef setPendingInitLoop(ProgramStateRef State,
+const CXXConstructExpr *E,
+const LocationContext *LCtx,
+unsigned Idx);
+
+  static ProgramStateRef

[PATCH] D130551: [pseudo] Allow opaque nodes to represent terminals

2022-07-26 Thread Sam McCall via Phabricator via cfe-commits

sammccall created this revision.
sammccall added reviewers: hokein, usaxena95.
Herald added a project: All.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, alextsao1999.
Herald added a project: clang-tools-extra.

This allows incomplete code such as `namespace foo {` to be modeled as a
normal sequence with the missing } represented by an empty opaque node.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130551

Files:
  clang-tools-extra/pseudo/lib/GLR.cpp
  clang-tools-extra/pseudo/unittests/GLRTest.cpp


Index: clang-tools-extra/pseudo/unittests/GLRTest.cpp
===
--- clang-tools-extra/pseudo/unittests/GLRTest.cpp
+++ clang-tools-extra/pseudo/unittests/GLRTest.cpp
@@ -604,6 +604,28 @@
 "[  5, end) └─} := tok[5]\n");
 }
 
+TEST_F(GLRTest, RecoverTerminal) {
+  build(R"bnf(
+_ := stmt
+
+stmt := IDENTIFIER ; [recover=Skip]
+  )bnf");
+  TestLang.Table = LRTable::buildSLR(TestLang.G);
+  TestLang.RecoveryStrategies.try_emplace(
+  extensionID("Skip"),
+  [](Token::Index Start, const TokenStream &) { return Start + 1; });
+  clang::LangOptions LOptions;
+  TokenStream Tokens = cook(lex("foo", LOptions), LOptions);
+
+  const ForestNode  =
+  glrParse({Tokens, Arena, GSStack}, id("stmt"), TestLang);
+  EXPECT_EQ(Parsed.dumpRecursive(TestLang.G),
+"[  0, end) stmt := IDENTIFIER ; [recover=Skip]\n"
+"[  0,   1) ├─IDENTIFIER := tok[0]\n"
+"[  1, end) └─; := \n");
+}
+
+
 TEST_F(GLRTest, NoExplicitAccept) {
   build(R"bnf(
 _ := test
Index: clang-tools-extra/pseudo/lib/GLR.cpp
===
--- clang-tools-extra/pseudo/lib/GLR.cpp
+++ clang-tools-extra/pseudo/lib/GLR.cpp
@@ -182,9 +182,13 @@
   for (const PlaceholderRecovery *Option : BestOptions) {
 const ForestNode  =
 Params.Forest.createOpaque(Option->Symbol, RecoveryRange->Begin);
-const GSS::Node *NewHead = Params.GSStack.addNode(
-*Lang.Table.getGoToState(Option->RecoveryNode->State, Option->Symbol),
-, {Option->RecoveryNode});
+LRTable::StateID OldState = Option->RecoveryNode->State;
+LRTable::StateID NewState =
+isToken(Option->Symbol)
+? *Lang.Table.getShiftState(OldState, Option->Symbol)
+: *Lang.Table.getGoToState(OldState, Option->Symbol);
+const GSS::Node *NewHead =
+Params.GSStack.addNode(NewState, , {Option->RecoveryNode});
 NewHeads.push_back(NewHead);
   }
   TokenIndex = RecoveryRange->End;


Index: clang-tools-extra/pseudo/unittests/GLRTest.cpp
===
--- clang-tools-extra/pseudo/unittests/GLRTest.cpp
+++ clang-tools-extra/pseudo/unittests/GLRTest.cpp
@@ -604,6 +604,28 @@
 "[  5, end) ââ} := tok[5]\n");
 }
 
+TEST_F(GLRTest, RecoverTerminal) {
+  build(R"bnf(
+_ := stmt
+
+stmt := IDENTIFIER ; [recover=Skip]
+  )bnf");
+  TestLang.Table = LRTable::buildSLR(TestLang.G);
+  TestLang.RecoveryStrategies.try_emplace(
+  extensionID("Skip"),
+  [](Token::Index Start, const TokenStream &) { return Start + 1; });
+  clang::LangOptions LOptions;
+  TokenStream Tokens = cook(lex("foo", LOptions), LOptions);
+
+  const ForestNode  =
+  glrParse({Tokens, Arena, GSStack}, id("stmt"), TestLang);
+  EXPECT_EQ(Parsed.dumpRecursive(TestLang.G),
+"[  0, end) stmt := IDENTIFIER ; [recover=Skip]\n"
+"[  0,   1) ââIDENTIFIER := tok[0]\n"
+"[  1, end) ââ; := \n");
+}
+
+
 TEST_F(GLRTest, NoExplicitAccept) {
   build(R"bnf(
 _ := test
Index: clang-tools-extra/pseudo/lib/GLR.cpp
===
--- clang-tools-extra/pseudo/lib/GLR.cpp
+++ clang-tools-extra/pseudo/lib/GLR.cpp
@@ -182,9 +182,13 @@
   for (const PlaceholderRecovery *Option : BestOptions) {
 const ForestNode  =
 Params.Forest.createOpaque(Option->Symbol, RecoveryRange->Begin);
-const GSS::Node *NewHead = Params.GSStack.addNode(
-*Lang.Table.getGoToState(Option->RecoveryNode->State, Option->Symbol),
-, {Option->RecoveryNode});
+LRTable::StateID OldState = Option->RecoveryNode->State;
+LRTable::StateID NewState =
+isToken(Option->Symbol)
+? *Lang.Table.getShiftState(OldState, Option->Symbol)
+: *Lang.Table.getGoToState(OldState, Option->Symbol);
+const GSS::Node *NewHead =
+Params.GSStack.addNode(NewState, , {Option->RecoveryNode});
 NewHeads.push_back(NewHead);
   }
   TokenIndex = RecoveryRange->End;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130550: [pseudo] Start rules are `_ := start-symbol EOF`, improve recovery.

2022-07-26 Thread Sam McCall via Phabricator via cfe-commits

sammccall created this revision.
sammccall added a reviewer: hokein.
Herald added a project: All.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, alextsao1999.
Herald added a project: clang-tools-extra.

Previously we were calling glrRecover() ad-hoc at the end of input.
Two main problems with this:

- glrRecover() on two separate code paths is inelegant
- We may have to recover several times in succession (e.g. to exit from nested 
scopes), so we need a loop at end-of-file

Having an actual shift action for an EOF terminal allows us to handle
both concerns in the main shift/recover/reduce loop.

This revealed a recovery design bug where recovery could enter a loop by
repeatedly choosing the same parent to identically recover from.
Addressed this by allowing each node to be used as a recovery base once.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130550

Files:
  clang-tools-extra/pseudo/include/clang-pseudo/GLR.h
  clang-tools-extra/pseudo/lib/Forest.cpp
  clang-tools-extra/pseudo/lib/GLR.cpp
  clang-tools-extra/pseudo/lib/cxx/cxx.bnf
  clang-tools-extra/pseudo/lib/grammar/LRGraph.cpp
  clang-tools-extra/pseudo/test/lr-build-basic.test
  clang-tools-extra/pseudo/test/lr-build-conflicts.test
  clang-tools-extra/pseudo/unittests/ForestTest.cpp
  clang-tools-extra/pseudo/unittests/GLRTest.cpp

Index: clang-tools-extra/pseudo/unittests/GLRTest.cpp
===
--- clang-tools-extra/pseudo/unittests/GLRTest.cpp
+++ clang-tools-extra/pseudo/unittests/GLRTest.cpp
@@ -509,7 +509,7 @@
   // item `expr := â¢ IDENTIFIER`, and both have different goto states on the
   // nonterminal `expr`.
   build(R"bnf(
-_ := test
+_ := test EOF
 
 test := { expr
 test := { IDENTIFIER
@@ -548,7 +548,7 @@
   // foo should be reduced first, so that in step 2 we have completed reduces
   // for test, and form an ambiguous forest node.
   build(R"bnf(
-_ := test
+_ := test EOF
 
 test := IDENTIFIER
 test := foo
@@ -575,7 +575,7 @@
   //  - multiple possible recovery rules
   //  - recovery from outer scopes is rejected
   build(R"bnf(
-_ := block
+_ := block EOF
 
 block := { block [recover=Braces] }
 block := { numbers [recover=Braces] }
@@ -604,9 +604,42 @@
 "[  5, end) ââ} := tok[5]\n");
 }
 
+TEST_F(GLRTest, RepeatedRecovery) {
+  // We require multiple steps of recovery at eof and then a reduction in order
+  // to successfully parse.
+  build(R"bnf(
+_ := function EOF
+# FIXME: this forces EOF to be in follow(signature).
+# Remove it once we use unconstrained reduction for recovery.
+_ := signature EOF
+
+function := signature body [recover=Skip]
+signature := IDENTIFIER params [recover=Skip]
+params := ( )
+body := { }
+  )bnf");
+  TestLang.Table = LRTable::buildSLR(TestLang.G);
+  llvm::errs() << TestLang.Table.dumpForTests(TestLang.G);
+  TestLang.RecoveryStrategies.try_emplace(
+  extensionID("Skip"),
+  [](Token::Index Start, const TokenStream &) { return Start; });
+  clang::LangOptions LOptions;
+  TokenStream Tokens = cook(lex("main", LOptions), LOptions);
+
+  const ForestNode  =
+  glrParse({Tokens, Arena, GSStack}, id("function"), TestLang);
+  EXPECT_EQ(Parsed.dumpRecursive(TestLang.G),
+"[  0, end) function := signature body [recover=Skip]\n"
+"[  0,   1) ââsignature := IDENTIFIER params [recover=Skip]\n"
+"[  0,   1) â ââIDENTIFIER := tok[0]\n"
+"[  1,   1) â ââparams := \n"
+"[  1, end) ââbody := \n");
+}
+
+
 TEST_F(GLRTest, NoExplicitAccept) {
   build(R"bnf(
-_ := test
+_ := test EOF
 
 test := IDENTIFIER test
 test := IDENTIFIER
@@ -629,7 +662,7 @@
 
 TEST_F(GLRTest, GuardExtension) {
   build(R"bnf(
-_ := start
+_ := start EOF
 
 start := IDENTIFIER [guard]
   )bnf");
Index: clang-tools-extra/pseudo/unittests/ForestTest.cpp
===
--- clang-tools-extra/pseudo/unittests/ForestTest.cpp
+++ clang-tools-extra/pseudo/unittests/ForestTest.cpp
@@ -54,7 +54,7 @@
 
 TEST_F(ForestTest, DumpBasic) {
   build(R"cpp(
-_ := add-expression
+_ := add-expression EOF
 add-expression := id-expression + id-expression
 id-expression := IDENTIFIER
   )cpp");
@@ -64,7 +64,7 @@
   cook(lex("a + b", clang::LangOptions()), clang::LangOptions());
 
   auto T = Arena.createTerminals(TS);
-  ASSERT_EQ(T.size(), 3u);
+  ASSERT_EQ(T.size(), 4u);
   const auto *Left = (
   symbol("id-expression"), ruleFor("id-expression"), {()});
   const auto *Right = (symbol("id-expression"),
@@ -89,9 +89,9 @@
 
 TEST_F(ForestTest, DumpAmbiguousAndRefs) {
   build(R"cpp(
-_ := type
-type := class-type # rule 3
-type := enum-type # rule 4
+_ := type EOF
+type := class-type # rule 4
+type := enum-type # rule 5

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

2022-07-26 Thread Chuanqi Xu via Phabricator via cfe-commits

ChuanqiXu added inline comments.



Comment at: clang/lib/Sema/Scope.cpp:152-154
+  // Consider the variable as NRVO candidate if the return slot is available
+  // for it in the current scope, or if it can be available in outer scopes.
+  NRVO = CanBePutInReturnSlot ? VD : nullptr;

rusyaev-roman wrote:
> ChuanqiXu wrote:
> > rusyaev-roman wrote:
> > > ChuanqiXu wrote:
> > > > rusyaev-roman wrote:
> > > > > ChuanqiXu wrote:
> > > > > > rusyaev-roman wrote:
> > > > > > > ChuanqiXu wrote:
> > > > > > > > What if NRVO contains a value already? It is possible due to 
> > > > > > > > the value of NRVO could be set by its children.
> > > > > > > Actually this is intention. If the parent has already NRVO 
> > > > > > > candidate, then it should be invalidated (or not). Let's consider 
> > > > > > > the following examples:
> > > > > > > 
> > > > > > > 
> > > > > > > ```
> > > > > > > X foo(bool b) {
> > > > > > >X x;
> > > > > > >X y;
> > > > > > >if (b)
> > > > > > >   return x;
> > > > > > >else
> > > > > > >   return y; // when we process this return statement, the 
> > > > > > > parent has already NRVO and it will be invalidated (this is 
> > > > > > > correct behavior)
> > > > > > > }
> > > > > > > ```
> > > > > > > 
> > > > > > > ```
> > > > > > > X foo(bool b) {
> > > > > > >X x;
> > > > > > >if (b)
> > > > > > >   return x;
> > > > > > >
> > > > > > >X y;
> > > > > > >// when we process this return statement, the parent has 
> > > > > > > already NRVO and it WON't be invalidated
> > > > > > >//  (this is correct behavior), because a return slot will be 
> > > > > > > available for it
> > > > > > >return y;
> > > > > > > }
> > > > > > > ```
> > > > > > > 
> > > > > > > ```
> > > > > > > X foo(bool b) {
> > > > > > >X x;
> > > > > > >if (b)
> > > > > > >   return x;
> > > > > > > 
> > > > > > >// when we process this return statement, the parent has 
> > > > > > > already NRVO and it WON't be invalidated (this is correct 
> > > > > > > behavior)
> > > > > > >return x;
> > > > > > > }
> > > > > > > ```
> > > > > > > 
> > > > > > > ```
> > > > > > > X foo(bool b, X x) {
> > > > > > >X y;
> > > > > > >
> > > > > > >if (b)
> > > > > > >   return x;
> > > > > > > 
> > > > > > >// when we process this return statement, the parent contains 
> > > > > > > nullptr (invalid candidate) and it will be invalidated (this is 
> > > > > > > correct behavior)
> > > > > > >return y;
> > > > > > > }
> > > > > > > ```
> > > > > > > 
> > > > > > > ```
> > > > > > > X foo(bool b, X x) {
> > > > > > >if (b)
> > > > > > >   return x;
> > > > > > > 
> > > > > > >X y;
> > > > > > >// when we process this return statement, the parent contains 
> > > > > > > nullptr (invalid candidate) and it WON't be invalidated (this is 
> > > > > > > correct behavior)
> > > > > > >return y;
> > > > > > > }
> > > > > > > ```
> > > > > > Oh, I see. Tricky. I don't find invalid cases now. But I recommend 
> > > > > > to comment that the children would maintain the `ReturnSlots` of 
> > > > > > their parents. (This is anti-intuition)
> > > > > > 
> > > > > > Have you tested any larger projects? Like libc++, libstdc++ or 
> > > > > > something like folly. I feel we need to do such tests to avoid we 
> > > > > > get anything wrong.
> > > > > I've already added a comment at the beginning of 
> > > > > `updateNRVOCandidate` function where this point is mentioned: 
> > > > > ```
> > > > > //  ... Therefore, we need to clear return slots for other
> > > > > //  variables defined before the current return statement in the 
> > > > > current
> > > > > //  scope and in outer scopes.
> > > > > ```
> > > > > If it's not enough, please let me know.
> > > > > 
> > > > > 
> > > > > > Have you tested any larger projects?
> > > > > 
> > > > > Yes, I've built the `clang` itself and `compiler-rt` project. Then 
> > > > > I've checked them to run 'check-all' (on built clang and 
> > > > > compiler-rt). Everything  works.
> > > > Great! Clang should be large enough.
> > > Thanks a lot for the careful review!
> > > 
> > > @ChuanqiXu  , could you land this patch please?
> > > 
> > > Many thanks to @Izaron for the original implementation.
> > Sure. What's your prefer Name and Mail address?
> Thanks!
> 
> Roman Rusyaev 
Oh, I forgot you need edit the ReleaseNotes at clang/docs/ReleaseNotes.rst


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119792/new/

https://reviews.llvm.org/D119792

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

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

2022-07-26 Thread Roman Rusyaev via Phabricator via cfe-commits

rusyaev-roman added inline comments.



Comment at: clang/lib/Sema/Scope.cpp:152-154
+  // Consider the variable as NRVO candidate if the return slot is available
+  // for it in the current scope, or if it can be available in outer scopes.
+  NRVO = CanBePutInReturnSlot ? VD : nullptr;

ChuanqiXu wrote:
> rusyaev-roman wrote:
> > ChuanqiXu wrote:
> > > rusyaev-roman wrote:
> > > > ChuanqiXu wrote:
> > > > > rusyaev-roman wrote:
> > > > > > ChuanqiXu wrote:
> > > > > > > What if NRVO contains a value already? It is possible due to the 
> > > > > > > value of NRVO could be set by its children.
> > > > > > Actually this is intention. If the parent has already NRVO 
> > > > > > candidate, then it should be invalidated (or not). Let's consider 
> > > > > > the following examples:
> > > > > > 
> > > > > > 
> > > > > > ```
> > > > > > X foo(bool b) {
> > > > > >X x;
> > > > > >X y;
> > > > > >if (b)
> > > > > >   return x;
> > > > > >else
> > > > > >   return y; // when we process this return statement, the 
> > > > > > parent has already NRVO and it will be invalidated (this is correct 
> > > > > > behavior)
> > > > > > }
> > > > > > ```
> > > > > > 
> > > > > > ```
> > > > > > X foo(bool b) {
> > > > > >X x;
> > > > > >if (b)
> > > > > >   return x;
> > > > > >
> > > > > >X y;
> > > > > >// when we process this return statement, the parent has already 
> > > > > > NRVO and it WON't be invalidated
> > > > > >//  (this is correct behavior), because a return slot will be 
> > > > > > available for it
> > > > > >return y;
> > > > > > }
> > > > > > ```
> > > > > > 
> > > > > > ```
> > > > > > X foo(bool b) {
> > > > > >X x;
> > > > > >if (b)
> > > > > >   return x;
> > > > > > 
> > > > > >// when we process this return statement, the parent has already 
> > > > > > NRVO and it WON't be invalidated (this is correct behavior)
> > > > > >return x;
> > > > > > }
> > > > > > ```
> > > > > > 
> > > > > > ```
> > > > > > X foo(bool b, X x) {
> > > > > >X y;
> > > > > >
> > > > > >if (b)
> > > > > >   return x;
> > > > > > 
> > > > > >// when we process this return statement, the parent contains 
> > > > > > nullptr (invalid candidate) and it will be invalidated (this is 
> > > > > > correct behavior)
> > > > > >return y;
> > > > > > }
> > > > > > ```
> > > > > > 
> > > > > > ```
> > > > > > X foo(bool b, X x) {
> > > > > >if (b)
> > > > > >   return x;
> > > > > > 
> > > > > >X y;
> > > > > >// when we process this return statement, the parent contains 
> > > > > > nullptr (invalid candidate) and it WON't be invalidated (this is 
> > > > > > correct behavior)
> > > > > >return y;
> > > > > > }
> > > > > > ```
> > > > > Oh, I see. Tricky. I don't find invalid cases now. But I recommend to 
> > > > > comment that the children would maintain the `ReturnSlots` of their 
> > > > > parents. (This is anti-intuition)
> > > > > 
> > > > > Have you tested any larger projects? Like libc++, libstdc++ or 
> > > > > something like folly. I feel we need to do such tests to avoid we get 
> > > > > anything wrong.
> > > > I've already added a comment at the beginning of `updateNRVOCandidate` 
> > > > function where this point is mentioned: 
> > > > ```
> > > > //  ... Therefore, we need to clear return slots for other
> > > > //  variables defined before the current return statement in the 
> > > > current
> > > > //  scope and in outer scopes.
> > > > ```
> > > > If it's not enough, please let me know.
> > > > 
> > > > 
> > > > > Have you tested any larger projects?
> > > > 
> > > > Yes, I've built the `clang` itself and `compiler-rt` project. Then I've 
> > > > checked them to run 'check-all' (on built clang and compiler-rt). 
> > > > Everything  works.
> > > Great! Clang should be large enough.
> > Thanks a lot for the careful review!
> > 
> > @ChuanqiXu  , could you land this patch please?
> > 
> > Many thanks to @Izaron for the original implementation.
> Sure. What's your prefer Name and Mail address?
Thanks!

Roman Rusyaev 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119792/new/

https://reviews.llvm.org/D119792

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

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

2022-07-26 Thread Chuanqi Xu via Phabricator via cfe-commits

ChuanqiXu added inline comments.



Comment at: clang/lib/Sema/Scope.cpp:152-154
+  // Consider the variable as NRVO candidate if the return slot is available
+  // for it in the current scope, or if it can be available in outer scopes.
+  NRVO = CanBePutInReturnSlot ? VD : nullptr;

rusyaev-roman wrote:
> ChuanqiXu wrote:
> > rusyaev-roman wrote:
> > > ChuanqiXu wrote:
> > > > rusyaev-roman wrote:
> > > > > ChuanqiXu wrote:
> > > > > > What if NRVO contains a value already? It is possible due to the 
> > > > > > value of NRVO could be set by its children.
> > > > > Actually this is intention. If the parent has already NRVO candidate, 
> > > > > then it should be invalidated (or not). Let's consider the following 
> > > > > examples:
> > > > > 
> > > > > 
> > > > > ```
> > > > > X foo(bool b) {
> > > > >X x;
> > > > >X y;
> > > > >if (b)
> > > > >   return x;
> > > > >else
> > > > >   return y; // when we process this return statement, the parent 
> > > > > has already NRVO and it will be invalidated (this is correct behavior)
> > > > > }
> > > > > ```
> > > > > 
> > > > > ```
> > > > > X foo(bool b) {
> > > > >X x;
> > > > >if (b)
> > > > >   return x;
> > > > >
> > > > >X y;
> > > > >// when we process this return statement, the parent has already 
> > > > > NRVO and it WON't be invalidated
> > > > >//  (this is correct behavior), because a return slot will be 
> > > > > available for it
> > > > >return y;
> > > > > }
> > > > > ```
> > > > > 
> > > > > ```
> > > > > X foo(bool b) {
> > > > >X x;
> > > > >if (b)
> > > > >   return x;
> > > > > 
> > > > >// when we process this return statement, the parent has already 
> > > > > NRVO and it WON't be invalidated (this is correct behavior)
> > > > >return x;
> > > > > }
> > > > > ```
> > > > > 
> > > > > ```
> > > > > X foo(bool b, X x) {
> > > > >X y;
> > > > >
> > > > >if (b)
> > > > >   return x;
> > > > > 
> > > > >// when we process this return statement, the parent contains 
> > > > > nullptr (invalid candidate) and it will be invalidated (this is 
> > > > > correct behavior)
> > > > >return y;
> > > > > }
> > > > > ```
> > > > > 
> > > > > ```
> > > > > X foo(bool b, X x) {
> > > > >if (b)
> > > > >   return x;
> > > > > 
> > > > >X y;
> > > > >// when we process this return statement, the parent contains 
> > > > > nullptr (invalid candidate) and it WON't be invalidated (this is 
> > > > > correct behavior)
> > > > >return y;
> > > > > }
> > > > > ```
> > > > Oh, I see. Tricky. I don't find invalid cases now. But I recommend to 
> > > > comment that the children would maintain the `ReturnSlots` of their 
> > > > parents. (This is anti-intuition)
> > > > 
> > > > Have you tested any larger projects? Like libc++, libstdc++ or 
> > > > something like folly. I feel we need to do such tests to avoid we get 
> > > > anything wrong.
> > > I've already added a comment at the beginning of `updateNRVOCandidate` 
> > > function where this point is mentioned: 
> > > ```
> > > //  ... Therefore, we need to clear return slots for other
> > > //  variables defined before the current return statement in the 
> > > current
> > > //  scope and in outer scopes.
> > > ```
> > > If it's not enough, please let me know.
> > > 
> > > 
> > > > Have you tested any larger projects?
> > > 
> > > Yes, I've built the `clang` itself and `compiler-rt` project. Then I've 
> > > checked them to run 'check-all' (on built clang and compiler-rt). 
> > > Everything  works.
> > Great! Clang should be large enough.
> Thanks a lot for the careful review!
> 
> @ChuanqiXu  , could you land this patch please?
> 
> Many thanks to @Izaron for the original implementation.
Sure. What's your prefer Name and Mail address?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119792/new/

https://reviews.llvm.org/D119792

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

[PATCH] D130522: [clang][dataflow] Fix SAT solver crashes on `X ^ X` and `X v X`

2022-07-26 Thread Stanislav Gatev via Phabricator via cfe-commits

sgatev accepted this revision.
sgatev added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp:273
+
+// Visit a sub-value of `Val` (pick any, they are identical).
+  } else {

Let's visit `C->getLeftSubValue()` here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130522/new/

https://reviews.llvm.org/D130522

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

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

2022-07-26 Thread Roman Rusyaev via Phabricator via cfe-commits

rusyaev-roman added inline comments.



Comment at: clang/lib/Sema/Scope.cpp:152-154
+  // Consider the variable as NRVO candidate if the return slot is available
+  // for it in the current scope, or if it can be available in outer scopes.
+  NRVO = CanBePutInReturnSlot ? VD : nullptr;

ChuanqiXu wrote:
> rusyaev-roman wrote:
> > ChuanqiXu wrote:
> > > rusyaev-roman wrote:
> > > > ChuanqiXu wrote:
> > > > > What if NRVO contains a value already? It is possible due to the 
> > > > > value of NRVO could be set by its children.
> > > > Actually this is intention. If the parent has already NRVO candidate, 
> > > > then it should be invalidated (or not). Let's consider the following 
> > > > examples:
> > > > 
> > > > 
> > > > ```
> > > > X foo(bool b) {
> > > >X x;
> > > >X y;
> > > >if (b)
> > > >   return x;
> > > >else
> > > >   return y; // when we process this return statement, the parent 
> > > > has already NRVO and it will be invalidated (this is correct behavior)
> > > > }
> > > > ```
> > > > 
> > > > ```
> > > > X foo(bool b) {
> > > >X x;
> > > >if (b)
> > > >   return x;
> > > >
> > > >X y;
> > > >// when we process this return statement, the parent has already 
> > > > NRVO and it WON't be invalidated
> > > >//  (this is correct behavior), because a return slot will be 
> > > > available for it
> > > >return y;
> > > > }
> > > > ```
> > > > 
> > > > ```
> > > > X foo(bool b) {
> > > >X x;
> > > >if (b)
> > > >   return x;
> > > > 
> > > >// when we process this return statement, the parent has already 
> > > > NRVO and it WON't be invalidated (this is correct behavior)
> > > >return x;
> > > > }
> > > > ```
> > > > 
> > > > ```
> > > > X foo(bool b, X x) {
> > > >X y;
> > > >
> > > >if (b)
> > > >   return x;
> > > > 
> > > >// when we process this return statement, the parent contains 
> > > > nullptr (invalid candidate) and it will be invalidated (this is correct 
> > > > behavior)
> > > >return y;
> > > > }
> > > > ```
> > > > 
> > > > ```
> > > > X foo(bool b, X x) {
> > > >if (b)
> > > >   return x;
> > > > 
> > > >X y;
> > > >// when we process this return statement, the parent contains 
> > > > nullptr (invalid candidate) and it WON't be invalidated (this is 
> > > > correct behavior)
> > > >return y;
> > > > }
> > > > ```
> > > Oh, I see. Tricky. I don't find invalid cases now. But I recommend to 
> > > comment that the children would maintain the `ReturnSlots` of their 
> > > parents. (This is anti-intuition)
> > > 
> > > Have you tested any larger projects? Like libc++, libstdc++ or something 
> > > like folly. I feel we need to do such tests to avoid we get anything 
> > > wrong.
> > I've already added a comment at the beginning of `updateNRVOCandidate` 
> > function where this point is mentioned: 
> > ```
> > //  ... Therefore, we need to clear return slots for other
> > //  variables defined before the current return statement in the current
> > //  scope and in outer scopes.
> > ```
> > If it's not enough, please let me know.
> > 
> > 
> > > Have you tested any larger projects?
> > 
> > Yes, I've built the `clang` itself and `compiler-rt` project. Then I've 
> > checked them to run 'check-all' (on built clang and compiler-rt). 
> > Everything  works.
> Great! Clang should be large enough.
Thanks a lot for the careful review!

@ChuanqiXu  , could you land this patch please?

Many thanks to @Izaron for the original implementation.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119792/new/

https://reviews.llvm.org/D119792

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

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

2022-07-26 Thread Chuanqi Xu via Phabricator via cfe-commits

ChuanqiXu accepted this revision.
ChuanqiXu added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/Sema/Scope.cpp:152-154
+  // Consider the variable as NRVO candidate if the return slot is available
+  // for it in the current scope, or if it can be available in outer scopes.
+  NRVO = CanBePutInReturnSlot ? VD : nullptr;

rusyaev-roman wrote:
> ChuanqiXu wrote:
> > rusyaev-roman wrote:
> > > ChuanqiXu wrote:
> > > > What if NRVO contains a value already? It is possible due to the value 
> > > > of NRVO could be set by its children.
> > > Actually this is intention. If the parent has already NRVO candidate, 
> > > then it should be invalidated (or not). Let's consider the following 
> > > examples:
> > > 
> > > 
> > > ```
> > > X foo(bool b) {
> > >X x;
> > >X y;
> > >if (b)
> > >   return x;
> > >else
> > >   return y; // when we process this return statement, the parent has 
> > > already NRVO and it will be invalidated (this is correct behavior)
> > > }
> > > ```
> > > 
> > > ```
> > > X foo(bool b) {
> > >X x;
> > >if (b)
> > >   return x;
> > >
> > >X y;
> > >// when we process this return statement, the parent has already NRVO 
> > > and it WON't be invalidated
> > >//  (this is correct behavior), because a return slot will be 
> > > available for it
> > >return y;
> > > }
> > > ```
> > > 
> > > ```
> > > X foo(bool b) {
> > >X x;
> > >if (b)
> > >   return x;
> > > 
> > >// when we process this return statement, the parent has already NRVO 
> > > and it WON't be invalidated (this is correct behavior)
> > >return x;
> > > }
> > > ```
> > > 
> > > ```
> > > X foo(bool b, X x) {
> > >X y;
> > >
> > >if (b)
> > >   return x;
> > > 
> > >// when we process this return statement, the parent contains nullptr 
> > > (invalid candidate) and it will be invalidated (this is correct behavior)
> > >return y;
> > > }
> > > ```
> > > 
> > > ```
> > > X foo(bool b, X x) {
> > >if (b)
> > >   return x;
> > > 
> > >X y;
> > >// when we process this return statement, the parent contains nullptr 
> > > (invalid candidate) and it WON't be invalidated (this is correct behavior)
> > >return y;
> > > }
> > > ```
> > Oh, I see. Tricky. I don't find invalid cases now. But I recommend to 
> > comment that the children would maintain the `ReturnSlots` of their 
> > parents. (This is anti-intuition)
> > 
> > Have you tested any larger projects? Like libc++, libstdc++ or something 
> > like folly. I feel we need to do such tests to avoid we get anything wrong.
> I've already added a comment at the beginning of `updateNRVOCandidate` 
> function where this point is mentioned: 
> ```
> //  ... Therefore, we need to clear return slots for other
> //  variables defined before the current return statement in the current
> //  scope and in outer scopes.
> ```
> If it's not enough, please let me know.
> 
> 
> > Have you tested any larger projects?
> 
> Yes, I've built the `clang` itself and `compiler-rt` project. Then I've 
> checked them to run 'check-all' (on built clang and compiler-rt). Everything  
> works.
Great! Clang should be large enough.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119792/new/

https://reviews.llvm.org/D119792

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

[PATCH] D130411: [clang-format] Fix a hang when formatting C# $@ string literals

2022-07-26 Thread Owen Pan via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0ffb3dd33ee1: [clang-format] Fix a hang when formatting C# 
$@ string literals (authored by owenpan).

Changed prior to commit:
  https://reviews.llvm.org/D130411?vs=447041=447575#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130411/new/

https://reviews.llvm.org/D130411

Files:
  clang/lib/Format/FormatTokenLexer.cpp
  clang/unittests/Format/FormatTestCSharp.cpp

Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -574,6 +574,7 @@
   verifyFormat(R"(string str = @;)", Style);
   verifyFormat(R"(string str = @"""Hello world""";)", Style);
   verifyFormat(R"(string str = $@"""Hello {friend}""";)", Style);
+  verifyFormat(R"(return $@"Foo ""/foo?f={Request.Query["f"]}""";)", Style);
 }
 
 TEST_F(FormatTestCSharp, CSharpQuotesInInterpolatedStrings) {
Index: clang/lib/Format/FormatTokenLexer.cpp
===
--- clang/lib/Format/FormatTokenLexer.cpp
+++ clang/lib/Format/FormatTokenLexer.cpp
@@ -239,55 +239,6 @@
   if (Tokens.size() < 2)
 return false;
 
-  // Interpolated strings could contain { } with " characters inside.
-  // $"{x ?? "null"}"
-  // should not be split into $"{x ?? ", null, "}" but should treated as a
-  // single string-literal.
-  //
-  // We opt not to try and format expressions inside {} within a C#
-  // interpolated string. Formatting expressions within an interpolated string
-  // would require similar work as that done for JavaScript template strings
-  // in `handleTemplateStrings()`.
-  auto  = *(Tokens.end() - 2);
-  if (CSharpInterpolatedString->getType() == TT_CSharpStringLiteral &&
-  (CSharpInterpolatedString->TokenText.startswith(R"($")") ||
-   CSharpInterpolatedString->TokenText.startswith(R"($@")"))) {
-int UnmatchedOpeningBraceCount = 0;
-
-auto TokenTextSize = CSharpInterpolatedString->TokenText.size();
-for (size_t Index = 0; Index < TokenTextSize; ++Index) {
-  char C = CSharpInterpolatedString->TokenText[Index];
-  if (C == '{') {
-// "{{"  inside an interpolated string is an escaped '{' so skip it.
-if (Index + 1 < TokenTextSize &&
-CSharpInterpolatedString->TokenText[Index + 1] == '{') {
-  ++Index;
-  continue;
-}
-++UnmatchedOpeningBraceCount;
-  } else if (C == '}') {
-// "}}"  inside an interpolated string is an escaped '}' so skip it.
-if (Index + 1 < TokenTextSize &&
-CSharpInterpolatedString->TokenText[Index + 1] == '}') {
-  ++Index;
-  continue;
-}
---UnmatchedOpeningBraceCount;
-  }
-}
-
-if (UnmatchedOpeningBraceCount > 0) {
-  auto  = *(Tokens.end() - 1);
-  CSharpInterpolatedString->TokenText =
-  StringRef(CSharpInterpolatedString->TokenText.begin(),
-NextToken->TokenText.end() -
-CSharpInterpolatedString->TokenText.begin());
-  CSharpInterpolatedString->ColumnWidth += NextToken->ColumnWidth;
-  Tokens.erase(Tokens.end() - 1);
-  return true;
-}
-  }
-
   // Look for @"aa" or $"aa".
   auto  = *(Tokens.end() - 1);
   if (!String->is(tok::string_literal))
@@ -571,45 +522,105 @@
   resetLexer(SourceMgr.getFileOffset(Lex->getSourceLocation(Offset)));
 }
 
+static auto lexCSharpString(const char *Begin, const char *End, bool Verbatim,
+bool Interpolated) {
+  auto Repeated = [, End]() {
+return Begin + 1 < End && Begin[1] == Begin[0];
+  };
+
+  // Look for a terminating '"' in the current file buffer.
+  // Make no effort to format code within an interpolated or verbatim string.
+  //
+  // Interpolated strings could contain { } with " characters inside.
+  // $"{x ?? "null"}"
+  // should not be split into $"{x ?? ", null, "}" but should be treated as a
+  // single string-literal.
+  //
+  // We opt not to try and format expressions inside {} within a C#
+  // interpolated string. Formatting expressions within an interpolated string
+  // would require similar work as that done for JavaScript template strings
+  // in `handleTemplateStrings()`.
+  for (int UnmatchedOpeningBraceCount = 0; Begin < End; ++Begin) {
+switch (*Begin) {
+case '\\':
+  if (!Verbatim)
+++Begin;
+  break;
+case '{':
+  if (Interpolated) {
+// {{ inside an interpolated string is escaped, so skip it.
+if (Repeated())
+  ++Begin;
+else
+  ++UnmatchedOpeningBraceCount;
+  }
+  break;
+case '}':
+  if (Interpolated) {
+// }} inside an interpolated string is escaped, so skip it.
+if

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

2022-07-26 Thread Roman Rusyaev via Phabricator via cfe-commits

rusyaev-roman added inline comments.



Comment at: clang/lib/Sema/Scope.cpp:152-154
+  // Consider the variable as NRVO candidate if the return slot is available
+  // for it in the current scope, or if it can be available in outer scopes.
+  NRVO = CanBePutInReturnSlot ? VD : nullptr;

ChuanqiXu wrote:
> rusyaev-roman wrote:
> > ChuanqiXu wrote:
> > > What if NRVO contains a value already? It is possible due to the value of 
> > > NRVO could be set by its children.
> > Actually this is intention. If the parent has already NRVO candidate, then 
> > it should be invalidated (or not). Let's consider the following examples:
> > 
> > 
> > ```
> > X foo(bool b) {
> >X x;
> >X y;
> >if (b)
> >   return x;
> >else
> >   return y; // when we process this return statement, the parent has 
> > already NRVO and it will be invalidated (this is correct behavior)
> > }
> > ```
> > 
> > ```
> > X foo(bool b) {
> >X x;
> >if (b)
> >   return x;
> >
> >X y;
> >// when we process this return statement, the parent has already NRVO 
> > and it WON't be invalidated
> >//  (this is correct behavior), because a return slot will be available 
> > for it
> >return y;
> > }
> > ```
> > 
> > ```
> > X foo(bool b) {
> >X x;
> >if (b)
> >   return x;
> > 
> >// when we process this return statement, the parent has already NRVO 
> > and it WON't be invalidated (this is correct behavior)
> >return x;
> > }
> > ```
> > 
> > ```
> > X foo(bool b, X x) {
> >X y;
> >
> >if (b)
> >   return x;
> > 
> >// when we process this return statement, the parent contains nullptr 
> > (invalid candidate) and it will be invalidated (this is correct behavior)
> >return y;
> > }
> > ```
> > 
> > ```
> > X foo(bool b, X x) {
> >if (b)
> >   return x;
> > 
> >X y;
> >// when we process this return statement, the parent contains nullptr 
> > (invalid candidate) and it WON't be invalidated (this is correct behavior)
> >return y;
> > }
> > ```
> Oh, I see. Tricky. I don't find invalid cases now. But I recommend to comment 
> that the children would maintain the `ReturnSlots` of their parents. (This is 
> anti-intuition)
> 
> Have you tested any larger projects? Like libc++, libstdc++ or something like 
> folly. I feel we need to do such tests to avoid we get anything wrong.
I've already added a comment at the beginning of `updateNRVOCandidate` function 
where this point is mentioned: 
```
//  ... Therefore, we need to clear return slots for other
//  variables defined before the current return statement in the current
//  scope and in outer scopes.
```
If it's not enough, please let me know.


> Have you tested any larger projects?

Yes, I've built the `clang` itself and `compiler-rt` project. Then I've checked 
them to run 'check-all' (on built clang and compiler-rt). Everything  works.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119792/new/

https://reviews.llvm.org/D119792

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

[PATCH] D130331: [C++20] [Modules] Disable preferred_name when writing a C++20 Module interface

2022-07-26 Thread Chuanqi Xu via Phabricator via cfe-commits

ChuanqiXu added a comment.

Sorry for the frequent ping since 15.x is going to be branched. @aaron.ballman 
@tahonermann I know this might not look good to you. But I want to ask if there 
is any objection? Personally, the preferred_name attribute is not so important 
and it blocks something important. I really want to get this landed. And as 
@erichkeane said in D129748 , I'll take an 
eye on this.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130331/new/

https://reviews.llvm.org/D130331

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

[clang] 0ffb3dd - [clang-format] Fix a hang when formatting C# $@ string literals

2022-07-26 Thread via cfe-commits


Author: owenca
Date: 2022-07-25T23:17:54-07:00
New Revision: 0ffb3dd33ee1a50a6ab5db80bb8caee9133e66dc

URL: 
https://github.com/llvm/llvm-project/commit/0ffb3dd33ee1a50a6ab5db80bb8caee9133e66dc
DIFF: 
https://github.com/llvm/llvm-project/commit/0ffb3dd33ee1a50a6ab5db80bb8caee9133e66dc.diff

LOG: [clang-format] Fix a hang when formatting C# $@ string literals

Fixes #56624.

Differential Revision: https://reviews.llvm.org/D130411

Added: 


Modified: 
clang/lib/Format/FormatTokenLexer.cpp
clang/unittests/Format/FormatTestCSharp.cpp

Removed: 




diff  --git a/clang/lib/Format/FormatTokenLexer.cpp 
b/clang/lib/Format/FormatTokenLexer.cpp
index 66f03dcb53a12..3f9b68ccbb39f 100644
--- a/clang/lib/Format/FormatTokenLexer.cpp
+++ b/clang/lib/Format/FormatTokenLexer.cpp
@@ -239,55 +239,6 @@ bool FormatTokenLexer::tryMergeCSharpStringLiteral() {
   if (Tokens.size() < 2)
 return false;
 
-  // Interpolated strings could contain { } with " characters inside.
-  // $"{x ?? "null"}"
-  // should not be split into $"{x ?? ", null, "}" but should treated as a
-  // single string-literal.
-  //
-  // We opt not to try and format expressions inside {} within a C#
-  // interpolated string. Formatting expressions within an interpolated string
-  // would require similar work as that done for JavaScript template strings
-  // in `handleTemplateStrings()`.
-  auto  = *(Tokens.end() - 2);
-  if (CSharpInterpolatedString->getType() == TT_CSharpStringLiteral &&
-  (CSharpInterpolatedString->TokenText.startswith(R"($")") ||
-   CSharpInterpolatedString->TokenText.startswith(R"($@")"))) {
-int UnmatchedOpeningBraceCount = 0;
-
-auto TokenTextSize = CSharpInterpolatedString->TokenText.size();
-for (size_t Index = 0; Index < TokenTextSize; ++Index) {
-  char C = CSharpInterpolatedString->TokenText[Index];
-  if (C == '{') {
-// "{{"  inside an interpolated string is an escaped '{' so skip it.
-if (Index + 1 < TokenTextSize &&
-CSharpInterpolatedString->TokenText[Index + 1] == '{') {
-  ++Index;
-  continue;
-}
-++UnmatchedOpeningBraceCount;
-  } else if (C == '}') {
-// "}}"  inside an interpolated string is an escaped '}' so skip it.
-if (Index + 1 < TokenTextSize &&
-CSharpInterpolatedString->TokenText[Index + 1] == '}') {
-  ++Index;
-  continue;
-}
---UnmatchedOpeningBraceCount;
-  }
-}
-
-if (UnmatchedOpeningBraceCount > 0) {
-  auto  = *(Tokens.end() - 1);
-  CSharpInterpolatedString->TokenText =
-  StringRef(CSharpInterpolatedString->TokenText.begin(),
-NextToken->TokenText.end() -
-CSharpInterpolatedString->TokenText.begin());
-  CSharpInterpolatedString->ColumnWidth += NextToken->ColumnWidth;
-  Tokens.erase(Tokens.end() - 1);
-  return true;
-}
-  }
-
   // Look for @"aa" or $"aa".
   auto  = *(Tokens.end() - 1);
   if (!String->is(tok::string_literal))
@@ -571,45 +522,105 @@ void FormatTokenLexer::tryParseJSRegexLiteral() {
   resetLexer(SourceMgr.getFileOffset(Lex->getSourceLocation(Offset)));
 }
 
+static auto lexCSharpString(const char *Begin, const char *End, bool Verbatim,
+bool Interpolated) {
+  auto Repeated = [, End]() {
+return Begin + 1 < End && Begin[1] == Begin[0];
+  };
+
+  // Look for a terminating '"' in the current file buffer.
+  // Make no effort to format code within an interpolated or verbatim string.
+  //
+  // Interpolated strings could contain { } with " characters inside.
+  // $"{x ?? "null"}"
+  // should not be split into $"{x ?? ", null, "}" but should be treated as a
+  // single string-literal.
+  //
+  // We opt not to try and format expressions inside {} within a C#
+  // interpolated string. Formatting expressions within an interpolated string
+  // would require similar work as that done for JavaScript template strings
+  // in `handleTemplateStrings()`.
+  for (int UnmatchedOpeningBraceCount = 0; Begin < End; ++Begin) {
+switch (*Begin) {
+case '\\':
+  if (!Verbatim)
+++Begin;
+  break;
+case '{':
+  if (Interpolated) {
+// {{ inside an interpolated string is escaped, so skip it.
+if (Repeated())
+  ++Begin;
+else
+  ++UnmatchedOpeningBraceCount;
+  }
+  break;
+case '}':
+  if (Interpolated) {
+// }} inside an interpolated string is escaped, so skip it.
+if (Repeated())
+  ++Begin;
+else if (UnmatchedOpeningBraceCount > 0)
+  --UnmatchedOpeningBraceCount;
+else
+  return End;
+  }
+  break;
+case '"':
+  if (UnmatchedOpeningBraceCount > 0)
+break;
+  // "" within a verbatim string is an escaped double quote: skip it.
+  if (Verbatim

[PATCH] D130514: [CMake][Fuchsia] Enable assertions and backtraces in stage 1 build

2022-07-26 Thread Alex Brachet via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0df7d8bc355d: [CMake][Fuchsia] Enable assertions and 
backtraces in stage 1 build (authored by abrachet).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130514/new/

https://reviews.llvm.org/D130514

Files:
  clang/cmake/caches/Fuchsia.cmake


Index: clang/cmake/caches/Fuchsia.cmake
===
--- clang/cmake/caches/Fuchsia.cmake
+++ clang/cmake/caches/Fuchsia.cmake
@@ -34,8 +34,8 @@
 set(ENABLE_LINKER_BUILD_ID ON CACHE BOOL "")
 set(ENABLE_X86_RELAX_RELOCATIONS ON CACHE BOOL "")
 
-set(LLVM_ENABLE_ASSERTIONS OFF CACHE BOOL "")
-set(LLVM_ENABLE_BACKTRACES OFF CACHE BOOL "")
+set(LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
+set(LLVM_ENABLE_BACKTRACES ON CACHE BOOL "")
 set(CMAKE_BUILD_TYPE Release CACHE STRING "")
 if(APPLE)
   set(CMAKE_OSX_DEPLOYMENT_TARGET "10.13" CACHE STRING "")


Index: clang/cmake/caches/Fuchsia.cmake
===
--- clang/cmake/caches/Fuchsia.cmake
+++ clang/cmake/caches/Fuchsia.cmake
@@ -34,8 +34,8 @@
 set(ENABLE_LINKER_BUILD_ID ON CACHE BOOL "")
 set(ENABLE_X86_RELAX_RELOCATIONS ON CACHE BOOL "")
 
-set(LLVM_ENABLE_ASSERTIONS OFF CACHE BOOL "")
-set(LLVM_ENABLE_BACKTRACES OFF CACHE BOOL "")
+set(LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
+set(LLVM_ENABLE_BACKTRACES ON CACHE BOOL "")
 set(CMAKE_BUILD_TYPE Release CACHE STRING "")
 if(APPLE)
   set(CMAKE_OSX_DEPLOYMENT_TARGET "10.13" CACHE STRING "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 0df7d8b - [CMake][Fuchsia] Enable assertions and backtraces in stage 1 build

2022-07-26 Thread Alex Brachet via cfe-commits


Author: Alex Brachet
Date: 2022-07-26T06:09:38Z
New Revision: 0df7d8bc355dd506bb1a330b9f73e611f0deaf9f

URL: 
https://github.com/llvm/llvm-project/commit/0df7d8bc355dd506bb1a330b9f73e611f0deaf9f
DIFF: 
https://github.com/llvm/llvm-project/commit/0df7d8bc355dd506bb1a330b9f73e611f0deaf9f.diff

LOG: [CMake][Fuchsia] Enable assertions and backtraces in stage 1 build

Differential Revision: https://reviews.llvm.org/D130514

Added: 


Modified: 
clang/cmake/caches/Fuchsia.cmake

Removed: 




diff  --git a/clang/cmake/caches/Fuchsia.cmake 
b/clang/cmake/caches/Fuchsia.cmake
index 73ef571507179..1978195c267d9 100644
--- a/clang/cmake/caches/Fuchsia.cmake
+++ b/clang/cmake/caches/Fuchsia.cmake
@@ -34,8 +34,8 @@ set(CLANG_PLUGIN_SUPPORT OFF CACHE BOOL "")
 set(ENABLE_LINKER_BUILD_ID ON CACHE BOOL "")
 set(ENABLE_X86_RELAX_RELOCATIONS ON CACHE BOOL "")
 
-set(LLVM_ENABLE_ASSERTIONS OFF CACHE BOOL "")
-set(LLVM_ENABLE_BACKTRACES OFF CACHE BOOL "")
+set(LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
+set(LLVM_ENABLE_BACKTRACES ON CACHE BOOL "")
 set(CMAKE_BUILD_TYPE Release CACHE STRING "")
 if(APPLE)
   set(CMAKE_OSX_DEPLOYMENT_TARGET "10.13" CACHE STRING "")



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

[clang-tools-extra] 3f3930a - Remove redundaunt virtual specifiers (NFC)

2022-07-26 Thread Kazu Hirata via cfe-commits


Author: Kazu Hirata
Date: 2022-07-25T23:00:59-07:00
New Revision: 3f3930a451e118e82885a6dd20e3918427b816c2

URL: 
https://github.com/llvm/llvm-project/commit/3f3930a451e118e82885a6dd20e3918427b816c2
DIFF: 
https://github.com/llvm/llvm-project/commit/3f3930a451e118e82885a6dd20e3918427b816c2.diff

LOG: Remove redundaunt virtual specifiers (NFC)

Identified with tidy-modernize-use-override.

Added: 


Modified: 
clang-tools-extra/clang-tidy/bugprone/SharedPtrArrayMismatchCheck.h
clang-tools-extra/clangd/AST.cpp
clang-tools-extra/clangd/support/ThreadsafeFS.cpp
clang/lib/Basic/Targets/X86.h
clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
clang/lib/CodeGen/MicrosoftCXXABI.cpp
clang/lib/Sema/Sema.cpp
clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h
llvm/lib/MC/ELFObjectWriter.cpp
llvm/lib/Target/Mips/MipsPreLegalizerCombiner.cpp
llvm/lib/Target/RISCV/RISCVInstrInfo.h
llvm/lib/Target/X86/X86InstrInfo.h
llvm/lib/Transforms/IPO/AttributorAttributes.cpp
llvm/lib/Transforms/InstCombine/InstCombineInternal.h
polly/include/polly/CodeGen/BlockGenerators.h
polly/include/polly/CodeGen/LoopGeneratorsGOMP.h
polly/include/polly/CodeGen/LoopGeneratorsKMP.h
polly/include/polly/ScopPass.h
polly/lib/CodeGen/CodegenCleanup.cpp
polly/lib/Support/DumpFunctionPass.cpp
polly/lib/Support/DumpModulePass.cpp
polly/lib/Transform/DeLICM.cpp
polly/lib/Transform/FlattenSchedule.cpp
polly/lib/Transform/Simplify.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/bugprone/SharedPtrArrayMismatchCheck.h 
b/clang-tools-extra/clang-tidy/bugprone/SharedPtrArrayMismatchCheck.h
index bb549c5a2c695..95b8e22588a50 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SharedPtrArrayMismatchCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/SharedPtrArrayMismatchCheck.h
@@ -28,7 +28,7 @@ class SharedPtrArrayMismatchCheck : public 
SmartPtrArrayMismatchCheck {
   SharedPtrArrayMismatchCheck(StringRef Name, ClangTidyContext *Context);
 
 protected:
-  virtual SmartPtrClassMatcher getSmartPointerClassMatcher() const override;
+  SmartPtrClassMatcher getSmartPointerClassMatcher() const override;
 };
 
 } // namespace bugprone

diff  --git a/clang-tools-extra/clangd/AST.cpp 
b/clang-tools-extra/clangd/AST.cpp
index 0c67c548e6c20..f7d526fab0963 100644
--- a/clang-tools-extra/clangd/AST.cpp
+++ b/clang-tools-extra/clangd/AST.cpp
@@ -368,7 +368,7 @@ std::string printType(const QualType QT, const DeclContext 
,
   public:
 PrintCB(const DeclContext *CurContext) : CurContext(CurContext) {}
 virtual ~PrintCB() {}
-virtual bool isScopeVisible(const DeclContext *DC) const override {
+bool isScopeVisible(const DeclContext *DC) const override {
   return DC->Encloses(CurContext);
 }
 

diff  --git a/clang-tools-extra/clangd/support/ThreadsafeFS.cpp 
b/clang-tools-extra/clangd/support/ThreadsafeFS.cpp
index efd7e8cc6079c..dca04762b49a6 100644
--- a/clang-tools-extra/clangd/support/ThreadsafeFS.cpp
+++ b/clang-tools-extra/clangd/support/ThreadsafeFS.cpp
@@ -53,7 +53,7 @@ class VolatileFileSystem : public llvm::vfs::ProxyFileSystem {
   assert(this->Wrapped);
 }
 
-virtual llvm::ErrorOr>
+llvm::ErrorOr>
 getBuffer(const llvm::Twine , int64_t FileSize,
   bool RequiresNullTerminator, bool /*IsVolatile*/) override {
   return Wrapped->getBuffer(Name, FileSize, RequiresNullTerminator,

diff  --git a/clang/lib/Basic/Targets/X86.h b/clang/lib/Basic/Targets/X86.h
index ea98dcf42de65..0affa58b2f4c0 100644
--- a/clang/lib/Basic/Targets/X86.h
+++ b/clang/lib/Basic/Targets/X86.h
@@ -229,12 +229,12 @@ class LLVM_LIBRARY_VISIBILITY X86TargetInfo : public 
TargetInfo {
   bool validateInputSize(const llvm::StringMap ,
  StringRef Constraint, unsigned Size) const override;
 
-  virtual bool
+  bool
   checkCFProtectionReturnSupported(DiagnosticsEngine ) const override {
 return true;
   };
 
-  virtual bool
+  bool
   checkCFProtectionBranchSupported(DiagnosticsEngine ) const override {
 return true;
   };

diff  --git a/clang/lib/CodeGen/CGOpenMPRuntimeGPU.h 
b/clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
index 1d30c5061743a..ff585efa3fce2 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
+++ b/clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
@@ -186,17 +186,16 @@ class CGOpenMPRuntimeGPU : public CGOpenMPRuntime {
 
   /// Emit call to void __kmpc_push_proc_bind(ident_t *loc, kmp_int32
   /// global_tid, int proc_bind) to generate code for 'proc_bind' clause.
-  virtual void emitProcBindClause(CodeGenFunction ,
-  llvm::omp::ProcBindKind ProcBind,
-  SourceLocation Loc) override;
+

[clang] ae002f8 - Use isa instead of dyn_cast (NFC)

2022-07-26 Thread Kazu Hirata via cfe-commits


Author: Kazu Hirata
Date: 2022-07-25T23:00:58-07:00
New Revision: ae002f8bca11bf652fa4d2683c8a627fa77158a8

URL: 
https://github.com/llvm/llvm-project/commit/ae002f8bca11bf652fa4d2683c8a627fa77158a8
DIFF: 
https://github.com/llvm/llvm-project/commit/ae002f8bca11bf652fa4d2683c8a627fa77158a8.diff

LOG: Use isa instead of dyn_cast (NFC)

Added: 


Modified: 
clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
llvm/tools/llvm-dwarfutil/llvm-dwarfutil.cpp
mlir/lib/TableGen/Pattern.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp 
b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index 08fac9fb2e698..f12a0ee1fc6db 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -908,7 +908,7 @@ void ExprEngine::VisitCXXNewExpr(const CXXNewExpr *CNE, 
ExplodedNode *Pred,
   // values are properly placed inside the required region, however if an
   // initializer list is used, this doesn't happen automatically.
   auto *Init = CNE->getInitializer();
-  bool isInitList = dyn_cast_or_null(Init);
+  bool isInitList = isa_and_nonnull(Init);
 
   QualType ObjTy =
   isInitList ? Init->getType() : CNE->getType()->getPointeeType();

diff  --git a/llvm/tools/llvm-dwarfutil/llvm-dwarfutil.cpp 
b/llvm/tools/llvm-dwarfutil/llvm-dwarfutil.cpp
index e77c82e0fad9a..c91674426bbb0 100644
--- a/llvm/tools/llvm-dwarfutil/llvm-dwarfutil.cpp
+++ b/llvm/tools/llvm-dwarfutil/llvm-dwarfutil.cpp
@@ -292,7 +292,7 @@ using DebugInfoBits = SmallString<1>;
 static Error addSectionsFromLinkedData(objcopy::ConfigManager ,
ObjectFile ,
DebugInfoBits ) {
-  if (dyn_cast>()) {
+  if (isa>()) {
 Expected> MemFile = ELFObjectFile::create(
 MemoryBufferRef(LinkedDebugInfoBits, ""));
 if (!MemFile)
@@ -300,7 +300,7 @@ static Error 
addSectionsFromLinkedData(objcopy::ConfigManager ,
 
 if (Error Err = setConfigToAddNewDebugSections(Config, *MemFile))
   return Err;
-  } else if (dyn_cast>()) {
+  } else if (isa>()) {
 Expected> MemFile = ELFObjectFile::create(
 MemoryBufferRef(LinkedDebugInfoBits, ""));
 if (!MemFile)
@@ -308,7 +308,7 @@ static Error 
addSectionsFromLinkedData(objcopy::ConfigManager ,
 
 if (Error Err = setConfigToAddNewDebugSections(Config, *MemFile))
   return Err;
-  } else if (dyn_cast>()) {
+  } else if (isa>()) {
 Expected> MemFile = ELFObjectFile::create(
 MemoryBufferRef(LinkedDebugInfoBits, ""));
 if (!MemFile)
@@ -316,7 +316,7 @@ static Error 
addSectionsFromLinkedData(objcopy::ConfigManager ,
 
 if (Error Err = setConfigToAddNewDebugSections(Config, *MemFile))
   return Err;
-  } else if (dyn_cast>()) {
+  } else if (isa>()) {
 Expected> MemFile = ELFObjectFile::create(
 MemoryBufferRef(LinkedDebugInfoBits, ""));
 if (!MemFile)

diff  --git a/mlir/lib/TableGen/Pattern.cpp b/mlir/lib/TableGen/Pattern.cpp
index de767d6d3a6a1..d833de5100cc3 100644
--- a/mlir/lib/TableGen/Pattern.cpp
+++ b/mlir/lib/TableGen/Pattern.cpp
@@ -33,7 +33,7 @@ using llvm::formatv;
 
//===--===//
 
 bool DagLeaf::isUnspecified() const {
-  return dyn_cast_or_null(def);
+  return isa_and_nonnull(def);
 }
 
 bool DagLeaf::isOperandMatcher() const {



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

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

2022-07-26 Thread Chuanqi Xu via Phabricator via cfe-commits

ChuanqiXu added inline comments.



Comment at: clang/lib/Sema/Scope.cpp:152-154
+  // Consider the variable as NRVO candidate if the return slot is available
+  // for it in the current scope, or if it can be available in outer scopes.
+  NRVO = CanBePutInReturnSlot ? VD : nullptr;

rusyaev-roman wrote:
> ChuanqiXu wrote:
> > What if NRVO contains a value already? It is possible due to the value of 
> > NRVO could be set by its children.
> Actually this is intention. If the parent has already NRVO candidate, then it 
> should be invalidated (or not). Let's consider the following examples:
> 
> 
> ```
> X foo(bool b) {
>X x;
>X y;
>if (b)
>   return x;
>else
>   return y; // when we process this return statement, the parent has 
> already NRVO and it will be invalidated (this is correct behavior)
> }
> ```
> 
> ```
> X foo(bool b) {
>X x;
>if (b)
>   return x;
>
>X y;
>// when we process this return statement, the parent has already NRVO and 
> it WON't be invalidated
>//  (this is correct behavior), because a return slot will be available 
> for it
>return y;
> }
> ```
> 
> ```
> X foo(bool b) {
>X x;
>if (b)
>   return x;
> 
>// when we process this return statement, the parent has already NRVO and 
> it WON't be invalidated (this is correct behavior)
>return x;
> }
> ```
> 
> ```
> X foo(bool b, X x) {
>X y;
>
>if (b)
>   return x;
> 
>// when we process this return statement, the parent contains nullptr 
> (invalid candidate) and it will be invalidated (this is correct behavior)
>return y;
> }
> ```
> 
> ```
> X foo(bool b, X x) {
>if (b)
>   return x;
> 
>X y;
>// when we process this return statement, the parent contains nullptr 
> (invalid candidate) and it WON't be invalidated (this is correct behavior)
>return y;
> }
> ```
Oh, I see. Tricky. I don't find invalid cases now. But I recommend to comment 
that the children would maintain the `ReturnSlots` of their parents. (This is 
anti-intuition)

Have you tested any larger projects? Like libc++, libstdc++ or something like 
folly. I feel we need to do such tests to avoid we get anything wrong.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119792/new/

https://reviews.llvm.org/D119792

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

< 1 2 3

201 - 247 of 247 matches

Mail list logo