[PATCH] D117380: [RISCV] [Clang] Add attra for crc32_d/crc32c_d

2022-01-15 Thread Craig Topper via Phabricator via cfe-commits
craig.topper accepted this revision.
craig.topper added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117380

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


[PATCH] D93298: [RISCV] add the MC layer support of Zfinx extension

2022-01-15 Thread Shao-Ce SUN via Phabricator via cfe-commits
achieveartificialintelligence updated this revision to Diff 400256.
achieveartificialintelligence added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93298

Files:
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/Disassembler/RISCVDisassembler.cpp
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVInstrInfoD.td
  llvm/lib/Target/RISCV/RISCVInstrInfoF.td
  llvm/lib/Target/RISCV/RISCVInstrInfoZfh.td
  llvm/lib/Target/RISCV/RISCVRegisterInfo.td
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/MC/RISCV/attribute-arch.s
  llvm/test/MC/RISCV/rv32i-invalid.s
  llvm/test/MC/RISCV/rv32zdinx-invalid.s
  llvm/test/MC/RISCV/rv32zdinx-valid.s
  llvm/test/MC/RISCV/rv32zfinx-invalid.s
  llvm/test/MC/RISCV/rv32zfinx-valid.s
  llvm/test/MC/RISCV/rv32zhinx-invalid.s
  llvm/test/MC/RISCV/rv32zhinx-valid.s
  llvm/test/MC/RISCV/rv32zhinxmin-invalid.s
  llvm/test/MC/RISCV/rv32zhinxmin-valid.s
  llvm/test/MC/RISCV/rv64zdinx-invalid.s
  llvm/test/MC/RISCV/rv64zdinx-valid.s
  llvm/test/MC/RISCV/rv64zfinx-invalid.s
  llvm/test/MC/RISCV/rv64zfinx-valid.s
  llvm/test/MC/RISCV/rv64zhinx-invalid.s
  llvm/test/MC/RISCV/rv64zhinx-valid.s
  llvm/test/MC/RISCV/rv64zhinxmin-invalid.s
  llvm/test/MC/RISCV/rv64zhinxmin-valid.s
  llvm/test/MC/RISCV/rvzdinx-aliases-valid.s
  llvm/test/MC/RISCV/rvzfinx-aliases-valid.s
  llvm/test/MC/RISCV/rvzhinx-aliases-valid.s

Index: llvm/test/MC/RISCV/rvzhinx-aliases-valid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/rvzhinx-aliases-valid.s
@@ -0,0 +1,82 @@
+# RUN: llvm-mc %s -triple=riscv32 -mattr=+zhinx -riscv-no-aliases \
+# RUN: | FileCheck -check-prefix=CHECK-INST %s
+# RUN: llvm-mc %s -triple=riscv32 -mattr=+zhinx \
+# RUN: | FileCheck -check-prefix=CHECK-ALIAS %s
+# RUN: llvm-mc %s -triple=riscv64 -mattr=+zhinx -riscv-no-aliases \
+# RUN: | FileCheck -check-prefix=CHECK-INST %s
+# RUN: llvm-mc %s -triple=riscv64 -mattr=+zhinx \
+# RUN: | FileCheck -check-prefix=CHECK-ALIAS %s
+# RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+zhinx %s \
+# RUN: | llvm-objdump -d --mattr=+zhinx -M no-aliases - \
+# RUN: | FileCheck -check-prefix=CHECK-INST %s
+# RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+zhinx %s \
+# RUN: | llvm-objdump -d --mattr=+zhinx - \
+# RUN: | FileCheck -check-prefix=CHECK-ALIAS %s
+# RUN: llvm-mc -filetype=obj -triple riscv64 -mattr=+zhinx %s \
+# RUN: | llvm-objdump -d --mattr=+zhinx -M no-aliases - \
+# RUN: | FileCheck -check-prefix=CHECK-INST %s
+# RUN: llvm-mc -filetype=obj -triple riscv64 -mattr=+zhinx %s \
+# RUN: | llvm-objdump -d --mattr=+zhinx - \
+# RUN: | FileCheck -check-prefix=CHECK-ALIAS %s
+
+##===--===##
+## Assembler Pseudo Instructions (User-Level ISA, Version 2.2, Chapter 20)
+##===--===##
+
+# CHECK-INST: fsgnjx.h s1, s2, s2
+# CHECK-ALIAS: fabs.h s1, s2
+fabs.h s1, s2
+# CHECK-INST: fsgnjn.h s2, s3, s3
+# CHECK-ALIAS: fneg.h s2, s3
+fneg.h s2, s3
+
+# CHECK-INST: flt.h tp, s6, s5
+# CHECK-ALIAS: flt.h tp, s6, s5
+fgt.h x4, s5, s6
+# CHECK-INST: fle.h t2, s1, s0
+# CHECK-ALIAS: fle.h t2, s1, s0
+fge.h x7, x8, x9
+
+##===--===##
+## Aliases which omit the rounding mode.
+##===--===##
+
+# CHECK-INST: fmadd.h a0, a1, a2, a3, dyn
+# CHECK-ALIAS: fmadd.h a0, a1, a2, a3
+fmadd.h x10, x11, x12, x13
+# CHECK-INST: fmsub.h a4, a5, a6, a7, dyn
+# CHECK-ALIAS: fmsub.h a4, a5, a6, a7
+fmsub.h x14, x15, x16, x17
+# CHECK-INST: fnmsub.h s2, s3, s4, s5, dyn
+# CHECK-ALIAS: fnmsub.h s2, s3, s4, s5
+fnmsub.h x18, x19, x20, x21
+# CHECK-INST: fnmadd.h s6, s7, s8, s9, dyn
+# CHECK-ALIAS: fnmadd.h s6, s7, s8, s9
+fnmadd.h x22, x23, x24, x25
+# CHECK-INST: fadd.h s10, s11, t3, dyn
+# CHECK-ALIAS: fadd.h s10, s11, t3
+fadd.h x26, x27, x28
+# CHECK-INST: fsub.h t4, t5, t6, dyn
+# CHECK-ALIAS: fsub.h t4, t5, t6
+fsub.h x29, x30, x31
+# CHECK-INST: fmul.h s0, s1, s2, dyn
+# CHECK-ALIAS: fmul.h s0, s1, s2
+fmul.h s0, s1, s2
+# CHECK-INST: fdiv.h s3, s4, s5, dyn
+# CHECK-ALIAS: fdiv.h s3, s4, s5
+fdiv.h s3, s4, s5
+# CHECK-INST: fsqrt.h s6, s7, dyn
+# CHECK-ALIAS: fsqrt.h s6, s7
+fsqrt.h s6, s7
+# CHECK-INST: fcvt.w.h a0, s5, dyn
+# CHECK-ALIAS: fcvt.w.h a0, s5
+fcvt.w.h a0, s5
+# CHECK-INST: fcvt.wu.h a1, s6, dyn
+# CHECK-ALIAS: fcvt.wu.h a1, s6
+fcvt.wu.h a1, s6
+# CHECK-INST: fcvt.h.w t6, a4, dyn
+# CHECK-ALIAS: fcvt.h.w t6, a4
+fcvt.h.w t6, a4
+# CHECK-INST: fcvt.h.wu s0, a5, dyn
+# CHECK-ALIAS: fcvt.h.wu s0, a5
+fcvt.h.wu s0, a5
Index: llvm/test/MC/RISCV/rvzfinx-aliases-valid.s

[clang] 8123e2e - [RISCV][Clang] Add attrs to crc32_d/crc32c_d

2022-01-15 Thread Ben Shi via cfe-commits

Author: Chenbing.Zheng
Date: 2022-01-15T08:40:09Z
New Revision: 8123e2ed7490440693ed69d00618aa4f13e70b50

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

LOG: [RISCV][Clang] Add attrs to crc32_d/crc32c_d

Reviewed By: craig.topper

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

Added: 


Modified: 
clang/include/clang/Basic/BuiltinsRISCV.def

Removed: 




diff  --git a/clang/include/clang/Basic/BuiltinsRISCV.def 
b/clang/include/clang/Basic/BuiltinsRISCV.def
index 81774d984af93..568ab0bbaec78 100644
--- a/clang/include/clang/Basic/BuiltinsRISCV.def
+++ b/clang/include/clang/Basic/BuiltinsRISCV.def
@@ -58,8 +58,8 @@ TARGET_BUILTIN(__builtin_riscv_crc32_w, "LiLi", "nc", 
"experimental-zbr")
 TARGET_BUILTIN(__builtin_riscv_crc32c_b, "LiLi", "nc", "experimental-zbr")
 TARGET_BUILTIN(__builtin_riscv_crc32c_h, "LiLi", "nc", "experimental-zbr")
 TARGET_BUILTIN(__builtin_riscv_crc32c_w, "LiLi", "nc", "experimental-zbr")
-TARGET_BUILTIN(__builtin_riscv_crc32_d, "LiLi", "nc", "experimental-zbr")
-TARGET_BUILTIN(__builtin_riscv_crc32c_d, "LiLi", "nc", "experimental-zbr")
+TARGET_BUILTIN(__builtin_riscv_crc32_d, "LiLi", "nc", "experimental-zbr,64bit")
+TARGET_BUILTIN(__builtin_riscv_crc32c_d, "LiLi", "nc", 
"experimental-zbr,64bit")
 
 #undef BUILTIN
 #undef TARGET_BUILTIN



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


[PATCH] D117380: [RISCV] [Clang] Add attra for crc32_d/crc32c_d

2022-01-15 Thread Ben Shi 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 rG8123e2ed7490: [RISCV][Clang] Add attrs to crc32_d/crc32c_d 
(authored by Chenbing.Zheng, committed by benshi001).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117380

Files:
  clang/include/clang/Basic/BuiltinsRISCV.def


Index: clang/include/clang/Basic/BuiltinsRISCV.def
===
--- clang/include/clang/Basic/BuiltinsRISCV.def
+++ clang/include/clang/Basic/BuiltinsRISCV.def
@@ -58,8 +58,8 @@
 TARGET_BUILTIN(__builtin_riscv_crc32c_b, "LiLi", "nc", "experimental-zbr")
 TARGET_BUILTIN(__builtin_riscv_crc32c_h, "LiLi", "nc", "experimental-zbr")
 TARGET_BUILTIN(__builtin_riscv_crc32c_w, "LiLi", "nc", "experimental-zbr")
-TARGET_BUILTIN(__builtin_riscv_crc32_d, "LiLi", "nc", "experimental-zbr")
-TARGET_BUILTIN(__builtin_riscv_crc32c_d, "LiLi", "nc", "experimental-zbr")
+TARGET_BUILTIN(__builtin_riscv_crc32_d, "LiLi", "nc", "experimental-zbr,64bit")
+TARGET_BUILTIN(__builtin_riscv_crc32c_d, "LiLi", "nc", 
"experimental-zbr,64bit")
 
 #undef BUILTIN
 #undef TARGET_BUILTIN


Index: clang/include/clang/Basic/BuiltinsRISCV.def
===
--- clang/include/clang/Basic/BuiltinsRISCV.def
+++ clang/include/clang/Basic/BuiltinsRISCV.def
@@ -58,8 +58,8 @@
 TARGET_BUILTIN(__builtin_riscv_crc32c_b, "LiLi", "nc", "experimental-zbr")
 TARGET_BUILTIN(__builtin_riscv_crc32c_h, "LiLi", "nc", "experimental-zbr")
 TARGET_BUILTIN(__builtin_riscv_crc32c_w, "LiLi", "nc", "experimental-zbr")
-TARGET_BUILTIN(__builtin_riscv_crc32_d, "LiLi", "nc", "experimental-zbr")
-TARGET_BUILTIN(__builtin_riscv_crc32c_d, "LiLi", "nc", "experimental-zbr")
+TARGET_BUILTIN(__builtin_riscv_crc32_d, "LiLi", "nc", "experimental-zbr,64bit")
+TARGET_BUILTIN(__builtin_riscv_crc32c_d, "LiLi", "nc", "experimental-zbr,64bit")
 
 #undef BUILTIN
 #undef TARGET_BUILTIN
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D117390: [AST] Reformat CastExpr unittest suite

2022-01-15 Thread Kim Gräsman via Phabricator via cfe-commits
kimgr created this revision.
kimgr requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

In preparation for adding new tests. No functional change.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117390

Files:
  clang/unittests/Tooling/CastExprTest.cpp


Index: clang/unittests/Tooling/CastExprTest.cpp
===
--- clang/unittests/Tooling/CastExprTest.cpp
+++ clang/unittests/Tooling/CastExprTest.cpp
@@ -23,15 +23,15 @@
 };
 
 TEST(CastExprTest, GetSubExprAsWrittenThroughMaterializedTemporary) {
-CastExprVisitor Visitor;
-Visitor.OnExplicitCast = [](ExplicitCastExpr *Expr) {
-  auto Sub = Expr->getSubExprAsWritten();
-  EXPECT_TRUE(isa(Sub))
+  CastExprVisitor Visitor;
+  Visitor.OnExplicitCast = [](ExplicitCastExpr *Expr) {
+auto Sub = Expr->getSubExprAsWritten();
+EXPECT_TRUE(isa(Sub))
 << "Expected DeclRefExpr, but saw " << Sub->getStmtClassName();
-};
-Visitor.runOver("struct S1 {};\n"
-"struct S2 { operator S1(); };\n"
-"S1 f(S2 s) { return static_cast(s); }\n");
+  };
+  Visitor.runOver("struct S1 {};\n"
+  "struct S2 { operator S1(); };\n"
+  "S1 f(S2 s) { return static_cast(s); }\n");
 }
 
 // Verify that getSubExprAsWritten looks through a ConstantExpr in a scenario
@@ -43,15 +43,15 @@
 // `-CXXConstructExpr 'S' 'void (int)'
 //   `-IntegerLiteral 'int' 0
 TEST(CastExprTest, GetSubExprAsWrittenThroughConstantExpr) {
-CastExprVisitor Visitor;
-Visitor.OnExplicitCast = [](ExplicitCastExpr *Expr) {
-  auto *Sub = Expr->getSubExprAsWritten();
-  EXPECT_TRUE(isa(Sub))
+  CastExprVisitor Visitor;
+  Visitor.OnExplicitCast = [](ExplicitCastExpr *Expr) {
+auto *Sub = Expr->getSubExprAsWritten();
+EXPECT_TRUE(isa(Sub))
 << "Expected IntegerLiteral, but saw " << Sub->getStmtClassName();
-};
-Visitor.runOver("struct S { consteval S(int) {} };\n"
-"S f() { return S(0); }\n",
-CastExprVisitor::Lang_CXX2a);
+  };
+  Visitor.runOver("struct S { consteval S(int) {} };\n"
+  "S f() { return S(0); }\n",
+  CastExprVisitor::Lang_CXX2a);
 }
 
-}
+} // namespace


Index: clang/unittests/Tooling/CastExprTest.cpp
===
--- clang/unittests/Tooling/CastExprTest.cpp
+++ clang/unittests/Tooling/CastExprTest.cpp
@@ -23,15 +23,15 @@
 };
 
 TEST(CastExprTest, GetSubExprAsWrittenThroughMaterializedTemporary) {
-CastExprVisitor Visitor;
-Visitor.OnExplicitCast = [](ExplicitCastExpr *Expr) {
-  auto Sub = Expr->getSubExprAsWritten();
-  EXPECT_TRUE(isa(Sub))
+  CastExprVisitor Visitor;
+  Visitor.OnExplicitCast = [](ExplicitCastExpr *Expr) {
+auto Sub = Expr->getSubExprAsWritten();
+EXPECT_TRUE(isa(Sub))
 << "Expected DeclRefExpr, but saw " << Sub->getStmtClassName();
-};
-Visitor.runOver("struct S1 {};\n"
-"struct S2 { operator S1(); };\n"
-"S1 f(S2 s) { return static_cast(s); }\n");
+  };
+  Visitor.runOver("struct S1 {};\n"
+  "struct S2 { operator S1(); };\n"
+  "S1 f(S2 s) { return static_cast(s); }\n");
 }
 
 // Verify that getSubExprAsWritten looks through a ConstantExpr in a scenario
@@ -43,15 +43,15 @@
 // `-CXXConstructExpr 'S' 'void (int)'
 //   `-IntegerLiteral 'int' 0
 TEST(CastExprTest, GetSubExprAsWrittenThroughConstantExpr) {
-CastExprVisitor Visitor;
-Visitor.OnExplicitCast = [](ExplicitCastExpr *Expr) {
-  auto *Sub = Expr->getSubExprAsWritten();
-  EXPECT_TRUE(isa(Sub))
+  CastExprVisitor Visitor;
+  Visitor.OnExplicitCast = [](ExplicitCastExpr *Expr) {
+auto *Sub = Expr->getSubExprAsWritten();
+EXPECT_TRUE(isa(Sub))
 << "Expected IntegerLiteral, but saw " << Sub->getStmtClassName();
-};
-Visitor.runOver("struct S { consteval S(int) {} };\n"
-"S f() { return S(0); }\n",
-CastExprVisitor::Lang_CXX2a);
+  };
+  Visitor.runOver("struct S { consteval S(int) {} };\n"
+  "S f() { return S(0); }\n",
+  CastExprVisitor::Lang_CXX2a);
 }
 
-}
+} // namespace
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D117391: [AST] Ignore implicit nodes in CastExpr::getConversionFunction

2022-01-15 Thread Kim Gräsman via Phabricator via cfe-commits
kimgr created this revision.
kimgr requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Under -std=c++20 with use of consteval, the subexpression hierarchy sometimes
contains ConstantExpr nodes that getConversionFunction did not expect.

CastExpr::getConversionFunction is very rarely used, so this was difficult to
trigger in the compiler. It is possible to reproduce using -ast-dump=json, which
triggers an assertion for inputs with consteval implicit casts.

In AST-based tools, however, it surfaces quite easily if they try to inspect the
conversion function of a visited CastExpr.

Add two new testcases, both for implicit constructor conversions and
user-defined conversions with consteval.

Depends on D117390 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117391

Files:
  clang/lib/AST/Expr.cpp
  clang/unittests/Tooling/CastExprTest.cpp


Index: clang/unittests/Tooling/CastExprTest.cpp
===
--- clang/unittests/Tooling/CastExprTest.cpp
+++ clang/unittests/Tooling/CastExprTest.cpp
@@ -14,12 +14,19 @@
 
 struct CastExprVisitor : TestVisitor {
   std::function OnExplicitCast;
+  std::function OnCast;
 
   bool VisitExplicitCastExpr(ExplicitCastExpr *Expr) {
 if (OnExplicitCast)
   OnExplicitCast(Expr);
 return true;
   }
+
+  bool VisitCastExpr(CastExpr *Expr) {
+if (OnCast)
+  OnCast(Expr);
+return true;
+  }
 };
 
 TEST(CastExprTest, GetSubExprAsWrittenThroughMaterializedTemporary) {
@@ -54,4 +61,57 @@
   CastExprVisitor::Lang_CXX2a);
 }
 
+// Verify that getConversionFunction looks through a ConstantExpr for implicit
+// constructor conversions (https://github.com/llvm/llvm-project/issues/53044):
+//
+// `-ImplicitCastExpr 'S' 
+// `-ConstantExpr 'S'
+//   |-value: Struct
+//   `-CXXConstructExpr 'S' 'void (const char *)'
+// `-ImplicitCastExpr 'const char *' 
+//   `-StringLiteral 'const char [7]' lvalue "foobar"
+TEST(CastExprTest, GetCtorConversionFunctionThroughConstantExpr) {
+  CastExprVisitor Visitor;
+  Visitor.OnCast = [](CastExpr *Expr) {
+if (Expr->getCastKind() == CK_ConstructorConversion) {
+  auto *Conv = Expr->getConversionFunction();
+  EXPECT_TRUE(isa(Conv))
+  << "Expected CXXConstructorDecl, but saw " << 
Conv->getDeclKindName();
+}
+  };
+  Visitor.runOver("struct X { consteval X(const char *) {} };\n"
+  "void f() { X x = \"foobar\"; }\n",
+  CastExprVisitor::Lang_CXX2a);
+}
+
+// Verify that getConversionFunction looks through a ConstantExpr for implicit
+// user-defined conversions.
+//
+// `-ImplicitCastExpr 'const char *' 
+//   `-ConstantExpr 'const char *'
+// |-value: LValue
+// `-CXXMemberCallExpr 'const char *'
+//   `-MemberExpr '' .operator const char *
+// `-DeclRefExpr 'const X' lvalue Var 'x' 'const X'
+TEST(CastExprTest, GetUserDefinedConversionFunctionThroughConstantExpr) {
+  CastExprVisitor Visitor;
+  Visitor.OnCast = [](CastExpr *Expr) {
+if (Expr->getCastKind() == CK_UserDefinedConversion) {
+  auto *Conv = Expr->getConversionFunction();
+  EXPECT_TRUE(isa(Conv))
+  << "Expected CXXMethodDecl, but saw " << Conv->getDeclKindName();
+}
+  };
+  Visitor.runOver("struct X {\n"
+  "  consteval operator const char *() const {\n"
+  "return nullptr;\n"
+  "  }\n"
+  "};\n"
+  "const char *f() {\n"
+  "  constexpr X x;\n";
+  "  return x;\n"
+  "}\n",
+  CastExprVisitor::Lang_CXX2a);
+}
+
 } // namespace
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -1944,6 +1944,7 @@
 
   for (const CastExpr *E = this; E; E = dyn_cast(SubExpr)) {
 SubExpr = skipImplicitTemporary(E->getSubExpr());
+SubExpr = SubExpr->IgnoreImplicit();
 
 if (E->getCastKind() == CK_ConstructorConversion)
   return cast(SubExpr)->getConstructor();


Index: clang/unittests/Tooling/CastExprTest.cpp
===
--- clang/unittests/Tooling/CastExprTest.cpp
+++ clang/unittests/Tooling/CastExprTest.cpp
@@ -14,12 +14,19 @@
 
 struct CastExprVisitor : TestVisitor {
   std::function OnExplicitCast;
+  std::function OnCast;
 
   bool VisitExplicitCastExpr(ExplicitCastExpr *Expr) {
 if (OnExplicitCast)
   OnExplicitCast(Expr);
 return true;
   }
+
+  bool VisitCastExpr(CastExpr *Expr) {
+if (OnCast)
+  OnCast(Expr);
+return true;
+  }
 };
 
 TEST(CastExprTest, GetSubExprAsWrittenThroughMaterializedTemporary) {
@@ -54,4 +61,57 @@
   CastExprVisitor::Lang_CXX2a);
 }
 
+// Verify that getConversionFunction looks through a ConstantExpr for implicit
+

[PATCH] D116875: [clang-tidy] Add performance-inefficient-array-traversal check

2022-01-15 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 400266.
njames93 added a comment.

Add support for array access `Arr[I * JExtent + J]`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116875

Files:
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/InefficientArrayTraversalCheck.cpp
  clang-tools-extra/clang-tidy/performance/InefficientArrayTraversalCheck.h
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-array-traversal.rst
  
clang-tools-extra/test/clang-tidy/checkers/performance-inefficient-array-traversal.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-inefficient-array-traversal.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-inefficient-array-traversal.cpp
@@ -0,0 +1,107 @@
+// RUN: %check_clang_tidy %s performance-inefficient-array-traversal %t
+
+constexpr unsigned Rows = 10U;
+constexpr unsigned Cols = 16U;
+
+int Arr[Rows][Cols];
+int *PtrArr[Cols];
+int **Ptr;
+int FlatArray[Rows * Cols];
+
+void foo();
+
+void warned() {
+  for (unsigned I = 0; I < Cols; ++I) {
+for (unsigned J = 0; J < Rows; ++J) {
+  Arr[J][I]++;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Nonsequential array traversal can harm performance
+  // CHECK-MESSAGES: :[[@LINE-3]]:5: note: Row index 'J' incremented in this loop
+  // CHECK-MESSAGES: :[[@LINE-5]]:3: note: Column index 'I' incremented in this loop
+}
+  }
+  for (unsigned I = 0; I < Cols; ++I)
+for (unsigned J = 0; J < Rows; ++J)
+  Arr[J][I]++;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Nonsequential array traversal can harm performance
+
+  // Ensure it works on array access that use pointers
+  for (unsigned I = 0; I < Cols; ++I)
+for (unsigned J = 0; J < Rows; ++J)
+  PtrArr[J][I]++;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Nonsequential array traversal can harm performance
+
+  for (unsigned I = 0; I < Cols; ++I)
+for (unsigned J = 0; J < Rows; ++J)
+  Ptr[J][I]++;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Nonsequential array traversal can harm performance
+
+  // Detect += 1 as the loop incriment
+  for (unsigned I = 0; I < Cols; I += 1)
+for (unsigned J = 0; J < Rows; J += 1)
+  Arr[J][I]++;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Nonsequential array traversal can harm performance
+
+  // Still warn on this as calls inside the inner loop shouldn't complicate a fix.
+  for (unsigned I = 0; I < Cols; ++I) {
+for (unsigned J = 0; J < Rows; ++J) {
+  foo();
+  Arr[J][I]++;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Nonsequential array traversal can harm performance
+  foo();
+}
+  }
+  for (unsigned I = 0; I < Cols; ++I) {
+for (unsigned J = 0; J < Rows; ++J) {
+  FlatArray[J * Cols + I]++;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: Nonsequential array traversal can harm performance
+  // CHECK-MESSAGES: :[[@LINE-3]]:5: note: Row index 'J' incremented in this loop
+  // CHECK-MESSAGES: :[[@LINE-5]]:3: note: Column index 'I' incremented in this loop
+}
+  }
+}
+
+void ignored() {
+  // Correct traversal.
+  for (unsigned I = 0; I < Rows; ++I) {
+for (unsigned J = 0; J < Cols; ++J) {
+  Arr[I][J]++;
+}
+  }
+  for (unsigned I = 0; I < Rows; ++I) {
+for (unsigned J = 0; J < Cols; ++J) {
+  PtrArr[I][J]++;
+}
+  }
+  for (unsigned I = 0; I < Rows; ++I) {
+for (unsigned J = 0; J < Cols; ++J) {
+  FlatArray[I * Cols + J]++;
+}
+  }
+  // Don'w warn on these cases as extra code inside the outer loop could complicate a fix.
+  for (unsigned I = 0; I < Cols; ++I) {
+foo();
+for (unsigned J = 0; J < Rows; ++J) {
+  Arr[J][I]++;
+}
+  }
+  for (unsigned I = 0; I < Cols; ++I) {
+for (unsigned J = 0; J < Rows; ++J) {
+  Arr[J][I]++;
+}
+foo();
+  }
+
+  // Nested loop increments incorrect variable, don't warn on the traversal.
+  for (unsigned I = 0; I < Cols; ++I)
+for (unsigned J = 0; J < Rows; ++I)
+  Arr[J][I]++;
+
+  // These 2 loops are questionable and definitely a memory safety bug,
+  // but this is not the point of this check.
+  for (unsigned I = 0; I < Cols; ++I)
+for (unsigned J = 0; J < Rows; ++I)
+  FlatArray[I * Cols + J]++;
+  for (unsigned I = 0; I < Rows; ++I)
+for (unsigned J = 0; J < Cols; ++I)
+  FlatArray[J * Rows + I]++;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-array-traversal.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/performance-inefficient-array-traversal.rst
@@ -0,0 +1,24 @@
+.. title:: cl

[PATCH] D117391: [AST] Ignore implicit nodes in CastExpr::getConversionFunction

2022-01-15 Thread Kim Gräsman via Phabricator via cfe-commits
kimgr added a comment.

Oh yeah, this closes bug https://github.com/llvm/llvm-project/issues/53044. Do 
I mention that in the commit message/patch summary?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117391

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


[PATCH] D117306: [clang-tidy] Add new check 'shared-ptr-array-mismatch'.

2022-01-15 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

How does this check play with the `modernize-make-shared` check?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117306

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


[PATCH] D95046: [clangd] Add option to use dirty file contents when building preambles.

2022-01-15 Thread Nathan James via Phabricator via cfe-commits
njames93 updated this revision to Diff 400267.
njames93 added a comment.
Herald added a project: clang-tools-extra.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95046

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -501,6 +501,12 @@
 init(true),
 };
 
+opt UseDirtyPreambles{"use-dirty-preambles", cat(Misc),
+desc("Use files open in the editor when building "
+ "pre-ambles instead of reading from the 
disk"),
+Hidden,
+init(ClangdServer::Options().UseDirtyPreambles)};
+
 #if defined(__GLIBC__) && CLANGD_MALLOC_TRIM
 opt EnableMallocTrim{
 "malloc-trim",
@@ -938,6 +944,7 @@
 ClangTidyOptProvider = combine(std::move(Providers));
 Opts.ClangTidyProvider = ClangTidyOptProvider;
   }
+  Opts.UseDirtyPreambles = UseDirtyPreambles;
   Opts.QueryDriverGlobs = std::move(QueryDriverGlobs);
   Opts.TweakFilter = [&](const Tweak &T) {
 if (T.hidden() && !HiddenFeatures)
Index: clang-tools-extra/clangd/ClangdServer.h
===
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -165,6 +165,8 @@
 bool FoldingRanges = false;
 
 FeatureModuleSet *FeatureModules = nullptr;
+/// If true, use the dirty buffer contents when building Preambles.
+bool UseDirtyPreambles = false;
 
 explicit operator TUScheduler::Options() const;
   };
@@ -390,6 +392,9 @@
 private:
   FeatureModuleSet *FeatureModules;
   const GlobalCompilationDatabase &CDB;
+  const ThreadsafeFS &getPreambleFS() const {
+return UseDirtyPreambles ? *DirtyFS : TFS;
+  }
   const ThreadsafeFS &TFS;
 
   Path ResourceDir;
@@ -409,6 +414,8 @@
   // When set, provides clang-tidy options for a specific file.
   TidyProviderRef ClangTidyProvider;
 
+  bool UseDirtyPreambles = false;
+
   // GUARDED_BY(CachedCompletionFuzzyFindRequestMutex)
   llvm::StringMap>
   CachedCompletionFuzzyFindRequestByFile;
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -156,6 +156,7 @@
 : FeatureModules(Opts.FeatureModules), CDB(CDB), TFS(TFS),
   DynamicIdx(Opts.BuildDynamicSymbolIndex ? new FileIndex() : nullptr),
   ClangTidyProvider(Opts.ClangTidyProvider),
+  UseDirtyPreambles(Opts.UseDirtyPreambles),
   WorkspaceRoot(Opts.WorkspaceRoot),
   Transient(Opts.ImplicitCancellation ? TUScheduler::InvalidateOnUpdate
   : TUScheduler::NoInvalidation),
@@ -228,7 +229,7 @@
 
   // Compile command is set asynchronously during update, as it can be slow.
   ParseInputs Inputs;
-  Inputs.TFS = &TFS;
+  Inputs.TFS = &getPreambleFS();
   Inputs.Contents = std::string(Contents);
   Inputs.Version = std::move(ActualVersion);
   Inputs.ForceRebuild = ForceRebuild;
@@ -368,7 +369,7 @@
 SpecFuzzyFind->CachedReq = 
CachedCompletionFuzzyFindRequestByFile[File];
   }
 }
-ParseInputs ParseInput{IP->Command, &TFS, IP->Contents.str()};
+ParseInputs ParseInput{IP->Command, &getPreambleFS(), IP->Contents.str()};
 ParseInput.Index = Index;
 
 CodeCompleteOpts.MainFileSignals = IP->Signals;
@@ -416,7 +417,7 @@
 if (!PreambleData)
   return CB(error("Failed to parse includes"));
 
-ParseInputs ParseInput{IP->Command, &TFS, IP->Contents.str()};
+ParseInputs ParseInput{IP->Command, &getPreambleFS(), IP->Contents.str()};
 ParseInput.Index = Index;
 CB(clangd::signatureHelp(File, Pos, *PreambleData, ParseInput,
  DocumentationFormat));


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -501,6 +501,12 @@
 init(true),
 };
 
+opt UseDirtyPreambles{"use-dirty-preambles", cat(Misc),
+desc("Use files open in the editor when building "
+ "pre-ambles instead of reading from the disk"),
+Hidden,
+init(ClangdServer::Options().UseDirtyPreambles)};
+
 #if defined(__GLIBC__) && CLANGD_MALLOC_TRIM
 opt EnableMallocTrim{
 "malloc-trim",
@@ -938,6 +944,7 @@
 ClangTidyOptProvider = combine(std::move(Providers));
 Opts.ClangTidyProvider = ClangTidyOptProvider;
   }
+  Opts.U

[PATCH] D95046: [clangd] Add option to use dirty file contents when building preambles.

2022-01-15 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

@sammccall What else is needed for this to go in?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95046

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


[PATCH] D100092: [clang-tidy] cppcoreguidelines-declare-loop-variable-in-the-initializer: a new check

2022-01-15 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.
Herald added a subscriber: carlosgalvezp.

Sorry its been so long, How does this handle cases where the variable contains 
resources. Like a vector being used in a loop where you don't want to release 
its memory between each iteration.

  void foo(){
std::vector Buffer;
for (int i = 0; i < limit;++i) {
  Buffer.clear();
  // Do something with buffer
}
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100092

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


[PATCH] D117205: [clang-tidy] Support custom fix hint for cppcoreguidelines-pro-bounds-constant-array-index

2022-01-15 Thread Nathan James via Phabricator via cfe-commits
njames93 added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp:26
+utils::IncludeSorter::IS_LLVM)),
+  FixHint(Options.get("FixHint", "gsl::at()")) {}
 

Should elide the braces here for the option and insert them at the warning 
stage.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp:79-82
 auto Diag = diag(Matched->getExprLoc(),
- "do not use array subscript when the index is "
- "not an integer constant expression; use gsl::at() "
- "instead");
-if (!GslHeader.empty()) {
+ std::string("do not use array subscript when the index is 
"
+ "not an integer constant expression; use ") +
+ FixHint + " instead");





Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp:84
+
+if (!GslHeader.empty() && (FixHint == "gsl::at()")) {
   Diag << FixItHint::CreateInsertion(BaseRange.getBegin(), "gsl::at(")

This restriction on only offering a fix if using `gsl::at` seem a little too 
restrictive, especially when you factor that the GslHeader is defaulted to an 
empty string and that is already required for a fix to be emitted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117205

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


[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2022-01-15 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 400280.
JonasToth marked 10 inline comments as done.
JonasToth added a comment.

- Merge branch 'main' into feature_rebase_const_transform_20210808
- fix: fixes for most review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-const-correctness.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-cxx17.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-pointer-as-values.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-templates.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-pointer-as-values.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-values.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-unaligned.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp
@@ -0,0 +1,953 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-const-correctness %t -- \
+// RUN:   -config="{CheckOptions: [\
+// RUN:   {key: 'cppcoreguidelines-const-correctness.TransformValues', value: 1}, \
+// RUN:   {key: 'cppcoreguidelines-const-correctness.WarnPointersAsValues', value: 0}, \
+// RUN:   {key: 'cppcoreguidelines-const-correctness.TransformPointersAsValues', value: 0}, \
+// RUN:   ]}" -- -fno-delayed-template-parsing
+
+// --- Provide test samples for primitive builtins -
+// - every 'p_*' variable is a 'potential_const_*' variable
+// - every 'np_*' variable is a 'non_potential_const_*' variable
+
+bool global;
+char np_global = 0; // globals can't be known to be const
+
+namespace foo {
+int scoped;
+float np_scoped = 1; // namespace variables are like globals
+} // namespace foo
+
+// Lambdas should be ignored, because they do not follow the normal variable
+// semantic (e.g. the type is only known to the compiler).
+void lambdas() {
+  auto Lambda = [](int i) { return i < 0; };
+}
+
+void some_function(double, wchar_t);
+
+void some_function(double np_arg0, wchar_t np_arg1) {
+  int p_local0 = 2;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared 'const'
+
+  int np_local0;
+  const int np_local1 = 42;
+
+  unsigned int np_local2 = 3;
+  np_local2 <<= 4;
+
+  int np_local3 = 4;
+  ++np_local3;
+  int np_local4 = 4;
+  np_local4++;
+
+  int np_local5 = 4;
+  --np_local5;
+  int np_local6 = 4;
+  np_local6--;
+}
+
+void nested_scopes() {
+  int p_local0 = 2;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared 'const'
+  int np_local0 = 42;
+
+  {
+int p_local1 = 42;
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: variable 'p_local1' of type 'int' can be declared 'const'
+np_local0 *= 2;
+  }
+}
+
+void ignore_reference_to_pointers() {
+  int *np_local0 = nullptr;
+  int *&np_local1 = np_local0;
+}
+
+void some_lambda_environment_capture_all_by_reference(double np_arg0) {
+  int np_local0 = 0;
+  int p_local0 = 1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared 'const'
+
+  int np_local2;
+  const int np_local3 = 2;
+
+  // Capturing all variables by reference prohibits making them const.
+  [&]() { ++np_local0; };
+
+  int p_local1 = 0;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared 'const'
+}
+
+void some_lambda_environment_capture_all_by_value(double np_arg0) {
+  int np_local0 = 0;
+  int p_local0 = 1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared 'const'
+
+  int np_local1;
+  const int np_local2 = 2;
+
+  // Capturing by value has no influence on them.
+  [=]() { (void)p_local0; };
+
+  np_local0 += 10;
+}
+
+void function_inout_pointer(int *inout);
+void function_in_pointer(const int *in);
+
+void some_pointer_taking(int *out) {
+  int np_local0 = 42;
+  const int *const p0_np_local0 = &np_local0;
+  int *const p1_np_local0 = &np_local0;
+
+  int np_local1 = 42;
+  const int *const p0_np_local1 = &np_local1;
+  int *const p1_np_local1 = &np

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2022-01-15 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.h:31
+WarnPointersAsValues(Options.get("WarnPointersAsValues", 0)),
+TransformValues(Options.get("TransformValues", 1)),
+TransformReferences(Options.get("TransformReferences", 1)),

aaron.ballman wrote:
> JonasToth wrote:
> > aaron.ballman wrote:
> > > JonasToth wrote:
> > > > tiagoma wrote:
> > > > > JonasToth wrote:
> > > > > > Deactivating the transformations by default, as they are not ready 
> > > > > > for production yet.
> > > > > Ok. This got me off-guard. I would rather have this on.
> > > > the transformations were buggy and required a lot of fix-up. This might 
> > > > not be the case, but i would rather have it non-destructive by default 
> > > > because i think many people will throw this check at their code-base.
> > > > 
> > > > if the transformations are solid already we can activate it again (or 
> > > > maybe references only or so).
> > > Are we still intending to be conservative here? This defaults some of the 
> > > Transform to 1 instead of 0, but that's different than what the 
> > > documentation currently reads.
> > good question. given the current result of it being quiet solid we might 
> > have it on? But I am fine with it being off.
> > I am not sure if it would induce too much transformations to handle. I am 
> > unsure.
> Based on the test coverage we have, I think I'd be fine with it being on by 
> default, but I do think the most conservative approach would be to be off by 
> default. I don't have a strong opinion either way aside from "the 
> documentation should match the code."
I matched the documentation to that its on by default. It is kinda what the 
check promises, so I think having it off is not user-friendly.
I removed the `Experimental` note for it as well. It seems to be "good enough" 
that it does not interfere with everyday programming and even complex things.
The different validation runs seem to agree somewhat (even though there are 
still issues).



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-values.cpp:40
+void define_locals(T np_arg0, T &np_arg1, int np_arg2) {
+  T np_local0 = 0;
+  int p_local1 = 42;

aaron.ballman wrote:
> JonasToth wrote:
> > aaron.ballman wrote:
> > > Are we being conservative here about the diagnostic? It seems like the 
> > > instantiations in the TU would mean that this could be declared `const`.
> > Yes, the check ignores templates when matching. The reasoning behind this 
> > was: too much at once. TU-local templates should be transformable, but it 
> > must be ensured that all instantiations can be `const`. Probably the best 
> > solution would be that the template uses concepts and the concepts allow 
> > `const` at this place.
> > This is hopefully follow-up :)
> Okay, that sounds good to me!
Yes we are conservative. Once its a templated variable the analysis bails out. 
Non-templated variables in a template are analyzed though.

The 'check all instantiations' are not implemented, but there is already 
something that goes a little bit in that direction.
Line 118: `TemplateDiagnosticsCache.contains(Variable->getBeginLoc()))`, which 
is necessary to deduplicate emitted diagnostics in templates.
Similarly, TU-local templates could be checked all-together and if the variable 
could be `const` every single time, the diagnostic can be emitted.

But I feel that checking the concepts would be a much better approach to this. 
This would include the non-local templates as well. At the same time, most 
templates don't use them yet.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-values.cpp:114
+
+void casts() {
+  decltype(sizeof(void *)) p_local0 = 42;

aaron.ballman wrote:
> The test really doesn't have anything to do with casts, does it?
Thats correct :D Renamed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943

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


[PATCH] D112408: [RISCV] Add the zve extension according to the v1.0 spec

2022-01-15 Thread Yueh-Ting Chen via Phabricator via cfe-commits
eopXD marked 7 inline comments as done.
eopXD added inline comments.



Comment at: clang/lib/Basic/Targets/RISCV.cpp:184
 Builder.defineMacro("__riscv_v_min_vlen", Twine(MinVLen));
+Builder.defineMacro("__riscv_v_max_eew", Twine(MaxEew));
+Builder.defineMacro("__riscv_v_max_eew_fp", Twine(MaxEewFp));

craig.topper wrote:
> khchen wrote:
> > craig.topper wrote:
> > > Would't ELEN be the correct term here? Not EEW.
> > https://github.com/riscv/riscv-v-spec/blob/master/v-spec.adoc#182-zve-vector-extensions-for-embedded-processors
> >  shows zve* extensions have `Supported EEW`, I guess it's why the term is 
> > `EEW`.
> > 
> > 
> Is that because that section talks about them as a set of values rather than 
> a single maximum?
I think Zve is restricting the EEW, not ELEN.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112408

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


[PATCH] D117398: [clang-format] Fix bug in parsing `operator<` with template

2022-01-15 Thread Jino Park via Phabricator via cfe-commits
pjessesco created this revision.
pjessesco requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

https://github.com/llvm/llvm-project/issues/44601

This patch handles a bug when parsing a below example code :

  template  class S;
  
  template  bool operator<(S const &x, S const &y) {
return x.i < y.i;
  }
  
  template  class S {
int i = 42;
friend bool operator<<>(S const &, S const &);
  };
  
  int main() { return S{} < S{}; }

which parse `<<>` to `<< >`, not `< <>` in terms of tokens as discussed in 
discord.

1. Add a condition in `tryMergeLessLess()` considering `operator` keyword and 
`>`
2. Force to leave a whitespace between `tok::less` and a template opener
3. Add unit test


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117398

Files:
  clang/lib/Format/FormatTokenLexer.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9275,6 +9275,7 @@
   verifyFormat("operator SomeType();");
   verifyFormat("operator SomeType();");
   verifyFormat("operator SomeType>();");
+  verifyFormat("operator< <>();");
   verifyFormat("void *operator new(std::size_t size);");
   verifyFormat("void *operator new[](std::size_t size);");
   verifyFormat("void operator delete(void *ptr);");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3345,6 +3345,8 @@
 if (Right.is(tok::l_brace) && Right.is(BK_BracedInit) &&
 !Left.opensScope() && Style.SpaceBeforeCpp11BracedList)
   return true;
+if (Left.is(tok::less) && Right.is(TT_TemplateOpener))
+  return true;
   } else if (Style.Language == FormatStyle::LK_Proto ||
  Style.Language == FormatStyle::LK_TextProto) {
 if (Right.is(tok::period) &&
Index: clang/lib/Format/FormatTokenLexer.cpp
===
--- clang/lib/Format/FormatTokenLexer.cpp
+++ clang/lib/Format/FormatTokenLexer.cpp
@@ -429,9 +429,10 @@
   if (Tokens.size() < 3)
 return false;
 
+  auto Forth = (Tokens.end() - 4)[0];
   bool FourthTokenIsLess = false;
   if (Tokens.size() > 3)
-FourthTokenIsLess = (Tokens.end() - 4)[0]->is(tok::less);
+FourthTokenIsLess = Forth->is(tok::less);
 
   auto First = Tokens.end() - 3;
   if (First[2]->is(tok::less) || First[1]->isNot(tok::less) ||
@@ -443,6 +444,10 @@
   First[1]->WhitespaceRange.getEnd())
 return false;
 
+  // Do not remove a whitespace between the two "<" e.g. "operator<<>".
+  if (First[2]->is(tok::greater) && Forth->is(tok::kw_operator))
+return false;
+
   First[0]->Tok.setKind(tok::lessless);
   First[0]->TokenText = "<<";
   First[0]->ColumnWidth += 1;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9275,6 +9275,7 @@
   verifyFormat("operator SomeType();");
   verifyFormat("operator SomeType();");
   verifyFormat("operator SomeType>();");
+  verifyFormat("operator< <>();");
   verifyFormat("void *operator new(std::size_t size);");
   verifyFormat("void *operator new[](std::size_t size);");
   verifyFormat("void operator delete(void *ptr);");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3345,6 +3345,8 @@
 if (Right.is(tok::l_brace) && Right.is(BK_BracedInit) &&
 !Left.opensScope() && Style.SpaceBeforeCpp11BracedList)
   return true;
+if (Left.is(tok::less) && Right.is(TT_TemplateOpener))
+  return true;
   } else if (Style.Language == FormatStyle::LK_Proto ||
  Style.Language == FormatStyle::LK_TextProto) {
 if (Right.is(tok::period) &&
Index: clang/lib/Format/FormatTokenLexer.cpp
===
--- clang/lib/Format/FormatTokenLexer.cpp
+++ clang/lib/Format/FormatTokenLexer.cpp
@@ -429,9 +429,10 @@
   if (Tokens.size() < 3)
 return false;
 
+  auto Forth = (Tokens.end() - 4)[0];
   bool FourthTokenIsLess = false;
   if (Tokens.size() > 3)
-FourthTokenIsLess = (Tokens.end() - 4)[0]->is(tok::less);
+FourthTokenIsLess = Forth->is(tok::less);
 
   auto First = Tokens.end() - 3;
   if (First[2]->is(tok::less) || First[1]->isNot(tok::less) ||
@@ -443,6 +444,10 @@
   First[1]->WhitespaceRange.getEnd())
 return false;
 
+  // Do not remove a whitespace between the two "<" e.g. "operator<<>".
+  if (First[2]->is(tok::greater) && Forth->is(tok::kw_operator))
+return false;
+
   Firs

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2022-01-15 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 400288.
JonasToth marked 2 inline comments as done.
JonasToth added a comment.

- Merge branch 'main' into feature_rebase_const_transform_20210808
- fix: address other review comments
- remove one false positive with pointers in a range-based for loop on arrays


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-const-correctness.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-cxx17.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-pointer-as-values.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-templates.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-pointer-as-values.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-values.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-unaligned.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp
  clang/lib/Analysis/ExprMutationAnalyzer.cpp

Index: clang/lib/Analysis/ExprMutationAnalyzer.cpp
===
--- clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -445,14 +445,16 @@
   // array is considered modified if the loop-variable is a non-const reference.
   const auto DeclStmtToNonRefToArray = declStmt(hasSingleDecl(varDecl(hasType(
   hasUnqualifiedDesugaredType(referenceType(pointee(arrayType(;
-  const auto RefToArrayRefToElements = match(
-  findAll(stmt(cxxForRangeStmt(
-   hasLoopVariable(varDecl(hasType(nonConstReferenceType()))
-   .bind(NodeID::value)),
-   hasRangeStmt(DeclStmtToNonRefToArray),
-   hasRangeInit(canResolveToExpr(equalsNode(Exp)
-  .bind("stmt")),
-  Stm, Context);
+  const auto RefToArrayRefToElements =
+  match(findAll(stmt(cxxForRangeStmt(
+ hasLoopVariable(
+ varDecl(anyOf(hasType(nonConstReferenceType()),
+   hasType(nonConstPointerType(
+ .bind(NodeID::value)),
+ hasRangeStmt(DeclStmtToNonRefToArray),
+ hasRangeInit(canResolveToExpr(equalsNode(Exp)
+.bind("stmt")),
+Stm, Context);
 
   if (const auto *BadRangeInitFromArray =
   selectFirst("stmt", RefToArrayRefToElements))
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp
@@ -0,0 +1,958 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-const-correctness %t -- \
+// RUN:   -config="{CheckOptions: [\
+// RUN:   {key: 'cppcoreguidelines-const-correctness.TransformValues', value: 1}, \
+// RUN:   {key: 'cppcoreguidelines-const-correctness.WarnPointersAsValues', value: 0}, \
+// RUN:   {key: 'cppcoreguidelines-const-correctness.TransformPointersAsValues', value: 0}, \
+// RUN:   ]}" -- -fno-delayed-template-parsing
+
+// --- Provide test samples for primitive builtins -
+// - every 'p_*' variable is a 'potential_const_*' variable
+// - every 'np_*' variable is a 'non_potential_const_*' variable
+
+bool global;
+char np_global = 0; // globals can't be known to be const
+
+// FIXME: 'static' globals are not matched right now. They could be analyzed but aren't right now.
+static int p_static_global = 42;
+
+namespace foo {
+int scoped;
+float np_scoped = 1; // namespace variables are like globals
+} // namespace foo
+
+// FIXME: Similary to 'static' globals, anonymous globals are not matched and analyzed.
+namespace {
+int np_anonymous_global;
+int p_anonymous_global = 43;
+} // namespace
+
+// Lambdas should be ignored, because they do not follow the normal variable
+// semantic (e.g. the type is only known to the compiler).
+void lambdas() {
+  auto Lambda = [](int i) { return i < 0; };
+}
+
+void some_function(double, wchar_t);
+
+void some_function(double np_arg0, wchar_t np_arg1) {
+  int p_local0 = 2;
+  // CHECK-MES

[PATCH] D117398: [clang-format] Fix bug in parsing `operator<` with template

2022-01-15 Thread Jino Park via Phabricator via cfe-commits
pjessesco updated this revision to Diff 400290.
pjessesco added a comment.

Add deleted newline in `FormatTest.cpp` by mistake


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

https://reviews.llvm.org/D117398

Files:
  clang/lib/Format/FormatTokenLexer.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9275,6 +9275,8 @@
   verifyFormat("operator SomeType();");
   verifyFormat("operator SomeType();");
   verifyFormat("operator SomeType>();");
+  verifyFormat("operator< <>();");
+  
   verifyFormat("void *operator new(std::size_t size);");
   verifyFormat("void *operator new[](std::size_t size);");
   verifyFormat("void operator delete(void *ptr);");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3345,6 +3345,8 @@
 if (Right.is(tok::l_brace) && Right.is(BK_BracedInit) &&
 !Left.opensScope() && Style.SpaceBeforeCpp11BracedList)
   return true;
+if (Left.is(tok::less) && Right.is(TT_TemplateOpener))
+  return true;
   } else if (Style.Language == FormatStyle::LK_Proto ||
  Style.Language == FormatStyle::LK_TextProto) {
 if (Right.is(tok::period) &&
Index: clang/lib/Format/FormatTokenLexer.cpp
===
--- clang/lib/Format/FormatTokenLexer.cpp
+++ clang/lib/Format/FormatTokenLexer.cpp
@@ -429,9 +429,10 @@
   if (Tokens.size() < 3)
 return false;
 
+  auto Forth = (Tokens.end() - 4)[0];
   bool FourthTokenIsLess = false;
   if (Tokens.size() > 3)
-FourthTokenIsLess = (Tokens.end() - 4)[0]->is(tok::less);
+FourthTokenIsLess = Forth->is(tok::less);
 
   auto First = Tokens.end() - 3;
   if (First[2]->is(tok::less) || First[1]->isNot(tok::less) ||
@@ -443,6 +444,10 @@
   First[1]->WhitespaceRange.getEnd())
 return false;
 
+  // Do not remove a whitespace between the two "<" e.g. "operator<<>".
+  if (First[2]->is(tok::greater) && Forth->is(tok::kw_operator))
+return false;
+
   First[0]->Tok.setKind(tok::lessless);
   First[0]->TokenText = "<<";
   First[0]->ColumnWidth += 1;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9275,6 +9275,8 @@
   verifyFormat("operator SomeType();");
   verifyFormat("operator SomeType();");
   verifyFormat("operator SomeType>();");
+  verifyFormat("operator< <>();");
+  
   verifyFormat("void *operator new(std::size_t size);");
   verifyFormat("void *operator new[](std::size_t size);");
   verifyFormat("void operator delete(void *ptr);");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3345,6 +3345,8 @@
 if (Right.is(tok::l_brace) && Right.is(BK_BracedInit) &&
 !Left.opensScope() && Style.SpaceBeforeCpp11BracedList)
   return true;
+if (Left.is(tok::less) && Right.is(TT_TemplateOpener))
+  return true;
   } else if (Style.Language == FormatStyle::LK_Proto ||
  Style.Language == FormatStyle::LK_TextProto) {
 if (Right.is(tok::period) &&
Index: clang/lib/Format/FormatTokenLexer.cpp
===
--- clang/lib/Format/FormatTokenLexer.cpp
+++ clang/lib/Format/FormatTokenLexer.cpp
@@ -429,9 +429,10 @@
   if (Tokens.size() < 3)
 return false;
 
+  auto Forth = (Tokens.end() - 4)[0];
   bool FourthTokenIsLess = false;
   if (Tokens.size() > 3)
-FourthTokenIsLess = (Tokens.end() - 4)[0]->is(tok::less);
+FourthTokenIsLess = Forth->is(tok::less);
 
   auto First = Tokens.end() - 3;
   if (First[2]->is(tok::less) || First[1]->isNot(tok::less) ||
@@ -443,6 +444,10 @@
   First[1]->WhitespaceRange.getEnd())
 return false;
 
+  // Do not remove a whitespace between the two "<" e.g. "operator<<>".
+  if (First[2]->is(tok::greater) && Forth->is(tok::kw_operator))
+return false;
+
   First[0]->Tok.setKind(tok::lessless);
   First[0]->TokenText = "<<";
   First[0]->ColumnWidth += 1;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2022-01-15 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth marked an inline comment as done.
JonasToth added a comment.

addressed all outstanding review comments.




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-values.cpp:107-108
+  int *np_local3[2] = {&np_local0[0], &np_local0[1]};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'np_local3' of type 'int 
*[2]' can be declared 'const'
+  // CHECK-FIXES: const
+  for (int *non_const_ptr : np_local3) {

aaron.ballman wrote:
> This is a false positive. Making `np_local3` be `const` will break the code: 
> https://godbolt.org/z/EExKfsW4G
Thanks for spotting this!
The analyzer did not consider the `int * non_const_ptr: np_local3` to be a 
mutation, as its not a reference type.

```
  444   // The range variable is a reference to a builtin array. In that case 
the
1   // array is considered **modified if the loop-variable is a non-const 
reference**.
2   const auto DeclStmtToNonRefToArray = 
declStmt(hasSingleDecl(varDecl(hasType(
3   
hasUnqualifiedDesugaredType(referenceType(pointee(arrayType(;
4   const auto RefToArrayRefToElements = match(
5   findAll(stmt(cxxForRangeStmt(
6
hasLoopVariable(varDecl(hasType(nonConstReferenceType()))
7.bind(NodeID::value)),
8hasRangeStmt(DeclStmtToNonRefToArray),
9hasRangeInit(canResolveToExpr(equalsNode(Exp)
   10   .bind("stmt")),
   11   Stm, Context);
 ```

Added the fix to the patch, is that ok with you?



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp:17
+int scoped;
+float np_scoped = 1; // namespace variables are like globals
+} // namespace foo

aaron.ballman wrote:
> Similarly, showing anonymous namespace variables would help, as those are 
> like a static global and can also theoretically known to be const. (Note, I 
> don't insist on catching those cases! Just would like test coverage with some 
> comments on what to expect + docs if it seems important to tell users.)
Globals are not matched in any way. But there is 
`cppcoreguidelines-avoid-non-const-global-variables` which warns for non-const 
globals already.

The docs say `This check implements detection of local variables which could be 
declared as`, so its implicit that globals are not caught. I think that its 
enough. I will add a reference to 
`cppcoreguidelines-avoid-non-const-global-variables` though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943

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


[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2022-01-15 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 400291.
JonasToth marked an inline comment as done.
JonasToth added a comment.

- improve documentation a bit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-const-correctness.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-cxx17.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-pointer-as-values.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-templates.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-pointer-as-values.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-values.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-unaligned.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp
  clang/lib/Analysis/ExprMutationAnalyzer.cpp

Index: clang/lib/Analysis/ExprMutationAnalyzer.cpp
===
--- clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -445,14 +445,16 @@
   // array is considered modified if the loop-variable is a non-const reference.
   const auto DeclStmtToNonRefToArray = declStmt(hasSingleDecl(varDecl(hasType(
   hasUnqualifiedDesugaredType(referenceType(pointee(arrayType(;
-  const auto RefToArrayRefToElements = match(
-  findAll(stmt(cxxForRangeStmt(
-   hasLoopVariable(varDecl(hasType(nonConstReferenceType()))
-   .bind(NodeID::value)),
-   hasRangeStmt(DeclStmtToNonRefToArray),
-   hasRangeInit(canResolveToExpr(equalsNode(Exp)
-  .bind("stmt")),
-  Stm, Context);
+  const auto RefToArrayRefToElements =
+  match(findAll(stmt(cxxForRangeStmt(
+ hasLoopVariable(
+ varDecl(anyOf(hasType(nonConstReferenceType()),
+   hasType(nonConstPointerType(
+ .bind(NodeID::value)),
+ hasRangeStmt(DeclStmtToNonRefToArray),
+ hasRangeInit(canResolveToExpr(equalsNode(Exp)
+.bind("stmt")),
+Stm, Context);
 
   if (const auto *BadRangeInitFromArray =
   selectFirst("stmt", RefToArrayRefToElements))
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp
@@ -0,0 +1,958 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-const-correctness %t -- \
+// RUN:   -config="{CheckOptions: [\
+// RUN:   {key: 'cppcoreguidelines-const-correctness.TransformValues', value: 1}, \
+// RUN:   {key: 'cppcoreguidelines-const-correctness.WarnPointersAsValues', value: 0}, \
+// RUN:   {key: 'cppcoreguidelines-const-correctness.TransformPointersAsValues', value: 0}, \
+// RUN:   ]}" -- -fno-delayed-template-parsing
+
+// --- Provide test samples for primitive builtins -
+// - every 'p_*' variable is a 'potential_const_*' variable
+// - every 'np_*' variable is a 'non_potential_const_*' variable
+
+bool global;
+char np_global = 0; // globals can't be known to be const
+
+// FIXME: 'static' globals are not matched right now. They could be analyzed but aren't right now.
+static int p_static_global = 42;
+
+namespace foo {
+int scoped;
+float np_scoped = 1; // namespace variables are like globals
+} // namespace foo
+
+// FIXME: Similary to 'static' globals, anonymous globals are not matched and analyzed.
+namespace {
+int np_anonymous_global;
+int p_anonymous_global = 43;
+} // namespace
+
+// Lambdas should be ignored, because they do not follow the normal variable
+// semantic (e.g. the type is only known to the compiler).
+void lambdas() {
+  auto Lambda = [](int i) { return i < 0; };
+}
+
+void some_function(double, wchar_t);
+
+void some_function(double np_arg0, wchar_t np_arg1) {
+  int p_local0 = 2;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared 'const'
+
+  int np_local0;
+  const int np_local1 = 42;
+
+  unsigned 

[PATCH] D117362: [OpenMP] Remove hidden visibility for declare target variables

2022-01-15 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 updated this revision to Diff 400292.
jhuber6 added a comment.

Changing method to use an early exit, previously visibility would still be 
hidden for some constructs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117362

Files:
  clang/lib/AST/Decl.cpp
  clang/test/OpenMP/declare_target_codegen.cpp
  clang/test/OpenMP/declare_target_only_one_side_compilation.cpp


Index: clang/test/OpenMP/declare_target_only_one_side_compilation.cpp
===
--- clang/test/OpenMP/declare_target_only_one_side_compilation.cpp
+++ clang/test/OpenMP/declare_target_only_one_side_compilation.cpp
@@ -57,11 +57,11 @@
 
 // TODO: It is odd, probably wrong, that we don't mangle all variables.
 
-// DEVICE-DAG: @G1 = hidden {{.*}}global i32 0, align 4
+// DEVICE-DAG: @G1 = {{.*}}global i32 0, align 4
 // DEVICE-DAG: @_ZL2G2 = internal {{.*}}global i32 0, align 4
-// DEVICE-DAG: @G3 = hidden {{.*}}global i32 0, align 4
+// DEVICE-DAG: @G3 = {{.*}}global i32 0, align 4
 // DEVICE-DAG: @_ZL2G4 = internal {{.*}}global i32 0, align 4
-// DEVICE-DAG: @G5 = hidden {{.*}}global i32 0, align 4
+// DEVICE-DAG: @G5 = {{.*}}global i32 0, align 4
 // DEVICE-DAG: @_ZL2G6 = internal {{.*}}global i32 0, align 4
 // DEVICE-NOT: ref
 // DEVICE-NOT: llvm.used
Index: clang/test/OpenMP/declare_target_codegen.cpp
===
--- clang/test/OpenMP/declare_target_codegen.cpp
+++ clang/test/OpenMP/declare_target_codegen.cpp
@@ -26,25 +26,25 @@
 // CHECK-NOT: define {{.*}}{{baz1|baz4|maini1|Base|virtual_}}
 // CHECK-DAG: Bake
 // CHECK-NOT: @{{hhh|ggg|fff|eee}} =
-// CHECK-DAG: @flag = hidden global i8 undef,
+// CHECK-DAG: @flag = global i8 undef,
 // CHECK-DAG: @aaa = external global i32,
-// CHECK-DAG: @bbb ={{ hidden | }}global i32 0,
+// CHECK-DAG: @bbb = global i32 0,
 // CHECK-DAG: weak constant %struct.__tgt_offload_entry { i8* bitcast (i32* 
@bbb to i8*),
 // CHECK-DAG: @ccc = external global i32,
-// CHECK-DAG: @ddd ={{ hidden | }}global i32 0,
+// CHECK-DAG: @ddd = global i32 0,
 // CHECK-DAG: @hhh_decl_tgt_ref_ptr = weak global i32* null
 // CHECK-DAG: @ggg_decl_tgt_ref_ptr = weak global i32* null
 // CHECK-DAG: @fff_decl_tgt_ref_ptr = weak global i32* null
 // CHECK-DAG: @eee_decl_tgt_ref_ptr = weak global i32* null
 // CHECK-DAG: @{{.*}}maini1{{.*}}aaa = internal global i64 23,
 // CHECK-DAG: @pair = {{.*}}addrspace(3) global %struct.PAIR undef
-// CHECK-DAG: @b ={{ hidden | }}global i32 15,
-// CHECK-DAG: @d ={{ hidden | }}global i32 0,
+// CHECK-DAG: @b = global i32 15,
+// CHECK-DAG: @d = global i32 0,
 // CHECK-DAG: @c = external global i32,
-// CHECK-DAG: @globals ={{ hidden | }}global %struct.S zeroinitializer,
+// CHECK-DAG: @globals = global %struct.S zeroinitializer,
 // CHECK-DAG: [[STAT:@.+stat]] = internal global %struct.S zeroinitializer,
 // CHECK-DAG: [[STAT_REF:@.+]] = internal constant %struct.S* [[STAT]]
-// CHECK-DAG: @out_decl_target ={{ hidden | }}global i32 0,
+// CHECK-DAG: @out_decl_target = global i32 0,
 // CHECK-DAG: @llvm.used = appending global [2 x i8*] [i8* bitcast (void ()* 
@__omp_offloading__{{.+}}_globals_l[[@LINE+84]]_ctor to i8*), i8* bitcast (void 
()* @__omp_offloading__{{.+}}_stat_l[[@LINE+85]]_ctor to i8*)],
 // CHECK-DAG: @llvm.compiler.used = appending global [1 x i8*] [i8* bitcast 
(%struct.S** [[STAT_REF]] to i8*)],
 
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -786,6 +786,11 @@
 //
 // Note that we don't want to make the variable non-external
 // because of this, but unique-external linkage suits us.
+
+// We need variables inside declare target directives to be visible.
+if (OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(Var))
+  return LinkageInfo::external();
+
 if (Context.getLangOpts().CPlusPlus && !isFirstInExternCContext(Var) &&
 !IgnoreVarTypeLinkage) {
   LinkageInfo TypeLV = getLVForType(*Var->getType(), computation);


Index: clang/test/OpenMP/declare_target_only_one_side_compilation.cpp
===
--- clang/test/OpenMP/declare_target_only_one_side_compilation.cpp
+++ clang/test/OpenMP/declare_target_only_one_side_compilation.cpp
@@ -57,11 +57,11 @@
 
 // TODO: It is odd, probably wrong, that we don't mangle all variables.
 
-// DEVICE-DAG: @G1 = hidden {{.*}}global i32 0, align 4
+// DEVICE-DAG: @G1 = {{.*}}global i32 0, align 4
 // DEVICE-DAG: @_ZL2G2 = internal {{.*}}global i32 0, align 4
-// DEVICE-DAG: @G3 = hidden {{.*}}global i32 0, align 4
+// DEVICE-DAG: @G3 = {{.*}}global i32 0, align 4
 // DEVICE-DAG: @_ZL2G4 = internal {{.*}}global i32 0, align 4
-// DEVICE-DAG: @G5 = hidden {{.*}}global i32 0, align 4
+// DEVICE-DAG: @G5 = {{.*}}global i32 0, align 4
 // DEVI

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2022-01-15 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 400293.
JonasToth added a comment.

- fix release note ordering


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-const-correctness.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-cxx17.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-pointer-as-values.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-templates.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-pointer-as-values.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-values.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-unaligned.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp
  clang/lib/Analysis/ExprMutationAnalyzer.cpp

Index: clang/lib/Analysis/ExprMutationAnalyzer.cpp
===
--- clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -445,14 +445,16 @@
   // array is considered modified if the loop-variable is a non-const reference.
   const auto DeclStmtToNonRefToArray = declStmt(hasSingleDecl(varDecl(hasType(
   hasUnqualifiedDesugaredType(referenceType(pointee(arrayType(;
-  const auto RefToArrayRefToElements = match(
-  findAll(stmt(cxxForRangeStmt(
-   hasLoopVariable(varDecl(hasType(nonConstReferenceType()))
-   .bind(NodeID::value)),
-   hasRangeStmt(DeclStmtToNonRefToArray),
-   hasRangeInit(canResolveToExpr(equalsNode(Exp)
-  .bind("stmt")),
-  Stm, Context);
+  const auto RefToArrayRefToElements =
+  match(findAll(stmt(cxxForRangeStmt(
+ hasLoopVariable(
+ varDecl(anyOf(hasType(nonConstReferenceType()),
+   hasType(nonConstPointerType(
+ .bind(NodeID::value)),
+ hasRangeStmt(DeclStmtToNonRefToArray),
+ hasRangeInit(canResolveToExpr(equalsNode(Exp)
+.bind("stmt")),
+Stm, Context);
 
   if (const auto *BadRangeInitFromArray =
   selectFirst("stmt", RefToArrayRefToElements))
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp
@@ -0,0 +1,958 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-const-correctness %t -- \
+// RUN:   -config="{CheckOptions: [\
+// RUN:   {key: 'cppcoreguidelines-const-correctness.TransformValues', value: 1}, \
+// RUN:   {key: 'cppcoreguidelines-const-correctness.WarnPointersAsValues', value: 0}, \
+// RUN:   {key: 'cppcoreguidelines-const-correctness.TransformPointersAsValues', value: 0}, \
+// RUN:   ]}" -- -fno-delayed-template-parsing
+
+// --- Provide test samples for primitive builtins -
+// - every 'p_*' variable is a 'potential_const_*' variable
+// - every 'np_*' variable is a 'non_potential_const_*' variable
+
+bool global;
+char np_global = 0; // globals can't be known to be const
+
+// FIXME: 'static' globals are not matched right now. They could be analyzed but aren't right now.
+static int p_static_global = 42;
+
+namespace foo {
+int scoped;
+float np_scoped = 1; // namespace variables are like globals
+} // namespace foo
+
+// FIXME: Similary to 'static' globals, anonymous globals are not matched and analyzed.
+namespace {
+int np_anonymous_global;
+int p_anonymous_global = 43;
+} // namespace
+
+// Lambdas should be ignored, because they do not follow the normal variable
+// semantic (e.g. the type is only known to the compiler).
+void lambdas() {
+  auto Lambda = [](int i) { return i < 0; };
+}
+
+void some_function(double, wchar_t);
+
+void some_function(double np_arg0, wchar_t np_arg1) {
+  int p_local0 = 2;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared 'const'
+
+  int np_local0;
+  const int np_local1 = 42;
+
+  unsigned int np_local2 = 3;
+  np_local2 <<= 4;
+
+  in

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

2022-01-15 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth updated this revision to Diff 400296.
JonasToth added a comment.

- fix: adjust unit test for new array-pointer condition in range-based for loops


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-const-correctness.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-cxx17.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-pointer-as-values.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-templates.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-pointer-as-values.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-values.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-unaligned.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp
  clang/lib/Analysis/ExprMutationAnalyzer.cpp
  clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp

Index: clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
===
--- clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -1251,13 +1251,13 @@
   AST =
   buildASTFromCode("void f() { int* x[2]; for (int* e : x) e = nullptr; }");
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_FALSE(isMutated(Results, AST.get()));
+  EXPECT_TRUE(isMutated(Results, AST.get()));
 
   AST = buildASTFromCode(
   "typedef int* IntPtr;"
   "void f() { int* x[2]; for (IntPtr e : x) e = nullptr; }");
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_FALSE(isMutated(Results, AST.get()));
+  EXPECT_TRUE(isMutated(Results, AST.get()));
 }
 
 TEST(ExprMutationAnalyzerTest, RangeForArrayByConstRef) {
Index: clang/lib/Analysis/ExprMutationAnalyzer.cpp
===
--- clang/lib/Analysis/ExprMutationAnalyzer.cpp
+++ clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -445,14 +445,16 @@
   // array is considered modified if the loop-variable is a non-const reference.
   const auto DeclStmtToNonRefToArray = declStmt(hasSingleDecl(varDecl(hasType(
   hasUnqualifiedDesugaredType(referenceType(pointee(arrayType(;
-  const auto RefToArrayRefToElements = match(
-  findAll(stmt(cxxForRangeStmt(
-   hasLoopVariable(varDecl(hasType(nonConstReferenceType()))
-   .bind(NodeID::value)),
-   hasRangeStmt(DeclStmtToNonRefToArray),
-   hasRangeInit(canResolveToExpr(equalsNode(Exp)
-  .bind("stmt")),
-  Stm, Context);
+  const auto RefToArrayRefToElements =
+  match(findAll(stmt(cxxForRangeStmt(
+ hasLoopVariable(
+ varDecl(anyOf(hasType(nonConstReferenceType()),
+   hasType(nonConstPointerType(
+ .bind(NodeID::value)),
+ hasRangeStmt(DeclStmtToNonRefToArray),
+ hasRangeInit(canResolveToExpr(equalsNode(Exp)
+.bind("stmt")),
+Stm, Context);
 
   if (const auto *BadRangeInitFromArray =
   selectFirst("stmt", RefToArrayRefToElements))
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp
@@ -0,0 +1,958 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-const-correctness %t -- \
+// RUN:   -config="{CheckOptions: [\
+// RUN:   {key: 'cppcoreguidelines-const-correctness.TransformValues', value: 1}, \
+// RUN:   {key: 'cppcoreguidelines-const-correctness.WarnPointersAsValues', value: 0}, \
+// RUN:   {key: 'cppcoreguidelines-const-correctness.TransformPointersAsValues', value: 0}, \
+// RUN:   ]}" -- -fno-delayed-template-parsing
+
+// --- Provide test samples for primitive builtins -
+// - every 'p_*' variable is a 'potential_const_*' variable
+// - every 'np_*' variable is a 'non_potential_const_*' variable
+
+bool global;
+char np_glo

[PATCH] D116478: [clang-tidy] A comma-separated list of the names of functions or methods to be considered as not having side-effects

2022-01-15 Thread Zinovy Nis via Phabricator via cfe-commits
zinovy.nis updated this revision to Diff 400301.
zinovy.nis added a comment.

- Support regexp for list of ignored functions.


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

https://reviews.llvm.org/D116478

Files:
  clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-assert-side-effect.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-assert-side-effect.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-assert-side-effect.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-assert-side-effect.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-assert-side-effect.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s bugprone-assert-side-effect %t -- -config="{CheckOptions: [{key: bugprone-assert-side-effect.CheckFunctionCalls, value: true}, {key: bugprone-assert-side-effect.AssertMacros, value: 'assert,assert2,my_assert,convoluted_assert,msvc_assert'}]}" -- -fexceptions
+// RUN: %check_clang_tidy %s bugprone-assert-side-effect %t -- -config="{CheckOptions: [{key: bugprone-assert-side-effect.CheckFunctionCalls, value: true}, {key: bugprone-assert-side-effect.AssertMacros, value: 'assert,assert2,my_assert,convoluted_assert,msvc_assert'}, {key: bugprone-assert-side-effect.IgnoredFunctions, value: 'MyClass::badButIgnoredFunc'}]}" -- -fexceptions
 
 //===--- assert definition block --===//
 int abort() { return 0; }
@@ -43,9 +43,12 @@
 
 //===--===//
 
+bool badButIgnoredFunc(int a, int b) { return a * b > 0; }
+
 class MyClass {
 public:
   bool badFunc(int a, int b) { return a * b > 0; }
+  bool badButIgnoredFunc(int a, int b) { return a * b > 0; }
   bool goodFunc(int a, int b) const { return a * b > 0; }
 
   MyClass &operator=(const MyClass &rhs) { return *this; }
@@ -57,6 +60,11 @@
   void operator delete(void *p) {}
 };
 
+class SomeoneElseClass {
+public:
+  bool badButIgnoredFunc(int a, int b) { return a * b > 0; }
+};
+
 bool freeFunction() {
   return true;
 }
@@ -85,8 +93,16 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in assert() condition discarded in release builds
 
   MyClass mc;
+  SomeoneElseClass sec;
   assert(mc.badFunc(0, 1));
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in assert() condition discarded in release builds
+  assert(mc.badButIgnoredFunc(0, 1));
+  // badButIgnoredFunc is not ignored as only class members are ignored by the config
+  assert(badButIgnoredFunc(0, 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in assert() condition discarded in release builds
+  // sec.badButIgnoredFunc is not ignored as only MyClass members are ignored by the config
+  assert(sec.badButIgnoredFunc(0, 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in assert() condition discarded in release builds
   assert(mc.goodFunc(0, 1));
 
   MyClass mc2;
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-assert-side-effect.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone-assert-side-effect.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-assert-side-effect.rst
@@ -21,3 +21,13 @@
Whether to treat non-const member and non-member functions as they produce
side effects. Disabled by default because it can increase the number of false
positive warnings.
+
+.. option:: IgnoredFunctions
+
+   A semicolon-separated list of the names of functions or methods to be
+   considered as not having side-effects. Regular expressions are accepted,
+   e.g. `[Rr]ef(erence)?$` matches every type with suffix `Ref`, `ref`,
+   `Reference` and `reference`. The default is empty. If a name in the list
+   contains the sequence `::` it is matched against the qualified typename
+   (i.e. `namespace::Type`, otherwise it is matched against only
+   the type name (i.e. `Type`).
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -153,7 +153,12 @@
 Changes in existing checks
 ^^
 
+- :doc:`bugprone-assert-side-effect `
+  check now supports a ``IgnoredFunctions`` option to explicitly consider the specified
+  functions or methods as not any having side-effects.
+
 - Removed default setting ``cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors = "true"``,
+  from :doc:`cppcoreguidelines-explicit-virtual-functions `
   to match the current state of the C++ Core Guidelines.
 
 - Updated :doc:`google-readability-casting
@@ -165,10 +170,10 @@
 
 - Fixed a false positive in :doc:`bugprone-throw-keywor

[PATCH] D116478: [clang-tidy] A comma-separated list of the names of functions or methods to be considered as not having side-effects

2022-01-15 Thread Zinovy Nis via Phabricator via cfe-commits
zinovy.nis marked 4 inline comments as done.
zinovy.nis added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp:57-60
 if (const auto *FuncDecl = CExpr->getDirectCallee()) {
   if (FuncDecl->getDeclName().isIdentifier() &&
-  FuncDecl->getName() == "__builtin_expect") // exceptions come here
+  IgnoredFunctions.contains(
+  FuncDecl->getName())) // exceptions come here

aaron.ballman wrote:
> This doesn't seem quite right to me (test coverage would help) in the case 
> where the user is specifying a (potentially partially) qualified function 
> name. e.g., imagine an `IgnoredFunctions` list of 
> `my::fancy_func,::other_func,yet::another_func` where `my` is a namespace 
> containing a function named `fancy_func`, and `yet` is a class with a static 
> function named `another_func`. I think this code will only consider the name 
> of the function itself, but not any part of its qualified name.
> 
> I think we typically implement function name exclusions via the 
> `matchesAnyListedName()` AST matcher, as in: 
> https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp#L92
Thanks a lot!
Done.

BTW, now we have "," separator for macros and ";" for ignored functions. 
Doesn't sound reasonable. What do you think?



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-assert-side-effect.cpp:91
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in assert() 
condition discarded in release builds
+  assert(mc.badButIgnoredFunc(0, 1));
   assert(mc.goodFunc(0, 1));

aaron.ballman wrote:
> FWIW, I think this can be confusing in practice; the function called is 
> `::MyClass:badButIgnoredFunc()`. Further, if I had a global function named 
> `badButIgnoredFunc()` it would *also* be ignored and I'd have no way to 
> configure to distinguish between the two.
Added a new test case for it.


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

https://reviews.llvm.org/D116478

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


[PATCH] D116478: [clang-tidy] A semicolon-separated list of the names of functions or methods to be considered as not having side-effects

2022-01-15 Thread Zinovy Nis via Phabricator via cfe-commits
zinovy.nis updated this revision to Diff 400303.
zinovy.nis marked 2 inline comments as done.

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

https://reviews.llvm.org/D116478

Files:
  clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/AssertSideEffectCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-assert-side-effect.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-assert-side-effect.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-assert-side-effect.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-assert-side-effect.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-assert-side-effect.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s bugprone-assert-side-effect %t -- -config="{CheckOptions: [{key: bugprone-assert-side-effect.CheckFunctionCalls, value: true}, {key: bugprone-assert-side-effect.AssertMacros, value: 'assert,assert2,my_assert,convoluted_assert,msvc_assert'}]}" -- -fexceptions
+// RUN: %check_clang_tidy %s bugprone-assert-side-effect %t -- -config="{CheckOptions: [{key: bugprone-assert-side-effect.CheckFunctionCalls, value: true}, {key: bugprone-assert-side-effect.AssertMacros, value: 'assert,assert2,my_assert,convoluted_assert,msvc_assert'}, {key: bugprone-assert-side-effect.IgnoredFunctions, value: 'MyClass::badButIgnoredFunc'}]}" -- -fexceptions
 
 //===--- assert definition block --===//
 int abort() { return 0; }
@@ -43,9 +43,12 @@
 
 //===--===//
 
+bool badButIgnoredFunc(int a, int b) { return a * b > 0; }
+
 class MyClass {
 public:
   bool badFunc(int a, int b) { return a * b > 0; }
+  bool badButIgnoredFunc(int a, int b) { return a * b > 0; }
   bool goodFunc(int a, int b) const { return a * b > 0; }
 
   MyClass &operator=(const MyClass &rhs) { return *this; }
@@ -57,6 +60,11 @@
   void operator delete(void *p) {}
 };
 
+class SomeoneElseClass {
+public:
+  bool badButIgnoredFunc(int a, int b) { return a * b > 0; }
+};
+
 bool freeFunction() {
   return true;
 }
@@ -85,8 +93,16 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in assert() condition discarded in release builds
 
   MyClass mc;
+  SomeoneElseClass sec;
   assert(mc.badFunc(0, 1));
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in assert() condition discarded in release builds
+  assert(mc.badButIgnoredFunc(0, 1));
+  // badButIgnoredFunc is not ignored as only class members are ignored by the config
+  assert(badButIgnoredFunc(0, 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in assert() condition discarded in release builds
+  // sec.badButIgnoredFunc is not ignored as only MyClass members are ignored by the config
+  assert(sec.badButIgnoredFunc(0, 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: side effect in assert() condition discarded in release builds
   assert(mc.goodFunc(0, 1));
 
   MyClass mc2;
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-assert-side-effect.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone-assert-side-effect.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-assert-side-effect.rst
@@ -21,3 +21,13 @@
Whether to treat non-const member and non-member functions as they produce
side effects. Disabled by default because it can increase the number of false
positive warnings.
+
+.. option:: IgnoredFunctions
+
+   A semicolon-separated list of the names of functions or methods to be
+   considered as not having side-effects. Regular expressions are accepted,
+   e.g. `[Rr]ef(erence)?$` matches every type with suffix `Ref`, `ref`,
+   `Reference` and `reference`. The default is empty. If a name in the list
+   contains the sequence `::` it is matched against the qualified typename
+   (i.e. `namespace::Type`, otherwise it is matched against only
+   the type name (i.e. `Type`).
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -153,7 +153,12 @@
 Changes in existing checks
 ^^
 
+- :doc:`bugprone-assert-side-effect `
+  check now supports a ``IgnoredFunctions`` option to explicitly consider the specified
+  functions or methods as not any having side-effects.
+
 - Removed default setting ``cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors = "true"``,
+  from :doc:`cppcoreguidelines-explicit-virtual-functions `
   to match the current state of the C++ Core Guidelines.
 
 - Updated :doc:`google-readability-casting
@@ -165,10 +170,10 @@
 
 - Fixed a false positive in :doc:`bugprone-throw-keyword-missing
   ` when creating an e

[PATCH] D112408: [RISCV] Add the zve extension according to the v1.0 spec

2022-01-15 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added inline comments.



Comment at: clang/lib/Basic/Targets/RISCV.cpp:184
 Builder.defineMacro("__riscv_v_min_vlen", Twine(MinVLen));
+Builder.defineMacro("__riscv_v_max_eew", Twine(MaxEew));
+Builder.defineMacro("__riscv_v_max_eew_fp", Twine(MaxEewFp));

eopXD wrote:
> craig.topper wrote:
> > khchen wrote:
> > > craig.topper wrote:
> > > > Would't ELEN be the correct term here? Not EEW.
> > > https://github.com/riscv/riscv-v-spec/blob/master/v-spec.adoc#182-zve-vector-extensions-for-embedded-processors
> > >  shows zve* extensions have `Supported EEW`, I guess it's why the term is 
> > > `EEW`.
> > > 
> > > 
> > Is that because that section talks about them as a set of values rather 
> > than a single maximum?
> I think Zve is restricting the EEW, not ELEN.
The spec defines ELEN as "The maximum size in bits of a vector element that any 
operation can produce or consume" That sounds like maximum EEW to me.

This statement appears in section 3.4.2

"For standard vector extensions with
ELEN=32, fractional LMULs of 1/2 and 1/4 must be supported. For standard vector 
extensions with ELEN=64, fractional
LMULs of 1/2, 1/4, and 1/8 must be supported."

I take "standard vector extensions with ELEN=32" to mean Zve32x, Zve32f.

And "standard vector extensions with ELEN=64" to mean Zve64x, Zve64f, Zve64d, 
and V.

Am I interpreting that incorrectly?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112408

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


[PATCH] D112408: [RISCV] Add the zve extension according to the v1.0 spec

2022-01-15 Thread Yueh-Ting Chen via Phabricator via cfe-commits
eopXD marked an inline comment as not done.
eopXD added inline comments.



Comment at: clang/lib/Basic/Targets/RISCV.cpp:184
 Builder.defineMacro("__riscv_v_min_vlen", Twine(MinVLen));
+Builder.defineMacro("__riscv_v_max_eew", Twine(MaxEew));
+Builder.defineMacro("__riscv_v_max_eew_fp", Twine(MaxEewFp));

craig.topper wrote:
> eopXD wrote:
> > craig.topper wrote:
> > > khchen wrote:
> > > > craig.topper wrote:
> > > > > Would't ELEN be the correct term here? Not EEW.
> > > > https://github.com/riscv/riscv-v-spec/blob/master/v-spec.adoc#182-zve-vector-extensions-for-embedded-processors
> > > >  shows zve* extensions have `Supported EEW`, I guess it's why the term 
> > > > is `EEW`.
> > > > 
> > > > 
> > > Is that because that section talks about them as a set of values rather 
> > > than a single maximum?
> > I think Zve is restricting the EEW, not ELEN.
> The spec defines ELEN as "The maximum size in bits of a vector element that 
> any operation can produce or consume" That sounds like maximum EEW to me.
> 
> This statement appears in section 3.4.2
> 
> "For standard vector extensions with
> ELEN=32, fractional LMULs of 1/2 and 1/4 must be supported. For standard 
> vector extensions with ELEN=64, fractional
> LMULs of 1/2, 1/4, and 1/8 must be supported."
> 
> I take "standard vector extensions with ELEN=32" to mean Zve32x, Zve32f.
> 
> And "standard vector extensions with ELEN=64" to mean Zve64x, Zve64f, Zve64d, 
> and V.
> 
> Am I interpreting that incorrectly?
Yes I think you are correct. Maximum EEW is an alias of ELEN.

I see that the discussion in 
https://github.com/riscv-non-isa/riscv-c-api-doc/pull/21 hasn't conclude on 
whether changing the name into `elen` and `elen_fp`. 

Should this patch pend until conclusions are drawn?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112408

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


[clang] c84b8be - [AArch64] clang support for Armv8.8/9.3 MOPS

2022-01-15 Thread via cfe-commits

Author: Lucas Prates
Date: 2022-01-15T19:52:30Z
New Revision: c84b8be516bcc4d021ff804169d58a7b3104e050

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

LOG: [AArch64] clang support for Armv8.8/9.3 MOPS

This introduces clang command line support for the new Armv8.8-A and
Armv9.3-A instructions for standardising memcpy, memset and memmove
operations, which was previously introduced into LLVM in
https://reviews.llvm.org/D116157.

Patch by Lucas Prates, Tomas Matheson and Son Tuan Vu.

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

Added: 
clang/test/Driver/aarch64-mops.c

Modified: 
clang/lib/Basic/Targets/AArch64.cpp
clang/lib/Basic/Targets/AArch64.h
llvm/include/llvm/Support/AArch64TargetParser.def
llvm/include/llvm/Support/AArch64TargetParser.h
llvm/lib/Support/AArch64TargetParser.cpp
llvm/unittests/Support/TargetParserTest.cpp

Removed: 




diff  --git a/clang/lib/Basic/Targets/AArch64.cpp 
b/clang/lib/Basic/Targets/AArch64.cpp
index 4a2d2a8f7f04d..8e23cc4c421a5 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -544,6 +544,7 @@ bool 
AArch64TargetInfo::handleTargetFeatures(std::vector &Features,
   HasMatmulFP32 = false;
   HasLSE = false;
   HasHBC = false;
+  HasMOPS = false;
 
   ArchKind = llvm::AArch64::ArchKind::INVALID;
 

diff  --git a/clang/lib/Basic/Targets/AArch64.h 
b/clang/lib/Basic/Targets/AArch64.h
index aa1ee7708b0d9..b9e6e3214c44d 100644
--- a/clang/lib/Basic/Targets/AArch64.h
+++ b/clang/lib/Basic/Targets/AArch64.h
@@ -54,6 +54,7 @@ class LLVM_LIBRARY_VISIBILITY AArch64TargetInfo : public 
TargetInfo {
   bool HasLSE;
   bool HasFlagM;
   bool HasHBC;
+  bool HasMOPS;
 
   llvm::AArch64::ArchKind ArchKind;
 

diff  --git a/clang/test/Driver/aarch64-mops.c 
b/clang/test/Driver/aarch64-mops.c
new file mode 100644
index 0..4cd48143033d6
--- /dev/null
+++ b/clang/test/Driver/aarch64-mops.c
@@ -0,0 +1,6 @@
+// Test that target feature mops is implemented and available correctly
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.8-a+mops %s 
2>&1 | FileCheck %s
+// CHECK: "-target-feature" "+mops"
+
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.8-a+nomops %s 
2>&1 | FileCheck %s --check-prefix=NO_MOPS
+// NO_MOPS: "-target-feature" "-mops"

diff  --git a/llvm/include/llvm/Support/AArch64TargetParser.def 
b/llvm/include/llvm/Support/AArch64TargetParser.def
index 2b2b519a5c1b9..9d92d96e67a2d 100644
--- a/llvm/include/llvm/Support/AArch64TargetParser.def
+++ b/llvm/include/llvm/Support/AArch64TargetParser.def
@@ -145,6 +145,7 @@ AARCH64_ARCH_EXT_NAME("sme",  AArch64::AEK_SME, 
"+sme",
 AARCH64_ARCH_EXT_NAME("sme-f64",  AArch64::AEK_SMEF64,  "+sme-f64",
  "-sme-f64")
 AARCH64_ARCH_EXT_NAME("sme-i64",  AArch64::AEK_SMEI64,  "+sme-i64",
  "-sme-i64")
 AARCH64_ARCH_EXT_NAME("hbc",  AArch64::AEK_HBC, "+hbc",
  "-hbc")
+AARCH64_ARCH_EXT_NAME("mops", AArch64::AEK_MOPS,"+mops",   
  "-mops")
 #undef AARCH64_ARCH_EXT_NAME
 
 #ifndef AARCH64_CPU_NAME

diff  --git a/llvm/include/llvm/Support/AArch64TargetParser.h 
b/llvm/include/llvm/Support/AArch64TargetParser.h
index 21bca23c3041f..b998bb481d8da 100644
--- a/llvm/include/llvm/Support/AArch64TargetParser.h
+++ b/llvm/include/llvm/Support/AArch64TargetParser.h
@@ -70,6 +70,7 @@ enum ArchExtKind : uint64_t {
   AEK_SMEF64 =  1ULL << 38,
   AEK_SMEI64 =  1ULL << 39,
   AEK_HBC = 1ULL << 40,
+  AEK_MOPS =1ULL << 41,
 };
 
 enum class ArchKind {

diff  --git a/llvm/lib/Support/AArch64TargetParser.cpp 
b/llvm/lib/Support/AArch64TargetParser.cpp
index d4957d4359eb5..9f1c17104cf43 100644
--- a/llvm/lib/Support/AArch64TargetParser.cpp
+++ b/llvm/lib/Support/AArch64TargetParser.cpp
@@ -116,6 +116,8 @@ bool AArch64::getExtensionFeatures(uint64_t Extensions,
 Features.push_back("+sme-i64");
   if (Extensions & AArch64::AEK_HBC)
 Features.push_back("+hbc");
+  if (Extensions & AArch64::AEK_MOPS)
+Features.push_back("+mops");
 
   return true;
 }

diff  --git a/llvm/unittests/Support/TargetParserTest.cpp 
b/llvm/unittests/Support/TargetParserTest.cpp
index bbba9f41cc950..9e2a29a1dc0e0 100644
--- a/llvm/unittests/Support/TargetParserTest.cpp
+++ b/llvm/unittests/Support/TargetParserTest.cpp
@@ -1519,6 +1519,7 @@ TEST(TargetParserTest, AArch64ArchExtFeature) {
   {"sme-f64", "nosme-f64", "+sme-f64", "-sme-f64"},
   {"sme-i64", "nosme-i64", "+sme-i64", "-sme-i64"},
   {"hbc", "nohbc", "+hbc", "-hbc"},
+  {"mops", "nomops", "+mops", "-mops"},
   };
 
   for (unsigned i = 0; i < array_lengthof(ArchExt); i++) {



___
cfe-commits mailing list

[PATCH] D117271: [AArch64] clang support for Armv8.8/9.3 MOPS

2022-01-15 Thread Son Tuan Vu 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 rGc84b8be516bc: [AArch64] clang support for Armv8.8/9.3 MOPS 
(authored by pratlucas, committed by tyb0807).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117271

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/test/Driver/aarch64-mops.c
  llvm/include/llvm/Support/AArch64TargetParser.def
  llvm/include/llvm/Support/AArch64TargetParser.h
  llvm/lib/Support/AArch64TargetParser.cpp
  llvm/unittests/Support/TargetParserTest.cpp


Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -1519,6 +1519,7 @@
   {"sme-f64", "nosme-f64", "+sme-f64", "-sme-f64"},
   {"sme-i64", "nosme-i64", "+sme-i64", "-sme-i64"},
   {"hbc", "nohbc", "+hbc", "-hbc"},
+  {"mops", "nomops", "+mops", "-mops"},
   };
 
   for (unsigned i = 0; i < array_lengthof(ArchExt); i++) {
Index: llvm/lib/Support/AArch64TargetParser.cpp
===
--- llvm/lib/Support/AArch64TargetParser.cpp
+++ llvm/lib/Support/AArch64TargetParser.cpp
@@ -116,6 +116,8 @@
 Features.push_back("+sme-i64");
   if (Extensions & AArch64::AEK_HBC)
 Features.push_back("+hbc");
+  if (Extensions & AArch64::AEK_MOPS)
+Features.push_back("+mops");
 
   return true;
 }
Index: llvm/include/llvm/Support/AArch64TargetParser.h
===
--- llvm/include/llvm/Support/AArch64TargetParser.h
+++ llvm/include/llvm/Support/AArch64TargetParser.h
@@ -70,6 +70,7 @@
   AEK_SMEF64 =  1ULL << 38,
   AEK_SMEI64 =  1ULL << 39,
   AEK_HBC = 1ULL << 40,
+  AEK_MOPS =1ULL << 41,
 };
 
 enum class ArchKind {
Index: llvm/include/llvm/Support/AArch64TargetParser.def
===
--- llvm/include/llvm/Support/AArch64TargetParser.def
+++ llvm/include/llvm/Support/AArch64TargetParser.def
@@ -145,6 +145,7 @@
 AARCH64_ARCH_EXT_NAME("sme-f64",  AArch64::AEK_SMEF64,  "+sme-f64",
  "-sme-f64")
 AARCH64_ARCH_EXT_NAME("sme-i64",  AArch64::AEK_SMEI64,  "+sme-i64",
  "-sme-i64")
 AARCH64_ARCH_EXT_NAME("hbc",  AArch64::AEK_HBC, "+hbc",
  "-hbc")
+AARCH64_ARCH_EXT_NAME("mops", AArch64::AEK_MOPS,"+mops",   
  "-mops")
 #undef AARCH64_ARCH_EXT_NAME
 
 #ifndef AARCH64_CPU_NAME
Index: clang/test/Driver/aarch64-mops.c
===
--- /dev/null
+++ clang/test/Driver/aarch64-mops.c
@@ -0,0 +1,6 @@
+// Test that target feature mops is implemented and available correctly
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.8-a+mops %s 
2>&1 | FileCheck %s
+// CHECK: "-target-feature" "+mops"
+
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.8-a+nomops %s 
2>&1 | FileCheck %s --check-prefix=NO_MOPS
+// NO_MOPS: "-target-feature" "-mops"
Index: clang/lib/Basic/Targets/AArch64.h
===
--- clang/lib/Basic/Targets/AArch64.h
+++ clang/lib/Basic/Targets/AArch64.h
@@ -54,6 +54,7 @@
   bool HasLSE;
   bool HasFlagM;
   bool HasHBC;
+  bool HasMOPS;
 
   llvm::AArch64::ArchKind ArchKind;
 
Index: clang/lib/Basic/Targets/AArch64.cpp
===
--- clang/lib/Basic/Targets/AArch64.cpp
+++ clang/lib/Basic/Targets/AArch64.cpp
@@ -544,6 +544,7 @@
   HasMatmulFP32 = false;
   HasLSE = false;
   HasHBC = false;
+  HasMOPS = false;
 
   ArchKind = llvm::AArch64::ArchKind::INVALID;
 


Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -1519,6 +1519,7 @@
   {"sme-f64", "nosme-f64", "+sme-f64", "-sme-f64"},
   {"sme-i64", "nosme-i64", "+sme-i64", "-sme-i64"},
   {"hbc", "nohbc", "+hbc", "-hbc"},
+  {"mops", "nomops", "+mops", "-mops"},
   };
 
   for (unsigned i = 0; i < array_lengthof(ArchExt); i++) {
Index: llvm/lib/Support/AArch64TargetParser.cpp
===
--- llvm/lib/Support/AArch64TargetParser.cpp
+++ llvm/lib/Support/AArch64TargetParser.cpp
@@ -116,6 +116,8 @@
 Features.push_back("+sme-i64");
   if (Extensions & AArch64::AEK_HBC)
 Features.push_back("+hbc");
+  if (Extensions & AArch64::AEK_MOPS)
+Features.push_back("+mops");
 
   return true;
 }
Index: llvm/include/llvm/Support/AArch64TargetParser.h
===
--- llv

LLVM build master will be restarted shortly

2022-01-15 Thread Galina Kistanova via cfe-commits
 Hello,

LLVM build master will be restarted shortly.

Thanks

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


[clang] c63a317 - [AttrBuilder] Remove ctor accepting AttributeList and Index

2022-01-15 Thread Nikita Popov via cfe-commits

Author: Nikita Popov
Date: 2022-01-15T22:39:31+01:00
New Revision: c63a3175c2947e8c1a2d3bbe16a8586600705c54

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

LOG: [AttrBuilder] Remove ctor accepting AttributeList and Index

Use the AttributeSet constructor instead. There's no good reason
why AttrBuilder itself should exact the AttributeSet from the
AttributeList. Moving this out of the AttrBuilder generally results
in cleaner code.

Added: 


Modified: 
clang/lib/CodeGen/CodeGenModule.cpp
llvm/include/llvm/IR/Attributes.h
llvm/lib/CodeGen/Analysis.cpp
llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
llvm/lib/IR/Attributes.cpp
llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
llvm/lib/Transforms/Utils/CallPromotionUtils.cpp
llvm/lib/Transforms/Utils/InlineFunction.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index bd6fc49beda00..f710b9ad067d4 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -3792,7 +3792,7 @@ llvm::Constant *CodeGenModule::GetOrCreateLLVMFunction(
   if (D)
 SetFunctionAttributes(GD, F, IsIncompleteFunction, IsThunk);
   if (ExtraAttrs.hasFnAttrs()) {
-llvm::AttrBuilder B(F->getContext(), ExtraAttrs, 
llvm::AttributeList::FunctionIndex);
+llvm::AttrBuilder B(F->getContext(), ExtraAttrs.getFnAttrs());
 F->addFnAttrs(B);
   }
 

diff  --git a/llvm/include/llvm/IR/Attributes.h 
b/llvm/include/llvm/IR/Attributes.h
index d5fe55652179e..5f20b82a62cdd 100644
--- a/llvm/include/llvm/IR/Attributes.h
+++ b/llvm/include/llvm/IR/Attributes.h
@@ -1014,7 +1014,6 @@ class AttrBuilder {
 addAttribute(A);
   }
 
-  AttrBuilder(LLVMContext &Ctx, AttributeList AS, unsigned Idx);
   AttrBuilder(LLVMContext &Ctx, AttributeSet AS);
 
   void clear();

diff  --git a/llvm/lib/CodeGen/Analysis.cpp b/llvm/lib/CodeGen/Analysis.cpp
index 4c05d8ba33a9d..e8fef505e43d7 100644
--- a/llvm/lib/CodeGen/Analysis.cpp
+++ b/llvm/lib/CodeGen/Analysis.cpp
@@ -577,9 +577,9 @@ bool llvm::attributesPermitTailCall(const Function *F, 
const Instruction *I,
   bool &ADS = AllowDifferingSizes ? *AllowDifferingSizes : DummyADS;
   ADS = true;
 
-  AttrBuilder CallerAttrs(F->getContext(), F->getAttributes(), 
AttributeList::ReturnIndex);
-  AttrBuilder CalleeAttrs(F->getContext(), cast(I)->getAttributes(),
-  AttributeList::ReturnIndex);
+  AttrBuilder CallerAttrs(F->getContext(), F->getAttributes().getRetAttrs());
+  AttrBuilder CalleeAttrs(F->getContext(),
+  cast(I)->getAttributes().getRetAttrs());
 
   // Following attributes are completely benign as far as calling convention
   // goes, they shouldn't affect whether the call is a tail call.

diff  --git a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp 
b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
index e2bfc5bbe8215..c68b83b6ac6ed 100644
--- a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
@@ -564,7 +564,7 @@ static bool isLibCallInTailPosition(MachineInstr &MI,
   // the return. Ignore NoAlias and NonNull because they don't affect the
   // call sequence.
   AttributeList CallerAttrs = F.getAttributes();
-  if (AttrBuilder(F.getContext(), CallerAttrs, AttributeList::ReturnIndex)
+  if (AttrBuilder(F.getContext(), CallerAttrs.getRetAttrs())
   .removeAttribute(Attribute::NoAlias)
   .removeAttribute(Attribute::NonNull)
   .hasAttributes())

diff  --git a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp 
b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
index 40b422fd740dc..3e58eb93d9100 100644
--- a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
@@ -60,7 +60,7 @@ bool TargetLowering::isInTailCallPosition(SelectionDAG &DAG, 
SDNode *Node,
   // Conservatively require the attributes of the call to match those of
   // the return. Ignore following attributes because they don't affect the
   // call sequence.
-  AttrBuilder CallerAttrs(F.getContext(), F.getAttributes(), 
AttributeList::ReturnIndex);
+  AttrBuilder CallerAttrs(F.getContext(), F.getAttributes().getRetAttrs());
   for (const auto &Attr : {Attribute::Alignment, Attribute::Dereferenceable,
Attribute::DereferenceableOrNull, 
Attribute::NoAlias,
Attribute::NonNull})

diff  --git a/llvm/lib/IR/Attributes.cpp b/llvm/lib/IR/Attributes.cpp
index 542b47001160c..c680f9fc56153 100644
--- a/llvm/lib/IR/Attributes.cpp
+++ b/llvm/lib/IR/Attributes.cpp
@@ -1548,14 +1548,6 @@ LLVM_DUMP_METHOD void AttributeList::dump() const { 
print(dbgs()); }
 // AttrBuilder M

[PATCH] D111400: [Clang] Implement P2242R3

2022-01-15 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 400317.
cor3ntin added a comment.

Fix tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111400

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/basic/basic.types/p10.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/dtor.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3-2b.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/SemaCXX/constant-expression-cxx14.cpp
  clang/test/SemaCXX/constant-expression-cxx2b.cpp
  clang/test/SemaCXX/cxx1z-constexpr-lambdas.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1358,7 +1358,7 @@
 
   Non-literal variables (and labels and gotos) in constexpr functions
   https://wg21.link/P2242R3";>P2242R3
-  No
+  Clang 14
 
 
   Character encoding of diagnostic text
Index: clang/test/SemaCXX/cxx1z-constexpr-lambdas.cpp
===
--- clang/test/SemaCXX/cxx1z-constexpr-lambdas.cpp
+++ clang/test/SemaCXX/cxx1z-constexpr-lambdas.cpp
@@ -115,11 +115,6 @@
 constexpr const char *Str = "abc";
 static_assert(fp4(Str) == Str);
 
-auto NCL = [](int i) { static int j; return j; }; //expected-note{{declared here}}
-constexpr int (*fp5)(int) = NCL;
-constexpr int I =  //expected-error{{must be initialized by a constant expression}}
-  fp5(5); //expected-note{{non-constexpr function}} 
-
 namespace test_dont_always_instantiate_constexpr_templates {
 
 auto explicit_return_type = [](auto x) -> int { return x.get(); };
Index: clang/test/SemaCXX/constant-expression-cxx2b.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constant-expression-cxx2b.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -verify %s -fcxx-exceptions -triple=x86_64-linux-gnu
+
+constexpr int f(int n) {  // expected-error {{constexpr function never produces a constant expression}}
+  static const int m = n; //  expected-note {{declared here}}
+  return m;   //  expected-note {{initializer of 'm' is not a constant expression}}
+}
+constexpr int g(int n) {// expected-error {{constexpr function never produces a constant expression}}
+  thread_local const int m = n; //  expected-note {{declared here}}
+  return m; //  expected-note {{initializer of 'm' is not a constant expression}}
+}
+
+constexpr int h(int n) {
+  if (!n)
+return 0;
+  static const int m = n;
+  return m;
+}
+constexpr int i(int n) {
+  if (!n)
+return 0;
+  thread_local const int m = n;
+  return m;
+}
Index: clang/test/SemaCXX/constant-expression-cxx14.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx14.cpp
+++ clang/test/SemaCXX/constant-expression-cxx14.cpp
@@ -44,14 +44,6 @@
   return 3 * k3 + 5 * k2 + n * k - 20;
 }
 static_assert(g(2) == 42, "");
-constexpr int h(int n) {
-  static const int m = n; // expected-error {{static variable not permitted in a constexpr function}}
-  return m;
-}
-constexpr int i(int n) {
-  thread_local const int m = n; // expected-error {{thread_local variable not permitted in a constexpr function}}
-  return m;
-}
 
 // if-statements can be used in constexpr functions.
 constexpr int j(int k) {
@@ -65,7 +57,9 @@
   return 2;
 }
   }
-} // expected-note 2{{control reached end of constexpr function}}
+} // expected-warning {{non-void}} \
+  //expected-note 2{{control reached end of constexpr function}}
+
 static_assert(j(0) == -3, "");
 static_assert(j(1) == 5, "");
 static_assert(j(2), ""); // expected-error {{constant expression}} expected-note {{in call to 'j(2)'}}
Index: clang/test/Lexer/cxx-features.cpp
===
--- clang/test/Lexer/cxx-features.cpp
+++ clang/test/Lexer/cxx-features.cpp
@@ -277,7 +277,7 @@
 #error "wrong value for __cpp_lambdas"
 #endif
 
-#if check(constexpr, 0, 200704, 201304, 201603, 201907, 201907)
+#if check(constexpr, 0, 200704, 201304, 201603, 201907, 202110)
 #error "wrong value for __cpp_constexpr"
 #endif
 
Index: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
===
--- clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
+++ clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
@@ -1,6 +1,7 @@
-// RUN: %clang_cc1 -verify -fcxx-exceptions -triple=x86_64-linux-gnu -std=c++11 -Werror=c++14-extensions -Werror=c++20-extensions %s
-// RUN: %clang_cc1 -verify -fcxx-exceptions -triple=x86_64-linux-gnu -std=c++14 -DC

[PATCH] D111400: [Clang] Implement P2242R3

2022-01-15 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment.

In D111400#3243826 , @aaron.ballman 
wrote:

> In D111400#3172097 , @cor3ntin 
> wrote:
>
>> Regression compared to the status quo:
>> This code no longer warns (as noted by Hubert above)
>>
>>   auto f = [](bool b) {
>> if (b) return 42;
>> static int x = 0;
>> return x;
>>   };
>>   constexpr int x = f(true);
>>   const int *p = &x;
>>
>> GCC doesn't warn and... if we wanted to produce a warning there, I have no 
>> idea how to go about it.
>
> I think I found the issue in the code, but one thing that's strange is that 
> we don't seem to treat it as an extension but instead issue an error, but the 
> behavior is consistent with other things we handle as an extension there 
> (e.g., a local variable in C++14 mode).

Yes, that was a bug, but the code above cannot be diagnose.
at the time when the lambda f() is parsed, there is no indication that it must 
be usable in a constexpr context, and so the compiler doesn't emit a diagnostic.
It does when the call operator is marked explicitly `constexpr`

  auto f = [] (bool b) constexpr {
if (b) return 42;
static int x = 0;
return x;
  };
  constexpr int x = f(true);
  const int *p = &x;

This behavior is identical to GCC's https://compiler-explorer.com/z/xor3oYGMa


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111400

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


[PATCH] D117407: [clang] Add include path for cppwinrt on Windows SDK 10.0.17134+

2022-01-15 Thread Kagami Sascha Rosylight via Phabricator via cfe-commits
saschanaz created this revision.
saschanaz added a reviewer: hans.
saschanaz requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This fixes https://github.com/llvm/llvm-project/issues/53112 by adding cppwinrt 
to the include path when the SDK version is higher than 10.0.17134.0.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117407

Files:
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/test/Driver/cl-sysroot.cpp


Index: clang/test/Driver/cl-sysroot.cpp
===
--- clang/test/Driver/cl-sysroot.cpp
+++ clang/test/Driver/cl-sysroot.cpp
@@ -17,6 +17,7 @@
 // CHECK: "-internal-isystem" "[[ROOT]]{{/|}}Windows 
Kits{{/|}}10{{/|}}Include{{/|}}10.0.19041.0{{/|}}shared"
 // CHECK: "-internal-isystem" "[[ROOT]]{{/|}}Windows 
Kits{{/|}}10{{/|}}Include{{/|}}10.0.19041.0{{/|}}um"
 // CHECK: "-internal-isystem" "[[ROOT]]{{/|}}Windows 
Kits{{/|}}10{{/|}}Include{{/|}}10.0.19041.0{{/|}}winrt"
+// CHECK: "-internal-isystem" "[[ROOT]]{{/|}}Windows 
Kits{{/|}}10{{/|}}Include{{/|}}10.0.19041.0{{/|}}cppwinrt"
 
 // CHECK: "-libpath:[[ROOT]]{{/|}}DIA SDK{{/|}}lib{{/|}}amd64"
 // CHECK: 
"-libpath:[[ROOT]]{{/|}}VC{{/|}}Tools{{/|}}MSVC{{/|}}27.1828.18284{{/|}}lib{{/|}}x64"
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -1333,6 +1333,15 @@
 AddSystemIncludeWithSubfolder(DriverArgs, CC1Args, WindowsSDKDir,
   "Include", windowsSDKIncludeVersion,
   "winrt");
+if (major >= 10) {
+  llvm::VersionTuple Tuple;
+  if (!Tuple.tryParse(windowsSDKIncludeVersion) &&
+  Tuple.getSubminor().getValueOr(0) >= 17134) {
+AddSystemIncludeWithSubfolder(DriverArgs, CC1Args, WindowsSDKDir,
+  "Include", windowsSDKIncludeVersion,
+  "cppwinrt");
+  }
+}
   } else {
 AddSystemIncludeWithSubfolder(DriverArgs, CC1Args, WindowsSDKDir,
   "Include");


Index: clang/test/Driver/cl-sysroot.cpp
===
--- clang/test/Driver/cl-sysroot.cpp
+++ clang/test/Driver/cl-sysroot.cpp
@@ -17,6 +17,7 @@
 // CHECK: "-internal-isystem" "[[ROOT]]{{/|}}Windows Kits{{/|}}10{{/|}}Include{{/|}}10.0.19041.0{{/|}}shared"
 // CHECK: "-internal-isystem" "[[ROOT]]{{/|}}Windows Kits{{/|}}10{{/|}}Include{{/|}}10.0.19041.0{{/|}}um"
 // CHECK: "-internal-isystem" "[[ROOT]]{{/|}}Windows Kits{{/|}}10{{/|}}Include{{/|}}10.0.19041.0{{/|}}winrt"
+// CHECK: "-internal-isystem" "[[ROOT]]{{/|}}Windows Kits{{/|}}10{{/|}}Include{{/|}}10.0.19041.0{{/|}}cppwinrt"
 
 // CHECK: "-libpath:[[ROOT]]{{/|}}DIA SDK{{/|}}lib{{/|}}amd64"
 // CHECK: "-libpath:[[ROOT]]{{/|}}VC{{/|}}Tools{{/|}}MSVC{{/|}}27.1828.18284{{/|}}lib{{/|}}x64"
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -1333,6 +1333,15 @@
 AddSystemIncludeWithSubfolder(DriverArgs, CC1Args, WindowsSDKDir,
   "Include", windowsSDKIncludeVersion,
   "winrt");
+if (major >= 10) {
+  llvm::VersionTuple Tuple;
+  if (!Tuple.tryParse(windowsSDKIncludeVersion) &&
+  Tuple.getSubminor().getValueOr(0) >= 17134) {
+AddSystemIncludeWithSubfolder(DriverArgs, CC1Args, WindowsSDKDir,
+  "Include", windowsSDKIncludeVersion,
+  "cppwinrt");
+  }
+}
   } else {
 AddSystemIncludeWithSubfolder(DriverArgs, CC1Args, WindowsSDKDir,
   "Include");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

2022-01-15 Thread Nuno Lopes via Phabricator via cfe-commits
nlopes added a comment.

In D105169#3244624 , @nathanchance 
wrote:

> I can reduce all of these down for you and/or I can start an email thread 
> with the `objtool` maintainers to see if there is a way to fix or avoid these 
> warnings on the `objtool` side and include you in that discussion, if LLVM is 
> not really doing anything wrong here. I am by no means an expert in this area 
> and I don't want to delay this anymore but I want to avoid regressing our 
> builds, as `objtool` regularly helps us spot compiler bugs. Perhaps 
> @nickdesaulniers has some other thoughts?

LLVM is not doing anything wrong here. The issue is that once we have UB, LLVM 
is not required to lay out the functions nicely. For example, issue #53118 is 
just a "nice to fix", it's not a bug.
On the other hand, I understand that fixing `objtool` is likely impossible if 
we consider this UB thing as assembly doesn't have enough information to 
distinguish between a compiler bug and a UB case. I just don't know how 
frequent and/or useful these warnings are.

I don't think we should hold this patch anymore as it's not wrong and the 
side-effects are understood. We can and should try to reduce those kernel 
warnings to zero, but we cannot put all that burden on this patch's author.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105169

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


[PATCH] D111400: [Clang] Implement P2242R3

2022-01-15 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 400325.
cor3ntin added a comment.

run clang-format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111400

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/basic/basic.types/p10.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/dtor.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3-2b.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/SemaCXX/constant-expression-cxx14.cpp
  clang/test/SemaCXX/constant-expression-cxx2b.cpp
  clang/test/SemaCXX/cxx1z-constexpr-lambdas.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1358,7 +1358,7 @@
 
   Non-literal variables (and labels and gotos) in constexpr functions
   https://wg21.link/P2242R3";>P2242R3
-  No
+  Clang 14
 
 
   Character encoding of diagnostic text
Index: clang/test/SemaCXX/cxx1z-constexpr-lambdas.cpp
===
--- clang/test/SemaCXX/cxx1z-constexpr-lambdas.cpp
+++ clang/test/SemaCXX/cxx1z-constexpr-lambdas.cpp
@@ -115,11 +115,6 @@
 constexpr const char *Str = "abc";
 static_assert(fp4(Str) == Str);
 
-auto NCL = [](int i) { static int j; return j; }; //expected-note{{declared here}}
-constexpr int (*fp5)(int) = NCL;
-constexpr int I =  //expected-error{{must be initialized by a constant expression}}
-  fp5(5); //expected-note{{non-constexpr function}} 
-
 namespace test_dont_always_instantiate_constexpr_templates {
 
 auto explicit_return_type = [](auto x) -> int { return x.get(); };
Index: clang/test/SemaCXX/constant-expression-cxx2b.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constant-expression-cxx2b.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -std=c++2b -fsyntax-only -verify %s -fcxx-exceptions -triple=x86_64-linux-gnu
+
+constexpr int f(int n) {  // expected-error {{constexpr function never produces a constant expression}}
+  static const int m = n; //  expected-note {{declared here}}
+  return m;   //  expected-note {{initializer of 'm' is not a constant expression}}
+}
+constexpr int g(int n) {// expected-error {{constexpr function never produces a constant expression}}
+  thread_local const int m = n; //  expected-note {{declared here}}
+  return m; //  expected-note {{initializer of 'm' is not a constant expression}}
+}
+
+constexpr int h(int n) {
+  if (!n)
+return 0;
+  static const int m = n;
+  return m;
+}
+constexpr int i(int n) {
+  if (!n)
+return 0;
+  thread_local const int m = n;
+  return m;
+}
Index: clang/test/SemaCXX/constant-expression-cxx14.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx14.cpp
+++ clang/test/SemaCXX/constant-expression-cxx14.cpp
@@ -44,14 +44,6 @@
   return 3 * k3 + 5 * k2 + n * k - 20;
 }
 static_assert(g(2) == 42, "");
-constexpr int h(int n) {
-  static const int m = n; // expected-error {{static variable not permitted in a constexpr function}}
-  return m;
-}
-constexpr int i(int n) {
-  thread_local const int m = n; // expected-error {{thread_local variable not permitted in a constexpr function}}
-  return m;
-}
 
 // if-statements can be used in constexpr functions.
 constexpr int j(int k) {
@@ -65,7 +57,9 @@
   return 2;
 }
   }
-} // expected-note 2{{control reached end of constexpr function}}
+} // expected-warning {{non-void}} \
+  //expected-note 2{{control reached end of constexpr function}}
+
 static_assert(j(0) == -3, "");
 static_assert(j(1) == 5, "");
 static_assert(j(2), ""); // expected-error {{constant expression}} expected-note {{in call to 'j(2)'}}
Index: clang/test/Lexer/cxx-features.cpp
===
--- clang/test/Lexer/cxx-features.cpp
+++ clang/test/Lexer/cxx-features.cpp
@@ -277,7 +277,7 @@
 #error "wrong value for __cpp_lambdas"
 #endif
 
-#if check(constexpr, 0, 200704, 201304, 201603, 201907, 201907)
+#if check(constexpr, 0, 200704, 201304, 201603, 201907, 202110)
 #error "wrong value for __cpp_constexpr"
 #endif
 
Index: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
===
--- clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
+++ clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
@@ -1,6 +1,7 @@
-// RUN: %clang_cc1 -verify -fcxx-exceptions -triple=x86_64-linux-gnu -std=c++11 -Werror=c++14-extensions -Werror=c++20-extensions %s
-// RUN: %clang_cc1 -verify -fcxx-exceptions -triple=x86_64-linux-gnu -std=c

[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

2022-01-15 Thread Nathan Chancellor via Phabricator via cfe-commits
nathanchance added a comment.

In D105169#3246378 , @nlopes wrote:

> In D105169#3244624 , @nathanchance 
> wrote:
>
>> I can reduce all of these down for you and/or I can start an email thread 
>> with the `objtool` maintainers to see if there is a way to fix or avoid 
>> these warnings on the `objtool` side and include you in that discussion, if 
>> LLVM is not really doing anything wrong here. I am by no means an expert in 
>> this area and I don't want to delay this anymore but I want to avoid 
>> regressing our builds, as `objtool` regularly helps us spot compiler bugs. 
>> Perhaps @nickdesaulniers has some other thoughts?
>
> LLVM is not doing anything wrong here. The issue is that once we have UB, 
> LLVM is not required to lay out the functions nicely. For example, issue 
> #53118 is just a "nice to fix", it's not a bug.
> On the other hand, I understand that fixing `objtool` is likely impossible if 
> we consider this UB thing as assembly doesn't have enough information to 
> distinguish between a compiler bug and a UB case. I just don't know how 
> frequent and/or useful these warnings are.
>
> I don't think we should hold this patch anymore as it's not wrong and the 
> side-effects are understood. We can and should try to reduce those kernel 
> warnings to zero, but we cannot put all that burden on this patch's author.

Fair enough. We will just try to deal with this change on the kernel side in 
one way or another.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105169

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


[PATCH] D117362: [OpenMP] Remove hidden visibility for declare target variables

2022-01-15 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 updated this revision to Diff 400328.
jhuber6 added a comment.

Updating again, wasn't handling static variables declared in a record context
properly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117362

Files:
  clang/lib/AST/Decl.cpp
  clang/test/OpenMP/declare_target_codegen.cpp
  clang/test/OpenMP/declare_target_only_one_side_compilation.cpp


Index: clang/test/OpenMP/declare_target_only_one_side_compilation.cpp
===
--- clang/test/OpenMP/declare_target_only_one_side_compilation.cpp
+++ clang/test/OpenMP/declare_target_only_one_side_compilation.cpp
@@ -57,11 +57,11 @@
 
 // TODO: It is odd, probably wrong, that we don't mangle all variables.
 
-// DEVICE-DAG: @G1 = hidden {{.*}}global i32 0, align 4
+// DEVICE-DAG: @G1 = {{.*}}global i32 0, align 4
 // DEVICE-DAG: @_ZL2G2 = internal {{.*}}global i32 0, align 4
-// DEVICE-DAG: @G3 = hidden {{.*}}global i32 0, align 4
+// DEVICE-DAG: @G3 = {{.*}}global i32 0, align 4
 // DEVICE-DAG: @_ZL2G4 = internal {{.*}}global i32 0, align 4
-// DEVICE-DAG: @G5 = hidden {{.*}}global i32 0, align 4
+// DEVICE-DAG: @G5 = {{.*}}global i32 0, align 4
 // DEVICE-DAG: @_ZL2G6 = internal {{.*}}global i32 0, align 4
 // DEVICE-NOT: ref
 // DEVICE-NOT: llvm.used
Index: clang/test/OpenMP/declare_target_codegen.cpp
===
--- clang/test/OpenMP/declare_target_codegen.cpp
+++ clang/test/OpenMP/declare_target_codegen.cpp
@@ -26,25 +26,26 @@
 // CHECK-NOT: define {{.*}}{{baz1|baz4|maini1|Base|virtual_}}
 // CHECK-DAG: Bake
 // CHECK-NOT: @{{hhh|ggg|fff|eee}} =
-// CHECK-DAG: @flag = hidden global i8 undef,
+// CHECK-DAG: @flag = global i8 undef,
 // CHECK-DAG: @aaa = external global i32,
-// CHECK-DAG: @bbb ={{ hidden | }}global i32 0,
+// CHECK-DAG: @bbb = global i32 0,
 // CHECK-DAG: weak constant %struct.__tgt_offload_entry { i8* bitcast (i32* 
@bbb to i8*),
 // CHECK-DAG: @ccc = external global i32,
-// CHECK-DAG: @ddd ={{ hidden | }}global i32 0,
+// CHECK-DAG: @ddd = global i32 0,
 // CHECK-DAG: @hhh_decl_tgt_ref_ptr = weak global i32* null
 // CHECK-DAG: @ggg_decl_tgt_ref_ptr = weak global i32* null
 // CHECK-DAG: @fff_decl_tgt_ref_ptr = weak global i32* null
 // CHECK-DAG: @eee_decl_tgt_ref_ptr = weak global i32* null
 // CHECK-DAG: @{{.*}}maini1{{.*}}aaa = internal global i64 23,
 // CHECK-DAG: @pair = {{.*}}addrspace(3) global %struct.PAIR undef
-// CHECK-DAG: @b ={{ hidden | }}global i32 15,
-// CHECK-DAG: @d ={{ hidden | }}global i32 0,
+// CHECK-DAG: @_ZN2SS3SSSE = global i32 1,
+// CHECK-DAG: @b = global i32 15,
+// CHECK-DAG: @d = global i32 0,
 // CHECK-DAG: @c = external global i32,
-// CHECK-DAG: @globals ={{ hidden | }}global %struct.S zeroinitializer,
+// CHECK-DAG: @globals = global %struct.S zeroinitializer,
 // CHECK-DAG: [[STAT:@.+stat]] = internal global %struct.S zeroinitializer,
 // CHECK-DAG: [[STAT_REF:@.+]] = internal constant %struct.S* [[STAT]]
-// CHECK-DAG: @out_decl_target ={{ hidden | }}global i32 0,
+// CHECK-DAG: @out_decl_target = global i32 0,
 // CHECK-DAG: @llvm.used = appending global [2 x i8*] [i8* bitcast (void ()* 
@__omp_offloading__{{.+}}_globals_l[[@LINE+84]]_ctor to i8*), i8* bitcast (void 
()* @__omp_offloading__{{.+}}_stat_l[[@LINE+85]]_ctor to i8*)],
 // CHECK-DAG: @llvm.compiler.used = appending global [1 x i8*] [i8* bitcast 
(%struct.S** [[STAT_REF]] to i8*)],
 
@@ -283,4 +284,11 @@
   X->emitted();
 }
 #pragma omp end declare target
+
+struct SS {
+#pragma omp declare target
+  static int SSS;
+#pragma omp end declare target
+};
+int SS::SSS = 1;
 #endif
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -786,6 +786,11 @@
 //
 // Note that we don't want to make the variable non-external
 // because of this, but unique-external linkage suits us.
+
+// We need variables inside OpenMP declare target directives to be visible.
+if (OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(Var))
+  return LinkageInfo::external();
+
 if (Context.getLangOpts().CPlusPlus && !isFirstInExternCContext(Var) &&
 !IgnoreVarTypeLinkage) {
   LinkageInfo TypeLV = getLVForType(*Var->getType(), computation);
@@ -1069,6 +1074,12 @@
 
   // Finally, merge in information from the class.
   LV.mergeMaybeWithVisibility(classLV, considerClassVisibility);
+
+  // We need variables inside OpenMP declare target directives to be visible.
+  if (const VarDecl *VD = dyn_cast(D))
+if (OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(VD))
+  return LinkageInfo(LV.getLinkage(), DefaultVisibility, false);
+
   return LV;
 }
 


Index: clang/test/OpenMP/declare_target_only_one_side_compilation.cpp
===
--- clang/test/OpenMP/declare_ta

[PATCH] D117129: [clang-tidy] Extract Class IncluderClangTidyCheck

2022-01-15 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

In D117129#3239514 , 
@LegalizeAdulthood wrote:

> In D117129#3239143 , @njames93 
> wrote:
>
>> My proposed idea was putting the `IncludeInserter` instance
>> inside the `ClangTidyContext`, but then only registering it if
>> any checks actually want to use it.
>
> Use of the `IncludeInserter` would be signaled by an argument
> to the ctor fo r`ClangTidyCheck` in the c'tor for the derived
> class?

There is a hurdle to overcome with this, Currently each check can have its own 
include style - which doesn't really make sense. Hopefully people actually just 
use the global version of the option instead.
However if we were to coalesce all checks to use the same `IncludeInserter`, we 
would have to force clang-tidy to just read the global version.
Going this route it'd be best if we warned on configs that specify their own 
IncludeStyle. I've created a draft implementation above, definitely needs work 
though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117129

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


[PATCH] D117391: [AST] Ignore implicit nodes in CastExpr::getConversionFunction

2022-01-15 Thread David Rector via Phabricator via cfe-commits
davrec accepted this revision.
davrec added a comment.
This revision is now accepted and ready to land.

Looks good, thanks for fixing this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117391

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


[PATCH] D117414: [clang-format] Add return code to git-clang-format

2022-01-15 Thread Owen Pan via Phabricator via cfe-commits
owenpan created this revision.
owenpan added a reviewer: MyDeveloperDay.
owenpan added a project: clang-format.
owenpan requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

https://github.com/llvm/llvm-project/issues/53220


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117414

Files:
  clang/tools/clang-format/git-clang-format


Index: clang/tools/clang-format/git-clang-format
===
--- clang/tools/clang-format/git-clang-format
+++ clang/tools/clang-format/git-clang-format
@@ -161,10 +161,12 @@
   print('Running clang-format on the following files:')
   for filename in changed_lines:
 print('%s' % filename)
+
   if not changed_lines:
 if opts.verbose >= 0:
   print('no modified files to format')
-return
+return 0
+
   if len(commits) > 1:
 old_tree = commits[1]
 new_tree = run_clang_format_and_save_to_tree(changed_lines,
@@ -179,10 +181,13 @@
   if opts.verbose >= 1:
 print('old tree: %s' % old_tree)
 print('new tree: %s' % new_tree)
+
   if old_tree == new_tree:
 if opts.verbose >= 0:
   print('clang-format did not modify any files')
-  elif opts.diff:
+return 0
+
+  if opts.diff:
 print_diff(old_tree, new_tree)
   elif opts.diffstat:
 print_diffstat(old_tree, new_tree)
@@ -194,6 +199,8 @@
   for filename in changed_files:
 print('%s' % filename)
 
+  return 1
+
 
 def load_git_config(non_string_options=None):
   """Return the git configuration as a dictionary.


Index: clang/tools/clang-format/git-clang-format
===
--- clang/tools/clang-format/git-clang-format
+++ clang/tools/clang-format/git-clang-format
@@ -161,10 +161,12 @@
   print('Running clang-format on the following files:')
   for filename in changed_lines:
 print('%s' % filename)
+
   if not changed_lines:
 if opts.verbose >= 0:
   print('no modified files to format')
-return
+return 0
+
   if len(commits) > 1:
 old_tree = commits[1]
 new_tree = run_clang_format_and_save_to_tree(changed_lines,
@@ -179,10 +181,13 @@
   if opts.verbose >= 1:
 print('old tree: %s' % old_tree)
 print('new tree: %s' % new_tree)
+
   if old_tree == new_tree:
 if opts.verbose >= 0:
   print('clang-format did not modify any files')
-  elif opts.diff:
+return 0
+
+  if opts.diff:
 print_diff(old_tree, new_tree)
   elif opts.diffstat:
 print_diffstat(old_tree, new_tree)
@@ -194,6 +199,8 @@
   for filename in changed_files:
 print('%s' % filename)
 
+  return 1
+
 
 def load_git_config(non_string_options=None):
   """Return the git configuration as a dictionary.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D117415: Handle C variables with name that matches c++ access specifier

2022-01-15 Thread psigillito via Phabricator via cfe-commits
psigillito created this revision.
psigillito requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117415

Files:
  .arclint
  clang/lib/Format/FormatToken.h
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3030,6 +3030,8 @@
"label:\n"
"  signals.baz();\n"
"}");
+  // Var that looks like accessSpecifier
+  verifyFormat("private.p = 1;");
 }
 
 TEST_F(FormatTest, SeparatesLogicalBlocks) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2499,9 +2499,17 @@
   if (FormatTok->isOneOf(Keywords.kw_slots, Keywords.kw_qslots))
 nextToken();
   // Otherwise, we don't know what it is, and we'd better keep the next token.
-  if (FormatTok->Tok.is(tok::colon))
+  if (FormatTok->Tok.is(tok::colon)) {
 nextToken();
-  addUnwrappedLine();
+addUnwrappedLine();
+return;
+  }
+  // if followed by an operator probably intended to be a variable, dont treat
+  // as access specifier
+  if (clang::format::OperatorsFollowingVar.find(FormatTok->Tok.getKind()) ==
+  OperatorsFollowingVar.end()) {
+addUnwrappedLine();
+  }
 }
 
 void UnwrappedLineParser::parseConcept() {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -100,7 +100,14 @@
 if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   return 0;
-if (RootToken.isAccessSpecifier(false) ||
+if (RootToken.isAccessSpecifier(Style.Language == FormatStyle::LK_Cpp) ||
+(RootToken.Next &&
+ RootToken.Next->isOneOf(Keywords.kw_slots, Keywords.kw_qslots) &&
+ RootToken.Next->Next && RootToken.Next->Next->is(tok::colon)) ||
+(RootToken.isAccessSpecifier(false) &&
+ (!RootToken.Next ||
+  CppOperatorsFollowingVar.find(RootToken.Next->Tok.getKind()) ==
+  clang::format::CppOperatorsFollowingVar.end())) ||
 RootToken.isObjCAccessSpecifier() ||
 (RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
  RootToken.Next && RootToken.Next->is(tok::colon))) {
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -121,6 +121,32 @@
   TYPE(CSharpGenericTypeConstraintComma)   \
   TYPE(Unknown)
 
+static const std::set CppOperatorsFollowingVar = {
+tok::l_square,  tok::r_square,
+tok::l_paren,   tok::r_paren,
+tok::r_brace,   tok::period,
+tok::ellipsis,  tok::amp,
+tok::ampamp,tok::ampequal,
+tok::star,  tok::starequal,
+tok::plus,  tok::plusplus,
+tok::plusequal, tok::minus,
+tok::arrow, tok::minusminus,
+tok::minusequal,tok::exclaim,
+tok::exclaimequal,  tok::slash,
+tok::slashequal,tok::percent,
+tok::percentequal,  tok::less,
+tok::lessless,  tok::lessequal,
+tok::lesslessequal, tok::spaceship,
+tok::greater,   tok::greatergreater,
+tok::greaterequal,  tok::greatergreaterequal,
+tok::caret, tok::caretequal,
+tok::pipe,  tok::pipepipe,
+tok::pipeequal, tok::question,
+tok::semi,  tok::equal,
+tok::equalequal,tok::comma,
+tok::hash,  tok::hashhash,
+tok::hashat};
+
 /// Determines the semantic type of a syntactic token, e.g. whether "<" is a
 /// template opener or binary operator.
 enum TokenType : uint8_t {
Index: .arclint
===
--- .arclint
+++ .arclint
@@ -2,7 +2,7 @@
   "linters": {
 "clang-format": {
   "type": "script-and-regex",
-  "script-and-regex.script": "bash utils/arcanist/clang-format.sh",
+  "script-and-regex.script": "cmd utils\\arcanist\\clang-format.sh",
   "script-and-regex.regex": "/^(?P[[:alpha:]]+)\n(?P[^\n]+)\n(|(?P\\d),(?P\\d)\n(?P.*)\n(?P.*)\n)$/s",
   "include": [
 "(\\.(cc|cpp|h)$)"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D117416: Handle C variables with name that matches c++ access specifier

2022-01-15 Thread psigillito via Phabricator via cfe-commits
psigillito created this revision.
psigillito requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117416

Files:
  .arclint
  clang/lib/Format/FormatToken.h
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3030,6 +3030,8 @@
"label:\n"
"  signals.baz();\n"
"}");
+  // Var that looks like accessSpecifier
+  verifyFormat("private.p = 1;");
 }
 
 TEST_F(FormatTest, SeparatesLogicalBlocks) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2499,9 +2499,17 @@
   if (FormatTok->isOneOf(Keywords.kw_slots, Keywords.kw_qslots))
 nextToken();
   // Otherwise, we don't know what it is, and we'd better keep the next token.
-  if (FormatTok->Tok.is(tok::colon))
+  if (FormatTok->Tok.is(tok::colon)) {
 nextToken();
-  addUnwrappedLine();
+addUnwrappedLine();
+return;
+  }
+  // if followed by an operator probably intended to be a variable, dont treat
+  // as access specifier
+  if (clang::format::OperatorsFollowingVar.find(FormatTok->Tok.getKind()) ==
+  OperatorsFollowingVar.end()) {
+addUnwrappedLine();
+  }
 }
 
 void UnwrappedLineParser::parseConcept() {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -100,7 +100,14 @@
 if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
 Style.isCSharp())
   return 0;
-if (RootToken.isAccessSpecifier(false) ||
+if (RootToken.isAccessSpecifier(Style.Language == FormatStyle::LK_Cpp) ||
+(RootToken.Next &&
+ RootToken.Next->isOneOf(Keywords.kw_slots, Keywords.kw_qslots) &&
+ RootToken.Next->Next && RootToken.Next->Next->is(tok::colon)) ||
+(RootToken.isAccessSpecifier(false) &&
+ (!RootToken.Next ||
+  CppOperatorsFollowingVar.find(RootToken.Next->Tok.getKind()) ==
+  clang::format::CppOperatorsFollowingVar.end())) ||
 RootToken.isObjCAccessSpecifier() ||
 (RootToken.isOneOf(Keywords.kw_signals, Keywords.kw_qsignals) &&
  RootToken.Next && RootToken.Next->is(tok::colon))) {
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -121,6 +121,32 @@
   TYPE(CSharpGenericTypeConstraintComma)   \
   TYPE(Unknown)
 
+static const std::set CppOperatorsFollowingVar = {
+tok::l_square,  tok::r_square,
+tok::l_paren,   tok::r_paren,
+tok::r_brace,   tok::period,
+tok::ellipsis,  tok::amp,
+tok::ampamp,tok::ampequal,
+tok::star,  tok::starequal,
+tok::plus,  tok::plusplus,
+tok::plusequal, tok::minus,
+tok::arrow, tok::minusminus,
+tok::minusequal,tok::exclaim,
+tok::exclaimequal,  tok::slash,
+tok::slashequal,tok::percent,
+tok::percentequal,  tok::less,
+tok::lessless,  tok::lessequal,
+tok::lesslessequal, tok::spaceship,
+tok::greater,   tok::greatergreater,
+tok::greaterequal,  tok::greatergreaterequal,
+tok::caret, tok::caretequal,
+tok::pipe,  tok::pipepipe,
+tok::pipeequal, tok::question,
+tok::semi,  tok::equal,
+tok::equalequal,tok::comma,
+tok::hash,  tok::hashhash,
+tok::hashat};
+
 /// Determines the semantic type of a syntactic token, e.g. whether "<" is a
 /// template opener or binary operator.
 enum TokenType : uint8_t {
Index: .arclint
===
--- .arclint
+++ .arclint
@@ -2,7 +2,7 @@
   "linters": {
 "clang-format": {
   "type": "script-and-regex",
-  "script-and-regex.script": "bash utils/arcanist/clang-format.sh",
+  "script-and-regex.script": "cmd utils\\arcanist\\clang-format.sh",
   "script-and-regex.regex": "/^(?P[[:alpha:]]+)\n(?P[^\n]+)\n(|(?P\\d),(?P\\d)\n(?P.*)\n(?P.*)\n)$/s",
   "include": [
 "(\\.(cc|cpp|h)$)"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D117416: Handle C variables with name that matches c++ access specifier

2022-01-15 Thread psigillito via Phabricator via cfe-commits
psigillito added a comment.

Sorry, this is my first time using Phabricator. I am not sure if it matters 
that .arclint is changed in this review, it is not part of my commit.

This issue only appears to impact the accessSpecifier keywords. For example, a 
c variable declared with the name 'private' will be treated as an access 
specifier.

I tried to make parsing the identifiers more restrictive by requiring either an 
@ prefix for obj-c or a colon after the keyword for cpp. Unfortunately, that 
does not allow an incorrect access specifier (i.e without a colon) to be 
identified as an accessor. I think a missing colon is probably a common typo so 
I dont want to remove identifying a access specifier with a missing colon 
correctly.

I think the correct approach is to check if the var named 'private' is followed 
by an operator or symbol indicating it is variable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

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


[PATCH] D117398: [clang-format] Fix bug in parsing `operator<` with template

2022-01-15 Thread Jino Park via Phabricator via cfe-commits
pjessesco updated this revision to Diff 400341.
pjessesco added a comment.

Fix to pass unittest


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

https://reviews.llvm.org/D117398

Files:
  clang/lib/Format/FormatTokenLexer.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9275,6 +9275,8 @@
   verifyFormat("operator SomeType();");
   verifyFormat("operator SomeType();");
   verifyFormat("operator SomeType>();");
+  verifyFormat("operator< <>();");
+  
   verifyFormat("void *operator new(std::size_t size);");
   verifyFormat("void *operator new[](std::size_t size);");
   verifyFormat("void operator delete(void *ptr);");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3345,6 +3345,9 @@
 if (Right.is(tok::l_brace) && Right.is(BK_BracedInit) &&
 !Left.opensScope() && Style.SpaceBeforeCpp11BracedList)
   return true;
+if (Left.is(tok::less) && Left.is(TT_OverloadedOperator) &&
+Right.is(TT_TemplateOpener))
+  return true;
   } else if (Style.Language == FormatStyle::LK_Proto ||
  Style.Language == FormatStyle::LK_TextProto) {
 if (Right.is(tok::period) &&
Index: clang/lib/Format/FormatTokenLexer.cpp
===
--- clang/lib/Format/FormatTokenLexer.cpp
+++ clang/lib/Format/FormatTokenLexer.cpp
@@ -429,9 +429,10 @@
   if (Tokens.size() < 3)
 return false;
 
+  auto Forth = (Tokens.end() - 4)[0];
   bool FourthTokenIsLess = false;
   if (Tokens.size() > 3)
-FourthTokenIsLess = (Tokens.end() - 4)[0]->is(tok::less);
+FourthTokenIsLess = Forth->is(tok::less);
 
   auto First = Tokens.end() - 3;
   if (First[2]->is(tok::less) || First[1]->isNot(tok::less) ||
@@ -443,6 +444,10 @@
   First[1]->WhitespaceRange.getEnd())
 return false;
 
+  // Do not remove a whitespace between the two "<" e.g. "operator<<>".
+  if (First[2]->is(tok::greater) && Forth->is(tok::kw_operator))
+return false;
+
   First[0]->Tok.setKind(tok::lessless);
   First[0]->TokenText = "<<";
   First[0]->ColumnWidth += 1;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9275,6 +9275,8 @@
   verifyFormat("operator SomeType();");
   verifyFormat("operator SomeType();");
   verifyFormat("operator SomeType>();");
+  verifyFormat("operator< <>();");
+  
   verifyFormat("void *operator new(std::size_t size);");
   verifyFormat("void *operator new[](std::size_t size);");
   verifyFormat("void operator delete(void *ptr);");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3345,6 +3345,9 @@
 if (Right.is(tok::l_brace) && Right.is(BK_BracedInit) &&
 !Left.opensScope() && Style.SpaceBeforeCpp11BracedList)
   return true;
+if (Left.is(tok::less) && Left.is(TT_OverloadedOperator) &&
+Right.is(TT_TemplateOpener))
+  return true;
   } else if (Style.Language == FormatStyle::LK_Proto ||
  Style.Language == FormatStyle::LK_TextProto) {
 if (Right.is(tok::period) &&
Index: clang/lib/Format/FormatTokenLexer.cpp
===
--- clang/lib/Format/FormatTokenLexer.cpp
+++ clang/lib/Format/FormatTokenLexer.cpp
@@ -429,9 +429,10 @@
   if (Tokens.size() < 3)
 return false;
 
+  auto Forth = (Tokens.end() - 4)[0];
   bool FourthTokenIsLess = false;
   if (Tokens.size() > 3)
-FourthTokenIsLess = (Tokens.end() - 4)[0]->is(tok::less);
+FourthTokenIsLess = Forth->is(tok::less);
 
   auto First = Tokens.end() - 3;
   if (First[2]->is(tok::less) || First[1]->isNot(tok::less) ||
@@ -443,6 +444,10 @@
   First[1]->WhitespaceRange.getEnd())
 return false;
 
+  // Do not remove a whitespace between the two "<" e.g. "operator<<>".
+  if (First[2]->is(tok::greater) && Forth->is(tok::kw_operator))
+return false;
+
   First[0]->Tok.setKind(tok::lessless);
   First[0]->TokenText = "<<";
   First[0]->ColumnWidth += 1;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[libunwind] 4a678f8 - [cmake] Use `GNUInstallDirs` to support custom installation dirs.

2022-01-15 Thread John Ericson via cfe-commits

Author: John Ericson
Date: 2022-01-16T05:33:07Z
New Revision: 4a678f8072004eff9214c1a4e1836a14abb69535

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

LOG: [cmake] Use `GNUInstallDirs` to support custom installation dirs.

This is the original patch in my GNUInstallDirs series, now last to merge as 
the final piece!

It arose as a new draft of D28234. I initially did the unorthodox thing of 
pushing to that when I wasn't the original author, but since I ended up

 - Using `GNUInstallDirs`, rather than mimicking it, as the original author was 
hesitant to do but others requested.

 - Converting all the packages, not just LLVM, effecting many more projects 
than LLVM itself.

I figured it was time to make a new revision.

I have used this patch series (and many back-ports) as the basis of 
https://github.com/NixOS/nixpkgs/pull/111487 for my distro (NixOS), which was 
merged last spring (2021). It looked like people were generally on board in 
D28234, but I make note of this here in case extra motivation is useful.

---

As pointed out in the original issue, a central tension is that LLVM already 
has some partial support for these sorts of things. Variables like 
`COMPILER_RT_INSTALL_PATH` have already been dealt with. Variables like 
`LLVM_LIBDIR_SUFFIX` however, will require further work, so that we may use 
`CMAKE_INSTALL_LIBDIR`.

These remaining items will be addressed in further patches. What is here is now 
rote and so we should get it out of the way before dealing more intricately 
with the remainder.

Reviewed By: #libunwind, #libc, #libc_abi, compnerd

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

Added: 


Modified: 
clang-tools-extra/CMakeLists.txt
clang-tools-extra/clang-doc/tool/CMakeLists.txt
clang-tools-extra/clang-include-fixer/find-all-symbols/tool/CMakeLists.txt
clang-tools-extra/clang-include-fixer/tool/CMakeLists.txt
clang-tools-extra/clang-tidy/CMakeLists.txt
clang-tools-extra/clang-tidy/tool/CMakeLists.txt
clang-tools-extra/modularize/CMakeLists.txt
clang/CMakeLists.txt
clang/cmake/modules/AddClang.cmake
clang/tools/c-index-test/CMakeLists.txt
clang/tools/clang-format/CMakeLists.txt
clang/tools/clang-nvlink-wrapper/CMakeLists.txt
clang/tools/clang-rename/CMakeLists.txt
clang/tools/libclang/CMakeLists.txt
clang/tools/scan-build-py/CMakeLists.txt
clang/tools/scan-build/CMakeLists.txt
clang/tools/scan-view/CMakeLists.txt
clang/utils/hmaptool/CMakeLists.txt
compiler-rt/cmake/base-config-ix.cmake
libc/CMakeLists.txt
libcxx/CMakeLists.txt
libcxx/cmake/Modules/HandleLibCXXABI.cmake
libcxxabi/CMakeLists.txt
libunwind/CMakeLists.txt
llvm/cmake/modules/LLVMInstallSymlink.cmake
mlir/CMakeLists.txt
mlir/cmake/modules/AddMLIR.cmake
openmp/CMakeLists.txt
openmp/libompd/src/CMakeLists.txt
openmp/runtime/cmake/LibompCheckLinkerFlag.cmake
openmp/runtime/src/CMakeLists.txt
openmp/tools/multiplex/CMakeLists.txt
polly/CMakeLists.txt
polly/cmake/CMakeLists.txt
polly/lib/External/CMakeLists.txt
pstl/CMakeLists.txt

Removed: 




diff  --git a/clang-tools-extra/CMakeLists.txt 
b/clang-tools-extra/CMakeLists.txt
index 2e73b6ba81d2..7b8274a97336 100644
--- a/clang-tools-extra/CMakeLists.txt
+++ b/clang-tools-extra/CMakeLists.txt
@@ -1,4 +1,5 @@
 include(CMakeDependentOption)
+include(GNUInstallDirs)
 
 option(CLANG_TIDY_ENABLE_STATIC_ANALYZER
   "Include static analyzer checks in clang-tidy" ON)

diff  --git a/clang-tools-extra/clang-doc/tool/CMakeLists.txt 
b/clang-tools-extra/clang-doc/tool/CMakeLists.txt
index 7e7147886916..fb8317b27293 100644
--- a/clang-tools-extra/clang-doc/tool/CMakeLists.txt
+++ b/clang-tools-extra/clang-doc/tool/CMakeLists.txt
@@ -19,9 +19,9 @@ target_link_libraries(clang-doc
   )
 
 install(FILES ../assets/clang-doc-default-stylesheet.css
-  DESTINATION share/clang
+  DESTINATION "${CMAKE_INSTALL_DATADIR}/clang"
   COMPONENT clang-doc)
 
 install(FILES ../assets/index.js
-  DESTINATION share/clang
+  DESTINATION "${CMAKE_INSTALL_DATADIR}/clang"
   COMPONENT clang-doc)

diff  --git 
a/clang-tools-extra/clang-include-fixer/find-all-symbols/tool/CMakeLists.txt 
b/clang-tools-extra/clang-include-fixer/find-all-symbols/tool/CMakeLists.txt
index 8f5509d22e24..e6926a0d5bd1 100644
--- a/clang-tools-extra/clang-include-fixer/find-all-symbols/tool/CMakeLists.txt
+++ b/clang-tools-extra/clang-include-fixer/find-all-symbols/tool/CMakeLists.txt
@@ -20,5 +20,5 @@ target_link_libraries(find-all-symbols
   )
 
 install(PROGRAMS run-find-all-symbols.py
-  DESTINATION share/clang
+  DESTINATION "${CMAKE_INSTALL_DATADIR}/clang"
   COMPONENT find-all-symbols)

diff  --git a/clang-tools-extra/clang-include-fixer/tool/CMakeList

[PATCH] D99484: [cmake] Use `GNUInstallDirs` to support custom installation dirs.

2022-01-15 Thread John Ericson 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 rG4a678f807200: [cmake] Use `GNUInstallDirs` to support custom 
installation dirs. (authored by Ericson2314).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99484

Files:
  clang-tools-extra/CMakeLists.txt
  clang-tools-extra/clang-doc/tool/CMakeLists.txt
  clang-tools-extra/clang-include-fixer/find-all-symbols/tool/CMakeLists.txt
  clang-tools-extra/clang-include-fixer/tool/CMakeLists.txt
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/tool/CMakeLists.txt
  clang-tools-extra/modularize/CMakeLists.txt
  clang/CMakeLists.txt
  clang/cmake/modules/AddClang.cmake
  clang/tools/c-index-test/CMakeLists.txt
  clang/tools/clang-format/CMakeLists.txt
  clang/tools/clang-nvlink-wrapper/CMakeLists.txt
  clang/tools/clang-rename/CMakeLists.txt
  clang/tools/libclang/CMakeLists.txt
  clang/tools/scan-build-py/CMakeLists.txt
  clang/tools/scan-build/CMakeLists.txt
  clang/tools/scan-view/CMakeLists.txt
  clang/utils/hmaptool/CMakeLists.txt
  compiler-rt/cmake/base-config-ix.cmake
  libc/CMakeLists.txt
  libcxx/CMakeLists.txt
  libcxx/cmake/Modules/HandleLibCXXABI.cmake
  libcxxabi/CMakeLists.txt
  libunwind/CMakeLists.txt
  llvm/cmake/modules/LLVMInstallSymlink.cmake
  mlir/CMakeLists.txt
  mlir/cmake/modules/AddMLIR.cmake
  openmp/CMakeLists.txt
  openmp/libompd/src/CMakeLists.txt
  openmp/runtime/cmake/LibompCheckLinkerFlag.cmake
  openmp/runtime/src/CMakeLists.txt
  openmp/tools/multiplex/CMakeLists.txt
  polly/CMakeLists.txt
  polly/cmake/CMakeLists.txt
  polly/lib/External/CMakeLists.txt
  pstl/CMakeLists.txt

Index: pstl/CMakeLists.txt
===
--- pstl/CMakeLists.txt
+++ pstl/CMakeLists.txt
@@ -7,6 +7,8 @@
 #===--===##
 cmake_minimum_required(VERSION 3.13.4)
 
+include(GNUInstallDirs)
+
 set(PARALLELSTL_VERSION_FILE "${CMAKE_CURRENT_SOURCE_DIR}/include/pstl/internal/pstl_config.h")
 file(STRINGS "${PARALLELSTL_VERSION_FILE}" PARALLELSTL_VERSION_SOURCE REGEX "#define _PSTL_VERSION .*$")
 string(REGEX REPLACE "#define _PSTL_VERSION (.*)$" "\\1" PARALLELSTL_VERSION_SOURCE "${PARALLELSTL_VERSION_SOURCE}")
@@ -90,10 +92,10 @@
   "${CMAKE_CURRENT_BINARY_DIR}/ParallelSTLConfigVersion.cmake"
 DESTINATION lib/cmake/ParallelSTL)
 install(DIRECTORY include/
-DESTINATION include
+DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}"
 PATTERN "*.in" EXCLUDE)
 install(FILES "${PSTL_CONFIG_SITE_PATH}"
-DESTINATION include)
+DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}")
 
 add_custom_target(install-pstl
   COMMAND "${CMAKE_COMMAND}" -P "${PROJECT_BINARY_DIR}/cmake_install.cmake" -DCOMPONENT=ParallelSTL)
Index: polly/lib/External/CMakeLists.txt
===
--- polly/lib/External/CMakeLists.txt
+++ polly/lib/External/CMakeLists.txt
@@ -290,7 +290,7 @@
 install(DIRECTORY
   ${ISL_SOURCE_DIR}/include/
   ${ISL_BINARY_DIR}/include/
-  DESTINATION include/polly
+  DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/polly"
   FILES_MATCHING
   PATTERN "*.h"
   PATTERN "CMakeFiles" EXCLUDE
Index: polly/cmake/CMakeLists.txt
===
--- polly/cmake/CMakeLists.txt
+++ polly/cmake/CMakeLists.txt
@@ -1,5 +1,6 @@
 # Keep this in sync with llvm/cmake/CMakeLists.txt!
 
+include(ExtendPath)
 include(FindPrefixFromConfig)
 
 set(LLVM_INSTALL_PACKAGE_DIR "lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm")
@@ -83,17 +84,18 @@
 # Generate PollyConfig.cmake for the install tree.
 unset(POLLY_EXPORTS)
 find_prefix_from_config(POLLY_CONFIG_CODE POLLY_INSTALL_PREFIX "${POLLY_INSTALL_PACKAGE_DIR}")
-set(POLLY_CONFIG_LLVM_CMAKE_DIR "\${POLLY_INSTALL_PREFIX}/${LLVM_INSTALL_PACKAGE_DIR}")
-set(POLLY_CONFIG_CMAKE_DIR "\${POLLY_INSTALL_PREFIX}/${POLLY_INSTALL_PACKAGE_DIR}")
-set(POLLY_CONFIG_LIBRARY_DIRS "\${POLLY_INSTALL_PREFIX}/lib${LLVM_LIBDIR_SUFFIX}")
+extend_path(POLLY_CONFIG_LLVM_CMAKE_DIR "\${POLLY_INSTALL_PREFIX}" "${LLVM_INSTALL_PACKAGE_DIR}")
+extend_path(POLLY_CONFIG_CMAKE_DIR "\${POLLY_INSTALL_PREFIX}" "${POLLY_INSTALL_PACKAGE_DIR}")
+extend_path(POLLY_CONFIG_LIBRARY_DIRS "\${POLLY_INSTALL_PREFIX}" "lib${LLVM_LIBDIR_SUFFIX}")
+extend_path(base_includedir "\${POLLY_INSTALL_PREFIX}" "${CMAKE_INSTALL_INCLUDEDIR}")
 if (POLLY_BUNDLED_ISL)
   set(POLLY_CONFIG_INCLUDE_DIRS
-"\${POLLY_INSTALL_PREFIX}/include"
-"\${POLLY_INSTALL_PREFIX}/include/polly"
+"${base_includedir}"
+"${base_includedir}/polly"
 )
 else()
   set(POLLY_CONFIG_INCLUDE_DIRS
-"\${POLLY_INSTALL_PREFIX}/include"
+"${base_includedir}"
 ${ISL_INCLUDE_DIRS}
 )
 endif()
@@ -110,12 +112,12 @@
 

[libunwind] da77db5 - Revert "[cmake] Use `GNUInstallDirs` to support custom installation dirs."

2022-01-15 Thread John Ericson via cfe-commits

Author: John Ericson
Date: 2022-01-16T05:48:30Z
New Revision: da77db58d7629a3bfea1a0053aa9c29764b0bc2b

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

LOG: Revert "[cmake] Use `GNUInstallDirs` to support custom installation dirs."

https://lab.llvm.org/buildbot/#/builders/46/builds/21146 Still have
this odd error, not sure how to reproduce, so I will just try breaking
up my patch.

This reverts commit 4a678f8072004eff9214c1a4e1836a14abb69535.

Added: 


Modified: 
clang-tools-extra/CMakeLists.txt
clang-tools-extra/clang-doc/tool/CMakeLists.txt
clang-tools-extra/clang-include-fixer/find-all-symbols/tool/CMakeLists.txt
clang-tools-extra/clang-include-fixer/tool/CMakeLists.txt
clang-tools-extra/clang-tidy/CMakeLists.txt
clang-tools-extra/clang-tidy/tool/CMakeLists.txt
clang-tools-extra/modularize/CMakeLists.txt
clang/CMakeLists.txt
clang/cmake/modules/AddClang.cmake
clang/tools/c-index-test/CMakeLists.txt
clang/tools/clang-format/CMakeLists.txt
clang/tools/clang-nvlink-wrapper/CMakeLists.txt
clang/tools/clang-rename/CMakeLists.txt
clang/tools/libclang/CMakeLists.txt
clang/tools/scan-build-py/CMakeLists.txt
clang/tools/scan-build/CMakeLists.txt
clang/tools/scan-view/CMakeLists.txt
clang/utils/hmaptool/CMakeLists.txt
compiler-rt/cmake/base-config-ix.cmake
libc/CMakeLists.txt
libcxx/CMakeLists.txt
libcxx/cmake/Modules/HandleLibCXXABI.cmake
libcxxabi/CMakeLists.txt
libunwind/CMakeLists.txt
llvm/cmake/modules/LLVMInstallSymlink.cmake
mlir/CMakeLists.txt
mlir/cmake/modules/AddMLIR.cmake
openmp/CMakeLists.txt
openmp/libompd/src/CMakeLists.txt
openmp/runtime/cmake/LibompCheckLinkerFlag.cmake
openmp/runtime/src/CMakeLists.txt
openmp/tools/multiplex/CMakeLists.txt
polly/CMakeLists.txt
polly/cmake/CMakeLists.txt
polly/lib/External/CMakeLists.txt
pstl/CMakeLists.txt

Removed: 




diff  --git a/clang-tools-extra/CMakeLists.txt 
b/clang-tools-extra/CMakeLists.txt
index 7b8274a97336b..2e73b6ba81d2e 100644
--- a/clang-tools-extra/CMakeLists.txt
+++ b/clang-tools-extra/CMakeLists.txt
@@ -1,5 +1,4 @@
 include(CMakeDependentOption)
-include(GNUInstallDirs)
 
 option(CLANG_TIDY_ENABLE_STATIC_ANALYZER
   "Include static analyzer checks in clang-tidy" ON)

diff  --git a/clang-tools-extra/clang-doc/tool/CMakeLists.txt 
b/clang-tools-extra/clang-doc/tool/CMakeLists.txt
index fb8317b272932..7e71478869160 100644
--- a/clang-tools-extra/clang-doc/tool/CMakeLists.txt
+++ b/clang-tools-extra/clang-doc/tool/CMakeLists.txt
@@ -19,9 +19,9 @@ target_link_libraries(clang-doc
   )
 
 install(FILES ../assets/clang-doc-default-stylesheet.css
-  DESTINATION "${CMAKE_INSTALL_DATADIR}/clang"
+  DESTINATION share/clang
   COMPONENT clang-doc)
 
 install(FILES ../assets/index.js
-  DESTINATION "${CMAKE_INSTALL_DATADIR}/clang"
+  DESTINATION share/clang
   COMPONENT clang-doc)

diff  --git 
a/clang-tools-extra/clang-include-fixer/find-all-symbols/tool/CMakeLists.txt 
b/clang-tools-extra/clang-include-fixer/find-all-symbols/tool/CMakeLists.txt
index e6926a0d5bd10..8f5509d22e24a 100644
--- a/clang-tools-extra/clang-include-fixer/find-all-symbols/tool/CMakeLists.txt
+++ b/clang-tools-extra/clang-include-fixer/find-all-symbols/tool/CMakeLists.txt
@@ -20,5 +20,5 @@ target_link_libraries(find-all-symbols
   )
 
 install(PROGRAMS run-find-all-symbols.py
-  DESTINATION "${CMAKE_INSTALL_DATADIR}/clang"
+  DESTINATION share/clang
   COMPONENT find-all-symbols)

diff  --git a/clang-tools-extra/clang-include-fixer/tool/CMakeLists.txt 
b/clang-tools-extra/clang-include-fixer/tool/CMakeLists.txt
index 5b9e00ab87cd8..3936ac1e8a5a5 100644
--- a/clang-tools-extra/clang-include-fixer/tool/CMakeLists.txt
+++ b/clang-tools-extra/clang-include-fixer/tool/CMakeLists.txt
@@ -21,8 +21,8 @@ target_link_libraries(clang-include-fixer
   )
 
 install(PROGRAMS clang-include-fixer.el
-  DESTINATION "${CMAKE_INSTALL_DATADIR}/clang"
+  DESTINATION share/clang
   COMPONENT clang-include-fixer)
 install(PROGRAMS clang-include-fixer.py
-  DESTINATION "${CMAKE_INSTALL_DATADIR}/clang"
+  DESTINATION share/clang
   COMPONENT clang-include-fixer)

diff  --git a/clang-tools-extra/clang-tidy/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/CMakeLists.txt
index 075e9f9909d65..455645050d93d 100644
--- a/clang-tools-extra/clang-tidy/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/CMakeLists.txt
@@ -113,7 +113,7 @@ add_subdirectory(utils)
 
 if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
   install(DIRECTORY .
-DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/clang-tidy"
+DESTINATION include/clang-tidy
 COMPONENT clang-tidy-headers
 FILES_MATCHING
 PATTERN "*.h"

diff  --git a/clang-tools-extra/clang-tidy/tool/CMakeLists.txt

[PATCH] D99484: [cmake] Use `GNUInstallDirs` to support custom installation dirs.

2022-01-15 Thread John Ericson via Phabricator via cfe-commits
Ericson2314 reopened this revision.
Ericson2314 added a comment.
This revision is now accepted and ready to land.

Will try to break up the pathc further for better troubleshooting, but keeping 
this open to track progress.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99484

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


[PATCH] D117419: [clang][cmake] Use `GNUInstallDirs` to support custom installation dirs

2022-01-15 Thread John Ericson via Phabricator via cfe-commits
Ericson2314 created this revision.
Herald added subscribers: arphaman, whisperity, mgorny.
Ericson2314 requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

I am breaking apart D99484  so the cause of 
build failures is easier to
understand.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117419

Files:
  clang/CMakeLists.txt
  clang/cmake/modules/AddClang.cmake
  clang/cmake/modules/CMakeLists.txt
  clang/tools/c-index-test/CMakeLists.txt
  clang/tools/clang-format/CMakeLists.txt
  clang/tools/clang-nvlink-wrapper/CMakeLists.txt
  clang/tools/clang-rename/CMakeLists.txt
  clang/tools/libclang/CMakeLists.txt
  clang/tools/scan-build-py/CMakeLists.txt
  clang/tools/scan-build/CMakeLists.txt
  clang/tools/scan-view/CMakeLists.txt
  clang/utils/hmaptool/CMakeLists.txt
  llvm/cmake/modules/LLVMInstallSymlink.cmake

Index: llvm/cmake/modules/LLVMInstallSymlink.cmake
===
--- llvm/cmake/modules/LLVMInstallSymlink.cmake
+++ llvm/cmake/modules/LLVMInstallSymlink.cmake
@@ -6,7 +6,8 @@
 
 function(install_symlink name target outdir)
   set(DESTDIR $ENV{DESTDIR})
-  set(bindir "${DESTDIR}${CMAKE_INSTALL_PREFIX}/${outdir}")
+  GNUInstallDirs_get_absolute_install_dir(bindir "${outdir}" BINDIR)
+  set(bindir "${DESTDIR}${bindir}")
 
   message(STATUS "Creating ${name}")
 
Index: clang/utils/hmaptool/CMakeLists.txt
===
--- clang/utils/hmaptool/CMakeLists.txt
+++ clang/utils/hmaptool/CMakeLists.txt
@@ -10,7 +10,7 @@
 
 list(APPEND Depends ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/${CLANG_HMAPTOOL})
 install(PROGRAMS ${CLANG_HMAPTOOL}
-DESTINATION bin
+DESTINATION "${CMAKE_INSTALL_BINDIR}"
 COMPONENT hmaptool)
 
 add_custom_target(hmaptool ALL DEPENDS ${Depends})
Index: clang/tools/scan-view/CMakeLists.txt
===
--- clang/tools/scan-view/CMakeLists.txt
+++ clang/tools/scan-view/CMakeLists.txt
@@ -20,7 +20,7 @@
DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/bin/${BinFile})
 list(APPEND Depends ${CMAKE_BINARY_DIR}/bin/${BinFile})
 install(PROGRAMS bin/${BinFile}
-DESTINATION bin
+DESTINATION "${CMAKE_INSTALL_BINDIR}"
 COMPONENT scan-view)
   endforeach()
 
@@ -34,7 +34,7 @@
DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/share/${ShareFile})
 list(APPEND Depends ${CMAKE_BINARY_DIR}/share/scan-view/${ShareFile})
 install(FILES share/${ShareFile}
-DESTINATION share/scan-view
+DESTINATION "${CMAKE_INSTALL_DATADIR}/scan-view"
 COMPONENT scan-view)
   endforeach()
 
Index: clang/tools/scan-build/CMakeLists.txt
===
--- clang/tools/scan-build/CMakeLists.txt
+++ clang/tools/scan-build/CMakeLists.txt
@@ -47,7 +47,7 @@
DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/bin/${BinFile})
 list(APPEND Depends ${CMAKE_BINARY_DIR}/bin/${BinFile})
 install(PROGRAMS bin/${BinFile}
-DESTINATION bin
+DESTINATION "${CMAKE_INSTALL_BINDIR}"
 COMPONENT scan-build)
   endforeach()
 
@@ -61,7 +61,7 @@
DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/libexec/${LibexecFile})
 list(APPEND Depends ${CMAKE_BINARY_DIR}/libexec/${LibexecFile})
 install(PROGRAMS libexec/${LibexecFile}
-DESTINATION libexec
+DESTINATION "${CMAKE_INSTALL_LIBEXECDIR}"
 COMPONENT scan-build)
   endforeach()
 
@@ -89,7 +89,7 @@
DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/share/scan-build/${ShareFile})
 list(APPEND Depends ${CMAKE_BINARY_DIR}/share/scan-build/${ShareFile})
 install(FILES share/scan-build/${ShareFile}
-DESTINATION share/scan-build
+DESTINATION "${CMAKE_INSTALL_DATADIR}/scan-build"
 COMPONENT scan-build)
   endforeach()
 
Index: clang/tools/scan-build-py/CMakeLists.txt
===
--- clang/tools/scan-build-py/CMakeLists.txt
+++ clang/tools/scan-build-py/CMakeLists.txt
@@ -43,7 +43,7 @@
  ${CMAKE_BINARY_DIR}/bin/scan-build-py
DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/bin/scan-build)
 install (PROGRAMS "bin/scan-build"
- DESTINATION bin
+ DESTINATION "${CMAKE_INSTALL_BINDIR}"
  RENAME scan-build-py
  COMPONENT scan-build-py)
 list(APPEND Depends ${CMAKE_BINARY_DIR}/bin/scan-build-py)
@@ -56,7 +56,7 @@
  ${CMAKE_BINARY_DIR}/bin/
DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/bin/${BinFile})
 install(PROGRAMS bin/${BinFile}
-DESTINATION bin
+DESTINATION "${CMAKE_INSTALL_BINDIR}"
   

[PATCH] D99484: [cmake] Use `GNUInstallDirs` to support custom installation dirs.

2022-01-15 Thread John Ericson via Phabricator via cfe-commits
Ericson2314 added a comment.

OK I split out D117417 , D117418 
, D117419 , 
and D117420 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99484

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


[PATCH] D117129: [clang-tidy] Extract Class IncluderClangTidyCheck

2022-01-15 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood updated this revision to Diff 400352.
LegalizeAdulthood added a comment.

Eliminate an intermediate class and move the IncludeInserter
functionality into ClangTidyCheck with an enum to indicate
opt-in to use of the IncludeInserter.


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

https://reviews.llvm.org/D117129

Files:
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tools-extra/clang-tidy/OptionEnumMapping.h
  clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
  clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.h
  
clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp
  
clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.h
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.h
  clang-tools-extra/clang-tidy/misc/UniqueptrResetReleaseCheck.cpp
  clang-tools-extra/clang-tidy/misc/UniqueptrResetReleaseCheck.h
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.h
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.h
  clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
  clang-tools-extra/clang-tidy/modernize/PassByValueCheck.h
  clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.h
  clang-tools-extra/clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceRandomShuffleCheck.h
  clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp
  clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.h
  clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.h
  clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.h
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/clang-tidy/utils/IncludeSorter.h
  clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h

Index: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
===
--- clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
+++ clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
@@ -10,7 +10,6 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_TRANSFORMER_CLANG_TIDY_CHECK_H
 
 #include "../ClangTidyCheck.h"
-#include "IncludeInserter.h"
 #include "IncludeSorter.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Tooling/Transformer/Transformer.h"
@@ -58,15 +57,9 @@
   TransformerClangTidyCheck(transformer::RewriteRule R, StringRef Name,
 ClangTidyContext *Context);
 
-  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
-   Preprocessor *ModuleExpanderPP) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) final;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) final;
 
-  /// Derived classes that override this function should call this method from
-  /// the overridden method.
-  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
-
   /// Set the rule that this check implements.  All cases in the rule must have
   /// a non-null \c Explanation, even though \c Explanation is optional for
   /// RewriteRule in general. Because the primary purpose of clang-tidy checks
@@ -78,7 +71,6 @@
 
 private:
   transformer::RewriteRule Rule;
-  IncludeInserter Inserter;
 };
 
 } // namespace utils
Index: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
@@ -29,9 +29,7 @@
 
 TransformerClangTidyCheck::TransformerClangTidyCheck(StringRef Name,
  ClangTidyContext *Context)
-: ClangTidyCheck(Name, Context),
-  Inserter(
-  Options.getLocalOrGlobal("IncludeStyle", IncludeSorter::IS_LLVM)) {}
+: ClangTidyCheck(Name, Context, CheckFeatures::UseIncludeInserter) {}
 
 // This constructor cannot dispatch to the simpler one (below), because, in
 // order to get meaningful results from `getLangOpts` and `Options`, we need the
@@ -60,11 +58,6 @@
   Rule = std::move(R);
 }
 
-void TransformerClangTidyCheck::r

[PATCH] D117129: [clang-tidy] Extract Class IncluderClangTidyCheck

2022-01-15 Thread Richard via Phabricator via cfe-commits
LegalizeAdulthood added a comment.

In D117129#3246403 , @njames93 wrote:

> There is a hurdle to overcome with this, Currently each check can have its 
> own include style - which doesn't really make sense. Hopefully people 
> actually just use the global version of the option instead.
> However if we were to coalesce all checks to use the same `IncludeInserter`, 
> we would have to force clang-tidy to just read the global version.
> Going this route it'd be best if we warned on configs that specify their own 
> IncludeStyle. I've created a draft implementation above, definitely needs 
> work though.

I pushed the functionality down into ClangTidyCheck
instead of introducing an intermediate class.  Does this
address your concern?


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

https://reviews.llvm.org/D117129

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


[PATCH] D117129: [clang-tidy] Extract Class IncluderClangTidyCheck

2022-01-15 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

In D117129#3246610 , 
@LegalizeAdulthood wrote:

> In D117129#3246403 , @njames93 
> wrote:
>
>> 
>
> I pushed the functionality down into ClangTidyCheck
> instead of introducing an intermediate class.  Does this
> address your concern?

This is a step backwards. Now every check is paying for an IncludeInserter that 
most never use. I've put D117409  up that, 
despite being a little rough round the edges, solves the problem in a way where 
there is only 1 shared Inserter.


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

https://reviews.llvm.org/D117129

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