[PATCH] D115064: [clang-format][NFC] Replace deque with vector

2021-12-04 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment.

In D115064#3170615 , @curdeius wrote:

> Maybe using `llvm::SmallVector` with some well-thought (data-based) static 
> number of elements would be better here, WDYT?

I tend to agree as LLVM style 

 prefers `SmallVector` to `vector`.


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

https://reviews.llvm.org/D115064

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


[PATCH] D115108: [clangd] Print type for VarTemplateDecl in hover.

2021-12-04 Thread liu hui via Phabricator via cfe-commits
lh123 created this revision.
lh123 added reviewers: kadircet, sammccall.
lh123 added a project: clang-tools-extra.
Herald added subscribers: usaxena95, arphaman.
lh123 requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.

Print type for `VarTemplateDecl` in hover.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115108

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -901,6 +901,35 @@
  HI.Kind = index::SymbolKind::Unknown;
  HI.Type = "int[10]";
  HI.Value = "{1}";
+   }},
+  {// Var template decl
+   R"cpp(
+  using m_int = int;
+
+  template  m_int ^[[arr]][Size];
+ )cpp",
+   [](HoverInfo &HI) {
+ HI.Name = "arr";
+ HI.Kind = index::SymbolKind::Variable;
+ HI.Type = "m_int[Size]";
+ HI.NamespaceScope = "";
+ HI.Definition = "template  m_int arr[Size]";
+ HI.TemplateParameters = {{{"int"}, {"Size"}, llvm::None}};
+   }},
+  {// Var template decl specialization
+   R"cpp(
+  using m_int = int;
+
+  template  m_int arr[Size];
+
+  template <> m_int ^[[arr]]<4>[4];
+ )cpp",
+   [](HoverInfo &HI) {
+ HI.Name = "arr<4>";
+ HI.Kind = index::SymbolKind::Variable;
+ HI.Type = "m_int[4]";
+ HI.NamespaceScope = "";
+ HI.Definition = "m_int arr[4]";
}}};
   for (const auto &Case : Cases) {
 SCOPED_TRACE(Case.Code);
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -594,6 +594,8 @@
 HI.Type = TTP->wasDeclaredWithTypename() ? "typename" : "class";
   else if (const auto *TTP = dyn_cast(D))
 HI.Type = printType(TTP, PP);
+  else if (const auto *VT = dyn_cast(D))
+HI.Type = printType(VT->getTemplatedDecl()->getType(), PP);
 
   // Fill in value with evaluated initializer if possible.
   if (const auto *Var = dyn_cast(D)) {


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -901,6 +901,35 @@
  HI.Kind = index::SymbolKind::Unknown;
  HI.Type = "int[10]";
  HI.Value = "{1}";
+   }},
+  {// Var template decl
+   R"cpp(
+  using m_int = int;
+
+  template  m_int ^[[arr]][Size];
+ )cpp",
+   [](HoverInfo &HI) {
+ HI.Name = "arr";
+ HI.Kind = index::SymbolKind::Variable;
+ HI.Type = "m_int[Size]";
+ HI.NamespaceScope = "";
+ HI.Definition = "template  m_int arr[Size]";
+ HI.TemplateParameters = {{{"int"}, {"Size"}, llvm::None}};
+   }},
+  {// Var template decl specialization
+   R"cpp(
+  using m_int = int;
+
+  template  m_int arr[Size];
+
+  template <> m_int ^[[arr]]<4>[4];
+ )cpp",
+   [](HoverInfo &HI) {
+ HI.Name = "arr<4>";
+ HI.Kind = index::SymbolKind::Variable;
+ HI.Type = "m_int[4]";
+ HI.NamespaceScope = "";
+ HI.Definition = "m_int arr[4]";
}}};
   for (const auto &Case : Cases) {
 SCOPED_TRACE(Case.Code);
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -594,6 +594,8 @@
 HI.Type = TTP->wasDeclaredWithTypename() ? "typename" : "class";
   else if (const auto *TTP = dyn_cast(D))
 HI.Type = printType(TTP, PP);
+  else if (const auto *VT = dyn_cast(D))
+HI.Type = printType(VT->getTemplatedDecl()->getType(), PP);
 
   // Fill in value with evaluated initializer if possible.
   if (const auto *Var = dyn_cast(D)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D115107: [clang][clangd] Desugar array type.

2021-12-04 Thread liu hui via Phabricator via cfe-commits
lh123 created this revision.
lh123 added reviewers: sammccall, kadircet.
lh123 added projects: clang, clang-tools-extra.
Herald added subscribers: usaxena95, arphaman.
lh123 requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.

Desugar array type.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115107

Files:
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang/lib/AST/ASTDiagnostic.cpp
  clang/test/Misc/diag-aka-types.cpp

Index: clang/test/Misc/diag-aka-types.cpp
===
--- clang/test/Misc/diag-aka-types.cpp
+++ clang/test/Misc/diag-aka-types.cpp
@@ -62,3 +62,9 @@
 void (&f3)(decltype(1 + 2)) = 0; // expected-error{{non-const lvalue reference to type 'void (decltype(1 + 2))' (aka 'void (int)') cannot bind to a temporary of type 'int'}}
 decltype(1+2) (&f4)(double, decltype(1 + 2)) = 0; // expected-error{{non-const lvalue reference to type 'decltype(1 + 2) (double, decltype(1 + 2))' (aka 'int (double, int)') cannot bind to a temporary of type 'int'}}
 auto (&f5)() -> decltype(1+2) = 0; // expected-error{{non-const lvalue reference to type 'auto () -> decltype(1 + 2)' (aka 'auto () -> int') cannot bind to a temporary of type 'int'}}
+
+using C = decltype(1+2);;
+C a6[10];
+extern C a8[];
+int a9 = a6; // expected-error{{cannot initialize a variable of type 'int' with an lvalue of type 'C[10]' (aka 'int[10]')}}
+int a10 = a8; // expected-error{{cannot initialize a variable of type 'int' with an lvalue of type 'C[]' (aka 'int[]')}}
Index: clang/lib/AST/ASTDiagnostic.cpp
===
--- clang/lib/AST/ASTDiagnostic.cpp
+++ clang/lib/AST/ASTDiagnostic.cpp
@@ -131,6 +131,38 @@
   }
 }
 
+if (const auto *CAT = dyn_cast(Ty)) {
+  QualType ElementTy =
+  desugarForDiagnostic(Context, CAT->getElementType(), ShouldAKA);
+  QT = Context.getConstantArrayType(
+  ElementTy, CAT->getSize(), CAT->getSizeExpr(), CAT->getSizeModifier(),
+  CAT->getIndexTypeCVRQualifiers());
+  break;
+}
+if (const auto *VAT = dyn_cast(Ty)) {
+  QualType ElementTy =
+  desugarForDiagnostic(Context, VAT->getElementType(), ShouldAKA);
+  QT = Context.getVariableArrayType(
+  ElementTy, VAT->getSizeExpr(), VAT->getSizeModifier(),
+  VAT->getIndexTypeCVRQualifiers(), VAT->getBracketsRange());
+  break;
+}
+if (const auto *DSAT = dyn_cast(Ty)) {
+  QualType ElementTy =
+  desugarForDiagnostic(Context, DSAT->getElementType(), ShouldAKA);
+  QT = Context.getDependentSizedArrayType(
+  ElementTy, DSAT->getSizeExpr(), DSAT->getSizeModifier(),
+  DSAT->getIndexTypeCVRQualifiers(), DSAT->getBracketsRange());
+  break;
+}
+if (const auto *IAT = dyn_cast(Ty)) {
+  QualType ElementTy =
+  desugarForDiagnostic(Context, IAT->getElementType(), ShouldAKA);
+  QT = Context.getIncompleteArrayType(ElementTy, IAT->getSizeModifier(),
+  IAT->getIndexTypeCVRQualifiers());
+  break;
+}
+
 // Don't desugar magic Objective-C types.
 if (QualType(Ty,0) == Context.getObjCIdType() ||
 QualType(Ty,0) == Context.getObjCClassType() ||
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -969,6 +969,52 @@
  HI.Definition = "template  using AA = A";
  HI.Type = {"A", "type-parameter-0-0"}; // FIXME: should be 'T'
  HI.TemplateParameters = {{{"typename"}, std::string("T"), llvm::None}};
+   }},
+  {// Constant array
+   R"cpp(
+  using m_int = int;
+
+  m_int ^[[arr]][10];
+ )cpp",
+   [](HoverInfo &HI) {
+ HI.Name = "arr";
+ HI.NamespaceScope = "";
+ HI.LocalScope = "";
+ HI.Kind = index::SymbolKind::Variable;
+ HI.Definition = "m_int arr[10]";
+ HI.Type = {"m_int[10]", "int[10]"};
+   }},
+  {// Incomplete array
+   R"cpp(
+  using m_int = int;
+
+  extern m_int ^[[arr]][];
+ )cpp",
+   [](HoverInfo &HI) {
+ HI.Name = "arr";
+ HI.NamespaceScope = "";
+ HI.LocalScope = "";
+ HI.Kind = index::SymbolKind::Variable;
+ HI.Definition = "extern m_int arr[]";
+ HI.Type = {"m_int[]", "int[]"};
+   }},
+  {// Dependent size array
+   R"cpp(
+  using m_int = int;
+
+  template
+  struct Test {
+m_int ^[[arr]][Size];
+  };
+ )cpp",
+   [](HoverInfo &HI) {
+ HI.Name = "arr";
+ HI.NamespaceScope = "";
+ HI.LocalScope = "Test::";
+ HI.AccessSpecifier = "public";
+ HI.Kind = index::SymbolKind::Field;
+   

[PATCH] D114497: [PowerPC] Drop stdlib paths in freestanding tests

2021-12-04 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

LGTM (in a trip, slow to reply)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114497

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


[PATCH] D115106: [clang-tidy] Fix `readability-static-accessed-through-instance` false negative for static methods

2021-12-04 Thread Fabian Wolff via Phabricator via cfe-commits
fwolff created this revision.
fwolff added reviewers: aaron.ballman, simon.giesecke, jcking1034.
fwolff added a project: clang-tools-extra.
Herald added subscribers: carlosgalvezp, xazax.hun.
fwolff requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes PR#52519 . The problem 
actually has nothing to do with the out-of-line definition being `inline`; the 
problem is that `hasDeclaration()` of the `memberExpr()` will match the 
out-of-line definition, which obviously isn't marked `static`, so 
`isStaticStorageClass()` won't match. To find out whether a member function is 
`static`, one needs to look at the //canonical// declaration, but there doesn't 
seem to be a matcher for that yet, so I've added an `isStatic()` narrowing 
matcher for `CXXMethodDecl`s, similar to the `isConst()` matcher that already 
exists.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115106

Files:
  
clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-static-accessed-through-instance.cpp
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2017,6 +2017,17 @@
   notMatches("struct A { void foo(); };", cxxMethodDecl(isConst(;
 }
 
+TEST_P(ASTMatchersTest, IsStatic) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+
+  EXPECT_TRUE(
+  matches("struct A { static void foo(); };", cxxMethodDecl(isStatic(;
+  EXPECT_TRUE(
+  notMatches("struct A { void foo(); };", cxxMethodDecl(isStatic(;
+}
+
 TEST_P(ASTMatchersTest, IsOverride) {
   if (!GetParam().isCXX()) {
 return;
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -451,6 +451,7 @@
   REGISTER_MATCHER(isSharedKind);
   REGISTER_MATCHER(isSignedInteger);
   REGISTER_MATCHER(isStandaloneDirective);
+  REGISTER_MATCHER(isStatic);
   REGISTER_MATCHER(isStaticLocal);
   REGISTER_MATCHER(isStaticStorageClass);
   REGISTER_MATCHER(isStruct);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -6003,6 +6003,19 @@
   return Node.isConst();
 }
 
+/// Matches if the given method declaration is static.
+///
+/// Given
+/// \code
+/// struct A {
+///   static void foo();
+///   void bar();
+/// };
+/// \endcode
+///
+/// cxxMethodDecl(isStatic()) matches A::foo() but not A::bar()
+AST_MATCHER(CXXMethodDecl, isStatic) { return Node.isStatic(); }
+
 /// Matches if the given method declaration declares a copy assignment
 /// operator.
 ///
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -3322,6 +3322,19 @@
 
 
 
+MatcherCXXMethodDecl>isStatic
+Matches if the given method declaration is static.
+
+Given
+struct A {
+  static void foo();
+  void bar();
+};
+
+cxxMethodDecl(isStatic()) matches A::foo() but not A::bar()
+
+
+
 MatcherCXXMethodDecl>isCopyAssignmentOperator
 Matches if the given method declaration declares a copy assignment
 operator.
Index: clang-tools-extra/test/clang-tidy/checkers/readability-static-accessed-through-instance.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-static-accessed-through-instance.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-static-accessed-through-instance.cpp
@@ -263,3 +263,20 @@
 // CHECK-MESSAGES-NOT: :[[@LINE-1]]:10: warning: static member
 
 } // namespace Bugzilla_48758
+
+// https://bugs.llvm.org/show_bug.cgi?id=52519
+struct PR52519 {
+  static PR52519 &getInstance();
+  static int getInt();
+};
+
+int PR52519::getInt() {
+  return 42;
+}
+
+int PR52519_bar() {
+  auto &foo = PR52519::getInstance();
+  return foo.getInt();
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: static member accessed through instance [readability-static-accessed-through-instance]
+  // CHECK-FIXES: {{^}}  return PR52519::getInt();{{$}}
+}
Index: clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
==

[PATCH] D114497: [PowerPC] Drop stdlib paths in freestanding tests

2021-12-04 Thread Zhihao Yuan via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG41a0e850fa30: [PowerPC] Drop stdlib paths in freestanding 
tests (authored by lichray).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114497

Files:
  clang/test/CodeGen/ppc-xmmintrin.c


Index: clang/test/CodeGen/ppc-xmmintrin.c
===
--- clang/test/CodeGen/ppc-xmmintrin.c
+++ clang/test/CodeGen/ppc-xmmintrin.c
@@ -10,13 +10,13 @@
 // RUN: %clang -x c++ -fsyntax-only -target powerpc64le-unknown-linux-gnu 
-mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
 // RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns
 
-// RUN: %clang -S -emit-llvm -target powerpc64-unknown-freebsd13.0 -mcpu=pwr8 
-ffreestanding -DNO_WARN_X86_INTRINSICS %s \
+// RUN: %clang -S -emit-llvm -target powerpc64-unknown-freebsd13.0 -mcpu=pwr8 
-ffreestanding -nostdlibinc -DNO_WARN_X86_INTRINSICS %s \
 // RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | 
llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-BE
-// RUN: %clang -x c++ -fsyntax-only -target powerpc64-unknown-freebsd13.0 
-mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
+// RUN: %clang -x c++ -fsyntax-only -target powerpc64-unknown-freebsd13.0 
-mcpu=pwr8 -ffreestanding -nostdlibinc -DNO_WARN_X86_INTRINSICS %s \
 // RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns
-// RUN: %clang -S -emit-llvm -target powerpc64le-unknown-freebsd13.0 
-mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
+// RUN: %clang -S -emit-llvm -target powerpc64le-unknown-freebsd13.0 
-mcpu=pwr8 -ffreestanding -nostdlibinc -DNO_WARN_X86_INTRINSICS %s \
 // RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | 
llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-LE
-// RUN: %clang -x c++ -fsyntax-only -target powerpc64le-unknown-freebsd13.0 
-mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
+// RUN: %clang -x c++ -fsyntax-only -target powerpc64le-unknown-freebsd13.0 
-mcpu=pwr8 -ffreestanding -nostdlibinc -DNO_WARN_X86_INTRINSICS %s \
 // RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns
 
 #include 


Index: clang/test/CodeGen/ppc-xmmintrin.c
===
--- clang/test/CodeGen/ppc-xmmintrin.c
+++ clang/test/CodeGen/ppc-xmmintrin.c
@@ -10,13 +10,13 @@
 // RUN: %clang -x c++ -fsyntax-only -target powerpc64le-unknown-linux-gnu -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
 // RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns
 
-// RUN: %clang -S -emit-llvm -target powerpc64-unknown-freebsd13.0 -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
+// RUN: %clang -S -emit-llvm -target powerpc64-unknown-freebsd13.0 -mcpu=pwr8 -ffreestanding -nostdlibinc -DNO_WARN_X86_INTRINSICS %s \
 // RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-BE
-// RUN: %clang -x c++ -fsyntax-only -target powerpc64-unknown-freebsd13.0 -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
+// RUN: %clang -x c++ -fsyntax-only -target powerpc64-unknown-freebsd13.0 -mcpu=pwr8 -ffreestanding -nostdlibinc -DNO_WARN_X86_INTRINSICS %s \
 // RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns
-// RUN: %clang -S -emit-llvm -target powerpc64le-unknown-freebsd13.0 -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
+// RUN: %clang -S -emit-llvm -target powerpc64le-unknown-freebsd13.0 -mcpu=pwr8 -ffreestanding -nostdlibinc -DNO_WARN_X86_INTRINSICS %s \
 // RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-LE
-// RUN: %clang -x c++ -fsyntax-only -target powerpc64le-unknown-freebsd13.0 -mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
+// RUN: %clang -x c++ -fsyntax-only -target powerpc64le-unknown-freebsd13.0 -mcpu=pwr8 -ffreestanding -nostdlibinc -DNO_WARN_X86_INTRINSICS %s \
 // RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns
 
 #include 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 41a0e85 - [PowerPC] Drop stdlib paths in freestanding tests

2021-12-04 Thread Zhihao Yuan via cfe-commits

Author: Zhihao Yuan
Date: 2021-12-04T16:51:13-06:00
New Revision: 41a0e850fa30acf2ffd1c4ffda335f07ea0c249b

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

LOG: [PowerPC] Drop stdlib paths in freestanding tests

When targeting FreeBSD on a Linux host with a copy
of system libc++, Clang prepends /usr/include/c++/v1
to the search paths even with -ffreestanding, and
fails to compile a program with a
single #include 

Dropping the path with -nostdlibinc.

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

Added: 


Modified: 
clang/test/CodeGen/ppc-xmmintrin.c

Removed: 




diff  --git a/clang/test/CodeGen/ppc-xmmintrin.c 
b/clang/test/CodeGen/ppc-xmmintrin.c
index 158c235c7d894..07bbe71667c98 100644
--- a/clang/test/CodeGen/ppc-xmmintrin.c
+++ b/clang/test/CodeGen/ppc-xmmintrin.c
@@ -10,13 +10,13 @@
 // RUN: %clang -x c++ -fsyntax-only -target powerpc64le-unknown-linux-gnu 
-mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
 // RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns
 
-// RUN: %clang -S -emit-llvm -target powerpc64-unknown-freebsd13.0 -mcpu=pwr8 
-ffreestanding -DNO_WARN_X86_INTRINSICS %s \
+// RUN: %clang -S -emit-llvm -target powerpc64-unknown-freebsd13.0 -mcpu=pwr8 
-ffreestanding -nostdlibinc -DNO_WARN_X86_INTRINSICS %s \
 // RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | 
llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-BE
-// RUN: %clang -x c++ -fsyntax-only -target powerpc64-unknown-freebsd13.0 
-mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
+// RUN: %clang -x c++ -fsyntax-only -target powerpc64-unknown-freebsd13.0 
-mcpu=pwr8 -ffreestanding -nostdlibinc -DNO_WARN_X86_INTRINSICS %s \
 // RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns
-// RUN: %clang -S -emit-llvm -target powerpc64le-unknown-freebsd13.0 
-mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
+// RUN: %clang -S -emit-llvm -target powerpc64le-unknown-freebsd13.0 
-mcpu=pwr8 -ffreestanding -nostdlibinc -DNO_WARN_X86_INTRINSICS %s \
 // RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | 
llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-LE
-// RUN: %clang -x c++ -fsyntax-only -target powerpc64le-unknown-freebsd13.0 
-mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
+// RUN: %clang -x c++ -fsyntax-only -target powerpc64le-unknown-freebsd13.0 
-mcpu=pwr8 -ffreestanding -nostdlibinc -DNO_WARN_X86_INTRINSICS %s \
 // RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns
 
 #include 



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


[PATCH] D115060: [clang-format][NFC] Code Tidies in UnwrappedLineFormatter

2021-12-04 Thread Owen Pan via Phabricator via cfe-commits
owenpan accepted this revision.
owenpan added a comment.

LGTM.


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

https://reviews.llvm.org/D115060

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


[PATCH] D115103: Leak Sanitizer port to Windows

2021-12-04 Thread Clemens Wasser via Phabricator via cfe-commits
clemenswasser updated this revision to Diff 391875.
clemenswasser added a comment.

Use LF line endings


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

https://reviews.llvm.org/D115103

Files:
  clang/lib/Driver/ToolChains/MSVC.cpp
  compiler-rt/cmake/config-ix.cmake
  compiler-rt/lib/lsan/CMakeLists.txt
  compiler-rt/lib/lsan/lsan.h
  compiler-rt/lib/lsan/lsan_allocator.cpp
  compiler-rt/lib/lsan/lsan_common.cpp
  compiler-rt/lib/lsan/lsan_common.h
  compiler-rt/lib/lsan/lsan_common_win.cpp
  compiler-rt/lib/lsan/lsan_interceptors.cpp
  compiler-rt/lib/lsan/lsan_win.cpp
  compiler-rt/lib/lsan/lsan_win.h
  compiler-rt/lib/sanitizer_common/CMakeLists.txt
  compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_win.cpp
  compiler-rt/test/lsan/lit.common.cfg.py

Index: compiler-rt/test/lsan/lit.common.cfg.py
===
--- compiler-rt/test/lsan/lit.common.cfg.py
+++ compiler-rt/test/lsan/lit.common.cfg.py
@@ -79,7 +79,8 @@
 supported_linux = (not config.android) and config.host_os == 'Linux' and config.host_arch in ['aarch64', 'x86_64', 'ppc64', 'ppc64le', 'mips64', 'riscv64', 'arm', 'armhf', 'armv7l', 's390x']
 supported_darwin = config.host_os == 'Darwin' and config.target_arch in ['x86_64']
 supported_netbsd = config.host_os == 'NetBSD' and config.target_arch in ['x86_64', 'i386']
-if not (supported_android or supported_linux or supported_darwin or supported_netbsd):
+supported_windows = config.host_os == 'Windows'
+if not (supported_android or supported_linux or supported_darwin or supported_netbsd or supported_windows):
   config.unsupported = True
 
 # Don't support Thumb due to broken fast unwinder
Index: compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_win.cpp
===
--- /dev/null
+++ compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_win.cpp
@@ -0,0 +1,138 @@
+//===-- sanitizer_stoptheworld_win.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// See sanitizer_stoptheworld.h for details.
+//
+//===--===//
+
+#include "sanitizer_common.h"
+#include "sanitizer_internal_defs.h"
+#include "sanitizer_platform.h"
+
+#if SANITIZER_WINDOWS
+
+#  define WIN32_LEAN_AND_MEAN
+#  include 
+
+// windows.h needs to be included before tlhelp32.h
+#  include 
+
+#  include "sanitizer_stoptheworld.h"
+
+namespace __sanitizer {
+
+struct SuspendedThreadsListWindows final : public SuspendedThreadsList {
+  InternalMmapVector threadHandles;
+  InternalMmapVector threadIds;
+
+  SuspendedThreadsListWindows() {
+threadIds.reserve(1024);
+threadHandles.reserve(1024);
+  }
+
+  PtraceRegistersStatus GetRegistersAndSP(uptr index,
+  InternalMmapVector *buffer,
+  uptr *sp) const override;
+
+  tid_t GetThreadID(uptr index) const override;
+  uptr ThreadCount() const override;
+};
+
+PtraceRegistersStatus SuspendedThreadsListWindows::GetRegistersAndSP(
+uptr index, InternalMmapVector *buffer, uptr *sp) const {
+  CHECK_LT(index, threadHandles.size());
+
+  CONTEXT thread_context;
+  thread_context.ContextFlags = CONTEXT_ALL;
+  CHECK(GetThreadContext(threadHandles[index], &thread_context));
+
+  buffer->resize(RoundUpTo(sizeof(thread_context), sizeof(uptr)) /
+ sizeof(uptr));
+  internal_memcpy(buffer->data(), &thread_context, sizeof(thread_context));
+
+  *sp = thread_context.Rsp;
+
+  return REGISTERS_AVAILABLE;
+}
+
+tid_t SuspendedThreadsListWindows::GetThreadID(uptr index) const {
+  CHECK_LT(index, threadIds.size());
+  return threadIds[index];
+}
+
+uptr SuspendedThreadsListWindows::ThreadCount() const {
+  return threadIds.size();
+}
+
+struct RunThreadArgs {
+  StopTheWorldCallback callback;
+  void *argument;
+};
+
+DWORD WINAPI RunThread(void *argument) {
+  RunThreadArgs *run_args = (RunThreadArgs *)argument;
+  const HANDLE threads = CreateToolhelp32Snapshot(TH32CS_SNAPTHREAD, 0);
+
+  CHECK(threads != INVALID_HANDLE_VALUE);
+
+  const DWORD this_thread = GetCurrentThreadId();
+  const DWORD this_process = GetCurrentProcessId();
+
+  SuspendedThreadsListWindows suspended_threads_list;
+
+  THREADENTRY32 thread_entry;
+  thread_entry.dwSize = sizeof(thread_entry);
+  if (Thread32First(threads, &thread_entry)) {
+do {
+  if (thread_entry.dwSize >=
+  FIELD_OFFSET(THREADENTRY32, th32OwnerProcessID) +
+  sizeof(thread_entry.th32OwnerProcessID)) {
+if (thread_entry.th32ThreadID == this_thread ||
+thread_entry.th32OwnerProcessID != this_process)
+  cont

[PATCH] D115103: Leak Sanitizer port to Windows

2021-12-04 Thread Clemens Wasser via Phabricator via cfe-commits
clemenswasser updated this revision to Diff 391873.
clemenswasser added a comment.

Apply clang-format


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

https://reviews.llvm.org/D115103

Files:
  clang/lib/Driver/ToolChains/MSVC.cpp
  compiler-rt/cmake/config-ix.cmake
  compiler-rt/lib/lsan/CMakeLists.txt
  compiler-rt/lib/lsan/lsan.h
  compiler-rt/lib/lsan/lsan_allocator.cpp
  compiler-rt/lib/lsan/lsan_common.cpp
  compiler-rt/lib/lsan/lsan_common.h
  compiler-rt/lib/lsan/lsan_common_win.cpp
  compiler-rt/lib/lsan/lsan_interceptors.cpp
  compiler-rt/lib/lsan/lsan_win.cpp
  compiler-rt/lib/lsan/lsan_win.h
  compiler-rt/lib/sanitizer_common/CMakeLists.txt
  compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_win.cpp
  compiler-rt/test/lsan/lit.common.cfg.py

Index: compiler-rt/test/lsan/lit.common.cfg.py
===
--- compiler-rt/test/lsan/lit.common.cfg.py
+++ compiler-rt/test/lsan/lit.common.cfg.py
@@ -79,7 +79,8 @@
 supported_linux = (not config.android) and config.host_os == 'Linux' and config.host_arch in ['aarch64', 'x86_64', 'ppc64', 'ppc64le', 'mips64', 'riscv64', 'arm', 'armhf', 'armv7l', 's390x']
 supported_darwin = config.host_os == 'Darwin' and config.target_arch in ['x86_64']
 supported_netbsd = config.host_os == 'NetBSD' and config.target_arch in ['x86_64', 'i386']
-if not (supported_android or supported_linux or supported_darwin or supported_netbsd):
+supported_windows = config.host_os == 'Windows'
+if not (supported_android or supported_linux or supported_darwin or supported_netbsd or supported_windows):
   config.unsupported = True
 
 # Don't support Thumb due to broken fast unwinder
Index: compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_win.cpp
===
--- /dev/null
+++ compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_win.cpp
@@ -0,0 +1,138 @@
+//===-- sanitizer_stoptheworld_win.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// See sanitizer_stoptheworld.h for details.
+//
+//===--===//
+
+#include "sanitizer_common.h"
+#include "sanitizer_internal_defs.h"
+#include "sanitizer_platform.h"
+
+#if SANITIZER_WINDOWS
+
+#  define WIN32_LEAN_AND_MEAN
+#  include 
+
+// windows.h needs to be included before tlhelp32.h
+#  include 
+
+#  include "sanitizer_stoptheworld.h"
+
+namespace __sanitizer {
+
+struct SuspendedThreadsListWindows final : public SuspendedThreadsList {
+  InternalMmapVector threadHandles;
+  InternalMmapVector threadIds;
+
+  SuspendedThreadsListWindows() {
+threadIds.reserve(1024);
+threadHandles.reserve(1024);
+  }
+
+  PtraceRegistersStatus GetRegistersAndSP(uptr index,
+  InternalMmapVector *buffer,
+  uptr *sp) const override;
+
+  tid_t GetThreadID(uptr index) const override;
+  uptr ThreadCount() const override;
+};
+
+PtraceRegistersStatus SuspendedThreadsListWindows::GetRegistersAndSP(
+uptr index, InternalMmapVector *buffer, uptr *sp) const {
+  CHECK_LT(index, threadHandles.size());
+
+  CONTEXT thread_context;
+  thread_context.ContextFlags = CONTEXT_ALL;
+  CHECK(GetThreadContext(threadHandles[index], &thread_context));
+
+  buffer->resize(RoundUpTo(sizeof(thread_context), sizeof(uptr)) /
+ sizeof(uptr));
+  internal_memcpy(buffer->data(), &thread_context, sizeof(thread_context));
+
+  *sp = thread_context.Rsp;
+
+  return REGISTERS_AVAILABLE;
+}
+
+tid_t SuspendedThreadsListWindows::GetThreadID(uptr index) const {
+  CHECK_LT(index, threadIds.size());
+  return threadIds[index];
+}
+
+uptr SuspendedThreadsListWindows::ThreadCount() const {
+  return threadIds.size();
+}
+
+struct RunThreadArgs {
+  StopTheWorldCallback callback;
+  void *argument;
+};
+
+DWORD WINAPI RunThread(void *argument) {
+  RunThreadArgs *run_args = (RunThreadArgs *)argument;
+  const HANDLE threads = CreateToolhelp32Snapshot(TH32CS_SNAPTHREAD, 0);
+
+  CHECK(threads != INVALID_HANDLE_VALUE);
+
+  const DWORD this_thread = GetCurrentThreadId();
+  const DWORD this_process = GetCurrentProcessId();
+
+  SuspendedThreadsListWindows suspended_threads_list;
+
+  THREADENTRY32 thread_entry;
+  thread_entry.dwSize = sizeof(thread_entry);
+  if (Thread32First(threads, &thread_entry)) {
+do {
+  if (thread_entry.dwSize >=
+  FIELD_OFFSET(THREADENTRY32, th32OwnerProcessID) +
+  sizeof(thread_entry.th32OwnerProcessID)) {
+if (thread_entry.th32ThreadID == this_thread ||
+thread_entry.th32OwnerProcessID != this_process)
+  conti

[PATCH] D115103: Leak Sanitizer port to Windows

2021-12-04 Thread Clemens Wasser via Phabricator via cfe-commits
clemenswasser updated this revision to Diff 391872.
clemenswasser added a comment.

Move `__lsan_init` call inside of `SANITIZER_WINDOWS` ifdef


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

https://reviews.llvm.org/D115103

Files:
  clang/lib/Driver/ToolChains/MSVC.cpp
  compiler-rt/cmake/config-ix.cmake
  compiler-rt/lib/lsan/CMakeLists.txt
  compiler-rt/lib/lsan/lsan.h
  compiler-rt/lib/lsan/lsan_allocator.cpp
  compiler-rt/lib/lsan/lsan_common.cpp
  compiler-rt/lib/lsan/lsan_common.h
  compiler-rt/lib/lsan/lsan_common_win.cpp
  compiler-rt/lib/lsan/lsan_interceptors.cpp
  compiler-rt/lib/lsan/lsan_win.cpp
  compiler-rt/lib/lsan/lsan_win.h
  compiler-rt/lib/sanitizer_common/CMakeLists.txt
  compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_win.cpp
  compiler-rt/test/lsan/lit.common.cfg.py

Index: compiler-rt/test/lsan/lit.common.cfg.py
===
--- compiler-rt/test/lsan/lit.common.cfg.py
+++ compiler-rt/test/lsan/lit.common.cfg.py
@@ -79,7 +79,8 @@
 supported_linux = (not config.android) and config.host_os == 'Linux' and config.host_arch in ['aarch64', 'x86_64', 'ppc64', 'ppc64le', 'mips64', 'riscv64', 'arm', 'armhf', 'armv7l', 's390x']
 supported_darwin = config.host_os == 'Darwin' and config.target_arch in ['x86_64']
 supported_netbsd = config.host_os == 'NetBSD' and config.target_arch in ['x86_64', 'i386']
-if not (supported_android or supported_linux or supported_darwin or supported_netbsd):
+supported_windows = config.host_os == 'Windows'
+if not (supported_android or supported_linux or supported_darwin or supported_netbsd or supported_windows):
   config.unsupported = True
 
 # Don't support Thumb due to broken fast unwinder
Index: compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_win.cpp
===
--- /dev/null
+++ compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_win.cpp
@@ -0,0 +1,137 @@
+//===-- sanitizer_stoptheworld_win.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// See sanitizer_stoptheworld.h for details.
+//
+//===--===//
+
+#include "sanitizer_common.h"
+#include "sanitizer_internal_defs.h"
+#include "sanitizer_platform.h"
+
+#if SANITIZER_WINDOWS
+
+#define WIN32_LEAN_AND_MEAN
+#include 
+
+#include 
+
+#include "sanitizer_stoptheworld.h"
+
+
+namespace __sanitizer {
+
+struct SuspendedThreadsListWindows final : public SuspendedThreadsList {
+  InternalMmapVector threadHandles;
+  InternalMmapVector threadIds;
+
+  SuspendedThreadsListWindows() {
+threadIds.reserve(1024);
+threadHandles.reserve(1024);
+  }
+
+  PtraceRegistersStatus GetRegistersAndSP(uptr index,
+  InternalMmapVector *buffer,
+  uptr *sp) const override;
+
+  tid_t GetThreadID(uptr index) const override;
+  uptr ThreadCount() const override;
+};
+
+PtraceRegistersStatus SuspendedThreadsListWindows::GetRegistersAndSP(
+uptr index, InternalMmapVector *buffer, uptr *sp) const {
+  CHECK_LT(index, threadHandles.size());
+
+  CONTEXT thread_context;
+  thread_context.ContextFlags = CONTEXT_ALL;
+  CHECK(GetThreadContext(threadHandles[index], &thread_context));
+
+  buffer->resize(RoundUpTo(sizeof(thread_context), sizeof(uptr)) / sizeof(uptr));
+  internal_memcpy(buffer->data(), &thread_context, sizeof(thread_context));
+
+  *sp = thread_context.Rsp;
+
+  return REGISTERS_AVAILABLE;
+}
+
+tid_t SuspendedThreadsListWindows::GetThreadID(uptr index) const {
+  CHECK_LT(index, threadIds.size());
+  return threadIds[index];
+}
+
+uptr SuspendedThreadsListWindows::ThreadCount() const {
+  return threadIds.size();
+}
+
+struct RunThreadArgs {
+  StopTheWorldCallback callback;
+  void *argument;
+};
+
+DWORD WINAPI RunThread(void *argument) {
+  RunThreadArgs *run_args = (RunThreadArgs *)argument;
+  const HANDLE threads = CreateToolhelp32Snapshot(TH32CS_SNAPTHREAD, 0);
+
+  CHECK(threads != INVALID_HANDLE_VALUE);
+
+  const DWORD this_thread = GetCurrentThreadId();
+  const DWORD this_process = GetCurrentProcessId();
+
+  SuspendedThreadsListWindows suspended_threads_list;
+
+  THREADENTRY32 thread_entry;
+  thread_entry.dwSize = sizeof(thread_entry);
+  if (Thread32First(threads, &thread_entry)) {
+do {
+  if (thread_entry.dwSize >=
+  FIELD_OFFSET(THREADENTRY32, th32OwnerProcessID) +
+  sizeof(thread_entry.th32OwnerProcessID)) {
+if (thread_entry.th32ThreadID == this_thread ||
+thread_entry.th32OwnerProcessID != this_process)
+  continue;
+
+const HANDLE thread 

[PATCH] D113863: [clang-tidy] Make `readability-container-data-pointer` more robust

2021-12-04 Thread Fabian Wolff via Phabricator via cfe-commits
fwolff updated this revision to Diff 391871.

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

https://reviews.llvm.org/D113863

Files:
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp
@@ -109,3 +109,38 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
   // CHECK-FIXES: {{^  }}return v.data();{{$}}
 }
+
+template 
+struct container_without_data {
+  using size_type = size_t;
+  T &operator[](size_type);
+  const T &operator[](size_type) const;
+};
+
+template 
+const T *n(const container_without_data &c) {
+  // c has no "data" member function, so there should not be a warning here:
+  return &c[0];
+}
+
+const int *o(const std::vector>> &v, const size_t idx1, const size_t idx2) {
+  return &v[idx1][idx2][0];
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}return v[idx1][idx2].data();{{$}}
+}
+
+std::vector &select(std::vector &u, std::vector &v) {
+  return v;
+}
+
+int *p(std::vector &v1, std::vector &v2) {
+  return &select(*&v1, v2)[0];
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}return select(*&v1, v2).data();{{$}}
+}
+
+int *q(std::vector ***v) {
+  return &(***v)[0];
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}return (**v)->data();{{$}}
+}
Index: clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
@@ -16,6 +16,13 @@
 namespace clang {
 namespace tidy {
 namespace readability {
+
+constexpr llvm::StringLiteral ContainerExprName = "container-expr";
+constexpr llvm::StringLiteral DerefContainerExprName = "deref-container-expr";
+constexpr llvm::StringLiteral AddrOfContainerExprName =
+"addr-of-container-expr";
+constexpr llvm::StringLiteral AddressOfName = "address-of";
+
 ContainerDataPointerCheck::ContainerDataPointerCheck(StringRef Name,
  ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context) {}
@@ -38,69 +45,65 @@
   const auto Container =
   qualType(anyOf(NonTemplateContainerType, TemplateContainerType));
 
+  const auto ContainerExpr = anyOf(
+  unaryOperator(
+  hasOperatorName("*"),
+  hasUnaryOperand(
+  expr(hasType(pointsTo(Container))).bind(DerefContainerExprName)))
+  .bind(ContainerExprName),
+  unaryOperator(hasOperatorName("&"),
+hasUnaryOperand(expr(anyOf(hasType(Container),
+   hasType(references(Container
+.bind(AddrOfContainerExprName)))
+  .bind(ContainerExprName),
+  expr(anyOf(hasType(Container), hasType(pointsTo(Container)),
+ hasType(references(Container
+  .bind(ContainerExprName));
+
+  const auto Zero = integerLiteral(equals(0));
+
+  const auto SubscriptOperator = callee(cxxMethodDecl(hasName("operator[]")));
+
   Finder->addMatcher(
   unaryOperator(
   unless(isExpansionInSystemHeader()), hasOperatorName("&"),
-  hasUnaryOperand(anyOf(
-  ignoringParenImpCasts(
-  cxxOperatorCallExpr(
-  callee(cxxMethodDecl(hasName("operator[]"))
- .bind("operator[]")),
-  argumentCountIs(2),
-  hasArgument(
-  0,
-  anyOf(ignoringParenImpCasts(
-declRefExpr(
-to(varDecl(anyOf(
-hasType(Container),
-hasType(references(Container))
-.bind("var")),
-ignoringParenImpCasts(hasDescendant(
-  

[PATCH] D115103: Leak Sanitizer port to Windows

2021-12-04 Thread Clemens Wasser via Phabricator via cfe-commits
clemenswasser updated this revision to Diff 391870.
clemenswasser added a comment.

Changed to core.autocrlf=false


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

https://reviews.llvm.org/D115103

Files:
  clang/lib/Driver/ToolChains/MSVC.cpp
  compiler-rt/cmake/config-ix.cmake
  compiler-rt/lib/lsan/CMakeLists.txt
  compiler-rt/lib/lsan/lsan.h
  compiler-rt/lib/lsan/lsan_allocator.cpp
  compiler-rt/lib/lsan/lsan_common.cpp
  compiler-rt/lib/lsan/lsan_common.h
  compiler-rt/lib/lsan/lsan_common_win.cpp
  compiler-rt/lib/lsan/lsan_interceptors.cpp
  compiler-rt/lib/lsan/lsan_win.cpp
  compiler-rt/lib/lsan/lsan_win.h
  compiler-rt/lib/sanitizer_common/CMakeLists.txt
  compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_win.cpp
  compiler-rt/test/lsan/lit.common.cfg.py

Index: compiler-rt/test/lsan/lit.common.cfg.py
===
--- compiler-rt/test/lsan/lit.common.cfg.py
+++ compiler-rt/test/lsan/lit.common.cfg.py
@@ -79,7 +79,8 @@
 supported_linux = (not config.android) and config.host_os == 'Linux' and config.host_arch in ['aarch64', 'x86_64', 'ppc64', 'ppc64le', 'mips64', 'riscv64', 'arm', 'armhf', 'armv7l', 's390x']
 supported_darwin = config.host_os == 'Darwin' and config.target_arch in ['x86_64']
 supported_netbsd = config.host_os == 'NetBSD' and config.target_arch in ['x86_64', 'i386']
-if not (supported_android or supported_linux or supported_darwin or supported_netbsd):
+supported_windows = config.host_os == 'Windows'
+if not (supported_android or supported_linux or supported_darwin or supported_netbsd or supported_windows):
   config.unsupported = True
 
 # Don't support Thumb due to broken fast unwinder
Index: compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_win.cpp
===
--- /dev/null
+++ compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_win.cpp
@@ -0,0 +1,137 @@
+//===-- sanitizer_stoptheworld_win.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// See sanitizer_stoptheworld.h for details.
+//
+//===--===//
+
+#include "sanitizer_common.h"
+#include "sanitizer_internal_defs.h"
+#include "sanitizer_platform.h"
+
+#if SANITIZER_WINDOWS
+
+#define WIN32_LEAN_AND_MEAN
+#include 
+
+#include 
+
+#include "sanitizer_stoptheworld.h"
+
+
+namespace __sanitizer {
+
+struct SuspendedThreadsListWindows final : public SuspendedThreadsList {
+  InternalMmapVector threadHandles;
+  InternalMmapVector threadIds;
+
+  SuspendedThreadsListWindows() {
+threadIds.reserve(1024);
+threadHandles.reserve(1024);
+  }
+
+  PtraceRegistersStatus GetRegistersAndSP(uptr index,
+  InternalMmapVector *buffer,
+  uptr *sp) const override;
+
+  tid_t GetThreadID(uptr index) const override;
+  uptr ThreadCount() const override;
+};
+
+PtraceRegistersStatus SuspendedThreadsListWindows::GetRegistersAndSP(
+uptr index, InternalMmapVector *buffer, uptr *sp) const {
+  CHECK_LT(index, threadHandles.size());
+
+  CONTEXT thread_context;
+  thread_context.ContextFlags = CONTEXT_ALL;
+  CHECK(GetThreadContext(threadHandles[index], &thread_context));
+
+  buffer->resize(RoundUpTo(sizeof(thread_context), sizeof(uptr)) / sizeof(uptr));
+  internal_memcpy(buffer->data(), &thread_context, sizeof(thread_context));
+
+  *sp = thread_context.Rsp;
+
+  return REGISTERS_AVAILABLE;
+}
+
+tid_t SuspendedThreadsListWindows::GetThreadID(uptr index) const {
+  CHECK_LT(index, threadIds.size());
+  return threadIds[index];
+}
+
+uptr SuspendedThreadsListWindows::ThreadCount() const {
+  return threadIds.size();
+}
+
+struct RunThreadArgs {
+  StopTheWorldCallback callback;
+  void *argument;
+};
+
+DWORD WINAPI RunThread(void *argument) {
+  RunThreadArgs *run_args = (RunThreadArgs *)argument;
+  const HANDLE threads = CreateToolhelp32Snapshot(TH32CS_SNAPTHREAD, 0);
+
+  CHECK(threads != INVALID_HANDLE_VALUE);
+
+  const DWORD this_thread = GetCurrentThreadId();
+  const DWORD this_process = GetCurrentProcessId();
+
+  SuspendedThreadsListWindows suspended_threads_list;
+
+  THREADENTRY32 thread_entry;
+  thread_entry.dwSize = sizeof(thread_entry);
+  if (Thread32First(threads, &thread_entry)) {
+do {
+  if (thread_entry.dwSize >=
+  FIELD_OFFSET(THREADENTRY32, th32OwnerProcessID) +
+  sizeof(thread_entry.th32OwnerProcessID)) {
+if (thread_entry.th32ThreadID == this_thread ||
+thread_entry.th32OwnerProcessID != this_process)
+  continue;
+
+const HANDLE thread =
+OpenThread(THR

[PATCH] D113863: [clang-tidy] Make `readability-container-data-pointer` more robust

2021-12-04 Thread Fabian Wolff via Phabricator via cfe-commits
fwolff marked 3 inline comments as done.
fwolff added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp:76
+arraySubscriptExpr(hasLHS(ContainerExpr), hasRHS(Zero))
+  .bind(AddressOfName),
   this);

compnerd wrote:
> Nice, I do like this, it is quite a bit more succinct.  One thing that is a 
> bit less clear to me is that the previous expression was a bit more 
> restrictive in the parameter types to certain expressions, is there a reason 
> that you don't expect that to matter much in practice?  If a malformed input 
> is provided, we could now match certain things that we didn't previously, or 
> am I not matching up the conditions carefully enough?
I have indeed changed things slightly to make everything more consistent. For 
instance, calling `&vp->operator[](0)` currently does not yield a warning ([[ 
https://godbolt.org/z/M1hGvGdWr | Godbolt link ]]), but `&v.operator[](0)` 
does, because the old check allowed a pointer to a container for a 
`cxxOperatorCallExpr()` (line 62 on the left) but not for 
`cxxMemberCallExpr()`. I think this was simply an oversight because the check 
was written in such a redundant way; my version covers both.

So yes, I have made some checks a bit less restrictive, but I think that's an 
improvement. Or which cases were you thinking of exactly?



Comment at: 
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp:99
+  Lexer::getSourceText(CharSourceRange::getTokenRange(SourceRange),
+   *Result.SourceManager, getLangOpts()));
+

compnerd wrote:
> The diff shows up odd here, is there a hard tab or is that just the rendering?
I think this is Phabricator's way of saying that the only thing that was 
changed in this line is the indentation. I haven't used any tabs.


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

https://reviews.llvm.org/D113863

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


[PATCH] D115061: [clang-format][NFC] Prefer pass by reference

2021-12-04 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment.

I share Arthur's point of view and I prefer the pointers. I don't see an 
advantage of this proposed change.
If only we had C#'s `out`...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115061

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


[PATCH] D113863: [clang-tidy] Make `readability-container-data-pointer` more robust

2021-12-04 Thread Fabian Wolff via Phabricator via cfe-commits
fwolff updated this revision to Diff 391869.
fwolff marked an inline comment as done.

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

https://reviews.llvm.org/D113863

Files:
  clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-container-data-pointer.cpp
@@ -109,3 +109,38 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
   // CHECK-FIXES: {{^  }}return v.data();{{$}}
 }
+
+template 
+struct container_without_data {
+  using size_type = size_t;
+  T &operator[](size_type);
+  const T &operator[](size_type) const;
+};
+
+template 
+const T *n(const container_without_data &c) {
+  // c has no "data" member function, so there should not be a warning here:
+  return &c[0];
+}
+
+const int *o(const std::vector>> &v, const size_t idx1, const size_t idx2) {
+  return &v[idx1][idx2][0];
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}return v[idx1][idx2].data();{{$}}
+}
+
+std::vector &select(std::vector &u, std::vector &v) {
+  return v;
+}
+
+int *p(std::vector &v1, std::vector &v2) {
+  return &select(*&v1, v2)[0];
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}return select(*&v1, v2).data();{{$}}
+}
+
+int *q(std::vector ***v) {
+  return &(***v)[0];
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'data' should be used for accessing the data pointer instead of taking the address of the 0-th element [readability-container-data-pointer]
+  // CHECK-FIXES: {{^  }}return (**v)->data();{{$}}
+}
Index: clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp
@@ -16,6 +16,12 @@
 namespace clang {
 namespace tidy {
 namespace readability {
+
+constexpr llvm::StringLiteral ContainerExprName = "container-expr";
+constexpr llvm::StringLiteral DerefContainerExprName = "deref-container-expr";
+constexpr llvm::StringLiteral AddrOfContainerExprName = "addr-of-container-expr";
+constexpr llvm::StringLiteral AddressOfName = "address-of";
+
 ContainerDataPointerCheck::ContainerDataPointerCheck(StringRef Name,
  ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context) {}
@@ -38,69 +44,65 @@
   const auto Container =
   qualType(anyOf(NonTemplateContainerType, TemplateContainerType));
 
+  const auto ContainerExpr = anyOf(
+  unaryOperator(
+  hasOperatorName("*"),
+  hasUnaryOperand(
+  expr(hasType(pointsTo(Container))).bind(DerefContainerExprName)))
+  .bind(ContainerExprName),
+  unaryOperator(hasOperatorName("&"),
+hasUnaryOperand(expr(anyOf(hasType(Container),
+   hasType(references(Container
+.bind(AddrOfContainerExprName)))
+  .bind(ContainerExprName),
+  expr(anyOf(hasType(Container), hasType(pointsTo(Container)),
+ hasType(references(Container
+  .bind(ContainerExprName));
+
+  const auto Zero = integerLiteral(equals(0));
+
+  const auto SubscriptOperator = callee(cxxMethodDecl(hasName("operator[]")));
+
   Finder->addMatcher(
   unaryOperator(
   unless(isExpansionInSystemHeader()), hasOperatorName("&"),
-  hasUnaryOperand(anyOf(
-  ignoringParenImpCasts(
-  cxxOperatorCallExpr(
-  callee(cxxMethodDecl(hasName("operator[]"))
- .bind("operator[]")),
-  argumentCountIs(2),
-  hasArgument(
-  0,
-  anyOf(ignoringParenImpCasts(
-declRefExpr(
-to(varDecl(anyOf(
-hasType(Container),
-hasType(references(Container))
-.bind("var")),
-ignoringPare

[PATCH] D115061: [clang-format][NFC] Prefer pass by reference

2021-12-04 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment.

This was not intended to be pushed, I forgot to move it out of the chunk. If we 
come to the conclusion that we want pointers I will revert this. If no one 
objects in the next say 3 days I will leave it so.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115061

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


[PATCH] D115072: [clang-format][NFC] Use member directly

2021-12-04 Thread Björn Schäpers via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6e8678903523: [clang-format][NFC] Use member directly 
(authored by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115072

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp


Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -62,7 +62,7 @@
   Indent = Line.Level * IndentWidth + AdditionalIndent;
 } else {
   IndentForLevel.resize(Line.Level + 1);
-  Indent = getIndent(IndentForLevel, Line.Level);
+  Indent = getIndent(Line.Level);
 }
 if (static_cast(Indent) + Offset >= 0)
   Indent += Offset;
@@ -118,12 +118,12 @@
   /// \p IndentForLevel must contain the indent for the level \c l
   /// at \p IndentForLevel[l], or a value < 0 if the indent for
   /// that level is unknown.
-  unsigned getIndent(ArrayRef IndentForLevel, unsigned Level) {
+  unsigned getIndent(unsigned Level) const {
 if (IndentForLevel[Level] != -1)
   return IndentForLevel[Level];
 if (Level == 0)
   return 0;
-return getIndent(IndentForLevel, Level - 1) + Style.IndentWidth;
+return getIndent(Level - 1) + Style.IndentWidth;
   }
 
   const FormatStyle &Style;


Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -62,7 +62,7 @@
   Indent = Line.Level * IndentWidth + AdditionalIndent;
 } else {
   IndentForLevel.resize(Line.Level + 1);
-  Indent = getIndent(IndentForLevel, Line.Level);
+  Indent = getIndent(Line.Level);
 }
 if (static_cast(Indent) + Offset >= 0)
   Indent += Offset;
@@ -118,12 +118,12 @@
   /// \p IndentForLevel must contain the indent for the level \c l
   /// at \p IndentForLevel[l], or a value < 0 if the indent for
   /// that level is unknown.
-  unsigned getIndent(ArrayRef IndentForLevel, unsigned Level) {
+  unsigned getIndent(unsigned Level) const {
 if (IndentForLevel[Level] != -1)
   return IndentForLevel[Level];
 if (Level == 0)
   return 0;
-return getIndent(IndentForLevel, Level - 1) + Style.IndentWidth;
+return getIndent(Level - 1) + Style.IndentWidth;
   }
 
   const FormatStyle &Style;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D115071: [clang-format][NFC] Use range based for for fake l parens

2021-12-04 Thread Björn Schäpers via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG88fa4bfe1ec2: [clang-format][NFC] Use range based for for 
fake l parens (authored by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115071

Files:
  clang/lib/Format/ContinuationIndenter.cpp


Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1351,10 +1351,7 @@
 (Previous->getPrecedence() == prec::Assignment &&
  Style.AlignOperands != FormatStyle::OAS_DontAlign) ||
 Previous->is(TT_ObjCMethodExpr)));
-  for (SmallVectorImpl::const_reverse_iterator
-   I = Current.FakeLParens.rbegin(),
-   E = Current.FakeLParens.rend();
-   I != E; ++I) {
+  for (const auto &PrecedenceLevel : llvm::reverse(Current.FakeLParens)) {
 ParenState NewParenState = State.Stack.back();
 NewParenState.Tok = nullptr;
 NewParenState.ContainsLineBreak = false;
@@ -1366,7 +1363,7 @@
 NewParenState.NoLineBreak || State.Stack.back().NoLineBreakInOperand;
 
 // Don't propagate AvoidBinPacking into subexpressions of arg/param lists.
-if (*I > prec::Comma)
+if (PrecedenceLevel > prec::Comma)
   NewParenState.AvoidBinPacking = false;
 
 // Indent from 'LastSpace' unless these are fake parentheses encapsulating
@@ -1374,11 +1371,11 @@
 // brackets is disabled.
 if (!Current.isTrailingComment() &&
 (Style.AlignOperands != FormatStyle::OAS_DontAlign ||
- *I < prec::Assignment) &&
+ PrecedenceLevel < prec::Assignment) &&
 (!Previous || Previous->isNot(tok::kw_return) ||
- (Style.Language != FormatStyle::LK_Java && *I > 0)) &&
+ (Style.Language != FormatStyle::LK_Java && PrecedenceLevel > 0)) &&
 (Style.AlignAfterOpenBracket != FormatStyle::BAS_DontAlign ||
- *I != prec::Comma || Current.NestingLevel == 0)) {
+ PrecedenceLevel != prec::Comma || Current.NestingLevel == 0)) {
   NewParenState.Indent =
   std::max(std::max(State.Column, NewParenState.Indent),
State.Stack.back().LastSpace);
@@ -1387,7 +1384,7 @@
 if (Previous &&
 (Previous->getPrecedence() == prec::Assignment ||
  Previous->is(tok::kw_return) ||
- (*I == prec::Conditional && Previous->is(tok::question) &&
+ (PrecedenceLevel == prec::Conditional && Previous->is(tok::question) 
&&
   Previous->is(TT_ConditionalExpr))) &&
 !Newline) {
   // If BreakBeforeBinaryOperators is set, un-indent a bit to account for
@@ -1405,9 +1402,9 @@
 //   ParameterToInnerFunction));
 //   OuterFunction(SomeObject.InnerFunctionCall( // break
 //   ParameterToInnerFunction));
-if (*I > prec::Unknown)
+if (PrecedenceLevel > prec::Unknown)
   NewParenState.LastSpace = std::max(NewParenState.LastSpace, 
State.Column);
-if (*I != prec::Conditional && !Current.is(TT_UnaryOperator) &&
+if (PrecedenceLevel != prec::Conditional && !Current.is(TT_UnaryOperator) 
&&
 Style.AlignAfterOpenBracket != FormatStyle::BAS_DontAlign)
   NewParenState.StartOfFunctionCall = State.Column;
 
@@ -1416,17 +1413,18 @@
 // an assignment (i.e. *I <= prec::Assignment) as those have different
 // indentation rules. Indent other expression, unless the indentation needs
 // to be skipped.
-if (*I == prec::Conditional && Previous && Previous->is(tok::colon) &&
-Previous->is(TT_ConditionalExpr) && I == Current.FakeLParens.rbegin() 
&&
+if (PrecedenceLevel == prec::Conditional && Previous &&
+Previous->is(tok::colon) && Previous->is(TT_ConditionalExpr) &&
+&PrecedenceLevel == &Current.FakeLParens.back() &&
 !State.Stack.back().IsWrappedConditional) {
   NewParenState.IsChainedConditional = true;
   NewParenState.UnindentOperator = State.Stack.back().UnindentOperator;
-} else if (*I == prec::Conditional ||
-   (!SkipFirstExtraIndent && *I > prec::Assignment &&
+} else if (PrecedenceLevel == prec::Conditional ||
+   (!SkipFirstExtraIndent && PrecedenceLevel > prec::Assignment &&
 !Current.isTrailingComment())) {
   NewParenState.Indent += Style.ContinuationIndentWidth;
 }
-if ((Previous && !Previous->opensScope()) || *I != prec::Comma)
+if ((Previous && !Previous->opensScope()) || PrecedenceLevel != 
prec::Comma)
   NewParenState.BreakBeforeParameter = false;
 State.Stack.push_back(NewParenState);
 SkipFirstExtraIndent = false;


Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/Continuati

[PATCH] D115070: [clang-format][NFC] Early return when nothing to do

2021-12-04 Thread Björn Schäpers via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4041f16bb489: [clang-format][NFC] Early return when nothing 
to do (authored by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115070

Files:
  clang/lib/Format/ContinuationIndenter.cpp


Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1337,6 +1337,9 @@
 void ContinuationIndenter::moveStatePastFakeLParens(LineState &State,
 bool Newline) {
   const FormatToken &Current = *State.NextToken;
+  if (Current.FakeLParens.empty())
+return;
+
   const FormatToken *Previous = Current.getPreviousNonComment();
 
   // Don't add extra indentation for the first fake parenthesis after


Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1337,6 +1337,9 @@
 void ContinuationIndenter::moveStatePastFakeLParens(LineState &State,
 bool Newline) {
   const FormatToken &Current = *State.NextToken;
+  if (Current.FakeLParens.empty())
+return;
+
   const FormatToken *Previous = Current.getPreviousNonComment();
 
   // Don't add extra indentation for the first fake parenthesis after
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D115067: [clang-format][NFC] Use range based for

2021-12-04 Thread Björn Schäpers via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc25536e4feed: [clang-format][NFC] Use range based for 
(authored by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115067

Files:
  clang/lib/Format/TokenAnnotator.cpp


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2373,11 +2373,9 @@
 }
 
 void TokenAnnotator::annotate(AnnotatedLine &Line) {
-  for (SmallVectorImpl::iterator I = Line.Children.begin(),
-  E = Line.Children.end();
-   I != E; ++I) {
-annotate(**I);
-  }
+  for (auto &Child : Line.Children)
+annotate(*Child);
+
   AnnotatingParser Parser(Style, Line, Keywords);
   Line.Type = Parser.parseLine();
 


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2373,11 +2373,9 @@
 }
 
 void TokenAnnotator::annotate(AnnotatedLine &Line) {
-  for (SmallVectorImpl::iterator I = Line.Children.begin(),
-  E = Line.Children.end();
-   I != E; ++I) {
-annotate(**I);
-  }
+  for (auto &Child : Line.Children)
+annotate(*Child);
+
   AnnotatingParser Parser(Style, Line, Keywords);
   Line.Type = Parser.parseLine();
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D115068: [clang-format][NFC] Move static variable in scope

2021-12-04 Thread Björn Schäpers via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8d1c85454daa: [clang-format][NFC] Move static variable in 
scope (authored by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115068

Files:
  clang/lib/Format/ContinuationIndenter.cpp


Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -493,14 +493,14 @@
   return true;
   }
 
-  // Break after the closing parenthesis of TypeScript decorators before
-  // functions, getters and setters.
-  static const llvm::StringSet<> BreakBeforeDecoratedTokens = {"get", "set",
-   "function"};
   if (Style.Language == FormatStyle::LK_JavaScript &&
-  BreakBeforeDecoratedTokens.contains(Current.TokenText) &&
   Previous.is(tok::r_paren) && Previous.is(TT_JavaAnnotation)) {
-return true;
+// Break after the closing parenthesis of TypeScript decorators before
+// functions, getters and setters.
+static const llvm::StringSet<> BreakBeforeDecoratedTokens = {"get", "set",
+ "function"};
+if (BreakBeforeDecoratedTokens.contains(Current.TokenText))
+  return true;
   }
 
   // If the return type spans multiple lines, wrap before the function name.


Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -493,14 +493,14 @@
   return true;
   }
 
-  // Break after the closing parenthesis of TypeScript decorators before
-  // functions, getters and setters.
-  static const llvm::StringSet<> BreakBeforeDecoratedTokens = {"get", "set",
-   "function"};
   if (Style.Language == FormatStyle::LK_JavaScript &&
-  BreakBeforeDecoratedTokens.contains(Current.TokenText) &&
   Previous.is(tok::r_paren) && Previous.is(TT_JavaAnnotation)) {
-return true;
+// Break after the closing parenthesis of TypeScript decorators before
+// functions, getters and setters.
+static const llvm::StringSet<> BreakBeforeDecoratedTokens = {"get", "set",
+ "function"};
+if (BreakBeforeDecoratedTokens.contains(Current.TokenText))
+  return true;
   }
 
   // If the return type spans multiple lines, wrap before the function name.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D115066: [clang-format][NFC] Reorder conditions

2021-12-04 Thread Björn Schäpers via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4483e9b5279b: [clang-format][NFC] Reorder conditions 
(authored by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115066

Files:
  clang/lib/Format/TokenAnnotator.cpp


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2173,8 +2173,8 @@
 
   int CurrentPrecedence = getCurrentPrecedence();
 
-  if (Current && Current->is(TT_SelectorName) &&
-  Precedence == CurrentPrecedence) {
+  if (Precedence == CurrentPrecedence && Current &&
+  Current->is(TT_SelectorName)) {
 if (LatestOperator)
   addFakeParenthesis(Start, prec::Level(Precedence));
 Start = Current;


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2173,8 +2173,8 @@
 
   int CurrentPrecedence = getCurrentPrecedence();
 
-  if (Current && Current->is(TT_SelectorName) &&
-  Precedence == CurrentPrecedence) {
+  if (Precedence == CurrentPrecedence && Current &&
+  Current->is(TT_SelectorName)) {
 if (LatestOperator)
   addFakeParenthesis(Start, prec::Level(Precedence));
 Start = Current;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D115065: [clang-format][NFC] Merge two calls of isOneOf

2021-12-04 Thread Björn Schäpers via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5878ac7d2db6: [clang-format][NFC] Merge two calls of isOneOf 
(authored by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115065

Files:
  clang/lib/Format/TokenAnnotator.cpp


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2034,9 +2034,8 @@
tok::comma, tok::semi, tok::kw_return, tok::colon,
tok::kw_co_return, tok::kw_co_await,
tok::kw_co_yield, tok::equal, tok::kw_delete,
-   tok::kw_sizeof, tok::kw_throw) ||
-PrevToken->isOneOf(TT_BinaryOperator, TT_ConditionalExpr,
-   TT_UnaryOperator, TT_CastRParen))
+   tok::kw_sizeof, tok::kw_throw, TT_BinaryOperator,
+   TT_ConditionalExpr, TT_UnaryOperator, 
TT_CastRParen))
   return TT_UnaryOperator;
 
 if (NextToken->is(tok::l_square) && NextToken->isNot(TT_LambdaLSquare))


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2034,9 +2034,8 @@
tok::comma, tok::semi, tok::kw_return, tok::colon,
tok::kw_co_return, tok::kw_co_await,
tok::kw_co_yield, tok::equal, tok::kw_delete,
-   tok::kw_sizeof, tok::kw_throw) ||
-PrevToken->isOneOf(TT_BinaryOperator, TT_ConditionalExpr,
-   TT_UnaryOperator, TT_CastRParen))
+   tok::kw_sizeof, tok::kw_throw, TT_BinaryOperator,
+   TT_ConditionalExpr, TT_UnaryOperator, TT_CastRParen))
   return TT_UnaryOperator;
 
 if (NextToken->is(tok::l_square) && NextToken->isNot(TT_LambdaLSquare))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D115063: [clang-format][NFC] Rename variable so no shadowing happens

2021-12-04 Thread Björn Schäpers via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe7fdeda2c9db: [clang-format][NFC] Rename variable so no 
shadowing happens (authored by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115063

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp


Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -1015,9 +1015,9 @@
 QueueType Queue;
 
 // Insert start element into queue.
-StateNode *Node =
+StateNode *RootNode =
 new (Allocator.Allocate()) StateNode(InitialState, false, nullptr);
-Queue.push(QueueItem(OrderedPenalty(0, Count), Node));
+Queue.push(QueueItem(OrderedPenalty(0, Count), RootNode));
 ++Count;
 
 unsigned Penalty = 0;


Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -1015,9 +1015,9 @@
 QueueType Queue;
 
 // Insert start element into queue.
-StateNode *Node =
+StateNode *RootNode =
 new (Allocator.Allocate()) StateNode(InitialState, false, nullptr);
-Queue.push(QueueItem(OrderedPenalty(0, Count), Node));
+Queue.push(QueueItem(OrderedPenalty(0, Count), RootNode));
 ++Count;
 
 unsigned Penalty = 0;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 6e86789 - [clang-format][NFC] Use member directly

2021-12-04 Thread Björn Schäpers via cfe-commits

Author: Björn Schäpers
Date: 2021-12-04T21:29:31+01:00
New Revision: 6e8678903523019903222e4521a5e41af743cab0

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

LOG: [clang-format][NFC] Use member directly

Instead of passing it as argument to the member function.

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

Added: 


Modified: 
clang/lib/Format/UnwrappedLineFormatter.cpp

Removed: 




diff  --git a/clang/lib/Format/UnwrappedLineFormatter.cpp 
b/clang/lib/Format/UnwrappedLineFormatter.cpp
index 9753c24bfcdb..4c341aadbd01 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -62,7 +62,7 @@ class LevelIndentTracker {
   Indent = Line.Level * IndentWidth + AdditionalIndent;
 } else {
   IndentForLevel.resize(Line.Level + 1);
-  Indent = getIndent(IndentForLevel, Line.Level);
+  Indent = getIndent(Line.Level);
 }
 if (static_cast(Indent) + Offset >= 0)
   Indent += Offset;
@@ -118,12 +118,12 @@ class LevelIndentTracker {
   /// \p IndentForLevel must contain the indent for the level \c l
   /// at \p IndentForLevel[l], or a value < 0 if the indent for
   /// that level is unknown.
-  unsigned getIndent(ArrayRef IndentForLevel, unsigned Level) {
+  unsigned getIndent(unsigned Level) const {
 if (IndentForLevel[Level] != -1)
   return IndentForLevel[Level];
 if (Level == 0)
   return 0;
-return getIndent(IndentForLevel, Level - 1) + Style.IndentWidth;
+return getIndent(Level - 1) + Style.IndentWidth;
   }
 
   const FormatStyle &Style;



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


[clang] 88fa4bf - [clang-format][NFC] Use range based for for fake l parens

2021-12-04 Thread Björn Schäpers via cfe-commits

Author: Björn Schäpers
Date: 2021-12-04T21:29:30+01:00
New Revision: 88fa4bfe1ec25e0ae74566bfc2ba213f2f96b44c

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

LOG: [clang-format][NFC] Use range based for for fake l parens

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

Added: 


Modified: 
clang/lib/Format/ContinuationIndenter.cpp

Removed: 




diff  --git a/clang/lib/Format/ContinuationIndenter.cpp 
b/clang/lib/Format/ContinuationIndenter.cpp
index 04d05e16c1a0..104df6494d8d 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -1351,10 +1351,7 @@ void 
ContinuationIndenter::moveStatePastFakeLParens(LineState &State,
 (Previous->getPrecedence() == prec::Assignment &&
  Style.AlignOperands != FormatStyle::OAS_DontAlign) ||
 Previous->is(TT_ObjCMethodExpr)));
-  for (SmallVectorImpl::const_reverse_iterator
-   I = Current.FakeLParens.rbegin(),
-   E = Current.FakeLParens.rend();
-   I != E; ++I) {
+  for (const auto &PrecedenceLevel : llvm::reverse(Current.FakeLParens)) {
 ParenState NewParenState = State.Stack.back();
 NewParenState.Tok = nullptr;
 NewParenState.ContainsLineBreak = false;
@@ -1366,7 +1363,7 @@ void 
ContinuationIndenter::moveStatePastFakeLParens(LineState &State,
 NewParenState.NoLineBreak || State.Stack.back().NoLineBreakInOperand;
 
 // Don't propagate AvoidBinPacking into subexpressions of arg/param lists.
-if (*I > prec::Comma)
+if (PrecedenceLevel > prec::Comma)
   NewParenState.AvoidBinPacking = false;
 
 // Indent from 'LastSpace' unless these are fake parentheses encapsulating
@@ -1374,11 +1371,11 @@ void 
ContinuationIndenter::moveStatePastFakeLParens(LineState &State,
 // brackets is disabled.
 if (!Current.isTrailingComment() &&
 (Style.AlignOperands != FormatStyle::OAS_DontAlign ||
- *I < prec::Assignment) &&
+ PrecedenceLevel < prec::Assignment) &&
 (!Previous || Previous->isNot(tok::kw_return) ||
- (Style.Language != FormatStyle::LK_Java && *I > 0)) &&
+ (Style.Language != FormatStyle::LK_Java && PrecedenceLevel > 0)) &&
 (Style.AlignAfterOpenBracket != FormatStyle::BAS_DontAlign ||
- *I != prec::Comma || Current.NestingLevel == 0)) {
+ PrecedenceLevel != prec::Comma || Current.NestingLevel == 0)) {
   NewParenState.Indent =
   std::max(std::max(State.Column, NewParenState.Indent),
State.Stack.back().LastSpace);
@@ -1387,7 +1384,7 @@ void 
ContinuationIndenter::moveStatePastFakeLParens(LineState &State,
 if (Previous &&
 (Previous->getPrecedence() == prec::Assignment ||
  Previous->is(tok::kw_return) ||
- (*I == prec::Conditional && Previous->is(tok::question) &&
+ (PrecedenceLevel == prec::Conditional && Previous->is(tok::question) 
&&
   Previous->is(TT_ConditionalExpr))) &&
 !Newline) {
   // If BreakBeforeBinaryOperators is set, un-indent a bit to account for
@@ -1405,9 +1402,9 @@ void 
ContinuationIndenter::moveStatePastFakeLParens(LineState &State,
 //   ParameterToInnerFunction));
 //   OuterFunction(SomeObject.InnerFunctionCall( // break
 //   ParameterToInnerFunction));
-if (*I > prec::Unknown)
+if (PrecedenceLevel > prec::Unknown)
   NewParenState.LastSpace = std::max(NewParenState.LastSpace, 
State.Column);
-if (*I != prec::Conditional && !Current.is(TT_UnaryOperator) &&
+if (PrecedenceLevel != prec::Conditional && !Current.is(TT_UnaryOperator) 
&&
 Style.AlignAfterOpenBracket != FormatStyle::BAS_DontAlign)
   NewParenState.StartOfFunctionCall = State.Column;
 
@@ -1416,17 +1413,18 @@ void 
ContinuationIndenter::moveStatePastFakeLParens(LineState &State,
 // an assignment (i.e. *I <= prec::Assignment) as those have 
diff erent
 // indentation rules. Indent other expression, unless the indentation needs
 // to be skipped.
-if (*I == prec::Conditional && Previous && Previous->is(tok::colon) &&
-Previous->is(TT_ConditionalExpr) && I == Current.FakeLParens.rbegin() 
&&
+if (PrecedenceLevel == prec::Conditional && Previous &&
+Previous->is(tok::colon) && Previous->is(TT_ConditionalExpr) &&
+&PrecedenceLevel == &Current.FakeLParens.back() &&
 !State.Stack.back().IsWrappedConditional) {
   NewParenState.IsChainedConditional = true;
   NewParenState.UnindentOperator = State.Stack.back().UnindentOperator;
-} else if (*I == prec::Conditional ||
-   (!SkipFirstExtraIndent && *I > prec::Assignment &&
+} else if (PrecedenceLevel == prec::Conditional ||
+   (!Skip

[PATCH] D115061: [clang-format][NFC] Prefer pass by reference

2021-12-04 Thread Björn Schäpers via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG25f637913fe3: [clang-format][NFC] Prefer pass by reference 
(authored by HazardyKnusperkeks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115061

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp


Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -1044,9 +1044,9 @@
 
   FormatDecision LastFormat = Node->State.NextToken->getDecision();
   if (LastFormat == FD_Unformatted || LastFormat == FD_Continue)
-addNextStateToQueue(Penalty, Node, /*NewLine=*/false, &Count, &Queue);
+addNextStateToQueue(Penalty, Node, /*NewLine=*/false, Count, Queue);
   if (LastFormat == FD_Unformatted || LastFormat == FD_Break)
-addNextStateToQueue(Penalty, Node, /*NewLine=*/true, &Count, &Queue);
+addNextStateToQueue(Penalty, Node, /*NewLine=*/true, Count, Queue);
 }
 
 if (Queue.empty()) {
@@ -1072,7 +1072,7 @@
   /// Assume the current state is \p PreviousNode and has been reached with a
   /// penalty of \p Penalty. Insert a line break if \p NewLine is \c true.
   void addNextStateToQueue(unsigned Penalty, StateNode *PreviousNode,
-   bool NewLine, unsigned *Count, QueueType *Queue) {
+   bool NewLine, unsigned &Count, QueueType &Queue) {
 if (NewLine && !Indenter->canBreak(PreviousNode->State))
   return;
 if (!NewLine && Indenter->mustBreak(PreviousNode->State))
@@ -1085,8 +1085,8 @@
 
 Penalty += Indenter->addTokenToState(Node->State, NewLine, true);
 
-Queue->push(QueueItem(OrderedPenalty(Penalty, *Count), Node));
-++(*Count);
+Queue.push(QueueItem(OrderedPenalty(Penalty, Count), Node));
+++Count;
   }
 
   /// Applies the best formatting by reconstructing the path in the


Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -1044,9 +1044,9 @@
 
   FormatDecision LastFormat = Node->State.NextToken->getDecision();
   if (LastFormat == FD_Unformatted || LastFormat == FD_Continue)
-addNextStateToQueue(Penalty, Node, /*NewLine=*/false, &Count, &Queue);
+addNextStateToQueue(Penalty, Node, /*NewLine=*/false, Count, Queue);
   if (LastFormat == FD_Unformatted || LastFormat == FD_Break)
-addNextStateToQueue(Penalty, Node, /*NewLine=*/true, &Count, &Queue);
+addNextStateToQueue(Penalty, Node, /*NewLine=*/true, Count, Queue);
 }
 
 if (Queue.empty()) {
@@ -1072,7 +1072,7 @@
   /// Assume the current state is \p PreviousNode and has been reached with a
   /// penalty of \p Penalty. Insert a line break if \p NewLine is \c true.
   void addNextStateToQueue(unsigned Penalty, StateNode *PreviousNode,
-   bool NewLine, unsigned *Count, QueueType *Queue) {
+   bool NewLine, unsigned &Count, QueueType &Queue) {
 if (NewLine && !Indenter->canBreak(PreviousNode->State))
   return;
 if (!NewLine && Indenter->mustBreak(PreviousNode->State))
@@ -1085,8 +1085,8 @@
 
 Penalty += Indenter->addTokenToState(Node->State, NewLine, true);
 
-Queue->push(QueueItem(OrderedPenalty(Penalty, *Count), Node));
-++(*Count);
+Queue.push(QueueItem(OrderedPenalty(Penalty, Count), Node));
+++Count;
   }
 
   /// Applies the best formatting by reconstructing the path in the
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 4041f16 - [clang-format][NFC] Early return when nothing to do

2021-12-04 Thread Björn Schäpers via cfe-commits

Author: Björn Schäpers
Date: 2021-12-04T21:29:30+01:00
New Revision: 4041f16bb489d6cbdebfed33a2c5cc586cb3839b

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

LOG: [clang-format][NFC] Early return when nothing to do

Do not compute SkipFirstExtraIndent just to see that there are no fake l
parens.

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

Added: 


Modified: 
clang/lib/Format/ContinuationIndenter.cpp

Removed: 




diff  --git a/clang/lib/Format/ContinuationIndenter.cpp 
b/clang/lib/Format/ContinuationIndenter.cpp
index cbb8d5093ca0..04d05e16c1a0 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -1337,6 +1337,9 @@ unsigned 
ContinuationIndenter::moveStateToNextToken(LineState &State,
 void ContinuationIndenter::moveStatePastFakeLParens(LineState &State,
 bool Newline) {
   const FormatToken &Current = *State.NextToken;
+  if (Current.FakeLParens.empty())
+return;
+
   const FormatToken *Previous = Current.getPreviousNonComment();
 
   // Don't add extra indentation for the first fake parenthesis after



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


[clang] 8d1c854 - [clang-format][NFC] Move static variable in scope

2021-12-04 Thread Björn Schäpers via cfe-commits

Author: Björn Schäpers
Date: 2021-12-04T21:29:30+01:00
New Revision: 8d1c85454daa40bd663d0ef4e8262fe6fc0f21ad

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

LOG: [clang-format][NFC] Move static variable in scope

Let only the JS/TS users pay for the initialistation.

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

Added: 


Modified: 
clang/lib/Format/ContinuationIndenter.cpp

Removed: 




diff  --git a/clang/lib/Format/ContinuationIndenter.cpp 
b/clang/lib/Format/ContinuationIndenter.cpp
index 5073f5105d05..cbb8d5093ca0 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -493,14 +493,14 @@ bool ContinuationIndenter::mustBreak(const LineState 
&State) {
   return true;
   }
 
-  // Break after the closing parenthesis of TypeScript decorators before
-  // functions, getters and setters.
-  static const llvm::StringSet<> BreakBeforeDecoratedTokens = {"get", "set",
-   "function"};
   if (Style.Language == FormatStyle::LK_JavaScript &&
-  BreakBeforeDecoratedTokens.contains(Current.TokenText) &&
   Previous.is(tok::r_paren) && Previous.is(TT_JavaAnnotation)) {
-return true;
+// Break after the closing parenthesis of TypeScript decorators before
+// functions, getters and setters.
+static const llvm::StringSet<> BreakBeforeDecoratedTokens = {"get", "set",
+ "function"};
+if (BreakBeforeDecoratedTokens.contains(Current.TokenText))
+  return true;
   }
 
   // If the return type spans multiple lines, wrap before the function name.



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


[clang] c25536e - [clang-format][NFC] Use range based for

2021-12-04 Thread Björn Schäpers via cfe-commits

Author: Björn Schäpers
Date: 2021-12-04T21:29:30+01:00
New Revision: c25536e4feed59f8a8685fac3ba225057b76a358

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

LOG: [clang-format][NFC] Use range based for

That's much easier to read.

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

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index cc6d0662c9d1..6b7f135e3213 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -2373,11 +2373,9 @@ static unsigned maxNestingDepth(const AnnotatedLine 
&Line) {
 }
 
 void TokenAnnotator::annotate(AnnotatedLine &Line) {
-  for (SmallVectorImpl::iterator I = Line.Children.begin(),
-  E = Line.Children.end();
-   I != E; ++I) {
-annotate(**I);
-  }
+  for (auto &Child : Line.Children)
+annotate(*Child);
+
   AnnotatingParser Parser(Style, Line, Keywords);
   Line.Type = Parser.parseLine();
 



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


[clang] 4483e9b - [clang-format][NFC] Reorder conditions

2021-12-04 Thread Björn Schäpers via cfe-commits

Author: Björn Schäpers
Date: 2021-12-04T21:29:29+01:00
New Revision: 4483e9b5279bc71c9da7bbb0e1524c3671493ab6

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

LOG: [clang-format][NFC] Reorder conditions

Prefer to check the local variables first before dereferencing the
pointer.

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

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 3633c7cf2c27..cc6d0662c9d1 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -2173,8 +2173,8 @@ class ExpressionParser {
 
   int CurrentPrecedence = getCurrentPrecedence();
 
-  if (Current && Current->is(TT_SelectorName) &&
-  Precedence == CurrentPrecedence) {
+  if (Precedence == CurrentPrecedence && Current &&
+  Current->is(TT_SelectorName)) {
 if (LatestOperator)
   addFakeParenthesis(Start, prec::Level(Precedence));
 Start = Current;



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


[clang] 5878ac7 - [clang-format][NFC] Merge two calls of isOneOf

2021-12-04 Thread Björn Schäpers via cfe-commits

Author: Björn Schäpers
Date: 2021-12-04T21:29:29+01:00
New Revision: 5878ac7d2db692779c0d049a13cb29ed19d51ca9

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

LOG: [clang-format][NFC] Merge two calls of isOneOf

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

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index a94d8cdc3b04..3633c7cf2c27 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -2034,9 +2034,8 @@ class AnnotatingParser {
tok::comma, tok::semi, tok::kw_return, tok::colon,
tok::kw_co_return, tok::kw_co_await,
tok::kw_co_yield, tok::equal, tok::kw_delete,
-   tok::kw_sizeof, tok::kw_throw) ||
-PrevToken->isOneOf(TT_BinaryOperator, TT_ConditionalExpr,
-   TT_UnaryOperator, TT_CastRParen))
+   tok::kw_sizeof, tok::kw_throw, TT_BinaryOperator,
+   TT_ConditionalExpr, TT_UnaryOperator, 
TT_CastRParen))
   return TT_UnaryOperator;
 
 if (NextToken->is(tok::l_square) && NextToken->isNot(TT_LambdaLSquare))



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


[clang] e7fdeda - [clang-format][NFC] Rename variable so no shadowing happens

2021-12-04 Thread Björn Schäpers via cfe-commits

Author: Björn Schäpers
Date: 2021-12-04T21:29:29+01:00
New Revision: e7fdeda2c9dbe545445335f060eb450d15577aec

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

LOG: [clang-format][NFC] Rename variable so no shadowing happens

In the loop there is also a Node.

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

Added: 


Modified: 
clang/lib/Format/UnwrappedLineFormatter.cpp

Removed: 




diff  --git a/clang/lib/Format/UnwrappedLineFormatter.cpp 
b/clang/lib/Format/UnwrappedLineFormatter.cpp
index b9175c59216ba..9753c24bfcdb1 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -1015,9 +1015,9 @@ class OptimizingLineFormatter : public LineFormatter {
 QueueType Queue;
 
 // Insert start element into queue.
-StateNode *Node =
+StateNode *RootNode =
 new (Allocator.Allocate()) StateNode(InitialState, false, nullptr);
-Queue.push(QueueItem(OrderedPenalty(0, Count), Node));
+Queue.push(QueueItem(OrderedPenalty(0, Count), RootNode));
 ++Count;
 
 unsigned Penalty = 0;



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


[clang] 25f6379 - [clang-format][NFC] Prefer pass by reference

2021-12-04 Thread Björn Schäpers via cfe-commits

Author: Björn Schäpers
Date: 2021-12-04T21:29:29+01:00
New Revision: 25f637913fe31b6d23e78ff07c725bb537dd3b97

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

LOG: [clang-format][NFC] Prefer pass by reference

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

Added: 


Modified: 
clang/lib/Format/UnwrappedLineFormatter.cpp

Removed: 




diff  --git a/clang/lib/Format/UnwrappedLineFormatter.cpp 
b/clang/lib/Format/UnwrappedLineFormatter.cpp
index d099cfee9dea2..b9175c59216ba 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -1044,9 +1044,9 @@ class OptimizingLineFormatter : public LineFormatter {
 
   FormatDecision LastFormat = Node->State.NextToken->getDecision();
   if (LastFormat == FD_Unformatted || LastFormat == FD_Continue)
-addNextStateToQueue(Penalty, Node, /*NewLine=*/false, &Count, &Queue);
+addNextStateToQueue(Penalty, Node, /*NewLine=*/false, Count, Queue);
   if (LastFormat == FD_Unformatted || LastFormat == FD_Break)
-addNextStateToQueue(Penalty, Node, /*NewLine=*/true, &Count, &Queue);
+addNextStateToQueue(Penalty, Node, /*NewLine=*/true, Count, Queue);
 }
 
 if (Queue.empty()) {
@@ -1072,7 +1072,7 @@ class OptimizingLineFormatter : public LineFormatter {
   /// Assume the current state is \p PreviousNode and has been reached with a
   /// penalty of \p Penalty. Insert a line break if \p NewLine is \c true.
   void addNextStateToQueue(unsigned Penalty, StateNode *PreviousNode,
-   bool NewLine, unsigned *Count, QueueType *Queue) {
+   bool NewLine, unsigned &Count, QueueType &Queue) {
 if (NewLine && !Indenter->canBreak(PreviousNode->State))
   return;
 if (!NewLine && Indenter->mustBreak(PreviousNode->State))
@@ -1085,8 +1085,8 @@ class OptimizingLineFormatter : public LineFormatter {
 
 Penalty += Indenter->addTokenToState(Node->State, NewLine, true);
 
-Queue->push(QueueItem(OrderedPenalty(Penalty, *Count), Node));
-++(*Count);
+Queue.push(QueueItem(OrderedPenalty(Penalty, Count), Node));
+++Count;
   }
 
   /// Applies the best formatting by reconstructing the path in the



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


[PATCH] D115069: [clang-format][NFC] Merge another two calls to isOneOf

2021-12-04 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision.
curdeius added a comment.

LGTM.


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

https://reviews.llvm.org/D115069

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


[PATCH] D115069: [clang-format][NFC] Merge another two calls to isOneOf

2021-12-04 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks updated this revision to Diff 391857.
HazardyKnusperkeks marked 2 inline comments as done.
HazardyKnusperkeks added a comment.

Incorporated feedback.


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

https://reviews.llvm.org/D115069

Files:
  clang/lib/Format/ContinuationIndenter.cpp


Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1290,10 +1290,9 @@
 State.Stack[i].NoLineBreak = true;
 State.Stack[State.Stack.size() - 2].NestedBlockInlined = false;
   }
-  if (Previous &&
-  (Previous->isOneOf(tok::l_paren, tok::comma, tok::colon) ||
-   Previous->isOneOf(TT_BinaryOperator, TT_ConditionalExpr)) &&
-  !Previous->isOneOf(TT_DictLiteral, TT_ObjCMethodExpr)) {
+  if (Previous && (Previous->isOneOf(TT_BinaryOperator, TT_ConditionalExpr) ||
+   (Previous->isOneOf(tok::l_paren, tok::comma, tok::colon) &&
+!Previous->isOneOf(TT_DictLiteral, TT_ObjCMethodExpr {
 State.Stack.back().NestedBlockInlined =
 !Newline && hasNestedBlockInlined(Previous, Current, Style);
   }


Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1290,10 +1290,9 @@
 State.Stack[i].NoLineBreak = true;
 State.Stack[State.Stack.size() - 2].NestedBlockInlined = false;
   }
-  if (Previous &&
-  (Previous->isOneOf(tok::l_paren, tok::comma, tok::colon) ||
-   Previous->isOneOf(TT_BinaryOperator, TT_ConditionalExpr)) &&
-  !Previous->isOneOf(TT_DictLiteral, TT_ObjCMethodExpr)) {
+  if (Previous && (Previous->isOneOf(TT_BinaryOperator, TT_ConditionalExpr) ||
+   (Previous->isOneOf(tok::l_paren, tok::comma, tok::colon) &&
+!Previous->isOneOf(TT_DictLiteral, TT_ObjCMethodExpr {
 State.Stack.back().NestedBlockInlined =
 !Newline && hasNestedBlockInlined(Previous, Current, Style);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D115050: [clang-format] PR48916 PointerAlignment not working when using C++20 init-statement in for loop

2021-12-04 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3054
+  return true;
+// & // comment
+if (Right.is(TT_BlockComment))

Block, not line, comment



Comment at: clang/unittests/Format/FormatTest.cpp:1947
   verifyFormat("int&& c = f3();", Style);
+  verifyFormat("for (auto a = 0, b = 0; const auto& c : {1, 2, 3})", Style);
 

How about pointers/references in the init? Also, please test sth else than 
auto, both in init and as the loop variable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115050

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


[PATCH] D115064: [clang-format][NFC] Replace deque with vector

2021-12-04 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks updated this revision to Diff 391855.
HazardyKnusperkeks added a comment.

Formatting fixed.


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

https://reviews.llvm.org/D115064

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp


Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -1096,22 +1096,22 @@
   /// Applies the best formatting by reconstructing the path in the
   /// solution space that leads to \c Best.
   void reconstructPath(LineState &State, StateNode *Best) {
-std::deque Path;
+std::vector Path;
 // We do not need a break before the initial token.
 while (Best->Previous) {
-  Path.push_front(Best);
+  Path.push_back(Best);
   Best = Best->Previous;
 }
-for (auto I = Path.begin(), E = Path.end(); I != E; ++I) {
+for (const auto &Node : llvm::reverse(Path)) {
   unsigned Penalty = 0;
-  formatChildren(State, (*I)->NewLine, /*DryRun=*/false, Penalty);
-  Penalty += Indenter->addTokenToState(State, (*I)->NewLine, false);
+  formatChildren(State, Node->NewLine, /*DryRun=*/false, Penalty);
+  Penalty += Indenter->addTokenToState(State, Node->NewLine, false);
 
   LLVM_DEBUG({
-printLineState((*I)->Previous->State);
-if ((*I)->NewLine) {
+printLineState(Node->Previous->State);
+if (Node->NewLine) {
   llvm::dbgs() << "Penalty for placing "
-   << (*I)->Previous->State.NextToken->Tok.getName()
+   << Node->Previous->State.NextToken->Tok.getName()
<< " on a new line: " << Penalty << "\n";
 }
   });


Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -1096,22 +1096,22 @@
   /// Applies the best formatting by reconstructing the path in the
   /// solution space that leads to \c Best.
   void reconstructPath(LineState &State, StateNode *Best) {
-std::deque Path;
+std::vector Path;
 // We do not need a break before the initial token.
 while (Best->Previous) {
-  Path.push_front(Best);
+  Path.push_back(Best);
   Best = Best->Previous;
 }
-for (auto I = Path.begin(), E = Path.end(); I != E; ++I) {
+for (const auto &Node : llvm::reverse(Path)) {
   unsigned Penalty = 0;
-  formatChildren(State, (*I)->NewLine, /*DryRun=*/false, Penalty);
-  Penalty += Indenter->addTokenToState(State, (*I)->NewLine, false);
+  formatChildren(State, Node->NewLine, /*DryRun=*/false, Penalty);
+  Penalty += Indenter->addTokenToState(State, Node->NewLine, false);
 
   LLVM_DEBUG({
-printLineState((*I)->Previous->State);
-if ((*I)->NewLine) {
+printLineState(Node->Previous->State);
+if (Node->NewLine) {
   llvm::dbgs() << "Penalty for placing "
-   << (*I)->Previous->State.NextToken->Tok.getName()
+   << Node->Previous->State.NextToken->Tok.getName()
<< " on a new line: " << Penalty << "\n";
 }
   });
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D115064: [clang-format][NFC] Replace deque with vector

2021-12-04 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment.

In D115064#3170615 , @curdeius wrote:

> Deque, contrary to the vector, doesn't need to move the elements when 
> growing. I'm not sure if that's relevant here though.
> Have you checked what's on average the maximum size of `Path` on some larger 
> repo?
> Maybe using `llvm::SmallVector` with some well-thought (data-based) static 
> number of elements would be better here, WDYT?

I have not checked what the sizes are. But I have thought about reserving the 
upper bound (all visited states) so there is only one allocation.
The moving of the content shouldn't be problematic, since we only have pointers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115064

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


[PATCH] D115103: Leak Sanitizer port to Windows

2021-12-04 Thread Clemens Wasser via Phabricator via cfe-commits
clemenswasser created this revision.
clemenswasser added a project: Sanitizers.
Herald added subscribers: abrachet, phosek, krytarowski, mgorny.
clemenswasser requested review of this revision.
Herald added a project: clang.
Herald added subscribers: Sanitizers, cfe-commits.

This was done by me in a few hours, so don't expect too much ;)

Initial Leak Sanitizer port to Windows.
The main part of the port was to implement `StopTheWorld` for Windows.
Many tests of the lsan test suite seem to run fine (leaks are correctly being 
reported), however `__lsan_ignore_object` doesn't seem to work.
Some other tests that fail are: new_array_with_dtor_0 and large_allocation_leak.
The test suite for lsan won't run for me:

  $ "LSAN_BASE=use_stacks=0:use_registers=0"
  'LSAN_BASE=use_stacks=0:use_registers=0': command not found

Also I need to figure out, how to get clang to auto link the lsan runtime when 
specifying `-fsanitize=address` on Windows.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115103

Files:
  clang/lib/Driver/ToolChains/MSVC.cpp
  compiler-rt/cmake/config-ix.cmake
  compiler-rt/lib/lsan/CMakeLists.txt
  compiler-rt/lib/lsan/lsan.h
  compiler-rt/lib/lsan/lsan_allocator.cpp
  compiler-rt/lib/lsan/lsan_common.cpp
  compiler-rt/lib/lsan/lsan_common.h
  compiler-rt/lib/lsan/lsan_common_win.cpp
  compiler-rt/lib/lsan/lsan_interceptors.cpp
  compiler-rt/lib/lsan/lsan_win.cpp
  compiler-rt/lib/lsan/lsan_win.h
  compiler-rt/lib/sanitizer_common/CMakeLists.txt
  compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_win.cpp
  compiler-rt/test/lsan/lit.common.cfg.py

Index: compiler-rt/test/lsan/lit.common.cfg.py
===
--- compiler-rt/test/lsan/lit.common.cfg.py
+++ compiler-rt/test/lsan/lit.common.cfg.py
@@ -79,7 +79,8 @@
 supported_linux = (not config.android) and config.host_os == 'Linux' and config.host_arch in ['aarch64', 'x86_64', 'ppc64', 'ppc64le', 'mips64', 'riscv64', 'arm', 'armhf', 'armv7l', 's390x']
 supported_darwin = config.host_os == 'Darwin' and config.target_arch in ['x86_64']
 supported_netbsd = config.host_os == 'NetBSD' and config.target_arch in ['x86_64', 'i386']
-if not (supported_android or supported_linux or supported_darwin or supported_netbsd):
+supported_windows = config.host_os == 'Windows'
+if not (supported_android or supported_linux or supported_darwin or supported_netbsd or supported_windows):
   config.unsupported = True
 
 # Don't support Thumb due to broken fast unwinder
Index: compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_win.cpp
===
--- /dev/null
+++ compiler-rt/lib/sanitizer_common/sanitizer_stoptheworld_win.cpp
@@ -0,0 +1,137 @@
+//===-- sanitizer_stoptheworld_win.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// See sanitizer_stoptheworld.h for details.
+//
+//===--===//
+
+#include "sanitizer_common.h"
+#include "sanitizer_internal_defs.h"
+#include "sanitizer_platform.h"
+
+#if SANITIZER_WINDOWS
+
+#define WIN32_LEAN_AND_MEAN
+#include 
+
+#include 
+
+#include "sanitizer_stoptheworld.h"
+
+
+namespace __sanitizer {
+
+struct SuspendedThreadsListWindows final : public SuspendedThreadsList {
+  InternalMmapVector threadHandles;
+  InternalMmapVector threadIds;
+
+  SuspendedThreadsListWindows() {
+threadIds.reserve(1024);
+threadHandles.reserve(1024);
+  }
+
+  PtraceRegistersStatus GetRegistersAndSP(uptr index,
+  InternalMmapVector *buffer,
+  uptr *sp) const override;
+
+  tid_t GetThreadID(uptr index) const override;
+  uptr ThreadCount() const override;
+};
+
+PtraceRegistersStatus SuspendedThreadsListWindows::GetRegistersAndSP(
+uptr index, InternalMmapVector *buffer, uptr *sp) const {
+  CHECK_LT(index, threadHandles.size());
+
+  CONTEXT thread_context;
+  thread_context.ContextFlags = CONTEXT_ALL;
+  CHECK(GetThreadContext(threadHandles[index], &thread_context));
+
+  buffer->resize(RoundUpTo(sizeof(thread_context), sizeof(uptr)) / sizeof(uptr));
+  internal_memcpy(buffer->data(), &thread_context, sizeof(thread_context));
+
+  *sp = thread_context.Rsp;
+
+  return REGISTERS_AVAILABLE;
+}
+
+tid_t SuspendedThreadsListWindows::GetThreadID(uptr index) const {
+  CHECK_LT(index, threadIds.size());
+  return threadIds[index];
+}
+
+uptr SuspendedThreadsListWindows::ThreadCount() const {
+  return threadIds.size();
+}
+
+struct RunThreadArgs {
+  StopTheWorldCallback callback;
+  void *argument;
+};
+
+DWORD WINAPI RunThread(void *argument) {
+  RunThreadArgs *ru

[PATCH] D115060: [clang-format][NFC] Code Tidies in UnwrappedLineFormatter

2021-12-04 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks updated this revision to Diff 391853.
HazardyKnusperkeks edited the summary of this revision.
HazardyKnusperkeks added a comment.

Fixed formatting and added a name for I[-1].

I[i + 1] is used in the loop condition, which also checks if i + 1 is valid, so 
do not give a name here.


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

https://reviews.llvm.org/D115060

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp

Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -211,10 +211,12 @@
 const AnnotatedLine *TheLine = *I;
 if (TheLine->Last->is(TT_LineComment))
   return 0;
-if (I[1]->Type == LT_Invalid || I[1]->First->MustBreakBefore)
+const auto &NextLine = *I[1];
+const auto &PreviousLine = *I[-1];
+if (NextLine.Type == LT_Invalid || NextLine.First->MustBreakBefore)
   return 0;
 if (TheLine->InPPDirective &&
-(!I[1]->InPPDirective || I[1]->First->HasUnescapedNewline))
+(!NextLine.InPPDirective || NextLine.First->HasUnescapedNewline))
   return 0;
 
 if (Style.ColumnLimit > 0 && Indent > Style.ColumnLimit)
@@ -231,15 +233,15 @@
 if (TheLine->Last->is(TT_FunctionLBrace) &&
 TheLine->First == TheLine->Last &&
 !Style.BraceWrapping.SplitEmptyFunction &&
-I[1]->First->is(tok::r_brace))
+NextLine.First->is(tok::r_brace))
   return tryMergeSimpleBlock(I, E, Limit);
 
 // Handle empty record blocks where the brace has already been wrapped
 if (TheLine->Last->is(tok::l_brace) && TheLine->First == TheLine->Last &&
 I != AnnotatedLines.begin()) {
-  bool EmptyBlock = I[1]->First->is(tok::r_brace);
+  bool EmptyBlock = NextLine.First->is(tok::r_brace);
 
-  const FormatToken *Tok = I[-1]->First;
+  const FormatToken *Tok = PreviousLine.First;
   if (Tok && Tok->is(tok::comment))
 Tok = Tok->getNextNonComment();
 
@@ -267,7 +269,7 @@
 bool MergeShortFunctions =
 Style.AllowShortFunctionsOnASingleLine == FormatStyle::SFS_All ||
 (Style.AllowShortFunctionsOnASingleLine >= FormatStyle::SFS_Empty &&
- I[1]->First->is(tok::r_brace)) ||
+ NextLine.First->is(tok::r_brace)) ||
 (Style.AllowShortFunctionsOnASingleLine & FormatStyle::SFS_InlineOnly &&
  TheLine->Level != 0);
 
@@ -312,73 +314,75 @@
   return MergeShortFunctions ? tryMergeSimpleBlock(I, E, Limit) : 0;
 }
 // Try to merge a control statement block with left brace unwrapped
-if (TheLine->Last->is(tok::l_brace) && TheLine->First != TheLine->Last &&
+if (TheLine->Last->is(tok::l_brace) &&
 TheLine->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for)) {
   return Style.AllowShortBlocksOnASingleLine != FormatStyle::SBS_Never
  ? tryMergeSimpleBlock(I, E, Limit)
  : 0;
 }
 // Try to merge a control statement block with left brace wrapped
-if (I[1]->First->is(tok::l_brace) &&
-(TheLine->First->isOneOf(tok::kw_if, tok::kw_else, tok::kw_while,
- tok::kw_for, tok::kw_switch, tok::kw_try,
- tok::kw_do, TT_ForEachMacro) ||
- (TheLine->First->is(tok::r_brace) && TheLine->First->Next &&
-  TheLine->First->Next->isOneOf(tok::kw_else, tok::kw_catch))) &&
-Style.BraceWrapping.AfterControlStatement ==
-FormatStyle::BWACS_MultiLine) {
-  // If possible, merge the next line's wrapped left brace with the current
-  // line. Otherwise, leave it on the next line, as this is a multi-line
-  // control statement.
-  return (Style.ColumnLimit == 0 ||
-  TheLine->Last->TotalLength <= Style.ColumnLimit)
- ? 1
- : 0;
-} else if (I[1]->First->is(tok::l_brace) &&
-   TheLine->First->isOneOf(tok::kw_if, tok::kw_else, tok::kw_while,
-   tok::kw_for)) {
-  return (Style.BraceWrapping.AfterControlStatement ==
-  FormatStyle::BWACS_Always)
- ? tryMergeSimpleBlock(I, E, Limit)
- : 0;
-} else if (I[1]->First->is(tok::l_brace) &&
-   TheLine->First->isOneOf(tok::kw_else, tok::kw_catch) &&
-   Style.BraceWrapping.AfterControlStatement ==
-   FormatStyle::BWACS_MultiLine) {
-  // This case if different from the upper BWACS_MultiLine processing
-  // in that a preceding r_brace is not on the same line as else/catch
-  // most likely because of BeforeElse/BeforeCatch set to true.
-  // If the line length doesn't fit ColumnLimit, leave l_brace on the
-  // next line to respect the BWACS_MultiLine.
-  return (Style.ColumnLimit == 0 ||
-  TheLine->Last->TotalLength <= Style.ColumnLimit)
-  

[PATCH] D113804: [clang-tidy] Fix behavior of `modernize-use-using` with nested structs/unions

2021-12-04 Thread Fabian Wolff via Phabricator via cfe-commits
fwolff updated this revision to Diff 391852.
fwolff added a comment.

Thanks for your comments @whisperity! They should all be fixed now.


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

https://reviews.llvm.org/D113804

Files:
  clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
  clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
@@ -302,3 +302,15 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'using' instead of 'typedef'
   // CHECK-FIXES: using b = InjectedClassNameWithUnnamedArgument;
 };
+
+typedef struct { int a; union { int b; }; } PR50990;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using PR50990 = struct { int a; union { int b; }; };
+
+typedef struct { struct { int a; struct { struct { int b; } c; int d; } e; } f; int g; } PR50990_nested;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using PR50990_nested = struct { struct { int a; struct { struct { int b; } c; int d; } e; } f; int g; };
+
+typedef struct { struct { int a; } b; union { int c; float d; struct { int e; }; }; struct { double f; } g; } PR50990_siblings;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using PR50990_siblings = struct { struct { int a; } b; union { int c; float d; struct { int e; }; }; struct { double f; } g; };
Index: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
===
--- clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
+++ clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
@@ -23,7 +23,8 @@
 
   const bool IgnoreMacros;
   SourceLocation LastReplacementEnd;
-  SourceRange LastTagDeclRange;
+  llvm::DenseMap LastTagDeclRanges;
+
   std::string FirstTypedefType;
   std::string FirstTypedefName;
 
Index: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -16,6 +16,10 @@
 namespace tidy {
 namespace modernize {
 
+static constexpr llvm::StringLiteral ParentDeclName = "parent-decl";
+static constexpr llvm::StringLiteral TagDeclName = "tag-decl";
+static constexpr llvm::StringLiteral TypedefName = "typedef";
+
 UseUsingCheck::UseUsingCheck(StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
   IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {}
@@ -25,23 +29,45 @@
 }
 
 void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(typedefDecl(unless(isInstantiated())).bind("typedef"),
+  Finder->addMatcher(typedefDecl(unless(isInstantiated()),
+ hasParent(decl().bind(ParentDeclName)))
+ .bind(TypedefName),
  this);
-  // This matcher used to find tag declarations in source code within typedefs.
-  // They appear in the AST just *prior* to the typedefs.
-  Finder->addMatcher(tagDecl(unless(isImplicit())).bind("tagdecl"), this);
+
+  // This matcher is used to find tag declarations in source code within
+  // typedefs. They appear in the AST just *prior* to the typedefs.
+  Finder->addMatcher(
+  tagDecl(
+  anyOf(allOf(unless(anyOf(isImplicit(),
+   classTemplateSpecializationDecl())),
+  hasParent(decl().bind(ParentDeclName))),
+// We want the parent of the ClassTemplateDecl, not the parent
+// of the specialization.
+classTemplateSpecializationDecl(hasAncestor(
+classTemplateDecl(hasParent(decl().bind(ParentDeclName)))
+  .bind(TagDeclName),
+  this);
 }
 
 void UseUsingCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *ParentDecl = Result.Nodes.getNodeAs(ParentDeclName);
+  if (!ParentDecl)
+return;
+
   // Match CXXRecordDecl only to store the range of the last non-implicit full
   // declaration, to later check whether it's within the typdef itself.
-  const auto *MatchedTagDecl = Result.Nodes.getNodeAs("tagdecl");
+  const auto *MatchedTagDecl = Result.Nodes.getNodeAs(TagDeclName);
   if (MatchedTagDecl) {
-LastTagDeclRange = MatchedTagDecl->getSourceRange();
+// It is not sufficient to just track the last TagDecl that we've seen,
+// because if one struct or union is nested inside another, the last TagDecl
+// before the typedef will be the nested one (PR#50990). Therefore, we also
+// kee

[PATCH] D114197: [clang-tidy] Fix false positives involving type aliases in `misc-unconventional-assign-operator` check

2021-12-04 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator.cpp:151
+  using Alias3 = TemplateTypeAlias;
+  Alias3 &operator=(int) { return *this; }
+};

fwolff wrote:
> whisperity wrote:
> > This is a no-warn due to the parameter being a completely unrelated type, 
> > right? Might worth a comment. I don't see at first glance why a warning 
> > should not happen here.
> Exactly. I've added a comment in the `TypeAlias` struct above, because that 
> one comes first. I've also updated the documentation for this check to make 
> this clearer.
> This is a no-warn due to the parameter being a completely unrelated type, 
> right?

No. If that were the case, this would be a terrible test case, full of 
irrelevant cruft. :)

Once you look past all the levels of type aliases, this is equivalent to
```
template
struct TTA {
TTA& operator=(int) { return *this; }
};
```
and so no warning is given because it's correct code.


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

https://reviews.llvm.org/D114197

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


[PATCH] D113863: [clang-tidy] Make `readability-container-data-pointer` more robust

2021-12-04 Thread Saleem Abdulrasool via Phabricator via cfe-commits
compnerd added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp:22
+constexpr llvm::StringLiteral DerefContainerExprName = "deref-container-expr";
+constexpr llvm::StringLiteral AddrofContainerExprName = 
"addrof-container-expr";
+constexpr llvm::StringLiteral AddressOfName = "address-of";

Would you mind using `addr-of-container-expr` and renaming this to 
`AddrOfContainerExprName`?



Comment at: 
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp:76
+arraySubscriptExpr(hasLHS(ContainerExpr), hasRHS(Zero))
+  .bind(AddressOfName),
   this);

Nice, I do like this, it is quite a bit more succinct.  One thing that is a bit 
less clear to me is that the previous expression was a bit more restrictive in 
the parameter types to certain expressions, is there a reason that you don't 
expect that to matter much in practice?  If a malformed input is provided, we 
could now match certain things that we didn't previously, or am I not matching 
up the conditions carefully enough?



Comment at: 
clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp:99
+  Lexer::getSourceText(CharSourceRange::getTokenRange(SourceRange),
+   *Result.SourceManager, getLangOpts()));
+

The diff shows up odd here, is there a hard tab or is that just the rendering?


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

https://reviews.llvm.org/D113863

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


[PATCH] D114197: [clang-tidy] Fix false positives involving type aliases in `misc-unconventional-assign-operator` check

2021-12-04 Thread Fabian Wolff via Phabricator via cfe-commits
fwolff marked 2 inline comments as done.
fwolff added a comment.

Thanks for your comments @whisperity. I think I've addressed them, could you 
have another look?




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator.cpp:151
+  using Alias3 = TemplateTypeAlias;
+  Alias3 &operator=(int) { return *this; }
+};

whisperity wrote:
> This is a no-warn due to the parameter being a completely unrelated type, 
> right? Might worth a comment. I don't see at first glance why a warning 
> should not happen here.
Exactly. I've added a comment in the `TypeAlias` struct above, because that one 
comes first. I've also updated the documentation for this check to make this 
clearer.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator.cpp:152
+  Alias3 &operator=(int) { return *this; }
+};

whisperity wrote:
> What about `Alias3& operator =(const Alias1&) { return 
> *this; }`? That should trigger a warning as it is an unrelated type, right?
Yes, I've added a test for this (but with `double` as the argument type, 
because `const Alias1&` gives a warning:
```
warning: 'const' qualifier on reference type 'TemplateTypeAlias::Alias1' (aka 
'TemplateTypeAlias &') has no effect [-Wignored-qualifiers]
```


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

https://reviews.llvm.org/D114197

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


[PATCH] D114197: [clang-tidy] Fix false positives involving type aliases in `misc-unconventional-assign-operator` check

2021-12-04 Thread Fabian Wolff via Phabricator via cfe-commits
fwolff updated this revision to Diff 391850.

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

https://reviews.llvm.org/D114197

Files:
  clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
  
clang-tools-extra/docs/clang-tidy/checks/misc-unconventional-assign-operator.rst
  
clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator.cpp
@@ -127,3 +127,30 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: operator=() should always 
return '*this'
   }
 };
+
+// Check that no false positives are issued when using type aliases
+struct TypeAlias {
+  using Alias = TypeAlias;
+  // This is correct and should not produce any warnings:
+  Alias &operator=(const Alias &) { return *this; }
+
+  using AliasRef = Alias &;
+  // So is this (assignments from other types are fine):
+  AliasRef operator=(int) { return *this; }
+};
+
+// Same check as above for a template class
+template 
+struct TemplateTypeAlias {
+  using Alias1 = TemplateTypeAlias &;
+  using Alias2 = TemplateTypeAlias const &;
+  Alias1 operator=(Alias2) { return *this; }
+
+  template 
+  using Alias3 = TemplateTypeAlias;
+  Alias3 &operator=(int) { return *this; }
+
+  // Using a different type parameter in the return type should give a warning
+  Alias3& operator=(double) { return *this; }
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should return 
'TemplateTypeAlias&' [misc-unconventional-assign-operator]
+};
Index: 
clang-tools-extra/docs/clang-tidy/checks/misc-unconventional-assign-operator.rst
===
--- 
clang-tools-extra/docs/clang-tidy/checks/misc-unconventional-assign-operator.rst
+++ 
clang-tools-extra/docs/clang-tidy/checks/misc-unconventional-assign-operator.rst
@@ -8,6 +8,8 @@
 types and definitions with good return type but wrong ``return`` statements.
 
   * The return type must be ``Class&``.
-  * Works with move-assign and assign by value.
+  * The assignment may be from the class type by value, const lvalue
+reference, or non-const rvalue reference, or from a completely different
+type (e.g. ``int``).
   * Private and deleted operators are ignored.
   * The operator must always return ``*this``.
Index: clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
@@ -18,13 +18,14 @@
 
 void UnconventionalAssignOperatorCheck::registerMatchers(
 ast_matchers::MatchFinder *Finder) {
-  const auto HasGoodReturnType = cxxMethodDecl(returns(lValueReferenceType(
-  pointee(unless(isConstQualified()),
-  anyOf(autoType(), hasDeclaration(equalsBoundNode("class")));
+  const auto HasGoodReturnType =
+  cxxMethodDecl(returns(hasCanonicalType(lValueReferenceType(pointee(
+  unless(isConstQualified()),
+  anyOf(autoType(), hasDeclaration(equalsBoundNode("class";
 
-  const auto IsSelf = qualType(
+  const auto IsSelf = qualType(hasCanonicalType(
   anyOf(hasDeclaration(equalsBoundNode("class")),
-referenceType(pointee(hasDeclaration(equalsBoundNode("class"));
+
referenceType(pointee(hasDeclaration(equalsBoundNode("class")));
   const auto IsAssign =
   cxxMethodDecl(unless(anyOf(isDeleted(), isPrivate(), isImplicit())),
 hasName("operator="), ofClass(recordDecl().bind("class")))
@@ -37,9 +38,9 @@
   cxxMethodDecl(IsAssign, unless(HasGoodReturnType)).bind("ReturnType"),
   this);
 
-  const auto BadSelf = referenceType(
+  const auto BadSelf = qualType(hasCanonicalType(referenceType(
   anyOf(lValueReferenceType(pointee(unless(isConstQualified(,
-rValueReferenceType(pointee(isConstQualified();
+rValueReferenceType(pointee(isConstQualified()));
 
   Finder->addMatcher(
   cxxMethodDecl(IsSelfAssign,


Index: clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator.cpp
@@ -127,3 +127,30 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: operator=() should always return '*this'
   }
 };
+
+// Check that no false positives are issued when using type aliases
+struct TypeAlias {
+  using Alias = TypeAlias;
+  // This is correct and s

[PATCH] D113518: [clang][Sema] Create delegating constructors even in templates

2021-12-04 Thread Fabian Wolff via Phabricator via cfe-commits
fwolff marked 2 inline comments as done.
fwolff added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:4524-4525
   if (!Dependent) {
-if (Context.hasSameUnqualifiedType(QualType(ClassDecl->getTypeForDecl(),0),
-   BaseType))
+if (Delegating)
   return BuildDelegatingInitializer(BaseTInfo, Init, ClassDecl);
 

aaron.ballman wrote:
> Looking at the source for `BuildDelegatingInitializer()`, it looks like we 
> should still be able to call this in a dependent context. In fact, it has a 
> fixme comment specifically about that:
> ```
> // If we are in a dependent context, template instantiation will
> // perform this type-checking again. Just save the arguments that we
> // received in a ParenListExpr.
> // FIXME: This isn't quite ideal, since our ASTs don't capture all
> // of the information that we have about the base
> // initializer. However, deconstructing the ASTs is a dicey process,
> // and this approach is far more likely to get the corner cases right.
> ```
> I'm wondering if the better fix here is to hoist the delegation check out of 
> the `if (!Dependent)`. Did you try that and run into issues?
Yes, I have tried this, and it leads to a crash [[ 
https://github.com/llvm/llvm-project/blob/ca2f53897a2f2a60d8cb1538d5fcf930d814e9f5/clang/lib/Sema/SemaDeclCXX.cpp#L
 | here ]] (because the cast fails). Apparently, this code path in 
`BuildDelegatingInitializer()` is never exercised for dependent contexts, 
despite the comment, and therefore doesn't work.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5071
 
-  if (CXXDestructorDecl *Dtor = LookupDestructor(Constructor->getParent())) {
-MarkFunctionReferenced(Initializer->getSourceLocation(), Dtor);
-DiagnoseUseOfDecl(Dtor, Initializer->getSourceLocation());
-  }
+  if (!Constructor->isDependentContext()) {
+if (CXXDestructorDecl *Dtor = LookupDestructor(Constructor->getParent())) {

aaron.ballman wrote:
> Can you explain why this change was needed?
Without this, clang crashes because `LookupDestructor()` calls 
`LookupSpecialMember()`, which then runs into [[ 
https://github.com/llvm/llvm-project/blob/ca2f53897a2f2a60d8cb1538d5fcf930d814e9f5/clang/lib/Sema/SemaLookup.cpp#L3064-L3065
 | this assertion ]]:
```
clang-14: /[...]/llvm-project/clang/lib/Sema/SemaLookup.cpp:3064: 
clang::Sema::SpecialMemberOverloadResult 
clang::Sema::LookupSpecialMember(clang::CXXRecordDecl*, 
clang::Sema::CXXSpecialMember, bool, bool, bool, bool, bool): Assertion 
`CanDeclareSpecialMemberFunction(RD) && "doing special member lookup into 
record that isn't fully complete"' failed.
```
Previously, this wasn't necessary because no delegating constructors would be 
generated for templates, and so this function wasn't ever called in a dependent 
context.


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

https://reviews.llvm.org/D113518

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


[PATCH] D109751: [Clang] Support conversion between PPC double-double and IEEE float128

2021-12-04 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments.



Comment at: clang/test/Sema/float128-ld-incompatibility.cpp:13
+long double ld{qf()}; // expected-error {{non-constant-expression cannot be 
narrowed from type '__float128' to 'long double' in initializer list}} 
expected-note {{insert an explicit cast to silence this issue}}
+__float128 q{ldf()}; // expected-no-error
 

hubert.reinterpretcast wrote:
> hubert.reinterpretcast wrote:
> > Should also test `__ibm128` cases.
> The C++ committee has advised that this implicit conversion should be 
> considered ill-formed (see other comment).
> 
> Note that the //allowed// implicit conversion from `__ibm128` to `long 
> double` (and vice versa) is still a conversion, which means that overload 
> resolution is still a problem:
> ```
> void f(__ibm128);
> void f(int);
> void g(long double d) { return f(d); } // okay with GCC but not Clang; 
> https://godbolt.org/z/fonsEbbY1
> ```
Even if the implicit conversion is to be allowed, there is not much reason why 
this is not considered narrowing (given the revised definition of narrowing 
conversions).

GCC's behaviour around the narrowing check has limited value as a reference 
point: Which of `__float128` or `__ibm128` is considered by GCC on Power to be 
the "wider type" depends on the `-mabi` option. That is, the behaviour is not 
consistent with there being a principled decision made by the GCC developers as 
to which representation format is "wider".



Comment at: clang/test/Sema/float128-ld-incompatibility.cpp:36-37
   q / ld; // expected-error {{invalid operands to binary expression 
('__float128' and 'long double')}}
-  ld = q; // expected-error {{assigning to 'long double' from incompatible 
type '__float128'}}
-  q = ld; // expected-error {{assigning to '__float128' from incompatible type 
'long double'}}
+  ld = q; // expected-no-error {{assigning to 'long double' from incompatible 
type '__float128'}}
+  q = ld; // expected-no-error {{assigning to '__float128' from incompatible 
type 'long double'}}
   q + b ? q : ld; // expected-error {{incompatible operand types ('__float128' 
and 'long double')}}

hubert.reinterpretcast wrote:
> The C++ committee has advised that these are not okay as implicit conversions:
> http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p1467r7.html#implicit
> 
> Additional lines testing `static_cast` would be appropriate.
> 
I guess this could be controversial since converting from `__float128` to 
`float` (for example) would also be prohibited (if `__float128` were to be 
considered to be an extended floating-point type).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109751

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


[PATCH] D113369: [clang-format] Extend SpaceBeforeParens for requires

2021-12-04 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:3756
+  * ``bool AfterRequiresClause`` If ``true``, put space between requires 
keyword in a requires clause and
+opening parentheses, if is are one.
+

HazardyKnusperkeks wrote:
> Quuxplusone wrote:
> > HazardyKnusperkeks wrote:
> > > curdeius wrote:
> > > > You meant "if there is one", right?
> > > Yeah
> > IMO these options should be named `InRequiresClause` and 
> > `InRequiresExpression`: that's where the space is going. The space doesn't 
> > go //after// the requires-clause. The space doesn't go //after// the 
> > requires-expression.
> > It occurs to me that the name of this option could be analogous to the name 
> > of the option that controls `[]() {}` versus `[] () {}`... except that it 
> > looks like there is no such option (and I'm happy about that). :) Also, the 
> > name of that option would probably just be `AfterCaptureList`, which 
> > doesn't help us in this case.
> I get your point, but the space does not go anywhere in the 
> clause/expression, so `AfterRequiresForClauses`?
(I'd avoid the word `For`, because of keyword `for`.)
A //requires-expression// is the whole expression, `requires + param-list + 
body`: https://eel.is/c++draft/expr.prim.req#nt:requires-expression
A //requires-clause// is the whole clause, `requires + logical-or-expr`: 
https://eel.is/c++draft/temp.pre#nt:requires-clause
Does that resolve your concern about the word `In`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113369

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


[PATCH] D113369: [clang-format] Extend SpaceBeforeParens for requires

2021-12-04 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:3756
+  * ``bool AfterRequiresClause`` If ``true``, put space between requires 
keyword in a requires clause and
+opening parentheses, if is are one.
+

Quuxplusone wrote:
> HazardyKnusperkeks wrote:
> > curdeius wrote:
> > > You meant "if there is one", right?
> > Yeah
> IMO these options should be named `InRequiresClause` and 
> `InRequiresExpression`: that's where the space is going. The space doesn't go 
> //after// the requires-clause. The space doesn't go //after// the 
> requires-expression.
> It occurs to me that the name of this option could be analogous to the name 
> of the option that controls `[]() {}` versus `[] () {}`... except that it 
> looks like there is no such option (and I'm happy about that). :) Also, the 
> name of that option would probably just be `AfterCaptureList`, which doesn't 
> help us in this case.
I get your point, but the space does not go anywhere in the clause/expression, 
so `AfterRequiresForClauses`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113369

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


[PATCH] D113319: [clang-format] Improve require and concept handling

2021-12-04 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:3464-3483
+  * ``RCPS_TwoLines`` (in configuration: ``TwoLines``)
+The clause always gets its own line, and the content of the clause go
+into the next line with some indentation.
+
+.. code-block:: c++
+
+  template

HazardyKnusperkeks wrote:
> Quuxplusone wrote:
> > This option strikes me as actively harmful; I think you should remove it 
> > from the PR completely. Nobody does this today and nobody should do it in 
> > the future either.
> I will never use it, so I have no strong opinion in that, because it was 
> really easy to add. I will let others decide if we want to offer it.
> it was really easy to add

Even easier not to add. ;)



Comment at: clang/include/clang/Format/Format.h:3130-3136
+  /// \brief The position of the ``requires`` clause for class templates.
+  /// \version 14
+  RequiresClausePositionStyle RequiresClausePositionForClasses;
+
+  /// \brief The position of the ``requires`` clause for function templates.
+  /// \version 14
+  RequiresClausePositionStyle RequiresClausePositionForFunctions;

HazardyKnusperkeks wrote:
> Quuxplusone wrote:
> > What about for variable templates? What about for alias templates? What 
> > about for deduction guides?
> > It makes sense to me to have //one// option for this. It doesn't make sense 
> > to have 2, 3, or 4. Having 5 feels insane. So, I think you should just have 
> > one option that applies consistently to all `requires`-clauses everywhere 
> > in the code.
> That makes sense, in my defense I thought about it and came to the conclusion 
> there would be no need for requires on variable templates, and the others I 
> did not think of.
> 
> Why I have chosen to options is maybe someone wants
> ```template 
> requires ...
> class ...```
> 
> But also
> ```template 
> void foo(T) requires ... {
> ```
> 
> What's your opinion on that?
Well, I see that there is a machine-readable distinction between "`requires` 
after `template`" versus "`requires` after function parameter list," i.e. 
between
```
template requires X
void f(T);

template
void f(T) requires X;
```
The latter position is not (currently) valid for non-functions (e.g. 
`template int v requires X;` is a syntax error).
However, as usual, I don't think that the user should //want// to format these 
differently.
If you're using clang-format to format your code, then you should definitely 
want function-parameter-list-requires-clauses on their own line for sure, to 
avoid confusing strings of tokens such as
```
void f() const noexcept requires foobar;  // bad

void f() const noexcept  // better
  requires foobar;
```
If you do that, then (in my proposed world) you would also automatically get
```
template
  requires foobar  // automatically
void f();

template requires foobar  // can't get this
void f();

template  // could also get this, via manual intervention
void f();
```
And personally I see nothing wrong with that state of affairs.



Comment at: clang/unittests/Format/FormatTest.cpp:22288-22291
+  verifyFormat("template \n"
+   "concept C = ((false || foo()) && C2) || "
+   "(std::trait::value && Baz) ||\n "
+   "   sizeof(T) >= 6;");

HazardyKnusperkeks wrote:
> Quuxplusone wrote:
> > Nit: I was initially very confused by the formatting here, until I realized 
> > that lines 22289 and 22290 are two lines //physically// but represent one 
> > single line in the //test case//. Strongly recommend eliminating the 
> > gratuitous line break.
> I'm not really happy wit the two solutions which come to my mind:
> Using a Style with a lower column limit, or using // clang format off.
I guess the higher-level question (which I didn't think of until seeing many 
other tests) is, redesign this test case to be more targeted and thus shorter. 
E.g. could you `s/std::trait::value/Bar/` or would that destroy the point of 
the test? (I'm not sure because I'm not sure what the point of the test is.)



Comment at: clang/unittests/Format/FormatTest.cpp:22374-22378
+  "template \n"
+  "concept C = decltype([]() { return std::true_type{}; }())::value &&\n"
+  "requires(T t) {\n"
+  "  t.bar();\n"
+  "} && sizeof(T) <= 8;");

HazardyKnusperkeks wrote:
> Quuxplusone wrote:
> > This looks wrong. Current:
> > ```
> > template \n"
> > concept C = decltype([]() { return std::true_type{}; }())::value &&
> > requires(T t) {
> >   t.bar();
> > } && sizeof(T) <= 8;
> > ```
> > but should be:
> > ```
> > template \n"
> > concept C = decltype([]() { return std::true_type{}; }())::value &&
> >   requires(T t) {
> > t.bar();
> >   } && sizeof(T) <= 8;
> > ```
> > (assuming that all the relevant indent-widths are set to `2`).
> For me personally it should look
> ```template \n"
> concept C = dec

[PATCH] D115064: [clang-format][NFC] Replace deque with vector

2021-12-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.
This revision is now accepted and ready to land.

This seems reasonable its as if we only ever push_back and iterate backwards?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115064

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


[PATCH] D115069: [clang-format][NFC] Merge another two calls to isOneOf

2021-12-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision.
MyDeveloperDay added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:1294-1296
+  Previous->isOneOf(tok::l_paren, tok::comma, tok::colon, 
TT_BinaryOperator,
+TT_ConditionalExpr) &&
   !Previous->isOneOf(TT_DictLiteral, TT_ObjCMethodExpr)) {

HazardyKnusperkeks wrote:
> curdeius wrote:
> > Unrelated to your change, it only shed light on this:
> > This condition looks strange to me. How can `Previous` be at the same time 
> > two different `TokenType`s (?), for instance, both `TT_BinaryOperator` and 
> > `TT_DictLiteral`?
> > 
> > Shouldn't it be this?
> Yeah, I will update it.
This is in my view one of the reasons why I personally like it when we comment 
the condition with an example and don't have these huge compound expressions.

Often they are covering corner cases. (we should call them out)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115069

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


[PATCH] D113369: [clang-format] Extend SpaceBeforeParens for requires

2021-12-04 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:3756
+  * ``bool AfterRequiresClause`` If ``true``, put space between requires 
keyword in a requires clause and
+opening parentheses, if is are one.
+

HazardyKnusperkeks wrote:
> curdeius wrote:
> > You meant "if there is one", right?
> Yeah
IMO these options should be named `InRequiresClause` and 
`InRequiresExpression`: that's where the space is going. The space doesn't go 
//after// the requires-clause. The space doesn't go //after// the 
requires-expression.
It occurs to me that the name of this option could be analogous to the name of 
the option that controls `[]() {}` versus `[] () {}`... except that it looks 
like there is no such option (and I'm happy about that). :) Also, the name of 
that option would probably just be `AfterCaptureList`, which doesn't help us in 
this case.



Comment at: clang/include/clang/Format/Format.h:3371
+/// If ``true``, put space between requires keyword in a requires clause 
and
+/// opening parentheses, if there is one.
+/// \code

Here and line 3380, and possibly elsewhere: `s/parentheses/parenthesis/`



Comment at: clang/unittests/Format/FormatTest.cpp:14369
+  "{}",
+  SpaceAfterRequires);
 }

Throughout, I'd like to see simpler test cases and more of them. E.g. I think 
this would suffice, in lieu of lines 14305–14369:
```
SpaceAfterRequires.SpaceBeforeParensOptions.InRequiresClause = true;
SpaceAfterRequires.SpaceBeforeParensOptions.InRequiresExpression = true;
verifyFormat("void f(auto x) requires (requires (int i) { x+i; }) { }", 
SpaceAfterRequires);
verifyFormat("if (requires (int i) { x+i; }) return;", SpaceAfterRequires);
verifyFormat("bool b = requires (int i) { x+i; };", SpaceAfterRequires);

SpaceAfterRequires.SpaceBeforeParensOptions.InRequiresClause = true;
SpaceAfterRequires.SpaceBeforeParensOptions.InRequiresExpression = false;
verifyFormat("void f(auto x) requires (requires(int i) { x+i; }) { }", 
SpaceAfterRequires);
verifyFormat("if (requires(int i) { x+i; }) return;", SpaceAfterRequires);
verifyFormat("bool b = requires(int i) { x+i; };", SpaceAfterRequires);

SpaceAfterRequires.SpaceBeforeParensOptions.InRequiresClause = false;
SpaceAfterRequires.SpaceBeforeParensOptions.InRequiresExpression = true;
verifyFormat("void f(auto x) requires(requires (int i) { x+i; }) { }", 
SpaceAfterRequires);
verifyFormat("if (requires (int i) { x+i; }) return;", SpaceAfterRequires);
verifyFormat("bool b = requires (int i) { x+i; };", SpaceAfterRequires);

SpaceAfterRequires.SpaceBeforeParensOptions.InRequiresClause = false;
SpaceAfterRequires.SpaceBeforeParensOptions.InRequiresExpression = false;
verifyFormat("void f(auto x) requires(requires(int i) { x+i; }) { }", 
SpaceAfterRequires);
verifyFormat("if (requires(int i) { x+i; }) return;", SpaceAfterRequires);
verifyFormat("bool b = requires(int i) { x+i; };", SpaceAfterRequires);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113369

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


[PATCH] D114533: LLVM IR should allow bitcast between address spaces with the same size.

2021-12-04 Thread krishna chaitanya sankisa via Phabricator via cfe-commits
skc7 updated this revision to Diff 391836.
skc7 added a comment.

updated test IR files.


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

https://reviews.llvm.org/D114533

Files:
  clang/lib/CodeGen/CGAtomic.cpp
  llvm/docs/LangRef.rst
  llvm/include/llvm/Analysis/TargetFolder.h
  llvm/include/llvm/IR/ConstantFolder.h
  llvm/include/llvm/IR/Constants.h
  llvm/include/llvm/IR/IRBuilder.h
  llvm/include/llvm/IR/IRBuilderFolder.h
  llvm/include/llvm/IR/InstrTypes.h
  llvm/include/llvm/IR/Instructions.h
  llvm/include/llvm/IR/NoFolder.h
  llvm/lib/Analysis/ConstantFolding.cpp
  llvm/lib/Analysis/LoopUnrollAnalyzer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/IR/AutoUpgrade.cpp
  llvm/lib/IR/Constants.cpp
  llvm/lib/IR/Instructions.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Transforms/Coroutines/Coroutines.cpp
  llvm/lib/Transforms/Utils/VNCoercion.cpp
  llvm/test/Transforms/GVN/gvn-eliminate-inttoptr-ptrtoint-for-load.ll
  llvm/test/Transforms/GVN/gvn-eliminate-inttoptr-ptrtoint-for-vector-load.ll
  
llvm/test/Transforms/GVN/gvn-eliminate-inttoptr-ptrtoint-for-vector-ptr-load.ll
  llvm/test/Verifier/bitcast-vector-pointer-as-neg.ll
  llvm/test/Verifier/bitcast-vector-pointer-different-addrspace-illegal.ll
  llvm/test/Verifier/bitcast-vector-pointer-neg.ll
  llvm/test/Verifier/bitcast-vector-pointer-pos.ll
  llvm/test/Verifier/bitcast-vector-pointer-same-addrspace.ll
  llvm/unittests/IR/InstructionsTest.cpp

Index: llvm/unittests/IR/InstructionsTest.cpp
===
--- llvm/unittests/IR/InstructionsTest.cpp
+++ llvm/unittests/IR/InstructionsTest.cpp
@@ -189,6 +189,10 @@
 TEST(InstructionsTest, CastInst) {
   LLVMContext C;
 
+  DataLayout DL("e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-"
+"p6:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-"
+"v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5-ni:7");
+
   Type *Int8Ty = Type::getInt8Ty(C);
   Type *Int16Ty = Type::getInt16Ty(C);
   Type *Int32Ty = Type::getInt32Ty(C);
@@ -217,7 +221,11 @@
   Type *Int32PtrAS1Ty = PointerType::get(Int32Ty, 1);
   Type *Int64PtrAS1Ty = PointerType::get(Int64Ty, 1);
 
+  Type *Int32PtrAS2Ty = PointerType::get(Int32Ty, 2);
+  Type *Int32PtrAS3Ty = PointerType::get(Int32Ty, 3);
+
   Type *V2Int32PtrAS1Ty = FixedVectorType::get(Int32PtrAS1Ty, 2);
+  Type *V2Int32PtrAS2Ty = FixedVectorType::get(Int32PtrAS2Ty, 2);
   Type *V2Int64PtrAS1Ty = FixedVectorType::get(Int64PtrAS1Ty, 2);
   Type *V4Int32PtrAS1Ty = FixedVectorType::get(Int32PtrAS1Ty, 4);
   Type *VScaleV4Int32PtrAS1Ty = ScalableVectorType::get(Int32PtrAS1Ty, 4);
@@ -238,50 +246,52 @@
   EXPECT_EQ(CastInst::Trunc, CastInst::getCastOpcode(c64, true, V8x8Ty, true));
   EXPECT_EQ(CastInst::SExt, CastInst::getCastOpcode(c8, true, V8x64Ty, true));
 
-  EXPECT_FALSE(CastInst::isBitCastable(V8x8Ty, X86MMXTy));
-  EXPECT_FALSE(CastInst::isBitCastable(X86MMXTy, V8x8Ty));
-  EXPECT_FALSE(CastInst::isBitCastable(Int64Ty, X86MMXTy));
-  EXPECT_FALSE(CastInst::isBitCastable(V8x64Ty, V8x8Ty));
-  EXPECT_FALSE(CastInst::isBitCastable(V8x8Ty, V8x64Ty));
-
-  // Check address space casts are rejected since we don't know the sizes here
-  EXPECT_FALSE(CastInst::isBitCastable(Int32PtrTy, Int32PtrAS1Ty));
-  EXPECT_FALSE(CastInst::isBitCastable(Int32PtrAS1Ty, Int32PtrTy));
-  EXPECT_FALSE(CastInst::isBitCastable(V2Int32PtrTy, V2Int32PtrAS1Ty));
-  EXPECT_FALSE(CastInst::isBitCastable(V2Int32PtrAS1Ty, V2Int32PtrTy));
-  EXPECT_TRUE(CastInst::isBitCastable(V2Int32PtrAS1Ty, V2Int64PtrAS1Ty));
+  EXPECT_FALSE(CastInst::isBitCastable(V8x8Ty, X86MMXTy, DL));
+  EXPECT_FALSE(CastInst::isBitCastable(X86MMXTy, V8x8Ty, DL));
+  EXPECT_FALSE(CastInst::isBitCastable(Int64Ty, X86MMXTy, DL));
+  EXPECT_FALSE(CastInst::isBitCastable(V8x64Ty, V8x8Ty, DL));
+  EXPECT_FALSE(CastInst::isBitCastable(V8x8Ty, V8x64Ty, DL));
+
+  // Check validity of casts here
+  EXPECT_TRUE(CastInst::isBitCastable(Int32PtrAS2Ty, Int32PtrAS3Ty, DL));
+  EXPECT_FALSE(CastInst::isBitCastable(Int32PtrAS1Ty, Int32PtrAS2Ty, DL));
+  EXPECT_TRUE(CastInst::isBitCastable(Int32PtrTy, Int32PtrAS1Ty, DL));
+  EXPECT_TRUE(CastInst::isBitCastable(Int32PtrAS1Ty, Int32PtrTy, DL));
+  EXPECT_TRUE(CastInst::isBitCastable(V2Int32PtrTy, V2Int32PtrAS1Ty, DL));
+  EXPECT_TRUE(CastInst::isBitCastable(V2Int32PtrAS1Ty, V2Int32PtrTy, DL));
+  EXPECT_TRUE(CastInst::isBitCastable(V2Int32PtrAS1Ty, V2Int64PtrAS1Ty, DL));
+  EXPECT_FALSE(CastInst::isBitCastable(V2Int32PtrAS1Ty, V2Int32PtrAS2Ty, DL));
   EXPECT_EQ(CastInst::AddrSpaceCast, CastInst::getCastOpcode(v2ptr32, true,
  V2Int32PtrAS1Ty,
  true));
 
   // Test mismatched number of elements for pointers
-  EXPECT_FALSE(CastInst::isBitCastable(V2Int32PtrAS1Ty, V4Int64PtrAS1Ty));
-  EXPECT_FALSE(CastInst::isBitCastable(V4Int64PtrAS1Ty, V2Int32PtrAS1Ty));
-  EXP

[PATCH] D115031: [AST] Print NTTP args as string-literals when possible

2021-12-04 Thread Zhihao Yuan via Phabricator via cfe-commits
lichray updated this revision to Diff 391833.
lichray marked an inline comment as done.
lichray added a comment.

- Switch to `llvm::SmallString`
- Refactor code that prints C-style builtin escape sequences
- Stop printing strings with embedded NULs in NTTP types
- Add an `EntireContentsOfLargeArray` pretty-print policy


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115031

Files:
  clang/include/clang/AST/PrettyPrinter.h
  clang/include/clang/Basic/CharInfo.h
  clang/lib/AST/APValue.cpp
  clang/lib/AST/Expr.cpp
  clang/test/SemaTemplate/temp_arg_string_printing.cpp

Index: clang/test/SemaTemplate/temp_arg_string_printing.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/temp_arg_string_printing.cpp
@@ -0,0 +1,141 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -ast-print %s | FileCheck %s
+
+using size_t = __SIZE_TYPE__;
+static_assert(__has_builtin(__make_integer_seq));
+
+template  class idx_seq {};
+template  using make_idx_seq = __make_integer_seq;
+
+template 
+struct Str {
+  constexpr Str(CharT const (&s)[N]) : Str(s, make_idx_seq()) {}
+  CharT value[N];
+
+private:
+  template 
+  constexpr Str(CharT const (&s)[N], idx_seq) : value{s[I]...} {}
+};
+
+template  class ASCII {};
+
+void not_string() {
+  // CHECK{LITERAL}: ASCII<{{9, -1, 42}}>
+  new ASCII<(int[]){9, -1, 42}>;
+  // CHECK{LITERAL}: ASCII<{{3.14e+00, 0.00e+00, 4.20e+01}}>
+  new ASCII<(double[]){3.14, 0., 42.}>;
+}
+
+void narrow() {
+  // CHECK{LITERAL}: ASCII<{""}>
+  new ASCII<"">;
+  // CHECK{LITERAL}: ASCII<{"the quick brown fox jumps"}>
+  new ASCII<"the quick brown fox jumps">;
+  // CHECK{LITERAL}: ASCII<{"OVER THE LAZY DOG 0123456789"}>
+  new ASCII<"OVER THE LAZY DOG 0123456789">;
+  // CHECK{LITERAL}: ASCII<{"\\`~!@#$%^&*()_+-={}[]|\'\";:,.<>?/"}>
+  new ASCII?/)">;
+  // CHECK{LITERAL}: ASCII<{{101, 115, 99, 97, 112, 101, 0, 0}}>
+  new ASCII<"escape\0">;
+  // CHECK{LITERAL}: ASCII<{"escape\r\n"}>
+  new ASCII<"escape\r\n">;
+  // CHECK{LITERAL}: ASCII<{"escape\\\t\f\v"}>
+  new ASCII<"escape\\\t\f\v">;
+  // CHECK{LITERAL}: ASCII<{"escape\a\bc"}>
+  new ASCII<"escape\a\b\c">;
+  // CHECK{LITERAL}: ASCII<{{110, 111, 116, 17, 0}}>
+  new ASCII<"not\x11">;
+  // CHECK{LITERAL}: ASCII<{{18, 20, 127, 16, 1, 32, 97, 98, 99, 0}}>
+  new ASCII<"\x12\x14\x7f\x10\x01 abc">;
+  // CHECK{LITERAL}: ASCII<{{18, 20, 127, 16, 1, 32, 97, 98, 99, 100, ...}}>
+  new ASCII<"\x12\x14\x7f\x10\x01 abcd">;
+  // CHECK{LITERAL}: ASCII<{"print more characters as string"}>
+  new ASCII<"print more characters as string">;
+  // CHECK{LITERAL}: ASCII<{"print even more characters as string"}>
+  new ASCII<"print even more characters as string">;
+  // CHECK{LITERAL}: ASCII<{"print many characters no more than[...]"}>
+  new ASCII<"print many characters no more than a limit">;
+  // CHECK{LITERAL}: ASCII<{"\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r"}>
+  new ASCII<"\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r">;
+  // CHECK{LITERAL}: ASCII<{"\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n[...]"}>
+  new ASCII<"\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n">;
+}
+
+void wide() {
+  // CHECK{LITERAL}: ASCII<{L""}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"the quick brown fox jumps"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"OVER THE LAZY DOG 0123456789"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"\\`~!@#$%^&*()_+-={}[]|\'\";:,.<>?/"}>
+  new ASCII?/)">;
+  // CHECK{LITERAL}: ASCII<{{101, 115, 99, 97, 112, 101, 0, 0}}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"escape\r\n"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"escape\\\t\f\v"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"escape\a\bc"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{{110, 111, 116, 17, 0}}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{{18, 20, 255, 22909, 136, 32, 97, 98, 99, 0}}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{{18, 20, 255, 22909, 136, 32, 97, 98, 99, 100, ...}}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"print more characters as string"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"print even more characters as string"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"print many characters no more than[...]"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r\r"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{L"\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n[...]"}>
+  new ASCII;
+}
+
+void utf8() {
+  // CHECK{LITERAL}: ASCII<{u8""}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{u8"\\`~!@#$%^&*()_+-={}[]|\'\";:,.<>?/"}>
+  new ASCII?/)">;
+  // CHECK{LITERAL}: ASCII<{{101, 115, 99, 97, 112, 101, 0, 0}}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{u8"escape\r\n"}>
+  new ASCII;
+  // CHECK{LITERAL}: ASCII<{{229,

[PATCH] D115031: [AST] Print NTTP args as string-literals when possible

2021-12-04 Thread Zhihao Yuan via Phabricator via cfe-commits
lichray marked 5 inline comments as done.
lichray added inline comments.



Comment at: clang/lib/AST/APValue.cpp:637-639
+  // Nothing we can do about a sequence that is not null-terminated
+  if (!Data[--Size].getInt().isZero())
+return false;

lichray wrote:
> lichray wrote:
> > rsmith wrote:
> > > lichray wrote:
> > > > rsmith wrote:
> > > > > We should drop all trailing zeroes here, because array initialization 
> > > > > from a string literal will reconstruct them.
> > > > Are you sure? `MyType<{""}>` and `MyType<{"\0\0\0"}>` are different 
> > > > types.
> > > That's a separate bug. `APValue`'s printing is not intended to identify 
> > > the type of the result, only to identify the value in the case where we 
> > > already know the type. Eg, we don't print a type suffix on a 
> > > pretty-printed integer value. For the specific case of template 
> > > parameters, it's the responsibility of the template argument printing 
> > > code to uniquely identify the template argument.
> > > 
> > > When a template parameter has a deduced class type, we should include 
> > > that type in the printed output in order to uniquely identify the 
> > > specialization, but we don't, because that's not been implemented yet. 
> > > When that's fixed, we should print those cases as `MyType > > 1>{""}>` and `MyType{""}>`. This is necessary anyway, 
> > > because we can't in general assume that the resulting value is enough to 
> > > indicate what type CTAD will have selected.
> > > 
> > > We already handle this properly in some cases, but see this FIXMEs: 
> > > https://github.com/llvm/llvm-project/blob/6c2be3015e07f25912b8cd5b75214c532f568638/clang/lib/AST/TemplateBase.cpp#L433
> > > 
> > > See also some related issues: https://godbolt.org/z/YhcdMYx6n (note that 
> > > the non-struct enum case is handled properly but the enum-in-struct case 
> > > is not).
> > > That's a separate bug. [...]
> > > 
> > > When a template parameter has a deduced class type, we should include 
> > > that type in the printed output in order to uniquely identify the 
> > > specialization, but we don't, because that's not been implemented yet. 
> > > When that's fixed, we should print those cases as `MyType > > 1>{""}>` and `MyType{""}>`. This is necessary anyway, 
> > > because we can't in general assume that the resulting value is enough to 
> > > indicate what type CTAD will have selected.
> > >
> > 
> > But it looks like a feature rather than a bug? String literals provided 
> > both type and value to emphasis a structural type's value for equivalence 
> > comparison. The reason of why `MyType<{""}>` and `MyType<"\0\0\0">` are 
> > different looks more obvious to me.
> > 
> > > See also some related issues: https://godbolt.org/z/YhcdMYx6n (note that 
> > > the non-struct enum case is handled properly but the enum-in-struct case 
> > > is not).
> > 
> > I'm fine if a specific NTTP, `A`, is treated differently from `auto`. But 
> > the last case is similar to this https://godbolt.org/z/YcscTrchM Which I 
> > thought about printing something like `Y<{(E[]){97, 98}}>` :)
> That discussion looks OT... For now, not shrinking `"\0\0\0"` down to `""` 
> follows the existing logic, i.e., we are printing `{0, 0, 0}` even though 
> it's an array.
> 
Nvm, I did not realize that null characters cannot be safely escaped with just 
`\0` without looking at the characters that follow it. Dropping that case and 
fallback to print the integer sequence.



Comment at: clang/lib/AST/APValue.cpp:645
+
+  // Better than printing a two-digit sequence of 10 integers.
+  StringRef Ellipsis;

rsmith wrote:
> ...except that some callers want output that uniquely identifies the template 
> argument, and moreover some callers want output that is valid C++ code that 
> produces the same template-id.
> 
> It'd be better to do this behind a `PrintingPolicy` flag that the diagnostics 
> engine sets. But we'd want to turn that flag off during template type diffing.
Added `Policy.EntireContentsOfLargeArray`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115031

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


[PATCH] D113179: [Passes] Move AggressiveInstCombine after InstCombine

2021-12-04 Thread Anton Afanasyev 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 rGc34d157fc739: [Passes] Move AggressiveInstCombine after 
InstCombine (authored by anton-afanasyev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113179

Files:
  clang/test/CodeGen/thinlto-distributed-newpm.ll
  llvm/lib/Passes/PassBuilderPipelines.cpp
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-lto-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
  llvm/test/Transforms/PhaseOrdering/X86/pr52289.ll

Index: llvm/test/Transforms/PhaseOrdering/X86/pr52289.ll
===
--- llvm/test/Transforms/PhaseOrdering/X86/pr52289.ll
+++ llvm/test/Transforms/PhaseOrdering/X86/pr52289.ll
@@ -6,16 +6,6 @@
 define i32 @main(i32 %a) {
 ; CHECK-LABEL: @main(
 ; CHECK-NEXT:  if.end:
-; CHECK-NEXT:[[TMP0:%.*]] = trunc i32 [[A:%.*]] to i8
-; CHECK-NEXT:[[TMP1:%.*]] = add i8 [[TMP0]], 1
-; CHECK-NEXT:[[CONV:%.*]] = and i8 [[TMP1]], 8
-; CHECK-NEXT:[[CMP_I_NOT:%.*]] = icmp eq i8 [[CONV]], 0
-; CHECK-NEXT:[[SHL_I:%.*]] = select i1 [[CMP_I_NOT]], i8 7, i8 0
-; CHECK-NEXT:[[COND_I:%.*]] = shl i8 [[CONV]], [[SHL_I]]
-; CHECK-NEXT:[[CONV1:%.*]] = sext i8 [[COND_I]] to i32
-; CHECK-NEXT:[[SEXT:%.*]] = mul i32 [[CONV1]], 1355350016
-; CHECK-NEXT:[[TOBOOL:%.*]] = icmp ne i32 [[SEXT]], 65536
-; CHECK-NEXT:tail call void @llvm.assume(i1 [[TOBOOL]])
 ; CHECK-NEXT:ret i32 0
 ;
   %inc = add nsw i32 %a, 1
Index: llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
===
--- llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
+++ llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
@@ -95,8 +95,8 @@
 ; CHECK-O23SZ-NEXT: Running pass: CorrelatedValuePropagationPass
 ; CHECK-O23SZ-NEXT: Invalidating analysis: LazyValueAnalysis
 ; CHECK-O-NEXT: Running pass: SimplifyCFGPass
-; CHECK-O3-NEXT: Running pass: AggressiveInstCombinePass
 ; CHECK-O-NEXT: Running pass: InstCombinePass
+; CHECK-O3-NEXT: Running pass: AggressiveInstCombinePass
 ; CHECK-O1-NEXT: Running pass: LibCallsShrinkWrapPass
 ; CHECK-O2-NEXT: Running pass: LibCallsShrinkWrapPass
 ; CHECK-O3-NEXT: Running pass: LibCallsShrinkWrapPass
Index: llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
===
--- llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
+++ llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
@@ -121,7 +121,6 @@
 ; CHECK-O23SZ-NEXT: Running pass: CorrelatedValuePropagationPass
 ; CHECK-O23SZ-NEXT: Invalidating analysis: LazyValueAnalysis
 ; CHECK-O-NEXT: Running pass: SimplifyCFGPass
-; CHECK-O3-NEXT: Running pass: AggressiveInstCombinePass
 ; CHECK-O-NEXT: Running pass: InstCombinePass
 ; CHECK-O-NEXT: Running analysis: BlockFrequencyAnalysis on foo
 ; These next two can appear in any order since they are accessed as parameters
@@ -129,6 +128,7 @@
 ; CHECK-O-DAG: Running analysis: LoopAnalysis on foo
 ; CHECK-O-DAG: Running analysis: BranchProbabilityAnalysis on foo
 ; CHECK-O-DAG: Running analysis: PostDominatorTreeAnalysis on foo
+; CHECK-O3-NEXT: Running pass: AggressiveInstCombinePass
 ; CHECK-O1-NEXT: Running pass: LibCallsShrinkWrapPass
 ; CHECK-O2-NEXT: Running pass: LibCallsShrinkWrapPass
 ; CHECK-O3-NEXT: Running pass: LibCallsShrinkWrapPass
Index: llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
===
--- llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
+++ llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
@@ -100,8 +100,8 @@
 ; CHECK-O23SZ-NEXT: Running pass: CorrelatedValuePropagationPass
 ; CHECK-O23SZ-NEXT: Invalidating analysis: LazyValueAnalysis
 ; CHECK-O-NEXT: Running pass: SimplifyCFGPass
-; CHECK-O3-NEXT: Running pass: AggressiveInstCombinePass
 ; CHECK-O-NEXT: Running pass: InstCombinePass
+; CHECK-O3-NEXT: Running pass: AggressiveInstCombinePass
 ; CHECK-O1-NEXT: Running pass: LibCallsShrinkWrapPass
 ; CHECK-O2-NEXT: Running pass: LibCallsShrinkWrapPass
 ; CHECK-O3-NEXT: Running pass: LibCallsShrinkWrapPass
Index: llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
===
--- llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
+++ llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
@@ -91,8 +91,8 @@
 ; CHECK-O23SZ-NEXT: Running pass: CorrelatedValuePropagationPass
 ; CHECK-O23SZ-NEXT: Invalidating analysis: LazyValueAnalysis
 ; C

[clang] c34d157 - [Passes] Move AggressiveInstCombine after InstCombine

2021-12-04 Thread Anton Afanasyev via cfe-commits

Author: Anton Afanasyev
Date: 2021-12-04T14:22:43+03:00
New Revision: c34d157fc73954366371aaca6291739921933e86

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

LOG: [Passes] Move AggressiveInstCombine after InstCombine

Swap AIC and IC neighbouring in pipeline. This looks more natural and even
almost has no effect for now (three slightly touched tests of test-suite). Also
this could be the first step towards merging AIC (or its part) to -O2 pipeline.

After several changes in AIC (like D108091, D108201, D107766, D109515, D109236)
there've been observed several regressions (like PR52078, PR52253, PR52289)
that were fixed in different passes (see D111330, D112721) by extending their
functionality, but these regressions were exposed since changed AIC prevents IC
from making some of early optimizations.

This is common problem and it should be fixed by just moving AIC after IC
which looks more logically by itself: make aggressive instruction combining
only after failed ordinary one.

Fixes PR52289

Reviewed By: spatel, RKSimon

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

Added: 


Modified: 
clang/test/CodeGen/thinlto-distributed-newpm.ll
llvm/lib/Passes/PassBuilderPipelines.cpp
llvm/test/Other/new-pm-defaults.ll
llvm/test/Other/new-pm-lto-defaults.ll
llvm/test/Other/new-pm-thinlto-defaults.ll
llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
llvm/test/Transforms/PhaseOrdering/X86/pr52289.ll

Removed: 




diff  --git a/clang/test/CodeGen/thinlto-distributed-newpm.ll 
b/clang/test/CodeGen/thinlto-distributed-newpm.ll
index 8ec8b7c5e4aff..177d0375c53b1 100644
--- a/clang/test/CodeGen/thinlto-distributed-newpm.ll
+++ b/clang/test/CodeGen/thinlto-distributed-newpm.ll
@@ -46,8 +46,8 @@
 ; CHECK-O: Running pass: JumpThreadingPass on main
 ; CHECK-O: Running pass: CorrelatedValuePropagationPass on main
 ; CHECK-O: Running pass: SimplifyCFGPass on main
-; CHECK-O3: Running pass: AggressiveInstCombinePass on main
 ; CHECK-O: Running pass: InstCombinePass on main
+; CHECK-O3: Running pass: AggressiveInstCombinePass on main
 ; CHECK-O: Running pass: LibCallsShrinkWrapPass on main
 ; CHECK-O: Running pass: TailCallElimPass on main
 ; CHECK-O: Running pass: SimplifyCFGPass on main

diff  --git a/llvm/lib/Passes/PassBuilderPipelines.cpp 
b/llvm/lib/Passes/PassBuilderPipelines.cpp
index 46d6b056e41ba..0cc329c94fdd3 100644
--- a/llvm/lib/Passes/PassBuilderPipelines.cpp
+++ b/llvm/lib/Passes/PassBuilderPipelines.cpp
@@ -422,9 +422,9 @@ 
PassBuilder::buildFunctionSimplificationPipeline(OptimizationLevel Level,
   FPM.addPass(CorrelatedValuePropagationPass());
 
   FPM.addPass(SimplifyCFGPass());
+  FPM.addPass(InstCombinePass());
   if (Level == OptimizationLevel::O3)
 FPM.addPass(AggressiveInstCombinePass());
-  FPM.addPass(InstCombinePass());
 
   if (!Level.isOptimizingForSize())
 FPM.addPass(LibCallsShrinkWrapPass());
@@ -1536,9 +1536,9 @@ PassBuilder::buildLTODefaultPipeline(OptimizationLevel 
Level,
   // function pointers.  When this happens, we often have to resolve varargs
   // calls, etc, so let instcombine do this.
   FunctionPassManager PeepholeFPM;
+  PeepholeFPM.addPass(InstCombinePass());
   if (Level == OptimizationLevel::O3)
 PeepholeFPM.addPass(AggressiveInstCombinePass());
-  PeepholeFPM.addPass(InstCombinePass());
   invokePeepholeEPCallbacks(PeepholeFPM, Level);
 
   MPM.addPass(createModuleToFunctionPassAdaptor(std::move(PeepholeFPM),

diff  --git a/llvm/test/Other/new-pm-defaults.ll 
b/llvm/test/Other/new-pm-defaults.ll
index 0539acc32b8d3..71875840bf84c 100644
--- a/llvm/test/Other/new-pm-defaults.ll
+++ b/llvm/test/Other/new-pm-defaults.ll
@@ -138,8 +138,8 @@
 ; CHECK-O23SZ-NEXT: Running pass: CorrelatedValuePropagationPass
 ; CHECK-O23SZ-NEXT: Invalidating analysis: LazyValueAnalysis
 ; CHECK-O-NEXT: Running pass: SimplifyCFGPass
-; CHECK-O3-NEXT: AggressiveInstCombinePass
 ; CHECK-O-NEXT: Running pass: InstCombinePass
+; CHECK-O3-NEXT: AggressiveInstCombinePass
 ; CHECK-O1-NEXT: Running pass: LibCallsShrinkWrapPass
 ; CHECK-O2-NEXT: Running pass: LibCallsShrinkWrapPass
 ; CHECK-O3-NEXT: Running pass: LibCallsShrinkWrapPass

diff  --git a/llvm/test/Other/new-pm-lto-defaults.ll 
b/llvm/test/Other/new-pm-lto-defaults.ll
index a40c366171f55..73fe5f20c91ea 100644
--- a/llvm/test/Other/new-pm-lto-defaults.ll
+++ b/llvm/test/Other/new-pm-lto-defaults.ll
@@ -65,8 +65,8 @@
 ; CHECK-O23SZ-NEXT: Running pass: PromotePass
 ; CHECK-O23SZ-NEXT: Running pass: ConstantMergePass
 ; CHECK-O23SZ-NEXT: Running pass: DeadArgumentEliminationPass
-; CH

[PATCH] D113179: [Passes] Move AggressiveInstCombine after InstCombine

2021-12-04 Thread Anton Afanasyev via Phabricator via cfe-commits
anton-afanasyev updated this revision to Diff 391823.
anton-afanasyev added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Update tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113179

Files:
  clang/test/CodeGen/thinlto-distributed-newpm.ll
  llvm/lib/Passes/PassBuilderPipelines.cpp
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-lto-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
  llvm/test/Transforms/PhaseOrdering/X86/pr52289.ll

Index: llvm/test/Transforms/PhaseOrdering/X86/pr52289.ll
===
--- llvm/test/Transforms/PhaseOrdering/X86/pr52289.ll
+++ llvm/test/Transforms/PhaseOrdering/X86/pr52289.ll
@@ -6,16 +6,6 @@
 define i32 @main(i32 %a) {
 ; CHECK-LABEL: @main(
 ; CHECK-NEXT:  if.end:
-; CHECK-NEXT:[[TMP0:%.*]] = trunc i32 [[A:%.*]] to i8
-; CHECK-NEXT:[[TMP1:%.*]] = add i8 [[TMP0]], 1
-; CHECK-NEXT:[[CONV:%.*]] = and i8 [[TMP1]], 8
-; CHECK-NEXT:[[CMP_I_NOT:%.*]] = icmp eq i8 [[CONV]], 0
-; CHECK-NEXT:[[SHL_I:%.*]] = select i1 [[CMP_I_NOT]], i8 7, i8 0
-; CHECK-NEXT:[[COND_I:%.*]] = shl i8 [[CONV]], [[SHL_I]]
-; CHECK-NEXT:[[CONV1:%.*]] = sext i8 [[COND_I]] to i32
-; CHECK-NEXT:[[SEXT:%.*]] = mul i32 [[CONV1]], 1355350016
-; CHECK-NEXT:[[TOBOOL:%.*]] = icmp ne i32 [[SEXT]], 65536
-; CHECK-NEXT:tail call void @llvm.assume(i1 [[TOBOOL]])
 ; CHECK-NEXT:ret i32 0
 ;
   %inc = add nsw i32 %a, 1
Index: llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
===
--- llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
+++ llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
@@ -95,8 +95,8 @@
 ; CHECK-O23SZ-NEXT: Running pass: CorrelatedValuePropagationPass
 ; CHECK-O23SZ-NEXT: Invalidating analysis: LazyValueAnalysis
 ; CHECK-O-NEXT: Running pass: SimplifyCFGPass
-; CHECK-O3-NEXT: Running pass: AggressiveInstCombinePass
 ; CHECK-O-NEXT: Running pass: InstCombinePass
+; CHECK-O3-NEXT: Running pass: AggressiveInstCombinePass
 ; CHECK-O1-NEXT: Running pass: LibCallsShrinkWrapPass
 ; CHECK-O2-NEXT: Running pass: LibCallsShrinkWrapPass
 ; CHECK-O3-NEXT: Running pass: LibCallsShrinkWrapPass
Index: llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
===
--- llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
+++ llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
@@ -121,7 +121,6 @@
 ; CHECK-O23SZ-NEXT: Running pass: CorrelatedValuePropagationPass
 ; CHECK-O23SZ-NEXT: Invalidating analysis: LazyValueAnalysis
 ; CHECK-O-NEXT: Running pass: SimplifyCFGPass
-; CHECK-O3-NEXT: Running pass: AggressiveInstCombinePass
 ; CHECK-O-NEXT: Running pass: InstCombinePass
 ; CHECK-O-NEXT: Running analysis: BlockFrequencyAnalysis on foo
 ; These next two can appear in any order since they are accessed as parameters
@@ -129,6 +128,7 @@
 ; CHECK-O-DAG: Running analysis: LoopAnalysis on foo
 ; CHECK-O-DAG: Running analysis: BranchProbabilityAnalysis on foo
 ; CHECK-O-DAG: Running analysis: PostDominatorTreeAnalysis on foo
+; CHECK-O3-NEXT: Running pass: AggressiveInstCombinePass
 ; CHECK-O1-NEXT: Running pass: LibCallsShrinkWrapPass
 ; CHECK-O2-NEXT: Running pass: LibCallsShrinkWrapPass
 ; CHECK-O3-NEXT: Running pass: LibCallsShrinkWrapPass
Index: llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
===
--- llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
+++ llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
@@ -100,8 +100,8 @@
 ; CHECK-O23SZ-NEXT: Running pass: CorrelatedValuePropagationPass
 ; CHECK-O23SZ-NEXT: Invalidating analysis: LazyValueAnalysis
 ; CHECK-O-NEXT: Running pass: SimplifyCFGPass
-; CHECK-O3-NEXT: Running pass: AggressiveInstCombinePass
 ; CHECK-O-NEXT: Running pass: InstCombinePass
+; CHECK-O3-NEXT: Running pass: AggressiveInstCombinePass
 ; CHECK-O1-NEXT: Running pass: LibCallsShrinkWrapPass
 ; CHECK-O2-NEXT: Running pass: LibCallsShrinkWrapPass
 ; CHECK-O3-NEXT: Running pass: LibCallsShrinkWrapPass
Index: llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
===
--- llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
+++ llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
@@ -91,8 +91,8 @@
 ; CHECK-O23SZ-NEXT: Running pass: CorrelatedValuePropagationPass
 ; CHECK-O23SZ-NEXT: Invalidating analysis: LazyValueAnalysis
 ; CHECK-O-NEXT: Running pass: SimplifyCFGPass
-; CHECK-O3-NEXT: Running pass: A

[PATCH] D115032: [AMDGPU] Change llvm.amdgcn.image.bvh.intersect.ray to take vec3 args

2021-12-04 Thread Jay Foad 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 rG2774bad11242: [AMDGPU] Change 
llvm.amdgcn.image.bvh.intersect.ray to take vec3 args (authored by foad).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115032

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGenOpenCL/builtins-amdgcn-raytracing.cl
  llvm/include/llvm/IR/IntrinsicsAMDGPU.td
  llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.intersect_ray.ll
  llvm/test/CodeGen/AMDGPU/llvm.amdgcn.intersect_ray.ll

Index: llvm/test/CodeGen/AMDGPU/llvm.amdgcn.intersect_ray.ll
===
--- llvm/test/CodeGen/AMDGPU/llvm.amdgcn.intersect_ray.ll
+++ llvm/test/CodeGen/AMDGPU/llvm.amdgcn.intersect_ray.ll
@@ -3,15 +3,15 @@
 ; RUN: llc -march=amdgcn -mcpu=gfx1030 -verify-machineinstrs < %s | FileCheck -check-prefixes=GCN,GFX1030 %s
 ; RUN: not --crash llc -march=amdgcn -mcpu=gfx1012 -verify-machineinstrs < %s 2>&1 | FileCheck -check-prefix=ERR %s
 
-; uint4 llvm.amdgcn.image.bvh.intersect.ray.i32.v4f32(uint node_ptr, float ray_extent, float4 ray_origin, float4 ray_dir, float4 ray_inv_dir, uint4 texture_descr)
-; uint4 llvm.amdgcn.image.bvh.intersect.ray.i32.v4f16(uint node_ptr, float ray_extent, float4 ray_origin, half4 ray_dir, half4 ray_inv_dir, uint4 texture_descr)
-; uint4 llvm.amdgcn.image.bvh.intersect.ray.i64.v4f32(ulong node_ptr, float ray_extent, float4 ray_origin, float4 ray_dir, float4 ray_inv_dir, uint4 texture_descr)
-; uint4 llvm.amdgcn.image.bvh.intersect.ray.i64.v4f16(ulong node_ptr, float ray_extent, float4 ray_origin, half4 ray_dir, half4 ray_inv_dir, uint4 texture_descr)
+; uint4 llvm.amdgcn.image.bvh.intersect.ray.i32.v4f32(uint node_ptr, float ray_extent, float3 ray_origin, float3 ray_dir, float3 ray_inv_dir, uint4 texture_descr)
+; uint4 llvm.amdgcn.image.bvh.intersect.ray.i32.v4f16(uint node_ptr, float ray_extent, float3 ray_origin, half3 ray_dir, half3 ray_inv_dir, uint4 texture_descr)
+; uint4 llvm.amdgcn.image.bvh.intersect.ray.i64.v4f32(ulong node_ptr, float ray_extent, float3 ray_origin, float3 ray_dir, float3 ray_inv_dir, uint4 texture_descr)
+; uint4 llvm.amdgcn.image.bvh.intersect.ray.i64.v4f16(ulong node_ptr, float ray_extent, float3 ray_origin, half3 ray_dir, half3 ray_inv_dir, uint4 texture_descr)
 
-declare <4 x i32> @llvm.amdgcn.image.bvh.intersect.ray.i32.v4f32(i32, float, <4 x float>, <4 x float>, <4 x float>, <4 x i32>)
-declare <4 x i32> @llvm.amdgcn.image.bvh.intersect.ray.i32.v4f16(i32, float, <4 x float>, <4 x half>, <4 x half>, <4 x i32>)
-declare <4 x i32> @llvm.amdgcn.image.bvh.intersect.ray.i64.v4f32(i64, float, <4 x float>, <4 x float>, <4 x float>, <4 x i32>)
-declare <4 x i32> @llvm.amdgcn.image.bvh.intersect.ray.i64.v4f16(i64, float, <4 x float>, <4 x half>, <4 x half>, <4 x i32>)
+declare <4 x i32> @llvm.amdgcn.image.bvh.intersect.ray.i32.v4f32(i32, float, <3 x float>, <3 x float>, <3 x float>, <4 x i32>)
+declare <4 x i32> @llvm.amdgcn.image.bvh.intersect.ray.i32.v4f16(i32, float, <3 x float>, <3 x half>, <3 x half>, <4 x i32>)
+declare <4 x i32> @llvm.amdgcn.image.bvh.intersect.ray.i64.v4f32(i64, float, <3 x float>, <3 x float>, <3 x float>, <4 x i32>)
+declare <4 x i32> @llvm.amdgcn.image.bvh.intersect.ray.i64.v4f16(i64, float, <3 x float>, <3 x half>, <3 x half>, <4 x i32>)
 
 ; ERR: in function image_bvh_intersect_ray{{.*}}intrinsic not supported on subtarget
 ; Arguments are flattened to represent the actual VGPR_A layout, so we have no
@@ -23,43 +23,43 @@
 ; GCN-NEXT:s_waitcnt vmcnt(0)
 ; GCN-NEXT:; return to shader part epilog
 main_body:
-  %ray_origin0 = insertelement <4 x float> undef, float %ray_origin_x, i32 0
-  %ray_origin1 = insertelement <4 x float> %ray_origin0, float %ray_origin_y, i32 1
-  %ray_origin = insertelement <4 x float> %ray_origin1, float %ray_origin_z, i32 2
-  %ray_dir0 = insertelement <4 x float> undef, float %ray_dir_x, i32 0
-  %ray_dir1 = insertelement <4 x float> %ray_dir0, float %ray_dir_y, i32 1
-  %ray_dir = insertelement <4 x float> %ray_dir1, float %ray_dir_z, i32 2
-  %ray_inv_dir0 = insertelement <4 x float> undef, float %ray_inv_dir_x, i32 0
-  %ray_inv_dir1 = insertelement <4 x float> %ray_inv_dir0, float %ray_inv_dir_y, i32 1
-  %ray_inv_dir = insertelement <4 x float> %ray_inv_dir1, float %ray_inv_dir_z, i32 2
-  %v = call <4 x i32> @llvm.amdgcn.image.bvh.intersect.ray.i32.v4f32(i32 %node_ptr, float %ray_extent, <4 x float> %ray_origin, <4 x float> %ray_dir, <4 x float> %ray_inv_dir, <4 x i32> %tdescr)
+  %ray_origin0 = insertelement <3 x float> undef, float %ray_origin_x, i32 0
+  %ray_origin1 = insertelement <3 x float> %ray_origin0, float %ray_origin_y, i32 1
+  %ray_origin = insertelement <3 x float> %ray_origin1, float

[clang] 2774bad - [AMDGPU] Change llvm.amdgcn.image.bvh.intersect.ray to take vec3 args

2021-12-04 Thread Jay Foad via cfe-commits

Author: Jay Foad
Date: 2021-12-04T10:32:11Z
New Revision: 2774bad1124215571ab154afcb5478c78cf46344

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

LOG: [AMDGPU] Change llvm.amdgcn.image.bvh.intersect.ray to take vec3 args

The ray_origin, ray_dir and ray_inv_dir arguments should all be vec3 to
match how the hardware instruction works.

Don't change the API of the corresponding OpenCL builtins.

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

Added: 


Modified: 
clang/lib/CodeGen/CGBuiltin.cpp
clang/test/CodeGenOpenCL/builtins-amdgcn-raytracing.cl
llvm/include/llvm/IR/IntrinsicsAMDGPU.td
llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp
llvm/lib/Target/AMDGPU/SIISelLowering.cpp
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.intersect_ray.ll
llvm/test/CodeGen/AMDGPU/llvm.amdgcn.intersect_ray.ll

Removed: 




diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 0a98b5b633223..44a24d23d3c85 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -16592,6 +16592,15 @@ Value *CodeGenFunction::EmitAMDGPUBuiltinExpr(unsigned 
BuiltinID,
 llvm::Value *RayInverseDir = EmitScalarExpr(E->getArg(4));
 llvm::Value *TextureDescr = EmitScalarExpr(E->getArg(5));
 
+// The builtins take these arguments as vec4 where the last element is
+// ignored. The intrinsic takes them as vec3.
+RayOrigin = Builder.CreateShuffleVector(RayOrigin, RayOrigin,
+ArrayRef{0, 1, 2});
+RayDir =
+Builder.CreateShuffleVector(RayDir, RayDir, ArrayRef{0, 1, 2});
+RayInverseDir = Builder.CreateShuffleVector(RayInverseDir, RayInverseDir,
+ArrayRef{0, 1, 2});
+
 Function *F = CGM.getIntrinsic(Intrinsic::amdgcn_image_bvh_intersect_ray,
{NodePtr->getType(), RayDir->getType()});
 return Builder.CreateCall(F, {NodePtr, RayExtent, RayOrigin, RayDir,

diff  --git a/clang/test/CodeGenOpenCL/builtins-amdgcn-raytracing.cl 
b/clang/test/CodeGenOpenCL/builtins-amdgcn-raytracing.cl
index 805d17a392b31..3c90c9a495e09 100644
--- a/clang/test/CodeGenOpenCL/builtins-amdgcn-raytracing.cl
+++ b/clang/test/CodeGenOpenCL/builtins-amdgcn-raytracing.cl
@@ -19,7 +19,7 @@ typedef double double4 __attribute__((ext_vector_type(4)));
 typedef half half4 __attribute__((ext_vector_type(4)));
 typedef uint uint4 __attribute__((ext_vector_type(4)));
 
-// CHECK: call <4 x i32> @llvm.amdgcn.image.bvh.intersect.ray.i32.v4f32
+// CHECK: call <4 x i32> @llvm.amdgcn.image.bvh.intersect.ray.i32.v3f32
 // ISA: image_bvh_intersect_ray
 void test_image_bvh_intersect_ray(global uint4* out, uint node_ptr,
   float ray_extent, float4 ray_origin, float4 ray_dir, float4 ray_inv_dir,
@@ -29,7 +29,7 @@ void test_image_bvh_intersect_ray(global uint4* out, uint 
node_ptr,
ray_origin, ray_dir, ray_inv_dir, texture_descr);
 }
 
-// CHECK: call <4 x i32> @llvm.amdgcn.image.bvh.intersect.ray.i32.v4f16
+// CHECK: call <4 x i32> @llvm.amdgcn.image.bvh.intersect.ray.i32.v3f16
 // ISA: image_bvh_intersect_ray
 void test_image_bvh_intersect_ray_h(global uint4* out, uint node_ptr,
   float ray_extent, float4 ray_origin, half4 ray_dir, half4 ray_inv_dir,
@@ -39,7 +39,7 @@ void test_image_bvh_intersect_ray_h(global uint4* out, uint 
node_ptr,
ray_origin, ray_dir, ray_inv_dir, texture_descr);
 }
 
-// CHECK: call <4 x i32> @llvm.amdgcn.image.bvh.intersect.ray.i64.v4f32
+// CHECK: call <4 x i32> @llvm.amdgcn.image.bvh.intersect.ray.i64.v3f32
 // ISA: image_bvh_intersect_ray
 void test_image_bvh_intersect_ray_l(global uint4* out, ulong node_ptr,
   float ray_extent, float4 ray_origin, float4 ray_dir, float4 ray_inv_dir,
@@ -49,7 +49,7 @@ void test_image_bvh_intersect_ray_l(global uint4* out, ulong 
node_ptr,
ray_origin, ray_dir, ray_inv_dir, texture_descr);
 }
 
-// CHECK: call <4 x i32> @llvm.amdgcn.image.bvh.intersect.ray.i64.v4f16
+// CHECK: call <4 x i32> @llvm.amdgcn.image.bvh.intersect.ray.i64.v3f16
 // ISA: image_bvh_intersect_ray
 void test_image_bvh_intersect_ray_lh(global uint4* out, ulong node_ptr,
   float ray_extent, float4 ray_origin, half4 ray_dir, half4 ray_inv_dir,

diff  --git a/llvm/include/llvm/IR/IntrinsicsAMDGPU.td 
b/llvm/include/llvm/IR/IntrinsicsAMDGPU.td
index 0a44670de76e7..91c2dc0102597 100644
--- a/llvm/include/llvm/IR/IntrinsicsAMDGPU.td
+++ b/llvm/include/llvm/IR/IntrinsicsAMDGPU.td
@@ -1789,9 +1789,11 @@ def int_amdgcn_global_atomic_csub : 
AMDGPUGlobalAtomicRtn;
 
 // uint4 llvm.amdgcn.image.bvh.intersect.ray , , 
,
 //   , , 

+//  is i32 or i64.
+//  and  are both v3f16 or both v3f32.
 def int_amdgcn_image_bvh_i

[PATCH] D115094: Fix -Wdeclaration-after-statement doesn't work when used with -std=c99

2021-12-04 Thread ram via Phabricator via cfe-commits
ram7bhupal created this revision.
ram7bhupal added reviewers: dblaikie, aaron.ballman.
ram7bhupal requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

-Wdeclaration-after-statement doesn't do anything if combined with -std=c99 or 
newer.

Take a look at the following program:

// prog.c
#include 

int main(void)
{

  printf("hello world\n");
  int i = 0;
  
  return 0;

}

If I compile it with clang with the following command:

$ clang -std=c99 -Wdeclaration-after-statement prog.c

it produces no warnings.

If I compile the same code with gcc with the following command:

$ gcc -std=c99 -Wdeclaration-after-statement prog.c

it produces the following warning:

prog.c: In function ‘main’:
prog.c:6:9: warning: ISO C90 forbids mixed declarations and code 
[-Wdeclaration-after-statement]

  6 | int i = 0;
| ^~~

This is the behavior I would like to have with clang, but it only produces this 
warning if I use it with -std=c90 or -std=c89 or -ansi, like this:

$ clang -std=c90 -Wdeclaration-after-statement prog.c
prog.c:6:6: warning: ISO C90 forbids mixing declarations and code 
[-Wdeclaration-after-statement]

  int i = 0;
  ^

1 warning generated.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115094

Files:
  clang/lib/Sema/SemaStmt.cpp


Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -410,9 +410,9 @@
ArrayRef Elts, bool isStmtExpr) {
   const unsigned NumElts = Elts.size();
 
-  // If we're in C89 mode, check that we don't have any decls after stmts.  If
+  // Check that we don't have any decls after stmts.  If
   // so, emit an extension diagnostic.
-  if (!getLangOpts().C99 && !getLangOpts().CPlusPlus) {
+  if (!getLangOpts().CPlusPlus) {
 // Note that __extension__ can be around a decl.
 unsigned i = 0;
 // Skip over all declarations.


Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -410,9 +410,9 @@
ArrayRef Elts, bool isStmtExpr) {
   const unsigned NumElts = Elts.size();
 
-  // If we're in C89 mode, check that we don't have any decls after stmts.  If
+  // Check that we don't have any decls after stmts.  If
   // so, emit an extension diagnostic.
-  if (!getLangOpts().C99 && !getLangOpts().CPlusPlus) {
+  if (!getLangOpts().CPlusPlus) {
 // Note that __extension__ can be around a decl.
 unsigned i = 0;
 // Skip over all declarations.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D113422: [clang-tidy][NFC] Move CachedGlobList to GlobList.h

2021-12-04 Thread Carlos Galvez via Phabricator via cfe-commits
carlosgalvezp added a comment.

Thanks a lot for the review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113422

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


[PATCH] D113422: [clang-tidy][NFC] Move CachedGlobList to GlobList.h

2021-12-04 Thread Carlos Galvez via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG946eb7a037d5: [clang-tidy][NFC] Move CachedGlobList to 
GlobList.h (authored by carlosgalvezp).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113422

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tools-extra/clang-tidy/GlobList.cpp
  clang-tools-extra/clang-tidy/GlobList.h
  clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp
@@ -4,15 +4,20 @@
 namespace clang {
 namespace tidy {
 
-TEST(GlobList, Empty) {
-  GlobList Filter("");
+template  struct GlobListTest : public ::testing::Test {};
+
+using GlobListTypes = ::testing::Types;
+TYPED_TEST_SUITE(GlobListTest, GlobListTypes);
+
+TYPED_TEST(GlobListTest, Empty) {
+  TypeParam Filter("");
 
   EXPECT_TRUE(Filter.contains(""));
   EXPECT_FALSE(Filter.contains("aaa"));
 }
 
-TEST(GlobList, Nothing) {
-  GlobList Filter("-*");
+TYPED_TEST(GlobListTest, Nothing) {
+  TypeParam Filter("-*");
 
   EXPECT_FALSE(Filter.contains(""));
   EXPECT_FALSE(Filter.contains("a"));
@@ -21,8 +26,8 @@
   EXPECT_FALSE(Filter.contains("*"));
 }
 
-TEST(GlobList, Everything) {
-  GlobList Filter("*");
+TYPED_TEST(GlobListTest, Everything) {
+  TypeParam Filter("*");
 
   EXPECT_TRUE(Filter.contains(""));
   EXPECT_TRUE(Filter.contains(""));
@@ -31,8 +36,8 @@
   EXPECT_TRUE(Filter.contains("*"));
 }
 
-TEST(GlobList, OneSimplePattern) {
-  GlobList Filter("aaa");
+TYPED_TEST(GlobListTest, OneSimplePattern) {
+  TypeParam Filter("aaa");
 
   EXPECT_TRUE(Filter.contains("aaa"));
   EXPECT_FALSE(Filter.contains(""));
@@ -41,8 +46,8 @@
   EXPECT_FALSE(Filter.contains("bbb"));
 }
 
-TEST(GlobList, TwoSimplePatterns) {
-  GlobList Filter("aaa,bbb");
+TYPED_TEST(GlobListTest, TwoSimplePatterns) {
+  TypeParam Filter("aaa,bbb");
 
   EXPECT_TRUE(Filter.contains("aaa"));
   EXPECT_TRUE(Filter.contains("bbb"));
@@ -52,11 +57,11 @@
   EXPECT_FALSE(Filter.contains(""));
 }
 
-TEST(GlobList, PatternPriority) {
+TYPED_TEST(GlobListTest, PatternPriority) {
   // The last glob that matches the string decides whether that string is
   // included or excluded.
   {
-GlobList Filter("a*,-aaa");
+TypeParam Filter("a*,-aaa");
 
 EXPECT_FALSE(Filter.contains(""));
 EXPECT_TRUE(Filter.contains("a"));
@@ -65,7 +70,7 @@
 EXPECT_TRUE(Filter.contains(""));
   }
   {
-GlobList Filter("-aaa,a*");
+TypeParam Filter("-aaa,a*");
 
 EXPECT_FALSE(Filter.contains(""));
 EXPECT_TRUE(Filter.contains("a"));
@@ -75,15 +80,16 @@
   }
 }
 
-TEST(GlobList, WhitespacesAtBegin) {
-  GlobList Filter("-*,   a.b.*");
+TYPED_TEST(GlobListTest, WhitespacesAtBegin) {
+  TypeParam Filter("-*,   a.b.*");
 
   EXPECT_TRUE(Filter.contains("a.b.c"));
   EXPECT_FALSE(Filter.contains("b.c"));
 }
 
-TEST(GlobList, Complex) {
-  GlobList Filter("*,-a.*, -b.*, \r  \n  a.1.* ,-a.1.A.*,-..,-...,-..+,-*$, -*qwe* ");
+TYPED_TEST(GlobListTest, Complex) {
+  TypeParam Filter(
+  "*,-a.*, -b.*, \r  \n  a.1.* ,-a.1.A.*,-..,-...,-..+,-*$, -*qwe* ");
 
   EXPECT_TRUE(Filter.contains("aaa"));
   EXPECT_TRUE(Filter.contains("qqq"));
Index: clang-tools-extra/clang-tidy/GlobList.h
===
--- clang-tools-extra/clang-tidy/GlobList.h
+++ clang-tools-extra/clang-tidy/GlobList.h
@@ -11,6 +11,7 @@
 
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Regex.h"
 
@@ -24,6 +25,8 @@
 /// them in the order of appearance in the list.
 class GlobList {
 public:
+  virtual ~GlobList() = default;
+
   /// \p Globs is a comma-separated list of globs (only the '*' metacharacter is
   /// supported) with an optional '-' prefix to denote exclusion.
   ///
@@ -36,10 +39,9 @@
 
   /// Returns \c true if the pattern matches \p S. The result is the last
   /// matching glob's Positive flag.
-  bool contains(StringRef S) const;
+  virtual bool contains(StringRef S) const;
 
 private:
-
   struct GlobListItem {
 bool IsPositive;
 llvm::Regex Regex;
@@ -47,7 +49,21 @@
   SmallVector Items;
 };
 
-} // end namespace tidy
-} // end namespace clang
+/// A \p GlobList that caches search results, so that search is performed only
+/// once for the same query.
+class CachedGlobList final : public GlobList {
+public:
+  using GlobList::GlobList;
+
+  /// \see GlobList::contains
+  bool contains(StringRef S) const override;
+
+private:
+  enum Tristate { None, Yes, No };
+  mutable llvm::StringMap Cache;
+};
+
+} // namespace tidy
+} // namespace 

[clang-tools-extra] 946eb7a - [clang-tidy][NFC] Move CachedGlobList to GlobList.h

2021-12-04 Thread Carlos Galvez via cfe-commits

Author: Carlos Galvez
Date: 2021-12-04T08:50:49Z
New Revision: 946eb7a037d5f83ea9cdc99bac0f939ddd344e09

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

LOG: [clang-tidy][NFC] Move CachedGlobList to GlobList.h

Currently it's hidden inside ClangTidyDiagnosticConsumer,
so it's hard to know it exists.

Given that there are multiple uses of globs in clang-tidy,
it makes sense to have these classes publicly available
for other use cases that might benefit from it.

Also, add unit test by converting the existing tests
for GlobList into typed tests.

Reviewed By: salman-javed-nz

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
clang-tools-extra/clang-tidy/GlobList.cpp
clang-tools-extra/clang-tidy/GlobList.h
clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp 
b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 66329328757bd..b09079b5e8ba4 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -155,29 +155,6 @@ ClangTidyError::ClangTidyError(StringRef CheckName,
 : tooling::Diagnostic(CheckName, DiagLevel, BuildDirectory),
   IsWarningAsError(IsWarningAsError) {}
 
-class ClangTidyContext::CachedGlobList {
-public:
-  CachedGlobList(StringRef Globs) : Globs(Globs) {}
-
-  bool contains(StringRef S) {
-switch (auto &Result = Cache[S]) {
-case Yes:
-  return true;
-case No:
-  return false;
-case None:
-  Result = Globs.contains(S) ? Yes : No;
-  return Result == Yes;
-}
-llvm_unreachable("invalid enum");
-  }
-
-private:
-  GlobList Globs;
-  enum Tristate { None, Yes, No };
-  llvm::StringMap Cache;
-};
-
 ClangTidyContext::ClangTidyContext(
 std::unique_ptr OptionsProvider,
 bool AllowEnablingAnalyzerAlphaCheckers)

diff  --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h 
b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
index 129c06f60f569..23800f5622f69 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -29,6 +29,7 @@ class CompilationDatabase;
 } // namespace tooling
 
 namespace tidy {
+class CachedGlobList;
 
 /// A detected error complete with information to display diagnostic and
 /// automatic fix.
@@ -191,7 +192,7 @@ class ClangTidyContext {
 
   std::string CurrentFile;
   ClangTidyOptions CurrentOptions;
-  class CachedGlobList;
+
   std::unique_ptr CheckFilter;
   std::unique_ptr WarningAsErrorFilter;
 

diff  --git a/clang-tools-extra/clang-tidy/GlobList.cpp 
b/clang-tools-extra/clang-tidy/GlobList.cpp
index ccdf856083759..b594d943cc075 100644
--- a/clang-tools-extra/clang-tidy/GlobList.cpp
+++ b/clang-tools-extra/clang-tidy/GlobList.cpp
@@ -9,8 +9,8 @@
 #include "GlobList.h"
 #include "llvm/ADT/SmallString.h"
 
-using namespace clang;
-using namespace tidy;
+namespace clang {
+namespace tidy {
 
 // Returns true if GlobList starts with the negative indicator ('-'), removes 
it
 // from the GlobList.
@@ -62,3 +62,19 @@ bool GlobList::contains(StringRef S) const {
   }
   return false;
 }
+
+bool CachedGlobList::contains(StringRef S) const {
+  switch (auto &Result = Cache[S]) {
+  case Yes:
+return true;
+  case No:
+return false;
+  case None:
+Result = GlobList::contains(S) ? Yes : No;
+return Result == Yes;
+  }
+  llvm_unreachable("invalid enum");
+}
+
+} // namespace tidy
+} // namespace clang

diff  --git a/clang-tools-extra/clang-tidy/GlobList.h 
b/clang-tools-extra/clang-tidy/GlobList.h
index 17730078b6f31..de7020ef3f165 100644
--- a/clang-tools-extra/clang-tidy/GlobList.h
+++ b/clang-tools-extra/clang-tidy/GlobList.h
@@ -11,6 +11,7 @@
 
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Regex.h"
 
@@ -24,6 +25,8 @@ namespace tidy {
 /// them in the order of appearance in the list.
 class GlobList {
 public:
+  virtual ~GlobList() = default;
+
   /// \p Globs is a comma-separated list of globs (only the '*' metacharacter 
is
   /// supported) with an optional '-' prefix to denote exclusion.
   ///
@@ -36,10 +39,9 @@ class GlobList {
 
   /// Returns \c true if the pattern matches \p S. The result is the last
   /// matching glob's Positive flag.
-  bool contains(StringRef S) const;
+  virtual bool contains(StringRef S) const;
 
 private:
-
   struct GlobListItem {
 bool IsPositive;
 llvm::Regex Regex;
@@ -4