[PATCH] D57642: [X86] Update clobber list in a test after D57641. Remove filter for 'fpsw' in MS inline asm clobber list generation since the backend now uses 'fpsr'.

2019-02-04 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL353142: [X86] Change MS inline asm clobber list filter to 
check for fpsr instead of… (authored by ctopper, committed by ).
Herald added a project: LLVM.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57642

Files:
  cfe/trunk/lib/Parse/ParseStmtAsm.cpp


Index: cfe/trunk/lib/Parse/ParseStmtAsm.cpp
===
--- cfe/trunk/lib/Parse/ParseStmtAsm.cpp
+++ cfe/trunk/lib/Parse/ParseStmtAsm.cpp
@@ -636,7 +636,7 @@
   // Filter out "fpsw" and "mxcsr". They aren't valid GCC asm clobber
   // constraints. Clang always adds fpsr to the clobber list anyway.
   llvm::erase_if(Clobbers, [](const std::string ) {
-return C == "fpsw" || C == "mxcsr";
+return C == "fpsr" || C == "mxcsr";
   });
 
   // Build the vector of clobber StringRefs.


Index: cfe/trunk/lib/Parse/ParseStmtAsm.cpp
===
--- cfe/trunk/lib/Parse/ParseStmtAsm.cpp
+++ cfe/trunk/lib/Parse/ParseStmtAsm.cpp
@@ -636,7 +636,7 @@
   // Filter out "fpsw" and "mxcsr". They aren't valid GCC asm clobber
   // constraints. Clang always adds fpsr to the clobber list anyway.
   llvm::erase_if(Clobbers, [](const std::string ) {
-return C == "fpsw" || C == "mxcsr";
+return C == "fpsr" || C == "mxcsr";
   });
 
   // Build the vector of clobber StringRefs.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r353142 - [X86] Change MS inline asm clobber list filter to check for 'fpsr' instead of 'fpsw' after D57641.

2019-02-04 Thread Craig Topper via cfe-commits
Author: ctopper
Date: Mon Feb  4 22:13:14 2019
New Revision: 353142

URL: http://llvm.org/viewvc/llvm-project?rev=353142=rev
Log:
[X86] Change MS inline asm clobber list filter to check for 'fpsr' instead of 
'fpsw' after D57641.

Summary: The backend used to print the x87 FPSW register as 'fpsw', but gcc 
inline asm uses 'fpsr'. After D57641, the backend now uses 'fpsr' to match.

Reviewers: rnk

Reviewed By: rnk

Subscribers: eraman, cfe-commits, llvm-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/Parse/ParseStmtAsm.cpp

Modified: cfe/trunk/lib/Parse/ParseStmtAsm.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmtAsm.cpp?rev=353142=353141=353142=diff
==
--- cfe/trunk/lib/Parse/ParseStmtAsm.cpp (original)
+++ cfe/trunk/lib/Parse/ParseStmtAsm.cpp Mon Feb  4 22:13:14 2019
@@ -636,7 +636,7 @@ StmtResult Parser::ParseMicrosoftAsmStat
   // Filter out "fpsw" and "mxcsr". They aren't valid GCC asm clobber
   // constraints. Clang always adds fpsr to the clobber list anyway.
   llvm::erase_if(Clobbers, [](const std::string ) {
-return C == "fpsw" || C == "mxcsr";
+return C == "fpsr" || C == "mxcsr";
   });
 
   // Build the vector of clobber StringRefs.


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


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

2019-02-04 Thread Thomas Lively via Phabricator via cfe-commits
tlively created this revision.
tlively added a reviewer: aheejin.
Herald added subscribers: cfe-commits, sunfish, hiraditya, jgravelle-google, 
sbc100, dschuff.
Herald added a project: clang.

implements llvm intrinsics and clang intrinsics for
memory.init and data.drop.


Repository:
  rG LLVM Github Monorepo

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/test/CodeGen/WebAssembly/bulk-memory-intrinsics.ll

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 0, 3, $0, $1, $2
+; CHECK-NEXT: return
+declare void @llvm.wasm.memory.init.i32.i32.i8(i32, i32, i8*, i32, i32)
+define void @memory_init(i8* %dest, i32 %offset, i32 %size) {
+  call void @llvm.wasm.memory.init.i32.i32.i8(
+i32 0, i32 3, i8* %dest, i32 %offset, i32 %size
+  )
+  ret void
+}
+
+; CHECK-LABEL: data_drop:
+; CHECK-NEXT: .functype  data_drop () -> ()
+; CHECK-NEXT: data.drop 3
+; CHECK-NEXT: return
+declare void @llvm.wasm.data.drop.i32(i32)
+define void @data_drop() {
+  call void @llvm.wasm.data.drop.i32(i32 3)
+  ret void
+}
Index: llvm/lib/Target/WebAssembly/WebAssemblyInstrBulkMemory.td
===
--- llvm/lib/Target/WebAssembly/WebAssemblyInstrBulkMemory.td
+++ llvm/lib/Target/WebAssembly/WebAssemblyInstrBulkMemory.td
@@ -27,6 +27,31 @@
 def wasm_memcpy : SDNode<"WebAssemblyISD::MEMORY_COPY", wasm_memcpy_t,
  [SDNPHasChain, SDNPMayLoad, SDNPMayStore]>;
 
+//===--===//
+// memory.init
+//===--===//
+
+let mayStore = 1 in
+defm MEMORY_INIT :
+  BULK_I<(outs),
+ (ins i32imm_op:$seg, i32imm_op:$idx, I32:$dest,
+  I32:$offset, I32:$size),
+ (outs), (ins i32imm_op:$seg, i32imm_op:$idx),
+ [(int_wasm_memory_init (i32 imm:$seg), (i32 imm:$idx), I32:$dest,
+I32:$offset, I32:$size
+  )],
+ "memory.init\t$seg, $idx, $dest, $offset, $size",
+ "memory.init\t$seg, $idx", 0x08>;
+
+//===--===//
+// data.drop
+//===--===//
+
+defm DATA_DROP :
+  BULK_I<(outs), (ins i32imm_op:$seg), (outs), (ins i32imm_op:$seg),
+ [(int_wasm_data_drop (i32 imm:$seg))],
+ "data.drop\t$seg", "data.drop\t$seg", 0x09>;
+
 //===--===//
 // memory.copy
 //===--===//
Index: llvm/include/llvm/IR/IntrinsicsWebAssembly.td
===
--- llvm/include/llvm/IR/IntrinsicsWebAssembly.td
+++ llvm/include/llvm/IR/IntrinsicsWebAssembly.td
@@ -110,4 +110,20 @@
 [llvm_anyvector_ty],
 [IntrNoMem, IntrSpeculatable]>;
 
+//===--===//
+// Bulk memory intrinsics
+//===--===//
+
+def int_wasm_memory_init :
+  Intrinsic<[],
+[llvm_anyint_ty, llvm_anyint_ty, LLVMPointerType,
+ llvm_i32_ty, llvm_i32_ty],
+[IntrWriteMem, IntrArgMemOnly, WriteOnly<1>],
+"", [SDNPMemOperand]>;
+def int_wasm_data_drop :
+  Intrinsic<[],
+[llvm_anyint_ty],
+[IntrNoDuplicate, IntrHasSideEffects],
+"", [SDNPMemOperand]>;
+
 } // TargetPrefix = "wasm"
Index: clang/test/CodeGen/builtins-wasm.c
===
--- clang/test/CodeGen/builtins-wasm.c
+++ clang/test/CodeGen/builtins-wasm.c
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -triple wasm32-unknown-unknown -target-feature +unimplemented-simd128 -target-feature +nontrapping-fptoint -target-feature +exception-handling -fno-lax-vector-conversions -O3 -emit-llvm -o - %s | FileCheck %s -check-prefixes WEBASSEMBLY,WEBASSEMBLY32
-// RUN: %clang_cc1 -triple wasm64-unknown-unknown -target-feature +unimplemented-simd128 

[PATCH] D47358: : Implement {un, }synchronized_pool_resource.

2019-02-04 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 185233.
Quuxplusone added a subscriber: AlisdairM.
Quuxplusone added a comment.

Rebased on master. @EricWF (cc @AlisdairM) ping!


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D47358

Files:
  include/experimental/memory_resource
  src/experimental/memory_resource.cpp
  
test/libcxx/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsynchronized_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.ctor/ctor_does_not_allocate.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.ctor/sync_with_default_resource.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.ctor/unsync_with_default_resource.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/equality.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/sync_allocate.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/sync_allocate_overaligned_request.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/sync_allocate_reuse_blocks.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/sync_deallocate_matches_allocate.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_allocate.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_allocate_overaligned_request.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_allocate_reuse_blocks.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_deallocate_matches_allocate.pass.cpp
  test/std/experimental/memory/memory.resource.pool/pool_options.pass.cpp
  test/support/count_new.hpp

Index: test/support/count_new.hpp
===
--- test/support/count_new.hpp
+++ test/support/count_new.hpp
@@ -210,6 +210,11 @@
 return disable_checking || n != delete_called;
 }
 
+bool checkDeleteCalledGreaterThan(int n) const
+{
+return disable_checking || delete_called > n;
+}
+
 bool checkAlignedNewCalledEq(int n) const
 {
 return disable_checking || n == aligned_new_called;
Index: test/std/experimental/memory/memory.resource.pool/pool_options.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.pool/pool_options.pass.cpp
@@ -0,0 +1,26 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03, c++11, c++14
+// UNSUPPORTED: apple-clang-7
+
+// 
+
+// struct pool_options
+
+#include 
+#include 
+
+int main()
+{
+const std::experimental::pmr::pool_options p;
+assert(p.max_blocks_per_chunk == 0);
+assert(p.largest_required_pool_block == 0);
+}
Index: test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_deallocate_matches_allocate.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_deallocate_matches_allocate.pass.cpp
@@ -0,0 +1,110 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class unsynchronized_pool_resource
+
+#include 
+#include 
+#include 
+
+struct allocation_record {
+size_t bytes;
+size_t align;
+explicit allocation_record(size_t b, size_t a) : bytes(b), align(a) {}
+bool operator==(const allocation_record& rhs) const {
+return (bytes == rhs.bytes) && (align == rhs.align);
+}
+bool operator<(const allocation_record& rhs) const {
+if (bytes != rhs.bytes) return (bytes < rhs.bytes);
+return (align < rhs.align);
+}
+};
+
+class test_resource : public std::experimental::pmr::memory_resource {
+void *do_allocate(size_t bytes, size_t align) override {
+void *result = std::experimental::pmr::new_delete_resource()->allocate(bytes, align);
+successful_allocations.emplace_back(bytes, align);
+return 

[PATCH] D47111: : Implement monotonic_buffer_resource.

2019-02-04 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 185229.
Quuxplusone added a subscriber: AlisdairM.
Quuxplusone added a comment.

Rebased on master. @ericwf (cc @AlisdairM) ping!


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D47111

Files:
  include/experimental/memory_resource
  src/experimental/memory_resource.cpp
  
test/libcxx/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/copy_move.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/with_default_resource.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.ctor/without_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_deallocate.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_exception_safety.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_initial_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_underaligned_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_from_zero_sized_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_in_geometric_progression.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_overaligned_request.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
  
test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp

Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/equality.pass.cpp
@@ -0,0 +1,62 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class monotonic_buffer_resource
+
+#include 
+#include 
+#include 
+#include 
+
+struct assert_on_compare : public std::experimental::pmr::memory_resource
+{
+protected:
+void *do_allocate(size_t, size_t)
+{ assert(false); }
+
+void do_deallocate(void *, size_t, size_t)
+{ assert(false); }
+
+bool do_is_equal(std::experimental::pmr::memory_resource const &) const noexcept
+{ assert(false); }
+};
+
+int main()
+{
+// Same object
+{
+std::experimental::pmr::monotonic_buffer_resource r1;
+std::experimental::pmr::monotonic_buffer_resource r2;
+assert(r1 == r1);
+assert(r1 != r2);
+
+std::experimental::pmr::memory_resource & p1 = r1;
+std::experimental::pmr::memory_resource & p2 = r2;
+assert(p1 == p1);
+assert(p1 != p2);
+assert(p1 == r1);
+assert(r1 == p1);
+assert(p1 != r2);
+assert(r2 != p1);
+}
+// Different types
+{
+std::experimental::pmr::monotonic_buffer_resource mono1;
+std::experimental::pmr::memory_resource & r1 = mono1;
+assert_on_compare c;
+std::experimental::pmr::memory_resource & r2 = c;
+assert(r1 != r2);
+assert(!(r1 == r2));
+}
+}
Index: test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.monotonic.buffer/monotonic.buffer.mem/allocate_with_initial_size.pass.cpp
@@ -0,0 +1,51 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class monotonic_buffer_resource
+
+#include 
+#include 
+
+#include "count_new.hpp"
+
+void test(size_t initial_buffer_size)
+{
+globalMemCounter.reset();
+
+std::experimental::pmr::monotonic_buffer_resource mono1(
+initial_buffer_size,
+ 

[libunwind] r353137 - [CMake] Update lit test configuration

2019-02-04 Thread Petr Hosek via cfe-commits
Author: phosek
Date: Mon Feb  4 20:44:03 2019
New Revision: 353137

URL: http://llvm.org/viewvc/llvm-project?rev=353137=rev
Log:
[CMake] Update lit test configuration

There are several changes:
- Don't stringify Pythonized bools (that's why we're Pythonizing them)
- Support specifying target and sysroot via CMake variables
- Use consistent spelling for --target, --sysroot, --gcc-toolchain

Modified:
libunwind/trunk/test/CMakeLists.txt
libunwind/trunk/test/lit.site.cfg.in

Modified: libunwind/trunk/test/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/test/CMakeLists.txt?rev=353137=353136=353137=diff
==
--- libunwind/trunk/test/CMakeLists.txt (original)
+++ libunwind/trunk/test/CMakeLists.txt Mon Feb  4 20:44:03 2019
@@ -16,6 +16,7 @@ pythonize_bool(LIBCXX_ENABLE_SHARED)
 pythonize_bool(LIBUNWIND_ENABLE_SHARED)
 pythonize_bool(LIBUNWIND_ENABLE_THREADS)
 pythonize_bool(LIBUNWIND_ENABLE_EXCEPTIONS)
+pythonize_bool(LIBUNWIND_USE_COMPILER_RT)
 pythonize_bool(LIBUNWIND_BUILD_EXTERNAL_THREAD_LIBRARY)
 set(LIBUNWIND_TARGET_INFO "libcxx.test.target_info.LocalTI" CACHE STRING
 "TargetInfo to use when setting up test environment.")

Modified: libunwind/trunk/test/lit.site.cfg.in
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/test/lit.site.cfg.in?rev=353137=353136=353137=diff
==
--- libunwind/trunk/test/lit.site.cfg.in (original)
+++ libunwind/trunk/test/lit.site.cfg.in Mon Feb  4 20:44:03 2019
@@ -7,23 +7,24 @@ config.abi_library_path = "@LIBU
 config.libcxx_src_root  = "@LIBUNWIND_LIBCXX_PATH@"
 config.libunwind_headers= "@LIBUNWIND_SOURCE_DIR@/include"
 config.cxx_library_root = "@LIBUNWIND_LIBCXX_LIBRARY_PATH@"
-config.llvm_unwinder= "1"
-config.enable_threads   = "@LIBUNWIND_ENABLE_THREADS@"
+config.llvm_unwinder= True
+config.compiler_rt  = @LIBUNWIND_USE_COMPILER_RT@
+config.enable_threads   = @LIBUNWIND_ENABLE_THREADS@
 config.use_sanitizer= "@LLVM_USE_SANITIZER@"
-config.enable_32bit = "@LIBUNWIND_BUILD_32_BITS@"
+config.enable_32bit = @LIBUNWIND_BUILD_32_BITS@
 config.target_info  = "@LIBUNWIND_TARGET_INFO@"
 config.test_linker_flags= "@LIBUNWIND_TEST_LINKER_FLAGS@"
 config.test_compiler_flags  = "@LIBUNWIND_TEST_COMPILER_FLAGS@"
 config.executor = "@LIBUNWIND_EXECUTOR@"
-config.libunwind_shared = "@LIBUNWIND_ENABLE_SHARED@"
-config.enable_shared= "@LIBCXX_ENABLE_SHARED@"
-config.enable_exceptions= "@LIBUNWIND_ENABLE_EXCEPTIONS@"
+config.libunwind_shared = @LIBUNWIND_ENABLE_SHARED@
+config.enable_shared= @LIBCXX_ENABLE_SHARED@
+config.enable_exceptions= @LIBUNWIND_ENABLE_EXCEPTIONS@
 config.host_triple  = "@LLVM_HOST_TRIPLE@"
 config.target_triple= "@TARGET_TRIPLE@"
 config.use_target   = bool("@LIBUNWIND_TARGET_TRIPLE@")
 config.sysroot  = "@LIBUNWIND_SYSROOT@"
 config.gcc_toolchain= "@LIBUNWIND_GCC_TOOLCHAIN@"
-config.cxx_ext_threads  = "@LIBUNWIND_BUILD_EXTERNAL_THREAD_LIBRARY@"
+config.cxx_ext_threads  = @LIBUNWIND_BUILD_EXTERNAL_THREAD_LIBRARY@
 
 # Let the main config do the real work.
 lit_config.load_config(config, "@LIBUNWIND_SOURCE_DIR@/test/lit.cfg")


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


[PATCH] D47344: LWG 2843 "Unclear behavior of std::pmr::memory_resource::do_allocate()"

2019-02-04 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 185225.
Quuxplusone added a comment.

Rebased on master. @EricWF ping! It's been quite a few months...


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D47344

Files:
  src/experimental/memory_resource.cpp


Index: src/experimental/memory_resource.cpp
===
--- src/experimental/memory_resource.cpp
+++ src/experimental/memory_resource.cpp
@@ -25,19 +25,20 @@
 class _LIBCPP_TYPE_VIS __new_delete_memory_resource_imp
 : public memory_resource
 {
-public:
-~__new_delete_memory_resource_imp() = default;
-
-protected:
-virtual void* do_allocate(size_t __size, size_t __align)
-{ return _VSTD::__libcpp_allocate(__size, __align); /* FIXME */}
+void *do_allocate(size_t size, size_t align) override {
+#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+if (__is_overaligned_for_new(align))
+__throw_bad_alloc();
+#endif
+return _VSTD::__libcpp_allocate(size, align);
+}
 
-virtual void do_deallocate(void* __p, size_t __n, size_t __align) {
-  _VSTD::__libcpp_deallocate(__p, __n, __align); /* FIXME */
+void do_deallocate(void *p, size_t n, size_t align) override {
+  _VSTD::__libcpp_deallocate(p, n, align);
 }
 
-virtual bool do_is_equal(memory_resource const & __other) const _NOEXCEPT
-{ return &__other == this; }
+bool do_is_equal(memory_resource const & other) const _NOEXCEPT override
+{ return  == this; }
 };
 
 // null_memory_resource()


Index: src/experimental/memory_resource.cpp
===
--- src/experimental/memory_resource.cpp
+++ src/experimental/memory_resource.cpp
@@ -25,19 +25,20 @@
 class _LIBCPP_TYPE_VIS __new_delete_memory_resource_imp
 : public memory_resource
 {
-public:
-~__new_delete_memory_resource_imp() = default;
-
-protected:
-virtual void* do_allocate(size_t __size, size_t __align)
-{ return _VSTD::__libcpp_allocate(__size, __align); /* FIXME */}
+void *do_allocate(size_t size, size_t align) override {
+#ifdef _LIBCPP_HAS_NO_ALIGNED_ALLOCATION
+if (__is_overaligned_for_new(align))
+__throw_bad_alloc();
+#endif
+return _VSTD::__libcpp_allocate(size, align);
+}
 
-virtual void do_deallocate(void* __p, size_t __n, size_t __align) {
-  _VSTD::__libcpp_deallocate(__p, __n, __align); /* FIXME */
+void do_deallocate(void *p, size_t n, size_t align) override {
+  _VSTD::__libcpp_deallocate(p, n, align);
 }
 
-virtual bool do_is_equal(memory_resource const & __other) const _NOEXCEPT
-{ return &__other == this; }
+bool do_is_equal(memory_resource const & other) const _NOEXCEPT override
+{ return  == this; }
 };
 
 // null_memory_resource()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D57732: Correct inf typo

2019-02-04 Thread Andrew Gaul via Phabricator via cfe-commits
gaul created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D57732

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -776,7 +776,7 @@
 /// set, and the function could/should not be put on a single line (as per
 /// `AllowShortFunctionsOnASingleLine` and constructor formatting options).
 /// \code
-///   int f()   vs.   inf f()
+///   int f()   vs.   int f()
 ///   {}  {
 ///   }
 /// \endcode
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -755,7 +755,7 @@
 
 .. code-block:: c++
 
-  int f()   vs.   inf f()
+  int f()   vs.   int f()
   {}  {
   }
 


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -776,7 +776,7 @@
 /// set, and the function could/should not be put on a single line (as per
 /// `AllowShortFunctionsOnASingleLine` and constructor formatting options).
 /// \code
-///   int f()   vs.   inf f()
+///   int f()   vs.   int f()
 ///   {}  {
 ///   }
 /// \endcode
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -755,7 +755,7 @@
 
 .. code-block:: c++
 
-  int f()   vs.   inf f()
+  int f()   vs.   int f()
   {}  {
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D57668: [opaque pointer types] Pass function types for runtime function calls.

2019-02-04 Thread David Blaikie via Phabricator via cfe-commits
dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Great - thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57668



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


[PATCH] D57636: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg

2019-02-04 Thread Mike Hommey via Phabricator via cfe-commits
glandium updated this revision to Diff 185218.
glandium edited the summary of this revision.
glandium added a comment.

Updated EmitAArch64BuiltinExpr per https://reviews.llvm.org/D57636#1383751 and 
the testcase per https://reviews.llvm.org/D57636#1384348


Repository:
  rC Clang

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

https://reviews.llvm.org/D57636

Files:
  include/clang/Basic/BuiltinsAArch64.def
  lib/CodeGen/CGBuiltin.cpp
  lib/Headers/intrin.h
  test/CodeGen/arm64-microsoft-status-reg.cpp

Index: test/CodeGen/arm64-microsoft-status-reg.cpp
===
--- test/CodeGen/arm64-microsoft-status-reg.cpp
+++ test/CodeGen/arm64-microsoft-status-reg.cpp
@@ -23,87 +23,101 @@
 #define ARM64_TPIDRRO_EL0   ARM64_SYSREG(3,3,13, 0,3)  // Thread ID Register, User Read Only [CP15_TPIDRURO]
 #define ARM64_TPIDR_EL1 ARM64_SYSREG(3,0,13, 0,4)  // Thread ID Register, Privileged Only [CP15_TPIDRPRW]
 
-void check_ReadWriteStatusReg(int v) {
-  int ret;
+// From intrin.h
+__int64 _ReadStatusReg(int);
+void _WriteStatusReg(int, __int64);
+
+void check_ReadWriteStatusReg(__int64 v) {
+  __int64 ret;
   ret = _ReadStatusReg(ARM64_CNTVCT);
-// CHECK-ASM: mrs x8, CNTVCT_EL0
-// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD2:.*]])
+// CHECK-ASM: mrs x0, CNTVCT_EL0
+// CHECK-IR: %[[VAR:.*]] = call i64 @llvm.read_register.i64(metadata ![[MD2:.*]])
+// CHECK-IR-NEXT: store i64 %[[VAR]]
 
   ret = _ReadStatusReg(ARM64_PMCCNTR_EL0);
-// CHECK-ASM: mrs x8, PMCCNTR_EL0
-// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD3:.*]])
+// CHECK-ASM: mrs x0, PMCCNTR_EL0
+// CHECK-IR: %[[VAR:.*]] = call i64 @llvm.read_register.i64(metadata ![[MD3:.*]])
+// CHECK-IR-NEXT: store i64 %[[VAR]]
 
   ret = _ReadStatusReg(ARM64_PMSELR_EL0);
-// CHECK-ASM: mrs x8, PMSELR_EL0
-// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD4:.*]])
+// CHECK-ASM: mrs x0, PMSELR_EL0
+// CHECK-IR: %[[VAR:.*]] = call i64 @llvm.read_register.i64(metadata ![[MD4:.*]])
+// CHECK-IR-NEXT: store i64 %[[VAR]]
 
   ret = _ReadStatusReg(ARM64_PMXEVCNTR_EL0);
-// CHECK-ASM: mrs x8, PMXEVCNTR_EL0
-// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD5:.*]])
+// CHECK-ASM: mrs x0, PMXEVCNTR_EL0
+// CHECK-IR: %[[VAR:.*]] = call i64 @llvm.read_register.i64(metadata ![[MD5:.*]])
+// CHECK-IR-NEXT: store i64 %[[VAR]]
 
   ret = _ReadStatusReg(ARM64_PMXEVCNTRn_EL0(0));
-// CHECK-ASM: mrs x8, PMEVCNTR0_EL0
-// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD6:.*]])
+// CHECK-ASM: mrs x0, PMEVCNTR0_EL0
+// CHECK-IR: %[[VAR:.*]] = call i64 @llvm.read_register.i64(metadata ![[MD6:.*]])
+// CHECK-IR-NEXT: store i64 %[[VAR]]
 
   ret = _ReadStatusReg(ARM64_PMXEVCNTRn_EL0(1));
-// CHECK-ASM: mrs x8, PMEVCNTR1_EL0
-// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD7:.*]])
+// CHECK-ASM: mrs x0, PMEVCNTR1_EL0
+// CHECK-IR: %[[VAR:.*]] = call i64 @llvm.read_register.i64(metadata ![[MD7:.*]])
+// CHECK-IR-NEXT: store i64 %[[VAR]]
 
   ret = _ReadStatusReg(ARM64_PMXEVCNTRn_EL0(30));
-// CHECK-ASM: mrs x8, PMEVCNTR30_EL0
-// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD8:.*]])
+// CHECK-ASM: mrs x0, PMEVCNTR30_EL0
+// CHECK-IR: %[[VAR:.*]] = call i64 @llvm.read_register.i64(metadata ![[MD8:.*]])
+// CHECK-IR-NEXT: store i64 %[[VAR]]
 
   ret = _ReadStatusReg(ARM64_TPIDR_EL0);
-// CHECK-ASM: mrs x8, TPIDR_EL0
-// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD9:.*]])
+// CHECK-ASM: mrs x0, TPIDR_EL0
+// CHECK-IR: %[[VAR:.*]] = call i64 @llvm.read_register.i64(metadata ![[MD9:.*]])
+// CHECK-IR-NEXT: store i64 %[[VAR]]
 
   ret = _ReadStatusReg(ARM64_TPIDRRO_EL0);
-// CHECK-ASM: mrs x8, TPIDRRO_EL0
-// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD10:.*]])
+// CHECK-ASM: mrs x0, TPIDRRO_EL0
+// CHECK-IR: %[[VAR:.*]] = call i64 @llvm.read_register.i64(metadata ![[MD10:.*]])
+// CHECK-IR-NEXT: store i64 %[[VAR]]
 
   ret = _ReadStatusReg(ARM64_TPIDR_EL1);
-// CHECK-ASM: mrs x8, TPIDR_EL1
-// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD11:.*]])
+// CHECK-ASM: mrs x0, TPIDR_EL1
+// CHECK-IR: %[[VAR:.*]] = call i64 @llvm.read_register.i64(metadata ![[MD11:.*]])
+// CHECK-IR-NEXT: store i64 %[[VAR]]
 
 
   _WriteStatusReg(ARM64_CNTVCT, v);
-// CHECK-ASM: msr S3_3_C14_C0_2, x8
+// CHECK-ASM: msr S3_3_C14_C0_2, x0
 // CHECK-IR: call void @llvm.write_register.i64(metadata ![[MD2:.*]], i64 {{%.*}})
 
   _WriteStatusReg(ARM64_PMCCNTR_EL0, v);
-// CHECK-ASM: msr PMCCNTR_EL0, x8
+// CHECK-ASM: msr PMCCNTR_EL0, x0
 // CHECK-IR: call void @llvm.write_register.i64(metadata ![[MD3:.*]], i64 {{%.*}})
 
   _WriteStatusReg(ARM64_PMSELR_EL0, v);
-// CHECK-ASM: msr PMSELR_EL0, x8
+// CHECK-ASM: msr PMSELR_EL0, x0
 // CHECK-IR: call void @llvm.write_register.i64(metadata ![[MD4:.*]], i64 {{%.*}})
 
   

[PATCH] D57640: [NewPM][MSan] Add Options Handling

2019-02-04 Thread Chandler Carruth via Phabricator via cfe-commits
chandlerc added a comment.

Minor past-commit nit.




Comment at: lib/CodeGen/BackendUtil.cpp:1039
 [](FunctionPassManager , PassBuilder::OptimizationLevel Level) 
{
-  FPM.addPass(MemorySanitizerPass());
+  FPM.addPass(MemorySanitizerPass({}));
 });

This change isn't needed any more (since you added the default back).


Repository:
  rC Clang

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

https://reviews.llvm.org/D57640



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


[PATCH] D57664: [opaque pointer types] Fix the CallInfo passed to EmitCall in some edge cases.

2019-02-04 Thread James Y Knight via Phabricator via cfe-commits
jyknight marked an inline comment as done.
jyknight added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:3837
+// having pointee types).
+llvm::FunctionType *IRFuncTyFromInfo = 
getTypes().GetFunctionType(CallInfo);
+assert(IRFuncTy == IRFuncTyFromInfo);

dblaikie wrote:
> This will be warned as unused in a release build.
> 
> Would this be hideous if it's just all one big assert?
> 
>   assert((CallInfo.isVariadic && CallInfo.getArgStruct) || IRFuncTy == 
> getTypes().GetFunctionType(CallInfo));
> 
> (I think that's accurate?)
Clearer IMO to just put #ifndef NDEBUG around the block, so I'll do that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57664



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


[PATCH] D57668: [opaque pointer types] Pass function types for runtime function calls.

2019-02-04 Thread James Y Knight via Phabricator via cfe-commits
jyknight marked an inline comment as done.
jyknight added inline comments.



Comment at: clang/lib/CodeGen/CGObjC.cpp:1028-1030
+llvm::Constant *getPropertyFn = cast(
+CGM.getObjCRuntime().GetPropertyGetFunction().getCallee());
 if (!getPropertyFn) {

dblaikie wrote:
> This code looks like it implies that the 'if' is never hit? (since cast 
> doesn't propagate null (it asserts/fails/UB on null)) - should this be 
> cast_or_null instead? Or "if(!CGM.getObjCRuntime().getPropertyGetFunction())" 
> ?)
> 
> (there are a few similar instances later on)
Indeed, good catch! I don't know if that actually happens, but this code also 
didn't even need these casts.

Removed the casts, both simplifying the code, and fixing that potential issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57668



___
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

2019-02-04 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: lib/AST/Stmt.cpp:457-460
   this->NumOutputs = NumOutputs;
   this->NumInputs = NumInputs;
   this->NumClobbers = NumClobbers;
+  this->NumLabels = NumLabels;

jyu2 wrote:
> rsmith wrote:
> > Please assert that you don't have both outputs and labels here. (If you 
> > did, you would assign them the same slots within `Exprs`.)
> > 
> > You also seem to be missing `Sema` enforcement of the rule that you cannot 
> > have both outputs and labels. (If you want to actually support that as an 
> > intentional extension to the GCC functionality, you should change the 
> > implementation of `GCCAsmStmt` to use different slots for outputs and 
> > labels, add some tests, and add a `-Wgcc-compat` warning for that case. 
> > Seems better to just add an error for it for now.)
> This is enforcement during the parer.  
> 
> for example:
> a.c:12:23: error: expected ':'
> asm goto ("decl %0;" :"a": "m"(cond) : "memory" );
> 
> Do you think this is enough for now?
> Thanks.
> Jennifer
Thanks, I see it now. Please still add the assert here.

I'd also like a custom diagnostic for that parse error; it'd seem easy and 
useful to add an "asm goto cannot have output constraints" error.


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

https://reviews.llvm.org/D56571



___
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

2019-02-04 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 added inline comments.



Comment at: lib/AST/Stmt.cpp:457-460
   this->NumOutputs = NumOutputs;
   this->NumInputs = NumInputs;
   this->NumClobbers = NumClobbers;
+  this->NumLabels = NumLabels;

rsmith wrote:
> Please assert that you don't have both outputs and labels here. (If you did, 
> you would assign them the same slots within `Exprs`.)
> 
> You also seem to be missing `Sema` enforcement of the rule that you cannot 
> have both outputs and labels. (If you want to actually support that as an 
> intentional extension to the GCC functionality, you should change the 
> implementation of `GCCAsmStmt` to use different slots for outputs and labels, 
> add some tests, and add a `-Wgcc-compat` warning for that case. Seems better 
> to just add an error for it for now.)
This is enforcement during the parer.  

for example:
a.c:12:23: error: expected ':'
asm goto ("decl %0;" :"a": "m"(cond) : "memory" );

Do you think this is enough for now?
Thanks.
Jennifer



Comment at: lib/Analysis/CFG.cpp:1445-1458
+  LabelMapTy::iterator LI = LabelMap.find(G->getLabel());;
+  // If there is no target for the goto, then we are looking at an
+  // incomplete AST.  Handle this by not registering a successor.
+  if (LI == LabelMap.end()) continue;
 
-JumpTarget JT = LI->second;
-prependAutomaticObjLifetimeWithTerminator(B, I->scopePosition,
-  JT.scopePosition);
-prependAutomaticObjDtorsWithTerminator(B, I->scopePosition,
-   JT.scopePosition);
-const VarDecl *VD = prependAutomaticObjScopeEndWithTerminator(
-B, I->scopePosition, JT.scopePosition);
-appendScopeBegin(JT.block, VD, G);
-addSuccessor(B, JT.block);
+  JumpTarget JT = LI->second;
+  prependAutomaticObjLifetimeWithTerminator(B, I->scopePosition,

rsmith wrote:
> Please factor out this duplicated code into a local lambda rather than 
> copy-pasting it.
Thanks.  Changed.



Comment at: lib/Analysis/CFG.cpp:1461
+if (auto *G = dyn_cast(B->getTerminator())) {
+  if (G->isAsmGoto()) {
+for (auto *E : G->labels()) {

rsmith wrote:
> This check is redundant; there will be no labels if it's not an `asm goto` so 
> you can just perform the below loop unconditionally.
Okay removed.  Thanks.



Comment at: lib/Analysis/CFG.cpp:3139-3141
+  // We will need to backpatch this block later.
+  BackpatchBlocks.push_back(JumpSource(Block, ScopePos));
+  return Block;

rsmith wrote:
> Do we not need the handling for the other labels if we hit this condition for 
> one label?
It is handled in backpatch processing which go though each label and build CFG.



Comment at: lib/Sema/JumpDiagnostics.cpp:347
+  LabelAndGotoScopes[S] = ParentScope;
+  Jumps.push_back(S);
+}

efriedma wrote:
> jyu2 wrote:
> > efriedma wrote:
> > > This doesn't look right; I think we need to add it to IndirectJumps 
> > > instead.  This probably impacts a testcase like the following:
> > > 
> > > ```
> > > struct S { ~S(); };
> > > int f() {
> > >   {
> > > S s;
> > > asm goto(""BAR);
> > > return 1;
> > >   }
> > > BAR:
> > >   return 0;
> > > }
> > > ```
> > > 
> > > (gcc currently accepts this and skips running the destructor, but I'm 
> > > pretty sure that's a bug.)
> > Hi Eli,
> > I see both g++ and clang++ with my change call ~S.  Am I missing something? 
> >  Thanks.
> > 
> > 1>clang++ j.cpp
> > 1>./a.out
> > ~S()
> > 1>g++ j.cpp
> > 1>./a.out
> > ~S()
> > 
> > Here is the test case:
> > 1>cat j.cpp
> > extern "C" int printf (const char *,...);
> > struct S { ~S() {printf("~S()\n");}; };
> > int f() {
> >   {
> > S s;
> > asm goto(""BAR);
> > return 1;
> >   }
> > BAR:
> >   return 0;
> > }
> > 
> > int main()
> > {
> >   f();
> > }
> > 
> Oh, sorry, I wasn't paying attention to the actual contents of the asm.  If 
> you modify the asm goto slightly, to `asm goto("jmp %0"BAR);`, you'll see 
> that the destructor doesn't run, at least with gcc.  (This is different from 
> the way `goto BAR;` works.)
Thanks.  I see, the cleanup code is not generated for asm goto.

For clang with our current implementation, llvm emit error for this.  But the 
error message isn't clear.

You are right, we should emit error in clang.  I will look into tomorrow. 

Thanks.



Comment at: lib/Sema/SemaStmtAsm.cpp:656
+  // Check for duplicate asm operand name between input, output and label 
lists.
+  typedef std::pair  MyItemType;
+  SmallVector MyList;

rsmith wrote:
> No space after `pair`, please.
Sorry, changed.



Comment at: lib/Sema/SemaStmtAsm.cpp:659-662
+  for (unsigned i = 0, e = NumOutputs + NumInputs + NumLabels; i != e; ++i)
+if (NS->getIdentifier(i) && 

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

2019-02-04 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 updated this revision to Diff 185206.
jyu2 marked 8 inline comments as done.
jyu2 added a comment.

More review comments addressed.


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

https://reviews.llvm.org/D56571

Files:
  include/clang/AST/Stmt.h
  include/clang/Basic/DiagnosticASTKinds.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.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,18 @@
 
 // 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 goto statement to its label}}
+  // expected-error@+1 {{cannot jump from this goto statement to its label}}
+  asm volatile goto("testl %0, %0; jne %l1;" :: "r"(n)::label_true, loop);
+  // expected-note@+1 {{jump bypasses initialization of variable length array}}
+  return ({int a[n];label_true: 2;});
+  // expected-note@+1 {{jump bypasses initialization of variable length array}}
+  int b[n];
+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,28 @@
   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 {{operand with 'l' modifier must refer to a label}}
+  asm ("jne %l0"::"r"(&));
+  // expected-error@+1 {{invalid operand number in inline asm string}}
+  asm ("jne %l0":::);
+  // expected-error@+1 {{operand with 'l' modifier must refer to a label}}
+  asm goto ("jne %l0"::"r"(x)::lab);
+  asm goto ("jne %l0"lab);
+}
Index: test/Parser/asm.cpp
===
--- test/Parser/asm.cpp
+++ test/Parser/asm.cpp
@@ -7,3 +7,54 @@
 int foo5 asm (U"bar5"); // expected-error {{cannot use unicode string literal in 'asm'}}
 int foo6 asm ("bar6"_x); // expected-error {{string literal with user-defined suffix cannot be used here}}
 int foo6 asm ("" L"bar7"); // expected-error {{cannot use wide string literal in 'asm'}}
+
+int zoo ()
+{
+  int x,cond,*e;
+  // expected-error@+1 {{expected ')'}}
+  asm ("mov %[e], %[e]" : : [e] "rm" (*e)::a)
+  // expected-error@+1  {{expected ':'}}
+  asm goto ("decl %0; jnz %l[a]" :"=r"(x): "m"(x) : "memory" : a);
+  // expected-error@+1 {{expected identifie}}
+  asm goto ("decl %0;" :: "m"(x) : "memory" : );
+  // expected-error@+1  {{expected ':'}}
+  asm goto ("decl %0;" :: "m"(x) : "memory" );
+  // expected-error@+1 {{use of undeclared label 'x'}}
+  asm goto ("decl %0;" :: "m"(x) : "memory" :x);
+  // expected-error@+1 {{use of undeclared label 'b'}}
+  asm goto ("decl %0;" :: "m"(x) : "memory" :b);
+  // expected-error@+1 {{invalid operand number in inline asm string}}
+  asm goto ("testl %0, %0; jne %l3;" :: "r"(cond)::label_true, loop)
+  // expected-error@+1 {{unknown symbolic operand name in inline assembly string}}
+  asm goto 

[PATCH] D57722: [Clang][NCF] Sanitizer options: helpers to check if {Kernel, Hardware}ASan is enabled

2019-02-04 Thread Evgenii Stepanov via Phabricator via cfe-commits
eugenis added a comment.

I find the current code more readable than with this change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57722



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


[PATCH] D57642: [X86] Update clobber list in a test after D57641. Remove filter for 'fpsw' in MS inline asm clobber list generation since the backend now uses 'fpsr'.

2019-02-04 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D57642



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


[PATCH] D54429: [analyzer] Creating standard Sphinx documentation

2019-02-04 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

Dunno but made a random guess in the inline comment.




Comment at: docs/analyzer/checkers.rst:1946
+Checkers used for debugging the analyzer.
+:doc:`DebugChecks` page contains a detailed description.
+

The error is on this line. It might be that this link should mention the 
directory (something like `developer-docs/DebugChecks`). Previously 
`DebugChecks` was on the top level, so it wasn't necessary.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54429



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


[PATCH] D54429: [analyzer] Creating standard Sphinx documentation

2019-02-04 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

I'm getting a single buildbot failure, but I'm a little unsure how to fix it. 
Could you help?

http://lab.llvm.org:8011/builders/clang-sphinx-docs/builds/36985

  24.610 [1/1/1] Generating html Sphinx documentation for clang into 
"/home/buildbot/llvm-build-dir/clang-sphinx-docs/llvm/build/tools/clang/docs/html"
  FAILED: tools/clang/docs/CMakeFiles/docs-clang-html 
  cd 
/home/buildbot/llvm-build-dir/clang-sphinx-docs/llvm/build/tools/clang/docs && 
/usr/bin/sphinx-build -b html -d 
/home/buildbot/llvm-build-dir/clang-sphinx-docs/llvm/build/tools/clang/docs/_doctrees-clang-html
 -q -W 
/home/buildbot/llvm-build-dir/clang-sphinx-docs/llvm/src/tools/clang/docs 
/home/buildbot/llvm-build-dir/clang-sphinx-docs/llvm/build/tools/clang/docs/html
  
  Warning, treated as error:
  
/home/buildbot/llvm-build-dir/clang-sphinx-docs/llvm/src/tools/clang/docs/analyzer/checkers.rst:1945:
 WARNING: unknown document: DebugChecks


Repository:
  rC Clang

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

https://reviews.llvm.org/D54429



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


[PATCH] D57642: [X86] Update clobber list in a test after D57641. Remove filter for 'fpsw' in MS inline asm clobber list generation since the backend now uses 'fpsr'.

2019-02-04 Thread Craig Topper via Phabricator via cfe-commits
craig.topper updated this revision to Diff 185201.
craig.topper added a comment.

Keep the filter on fpsw, but change the string to fpsr


Repository:
  rC Clang

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

https://reviews.llvm.org/D57642

Files:
  lib/Parse/ParseStmtAsm.cpp


Index: lib/Parse/ParseStmtAsm.cpp
===
--- lib/Parse/ParseStmtAsm.cpp
+++ lib/Parse/ParseStmtAsm.cpp
@@ -636,7 +636,7 @@
   // Filter out "fpsw" and "mxcsr". They aren't valid GCC asm clobber
   // constraints. Clang always adds fpsr to the clobber list anyway.
   llvm::erase_if(Clobbers, [](const std::string ) {
-return C == "fpsw" || C == "mxcsr";
+return C == "fpsr" || C == "mxcsr";
   });
 
   // Build the vector of clobber StringRefs.


Index: lib/Parse/ParseStmtAsm.cpp
===
--- lib/Parse/ParseStmtAsm.cpp
+++ lib/Parse/ParseStmtAsm.cpp
@@ -636,7 +636,7 @@
   // Filter out "fpsw" and "mxcsr". They aren't valid GCC asm clobber
   // constraints. Clang always adds fpsr to the clobber list anyway.
   llvm::erase_if(Clobbers, [](const std::string ) {
-return C == "fpsw" || C == "mxcsr";
+return C == "fpsr" || C == "mxcsr";
   });
 
   // Build the vector of clobber StringRefs.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

2019-02-04 Thread Dominic Ferreira via Phabricator via cfe-commits
domdom updated this revision to Diff 185196.
domdom added a comment.

Switched  getIndexOfLastNonNullStmt to return Optional instead of 
Stmt* as per comments.


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

https://reviews.llvm.org/D57086

Files:
  clang/include/clang/AST/Stmt.h
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/AST/ast-dump-stmt.c
  clang/test/CodeGen/exprs.c
  clang/test/Sema/statements.c

Index: clang/test/Sema/statements.c
===
--- clang/test/Sema/statements.c
+++ clang/test/Sema/statements.c
@@ -119,3 +119,17 @@
 SIZE = sizeof(({unsigned long __ptr; __ptr;}))
   };
 }
+
+// GCC ignores empty statements at the end of compound expressions where the
+// result type is concerned.
+void test13() {
+  int a;
+  a = ({1;});
+  a = ({1;;});
+  a = ({int x = 1; (void)x;}); // expected-error {{assigning to 'int' from incompatible type 'void'}}
+  a = ({int x = 1; (void)x;;}); // expected-error {{assigning to 'int' from incompatible type 'void'}}
+}
+
+void test14() { return ({}); }
+void test15() { return ({}); }
+void test16() { return ({test:;;}); }
Index: clang/test/CodeGen/exprs.c
===
--- clang/test/CodeGen/exprs.c
+++ clang/test/CodeGen/exprs.c
@@ -196,3 +196,13 @@
 }
 // CHECK-LABEL: define void @f18()
 // CHECK: call i32 @returns_int()
+
+// Ensure the right stmt is returned
+int f19() {
+  return ({ 3;;4;; });
+}
+// CHECK-LABEL: define i32 @f19()
+// CHECK: [[T:%.*]] = alloca i32
+// CHECK: store i32 4, i32* [[T]]
+// CHECK: [[L:%.*]] = load i32, i32* [[T]]
+// CHECK: ret i32 [[L]]
Index: clang/test/AST/ast-dump-stmt.c
===
--- clang/test/AST/ast-dump-stmt.c
+++ clang/test/AST/ast-dump-stmt.c
@@ -372,4 +372,14 @@
   // CHECK-NEXT: IntegerLiteral 0x{{[^ ]*}}  'int' 10
   // CHECK-NEXT: ImplicitCastExpr
   // CHECK-NEXT: DeclRefExpr 0x{{[^ ]*}}  'int' lvalue Var 0x{{[^ ]*}} 'a' 'int'
+  ({int a = 10; a;;;});
+  // CHECK-NEXT: StmtExpr 0x{{[^ ]*}}  'int'
+  // CHECK-NEXT: CompoundStmt
+  // CHECK-NEXT: DeclStmt
+  // CHECK-NEXT: VarDecl 0x{{[^ ]*}}  col:9 used a 'int' cinit
+  // CHECK-NEXT: IntegerLiteral 0x{{[^ ]*}}  'int' 10
+  // CHECK-NEXT: ImplicitCastExpr
+  // CHECK-NEXT: DeclRefExpr 0x{{[^ ]*}}  'int' lvalue Var 0x{{[^ ]*}} 'a' 'int'
+  // CHECK-NEXT: NullStmt
+  // CHECK-NEXT: NullStmt
 }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -13320,11 +13320,12 @@
   // More semantic analysis is needed.
 
   // If there are sub-stmts in the compound stmt, take the type of the last one
-  // as the type of the stmtexpr.
+  // as the type of the stmtexpr. For GCC compatibility this excludes trailing
+  // NullStmts.
   QualType Ty = Context.VoidTy;
   bool StmtExprMayBindToTemp = false;
   if (!Compound->body_empty()) {
-Stmt *LastStmt = Compound->body_back();
+Stmt *LastStmt = Compound->getStmtExprResult();
 LabelStmt *LastLabelStmt = nullptr;
 // If LastStmt is a label, skip down through into the body.
 while (LabelStmt *Label = dyn_cast(LastStmt)) {
@@ -13360,7 +13361,7 @@
   return ExprError();
 if (LastExpr.get() != nullptr) {
   if (!LastLabelStmt)
-Compound->setLastStmt(LastExpr.get());
+Compound->setStmtExpr(LastExpr.get());
   else
 LastLabelStmt->setSubStmt(LastExpr.get());
   StmtExprMayBindToTemp = true;
Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -959,10 +959,16 @@
 
 bool Parser::isExprValueDiscarded() {
   if (Actions.isCurCompoundStmtAStmtExpr()) {
-// Look to see if the next two tokens close the statement expression;
-// if so, this expression statement is the last statement in a
-// statment expression.
-return Tok.isNot(tok::r_brace) || NextToken().isNot(tok::r_paren);
+// For GCC compatibility we skip past NullStmts.
+unsigned LookAhead = 0;
+while (GetLookAheadToken(LookAhead).is(tok::semi)) {
+  ++LookAhead;
+}
+// Then look to see if the next two tokens close the statement expression;
+// if so, this expression statement is the last statement in a statment
+// expression.
+return GetLookAheadToken(LookAhead).isNot(tok::r_brace) ||
+   GetLookAheadToken(LookAhead + 1).isNot(tok::r_paren);
   }
   return true;
 }
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -382,36 +382,40 @@
   bool GetLast,
  

[PATCH] D57636: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg

2019-02-04 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

In test/CodeGen/arm64-microsoft-status-reg.cpp, you can write something like 
`// CHECK-IR: %[[VAR:.*]] = call i64 @llvm.read_register.i64(metadata 
![[MD2:.*]])`, then `// CHECK-IR-NEXT: store i64 %[[VAR]]` on the next line. 
See also http://llvm.org/docs/CommandGuide/FileCheck.html .


Repository:
  rC Clang

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

https://reviews.llvm.org/D57636



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


[PATCH] D57722: [Clang][NCF] Sanitizer options: helpers to check if {Kernel, Hardware}ASan is enabled

2019-02-04 Thread Vedant Kumar via Phabricator via cfe-commits
vsk added reviewers: eugenis, kcc.
vsk added inline comments.



Comment at: clang/include/clang/Basic/Sanitizers.h:89
+  /// * Hardware Kernel ASan
+  bool hasASanOrKASanOrHWASanOrHWKASan() const {
+return hasASanOrKASan() || hasHWASanOrHWKASan();

Sorry to bike-shed, but this seems a bit hard to read. How about: 'hasAnyASan'?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57722



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


[PATCH] D57636: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg

2019-02-04 Thread Mike Hommey via Phabricator via cfe-commits
glandium added a comment.

In D57636#1383566 , @efriedma wrote:

> (It should be possible to check that we aren't inserting incorrect 
> truncation/extension operations in the IR.)


I don't know how to do that.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57636



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


[PATCH] D57722: [Clang][NCF] Sanitizer options: helpers to check if {Kernel, Hardware}ASan is enabled

2019-02-04 Thread Julian Lettner via Phabricator via cfe-commits
yln created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Kernel ASan is basically ASan with a few specific settings, but has a
seperate front-end flag. Provide a helper that checks for both, so new
comitters like myself are less likely to forget the Kernel variant.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D57722

Files:
  clang/include/clang/Basic/Features.def
  clang/include/clang/Basic/Sanitizers.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/SanitizerMetadata.cpp

Index: clang/lib/CodeGen/SanitizerMetadata.cpp
===
--- clang/lib/CodeGen/SanitizerMetadata.cpp
+++ clang/lib/CodeGen/SanitizerMetadata.cpp
@@ -24,10 +24,7 @@
SourceLocation Loc, StringRef Name,
QualType Ty, bool IsDynInit,
bool IsBlacklisted) {
-  if (!CGM.getLangOpts().Sanitize.hasOneOf(SanitizerKind::Address |
-   SanitizerKind::KernelAddress |
-   SanitizerKind::HWAddress |
-   SanitizerKind::KernelHWAddress))
+  if (!CGM.getLangOpts().Sanitize.hasASanOrKASanOrHWASanOrHWKASan())
 return;
   IsDynInit &= !CGM.isInSanitizerBlacklist(GV, Loc, Ty, "init");
   IsBlacklisted |= CGM.isInSanitizerBlacklist(GV, Loc, Ty);
@@ -58,10 +55,7 @@
 
 void SanitizerMetadata::reportGlobalToASan(llvm::GlobalVariable *GV,
const VarDecl , bool IsDynInit) {
-  if (!CGM.getLangOpts().Sanitize.hasOneOf(SanitizerKind::Address |
-   SanitizerKind::KernelAddress |
-   SanitizerKind::HWAddress |
-   SanitizerKind::KernelHWAddress))
+  if (!CGM.getLangOpts().Sanitize.hasASanOrKASanOrHWASanOrHWKASan())
 return;
   std::string QualName;
   llvm::raw_string_ostream OS(QualName);
@@ -78,10 +72,7 @@
 void SanitizerMetadata::disableSanitizerForGlobal(llvm::GlobalVariable *GV) {
   // For now, just make sure the global is not modified by the ASan
   // instrumentation.
-  if (CGM.getLangOpts().Sanitize.hasOneOf(SanitizerKind::Address |
-  SanitizerKind::KernelAddress |
-  SanitizerKind::HWAddress |
-  SanitizerKind::KernelHWAddress))
+  if (CGM.getLangOpts().Sanitize.hasASanOrKASanOrHWASanOrHWKASan())
 reportGlobalToASan(GV, SourceLocation(), "", QualType(), false, true);
 }
 
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -876,9 +876,9 @@
   }
 
   // Apply sanitizer attributes to the function.
-  if (SanOpts.hasOneOf(SanitizerKind::Address | SanitizerKind::KernelAddress))
+  if (SanOpts.hasASanOrKASan())
 Fn->addFnAttr(llvm::Attribute::SanitizeAddress);
-  if (SanOpts.hasOneOf(SanitizerKind::HWAddress | SanitizerKind::KernelHWAddress))
+  if (SanOpts.hasHWASanOrHWKASan())
 Fn->addFnAttr(llvm::Attribute::SanitizeHWAddress);
   if (SanOpts.has(SanitizerKind::Thread))
 Fn->addFnAttr(llvm::Attribute::SanitizeThread);
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -4403,8 +4403,7 @@
 
   // Avoid incompatibility with ASan which relies on the `noreturn`
   // attribute to insert handler calls.
-  if (SanOpts.hasOneOf(SanitizerKind::Address |
-   SanitizerKind::KernelAddress)) {
+  if (SanOpts.hasASanOrKASan()) {
 SanitizerScope SanScope(this);
 llvm::IRBuilder<>::InsertPointGuard IPGuard(Builder);
 Builder.SetInsertPoint(CI);
Index: clang/include/clang/Basic/Sanitizers.h
===
--- clang/include/clang/Basic/Sanitizers.h
+++ clang/include/clang/Basic/Sanitizers.h
@@ -70,6 +70,25 @@
 
   /// Bitmask of enabled sanitizers.
   SanitizerMask Mask = 0;
+
+  /// Check if ASan or Kernel ASan are enabled.
+  bool hasASanOrKASan() const {
+return hasOneOf(SanitizerKind::Address | SanitizerKind::KernelAddress);
+  }
+
+  /// Check if Hardware ASan or Hardware Kernel ASan are enabled.
+  bool hasHWASanOrHWKASan() const {
+return hasOneOf(SanitizerKind::HWAddress | SanitizerKind::KernelHWAddress);
+  }
+
+  /// Check if any of the following are enabled:
+  /// * ASan
+  /// * Kernel ASan
+  /// * Hardware ASan
+  /// * Hardware Kernel ASan
+  bool hasASanOrKASanOrHWASanOrHWKASan() const {
+return hasASanOrKASan() || hasHWASanOrHWKASan();
+  }
 };
 
 /// Parse a 

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

2019-02-04 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments.



Comment at: lib/Sema/JumpDiagnostics.cpp:347
+  LabelAndGotoScopes[S] = ParentScope;
+  Jumps.push_back(S);
+}

jyu2 wrote:
> efriedma wrote:
> > This doesn't look right; I think we need to add it to IndirectJumps 
> > instead.  This probably impacts a testcase like the following:
> > 
> > ```
> > struct S { ~S(); };
> > int f() {
> >   {
> > S s;
> > asm goto(""BAR);
> > return 1;
> >   }
> > BAR:
> >   return 0;
> > }
> > ```
> > 
> > (gcc currently accepts this and skips running the destructor, but I'm 
> > pretty sure that's a bug.)
> Hi Eli,
> I see both g++ and clang++ with my change call ~S.  Am I missing something?  
> Thanks.
> 
> 1>clang++ j.cpp
> 1>./a.out
> ~S()
> 1>g++ j.cpp
> 1>./a.out
> ~S()
> 
> Here is the test case:
> 1>cat j.cpp
> extern "C" int printf (const char *,...);
> struct S { ~S() {printf("~S()\n");}; };
> int f() {
>   {
> S s;
> asm goto(""BAR);
> return 1;
>   }
> BAR:
>   return 0;
> }
> 
> int main()
> {
>   f();
> }
> 
Oh, sorry, I wasn't paying attention to the actual contents of the asm.  If you 
modify the asm goto slightly, to `asm goto("jmp %0"BAR);`, you'll see that 
the destructor doesn't run, at least with gcc.  (This is different from the way 
`goto BAR;` works.)


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

https://reviews.llvm.org/D56571



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


[PATCH] D57711: [Sanitizers] UBSan unreachable incompatible with Kernel ASan

2019-02-04 Thread Julian Lettner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC353120: [Sanitizers] UBSan unreachable incompatible with 
Kernel ASan (authored by yln, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57711?vs=185153=185178#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D57711

Files:
  lib/CodeGen/CGCall.cpp
  test/CodeGen/ubsan-asan-noreturn.c


Index: test/CodeGen/ubsan-asan-noreturn.c
===
--- test/CodeGen/ubsan-asan-noreturn.c
+++ test/CodeGen/ubsan-asan-noreturn.c
@@ -1,6 +1,7 @@
 // Ensure compatiblity of UBSan unreachable with ASan in the presence of
 // noreturn functions.
-// RUN: %clang_cc1 -fsanitize=unreachable,address -triple x86_64-linux 
-emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=unreachable,address-triple x86_64-linux 
-emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=unreachable,kernel-address -triple x86_64-linux 
-emit-llvm -o - %s | FileCheck %s
 
 void my_longjmp(void) __attribute__((noreturn));
 
Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -4403,7 +4403,8 @@
 
   // Avoid incompatibility with ASan which relies on the `noreturn`
   // attribute to insert handler calls.
-  if (SanOpts.has(SanitizerKind::Address)) {
+  if (SanOpts.hasOneOf(SanitizerKind::Address |
+   SanitizerKind::KernelAddress)) {
 SanitizerScope SanScope(this);
 llvm::IRBuilder<>::InsertPointGuard IPGuard(Builder);
 Builder.SetInsertPoint(CI);


Index: test/CodeGen/ubsan-asan-noreturn.c
===
--- test/CodeGen/ubsan-asan-noreturn.c
+++ test/CodeGen/ubsan-asan-noreturn.c
@@ -1,6 +1,7 @@
 // Ensure compatiblity of UBSan unreachable with ASan in the presence of
 // noreturn functions.
-// RUN: %clang_cc1 -fsanitize=unreachable,address -triple x86_64-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=unreachable,address-triple x86_64-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=unreachable,kernel-address -triple x86_64-linux -emit-llvm -o - %s | FileCheck %s
 
 void my_longjmp(void) __attribute__((noreturn));
 
Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -4403,7 +4403,8 @@
 
   // Avoid incompatibility with ASan which relies on the `noreturn`
   // attribute to insert handler calls.
-  if (SanOpts.has(SanitizerKind::Address)) {
+  if (SanOpts.hasOneOf(SanitizerKind::Address |
+   SanitizerKind::KernelAddress)) {
 SanitizerScope SanScope(this);
 llvm::IRBuilder<>::InsertPointGuard IPGuard(Builder);
 Builder.SetInsertPoint(CI);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r353120 - [Sanitizers] UBSan unreachable incompatible with Kernel ASan

2019-02-04 Thread Julian Lettner via cfe-commits
Author: yln
Date: Mon Feb  4 15:37:50 2019
New Revision: 353120

URL: http://llvm.org/viewvc/llvm-project?rev=353120=rev
Log:
[Sanitizers] UBSan unreachable incompatible with Kernel ASan

Summary:
This is a follow up for https://reviews.llvm.org/D57278. The previous
revision should have also included Kernel ASan.

rdar://problem/40723397

Subscribers: cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/test/CodeGen/ubsan-asan-noreturn.c

Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=353120=353119=353120=diff
==
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Mon Feb  4 15:37:50 2019
@@ -4403,7 +4403,8 @@ RValue CodeGenFunction::EmitCall(const C
 
   // Avoid incompatibility with ASan which relies on the `noreturn`
   // attribute to insert handler calls.
-  if (SanOpts.has(SanitizerKind::Address)) {
+  if (SanOpts.hasOneOf(SanitizerKind::Address |
+   SanitizerKind::KernelAddress)) {
 SanitizerScope SanScope(this);
 llvm::IRBuilder<>::InsertPointGuard IPGuard(Builder);
 Builder.SetInsertPoint(CI);

Modified: cfe/trunk/test/CodeGen/ubsan-asan-noreturn.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ubsan-asan-noreturn.c?rev=353120=353119=353120=diff
==
--- cfe/trunk/test/CodeGen/ubsan-asan-noreturn.c (original)
+++ cfe/trunk/test/CodeGen/ubsan-asan-noreturn.c Mon Feb  4 15:37:50 2019
@@ -1,6 +1,7 @@
 // Ensure compatiblity of UBSan unreachable with ASan in the presence of
 // noreturn functions.
-// RUN: %clang_cc1 -fsanitize=unreachable,address -triple x86_64-linux 
-emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=unreachable,address-triple x86_64-linux 
-emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=unreachable,kernel-address -triple x86_64-linux 
-emit-llvm -o - %s | FileCheck %s
 
 void my_longjmp(void) __attribute__((noreturn));
 


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


[PATCH] D56555: Add Attribute to define nonlazy objc classes

2019-02-04 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL353116: [OBJC] Add attribute to mark Objective C class as 
non-lazy (authored by joseph_daniels, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D56555?vs=184671=185177#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D56555

Files:
  cfe/trunk/include/clang/Basic/Attr.td
  cfe/trunk/include/clang/Basic/AttrDocs.td
  cfe/trunk/lib/CodeGen/CGObjCMac.cpp
  cfe/trunk/lib/Sema/SemaDeclAttr.cpp
  cfe/trunk/test/CodeGenObjC/non-lazy-classes.m
  cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
  cfe/trunk/test/SemaObjC/attr-objc-non-lazy.m

Index: cfe/trunk/include/clang/Basic/AttrDocs.td
===
--- cfe/trunk/include/clang/Basic/AttrDocs.td
+++ cfe/trunk/include/clang/Basic/AttrDocs.td
@@ -3699,6 +3699,19 @@
   }];
 }
 
+def ObjCNonLazyClassDocs : Documentation {
+  let Category = DocCatType;
+  let Content = [{
+This attribute can be added to an Objective-C ``@interface`` declaration to
+add the class to the list of non-lazily initialized classes. A non-lazy class
+will be initialized eagerly when the Objective-C runtime is loaded.  This is
+required for certain system classes which have instances allocated in
+non-standard ways, such as the classes for blocks and constant strings.  Adding
+this attribute is essentially equivalent to providing a trivial `+load` method 
+but avoids the (fairly small) load-time overheads associated with defining and
+calling such a method.
+  }];
+}
 
 def SelectAnyDocs : Documentation {
   let Category = DocCatType;
Index: cfe/trunk/include/clang/Basic/Attr.td
===
--- cfe/trunk/include/clang/Basic/Attr.td
+++ cfe/trunk/include/clang/Basic/Attr.td
@@ -1758,6 +1758,13 @@
   let Documentation = [Undocumented];
 }
 
+def ObjCNonLazyClass : Attr {
+  let Spellings = [Clang<"objc_nonlazy_class">];
+  let Subjects = SubjectList<[ObjCInterface], ErrorDiag>;
+  let LangOpts = [ObjC];
+  let Documentation = [ObjCNonLazyClassDocs];
+}
+
 def ObjCSubclassingRestricted : InheritableAttr {
   let Spellings = [Clang<"objc_subclassing_restricted">];
   let Subjects = SubjectList<[ObjCInterface], ErrorDiag>;
Index: cfe/trunk/test/CodeGenObjC/non-lazy-classes.m
===
--- cfe/trunk/test/CodeGenObjC/non-lazy-classes.m
+++ cfe/trunk/test/CodeGenObjC/non-lazy-classes.m
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -triple x86_64-apple-darwin10 -Wno-objc-root-class -emit-llvm -o - %s | \
 // RUN: FileCheck %s
-// CHECK: @"OBJC_LABEL_NONLAZY_CLASS_$" = private global [1 x {{.*}}] {{.*}}@"OBJC_CLASS_$_A"{{.*}}, section "__DATA,__objc_nlclslist,regular,no_dead_strip", align 8
+// CHECK: @"OBJC_LABEL_NONLAZY_CLASS_$" = private global [2 x {{.*}}]{{.*}}@"OBJC_CLASS_$_A"{{.*}},{{.*}}@"OBJC_CLASS_$_D"{{.*}} section "__DATA,__objc_nlclslist,regular,no_dead_strip", align 8
 // CHECK: @"OBJC_LABEL_NONLAZY_CATEGORY_$" = private global [1 x {{.*}}] {{.*}}@"\01l_OBJC_$_CATEGORY_A_$_Cat"{{.*}}, section "__DATA,__objc_nlcatlist,regular,no_dead_strip", align 8
 
 @interface A @end
@@ -30,3 +30,8 @@
 @interface C : A @end
 @implementation C
 @end
+
+__attribute__((objc_nonlazy_class))
+@interface D @end
+
+@implementation D @end
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
@@ -100,6 +100,7 @@
 // CHECK-NEXT: ObjCExplicitProtocolImpl (SubjectMatchRule_objc_protocol)
 // CHECK-NEXT: ObjCExternallyRetained (SubjectMatchRule_variable_not_is_parameter, SubjectMatchRule_function, SubjectMatchRule_block, SubjectMatchRule_objc_method)
 // CHECK-NEXT: ObjCMethodFamily (SubjectMatchRule_objc_method)
+// CHECK-NEXT: ObjCNonLazyClass (SubjectMatchRule_objc_interface)
 // CHECK-NEXT: ObjCPreciseLifetime (SubjectMatchRule_variable)
 // CHECK-NEXT: ObjCRequiresPropertyDefs (SubjectMatchRule_objc_interface)
 // CHECK-NEXT: ObjCRequiresSuper (SubjectMatchRule_objc_method)
Index: cfe/trunk/test/SemaObjC/attr-objc-non-lazy.m
===
--- cfe/trunk/test/SemaObjC/attr-objc-non-lazy.m
+++ cfe/trunk/test/SemaObjC/attr-objc-non-lazy.m
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -verify -Wno-objc-root-class  -fsyntax-only  %s
+
+__attribute__((objc_nonlazy_class))
+@interface A
+@end
+@implementation A
+@end
+
+__attribute__((objc_nonlazy_class)) int X; // expected-error {{'objc_nonlazy_class' attribute only applies to Objective-C interfaces}}
+
+__attribute__((objc_nonlazy_class()))
+@interface B
+@end

[PATCH] D57712: [SemaObjC] Don't infer the availabilty of messages to +new from -init if the receiver has Class type

2019-02-04 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC353115: [SemaObjC] Dont infer the availabilty of +new 
from -init if the receiver has… (authored by epilk, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57712?vs=185150=185175#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D57712

Files:
  lib/Sema/SemaExprObjC.cpp
  test/SemaObjC/infer-availability-from-init.m


Index: test/SemaObjC/infer-availability-from-init.m
===
--- test/SemaObjC/infer-availability-from-init.m
+++ test/SemaObjC/infer-availability-from-init.m
@@ -56,3 +56,16 @@
   [self new];
 }
 @end
+
+@interface NoInit : NSObject
+-(instancetype)init __attribute__((unavailable)); // expected-note {{'init' 
has been explicitly marked unavailable here}}
+@end
+
+@interface NoInitSub : NoInit @end
+
+@implementation NoInitSub
+-(void)meth:(Class)c {
+  [c new]; // No error; unknown interface.
+  [NoInitSub new]; // expected-error {{'new' is unavailable}}
+}
+@end
Index: lib/Sema/SemaExprObjC.cpp
===
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -2805,8 +2805,8 @@
   } else {
 if (ObjCMethodDecl *CurMeth = getCurMethodDecl()) {
   if (ObjCInterfaceDecl *ClassDecl = CurMeth->getClassInterface()) {
-// FIXME: Is this correct? Why are we assuming that a message to
-// Class will call a method in the current interface?
+// As a guess, try looking for the method in the current interface.
+// This very well may not produce the "right" method.
 
 // First check the public methods in the class interface.
 Method = ClassDecl->lookupClassMethod(Sel);
@@ -2814,8 +2814,7 @@
 if (!Method)
   Method = ClassDecl->lookupPrivateClassMethod(Sel);
 
-if (Method && DiagnoseUseOfDecl(Method, SelectorSlotLocs, nullptr,
-false, false, ClassDecl))
+if (Method && DiagnoseUseOfDecl(Method, SelectorSlotLocs))
   return ExprError();
   }
 }


Index: test/SemaObjC/infer-availability-from-init.m
===
--- test/SemaObjC/infer-availability-from-init.m
+++ test/SemaObjC/infer-availability-from-init.m
@@ -56,3 +56,16 @@
   [self new];
 }
 @end
+
+@interface NoInit : NSObject
+-(instancetype)init __attribute__((unavailable)); // expected-note {{'init' has been explicitly marked unavailable here}}
+@end
+
+@interface NoInitSub : NoInit @end
+
+@implementation NoInitSub
+-(void)meth:(Class)c {
+  [c new]; // No error; unknown interface.
+  [NoInitSub new]; // expected-error {{'new' is unavailable}}
+}
+@end
Index: lib/Sema/SemaExprObjC.cpp
===
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -2805,8 +2805,8 @@
   } else {
 if (ObjCMethodDecl *CurMeth = getCurMethodDecl()) {
   if (ObjCInterfaceDecl *ClassDecl = CurMeth->getClassInterface()) {
-// FIXME: Is this correct? Why are we assuming that a message to
-// Class will call a method in the current interface?
+// As a guess, try looking for the method in the current interface.
+// This very well may not produce the "right" method.
 
 // First check the public methods in the class interface.
 Method = ClassDecl->lookupClassMethod(Sel);
@@ -2814,8 +2814,7 @@
 if (!Method)
   Method = ClassDecl->lookupPrivateClassMethod(Sel);
 
-if (Method && DiagnoseUseOfDecl(Method, SelectorSlotLocs, nullptr,
-false, false, ClassDecl))
+if (Method && DiagnoseUseOfDecl(Method, SelectorSlotLocs))
   return ExprError();
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r353115 - [SemaObjC] Don't infer the availabilty of +new from -init if the receiver has Class type

2019-02-04 Thread Erik Pilkington via cfe-commits
Author: epilk
Date: Mon Feb  4 15:30:57 2019
New Revision: 353115

URL: http://llvm.org/viewvc/llvm-project?rev=353115=rev
Log:
[SemaObjC] Don't infer the availabilty of +new from -init if the receiver has 
Class type

rdar://47713266

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

Modified:
cfe/trunk/lib/Sema/SemaExprObjC.cpp
cfe/trunk/test/SemaObjC/infer-availability-from-init.m

Modified: cfe/trunk/lib/Sema/SemaExprObjC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprObjC.cpp?rev=353115=353114=353115=diff
==
--- cfe/trunk/lib/Sema/SemaExprObjC.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprObjC.cpp Mon Feb  4 15:30:57 2019
@@ -2805,8 +2805,8 @@ ExprResult Sema::BuildInstanceMessage(Ex
   } else {
 if (ObjCMethodDecl *CurMeth = getCurMethodDecl()) {
   if (ObjCInterfaceDecl *ClassDecl = CurMeth->getClassInterface()) {
-// FIXME: Is this correct? Why are we assuming that a message to
-// Class will call a method in the current interface?
+// As a guess, try looking for the method in the current interface.
+// This very well may not produce the "right" method.
 
 // First check the public methods in the class interface.
 Method = ClassDecl->lookupClassMethod(Sel);
@@ -2814,8 +2814,7 @@ ExprResult Sema::BuildInstanceMessage(Ex
 if (!Method)
   Method = ClassDecl->lookupPrivateClassMethod(Sel);
 
-if (Method && DiagnoseUseOfDecl(Method, SelectorSlotLocs, nullptr,
-false, false, ClassDecl))
+if (Method && DiagnoseUseOfDecl(Method, SelectorSlotLocs))
   return ExprError();
   }
 }

Modified: cfe/trunk/test/SemaObjC/infer-availability-from-init.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/infer-availability-from-init.m?rev=353115=353114=353115=diff
==
--- cfe/trunk/test/SemaObjC/infer-availability-from-init.m (original)
+++ cfe/trunk/test/SemaObjC/infer-availability-from-init.m Mon Feb  4 15:30:57 
2019
@@ -56,3 +56,16 @@ void usenotmyobject() {
   [self new];
 }
 @end
+
+@interface NoInit : NSObject
+-(instancetype)init __attribute__((unavailable)); // expected-note {{'init' 
has been explicitly marked unavailable here}}
+@end
+
+@interface NoInitSub : NoInit @end
+
+@implementation NoInitSub
+-(void)meth:(Class)c {
+  [c new]; // No error; unknown interface.
+  [NoInitSub new]; // expected-error {{'new' is unavailable}}
+}
+@end


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


[PATCH] D57642: [X86] Update clobber list in a test after D57641. Remove filter for 'fpsw' in MS inline asm clobber list generation since the backend now uses 'fpsr'.

2019-02-04 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments.



Comment at: lib/Parse/ParseStmtAsm.cpp:637-639
   // constraints. Clang always adds fpsr to the clobber list anyway.
   llvm::erase_if(Clobbers, [](const std::string ) {
+return C == "mxcsr";

In light of this comment about "clang always adding fpsr anyway" I'd rather 
keep the fpsr filter here.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57642



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


[PATCH] D57626: Disallow trivial_abi on a class if all copy and move constructors are deleted

2019-02-04 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

In D57626#1382391 , @Quuxplusone wrote:

> I admit that this `lock_guard` example is contrived and generally ill-advised 
> ,
>  but its ill-advisedness seems like a higher-level concern that shouldn't be 
> "enforced" by fiddling with the rules of [[trivial_abi]], so I hope that's 
> not what's going on here.


`[[trivial_abi]]` (at least right now) only affects whether user-provided 
special member functions are considered to be trivial for ABI purposes. A class 
whose copy and move constructor are both deleted is not passed in registers by 
the ABI; that has nothing to do with triviality, so it's unaffected by 
`[[trivial_abi]]` as currently specified. We could "fiddle with the rules of 
[[trivial_abi]]" to *make* that work (that's what the previous approach for 
this case did), but as you note, this is an ill-advised case, and fiddling with 
the rules to give it special behavior doesn't seem like worthwhile complexity. 
Our design intent was to produce a diagnostic if `[[trivial_abi]]` is specified 
on a non-template class that we can't actually pass in registers; this patch 
fixes a hole in our implementation of that design by adding a missing 
diagnostic.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57626



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


[PATCH] D57642: [X86] Update clobber list in a test after D57641. Remove filter for 'fpsw' in MS inline asm clobber list generation since the backend now uses 'fpsr'.

2019-02-04 Thread Craig Topper via Phabricator via cfe-commits
craig.topper marked an inline comment as done.
craig.topper added inline comments.



Comment at: test/CodeGen/ms-inline-asm.c:574
 // CHECK: fistp dword ptr $0
-// CHECK: "=*m,*m,~{dirflag},~{fpsr},~{flags}"(i32* %{{.*}}, float* %{{.*}})
+// CHECK: "=*m,*m,~{fpsr},~{dirflag},~{fpsr},~{flags}"(i32* %{{.*}}, float* 
%{{.*}})
 }

rnk wrote:
> Two fpsr? If we manually add that as an extra constraint, maybe we should 
> remove that logic in this change.
We slap "~{dirflag},~{fpsr},~{flags}" blindly onto both gcc and MS inline 
assembly. There are tests cases in this file with two ~{flags} as well.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57642



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


[PATCH] D57626: Disallow trivial_abi on a class if all copy and move constructors are deleted

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



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2953
+  "have bases of non-trivial class types|have virtual bases|"
+  "have __weak fields under ARC|have fields of non-trivial class types}0">;
 

nit: "of non-trivial class types" should be "of non-trivial class type" in both 
places.

And I would write "are not move-constructible" rather than "don't have 
non-deleted copy/move constructors". Double negations aren't non-bad.

Actually I would rephrase this as `'trivial_abi' is disallowed on this class 
because it %select{is not move-constructible|is polymorphic|has a base of 
non-trivial class type|has a virtual base|has a __weak field|has a field of 
non-trivial class type}`, i.e., we're not just giving information about 
"classes" in general, we're talking about "this class" specifically. We could 
even name the class if we're feeling generous.



Comment at: lib/Sema/SemaDeclCXX.cpp:7886
+return false;
+  };
+

How confident are we that this logic is correct?  I ask because I need 
something similar for my own diagnostic in D50119. If this logic is rock-solid 
(no lurking corner-case bugs), I should copy it — and/or it should be moved 
into a helper member function on `CXXRecordDecl` so that other people can call 
it too.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57626



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


[PATCH] D57642: [X86] Update clobber list in a test after D57641. Remove filter for 'fpsw' in MS inline asm clobber list generation since the backend now uses 'fpsr'.

2019-02-04 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments.



Comment at: test/CodeGen/ms-inline-asm.c:574
 // CHECK: fistp dword ptr $0
-// CHECK: "=*m,*m,~{dirflag},~{fpsr},~{flags}"(i32* %{{.*}}, float* %{{.*}})
+// CHECK: "=*m,*m,~{fpsr},~{dirflag},~{fpsr},~{flags}"(i32* %{{.*}}, float* 
%{{.*}})
 }

Two fpsr? If we manually add that as an extra constraint, maybe we should 
remove that logic in this change.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57642



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


[PATCH] D57626: Disallow trivial_abi on a class if all copy and move constructors are deleted

2019-02-04 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak updated this revision to Diff 185157.
ahatanak added a comment.

Add a note diagnostic to inform the user of the reason for not allowing 
annotating a class with `trivial_abi`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57626

Files:
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDeclCXX.cpp
  test/SemaObjCXX/attr-trivial-abi.mm

Index: test/SemaObjCXX/attr-trivial-abi.mm
===
--- test/SemaObjCXX/attr-trivial-abi.mm
+++ test/SemaObjCXX/attr-trivial-abi.mm
@@ -10,23 +10,23 @@
   int a;
 };
 
-struct __attribute__((trivial_abi)) S2 { // expected-warning {{'trivial_abi' cannot be applied to 'S2'}}
+struct __attribute__((trivial_abi)) S2 { // expected-warning {{'trivial_abi' cannot be applied to 'S2'}} expected-note {{have __weak fields under ARC}}
   __weak id a;
 };
 
-struct __attribute__((trivial_abi)) S3 { // expected-warning {{'trivial_abi' cannot be applied to 'S3'}}
+struct __attribute__((trivial_abi)) S3 { // expected-warning {{'trivial_abi' cannot be applied to 'S3'}} expected-note {{are polymorphic}}
   virtual void m();
 };
 
 struct S3_2 {
   virtual void m();
-} __attribute__((trivial_abi)); // expected-warning {{'trivial_abi' cannot be applied to 'S3_2'}}
+} __attribute__((trivial_abi)); // expected-warning {{'trivial_abi' cannot be applied to 'S3_2'}} expected-note {{are polymorphic}}
 
 struct S4 {
   int a;
 };
 
-struct __attribute__((trivial_abi)) S5 : public virtual S4 { // expected-warning {{'trivial_abi' cannot be applied to 'S5'}}
+struct __attribute__((trivial_abi)) S5 : public virtual S4 { // expected-warning {{'trivial_abi' cannot be applied to 'S5'}} expected-note {{have virtual bases}}
 };
 
 struct __attribute__((trivial_abi)) S9 : public S4 {
@@ -36,19 +36,19 @@
   __weak id a;
 };
 
-struct __attribute__((trivial_abi)) S12 { // expected-warning {{'trivial_abi' cannot be applied to 'S12'}}
+struct __attribute__((trivial_abi)) S12 { // expected-warning {{'trivial_abi' cannot be applied to 'S12'}} expected-note {{have __weak fields under ARC}}
   __weak id a;
 };
 
-struct __attribute__((trivial_abi)) S13 { // expected-warning {{'trivial_abi' cannot be applied to 'S13'}}
+struct __attribute__((trivial_abi)) S13 { // expected-warning {{'trivial_abi' cannot be applied to 'S13'}} expected-note {{have __weak fields under ARC}}
   __weak id a[2];
 };
 
-struct __attribute__((trivial_abi)) S7 { // expected-warning {{'trivial_abi' cannot be applied to 'S7'}}
+struct __attribute__((trivial_abi)) S7 { // expected-warning {{'trivial_abi' cannot be applied to 'S7'}} expected-note {{have fields of non-trivial class types}}
   S6 a;
 };
 
-struct __attribute__((trivial_abi)) S11 { // expected-warning {{'trivial_abi' cannot be applied to 'S11'}}
+struct __attribute__((trivial_abi)) S11 { // expected-warning {{'trivial_abi' cannot be applied to 'S11'}} expected-note {{have fields of non-trivial class types}}
   S6 a[2];
 };
 
@@ -66,7 +66,7 @@
 S10<__weak id> p2;
 
 template<>
-struct __attribute__((trivial_abi)) S10 { // expected-warning {{'trivial_abi' cannot be applied to 'S10'}}
+struct __attribute__((trivial_abi)) S10 { // expected-warning {{'trivial_abi' cannot be applied to 'S10'}} expected-note {{have __weak fields under ARC}}
   __weak id a;
 };
 
@@ -90,8 +90,41 @@
 S16 s16;
 
 template
-struct __attribute__((trivial_abi)) S17 { // expected-warning {{'trivial_abi' cannot be applied to 'S17'}}
+struct __attribute__((trivial_abi)) S17 { // expected-warning {{'trivial_abi' cannot be applied to 'S17'}} expected-note {{have __weak fields under ARC}}
   __weak id a;
 };
 
 S17 s17;
+
+namespace deletedCopyMoveConstructor {
+  struct __attribute__((trivial_abi)) CopyMoveDeleted { // expected-warning {{'trivial_abi' cannot be applied to 'CopyMoveDeleted'}} expected-note {{"don't have non-deleted copy/move constructors}}
+CopyMoveDeleted(const CopyMoveDeleted &) = delete;
+CopyMoveDeleted(CopyMoveDeleted &&) = delete;
+  };
+
+  struct __attribute__((trivial_abi)) S18 { // expected-warning {{'trivial_abi' cannot be applied to 'S18'}} expected-note {{"don't have non-deleted copy/move constructors}}
+CopyMoveDeleted a;
+S18(const S18 &);
+S18(S18 &&);
+  };
+
+  struct __attribute__((trivial_abi)) CopyDeleted {
+CopyDeleted(const CopyDeleted &) = delete;
+CopyDeleted(CopyDeleted &&) = default;
+  };
+
+  struct __attribute__((trivial_abi)) MoveDeleted {
+MoveDeleted(const MoveDeleted &) = default;
+MoveDeleted(MoveDeleted &&) = delete;
+  };
+
+  struct __attribute__((trivial_abi)) S19 { // expected-warning {{'trivial_abi' cannot be applied to 'S19'}} expected-note {{"don't have non-deleted copy/move constructors}}
+CopyDeleted a;
+MoveDeleted b;
+  };
+
+  // This is fine since the move constructor isn't deleted.
+  struct __attribute__((trivial_abi)) S20 {
+

[PATCH] D57716: Let AMDGPU compile MSVC headers containing vectorcall

2019-02-04 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl created this revision.
yaxunl added a reviewer: rjmccall.
Herald added subscribers: t-tye, tpr, dstuttard, nhaehnle, wdng, jvesely, 
kzhuravl.

MSVC header files using vectorcall to differentiate overloaded functions, which
causes failure for AMDGPU target. Let AMDGPU target recognize vectorcall
calling convention so that these header files can be compiled.


https://reviews.llvm.org/D57716

Files:
  include/clang/Basic/Specifiers.h
  lib/AST/ItaniumMangle.cpp
  lib/AST/Type.cpp
  lib/AST/TypePrinter.cpp
  lib/Basic/Targets/AMDGPU.h
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/CGDebugInfo.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/SemaCUDA/amdgpu-windows-vectorcall.cu

Index: test/SemaCUDA/amdgpu-windows-vectorcall.cu
===
--- /dev/null
+++ test/SemaCUDA/amdgpu-windows-vectorcall.cu
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -aux-triple x86_64-pc-windows-msvc -fms-compatibility -fcuda-is-device -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+template
+ struct A
+ {
+ };
+
+template struct A<_Ret (__cdecl _Arg0::*)(_Types) > { }; 
+template struct A<_Ret (__vectorcall _Arg0::*)(_Types) > {};
+
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -4568,7 +4568,8 @@
 CC = CC_Swift;
 break;
   case ParsedAttr::AT_VectorCall:
-CC = CC_X86VectorCall;
+CC = Context.getTargetInfo().getTriple().isAMDGPU()
+  ? CC_AMDGPUVectorCall : CC_X86VectorCall;
 break;
   case ParsedAttr::AT_AArch64VectorPcs:
 CC = CC_AArch64VectorCall;
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -1113,6 +1113,7 @@
 static unsigned getDwarfCC(CallingConv CC) {
   switch (CC) {
   case CC_C:
+  case CC_AMDGPUVectorCall:
 // Avoid emitting DW_AT_calling_convention if the C convention was used.
 return 0;
 
Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -58,6 +58,7 @@
   case CC_X86Pascal: return llvm::CallingConv::C;
   // TODO: Add support for __vectorcall to LLVM.
   case CC_X86VectorCall: return llvm::CallingConv::X86_VectorCall;
+  case CC_AMDGPUVectorCall: return llvm::CallingConv::C;
   case CC_AArch64VectorCall: return llvm::CallingConv::AArch64_VectorCall;
   case CC_SpirFunction: return llvm::CallingConv::SPIR_FUNC;
   case CC_OpenCLKernel: return CGM.getTargetCodeGenInfo().getOpenCLKernelCallingConv();
Index: lib/Basic/Targets/AMDGPU.h
===
--- lib/Basic/Targets/AMDGPU.h
+++ lib/Basic/Targets/AMDGPU.h
@@ -341,6 +341,7 @@
   return CCCR_Warning;
 case CC_C:
 case CC_OpenCLKernel:
+case CC_AMDGPUVectorCall:
   return CCCR_OK;
 }
   }
Index: lib/AST/TypePrinter.cpp
===
--- lib/AST/TypePrinter.cpp
+++ lib/AST/TypePrinter.cpp
@@ -862,6 +862,7 @@
   OS << " __attribute__((thiscall))";
   break;
 case CC_X86VectorCall:
+case CC_AMDGPUVectorCall:
   OS << " __attribute__((vectorcall))";
   break;
 case CC_X86Pascal:
Index: lib/AST/Type.cpp
===
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -2826,6 +2826,7 @@
   case CC_X86ThisCall: return "thiscall";
   case CC_X86Pascal: return "pascal";
   case CC_X86VectorCall: return "vectorcall";
+  case CC_AMDGPUVectorCall: return "amdgpu_vectorcall";
   case CC_Win64: return "ms_abi";
   case CC_X86_64SysV: return "sysv_abi";
   case CC_X86RegCall : return "regcall";
Index: lib/AST/ItaniumMangle.cpp
===
--- lib/AST/ItaniumMangle.cpp
+++ lib/AST/ItaniumMangle.cpp
@@ -2648,6 +2648,7 @@
 return "";
 
   case CC_X86VectorCall:
+  case CC_AMDGPUVectorCall:
   case CC_X86Pascal:
   case CC_X86RegCall:
   case CC_AAPCS:
Index: include/clang/Basic/Specifiers.h
===
--- include/clang/Basic/Specifiers.h
+++ include/clang/Basic/Specifiers.h
@@ -251,6 +251,7 @@
 CC_PreserveMost, // __attribute__((preserve_most))
 CC_PreserveAll,  // __attribute__((preserve_all))
 CC_AArch64VectorCall, // __attribute__((aarch64_vector_pcs))
+CC_AMDGPUVectorCall, // __attribute__((vectorcall))
   };
 
   /// Checks whether the given calling convention supports variadic
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D57219: [Fixed Point Arithmetic] Fixed Point Comparisons

2019-02-04 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3444
   case BO_NE:
+return Builder.CreateICmpNE(FullLHS, FullRHS);
+  case BO_Mul:

leonardchan wrote:
> rjmccall wrote:
> > leonardchan wrote:
> > > rjmccall wrote:
> > > > Are padding bits guaranteed zero or unspecified?  Or are we just not 
> > > > really supporting padding bits all the way to IRGen at this time?
> > > I believe the consensus was leaving them unspecified, so operations that 
> > > can cause overflow into them would result in undefined behavior.
> > If I'm understanding you correctly, you're saying that (1) we are assuming 
> > that inputs are zero-padded and (2) we are taking advantage of the 
> > non-saturating-overflow-is-UB rule to avoid clearing the padding bits after 
> > arithmetic.  That's actually what I meant by "guaranteed zero", so we have 
> > our labels reversed, but at least we understand each other now.
> > 
> > (I suppose you're thinking of this as "unspecified" because non-saturating 
> > arithmetic can leave an unspecified value there.  I think of this as a 
> > "guarantee" because it's basically a representational invariant: it's a 
> > precondition for correctness that the bit is zero, and all operations 
> > guarantee that the bit will be zero in their well-defined cases (but 
> > overflow is not well-defined).  Just a difference in perspective, I guess.)
> > 
> > Is this written down somewhere?  Are there targets that use the opposite 
> > ABI rule that we might need to support someday?
> > 
> > At any rate, I think it's worth adding a short comment here explaining why 
> > it's okay to do a normal comparison despite the possibility of padding 
> > bits.  Along those lines, is there any value in communicating to the 
> > backend that padded-unsigned comparisons could reasonably be done with 
> > either a signed or unsigned comparison?  Are there interesting targets 
> > where one or the other is cheaper?
> Yes. We assume inputs are zero padded, and that non-saturating overflow is 
> undefined so we do not need to clear the padding after writing a new value. 
> Sorry for the misunderstanding. I see what you mean by guaranteed zero.
> 
> Overflow in general is controlled by the `FX_FRACT_OVERFLOW` and 
> `FX_ACCUM_OVERFLOW` pragmas, where if these are set to `DEFAULT`, operations 
> that can overflow with these types is undefined according to the standard 
> (4.1.3). In terms of padding bits, clause 6.2.6.3 mentions that the values of 
> padding bits are "unspecified". I imagine this means that we can assume the 
> value to be whatever we want it to, so we can assume that these values are a 
> guaranteed zero.
> 
> I believe @ebevhan requested this being added since his downstream 
> implementation used padding to match the scales of signed and unsigned types, 
> so he may be able to offer more information on targets with different ABIs. 
> We don't plan to use any special hardware, so we're following the "typical 
> desktop processor" layout that uses the whole underlying int and no padding 
> (mentioned in Annex 3).
> 
> In the same section, the standard also mentions types for other targets that 
> may have padding, so there could be some value in indicating to the backend 
> that for these particular targets, this part of the operand is padding, so 
> select an appropriate operation that performs a comparison on this size type. 
> But I don't know much about these processors and would just be guessing at 
> how they work.
Okay.  If we ever have a target that uses an ABI that (1) includes padding but 
(2) considers it to be "unspecified" in the sense of "it can validly be 
non-zero", we'll have to change this code, but I agree it's the right move to 
not preemptively generalize to cover that possibility.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57219



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


[PATCH] D54429: [analyzer] Creating standard Sphinx documentation

2019-02-04 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

I'll do the honors. Thanks!


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

https://reviews.llvm.org/D54429



___
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

2019-02-04 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: lib/AST/Stmt.cpp:457-460
   this->NumOutputs = NumOutputs;
   this->NumInputs = NumInputs;
   this->NumClobbers = NumClobbers;
+  this->NumLabels = NumLabels;

Please assert that you don't have both outputs and labels here. (If you did, 
you would assign them the same slots within `Exprs`.)

You also seem to be missing `Sema` enforcement of the rule that you cannot have 
both outputs and labels. (If you want to actually support that as an 
intentional extension to the GCC functionality, you should change the 
implementation of `GCCAsmStmt` to use different slots for outputs and labels, 
add some tests, and add a `-Wgcc-compat` warning for that case. Seems better to 
just add an error for it for now.)



Comment at: lib/Analysis/CFG.cpp:1445-1458
+  LabelMapTy::iterator LI = LabelMap.find(G->getLabel());;
+  // If there is no target for the goto, then we are looking at an
+  // incomplete AST.  Handle this by not registering a successor.
+  if (LI == LabelMap.end()) continue;
 
-JumpTarget JT = LI->second;
-prependAutomaticObjLifetimeWithTerminator(B, I->scopePosition,
-  JT.scopePosition);
-prependAutomaticObjDtorsWithTerminator(B, I->scopePosition,
-   JT.scopePosition);
-const VarDecl *VD = prependAutomaticObjScopeEndWithTerminator(
-B, I->scopePosition, JT.scopePosition);
-appendScopeBegin(JT.block, VD, G);
-addSuccessor(B, JT.block);
+  JumpTarget JT = LI->second;
+  prependAutomaticObjLifetimeWithTerminator(B, I->scopePosition,

Please factor out this duplicated code into a local lambda rather than 
copy-pasting it.



Comment at: lib/Analysis/CFG.cpp:1461
+if (auto *G = dyn_cast(B->getTerminator())) {
+  if (G->isAsmGoto()) {
+for (auto *E : G->labels()) {

This check is redundant; there will be no labels if it's not an `asm goto` so 
you can just perform the below loop unconditionally.



Comment at: lib/Analysis/CFG.cpp:3139-3141
+  // We will need to backpatch this block later.
+  BackpatchBlocks.push_back(JumpSource(Block, ScopePos));
+  return Block;

Do we not need the handling for the other labels if we hit this condition for 
one label?



Comment at: lib/Sema/SemaStmtAsm.cpp:656
+  // Check for duplicate asm operand name between input, output and label 
lists.
+  typedef std::pair  MyItemType;
+  SmallVector MyList;

No space after `pair`, please.



Comment at: lib/Sema/SemaStmtAsm.cpp:659-662
+  for (unsigned i = 0, e = NumOutputs + NumInputs + NumLabels; i != e; ++i)
+if (NS->getIdentifier(i) && !NS->getIdentifier(i)->getName().empty())
+  MyList.emplace_back(std::make_pair(NS->getIdentifier(i)->getName(),
+ NS->getOperandExpr(i)));

Looping over all three lists of expressions here is effectively hardcoding an 
implementation detail of `GCCAsmStmt` into this code, violating its 
encapsulation. Instead, please write three loops (iterating over the inputs, 
outputs, and labels), and remove `getOperandExpr` entirely.



Comment at: lib/Sema/SemaStmtAsm.cpp:660
+  for (unsigned i = 0, e = NumOutputs + NumInputs + NumLabels; i != e; ++i)
+if (NS->getIdentifier(i) && !NS->getIdentifier(i)->getName().empty())
+  MyList.emplace_back(std::make_pair(NS->getIdentifier(i)->getName(),

Checking whether the name is empty is redundant: we should never create an 
`IdentifierInfo` representing an empty string.


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

https://reviews.llvm.org/D56571



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


[PATCH] D57711: [Sanitizers] UBSan unreachable incompatible with Kernel ASan

2019-02-04 Thread Evgenii Stepanov via Phabricator via cfe-commits
eugenis accepted this revision.
eugenis added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57711



___
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

2019-02-04 Thread Russell McClellan via Phabricator via cfe-commits
russellmcc added a comment.

Bump!  Thanks for the consideration


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] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-04 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 marked 2 inline comments as done.
jyu2 added inline comments.



Comment at: lib/Sema/JumpDiagnostics.cpp:347
+  LabelAndGotoScopes[S] = ParentScope;
+  Jumps.push_back(S);
+}

efriedma wrote:
> This doesn't look right; I think we need to add it to IndirectJumps instead.  
> This probably impacts a testcase like the following:
> 
> ```
> struct S { ~S(); };
> int f() {
>   {
> S s;
> asm goto(""BAR);
> return 1;
>   }
> BAR:
>   return 0;
> }
> ```
> 
> (gcc currently accepts this and skips running the destructor, but I'm pretty 
> sure that's a bug.)
Hi Eli,
I see both g++ and clang++ with my change call ~S.  Am I missing something?  
Thanks.

1>clang++ j.cpp
1>./a.out
~S()
1>g++ j.cpp
1>./a.out
~S()

Here is the test case:
1>cat j.cpp
extern "C" int printf (const char *,...);
struct S { ~S() {printf("~S()\n");}; };
int f() {
  {
S s;
asm goto(""BAR);
return 1;
  }
BAR:
  return 0;
}

int main()
{
  f();
}



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

https://reviews.llvm.org/D56571



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


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

2019-02-04 Thread Russell McClellan via Phabricator via cfe-commits
russellmcc added a comment.

Bump!  Thanks for the consideration.


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


[PATCH] D57711: [Sanitizers] UBSan unreachable incompatible with Kernel ASan

2019-02-04 Thread Vedant Kumar via Phabricator via cfe-commits
vsk accepted this revision.
vsk added a comment.
This revision is now accepted and ready to land.

LGTM, thanks. It might make sense to add a predicate to SanitizerSet to make it 
easier to check when ASan is enabled, either pre-commit or as a follow-up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57711



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


[PATCH] D57712: [SemaObjC] Don't infer the availabilty of messages to +new from -init if the receiver has Class type

2019-02-04 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman accepted this revision.
arphaman added a comment.
This revision is now accepted and ready to land.

LGTM, Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D57712



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


[PATCH] D57712: [SemaObjC] Don't infer the availabilty of messages to +new from -init if the receiver has Class type

2019-02-04 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington created this revision.
erik.pilkington added a reviewer: arphaman.
Herald added subscribers: dexonsmith, jkorous.
Herald added a project: clang.

Fixes rdar://47713266

Thanks for taking a look!
Erik


Repository:
  rC Clang

https://reviews.llvm.org/D57712

Files:
  clang/lib/Sema/SemaExprObjC.cpp
  clang/test/SemaObjC/infer-availability-from-init.m


Index: clang/test/SemaObjC/infer-availability-from-init.m
===
--- clang/test/SemaObjC/infer-availability-from-init.m
+++ clang/test/SemaObjC/infer-availability-from-init.m
@@ -56,3 +56,16 @@
   [self new];
 }
 @end
+
+@interface NoInit : NSObject
+-(instancetype)init __attribute__((unavailable)); // expected-note {{'init' 
has been explicitly marked unavailable here}}
+@end
+
+@interface NoInitSub : NoInit @end
+
+@implementation NoInitSub
+-(void)meth:(Class)c {
+  [c new]; // No error; unknown interface.
+  [NoInitSub new]; // expected-error {{'new' is unavailable}}
+}
+@end
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -2805,8 +2805,8 @@
   } else {
 if (ObjCMethodDecl *CurMeth = getCurMethodDecl()) {
   if (ObjCInterfaceDecl *ClassDecl = CurMeth->getClassInterface()) {
-// FIXME: Is this correct? Why are we assuming that a message to
-// Class will call a method in the current interface?
+// As a guess, try looking for the method in the current interface.
+// This very well may not produce the "right" method.
 
 // First check the public methods in the class interface.
 Method = ClassDecl->lookupClassMethod(Sel);
@@ -2814,8 +2814,7 @@
 if (!Method)
   Method = ClassDecl->lookupPrivateClassMethod(Sel);
 
-if (Method && DiagnoseUseOfDecl(Method, SelectorSlotLocs, nullptr,
-false, false, ClassDecl))
+if (Method && DiagnoseUseOfDecl(Method, SelectorSlotLocs))
   return ExprError();
   }
 }


Index: clang/test/SemaObjC/infer-availability-from-init.m
===
--- clang/test/SemaObjC/infer-availability-from-init.m
+++ clang/test/SemaObjC/infer-availability-from-init.m
@@ -56,3 +56,16 @@
   [self new];
 }
 @end
+
+@interface NoInit : NSObject
+-(instancetype)init __attribute__((unavailable)); // expected-note {{'init' has been explicitly marked unavailable here}}
+@end
+
+@interface NoInitSub : NoInit @end
+
+@implementation NoInitSub
+-(void)meth:(Class)c {
+  [c new]; // No error; unknown interface.
+  [NoInitSub new]; // expected-error {{'new' is unavailable}}
+}
+@end
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -2805,8 +2805,8 @@
   } else {
 if (ObjCMethodDecl *CurMeth = getCurMethodDecl()) {
   if (ObjCInterfaceDecl *ClassDecl = CurMeth->getClassInterface()) {
-// FIXME: Is this correct? Why are we assuming that a message to
-// Class will call a method in the current interface?
+// As a guess, try looking for the method in the current interface.
+// This very well may not produce the "right" method.
 
 // First check the public methods in the class interface.
 Method = ClassDecl->lookupClassMethod(Sel);
@@ -2814,8 +2814,7 @@
 if (!Method)
   Method = ClassDecl->lookupPrivateClassMethod(Sel);
 
-if (Method && DiagnoseUseOfDecl(Method, SelectorSlotLocs, nullptr,
-false, false, ClassDecl))
+if (Method && DiagnoseUseOfDecl(Method, SelectorSlotLocs))
   return ExprError();
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D57711: [Sanitizers] UBSan unreachable incompatible with Kernel ASan

2019-02-04 Thread Julian Lettner via Phabricator via cfe-commits
yln created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is a follow up for https://reviews.llvm.org/D57278. The previous
revision should have also included Kernel ASan.

rdar://problem/40723397


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D57711

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGen/ubsan-asan-noreturn.c
  llvm/test/Instrumentation/AddressSanitizer/instrument-no-return.ll


Index: llvm/test/Instrumentation/AddressSanitizer/instrument-no-return.ll
===
--- llvm/test/Instrumentation/AddressSanitizer/instrument-no-return.ll
+++ llvm/test/Instrumentation/AddressSanitizer/instrument-no-return.ll
@@ -35,6 +35,15 @@
 ; CHECK-NOT:call void @__asan_handle_no_return
 ; CHECK:call void @NoReturnFunc
 
+; Do *not* instrument functions without ASan
+define i32 @Call4() {
+  call void @NoReturnFunc() noreturn
+  unreachable
+}
+; CHECK-LABEL:  @Call4
+; CHECK-NOT:call void @__asan_handle_no_return
+; CHECK:call void @NoReturnFunc
+
 declare i32 @__gxx_personality_v0(...)
 
 define i64 @Invoke1() nounwind uwtable ssp sanitize_address personality i8* 
bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
Index: clang/test/CodeGen/ubsan-asan-noreturn.c
===
--- clang/test/CodeGen/ubsan-asan-noreturn.c
+++ clang/test/CodeGen/ubsan-asan-noreturn.c
@@ -1,6 +1,7 @@
 // Ensure compatiblity of UBSan unreachable with ASan in the presence of
 // noreturn functions.
-// RUN: %clang_cc1 -fsanitize=unreachable,address -triple x86_64-linux 
-emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=unreachable,address-triple x86_64-linux 
-emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=unreachable,kernel-address -triple x86_64-linux 
-emit-llvm -o - %s | FileCheck %s
 
 void my_longjmp(void) __attribute__((noreturn));
 
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -4403,7 +4403,8 @@
 
   // Avoid incompatibility with ASan which relies on the `noreturn`
   // attribute to insert handler calls.
-  if (SanOpts.has(SanitizerKind::Address)) {
+  if (SanOpts.hasOneOf(SanitizerKind::Address |
+   SanitizerKind::KernelAddress)) {
 SanitizerScope SanScope(this);
 llvm::IRBuilder<>::InsertPointGuard IPGuard(Builder);
 Builder.SetInsertPoint(CI);


Index: llvm/test/Instrumentation/AddressSanitizer/instrument-no-return.ll
===
--- llvm/test/Instrumentation/AddressSanitizer/instrument-no-return.ll
+++ llvm/test/Instrumentation/AddressSanitizer/instrument-no-return.ll
@@ -35,6 +35,15 @@
 ; CHECK-NOT:call void @__asan_handle_no_return
 ; CHECK:call void @NoReturnFunc
 
+; Do *not* instrument functions without ASan
+define i32 @Call4() {
+  call void @NoReturnFunc() noreturn
+  unreachable
+}
+; CHECK-LABEL:  @Call4
+; CHECK-NOT:call void @__asan_handle_no_return
+; CHECK:call void @NoReturnFunc
+
 declare i32 @__gxx_personality_v0(...)
 
 define i64 @Invoke1() nounwind uwtable ssp sanitize_address personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
Index: clang/test/CodeGen/ubsan-asan-noreturn.c
===
--- clang/test/CodeGen/ubsan-asan-noreturn.c
+++ clang/test/CodeGen/ubsan-asan-noreturn.c
@@ -1,6 +1,7 @@
 // Ensure compatiblity of UBSan unreachable with ASan in the presence of
 // noreturn functions.
-// RUN: %clang_cc1 -fsanitize=unreachable,address -triple x86_64-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=unreachable,address-triple x86_64-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=unreachable,kernel-address -triple x86_64-linux -emit-llvm -o - %s | FileCheck %s
 
 void my_longjmp(void) __attribute__((noreturn));
 
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -4403,7 +4403,8 @@
 
   // Avoid incompatibility with ASan which relies on the `noreturn`
   // attribute to insert handler calls.
-  if (SanOpts.has(SanitizerKind::Address)) {
+  if (SanOpts.hasOneOf(SanitizerKind::Address |
+   SanitizerKind::KernelAddress)) {
 SanitizerScope SanScope(this);
 llvm::IRBuilder<>::InsertPointGuard IPGuard(Builder);
 Builder.SetInsertPoint(CI);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D57674: [clang-tidy] Add options to bugprone-argument-comment to add missing argument comments to literals

2019-02-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 185148.
MyDeveloperDay edited the summary of this revision.
MyDeveloperDay added a comment.

Address review comments
Add support for additional StringLiterals,CharacterLiterals,UserDefinedLiterals 
and NullPtrs
Simplify the Options names from  "AddCommentsToXXX" to "CommentXXX"
Add extra unit tests to support additional supported literal types


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

https://reviews.llvm.org/D57674

Files:
  clang-tidy/bugprone/ArgumentCommentCheck.cpp
  clang-tidy/bugprone/ArgumentCommentCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-argument-comment.rst
  test/clang-tidy/bugprone-argument-comment-literals.cpp

Index: test/clang-tidy/bugprone-argument-comment-literals.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-argument-comment-literals.cpp
@@ -0,0 +1,107 @@
+// RUN: %check_clang_tidy %s bugprone-argument-comment %t -- \
+// RUN:   -config="{CheckOptions: [{key: CommentBoolLiterals, value: 1},{key: CommentIntegerLiterals, value: 1}, {key: CommentFloatLiterals, value: 1}, {key: CommentUserDefinedLiterals, value: 1}, {key: CommentStringLiterals, value: 1}, {key: CommentNullPtrs, value: 1}, {key: CommentCharacterLiterals, value: 1}]}" --
+
+struct A {
+  void foo(bool abc);
+  void foo(bool abc, bool cde);
+  void foo(const char *, bool abc);
+  void foo(int iabc);
+  void foo(float fabc);
+  void foo(double dabc);
+  void foo(const char *strabc);
+  void fooW(const wchar_t *wstrabc);
+  void fooPtr(A *ptrabc);
+  void foo(char chabc);
+};
+
+double operator"" _km(long double);
+
+void test() {
+  A a;
+
+  a.foo(true);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/true);
+
+  a.foo(false);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/false);
+
+  a.foo(true, false);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-MESSAGES: [[@LINE-2]]:15: warning: argument comment missing for literal argument 'cde' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/true, /*cde=*/false);
+
+  a.foo(false, true);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-MESSAGES: [[@LINE-2]]:16: warning: argument comment missing for literal argument 'cde' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/false, /*cde=*/true);
+
+  a.foo(/*abc=*/false, true);
+  // CHECK-MESSAGES: [[@LINE-1]]:24: warning: argument comment missing for literal argument 'cde' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/false, /*cde=*/true);
+
+  a.foo(false, /*cde=*/true);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*abc=*/false, /*cde=*/true);
+
+  bool val1 = true;
+  bool val2 = false;
+  a.foo(val1, val2);
+
+  a.foo("", true);
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: argument comment missing for literal argument 'abc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo("", /*abc=*/true);
+
+  a.foo(0);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'iabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*iabc=*/0);
+
+  a.foo(1.0f);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'fabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*fabc=*/1.0f);
+
+  a.foo(1.0);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'dabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*dabc=*/1.0);
+
+  int val3 = 10;
+  a.foo(val3);
+
+  float val4 = 10.0;
+  a.foo(val4);
+
+  double val5 = 10.0;
+  a.foo(val5);
+
+  a.foo("Hello World");
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'strabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*strabc=*/"Hello World");
+  //
+  a.fooW(L"Hello World");
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: argument comment missing for literal argument 'wstrabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.fooW(/*wstrabc=*/L"Hello World");
+
+  a.fooPtr(nullptr);
+  // CHECK-MESSAGES: [[@LINE-1]]:12: warning: argument comment missing for literal argument 'ptrabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.fooPtr(/*ptrabc=*/nullptr);
+
+  a.foo(402.0_km);
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument comment missing for literal argument 'dabc' [bugprone-argument-comment]
+  // CHECK-FIXES: a.foo(/*dabc=*/402.0_km);
+
+  a.foo('A');
+  // CHECK-MESSAGES: [[@LINE-1]]:9: warning: argument 

[PATCH] D57668: [opaque pointer types] Pass function types for runtime function calls.

2019-02-04 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added inline comments.



Comment at: clang/lib/CodeGen/CGObjC.cpp:1028-1030
+llvm::Constant *getPropertyFn = cast(
+CGM.getObjCRuntime().GetPropertyGetFunction().getCallee());
 if (!getPropertyFn) {

This code looks like it implies that the 'if' is never hit? (since cast doesn't 
propagate null (it asserts/fails/UB on null)) - should this be cast_or_null 
instead? Or "if(!CGM.getObjCRuntime().getPropertyGetFunction())" ?)

(there are a few similar instances later on)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57668



___
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

2019-02-04 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments.



Comment at: lib/Sema/JumpDiagnostics.cpp:347
+  LabelAndGotoScopes[S] = ParentScope;
+  Jumps.push_back(S);
+}

This doesn't look right; I think we need to add it to IndirectJumps instead.  
This probably impacts a testcase like the following:

```
struct S { ~S(); };
int f() {
  {
S s;
asm goto(""BAR);
return 1;
  }
BAR:
  return 0;
}
```

(gcc currently accepts this and skips running the destructor, but I'm pretty 
sure that's a bug.)


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

https://reviews.llvm.org/D56571



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


[PATCH] D57636: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg

2019-02-04 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

Yes, we should fix CodeGenFunction::EmitAArch64BuiltinExpr to eliminated the 
unnecessary calls to CreateZext/CreateTrunc.  (With this patch, they're no-ops, 
but better to clean up the code.)


Repository:
  rC Clang

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

https://reviews.llvm.org/D57636



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


[PATCH] D57665: [clang-tidy] Handle unions with existing default-member-init

2019-02-04 Thread Malcolm Parsons via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE353092: [clang-tidy] Handle unions with existing 
default-member-init (authored by malcolm.parsons, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57665?vs=184976=185141#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57665

Files:
  clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
  test/clang-tidy/modernize-use-default-member-init.cpp


Index: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
===
--- clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
+++ clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
@@ -273,7 +273,7 @@
 
 void UseDefaultMemberInitCheck::checkExistingInit(
 const MatchFinder::MatchResult , const CXXCtorInitializer *Init) {
-  const FieldDecl *Field = Init->getMember();
+  const FieldDecl *Field = Init->getAnyMember();
 
   if (!sameValue(Field->getInClassInitializer(), Init->getInit()))
 return;
Index: test/clang-tidy/modernize-use-default-member-init.cpp
===
--- test/clang-tidy/modernize-use-default-member-init.cpp
+++ test/clang-tidy/modernize-use-default-member-init.cpp
@@ -382,6 +382,16 @@
   const char *e4 = "bar";
 };
 
+struct UnionExisting {
+  UnionExisting() : e(5.0) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: member initializer for 'e' is 
redundant
+  // CHECK-FIXES: UnionExisting()  {}
+  union {
+int i;
+double e = 5.0;
+  };
+};
+
 template 
 struct NegativeTemplateExisting {
   NegativeTemplateExisting(int) : t(0) {}


Index: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
===
--- clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
+++ clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
@@ -273,7 +273,7 @@
 
 void UseDefaultMemberInitCheck::checkExistingInit(
 const MatchFinder::MatchResult , const CXXCtorInitializer *Init) {
-  const FieldDecl *Field = Init->getMember();
+  const FieldDecl *Field = Init->getAnyMember();
 
   if (!sameValue(Field->getInClassInitializer(), Init->getInit()))
 return;
Index: test/clang-tidy/modernize-use-default-member-init.cpp
===
--- test/clang-tidy/modernize-use-default-member-init.cpp
+++ test/clang-tidy/modernize-use-default-member-init.cpp
@@ -382,6 +382,16 @@
   const char *e4 = "bar";
 };
 
+struct UnionExisting {
+  UnionExisting() : e(5.0) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: member initializer for 'e' is redundant
+  // CHECK-FIXES: UnionExisting()  {}
+  union {
+int i;
+double e = 5.0;
+  };
+};
+
 template 
 struct NegativeTemplateExisting {
   NegativeTemplateExisting(int) : t(0) {}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang-tools-extra] r353092 - [clang-tidy] Handle unions with existing default-member-init

2019-02-04 Thread Malcolm Parsons via cfe-commits
Author: malcolm.parsons
Date: Mon Feb  4 13:09:31 2019
New Revision: 353092

URL: http://llvm.org/viewvc/llvm-project?rev=353092=rev
Log:
[clang-tidy] Handle unions with existing default-member-init

Summary:
clang-tidy's modernize-use-default-member-init was crashing for unions
with an existing default member initializer.

Fixes PR40492

Reviewers: aaron.ballman, alexfh, JonasToth

Reviewed By: JonasToth

Subscribers: JonasToth, riccibruno, xazax.hun, cfe-commits

Tags: #clang, #clang-tools-extra

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

Modified:
clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp

clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp?rev=353092=353091=353092=diff
==
--- clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp 
Mon Feb  4 13:09:31 2019
@@ -273,7 +273,7 @@ void UseDefaultMemberInitCheck::checkDef
 
 void UseDefaultMemberInitCheck::checkExistingInit(
 const MatchFinder::MatchResult , const CXXCtorInitializer *Init) {
-  const FieldDecl *Field = Init->getMember();
+  const FieldDecl *Field = Init->getAnyMember();
 
   if (!sameValue(Field->getInClassInitializer(), Init->getInit()))
 return;

Modified: 
clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init.cpp?rev=353092=353091=353092=diff
==
--- 
clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init.cpp 
(original)
+++ 
clang-tools-extra/trunk/test/clang-tidy/modernize-use-default-member-init.cpp 
Mon Feb  4 13:09:31 2019
@@ -382,6 +382,16 @@ struct ExistingString {
   const char *e4 = "bar";
 };
 
+struct UnionExisting {
+  UnionExisting() : e(5.0) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: member initializer for 'e' is 
redundant
+  // CHECK-FIXES: UnionExisting()  {}
+  union {
+int i;
+double e = 5.0;
+  };
+};
+
 template 
 struct NegativeTemplateExisting {
   NegativeTemplateExisting(int) : t(0) {}


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


[PATCH] D57664: [opaque pointer types] Fix the CallInfo passed to EmitCall in some edge cases.

2019-02-04 Thread David Blaikie via Phabricator via cfe-commits
dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Seems reasonable to me - but if you like you can wait for Richard who might 
have a more nuanced understanding of the code. (but I'm OK signing off on this 
if you are & Richard can provide any extra feedback post-commit)




Comment at: clang/lib/CodeGen/CGCall.cpp:3837
+// having pointee types).
+llvm::FunctionType *IRFuncTyFromInfo = 
getTypes().GetFunctionType(CallInfo);
+assert(IRFuncTy == IRFuncTyFromInfo);

This will be warned as unused in a release build.

Would this be hideous if it's just all one big assert?

  assert((CallInfo.isVariadic && CallInfo.getArgStruct) || IRFuncTy == 
getTypes().GetFunctionType(CallInfo));

(I think that's accurate?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57664



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


r353090 - [NewPM][MSan] Add Options Handling

2019-02-04 Thread Philip Pfaffe via cfe-commits
Author: pfaffe
Date: Mon Feb  4 13:02:49 2019
New Revision: 353090

URL: http://llvm.org/viewvc/llvm-project?rev=353090=rev
Log:
[NewPM][MSan] Add Options Handling

Summary: This patch enables passing options to msan via the passes pipeline, 
e.e., -passes=msan.

Reviewers: chandlerc, fedor.sergeev, leonardchan

Subscribers: hiraditya, bollu, llvm-commits

Tags: #llvm

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

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

Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=353090=353089=353090=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Mon Feb  4 13:02:49 2019
@@ -279,7 +279,8 @@ static void addGeneralOptsForMemorySanit
   const CodeGenOptions  = BuilderWrapper.getCGOpts();
   int TrackOrigins = CGOpts.SanitizeMemoryTrackOrigins;
   bool Recover = CGOpts.SanitizeRecover.has(SanitizerKind::Memory);
-  PM.add(createMemorySanitizerLegacyPassPass(TrackOrigins, Recover, 
CompileKernel));
+  PM.add(createMemorySanitizerLegacyPassPass(
+  MemorySanitizerOptions{TrackOrigins, Recover, CompileKernel}));
 
   // MemorySanitizer inserts complex instrumentation that mostly follows
   // the logic of the original code, but operates on "shadow" values.
@@ -1035,7 +1036,7 @@ void EmitAssemblyHelper::EmitAssemblyWit
   if (LangOpts.Sanitize.has(SanitizerKind::Memory))
 PB.registerOptimizerLastEPCallback(
 [](FunctionPassManager , PassBuilder::OptimizationLevel Level) 
{
-  FPM.addPass(MemorySanitizerPass());
+  FPM.addPass(MemorySanitizerPass({}));
 });
   if (LangOpts.Sanitize.has(SanitizerKind::Thread))
 PB.registerOptimizerLastEPCallback(


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


[PATCH] D57640: [NewPM][MSan] Add Options Handling

2019-02-04 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC353090: [NewPM][MSan] Add Options Handling (authored by 
pfaffe, committed by ).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57640?vs=185133=185139#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D57640

Files:
  lib/CodeGen/BackendUtil.cpp


Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -279,7 +279,8 @@
   const CodeGenOptions  = BuilderWrapper.getCGOpts();
   int TrackOrigins = CGOpts.SanitizeMemoryTrackOrigins;
   bool Recover = CGOpts.SanitizeRecover.has(SanitizerKind::Memory);
-  PM.add(createMemorySanitizerLegacyPassPass(TrackOrigins, Recover, 
CompileKernel));
+  PM.add(createMemorySanitizerLegacyPassPass(
+  MemorySanitizerOptions{TrackOrigins, Recover, CompileKernel}));
 
   // MemorySanitizer inserts complex instrumentation that mostly follows
   // the logic of the original code, but operates on "shadow" values.
@@ -1035,7 +1036,7 @@
   if (LangOpts.Sanitize.has(SanitizerKind::Memory))
 PB.registerOptimizerLastEPCallback(
 [](FunctionPassManager , PassBuilder::OptimizationLevel Level) 
{
-  FPM.addPass(MemorySanitizerPass());
+  FPM.addPass(MemorySanitizerPass({}));
 });
   if (LangOpts.Sanitize.has(SanitizerKind::Thread))
 PB.registerOptimizerLastEPCallback(


Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -279,7 +279,8 @@
   const CodeGenOptions  = BuilderWrapper.getCGOpts();
   int TrackOrigins = CGOpts.SanitizeMemoryTrackOrigins;
   bool Recover = CGOpts.SanitizeRecover.has(SanitizerKind::Memory);
-  PM.add(createMemorySanitizerLegacyPassPass(TrackOrigins, Recover, CompileKernel));
+  PM.add(createMemorySanitizerLegacyPassPass(
+  MemorySanitizerOptions{TrackOrigins, Recover, CompileKernel}));
 
   // MemorySanitizer inserts complex instrumentation that mostly follows
   // the logic of the original code, but operates on "shadow" values.
@@ -1035,7 +1036,7 @@
   if (LangOpts.Sanitize.has(SanitizerKind::Memory))
 PB.registerOptimizerLastEPCallback(
 [](FunctionPassManager , PassBuilder::OptimizationLevel Level) {
-  FPM.addPass(MemorySanitizerPass());
+  FPM.addPass(MemorySanitizerPass({}));
 });
   if (LangOpts.Sanitize.has(SanitizerKind::Thread))
 PB.registerOptimizerLastEPCallback(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: r351629 - Emit !callback metadata and introduce the callback attribute

2019-02-04 Thread Doerfert, Johannes via cfe-commits
Should be fixed in r353088 (git c3707cc5d91). Can you verify it now is as you 
expect it?

From: Eli Friedman 
Sent: Thursday, January 31, 2019 18:17
To: Doerfert, Johannes; Chandler Carruth; cfe-commits@lists.llvm.org
Cc: Raja Venkateswaran
Subject: RE: r351629 - Emit !callback metadata and introduce the callback 
attribute

(Comments inline.)

> -Original Message-
> From: cfe-commits  On Behalf Of
> Doerfert, Johannes Rudolf via cfe-commits
> Sent: Tuesday, January 22, 2019 9:29 AM
> To: Chandler Carruth 
> Cc: cfe-commits cfe 
> Subject: Re: r351629 - Emit !callback metadata and introduce the callback
> attribute
>
> > Another thing I notecide is that this code assumes the system has
> > `pthread.h` -- what about systems without it? I mean, you can disable the
> > test, but it seems bad to lose test coverage just because of that.
>
> So far, I disabled the test with a later addition which makes sure this
> test is only run under Linux. I'm unsure why we loose coverage because
> of that?

There isn't any way to safely disable the test without disabling it everywhere. 
 Specifically, the current version requires both that the host is Unix, and 
that the default target is the host.  Otherwise, the compiler might not be able 
to find and/or build pthread.h .

>
> > I would much prefer that you provide your own stub `pthread.h` in the
> > Inputs/... tree of the test suite and use that to test this in a portable
> > way.
>
> I do not completely follow but I'm open to improving the test. Basically
> I have to make sure the builtin recognition will trigger on the header
> file and the contained declaration. If we can somehow do this in a
> portable way I'm all for it. Is that how we test other builtin gnu extensions?

You don't need a header file; clang doesn't actually care where the builtin is 
declared.  You can just write "void pthread_create(void*,void*,void*(void*), 
void*);" or whatever directly in the C file.  It'll trigger a warning, but with 
your patch, there's no way to declare pthread_create without triggering a 
warning, so that hardly matters.

On a related note, code generation tests should use %clang_cc1.  Among other 
things, this avoids accidentally including headers from the host system.

-Eli

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


r353088 - Generalize pthread callback test case

2019-02-04 Thread Johannes Doerfert via cfe-commits
Author: jdoerfert
Date: Mon Feb  4 12:42:38 2019
New Revision: 353088

URL: http://llvm.org/viewvc/llvm-project?rev=353088=rev
Log:
Generalize pthread callback test case

Changes suggested by Eli Friedman 

Modified:
cfe/trunk/test/CodeGen/callback_pthread_create.c

Modified: cfe/trunk/test/CodeGen/callback_pthread_create.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/callback_pthread_create.c?rev=353088=353087=353088=diff
==
--- cfe/trunk/test/CodeGen/callback_pthread_create.c (original)
+++ cfe/trunk/test/CodeGen/callback_pthread_create.c Mon Feb  4 12:42:38 2019
@@ -1,14 +1,22 @@
-// RUN: %clang -O1 %s -S -c -emit-llvm -o - | FileCheck %s
-// RUN: %clang -O1 %s -S -c -emit-llvm -o - | opt -ipconstprop -S | FileCheck 
--check-prefix=IPCP %s
-
-// This is a linux only test for now due to the include.
-// UNSUPPORTED: !linux
+// RUN: %clang_cc1 -O1 %s -S -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -O1 %s -S -emit-llvm -o - | opt -ipconstprop -S | FileCheck 
--check-prefix=IPCP %s
 
 // CHECK: declare !callback ![[cid:[0-9]+]] {{.*}}i32 @pthread_create
 // CHECK: ![[cid]] = !{![[cidb:[0-9]+]]}
 // CHECK: ![[cidb]] = !{i64 2, i64 3, i1 false}
 
-#include 
+// Taken from test/Analysis/retain-release.m
+//{
+struct _opaque_pthread_t {};
+struct _opaque_pthread_attr_t {};
+typedef struct _opaque_pthread_t *__darwin_pthread_t;
+typedef struct _opaque_pthread_attr_t __darwin_pthread_attr_t;
+typedef __darwin_pthread_t pthread_t;
+typedef __darwin_pthread_attr_t pthread_attr_t;
+
+int pthread_create(pthread_t *, const pthread_attr_t *,
+   void *(*)(void *), void *);
+//}
 
 const int GlobalVar = 0;
 
@@ -26,8 +34,8 @@ static void *callee1(void *payload) {
 
 void foo() {
   pthread_t MyFirstThread;
-  pthread_create(, NULL, callee0, NULL);
+  pthread_create(, 0, callee0, 0);
 
   pthread_t MySecondThread;
-  pthread_create(, NULL, callee1, (void *));
+  pthread_create(, 0, callee1, (void *));
 }


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


[PATCH] D57690: [OPENMP] issue error messages for multiple teams contructs in a target constructs

2019-02-04 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev accepted this revision.
ABataev added a comment.
This revision is now accepted and ready to land.

LG


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

https://reviews.llvm.org/D57690



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


[PATCH] D57690: [OPENMP] issue error messages for multiple teams contructs in a target constructs

2019-02-04 Thread Kelvin Li via Phabricator via cfe-commits
kkwli0 updated this revision to Diff 185125.
kkwli0 marked an inline comment as done.
kkwli0 added a comment.

Update based on review comment.


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

https://reviews.llvm.org/D57690

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/nesting_of_regions.cpp


Index: clang/test/OpenMP/nesting_of_regions.cpp
===
--- clang/test/OpenMP/nesting_of_regions.cpp
+++ clang/test/OpenMP/nesting_of_regions.cpp
@@ -4080,6 +4080,13 @@
   }
 #pragma omp target // expected-error {{target construct with nested teams 
region contains statements outside of the teams construct}}
   {
+#pragma omp teams // expected-note {{directive outside teams construct here}}
+++a;
+#pragma omp teams // expected-note {{nested teams construct here}}
+++a;
+  }
+#pragma omp target // expected-error {{target construct with nested teams 
region contains statements outside of the teams construct}}
+  {
 ++a;   // expected-note {{statement outside teams construct here}}
 #pragma omp teams  // expected-note {{nested teams construct here}}
 ++a;
@@ -12693,6 +12700,13 @@
   }
 #pragma omp target // expected-error {{target construct with nested teams 
region contains statements outside of the teams construct}}
   {
+#pragma omp teams // expected-note {{directive outside teams construct here}}
+++a;
+#pragma omp teams // expected-note {{nested teams construct here}}
+++a;
+  }
+#pragma omp target // expected-error {{target construct with nested teams 
region contains statements outside of the teams construct}}
+  {
 ++a;  // expected-note {{statement outside teams construct here}}
 #pragma omp teams // expected-note {{nested teams construct here}}
 ++a;
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -7067,7 +7067,9 @@
   auto I = CS->body_begin();
   while (I != CS->body_end()) {
 const auto *OED = dyn_cast(*I);
-if (!OED || !isOpenMPTeamsDirective(OED->getDirectiveKind())) {
+if (!OED || !isOpenMPTeamsDirective(OED->getDirectiveKind()) ||
+OMPTeamsFound) {
+
   OMPTeamsFound = false;
   break;
 }


Index: clang/test/OpenMP/nesting_of_regions.cpp
===
--- clang/test/OpenMP/nesting_of_regions.cpp
+++ clang/test/OpenMP/nesting_of_regions.cpp
@@ -4080,6 +4080,13 @@
   }
 #pragma omp target // expected-error {{target construct with nested teams region contains statements outside of the teams construct}}
   {
+#pragma omp teams // expected-note {{directive outside teams construct here}}
+++a;
+#pragma omp teams // expected-note {{nested teams construct here}}
+++a;
+  }
+#pragma omp target // expected-error {{target construct with nested teams region contains statements outside of the teams construct}}
+  {
 ++a;   // expected-note {{statement outside teams construct here}}
 #pragma omp teams  // expected-note {{nested teams construct here}}
 ++a;
@@ -12693,6 +12700,13 @@
   }
 #pragma omp target // expected-error {{target construct with nested teams region contains statements outside of the teams construct}}
   {
+#pragma omp teams // expected-note {{directive outside teams construct here}}
+++a;
+#pragma omp teams // expected-note {{nested teams construct here}}
+++a;
+  }
+#pragma omp target // expected-error {{target construct with nested teams region contains statements outside of the teams construct}}
+  {
 ++a;  // expected-note {{statement outside teams construct here}}
 #pragma omp teams // expected-note {{nested teams construct here}}
 ++a;
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -7067,7 +7067,9 @@
   auto I = CS->body_begin();
   while (I != CS->body_end()) {
 const auto *OED = dyn_cast(*I);
-if (!OED || !isOpenMPTeamsDirective(OED->getDirectiveKind())) {
+if (!OED || !isOpenMPTeamsDirective(OED->getDirectiveKind()) ||
+OMPTeamsFound) {
+
   OMPTeamsFound = false;
   break;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D57690: [OPENMP] issue error messages for multiple teams contructs in a target constructs

2019-02-04 Thread Kelvin Li via Phabricator via cfe-commits
kkwli0 marked 2 inline comments as done.
kkwli0 added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:7070
 const auto *OED = dyn_cast(*I);
-if (!OED || !isOpenMPTeamsDirective(OED->getDirectiveKind())) {
+if (!OED || !isOpenMPTeamsDirective(OED->getDirectiveKind()) ||
+(isOpenMPTeamsDirective(OED->getDirectiveKind()) &&

ABataev wrote:
> just `if (!OED || !isOpenMPTeamsDirective(OED->getDirectiveKind() || 
> OMPTeamsFound)`
Yep, it simplifies the logic.


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

https://reviews.llvm.org/D57690



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


[PATCH] D40815: [libcxxabi] Use the correct variable name for target triple in lit

2019-02-04 Thread Petr Hosek via Phabricator via cfe-commits
phosek abandoned this revision.
phosek added a comment.
Herald added subscribers: libcxx-commits, ldionne.

This is no longer needed after D57670  landed.


Repository:
  rCXXA libc++abi

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

https://reviews.llvm.org/D40815



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


[PATCH] D40816: [libunwind] Use the correct variable name for target triple in lit

2019-02-04 Thread Petr Hosek via Phabricator via cfe-commits
phosek abandoned this revision.
phosek added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

This is no longer needed after D57670  landed.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D40816



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


[PATCH] D40814: [libcxx] Use the correct variable name for target triple in lit

2019-02-04 Thread Petr Hosek via Phabricator via cfe-commits
phosek abandoned this revision.
phosek added a comment.
Herald added subscribers: libcxx-commits, ldionne.

This is no longer needed after D57670  landed.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D40814



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


[PATCH] D57636: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg

2019-02-04 Thread Tom Tan via Phabricator via cfe-commits
TomTan added a comment.

Thanks for finding out and fixing this. Seems there is also issue in expanding 
`_WriteStatusReg` in `CodeGenFunction::EmitAArch64BuiltinExpr`. The last 
argument for `_WriteStatusReg` is __zero extended__ to `__in64`, which is not 
expected (see below link).

https://github.com/llvm-mirror/clang/blob/8070ca12f87e66f76db528c107e9d291f4a91498/lib/CodeGen/CGBuiltin.cpp#L7100


Repository:
  rC Clang

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

https://reviews.llvm.org/D57636



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


[PATCH] D57631: [COFF, ARM64] Add ARM64 support for MS intrinsic _fastfail

2019-02-04 Thread Tom Tan via Phabricator via cfe-commits
TomTan added a comment.

In D57631#1381806 , @efriedma wrote:

> LGTM


Thanks Eli. Could you please help commit this change when it is ready?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57631



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


[libunwind] r353084 - [CMake] Support CMake variables for setting target, sysroot and toolchain

2019-02-04 Thread Petr Hosek via cfe-commits
Author: phosek
Date: Mon Feb  4 12:02:26 2019
New Revision: 353084

URL: http://llvm.org/viewvc/llvm-project?rev=353084=rev
Log:
[CMake] Support CMake variables for setting target, sysroot and toolchain

CMake has a standard way of setting target triple, sysroot and external
toolchain through CMAKE__COMPILER_TARGET, CMAKE_SYSROOT and
CMAKE__COMPILER_EXTERNAL_TOOLCHAIN. These are turned into
corresponding --target=, --sysroot= and --gcc-toolchain= variables add
included appended to CMAKE__FLAGS.

libunwind, libc++abi, libc++ provides their own mechanism through
_TARGET_TRIPLE, _SYSROOT and _GCC_TOOLCHAIN
variables. These are also passed to lit via lit.site.cfg, and lit config
uses these to set the corresponding compiler flags when building tessts.

This means that there are two different ways of setting target, sysroot
and toolchain, but only one is properly supported in lit. This change
extends CMake build for libunwind, libc++abi and libc++ to also support
the CMake variables in addition to project specific ones in lit.

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

Modified:
libunwind/trunk/CMakeLists.txt
libunwind/trunk/test/lit.site.cfg.in

Modified: libunwind/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/CMakeLists.txt?rev=353084=353083=353084=diff
==
--- libunwind/trunk/CMakeLists.txt (original)
+++ libunwind/trunk/CMakeLists.txt Mon Feb  4 12:02:26 2019
@@ -225,12 +225,22 @@ macro(add_target_flags_if condition var)
 endmacro()
 
 add_target_flags_if(LIBUNWIND_BUILD_32_BITS "-m32")
-add_target_flags_if(LIBUNWIND_TARGET_TRIPLE
-  "--target=${LIBUNWIND_TARGET_TRIPLE}")
-add_target_flags_if(LIBUNWIND_GCC_TOOLCHAIN
-  "--gcc-toolchain=${LIBUNWIND_GCC_TOOLCHAIN}")
-add_target_flags_if(LIBUNWIND_SYSROOT
-  "--sysroot=${LIBUNWIND_SYSROOT}")
+
+if(LIBUNWIND_TARGET_TRIPLE)
+  add_target_flags("--target=${LIBUNWIND_TARGET_TRIPLE}")
+elseif(CMAKE_CXX_COMPILER_TARGET)
+  set(LIBUNWIND_TARGET_TRIPLE "${CMAKE_CXX_COMPILER_TARGET}")
+endif()
+if(LIBUNWIND_GCC_TOOLCHAIN)
+  add_target_flags("--gcc-toolchain=${LIBUNWIND_GCC_TOOLCHAIN}")
+elseif(CMAKE_CXX_COMPILER_EXTERNAL_TOOLCHAIN)
+  set(LIBUNWIND_GCC_TOOLCHAIN "${CMAKE_CXX_COMPILER_EXTERNAL_TOOLCHAIN}")
+endif()
+if(LIBUNWIND_SYSROOT)
+  add_target_flags("--sysroot=${LIBUNWIND_SYSROOT}")
+elseif(CMAKE_SYSROOT)
+  set(LIBUNWIND_SYSROOT "${CMAKE_SYSROOT}")
+endif()
 
 if (LIBUNWIND_TARGET_TRIPLE)
   set(TARGET_TRIPLE "${LIBUNWIND_TARGET_TRIPLE}")

Modified: libunwind/trunk/test/lit.site.cfg.in
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/test/lit.site.cfg.in?rev=353084=353083=353084=diff
==
--- libunwind/trunk/test/lit.site.cfg.in (original)
+++ libunwind/trunk/test/lit.site.cfg.in Mon Feb  4 12:02:26 2019
@@ -20,7 +20,7 @@ config.enable_shared= "@LIBC
 config.enable_exceptions= "@LIBUNWIND_ENABLE_EXCEPTIONS@"
 config.host_triple  = "@LLVM_HOST_TRIPLE@"
 config.target_triple= "@TARGET_TRIPLE@"
-config.use_target   = len("@LIBUNWIND_TARGET_TRIPLE@") > 0
+config.use_target   = bool("@LIBUNWIND_TARGET_TRIPLE@")
 config.sysroot  = "@LIBUNWIND_SYSROOT@"
 config.gcc_toolchain= "@LIBUNWIND_GCC_TOOLCHAIN@"
 config.cxx_ext_threads  = "@LIBUNWIND_BUILD_EXTERNAL_THREAD_LIBRARY@"


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


[PATCH] D57626: Disallow trivial_abi on a class if all copy and move constructors are deleted

2019-02-04 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

Richard felt that this was an odd special case, and I was happy to use the old 
language-designer's dodge of banning something today so that we can decide to 
allow it tomorrow.  This isn't SFINAE-able.

...of course, if it's just a *warning* that this isn't allowed, that dodge 
doesn't quite work.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57626



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


[PATCH] D57524: Fix ICE on attempt to add an addr space qual to a type qualified by an addr space

2019-02-04 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

Okay.


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

https://reviews.llvm.org/D57524



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


[PATCH] D57674: [clang-tidy] Add options to bugprone-argument-comment to add missing argument comments to literals

2019-02-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

Can you drop the file mode changes that are in this review?




Comment at: clang-tidy/bugprone/ArgumentCommentCheck.cpp:292-293
+// If the argument comments are missing for literals add them.
+if (Comments.empty()) {
+  if (((isa(Args[I]) && AddCommentsToBoolLiterals) ||
+   (isa(Args[I]) && AddCommentsToIntegerLiterals) ||

These can be combined into a single `if` statement.



Comment at: clang-tidy/bugprone/ArgumentCommentCheck.cpp:300-301
+diag(Args[I]->getBeginLoc(),
+ "argument comment missing for literal argument"
+ " %0")
+<< II

Why is this split into two lines?



Comment at: clang-tidy/bugprone/ArgumentCommentCheck.cpp:304
+<< FixItHint::CreateInsertion(Args[I]->getBeginLoc(), ArgComment);
+continue;
+  }

This `continue` can be dropped without changing the semantics, correct?



Comment at: clang-tidy/bugprone/ArgumentCommentCheck.h:44-46
+  const bool AddCommentsToBoolLiterals;
+  const bool AddCommentsToIntegerLiterals;
+  const bool AddCommentsToFloatLiterals;

Why not character or string literals?
What about `nullptr` literals or UDLs?



Comment at: docs/clang-tidy/checks/bugprone-argument-comment.rst:40
+
+  void foo(bool turn_key,bool press_button);
+

Format the code examples from our style guide as well (same below).


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

https://reviews.llvm.org/D57674



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


[PATCH] D57674: [clang-tidy] Add options to bugprone-argument-comment to add missing argument comments to literals

2019-02-04 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments.



Comment at: docs/ReleaseNotes.rst:82
 
+- The :doc:`bugprone-argument-comment
+  ` now supports 

Please move changes in existing checks after list of new checks.


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

https://reviews.llvm.org/D57674



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


[PATCH] D57353: [clang-tidy] Add the abseil-duration-unnecessary-conversion check

2019-02-04 Thread Hyrum Wright via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE353079: [clang-tidy] Add the 
abseil-duration-unnecessary-conversion check (authored by hwright, committed by 
).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D57353?vs=185057=185104#toc

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57353

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp
  clang-tidy/abseil/DurationUnnecessaryConversionCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/Inputs/absl/time/time.h
  test/clang-tidy/abseil-duration-unnecessary-conversion.cpp

Index: clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp
===
--- clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp
+++ clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp
@@ -0,0 +1,58 @@
+//===--- DurationUnnecessaryConversionCheck.cpp - clang-tidy
+//---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "DurationUnnecessaryConversionCheck.h"
+#include "DurationRewriter.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Tooling/FixIt.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+void DurationUnnecessaryConversionCheck::registerMatchers(MatchFinder *Finder) {
+  for (const auto  : {"Hours", "Minutes", "Seconds", "Milliseconds",
+"Microseconds", "Nanoseconds"}) {
+std::string DurationFactory = (llvm::Twine("::absl::") + Scale).str();
+std::string FloatConversion =
+(llvm::Twine("::absl::ToDouble") + Scale).str();
+std::string IntegerConversion =
+(llvm::Twine("::absl::ToInt64") + Scale).str();
+
+Finder->addMatcher(
+callExpr(
+callee(functionDecl(hasName(DurationFactory))),
+hasArgument(0, callExpr(callee(functionDecl(hasAnyName(
+FloatConversion, IntegerConversion))),
+hasArgument(0, expr().bind("arg")
+.bind("call"),
+this);
+  }
+}
+
+void DurationUnnecessaryConversionCheck::check(
+const MatchFinder::MatchResult ) {
+  const auto *OuterCall = Result.Nodes.getNodeAs("call");
+  const auto *Arg = Result.Nodes.getNodeAs("arg");
+
+  if (!isNotInMacro(Result, OuterCall))
+return;
+
+  diag(OuterCall->getBeginLoc(), "remove unnecessary absl::Duration conversions")
+  << FixItHint::CreateReplacement(
+ OuterCall->getSourceRange(),
+ tooling::fixit::getText(*Arg, *Result.Context));
+}
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/abseil/CMakeLists.txt
===
--- clang-tidy/abseil/CMakeLists.txt
+++ clang-tidy/abseil/CMakeLists.txt
@@ -10,6 +10,7 @@
   DurationFactoryScaleCheck.cpp
   DurationRewriter.cpp
   DurationSubtractionCheck.cpp
+  DurationUnnecessaryConversionCheck.cpp
   FasterStrsplitDelimiterCheck.cpp
   NoInternalDependenciesCheck.cpp
   NoNamespaceCheck.cpp
Index: clang-tidy/abseil/DurationUnnecessaryConversionCheck.h
===
--- clang-tidy/abseil/DurationUnnecessaryConversionCheck.h
+++ clang-tidy/abseil/DurationUnnecessaryConversionCheck.h
@@ -0,0 +1,35 @@
+//===--- DurationUnnecessaryConversionCheck.h - clang-tidy --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_TIMEDOUBLECONVERSIONCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_TIMEDOUBLECONVERSIONCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// Finds and fixes cases where ``absl::Duration`` values are being converted
+/// to numeric types and back again.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-duration-unnecessary-conversion.html
+class DurationUnnecessaryConversionCheck : public ClangTidyCheck {
+public:
+  DurationUnnecessaryConversionCheck(StringRef Name, ClangTidyContext 

[clang-tools-extra] r353079 - [clang-tidy] Add the abseil-duration-unnecessary-conversion check

2019-02-04 Thread Hyrum Wright via cfe-commits
Author: hwright
Date: Mon Feb  4 11:28:20 2019
New Revision: 353079

URL: http://llvm.org/viewvc/llvm-project?rev=353079=rev
Log:
[clang-tidy] Add the abseil-duration-unnecessary-conversion check

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

Added:

clang-tools-extra/trunk/clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp

clang-tools-extra/trunk/clang-tidy/abseil/DurationUnnecessaryConversionCheck.h

clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst

clang-tools-extra/trunk/test/clang-tidy/abseil-duration-unnecessary-conversion.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/time/time.h

Modified: clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp?rev=353079=353078=353079=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp Mon Feb  4 
11:28:20 2019
@@ -16,6 +16,7 @@
 #include "DurationFactoryFloatCheck.h"
 #include "DurationFactoryScaleCheck.h"
 #include "DurationSubtractionCheck.h"
+#include "DurationUnnecessaryConversionCheck.h"
 #include "FasterStrsplitDelimiterCheck.h"
 #include "NoInternalDependenciesCheck.h"
 #include "NoNamespaceCheck.h"
@@ -45,6 +46,8 @@ public:
 "abseil-duration-factory-scale");
 CheckFactories.registerCheck(
 "abseil-duration-subtraction");
+CheckFactories.registerCheck(
+"abseil-duration-unnecessary-conversion");
 CheckFactories.registerCheck(
 "abseil-faster-strsplit-delimiter");
 CheckFactories.registerCheck(

Modified: clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt?rev=353079=353078=353079=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt Mon Feb  4 
11:28:20 2019
@@ -10,6 +10,7 @@ add_clang_library(clangTidyAbseilModule
   DurationFactoryScaleCheck.cpp
   DurationRewriter.cpp
   DurationSubtractionCheck.cpp
+  DurationUnnecessaryConversionCheck.cpp
   FasterStrsplitDelimiterCheck.cpp
   NoInternalDependenciesCheck.cpp
   NoNamespaceCheck.cpp

Added: 
clang-tools-extra/trunk/clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp?rev=353079=auto
==
--- 
clang-tools-extra/trunk/clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp
 (added)
+++ 
clang-tools-extra/trunk/clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp
 Mon Feb  4 11:28:20 2019
@@ -0,0 +1,58 @@
+//===--- DurationUnnecessaryConversionCheck.cpp - clang-tidy
+//---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "DurationUnnecessaryConversionCheck.h"
+#include "DurationRewriter.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Tooling/FixIt.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+void DurationUnnecessaryConversionCheck::registerMatchers(MatchFinder *Finder) 
{
+  for (const auto  : {"Hours", "Minutes", "Seconds", "Milliseconds",
+"Microseconds", "Nanoseconds"}) {
+std::string DurationFactory = (llvm::Twine("::absl::") + Scale).str();
+std::string FloatConversion =
+(llvm::Twine("::absl::ToDouble") + Scale).str();
+std::string IntegerConversion =
+(llvm::Twine("::absl::ToInt64") + Scale).str();
+
+Finder->addMatcher(
+callExpr(
+callee(functionDecl(hasName(DurationFactory))),
+hasArgument(0, callExpr(callee(functionDecl(hasAnyName(
+FloatConversion, IntegerConversion))),
+hasArgument(0, expr().bind("arg")
+.bind("call"),
+this);
+  }
+}
+
+void DurationUnnecessaryConversionCheck::check(
+const MatchFinder::MatchResult ) {
+  const auto *OuterCall = Result.Nodes.getNodeAs("call");
+  const 

[PATCH] D56935: [NewPM] Add support for new-PM plugins to clang

2019-02-04 Thread Marco Elver via Phabricator via cfe-commits
melver added a comment.

In D56935#1382156 , @philip.pfaffe 
wrote:

> Landed it for you in r352972. Thanks!


Great, thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D56935



___
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

2019-02-04 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 marked 12 inline comments as done.
jyu2 added inline comments.



Comment at: include/clang/AST/Stmt.h:1008
+ (*iterator_adaptor_base::I)->getStmtClass() <= lastExprConstant);
+  return *reinterpret_cast(iterator_adaptor_base::I);
 }

rsmith wrote:
> Ugh. This cast violates strict aliasing, and even in the absence of strict 
> aliasing won't work unless the `Stmt` base class is at offset 0 in `T`. The 
> preceding assert is also wrong, as it's asserting that `*I` is an `Expr`, not 
> that it's a `T`.
> 
> After r352925, you can use `CastIterator` instead; that should 
> substantially reduce the churn in this patch.
Hi Richard,
Thanks for explain for this. And thank you so much for proving CastIterator!!  
I change the code to use this.




Comment at: include/clang/AST/Stmt.h:2842
+
+  bool isGCCAsmGoto() const {
+return NumLabels > 0;

rsmith wrote:
> This is already inside a class `GCCAsmStmt`; the `GCC` here seems unnecessary.
Remove GCC



Comment at: include/clang/AST/Stmt.h:2860
+  StringRef getLabelName(unsigned i) const;
+  Expr *getExpr(unsigned i) const;
+  using labels_iterator = ExprIterator;

rsmith wrote:
> Please give this a better name; `getExpr` is pretty meaningless.
should I change to getOperandExpr?
Thanks.



Comment at: lib/Analysis/CFG.cpp:1474
+  appendScopeBegin(JT.block, VD, G);
+  addSuccessor(B, JT.block);
+}

jyu2 wrote:
> efriedma wrote:
> > Please don't copy-paste code.
> :-(  changed
I did this during in VisitGCCAsmStmt by not call addSuccessor when we need 
backpatch the the labels.



Comment at: lib/Analysis/CFG.cpp:1466
+  LabelMapTy::iterator LI = LabelMap.find(E->getLabel());
+  if (LI == LabelMap.end()) continue;
+  JT = LI->second;

rsmith wrote:
> If this happens for every iteration of the loop, `VD` is used uninitialized 
> after the loop.
I am totally wrong.  Changed



Comment at: lib/Parse/ParseStmtAsm.cpp:839
+if (Tok.isNot(tok::r_paren)) {
+  while (1) {
+LabelDecl *LD = Actions.LookupOrCreateLabel(Tok.getIdentifierInfo(),

rsmith wrote:
> `while (true)`
Yes.  Changed.
Thanks.


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

https://reviews.llvm.org/D56571



___
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

2019-02-04 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

How do we actually want this to work for LTO?  Would it make sense to encode 
this on global variables somehow, to allow different thresholds for different 
source files?


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] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-04 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 updated this revision to Diff 185094.
jyu2 added a comment.

1> Add code for scope checking
2> Using CastInterator
3> CFG change to remove duplicate Block list


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

https://reviews.llvm.org/D56571

Files:
  include/clang/AST/Stmt.h
  include/clang/Basic/DiagnosticASTKinds.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.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,18 @@
 
 // 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 goto statement to its label}}
+  // expected-error@+1 {{cannot jump from this goto statement to its label}}
+  asm volatile goto("testl %0, %0; jne %l1;" :: "r"(n)::label_true, loop);
+  // expected-note@+1 {{jump bypasses initialization of variable length array}}
+  return ({int a[n];label_true: 2;});
+  // expected-note@+1 {{jump bypasses initialization of variable length array}}
+  int b[n];
+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,28 @@
   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 {{operand with 'l' modifier must refer to a label}}
+  asm ("jne %l0"::"r"(&));
+  // expected-error@+1 {{invalid operand number in inline asm string}}
+  asm ("jne %l0":::);
+  // expected-error@+1 {{operand with 'l' modifier must refer to a label}}
+  asm goto ("jne %l0"::"r"(x)::lab);
+  asm goto ("jne %l0"lab);
+}
Index: test/Parser/asm.cpp
===
--- test/Parser/asm.cpp
+++ test/Parser/asm.cpp
@@ -7,3 +7,54 @@
 int foo5 asm (U"bar5"); // expected-error {{cannot use unicode string literal in 'asm'}}
 int foo6 asm ("bar6"_x); // expected-error {{string literal with user-defined suffix cannot be used here}}
 int foo6 asm ("" L"bar7"); // expected-error {{cannot use wide string literal in 'asm'}}
+
+int zoo ()
+{
+  int x,cond,*e;
+  // expected-error@+1 {{expected ')'}}
+  asm ("mov %[e], %[e]" : : [e] "rm" (*e)::a)
+  // expected-error@+1  {{expected ':'}}
+  asm goto ("decl %0; jnz %l[a]" :"=r"(x): "m"(x) : "memory" : a);
+  // expected-error@+1 {{expected identifie}}
+  asm goto ("decl %0;" :: "m"(x) : "memory" : );
+  // expected-error@+1  {{expected ':'}}
+  asm goto ("decl %0;" :: "m"(x) : "memory" );
+  // expected-error@+1 {{use of undeclared label 'x'}}
+  asm goto ("decl %0;" :: "m"(x) : "memory" :x);
+  // expected-error@+1 {{use of undeclared label 'b'}}
+  asm goto ("decl %0;" :: "m"(x) : "memory" :b);
+  // expected-error@+1 {{invalid operand number in inline asm string}}
+  asm goto ("testl %0, %0; jne %l3;" :: "r"(cond)::label_true, loop)
+  // expected-error@+1 {{unknown symbolic operand name in inline 

[PATCH] D57636: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg

2019-02-04 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a subscriber: TomTan.
efriedma added a comment.

Missing testcase changes.  (It should be possible to check that we aren't 
inserting incorrect truncation/extension operations in the IR.)


Repository:
  rC Clang

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

https://reviews.llvm.org/D57636



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


[PATCH] D57581: Explicitly add language standard option to test cases that rely on the C++14 default

2019-02-04 Thread Nemanja Ivanovic via Phabricator via cfe-commits
nemanjai updated this revision to Diff 185093.
nemanjai added a comment.

Changed the option to standard C++ rather than GNU extensions.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57581

Files:
  test/CodeCompletion/crash-skipped-bodies-template-inst.cpp
  test/CodeCompletion/skip-auto-funcs.cpp
  test/CodeGenCXX/auto-var-init.cpp
  test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
  test/CodeGenCXX/new-overflow.cpp
  test/CodeGenCXX/new.cpp
  test/Lexer/cxx-features.cpp
  test/Lexer/half-literal.cpp
  test/Modules/friend-definition-2.cpp
  test/Modules/merge-lambdas.cpp
  test/SemaCXX/int-ptr-cast-SFINAE.cpp
  test/SemaTemplate/argument-dependent-lookup.cpp
  test/SemaTemplate/class-template-decl.cpp
  test/SemaTemplate/typo-dependent-name.cpp

Index: test/SemaTemplate/typo-dependent-name.cpp
===
--- test/SemaTemplate/typo-dependent-name.cpp
+++ test/SemaTemplate/typo-dependent-name.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify %s
 
 using nullptr_t = decltype(nullptr);
 
Index: test/SemaTemplate/class-template-decl.cpp
===
--- test/SemaTemplate/class-template-decl.cpp
+++ test/SemaTemplate/class-template-decl.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify %s
 
 template class A;
 
Index: test/SemaTemplate/argument-dependent-lookup.cpp
===
--- test/SemaTemplate/argument-dependent-lookup.cpp
+++ test/SemaTemplate/argument-dependent-lookup.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -verify %s
-// RUN: %clang_cc1 -verify %s -DHAVE_UNQUALIFIED_LOOKUP_RESULTS
+// RUN: %clang_cc1 -std=c++14 -verify %s
+// RUN: %clang_cc1 -std=c++14 -verify %s -DHAVE_UNQUALIFIED_LOOKUP_RESULTS
 // expected-no-diagnostics
 
 namespace address_of {
Index: test/SemaCXX/int-ptr-cast-SFINAE.cpp
===
--- test/SemaCXX/int-ptr-cast-SFINAE.cpp
+++ test/SemaCXX/int-ptr-cast-SFINAE.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++14
 // RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++17
 
 void foo(int* a, int *b) {
Index: test/Modules/merge-lambdas.cpp
===
--- test/Modules/merge-lambdas.cpp
+++ test/Modules/merge-lambdas.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fmodules -verify %s -emit-llvm-only
+// RUN: %clang_cc1 -std=c++14 -fmodules -verify %s -emit-llvm-only
 // expected-no-diagnostics
 
 #pragma clang module build A
Index: test/Modules/friend-definition-2.cpp
===
--- test/Modules/friend-definition-2.cpp
+++ test/Modules/friend-definition-2.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fmodules %s -verify
-// RUN: %clang_cc1 -fmodules %s -verify -triple i686-windows
+// RUN: %clang_cc1 -std=c++14 -fmodules %s -verify
+// RUN: %clang_cc1 -std=c++14 -fmodules %s -verify -triple i686-windows
 // expected-no-diagnostics
 #pragma clang module build A
 module A {}
Index: test/Lexer/half-literal.cpp
===
--- test/Lexer/half-literal.cpp
+++ test/Lexer/half-literal.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -pedantic -triple aarch64-linux-gnu %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify -pedantic -triple aarch64-linux-gnu %s
 float a = 1.0h; // expected-error{{no matching literal operator for call to 'operator""h' with argument of type 'long double' or 'const char *', and no matching literal operator template}}
 float b = 1.0H; // expected-error{{invalid suffix 'H' on floating constant}}
 
Index: test/Lexer/cxx-features.cpp
===
--- test/Lexer/cxx-features.cpp
+++ test/Lexer/cxx-features.cpp
@@ -6,9 +6,9 @@
 //
 // RUN: %clang_cc1 -std=c++17 -fcxx-exceptions -fsized-deallocation -frelaxed-template-template-args -DRELAXED_TEMPLATE_TEMPLATE_ARGS=1 -verify %s
 // RUN: %clang_cc1 -std=c++17 -fcxx-exceptions -fsized-deallocation -fconcepts-ts -DCONCEPTS_TS=1 -verify %s
-// RUN: %clang_cc1 -fno-rtti -fno-threadsafe-statics -verify %s -DNO_EXCEPTIONS -DNO_RTTI -DNO_THREADSAFE_STATICS -fsized-deallocation
-// RUN: %clang_cc1 -fcoroutines-ts -DNO_EXCEPTIONS -DCOROUTINES -verify -fsized-deallocation %s
-// RUN: %clang_cc1 -fchar8_t -DNO_EXCEPTIONS -DCHAR8_T -verify -fsized-deallocation %s
+// RUN: %clang_cc1 -std=c++14 -fno-rtti -fno-threadsafe-statics -verify %s -DNO_EXCEPTIONS -DNO_RTTI -DNO_THREADSAFE_STATICS -fsized-deallocation
+// RUN: %clang_cc1 -std=c++14 -fcoroutines-ts -DNO_EXCEPTIONS -DCOROUTINES -verify -fsized-deallocation %s
+// RUN: 

[PATCH] D57577: Make predefined FLT16 macros conditional on support for the type

2019-02-04 Thread Nemanja Ivanovic via Phabricator via cfe-commits
nemanjai updated this revision to Diff 185088.
nemanjai added a comment.

As mentioned in a comment, the WASM tests weren't really meant to indicate that 
WASM supports the type. Removed the changes to the WASM target.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57577

Files:
  lib/Frontend/InitPreprocessor.cpp
  test/Headers/float16.c
  test/Preprocessor/init.c


Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -9166,20 +9166,6 @@
 // WEBASSEMBLY-NOT:#define __ELF__
 // WEBASSEMBLY-NEXT:#define __FINITE_MATH_ONLY__ 0
 // WEBASSEMBLY-NEXT:#define __FLOAT128__ 1
-// WEBASSEMBLY-NEXT:#define __FLT16_DECIMAL_DIG__ 5
-// WEBASSEMBLY-NEXT:#define __FLT16_DENORM_MIN__ 5.9604644775390625e-8F16
-// WEBASSEMBLY-NEXT:#define __FLT16_DIG__ 3
-// WEBASSEMBLY-NEXT:#define __FLT16_EPSILON__ 9.765625e-4F16
-// WEBASSEMBLY-NEXT:#define __FLT16_HAS_DENORM__ 1
-// WEBASSEMBLY-NEXT:#define __FLT16_HAS_INFINITY__ 1
-// WEBASSEMBLY-NEXT:#define __FLT16_HAS_QUIET_NAN__ 1
-// WEBASSEMBLY-NEXT:#define __FLT16_MANT_DIG__ 11
-// WEBASSEMBLY-NEXT:#define __FLT16_MAX_10_EXP__ 4
-// WEBASSEMBLY-NEXT:#define __FLT16_MAX_EXP__ 15
-// WEBASSEMBLY-NEXT:#define __FLT16_MAX__ 6.5504e+4F16
-// WEBASSEMBLY-NEXT:#define __FLT16_MIN_10_EXP__ (-13)
-// WEBASSEMBLY-NEXT:#define __FLT16_MIN_EXP__ (-14)
-// WEBASSEMBLY-NEXT:#define __FLT16_MIN__ 6.103515625e-5F16
 // WEBASSEMBLY-NEXT:#define __FLT_DECIMAL_DIG__ 9
 // WEBASSEMBLY-NEXT:#define __FLT_DENORM_MIN__ 1.40129846e-45F
 // WEBASSEMBLY-NEXT:#define __FLT_DIG__ 6
Index: test/Headers/float16.c
===
--- test/Headers/float16.c
+++ test/Headers/float16.c
@@ -1,7 +1,11 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -ffreestanding %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -ffreestanding %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c11 -ffreestanding %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -x c++ -ffreestanding %s
+// RUN: %clang_cc1 -triple=aarch64-none-none -fsyntax-only -verify -std=c89 \
+// RUN:   -ffreestanding %s
+// RUN: %clang_cc1 -triple=aarch64-none-none -fsyntax-only -verify \
+// RUN:   -std=c99 -ffreestanding %s
+// RUN: %clang_cc1 -triple=aarch64-none-none -fsyntax-only -verify -std=c11 \
+// RUN:   -ffreestanding %s
+// RUN: %clang_cc1 -triple=aarch64-none-none -fsyntax-only -verify \
+// RUN:   -std=c++11 -x c++ -ffreestanding %s
 // expected-no-diagnostics
 
 #define __STDC_WANT_IEC_60559_TYPES_EXT__
Index: lib/Frontend/InitPreprocessor.cpp
===
--- lib/Frontend/InitPreprocessor.cpp
+++ lib/Frontend/InitPreprocessor.cpp
@@ -830,7 +830,8 @@
   DefineFmt("__UINTPTR", TI.getUIntPtrType(), TI, Builder);
   DefineTypeWidth("__UINTPTR_WIDTH__", TI.getUIntPtrType(), TI, Builder);
 
-  DefineFloatMacros(Builder, "FLT16", (), "F16");
+  if (TI.hasFloat16Type())
+DefineFloatMacros(Builder, "FLT16", (), "F16");
   DefineFloatMacros(Builder, "FLT", (), "F");
   DefineFloatMacros(Builder, "DBL", (), "");
   DefineFloatMacros(Builder, "LDBL", (), "L");


Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -9166,20 +9166,6 @@
 // WEBASSEMBLY-NOT:#define __ELF__
 // WEBASSEMBLY-NEXT:#define __FINITE_MATH_ONLY__ 0
 // WEBASSEMBLY-NEXT:#define __FLOAT128__ 1
-// WEBASSEMBLY-NEXT:#define __FLT16_DECIMAL_DIG__ 5
-// WEBASSEMBLY-NEXT:#define __FLT16_DENORM_MIN__ 5.9604644775390625e-8F16
-// WEBASSEMBLY-NEXT:#define __FLT16_DIG__ 3
-// WEBASSEMBLY-NEXT:#define __FLT16_EPSILON__ 9.765625e-4F16
-// WEBASSEMBLY-NEXT:#define __FLT16_HAS_DENORM__ 1
-// WEBASSEMBLY-NEXT:#define __FLT16_HAS_INFINITY__ 1
-// WEBASSEMBLY-NEXT:#define __FLT16_HAS_QUIET_NAN__ 1
-// WEBASSEMBLY-NEXT:#define __FLT16_MANT_DIG__ 11
-// WEBASSEMBLY-NEXT:#define __FLT16_MAX_10_EXP__ 4
-// WEBASSEMBLY-NEXT:#define __FLT16_MAX_EXP__ 15
-// WEBASSEMBLY-NEXT:#define __FLT16_MAX__ 6.5504e+4F16
-// WEBASSEMBLY-NEXT:#define __FLT16_MIN_10_EXP__ (-13)
-// WEBASSEMBLY-NEXT:#define __FLT16_MIN_EXP__ (-14)
-// WEBASSEMBLY-NEXT:#define __FLT16_MIN__ 6.103515625e-5F16
 // WEBASSEMBLY-NEXT:#define __FLT_DECIMAL_DIG__ 9
 // WEBASSEMBLY-NEXT:#define __FLT_DENORM_MIN__ 1.40129846e-45F
 // WEBASSEMBLY-NEXT:#define __FLT_DIG__ 6
Index: test/Headers/float16.c
===
--- test/Headers/float16.c
+++ test/Headers/float16.c
@@ -1,7 +1,11 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c89 -ffreestanding %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -ffreestanding %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c11 -ffreestanding %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -x c++ -ffreestanding %s
+// RUN: %clang_cc1 

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

2019-02-04 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 commandeered this revision.
jyu2 edited reviewers, added: craig.topper; removed: jyu2.
jyu2 added inline comments.



Comment at: lib/Analysis/CFG.cpp:2126
+  if (cast(S)->isGCCAsmGoto())
+return VisitGCCAsmStmt(cast(S));
+  return VisitStmt(S, asc);

rsmith wrote:
> This is not an appropriate function name for a function that is only called 
> on `asm goto`. Given the convention here is to match the `Stmt` class 
> hierarchy with the `Visit` functions, the branch on `asm goto` should be in 
> the callee.
Changed.



Comment at: lib/CodeGen/CGStmt.cpp:1884
+template 
+void CodeGenFunction::UpdateCallInst(
+T , bool HasSideEffect, bool ReadOnly, bool ReadNone,

efriedma wrote:
> rsmith wrote:
> > This is not a reasonable function name for a function that is specific to 
> > `AsmStmt`s. Please also make this a (static) non-member function, since it 
> > cannot be called outside this source file.
> Does this really need to be a template function?  I think all the functions 
> you're calling on "Result" are members of CallBase.
How about UpdateAsmCallInst?



Comment at: lib/CodeGen/CGStmt.cpp:1884
+template 
+void CodeGenFunction::UpdateCallInst(
+T , bool HasSideEffect, bool ReadOnly, bool ReadNone,

jyu2 wrote:
> efriedma wrote:
> > rsmith wrote:
> > > This is not a reasonable function name for a function that is specific to 
> > > `AsmStmt`s. Please also make this a (static) non-member function, since 
> > > it cannot be called outside this source file.
> > Does this really need to be a template function?  I think all the functions 
> > you're calling on "Result" are members of CallBase.
> How about UpdateAsmCallInst?
Yes. I am using CallBase instead template.



Comment at: lib/CodeGen/CGStmt.cpp:2188
+ std::to_string(NumGccAsmGotoStmts));
+  NumGccAsmGotoStmts++;
+}

efriedma wrote:
> The NumGccAsmGotoStmts variable is pointless; if names are enabled, 
> createBasicBlock will automatically number all the blocks named "normal".
Okay changed.  Thanks.



Comment at: lib/CodeGen/CGStmt.cpp:2238-2245
+UpdateCallInst(*Result, HasSideEffect, ReadOnly,
+ ReadNone, S, ResultRegTypes, RegResults);
+EmitBlock(Fallthrough);
   } else {
-for (unsigned i = 0, e = ResultRegTypes.size(); i != e; ++i) {
-  llvm::Value *Tmp = Builder.CreateExtractValue(Result, i, "asmresult");
-  RegResults.push_back(Tmp);
-}
+llvm::CallInst *Result =
+Builder.CreateCall(IA, Args, getBundlesForFunclet(IA));
+UpdateCallInst(*Result, HasSideEffect, ReadOnly, ReadNone,

rsmith wrote:
> No need to explicitly specify the template argument here; it's deducible.
Thanks.  Changed.



Comment at: lib/Parse/ParseStmtAsm.cpp:833-837
+if (Tok.isNot(tok::identifier)) {
+  Diag(Tok, diag::err_expected) << tok::identifier;
+  SkipUntil(tok::r_paren, StopAtSemi);
+  return StmtError();
+}

rsmith wrote:
> This should be inside the loop below.
Yes.  Change.



Comment at: test/Parser/asm-goto.c:1
+// RUN: %clang_cc1 %s
+

efriedma wrote:
> What is this testing?
Just testing asm goto can parser those input operands and labels.  Not error 
checking.  

Error checking is in Sema/asm-goto


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

https://reviews.llvm.org/D56571



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


[PATCH] D57353: [clang-tidy] Add the abseil-duration-unnecessary-conversion check

2019-02-04 Thread Haojian Wu via Phabricator via cfe-commits
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp:50
+
+  diag(OuterCall->getBeginLoc(), "remove double conversion of absl::Duration")
+  << FixItHint::CreateReplacement(

also avoid `double` word in the message. 



Comment at: docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst:17
+
+  // Suggestion - Remove double conversion
+  absl::Duration d2 = d1;

and here.



Comment at: docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst:25
+
+  // Suggestion - Remove double conversion
+  absl::Duration d2 = d1;

and here.


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

https://reviews.llvm.org/D57353



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


[PATCH] D57662: [clang-tidy] Parallelise clang-tidy-diff.py

2019-02-04 Thread Zinovy Nis via Phabricator via cfe-commits
zinovy.nis updated this revision to Diff 185087.

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

https://reviews.llvm.org/D57662

Files:
  clang-tidy/tool/clang-tidy-diff.py

Index: clang-tidy/tool/clang-tidy-diff.py
===
--- clang-tidy/tool/clang-tidy-diff.py
+++ clang-tidy/tool/clang-tidy-diff.py
@@ -25,9 +25,56 @@
 
 import argparse
 import json
+import multiprocessing
+import os
 import re
 import subprocess
 import sys
+import threading
+
+is_py2 = sys.version[0] == '2'
+
+if is_py2:
+import Queue as queue
+else:
+import queue as queue
+
+def run_tidy(task_queue, lock, timeout):
+  watchdog = None
+  while True:
+command = task_queue.get()
+try:
+  proc = subprocess.Popen(command,
+  stdout=subprocess.PIPE,
+  stderr=subprocess.PIPE)
+
+  if not timeout is None:
+watchdog = threading.Timer(timeout, proc.kill)
+watchdog.start()
+
+  stdout, stderr = proc.communicate()
+
+  with lock:
+sys.stdout.write(' '.join(command) + '\n' + stdout.decode('utf-8') + '\n')
+if stderr:
+  sys.stderr.write(stderr.decode('utf-8') + '\n')
+except:
+  with lock:
+sys.stderr.write('Failed: ' + str(sys.exc_info()[0]) + ' '.join(command) + '\n')
+finally:
+  with lock:
+if (not timeout is None) and (not watchdog is None):
+  if not watchdog.is_alive():
+  sys.stderr.write('Terminated by timeout: ' + ' '.join(command) + '\n')
+  watchdog.cancel()
+  task_queue.task_done()
+
+
+def run_workers(max_tasks, tidy_caller, task_queue, lock, timeout):
+  for _ in range(max_tasks):
+t = threading.Thread(target=tidy_caller, args=(task_queue, lock, timeout))
+t.daemon = True
+t.start()
 
 
 def main():
@@ -48,6 +95,10 @@
   help='custom pattern selecting file paths to check '
   '(case insensitive, overridden by -regex)')
 
+  parser.add_argument('-j', type=int, default=0,
+  help='number of tidy instances to be run in parallel.')
+  parser.add_argument('-timeout', type=int, default=None,
+  help='timeout per each file in seconds.')
   parser.add_argument('-fix', action='store_true', default=False,
   help='apply suggested fixes')
   parser.add_argument('-checks',
@@ -84,7 +135,7 @@
 match = re.search('^\+\+\+\ \"?(.*?/){%s}([^ \t\n\"]*)' % args.p, line)
 if match:
   filename = match.group(2)
-if filename == None:
+if filename is None:
   continue
 
 if args.regex is not None:
@@ -102,44 +153,64 @@
 line_count = int(match.group(3))
   if line_count == 0:
 continue
-  end_line = start_line + line_count - 1;
+  end_line = start_line + line_count - 1
   lines_by_file.setdefault(filename, []).append([start_line, end_line])
 
-  if len(lines_by_file) == 0:
+  if not any(lines_by_file):
 print("No relevant changes found.")
 sys.exit(0)
 
-  line_filter_json = json.dumps(
-[{"name" : name, "lines" : lines_by_file[name]} for name in lines_by_file],
-separators = (',', ':'))
+  max_task = args.j
+  if max_task == 0:
+  max_task = multiprocessing.cpu_count()
+  max_task = min(len(lines_by_file), max_task)
 
-  quote = "";
-  if sys.platform == 'win32':
-line_filter_json=re.sub(r'"', r'"""', line_filter_json)
-  else:
-quote = "'";
+  # Tasks for clang-tidy.
+  task_queue = queue.Queue(max_task)
+  # A lock for console output.
+  lock = threading.Lock()
 
-  # Run clang-tidy on files containing changes.
-  command = [args.clang_tidy_binary]
-  command.append('-line-filter=' + quote + line_filter_json + quote)
+  # Run a pool of clang-tidy workers.
+  run_workers(max_task, run_tidy, task_queue, lock, args.timeout)
+
+  quote = ""
+  if sys.platform != 'win32':
+quote = "'"
+
+  # Form the common args list.
+  common_clang_tidy_args = []
   if args.fix:
-command.append('-fix')
+common_clang_tidy_args.append('-fix')
   if args.export_fixes:
-command.append('-export-fixes=' + args.export_fixes)
+common_clang_tidy_args.append('-export-fixes=' + args.export_fixes)
   if args.checks != '':
-command.append('-checks=' + quote + args.checks + quote)
+common_clang_tidy_args.append('-checks=' + quote + args.checks + quote)
   if args.quiet:
-command.append('-quiet')
+common_clang_tidy_args.append('-quiet')
   if args.build_path is not None:
-command.append('-p=%s' % args.build_path)
-  command.extend(lines_by_file.keys())
+common_clang_tidy_args.append('-p=%s' % args.build_path)
   for arg in args.extra_arg:
-  command.append('-extra-arg=%s' % arg)
+common_clang_tidy_args.append('-extra-arg=%s' % arg)
   for arg in args.extra_arg_before:
-  command.append('-extra-arg-before=%s' % arg)
-  command.extend(clang_tidy_args)
+

[PATCH] D57662: [clang-tidy] Parallelise clang-tidy-diff.py

2019-02-04 Thread Zinovy Nis via Phabricator via cfe-commits
zinovy.nis updated this revision to Diff 185085.
zinovy.nis added a comment.

Fixed minor typos.


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

https://reviews.llvm.org/D57662

Files:
  clang-tidy/tool/clang-tidy-diff.py

Index: clang-tidy/tool/clang-tidy-diff.py
===
--- clang-tidy/tool/clang-tidy-diff.py
+++ clang-tidy/tool/clang-tidy-diff.py
@@ -25,9 +25,56 @@
 
 import argparse
 import json
+import multiprocessing
+import os
 import re
 import subprocess
 import sys
+import threading
+
+is_py2 = sys.version[0] == '2'
+
+if is_py2:
+import Queue as queue
+else:
+import queue as queue
+
+def run_tidy(task_queue, lock, timeout):
+  watchdog = None
+  while True:
+command = task_queue.get()
+try:
+  proc = subprocess.Popen(command,
+  stdout=subprocess.PIPE,
+  stderr=subprocess.PIPE)
+
+  if not timeout is None:
+watchdog = threading.Timer(timeout, proc.kill)
+watchdog.start()
+
+  stdout, stderr = proc.communicate()
+
+  with lock:
+sys.stdout.write(' '.join(command) + '\n' + stdout.decode('utf-8') + '\n')
+if stderr:
+  sys.stderr.write(stderr.decode('utf-8') + '\n')
+except:
+  with lock:
+sys.stderr.write('Failed: ' + str(sys.exc_info()[0]) + ' '.join(command) + '\n')
+finally:
+  with lock:
+if (not timeout is None) and (not watchdog is None):
+  if not watchdog.is_alive():
+  sys.stderr.write('Terminated by timeout: ' + ' '.join(command) + '\n')
+  watchdog.cancel()
+  task_queue.task_done()
+
+
+def run_workers(max_tasks, tidy_caller, task_queue, lock, timeout):
+  for _ in range(max_tasks):
+t = threading.Thread(target=tidy_caller, args=(task_queue, lock, timeout))
+t.daemon = True
+t.start()
 
 
 def main():
@@ -48,6 +95,10 @@
   help='custom pattern selecting file paths to check '
   '(case insensitive, overridden by -regex)')
 
+  parser.add_argument('-j', type=int, default=0,
+  help='number of tidy instances to be run in parallel.')
+  parser.add_argument('-timeout', type=int, default=None,
+  help='timeout per each file in seconds.')
   parser.add_argument('-fix', action='store_true', default=False,
   help='apply suggested fixes')
   parser.add_argument('-checks',
@@ -84,7 +135,7 @@
 match = re.search('^\+\+\+\ \"?(.*?/){%s}([^ \t\n\"]*)' % args.p, line)
 if match:
   filename = match.group(2)
-if filename == None:
+if filename is None:
   continue
 
 if args.regex is not None:
@@ -102,44 +153,65 @@
 line_count = int(match.group(3))
   if line_count == 0:
 continue
-  end_line = start_line + line_count - 1;
+  end_line = start_line + line_count - 1
   lines_by_file.setdefault(filename, []).append([start_line, end_line])
 
-  if len(lines_by_file) == 0:
+  if not any(lines_by_file):
 print("No relevant changes found.")
 sys.exit(0)
 
-  line_filter_json = json.dumps(
-[{"name" : name, "lines" : lines_by_file[name]} for name in lines_by_file],
-separators = (',', ':'))
+  max_task = args.j
+  if max_task == 0:
+  max_task = multiprocessing.cpu_count()
+  max_task = min(len(lines_by_file), max_task)
 
-  quote = "";
-  if sys.platform == 'win32':
-line_filter_json=re.sub(r'"', r'"""', line_filter_json)
-  else:
-quote = "'";
+  # Tasks for clang-tidy.
+  task_queue = queue.Queue(max_task)
+  # A lock for console output.
+  lock = threading.Lock()
 
-  # Run clang-tidy on files containing changes.
-  command = [args.clang_tidy_binary]
-  command.append('-line-filter=' + quote + line_filter_json + quote)
+  # Run a pool of clang-tidy workers.
+  run_workers(max_task, run_tidy, task_queue, lock, args.timeout)
+
+  # Run a watchdog.
+  quote = ""
+  if sys.platform != 'win32':
+quote = "'"
+
+  # Form the common args list.
+  common_clang_tidy_args = []
   if args.fix:
-command.append('-fix')
+common_clang_tidy_args.append('-fix')
   if args.export_fixes:
-command.append('-export-fixes=' + args.export_fixes)
+common_clang_tidy_args.append('-export-fixes=' + args.export_fixes)
   if args.checks != '':
-command.append('-checks=' + quote + args.checks + quote)
+common_clang_tidy_args.append('-checks=' + quote + args.checks + quote)
   if args.quiet:
-command.append('-quiet')
+common_clang_tidy_args.append('-quiet')
   if args.build_path is not None:
-command.append('-p=%s' % args.build_path)
-  command.extend(lines_by_file.keys())
+common_clang_tidy_args.append('-p=%s' % args.build_path)
   for arg in args.extra_arg:
-  command.append('-extra-arg=%s' % arg)
+common_clang_tidy_args.append('-extra-arg=%s' % arg)
   for arg in args.extra_arg_before:
-  

[PATCH] D57464: Generalize method overloading on addr spaces to C++

2019-02-04 Thread Anastasia Stulova via Phabricator via cfe-commits
Anastasia marked an inline comment as done.
Anastasia added inline comments.



Comment at: lib/Parse/ParseDeclCXX.cpp:2313
+}
+  }
 

ebevhan wrote:
> Anastasia wrote:
> > ebevhan wrote:
> > > Anastasia wrote:
> > > > Anastasia wrote:
> > > > > ebevhan wrote:
> > > > > > Is there a reason that the attributes are parsed here and not in 
> > > > > > `ParseFunctionDeclarator` like the rest of the ref-qualifiers (and 
> > > > > > the OpenCL++ ASes)?
> > > > > > 
> > > > > > That is possibly why the parser is getting confused with the 
> > > > > > trailing return.
> > > > > Good question! I have a feeling that this was done to unify parsing 
> > > > > of all CXX members, not just methods. For collecting the method 
> > > > > qualifiers it would certainly help making flow more coherent if this 
> > > > > was moved to `ParseFunctionDeclarator`. I will try to experiment with 
> > > > > this a bit more. What I found strange that the attributes here are 
> > > > > attached to the function type itself instead of its  qualifiers. May 
> > > > > be @rjmccall can shed some more light on the overall flow...
> > > > I looked at this a bit more and it seems that all the GNU attributes 
> > > > other than addr spaces belong to the function (not to be applied to 
> > > > `this`) for example 
> > > > https://blog.timac.org/2016/0716-constructor-and-destructor-attributes/.
> > > >  It seems correct to parse them here and apply to the function type. 
> > > > Although we could separate parsing of addr space attribute only and 
> > > > move into `ParseFunctionDeclarator`.  However this will only be needed 
> > > > for the function type not for the data members. So not sure whether it 
> > > > will make the flow any cleaner.
> > > > I looked at this a bit more and it seems that all the GNU attributes 
> > > > other than addr spaces belong to the function (not to be applied to 
> > > > this) 
> > > 
> > > Hm, I know what you mean. It's why Clang usually complains when you try 
> > > placing an AS attribute after a function declarator, because it's trying 
> > > to apply it to the function rather than the method qualifiers.
> > > 
> > > > However this will only be needed for the function type not for the data 
> > > > members. 
> > > 
> > > But we aren't really interested in data members, are we? Like struct 
> > > fields, non-static data members cannot be qualified with ASes (they 
> > > should 'inherit' the AS from the object type), and static ones should 
> > > probably just accept ASes as part of the member type itself.
> > > 
> > > These patches are to enable AS-qualifiers on methods, so it only stands 
> > > to reason that those ASes would be parsed along with the other method 
> > > qualifiers.
> > > 
> > > ParseFunctionDeclarator calls ParseTypeQualifierListOpt to get the 
> > > cv-qualifier-seq for the method qualifiers. Currently it's set to 
> > > `AR_NoAttributesParsed`. If we enable parsing attributes there, then the 
> > > qualifier parsing might 'eat' attributes that were possibly meant for the 
> > > function.
> > > 
> > > This is just a guess, but what I think you could do is include attributes 
> > > in the cv-qualifier parsing with `AR_GNUAttributesParsed`, look for any 
> > > AS attributes, take their AS and mark them invalid, then move the 
> > > attributes (somehow) from the qualifier-DeclSpec to the `FnAttrs` 
> > > function attribute list.
> > > 
> > > (The reason I'm concerned about where/how the qualifiers are parsed is 
> > > because this approach only works for custom ASes applied with the GNU 
> > > attribute directly. It won't work if custom address spaces are given with 
> > > a custom keyword qualifier, like in OpenCL. If the ASes are parsed into 
> > > the DeclSpec used for the other ref-qualifiers, then both of these cases 
> > > should 'just work'.)
> > > But we aren't really interested in data members, are we? Like struct 
> > > fields, non-static data members cannot be qualified with ASes (they 
> > > should 'inherit' the AS from the object type), and static ones should 
> > > probably just accept ASes as part of the member type itself.
> >  
> > Pointer data members can be qualified by and address space, but this should 
> > be parsed before.
> > 
> > 
> > > This is just a guess, but what I think you could do is include attributes 
> > > in the cv-qualifier parsing with AR_GNUAttributesParsed, look for any AS 
> > > attributes, take their AS and mark them invalid, then move the attributes 
> > > (somehow) from the qualifier-DeclSpec to the FnAttrs function attribute 
> > > list.
> > 
> > Ok, I will try that. Thanks for sharing your ideas!
> > 
> > Pointer data members can be qualified by and address space, but this should 
> > be parsed before.
> 
> Right, pointer-to-member is a thing. I always forget about those.
It seems unfortunately that moving parsing of GNU attributes into 
`ParseFunctionDeclarator` might not be a good viable alternative. One of the 

[PATCH] D57690: [OPENMP] issue error messages for multiple teams contructs in a target constructs

2019-02-04 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:7070
 const auto *OED = dyn_cast(*I);
-if (!OED || !isOpenMPTeamsDirective(OED->getDirectiveKind())) {
+if (!OED || !isOpenMPTeamsDirective(OED->getDirectiveKind()) ||
+(isOpenMPTeamsDirective(OED->getDirectiveKind()) &&

just `if (!OED || !isOpenMPTeamsDirective(OED->getDirectiveKind() || 
OMPTeamsFound)`


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

https://reviews.llvm.org/D57690



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


[PATCH] D57508: [clangd] Enable include insertion for dynamic index

2019-02-04 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL353054: [clangd] Enable include insertion for static index 
(authored by kadircet, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57508

Files:
  clang-tools-extra/trunk/clangd/ClangdServer.cpp
  clang-tools-extra/trunk/clangd/ClangdUnit.cpp
  clang-tools-extra/trunk/clangd/ClangdUnit.h
  clang-tools-extra/trunk/clangd/TUScheduler.cpp
  clang-tools-extra/trunk/clangd/TUScheduler.h
  clang-tools-extra/trunk/clangd/index/FileIndex.cpp
  clang-tools-extra/trunk/clangd/index/FileIndex.h
  clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
  clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
  clang-tools-extra/trunk/unittests/clangd/TestTU.cpp

Index: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
@@ -645,6 +645,36 @@
   EXPECT_THAT(Results, ElementsAre(Named("ifndef")));
 }
 
+TEST(CompletionTest, DynamicIndexIncludeInsertion) {
+  MockFSProvider FS;
+  MockCompilationDatabase CDB;
+  IgnoreDiagnostics DiagConsumer;
+  ClangdServer::Options Opts = ClangdServer::optsForTest();
+  Opts.BuildDynamicSymbolIndex = true;
+  ClangdServer Server(CDB, FS, DiagConsumer, Opts);
+
+  FS.Files[testPath("foo_header.h")] = R"cpp(
+struct Foo {
+   // Member doc
+   int foo();
+};
+  )cpp";
+  const std::string FileContent(R"cpp(
+#include "foo_header.h"
+int Foo::foo() {
+  return 42;
+}
+  )cpp");
+  Server.addDocument(testPath("foo_impl.cpp"), FileContent);
+  // Wait for the dynamic index being built.
+  ASSERT_TRUE(Server.blockUntilIdleForTest());
+  EXPECT_THAT(
+  completions(Server, "Foo^ foo;").Completions,
+  ElementsAre(AllOf(Named("Foo"),
+HasInclude('"' + testPath("foo_header.h") + '"'),
+InsertInclude(;
+}
+
 TEST(CompletionTest, DynamicIndexMultiFile) {
   MockFSProvider FS;
   MockCompilationDatabase CDB;
Index: clang-tools-extra/trunk/unittests/clangd/TestTU.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/TestTU.cpp
+++ clang-tools-extra/trunk/unittests/clangd/TestTU.cpp
@@ -59,13 +59,15 @@
 
 SymbolSlab TestTU::headerSymbols() const {
   auto AST = build();
-  return indexHeaderSymbols(AST.getASTContext(), AST.getPreprocessorPtr());
+  return indexHeaderSymbols(AST.getASTContext(), AST.getPreprocessorPtr(),
+AST.getCanonicalIncludes());
 }
 
 std::unique_ptr TestTU::index() const {
   auto AST = build();
   auto Idx = llvm::make_unique(/*UseDex=*/true);
-  Idx->updatePreamble(Filename, AST.getASTContext(), AST.getPreprocessorPtr());
+  Idx->updatePreamble(Filename, AST.getASTContext(), AST.getPreprocessorPtr(),
+  AST.getCanonicalIncludes());
   Idx->updateMain(Filename, AST);
   return std::move(Idx);
 }
Index: clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
@@ -12,6 +12,7 @@
 #include "SyncAPI.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "index/CanonicalIncludes.h"
 #include "index/FileIndex.h"
 #include "index/Index.h"
 #include "clang/Frontend/CompilerInvocation.h"
@@ -147,8 +148,8 @@
   File.HeaderFilename = (Basename + ".h").str();
   File.HeaderCode = Code;
   auto AST = File.build();
-  M.updatePreamble(File.Filename, AST.getASTContext(),
-   AST.getPreprocessorPtr());
+  M.updatePreamble(File.Filename, AST.getASTContext(), AST.getPreprocessorPtr(),
+   AST.getCanonicalIncludes());
 }
 
 TEST(FileIndexTest, CustomizedURIScheme) {
@@ -199,13 +200,16 @@
QName("X::f")));
 }
 
-TEST(FileIndexTest, NoIncludeCollected) {
+TEST(FileIndexTest, IncludeCollected) {
   FileIndex M;
-  update(M, "f", "class string {};");
+  update(
+  M, "f",
+  "// IWYU pragma: private, include \nclass string {};");
 
   auto Symbols = runFuzzyFind(M, "");
   EXPECT_THAT(Symbols, ElementsAre(_));
-  EXPECT_THAT(Symbols.begin()->IncludeHeaders, IsEmpty());
+  EXPECT_THAT(Symbols.begin()->IncludeHeaders.front().IncludeHeader,
+  "");
 }
 
 TEST(FileIndexTest, TemplateParamsInLabel) {
@@ -270,10 +274,11 @@
   buildPreamble(
   FooCpp, *CI, /*OldPreamble=*/nullptr, tooling::CompileCommand(), PI,
   std::make_shared(), /*StoreInMemory=*/true,
-  [&](ASTContext , std::shared_ptr PP) {
+  [&](ASTContext , std::shared_ptr PP,
+

[clang-tools-extra] r353054 - [clangd] Enable include insertion for static index

2019-02-04 Thread Kadir Cetinkaya via cfe-commits
Author: kadircet
Date: Mon Feb  4 08:19:57 2019
New Revision: 353054

URL: http://llvm.org/viewvc/llvm-project?rev=353054=rev
Log:
[clangd] Enable include insertion for static index

Summary:
This enables include insertion by adding canonical includes into
preambledata.

Reviewers: ioeric, ilya-biryukov

Subscribers: javed.absar, MaskRay, jkorous, arphaman, cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.h
clang-tools-extra/trunk/clangd/TUScheduler.cpp
clang-tools-extra/trunk/clangd/TUScheduler.h
clang-tools-extra/trunk/clangd/index/FileIndex.cpp
clang-tools-extra/trunk/clangd/index/FileIndex.h
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp
clang-tools-extra/trunk/unittests/clangd/TestTU.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=353054=353053=353054=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Mon Feb  4 08:19:57 2019
@@ -13,6 +13,7 @@
 #include "Headers.h"
 #include "SourceCode.h"
 #include "Trace.h"
+#include "index/CanonicalIncludes.h"
 #include "index/FileIndex.h"
 #include "index/Merge.h"
 #include "refactor/Tweak.h"
@@ -69,9 +70,10 @@ struct UpdateIndexCallbacks : public Par
   : FIndex(FIndex), DiagConsumer(DiagConsumer) {}
 
   void onPreambleAST(PathRef Path, ASTContext ,
- std::shared_ptr PP) override {
+ std::shared_ptr PP,
+ const CanonicalIncludes ) override {
 if (FIndex)
-  FIndex->updatePreamble(Path, Ctx, std::move(PP));
+  FIndex->updatePreamble(Path, Ctx, std::move(PP), CanonIncludes);
   }
 
   void onMainAST(PathRef Path, ParsedAST ) override {

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=353054=353053=353054=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Mon Feb  4 08:19:57 2019
@@ -16,6 +16,7 @@
 #include "Logger.h"
 #include "SourceCode.h"
 #include "Trace.h"
+#include "index/CanonicalIncludes.h"
 #include "index/Index.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/Basic/LangOptions.h"
@@ -99,11 +100,16 @@ public:
 
   IncludeStructure takeIncludes() { return std::move(Includes); }
 
+  CanonicalIncludes takeCanonicalIncludes() {
+addSystemHeadersMapping();
+return std::move(CanonIncludes);
+  }
+
   void AfterExecute(CompilerInstance ) override {
 if (!ParsedCallback)
   return;
 trace::Span Tracer("Running PreambleCallback");
-ParsedCallback(CI.getASTContext(), CI.getPreprocessorPtr());
+ParsedCallback(CI.getASTContext(), CI.getPreprocessorPtr(), CanonIncludes);
   }
 
   void BeforeExecute(CompilerInstance ) override {
@@ -115,10 +121,17 @@ public:
 return collectIncludeStructureCallback(*SourceMgr, );
   }
 
+  CommentHandler *getCommentHandler() override {
+IWYUHandler = collectIWYUHeaderMaps();
+return IWYUHandler.get();
+  }
+
 private:
   PathRef File;
   PreambleParsedCallback ParsedCallback;
   IncludeStructure Includes;
+  CanonicalIncludes CanonIncludes;
+  std::unique_ptr IWYUHandler = nullptr;
   SourceManager *SourceMgr = nullptr;
 };
 
@@ -324,6 +337,17 @@ ParsedAST::build(std::unique_ptrgetPreprocessor().addPPCallbacks(
   collectIncludeStructureCallback(Clang->getSourceManager(), ));
 
+  // Copy over the includes from the preamble, then combine with the
+  // non-preamble includes below.
+  CanonicalIncludes CanonIncludes;
+  if (Preamble)
+CanonIncludes = Preamble->CanonIncludes;
+  else
+addSystemHeadersMapping();
+  std::unique_ptr IWYUHandler =
+  collectIWYUHeaderMaps();
+  Clang->getPreprocessor().addCommentHandler(IWYUHandler.get());
+
   if (!Action->Execute())
 log("Execute() failed when building AST for {0}", MainInput.getFile());
 
@@ -353,7 +377,7 @@ ParsedAST::build(std::unique_ptrDiags.begin(), 
Preamble->Diags.end());
   return ParsedAST(std::move(Preamble), std::move(Clang), std::move(Action),
std::move(ParsedDecls), std::move(Diags),
-   std::move(Includes));
+   std::move(Includes), std::move(CanonIncludes));
 }
 
 ParsedAST::ParsedAST(ParsedAST &) = default;
@@ -429,21 +453,28 @@ const IncludeStructure ::getIn
   return Includes;
 }
 
+const CanonicalIncludes ::getCanonicalIncludes() const {
+  return CanonIncludes;
+}
+
 

[PATCH] D57353: [clang-tidy] Add the abseil-duration-unnecessary-conversion check

2019-02-04 Thread Hyrum Wright via Phabricator via cfe-commits
hwright added a comment.

@hokein Thanks for the suggestion on the name, I was looking for something a 
little less confusing.


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

https://reviews.llvm.org/D57353



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


[PATCH] D57353: [clang-tidy] Add the abseil-duration-unnecessary-conversion check

2019-02-04 Thread Hyrum Wright via Phabricator via cfe-commits
hwright updated this revision to Diff 185057.
hwright marked 4 inline comments as done.
hwright retitled this revision from "[clang-tidy] Add the 
abseil-duration-double-conversion check" to "[clang-tidy] Add the 
abseil-duration-unnecessary-conversion check".
hwright added a comment.

Renamed to `abseil-duration-unnecessary-conversion`


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

https://reviews.llvm.org/D57353

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/DurationUnnecessaryConversionCheck.cpp
  clang-tidy/abseil/DurationUnnecessaryConversionCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/Inputs/absl/time/time.h
  test/clang-tidy/abseil-duration-unnecessary-conversion.cpp

Index: test/clang-tidy/abseil-duration-unnecessary-conversion.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-duration-unnecessary-conversion.cpp
@@ -0,0 +1,69 @@
+// RUN: %check_clang_tidy %s abseil-duration-unnecessary-conversion %t -- -- -I%S/Inputs
+
+#include "absl/time/time.h"
+
+void f() {
+  absl::Duration d1, d2;
+
+  // Floating point
+  d2 = absl::Hours(absl::ToDoubleHours(d1));
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-unnecessary-conversion]
+  // CHECK-FIXES: d1
+  d2 = absl::Minutes(absl::ToDoubleMinutes(d1));
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-unnecessary-conversion]
+  // CHECK-FIXES: d1
+  d2 = absl::Seconds(absl::ToDoubleSeconds(d1));
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-unnecessary-conversion]
+  // CHECK-FIXES: d1
+  d2 = absl::Milliseconds(absl::ToDoubleMilliseconds(d1));
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-unnecessary-conversion]
+  // CHECK-FIXES: d1
+  d2 = absl::Microseconds(absl::ToDoubleMicroseconds(d1));
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-unnecessary-conversion]
+  // CHECK-FIXES: d1
+  d2 = absl::Nanoseconds(absl::ToDoubleNanoseconds(d1));
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-unnecessary-conversion]
+  // CHECK-FIXES: d1
+
+  // Integer point
+  d2 = absl::Hours(absl::ToInt64Hours(d1));
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-unnecessary-conversion]
+  // CHECK-FIXES: d1
+  d2 = absl::Minutes(absl::ToInt64Minutes(d1));
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-unnecessary-conversion]
+  // CHECK-FIXES: d1
+  d2 = absl::Seconds(absl::ToInt64Seconds(d1));
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-unnecessary-conversion]
+  // CHECK-FIXES: d1
+  d2 = absl::Milliseconds(absl::ToInt64Milliseconds(d1));
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-unnecessary-conversion]
+  // CHECK-FIXES: d1
+  d2 = absl::Microseconds(absl::ToInt64Microseconds(d1));
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-unnecessary-conversion]
+  // CHECK-FIXES: d1
+  d2 = absl::Nanoseconds(absl::ToInt64Nanoseconds(d1));
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: remove double conversion of absl::Duration [abseil-duration-unnecessary-conversion]
+  // CHECK-FIXES: d1
+
+  // As macro argument
+#define PLUS_FIVE_S(x) x + absl::Seconds(5)
+  d2 = PLUS_FIVE_S(absl::Seconds(absl::ToInt64Seconds(d1)));
+  // CHECK-MESSAGES: [[@LINE-1]]:20: warning: remove double conversion of absl::Duration [abseil-duration-unnecessary-conversion]
+  // CHECK-FIXES: PLUS_FIVE_S(d1)
+#undef PLUS_FIVE_S
+
+  // Split by macro: should not change
+#define TOSECONDS(x) absl::Seconds(x)
+  d2 = TOSECONDS(absl::ToInt64Seconds(d1));
+#undef TOSECONDS
+
+  // Don't change something inside a macro definition
+#define VALUE(x) absl::Hours(absl::ToInt64Hours(x));
+  d2 = VALUE(d1);
+#undef VALUE
+
+  // These should not match
+  d2 = absl::Seconds(absl::ToDoubleMilliseconds(d1));
+  d2 = absl::Seconds(4);
+  int i = absl::ToInt64Milliseconds(d1);
+}
Index: test/clang-tidy/Inputs/absl/time/time.h
===
--- test/clang-tidy/Inputs/absl/time/time.h
+++ test/clang-tidy/Inputs/absl/time/time.h
@@ -14,6 +14,8 @@
   Duration /=(float r);
   Duration /=(double r);
   template  Duration /=(T r);
+
+  Duration +(Duration d);
 };
 
 template  Duration operator*(Duration lhs, T rhs);
Index: docs/clang-tidy/checks/list.rst

[PATCH] D57690: [OPENMP] issue error messages for multiple teams contructs in a target constructs

2019-02-04 Thread Kelvin Li via Phabricator via cfe-commits
kkwli0 created this revision.
kkwli0 added a reviewer: ABataev.
Herald added a subscriber: guansong.

The compiler does not generate any error messages if there are more than one 
teams construct inside a target constructs.

  #pragma omp target
  {
#pragma omp teams
{  ...  }
  
#pragma omp teams
{ ... }
  }

After the fix, the error messages are issued.

  teams.c:4:9: error: target construct with nested teams region contains 
statements
outside of the teams construct
  #pragma omp target
  ^
  teams.c:14:11: note: nested teams construct here
#pragma omp teams
^
  teams.c:9:11: note: directive outside teams construct here
#pragma omp teams
^
  1 error generated.


https://reviews.llvm.org/D57690

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/nesting_of_regions.cpp


Index: clang/test/OpenMP/nesting_of_regions.cpp
===
--- clang/test/OpenMP/nesting_of_regions.cpp
+++ clang/test/OpenMP/nesting_of_regions.cpp
@@ -4080,6 +4080,13 @@
   }
 #pragma omp target // expected-error {{target construct with nested teams 
region contains statements outside of the teams construct}}
   {
+#pragma omp teams // expected-note {{directive outside teams construct here}}
+++a;
+#pragma omp teams // expected-note {{nested teams construct here}}
+++a;
+  }
+#pragma omp target // expected-error {{target construct with nested teams 
region contains statements outside of the teams construct}}
+  {
 ++a;   // expected-note {{statement outside teams construct here}}
 #pragma omp teams  // expected-note {{nested teams construct here}}
 ++a;
@@ -12693,6 +12700,13 @@
   }
 #pragma omp target // expected-error {{target construct with nested teams 
region contains statements outside of the teams construct}}
   {
+#pragma omp teams // expected-note {{directive outside teams construct here}}
+++a;
+#pragma omp teams // expected-note {{nested teams construct here}}
+++a;
+  }
+#pragma omp target // expected-error {{target construct with nested teams 
region contains statements outside of the teams construct}}
+  {
 ++a;  // expected-note {{statement outside teams construct here}}
 #pragma omp teams // expected-note {{nested teams construct here}}
 ++a;
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -7067,7 +7067,10 @@
   auto I = CS->body_begin();
   while (I != CS->body_end()) {
 const auto *OED = dyn_cast(*I);
-if (!OED || !isOpenMPTeamsDirective(OED->getDirectiveKind())) {
+if (!OED || !isOpenMPTeamsDirective(OED->getDirectiveKind()) ||
+(isOpenMPTeamsDirective(OED->getDirectiveKind()) &&
+ OMPTeamsFound)) {
+
   OMPTeamsFound = false;
   break;
 }


Index: clang/test/OpenMP/nesting_of_regions.cpp
===
--- clang/test/OpenMP/nesting_of_regions.cpp
+++ clang/test/OpenMP/nesting_of_regions.cpp
@@ -4080,6 +4080,13 @@
   }
 #pragma omp target // expected-error {{target construct with nested teams region contains statements outside of the teams construct}}
   {
+#pragma omp teams // expected-note {{directive outside teams construct here}}
+++a;
+#pragma omp teams // expected-note {{nested teams construct here}}
+++a;
+  }
+#pragma omp target // expected-error {{target construct with nested teams region contains statements outside of the teams construct}}
+  {
 ++a;   // expected-note {{statement outside teams construct here}}
 #pragma omp teams  // expected-note {{nested teams construct here}}
 ++a;
@@ -12693,6 +12700,13 @@
   }
 #pragma omp target // expected-error {{target construct with nested teams region contains statements outside of the teams construct}}
   {
+#pragma omp teams // expected-note {{directive outside teams construct here}}
+++a;
+#pragma omp teams // expected-note {{nested teams construct here}}
+++a;
+  }
+#pragma omp target // expected-error {{target construct with nested teams region contains statements outside of the teams construct}}
+  {
 ++a;  // expected-note {{statement outside teams construct here}}
 #pragma omp teams // expected-note {{nested teams construct here}}
 ++a;
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -7067,7 +7067,10 @@
   auto I = CS->body_begin();
   while (I != CS->body_end()) {
 const auto *OED = dyn_cast(*I);
-if (!OED || !isOpenMPTeamsDirective(OED->getDirectiveKind())) {
+if (!OED || !isOpenMPTeamsDirective(OED->getDirectiveKind()) ||
+(isOpenMPTeamsDirective(OED->getDirectiveKind()) &&
+ OMPTeamsFound)) {
+

[PATCH] D57508: [clangd] Enable include insertion for dynamic index

2019-02-04 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 185052.
kadircet added a comment.

- Delete unnecessary include


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57508

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/TUScheduler.cpp
  clangd/TUScheduler.h
  clangd/index/FileIndex.cpp
  clangd/index/FileIndex.h
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/FileIndexTests.cpp
  unittests/clangd/TestTU.cpp

Index: unittests/clangd/TestTU.cpp
===
--- unittests/clangd/TestTU.cpp
+++ unittests/clangd/TestTU.cpp
@@ -59,13 +59,15 @@
 
 SymbolSlab TestTU::headerSymbols() const {
   auto AST = build();
-  return indexHeaderSymbols(AST.getASTContext(), AST.getPreprocessorPtr());
+  return indexHeaderSymbols(AST.getASTContext(), AST.getPreprocessorPtr(),
+AST.getCanonicalIncludes());
 }
 
 std::unique_ptr TestTU::index() const {
   auto AST = build();
   auto Idx = llvm::make_unique(/*UseDex=*/true);
-  Idx->updatePreamble(Filename, AST.getASTContext(), AST.getPreprocessorPtr());
+  Idx->updatePreamble(Filename, AST.getASTContext(), AST.getPreprocessorPtr(),
+  AST.getCanonicalIncludes());
   Idx->updateMain(Filename, AST);
   return std::move(Idx);
 }
Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -12,6 +12,7 @@
 #include "SyncAPI.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "index/CanonicalIncludes.h"
 #include "index/FileIndex.h"
 #include "index/Index.h"
 #include "clang/Frontend/CompilerInvocation.h"
@@ -147,8 +148,8 @@
   File.HeaderFilename = (Basename + ".h").str();
   File.HeaderCode = Code;
   auto AST = File.build();
-  M.updatePreamble(File.Filename, AST.getASTContext(),
-   AST.getPreprocessorPtr());
+  M.updatePreamble(File.Filename, AST.getASTContext(), AST.getPreprocessorPtr(),
+   AST.getCanonicalIncludes());
 }
 
 TEST(FileIndexTest, CustomizedURIScheme) {
@@ -199,13 +200,16 @@
QName("X::f")));
 }
 
-TEST(FileIndexTest, NoIncludeCollected) {
+TEST(FileIndexTest, IncludeCollected) {
   FileIndex M;
-  update(M, "f", "class string {};");
+  update(
+  M, "f",
+  "// IWYU pragma: private, include \nclass string {};");
 
   auto Symbols = runFuzzyFind(M, "");
   EXPECT_THAT(Symbols, ElementsAre(_));
-  EXPECT_THAT(Symbols.begin()->IncludeHeaders, IsEmpty());
+  EXPECT_THAT(Symbols.begin()->IncludeHeaders.front().IncludeHeader,
+  "");
 }
 
 TEST(FileIndexTest, TemplateParamsInLabel) {
@@ -270,10 +274,11 @@
   buildPreamble(
   FooCpp, *CI, /*OldPreamble=*/nullptr, tooling::CompileCommand(), PI,
   std::make_shared(), /*StoreInMemory=*/true,
-  [&](ASTContext , std::shared_ptr PP) {
+  [&](ASTContext , std::shared_ptr PP,
+  const CanonicalIncludes ) {
 EXPECT_FALSE(IndexUpdated) << "Expected only a single index update";
 IndexUpdated = true;
-Index.updatePreamble(FooCpp, Ctx, std::move(PP));
+Index.updatePreamble(FooCpp, Ctx, std::move(PP), CanonIncludes);
   });
   ASSERT_TRUE(IndexUpdated);
 
@@ -358,7 +363,8 @@
   MainFile, *buildCompilerInvocation(PI), /*OldPreamble=*/nullptr,
   tooling::CompileCommand(), PI, std::make_shared(),
   /*StoreInMemory=*/true,
-  [&](ASTContext , std::shared_ptr PP) {});
+  [&](ASTContext , std::shared_ptr PP,
+  const CanonicalIncludes &) {});
   // Build AST for main file with preamble.
   auto AST =
   ParsedAST::build(createInvocationFromCommandLine(Cmd), PreambleData,
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -645,6 +645,36 @@
   EXPECT_THAT(Results, ElementsAre(Named("ifndef")));
 }
 
+TEST(CompletionTest, DynamicIndexIncludeInsertion) {
+  MockFSProvider FS;
+  MockCompilationDatabase CDB;
+  IgnoreDiagnostics DiagConsumer;
+  ClangdServer::Options Opts = ClangdServer::optsForTest();
+  Opts.BuildDynamicSymbolIndex = true;
+  ClangdServer Server(CDB, FS, DiagConsumer, Opts);
+
+  FS.Files[testPath("foo_header.h")] = R"cpp(
+struct Foo {
+   // Member doc
+   int foo();
+};
+  )cpp";
+  const std::string FileContent(R"cpp(
+#include "foo_header.h"
+int Foo::foo() {
+  return 42;
+}
+  )cpp");
+  Server.addDocument(testPath("foo_impl.cpp"), FileContent);
+  // Wait for the dynamic index being built.
+  ASSERT_TRUE(Server.blockUntilIdleForTest());
+  EXPECT_THAT(
+  completions(Server, "Foo^ foo;").Completions,
+  ElementsAre(AllOf(Named("Foo"),
+HasInclude('"' + 

[PATCH] D57508: [clangd] Enable include insertion for dynamic index

2019-02-04 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet marked an inline comment as done.
kadircet added a comment.

I don't think so, since the only reason this already wasn't enabled was because 
there was a fixme ? Also we want people to use -background-index by default in 
future so I think it should be OK


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57508



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


[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

2019-02-04 Thread Ronald Wampler via Phabricator via cfe-commits
rdwampler created this revision.
rdwampler added reviewers: djasper, klimek.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This option `AllowShortLambdasOnASingleLine` similar to the other `AllowShort*` 
options, but applied to C++ lambdas.

I considered making this dependent on `AllowShortFunctionsOnASingleLine`, but 
could not think of how the breaking should be behave when the option is set to 
`Inline` and `InlineOnly`.

I could not find a specific coding style that requires short lambdas to 
//always// break on a new line, but I do think it's fairly common practice. 
Also, I note that the patch: https://reviews.llvm.org/D44609 attempts to add an 
option to allow formatting lambda using the Allman style braces. Here is a 
coding style that requires Allman style braces, but allows lambdas to be on 
single line if they are short. So, maybe this option would be required to 
handle this situation?

Addresses: https://bugs.llvm.org//show_bug.cgi?id=32151


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D57687

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11954,6 +11954,30 @@
   "> {\n"
   "  //\n"
   "});");
+
+  FormatStyle DoNotMerge = getLLVMStyle();
+  DoNotMerge.AllowShortLambdasOnASingleLine = FormatStyle::SLS_None;
+  verifyFormat("auto c = []() {\n"
+   "  return b;\n"
+   "};",
+   "auto c = []() { return b; };", DoNotMerge);
+  verifyFormat("auto c = []() {\n"
+   "};",
+   " auto c = []() {};", DoNotMerge);
+
+  FormatStyle MergeEmptyOnly = getLLVMStyle();
+  MergeEmptyOnly.AllowShortLambdasOnASingleLine = FormatStyle::SLS_Empty;
+  verifyFormat("auto c = []() {\n"
+   "  return b;\n"
+   "};",
+   "auto c = []() {\n"
+   " return b;\n"
+   " };",
+   MergeEmptyOnly);
+  verifyFormat("auto c = []() {};",
+   "auto c = []() {\n"
+   "};",
+   MergeEmptyOnly);
 }
 
 TEST_F(FormatTest, EmptyLinesInLambdas) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1433,6 +1433,7 @@
   return true;
 }
   }
+  FormatTok->Type = TT_LambdaLBrace;
   LSquare.Type = TT_LambdaLSquare;
   parseChildBlock();
   return true;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -365,7 +365,7 @@
   // specifier parameter, although this is technically valid:
   // [[foo(:)]]
   if (AttrTok->is(tok::colon) ||
-  AttrTok->startsSequence(tok::identifier, tok::identifier) || 
+  AttrTok->startsSequence(tok::identifier, tok::identifier) ||
   AttrTok->startsSequence(tok::r_paren, tok::identifier))
 return false;
   if (AttrTok->is(tok::ellipsis))
@@ -1138,11 +1138,11 @@
 
 // Reset token type in case we have already looked at it and then
 // recovered from an error (e.g. failure to find the matching >).
-if (!CurrentToken->isOneOf(TT_LambdaLSquare, TT_ForEachMacro,
-   TT_FunctionLBrace, TT_ImplicitStringLiteral,
-   TT_InlineASMBrace, TT_JsFatArrow, TT_LambdaArrow,
-   TT_OverloadedOperator, TT_RegexLiteral,
-   TT_TemplateString, TT_ObjCStringLiteral))
+if (!CurrentToken->isOneOf(
+TT_LambdaLSquare, TT_LambdaLBrace, TT_ForEachMacro,
+TT_FunctionLBrace, TT_ImplicitStringLiteral, TT_InlineASMBrace,
+TT_JsFatArrow, TT_LambdaArrow, TT_OverloadedOperator,
+TT_RegexLiteral, TT_TemplateString, TT_ObjCStringLiteral))
   CurrentToken->Type = TT_Unknown;
 CurrentToken->Role.reset();
 CurrentToken->MatchingParen = nullptr;
@@ -2835,7 +2835,7 @@
 // Returns 'true' if 'Tok' is a brace we'd want to break before in Allman style.
 static bool isAllmanBrace(const FormatToken ) {
   return Tok.is(tok::l_brace) && Tok.BlockKind == BK_Block &&
- !Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral);
+ !Tok.isOneOf(TT_ObjCBlockLBrace, TT_LambdaLBrace, TT_DictLiteral);
 }
 
 bool TokenAnnotator::mustBreakBefore(const AnnotatedLine ,
@@ -2962,6 +2962,10 @@

[PATCH] D57659: [Sema] SequenceChecker: Add some comments + related small NFCs in preparation of the following patches

2019-02-04 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno marked an inline comment as done.
riccibruno added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:11748
+/// The expressions corresponding to each usage kind.
+Expr *UsageExprs[UK_Count]{};
+

aaron.ballman wrote:
> You can drop the `{}` here and below.
I don't think I can (although I can drop it for the array of 
`SequenceTree::Seq`). I want to value-initialize each expression in this array 
so that they are all null.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57659



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


[PATCH] D57508: [clangd] Enable include insertion for dynamic index

2019-02-04 Thread Eric Liu via Phabricator via cfe-commits
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.

lgtm.

Note that this will enable include insertions for everyone (even without index 
switched on). Not sure if we should provide options.




Comment at: clangd/ClangdServer.cpp:16
 #include "Trace.h"
+#include "XRefs.h"
+#include "index/CanonicalIncludes.h"

nit: is this used?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57508



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


[PATCH] D57660: [Sema] SequenceChecker: Handle references and members

2019-02-04 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno updated this revision to Diff 185048.
riccibruno added a comment.

Added tests for nested/anonymous unions and local structs.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57660

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/SemaCXX/warn-unsequenced.cpp

Index: test/SemaCXX/warn-unsequenced.cpp
===
--- test/SemaCXX/warn-unsequenced.cpp
+++ test/SemaCXX/warn-unsequenced.cpp
@@ -131,54 +131,53 @@
 
   // Operations with a single member of the implicit object.
   ++a = 0; // no-warning
-  a + ++a; // expected-warning {{unsequenced modification and access to 'a'}}
+  a + ++a; // expected-warning {{unsequenced modification and access to member 'a'}}
   a = ++a; // no-warning
-  a + a++; // expected-warning {{unsequenced modification and access to 'a'}}
-  a = a++; // expected-warning {{multiple unsequenced modifications to 'a'}}
+  a + a++; // expected-warning {{unsequenced modification and access to member 'a'}}
+  a = a++; // expected-warning {{multiple unsequenced modifications to member 'a'}}
   ++ ++a; // no-warning
   (a++, a++); // no-warning
-  ++a + ++a; // expected-warning {{multiple unsequenced modifications to 'a'}}
-  a++ + a++; // expected-warning {{multiple unsequenced modifications to 'a'}}
+  ++a + ++a; // expected-warning {{multiple unsequenced modifications to member 'a'}}
+  a++ + a++; // expected-warning {{multiple unsequenced modifications to member 'a'}}
   (a++, a) = 0; // no-warning
   a = xs[++a]; // no-warning
-  a = xs[a++]; // expected-warning {{multiple unsequenced modifications to 'a'}}
-  (a ? xs[0] : xs[1]) = ++a; // expected-warning {{unsequenced modification and access to 'a'}}
+  a = xs[a++]; // expected-warning {{multiple unsequenced modifications to member 'a'}}
+  (a ? xs[0] : xs[1]) = ++a; // expected-warning {{unsequenced modification and access to member 'a'}}
   a = (++a, ++a); // no-warning
   a = (a++, ++a); // no-warning
-  a = (a++, a++); // expected-warning {{multiple unsequenced modifications to 'a'}}
+  a = (a++, a++); // expected-warning {{multiple unsequenced modifications to member 'a'}}
   f(a, a); // no-warning
-  f(a = 0, a); // expected-warning {{unsequenced modification and access to 'a'}}
-  f(a, a += 0); // expected-warning {{unsequenced modification and access to 'a'}}
-  f(a = 0, a = 0); // expected-warning {{multiple unsequenced modifications to 'a'}}
+  f(a = 0, a); // expected-warning {{unsequenced modification and access to member 'a'}}
+  f(a, a += 0); // expected-warning {{unsequenced modification and access to member 'a'}}
+  f(a = 0, a = 0); // expected-warning {{multiple unsequenced modifications to member 'a'}}
   a = f(++a); // no-warning
   a = f(a++); // no-warning
-  a = f(++a, a++); // expected-warning {{multiple unsequenced modifications to 'a'}}
+  a = f(++a, a++); // expected-warning {{multiple unsequenced modifications to member 'a'}}
 
   // Operations with a single member of the other object 's'.
-  // TODO: For now this is completely unhandled.
   ++s.a = 0; // no-warning
-  s.a + ++s.a; // no-warning TODO {{unsequenced modification and access to }}
+  s.a + ++s.a; // expected-warning {{unsequenced modification and access to member 'a' of 's'}}
   s.a = ++s.a; // no-warning
-  s.a + s.a++; // no-warning TODO {{unsequenced modification and access to }}
-  s.a = s.a++; // no-warning TODO {{multiple unsequenced modifications to }}
+  s.a + s.a++; // expected-warning {{unsequenced modification and access to member 'a' of 's'}}
+  s.a = s.a++; // expected-warning {{multiple unsequenced modifications to member 'a' of 's'}}
   ++ ++s.a; // no-warning
   (s.a++, s.a++); // no-warning
-  ++s.a + ++s.a; // no-warning TODO {{multiple unsequenced modifications to }}
-  s.a++ + s.a++; // no-warning TODO {{multiple unsequenced modifications to }}
+  ++s.a + ++s.a; // expected-warning {{multiple unsequenced modifications to member 'a' of 's'}}
+  s.a++ + s.a++; // expected-warning {{multiple unsequenced modifications to member 'a' of 's'}}
   (s.a++, s.a) = 0; // no-warning
   s.a = xs[++s.a]; // no-warning
-  s.a = xs[s.a++]; // no-warning TODO {{multiple unsequenced modifications to }}
-  (s.a ? xs[0] : xs[1]) = ++s.a; // no-warning TODO {{unsequenced modification and access to }}
+  s.a = xs[s.a++]; // expected-warning {{multiple unsequenced modifications to member 'a' of 's'}}
+  (s.a ? xs[0] : xs[1]) = ++s.a; // expected-warning {{unsequenced modification and access to member 'a' of 's'}}
   s.a = (++s.a, ++s.a); // no-warning
   s.a = (s.a++, ++s.a); // no-warning
-  s.a = (s.a++, s.a++); // no-warning TODO {{multiple unsequenced modifications to }}
+  s.a = (s.a++, s.a++); // expected-warning {{multiple unsequenced modifications to member 'a' of 's'}}
   f(s.a, s.a); // no-warning
-  f(s.a = 0, s.a); // no-warning TODO {{unsequenced modification and 

  1   2   >