[PATCH] D114527: [VE] Support multiple architectures installation

[Kazushi Marukawa via Phabricator via cfe-commits]

kaz7 updated this revision to Diff 391223.
kaz7 added a comment.

Rebase to the latest main.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114527

Files:
  clang/lib/Driver/ToolChains/VEToolchain.cpp
  clang/test/Driver/Inputs/basic_ve_tree/bin/.keep
  clang/test/Driver/Inputs/basic_ve_tree/include/c++/v1/.keep
  
clang/test/Driver/Inputs/basic_ve_tree/include/ve-unknown-linux-gnu/c++/v1/.keep
  clang/test/Driver/ve-toolchain.c
  clang/test/Driver/ve-toolchain.cpp

Index: clang/test/Driver/ve-toolchain.cpp
===
--- clang/test/Driver/ve-toolchain.cpp
+++ clang/test/Driver/ve-toolchain.cpp
@@ -4,72 +4,89 @@
 ///-
 /// Checking dwarf-version
 
-// RUN: %clangxx -### -g -target ve %s 2>&1 | FileCheck -check-prefix=DWARF_VER %s
+// RUN: %clangxx -### -g -target ve-unknown-linux-gnu \
+// RUN: %s 2>&1 | FileCheck -check-prefix=DWARF_VER %s
 // DWARF_VER: "-dwarf-version=4"
 
 ///-
 /// Checking include-path
 
-// RUN: %clangxx -### -target ve --sysroot %S/Inputs/basic_ve_tree %s \
-// RUN: -resource-dir=%S/Input/basic_ve_tree/resource_dir \
+// RUN: %clangxx -### -target ve-unknown-linux-gnu \
+// RUN: --sysroot %S/Inputs/basic_ve_tree %s \
+// RUN: -ccc-install-dir %S/Inputs/basic_ve_tree/bin \
+// RUN: -resource-dir=%S/Inputs/basic_ve_tree/resource_dir \
 // RUN: 2>&1 | FileCheck -check-prefix=DEFINC %s
 // DEFINC: clang{{.*}} "-cc1"
 // DEFINC-SAME: "-nostdsysteminc"
 // DEFINC-SAME: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // DEFINC-SAME: "-isysroot" "[[SYSROOT:[^"]+]]"
-// DEFINC-SAME: "-internal-isystem" "[[RESOURCE_DIR]]/include/c++/v1"
+// DEFINC-SAME: "-internal-isystem" "{{.*}}/bin/../include/ve-unknown-linux-gnu/c++/v1"
+// DEFINC-SAME: "-internal-isystem" "{{.*}}/bin/../include/c++/v1"
 // DEFINC-SAME: "-internal-isystem" "[[RESOURCE_DIR]]/include"
 // DEFINC-SAME: "-internal-isystem" "[[SYSROOT]]/opt/nec/ve/include"
 
-// RUN: %clangxx -### -target ve --sysroot %S/Inputs/basic_ve_tree %s \
-// RUN: -resource-dir=%S/Input/basic_ve_tree/resource_dir \
+// RUN: %clangxx -### -target ve-unknown-linux-gnu \
+// RUN: --sysroot %S/Inputs/basic_ve_tree %s \
+// RUN: -ccc-install-dir %S/Inputs/basic_ve_tree/bin \
+// RUN: -resource-dir=%S/Inputs/basic_ve_tree/resource_dir \
 // RUN: -nostdlibinc 2>&1 | FileCheck -check-prefix=NOSTDLIBINC %s
 // NOSTDLIBINC: clang{{.*}} "-cc1"
 // NOSTDLIBINC-SAME: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // NOSTDLIBINC-SAME: "-isysroot" "[[SYSROOT:[^"]+]]"
-// NOSTDLIBINC-NOT: "-internal-isystem" "[[RESOURCE_DIR]]/include/c++/v1"
+// NOSTDLIBINC-NOT: "-internal-isystem" "{{.*}}/bin/../include/ve-unknown-linux-gnu/c++/v1"
+// NOSTDLIBINC-NOT: "-internal-isystem" "{{.*}}/bin/../include/c++/v1"
 // NOSTDLIBINC-SAME: "-internal-isystem" "[[RESOURCE_DIR]]/include"
 // NOSTDLIBINC-NOT: "-internal-isystem" "[[SYSROOT]]/opt/nec/ve/include"
 
-// RUN: %clangxx -### -target ve --sysroot %S/Inputs/basic_ve_tree %s \
-// RUN: -resource-dir=%S/Input/basic_ve_tree/resource_dir \
+// RUN: %clangxx -### -target ve-unknown-linux-gnu \
+// RUN: --sysroot %S/Inputs/basic_ve_tree %s \
+// RUN: -ccc-install-dir %S/Inputs/basic_ve_tree/bin \
+// RUN: -resource-dir=%S/Inputs/basic_ve_tree/resource_dir \
 // RUN: -nobuiltininc 2>&1 | FileCheck -check-prefix=NOBUILTININC %s
 // NOBUILTININC: clang{{.*}} "-cc1"
 // NOBUILTININC-SAME: "-nobuiltininc"
 // NOBUILTININC-SAME: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // NOBUILTININC-SAME: "-isysroot" "[[SYSROOT:[^"]+]]"
-// NOBUILTININC-SAME: "-internal-isystem" "[[RESOURCE_DIR]]/include/c++/v1"
+// NOBUILTININC-SAME: "-internal-isystem" "{{.*}}/bin/../include/ve-unknown-linux-gnu/c++/v1"
+// NOBUILTININC-SAME: "-internal-isystem" "{{.*}}/bin/../include/c++/v1"
 // NOBUILTININC-NOT: "-internal-isystem" "[[RESOURCE_DIR]]/include"
 // NOBUILTININC-SAME: "-internal-isystem" "[[SYSROOT]]/opt/nec/ve/include"
 
-// RUN: %clangxx -### -target ve --sysroot %S/Inputs/basic_ve_tree %s \
-// RUN: -resource-dir=%S/Input/basic_ve_tree/resource_dir \
+// RUN: %clangxx -### -target ve-unknown-linux-gnu \
+// RUN: --sysroot %S/Inputs/basic_ve_tree %s \
+// RUN: -ccc-install-dir %S/Inputs/basic_ve_tree/bin \
+// RUN: -resource-dir=%S/Inputs/basic_ve_tree/resource_dir \
 // RUN: -nostdinc 2>&1 | FileCheck -check-prefix=NOSTDINC %s
 // NOSTDINC: clang{{.*}} "-cc1"
 // NOSTDINC-SAME: "-nobuiltininc"
 // NOSTDINC-SAME: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // NOSTDINC-SAME: "-isysroot" "[[SYSROOT:[^"]+]]"
-// NOSTDINC-NOT: "-internal-isystem" "[[RESOURCE_DIR]]/include/c++/v1"
+// NOSTDINC-NOT: "-internal-isystem" "{{.*}}/bin/../include/ve-unknown-linux-gnu/c++/v1"
+// NOSTDINC-NOT: "-inter

[PATCH] D114890: [OpenMP] Make the new device runtime the default

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

This will definitely break amdgpu bot without D114865 
 landed first.

However that patch is currently blocked by Matt, so we may want to land this 
and disable the amdgpu buildbot until the backend is fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114890

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

[PATCH] D114849: [AMDGPU][clang] Fix __builtin_nontemporal_store() failure on AMDGPU

[krishna chaitanya sankisa 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 rG16b781e6d16d: [AMDGPU][clang] Fix  
__builtin_nontemporal_store() failure on AMDGPU (authored by skc7).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114849

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGenOpenCL/amdgcn-non-temporal-store.cl


Index: clang/test/CodeGenOpenCL/amdgcn-non-temporal-store.cl
===
--- /dev/null
+++ clang/test/CodeGenOpenCL/amdgcn-non-temporal-store.cl
@@ -0,0 +1,8 @@
+// REQUIRES: amdgpu-registered-target
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -S -emit-llvm -o - %s | 
FileCheck %s
+// CHECK-LABEL: @test_non_temporal_store_kernel
+// CHECK: store i32 0, i32 addrspace(1)* %{{.*}}, align 4, !tbaa !{{.*}}, 
!nontemporal {{.*}}
+
+kernel void test_non_temporal_store_kernel(global unsigned int* io) {
+  __builtin_nontemporal_store(0, io);
+}
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -170,8 +170,9 @@
 
   // Convert the type of the pointer to a pointer to the stored type.
   Val = CGF.EmitToMemory(Val, E->getArg(0)->getType());
+  unsigned SrcAddrSpace = Address->getType()->getPointerAddressSpace();
   Value *BC = CGF.Builder.CreateBitCast(
-  Address, llvm::PointerType::getUnqual(Val->getType()), "cast");
+  Address, llvm::PointerType::get(Val->getType(), SrcAddrSpace), "cast");
   LValue LV = CGF.MakeNaturalAlignAddrLValue(BC, E->getArg(0)->getType());
   LV.setNontemporal(true);
   CGF.EmitStoreOfScalar(Val, LV, false);


Index: clang/test/CodeGenOpenCL/amdgcn-non-temporal-store.cl
===
--- /dev/null
+++ clang/test/CodeGenOpenCL/amdgcn-non-temporal-store.cl
@@ -0,0 +1,8 @@
+// REQUIRES: amdgpu-registered-target
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -S -emit-llvm -o - %s | FileCheck %s
+// CHECK-LABEL: @test_non_temporal_store_kernel
+// CHECK: store i32 0, i32 addrspace(1)* %{{.*}}, align 4, !tbaa !{{.*}}, !nontemporal {{.*}}
+
+kernel void test_non_temporal_store_kernel(global unsigned int* io) {
+  __builtin_nontemporal_store(0, io);
+}
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -170,8 +170,9 @@
 
   // Convert the type of the pointer to a pointer to the stored type.
   Val = CGF.EmitToMemory(Val, E->getArg(0)->getType());
+  unsigned SrcAddrSpace = Address->getType()->getPointerAddressSpace();
   Value *BC = CGF.Builder.CreateBitCast(
-  Address, llvm::PointerType::getUnqual(Val->getType()), "cast");
+  Address, llvm::PointerType::get(Val->getType(), SrcAddrSpace), "cast");
   LValue LV = CGF.MakeNaturalAlignAddrLValue(BC, E->getArg(0)->getType());
   LV.setNontemporal(true);
   CGF.EmitStoreOfScalar(Val, LV, false);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 16b781e - [AMDGPU][clang] Fix __builtin_nontemporal_store() failure on AMDGPU

[via cfe-commits]

Author: skc7
Date: 2021-12-02T05:53:25Z
New Revision: 16b781e6d16dead414a7036c8b59f1700ea49251

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

LOG: [AMDGPU][clang] Fix  __builtin_nontemporal_store() failure on AMDGPU

Reviewed By: yaxunl, sameerds

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

Added: 
clang/test/CodeGenOpenCL/amdgcn-non-temporal-store.cl

Modified: 
clang/lib/CodeGen/CGBuiltin.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 849423c8b9bae..5d6df59cc4059 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -170,8 +170,9 @@ static Value *EmitNontemporalStore(CodeGenFunction &CGF, 
const CallExpr *E) {
 
   // Convert the type of the pointer to a pointer to the stored type.
   Val = CGF.EmitToMemory(Val, E->getArg(0)->getType());
+  unsigned SrcAddrSpace = Address->getType()->getPointerAddressSpace();
   Value *BC = CGF.Builder.CreateBitCast(
-  Address, llvm::PointerType::getUnqual(Val->getType()), "cast");
+  Address, llvm::PointerType::get(Val->getType(), SrcAddrSpace), "cast");
   LValue LV = CGF.MakeNaturalAlignAddrLValue(BC, E->getArg(0)->getType());
   LV.setNontemporal(true);
   CGF.EmitStoreOfScalar(Val, LV, false);

diff  --git a/clang/test/CodeGenOpenCL/amdgcn-non-temporal-store.cl 
b/clang/test/CodeGenOpenCL/amdgcn-non-temporal-store.cl
new file mode 100644
index 0..539d857080e27
--- /dev/null
+++ b/clang/test/CodeGenOpenCL/amdgcn-non-temporal-store.cl
@@ -0,0 +1,8 @@
+// REQUIRES: amdgpu-registered-target
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -S -emit-llvm -o - %s | 
FileCheck %s
+// CHECK-LABEL: @test_non_temporal_store_kernel
+// CHECK: store i32 0, i32 addrspace(1)* %{{.*}}, align 4, !tbaa !{{.*}}, 
!nontemporal {{.*}}
+
+kernel void test_non_temporal_store_kernel(global unsigned int* io) {
+  __builtin_nontemporal_store(0, io);
+}



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

[PATCH] D114842: [lld-macho] Remove old macho darwin lld

[Vy Nguyen via Phabricator via cfe-commits]

oontvoo accepted this revision.
oontvoo added a comment.

LG - thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114842

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

[PATCH] D114833: [modules] Fix ambiguous name lookup for enum constants from hidden submodules.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a subscriber: Bigcheese.
vsapsai added a comment.

Adding Michael who is infinitely better than me in C++.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114833

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

[PATCH] D114833: [modules] Fix ambiguous name lookup for enum constants from hidden submodules.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

As I was trying to replicate C++ behavior I've created a test case 
ambiguous-anonymous-enum-lookup.cpp. And it results in diagnostics

  
clang/test/Modules/Output/ambiguous-anonymous-enum-lookup.cpp.tmp/test.cpp:6:10:
 warning: ambiguous use of internal linkage declaration 'kAnonymousEnumValue' 
defined in multiple modules [-Wmodules-ambiguous-internal-linkage]
return kAnonymousEnumValue;
   ^
  
clang/test/Modules/Output/ambiguous-anonymous-enum-lookup.cpp.tmp/include/textual.h:5:3:
 note: declared here
kAnonymousEnumValue = 0,
^
  
clang/test/Modules/Output/ambiguous-anonymous-enum-lookup.cpp.tmp/include/textual.h:5:3:
 note: declared here in module 'Piecewise.InitiallyHidden'
kAnonymousEnumValue = 0,
^

Is this warning correct or is it wrong and we have the same bug both in ObjC 
and C++? I feel I am biased (and bad at C++ linkage rules), so need a separate 
opinion on correctness.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114833

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

[PATCH] D114890: [OpenMP] Make the new device runtime the default

[Ron Lieberman via Phabricator via cfe-commits]

ronlieb added a comment.

based on Shilei's last comment, do it in the morning ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114890

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

[PATCH] D114890: [OpenMP] Make the new device runtime the default

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

In D114890#3165883 , @jhuber6 wrote:

> In D114890#3165879 , @ronlieb wrote:
>
>> works for me, i think Greg is ok with it too, we chatted internally an hour 
>> or so ago
>
> Should I just land it now and sleep or wait until tomorrow? Whichever causes 
> the least downtime for you.

like Jon said, you also have to disable the test for the "new" one because the 
default one now is the new one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114890

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

[PATCH] D114890: [OpenMP] Make the new device runtime the default

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D114890#3165879 , @ronlieb wrote:

> works for me, i think Greg is ok with it too, we chatted internally an hour 
> or so ago

Should I just land it now and sleep or wait until tomorrow? Whichever causes 
the least downtime for you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114890

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

[PATCH] D114833: [modules] Fix ambiguous name lookup for enum constants from hidden submodules.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai updated this revision to Diff 391189.
vsapsai added a comment.

Attempt to restore a previous commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114833

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Modules/ambiguous-anonymous-enum-lookup.cpp
  clang/test/Modules/redefinition-c-tagtypes.m


Index: clang/test/Modules/redefinition-c-tagtypes.m
===
--- clang/test/Modules/redefinition-c-tagtypes.m
+++ clang/test/Modules/redefinition-c-tagtypes.m
@@ -32,6 +32,10 @@
   TRD = 55
 };
 
+int testReferencingNSEConstant() {
+  return FST;
+}
+
 #define NS_ENUM(_type, _name) \
   enum _name : _type _name;   \
   enum _name : _type
@@ -42,7 +46,11 @@
   MinXOther = MinX,
 #else
   MinXOther = TRD, // expected-note {{enumerator 'MinXOther' with value 55 
here}}
-  // expected-error@redefinition-c-tagtypes.m:39 {{type 'enum NSMyEnum' has 
incompatible definitions}}
+  // expected-error@redefinition-c-tagtypes.m:43 {{type 'enum NSMyEnum' has 
incompatible definitions}}
   // expected-note@Inputs/F.framework/PrivateHeaders/NS.h:18 {{enumerator 
'MinXOther' with value 11 here}}
 #endif
 };
+
+int testReferencingNSMyEnumConstant() {
+  return MinX;
+}
Index: clang/test/Modules/ambiguous-anonymous-enum-lookup.cpp
===
--- /dev/null
+++ clang/test/Modules/ambiguous-anonymous-enum-lookup.cpp
@@ -0,0 +1,41 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps 
-fmodules-cache-path=%t/modules.cache -I %t/include %t/test.cpp -verify
+
+//--- include/textual.h
+#ifndef TEXTUAL_H
+#define TEXTUAL_H
+
+enum {
+  kAnonymousEnumValue = 0,
+};
+
+#endif
+
+//--- include/empty.h
+// empty
+
+//--- include/initially_hidden.h
+#include 
+
+//--- include/module.modulemap
+module Piecewise {
+  module Empty {
+header "empty.h"
+export *
+  }
+  module InitiallyHidden {
+header "initially_hidden.h"
+export *
+  }
+}
+
+//--- test.cpp
+#include 
+#include 
+#include 
+
+// expected-no-diagnostics
+int testReferencingAnonymousEnumConstant() {
+  return kAnonymousEnumValue;
+}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16598,6 +16598,19 @@
 
   // Make the previous decl visible.
   makeMergedDefinitionVisible(SkipBody.Previous);
+  // For the ignored `EnumDecl` definition remove its `EnumConstantDecl` from
+  // global name lookup to avoid future ambiguous lookups. Doing this for 
ObjC/C
+  // to mimic early return in `Sema::ActOnTag` that avoids calling
+  // `PushOnScopeChains` for duplicate `TagDecl` definitions. Doing it only for
+  // `EnumDecl` among all `TagDecl` because enum constants have global name
+  // lookup while record field lookup is limited to a record.
+  if (!getLangOpts().CPlusPlus) {
+if (const auto *ED = dyn_cast(SkipBody.New))
+  for (EnumDecl::enumerator_iterator EC = ED->enumerator_begin(),
+ End = ED->enumerator_end();
+   EC != End; ++EC)
+IdResolver.RemoveDecl(*EC);
+  }
   return true;
 }
 


Index: clang/test/Modules/redefinition-c-tagtypes.m
===
--- clang/test/Modules/redefinition-c-tagtypes.m
+++ clang/test/Modules/redefinition-c-tagtypes.m
@@ -32,6 +32,10 @@
   TRD = 55
 };
 
+int testReferencingNSEConstant() {
+  return FST;
+}
+
 #define NS_ENUM(_type, _name) \
   enum _name : _type _name;   \
   enum _name : _type
@@ -42,7 +46,11 @@
   MinXOther = MinX,
 #else
   MinXOther = TRD, // expected-note {{enumerator 'MinXOther' with value 55 here}}
-  // expected-error@redefinition-c-tagtypes.m:39 {{type 'enum NSMyEnum' has incompatible definitions}}
+  // expected-error@redefinition-c-tagtypes.m:43 {{type 'enum NSMyEnum' has incompatible definitions}}
   // expected-note@Inputs/F.framework/PrivateHeaders/NS.h:18 {{enumerator 'MinXOther' with value 11 here}}
 #endif
 };
+
+int testReferencingNSMyEnumConstant() {
+  return MinX;
+}
Index: clang/test/Modules/ambiguous-anonymous-enum-lookup.cpp
===
--- /dev/null
+++ clang/test/Modules/ambiguous-anonymous-enum-lookup.cpp
@@ -0,0 +1,41 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache -I %t/include %t/test.cpp -verify
+
+//--- include/textual.h
+#ifndef TEXTUAL_H
+#define TEXTUAL_H
+
+enum {
+  kAnonymousEnumValue = 0,
+};
+
+#endif
+
+//--- include/empty.h
+// empty
+
+//--- include/initially_hidden.h
+#include 
+
+//--- include/module.modulemap
+module Piecewise {
+  module Empty {
+header "empty.h"
+export *
+  }
+  module InitiallyHidden {
+  

[PATCH] D114890: [OpenMP] Make the new device runtime the default

[Ron Lieberman via Phabricator via cfe-commits]

ronlieb accepted this revision.
ronlieb added a comment.

works for me, i think Greg is ok with it too, we chatted internally an hour or 
so ago


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114890

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

[PATCH] D114890: [OpenMP] Make the new device runtime the default

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D114890#3165799 , @ronlieb wrote:

> perhaps we can try this patch as is, and if it passes buildbot, let the new 
> DeviceRTL be the default upstream for all targets.
> if it fails the AMDGPU buildbot, then perhaps apply the above suggested 
> change of leaving old runtime default for now for AMD.
> or consider some other soultion.
>
> in other words, how about we land it, and be ready to revert if bot goes red?

I could land this and make a follow-up patch doing what @gregrodgers suggests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114890

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

[PATCH] D114833: [modules] Fix ambiguous name lookup for enum constants from hidden submodules.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai updated this revision to Diff 391187.
vsapsai added a comment.

Add a test case for referencing an anonymous enum constant in C++.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114833

Files:
  clang/test/Modules/ambiguous-anonymous-enum-lookup.cpp


Index: clang/test/Modules/ambiguous-anonymous-enum-lookup.cpp
===
--- /dev/null
+++ clang/test/Modules/ambiguous-anonymous-enum-lookup.cpp
@@ -0,0 +1,41 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps 
-fmodules-cache-path=%t/modules.cache -I %t/include %t/test.cpp -verify
+
+//--- include/textual.h
+#ifndef TEXTUAL_H
+#define TEXTUAL_H
+
+enum {
+  kAnonymousEnumValue = 0,
+};
+
+#endif
+
+//--- include/empty.h
+// empty
+
+//--- include/initially_hidden.h
+#include 
+
+//--- include/module.modulemap
+module Piecewise {
+  module Empty {
+header "empty.h"
+export *
+  }
+  module InitiallyHidden {
+header "initially_hidden.h"
+export *
+  }
+}
+
+//--- test.cpp
+#include 
+#include 
+#include 
+
+// expected-no-diagnostics
+int testReferencingAnonymousEnumConstant() {
+  return kAnonymousEnumValue;
+}


Index: clang/test/Modules/ambiguous-anonymous-enum-lookup.cpp
===
--- /dev/null
+++ clang/test/Modules/ambiguous-anonymous-enum-lookup.cpp
@@ -0,0 +1,41 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache -I %t/include %t/test.cpp -verify
+
+//--- include/textual.h
+#ifndef TEXTUAL_H
+#define TEXTUAL_H
+
+enum {
+  kAnonymousEnumValue = 0,
+};
+
+#endif
+
+//--- include/empty.h
+// empty
+
+//--- include/initially_hidden.h
+#include 
+
+//--- include/module.modulemap
+module Piecewise {
+  module Empty {
+header "empty.h"
+export *
+  }
+  module InitiallyHidden {
+header "initially_hidden.h"
+export *
+  }
+}
+
+//--- test.cpp
+#include 
+#include 
+#include 
+
+// expected-no-diagnostics
+int testReferencingAnonymousEnumConstant() {
+  return kAnonymousEnumValue;
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112626: Convert float to double on __builtin_dump_struct

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2090-2094
+// Variadic functions expect the caller to promote float to double.
+if (CanonicalType == Context.FloatTy) {
+  FieldPtr =
+  CGF.Builder.CreateFPExt(FieldPtr, CGF.ConvertType(Context.DoubleTy));
+}

aaron.ballman wrote:
> This change is an improvement as far as it goes, but I think we might be 
> missing other floating-point promotions here. For example, `__fp16` fields 
> also seem to be unusable: https://godbolt.org/z/z3a45f9YE
> 
> Also, we don't seem to handle the integer promotions at all (but still get 
> correct results there), so I think we're getting the correct behavior there 
> by chance rather than by design. Oh, yeah, note the differences here: 
> https://godbolt.org/z/f13eq3668
> 
> ```
> foo:
>   ...
>   %7 = load i8, i8* %4, align 1, !dbg !217
>   %8 = call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([6 x i8], [6 x 
> i8]* @2, i32 0, i32 0), i8 %7), !dbg !217
>   ...
> 
> bar:
>   ...
>   %2 = load i8, i8* %1, align 1, !dbg !222
>   %3 = zext i8 %2 to i32, !dbg !222
>   %4 = call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([5 x i8], [5 x 
> i8]* @.str, i64 0, i64 0), i32 %3), !dbg !223
>   ...
> ```
> I think we should probably fix all of the promotion problems at once rather 
> than piecemeal.
> 
It's actually really annoying that this logic has to be duplicated in IRGen 
instead of being able to take advantage of the existing promotion logic in 
Sema.  Can we just generate a helper function in Sema and somehow link it to 
the builtin call?

Um.  Also, the `static` local DenseMap in the code above this is totally 
unacceptable and should not have been committed.  Clang is supposed to be 
embeddable as a library and should not be using global mutable variables.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112626

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

[PATCH] D111566: [SYCL] Fix function pointer address space

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CodeGenTypes.cpp:636-638
+unsigned AS = PointeeType->isFunctionTy()
+  ? getDataLayout().getProgramAddressSpace()
+  : Context.getTargetAddressSpace(ETy);

aaron.ballman wrote:
> eandrews wrote:
> > aaron.ballman wrote:
> > > The review summary says that this is a fix for SYCL, but the fix itself 
> > > happens for all targets, not just SYCL. If that's intentional, are we 
> > > sure it's correct?
> > Yes this affects all targets. To be honest, I'm not sure if this change is 
> > correct for CUDA/openCL, etc. My first patch (which I didn't upload) 
> > restricted the change to SYCL. However, I saw the same thing was done in a 
> > generic manner for function pointers  - 
> > https://github.com/llvm/llvm-project/commit/57fd86de879cf2b4c7001b6d0a09df60877ce24d,
> >  and so followed the same logic. I'm hoping reviewers more familiar with 
> > address spaces can help here. 
> @Anastasia -- can you comment as OpenCL code owner?
I think the more systematic fix is probably for `getTargetAddressSpace` to 
check for function types and return the program address space, yeah.


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

https://reviews.llvm.org/D111566

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

[PATCH] D114890: [OpenMP] Make the new device runtime the default

[Ron Lieberman via Phabricator via cfe-commits]

ronlieb added a comment.

perhaps we can try this patch as is, and if it passes buildbot, let the new 
DeviceRTL be the default upstream for all targets.
if it fails the AMDGPU buildbot, then perhaps apply the above suggested change 
of leaving old runtime default for now for AMD.
or consider some other soultion.

in other words, how about we land it, and be ready to revert if bot goes red?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114890

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

LLVM build master will be restarted soon

[Galina Kistanova via cfe-commits]

 Hello,

LLVM build master will be restarted in the nearest hour.

Thanks

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

[PATCH] D114890: [OpenMP] Make the new device runtime the default

[Greg Rodgers via Phabricator via cfe-commits]

gregrodgers requested changes to this revision.
gregrodgers added a comment.
This revision now requires changes to proceed.

I forgot to add the "request changes" action.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114890

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

[PATCH] D114890: [OpenMP] Make the new device runtime the default

[Greg Rodgers via Phabricator via cfe-commits]

gregrodgers added a comment.

We want amdgcn to remain on old deviceRTL till we have verified it .   I made 
inline comments on how this could be done.




Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5905
   // runtime.
   if (Args.hasFlag(options::OPT_fopenmp_target_new_runtime,
options::OPT_fno_openmp_target_new_runtime,

Add a check for amdgcn something like this. 



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5907
options::OPT_fno_openmp_target_new_runtime,
-   /*Default=*/false))
+   /*Default=*/true))
 CmdArgs.push_back("-fopenmp-target-new-runtime");

This will keep AMDGCN on old runtime till we are ready to switch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114890

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

[PATCH] D114859: [clang-format] Add better support for co-routinues

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu accepted this revision.
ChuanqiXu added a comment.

LGTM. Thanks!


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

https://reviews.llvm.org/D114859

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

[PATCH] D114902: [Attrs] Elaborate on the trivial_abi documentation

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:3217
+are balanced, and that this attribute can safely be applied to
+reference-counting smart pointers.
+

I would change the first sentence here to:

> Arguments of ``trivial_abi`` type are destroyed in the callee, not the caller.

The rest of this paragraph seems to be restating the first sentence in ways 
that I would expect users to find more confusing than helpful.

You might add something like: "On some platforms, arguments are normally not 
destroyed until the end of the full expression containing the call; 
``trivial_abi`` arguments will therefore be destroyed earlier than usual, 
immediately before the called function returns."



Comment at: clang/include/clang/Basic/AttrDocs.td:3267
 and is passed as an argument by value, the convention is that the callee will
 destroy the object before returning.
 

This paragraph is redundant with the new paragraph above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114902

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

[PATCH] D114619: [Analyzer][solver] Do not remove the simplified symbol from the eq class

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Thanks for the fix!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114619

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

[PATCH] D113429: [clang-tidy] Use `hasCanonicalType()` matcher in `bugprone-unused-raii` check

[Fabian Wolff via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG987a21522f2c: [clang-tidy] Use `hasCanonicalType()` matcher 
in `bugprone-unused-raii` check (authored by fwolff).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113429

Files:
  clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp
@@ -82,6 +82,28 @@
   (void)i;
 }
 
+template 
+void aliastest() {
+  using X = Foo;
+  using Y = X;
+  using Z = Y;
+  Z(42);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately 
after creation; did you mean to name the object?
+  // CHECK-FIXES: Z give_me_a_name(42);
+
+  typedef Z ZT;
+  ZT(42, 13);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately 
after creation; did you mean to name the object?
+  // CHECK-FIXES: ZT give_me_a_name(42, 13);
+
+  using TT = TCtorDefaultArg;
+  TT(42);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately 
after creation; did you mean to name the object?
+  // CHECK-FIXES: TT give_me_a_name(42);
+
+  (void)0;
+}
+
 void test() {
   Foo(42);
 // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after 
creation; did you mean to name the object?
Index: clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
@@ -29,10 +29,11 @@
   Finder->addMatcher(
   mapAnyOf(cxxConstructExpr, cxxUnresolvedConstructExpr)
   .with(hasParent(compoundStmt().bind("compound")),
-anyOf(hasType(cxxRecordDecl(hasNonTrivialDestructor())),
-  hasType(templateSpecializationType(
+anyOf(hasType(hasCanonicalType(recordType(hasDeclaration(
+  cxxRecordDecl(hasNonTrivialDestructor()),
+  hasType(hasCanonicalType(templateSpecializationType(
   hasDeclaration(classTemplateDecl(has(
-  cxxRecordDecl(hasNonTrivialDestructor()
+  cxxRecordDecl(hasNonTrivialDestructor())
   .bind("expr"),
   this);
 }


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp
@@ -82,6 +82,28 @@
   (void)i;
 }
 
+template 
+void aliastest() {
+  using X = Foo;
+  using Y = X;
+  using Z = Y;
+  Z(42);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
+  // CHECK-FIXES: Z give_me_a_name(42);
+
+  typedef Z ZT;
+  ZT(42, 13);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
+  // CHECK-FIXES: ZT give_me_a_name(42, 13);
+
+  using TT = TCtorDefaultArg;
+  TT(42);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
+  // CHECK-FIXES: TT give_me_a_name(42);
+
+  (void)0;
+}
+
 void test() {
   Foo(42);
 // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
Index: clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
@@ -29,10 +29,11 @@
   Finder->addMatcher(
   mapAnyOf(cxxConstructExpr, cxxUnresolvedConstructExpr)
   .with(hasParent(compoundStmt().bind("compound")),
-anyOf(hasType(cxxRecordDecl(hasNonTrivialDestructor())),
-  hasType(templateSpecializationType(
+anyOf(hasType(hasCanonicalType(recordType(hasDeclaration(
+  cxxRecordDecl(hasNonTrivialDestructor()),
+  hasType(hasCanonicalType(templateSpecializationType(
   hasDeclaration(classTemplateDecl(has(
-  cxxRecordDecl(hasNonTrivialDestructor()
+  cxxRecordDecl(hasNonTrivialDestructor())
   .bind("expr"),
   this);
 }
___
cfe-commits mailing list
cfe-comm

[clang-tools-extra] 987a215 - [clang-tidy] Use `hasCanonicalType()` matcher in `bugprone-unused-raii` check

[Fabian Wolff via cfe-commits]

Author: Fabian Wolff
Date: 2021-12-02T01:53:12+01:00
New Revision: 987a21522f2c7d799d0c2a720b3315a4fb6d1e74

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

LOG: [clang-tidy] Use `hasCanonicalType()` matcher in `bugprone-unused-raii` 
check

Fixes PR#52217.

Reviewed By: simon.giesecke

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
index 9b8d8d7bf5f4c..435c47348c7c0 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
@@ -29,10 +29,11 @@ void UnusedRaiiCheck::registerMatchers(MatchFinder *Finder) 
{
   Finder->addMatcher(
   mapAnyOf(cxxConstructExpr, cxxUnresolvedConstructExpr)
   .with(hasParent(compoundStmt().bind("compound")),
-anyOf(hasType(cxxRecordDecl(hasNonTrivialDestructor())),
-  hasType(templateSpecializationType(
+anyOf(hasType(hasCanonicalType(recordType(hasDeclaration(
+  cxxRecordDecl(hasNonTrivialDestructor()),
+  hasType(hasCanonicalType(templateSpecializationType(
   hasDeclaration(classTemplateDecl(has(
-  cxxRecordDecl(hasNonTrivialDestructor()
+  cxxRecordDecl(hasNonTrivialDestructor())
   .bind("expr"),
   this);
 }

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp
index 94643676f45ce..1b285768fdb33 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp
@@ -82,6 +82,28 @@ void templatetest() {
   (void)i;
 }
 
+template 
+void aliastest() {
+  using X = Foo;
+  using Y = X;
+  using Z = Y;
+  Z(42);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately 
after creation; did you mean to name the object?
+  // CHECK-FIXES: Z give_me_a_name(42);
+
+  typedef Z ZT;
+  ZT(42, 13);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately 
after creation; did you mean to name the object?
+  // CHECK-FIXES: ZT give_me_a_name(42, 13);
+
+  using TT = TCtorDefaultArg;
+  TT(42);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately 
after creation; did you mean to name the object?
+  // CHECK-FIXES: TT give_me_a_name(42);
+
+  (void)0;
+}
+
 void test() {
   Foo(42);
 // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after 
creation; did you mean to name the object?



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

[PATCH] D114206: [Clang][ScanDeps] Use the virtual path for module maps

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese updated this revision to Diff 391166.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114206

Files:
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
  clang/test/ClangScanDeps/modulemap-via-vfs.m


Index: clang/test/ClangScanDeps/modulemap-via-vfs.m
===
--- /dev/null
+++ clang/test/ClangScanDeps/modulemap-via-vfs.m
@@ -0,0 +1,56 @@
+// RUN: rm -rf %t.dir
+// RUN: split-file %s %t.dir
+// RUN: sed -e "s|DIR|%/t.dir|g" %t.dir/build/compile-commands.json.in > 
%t.dir/build/compile-commands.json
+// RUN: sed -e "s|DIR|%/t.dir|g" %t.dir/build/vfs.yaml.in > 
%t.dir/build/vfs.yaml
+// RUN: clang-scan-deps -compilation-database 
%t.dir/build/compile-commands.json -j 1 -format experimental-full \
+// RUN:   -mode preprocess-minimized-sources -generate-modules-path-args > 
%t.db
+// RUN: %python %S/../../utils/module-deps-to-rsp.py %t.db --module-name=A > 
%t.A.cc1.rsp
+// RUN: cat %t.A.cc1.rsp | sed 's:\?:/:g' | FileCheck %s
+
+// CHECK-NOT: build/module.modulemap
+// CHECK: A/module.modulemap
+
+//--- build/compile-commands.json.in
+
+[
+{
+  "directory": "DIR",
+  "command": "clang DIR/main.m -Imodules/A -fmodules 
-fmodules-cache-path=module-cache -fimplicit-modules -fimplicit-module-maps 
-ivfsoverlay build/vfs.yaml",
+  "file": "DIR/main.m"
+}
+]
+
+//--- build/module.modulemap
+
+module A {
+  umbrella header "A.h"
+}
+
+//--- modules/A/A.h
+
+typedef int A_t;
+
+//--- build/vfs.yaml.in
+
+{
+  "version": 0,
+  "case-sensitive": "false",
+  "roots": [
+  {
+ "contents": [
+ {
+"external-contents": "DIR/build/module.modulemap",
+"name": "module.modulemap",
+"type": "file"
+ }],
+ "name": "DIR/modules/A",
+ "type": "directory"
+  }
+  ]
+}
+
+//--- main.m
+
+@import A;
+
+A_t a = 0;
Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -233,7 +233,13 @@
.getHeaderSearchInfo()
.getModuleMap()
.getModuleMapFileForUniquing(M);
-  MD.ClangModuleMapFile = std::string(ModuleMap ? ModuleMap->getName() : "");
+
+  if (ModuleMap) {
+StringRef Path = ModuleMap->tryGetRealPathName();
+if (Path.empty())
+  Path = ModuleMap->getName();
+MD.ClangModuleMapFile = std::string(Path);
+  }
 
   serialization::ModuleFile *MF =
   MDC.ScanInstance.getASTReader()->getModuleManager().lookup(


Index: clang/test/ClangScanDeps/modulemap-via-vfs.m
===
--- /dev/null
+++ clang/test/ClangScanDeps/modulemap-via-vfs.m
@@ -0,0 +1,56 @@
+// RUN: rm -rf %t.dir
+// RUN: split-file %s %t.dir
+// RUN: sed -e "s|DIR|%/t.dir|g" %t.dir/build/compile-commands.json.in > %t.dir/build/compile-commands.json
+// RUN: sed -e "s|DIR|%/t.dir|g" %t.dir/build/vfs.yaml.in > %t.dir/build/vfs.yaml
+// RUN: clang-scan-deps -compilation-database %t.dir/build/compile-commands.json -j 1 -format experimental-full \
+// RUN:   -mode preprocess-minimized-sources -generate-modules-path-args > %t.db
+// RUN: %python %S/../../utils/module-deps-to-rsp.py %t.db --module-name=A > %t.A.cc1.rsp
+// RUN: cat %t.A.cc1.rsp | sed 's:\?:/:g' | FileCheck %s
+
+// CHECK-NOT: build/module.modulemap
+// CHECK: A/module.modulemap
+
+//--- build/compile-commands.json.in
+
+[
+{
+  "directory": "DIR",
+  "command": "clang DIR/main.m -Imodules/A -fmodules -fmodules-cache-path=module-cache -fimplicit-modules -fimplicit-module-maps -ivfsoverlay build/vfs.yaml",
+  "file": "DIR/main.m"
+}
+]
+
+//--- build/module.modulemap
+
+module A {
+  umbrella header "A.h"
+}
+
+//--- modules/A/A.h
+
+typedef int A_t;
+
+//--- build/vfs.yaml.in
+
+{
+  "version": 0,
+  "case-sensitive": "false",
+  "roots": [
+  {
+ "contents": [
+ {
+"external-contents": "DIR/build/module.modulemap",
+"name": "module.modulemap",
+"type": "file"
+ }],
+ "name": "DIR/modules/A",
+ "type": "directory"
+  }
+  ]
+}
+
+//--- main.m
+
+@import A;
+
+A_t a = 0;
Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -233,7 +233,13 @@
.getHeaderSearchInfo()
.getModuleMap()
.getModuleMapFileForUniquing(M);
-  MD.ClangModuleMapFile = std::string(ModuleMap ? ModuleMap->getName() : "");
+
+  if (ModuleMap) {
+StringRef Path = ModuleMap->tryGetRealPathName();

[PATCH] D111400: [Clang] Implement P2242R3

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D111400#3164301 , @cor3ntin wrote:

> If the issue is regarding the support and extension warning in C++20 and 
> older modes, it's something I can address by conserving the status quo in 
> older versions.

I think preserving the status quo in older modes is fine.

> If the ask is a more involved modification of how clang does SFINAE in 
> general, i don't think that i can take that on.

As long as this patch isn't making a change to the older modes, then there's 
nothing it's doing that requires special attention of the "disable if checking 
for SFINAE purposes" variety.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111400

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

[PATCH] D114206: [Clang][ScanDeps] Use the virtual path for module maps

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese added a comment.

There were two issues for Windows.

The first is an actual bug in the Clang VFS that should be fixed separately. 
You cannot pass just `main.m` as the input to Clang. Clang asks for its parent 
directory textually, which is `.`. It then tries to stat `.`, which the VFS 
rejects. Not sure why it's OK on non-Windows.

The other issue is not handling path separators in the FileCheck test. Fixed by 
adding `sed 's:\?:/:g'`.

I'm uploading a new patch and will commit on clean precommit tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114206

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

[PATCH] D114299: [clang-tidy] Fix `readability-redundant-declaration` false positive for template friend declaration

[Fabian Wolff via Phabricator via cfe-commits]

fwolff added a comment.

In D114299#3164127 , @aaron.ballman 
wrote:

> Hmm, the test case you added passes without the patch applied: 
> https://godbolt.org/z/T9TerMYGz

You are right; I have fixed the test now, and I've also added your other 
example as a second test.


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

https://reviews.llvm.org/D114299

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

[PATCH] D114299: [clang-tidy] Fix `readability-redundant-declaration` false positive for template friend declaration

[Fabian Wolff via Phabricator via cfe-commits]

fwolff updated this revision to Diff 391163.

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

https://reviews.llvm.org/D114299

Files:
  clang-tools-extra/clang-tidy/readability/RedundantDeclarationCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-declaration.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-declaration.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-declaration.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-declaration.cpp
@@ -70,6 +70,32 @@
 
 void enemy();
 
+template 
+struct TemplateFriendly {
+  template 
+  friend void generic_friend();
+};
+
+template 
+void generic_friend() {}
+
+TemplateFriendly template_friendly;
+
+template 
+struct TemplateFriendly2 {
+  template 
+  friend void generic_friend2() {}
+};
+
+template 
+void generic_friend2();
+
+void generic_friend_caller() {
+  TemplateFriendly2 f;
+  generic_friend2();
+}
+
+
 namespace macros {
 #define DECLARE(x) extern int x
 #define DEFINE(x) extern int x; int x = 42
Index: clang-tools-extra/clang-tidy/readability/RedundantDeclarationCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/RedundantDeclarationCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/RedundantDeclarationCheck.cpp
@@ -37,7 +37,7 @@
   functionDecl(unless(anyOf(
   isDefinition(), isDefaulted(),
   doesDeclarationForceExternallyVisibleDefinition(),
-  hasParent(friendDecl()))
+  hasAncestor(friendDecl()))
   .bind("Decl"),
   this);
 }


Index: clang-tools-extra/test/clang-tidy/checkers/readability-redundant-declaration.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-redundant-declaration.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-redundant-declaration.cpp
@@ -70,6 +70,32 @@
 
 void enemy();
 
+template 
+struct TemplateFriendly {
+  template 
+  friend void generic_friend();
+};
+
+template 
+void generic_friend() {}
+
+TemplateFriendly template_friendly;
+
+template 
+struct TemplateFriendly2 {
+  template 
+  friend void generic_friend2() {}
+};
+
+template 
+void generic_friend2();
+
+void generic_friend_caller() {
+  TemplateFriendly2 f;
+  generic_friend2();
+}
+
+
 namespace macros {
 #define DECLARE(x) extern int x
 #define DEFINE(x) extern int x; int x = 42
Index: clang-tools-extra/clang-tidy/readability/RedundantDeclarationCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/RedundantDeclarationCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/RedundantDeclarationCheck.cpp
@@ -37,7 +37,7 @@
   functionDecl(unless(anyOf(
   isDefinition(), isDefaulted(),
   doesDeclarationForceExternallyVisibleDefinition(),
-  hasParent(friendDecl()))
+  hasAncestor(friendDecl()))
   .bind("Decl"),
   this);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi

[Sami Tolvanen via Phabricator via cfe-commits]

samitolvanen updated this revision to Diff 391159.
samitolvanen marked 5 inline comments as done.
samitolvanen added a comment.

Addressed comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108479

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Expr.h
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
  clang/test/CodeGen/builtin-function-start.cpp
  clang/test/SemaCXX/builtins.cpp

Index: clang/test/SemaCXX/builtins.cpp
===
--- clang/test/SemaCXX/builtins.cpp
+++ clang/test/SemaCXX/builtins.cpp
@@ -39,6 +39,13 @@
   S *ptmp = __builtin_addressof(S{}); // expected-error {{taking the address of a temporary}}
 }
 
+namespace function_start {
+void a(void) {}
+int n;
+void *p = __builtin_function_start(n);   // expected-error {{argument must be a function}}
+static_assert(__builtin_function_start(a) == a, ""); // expected-error {{static_assert expression is not an integral constant expression}}
+} // namespace function_start
+
 void no_ms_builtins() {
   __assume(1); // expected-error {{use of undeclared}}
   __noop(1); // expected-error {{use of undeclared}}
Index: clang/test/CodeGen/builtin-function-start.cpp
===
--- /dev/null
+++ clang/test/CodeGen/builtin-function-start.cpp
@@ -0,0 +1,56 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fsanitize=cfi-icall -o - %s | FileCheck %s
+
+#if !__has_builtin(__builtin_function_start)
+#error "missing __builtin_function_start"
+#endif
+
+void a(void) {}
+// CHECK: @e = global i8* bitcast (void ()* no_cfi @_Z1av to i8*)
+const void *e = __builtin_function_start(a);
+
+constexpr void (*d)() = &a;
+// CHECK: @f = global i8* bitcast (void ()* no_cfi @_Z1av to i8*)
+const void *f = __builtin_function_start(d);
+
+void b(void) {}
+// CHECK: @g = global [2 x i8*] [i8* bitcast (void ()* @_Z1bv to i8*), i8* bitcast (void ()* no_cfi @_Z1bv to i8*)]
+void *g[] = {(void *)b, __builtin_function_start(b)};
+
+void c(void *p) {}
+
+class A {
+public:
+  void f();
+  virtual void g();
+  static void h();
+  int i() const;
+  int i(int n) const;
+};
+
+void A::f() {}
+void A::g() {}
+void A::h() {}
+
+// CHECK: define {{.*}}i32 @_ZNK1A1iEv(%class.A* {{.*}}%this)
+int A::i() const { return 0; }
+
+// CHECK: define {{.*}}i32 @_ZNK1A1iEi(%class.A* {{.*}}%this, i32 %n)
+int A::i(int n) const { return 0; }
+
+void h(void) {
+  // CHECK: store i8* bitcast (void ()* no_cfi @_Z1bv to i8*), i8** %g
+  void *g = __builtin_function_start(b);
+  // call void @_Z1cPv(i8* bitcast (void ()* no_cfi @_Z1av to i8*))
+  c(__builtin_function_start(a));
+
+  // CHECK: store i8* bitcast (void (%class.A*)* no_cfi @_ZN1A1fEv to i8*), i8** %Af
+  void *Af = __builtin_function_start(&A::f);
+  // CHECK: store i8* bitcast (void (%class.A*)* no_cfi @_ZN1A1gEv to i8*), i8** %Ag
+  void *Ag = __builtin_function_start(&A::g);
+  // CHECK: store i8* bitcast (void ()* no_cfi @_ZN1A1hEv to i8*), i8** %Ah
+  void *Ah = __builtin_function_start(&A::h);
+  // CHECK: store i8* bitcast (i32 (%class.A*)* no_cfi @_ZNK1A1iEv to i8*), i8** %Ai1
+  void *Ai1 = __builtin_function_start((int(A::*)() const) & A::i);
+  // CHECK: store i8* bitcast (i32 (%class.A*, i32)* no_cfi @_ZNK1A1iEi to i8*), i8** %Ai2
+  void *Ai2 = __builtin_function_start((int(A::*)(int) const) & A::i);
+}
Index: clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -66,7 +66,8 @@
   case Builtin::BI__builtin_expect:
   case Builtin::BI__builtin_expect_with_probability:
   case Builtin::BI__builtin_assume_aligned:
-  case Builtin::BI__builtin_addressof: {
+  case Builtin::BI__builtin_addressof:
+  case Builtin::BI__builtin_function_start: {
 // For __builtin_unpredictable, __builtin_expect,
 // __builtin_expect_with_probability and __builtin_assume_aligned,
 // just return the value of the subexpression.
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -195,6 +195,29 @@
   return false;
 }
 
+/// Check that the argument to __builtin_function_start is a function.
+static bool SemaBuiltinFunctionStart(Sema &S, CallExpr *TheCall) {
+  if (checkArgCount(S, TheCall, 1))
+return true;
+
+  ExprResult Arg 

[PATCH] D114530: [clang][scan-build] Use cc/c++ instead of gcc/g++ on FreeBSD.

[Brooks Davis via Phabricator via cfe-commits]

brooks added a comment.

Thanks, this will kill off one of the patches in our ports.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114530

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

[PATCH] D114732: [clang] Mark `trivial_abi` types as "trivially relocatable".

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D114732#3159247 , @Quuxplusone 
wrote:

> - D50119  has more extensive tests, and has 
> been live on https://p1144.godbolt.org/z/axx9Wj3r5 for a couple years now; if 
> the consensus is that Clang wants "part of D50119 
>  but not all of it," then IMHO it would be 
> reasonable to put some comments on D50119  
> with the goal of stripping it //down//, rather than trying to build //up// a 
> smaller version of it in parallel.

I've compared the two patches, and as far as I can tell, this patch is 
effectively the subset of D50119  that 
includes only the new type trait and not the new attributes, with the following 
exceptions:

- No manual `__has_extension` support. This is an improvement in this patch; 
the intended way to test for type trait primitives is with `__has_builtin`, and 
that has built-in knowledge of type traits. I think this may have changed since 
D50119  was first proposed; `__has_builtin` 
has not always supported type trait builtins.
- This patch has an unnecessary check for trivial destruction of trivially 
copyable types (commented in this review).
- This patch treats trivial-for-calls as implying trivially-relocatable, and in 
particular this means that `[[trivial_abi]]` types are implicitly 
trivially-relocatable, as discussed below.
- `-ast-dump` support. I'd be inclined to omit that from this change; until / 
unless we have an attribute that makes a class trivially-relocatable but not 
trivial for calls, adding dump support for `canPassInRegisters` rather than 
`isTriviallyRelocatable` seems better-motivated, but wouldn't make sense as 
part of this patch.
- Different set of test cases.

Even if we want to land all of the functionality in D50119 
 now, splitting out the new type trait and the 
new attribute into separate patches makes sense to me.

> - (Major point of contention) To me, `[[trivial_abi]]` does not imply 
> `[[trivially_relocatable]]` at all. I believe @rjmccall disagreed a few years 
> ago: basically I said "The two attributes are orthogonal," with practical 
> demonstration, and basically he said "yes, in practice you're right, but 
> philosophically that's a bug and we do actually //intend// `[[trivial_abi]]` 
> to imply `[[trivially_relocatable]]` even though the compiler doesn't enforce 
> it" (but then AFAIK nothing was ever done about that).

I agree with @rjmccall on this: `[[trivial_abi]]` is meant to imply both that 
it's correct to trivially relocate and that we should perform a trivial 
relocation when making a function call. The latter without the former doesn't 
make sense to me. I agree that the former without the latter does make sense, 
and I don't think this change is an impediment to supporting that.

Personally I'm more comfortable landing this subset of D50119 
 now than I would be with landing all of 
D50119 , given that the type trait seems 
well-motivated and unlikely to have any serious conflicts with an 
eventually-standardized feature. John's concern that the semantics of an 
eventual `[[trivially_relocatable]]` attribute may differ from what's in D50119 
 seem well-founded to me.




Comment at: clang/include/clang/Basic/AttrDocs.td:3211-3215
+The compiler will pass and return a trivially relocatable type using the C ABI
+for the underlying type, even when the type would otherwise be considered
+non-trivially-relocatable. If a type is trivially relocatable, has a
+non-trivial destructor, and is passed as an argument by value, the convention
+is that the callee will destroy the object before returning.

I think this documentation change has mixed together two different things. The 
revised wording says that `[[trivial_abi]]` implies trivially-relocatable and 
that trivially-relocatable implies passing using the C ABI, which is wrong: the 
second implication does not hold. What we should say is that `[[trivial_abi]]` 
(if we're not in the "has no effect" case described below) implies both that 
the type is trivially-relocatable, and that it is passed using the C ABI (that 
is, that we trivially relocate in argument passing).

Instead of the wording changes you have here, perhaps we could leave the old 
wording alone and add a paragraph that says that a type with the attribute is 
assumed to be trivially-relocatable for the purpose of 
`__is_trivially_relocatable`, but that Clang doesn't yet take advantage of this 
fact anywhere other than argument passing.



Comment at: clang/lib/AST/Type.cpp:2500
+return BaseElementType.isTriviallyCopyableType(Context) &&
+   !BaseElementType.isDestructedType();
+  }

Yo

[PATCH] D113148: Add new clang-tidy check for string_view(nullptr)

[CJ Johnson via Phabricator via cfe-commits]

CJ-Johnson updated this revision to Diff 391140.
CJ-Johnson added a comment.

Delete non-functional test cases (which were previously commented out)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113148

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-stringview-nullptr.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp
@@ -0,0 +1,1102 @@
+// RUN: %check_clang_tidy %s bugprone-stringview-nullptr -std=c++17 %t
+
+namespace std {
+
+using size_t = long long;
+using nullptr_t = decltype(nullptr);
+
+template 
+T &&declval();
+
+template 
+struct type_identity { using type = T; };
+template 
+using type_identity_t = typename type_identity::type;
+
+template 
+class basic_string_view {
+public:
+  basic_string_view();
+
+  basic_string_view(const CharT *);
+
+  // Not present in C++17 and C++20:
+  // basic_string_view(std::nullptr_t);
+
+  basic_string_view(const CharT *, size_t);
+
+  basic_string_view(const basic_string_view &);
+
+  basic_string_view &operator=(const basic_string_view &);
+};
+
+template 
+bool operator<(basic_string_view, basic_string_view);
+template 
+bool operator<(type_identity_t>,
+   basic_string_view);
+template 
+bool operator<(basic_string_view,
+   type_identity_t>);
+
+template 
+bool operator<=(basic_string_view, basic_string_view);
+template 
+bool operator<=(type_identity_t>,
+basic_string_view);
+template 
+bool operator<=(basic_string_view,
+type_identity_t>);
+
+template 
+bool operator>(basic_string_view, basic_string_view);
+template 
+bool operator>(type_identity_t>,
+   basic_string_view);
+template 
+bool operator>(basic_string_view,
+   type_identity_t>);
+
+template 
+bool operator>=(basic_string_view, basic_string_view);
+template 
+bool operator>=(type_identity_t>,
+basic_string_view);
+template 
+bool operator>=(basic_string_view,
+type_identity_t>);
+
+template 
+bool operator==(basic_string_view, basic_string_view);
+template 
+bool operator==(type_identity_t>,
+basic_string_view);
+template 
+bool operator==(basic_string_view,
+type_identity_t>);
+
+template 
+bool operator!=(basic_string_view, basic_string_view);
+template 
+bool operator!=(type_identity_t>,
+basic_string_view);
+template 
+bool operator!=(basic_string_view,
+type_identity_t>);
+
+using string_view = basic_string_view;
+
+} // namespace std
+
+void function(std::string_view);
+void function(std::string_view, std::string_view);
+
+void temporary_construction() /* a */ {
+  // Functional Cast
+  {
+(void)(std::string_view(nullptr)) /* a1 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing basic_string_view from null is undefined; replace with the default constructor
+// CHECK-FIXES: {{^}}(void)(std::string_view()) /* a1 */;
+
+(void)(std::string_view((nullptr))) /* a2 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing{{.*}}default
+// CHECK-FIXES: {{^}}(void)(std::string_view()) /* a2 */;
+
+(void)(std::string_view({nullptr})) /* a3 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing{{.*}}default
+// CHECK-FIXES: {{^}}(void)(std::string_view()) /* a3 */;
+
+(void)(std::string_view({(nullptr)})) /* a4 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing{{.*}}default
+// CHECK-FIXES: {{^}}(void)(std::string_view()) /* a4 */;
+
+(void)(std::string_view({})) /* a5 */; // Default `const CharT*`
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing{{.*}}default
+// CHECK-FIXES: {{^}}(void)(std::string_view()) /* a5 */;
+  }
+
+  // Temporary Object
+  {
+(void)(std::string_view{nullptr}) /* a6 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing{{.*}}default
+// CHECK-FIXES: {{^}}(void)(std::string_view{}) /* a6 */;
+
+(void)(std::string_view{(nullptr)}) /* a7 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing{{.*}}default
+// CHECK-FIXES: {{^}}(void)(std::string_view{}) /* a7 */;
+
+(void)(std::string_view{{nullptr}}) /* a8 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing{{.*}}default
+// CHECK-FIXES: {{^}}(void)(st

[PATCH] D114887: [Analyzer][solver] Simplification: Do a fixpoint iteration before the eq class merge

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D114887#3165397 , @steakhal wrote:

> Also, please provide the coverage of the new test case (only excercising the 
> new test case not the whole test file) I really want to make sure that it 
> will cover the loop as it supposed to.
>
> Oh, I just now see that there is no test. Now I'm really courious.

There is a test case in D103317  (with the so 
many `b`s) which covers the loop. That patch is the dependent patch of the 
reverted patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114887

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

[PATCH] D113148: Add new clang-tidy check for string_view(nullptr)

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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

To other reviewers: barring any additional comments, I will push this patch 
tomorrow morning (CJ doesn't have commit rights).




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp:117-130
+// (void)(const std::string_view(nullptr)) /* a6 */;
+// CV qualifiers do not compile in this context
+
+// (void)(const std::string_view((nullptr))) /* a7 */;
+// CV qualifiers do not compile in this context
+
+// (void)(const std::string_view({nullptr})) /* a8 */;

nit: personally, i'd just delete these -- anything that doens't compile isn't 
really of interest to clang tidy. While I see the educational value, these 
kinds of comments are prone to bitrot (since we're not actually testing 
anything) and therefore have questionable value even for education.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp:134-147
+// (void)(const std::string_view{nullptr}) /* a16 */;
+// CV qualifiers do not compile in this context
+
+// (void)(const std::string_view{(nullptr)}) /* a17 */;
+// CV qualifiers do not compile in this context
+
+// (void)(const std::string_view{{nullptr}}) /* a18 */;

CJ-Johnson wrote:
> ymandel wrote:
> > what are these lines doing?
> These lines are not "tests" themselves but they clearly document that all of 
> the various permutations have been considered. If someone reads the test file 
> and thinks "what about this other case?", these demonstrate that such other 
> cases have been considered but are not valid :)
ah, i see. maybe put in a comment to that respect?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113148

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

[PATCH] D114887: [Analyzer][solver] Simplification: Do a fixpoint iteration before the eq class merge

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

Also, please provide the coverage of the new test case (only excercising the 
new test case not the whole test file) I really want to make sure that it will 
cover the loop as it supposed to.

Oh, I just now see that there is no test. Now I'm really courious.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114887

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

[PATCH] D114887: [Analyzer][solver] Simplification: Do a fixpoint iteration before the eq class merge

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

I think I'm in favor of this change. However, Im also slightly in favor of 
reverting stuff instead of rushing things. Let's hope it will resolve the issue 
of the previous offending commit.

Accepted.




Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2220
+  const SymbolRef Sym) {
+  SVal Val = SVB.makeSymbolVal(Sym);
+  SVal SimplifiedVal = SVB.simplifySVal(State, Val);

Couldnt we just create an SVal from a SymExpr? Why do we need the SVB foe this?



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:
+  SVal SimplifiedVal = SVB.simplifySVal(State, Val);
+  // Do the simplification until we can.
+  while (SimplifiedVal != Val) {

Nit: It doesnt seem to be grammatically correct.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2218
+LLVM_NODISCARD
+static SVal simplifyUntilFixpoint(SValBuilder &SVB, ProgramStateRef State,
+  const SymbolRef Sym) {

martong wrote:
> Perhaps this should be done in `SimpleSValbuilder::simplifySVal`, I think 
> this should be the responsibility of the SValBuilder compoment.
My thought was the same.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114887

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

[PATCH] D114908: [clang] Don't call inheritDefaultTemplateArguments() on CXXDeductionGuideDecl's template parameters

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks created this revision.
aeubanks added a reviewer: rsmith.
aeubanks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

A CXXDeductionGuideDecl references its class's template parameters
(rather than cloning them). This causes us to currently call
inheritDefaultTemplateArguments() on the class template parameters
twice, which causes crashes because DefaultArgStorage::setInherited()
should only be called once.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114908

Files:
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/test/Modules/Inputs/ctad/a.h
  clang/test/Modules/Inputs/ctad/b.h
  clang/test/Modules/Inputs/ctad/module.modulemap
  clang/test/Modules/ctad.cpp


Index: clang/test/Modules/ctad.cpp
===
--- /dev/null
+++ clang/test/Modules/ctad.cpp
@@ -0,0 +1,12 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c++ -emit-module 
%S/Inputs/ctad/module.modulemap -fmodule-name=a -o %t/a.pcm -std=c++17
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c++ -emit-module 
%S/Inputs/ctad/module.modulemap -fmodule-name=b -o %t/b.pcm -std=c++17
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c++ -emit-llvm 
-I%S/Inputs/ctad -o - %s -fmodule-file=%t/a.pcm -fmodule-file=%t/b.pcm 
-std=c++17 | FileCheck %s
+
+// CHECK: @_a = global
+// CHECK: @_b = global
+// CHECK: call void @__cxx_global_var_init()
+// CHECK: call void @__cxx_global_var_init.1()
+
+#include "a.h"
+#include "b.h"
Index: clang/test/Modules/Inputs/ctad/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/ctad/module.modulemap
@@ -0,0 +1,2 @@
+module a { header "a.h" export * }
+module b { header "b.h" export * }
Index: clang/test/Modules/Inputs/ctad/b.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/ctad/b.h
@@ -0,0 +1,7 @@
+#pragma once
+
+template struct A {
+  A(T) {}
+};
+
+A _b(5);
Index: clang/test/Modules/Inputs/ctad/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/ctad/a.h
@@ -0,0 +1,7 @@
+#pragma once
+
+template struct A {
+  A(T) {}
+};
+
+A _a(2);
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -3781,9 +3781,13 @@
 
   // If the declaration declares a template, it may inherit default arguments
   // from the previous declaration.
-  if (auto *TD = dyn_cast(D))
-inheritDefaultTemplateArguments(Reader.getContext(),
-cast(Previous), TD);
+  if (auto *TD = dyn_cast(D)) {
+// CXXDeductionGuideDecls reference the class template parameters so we 
need
+// to make sure not to call this twice on the same template parameters.
+if (!isa(TD->getTemplatedDecl()))
+  inheritDefaultTemplateArguments(Reader.getContext(),
+  cast(Previous), TD);
+  }
 
   // If any of the declaration in the chain contains an Inheritable attribute,
   // it needs to be added to all the declarations in the redeclarable chain.


Index: clang/test/Modules/ctad.cpp
===
--- /dev/null
+++ clang/test/Modules/ctad.cpp
@@ -0,0 +1,12 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c++ -emit-module %S/Inputs/ctad/module.modulemap -fmodule-name=a -o %t/a.pcm -std=c++17
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c++ -emit-module %S/Inputs/ctad/module.modulemap -fmodule-name=b -o %t/b.pcm -std=c++17
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c++ -emit-llvm -I%S/Inputs/ctad -o - %s -fmodule-file=%t/a.pcm -fmodule-file=%t/b.pcm -std=c++17 | FileCheck %s
+
+// CHECK: @_a = global
+// CHECK: @_b = global
+// CHECK: call void @__cxx_global_var_init()
+// CHECK: call void @__cxx_global_var_init.1()
+
+#include "a.h"
+#include "b.h"
Index: clang/test/Modules/Inputs/ctad/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/ctad/module.modulemap
@@ -0,0 +1,2 @@
+module a { header "a.h" export * }
+module b { header "b.h" export * }
Index: clang/test/Modules/Inputs/ctad/b.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/ctad/b.h
@@ -0,0 +1,7 @@
+#pragma once
+
+template struct A {
+  A(T) {}
+};
+
+A _b(5);
Index: clang/test/Modules/Inputs/ctad/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/ctad/a.h
@@ -0,0 +1,7 @@
+#pragma once
+
+template struct A {
+  A(T) {}
+};
+
+A _a(2);
Index: clang/lib/Serialization/ASTReaderDecl.cpp
==

[PATCH] D114619: [Analyzer][solver] Do not remove the simplified symbol from the eq class

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

I've had the confidence to commit D114887 , 
the test should not be flaky anymore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114619

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

[PATCH] D114530: [clang][scan-build] Use cc/c++ instead of gcc/g++ on FreeBSD.

[Michał Górny via Phabricator via cfe-commits]

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

I really hate that we need to hardcode defaults like this but this is outside 
this patch. IMHO `cc`/`c++` should work everywhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114530

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

[clang] 20f8733 - [Analyzer][solver] Simplification: Do a fixpoint iteration before the eq class merge

[Gabor Marton via cfe-commits]

Author: Gabor Marton
Date: 2021-12-01T22:23:41+01:00
New Revision: 20f8733d4b8d5bdb93080b8824de57b7fae31785

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

LOG: [Analyzer][solver] Simplification: Do a fixpoint iteration before the eq 
class merge

This reverts commit f02c5f3478318075d1a469203900e452ba651421 and
addresses the issue mentioned in D114619 differently.

Repeating the issue here:
Currently, during symbol simplification we remove the original member
symbol from the equivalence class (`ClassMembers` trait). However, we
keep the reverse link (`ClassMap` trait), in order to be able the query
the related constraints even for the old member. This asymmetry can lead
to a problem when we merge equivalence classes:
```
ClassA: [a, b]   // ClassMembers trait,
a->a, b->a   // ClassMap trait, a is the representative symbol
```
Now let,s delete `a`:
```
ClassA: [b]
a->a, b->a
```
Let's merge ClassA into the trivial class `c`:
```
ClassA: [c, b]
c->c, b->c, a->a
```
Now, after the merge operation, `c` and `a` are actually in different
equivalence classes, which is inconsistent.

This issue manifests in a test case (added in D103317):
```
void recurring_symbol(int b) {
  if (b * b != b)
if ((b * b) * b * b != (b * b) * b)
  if (b * b == 1)
}
```
Before the simplification we have these equivalence classes:
```
trivial EQ1: [b * b != b]
trivial EQ2: [(b * b) * b * b != (b * b) * b]
```

During the simplification with `b * b == 1`, EQ1 is merged with `1 != b`
`EQ1: [b * b != b, 1 != b]` and we remove the complex symbol, so
`EQ1: [1 != b]`
Then we start to simplify the only symbol in EQ2:
`(b * b) * b * b != (b * b) * b --> 1 * b * b != 1 * b --> b * b != b`
But `b * b != b` is such a symbol that had been removed previously from
EQ1, thus we reach the above mentioned inconsistency.

This patch addresses the issue by making it impossible to synthesise a
symbol that had been simplified before. We achieve this by simplifying
the given symbol to the absolute simplest form.

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
clang/test/Analysis/expr-inspection-printState-eq-classes.c
clang/test/Analysis/symbol-simplification-disequality-info.cpp
clang/test/Analysis/symbol-simplification-fixpoint-one-iteration.cpp
clang/test/Analysis/symbol-simplification-fixpoint-two-iterations.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp 
b/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
index 3f8d9e4298a09..23c67c64f9752 100644
--- a/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -601,6 +601,10 @@ class EquivalenceClass : public llvm::FoldingSetNode {
   LLVM_NODISCARD static inline Optional
   areEqual(ProgramStateRef State, SymbolRef First, SymbolRef Second);
 
+  /// Remove one member from the class.
+  LLVM_NODISCARD ProgramStateRef removeMember(ProgramStateRef State,
+  const SymbolRef Old);
+
   /// Iterate over all symbols and try to simplify them.
   LLVM_NODISCARD static inline ProgramStateRef simplify(SValBuilder &SVB,
 RangeSet::Factory &F,
@@ -2132,6 +2136,34 @@ inline Optional 
EquivalenceClass::areEqual(ProgramStateRef State,
   return llvm::None;
 }
 
+LLVM_NODISCARD ProgramStateRef
+EquivalenceClass::removeMember(ProgramStateRef State, const SymbolRef Old) {
+
+  SymbolSet ClsMembers = getClassMembers(State);
+  assert(ClsMembers.contains(Old));
+
+  // We don't remove `Old`'s Sym->Class relation for two reasons:
+  // 1) This way constraints for the old symbol can still be found via it's
+  // equivalence class that it used to be the member of.
+  // 2) Performance and resource reasons. We can spare one removal and thus one
+  // additional tree in the forest of `ClassMap`.
+
+  // Remove `Old`'s Class->Sym relation.
+  SymbolSet::Factory &F = getMembersFactory(State);
+  ClassMembersTy::Factory &EMFactory = State->get_context();
+  ClsMembers = F.remove(ClsMembers, Old);
+  // Ensure another precondition of the removeMember function (we can check
+  // this only with isEmpty, thus we have to do the remove first).
+  assert(!ClsMembers.isEmpty() &&
+ "Class should have had at least two members before member removal");
+  // Overwrite the existing members assigned to this class.
+  ClassMembersTy ClassMembersMap = State->get();
+  ClassMembersMap = EMFactory.add(ClassMembersMap, *this, ClsMembers);
+  State = State->set(ClassMembersMap);
+
+  return State;
+}
+
 // Re-evaluate an SVal with top-level `State->assume` logic.
 LLVM_NODISCARD Progra

[PATCH] D114887: [Analyzer][solver] Simplification: Do a fixpoint iteration before the eq class merge

[Gabor Marton via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG20f8733d4b8d: [Analyzer][solver] Simplification: Do a 
fixpoint iteration before the eq class… (authored by martong).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114887

Files:
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/test/Analysis/expr-inspection-printState-eq-classes.c
  clang/test/Analysis/symbol-simplification-disequality-info.cpp
  clang/test/Analysis/symbol-simplification-fixpoint-one-iteration.cpp
  clang/test/Analysis/symbol-simplification-fixpoint-two-iterations.cpp

Index: clang/test/Analysis/symbol-simplification-fixpoint-two-iterations.cpp
===
--- clang/test/Analysis/symbol-simplification-fixpoint-two-iterations.cpp
+++ clang/test/Analysis/symbol-simplification-fixpoint-two-iterations.cpp
@@ -27,20 +27,17 @@
   if (b != 0)
 return;
   clang_analyzer_printState();
-  // CHECK:  "constraints": [
-  // CHECK-NEXT:   { "symbol": "(((reg_$0) + (reg_$1)) + (reg_$2)) != (reg_$3)", "range": "{ [0, 0] }" },
-  // CHECK-NEXT:   { "symbol": "((reg_$0) + (reg_$2)) != (reg_$3)", "range": "{ [0, 0] }" },
-  // CHECK-NEXT:   { "symbol": "(reg_$0) != (reg_$3)", "range": "{ [0, 0] }" },
-  // CHECK-NEXT:   { "symbol": "(reg_$2) + (reg_$1)", "range": "{ [0, 0] }" },
-  // CHECK-NEXT:   { "symbol": "reg_$1", "range": "{ [0, 0] }" },
-  // CHECK-NEXT:   { "symbol": "reg_$2", "range": "{ [0, 0] }" }
-  // CHECK-NEXT: ],
-  // CHECK-NEXT: "equivalence_classes": [
-  // CHECK-NEXT:   [ "(((reg_$0) + (reg_$1)) + (reg_$2)) != (reg_$3)", "((reg_$0) + (reg_$2)) != (reg_$3)", "(reg_$0) != (reg_$3)" ],
-  // CHECK-NEXT:   [ "((reg_$0) + (reg_$1)) + (reg_$2)", "(reg_$0) + (reg_$2)", "reg_$0", "reg_$3" ],
-  // CHECK-NEXT:   [ "(reg_$2) + (reg_$1)", "reg_$2" ]
-  // CHECK-NEXT: ],
-  // CHECK-NEXT: "disequality_info": null,
+  // CHECK:   "constraints": [
+  // CHECK-NEXT:{ "symbol": "(reg_$0) != (reg_$3)", "range": "{ [0, 0] }" },
+  // CHECK-NEXT:{ "symbol": "reg_$1", "range": "{ [0, 0] }" },
+  // CHECK-NEXT:{ "symbol": "reg_$2", "range": "{ [0, 0] }" }
+  // CHECK-NEXT:  ],
+  // CHECK-NEXT:  "equivalence_classes": [
+  // CHECK-NEXT:[ "(reg_$0) != (reg_$3)" ],
+  // CHECK-NEXT:[ "reg_$0", "reg_$3" ],
+  // CHECK-NEXT:[ "reg_$2" ]
+  // CHECK-NEXT:  ],
+  // CHECK-NEXT:  "disequality_info": null,
 
   // Keep the symbols and the constraints! alive.
   (void)(a * b * c * d);
Index: clang/test/Analysis/symbol-simplification-fixpoint-one-iteration.cpp
===
--- clang/test/Analysis/symbol-simplification-fixpoint-one-iteration.cpp
+++ clang/test/Analysis/symbol-simplification-fixpoint-one-iteration.cpp
@@ -24,15 +24,14 @@
   if (b != 0)
 return;
   clang_analyzer_printState();
-  // CHECK:  "constraints": [
-  // CHECK-NEXT:   { "symbol": "((reg_$0) + (reg_$1)) != (reg_$2)", "range": "{ [0, 0] }" },
-  // CHECK-NEXT:   { "symbol": "(reg_$0) != (reg_$2)", "range": "{ [0, 0] }" },
-  // CHECK-NEXT:   { "symbol": "reg_$1", "range": "{ [0, 0] }" }
-  // CHECK-NEXT: ],
-  // CHECK-NEXT: "equivalence_classes": [
-  // CHECK-NEXT:   [ "((reg_$0) + (reg_$1)) != (reg_$2)", "(reg_$0) != (reg_$2)" ],
-  // CHECK-NEXT:   [ "(reg_$0) + (reg_$1)", "reg_$0", "reg_$2" ]
-  // CHECK-NEXT: ],
+  // CHECK:"constraints": [
+  // CHECK-NEXT: { "symbol": "(reg_$0) != (reg_$2)", "range": "{ [0, 0] }" },
+  // CHECK-NEXT: { "symbol": "reg_$1", "range": "{ [0, 0] }" }
+  // CHECK-NEXT:   ],
+  // CHECK-NEXT:   "equivalence_classes": [
+  // CHECK-NEXT: [ "(reg_$0) != (reg_$2)" ],
+  // CHECK-NEXT: [ "reg_$0", "reg_$2" ]
+  // CHECK-NEXT:   ],
   // CHECK-NEXT: "disequality_info": null,
 
   // Keep the symbols and the constraints! alive.
Index: clang/test/Analysis/symbol-simplification-disequality-info.cpp
===
--- clang/test/Analysis/symbol-simplification-disequality-info.cpp
+++ clang/test/Analysis/symbol-simplification-disequality-info.cpp
@@ -12,18 +12,18 @@
   if (a + b + c == d)
 return;
   clang_analyzer_printState();
-  // CHECK:  "disequality_info": [
-  // CHECK-NEXT:   {
-  // CHECK-NEXT: "class": [ "((reg_$0) + (reg_$1)) + (reg_$2)" ],
-  // CHECK-NEXT: "disequal_to": [
-  // CHECK-NEXT:   [ "reg_$3" ]]
-  // CHECK-NEXT:   },
-  // CHECK-NEXT:   {
-  // CHECK-NEXT: "class": [ "reg_$3" ],
-  // CHECK-NEXT: "disequal_to": [
-  // CHECK-NEXT:   [ "((reg_$0) + (reg_$1)) + (reg_$2)" ]]
-  // CHECK-NEXT:   }
-  // CHECK-NEXT: ],
+  // CHECK:   "disequality_info": [
+  // CHECK-NEXT:{
+  // CHECK-NEXT:  "class

[PATCH] D114887: [Analyzer][solver] Simplification: Do a fixpoint iteration before the eq class merge

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

I am attaching the performance measurement results:

F20834663: removemember_revert_with_another_fixpoint.png 


F20834673: stats.html 

There is no any noticeable difference.

I am going to commit this as is because there is a push to revert D114619 
 (because of a flaky test) and this commit 
does that. On the other hand this is not a pure revert, so I apologize for the 
reviewers. If we were to purely revert D114619 
 then we'd have to revert its dependent child 
patch as well.

I have a strong confidence in this patch since I had a verbal discussion about 
this with @steakhal and he verified the basic idea. I am going to address my 
comment about `simplifyUntilFixpoint` in a follow-up patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114887

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

[PATCH] D114833: [modules] Fix ambiguous name lookup for enum constants from hidden submodules.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

After more testing and thinking I've realized we are still not handling 
anonymous enums properly. On one hand anonymous EnumDecl aren't duplicates and 
we aren't making a hidden EnumDecl + EnumConstantDecl visible and don't have 
ambiguity problems. On the other hand, when we do make them visible through 
explicit import, we are hitting ambiguous references again.

Need to check the previous discussions and see if can bring ObjC/C closer to 
C++ in context of handling these duplicates. In ASTReaderDecl I was trying to 
keep ObjC/C support similar to C++ (modulo templates) and it turned out to be 
better for comprehension.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114833

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

[PATCH] D114849: [AMDGPU][clang] Fix __builtin_nontemporal_store() failure on AMDGPU

[Yaxun Liu via Phabricator via cfe-commits]

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

LGTM. Thanks.


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

https://reviews.llvm.org/D114849

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

[PATCH] D112648: [clang-tidy] Don't offer partial fix-its for `modernize-pass-by-value`

[Adrian Vogelsgesang via Phabricator via cfe-commits]

avogelsgesang added a comment.

@aaron.ballman can you please commit this on my behalf? I don't have commit 
access


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112648

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

[PATCH] D114454: [NFC][AIX]Disable unstable CSA tests failing on AIX

[Steven Wan via Phabricator via cfe-commits]

stevewan added a comment.

> but we could pick a couple of esoteric targets where integrals differ 
> significantly from what we have on x86_64

Turns out `wasm` and `darwin` also have a similar problem, I added them to the 
skip list for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114454

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

[PATCH] D112648: [clang-tidy] Don't offer partial fix-its for `modernize-pass-by-value`

[Adrian Vogelsgesang via Phabricator via cfe-commits]

avogelsgesang updated this revision to Diff 391121.
avogelsgesang marked an inline comment as done.
avogelsgesang added a comment.

Add missing dot to comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112648

Files:
  clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp
@@ -231,5 +231,18 @@
 
 struct U {
   U(const POD &M) : M(M) {}
+  // CHECK-FIXES: U(const POD &M) : M(M) {}
   POD M;
 };
+
+// The rewrite can't look through `typedefs` and `using`.
+// Test that we don't partially rewrite one decl without rewriting the other.
+using MovableConstRef = const Movable &;
+struct V {
+  V(MovableConstRef M);
+  // CHECK-FIXES: V(MovableConstRef M);
+  Movable M;
+};
+V::V(const Movable &M) : M(M) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: pass by value and use std::move
+// CHECK-FIXES: V::V(const Movable &M) : M(M) {}
Index: clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
@@ -190,24 +190,35 @@
 
   auto Diag = diag(ParamDecl->getBeginLoc(), "pass by value and use 
std::move");
 
-  // Iterate over all declarations of the constructor.
-  for (const ParmVarDecl *ParmDecl : collectParamDecls(Ctor, ParamDecl)) {
-auto ParamTL = ParmDecl->getTypeSourceInfo()->getTypeLoc();
-auto RefTL = ParamTL.getAs();
+  // If we received a `const&` type, we need to rewrite the function
+  // declarations.
+  if (ParamDecl->getType()->isLValueReferenceType()) {
+// Check if we can succesfully rewrite all declarations of the constructor.
+for (const ParmVarDecl *ParmDecl : collectParamDecls(Ctor, ParamDecl)) {
+  TypeLoc ParamTL = ParmDecl->getTypeSourceInfo()->getTypeLoc();
+  ReferenceTypeLoc RefTL = ParamTL.getAs();
+  if (RefTL.isNull()) {
+// We cannot rewrite this instance. The type is probably hidden behind
+// some `typedef`. Do not offer a fix-it in this case.
+return;
+  }
+}
+// Rewrite all declarations.
+for (const ParmVarDecl *ParmDecl : collectParamDecls(Ctor, ParamDecl)) {
+  TypeLoc ParamTL = ParmDecl->getTypeSourceInfo()->getTypeLoc();
+  ReferenceTypeLoc RefTL = ParamTL.getAs();
 
-// Do not replace if it is already a value, skip.
-if (RefTL.isNull())
-  continue;
-
-TypeLoc ValueTL = RefTL.getPointeeLoc();
-auto TypeRange = CharSourceRange::getTokenRange(ParmDecl->getBeginLoc(),
-ParamTL.getEndLoc());
-std::string ValueStr = Lexer::getSourceText(CharSourceRange::getTokenRange(
-ValueTL.getSourceRange()),
-SM, getLangOpts())
-   .str();
-ValueStr += ' ';
-Diag << FixItHint::CreateReplacement(TypeRange, ValueStr);
+  TypeLoc ValueTL = RefTL.getPointeeLoc();
+  CharSourceRange TypeRange = CharSourceRange::getTokenRange(
+  ParmDecl->getBeginLoc(), ParamTL.getEndLoc());
+  std::string ValueStr =
+  Lexer::getSourceText(
+  CharSourceRange::getTokenRange(ValueTL.getSourceRange()), SM,
+  getLangOpts())
+  .str();
+  ValueStr += ' ';
+  Diag << FixItHint::CreateReplacement(TypeRange, ValueStr);
+}
   }
 
   // Use std::move in the initialization list.


Index: clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-pass-by-value.cpp
@@ -231,5 +231,18 @@
 
 struct U {
   U(const POD &M) : M(M) {}
+  // CHECK-FIXES: U(const POD &M) : M(M) {}
   POD M;
 };
+
+// The rewrite can't look through `typedefs` and `using`.
+// Test that we don't partially rewrite one decl without rewriting the other.
+using MovableConstRef = const Movable &;
+struct V {
+  V(MovableConstRef M);
+  // CHECK-FIXES: V(MovableConstRef M);
+  Movable M;
+};
+V::V(const Movable &M) : M(M) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: pass by value and use std::move
+// CHECK-FIXES: V::V(const Movable &M) : M(M) {}
Index: clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp

[PATCH] D114454: [NFC][AIX]Disable unstable CSA tests failing on AIX

[Steven Wan via Phabricator via cfe-commits]

stevewan updated this revision to Diff 391120.
stevewan added a comment.
Herald added subscribers: luke957, s.egerton, simoncook, fedor.sergeev, aheejin.

Add targets we are interested in testing


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114454

Files:
  clang/unittests/StaticAnalyzer/CMakeLists.txt
  clang/unittests/StaticAnalyzer/SValTest.cpp


Index: clang/unittests/StaticAnalyzer/SValTest.cpp
===
--- clang/unittests/StaticAnalyzer/SValTest.cpp
+++ clang/unittests/StaticAnalyzer/SValTest.cpp
@@ -21,6 +21,7 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "clang/StaticAnalyzer/Frontend/AnalysisConsumer.h"
 #include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
+#include "clang/Testing/TestClangConfig.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
@@ -91,6 +92,21 @@
   mutable SVals CollectedSVals;
 };
 
+// Fixture class for parameterized SValTest
+class SValTest : public testing::TestWithParam {
+protected:
+  // FIXME: The tests "GetConstType" and "GetLocAsIntType" infer the type of
+  // integrals based on their bitwidth. This is not a reliable method on
+  // platforms where different integrals have the same width.
+  bool skipTest(StringRef TestName) {
+std::string target = GetParam().Target;
+return (target == "powerpc-ibm-aix" || target == "i686-apple-darwin9" ||
+target == "wasm32-unknown-unknown" ||
+target == "wasm64-unknown-unknown") &&
+   (TestName == "GetConstType" || TestName == "GetLocAsIntType");
+  }
+};
+
 // SVAL_TEST is a combined way of providing a short code snippet and
 // to test some programmatic predicates on symbolic values produced by the
 // engine for the actual code.
@@ -135,7 +151,16 @@
 });
\
   }
\

\
-  TEST(SValTest, NAME) { runCheckerOnCode(CODE); }   
\
+  TEST_P(SValTest, NAME) { 
\
+if (skipTest(#NAME)) { 
\
+  std::string target = GetParam().Target;  
\
+  GTEST_SKIP() << "certain integrals have the same bitwidth on "   
\
+   << target;  
\
+  return;  
\
+}  
\
+runCheckerOnCodeWithArgs(
\
+CODE, GetParam().getCommandLineArgs());
\
+  }
\
   void NAME##SValCollector::test(ExprEngine &Engine,   
\
  const ASTContext &Context) const
 
@@ -361,6 +386,31 @@
   EXPECT_EQ(Context.VoidPtrTy, B.getType(Context));
 }
 
+std::vector allTestClangConfigs() {
+  std::vector all_configs;
+  TestClangConfig config;
+  config.Language = Lang_CXX14;
+  for (std::string target :
+   {"i686-pc-windows-msvc",   "i686-apple-darwin9",
+"x86_64-apple-darwin9",   "x86_64-scei-ps4",
+"x86_64-windows-msvc","x86_64-unknown-linux",
+"x86_64-apple-macosx","x86_64-apple-ios14.0",
+"wasm32-unknown-unknown", "wasm64-unknown-unknown",
+"thumb-pc-win32", "sparc64-none-openbsd",
+"sparc-none-none","riscv64-unknown-linux",
+"ppc64-windows-msvc", "powerpc-ibm-aix",
+"powerpc64-ibm-aix",  "s390x-ibm-zos",
+"armv7-pc-windows-msvc",  "aarch64-pc-windows-msvc",
+"xcore-xmos-elf"}) {
+config.Target = target;
+all_configs.push_back(config);
+  }
+  return all_configs;
+}
+
+INSTANTIATE_TEST_SUITE_P(SValTests, SValTest,
+ testing::ValuesIn(allTestClangConfigs()));
+
 } // namespace
 } // namespace ento
 } // namespace clang
Index: clang/unittests/StaticAnalyzer/CMakeLists.txt
===
--- clang/unittests/StaticAnalyzer/CMakeLists.txt
+++ clang/unittests/StaticAnalyzer/CMakeLists.txt
@@ -31,5 +31,6 @@
   clangSerialization
   clangStaticAnalyzerCore
   clangStaticAnalyzerFrontend
+  clangTesting
   clangTooling
   )


Index: clang/unittests/StaticAnalyzer/SValTest.cpp
===
--- clang/unittests/StaticAnalyzer/SValTest.cpp
+++ clang/unittests/StaticAnalyzer/SValTest.cpp
@@ -21,6 +21,7 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "clang/StaticAnalyzer/Frontend

[PATCH] D114902: [Attrs] Elaborate on the trivial_abi documentation

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

LgenerallyGTM!

- I would say "special member functions" instead of "methods"
- instead of "passed indirectly by address", consider saying "passed by hidden 
reference" — is this something people will get, or is it a me-ism? :) There are 
lots of Google hits for "by hidden reference," including apparently some ABI 
docs.
- "mark [...] as safe to //copy// as part of a function call" — Here you want a 
phrase that indicates "copy the bits, but don't 'copy' in the C++ sense of 
calling the copy constructor." One technically correct option would be to say 
"trivially relocate" (you mean "relocate" not "copy", and it's going to be 
trivial because you're just copying the bits). Alternatively, maybe say 
something more verbose about how the object will be seen to "jump" from one 
address to another with no intervening ctor or dtor calls — and this "jump" 
will happen precisely at the point of the call. The object goes from //memory// 
(in the caller's frame), into a //register//, and then back into //memory// 
again (at a new address in the callee's frame). This attribute is safe to use 
only when such "jumps" are unobservable by (or harmless to) the program.  [I 
know //you// know this; I just don't think your wording makes it crystal clear 
to the future //reader// that this is what you mean.]
- 
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p1144r5.html#non-trivial-samples
 contains all the examples of non-trivially-relocatable types I can think of. A 
big one is libc++ `std::list` and `std::map`. You allude to these with the 
words "Objects that register themselves into a doubly linked list", but I think 
that requires eagle eyes on the reader's part. "Objects whose address escapes 
onto the heap ... `std::list`" probably deserves its own dedicated bullet 
point, called out separately from "side table of live instances [...] debug 
iterators."
- Also, objects with vptrs should never be considered trivially relocatable; 
but that's so arcane that I'm happy not to mention it here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114902

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

[PATCH] D114619: [Analyzer][solver] Do not remove the simplified symbol from the eq class

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

@thakis, if the issue is really disturbing and cannot wait until the review of 
D114887  finishes then please do a revert of 
this patch. But then the dependent child patch D103317 
 needs to be reverted as well.
I am trying my best to make D114887  reviewed 
in the next 24 hours.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114619

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

[PATCH] D114649: [libc++] Implement not-yet-voted LWG3436

[Louis Dionne via Phabricator via cfe-commits]

ldionne marked an inline comment as done.
ldionne added a comment.

In D114649#3157040 , @Mordante wrote:

> Since this patch requires both an update to both Clang and libc++ I think it 
> would land the Clang part in a separate patch. Then wait for the CI to have 
> ToT with this version and land that separately.
> For now can you run the CI with the Bootstrap build?

D114649  is the Clang patch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114649

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

[PATCH] D114903: [clang] Support array placement new in constant expressions

[Louis Dionne via Phabricator via cfe-commits]

ldionne created this revision.
ldionne added a reviewer: rsmith.
ldionne requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is necessary for std::construct_at to work on arrays inside a
constant expression.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114903

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp


Index: clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp
===
--- clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp
+++ clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp
@@ -96,14 +96,28 @@
   return a == 42;
 }
 
+void *operator new[](std::size_t, void *p) { return p; }
+constexpr bool no_array_placement_new_in_user_code() { // expected-error 
{{never produces a constant expression}}
+  int a[3];
+  new (&a) int[3](); // expected-note {{call to placement 'operator new[]'}}
+  return a[0] == 0 && a[1] == 0 && a[2] == 0;
+}
+
 namespace std {
   constexpr bool placement_new_in_stdlib() {
 int a;
 new (&a) int(42);
 return a == 42;
   }
+
+  constexpr bool array_placement_new_in_stdlib() {
+int a[3];
+new (&a) int[3]{42, 43, 44};
+return a[0] == 42 && a[1] == 43 && a[2] == 44;
+  }
 }
 static_assert(std::placement_new_in_stdlib());
+static_assert(std::array_placement_new_in_stdlib());
 
 namespace std {
   template
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -9358,8 +9358,7 @@
   bool IsNothrow = false;
   bool IsPlacement = false;
   if (OperatorNew->isReservedGlobalPlacementOperator() &&
-  Info.CurrentCall->isStdFunction() && !E->isArray()) {
-// FIXME Support array placement new.
+  Info.CurrentCall->isStdFunction()) {
 assert(E->getNumPlacementArgs() == 1);
 if (!EvaluatePointer(E->getPlacementArg(0), Result, Info))
   return false;


Index: clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp
===
--- clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp
+++ clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp
@@ -96,14 +96,28 @@
   return a == 42;
 }
 
+void *operator new[](std::size_t, void *p) { return p; }
+constexpr bool no_array_placement_new_in_user_code() { // expected-error {{never produces a constant expression}}
+  int a[3];
+  new (&a) int[3](); // expected-note {{call to placement 'operator new[]'}}
+  return a[0] == 0 && a[1] == 0 && a[2] == 0;
+}
+
 namespace std {
   constexpr bool placement_new_in_stdlib() {
 int a;
 new (&a) int(42);
 return a == 42;
   }
+
+  constexpr bool array_placement_new_in_stdlib() {
+int a[3];
+new (&a) int[3]{42, 43, 44};
+return a[0] == 42 && a[1] == 43 && a[2] == 44;
+  }
 }
 static_assert(std::placement_new_in_stdlib());
+static_assert(std::array_placement_new_in_stdlib());
 
 namespace std {
   template
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -9358,8 +9358,7 @@
   bool IsNothrow = false;
   bool IsPlacement = false;
   if (OperatorNew->isReservedGlobalPlacementOperator() &&
-  Info.CurrentCall->isStdFunction() && !E->isArray()) {
-// FIXME Support array placement new.
+  Info.CurrentCall->isStdFunction()) {
 assert(E->getNumPlacementArgs() == 1);
 if (!EvaluatePointer(E->getPlacementArg(0), Result, Info))
   return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114809: Fix documentation for `forEachLambdaCapture` and `hasAnyCapture`

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM! I had no idea...

(As a follow-up question, should we be removing the `using` declarations 
starting around line 141 or so? Many of them are not used, and if the 
documentation dumper can't look through typedefs and gets the documentation 
wrong, perhaps we want to discourage using them entirely?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114809

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

[PATCH] D114902: [Attrs] Elaborate on the trivial_abi documentation

[Reid Kleckner via Phabricator via cfe-commits]

rnk created this revision.
rnk added reviewers: aaron.ballman, rjmccall, dblaikie, ahatanak.
rnk requested review of this revision.
Herald added a project: clang.

My team recently answered some questions about the trivial_abi
attribute. This change attempts to distill those answers into
user-focused documentation. The previous documentation is somewhat
technical, and the new documentation tries to focus on documenting when
the attribute is safe to use, and what impact the user should expect to
see from the attribute.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114902

Files:
  clang/include/clang/Basic/AttrDocs.td


Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -3197,10 +3197,50 @@
 def TrivialABIDocs : Documentation {
   let Category = DocCatDecl;
   let Content = [{
-The ``trivial_abi`` attribute can be applied to a C++ class, struct, or union.
-It instructs the compiler to pass and return the type using the C ABI for the
-underlying type when the type would otherwise be considered non-trivial for the
-purpose of calls.
+The ``trivial_abi`` attribute helps the compiler pass some C++ objects more
+efficiently.
+
+Normally, the compiler must pass records (classes, structs, or unions) with
+non-trivial copy constructors or destructors very carefully. When these methods
+are provided, the compiler can no longer freely copy the bytes of the object
+when it is passed by value. As a result, such objects are typically passed
+indirectly by address. This is usually less efficient for small objects such as
+``std::unique_ptr`` and other smart pointers. Smart pointers are often
+passed by value in performance-sensitive code, so this attribute exists to
+allow the user to mark a C++ class, struct, or union as safe to copy as part of
+a function call.
+
+Instances of ``trivial_abi`` classes are always destroyed in the callee, and
+not the caller. The destructor is not called on the storage allocated for the
+object in the caller. This ensures that calls to constructors and destructors
+are balanced, and that this attribute can safely be applied to
+reference-counting smart pointers.
+
+It is not safe to apply the ``trivial_abi`` attribute to classes with
+constructors that capture pointers to the object or its subobjects (fields and
+bases). Most constructors do not capture interior object pointers, so this
+attribute can be safely applied to many value types. Two unsafe common use
+cases to look out for are:
+
+1. "Small" objects: The small string optimization often requires taking the
+   address of an array field and storing it into a pointer field. This is
+   unsafe, as the interior pointer becomes stale when the object is copied into
+   the callee.
+2. Self-registration: Objects that register themselves into a doubly linked
+   list or side table of live instances cannot use ``trivial_abi``. This
+   pattern can be used to implement debug iterators, for example.
+
+The ``trivial_abi`` attribute changes the ABI of all functions that pass or
+return objects of annotated types by value, so it is not safe to apply to types
+passed across stable ABI boundaries.
+
+The ``trivial_abi`` attribute does **not** guarantee that the marked object
+will be passed in registers. It only disables the logic that causes objects
+that are non-trivial for the purpose of calls to be passed indirectly. The
+remaining rules of the platform's calling convention often require that large
+records be passed indirectly. This attribute is most beneficial for
+pointer-sized records such as smart pointers.
+
 A class annotated with ``trivial_abi`` can have non-trivial destructors or
 copy/move constructors without automatically becoming non-trivial for the
 purposes of calls. For example:


Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -3197,10 +3197,50 @@
 def TrivialABIDocs : Documentation {
   let Category = DocCatDecl;
   let Content = [{
-The ``trivial_abi`` attribute can be applied to a C++ class, struct, or union.
-It instructs the compiler to pass and return the type using the C ABI for the
-underlying type when the type would otherwise be considered non-trivial for the
-purpose of calls.
+The ``trivial_abi`` attribute helps the compiler pass some C++ objects more
+efficiently.
+
+Normally, the compiler must pass records (classes, structs, or unions) with
+non-trivial copy constructors or destructors very carefully. When these methods
+are provided, the compiler can no longer freely copy the bytes of the object
+when it is passed by value. As a result, such objects are typically passed
+indirectly by address. This is usually less efficient for small objects such as
+``std::unique_ptr`` and other smart pointe

[PATCH] D113828: [clang-tidy] Fix false positives in `fuchsia-trailing-return` check involving deduction guides

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

In D113828#3165101 , @fwolff wrote:

> In D113828#3164016 , @aaron.ballman 
> wrote:
>
>> LGTM! Can you add a release note about the bug fix?
>
> Done. Thank you for the review! Can you also commit it for me? You can use 
> name and email as in `6259016361345e09f0607ef4e037e00bcbe4bd40`. Thanks!

I've commit on your behalf in 844a8d3cecb4cc40e5d9694bcf111518910ea2ff 
, thanks! 
Btw, you might want to consider obtaining your own commit privileges now that 
you have several patches under your belt. For more information on that, see: 
https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access


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

https://reviews.llvm.org/D113828

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

[clang-tools-extra] 844a8d3 - Fix false positives in `fuchsia-trailing-return` check involving deduction guides

[Aaron Ballman via cfe-commits]

Author: Fabian Wolff
Date: 2021-12-01T15:28:01-05:00
New Revision: 844a8d3cecb4cc40e5d9694bcf111518910ea2ff

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

LOG: Fix false positives in `fuchsia-trailing-return` check involving deduction 
guides

Fixes PR#47614. Deduction guides, implicit or user-defined, look like
function declarations in the AST. They aren't really functions, though,
and they always have a trailing return type, so it doesn't make sense
to issue this warning for them.

Added: 


Modified: 
clang-tools-extra/clang-tidy/fuchsia/TrailingReturnCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/fuchsia-trailing-return.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/fuchsia/TrailingReturnCheck.cpp 
b/clang-tools-extra/clang-tidy/fuchsia/TrailingReturnCheck.cpp
index 4ade4767190ef..2902e34fe860f 100644
--- a/clang-tools-extra/clang-tidy/fuchsia/TrailingReturnCheck.cpp
+++ b/clang-tools-extra/clang-tidy/fuchsia/TrailingReturnCheck.cpp
@@ -17,12 +17,6 @@ namespace clang {
 namespace tidy {
 namespace fuchsia {
 
-namespace {
-AST_MATCHER(FunctionDecl, hasTrailingReturn) {
-  return Node.getType()->castAs()->hasTrailingReturn();
-}
-} // namespace
-
 void TrailingReturnCheck::registerMatchers(MatchFinder *Finder) {
   // Functions that have trailing returns are disallowed, except for those
   // using decltype specifiers and lambda with otherwise unutterable
@@ -30,15 +24,16 @@ void TrailingReturnCheck::registerMatchers(MatchFinder 
*Finder) {
   Finder->addMatcher(
   functionDecl(hasTrailingReturn(),
unless(anyOf(returns(decltypeType()),
-hasParent(cxxRecordDecl(isLambda())
+hasParent(cxxRecordDecl(isLambda())),
+cxxDeductionGuideDecl(
   .bind("decl"),
   this);
 }
 
 void TrailingReturnCheck::check(const MatchFinder::MatchResult &Result) {
-  if (const auto *D = Result.Nodes.getNodeAs("decl"))
+  if (const auto *D = Result.Nodes.getNodeAs("decl"))
 diag(D->getBeginLoc(),
- "a trailing return type is disallowed for this type of declaration");
+ "a trailing return type is disallowed for this function declaration");
 }
 
 } // namespace fuchsia

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index fa03d7eac99e0..8991bac7c1f8a 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -140,6 +140,9 @@ Changes in existing checks
   ` to diagnose and fix 
functional
   casts, to achieve feature parity with the corresponding ``cpplint.py`` check.
 
+- Fixed a false positive in :doc:`fuchsia-trailing-return
+  ` for C++17 deduction guides.
+
 Removed checks
 ^^
 

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/fuchsia-trailing-return.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/fuchsia-trailing-return.cpp
index f6c943ac873e7..14c4452e4fdc2 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/fuchsia-trailing-return.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/fuchsia-trailing-return.cpp
@@ -1,9 +1,9 @@
-// RUN: %check_clang_tidy %s fuchsia-trailing-return %t
+// RUN: %check_clang_tidy -std=c++17-or-later %s fuchsia-trailing-return %t
 
 int add_one(const int arg) { return arg; }
 
 auto get_add_one() -> int (*)(const int) {
-  // CHECK-MESSAGES: [[@LINE-1]]:1: warning: a trailing return type is 
disallowed for this type of declaration
+  // CHECK-MESSAGES: [[@LINE-1]]:1: warning: a trailing return type is 
disallowed for this function declaration
   // CHECK-NEXT: auto get_add_one() -> int (*)(const int) {
   return add_one;
 }
@@ -21,3 +21,31 @@ template 
 auto fn(const T1 &lhs, const T2 &rhs) -> decltype(lhs + rhs) {
   return lhs + rhs;
 }
+
+// Now check that implicit and explicit C++17 deduction guides don't trigger 
this warning (PR#47614).
+
+template 
+struct ImplicitDeductionGuides {
+  ImplicitDeductionGuides(const T &);
+};
+
+template 
+struct pair {
+  A first;
+  B second;
+};
+
+template 
+struct UserDefinedDeductionGuides {
+  UserDefinedDeductionGuides(T);
+  template 
+  UserDefinedDeductionGuides(T1, T2);
+};
+
+template 
+UserDefinedDeductionGuides(T1, T2) -> UserDefinedDeductionGuides>;
+
+void foo() {
+  ImplicitDeductionGuides X(42);
+  UserDefinedDeductionGuides s(1, "abc");
+}



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

[PATCH] D114619: [Analyzer][solver] Do not remove the simplified symbol from the eq class

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D114619#3164898 , @thakis wrote:

> The test sometimes fails flakily: http://45.33.8.238/macm1/22813/step_7.txt

Thanks @thakis for the report, I am aware of the issue and addressing that in
https://reviews.llvm.org/D114887


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114619

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

[PATCH] D114887: [Analyzer][solver] Simplification: Do a fixpoint iteration before the eq class merge

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2218
+LLVM_NODISCARD
+static SVal simplifyUntilFixpoint(SValBuilder &SVB, ProgramStateRef State,
+  const SymbolRef Sym) {

Perhaps this should be done in `SimpleSValbuilder::simplifySVal`, I think this 
should be the responsibility of the SValBuilder compoment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114887

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

[PATCH] D113622: [wip] [analyzer] support ignoring use-after-free checking with reference_counted attribute

[Chris D'Angelo via Phabricator via cfe-commits]

chrisdangelo added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1735
+  let Spellings = [Clang<"reference_counted">];
+  let Subjects = SubjectList<[Record]>;
+  let Documentation = [Undocumented];

I've discussed a bit with Devin Coughlin yesterday. Devin would like to be sure 
that we have appropriate warnings showing if this attribute is misused.

Previously, when I have been testing the use of this attribute, I mistakenly 
added the attribute annotation after "typedef" and before "struct". This was 
causing the attribute to not be discovered when the analyzer attempts to 
inspect the declaration. I don't recall if a compiler warning was being emitted.

This note is being written as a reminder to be evaluate what warning support is 
already included or adding support if necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113622

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

[PATCH] D113828: [clang-tidy] Fix false positives in `fuchsia-trailing-return` check involving deduction guides

[Fabian Wolff via Phabricator via cfe-commits]

fwolff updated this revision to Diff 391102.
fwolff added a comment.

In D113828#3164016 , @aaron.ballman 
wrote:

> LGTM! Can you add a release note about the bug fix?

Done. Thank you for the review! Can you also commit it for me? You can use name 
and email as in `6259016361345e09f0607ef4e037e00bcbe4bd40`. Thanks!


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

https://reviews.llvm.org/D113828

Files:
  clang-tools-extra/clang-tidy/fuchsia/TrailingReturnCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/fuchsia-trailing-return.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/fuchsia-trailing-return.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/fuchsia-trailing-return.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/fuchsia-trailing-return.cpp
@@ -1,9 +1,9 @@
-// RUN: %check_clang_tidy %s fuchsia-trailing-return %t
+// RUN: %check_clang_tidy -std=c++17-or-later %s fuchsia-trailing-return %t
 
 int add_one(const int arg) { return arg; }
 
 auto get_add_one() -> int (*)(const int) {
-  // CHECK-MESSAGES: [[@LINE-1]]:1: warning: a trailing return type is 
disallowed for this type of declaration
+  // CHECK-MESSAGES: [[@LINE-1]]:1: warning: a trailing return type is 
disallowed for this function declaration
   // CHECK-NEXT: auto get_add_one() -> int (*)(const int) {
   return add_one;
 }
@@ -21,3 +21,31 @@
 auto fn(const T1 &lhs, const T2 &rhs) -> decltype(lhs + rhs) {
   return lhs + rhs;
 }
+
+// Now check that implicit and explicit C++17 deduction guides don't trigger 
this warning (PR#47614).
+
+template 
+struct ImplicitDeductionGuides {
+  ImplicitDeductionGuides(const T &);
+};
+
+template 
+struct pair {
+  A first;
+  B second;
+};
+
+template 
+struct UserDefinedDeductionGuides {
+  UserDefinedDeductionGuides(T);
+  template 
+  UserDefinedDeductionGuides(T1, T2);
+};
+
+template 
+UserDefinedDeductionGuides(T1, T2) -> UserDefinedDeductionGuides>;
+
+void foo() {
+  ImplicitDeductionGuides X(42);
+  UserDefinedDeductionGuides s(1, "abc");
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -140,6 +140,9 @@
   ` to diagnose and fix 
functional
   casts, to achieve feature parity with the corresponding ``cpplint.py`` check.
 
+- Fixed a false positive in :doc:`fuchsia-trailing-return
+  ` for C++17 deduction guides.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/fuchsia/TrailingReturnCheck.cpp
===
--- clang-tools-extra/clang-tidy/fuchsia/TrailingReturnCheck.cpp
+++ clang-tools-extra/clang-tidy/fuchsia/TrailingReturnCheck.cpp
@@ -17,12 +17,6 @@
 namespace tidy {
 namespace fuchsia {
 
-namespace {
-AST_MATCHER(FunctionDecl, hasTrailingReturn) {
-  return Node.getType()->castAs()->hasTrailingReturn();
-}
-} // namespace
-
 void TrailingReturnCheck::registerMatchers(MatchFinder *Finder) {
   // Functions that have trailing returns are disallowed, except for those
   // using decltype specifiers and lambda with otherwise unutterable
@@ -30,15 +24,16 @@
   Finder->addMatcher(
   functionDecl(hasTrailingReturn(),
unless(anyOf(returns(decltypeType()),
-hasParent(cxxRecordDecl(isLambda())
+hasParent(cxxRecordDecl(isLambda())),
+cxxDeductionGuideDecl(
   .bind("decl"),
   this);
 }
 
 void TrailingReturnCheck::check(const MatchFinder::MatchResult &Result) {
-  if (const auto *D = Result.Nodes.getNodeAs("decl"))
+  if (const auto *D = Result.Nodes.getNodeAs("decl"))
 diag(D->getBeginLoc(),
- "a trailing return type is disallowed for this type of declaration");
+ "a trailing return type is disallowed for this function declaration");
 }
 
 } // namespace fuchsia


Index: clang-tools-extra/test/clang-tidy/checkers/fuchsia-trailing-return.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/fuchsia-trailing-return.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/fuchsia-trailing-return.cpp
@@ -1,9 +1,9 @@
-// RUN: %check_clang_tidy %s fuchsia-trailing-return %t
+// RUN: %check_clang_tidy -std=c++17-or-later %s fuchsia-trailing-return %t
 
 int add_one(const int arg) { return arg; }
 
 auto get_add_one() -> int (*)(const int) {
-  // CHECK-MESSAGES: [[@LINE-1]]:1: warning: a trailing return type is disallowed for this type of declaration
+  // CHECK-MESSAGES: [[@LINE-1]]:1: warning: a trailing return type is disallowed for this function declaration
   // CHECK-NEXT: auto get_add_one() -> int (*)(const int) {
   return add_one;
 }
@@ -21,3 +21,31 @@
 auto fn(cons

[PATCH] D112648: [clang-tidy] Don't offer partial fix-its for `modernize-pass-by-value`

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from a small commenting nit.




Comment at: clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp:194
+  // If we received a `const&` type, we need to rewrite the function
+  // declarations
+  if (ParamDecl->getType()->isLValueReferenceType()) {





Comment at: clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp:198
+for (const ParmVarDecl *ParmDecl : collectParamDecls(Ctor, ParamDecl)) {
+  auto ParamTL = ParmDecl->getTypeSourceInfo()->getTypeLoc();
+  auto RefTL = ParamTL.getAs();

avogelsgesang wrote:
> aaron.ballman wrote:
> > Please spell out this type (per our usual coding conventions). The use on 
> > the next line is fine because the type name is spelled out in the 
> > initialization.
> Thanks for the hint. I wasn't aware of that convention. I fixed it here.
> 
> Should I also fix the other violations in this file?
> 
> E.g., a couple of lines above, we have
> 
> ```
> auto Diag = diag(ParamDecl->getBeginLoc(), "pass by value and use std::move");
> ```
> 
> which I guess should be
> 
> ```
> clang::DiagnosticBuilder Diag = diag(ParamDecl->getBeginLoc(), "pass by value 
> and use std::move");
> ```
> Thanks for the hint. I wasn't aware of that convention.

No problem! In case you haven't seen it, we document our coding style here: 
https://llvm.org/docs/CodingStandards.html

> Should I also fix the other violations in this file?

If you want to, you can do it as an NFC change separate from these changes, but 
I certainly don't insist.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112648

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

[PATCH] D114439: [Annotation] Allow parameter pack expansions in annotate attribute

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

>> I think the safest bet is to be conservative and not allow mixing packs with 
>> variadics, and not allowing multiple packs. We should be able to diagnose 
>> that situation from ClangAttrEmitter.cpp to help attribute authors out. 
>> However, it'd be worth adding a FIXME comment to that diagnostic logic 
>> asking whether we want to relax the behavior at some point. But if you feel 
>> strongly that we should support these situations initially, I wouldn't be 
>> opposed.

From a practical perspective, the only difference between a list of 
`ExprArgument` and `VariadicExprArgument` is the enforced arity I would 
also consider seeing if we can just generalize the pack logic to work for BOTH 
cases.  I'd be all for limiting the pack/ExprArgument list to be the 'last' 
argument to simplify it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114439

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

[PATCH] D102107: [OpenMP] Codegen aggregate for outlined function captures

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

This works approximately as well as trunk does for me, provided D114865 
 is also applied. My baseline is not totally 
solid but I think there's a credible chance this would pass the buildbot, 
provided D114865  went in first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102107

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

[PATCH] D102107: [OpenMP] Codegen aggregate for outlined function captures

[Giorgis Georgakoudis via Phabricator via cfe-commits]

ggeorgakoudis updated this revision to Diff 391099.
ggeorgakoudis added a comment.

Rebase, address comment, update few tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102107

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/AST/ast-dump-openmp-distribute-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-distribute-parallel-for.c
  clang/test/AST/ast-dump-openmp-target-teams-distribute-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-target-teams-distribute-parallel-for.c
  clang/test/AST/ast-dump-openmp-teams-distribute-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-teams-distribute-parallel-for.c
  clang/test/CodeGenCXX/observe-noexcept.cpp
  clang/test/OpenMP/cancel_codegen.cpp
  clang/test/OpenMP/cancellation_point_codegen.cpp
  clang/test/OpenMP/debug-info-complex-byval.cpp
  clang/test/OpenMP/debug-info-openmp-array.cpp
  clang/test/OpenMP/declare_target_codegen_globalization.cpp
  clang/test/OpenMP/declare_variant_construct_codegen_1.c
  clang/test/OpenMP/distribute_codegen.cpp
  clang/test/OpenMP/distribute_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_if_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_num_threads_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_private_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_proc_bind_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_reduction_task_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_if_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_num_threads_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_private_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_proc_bind_codegen.cpp
  clang/test/OpenMP/distribute_private_codegen.cpp
  clang/test/OpenMP/distribute_simd_codegen.cpp
  clang/test/OpenMP/distribute_simd_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_simd_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_simd_private_codegen.cpp
  clang/test/OpenMP/distribute_simd_reduction_codegen.cpp
  clang/test/OpenMP/for_firstprivate_codegen.cpp
  clang/test/OpenMP/for_lastprivate_codegen.cpp
  clang/test/OpenMP/for_linear_codegen.cpp
  clang/test/OpenMP/for_private_codegen.cpp
  clang/test/OpenMP/for_reduction_codegen.cpp
  clang/test/OpenMP/for_reduction_codegen_UDR.cpp
  clang/test/OpenMP/for_reduction_task_codegen.cpp
  clang/test/OpenMP/master_taskloop_in_reduction_codegen.cpp
  clang/test/OpenMP/master_taskloop_simd_in_reduction_codegen.cpp
  clang/test/OpenMP/metadirective_device_kind_codegen.c
  clang/test/OpenMP/metadirective_device_kind_codegen.cpp
  clang/test/OpenMP/metadirective_implementation_codegen.cpp
  clang/test/OpenMP/nvptx_allocate_codegen.cpp
  clang/test/OpenMP/nvptx_data_sharing.cpp
  clang/test/OpenMP/nvptx_distribute_parallel_generic_mode_codegen.cpp
  clang/test/OpenMP/nvptx_lambda_capturing.cpp
  clang/test/OpenMP/nvptx_lambda_pointer_capturing.cpp
  clang/test/OpenMP/nvptx_multi_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_nested_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_parallel_for_codegen.cpp
  clang/test/OpenMP/nvptx_target_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_num_threads_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
  clang/test/OpenMP/nvptx_target_teams_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp
  
clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_generic_mode_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/nvptx_teams_codegen.cpp
  clang/test/OpenMP/nvptx_teams_reduction_codegen.cpp
  clang/test/OpenMP/openmp_win_codegen.cpp
  clang/test/OpenMP/parallel_codegen.cpp
  clang/test/OpenMP/parallel_copyin_codegen.cpp
  clang/test/OpenMP/parallel_firstprivate_codegen.cpp
  clang/test/OpenMP/parallel_for_codegen.cpp
  clang/test/OpenMP/parallel_for_lastprivate_conditional.cpp
  clang/test/OpenMP/parallel_for_linear_codegen.cpp
  clang/test/OpenMP/parallel_for_reduction_task_codegen.cpp
  clang/test/OpenMP/parallel_for_simd_aligned_codegen.cpp
  clang/test/Op

[PATCH] D114439: [Annotation] Allow parameter pack expansions in annotate attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman requested changes to this revision.
aaron.ballman added inline comments.
This revision now requires changes to proceed.



Comment at: clang/include/clang/Basic/Attr.td:544-546
+  // Set to true for attributes that support parameter pack expansion in its
+  // arguments.
+  bit ParmExpansionArgsSupport = 0;

From a design perspective, I think this is a property of the parameters of an 
attribute rather than of an attribute itself. So I'd rather not add a bit to 
the `Attr` class, but instead modify the `Argument` class. There are a few 
approaches to that which I can think of, and I'm not certain which is the best 
approach.

1) Introduce `ExpressionPackArgument` and attributes can opt into using it if 
they want to support packs.
2) Add this bit to `VariadicExprArgument` and attributes can opt into using it 
if they want to support packs.
3) Change `VariadicExprArgument` to also mean "allows packs" and all attributes 
using this automatically support packs.

I think that my conservative preference is for #2, but my intuition is that #3 
is where we probably ultimately want to get to.

There are definitely some open questions with this approach that we'll have to 
figure out:

* Can you mix packs with variadics in the same attribute?
* Can you have two pack arguments in the same attribute?
* Does the tablegen design also seem likely to work when we want to introduce 
type packs instead of value packs?

I think the safest bet is to be conservative and not allow mixing packs with 
variadics, and not allowing multiple packs. We should be able to diagnose that 
situation from ClangAttrEmitter.cpp to help attribute authors out. However, 
it'd be worth adding a FIXME comment to that diagnostic logic asking whether we 
want to relax the behavior at some point. But if you feel strongly that we 
should support these situations initially, I wouldn't be opposed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114439

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

[PATCH] D114890: [OpenMP] Make the new device runtime the default

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

In D114890#3164994 , @JonChesterfield 
wrote:

> In D114890#3164899 , 
> @tianshilei1992 wrote:
>
>> Do we still want to run tests for the old device runtime?
>
> Maybe? We definitely don't want to run the tests for the new one twice

Oh that's definitely right.

I'm thinking we should also disable testing for the old device runtime.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114890

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

[PATCH] D113372: [Driver] Add CLANG_DEFAULT_PIE to emulate GCC --enable-default-pie

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a subscriber: felixonmars.
MaskRay added a comment.

I will wait until Dec 14 before committing.

Hope Arch Linux folks can test this, too: @felixonmars @foutrelis


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113372

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

[PATCH] D113372: [Driver] Add CLANG_DEFAULT_PIE to emulate GCC --enable-default-pie

[Sam James via Phabricator via cfe-commits]

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

This is working well here on Gentoo.

This brings some long-desired feature parity to Clang which we've been wanting 
downstream: GCC has had this option for quite a few years (after pushing from 
various downstreams, including us) and this ends up biting folks who want to 
try migrate to Clang as their system compiler.

The change is rather conservative given we're defaulting to PIE being off for 
now (and this is the same as GCC anyway).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113372

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

[PATCH] D114890: [OpenMP] Make the new device runtime the default

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

In D114890#3164899 , @tianshilei1992 
wrote:

> Do we still want to run tests for the old device runtime?

Maybe? We definitely don't want to run the tests for the new one twice


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114890

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

[PATCH] D114859: [clang-format] Add better support for co-routinues

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:22727
+
+TEST_F(FormatTest, CoRoutineawait) {
+  verifyFormat("int x = co_await foo();");

MyDeveloperDay wrote:
> Quuxplusone wrote:
> > 
> naming of the tests is to allow easy running of all CoRoutine tests
> 
> `./FormatTests --gtest_filter=*CoRoutine*`
That's a good rationale for consistency, but the English/C++ word is still 
"coroutine", not "CoRoutine." If there are other places that need changing, 
maybe it makes sense to mass-rename in a separate commit. The main thing I'm 
saying is "CoRoutine" is universally wrong. :)

(I also don't see why `*Coroutine*` should be a more useful dimension to want 
to filter on, than `*Cpp20*` or `*Keywords*` or `*Unary*` or whatever. I'd also 
be surprised if clang-format's tests ever run slow enough that filtering is 
//desired//. But I don't know and will defer to you-or-whoever.)


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

https://reviews.llvm.org/D114859

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

[PATCH] D114848: [Analysis] Ignore casts and unary ops for uninitialized values

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 391086.
void marked an inline comment as done.
void added a comment.

Pretend that I've spoken English for most of my lyfe.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114848

Files:
  clang/lib/Analysis/UninitializedValues.cpp
  clang/test/Analysis/uninit-asm-goto.cpp


Index: clang/test/Analysis/uninit-asm-goto.cpp
===
--- clang/test/Analysis/uninit-asm-goto.cpp
+++ clang/test/Analysis/uninit-asm-goto.cpp
@@ -57,3 +57,15 @@
 indirect:
   return -2;
 }
+
+// test6: Expect no diagnostics.
+int test6(unsigned int *x) {
+  unsigned int val;
+
+  // See through casts and unary operators.
+  asm goto("nop" : "=r" (*(unsigned int *)(&val)) ::: indirect);
+  *x = val;
+  return 0;
+indirect:
+  return -1;
+}
Index: clang/lib/Analysis/UninitializedValues.cpp
===
--- clang/lib/Analysis/UninitializedValues.cpp
+++ clang/lib/Analysis/UninitializedValues.cpp
@@ -591,8 +591,8 @@
 
 if (AtPredExit == MayUninitialized) {
   // If the predecessor's terminator is an "asm goto" that initializes
-  // the variable, then it won't be counted as "initialized" on the
-  // non-fallthrough paths.
+  // the variable, then don't count it as "initialized" on the indirect
+  // paths.
   CFGTerminator term = Pred->getTerminator();
   if (const auto *as = dyn_cast_or_null(term.getStmt())) {
 const CFGBlock *fallthrough = *Pred->succ_begin();
@@ -810,13 +810,22 @@
   if (!as->isAsmGoto())
 return;
 
-  for (const Expr *o : as->outputs())
-if (const VarDecl *VD = findVar(o).getDecl())
+  ASTContext &C = ac.getASTContext();
+  for (const Expr *O : as->outputs()) {
+const Expr *Ex = stripCasts(C, O);
+
+// Strip away any unary operators. Invalid l-values are reported by other
+// semantic analysis passes.
+while (isa(Ex))
+  Ex = stripCasts(C, dyn_cast(Ex)->getSubExpr());
+
+if (const VarDecl *VD = findVar(Ex).getDecl())
   if (vals[VD] != Initialized)
 // If the variable isn't initialized by the time we get here, then we
 // mark it as potentially uninitialized for those cases where it's used
 // on an indirect path, where it's not guaranteed to be defined.
 vals[VD] = MayUninitialized;
+  }
 }
 
 void TransferFunctions::VisitObjCMessageExpr(ObjCMessageExpr *ME) {


Index: clang/test/Analysis/uninit-asm-goto.cpp
===
--- clang/test/Analysis/uninit-asm-goto.cpp
+++ clang/test/Analysis/uninit-asm-goto.cpp
@@ -57,3 +57,15 @@
 indirect:
   return -2;
 }
+
+// test6: Expect no diagnostics.
+int test6(unsigned int *x) {
+  unsigned int val;
+
+  // See through casts and unary operators.
+  asm goto("nop" : "=r" (*(unsigned int *)(&val)) ::: indirect);
+  *x = val;
+  return 0;
+indirect:
+  return -1;
+}
Index: clang/lib/Analysis/UninitializedValues.cpp
===
--- clang/lib/Analysis/UninitializedValues.cpp
+++ clang/lib/Analysis/UninitializedValues.cpp
@@ -591,8 +591,8 @@
 
 if (AtPredExit == MayUninitialized) {
   // If the predecessor's terminator is an "asm goto" that initializes
-  // the variable, then it won't be counted as "initialized" on the
-  // non-fallthrough paths.
+  // the variable, then don't count it as "initialized" on the indirect
+  // paths.
   CFGTerminator term = Pred->getTerminator();
   if (const auto *as = dyn_cast_or_null(term.getStmt())) {
 const CFGBlock *fallthrough = *Pred->succ_begin();
@@ -810,13 +810,22 @@
   if (!as->isAsmGoto())
 return;
 
-  for (const Expr *o : as->outputs())
-if (const VarDecl *VD = findVar(o).getDecl())
+  ASTContext &C = ac.getASTContext();
+  for (const Expr *O : as->outputs()) {
+const Expr *Ex = stripCasts(C, O);
+
+// Strip away any unary operators. Invalid l-values are reported by other
+// semantic analysis passes.
+while (isa(Ex))
+  Ex = stripCasts(C, dyn_cast(Ex)->getSubExpr());
+
+if (const VarDecl *VD = findVar(Ex).getDecl())
   if (vals[VD] != Initialized)
 // If the variable isn't initialized by the time we get here, then we
 // mark it as potentially uninitialized for those cases where it's used
 // on an indirect path, where it's not guaranteed to be defined.
 vals[VD] = MayUninitialized;
+  }
 }
 
 void TransferFunctions::VisitObjCMessageExpr(ObjCMessageExpr *ME) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114848: [Analysis] Ignore casts and unary ops for uninitialized values

[Bill Wendling via Phabricator via cfe-commits]

void added inline comments.



Comment at: clang/lib/Analysis/UninitializedValues.cpp:594
   // If the predecessor's terminator is an "asm goto" that initializes
-  // the variable, then it won't be counted as "initialized" on the
-  // non-fallthrough paths.
+  // the variable, then it's don't count it as "initialized" on the
+  // indirect paths.

nathanchance wrote:
> Should "it's don't count" be "don't count"?
You does not like mine English? :-P


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114848

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

[PATCH] D114890: [OpenMP] Make the new device runtime the default

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

In D114890#3164885 , @JonChesterfield 
wrote:

> D114891  enables this for the amdgpu tests.
>
> This patch will leave the nvptx tests running on the new runtime twice, and 
> not on the old runtime at all, I think. lit.cfg should probably use old + new 
> explicitly, instead of default + new

Do we still want to run tests for the old device runtime?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114890

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

[PATCH] D114619: [Analyzer][solver] Do not remove the simplified symbol from the eq class

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

The test sometimes fails flakily: http://45.33.8.238/macm1/22813/step_7.txt


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114619

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

[PATCH] D114890: [OpenMP] Make the new device runtime the default

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

D114891  enables this for the amdgpu tests.

This patch will leave the nvptx tests running on the new runtime twice, and not 
on the old runtime at all, I think. lit.cfg should probably use old + new 
explicitly, instead of default + new


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114890

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

[PATCH] D114865: [AMDGPU][OpenMP] Use -amdgpu-fixed-function-abi

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

Run a bunch of tests locally. This patch or something equivalent is a 
precondition on using the new device runtime on amdgpu, which we are very much 
out of time on (see D114890  which will break 
us as soon as it lands without this)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114865

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

[PATCH] D114890: [OpenMP] Make the new device runtime the default

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 391079.
jhuber6 added a comment.

Fixing driver tests that change with this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114890

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/Cuda.cpp
  clang/test/Driver/amdgpu-openmp-toolchain.c
  clang/test/Driver/openmp-offload-gpu.c

Index: clang/test/Driver/openmp-offload-gpu.c
===
--- clang/test/Driver/openmp-offload-gpu.c
+++ clang/test/Driver/openmp-offload-gpu.c
@@ -162,8 +162,8 @@
 // RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda \
 // RUN:   --libomptarget-nvptx-bc-path=%S/Inputs/libomptarget \
 // RUN:   -Xopenmp-target -march=sm_35 --cuda-path=%S/Inputs/CUDA_102/usr/local/cuda \
-// RUN:   -fopenmp-relocatable-target -save-temps -no-canonical-prefixes %s 2>&1 \
-// RUN:   | FileCheck -check-prefix=CHK-BCLIB-DIR %s
+// RUN:   -fopenmp-relocatable-target -fno-openmp-target-new-runtime -save-temps \
+// RUN:   -no-canonical-prefixes %s 2>&1 | FileCheck -check-prefix=CHK-BCLIB-DIR %s
 
 /// Check with the new runtime enabled
 // RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda \
@@ -185,8 +185,8 @@
 /// Create a bogus bitcode library and find it with LIBRARY_PATH
 // RUN:   env LIBRARY_PATH=%S/Inputs/libomptarget/subdir %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda \
 // RUN:   -Xopenmp-target -march=sm_35 --cuda-path=%S/Inputs/CUDA_102/usr/local/cuda \
-// RUN:   -fopenmp-relocatable-target -save-temps -no-canonical-prefixes %s 2>&1 \
-// RUN:   | FileCheck -check-prefix=CHK-ENV-BCLIB %s
+// RUN:   -fopenmp-relocatable-target -fno-openmp-target-new-runtime -save-temps \
+// RUN:   -no-canonical-prefixes %s 2>&1 | FileCheck -check-prefix=CHK-ENV-BCLIB %s
 
 // CHK-BCLIB: clang{{.*}}-triple{{.*}}nvptx64-nvidia-cuda{{.*}}-mlink-builtin-bitcode{{.*}}libomptarget-nvptx-test.bc
 // CHK-BCLIB-DIR: clang{{.*}}-triple{{.*}}nvptx64-nvidia-cuda{{.*}}-mlink-builtin-bitcode{{.*}}libomptarget{{/|}}libomptarget-nvptx-sm_35.bc
@@ -204,7 +204,7 @@
 // RUN:   -fopenmp-relocatable-target -save-temps -no-canonical-prefixes %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHK-BCLIB-WARN %s
 
-// CHK-BCLIB-WARN: no library 'libomptarget-nvptx-sm_35.bc' found in the default clang lib directory or in LIBRARY_PATH; use '--libomptarget-nvptx-bc-path' to specify nvptx bitcode library
+// CHK-BCLIB-WARN: no library 'libomptarget-new-nvptx-sm_35.bc' found in the default clang lib directory or in LIBRARY_PATH; use '--libomptarget-nvptx-bc-path' to specify nvptx bitcode library
 
 /// ###
 
Index: clang/test/Driver/amdgpu-openmp-toolchain.c
===
--- clang/test/Driver/amdgpu-openmp-toolchain.c
+++ clang/test/Driver/amdgpu-openmp-toolchain.c
@@ -1,6 +1,6 @@
 // REQUIRES: x86-registered-target
 // REQUIRES: amdgpu-registered-target
-// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa -Xopenmp-target=amdgcn-amd-amdhsa -march=gfx906 --libomptarget-amdgcn-bc-path=%S/Inputs/hip_dev_lib %s 2>&1 \
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa -fno-openmp-target-new-runtime -Xopenmp-target=amdgcn-amd-amdhsa -march=gfx906 --libomptarget-amdgcn-bc-path=%S/Inputs/hip_dev_lib %s 2>&1 \
 // RUN:   | FileCheck %s
 
 // verify the tools invocations
@@ -14,7 +14,7 @@
 // CHECK: clang{{.*}}"-cc1" "-triple" "x86_64-unknown-linux-gnu"{{.*}}"-o" "{{.*}}a-{{.*}}.o" "-x" "ir" "{{.*}}a-{{.*}}.bc"
 // CHECK: ld{{.*}}"-o" "a.out"{{.*}}"{{.*}}amdgpu-openmp-toolchain-{{.*}}.o" "{{.*}}a-{{.*}}.o" "-lomp" "-lomptarget"
 
-// RUN:   %clang -ccc-print-phases --target=x86_64-unknown-linux-gnu -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa -Xopenmp-target=amdgcn-amd-amdhsa -march=gfx906 %s 2>&1 \
+// RUN:   %clang -ccc-print-phases --target=x86_64-unknown-linux-gnu -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa -fno-openmp-target-new-runtime -Xopenmp-target=amdgcn-amd-amdhsa -march=gfx906 %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-PHASES %s
 // phases
 // CHECK-PHASES: 0: input, "{{.*}}amdgpu-openmp-toolchain.c", c, (host-openmp)
@@ -36,13 +36,13 @@
 // CHECK-PHASES: 16: linker, {4, 15}, image, (host-openmp)
 
 // handling of --libomptarget-amdgcn-bc-path
-// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa -Xopenmp-target=amdgcn-amd-amdhsa -march=gfx803 --libomptarget-amdgcn-bc-path=%S/Inputs/hip_dev_lib/libomptarget-amdgcn-gfx803.bc %s 2>&1 | FileCheck %s --check-prefix=CHECK-LIBOMPTARGET
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp -fopenmp-targe

[PATCH] D114025: [clang][NFC] Inclusive terms: replace some uses of sanity in clang

[Zarko Todorovski via Phabricator via cfe-commits]

ZarkoCA added a comment.

In D114025#3162140 , @rjmccall wrote:

> Those all look good, thanks.

Committed here 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114025

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

[PATCH] D114890: [OpenMP] Make the new device runtime the default

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

That's quite the change! I think it's about time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114890

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

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

[Michael Benfield via Phabricator via cfe-commits]

mbenfield updated this revision to Diff 391076.
mbenfield marked 8 inline comments as done.
mbenfield added a comment.

Replace DeclFD->getName() with just DeclFD.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112024

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ParsedAttr.h
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Sema/attr-diagnose-as-builtin.c

Index: clang/test/Sema/attr-diagnose-as-builtin.c
===
--- /dev/null
+++ clang/test/Sema/attr-diagnose-as-builtin.c
@@ -0,0 +1,118 @@
+// RUN: %clang_cc1 -Wfortify-source -triple x86_64-apple-macosx10.14.0 %s -verify
+// RUN: %clang_cc1 -Wfortify-source -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify
+
+typedef unsigned long size_t;
+
+__attribute__((diagnose_as_builtin(__builtin_memcpy, 3, 1, 2))) int x; // expected-warning {{'diagnose_as_builtin' attribute only applies to functions}}
+
+void *test_memcpy(const void *src, size_t c, void *dst) __attribute__((diagnose_as_builtin(__builtin_memcpy, 3, 1, 2))) {
+  return __builtin_memcpy(dst, src, c);
+}
+
+void call_test_memcpy() {
+  char bufferA[10];
+  char bufferB[11];
+  test_memcpy(bufferB, 10, bufferA);
+  test_memcpy(bufferB, 11, bufferA); // expected-warning {{'memcpy' will always overflow; destination buffer has size 10, but size argument is 11}}
+}
+
+void failure_function_doesnt_exist() __attribute__((diagnose_as_builtin(__function_that_doesnt_exist))) {} // expected-error {{use of undeclared identifier '__function_that_doesnt_exist'}}
+
+void not_a_builtin() {}
+
+void failure_not_a_builtin() __attribute__((diagnose_as_builtin(not_a_builtin))) {} // expected-error {{'diagnose_as_builtin' attribute requires parameter 1 to be a builtin function}}
+
+void failure_too_many_parameters(void *dst, const void *src, size_t count, size_t nothing) __attribute__((diagnose_as_builtin(__builtin_memcpy, 1, 2, 3, 4))) {} // expected-error {{'diagnose_as_builtin' attribute references function '__builtin_memcpy', which takes exactly 3 arguments}}
+
+void failure_too_few_parameters(void *dst, const void *src) __attribute__((diagnose_as_builtin(__builtin_memcpy, 1, 2))) {} // expected-error{{'diagnose_as_builtin' attribute references function '__builtin_memcpy', which takes exactly 3 arguments}}
+
+void failure_not_an_integer(void *dst, const void *src, size_t count) __attribute__((diagnose_as_builtin(__builtin_memcpy, "abc", 2, 3))) {} // expected-error{{'diagnose_as_builtin' attribute requires parameter 2 to be an integer constant}}
+
+void failure_not_a_builtin2() __attribute__((diagnose_as_builtin("abc"))) {} // expected-error{{'diagnose_as_builtin' attribute requires parameter 1 to be a builtin function}}
+
+void failure_parameter_index_bounds(void *dst, const void *src) __attribute__((diagnose_as_builtin(__builtin_memcpy, 1, 2, 3))) {} // expected-error{{'diagnose_as_builtin' attribute references parameter 3, but the function 'failure_parameter_index_bounds' has only 2 parameters}}
+
+void failure_parameter_types(double dst, const void *src, size_t count) __attribute__((diagnose_as_builtin(__builtin_memcpy, 1, 2, 3))) {} // expected-error{{'diagnose_as_builtin' attribute parameter types do not match: parameter 1 of function 'failure_parameter_types' has type 'double', but parameter 1 of function '__builtin_memcpy' has type 'void *'}}
+
+void to_redeclare(void *dst, const void *src, size_t count);
+
+void use_to_redeclare() {
+  char src[10];
+  char dst[9];
+  // We shouldn't get an error yet.
+  to_redeclare(dst, src, 10);
+}
+
+void to_redeclare(void *dst, const void *src, size_t count) __attribute((diagnose_as_builtin(__builtin_memcpy, 1, 2, 3)));
+
+void error_to_redeclare() {
+  char src[10];
+  char dst[9];
+  // Now we get an error.
+  to_redeclare(dst, src, 10); // expected-warning {{'memcpy' will always overflow; destination buffer has size 9, but size argument is 10}}
+}
+
+// Make sure this works even with extra qualifiers and the pass_object_size attribute.
+void *memcpy2(void *const dst __attribute__((pass_object_size(0))), const void *src, size_t copy_amount) __attribute((diagnose_as_builtin(__builtin_memcpy, 1, 2, 3)));
+
+void call_memcpy2() {
+  char buf1[10];
+  char buf2[11];
+  memcpy2(buf1, buf2, 11); // expected-warning {{'memcpy' will always overflow; destination buffer has size 10, but size argument is 11}}
+}
+
+// We require the types to be identical, modulo canonicalization and qualifiers.
+// Maybe this could be relaxed if it proves too restrictive.
+void failure_type(void *dest, char val, size_t n) __attribute__((diagnose_as_builtin(__builtin_memset, 1, 2, 3))) {} // expected-error {{'diagnose_as_builtin' attribute parameter types do not match: parameter 2 of function 'failure_type'

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

[Adrian Vogelsgesang via Phabricator via cfe-commits]

avogelsgesang marked 2 inline comments as done.
avogelsgesang added a comment.

Thanks for the instructions on how to build the documentation!
I fixed a build issue in the docs (incorrect length of the table footer) and 
updated the wording slightly

I am not planning any additional changes. If there is anything else you would 
want me to change, please let me know.
Otherwise, in case you approve this change, can you please also merge it on my 
behalf? I do not have commit access


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112646

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

[clang] 3ee685f - [NFC][Clang] Fix some comments in clang

[Zarko Todorovski via cfe-commits]

Author: Zarko Todorovski
Date: 2021-12-01T13:36:46-05:00
New Revision: 3ee685f98abfc074419371b372681b56f7fd1a37

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

LOG: [NFC][Clang] Fix some comments in clang

Applying post commit comment suggestions from https://reviews.llvm.org/D114025

Added: 


Modified: 
clang/include/clang/Analysis/CFG.h
clang/lib/Sema/SemaChecking.cpp
clang/lib/Sema/SemaExprCXX.cpp

Removed: 




diff  --git a/clang/include/clang/Analysis/CFG.h 
b/clang/include/clang/Analysis/CFG.h
index 3b9b22e87f35c..b8e453fcc235a 100644
--- a/clang/include/clang/Analysis/CFG.h
+++ b/clang/include/clang/Analysis/CFG.h
@@ -515,7 +515,7 @@ class CFGTerminator {
 /// of the most derived class while we're in the base class.
 VirtualBaseBranch,
 
-/// Number of 
diff erent kinds, for validity checks. We subtract 1 so that
+/// Number of 
diff erent kinds, for assertions. We subtract 1 so that
 /// to keep receiving compiler warnings when we don't cover all enum values
 /// in a switch.
 NumKindsMinusOne = VirtualBaseBranch

diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index ee06425dc654a..33e2b3b5027d4 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -5546,8 +5546,8 @@ ExprResult Sema::BuildAtomicExpr(SourceRange CallRange, 
SourceRange ExprRange,
 
   // For an arithmetic operation, the implied arithmetic must be well-formed.
   if (Form == Arithmetic) {
-// GCC does not enforce these rules for GNU atomics, but we do, because if
-// we didn't it would be very confusing. FIXME:  For whom? How so?
+// GCC does not enforce these rules for GNU atomics, but we do to help 
catch
+// trivial type errors.
 auto IsAllowedValueType = [&](QualType ValType) {
   if (ValType->isIntegerType())
 return true;
@@ -5588,8 +5588,9 @@ ExprResult Sema::BuildAtomicExpr(SourceRange CallRange, 
SourceRange ExprRange,
   if (!IsC11 && !AtomTy.isTriviallyCopyableType(Context) &&
   !AtomTy->isScalarType()) {
 // For GNU atomics, require a trivially-copyable type. This is not part of
-// the GNU atomics specification, but we enforce it, because if we didn't 
it
-// would be very confusing. FIXME:  For whom? How so?
+// the GNU atomics specification but we enforce it for consistency with
+// other atomics which generally all require a trivially-copyable type. 
This
+// is because atomics just copy bits.
 Diag(ExprRange.getBegin(), diag::err_atomic_op_needs_trivial_copy)
 << Ptr->getType() << Ptr->getSourceRange();
 return ExprError();

diff  --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 6352525845620..d25f329f85e45 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -1508,8 +1508,9 @@ Sema::BuildCXXTypeConstructExpr(TypeSourceInfo *TInfo,
   }
 
   // Only construct objects with object types.
-  // There doesn't seem to be an explicit rule for this but functions are
-  // not objects, so they cannot take initializers.
+  // The standard doesn't explicitly forbid function types here, but that's an
+  // obvious oversight, as there's no way to dynamically construct a function
+  // in general.
   if (Ty->isFunctionType())
 return ExprError(Diag(TyBeginLoc, diag::err_init_for_function_type)
<< Ty << FullRange);



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

[PATCH] D112646: [clang-tidy] Add `readability-container-contains` check

[Adrian Vogelsgesang via Phabricator via cfe-commits]

avogelsgesang updated this revision to Diff 391075.
avogelsgesang added a comment.

Fix build error in documentation and update wording


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112646

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.cpp
  clang-tools-extra/clang-tidy/readability/ContainerContainsCheck.h
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-container-contains.rst
  clang-tools-extra/test/clang-tidy/checkers/readability-container-contains.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-container-contains.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-container-contains.cpp
@@ -0,0 +1,196 @@
+// RUN: %check_clang_tidy -std=c++20 %s readability-container-contains %t
+
+// Some *very* simplified versions of `map` etc.
+namespace std {
+
+template 
+struct map {
+  unsigned count(const Key &K) const;
+  bool contains(const Key &K) const;
+  void *find(const Key &K);
+  void *end();
+};
+
+template 
+struct set {
+  unsigned count(const Key &K) const;
+  bool contains(const Key &K) const;
+};
+
+template 
+struct unordered_set {
+  unsigned count(const Key &K) const;
+  bool contains(const Key &K) const;
+};
+
+template 
+struct multimap {
+  unsigned count(const Key &K) const;
+  bool contains(const Key &K) const;
+};
+
+} // namespace std
+
+// Check that we detect various common ways to check for membership
+int testDifferentCheckTypes(std::map &MyMap) {
+  if (MyMap.count(0))
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use 'contains' to check for membership [readability-container-contains]
+// CHECK-FIXES: if (MyMap.contains(0))
+return 1;
+  bool C1 = MyMap.count(1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: bool C1 = MyMap.contains(1);
+  auto C2 = static_cast(MyMap.count(1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:37: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C2 = static_cast(MyMap.contains(1));
+  auto C3 = MyMap.count(2) != 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C3 = MyMap.contains(2);
+  auto C4 = MyMap.count(3) > 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C4 = MyMap.contains(3);
+  auto C5 = MyMap.count(4) >= 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C5 = MyMap.contains(4);
+  auto C6 = MyMap.find(5) != MyMap.end();
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C6 = MyMap.contains(5);
+  return C1 + C2 + C3 + C4 + C5 + C6;
+}
+
+// Check that we detect various common ways to check for non-membership
+int testNegativeChecks(std::map &MyMap) {
+  bool C1 = !MyMap.count(-1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: bool C1 = !MyMap.contains(-1);
+  auto C2 = MyMap.count(-2) == 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C2 = !MyMap.contains(-2);
+  auto C3 = MyMap.count(-3) <= 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C3 = !MyMap.contains(-3);
+  auto C4 = MyMap.count(-4) < 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C4 = !MyMap.contains(-4);
+  auto C5 = MyMap.find(-5) == MyMap.end();
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: auto C5 = !MyMap.contains(-5);
+  return C1 + C2 + C3 + C4 + C5;
+}
+
+// Check for various types
+int testDifferentTypes(std::map &M, std::unordered_set &US, std::set &S, std::multimap &MM) {
+  bool C1 = M.count(1001);
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES: bool C1 = M.contains(1001);
+  bool C2 = US.count(1002);
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: use 'contains' to check for membership [readability-container-contains]
+  // CHECK-FIXES

[PATCH] D114890: [OpenMP] Make the new device runtime the default

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 created this revision.
jhuber6 added reviewers: jdoerfert, JonChesterfield, tianshilei1992.
Herald added subscribers: dang, kerbowa, guansong, yaxunl, nhaehnle, jvesely.
jhuber6 requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

This patch changes the `-fopenmp-target-new-runtime` option which controls if
the new or old device runtime is used to be true by default.  Disabling this to
use the old runtime now requires using `-fno-openmp-target-new-runtime`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114890

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/Cuda.cpp


Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -745,7 +745,7 @@
 
 std::string BitcodeSuffix;
 if (DriverArgs.hasFlag(options::OPT_fopenmp_target_new_runtime,
-   options::OPT_fno_openmp_target_new_runtime, false))
+   options::OPT_fno_openmp_target_new_runtime, true))
   BitcodeSuffix = "new-nvptx-" + GpuArch.str();
 else
   BitcodeSuffix = "nvptx-" + GpuArch.str();
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5904,7 +5904,7 @@
   // runtime.
   if (Args.hasFlag(options::OPT_fopenmp_target_new_runtime,
options::OPT_fno_openmp_target_new_runtime,
-   /*Default=*/false))
+   /*Default=*/true))
 CmdArgs.push_back("-fopenmp-target-new-runtime");
 
   // When in OpenMP offloading mode, enable debugging on the device.
Index: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
+++ clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
@@ -267,7 +267,7 @@
 
   std::string BitcodeSuffix;
   if (DriverArgs.hasFlag(options::OPT_fopenmp_target_new_runtime,
- options::OPT_fno_openmp_target_new_runtime, false))
+ options::OPT_fno_openmp_target_new_runtime, true))
 BitcodeSuffix = "new-amdgpu-" + GPUArch;
   else
 BitcodeSuffix = "amdgcn-" + GPUArch;
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2442,7 +2442,7 @@
 def fno_openmp_assume_threads_oversubscription : Flag<["-"], 
"fno-openmp-assume-threads-oversubscription">, 
   Group, Flags<[CC1Option, NoArgumentUnused, HelpHidden]>;
 defm openmp_target_new_runtime: BoolFOption<"openmp-target-new-runtime",
-  LangOpts<"OpenMPTargetNewRuntime">, DefaultFalse,
+  LangOpts<"OpenMPTargetNewRuntime">, DefaultTrue,
   PosFlag,
   NegFlag>;
 defm openmp_optimistic_collapse : BoolFOption<"openmp-optimistic-collapse",


Index: clang/lib/Driver/ToolChains/Cuda.cpp
===
--- clang/lib/Driver/ToolChains/Cuda.cpp
+++ clang/lib/Driver/ToolChains/Cuda.cpp
@@ -745,7 +745,7 @@
 
 std::string BitcodeSuffix;
 if (DriverArgs.hasFlag(options::OPT_fopenmp_target_new_runtime,
-   options::OPT_fno_openmp_target_new_runtime, false))
+   options::OPT_fno_openmp_target_new_runtime, true))
   BitcodeSuffix = "new-nvptx-" + GpuArch.str();
 else
   BitcodeSuffix = "nvptx-" + GpuArch.str();
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5904,7 +5904,7 @@
   // runtime.
   if (Args.hasFlag(options::OPT_fopenmp_target_new_runtime,
options::OPT_fno_openmp_target_new_runtime,
-   /*Default=*/false))
+   /*Default=*/true))
 CmdArgs.push_back("-fopenmp-target-new-runtime");
 
   // When in OpenMP offloading mode, enable debugging on the device.
Index: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
+++ clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
@@ -267,7 +267,7 @@
 
   std::string BitcodeSuffix;
   if (DriverArgs.hasFlag(options::OPT_fopenmp_target_new_runtime,
- options::OPT_fno_openmp_target_new_runtime, false))
+ options::OPT_fno_openmp_target_new_runtime, true))
 BitcodeSuffix = "new-amdgpu-" + GPUArch;
   else
 BitcodeSuffix = "amdgcn-" + GPUArch;
Index

[PATCH] D113749: [Clang] Fix nesting of discarded and immediate contexts.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

I've commit on your behalf in 6eeda06c1d22da2b9fe96a2569a8a0f8e4f36880 
, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113749

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

[PATCH] D102107: [OpenMP] Codegen aggregate for outlined function captures

[Alexey Bataev via Phabricator via cfe-commits]

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

LG wit a nit




Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:1567
+} else
+  AggregatePtr = llvm::Constant::getNullValue(OMPBuilder.VoidPtr);
 

Enclose into braces too


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102107

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

[PATCH] D102107: [OpenMP] Codegen aggregate for outlined function captures

[Giorgis Georgakoudis via Phabricator via cfe-commits]

ggeorgakoudis added a comment.

Ping!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102107

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

[clang] 6eeda06 - [Clang] Fix nesting of discarded and immediate contexts.

[Aaron Ballman via cfe-commits]

Author: Corentin Jabot
Date: 2021-12-01T12:58:32-05:00
New Revision: 6eeda06c1d22da2b9fe96a2569a8a0f8e4f36880

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

LOG: [Clang] Fix nesting of discarded and immediate contexts.

In C++23, discarded statements and if consteval statements can nest
arbitrarily. To support that, we keep track of whether the parent of
the current evaluation context is discarded or immediate.

This is done at the construction of an evaluation context
to improve performance.

Fixes https://bugs.llvm.org/show_bug.cgi?id=52231

Added: 


Modified: 
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaStmt.cpp
clang/test/SemaCXX/cxx2b-consteval-if.cpp

Removed: 




diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index c969d97baccce..1a82a9498d1d3 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -1296,6 +1296,11 @@ class Sema final {
   EK_Decltype, EK_TemplateArgument, EK_Other
 } ExprContext;
 
+// A context can be nested in both a discarded statement context and
+// an immediate function context, so they need to be tracked independently.
+bool InDiscardedStatement;
+bool InImmediateFunctionContext;
+
 ExpressionEvaluationContextRecord(ExpressionEvaluationContext Context,
   unsigned NumCleanupObjects,
   CleanupInfo ParentCleanup,
@@ -1303,7 +1308,8 @@ class Sema final {
   ExpressionKind ExprContext)
 : Context(Context), ParentCleanup(ParentCleanup),
   NumCleanupObjects(NumCleanupObjects), NumTypos(0),
-  ManglingContextDecl(ManglingContextDecl), ExprContext(ExprContext) {}
+  ManglingContextDecl(ManglingContextDecl), ExprContext(ExprContext),
+  InDiscardedStatement(false), InImmediateFunctionContext(false) {}
 
 bool isUnevaluated() const {
   return Context == ExpressionEvaluationContext::Unevaluated ||
@@ -1317,7 +1323,13 @@ class Sema final {
 }
 
 bool isImmediateFunctionContext() const {
-  return Context == ExpressionEvaluationContext::ImmediateFunctionContext;
+  return Context == ExpressionEvaluationContext::ImmediateFunctionContext 
||
+ InImmediateFunctionContext;
+}
+
+bool isDiscardedStatementContext() const {
+  return Context == ExpressionEvaluationContext::DiscardedStatement ||
+ InDiscardedStatement;
 }
   };
 
@@ -9154,14 +9166,7 @@ class Sema final {
   bool isImmediateFunctionContext() const {
 assert(!ExprEvalContexts.empty() &&
"Must be in an expression evaluation context");
-for (const ExpressionEvaluationContextRecord &context :
- llvm::reverse(ExprEvalContexts)) {
-  if (context.isImmediateFunctionContext())
-return true;
-  if (context.isUnevaluated())
-return false;
-}
-return false;
+return ExprEvalContexts.back().isImmediateFunctionContext();
   }
 
   /// RAII class used to determine whether SFINAE has

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 8592335e20d31..b305d4e5b92f5 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -16566,6 +16566,17 @@ Sema::PushExpressionEvaluationContext(
 ExpressionEvaluationContextRecord::ExpressionKind ExprContext) {
   ExprEvalContexts.emplace_back(NewContext, ExprCleanupObjects.size(), Cleanup,
 LambdaContextDecl, ExprContext);
+
+  // Discarded statements and immediate contexts nested in other
+  // discarded statements or immediate context are themselves
+  // a discarded statement or an immediate context, respectively.
+  ExprEvalContexts.back().InDiscardedStatement =
+  ExprEvalContexts[ExprEvalContexts.size() - 2]
+  .isDiscardedStatementContext();
+  ExprEvalContexts.back().InImmediateFunctionContext =
+  ExprEvalContexts[ExprEvalContexts.size() - 2]
+  .isImmediateFunctionContext();
+
   Cleanup.reset();
   if (!MaybeODRUseExprs.empty())
 std::swap(MaybeODRUseExprs, ExprEvalContexts.back().SavedMaybeODRUseExprs);
@@ -18965,6 +18976,10 @@ bool Sema::DiagIfReachable(SourceLocation Loc, 
ArrayRef Stmts,
 /// during overload resolution or within sizeof/alignof/typeof/typeid.
 bool Sema::DiagRuntimeBehavior(SourceLocation Loc, ArrayRef Stmts,
const PartialDiagnostic &PD) {
+
+  if (ExprEvalContexts.back().isDiscardedStatementContext())
+return false;
+
   switch (ExprEvalContexts.back().Context) {
   case ExpressionEvaluationContext::Unevaluated:
   case ExpressionEvaluationContext::UnevaluatedList:

diff  --git a/clang/lib/Sem

[libunwind] dc1244d - [runtimes] Move WARNING to FATAL_ERROR for folks using FOO_BUILD_32_BITS

[Louis Dionne via cfe-commits]

Author: Louis Dionne
Date: 2021-12-01T12:57:30-05:00
New Revision: dc1244dc4e76316ab24596545951d3dc47359875

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

LOG: [runtimes] Move WARNING to FATAL_ERROR for folks using FOO_BUILD_32_BITS

Added: 


Modified: 
libcxx/CMakeLists.txt
libcxxabi/CMakeLists.txt
libunwind/CMakeLists.txt

Removed: 




diff  --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index 9670d93e3d981..b28dd00f6159a 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -263,7 +263,7 @@ option(LIBCXXABI_USE_LLVM_UNWINDER "Build and use the LLVM 
unwinder." OFF)
 # Target options --
 option(LIBCXX_BUILD_32_BITS "Build 32 bit multilib libc++. This option is not 
supported anymore when building the runtimes. Please specify a full triple 
instead." ${LLVM_BUILD_32_BITS})
 if (LIBCXX_BUILD_32_BITS)
-  message(WARNING "LIBCXX_BUILD_32_BITS is not supported anymore when building 
the runtimes, please specify a full triple instead.")
+  message(FATAL_ERROR "LIBCXX_BUILD_32_BITS is not supported anymore when 
building the runtimes, please specify a full triple instead.")
 endif()
 
 if(CMAKE_CXX_COMPILER_TARGET)

diff  --git a/libcxxabi/CMakeLists.txt b/libcxxabi/CMakeLists.txt
index 0d513884ed4d7..12bcd2eee0996 100644
--- a/libcxxabi/CMakeLists.txt
+++ b/libcxxabi/CMakeLists.txt
@@ -110,7 +110,7 @@ option(LIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS
which case the definition in libc++abi should be turned off." ON)
 option(LIBCXXABI_BUILD_32_BITS "Build 32 bit multilib libc++abi. This option 
is not supported anymore when building the runtimes. Please specify a full 
triple instead." ${LLVM_BUILD_32_BITS})
 if (LIBCXXABI_BUILD_32_BITS)
-  message(WARNING "LIBCXXABI_BUILD_32_BITS is not supported anymore when 
building the runtimes, please specify a full triple instead.")
+  message(FATAL_ERROR "LIBCXXABI_BUILD_32_BITS is not supported anymore when 
building the runtimes, please specify a full triple instead.")
 endif()
 
 option(LIBCXXABI_INCLUDE_TESTS "Generate build targets for the libc++abi unit 
tests." ${LLVM_INCLUDE_TESTS})

diff  --git a/libunwind/CMakeLists.txt b/libunwind/CMakeLists.txt
index 3727d11598366..a63dc453ffb6c 100644
--- a/libunwind/CMakeLists.txt
+++ b/libunwind/CMakeLists.txt
@@ -59,7 +59,7 @@ include(HandleCompilerRT)
 # Define options.
 option(LIBUNWIND_BUILD_32_BITS "Build 32 bit multilib libunwind. This option 
is not supported anymore when building the runtimes. Please specify a full 
triple instead." ${LLVM_BUILD_32_BITS})
 if (LIBUNWIND_BUILD_32_BITS)
-  message(WARNING "LIBUNWIND_BUILD_32_BITS is not supported anymore when 
building the runtimes, please specify a full triple instead.")
+  message(FATAL_ERROR "LIBUNWIND_BUILD_32_BITS is not supported anymore when 
building the runtimes, please specify a full triple instead.")
 endif()
 
 option(LIBUNWIND_ENABLE_CET "Build libunwind with CET enabled." OFF)



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

[libunwind] fa1c077 - [runtimes] Remove support for GCC-style 32 bit multilib builds

[Louis Dionne via cfe-commits]

Author: Louis Dionne
Date: 2021-12-01T12:57:01-05:00
New Revision: fa1c077b41ae1335332d65399802f2c68e82ca7b

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

LOG: [runtimes] Remove support for GCC-style 32 bit multilib builds

This patch removes the ability to build the runtimes in the 32 bit
multilib configuration, i.e. using -m32. Instead of doing this, one
should cross-compile the runtimes for the appropriate target triple,
like we do for all other triples.

As it stands, -m32 has several issues, which all seem to be related to
the fact that it's not well supported by the operating systems that
libc++ support. The simplest path towards fixing this is to remove
support for the configuration, which is also the best course of action
if there is little interest for keeping that configuration. If there
is a desire to keep this configuration around, we'll need to do some
work to figure out the underlying issues and fix them.

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

Added: 


Modified: 
libcxx/CMakeLists.txt
libcxx/docs/BuildingLibcxx.rst
libcxx/docs/ReleaseNotes.rst
libcxx/test/CMakeLists.txt

libcxx/test/libcxx/input.output/filesystems/class.directory_entry/directory_entry.mods/last_write_time.pass.cpp

libcxx/test/libcxx/thread/thread.condition/PR30202_notify_from_pthread_created_thread.pass.cpp

libcxx/test/libcxx/thread/thread.threads/thread.thread.this/sleep_for.pass.cpp

libcxx/test/libcxx/thread/thread.threads/thread.thread.this/sleep_for.signals.pass.cpp

libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.copy_file/copy_file.pass.cpp

libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp
libcxx/test/std/thread/futures/futures.async/async.pass.cpp
libcxx/test/std/thread/futures/futures.shared_future/wait.pass.cpp
libcxx/test/std/thread/futures/futures.shared_future/wait_for.pass.cpp
libcxx/test/std/thread/futures/futures.shared_future/wait_until.pass.cpp
libcxx/test/std/thread/futures/futures.unique_future/wait.pass.cpp
libcxx/test/std/thread/futures/futures.unique_future/wait_for.pass.cpp
libcxx/test/std/thread/futures/futures.unique_future/wait_until.pass.cpp
libcxx/test/std/thread/thread.barrier/arrive.pass.cpp
libcxx/test/std/thread/thread.barrier/arrive_and_drop.pass.cpp
libcxx/test/std/thread/thread.barrier/arrive_and_wait.pass.cpp
libcxx/test/std/thread/thread.barrier/completion.pass.cpp
libcxx/test/std/thread/thread.condition/notify_all_at_thread_exit.pass.cpp

libcxx/test/std/thread/thread.condition/thread.condition.condvar/notify_all.pass.cpp

libcxx/test/std/thread/thread.condition/thread.condition.condvar/wait_for.pass.cpp

libcxx/test/std/thread/thread.condition/thread.condition.condvar/wait_for_pred.pass.cpp

libcxx/test/std/thread/thread.condition/thread.condition.condvar/wait_until.pass.cpp

libcxx/test/std/thread/thread.condition/thread.condition.condvar/wait_until_pred.pass.cpp

libcxx/test/std/thread/thread.condition/thread.condition.condvarany/notify_one.pass.cpp

libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_for.pass.cpp

libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_for_pred.pass.cpp

libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_until.pass.cpp

libcxx/test/std/thread/thread.condition/thread.condition.condvarany/wait_until_pred.pass.cpp
libcxx/test/std/thread/thread.latch/arrive_and_wait.pass.cpp
libcxx/test/std/thread/thread.latch/count_down.pass.cpp

libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.cons/mutex.pass.cpp

libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.cons/mutex_duration.pass.cpp

libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.cons/mutex_time_point.pass.cpp

libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.cons/mutex_try_to_lock.pass.cpp

libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.locking/lock.pass.cpp

libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.locking/try_lock_until.pass.cpp

libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.unique/thread.lock.unique.cons/mutex.pass.cpp

libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.unique/thread.lock.unique.cons/mutex_duration.pass.cpp

libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.unique/thread.lock.unique.cons/mutex_time_point.pass.cpp

libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.unique/thread.lock.unique.cons/mutex_try_to_lock.pass.cpp

libcxx/test/std/thread/thread.mutex

[PATCH] D113148: Add new clang-tidy check for string_view(nullptr)

[CJ Johnson via Phabricator via cfe-commits]

CJ-Johnson updated this revision to Diff 391068.
CJ-Johnson added a comment.

Add missing comment lines


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113148

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StringviewNullptrCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-stringview-nullptr.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-stringview-nullptr.cpp
@@ -0,0 +1,1438 @@
+// RUN: %check_clang_tidy %s bugprone-stringview-nullptr -std=c++17 %t
+
+namespace std {
+
+using size_t = long long;
+using nullptr_t = decltype(nullptr);
+
+template 
+T &&declval();
+
+template 
+struct type_identity { using type = T; };
+template 
+using type_identity_t = typename type_identity::type;
+
+template 
+class basic_string_view {
+public:
+  basic_string_view();
+
+  basic_string_view(const CharT *);
+
+  // Not present in C++17 and C++20:
+  // basic_string_view(std::nullptr_t);
+
+  basic_string_view(const CharT *, size_t);
+
+  basic_string_view(const basic_string_view &);
+
+  basic_string_view &operator=(const basic_string_view &);
+};
+
+template 
+bool operator<(basic_string_view, basic_string_view);
+template 
+bool operator<(type_identity_t>,
+   basic_string_view);
+template 
+bool operator<(basic_string_view,
+   type_identity_t>);
+
+template 
+bool operator<=(basic_string_view, basic_string_view);
+template 
+bool operator<=(type_identity_t>,
+basic_string_view);
+template 
+bool operator<=(basic_string_view,
+type_identity_t>);
+
+template 
+bool operator>(basic_string_view, basic_string_view);
+template 
+bool operator>(type_identity_t>,
+   basic_string_view);
+template 
+bool operator>(basic_string_view,
+   type_identity_t>);
+
+template 
+bool operator>=(basic_string_view, basic_string_view);
+template 
+bool operator>=(type_identity_t>,
+basic_string_view);
+template 
+bool operator>=(basic_string_view,
+type_identity_t>);
+
+template 
+bool operator==(basic_string_view, basic_string_view);
+template 
+bool operator==(type_identity_t>,
+basic_string_view);
+template 
+bool operator==(basic_string_view,
+type_identity_t>);
+
+template 
+bool operator!=(basic_string_view, basic_string_view);
+template 
+bool operator!=(type_identity_t>,
+basic_string_view);
+template 
+bool operator!=(basic_string_view,
+type_identity_t>);
+
+using string_view = basic_string_view;
+
+} // namespace std
+
+void function(std::string_view);
+void function(std::string_view, std::string_view);
+
+void temporary_construction() /* a */ {
+  // Functional Cast
+  {
+(void)(std::string_view(nullptr)) /* a1 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing basic_string_view from null is undefined; replace with the default constructor
+// CHECK-FIXES: {{^}}(void)(std::string_view()) /* a1 */;
+
+(void)(std::string_view((nullptr))) /* a2 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing{{.*}}default
+// CHECK-FIXES: {{^}}(void)(std::string_view()) /* a2 */;
+
+(void)(std::string_view({nullptr})) /* a3 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing{{.*}}default
+// CHECK-FIXES: {{^}}(void)(std::string_view()) /* a3 */;
+
+(void)(std::string_view({(nullptr)})) /* a4 */;
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing{{.*}}default
+// CHECK-FIXES: {{^}}(void)(std::string_view()) /* a4 */;
+
+(void)(std::string_view({})) /* a5 */; // Default `const CharT*`
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: constructing{{.*}}default
+// CHECK-FIXES: {{^}}(void)(std::string_view()) /* a5 */;
+
+// (void)(const std::string_view(nullptr)) /* a6 */;
+// CV qualifiers do not compile in this context
+
+// (void)(const std::string_view((nullptr))) /* a7 */;
+// CV qualifiers do not compile in this context
+
+// (void)(const std::string_view({nullptr})) /* a8 */;
+// CV qualifiers do not compile in this context
+
+// (void)(const std::string_view({(nullptr)})) /* a9 */;
+// CV qualifiers do not compile in this context
+
+// (void)(const std::string_view({})) /* a10 */; // Default `const CharT*`
+// CV qualifiers do not compile in this context
+  }
+
+  // Temporary Object
+  {
+(

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

[Michael Benfield via Phabricator via cfe-commits]

mbenfield updated this revision to Diff 391067.
mbenfield added a comment.

Revert spurious whitespace change.

assert(D)

No else after return.

Allow attribute to be applied to static member functions and test this.

Document that it can't be applied to a non-static member function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112024

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ParsedAttr.h
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Sema/attr-diagnose-as-builtin.c

Index: clang/test/Sema/attr-diagnose-as-builtin.c
===
--- /dev/null
+++ clang/test/Sema/attr-diagnose-as-builtin.c
@@ -0,0 +1,118 @@
+// RUN: %clang_cc1 -Wfortify-source -triple x86_64-apple-macosx10.14.0 %s -verify
+// RUN: %clang_cc1 -Wfortify-source -xc++ -triple x86_64-apple-macosx10.14.0 %s -verify
+
+typedef unsigned long size_t;
+
+__attribute__((diagnose_as_builtin(__builtin_memcpy, 3, 1, 2))) int x; // expected-warning {{'diagnose_as_builtin' attribute only applies to functions}}
+
+void *test_memcpy(const void *src, size_t c, void *dst) __attribute__((diagnose_as_builtin(__builtin_memcpy, 3, 1, 2))) {
+  return __builtin_memcpy(dst, src, c);
+}
+
+void call_test_memcpy() {
+  char bufferA[10];
+  char bufferB[11];
+  test_memcpy(bufferB, 10, bufferA);
+  test_memcpy(bufferB, 11, bufferA); // expected-warning {{'memcpy' will always overflow; destination buffer has size 10, but size argument is 11}}
+}
+
+void failure_function_doesnt_exist() __attribute__((diagnose_as_builtin(__function_that_doesnt_exist))) {} // expected-error {{use of undeclared identifier '__function_that_doesnt_exist'}}
+
+void not_a_builtin() {}
+
+void failure_not_a_builtin() __attribute__((diagnose_as_builtin(not_a_builtin))) {} // expected-error {{'diagnose_as_builtin' attribute requires parameter 1 to be a builtin function}}
+
+void failure_too_many_parameters(void *dst, const void *src, size_t count, size_t nothing) __attribute__((diagnose_as_builtin(__builtin_memcpy, 1, 2, 3, 4))) {} // expected-error {{'diagnose_as_builtin' attribute references function '__builtin_memcpy', which takes exactly 3 arguments}}
+
+void failure_too_few_parameters(void *dst, const void *src) __attribute__((diagnose_as_builtin(__builtin_memcpy, 1, 2))) {} // expected-error{{'diagnose_as_builtin' attribute references function '__builtin_memcpy', which takes exactly 3 arguments}}
+
+void failure_not_an_integer(void *dst, const void *src, size_t count) __attribute__((diagnose_as_builtin(__builtin_memcpy, "abc", 2, 3))) {} // expected-error{{'diagnose_as_builtin' attribute requires parameter 2 to be an integer constant}}
+
+void failure_not_a_builtin2() __attribute__((diagnose_as_builtin("abc"))) {} // expected-error{{'diagnose_as_builtin' attribute requires parameter 1 to be a builtin function}}
+
+void failure_parameter_index_bounds(void *dst, const void *src) __attribute__((diagnose_as_builtin(__builtin_memcpy, 1, 2, 3))) {} // expected-error{{'diagnose_as_builtin' attribute references parameter 3, but the function failure_parameter_index_bounds has only 2 parameters}}
+
+void failure_parameter_types(double dst, const void *src, size_t count) __attribute__((diagnose_as_builtin(__builtin_memcpy, 1, 2, 3))) {} // expected-error{{'diagnose_as_builtin' attribute parameter types do not match: parameter 1 of function 'failure_parameter_types' has type 'double', but parameter 1 of function '__builtin_memcpy' has type 'void *'}}
+
+void to_redeclare(void *dst, const void *src, size_t count);
+
+void use_to_redeclare() {
+  char src[10];
+  char dst[9];
+  // We shouldn't get an error yet.
+  to_redeclare(dst, src, 10);
+}
+
+void to_redeclare(void *dst, const void *src, size_t count) __attribute((diagnose_as_builtin(__builtin_memcpy, 1, 2, 3)));
+
+void error_to_redeclare() {
+  char src[10];
+  char dst[9];
+  // Now we get an error.
+  to_redeclare(dst, src, 10); // expected-warning {{'memcpy' will always overflow; destination buffer has size 9, but size argument is 10}}
+}
+
+// Make sure this works even with extra qualifiers and the pass_object_size attribute.
+void *memcpy2(void *const dst __attribute__((pass_object_size(0))), const void *src, size_t copy_amount) __attribute((diagnose_as_builtin(__builtin_memcpy, 1, 2, 3)));
+
+void call_memcpy2() {
+  char buf1[10];
+  char buf2[11];
+  memcpy2(buf1, buf2, 11); // expected-warning {{'memcpy' will always overflow; destination buffer has size 10, but size argument is 11}}
+}
+
+// We require the types to be identical, modulo canonicalization and qualifiers.
+// Maybe this could be relaxed if it proves too restrictive.
+void failure_type(void *dest, char val, size_t n) __attribute__((diagnose_as_builtin(__builtin_memset, 1, 2, 3))) {} 

[PATCH] D114842: [lld-macho] Remove old macho darwin lld

[Keith Smiley via Phabricator via cfe-commits]

keith updated this revision to Diff 391065.
keith marked an inline comment as done.
keith added a comment.

Remove lld/lib directory


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114842

Files:
  lld/CMakeLists.txt
  lld/include/lld/Common/Driver.h
  lld/include/lld/Core/Reference.h
  lld/include/lld/ReaderWriter/MachOLinkingContext.h
  lld/include/lld/ReaderWriter/YamlContext.h
  lld/lib/CMakeLists.txt
  lld/lib/Core/CMakeLists.txt
  lld/lib/Core/DefinedAtom.cpp
  lld/lib/Core/Error.cpp
  lld/lib/Core/File.cpp
  lld/lib/Core/LinkingContext.cpp
  lld/lib/Core/Reader.cpp
  lld/lib/Core/Resolver.cpp
  lld/lib/Core/SymbolTable.cpp
  lld/lib/Core/Writer.cpp
  lld/lib/Driver/CMakeLists.txt
  lld/lib/Driver/DarwinLdDriver.cpp
  lld/lib/Driver/DarwinLdOptions.td
  lld/lib/ReaderWriter/CMakeLists.txt
  lld/lib/ReaderWriter/FileArchive.cpp
  lld/lib/ReaderWriter/MachO/ArchHandler.cpp
  lld/lib/ReaderWriter/MachO/ArchHandler.h
  lld/lib/ReaderWriter/MachO/ArchHandler_arm.cpp
  lld/lib/ReaderWriter/MachO/ArchHandler_arm64.cpp
  lld/lib/ReaderWriter/MachO/ArchHandler_x86.cpp
  lld/lib/ReaderWriter/MachO/ArchHandler_x86_64.cpp
  lld/lib/ReaderWriter/MachO/Atoms.h
  lld/lib/ReaderWriter/MachO/CMakeLists.txt
  lld/lib/ReaderWriter/MachO/CompactUnwindPass.cpp
  lld/lib/ReaderWriter/MachO/DebugInfo.h
  lld/lib/ReaderWriter/MachO/ExecutableAtoms.h
  lld/lib/ReaderWriter/MachO/File.h
  lld/lib/ReaderWriter/MachO/FlatNamespaceFile.h
  lld/lib/ReaderWriter/MachO/GOTPass.cpp
  lld/lib/ReaderWriter/MachO/LayoutPass.cpp
  lld/lib/ReaderWriter/MachO/LayoutPass.h
  lld/lib/ReaderWriter/MachO/MachOLinkingContext.cpp
  lld/lib/ReaderWriter/MachO/MachONormalizedFile.h
  lld/lib/ReaderWriter/MachO/MachONormalizedFileBinaryReader.cpp
  lld/lib/ReaderWriter/MachO/MachONormalizedFileBinaryUtils.h
  lld/lib/ReaderWriter/MachO/MachONormalizedFileBinaryWriter.cpp
  lld/lib/ReaderWriter/MachO/MachONormalizedFileFromAtoms.cpp
  lld/lib/ReaderWriter/MachO/MachONormalizedFileToAtoms.cpp
  lld/lib/ReaderWriter/MachO/MachONormalizedFileYAML.cpp
  lld/lib/ReaderWriter/MachO/MachOPasses.h
  lld/lib/ReaderWriter/MachO/ObjCPass.cpp
  lld/lib/ReaderWriter/MachO/SectCreateFile.h
  lld/lib/ReaderWriter/MachO/ShimPass.cpp
  lld/lib/ReaderWriter/MachO/StubsPass.cpp
  lld/lib/ReaderWriter/MachO/TLVPass.cpp
  lld/lib/ReaderWriter/MachO/WriterMachO.cpp
  lld/lib/ReaderWriter/YAML/CMakeLists.txt
  lld/lib/ReaderWriter/YAML/ReaderWriterYAML.cpp
  lld/test/CMakeLists.txt
  lld/test/darwin/Inputs/native-and-mach-o.objtxt
  lld/test/darwin/Inputs/native-and-mach-o2.objtxt
  lld/test/darwin/cmdline-lto_library.objtxt
  lld/test/darwin/cmdline-objc_gc.objtxt
  lld/test/darwin/cmdline-objc_gc_compaction.objtxt
  lld/test/darwin/cmdline-objc_gc_only.objtxt
  lld/test/darwin/native-and-mach-o.objtxt
  lld/test/mach-o/Inputs/DependencyDump.py
  lld/test/mach-o/Inputs/MacOSX.sdk/usr/lib/libSystem.tbd
  lld/test/mach-o/Inputs/PIE.yaml
  lld/test/mach-o/Inputs/arm-interworking.yaml
  lld/test/mach-o/Inputs/arm-shims.yaml
  lld/test/mach-o/Inputs/arm64/libSystem.yaml
  lld/test/mach-o/Inputs/armv7/libSystem.yaml
  lld/test/mach-o/Inputs/bar.yaml
  lld/test/mach-o/Inputs/cstring-sections.yaml
  lld/test/mach-o/Inputs/exported_symbols_list.exp
  lld/test/mach-o/Inputs/full.filelist
  lld/test/mach-o/Inputs/got-order.yaml
  lld/test/mach-o/Inputs/got-order2.yaml
  lld/test/mach-o/Inputs/hello-world-arm64.yaml
  lld/test/mach-o/Inputs/hello-world-armv6.yaml
  lld/test/mach-o/Inputs/hello-world-armv7.yaml
  lld/test/mach-o/Inputs/hello-world-x86.yaml
  lld/test/mach-o/Inputs/hello-world-x86_64.yaml
  lld/test/mach-o/Inputs/hw.raw_bytes
  lld/test/mach-o/Inputs/interposing-section.yaml
  lld/test/mach-o/Inputs/lazy-bind-x86_64-2.yaml
  lld/test/mach-o/Inputs/lazy-bind-x86_64-3.yaml
  lld/test/mach-o/Inputs/lazy-bind-x86_64.yaml
  lld/test/mach-o/Inputs/lib-search-paths/usr/lib/libmyshared.dylib
  lld/test/mach-o/Inputs/lib-search-paths/usr/lib/libmystatic.a
  lld/test/mach-o/Inputs/lib-search-paths/usr/local/lib/file.o
  lld/test/mach-o/Inputs/libbar.a
  lld/test/mach-o/Inputs/libfoo.a
  lld/test/mach-o/Inputs/no-version-min-load-command-object.yaml
  lld/test/mach-o/Inputs/order_file-basic.order
  lld/test/mach-o/Inputs/partial.filelist
  lld/test/mach-o/Inputs/re-exported-dylib-ordinal.yaml
  lld/test/mach-o/Inputs/re-exported-dylib-ordinal2.yaml
  lld/test/mach-o/Inputs/re-exported-dylib-ordinal3.yaml
  lld/test/mach-o/Inputs/swift-version-1.yaml
  lld/test/mach-o/Inputs/unwind-info-simple-arm64.yaml
  lld/test/mach-o/Inputs/use-dylib-install-names.yaml
  lld/test/mach-o/Inputs/use-simple-dylib.yaml
  lld/test/mach-o/Inputs/write-final-sections.yaml
  lld/test/mach-o/Inputs/wrong-arch-error.yaml
  lld/test/mach-o/Inputs/x86/libSystem.yaml
  lld/test/mach-o/Inputs/x86_64/libSystem.yaml
  lld/test/mach-o/PIE.yaml
  lld/test/mach-o/align_text.yaml
  lld/test/mach-o/arm-interworking-movw.yaml
 

[PATCH] D112410: [SPIR-V] Add a toolchain for SPIR-V in clang

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 391064.
Anastasia added a comment.

- Exported full diff.
- Added forgotten test cases.
- Fixed typo in docs.


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

https://reviews.llvm.org/D112410

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Tool.h
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/Clang.h
  clang/lib/Driver/ToolChains/SPIRV.cpp
  clang/lib/Driver/ToolChains/SPIRV.h
  clang/test/Driver/spirv-toolchain.cl
  clang/test/Misc/warning-flags.c

Index: clang/test/Misc/warning-flags.c
===
--- clang/test/Misc/warning-flags.c
+++ clang/test/Misc/warning-flags.c
@@ -18,7 +18,7 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (67):
+CHECK: Warnings without flags (68):
 
 CHECK-NEXT:   ext_expected_semi_decl_list
 CHECK-NEXT:   ext_explicit_specialization_storage_class
@@ -47,6 +47,7 @@
 CHECK-NEXT:   warn_drv_assuming_mfloat_abi_is
 CHECK-NEXT:   warn_drv_clang_unsupported
 CHECK-NEXT:   warn_drv_pch_not_first_include
+CHECK-NEXT:   warn_drv_spirv_linking_multiple_inputs_unsupported
 CHECK-NEXT:   warn_dup_category_def
 CHECK-NEXT:   warn_enum_value_overflow
 CHECK-NEXT:   warn_expected_qualified_after_typename
Index: clang/test/Driver/spirv-toolchain.cl
===
--- /dev/null
+++ clang/test/Driver/spirv-toolchain.cl
@@ -0,0 +1,65 @@
+// Check object emission.
+// RUN: %clang -### -no-canonical-prefixes -target spirv64 -x cl -c %s 2>&1 | FileCheck --check-prefix=SPV64 %s
+// RUN: %clang -### -target spirv64 %s 2>&1 | FileCheck --check-prefix=SPV64 %s
+// RUN: %clang -### -no-canonical-prefixes -target spirv64 -x ir -c %s 2>&1 | FileCheck --check-prefix=SPV64 %s
+// RUN: %clang -### -no-canonical-prefixes -target spirv64 -x clcpp -c %s 2>&1 | FileCheck --check-prefix=SPV64 %s
+// RUN: %clang -### -no-canonical-prefixes -target spirv64 -x c -c %s 2>&1 | FileCheck --check-prefix=SPV64 %s
+
+// SPV64: clang{{.*}} "-cc1" "-triple" "spirv64"
+// SPV64-SAME: "-o" [[BC:".*bc"]]
+// SPV64: {{".*llvm-spirv.*"}} [[BC]] "-o" {{".*o"}}
+
+// RUN: %clang -### -no-canonical-prefixes -target spirv32 -x cl -c %s 2>&1 | FileCheck --check-prefix=SPV32 %s
+// RUN: %clang -### -target spirv32 %s 2>&1 | FileCheck --check-prefix=SPV32 %s
+// RUN: %clang -### -no-canonical-prefixes -target spirv32 -x ir -c %s 2>&1 | FileCheck --check-prefix=SPV32 %s
+// RUN: %clang -### -no-canonical-prefixes -target spirv32 -x clcpp -c %s 2>&1 | FileCheck --check-prefix=SPV32 %s
+// RUN: %clang -### -no-canonical-prefixes -target spirv32 -x c -c %s 2>&1 | FileCheck --check-prefix=SPV32 %s
+
+// SPV32: clang{{.*}} "-cc1" "-triple" "spirv32"
+// SPV32-SAME: "-o" [[BC:".*bc"]]
+// SPV32: {{".*llvm-spirv.*"}} [[BC]] "-o" {{".*o"}}
+
+//-
+// Check Assembly emission.
+// RUN: %clang -### -no-canonical-prefixes -target spirv64 -x cl -S %s 2>&1 | FileCheck --check-prefix=SPT64 %s
+// RUN: %clang -### -no-canonical-prefixes -target spirv64 -x ir -S %s 2>&1 | FileCheck --check-prefix=SPT64 %s
+// RUN: %clang -### -no-canonical-prefixes -target spirv64 -x clcpp -c %s 2>&1 | FileCheck --check-prefix=SPV64 %s
+// RUN: %clang -### -no-canonical-prefixes -target spirv64 -x c -S %s 2>&1 | FileCheck --check-prefix=SPT64 %s
+
+// SPT64: clang{{.*}} "-cc1" "-triple" "spirv64"
+// SPT64-SAME: "-o" [[BC:".*bc"]]
+// SPT64: {{".*llvm-spirv.*"}} [[BC]] "-spirv-text" "-o" {{".*s"}}
+
+// RUN: %clang -### -no-canonical-prefixes -target spirv32 -x cl -S %s 2>&1 | FileCheck --check-prefix=SPT32 %s
+// RUN: %clang -### -no-canonical-prefixes -target spirv32 -x ir -S %s 2>&1 | FileCheck --check-prefix=SPT32 %s
+// RUN: %clang -### -no-canonical-prefixes -target spirv32 -x clcpp -c %s 2>&1 | FileCheck --check-prefix=SPV32 %s
+// RUN: %clang -### -no-canonical-prefixes -target spirv32 -x c -S %s 2>&1 | FileCheck --check-prefix=SPT32 %s
+
+// SPT32: clang{{.*}} "-cc1" "-triple" "spirv32"
+// SPT32-SAME: "-o" [[BC:".*bc"]]
+// SPT32: {{".*llvm-spirv.*"}} [[BC]] "-spirv-text" "-o" {{".*s"}}
+
+//-
+// Check assembly input -> object output
+// RUN: %clang -### -no-canonical-prefixes -target spirv64 -x assembler -c %s 2>&1 | FileCheck --check-prefix=ASM %s
+// RUN: %clang -### -no-canonical-prefixes -target spirv32 -x assembler -c %s 2>&1 | FileCheck --check-prefix=ASM %s
+// ASM: {{".*llvm-spirv.*"}} {{".*"}} "-to-binary" "-o" {{".*o"}}
+
+//-
+// Check --save-temps.
+// RUN: %clang -### -no-canonical-prefixes -ta

[PATCH] D110745: Redefine deref(N) attribute family as point-in-time semantics (aka deref-at-point)

[Philip Reames via Phabricator via cfe-commits]

reames abandoned this revision.
reames added a comment.

I'm stopping work on this.  This has already exceeded the amount of work which 
is worthwhile for me, and it seems there is yet more work needed.

On @nikic's prompting, I finally went ahead and got the test-suite setup and 
tested a clang version with and without the flag thrown.  Unfortunately, the 
results were not pretty.  We have significant regressions in some of the 
memset/memcpy tests.   I did not bother to dig into why in detail, but I 
suspect the lack of context sensitivity is biting us.

Between the measured regressions on both C++ and rust, I don't think this can 
go in.

At this point, I've done everything I reasonable can to drive this to 
conclusion.  My actual motivation for this was a purely defensive effort to 
avoid regressing Java performance when this someday got fixed, and to make a 
good faith effort to justify my objections to Johannes' original patches.  That 
is complete.

Frankly, I think it's incredibly unfortunate that clang has an active 
miscompile, and no one seems motivated to fix that after *years* of it being 
there.  However, I have no commercial interest in clang, and the amount of work 
that seems to be remaining is well beyond anything I'm willing to do on a 
volunteer basis.

Let me summarize some ideas on future direction for the next poor person who 
stumbles into this rats nest.

The approach taken in this round of trying to infer scoped dereferenceability 
from existing attributes and to strengthen the inference of such to cover 
practical cases has been partially successful, but I no longer believe can be 
pushed across the finish line.  The problem here is not technical, but 
political.  We appear to have unresolved disagreements about the semantics of 
attributes, and the review process towards resolving those disagreements touch 
on many otherwise disjoint parts of the project.  I would definitely not advise 
moving further in this direction unless you greatly enjoy herding cats.

We could implement a contextual dereferenceability analysis.  This is useful to 
have no matter what, but requires extending the current must-execute logic and 
finding ways to efficiently make that information available cheaply through 
much of the pass pipeline.  I have some ideas on that, and if someone wants to 
brainstorm this, feel free to reach out.  However, I think it needs to be said 
that its unclear if a "perfect" version of this analysis is enough to recover 
the scoped facts in all cases.  This is a fairly speculative approach, and it 
might not be enough.

The approach taken in D61652  of splitting the 
dereferenceability attribute into two is a bit ugly.  The objection to this 
approach in this round was mostly driven by the observation that the 
"alloc-size" attribute had the same semantic split around whether the implied 
dereferenceability was scoped or not.  The good news is that the work done in 
this round was enough to cover performance regressions from the "alloc-size" 
version, and at this point, the only checked in code for "alloc-size" uses the 
non-globally dereferenceable semantics.  (We had to because it was actively 
miscompiling otherwise.)

Personally, if I was motivated to continue working on this, I'd probably 
resurrect D61652  and call it a day.

@nikic - Since you now have the sole remaining frontend for which dropping 
global deref is a performance regression without also being a correctness fix, 
any chance you're interested in driving this further?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110745

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

[PATCH] D114379: [OMPIRBuilder] Add support for simd (loop) directive.

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added inline comments.



Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:2586-2587
 void CodeGenFunction::EmitOMPSimdDirective(const OMPSimdDirective &S) {
+
+  bool UseOMPIRBuilder = CGM.getLangOpts().OpenMPIRBuilder;
+  if (UseOMPIRBuilder) {

arnamoy10 wrote:
> Meinersbur wrote:
> > [nit] Unnecessary whitespace
> I took a look at `isSupportedByOpenMPIRBuilder()` and seems like it is more 
> suitable for a check against work-sharing loop construct, not for `simd`.  
> 
> And for `simd`, I am not sure what features I should be checking for.  One 
> check I can think of is checking whether any of the not-yet-supported clauses 
> is present in the simd directive or not.  Am I on the right track?  
Indeed, `isSupportedByOpenMPIRBuilder` is for the workshoring-loop, you could 
introduce another one for simd. Or modify it to accept `OMPExecutableDirective` 
as an argument. This might be useful for combined constructs such as `for simd`.

Best approach IMHO is to whitelist clauses. Reject any clause you did not 
test/add support yet. This way we can fallback to the non-OpenMPIRBuilder 
codegen and not break things that worked without `-fopenmp-enable-irbuilder` (a 
lot of code already does not work `-fopenmp-enable-irbuilder`, so don't put too 
much effort into getting the interaction between OpenMPIRBuilder and current 
codegen to work; but we need an overview what is already supported and which 
clauses are still require some work)



Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:2594
+// Emit the associated statement and get its loop representation.
+auto DL = SourceLocToDebugLoc(S.getBeginLoc());
+const Stmt *Inner = S.getRawStmt();

arnamoy10 wrote:
> Meinersbur wrote:
> > [style] According to the [[ 
> > https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
> >  | LLVM coding standard ]], `auto` is only used in specific cases.
> I am inspired by [[ 
> https://github.com/llvm/llvm-project/blob/003c9c7457d0be5deeca7eee84ab5f110bf6/clang/lib/CodeGen/CGStmtOpenMP.cpp#L2611
>  | existing code ]] though
There already is a lot of code violating the coding standard, either because 
the coding standard changed or it slipped through a review. A prominent example 
is that the methods of the IRBuilder are still PascalCase, although the current 
standard says they should use camelCase. Same applies to `SourceLocToDebugLoc` 
itself (which every time I see it makes me think `SourceLocToDebugLoc` is a 
type)

Not a reason to not use the newest coding standard for new code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114379

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

[clang] 6dd0bfa - [Clang][VE] Fix toolchain test when -DCLANG_DEFAULT_LINKER=lld in use

[Simon Moll via cfe-commits]

Author: Simon Moll
Date: 2021-12-01T18:26:23+01:00
New Revision: 6dd0bfad0d3c7bfaa3442ad2382ce3bd8ad94be3

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

LOG: [Clang][VE] Fix toolchain test when -DCLANG_DEFAULT_LINKER=lld in use

The CLANG_DEFAULT_LINKER flag overrides the default toolchain linker.
VE strictly requires 'nld' to be the default linker.  This causes a test
failure in test/Driver/ve-toolchain.cpp when configured with
CLANG_DEFAULT_LINKER!=ld

  Failure in clang-ppc64le-rhel
  (https://lab.llvm.org/buildbot/#/builders/57/builds/12628)

Until default linker selection with CLANG_DEFAULT_LINKER!=ld is fixed
proper, we manually specify '-fuse-ld=ld' (ie the toolchain default
linker) in the ve-toolchain tests.

Added: 


Modified: 
clang/test/Driver/ve-toolchain.c
clang/test/Driver/ve-toolchain.cpp

Removed: 




diff  --git a/clang/test/Driver/ve-toolchain.c 
b/clang/test/Driver/ve-toolchain.c
index 3d6fde1447090..eb12fdffa7f6b 100644
--- a/clang/test/Driver/ve-toolchain.c
+++ b/clang/test/Driver/ve-toolchain.c
@@ -59,9 +59,11 @@
 
///-
 /// Checking -fintegrated-as
 
-// RUN: %clang -### -target ve -x assembler %s 2>&1 | \
+// RUN: %clang -### -target ve \
+// RUN:-x assembler -fuse-ld=ld %s 2>&1 | \
 // RUN:FileCheck -check-prefix=AS %s
-// RUN: %clang -### -target ve -fno-integrated-as -x assembler %s 2>&1 | \
+// RUN: %clang -### -target ve \
+// RUN:-fno-integrated-as -fuse-ld=ld -x assembler %s 2>&1 | \
 // RUN:FileCheck -check-prefix=NAS %s
 
 // AS: clang{{.*}} "-cc1as"
@@ -80,6 +82,7 @@
 // RUN: %clang -### -target ve-unknown-linux-gnu \
 // RUN: --sysroot %S/Inputs/basic_ve_tree \
 // RUN: -resource-dir=%S/Inputs/basic_ve_tree/resource_dir \
+// RUN: -fuse-ld=ld \
 // RUN: %s 2>&1 | FileCheck -check-prefix=DEF %s
 
 // DEF:  clang{{.*}}" "-cc1"

diff  --git a/clang/test/Driver/ve-toolchain.cpp 
b/clang/test/Driver/ve-toolchain.cpp
index 55bcf5b1a09c2..6567f9606b754 100644
--- a/clang/test/Driver/ve-toolchain.cpp
+++ b/clang/test/Driver/ve-toolchain.cpp
@@ -90,9 +90,11 @@
 
///-
 /// Checking -fintegrated-as
 
-// RUN: %clangxx -### -target ve -x assembler %s 2>&1 | \
+// RUN: %clangxx -### -target ve \
+// RUN:-x assembler -fuse-ld=ld %s 2>&1 | \
 // RUN:FileCheck -check-prefix=AS %s
-// RUN: %clangxx -### -target ve -fno-integrated-as -x assembler %s 2>&1 | \
+// RUN: %clangxx -### -target ve \
+// RUN:-fno-integrated-as -x assembler -fuse-ld=ld %s 2>&1 | \
 // RUN:FileCheck -check-prefix=NAS %s
 
 // AS: clang{{.*}} "-cc1as"
@@ -110,6 +112,7 @@
 
 // RUN: %clangxx -### -target ve-unknown-linux-gnu \
 // RUN: --sysroot %S/Inputs/basic_ve_tree \
+// RUN: -fuse-ld=ld \
 // RUN: -resource-dir=%S/Inputs/basic_ve_tree/resource_dir \
 // RUN: --stdlib=c++ %s 2>&1 | FileCheck -check-prefix=DEF %s
 



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

[PATCH] D112648: [clang-tidy] Improve fix-its for `modernize-pass-by-value` check

[Adrian Vogelsgesang via Phabricator via cfe-commits]

avogelsgesang added inline comments.



Comment at: clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp:198
+for (const ParmVarDecl *ParmDecl : collectParamDecls(Ctor, ParamDecl)) {
+  auto ParamTL = ParmDecl->getTypeSourceInfo()->getTypeLoc();
+  auto RefTL = ParamTL.getAs();

aaron.ballman wrote:
> Please spell out this type (per our usual coding conventions). The use on the 
> next line is fine because the type name is spelled out in the initialization.
Thanks for the hint. I wasn't aware of that convention. I fixed it here.

Should I also fix the other violations in this file?

E.g., a couple of lines above, we have

```
auto Diag = diag(ParamDecl->getBeginLoc(), "pass by value and use std::move");
```

which I guess should be

```
clang::DiagnosticBuilder Diag = diag(ParamDecl->getBeginLoc(), "pass by value 
and use std::move");
```



Comment at: clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp:220
 
-TypeLoc ValueTL = RefTL.getPointeeLoc();
-auto TypeRange = CharSourceRange::getTokenRange(ParmDecl->getBeginLoc(),
-ParamTL.getEndLoc());
-std::string ValueStr = Lexer::getSourceText(CharSourceRange::getTokenRange(
-ValueTL.getSourceRange()),
-SM, getLangOpts())
-   .str();
-ValueStr += ' ';
-Diag << FixItHint::CreateReplacement(TypeRange, ValueStr);
+  // Check if we need to append an additional whitespace
+  auto TypeRangeEnd =

aaron.ballman wrote:
> I don't think we should take these changes, unless they're strictly necessary 
> for program correctness. The rule of thumb in clang-tidy for fix-its is that 
> we do not make much attempt to have pretty formatting from the fix (the fix 
> needs to be correct, but doesn't need to be formatted); the user can run 
> clang-format after applying fixes. Someday, we'd like to consume clang-format 
> as a library and automatically reformat the code after applying fixes instead 
> of trying to catch every formatting concern in each individual check.
sounds good. Removed the whitespace related parts from this patch



Comment at: clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp:221-225
+  auto TypeRangeEnd =
+  Lexer::getLocForEndOfToken(ParamTL.getEndLoc(), 0, SM, 
getLangOpts());
+  auto NextChar = Lexer::getSourceText(
+  CharSourceRange::getCharRange(TypeRangeEnd,
+TypeRangeEnd.getLocWithOffset(1)),

aaron.ballman wrote:
> Please spell out the types.
no longer applicable after removing the whitespace related part of this patch



Comment at: clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp:227-229
+  if (NextChar != " ") {
+ValueStr += ' ';
+  }

aaron.ballman wrote:
> (Style guideline nit)
no longer applicable after removing the whitespace related part of this patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112648

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