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

[Xiang Zhang via Phabricator via cfe-commits]

xiangzhangllvm added a comment.

Hi dears,  Could you please help me merge the patch. Thank you!


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

r353802 - [X86] Use the new unaligned vector typedefs for the loadu/storeu intrinsics pointer arguments.

[Craig Topper via cfe-commits]

Author: ctopper
Date: Mon Feb 11 23:44:40 2019
New Revision: 353802

URL: http://llvm.org/viewvc/llvm-project?rev=353802=rev
Log:
[X86] Use the new unaligned vector typedefs for the loadu/storeu intrinsics 
pointer arguments.

This matches what gcc does and what was suggested by rnk in PR20670.

Modified:
cfe/trunk/lib/Headers/avxintrin.h
cfe/trunk/lib/Headers/emmintrin.h

Modified: cfe/trunk/lib/Headers/avxintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/avxintrin.h?rev=353802=353801=353802=diff
==
--- cfe/trunk/lib/Headers/avxintrin.h (original)
+++ cfe/trunk/lib/Headers/avxintrin.h Mon Feb 11 23:44:40 2019
@@ -3170,7 +3170,7 @@ _mm256_load_si256(__m256i const *__p)
 ///A pointer to a 256-bit integer vector containing integer values.
 /// \returns A 256-bit integer vector containing the moved values.
 static __inline __m256i __DEFAULT_FN_ATTRS
-_mm256_loadu_si256(__m256i const *__p)
+_mm256_loadu_si256(__m256i_u const *__p)
 {
   struct __loadu_si256 {
 __m256i_u __v;
@@ -3305,7 +3305,7 @@ _mm256_store_si256(__m256i *__p, __m256i
 /// \param __a
 ///A 256-bit integer vector containing the values to be moved.
 static __inline void __DEFAULT_FN_ATTRS
-_mm256_storeu_si256(__m256i *__p, __m256i __a)
+_mm256_storeu_si256(__m256i_u *__p, __m256i __a)
 {
   struct __storeu_si256 {
 __m256i_u __v;
@@ -4838,7 +4838,7 @@ _mm256_loadu2_m128d(double const *__addr
 ///address of the memory location does not have to be aligned.
 /// \returns A 256-bit integer vector containing the concatenated result.
 static __inline __m256i __DEFAULT_FN_ATTRS
-_mm256_loadu2_m128i(__m128i const *__addr_hi, __m128i const *__addr_lo)
+_mm256_loadu2_m128i(__m128i_u const *__addr_hi, __m128i_u const *__addr_lo)
 {
   __m256i __v256 = _mm256_castsi128_si256(_mm_loadu_si128(__addr_lo));
   return _mm256_insertf128_si256(__v256, _mm_loadu_si128(__addr_hi), 1);
@@ -4922,7 +4922,7 @@ _mm256_storeu2_m128d(double *__addr_hi,
 /// \param __a
 ///A 256-bit integer vector.
 static __inline void __DEFAULT_FN_ATTRS
-_mm256_storeu2_m128i(__m128i *__addr_hi, __m128i *__addr_lo, __m256i __a)
+_mm256_storeu2_m128i(__m128i_u *__addr_hi, __m128i_u *__addr_lo, __m256i __a)
 {
   __m128i __v128;
 

Modified: cfe/trunk/lib/Headers/emmintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/emmintrin.h?rev=353802=353801=353802=diff
==
--- cfe/trunk/lib/Headers/emmintrin.h (original)
+++ cfe/trunk/lib/Headers/emmintrin.h Mon Feb 11 23:44:40 2019
@@ -3567,7 +3567,7 @@ _mm_load_si128(__m128i const *__p)
 ///A pointer to a memory location containing integer values.
 /// \returns A 128-bit integer vector containing the moved values.
 static __inline__ __m128i __DEFAULT_FN_ATTRS
-_mm_loadu_si128(__m128i const *__p)
+_mm_loadu_si128(__m128i_u const *__p)
 {
   struct __loadu_si128 {
 __m128i_u __v;
@@ -3588,7 +3588,7 @@ _mm_loadu_si128(__m128i const *__p)
 /// \returns A 128-bit vector of [2 x i64]. The lower order bits contain the
 ///moved value. The higher order bits are cleared.
 static __inline__ __m128i __DEFAULT_FN_ATTRS
-_mm_loadl_epi64(__m128i const *__p)
+_mm_loadl_epi64(__m128i_u const *__p)
 {
   struct __mm_loadl_epi64_struct {
 long long __u;
@@ -4030,7 +4030,7 @@ _mm_store_si128(__m128i *__p, __m128i __
 /// \param __b
 ///A 128-bit integer vector containing the values to be moved.
 static __inline__ void __DEFAULT_FN_ATTRS
-_mm_storeu_si128(__m128i *__p, __m128i __b)
+_mm_storeu_si128(__m128i_u *__p, __m128i __b)
 {
   struct __storeu_si128 {
 __m128i_u __v;
@@ -4142,7 +4142,7 @@ _mm_maskmoveu_si128(__m128i __d, __m128i
 ///A 128-bit integer vector of [2 x i64]. The lower 64 bits contain the
 ///value to be stored.
 static __inline__ void __DEFAULT_FN_ATTRS
-_mm_storel_epi64(__m128i *__p, __m128i __a)
+_mm_storel_epi64(__m128i_u *__p, __m128i __a)
 {
   struct __mm_storel_epi64_struct {
 long long __u;


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

Re: r353729 - Attempt to pacify bots more after r353718 and r353725

[Mikael Holmén via cfe-commits]

Same thing for me with our downstream build bots.

When we compile clang with gcc 7.4 and then run the testcase I get this 
output

define dso_local i64 @test__shiftleft128(i64 %l, i64 %h, i8 %d) 
local_unnamed_addr #0 {
entry:
   %0 = zext i64 %l to i128
   %1 = zext i64 %h to i128
   %2 = shl nuw i128 %1, 64
   %3 = or i128 %2, %0
   %4 = and i8 %d, 63
   %5 = zext i8 %4 to i128
   %6 = shl i128 %3, %5
   %7 = lshr i128 %6, 64
   %8 = trunc i128 %7 to i64
   ret i64 %8
}

and when I compile clang with clang 3.6 and run the test I get this:

define dso_local i64 @test__shiftleft128(i64 %l, i64 %h, i8 %d) 
local_unnamed_addr #0 {
entry:
   %0 = zext i64 %h to i128
   %1 = shl nuw i128 %0, 64
   %2 = zext i64 %l to i128
   %3 = or i128 %1, %2
   %4 = and i8 %d, 63
   %5 = zext i8 %4 to i128
   %6 = shl i128 %3, %5
   %7 = lshr i128 %6, 64
   %8 = trunc i128 %7 to i64
   ret i64 %8
}

/Mikael

On 2/12/19 2:03 AM, Nico Weber via cfe-commits wrote:
> Thank you for the .ll files!
> 
> the -4.ll file you sent me contains:
> 
> define dso_local i64 @"?test__shiftleft128@@YA_K_K0E@Z"(i64 %l, i64 %h, 
> i8 %d) local_unnamed_addr #0 {
> entry:
>    %0 = zext i64 %h to i128
>    %1 = shl nuw i128 %0, 64
>    %2 = zext i64 %l to i128
>    %3 = or i128 %1, %2
>    %4 = and i8 %d, 63
>    %5 = zext i8 %4 to i128
>    %6 = shl i128 %3, %5
>    %7 = lshr i128 %6, 64
>    %8 = trunc i128 %7 to i64
>    ret i64 %8
> }
> 
> On my local system, I get
> 
> ; Function Attrs: minsize norecurse nounwind optsize readnone
> define dso_local i64 @test__shiftleft128(i64 %l, i64 %h, i8 %d) 
> local_unnamed_addr #0 {
> entry:
>    %0 = zext i64 %l to i128
>    %1 = zext i64 %h to i128
>    %2 = shl nuw i128 %1, 64
>    %3 = or i128 %2, %0
>    %4 = and i8 %d, 63
>    %5 = zext i8 %4 to i128
>    %6 = shl i128 %3, %5
>    %7 = lshr i128 %6, 64
>    %8 = trunc i128 %7 to i64
>    ret i64 %8
> }
> 
> That's identical except for the order of the instructions (which in turn 
> changes some % numbers).
> 
> That's surprising to me; I thought LLVM IR is deterministic (and if it 
> wasn't, many other tests wouldn't work either).
> 
> On Mon, Feb 11, 2019 at 4:20 PM Galina Kistanova  > wrote:
> 
> Hello Nico,
> 
> This builders fail on your test as well -
> 
> http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/15736,
> http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/4242.
> 
> Please find attached the 2 temp files you can use to reliably run
> against your FileCheck patterns.
> Hope this would help debugging.
> 
> Please also notice the warnings each of the RUN command produces.
> The warnings should be quite easy to reproduce and address.
> 
> In the mean time, could you revert the change unless you expect the
> fix coming really soon, please?
> It is not a good idea to keep the bots red for long.
> 
> Here is the log:
> --
> 
> 
> C:\>c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\bin\clang.exe
> -cc1 -internal-isystem
> 
> c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\lib\clang\9.0.0\include
> -nostdsysteminc -ffreestanding -fms-extensions -fms-compatibility
> -fms-compatibility-version=17.00 -triple i686--windows -Oz
> -emit-llvm
> 
> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\CodeGen\ms-x86-intrinsics.c
> -o - > \tmp-1\ms-x86-intrinsics-1.ll
> 
> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\CodeGen\ms-x86-intrinsics.c:10:10:
> warning: implicitly declaring library function '__readfsbyte' with
> type 'unsigned char (unsigned long)'
>    return __readfsbyte(++Offset);
>           ^
> 
> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\CodeGen\ms-x86-intrinsics.c:10:10:
> note: include the header  or explicitly provide a
> declaration for '__readfsbyte'
> 
> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\CodeGen\ms-x86-intrinsics.c:19:10:
> warning: implicitly declaring library function '__readfsword' with
> type 'unsigned short (unsigned long)'
>    return __readfsword(++Offset);
>           ^
> 
> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\CodeGen\ms-x86-intrinsics.c:19:10:
> note: include the header  or explicitly provide a
> declaration for '__readfsword'
> 
> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\CodeGen\ms-x86-intrinsics.c:28:10:
> warning: implicitly declaring library function '__readfsdword' with
> type 'unsigned long (unsigned long)'
>    return __readfsdword(++Offset);
> 

[PATCH] D57736: [WebAssembly] Bulk memory intrinsics and builtins

[Thomas Lively via Phabricator via cfe-commits]

tlively added inline comments.



Comment at: clang/include/clang/Basic/BuiltinsWebAssembly.def:29
+// Bulk memory builtins
+TARGET_BUILTIN(__builtin_wasm_memory_init, "vIiv*ii", "", "bulk-memory")
+TARGET_BUILTIN(__builtin_wasm_data_drop, "vIi", "", "bulk-memory")

aheejin wrote:
> - When do we use `Ii` instead of `i`?
> - Shouldn't we add the memory index field as well, even if that means a user 
> always has to set it to 0? Are we gonna add a new builtin once multiple 
> memories are available?
> - Shouldn't the segment index, segment offset, and the size operands be `Ui` 
> because they cannot be negative?
We use `Ii` instead of `i` when the argument needs to be a compile time 
constant because it is encoded as a literal in the corresponding instruction.

I was imagining that we could add a new builtin once multiple memories are 
available, but perhaps it would be better to add an argument to the builtin now 
and error out if it is not zero. I will do that.

Yes, `Ui` seems reasonable.





Comment at: clang/include/clang/Basic/BuiltinsWebAssembly.def:30
+TARGET_BUILTIN(__builtin_wasm_memory_init, "vIiv*ii", "", "bulk-memory")
+TARGET_BUILTIN(__builtin_wasm_data_drop, "vIi", "", "bulk-memory")
+

aheejin wrote:
> The same thing..
> - Shouldn't the segment index be `Ui`?
> - Shouldn't we add the memory index field as well?
> 
 - Done

 - This intrinsic doesn't have a memory index, since it doesn't write anything 
into memory.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:13477
+if (!SegArg->isIntegerConstantExpr(SegConst, getContext()))
+  llvm_unreachable("Constant arg isn't actually constant?");
+llvm::Type *SegType = ConvertType(SegArg->getType());

aheejin wrote:
> Not sure if we can use `llvm_unreachable` here, because we can certainly 
> reach here if a user uses this builtin with a non-const variable. In this 
> file people often just used `assert` for user errors, which is not ideal 
> either.
> 
> I haven't used it myself, but looking at the code, the recommended way to 
> signal an error looks like to use [[ 
> https://github.com/llvm/llvm-project/blob/db7fbcb038f095622a3e6847ecd6ff80bdc2483a/clang/lib/CodeGen/CodeGenFunction.h#L2092-L2094
>  | `CodeGenFunction::ErrorUnsupported` ]] function, as in [[ 
> https://github.com/llvm/llvm-project/blob/0e04ebdcda44ef90e25abd0a2e65cc755ae8bc37/clang/lib/CodeGen/CGBuiltin.cpp#L2458-L2460
>  | here ]]. We used `llvm_unreachable` for SIMD builtins too, but maybe we 
> can fix it later.
`llvm_unreachable` is appropriate here because non-constant expressions will 
have been caught earlier by the type checking.

A follow-up PR updating SIMD intrinsics to use `Ui` and appropriate error 
handling sounds good.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:13488
+Function *Callee = CGM.getIntrinsic(Intrinsic::wasm_memory_init,
+{SegType, IdxType, DstType});
+return Builder.CreateCall(Callee, Args);

aheejin wrote:
> Do we need to pass types here to make intrinsic names overloaded like 
> `llvm.wasm.memory.init.i32.i32.i8`, unless this intrinsic also support 
> operands of other types, which it doesn't? The same for `data.drop` builtin.
Fixed. I had been copying `llvm.memcpy`, but I agree it is better to not be 
polymorphic.



Comment at: llvm/include/llvm/IR/IntrinsicsWebAssembly.td:121
+ llvm_i32_ty, llvm_i32_ty],
+[IntrWriteMem, IntrArgMemOnly, WriteOnly<1>],
+"", [SDNPMemOperand]>;

aheejin wrote:
> - Isn't the pointer argument number not 1 but 2?
> - I guess this should have `IntrHasSideEffects` as well, as in `data.drop`?
> - I don't know much how they are handled differently in compilation, but 
> because we can access some other memory, which holds the segment part, during 
> execution, how about `IntrInaccessibleMemOrArgMemOnly` instead of 
> `IntrArgMemOnly`?
- Yes, good catch.

- Ok, I'm not sure it's necessary but it can't hurt.

- I'm not sure this is necessary either, but again it can't hurt. Better safe 
than sorry.



Comment at: llvm/include/llvm/IR/IntrinsicsWebAssembly.td:122
+[IntrWriteMem, IntrArgMemOnly, WriteOnly<1>],
+"", [SDNPMemOperand]>;
+def int_wasm_data_drop :

aheejin wrote:
> I told you we needed this, but on second thought, because we don't really use 
> `MachineMemOperand`, maybe we don't need it..? :$ The [[ 
> https://github.com/llvm/llvm-project/blob/dc2c93017f8bf2a2c10b8e024f8f4a6409db/llvm/include/llvm/IR/Intrinsics.td#L483-L496
>  | standard memcpy/memmove/memset intrinsics ]] don't have it either. So if 
> the code runs ok without these I think we can delete it. The same for 
> `data.drop` intrinsic. Sorry for the incorrect info and confusion.
No problem!


Repository:
  rG LLVM Github Monorepo

CHANGES 

[PATCH] D57736: [WebAssembly] Bulk memory intrinsics and builtins

[Thomas Lively via Phabricator via cfe-commits]

tlively updated this revision to Diff 186393.
tlively marked 12 inline comments as done.
tlively added a comment.

- Address comments
  - Use `Ui` in builtin signatures
  - Remove unnecessary intrinsic polymorphism
  - Tweak intrinsic properties


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57736

Files:
  clang/include/clang/Basic/BuiltinsWebAssembly.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-wasm.c
  llvm/include/llvm/IR/IntrinsicsWebAssembly.td
  llvm/lib/Target/WebAssembly/WebAssemblyInstrBulkMemory.td
  llvm/lib/Target/WebAssembly/WebAssemblySelectionDAGInfo.cpp
  llvm/test/CodeGen/WebAssembly/bulk-memory-intrinsics.ll
  llvm/test/CodeGen/WebAssembly/bulk-memory.ll

Index: llvm/test/CodeGen/WebAssembly/bulk-memory.ll
===
--- llvm/test/CodeGen/WebAssembly/bulk-memory.ll
+++ llvm/test/CodeGen/WebAssembly/bulk-memory.ll
@@ -6,14 +6,17 @@
 target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
 target triple = "wasm32-unknown-unknown"
 
+declare void @llvm.memcpy.p0i8.p0i8.i32(i8*, i8*, i32, i1)
+declare void @llvm.memcpy.p0i32.p0i32.i32(i32*, i32*, i32, i1)
+
+declare void @llvm.memmove.p0i8.p0i8.i32(i8*, i8*, i32, i1)
+declare void @llvm.memmove.p0i32.p0i32.i32(i32*, i32*, i32, i1)
+
 ; CHECK-LABEL: memcpy_i8:
 ; NO-BULK-MEM-NOT: memory.copy
 ; BULK-MEM-NEXT: .functype memcpy_i8 (i32, i32, i32) -> ()
-; BULK-MEM-NEXT: memory.copy $0, $1, $2
+; BULK-MEM-NEXT: memory.copy 0, 0, $0, $1, $2
 ; BULK-MEM-NEXT: return
-declare void @llvm.memcpy.p0i8.p0i8.i32(
-  i8* %dest, i8* %src, i32 %len, i1 %volatile
-)
 define void @memcpy_i8(i8* %dest, i8* %src, i32 %len) {
   call void @llvm.memcpy.p0i8.p0i8.i32(i8* %dest, i8* %src, i32 %len, i1 0)
   ret void
@@ -22,11 +25,8 @@
 ; CHECK-LABEL: memmove_i8:
 ; NO-BULK-MEM-NOT: memory.copy
 ; BULK-MEM-NEXT: .functype memmove_i8 (i32, i32, i32) -> ()
-; BULK-MEM-NEXT: memory.copy $0, $1, $2
+; BULK-MEM-NEXT: memory.copy 0, 0, $0, $1, $2
 ; BULK-MEM-NEXT: return
-declare void @llvm.memmove.p0i8.p0i8.i32(
-  i8* %dest, i8* %src, i32 %len, i1 %volatile
-)
 define void @memmove_i8(i8* %dest, i8* %src, i32 %len) {
   call void @llvm.memmove.p0i8.p0i8.i32(i8* %dest, i8* %src, i32 %len, i1 0)
   ret void
@@ -35,11 +35,8 @@
 ; CHECK-LABEL: memcpy_i32:
 ; NO-BULK-MEM-NOT: memory.copy
 ; BULK-MEM-NEXT: .functype memcpy_i32 (i32, i32, i32) -> ()
-; BULK-MEM-NEXT: memory.copy $0, $1, $2
+; BULK-MEM-NEXT: memory.copy 0, 0, $0, $1, $2
 ; BULK-MEM-NEXT: return
-declare void @llvm.memcpy.p0i32.p0i32.i32(
-  i32* %dest, i32* %src, i32 %len, i1 %volatile
-)
 define void @memcpy_i32(i32* %dest, i32* %src, i32 %len) {
   call void @llvm.memcpy.p0i32.p0i32.i32(i32* %dest, i32* %src, i32 %len, i1 0)
   ret void
@@ -48,11 +45,8 @@
 ; CHECK-LABEL: memmove_i32:
 ; NO-BULK-MEM-NOT: memory.copy
 ; BULK-MEM-NEXT: .functype memmove_i32 (i32, i32, i32) -> ()
-; BULK-MEM-NEXT: memory.copy $0, $1, $2
+; BULK-MEM-NEXT: memory.copy 0, 0, $0, $1, $2
 ; BULK-MEM-NEXT: return
-declare void @llvm.memmove.p0i32.p0i32.i32(
-  i32* %dest, i32* %src, i32 %len, i1 %volatile
-)
 define void @memmove_i32(i32* %dest, i32* %src, i32 %len) {
   call void @llvm.memmove.p0i32.p0i32.i32(i32* %dest, i32* %src, i32 %len, i1 0)
   ret void
@@ -82,7 +76,7 @@
 ; NO-BULK-MEM-NOT: memory.copy
 ; BULK-MEM-NEXT: .functype memcpy_1024 (i32, i32) -> ()
 ; BULK-MEM-NEXT: i32.const $push[[L0:[0-9]+]]=, 1024
-; BULK-MEM-NEXT: memory.copy $0, $1, $pop[[L0]]
+; BULK-MEM-NEXT: memory.copy 0, 0, $0, $1, $pop[[L0]]
 ; BULK-MEM-NEXT: return
 define void @memcpy_1024(i8* %dest, i8* %src) {
   call void @llvm.memcpy.p0i8.p0i8.i32(i8* %dest, i8* %src, i32 1024, i1 0)
@@ -93,7 +87,7 @@
 ; NO-BULK-MEM-NOT: memory.copy
 ; BULK-MEM-NEXT: .functype memmove_1024 (i32, i32) -> ()
 ; BULK-MEM-NEXT: i32.const $push[[L0:[0-9]+]]=, 1024
-; BULK-MEM-NEXT: memory.copy $0, $1, $pop[[L0]]
+; BULK-MEM-NEXT: memory.copy 0, 0, $0, $1, $pop[[L0]]
 ; BULK-MEM-NEXT: return
 define void @memmove_1024(i8* %dest, i8* %src) {
   call void @llvm.memmove.p0i8.p0i8.i32(i8* %dest, i8* %src, i32 1024, i1 0)
Index: llvm/test/CodeGen/WebAssembly/bulk-memory-intrinsics.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/bulk-memory-intrinsics.ll
@@ -0,0 +1,28 @@
+; RUN: llc < %s -asm-verbose=false -verify-machineinstrs -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -wasm-keep-registers -mattr=+bulk-memory | FileCheck %s
+
+; Test that bulk memory intrinsics lower correctly
+
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-unknown"
+
+; CHECK-LABEL: memory_init:
+; CHECK-NEXT: .functype  memory_init (i32, i32, i32) -> ()
+; CHECK-NEXT: memory.init 3, 0, $0, $1, $2
+; CHECK-NEXT: return
+declare void @llvm.wasm.memory.init(i32, i32, i8*, i32, i32)
+define void @memory_init(i8* 

[PATCH] D58095: [clang-tidy] Make google-objc-function-naming ignore implicit functions 

[Stephane Moore via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/test/clang-tidy/google-objc-function-naming.m:10
+// function would be declared in a system header.
+int printf(const char *, ...);  // NOLINT(google-objc-function-naming)
+

Thus far I have been unsuccessful in using line markers to simulate this 
declaration being in a system header but I did discover precedence for using 
NOLINT to suppress diagnostics in some of the clang-tidy tests:
https://github.com/llvm/llvm-project/blob/master/clang-tools-extra/test/clang-tidy/google-runtime-int-std.cpp#L11
https://github.com/llvm/llvm-project/blob/master/clang-tools-extra/test/clang-tidy/google-runtime-int.cpp#L6

I think it should be reasonable to suppress the diagnostic here with a comment 
explaining why. Let me know if you don't think that's an appropriate solution 
and I can continue investigating for a potential solution using line markers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58095



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

[PATCH] D58095: [clang-tidy] Make google-objc-function-naming ignore implicit functions 

[Stephane Moore via Phabricator via cfe-commits]

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

Implicit functions are outside the control of source authors and should
be exempt from style restrictions.

Tested via running clang tools tests.

This is an amended followup to https://reviews.llvm.org/D57207


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58095

Files:
  clang-tools-extra/clang-tidy/google/FunctionNamingCheck.cpp
  clang-tools-extra/test/clang-tidy/google-objc-function-naming.m


Index: clang-tools-extra/test/clang-tidy/google-objc-function-naming.m
===
--- clang-tools-extra/test/clang-tidy/google-objc-function-naming.m
+++ clang-tools-extra/test/clang-tidy/google-objc-function-naming.m
@@ -1,5 +1,20 @@
 // RUN: %check_clang_tidy %s google-objc-function-naming %t
 
+// Declare a builtin function so that we can invoke the function below to
+// generate an implicit function declaration (we intentionally do not import
+//  as we cannot guarantee its availability).
+//
+// google-objc-function-naming is suppressed for this declaration as it must
+// match the builtin function declaration. Under normal conditions, this
+// function would be declared in a system header.
+int printf(const char *, ...);  // NOLINT(google-objc-function-naming)
+
+static void TestImplicitFunctionDeclaration(int a) {
+  // Call a builtin function so that the compiler generates an implicit
+  // function declaration.
+  printf("%d", a);
+}
+
 typedef _Bool bool;
 
 static bool ispositive(int a) { return a > 0; }
Index: clang-tools-extra/clang-tidy/google/FunctionNamingCheck.cpp
===
--- clang-tools-extra/clang-tidy/google/FunctionNamingCheck.cpp
+++ clang-tools-extra/clang-tidy/google/FunctionNamingCheck.cpp
@@ -93,12 +93,16 @@
   if (!getLangOpts().ObjC)
 return;
 
-  // Match function declarations that are not in system headers and are not
-  // main.
+  // Enforce Objective-C function naming conventions on all functions except:
+  // • Functions defined in system headers.
+  // • C++ member functions.
+  // • Namespaced functions.
+  // • Implicitly defined functions.
+  // • The main function.
   Finder->addMatcher(
   functionDecl(
   unless(anyOf(isExpansionInSystemHeader(), cxxMethodDecl(),
-   hasAncestor(namespaceDecl()), isMain(),
+   hasAncestor(namespaceDecl()), isMain(), isImplicit(),
matchesName(validFunctionNameRegex(true)),
allOf(isStaticStorageClass(),
  matchesName(validFunctionNameRegex(false))


Index: clang-tools-extra/test/clang-tidy/google-objc-function-naming.m
===
--- clang-tools-extra/test/clang-tidy/google-objc-function-naming.m
+++ clang-tools-extra/test/clang-tidy/google-objc-function-naming.m
@@ -1,5 +1,20 @@
 // RUN: %check_clang_tidy %s google-objc-function-naming %t
 
+// Declare a builtin function so that we can invoke the function below to
+// generate an implicit function declaration (we intentionally do not import
+//  as we cannot guarantee its availability).
+//
+// google-objc-function-naming is suppressed for this declaration as it must
+// match the builtin function declaration. Under normal conditions, this
+// function would be declared in a system header.
+int printf(const char *, ...);  // NOLINT(google-objc-function-naming)
+
+static void TestImplicitFunctionDeclaration(int a) {
+  // Call a builtin function so that the compiler generates an implicit
+  // function declaration.
+  printf("%d", a);
+}
+
 typedef _Bool bool;
 
 static bool ispositive(int a) { return a > 0; }
Index: clang-tools-extra/clang-tidy/google/FunctionNamingCheck.cpp
===
--- clang-tools-extra/clang-tidy/google/FunctionNamingCheck.cpp
+++ clang-tools-extra/clang-tidy/google/FunctionNamingCheck.cpp
@@ -93,12 +93,16 @@
   if (!getLangOpts().ObjC)
 return;
 
-  // Match function declarations that are not in system headers and are not
-  // main.
+  // Enforce Objective-C function naming conventions on all functions except:
+  // • Functions defined in system headers.
+  // • C++ member functions.
+  // • Namespaced functions.
+  // • Implicitly defined functions.
+  // • The main function.
   Finder->addMatcher(
   functionDecl(
   unless(anyOf(isExpansionInSystemHeader(), cxxMethodDecl(),
-   hasAncestor(namespaceDecl()), isMain(),
+   hasAncestor(namespaceDecl()), isMain(), isImplicit(),
matchesName(validFunctionNameRegex(true)),
allOf(isStaticStorageClass(),
  matchesName(validFunctionNameRegex(false))

[PATCH] D58089: Add missing library dependencies in CMakeLists.txt

[Petr Hosek via Phabricator via cfe-commits]

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

LGTM


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58089



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

[PATCH] D58094: Fix -Wnonportable-include-path suppression for header maps with absolute paths.

[Volodymyr Sapsai via Phabricator via cfe-commits]

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



Comment at: clang/lib/Lex/HeaderSearch.cpp:388
   } else {
 Result = HS.getFileMgr().getFile(Dest);
   }

I have considered changing this to `.getFile(Dest, /*OpenFile=*/true)` so that 
the bug can be triggered without a double include. But decided not to do so 
because it seems weird to open a file (and make an extra syscall) to make it 
easier to ignore the filename case mismatch later.


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

https://reviews.llvm.org/D58094



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

[PATCH] D57984: PR40642: Fix determination of whether the final statement of a statementexpression is a discarded-value expression.

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

rsmith added a comment.

In D57984#1394067 , @ABataev wrote:

> In D57984#1394050 , @rsmith wrote:
>
> > @ABataev Is it intentional that we do not propagate `Allowed` through 
> > labels? For example:
> >
> >   void f() {
> > #pragma omp barrier // ok
> >  
> >   label:
> > #pragma omp barrier // error, "cannot be an immediate substatement"
> >  
> >   label:
> > ;
> > #pragma omp barrier // ok
> >   }
> >
> >
> > ?
>
>
> No, it is a bug.


Great, then I'll unify this new flag with the `Allowed` mechanism and fix the 
bug as part of this change. Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D57984



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

[PATCH] D58094: Fix -Wnonportable-include-path suppression for header maps with absolute paths.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai created this revision.
vsapsai added reviewers: dexonsmith, bruno.
Herald added a subscriber: jkorous.

In `DirectoryLookup::LookupFile` parameter `HasBeenMapped` doesn't cover
the case when clang finds a file through a header map but doesn't remap
the lookup filename because the target path is an absolute path. As a
result, -Wnonportable-include-path suppression for header maps
introduced in r301592 wasn't triggered.

Change parameter `HasBeenMapped` to `IsInHeaderMap` and use parameter
`MappedName` to track the filename remapping. This way we can handle
both relative and absolute paths in header maps, and account for their
specific properties, like filename remapping being a property preserved
across lookups in multiple directories.

rdar://problem/39516483


https://reviews.llvm.org/D58094

Files:
  clang/include/clang/Lex/DirectoryLookup.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap.json
  clang/test/Preprocessor/Inputs/nonportable-hmaps/headers/foo/Bar.h
  clang/test/Preprocessor/nonportable-include-with-hmap.c

Index: clang/test/Preprocessor/nonportable-include-with-hmap.c
===
--- clang/test/Preprocessor/nonportable-include-with-hmap.c
+++ clang/test/Preprocessor/nonportable-include-with-hmap.c
@@ -1,5 +1,9 @@
+// REQUIRES: shell
+
 // RUN: rm -f %t.hmap
-// RUN: %hmaptool write %S/Inputs/nonportable-hmaps/foo.hmap.json %t.hmap
+// RUN: sed -e "s:INPUTS_DIR:%S/Inputs:g" \
+// RUN:   %S/Inputs/nonportable-hmaps/foo.hmap.json > %t.hmap.json
+// RUN: %hmaptool write %t.hmap.json %t.hmap
 // RUN: %clang_cc1 -Eonly\
 // RUN:   -I%t.hmap \
 // RUN:   -I%S/Inputs/nonportable-hmaps  \
@@ -16,3 +20,11 @@
 //
 // There is nothing nonportable; -Wnonportable-include-path should not fire.
 #include "Foo/Foo.h" // expected-no-diagnostics
+
+// Verify files with absolute paths in the header map are handled too.
+// "Bar.h" is included twice to make sure that when we see potentially
+// nonportable path, the file has been already discovered through a relative
+// path which triggers the file to be opened and `FileEntry::RealPathName`
+// to be set.
+#include "Bar.h"
+#include "Foo/Bar.h" // expected-no-diagnostics
Index: clang/test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap.json
===
--- clang/test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap.json
+++ clang/test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap.json
@@ -1,6 +1,8 @@
 {
   "mappings" :
 {
- "Foo/Foo.h" : "headers/foo/Foo.h"
+ "Foo/Foo.h" : "headers/foo/Foo.h",
+ "Bar.h" : "headers/foo/Bar.h",
+ "Foo/Bar.h" : "INPUTS_DIR/nonportable-hmaps/headers/foo/Bar.h"
 }
 }
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -335,10 +335,11 @@
 ModuleMap::KnownHeader *SuggestedModule,
 bool ,
 bool ,
-bool ,
+bool ,
 SmallVectorImpl ) const {
   InUserSpecifiedSystemFramework = false;
-  HasBeenMapped = false;
+  IsInHeaderMap = false;
+  MappedName.clear();
 
   SmallString<1024> TmpDir;
   if (isNormalDir()) {
@@ -372,16 +373,16 @@
   if (Dest.empty())
 return nullptr;
 
+  IsInHeaderMap = true;
+
   const FileEntry *Result;
 
   // Check if the headermap maps the filename to a framework include
   // ("Foo.h" -> "Foo/Foo.h"), in which case continue header lookup using the
   // framework include.
   if (llvm::sys::path::is_relative(Dest)) {
-MappedName.clear();
 MappedName.append(Dest.begin(), Dest.end());
 Filename = StringRef(MappedName.begin(), MappedName.size());
-HasBeenMapped = true;
 Result = HM->LookupFile(Filename, HS.getFileMgr());
   } else {
 Result = HS.getFileMgr().getFile(Dest);
@@ -852,26 +853,30 @@
   }
 
   SmallString<64> MappedName;
+  bool HasBeenMapped = false;
 
   // Check each directory in sequence to see if it contains this file.
   for (; i != SearchDirs.size(); ++i) {
 bool InUserSpecifiedSystemFramework = false;
-bool HasBeenMapped = false;
+bool CurrentInHeaderMap = false;
 bool IsFrameworkFoundInDir = false;
 const FileEntry *FE = SearchDirs[i].LookupFile(
 Filename, *this, IncludeLoc, SearchPath, RelativePath, RequestingModule,
 SuggestedModule, InUserSpecifiedSystemFramework, IsFrameworkFoundInDir,
-HasBeenMapped, MappedName);
-if (HasBeenMapped) {
+CurrentInHeaderMap, MappedName);
+if (!MappedName.empty()) {
+  assert(CurrentInHeaderMap && "MappedName should come from a header map");
   CacheLookup.MappedName =
-  copyString(Filename, LookupFileCache.getAllocator());
-  if (IsMapped)
-*IsMapped = true;
+  copyString(MappedName, LookupFileCache.getAllocator());
+  HasBeenMapped = true;
 }
 if 

[PATCH] D10833: Retrieve BinaryOperator::getOpcode and BinaryOperator::getOpcodeStr via libclang and its python interface

[dyhe83 via Phabricator via cfe-commits]

dyhe83 added a comment.

In D10833#970906 , @milianw wrote:

> still looks good to me. can someone else please review and commit this?


ping


Repository:
  rC Clang

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

https://reviews.llvm.org/D10833



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

[PATCH] D58072: Make ModuleDependencyCollector's method virtual (NFC)

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere added a comment.

In D58072#1393817 , @bruno wrote:

> Not really. Would making only the `attachTo*` methods virtual enough though?


You mean making them **not** virtual? They're the only ones I don't override.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58072



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

[PATCH] D57497: [RISCV] Passing small data limitation value to RISCV backend

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> Did you mean declare as a target feature in RISCV.td or I misunderstanding 
> something?

That's sort of the right idea, but I don't think it works in this context 
because we aren't trying to change the generated code for a function; we 
actually need to stick the global into a specific section.  Maybe worth sending 
an email to llvmdev to discuss the right way to represent this in IR?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57497



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

[PATCH] D57497: [RISCV] Passing small data limitation value to RISCV backend

[Shiva Chen via Phabricator via cfe-commits]

shiva0217 added a comment.

In D57497#1388350 , @apazos wrote:

> If this is a target flag in GCC, shouldn't we make it a LLVM Target feature 
> and pass it as -mattr, just like done for mrelax?


Hi Ana,
It seems that most of the -mattr features only obtain on/off without assigning 
a value.
For -mfpu=vfp4,  each fpu configuration will be declared as a feature (e.g. 
FeatureVFP4) and the configurations are limited,
but the threshold value could be any number. 
Did you mean declare as a target feature in `RISCV.td` or I misunderstanding 
something?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57497



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

r353791 - Fix VS2015 build problem introduced by r353370.

[Douglas Yung via cfe-commits]

Author: dyung
Date: Mon Feb 11 18:17:51 2019
New Revision: 353791

URL: http://llvm.org/viewvc/llvm-project?rev=353791=rev
Log:
Fix VS2015 build problem introduced by r353370.

Modified:

cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h?rev=353791=353790=353791=diff
==
--- 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
 (original)
+++ 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
 Mon Feb 11 18:17:51 2019
@@ -19,8 +19,8 @@
 
 typedef llvm::ImmutableSet<
 std::pair>
-ConstraintSMTTy;
-REGISTER_TRAIT_WITH_PROGRAMSTATE(ConstraintSMT, ConstraintSMTTy)
+ConstraintSMTType;
+REGISTER_TRAIT_WITH_PROGRAMSTATE(ConstraintSMT, ConstraintSMTType)
 
 namespace clang {
 namespace ento {


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

[PATCH] D58091: Customize warnings for missing built-in type

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert created this revision.
jdoerfert added reviewers: jyknight, lebedev.ri, aaron.ballman, bcain.
Herald added subscribers: jfb, bollu, krytarowski, emaste.
Herald added a project: clang.

If we detect a built-in declaration for which we cannot derive a type
matching the pattern in the Builtins.def file, we currently emit a
warning that the respective header is needed. However, this is not
necessarily the behavior we want as it has no connection to the location
of the declaration (which can actually be in the header in question).
Instead, this warning is generated

- if we could not build the type for the pattern on file (for some reason). 
Here we should make the reason explicit. The actual problem is otherwise 
circumvented as the warning is misleading, see [0] for an example.
- if we could not build the type for the pattern because we do not have a type 
on record, possible since D55483 , we should 
not emit any warning. See [1] for a legitimate problem.

This patch address both cases. For the "setjmp" family a new warning is
introduced and for built-ins without type on record, so far
"pthread_create", we do not emit the warning anymore.

Also see: PR40692

[0] https://lkml.org/lkml/2019/1/11/718
[1] https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=235583


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58091

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Analysis/retain-release.m
  clang/test/Sema/builtin-setjmp.c
  clang/test/Sema/implicit-builtin-decl.c

Index: clang/test/Sema/implicit-builtin-decl.c
===
--- clang/test/Sema/implicit-builtin-decl.c
+++ clang/test/Sema/implicit-builtin-decl.c
@@ -55,14 +55,17 @@
 
 void snprintf() { }
 
-// PR8316
-void longjmp(); // expected-warning{{declaration of built-in function 'longjmp' requires inclusion of the header }}
+// PR8316 & PR40692
+void longjmp(); // expected-warning{{declaration of built-in 'longjmp' requires the definition of the 'jmp_buf' type}}
 
 extern float fmaxf(float, float);
 
 struct __jmp_buf_tag {};
-void sigsetjmp(struct __jmp_buf_tag[1], int); // expected-warning{{declaration of built-in function 'sigsetjmp' requires inclusion of the header }}
+void sigsetjmp(struct __jmp_buf_tag[1], int); // expected-warning{{declaration of built-in 'sigsetjmp' requires the definition of the 'jmp_buf' type}}
 
 // CHECK: FunctionDecl {{.*}}  col:6 sigsetjmp '
 // CHECK-NOT: FunctionDecl
 // CHECK: ReturnsTwiceAttr {{.*}} <{{.*}}> Implicit
+
+// PR40692
+void pthread_create(); // no warning expected
Index: clang/test/Sema/builtin-setjmp.c
===
--- /dev/null
+++ clang/test/Sema/builtin-setjmp.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -verify=no_jmp_buf -DNO_JMP_BUF %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -verify=redecl %s
+
+#ifdef NO_JMP_BUF
+extern long setjmp(long *);   // no_jmp_buf-warning {{declaration of built-in 'setjmp' requires the definition of the 'jmp_buf' type}}
+#else
+typedef long jmp_buf;
+extern int setjmp(char);  // redecl-warning@8 {{incompatible redeclaration of library function 'setjmp'}}
+  // redecl-note@8{{'setjmp' is a builtin with type 'int (jmp_buf)' (aka 'int (long)')}}
+#endif
Index: clang/test/Analysis/retain-release.m
===
--- clang/test/Analysis/retain-release.m
+++ clang/test/Analysis/retain-release.m
@@ -2,7 +2,7 @@
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10\
 // RUN: -analyzer-checker=core,osx.coreFoundation.CFRetainRelease\
 // RUN: -analyzer-checker=osx.cocoa.ClassRelease,osx.cocoa.RetainCount\
-// RUN: -analyzer-checker=debug.ExprInspection -fblocks -verify=expected,C %s\
+// RUN: -analyzer-checker=debug.ExprInspection -fblocks -verify %s\
 // RUN: -Wno-objc-root-class -analyzer-output=plist -o %t.objc.plist
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10\
 // RUN: -analyzer-checker=core,osx.coreFoundation.CFRetainRelease\
@@ -1231,7 +1231,7 @@
 typedef unsigned long __darwin_pthread_key_t;
 typedef __darwin_pthread_key_t pthread_key_t;
 
-int pthread_create(pthread_t *, const pthread_attr_t *,  // C-warning{{declaration of built-in function 'pthread_create' requires inclusion of the header }}
+int pthread_create(pthread_t *, const pthread_attr_t *,
void *(*)(void *), void *);
 
 int pthread_setspecific(pthread_key_t key, const void *value);
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -1955,10 +1955,27 @@
   ASTContext::GetBuiltinTypeError Error;
   QualType R = Context.GetBuiltinType(ID, Error);
   if (Error) {
- 

[PATCH] D57984: PR40642: Fix determination of whether the final statement of a statementexpression is a discarded-value expression.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D57984#1394050 , @rsmith wrote:

> @ABataev Is it intentional that we do not propagate `Allowed` through labels? 
> For example:
>
>   void f() {
> #pragma omp barrier // ok
>  
>   label:
> #pragma omp barrier // error, "cannot be an immediate substatement"
>  
>   label:
> ;
> #pragma omp barrier // ok
>   }
>
>
> ?


No, it is a bug.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57984



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

[PATCH] D57984: PR40642: Fix determination of whether the final statement of a statementexpression is a discarded-value expression.

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

rsmith added a comment.

@ABataev Is it intentional that we do not propagate `Allowed` through labels? 
For example:

  void f() {
#pragma omp barrier // ok
  
  label:
#pragma omp barrier // error, "cannot be an immediate substatement"
  
  label:
;
#pragma omp barrier // ok
  }

?




Comment at: include/clang/AST/Stmt.h:1598-1602
+  const Expr *getExprStmt() const;
+  Expr *getExprStmt() {
+const ValueStmt *ConstThis = this;
+return const_cast(ConstThis->getExprStmt());
+  }

aaron.ballman wrote:
> We typically implement this in reverse, where the non-const version holds the 
> actual implementation and the const version performs a `const_cast`.
We do. Do you think that's preferable? I like that this way around proves that 
the `const` version is const-correct, but it's a tiny benefit and I'm fine with 
just being consistent.



Comment at: include/clang/Parse/Parser.h:374
+  /// a statement expression and builds a suitable expression statement.
+  StmtResult handleExprStmt(ExprResult E, WithinStmtExpr IsInStmtExpr);
 

aaron.ballman wrote:
> Rather than passing around `IsInStmtExpr` to a bunch of parser APIs, would it 
> make more sense to add an RAII object that sets some state on `Parser` to 
> test it? The proliferation of arguments seems unfortunate given how much of a 
> corner case statement expressions are.
Yeah, I tried that approach first, but the parser state turns out to be much 
worse, because it puts a burden on every form of statement that constructs a 
nested parsing context to reset that state. We can put the resetting on 
`ParseScope`, but it still needs to have an effect in the case where the scope 
is disabled, which is surprising, and there's no guarantee such statement 
constructs that can end in an expression will have a nested scope (consider, 
for instance, constructs like `return x;` or `goto *label;`). This is really 
local state that should only be propagated through a very small number of 
syntactic constructs rather than global state.

Maybe we could combine the new flag with the `AllowOpenMPStandalone` / 
`AllowedConstructsKind` flag into a more general kind of "statement context". 
The propagation rules aren't quite the same (`AllowOpenMPStandalone` doesn't 
get propagated through labels whereas `IsInStmtExpr` does), which is a little 
awkward, but maybe that's not so bad -- and maybe that's actually a bug in the 
OpenMP implementation?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57984



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

Re: r353729 - Attempt to pacify bots more after r353718 and r353725

[Nico Weber via cfe-commits]

Thank you for the .ll files!

the -4.ll file you sent me contains:

define dso_local i64 @"?test__shiftleft128@@YA_K_K0E@Z"(i64 %l, i64 %h, i8
%d) local_unnamed_addr #0 {
entry:
  %0 = zext i64 %h to i128
  %1 = shl nuw i128 %0, 64
  %2 = zext i64 %l to i128
  %3 = or i128 %1, %2
  %4 = and i8 %d, 63
  %5 = zext i8 %4 to i128
  %6 = shl i128 %3, %5
  %7 = lshr i128 %6, 64
  %8 = trunc i128 %7 to i64
  ret i64 %8
}

On my local system, I get

; Function Attrs: minsize norecurse nounwind optsize readnone
define dso_local i64 @test__shiftleft128(i64 %l, i64 %h, i8 %d)
local_unnamed_addr #0 {
entry:
  %0 = zext i64 %l to i128
  %1 = zext i64 %h to i128
  %2 = shl nuw i128 %1, 64
  %3 = or i128 %2, %0
  %4 = and i8 %d, 63
  %5 = zext i8 %4 to i128
  %6 = shl i128 %3, %5
  %7 = lshr i128 %6, 64
  %8 = trunc i128 %7 to i64
  ret i64 %8
}

That's identical except for the order of the instructions (which in turn
changes some % numbers).

That's surprising to me; I thought LLVM IR is deterministic (and if it
wasn't, many other tests wouldn't work either).

On Mon, Feb 11, 2019 at 4:20 PM Galina Kistanova 
wrote:

> Hello Nico,
>
> This builders fail on your test as well -
> http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/15736
> , http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/4242.
>
> Please find attached the 2 temp files you can use to reliably run against
> your FileCheck patterns.
> Hope this would help debugging.
>
> Please also notice the warnings each of the RUN command produces. The
> warnings should be quite easy to reproduce and address.
>
> In the mean time, could you revert the change unless you expect the fix
> coming really soon, please?
> It is not a good idea to keep the bots red for long.
>
> Here is the log:
> --
>
> C:\>c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\bin\clang.exe
> -cc1 -internal-isystem
> c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\lib\clang\9.0.0\include
> -nostdsysteminc -ffreestanding -fms-extensions -fms-compatibility
> -fms-compatibility-version=17.00 -triple i686--windows -Oz -emit-llvm
> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\CodeGen\ms-x86-intrinsics.c
> -o - > \tmp-1\ms-x86-intrinsics-1.ll
> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\CodeGen\ms-x86-intrinsics.c:10:10:
> warning: implicitly declaring library function '__readfsbyte' with type
> 'unsigned char (unsigned long)'
>   return __readfsbyte(++Offset);
>  ^
> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\CodeGen\ms-x86-intrinsics.c:10:10:
> note: include the header  or explicitly provide a declaration for
> '__readfsbyte'
> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\CodeGen\ms-x86-intrinsics.c:19:10:
> warning: implicitly declaring library function '__readfsword' with type
> 'unsigned short (unsigned long)'
>   return __readfsword(++Offset);
>  ^
> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\CodeGen\ms-x86-intrinsics.c:19:10:
> note: include the header  or explicitly provide a declaration for
> '__readfsword'
> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\CodeGen\ms-x86-intrinsics.c:28:10:
> warning: implicitly declaring library function '__readfsdword' with type
> 'unsigned long (unsigned long)'
>   return __readfsdword(++Offset);
>  ^
> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\CodeGen\ms-x86-intrinsics.c:28:10:
> note: include the header  or explicitly provide a declaration for
> '__readfsdword'
> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\CodeGen\ms-x86-intrinsics.c:37:10:
> warning: implicitly declaring library function '__readfsqword' with type
> 'unsigned long long (unsigned long)'
>   return __readfsqword(++Offset);
>  ^
> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\CodeGen\ms-x86-intrinsics.c:37:10:
> note: include the header  or explicitly provide a declaration for
> '__readfsqword'
> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\CodeGen\ms-x86-intrinsics.c:47:10:
> warning: implicitly declaring library function '__emul' with type 'long
> long (int, int)'
>   return __emul(a, b);
>  ^
> C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\CodeGen\ms-x86-intrinsics.c:47:10:
> note: include the header  or explicitly provide a declaration for
> '__emul'
> 

[PATCH] D58089: Add missing library dependencies in CMakeLists.txt

[Nicholas Allegra via Phabricator via cfe-commits]

comex created this revision.
comex added reviewers: jkorous, phosek.
Herald added subscribers: cfe-commits, kadircet, arphaman, ioeric, 
ilya-biryukov, mgorny.
Herald added a project: clang.

Fixes build in BUILD_SHARED_LIBS mode.

Removes the "DEPENDS clangdXpcJsonConversions" line as LINK_LIBS already 
implies a dependency AFAIK.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D58089

Files:
  clang-tools-extra/clangd/xpc/CMakeLists.txt


Index: clang-tools-extra/clangd/xpc/CMakeLists.txt
===
--- clang-tools-extra/clangd/xpc/CMakeLists.txt
+++ clang-tools-extra/clangd/xpc/CMakeLists.txt
@@ -20,10 +20,10 @@
 
 add_clang_library(clangdXpcJsonConversions
   Conversion.cpp
+  LINK_LIBS clangDaemon
   )
 
 add_clang_library(clangdXpcTransport
   XPCTransport.cpp
-  DEPENDS clangdXpcJsonConversions
-  LINK_LIBS clangdXpcJsonConversions
+  LINK_LIBS clangDaemon clangdXpcJsonConversions
   )


Index: clang-tools-extra/clangd/xpc/CMakeLists.txt
===
--- clang-tools-extra/clangd/xpc/CMakeLists.txt
+++ clang-tools-extra/clangd/xpc/CMakeLists.txt
@@ -20,10 +20,10 @@
 
 add_clang_library(clangdXpcJsonConversions
   Conversion.cpp
+  LINK_LIBS clangDaemon
   )
 
 add_clang_library(clangdXpcTransport
   XPCTransport.cpp
-  DEPENDS clangdXpcJsonConversions
-  LINK_LIBS clangdXpcJsonConversions
+  LINK_LIBS clangDaemon clangdXpcJsonConversions
   )
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57930: [Driver] Verify GCCInstallation is valid

[Daniel Mentz via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/Linux.cpp:259-260
 
   if (GCCInstallation.getParentLibPath().find("opt/rh/devtoolset") !=
   StringRef::npos)
 // With devtoolset on RHEL, we want to add a bin directory that is relative

tstellar wrote:
> Do we need to add the same check here too?
I'd say no, we don't need it here. If GCCInstallation is not valid, then 
GCCInstallation.getParentLibPath() will probably return the empty string, and 
.find("opt/rh/devtoolset") on that empty string will probably return 
StringRef::npos, in which case the if body is not run.
I understand the concern, but I think, in this case, we got lucky.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57930



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

[PATCH] D54978: Move the SMT API to LLVM

[Brian Rzycki via Phabricator via cfe-commits]

brzycki added a comment.

In D54978#1393968 , @thakis wrote:

> Do you understand why the default matters for you? You seem to explicitly 
> disable the setting, and you still get Z3 as part of your build. Did you make 
> a clean build dir before turning it to OFF?


Yes, Please see my recreation instructions above. I created a new, empty 
`build` directory.

> If so, I don't understand why the default setting is important to you and why 
> this doesn't work for you. (I don't disagree with the default being off, I'm 
> just confused why things don't work for you.)

As I have stated several times, the CMake option `-D 
LLVM_OPTIMIZED_TABLEGEN=ON` spawns a sub-command of CMake and **is required for 
the break to occur**. I don't know how to make this any more clear. If you 
build with optimized tablegen, it breaks. I strongly suspect an interaction 
between LLVM's top-level CMake and the TableGen one but I haven't had time to 
debug it down to the exact cause.

It is important to me because the detection of the correct version of Z3 is 
imprecise, at best.  If a Z3 library is found  I have no way to guarantee a 
build I run will not attempt to include the library.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54978



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

[PATCH] D57976: -gmodules: Don't emit incomplete breadcrumbs pointing to nonexistant PCM files.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D57976#1393976 , @aprantl wrote:

> In D57976#1393939 , @dblaikie wrote:
>
> > This, I guess, is part of the impact of moving towards explicit modules 
> > (-fmodule-name is for building a module with that name, right?)?
>
>
> That option is overloaded. It's used to specify the name of an explicit 
> modules too, but in this context particularly it is used to specify that 
> clang is compiling the Framework that defines the module with that name, 
> which has the effect of *not* building the module of that name as a clang 
> module even though -fmodules was specified and every other module is built as 
> a module. This is necessary for developing Frameworks that are part of the 
> macOS SDK to avoid accidentally include the module from the OS instead of the 
> one being developed. That's a quite specific use-case.
>
> > With explicit modules there is the option for modular code generation - 
> > which doesn't require any specific DWARF consumer support (so should "just 
> > work" - if you've a build system you can teach to generate a .o file from 
> > the .pcm, and link that .o with everything else)
>
> Different can of worms, I believe.


Ah, fair enough - thanks for the context!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57976



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

Re: r353370 - Generalised the SMT state constraints

[Mikhail Ramalho via cfe-commits]

Hi Douglas, thank you for the report.

We are discussing the issue in https://reviews.llvm.org/D54975.

@michaelplatings said he found a solution and I asked him to run the tests
locally. If it indeed fixes the issue, I'd say we push it.



Em seg, 11 de fev de 2019 às 20:13,  escreveu:

> Hi Mikhail,
>
> Your commit seems to be causing a build failure on our internal Windows
> build bot that uses Visual Studio 2015. Can you take a look?
>
> C:\src\upstream\llvm_clean\tools\clang\include\clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h(23):
> error C2872: 'ConstraintSMTTy': ambiguous symbol
> [C:\src\upstream\353370-PS4-Release\tools\clang\lib\StaticAnalyzer\Core\clangStaticAnalyzerCore.vcxproj]
>
> C:\src\upstream\llvm_clean\tools\clang\include\clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h(22):
> note: could be 'llvm::ImmutableSet clang::ento::SMTExpr *>,llvm::ImutContainerInfo> ConstraintSMTTy'
>   with
>   [
>   ValT=std::pair clang::ento::SMTExpr *>
>   ]
>
> C:\src\upstream\llvm_clean\tools\clang\include\clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h(23):
> note: or   '`anonymous-namespace'::ConstraintSMTTy'
>
> I was able to reproduce the build failure locally on my Windows machine
> using Visual Studio 2015 Update 3 (version 19.00.24215.1). Here are the
> cmake and build commands I used:
>
> CMake.exe -G "Visual Studio 14 Win64" -DCMAKE_BUILD_TYPE=Release
> -DLLVM_ENABLE_ASSERTIONS=ON -DLLVM_DEFAULT_TARGET_TRIPLE=x86_64-scei-ps4
> -DLLVM_TOOL_COMPILER_RT_BUILD:BOOL=OFF -DLLVM_BUILD_TESTS:BOOL=ON
> -DLLVM_BUILD_EXAMPLES:BOOL=ON -DCLANG_BUILD_EXAMPLES:BOOL=ON
> -DLLVM_TARGETS_TO_BUILD=X86 -DLLVM_LIT_ARGS="-v -j8"
> C:\src\upstream\llvm_clean
>
> Douglas Yung
>
> -Original Message-
> From: cfe-commits  On Behalf Of
> Mikhail R. Gadelha via cfe-commits
> Sent: Wednesday, February 6, 2019 19:18
> To: cfe-commits@lists.llvm.org
> Subject: r353370 - Generalised the SMT state constraints
>
> Author: mramalho
> Date: Wed Feb  6 19:17:36 2019
> New Revision: 353370
>
> URL: http://llvm.org/viewvc/llvm-project?rev=353370=rev
> Log:
> Generalised the SMT state constraints
>
> This patch moves the ConstraintSMT definition to the SMTConstraintManager
> header to make it easier to move the Z3 backend around.
>
> We achieve this by not using shared_ptr  anymore, as llvm::ImmutableSet
> doesn't seem to like it.
>
> The solver specific exprs and sorts are cached in the Z3Solver object now
> and we move pointers to those objects around.
>
> As a nice side-effect, SMTConstraintManager doesn't have to be a template
> anymore. Yay!
>
> Differential Revision: https://reviews.llvm.org/D54975
>
> Modified:
>
> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTExpr.h
> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTSolver.h
> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTSort.h
> cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
>
> Modified:
> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h?rev=353370=353369=353370=diff
>
> ==
> ---
> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
> (original)
> +++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstra
> +++ intManager.h Wed Feb  6 19:17:36 2019
> @@ -17,10 +17,14 @@
>  #include
> "clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h"
>  #include "clang/StaticAnalyzer/Core/PathSensitive/SMTConv.h"
>
> +typedef llvm::ImmutableSet<
> +std::pair>
> +ConstraintSMTTy;
> +REGISTER_TRAIT_WITH_PROGRAMSTATE(ConstraintSMT, ConstraintSMTTy)
> +
>  namespace clang {
>  namespace ento {
>
> -template   class
> SMTConstraintManager : public clang::ento::SimpleConstraintManager {
>SMTSolverRef 
>
> @@ -212,7 +216,7 @@ public:
>  OS << nl << sep << "Constraints:";
>  for (auto I = CZ.begin(), E = CZ.end(); I != E; ++I) {
>OS << nl << ' ' << I->first << " : ";
> -  I->second.print(OS);
> +  I->second->print(OS);
>  }
>  OS << nl;
>}
> @@ -272,8 +276,7 @@ protected:
>   const SMTExprRef ) {
>  // Check the model, avoid simplifying AST to save time
>  if (checkModel(State, Sym, Exp).isConstrainedTrue())
> -  return State->add(
> -  std::make_pair(Sym, static_cast(*Exp)));
> +  return State->add(std::make_pair(Sym, Exp));
>
>  return nullptr;
>}
> @@ -289,9 +292,9 @@ protected:
>  if (I != IE) {
>std::vector ASTs;
>
> -  SMTExprRef Constraint = Solver->newExprRef(I++->second);
> +  SMTExprRef Constraint = I++->second;
>

[PATCH] D57976: -gmodules: Don't emit incomplete breadcrumbs pointing to nonexistant PCM files.

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

In D57976#1393939 , @dblaikie wrote:

> This, I guess, is part of the impact of moving towards explicit modules 
> (-fmodule-name is for building a module with that name, right?)?


That option is overloaded. It's used to specify the name of an explicit modules 
too, but in this context particularly it is used to specify that clang is 
compiling the Framework that defines the module with that name, which has the 
effect of *not* building the module of that name as a clang module even though 
-fmodules was specified and every other module is built as a module. This is 
necessary for developing Frameworks that are part of the macOS SDK to avoid 
accidentally include the module from the OS instead of the one being developed. 
That's a quite specific use-case.

> With explicit modules there is the option for modular code generation - which 
> doesn't require any specific DWARF consumer support (so should "just work" - 
> if you've a build system you can teach to generate a .o file from the .pcm, 
> and link that .o with everything else)

Different can of worms, I believe.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57976



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

RE: r353370 - Generalised the SMT state constraints

[via cfe-commits]

Hi Mikhail,

Your commit seems to be causing a build failure on our internal Windows build 
bot that uses Visual Studio 2015. Can you take a look?

C:\src\upstream\llvm_clean\tools\clang\include\clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h(23):
 error C2872: 'ConstraintSMTTy': ambiguous symbol 
[C:\src\upstream\353370-PS4-Release\tools\clang\lib\StaticAnalyzer\Core\clangStaticAnalyzerCore.vcxproj]
  
C:\src\upstream\llvm_clean\tools\clang\include\clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h(22):
 note: could be 'llvm::ImmutableSet,llvm::ImutContainerInfo> ConstraintSMTTy'
  with
  [
  ValT=std::pair
  ]
  
C:\src\upstream\llvm_clean\tools\clang\include\clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h(23):
 note: or   '`anonymous-namespace'::ConstraintSMTTy'

I was able to reproduce the build failure locally on my Windows machine using 
Visual Studio 2015 Update 3 (version 19.00.24215.1). Here are the cmake and 
build commands I used:

CMake.exe -G "Visual Studio 14 Win64" -DCMAKE_BUILD_TYPE=Release 
-DLLVM_ENABLE_ASSERTIONS=ON -DLLVM_DEFAULT_TARGET_TRIPLE=x86_64-scei-ps4 
-DLLVM_TOOL_COMPILER_RT_BUILD:BOOL=OFF -DLLVM_BUILD_TESTS:BOOL=ON 
-DLLVM_BUILD_EXAMPLES:BOOL=ON -DCLANG_BUILD_EXAMPLES:BOOL=ON 
-DLLVM_TARGETS_TO_BUILD=X86 -DLLVM_LIT_ARGS="-v -j8" C:\src\upstream\llvm_clean

Douglas Yung

-Original Message-
From: cfe-commits  On Behalf Of Mikhail R. 
Gadelha via cfe-commits
Sent: Wednesday, February 6, 2019 19:18
To: cfe-commits@lists.llvm.org
Subject: r353370 - Generalised the SMT state constraints

Author: mramalho
Date: Wed Feb  6 19:17:36 2019
New Revision: 353370

URL: http://llvm.org/viewvc/llvm-project?rev=353370=rev
Log:
Generalised the SMT state constraints

This patch moves the ConstraintSMT definition to the SMTConstraintManager 
header to make it easier to move the Z3 backend around.

We achieve this by not using shared_ptr  anymore, as llvm::ImmutableSet doesn't 
seem to like it.

The solver specific exprs and sorts are cached in the Z3Solver object now and 
we move pointers to those objects around.

As a nice side-effect, SMTConstraintManager doesn't have to be a template 
anymore. Yay!

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

Modified:

cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTExpr.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTSolver.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTSort.h
cfe/trunk/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h?rev=353370=353369=353370=diff
==
--- 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
 (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstra
+++ intManager.h Wed Feb  6 19:17:36 2019
@@ -17,10 +17,14 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SMTConv.h"
 
+typedef llvm::ImmutableSet<
+std::pair>
+ConstraintSMTTy;
+REGISTER_TRAIT_WITH_PROGRAMSTATE(ConstraintSMT, ConstraintSMTTy)
+
 namespace clang {
 namespace ento {
 
-template   class 
SMTConstraintManager : public clang::ento::SimpleConstraintManager {
   SMTSolverRef 
 
@@ -212,7 +216,7 @@ public:
 OS << nl << sep << "Constraints:";
 for (auto I = CZ.begin(), E = CZ.end(); I != E; ++I) {
   OS << nl << ' ' << I->first << " : ";
-  I->second.print(OS);
+  I->second->print(OS);
 }
 OS << nl;
   }
@@ -272,8 +276,7 @@ protected:
  const SMTExprRef ) {
 // Check the model, avoid simplifying AST to save time
 if (checkModel(State, Sym, Exp).isConstrainedTrue())
-  return State->add(
-  std::make_pair(Sym, static_cast(*Exp)));
+  return State->add(std::make_pair(Sym, Exp));
 
 return nullptr;
   }
@@ -289,9 +292,9 @@ protected:
 if (I != IE) {
   std::vector ASTs;
 
-  SMTExprRef Constraint = Solver->newExprRef(I++->second);
+  SMTExprRef Constraint = I++->second;
   while (I != IE) {
-Constraint = Solver->mkAnd(Constraint, 
Solver->newExprRef(I++->second));
+Constraint = Solver->mkAnd(Constraint, I++->second);
   }
 
   Solver->addConstraint(Constraint);
@@ -301,8 +304,8 @@ protected:
   // Generate and check a Z3 model, using the given constraint.
   ConditionTruthVal checkModel(ProgramStateRef State, SymbolRef Sym,
const SMTExprRef ) const {
-ProgramStateRef NewState = State->add(
-std::make_pair(Sym, 

[PATCH] D54978: Move the SMT API to LLVM

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

> In D54978#1393552 , @ddcc wrote:
> 
>> The likely reason for this versioning problem is that the current versioning 
>> implementation in FindZ3.cmake is best-effort only.
> 
> 
> Thank you @ddcc for this explanation. If that's the case I'd really prefer if 
> `LLVM_ENABLE_Z3_SOLVER` was explicit opt-in and defaulted to `OFF` regardless 
> of what `find_package` returned.

Do you understand why the default matters for you? You seem to explicitly 
disable the setting, and you still get Z3 as part of your build. Did you make a 
clean build dir before turning it to OFF? If so, I don't understand why the 
default setting is important to you and why this doesn't work for you. (I don't 
disagree with the default being off, I'm just confused why things don't work 
for you.)


Repository:
  rC Clang

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

https://reviews.llvm.org/D54978



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

[PATCH] D57976: -gmodules: Don't emit incomplete breadcrumbs pointing to nonexistant PCM files.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

This, I guess, is part of the impact of moving towards explicit modules 
(-fmodule-name is for building a module with that name, right?)?

With explicit modules there is the option for modular code generation - which 
doesn't require any specific DWARF consumer support (so should "just work" - if 
you've a build system you can teach to generate a .o file from the .pcm, and 
link that .o with everything else)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57976



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

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 updated this revision to Diff 186356.
jyu2 added a comment.

I find some ambiguous error for asm-goto out of scope.
Instead call VerifyIndirectOrAsmJumps one time, call that function twice one 
for indirect-goto one for asm-goto.

Let me know if you see any problems.

Thanks for all the review.
Jennifer


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

https://reviews.llvm.org/D56571

Files:
  include/clang/AST/Stmt.h
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/AST/ASTImporter.cpp
  lib/AST/Stmt.cpp
  lib/AST/StmtPrinter.cpp
  lib/AST/StmtProfile.cpp
  lib/Analysis/CFG.cpp
  lib/CodeGen/CGStmt.cpp
  lib/Parse/ParseStmtAsm.cpp
  lib/Sema/JumpDiagnostics.cpp
  lib/Sema/SemaStmtAsm.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  test/Analysis/asm-goto.cpp
  test/CodeGen/asm-goto.c
  test/CodeGen/asm.c
  test/CodeGen/inline-asm-mixed-style.c
  test/Coverage/c-language-features.inc
  test/PCH/asm.h
  test/Parser/asm.c
  test/Parser/asm.cpp
  test/Sema/asm-goto.cpp
  test/Sema/asm.c
  test/Sema/inline-asm-validate-tmpl.cpp
  test/Sema/scope-check.c

Index: test/Sema/scope-check.c
===
--- test/Sema/scope-check.c
+++ test/Sema/scope-check.c
@@ -232,3 +232,19 @@
 
 // rdar://9024687
 int test16(int [sizeof &]); // expected-error {{use of address-of-label extension outside of a function body}}
+
+//Asm goto:
+int test16(int n)
+{
+  // expected-error@+2 {{cannot jump from this asm goto statement to one of its possible targets}}
+  // expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
+  asm volatile goto("testl %0, %0; jne %l1;" :: "r"(n)::label_true, loop);
+  // expected-note@+2 {{jump bypasses initialization of variable length array}}
+  // expected-note@+1 {{possible target of asm goto statement}}
+  return ({int a[n];label_true: 2;});
+  // expected-note@+1 {{jump bypasses initialization of variable length array}}
+  int b[n];
+// expected-note@+1 {{possible target of asm goto statement}}
+loop:
+  return 0;
+}
Index: test/Sema/inline-asm-validate-tmpl.cpp
===
--- test/Sema/inline-asm-validate-tmpl.cpp
+++ test/Sema/inline-asm-validate-tmpl.cpp
@@ -23,3 +23,13 @@
 	asm("rol %1, %0" :"=r"(value): "I"(N + 1));
 }
 int	foo() { testc<2>(10); }
+
+// these should compile without error
+template  bool testd()
+{
+  __asm goto ("" : : : : lab);
+  return true;
+lab:
+  return false;
+}
+bool foox() { return testd<0> (); }
Index: test/Sema/asm.c
===
--- test/Sema/asm.c
+++ test/Sema/asm.c
@@ -295,3 +295,24 @@
   return r0 + r1;
 }
 
+void test18()
+{
+  // expected-error@+2 {{duplicate use of asm operand name "lab"}}
+  // expected-note@+1 {{asm operand name "lab" first referenced here}}
+  asm goto ("" : : : : lab, lab, lab2, lab);
+  // expected-error@+2 {{duplicate use of asm operand name "lab"}}
+  // expected-note@+1 {{asm operand name "lab" first referenced here}}
+  asm goto ("xorw %[lab], %[lab]; je %l[lab]" : : [lab] "i" (0) : : lab);
+lab:;
+lab2:;
+  int x,x1;
+  // expected-error@+2 {{duplicate use of asm operand name "lab"}}
+  // expected-note@+1 {{asm operand name "lab" first referenced here}}
+  asm ("" : [lab] "=r" (x),[lab] "+r" (x) : [lab1] "r" (x));
+  // expected-error@+2 {{duplicate use of asm operand name "lab"}}
+  // expected-note@+1 {{asm operand name "lab" first referenced here}}
+  asm ("" : [lab] "=r" (x1) : [lab] "r" (x));
+  // expected-error@+1 {{invalid operand number in inline asm string}}
+  asm ("jne %l0":::);
+  asm goto ("jne %l0"lab);
+}
Index: test/Sema/asm-goto.cpp
===
--- /dev/null
+++ test/Sema/asm-goto.cpp
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 %s -triple i386-pc-linux-gnu -verify -fsyntax-only
+
+struct NonTrivial {
+  ~NonTrivial();
+  int f(int);
+private:
+  int k;
+};
+void JumpDiagnostics(int n) {
+// expected-error@+1 {{cannot jump from this goto statement to its label}}
+  goto DirectJump;
+// expected-note@+1 {{jump bypasses variable with a non-trivial destructor}}
+  NonTrivial tnp1;
+
+DirectJump:
+// expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
+  asm goto("jmp %l0;" Later);
+// expected-note@+1 {{jump bypasses variable with a non-trivial destructor}}
+  NonTrivial tnp2;
+// expected-note@+1 {{possible target of asm goto statement}}
+Later:
+  return;
+}
+
+struct S { ~S(); };
+void foo(int a) {
+  if (a) {
+FOO:
+// expected-note@+2 {{jump exits scope of variable with non-trivial destructor}}
+// expected-note@+1 {{jump exits scope of variable with non-trivial destructor}}
+S s;
+void *p = &
+// expected-error@+1 {{cannot jump from this asm goto 

[PATCH] D57236: [ASTImporter] Unify redecl chain tests as type parameterized tests

[Aleksei Sidorin via Phabricator via cfe-commits]

a_sidorin added a comment.

Hi Gabor,

The refactoring looks great. I have some minor comments inline.




Comment at: unittests/AST/ASTImporterTest.cpp:3549
+  void TypedTest_ImportDefinitionThenPrototype() {
+Decl *FromTU0 = getTuDecl(getDefinition(), Lang_CXX, "input0.cc");
+Decl *FromTU1 = getTuDecl(getPrototype(), Lang_CXX, "input1.cc");

I would ask you not to use numbers when possible: these names are hard to keep 
in mind. There can be a nice naming scheme like 
"FromTUDef/FromTUProto/FromTUProtoDef|, I think it would be much better.



Comment at: unittests/AST/ASTImporterTest.cpp:3704
+// FIXME This does not pass, possible error with Class import.
+//ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Class,
+
//ImportDefinitionAfterImportedPrototype);

Is it possible to disable the tests instead of commenting them out?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57236



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

r353765 - Add a new attribute, fortify_stdlib

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Mon Feb 11 15:21:39 2019
New Revision: 353765

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

This attribute applies to declarations of C stdlib functions
(sprintf, memcpy...) that have known fortified variants
(__sprintf_chk, __memcpy_chk, ...). When applied, clang will emit
calls to the fortified variant functions instead of calls to the
defaults.

In GCC, this is done by adding gnu_inline-style wrapper functions,
but that doesn't work for us for variadic functions because we don't
support __builtin_va_arg_pack (and have no intention to).

This attribute takes two arguments, the first is 'type' argument
passed through to __builtin_object_size, and the second is a flag
argument that gets passed through to the variadic checking variants.

rdar://47905754

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

Added:
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=353765=353764=353765=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Mon Feb 11 15:21:39 2019
@@ -1567,6 +1567,13 @@ 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=353765=353764=353765=diff
==
--- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
+++ cfe/trunk/include/clang/Basic/AttrDocs.td Mon Feb 11 15:21:39 2019
@@ -965,6 +965,43 @@ 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=353765=353764=353765=diff
==
--- cfe/trunk/include/clang/Basic/Builtins.h (original)
+++ cfe/trunk/include/clang/Basic/Builtins.h Mon Feb 11 15:21:39 2019
@@ -242,7 +242,13 @@ 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: 

[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 marked 8 inline comments as done.
erik.pilkington added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:987-989
+The first argument to the attribute is the type passed to
+`__builtin_object_size`, and the second is the flag that the fortified format
+functions accept.

aaron.ballman wrote:
> erik.pilkington wrote:
> > aaron.ballman wrote:
> > > Maybe I'm being dense, but I still have no idea what I'd pass for either 
> > > of these arguments. When I hear "type", I immediately assume I should 
> > > pass in something like 'int', but that seems wrong given that this is an 
> > > integer argument. Is there some public documentation we could link to 
> > > with a "for more information, see " snippet? Similar for the 
> > > fortified format function flag.
> > > 
> > > However, I'm also starting to wonder why this attribute is interesting to 
> > > users. Why not infer this attribute automatically, since there's only a 
> > > specified set of standard library functions that it can be applied to 
> > > anyway? Is it a bad idea to try to infer this for some reason?
> > Yeah, I guess we could link to GCC's docs for `__builtin_object_size`, I'll 
> > update this. I think the flag argument has something to do with enabling 
> > checking `%n` output parameters, but I'm not totally sure and don't want to 
> > spread any misinformation in the clang docs. On our implementation, the 
> > value is just ignored. 
> > 
> > This attribute would never really be used by users that aren't C library 
> > implementers. The problem with doing this automatically is that library 
> > users need to be able to customize the checking mode by defining the 
> > `_FORTIFY_SOURCE` macro to a level in their `.c` files. We can't do this 
> > based on the presence of that macro for a couple reasons, firstly, I'm not 
> > sure we can assume that all `*_chk` variants are present just because 
> > `_FORTIFY_SOURCE` is defined (whether the `_chk` variants are present seems 
> > like a decision best left to the library). And secondly because that would 
> > mean that `clang -E t.c -o - | clang -xc -` would generate different code 
> > from `clang t.c`. Given that, it seems like an attribute is the best 
> > customization point.
> Thank you for the explanation -- I agree that an attribute probably makes 
> sense then. I'd appreciate a note in the docs saying something along the 
> lines of "This attribute is intended for use within standard C library 
> implementations and should not generally be used for user applications." or 
> some such. WDYT?
Sure, I added that in the commit.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6437
+S.Diag(AL.getArgAsExpr(0)->getBeginLoc(),
+   diag::err_attribute_argument_outof_range)
+<< AL << 0 << 3;

aaron.ballman wrote:
> lol, I'll fix that typo after you land your patch.
Thanks, guess you're less lazy then me :)


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] D57855: [analyzer] Reimplement checker options

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

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



Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:384
+  CheckerOptions<[
+CmdLineOption It would be great if there were a way to define options once and reuse them.
I think it's possible with something similar:

```
def OptimisticOption :
  CmdLineOption;

def DynamicMemoryModeling: Checker<"DynamicMemoryModeling">,
  HelpText<"The base of several malloc() related checkers. On it's own it "
   "emits no reports, but adds valuable information to the analysis "
   "when enabled.">,
  CheckerOptions<[
OptimisticOption
  ]>,
  Dependencies<[CStringModeling]>,
  Documentation;

```


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

https://reviews.llvm.org/D57855



___
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

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL353765: Add a new attribute, fortify_stdlib (authored by 
epilk, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57918?vs=186300=186351#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57918

Files:
  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/CodeGen/fortify-std-lib.c
  cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
  cfe/trunk/test/Sema/fortify-std-lib.c

Index: cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
+++ cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -52,6 +52,7 @@
 // CHECK-NEXT: ExternalSourceSymbol ((SubjectMatchRule_record, SubjectMatchRule_enum, SubjectMatchRule_enum_constant, SubjectMatchRule_field, SubjectMatchRule_function, SubjectMatchRule_namespace, SubjectMatchRule_objc_category, SubjectMatchRule_objc_interface, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property, SubjectMatchRule_objc_protocol, SubjectMatchRule_record, SubjectMatchRule_type_alias, SubjectMatchRule_variable))
 // CHECK-NEXT: FlagEnum (SubjectMatchRule_enum)
 // CHECK-NEXT: Flatten (SubjectMatchRule_function)
+// CHECK-NEXT: FortifyStdLib (SubjectMatchRule_function)
 // CHECK-NEXT: GNUInline (SubjectMatchRule_function)
 // CHECK-NEXT: Hot (SubjectMatchRule_function)
 // CHECK-NEXT: IBAction (SubjectMatchRule_objc_method_is_instance)
Index: cfe/trunk/test/Sema/fortify-std-lib.c
===
--- cfe/trunk/test/Sema/fortify-std-lib.c
+++ cfe/trunk/test/Sema/fortify-std-lib.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 -fsyntax-only %s -verify
+
+typedef unsigned long size_t;
+
+__attribute__((fortify_stdlib(0, 0)))
+int not_anything_special(); // expected-error {{'fortify_stdlib' attribute applied to an unknown function}}
+
+__attribute__((fortify_stdlib(4, 0))) // expected-error {{'fortify_stdlib' attribute requires integer constant between 0 and 3 inclusive}}
+int sprintf(char *, const char *, ...);
+
+__attribute__((fortify_stdlib())) // expected-error {{'fortify_stdlib' attribute requires exactly 2 arguments}}
+int sprintf(char *, const char *, ...);
+
+__attribute__((fortify_stdlib(1, 2, 3))) // expected-error {{'fortify_stdlib' attribute requires exactly 2 arguments}}
+int sprintf(char *, const char *, ...);
+
+__attribute__((fortify_stdlib(-1, 2))) // expected-error {{'fortify_stdlib' attribute requires a non-negative integral compile time constant expression}}
+int sprintf(char *, const char *, ...);
Index: cfe/trunk/test/CodeGen/fortify-std-lib.c
===
--- cfe/trunk/test/CodeGen/fortify-std-lib.c
+++ cfe/trunk/test/CodeGen/fortify-std-lib.c
@@ -0,0 +1,220 @@
+// RUN: %clang_cc1   -triple x86_64-apple-macosx10.14.0 %s -emit-llvm -O0 -disable-llvm-passes -o - -Wno-format-security | FileCheck %s
+// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -emit-llvm -O0 -disable-llvm-passes -o - -Wno-format-security | FileCheck %s
+
+#ifdef __cplusplus
+#define EXTERN extern "C"
+#else
+#define EXTERN
+#endif
+
+#define FSL(x,y) __attribute__((fortify_stdlib(x,y)))
+typedef unsigned long size_t;
+
+FSL(0, 0) EXTERN
+void *memcpy(void *dst, const void *src, size_t sz);
+
+EXTERN
+void call_memcpy(void *dst, const void *src, size_t sz) {
+  memcpy(dst, src, sz);
+  // CHECK-LABEL: define void @call_memcpy
+  // CHECK: [[REG:%[0-9]+]] = call i64 @llvm.objectsize.i64.p0i8(i8*{{.*}}, i1 false, i1 true, i1 false)
+  // CHECK: call i8* @__memcpy_chk(i8* {{.*}}, i8* {{.*}}, i64 {{.*}}, i64 [[REG]])
+}
+
+FSL(0, 0) EXTERN
+void *memmove(void *dst, const void *src, size_t sz);
+
+EXTERN
+void call_memmove(void *dst, const void *src, size_t sz) {
+  memmove(dst, src, sz);
+  // CHECK-LABEL: define void @call_memmove
+  // CHECK: [[REG:%[0-9]+]] = call i64 @llvm.objectsize.i64.p0i8(i8*{{.*}}, i1 false, i1 true, i1 false)
+  // CHECK: call i8* @__memmove_chk(i8* {{.*}}, i8* {{.*}}, i64 {{.*}}, i64 [[REG]])
+}
+
+FSL(0, 0) EXTERN
+void *memset(void *dst, int c, size_t sz);
+
+EXTERN
+void call_memset(void *dst, int c, size_t sz) {
+  memset(dst, c, sz);
+  // CHECK-LABEL: define void @call_memset
+  // CHECK: [[REG:%[0-9]+]] = call i64 @llvm.objectsize.i64.p0i8(i8*{{.*}}, i1 false, i1 true, i1 false)
+  // CHECK: 

[PATCH] D57855: [analyzer] Reimplement checker options

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

Szelethus updated this revision to Diff 186350.
Szelethus added a comment.

- Removed the accidentally added `nullability:Optimistic` entry,
- Reworded `unix.DynamicMemoryModeling:Optimistic`.


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

https://reviews.llvm.org/D57855

Files:
  include/clang/Basic/DiagnosticCommonKinds.td
  include/clang/StaticAnalyzer/Checkers/CheckerBase.td
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
  lib/Frontend/CompilerInvocation.cpp
  lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
  lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
  test/Analysis/disable-all-checks.c
  test/Analysis/invalid-checker-option.c
  utils/TableGen/ClangSACheckersEmitter.cpp

Index: utils/TableGen/ClangSACheckersEmitter.cpp
===
--- utils/TableGen/ClangSACheckersEmitter.cpp
+++ utils/TableGen/ClangSACheckersEmitter.cpp
@@ -90,6 +90,23 @@
   .str();
 }
 
+static std::string getCheckerOptionType(const Record ) {
+  if (BitsInit *BI = R.getValueAsBitsInit("Type")) {
+switch(getValueFromBitsInit(BI, R)) {
+  case 0:
+return "int";
+  case 1:
+return "string";
+  case 2:
+return "bool";
+}
+  }
+  PrintFatalError(R.getLoc(),
+  "unable to parse command line option type for "
+  + getCheckerFullName());
+  return "";
+}
+
 static void printChecker(llvm::raw_ostream , const Record ) {
 OS << "CHECKER(" << "\"";
 OS.write_escaped(getCheckerFullName()) << "\", ";
@@ -134,6 +151,45 @@
   OS << "#endif // GET_PACKAGES\n"
 "\n";
 
+  // Emit a package option.
+  //
+  // PACKAGE_OPTION(OPTIONTYPE, PACKAGENAME, OPTIONNAME, DESCRIPTION, DEFAULT)
+  //   - OPTIONTYPE: Type of the option, whether it's integer or boolean etc.
+  // This is important for validating user input. Note that
+  // it's a string, rather than an actual type: since we can
+  // load checkers runtime, we can't use template hackery for
+  // sorting this out compile-time.
+  //   - PACKAGENAME: Name of the package.
+  //   - OPTIONNAME: Name of the option.
+  //   - DESCRIPTION
+  //   - DEFAULT: The default value for this option.
+  //
+  // The full option can be specified in the command like like this:
+  //   -analyzer-config PACKAGENAME:OPTIONNAME=VALUE
+  OS << "\n"
+"#ifdef GET_PACKAGE_OPTIONS\n";
+  for (const Record *package : packages) {
+
+if (package->isValueUnset("PackageOptions"))
+  continue;
+
+std::vector PackageOptions = package
+   ->getValueAsListOfDefs("PackageOptions");
+for (Record *PackageOpt : PackageOptions) {
+  OS << "PACKAGE_OPTION(\"";
+  OS.write_escaped(getCheckerOptionType(*PackageOpt)) << "\", \"";
+  OS.write_escaped(getPackageFullName(package)) << "\", ";
+  OS << '\"' << getStringValue(*PackageOpt, "CmdFlag") << "\", ";
+  OS << '\"';
+  OS.write_escaped(getStringValue(*PackageOpt, "Desc")) << "\", ";
+  OS << '\"';
+  OS.write_escaped(getStringValue(*PackageOpt, "DefaultVal")) << "\"";
+  OS << ")\n";
+}
+  }
+  OS << "#endif // GET_PACKAGE_OPTIONS\n"
+"\n";
+
   // Emit checkers.
   //
   // CHECKER(FULLNAME, CLASS, HELPTEXT)
@@ -176,5 +232,46 @@
   }
   OS << "\n"
 "#endif // GET_CHECKER_DEPENDENCIES\n";
+
+  // Emit a package option.
+  //
+  // CHECKER_OPTION(OPTIONTYPE, CHECKERNAME, OPTIONNAME, DESCRIPTION, DEFAULT)
+  //   - OPTIONTYPE: Type of the option, whether it's integer or boolean etc.
+  // This is important for validating user input. Note that
+  // it's a string, rather than an actual type: since we can
+  // load checkers runtime, we can't use template hackery for
+  // sorting this out compile-time.
+  //   - CHECKERNAME: Name of the package.
+  //   - OPTIONNAME: Name of the option.
+  //   - DESCRIPTION
+  //   - DEFAULT: The default value for this option.
+  //
+  // The full option can be specified in the command like like this:
+  //   -analyzer-config CHECKERNAME:OPTIONNAME=VALUE
+  OS << "\n"
+"#ifdef GET_CHECKER_OPTIONS\n";
+  for (const Record *checker : checkers) {
+
+if (checker->isValueUnset("CheckerOptions"))
+  continue;
+
+
+std::vector CheckerOptions = checker
+   ->getValueAsListOfDefs("CheckerOptions");
+for (Record *CheckerOpt : CheckerOptions) {
+  OS << "CHECKER_OPTION(\"";
+  OS << getCheckerOptionType(*CheckerOpt) << "\", \"";
+  OS.write_escaped(getCheckerFullName(checker)) << "\", ";
+  OS << '\"' << getStringValue(*CheckerOpt, "CmdFlag") << "\", ";
+  OS << '\"';
+  OS.write_escaped(getStringValue(*CheckerOpt, "Desc")) << "\", ";
+  OS << '\"';
+  OS.write_escaped(getStringValue(*CheckerOpt, 

[PATCH] D58069: [Sema] Mark GNU compound literal array init as an rvalue.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

> The analyzer test change maybe indicates we could simplify the analyzer code 
> a little with this fix? Apparently a hack was added to support lvalues in 
> initializers in r315750, but I'm not really familiar with the relevant code.

Nope, unfortunately, not much can be simplified. This hack is much deeper and 
covers many more cases than that and was added much earlier :) But thanks for 
attracting my attention to this!


Repository:
  rC Clang

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

https://reviews.llvm.org/D58069



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

[PATCH] D54881: [clang-format] Prevent Clang-Format from editing leading whitespace on lines outside of the format range

[Russell McClellan via Phabricator via cfe-commits]

russellmcc added a comment.

Bump! Thanks for the consideration!  Again, this is preventing us from rolling 
out automated style checking on diffs at work.


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

https://reviews.llvm.org/D54881



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

[PATCH] D57850: [analyzer] Emit an error rather than assert on invalid checker option input

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

Szelethus updated this revision to Diff 186346.
Szelethus added a comment.

- Added a convenience method to `CheckerManager` for reporting invalid user 
input
- Edited the error message to be more flexible


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

https://reviews.llvm.org/D57850

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  include/clang/StaticAnalyzer/Core/CheckerManager.h
  lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
  lib/StaticAnalyzer/Checkers/CloneChecker.cpp
  lib/StaticAnalyzer/Checkers/MoveChecker.cpp
  lib/StaticAnalyzer/Checkers/PaddingChecker.cpp
  lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
  lib/StaticAnalyzer/Core/CheckerManager.cpp
  test/Analysis/copypaste/suspicious-clones.cpp
  test/Analysis/cxx-uninitialized-object-unionlike-constructs.cpp
  test/Analysis/outofbound.c
  test/Analysis/padding_c.c
  test/Analysis/undef-buffers.c
  test/Analysis/use-after-move.cpp

Index: test/Analysis/use-after-move.cpp
===
--- test/Analysis/use-after-move.cpp
+++ test/Analysis/use-after-move.cpp
@@ -27,6 +27,17 @@
 // RUN:  -analyzer-config cplusplus.Move:WarnOn=All -DAGGRESSIVE\
 // RUN:  -analyzer-checker debug.ExprInspection
 
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=cplusplus.Move \
+// RUN:   -analyzer-config cplusplus.Move:WarnOn="a bunch of things" \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-MOVE-INVALID-VALUE
+
+// CHECK-MOVE-INVALID-VALUE: (frontend): invalid input for checker option
+// CHECK-MOVE-INVALID-VALUE-SAME: 'cplusplus.Move:WarnOn', that expects either
+// CHECK-MOVE-INVALID-VALUE-SAME: "KnownsOnly", "KnownsAndLocals" or "All"
+// CHECK-MOVE-INVALID-VALUE-SAME: string value
+
 #include "Inputs/system-header-simulator-cxx.h"
 
 void clang_analyzer_warnIfReached();
Index: test/Analysis/undef-buffers.c
===
--- test/Analysis/undef-buffers.c
+++ test/Analysis/undef-buffers.c
@@ -2,7 +2,7 @@
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=unix \
 // RUN:   -analyzer-checker=core.uninitialized \
-// RUN:   -analyzer-config unix:Optimistic=true
+// RUN:   -analyzer-config unix.DynamicMemoryModeling:Optimistic=true
 
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
Index: test/Analysis/padding_c.c
===
--- test/Analysis/padding_c.c
+++ test/Analysis/padding_c.c
@@ -1,4 +1,16 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=optin.performance -analyzer-config optin.performance.Padding:AllowedPad=2 -verify %s
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=optin.performance \
+// RUN:   -analyzer-config optin.performance.Padding:AllowedPad=2
+
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=optin.performance.Padding \
+// RUN:   -analyzer-config optin.performance.Padding:AllowedPad=-10 \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-PAD-NEGATIVE-VALUE
+
+// CHECK-PAD-NEGATIVE-VALUE: (frontend): invalid input for checker option
+// CHECK-PAD-NEGATIVE-VALUE-SAME: 'optin.performance.Padding:AllowedPad', that
+// CHECK-PAD-NEGATIVE-VALUE-SAME: expects a non-negative value
 
 #if __has_include()
 #include 
Index: test/Analysis/outofbound.c
===
--- test/Analysis/outofbound.c
+++ test/Analysis/outofbound.c
@@ -2,7 +2,7 @@
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=unix \
 // RUN:   -analyzer-checker=alpha.security.ArrayBound \
-// RUN:   -analyzer-config unix:Optimistic=true
+// RUN:   -analyzer-config unix.DynamicMemoryModeling:Optimistic=true
 
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
Index: test/Analysis/cxx-uninitialized-object-unionlike-constructs.cpp
===
--- test/Analysis/cxx-uninitialized-object-unionlike-constructs.cpp
+++ test/Analysis/cxx-uninitialized-object-unionlike-constructs.cpp
@@ -3,6 +3,20 @@
 // RUN:   -analyzer-config alpha.cplusplus.UninitializedObject:IgnoreRecordsWithField="[Tt]ag|[Kk]ind" \
 // RUN:   -std=c++11 -verify  %s
 
+// RUN: not %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=alpha.cplusplus.UninitializedObject \
+// RUN:   -analyzer-config \
+// RUN: alpha.cplusplus.UninitializedObject:IgnoreRecordsWithField="([)]" \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-UNINIT-INVALID-REGEX
+
+// CHECK-UNINIT-INVALID-REGEX: (frontend): invalid input for checker option
+// CHECK-UNINIT-INVALID-REGEX-SAME: 'alpha.cplusplus.UninitializedObject:IgnoreRecordsWithField',
+// CHECK-UNINIT-INVALID-REGEX-SAME: that expects a valid regex, building failed
+// CHECK-UNINIT-INVALID-REGEX-SAME: with error 

[PATCH] D40988: Clang-format: add finer-grained options for putting all arguments on one line

[Russell McClellan via Phabricator via cfe-commits]

russellmcc added a comment.

Bump!  Still watching this space - thanks so much for your time!


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

https://reviews.llvm.org/D40988



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

r353762 - [Sema] Mark GNU compound literal array init as an rvalue.

[Eli Friedman via cfe-commits]

Author: efriedma
Date: Mon Feb 11 14:54:27 2019
New Revision: 353762

URL: http://llvm.org/viewvc/llvm-project?rev=353762=rev
Log:
[Sema] Mark GNU compound literal array init as an rvalue.

Basically the same issue as string init, except it didn't really have
any visible consequences before I removed the implicit lvalue-to-rvalue
conversion from CodeGen.

While I'm here, a couple minor drive-by cleanups: IgnoreParens never
returns a ConstantExpr, and there was a potential crash with string init
involving a ChooseExpr.

The analyzer test change maybe indicates we could simplify the analyzer
code a little with this fix?  Apparently a hack was added to support
lvalues in initializers in r315750, but I'm not really familiar with the
relevant code.

Fixes regression reported in the kernel build at
https://bugs.llvm.org/show_bug.cgi?id=40430#c6 .

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


Modified:
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/test/Analysis/compound-literals.c
cfe/trunk/test/CodeGen/compound-literal.c

Modified: cfe/trunk/lib/Sema/SemaInit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=353762=353761=353762=diff
==
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)
+++ cfe/trunk/lib/Sema/SemaInit.cpp Mon Feb 11 14:54:27 2019
@@ -145,16 +145,42 @@ static void updateStringLiteralType(Expr
   while (true) {
 E->setType(Ty);
 E->setValueKind(VK_RValue);
-if (isa(E) || isa(E))
+if (isa(E) || isa(E)) {
   break;
-else if (ParenExpr *PE = dyn_cast(E))
+} else if (ParenExpr *PE = dyn_cast(E)) {
   E = PE->getSubExpr();
-else if (UnaryOperator *UO = dyn_cast(E))
+} else if (UnaryOperator *UO = dyn_cast(E)) {
+  assert(UO->getOpcode() == UO_Extension);
   E = UO->getSubExpr();
-else if (GenericSelectionExpr *GSE = dyn_cast(E))
+} else if (GenericSelectionExpr *GSE = dyn_cast(E)) {
   E = GSE->getResultExpr();
-else
+} else if (ChooseExpr *CE = dyn_cast(E)) {
+  E = CE->getChosenSubExpr();
+} else {
   llvm_unreachable("unexpected expr in string literal init");
+}
+  }
+}
+
+/// Fix a compound literal initializing an array so it's correctly marked
+/// as an rvalue.
+static void updateGNUCompoundLiteralRValue(Expr *E) {
+  while (true) {
+E->setValueKind(VK_RValue);
+if (isa(E)) {
+  break;
+} else if (ParenExpr *PE = dyn_cast(E)) {
+  E = PE->getSubExpr();
+} else if (UnaryOperator *UO = dyn_cast(E)) {
+  assert(UO->getOpcode() == UO_Extension);
+  E = UO->getSubExpr();
+} else if (GenericSelectionExpr *GSE = dyn_cast(E)) {
+  E = GSE->getResultExpr();
+} else if (ChooseExpr *CE = dyn_cast(E)) {
+  E = CE->getChosenSubExpr();
+} else {
+  llvm_unreachable("unexpected expr in array compound literal init");
+}
   }
 }
 
@@ -5542,8 +5568,7 @@ void InitializationSequence::InitializeF
 // array from a compound literal that creates an array of the same
 // type, so long as the initializer has no side effects.
 if (!S.getLangOpts().CPlusPlus && Initializer &&
-(isa(Initializer->IgnoreParens()) ||
- isa(Initializer->IgnoreParens())) &&
+isa(Initializer->IgnoreParens()) &&
 Initializer->getType()->isArrayType()) {
   const ArrayType *SourceAT
 = Context.getAsArrayType(Initializer->getType());
@@ -7956,6 +7981,7 @@ ExprResult InitializationSequence::Perfo
   S.Diag(Kind.getLocation(), diag::ext_array_init_copy)
 << Step->Type << CurInit.get()->getType()
 << CurInit.get()->getSourceRange();
+  updateGNUCompoundLiteralRValue(CurInit.get());
   LLVM_FALLTHROUGH;
 case SK_ArrayInit:
   // If the destination type is an incomplete array type, update the

Modified: cfe/trunk/test/Analysis/compound-literals.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/compound-literals.c?rev=353762=353761=353762=diff
==
--- cfe/trunk/test/Analysis/compound-literals.c (original)
+++ cfe/trunk/test/Analysis/compound-literals.c Mon Feb 11 14:54:27 2019
@@ -4,6 +4,5 @@ void clang_analyzer_eval(int);
 // pr28449: Used to crash.
 void foo(void) {
   static const unsigned short array[] = (const unsigned short[]){0x0F00};
-  // FIXME: Should be true.
-  clang_analyzer_eval(array[0] == 0x0F00); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(array[0] == 0x0F00); // expected-warning{{TRUE}}
 }

Modified: cfe/trunk/test/CodeGen/compound-literal.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/compound-literal.c?rev=353762=353761=353762=diff
==
--- cfe/trunk/test/CodeGen/compound-literal.c (original)
+++ cfe/trunk/test/CodeGen/compound-literal.c Mon Feb 11 14:54:27 2019
@@ -11,6 +11,11 @@ _Complex 

[PATCH] D58069: [Sema] Mark GNU compound literal array init as an rvalue.

[Eli Friedman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
efriedma marked an inline comment as done.
Closed by commit rC353762: [Sema] Mark GNU compound literal array init as an 
rvalue. (authored by efriedma, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D58069?vs=186309=186345#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D58069

Files:
  lib/Sema/SemaInit.cpp
  test/Analysis/compound-literals.c
  test/CodeGen/compound-literal.c

Index: test/CodeGen/compound-literal.c
===
--- test/CodeGen/compound-literal.c
+++ test/CodeGen/compound-literal.c
@@ -11,6 +11,11 @@
 typedef int v4i32 __attribute((vector_size(16)));
 v4i32 *y = &(v4i32){1,2,3,4};
 
+// Check generated code for GNU constant array init from compound literal,
+// for a global variable.
+// CHECK: @compound_array = global [8 x i32] [i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 8]
+int compound_array[] = __extension__(__builtin_choose_expr(0, 0, _Generic(1, int: (int[]){1, 2, 3, 4, 5, 6, 7, 8})));
+
 void xxx() {
 int* a = &(int){1};
 struct s {int a, b, c;} * b = &(struct s) {1, 2, 3};
@@ -82,3 +87,13 @@
   const void *b = MyCLH;
   return a == b;
 }
+
+// Check generated code for GNU constant array init from compound literal,
+// for a local variable.
+// CHECK-LABEL: define i32 @compound_array_fn()
+// CHECK: [[COMPOUND_ARRAY:%.*]] = alloca [8 x i32]
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}}, i64 32, i1 false)
+int compound_array_fn() {
+  int compound_array[] = (int[]){1,2,3,4,5,6,7,8};
+  return compound_array[0];
+}
Index: test/Analysis/compound-literals.c
===
--- test/Analysis/compound-literals.c
+++ test/Analysis/compound-literals.c
@@ -4,6 +4,5 @@
 // pr28449: Used to crash.
 void foo(void) {
   static const unsigned short array[] = (const unsigned short[]){0x0F00};
-  // FIXME: Should be true.
-  clang_analyzer_eval(array[0] == 0x0F00); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(array[0] == 0x0F00); // expected-warning{{TRUE}}
 }
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -145,16 +145,42 @@
   while (true) {
 E->setType(Ty);
 E->setValueKind(VK_RValue);
-if (isa(E) || isa(E))
+if (isa(E) || isa(E)) {
   break;
-else if (ParenExpr *PE = dyn_cast(E))
+} else if (ParenExpr *PE = dyn_cast(E)) {
   E = PE->getSubExpr();
-else if (UnaryOperator *UO = dyn_cast(E))
+} else if (UnaryOperator *UO = dyn_cast(E)) {
+  assert(UO->getOpcode() == UO_Extension);
   E = UO->getSubExpr();
-else if (GenericSelectionExpr *GSE = dyn_cast(E))
+} else if (GenericSelectionExpr *GSE = dyn_cast(E)) {
   E = GSE->getResultExpr();
-else
+} else if (ChooseExpr *CE = dyn_cast(E)) {
+  E = CE->getChosenSubExpr();
+} else {
   llvm_unreachable("unexpected expr in string literal init");
+}
+  }
+}
+
+/// Fix a compound literal initializing an array so it's correctly marked
+/// as an rvalue.
+static void updateGNUCompoundLiteralRValue(Expr *E) {
+  while (true) {
+E->setValueKind(VK_RValue);
+if (isa(E)) {
+  break;
+} else if (ParenExpr *PE = dyn_cast(E)) {
+  E = PE->getSubExpr();
+} else if (UnaryOperator *UO = dyn_cast(E)) {
+  assert(UO->getOpcode() == UO_Extension);
+  E = UO->getSubExpr();
+} else if (GenericSelectionExpr *GSE = dyn_cast(E)) {
+  E = GSE->getResultExpr();
+} else if (ChooseExpr *CE = dyn_cast(E)) {
+  E = CE->getChosenSubExpr();
+} else {
+  llvm_unreachable("unexpected expr in array compound literal init");
+}
   }
 }
 
@@ -5542,8 +5568,7 @@
 // array from a compound literal that creates an array of the same
 // type, so long as the initializer has no side effects.
 if (!S.getLangOpts().CPlusPlus && Initializer &&
-(isa(Initializer->IgnoreParens()) ||
- isa(Initializer->IgnoreParens())) &&
+isa(Initializer->IgnoreParens()) &&
 Initializer->getType()->isArrayType()) {
   const ArrayType *SourceAT
 = Context.getAsArrayType(Initializer->getType());
@@ -7956,6 +7981,7 @@
   S.Diag(Kind.getLocation(), diag::ext_array_init_copy)
 << Step->Type << CurInit.get()->getType()
 << CurInit.get()->getSourceRange();
+  updateGNUCompoundLiteralRValue(CurInit.get());
   LLVM_FALLTHROUGH;
 case SK_ArrayInit:
   // If the destination type is an incomplete array type, update the
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58069: [Sema] Mark GNU compound literal array init as an rvalue.

[Eli Friedman via Phabricator via cfe-commits]

efriedma marked 2 inline comments as done.
efriedma added inline comments.



Comment at: lib/Sema/SemaInit.cpp:172-173
+  E = PE->getSubExpr();
+else if (UnaryOperator *UO = dyn_cast(E))
+  E = UO->getSubExpr();
+else if (GenericSelectionExpr *GSE = dyn_cast(E))

rsmith wrote:
> I guess this is just for `UO_Extension`. Is it worth asserting that?
I don't think it's likely that anyone would ever trip the assertion, but maybe 
worthwhile just to document the meaning, sure.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58069



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

[PATCH] D57874: [WebAssembly] Make thread-related options consistent

[Heejin Ahn via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL353761: [WebAssembly] Make thread-related options consistent 
(authored by aheejin, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57874

Files:
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/include/clang/Driver/ToolChain.h
  cfe/trunk/lib/Driver/Driver.cpp
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp
  cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp
  cfe/trunk/lib/Driver/ToolChains/WebAssembly.h
  cfe/trunk/test/Driver/wasm-toolchain.c
  cfe/trunk/test/Preprocessor/wasm-target-features.c

Index: cfe/trunk/test/Preprocessor/wasm-target-features.c
===
--- cfe/trunk/test/Preprocessor/wasm-target-features.c
+++ cfe/trunk/test/Preprocessor/wasm-target-features.c
@@ -52,11 +52,13 @@
 //
 // BULK-MEMORY:#define __wasm_bulk_memory__ 1{{$}}
 //
+// We don't use -matomics directly and '-mthread-model posix' sets the atomics
+// target feature.
 // RUN: %clang -E -dM %s -o - 2>&1 \
-// RUN: -target wasm32-unknown-unknown -matomics \
+// RUN: -target wasm32-unknown-unknown -mthread-model posix \
 // RUN:   | FileCheck %s -check-prefix=ATOMICS
 // RUN: %clang -E -dM %s -o - 2>&1 \
-// RUN: -target wasm64-unknown-unknown -matomics \
+// RUN: -target wasm64-unknown-unknown -mthread-model posix \
 // RUN:   | FileCheck %s -check-prefix=ATOMICS
 //
 // ATOMICS:#define __wasm_atomics__ 1{{$}}
Index: cfe/trunk/test/Driver/wasm-toolchain.c
===
--- cfe/trunk/test/Driver/wasm-toolchain.c
+++ cfe/trunk/test/Driver/wasm-toolchain.c
@@ -38,3 +38,16 @@
 
 // RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-wasi-musl --sysroot=/foo %s 2>&1 | FileCheck -check-prefix=COMPILE %s
 // COMPILE: clang{{.*}}" "-cc1" {{.*}} "-internal-isystem" "/foo/include/wasm32-wasi-musl" "-internal-isystem" "/foo/include"
+
+// Thread-related command line tests.
+// - '-mthread-model' sets '-target-feature +matomics'
+// - '-phread' sets both '-target-featuer +atomics' and '-mthread-model posix'
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -mthread-model posix 2>&1 | FileCheck -check-prefix=POSIX %s
+// POSIX: clang{{.*}}" "-cc1" {{.*}} "-target-feature" "+atomics"
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -pthread 2>&1 | FileCheck -check-prefix=PTHREAD %s
+// PTHREAD: clang{{.*}}" "-cc1" {{.*}} "-mthread-model" "posix" {{.*}} "-target-feature" "+atomics"
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s -pthread -mthread-model single 2>&1 | FileCheck -check-prefix=THREAD_OPT_ERROR %s
+// THREAD_OPT_ERROR: invalid argument '-pthread' not allowed with '-mthread-model single'
Index: cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp
+++ cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp
@@ -20,6 +20,27 @@
 using namespace clang;
 using namespace llvm::opt;
 
+void parseThreadArgs(const Driver , const ArgList ,
+ bool , StringRef ,
+ bool CheckForErrors = true) {
+  // Default value for -pthread / -mthread-model options, each being false /
+  // "single".
+  Pthread =
+  DriverArgs.hasFlag(options::OPT_pthread, options::OPT_no_pthread, false);
+  ThreadModel =
+  DriverArgs.getLastArgValue(options::OPT_mthread_model, "single");
+  if (!CheckForErrors)
+return;
+
+  // Did user explicitly specify -mthread-model / -pthread?
+  bool HasThreadModel = DriverArgs.hasArg(options::OPT_mthread_model);
+  bool HasPthread = Pthread && DriverArgs.hasArg(options::OPT_pthread);
+  // '-pthread' cannot be used with '-mthread-model single'
+  if (HasPthread && HasThreadModel && ThreadModel == "single")
+Driver.Diag(diag::err_drv_argument_not_allowed_with)
+<< "-pthread" << "-mthread-model single";
+}
+
 wasm::Linker::Linker(const ToolChain )
 : GnuTool("wasm::Linker", "lld", TC) {}
 
@@ -123,6 +144,17 @@
   if (DriverArgs.hasFlag(clang::driver::options::OPT_fuse_init_array,
  options::OPT_fno_use_init_array, true))
 CC1Args.push_back("-fuse-init-array");
+
+  // Either '-mthread-model posix' or '-pthread' sets '-target-feature
+  // +atomics'. We intentionally didn't create '-matomics' and set the atomics
+  // target feature here depending on the other two options.
+  bool Pthread = false;
+  StringRef ThreadModel = "";
+  parseThreadArgs(getDriver(), DriverArgs, Pthread, ThreadModel);
+  if (Pthread || ThreadModel != "single") {
+CC1Args.push_back("-target-feature");
+CC1Args.push_back("+atomics");
+  

r353761 - [WebAssembly] Make thread-related options consistent

[Heejin Ahn via cfe-commits]

Author: aheejin
Date: Mon Feb 11 14:47:50 2019
New Revision: 353761

URL: http://llvm.org/viewvc/llvm-project?rev=353761=rev
Log:
[WebAssembly] Make thread-related options consistent

Summary:
There have been three options related to threads and users had to set
all three of them separately to get the correct compilation results.
This makes sure the relationship between the options makes sense and
sets necessary options for users if only part of the necessary options
are specified. This does:

- Remove `-matomics`; this option alone does not enable anything, so
  removed it to not confuse users.
- `-mthread-model posix` sets `-target-feature +atomics`
- `-pthread` sets both `-target-feature +atomics` and
  `-mthread-model posix`
Also errors out when explicitly given options don't match, such as
`-pthread` is given with `-mthread-model single`.

Reviewers: dschuff, sbc100, tlively, sunfish

Subscribers: jgravelle-google, jfb, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Driver/ToolChain.h
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp
cfe/trunk/lib/Driver/ToolChains/WebAssembly.h
cfe/trunk/test/Driver/wasm-toolchain.c
cfe/trunk/test/Preprocessor/wasm-target-features.c

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=353761=353760=353761=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Mon Feb 11 14:47:50 2019
@@ -2160,8 +2160,6 @@ def mexception_handing : Flag<["-"], "me
 def mno_exception_handing : Flag<["-"], "mno-exception-handling">, 
Group;
 def mbulk_memory : Flag<["-"], "mbulk-memory">, Group;
 def mno_bulk_memory : Flag<["-"], "mno-bulk-memory">, 
Group;
-def matomics : Flag<["-"], "matomics">, Group;
-def mno_atomics : Flag<["-"], "mno-atomics">, Group;
 
 def mamdgpu_debugger_abi : Joined<["-"], "mamdgpu-debugger-abi=">,
   Flags<[HelpHidden]>,

Modified: cfe/trunk/include/clang/Driver/ToolChain.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/ToolChain.h?rev=353761=353760=353761=diff
==
--- cfe/trunk/include/clang/Driver/ToolChain.h (original)
+++ cfe/trunk/include/clang/Driver/ToolChain.h Mon Feb 11 14:47:50 2019
@@ -453,7 +453,9 @@ public:
   virtual bool SupportsEmbeddedBitcode() const { return false; }
 
   /// getThreadModel() - Which thread model does this target use?
-  virtual std::string getThreadModel() const { return "posix"; }
+  virtual std::string getThreadModel(const llvm::opt::ArgList &) const {
+return "posix";
+  }
 
   /// isThreadModelSupported() - Does this target support a thread model?
   virtual bool isThreadModelSupported(const StringRef Model) const;

Modified: cfe/trunk/lib/Driver/Driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=353761=353760=353761=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Mon Feb 11 14:47:50 2019
@@ -1528,7 +1528,7 @@ void Driver::PrintVersion(const Compilat
 if (TC.isThreadModelSupported(A->getValue()))
   OS << "Thread model: " << A->getValue();
   } else
-OS << "Thread model: " << TC.getThreadModel();
+OS << "Thread model: " << TC.getThreadModel(C.getArgs());
   OS << '\n';
 
   // Print out the install directory.

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=353761=353760=353761=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Mon Feb 11 14:47:50 2019
@@ -3823,7 +3823,7 @@ void Clang::ConstructJob(Compilation ,
 CmdArgs.push_back(A->getValue());
   }
   else
-CmdArgs.push_back(Args.MakeArgString(TC.getThreadModel()));
+CmdArgs.push_back(Args.MakeArgString(TC.getThreadModel(Args)));
 
   Args.AddLastArg(CmdArgs, options::OPT_fveclib);
 

Modified: cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp?rev=353761=353760=353761=diff
==
--- cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/WebAssembly.cpp Mon Feb 11 14:47:50 2019
@@ -20,6 +20,27 @@ using namespace clang::driver::toolchain
 using namespace clang;
 using namespace llvm::opt;
 
+void parseThreadArgs(const Driver , const 

[PATCH] D58072: Make ModuleDependencyCollector's method virtual (NFC)

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added a comment.

Not really. Would making only the `attachTo*` methods virtual enough though?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58072



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

[PATCH] D57265: [PM/CC1] Add -f[no-]split-cold-code CC1 options to toggle splitting

[Fedor Sergeev via Phabricator via cfe-commits]

fedor.sergeev added a comment.

In D57265#1393453 , @vsk wrote:

> > I'd prefer not adding this kind of state to PassBuilder. SplitColdCode is 
> > soemthing that refers to the construction of one particular pipeline, not 
> > to pipeline-building in general. It should be an argument passed down 
> > through the build*Pipeline calls.
>
> I'm not sure I understand the distinction being made here -- could you 
> elaborate? Isn't enabling a pass related to pipeline-building in general? If 
> not, I don't see how arguments to build*Pipeline //do// become related to 
> pipeline-building in general.
>
> For context, I've modeled the addition of the SplitColdCode member to 
> PassBuilder on PGOOpt (c.f. PGOOptions::RunProfileGen). One thing I like 
> about this approach is that it doesn't require changing IR, or changing very 
> many different APIs. But if it's really not reasonable, I'm happy to take 
> another shot at it.


I cant say for PGOOpt in particular, but overall idea of PassBuilder is that, 
well, it is not "builder" as per "builder pattern".
You do not fill it with parts of a complex pipeline object and then press a 
magical "build" button.
Instead you ask it to build various "named" pipelines, or just parse it from 
text.
If you compare with PassManagerBuilder, which contains a dozen of various data 
members that seem to drive the pipeline construction,
PassBuilder contains only a few. And the purpose of PassBuilder members is to 
be used during individual *pass*es creation.

Say, you wont find OptLevel there. Instead it is being passed as an argument to 
build*Pipeline functions.

As for PGOOpts - it seems to be the only member that stands out.
And to me it looks like we should remove it from PassBuilder either, and start 
passing it to build*Pipeline functions as well.
But honestly, this is the first time I really looked through the PGOOpts code, 
so I might be well mistaken.


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

https://reviews.llvm.org/D57265



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

[clang-tools-extra] r353760 - [NFC][clangd] Remove unused lambda capture

[Francis Visoiu Mistrih via cfe-commits]

Author: thegameg
Date: Mon Feb 11 14:36:47 2019
New Revision: 353760

URL: http://llvm.org/viewvc/llvm-project?rev=353760=rev
Log:
[NFC][clangd] Remove unused lambda capture

Avoid this warning:

llvm/clang-tools-extra/clangd/ClangdServer.cpp:365:23: warning: lambda
capture 'this' is not used [-Wunused-lambda-capture]

  auto Action = [Sel, this](decltype(CB) CB, std::string File,
~~^~~~
1 warning generated.

Modified:
clang-tools-extra/trunk/clangd/ClangdServer.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=353760=353759=353760=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Mon Feb 11 14:36:47 2019
@@ -362,7 +362,7 @@ void ClangdServer::enumerateTweaks(PathR
 
 void ClangdServer::applyTweak(PathRef File, Range Sel, StringRef TweakID,
   Callback CB) {
-  auto Action = [Sel, this](decltype(CB) CB, std::string File,
+  auto Action = [Sel](decltype(CB) CB, std::string File,
 std::string TweakID,
 Expected InpAST) {
 if (!InpAST)


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

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

[Lingda Li via Phabricator via cfe-commits]

lildmh updated this revision to Diff 186338.

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

https://reviews.llvm.org/D58074

Files:
  include/clang/AST/OpenMPClause.h
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/OpenMPKinds.def
  include/clang/Parse/Parser.h
  include/clang/Sema/Sema.h
  lib/AST/OpenMPClause.cpp
  lib/Parse/ParseOpenMP.cpp
  lib/Sema/SemaLookup.cpp
  lib/Sema/SemaOpenMP.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  test/OpenMP/declare_mapper_ast_print.c
  test/OpenMP/declare_mapper_ast_print.cpp
  test/OpenMP/declare_mapper_messages.c
  test/OpenMP/declare_mapper_messages.cpp
  test/OpenMP/target_map_messages.cpp
  test/OpenMP/target_parallel_for_map_messages.cpp
  test/OpenMP/target_parallel_for_simd_map_messages.cpp
  test/OpenMP/target_parallel_map_messages.cpp
  test/OpenMP/target_simd_map_messages.cpp
  test/OpenMP/target_teams_distribute_map_messages.cpp
  test/OpenMP/target_teams_distribute_parallel_for_map_messages.cpp
  test/OpenMP/target_teams_distribute_parallel_for_simd_map_messages.cpp
  test/OpenMP/target_teams_distribute_simd_map_messages.cpp
  test/OpenMP/target_teams_map_messages.cpp

Index: test/OpenMP/target_teams_map_messages.cpp
===
--- test/OpenMP/target_teams_map_messages.cpp
+++ test/OpenMP/target_teams_map_messages.cpp
@@ -454,7 +454,7 @@
 
 #pragma omp target data map(always, tofrom: x)
 #pragma omp target data map(always: x) // expected-error {{missing map type}}
-#pragma omp target data map(tofrom, always: x) // expected-error {{incorrect map type modifier, expected 'always' or 'close'}} expected-error {{missing map type}}
+#pragma omp target data map(tofrom, always: x) // expected-error {{incorrect map type modifier, expected 'always', 'close', or 'mapper'}} expected-error {{missing map type}}
 #pragma omp target data map(always, tofrom: always, tofrom, x)
 #pragma omp target teams map(tofrom j) // expected-error {{expected ',' or ')' in 'map' clause}}
   foo();
@@ -529,7 +529,7 @@
 
 #pragma omp target data map(always, tofrom: x)
 #pragma omp target data map(always: x) // expected-error {{missing map type}}
-#pragma omp target data map(tofrom, always: x) // expected-error {{incorrect map type modifier, expected 'always' or 'close'}} expected-error {{missing map type}}
+#pragma omp target data map(tofrom, always: x) // expected-error {{incorrect map type modifier, expected 'always', 'close', or 'mapper'}} expected-error {{missing map type}}
 #pragma omp target data map(always, tofrom: always, tofrom, x)
 #pragma omp target teams map(tofrom j) // expected-error {{expected ',' or ')' in 'map' clause}}
   foo();
Index: test/OpenMP/target_teams_distribute_simd_map_messages.cpp
===
--- test/OpenMP/target_teams_distribute_simd_map_messages.cpp
+++ test/OpenMP/target_teams_distribute_simd_map_messages.cpp
@@ -163,7 +163,7 @@
   for (i = 0; i < argc; ++i) foo();
 #pragma omp target teams distribute simd map(always: x) // expected-error {{missing map type}}
   for (i = 0; i < argc; ++i) foo();
-#pragma omp target teams distribute simd map(tofrom, always: x) // expected-error {{incorrect map type modifier, expected 'always' or 'close'}} expected-error {{missing map type}}
+#pragma omp target teams distribute simd map(tofrom, always: x) // expected-error {{incorrect map type modifier, expected 'always', 'close', or 'mapper'}} expected-error {{missing map type}}
   for (i = 0; i < argc; ++i) foo();
 #pragma omp target teams distribute simd map(always, tofrom: always, tofrom, x)
   for (i = 0; i < argc; ++i) foo();
@@ -271,7 +271,7 @@
   for (i = 0; i < argc; ++i) foo();
 #pragma omp target teams distribute simd map(always: x) // expected-error {{missing map type}}
   for (i = 0; i < argc; ++i) foo();
-#pragma omp target teams distribute simd map(tofrom, always: x) // expected-error {{incorrect map type modifier, expected 'always' or 'close'}} expected-error {{missing map type}}
+#pragma omp target teams distribute simd map(tofrom, always: x) // expected-error {{incorrect map type modifier, expected 'always', 'close', or 'mapper'}} expected-error {{missing map type}}
   for (i = 0; i < argc; ++i) foo();
 #pragma omp target teams distribute simd map(always, tofrom: always, tofrom, x)
   for (i = 0; i < argc; ++i) foo();
Index: test/OpenMP/target_teams_distribute_parallel_for_simd_map_messages.cpp
===
--- test/OpenMP/target_teams_distribute_parallel_for_simd_map_messages.cpp
+++ test/OpenMP/target_teams_distribute_parallel_for_simd_map_messages.cpp
@@ -163,7 +163,7 @@
   for (i = 0; i < argc; ++i) foo();
 #pragma omp target teams distribute parallel for simd map(always: x) // expected-error {{missing map 

[PATCH] D57984: PR40642: Fix determination of whether the final statement of a statementexpression is a discarded-value expression.

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

rsmith updated this revision to Diff 186337.
rsmith marked an inline comment as done.
rsmith added a comment.

Address a couple of review comments.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57984

Files:
  include/clang/AST/Expr.h
  include/clang/AST/Stmt.h
  include/clang/Basic/StmtNodes.td
  include/clang/Parse/Parser.h
  include/clang/Sema/Sema.h
  lib/AST/Stmt.cpp
  lib/CodeGen/CGStmt.cpp
  lib/Parse/ParseObjc.cpp
  lib/Parse/ParseStmt.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaStmt.cpp
  lib/Sema/TreeTransform.h
  test/CodeGenCXX/stmtexpr.cpp
  test/CodeGenCXX/volatile.cpp

Index: test/CodeGenCXX/volatile.cpp
===
--- test/CodeGenCXX/volatile.cpp
+++ test/CodeGenCXX/volatile.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin10 -emit-llvm -std=c++98 -o - | FileCheck %s
+// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin10 -emit-llvm -std=c++98 -o - | FileCheck -check-prefix=CHECK -check-prefix=CHECK98 %s
 // RUN: %clang_cc1 %s -triple=x86_64-apple-darwin10 -emit-llvm -std=c++11 -o - | FileCheck -check-prefix=CHECK -check-prefix=CHECK11 %s
 
 // Check that IR gen doesn't try to do an lvalue-to-rvalue conversion
@@ -33,3 +33,19 @@
 *x;
   }
 }
+
+namespace PR40642 {
+  template  struct S {
+// CHECK-LABEL: define {{.*}} @_ZN7PR406421SIiE3fooEv(
+void foo() {
+  // CHECK98-NOT: load volatile
+  // CHECK11: load volatile
+  if (true)
+reinterpret_cast(m_ptr)[0];
+  // CHECK: }
+}
+int *m_ptr;
+  };
+
+  void f(S *x) { x->foo(); }
+}
Index: test/CodeGenCXX/stmtexpr.cpp
===
--- test/CodeGenCXX/stmtexpr.cpp
+++ test/CodeGenCXX/stmtexpr.cpp
@@ -190,3 +190,79 @@
 // CHECK: %[[v2:[^ ]*]] = load float, float* %[[tmp2]]
 // CHECK: store float %[[v1]], float* %v.realp
 // CHECK: store float %[[v2]], float* %v.imagp
+
+extern "C" void then(int);
+
+// CHECK-LABEL: @{{.*}}volatile_load
+void volatile_load() {
+  volatile int n;
+
+  // CHECK-NOT: load volatile
+  // CHECK: load volatile
+  // CHECK-NOT: load volatile
+  ({n;});
+
+  // CHECK-LABEL: @then(i32 1)
+  then(1);
+
+  // CHECK-NOT: load volatile
+  // CHECK: load volatile
+  // CHECK-NOT: load volatile
+  ({goto lab; lab: n;});
+
+  // CHECK-LABEL: @then(i32 2)
+  then(2);
+
+  // CHECK-NOT: load volatile
+  // CHECK: load volatile
+  // CHECK-NOT: load volatile
+  ({[[gsl::suppress("foo")]] n;});
+
+  // CHECK-LABEL: @then(i32 3)
+  then(3);
+
+  // CHECK-NOT: load volatile
+  // CHECK: load volatile
+  // CHECK-NOT: load volatile
+  ({if (true) n;});
+
+  // CHECK: }
+}
+
+// CHECK-LABEL: @{{.*}}volatile_load_template
+template
+void volatile_load_template() {
+  volatile T n;
+
+  // CHECK-NOT: load volatile
+  // CHECK: load volatile
+  // CHECK-NOT: load volatile
+  ({n;});
+
+  // CHECK-LABEL: @then(i32 1)
+  then(1);
+
+  // CHECK-NOT: load volatile
+  // CHECK: load volatile
+  // CHECK-NOT: load volatile
+  ({goto lab; lab: n;});
+
+  // CHECK-LABEL: @then(i32 2)
+  then(2);
+
+  // CHECK-NOT: load volatile
+  // CHECK: load volatile
+  // CHECK-NOT: load volatile
+  ({[[gsl::suppress("foo")]] n;});
+
+  // CHECK-LABEL: @then(i32 3)
+  then(3);
+
+  // CHECK-NOT: load volatile
+  // CHECK: load volatile
+  // CHECK-NOT: load volatile
+  ({if (true) n;});
+
+  // CHECK: }
+}
+template void volatile_load_template();
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -318,6 +318,13 @@
   TypeSourceInfo *TransformTypeWithDeducedTST(TypeSourceInfo *DI);
   /// @}
 
+  /// The reason why the value of a statement is not discarded, if any.
+  enum StmtDiscardKind {
+SDK_Discarded,
+SDK_NotDiscarded,
+SDK_StmtExprResult,
+  };
+
   /// Transform the given statement.
   ///
   /// By default, this routine transforms a statement by delegating to the
@@ -327,7 +334,7 @@
   /// other mechanism.
   ///
   /// \returns the transformed statement.
-  StmtResult TransformStmt(Stmt *S, bool DiscardedValue = false);
+  StmtResult TransformStmt(Stmt *S, StmtDiscardKind SDK = SDK_Discarded);
 
   /// Transform the given statement.
   ///
@@ -672,6 +679,9 @@
 #define STMT(Node, Parent)\
   LLVM_ATTRIBUTE_NOINLINE \
   StmtResult Transform##Node(Node *S);
+#define VALUESTMT(Node, Parent)   \
+  LLVM_ATTRIBUTE_NOINLINE \
+  StmtResult Transform##Node(Node *S, StmtDiscardKind SDK);
 #define EXPR(Node, Parent)\
   LLVM_ATTRIBUTE_NOINLINE \
   ExprResult Transform##Node(Node *E);
@@ -3270,7 +3280,7 @@
 };
 
 template 
-StmtResult TreeTransform::TransformStmt(Stmt *S, bool DiscardedValue) {
+StmtResult TreeTransform::TransformStmt(Stmt *S, StmtDiscardKind SDK) {
   if (!S)
 return S;
 
@@ -3278,8 +3288,12 @@
   case 

[PATCH] D57850: [analyzer] Emit an error rather than assert on invalid checker option input

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

Szelethus marked 2 inline comments as done.
Szelethus added inline comments.



Comment at: include/clang/Basic/DiagnosticDriverKinds.td:304-305
 def err_analyzer_config_unknown : Error<"unknown analyzer-config '%0'">;
+def err_analyzer_checker_option_invalid_input : Error<
+  "invalid input for checker option '%0', that expects %1 value">;
 

NoQ wrote:
> I suggest hardcoding the word "value" in every message rather than putting it 
> here, because it may be desirable to substitute it with something more 
> specific or more accurate in every checker's specific circumstances.
> 
> Eg., "a non-negative integer" would be more precise than "a non-negative 
> value".
Yup, makes total sense!



Comment at: lib/StaticAnalyzer/Checkers/PaddingChecker.cpp:352-355
+  if (Checker->AllowedPad < 0)
+
Mgr.getDiagnostics().Report(diag::err_analyzer_checker_option_invalid_input)
+<< (llvm::Twine() + Checker->getTagDescription() + ":AllowedPad").str()
+<< "a non-negative";

NoQ wrote:
> 
> I passively wish for a certain amount of de-duplication that wouldn't require 
> every checker to obtain a diagnostics engine every time it tries to read an 
> option. Eg.,
> ```lang=c++
>   auto *Checker = Mgr.registerChecker();
>   Checker->AllowedPad = Mgr.getAnalyzerOptions()
>   .getCheckerIntegerOption(Checker, "AllowedPad", 24);
>   if (Checker->AllowedPad < 0)
> Mgr.reportInvalidOptionValue(Checker, "AllowedPad", "a non-negative");
> ```
> 
> Or maybe even something like that:
> 
> ```lang=c++
>   auto *Checker = Mgr.registerChecker();
>   Checker->AllowedPad = Mgr.getAnalyzerOptions()
>   .getCheckerIntegerOption(Checker, "AllowedPad", 24,
>   [](int x) -> Option {
>   if (x < 0) {
> // Makes getCheckerIntegerOption() emit a diagnostic
> // and return the default value.
> return "a non-negative";
>   }
>   // Makes getCheckerIntegerOption() successfully return
>   // the user-specified value.
>   return None;
>   });
> ```
> I.e., a validator lambda.
First one, sure. I'm a little unsure about the second: No other "options"-like 
classes have access to a `DiagnosticsEngine` in clang, as far as I'm aware, and 
I guess keeping `AnalyzerOptions` as simple is possible is preferred. Not only 
that, but a validator lambda seems an to be an overkill (though really-really 
cool) solution. Your first bit of code is far more readable IMO.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57850



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

[PATCH] D58069: [Sema] Mark GNU compound literal array init as an rvalue.

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

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



Comment at: lib/Sema/SemaInit.cpp:172-173
+  E = PE->getSubExpr();
+else if (UnaryOperator *UO = dyn_cast(E))
+  E = UO->getSubExpr();
+else if (GenericSelectionExpr *GSE = dyn_cast(E))

I guess this is just for `UO_Extension`. Is it worth asserting that?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58069



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

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

[Lingda Li via Phabricator via cfe-commits]

lildmh created this revision.
lildmh added reviewers: ABataev, hfinkel, Meinersbur.
lildmh added a project: OpenMP.
Herald added subscribers: cfe-commits, guansong.
Herald added a project: clang.

This patch implements the parsing and sema support for OpenMP map clauses with 
potential user-defined mapper attached. User defined mapper is a new feature in 
OpenMP 5.0. A map clause can have an explicit or implicit associated mapper, 
which instructs the compiler to generate extra data mapping. An example is 
shown below:

  struct S { int len; int *d; };
  #pragma omp declare mapper(id: struct S s) map(s, s.d[0:s.len])
  struct S ss;
  #pragma omp target map(mapper(id) tofrom: ss) // use the mapper with name 
'id' to map ss


Repository:
  rC Clang

https://reviews.llvm.org/D58074

Files:
  include/clang/AST/OpenMPClause.h
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/OpenMPKinds.def
  include/clang/Parse/Parser.h
  include/clang/Sema/Sema.h
  lib/AST/OpenMPClause.cpp
  lib/Parse/ParseOpenMP.cpp
  lib/Sema/SemaLookup.cpp
  lib/Sema/SemaOpenMP.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  test/OpenMP/declare_mapper_ast_print.c
  test/OpenMP/declare_mapper_ast_print.cpp
  test/OpenMP/declare_mapper_messages.c
  test/OpenMP/declare_mapper_messages.cpp
  test/OpenMP/target_map_messages.cpp
  test/OpenMP/target_parallel_for_map_messages.cpp
  test/OpenMP/target_parallel_for_simd_map_messages.cpp
  test/OpenMP/target_parallel_map_messages.cpp
  test/OpenMP/target_simd_map_messages.cpp
  test/OpenMP/target_teams_distribute_map_messages.cpp
  test/OpenMP/target_teams_distribute_parallel_for_map_messages.cpp
  test/OpenMP/target_teams_distribute_parallel_for_simd_map_messages.cpp
  test/OpenMP/target_teams_distribute_simd_map_messages.cpp
  test/OpenMP/target_teams_map_messages.cpp

Index: test/OpenMP/target_teams_map_messages.cpp
===
--- test/OpenMP/target_teams_map_messages.cpp
+++ test/OpenMP/target_teams_map_messages.cpp
@@ -454,7 +454,7 @@
 
 #pragma omp target data map(always, tofrom: x)
 #pragma omp target data map(always: x) // expected-error {{missing map type}}
-#pragma omp target data map(tofrom, always: x) // expected-error {{incorrect map type modifier, expected 'always' or 'close'}} expected-error {{missing map type}}
+#pragma omp target data map(tofrom, always: x) // expected-error {{incorrect map type modifier, expected 'always', 'close', or 'mapper'}} expected-error {{missing map type}}
 #pragma omp target data map(always, tofrom: always, tofrom, x)
 #pragma omp target teams map(tofrom j) // expected-error {{expected ',' or ')' in 'map' clause}}
   foo();
@@ -529,7 +529,7 @@
 
 #pragma omp target data map(always, tofrom: x)
 #pragma omp target data map(always: x) // expected-error {{missing map type}}
-#pragma omp target data map(tofrom, always: x) // expected-error {{incorrect map type modifier, expected 'always' or 'close'}} expected-error {{missing map type}}
+#pragma omp target data map(tofrom, always: x) // expected-error {{incorrect map type modifier, expected 'always', 'close', or 'mapper'}} expected-error {{missing map type}}
 #pragma omp target data map(always, tofrom: always, tofrom, x)
 #pragma omp target teams map(tofrom j) // expected-error {{expected ',' or ')' in 'map' clause}}
   foo();
Index: test/OpenMP/target_teams_distribute_simd_map_messages.cpp
===
--- test/OpenMP/target_teams_distribute_simd_map_messages.cpp
+++ test/OpenMP/target_teams_distribute_simd_map_messages.cpp
@@ -163,7 +163,7 @@
   for (i = 0; i < argc; ++i) foo();
 #pragma omp target teams distribute simd map(always: x) // expected-error {{missing map type}}
   for (i = 0; i < argc; ++i) foo();
-#pragma omp target teams distribute simd map(tofrom, always: x) // expected-error {{incorrect map type modifier, expected 'always' or 'close'}} expected-error {{missing map type}}
+#pragma omp target teams distribute simd map(tofrom, always: x) // expected-error {{incorrect map type modifier, expected 'always', 'close', or 'mapper'}} expected-error {{missing map type}}
   for (i = 0; i < argc; ++i) foo();
 #pragma omp target teams distribute simd map(always, tofrom: always, tofrom, x)
   for (i = 0; i < argc; ++i) foo();
@@ -271,7 +271,7 @@
   for (i = 0; i < argc; ++i) foo();
 #pragma omp target teams distribute simd map(always: x) // expected-error {{missing map type}}
   for (i = 0; i < argc; ++i) foo();
-#pragma omp target teams distribute simd map(tofrom, always: x) // expected-error {{incorrect map type modifier, expected 'always' or 'close'}} expected-error {{missing map type}}
+#pragma omp target teams distribute simd map(tofrom, always: x) // expected-error {{incorrect map type modifier, expected 'always', 'close', or 'mapper'}} 

[PATCH] D57890: [analyzer] Fix in self assignment checker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Sounds reasonable, but it also sounds like something that should be 
reproducible on the upstream clang. Do you have a code snippet that causes the 
problematic AST to appear? Even if we don't have the false positive up here in 
upstream, Is it something we can test via `-analyzer-display-progress | 
FileCheck` or with the help of the analysis order checker or something like 
that?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57890



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

[PATCH] D58067: [Analyzer] Crash fix for FindLastStoreBRVisitor

[Artem Dergachev via Phabricator via cfe-commits]

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

Aha, right, indeed, thanks, nice!

We should eventually remove `operator==()` from the `SVal` class because it 
doesn't do anything you'd possibly imagine it to do. However we do need to come 
up with adequate alternatives. And by looking at this patch, it seems that we 
actually need two alternatives:

1. An attempt to figure out that two `SVal`s are equal. The result of such 
comparison is state-specific - eg., ($a == $b) may be true in states that 
follow the true-branch of "if (a == b)" and false otherwise. This is how 
`evalBinOp(BO_EQ)` should behave. And this sounds impossible to implement 
without a perfect constraint solver, so the user should always be prepared to 
receive an unknown answer in some cases.
2. An attempt to figure out if two `SVal`s are representing the same value, 
that is, you cannot replace one value with another without an assignment. This 
is what we need in this patch. Eg., in

  int a;
  int b;
  
  void foo() {
if (a == b) {
  b = a;
} 
  }

we don't mind* noticing that an assignment has happened by noticing that `$b` 
was replaced with `$a`. I guess the author suspected that `SVal::operator==()` 
does exactly that; however, as you point out, for lazy compound values this is 
not the case because they're "not normalized", i.e. contain more information 
than necessary. And in fact it's not really possible to compare two lazy 
compound values (even for "sameness") without a perfect constraint solver 
(consider a symbolic-offset binding that would or would not overlap with the 
parent region of the LCV, depending on constraint offsets).

As far as i understand, your approach now misses assignments to fields within 
the lazy compound value (because you're not checking that the corresponding 
stores are actually equal when restricted to the given parent region), which 
may probably still lead to confusing diagnostics, but that's definitely better 
than before.

__
*Though ideally i'd definitely prefer noticing that an assignment has happened 
simply by realizing that there's an assignment operator at this program point. 
But that's a bigger question of why is our whole visitor infrastructure so 
inside-out. I mean, why does it have to reverse-engineer `ExprEngine`'s logic 
instead of simply communicating with it somehow?...




Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:156-158
+/// Two values match if either they are equal, or for LazyCompoundVals their
+/// regions are equal and their stores are equal to the stores of their
+/// exploded nodes.

I think it's more important for a comment to explain *why* exactly does the 
code do whatever it does, than *what* specifically does it do. I.e., could you 
say something about how comparing //internal representations// of symbolic 
values (via `SVal::operator==()`) is a valid way to check if the value was 
updated (even if the new value is in fact equal to the old value), unless it's 
a `LazyCompoundValue` that may have a different internal representation every 
time it is loaded from the state? ...Which is why you do an //approximate// 
comparison for lazy compound values, checking that they are the immediate 
snapshots of the tracked region's bindings within the node's respective states 
but not really checking that these snapshots actually contain the same set of 
bindings.

Also, is it possible that these values both have the same region that is 
different from the region `R` we're tracking? I suspect that it isn't possible 
with immediate loads from the `RegionStore` as it currently is, but i wouldn't 
expect this to happen in the general case or be future-proof. Maybe we should 
assert that - either within this function or immediately after this function.



Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:159
+/// exploded nodes.
+bool matchesValue(const ExplodedNode *LeftNode, SVal LeftVal,
+  const ExplodedNode *RightNode, SVal RightVal) {

baloghadamsoftware wrote:
> Maybe we should find a better name. Even better we could place this function 
> into `LazyCompoundVal` but with 'Store` or `ProgramStateRef` parameters 
> instead of `ExplodedNode*`.
Hmm, dunno. "`hasVisibleUpdate()`"?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58067



___
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

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from some very minor nits.




Comment at: clang/include/clang/Basic/AttrDocs.td:987-989
+The first argument to the attribute is the type passed to
+`__builtin_object_size`, and the second is the flag that the fortified format
+functions accept.

erik.pilkington wrote:
> aaron.ballman wrote:
> > Maybe I'm being dense, but I still have no idea what I'd pass for either of 
> > these arguments. When I hear "type", I immediately assume I should pass in 
> > something like 'int', but that seems wrong given that this is an integer 
> > argument. Is there some public documentation we could link to with a "for 
> > more information, see " snippet? Similar for the fortified format 
> > function flag.
> > 
> > However, I'm also starting to wonder why this attribute is interesting to 
> > users. Why not infer this attribute automatically, since there's only a 
> > specified set of standard library functions that it can be applied to 
> > anyway? Is it a bad idea to try to infer this for some reason?
> Yeah, I guess we could link to GCC's docs for `__builtin_object_size`, I'll 
> update this. I think the flag argument has something to do with enabling 
> checking `%n` output parameters, but I'm not totally sure and don't want to 
> spread any misinformation in the clang docs. On our implementation, the value 
> is just ignored. 
> 
> This attribute would never really be used by users that aren't C library 
> implementers. The problem with doing this automatically is that library users 
> need to be able to customize the checking mode by defining the 
> `_FORTIFY_SOURCE` macro to a level in their `.c` files. We can't do this 
> based on the presence of that macro for a couple reasons, firstly, I'm not 
> sure we can assume that all `*_chk` variants are present just because 
> `_FORTIFY_SOURCE` is defined (whether the `_chk` variants are present seems 
> like a decision best left to the library). And secondly because that would 
> mean that `clang -E t.c -o - | clang -xc -` would generate different code 
> from `clang t.c`. Given that, it seems like an attribute is the best 
> customization point.
Thank you for the explanation -- I agree that an attribute probably makes sense 
then. I'd appreciate a note in the docs saying something along the lines of 
"This attribute is intended for use within standard C library implementations 
and should not generally be used for user applications." or some such. WDYT?



Comment at: clang/include/clang/Basic/AttrDocs.td:994-1012
+  - memcpy
+  - memmove
+  - memset
+  - stpcpy
+  - strcat
+  - strcpy
+  - strlcat

Rather than enumerate a long list, can you shorten it vertically by grouping 
the functions? e.g., strpcpy, strcpy, et. al. on the same line



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:1491
+  llvm::Value *FlagVal = llvm::ConstantInt::get(IntTy, Flag);
+  auto emitObjSize = [&]() -> llvm::Value * {
+return evaluateOrEmitBuiltinObjectSize(CE->getArg(0), BOSType, SizeTy,

Is the explicit trailing return here needed? I would have assumed this could be 
inferred.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:1548
+  assert(Err == ASTContext::GE_None && "Should not codegen an error");
+  llvm::FunctionType *LLVMVariantTy =
+  cast(ConvertType(VariantTy));

`auto *`



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:1574
 
+  if (auto *Attr = FD->getAttr())
+return emitFortifiedStdLibCall(*this, E, BuiltinID, Attr->getType(),

`const auto *` and some other identifier than `Attr` (so it won't conflict with 
the type name).



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6437
+S.Diag(AL.getArgAsExpr(0)->getBeginLoc(),
+   diag::err_attribute_argument_outof_range)
+<< AL << 0 << 3;

lol, I'll fix that typo after you land your patch.


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] D58072: Make ModuleDependencyCollector's method virtual (NFC)

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere added a comment.

In D58072#1393640 , @bruno wrote:

> How much of the `ModuleDependencyCollector` will be reused as is by LLDB? I 
> wonder about the tradeoff versus inheriting from `DependencyCollector` 
> directly.


We reuse the `attachTo` methods, which in turn use the 
`ModuleDependencyListener`, `ModuleDependencyPPCallbacks` and 
`ModuleDependencyMMCallbacks`. Do you think it's worth re-implementing those?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58072



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

[PATCH] D57896: Variable names rule

[Jordan Rupprecht via Phabricator via cfe-commits]

rupprecht added a comment.

In D57896#1391925 , 
@hubert.reinterpretcast wrote:

> In D57896#1391611 , @zturner wrote:
>
> > Is this actually any better?  Whereas before we can’t differentiate type 
> > names and variable names, under this proposal we can’t differentiate type 
> > names and function names.  So it seems a bit of “6 of 1, half dozen of 
> > another”
>
>
> Perhaps you mistyped? The proposal does not change the status quo of either 
> type names nor function names. If you mean that we can't differentiate 
> variable names and function names, then it seems worthwhile to point out that 
> the actual letters (not just the case of said letters) matter too. Whereas 
> the guidelines state that types and variables should have names that are 
> nouns, the guidelines state that functions should have names that are verb 
> phrases.


There is still overlap, e.g. "process" can be a noun (a Linux process) or a 
verb (to process something)

I think it should also be pointed out there is not zero overhead -- it's not a 
lot (at least for native English speakers, which many LLVM developers are not), 
but determining if a word is a verb or a noun is harder than looking at the 
casing. Small, but worth observing.

A different convention, e.g. `lower_case`, avoids this. Personally, I'd prefer 
that, but I'm also fine with lowerCamelCase just so we can stop using 
UpperCamelCase.




Comment at: llvm/docs/CodingStandards.rst:1195
+  be camel case, and start with a lower case letter (e.g. ``leader`` or
+  ``boats``). It is also acceptable to use ``UpperCamelCase`` for consistency
+  with existing code.

It would be nice for this section to be expanded a bit, just to avoid 
inevitable code review churn, e.g. if I'm adding 50 lines to a 200 line file, 
am I allowed to change the existing var names elsewhere in the file or method, 
or is that outside the scope of my change? If I'm reviewing that patch, do I 
tell the author they have to be consistent and revert other changes? etc.

Is there any plan to use clang-tidy to do a global cleanup, or is this going to 
be a totally ad-hoc migration -- variables use the new scheme only when the 
code is updated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57896



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

[PATCH] D58072: Make ModuleDependencyCollector's method virtual (NFC)

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added a comment.

How much of the `ModuleDependencyCollector` will be reused as is by LLDB? I 
wonder about the tradeoff versus inheriting from `DependencyCollector` directly.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58072



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

[PATCH] D17444: PR26672: [MSVC] Clang does not recognize "static_assert" keyword in C mode

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

> Also, I suppose this is really actually an -fms-compatibility thing not 
> -fms-extensions because it takes an identifier from outside the implementer's 
> namespace.

I just verified, it really does take the identifier and it does not treat 
`static_assert` as a macro (even when you include assert.h). I suppose we 
probably should treat this as a keyword like MSVC seems to be doing, rather 
than interposing on assert.h, because the interposition would give subtly 
different resulting behavior.


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

https://reviews.llvm.org/D17444



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

[PATCH] D58072: Make ModuleDependencyCollector's method virtual (NFC)

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: bruno, vsapsai.
Herald added a project: clang.

For reproducers in LLDB we want to hook up into the existing clang 
infrastructure. To make that happen we need to be able to override the 
ModuleDependencyCollector's methods.


Repository:
  rC Clang

https://reviews.llvm.org/D58072

Files:
  clang/include/clang/Frontend/Utils.h


Index: clang/include/clang/Frontend/Utils.h
===
--- clang/include/clang/Frontend/Utils.h
+++ clang/include/clang/Frontend/Utils.h
@@ -145,18 +145,18 @@
   ~ModuleDependencyCollector() override { writeFileMap(); }
 
   StringRef getDest() { return DestDir; }
-  bool insertSeen(StringRef Filename) { return Seen.insert(Filename).second; }
-  void addFile(StringRef Filename, StringRef FileDst = {});
+  virtual bool insertSeen(StringRef Filename) { return 
Seen.insert(Filename).second; }
+  virtual void addFile(StringRef Filename, StringRef FileDst = {});
 
-  void addFileMapping(StringRef VPath, StringRef RPath) {
+  virtual void addFileMapping(StringRef VPath, StringRef RPath) {
 VFSWriter.addFileMapping(VPath, RPath);
   }
 
-  void attachToPreprocessor(Preprocessor ) override;
-  void attachToASTReader(ASTReader ) override;
+  virtual void attachToPreprocessor(Preprocessor ) override;
+  virtual void attachToASTReader(ASTReader ) override;
 
-  void writeFileMap();
-  bool hasErrors() { return HasErrors; }
+  virtual void writeFileMap();
+  virtual bool hasErrors() { return HasErrors; }
 };
 
 /// AttachDependencyGraphGen - Create a dependency graph generator, and attach


Index: clang/include/clang/Frontend/Utils.h
===
--- clang/include/clang/Frontend/Utils.h
+++ clang/include/clang/Frontend/Utils.h
@@ -145,18 +145,18 @@
   ~ModuleDependencyCollector() override { writeFileMap(); }
 
   StringRef getDest() { return DestDir; }
-  bool insertSeen(StringRef Filename) { return Seen.insert(Filename).second; }
-  void addFile(StringRef Filename, StringRef FileDst = {});
+  virtual bool insertSeen(StringRef Filename) { return Seen.insert(Filename).second; }
+  virtual void addFile(StringRef Filename, StringRef FileDst = {});
 
-  void addFileMapping(StringRef VPath, StringRef RPath) {
+  virtual void addFileMapping(StringRef VPath, StringRef RPath) {
 VFSWriter.addFileMapping(VPath, RPath);
   }
 
-  void attachToPreprocessor(Preprocessor ) override;
-  void attachToASTReader(ASTReader ) override;
+  virtual void attachToPreprocessor(Preprocessor ) override;
+  virtual void attachToASTReader(ASTReader ) override;
 
-  void writeFileMap();
-  bool hasErrors() { return HasErrors; }
+  virtual void writeFileMap();
+  virtual bool hasErrors() { return HasErrors; }
 };
 
 /// AttachDependencyGraphGen - Create a dependency graph generator, and attach
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r353729 - Attempt to pacify bots more after r353718 and r353725

[Galina Kistanova via cfe-commits]

Hello Nico,

This builders fail on your test as well -
http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/15736
, http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/4242.

Please find attached the 2 temp files you can use to reliably run against
your FileCheck patterns.
Hope this would help debugging.

Please also notice the warnings each of the RUN command produces. The
warnings should be quite easy to reproduce and address.

In the mean time, could you revert the change unless you expect the fix
coming really soon, please?
It is not a good idea to keep the bots red for long.

Here is the log:
--

C:\>c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\bin\clang.exe
-cc1 -internal-isystem
c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\lib\clang\9.0.0\include
-nostdsysteminc -ffreestanding -fms-extensions -fms-compatibility
-fms-compatibility-version=17.00 -triple i686--windows -Oz -emit-llvm
C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\CodeGen\ms-x86-intrinsics.c
-o - > \tmp-1\ms-x86-intrinsics-1.ll
C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\CodeGen\ms-x86-intrinsics.c:10:10:
warning: implicitly declaring library function '__readfsbyte' with type
'unsigned char (unsigned long)'
  return __readfsbyte(++Offset);
 ^
C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\CodeGen\ms-x86-intrinsics.c:10:10:
note: include the header  or explicitly provide a declaration for
'__readfsbyte'
C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\CodeGen\ms-x86-intrinsics.c:19:10:
warning: implicitly declaring library function '__readfsword' with type
'unsigned short (unsigned long)'
  return __readfsword(++Offset);
 ^
C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\CodeGen\ms-x86-intrinsics.c:19:10:
note: include the header  or explicitly provide a declaration for
'__readfsword'
C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\CodeGen\ms-x86-intrinsics.c:28:10:
warning: implicitly declaring library function '__readfsdword' with type
'unsigned long (unsigned long)'
  return __readfsdword(++Offset);
 ^
C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\CodeGen\ms-x86-intrinsics.c:28:10:
note: include the header  or explicitly provide a declaration for
'__readfsdword'
C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\CodeGen\ms-x86-intrinsics.c:37:10:
warning: implicitly declaring library function '__readfsqword' with type
'unsigned long long (unsigned long)'
  return __readfsqword(++Offset);
 ^
C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\CodeGen\ms-x86-intrinsics.c:37:10:
note: include the header  or explicitly provide a declaration for
'__readfsqword'
C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\CodeGen\ms-x86-intrinsics.c:47:10:
warning: implicitly declaring library function '__emul' with type 'long
long (int, int)'
  return __emul(a, b);
 ^
C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\CodeGen\ms-x86-intrinsics.c:47:10:
note: include the header  or explicitly provide a declaration for
'__emul'
C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\CodeGen\ms-x86-intrinsics.c:56:10:
warning: implicitly declaring library function '__emulu' with type
'unsigned long long (unsigned int, unsigned int)'
  return __emulu(a, b);
 ^
C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\CodeGen\ms-x86-intrinsics.c:56:10:
note: include the header  or explicitly provide a declaration for
'__emulu'
6 warnings generated.

---

C:\>c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\bin\clang.exe
-cc1 -internal-isystem
c:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.obj\lib\clang\9.0.0\include
-nostdsysteminc -ffreestanding -fms-extensions -fms-compatibility
-fms-compatibility-version=17.00  -triple x86_64--windows -Oz
-emit-llvm
C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\CodeGen\ms-x86-intrinsics.c
-o - > \tmp-1\ms-x86-intrinsics-4.ll
C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\test\CodeGen\ms-x86-intrinsics.c:47:10:
warning: implicitly declaring library function '__emul' with type 'long
long (int, int)'
  return __emul(a, b);
 

[PATCH] D54978: Move the SMT API to LLVM

[Brian Rzycki via Phabricator via cfe-commits]

brzycki added a comment.

In D54978#1392464 , @Szelethus wrote:

> Shouldn't that be off by default?


The default for `LLVM_ENABLE_Z3_SOLVER` depends entirely on what CMake detects 
from `find_package()`. Here is the relevant code in `llvm/CMakeLists.txt`:

  find_package(Z3 4.7.1)
  ...
  set(LLVM_ENABLE_Z3_SOLVER_DEFAULT "${Z3_FOUND}")
  
  option(LLVM_ENABLE_Z3_SOLVER
"Enable Support for the Z3 constraint solver in LLVM."
${LLVM_ENABLE_Z3_SOLVER_DEFAULT}
  )

If `find_package()` idenfiies a suitable `Z3_FOUND` the default is to enable 
builds with the Z3 framework. In other words the entire Z3 feature is manual 
opt-out and implicit opt-in when CMake thinks a suitable Z3 library is found.

In D54978#1393552 , @ddcc wrote:

> The likely reason for this versioning problem is that the current versioning 
> implementation in FindZ3.cmake is best-effort only.


Thank you @ddcc for this explanation. If that's the case I'd really prefer if 
`LLVM_ENABLE_Z3_SOLVER` was explicit opt-in and defaulted to `OFF` regardless 
of what `find_package` returned.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54978



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

[PATCH] D57655: clang-format with UseTab: Always sometimes doesn't insert the right amount of tabs.

[Hylke Kleve via Phabricator via cfe-commits]

hyklv added a comment.

In D57655#1382437 , @lebedev.ri wrote:

> Test?
>  Is this a fix, or a new formatting style?


a fix.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57655



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

[PATCH] D57655: clang-format with UseTab: Always sometimes doesn't insert the right amount of tabs.

[Hylke Kleve via Phabricator via cfe-commits]

hyklv added a comment.

In D57655#1382855 , @MyDeveloperDay 
wrote:

> The reviewers set a high barrier of entry, you need to at least start by 
> adding some unit tests (or someone will break your code in the future), if 
> you think it was a bug it might be worth logging that in bugzilla too!


thanks for pointing me in the right direction! i added a test.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57655



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

[PATCH] D57655: clang-format with UseTab: Always sometimes doesn't insert the right amount of tabs.

[Hylke Kleve via Phabricator via cfe-commits]

hyklv updated this revision to Diff 186315.
hyklv marked an inline comment as done.
hyklv added a comment.

I removed accidental changes in the copyright notice, updated the code to 
insert one space instead of one tab. Added a test case for the alignment.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57655

Files:
  D:/Format/llvm/tools/clang/lib/Format/WhitespaceManager.cpp
  D:/Format/llvm/tools/clang/unittests/Format/FormatTest.cpp


Index: D:/Format/llvm/tools/clang/unittests/Format/FormatTest.cpp
===
--- D:/Format/llvm/tools/clang/unittests/Format/FormatTest.cpp
+++ D:/Format/llvm/tools/clang/unittests/Format/FormatTest.cpp
@@ -8739,6 +8739,9 @@
"\t\tparameter2); \\\n"
"\t}",
Tab);
+  verifyFormat("int a;\t  // x\n"
+   "int ; // x\n",
+   Tab);
 
   Tab.TabWidth = 4;
   Tab.IndentWidth = 8;
Index: D:/Format/llvm/tools/clang/lib/Format/WhitespaceManager.cpp
===
--- D:/Format/llvm/tools/clang/lib/Format/WhitespaceManager.cpp
+++ D:/Format/llvm/tools/clang/lib/Format/WhitespaceManager.cpp
@@ -679,13 +679,17 @@
   case FormatStyle::UT_Always: {
 unsigned FirstTabWidth =
 Style.TabWidth - WhitespaceStartColumn % Style.TabWidth;
-// Indent with tabs only when there's at least one full tab.
-if (FirstTabWidth + Style.TabWidth <= Spaces) {
+// Insert only spaces when we want to end up before the next tab.
+if (Spaces < FirstTabWidth || Spaces == 1) {
+  Text.append(Spaces, ' ');
+} else {
+  // Align to next tab.
   Spaces -= FirstTabWidth;
   Text.append("\t");
+
+  Text.append(Spaces / Style.TabWidth, '\t');
+  Text.append(Spaces % Style.TabWidth, ' ');
 }
-Text.append(Spaces / Style.TabWidth, '\t');
-Text.append(Spaces % Style.TabWidth, ' ');
 break;
   }
   case FormatStyle::UT_ForIndentation:


Index: D:/Format/llvm/tools/clang/unittests/Format/FormatTest.cpp
===
--- D:/Format/llvm/tools/clang/unittests/Format/FormatTest.cpp
+++ D:/Format/llvm/tools/clang/unittests/Format/FormatTest.cpp
@@ -8739,6 +8739,9 @@
"\t\tparameter2); \\\n"
"\t}",
Tab);
+  verifyFormat("int a;\t  // x\n"
+   "int ; // x\n",
+   Tab);
 
   Tab.TabWidth = 4;
   Tab.IndentWidth = 8;
Index: D:/Format/llvm/tools/clang/lib/Format/WhitespaceManager.cpp
===
--- D:/Format/llvm/tools/clang/lib/Format/WhitespaceManager.cpp
+++ D:/Format/llvm/tools/clang/lib/Format/WhitespaceManager.cpp
@@ -679,13 +679,17 @@
   case FormatStyle::UT_Always: {
 unsigned FirstTabWidth =
 Style.TabWidth - WhitespaceStartColumn % Style.TabWidth;
-// Indent with tabs only when there's at least one full tab.
-if (FirstTabWidth + Style.TabWidth <= Spaces) {
+// Insert only spaces when we want to end up before the next tab.
+if (Spaces < FirstTabWidth || Spaces == 1) {
+  Text.append(Spaces, ' ');
+} else {
+  // Align to next tab.
   Spaces -= FirstTabWidth;
   Text.append("\t");
+
+  Text.append(Spaces / Style.TabWidth, '\t');
+  Text.append(Spaces % Style.TabWidth, ' ');
 }
-Text.append(Spaces / Style.TabWidth, '\t');
-Text.append(Spaces % Style.TabWidth, ' ');
 break;
   }
   case FormatStyle::UT_ForIndentation:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D17444: PR26672: [MSVC] Clang does not recognize "static_assert" keyword in C mode

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In D17444#1393509 , @thakis wrote:

> Given that we already have interposition headers and users already have to 
> hold the compiler right, does it matter if we have one more interposing 
> header?


I'd say it's one more bit of technical debt that we'll have to figure out how 
to undo later. I'm OK doing it if @rsmith still feels that we shouldn't do this 
in the compiler.

Also, I suppose this is really actually an -fms-compatibility thing not 
-fms-extensions because it takes an identifier from outside the implementer's 
namespace.


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

https://reviews.llvm.org/D17444



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

[PATCH] D49863: [istream] Fix error flags and exceptions propagated from input stream operations

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.
Herald added a subscriber: jkorous.

I would like to merge this even though http://wg21.link/p1264 has not been 
voted into C++ yet, because this is arguably a bug and libc++ differs from 
other stdlibs in this regard. Once http://wg21.link/p1264 is merged into C++ in 
one way of another, libc++ will simply have nothing to do.

@mclow.lists Are you OK with that?


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D49863



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

[PATCH] D54978: Move the SMT API to LLVM

[Dominic Chen via Phabricator via cfe-commits]

ddcc added subscribers: delcypher, ddcc.
ddcc added a comment.

The likely reason for this versioning problem is that the current versioning 
implementation in FindZ3.cmake is best-effort only: among other conditions, if 
the z3 binary is available, it will execute it and parse out the version number 
from standard output, otherwise, it fails silently. This is because upstream Z3 
doesn't define the API version in a header file, and uses a homebrew 
python-based build system that also doesn't export the version. I believe 
@delcypher 's CMake-based build system for upstream Z3 might solve this 
problem, but I haven't looked at it in a long time, and it it appears to be 
stalled ( https://github.com/Z3Prover/z3/issues/986 ).

I also agree that more notice about this patch would have been appreciated; I 
didn't hear it until I read LLVM weekly today.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54978



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

[PATCH] D58069: [Sema] Mark GNU compound literal array init as an rvalue.

[Eli Friedman via Phabricator via cfe-commits]

efriedma created this revision.
efriedma added a reviewer: rsmith.
Herald added a project: clang.

Basically the same issue as string init, except it didn't really have any 
visible consequences before I removed the implicit lvalue-to-rvalue conversion 
from CodeGen.

While I'm here, a couple minor drive-by cleanups: IgnoreParens never returns a 
ConstantExpr, and there was a potential crash with string init involving a 
ChooseExpr.

The analyzer test change maybe indicates we could simplify the analyzer code a 
little with this fix?  Apparently a hack was added to support lvalues in 
initializers in r315750, but I'm not really familiar with the relevant code.

Fixes regression reported in the kernel build at 
https://bugs.llvm.org/show_bug.cgi?id=40430#c6 .


Repository:
  rC Clang

https://reviews.llvm.org/D58069

Files:
  lib/Sema/SemaInit.cpp
  test/Analysis/compound-literals.c
  test/CodeGen/compound-literal.c


Index: test/CodeGen/compound-literal.c
===
--- test/CodeGen/compound-literal.c
+++ test/CodeGen/compound-literal.c
@@ -11,6 +11,11 @@
 typedef int v4i32 __attribute((vector_size(16)));
 v4i32 *y = &(v4i32){1,2,3,4};
 
+// Check generated code for GNU constant array init from compound literal,
+// for a global variable.
+// CHECK: @compound_array = global [8 x i32] [i32 1, i32 2, i32 3, i32 4, i32 
5, i32 6, i32 7, i32 8]
+int compound_array[] = __extension__(__builtin_choose_expr(0, 0, _Generic(1, 
int: (int[]){1, 2, 3, 4, 5, 6, 7, 8})));
+
 void xxx() {
 int* a = &(int){1};
 struct s {int a, b, c;} * b = &(struct s) {1, 2, 3};
@@ -82,3 +87,13 @@
   const void *b = MyCLH;
   return a == b;
 }
+
+// Check generated code for GNU constant array init from compound literal,
+// for a local variable.
+// CHECK-LABEL: define i32 @compound_array_fn()
+// CHECK: [[COMPOUND_ARRAY:%.*]] = alloca [8 x i32]
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}}, i64 32, i1 false)
+int compound_array_fn() {
+  int compound_array[] = (int[]){1,2,3,4,5,6,7,8};
+  return compound_array[0];
+}
Index: test/Analysis/compound-literals.c
===
--- test/Analysis/compound-literals.c
+++ test/Analysis/compound-literals.c
@@ -4,6 +4,5 @@
 // pr28449: Used to crash.
 void foo(void) {
   static const unsigned short array[] = (const unsigned short[]){0x0F00};
-  // FIXME: Should be true.
-  clang_analyzer_eval(array[0] == 0x0F00); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(array[0] == 0x0F00); // expected-warning{{TRUE}}
 }
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -153,11 +153,33 @@
   E = UO->getSubExpr();
 else if (GenericSelectionExpr *GSE = dyn_cast(E))
   E = GSE->getResultExpr();
+else if (ChooseExpr *CE = dyn_cast(E))
+  E = CE->getChosenSubExpr();
 else
   llvm_unreachable("unexpected expr in string literal init");
   }
 }
 
+/// Fix a compound literal initializing an array so it's correctly marked
+/// as an rvalue.
+static void updateGNUCompoundLiteralRValue(Expr *E) {
+  while (true) {
+E->setValueKind(VK_RValue);
+if (isa(E))
+  break;
+else if (ParenExpr *PE = dyn_cast(E))
+  E = PE->getSubExpr();
+else if (UnaryOperator *UO = dyn_cast(E))
+  E = UO->getSubExpr();
+else if (GenericSelectionExpr *GSE = dyn_cast(E))
+  E = GSE->getResultExpr();
+else if (ChooseExpr *CE = dyn_cast(E))
+  E = CE->getChosenSubExpr();
+else
+  llvm_unreachable("unexpected expr in array compound literal init");
+  }
+}
+
 static void CheckStringInit(Expr *Str, QualType , const ArrayType *AT,
 Sema ) {
   // Get the length of the string as parsed.
@@ -5542,8 +5564,7 @@
 // array from a compound literal that creates an array of the same
 // type, so long as the initializer has no side effects.
 if (!S.getLangOpts().CPlusPlus && Initializer &&
-(isa(Initializer->IgnoreParens()) ||
- isa(Initializer->IgnoreParens())) &&
+isa(Initializer->IgnoreParens()) &&
 Initializer->getType()->isArrayType()) {
   const ArrayType *SourceAT
 = Context.getAsArrayType(Initializer->getType());
@@ -7956,6 +7977,7 @@
   S.Diag(Kind.getLocation(), diag::ext_array_init_copy)
 << Step->Type << CurInit.get()->getType()
 << CurInit.get()->getSourceRange();
+  updateGNUCompoundLiteralRValue(CurInit.get());
   LLVM_FALLTHROUGH;
 case SK_ArrayInit:
   // If the destination type is an incomplete array type, update the


Index: test/CodeGen/compound-literal.c
===
--- test/CodeGen/compound-literal.c
+++ test/CodeGen/compound-literal.c
@@ -11,6 +11,11 @@
 typedef int v4i32 __attribute((vector_size(16)));
 v4i32 *y = &(v4i32){1,2,3,4};
 
+// Check generated code for 

[PATCH] D57915: [COFF, ARM64] Remove definitions for _byteswap library functions

[Tom Tan via Phabricator via cfe-commits]

TomTan added a comment.

In D57915#1393510 , @thakis wrote:

> In D57915#1389722 , @TomTan wrote:
>
> > In D57915#1389560 , @lebedev.ri 
> > wrote:
> >
> > > In D57915#1389549 , @TomTan 
> > > wrote:
> > >
> > > > Added the tests back. Clang IR should not lower these to bswap calls 
> > > > because they are global library functions. It might be slower to make 
> > > > the call to library function than bswap, but this is the same for other 
> > > > architectures supported by Windows. And just redefine global library 
> > > > function triggers link error in some scenarios.
> > >
> > >
> > > I think there is some deeper reasoning is being omitted here.
> > >  //Why// does the fact that those are "global library functions" bans 
> > > clang from lowering them into IR?
> > >  (and thus, "prevents" LLVM form doing optimizations)
> >
> >
> > The current issue could be caused by the implementation of comdat selection 
> > in LLD as below which provides more strict conflict check than MSVC link 
> > does.
>
>
> lld isn't supposed to be more strict than link.exe. lld used to not implement 
> the comdat selection field until recently, so lld got more strict – but it 
> should've gotten only as strict as link.exe, not moreso. Do you have an 
> example where lld is more strict than link.exe?


My previous reply is LLD comdat linking issue was misleading. The current issue 
is that when function declaration (`extern`) and definition (`static inline`) 
shown up in sequence, the function declaration property `extern` wins over 
`static` in the resulting COMDAT, which causes linking issue for both LLD and 
link.exe. But this issue was just exposed by the recent addition of duplicated 
symbols check in LLD.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57915



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

[PATCH] D58061: Fix a few tests that were missing ':' on CHECK lines and weren't testing anything.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC353744: Fix a few tests that were missing : on 
CHECK lines and werent testing… (authored by nico, committed by ).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D58061?vs=186273=186312#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D58061

Files:
  test/AST/ast-dump-attr.cpp
  test/CodeGenCXX/arm-swiftcall.cpp
  test/CodeGenCXX/cxx1z-init-statement.cpp
  test/CodeGenCXX/cxx2a-compare.cpp
  test/CodeGenCXX/debug-info-inheriting-constructor.cpp
  test/CodeGenCXX/vtable-layout.cpp
  test/CodeGenObjC/gnu-init.m
  test/CodeGenObjC/os_log.m

Index: test/AST/ast-dump-attr.cpp
===
--- test/AST/ast-dump-attr.cpp
+++ test/AST/ast-dump-attr.cpp
@@ -197,7 +197,7 @@
   [[gsl::suppress("on-decl")]]
   void TestSuppressFunction();
   // CHECK: FunctionDecl{{.*}} TestSuppressFunction
-  // CHECK-NEXT SuppressAttr{{.*}} on-decl
+  // CHECK-NEXT: SuppressAttr{{.*}} on-decl
 
   void f() {
   int *i;
Index: test/CodeGenObjC/gnu-init.m
===
--- test/CodeGenObjC/gnu-init.m
+++ test/CodeGenObjC/gnu-init.m
@@ -14,7 +14,7 @@
 
 // Check that we get a class ref to the defined class.
 // CHECK-NEW: @._OBJC_INIT_CLASS_X = global 
-// CHECK-NEW-SAME* @._OBJC_CLASS_X, section "__objc_classes"
+// CHECK-NEW-SAME: @._OBJC_CLASS_X, section "__objc_classes"
 
 // Check that we emit the section start and end symbols as hidden globals.
 // CHECK-NEW: @__start___objc_selectors = external hidden global i8*
Index: test/CodeGenObjC/os_log.m
===
--- test/CodeGenObjC/os_log.m
+++ test/CodeGenObjC/os_log.m
@@ -50,7 +50,7 @@
   // CHECK-O0: %[[V4:.*]] = ptrtoint %[[TY0]]* %[[V3]] to i64
   // CHECK-O0: call void @__os_log_helper_1_2_1_8_64(i8* %[[V0]], i64 %[[V4]])
   // CHECK-O0: %[[V5:.*]] = bitcast %[[TY0]]* %[[V3]] to i8*
-  // CHECK-O0-NOT call void (...) @llvm.objc.clang.arc.use({{.*}}
+  // CHECK-O0-NOT: call void (...) @llvm.objc.clang.arc.use({{.*}}
   // CHECK-O0: call void @llvm.objc.release(i8* %[[V5]])
   // CHECK-O0: ret i8* %[[V0]]
 }
Index: test/CodeGenCXX/cxx2a-compare.cpp
===
--- test/CodeGenCXX/cxx2a-compare.cpp
+++ test/CodeGenCXX/cxx2a-compare.cpp
@@ -80,7 +80,7 @@
   // CHECK: %cmp.ptr.null = icmp eq [[TY]] %lhs.memptr.ptr, 0
   // CHECK: %cmp.adj = icmp eq [[TY]] %lhs.memptr.adj, %rhs.memptr.adj
   // CHECK: %[[OR:.*]] = or i1
-  // CHECK-SAME %cmp.adj
+  // CHECK-SAME: %cmp.adj
   // CHECK: %memptr.eq = and i1 %cmp.ptr, %[[OR]]
   // CHECK: %sel.eq = select i1 %memptr.eq, i8 [[EQ]], i8 [[NE]]
   // CHECK: %__value_ = getelementptr inbounds %[[SE]], %[[SE]]* %[[DEST]]
Index: test/CodeGenCXX/cxx1z-init-statement.cpp
===
--- test/CodeGenCXX/cxx1z-init-statement.cpp
+++ test/CodeGenCXX/cxx1z-init-statement.cpp
@@ -5,7 +5,7 @@
   // CHECK:  %[[A:.*]] = alloca i32, align 4
   // CHECK-NEXT: store i32 5, i32* %[[A]], align 4
   // CHECK-NEXT: %[[B:.*]] = load i32, i32* %[[A]], align 4
-  // CHECK-NEXT  %[[C:.*]] = icmp slt i32 %[[B]], 8
+  // CHECK-NEXT: %[[C:.*]] = icmp slt i32 %[[B]], 8
   if (int a = 5; a < 8)
 ;
 }
Index: test/CodeGenCXX/arm-swiftcall.cpp
===
--- test/CodeGenCXX/arm-swiftcall.cpp
+++ test/CodeGenCXX/arm-swiftcall.cpp
@@ -120,6 +120,6 @@
 class struct_trivial {
   int x;
 };
-// CHECK-LABEL define void @test_struct_trivial(i32{{( %.*)?}})
+// CHECK-LABEL: define swiftcc void @test_struct_trivial(i32{{( %.*)?}})
 extern "C" SWIFTCALL
 void test_struct_trivial(struct_trivial triv) {}
Index: test/CodeGenCXX/vtable-layout.cpp
===
--- test/CodeGenCXX/vtable-layout.cpp
+++ test/CodeGenCXX/vtable-layout.cpp
@@ -1412,7 +1412,7 @@
 // CHECK-35-NEXT:   13 | void Test28::B::b()
 //
 // CHECK-35:  VTable indices for 'Test28::E' (1 entries).
-// CHECK-35-NEXT :   0 | void Test28::E::e()
+// CHECK-35-NEXT:0 | void Test28::E::e()
 
 // CHECK-35:  Construction vtable for ('Test28::D', 0) in 'Test28::E' (13 entries).
 // CHECK-35-NEXT:0 | vbase_offset (8)
Index: test/CodeGenCXX/debug-info-inheriting-constructor.cpp
===
--- test/CodeGenCXX/debug-info-inheriting-constructor.cpp
+++ test/CodeGenCXX/debug-info-inheriting-constructor.cpp
@@ -9,9 +9,9 @@
 
 A::A(int i, ...) {}
 // CHECK: define void @{{.*}}foo
-// CHECK-NOT ret void
+// CHECK-NOT: ret void
 // CHECK: call void @llvm.dbg.declare
-// CHECK-NOT ret void
+// CHECK-NOT: ret void
 // CHECK: call void @llvm.dbg.declare(metadata %{{.*}}** 

[PATCH] D57874: [WebAssembly] Make thread-related options consistent

[Heejin Ahn via Phabricator via cfe-commits]

aheejin added a comment.

If people have opinions on this final version, please let me know.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874



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

r353744 - Fix a few tests that were missing ':' on CHECK lines and weren't testing anything.

[Nico Weber via cfe-commits]

Author: nico
Date: Mon Feb 11 12:33:22 2019
New Revision: 353744

URL: http://llvm.org/viewvc/llvm-project?rev=353744=rev
Log:
Fix a few tests that were missing ':' on CHECK lines and weren't testing 
anything.

Found by `git grep '\/\/ CHECK-[^: ]* ' clang/test/ | grep -v RUN:`.

Also tweak CodeGenCXX/arm-swiftcall.cpp to still pass now that it checks more.

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

Modified:
cfe/trunk/test/AST/ast-dump-attr.cpp
cfe/trunk/test/CodeGenCXX/arm-swiftcall.cpp
cfe/trunk/test/CodeGenCXX/cxx1z-init-statement.cpp
cfe/trunk/test/CodeGenCXX/cxx2a-compare.cpp
cfe/trunk/test/CodeGenCXX/debug-info-inheriting-constructor.cpp
cfe/trunk/test/CodeGenCXX/vtable-layout.cpp
cfe/trunk/test/CodeGenObjC/gnu-init.m
cfe/trunk/test/CodeGenObjC/os_log.m

Modified: cfe/trunk/test/AST/ast-dump-attr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/AST/ast-dump-attr.cpp?rev=353744=353743=353744=diff
==
--- cfe/trunk/test/AST/ast-dump-attr.cpp (original)
+++ cfe/trunk/test/AST/ast-dump-attr.cpp Mon Feb 11 12:33:22 2019
@@ -197,7 +197,7 @@ namespace TestSuppress {
   [[gsl::suppress("on-decl")]]
   void TestSuppressFunction();
   // CHECK: FunctionDecl{{.*}} TestSuppressFunction
-  // CHECK-NEXT SuppressAttr{{.*}} on-decl
+  // CHECK-NEXT: SuppressAttr{{.*}} on-decl
 
   void f() {
   int *i;

Modified: cfe/trunk/test/CodeGenCXX/arm-swiftcall.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/arm-swiftcall.cpp?rev=353744=353743=353744=diff
==
--- cfe/trunk/test/CodeGenCXX/arm-swiftcall.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/arm-swiftcall.cpp Mon Feb 11 12:33:22 2019
@@ -120,6 +120,6 @@ TEST(struct_indirect_1)
 class struct_trivial {
   int x;
 };
-// CHECK-LABEL define void @test_struct_trivial(i32{{( %.*)?}})
+// CHECK-LABEL: define swiftcc void @test_struct_trivial(i32{{( %.*)?}})
 extern "C" SWIFTCALL
 void test_struct_trivial(struct_trivial triv) {}

Modified: cfe/trunk/test/CodeGenCXX/cxx1z-init-statement.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/cxx1z-init-statement.cpp?rev=353744=353743=353744=diff
==
--- cfe/trunk/test/CodeGenCXX/cxx1z-init-statement.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/cxx1z-init-statement.cpp Mon Feb 11 12:33:22 2019
@@ -5,7 +5,7 @@ void f() {
   // CHECK:  %[[A:.*]] = alloca i32, align 4
   // CHECK-NEXT: store i32 5, i32* %[[A]], align 4
   // CHECK-NEXT: %[[B:.*]] = load i32, i32* %[[A]], align 4
-  // CHECK-NEXT  %[[C:.*]] = icmp slt i32 %[[B]], 8
+  // CHECK-NEXT: %[[C:.*]] = icmp slt i32 %[[B]], 8
   if (int a = 5; a < 8)
 ;
 }

Modified: cfe/trunk/test/CodeGenCXX/cxx2a-compare.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/cxx2a-compare.cpp?rev=353744=353743=353744=diff
==
--- cfe/trunk/test/CodeGenCXX/cxx2a-compare.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/cxx2a-compare.cpp Mon Feb 11 12:33:22 2019
@@ -80,7 +80,7 @@ auto mem_ptr_test(MemPtrT x, MemPtrT y)
   // CHECK: %cmp.ptr.null = icmp eq [[TY]] %lhs.memptr.ptr, 0
   // CHECK: %cmp.adj = icmp eq [[TY]] %lhs.memptr.adj, %rhs.memptr.adj
   // CHECK: %[[OR:.*]] = or i1
-  // CHECK-SAME %cmp.adj
+  // CHECK-SAME: %cmp.adj
   // CHECK: %memptr.eq = and i1 %cmp.ptr, %[[OR]]
   // CHECK: %sel.eq = select i1 %memptr.eq, i8 [[EQ]], i8 [[NE]]
   // CHECK: %__value_ = getelementptr inbounds %[[SE]], %[[SE]]* %[[DEST]]

Modified: cfe/trunk/test/CodeGenCXX/debug-info-inheriting-constructor.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-inheriting-constructor.cpp?rev=353744=353743=353744=diff
==
--- cfe/trunk/test/CodeGenCXX/debug-info-inheriting-constructor.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/debug-info-inheriting-constructor.cpp Mon Feb 11 
12:33:22 2019
@@ -9,9 +9,9 @@ struct B : A {
 
 A::A(int i, ...) {}
 // CHECK: define void @{{.*}}foo
-// CHECK-NOT ret void
+// CHECK-NOT: ret void
 // CHECK: call void @llvm.dbg.declare
-// CHECK-NOT ret void
+// CHECK-NOT: ret void
 // CHECK: call void @llvm.dbg.declare(metadata %{{.*}}** %{{[^,]+}},
 // CHECK-SAME: metadata ![[THIS:[0-9]+]], metadata !DIExpression()), !dbg 
![[LOC:[0-9]+]]
 // CHECK: ret void, !dbg ![[NOINL:[0-9]+]]

Modified: cfe/trunk/test/CodeGenCXX/vtable-layout.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/vtable-layout.cpp?rev=353744=353743=353744=diff
==
--- cfe/trunk/test/CodeGenCXX/vtable-layout.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/vtable-layout.cpp Mon Feb 11 12:33:22 2019
@@ 

[PATCH] D58061: Fix a few tests that were missing ':' on CHECK lines and weren't testing anything.

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

There has been some progress recently on better FileCheck diagnosis of likely 
test-writing issues, although to date it mostly requires the human to ask "what 
is going on here?" explicitly.  I can see adding a check to detect the 
missing-colon-on-otherwise-valid-directive case (including the usual suffixes), 
and my guess is that isn't too likely to cause excessive churn.  The deeper we 
go into typo-detection the more likely we are to trip over false positives, but 
this small step seems like a worthy change.
Happy to review such a patch, or somebody can file a PR and CC me.


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

https://reviews.llvm.org/D58061



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

[PATCH] D58010: [CodeGen] Set construction vtable visibility after creating initializer

[Petr Hosek via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL353742: [CodeGen] Set construction vtable visibility after 
creating initializer (authored by phosek, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58010?vs=186142=186308#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58010

Files:
  cfe/trunk/lib/CodeGen/CGVTables.cpp
  cfe/trunk/test/CodeGen/construction-vtable-visibility.cpp


Index: cfe/trunk/test/CodeGen/construction-vtable-visibility.cpp
===
--- cfe/trunk/test/CodeGen/construction-vtable-visibility.cpp
+++ cfe/trunk/test/CodeGen/construction-vtable-visibility.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple x86_64-linux-unknown -fvisibility hidden -emit-llvm 
%s -o - | FileCheck %s
+
+struct Base {};
+
+class Parent1 : virtual public Base {};
+
+class Parent2 : virtual public Base {};
+
+class Child : public Parent1, public Parent2 {};
+
+void test() {
+  Child x;
+}
+
+// CHECK: @_ZTC5Child0_7Parent1 = linkonce_odr hidden unnamed_addr constant
+// CHECK: @_ZTC5Child8_7Parent2 = linkonce_odr hidden unnamed_addr constant
Index: cfe/trunk/lib/CodeGen/CGVTables.cpp
===
--- cfe/trunk/lib/CodeGen/CGVTables.cpp
+++ cfe/trunk/lib/CodeGen/CGVTables.cpp
@@ -761,7 +761,6 @@
   // Create the variable that will hold the construction vtable.
   llvm::GlobalVariable *VTable =
   CGM.CreateOrReplaceCXXRuntimeVariable(Name, VTType, Linkage, Align);
-  CGM.setGVProperties(VTable, RD);
 
   // V-tables are always unnamed_addr.
   VTable->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
@@ -775,6 +774,11 @@
   createVTableInitializer(components, *VTLayout, RTTI);
   components.finishAndSetAsInitializer(VTable);
 
+  // Set properties only after the initializer has been set to ensure that the
+  // GV is treated as definition and not declaration.
+  assert(!VTable->isDeclaration() && "Shouldn't set properties on 
declaration");
+  CGM.setGVProperties(VTable, RD);
+
   CGM.EmitVTableTypeMetadata(VTable, *VTLayout.get());
 
   return VTable;


Index: cfe/trunk/test/CodeGen/construction-vtable-visibility.cpp
===
--- cfe/trunk/test/CodeGen/construction-vtable-visibility.cpp
+++ cfe/trunk/test/CodeGen/construction-vtable-visibility.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple x86_64-linux-unknown -fvisibility hidden -emit-llvm %s -o - | FileCheck %s
+
+struct Base {};
+
+class Parent1 : virtual public Base {};
+
+class Parent2 : virtual public Base {};
+
+class Child : public Parent1, public Parent2 {};
+
+void test() {
+  Child x;
+}
+
+// CHECK: @_ZTC5Child0_7Parent1 = linkonce_odr hidden unnamed_addr constant
+// CHECK: @_ZTC5Child8_7Parent2 = linkonce_odr hidden unnamed_addr constant
Index: cfe/trunk/lib/CodeGen/CGVTables.cpp
===
--- cfe/trunk/lib/CodeGen/CGVTables.cpp
+++ cfe/trunk/lib/CodeGen/CGVTables.cpp
@@ -761,7 +761,6 @@
   // Create the variable that will hold the construction vtable.
   llvm::GlobalVariable *VTable =
   CGM.CreateOrReplaceCXXRuntimeVariable(Name, VTType, Linkage, Align);
-  CGM.setGVProperties(VTable, RD);
 
   // V-tables are always unnamed_addr.
   VTable->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
@@ -775,6 +774,11 @@
   createVTableInitializer(components, *VTLayout, RTTI);
   components.finishAndSetAsInitializer(VTable);
 
+  // Set properties only after the initializer has been set to ensure that the
+  // GV is treated as definition and not declaration.
+  assert(!VTable->isDeclaration() && "Shouldn't set properties on declaration");
+  CGM.setGVProperties(VTable, RD);
+
   CGM.EmitVTableTypeMetadata(VTable, *VTLayout.get());
 
   return VTable;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57915: [COFF, ARM64] Remove definitions for _byteswap library functions

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

In D57915#1389722 , @TomTan wrote:

> In D57915#1389560 , @lebedev.ri 
> wrote:
>
> > In D57915#1389549 , @TomTan wrote:
> >
> > > Added the tests back. Clang IR should not lower these to bswap calls 
> > > because they are global library functions. It might be slower to make the 
> > > call to library function than bswap, but this is the same for other 
> > > architectures supported by Windows. And just redefine global library 
> > > function triggers link error in some scenarios.
> >
> >
> > I think there is some deeper reasoning is being omitted here.
> >  //Why// does the fact that those are "global library functions" bans clang 
> > from lowering them into IR?
> >  (and thus, "prevents" LLVM form doing optimizations)
>
>
> The current issue could be caused by the implementation of comdat selection 
> in LLD as below which provides more strict conflict check than MSVC link does.


lld isn't supposed to be more strict than link.exe. lld used to not implement 
the comdat selection field until recently, so lld got more strict – but it 
should've gotten only as strict as link.exe, not moreso. Do you have an example 
where lld is more strict than link.exe?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57915



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

r353742 - [CodeGen] Set construction vtable visibility after creating initializer

[Petr Hosek via cfe-commits]

Author: phosek
Date: Mon Feb 11 12:13:42 2019
New Revision: 353742

URL: http://llvm.org/viewvc/llvm-project?rev=353742=rev
Log:
[CodeGen] Set construction vtable visibility after creating initializer

We must only set the construction vtable visibility after we create the
vtable initializer, otherwise the global value will be treated as
declaration rather than definition and the visibility won't be set.

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

Added:
cfe/trunk/test/CodeGen/construction-vtable-visibility.cpp
Modified:
cfe/trunk/lib/CodeGen/CGVTables.cpp

Modified: cfe/trunk/lib/CodeGen/CGVTables.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTables.cpp?rev=353742=353741=353742=diff
==
--- cfe/trunk/lib/CodeGen/CGVTables.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGVTables.cpp Mon Feb 11 12:13:42 2019
@@ -761,7 +761,6 @@ CodeGenVTables::GenerateConstructionVTab
   // Create the variable that will hold the construction vtable.
   llvm::GlobalVariable *VTable =
   CGM.CreateOrReplaceCXXRuntimeVariable(Name, VTType, Linkage, Align);
-  CGM.setGVProperties(VTable, RD);
 
   // V-tables are always unnamed_addr.
   VTable->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
@@ -775,6 +774,11 @@ CodeGenVTables::GenerateConstructionVTab
   createVTableInitializer(components, *VTLayout, RTTI);
   components.finishAndSetAsInitializer(VTable);
 
+  // Set properties only after the initializer has been set to ensure that the
+  // GV is treated as definition and not declaration.
+  assert(!VTable->isDeclaration() && "Shouldn't set properties on 
declaration");
+  CGM.setGVProperties(VTable, RD);
+
   CGM.EmitVTableTypeMetadata(VTable, *VTLayout.get());
 
   return VTable;

Added: cfe/trunk/test/CodeGen/construction-vtable-visibility.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/construction-vtable-visibility.cpp?rev=353742=auto
==
--- cfe/trunk/test/CodeGen/construction-vtable-visibility.cpp (added)
+++ cfe/trunk/test/CodeGen/construction-vtable-visibility.cpp Mon Feb 11 
12:13:42 2019
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple x86_64-linux-unknown -fvisibility hidden -emit-llvm 
%s -o - | FileCheck %s
+
+struct Base {};
+
+class Parent1 : virtual public Base {};
+
+class Parent2 : virtual public Base {};
+
+class Child : public Parent1, public Parent2 {};
+
+void test() {
+  Child x;
+}
+
+// CHECK: @_ZTC5Child0_7Parent1 = linkonce_odr hidden unnamed_addr constant
+// CHECK: @_ZTC5Child8_7Parent2 = linkonce_odr hidden unnamed_addr constant


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

Re: r353729 - Attempt to pacify bots more after r353718 and r353725

[Nico Weber via cfe-commits]

This still didn't help.

I can't repro the failures onhttp://
lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast
, I tried Rel+Asserts and Rel-Asserts builds. Can anyone else repro this?

On Mon, Feb 11, 2019 at 1:01 PM Nico Weber via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: nico
> Date: Mon Feb 11 10:01:27 2019
> New Revision: 353729
>
> URL: http://llvm.org/viewvc/llvm-project?rev=353729=rev
> Log:
> Attempt to pacify bots more after r353718 and r353725
>
> Modified:
> cfe/trunk/test/CodeGen/ms-x86-intrinsics.c
>
> Modified: cfe/trunk/test/CodeGen/ms-x86-intrinsics.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms-x86-intrinsics.c?rev=353729=353728=353729=diff
>
> ==
> --- cfe/trunk/test/CodeGen/ms-x86-intrinsics.c (original)
> +++ cfe/trunk/test/CodeGen/ms-x86-intrinsics.c Mon Feb 11 10:01:27 2019
> @@ -145,9 +145,9 @@ unsigned __int64 test__shiftleft128(unsi
>  }
>  // CHECK-X64-LABEL: define dso_local i64 @test__shiftleft128(i64 %l, i64
> %h, i8 %d)
>  // CHECK-X64:  = zext i64 %{{.*}} to i128
> -// CHECK-X64:  = shl nuw i128 %0, 64
> +// CHECK-X64:  = shl nuw i128 %{{.*}}, 64
>  // CHECK-X64:  = zext i64 %{{.*}} to i128
> -// CHECK-X64:  = or i128 %{{.*}}, %{{.*}}
> +// CHECK-X64:  = or i128 %
>  // CHECK-X64:  = and i8 %{{.*}}, 63
>  // CHECK-X64:  = shl i128 %
>  // CHECK-X64:  = lshr i128 %
> @@ -160,7 +160,7 @@ unsigned __int64 test__shiftright128(uns
>  }
>  // CHECK-X64-LABEL: define dso_local i64 @test__shiftright128(i64 %l, i64
> %h, i8 %d)
>  // CHECK-X64:  = zext i64 %{{.*}} to i128
> -// CHECK-X64:  = shl nuw i128 %
> +// CHECK-X64:  = shl nuw i128 %{{.*}}, 64
>  // CHECK-X64:  = zext i64 %{{.*}} to i128
>  // CHECK-X64:  = or i128 %
>  // CHECK-X64:  = and i8 %{{.*}}, 63
>
>
> ___
> 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] D17444: PR26672: [MSVC] Clang does not recognize "static_assert" keyword in C mode

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

In D17444#1393339 , @rnk wrote:

> In D17444#1392823 , @thakis wrote:
>
> > Instead of this patch we could have an assert.h wrapper in lib/Headers that 
> > defines static_assert to _Static_assert in ms mode for C files, and I 
> > suppose that's a cleaner fix. But I hope it's not controversial that we 
> > should try and support standard C programs?
>
>
> I'd prefer it if we didn't shadow more MSVC CRT headers like assert.h. Users 
> have a long history of holding the compiler wrong, losing these 
> interpositions, and having things not work. If we could just work out the 
> box, users will have less problems, we'll have fewer bug reports, etc, etc. I 
> still think we should follow MSVC and add this is a plain old C++ extension 
> to C under -fms-extensions.


Given that we already have interposition headers and users already have to hold 
the compiler right, does it matter if we have one more interposing header?


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

https://reviews.llvm.org/D17444



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

[PATCH] D57915: [COFF, ARM64] Remove definitions for _byteswap library functions

[Tom Tan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL353740: [COFF, ARM64] Remove definitions for _byteswap 
library functions (authored by TomTan, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57915?vs=185849=186306#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57915

Files:
  cfe/trunk/lib/Headers/intrin.h
  cfe/trunk/test/Headers/ms-arm64-intrin.cpp


Index: cfe/trunk/test/Headers/ms-arm64-intrin.cpp
===
--- cfe/trunk/test/Headers/ms-arm64-intrin.cpp
+++ cfe/trunk/test/Headers/ms-arm64-intrin.cpp
@@ -14,16 +14,16 @@
 }
 
 unsigned short check_byteswap_ushort(unsigned short val) {
-// CHECK: call i16 @llvm.bswap.i16(i16 %val)
+// CHECK: call i16 @_byteswap_ushort(i16 %val)
   return _byteswap_ushort(val);
 }
 
 unsigned long check_byteswap_ulong(unsigned long val) {
-// CHECK: call i32 @llvm.bswap.i32(i32 %val)
+// CHECK: call i32 @_byteswap_ulong(i32 %val)
   return _byteswap_ulong(val);
 }
 
 unsigned __int64 check_byteswap_uint64(unsigned __int64 val) {
-// CHECK: call i64 @llvm.bswap.i64(i64 %val)
+// CHECK: call i64 @_byteswap_uint64(i64 %val)
   return _byteswap_uint64(val);
 }
Index: cfe/trunk/lib/Headers/intrin.h
===
--- cfe/trunk/lib/Headers/intrin.h
+++ cfe/trunk/lib/Headers/intrin.h
@@ -557,15 +557,9 @@
 __int64 _ReadStatusReg(int);
 void _WriteStatusReg(int, __int64);
 
-static inline unsigned short _byteswap_ushort (unsigned short val) {
-  return __builtin_bswap16(val);
-}
-static inline unsigned long _byteswap_ulong (unsigned long val) {
-  return __builtin_bswap32(val);
-}
-static inline unsigned __int64 _byteswap_uint64 (unsigned __int64 val) {
-  return __builtin_bswap64(val);
-}
+unsigned short __cdecl _byteswap_ushort(unsigned short val);
+unsigned long __cdecl _byteswap_ulong (unsigned long val);
+unsigned __int64 __cdecl _byteswap_uint64(unsigned __int64 val);
 #endif
 
 
/**\


Index: cfe/trunk/test/Headers/ms-arm64-intrin.cpp
===
--- cfe/trunk/test/Headers/ms-arm64-intrin.cpp
+++ cfe/trunk/test/Headers/ms-arm64-intrin.cpp
@@ -14,16 +14,16 @@
 }
 
 unsigned short check_byteswap_ushort(unsigned short val) {
-// CHECK: call i16 @llvm.bswap.i16(i16 %val)
+// CHECK: call i16 @_byteswap_ushort(i16 %val)
   return _byteswap_ushort(val);
 }
 
 unsigned long check_byteswap_ulong(unsigned long val) {
-// CHECK: call i32 @llvm.bswap.i32(i32 %val)
+// CHECK: call i32 @_byteswap_ulong(i32 %val)
   return _byteswap_ulong(val);
 }
 
 unsigned __int64 check_byteswap_uint64(unsigned __int64 val) {
-// CHECK: call i64 @llvm.bswap.i64(i64 %val)
+// CHECK: call i64 @_byteswap_uint64(i64 %val)
   return _byteswap_uint64(val);
 }
Index: cfe/trunk/lib/Headers/intrin.h
===
--- cfe/trunk/lib/Headers/intrin.h
+++ cfe/trunk/lib/Headers/intrin.h
@@ -557,15 +557,9 @@
 __int64 _ReadStatusReg(int);
 void _WriteStatusReg(int, __int64);
 
-static inline unsigned short _byteswap_ushort (unsigned short val) {
-  return __builtin_bswap16(val);
-}
-static inline unsigned long _byteswap_ulong (unsigned long val) {
-  return __builtin_bswap32(val);
-}
-static inline unsigned __int64 _byteswap_uint64 (unsigned __int64 val) {
-  return __builtin_bswap64(val);
-}
+unsigned short __cdecl _byteswap_ushort(unsigned short val);
+unsigned long __cdecl _byteswap_ulong (unsigned long val);
+unsigned __int64 __cdecl _byteswap_uint64(unsigned __int64 val);
 #endif
 
 /**\
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r353740 - [COFF, ARM64] Remove definitions for _byteswap library functions

[Tom Tan via cfe-commits]

Author: tomtan
Date: Mon Feb 11 12:04:02 2019
New Revision: 353740

URL: http://llvm.org/viewvc/llvm-project?rev=353740=rev
Log:
[COFF, ARM64] Remove definitions for _byteswap library functions

_byteswap_* functions are are implemented in below file as normal function
from libucrt.lib and declared in stdlib.h. Define them in intrin.h triggers
lld error "conflicting comdat type" and "duplicate symbols" which was just
added to LLD (https://reviews.llvm.org/D57324).

C:\Program Files (x86)\Windows 
Kits\10\Source\10.0.17763.0\ucrt\stdlib\byteswap.cpp

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

Modified:
cfe/trunk/lib/Headers/intrin.h
cfe/trunk/test/Headers/ms-arm64-intrin.cpp

Modified: cfe/trunk/lib/Headers/intrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/intrin.h?rev=353740=353739=353740=diff
==
--- cfe/trunk/lib/Headers/intrin.h (original)
+++ cfe/trunk/lib/Headers/intrin.h Mon Feb 11 12:04:02 2019
@@ -557,15 +557,9 @@ long _InterlockedAdd(long volatile *Adde
 __int64 _ReadStatusReg(int);
 void _WriteStatusReg(int, __int64);
 
-static inline unsigned short _byteswap_ushort (unsigned short val) {
-  return __builtin_bswap16(val);
-}
-static inline unsigned long _byteswap_ulong (unsigned long val) {
-  return __builtin_bswap32(val);
-}
-static inline unsigned __int64 _byteswap_uint64 (unsigned __int64 val) {
-  return __builtin_bswap64(val);
-}
+unsigned short __cdecl _byteswap_ushort(unsigned short val);
+unsigned long __cdecl _byteswap_ulong (unsigned long val);
+unsigned __int64 __cdecl _byteswap_uint64(unsigned __int64 val);
 #endif
 
 
/**\

Modified: cfe/trunk/test/Headers/ms-arm64-intrin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Headers/ms-arm64-intrin.cpp?rev=353740=353739=353740=diff
==
--- cfe/trunk/test/Headers/ms-arm64-intrin.cpp (original)
+++ cfe/trunk/test/Headers/ms-arm64-intrin.cpp Mon Feb 11 12:04:02 2019
@@ -14,16 +14,16 @@ void check_nop() {
 }
 
 unsigned short check_byteswap_ushort(unsigned short val) {
-// CHECK: call i16 @llvm.bswap.i16(i16 %val)
+// CHECK: call i16 @_byteswap_ushort(i16 %val)
   return _byteswap_ushort(val);
 }
 
 unsigned long check_byteswap_ulong(unsigned long val) {
-// CHECK: call i32 @llvm.bswap.i32(i32 %val)
+// CHECK: call i32 @_byteswap_ulong(i32 %val)
   return _byteswap_ulong(val);
 }
 
 unsigned __int64 check_byteswap_uint64(unsigned __int64 val) {
-// CHECK: call i64 @llvm.bswap.i64(i64 %val)
+// CHECK: call i64 @_byteswap_uint64(i64 %val)
   return _byteswap_uint64(val);
 }


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

[PATCH] D58067: [Analyzer] Crash fix for FindLastStoreBRVisitor

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware marked 3 inline comments as done.
baloghadamsoftware added a comment.

I tried very hard to create a test case where we are crashing on a true 
positive but I did not succeed. I am not sure whether it is possible so fixing 
the false positive in `CallAndMessageUnInitRefArg` also fixes the crash without 
this patch. However I am confident the bug is still a bug in the visitor and 
maybe in the future it will be used for complex values as well which could be 
`LazyCompoundVal`s. Also you can see in the test case `uninit-vals.m` that even 
if it does not crash it prints nonsense bug path notes caused by this same bug 
which is fixed by this patch.




Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:159
+/// exploded nodes.
+bool matchesValue(const ExplodedNode *LeftNode, SVal LeftVal,
+  const ExplodedNode *RightNode, SVal RightVal) {

Maybe we should find a better name. Even better we could place this function 
into `LazyCompoundVal` but with 'Store` or `ProgramStateRef` parameters instead 
of `ExplodedNode*`.



Comment at: test/Analysis/uninit-vals.m:401
 
-  b = a; // expected-note{{Value assigned to 'c'}}
   clang_analyzer_eval(b.y == 2); // expected-warning{{TRUE}}

What was this nonsense?



Comment at: test/Analysis/uninit-vals.m:418
 
-  b = a; // expected-note{{Value assigned to 'c'}}
   clang_analyzer_eval(b.x == 1); // expected-warning{{TRUE}}

This one too...


Repository:
  rC Clang

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

https://reviews.llvm.org/D58067



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

[PATCH] D58067: [Analyzer] Crash fix for FindLastStoreBRVisitor

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware created this revision.
baloghadamsoftware added reviewers: NoQ, dcoughlin.
baloghadamsoftware added a project: clang.
Herald added subscribers: gamesh411, donat.nagy, mikhail.ramalho, a.sidorin, 
szepet.

This patch is a fix for bug 40625 .

`FindLastStoreBRVisitor` tries to find the first node in the exploded graph 
where the current value was assigned to a region. This node is called the 
"store site". It is identified by a pair of `Pred` and `Succ` nodes where 
`Succ` already has the binding for the value while `Pred` does not have it. 
However the visitor mistakenly identifies a node pair as the store site where 
the value is a `LazyCompoundVal` and `Pred` does not have a store yet but 
`Succ` has it. In this case the `LazyCompoundVal` is different in the `Pred` 
node because it also contains the store which is different in the two nodes. 
This error may lead to crashes (a declaration is cast to a parameter 
declaration without check) or misleading bug path notes.

In this patch we fix this problem by checking for unequal `LazyCompoundVals`: 
if their region is equal, and their store is the same as the store of their 
nodes we consider them as equal when looking for the store site.


Repository:
  rC Clang

https://reviews.llvm.org/D58067

Files:
  lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  test/Analysis/PR40625.cpp
  test/Analysis/const_array.cpp
  test/Analysis/uninit-vals.m

Index: test/Analysis/uninit-vals.m
===
--- test/Analysis/uninit-vals.m
+++ test/Analysis/uninit-vals.m
@@ -394,11 +394,11 @@
   struct {
 int : 4;
 int y : 4;
-  } a, b, c;
+  } a, b, c; // expected-note{{'c' initialized here}}
 
   a.y = 2;
 
-  b = a; // expected-note{{Value assigned to 'c'}}
+  b = a;
   clang_analyzer_eval(b.y == 2); // expected-warning{{TRUE}}
  // expected-note@-1{{TRUE}}
 
@@ -411,11 +411,11 @@
   struct {
 int x : 4;
 int : 4;
-  } a, b, c;
+  } a, b, c; // expected-note{{'c' initialized here}}
 
   a.x = 1;
 
-  b = a; // expected-note{{Value assigned to 'c'}}
+  b = a;
   clang_analyzer_eval(b.x == 1); // expected-warning{{TRUE}}
  // expected-note@-1{{TRUE}}
 
Index: test/Analysis/const_array.cpp
===
--- test/Analysis/const_array.cpp
+++ /dev/null
@@ -1,22 +0,0 @@
-// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,alpha.core.CallAndMessageUnInitRefArg  %s -verify
-
-// expect-no-diagnostics
-
-const int arr[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
-
-void f(const int *begin, const int *end) {
-  int sum = 0;
-  for (const int *p = begin; p != end; ++p) {
-sum += *p;
-  }
-}
-
-typedef const int intarray[10];
-
-void g(const intarray ) {
-  f(arrr, arrr+sizeof(arrr));
-}
-
-void h() {
-  g(arr);
-}
Index: test/Analysis/PR40625.cpp
===
--- /dev/null
+++ test/Analysis/PR40625.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,alpha.core.CallAndMessageUnInitRefArg  %s -verify
+
+void f(const int *end);
+
+void g(const int ()[10]) {
+  f(arrr+sizeof(arrr)); // expected-warning{{1st function call argument is a pointer to uninitialized value}}
+  // FIXME: This is a false positive that should be fixed. Until then this
+  //tests the crash fix in FindLastStoreBRVisitor (beside
+  //uninit-vals.m).
+}
+
+void h() {
+  int arr[10];
+
+  g(arr);
+}
Index: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -153,6 +153,27 @@
   return E;
 }
 
+/// Two values match if either they are equal, or for LazyCompoundVals their
+/// regions are equal and their stores are equal to the stores of their
+/// exploded nodes.
+bool matchesValue(const ExplodedNode *LeftNode, SVal LeftVal,
+  const ExplodedNode *RightNode, SVal RightVal) {
+  if (LeftVal == RightVal)
+return true;
+
+  const auto LLCV = LeftVal.getAs();
+  if (!LLCV)
+return false;
+
+  const auto RLCV = RightVal.getAs();
+  if (!RLCV)
+return false;
+
+  return LLCV->getRegion() == RLCV->getRegion() &&
+LLCV->getStore() == LeftNode->getState()->getStore() &&
+RLCV->getStore() == RightNode->getState()->getStore();
+}
+
 //===--===//
 // Definitions for bug reporter visitors.
 //===--===//
@@ -1177,7 +1198,7 @@
 if (Succ->getState()->getSVal(R) != V)
   return nullptr;
 
-if (Pred->getState()->getSVal(R) == V) {
+if (matchesValue(Pred, Pred->getState()->getSVal(R), Succ, V)) {
   Optional PS = 

[PATCH] D58061: Fix a few tests that were missing ':' on CHECK lines and weren't testing anything.

[John McCall via Phabricator via cfe-commits]

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

Good catch!  Is there a reasonable way to just make FileCheck itself enforce 
this, or does that cause too many false positives with tests that are using 
e.g. `x86_64` as the entire prefix?

If we were starting over again, I think ideally the check-prefix would be 
unconditionally treated as flagging something for FileCheck, so that any sort 
of unrecognized command after it would produce an error.  I doubt there's any 
real use-case for writing e.g. `CHECK` in a test case.  But that's no longer 
possible because of the widespread idiom of using things like `CHECK-NATIVE` as 
conditionally-enabled prefixes, which is of course ambiguous with typoing a 
command like `CHECK-NEXT` or thinking that `CHECK-CONT` should exist.  But it'd 
be nice to get as close to that as we can without rewriting ten thousand 
`FileCheck` tests.


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

https://reviews.llvm.org/D58061



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

[PATCH] D57265: [PM/CC1] Add -f[no-]split-cold-code CC1 options to toggle splitting

[Vedant Kumar via Phabricator via cfe-commits]

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

> I'd prefer not adding this kind of state to PassBuilder. SplitColdCode is 
> soemthing that refers to the construction of one particular pipeline, not to 
> pipeline-building in general. It should be an argument passed down through 
> the build*Pipeline calls.

I'm not sure I understand the distinction being made here -- could you 
elaborate? Isn't enabling a pass related to pipeline-building in general? If 
not, I don't see how arguments to build*Pipeline //do// become related to 
pipeline-building in general.

For context, I've modeled the addition of the SplitColdCode member to 
PassBuilder on PGOOpt (c.f. PGOOptions::RunProfileGen). One thing I like about 
this approach is that it doesn't require changing IR, or changing very many 
different APIs. But if it's really not reasonable, I'm happy to take another 
shot at it.




Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1331
+  Opts.SplitColdCode =
+  (Opts.OptimizationLevel > 0) && (Opts.OptimizeSize != 2) &&
+  Args.hasFlag(OPT_fsplit_cold_code, OPT_fno_split_cold_code, false);

tejohnson wrote:
> would it be appropriate to give a warning when being ignored?
I think so, I'll add this in an update.


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

https://reviews.llvm.org/D57265



___
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 updated this revision to Diff 186300.
erik.pilkington added a comment.

Link to the GCC docs for `__builitn_object_size`.


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

https://reviews.llvm.org/D57918

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

Index: clang/test/Sema/fortify-std-lib.c
===
--- /dev/null
+++ clang/test/Sema/fortify-std-lib.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 -fsyntax-only %s -verify
+
+typedef unsigned long size_t;
+
+__attribute__((fortify_stdlib(0, 0)))
+int not_anything_special(); // expected-error {{'fortify_stdlib' attribute applied to an unknown function}}
+
+__attribute__((fortify_stdlib(4, 0))) // expected-error {{'fortify_stdlib' attribute requires integer constant between 0 and 3 inclusive}}
+int sprintf(char *, const char *, ...);
+
+__attribute__((fortify_stdlib())) // expected-error {{'fortify_stdlib' attribute requires exactly 2 arguments}}
+int sprintf(char *, const char *, ...);
+
+__attribute__((fortify_stdlib(1, 2, 3))) // expected-error {{'fortify_stdlib' attribute requires exactly 2 arguments}}
+int sprintf(char *, const char *, ...);
+
+__attribute__((fortify_stdlib(-1, 2))) // expected-error {{'fortify_stdlib' attribute requires a non-negative integral compile time constant expression}}
+int sprintf(char *, const char *, ...);
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -52,6 +52,7 @@
 // CHECK-NEXT: ExternalSourceSymbol ((SubjectMatchRule_record, SubjectMatchRule_enum, SubjectMatchRule_enum_constant, SubjectMatchRule_field, SubjectMatchRule_function, SubjectMatchRule_namespace, SubjectMatchRule_objc_category, SubjectMatchRule_objc_interface, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property, SubjectMatchRule_objc_protocol, SubjectMatchRule_record, SubjectMatchRule_type_alias, SubjectMatchRule_variable))
 // CHECK-NEXT: FlagEnum (SubjectMatchRule_enum)
 // CHECK-NEXT: Flatten (SubjectMatchRule_function)
+// CHECK-NEXT: FortifyStdLib (SubjectMatchRule_function)
 // CHECK-NEXT: GNUInline (SubjectMatchRule_function)
 // CHECK-NEXT: Hot (SubjectMatchRule_function)
 // CHECK-NEXT: IBAction (SubjectMatchRule_objc_method_is_instance)
Index: clang/test/CodeGen/fortify-std-lib.c
===
--- /dev/null
+++ clang/test/CodeGen/fortify-std-lib.c
@@ -0,0 +1,220 @@
+// RUN: %clang_cc1   -triple x86_64-apple-macosx10.14.0 %s -emit-llvm -O0 -disable-llvm-passes -o - -Wno-format-security | FileCheck %s
+// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -emit-llvm -O0 -disable-llvm-passes -o - -Wno-format-security | FileCheck %s
+
+#ifdef __cplusplus
+#define EXTERN extern "C"
+#else
+#define EXTERN
+#endif
+
+#define FSL(x,y) __attribute__((fortify_stdlib(x,y)))
+typedef unsigned long size_t;
+
+FSL(0, 0) EXTERN
+void *memcpy(void *dst, const void *src, size_t sz);
+
+EXTERN
+void call_memcpy(void *dst, const void *src, size_t sz) {
+  memcpy(dst, src, sz);
+  // CHECK-LABEL: define void @call_memcpy
+  // CHECK: [[REG:%[0-9]+]] = call i64 @llvm.objectsize.i64.p0i8(i8*{{.*}}, i1 false, i1 true, i1 false)
+  // CHECK: call i8* @__memcpy_chk(i8* {{.*}}, i8* {{.*}}, i64 {{.*}}, i64 [[REG]])
+}
+
+FSL(0, 0) EXTERN
+void *memmove(void *dst, const void *src, size_t sz);
+
+EXTERN
+void call_memmove(void *dst, const void *src, size_t sz) {
+  memmove(dst, src, sz);
+  // CHECK-LABEL: define void @call_memmove
+  // CHECK: [[REG:%[0-9]+]] = call i64 @llvm.objectsize.i64.p0i8(i8*{{.*}}, i1 false, i1 true, i1 false)
+  // CHECK: call i8* @__memmove_chk(i8* {{.*}}, i8* {{.*}}, i64 {{.*}}, i64 [[REG]])
+}
+
+FSL(0, 0) EXTERN
+void *memset(void *dst, int c, size_t sz);
+
+EXTERN
+void call_memset(void *dst, int c, size_t sz) {
+  memset(dst, c, sz);
+  // CHECK-LABEL: define void @call_memset
+  // CHECK: [[REG:%[0-9]+]] = call i64 @llvm.objectsize.i64.p0i8(i8*{{.*}}, i1 false, i1 true, i1 false)
+  // CHECK: call i8* @__memset_chk(i8* {{.*}}, i32 {{.*}}, i64 {{.*}}, i64 [[REG]])
+}
+
+FSL(0, 0) EXTERN
+char *stpcpy(char* dst, const char *src);
+
+EXTERN
+void call_stpcpy(char *dst, const char *src) {
+  stpcpy(dst, src);
+  // CHECK-LABEL: define void @call_stpcpy
+  // CHECK: [[REG:%[0-9]+]] = call i64 @llvm.objectsize.i64.p0i8(i8*{{.*}}, i1 

[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 marked an inline comment as done.
erik.pilkington added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:987-989
+The first argument to the attribute is the type passed to
+`__builtin_object_size`, and the second is the flag that the fortified format
+functions accept.

aaron.ballman wrote:
> Maybe I'm being dense, but I still have no idea what I'd pass for either of 
> these arguments. When I hear "type", I immediately assume I should pass in 
> something like 'int', but that seems wrong given that this is an integer 
> argument. Is there some public documentation we could link to with a "for 
> more information, see " snippet? Similar for the fortified format 
> function flag.
> 
> However, I'm also starting to wonder why this attribute is interesting to 
> users. Why not infer this attribute automatically, since there's only a 
> specified set of standard library functions that it can be applied to anyway? 
> Is it a bad idea to try to infer this for some reason?
Yeah, I guess we could link to GCC's docs for `__builtin_object_size`, I'll 
update this. I think the flag argument has something to do with enabling 
checking `%n` output parameters, but I'm not totally sure and don't want to 
spread any misinformation in the clang docs. On our implementation, the value 
is just ignored. 

This attribute would never really be used by users that aren't C library 
implementers. The problem with doing this automatically is that library users 
need to be able to customize the checking mode by defining the 
`_FORTIFY_SOURCE` macro to a level in their `.c` files. We can't do this based 
on the presence of that macro for a couple reasons, firstly, I'm not sure we 
can assume that all `*_chk` variants are present just because `_FORTIFY_SOURCE` 
is defined (whether the `_chk` variants are present seems like a decision best 
left to the library). And secondly because that would mean that `clang -E t.c 
-o - | clang -xc -` would generate different code from `clang t.c`. Given that, 
it seems like an attribute is the best customization point.


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

RE: r353718 - Make test actually test something (colons were missing)

[via cfe-commits]

I thin the Best Fix is probably to make FileCheck diag if a line starts with 
(after a few whitelisted comment chars like // and #) a check-prefix but then 
isn't followed by : (maybe after -NOT, -SAME, -LABEL etc).

FileCheck doesn't explicitly pass over leading comment characters, it just 
pattern-matches the check names followed by colon or hyphen. You can put 
arbitrary text in front of a check name and FileCheck won't care (although your 
reviewers might).
I can see the value in a diag like this.  It does mean any use of a check name 
that isn't *intended* to be a directive would be flagged (e.g., in a random 
comment within the test).  That pattern-match is case-sensitive and our 
convention is to use uppercase check names, so it probably wouldn't be TOO bad.
If anybody wants to do this I'm happy to review it.  Or file a PR and cc me.
--paulr
who has done a lot of FileCheck patch reviewing lately

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

[PATCH] D58016: fixes copy constructor generation of classes containing 0-length arrays followed by exactly 1 trivial field (fixes #40682)

[Joran Bigalet via Phabricator via cfe-commits]

jbigalet added a comment.

Thanks!
I don't have commit rights btw


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58016



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

[PATCH] D57991: [Driver][Darwin] Emit an error when using -pg on OS without support for it.

[Steven Wu via Phabricator via cfe-commits]

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

LGTM with a suggestion to make code cleaner.




Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:101
+  "the clang compiler does not support -pg option on Darwin">;
+def err_drv_clang_unsupported_opt_pg_darwin_osx: Error<
+  "the clang compiler does not support -pg option on versions of OS X 10.9 and 
later">;

Might be cleaner if you use %select here.


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

https://reviews.llvm.org/D57991



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

[PATCH] D58016: fixes copy constructor generation of classes containing 0-length arrays followed by exactly 1 trivial field (fixes #40682)

[John McCall via Phabricator via cfe-commits]

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

Ah, thanks, seems like a reasonable fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58016



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

[PATCH] D17444: PR26672: [MSVC] Clang does not recognize "static_assert" keyword in C mode

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In D17444#1392823 , @thakis wrote:

> Instead of this patch we could have an assert.h wrapper in lib/Headers that 
> defines static_assert to _Static_assert in ms mode for C files, and I suppose 
> that's a cleaner fix. But I hope it's not controversial that we should try 
> and support standard C programs?


I'd prefer it if we didn't shadow more MSVC CRT headers like assert.h. Users 
have a long history of holding the compiler wrong, losing these interpositions, 
and having things not work. If we could just work out the box, users will have 
less problems, we'll have fewer bug reports, etc, etc. I still think we should 
follow MSVC and add this is a plain old C++ extension to C under 
-fms-extensions.


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

https://reviews.llvm.org/D17444



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

[PATCH] D56871: [AMDGPU] Require at least protected visibility for certain symbols

[Yaxun Liu via Phabricator via cfe-commits]

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

LGTM. Thanks!


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

https://reviews.llvm.org/D56871



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

[PATCH] D58062: Support framework import/include auto-completion

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: lib/Sema/SemaCodeComplete.cpp:8375
+  auto AddFilesFromIncludeDir = [&](StringRef IncludeDir, bool IsSystem,
+bool isFramework) {
+bool stripFrameworkSuffix = false;

nit: `IsFramework`, `StripFrameworkSuffix` etc



Comment at: lib/Sema/SemaCodeComplete.cpp:8375
+  auto AddFilesFromIncludeDir = [&](StringRef IncludeDir, bool IsSystem,
+bool isFramework) {
+bool stripFrameworkSuffix = false;

sammccall wrote:
> nit: `IsFramework`, `StripFrameworkSuffix` etc
probably clearer just to pass in the `DirectoryLookup::LookupType_t`, rather 
than another bool.



Comment at: lib/Sema/SemaCodeComplete.cpp:8378
 llvm::SmallString<128> Dir = IncludeDir;
-if (!NativeRelDir.empty())
+if (isFramework) {
+  if (NativeRelDir.empty()) {

I don't think keeping the `stripFrameworkSuffix` state is worth scattering the 
reader's attention here.

I'd suggest instead:

```
if (!NativeRelDir.empty()) {
  if (LookupType == LT_Framework) {
// In a framework directory, Foo/Bar/ is actually 
Foo.framework/Headers/Bar/.
...
  } else {
append(Dir, NativeRelDir);
  }
}
```

or even pass Dir, NativeRelDir, and LookupType to a helper function to sort it 
out.

Below, you can just check NativeRelDir.empty() && type == Framework again...

If you do choose to keep it, please give it a descriptive name rather than an 
imperative one, e.g. `ListingFrameworksDir`



Comment at: lib/Sema/SemaCodeComplete.cpp:8378
 llvm::SmallString<128> Dir = IncludeDir;
-if (!NativeRelDir.empty())
+if (isFramework) {
+  if (NativeRelDir.empty()) {

sammccall wrote:
> I don't think keeping the `stripFrameworkSuffix` state is worth scattering 
> the reader's attention here.
> 
> I'd suggest instead:
> 
> ```
> if (!NativeRelDir.empty()) {
>   if (LookupType == LT_Framework) {
> // In a framework directory, Foo/Bar/ is actually 
> Foo.framework/Headers/Bar/.
> ...
>   } else {
> append(Dir, NativeRelDir);
>   }
> }
> ```
> 
> or even pass Dir, NativeRelDir, and LookupType to a helper function to sort 
> it out.
> 
> Below, you can just check NativeRelDir.empty() && type == Framework again...
> 
> If you do choose to keep it, please give it a descriptive name rather than an 
> imperative one, e.g. `ListingFrameworksDir`
this needs some explanation (mostly the why, not the how)



Comment at: lib/Sema/SemaCodeComplete.cpp:8388
+  Framework += ".framework";
+  llvm::sys::path::append(Dir, Framework, "Headers");
+  llvm::sys::path::append(Dir, ++Begin, End);

just `append(Dir, *Begin + ".framework", "Headers")` ?
append takes twines I think.



Comment at: lib/Sema/SemaCodeComplete.cpp:8390
+  llvm::sys::path::append(Dir, ++Begin, End);
+} else {
+  SmallString<32> Framework(NativeRelDir);

why is the if/else needed? the code above should cover both cases, the last 
append is a no-op if ++begin == end.



Comment at: lib/Sema/SemaCodeComplete.cpp:8409
   case llvm::sys::fs::file_type::directory_file:
-AddCompletion(Filename, /*IsDirectory=*/true);
+if (stripFrameworkSuffix && Filename.endswith(".framework")) {
+  auto FrameworkName = Filename.substr(0, Filename.size() - 10);

if a framework directory contains a subdirectory that does not end in 
".framework", is it actually legal to include from it? Maybe we should be 
omitting it.



Comment at: lib/Sema/SemaCodeComplete.cpp:8410
+if (stripFrameworkSuffix && Filename.endswith(".framework")) {
+  auto FrameworkName = Filename.substr(0, Filename.size() - 10);
+  AddCompletion(FrameworkName, /*IsDirectory=*/true);

```
if (should strip suffix)
  Filename.consume_back(".framework")
```
(no need for else)



Comment at: lib/Sema/SemaCodeComplete.cpp:8410
+if (stripFrameworkSuffix && Filename.endswith(".framework")) {
+  auto FrameworkName = Filename.substr(0, Filename.size() - 10);
+  AddCompletion(FrameworkName, /*IsDirectory=*/true);

sammccall wrote:
> ```
> if (should strip suffix)
>   Filename.consume_back(".framework")
> ```
> (no need for else)
one-line comment should be enough to explain the intent here: `// Entries in a 
framework directory have a .framework suffix, this does not appear in the 
source code`


Repository:
  rC Clang

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

https://reviews.llvm.org/D58062



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

[PATCH] D58065: [analyzer] Document the frontend library

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

Szelethus created this revision.
Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, dkrupp, rnkovacs, 
a.sidorin, aaron.ballman.
Herald added subscribers: cfe-commits, gamesh411, donat.nagy, mikhail.ramalho, 
szepet, baloghadamsoftware, whisperity.
Herald added a project: clang.

Since I've had my fair share of complaining about the lack of documentation, 
it's time I did my part, hence this patch, which provides documentation for a 
good majority of the frontend library.

Preview:
http://cc.elte.hu/clang-docs/docs/analyzer-frontend-html/analyzer/developer-docs/FrontendLibrary.html

Several more items could be added, describing how `AnalysisConsumer` interacts 
with `AnalysisManager` would be the most important of those. For now, I put the 
greatest emphasis on checker registration.

What should this be expanded with? Is this document too large, maybe I should 
split it up? I haven't documented much so far, so I'd happily take any feedback.


Repository:
  rC Clang

https://reviews.llvm.org/D58065

Files:
  docs/analyzer/developer-docs.rst
  docs/analyzer/developer-docs/FrontendLibrary.rst
  include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h

Index: include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
===
--- include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
+++ include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
@@ -16,46 +16,6 @@
 #include 
 #include 
 
-// FIXME: move this information to an HTML file in docs/.
-// At the very least, a checker plugin is a dynamic library that exports
-// clang_analyzerAPIVersionString. This should be defined as follows:
-//
-//   extern "C"
-//   const char clang_analyzerAPIVersionString[] =
-// CLANG_ANALYZER_API_VERSION_STRING;
-//
-// This is used to check whether the current version of the analyzer is known to
-// be incompatible with a plugin. Plugins with incompatible version strings,
-// or without a version string at all, will not be loaded.
-//
-// To add a custom checker to the analyzer, the plugin must also define the
-// function clang_registerCheckers. For example:
-//
-//extern "C"
-//void clang_registerCheckers (CheckerRegistry ) {
-//  registry.addChecker("example.MainCallChecker",
-//"Disallows calls to functions called main");
-//}
-//
-// The first method argument is the full name of the checker, including its
-// enclosing package. By convention, the registered name of a checker is the
-// name of the associated class (the template argument).
-// The second method argument is a short human-readable description of the
-// checker.
-//
-// The clang_registerCheckers function may add any number of checkers to the
-// registry. If any checkers require additional initialization, use the three-
-// argument form of CheckerRegistry::addChecker.
-//
-// To load a checker plugin, specify the full path to the dynamic library as
-// the argument to the -load option in the cc1 frontend. You can then enable
-// your custom checker using the -analyzer-checker:
-//
-//   clang -cc1 -load  -analyze
-// -analyzer-checker=
-//
-// For a complete working example, see examples/analyzer-plugin.
-
 #ifndef CLANG_ANALYZER_API_VERSION_STRING
 // FIXME: The Clang version string is not particularly granular;
 // the analyzer infrastructure can change a lot between releases.
Index: docs/analyzer/developer-docs/FrontendLibrary.rst
===
--- /dev/null
+++ docs/analyzer/developer-docs/FrontendLibrary.rst
@@ -0,0 +1,643 @@
+
+Frontend Library (libStaticAnalyzerFrontend)
+
+
+.. contents:: Table of Contents
+   :depth: 4
+
+Introduction
+
+
+This document will describe the frontend of the Static Analyzer, basically
+everything from compiling the analyzer from source, through it's invocation up
+to the beginning of the analysis. It will touch on topics such as
+
+* How the analyzer is compiled, how tools such as TableGen are used to generate
+  some of the code,
+* How to invoke the analyzer,
+* How crucial objects of the analyzer are initialized before the actual
+  analysis begins, like
+
+  * The `AnalyzerOptions` class, which entails how the command line options are
+parsed,
+  * The `CheckerManager` class, which entails how the checkers of the analyzer
+are registered and loaded into it,
+  * No list is complete without at least a third item.
+
+* How certain errors are handled with regards to backward compatibility,
+
+starting from how an entry in the TableGen gets processed during the
+compilation of the project, how this process begins runtime when the analyzer
+is invoked, up to the point where the actual analysis begins.
+
+The document will rely on the reader having a basic understanding about what
+checkers are, have invoked the analyzer at least a few times from the command
+line. 

[PATCH] D45978: dllexport const variables must have external linkage.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: test/Sema/dllexport-1.c:1-4
+// RUN: %clang_cc1 -triple i686-win32 -emit-llvm -fms-extensions -std=c99 < 
%s| FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-win32 -emit-llvm -fms-extensions -std=c11 < 
%s | FileCheck %s
+// RUN: %clang_cc1 -triple i686-mingw32 -emit-llvm -fms-extensions -std=c11 < 
%s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-mingw32 -emit-llvm -fms-extensions -std=c11 
< %s | FileCheck %s

This test should live in CodeGen not Sema.



Comment at: test/Sema/dllexport-1.c:8
+
+// CHECK: @y = common dso_local dllexport global i32 0, align 4
+

Are x and z also exported as expected?



Comment at: test/Sema/dllexport-1.cpp:4
+
+// CHECK: @"?x@@3HB" = dso_local dllexport constant i32 3, align 4
+

This test has no FileCheck line.

The goal here is for the tests in Sema to test the semantic behavior (that 
warnings are issued when expected and not issued when not expected) and to add 
tests in CodeGen to test the generated code (are things properly exported, are 
things we don't expect to be exported not exported, etc). I think you should 
split your tests accordingly rather than try to verify too much at once.


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

https://reviews.llvm.org/D45978



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

r353729 - Attempt to pacify bots more after r353718 and r353725

[Nico Weber via cfe-commits]

Author: nico
Date: Mon Feb 11 10:01:27 2019
New Revision: 353729

URL: http://llvm.org/viewvc/llvm-project?rev=353729=rev
Log:
Attempt to pacify bots more after r353718 and r353725

Modified:
cfe/trunk/test/CodeGen/ms-x86-intrinsics.c

Modified: cfe/trunk/test/CodeGen/ms-x86-intrinsics.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms-x86-intrinsics.c?rev=353729=353728=353729=diff
==
--- cfe/trunk/test/CodeGen/ms-x86-intrinsics.c (original)
+++ cfe/trunk/test/CodeGen/ms-x86-intrinsics.c Mon Feb 11 10:01:27 2019
@@ -145,9 +145,9 @@ unsigned __int64 test__shiftleft128(unsi
 }
 // CHECK-X64-LABEL: define dso_local i64 @test__shiftleft128(i64 %l, i64 %h, 
i8 %d)
 // CHECK-X64:  = zext i64 %{{.*}} to i128
-// CHECK-X64:  = shl nuw i128 %0, 64
+// CHECK-X64:  = shl nuw i128 %{{.*}}, 64
 // CHECK-X64:  = zext i64 %{{.*}} to i128
-// CHECK-X64:  = or i128 %{{.*}}, %{{.*}}
+// CHECK-X64:  = or i128 %
 // CHECK-X64:  = and i8 %{{.*}}, 63
 // CHECK-X64:  = shl i128 %
 // CHECK-X64:  = lshr i128 %
@@ -160,7 +160,7 @@ unsigned __int64 test__shiftright128(uns
 }
 // CHECK-X64-LABEL: define dso_local i64 @test__shiftright128(i64 %l, i64 %h, 
i8 %d)
 // CHECK-X64:  = zext i64 %{{.*}} to i128
-// CHECK-X64:  = shl nuw i128 %
+// CHECK-X64:  = shl nuw i128 %{{.*}}, 64
 // CHECK-X64:  = zext i64 %{{.*}} to i128
 // CHECK-X64:  = or i128 %
 // CHECK-X64:  = and i8 %{{.*}}, 63


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

[PATCH] D56871: [AMDGPU] Require at least protected visibility for certain symbols

[Scott Linder via Phabricator via cfe-commits]

scott.linder added a comment.

Ping


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

https://reviews.llvm.org/D56871



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

r353725 - Attempt to pacify bots after r353718

[Nico Weber via cfe-commits]

Author: nico
Date: Mon Feb 11 09:30:25 2019
New Revision: 353725

URL: http://llvm.org/viewvc/llvm-project?rev=353725=rev
Log:
Attempt to pacify bots after r353718

Modified:
cfe/trunk/test/CodeGen/ms-x86-intrinsics.c

Modified: cfe/trunk/test/CodeGen/ms-x86-intrinsics.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms-x86-intrinsics.c?rev=353725=353724=353725=diff
==
--- cfe/trunk/test/CodeGen/ms-x86-intrinsics.c (original)
+++ cfe/trunk/test/CodeGen/ms-x86-intrinsics.c Mon Feb 11 09:30:25 2019
@@ -144,11 +144,11 @@ unsigned __int64 test__shiftleft128(unsi
   return __shiftleft128(l, h, d);
 }
 // CHECK-X64-LABEL: define dso_local i64 @test__shiftleft128(i64 %l, i64 %h, 
i8 %d)
-// CHECK-X64:  = zext i64 %h to i128
+// CHECK-X64:  = zext i64 %{{.*}} to i128
 // CHECK-X64:  = shl nuw i128 %0, 64
-// CHECK-X64:  = zext i64 %l to i128
-// CHECK-X64:  = or i128 %1, %2
-// CHECK-X64:  = and i8 %d, 63
+// CHECK-X64:  = zext i64 %{{.*}} to i128
+// CHECK-X64:  = or i128 %{{.*}}, %{{.*}}
+// CHECK-X64:  = and i8 %{{.*}}, 63
 // CHECK-X64:  = shl i128 %
 // CHECK-X64:  = lshr i128 %
 // CHECK-X64:  = trunc i128 %
@@ -159,11 +159,11 @@ unsigned __int64 test__shiftright128(uns
   return __shiftright128(l, h, d);
 }
 // CHECK-X64-LABEL: define dso_local i64 @test__shiftright128(i64 %l, i64 %h, 
i8 %d)
-// CHECK-X64:  = zext i64 %h to i128
+// CHECK-X64:  = zext i64 %{{.*}} to i128
 // CHECK-X64:  = shl nuw i128 %
-// CHECK-X64:  = zext i64 %l to i128
+// CHECK-X64:  = zext i64 %{{.*}} to i128
 // CHECK-X64:  = or i128 %
-// CHECK-X64:  = and i8 %d, 63
+// CHECK-X64:  = and i8 %{{.*}}, 63
 // CHECK-X64:  = lshr i128 %
 // CHECK-X64:  = trunc i128 %
 // CHECK-X64:  ret i64 %


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