[clang-tools-extra] r363034 - [clangd] Fix gcc warning by removing extra "; "

[Mikael Holmen via cfe-commits]

Author: uabelho
Date: Mon Jun 10 23:02:01 2019
New Revision: 363034

URL: http://llvm.org/viewvc/llvm-project?rev=363034=rev
Log:
[clangd] Fix gcc warning by removing extra ";"

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

Modified: clang-tools-extra/trunk/clangd/Format.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Format.cpp?rev=363034=363033=363034=diff
==
--- clang-tools-extra/trunk/clangd/Format.cpp (original)
+++ clang-tools-extra/trunk/clangd/Format.cpp Mon Jun 10 23:02:01 2019
@@ -91,7 +91,7 @@ tooling::Replacement replacement(llvm::S
   // The filename is required but ignored.
   return tooling::Replacement(Filename, From.data() - Code.data(),
   From.size(), To);
-};
+}
 
 // High-level representation of incremental formatting changes.
 // The changes are made in two steps.


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

[PATCH] D62244: [AMDGPU] Enable the implicit arguments for HIP (CLANG)

[Christudasan Devadasan via Phabricator via cfe-commits]

cdevadas updated this revision to Diff 203975.
cdevadas added a comment.

simplified the check in the test case.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62244

Files:
  lib/CodeGen/TargetInfo.cpp
  test/CodeGenCUDA/amdgpu-hip-implicit-kernarg.cu


Index: test/CodeGenCUDA/amdgpu-hip-implicit-kernarg.cu
===
--- /dev/null
+++ test/CodeGenCUDA/amdgpu-hip-implicit-kernarg.cu
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -emit-llvm -x hip -o - %s | 
FileCheck %s
+#include "Inputs/cuda.h"
+
+__global__ void hip_kernel_temp() {
+}
+
+// CHECK: attributes #0 = { noinline nounwind optnone 
"amdgpu-implicitarg-num-bytes"="48"
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -7853,7 +7853,8 @@
   const auto *ReqdWGS = M.getLangOpts().OpenCL ?
 FD->getAttr() : nullptr;
 
-  if (M.getLangOpts().OpenCL && FD->hasAttr() &&
+  if (((M.getLangOpts().OpenCL && FD->hasAttr()) ||
+  (M.getLangOpts().HIP && FD->hasAttr())) &&
   (M.getTriple().getOS() == llvm::Triple::AMDHSA))
 F->addFnAttr("amdgpu-implicitarg-num-bytes", "48");
 


Index: test/CodeGenCUDA/amdgpu-hip-implicit-kernarg.cu
===
--- /dev/null
+++ test/CodeGenCUDA/amdgpu-hip-implicit-kernarg.cu
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -emit-llvm -x hip -o - %s | FileCheck %s
+#include "Inputs/cuda.h"
+
+__global__ void hip_kernel_temp() {
+}
+
+// CHECK: attributes #0 = { noinline nounwind optnone "amdgpu-implicitarg-num-bytes"="48"
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -7853,7 +7853,8 @@
   const auto *ReqdWGS = M.getLangOpts().OpenCL ?
 FD->getAttr() : nullptr;
 
-  if (M.getLangOpts().OpenCL && FD->hasAttr() &&
+  if (((M.getLangOpts().OpenCL && FD->hasAttr()) ||
+  (M.getLangOpts().HIP && FD->hasAttr())) &&
   (M.getTriple().getOS() == llvm::Triple::AMDHSA))
 F->addFnAttr("amdgpu-implicitarg-num-bytes", "48");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59919: [Attributor] Deduce "returned" argument attribute

[Johannes Doerfert via Phabricator via cfe-commits]

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

Update tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59919

Files:
  clang/test/CodeGenOpenCL/as_type.cl
  llvm/include/llvm/Transforms/IPO/Attributor.h
  llvm/lib/Transforms/IPO/Attributor.cpp
  llvm/test/Transforms/FunctionAttrs/arg_nocapture.ll
  llvm/test/Transforms/FunctionAttrs/arg_returned.ll
  llvm/test/Transforms/FunctionAttrs/read_write_returned_arguments_scc.ll

Index: llvm/test/Transforms/FunctionAttrs/read_write_returned_arguments_scc.ll
===
--- llvm/test/Transforms/FunctionAttrs/read_write_returned_arguments_scc.ll
+++ llvm/test/Transforms/FunctionAttrs/read_write_returned_arguments_scc.ll
@@ -31,7 +31,7 @@
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 
 ; CHECK: Function Attrs: nounwind
-; CHECK-NEXT: define i32* @external_ret2_nrw(i32* %n0, i32* %r0, i32* %w0)
+; CHECK-NEXT: define i32* @external_ret2_nrw(i32* %n0, i32* %r0, i32* returned %w0)
 define i32* @external_ret2_nrw(i32* %n0, i32* %r0, i32* %w0) {
 entry:
   %call = call i32* @internal_ret0_nw(i32* %n0, i32* %w0)
@@ -42,7 +42,7 @@
 }
 
 ; CHECK: Function Attrs: nounwind
-; CHECK-NEXT: define internal i32* @internal_ret0_nw(i32* %n0, i32* %w0)
+; CHECK-NEXT: define internal i32* @internal_ret0_nw(i32* returned %n0, i32* %w0)
 define internal i32* @internal_ret0_nw(i32* %n0, i32* %w0) {
 entry:
   %r0 = alloca i32, align 4
@@ -71,7 +71,7 @@
 }
 
 ; CHECK: Function Attrs: nounwind
-; CHECK-NEXT: define internal i32* @internal_ret1_rrw(i32* %r0, i32* %r1, i32* %w0)
+; CHECK-NEXT: define internal i32* @internal_ret1_rrw(i32* %r0, i32* returned %r1, i32* %w0)
 define internal i32* @internal_ret1_rrw(i32* %r0, i32* %r1, i32* %w0) {
 entry:
   %0 = load i32, i32* %r0, align 4
@@ -122,7 +122,7 @@
 }
 
 ; CHECK: Function Attrs: nounwind
-; CHECK-NEXT: define internal i32* @internal_ret1_rw(i32* %r0, i32* %w0)
+; CHECK-NEXT: define internal i32* @internal_ret1_rw(i32* %r0, i32* returned %w0)
 define internal i32* @internal_ret1_rw(i32* %r0, i32* %w0) {
 entry:
   %0 = load i32, i32* %r0, align 4
@@ -148,7 +148,7 @@
 }
 
 ; CHECK: Function Attrs: nounwind
-; CHECK-NEXT: define i32* @external_source_ret2_nrw(i32* %n0, i32* %r0, i32* %w0)
+; CHECK-NEXT: define i32* @external_source_ret2_nrw(i32* %n0, i32* %r0, i32* returned %w0)
 define i32* @external_source_ret2_nrw(i32* %n0, i32* %r0, i32* %w0) {
 entry:
   %call = call i32* @external_sink_ret2_nrw(i32* %n0, i32* %r0, i32* %w0)
Index: llvm/test/Transforms/FunctionAttrs/arg_returned.ll
===
--- llvm/test/Transforms/FunctionAttrs/arg_returned.ll
+++ llvm/test/Transforms/FunctionAttrs/arg_returned.ll
@@ -1,5 +1,6 @@
-; RUN: opt -functionattrs -attributor -attributor-disable=false -S < %s | FileCheck %s
-; RUN: opt -functionattrs -attributor -attributor-disable=false -attributor-verify=true -S < %s | FileCheck %s
+; RUN: opt -functionattrs -S < %s | FileCheck %s --check-prefix=FNATTR
+; RUN: opt -attributor -attributor-disable=false -S < %s | FileCheck %s --check-prefix=ATTRIBUTOR
+; RUN: opt -attributor -attributor-disable=false -functionattrs -S < %s | FileCheck %s --check-prefix=BOTH
 ;
 ; Test cases specifically designed for the "returned" argument attribute.
 ; We use FIXME's to indicate problems and missing attributes.
@@ -7,16 +8,24 @@
 
 ; TEST SCC test returning an integer value argument
 ;
-; CHECK: Function Attrs: noinline norecurse nounwind readnone uwtable
-; CHECK: define i32 @sink_r0(i32 returned %r)
-;
-; FIXME: returned on %r missing:
-; CHECK: Function Attrs: noinline nounwind readnone uwtable
-; CHECK: define i32 @scc_r1(i32 %a, i32 %r, i32 %b)
-;
-; FIXME: returned on %r missing:
-; CHECK: Function Attrs: noinline nounwind readnone uwtable
-; CHECK: define i32 @scc_r2(i32 %a, i32 %b, i32 %r)
+; BOTH: Function Attrs: noinline norecurse nounwind readnone uwtable
+; BOTH-NEXT: define i32 @sink_r0(i32 returned %r)
+; BOTH: Function Attrs: noinline nounwind readnone uwtable
+; BOTH-NEXT: define i32 @scc_r1(i32 %a, i32 returned %r, i32 %b)
+; BOTH: Function Attrs: noinline nounwind readnone uwtable
+; BOTH-NEXT: define i32 @scc_r2(i32 %a, i32 %b, i32 returned %r)
+; BOTH: Function Attrs: noinline nounwind readnone uwtable
+; BOTH-NEXT: define i32 @scc_rX(i32 %a, i32 %b, i32 %r)
+;
+; FNATTR: define i32 @sink_r0(i32 returned %r)
+; FNATTR: define i32 @scc_r1(i32 %a, i32 %r, i32 %b)
+; FNATTR: define i32 @scc_r2(i32 %a, i32 %b, i32 %r)
+; FNATTR: define i32 @scc_rX(i32 %a, i32 %b, i32 %r)
+;
+; ATTRIBUTOR: define i32 @sink_r0(i32 returned %r)
+; ATTRIBUTOR: define i32 @scc_r1(i32 %a, i32 returned %r, i32 %b)
+; ATTRIBUTOR: define i32 @scc_r2(i32 %a, i32 %b, i32 returned %r)
+; ATTRIBUTOR: define i32 @scc_rX(i32 %a, i32 %b, i32 %r)
 ;
 ; int scc_r1(int a, int b, int r);
 ; int 

[PATCH] D62831: [CodeGen][ObjC] Add attribute "arc_retain_agnostic" to ObjC globals that are retain-agnostic

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

I agree. Something like `arc_inert` is probably a better name in this case for 
the reason you mentioned.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62831



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

[libclc] r363031 - Creating release candidate rc2 from release_801 branch

[Tom Stellard via cfe-commits]

Author: tstellar
Date: Mon Jun 10 21:00:01 2019
New Revision: 363031

URL: http://llvm.org/viewvc/llvm-project?rev=363031=rev
Log:
Creating release candidate rc2 from release_801 branch

Added:
libclc/tags/RELEASE_801/rc2/
  - copied from r363030, libclc/branches/release_80/

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

[libunwind] r363031 - Creating release candidate rc2 from release_801 branch

[Tom Stellard via cfe-commits]

Author: tstellar
Date: Mon Jun 10 21:00:01 2019
New Revision: 363031

URL: http://llvm.org/viewvc/llvm-project?rev=363031=rev
Log:
Creating release candidate rc2 from release_801 branch

Added:
libunwind/tags/RELEASE_801/rc2/
  - copied from r363030, libunwind/branches/release_80/

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

[PATCH] D62831: [CodeGen][ObjC] Add attribute "arc_retain_agnostic" to ObjC globals that are retain-agnostic

[Michael Gottesman via Phabricator via cfe-commits]

gottesmm added a comment.

This is exactly what I was imagining! This will enable the frontend to opt into 
this optimization without having to touch the optimizer. One nit: can we use a 
different name than "arc_retain_agnostic". Have you considered something like 
"arc_inert"? My fear is that at a glance (without thinking), you would think 
that the attribute would have something only to do with retain when we are 
really talking about ARC value operations. Beyond that looks great!


Repository:
  rC Clang

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

https://reviews.llvm.org/D62831



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

[libunwind] r363030 - Merging r360862:

[Tom Stellard via cfe-commits]

Author: tstellar
Date: Mon Jun 10 20:47:50 2019
New Revision: 363030

URL: http://llvm.org/viewvc/llvm-project?rev=363030=rev
Log:
Merging r360862:


r360862 | mstorsjo | 2019-05-15 23:49:20 -0700 (Wed, 15 May 2019) | 12 lines

[PPC] Fix 32-bit build of libunwind

Clang integrated assembler was unable to build libunwind PPC32 assembly code,
present in functions used to save/restore register context.

This change consists in replacing the assembly style used in libunwind source,
to one that is compatible with both Clang integrated assembler as well as
GNU assembler.

Patch by Leandro Lupori!

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


Modified:
libunwind/branches/release_80/src/UnwindRegistersRestore.S
libunwind/branches/release_80/src/UnwindRegistersSave.S
libunwind/branches/release_80/src/assembly.h

Modified: libunwind/branches/release_80/src/UnwindRegistersRestore.S
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/branches/release_80/src/UnwindRegistersRestore.S?rev=363030=363029=363030=diff
==
--- libunwind/branches/release_80/src/UnwindRegistersRestore.S (original)
+++ libunwind/branches/release_80/src/UnwindRegistersRestore.S Mon Jun 10 
20:47:50 2019
@@ -396,119 +396,119 @@ Lnovec:
 #elif defined(__ppc__)
 
 DEFINE_LIBUNWIND_PRIVATE_FUNCTION(_ZN9libunwind13Registers_ppc6jumptoEv)
-;
-; void libunwind::Registers_ppc::jumpto()
-;
-; On entry:
-;  thread_state pointer is in r3
-;
-
-  ; restore integral registerrs
-  ; skip r0 for now
-  ; skip r1 for now
-  lwz r2, 16(r3)
-  ; skip r3 for now
-  ; skip r4 for now
-  ; skip r5 for now
-  lwz r6, 32(r3)
-  lwz r7, 36(r3)
-  lwz r8, 40(r3)
-  lwz r9, 44(r3)
-  lwzr10, 48(r3)
-  lwzr11, 52(r3)
-  lwzr12, 56(r3)
-  lwzr13, 60(r3)
-  lwzr14, 64(r3)
-  lwzr15, 68(r3)
-  lwzr16, 72(r3)
-  lwzr17, 76(r3)
-  lwzr18, 80(r3)
-  lwzr19, 84(r3)
-  lwzr20, 88(r3)
-  lwzr21, 92(r3)
-  lwzr22, 96(r3)
-  lwzr23,100(r3)
-  lwzr24,104(r3)
-  lwzr25,108(r3)
-  lwzr26,112(r3)
-  lwzr27,116(r3)
-  lwzr28,120(r3)
-  lwzr29,124(r3)
-  lwzr30,128(r3)
-  lwzr31,132(r3)
-
-  ; restore float registers
-  lfdf0, 160(r3)
-  lfdf1, 168(r3)
-  lfdf2, 176(r3)
-  lfdf3, 184(r3)
-  lfdf4, 192(r3)
-  lfdf5, 200(r3)
-  lfdf6, 208(r3)
-  lfdf7, 216(r3)
-  lfdf8, 224(r3)
-  lfdf9, 232(r3)
-  lfdf10,240(r3)
-  lfdf11,248(r3)
-  lfdf12,256(r3)
-  lfdf13,264(r3)
-  lfdf14,272(r3)
-  lfdf15,280(r3)
-  lfdf16,288(r3)
-  lfdf17,296(r3)
-  lfdf18,304(r3)
-  lfdf19,312(r3)
-  lfdf20,320(r3)
-  lfdf21,328(r3)
-  lfdf22,336(r3)
-  lfdf23,344(r3)
-  lfdf24,352(r3)
-  lfdf25,360(r3)
-  lfdf26,368(r3)
-  lfdf27,376(r3)
-  lfdf28,384(r3)
-  lfdf29,392(r3)
-  lfdf30,400(r3)
-  lfdf31,408(r3)
-
-  ; restore vector registers if any are in use
-  lwzr5,156(r3)  ; test VRsave
-  cmpwi  r5,0
-  beqLnovec
-
-  subi  r4,r1,16
-  rlwinm  r4,r4,0,0,27  ; mask low 4-bits
-  ; r4 is now a 16-byte aligned pointer into the red zone
-  ; the _vectorRegisters may not be 16-byte aligned so copy via red zone temp 
buffer
-
+//
+// void libunwind::Registers_ppc::jumpto()
+//
+// On entry:
+//  thread_state pointer is in r3
+//
+
+  // restore integral registerrs
+  // skip r0 for now
+  // skip r1 for now
+  lwz %r2,  16(%r3)
+  // skip r3 for now
+  // skip r4 for now
+  // skip r5 for now
+  lwz %r6,  32(%r3)
+  lwz %r7,  36(%r3)
+  lwz %r8,  40(%r3)
+  lwz %r9,  44(%r3)
+  lwz %r10, 48(%r3)
+  lwz %r11, 52(%r3)
+  lwz %r12, 56(%r3)
+  lwz %r13, 60(%r3)
+  lwz %r14, 64(%r3)
+  lwz %r15, 68(%r3)
+  lwz %r16, 72(%r3)
+  lwz %r17, 76(%r3)
+  lwz %r18, 80(%r3)
+  lwz %r19, 84(%r3)
+  lwz %r20, 88(%r3)
+  lwz %r21, 92(%r3)
+  lwz %r22, 96(%r3)
+  lwz %r23,100(%r3)
+  lwz %r24,104(%r3)
+  lwz %r25,108(%r3)
+  lwz %r26,112(%r3)
+  lwz %r27,116(%r3)
+  lwz %r28,120(%r3)
+  lwz %r29,124(%r3)
+  lwz %r30,128(%r3)
+  lwz %r31,132(%r3)
+
+  // restore float registers
+  lfd %f0, 160(%r3)
+  lfd %f1, 168(%r3)
+  lfd %f2, 176(%r3)
+  lfd %f3, 184(%r3)
+  lfd %f4, 192(%r3)
+  lfd %f5, 200(%r3)
+  lfd %f6, 208(%r3)
+  lfd %f7, 216(%r3)
+  lfd %f8, 224(%r3)
+  lfd %f9, 232(%r3)
+  lfd %f10,240(%r3)
+  lfd %f11,248(%r3)
+  lfd %f12,256(%r3)
+  lfd %f13,264(%r3)
+  lfd %f14,272(%r3)
+  lfd %f15,280(%r3)
+  lfd %f16,288(%r3)
+  lfd %f17,296(%r3)
+  lfd %f18,304(%r3)
+  lfd %f19,312(%r3)
+  lfd %f20,320(%r3)
+  lfd %f21,328(%r3)
+  lfd %f22,336(%r3)
+  lfd 

[PATCH] D44865: [libc++] Implement P0608R3 - A sane variant converting constructor

[Zhihao Yuan via Phabricator via cfe-commits]

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



Comment at: 
test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp:130
   {
 using V = std::variant;
 static_assert(!std::is_assignable::value, "ambiguous");

mclow.lists wrote:
> If you really want to check that these are "ambiguous" , or "no matching 
> operator=", etc, the way to do that is to define a fail.cpp test, and check 
> the error messages.
>  
(the messages in static_assert are just notes to people who read this code)

I added additional fail.cpp tests.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D44865



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

[PATCH] D44865: [libc++] Implement P0608R3 - A sane variant converting constructor

[Zhihao Yuan via Phabricator via cfe-commits]

lichray updated this revision to Diff 203966.
lichray added a comment.

Add fail tests


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D44865

Files:
  include/variant
  test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp
  test/std/utilities/variant/variant.variant/variant.assign/conv.fail.cpp
  test/std/utilities/variant/variant.variant/variant.ctor/T.pass.cpp
  test/std/utilities/variant/variant.variant/variant.ctor/conv.fail.cpp
  www/cxx2a_status.html

Index: www/cxx2a_status.html
===
--- www/cxx2a_status.html
+++ www/cxx2a_status.html
@@ -115,7 +115,7 @@
 	https://wg21.link/P0591R4;>P0591R4LWGUtility functions to implement uses-allocator constructionSan Diego 
 	https://wg21.link/P0595R2;>P0595R2CWGP0595R2 std::is_constant_evaluated()San DiegoComplete9.0
 	https://wg21.link/P0602R4;>P0602R4LWGvariant and optional should propagate copy/move trivialitySan DiegoComplete8.0
-	https://wg21.link/P0608R3;>P0608R3LWGA sane variant converting constructorSan Diego 
+	https://wg21.link/P0608R3;>P0608R3LWGA sane variant converting constructorSan DiegoComplete9.0
 	https://wg21.link/P0655R1;>P0655R1LWGvisitR: Explicit Return Type for visitSan Diego 
 	https://wg21.link/P0771R1;>P0771R1LWGstd::function move constructor should be noexceptSan DiegoComplete6.0
 	https://wg21.link/P0896R4;>P0896R4LWGThe One Ranges ProposalSan Diego 
Index: test/std/utilities/variant/variant.variant/variant.ctor/conv.fail.cpp
===
--- /dev/null
+++ test/std/utilities/variant/variant.variant/variant.ctor/conv.fail.cpp
@@ -0,0 +1,39 @@
+// -*- 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
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03, c++11, c++14
+
+// 
+
+// template  class variant;
+
+// template  constexpr variant(T&&) noexcept(see below);
+
+#include 
+#include 
+#include 
+
+int main(int, char**)
+{
+  std::variant v1 = 1; // expected-error {{no viable conversion}}
+  std::variant v2 = 1; // expected-error {{no viable conversion}}
+  std::variant v3 = 1; // expected-error {{no viable conversion}}
+
+  std::variant v4 = 1; // expected-error {{no viable conversion}}
+  std::variant v5 = 1; // expected-error {{no viable conversion}}
+  std::variant v6 = 1; // expected-error {{no viable conversion}}
+
+  std::variant v7 = "meow"; // expected-error {{no viable conversion}}
+  std::variant v8 = "meow"; // expected-error {{no viable conversion}}
+  std::variant v9 = "meow"; // expected-error {{no viable conversion}}
+
+  std::variant v10 = std::true_type(); // expected-error {{no viable conversion}}
+  std::variant v11 = std::unique_ptr(); // expected-error {{no viable conversion}}
+  std::variant v12 = nullptr; // expected-error {{no viable conversion}}
+}
Index: test/std/utilities/variant/variant.variant/variant.ctor/T.pass.cpp
===
--- test/std/utilities/variant/variant.variant/variant.ctor/T.pass.cpp
+++ test/std/utilities/variant/variant.variant/variant.ctor/T.pass.cpp
@@ -20,8 +20,8 @@
 #include 
 #include 
 #include 
+#include 
 
-#include "test_convertible.hpp"
 #include "test_macros.h"
 #include "variant_test_helpers.hpp"
 
@@ -39,6 +39,8 @@
 
 struct AnyConstructible { template  AnyConstructible(T&&) {} };
 struct NoConstructible { NoConstructible() = delete; };
+template 
+struct RValueConvertibleFrom { RValueConvertibleFrom(T&&) {} };
 
 void test_T_ctor_noexcept() {
   {
@@ -53,7 +55,7 @@
 
 void test_T_ctor_sfinae() {
   {
-using V = std::variant;
+using V = std::variant;
 static_assert(!std::is_constructible::value, "ambiguous");
   }
   {
@@ -66,6 +68,32 @@
   "no matching constructor");
   }
   {
+using V = std::variant;
+static_assert(!std::is_constructible::value,
+  "no matching constructor");
+  }
+  {
+using V = std::variant, bool>;
+static_assert(!std::is_constructible>::value,
+  "no explicit bool in constructor");
+struct X {
+  operator void*();
+};
+static_assert(!std::is_constructible::value,
+  "no boolean conversion in constructor");
+static_assert(!std::is_constructible::value,
+  "no converted to bool in constructor");
+  }
+  {
+struct X {};
+struct Y {
+  operator X();
+};
+using V = std::variant;
+static_assert(std::is_constructible::value,
+  "regression on user-defined conversions in constructor");
+  }
+  {
 using V = std::variant;
 

[PATCH] D62988: Add an attribute to allow fields of non-trivial types in C unions

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

In D62988#1537082 , @rjmccall wrote:

> Does this lead to C/C++ ABI mismatches?  Should we just honor this in C++ as 
> well by ignoring it when deciding to delete special members?  Is taking such 
> a general name a good idea if it's language-specific?  Richard, thoughts?


This is a C-only attribute, so clang will emit a diagnostic (warning 'attribute 
ignored') if the attribute is used to annotate a member of a C++ union. I think 
that would be sufficient to prevent possible C/C++ ABI mismatches?


Repository:
  rC Clang

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

https://reviews.llvm.org/D62988



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

[PATCH] D44865: [libc++] Implement P0608R3 - A sane variant converting constructor

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added inline comments.



Comment at: include/variant:1128
+template 
+struct __overload
+: __overload_bool<__overload<_Types...>, bool const volatile> {};

EricWF wrote:
> Do we even support volatile types in variant?
[[ http://eel.is/c++draft/variant.variant#2 | Yes. ]]

> All types in Types shall be (possibly cv-qualified) object types that are not 
> arrays.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D44865



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

[PATCH] D44865: [libc++] Implement P0608R3 - A sane variant converting constructor

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added inline comments.



Comment at: include/variant:1128
+template 
+struct __overload
+: __overload_bool<__overload<_Types...>, bool const volatile> {};

Do we even support volatile types in variant?


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D44865



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

[PATCH] D63118: [analyzer] DeadStores: Add a crude suppression files generated by DriverKit IIG.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added a reviewer: dcoughlin.
Herald added subscribers: cfe-commits, Charusso, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: clang.

IIG is a replacement for MIG in DriverKit: IIG is autogenerating C++ code. 
Suppress dead store warnings on such code, as the tool seems to be producing 
them regularly, and the users of IIG are not in position to address these 
warnings, as they don't control the autogenerated code. IIG-generated code is 
identified by looking at the comments at the top of the file.


Repository:
  rC Clang

https://reviews.llvm.org/D63118

Files:
  clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
  clang/test/Analysis/deadstores-driverkit.cpp
  clang/test/Analysis/os_object_base.h


Index: clang/test/Analysis/os_object_base.h
===
--- clang/test/Analysis/os_object_base.h
+++ clang/test/Analysis/os_object_base.h
@@ -19,6 +19,9 @@
 
 using size_t = decltype(sizeof(int));
 
+typedef int kern_return_t;
+struct IORPC {};
+
 struct OSMetaClass;
 
 struct OSMetaClassBase {
@@ -37,8 +40,13 @@
 
   virtual void free();
   virtual ~OSMetaClassBase(){};
+
+  kern_return_t Invoke(IORPC invoke);
 };
 
+typedef kern_return_t (*OSDispatchMethod)(OSMetaClassBase *self,
+  const IORPC rpc);
+
 struct OSObject : public OSMetaClassBase {
   virtual ~OSObject(){}
 
Index: clang/test/Analysis/deadstores-driverkit.cpp
===
--- /dev/null
+++ clang/test/Analysis/deadstores-driverkit.cpp
@@ -0,0 +1,24 @@
+/* iig generated from SomethingSomething.iig */
+
+// The comment above is the whole point of the test.
+// That's how the suppression works.
+// It needs to be on the top.
+// Run-lines can wait.
+
+// RUN: %clang_analyze_cc1 -w -triple x86_64-apple-driverkit19.0 \
+// RUN:   -analyzer-checker=deadcode -verify %s
+
+// expected-no-diagnostics
+
+#include "os_object_base.h"
+
+class OSSomething {
+  kern_return_t Invoke(const IORPC);
+  void foo(OSDispatchMethod supermethod) {
+kern_return_t ret;
+IORPC rpc;
+// Test the DriverKit specific suppression in the dead stores checker.
+if (supermethod) ret = supermethod((OSObject *)this, rpc); // no-warning
+else ret = ((OSObject *)this)->Invoke(rpc); // no-warning
+  }
+};
Index: clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
@@ -160,6 +160,26 @@
 return InEH->count(D);
   }
 
+  bool isSuppressed(SourceRange R) {
+SourceManager  = Ctx.getSourceManager();
+SourceLocation Loc = R.getBegin();
+if (!Loc.isValid())
+  return false;
+
+FileID FID = SMgr.getFileID(Loc);
+bool Invalid = false;
+StringRef Data = SMgr.getBufferData(FID, );
+if (Invalid)
+  return false;
+
+// Files autogenerated by DriverKit IIG contain some dead stores that
+// we don't want to report.
+if (Data.startswith("/* iig generated from"))
+  return true;
+
+return false;
+  }
+
   void Report(const VarDecl *V, DeadStoreKind dsk,
   PathDiagnosticLocation L, SourceRange R) {
 if (Escaped.count(V))
@@ -175,6 +195,9 @@
 if (!reachableCode->isReachable(currentBlock))
   return;
 
+if (isSuppressed(R))
+  return;
+
 SmallString<64> buf;
 llvm::raw_svector_ostream os(buf);
 const char *BugType = nullptr;


Index: clang/test/Analysis/os_object_base.h
===
--- clang/test/Analysis/os_object_base.h
+++ clang/test/Analysis/os_object_base.h
@@ -19,6 +19,9 @@
 
 using size_t = decltype(sizeof(int));
 
+typedef int kern_return_t;
+struct IORPC {};
+
 struct OSMetaClass;
 
 struct OSMetaClassBase {
@@ -37,8 +40,13 @@
 
   virtual void free();
   virtual ~OSMetaClassBase(){};
+
+  kern_return_t Invoke(IORPC invoke);
 };
 
+typedef kern_return_t (*OSDispatchMethod)(OSMetaClassBase *self,
+  const IORPC rpc);
+
 struct OSObject : public OSMetaClassBase {
   virtual ~OSObject(){}
 
Index: clang/test/Analysis/deadstores-driverkit.cpp
===
--- /dev/null
+++ clang/test/Analysis/deadstores-driverkit.cpp
@@ -0,0 +1,24 @@
+/* iig generated from SomethingSomething.iig */
+
+// The comment above is the whole point of the test.
+// That's how the suppression works.
+// It needs to be on the top.
+// Run-lines can wait.
+
+// RUN: %clang_analyze_cc1 -w -triple x86_64-apple-driverkit19.0 \
+// RUN:   -analyzer-checker=deadcode -verify %s
+
+// expected-no-diagnostics
+
+#include "os_object_base.h"
+
+class OSSomething {
+  kern_return_t Invoke(const IORPC);
+  void 

[PATCH] D63117: [analyzer] RetainCount: Add support for OSRequiredCast().

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added a reviewer: dcoughlin.
Herald added subscribers: cfe-commits, Charusso, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: clang.

It's a new API for custom RTTI in Apple IOKit/DriverKit framework that is 
similar to `OSDynamicCast()` that's already supported, but crashes with 
assertion instead of returning null (and therefore causing UB when the cast 
fails unexpectedly). Kinda like `cast_or_null<>` as opposed to 
`dyn_cast_or_null<>` in LLVM's RTTI.

Historically, `RetainCountChecker` is responsible for modeling `OSDynamicCast`, 
so i simply extend this functionality.


Repository:
  rC Clang

https://reviews.llvm.org/D63117

Files:
  clang/lib/Analysis/RetainSummaryManager.cpp
  clang/test/Analysis/os_object_base.h
  clang/test/Analysis/osobject-retain-release.cpp


Index: clang/test/Analysis/osobject-retain-release.cpp
===
--- clang/test/Analysis/osobject-retain-release.cpp
+++ clang/test/Analysis/osobject-retain-release.cpp
@@ -1,9 +1,11 @@
 // RUN: %clang_analyze_cc1 -fblocks -analyze -analyzer-output=text\
-// RUN:-analyzer-checker=core,osx -verify %s
+// RUN:   -analyzer-checker=core,osx,debug.ExprInspection -verify %s
 
 #include "os_object_base.h"
 #include "os_smart_ptr.h"
 
+void clang_analyzer_eval(bool);
+
 struct OSIterator : public OSObject {
   static const OSMetaClass * const metaClass;
 };
@@ -483,6 +485,23 @@
   arr->release();
 }
 
+void check_required_cast() {
+  OSArray *arr = OSRequiredCast(OSArray, OSObject::generateObject(1));
+  arr->release(); // no-warning
+}
+
+void check_cast_behavior(OSObject *obj) {
+  OSArray *arr1 = OSDynamicCast(OSArray, obj);
+  clang_analyzer_eval(arr1 == obj); // expected-warning{{TRUE}}
+// expected-note@-1{{TRUE}}
+// expected-note@-2{{Assuming 'arr1' is 
not equal to 'obj'}}
+// expected-warning@-3{{FALSE}}
+// expected-note@-4   {{FALSE}}
+  OSArray *arr2 = OSRequiredCast(OSArray, obj);
+  clang_analyzer_eval(arr2 == obj); // expected-warning{{TRUE}}
+// expected-note@-1{{TRUE}}
+}
+
 unsigned int check_dynamic_cast_no_null_on_orig(OSObject *obj) {
   OSArray *arr = OSDynamicCast(OSArray, obj);
   if (arr) {
Index: clang/test/Analysis/os_object_base.h
===
--- clang/test/Analysis/os_object_base.h
+++ clang/test/Analysis/os_object_base.h
@@ -12,6 +12,8 @@
 
 #define OSDynamicCast(type, inst)   \
 ((type *) OSMetaClassBase::safeMetaCast((inst), OSTypeID(type)))
+#define OSRequiredCast(type, inst)   \
+((type *) OSMetaClassBase::requiredMetaCast((inst), OSTypeID(type)))
 
 #define OSTypeAlloc(type)   ((type *) ((type::metaClass)->alloc()))
 
@@ -22,6 +24,8 @@
 struct OSMetaClassBase {
   static OSMetaClassBase *safeMetaCast(const OSMetaClassBase *inst,
const OSMetaClass *meta);
+  static OSMetaClassBase *requiredMetaCast(const OSMetaClassBase *inst,
+   const OSMetaClass *meta);
 
   OSMetaClassBase *metaCast(const char *toMeta);
 
Index: clang/lib/Analysis/RetainSummaryManager.cpp
===
--- clang/lib/Analysis/RetainSummaryManager.cpp
+++ clang/lib/Analysis/RetainSummaryManager.cpp
@@ -152,6 +152,10 @@
   return S == "safeMetaCast";
 }
 
+static bool isOSObjectRequiredCast(StringRef S) {
+  return S == "requiredMetaCast";
+}
+
 static bool isOSObjectThisCast(StringRef S) {
   return S == "metaCast";
 }
@@ -234,7 +238,8 @@
   if (RetTy->isPointerType()) {
 const CXXRecordDecl *PD = RetTy->getPointeeType()->getAsCXXRecordDecl();
 if (PD && isOSObjectSubclass(PD)) {
-  if (isOSObjectDynamicCast(FName) || isOSObjectThisCast(FName))
+  if (isOSObjectDynamicCast(FName) || isOSObjectRequiredCast(FName) ||
+  isOSObjectThisCast(FName))
 return getDefaultSummary();
 
   // TODO: Add support for the slightly common *Matching(table) idiom.
@@ -745,6 +750,8 @@
 if (TrackOSObjects) {
   if (isOSObjectDynamicCast(FName) && FD->param_size() >= 1) {
 return BehaviorSummary::IdentityOrZero;
+  } else if (isOSObjectRequiredCast(FName) && FD->param_size() >= 1) {
+return BehaviorSummary::Identity;
   } else if (isOSObjectThisCast(FName) && isa(FD) &&
  !cast(FD)->isStatic()) {
 return BehaviorSummary::IdentityThis;


Index: clang/test/Analysis/osobject-retain-release.cpp
===
--- clang/test/Analysis/osobject-retain-release.cpp
+++ clang/test/Analysis/osobject-retain-release.cpp
@@ -1,9 +1,11 @@
 // RUN: %clang_analyze_cc1 -fblocks -analyze 

[PATCH] D63093: [analyzer] WIP: MallocChecker: Release temporary CXXNewExpr

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In such cases i recommend starting with writing down a test. Like in TDD: first 
test, //then// code.

The general direction doesn't seem reasonable to me; it introduces some 
pattern-matching for a specific scenario, but it's unclear why is this scenario 
a problem on its own. We might eventually do something similar, but I recommend 
//fully// debugging the false positive - i.e., understanding what exactly is 
wrong with it, before picking a suppression mechanism.




Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1119-1122
+  // Set the symbol's state to Released.
+  State = State->set(
+  Sym, RefState::getReleased(NE->isArray() ? AF_CXXNewArray : 
AF_CXXNew,
+ NE));

I think it's clearly too early for marking the pointer as released. I.e., what 
about:
```lang=c++
auto x = std::shared_ptr(new int); // the pointer is marked as released
use(x.get()); // use-after-free???
```


Repository:
  rC Clang

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

https://reviews.llvm.org/D63093



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

[PATCH] D63080: [analyzer] Track indices of arrays

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Whoa, this looks like a much needed improvement, i'm glad that you found it!




Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1681
+trackExpressionValue(
+LVNode, Arr->getIdx(), report, EnableNullFPSuppression);
+

Mmm, dunno about null fp suppression. We're, like, talking about integers. 
Integers are more often zero than null. We generally do support some FP 
suppressions for integers as well (i.e., `core.DivideZero` uses them), but in 
this case it doesn't sound as if `0` is anyhow special.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63080



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

[PATCH] D62883: [analyzer] Track conditions of terminator statements on which the reported node depends on

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D62883#1536894 , @Szelethus wrote:

> I'm not sure how long it'll take for me to figure out what's wrong, but these 
> numbers are so ridiculous, I suspect a programming error rather then an 
> algorithmic issue.
>
> edit: I seem to have found a solution, I'll share more later when I get to 
> evaluate this :)


At a glance, it might be that `bugreporter::trackExpressionValue()` is called 
on every node within the block, rather than only once in a lifetime of the 
visitor.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62883



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

[PATCH] D63086: [analyzer][NoStoreFuncVisitor][NFC] Move methods out-of-line, turn some to static functions

[Artem Dergachev via Phabricator via cfe-commits]

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

Thx for the cleanup!




Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:340
+private:
+  /// Attempts to find the region of interest in a given CXX decl,
+  /// by either following the base classes or fields.

Wait, it's not necessarily a CXX decl.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63086



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

[PATCH] D62926: [analyzer] ReturnVisitor: Bypass everything to see inlined calls

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:842-849
+  if (Optional CEE = Node->getLocationAs())
 if (CEE->getCalleeContext()->getCallSite() == S)
   break;
-  if (auto SP = Node->getLocationAs())
-if (SP->getStmt() == S)
-  break;
+
+  if (!IsBypass)
+if (Optional SP = Node->getLocationAs())
+  if (SP->getStmt() == S)

Charusso wrote:
> NoQ wrote:
> > Comparing statements is usually insufficient because the same statement may 
> > appear multiple times due to recursion. When recursion occurs, you may 
> > reach the same statement in a different location context. You should think 
> > in terms of (statement, location context) pairs to avoid these problems. 
> > Your aim here is to find the `CallExitEnd` node that corresponds to 
> > returning from an inlined operator new to the current location context. You 
> > should stop searching when you find an unrelated statement in the current 
> > location context or when you exit the current location context entirely.
> I have made a little test when we have a 25-second long Static Analyzer run 
> with predefined names and checker. The loop ran 500 times in summary and we 
> have some serious performance impacts at other places.
> 
> We exit the current context to see inlined calls, so that could not work 
> sadly. If you remove that nonsense second condition we run at the same time, 
> so if we have not got any problem since 7 years ago I think it is good to go.
You should break the loop as soon as we go past the new-expression that we've 
started with in the stack frame that we've started with. That is, you should at 
most go through the constructor within the new-expression, and then break. You 
shouldn't explore the whole graph to the root every time the operator-new call 
isn't inlined.

This might still be slow in some rare cases, but it should be much faster.


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

https://reviews.llvm.org/D62926



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

[PATCH] D62978: [analyzer] ReturnVisitor: Handle unknown ReturnStmts better

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/test/Analysis/diagnostics/find_last_store.c:9
 void no_find_last_store() {
-  c *e = d(); // expected-note{{Calling 'd'}}
-  // expected-note@-1{{Returning from 'd'}}
-  // expected-note@-2{{'e' initialized here}}
+  c *e = d(); // expected-note{{'e' initialized here}}
 

This remaining note is also unnecessary. You can safely stop tracking the value 
at `e || ...`. In particular, `ReturnVisitor` is not at fault.

That said, when we renamed `trackNullOrUndefValue` to `trackExpressionValue`, 
we kinda assumed that it's not really important whether the value is null/undef 
or not - the function simply tracks the value. This change would break this 
invariant, causing null values to be handled in a special manner. I recommend 
adding another flag argument (similar to `EnableNullFPSuppression`) that would 
allow the caller to tell whether it's interested in the null or in the "whole" 
value (defaulting to the latter).


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

https://reviews.llvm.org/D62978



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

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.
Herald added a subscriber: Charusso.
Herald added a project: clang.

Hey, i'm seeing a crash in this checker, would you like to look at it? It looks 
as if you're not being careful about dereferences/lvalue-to-rvalue-casts so it 
tries to compare `` to `e1`.

**$ `cat repro.c`**

  enum E { e1 };
  
  void foo() {
enum E e;
e;
  }

**$ `clang --analyze repro.c -Xclang 
-analyzer-checker=alpha.cplusplus.EnumCastOutOfRange`**

  Assertion failed: (op == BO_Add), function evalBinOp, file 
/Users/adergachev/llvm/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp, line 427.
  
  Stack dump:
  0.Program arguments: /Users/adergachev/debug/bin/clang-9 -cc1 -triple 
x86_64-apple-macosx10.14.0 -Wdeprecated-objc-isa-usage 
-Werror=deprecated-objc-isa-usage -analyze -disable-free -main-file-name 
repro.c -analyzer-store=region -analyzer-opt-analyze-nested-blocks 
-analyzer-checker=core -analyzer-checker=apiModeling -analyzer-checker=unix 
-analyzer-checker=osx -analyzer-checker=deadcode 
-analyzer-checker=security.insecureAPI.UncheckedReturn 
-analyzer-checker=security.insecureAPI.getpw 
-analyzer-checker=security.insecureAPI.gets 
-analyzer-checker=security.insecureAPI.mktemp 
-analyzer-checker=security.insecureAPI.mkstemp 
-analyzer-checker=security.insecureAPI.vfork 
-analyzer-checker=nullability.NullPassedToNonnull 
-analyzer-checker=nullability.NullReturnedFromNonnull -analyzer-output plist -w 
-mrelocation-model pic -pic-level 2 -mthread-model posix -mdisable-fp-elim 
-masm-verbose -munwind-tables -target-cpu penryn -dwarf-column-info 
-debugger-tuning=lldb -ggnu-pubnames -target-linker-version 510.2 -resource-dir 
/Users/adergachev/debug/lib/clang/9.0.0 -internal-isystem /usr/local/include 
-internal-isystem /Users/adergachev/debug/lib/clang/9.0.0/include 
-internal-externc-isystem /usr/include -fdebug-compilation-dir 
/Users/adergachev/test -ferror-limit 19 -fmessage-length 142 -stack-protector 1 
-fblocks -fencode-extended-block-signature -fregister-global-dtors-with-atexit 
-fobjc-runtime=macosx-10.14.0 -fmax-type-align=16 -fdiagnostics-show-option 
-fcolor-diagnostics -analyzer-checker=alpha.cplusplus.EnumCastOutOfRange -o 
repro.plist -x c repro.c
  1. parser at end of file
  2.While analyzing stack:
#0 Calling foo
  3.repro.c:5:3: Error evaluating statement
  4.repro.c:5:3: Error evaluating statement
  0  clang-9  0x0001043f98cc 
llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 60
  1  clang-9  0x0001043f9e89 
PrintStackTraceSignalHandler(void*) + 25
  2  clang-9  0x0001043f7bd6 llvm::sys::RunSignalHandlers() 
+ 118
  3  clang-9  0x0001043fd032 SignalHandler(int) + 210
  4  libsystem_platform.dylib 0x7fff63a0eb5d _sigtramp + 29
  5  clang-9  0x00010a444d08 
llvm::DenseMapInfo::Tombstone + 3005112
  6  libsystem_c.dylib0x7fff638ce6a6 abort + 127
  7  libsystem_c.dylib0x7fff6389720d basename_r + 0
  8  clang-9  0x000107048c06 
clang::ento::SValBuilder::evalBinOp(llvm::IntrusiveRefCntPtr, clang::BinaryOperatorKind, clang::ento::SVal, clang::ento::SVal, 
clang::QualType) + 950
  9  clang-9  0x000107048ef0 
clang::ento::SValBuilder::evalEQ(llvm::IntrusiveRefCntPtr, clang::ento::SVal, clang::ento::SVal) + 144
  10 clang-9  0x000107048f82 
clang::ento::SValBuilder::evalEQ(llvm::IntrusiveRefCntPtr, clang::ento::DefinedOrUnknownSVal, clang::ento::DefinedOrUnknownSVal) 
+ 114
  11 clang-9  0x000106afe56f (anonymous 
namespace)::ConstraintBasedEQEvaluator::operator()(llvm::APSInt const&) + 175
  12 clang-9  0x000106afe3ef bool 
std::__1::any_of(llvm::APSInt*, llvm::APSInt*, 
(anonymous namespace)::ConstraintBasedEQEvaluator) + 47
  13 clang-9  0x000106afdd18 bool 
llvm::any_of&, (anonymous 
namespace)::ConstraintBasedEQEvaluator>(llvm::SmallVector&, 
(anonymous namespace)::ConstraintBasedEQEvaluator) + 72
  14 clang-9  0x000106afdbb9 (anonymous 
namespace)::EnumCastOutOfRangeChecker::checkPreStmt(clang::CastExpr const*, 
clang::ento::CheckerContext&) const + 297
  15 clang-9  0x000106afda85 void 
clang::ento::check::PreStmt::_checkStmt<(anonymous 
namespace)::EnumCastOutOfRangeChecker>(void*, clang::Stmt const*, 
clang::ento::CheckerContext&) + 53
  16 clang-9  0x000106f128a2 clang::ento::CheckerFn::operator()(clang::Stmt 
const*, clang::ento::CheckerContext&) const + 66
  17 clang-9  0x000106f1232c (anonymous 
namespace)::CheckStmtContext::runChecker(clang::ento::CheckerFn, clang::ento::NodeBuilder&, 
clang::ento::ExplodedNode*) + 220
  18 clang-9  0x000106effd71 void 
expandGraphWithCheckers<(anonymous namespace)::CheckStmtContext>((anonymous 
namespace)::CheckStmtContext, clang::ento::ExplodedNodeSet&, 

[PATCH] D62363: [X86] Enable intrinsics that convert float and bf16 data to each other

[Pengfei Wang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL363018: [X86] Enable intrinsics that convert float and bf16 
data to each other (authored by pengfei, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62363?vs=203762=203950#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62363

Files:
  cfe/trunk/include/clang/Basic/BuiltinsX86.def
  cfe/trunk/lib/CodeGen/CGBuiltin.cpp
  cfe/trunk/lib/Headers/avx512bf16intrin.h
  cfe/trunk/lib/Headers/avx512vlbf16intrin.h
  cfe/trunk/test/CodeGen/avx512bf16-builtins.c
  cfe/trunk/test/CodeGen/avx512vlbf16-builtins.c

Index: cfe/trunk/lib/Headers/avx512vlbf16intrin.h
===
--- cfe/trunk/lib/Headers/avx512vlbf16intrin.h
+++ cfe/trunk/lib/Headers/avx512vlbf16intrin.h
@@ -403,6 +403,71 @@
 (__v8sf)_mm256_setzero_si256());
 }
 
+/// Convert One Single float Data to One BF16 Data.
+///
+/// \headerfile 
+///
+/// This intrinsic corresponds to the  VCVTNEPS2BF16  instructions.
+///
+/// \param __A
+///A float data.
+/// \returns A bf16 data whose sign field and exponent field keep unchanged,
+///and fraction field is truncated to 7 bits.
+static __inline__ __bfloat16 __DEFAULT_FN_ATTRS128 _mm_cvtness_sbh(float __A) {
+  __v4sf __V = {__A, 0, 0, 0};
+  __v8hi __R = __builtin_ia32_cvtneps2bf16_128_mask(
+  (__v4sf)__V, (__v8hi)_mm_undefined_si128(), (__mmask8)-1);
+  return __R[0];
+}
+
+/// Convert Packed BF16 Data to Packed float Data.
+///
+/// \headerfile 
+///
+/// \param __A
+///A 128-bit vector of [8 x bfloat].
+/// \returns A 256-bit vector of [8 x float] come from convertion of __A
+static __inline__ __m256 __DEFAULT_FN_ATTRS256 _mm256_cvtpbh_ps(__m128bh __A) {
+  return _mm256_castsi256_ps((__m256i)_mm256_slli_epi32(
+  (__m256i)_mm256_cvtepi16_epi32((__m128i)__A), 16));
+}
+
+/// Convert Packed BF16 Data to Packed float Data using zeroing mask.
+///
+/// \headerfile 
+///
+/// \param __U
+///A 8-bit mask. Elements are zeroed out when the corresponding mask
+///bit is not set.
+/// \param __A
+///A 128-bit vector of [8 x bfloat].
+/// \returns A 256-bit vector of [8 x float] come from convertion of __A
+static __inline__ __m256 __DEFAULT_FN_ATTRS256
+_mm256_maskz_cvtpbh_ps(__mmask8 __U, __m128bh __A) {
+  return _mm256_castsi256_ps((__m256i)_mm256_slli_epi32(
+  (__m256i)_mm256_maskz_cvtepi16_epi32((__mmask8)__U, (__m128i)__A), 16));
+}
+
+/// Convert Packed BF16 Data to Packed float Data using merging mask.
+///
+/// \headerfile 
+///
+/// \param __S
+///A 256-bit vector of [8 x float]. Elements are copied from __S when
+/// the corresponding mask bit is not set.
+/// \param __U
+///A 8-bit mask. Elements are zeroed out when the corresponding mask
+///bit is not set.
+/// \param __A
+///A 128-bit vector of [8 x bfloat].
+/// \returns A 256-bit vector of [8 x float] come from convertion of __A
+static __inline__ __m256 __DEFAULT_FN_ATTRS256
+_mm256_mask_cvtpbh_ps(__m256 __S, __mmask8 __U, __m128bh __A) {
+  return _mm256_castsi256_ps((__m256i)_mm256_mask_slli_epi32(
+  (__m256i)__S, (__mmask8)__U, (__m256i)_mm256_cvtepi16_epi32((__m128i)__A),
+  16));
+}
+
 #undef __DEFAULT_FN_ATTRS128
 #undef __DEFAULT_FN_ATTRS256
 
Index: cfe/trunk/lib/Headers/avx512bf16intrin.h
===
--- cfe/trunk/lib/Headers/avx512bf16intrin.h
+++ cfe/trunk/lib/Headers/avx512bf16intrin.h
@@ -15,10 +15,27 @@
 
 typedef short __m512bh __attribute__((__vector_size__(64), __aligned__(64)));
 typedef short __m256bh __attribute__((__vector_size__(32), __aligned__(32)));
+typedef unsigned short __bfloat16;
 
 #define __DEFAULT_FN_ATTRS512 \
   __attribute__((__always_inline__, __nodebug__, __target__("avx512bf16"), \
  __min_vector_width__(512)))
+#define __DEFAULT_FN_ATTRS \
+  __attribute__((__always_inline__, __nodebug__, __target__("avx512bf16")))
+
+/// Convert One BF16 Data to One Single Float Data.
+///
+/// \headerfile 
+///
+/// This intrinsic does not correspond to a specific instruction.
+///
+/// \param __A
+///A bfloat data.
+/// \returns A float data whose sign field and exponent field keep unchanged,
+///and fraction field is extended to 23 bits.
+static __inline__ float __DEFAULT_FN_ATTRS _mm_cvtsbh_ss(__bfloat16 __A) {
+  return __builtin_ia32_cvtsbf162ss_32(__A);
+}
 
 /// Convert Two Packed Single Data to One Packed BF16 Data.
 ///
@@ -209,6 +226,54 @@
(__v16sf)_mm512_setzero_si512());
 }
 
+/// Convert Packed BF16 Data to Packed float Data.
+///
+/// \headerfile 
+///
+/// \param __A
+///A 256-bit vector of [16 x bfloat].
+/// \returns A 

r363018 - [X86] Enable intrinsics that convert float and bf16 data to each other

[Pengfei Wang via cfe-commits]

Author: pengfei
Date: Mon Jun 10 18:17:28 2019
New Revision: 363018

URL: http://llvm.org/viewvc/llvm-project?rev=363018=rev
Log:
[X86] Enable intrinsics that convert float and bf16 data to each other

Scalar version :
_mm_cvtsbh_ss , _mm_cvtness_sbh

Vector version:
_mm512_cvtpbh_ps , _mm256_cvtpbh_ps
_mm512_maskz_cvtpbh_ps , _mm256_maskz_cvtpbh_ps
_mm512_mask_cvtpbh_ps , _mm256_mask_cvtpbh_ps

Patch by Shengchen Kan (skan)

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

Modified:
cfe/trunk/include/clang/Basic/BuiltinsX86.def
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/Headers/avx512bf16intrin.h
cfe/trunk/lib/Headers/avx512vlbf16intrin.h
cfe/trunk/test/CodeGen/avx512bf16-builtins.c
cfe/trunk/test/CodeGen/avx512vlbf16-builtins.c

Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=363018=363017=363018=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Mon Jun 10 18:17:28 2019
@@ -1831,6 +1831,8 @@ TARGET_BUILTIN(__builtin_ia32_cvtusi2ss3
 TARGET_BUILTIN(__builtin_ia32_vpmultishiftqb512, "V64cV64cV64c", "ncV:512:", 
"avx512vbmi")
 TARGET_BUILTIN(__builtin_ia32_vpmultishiftqb128, "V16cV16cV16c", "ncV:128:", 
"avx512vbmi,avx512vl")
 TARGET_BUILTIN(__builtin_ia32_vpmultishiftqb256, "V32cV32cV32c", "ncV:256:", 
"avx512vbmi,avx512vl")
+
+// bf16 intrinsics
 TARGET_BUILTIN(__builtin_ia32_cvtne2ps2bf16_128, "V8sV4fV4f", "ncV:128:", 
"avx512bf16,avx512vl")
 TARGET_BUILTIN(__builtin_ia32_cvtne2ps2bf16_256, "V16sV8fV8f", "ncV:256:", 
"avx512bf16,avx512vl")
 TARGET_BUILTIN(__builtin_ia32_cvtne2ps2bf16_512, "V32sV16fV16f", "ncV:512:", 
"avx512bf16")
@@ -1840,6 +1842,8 @@ TARGET_BUILTIN(__builtin_ia32_cvtneps2bf
 TARGET_BUILTIN(__builtin_ia32_dpbf16ps_128, "V4fV4fV4iV4i", "ncV:128:", 
"avx512bf16,avx512vl")
 TARGET_BUILTIN(__builtin_ia32_dpbf16ps_256, "V8fV8fV8iV8i", "ncV:256:", 
"avx512bf16,avx512vl")
 TARGET_BUILTIN(__builtin_ia32_dpbf16ps_512, "V16fV16fV16iV16i", "ncV:512:", 
"avx512bf16")
+TARGET_BUILTIN(__builtin_ia32_cvtsbf162ss_32, "fUs", "nc", "avx512bf16")
+
 TARGET_BUILTIN(__builtin_ia32_vp2intersect_q_512, "vV8LLiV8LLiUc*Uc*", 
"nV:512:", "avx512vp2intersect")
 TARGET_BUILTIN(__builtin_ia32_vp2intersect_q_256, "vV4LLiV4LLiUc*Uc*", 
"nV:256:", "avx512vp2intersect,avx512vl")
 TARGET_BUILTIN(__builtin_ia32_vp2intersect_q_128, "vV2LLiV2LLiUc*Uc*", 
"nV:128:", "avx512vp2intersect,avx512vl")

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=363018=363017=363018=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Mon Jun 10 18:17:28 2019
@@ -9795,6 +9795,18 @@ Value *CodeGenFunction::EmitX86CpuIs(con
   return EmitX86CpuIs(CPUStr);
 }
 
+// Convert a BF16 to a float.
+static Value *EmitX86CvtBF16ToFloatExpr(CodeGenFunction ,
+const CallExpr *E,
+ArrayRef Ops) {
+  llvm::Type *Int32Ty = CGF.Builder.getInt32Ty();
+  Value *ZeroExt = CGF.Builder.CreateZExt(Ops[0], Int32Ty);
+  Value *Shl = CGF.Builder.CreateShl(ZeroExt, 16);
+  llvm::Type *ResultType = CGF.ConvertType(E->getType());
+  Value *BitCast = CGF.Builder.CreateBitCast(Shl, ResultType);
+  return BitCast;
+}
+
 Value *CodeGenFunction::EmitX86CpuIs(StringRef CPUStr) {
 
   llvm::Type *Int32Ty = Builder.getInt32Ty();
@@ -11941,6 +11953,8 @@ Value *CodeGenFunction::EmitX86BuiltinEx
 Intrinsic::ID IID = Intrinsic::x86_avx512bf16_mask_cvtneps2bf16_128;
 return Builder.CreateCall(CGM.getIntrinsic(IID), Ops);
   }
+  case X86::BI__builtin_ia32_cvtsbf162ss_32:
+return EmitX86CvtBF16ToFloatExpr(*this, E, Ops);
 
   case X86::BI__builtin_ia32_cvtneps2bf16_256_mask:
   case X86::BI__builtin_ia32_cvtneps2bf16_512_mask: {

Modified: cfe/trunk/lib/Headers/avx512bf16intrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/avx512bf16intrin.h?rev=363018=363017=363018=diff
==
--- cfe/trunk/lib/Headers/avx512bf16intrin.h (original)
+++ cfe/trunk/lib/Headers/avx512bf16intrin.h Mon Jun 10 18:17:28 2019
@@ -15,10 +15,27 @@
 
 typedef short __m512bh __attribute__((__vector_size__(64), __aligned__(64)));
 typedef short __m256bh __attribute__((__vector_size__(32), __aligned__(32)));
+typedef unsigned short __bfloat16;
 
 #define __DEFAULT_FN_ATTRS512 \
   __attribute__((__always_inline__, __nodebug__, __target__("avx512bf16"), \
  __min_vector_width__(512)))
+#define __DEFAULT_FN_ATTRS 
\
+  __attribute__((__always_inline__, __nodebug__, __target__("avx512bf16")))

[PATCH] D62225: [clang][NewPM] Fixing -O0 tests that are broken under new PM

[Chandler Carruth via Phabricator via cfe-commits]

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

I think this ultimately needs to be split up into smaller patches. A bunch of 
these things can be landed independently. Here is my first cut at things to 
split out, each one into its own patch.

1. the LLVM change to the always inliner
2. the Clang change to how we build the always inliner
3. the PGO pipeline changes (which I have to admit I still don't fully 
understand)
4. The additions of `-fno-experimental-new-pass-manager` for test cases that 
are explicitly testing legacy PM behavior
5. switching tests to be resilient to changes in attribute group numbering (and 
adding a RUN line w/ the new PM to ensure we don't regress)

for #5 (or others) where *some* testing needs to be working before they can 
land, just sequence them after whatever they depend on

Other things I think can also be split out, but I suspect into *different* 
changes from what you have here:

6. Instead of passing `-fno-experimental-new-pass-manager` for tests that use 
`-O` but don't specify a number, Clang should pick a consistent value for the 
level I think

I'd be interested to then see what is left here.




Comment at: clang/lib/CodeGen/BackendUtil.cpp:1104-1105
   // which is just that always inlining occurs.
-  MPM.addPass(AlwaysInlinerPass());
+  // We always pass false here since according to the legacy PM logic for
+  // enabling lifetime intrinsics, we should not be compiling with O0.
+  MPM.addPass(AlwaysInlinerPass(/*InsertLifetimeIntrinsics=*/false));

leonardchan wrote:
> serge-sans-paille wrote:
> > echristo wrote:
> > > Can you elaborate more here? We do turn on the always inliner at O0 which 
> > > makes this comment a bit confusing.
> > I guess he means 
> > 
> > We always pass false here since according to the legacy PM logic for 
> > enabling lifetime intrinsics, they are not required with O0
> > 
> Yup, my bad. This is what I meant with this comment. Always inlining is used. 
> It's the lifetime intrinsics that aren't always used.
I don't think explaining it in terms of one pass manager or another is the righ 
thing to do.

Instead, I'd say what the desired result is:

```
Build a minimal pipeline based on the semantics required by Clang,
which is just that always inlining occurs. Further, disable generating
lifetime intrinsics to avoid enabling further optimizations during
code generation.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62225



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

[PATCH] D62888: [NewPM] Port Sancov

[Chandler Carruth via Phabricator via cfe-commits]

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

I would just change this to have the module pass loop over the functions -- 
that seems like it'll be much cleaner.

As it is, I'm not seeing where the loop actually happens. But rather than 
trying to figure that out, just seems better to turn it into an explicit loop. 
That way you can get rid of all the check analysis overhead I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62888



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

[PATCH] D63108: [OpenMP] Add support for handling declare target to clause when unified memory is required

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea created this revision.
gtbercea added reviewers: ABataev, AlexEichenberger, caomhin.
Herald added subscribers: cfe-commits, jdoerfert, guansong.
Herald added a project: clang.

This patch adds support for the handling of the variables under the declare 
target to clause.

The variables in this case are handled like link variables are. A pointer is 
created on the host and then mapped to the device. The runtime will then copy 
the address of the host variable in the device pointer.


Repository:
  rC Clang

https://reviews.llvm.org/D63108

Files:
  lib/CodeGen/CGDeclCXX.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGOpenMPRuntime.h
  lib/CodeGen/CodeGenModule.cpp
  test/OpenMP/nvptx_target_requires_unified_shared_memory.cpp

Index: test/OpenMP/nvptx_target_requires_unified_shared_memory.cpp
===
--- test/OpenMP/nvptx_target_requires_unified_shared_memory.cpp
+++ test/OpenMP/nvptx_target_requires_unified_shared_memory.cpp
@@ -8,16 +8,18 @@
 #define N 1000
 
 double var = 10.0;
+double to_var = 20.0;
 
 #pragma omp requires unified_shared_memory
 #pragma omp declare target link(var)
+#pragma omp declare target to(to_var)
 
 int bar(int n){
   double sum = 0;
 
 #pragma omp target
   for(int i = 0; i < n; i++) {
-sum += var;
+sum += var + to_var;
   }
 
   return sum;
@@ -26,9 +28,20 @@
 // CHECK: [[VAR:@.+]] = global double 1.00e+01
 // CHECK: [[VAR_DECL_TGT_LINK_PTR:@.+]] = global double* [[VAR]]
 
+// CHECK: [[TO_VAR:@.+]] = global double 2.00e+01
+// CHECK: [[VAR_DECL_TGT_TO_PTR:@.+]] = global double* [[TO_VAR]]
+
 // CHECK: [[OFFLOAD_SIZES:@.+]] = private unnamed_addr constant [2 x i64] [i64 4, i64 8]
 // CHECK: [[OFFLOAD_MAPTYPES:@.+]] = private unnamed_addr constant [2 x i64] [i64 800, i64 800]
 
+// CHECK: [[OMP_OFFLOAD_ENTRY_LINK_VAR_PTR_NAME:@.+]] = internal unnamed_addr constant [22 x i8]
+// CHECK: [[OMP_OFFLOAD_ENTRY_LINK_VAR_PTR:@.+]] = weak constant %struct.__tgt_offload_entry { i8* bitcast (double** [[VAR_DECL_TGT_LINK_PTR]] to i8*), i8* getelementptr inbounds ([22 x i8], [22 x i8]* [[OMP_OFFLOAD_ENTRY_LINK_VAR_PTR_NAME]], i32 0, i32 0), i64 8, i32 1, i32 0 }, section ".omp_offloading.entries"
+
+// CHECK: [[OMP_OFFLOAD_ENTRY_TO_VAR_PTR_NAME:@.+]] = internal unnamed_addr constant [23 x i8]
+// CHECK: [[OMP_OFFLOAD_ENTRY_TO_VAR_PTR:@.+]] = weak constant %struct.__tgt_offload_entry { i8* bitcast (double** [[VAR_DECL_TGT_TO_PTR]] to i8*), i8* getelementptr inbounds ([23 x i8], [23 x i8]* [[OMP_OFFLOAD_ENTRY_TO_VAR_PTR_NAME]], i32 0, i32 0), i64 8, i32 0, i32 0 }, section ".omp_offloading.entries"
+
+// CHECK: @llvm.used = appending global [2 x i8*] [i8* bitcast (double** [[VAR_DECL_TGT_LINK_PTR]] to i8*), i8* bitcast (double** [[VAR_DECL_TGT_TO_PTR]] to i8*)], section "llvm.metadata"
+
 // CHECK: [[N_CASTED:%.+]] = alloca i64
 // CHECK: [[SUM_CASTED:%.+]] = alloca i64
 
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -2477,10 +2477,13 @@
 if (llvm::Optional Res =
 OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(VD)) {
   if (*Res == OMPDeclareTargetDeclAttr::MT_To) {
-(void)GetAddrOfGlobalVar(VD);
+if (getOpenMPRuntime().hasRequiresUnifiedSharedMemory())
+  (void)getOpenMPRuntime().getAddrOfDeclareTargetToUnderUnifiedMem(VD);
+else
+  (void)GetAddrOfGlobalVar(VD);
   } else {
 assert(*Res == OMPDeclareTargetDeclAttr::MT_Link &&
-   "link claue expected.");
+   "link clause expected.");
 (void)getOpenMPRuntime().getAddrOfDeclareTargetLink(VD);
   }
   return;
Index: lib/CodeGen/CGOpenMPRuntime.h
===
--- lib/CodeGen/CGOpenMPRuntime.h
+++ lib/CodeGen/CGOpenMPRuntime.h
@@ -1120,6 +1120,8 @@
  Address VDAddr,
  SourceLocation Loc);
 
+  virtual Address getAddrOfDeclareTargetToUnderUnifiedMem(const VarDecl *VD);
+
   /// Returns the address of the variable marked as declare target with link
   /// clause.
   virtual Address getAddrOfDeclareTargetLink(const VarDecl *VD);
Index: lib/CodeGen/CGOpenMPRuntime.cpp
===
--- lib/CodeGen/CGOpenMPRuntime.cpp
+++ lib/CodeGen/CGOpenMPRuntime.cpp
@@ -2531,11 +2531,16 @@
 return Address::invalid();
   llvm::Optional Res =
   OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(VD);
-  if (Res && *Res == OMPDeclareTargetDeclAttr::MT_Link) {
+  if (Res && (*Res == OMPDeclareTargetDeclAttr::MT_Link ||
+  (*Res == OMPDeclareTargetDeclAttr::MT_To &&
+   HasRequiresUnifiedSharedMemory))) {
 SmallString<64> PtrName;
  

[PATCH] D63101: [Frontend] SetUpDiagnosticLog should handle unowned diagnostic consumer in the compiler

[Alex Lorenz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL363009: [Frontend] SetUpDiagnosticLog should handle unowned 
diagnostic consumer (authored by arphaman, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63101?vs=203910=203931#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63101

Files:
  cfe/trunk/lib/Frontend/CompilerInstance.cpp
  cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp


Index: cfe/trunk/lib/Frontend/CompilerInstance.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInstance.cpp
+++ cfe/trunk/lib/Frontend/CompilerInstance.cpp
@@ -232,9 +232,13 @@
 
std::move(StreamOwner));
   if (CodeGenOpts)
 Logger->setDwarfDebugFlags(CodeGenOpts->DwarfDebugFlags);
-  assert(Diags.ownsClient());
-  Diags.setClient(
-  new ChainedDiagnosticConsumer(Diags.takeClient(), std::move(Logger)));
+  if (Diags.ownsClient()) {
+Diags.setClient(
+new ChainedDiagnosticConsumer(Diags.takeClient(), std::move(Logger)));
+  } else {
+Diags.setClient(
+new ChainedDiagnosticConsumer(Diags.getClient(), std::move(Logger)));
+  }
 }
 
 static void SetupSerializedDiagnostics(DiagnosticOptions *DiagOpts,
Index: cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp
===
--- cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp
+++ cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp
@@ -8,6 +8,7 @@
 
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/ToolOutputFile.h"
@@ -70,4 +71,21 @@
   ASSERT_TRUE(Instance.getFileManager().getFile("vfs-virtual.file"));
 }
 
+TEST(CompilerInstance, AllowDiagnosticLogWithUnownedDiagnosticConsumer) {
+  auto DiagOpts = new DiagnosticOptions();
+  DiagOpts->DiagnosticLogFile = "log.diags";
+
+  // Create the diagnostic engine with unowned consumer.
+  std::string DiagnosticOutput;
+  llvm::raw_string_ostream DiagnosticsOS(DiagnosticOutput);
+  auto DiagPrinter = llvm::make_unique(
+  DiagnosticsOS, new DiagnosticOptions());
+  CompilerInstance Instance;
+  IntrusiveRefCntPtr Diags = Instance.createDiagnostics(
+  DiagOpts, DiagPrinter.get(), /*ShouldOwnClient=*/false);
+
+  Diags->Report(diag::err_expected) << "no crash";
+  ASSERT_EQ(DiagnosticsOS.str(), "error: expected no crash\n");
+}
+
 } // anonymous namespace


Index: cfe/trunk/lib/Frontend/CompilerInstance.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInstance.cpp
+++ cfe/trunk/lib/Frontend/CompilerInstance.cpp
@@ -232,9 +232,13 @@
 std::move(StreamOwner));
   if (CodeGenOpts)
 Logger->setDwarfDebugFlags(CodeGenOpts->DwarfDebugFlags);
-  assert(Diags.ownsClient());
-  Diags.setClient(
-  new ChainedDiagnosticConsumer(Diags.takeClient(), std::move(Logger)));
+  if (Diags.ownsClient()) {
+Diags.setClient(
+new ChainedDiagnosticConsumer(Diags.takeClient(), std::move(Logger)));
+  } else {
+Diags.setClient(
+new ChainedDiagnosticConsumer(Diags.getClient(), std::move(Logger)));
+  }
 }
 
 static void SetupSerializedDiagnostics(DiagnosticOptions *DiagOpts,
Index: cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp
===
--- cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp
+++ cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp
@@ -8,6 +8,7 @@
 
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/ToolOutputFile.h"
@@ -70,4 +71,21 @@
   ASSERT_TRUE(Instance.getFileManager().getFile("vfs-virtual.file"));
 }
 
+TEST(CompilerInstance, AllowDiagnosticLogWithUnownedDiagnosticConsumer) {
+  auto DiagOpts = new DiagnosticOptions();
+  DiagOpts->DiagnosticLogFile = "log.diags";
+
+  // Create the diagnostic engine with unowned consumer.
+  std::string DiagnosticOutput;
+  llvm::raw_string_ostream DiagnosticsOS(DiagnosticOutput);
+  auto DiagPrinter = llvm::make_unique(
+  DiagnosticsOS, new DiagnosticOptions());
+  CompilerInstance Instance;
+  IntrusiveRefCntPtr Diags = Instance.createDiagnostics(
+  DiagOpts, DiagPrinter.get(), /*ShouldOwnClient=*/false);
+
+  Diags->Report(diag::err_expected) << "no crash";
+  ASSERT_EQ(DiagnosticsOS.str(), "error: expected no crash\n");
+}
+
 } // anonymous namespace

r363009 - [Frontend] SetUpDiagnosticLog should handle unowned diagnostic consumer

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Mon Jun 10 16:32:42 2019
New Revision: 363009

URL: http://llvm.org/viewvc/llvm-project?rev=363009=rev
Log:
[Frontend] SetUpDiagnosticLog should handle unowned diagnostic consumer
in the compiler

The function SetUpDiagnosticLog that was called from createDiagnostics didn't
handle the case where the diagnostics engine didn't own the diagnostics 
consumer.
This is a potential problem for a clang tool, in particular some of the 
follow-up
patches for clang-scan-deps will need this fix.

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

Modified:
cfe/trunk/lib/Frontend/CompilerInstance.cpp
cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp

Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=363009=363008=363009=diff
==
--- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Mon Jun 10 16:32:42 2019
@@ -232,9 +232,13 @@ static void SetUpDiagnosticLog(Diagnosti
 
std::move(StreamOwner));
   if (CodeGenOpts)
 Logger->setDwarfDebugFlags(CodeGenOpts->DwarfDebugFlags);
-  assert(Diags.ownsClient());
-  Diags.setClient(
-  new ChainedDiagnosticConsumer(Diags.takeClient(), std::move(Logger)));
+  if (Diags.ownsClient()) {
+Diags.setClient(
+new ChainedDiagnosticConsumer(Diags.takeClient(), std::move(Logger)));
+  } else {
+Diags.setClient(
+new ChainedDiagnosticConsumer(Diags.getClient(), std::move(Logger)));
+  }
 }
 
 static void SetupSerializedDiagnostics(DiagnosticOptions *DiagOpts,

Modified: cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp?rev=363009=363008=363009=diff
==
--- cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp (original)
+++ cfe/trunk/unittests/Frontend/CompilerInstanceTest.cpp Mon Jun 10 16:32:42 
2019
@@ -8,6 +8,7 @@
 
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/ToolOutputFile.h"
@@ -70,4 +71,21 @@ TEST(CompilerInstance, DefaultVFSOverlay
   ASSERT_TRUE(Instance.getFileManager().getFile("vfs-virtual.file"));
 }
 
+TEST(CompilerInstance, AllowDiagnosticLogWithUnownedDiagnosticConsumer) {
+  auto DiagOpts = new DiagnosticOptions();
+  DiagOpts->DiagnosticLogFile = "log.diags";
+
+  // Create the diagnostic engine with unowned consumer.
+  std::string DiagnosticOutput;
+  llvm::raw_string_ostream DiagnosticsOS(DiagnosticOutput);
+  auto DiagPrinter = llvm::make_unique(
+  DiagnosticsOS, new DiagnosticOptions());
+  CompilerInstance Instance;
+  IntrusiveRefCntPtr Diags = Instance.createDiagnostics(
+  DiagOpts, DiagPrinter.get(), /*ShouldOwnClient=*/false);
+
+  Diags->Report(diag::err_expected) << "no crash";
+  ASSERT_EQ(DiagnosticsOS.str(), "error: expected no crash\n");
+}
+
 } // anonymous namespace


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

r363007 - Revert r362994 & co "[analyzer][tests] Add normalize_plist to replace diff_plist"

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Mon Jun 10 16:25:43 2019
New Revision: 363007

URL: http://llvm.org/viewvc/llvm-project?rev=363007=rev
Log:
Revert r362994 & co "[analyzer][tests] Add normalize_plist to replace 
diff_plist"

Reverts r362998, r362996, and r362994 because the tests do not pass on
Windows due to CRLF changes. Adding back `-w` to diff is not enough, the
new grep substitution doesn't work on Windows, and fixing it is
non-trivial.

Modified:
cfe/trunk/test/Analysis/MismatchedDeallocator-path-notes.cpp
cfe/trunk/test/Analysis/NewDelete-path-notes.cpp
cfe/trunk/test/Analysis/conditional-path-notes.c
cfe/trunk/test/Analysis/copypaste/plist-diagnostics-notes-as-events.cpp
cfe/trunk/test/Analysis/copypaste/plist-diagnostics.cpp
cfe/trunk/test/Analysis/cxx-for-range.cpp
cfe/trunk/test/Analysis/diagnostics/deref-track-symbolic-region.c
cfe/trunk/test/Analysis/diagnostics/plist-diagnostics-include-check.cpp
cfe/trunk/test/Analysis/diagnostics/plist-multi-file.c
cfe/trunk/test/Analysis/diagnostics/report-issues-within-main-file.cpp
cfe/trunk/test/Analysis/diagnostics/undef-value-caller.c
cfe/trunk/test/Analysis/diagnostics/undef-value-param.c
cfe/trunk/test/Analysis/diagnostics/undef-value-param.m
cfe/trunk/test/Analysis/edges-new.mm
cfe/trunk/test/Analysis/generics.m
cfe/trunk/test/Analysis/inline-plist.c
cfe/trunk/test/Analysis/inline-unique-reports.c
cfe/trunk/test/Analysis/inlining/eager-reclamation-path-notes.c
cfe/trunk/test/Analysis/inlining/eager-reclamation-path-notes.cpp
cfe/trunk/test/Analysis/inlining/path-notes.c
cfe/trunk/test/Analysis/inlining/path-notes.cpp
cfe/trunk/test/Analysis/inlining/path-notes.m
cfe/trunk/test/Analysis/lambda-notes.cpp
cfe/trunk/test/Analysis/lit.local.cfg
cfe/trunk/test/Analysis/malloc-plist.c
cfe/trunk/test/Analysis/method-call-path-notes.cpp
cfe/trunk/test/Analysis/model-file.cpp
cfe/trunk/test/Analysis/null-deref-path-notes.m
cfe/trunk/test/Analysis/nullability-notes.m
cfe/trunk/test/Analysis/objc-arc.m
cfe/trunk/test/Analysis/objc-radar17039661.m
cfe/trunk/test/Analysis/plist-macros-with-expansion.cpp
cfe/trunk/test/Analysis/plist-macros.cpp
cfe/trunk/test/Analysis/plist-output-alternate.m
cfe/trunk/test/Analysis/plist-output.m
cfe/trunk/test/Analysis/retain-release-path-notes.m
cfe/trunk/test/Analysis/retain-release.m
cfe/trunk/test/Analysis/unix-fns.c

Modified: cfe/trunk/test/Analysis/MismatchedDeallocator-path-notes.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/MismatchedDeallocator-path-notes.cpp?rev=363007=363006=363007=diff
==
--- cfe/trunk/test/Analysis/MismatchedDeallocator-path-notes.cpp (original)
+++ cfe/trunk/test/Analysis/MismatchedDeallocator-path-notes.cpp Mon Jun 10 
16:25:43 2019
@@ -1,6 +1,6 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.MismatchedDeallocator 
-analyzer-output=text -verify %s
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.MismatchedDeallocator 
-analyzer-output=plist %s -o %t.plist
-// RUN: tail -n +11 %t.plist | %normalize_plist | diff -u 
%S/copypaste/Inputs/expected-plists/MismatchedDeallocator-path-notes.cpp.plist -
+// RUN: tail -n +11 %t.plist | %diff_plist 
%S/copypaste/Inputs/expected-plists/MismatchedDeallocator-path-notes.cpp.plist -
 
 void changePointee(int *p);
 int *allocIntArray(unsigned c) {

Modified: cfe/trunk/test/Analysis/NewDelete-path-notes.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/NewDelete-path-notes.cpp?rev=363007=363006=363007=diff
==
--- cfe/trunk/test/Analysis/NewDelete-path-notes.cpp (original)
+++ cfe/trunk/test/Analysis/NewDelete-path-notes.cpp Mon Jun 10 16:25:43 2019
@@ -11,7 +11,7 @@
 // RUN:   -analyzer-checker=cplusplus.NewDelete,unix.Malloc \
 // RUN:   -analyzer-config add-pop-up-notes=false \
 // RUN:   -analyzer-output=plist %s -o %t.plist
-// RUN: %normalize_plist <%t.plist | diff -u \
+// RUN: cat %t.plist | %diff_plist \
 // RUN:   %S/Inputs/expected-plists/NewDelete-path-notes.cpp.plist -
 
 void test() {

Modified: cfe/trunk/test/Analysis/conditional-path-notes.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/conditional-path-notes.c?rev=363007=363006=363007=diff
==
--- cfe/trunk/test/Analysis/conditional-path-notes.c (original)
+++ cfe/trunk/test/Analysis/conditional-path-notes.c Mon Jun 10 16:25:43 2019
@@ -1,6 +1,6 @@
 // RUN: %clang_analyze_cc1 %s -analyzer-checker=core.NullDereference 
-analyzer-output=text -verify
 // RUN: %clang_analyze_cc1 %s -analyzer-checker=core.NullDereference 
-analyzer-output=plist -o %t
-// RUN: %normalize_plist <%t | diff -u 
%S/Inputs/expected-plists/conditional-path-notes.c.plist -
+// RUN: cat %t 

[PATCH] D59744: Fix i386 ABI "__m64" type bug

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/CodeGen/TargetInfo.cpp:1005
   bool IsMCUABI;
+  bool IsLinuxABI;
   unsigned DefaultNumRegisterParameters;

mgorny wrote:
> Maybe replace the two booleans with something alike `IsPassInMMXRegABI`? And 
> while at it, include NetBSD there, please.
`CGT` is a member variable, so you can just query the target fresh in your 
`isPassInMMXRegABI` method.  The check upfront for a 64-bit vector type should 
keep this well out of the fast path.


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

https://reviews.llvm.org/D59744



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

[PATCH] D63072: [clang] Fixing incorrect implicit deduction guides (PR41549)

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

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

Thanks!


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

https://reviews.llvm.org/D63072



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

[PATCH] D62988: Add an attribute to allow fields of non-trivial types in C unions

[John McCall via Phabricator via cfe-commits]

rjmccall added a reviewer: rsmith.
rjmccall added a comment.

Does this lead to C/C++ ABI mismatches?  Should we just honor this in C++ as 
well by ignoring it when deciding to delete special members?  Is taking such a 
general name a good idea if it's language-specific?  Richard, thoughts?


Repository:
  rC Clang

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

https://reviews.llvm.org/D62988



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

[PATCH] D50360: [Concepts] Requires Expressions

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

rsmith added inline comments.



Comment at: include/clang/AST/DeclCXX.h:2051
+/// \code
+/// template requires requires (T t) { {t++} -> Regular };
+/// \endcode

The semicolon here is in the wrong place (should be before the final `}`, not 
after).



Comment at: include/clang/AST/DeclTemplate.h:1185
   bool wereArgumentsSpecified() const {
-return ArgsAsWritten == nullptr;
+return ArgsAsWritten != nullptr;
   }

Please move this bugfix earlier in the patch series.



Comment at: include/clang/AST/ExprCXX.h:4634
+  llvm::SmallVector Requirements;
+  SourceLocation RBraceLoc;
+

Reorder this next to `RequiresKWLoc`. `SourceLocation`s are 4 bytes, whereas 
for most platforms we care about, pointers are size-8 and align-8, so the 
reordering will save 8 bytes of padding.



Comment at: include/clang/AST/ExprCXX.h:4645
+
+  void setLocalParameters(ArrayRef LocalParameters);
+  ArrayRef getLocalParameters() const { return LocalParameters; 
}

Please avoid adding setters to `Expr` subclasses if possible. We want the AST 
to be immutable. (Befriend `ASTReaderStmt` instead.)



Comment at: include/clang/AST/ExprCXX.h:4674
+  llvm::SmallVector LocalParameters;
+  llvm::SmallVector Requirements;
+  SourceLocation RBraceLoc;

riccibruno wrote:
> Can you tail-allocate them ?
Using a `SmallVector` here is a bug; the destructor is not run on `Expr` nodes 
so this will leak memory. Please do change to using tail allocation here.



Comment at: include/clang/Basic/DiagnosticParseKinds.td:752
+def err_requires_expr_simple_requirement_unexpected_tok : Error<
+  "unexpected %0 after expression. Did you intend to use a compound "
+  "requirement? (with '{' '}' around the expression)">;

`. Did` -> `; did` so this fits the general diagnostic pattern regardless of 
whether the diagnostic renderer capitalizes the diagnostic.



Comment at: include/clang/Basic/DiagnosticParseKinds.td:753
+  "unexpected %0 after expression. Did you intend to use a compound "
+  "requirement? (with '{' '}' around the expression)">;
 

Move `?` to the end.



Comment at: lib/AST/ExprCXX.cpp:32-35
 #include "clang/Sema/Template.h"
 #include "clang/Sema/SemaDiagnostic.h"
 #include "clang/Sema/Sema.h"
+#include "clang/Sema/Overload.h"

These `#include`s are all unacceptable. `AST` is layered below `Sema`, and 
cannot depend on `Sema` headers and classes.



Comment at: lib/AST/ExprConstant.cpp:9953-9955
+  if (E->isValueDependent()) {
+return Error(E);
+  }

It is a logic error for the expression evaluator to encounter a value-dependent 
expression, and we assert on the way into the evaluator if we would encounter 
one. You don't need to check this here.



Comment at: lib/AST/StmtProfile.cpp:1325
+  //expression. It is equivalent to the simple-requirement x++; [...]
+  // We therefore do not profile isSimple() here.
+  ID.AddBoolean(ExprReq->getNoexceptLoc().isValid());

We don't /need to/ profile `isSimple`, but we still could. (This "is equivalent 
to" doesn't override the general ODR requirement that you spell the expression 
with the same token sequence.)

Do we mangle simple and compound requirements the same way? (Has a mangling for 
requires-expressions even been proposed on the Itanium ABI list yet?)



Comment at: lib/Parse/ParseExprCXX.cpp:3097
+Braces.consumeClose();
+return ExprError();
+  }

Recovering by producing an (always-satisfied) `RequiresExpr` with no 
requirements would seem reasonable here.



Comment at: lib/Parse/ParseExprCXX.cpp:3113
+switch (Tok.getKind()) {
+case tok::kw_typename: {
+  // Type requirement

This is incorrect. A //simple-requirement// can begin with the `typename` 
keyword. (Eg, `requires { typename T::type() + 1; }`)

The right way to handle this is to call `TryAnnotateTypeOrScopeToken()` when 
you see `tok::kw_typename`, and then detect a `type-requirement` as a 
`tok::annot_typename` followed by a `tok::semi`. (By following that approach, 
you can also handle cases where the `typename` is missing.) You'll need to deal 
specially with the case where the //nested-name-specifier// is omitted, since 
in that case the `typename` keyword does not form part of a 
//typename-specifier//; in that case, after `TryAnnotateTypeOrScopeToken()` 
you'll have a `tok::kw_typename`, `tok::identifier`, `tok::semi` sequence or a 
`tok::kw_typename`, `tok::annot_template_id`, `tok::semi` sequence to detect 
and special-case.



Comment at: lib/Parse/ParseExprCXX.cpp:3125
+  SS.isInvalid()) {
+Braces.skipToEnd();
+Actions.ActOnExitRequiresExpr();

[PATCH] D62975: Require stdcall etc parameters to be complete on ODR use

[Reid Kleckner via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL363000: Require stdcall etc parameters to be complete on ODR 
use (authored by rnk, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62975?vs=203423=203922#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62975

Files:
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/test/Sema/calling-conv-complete-params.c

Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2985,6 +2985,9 @@
 def warn_declspec_allocator_nonpointer : Warning<
   "ignoring __declspec(allocator) because the function return type %0 is not "
   "a pointer or reference type">, InGroup;
+def err_cconv_incomplete_param_type : Error<
+  "parameter %0 must have a complete type to use function %1 with the %2 "
+  "calling convention">;
 
 def ext_cannot_use_trivial_abi : ExtWarn<
   "'trivial_abi' cannot be applied to %0">, InGroup;
Index: cfe/trunk/test/Sema/calling-conv-complete-params.c
===
--- cfe/trunk/test/Sema/calling-conv-complete-params.c
+++ cfe/trunk/test/Sema/calling-conv-complete-params.c
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 -fsyntax-only -fms-extensions -verify -triple i686-pc-win32 %s
+// RUN: %clang_cc1 -fsyntax-only -fms-extensions -verify -triple x86_64-pc-win32 %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -fms-extensions -verify -triple i686-pc-win32 %s
+// RUN: %clang_cc1 -x c++ -DEXTERN_C='extern "C"' -fsyntax-only -fms-extensions -verify -triple i686-pc-win32 %s
+
+#ifndef EXTERN_C
+#define EXTERN_C
+#if defined(__cplusplus)
+#define EXPECT_NODIAG
+// expected-no-diagnostics
+#endif
+#endif
+
+#ifndef EXPECT_NODIAG
+// expected-note-re@+2 1+ {{forward declaration of '{{(struct )?}}Foo'}}
+#endif
+struct Foo;
+
+EXTERN_C void __stdcall fwd_std(struct Foo p);
+#if !defined(EXPECT_NODIAG) && defined(_M_IX86)
+// expected-error@+2 {{parameter 'p' must have a complete type to use function 'fwd_std' with the stdcall calling convention}}
+#endif
+void (__stdcall *fp_fwd_std)(struct Foo) = _std;
+
+EXTERN_C void __fastcall fwd_fast(struct Foo p);
+#if !defined(EXPECT_NODIAG) && defined(_M_IX86)
+// expected-error@+2 {{parameter 'p' must have a complete type to use function 'fwd_fast' with the fastcall calling convention}}
+#endif
+void (__fastcall *fp_fwd_fast)(struct Foo) = _fast;
+
+EXTERN_C void __vectorcall fwd_vector(struct Foo p);
+#if !defined(EXPECT_NODIAG)
+// expected-error@+2 {{parameter 'p' must have a complete type to use function 'fwd_vector' with the vectorcall calling convention}}
+#endif
+void (__vectorcall *fp_fwd_vector)(struct Foo) = _vector;
+
+#if defined(__cplusplus)
+template  struct TemplateWrapper {
+#ifndef EXPECT_NODIAG
+  // expected-error@+2 {{field has incomplete type 'Foo'}}
+#endif
+  T field;
+};
+
+EXTERN_C void __vectorcall tpl_ok(TemplateWrapper p);
+void(__vectorcall *fp_tpl_ok)(TemplateWrapper) = _ok;
+
+EXTERN_C void __vectorcall tpl_fast(TemplateWrapper p);
+#ifndef EXPECT_NODIAG
+// expected-note@+2 {{requested here}}
+#endif
+void(__vectorcall *fp_tpl_fast)(TemplateWrapper) = _fast;
+#endif
Index: cfe/trunk/lib/Sema/SemaExpr.cpp
===
--- cfe/trunk/lib/Sema/SemaExpr.cpp
+++ cfe/trunk/lib/Sema/SemaExpr.cpp
@@ -14793,6 +14793,81 @@
   llvm_unreachable("Invalid context");
 }
 
+/// Return true if this function has a calling convention that requires mangling
+/// in the size of the parameter pack.
+static bool funcHasParameterSizeMangling(Sema , FunctionDecl *FD) {
+  // These manglings don't do anything on non-Windows or non-x86 platforms, so
+  // we don't need parameter type sizes.
+  const llvm::Triple  = S.Context.getTargetInfo().getTriple();
+  if (!TT.isOSWindows() || (TT.getArch() != llvm::Triple::x86 &&
+TT.getArch() != llvm::Triple::x86_64))
+return false;
+
+  // If this is C++ and this isn't an extern "C" function, parameters do not
+  // need to be complete. In this case, C++ mangling will apply, which doesn't
+  // use the size of the parameters.
+  if (S.getLangOpts().CPlusPlus && !FD->isExternC())
+return false;
+
+  // Stdcall, fastcall, and vectorcall need this special treatment.
+  CallingConv CC = FD->getType()->castAs()->getCallConv();
+  switch (CC) {
+  case CC_X86StdCall:
+  case CC_X86FastCall:
+  case CC_X86VectorCall:
+return true;
+  default:
+break;
+  }
+  return false;
+}
+
+/// Require that all of the parameter types of function be complete. Normally,
+/// parameter types are only required to be complete 

r363000 - Require stdcall etc parameters to be complete on ODR use

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Mon Jun 10 15:53:12 2019
New Revision: 363000

URL: http://llvm.org/viewvc/llvm-project?rev=363000=rev
Log:
Require stdcall etc parameters to be complete on ODR use

Functions using stdcall, fastcall, or vectorcall with C linkage mangle
in the size of the parameter pack. Calculating the size of the pack
requires the parameter types to complete, which may require template
instantiation.

Previously, we would crash during IRgen when requesting the size of
incomplete or uninstantiated types, as in this reduced example:
  struct Foo;
  void __fastcall bar(struct Foo o);
  void (__fastcall *fp)(struct Foo) = 

Reported in Chromium here: https://crbug.com/971245

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

Added:
cfe/trunk/test/Sema/calling-conv-complete-params.c
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaExpr.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=363000=362999=363000=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Jun 10 15:53:12 
2019
@@ -2985,6 +2985,9 @@ def err_invalid_attribute_on_virtual_fun
 def warn_declspec_allocator_nonpointer : Warning<
   "ignoring __declspec(allocator) because the function return type %0 is not "
   "a pointer or reference type">, InGroup;
+def err_cconv_incomplete_param_type : Error<
+  "parameter %0 must have a complete type to use function %1 with the %2 "
+  "calling convention">;
 
 def ext_cannot_use_trivial_abi : ExtWarn<
   "'trivial_abi' cannot be applied to %0">, InGroup;

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=363000=362999=363000=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Mon Jun 10 15:53:12 2019
@@ -14793,6 +14793,81 @@ static bool isPotentiallyConstantEvaluat
   llvm_unreachable("Invalid context");
 }
 
+/// Return true if this function has a calling convention that requires 
mangling
+/// in the size of the parameter pack.
+static bool funcHasParameterSizeMangling(Sema , FunctionDecl *FD) {
+  // These manglings don't do anything on non-Windows or non-x86 platforms, so
+  // we don't need parameter type sizes.
+  const llvm::Triple  = S.Context.getTargetInfo().getTriple();
+  if (!TT.isOSWindows() || (TT.getArch() != llvm::Triple::x86 &&
+TT.getArch() != llvm::Triple::x86_64))
+return false;
+
+  // If this is C++ and this isn't an extern "C" function, parameters do not
+  // need to be complete. In this case, C++ mangling will apply, which doesn't
+  // use the size of the parameters.
+  if (S.getLangOpts().CPlusPlus && !FD->isExternC())
+return false;
+
+  // Stdcall, fastcall, and vectorcall need this special treatment.
+  CallingConv CC = FD->getType()->castAs()->getCallConv();
+  switch (CC) {
+  case CC_X86StdCall:
+  case CC_X86FastCall:
+  case CC_X86VectorCall:
+return true;
+  default:
+break;
+  }
+  return false;
+}
+
+/// Require that all of the parameter types of function be complete. Normally,
+/// parameter types are only required to be complete when a function is called
+/// or defined, but to mangle functions with certain calling conventions, the
+/// mangler needs to know the size of the parameter list. In this situation,
+/// MSVC doesn't emit an error or instantiate templates. Instead, MSVC mangles
+/// the function as _foo@0, i.e. zero bytes of parameters, which will usually
+/// result in a linker error. Clang doesn't implement this behavior, and 
instead
+/// attempts to error at compile time.
+static void CheckCompleteParameterTypesForMangler(Sema , FunctionDecl *FD,
+  SourceLocation Loc) {
+  class ParamIncompleteTypeDiagnoser : public Sema::TypeDiagnoser {
+FunctionDecl *FD;
+ParmVarDecl *Param;
+
+  public:
+ParamIncompleteTypeDiagnoser(FunctionDecl *FD, ParmVarDecl *Param)
+: FD(FD), Param(Param) {}
+
+void diagnose(Sema , SourceLocation Loc, QualType T) override {
+  CallingConv CC = FD->getType()->castAs()->getCallConv();
+  StringRef CCName;
+  switch (CC) {
+  case CC_X86StdCall:
+CCName = "stdcall";
+break;
+  case CC_X86FastCall:
+CCName = "fastcall";
+break;
+  case CC_X86VectorCall:
+CCName = "vectorcall";
+break;
+  default:
+llvm_unreachable("CC does not need mangling");
+  }
+
+  S.Diag(Loc, diag::err_cconv_incomplete_param_type)
+  << Param->getDeclName() << FD->getDeclName() << CCName;
+}
+  };
+
+  for (ParmVarDecl *Param : 

[PATCH] D62584: [OpenCL][PR42033] Deducing addr space with template parameter types

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Sema/TreeTransform.h:5363
+if (ResultType.getAddressSpace() != LangAS::Default &&
+(ResultType.getAddressSpace() != LangAS::opencl_private)) {
   SemaRef.Diag(TL.getReturnLoc().getBeginLoc(),

Anastasia wrote:
> I am trying to be a bit more helpful here although I am not sure if we should 
> instead require explicit template parameter and fail the template deduction 
> instead.
> 
> Basically, do we want the following code to always require specifying 
> template argument explicitly:
> 
> 
> ```
> template 
> T xxx(T *in) {
>   T *i = in;
>   return *i;
> }
> 
> __kernel void test() {
>   int foo[10];
>   xxx([0]); // if we deduce type from foo, it ends up being 
> qualified by __private that we currently diagnose. However private is default 
> (implicit) address space for return type so technically there is no danger in 
> just allowing xxx([0])
> }
> ```
Implicitly ignoring all address-space qualifiers on the return type seems like 
the right thing to do; I don't think it needs to be limited to `__private`.  
That's probably also the right thing to do for locals, but I'm less certain 
about it.


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

https://reviews.llvm.org/D62584



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

[PATCH] D62413: [OpenCL][PR41727] Prevent ICE on global dtors

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/CodeGen/CGDeclCXX.cpp:132
+  Argument = CGM.getTargetCodeGenInfo().performAddrSpaceCast(
+  CGM, Addr.getPointer(), SrcAS, LangAS::opencl_global, DestTy);
 

Anastasia wrote:
> rjmccall wrote:
> > Should this code be conditional to OpenCL?  And why does `_cxa_atexit` take 
> > a `__global` pointer instead of, say, a `__generic` one?
> The only objects that are destructible globally in OpenCL are `__global` and 
> `__constant`. However `__constant` isn't convertible to `__generic`. 
> Therefore, I am adding `__global` directly to avoid extra conversion. I am 
> not yet sure how to handle `__constant`though and how much destructing 
> objects in read-only memory segments would make sense anyway. I think I will 
> address this separately.
The pointer argument to `__cxa_atexit` is just an arbitrary bit of context and 
doesn't have to actually be the address of a global.  It's *convenient* to use 
the address of a global sometimes; e.g. you can use the global as the pointer 
and its destructor as the function, and then `__cxa_atexit` will just call the 
destructor for you without any additional code.  But as far as the runtime is 
concerned, the pointer could be `malloc`'ed or something; we've never had a 
need to do that in the ABI, but it's good future-proofing to allow it.

So there are three ways to get a global destructor to destroy a variable in 
`__constant`:
- You can pass the pointer bitcast'ed as long as `sizeof(__constant void*) <= 
sizeof(__cxa_atexit_context_pointer)`.
- You can ignore the argument and just materialize the address separately 
within the destructor function.
- You can allocate memory for a context and then store the pointer in that.

Obviously you should go with the one of the first two, but you should make sure 
your ABI doesn't preclude doing the latter in case it's useful for some future 
language feature.  In other words, it doesn't really matter whether this 
argument is notionally in `__global` as long as that's wide enough to pass a 
more-or-less arbitrary pointer through.


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

https://reviews.llvm.org/D62413



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

[PATCH] D62951: [analyzer][tests] Use normalize_plist in place of diff_plist (`tail` cases)

[Hubert Tong via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL362998: [analyzer][tests] Use normalize_plist in place of 
diff_plist (`tail` cases) (authored by hubert.reinterpretcast, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62951?vs=203708=203916#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62951

Files:
  cfe/trunk/test/Analysis/MismatchedDeallocator-path-notes.cpp
  cfe/trunk/test/Analysis/diagnostics/plist-diagnostics-include-check.cpp
  cfe/trunk/test/Analysis/diagnostics/plist-multi-file.c
  cfe/trunk/test/Analysis/lambda-notes.cpp
  cfe/trunk/test/Analysis/lit.local.cfg
  cfe/trunk/test/Analysis/malloc-plist.c


Index: cfe/trunk/test/Analysis/diagnostics/plist-multi-file.c
===
--- cfe/trunk/test/Analysis/diagnostics/plist-multi-file.c
+++ cfe/trunk/test/Analysis/diagnostics/plist-multi-file.c
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core 
-analyzer-output=plist-multi-file -o %t.plist -verify %s
-// RUN: tail -n +11 %t.plist | %diff_plist 
%S/Inputs/expected-plists/plist-multi-file.c.plist -
+// RUN: tail -n +11 %t.plist | %normalize_plist | diff -u 
%S/Inputs/expected-plists/plist-multi-file.c.plist -
 
 #include "plist-multi-file.h"
 
Index: cfe/trunk/test/Analysis/diagnostics/plist-diagnostics-include-check.cpp
===
--- cfe/trunk/test/Analysis/diagnostics/plist-diagnostics-include-check.cpp
+++ cfe/trunk/test/Analysis/diagnostics/plist-diagnostics-include-check.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection 
-analyzer-output=plist-multi-file %s -o %t.plist
-// RUN: tail -n +11 %t.plist | %diff_plist 
%S/Inputs/expected-plists/plist-diagnostics-include-check.cpp.plist -
+// RUN: tail -n +11 %t.plist | %normalize_plist | diff -u 
%S/Inputs/expected-plists/plist-diagnostics-include-check.cpp.plist -
 
 #include "Inputs/include/plist-diagnostics-include-check-macro.h"
 
Index: cfe/trunk/test/Analysis/malloc-plist.c
===
--- cfe/trunk/test/Analysis/malloc-plist.c
+++ cfe/trunk/test/Analysis/malloc-plist.c
@@ -1,6 +1,6 @@
 // RUN: rm -f %t
 // RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core,unix.Malloc 
-analyzer-output=plist -verify -o %t -analyzer-config eagerly-assume=false %s
-// RUN: tail -n +11 %t | %diff_plist 
%S/Inputs/expected-plists/malloc-plist.c.plist -
+// RUN: tail -n +11 %t | %normalize_plist | diff -u 
%S/Inputs/expected-plists/malloc-plist.c.plist -
 
 typedef __typeof(sizeof(int)) size_t;
 void *malloc(size_t);
Index: cfe/trunk/test/Analysis/lit.local.cfg
===
--- cfe/trunk/test/Analysis/lit.local.cfg
+++ cfe/trunk/test/Analysis/lit.local.cfg
@@ -9,11 +9,6 @@
 config.test_format = analyzer_test.AnalyzerTest(
 config.test_format.execute_external, config.use_z3_solver)
 
-# Diff command used by Clang Analyzer tests (when comparing .plist files
-# with reference output)
-config.substitutions.append(('%diff_plist',
-'diff -u -w -I "/" -I ".:" -I "version"'))
-
 # Filtering command used by Clang Analyzer tests (when comparing .plist files
 # with reference output)
 config.substitutions.append(('%normalize_plist',
Index: cfe/trunk/test/Analysis/lambda-notes.cpp
===
--- cfe/trunk/test/Analysis/lambda-notes.cpp
+++ cfe/trunk/test/Analysis/lambda-notes.cpp
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core.DivideZero 
-analyzer-config inline-lambdas=true -analyzer-output plist -verify %s -o %t
-// RUN: tail -n +11 %t | %diff_plist 
%S/Inputs/expected-plists/lambda-notes.cpp.plist -
+// RUN: tail -n +11 %t | %normalize_plist | diff -u 
%S/Inputs/expected-plists/lambda-notes.cpp.plist -
 
 
 // Diagnostic inside a lambda
Index: cfe/trunk/test/Analysis/MismatchedDeallocator-path-notes.cpp
===
--- cfe/trunk/test/Analysis/MismatchedDeallocator-path-notes.cpp
+++ cfe/trunk/test/Analysis/MismatchedDeallocator-path-notes.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.MismatchedDeallocator 
-analyzer-output=text -verify %s
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.MismatchedDeallocator 
-analyzer-output=plist %s -o %t.plist
-// RUN: tail -n +11 %t.plist | %diff_plist 
%S/copypaste/Inputs/expected-plists/MismatchedDeallocator-path-notes.cpp.plist -
+// RUN: tail -n +11 %t.plist | %normalize_plist | diff -u 
%S/copypaste/Inputs/expected-plists/MismatchedDeallocator-path-notes.cpp.plist -
 
 void 

r362998 - [analyzer][tests] Use normalize_plist in place of diff_plist (`tail` cases)

[Hubert Tong via cfe-commits]

Author: hubert.reinterpretcast
Date: Mon Jun 10 15:40:35 2019
New Revision: 362998

URL: http://llvm.org/viewvc/llvm-project?rev=362998=rev
Log:
[analyzer][tests] Use normalize_plist in place of diff_plist (`tail` cases)

Summary:
The `%diff_plist` lit substitution invokes `diff` with a non-portable
`-I` option. The intended effect can be achieved by normalizing the
inputs to `diff` beforehand. Such normalization can be done with
`grep -Ev`, which is also used by other tests.

This patch applies the change (adjusted for review comments) described
in http://lists.llvm.org/pipermail/cfe-dev/2019-April/061904.html
mechanically to the cases where the output file is piped to
`%diff_plist` via `tail`. `%diff_plist` is then, being unused, removed.

The changes were applied via a script.

Reviewers: NoQ, sfertile, xingxue, jasonliu, daltenty

Subscribers: xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, 
Szelethus, donat.nagy, dkrupp, Charusso, jsji, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/test/Analysis/MismatchedDeallocator-path-notes.cpp
cfe/trunk/test/Analysis/diagnostics/plist-diagnostics-include-check.cpp
cfe/trunk/test/Analysis/diagnostics/plist-multi-file.c
cfe/trunk/test/Analysis/lambda-notes.cpp
cfe/trunk/test/Analysis/lit.local.cfg
cfe/trunk/test/Analysis/malloc-plist.c

Modified: cfe/trunk/test/Analysis/MismatchedDeallocator-path-notes.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/MismatchedDeallocator-path-notes.cpp?rev=362998=362997=362998=diff
==
--- cfe/trunk/test/Analysis/MismatchedDeallocator-path-notes.cpp (original)
+++ cfe/trunk/test/Analysis/MismatchedDeallocator-path-notes.cpp Mon Jun 10 
15:40:35 2019
@@ -1,6 +1,6 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.MismatchedDeallocator 
-analyzer-output=text -verify %s
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.MismatchedDeallocator 
-analyzer-output=plist %s -o %t.plist
-// RUN: tail -n +11 %t.plist | %diff_plist 
%S/copypaste/Inputs/expected-plists/MismatchedDeallocator-path-notes.cpp.plist -
+// RUN: tail -n +11 %t.plist | %normalize_plist | diff -u 
%S/copypaste/Inputs/expected-plists/MismatchedDeallocator-path-notes.cpp.plist -
 
 void changePointee(int *p);
 int *allocIntArray(unsigned c) {

Modified: 
cfe/trunk/test/Analysis/diagnostics/plist-diagnostics-include-check.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/plist-diagnostics-include-check.cpp?rev=362998=362997=362998=diff
==
--- cfe/trunk/test/Analysis/diagnostics/plist-diagnostics-include-check.cpp 
(original)
+++ cfe/trunk/test/Analysis/diagnostics/plist-diagnostics-include-check.cpp Mon 
Jun 10 15:40:35 2019
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection 
-analyzer-output=plist-multi-file %s -o %t.plist
-// RUN: tail -n +11 %t.plist | %diff_plist 
%S/Inputs/expected-plists/plist-diagnostics-include-check.cpp.plist -
+// RUN: tail -n +11 %t.plist | %normalize_plist | diff -u 
%S/Inputs/expected-plists/plist-diagnostics-include-check.cpp.plist -
 
 #include "Inputs/include/plist-diagnostics-include-check-macro.h"
 

Modified: cfe/trunk/test/Analysis/diagnostics/plist-multi-file.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/plist-multi-file.c?rev=362998=362997=362998=diff
==
--- cfe/trunk/test/Analysis/diagnostics/plist-multi-file.c (original)
+++ cfe/trunk/test/Analysis/diagnostics/plist-multi-file.c Mon Jun 10 15:40:35 
2019
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core 
-analyzer-output=plist-multi-file -o %t.plist -verify %s
-// RUN: tail -n +11 %t.plist | %diff_plist 
%S/Inputs/expected-plists/plist-multi-file.c.plist -
+// RUN: tail -n +11 %t.plist | %normalize_plist | diff -u 
%S/Inputs/expected-plists/plist-multi-file.c.plist -
 
 #include "plist-multi-file.h"
 

Modified: cfe/trunk/test/Analysis/lambda-notes.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/lambda-notes.cpp?rev=362998=362997=362998=diff
==
--- cfe/trunk/test/Analysis/lambda-notes.cpp (original)
+++ cfe/trunk/test/Analysis/lambda-notes.cpp Mon Jun 10 15:40:35 2019
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core.DivideZero 
-analyzer-config inline-lambdas=true -analyzer-output plist -verify %s -o %t
-// RUN: tail -n +11 %t | %diff_plist 
%S/Inputs/expected-plists/lambda-notes.cpp.plist -
+// RUN: tail -n +11 %t | %normalize_plist | diff -u 
%S/Inputs/expected-plists/lambda-notes.cpp.plist -
 
 
 // Diagnostic inside a lambda

Modified: cfe/trunk/test/Analysis/lit.local.cfg
URL: 

[PATCH] D62950: [analyzer][tests] Use normalize_plist in place of diff_plist (`cat` cases)

[Hubert Tong via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL362996: [analyzer][tests] Use normalize_plist in place of 
diff_plist (`cat` cases) (authored by hubert.reinterpretcast, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62950

Files:
  cfe/trunk/test/Analysis/NewDelete-path-notes.cpp
  cfe/trunk/test/Analysis/conditional-path-notes.c
  cfe/trunk/test/Analysis/copypaste/plist-diagnostics-notes-as-events.cpp
  cfe/trunk/test/Analysis/copypaste/plist-diagnostics.cpp
  cfe/trunk/test/Analysis/cxx-for-range.cpp
  cfe/trunk/test/Analysis/diagnostics/deref-track-symbolic-region.c
  cfe/trunk/test/Analysis/diagnostics/report-issues-within-main-file.cpp
  cfe/trunk/test/Analysis/diagnostics/undef-value-caller.c
  cfe/trunk/test/Analysis/diagnostics/undef-value-param.c
  cfe/trunk/test/Analysis/diagnostics/undef-value-param.m
  cfe/trunk/test/Analysis/edges-new.mm
  cfe/trunk/test/Analysis/generics.m
  cfe/trunk/test/Analysis/inline-plist.c
  cfe/trunk/test/Analysis/inline-unique-reports.c
  cfe/trunk/test/Analysis/inlining/eager-reclamation-path-notes.c
  cfe/trunk/test/Analysis/inlining/eager-reclamation-path-notes.cpp
  cfe/trunk/test/Analysis/inlining/path-notes.c
  cfe/trunk/test/Analysis/inlining/path-notes.cpp
  cfe/trunk/test/Analysis/inlining/path-notes.m
  cfe/trunk/test/Analysis/method-call-path-notes.cpp
  cfe/trunk/test/Analysis/model-file.cpp
  cfe/trunk/test/Analysis/null-deref-path-notes.m
  cfe/trunk/test/Analysis/nullability-notes.m
  cfe/trunk/test/Analysis/objc-arc.m
  cfe/trunk/test/Analysis/objc-radar17039661.m
  cfe/trunk/test/Analysis/plist-macros-with-expansion.cpp
  cfe/trunk/test/Analysis/plist-macros.cpp
  cfe/trunk/test/Analysis/plist-output-alternate.m
  cfe/trunk/test/Analysis/plist-output.m
  cfe/trunk/test/Analysis/retain-release-path-notes.m
  cfe/trunk/test/Analysis/retain-release.m

Index: cfe/trunk/test/Analysis/retain-release-path-notes.m
===
--- cfe/trunk/test/Analysis/retain-release-path-notes.m
+++ cfe/trunk/test/Analysis/retain-release-path-notes.m
@@ -1,6 +1,6 @@
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,osx.coreFoundation.CFRetainRelease,osx.cocoa.ClassRelease,osx.cocoa.RetainCount -analyzer-store=region -analyzer-output=text -verify %s
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,osx.coreFoundation.CFRetainRelease,osx.cocoa.ClassRelease,osx.cocoa.RetainCount -analyzer-store=region -analyzer-output=plist-multi-file %s -o %t
-// RUN: cat %t | %diff_plist %S/Inputs/expected-plists/retain-release-path-notes.m.plist -
+// RUN: %normalize_plist <%t | diff -u %S/Inputs/expected-plists/retain-release-path-notes.m.plist -
 
 /***
 This file is for testing the path-sensitive notes for retain/release errors.
Index: cfe/trunk/test/Analysis/null-deref-path-notes.m
===
--- cfe/trunk/test/Analysis/null-deref-path-notes.m
+++ cfe/trunk/test/Analysis/null-deref-path-notes.m
@@ -1,6 +1,6 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-store=region -analyzer-output=text -fblocks -verify -Wno-objc-root-class %s
 // RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-store=region -analyzer-output=plist-multi-file -fblocks -Wno-objc-root-class %s -o %t
-// RUN: cat %t | %diff_plist %S/Inputs/expected-plists/null-deref-path-notes.m.plist -
+// RUN: %normalize_plist <%t | diff -u %S/Inputs/expected-plists/null-deref-path-notes.m.plist -
 
 @interface Root {
 @public
Index: cfe/trunk/test/Analysis/conditional-path-notes.c
===
--- cfe/trunk/test/Analysis/conditional-path-notes.c
+++ cfe/trunk/test/Analysis/conditional-path-notes.c
@@ -1,6 +1,6 @@
 // RUN: %clang_analyze_cc1 %s -analyzer-checker=core.NullDereference -analyzer-output=text -verify
 // RUN: %clang_analyze_cc1 %s -analyzer-checker=core.NullDereference -analyzer-output=plist -o %t
-// RUN: cat %t | %diff_plist %S/Inputs/expected-plists/conditional-path-notes.c.plist -
+// RUN: %normalize_plist <%t | diff -u %S/Inputs/expected-plists/conditional-path-notes.c.plist -
 
 void testCondOp(int *p) {
   int *x = p ? p : p;
Index: cfe/trunk/test/Analysis/plist-output.m
===
--- cfe/trunk/test/Analysis/plist-output.m
+++ cfe/trunk/test/Analysis/plist-output.m
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -analyzer-config eagerly-assume=false %s -analyzer-checker=osx.cocoa.RetainCount,deadcode.DeadStores,core -analyzer-output=plist -o %t.plist
-// RUN: cat %t.plist | %diff_plist %S/Inputs/expected-plists/plist-output.m.plist -

[PATCH] D63101: [Frontend] SetUpDiagnosticLog should handle unowned diagnostic consumer in the compiler

[Michael Spencer via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D63101



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

r362996 - [analyzer][tests] Use normalize_plist in place of diff_plist (`cat` cases)

[Hubert Tong via cfe-commits]

Author: hubert.reinterpretcast
Date: Mon Jun 10 15:37:31 2019
New Revision: 362996

URL: http://llvm.org/viewvc/llvm-project?rev=362996=rev
Log:
[analyzer][tests] Use normalize_plist in place of diff_plist (`cat` cases)

Summary:
The `%diff_plist` lit substitution invokes `diff` with a non-portable
`-I` option. The intended effect can be achieved by normalizing the
inputs to `diff` beforehand. Such normalization can be done with
`grep -Ev`, which is also used by other tests.

This patch applies the change (adjusted for review comments) described
in http://lists.llvm.org/pipermail/cfe-dev/2019-April/061904.html
mechanically to the cases where the output file is piped to
`%diff_plist` via `cat`.

The changes were applied via a script, except that
`clang/test/Analysis/NewDelete-path-notes.cpp` and
`clang/test/Analysis/plist-macros-with-expansion.cpp` were each adjusted
for the line-continuation on the relevant `RUN` step.

Reviewers: NoQ, sfertile, xingxue, jasonliu, daltenty

Subscribers: xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, 
Szelethus, donat.nagy, dkrupp, Charusso, jsji, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/test/Analysis/NewDelete-path-notes.cpp
cfe/trunk/test/Analysis/conditional-path-notes.c
cfe/trunk/test/Analysis/copypaste/plist-diagnostics-notes-as-events.cpp
cfe/trunk/test/Analysis/copypaste/plist-diagnostics.cpp
cfe/trunk/test/Analysis/cxx-for-range.cpp
cfe/trunk/test/Analysis/diagnostics/deref-track-symbolic-region.c
cfe/trunk/test/Analysis/diagnostics/report-issues-within-main-file.cpp
cfe/trunk/test/Analysis/diagnostics/undef-value-caller.c
cfe/trunk/test/Analysis/diagnostics/undef-value-param.c
cfe/trunk/test/Analysis/diagnostics/undef-value-param.m
cfe/trunk/test/Analysis/edges-new.mm
cfe/trunk/test/Analysis/generics.m
cfe/trunk/test/Analysis/inline-plist.c
cfe/trunk/test/Analysis/inline-unique-reports.c
cfe/trunk/test/Analysis/inlining/eager-reclamation-path-notes.c
cfe/trunk/test/Analysis/inlining/eager-reclamation-path-notes.cpp
cfe/trunk/test/Analysis/inlining/path-notes.c
cfe/trunk/test/Analysis/inlining/path-notes.cpp
cfe/trunk/test/Analysis/inlining/path-notes.m
cfe/trunk/test/Analysis/method-call-path-notes.cpp
cfe/trunk/test/Analysis/model-file.cpp
cfe/trunk/test/Analysis/null-deref-path-notes.m
cfe/trunk/test/Analysis/nullability-notes.m
cfe/trunk/test/Analysis/objc-arc.m
cfe/trunk/test/Analysis/objc-radar17039661.m
cfe/trunk/test/Analysis/plist-macros-with-expansion.cpp
cfe/trunk/test/Analysis/plist-macros.cpp
cfe/trunk/test/Analysis/plist-output-alternate.m
cfe/trunk/test/Analysis/plist-output.m
cfe/trunk/test/Analysis/retain-release-path-notes.m
cfe/trunk/test/Analysis/retain-release.m

Modified: cfe/trunk/test/Analysis/NewDelete-path-notes.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/NewDelete-path-notes.cpp?rev=362996=362995=362996=diff
==
--- cfe/trunk/test/Analysis/NewDelete-path-notes.cpp (original)
+++ cfe/trunk/test/Analysis/NewDelete-path-notes.cpp Mon Jun 10 15:37:31 2019
@@ -11,7 +11,7 @@
 // RUN:   -analyzer-checker=cplusplus.NewDelete,unix.Malloc \
 // RUN:   -analyzer-config add-pop-up-notes=false \
 // RUN:   -analyzer-output=plist %s -o %t.plist
-// RUN: cat %t.plist | %diff_plist \
+// RUN: %normalize_plist <%t.plist | diff -u \
 // RUN:   %S/Inputs/expected-plists/NewDelete-path-notes.cpp.plist -
 
 void test() {

Modified: cfe/trunk/test/Analysis/conditional-path-notes.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/conditional-path-notes.c?rev=362996=362995=362996=diff
==
--- cfe/trunk/test/Analysis/conditional-path-notes.c (original)
+++ cfe/trunk/test/Analysis/conditional-path-notes.c Mon Jun 10 15:37:31 2019
@@ -1,6 +1,6 @@
 // RUN: %clang_analyze_cc1 %s -analyzer-checker=core.NullDereference 
-analyzer-output=text -verify
 // RUN: %clang_analyze_cc1 %s -analyzer-checker=core.NullDereference 
-analyzer-output=plist -o %t
-// RUN: cat %t | %diff_plist 
%S/Inputs/expected-plists/conditional-path-notes.c.plist -
+// RUN: %normalize_plist <%t | diff -u 
%S/Inputs/expected-plists/conditional-path-notes.c.plist -
 
 void testCondOp(int *p) {
   int *x = p ? p : p;

Modified: 
cfe/trunk/test/Analysis/copypaste/plist-diagnostics-notes-as-events.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/copypaste/plist-diagnostics-notes-as-events.cpp?rev=362996=362995=362996=diff
==
--- cfe/trunk/test/Analysis/copypaste/plist-diagnostics-notes-as-events.cpp 
(original)
+++ cfe/trunk/test/Analysis/copypaste/plist-diagnostics-notes-as-events.cpp Mon 
Jun 10 15:37:31 2019
@@ 

[PATCH] D62949: [analyzer][tests] Add normalize_plist to replace diff_plist

[Hubert Tong via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL362994: [analyzer][tests] Add normalize_plist to replace 
diff_plist (authored by hubert.reinterpretcast, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62949

Files:
  cfe/trunk/test/Analysis/lit.local.cfg
  cfe/trunk/test/Analysis/unix-fns.c


Index: cfe/trunk/test/Analysis/unix-fns.c
===
--- cfe/trunk/test/Analysis/unix-fns.c
+++ cfe/trunk/test/Analysis/unix-fns.c
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 
-analyzer-checker=core,unix.API,osx.API,optin.portability %s 
-analyzer-store=region -analyzer-output=plist -analyzer-config faux-bodies=true 
 -fblocks -verify -o %t.plist
-// RUN: cat %t.plist | %diff_plist %S/Inputs/expected-plists/unix-fns.c.plist -
+// RUN: %normalize_plist <%t.plist | diff -u 
%S/Inputs/expected-plists/unix-fns.c.plist -
 // RUN: mkdir -p %t.dir
 // RUN: %clang_analyze_cc1 
-analyzer-checker=core,unix.API,osx.API,optin.portability -analyzer-output=html 
-analyzer-config faux-bodies=true -fblocks -o %t.dir %s
 // RUN: rm -fR %t.dir
Index: cfe/trunk/test/Analysis/lit.local.cfg
===
--- cfe/trunk/test/Analysis/lit.local.cfg
+++ cfe/trunk/test/Analysis/lit.local.cfg
@@ -14,6 +14,14 @@
 config.substitutions.append(('%diff_plist',
 'diff -u -w -I "/" -I ".:" -I "version"'))
 
+# Filtering command used by Clang Analyzer tests (when comparing .plist files
+# with reference output)
+config.substitutions.append(('%normalize_plist',
+"grep -Ev '%s|%s|%s'" %
+('^[[:space:]]*.* version .*$',
+ '^[[:space:]]*/.*$',
+ '^[[:space:]]*.:.*$')))
+
 # Diff command for testing SARIF output to reference output.
 config.substitutions.append(('%diff_sarif',
 '''diff -U1 -w -I ".*file:.*%basename_t" -I '"version":' -I 
"2\.0\.0\-csd\.[0-9]*\.beta\."'''))


Index: cfe/trunk/test/Analysis/unix-fns.c
===
--- cfe/trunk/test/Analysis/unix-fns.c
+++ cfe/trunk/test/Analysis/unix-fns.c
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,unix.API,osx.API,optin.portability %s -analyzer-store=region -analyzer-output=plist -analyzer-config faux-bodies=true  -fblocks -verify -o %t.plist
-// RUN: cat %t.plist | %diff_plist %S/Inputs/expected-plists/unix-fns.c.plist -
+// RUN: %normalize_plist <%t.plist | diff -u %S/Inputs/expected-plists/unix-fns.c.plist -
 // RUN: mkdir -p %t.dir
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.API,osx.API,optin.portability -analyzer-output=html -analyzer-config faux-bodies=true -fblocks -o %t.dir %s
 // RUN: rm -fR %t.dir
Index: cfe/trunk/test/Analysis/lit.local.cfg
===
--- cfe/trunk/test/Analysis/lit.local.cfg
+++ cfe/trunk/test/Analysis/lit.local.cfg
@@ -14,6 +14,14 @@
 config.substitutions.append(('%diff_plist',
 'diff -u -w -I "/" -I ".:" -I "version"'))
 
+# Filtering command used by Clang Analyzer tests (when comparing .plist files
+# with reference output)
+config.substitutions.append(('%normalize_plist',
+"grep -Ev '%s|%s|%s'" %
+('^[[:space:]]*.* version .*$',
+ '^[[:space:]]*/.*$',
+ '^[[:space:]]*.:.*$')))
+
 # Diff command for testing SARIF output to reference output.
 config.substitutions.append(('%diff_sarif',
 '''diff -U1 -w -I ".*file:.*%basename_t" -I '"version":' -I "2\.0\.0\-csd\.[0-9]*\.beta\."'''))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63101: [Frontend] SetUpDiagnosticLog should handle unowned diagnostic consumer in the compiler

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a subscriber: cfe-commits.
arphaman added a comment.

+ cue-commits


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

https://reviews.llvm.org/D63101



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

r362994 - [analyzer][tests] Add normalize_plist to replace diff_plist

[Hubert Tong via cfe-commits]

Author: hubert.reinterpretcast
Date: Mon Jun 10 15:33:34 2019
New Revision: 362994

URL: http://llvm.org/viewvc/llvm-project?rev=362994=rev
Log:
[analyzer][tests] Add normalize_plist to replace diff_plist

Summary:
The `%diff_plist` lit substitution invokes `diff` with a non-portable
`-I` option. The intended effect can be achieved by normalizing the
inputs to `diff` beforehand. Such normalization can be done with
`grep -Ev`, which is also used by other tests.

This patch applies the change (adjusted for review comments) described
in http://lists.llvm.org/pipermail/cfe-dev/2019-April/061904.html to the
specific case shown in the list message. Mechanical changes to the other
affected files will follow in later patches.

Reviewers: NoQ, sfertile, xingxue, jasonliu, daltenty

Reviewed By: NoQ

Subscribers: xazax.hun, baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho, 
Szelethus, donat.nagy, dkrupp, Charusso, jsji, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/test/Analysis/lit.local.cfg
cfe/trunk/test/Analysis/unix-fns.c

Modified: cfe/trunk/test/Analysis/lit.local.cfg
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/lit.local.cfg?rev=362994=362993=362994=diff
==
--- cfe/trunk/test/Analysis/lit.local.cfg (original)
+++ cfe/trunk/test/Analysis/lit.local.cfg Mon Jun 10 15:33:34 2019
@@ -14,6 +14,14 @@ config.test_format = analyzer_test.Analy
 config.substitutions.append(('%diff_plist',
 'diff -u -w -I "/" -I ".:" -I "version"'))
 
+# Filtering command used by Clang Analyzer tests (when comparing .plist files
+# with reference output)
+config.substitutions.append(('%normalize_plist',
+"grep -Ev '%s|%s|%s'" %
+('^[[:space:]]*.* version .*$',
+ '^[[:space:]]*/.*$',
+ '^[[:space:]]*.:.*$')))
+
 # Diff command for testing SARIF output to reference output.
 config.substitutions.append(('%diff_sarif',
 '''diff -U1 -w -I ".*file:.*%basename_t" -I '"version":' -I 
"2\.0\.0\-csd\.[0-9]*\.beta\."'''))

Modified: cfe/trunk/test/Analysis/unix-fns.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/unix-fns.c?rev=362994=362993=362994=diff
==
--- cfe/trunk/test/Analysis/unix-fns.c (original)
+++ cfe/trunk/test/Analysis/unix-fns.c Mon Jun 10 15:33:34 2019
@@ -1,5 +1,5 @@
 // RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 
-analyzer-checker=core,unix.API,osx.API,optin.portability %s 
-analyzer-store=region -analyzer-output=plist -analyzer-config faux-bodies=true 
 -fblocks -verify -o %t.plist
-// RUN: cat %t.plist | %diff_plist %S/Inputs/expected-plists/unix-fns.c.plist -
+// RUN: %normalize_plist <%t.plist | diff -u 
%S/Inputs/expected-plists/unix-fns.c.plist -
 // RUN: mkdir -p %t.dir
 // RUN: %clang_analyze_cc1 
-analyzer-checker=core,unix.API,osx.API,optin.portability -analyzer-output=html 
-analyzer-config faux-bodies=true -fblocks -o %t.dir %s
 // RUN: rm -fR %t.dir


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

[PATCH] D63041: [PlistSupport] Produce a newline to end plist output files

[Hubert Tong via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL362992: [PlistSupport] Produce a newline to end plist output 
files (authored by hubert.reinterpretcast, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63041

Files:
  cfe/trunk/lib/ARCMigrate/PlistReporter.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp


Index: cfe/trunk/lib/ARCMigrate/PlistReporter.cpp
===
--- cfe/trunk/lib/ARCMigrate/PlistReporter.cpp
+++ cfe/trunk/lib/ARCMigrate/PlistReporter.cpp
@@ -120,5 +120,5 @@
   o << " \n";
 
   // Finish.
-  o << "\n";
+  o << "\n\n";
 }
Index: cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -748,7 +748,7 @@
   }
 
   // Finish.
-  o << "\n";
+  o << "\n\n";
 }
 
 
//===--===//


Index: cfe/trunk/lib/ARCMigrate/PlistReporter.cpp
===
--- cfe/trunk/lib/ARCMigrate/PlistReporter.cpp
+++ cfe/trunk/lib/ARCMigrate/PlistReporter.cpp
@@ -120,5 +120,5 @@
   o << " \n";
 
   // Finish.
-  o << "\n";
+  o << "\n\n";
 }
Index: cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -748,7 +748,7 @@
   }
 
   // Finish.
-  o << "\n";
+  o << "\n\n";
 }
 
 //===--===//
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r362992 - [PlistSupport] Produce a newline to end plist output files

[Hubert Tong via cfe-commits]

Author: hubert.reinterpretcast
Date: Mon Jun 10 15:30:57 2019
New Revision: 362992

URL: http://llvm.org/viewvc/llvm-project?rev=362992=rev
Log:
[PlistSupport] Produce a newline to end plist output files

Summary:
As suggested in the review of D62949, this patch updates the plist
output to have a newline at the end of the file. This makes it so that
the plist output file qualifies as a POSIX text file, which increases
the consumability of the generated plist file in relation to various
tools.

Reviewers: NoQ, sfertile, xingxue, jasonliu, daltenty

Reviewed By: NoQ, xingxue

Subscribers: jsji, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/ARCMigrate/PlistReporter.cpp
cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp

Modified: cfe/trunk/lib/ARCMigrate/PlistReporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ARCMigrate/PlistReporter.cpp?rev=362992=362991=362992=diff
==
--- cfe/trunk/lib/ARCMigrate/PlistReporter.cpp (original)
+++ cfe/trunk/lib/ARCMigrate/PlistReporter.cpp Mon Jun 10 15:30:57 2019
@@ -120,5 +120,5 @@ void arcmt::writeARCDiagsToPlist(const s
   o << " \n";
 
   // Finish.
-  o << "\n";
+  o << "\n\n";
 }

Modified: cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp?rev=362992=362991=362992=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp Mon Jun 10 15:30:57 
2019
@@ -748,7 +748,7 @@ void PlistDiagnostics::FlushDiagnosticsI
   }
 
   // Finish.
-  o << "\n";
+  o << "\n\n";
 }
 
 
//===--===//


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

[PATCH] D44865: [libc++] Implement P0608R3 - A sane variant converting constructor

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

The patch looks fine to me, but I think you should consider making a couple of 
`T.fail.cpp` tests, and check to make sure you get the "right error".




Comment at: 
test/std/utilities/variant/variant.variant/variant.assign/T.pass.cpp:130
   {
 using V = std::variant;
 static_assert(!std::is_assignable::value, "ambiguous");

If you really want to check that these are "ambiguous" , or "no matching 
operator=", etc, the way to do that is to define a fail.cpp test, and check the 
error messages.
 


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D44865



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

[PATCH] D63029: [CUDA] Fix grep pattern in cuda-types.cu

[Hubert Tong via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
hubert.reinterpretcast marked an inline comment as done.
Closed by commit rL362991: [CUDA] Fix grep pattern in cuda-types.cu (authored 
by hubert.reinterpretcast, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63029

Files:
  cfe/trunk/test/Preprocessor/cuda-types.cu


Index: cfe/trunk/test/Preprocessor/cuda-types.cu
===
--- cfe/trunk/test/Preprocessor/cuda-types.cu
+++ cfe/trunk/test/Preprocessor/cuda-types.cu
@@ -8,41 +8,41 @@
 // RUN: mkdir -p %t
 
 // RUN: %clang --cuda-host-only -nocudainc -target i386-unknown-linux-gnu -x 
cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/i386-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/i386-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
i386-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/i386-device-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/i386-device-defines-filtered
 // RUN: diff %t/i386-host-defines-filtered %t/i386-device-defines-filtered
 
 // RUN: %clang --cuda-host-only -nocudainc -target x86_64-unknown-linux-gnu -x 
cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/x86_64-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/x86_64-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
x86_64-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/x86_64-device-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/x86_64-device-defines-filtered
 // RUN: diff %t/x86_64-host-defines-filtered %t/x86_64-device-defines-filtered
 
 // RUN: %clang --cuda-host-only -nocudainc -target powerpc64-unknown-linux-gnu 
-x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/powerpc64-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/powerpc64-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
powerpc64-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > 
%t/powerpc64-device-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > 
%t/powerpc64-device-defines-filtered
 // RUN: diff %t/powerpc64-host-defines-filtered 
%t/powerpc64-device-defines-filtered
 
 // RUN: %clang --cuda-host-only -nocudainc -target i386-windows-msvc -x cuda 
-E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/i386-msvc-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/i386-msvc-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
i386-windows-msvc -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > 
%t/i386-msvc-device-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > 
%t/i386-msvc-device-defines-filtered
 // RUN: diff %t/i386-msvc-host-defines-filtered 
%t/i386-msvc-device-defines-filtered
 
 // RUN: %clang --cuda-host-only -nocudainc -target x86_64-windows-msvc -x cuda 
-E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > 
%t/x86_64-msvc-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 

r362991 - [CUDA] Fix grep pattern in cuda-types.cu

[Hubert Tong via cfe-commits]

Author: hubert.reinterpretcast
Date: Mon Jun 10 15:28:20 2019
New Revision: 362991

URL: http://llvm.org/viewvc/llvm-project?rev=362991=rev
Log:
[CUDA] Fix grep pattern in cuda-types.cu

Summary:
vertical-line is not a BRE special character.

POSIX.1-2017 XBD Section 9.3.2 indicates that the interpretation of `\|`
is undefined. This patch uses EREs instead.

Additionally, the pattern is further fixed so that `SIZEOF` and `WIDTH`
macros are checked.

Reviewers: jlebar, daltenty, xingxue, jasonliu, tra

Reviewed By: tra

Subscribers: jfb, jsji, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/test/Preprocessor/cuda-types.cu

Modified: cfe/trunk/test/Preprocessor/cuda-types.cu
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/cuda-types.cu?rev=362991=362990=362991=diff
==
--- cfe/trunk/test/Preprocessor/cuda-types.cu (original)
+++ cfe/trunk/test/Preprocessor/cuda-types.cu Mon Jun 10 15:28:20 2019
@@ -8,41 +8,41 @@
 // RUN: mkdir -p %t
 
 // RUN: %clang --cuda-host-only -nocudainc -target i386-unknown-linux-gnu -x 
cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/i386-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/i386-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
i386-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/i386-device-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/i386-device-defines-filtered
 // RUN: diff %t/i386-host-defines-filtered %t/i386-device-defines-filtered
 
 // RUN: %clang --cuda-host-only -nocudainc -target x86_64-unknown-linux-gnu -x 
cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/x86_64-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/x86_64-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
x86_64-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/x86_64-device-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/x86_64-device-defines-filtered
 // RUN: diff %t/x86_64-host-defines-filtered %t/x86_64-device-defines-filtered
 
 // RUN: %clang --cuda-host-only -nocudainc -target powerpc64-unknown-linux-gnu 
-x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/powerpc64-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/powerpc64-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
powerpc64-unknown-linux-gnu -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > 
%t/powerpc64-device-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > 
%t/powerpc64-device-defines-filtered
 // RUN: diff %t/powerpc64-host-defines-filtered 
%t/powerpc64-device-defines-filtered
 
 // RUN: %clang --cuda-host-only -nocudainc -target i386-windows-msvc -x cuda 
-E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > %t/i386-msvc-host-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > %t/i386-msvc-host-defines-filtered
 // RUN: %clang --cuda-device-only -nocudainc -nocudalib -target 
i386-windows-msvc -x cuda -E -dM -o - /dev/null \
-// RUN:   | grep 'define __[^ ]*\(TYPE\|MAX\|SIZEOF|WIDTH\)\|define 
__GCC_ATOMIC' \
-// RUN:   | grep -v '__LDBL\|_LONG_DOUBLE' > 
%t/i386-msvc-device-defines-filtered
+// RUN:   | grep -E 'define __[^ ]*(TYPE|MAX|SIZEOF|WIDTH)|define 
__GCC_ATOMIC' \
+// RUN:   | grep -Ev '__LDBL|_LONG_DOUBLE' > 
%t/i386-msvc-device-defines-filtered
 // RUN: diff %t/i386-msvc-host-defines-filtered 

[PATCH] D62156: [Sema][PR41730] Diagnose addr space mismatch while constructing objects

[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, LGTM.




Comment at: test/SemaCXX/address-space-ctor.cpp:11
+//expected-note@-6{{candidate constructor (the implicit move constructor) not 
viable: no known conversion from 'int' to 'MyType &&' for 1st argument}}
+//expected-note@-6{{candidate constructor ignored: cannot be used to construct 
an object in address space '__attribute__((address_space(10)))'}}
+//expected-note@-8{{candidate constructor ignored: cannot be used to construct 
an object in address space '__attribute__((address_space(10)))'}}

Anastasia wrote:
> Not sure if we should change to:
>   cannot be used to construct an object with 
> '__attribute__((address_space(10)))'
> 
> Although for OpenCL it would be ok as is.
> 
> Or may be it's better to add custom printing of addr spaces?
Some sort of custom printing of address spaces would probably be best, yeah.  
Users probably think of them as whatever custom syntax/macro they normally 
write, not as an application of some weird attribute.  We do a similar sort of 
thing with vector types where we generally just print them as `float4` or 
whatever without an `aka` clause because we don't expect anyone to know about 
those attributes.


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

https://reviews.llvm.org/D62156



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

[PATCH] D63092: [Frontend] Use executable path when creating invocation from cmdline

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

One more thing, do you think it's reasonable to use 
`llvm::sys::findProgramByName(Args[0])`instead of `Args[0]` when creating the 
driver instance? One of the failure modes I ran into is the case where the 
generated compilation database would contain just the executable name, e.g. 
`clang++`. If you invoke that command, everything works as expected because 
driver resolves the binary to a full path 
,
 but when used with `clangd`, it'll fail because that resolution will never 
happen, the `Dir`/`InstalledDir` will be an empty path and attempt to resolve 
C++ library headers will end up with `/../include/c++/v1` which is invalid.

Alternative would be to make the compilation database specification even more 
strict and require that the compiler command is not only the first argument, 
but it's also a (full or relative) path to the compiler executable.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63092



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

[PATCH] D60233: [clang-scan-deps] initial outline of the tool that runs preprocessor to find dependencies over a JSON compilation database

[Alexandre Ganea via Phabricator via cfe-commits]

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



Comment at: clang/test/ClangScanDeps/Inputs/regular_cdb.json:4
+  "directory": "DIR",
+  "command": "clang -c DIR/regular_cdb.cpp -IInputs -MD -MF DIR/regular_cdb.d",
+  "file": "DIR/regular_cdb.cpp"

arphaman wrote:
> aganea wrote:
> > Is `-MD -MF` required for clang-scan-deps to work? Can't we append those 
> > arguments automatically, so that only include paths are needed in the CDB?
> > 
> > Would it possible for the tool to produce a collated file? A real-world 
> > usage would otherwise produce tens of thousands of files. Which then would 
> > be opened/parsed by the calling application.
> I agree, the tool shouldn't rely on those options being there. `-MD -MF` in 
> this test right now a crutch to get the dependencies printed somewhere.
> 
> I think the tool by default should print out the collated dependencies to 
> STDOUT instead of writing them to files that were specified for the build. 
> The `-MD -MF` options will not be required as well. This implementation 
> requires some changes in the dependency collector, which I am planning to do 
> as part of clang-scan-deps in the follow-up patches. Will it be ok to keep 
> this patch as it is right now, and transition to printing out the collated 
> dependencies without requiring the options as a follow-up?
> 
> 
Sounds good! Please cc me on the upcoming reviews, we have a good use-case for 
clang-scan-deps in [[ http://www.fastbuild.org/docs/home.html | Fastbuild ]]!



Comment at: clang/tools/clang-scan-deps/CMakeLists.txt:3
+Core
+Support
+)

Two space indent.


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

https://reviews.llvm.org/D60233



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

[PATCH] D44865: [libc++] Implement P0608R3 - A sane variant converting constructor

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

In D44865#1051228 , @EricWF wrote:

> This LGTM.
>
> Also I would like @mclow.lists input about applying this DR early since LWG
>  hasn't commented on it yet.


Even though LWG didn't vote it out as a DR; that's what it is, and I think that 
we would be doing our users a disservice by not providing this in C++17.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D44865



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

[PATCH] D63092: [Frontend] Use executable path when creating invocation from cmdline

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

In D63092#1536774 , @sammccall wrote:

> `argv[0]` does carry important information though, I think this will break a 
> lot of things. It's... concerning that no tests broke.
>
> For example, if it's `clang` or `g++` or `clang-cl` then that affects how 
> command lines are parsed (regardless of whether the path actually exists).


You're right, I can see how this change can break the driver mode detection.

> Additionally I don't think all tools that call 
> createInvocationFromCommandLine carry around all the files you want.
> 
> The wrapper tool case is a problem though, I see some options:
> 
> - change the process that generates the compilation database to emit a path 
> to the underlying compiler instead, as tooling currently expects. This may be 
> painful if e.g. goma pretends to be a regular toolchain. An argument can be 
> made it should pretend harder (e.g. symlinking directories around it so 
> discovery works as usual)

I think this is the right solution for compiler wrappers like goma, the problem 
is that this requires changing any tool that generates compilation database to 
have explicit notion of compiler launcher and exclude that from the command 
(today we just prepend the tool to the command 
).
 This includes tool like GN and potentially even Ninja which is a non-trivial 
task. Until that happens, `clangd` is unusable for projects like Fuchsia 
(`clangd` cannot find standard headers and we're getting too many warnings for 
those).

> - change the process that generates the CDB to inject the relevant directory 
> paths explicitly as flags

This would mean either duplicating the Clang driver logic inside the build 
system, which is something I'd like to avoid (why use the driver at all in that 
case?), or have some mechanism through which the driver would have to export 
all the necessary information (e.g. where to find standard library headers) for 
the build system to use.

> - make driver aware of the distinction between "invoked-as" and "current 
> binary" and use the right path in the right places

I'll look into this.

> (Incidentally if Driver is going to use the current binary, we shouldn't need 
> to pass it in as a parameter, right?)
> 
> There are some potentially-unsolvable cases here: e.g. if your wrapper is 
> `magic-remote-build` then maybe there's no standard library locally, or no 
> way to find it.

Do you think it's worth updating the compilation database documentation 
 to explicitly say 
that the command has to start with the compiler invocation? Currently it says 
that "The compile command executed." which is IMHO not sufficiently precise.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63092



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

[PATCH] D62883: [analyzer] Track conditions of terminator statements on which the reported node depends on

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

Szelethus added a comment.

I implemented the fixes you commented on, but during evaluation it turned out 
that my visitor eats ram for breakfast, and then goes for seconds. I mean, like 
5-30x the normal memory consumption, and the same for analysis speed. I counted 
the number of concurrent instances (incrementing a static counter in ctor, 
decrementing in dtor), and it seems to go up into the hundreds of thousands, 
even millions before restarting the counter. I killed the process before 
crippling the server.

I'm not sure how long it'll take for me to figure out what's wrong, but these 
numbers are so ridiculous, I suspect a programming error rather then an 
algorithmic issue.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62883



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

[clang-tools-extra] r362978 - [clangd] Remove old hidden -use-dex-index flag

[Sam McCall via cfe-commits]

Author: sammccall
Date: Mon Jun 10 13:38:16 2019
New Revision: 362978

URL: http://llvm.org/viewvc/llvm-project?rev=362978=rev
Log:
[clangd] Remove old hidden -use-dex-index flag

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

Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=362978=362977=362978=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Mon Jun 10 13:38:16 2019
@@ -88,8 +88,7 @@ public:
 /// opened files and uses the index to augment code completion results.
 bool BuildDynamicSymbolIndex = false;
 /// Use a heavier and faster in-memory index implementation.
-/// FIXME: we should make this true if it isn't too slow to build!.
-bool HeavyweightDynamicSymbolIndex = false;
+bool HeavyweightDynamicSymbolIndex = true;
 /// If true, ClangdServer automatically indexes files in the current 
project
 /// on background threads. The index is stored in the project root.
 bool BackgroundIndex = false;

Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=362978=362977=362978=diff
==
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Mon Jun 10 13:38:16 2019
@@ -34,11 +34,6 @@
 
 namespace clang {
 namespace clangd {
-// FIXME: remove this option when Dex is cheap enough.
-static llvm::cl::opt
-UseDex("use-dex-index",
-   llvm::cl::desc("Use experimental Dex dynamic index"),
-   llvm::cl::init(true), llvm::cl::Hidden);
 
 static llvm::cl::opt CompileCommandsDir(
 "compile-commands-dir",
@@ -447,7 +442,6 @@ int main(int argc, char *argv[]) {
   if (!ResourceDir.empty())
 Opts.ResourceDir = ResourceDir;
   Opts.BuildDynamicSymbolIndex = EnableIndex;
-  Opts.HeavyweightDynamicSymbolIndex = UseDex;
   Opts.BackgroundIndex = EnableBackgroundIndex;
   Opts.BackgroundIndexRebuildPeriodMs = BackgroundIndexRebuildPeriod;
   std::unique_ptr StaticIdx;


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

[PATCH] D63092: [Frontend] Use executable path when creating invocation from cmdline

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.
Herald added a subscriber: ormris.

`argv[0]` does carry important information though, I think this will break a 
lot of things. It's... concerning that no tests broke.

For example, if it's `clang` or `g++` or `clang-cl` then that affects how 
command lines are parsed (regardless of whether the path actually exists).

Additionally I don't think all tools that call createInvocationFromCommandLine 
carry around all the files you want.

The wrapper tool case is a problem though, I see some options:

- change the process that generates the compilation database to emit a path to 
the underlying compiler instead, as tooling currently expects. This may be 
painful if e.g. goma pretends to be a regular toolchain. An argument can be 
made it should pretend harder (e.g. symlinking directories around it so 
discovery works as usual)
- change the process that generates the CDB to inject the relevant directory 
paths explicitly as flags
- make driver aware of the distinction between "invoked-as" and "current 
binary" and use the right path in the right places

(Incidentally if Driver is going to use the current binary, we shouldn't need 
to pass it in as a parameter, right?)

There are some potentially-unsolvable cases here: e.g. if your wrapper is 
`magic-remote-build` then maybe there's no standard library locally, or no way 
to find it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63092



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

[PATCH] D60233: [clang-scan-deps] initial outline of the tool that runs preprocessor to find dependencies over a JSON compilation database

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: clang/test/ClangScanDeps/Inputs/regular_cdb.json:4
+  "directory": "DIR",
+  "command": "clang -c DIR/regular_cdb.cpp -IInputs -MD -MF DIR/regular_cdb.d",
+  "file": "DIR/regular_cdb.cpp"

aganea wrote:
> Is `-MD -MF` required for clang-scan-deps to work? Can't we append those 
> arguments automatically, so that only include paths are needed in the CDB?
> 
> Would it possible for the tool to produce a collated file? A real-world usage 
> would otherwise produce tens of thousands of files. Which then would be 
> opened/parsed by the calling application.
I agree, the tool shouldn't rely on those options being there. `-MD -MF` in 
this test right now a crutch to get the dependencies printed somewhere.

I think the tool by default should print out the collated dependencies to 
STDOUT instead of writing them to files that were specified for the build. The 
`-MD -MF` options will not be required as well. This implementation requires 
some changes in the dependency collector, which I am planning to do as part of 
clang-scan-deps in the follow-up patches. Will it be ok to keep this patch as 
it is right now, and transition to printing out the collated dependencies 
without requiring the options as a follow-up?




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

https://reviews.llvm.org/D60233



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

[PATCH] D60233: [clang-scan-deps] initial outline of the tool that runs preprocessor to find dependencies over a JSON compilation database

[Alex Lorenz via Phabricator via cfe-commits]

arphaman updated this revision to Diff 203887.
arphaman marked 3 inline comments as done.
arphaman added a comment.

- Add test for `-j2` to exercise multi threading.
- Add `InitLLVM`.


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

https://reviews.llvm.org/D60233

Files:
  clang/test/ClangScanDeps/Inputs/header.h
  clang/test/ClangScanDeps/Inputs/header2.h
  clang/test/ClangScanDeps/Inputs/regular_cdb.json
  clang/test/ClangScanDeps/regular_cdb.cpp
  clang/tools/CMakeLists.txt
  clang/tools/clang-scan-deps/CMakeLists.txt
  clang/tools/clang-scan-deps/ClangScanDeps.cpp

Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- /dev/null
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -0,0 +1,218 @@
+//===-- ClangScanDeps.cpp - Implementation of clang-scan-deps -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Frontend/PCHContainerOperations.h"
+#include "clang/FrontendTool/Utils.h"
+#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/JSONCompilationDatabase.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/InitLLVM.h"
+#include "llvm/Support/JSON.h"
+#include "llvm/Support/Options.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Program.h"
+#include "llvm/Support/Signals.h"
+#include "llvm/Support/Threading.h"
+#include 
+
+using namespace clang;
+
+namespace {
+
+/// A clang tool that runs the preprocessor only for the given compiler
+/// invocation.
+class PreprocessorOnlyTool : public tooling::ToolAction {
+public:
+  PreprocessorOnlyTool(StringRef WorkingDirectory)
+  : WorkingDirectory(WorkingDirectory) {}
+
+  bool runInvocation(std::shared_ptr Invocation,
+ FileManager *FileMgr,
+ std::shared_ptr PCHContainerOps,
+ DiagnosticConsumer *DiagConsumer) override {
+// Create a compiler instance to handle the actual work.
+CompilerInstance Compiler(std::move(PCHContainerOps));
+Compiler.setInvocation(std::move(Invocation));
+FileMgr->getFileSystemOpts().WorkingDir = WorkingDirectory;
+Compiler.setFileManager(FileMgr);
+
+// Create the compiler's actual diagnostics engine.
+Compiler.createDiagnostics(DiagConsumer, /*ShouldOwnClient=*/false);
+if (!Compiler.hasDiagnostics())
+  return false;
+
+Compiler.createSourceManager(*FileMgr);
+
+auto Action = llvm::make_unique();
+const bool Result = Compiler.ExecuteAction(*Action);
+FileMgr->clearStatCache();
+return Result;
+  }
+
+private:
+  StringRef WorkingDirectory;
+};
+
+/// A proxy file system that doesn't call `chdir` when changing the working
+/// directory of a clang tool.
+class ProxyFileSystemWithoutChdir : public llvm::vfs::ProxyFileSystem {
+public:
+  ProxyFileSystemWithoutChdir(
+  llvm::IntrusiveRefCntPtr FS)
+  : ProxyFileSystem(std::move(FS)) {}
+
+  llvm::ErrorOr getCurrentWorkingDirectory() const override {
+assert(!CWD.empty() && "empty CWD");
+return CWD;
+  }
+
+  std::error_code setCurrentWorkingDirectory(const Twine ) override {
+CWD = Path.str();
+return {};
+  }
+
+private:
+  std::string CWD;
+};
+
+/// The high-level implementation of the dependency discovery tool that runs on
+/// an individual worker thread.
+class DependencyScanningTool {
+public:
+  /// Construct a dependency scanning tool.
+  ///
+  /// \param Compilations The reference to the compilation database that's
+  /// used by the clang tool.
+  DependencyScanningTool(const tooling::CompilationDatabase )
+  : Compilations(Compilations) {
+PCHContainerOps = std::make_shared();
+BaseFS = new ProxyFileSystemWithoutChdir(llvm::vfs::getRealFileSystem());
+  }
+
+  /// Computes the dependencies for the given file.
+  ///
+  /// \returns True on error.
+  bool runOnFile(const std::string , StringRef CWD) {
+BaseFS->setCurrentWorkingDirectory(CWD);
+tooling::ClangTool Tool(Compilations, Input, PCHContainerOps, BaseFS);
+Tool.clearArgumentsAdjusters();
+Tool.setRestoreWorkingDir(false);
+PreprocessorOnlyTool Action(CWD);
+return Tool.run();
+  }
+
+private:
+  const tooling::CompilationDatabase 
+  std::shared_ptr PCHContainerOps;
+  /// The real filesystem used as a base for all the operations performed by the
+  /// tool.
+  llvm::IntrusiveRefCntPtr BaseFS;
+};
+
+llvm::cl::opt Help("h", llvm::cl::desc("Alias for -help"),
+ llvm::cl::Hidden);
+
+llvm::cl::OptionCategory DependencyScannerCategory("Tool options");
+

[PATCH] D63093: [analyzer] WIP: MallocChecker: Release temporary CXXNewExpr

[Csaba Dabis via Phabricator via cfe-commits]

Charusso planned changes to this revision.
Charusso added a comment.

This is heavily WIP as sometimes we have to release a `new` after we return it 
or a constructor did something with that. The direction is okay?


Repository:
  rC Clang

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

https://reviews.llvm.org/D63093



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

[PATCH] D63093: [analyzer] WIP: MallocChecker: Release temporary CXXNewExpr

[Csaba Dabis via Phabricator via cfe-commits]

Charusso created this revision.
Charusso added reviewers: NoQ, xazax.hun, ravikandhadai, baloghadamsoftware, 
Szelethus.
Charusso added a project: clang.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, rnkovacs, szepet.

-


Repository:
  rC Clang

https://reviews.llvm.org/D63093

Files:
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp


Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -156,21 +156,14 @@
 
 typedef std::pair LeakInfo;
 
-class MallocChecker : public Checker,
- check::EndFunction,
- check::PreCall,
- check::PostStmt,
- check::PostStmt,
- check::NewAllocator,
- check::PreStmt,
- check::PostStmt,
- check::PostObjCMessage,
- check::Location,
- eval::Assume>
-{
+class MallocChecker
+: public Checker,
+ check::EndFunction, check::PreCall,
+ check::PostStmt, check::PostStmt,
+ check::PostStmt, 
check::NewAllocator,
+ check::PreStmt, check::PostStmt,
+ check::PostObjCMessage, check::Location, eval::Assume> {
 public:
   MallocChecker()
   : II_alloca(nullptr), II_win_alloca(nullptr), II_malloc(nullptr),
@@ -211,6 +204,7 @@
   void checkPreCall(const CallEvent , CheckerContext ) const;
   void checkPostStmt(const CallExpr *CE, CheckerContext ) const;
   void checkPostStmt(const CXXNewExpr *NE, CheckerContext ) const;
+  void checkPostStmt(const CXXBindTemporaryExpr *BTE, CheckerContext ) const;
   void checkNewAllocator(const CXXNewExpr *NE, SVal Target,
  CheckerContext ) const;
   void checkPreStmt(const CXXDeleteExpr *DE, CheckerContext ) const;
@@ -489,7 +483,8 @@
   bool IsReleased = (S && S->isReleased()) &&
 (!SPrev || !SPrev->isReleased());
   assert(!IsReleased ||
- (Stmt && (isa(Stmt) || isa(Stmt))) ||
+ (Stmt && (isa(Stmt) || isa(Stmt) ||
+   isa(Stmt))) ||
  (!Stmt && S->getAllocationFamily() == AF_InnerBuffer));
   return IsReleased;
 }
@@ -1103,6 +1098,34 @@
 processNewAllocation(NE, C, Target);
 }
 
+void MallocChecker::checkPostStmt(const CXXBindTemporaryExpr *BTE,
+  CheckerContext ) const {
+  if (const auto *CE = dyn_cast(BTE->getSubExpr())) {
+if (CE->getNumArgs() == 0)
+  return;
+
+// If we catch a 'CXXNewExpr' set it is released as it is temporary.
+if (const auto *NE = dyn_cast(CE->getArg(0))) {
+  ProgramStateRef State = C.getState();
+  Optional RetVal = C.getSVal(NE);
+
+  SymbolRef Sym = RetVal->getAsLocSymbol();
+  assert(Sym);
+
+  if (const RefState *RS = State->get(Sym))
+if (RS->isReleased())
+  return;
+
+  // Set the symbol's state to Released.
+  State = State->set(
+  Sym, RefState::getReleased(NE->isArray() ? AF_CXXNewArray : 
AF_CXXNew,
+ NE));
+
+  C.addTransition(State);
+}
+  }
+}
+
 // Sets the extent value of the MemRegion allocated by
 // new expression NE to its size in Bytes.
 //


Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -156,21 +156,14 @@
 
 typedef std::pair LeakInfo;
 
-class MallocChecker : public Checker,
- check::EndFunction,
- check::PreCall,
- check::PostStmt,
- check::PostStmt,
- check::NewAllocator,
- check::PreStmt,
- check::PostStmt,
- check::PostObjCMessage,
- check::Location,
- eval::Assume>
-{
+class MallocChecker
+: public Checker,
+ check::EndFunction, check::PreCall,
+ check::PostStmt, check::PostStmt,
+ check::PostStmt, check::NewAllocator,
+ check::PreStmt, check::PostStmt,
+ check::PostObjCMessage, check::Location, eval::Assume> {
 public:
   MallocChecker()
   : II_alloca(nullptr), II_win_alloca(nullptr), 

[PATCH] D63092: [Frontend] Use executable path when creating invocation from cmdline

[Petr Hosek via Phabricator via cfe-commits]

phosek created this revision.
phosek added reviewers: ilya-biryukov, sammccall, bkramer, echristo.
Herald added subscribers: cfe-commits, kadircet.
Herald added a project: clang.

Args[0] isn't necessarily a correct path, it may not even be the path
to the compiler, for example when using tools like ccache, distcc or
goma the first argument in command recorded inside the compilation
database would be this tool, not the compiler. This path is then used
as installation dir inside the driver and is then used to derive other
toolchain specific paths, such as path to libc++ headers (which would
be ../include/c++/v1 relative to the compiler binary). When the first
argument is another tool, or even if it's Clang but the compilation
database contains a relative path, the installation dir is going to be
incorrect and the header resolution will fail which breaks tools like
clangd. So rather than relying blindly on the first argument, we'll use
the executable path (when available) instead.


Repository:
  rC Clang

https://reviews.llvm.org/D63092

Files:
  clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
  clang/unittests/libclang/LibclangTest.cpp


Index: clang/unittests/libclang/LibclangTest.cpp
===
--- clang/unittests/libclang/LibclangTest.cpp
+++ clang/unittests/libclang/LibclangTest.cpp
@@ -621,7 +621,10 @@
   std::string Clang = "bin/clang";
   WriteFile(Clang, "");
 
+  std::string InstalledDir = TestDir + "/bin";
+
   const char *Argv[] = {Clang.c_str(), "-target", "arm-linux-gnueabi",
+"-ccc-install-dir", InstalledDir.c_str(),
 "-stdlib=libstdc++", "--gcc-toolchain="};
 
   EXPECT_EQ(CXError_Success,
Index: clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
===
--- clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
+++ clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
@@ -43,8 +43,12 @@
   // FIXME: Find a cleaner way to force the driver into restricted modes.
   Args.push_back("-fsyntax-only");
 
+  void *P = reinterpret_cast(createInvocationFromCommandLine);
+  std::string ClangExecutable =
+  llvm::sys::fs::getMainExecutable(Args[0], P);
+
   // FIXME: We shouldn't have to pass in the path info.
-  driver::Driver TheDriver(Args[0], llvm::sys::getDefaultTargetTriple(),
+  driver::Driver TheDriver(ClangExecutable, 
llvm::sys::getDefaultTargetTriple(),
*Diags, VFS);
 
   // Don't check that inputs exist, they may have been remapped.


Index: clang/unittests/libclang/LibclangTest.cpp
===
--- clang/unittests/libclang/LibclangTest.cpp
+++ clang/unittests/libclang/LibclangTest.cpp
@@ -621,7 +621,10 @@
   std::string Clang = "bin/clang";
   WriteFile(Clang, "");
 
+  std::string InstalledDir = TestDir + "/bin";
+
   const char *Argv[] = {Clang.c_str(), "-target", "arm-linux-gnueabi",
+"-ccc-install-dir", InstalledDir.c_str(),
 "-stdlib=libstdc++", "--gcc-toolchain="};
 
   EXPECT_EQ(CXError_Success,
Index: clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
===
--- clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
+++ clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
@@ -43,8 +43,12 @@
   // FIXME: Find a cleaner way to force the driver into restricted modes.
   Args.push_back("-fsyntax-only");
 
+  void *P = reinterpret_cast(createInvocationFromCommandLine);
+  std::string ClangExecutable =
+  llvm::sys::fs::getMainExecutable(Args[0], P);
+
   // FIXME: We shouldn't have to pass in the path info.
-  driver::Driver TheDriver(Args[0], llvm::sys::getDefaultTargetTriple(),
+  driver::Driver TheDriver(ClangExecutable, llvm::sys::getDefaultTargetTriple(),
*Diags, VFS);
 
   // Don't check that inputs exist, they may have been remapped.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63041: [PlistSupport] Produce a newline to end plist output files

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.

Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D63041



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

[PATCH] D62978: [analyzer] ReturnVisitor: Handle unknown ReturnStmts better

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

In D62978#1533558 , @NoQ wrote:

> Ah, that positive!


Positive == true positive, not false positive, I got it.

> No, i don't think this is a valid way to suppress it.

Bought me, they are worth to report. The misleading reports made me think I 
have to suppress them.

Example (difference starts at note 33, at line 410):
F9159208: before.html 
F9159209: after.html 


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

https://reviews.llvm.org/D62978



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

[PATCH] D63091: [clangd] Add a capability to enable completions with fixes.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D63091

Files:
  clangd/ClangdLSPServer.cpp
  clangd/Protocol.cpp
  clangd/Protocol.h


Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -387,6 +387,11 @@
   /// textDocument.completion.completionItem.snippetSupport
   bool CompletionSnippets = false;
 
+  /// Client supports completions with additionalTextEdit near the cursor.
+  /// This is a clangd extension. (LSP says this is for unrelated text only).
+  /// textDocument.completion.editsNearCursor
+  bool CompletionFixes = false;
+
   /// Client supports hierarchical document symbols.
   bool HierarchicalDocumentSymbol = false;
 
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -293,6 +293,8 @@
 return false;
 }
   }
+  if (auto EditsNearCursor = Completion->getBoolean("editsNearCursor"))
+R.CompletionFixes = *EditsNearCursor;
 }
 if (auto *CodeAction = TextDocument->getObject("codeAction")) {
   if (CodeAction->getObject("codeActionLiteralSupport"))
Index: clangd/ClangdLSPServer.cpp
===
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -337,6 +337,7 @@
   applyConfiguration(Params.initializationOptions.ConfigSettings);
 
   CCOpts.EnableSnippets = Params.capabilities.CompletionSnippets;
+  CCOpts.IncludeFixIts = Params.capabilities.CompletionFixes;
   DiagOpts.EmbedFixesInDiagnostics = Params.capabilities.DiagnosticFixes;
   DiagOpts.SendDiagnosticCategory = Params.capabilities.DiagnosticCategory;
   DiagOpts.EmitRelatedLocations =


Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -387,6 +387,11 @@
   /// textDocument.completion.completionItem.snippetSupport
   bool CompletionSnippets = false;
 
+  /// Client supports completions with additionalTextEdit near the cursor.
+  /// This is a clangd extension. (LSP says this is for unrelated text only).
+  /// textDocument.completion.editsNearCursor
+  bool CompletionFixes = false;
+
   /// Client supports hierarchical document symbols.
   bool HierarchicalDocumentSymbol = false;
 
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -293,6 +293,8 @@
 return false;
 }
   }
+  if (auto EditsNearCursor = Completion->getBoolean("editsNearCursor"))
+R.CompletionFixes = *EditsNearCursor;
 }
 if (auto *CodeAction = TextDocument->getObject("codeAction")) {
   if (CodeAction->getObject("codeActionLiteralSupport"))
Index: clangd/ClangdLSPServer.cpp
===
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -337,6 +337,7 @@
   applyConfiguration(Params.initializationOptions.ConfigSettings);
 
   CCOpts.EnableSnippets = Params.capabilities.CompletionSnippets;
+  CCOpts.IncludeFixIts = Params.capabilities.CompletionFixes;
   DiagOpts.EmbedFixesInDiagnostics = Params.capabilities.DiagnosticFixes;
   DiagOpts.SendDiagnosticCategory = Params.capabilities.DiagnosticCategory;
   DiagOpts.EmitRelatedLocations =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62978: [analyzer] ReturnVisitor: Handle unknown ReturnStmts better

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 203876.
Charusso retitled this revision from "[analyzer] ReturnVisitor: Handle non-null 
ReturnStmts" to "[analyzer] ReturnVisitor: Handle unknown ReturnStmts better".
Charusso edited the summary of this revision.
Charusso added a comment.

- The report was too misleading.
- It turns out we have to keep non-nulls.
- Now we do not treat unknown values as known.


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

https://reviews.llvm.org/D62978

Files:
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/test/Analysis/diagnostics/find_last_store.c
  clang/test/Analysis/uninit-vals.c

Index: clang/test/Analysis/uninit-vals.c
===
--- clang/test/Analysis/uninit-vals.c
+++ clang/test/Analysis/uninit-vals.c
@@ -149,8 +149,6 @@
   RetVoidFuncType f = foo_radar12278788_fp;
   return ((RetIntFuncType)f)(); //expected-warning {{Undefined or garbage value returned to caller}}
 //expected-note@-1 {{Undefined or garbage value returned to caller}}
-//expected-note@-2 {{Calling 'foo_radar12278788_fp'}}
-//expected-note@-3 {{Returning from 'foo_radar12278788_fp'}}
 }
 
 void rdar13665798() {
@@ -164,8 +162,6 @@
 RetVoidFuncType f = foo_radar12278788_fp;
 return ((RetIntFuncType)f)(); //expected-warning {{Undefined or garbage value returned to caller}}
   //expected-note@-1 {{Undefined or garbage value returned to caller}}
-  //expected-note@-2 {{Calling 'foo_radar12278788_fp'}}
-  //expected-note@-3 {{Returning from 'foo_radar12278788_fp'}}
   }();
 }
 
@@ -182,18 +178,14 @@
 void use(struct Point p); 
 
 void testUseHalfPoint() {
-  struct Point p = getHalfPoint(); // expected-note{{Calling 'getHalfPoint'}}
-   // expected-note@-1{{Returning from 'getHalfPoint'}}
-   // expected-note@-2{{'p' initialized here}}
+  struct Point p = getHalfPoint(); // expected-note{{'p' initialized here}}
   use(p); // expected-warning{{uninitialized}}
   // expected-note@-1{{uninitialized}}
 }
 
 void testUseHalfPoint2() {
   struct Point p;
-  p = getHalfPoint(); // expected-note{{Calling 'getHalfPoint'}}
-  // expected-note@-1{{Returning from 'getHalfPoint'}}
-  // expected-note@-2{{Value assigned to 'p'}}
+  p = getHalfPoint(); // expected-note{{Value assigned to 'p'}}
   use(p); // expected-warning{{uninitialized}}
   // expected-note@-1{{uninitialized}}
 }
Index: clang/test/Analysis/diagnostics/find_last_store.c
===
--- clang/test/Analysis/diagnostics/find_last_store.c
+++ clang/test/Analysis/diagnostics/find_last_store.c
@@ -2,13 +2,11 @@
 typedef struct { float b; } c;
 void *a();
 void *d() {
-  return a(); // expected-note{{Returning pointer}}
+  return a();
 }
 
 void no_find_last_store() {
-  c *e = d(); // expected-note{{Calling 'd'}}
-  // expected-note@-1{{Returning from 'd'}}
-  // expected-note@-2{{'e' initialized here}}
+  c *e = d(); // expected-note{{'e' initialized here}}
 
   (void)(e || e->b); // expected-note{{Assuming 'e' is null}}
   // expected-note@-1{{Left side of '||' is false}}
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -874,7 +874,6 @@
   if (Optional RetLoc = RetVal.getAs())
 EnableNullFPSuppression = State->isNull(*RetLoc).isConstrainedTrue();
 
-BR.markInteresting(CalleeContext);
 BR.addVisitor(llvm::make_unique(CalleeContext,
EnableNullFPSuppression,
Options));
@@ -925,16 +924,13 @@
 
 RetE = RetE->IgnoreParenCasts();
 
-// If we're returning 0, we should track where that 0 came from.
-bugreporter::trackExpressionValue(N, RetE, BR, EnableNullFPSuppression);
-
 // Build an appropriate message based on the return value.
 SmallString<64> Msg;
 llvm::raw_svector_ostream Out(Msg);
 
+// Known to be null.
 if (State->isNull(V).isConstrainedTrue()) {
   if (V.getAs()) {
-
 // If we have counter-suppression enabled, make sure we keep visiting
 // future nodes. We want to emit a path note as well, in case
 // the report is resurrected as valid later on.
@@ -950,8 +946,8 @@
   } else {
 Out << "Returning zero";
   }
-
-} else {
+// Known to be non-null.
+} else if (State->isNonNull(V).isConstrainedTrue()) {
   if (auto CI = V.getAs()) {
 Out << "Returning the 

[PATCH] D63062: [clang-format] Added New Style Rule: BitFieldDeclsOnSeparateLines

[MyDeveloperDay via Phabricator via cfe-commits]

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



Comment at: docs/ClangFormatStyleOptions.rst:194
 
+**BitFieldDeclsOnSeparateLines** (``bool``)
+  If ``true``, Align Bitfield Declarations on separate lines.

Isn't the documentation normally alphabetric? shouldn't it be after 
AlignConsecutiveAssignments



Comment at: include/clang/Format/Format.h:104
 
+  /// If ``true``, Linesup Bitfield Declarations.
+  /// This will lineup Bitfield declarations on consecutive lines. This

Align



Comment at: include/clang/Format/Format.h:112
+  /// \endcode
+  bool BitFieldDeclsOnSeparateLines;
+

I think this should be alphabetic in this file (not sure though check the rest 
of it)

are you happy with the name?  BitFieldDeclsOnSeparateLines? 

1) people often spell Separate incorrectly (didn't you?), this could lead to 
misconfigured
2) isn't this really a ''Break'' rule

I want to say this might better as  something like "BreakAfterBitFieldDecl"





Comment at: lib/Format/ContinuationIndenter.cpp:284
+return true;
   if (!Current.CanBreakBefore && !(State.Stack.back().BreakBeforeClosingBrace 
&&
Current.closesBlockOrBlockTypeList(Style)))

something here doesn't feel quite right,, without trying the code change myself 
I cannot tell, did you ever try this code without having the same clause in 
canBreak() and mustBreak()? (i.e. just put it in mustBreak)

The reason I ask is I'm unclear as to why the other mustBreak() rules aren't 
here in canBreak() if thats the case



Comment at: lib/Format/TokenAnnotator.cpp:2921
+  Right.is(tok::identifier) && (Right.Next->is(tok::colon)))
+  return true;
   if (Right.NewlinesBefore > 1 && Style.MaxEmptyLinesToKeep > 0)

mgorny wrote:
> Misindent.
This code appears 3 times (does it need to appear 3 times?), do we need some 
sort of 

```
bool isBitField(FormatToken)
{
...
}
```

Should a bit field check for the existence of a number after the colon? I can't 
think of other C++ constructs that appear as

```
comma identifier colon
```

but given that clang-format is used for ObjC,ProtoBuf,Java,JavaScript,C# I'm 
pretty sure something odd is going to happen with JavaScript named parameters, 
to be honest I think this is going to cause the following to get reformatted 

MyFunctionCall({ xPosition: 20**, yPosition: 50,** width: 100, height: 5, 
drawingNow: true });


```
MyFunctionCall({ xPosition: 20**, yPosition: 50,**
width: 100, 
height: 5, 
drawingNow: true });
```

or something like that



Comment at: unittests/Format/FormatTest.cpp:3671
+  );
+} 
 

please add a test with comments (it will get logged)


```
unsigned int baz : 11, /*motor control flags*/
 add: 2/* control code for turning the lights on */ ,
 foo: 3 /* (unused */
```



Repository:
  rC Clang

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

https://reviews.llvm.org/D63062



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

[PATCH] D59919: [Attributor] Deduce "returned" argument attribute

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D59919#1535643 , @nicholas wrote:

> > CHANGED: build-libcalls   NumNoUnwind   
> >   4526 ->   3382 (   -25.276%)
>
> Why did the number of nounwinds drop?


I rerun the experiment with and without this commit. The numbers are the same 
as before except that there is no difference in NumNoUnwind. I don't know if I 
used a different baseline or if there was another problem (e.g., concurrency 
related) but I don't think this patch does affect the NumNoUnwind number at all.

Btw. I only report statistics that changed more than 1% as there seem to be 
some minimal variations all the time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59919



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

[PATCH] D63089: [clang] Warn on implicit boolean casts in more contexts (PR34180)

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Thanks !

You can check and take tests from my older patch https://reviews.llvm.org/D58878


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63089



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

[PATCH] D62926: [analyzer] ReturnVisitor: Bypass everything to see inlined calls

[Csaba Dabis via Phabricator via cfe-commits]

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



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:842-849
+  if (Optional CEE = Node->getLocationAs())
 if (CEE->getCalleeContext()->getCallSite() == S)
   break;
-  if (auto SP = Node->getLocationAs())
-if (SP->getStmt() == S)
-  break;
+
+  if (!IsBypass)
+if (Optional SP = Node->getLocationAs())
+  if (SP->getStmt() == S)

NoQ wrote:
> Comparing statements is usually insufficient because the same statement may 
> appear multiple times due to recursion. When recursion occurs, you may reach 
> the same statement in a different location context. You should think in terms 
> of (statement, location context) pairs to avoid these problems. Your aim here 
> is to find the `CallExitEnd` node that corresponds to returning from an 
> inlined operator new to the current location context. You should stop 
> searching when you find an unrelated statement in the current location 
> context or when you exit the current location context entirely.
I have made a little test when we have a 25-second long Static Analyzer run 
with predefined names and checker. The loop ran 500 times in summary and we 
have some serious performance impacts at other places.

We exit the current context to see inlined calls, so that could not work sadly. 
If you remove that nonsense second condition we run at the same time, so if we 
have not got any problem since 7 years ago I think it is good to go.


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

https://reviews.llvm.org/D62926



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

[PATCH] D62926: [analyzer] ReturnVisitor: Bypass everything to see inlined calls

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 203874.
Charusso retitled this revision from "[analyzer] ReturnVisitor: Bypass 
constructing objects to see inlined calls" to "[analyzer] ReturnVisitor: Bypass 
everything to see inlined calls".
Charusso added a comment.

- The most generic approach.


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

https://reviews.llvm.org/D62926

Files:
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/test/Analysis/new-ctor-null-throw.cpp
  clang/test/Analysis/new-ctor-null.cpp


Index: clang/test/Analysis/new-ctor-null.cpp
===
--- clang/test/Analysis/new-ctor-null.cpp
+++ clang/test/Analysis/new-ctor-null.cpp
@@ -1,4 +1,6 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection 
-analyzer-config c++-allocator-inlining=true -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,debug.ExprInspection \
+// RUN:  -verify %s
 
 void clang_analyzer_eval(bool);
 void clang_analyzer_warnIfReached();
@@ -24,7 +26,8 @@
 
 void testArrays() {
   S *s = new S[10]; // no-crash
-  s[0].x = 2; // expected-warning{{Dereference of null pointer}}
+  s[0].x = 2;
+  // no-warning: 'Dereference of null pointer' suppressed by ReturnVisitor.
 }
 
 int global;
Index: clang/test/Analysis/new-ctor-null-throw.cpp
===
--- clang/test/Analysis/new-ctor-null-throw.cpp
+++ clang/test/Analysis/new-ctor-null-throw.cpp
@@ -1,4 +1,9 @@
-// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,debug.ExprInspection 
-analyzer-config c++-allocator-inlining=true -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:  -analyzer-config suppress-null-return-paths=false \
+// RUN:  -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:  -DSUPPRESSED \
+// RUN:  -verify %s
 
 void clang_analyzer_eval(bool);
 
@@ -9,18 +14,41 @@
 // operator new.
 void *operator new(size_t size) {
   return nullptr;
+  // expected-warning@-1 {{'operator new' should not return a null pointer 
unless it is declared 'throw()' or 'noexcept'}}
 }
 void *operator new[](size_t size) {
   return nullptr;
+  // expected-warning@-1 {{'operator new[]' should not return a null pointer 
unless it is declared 'throw()' or 'noexcept'}}
 }
 
 struct S {
   int x;
   S() : x(1) {}
   ~S() {}
+  int getX() const { return x; }
 };
 
 void testArrays() {
   S *s = new S[10]; // no-crash
-  s[0].x = 2; // expected-warning{{Dereference of null pointer}}
+  s[0].x = 2;
+#ifndef SUPPRESSED
+  // expected-warning@-2 {{Dereference of null pointer}}
+#endif
 }
+
+void testCtor() {
+  S *s = new S();
+  s->x = 13;
+#ifndef SUPPRESSED
+  // expected-warning@-2 {{Access to field 'x' results in a dereference of a 
null pointer (loaded from variable 's')}}
+#endif
+}
+
+void testMethod() {
+  S *s = new S();
+  const int X = s->getX();
+#ifndef SUPPRESSED
+  // expected-warning@-2 {{Called C++ object pointer is null}}
+#endif
+}
+
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -834,12 +834,9 @@
 
 // First, find when we processed the statement.
 do {
-  if (auto CEE = Node->getLocationAs())
+  if (Optional CEE = Node->getLocationAs())
 if (CEE->getCalleeContext()->getCallSite() == S)
   break;
-  if (auto SP = Node->getLocationAs())
-if (SP->getStmt() == S)
-  break;
 
   Node = Node->getFirstPred();
 } while (Node);


Index: clang/test/Analysis/new-ctor-null.cpp
===
--- clang/test/Analysis/new-ctor-null.cpp
+++ clang/test/Analysis/new-ctor-null.cpp
@@ -1,4 +1,6 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config c++-allocator-inlining=true -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,debug.ExprInspection \
+// RUN:  -verify %s
 
 void clang_analyzer_eval(bool);
 void clang_analyzer_warnIfReached();
@@ -24,7 +26,8 @@
 
 void testArrays() {
   S *s = new S[10]; // no-crash
-  s[0].x = 2; // expected-warning{{Dereference of null pointer}}
+  s[0].x = 2;
+  // no-warning: 'Dereference of null pointer' suppressed by ReturnVisitor.
 }
 
 int global;
Index: clang/test/Analysis/new-ctor-null-throw.cpp
===
--- clang/test/Analysis/new-ctor-null-throw.cpp
+++ clang/test/Analysis/new-ctor-null-throw.cpp
@@ -1,4 +1,9 @@
-// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,debug.ExprInspection -analyzer-config c++-allocator-inlining=true -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:  -analyzer-config suppress-null-return-paths=false \
+// 

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[Michael Kruse via Phabricator via cfe-commits]

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

In D61446#1533076 , @serge-sans-paille 
wrote:

> That's what I tried to do when always adding Polly.cpp to PollyCore, but it 
> doesn't seem to be enough, I still need to invstigate why (it is actually 
> enough when using BUILD_SHARED_LIBS=ON)


With static libraries, you had the following structure:

  PollyCore.a:
RegisterPasses.o
Polly.o <-- static initializer here, but never used
  
  LLVMPolly.so (for use with -load mechanism)
RegisterPasses.o
Polly.o <-- static initializer here, executed when loading the .so
  
  opt:
main.o <-- call to initializePollyPass removed

With static linking, there is no reason for the linker to add Polly.o to the 
`opt` executable because no symbol from Polly.o (or any of PollyCore) was 
required to resolve any symbol in `opt`. Building a shared library using the 
object library files will include all object files, since it does not know 
which symbols will be used at runtime. I recommend reading 
http://www.lurklurk.org/linkers/linkers.html#staticlibs and 
https://www.bfilipek.com/2018/02/static-vars-static-lib.html.

What I proposed was

  PollyCore.a:
RegisterPasses.o
  
  LLVMPolly.so (for use with -load mechanism)
RegisterPasses.o
Polly.o
  
  opt: // or any ENABLE_PLUGINS tool
main.o
Polly.o // via add_executable(opt Polly.cpp) or target_sources

Adding the object file explicitly to the add_executable will add it 
unconditionally, even if no none of its symbol is required, like for 
LLVMPolly.so.

In the latest update you changed the location of the static initalizer to 
`RegisterPasses.o`, which contains the symbol `llvmGetPassPluginInfo` which is 
pulled-in by the new pass manager plugin system. This makes an unfortunate 
dependence of the static initializer on the `llvmGetPassPluginInfo` mechanism 
(which I think only works for `opt` in the current patch).

There is a reason why pass loading in Polly is organized as it is.




Comment at: llvm/cmake/modules/AddLLVM.cmake:812
+#   llvm::PassPluginLibraryInfo ${entry_point}();
+add_custom_target(LLVM_PLUGINS)  # target used to hold global properties 
referencable from generator-expression
+function(register_llvm_extension llvm_extension entry_point)

beanz wrote:
> Change this to `llvm-plugins` to match our convention and wrap it in `if (NOT 
> TARGET...)` so it doesn't error if AddLLVM is included twice.
[serious] I get multiple of these errors by cmake:
```
CMake Error at cmake/modules/AddLLVM.cmake:812 (add_custom_target):
  add_custom_target cannot create target "LLVM_PLUGINS" because another
  target with the same name already exists.  The existing target is a custom
  target created in source directory "/home/meinersbur/src/llvm".  See
  documentation for policy CMP0002 for more details.
Call Stack (most recent call first):
  projects/compiler-rt/unittests/CMakeLists.txt:2 (include)
```



Comment at: llvm/tools/opt/NewPMDriver.cpp:292
+#define HANDLE_EXTENSION(Ext)  
\
+  get##Ext##PluginInfo().RegisterPassBuilderCallbacks(PB);
+#include "llvm/Support/Extension.def"

[serious] This will pull-in the symbol `llvmGetPassPluginInfo` from the plugin 
for `opt`, but what about the other tools (bugpoint/clang)?



Comment at: polly/lib/Support/RegisterPasses.cpp:727
+extern "C" ::llvm::PassPluginLibraryInfo LLVM_ATTRIBUTE_WEAK
+llvmGetPassPluginInfo() {
+  return getPassPluginInfo();

[serious] Unfortunately, the new pass manager's plugin system relies on the 
function name to be `llvmGetPassPluginInfo` in each plugin. This works with 
multiple dynamic libraries all declaring the same name using the 
`PassPlugin::Load` mechanism, but linking them all statically will violate the 
one-definition-rule.

IMHO, Polly.cpp would have been a better place for this function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446



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

[PATCH] D63089: [clang] Warn on implicit boolean casts in more contexts (PR34180)

[Mateusz Maćkowski via Phabricator via cfe-commits]

m4tx created this revision.
m4tx added reviewers: lebedev.ri, rsmith, klimek.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

  int x;
  return x = 5;

For a code like above, GCC produces a warning suggesting using parentheses 
about the assignment. This change makes clang produce similar warning to, 
suggesting either changing the operator to `==`, or wrap the expression inside 
parentheses.

This is based off D45401 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63089

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/Sema/parentheses.c
  clang/test/Sema/parentheses.cpp


Index: clang/test/Sema/parentheses.cpp
===
--- clang/test/Sema/parentheses.cpp
+++ clang/test/Sema/parentheses.cpp
@@ -215,3 +215,15 @@
 // fix-it:"{{.*}}":{[[@LINE-9]]:20-[[@LINE-9]]:20}:")"
   }
 }
+
+bool return_assign() {
+  int i;
+  return i = 4; // expected-warning {{assignment as a condition}} \
+// expected-note{{place parentheses around the assignment to 
silence this warning}} \
+// expected-note{{use '==' to turn this assignment into an 
equality comparison}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:10}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:15-[[@LINE-4]]:15}:")"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:12-[[@LINE-5]]:13}:"=="
+
+  return (i = 4);
+}
Index: clang/test/Sema/parentheses.c
===
--- clang/test/Sema/parentheses.c
+++ clang/test/Sema/parentheses.c
@@ -14,6 +14,18 @@
   if ((i = 4)) {}
 }
 
+_Bool return_assign(void) {
+  int i;
+  return i = 4; // expected-warning {{assignment as a condition}} \
+// expected-note{{place parentheses around the assignment to 
silence this warning}} \
+// expected-note{{use '==' to turn this assignment into an 
equality comparison}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:10}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:15-[[@LINE-4]]:15}:")"
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:12-[[@LINE-5]]:13}:"=="
+
+  return (i = 4);
+}
+
 void bitwise_rel(unsigned i) {
   (void)(i & 0x2 == 0); // expected-warning {{& has lower precedence than ==}} 
\
 // expected-note{{place parentheses around the '==' 
expression to silence this warning}} \
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -3926,6 +3926,11 @@
 FromType = From->getType();
   }
 
+  // Warn on assignments inside implicit casts (= instead of ==)
+  if (SCS.Second == ICK_Boolean_Conversion || FromType == Context.BoolTy) {
+DiagnoseAssignmentAsCondition(From->IgnoreImpCasts());
+  }
+
   // If we're converting to an atomic type, first convert to the corresponding
   // non-atomic type.
   QualType ToAtomicType;
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -16375,7 +16375,10 @@
 
 ExprResult Sema::CheckBooleanCondition(SourceLocation Loc, Expr *E,
bool IsConstexpr) {
-  DiagnoseAssignmentAsCondition(E);
+  // C++ implicit casts are checked inside PerformImplicitConversion
+  if (!getLangOpts().CPlusPlus) {
+DiagnoseAssignmentAsCondition(E);
+  }
   if (ParenExpr *parenE = dyn_cast(E))
 DiagnoseEqualityWithExtraParens(parenE);
 
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -9700,6 +9700,13 @@
   << FD << getLangOpts().CPlusPlus11;
 }
   }
+
+  // C++ implicit casts are checked inside PerformImplicitConversion
+  if (!getLangOpts().CPlusPlus) {
+if (ImplicitCastExpr *CastExpr = dyn_cast(RetValExp)) {
+  DiagnoseAssignmentAsCondition(CastExpr->IgnoreImpCasts());
+}
+  }
 }
 
 //===--- CHECK: Floating-Point comparisons (-Wfloat-equal) ---===//


Index: clang/test/Sema/parentheses.cpp
===
--- clang/test/Sema/parentheses.cpp
+++ clang/test/Sema/parentheses.cpp
@@ -215,3 +215,15 @@
 // fix-it:"{{.*}}":{[[@LINE-9]]:20-[[@LINE-9]]:20}:")"
   }
 }
+
+bool return_assign() {
+  int i;
+  return i = 4; // expected-warning {{assignment as a condition}} \
+// expected-note{{place parentheses around the assignment to silence this warning}} \
+// expected-note{{use '==' to turn this assignment into an equality comparison}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:10}:"("
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:15-[[@LINE-4]]:15}:")"
+  // CHECK: 

[PATCH] D63088: [clang-tidy] misc-unused-parameters: don't comment out parameter name for C code

[Matthias Gehre via Phabricator via cfe-commits]

mgehre updated this revision to Diff 203873.
mgehre added a comment.

Fix rewrapped comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63088

Files:
  clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
  clang-tools-extra/test/clang-tidy/misc-unused-parameters.c


Index: clang-tools-extra/test/clang-tidy/misc-unused-parameters.c
===
--- clang-tools-extra/test/clang-tidy/misc-unused-parameters.c
+++ clang-tools-extra/test/clang-tidy/misc-unused-parameters.c
@@ -4,7 +4,7 @@
 // =
 void a(int i) {;}
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: parameter 'i' is unused 
[misc-unused-parameters]
-// CHECK-FIXES: {{^}}void a(int  /*i*/) {;}{{$}}
+// CHECK-FIXES: {{^}}void a(int i) {;}{{$}}
 
 static void b(); // In C, forward declarations can leave out parameters.
 static void b(int i) {;}
Index: clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
@@ -138,16 +138,20 @@
 Indexer = llvm::make_unique(*Result.Context);
   }
 
-  // Comment out parameter name for non-local functions.
+  // Cannot remove parameter for non-local functions.
   if (Function->isExternallyVisible() ||
   !Result.SourceManager->isInMainFile(Function->getLocation()) ||
   !Indexer->getOtherRefs(Function).empty() || isOverrideMethod(Function)) {
-SourceRange RemovalRange(Param->getLocation());
-// Note: We always add a space before the '/*' to not accidentally create a
-// '*/*' for pointer types, which doesn't start a comment. clang-format 
will
-// clean this up afterwards.
-MyDiag << FixItHint::CreateReplacement(
-RemovalRange, (Twine(" /*") + Param->getName() + "*/").str());
+
+// Comment out parameter name.
+if (Result.Context->getLangOpts().CPlusPlus) {
+  SourceRange RemovalRange(Param->getLocation());
+  // Note: We always add a space before the '/*' to not accidentally create
+  // a '*/*' for pointer types, which doesn't start a comment. clang-format
+  // will clean this up afterwards.
+  MyDiag << FixItHint::CreateReplacement(
+  RemovalRange, (Twine(" /*") + Param->getName() + "*/").str());
+}
 return;
   }
 


Index: clang-tools-extra/test/clang-tidy/misc-unused-parameters.c
===
--- clang-tools-extra/test/clang-tidy/misc-unused-parameters.c
+++ clang-tools-extra/test/clang-tidy/misc-unused-parameters.c
@@ -4,7 +4,7 @@
 // =
 void a(int i) {;}
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: parameter 'i' is unused [misc-unused-parameters]
-// CHECK-FIXES: {{^}}void a(int  /*i*/) {;}{{$}}
+// CHECK-FIXES: {{^}}void a(int i) {;}{{$}}
 
 static void b(); // In C, forward declarations can leave out parameters.
 static void b(int i) {;}
Index: clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
@@ -138,16 +138,20 @@
 Indexer = llvm::make_unique(*Result.Context);
   }
 
-  // Comment out parameter name for non-local functions.
+  // Cannot remove parameter for non-local functions.
   if (Function->isExternallyVisible() ||
   !Result.SourceManager->isInMainFile(Function->getLocation()) ||
   !Indexer->getOtherRefs(Function).empty() || isOverrideMethod(Function)) {
-SourceRange RemovalRange(Param->getLocation());
-// Note: We always add a space before the '/*' to not accidentally create a
-// '*/*' for pointer types, which doesn't start a comment. clang-format will
-// clean this up afterwards.
-MyDiag << FixItHint::CreateReplacement(
-RemovalRange, (Twine(" /*") + Param->getName() + "*/").str());
+
+// Comment out parameter name.
+if (Result.Context->getLangOpts().CPlusPlus) {
+  SourceRange RemovalRange(Param->getLocation());
+  // Note: We always add a space before the '/*' to not accidentally create
+  // a '*/*' for pointer types, which doesn't start a comment. clang-format
+  // will clean this up afterwards.
+  MyDiag << FixItHint::CreateReplacement(
+  RemovalRange, (Twine(" /*") + Param->getName() + "*/").str());
+}
 return;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63088: [clang-tidy] misc-unused-parameters: don't comment out parameter name for C code

[Matthias Gehre via Phabricator via cfe-commits]

mgehre created this revision.
mgehre added reviewers: klimek, ilya-biryukov, lebedev.ri, aaron.ballman.
Herald added a subscriber: xazax.hun.
Herald added a project: clang.

The fixit `int square(int /*num*/)` yields `error: parameter name omitted` for 
C code. Enable it only for C++ code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63088

Files:
  clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
  clang-tools-extra/test/clang-tidy/misc-unused-parameters.c


Index: clang-tools-extra/test/clang-tidy/misc-unused-parameters.c
===
--- clang-tools-extra/test/clang-tidy/misc-unused-parameters.c
+++ clang-tools-extra/test/clang-tidy/misc-unused-parameters.c
@@ -4,7 +4,7 @@
 // =
 void a(int i) {;}
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: parameter 'i' is unused 
[misc-unused-parameters]
-// CHECK-FIXES: {{^}}void a(int  /*i*/) {;}{{$}}
+// CHECK-FIXES: {{^}}void a(int i) {;}{{$}}
 
 static void b(); // In C, forward declarations can leave out parameters.
 static void b(int i) {;}
Index: clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
@@ -138,16 +138,21 @@
 Indexer = llvm::make_unique(*Result.Context);
   }
 
-  // Comment out parameter name for non-local functions.
+  // Cannot remove parameter for non-local functions.
   if (Function->isExternallyVisible() ||
   !Result.SourceManager->isInMainFile(Function->getLocation()) ||
   !Indexer->getOtherRefs(Function).empty() || isOverrideMethod(Function)) {
-SourceRange RemovalRange(Param->getLocation());
-// Note: We always add a space before the '/*' to not accidentally create a
-// '*/*' for pointer types, which doesn't start a comment. clang-format 
will
-// clean this up afterwards.
-MyDiag << FixItHint::CreateReplacement(
-RemovalRange, (Twine(" /*") + Param->getName() + "*/").str());
+
+// Comment out parameter name.
+if (Result.Context->getLangOpts().CPlusPlus) {
+  SourceRange RemovalRange(Param->getLocation());
+  // Note: We always add a space before the '/*' to not accidentally create
+  // a
+  // '*/*' for pointer types, which doesn't start a comment. clang-format
+  // will clean this up afterwards.
+  MyDiag << FixItHint::CreateReplacement(
+  RemovalRange, (Twine(" /*") + Param->getName() + "*/").str());
+}
 return;
   }
 


Index: clang-tools-extra/test/clang-tidy/misc-unused-parameters.c
===
--- clang-tools-extra/test/clang-tidy/misc-unused-parameters.c
+++ clang-tools-extra/test/clang-tidy/misc-unused-parameters.c
@@ -4,7 +4,7 @@
 // =
 void a(int i) {;}
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: parameter 'i' is unused [misc-unused-parameters]
-// CHECK-FIXES: {{^}}void a(int  /*i*/) {;}{{$}}
+// CHECK-FIXES: {{^}}void a(int i) {;}{{$}}
 
 static void b(); // In C, forward declarations can leave out parameters.
 static void b(int i) {;}
Index: clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
@@ -138,16 +138,21 @@
 Indexer = llvm::make_unique(*Result.Context);
   }
 
-  // Comment out parameter name for non-local functions.
+  // Cannot remove parameter for non-local functions.
   if (Function->isExternallyVisible() ||
   !Result.SourceManager->isInMainFile(Function->getLocation()) ||
   !Indexer->getOtherRefs(Function).empty() || isOverrideMethod(Function)) {
-SourceRange RemovalRange(Param->getLocation());
-// Note: We always add a space before the '/*' to not accidentally create a
-// '*/*' for pointer types, which doesn't start a comment. clang-format will
-// clean this up afterwards.
-MyDiag << FixItHint::CreateReplacement(
-RemovalRange, (Twine(" /*") + Param->getName() + "*/").str());
+
+// Comment out parameter name.
+if (Result.Context->getLangOpts().CPlusPlus) {
+  SourceRange RemovalRange(Param->getLocation());
+  // Note: We always add a space before the '/*' to not accidentally create
+  // a
+  // '*/*' for pointer types, which doesn't start a comment. clang-format
+  // will clean this up afterwards.
+  MyDiag << FixItHint::CreateReplacement(
+  RemovalRange, (Twine(" /*") + Param->getName() + "*/").str());
+}
 return;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63086: [analyzer][NoStoreFuncVisitor][NFC] Move methods out-of-line, turn some to static functions

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

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

I personally found NoStoreFuncVisitor very hard to read, and since I'm planning 
to make other changes to it later on, I think this time is as good as any to 
make it somewhat more readable.

- Make several methods static functions
- Move non-trivial methods out-of-line
- Add a divider
- Turn non-obvious `auto`s into `Optional`
- clang-format affected lines


Repository:
  rC Clang

https://reviews.llvm.org/D63086

Files:
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -196,37 +196,6 @@
   return {};
 }
 
-//===--===//
-// Implementation of BugReporterVisitor.
-//===--===//
-
-std::shared_ptr
-BugReporterVisitor::getEndPath(BugReporterContext &,
-   const ExplodedNode *, BugReport &) {
-  return nullptr;
-}
-
-void
-BugReporterVisitor::finalizeVisitor(BugReporterContext &,
-const ExplodedNode *, BugReport &) {}
-
-std::shared_ptr BugReporterVisitor::getDefaultEndPath(
-BugReporterContext , const ExplodedNode *EndPathNode, BugReport ) {
-  PathDiagnosticLocation L =
-PathDiagnosticLocation::createEndOfPath(EndPathNode,BRC.getSourceManager());
-
-  const auto  = BR.getRanges();
-
-  // Only add the statement itself as a range if we didn't specify any
-  // special ranges for this report.
-  auto P = std::make_shared(
-  L, BR.getDescription(), Ranges.begin() == Ranges.end());
-  for (SourceRange Range : Ranges)
-P->addRange(Range);
-
-  return P;
-}
-
 /// \return name of the macro inside the location \p Loc.
 static StringRef getMacroName(SourceLocation Loc,
 BugReporterContext ) {
@@ -251,36 +220,70 @@
 }
 
 /// \return Whether \c RegionOfInterest was modified at \p N,
-/// where \p ReturnState is a state associated with the return
-/// from the current frame.
-static bool wasRegionOfInterestModifiedAt(
-const SubRegion *RegionOfInterest,
-const ExplodedNode *N,
-SVal ValueAfter) {
+/// where \p ValueAfter is \c RegionOfInterest's value at the end of the
+/// stack frame.
+static bool wasRegionOfInterestModifiedAt(const SubRegion *RegionOfInterest,
+  const ExplodedNode *N,
+  SVal ValueAfter) {
   ProgramStateRef State = N->getState();
   ProgramStateManager  = N->getState()->getStateManager();
 
-  if (!N->getLocationAs()
-  && !N->getLocationAs()
-  && !N->getLocationAs())
+  if (!N->getLocationAs() && !N->getLocationAs() &&
+  !N->getLocationAs())
 return false;
 
   // Writing into region of interest.
   if (auto PS = N->getLocationAs())
 if (auto *BO = PS->getStmtAs())
   if (BO->isAssignmentOp() && RegionOfInterest->isSubRegionOf(
-N->getSVal(BO->getLHS()).getAsRegion()))
+  N->getSVal(BO->getLHS()).getAsRegion()))
 return true;
 
   // SVal after the state is possibly different.
   SVal ValueAtN = N->getState()->getSVal(RegionOfInterest);
-  if (!Mgr.getSValBuilder().areEqual(State, ValueAtN, ValueAfter).isConstrainedTrue() &&
+  if (!Mgr.getSValBuilder()
+   .areEqual(State, ValueAtN, ValueAfter)
+   .isConstrainedTrue() &&
   (!ValueAtN.isUndef() || !ValueAfter.isUndef()))
 return true;
 
   return false;
 }
 
+//===--===//
+// Implementation of BugReporterVisitor.
+//===--===//
+
+std::shared_ptr
+BugReporterVisitor::getEndPath(BugReporterContext &,
+   const ExplodedNode *, BugReport &) {
+  return nullptr;
+}
+
+void
+BugReporterVisitor::finalizeVisitor(BugReporterContext &,
+const ExplodedNode *, BugReport &) {}
+
+std::shared_ptr BugReporterVisitor::getDefaultEndPath(
+BugReporterContext , const ExplodedNode *EndPathNode, BugReport ) {
+  PathDiagnosticLocation L =
+PathDiagnosticLocation::createEndOfPath(EndPathNode,BRC.getSourceManager());
+
+  const auto  = BR.getRanges();
+
+  // Only add the statement itself as a range if we didn't specify any
+  // special ranges for this report.
+  auto P = std::make_shared(
+  L, BR.getDescription(), Ranges.begin() == Ranges.end());
+  for (SourceRange Range : Ranges)
+P->addRange(Range);

[PATCH] D61749: [clang-tidy] initial version of readability-static-const-method

[Matthias Gehre via Phabricator via cfe-commits]

mgehre added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61749



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

[PATCH] D63085: Provide a fix-it hint for -Wswitch, which adds missing cases. If there are >3 cases, the inserted text will contain newlines so it will not be shown in console output (but will be appl

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.

Lifted the helper for spelling one scope from another from
SemaCodeComplete -> AST, to reuse it. It should be useful in other contexts too.


Repository:
  rC Clang

https://reviews.llvm.org/D63085

Files:
  include/clang/AST/DeclBase.h
  lib/AST/DeclBase.cpp
  lib/Sema/SemaCodeComplete.cpp
  lib/Sema/SemaLookup.cpp
  lib/Sema/SemaStmt.cpp
  test/FixIt/fixit-enum.cpp

Index: test/FixIt/fixit-enum.cpp
===
--- /dev/null
+++ test/FixIt/fixit-enum.cpp
@@ -0,0 +1,19 @@
+// RUN: cp %s %t.cpp
+// RUN: not %clang_cc1 -fixit %t.cpp -Werror
+// RUN: %clang_cc1 %t.cpp -Werror
+
+namespace ns {
+inline namespace q { namespace { enum { A, B, C }; } }
+} // namespace ns
+
+namespace ns2 {
+enum class F { X, Y, Z };
+
+void test(F f, decltype(ns::A) g) {
+  switch (f) {}
+  // CHECK: YYY
+
+  switch (g) {}
+  // CHECK: ZZZ
+}
+} // namespace ns2
Index: lib/Sema/SemaStmt.cpp
===
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -1193,14 +1193,31 @@
 
   // Produce a nice diagnostic if multiple values aren't handled.
   if (!UnhandledNames.empty()) {
-DiagnosticBuilder DB = Diag(CondExpr->getExprLoc(),
-TheDefaultStmt ? diag::warn_def_missing_case
-   : diag::warn_missing_case)
-   << (int)UnhandledNames.size();
+DiagnosticBuilder DB =
+Diag(CondExpr->getExprLoc(), TheDefaultStmt
+ ? diag::warn_def_missing_case
+ : diag::warn_missing_case)
+<< (int)UnhandledNames.size();
 
 for (size_t I = 0, E = std::min(UnhandledNames.size(), (size_t)3);
  I != E; ++I)
   DB << UnhandledNames[I];
+
+std::string NewCases;
+llvm::raw_string_ostream NewCasesOS(NewCases);
+// Include \n to separate new cases added if there are many.
+// This suppresses printing the (long) insert text.
+char Sep = UnhandledNames.size() > 3 ? '\n' : ' ';
+const NestedNameSpecifier *Qual =
+CurContext->getQualifierFor(ED->isScoped() ? ED : ED->getParent());
+for (const auto& Name : UnhandledNames) {
+  NewCasesOS << "case ";
+  if (Qual)
+Qual->print(NewCasesOS, Context.getPrintingPolicy());
+  NewCasesOS << Name << ':' << Sep;
+}
+NewCasesOS << "break;";
+DB << FixItHint::CreateInsertion(SS->getEndLoc(), NewCasesOS.str());
   }
 
   if (!hasCasesNotInSwitch)
Index: lib/Sema/SemaLookup.cpp
===
--- lib/Sema/SemaLookup.cpp
+++ lib/Sema/SemaLookup.cpp
@@ -20,6 +20,7 @@
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/NestedNameSpecifier.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Lex/HeaderSearch.h"
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -648,50 +648,6 @@
   return iterator(DeclOrVector.get()->end());
 }
 
-/// Compute the qualification required to get from the current context
-/// (\p CurContext) to the target context (\p TargetContext).
-///
-/// \param Context the AST context in which the qualification will be used.
-///
-/// \param CurContext the context where an entity is being named, which is
-/// typically based on the current scope.
-///
-/// \param TargetContext the context in which the named entity actually
-/// resides.
-///
-/// \returns a nested name specifier that refers into the target context, or
-/// NULL if no qualification is needed.
-static NestedNameSpecifier *
-getRequiredQualification(ASTContext , const DeclContext *CurContext,
- const DeclContext *TargetContext) {
-  SmallVector TargetParents;
-
-  for (const DeclContext *CommonAncestor = TargetContext;
-   CommonAncestor && !CommonAncestor->Encloses(CurContext);
-   CommonAncestor = CommonAncestor->getLookupParent()) {
-if (CommonAncestor->isTransparentContext() ||
-CommonAncestor->isFunctionOrMethod())
-  continue;
-
-TargetParents.push_back(CommonAncestor);
-  }
-
-  NestedNameSpecifier *Result = nullptr;
-  while (!TargetParents.empty()) {
-const DeclContext *Parent = TargetParents.pop_back_val();
-
-if (const auto *Namespace = dyn_cast(Parent)) {
-  if (!Namespace->getIdentifier())
-continue;
-
-  Result = NestedNameSpecifier::Create(Context, Result, Namespace);
-} else if 

[PATCH] D63048: Update __VERSION__ to remove the hardcoded 4.2.1 version

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a subscriber: dexonsmith.
rnk added a comment.

Personally, I think it's safe to do this. I believe this "4.2.1" compat thing 
was something Apple added as part of their switch from GCC, and then llvm-gcc, 
over to Clang, and I think most of the code they care about probably doesn't 
rely on `__VERSION__` anymore. In any case, we should let @dexonsmith know.

As an alternative, maybe we should just stop defining the `__VERSION__` macro. 
It's ambiguous. What is it the version of? The compiler, or the standard 
library? What gives the compiler the right to take some identifier, even in the 
implementer's namespace, and use it without putting it in their namespace by 
adding `clang` or `GNUC` to it? We already define `__clang_version__`, which 
has the same information.




Comment at: lib/Frontend/InitPreprocessor.cpp:610
+  Builder.defineMacro("__VERSION__", "\"" CLANG_VERSION_STRING
+  " Compatible " + Twine(getClangFullCPPVersion()) +
+  "\"");

sylvestre.ledru wrote:
> lebedev.ri wrote:
> > So how does the entire `__VERSION__` macro looks like now?
> ```
> #define __VERSION__ "9.0.0 Compatible Clang 9.0.0 (trunk 362877)"
> 
> ```
> instead of
> 
> ```
> #define __VERSION__ "4.2.1 Compatible Clang 9.0.0 (trunk 362877)"
> ```
> 
> 
> 
> ```
> clang -dM -E -xc - < /dev/null 
> ```
> to see it
If we're going to make this change, I don't think we need to say `$VER 
Compatible Clang $VER`, we can just say `Clang $VER (trunk $REVISION)`, so this 
can be simplified to `getClangFullCPPVersion()`. For a vendor like Apple, it 
will become `Apple Clang N.M.Q`, which is probably fine. @dexonsmith 


Repository:
  rC Clang

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

https://reviews.llvm.org/D63048



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

[PATCH] D62962: Clang implementation of sizeless types

[JF Bastien via Phabricator via cfe-commits]

jfb added inline comments.



Comment at: test/Sema/sizeless-1.c:66
+  _Static_assert(__atomic_always_lock_free(1, _int8) == 
__atomic_always_lock_free(1, incomplete_ptr), "");
+  _Static_assert(__atomic_always_lock_free(2, _int8) == 
__atomic_always_lock_free(2, incomplete_ptr), "");
+

rsandifo-arm wrote:
> jfb wrote:
> > I expect sizeless types are never lock free.
> Yeah, that's right.  But AIUI `__atomic_always_lock_free(N, P)` asks whether 
> an N-byte access at P is lock-free (where P can be null to query standard 
> alignment).  So the question isn't whether sizeless types are lock-free, but 
> whether an N-byte access is lock-free given the alignment guarantees of P.  
> For this line the answer would be yes if `_int8` was aligned to a 
> 2-byte boundary.
> 
> The query isn't really that interesting for sizeless types.  The reason for 
> having it is that `IntExprEvaluator::VisitBuiltinCallExpr` says that 1-byte 
> accesses are lock-free if the target has lock-free accesses, whatever P 
> happens to be.  But for larger N it punts when P is a pointer to incomplete 
> type, because in that case it knows nothing about the alignment of P.  The 
> test is enforcing this behaviour for sizeless types too.
Ah yes, that makes sense. Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D62962



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

[PATCH] D63072: [clang] Fixing incorrect implicit deduction guides (PR41549)

[Tyker via Phabricator via cfe-commits]

Tyker updated this revision to Diff 203862.
Tyker added a comment.

fixed the issue.


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

https://reviews.llvm.org/D63072

Files:
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp


Index: clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
===
--- clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
+++ clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
@@ -489,6 +489,21 @@
 }
 #pragma clang diagnostic pop
 
+namespace PR41549 {
+
+template  struct umm;
+
+template 
+struct umm {
+  umm(H h = 0, P p = 0);
+};
+
+template  struct umm;
+
+umm m(1);
+
+}
+
 #else
 
 // expected-no-diagnostics
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -2052,6 +2052,12 @@
 
 void Sema::DeclareImplicitDeductionGuides(TemplateDecl *Template,
   SourceLocation Loc) {
+  if (CXXRecordDecl *DefRecord =
+  cast(Template->getTemplatedDecl())->getDefinition()) {
+TemplateDecl *DescribedTemplate = DefRecord->getDescribedClassTemplate();
+Template = DescribedTemplate ? DescribedTemplate : Template;
+  }
+
   DeclContext *DC = Template->getDeclContext();
   if (DC->isDependentContext())
 return;


Index: clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
===
--- clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
+++ clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
@@ -489,6 +489,21 @@
 }
 #pragma clang diagnostic pop
 
+namespace PR41549 {
+
+template  struct umm;
+
+template 
+struct umm {
+  umm(H h = 0, P p = 0);
+};
+
+template  struct umm;
+
+umm m(1);
+
+}
+
 #else
 
 // expected-no-diagnostics
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -2052,6 +2052,12 @@
 
 void Sema::DeclareImplicitDeductionGuides(TemplateDecl *Template,
   SourceLocation Loc) {
+  if (CXXRecordDecl *DefRecord =
+  cast(Template->getTemplatedDecl())->getDefinition()) {
+TemplateDecl *DescribedTemplate = DefRecord->getDescribedClassTemplate();
+Template = DescribedTemplate ? DescribedTemplate : Template;
+  }
+
   DeclContext *DC = Template->getDeclContext();
   if (DC->isDependentContext())
 return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62962: Clang implementation of sizeless types

[Richard Sandiford via Phabricator via cfe-commits]

rsandifo-arm marked 2 inline comments as done.
rsandifo-arm added inline comments.



Comment at: test/Sema/sizeless-1.c:66
+  _Static_assert(__atomic_always_lock_free(1, _int8) == 
__atomic_always_lock_free(1, incomplete_ptr), "");
+  _Static_assert(__atomic_always_lock_free(2, _int8) == 
__atomic_always_lock_free(2, incomplete_ptr), "");
+

jfb wrote:
> I expect sizeless types are never lock free.
Yeah, that's right.  But AIUI `__atomic_always_lock_free(N, P)` asks whether an 
N-byte access at P is lock-free (where P can be null to query standard 
alignment).  So the question isn't whether sizeless types are lock-free, but 
whether an N-byte access is lock-free given the alignment guarantees of P.  For 
this line the answer would be yes if `_int8` was aligned to a 2-byte 
boundary.

The query isn't really that interesting for sizeless types.  The reason for 
having it is that `IntExprEvaluator::VisitBuiltinCallExpr` says that 1-byte 
accesses are lock-free if the target has lock-free accesses, whatever P happens 
to be.  But for larger N it punts when P is a pointer to incomplete type, 
because in that case it knows nothing about the alignment of P.  The test is 
enforcing this behaviour for sizeless types too.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62962



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

[PATCH] D63062: [clang-format] Added New Style Rule: BitFieldDeclsOnSeparateLines

[Manikishan Ghantasala via Phabricator via cfe-commits]

Manikishan updated this revision to Diff 203860.
Manikishan marked 4 inline comments as done.
Manikishan added a comment.

Updated unittest


Repository:
  rC Clang

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

https://reviews.llvm.org/D63062

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

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -3656,6 +3656,19 @@
"#define A Just forcing a new line\n"
"ddd);");
 }
+TEST_F(FormatTest, AlignBitFieldDeclarationsOnConsecutiveLines){
+  FormatStyle Style = {};
+  Style.BitFieldDeclsOnSeparateLines = true;
+  verifyFormat(
+"unsigned int baz : 11,
+  aaa : 2,
+  foo : 3"
+  );
+  Style.BitFieldDeclsOnSeparateLines = false;
+  verifyFormat(
+"unsigned int baz : 11, aaa : 2, foo : 3"
+  );
+} 
 
 TEST_F(FormatTest, LineBreakingInBinaryExpressions) {
   verifyFormat(
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2916,6 +2916,9 @@
 bool TokenAnnotator::mustBreakBefore(const AnnotatedLine ,
  const FormatToken ) {
   const FormatToken  = *Right.Previous;
+  if (Right.Previous->is(tok::comma) && Style.BitFieldDeclsOnSeparateLines &&
+  Right.is(tok::identifier) && (Right.Next->is(tok::colon)))
+return true;
   if (Right.NewlinesBefore > 1 && Style.MaxEmptyLinesToKeep > 0)
 return true;
 
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -451,6 +451,7 @@
 IO.mapOptional("JavaScriptWrapImports", Style.JavaScriptWrapImports);
 IO.mapOptional("KeepEmptyLinesAtTheStartOfBlocks",
Style.KeepEmptyLinesAtTheStartOfBlocks);
+IO.mapOptional("BitFieldDeclsOnSeparateLines", Style.BitFieldDeclsOnSeparateLines);
 IO.mapOptional("MacroBlockBegin", Style.MacroBlockBegin);
 IO.mapOptional("MacroBlockEnd", Style.MacroBlockEnd);
 IO.mapOptional("MaxEmptyLinesToKeep", Style.MaxEmptyLinesToKeep);
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -278,6 +278,9 @@
   const FormatToken  = *State.NextToken;
   const FormatToken  = *Current.Previous;
   assert( == Current.Previous);
+  if (Previous.is(tok::comma) && Style.BitFieldDeclsOnSeparateLines &&
+  Current.is(tok::identifier) && Current.Next->is(tok::colon))
+return true;
   if (!Current.CanBreakBefore && !(State.Stack.back().BreakBeforeClosingBrace &&
Current.closesBlockOrBlockTypeList(Style)))
 return false;
@@ -329,6 +332,9 @@
 bool ContinuationIndenter::mustBreak(const LineState ) {
   const FormatToken  = *State.NextToken;
   const FormatToken  = *Current.Previous;
+  if (Previous.is(tok::comma) && Style.BitFieldDeclsOnSeparateLines &&
+  Current.is(tok::identifier) && Current.Next->is(tok::colon))
+return true; 
   if (Current.MustBreakBefore || Current.is(TT_InlineASMColon))
 return true;
   if (State.Stack.back().BreakBeforeClosingBrace &&
@@ -541,7 +547,7 @@
  unsigned ExtraSpaces) {
   FormatToken  = *State.NextToken;
   const FormatToken  = *State.NextToken->Previous;
-  if (Current.is(tok::equal) &&
+  if (Current.isOneOf(tok::equal, tok::colon) &&
   (State.Line->First->is(tok::kw_for) || Current.NestingLevel == 0) &&
   State.Stack.back().VariablePos == 0) {
 State.Stack.back().VariablePos = State.Column;
Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -101,6 +101,16 @@
   /// \endcode
   bool AlignConsecutiveDeclarations;
 
+  /// If ``true``, Linesup Bitfield Declarations.
+  /// This will lineup Bitfield declarations on consecutive lines. This
+  /// will result in formatting like
+  /// \code
+  ///  unsigned int  baz : 1, /* Bitfield; line up entries if desire*/
+  ///fuz : 5,
+  ///zap : 2;
+  /// \endcode
+  bool BitFieldDeclsOnSeparateLines;
+
   /// Different styles for aligning escaped newlines.
   enum EscapedNewlineAlignmentStyle {
 /// Don't align escaped newlines.
@@ -1950,6 +1960,7 @@
JavaScriptWrapImports == R.JavaScriptWrapImports &&
KeepEmptyLinesAtTheStartOfBlocks ==
R.KeepEmptyLinesAtTheStartOfBlocks &&
+   BitFieldDeclsOnSeparateLines == 

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: llvm/cmake/modules/AddLLVM.cmake:812
+#   llvm::PassPluginLibraryInfo ${entry_point}();
+add_custom_target(LLVM_PLUGINS)  # target used to hold global properties 
referencable from generator-expression
+function(register_llvm_extension llvm_extension entry_point)

Change this to `llvm-plugins` to match our convention and wrap it in `if (NOT 
TARGET...)` so it doesn't error if AddLLVM is included twice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446



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

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Lingda Li via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8729
+/// \code
+/// void .omp_mapper...(void *rt_mapper_handle,
+///   void *base, void *begin,

ABataev wrote:
> lildmh wrote:
> > ABataev wrote:
> > > lildmh wrote:
> > > > ABataev wrote:
> > > > > lildmh wrote:
> > > > > > ABataev wrote:
> > > > > > > This function looks like the universal one, regardless of the 
> > > > > > > type `` specifics. Do we really need to generate it 
> > > > > > > for each particular type and mapper? Or we could use the same 
> > > > > > > function for all types/mappers?
> > > > > > I think we need a particular mapper function for each type and 
> > > > > > mapper, because the code generated within the mapper function 
> > > > > > depends on what type and what mapper it is.
> > > > > Hmm, maybe I'm wrong but I don't see significant mapper or 
> > > > > type-specific dependencies in this mapper function. It uses the 
> > > > > pointer to type and size of the type, but this information can be 
> > > > > generalized, I think. Could you point the lines of code that are type 
> > > > > and mapper specific?
> > > > Code between line 8857-8965 is type and mapper specific. For instance, 
> > > > `generateAllInforForMapper` depends on the map clauses associated with 
> > > > the mapper and the internal structure of struct/class type, and 
> > > > generates difference code as a result. `BasePointers.size()` also 
> > > > depends on the above things.
> > > Most of these data can be passed as parameters to the function. It would 
> > > be good, if we could move this function to the libomptaret library and 
> > > reduce the number of changes (and, thus, complexity) of the compiler 
> > > itself. It is always easier to review and to maintain the source code 
> > > written in C/C++ rather than the changes in the compiler codegen. Plus, 
> > > it may reduce the size of the final code significantly, I assume. I would 
> > > appreciate it if you would try to move this function to libomptarget.
> > Hi Alexey, I don't think libomptarget can do this efficiently. For example,
> > 
> > ```
> > class C {
> >   int a;
> >   double *b
> > }
> > 
> > #pragma omp declare mapper(C c) map(c.a, c.b[0:1])
> > ```
> > 
> > The codegen can directly know there are 2 components (c.a, c.b[0]) in this 
> > mapper function (3 actually when we count the pointer), and it can also 
> > know the size, starting address, map type, etc. about these components. 
> > Passing all these information to libomptarget seems to be a bad idea. Or 
> > did I get your idea wrong?
> > 
> Yes, I understand this. But can we pass some additional parameters to this 
> function so we don't need to generate a unique copy of almost the same 
> function for all types/mappers? 
For different types/mappers, the skeleton of mapper functions are similar 
(i.e., the things outlined in the comment here). I would say most other code is 
unique, for instance, the code to prepare parameters of call to 
`__tgt_push_mapper_component`. These code should be much more compared with the 
skeleton shown here. I cannot think of a way to reduce the code by passing more 
parameters to this function. Please let me know if you have some suggestions.


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

https://reviews.llvm.org/D59474



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

[PATCH] D62960: SVE opaque type for C intrinsics demo

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added inline comments.



Comment at: lib/AST/ItaniumMangle.cpp:2680
+break;
+#include "clang/Basic/AArch64SVEACLETypes.def"
   }

jfb wrote:
> @rjmccall you probably should review this part.
Sorry for the drive by comment, but: All of these mangling should really be 
using the "vendor extension" production IMO:

` ::= u `

As is, these manglings intrude on the users's namespace, (i.e. if they had a 
type named `objc_selector` or something), and confuse demanglers which 
incorrectly assume these are substitutable (vendor extension builtin types are 
substitutable too though, but that should be handled here).


Repository:
  rC Clang

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

https://reviews.llvm.org/D62960



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

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8729
+/// \code
+/// void .omp_mapper...(void *rt_mapper_handle,
+///   void *base, void *begin,

lildmh wrote:
> ABataev wrote:
> > lildmh wrote:
> > > ABataev wrote:
> > > > lildmh wrote:
> > > > > ABataev wrote:
> > > > > > This function looks like the universal one, regardless of the type 
> > > > > > `` specifics. Do we really need to generate it for each 
> > > > > > particular type and mapper? Or we could use the same function for 
> > > > > > all types/mappers?
> > > > > I think we need a particular mapper function for each type and 
> > > > > mapper, because the code generated within the mapper function depends 
> > > > > on what type and what mapper it is.
> > > > Hmm, maybe I'm wrong but I don't see significant mapper or 
> > > > type-specific dependencies in this mapper function. It uses the pointer 
> > > > to type and size of the type, but this information can be generalized, 
> > > > I think. Could you point the lines of code that are type and mapper 
> > > > specific?
> > > Code between line 8857-8965 is type and mapper specific. For instance, 
> > > `generateAllInforForMapper` depends on the map clauses associated with 
> > > the mapper and the internal structure of struct/class type, and generates 
> > > difference code as a result. `BasePointers.size()` also depends on the 
> > > above things.
> > Most of these data can be passed as parameters to the function. It would be 
> > good, if we could move this function to the libomptaret library and reduce 
> > the number of changes (and, thus, complexity) of the compiler itself. It is 
> > always easier to review and to maintain the source code written in C/C++ 
> > rather than the changes in the compiler codegen. Plus, it may reduce the 
> > size of the final code significantly, I assume. I would appreciate it if 
> > you would try to move this function to libomptarget.
> Hi Alexey, I don't think libomptarget can do this efficiently. For example,
> 
> ```
> class C {
>   int a;
>   double *b
> }
> 
> #pragma omp declare mapper(C c) map(c.a, c.b[0:1])
> ```
> 
> The codegen can directly know there are 2 components (c.a, c.b[0]) in this 
> mapper function (3 actually when we count the pointer), and it can also know 
> the size, starting address, map type, etc. about these components. Passing 
> all these information to libomptarget seems to be a bad idea. Or did I get 
> your idea wrong?
> 
Yes, I understand this. But can we pass some additional parameters to this 
function so we don't need to generate a unique copy of almost the same function 
for all types/mappers? 


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

https://reviews.llvm.org/D59474



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

[PATCH] D63041: [PlistSupport] Produce a newline to end plist output files

[Xing Xue via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63041



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

[PATCH] D62960: SVE opaque type for C intrinsics demo

[JF Bastien via Phabricator via cfe-commits]

jfb added subscribers: rjmccall, jfb.
jfb added a comment.

Tests?




Comment at: lib/AST/ItaniumMangle.cpp:2680
+break;
+#include "clang/Basic/AArch64SVEACLETypes.def"
   }

@rjmccall you probably should review this part.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62960



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

[PATCH] D62962: Clang implementation of sizeless types

[JF Bastien via Phabricator via cfe-commits]

jfb added inline comments.



Comment at: test/Sema/sizeless-1.c:66
+  _Static_assert(__atomic_always_lock_free(1, _int8) == 
__atomic_always_lock_free(1, incomplete_ptr), "");
+  _Static_assert(__atomic_always_lock_free(2, _int8) == 
__atomic_always_lock_free(2, incomplete_ptr), "");
+

I expect sizeless types are never lock free.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62962



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

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Lingda Li via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8729
+/// \code
+/// void .omp_mapper...(void *rt_mapper_handle,
+///   void *base, void *begin,

ABataev wrote:
> lildmh wrote:
> > ABataev wrote:
> > > lildmh wrote:
> > > > ABataev wrote:
> > > > > This function looks like the universal one, regardless of the type 
> > > > > `` specifics. Do we really need to generate it for each 
> > > > > particular type and mapper? Or we could use the same function for all 
> > > > > types/mappers?
> > > > I think we need a particular mapper function for each type and mapper, 
> > > > because the code generated within the mapper function depends on what 
> > > > type and what mapper it is.
> > > Hmm, maybe I'm wrong but I don't see significant mapper or type-specific 
> > > dependencies in this mapper function. It uses the pointer to type and 
> > > size of the type, but this information can be generalized, I think. Could 
> > > you point the lines of code that are type and mapper specific?
> > Code between line 8857-8965 is type and mapper specific. For instance, 
> > `generateAllInforForMapper` depends on the map clauses associated with the 
> > mapper and the internal structure of struct/class type, and generates 
> > difference code as a result. `BasePointers.size()` also depends on the 
> > above things.
> Most of these data can be passed as parameters to the function. It would be 
> good, if we could move this function to the libomptaret library and reduce 
> the number of changes (and, thus, complexity) of the compiler itself. It is 
> always easier to review and to maintain the source code written in C/C++ 
> rather than the changes in the compiler codegen. Plus, it may reduce the size 
> of the final code significantly, I assume. I would appreciate it if you would 
> try to move this function to libomptarget.
Hi Alexey, I don't think libomptarget can do this efficiently. For example,

```
class C {
  int a;
  double *b
}

#pragma omp declare mapper(C c) map(c.a, c.b[0:1])
```

The codegen can directly know there are 2 components (c.a, c.b[0]) in this 
mapper function (3 actually when we count the pointer), and it can also know 
the size, starting address, map type, etc. about these components. Passing all 
these information to libomptarget seems to be a bad idea. Or did I get your 
idea wrong?



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

https://reviews.llvm.org/D59474



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

[PATCH] D61027: Fix crash on switch conditions of non-integer types in templates

[Elizabeth Andrews via Phabricator via cfe-commits]

eandrews updated this revision to Diff 203854.
eandrews edited the summary of this revision.
eandrews added a reviewer: rsmith.

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

https://reviews.llvm.org/D61027

Files:
  lib/AST/Expr.cpp
  lib/Sema/SemaChecking.cpp
  test/SemaTemplate/dependent-names.cpp
  test/SemaTemplate/enum-argument.cpp
  test/SemaTemplate/member-access-expr.cpp
  test/SemaTemplate/non-integral-switch-cond.cpp


Index: test/SemaTemplate/non-integral-switch-cond.cpp
===
--- /dev/null
+++ test/SemaTemplate/non-integral-switch-cond.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct NOT_AN_INTEGRAL_TYPE {};
+
+template 
+struct foo {
+  NOT_AN_INTEGRAL_TYPE Bad;
+  void run() {
+switch (Bad) { // expected-error {{statement requires expression of 
integer type ('NOT_AN_INTEGRAL_TYPE' invalid)}}
+case 0:
+  break;
+}
+  }
+};
Index: test/SemaTemplate/member-access-expr.cpp
===
--- test/SemaTemplate/member-access-expr.cpp
+++ test/SemaTemplate/member-access-expr.cpp
@@ -156,7 +156,7 @@
 void get(B **ptr) {
   // It's okay if at some point we figure out how to diagnose this
   // at instantiation time.
-  *ptr = field;
+  *ptr = field; // expected-error {{assigning to 'test6::B *' from 
incompatible type 'test6::A *}}
 }
   };
 }
Index: test/SemaTemplate/enum-argument.cpp
===
--- test/SemaTemplate/enum-argument.cpp
+++ test/SemaTemplate/enum-argument.cpp
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
-// expected-no-diagnostics
 
 enum Enum { val = 1 };
 template  struct C {
@@ -31,7 +30,7 @@
 unsigned long long bitfield : e0;
 
 void f(int j) {
-  bitfield + j;
+  bitfield + j; // expected-warning {{expression result unused}}
 }
   };
 }
Index: test/SemaTemplate/dependent-names.cpp
===
--- test/SemaTemplate/dependent-names.cpp
+++ test/SemaTemplate/dependent-names.cpp
@@ -273,9 +273,6 @@
   }
   int e[10];
 };
-void g() {
-  S().f(); // expected-note {{here}}
-}
   }
 
   namespace A2 {
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -14125,6 +14125,8 @@
   bool AnyIsPacked = false;
   do {
 QualType BaseType = ME->getBase()->getType();
+if (BaseType->isDependentType())
+  return;
 if (ME->isArrow())
   BaseType = BaseType->getPointeeType();
 RecordDecl *RD = BaseType->getAs()->getDecl();
Index: lib/AST/Expr.cpp
===
--- lib/AST/Expr.cpp
+++ lib/AST/Expr.cpp
@@ -1560,6 +1560,13 @@
   MemberExpr *E = new (Mem)
   MemberExpr(base, isarrow, OperatorLoc, memberdecl, nameinfo, ty, vk, ok);
 
+  if (!isa(memberdecl)) {
+DeclContext *DC = memberdecl->getDeclContext();
+CXXRecordDecl *RD = dyn_cast(DC);
+if (RD && RD->isDependentContext() && RD->isCurrentInstantiation(DC))
+  E->setTypeDependent(ty->isDependentType());
+  }
+
   if (hasQualOrFound) {
 // FIXME: Wrong. We should be looking at the member declaration we found.
 if (QualifierLoc && QualifierLoc.getNestedNameSpecifier()->isDependent()) {


Index: test/SemaTemplate/non-integral-switch-cond.cpp
===
--- /dev/null
+++ test/SemaTemplate/non-integral-switch-cond.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct NOT_AN_INTEGRAL_TYPE {};
+
+template 
+struct foo {
+  NOT_AN_INTEGRAL_TYPE Bad;
+  void run() {
+switch (Bad) { // expected-error {{statement requires expression of integer type ('NOT_AN_INTEGRAL_TYPE' invalid)}}
+case 0:
+  break;
+}
+  }
+};
Index: test/SemaTemplate/member-access-expr.cpp
===
--- test/SemaTemplate/member-access-expr.cpp
+++ test/SemaTemplate/member-access-expr.cpp
@@ -156,7 +156,7 @@
 void get(B **ptr) {
   // It's okay if at some point we figure out how to diagnose this
   // at instantiation time.
-  *ptr = field;
+  *ptr = field; // expected-error {{assigning to 'test6::B *' from incompatible type 'test6::A *}}
 }
   };
 }
Index: test/SemaTemplate/enum-argument.cpp
===
--- test/SemaTemplate/enum-argument.cpp
+++ test/SemaTemplate/enum-argument.cpp
@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
-// expected-no-diagnostics
 
 enum Enum { val = 1 };
 template  struct C {
@@ -31,7 +30,7 @@
 unsigned long long bitfield : e0;
 
 void f(int j) {
-  bitfield + j;
+  bitfield + j; // expected-warning {{expression result 

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8729
+/// \code
+/// void .omp_mapper...(void *rt_mapper_handle,
+///   void *base, void *begin,

lildmh wrote:
> ABataev wrote:
> > lildmh wrote:
> > > ABataev wrote:
> > > > This function looks like the universal one, regardless of the type 
> > > > `` specifics. Do we really need to generate it for each 
> > > > particular type and mapper? Or we could use the same function for all 
> > > > types/mappers?
> > > I think we need a particular mapper function for each type and mapper, 
> > > because the code generated within the mapper function depends on what 
> > > type and what mapper it is.
> > Hmm, maybe I'm wrong but I don't see significant mapper or type-specific 
> > dependencies in this mapper function. It uses the pointer to type and size 
> > of the type, but this information can be generalized, I think. Could you 
> > point the lines of code that are type and mapper specific?
> Code between line 8857-8965 is type and mapper specific. For instance, 
> `generateAllInforForMapper` depends on the map clauses associated with the 
> mapper and the internal structure of struct/class type, and generates 
> difference code as a result. `BasePointers.size()` also depends on the above 
> things.
Most of these data can be passed as parameters to the function. It would be 
good, if we could move this function to the libomptaret library and reduce the 
number of changes (and, thus, complexity) of the compiler itself. It is always 
easier to review and to maintain the source code written in C/C++ rather than 
the changes in the compiler codegen. Plus, it may reduce the size of the final 
code significantly, I assume. I would appreciate it if you would try to move 
this function to libomptarget.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8857
+  MappableExprsHandler MEHandler(*D, MapperCGF);
+  MEHandler.generateAllInfoForMapper(BasePointers, Pointers, Sizes, MapTypes);
+

These data can be passed as parameters to the function, no?


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

https://reviews.llvm.org/D59474



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

[PATCH] D63018: [X86] Attempt to make the Intel core CPU inheritance a little more readable and maintainable

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL362965: [X86] Attempt to make the Intel core CPU inheritance 
a little more readable and… (authored by ctopper, committed by ).
Herald added a project: LLVM.

Changed prior to commit:
  https://reviews.llvm.org/D63018?vs=203597=203852#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63018

Files:
  cfe/trunk/lib/Basic/Targets/X86.cpp


Index: cfe/trunk/lib/Basic/Targets/X86.cpp
===
--- cfe/trunk/lib/Basic/Targets/X86.cpp
+++ cfe/trunk/lib/Basic/Targets/X86.cpp
@@ -138,6 +138,25 @@
 setFeatureEnabledImpl(Features, "mmx", true);
 break;
 
+  case CK_Cooperlake:
+// CPX inherits all CLX features plus AVX512BF16
+setFeatureEnabledImpl(Features, "avx512bf16", true);
+LLVM_FALLTHROUGH;
+  case CK_Cascadelake:
+// CLX inherits all SKX features plus AVX512VNNI
+setFeatureEnabledImpl(Features, "avx512vnni", true);
+LLVM_FALLTHROUGH;
+  case CK_SkylakeServer:
+setFeatureEnabledImpl(Features, "avx512f", true);
+setFeatureEnabledImpl(Features, "avx512cd", true);
+setFeatureEnabledImpl(Features, "avx512dq", true);
+setFeatureEnabledImpl(Features, "avx512bw", true);
+setFeatureEnabledImpl(Features, "avx512vl", true);
+setFeatureEnabledImpl(Features, "clwb", true);
+setFeatureEnabledImpl(Features, "pku", true);
+// SkylakeServer cores inherits all SKL features, except SGX
+goto SkylakeCommon;
+
   case CK_IcelakeServer:
 setFeatureEnabledImpl(Features, "pconfig", true);
 setFeatureEnabledImpl(Features, "wbnoinvd", true);
@@ -148,45 +167,29 @@
 setFeatureEnabledImpl(Features, "vpclmulqdq", true);
 setFeatureEnabledImpl(Features, "avx512bitalg", true);
 setFeatureEnabledImpl(Features, "avx512vbmi2", true);
+setFeatureEnabledImpl(Features, "avx512vnni", true);
 setFeatureEnabledImpl(Features, "avx512vpopcntdq", true);
 setFeatureEnabledImpl(Features, "rdpid", true);
+setFeatureEnabledImpl(Features, "clwb", true);
 LLVM_FALLTHROUGH;
   case CK_Cannonlake:
-setFeatureEnabledImpl(Features, "avx512ifma", true);
-setFeatureEnabledImpl(Features, "avx512vbmi", true);
-setFeatureEnabledImpl(Features, "sha", true);
-LLVM_FALLTHROUGH;
-  case CK_Cooperlake:
-// Cannonlake, IcelakeClient and IcelakeServer have no AVX512BF16 feature
-if (Kind != CK_Cannonlake && Kind != CK_IcelakeClient &&
-Kind != CK_IcelakeServer)
-  // CPX inherits all CLX features plus AVX512BF16
-  setFeatureEnabledImpl(Features, "avx512bf16", true);
-LLVM_FALLTHROUGH;
-  case CK_Cascadelake:
-//Cannonlake has no VNNI feature inside while Icelake has
-if (Kind != CK_Cannonlake)
-  // CLK inherits all SKX features plus AVX512_VNNI
-  setFeatureEnabledImpl(Features, "avx512vnni", true);
-LLVM_FALLTHROUGH;
-  case CK_SkylakeServer:
 setFeatureEnabledImpl(Features, "avx512f", true);
 setFeatureEnabledImpl(Features, "avx512cd", true);
 setFeatureEnabledImpl(Features, "avx512dq", true);
 setFeatureEnabledImpl(Features, "avx512bw", true);
 setFeatureEnabledImpl(Features, "avx512vl", true);
+setFeatureEnabledImpl(Features, "avx512ifma", true);
+setFeatureEnabledImpl(Features, "avx512vbmi", true);
 setFeatureEnabledImpl(Features, "pku", true);
-if (Kind != CK_Cannonlake) // CNL inherits all SKX features, except CLWB
-  setFeatureEnabledImpl(Features, "clwb", true);
+setFeatureEnabledImpl(Features, "sha", true);
 LLVM_FALLTHROUGH;
   case CK_SkylakeClient:
+setFeatureEnabledImpl(Features, "sgx", true);
+// SkylakeServer cores inherits all SKL features, except SGX
+SkylakeCommon:
 setFeatureEnabledImpl(Features, "xsavec", true);
 setFeatureEnabledImpl(Features, "xsaves", true);
 setFeatureEnabledImpl(Features, "mpx", true);
-if (Kind != CK_SkylakeServer && Kind != CK_Cascadelake &&
-Kind != CK_Cooperlake)
-  // SKX/CLX/CPX inherits all SKL features, except SGX
-  setFeatureEnabledImpl(Features, "sgx", true);
 setFeatureEnabledImpl(Features, "clflushopt", true);
 setFeatureEnabledImpl(Features, "aes", true);
 LLVM_FALLTHROUGH;


Index: cfe/trunk/lib/Basic/Targets/X86.cpp
===
--- cfe/trunk/lib/Basic/Targets/X86.cpp
+++ cfe/trunk/lib/Basic/Targets/X86.cpp
@@ -138,6 +138,25 @@
 setFeatureEnabledImpl(Features, "mmx", true);
 break;
 
+  case CK_Cooperlake:
+// CPX inherits all CLX features plus AVX512BF16
+setFeatureEnabledImpl(Features, "avx512bf16", true);
+LLVM_FALLTHROUGH;
+  case CK_Cascadelake:
+// CLX inherits all SKX features plus AVX512VNNI
+setFeatureEnabledImpl(Features, "avx512vnni", true);
+LLVM_FALLTHROUGH;
+  case CK_SkylakeServer:
+setFeatureEnabledImpl(Features, "avx512f", 

r362965 - [X86] Attempt to make the Intel core CPU inheritance a little more readable and maintainable

[Craig Topper via cfe-commits]

Author: ctopper
Date: Mon Jun 10 09:59:28 2019
New Revision: 362965

URL: http://llvm.org/viewvc/llvm-project?rev=362965=rev
Log:
[X86] Attempt to make the Intel core CPU inheritance a little more readable and 
maintainable

The recently added cooperlake CPU has made our already ugly switch statement 
even worse. There's a CPU exclusion list around the bf16 feature in the cooper 
lake block. I worry that we'll have to keep adding new CPUs to that until bf16 
intercepts a client space CPU. We have several other exclusion lists in other 
parts of the switch due to skylakeserver, cascadelake, and cooperlake not 
having sgx. Another for cannonlake not having clwb but having all other 
features from skx.

This removes all these special ifs at the cost of some duplication of features 
and a goto. I've copied all of the skx features into either cannonlake or 
icelakeclient(for clwb). And pulled sklyakeserver, cascadelake, and cooperlake 
out of the main inheritance chain into their own chain. At the end of 
skylakeserver we merge back into the main chain at skylakeclient but below sgx. 
I think this is at least easier to follow.

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

Modified:
cfe/trunk/lib/Basic/Targets/X86.cpp

Modified: cfe/trunk/lib/Basic/Targets/X86.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/X86.cpp?rev=362965=362964=362965=diff
==
--- cfe/trunk/lib/Basic/Targets/X86.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/X86.cpp Mon Jun 10 09:59:28 2019
@@ -138,6 +138,25 @@ bool X86TargetInfo::initFeatureMap(
 setFeatureEnabledImpl(Features, "mmx", true);
 break;
 
+  case CK_Cooperlake:
+// CPX inherits all CLX features plus AVX512BF16
+setFeatureEnabledImpl(Features, "avx512bf16", true);
+LLVM_FALLTHROUGH;
+  case CK_Cascadelake:
+// CLX inherits all SKX features plus AVX512VNNI
+setFeatureEnabledImpl(Features, "avx512vnni", true);
+LLVM_FALLTHROUGH;
+  case CK_SkylakeServer:
+setFeatureEnabledImpl(Features, "avx512f", true);
+setFeatureEnabledImpl(Features, "avx512cd", true);
+setFeatureEnabledImpl(Features, "avx512dq", true);
+setFeatureEnabledImpl(Features, "avx512bw", true);
+setFeatureEnabledImpl(Features, "avx512vl", true);
+setFeatureEnabledImpl(Features, "clwb", true);
+setFeatureEnabledImpl(Features, "pku", true);
+// SkylakeServer cores inherits all SKL features, except SGX
+goto SkylakeCommon;
+
   case CK_IcelakeServer:
 setFeatureEnabledImpl(Features, "pconfig", true);
 setFeatureEnabledImpl(Features, "wbnoinvd", true);
@@ -148,45 +167,29 @@ bool X86TargetInfo::initFeatureMap(
 setFeatureEnabledImpl(Features, "vpclmulqdq", true);
 setFeatureEnabledImpl(Features, "avx512bitalg", true);
 setFeatureEnabledImpl(Features, "avx512vbmi2", true);
+setFeatureEnabledImpl(Features, "avx512vnni", true);
 setFeatureEnabledImpl(Features, "avx512vpopcntdq", true);
 setFeatureEnabledImpl(Features, "rdpid", true);
+setFeatureEnabledImpl(Features, "clwb", true);
 LLVM_FALLTHROUGH;
   case CK_Cannonlake:
-setFeatureEnabledImpl(Features, "avx512ifma", true);
-setFeatureEnabledImpl(Features, "avx512vbmi", true);
-setFeatureEnabledImpl(Features, "sha", true);
-LLVM_FALLTHROUGH;
-  case CK_Cooperlake:
-// Cannonlake, IcelakeClient and IcelakeServer have no AVX512BF16 feature
-if (Kind != CK_Cannonlake && Kind != CK_IcelakeClient &&
-Kind != CK_IcelakeServer)
-  // CPX inherits all CLX features plus AVX512BF16
-  setFeatureEnabledImpl(Features, "avx512bf16", true);
-LLVM_FALLTHROUGH;
-  case CK_Cascadelake:
-//Cannonlake has no VNNI feature inside while Icelake has
-if (Kind != CK_Cannonlake)
-  // CLK inherits all SKX features plus AVX512_VNNI
-  setFeatureEnabledImpl(Features, "avx512vnni", true);
-LLVM_FALLTHROUGH;
-  case CK_SkylakeServer:
 setFeatureEnabledImpl(Features, "avx512f", true);
 setFeatureEnabledImpl(Features, "avx512cd", true);
 setFeatureEnabledImpl(Features, "avx512dq", true);
 setFeatureEnabledImpl(Features, "avx512bw", true);
 setFeatureEnabledImpl(Features, "avx512vl", true);
+setFeatureEnabledImpl(Features, "avx512ifma", true);
+setFeatureEnabledImpl(Features, "avx512vbmi", true);
 setFeatureEnabledImpl(Features, "pku", true);
-if (Kind != CK_Cannonlake) // CNL inherits all SKX features, except CLWB
-  setFeatureEnabledImpl(Features, "clwb", true);
+setFeatureEnabledImpl(Features, "sha", true);
 LLVM_FALLTHROUGH;
   case CK_SkylakeClient:
+setFeatureEnabledImpl(Features, "sgx", true);
+// SkylakeServer cores inherits all SKL features, except SGX
+SkylakeCommon:
 setFeatureEnabledImpl(Features, "xsavec", true);
 setFeatureEnabledImpl(Features, "xsaves", true);
 setFeatureEnabledImpl(Features, "mpx", true);
-if (Kind != CK_SkylakeServer 

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Lingda Li via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8729
+/// \code
+/// void .omp_mapper...(void *rt_mapper_handle,
+///   void *base, void *begin,

ABataev wrote:
> lildmh wrote:
> > ABataev wrote:
> > > This function looks like the universal one, regardless of the type 
> > > `` specifics. Do we really need to generate it for each 
> > > particular type and mapper? Or we could use the same function for all 
> > > types/mappers?
> > I think we need a particular mapper function for each type and mapper, 
> > because the code generated within the mapper function depends on what type 
> > and what mapper it is.
> Hmm, maybe I'm wrong but I don't see significant mapper or type-specific 
> dependencies in this mapper function. It uses the pointer to type and size of 
> the type, but this information can be generalized, I think. Could you point 
> the lines of code that are type and mapper specific?
Code between line 8857-8965 is type and mapper specific. For instance, 
`generateAllInforForMapper` depends on the map clauses associated with the 
mapper and the internal structure of struct/class type, and generates 
difference code as a result. `BasePointers.size()` also depends on the above 
things.


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

https://reviews.llvm.org/D59474



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

[PATCH] D63081: [WebAssembly] Cleanup toolchain test files. NFC.

[Phabricator via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL362959: [WebAssembly] Cleanup toolchain test files. NFC. 
(authored by sbc, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63081?vs=203836=203842#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63081

Files:
  cfe/trunk/test/Driver/wasm-toolchain.c
  cfe/trunk/test/Driver/wasm-toolchain.cpp

Index: cfe/trunk/test/Driver/wasm-toolchain.cpp
===
--- cfe/trunk/test/Driver/wasm-toolchain.cpp
+++ cfe/trunk/test/Driver/wasm-toolchain.cpp
@@ -1,40 +1,52 @@
 // A basic clang -cc1 command-line. WebAssembly is somewhat special in
 // enabling -fvisibility=hidden by default.
 
-// RUN: %clangxx %s -### -no-canonical-prefixes -target wasm32-unknown-unknown 2>&1 | FileCheck -check-prefix=CC1 %s
+// RUN: %clangxx %s -### -no-canonical-prefixes -target wasm32-unknown-unknown 2>&1 \
+// RUN:   | FileCheck -check-prefix=CC1 %s
 // CC1: clang{{.*}} "-cc1" "-triple" "wasm32-unknown-unknown" {{.*}} "-fvisibility" "hidden" {{.*}}
 
 // Ditto, but ensure that a user -fvisibility=default disables the default
 // -fvisibility=hidden.
 
-// RUN: %clangxx %s -### -target wasm32-unknown-unknown -fvisibility=default 2>&1 | FileCheck -check-prefix=FVISIBILITY_DEFAULT %s
+// RUN: %clangxx %s -### -target wasm32-unknown-unknown -fvisibility=default 2>&1 \
+// RUN:   | FileCheck -check-prefix=FVISIBILITY_DEFAULT %s
 // FVISIBILITY_DEFAULT-NOT: hidden
 
 // A basic C++ link command-line with unknown OS.
 
-// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo --stdlib=c++ %s 2>&1 | FileCheck -check-prefix=LINK %s
+// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo --stdlib=c++ %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK %s
 // LINK: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK: wasm-ld{{.*}}" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C++ link command-line with optimization with unknown OS.
 
-// RUN: %clangxx -### -O2 -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s --stdlib=c++ 2>&1 | FileCheck -check-prefix=LINK_OPT %s
+// RUN: %clangxx -### -O2 -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s --stdlib=c++ 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_OPT %s
 // LINK_OPT: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK_OPT: wasm-ld{{.*}}" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C++ link command-line with known OS.
 
-// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo --stdlib=c++ %s 2>&1 | FileCheck -check-prefix=LINK_KNOWN %s
+// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo --stdlib=c++ %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_KNOWN %s
 // LINK_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" "-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C++ link command-line with optimization with known OS.
 
-// RUN: %clangxx -### -O2 -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo %s --stdlib=c++ 2>&1 | FileCheck -check-prefix=LINK_OPT_KNOWN %s
+// RUN: %clangxx -### -O2 -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo %s --stdlib=c++ 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_OPT_KNOWN %s
 // LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" "-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
 // A basic C++ compile command-line with known OS.
 
-// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo --stdlib=c++ %s 2>&1 | FileCheck -check-prefix=COMPILE %s
-// COMPILE: clang{{.*}}" "-cc1" {{.*}} "-internal-isystem" "/foo/include/wasm32-wasi/c++/v1" "-internal-isystem" "/foo/include/c++/v1" "-internal-isystem" "/foo/include/wasm32-wasi" "-internal-isystem" "/foo/include"
+// RUN: %clangxx -### -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo --stdlib=c++ %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=COMPILE %s
+// COMPILE: clang{{.*}}" "-cc1"
+// COMPILE: "-isysroot" "/foo"
+// COMPILE: "-internal-isystem" "/foo/include/wasm32-wasi/c++/v1"
+// COMPILE: "-internal-isystem" "/foo/include/c++/v1"
+// COMPILE: "-internal-isystem" "/foo/include/wasm32-wasi"
+// COMPILE: "-internal-isystem" "/foo/include"
Index: 

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:8729
+/// \code
+/// void .omp_mapper...(void *rt_mapper_handle,
+///   void *base, void *begin,

lildmh wrote:
> ABataev wrote:
> > This function looks like the universal one, regardless of the type 
> > `` specifics. Do we really need to generate it for each 
> > particular type and mapper? Or we could use the same function for all 
> > types/mappers?
> I think we need a particular mapper function for each type and mapper, 
> because the code generated within the mapper function depends on what type 
> and what mapper it is.
Hmm, maybe I'm wrong but I don't see significant mapper or type-specific 
dependencies in this mapper function. It uses the pointer to type and size of 
the type, but this information can be generalized, I think. Could you point the 
lines of code that are type and mapper specific?


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

https://reviews.llvm.org/D59474



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