[PATCH] D59287: [X86] Only define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 in 64-bit mode.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC356118: [X86] Only define 
__GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 in 64-bit mode. (authored by ctopper, 
committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D59287?vs=190376=190571#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D59287

Files:
  lib/Basic/Targets/X86.cpp
  test/Preprocessor/x86_target_features.c


Index: lib/Basic/Targets/X86.cpp
===
--- lib/Basic/Targets/X86.cpp
+++ lib/Basic/Targets/X86.cpp
@@ -1280,7 +1280,7 @@
   }
   if (CPU >= CK_i586)
 Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_8");
-  if (HasCX16)
+  if (HasCX16 && getTriple().getArch() == llvm::Triple::x86_64)
 Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_16");
 
   if (HasFloat128)
Index: test/Preprocessor/x86_target_features.c
===
--- test/Preprocessor/x86_target_features.c
+++ test/Preprocessor/x86_target_features.c
@@ -348,9 +348,13 @@
 
 // NOTBM-NOT: #define __TBM__ 1
 
-// RUN: %clang -target i386-unknown-unknown -march=pentiumpro -mcx16 -x c -E 
-dM -o - %s | FileCheck -match-full-lines --check-prefix=MCX16 %s
+// RUN: %clang -target i386-unknown-unknown -march=pentiumpro -mcx16 -x c -E 
-dM -o - %s | FileCheck -match-full-lines --check-prefix=MCX16-32 %s
 
-// MCX16: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 1
+// MCX16-32-NOT: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 1
+
+// RUN: %clang -target x86_64-unknown-unknown -march=x86-64 -mcx16 -x c -E -dM 
-o - %s | FileCheck -match-full-lines --check-prefix=MCX16-64 %s
+
+// MCX16-64: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 1
 
 // RUN: %clang -target i386-unknown-unknown -march=atom -mprfchw -x c -E -dM 
-o - %s | FileCheck -match-full-lines --check-prefix=PRFCHW %s
 


Index: lib/Basic/Targets/X86.cpp
===
--- lib/Basic/Targets/X86.cpp
+++ lib/Basic/Targets/X86.cpp
@@ -1280,7 +1280,7 @@
   }
   if (CPU >= CK_i586)
 Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_8");
-  if (HasCX16)
+  if (HasCX16 && getTriple().getArch() == llvm::Triple::x86_64)
 Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_16");
 
   if (HasFloat128)
Index: test/Preprocessor/x86_target_features.c
===
--- test/Preprocessor/x86_target_features.c
+++ test/Preprocessor/x86_target_features.c
@@ -348,9 +348,13 @@
 
 // NOTBM-NOT: #define __TBM__ 1
 
-// RUN: %clang -target i386-unknown-unknown -march=pentiumpro -mcx16 -x c -E -dM -o - %s | FileCheck -match-full-lines --check-prefix=MCX16 %s
+// RUN: %clang -target i386-unknown-unknown -march=pentiumpro -mcx16 -x c -E -dM -o - %s | FileCheck -match-full-lines --check-prefix=MCX16-32 %s
 
-// MCX16: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 1
+// MCX16-32-NOT: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 1
+
+// RUN: %clang -target x86_64-unknown-unknown -march=x86-64 -mcx16 -x c -E -dM -o - %s | FileCheck -match-full-lines --check-prefix=MCX16-64 %s
+
+// MCX16-64: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 1
 
 // RUN: %clang -target i386-unknown-unknown -march=atom -mprfchw -x c -E -dM -o - %s | FileCheck -match-full-lines --check-prefix=PRFCHW %s
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r356118 - [X86] Only define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 in 64-bit mode.

[Craig Topper via cfe-commits]

Author: ctopper
Date: Wed Mar 13 22:45:42 2019
New Revision: 356118

URL: http://llvm.org/viewvc/llvm-project?rev=356118=rev
Log:
[X86] Only define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 in 64-bit mode.

Summary:
This define should correspond to CMPXCHG16B being available which requires 
64-bit mode.

I checked and gcc also seems to only define this in 64-bit mode.

Reviewers: RKSimon, spatel, efriedma, jyknight, jfb

Reviewed By: jfb

Subscribers: jfb, cfe-commits, llvm-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/Basic/Targets/X86.cpp
cfe/trunk/test/Preprocessor/x86_target_features.c

Modified: cfe/trunk/lib/Basic/Targets/X86.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/X86.cpp?rev=356118=356117=356118=diff
==
--- cfe/trunk/lib/Basic/Targets/X86.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/X86.cpp Wed Mar 13 22:45:42 2019
@@ -1280,7 +1280,7 @@ void X86TargetInfo::getTargetDefines(con
   }
   if (CPU >= CK_i586)
 Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_8");
-  if (HasCX16)
+  if (HasCX16 && getTriple().getArch() == llvm::Triple::x86_64)
 Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_16");
 
   if (HasFloat128)

Modified: cfe/trunk/test/Preprocessor/x86_target_features.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/x86_target_features.c?rev=356118=356117=356118=diff
==
--- cfe/trunk/test/Preprocessor/x86_target_features.c (original)
+++ cfe/trunk/test/Preprocessor/x86_target_features.c Wed Mar 13 22:45:42 2019
@@ -348,9 +348,13 @@
 
 // NOTBM-NOT: #define __TBM__ 1
 
-// RUN: %clang -target i386-unknown-unknown -march=pentiumpro -mcx16 -x c -E 
-dM -o - %s | FileCheck -match-full-lines --check-prefix=MCX16 %s
+// RUN: %clang -target i386-unknown-unknown -march=pentiumpro -mcx16 -x c -E 
-dM -o - %s | FileCheck -match-full-lines --check-prefix=MCX16-32 %s
 
-// MCX16: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 1
+// MCX16-32-NOT: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 1
+
+// RUN: %clang -target x86_64-unknown-unknown -march=x86-64 -mcx16 -x c -E -dM 
-o - %s | FileCheck -match-full-lines --check-prefix=MCX16-64 %s
+
+// MCX16-64: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 1
 
 // RUN: %clang -target i386-unknown-unknown -march=atom -mprfchw -x c -E -dM 
-o - %s | FileCheck -match-full-lines --check-prefix=PRFCHW %s
 


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

[PATCH] D59347: [DebugInfo] Combine Trivial and NonTrivial flags

[Aaron Smith via Phabricator via cfe-commits]

asmith created this revision.
asmith added reviewers: rnk, zturner, dblaikie, probinson.
Herald added subscribers: cfe-commits, jdoerfert, aprantl.
Herald added a project: clang.

These flags are used when emitting debug info and needed to initialize 
subprogram and member function attributes (function options) for Codeview. 
These function options are used to create an accurate compiler type for UDT 
symbols (class/struct/union) from PDBs.

The Trivial flag was introduced in https://reviews.llvm.org/D45122

It's been pointed out that Trivial and NonTrivial may imply each other and that 
seems to be the case in the current tests. This change combines them into a 
single flag -- NonTrivial -- and updates the corresponding unit tests. There is 
an additional change to llvm to update the flags.


Repository:
  rC Clang

https://reviews.llvm.org/D59347

Files:
  lib/CodeGen/CGDebugInfo.cpp
  test/CodeGenCXX/debug-info-composite-triviality.cpp


Index: test/CodeGenCXX/debug-info-composite-triviality.cpp
===
--- test/CodeGenCXX/debug-info-composite-triviality.cpp
+++ test/CodeGenCXX/debug-info-composite-triviality.cpp
@@ -23,44 +23,6 @@
 // CHECK-DAG: !DIGlobalVariable(name: "GlobalVar", {{.*}}type: {{.*}}, 
isLocal: false, isDefinition: true)
 int GlobalVar = 0;
 
-// Cases to test composite type's triviality
-
-// CHECK-DAG: !DICompositeType({{.*}}, name: "Union",{{.*}}flags: 
{{.*}}DIFlagTrivial
-union Union {
-  int a;
-} Union;
-
-// CHECK-DAG: !DICompositeType({{.*}}, name: "Trivial",{{.*}}flags: 
{{.*}}DIFlagTrivial
-struct Trivial {
-  int i;
-} Trivial;
-
-// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialA",{{.*}}flags: 
{{.*}}DIFlagTrivial
-struct TrivialA {
-  TrivialA() = default;
-} TrivialA;
-
-// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialB",{{.*}}flags: 
{{.*}}DIFlagTrivial
-struct TrivialB {
-  int m;
-  TrivialB(int x) { m = x; }
-  TrivialB() = default;
-} TrivialB;
-
-// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialC",{{.*}}flags: 
{{.*}}DIFlagTrivial
-struct TrivialC {
-  struct Trivial x;
-} TrivialC;
-
-// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialD",{{.*}}flags: 
{{.*}}DIFlagTrivial
-struct NT {
-  NT() {};
-};
-struct TrivialD {
-  static struct NT x; // Member is non-trivial but is static.
-} TrivialD;
-
-
 // CHECK-DAG: !DICompositeType({{.*}}, name: "NonTrivial",{{.*}}flags: 
{{.*}}DIFlagNonTrivial
 struct NonTrivial {
   NonTrivial() {}
@@ -84,7 +46,3 @@
 // CHECK-DAG: !DICompositeType({{.*}}, name: "NonTrivialD",{{.*}}flags: 
{{.*}}DIFlagNonTrivial
 struct NonTrivialD : NonTrivial {
 } NonTrivialD;
-
-// CHECK-DAG: !DICompositeType({{.*}}, name: "NonTrivialE",{{.*}}flags: 
{{.*}}DIFlagNonTrivial
-struct NonTrivialE : Trivial, NonTrivial {
-} NonTrivialE;
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -3034,10 +3034,8 @@
 else
   Flags |= llvm::DINode::FlagTypePassByValue;
 
-// Record if a C++ record is trivial type.
-if (CXXRD->isTrivial())
-  Flags |= llvm::DINode::FlagTrivial;
-else
+// Record if a C++ record is non-trivial type.
+if (!CXXRD->isTrivial())
   Flags |= llvm::DINode::FlagNonTrivial;
   }
 


Index: test/CodeGenCXX/debug-info-composite-triviality.cpp
===
--- test/CodeGenCXX/debug-info-composite-triviality.cpp
+++ test/CodeGenCXX/debug-info-composite-triviality.cpp
@@ -23,44 +23,6 @@
 // CHECK-DAG: !DIGlobalVariable(name: "GlobalVar", {{.*}}type: {{.*}}, isLocal: false, isDefinition: true)
 int GlobalVar = 0;
 
-// Cases to test composite type's triviality
-
-// CHECK-DAG: !DICompositeType({{.*}}, name: "Union",{{.*}}flags: {{.*}}DIFlagTrivial
-union Union {
-  int a;
-} Union;
-
-// CHECK-DAG: !DICompositeType({{.*}}, name: "Trivial",{{.*}}flags: {{.*}}DIFlagTrivial
-struct Trivial {
-  int i;
-} Trivial;
-
-// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialA",{{.*}}flags: {{.*}}DIFlagTrivial
-struct TrivialA {
-  TrivialA() = default;
-} TrivialA;
-
-// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialB",{{.*}}flags: {{.*}}DIFlagTrivial
-struct TrivialB {
-  int m;
-  TrivialB(int x) { m = x; }
-  TrivialB() = default;
-} TrivialB;
-
-// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialC",{{.*}}flags: {{.*}}DIFlagTrivial
-struct TrivialC {
-  struct Trivial x;
-} TrivialC;
-
-// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialD",{{.*}}flags: {{.*}}DIFlagTrivial
-struct NT {
-  NT() {};
-};
-struct TrivialD {
-  static struct NT x; // Member is non-trivial but is static.
-} TrivialD;
-
-
 // CHECK-DAG: !DICompositeType({{.*}}, name: "NonTrivial",{{.*}}flags: {{.*}}DIFlagNonTrivial
 struct NonTrivial {
   NonTrivial() {}
@@ -84,7 +46,3 @@
 // CHECK-DAG: !DICompositeType({{.*}}, name: "NonTrivialD",{{.*}}flags: {{.*}}DIFlagNonTrivial
 struct NonTrivialD : 

[PATCH] D59346: [X86] Add gcc rotate intrinsics to ia32intrin.h

[Craig Topper via Phabricator via cfe-commits]

craig.topper updated this revision to Diff 190568.
craig.topper added a comment.

Fix bad comment copy/paste


Repository:
  rC Clang

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

https://reviews.llvm.org/D59346

Files:
  lib/Headers/ia32intrin.h
  test/CodeGen/rot-intrinsics.c

Index: test/CodeGen/rot-intrinsics.c
===
--- /dev/null
+++ test/CodeGen/rot-intrinsics.c
@@ -0,0 +1,120 @@
+// RUN: %clang_cc1 -ffreestanding -triple i686--linux -emit-llvm %s -o - | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
+// RUN: %clang_cc1 -ffreestanding -triple x86_64--linux -emit-llvm %s -o - | FileCheck %s --check-prefixes CHECK,CHECK-64BIT-LONG
+// RUN: %clang_cc1 -fms-extensions -fms-compatibility -ffreestanding %s -triple=i686-windows-msvc -target-feature +sse2 -emit-llvm -o - -Wall -Werror | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
+// RUN: %clang_cc1 -fms-extensions -fms-compatibility -ffreestanding %s -triple=x86_64-windows-msvc -target-feature +sse2 -emit-llvm -o - -Wall -Werror | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
+// RUN: %clang_cc1 -fms-extensions -fms-compatibility -fms-compatibility-version=17.00 -ffreestanding %s -triple=i686-windows-msvc -target-feature +sse2 -emit-llvm -o - -Wall -Werror | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
+// RUN: %clang_cc1 -fms-extensions -fms-compatibility -fms-compatibility-version=17.00 -ffreestanding %s -triple=x86_64-windows-msvc -target-feature +sse2 -emit-llvm -o - -Wall -Werror | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
+
+#include 
+
+unsigned char test__rolb(unsigned char value, int shift) {
+// CHECK-LABEL: i8 @test__rolb
+// CHECK:   [[R:%.*]] = call i8 @llvm.fshl.i8(i8 [[X:%.*]], i8 [[X]], i8 [[Y:%.*]])
+// CHECK:   ret i8 [[R]]
+  return __rolb(value, shift);
+}
+
+unsigned short test__rolw(unsigned short value, int shift) {
+// CHECK-LABEL: i16 @test__rolw
+// CHECK:   [[R:%.*]] = call i16 @llvm.fshl.i16(i16 [[X:%.*]], i16 [[X]], i16 [[Y:%.*]])
+// CHECK:   ret i16 [[R]]
+  return __rolw(value, shift);
+}
+
+unsigned int test__rold(unsigned int value, int shift) {
+// CHECK-LABEL: i32 @test__rold
+// CHECK:   [[R:%.*]] = call i32 @llvm.fshl.i32(i32 [[X:%.*]], i32 [[X]], i32 [[Y:%.*]])
+// CHECK:   ret i32 [[R]]
+  return __rold(value, shift);
+}
+
+#if defined(__x86_64__)
+unsigned long test__rolq(unsigned long value, int shift) {
+// CHECK-LONG-LABEL: i64 @test__rolq
+// CHECK-LONG:   [[R:%.*]] = call i64 @llvm.fshl.i64(i64 [[X:%.*]], i64 [[X]], i64 [[Y:%.*]])
+// CHECK-LONG:   ret i64 [[R]]
+  return __rolq(value, shift);
+}
+#endif
+
+unsigned char test__rorb(unsigned char value, int shift) {
+// CHECK-LABEL: i8 @test__rorb
+// CHECK:   [[R:%.*]] = call i8 @llvm.fshr.i8(i8 [[X:%.*]], i8 [[X]], i8 [[Y:%.*]])
+// CHECK:   ret i8 [[R]]
+  return __rorb(value, shift);
+}
+
+unsigned short test__rorw(unsigned short value, int shift) {
+// CHECK-LABEL: i16 @test__rorw
+// CHECK:   [[R:%.*]] = call i16 @llvm.fshr.i16(i16 [[X:%.*]], i16 [[X]], i16 [[Y:%.*]])
+// CHECK:   ret i16 [[R]]
+  return __rorw(value, shift);
+}
+
+unsigned int test__rord(unsigned int value, int shift) {
+// CHECK-LABEL: i32 @test__rord
+// CHECK:   [[R:%.*]] = call i32 @llvm.fshr.i32(i32 [[X:%.*]], i32 [[X]], i32 [[Y:%.*]])
+// CHECK:   ret i32 [[R]]
+  return __rord(value, shift);
+}
+
+#if defined(__x86_64__)
+unsigned long test__rorq(unsigned long value, int shift) {
+// CHECK-LONG-LABEL: i64 @test__rorq
+// CHECK-LONG:   [[R:%.*]] = call i64 @llvm.fshr.i64(i64 [[X:%.*]], i64 [[X]], i64 [[Y:%.*]])
+// CHECK-LONG:   ret i64 [[R]]
+  return __rorq(value, shift);
+}
+#endif
+
+unsigned short test_rotwl(unsigned short value, int shift) {
+// CHECK-LABEL: i16 @test_rotwl
+// CHECK:   [[R:%.*]] = call i16 @llvm.fshl.i16(i16 [[X:%.*]], i16 [[X]], i16 [[Y:%.*]])
+// CHECK:   ret i16 [[R]]
+  return _rotwl(value, shift);
+}
+
+unsigned int test_rotl(unsigned int value, int shift) {
+// CHECK-LABEL: i32 @test_rotl
+// CHECK:   [[R:%.*]] = call i32 @llvm.fshl.i32(i32 [[X:%.*]], i32 [[X]], i32 [[Y:%.*]])
+// CHECK:   ret i32 [[R]]
+  return _rotl(value, shift);
+}
+
+unsigned long test_lrotl(unsigned long value, int shift) {
+// CHECK-32BIT-LONG-LABEL: i32 @test_lrotl
+// CHECK-32BIT-LONG:   [[R:%.*]] = call i32 @llvm.fshl.i32(i32 [[X:%.*]], i32 [[X]], i32 [[Y:%.*]])
+// CHECK-32BIT-LONG:   ret i32 [[R]]
+//
+// CHECK-64BIT-LONG-LABEL: i64 @test_lrotl
+// CHECK-64BIT-LONG:   [[R:%.*]] = call i64 @llvm.fshl.i64(i64 [[X:%.*]], i64 [[X]], i64 [[Y:%.*]])
+// CHECK-64BIT-LONG:   ret i64 [[R]]
+  return _lrotl(value, shift);
+}
+
+
+unsigned short test_rotwr(unsigned short value, int shift) {
+// CHECK-LABEL: i16 @test_rotwr
+// CHECK:   [[R:%.*]] = call i16 @llvm.fshr.i16(i16 [[X:%.*]], i16 [[X]], i16 [[Y:%.*]])
+// CHECK:   ret i16 [[R]]
+  return _rotwr(value, shift);
+}
+
+unsigned int test_rotr(unsigned int value, int shift) {
+// CHECK-LABEL: i32 @test_rotr

[PATCH] D59346: [X86] Add gcc rotate intrinsics to ia32intrin.h

[Craig Topper via Phabricator via cfe-commits]

craig.topper updated this revision to Diff 190567.
craig.topper added a comment.

Add the test file


Repository:
  rC Clang

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

https://reviews.llvm.org/D59346

Files:
  lib/Headers/ia32intrin.h
  test/CodeGen/rot-intrinsics.c

Index: test/CodeGen/rot-intrinsics.c
===
--- /dev/null
+++ test/CodeGen/rot-intrinsics.c
@@ -0,0 +1,120 @@
+// RUN: %clang_cc1 -ffreestanding -triple i686--linux -emit-llvm %s -o - | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
+// RUN: %clang_cc1 -ffreestanding -triple x86_64--linux -emit-llvm %s -o - | FileCheck %s --check-prefixes CHECK,CHECK-64BIT-LONG
+// RUN: %clang_cc1 -fms-extensions -fms-compatibility -ffreestanding %s -triple=i686-windows-msvc -target-feature +sse2 -emit-llvm -o - -Wall -Werror | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
+// RUN: %clang_cc1 -fms-extensions -fms-compatibility -ffreestanding %s -triple=x86_64-windows-msvc -target-feature +sse2 -emit-llvm -o - -Wall -Werror | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
+// RUN: %clang_cc1 -fms-extensions -fms-compatibility -fms-compatibility-version=17.00 -ffreestanding %s -triple=i686-windows-msvc -target-feature +sse2 -emit-llvm -o - -Wall -Werror | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
+// RUN: %clang_cc1 -fms-extensions -fms-compatibility -fms-compatibility-version=17.00 -ffreestanding %s -triple=x86_64-windows-msvc -target-feature +sse2 -emit-llvm -o - -Wall -Werror | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
+
+#include 
+
+unsigned char test__rolb(unsigned char value, int shift) {
+// CHECK-LABEL: i8 @test__rolb
+// CHECK:   [[R:%.*]] = call i8 @llvm.fshl.i8(i8 [[X:%.*]], i8 [[X]], i8 [[Y:%.*]])
+// CHECK:   ret i8 [[R]]
+  return __rolb(value, shift);
+}
+
+unsigned short test__rolw(unsigned short value, int shift) {
+// CHECK-LABEL: i16 @test__rolw
+// CHECK:   [[R:%.*]] = call i16 @llvm.fshl.i16(i16 [[X:%.*]], i16 [[X]], i16 [[Y:%.*]])
+// CHECK:   ret i16 [[R]]
+  return __rolw(value, shift);
+}
+
+unsigned int test__rold(unsigned int value, int shift) {
+// CHECK-LABEL: i32 @test__rold
+// CHECK:   [[R:%.*]] = call i32 @llvm.fshl.i32(i32 [[X:%.*]], i32 [[X]], i32 [[Y:%.*]])
+// CHECK:   ret i32 [[R]]
+  return __rold(value, shift);
+}
+
+#if defined(__x86_64__)
+unsigned long test__rolq(unsigned long value, int shift) {
+// CHECK-LONG-LABEL: i64 @test__rolq
+// CHECK-LONG:   [[R:%.*]] = call i64 @llvm.fshl.i64(i64 [[X:%.*]], i64 [[X]], i64 [[Y:%.*]])
+// CHECK-LONG:   ret i64 [[R]]
+  return __rolq(value, shift);
+}
+#endif
+
+unsigned char test__rorb(unsigned char value, int shift) {
+// CHECK-LABEL: i8 @test__rorb
+// CHECK:   [[R:%.*]] = call i8 @llvm.fshr.i8(i8 [[X:%.*]], i8 [[X]], i8 [[Y:%.*]])
+// CHECK:   ret i8 [[R]]
+  return __rorb(value, shift);
+}
+
+unsigned short test__rorw(unsigned short value, int shift) {
+// CHECK-LABEL: i16 @test__rorw
+// CHECK:   [[R:%.*]] = call i16 @llvm.fshr.i16(i16 [[X:%.*]], i16 [[X]], i16 [[Y:%.*]])
+// CHECK:   ret i16 [[R]]
+  return __rorw(value, shift);
+}
+
+unsigned int test__rord(unsigned int value, int shift) {
+// CHECK-LABEL: i32 @test__rord
+// CHECK:   [[R:%.*]] = call i32 @llvm.fshr.i32(i32 [[X:%.*]], i32 [[X]], i32 [[Y:%.*]])
+// CHECK:   ret i32 [[R]]
+  return __rord(value, shift);
+}
+
+#if defined(__x86_64__)
+unsigned long test__rorq(unsigned long value, int shift) {
+// CHECK-LONG-LABEL: i64 @test__rorq
+// CHECK-LONG:   [[R:%.*]] = call i64 @llvm.fshr.i64(i64 [[X:%.*]], i64 [[X]], i64 [[Y:%.*]])
+// CHECK-LONG:   ret i64 [[R]]
+  return __rorq(value, shift);
+}
+#endif
+
+unsigned short test_rotwl(unsigned short value, int shift) {
+// CHECK-LABEL: i16 @test_rotwl
+// CHECK:   [[R:%.*]] = call i16 @llvm.fshl.i16(i16 [[X:%.*]], i16 [[X]], i16 [[Y:%.*]])
+// CHECK:   ret i16 [[R]]
+  return _rotwl(value, shift);
+}
+
+unsigned int test_rotl(unsigned int value, int shift) {
+// CHECK-LABEL: i32 @test_rotl
+// CHECK:   [[R:%.*]] = call i32 @llvm.fshl.i32(i32 [[X:%.*]], i32 [[X]], i32 [[Y:%.*]])
+// CHECK:   ret i32 [[R]]
+  return _rotl(value, shift);
+}
+
+unsigned long test_lrotl(unsigned long value, int shift) {
+// CHECK-32BIT-LONG-LABEL: i32 @test_lrotl
+// CHECK-32BIT-LONG:   [[R:%.*]] = call i32 @llvm.fshl.i32(i32 [[X:%.*]], i32 [[X]], i32 [[Y:%.*]])
+// CHECK-32BIT-LONG:   ret i32 [[R]]
+//
+// CHECK-64BIT-LONG-LABEL: i64 @test_lrotl
+// CHECK-64BIT-LONG:   [[R:%.*]] = call i64 @llvm.fshl.i64(i64 [[X:%.*]], i64 [[X]], i64 [[Y:%.*]])
+// CHECK-64BIT-LONG:   ret i64 [[R]]
+  return _lrotl(value, shift);
+}
+
+
+unsigned short test_rotwr(unsigned short value, int shift) {
+// CHECK-LABEL: i16 @test_rotwr
+// CHECK:   [[R:%.*]] = call i16 @llvm.fshr.i16(i16 [[X:%.*]], i16 [[X]], i16 [[Y:%.*]])
+// CHECK:   ret i16 [[R]]
+  return _rotwr(value, shift);
+}
+
+unsigned int test_rotr(unsigned int value, int shift) {
+// CHECK-LABEL: i32 @test_rotr
+// CHECK: 

[PATCH] D59346: [X86] Add gcc rotate intrinsics to ia32intrin.h

[Craig Topper via Phabricator via cfe-commits]

craig.topper created this revision.
craig.topper added reviewers: rnk, jyknight, RKSimon, spatel, erichkeane.
Herald added a subscriber: jdoerfert.
Herald added a project: clang.
craig.topper updated this revision to Diff 190567.
craig.topper added a comment.

Add the test file


This is another attempt at what Erich Keane tried to do in r355322.a

This adds __rolb, __rolw, __rold, __rolq and their ror equivalent as 
always_inline wrappers around __builtin_rotate* which will lower to funnel 
shift intrinsics in IR.

Additionally, when _MSC_VER is not defined we will define _rotl, _lrotl, _rotr, 
_lrotr as macros to one of the always_inline intrinsics mentioned above. Making 
sure that _lrotl/_lrotr use either 32 or 64 bit based on the size of long. 
These need to be macros because we have builtins with the same name for MS 
compatibility, but _MSC_VER isn't always defined when those builtins are 
enabled.

We also define _rotwl and _rotwr as macros aliasing to __rolw/__rorw just like 
gcc to complete the set. These don't need to be gated with _MSC_VER because 
these aren't MS builtins.

I've added tests both for non-MS and -ms-extensions with and without _MSC_VER 
being defined.


Repository:
  rC Clang

https://reviews.llvm.org/D59346

Files:
  lib/Headers/ia32intrin.h
  test/CodeGen/rot-intrinsics.c

Index: test/CodeGen/rot-intrinsics.c
===
--- /dev/null
+++ test/CodeGen/rot-intrinsics.c
@@ -0,0 +1,120 @@
+// RUN: %clang_cc1 -ffreestanding -triple i686--linux -emit-llvm %s -o - | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
+// RUN: %clang_cc1 -ffreestanding -triple x86_64--linux -emit-llvm %s -o - | FileCheck %s --check-prefixes CHECK,CHECK-64BIT-LONG
+// RUN: %clang_cc1 -fms-extensions -fms-compatibility -ffreestanding %s -triple=i686-windows-msvc -target-feature +sse2 -emit-llvm -o - -Wall -Werror | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
+// RUN: %clang_cc1 -fms-extensions -fms-compatibility -ffreestanding %s -triple=x86_64-windows-msvc -target-feature +sse2 -emit-llvm -o - -Wall -Werror | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
+// RUN: %clang_cc1 -fms-extensions -fms-compatibility -fms-compatibility-version=17.00 -ffreestanding %s -triple=i686-windows-msvc -target-feature +sse2 -emit-llvm -o - -Wall -Werror | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
+// RUN: %clang_cc1 -fms-extensions -fms-compatibility -fms-compatibility-version=17.00 -ffreestanding %s -triple=x86_64-windows-msvc -target-feature +sse2 -emit-llvm -o - -Wall -Werror | FileCheck %s --check-prefixes CHECK,CHECK-32BIT-LONG
+
+#include 
+
+unsigned char test__rolb(unsigned char value, int shift) {
+// CHECK-LABEL: i8 @test__rolb
+// CHECK:   [[R:%.*]] = call i8 @llvm.fshl.i8(i8 [[X:%.*]], i8 [[X]], i8 [[Y:%.*]])
+// CHECK:   ret i8 [[R]]
+  return __rolb(value, shift);
+}
+
+unsigned short test__rolw(unsigned short value, int shift) {
+// CHECK-LABEL: i16 @test__rolw
+// CHECK:   [[R:%.*]] = call i16 @llvm.fshl.i16(i16 [[X:%.*]], i16 [[X]], i16 [[Y:%.*]])
+// CHECK:   ret i16 [[R]]
+  return __rolw(value, shift);
+}
+
+unsigned int test__rold(unsigned int value, int shift) {
+// CHECK-LABEL: i32 @test__rold
+// CHECK:   [[R:%.*]] = call i32 @llvm.fshl.i32(i32 [[X:%.*]], i32 [[X]], i32 [[Y:%.*]])
+// CHECK:   ret i32 [[R]]
+  return __rold(value, shift);
+}
+
+#if defined(__x86_64__)
+unsigned long test__rolq(unsigned long value, int shift) {
+// CHECK-LONG-LABEL: i64 @test__rolq
+// CHECK-LONG:   [[R:%.*]] = call i64 @llvm.fshl.i64(i64 [[X:%.*]], i64 [[X]], i64 [[Y:%.*]])
+// CHECK-LONG:   ret i64 [[R]]
+  return __rolq(value, shift);
+}
+#endif
+
+unsigned char test__rorb(unsigned char value, int shift) {
+// CHECK-LABEL: i8 @test__rorb
+// CHECK:   [[R:%.*]] = call i8 @llvm.fshr.i8(i8 [[X:%.*]], i8 [[X]], i8 [[Y:%.*]])
+// CHECK:   ret i8 [[R]]
+  return __rorb(value, shift);
+}
+
+unsigned short test__rorw(unsigned short value, int shift) {
+// CHECK-LABEL: i16 @test__rorw
+// CHECK:   [[R:%.*]] = call i16 @llvm.fshr.i16(i16 [[X:%.*]], i16 [[X]], i16 [[Y:%.*]])
+// CHECK:   ret i16 [[R]]
+  return __rorw(value, shift);
+}
+
+unsigned int test__rord(unsigned int value, int shift) {
+// CHECK-LABEL: i32 @test__rord
+// CHECK:   [[R:%.*]] = call i32 @llvm.fshr.i32(i32 [[X:%.*]], i32 [[X]], i32 [[Y:%.*]])
+// CHECK:   ret i32 [[R]]
+  return __rord(value, shift);
+}
+
+#if defined(__x86_64__)
+unsigned long test__rorq(unsigned long value, int shift) {
+// CHECK-LONG-LABEL: i64 @test__rorq
+// CHECK-LONG:   [[R:%.*]] = call i64 @llvm.fshr.i64(i64 [[X:%.*]], i64 [[X]], i64 [[Y:%.*]])
+// CHECK-LONG:   ret i64 [[R]]
+  return __rorq(value, shift);
+}
+#endif
+
+unsigned short test_rotwl(unsigned short value, int shift) {
+// CHECK-LABEL: i16 @test_rotwl
+// CHECK:   [[R:%.*]] = call i16 @llvm.fshl.i16(i16 [[X:%.*]], i16 [[X]], i16 [[Y:%.*]])
+// CHECK:   ret i16 [[R]]
+  return _rotwl(value, shift);
+}
+
+unsigned int 

[PATCH] D59254: [RFC] Implementation of Clang randstruct

[Vlad Tsyrklevich via Phabricator via cfe-commits]

vlad.tsyrklevich added inline comments.



Comment at: clang/include/clang/AST/RecordFieldReorganizer.h:54
+  std::seed_seq Seq;
+  std::default_random_engine rng;
+};

timpugh wrote:
> connorkuehl wrote:
> > pcc wrote:
> > > I don't think we can use `default_random_engine` for this because the 
> > > behaviour would need to be consistent between C++ standard library 
> > > implementations, and the behaviour of `default_random_engine` is 
> > > implementation defined. Similarly, I don't think that we can use 
> > > `std::shuffle` (see note in 
> > > https://en.cppreference.com/w/cpp/algorithm/random_shuffle ).
> > Sure thing, we'll begin investigating alternatives.
> @pcc  if we were to swap `default_random_engine` for a pre-defined random 
> generator such as `mt19937_64` would this suffice? It is included in the 
> random number library.
> 
> https://en.cppreference.com/w/cpp/numeric/random
In that case the random numbers would be deterministic; however, std::shuffle 
would still vary by implementation (as mentioned in the note Peter linked to.) 
A quick search didn't reveal a deterministic shuffle in the LLVM code so you 
may have to implement one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59254



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

[PATCH] D59254: [RFC] Implementation of Clang randstruct

[Tim Pugh via Phabricator via cfe-commits]

timpugh added inline comments.



Comment at: clang/include/clang/AST/RecordFieldReorganizer.h:54
+  std::seed_seq Seq;
+  std::default_random_engine rng;
+};

connorkuehl wrote:
> pcc wrote:
> > I don't think we can use `default_random_engine` for this because the 
> > behaviour would need to be consistent between C++ standard library 
> > implementations, and the behaviour of `default_random_engine` is 
> > implementation defined. Similarly, I don't think that we can use 
> > `std::shuffle` (see note in 
> > https://en.cppreference.com/w/cpp/algorithm/random_shuffle ).
> Sure thing, we'll begin investigating alternatives.
@pcc  if we were to swap `default_random_engine` for a pre-defined random 
generator such as `mt19937_64` would this suffice? It is included in the random 
number library.

https://en.cppreference.com/w/cpp/numeric/random


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59254



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

[PATCH] D56990: Bugfix for Replacement of tied operand of inline asm

[Xiang Zhang via Phabricator via cfe-commits]

xiangzhangllvm added a comment.

In D56990#1426977 , @efriedma wrote:

> LGTM; I'll merge it tonight or tomorrow.


Thank you very much!


Repository:
  rC Clang

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

https://reviews.llvm.org/D56990



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

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

[Andy Zhang via Phabricator via cfe-commits]

axzhang updated this revision to Diff 190559.
axzhang added a comment.

Apologies for the extended hiatus. I have changed the implementation to an 
alias to modernize-make-unique, and have added an extra option to 
modernize-make-unique that ignores C++11 features such as list initializations, 
to allow `absl::make_unique` to function correctly. Also, after looking more 
into `absl::WrapUnique`, it seems like this also is not really the function to 
use for brace initializations, so no extra functionality needs to be added to 
the existing modernize-make-unique check.


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

https://reviews.llvm.org/D55044

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tidy/modernize/MakeSmartPtrCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-make-unique.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-make-unique.rst
  test/clang-tidy/abseil-make-unique.cpp

Index: test/clang-tidy/abseil-make-unique.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-make-unique.cpp
@@ -0,0 +1,127 @@
+// RUN: %check_clang_tidy %s abseil-make-unique %t -- -- -std=c++11 \
+
+namespace std {
+
+template 
+class default_delete {};
+
+template >
+class unique_ptr {
+public:
+  unique_ptr() {}
+  unique_ptr(type *ptr) {}
+  unique_ptr(const unique_ptr ) = delete;
+  unique_ptr(unique_ptr &) {}
+  ~unique_ptr() {}
+  type *() { return *ptr; }
+  type *operator->() { return ptr; }
+  type *release() { return ptr; }
+  void reset() {}
+  void reset(type *pt) {}
+  void reset(type pt) {}
+  unique_ptr =(unique_ptr &&) { return *this; }
+  template 
+  unique_ptr =(unique_ptr &&) { return *this; }
+
+private:
+  type *ptr;
+};
+
+}  // namespace std
+
+class A {
+ int x;
+ int y;
+
+ public:
+   A(int _x, int _y): x(_x), y(_y) {}
+};
+
+struct Base {
+  Base();
+};
+
+struct Derived : public Base {
+  Derived();
+};
+
+int* returnPointer();
+void expectPointer(std::unique_ptr p);
+
+std::unique_ptr makeAndReturnPointer() {
+  return std::unique_ptr(new int(0));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: return absl::make_unique(0);
+}
+
+void Positives() {
+  std::unique_ptr P1 = std::unique_ptr(new int(1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P1 = absl::make_unique(1);
+
+  P1.reset(new int(2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P1 = absl::make_unique(2);
+
+  // Non-primitive paramter
+  std::unique_ptr P2 = std::unique_ptr(new A(1, 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P2 = absl::make_unique(1, 2);
+
+  P2.reset(new A(3, 4));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P2 = absl::make_unique(3, 4);
+
+  // No arguments to new expression
+  std::unique_ptr P3 = std::unique_ptr(new int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P3 = absl::make_unique();
+
+  P3.reset(new int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P3 = absl::make_unique();
+
+  // Nested parentheses
+  std::unique_ptr P4 = std::unique_ptr((new int(3)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P4 = absl::make_unique(3);
+
+  P4 = std::unique_ptr(((new int(4;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P4 = absl::make_unique(4);
+
+  P4.reset((new int(5)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P4 = absl::make_unique(5);
+
+  // With auto
+  auto P5 = std::unique_ptr(new int());
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: auto P5 = absl::make_unique();
+
+  {
+// No std
+using namespace std;
+unique_ptr Q = unique_ptr(new int());
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use absl::make_unique instead [abseil-make-unique]
+// CHECK-FIXES: unique_ptr Q = absl::make_unique();
+
+Q = unique_ptr(new int());
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use absl::make_unique instead [abseil-make-unique]
+// CHECK-FIXES: Q = absl::make_unique();
+  }
+
+  // Create the unique_ptr as a parameter to a function
+  expectPointer(std::unique_ptr(new int()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use absl::make_unique instead 

[PATCH] D59123: [analyzer] RetainCount: Fix a crash when a function follows retain/autorelease naming convention but takes no arguments.

[Devin Coughlin via Phabricator via cfe-commits]

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

Sigh. Looks good!


Repository:
  rC Clang

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

https://reviews.llvm.org/D59123



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

[PATCH] D58514: Avoid needlessly copying blocks that initialize or are assigned to local auto variables to the heap

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I remember this coming up 7-8 years ago.  I intentionally decided against doing 
a copy/release when assigning to `__weak` because if the block wasn't already 
guaranteed to be copied then it was probably better to crash than to silently 
assign a value that's about to be deallocated.  Note that copying the block we 
assign into `wb` doesn't change anything about the block stored in `b`.

I don't know why Chromium is building a weak reference to a block in the first 
place, but assuming they have a good reason to be doing it, they should fix 
their code to force a copy before forming a weak reference.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58514



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

[PATCH] D57435: [clang-tidy] Add abseil-wrap-unique check

[Ryan Piantedosi via Phabricator via cfe-commits]

Dosi-Dough updated this revision to Diff 190552.
Dosi-Dough marked 11 inline comments as done.
Dosi-Dough added a comment.
Herald added a subscriber: jdoerfert.
Herald added a project: clang.

added style and performance refactoring based on code reviews


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57435

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/WrapUniqueCheck.cpp
  clang-tidy/abseil/WrapUniqueCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-wrap-unique.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-wrap-unique.cpp

Index: test/clang-tidy/abseil-wrap-unique.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-wrap-unique.cpp
@@ -0,0 +1,87 @@
+// RUN: %check_clang_tidy %s abseil-wrap-unique %t
+
+namespace std {
+
+template 
+class default_delete {};
+
+template >
+class unique_ptr {
+public:
+  unique_ptr() {}
+  unique_ptr(type *ptr) {}
+  unique_ptr(const unique_ptr ) = delete;
+  unique_ptr(unique_ptr &) {}
+  ~unique_ptr() {}
+  type *() { return *ptr; }
+  type *operator->() { return ptr; }
+  type *release() { return ptr; }
+  void reset() {}
+  void reset(type *pt) {}
+  void reset(type pt) {}
+  unique_ptr =(unique_ptr &&) { return *this; }
+  template 
+  unique_ptr =(unique_ptr &&) { return *this; }
+
+private:
+  type *ptr;
+};
+}  // namespace std
+
+class A {
+ public:
+  static A* NewA() {
+return new A();
+  }
+
+ private:
+  A() {}
+};
+
+class B {
+ public:
+  static B* NewB(int bIn) {
+return new B();
+  }
+
+ private:
+  B() {}
+};
+
+struct C {
+  int x;
+  int y;
+};
+/*
+std::unique_ptr returnPointer() {
+  return std::unique_ptr(A::NewA());
+}
+*/
+void positives() {
+  std::unique_ptr a;
+  a.reset(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: a = absl::WrapUnique(A::NewA())
+  
+  std::unique_ptr b(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: Perfer absl::WrapUnique to constructing unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: auto b = absl::WrapUnique(A::NewA())
+
+  int cIn;
+  std::unique_ptr c(B::NewB(cIn));
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: Perfer absl::WrapUnique to constructing unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: auto c = absl::WrapUnique(B::NewB(cIn))
+
+  int dIn;
+  std::unique_ptr d;
+  d.reset(B::NewB(dIn));
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: d = absl::WrapUnique(B::NewB(dIn))
+  
+  auto e = std::unique_ptr(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: e = absl::WrapUnique(A::NewA())
+
+  //std::unique_ptr e(new int[2] {1,2});
+}
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -20,6 +20,7 @@
abseil-string-find-startswith
abseil-time-subtraction
abseil-upgrade-duration-conversions
+   abseil-wrap-unique
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-wrap-unique.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/abseil-wrap-unique.rst
@@ -0,0 +1,31 @@
+.. title:: clang-tidy - abseil-wrap-unique
+
+abseil-wrap-unique
+==
+Looks for instances of factory functions which uses a non-public constructor
+that returns a ``std::unqiue_ptr`` then recommends using 
+``absl::wrap_unique(new T(...))``.
+
+.. code-block:: c++
+  class A {
+  public:
+static A* NewA() { return new A(); }
+
+  private:
+A() {}
+  };
+
+  std::unique_ptr a;
+
+  // Original - reset called with a static function returning a std::unqiue_ptr
+  a.reset(A::NewA());
+
+  // Suggested - reset ptr with absl::WrapUnique
+  a = absl::WrapUnique(A::NewA());
+
+  // Original - std::unique_ptr initialized with static function
+  std::unique_ptr b(A::NewA());
+
+  // Suggested - initialize with absl::WrapUnique instead
+  auto b = absl::WrapUnique(A::NewA())
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -91,6 +91,13 @@
   Finds and fixes ``absl::Time`` subtraction expressions to do subtraction
   in the Time domain instead of the numeric domain.
 
+- New :doc:`abseil-wrap-unique
+  ` check.
+
+  Looks for instances of factory functions which uses a non-public constructor
+  that returns a ``std::unqiue_ptr`` then recommends using 
+  ``absl::wrap_unique(new T(...))``.
+
 - New :doc:`google-readability-avoid-underscore-in-googletest-name

[PATCH] D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington updated this revision to Diff 190549.
erik.pilkington marked 16 inline comments as done.
erik.pilkington added a comment.

Address review comments. Thanks!


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

https://reviews.llvm.org/D58797

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Analysis/bstring.c
  clang/test/Analysis/null-deref-ps-region.c
  clang/test/Analysis/pr22954.c
  clang/test/Analysis/string.c
  clang/test/Sema/builtin-object-size.c
  clang/test/Sema/builtins.c
  clang/test/Sema/transpose-memset.c
  clang/test/Sema/warn-fortify-source.c
  clang/test/Sema/warn-strncat-size.c

Index: clang/test/Sema/warn-strncat-size.c
===
--- clang/test/Sema/warn-strncat-size.c
+++ clang/test/Sema/warn-strncat-size.c
@@ -39,7 +39,7 @@
   strncat(dest, "AAA", sizeof(dest) - strlen(dest)); // expected-warning{{the value of the size argument in 'strncat' is too large, might lead to a buffer overflow}} expected-note {{change the argument to be the free space in the destination buffer minus the terminating null byte}}
 
   strncat((*s5)->f2[x], s2, sizeof(s2)); // expected-warning {{size argument in 'strncat' call appears to be size of the source}} expected-note {{change the argument to be the free space in the destination buffer minus the terminating null byte}}
-  strncat(s1+3, s2, sizeof(s2)); // expected-warning {{size argument in 'strncat' call appears to be size of the source}}
+  strncat(s1+3, s2, sizeof(s2)); // expected-warning {{size argument in 'strncat' call appears to be size of the source}} expected-warning {{strncat' size argument is too large; destination buffer has size 97, but size argument is 200}}
   strncat(s4.f1, s2, sizeof(s2)); // expected-warning {{size argument in 'strncat' call appears to be size of the source}} expected-note {{change the argument to be the free space in the destination buffer minus the terminating null byte}}
 }
 
Index: clang/test/Sema/warn-fortify-source.c
===
--- /dev/null
+++ clang/test/Sema/warn-fortify-source.c
@@ -0,0 +1,83 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_PASS_OBJECT_SIZE
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 %s -verify -DUSE_BUILTINS
+
+typedef unsigned long size_t;
+
+#if defined(USE_PASS_OBJECT_SIZE)
+void *memcpy(void *dst, const void *src, size_t c);
+static void *memcpy(void *dst __attribute__((pass_object_size(1))), const void *src, size_t c) __attribute__((overloadable)) __asm__("merp");
+static void *memcpy(void *const dst __attribute__((pass_object_size(1))), const void *src, size_t c) __attribute__((overloadable)) {
+  return 0;
+}
+#elif defined(USE_BUILTINS)
+#define memcpy(x,y,z) __builtin_memcpy(x,y,z)
+#else
+void *memcpy(void *dst, const void *src, size_t c);
+#endif
+
+void call_memcpy() {
+  char dst[10];
+  char src[20];
+  memcpy(dst, src, 20); // expected-warning {{memcpy' will always overflow; destination buffer has size 10, but size argument is 20}}
+}
+
+void call_memcpy_type() {
+  struct pair {
+int first;
+int second;
+  };
+  struct pair p;
+  char buf[20];
+  memcpy(, buf, 20);
+#ifdef USE_PASS_OBJECT_SIZE
+  // Use the more strict checking mode on the pass_object_size attribute:
+  // expected-warning@-3 {{memcpy' will always overflow; destination buffer has size 4, but size argument is 20}}
+#else
+  // Or just fallback to type 0:
+  // expected-warning@-6 {{memcpy' will always overflow; destination buffer has size 8, but size argument is 20}}
+#endif
+}
+
+void call_strncat() {
+  char s1[10], s2[20];
+  __builtin_strncat(s2, s1, 20);
+  __builtin_strncat(s1, s2, 20); // expected-warning {{'strncat' size argument is too large; destination buffer has size 10, but size argument is 20}}
+}
+
+void call_strncpy() {
+  char s1[10], s2[20];
+  __builtin_strncpy(s2, s1, 20);
+  __builtin_strncpy(s1, s2, 20); // expected-warning {{'strncpy' size argument is too large; destination buffer has size 10, but size argument is 20}}
+}
+
+void call_stpncpy() {
+  char s1[10], s2[20];
+  __builtin_stpncpy(s2, s1, 20);
+  __builtin_stpncpy(s1, s2, 20); // expected-warning {{'stpncpy' size argument is too large; destination buffer has size 10, but size argument is 20}}
+}
+
+void call_memmove() {
+  char s1[10], s2[20];
+  __builtin_memmove(s2, s1, 20);
+  __builtin_memmove(s1, s2, 20); // expected-warning {{'memmove' will always overflow; destination buffer has size 10, but size argument is 20}}
+}
+
+void call_memset() {
+  char buf[10];
+  __builtin_memset(buf, 0xff, 10);
+  __builtin_memset(buf, 0xff, 

[PATCH] D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:319
+// If the parameter has a pass_object_size attribute, then we should use
+// it's (potentially) more strict checking mode. Otherwise, conservatively
+// assume type 0.

aaron.ballman wrote:
> it's -> its
The opposite mistake as above, lol.



Comment at: clang/lib/Sema/SemaChecking.cpp:321
+// assume type 0.
+int BOSType = 0;
+if (auto *POS = FD->getParamDecl(ObjIdx)->getAttr())

george.burgess.iv wrote:
> Should this also try to consider `fortify_stdlib`?
I reverted `fortify_stdlib` in r356103 (although you couldn't possible know 
this, since that was after you made this comment ;)). We're going to use your 
`pass_object_size` attribute instead.



Comment at: clang/lib/Sema/SemaChecking.cpp:367
+DiagID = diag::warn_memcpy_chk_overflow;
+if (!evaluateObjectSize(TheCall->getNumArgs()-1) ||
+!evaluateSizeArg(TheCall->getNumArgs()-2))

aaron.ballman wrote:
> george.burgess.iv wrote:
> > nit: All of these cases (and the two lambdas above) look super similar. 
> > Might it be clearer to just set `SizeIndex` and `ObjectIndex` variables 
> > from here, and actually `evaluate` them before `UsedSize.ule(ComputedSize)`?
> > 
> > If not, I'm OK with this as-is.
> Formatting looks off here -- run the patch through clang-format?
Sure, I guess it's a bit cleaner to do that.



Comment at: clang/lib/Sema/SemaChecking.cpp:423
+  StringRef FunctionName = getASTContext().BuiltinInfo.getName(BuiltinID);
+  if (DiagID == diag::warn_memcpy_chk_overflow) {
+// __builtin___memcpy_chk -> memcpy

aaron.ballman wrote:
> Why don't we want to do this for the new fortify diagnostics?
No reason, just was trying to avoid changing the output of the `_chk` 
diagnostic. We might as as well though, it cleans up the diagnostic.



Comment at: clang/lib/Sema/SemaExpr.cpp:5929
 
+checkFortifiedBuiltinMemoryFunction(FDecl, TheCall);
+

george.burgess.iv wrote:
> Why isn't this in CheckBuiltinFunctionCall?
For the same reason I added the `bool` parameter to `getBuiltinCallee`, we 
don't usually consider calls to `__attribute__((overloadable))` be builtins, so 
we never reach `CheckBuiltinFunctionCall` for them. We're planning on using 
that attribute though:

```
int sprintf(__attribute__((pass_object_size(_FORTIFY_LEVEL))) char *buffer, 
const char *format, ...) 
  __attribute__((overloadable)) 
__asm__("___sprintf_pass_object_size_chk");
```


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

https://reviews.llvm.org/D58797



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

[PATCH] D58514: Avoid needlessly copying blocks that initialize or are assigned to local auto variables to the heap

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

Do you mean copying the block to the heap before assigning it to `wb` and 
releasing it after the assignment inside `bar`? Wouldn't the block assigned to 
`wb` be deallocated after the release?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58514



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

[PATCH] D58514: Avoid needlessly copying blocks that initialize or are assigned to local auto variables to the heap

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

In D58514#1428567 , @ahatanak wrote:

> Sorry, I misread the code. If you change the parameter type of `bar` to 
> `BlockTy`, the code crashes. If the type is `id`, it doesn't crash because 
> IRGen copies the block to the heap in `foo` before passing it to `bar`.


Okay, that's what I expected.  (You didn't misread, I had just mistyped :/.)

How insane would it be to add a copy/dispose pair around assignments of blocks 
to weak references?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58514



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

[PATCH] D58514: Avoid needlessly copying blocks that initialize or are assigned to local auto variables to the heap

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

Sorry, I misread the code. If you change the parameter type of `bar` to 
`BlockTy`, the code crashes. If the type is `id`, it doesn't crash because 
IRGen copies the block to the heap in `foo` before passing it to `bar`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58514



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

[PATCH] D59336: [clang-tidy] Disable google-runtime-int in Objective-C++ 

[Stephane Moore via Phabricator via cfe-commits]

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

Thanks for the review!




Comment at: clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp:57
   // Find all TypeLocs. The relevant Style Guide rule only applies to C++.
-  if (!getLangOpts().CPlusPlus)
+  // This check is also not applied in Objective-C++ sources as Objective-C
+  // often uses built-in integer types other than `int`.

gribozavr wrote:
> gribozavr wrote:
> > "Objective-C and Objective-C++"?  Similarly in release notes.
> I see -- should have read the previous sentence, which you are not changing.  
> Never mind.
Yup, the previous sentence indicates that the check targets C++ and this added 
sentence indicates that Objective-C++ is considered an exception.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59336



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

[PATCH] D59304: Fix invocation of Gold plugin with LTO after r355331

[Nemanja Ivanovic via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL356111: Fix invocation of Gold plugin with LTO after r355331 
(authored by nemanjai, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59304?vs=190429=190543#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59304

Files:
  cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
  cfe/trunk/test/Driver/cspgo-lto.c


Index: cfe/trunk/test/Driver/cspgo-lto.c
===
--- cfe/trunk/test/Driver/cspgo-lto.c
+++ cfe/trunk/test/Driver/cspgo-lto.c
@@ -0,0 +1,6 @@
+// RUN: touch %t.o
+//
+// RUN: %clang -target x86_64-unknown-linux -### %t.o -flto=thin \
+// RUN:   -fprofile-use 2>&1 | FileCheck %s
+
+// CHECK: -plugin-opt=cs-profile-path=default.profdata
Index: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
@@ -464,8 +464,12 @@
   CmdArgs.push_back(
   
Args.MakeArgString("-plugin-opt=cs-profile-path=default_%m.profraw"));
   } else if (ProfileUseArg) {
+SmallString<128> Path(
+ProfileUseArg->getNumValues() == 0 ? "" : ProfileUseArg->getValue());
+if (Path.empty() || llvm::sys::fs::is_directory(Path))
+  llvm::sys::path::append(Path, "default.profdata");
 CmdArgs.push_back(Args.MakeArgString(Twine("-plugin-opt=cs-profile-path=") 
+
- ProfileUseArg->getValue()));
+ Path));
   }
 
   // Need this flag to turn on new pass manager via Gold plugin.


Index: cfe/trunk/test/Driver/cspgo-lto.c
===
--- cfe/trunk/test/Driver/cspgo-lto.c
+++ cfe/trunk/test/Driver/cspgo-lto.c
@@ -0,0 +1,6 @@
+// RUN: touch %t.o
+//
+// RUN: %clang -target x86_64-unknown-linux -### %t.o -flto=thin \
+// RUN:   -fprofile-use 2>&1 | FileCheck %s
+
+// CHECK: -plugin-opt=cs-profile-path=default.profdata
Index: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
@@ -464,8 +464,12 @@
   CmdArgs.push_back(
   Args.MakeArgString("-plugin-opt=cs-profile-path=default_%m.profraw"));
   } else if (ProfileUseArg) {
+SmallString<128> Path(
+ProfileUseArg->getNumValues() == 0 ? "" : ProfileUseArg->getValue());
+if (Path.empty() || llvm::sys::fs::is_directory(Path))
+  llvm::sys::path::append(Path, "default.profdata");
 CmdArgs.push_back(Args.MakeArgString(Twine("-plugin-opt=cs-profile-path=") +
- ProfileUseArg->getValue()));
+ Path));
   }
 
   // Need this flag to turn on new pass manager via Gold plugin.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r356111 - Fix invocation of Gold plugin with LTO after r355331

[Nemanja Ivanovic via cfe-commits]

Author: nemanjai
Date: Wed Mar 13 16:54:52 2019
New Revision: 356111

URL: http://llvm.org/viewvc/llvm-project?rev=356111=rev
Log:
Fix invocation of Gold plugin with LTO after r355331

The above commit breaks the usage of PGO and LTO when -fprofile-use is
supplied without a path. This patch changes the usage of this argument
to be inline with its use in addPGOAndCoverageFlags().

Differential revision: https://reviews.llvm.org/D59304

Added:
cfe/trunk/test/Driver/cspgo-lto.c
Modified:
cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp?rev=356111=356110=356111=diff
==
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp Wed Mar 13 16:54:52 2019
@@ -464,8 +464,12 @@ void tools::AddGoldPlugin(const ToolChai
   CmdArgs.push_back(
   
Args.MakeArgString("-plugin-opt=cs-profile-path=default_%m.profraw"));
   } else if (ProfileUseArg) {
+SmallString<128> Path(
+ProfileUseArg->getNumValues() == 0 ? "" : ProfileUseArg->getValue());
+if (Path.empty() || llvm::sys::fs::is_directory(Path))
+  llvm::sys::path::append(Path, "default.profdata");
 CmdArgs.push_back(Args.MakeArgString(Twine("-plugin-opt=cs-profile-path=") 
+
- ProfileUseArg->getValue()));
+ Path));
   }
 
   // Need this flag to turn on new pass manager via Gold plugin.

Added: cfe/trunk/test/Driver/cspgo-lto.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cspgo-lto.c?rev=356111=auto
==
--- cfe/trunk/test/Driver/cspgo-lto.c (added)
+++ cfe/trunk/test/Driver/cspgo-lto.c Wed Mar 13 16:54:52 2019
@@ -0,0 +1,6 @@
+// RUN: touch %t.o
+//
+// RUN: %clang -target x86_64-unknown-linux -### %t.o -flto=thin \
+// RUN:   -fprofile-use 2>&1 | FileCheck %s
+
+// CHECK: -plugin-opt=cs-profile-path=default.profdata


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

[PATCH] D58514: Avoid needlessly copying blocks that initialize or are assigned to local auto variables to the heap

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

In D58514#1428520 , @ahatanak wrote:

> That code doesn't crash because the block is retained at the entry of `bar` 
> and ARC optimizer doesn't remove the retain/release pairs in `bar`.


Oops, I meant:

  typedef void (^BlockTy)();
  
  BlockTy sb;
  __weak BlockTy wb;
  
  void bar(BlockTy b) {
wb = b;
sb = b;
  }
  
  void foo(id a) {
bar(^{ NSLog(@"foo %@", a); });
  }
  
  int main() {
auto x = [NSObject new];
foo(x);
sb();
wb();
return 0;
  }

Is it the same?  Does `b` get retained at the entry to `bar()`?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58514



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

[PATCH] D58514: Avoid needlessly copying blocks that initialize or are assigned to local auto variables to the heap

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

In D58514#1428495 , @dexonsmith wrote:

> In D58514#1428434 , @ahatanak wrote:
>
> > Seems like the chromium code is valid and shouldn't crash. John/Erik what 
> > do you think? The following code also crashes with this patch applied.
> >
> >   typedef void (^BlockTy)();
> >  
> >   BlockTy sb;
> >   __weak BlockTy wb;
> >  
> >   void foo(id a) {
> > auto b = ^{ NSLog(@"foo %@", a); };
> > wb = b; // block isn't copied to the heap.
> > sb = b; // block is copied to the heap.
> >   }
> >  
> >   int main() {
> > auto x = [NSObject new];
> > foo(x);
> > sb();
> > wb();
> > return 0;
> >   }
> >
>
>
> The assignment to `wb` seems like an escape of some sort.  What happens for 
> this similar code?
>
>   typedef void (^BlockTy)();
>  
>   BlockTy sb;
>   __weak BlockTy wb;
>  
>   void bar(id b) {
> wb = b;
> sb = b;
>   }
>  
>   void foo(id a) {
> bar(^{ NSLog(@"foo %@", a); });
>   }
>  
>   int main() {
> auto x = [NSObject new];
> foo(x);
> sb();
> wb();
> return 0;
>   }
>


That code doesn't crash because the block is retained at the entry of `bar` and 
ARC optimizer doesn't remove the retain/release pairs in `bar`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58514



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

[PATCH] D58514: Avoid needlessly copying blocks that initialize or are assigned to local auto variables to the heap

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

In D58514#1428434 , @ahatanak wrote:

> Seems like the chromium code is valid and shouldn't crash. John/Erik what do 
> you think? The following code also crashes with this patch applied.
>
>   typedef void (^BlockTy)();
>  
>   BlockTy sb;
>   __weak BlockTy wb;
>  
>   void foo(id a) {
> auto b = ^{ NSLog(@"foo %@", a); };
> wb = b; // block isn't copied to the heap.
> sb = b; // block is copied to the heap.
>   }
>  
>   int main() {
> auto x = [NSObject new];
> foo(x);
> sb();
> wb();
> return 0;
>   }
>


The assignment to `wb` seems like an escape of some sort.  What happens for 
this similar code?

  typedef void (^BlockTy)();
  
  BlockTy sb;
  __weak BlockTy wb;
  
  void bar(id b) {
wb = b;
sb = b;
  }
  
  void foo(id a) {
bar(^{ NSLog(@"foo %@", a); });
  }
  
  int main() {
auto x = [NSObject new];
foo(x);
sb();
wb();
return 0;
  }


Repository:
  rC Clang

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

https://reviews.llvm.org/D58514



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

[PATCH] D59223: Objective-C++11: Support static_assert() in @interface/@implementation ivar lists and method declarations

[Nico Weber via Phabricator via cfe-commits]

thakis updated this revision to Diff 190534.
thakis edited the summary of this revision.
thakis added a comment.

extension


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

https://reviews.llvm.org/D59223

Files:
  clang/include/clang/Basic/Features.def
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseObjc.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Parser/objc-static-assert.m
  clang/test/Parser/objc-static-assert.mm

Index: clang/test/Parser/objc-static-assert.mm
===
--- /dev/null
+++ clang/test/Parser/objc-static-assert.mm
@@ -0,0 +1,74 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-root-class %s
+// RUN: %clang_cc1 -std=c++98 -fsyntax-only -verify -Wno-objc-root-class %s
+
+#if !__has_feature(objc_c_static_assert)
+#error failed
+#endif
+
+#if __cplusplus >= 201103L
+
+#if !__has_feature(objc_cxx_static_assert)
+#error failed
+#endif
+
+// C++11
+
+@interface A {
+  int a;
+  static_assert(1, "");
+  _Static_assert(1, "");
+
+  static_assert(0, ""); // expected-error {{static_assert failed}}
+  _Static_assert(0, ""); // expected-error {{static_assert failed}}
+
+  static_assert(a, ""); // expected-error {{static_assert expression is not an integral constant expression}}
+  static_assert(sizeof(a) == 4, "");
+  static_assert(sizeof(a) == 3, ""); // expected-error {{static_assert failed}}
+}
+
+static_assert(1, "");
+_Static_assert(1, "");
+
+- (void)f;
+@end
+
+@implementation A {
+  int b;
+  static_assert(1, "");
+  _Static_assert(1, "");
+  static_assert(sizeof(b) == 4, "");
+  static_assert(sizeof(b) == 3, ""); // expected-error {{static_assert failed}}
+}
+
+static_assert(1, "");
+
+- (void)f {
+  static_assert(1, "");
+}
+@end
+
+@interface B
+@end
+
+@interface B () {
+  int b;
+  static_assert(sizeof(b) == 4, "");
+  static_assert(sizeof(b) == 3, ""); // expected-error {{static_assert failed}}
+}
+@end
+
+#else
+
+#if __has_feature(objc_cxx_static_assert)
+#error failed
+#endif
+
+// C++98
+@interface A {
+  int a;
+  static_assert(1, ""); // expected-error {{type name requires a specifier or qualifier}} expected-error{{expected parameter declarator}} expected-error {{expected ')'}} expected-note {{to match this '('}}
+  _Static_assert(1, "");
+  _Static_assert(0, ""); // expected-error {{static_assert failed}}
+}
+@end
+#endif
Index: clang/test/Parser/objc-static-assert.m
===
--- /dev/null
+++ clang/test/Parser/objc-static-assert.m
@@ -0,0 +1,54 @@
+// RUN: %clang_cc1 -fobjc-runtime=macosx-fragile -fsyntax-only -verify -Wno-objc-root-class %s
+// RUN: %clang_cc1 -std=c89 -fobjc-runtime=macosx-fragile -fsyntax-only -verify -Wno-objc-root-class %s
+
+
+#if __STDC_VERSION__ >= 201112L
+
+#if !__has_feature(objc_c_static_assert)
+#error failed
+#endif
+
+#if !__has_extension(objc_c_static_assert)
+#error failed
+#endif
+
+@interface A {
+  int a;
+  _Static_assert(1, "");
+  _Static_assert(0, ""); // expected-error {{static_assert failed}}
+
+  _Static_assert(a, ""); // expected-error {{use of undeclared identifier 'a'}}
+  _Static_assert(sizeof(a), ""); // expected-error {{use of undeclared identifier 'a'}}
+}
+
+_Static_assert(1, "");
+
+@end
+
+struct S {
+  @defs(A);
+};
+
+#else
+
+// _Static_assert is available before C11 as an extension, but -pedantic
+// warns on it.
+#if __has_feature(objc_c_static_assert)
+#error failed
+#endif
+
+#if !__has_extension(objc_c_static_assert)
+#error failed
+#endif
+
+@interface A {
+  int a;
+  _Static_assert(1, "");
+  _Static_assert(0, ""); // expected-error {{static_assert failed}}
+}
+
+_Static_assert(1, "");
+
+@end
+
+#endif
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -2998,7 +2998,6 @@
 
 // These shouldn't make it here.
 case Decl::ObjCAtDefsField:
-case Decl::ObjCIvar:
   llvm_unreachable("forming non-member reference to ivar?");
 
 // Enum constants are always r-values and never references.
@@ -3016,6 +3015,7 @@
 // exist in the high-level semantics.
 case Decl::Field:
 case Decl::IndirectField:
+case Decl::ObjCIvar:
   assert(getLangOpts().CPlusPlus &&
  "building reference to field in C?");
 
Index: clang/lib/Parse/ParseObjc.cpp
===
--- clang/lib/Parse/ParseObjc.cpp
+++ clang/lib/Parse/ParseObjc.cpp
@@ -623,6 +623,8 @@
 }
 // Ignore excess semicolons.
 if (Tok.is(tok::semi)) {
+  // FIXME: This should use ConsumeExtraSemi() for extraneous semicolons,
+  // to make -Wextra-semi diagnose them.
   ConsumeToken();
   continue;
 }
@@ -646,7 +648,19 @@
   // erroneous r_brace would cause an infinite loop if not handled here.
   if (Tok.is(tok::r_brace))
 break;
+
   ParsedAttributesWithRange 

[PATCH] D59223: Objective-C++11: Support static_assert() in @interface/@implementation ivar lists and method declarations

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Ah, got it, thanks for explaining!


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

https://reviews.llvm.org/D59223



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

[PATCH] D59264: [Driver] Support compiler-rt crtbegin.o/crtend.o for Linux

[Manoj Gupta via Phabricator via cfe-commits]

manojgupta added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:447
   const char *crtbegin;
-  if (Args.hasArg(options::OPT_static))
-crtbegin = isAndroid ? "crtbegin_static.o" : "crtbeginT.o";
-  else if (Args.hasArg(options::OPT_shared))
-crtbegin = isAndroid ? "crtbegin_so.o" : "crtbeginS.o";
-  else if (IsPIE || IsStaticPIE)
-crtbegin = isAndroid ? "crtbegin_dynamic.o" : "crtbeginS.o";
-  else
-crtbegin = isAndroid ? "crtbegin_dynamic.o" : "crtbegin.o";
-
-  if (HasCRTBeginEndFiles)
+  if (ToolChain.GetRuntimeLibType(Args) == ToolChain::RLT_CompilerRT &&
+  !isAndroid) {

phosek wrote:
> manojgupta wrote:
> > This is currently unconditional on using compiler-rt. Given that 
> > compiler-rt provides the crt*.o files only under the CMake flag 
> > COMPILER_RT_BUILD_CRT, shouldn't there be an equivalent clang CMake flag to 
> > conditionally enable this.
> `COMPILER_RT_BUILD_CRT` is `ON` by default now in D28791, but even if we 
> change that, I think this logic should be independent since you should be 
> able to build Clang separately from compiler-rt.
Since it is possible to build compiler-rt without CRT files, I believe that 
clang should also be able to use compiler-rt without compiler-rt provided CRT 
files.
Using them unconditionally will also break the case of using newer clang with 
older compiler-rt.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59264



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

[PATCH] D58514: Avoid needlessly copying blocks that initialize or are assigned to local auto variables to the heap

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

Seems like the chromium code is valid and shouldn't crash. John/Erik what do 
you think? The following code also crashes with this patch applied.

  typedef void (^BlockTy)();
  
  BlockTy sb;
  __weak BlockTy wb;
  
  void foo(id a) {
auto b = ^{ NSLog(@"foo %@", a); };
wb = b; // block isn't copied to the heap.
sb = b; // block is copied to the heap.
  }
  
  int main() {
auto x = [NSObject new];
foo(x);
sb();
wb();
return 0;
  }


Repository:
  rC Clang

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

https://reviews.llvm.org/D58514



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

[PATCH] D59223: Objective-C++11: Support static_assert() in @interface/@implementation ivar lists and method declarations

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added inline comments.



Comment at: clang/include/clang/Basic/Features.def:121
 FEATURE(objc_class_property, LangOpts.ObjC)
+FEATURE(objc_c_static_assert, true)
+FEATURE(objc_cxx_static_assert, LangOpts.CPlusPlus11)

thakis wrote:
> erik.pilkington wrote:
> > We should only claim `_Static_assert` is a feature in languages where its 
> > actually a formal language feature (C11), you can also use EXTENSION to 
> > indicate what languages we accept it in (see the comment in the file 
> > header).
> Sorry, I don't follow. clang accepts `_Static_assert` in C89. You're saying 
> you want `__has_feature(objc_c_static_assert)` to be false there? I'm 
> guessing because -pedantic warns on it?
> 
> But if we do this, there's no way to differentiate clang-in-std=c89 mode with 
> this patch from clang-without-this-patch, is there? Is that what you want? 
> i.e. replace `true` with `LangOpts.C11`?
> 
> (For now, I updated the test to check the __has_feature() is true in c89 too.)
> 
> (That seems like another argument to restore the single feature to me since 
> this working in objc is orthogonal on if the language version supports static 
> asserts.)
> Sorry, I don't follow. clang accepts _Static_assert in C89. You're saying you 
> want __has_feature(objc_c_static_assert) to be false there? I'm guessing 
> because -pedantic warns on it?

Yeah, `__has_feature` is meant to check for standard language features. 
`__has_extension` checks whether clang will accept it, regardless of what 
respective standard says. So `_Static_assert` here is a "standard" language 
feature in ObjC + C11 and an extension in ObjC + C99. I'm suggesting you write 
this like:

```
FEATURE(objc_c_static_assert, LangOpts.C11)
FEATURE(objc_cxx_static_assert, LangOpts.CPlusPlus11)
EXTENSION(objc_c_static_assert, true)
```

Just like we do now for c_static_assert in general:
```
FEATURE(c_static_assert, LangOpts.C11)
EXTENSION(c_static_assert, true)
```

That way, `__has_extension(objc_c_static_assert)` is always true (with a new 
compiler), and `__has_feature` only deals with standard language features. Does 
that make sense?


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

https://reviews.llvm.org/D59223



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

[PATCH] D59223: Objective-C++11: Support static_assert() in @interface/@implementation ivar lists and method declarations

[Nico Weber via Phabricator via cfe-commits]

thakis marked an inline comment as done.
thakis added inline comments.



Comment at: clang/include/clang/Basic/Features.def:121
 FEATURE(objc_class_property, LangOpts.ObjC)
+FEATURE(objc_c_static_assert, true)
+FEATURE(objc_cxx_static_assert, LangOpts.CPlusPlus11)

erik.pilkington wrote:
> We should only claim `_Static_assert` is a feature in languages where its 
> actually a formal language feature (C11), you can also use EXTENSION to 
> indicate what languages we accept it in (see the comment in the file header).
Sorry, I don't follow. clang accepts `_Static_assert` in C89. You're saying you 
want `__has_feature(objc_c_static_assert)` to be false there? I'm guessing 
because -pedantic warns on it?

But if we do this, there's no way to differentiate clang-in-std=c89 mode with 
this patch from clang-without-this-patch, is there? Is that what you want? i.e. 
replace `true` with `LangOpts.C11`?

(For now, I updated the test to check the __has_feature() is true in c89 too.)

(That seems like another argument to restore the single feature to me since 
this working in objc is orthogonal on if the language version supports static 
asserts.)


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

https://reviews.llvm.org/D59223



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

[PATCH] D59336: [clang-tidy] Disable google-runtime-int in Objective-C++ 

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr marked an inline comment as done.
gribozavr added inline comments.



Comment at: clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp:57
   // Find all TypeLocs. The relevant Style Guide rule only applies to C++.
-  if (!getLangOpts().CPlusPlus)
+  // This check is also not applied in Objective-C++ sources as Objective-C
+  // often uses built-in integer types other than `int`.

gribozavr wrote:
> "Objective-C and Objective-C++"?  Similarly in release notes.
I see -- should have read the previous sentence, which you are not changing.  
Never mind.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59336



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

[PATCH] D59336: [clang-tidy] Disable google-runtime-int in Objective-C++ 

[Dmitri Gribenko via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp:57
   // Find all TypeLocs. The relevant Style Guide rule only applies to C++.
-  if (!getLangOpts().CPlusPlus)
+  // This check is also not applied in Objective-C++ sources as Objective-C
+  // often uses built-in integer types other than `int`.

"Objective-C and Objective-C++"?  Similarly in release notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59336



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

LLVM buildmaster will be updated and restarted tonight

[Galina Kistanova via cfe-commits]

 Hello everyone,

LLVM buildmaster will be updated and restarted after 7PM Pacific time today.

Thanks

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

[PATCH] D59223: Objective-C++11: Support static_assert() in @interface/@implementation ivar lists and method declarations

[Nico Weber via Phabricator via cfe-commits]

thakis updated this revision to Diff 190521.
thakis added a comment.

c89


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

https://reviews.llvm.org/D59223

Files:
  clang/include/clang/Basic/Features.def
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseObjc.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Parser/objc-static-assert.m
  clang/test/Parser/objc-static-assert.mm

Index: clang/test/Parser/objc-static-assert.mm
===
--- /dev/null
+++ clang/test/Parser/objc-static-assert.mm
@@ -0,0 +1,74 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-root-class %s
+// RUN: %clang_cc1 -std=c++98 -fsyntax-only -verify -Wno-objc-root-class %s
+
+#if !__has_feature(objc_c_static_assert)
+#error failed
+#endif
+
+#if __cplusplus >= 201103L
+
+#if !__has_feature(objc_cxx_static_assert)
+#error failed
+#endif
+
+// C++11
+
+@interface A {
+  int a;
+  static_assert(1, "");
+  _Static_assert(1, "");
+
+  static_assert(0, ""); // expected-error {{static_assert failed}}
+  _Static_assert(0, ""); // expected-error {{static_assert failed}}
+
+  static_assert(a, ""); // expected-error {{static_assert expression is not an integral constant expression}}
+  static_assert(sizeof(a) == 4, "");
+  static_assert(sizeof(a) == 3, ""); // expected-error {{static_assert failed}}
+}
+
+static_assert(1, "");
+_Static_assert(1, "");
+
+- (void)f;
+@end
+
+@implementation A {
+  int b;
+  static_assert(1, "");
+  _Static_assert(1, "");
+  static_assert(sizeof(b) == 4, "");
+  static_assert(sizeof(b) == 3, ""); // expected-error {{static_assert failed}}
+}
+
+static_assert(1, "");
+
+- (void)f {
+  static_assert(1, "");
+}
+@end
+
+@interface B
+@end
+
+@interface B () {
+  int b;
+  static_assert(sizeof(b) == 4, "");
+  static_assert(sizeof(b) == 3, ""); // expected-error {{static_assert failed}}
+}
+@end
+
+#else
+
+#if __has_feature(objc_cxx_static_assert)
+#error failed
+#endif
+
+// C++98
+@interface A {
+  int a;
+  static_assert(1, ""); // expected-error {{type name requires a specifier or qualifier}} expected-error{{expected parameter declarator}} expected-error {{expected ')'}} expected-note {{to match this '('}}
+  _Static_assert(1, "");
+  _Static_assert(0, ""); // expected-error {{static_assert failed}}
+}
+@end
+#endif
Index: clang/test/Parser/objc-static-assert.m
===
--- /dev/null
+++ clang/test/Parser/objc-static-assert.m
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -fobjc-runtime=macosx-fragile -fsyntax-only -verify -Wno-objc-root-class %s
+// RUN: %clang_cc1 -std=c89 -fobjc-runtime=macosx-fragile -fsyntax-only -verify -Wno-objc-root-class %s
+
+#if !__has_feature(objc_c_static_assert)
+#error failed
+#endif
+
+@interface A {
+  int a;
+  _Static_assert(1, "");
+  _Static_assert(0, ""); // expected-error {{static_assert failed}}
+
+  _Static_assert(a, ""); // expected-error {{use of undeclared identifier 'a'}}
+  _Static_assert(sizeof(a), ""); // expected-error {{use of undeclared identifier 'a'}}
+}
+
+_Static_assert(1, "");
+
+@end
+
+struct S {
+  @defs(A);
+};
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -2998,7 +2998,6 @@
 
 // These shouldn't make it here.
 case Decl::ObjCAtDefsField:
-case Decl::ObjCIvar:
   llvm_unreachable("forming non-member reference to ivar?");
 
 // Enum constants are always r-values and never references.
@@ -3016,6 +3015,7 @@
 // exist in the high-level semantics.
 case Decl::Field:
 case Decl::IndirectField:
+case Decl::ObjCIvar:
   assert(getLangOpts().CPlusPlus &&
  "building reference to field in C?");
 
Index: clang/lib/Parse/ParseObjc.cpp
===
--- clang/lib/Parse/ParseObjc.cpp
+++ clang/lib/Parse/ParseObjc.cpp
@@ -623,6 +623,8 @@
 }
 // Ignore excess semicolons.
 if (Tok.is(tok::semi)) {
+  // FIXME: This should use ConsumeExtraSemi() for extraneous semicolons,
+  // to make -Wextra-semi diagnose them.
   ConsumeToken();
   continue;
 }
@@ -646,7 +648,19 @@
   // erroneous r_brace would cause an infinite loop if not handled here.
   if (Tok.is(tok::r_brace))
 break;
+
   ParsedAttributesWithRange attrs(AttrFactory);
+
+  // Since we call ParseDeclarationOrFunctionDefinition() instead of
+  // ParseExternalDeclaration() below (so that this doesn't parse nested
+  // @interfaces), this needs to duplicate some code from the latter.
+  if (Tok.isOneOf(tok::kw_static_assert, tok::kw__Static_assert)) {
+SourceLocation DeclEnd;
+allTUVariables.push_back(
+ParseDeclaration(DeclaratorContext::FileContext, DeclEnd, attrs));
+continue;
+  }
+
   

Re: [PATCH] D59336: [clang-tidy] Disable google-runtime-int in Objective-C++ 

[Stephane Moore via cfe-commits]

I was uncertain whether or not this change required new tests. A previous
change which disabled this check in languages other than C++ did not
include additional tests:
https://github.com/llvm/llvm-project/commit/ec3e5d6fd87862eb77a2b0320d79b9a4427d39df#diff-a491be84e1b831aeaea56c39b5eb898c

If there is a preference to add tests for this change, I can do so.

On Wed, Mar 13, 2019 at 3:21 PM Roman Lebedev  wrote:

> test?
>
> On Thu, Mar 14, 2019 at 1:17 AM Stephane Moore via Phabricator via
> cfe-commits  wrote:
> >
> > stephanemoore created this revision.
> > Herald added subscribers: cfe-commits, jdoerfert, xazax.hun.
> > Herald added a project: clang.
> >
> > In contrast to Google C++, Objective-C often uses built-in integer types
> > other than `int`. In fact, the Objective-C runtime itself defines the
> > types NSInteger¹ and NSUInteger² which are variant types depending on
> > the target architecture. The Objective-C style guide indicates that
> > usage of system types with variant sizes is appropriate when handling
> > values provided by system interfaces³. Objective-C++ is commonly the
> > result of conversion from Objective-C to Objective-C++ for the purpose
> > of integrating C++ functionality. The opposite of Objective-C++ being
> > used to expose Objective-C functionality to C++ is less common,
> > potentially because Objective-C has a signficantly more uneven presence
> > on different platforms compared to C++. This generally predisposes
> > Objective-C++ to commonly being more Objective-C than C++. Forcing
> > Objective-C++ developers to perform conversions between variant system
> types
> > and fixed size integer types depending on target architecture when
> > Objective-C++ commonly uses variant system types from Objective-C is
> > likely to lead to more bugs and overhead than benefit. For that reason,
> > this change proposes to disable google-runtime-int in Objective-C++.
> >
> > [1]
> https://developer.apple.com/documentation/objectivec/nsinteger?language=objc
> > [2]
> https://developer.apple.com/documentation/objectivec/nsuinteger?language=objc
> > [3] "Types long, NSInteger, NSUInteger, and CGFloat vary in size between
> > 32- and 64-bit builds. Use of these types is appropriate when handling
> > values exposed by system interfaces, but they should be avoided for most
> > other computations."
> >
> https://github.com/google/styleguide/blob/gh-pages/objcguide.md#types-with-inconsistent-sizes
> >
> >
> > Repository:
> >   rG LLVM Github Monorepo
> >
> > https://reviews.llvm.org/D59336
> >
> > Files:
> >   clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
> >
> >
> > Index: clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
> > ===
> > --- clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
> > +++ clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
> > @@ -54,7 +54,9 @@
> >
> >  void IntegerTypesCheck::registerMatchers(MatchFinder *Finder) {
> >// Find all TypeLocs. The relevant Style Guide rule only applies to
> C++.
> > -  if (!getLangOpts().CPlusPlus)
> > +  // This check is also not applied in Objective-C++ sources as
> Objective-C
> > +  // often uses built-in integer types other than `int`.
> > +  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC)
> >  return;
> >// Match any integer types, unless they are passed to a printf-based
> API:
> >//
> >
> >
> > ___
> > cfe-commits mailing list
> > cfe-commits@lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59336: [clang-tidy] Disable google-runtime-int in Objective-C++ 

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

Let me know if you think this change would benefit from added tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59336



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

[PATCH] D59336: [clang-tidy] Disable google-runtime-int in Objective-C++ 

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 190520.
stephanemoore added a comment.

Document change in release notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59336

Files:
  clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst


Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -115,6 +115,9 @@
   `CommentUserDefiniedLiterals`, `CommentStringLiterals`,
   `CommentCharacterLiterals` & `CommentNullPtrs` options.
 
+- The :doc:`google-runtime-int `
+  check has been disabled in Objective-C++.
+
 - The `Acronyms` and `IncludeDefaultAcronyms` options for the
   :doc:`objc-property-declaration 
`
   check have been removed.
Index: clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
===
--- clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
+++ clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
@@ -54,7 +54,9 @@
 
 void IntegerTypesCheck::registerMatchers(MatchFinder *Finder) {
   // Find all TypeLocs. The relevant Style Guide rule only applies to C++.
-  if (!getLangOpts().CPlusPlus)
+  // This check is also not applied in Objective-C++ sources as Objective-C
+  // often uses built-in integer types other than `int`.
+  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC)
 return;
   // Match any integer types, unless they are passed to a printf-based API:
   //


Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -115,6 +115,9 @@
   `CommentUserDefiniedLiterals`, `CommentStringLiterals`,
   `CommentCharacterLiterals` & `CommentNullPtrs` options.
 
+- The :doc:`google-runtime-int `
+  check has been disabled in Objective-C++.
+
 - The `Acronyms` and `IncludeDefaultAcronyms` options for the
   :doc:`objc-property-declaration `
   check have been removed.
Index: clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
===
--- clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
+++ clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
@@ -54,7 +54,9 @@
 
 void IntegerTypesCheck::registerMatchers(MatchFinder *Finder) {
   // Find all TypeLocs. The relevant Style Guide rule only applies to C++.
-  if (!getLangOpts().CPlusPlus)
+  // This check is also not applied in Objective-C++ sources as Objective-C
+  // often uses built-in integer types other than `int`.
+  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC)
 return;
   // Match any integer types, unless they are passed to a printf-based API:
   //
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r354843 - [CGDebugInfo] Set NonTrivial DIFlag to a c++ record if it's not trivial

[Reid Kleckner via cfe-commits]

Aaron, I've followed both links you point to to answer the questions being
asked, and to me they don't seem to contain obvious answers. I understand
that repeatedly answering the same question over and over again is tedious
and we are all busy, but please remind us why both DIFlagTrivial and
DIFlagNonTrivial are necessary. IIRC last time I looked at this the
DIFlagTrivial flag didn't exist, and it was added since we first discussed
this change.

On Wed, Mar 13, 2019 at 1:04 PM Aaron Smith 
wrote:

> Hi Paul,
>
>
>
> There are additional tests here that may answer your questions,
>
> https://reviews.llvm.org/rGe8475f78e2634d5d348d7ad746efc1e6526e72f5
>
>
>
> Aaron
>
>
>
> *From: *"paul.robin...@sony.com" 
> *Date: *Wednesday, March 13, 2019 at 12:49 PM
> *To: *Aaron Smith , "dblai...@gmail.com" <
> dblai...@gmail.com>, "r...@google.com" , "apra...@apple.com"
> , "jdevliegh...@apple.com" 
> *Cc: *"cfe-commits@lists.llvm.org" 
> *Subject: *RE: r354843 - [CGDebugInfo] Set NonTrivial DIFlag to a c++
> record if it's not trivial
>
>
>
> Hi Aaron,
>
> I think I am less clever than David, and it's not clear what the
> difference is between the two flags. In the review, Zach asked the question
> but I did not see a straight answer. What do these flags actually mean?
> What is the third state, with both flags off, implying not Trivial and not
> NonTrivial?  (At the very least it seems that the names could be improved.)
>
> Thanks,
>
> --paulr
>
>
>
> *From:* cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] *On
> Behalf Of *Aaron Smith via cfe-commits
> *Sent:* Monday, March 04, 2019 6:22 PM
> *To:* David Blaikie; Reid Kleckner; Adrian Prantl; Jonas Devlieghere
> *Cc:* cfe-commits@lists.llvm.org
> *Subject:* Re: r354843 - [CGDebugInfo] Set NonTrivial DIFlag to a c++
> record if it's not trivial
>
>
>
> Hi David, I can take a look at adding another test. Please read the code
> review which answers your question. A new flag is required. Thanks.
>
>
> --
>
> *From:* David Blaikie 
> *Sent:* Tuesday, March 5, 2019 12:51:27 AM
> *To:* Aaron Smith; Reid Kleckner; Adrian Prantl; Jonas Devlieghere
> *Cc:* cfe-commits@lists.llvm.org
> *Subject:* Re: r354843 - [CGDebugInfo] Set NonTrivial DIFlag to a c++
> record if it's not trivial
>
>
>
> Hi Aaron,
>
> I don't see any mention of this in D44406 - so it might have been good to
> have a separate review for this (or included this in the review of D44406,
> which I think is possible with the monorepo).
>
> Specifically - this change is missing test coverage (there should be a
> clang test that goes from C++ source code to LLVM IR & demonstrates the
> flag being emitted into the IR, etc).
>
> Also - what's the reason the non-triviality can't be implied by the
> absence of the trivial flag? (or the other way around) - the flags seem
> redundant with one another.
>
> On Mon, Feb 25, 2019 at 8:02 PM Aaron Smith via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> Author: asmith
> Date: Mon Feb 25 19:49:05 2019
> New Revision: 354843
>
> URL: http://llvm.org/viewvc/llvm-project?rev=354843=rev
> 
> Log:
> [CGDebugInfo] Set NonTrivial DIFlag to a c++ record if it's not trivial
>
> This goes with https://reviews.llvm.org/D44406
> 
>
>
> Modified:
> cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>
> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=354843=354842=354843=diff
> 
>
> ==
> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Mon Feb 25 19:49:05 2019
> @@ -3031,6 +3031,8 @@ llvm::DICompositeType *CGDebugInfo::Crea
>  // Record if a C++ record is trivial type.
>  if (CXXRD->isTrivial())
>Flags |= llvm::DINode::FlagTrivial;
> +else
> +  Flags |= llvm::DINode::FlagNonTrivial;
>}
>
>llvm::DICompositeType *RealDecl =
> 

Re: [PATCH] D59336: [clang-tidy] Disable google-runtime-int in Objective-C++ 

[Roman Lebedev via cfe-commits]

test?

On Thu, Mar 14, 2019 at 1:17 AM Stephane Moore via Phabricator via
cfe-commits  wrote:
>
> stephanemoore created this revision.
> Herald added subscribers: cfe-commits, jdoerfert, xazax.hun.
> Herald added a project: clang.
>
> In contrast to Google C++, Objective-C often uses built-in integer types
> other than `int`. In fact, the Objective-C runtime itself defines the
> types NSInteger¹ and NSUInteger² which are variant types depending on
> the target architecture. The Objective-C style guide indicates that
> usage of system types with variant sizes is appropriate when handling
> values provided by system interfaces³. Objective-C++ is commonly the
> result of conversion from Objective-C to Objective-C++ for the purpose
> of integrating C++ functionality. The opposite of Objective-C++ being
> used to expose Objective-C functionality to C++ is less common,
> potentially because Objective-C has a signficantly more uneven presence
> on different platforms compared to C++. This generally predisposes
> Objective-C++ to commonly being more Objective-C than C++. Forcing
> Objective-C++ developers to perform conversions between variant system types
> and fixed size integer types depending on target architecture when
> Objective-C++ commonly uses variant system types from Objective-C is
> likely to lead to more bugs and overhead than benefit. For that reason,
> this change proposes to disable google-runtime-int in Objective-C++.
>
> [1] 
> https://developer.apple.com/documentation/objectivec/nsinteger?language=objc
> [2] 
> https://developer.apple.com/documentation/objectivec/nsuinteger?language=objc
> [3] "Types long, NSInteger, NSUInteger, and CGFloat vary in size between
> 32- and 64-bit builds. Use of these types is appropriate when handling
> values exposed by system interfaces, but they should be avoided for most
> other computations."
> https://github.com/google/styleguide/blob/gh-pages/objcguide.md#types-with-inconsistent-sizes
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> https://reviews.llvm.org/D59336
>
> Files:
>   clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
>
>
> Index: clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
> ===
> --- clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
> +++ clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
> @@ -54,7 +54,9 @@
>
>  void IntegerTypesCheck::registerMatchers(MatchFinder *Finder) {
>// Find all TypeLocs. The relevant Style Guide rule only applies to C++.
> -  if (!getLangOpts().CPlusPlus)
> +  // This check is also not applied in Objective-C++ sources as Objective-C
> +  // often uses built-in integer types other than `int`.
> +  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC)
>  return;
>// Match any integer types, unless they are passed to a printf-based API:
>//
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59336: [clang-tidy] Disable google-runtime-int in Objective-C++ 

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore created this revision.
Herald added subscribers: cfe-commits, jdoerfert, xazax.hun.
Herald added a project: clang.

In contrast to Google C++, Objective-C often uses built-in integer types
other than `int`. In fact, the Objective-C runtime itself defines the
types NSInteger¹ and NSUInteger² which are variant types depending on
the target architecture. The Objective-C style guide indicates that
usage of system types with variant sizes is appropriate when handling
values provided by system interfaces³. Objective-C++ is commonly the
result of conversion from Objective-C to Objective-C++ for the purpose
of integrating C++ functionality. The opposite of Objective-C++ being
used to expose Objective-C functionality to C++ is less common,
potentially because Objective-C has a signficantly more uneven presence
on different platforms compared to C++. This generally predisposes
Objective-C++ to commonly being more Objective-C than C++. Forcing
Objective-C++ developers to perform conversions between variant system types
and fixed size integer types depending on target architecture when
Objective-C++ commonly uses variant system types from Objective-C is
likely to lead to more bugs and overhead than benefit. For that reason,
this change proposes to disable google-runtime-int in Objective-C++.

[1] https://developer.apple.com/documentation/objectivec/nsinteger?language=objc
[2] 
https://developer.apple.com/documentation/objectivec/nsuinteger?language=objc
[3] "Types long, NSInteger, NSUInteger, and CGFloat vary in size between
32- and 64-bit builds. Use of these types is appropriate when handling
values exposed by system interfaces, but they should be avoided for most
other computations."
https://github.com/google/styleguide/blob/gh-pages/objcguide.md#types-with-inconsistent-sizes


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59336

Files:
  clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp


Index: clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
===
--- clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
+++ clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
@@ -54,7 +54,9 @@
 
 void IntegerTypesCheck::registerMatchers(MatchFinder *Finder) {
   // Find all TypeLocs. The relevant Style Guide rule only applies to C++.
-  if (!getLangOpts().CPlusPlus)
+  // This check is also not applied in Objective-C++ sources as Objective-C
+  // often uses built-in integer types other than `int`.
+  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC)
 return;
   // Match any integer types, unless they are passed to a printf-based API:
   //


Index: clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
===
--- clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
+++ clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
@@ -54,7 +54,9 @@
 
 void IntegerTypesCheck::registerMatchers(MatchFinder *Finder) {
   // Find all TypeLocs. The relevant Style Guide rule only applies to C++.
-  if (!getLangOpts().CPlusPlus)
+  // This check is also not applied in Objective-C++ sources as Objective-C
+  // often uses built-in integer types other than `int`.
+  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC)
 return;
   // Match any integer types, unless they are passed to a printf-based API:
   //
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: clang/lib/Parse/ParseAST.cpp:172
+  {
+llvm::TimeTraceScope scope("Backend", "");
+Consumer->HandleTranslationUnit(S.getASTContext());

I think you may want to move this to `clang::EmitBackendOutput`, which is 
closer to real "backend-y" actions. I don't think there's any other heavy 
lifting that happens in HandleTranslationUnit, it looks like diagnostic 
teardown and setup for calling EmitBackendOutput.



Comment at: clang/lib/Parse/ParseDeclCXX.cpp:3115-3118
+  TIME_TRACE_OR_NULL(
+  TagDecl != nullptr && isa(TagDecl)
+  ? cast(TagDecl)->getQualifiedNameAsString().data()
+  : ""));

For example, this would be simplified by a TimeTraceScope callable overload.



Comment at: clang/lib/Parse/ParseTemplate.cpp:237-238
+  "ParseTemplate",
+  TIME_TRACE_OR_NULL(DeclaratorInfo.getIdentifier() != nullptr
+ ? DeclaratorInfo.getIdentifier()->getName().data()
+ : ""));

I guess this would be simplified as well with a callable.



Comment at: clang/lib/Sema/Sema.cpp:98
+if (llvm::TimeTraceProfilerEnabled()) {
+  auto fe = SM.getFileEntryForID(SM.getFileID(Loc));
+  llvm::TimeTraceProfilerBegin(

This doesn't match the LLVM naming and auto usage conventions: 
https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable

Yes, there is an active debate about changing the way we name variables, but 
please just match what we have for now.



Comment at: llvm/lib/IR/LegacyPassManager.cpp:1632-1634
+  bool profileTime = llvm::TimeTraceProfilerEnabled();
+  if (profileTime)
+llvm::TimeTraceProfilerBegin("OptFunction", F.getName().data());

Someone is going to have to figure out where the equivalent annotations should 
live in the new passmanager. I wasn't able to find it just by looking myself, 
so I won't ask you to do it. I guess it's in llvm/include/llvm/IR/PassManager.h.



Comment at: llvm/lib/Support/TimeProfiler.cpp:28
+
+static std::string EscapeString(const char *src) {
+  std::string os;

Here and else where things should be named the LLVM way for consistency:
https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
`escapeString`, `Src`, etc. Tedious, I know.



Comment at: llvm/lib/Support/TimeProfiler.cpp:70
+
+  void Begin(const std::string , const std::string ) {
+Entry e = {steady_clock::now(), {}, name, detail};

I'm tempted to micro-optimize this with StringRef and StringSaver, but I think 
it's unnecessary. Calling malloc may affect the profile, but probably not much.



Comment at: llvm/lib/Support/TimeProfiler.cpp:80
+
+// only include sections longer than 500us
+if (duration_cast(e.Duration).count() > 500)

Comments in this file need to be complete sentences with a leading capital and 
full stop.



Comment at: llvm/lib/Support/TimeProfiler.h:53
+struct TimeTraceScope {
+  TimeTraceScope(const char *name, const char *detail) {
+if (TimeTraceProfilerInstance != nullptr)

It'd be nice if these took StringRefs, it would avoid a fair amount of 
`.data()` and `.c_str()`, but it messes up TIME_TRACE_OR_NULL. Another way to 
delay the work would be to have an `llvm::function_ref detail` 
overload. The callable also allows the user to bind variables like this:
  TimeTraceScope timeScope("ParseTag", [&]() {
if (auto *TD = dyn_cast_or_null(D))
  return TD->getNameAsStr();
return "";
  });
Instead of the `isa(D) ? cast(D)->getNameAsStr() : ""` 
pattern that I see, which repeats TagDecl.


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

https://reviews.llvm.org/D58675



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

[PATCH] D55895: NFC: simplify Darwin environment handling

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.
Herald added a project: clang.

In D55895#1336777 , @arphaman wrote:

> I think we might need to run it past the internal builds to see if anything 
> breaks.


We've been running it for a while without breakage. Good to go?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55895



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

[PATCH] D57893: [analyzer] Fix function macro crash

[Kristóf Umann via Phabricator via cfe-commits]

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

Ah so it was a past-the-end iterator dereference error. Cheers!


Repository:
  rC Clang

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

https://reviews.llvm.org/D57893



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

[PATCH] D57918: Add an attribute that causes clang to emit fortified calls to C stdlib functions

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added a subscriber: jyknight.
erik.pilkington marked 2 inline comments as done.
erik.pilkington added a comment.
Herald added a subscriber: jdoerfert.

I reverted this in r356103:

  Revert "Add a new attribute, fortify_stdlib"
  
  This reverts commit r353765. After talking with our c stdlib folks, we decided
  to use the existing pass_object_size attribute to implement _FORTIFY_SOURCE
  wrappers, like Bionic does (I didn't realize that pass_object_size could be 
used
  for this purpose). Sorry for the flip/flop, and thanks to James Y. Knight for
  pointing this out to me.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57918



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

[PATCH] D59319: [OpenMP][Offloading][1/3] A generic and simple target region interface

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert marked 5 inline comments as done.
jdoerfert added inline comments.



Comment at: openmp/libomptarget/deviceRTLs/common/target_region.h:27
+
+/// The target region _kernel_ interface for GPUs
+///

ABataev wrote:
> All exported functions are declared in the `interface.h` file. I don't think 
> we need an extra interface file here
`interface.h`, or to be more precise for people that do not know, 
`deviceRTLs/nvptx/src/interface.h`, is nvptx specific. This file, 
`deviceRTLs/common/target_region.h`, is by design target agnostic and not 
placed _under_ the nvptx subfolder. If you are willing to move `interface.h` 
into a common space and remove the nvptx specific functions we can merge the 
two. Otherwise, I have strong reservations agains that and good reason not to 
do it.



Comment at: openmp/libomptarget/deviceRTLs/common/target_region.h:100
+///
+EXTERN int8_t __kmpc_target_region_kernel_init(bool UseSPMDMode,
+   bool RequiresOMPRuntime,

ABataev wrote:
> Better to use `ident_loc` for passing info about execution mode and 
> full/lightweight runtime.
Could you please explain why you think that? Adding indirection through a 
structure does not really seem beneficial to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59319



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

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

Very cool! I'll take a look, I wasn't aware this had been rebased and uploaded, 
I was thinking about doing it myself yesterday as a side project.

As I think I've said elsewhere, I'm really excited to give users the tools they 
need to analyze why their code compiles slowly. If we just give them the tools, 
maybe they will restructure their code on their own and they won't come asking, 
"why is the compiler so slow on my (many transitive includes | template 
metaprogram)?".

I'll take a look and try to get back with some thoughts.


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

https://reviews.llvm.org/D58675



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

r356103 - Revert "Add a new attribute, fortify_stdlib"

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Wed Mar 13 14:37:01 2019
New Revision: 356103

URL: http://llvm.org/viewvc/llvm-project?rev=356103=rev
Log:
Revert "Add a new attribute, fortify_stdlib"

This reverts commit r353765. After talking with our c stdlib folks, we decided
to use the existing pass_object_size attribute to implement _FORTIFY_SOURCE
wrappers, like Bionic does (I didn't realize that pass_object_size could be used
for this purpose). Sorry for the flip/flop, and thanks to James Y. Knight for
pointing this out to me.

Removed:
cfe/trunk/test/CodeGen/fortify-std-lib.c
cfe/trunk/test/Sema/fortify-std-lib.c
Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Basic/AttrDocs.td
cfe/trunk/include/clang/Basic/Builtins.h
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Basic/Builtins.cpp
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=356103=356102=356103=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Wed Mar 13 14:37:01 2019
@@ -1566,13 +1566,6 @@ def PassObjectSize : InheritableParamAtt
   let Documentation = [PassObjectSizeDocs];
 }
 
-def FortifyStdLib : InheritableAttr {
-  let Spellings = [Clang<"fortify_stdlib">];
-  let Args = [IntArgument<"Type">, IntArgument<"Flag">];
-  let Subjects = SubjectList<[Function]>;
-  let Documentation = [FortifyStdLibDocs];
-}
-
 // Nullability type attributes.
 def TypeNonNull : TypeAttr {
   let Spellings = [Keyword<"_Nonnull">];

Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=356103=356102=356103=diff
==
--- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
+++ cfe/trunk/include/clang/Basic/AttrDocs.td Wed Mar 13 14:37:01 2019
@@ -965,43 +965,6 @@ of the condition.
   }];
 }
 
-def FortifyStdLibDocs : Documentation {
-  let Category = DocCatFunction;
-  let Content = [{
-The ``fortify_stdlib`` attribute applies to declarations of C stdlib functions
-(memcpy, sprintf, ...), and causes clang to emit calls to their fortified
-variants with ``__builtin_object_size``. This attribute is intended for use
-within standard C library implementations and should not generally be used for
-user applications. For instance:
-
-.. code-block:: c
-
-  __attribute__((fortify_stdlib(0, 0)))
-  int sprintf(char *buf, const char *fmt, ...);
-
-  int main() {
-char buf[5];
-sprintf(buf, "%f", 42.0);
-// Clang generates code equivalent to:
-// __sprintf_chk(buf, 0, __builtin_object_size(0), "%f", 42.0);
-  }
-
-The first argument to the attribute is the integer `type` argument passed to
-`__builtin_object_size` (you can read more about `__builtin_object_size`
-`here 
`_),
-The second argument to this attribute is the flag that the fortified format
-functions accept.
-
-Only a specific set of standard library functions are supported:
-  - memcpy, memmove, memset
-  - stpcpy, strcat, strcpy
-  - strlcat, strlcpy
-  - strncat, strncpy, stpncpy
-  - snprintf, vsnprintf, sprintf, vsprintf
-  - fprintf, vfprintf, printf, vprintf
-}];
-}
-
 def ConvergentDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{

Modified: cfe/trunk/include/clang/Basic/Builtins.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.h?rev=356103=356102=356103=diff
==
--- cfe/trunk/include/clang/Basic/Builtins.h (original)
+++ cfe/trunk/include/clang/Basic/Builtins.h Wed Mar 13 14:37:01 2019
@@ -242,13 +242,7 @@ private:
   const char *Fmt) const;
 };
 
-/// For a given BuiltinID, return the ID of the fortified variant function. For
-/// instance, if Builtin::BIsprintf is passed, this function will return
-/// Builtin::BI__builtin___sprintf_chk. If BuiltinID doesn't have a fortified
-/// variant, 0 is returned.
-unsigned getFortifiedVariantFunction(unsigned BuiltinID);
-
-} // end namespace Builtin
+}
 
 /// Kinds of BuiltinTemplateDecl.
 enum BuiltinTemplateKind : int {

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=356103=356102=356103=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Mar 13 

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Zachary Turner via Phabricator via cfe-commits]

zturner added reviewers: rnk, hans.
zturner added a comment.

+reid and hans, as they might be interested in this.


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

https://reviews.llvm.org/D58675



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

[PATCH] D59264: [Driver] Support compiler-rt crtbegin.o/crtend.o for Linux

[Petr Hosek via Phabricator via cfe-commits]

phosek marked an inline comment as done.
phosek added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:447
   const char *crtbegin;
-  if (Args.hasArg(options::OPT_static))
-crtbegin = isAndroid ? "crtbegin_static.o" : "crtbeginT.o";
-  else if (Args.hasArg(options::OPT_shared))
-crtbegin = isAndroid ? "crtbegin_so.o" : "crtbeginS.o";
-  else if (IsPIE || IsStaticPIE)
-crtbegin = isAndroid ? "crtbegin_dynamic.o" : "crtbeginS.o";
-  else
-crtbegin = isAndroid ? "crtbegin_dynamic.o" : "crtbegin.o";
-
-  if (HasCRTBeginEndFiles)
+  if (ToolChain.GetRuntimeLibType(Args) == ToolChain::RLT_CompilerRT &&
+  !isAndroid) {

manojgupta wrote:
> This is currently unconditional on using compiler-rt. Given that compiler-rt 
> provides the crt*.o files only under the CMake flag COMPILER_RT_BUILD_CRT, 
> shouldn't there be an equivalent clang CMake flag to conditionally enable 
> this.
`COMPILER_RT_BUILD_CRT` is `ON` by default now in D28791, but even if we change 
that, I think this logic should be independent since you should be able to 
build Clang separately from compiler-rt.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59264



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

[PATCH] D57460: [OpenMP][Offloading] A generic and simple OpenMP target kernel interface

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert abandoned this revision.
jdoerfert added a comment.

Patch was split, new revisions can be found here:

  OpenMP: https://reviews.llvm.org/D59319
   Clang: https://reviews.llvm.org/D59328
LLVM: https://reviews.llvm.org/D59331


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57460



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

[PATCH] D59223: Objective-C++11: Support static_assert() in @interface/@implementation ivar lists and method declarations

[Erik Pilkington via Phabricator via cfe-commits]

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

LGTM, after one more comment in Features.def :)




Comment at: clang/include/clang/Basic/Features.def:121
 FEATURE(objc_class_property, LangOpts.ObjC)
+FEATURE(objc_c_static_assert, true)
+FEATURE(objc_cxx_static_assert, LangOpts.CPlusPlus11)

We should only claim `_Static_assert` is a feature in languages where its 
actually a formal language feature (C11), you can also use EXTENSION to 
indicate what languages we accept it in (see the comment in the file header).


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

https://reviews.llvm.org/D59223



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

[PATCH] D59287: [X86] Only define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 in 64-bit mode.

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added a comment.

I agree that its not great, but I don't think we're going to do much better - 
and matching gcc behaviours does make sense here.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59287



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

[PATCH] D59332: [clang-format] AlignConsecutiveDeclarations fails with attributes

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: rolandschulz, reuk, djasper, klimek.
MyDeveloperDay added a project: clang-tools-extra.

Addresses http://llvm.org/PR40418

When using `AlignConsecutiveDeclarations: true`

variables with Attributes would not be aligned correctly

  void f(int  extremlylongparametername1,
 long extremlylongparametername2,
 int[[clang::nodiscard]] extremlylongparametername3,
 int __attribute__((clang::nodiscard)) 
extremlylongparametername4) {
int   a;
unsigned long b;
int[[clang::nodiscard]] c;
int __attribute__((clang::nodiscard)) d;
  }

following this change, both parameters and variables with attributes will be 
aligned (space permitting)

  void f(int  extremlylongparametername1,
 long extremlylongparametername2,
 int  [[clang::nodiscard]] 
extremlylongparametername3,
 int __attribute__((clang::nodiscard)) 
extremlylongparametername4) {
int  a;
unsigned longb;
int  [[clang::nodiscard]] c;
int __attribute__((clang::nodiscard)) d;
  }




https://reviews.llvm.org/D59332

Files:
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10135,6 +10135,47 @@
"  unsigned c;\n"
"}",
Alignment);
+
+  // See llvm.org/PR40418
+  verifyFormat(
+  "void f(int  extremlylongparametername1,\n"
+  "   long extremlylongparametername2,\n"
+  "   int  [[a::b]] extremlylongparametername3,\n"
+  "   int __attribute__((a::b)) extremlylongparametername4) "
+  "{}\n",
+  Alignment);
+
+  verifyFormat("void f(int  extremlylongparametername1,\n"
+   "   long extremlylongparametername2,\n"
+   "   int  [[deprecated]] "
+   "extremlylongparametername3,\n"
+   "   int __attribute__((deprecated)) "
+   "extremlylongparametername4) "
+   "{}\n",
+   Alignment);
+
+  verifyFormat("int  a;\n"
+   "unsigned longb;\n"
+   "int __attribute__((a::b)) d;\n"
+   "int  [[a::b]] c;\n"
+   "int __attribute__((a::b)) e;\n"
+   "int __attribute__((depreciated)) f;\n"
+   "int __attribute__((aligned(16))) g;\n",
+   Alignment);
+
+  verifyFormat("int  a;\n"
+   "unsigned longb;\n"
+   "int __attribute__((a::b)) d;\n"
+   "int  [[deprecated]] c;\n"
+   "int __attribute__((a::b)) e;\n",
+   Alignment);
+
+  verifyFormat("int  a;\n"
+   "unsigned longb;\n"
+   "int __attribute__((deprecated)) de;\n"
+   "int  [[deprecated]] c;\n"
+   "int __attribute__((a::b)) e;\n",
+   Alignment);
 }
 
 TEST_F(FormatTest, LinuxBraceBreaking) {
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -17,8 +17,8 @@
 namespace clang {
 namespace format {
 
-bool WhitespaceManager::Change::IsBeforeInFile::
-operator()(const Change , const Change ) const {
+bool WhitespaceManager::Change::IsBeforeInFile::operator()(
+const Change , const Change ) const {
   return SourceMgr.isBeforeInTranslationUnit(
   C1.OriginalWhitespaceRange.getBegin(),
   C2.OriginalWhitespaceRange.getBegin());
@@ -465,6 +465,19 @@
 // definitions.
 return C.Tok->is(TT_StartOfName) ||
C.Tok->is(TT_FunctionDeclarationName) ||
+   C.Tok->startsSequence(
+   tok::l_paren, tok::l_paren, tok::identifier, 
tok::coloncolon,
+   tok::identifier, tok::r_paren, tok::r_paren) ||
+   C.Tok->startsSequence(tok::l_square, tok::l_square,
+ tok::identifier, tok::coloncolon,
+ tok::identifier, tok::r_square,
+ tok::r_square) ||
+   C.Tok->startsSequence(tok::l_square, tok::l_square,
+ tok::identifier, tok::r_square,
+ tok::r_square) ||
+   C.Tok->startsSequence(tok::l_paren, tok::l_paren,
+ tok::identifier, tok::r_paren,
+   

[PATCH] D59287: [X86] Only define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 in 64-bit mode.

[JF Bastien via Phabricator via cfe-commits]

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

OK, that seems unfortunate but unlikely and consistency terrible with GCC. 
Let's do it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59287



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

r356099 - [clang-format] Propagate inferred language to getLLVMStyle() in getPredefinedStyle()

[Jordan Rupprecht via cfe-commits]

Author: rupprecht
Date: Wed Mar 13 14:13:01 2019
New Revision: 356099

URL: http://llvm.org/viewvc/llvm-project?rev=356099=rev
Log:
[clang-format] Propagate inferred language to getLLVMStyle() in 
getPredefinedStyle()

rC355158 added an optional language parameter to getLLVMStyle(), but this 
parameter was not used in getPredefinedStyle(). Because unit tests directly 
specify the style, this codepath wasn't tested. Add an additional unit test for 
getStyle().

Modified:
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/Format.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=356099=356098=356099=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Wed Mar 13 14:13:01 2019
@@ -961,7 +961,7 @@ FormatStyle getNoStyle() {
 bool getPredefinedStyle(StringRef Name, FormatStyle::LanguageKind Language,
 FormatStyle *Style) {
   if (Name.equals_lower("llvm")) {
-*Style = getLLVMStyle();
+*Style = getLLVMStyle(Language);
   } else if (Name.equals_lower("chromium")) {
 *Style = getChromiumStyle(Language);
   } else if (Name.equals_lower("mozilla")) {

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=356099=356098=356099=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Wed Mar 13 14:13:01 2019
@@ -12714,6 +12714,11 @@ TEST(FormatStyle, GetStyleOfFile) {
   auto Style7 = getStyle("file", "/d/.clang-format", "LLVM", "", );
   ASSERT_FALSE((bool)Style7);
   llvm::consumeError(Style7.takeError());
+
+  // Test 8: inferred per-language defaults apply.
+  auto StyleTd = getStyle("file", "x.td", "llvm", "", );
+  ASSERT_TRUE((bool)StyleTd);
+  ASSERT_EQ(*StyleTd, getLLVMStyle(FormatStyle::LK_TableGen));
 }
 
 TEST_F(ReplacementTest, FormatCodeAfterReplacements) {


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

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added a comment.

@rsmith Any suggestions for good reviewers for this please?


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

https://reviews.llvm.org/D58675



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

[PATCH] D59287: [X86] Only define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 in 64-bit mode.

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

I think the only error we have for X86 is trying to use a -march for a cpu that 
only supports 32 bit but compiling 64 bit code.

I dont' think we can error for -mcx16 on a 32-bit target. For -march=native, 
the driver will call getHostCPUFeatures and get a list of features. As far as I 
can tell CPUID will report cx16 is supported even in 32-bit mode if the host 
CPU supports it in 64-bit mode. The driver will pass the list of features 
returned by getHostCPUFeatures onto the command line of the cc1 invocation. 
Those command line options will then be fed to 
initFeatureMap/setFeatureEnabled. We don't have the information to distinquish 
that from the user passing -mcx16. -mcx16 is intercepted by the driver and 
passed to cc1 in a similar way. I don't think we can filter cx16 from 
getHostCPUFeatures in 32-bit mode since the host might be running in 32-bit 
mode, but we could be passing -m64 to the compiler. So I don't think 
getHostCPUFeatures can look at the current operating mode of the CPU.

I guess we could error for -mcx16 in the driver itself. But due to march native 
we'd still have to protect the code in cc1. Unless we also had the driver 
filter the output of getHostCPUFeatures before passing to cc1. Which if we did 
that then we probably could error from initFeatureMap/setFeatureEnabled.

But gcc doesn't error for any use of cx16 in 32-bit mode so might be bad for 
compatibility anyway?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59287



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

[PATCH] D59328: [OpenMP][Offloading][2/3] Codegen for target regions (TRegions)

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert created this revision.
jdoerfert added a project: OpenMP.
jdoerfert added reviewers: ABataev, arpith-jacob, guraypp, gtbercea, hfinkel.
Herald added a project: clang.

The commit includes the Clang code generation for OpenMP target
constructs based on the interface target region (TRegion) interface.

The interface was introduced in https://reviews.llvm.org/D59319 .

This target code generation is a vastly simplified clone of the NVPTX
code generation but

- there is no NVPTX (or other) target specific code, at least there should not 
be any. The "checkArchForUnifiedAddressing" functionality should therefore be 
moved to a target specific location later on.
- we provide hooks for subclasses in order to perform front-end analysis, as an 
alternative of LLVM based optimizations, e.g., to enable SPMD-mode. (See 
isKnownSPMDMode, mayNeedRuntimeSupport, and mayPerformDataSharing)

The interface is deliberately simple to be easily analyzable in the
middle end. Design decisions included:

- Hide all (complex) implementation choices in the runtime library but allow 
complete removal of the abstraction once the runtime is inlined.
- Provide all runtime calls with sufficient, easy encoded information.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59328

Files:
  clang/lib/CodeGen/CGOpenMPRuntimeTRegion.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeTRegion.h
  clang/lib/CodeGen/CMakeLists.txt
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/OpenMP/target_tregion_no_SPMD_mode.c

Index: clang/test/OpenMP/target_tregion_no_SPMD_mode.c
===
--- /dev/null
+++ clang/test/OpenMP/target_tregion_no_SPMD_mode.c
@@ -0,0 +1,72 @@
+// RUN: %clang_cc1 -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
+// RUN: %clang_cc1 -fopenmp -triple nvptx64-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -mllvm -openmp-tregion-runtime -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s
+
+// CHECK: loop_in_loop_in_tregion
+// CHECK:  %0 = call i8 @__kmpc_target_region_kernel_init(i1 false, i1 true, i1 true, i1 true)
+// CHECK:  call void @__kmpc_target_region_kernel_deinit(i1 false, i1 true)
+void loop_in_loop_in_tregion(int *A, int *B) {
+#pragma omp target
+  for (int i = 0; i < 512; i++) {
+for (int j = 0; j < 1024; j++)
+  A[j] += B[i + j];
+  }
+}
+
+// CHECK: parallel_loops_and_accesses_in_tregion
+// CHECK:  %0 = call i8 @__kmpc_target_region_kernel_init(i1 false, i1 true, i1 true, i1 true)
+// CHECK:  call void @__kmpc_target_region_kernel_parallel(i1 false, i1 true, void (i8*, i8*)* @.omp_TRegion._wrapper, i8* undef, i16 0, i8* %2, i16 16, i1 false)
+// CHECK:  call void @__kmpc_target_region_kernel_parallel(i1 false, i1 true, void (i8*, i8*)* @.omp_TRegion.1_wrapper, i8* undef, i16 0, i8* %5, i16 16, i1 false)
+// CHECK:  call void @__kmpc_target_region_kernel_parallel(i1 false, i1 true, void (i8*, i8*)* @.omp_TRegion.2_wrapper, i8* undef, i16 0, i8* %8, i16 16, i1 false)
+// CHECK:  call void @__kmpc_target_region_kernel_deinit(i1 false, i1 true)
+void parallel_loops_and_accesses_in_tregion(int *A, int *B) {
+#pragma omp target
+  {
+#pragma omp parallel for
+for (int j = 0; j < 1024; j++)
+  A[j] += B[0 + j];
+#pragma omp parallel for
+for (int j = 0; j < 1024; j++)
+  A[j] += B[1 + j];
+#pragma omp parallel for
+for (int j = 0; j < 1024; j++)
+  A[j] += B[2 + j];
+
+// This needs a guard in SPMD mode
+A[0] = B[0];
+  }
+}
+
+void extern_func();
+static void parallel_loop(int *A, int *B, int i) {
+#pragma omp parallel for
+  for (int j = 0; j < 1024; j++)
+A[j] += B[i + j];
+}
+
+// CHECK: parallel_loop_in_function_in_loop_with_global_acc_in_tregion
+// CHECK:  %1 = call i8 @__kmpc_target_region_kernel_init(i1 false, i1 true, i1 true, i1 true)
+// CHECK:  call void @__kmpc_target_region_kernel_deinit(i1 false, i1 true)
+int Global[512];
+void parallel_loop_in_function_in_loop_with_global_acc_in_tregion(int *A, int *B) {
+#pragma omp target
+  for (int i = 0; i < 512; i++) {
+parallel_loop(A, B, i);
+Global[i]++;
+  }
+}
+
+// CHECK: parallel_loop
+// CHECK:  call void @__kmpc_target_region_kernel_parallel(i1 false, i1 true, void (i8*, i8*)* @.omp_TRegion.3_wrapper, i8* undef, i16 0, i8* %0, i16 24, i1 false)
+
+// CHECK: parallel_loops_in_functions_and_extern_func_in_tregion
+// CHECK:  %0 = call i8 @__kmpc_target_region_kernel_init(i1 false, i1 true, i1 true, i1 true)
+// CHECK:  call void @__kmpc_target_region_kernel_deinit(i1 false, i1 true)
+void parallel_loops_in_functions_and_extern_func_in_tregion(int *A, int *B) {
+#pragma omp target
+  {
+parallel_loop(A, B, 1);
+parallel_loop(A, B, 2);
+extern_func();
+parallel_loop(A, B, 3);
+  }
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ 

[PATCH] D59329: [LibTooling] Add NodeId, a strong type for AST-matcher node identifiers.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added a reviewer: ilya-biryukov.
ymandel added a project: clang.
Herald added subscribers: jfb, mgorny.

The standard matcher API uses StringRefs to identify bound nodes.  This patch
introduces a strong type thats allows distinguishing ids from arbitrary text in
APIs.  It additionally adds a templated version which can indicate the AST type
to which the id is bound.

This patch is the second in a series intended to improve the abstractions
available to users for writing source-to-source transformations.  A full
discussion of the end goal can be found on the cfe-dev list with subject "[RFC]
Easier source-to-source transformations with clang tooling".


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59329

Files:
  clang/include/clang/Tooling/Refactoring/NodeId.h
  clang/lib/Tooling/Refactoring/CMakeLists.txt
  clang/lib/Tooling/Refactoring/NodeId.cpp

Index: clang/lib/Tooling/Refactoring/NodeId.cpp
===
--- /dev/null
+++ clang/lib/Tooling/Refactoring/NodeId.cpp
@@ -0,0 +1,27 @@
+//===--- NodeId.cpp - NodeId implementation -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Tooling/Refactoring/NodeId.h"
+#include 
+
+namespace clang {
+namespace tooling {
+
+// For guaranteeing unique ids on NodeId creation.
+static size_t nextId() {
+  // Start with a relatively high number to avoid bugs if the user mixes
+  // explicitly-numbered ids with those generated with `NextId()`. Similarly, we
+  // choose a number that allows generated ids to be easily recognized.
+  static std::atomic Next();
+  return Next.fetch_add(1, std::memory_order_relaxed);
+}
+
+NodeId::NodeId() : NodeId(nextId()) {}
+
+} // namespace tooling
+} // namespace clang
Index: clang/lib/Tooling/Refactoring/CMakeLists.txt
===
--- clang/lib/Tooling/Refactoring/CMakeLists.txt
+++ clang/lib/Tooling/Refactoring/CMakeLists.txt
@@ -12,6 +12,7 @@
   Rename/USRFinder.cpp
   Rename/USRFindingAction.cpp
   Rename/USRLocFinder.cpp
+  NodeId.cpp
 
   LINK_LIBS
   clangAST
Index: clang/include/clang/Tooling/Refactoring/NodeId.h
===
--- /dev/null
+++ clang/include/clang/Tooling/Refactoring/NodeId.h
@@ -0,0 +1,85 @@
+//===--- NodeId.h - NodeId class --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+///
+/// \file This file defines abstractions for the bound identifiers used in AST
+/// matchers.
+///
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLING_REFACTOR_NODE_ID_H_
+#define LLVM_CLANG_TOOLING_REFACTOR_NODE_ID_H_
+
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "llvm/ADT/StringRef.h"
+#include 
+
+namespace clang {
+namespace tooling {
+
+/// A strong type for AST node identifiers.  The standard API uses StringRefs
+/// for identifiers.  The strong type allows us to distinguish ids from
+/// arbitrary text snippets in various APIs.
+class NodeId {
+public:
+  explicit NodeId(std::string Id) : Id(std::move(Id)) {}
+
+  /// Creates a NodeId whose name is based on \p Id. Guarantees that unique ids
+  /// map to unique NodeIds.
+  explicit NodeId(size_t Id) : Id("id" + std::to_string(Id)) {}
+
+  /// Creates a NodeId with a generated name. The name is guaranteed to be
+  /// unique with respect to other generated names.
+  NodeId();
+
+  llvm::StringRef id() const { return Id; }
+
+  /// Gets the AST node in \p Result corresponding to this NodeId, if
+  /// any. Otherwise, returns null.
+  template 
+  const Node *
+  getNodeAs(const ast_matchers::MatchFinder::MatchResult ) const {
+return Result.Nodes.getNodeAs(Id);
+  }
+
+private:
+  std::string Id;
+};
+
+/// Refinement of NodeId that identifies the intended node type for the id. This
+/// additional information allows us to select appropriate overloads or
+/// constrain use of various combinators. It also allows us to distinguish when
+/// a \c Expr node is intended as a \c Stmt, which influences the intended
+/// source range for the node.  \p Node is the AST node type corresponding to
+/// this id.
+template  class TypedNodeId : public NodeId {
+public:
+  using NodeId::NodeId;
+  using MatcherType = ast_matchers::internal::Matcher;
+
+  /// Creates a matcher corresponding to the AST-node type of this id 

[PATCH] D59327: [Sema] Fix a use-after-free of a _Nonnull ParsedAttr

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington created this revision.
erik.pilkington added reviewers: aaron.ballman, arphaman, rsmith.
Herald added subscribers: dexonsmith, jkorous.
Herald added a project: clang.

We were allocating the implicit attribute in the declarator's attribute pool, 
but putting into the declaration specifier's `ParsedAttributesView`. If there 
are multiple declarators, then we'll use the attribute from the declaration 
specifier after clearing out the declarators attribute pool. Fix this by 
allocating the attribute in the declaration specifier's pool. This problem was 
creating some nonsensical diagnostics and crashes on the testcase (only in 
NDEBUG, though).

rdar://48529718

Thanks for taking a look!
Erik


Repository:
  rC Clang

https://reviews.llvm.org/D59327

Files:
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaObjC/nonnull.m


Index: clang/test/SemaObjC/nonnull.m
===
--- clang/test/SemaObjC/nonnull.m
+++ clang/test/SemaObjC/nonnull.m
@@ -125,3 +125,9 @@
 }
 
 void (^PR23117)(int *) = ^(int *p1) __attribute__((nonnull(1))) {};
+
+typedef int *intptr;
+#pragma clang assume_nonnull begin
+intptr a, b;
+intptr c, (*d)();
+#pragma clang assume_nonnull end
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -4221,7 +4221,7 @@
   auto inferPointerNullability =
   [&](SimplePointerKind pointerKind, SourceLocation pointerLoc,
   SourceLocation pointerEndLoc,
-  ParsedAttributesView ) -> ParsedAttr * {
+  ParsedAttributesView , AttributePool ) -> ParsedAttr * {
 // We've seen a pointer.
 if (NumPointersRemaining > 0)
   --NumPointersRemaining;
@@ -4235,11 +4235,9 @@
   ParsedAttr::Syntax syntax = inferNullabilityCS
   ? ParsedAttr::AS_ContextSensitiveKeyword
   : ParsedAttr::AS_Keyword;
-  ParsedAttr *nullabilityAttr =
-  state.getDeclarator().getAttributePool().create(
-  S.getNullabilityKeyword(*inferNullability),
-  SourceRange(pointerLoc), nullptr, SourceLocation(), nullptr, 0,
-  syntax);
+  ParsedAttr *nullabilityAttr = Pool.create(
+  S.getNullabilityKeyword(*inferNullability), SourceRange(pointerLoc),
+  nullptr, SourceLocation(), nullptr, 0, syntax);
 
   attrs.addAtEnd(nullabilityAttr);
 
@@ -4298,7 +4296,8 @@
 if (auto *attr = inferPointerNullability(
 pointerKind, D.getDeclSpec().getTypeSpecTypeLoc(),
 D.getDeclSpec().getEndLoc(),
-D.getMutableDeclSpec().getAttributes())) {
+D.getMutableDeclSpec().getAttributes(),
+D.getMutableDeclSpec().getAttributePool())) {
   T = state.getAttributedType(
   createNullabilityAttr(Context, *attr, *inferNullability), T, T);
 }
@@ -4338,7 +4337,8 @@
 
   // Handle pointer nullability.
   inferPointerNullability(SimplePointerKind::BlockPointer, DeclType.Loc,
-  DeclType.EndLoc, DeclType.getAttrs());
+  DeclType.EndLoc, DeclType.getAttrs(),
+  state.getDeclarator().getAttributePool());
 
   T = S.BuildBlockPointerType(T, D.getIdentifierLoc(), Name);
   if (DeclType.Cls.TypeQuals || LangOpts.OpenCL) {
@@ -4360,7 +4360,8 @@
 
   // Handle pointer nullability
   inferPointerNullability(SimplePointerKind::Pointer, DeclType.Loc,
-  DeclType.EndLoc, DeclType.getAttrs());
+  DeclType.EndLoc, DeclType.getAttrs(),
+  state.getDeclarator().getAttributePool());
 
   if (LangOpts.ObjC && T->getAs()) {
 T = Context.getObjCObjectPointerType(T);
@@ -4892,7 +4893,8 @@
 
   // Handle pointer nullability.
   inferPointerNullability(SimplePointerKind::MemberPointer, DeclType.Loc,
-  DeclType.EndLoc, DeclType.getAttrs());
+  DeclType.EndLoc, DeclType.getAttrs(),
+  state.getDeclarator().getAttributePool());
 
   if (SS.isInvalid()) {
 // Avoid emitting extra errors if we already errored on the scope.


Index: clang/test/SemaObjC/nonnull.m
===
--- clang/test/SemaObjC/nonnull.m
+++ clang/test/SemaObjC/nonnull.m
@@ -125,3 +125,9 @@
 }
 
 void (^PR23117)(int *) = ^(int *p1) __attribute__((nonnull(1))) {};
+
+typedef int *intptr;
+#pragma clang assume_nonnull begin
+intptr a, b;
+intptr c, (*d)();
+#pragma clang assume_nonnull end
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -4221,7 +4221,7 @@
   auto inferPointerNullability =

[PATCH] D59264: [Driver] Support compiler-rt crtbegin.o/crtend.o for Linux

[Manoj Gupta via Phabricator via cfe-commits]

manojgupta added subscribers: manojgupta, llozano.
manojgupta added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:447
   const char *crtbegin;
-  if (Args.hasArg(options::OPT_static))
-crtbegin = isAndroid ? "crtbegin_static.o" : "crtbeginT.o";
-  else if (Args.hasArg(options::OPT_shared))
-crtbegin = isAndroid ? "crtbegin_so.o" : "crtbeginS.o";
-  else if (IsPIE || IsStaticPIE)
-crtbegin = isAndroid ? "crtbegin_dynamic.o" : "crtbeginS.o";
-  else
-crtbegin = isAndroid ? "crtbegin_dynamic.o" : "crtbegin.o";
-
-  if (HasCRTBeginEndFiles)
+  if (ToolChain.GetRuntimeLibType(Args) == ToolChain::RLT_CompilerRT &&
+  !isAndroid) {

This is currently unconditional on using compiler-rt. Given that compiler-rt 
provides the crt*.o files only under the CMake flag COMPILER_RT_BUILD_CRT, 
shouldn't there be an equivalent clang CMake flag to conditionally enable this.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59264



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

[PATCH] D59307: Patch llvm bug 41033 concerning atomicity of statement expressions

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D59307#1428101 , @mibintc wrote:

> In D59307#1427644 , @jfb wrote:
>
> > I think you also want to test C++ `std::atomic` ...
>
>
> The bug described in https://bugs.llvm.org/show_bug.cgi?id=41033 doesn't 
> occur using C++ atomic, I rewrote the test case this way, and it compiles 
> without error.  The "load" operation returns int since all the atomic 
> operations occur under the covers using builtins.   Does this convey the test 
> you have in mind?
>
> #include 
>  int fum(int y) {
>
>   std::atomic x(1);
>   y = ({x.load();});
>
> }


What I had in mind with `atomic` isn't relevant, because it would try to call 
`atomic(const atomic&) = delete;`. Your test case here isn't something I'm 
worried about.

`volatile` is still and issue.

I'm still not sure this is something we want.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59307



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

r356098 - [OPENMP]Fix PR37283: Assertion failure on openmp task with by reference

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Wed Mar 13 13:46:28 2019
New Revision: 356098

URL: http://llvm.org/viewvc/llvm-project?rev=356098=rev
Log:
[OPENMP]Fix PR37283: Assertion failure on openmp task with by reference
array.

If the firstprivate variable is a reference, we may incorrectly classify
the kind of the private copy. Use the type of the private copy instead
of the original shared variable.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/test/OpenMP/task_firstprivate_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=356098=356097=356098=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Wed Mar 13 13:46:28 2019
@@ -4705,7 +4705,7 @@ static void emitPrivatesInit(CodeGenFunc
 // Check if the variable is the target-based BasePointersArray,
 // PointersArray or SizesArray.
 LValue SharedRefLValue;
-QualType Type = OriginalVD->getType();
+QualType Type = PrivateLValue.getType();
 const FieldDecl *SharedField = CapturesInfo.lookup(OriginalVD);
 if (IsTargetTask && !SharedField) {
   assert(isa(OriginalVD) &&

Modified: cfe/trunk/test/OpenMP/task_firstprivate_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/task_firstprivate_codegen.cpp?rev=356098=356097=356098=diff
==
--- cfe/trunk/test/OpenMP/task_firstprivate_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/task_firstprivate_codegen.cpp Wed Mar 13 13:46:28 2019
@@ -473,12 +473,13 @@ struct St {
   ~St() {}
 };
 
-void array_func(int n, float a[n], St s[2]) {
+void array_func(int n, float a[n], St s[2], int(& p)[1]) {
 // ARRAY: call i8* @__kmpc_omp_task_alloc(
 // ARRAY: call i32 @__kmpc_omp_task(
 // ARRAY: store float** %{{.+}}, float*** %{{.+}},
 // ARRAY: store %struct.St** %{{.+}}, %struct.St*** %{{.+}},
-#pragma omp task firstprivate(a, s)
+// ARRAY: store [1 x i32]* %{{.+}}, [1 x i32]** %{{.+}},
+#pragma omp task firstprivate(a, s, p)
   ;
 }
 #endif


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

[PATCH] D59307: Patch llvm bug 41033 concerning atomicity of statement expressions

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

In D59307#1427644 , @jfb wrote:

> I think you also want to test C++ `std::atomic` ...


The bug described in https://bugs.llvm.org/show_bug.cgi?id=41033 doesn't occur 
using C++ atomic, I rewrote the test case this way, and it compiles without 
error.  The "load" operation returns int since all the atomic operations occur 
under the covers using builtins.   Does this convey the test you have in mind?

#include 
int fum(int y) {

  std::atomic x(1);
  y = ({x.load();});

}


Repository:
  rC Clang

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

https://reviews.llvm.org/D59307



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

[PATCH] D59319: [OpenMP][Offloading][1/3] A generic and simple target region interface

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: 
openmp/libomptarget/cmake/Modules/LibomptargetNVPTXBitcodeLibrary.cmake:81
 # if any of them are not supported, there is no point in finding out which are.
-set(compiler_flags_required -emit-llvm -O1 --cuda-device-only 
--cuda-path=${CUDA_TOOLKIT_ROOT_DIR})
+set(compiler_flags_required -emit-llvm -std=c++11 -O1 --cuda-device-only 
--cuda-path=${CUDA_TOOLKIT_ROOT_DIR})
 set(compiler_flags_required_src "extern \"C\" __device__ int thread() { return 
threadIdx.x; }")

It must be in a separate patch



Comment at: 
openmp/libomptarget/cmake/Modules/LibomptargetNVPTXBitcodeLibrary.cmake:88
 if (NOT LIBOMPTARGET_NVPTX_CUDA_COMPILER_SUPPORTS_FLAGS_REQUIRED)
+  message(ERROR "NO FLAG SUPPORT")
   return()

Same, if you really need it - separate patch



Comment at: 
openmp/libomptarget/cmake/Modules/LibomptargetNVPTXBitcodeLibrary.cmake:105
   if (NOT LIBOMPTARGET_NVPTX_CUDA_COMPILER_SUPPORTS_FCUDA_RDC)
+message(ERROR "NO FCUDA RDC")
 return()

Same here



Comment at: openmp/libomptarget/deviceRTLs/common/target_region.h:27
+
+/// The target region _kernel_ interface for GPUs
+///

All exported functions are declared in the `interface.h` file. I don't think we 
need an extra interface file here



Comment at: openmp/libomptarget/deviceRTLs/common/target_region.h:100
+///
+EXTERN int8_t __kmpc_target_region_kernel_init(bool UseSPMDMode,
+   bool RequiresOMPRuntime,

Better to use `ident_loc` for passing info about execution mode and 
full/lightweight runtime.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59319



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

[PATCH] D59287: [X86] Only define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 in 64-bit mode.

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D59287#1427945 , @craig.topper 
wrote:

> Is this ok with the backend fixed?


This is definitely better.

> Or do you want me factor this into HasCX16 which I think is only used by the 
> defineMacro and the return for hasFeature("cx16")? And I think 
> hasFeature("cx16") is only used by that getMaxAtomicWidth() code which is 
> only called on 64 bit.
> 
> Or we could maybe ignore "cx16" in setFeatureEnabled on 32 bit targets? But I 
> think that would break always_inline on a target attribute with cx16 in 32 
> bit mode which gcc does allow. https://godbolt.org/z/TW985s

I'm not sure. Does clang ever error out when you have inconsistent platform 
features and arch on the command line? That seems like what we should be doing 
here, no?
Because your change just hides a mistake, and clang is usually the only place 
where we catch mistakes (the rest of LLVM can't diagnose).


Repository:
  rC Clang

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

https://reviews.llvm.org/D59287



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

r356097 - [clang-format][NFC] Include TableGen in enum->string mapping used for debugging

[Jordan Rupprecht via cfe-commits]

Author: rupprecht
Date: Wed Mar 13 13:34:34 2019
New Revision: 356097

URL: http://llvm.org/viewvc/llvm-project?rev=356097=rev
Log:
[clang-format][NFC] Include TableGen in enum->string mapping used for debugging

Running `clang-format -debug` prints "Unknown" for tablegen files because of 
this missing mapping, even though it is recognized as a tablegen file.

Modified:
cfe/trunk/include/clang/Format/Format.h

Modified: cfe/trunk/include/clang/Format/Format.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=356097=356096=356097=diff
==
--- cfe/trunk/include/clang/Format/Format.h (original)
+++ cfe/trunk/include/clang/Format/Format.h Wed Mar 13 13:34:34 2019
@@ -2089,6 +2089,8 @@ inline StringRef getLanguageName(FormatS
 return "JavaScript";
   case FormatStyle::LK_Proto:
 return "Proto";
+  case FormatStyle::LK_TableGen:
+return "TableGen";
   case FormatStyle::LK_TextProto:
 return "TextProto";
   default:


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

[PATCH] D59316: [HIP-Clang] propagate -mllvm options to opt and llc

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

In D59316#1427996 , @ashi1 wrote:

> Hi Matt, that solution will need refactoring and testing. Currently, 
> HIP-Clang is following the same link flow as HCC


HCC is also an issue. I really want effort put into fixing this rather than a 
halfway solution that will make it easier to continue deferring a real fix. I 
started on step 1 a long time ago (D59321 ). 
Step 2 is to fix this to behave like the CudaToolchain handling instead of 
invoking the tools. Pretty much everything should be how that already does 
this, except for the minor details


Repository:
  rC Clang

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

https://reviews.llvm.org/D59316



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

[PATCH] D59321: WIP: AMDGPU: Teach toolchain to link rocm device libs

[Matt Arsenault via Phabricator via cfe-commits]

arsenm created this revision.
arsenm added reviewers: yaxunl, ashi1.
Herald added subscribers: jdoerfert, t-tye, tpr, dstuttard, nhaehnle, wdng, 
jvesely, kzhuravl.

This is pending finalization of what the device library build path/names are. 
I'm also not sure if some of the standard default library control flags should 
be used for these purposes, or if new ones are necessary


https://reviews.llvm.org/D59321

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/Driver/Options.td
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/AMDGPU.cpp
  lib/Driver/ToolChains/AMDGPU.h
  test/CodeGenOpenCL/amdgpu-debug-info-pointer-address-space.cl
  test/CodeGenOpenCL/amdgpu-debug-info-variable-expression.cl
  test/Driver/Inputs/rocm-device-libs/lib/irif.amdgcn.bc
  test/Driver/Inputs/rocm-device-libs/lib/ockl.amdgcn.bc
  
test/Driver/Inputs/rocm-device-libs/lib/oclc_correctly_rounded_sqrt_off.amdgcn.bc
  
test/Driver/Inputs/rocm-device-libs/lib/oclc_correctly_rounded_sqrt_on.amdgcn.bc
  test/Driver/Inputs/rocm-device-libs/lib/oclc_daz_opt_off.amdgcn.bc
  test/Driver/Inputs/rocm-device-libs/lib/oclc_daz_opt_on.amdgcn.bc
  test/Driver/Inputs/rocm-device-libs/lib/oclc_finite_only_off.amdgcn.bc
  test/Driver/Inputs/rocm-device-libs/lib/oclc_finite_only_on.amdgcn.bc
  test/Driver/Inputs/rocm-device-libs/lib/oclc_isa_version_803.amdgcn.bc
  test/Driver/Inputs/rocm-device-libs/lib/oclc_isa_version_900.amdgcn.bc
  test/Driver/Inputs/rocm-device-libs/lib/oclc_unsafe_math_off.amdgcn.bc
  test/Driver/Inputs/rocm-device-libs/lib/oclc_unsafe_math_on.amdgcn.bc
  test/Driver/Inputs/rocm-device-libs/lib/ocml.amdgcn.bc
  test/Driver/Inputs/rocm-device-libs/lib/opencl.amdgcn.bc
  test/Driver/amdgpu-visibility.cl
  test/Driver/rocm-detect.cl
  test/Driver/rocm-device-libs.cl
  test/Driver/rocm-not-found.cl

Index: test/Driver/rocm-not-found.cl
===
--- /dev/null
+++ test/Driver/rocm-not-found.cl
@@ -0,0 +1,11 @@
+// REQUIRES: clang-driver
+
+// Check that we raise an error if we're trying to compile OpenCL for amdhsa code but can't
+// find a ROCm install, unless -nodefaultlibs was passed.
+
+// RUN: %clang -### --sysroot=%s/no-rocm-there -target amdgcn--amdhsa %s 2>&1 | FileCheck %s --check-prefix ERR
+// RUN: %clang -### --rocm-path=%s/no-rocm-there -target amdgcn--amdhsa %s 2>&1 | FileCheck %s --check-prefix ERR
+// ERR: cannot find ROCm installation. Provide its path via --rocm-path, or pass -nodefaultlibs.
+
+// RUN: %clang -### -nodefaultlibs --rocm-path=%s/no-rocm-there %s 2>&1 | FileCheck %s --check-prefix OK
+// OK-NOT: cannot find ROCm installation.
Index: test/Driver/rocm-device-libs.cl
===
--- /dev/null
+++ test/Driver/rocm-device-libs.cl
@@ -0,0 +1,121 @@
+// REQUIRES: clang-driver
+// REQUIRES: amdgpu-registered-target
+
+// Test flush-denormals-to-zero enabled uses oclc_daz_opt_on
+
+// RUN: %clang -### -target amdgcn-amd-amdhsa \
+// RUN:   -x cl -mcpu=gfx900 \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
+// RUN:   %S/opencl.cl \
+// RUN: 2>&1 | FileCheck -dump-input-on-failure --check-prefixes=COMMON,COMMON-DEFAULT,GFX900-DEFAULT,GFX900 %s
+
+
+
+// Make sure the different denormal default is respected for gfx8
+// RUN: %clang -### -target amdgcn-amd-amdhsa \
+// RUN:   -x cl -mcpu=gfx803 \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
+// RUN:   %S/opencl.cl \
+// RUN: 2>&1 | FileCheck -dump-input-on-failure --check-prefixes=COMMON,COMMON-DEFAULT,GFX803-DEFAULT,GFX803 %s
+
+
+
+// Make sure the non-canonical name works
+// RUN: %clang -### -target amdgcn-amd-amdhsa \
+// RUN:   -x cl -mcpu=fiji \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
+// RUN:   %S/opencl.cl \
+// RUN: 2>&1 | FileCheck -dump-input-on-failure --check-prefixes=COMMON,COMMON-DEFAULT,GFX803-DEFAULT,GFX803 %s
+
+
+
+// RUN: %clang -### -target amdgcn-amd-amdhsa \
+// RUN:   -x cl -mcpu=gfx900 \
+// RUN:   -cl-denorms-are-zero \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
+// RUN:   %S/opencl.cl \
+// RUN: 2>&1 | FileCheck -dump-input-on-failure --check-prefixes=COMMON,COMMON-DAZ,GFX900 %s
+
+
+// RUN: %clang -### -target amdgcn-amd-amdhsa \
+// RUN:   -x cl -mcpu=gfx803 \
+// RUN:   -cl-denorms-are-zero \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
+// RUN:   %S/opencl.cl \
+// RUN: 2>&1 | FileCheck -dump-input-on-failure --check-prefixes=COMMON,COMMON-DAZ,GFX803 %s
+
+
+
+// RUN: %clang -### -target amdgcn-amd-amdhsa \
+// RUN:   -x cl -mcpu=gfx803 \
+// RUN:   -cl-finite-math-only \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
+// RUN:   %S/opencl.cl \
+// RUN: 2>&1 | FileCheck -dump-input-on-failure --check-prefixes=COMMON,COMMON-FINITE-ONLY,GFX803 %s
+
+
+
+// RUN: %clang -### -target amdgcn-amd-amdhsa\
+// RUN:   -x cl -mcpu=gfx803 \
+// RUN:   -cl-fp32-correctly-rounded-divide-sqrt \
+// RUN:   --rocm-path=%S/Inputs/rocm-device-libs \

[PATCH] D57893: [analyzer] Fix function macro crash

[Tibor Brunner via Phabricator via cfe-commits]

bruntib updated this revision to Diff 190485.
bruntib added a comment.

I added a condition before std::next() invocations to check if the next element 
is inside the valid interval. This fixes the crash of the build-bot. Sorry for 
the ugly bug.
I don't know if there is a more elegant solution.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57893

Files:
  lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
  test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
  test/Analysis/plist-macros-with-expansion.cpp

Index: test/Analysis/plist-macros-with-expansion.cpp
===
--- test/Analysis/plist-macros-with-expansion.cpp
+++ test/Analysis/plist-macros-with-expansion.cpp
@@ -451,3 +451,21 @@
 1 / value; // expected-warning{{Division by zero}}
// expected-warning@-1{{expression result unused}}
 }
+
+#define FOO(x) int foo() { return x; }
+#define APPLY_ZERO1(function) function(0)
+
+APPLY_ZERO1(FOO)
+void useZeroApplier1() { (void)(1 / foo()); } // expected-warning{{Division by zero}}
+
+// CHECK: nameAPPLY_ZERO1
+// CHECK-NEXT: expansionint foo() { return x; }(0)
+
+#define BAR(x) int bar() { return x; }
+#define APPLY_ZERO2 BAR(0)
+
+APPLY_ZERO2
+void useZeroApplier2() { (void)(1 / bar()); } // expected-warning{{Division by zero}}
+
+// CHECK: nameAPPLY_ZERO2
+// CHECK-NEXT: expansionint bar() { return 0; }
Index: test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
===
--- test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
+++ test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
@@ -5577,6 +5577,484 @@

   
   
+  
+   path
+   
+
+ kindcontrol
+ edges
+  
+   
+start
+ 
+  
+   line459
+   col33
+   file0
+  
+  
+   line459
+   col33
+   file0
+  
+ 
+end
+ 
+  
+   line459
+   col37
+   file0
+  
+  
+   line459
+   col39
+   file0
+  
+ 
+   
+  
+
+
+ kindevent
+ location
+ 
+  line459
+  col37
+  file0
+ 
+ ranges
+ 
+   
+
+ line459
+ col37
+ file0
+
+
+ line459
+ col41
+ file0
+
+   
+ 
+ depth0
+ extended_message
+ Calling foo
+ message
+ Calling foo
+
+
+ kindevent
+ location
+ 
+  line458
+  col1
+  file0
+ 
+ depth1
+ extended_message
+ Entered call from useZeroApplier1
+ message
+ Entered call from useZeroApplier1
+
+
+ kindevent
+ location
+ 
+  line458
+  col1
+  file0
+ 
+ ranges
+ 
+   
+
+ line458
+ col1
+ file0
+
+
+ line458
+ col16
+ file0
+
+   
+ 
+ depth1
+ extended_message
+ Returning zero
+ message
+ Returning zero
+
+
+ kindevent
+ location
+ 
+  line459
+  col37
+  file0
+ 
+ ranges
+ 
+   
+
+ line459
+ col37
+ file0
+
+
+ line459
+ col41
+ file0
+
+   
+ 
+ depth0
+ extended_message
+ Returning from foo
+ message
+ Returning from foo
+
+
+ kindcontrol
+ edges
+  
+   
+start
+ 
+  
+   line459
+   col37
+   file0
+  
+  
+   line459
+   col39
+   file0
+  
+ 
+end
+ 
+  
+   line459
+   col35
+   file0
+  
+  
+   line459
+   col35
+   file0
+  
+ 
+   
+  
+
+
+ kindevent
+ location
+ 
+  line459
+  col35
+  file0
+ 
+ ranges
+ 
+   
+
+ line459
+ col33
+ file0
+
+
+ line459
+ col41
+ file0
+
+   
+ 
+ depth0
+ extended_message
+ Division by zero
+ message
+ Division by zero
+
+   
+   macro_expansions
+   
+
+ location
+ 
+  line458
+  col1
+  file0
+ 
+ nameAPPLY_ZERO1
+ expansionint foo() { return x; }(0)
+
+   
+   descriptionDivision by zero
+   categoryLogic error
+   typeDivision by zero
+   check_namecore.DivideZero
+   
+   issue_hash_content_of_line_in_context7ff82561a6c752746649d05220deeb40
+  issue_context_kindfunction
+  issue_contextuseZeroApplier1
+  issue_hash_function_offset0
+  location

[PATCH] D59319: [OpenMP][Offloading][1/3] A generic and simple target region interface

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert updated this revision to Diff 190484.
jdoerfert added a comment.

Simplify the commmit further


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59319

Files:
  openmp/libomptarget/deviceRTLs/common/target_region.h
  openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt
  openmp/libomptarget/deviceRTLs/nvptx/src/omp_data.cu
  openmp/libomptarget/deviceRTLs/nvptx/src/omptarget-nvptx.h
  openmp/libomptarget/deviceRTLs/nvptx/src/target_region.cu

Index: openmp/libomptarget/deviceRTLs/nvptx/src/target_region.cu
===
--- /dev/null
+++ openmp/libomptarget/deviceRTLs/nvptx/src/target_region.cu
@@ -0,0 +1,197 @@
+//===-- target_region.cu  CUDA impl. of the target region interface -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file contains the implementation of the common target region interface.
+//
+//===--===//
+
+// Include the native definitions first as certain defines might be needed in
+// the common interface definition below.
+#include "omptarget-nvptx.h"
+#include "interface.h"
+
+#include "../../common/target_region.h"
+
+/// The pointer used to share memory between team threads.
+extern __device__ __shared__ target_region_shared_buffer
+_target_region_shared_memory;
+
+EXTERN char *__kmpc_target_region_kernel_get_shared_memory() {
+  return _target_region_shared_memory.begin();
+}
+EXTERN char *__kmpc_target_region_kernel_get_private_memory() {
+  return _target_region_shared_memory.begin() +
+ _target_region_shared_memory.get_offset();
+}
+
+/// Simple generic state machine for worker threads.
+INLINE static void
+__kmpc_target_region_state_machine(bool IsOMPRuntimeInitialized) {
+
+  do {
+void *WorkFn = 0;
+
+// Wait for the signal that we have a new work function.
+__kmpc_barrier_simple_spmd(NULL, 0);
+
+// Retrieve the work function from the runtime.
+bool IsActive = __kmpc_kernel_parallel(, IsOMPRuntimeInitialized);
+
+// If there is nothing more to do, break out of the state machine by
+// returning to the caller.
+if (!WorkFn)
+  return;
+
+if (IsActive) {
+  char *SharedVars = __kmpc_target_region_kernel_get_shared_memory();
+  char *PrivateVars = __kmpc_target_region_kernel_get_private_memory();
+
+  ((ParallelWorkFnTy)WorkFn)(SharedVars, PrivateVars);
+
+  __kmpc_kernel_end_parallel();
+}
+
+__kmpc_barrier_simple_spmd(NULL, 0);
+
+  } while (true);
+}
+
+/// Filter threads into masters and workers. If \p UseStateMachine is true,
+/// required workers will enter a state machine through and be trapped there.
+/// Master and surplus worker threads will return from this function immediately
+/// while required workers will only return once there is no more work. The
+/// return value indicates if the thread is a master (1), a surplus worker (0),
+/// or a finished required worker released from the state machine (-1).
+INLINE static int8_t
+__kmpc_target_region_thread_filter(unsigned ThreadLimit, bool UseStateMachine,
+   bool IsOMPRuntimeInitialized) {
+
+  unsigned TId = GetThreadIdInBlock();
+  bool IsWorker = TId < ThreadLimit;
+
+  if (IsWorker) {
+if (UseStateMachine)
+  __kmpc_target_region_state_machine(IsOMPRuntimeInitialized);
+return -1;
+  }
+
+  return TId == GetMasterThreadID();
+}
+
+EXTERN int8_t __kmpc_target_region_kernel_init(bool UseSPMDMode,
+   bool UseStateMachine,
+   bool RequiresOMPRuntime,
+   bool RequiresDataSharing) {
+  unsigned NumThreads = GetNumberOfThreadsInBlock();
+
+  // Handle the SPMD case first.
+  if (UseSPMDMode) {
+
+__kmpc_spmd_kernel_init(NumThreads, RequiresOMPRuntime,
+RequiresDataSharing);
+
+if (RequiresDataSharing)
+  __kmpc_data_sharing_init_stack_spmd();
+
+return 1;
+  }
+
+  // Reserve one WARP in non-SPMD mode for the masters.
+  unsigned ThreadLimit = NumThreads - WARPSIZE;
+  int8_t FilterVal = __kmpc_target_region_thread_filter(
+  ThreadLimit, UseStateMachine, RequiresOMPRuntime);
+
+  // If the filter returns 1 the executing thread is a team master which will
+  // initialize the kernel in the following.
+  if (FilterVal == 1) {
+__kmpc_kernel_init(ThreadLimit, RequiresOMPRuntime);
+__kmpc_data_sharing_init_stack();
+_target_region_shared_memory.init();
+  }
+
+  return FilterVal;
+}
+
+EXTERN void __kmpc_target_region_kernel_deinit(bool 

[PATCH] D59319: [OpenMP][Offloading][1/3] A generic and simple target region interface

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert created this revision.
jdoerfert added reviewers: ABataev, arpith-jacob, guraypp, gtbercea, hfinkel.
Herald added a project: OpenMP.

This patch introduces an alternative OpenMP GPU kernel offloading
interface called target kernel region (or TRegion).

The commit includes the runtime library implementation for the NVPTX
device plugin, implemented mostly in terms of the existing
functionality.

The interface is deliberately simple to be easily analyzable in the
middle end. Design decisions included:

- Hide all (complex) implementation choices in the runtime library but allow 
complete removal of the abstraction once the runtime is inlined.
- Provide all runtime calls with sufficient, easy encoded information.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59319

Files:
  openmp/libomptarget/cmake/Modules/LibomptargetNVPTXBitcodeLibrary.cmake
  openmp/libomptarget/deviceRTLs/common/target_region.h
  openmp/libomptarget/deviceRTLs/nvptx/CMakeLists.txt
  openmp/libomptarget/deviceRTLs/nvptx/src/omp_data.cu
  openmp/libomptarget/deviceRTLs/nvptx/src/omptarget-nvptx.h
  openmp/libomptarget/deviceRTLs/nvptx/src/target_region.cu

Index: openmp/libomptarget/deviceRTLs/nvptx/src/target_region.cu
===
--- /dev/null
+++ openmp/libomptarget/deviceRTLs/nvptx/src/target_region.cu
@@ -0,0 +1,197 @@
+//===-- target_region.cu  CUDA impl. of the target region interface -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file contains the implementation of the common target region interface.
+//
+//===--===//
+
+// Include the native definitions first as certain defines might be needed in
+// the common interface definition below.
+#include "omptarget-nvptx.h"
+#include "interface.h"
+
+#include "../../common/target_region.h"
+
+/// The pointer used to share memory between team threads.
+extern __device__ __shared__ target_region_shared_buffer
+_target_region_shared_memory;
+
+EXTERN char *__kmpc_target_region_kernel_get_shared_memory() {
+  return _target_region_shared_memory.begin();
+}
+EXTERN char *__kmpc_target_region_kernel_get_private_memory() {
+  return _target_region_shared_memory.begin() +
+ _target_region_shared_memory.get_offset();
+}
+
+/// Simple generic state machine for worker threads.
+INLINE static void
+__kmpc_target_region_state_machine(bool IsOMPRuntimeInitialized) {
+
+  do {
+void *WorkFn = 0;
+
+// Wait for the signal that we have a new work function.
+__kmpc_barrier_simple_spmd(NULL, 0);
+
+// Retrieve the work function from the runtime.
+bool IsActive = __kmpc_kernel_parallel(, IsOMPRuntimeInitialized);
+
+// If there is nothing more to do, break out of the state machine by
+// returning to the caller.
+if (!WorkFn)
+  return;
+
+if (IsActive) {
+  char *SharedVars = __kmpc_target_region_kernel_get_shared_memory();
+  char *PrivateVars = __kmpc_target_region_kernel_get_private_memory();
+
+  ((ParallelWorkFnTy)WorkFn)(SharedVars, PrivateVars);
+
+  __kmpc_kernel_end_parallel();
+}
+
+__kmpc_barrier_simple_spmd(NULL, 0);
+
+  } while (true);
+}
+
+/// Filter threads into masters and workers. If \p UseStateMachine is true,
+/// required workers will enter a state machine through and be trapped there.
+/// Master and surplus worker threads will return from this function immediately
+/// while required workers will only return once there is no more work. The
+/// return value indicates if the thread is a master (1), a surplus worker (0),
+/// or a finished required worker released from the state machine (-1).
+INLINE static int8_t
+__kmpc_target_region_thread_filter(unsigned ThreadLimit, bool UseStateMachine,
+   bool IsOMPRuntimeInitialized) {
+
+  unsigned TId = GetThreadIdInBlock();
+  bool IsWorker = TId < ThreadLimit;
+
+  if (IsWorker) {
+if (UseStateMachine)
+  __kmpc_target_region_state_machine(IsOMPRuntimeInitialized);
+return -1;
+  }
+
+  return TId == GetMasterThreadID();
+}
+
+EXTERN int8_t __kmpc_target_region_kernel_init(bool UseSPMDMode,
+   bool UseStateMachine,
+   bool RequiresOMPRuntime,
+   bool RequiresDataSharing) {
+  unsigned NumThreads = GetNumberOfThreadsInBlock();
+
+  // Handle the SPMD case first.
+  if (UseSPMDMode) {
+
+__kmpc_spmd_kernel_init(NumThreads, RequiresOMPRuntime,
+RequiresDataSharing);
+
+if (RequiresDataSharing)
+  

[PATCH] D59318: Add an overload for ClangTidy's diag method that allows users to provide a diagnostic name rather than using the check name when building a diagnostic.

[Harshal Lehri via Phabricator via cfe-commits]

htl created this revision.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59318

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidy.h


Index: clang-tools-extra/clang-tidy/ClangTidy.h
===
--- clang-tools-extra/clang-tidy/ClangTidy.h
+++ clang-tools-extra/clang-tidy/ClangTidy.h
@@ -165,6 +165,14 @@
   DiagnosticBuilder diag(SourceLocation Loc, StringRef Description,
  DiagnosticIDs::Level Level = DiagnosticIDs::Warning);
 
+  /// \brief Add a diagnostic with a user-specified name.
+  ///
+  /// Similar to the method above, but allows user to specify a DiagnosticName
+  /// of the Diagnostic created.
+  DiagnosticBuilder diag(StringRef DiagnosticName, SourceLocation Loc,
+ StringRef Description,
+ DiagnosticIDs::Level Level = DiagnosticIDs::Warning);
+
   /// \brief Should store all options supported by this check with their
   /// current values or default values for options that haven't been 
overridden.
   ///
Index: clang-tools-extra/clang-tidy/ClangTidy.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -439,6 +439,13 @@
   return Context->diag(CheckName, Loc, Message, Level);
 }
 
+DiagnosticBuilder ClangTidyCheck::diag(StringRef DiagnosticName,
+   SourceLocation Loc,
+   StringRef Description,
+   DiagnosticIDs::Level Level) {
+  return Context->diag(DiagnosticName, Loc, Description, Level);
+}
+
 void ClangTidyCheck::run(const ast_matchers::MatchFinder::MatchResult ) 
{
   // For historical reasons, checks don't implement the MatchFinder run()
   // callback directly. We keep the run()/check() distinction to avoid 
interface


Index: clang-tools-extra/clang-tidy/ClangTidy.h
===
--- clang-tools-extra/clang-tidy/ClangTidy.h
+++ clang-tools-extra/clang-tidy/ClangTidy.h
@@ -165,6 +165,14 @@
   DiagnosticBuilder diag(SourceLocation Loc, StringRef Description,
  DiagnosticIDs::Level Level = DiagnosticIDs::Warning);
 
+  /// \brief Add a diagnostic with a user-specified name.
+  ///
+  /// Similar to the method above, but allows user to specify a DiagnosticName
+  /// of the Diagnostic created.
+  DiagnosticBuilder diag(StringRef DiagnosticName, SourceLocation Loc,
+ StringRef Description,
+ DiagnosticIDs::Level Level = DiagnosticIDs::Warning);
+
   /// \brief Should store all options supported by this check with their
   /// current values or default values for options that haven't been overridden.
   ///
Index: clang-tools-extra/clang-tidy/ClangTidy.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -439,6 +439,13 @@
   return Context->diag(CheckName, Loc, Message, Level);
 }
 
+DiagnosticBuilder ClangTidyCheck::diag(StringRef DiagnosticName,
+   SourceLocation Loc,
+   StringRef Description,
+   DiagnosticIDs::Level Level) {
+  return Context->diag(DiagnosticName, Loc, Description, Level);
+}
+
 void ClangTidyCheck::run(const ast_matchers::MatchFinder::MatchResult ) {
   // For historical reasons, checks don't implement the MatchFinder run()
   // callback directly. We keep the run()/check() distinction to avoid interface
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58514: Avoid needlessly copying blocks that initialize or are assigned to local auto variables to the heap

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Hi ahatanak,

this causes a crash in chrome/ios. A reduced repro is at 
https://bugs.chromium.org/p/chromium/issues/detail?id=941680 . Is the code 
invalid, or is that a bug in the transform?

Thanks,
Nico


Repository:
  rC Clang

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

https://reviews.llvm.org/D58514



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

RE: r354843 - [CGDebugInfo] Set NonTrivial DIFlag to a c++ record if it's not trivial

[via cfe-commits]

Hi Aaron,
The additional tests tell me that every composite type is either Trivial or 
NonTrivial, which does not answer the question: Why do you need two flags? 
DIFlags are not a plentiful resource and we should be reluctant to use them up.
Thanks,
--paulr

From: Aaron Smith [mailto:aaron.sm...@microsoft.com]
Sent: Wednesday, March 13, 2019 4:05 PM
To: Robinson, Paul; dblai...@gmail.com; r...@google.com; apra...@apple.com; 
jdevliegh...@apple.com
Cc: cfe-commits@lists.llvm.org
Subject: Re: r354843 - [CGDebugInfo] Set NonTrivial DIFlag to a c++ record if 
it's not trivial

Hi Paul,

There are additional tests here that may answer your questions,
https://reviews.llvm.org/rGe8475f78e2634d5d348d7ad746efc1e6526e72f5

Aaron

From: "paul.robin...@sony.com" 
Date: Wednesday, March 13, 2019 at 12:49 PM
To: Aaron Smith , "dblai...@gmail.com" 
, "r...@google.com" , "apra...@apple.com" 
, "jdevliegh...@apple.com" 
Cc: "cfe-commits@lists.llvm.org" 
Subject: RE: r354843 - [CGDebugInfo] Set NonTrivial DIFlag to a c++ record if 
it's not trivial

Hi Aaron,
I think I am less clever than David, and it's not clear what the difference is 
between the two flags. In the review, Zach asked the question but I did not see 
a straight answer. What do these flags actually mean? What is the third state, 
with both flags off, implying not Trivial and not NonTrivial?  (At the very 
least it seems that the names could be improved.)
Thanks,
--paulr

From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf Of 
Aaron Smith via cfe-commits
Sent: Monday, March 04, 2019 6:22 PM
To: David Blaikie; Reid Kleckner; Adrian Prantl; Jonas Devlieghere
Cc: cfe-commits@lists.llvm.org
Subject: Re: r354843 - [CGDebugInfo] Set NonTrivial DIFlag to a c++ record if 
it's not trivial

Hi David, I can take a look at adding another test. Please read the code review 
which answers your question. A new flag is required. Thanks.


From: David Blaikie 
Sent: Tuesday, March 5, 2019 12:51:27 AM
To: Aaron Smith; Reid Kleckner; Adrian Prantl; Jonas Devlieghere
Cc: cfe-commits@lists.llvm.org
Subject: Re: r354843 - [CGDebugInfo] Set NonTrivial DIFlag to a c++ record if 
it's not trivial

Hi Aaron,

I don't see any mention of this in D44406 - so it might have been good to have 
a separate review for this (or included this in the review of D44406, which I 
think is possible with the monorepo).

Specifically - this change is missing test coverage (there should be a clang 
test that goes from C++ source code to LLVM IR & demonstrates the flag being 
emitted into the IR, etc).

Also - what's the reason the non-triviality can't be implied by the absence of 
the trivial flag? (or the other way around) - the flags seem redundant with one 
another.
On Mon, Feb 25, 2019 at 8:02 PM Aaron Smith via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: asmith
Date: Mon Feb 25 19:49:05 2019
New Revision: 354843

URL: 
http://llvm.org/viewvc/llvm-project?rev=354843=rev
Log:
[CGDebugInfo] Set NonTrivial DIFlag to a c++ record if it's not trivial

This goes with 
https://reviews.llvm.org/D44406


Modified:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=354843=354842=354843=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Mon Feb 25 19:49:05 2019
@@ -3031,6 +3031,8 @@ llvm::DICompositeType *CGDebugInfo::Crea
 // Record if a C++ record is trivial type.
 if (CXXRD->isTrivial())
   Flags |= llvm::DINode::FlagTrivial;
+else
+  Flags |= llvm::DINode::FlagNonTrivial;
   }

   llvm::DICompositeType *RealDecl = DBuilder.createReplaceableCompositeType(


___
cfe-commits mailing list
cfe-commits@lists.llvm.org

Re: r354843 - [CGDebugInfo] Set NonTrivial DIFlag to a c++ record if it's not trivial

[Aaron Smith via cfe-commits]

Hi Paul,

There are additional tests here that may answer your questions,
https://reviews.llvm.org/rGe8475f78e2634d5d348d7ad746efc1e6526e72f5

Aaron

From: "paul.robin...@sony.com" 
Date: Wednesday, March 13, 2019 at 12:49 PM
To: Aaron Smith , "dblai...@gmail.com" 
, "r...@google.com" , "apra...@apple.com" 
, "jdevliegh...@apple.com" 
Cc: "cfe-commits@lists.llvm.org" 
Subject: RE: r354843 - [CGDebugInfo] Set NonTrivial DIFlag to a c++ record if 
it's not trivial

Hi Aaron,
I think I am less clever than David, and it's not clear what the difference is 
between the two flags. In the review, Zach asked the question but I did not see 
a straight answer. What do these flags actually mean? What is the third state, 
with both flags off, implying not Trivial and not NonTrivial?  (At the very 
least it seems that the names could be improved.)
Thanks,
--paulr

From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf Of 
Aaron Smith via cfe-commits
Sent: Monday, March 04, 2019 6:22 PM
To: David Blaikie; Reid Kleckner; Adrian Prantl; Jonas Devlieghere
Cc: cfe-commits@lists.llvm.org
Subject: Re: r354843 - [CGDebugInfo] Set NonTrivial DIFlag to a c++ record if 
it's not trivial

Hi David, I can take a look at adding another test. Please read the code review 
which answers your question. A new flag is required. Thanks.


From: David Blaikie 
Sent: Tuesday, March 5, 2019 12:51:27 AM
To: Aaron Smith; Reid Kleckner; Adrian Prantl; Jonas Devlieghere
Cc: cfe-commits@lists.llvm.org
Subject: Re: r354843 - [CGDebugInfo] Set NonTrivial DIFlag to a c++ record if 
it's not trivial

Hi Aaron,

I don't see any mention of this in D44406 - so it might have been good to have 
a separate review for this (or included this in the review of D44406, which I 
think is possible with the monorepo).

Specifically - this change is missing test coverage (there should be a clang 
test that goes from C++ source code to LLVM IR & demonstrates the flag being 
emitted into the IR, etc).

Also - what's the reason the non-triviality can't be implied by the absence of 
the trivial flag? (or the other way around) - the flags seem redundant with one 
another.
On Mon, Feb 25, 2019 at 8:02 PM Aaron Smith via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: asmith
Date: Mon Feb 25 19:49:05 2019
New Revision: 354843

URL: 
http://llvm.org/viewvc/llvm-project?rev=354843=rev
Log:
[CGDebugInfo] Set NonTrivial DIFlag to a c++ record if it's not trivial

This goes with 
https://reviews.llvm.org/D44406


Modified:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=354843=354842=354843=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Mon Feb 25 19:49:05 2019
@@ -3031,6 +3031,8 @@ llvm::DICompositeType *CGDebugInfo::Crea
 // Record if a C++ record is trivial type.
 if (CXXRD->isTrivial())
   Flags |= llvm::DINode::FlagTrivial;
+else
+  Flags |= llvm::DINode::FlagNonTrivial;
   }

   llvm::DICompositeType *RealDecl = DBuilder.createReplaceableCompositeType(


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

[PATCH] D59316: [HIP-Clang] propagate -mllvm options to opt and llc

[Aaron Enye Shi via Phabricator via cfe-commits]

ashi1 added a comment.

Hi Matt, that solution will need refactoring and testing. Currently, HIP-Clang 
is following the same link flow as HCC


Repository:
  rC Clang

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

https://reviews.llvm.org/D59316



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

[PATCH] D59316: [HIP-Clang] propagate -mllvm options to opt and llc

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

The real solution is to stop invoking these tools separately. clang -cc1 should 
be used for everything


Repository:
  rC Clang

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

https://reviews.llvm.org/D59316



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

RE: r354843 - [CGDebugInfo] Set NonTrivial DIFlag to a c++ record if it's not trivial

[via cfe-commits]

Hi Aaron,
I think I am less clever than David, and it's not clear what the difference is 
between the two flags. In the review, Zach asked the question but I did not see 
a straight answer. What do these flags actually mean? What is the third state, 
with both flags off, implying not Trivial and not NonTrivial?  (At the very 
least it seems that the names could be improved.)
Thanks,
--paulr

From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf Of 
Aaron Smith via cfe-commits
Sent: Monday, March 04, 2019 6:22 PM
To: David Blaikie; Reid Kleckner; Adrian Prantl; Jonas Devlieghere
Cc: cfe-commits@lists.llvm.org
Subject: Re: r354843 - [CGDebugInfo] Set NonTrivial DIFlag to a c++ record if 
it's not trivial

Hi David, I can take a look at adding another test. Please read the code review 
which answers your question. A new flag is required. Thanks.


From: David Blaikie 
Sent: Tuesday, March 5, 2019 12:51:27 AM
To: Aaron Smith; Reid Kleckner; Adrian Prantl; Jonas Devlieghere
Cc: cfe-commits@lists.llvm.org
Subject: Re: r354843 - [CGDebugInfo] Set NonTrivial DIFlag to a c++ record if 
it's not trivial

Hi Aaron,

I don't see any mention of this in D44406 - so it might have been good to have 
a separate review for this (or included this in the review of D44406, which I 
think is possible with the monorepo).

Specifically - this change is missing test coverage (there should be a clang 
test that goes from C++ source code to LLVM IR & demonstrates the flag being 
emitted into the IR, etc).

Also - what's the reason the non-triviality can't be implied by the absence of 
the trivial flag? (or the other way around) - the flags seem redundant with one 
another.
On Mon, Feb 25, 2019 at 8:02 PM Aaron Smith via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: asmith
Date: Mon Feb 25 19:49:05 2019
New Revision: 354843

URL: 
http://llvm.org/viewvc/llvm-project?rev=354843=rev
Log:
[CGDebugInfo] Set NonTrivial DIFlag to a c++ record if it's not trivial

This goes with 
https://reviews.llvm.org/D44406


Modified:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=354843=354842=354843=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Mon Feb 25 19:49:05 2019
@@ -3031,6 +3031,8 @@ llvm::DICompositeType *CGDebugInfo::Crea
 // Record if a C++ record is trivial type.
 if (CXXRD->isTrivial())
   Flags |= llvm::DINode::FlagTrivial;
+else
+  Flags |= llvm::DINode::FlagNonTrivial;
   }

   llvm::DICompositeType *RealDecl = DBuilder.createReplaceableCompositeType(


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

[PATCH] D58556: [LibTooling] Add retrieval of extended AST-node source to FixIt library

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL356095: [LibTooling] Add retrieval of extended AST-node 
source to FixIt library (authored by ymandel, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58556?vs=190039=190479#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58556

Files:
  cfe/trunk/include/clang/Tooling/FixIt.h
  cfe/trunk/lib/Tooling/FixIt.cpp
  cfe/trunk/unittests/Tooling/FixItTest.cpp

Index: cfe/trunk/include/clang/Tooling/FixIt.h
===
--- cfe/trunk/include/clang/Tooling/FixIt.h
+++ cfe/trunk/include/clang/Tooling/FixIt.h
@@ -20,36 +20,81 @@
 #define LLVM_CLANG_TOOLING_FIXIT_H
 
 #include "clang/AST/ASTContext.h"
+#include "clang/Basic/TokenKinds.h"
 
 namespace clang {
 namespace tooling {
 namespace fixit {
 
 namespace internal {
-StringRef getText(SourceRange Range, const ASTContext );
+StringRef getText(CharSourceRange Range, const ASTContext );
 
-/// Returns the SourceRange of a SourceRange. This identity function is
-///used by the following template abstractions.
-inline SourceRange getSourceRange(const SourceRange ) { return Range; }
+/// Returns the token CharSourceRange corresponding to \p Range.
+inline CharSourceRange getSourceRange(const SourceRange ) {
+  return CharSourceRange::getTokenRange(Range);
+}
 
-/// Returns the SourceRange of the token at Location \p Loc.
-inline SourceRange getSourceRange(const SourceLocation ) {
-  return SourceRange(Loc);
+/// Returns the CharSourceRange of the token at Location \p Loc.
+inline CharSourceRange getSourceRange(const SourceLocation ) {
+  return CharSourceRange::getTokenRange(Loc, Loc);
 }
 
-/// Returns the SourceRange of an given Node. \p Node is typically a
+/// Returns the CharSourceRange of an given Node. \p Node is typically a
 ///'Stmt', 'Expr' or a 'Decl'.
-template  SourceRange getSourceRange(const T ) {
-  return Node.getSourceRange();
+template  CharSourceRange getSourceRange(const T ) {
+  return CharSourceRange::getTokenRange(Node.getSourceRange());
 }
+
+/// Extends \p Range to include the token \p Next, if it immediately follows the
+/// end of the range. Otherwise, returns \p Range unchanged.
+CharSourceRange maybeExtendRange(CharSourceRange Range, tok::TokenKind Next,
+ ASTContext );
 } // end namespace internal
 
-// Returns a textual representation of \p Node.
+/// Returns a textual representation of \p Node.
 template 
 StringRef getText(const T , const ASTContext ) {
   return internal::getText(internal::getSourceRange(Node), Context);
 }
 
+/// Returns the source range spanning the node, extended to include \p Next, if
+/// it immediately follows \p Node. Otherwise, returns the normal range of \p
+/// Node.  See comments on `getExtendedText()` for examples.
+template 
+CharSourceRange getExtendedRange(const T , tok::TokenKind Next,
+ ASTContext ) {
+  return internal::maybeExtendRange(internal::getSourceRange(Node), Next,
+Context);
+}
+
+/// Returns the source text of the node, extended to include \p Next, if it
+/// immediately follows the node. Otherwise, returns the text of just \p Node.
+///
+/// For example, given statements S1 and S2 below:
+/// \code
+///   {
+/// // S1:
+/// if (!x) return foo();
+/// // S2:
+/// if (!x) { return 3; }
+//}
+/// \endcode
+/// then
+/// \code
+///   getText(S1, Context) = "if (!x) return foo()"
+///   getExtendedText(S1, tok::TokenKind::semi, Context)
+/// = "if (!x) return foo();"
+///   getExtendedText(*S1.getThen(), tok::TokenKind::semi, Context)
+/// = "return foo();"
+///   getExtendedText(*S2.getThen(), tok::TokenKind::semi, Context)
+/// = getText(S2, Context) = "{ return 3; }"
+/// \endcode
+template 
+StringRef getExtendedText(const T , tok::TokenKind Next,
+  ASTContext ) {
+  return internal::getText(getExtendedRange(Node, Next, Context), Context);
+}
+
 // Returns a FixItHint to remove \p Node.
 // TODO: Add support for related syntactical elements (i.e. comments, ...).
 template  FixItHint createRemoval(const T ) {
Index: cfe/trunk/lib/Tooling/FixIt.cpp
===
--- cfe/trunk/lib/Tooling/FixIt.cpp
+++ cfe/trunk/lib/Tooling/FixIt.cpp
@@ -18,12 +18,20 @@
 namespace fixit {
 
 namespace internal {
-StringRef getText(SourceRange Range, const ASTContext ) {
-  return Lexer::getSourceText(CharSourceRange::getTokenRange(Range),
-  Context.getSourceManager(),
+StringRef getText(CharSourceRange Range, const ASTContext ) {
+  return Lexer::getSourceText(Range, Context.getSourceManager(),
   Context.getLangOpts());
 

r356095 - [LibTooling] Add retrieval of extended AST-node source to FixIt library

[Yitzhak Mandelbaum via cfe-commits]

Author: ymandel
Date: Wed Mar 13 12:48:51 2019
New Revision: 356095

URL: http://llvm.org/viewvc/llvm-project?rev=356095=rev
Log:
[LibTooling] Add retrieval of extended AST-node source to FixIt library

Summary:
Introduces variants of `getText` and `getSourceRange` that extract the source 
text of an AST node potentially with a trailing token.

Some of the new functions manipulate `CharSourceRange`s, rather than 
`SourceRange`s, because they document and dynamically enforce their type.  So, 
this revision also updates the corresponding existing FixIt functions to 
manipulate `CharSourceRange`s.  This change is not strictly necessary, but 
seems like the correct choice, to keep the API self-consistent.

This revision is the first in a series intended to improve the abstractions 
available to users for writing source-to-source transformations.  A full 
discussion of the end goal can be found on the cfe-dev list with subject "[RFC] 
Easier source-to-source transformations with clang tooling".

Reviewers: ilya-biryukov

Reviewed By: ilya-biryukov

Subscribers: kimgr, riccibruno, JonasToth, jdoerfert, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/include/clang/Tooling/FixIt.h
cfe/trunk/lib/Tooling/FixIt.cpp
cfe/trunk/unittests/Tooling/FixItTest.cpp

Modified: cfe/trunk/include/clang/Tooling/FixIt.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/FixIt.h?rev=356095=356094=356095=diff
==
--- cfe/trunk/include/clang/Tooling/FixIt.h (original)
+++ cfe/trunk/include/clang/Tooling/FixIt.h Wed Mar 13 12:48:51 2019
@@ -20,36 +20,81 @@
 #define LLVM_CLANG_TOOLING_FIXIT_H
 
 #include "clang/AST/ASTContext.h"
+#include "clang/Basic/TokenKinds.h"
 
 namespace clang {
 namespace tooling {
 namespace fixit {
 
 namespace internal {
-StringRef getText(SourceRange Range, const ASTContext );
+StringRef getText(CharSourceRange Range, const ASTContext );
 
-/// Returns the SourceRange of a SourceRange. This identity function is
-///used by the following template abstractions.
-inline SourceRange getSourceRange(const SourceRange ) { return Range; }
+/// Returns the token CharSourceRange corresponding to \p Range.
+inline CharSourceRange getSourceRange(const SourceRange ) {
+  return CharSourceRange::getTokenRange(Range);
+}
 
-/// Returns the SourceRange of the token at Location \p Loc.
-inline SourceRange getSourceRange(const SourceLocation ) {
-  return SourceRange(Loc);
+/// Returns the CharSourceRange of the token at Location \p Loc.
+inline CharSourceRange getSourceRange(const SourceLocation ) {
+  return CharSourceRange::getTokenRange(Loc, Loc);
 }
 
-/// Returns the SourceRange of an given Node. \p Node is typically a
+/// Returns the CharSourceRange of an given Node. \p Node is typically a
 ///'Stmt', 'Expr' or a 'Decl'.
-template  SourceRange getSourceRange(const T ) {
-  return Node.getSourceRange();
+template  CharSourceRange getSourceRange(const T ) {
+  return CharSourceRange::getTokenRange(Node.getSourceRange());
 }
+
+/// Extends \p Range to include the token \p Next, if it immediately follows 
the
+/// end of the range. Otherwise, returns \p Range unchanged.
+CharSourceRange maybeExtendRange(CharSourceRange Range, tok::TokenKind Next,
+ ASTContext );
 } // end namespace internal
 
-// Returns a textual representation of \p Node.
+/// Returns a textual representation of \p Node.
 template 
 StringRef getText(const T , const ASTContext ) {
   return internal::getText(internal::getSourceRange(Node), Context);
 }
 
+/// Returns the source range spanning the node, extended to include \p Next, if
+/// it immediately follows \p Node. Otherwise, returns the normal range of \p
+/// Node.  See comments on `getExtendedText()` for examples.
+template 
+CharSourceRange getExtendedRange(const T , tok::TokenKind Next,
+ ASTContext ) {
+  return internal::maybeExtendRange(internal::getSourceRange(Node), Next,
+Context);
+}
+
+/// Returns the source text of the node, extended to include \p Next, if it
+/// immediately follows the node. Otherwise, returns the text of just \p Node.
+///
+/// For example, given statements S1 and S2 below:
+/// \code
+///   {
+/// // S1:
+/// if (!x) return foo();
+/// // S2:
+/// if (!x) { return 3; }
+//}
+/// \endcode
+/// then
+/// \code
+///   getText(S1, Context) = "if (!x) return foo()"
+///   getExtendedText(S1, tok::TokenKind::semi, Context)
+/// = "if (!x) return foo();"
+///   getExtendedText(*S1.getThen(), tok::TokenKind::semi, Context)
+/// = "return foo();"
+///   getExtendedText(*S2.getThen(), tok::TokenKind::semi, Context)
+/// = getText(S2, Context) = "{ return 3; }"
+/// \endcode
+template 
+StringRef getExtendedText(const T , tok::TokenKind Next,
+   

[PATCH] D59316: [HIP-Clang] propagate -mllvm options to opt and llc

[Aaron Enye Shi via Phabricator via cfe-commits]

ashi1 created this revision.
ashi1 added a reviewer: yaxunl.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

I've updated the HIP ToolChain to pass OPT_mllvm options into OPT and LLC 
stages.

Also added a lit test to verify its properly passed down. All check-clang tests 
passing.


Repository:
  rC Clang

https://reviews.llvm.org/D59316

Files:
  lib/Driver/ToolChains/HIP.cpp
  test/Driver/hip-toolchain-mllvm.hip


Index: test/Driver/hip-toolchain-mllvm.hip
===
--- test/Driver/hip-toolchain-mllvm.hip
+++ test/Driver/hip-toolchain-mllvm.hip
@@ -0,0 +1,36 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 \
+// RUN:   -mllvm -amdgpu-function-calls=0 \
+// RUN:   %s 2>&1 | FileCheck %s
+
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu" "-emit-llvm-bc"
+// CHECK-SAME: {{.*}} "-target-cpu" "gfx803"
+// CHECK-SAME: {{.*}} "-mllvm" "-amdgpu-function-calls=0" {{.*}}
+
+// CHECK: [[OPT:".*opt"]] {{".*-gfx803-linked.*bc"}} 
"-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: "-mcpu=gfx803" "-amdgpu-function-calls=0"
+// CHECK-SAME: "-o" [[OPT_803_BC:".*-gfx803-optimized.*bc"]]
+
+// CHECK: [[LLC: ".*llc"]] [[OPT_803_BC]]
+// CHECK-SAME: "-mtriple=amdgcn-amd-amdhsa" "-filetype=obj"
+// CHECK-SAME: {{.*}} "-mcpu=gfx803"
+// CHECK-SAME: "-amdgpu-function-calls=0" "-o" {{".*-gfx803-.*o"}}
+
+// CHECK: [[CLANG]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu" "-emit-llvm-bc"
+// CHECK-SAME: {{.*}} "-target-cpu" "gfx900"
+// CHECK-SAME: {{.*}} "-mllvm" "-amdgpu-function-calls=0" {{.*}}
+
+// CHECK: [[OPT]] {{".*-gfx900-linked.*bc"}} "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: "-mcpu=gfx900" "-amdgpu-function-calls=0"
+// CHECK-SAME: "-o" [[OPT_900_BC:".*-gfx900-optimized.*bc"]]
+
+// CHECK: [[LLC]] [[OPT_900_BC]]
+// CHECK-SAME: "-mtriple=amdgcn-amd-amdhsa" "-filetype=obj"
+// CHECK-SAME: {{.*}} "-mcpu=gfx900"
+// CHECK-SAME: "-amdgpu-function-calls=0" "-o" {{".*-gfx900-.*o"}}
Index: lib/Driver/ToolChains/HIP.cpp
===
--- lib/Driver/ToolChains/HIP.cpp
+++ lib/Driver/ToolChains/HIP.cpp
@@ -140,6 +140,11 @@
   }
   OptArgs.push_back("-mtriple=amdgcn-amd-amdhsa");
   OptArgs.push_back(Args.MakeArgString("-mcpu=" + SubArchName));
+
+  for (const Arg *A : Args.filtered(options::OPT_mllvm)) {
+OptArgs.push_back(A->getValue(0));
+  }
+
   OptArgs.push_back("-o");
   std::string TmpFileName = C.getDriver().GetTemporaryPath(
   OutputFilePrefix.str() + "-optimized", "bc");
@@ -177,6 +182,10 @@
   if(!Features.empty())
 LlcArgs.push_back(Args.MakeArgString(MAttrString));

+  for (const Arg *A : Args.filtered(options::OPT_mllvm)) {
+LlcArgs.push_back(A->getValue(0));
+  }
+
   // Add output filename
   LlcArgs.push_back("-o");
   std::string LlcOutputFileName =


Index: test/Driver/hip-toolchain-mllvm.hip
===
--- test/Driver/hip-toolchain-mllvm.hip
+++ test/Driver/hip-toolchain-mllvm.hip
@@ -0,0 +1,36 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN: %clang -### -target x86_64-linux-gnu \
+// RUN:   -x hip --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 \
+// RUN:   -mllvm -amdgpu-function-calls=0 \
+// RUN:   %s 2>&1 | FileCheck %s
+
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu" "-emit-llvm-bc"
+// CHECK-SAME: {{.*}} "-target-cpu" "gfx803"
+// CHECK-SAME: {{.*}} "-mllvm" "-amdgpu-function-calls=0" {{.*}}
+
+// CHECK: [[OPT:".*opt"]] {{".*-gfx803-linked.*bc"}} "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: "-mcpu=gfx803" "-amdgpu-function-calls=0"
+// CHECK-SAME: "-o" [[OPT_803_BC:".*-gfx803-optimized.*bc"]]
+
+// CHECK: [[LLC: ".*llc"]] [[OPT_803_BC]]
+// CHECK-SAME: "-mtriple=amdgcn-amd-amdhsa" "-filetype=obj"
+// CHECK-SAME: {{.*}} "-mcpu=gfx803"
+// CHECK-SAME: "-amdgpu-function-calls=0" "-o" {{".*-gfx803-.*o"}}
+
+// CHECK: [[CLANG]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
+// CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu" "-emit-llvm-bc"
+// CHECK-SAME: {{.*}} "-target-cpu" "gfx900"
+// CHECK-SAME: {{.*}} "-mllvm" "-amdgpu-function-calls=0" {{.*}}
+
+// CHECK: [[OPT]] {{".*-gfx900-linked.*bc"}} "-mtriple=amdgcn-amd-amdhsa"
+// CHECK-SAME: "-mcpu=gfx900" "-amdgpu-function-calls=0"
+// CHECK-SAME: "-o" [[OPT_900_BC:".*-gfx900-optimized.*bc"]]
+
+// CHECK: [[LLC]] [[OPT_900_BC]]
+// CHECK-SAME: "-mtriple=amdgcn-amd-amdhsa" "-filetype=obj"
+// CHECK-SAME: {{.*}} "-mcpu=gfx900"
+// CHECK-SAME: "-amdgpu-function-calls=0" "-o" {{".*-gfx900-.*o"}}
Index: 

[PATCH] D59048: Add AIX Target Info

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: test/Headers/max_align.c:1
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// expected-no-diagnostics

lebedev.ri wrote:
> hubert.reinterpretcast wrote:
> > We may need to explicitly specify C11. It also seems that we should XFAIL 
> > Windows targets.
> Does this need an explicit `-triple=???` ?
I don't think this test is target-specific, I read the specifications (as 
opposed to the implementations) of `max_align_t` and `__BIGGEST_ALIGNMENT__` as 
saying that `_Alignof(max_align_t)` should be the same as 
`__BIGGEST_ALIGNMENT__`.

Granted, it seems `max_align_t` on Windows does not match 
`__BIGGEST_ALIGNMENT__`. So, a custom `malloc` implementation querying on one 
might not align sufficiently based on the other.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59048



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

[PATCH] D59287: [X86] Only define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 in 64-bit mode.

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

Is this ok with the backend fixed? Or do you want me factor this into HasCX16 
which I think is only used by the defineMacro and the return for 
hasFeature("cx16")? And I think hasFeature("cx16") is only used by that 
getMaxAtomicWidth() code which is only called on 64 bit.

Or we could maybe ignore "cx16" in setFeatureEnabled on 32 bit targets? But I 
think that would break always_inline on a target attribute with cx16 in 32 bit 
mode which gcc does allow. https://godbolt.org/z/TW985s


Repository:
  rC Clang

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

https://reviews.llvm.org/D59287



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

r356089 - [OPENMP]Disable ADL in C for user-defined reductions.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Wed Mar 13 12:31:34 2019
New Revision: 356089

URL: http://llvm.org/viewvc/llvm-project?rev=356089=rev
Log:
[OPENMP]Disable ADL in C for user-defined reductions.

C does not support ADL, disable it for C to prevent compiler crash.

Modified:
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/declare_reduction_messages.c

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=356089=356088=356089=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Wed Mar 13 12:31:34 2019
@@ -10958,35 +10958,37 @@ buildDeclareReductionRef(Sema ,
 }
   }
   // Perform ADL.
-  argumentDependentLookup(SemaRef, ReductionId, Loc, Ty, Lookups);
-  if (auto *VD = filterLookupForUDReductionAndMapper(
-  Lookups, [, Ty](ValueDecl *D) -> ValueDecl * {
-if (!D->isInvalidDecl() &&
-SemaRef.Context.hasSameType(D->getType(), Ty))
-  return D;
-return nullptr;
-  }))
-return SemaRef.BuildDeclRefExpr(VD, VD->getType().getNonReferenceType(),
-VK_LValue, Loc);
-  if (auto *VD = filterLookupForUDReductionAndMapper(
-  Lookups, [, Ty, Loc](ValueDecl *D) -> ValueDecl * {
-if (!D->isInvalidDecl() &&
-SemaRef.IsDerivedFrom(Loc, Ty, D->getType()) &&
-!Ty.isMoreQualifiedThan(D->getType()))
-  return D;
-return nullptr;
-  })) {
-CXXBasePaths Paths(/*FindAmbiguities=*/true, /*RecordPaths=*/true,
-   /*DetectVirtual=*/false);
-if (SemaRef.IsDerivedFrom(Loc, Ty, VD->getType(), Paths)) {
-  if (!Paths.isAmbiguous(SemaRef.Context.getCanonicalType(
-  VD->getType().getUnqualifiedType( {
-if (SemaRef.CheckBaseClassAccess(Loc, VD->getType(), Ty, Paths.front(),
- /*DiagID=*/0) !=
-Sema::AR_inaccessible) {
-  SemaRef.BuildBasePathArray(Paths, BasePath);
-  return SemaRef.BuildDeclRefExpr(
-  VD, VD->getType().getNonReferenceType(), VK_LValue, Loc);
+  if (SemaRef.getLangOpts().CPlusPlus) {
+argumentDependentLookup(SemaRef, ReductionId, Loc, Ty, Lookups);
+if (auto *VD = filterLookupForUDReductionAndMapper(
+Lookups, [, Ty](ValueDecl *D) -> ValueDecl * {
+  if (!D->isInvalidDecl() &&
+  SemaRef.Context.hasSameType(D->getType(), Ty))
+return D;
+  return nullptr;
+}))
+  return SemaRef.BuildDeclRefExpr(VD, VD->getType().getNonReferenceType(),
+  VK_LValue, Loc);
+if (auto *VD = filterLookupForUDReductionAndMapper(
+Lookups, [, Ty, Loc](ValueDecl *D) -> ValueDecl * {
+  if (!D->isInvalidDecl() &&
+  SemaRef.IsDerivedFrom(Loc, Ty, D->getType()) &&
+  !Ty.isMoreQualifiedThan(D->getType()))
+return D;
+  return nullptr;
+})) {
+  CXXBasePaths Paths(/*FindAmbiguities=*/true, /*RecordPaths=*/true,
+ /*DetectVirtual=*/false);
+  if (SemaRef.IsDerivedFrom(Loc, Ty, VD->getType(), Paths)) {
+if (!Paths.isAmbiguous(SemaRef.Context.getCanonicalType(
+VD->getType().getUnqualifiedType( {
+  if (SemaRef.CheckBaseClassAccess(
+  Loc, VD->getType(), Ty, Paths.front(),
+  /*DiagID=*/0) != Sema::AR_inaccessible) {
+SemaRef.BuildBasePathArray(Paths, BasePath);
+return SemaRef.BuildDeclRefExpr(
+VD, VD->getType().getNonReferenceType(), VK_LValue, Loc);
+  }
 }
   }
 }

Modified: cfe/trunk/test/OpenMP/declare_reduction_messages.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/declare_reduction_messages.c?rev=356089=356088=356089=diff
==
--- cfe/trunk/test/OpenMP/declare_reduction_messages.c (original)
+++ cfe/trunk/test/OpenMP/declare_reduction_messages.c Wed Mar 13 12:31:34 2019
@@ -41,7 +41,17 @@ int temp; // expected-note 6 {{'temp' de
 #pragma omp declare reduction(fun8 : long : omp_out += omp_in) 
initializer(omp_priv = 23)) // expected-warning {{extra tokens at the end of 
'#pragma omp declare reduction' are ignored}} expected-error {{redefinition of 
user-defined reduction for type 'long'}}
 #pragma omp declare reduction(fun9 : long : omp_out += omp_in) 
initializer(omp_priv = )// expected-error {{expected expression}}
 
+struct S {
+  int s;
+};
+#pragma omp declare reduction(+: struct S: omp_out.s += omp_in.s) // 
initializer(omp_priv = { .s = 0 })
+
 int fun(int arg) {
+  struct S s;// expected-note {{'s' defined here}}
+  s.s = 0;
+#pragma omp 

[PATCH] D59279: [Analyzer] Checker for non-determinism caused by iteration of unordered container of pointers

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang updated this revision to Diff 190472.

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

https://reviews.llvm.org/D59279

Files:
  docs/analyzer/checkers.rst
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/PointerIterationChecker.cpp
  test/Analysis/Inputs/system-header-simulator-cxx.h
  test/Analysis/ptr-iter.cpp
  www/analyzer/alpha_checks.html

Index: www/analyzer/alpha_checks.html
===
--- www/analyzer/alpha_checks.html
+++ www/analyzer/alpha_checks.html
@@ -1182,6 +1182,24 @@
 Name, DescriptionExample
 
 
+
+alpha.nondeterminism.PointerIteration
+(C++)
+Check for non-determinism caused by iterating unordered containers of pointers.
+
+
+// C++
+void test() {
+ int a = 1, b = 2;
+ std::unordered_set UnorderedPtrSet = {, };
+
+ for (auto i : UnorderedPtrSet) // warn
+   f(i);
+}
+
+
+
+
 
 alpha.nondeterminism.PointerSorting
 (C++)
Index: test/Analysis/ptr-iter.cpp
===
--- /dev/null
+++ test/Analysis/ptr-iter.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_analyze_cc1 %s -analyzer-output=text -verify \
+// RUN: -analyzer-checker=core,alpha.nondeterminism.PointerIteration
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+template
+void f(T x);
+
+void PointerIteration() {
+  int a = 1, b = 2;
+  std::set OrderedIntSet = {a, b};
+  std::set OrderedPtrSet = {, };
+  std::unordered_set UnorderedIntSet = {a, b};
+  std::unordered_set UnorderedPtrSet = {, };
+
+  for (auto i : OrderedIntSet) // no-warning
+f(i);
+
+  for (auto i : OrderedPtrSet) // no-warning
+f(i);
+
+  for (auto i : UnorderedIntSet) // no-warning
+f(i);
+
+  for (auto i : UnorderedPtrSet) // expected-warning {{Iteration of pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerIteration]
+// expected-note@-1 {{Iteration of pointer-like elements can result in non-deterministic ordering}} [alpha.nondeterminism.PointerIteration]
+f(i);
+}
Index: test/Analysis/Inputs/system-header-simulator-cxx.h
===
--- test/Analysis/Inputs/system-header-simulator-cxx.h
+++ test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -845,3 +845,64 @@
   template
   BidirIt stable_partition(BidirIt first, BidirIt last, UnaryPredicate p);
 }
+
+namespace std {
+
+template< class T = void >
+struct less;
+
+template< class T >
+struct allocator;
+
+template< class Key >
+struct hash;
+
+template<
+  class Key,
+  class Compare = std::less,
+  class Alloc = std::allocator
+> class set {
+  public:
+set(initializer_list __list) {}
+
+class iterator {
+public:
+  iterator(Key *key): ptr(key) {}
+  iterator operator++() { ++ptr; return *this; }
+  bool operator!=(const iterator ) const { return ptr != other.ptr; }
+  const Key *() const { return *ptr; }
+private:
+  Key *ptr;
+};
+
+  public:
+Key *val;
+iterator begin() const { return iterator(val); }
+iterator end() const { return iterator(val + 1); }
+};
+
+template<
+  class Key,
+  class Hash = std::hash,
+  class Compare = std::less,
+  class Alloc = std::allocator
+> class unordered_set {
+  public:
+unordered_set(initializer_list __list) {}
+
+class iterator {
+public:
+  iterator(Key *key): ptr(key) {}
+  iterator operator++() { ++ptr; return *this; }
+  bool operator!=(const iterator ) const { return ptr != other.ptr; }
+  const Key *() const { return *ptr; }
+private:
+  Key *ptr;
+};
+
+  public:
+Key *val;
+iterator begin() const { return iterator(val); }
+iterator end() const { return iterator(val + 1); }
+};
+}
Index: lib/StaticAnalyzer/Checkers/PointerIterationChecker.cpp
===
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/PointerIterationChecker.cpp
@@ -0,0 +1,100 @@
+//== PointerIterationChecker.cpp --- -*- C++ -*--=//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file defines PointerIterationChecker which checks for non-determinism
+// caused due to iteration of unordered containers of pointer elements.
+//
+//===--===//
+
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+
+using namespace clang;
+using namespace ento;
+using namespace ast_matchers;
+
+namespace {
+
+// ID 

r356088 - [Analyzer] Clean up test/Analysis/ptr-sort.cpp

[Mandeep Singh Grang via cfe-commits]

Author: mgrang
Date: Wed Mar 13 12:21:11 2019
New Revision: 356088

URL: http://llvm.org/viewvc/llvm-project?rev=356088=rev
Log:
[Analyzer] Clean up test/Analysis/ptr-sort.cpp

Modified:
cfe/trunk/test/Analysis/ptr-sort.cpp

Modified: cfe/trunk/test/Analysis/ptr-sort.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/ptr-sort.cpp?rev=356088=356087=356088=diff
==
--- cfe/trunk/test/Analysis/ptr-sort.cpp (original)
+++ cfe/trunk/test/Analysis/ptr-sort.cpp Wed Mar 13 12:21:11 2019
@@ -1,12 +1,13 @@
-// RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.nondeterminism.PointerSorting %s 
-analyzer-output=text -verify
+// RUN: %clang_analyze_cc1 %s -analyzer-output=text -verify \
+// RUN: -analyzer-checker=core,alpha.nondeterminism.PointerSorting
 
 #include "Inputs/system-header-simulator-cxx.h"
 
-bool f (int x) { return true; }
-bool g (int *x) { return true; }
+bool f(int x) { return true; }
+bool g(int *x) { return true; }
 
 void PointerSorting() {
-  int a = 1, b = 2, c = 3;
+  int a = 1, b = 2;
   std::vector V1 = {a, b};
   std::vector V2 = {, };
 


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

r356087 - Fix a failing test.

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Wed Mar 13 12:20:45 2019
New Revision: 356087

URL: http://llvm.org/viewvc/llvm-project?rev=356087=rev
Log:
Fix a failing test.

Modified:
cfe/trunk/test/Parser/pragma-attribute-context.cpp

Modified: cfe/trunk/test/Parser/pragma-attribute-context.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/pragma-attribute-context.cpp?rev=356087=356086=356087=diff
==
--- cfe/trunk/test/Parser/pragma-attribute-context.cpp (original)
+++ cfe/trunk/test/Parser/pragma-attribute-context.cpp Wed Mar 13 12:20:45 2019
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -verify -std=c++11 %s
-// RUN: %clang_cc1 -xobjective-c++ -verify -std=c++11 %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin9.0.0 -verify -std=c++11 %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin9.0.0 -xobjective-c++ -verify 
-std=c++11 %s
 
 #define BEGIN_PRAGMA _Pragma("clang attribute push 
(__attribute__((availability(macos, introduced=1000))), apply_to=function)")
 #define END_PRAGMA _Pragma("clang attribute pop")


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

[PATCH] D59048: Add AIX Target Info

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: test/Headers/max_align.c:1
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// expected-no-diagnostics

hubert.reinterpretcast wrote:
> We may need to explicitly specify C11. It also seems that we should XFAIL 
> Windows targets.
Does this need an explicit `-triple=???` ?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59048



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

[PATCH] D59279: [Analyzer] Checker for non-determinism caused by iteration of unordered container of pointers

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang added a comment.

> Could you please add a commit that changes the top of the file on your 
> previous commit? Feel free to do that without review (I can't commit it 
> myself atm).

Done in https://reviews.llvm.org/rC356086.


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

https://reviews.llvm.org/D59279



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

[PATCH] D59048: Add AIX Target Info

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: test/Headers/max_align.c:1
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// expected-no-diagnostics

We may need to explicitly specify C11. It also seems that we should XFAIL 
Windows targets.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59048



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

r356086 - [Analyzer] Update the LLVM license in PointerSortingChecker.cpp

[Mandeep Singh Grang via cfe-commits]

Author: mgrang
Date: Wed Mar 13 12:09:48 2019
New Revision: 356086

URL: http://llvm.org/viewvc/llvm-project?rev=356086=rev
Log:
[Analyzer] Update the LLVM license in PointerSortingChecker.cpp

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp?rev=356086=356085=356086=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp Wed Mar 13 
12:09:48 2019
@@ -1,9 +1,8 @@
-//=== PointerSortingChecker.cpp - Pointer sorting checker --*- C++ 
-*--===//
+//== PointerSortingChecker.cpp - -*- C++ 
-*--=//
 //
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 
//===--===//
 //


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

[PATCH] D59279: [Analyzer] Checker for non-determinism caused by iteration of unordered container of pointers

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

Could you please add a commit that changes the top of the file on your previous 
commit? Feel free to do that without review (I can't commit it myself atm).

In D59279#1427803 , @mgrang wrote:

> Also, what is the difference between these two: 
> https://clang.llvm.org/docs/analyzer/checkers.html, 
> https://clang-analyzer.llvm.org/available_checks.html. It seems they document 
> similar stuff. Should we add the doc for each new checker in both of these?


Umm, yea, we're in the middle of moving from the HTML format to the new sphinx 
format, so I think it's fine to ignore the latter, and only extend 
`docs/analyzer/checkers.rst`. Since the moving is a lot of manual labor, we 
haven't gotten around getting rid of it entirely just yet. But of course, it's 
welcome

In D59279#1427798 , @mgrang wrote:

> > The obvious question, why not implement this in clang-tidy?
>
> Going forward I would like to make these non-determinism checks more precise 
> by reasoning about what happens inside the loop, for example. I am not sure 
> if clang-tidy has support for such deeper reasoning. I thought clang-tidy is 
> more of a pattern matcher?


Yup, I'm sold on that.


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

https://reviews.llvm.org/D59279



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

[PATCH] D59283: Fixed global constant/variable naming check on C++ class for ObjC++ files.

[Yan Zhang via Phabricator via cfe-commits]

Wizard updated this revision to Diff 190462.
Wizard added a comment.

fix ObjC++ test


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59283

Files:
  clang-tidy/google/GlobalVariableDeclarationCheck.cpp
  test/clang-tidy/google-objc-global-variable-declaration.mm


Index: test/clang-tidy/google-objc-global-variable-declaration.mm
===
--- /dev/null
+++ test/clang-tidy/google-objc-global-variable-declaration.mm
@@ -0,0 +1,10 @@
+// RUN: %check_clang_tidy %s google-objc-global-variable-declaration %t
+
+@class NSString;
+static NSString* const myConstString = @"hello";
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable 
'myConstString' must have a name which starts with an appropriate prefix 
[google-objc-global-variable-declaration]
+// CHECK-FIXES: static NSString* const kMyConstString = @"hello";
+
+class MyTest {
+static int not_objc_style;
+};
\ No newline at end of file
Index: clang-tidy/google/GlobalVariableDeclarationCheck.cpp
===
--- clang-tidy/google/GlobalVariableDeclarationCheck.cpp
+++ clang-tidy/google/GlobalVariableDeclarationCheck.cpp
@@ -79,12 +79,16 @@
 void GlobalVariableDeclarationCheck::check(
 const MatchFinder::MatchResult ) {
   if (const auto *Decl = Result.Nodes.getNodeAs("global_var")) {
+if (Decl->isStaticDataMember())
+  return;
 diag(Decl->getLocation(),
  "non-const global variable '%0' must have a name which starts with "
  "'g[A-Z]'")
 << Decl->getName() << generateFixItHint(Decl, false);
   }
   if (const auto *Decl = Result.Nodes.getNodeAs("global_const")) {
+if (Decl->isStaticDataMember())
+  return;
 diag(Decl->getLocation(),
  "const global variable '%0' must have a name which starts with "
  "an appropriate prefix")


Index: test/clang-tidy/google-objc-global-variable-declaration.mm
===
--- /dev/null
+++ test/clang-tidy/google-objc-global-variable-declaration.mm
@@ -0,0 +1,10 @@
+// RUN: %check_clang_tidy %s google-objc-global-variable-declaration %t
+
+@class NSString;
+static NSString* const myConstString = @"hello";
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: const global variable 'myConstString' must have a name which starts with an appropriate prefix [google-objc-global-variable-declaration]
+// CHECK-FIXES: static NSString* const kMyConstString = @"hello";
+
+class MyTest {
+static int not_objc_style;
+};
\ No newline at end of file
Index: clang-tidy/google/GlobalVariableDeclarationCheck.cpp
===
--- clang-tidy/google/GlobalVariableDeclarationCheck.cpp
+++ clang-tidy/google/GlobalVariableDeclarationCheck.cpp
@@ -79,12 +79,16 @@
 void GlobalVariableDeclarationCheck::check(
 const MatchFinder::MatchResult ) {
   if (const auto *Decl = Result.Nodes.getNodeAs("global_var")) {
+if (Decl->isStaticDataMember())
+  return;
 diag(Decl->getLocation(),
  "non-const global variable '%0' must have a name which starts with "
  "'g[A-Z]'")
 << Decl->getName() << generateFixItHint(Decl, false);
   }
   if (const auto *Decl = Result.Nodes.getNodeAs("global_const")) {
+if (Decl->isStaticDataMember())
+  return;
 diag(Decl->getLocation(),
  "const global variable '%0' must have a name which starts with "
  "an appropriate prefix")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59283: Fixed global constant/variable naming check on C++ class for ObjC++ files.

[Yan Zhang via Phabricator via cfe-commits]

Wizard updated this revision to Diff 190461.
Wizard added a comment.

Resolve comments


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59283

Files:
  clang-tidy/google/GlobalVariableDeclarationCheck.cpp
  test/clang-tidy/google-objc-global-variable-declaration.mm


Index: test/clang-tidy/google-objc-global-variable-declaration.mm
===
--- /dev/null
+++ test/clang-tidy/google-objc-global-variable-declaration.mm
@@ -0,0 +1,5 @@
+// RUN: %check_clang_tidy %s google-objc-global-variable-declaration %t
+
+class MyTest {
+static int not_objc_style;
+};
\ No newline at end of file
Index: clang-tidy/google/GlobalVariableDeclarationCheck.cpp
===
--- clang-tidy/google/GlobalVariableDeclarationCheck.cpp
+++ clang-tidy/google/GlobalVariableDeclarationCheck.cpp
@@ -79,12 +79,16 @@
 void GlobalVariableDeclarationCheck::check(
 const MatchFinder::MatchResult ) {
   if (const auto *Decl = Result.Nodes.getNodeAs("global_var")) {
+if (Decl->isStaticDataMember())
+  return;
 diag(Decl->getLocation(),
  "non-const global variable '%0' must have a name which starts with "
  "'g[A-Z]'")
 << Decl->getName() << generateFixItHint(Decl, false);
   }
   if (const auto *Decl = Result.Nodes.getNodeAs("global_const")) {
+if (Decl->isStaticDataMember())
+  return;
 diag(Decl->getLocation(),
  "const global variable '%0' must have a name which starts with "
  "an appropriate prefix")


Index: test/clang-tidy/google-objc-global-variable-declaration.mm
===
--- /dev/null
+++ test/clang-tidy/google-objc-global-variable-declaration.mm
@@ -0,0 +1,5 @@
+// RUN: %check_clang_tidy %s google-objc-global-variable-declaration %t
+
+class MyTest {
+static int not_objc_style;
+};
\ No newline at end of file
Index: clang-tidy/google/GlobalVariableDeclarationCheck.cpp
===
--- clang-tidy/google/GlobalVariableDeclarationCheck.cpp
+++ clang-tidy/google/GlobalVariableDeclarationCheck.cpp
@@ -79,12 +79,16 @@
 void GlobalVariableDeclarationCheck::check(
 const MatchFinder::MatchResult ) {
   if (const auto *Decl = Result.Nodes.getNodeAs("global_var")) {
+if (Decl->isStaticDataMember())
+  return;
 diag(Decl->getLocation(),
  "non-const global variable '%0' must have a name which starts with "
  "'g[A-Z]'")
 << Decl->getName() << generateFixItHint(Decl, false);
   }
   if (const auto *Decl = Result.Nodes.getNodeAs("global_const")) {
+if (Decl->isStaticDataMember())
+  return;
 diag(Decl->getLocation(),
  "const global variable '%0' must have a name which starts with "
  "an appropriate prefix")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59307: Patch llvm bug 41033 concerning atomicity of statement expressions

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

This is my concern here: 
https://godbolt.org/z/icS4fa
The patch will change template instantiation.

GCC doesn't seem to allow using _Atomic in C++ mode, which is perhaps a 
necessary part of this solution?  I see we already consider overload sets with 
int and _Atomic(int) to be ambiguous (in addition to not considering conversion 
operators of _Atomic(int) at all??), so it is limited to type deduction. I 
could see this causing problems, since specializations of templates are allowed 
on int and _Atomic(int) despite not being used in overload sets; 
https://godbolt.org/z/oMAvpz


Repository:
  rC Clang

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

https://reviews.llvm.org/D59307



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

[PATCH] D59282: [Parse] Parse '#pragma clang attribute' as an external-declaration

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL356075: [Parse] Parse #pragma clang attribute as 
an external-declaration (authored by epilk, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59282?vs=190367=190458#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59282

Files:
  cfe/trunk/lib/Parse/Parser.cpp
  cfe/trunk/test/Parser/pragma-attribute-context.cpp


Index: cfe/trunk/test/Parser/pragma-attribute-context.cpp
===
--- cfe/trunk/test/Parser/pragma-attribute-context.cpp
+++ cfe/trunk/test/Parser/pragma-attribute-context.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -verify -std=c++11 %s
+// RUN: %clang_cc1 -xobjective-c++ -verify -std=c++11 %s
+
+#define BEGIN_PRAGMA _Pragma("clang attribute push 
(__attribute__((availability(macos, introduced=1000))), apply_to=function)")
+#define END_PRAGMA _Pragma("clang attribute pop")
+
+extern "C" {
+BEGIN_PRAGMA
+int f(); // expected-note{{'f' has been marked as being introduced in macOS 
1000 here}}
+END_PRAGMA
+}
+
+namespace my_ns {
+BEGIN_PRAGMA
+int g(); // expected-note{{'g' has been marked as being introduced in macOS 
1000 here}}
+END_PRAGMA
+namespace nested {
+BEGIN_PRAGMA
+int h(); // expected-note{{'h' has been marked as being introduced in macOS 
1000 here}}
+END_PRAGMA
+}
+}
+
+int a = f(); // expected-warning{{'f' is only available on macOS 1000 or 
newer}} expected-note{{annotate 'a'}}
+int b = my_ns::g(); // expected-warning{{'g' is only available on macOS 1000 
or newer}} expected-note{{annotate 'b'}}
+int c = my_ns::nested::h(); // expected-warning{{'h' is only available on 
macOS 1000 or newer}} expected-note{{annotate 'c'}}
+
+struct InStruct {
+  // FIXME: This asserts in Objective-C++!
+  // FIXME: This is a horrible diagnostic!
+#ifndef __OBJC__
+  BEGIN_PRAGMA // expected-error {{expected member name or ';' after 
declaration specifiers}}
+#endif
+};
Index: cfe/trunk/lib/Parse/Parser.cpp
===
--- cfe/trunk/lib/Parse/Parser.cpp
+++ cfe/trunk/lib/Parse/Parser.cpp
@@ -583,10 +583,6 @@
 ConsumeAnnotationToken();
 return false;
 
-  case tok::annot_pragma_attribute:
-HandlePragmaAttribute();
-return false;
-
   case tok::eof:
 // Late template parsing can begin.
 if (getLangOpts().DelayedTemplateParsing)
@@ -698,6 +694,9 @@
   case tok::annot_pragma_dump:
 HandlePragmaDump();
 return nullptr;
+  case tok::annot_pragma_attribute:
+HandlePragmaAttribute();
+return nullptr;
   case tok::semi:
 // Either a C++11 empty-declaration or attribute-declaration.
 SingleDecl =


Index: cfe/trunk/test/Parser/pragma-attribute-context.cpp
===
--- cfe/trunk/test/Parser/pragma-attribute-context.cpp
+++ cfe/trunk/test/Parser/pragma-attribute-context.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -verify -std=c++11 %s
+// RUN: %clang_cc1 -xobjective-c++ -verify -std=c++11 %s
+
+#define BEGIN_PRAGMA _Pragma("clang attribute push (__attribute__((availability(macos, introduced=1000))), apply_to=function)")
+#define END_PRAGMA _Pragma("clang attribute pop")
+
+extern "C" {
+BEGIN_PRAGMA
+int f(); // expected-note{{'f' has been marked as being introduced in macOS 1000 here}}
+END_PRAGMA
+}
+
+namespace my_ns {
+BEGIN_PRAGMA
+int g(); // expected-note{{'g' has been marked as being introduced in macOS 1000 here}}
+END_PRAGMA
+namespace nested {
+BEGIN_PRAGMA
+int h(); // expected-note{{'h' has been marked as being introduced in macOS 1000 here}}
+END_PRAGMA
+}
+}
+
+int a = f(); // expected-warning{{'f' is only available on macOS 1000 or newer}} expected-note{{annotate 'a'}}
+int b = my_ns::g(); // expected-warning{{'g' is only available on macOS 1000 or newer}} expected-note{{annotate 'b'}}
+int c = my_ns::nested::h(); // expected-warning{{'h' is only available on macOS 1000 or newer}} expected-note{{annotate 'c'}}
+
+struct InStruct {
+  // FIXME: This asserts in Objective-C++!
+  // FIXME: This is a horrible diagnostic!
+#ifndef __OBJC__
+  BEGIN_PRAGMA // expected-error {{expected member name or ';' after declaration specifiers}}
+#endif
+};
Index: cfe/trunk/lib/Parse/Parser.cpp
===
--- cfe/trunk/lib/Parse/Parser.cpp
+++ cfe/trunk/lib/Parse/Parser.cpp
@@ -583,10 +583,6 @@
 ConsumeAnnotationToken();
 return false;
 
-  case tok::annot_pragma_attribute:
-HandlePragmaAttribute();
-return false;
-
   case tok::eof:
 // Late template parsing can begin.
 if (getLangOpts().DelayedTemplateParsing)
@@ -698,6 +694,9 @@
   case tok::annot_pragma_dump:
 HandlePragmaDump();
 return nullptr;
+  case tok::annot_pragma_attribute:
+

r356075 - [Parse] Parse '#pragma clang attribute' as an external-declaration

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Wed Mar 13 11:30:59 2019
New Revision: 356075

URL: http://llvm.org/viewvc/llvm-project?rev=356075=rev
Log:
[Parse] Parse '#pragma clang attribute' as an external-declaration

Previously, we parsed it only in the top level, which excludes namespaces and
extern "C" blocks.

rdar://problem/48818890

Differential revision: https://reviews.llvm.org/D59282

Added:
cfe/trunk/test/Parser/pragma-attribute-context.cpp
Modified:
cfe/trunk/lib/Parse/Parser.cpp

Modified: cfe/trunk/lib/Parse/Parser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/Parser.cpp?rev=356075=356074=356075=diff
==
--- cfe/trunk/lib/Parse/Parser.cpp (original)
+++ cfe/trunk/lib/Parse/Parser.cpp Wed Mar 13 11:30:59 2019
@@ -583,10 +583,6 @@ bool Parser::ParseTopLevelDecl(DeclGroup
 ConsumeAnnotationToken();
 return false;
 
-  case tok::annot_pragma_attribute:
-HandlePragmaAttribute();
-return false;
-
   case tok::eof:
 // Late template parsing can begin.
 if (getLangOpts().DelayedTemplateParsing)
@@ -698,6 +694,9 @@ Parser::ParseExternalDeclaration(ParsedA
   case tok::annot_pragma_dump:
 HandlePragmaDump();
 return nullptr;
+  case tok::annot_pragma_attribute:
+HandlePragmaAttribute();
+return nullptr;
   case tok::semi:
 // Either a C++11 empty-declaration or attribute-declaration.
 SingleDecl =

Added: cfe/trunk/test/Parser/pragma-attribute-context.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/pragma-attribute-context.cpp?rev=356075=auto
==
--- cfe/trunk/test/Parser/pragma-attribute-context.cpp (added)
+++ cfe/trunk/test/Parser/pragma-attribute-context.cpp Wed Mar 13 11:30:59 2019
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -verify -std=c++11 %s
+// RUN: %clang_cc1 -xobjective-c++ -verify -std=c++11 %s
+
+#define BEGIN_PRAGMA _Pragma("clang attribute push 
(__attribute__((availability(macos, introduced=1000))), apply_to=function)")
+#define END_PRAGMA _Pragma("clang attribute pop")
+
+extern "C" {
+BEGIN_PRAGMA
+int f(); // expected-note{{'f' has been marked as being introduced in macOS 
1000 here}}
+END_PRAGMA
+}
+
+namespace my_ns {
+BEGIN_PRAGMA
+int g(); // expected-note{{'g' has been marked as being introduced in macOS 
1000 here}}
+END_PRAGMA
+namespace nested {
+BEGIN_PRAGMA
+int h(); // expected-note{{'h' has been marked as being introduced in macOS 
1000 here}}
+END_PRAGMA
+}
+}
+
+int a = f(); // expected-warning{{'f' is only available on macOS 1000 or 
newer}} expected-note{{annotate 'a'}}
+int b = my_ns::g(); // expected-warning{{'g' is only available on macOS 1000 
or newer}} expected-note{{annotate 'b'}}
+int c = my_ns::nested::h(); // expected-warning{{'h' is only available on 
macOS 1000 or newer}} expected-note{{annotate 'c'}}
+
+struct InStruct {
+  // FIXME: This asserts in Objective-C++!
+  // FIXME: This is a horrible diagnostic!
+#ifndef __OBJC__
+  BEGIN_PRAGMA // expected-error {{expected member name or ';' after 
declaration specifiers}}
+#endif
+};


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

[PATCH] D59307: Patch llvm bug 41033 concerning atomicity of statement expressions

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

In D59307#1427663 , @jfb wrote:

> Thinking some more, this is really a silly gotcha in code. We should probably 
> follow what we're about to do with wg21.link/P1152 
>  and ban chaining. Statement expressions 
> should therefore not be allowed to return `_Atomic`, `std::atomic`, nor 
> `volatile`.


If the atomic load which is the value of the StmtExpr drops the atomic 
attribute then this would trivially be true, and there would be no need to add 
a restriction. According to what Clark says, once the value is loaded it's just 
data.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59307



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

[PATCH] D59307: Patch llvm bug 41033 concerning atomicity of statement expressions

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

In D59307#1427665 , @jfb wrote:

> From an offline discussion, I'm told that @jwakely uses this in GCC headers 
> and expects some semantics from it? Jonathan, why?!?!


Yes this is how I stumbled into the issue.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59307



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