[PATCH] D64119: [PowerPC] Support constraint code "ww"

[Fangrui Song via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL365106: [PowerPC] Support constraint code "ww" 
(authored by MaskRay, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D64119?vs=207707&id=207971#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64119

Files:
  cfe/trunk/lib/Basic/Targets/PPC.h
  cfe/trunk/test/CodeGen/ppc64-inline-asm.c
  llvm/trunk/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/trunk/test/CodeGen/PowerPC/inlineasm-vsx-reg.ll
  llvm/trunk/test/CodeGen/PowerPC/vec-asm-disabled.ll

Index: llvm/trunk/test/CodeGen/PowerPC/vec-asm-disabled.ll
===
--- llvm/trunk/test/CodeGen/PowerPC/vec-asm-disabled.ll
+++ llvm/trunk/test/CodeGen/PowerPC/vec-asm-disabled.ll
@@ -19,5 +19,17 @@
 ; CHECK: error: couldn't allocate output register for constraint 'wi'
 }
 
+define float @test_ww(float %x, float %y) #0 {
+  %1 = tail call float asm "xsmaxdp ${0:x},${1:x},${2:x}", "=^ww,^ww,^ww"(float %x, float %y) #0
+  ret float %1
+; CHECK: error: couldn't allocate output register for constraint 'ww'
+}
+
+define double @test_ws(double %x, double %y) #0 {
+  %1 = tail call double asm "xsmaxdp ${0:x},${1:x},${2:x}", "=^ws,^ws,^ws"(double %x, double %y) #0
+  ret double %1
+; CHECK: error: couldn't allocate output register for constraint 'ws'
+}
+
 attributes #0 = { nounwind "target-features"="-vsx" }
 
Index: llvm/trunk/test/CodeGen/PowerPC/inlineasm-vsx-reg.ll
===
--- llvm/trunk/test/CodeGen/PowerPC/inlineasm-vsx-reg.ll
+++ llvm/trunk/test/CodeGen/PowerPC/inlineasm-vsx-reg.ll
@@ -38,3 +38,12 @@
 ; CHECK: mtvsrd v2, r1
 ; CHECK: #NO_APP
 }
+
+define float @test_ww(float %x, float %y) {
+  %1 = tail call float asm "xsmaxdp ${0:x}, ${1:x}, ${2:x}", "=^ww,^ww,^ww"(float %x, float %y)
+  ret float %1
+; CHECK-LABEL: test_ww:
+; CHECK: #APP
+; CHECK: xsmaxdp f1, f1, f2
+; CHECK: #NO_APP
+}
Index: llvm/trunk/lib/Target/PowerPC/PPCISelLowering.cpp
===
--- llvm/trunk/lib/Target/PowerPC/PPCISelLowering.cpp
+++ llvm/trunk/lib/Target/PowerPC/PPCISelLowering.cpp
@@ -13962,7 +13962,7 @@
 return C_RegisterClass;
   } else if (Constraint == "wa" || Constraint == "wd" ||
  Constraint == "wf" || Constraint == "ws" ||
- Constraint == "wi") {
+ Constraint == "wi" || Constraint == "ww") {
 return C_RegisterClass; // VSX registers.
   }
   return TargetLowering::getConstraintType(Constraint);
@@ -13990,10 +13990,12 @@
 StringRef(constraint) == "wf") &&
type->isVectorTy())
 return CW_Register;
-  else if (StringRef(constraint) == "ws" && type->isDoubleTy())
-return CW_Register;
   else if (StringRef(constraint) == "wi" && type->isIntegerTy(64))
 return CW_Register; // just hold 64-bit integers data.
+  else if (StringRef(constraint) == "ws" && type->isDoubleTy())
+return CW_Register;
+  else if (StringRef(constraint) == "ww" && type->isFloatTy())
+return CW_Register;
 
   switch (*constraint) {
   default:
@@ -14079,7 +14081,7 @@
  Constraint == "wf" || Constraint == "wi") &&
  Subtarget.hasVSX()) {
 return std::make_pair(0U, &PPC::VSRCRegClass);
-  } else if (Constraint == "ws" && Subtarget.hasVSX()) {
+  } else if ((Constraint == "ws" || Constraint == "ww") && Subtarget.hasVSX()) {
 if (VT == MVT::f32 && Subtarget.hasP8Vector())
   return std::make_pair(0U, &PPC::VSSRCRegClass);
 else
Index: cfe/trunk/lib/Basic/Targets/PPC.h
===
--- cfe/trunk/lib/Basic/Targets/PPC.h
+++ cfe/trunk/lib/Basic/Targets/PPC.h
@@ -207,7 +207,8 @@
   switch (Name[1]) {
   case 'd': // VSX vector register to hold vector double data
   case 'f': // VSX vector register to hold vector float data
-  case 's': // VSX vector register to hold scalar float data
+  case 's': // VSX vector register to hold scalar double data
+  case 'w': // VSX vector register to hold scalar double data
   case 'a': // Any VSX register
   case 'c': // An individual CR bit
   case 'i': // FP or VSX register to hold 64-bit integers data
Index: cfe/trunk/test/CodeGen/ppc64-inline-asm.c
===
--- cfe/trunk/test/CodeGen/ppc64-inline-asm.c
+++ cfe/trunk/test/CodeGen/ppc64-inline-asm.c
@@ -24,3 +24,16 @@
 // CHECK: call i8 asm "crand $0, $1, $2", "=^wc,^wc,^wc"(i8 %b1, i8 %b2)
 }
 
+float test_fmaxf(float x, float y) {
+  asm("xsmaxdp %x0, %x1, %x2" : "=ww"(x) : "ww"(x), "ww"(y));
+  return x;
+// CHECK-LABEL: float @test_fmaxf(float %x, float %y)
+// CHECK: call float asm "xsmaxdp ${0:x}, ${1:x}, ${2:x}", "=^ww,^ww,^ww"(float %x, float %y)
+}
+
+double test_fmax(double x, double y) {
+  asm("

r365106 - [PowerPC] Support constraint code "ww"

[Fangrui Song via cfe-commits]

Author: maskray
Date: Wed Jul  3 21:44:42 2019
New Revision: 365106

URL: http://llvm.org/viewvc/llvm-project?rev=365106&view=rev
Log:
[PowerPC] Support constraint code "ww"

Summary:
"ww" and "ws" are both constraint codes for VSX vector registers that
hold scalar double data. "ww" is preferred for float while "ws" is
preferred for double.

Reviewed By: jsji

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

Modified:
cfe/trunk/lib/Basic/Targets/PPC.h
cfe/trunk/test/CodeGen/ppc64-inline-asm.c

Modified: cfe/trunk/lib/Basic/Targets/PPC.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/PPC.h?rev=365106&r1=365105&r2=365106&view=diff
==
--- cfe/trunk/lib/Basic/Targets/PPC.h (original)
+++ cfe/trunk/lib/Basic/Targets/PPC.h Wed Jul  3 21:44:42 2019
@@ -207,7 +207,8 @@ public:
   switch (Name[1]) {
   case 'd': // VSX vector register to hold vector double data
   case 'f': // VSX vector register to hold vector float data
-  case 's': // VSX vector register to hold scalar float data
+  case 's': // VSX vector register to hold scalar double data
+  case 'w': // VSX vector register to hold scalar double data
   case 'a': // Any VSX register
   case 'c': // An individual CR bit
   case 'i': // FP or VSX register to hold 64-bit integers data

Modified: cfe/trunk/test/CodeGen/ppc64-inline-asm.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ppc64-inline-asm.c?rev=365106&r1=365105&r2=365106&view=diff
==
--- cfe/trunk/test/CodeGen/ppc64-inline-asm.c (original)
+++ cfe/trunk/test/CodeGen/ppc64-inline-asm.c Wed Jul  3 21:44:42 2019
@@ -24,3 +24,16 @@ unsigned char test_wc_i8(unsigned char b
 // CHECK: call i8 asm "crand $0, $1, $2", "=^wc,^wc,^wc"(i8 %b1, i8 %b2)
 }
 
+float test_fmaxf(float x, float y) {
+  asm("xsmaxdp %x0, %x1, %x2" : "=ww"(x) : "ww"(x), "ww"(y));
+  return x;
+// CHECK-LABEL: float @test_fmaxf(float %x, float %y)
+// CHECK: call float asm "xsmaxdp ${0:x}, ${1:x}, ${2:x}", 
"=^ww,^ww,^ww"(float %x, float %y)
+}
+
+double test_fmax(double x, double y) {
+  asm("xsmaxdp %x0, %x1, %x2" : "=ws"(x) : "ws"(x), "ws"(y));
+  return x;
+// CHECK-LABEL: double @test_fmax(double %x, double %y)
+// CHECK: call double asm "xsmaxdp ${0:x}, ${1:x}, ${2:x}", 
"=^ws,^ws,^ws"(double %x, double %y)
+}


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

[PATCH] D64119: [PowerPC] Support constraint code "ww"

[Fangrui Song via Phabricator via cfe-commits]

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

  float ws_float(float x, float y) {
__asm__ ("xsadddp %0, %1, %2" : "=ws"(x) : "ws"(x), "ws"(y));
return x;
  }
  float ww_float(float x, float y) {
__asm__ ("xsadddp %0, %1, %2" : "=ww"(x) : "ww"(x), "ww"(y));
return x;
  }
  
  double ws_double(double x, double y) {
__asm__ ("xsadddp %0, %1, %2" : "=ws"(x) : "ws"(x), "ws"(y));
return x;
  }
  double ww_double(double x, double y) {
__asm__ ("xsadddp %0, %1, %2" : "=ww"(x) : "ww"(x), "ww"(y));
return x;
  }



  % powerpc64le-linux-gnu-gcc -O2 a.c -S -o - | grep xsadd
  xsadddp 1, 1, 2
  xsadddp 1, 1, 2
  xsadddp 1, 1, 2
  xsadddp 1, 1, 2



  float scalar_ww_float(float x, float y) {
__asm__ ("fadds %0, %1, %2" : "=ww"(x) : "ww"(x), "ww"(y));
return x;
  }
  float scalar_ws_float(float x, float y) {
__asm__ ("fadds %0, %1, %2" : "=ws"(x) : "ws"(x), "ws"(y));
return x;
  }
  double scalar_ww_double(double x, double y) {
__asm__ ("fadds %0, %1, %2" : "=ww"(x) : "ww"(x), "ww"(y));
return x;
  }
  double scalar_ws_double(double x, double y) {
__asm__ ("fadds %0, %1, %2" : "=ws"(x) : "ws"(x), "ws"(y));
return x;
  }



  % powerpc64le-linux-gnu-gcc -O2 a.c -S -o - | grep fadds
  fadds 1, 1, 2
  fadds 1, 1, 2
  fadds 1, 1, 2
  fadds 1, 1, 2




Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:14080
 return std::make_pair(0U, &PPC::VSRCRegClass);
-  } else if (Constraint == "ws" && Subtarget.hasVSX()) {
+  } else if ((Constraint == "ws" || Constraint == "ww") && Subtarget.hasVSX()) 
{
 if (VT == MVT::f32 && Subtarget.hasP8Vector())

jsji wrote:
> MaskRay wrote:
> > jsji wrote:
> > > Should we exclude `FP` for `ws` and return `VFRCRegClass` instead of 
> > > `VSFRCRegClass` ?
> > Can you elaborate on what I should do? I'm not familiar with the register 
> > info stuff...
> `VSFRC` contains both `F8RC` and `VFRC`. `F8RC` is FP.
> So if `ws` can NOT use FP, then we should not use `VSFRC`.
> 
> However, if `ws` can use `FP` as well as you found in later GCC experiments, 
> then we don't need to do this.
```
float ws_float(float x, float y) {
  __asm__ ("xsadddp %0, %1, %2" : "=ws"(x) : "ws"(x), "ws"(y));
  return x;
}
float ww_float(float x, float y) {
  __asm__ ("xsadddp %0, %1, %2" : "=ww"(x) : "ww"(x), "ww"(y));
  return x;
}

double ws_double(double x, double y) {
  __asm__ ("xsadddp %0, %1, %2" : "=ws"(x) : "ws"(x), "ws"(y));
  return x;
}
double ww_double(double x, double y) {
  __asm__ ("xsadddp %0, %1, %2" : "=ww"(x) : "ww"(x), "ww"(y));
  return x;
}
```

```
% powerpc64le-linux-gnu-gcc -O2 a.c -S -o - | grep xsadd
xsadddp 1, 1, 2
xsadddp 1, 1, 2
xsadddp 1, 1, 2
xsadddp 1, 1, 2
```




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64119



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

[PATCH] D64067: [X86][PPC] Support -mlong-double-64

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

`-mlong-double-128` is not supported now. The mangling scheme of 
`-mlong-double-64` is consistent with gcc on x86 and ppc.

  % g++ a.cc -S -o - | grep '^_Z3foo'
  _Z3fooe:
  % g++ a.cc -S -o - -mlong-double-64 | grep '^_Z3foo'
  _Z3fooe:
  % g++ a.cc -S -o - -mlong-double-128 | grep '^_Z3foo'
  _Z3foog:
  
  % powerpc64le-linux-gnu-g++ a.cc -S -o - -mlong-double-64 | grep '^_Z3foo'
  _Z3fooe:
  % powerpc64le-linux-gnu-g++ a.cc -S -o - -mlong-double-128 
-mabi=ibmlongdouble | grep '^_Z3foo'
  _Z3foog:
  % powerpc64le-linux-gnu-g++ a.cc -S -o - -mlong-double-128 
-mabi=ieeelongdouble | grep '^_Z3foo'
  _Z3fooU10__float128:


Repository:
  rC Clang

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

https://reviews.llvm.org/D64067



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

[PATCH] D64067: [X86][PPC] Support -mlong-double-64

[Fangrui Song via Phabricator via cfe-commits]

MaskRay updated this revision to Diff 207965.
MaskRay edited the summary of this revision.
MaskRay added a comment.

Add tests to check mangling of long double

-mlong-double-64 has no effect on gcc x86.
On gcc ppc, -mlong-double-64 changes the mangled type from 'g' (ibmlongdouble) 
to 'e'


Repository:
  rC Clang

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

https://reviews.llvm.org/D64067

Files:
  include/clang/Basic/LangOptions.def
  include/clang/Driver/Options.td
  lib/Basic/TargetInfo.cpp
  lib/Basic/Targets/PPC.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/long-double-x86.c
  test/CodeGen/ppc64-align-long-double.c
  test/CodeGen/ppc64-long-double.cpp
  test/CodeGen/x86-long-double.cpp
  test/Driver/mlong-double-64.c

Index: test/Driver/mlong-double-64.c
===
--- /dev/null
+++ test/Driver/mlong-double-64.c
@@ -0,0 +1,11 @@
+// RUN: %clang -target powerpc-linux-musl -c -### %s -mlong-double-64 2>&1 | FileCheck %s
+// RUN: %clang -target powerpc64-pc-freebsd12 -c -### %s -mlong-double-64 2>&1 | FileCheck %s
+// RUN: %clang -target powerpc64le-linux-musl -c -### %s -mlong-double-64 2>&1 | FileCheck %s
+// RUN: %clang -target i686-linux-gnu -c -### %s -mlong-double-64 2>&1 | FileCheck %s
+// RUN: %clang -target x86_64-linux-musl -c -### %s -mlong-double-64 2>&1 | FileCheck %s
+
+// CHECK: "-mlong-double-64"
+
+// RUN: %clang -target aarch64 -c -### %s -mlong-double-64 2>&1 | FileCheck --check-prefix=ERR %s
+
+// ERR: error: unsupported option '-mlong-double-64' for target 'aarch64'
Index: test/CodeGen/x86-long-double.cpp
===
--- /dev/null
+++ test/CodeGen/x86-long-double.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=i686-apple-darwin9 -mlong-double-64 | \
+// RUN:   FileCheck --check-prefix=FP64-X32 %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=i686-apple-darwin9 | \
+// RUN:   FileCheck --check-prefix=FP80 %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=x86_64-linux-musl -mlong-double-64 | \
+// RUN:   FileCheck --check-prefix=FP64-X64 %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=x86_64-linux-musl | \
+// RUN:   FileCheck --check-prefix=FP80 %s
+
+// Check -malign-double increases the alignment from 4 to 8 on x86-32.
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=i686-linux-musl -mlong-double-64 \
+// RUN:   -malign-double | FileCheck --check-prefix=FP64-X64 %s
+// RUN: %clang_cc1 %s -emit-llvm -o - -triple=x86_64-linux-musl -mlong-double-64 \
+// RUN:   -malign-double | FileCheck --check-prefix=FP64-X64 %s
+
+long double x = 0;
+int size = sizeof(x);
+
+// FP64-X32: @x = global double {{.*}}, align 4
+// FP64-X32: @size = global i32 8
+// FP64-X64: @x = global double {{.*}}, align 8
+// FP64-X64: @size = global i32 8
+// FP80: @x = global x86_fp80 {{.*}}, align 16
+// FP80: @size = global i32 16
+
+long double foo(long double d) { return d; }
+
+// FP64-X32: double @_Z3fooe(double %d)
+// FP64-X64: double @_Z3fooe(double %d)
+// FP80: x86_fp80 @_Z3fooe(x86_fp80 %d)
Index: test/CodeGen/ppc64-long-double.cpp
===
--- /dev/null
+++ test/CodeGen/ppc64-long-double.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple powerpc64-linux-musl -emit-llvm -o - %s | \
+// RUN:   FileCheck --check-prefix=FP64 %s
+// RUN: %clang_cc1 -triple powerpc64-linux-gnu -emit-llvm -o - %s -mlong-double-64 | \
+// RUN:   FileCheck --check-prefix=FP64 %s
+// RUN: %clang_cc1 -triple powerpc64-linux-gnu -emit-llvm -o - %s | \
+// RUN:   FileCheck --check-prefix=IBM128 %s
+
+long double x = 0;
+int size = sizeof(x);
+
+// FP64: @x = global double {{.*}}, align 8
+// FP64: @size = global i32 8
+// IBM128: @x = global ppc_fp128 {{.*}}, align 16
+// IBM128: @size = global i32 16
+
+long double foo(long double d) { return d; }
+
+// FP64: double @_Z3fooe(double %d)
+// IBM128: ppc_fp128 @_Z3foog(ppc_fp128 %d)
Index: test/CodeGen/ppc64-align-long-double.c
===
--- test/CodeGen/ppc64-align-long-double.c
+++ /dev/null
@@ -1,16 +0,0 @@
-// REQUIRES: powerpc-registered-target
-// RUN: %clang_cc1 -triple powerpc64-unknown-linux-gnu -emit-llvm -o - %s | FileCheck %s
-
-struct S {
-  double a;
-  long double b;
-};
-
-// CHECK: %struct.{{[a-zA-Z0-9]+}} = type { double, ppc_fp128 }
-
-long double test (struct S x)
-{
-  return x.b;
-}
-
-// CHECK: %{{[0-9]}} = load ppc_fp128, ppc_fp128* %{{[a-zA-Z0-9]+}}, align 16
Index: test/CodeGen/long-double-x86.c
===
--- test/CodeGen/long-double-x86.c
+++ /dev/null
@@ -1,4 +0,0 @@
-// RUN: %clang_cc1 %s -emit-llvm -o - -triple=i686-apple-darwin9 | grep x86_fp80
-
-long double x = 0;
-int checksize[sizeof(x) == 16 ? 1 : -1];
Index: lib/Frontend/CompilerInvocation.cpp
==

[PATCH] D64119: [PowerPC] Support constraint code "ww"

[Jinsong Ji via Phabricator via cfe-commits]

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

LGTM. Thanks for investigating GCC behavior.




Comment at: clang/lib/Basic/Targets/PPC.h:211
+  case 's': // VSX vector register to hold scalar double data
+  case 'w': // VSX vector register to hold scalar double data
   case 'a': // Any VSX register

MaskRay wrote:
> jsji wrote:
> > Add some more comments for `w` to distinguish it from `s`?
> > 
> > Do we want to keep compatibility with GCC? 
> > According to 
> > https://gcc.gnu.org/onlinedocs/gcc-9.1.0/gcc/Machine-Constraints.html#Machine-Constraints,
> >  
> > `ww` is `FP or VSX register to perform float operations under -mvsx or 
> > NO_REGS.`, 
> > while `ws` is `VSX vector register to hold scalar double values `. 
> > 
> > So `ww` can use `FP` while `ws` can NOT ?
> I played with "ws" and "ww" but can't find any behavior difference from 
> assembly produced by powerpc64le-linux-gnu-gcc. I'll keep the current form 
> (which is known to make musl fmax/fmaxf build) unless the gcc semantics are 
> clearer.
OK. Thanks. So maybe it is just the misleading doc problem of GCC.



Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:14080
 return std::make_pair(0U, &PPC::VSRCRegClass);
-  } else if (Constraint == "ws" && Subtarget.hasVSX()) {
+  } else if ((Constraint == "ws" || Constraint == "ww") && Subtarget.hasVSX()) 
{
 if (VT == MVT::f32 && Subtarget.hasP8Vector())

MaskRay wrote:
> jsji wrote:
> > Should we exclude `FP` for `ws` and return `VFRCRegClass` instead of 
> > `VSFRCRegClass` ?
> Can you elaborate on what I should do? I'm not familiar with the register 
> info stuff...
`VSFRC` contains both `F8RC` and `VFRC`. `F8RC` is FP.
So if `ws` can NOT use FP, then we should not use `VSFRC`.

However, if `ws` can use `FP` as well as you found in later GCC experiments, 
then we don't need to do this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64119



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

[PATCH] D64119: [PowerPC] Support constraint code "ww"

[Fangrui Song via Phabricator via cfe-commits]

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



Comment at: clang/lib/Basic/Targets/PPC.h:211
+  case 's': // VSX vector register to hold scalar double data
+  case 'w': // VSX vector register to hold scalar double data
   case 'a': // Any VSX register

jsji wrote:
> Add some more comments for `w` to distinguish it from `s`?
> 
> Do we want to keep compatibility with GCC? 
> According to 
> https://gcc.gnu.org/onlinedocs/gcc-9.1.0/gcc/Machine-Constraints.html#Machine-Constraints,
>  
> `ww` is `FP or VSX register to perform float operations under -mvsx or 
> NO_REGS.`, 
> while `ws` is `VSX vector register to hold scalar double values `. 
> 
> So `ww` can use `FP` while `ws` can NOT ?
I played with "ws" and "ww" but can't find any behavior difference from 
assembly produced by powerpc64le-linux-gnu-gcc. I'll keep the current form 
(which is known to make musl fmax/fmaxf build) unless the gcc semantics are 
clearer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64119



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

[PATCH] D64119: [PowerPC] Support constraint code "ww"

[Fangrui Song via Phabricator via cfe-commits]

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



Comment at: llvm/test/CodeGen/PowerPC/inlineasm-vsx-reg.ll:42
+
+define float @test_ww(float %x, float %y) {
+  %1 = tail call float asm "xsmaxdp ${0:x}, ${1:x}, ${2:x}", 
"=^ww,^ww,^ww"(float %x, float %y)

jsji wrote:
> Maybe we should add another test for ws as well? The above test is actually 
> for 'x' modifier?
I think it is incorrect if the 'x' modifier is not used, so we probably don't 
have to check the no-modifier case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64119



___
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.

`ninja check-all` passed


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] D64128: [CodeGen] Generate llvm.ptrmask instead of inttoptr(and(ptrtoint, C)) if possible.

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In D64128#1569817 , @rjmccall wrote:

> The pointer/integer conversion is "implementation-defined", but it's not 
> totally unconstrained.  C notes that "The mapping functions for converting a 
> pointer to an integer or an integer to a pointer are intended to be 
> consistent with the addressing structure of the execution environment.", and 
> we do have to honor that.  The standard allows that "the result ... might not 
> point to an entity of the referenced type", but when in fact it's guaranteed 
> to do so (i.e. it's not just a coincidental result of an implementation 
> decision like the exact address of a global variable — no "guessing"), I do 
> think we have an obligation to make it work.  And on a practical level, there 
> has to be *some* way of playing clever address tricks in the language in 
> order to implement things like allocators and so forth.  So this makes me 
> very antsy.


I don't disagree. But I believe the question is if we have:

  int *x = malloc(4);
  int *y = malloc(4);
  if (x & ~15 == y) {
*(x & ~15) = 5; // Is this allowed, and if so, must the compiler assume 
that it might set the value of *y?
  }

I certainly agree that we must allow the implementation of allocators, etc. But 
allocators, I think, have the opposite problem. They actually have some large 
underlying objects (from mmap or whatever), and we want the rest of the system 
to treat some subobjects of these larger objects as though they were 
independent objects of some given types. From the point of view of the 
allocator, we have x, and we have `void *memory_pool`, and we need to allow `x 
& N` to point into `memory_pool`, but because, from the allocator's 
perspective, we never knew that x didn't point into memory_pool (as, in fact, 
it likely does), that should be fine (*).

There might be more of an issue, for example, if for a given object, I happen 
to know that there's some interesting structure at the beginning of its page 
(or some other boundary). If I also have a pointer to this structure via some 
other means, then maybe this will cause a problem. This kind of thing certainly 
falls outside of the C/C++ abstract machine, and I'd lean toward a flag for 
supporting it (not on by default). I'm assuming that this would be rare. If I'm 
wrong, then we shouldn't do this by default.

(*) We do have a problem if we inline the implementation of malloc, given how 
our noalias return attribute works, but that's a preexisting problem, and the 
malloc implementation should probably be compiled with -fno-builtin-malloc 
regardless.

> If the general language rules are too permissive for some interesting 
> optimization, it's fine to consider builtins that impose stronger 
> restrictions on their use.

I agree.

Also, and I could be wrong, but my impression is that all of this is extra - 
this motivating use case requires generating the intrinsic from the code in 
lib/CodeGen/TargetInfo.cpp - generating it from C/C++ expressions is just a 
potential additional benefit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64128



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

[PATCH] D62645: [Sema] Resolve placeholder types before type deduction to silence spurious `-Warc-repeated-use-of-weak` warnings

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: include/clang/Basic/Attr.td:1844
+  let Documentation = [Undocumented];
+}
+

Oh, please add a comment on this explaining what it means, since it's not based 
on a language feature.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62645



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

[PATCH] D62645: [Sema] Resolve placeholder types before type deduction to silence spurious `-Warc-repeated-use-of-weak` warnings

[John McCall via Phabricator via cfe-commits]

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

Thanks, LGTM.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62645



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

[PATCH] D64128: [CodeGen] Generate llvm.ptrmask instead of inttoptr(and(ptrtoint, C)) if possible.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

The pointer/integer conversion is "implementation-defined", but it's not 
totally unconstrained.  C notes that "The mapping functions for converting a 
pointer to an integer or an integer to a pointer are intended to be consistent 
with the addressing structure of the execution environment.", and we do have to 
honor that.  The standard allows that "the result ... might not point to an 
entity of the referenced type", but when in fact it's guaranteed to do so (i.e. 
it's not just a coincidental result of an implementation decision like the 
exact address of a global variable — no "guessing"), I do think we have an 
obligation to make it work.  And on a practical level, there has to be *some* 
way of playing clever address tricks in the language in order to implement 
things like allocators and so forth.  So this makes me very antsy.

If the general language rules are too permissive for some interesting 
optimization, it's fine to consider builtins that impose stronger restrictions 
on their use.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64128



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

[PATCH] D64119: [PowerPC] Support constraint code "ww"

[Fangrui Song via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:14080
 return std::make_pair(0U, &PPC::VSRCRegClass);
-  } else if (Constraint == "ws" && Subtarget.hasVSX()) {
+  } else if ((Constraint == "ws" || Constraint == "ww") && Subtarget.hasVSX()) 
{
 if (VT == MVT::f32 && Subtarget.hasP8Vector())

jsji wrote:
> Should we exclude `FP` for `ws` and return `VFRCRegClass` instead of 
> `VSFRCRegClass` ?
Can you elaborate on what I should do? I'm not familiar with the register info 
stuff...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64119



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

[PATCH] D64128: [CodeGen] Generate llvm.ptrmask instead of inttoptr(and(ptrtoint, C)) if possible.

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In D64128#1569590 , @efriedma wrote:

> > If they're all syntactically together like this, maybe that's safe?
>
> Having them together syntactically doesn't really help, I think; it might be 
> guarded by some code that does the same conversion (and if you repeat the 
> conversion, it has to produce the same result).


Indeed. That's correct (and also why the hasOneUse check at the IR level would 
have been ineffective). However...

In D64128#1569578 , @rjmccall wrote:

> I agree with Eli that this isn't obviously a legal transformation.  
> `llvm.ptrmask` appears to make semantic guarantees about e.g. the pointer 
> after the mask referring to the same underlying object, which means we can 
> only safely emit it when something about the source program makes that 
> guarantee.  It's not at all clear that C does so for an expression like `(T*) 
> ((intptr_t) x & N)`.


I think that this is the key point. First, at the IR level we have a problem 
because we have no way to robustly track pointer provenance information. If we 
have `if (a == b) { f(a); }` the optimizer can transform this code into `if (a 
== b) { f(b); }` and we've lost track of whether the parameter to f is based on 
a or b. At the source level we don't have this problem (because we have the 
unaltered expressions provided by the user, and can therefore use whatever 
provenance information that source implies).

Thus, as John says, the question is whether, at the source level, `(T*) 
((intptr_t) x & N)` always has, and only has, the same underlying objects as x 
- when executing the expression is well defined. In C++, I think that this is 
clearly true for implementations with "strict pointer safety" (6.6.5.4.3), as 
the rules for safely-derived pointer values state that, while you can get 
safely-derived pointer values using integer casts and bitwise operators, the 
result must be one that could have been safely derived from the original object 
using well-defined pointer arithmetic, and that's only true for pointers into 
some array pointed into by x (or one past the end). For implementations with 
"relaxed pointer safety", it's all implementation defined, so I don't see we 
couldn't choose our implementation-defined semantics to define this problem 
away (although we certainly need to be careful that we don't unintentionally 
make any significant body of preexisting code incompatible with Clang by doing 
so).

For C, we also need to be concerned with the definition of "based on" 
(6.7.3.1). In some philosophical sense, this seems trickier (i.e., what if 
modifying the value of x at some sequence point prior to the expression makes 
the expressions dead? Are we required, as part of the standardized through 
experiment, to also modify the other variables to keep the expression alive 
when performing the "based on" analysis, and do those modifications count for 
the purposes of determining the "based on" property?). Regardless, given that 
the intent is to enable optimizations, it seems reasonable to say that `(T*) 
((intptr_t) x & N)` is only based on x. For C, 6.3.2.3 makes the conversion 
validity itself implementation defined.

@rsmith , thoughts on this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64128



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

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: include/clang/AST/Type.h:1133
+  /// Check if this is or contains a non-trivial C struct/union type.
+  bool hasNonTrivialPrimitiveCStruct() const;
 

ahatanak wrote:
> rjmccall wrote:
> > You only want these checks to trigger on unions with non-trivial members 
> > (or structs containing them), right?  How about something like 
> > `hasNonTrivialPrimitiveCUnionMember()`?  Or maybe make it more descriptive 
> > for the use sites, like `isPrimitiveCRestrictedType()`?
> > 
> > Also, it would be nice if the fast path of this could be inlined so that 
> > clients usually didn't had to make a call at all.  You can write the 
> > `getBaseElementTypeUnsafe()->getAs()` part in an `inline` 
> > implementation at the bottom this file.
> Since we don't keep track of whether a struct or union is or contains unions 
> with non-trivial members, we'll have to use the visitors to detect such 
> structs or unions or, to do it faster, add a bit to `RecordDeclBits` that 
> indicates the presence of non-trivial unions. I guess it's okay to add 
> another bit to `RecordDeclBits`?
It looks like there's plenty of space in `RecordDeclBits`, yeah.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63753



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

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Thanks, this looks good to me.


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

https://reviews.llvm.org/D53157



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

[PATCH] D63915: [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of function calls

[Csaba Dabis via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL365103: [analyzer] ReturnValueChecker: Model the guaranteed 
boolean return value of… (authored by Charusso, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63915?vs=207939&id=207942#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63915

Files:
  cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
  cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
  cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  cfe/trunk/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  cfe/trunk/test/Analysis/return-value-guaranteed.cpp

Index: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
@@ -219,24 +219,34 @@
 Eng.getBugReporter().emitReport(std::move(R));
   }
 
-
   /// Produce a program point tag that displays an additional path note
   /// to the user. This is a lightweight alternative to the
   /// BugReporterVisitor mechanism: instead of visiting the bug report
   /// node-by-node to restore the sequence of events that led to discovering
   /// a bug, you can add notes as you add your transitions.
-  const NoteTag *getNoteTag(NoteTag::Callback &&Cb) {
-return Eng.getNoteTags().makeNoteTag(std::move(Cb));
+  ///
+  /// @param Cb Callback with 'BugReporterContext &, BugReport &' parameters.
+  /// @param IsPrunable Whether the note is prunable. It allows BugReporter
+  ///to omit the note from the report if it would make the displayed
+  ///bug path significantly shorter.
+  const NoteTag *getNoteTag(NoteTag::Callback &&Cb, bool IsPrunable = false) {
+return Eng.getNoteTags().makeNoteTag(std::move(Cb), IsPrunable);
   }
 
   /// A shorthand version of getNoteTag that doesn't require you to accept
   /// the BugReporterContext arguments when you don't need it.
-  const NoteTag *getNoteTag(std::function &&Cb) {
+  ///
+  /// @param Cb Callback only with 'BugReport &' parameter.
+  /// @param IsPrunable Whether the note is prunable. It allows BugReporter
+  ///to omit the note from the report if it would make the displayed
+  ///bug path significantly shorter.
+  const NoteTag *getNoteTag(std::function &&Cb,
+bool IsPrunable = false) {
 return getNoteTag(
-[Cb](BugReporterContext &, BugReport &BR) { return Cb(BR); });
+[Cb](BugReporterContext &, BugReport &BR) { return Cb(BR); },
+IsPrunable);
   }
 
-
   /// Returns the word that should be used to refer to the declaration
   /// in the report.
   StringRef getDeclDescription(const Decl *D);
Index: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
===
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -274,6 +274,10 @@
 
 let ParentPackage = APIModeling in {
 
+def ReturnValueChecker : Checker<"ReturnValue">,
+  HelpText<"Model the guaranteed boolean return value of function calls">,
+  Documentation;
+
 def StdCLibraryFunctionsChecker : Checker<"StdCLibraryFunctions">,
   HelpText<"Improve modeling of the C standard library functions">,
   Documentation;
Index: cfe/trunk/test/Analysis/return-value-guaranteed.cpp
===
--- cfe/trunk/test/Analysis/return-value-guaranteed.cpp
+++ cfe/trunk/test/Analysis/return-value-guaranteed.cpp
@@ -0,0 +1,91 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,apiModeling.ReturnValue \
+// RUN:  -analyzer-output=text -verify=class %s
+
+struct Foo { int Field; };
+bool problem();
+void doSomething();
+
+// We predefined the return value of 'MCAsmParser::Error' as true and we cannot
+// take the false-branches which leads to a "garbage value" false positive.
+namespace test_classes {
+struct MCAsmParser {
+  static bool Error();
+};
+
+bool parseFoo(Foo &F) {
+  if (problem()) {
+// class-note@-1 {{Assuming the condition is false}}
+// class-note@-2 {{Taking false branch}}
+return MCAsmParser::Error();
+  }
+
+  F.Field = 0;
+  // class-note@-1 {{The value 0 is assigned to 'F.Field'}}
+  return !MCAsmParser::Error();
+  // class-note@-1 {{'MCAsmParser::Error' returns true}}
+}
+
+bool parseFile() {
+  Foo F;
+  if (parseFoo(F)) {
+// class-note@-1 {{Calling 'parseFoo'}}
+// class-note@-2 {{Returning from 'parseFoo'}}
+// class-note@-3 {{Taking false branch}}
+return true;
+  }
+
+  if (F.Field == 0) 

r365103 - [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of function calls

[Csaba Dabis via cfe-commits]

Author: charusso
Date: Wed Jul  3 17:50:50 2019
New Revision: 365103

URL: http://llvm.org/viewvc/llvm-project?rev=365103&view=rev
Log:
[analyzer] ReturnValueChecker: Model the guaranteed boolean return value of 
function calls

Summary: It models the known LLVM methods paired with their class.

Reviewers: NoQ, xazax.hun, ravikandhadai, baloghadamsoftware, Szelethus

Reviewed By: NoQ

Subscribers: dschuff, aheejin, mgorny, szepet, rnkovacs, a.sidorin,
 mikhail.ramalho, donat.nagy, dkrupp, cfe-commits

Tags: #clang

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

Added:
cfe/trunk/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp
cfe/trunk/test/Analysis/return-value-guaranteed.cpp
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td?rev=365103&r1=365102&r2=365103&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td Wed Jul  3 
17:50:50 2019
@@ -274,6 +274,10 @@ def NullableReturnedFromNonnullChecker :
 
 let ParentPackage = APIModeling in {
 
+def ReturnValueChecker : Checker<"ReturnValue">,
+  HelpText<"Model the guaranteed boolean return value of function calls">,
+  Documentation;
+
 def StdCLibraryFunctionsChecker : Checker<"StdCLibraryFunctions">,
   HelpText<"Improve modeling of the C standard library functions">,
   Documentation;

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h?rev=365103&r1=365102&r2=365103&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h 
Wed Jul  3 17:50:50 2019
@@ -219,24 +219,34 @@ public:
 Eng.getBugReporter().emitReport(std::move(R));
   }
 
-
   /// Produce a program point tag that displays an additional path note
   /// to the user. This is a lightweight alternative to the
   /// BugReporterVisitor mechanism: instead of visiting the bug report
   /// node-by-node to restore the sequence of events that led to discovering
   /// a bug, you can add notes as you add your transitions.
-  const NoteTag *getNoteTag(NoteTag::Callback &&Cb) {
-return Eng.getNoteTags().makeNoteTag(std::move(Cb));
+  ///
+  /// @param Cb Callback with 'BugReporterContext &, BugReport &' parameters.
+  /// @param IsPrunable Whether the note is prunable. It allows BugReporter
+  ///to omit the note from the report if it would make the displayed
+  ///bug path significantly shorter.
+  const NoteTag *getNoteTag(NoteTag::Callback &&Cb, bool IsPrunable = false) {
+return Eng.getNoteTags().makeNoteTag(std::move(Cb), IsPrunable);
   }
 
   /// A shorthand version of getNoteTag that doesn't require you to accept
   /// the BugReporterContext arguments when you don't need it.
-  const NoteTag *getNoteTag(std::function &&Cb) {
+  ///
+  /// @param Cb Callback only with 'BugReport &' parameter.
+  /// @param IsPrunable Whether the note is prunable. It allows BugReporter
+  ///to omit the note from the report if it would make the displayed
+  ///bug path significantly shorter.
+  const NoteTag *getNoteTag(std::function &&Cb,
+bool IsPrunable = false) {
 return getNoteTag(
-[Cb](BugReporterContext &, BugReport &BR) { return Cb(BR); });
+[Cb](BugReporterContext &, BugReport &BR) { return Cb(BR); },
+IsPrunable);
   }
 
-
   /// Returns the word that should be used to refer to the declaration
   /// in the report.
   StringRef getDeclDescription(const Decl *D);

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt?rev=365103&r1=365102&r2=365103&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt Wed Jul  3 17:50:50 
2019
@@ -83,6 +83,7 @@ add_clang_library(clangStaticAnalyzerChe
   RetainCountChecker/RetainCountDiagnostics.cpp
   ReturnPointerRangeChecker.cpp
   ReturnUndefChecker.cpp
+  ReturnValueChecker.cpp
   RunLoopAutoreleaseLeakChecker.cpp
   SimpleStreamChecker.cpp
   SmartPtrModeling.cpp

Added: cfe/trunk/lib/S

[PATCH] D64164: [clang][ARM] Fix msvc arm{64} builtins to use int on LP64 systems.

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese added a comment.

In D64164#1569679 , @rnk wrote:

> Please check the commit message:
>
> > [clang][ARM] Fix msvc arm{64} builtins to use int on LLP64 systems.
>
> I think you mean "use int on LP64 systems", since long is 32-bits on LLP64, 
> right?
>
> Otherwise, sounds good.


Thanks, I did indeed mean LP64.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64164



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

[PATCH] D64164: [clang][ARM] Fix msvc arm{64} builtins to use int on LLP64 systems.

[Reid Kleckner via Phabricator via cfe-commits]

rnk accepted this revision.
rnk added a comment.

Please check the commit message:

> [clang][ARM] Fix msvc arm{64} builtins to use int on LLP64 systems.

I think you mean "use int on LP64 systems", since long is 32-bits on LLP64, 
right?

Otherwise, sounds good.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64164



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

[PATCH] D63915: [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of function calls

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 207939.
Charusso marked 9 inline comments as done.
Charusso added a comment.

- Done.


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

https://reviews.llvm.org/D63915

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp
  clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  clang/test/Analysis/return-value-guaranteed.cpp

Index: clang/test/Analysis/return-value-guaranteed.cpp
===
--- /dev/null
+++ clang/test/Analysis/return-value-guaranteed.cpp
@@ -0,0 +1,91 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,apiModeling.ReturnValue \
+// RUN:  -analyzer-output=text -verify=class %s
+
+struct Foo { int Field; };
+bool problem();
+void doSomething();
+
+// We predefined the return value of 'MCAsmParser::Error' as true and we cannot
+// take the false-branches which leads to a "garbage value" false positive.
+namespace test_classes {
+struct MCAsmParser {
+  static bool Error();
+};
+
+bool parseFoo(Foo &F) {
+  if (problem()) {
+// class-note@-1 {{Assuming the condition is false}}
+// class-note@-2 {{Taking false branch}}
+return MCAsmParser::Error();
+  }
+
+  F.Field = 0;
+  // class-note@-1 {{The value 0 is assigned to 'F.Field'}}
+  return !MCAsmParser::Error();
+  // class-note@-1 {{'MCAsmParser::Error' returns true}}
+}
+
+bool parseFile() {
+  Foo F;
+  if (parseFoo(F)) {
+// class-note@-1 {{Calling 'parseFoo'}}
+// class-note@-2 {{Returning from 'parseFoo'}}
+// class-note@-3 {{Taking false branch}}
+return true;
+  }
+
+  if (F.Field == 0) {
+// class-note@-1 {{Field 'Field' is equal to 0}}
+// class-note@-2 {{Taking true branch}}
+
+// no-warning: "The left operand of '==' is a garbage value" was here.
+doSomething();
+  }
+
+  (void)(1 / F.Field);
+  // class-warning@-1 {{Division by zero}}
+  // class-note@-2 {{Division by zero}}
+  return false;
+}
+} // namespace test_classes
+
+
+// We predefined 'MCAsmParser::Error' as returning true, but now it returns
+// false, which breaks our invariant. Test the notes.
+namespace test_break {
+struct MCAsmParser {
+  static bool Error() {
+return false; // class-note {{'MCAsmParser::Error' returns false}}
+  }
+};
+
+bool parseFoo(Foo &F) {
+  if (problem()) {
+// class-note@-1 {{Assuming the condition is false}}
+// class-note@-2 {{Taking false branch}}
+return !MCAsmParser::Error();
+  }
+
+  F.Field = 0;
+  // class-note@-1 {{The value 0 is assigned to 'F.Field'}}
+  return MCAsmParser::Error();
+  // class-note@-1 {{Calling 'MCAsmParser::Error'}}
+  // class-note@-2 {{Returning from 'MCAsmParser::Error'}}
+}
+
+bool parseFile() {
+  Foo F;
+  if (parseFoo(F)) {
+// class-note@-1 {{Calling 'parseFoo'}}
+// class-note@-2 {{Returning from 'parseFoo'}}
+// class-note@-3 {{Taking false branch}}
+return true;
+  }
+
+  (void)(1 / F.Field);
+  // class-warning@-1 {{Division by zero}}
+  // class-note@-2 {{Division by zero}}
+  return false;
+}
+} // namespace test_classes
Index: clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
===
--- clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -780,6 +780,9 @@
   NewAllocElt->getAllocatorExpr()->getBeginLoc(), SMng);
 }
 llvm_unreachable("Unexpected CFG element at front of block");
+  } else if (Optional FE = P.getAs()) {
+return PathDiagnosticLocation(FE->getStmt(), SMng,
+  FE->getLocationContext());
   } else {
 llvm_unreachable("Unexpected ProgramPoint");
   }
Index: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp
===
--- /dev/null
+++ clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp
@@ -0,0 +1,170 @@
+//===- ReturnValueChecker - Applies guaranteed return values *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This defines ReturnValueChecker, which checks for calls with guaranteed
+// boolean return value. It ensures the return value of each function call.
+//
+//===--===//
+
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/St

[PATCH] D63915: [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of function calls

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

Thanks for the reviews!

In D63915#1569410 , @Szelethus wrote:

> This checker isn't in alpha -- did you evaluate it on LLVM? Other than that, 
> looks great!


Yes, it is made for LLVM and tested out 4 times. Thanks!


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

https://reviews.llvm.org/D63915



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

[PATCH] D64123: Add clang-llvm-rename tool.

[Rui Ueyama via Phabricator via cfe-commits]

ruiu added a comment.

In D64123#1568096 , @Eugene.Zelenko 
wrote:

> There is clang-rename 
>  
> already. May be new functionality should be added there?


clang-rename seems to focus on renaming a variable in a selected region with 
the interactive use in mind. I.e. a user selects a variable on an IDE and 
rename its definition and all references.

I actually tried to create this tool based on clang-rename but failed because 
clang-rename was too slow for my purpose. It takes a non-negligible amount of 
time even for renaming a single variable, and I wanted to rename thousands or 
tens of thousands variables.

There are a few other significant differences between this tool and 
clang-rename as shown below:

- this tool renames a variable only when it can see the definition of the 
variable, while clang-rename renames a specified variable unconditionally
- clang-rename is an interactive tool, and you need to specify a new name by 
hand. On the other hand, this tool automatically generates a new name for each 
variable
- the core part of my tool is to find variables to rename, while clang-rename 
doesn't do anything for that

Overall, this tool is I think significantly different from clang-rename.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64123



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

[PATCH] D64123: Add clang-llvm-rename tool.

[Rui Ueyama via Phabricator via cfe-commits]

ruiu updated this revision to Diff 207937.
ruiu added a comment.

- removed a special rule for `E`
- do not lowercase global variables whose name is all uppercase
- OSec -> osec


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64123

Files:
  clang-tools-extra/CMakeLists.txt
  clang-tools-extra/clang-llvm-rename/CMakeLists.txt
  clang-tools-extra/clang-llvm-rename/ClangLLVMRename.cpp

Index: clang-tools-extra/clang-llvm-rename/ClangLLVMRename.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-llvm-rename/ClangLLVMRename.cpp
@@ -0,0 +1,275 @@
+//=== ClangLLVMRename.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This is a refactoring tool to rename variables so that they start with a
+// lowercase letter. This tool is intended to be used to rename variables in
+// LLVM codebase in which variable names start with an uppercase letter at
+// the moment.
+//
+//   Usage:
+//   clang-llmv-rename   ...
+//
+//  ... specify the paths of files in the CMake source tree. This
+// path is looked up in the compile command database.
+//
+//
+// For each variable in given files, the tool first check whether the
+// variable's definition is in one of given files or not, and rename it if
+// and only if it can find a definition of the variable. If the tool cannot
+// modify a definition of a variable, it doesn't rename it, in order to keep
+// a program compiles.
+//
+// Note that this tool is not perfect; it doesn't resolve or even detect
+// name conflicts caused by renaming. You may need to rename variables
+// before using this tool so that your program is free from name conflicts
+// due to lowercase/uppercase renaming.
+//
+//===--===//
+
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Rewrite/Core/Rewriter.h"
+#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/Execution.h"
+#include "clang/Tooling/Refactoring.h"
+#include "clang/Tooling/Refactoring/AtomicChange.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/StringSet.h"
+#include "llvm/ADT/StringSwitch.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Signals.h"
+
+using namespace clang;
+using namespace clang::ast_matchers;
+using namespace clang::tooling;
+using namespace llvm;
+
+namespace {
+class RenameCallback : public MatchFinder::MatchCallback {
+public:
+  RenameCallback(
+  std::map &FileToReplacements,
+  ArrayRef Paths)
+  : FileToReplacements(FileToReplacements) {
+for (StringRef S : Paths)
+  InputFiles.insert(canonicalizePath(S));
+  }
+
+  // This function is called for each AST pattern matche.
+  void run(const MatchFinder::MatchResult &Result) override {
+SourceManager &SM = *Result.SourceManager;
+
+if (auto *D = Result.Nodes.getNodeAs("VarDecl")) {
+  if (isa(D))
+return;
+  if (isGlobalConst(D))
+return;
+  convert(SM, D->getLocation(), D->getName());
+  return;
+}
+
+if (auto *D = Result.Nodes.getNodeAs("ParmVarDecl")) {
+  if (auto *Fn =
+  dyn_cast_or_null(D->getParentFunctionOrMethod()))
+if (Fn->isImplicit())
+  return;
+  convert(SM, D->getLocation(), "");
+  return;
+}
+
+if (auto *D = Result.Nodes.getNodeAs("FieldDecl")) {
+  convert(SM, D->getLocation(), "");
+  return;
+}
+
+if (auto *D = Result.Nodes.getNodeAs("DeclRefExpr")) {
+  if (!isInGivenFiles(SM, D->getFoundDecl()->getLocation()))
+return;
+  if (isa(D->getDecl()) ||
+  isa(D->getDecl()))
+return;
+  if (auto *Decl = dyn_cast(D->getFoundDecl()))
+if (isGlobalConst(Decl))
+  return;
+  if (D->getDecl()->getName().empty())
+return;
+  convert(SM, D->getLocation(), "");
+  return;
+}
+
+if (auto *D = Result.Nodes.getNodeAs("MemberExpr")) {
+  if (!isInGivenFiles(SM, D->getFoundDecl()->getLocation()))
+return;
+  if (D->getMemberDecl()->getName().empty())
+return;
+  convert(SM, D->getMemberLoc(), D->getMemberDecl()->getName());
+  return;
+}
+
+if (auto *D =
+Result.Nodes.getNodeAs("CXXCtorInitializer")) {
+  if (!isInGiv

[PATCH] D63908: hwasan: Improve precision of checks using short granule tags.

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis added inline comments.



Comment at: compiler-rt/lib/hwasan/hwasan_allocator.cpp:159
 ? (t ? t->GenerateRandomTag() : kFallbackAllocTag)
 : 0;
+uptr tag_size = orig_size ? orig_size : 1;

When !(flags()->tag_in_malloc && malloc_bisect(stack, orig_size)), the tail tag 
should be 0 as well.



Comment at: compiler-rt/lib/hwasan/hwasan_checks.h:69
+return true;
+  if (mem_tag > 15)
+return false;

s/15/kShadowAlignment -1/



Comment at: compiler-rt/lib/hwasan/hwasan_checks.h:113
+  if (UNLIKELY(tail_sz != 0 && !PossiblyShortTagMatches(
+   *shadow_last, end & ~0xfull, tail_sz))) {
+SigTrap<0x20 * (EA == ErrorAction::Recover) +

0xfull, nice :)
A named constant please.



Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:1195
+  auto *NewAI = new AllocaInst(
+  TypeWithPadding, AI->getType()->getAddressSpace(), nullptr, "", AI);
+  NewAI->takeName(AI);

Good. I think we will need to do the same in MTE patches, but for different 
reason.

There is something in BasicAA that thinks that a store of size 16 (in 
MachineInstr) can not possibly alias with a smaller alloca, so simply 
increasing alloca alignment is not enough.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63908



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

[PATCH] D62645: [Sema] Resolve placeholder types before type deduction to silence spurious `-Warc-repeated-use-of-weak` warnings

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 207931.
ahatanak marked 2 inline comments as done.
ahatanak added a comment.

Add an implicit-only attribute `ObjCDefaultedAnyToId` to avoid passing the flag 
down.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62645

Files:
  include/clang/AST/Expr.h
  include/clang/Basic/Attr.td
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprCXX.cpp
  test/SemaObjC/arc-repeated-weak.mm

Index: test/SemaObjC/arc-repeated-weak.mm
===
--- test/SemaObjC/arc-repeated-weak.mm
+++ test/SemaObjC/arc-repeated-weak.mm
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime-has-weak -fobjc-arc -fblocks -Wno-objc-root-class -std=c++11 -Warc-repeated-use-of-weak -verify %s
-// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime-has-weak -fobjc-weak -fblocks -Wno-objc-root-class -std=c++11 -Warc-repeated-use-of-weak -verify %s
+// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime-has-weak -fobjc-arc -fblocks -Wno-objc-root-class -std=c++14 -Warc-repeated-use-of-weak -verify %s
+// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime-has-weak -fobjc-weak -fblocks -Wno-objc-root-class -std=c++14 -Warc-repeated-use-of-weak -verify %s
 
 @interface Test {
 @public
@@ -467,6 +467,18 @@
   __typeof__(NSBundle2.foo2.weakProp) t5;
 }
 
+void testAuto() {
+  auto __weak wp = NSBundle2.foo2.weakProp;
+}
+
+void testLambdaCaptureInit() {
+  [capture(NSBundle2.foo2.weakProp)] {} ();
+}
+
+void testAutoNew() {
+  auto p = new auto(NSBundle2.foo2.weakProp);
+}
+
 // This used to crash in the constructor of WeakObjectProfileTy when a
 // DeclRefExpr was passed that didn't reference a VarDecl.
 
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -1793,6 +1793,14 @@
 NumInits = List->getNumExprs();
   }
 
+  for (unsigned I = 0, E = NumInits; I != E; ++I)
+if (Inits[I]->hasNonOverloadPlaceholderType()) {
+  ExprResult Result = CheckPlaceholderExpr(Inits[I]);
+  if (!Result.isUsable())
+return ExprError();
+  Inits[I] = Result.get();
+}
+
   // C++11 [expr.new]p15:
   //   A new-expression that creates an object of type T initializes that
   //   object as follows:
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -6678,6 +6678,14 @@
 ExprResult Sema::ActOnParenListExpr(SourceLocation L,
 SourceLocation R,
 MultiExprArg Val) {
+  for (size_t I = 0, E = Val.size(); I != E; ++I)
+if (Val[I]->hasNonOverloadPlaceholderType()) {
+  ExprResult Result = CheckPlaceholderExpr(Val[I]);
+  if (!Result.isUsable())
+return ExprError();
+  Val[I] = Result.get();
+}
+
   return ParenListExpr::Create(Context, L, Val, R);
 }
 
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -10985,18 +10985,6 @@
 return QualType();
   }
 
-  // Expressions default to 'id' when we're in a debugger.
-  bool DefaultedAnyToId = false;
-  if (getLangOpts().DebuggerCastResultToId &&
-  Init->getType() == Context.UnknownAnyTy && !IsInitCapture) {
-ExprResult Result = forceUnknownAnyToType(Init, Context.getObjCIdType());
-if (Result.isInvalid()) {
-  return QualType();
-}
-Init = Result.get();
-DefaultedAnyToId = true;
-  }
-
   // C++ [dcl.decomp]p1:
   //   If the assignment-expression [...] has array type A and no ref-qualifier
   //   is present, e has type cv A
@@ -11030,6 +11018,7 @@
   // checks.
   // We only want to warn outside of template instantiations, though:
   // inside a template, the 'id' could have come from a parameter.
+  bool DefaultedAnyToId = VDecl && VDecl->hasAttr();
   if (!inTemplateInstantiation() && !DefaultedAnyToId && !IsInitCapture &&
   !DeducedType.isNull() && DeducedType->isObjCIdType()) {
 SourceLocation Loc = TSI->getTypeLoc().getBeginLoc();
@@ -11108,6 +11097,27 @@
 }
 Init = Res.get();
 
+// Expressions default to 'id' when we're in a debugger
+// and we are assigning it to a variable of Objective-C pointer type.
+if (getLangOpts().DebuggerCastResultToId &&
+Init->getType() == Context.UnknownAnyTy) {
+  ExprResult Result = forceUnknownAnyToType(Init, Context.getObjCIdType());
+  if (Result.isInvalid()) {
+VDecl->setInvalidDecl();
+return;
+  }
+  Init = Result.get();
+  VDecl->addAttr(ObjCDefaultedAnyToIdAttr::CreateImplicit(Context));
+} else if (!Init->getType().isNull() &&
+   Init->hasNonOverloadPlaceholderType()) {
+  Res = CheckPlaceholderExpr(Init).get();
+  if (!Res.isUsable()) {
+VDecl->setIn

[PATCH] D64128: [CodeGen] Generate llvm.ptrmask instead of inttoptr(and(ptrtoint, C)) if possible.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> If they're all syntactically together like this, maybe that's safe?

Having them together syntactically doesn't really help, I think; it might be 
guarded by some code that does the same conversion (and if you repeat the 
conversion, it has to produce the same result).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64128



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

[PATCH] D63889: Check possible warnings on global initializers for reachability

[Nathan Huckleberry via Phabricator via cfe-commits]

Nathan-Huckleberry updated this revision to Diff 207925.
Nathan-Huckleberry added a comment.

- Stylistic fixes of function names and removal of namespace prefixes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63889

Files:
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/Sema/AnalysisBasedWarnings.h
  clang/lib/Analysis/AnalysisDeclContext.cpp
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/warn-unreachable-warning-var-decl.cpp

Index: clang/test/Sema/warn-unreachable-warning-var-decl.cpp
===
--- /dev/null
+++ clang/test/Sema/warn-unreachable-warning-var-decl.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -verify %s
+int e = 1 ? 0 : 1 / 0;
+int g = 1 ? 1 / 0 : 0; // expected-warning{{division by zero is undefined}}
+
+#define SHIFT(n) (((n) == 32) ? 0 : ((1 << (n)) - 1))
+
+int x = SHIFT(32);
+int y = SHIFT(0);
+
+// FIXME: Expressions in lambdas aren't ignored
+int z = []() {
+  return 1 ? 0 : 1 / 0; // expected-warning{{division by zero is undefined}}
+}();
+
+int f(void) {
+  int x = 1 ? 0 : 1 / 0;
+  return x;
+}
+
+template 
+struct X0 {
+  static T value;
+};
+
+template 
+struct X1 {
+  static T value;
+};
+
+template 
+T X0::value = 3.14; // expected-warning{{implicit conversion from 'double' to 'int' changes value from 3.14 to 3}}
+
+template 
+T X1::value = 1 ? 0 : 1 / 0;
+
+template struct X0; // expected-note{{in instantiation of static data member 'X0::value' requested here}}
+
+constexpr signed char c1 = 100 * 2;   // expected-warning{{changes value}}
+constexpr signed char c2 = '\x64' * '\2'; // expected-warning{{changes value}}
+constexpr int shr_32 = 0 >> 32;   // expected-error {{constant expression}} expected-note {{shift count 32 >= width of type}}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -4881,6 +4881,8 @@
"default argument expression has capturing blocks?");
   }
 
+  AnalysisWarnings.IssueWarningsForRegisteredVarDecl(Param);
+
   // We already type-checked the argument, so we know it works.
   // Just mark all of the declarations in this potentially-evaluated expression
   // as being "referenced".
@@ -1,8 +16668,8 @@
   case ExpressionEvaluationContext::PotentiallyEvaluated:
   case ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed:
 if (!Stmts.empty() && getCurFunctionOrMethodDecl()) {
-  FunctionScopes.back()->PossiblyUnreachableDiags.
-push_back(sema::PossiblyUnreachableDiag(PD, Loc, Stmts));
+  FunctionScopes.back()->PossiblyUnreachableDiags.push_back(
+  PossiblyUnreachableDiag(PD, Loc, Stmts));
   return true;
 }
 
@@ -16676,13 +16678,29 @@
 // but nonetheless is always required to be a constant expression, so we
 // can skip diagnosing.
 // FIXME: Using the mangling context here is a hack.
+//
+// Mangling context seems to only be defined on constexpr vardecl that
+// displayed errors.
+// This skips warnings that were already emitted as notes on errors.
 if (auto *VD = dyn_cast_or_null(
 ExprEvalContexts.back().ManglingContextDecl)) {
   if (VD->isConstexpr() ||
   (VD->isStaticDataMember() && VD->isFirstDecl() && !VD->isInline()))
 break;
-  // FIXME: For any other kind of variable, we should build a CFG for its
-  // initializer and check whether the context in question is reachable.
+}
+
+// For any other kind of variable, we should build a CFG for its
+// initializer and check whether the context in question is reachable.
+if (auto *VD = dyn_cast_or_null(CurContext->getLastDecl())) {
+  if (VD->getDefinition()) {
+VD = VD->getDefinition();
+  }
+  // FIXME: Some cases aren't picked up by path analysis currently
+  if (!Stmts.empty() && VD->isFileVarDecl()) {
+AnalysisWarnings.RegisterVarDeclWarning(
+VD, PossiblyUnreachableDiag(PD, Loc, Stmts));
+return true;
+  }
 }
 
 Diag(Loc, PD);
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -288,6 +288,9 @@
 UnparsedDefaultArgInstantiations.erase(InstPos);
   }
 
+  // Check for delayed warnings
+  AnalysisWarnings.IssueWarningsForRegisteredVarDecl(Param);
+
   return false;
 }
 
@@ -333,7 +336,21 @@
 return;
   }
 
+  // Temporarily change the context and add param to it.
+  // Allows DiagRuntimeBehavior to register this param with
+  // any possibly warnings.
+  // Param gets added back to context when function is added
+  // to context.
+  // FIXME: Params should probably be diagnosed afte

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

[Johannes Doerfert via Phabricator via cfe-commits]

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

Rebase (tests will be run later today)


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,

[PATCH] D64128: [CodeGen] Generate llvm.ptrmask instead of inttoptr(and(ptrtoint, C)) if possible.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I agree with Eli that this isn't obviously a legal transformation.  
`llvm.ptrmask` appears to make semantic guarantees about e.g. the pointer after 
the mask referring to the same underlying object, which means we can only 
safely emit it when something about the source program makes that guarantee.  
It's not at all clear that C does so for an expression like `(T*) ((intptr_t) x 
& N)`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64128



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

[PATCH] D64128: [CodeGen] Generate llvm.ptrmask instead of inttoptr(and(ptrtoint, C)) if possible.

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In D64128#1568857 , @efriedma wrote:

> I don't think this transform is valid, for the same reasons we don't do it in 
> IR optimizations.


I believe that in the problematic cases we previously discussed (e.g., from 
https://reviews.llvm.org/D59065#1449682), they all depend on some control 
dependency being introduced somewhere in between the initial pointer casts and 
the other operations. If they're all syntactically together like this, maybe 
that's safe?

Does this actually catch the ABI code that motivated this in the first place? 
Isn't that in lib/CodeGen/TargetInfo.cpp (e.g., in 
emitRoundPointerUpToAlignment)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64128



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

Re: [PATCH] D64149: [clang-scan-deps] use `-Wno-error` when scanning for dependencies

[Alex L via cfe-commits]

The warning output is suppressed, yes. Yeah, we should probably just
disable them. I'll follow up with a commit.

On Wed, 3 Jul 2019 at 15:25, Duncan P. N. Exon Smith via Phabricator <
revi...@reviews.llvm.org> wrote:

> dexonsmith added a comment.
>
> Is warning output suppressed?  If so, should we just/also disable all
> warnings?  (IIRC, the flag is `-w`.)
>
>
> Repository:
>   rL LLVM
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D64149/new/
>
> https://reviews.llvm.org/D64149
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63889: Check possible warnings on global initializers for reachability

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/include/clang/Sema/AnalysisBasedWarnings.h:95
 
+  void flushDiagnostics(SmallVector);
+

Methods should be UpperCamelCased.



Comment at: clang/include/clang/Sema/AnalysisBasedWarnings.h:112
+  void
+  emitPossiblyUnreachableDiags(AnalysisDeclContext &AC,
+   SmallVector PUDs);

UpperCamelCase



Comment at: clang/lib/Sema/SemaExpr.cpp:16672
+  FunctionScopes.back()->PossiblyUnreachableDiags.push_back(
+  clang::sema::PossiblyUnreachableDiag(PD, Loc, Stmts));
   return true;

does this need the `clang::` qualifier?



Comment at: clang/lib/Sema/SemaExpr.cpp:16701
+AnalysisWarnings.RegisterVarDeclWarning(
+VD, clang::sema::PossiblyUnreachableDiag(PD, Loc, Stmts));
+return true;

does this need `clang::`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63889



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

[PATCH] D64169: ARM MTE stack sanitizer.

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis updated this revision to Diff 207920.
eugenis added a comment.

fix bitcode docs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64169

Files:
  clang/include/clang/Basic/Features.def
  clang/include/clang/Basic/Sanitizers.def
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/SanitizerMetadata.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/test/CodeGen/memtag-attr.cpp
  clang/test/Driver/fsanitize.c
  clang/test/Lexer/has_feature_memtag_sanitizer.cpp
  clang/test/SemaCXX/attr-no-sanitize.cpp
  llvm/docs/BitCodeFormat.rst
  llvm/docs/LangRef.rst
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Transforms/IPO/ForceFunctionAttrs.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  llvm/test/Bitcode/attributes.ll
  llvm/test/Transforms/Inline/attributes.ll
  llvm/utils/emacs/llvm-mode.el

Index: llvm/utils/emacs/llvm-mode.el
===
--- llvm/utils/emacs/llvm-mode.el
+++ llvm/utils/emacs/llvm-mode.el
@@ -26,7 +26,7 @@
  "inaccessiblemem_or_argmemonly" "inlinehint" "jumptable" "minsize" "naked" "nobuiltin"
  "noduplicate" "noimplicitfloat" "noinline" "nonlazybind" "noredzone" "noreturn"
  "norecurse" "nounwind" "optnone" "optsize" "readnone" "readonly" "returns_twice"
- "speculatable" "ssp" "sspreq" "sspstrong" "safestack" "sanitize_address" "sanitize_hwaddress"
+ "speculatable" "ssp" "sspreq" "sspstrong" "safestack" "sanitize_address" "sanitize_hwaddress" "sanitize_memtag"
  "sanitize_thread" "sanitize_memory" "strictfp" "uwtable" "writeonly" "immarg") 'symbols) . font-lock-constant-face)
;; Variables
'("%[-a-zA-Z$._][-a-zA-Z$._0-9]*" . font-lock-variable-name-face)
Index: llvm/test/Transforms/Inline/attributes.ll
===
--- llvm/test/Transforms/Inline/attributes.ll
+++ llvm/test/Transforms/Inline/attributes.ll
@@ -22,6 +22,10 @@
   ret i32 %i
 }
 
+define i32 @sanitize_memtag_callee(i32 %i) sanitize_memtag {
+  ret i32 %i
+}
+
 define i32 @safestack_callee(i32 %i) safestack {
   ret i32 %i
 }
@@ -50,6 +54,10 @@
   ret i32 %i
 }
 
+define i32 @alwaysinline_sanitize_memtag_callee(i32 %i) alwaysinline sanitize_memtag {
+  ret i32 %i
+}
+
 define i32 @alwaysinline_safestack_callee(i32 %i) alwaysinline safestack {
   ret i32 %i
 }
@@ -104,6 +112,17 @@
 ; CHECK-NEXT: ret i32
 }
 
+define i32 @test_no_sanitize_memtag(i32 %arg) {
+  %x1 = call i32 @noattr_callee(i32 %arg)
+  %x2 = call i32 @sanitize_memtag_callee(i32 %x1)
+  %x3 = call i32 @alwaysinline_callee(i32 %x2)
+  %x4 = call i32 @alwaysinline_sanitize_memtag_callee(i32 %x3)
+  ret i32 %x4
+; CHECK-LABEL: @test_no_sanitize_memtag(
+; CHECK-NEXT: @sanitize_memtag_callee
+; CHECK-NEXT: ret i32
+}
+
 
 ; Check that:
 ;  * noattr callee is not inlined into sanitize_(address|memory|thread) caller,
@@ -154,6 +173,17 @@
 ; CHECK-NEXT: ret i32
 }
 
+define i32 @test_sanitize_memtag(i32 %arg) sanitize_memtag {
+  %x1 = call i32 @noattr_callee(i32 %arg)
+  %x2 = call i32 @sanitize_memtag_callee(i32 %x1)
+  %x3 = call i32 @alwaysinline_callee(i32 %x2)
+  %x4 = call i32 @alwaysinline_sanitize_memtag_callee(i32 %x3)
+  ret i32 %x4
+; CHECK-LABEL: @test_sanitize_memtag(
+; CHECK-NEXT: @noattr_callee
+; CHECK-NEXT: ret i32
+}
+
 define i32 @test_safestack(i32 %arg) safestack {
   %x1 = call i32 @noattr_callee(i32 %arg)
   %x2 = call i32 @safestack_callee(i32 %x1)
Index: llvm/test/Bitcode/attributes.ll
===
--- llvm/test/Bitcode/attributes.ll
+++ llvm/test/Bitcode/attributes.ll
@@ -204,7 +204,7 @@
 ; CHECK: define void @f34()
 {
 call void @nobuiltin() nobuiltin
-; CHECK: call void @nobuiltin() #37
+; CHECK: call void @nobuiltin() #38
 ret void;
 }
 
@@ -357,6 +357,12 @@
   ret void
 }
 
+; CHECK: define void @f61() #37
+define void @f61() sanitize_memtag
+{
+ret void;
+}
+
 ; CHECK: attributes #0 = { noreturn }
 ; CHECK: attributes #1 = { nounwind }
 ; CHECK: attributes #2 = { readnone }
@@ -394,4 +400,5 @@
 ; CHECK: attributes #34 = { sanitize_hwaddress }
 ; CHECK: attributes #35 = { shadowcallstack }
 ; CHECK: attributes #36 = { willreturn }
-; CHECK: attributes #37 = { nobuiltin }
+; CHECK: attributes #37 = { sanitize_memtag }
+; CHECK: attributes #38 = { nobuiltin }
Index: llvm/lib/Transforms/Utils/CodeExtractor.cpp
===
--- llvm/lib/Transforms/Utils/CodeExtrac

[PATCH] D64169: ARM MTE stack sanitizer.

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis created this revision.
eugenis added reviewers: pcc, hctim, vitalybuka, ostannard.
Herald added subscribers: dexonsmith, steven_wu, cryptoad, hiraditya, 
kristof.beyls, javed.absar, mehdi_amini, srhines.
Herald added projects: clang, LLVM.

Add "memtag" sanitizer that detects and mitigates stack memory issues
using armv8.5 Memory Tagging Extension.

It is similar in principle to HWASan, which is a software implementation
of the same idea, but there are enough differencies to warrant a new
sanitizer type IMHO. It is also expected to have very different
performance properties.

The new sanitizer does not have a runtime library (it may grow one
later, along with a "debugging" mode). Similar to SafeStack and
StackProtector, the instrumentation pass (in a follow up change) will be
inserted in all cases, but will only affect functions marked with the
new sanitize_memtag attribute.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64169

Files:
  clang/include/clang/Basic/Features.def
  clang/include/clang/Basic/Sanitizers.def
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/SanitizerMetadata.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/test/CodeGen/memtag-attr.cpp
  clang/test/Driver/fsanitize.c
  clang/test/Lexer/has_feature_memtag_sanitizer.cpp
  clang/test/SemaCXX/attr-no-sanitize.cpp
  llvm/docs/BitCodeFormat.rst
  llvm/docs/LangRef.rst
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Transforms/IPO/ForceFunctionAttrs.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  llvm/test/Bitcode/attributes.ll
  llvm/test/Transforms/Inline/attributes.ll
  llvm/utils/emacs/llvm-mode.el

Index: llvm/utils/emacs/llvm-mode.el
===
--- llvm/utils/emacs/llvm-mode.el
+++ llvm/utils/emacs/llvm-mode.el
@@ -26,7 +26,7 @@
  "inaccessiblemem_or_argmemonly" "inlinehint" "jumptable" "minsize" "naked" "nobuiltin"
  "noduplicate" "noimplicitfloat" "noinline" "nonlazybind" "noredzone" "noreturn"
  "norecurse" "nounwind" "optnone" "optsize" "readnone" "readonly" "returns_twice"
- "speculatable" "ssp" "sspreq" "sspstrong" "safestack" "sanitize_address" "sanitize_hwaddress"
+ "speculatable" "ssp" "sspreq" "sspstrong" "safestack" "sanitize_address" "sanitize_hwaddress" "sanitize_memtag"
  "sanitize_thread" "sanitize_memory" "strictfp" "uwtable" "writeonly" "immarg") 'symbols) . font-lock-constant-face)
;; Variables
'("%[-a-zA-Z$._][-a-zA-Z$._0-9]*" . font-lock-variable-name-face)
Index: llvm/test/Transforms/Inline/attributes.ll
===
--- llvm/test/Transforms/Inline/attributes.ll
+++ llvm/test/Transforms/Inline/attributes.ll
@@ -22,6 +22,10 @@
   ret i32 %i
 }
 
+define i32 @sanitize_memtag_callee(i32 %i) sanitize_memtag {
+  ret i32 %i
+}
+
 define i32 @safestack_callee(i32 %i) safestack {
   ret i32 %i
 }
@@ -50,6 +54,10 @@
   ret i32 %i
 }
 
+define i32 @alwaysinline_sanitize_memtag_callee(i32 %i) alwaysinline sanitize_memtag {
+  ret i32 %i
+}
+
 define i32 @alwaysinline_safestack_callee(i32 %i) alwaysinline safestack {
   ret i32 %i
 }
@@ -104,6 +112,17 @@
 ; CHECK-NEXT: ret i32
 }
 
+define i32 @test_no_sanitize_memtag(i32 %arg) {
+  %x1 = call i32 @noattr_callee(i32 %arg)
+  %x2 = call i32 @sanitize_memtag_callee(i32 %x1)
+  %x3 = call i32 @alwaysinline_callee(i32 %x2)
+  %x4 = call i32 @alwaysinline_sanitize_memtag_callee(i32 %x3)
+  ret i32 %x4
+; CHECK-LABEL: @test_no_sanitize_memtag(
+; CHECK-NEXT: @sanitize_memtag_callee
+; CHECK-NEXT: ret i32
+}
+
 
 ; Check that:
 ;  * noattr callee is not inlined into sanitize_(address|memory|thread) caller,
@@ -154,6 +173,17 @@
 ; CHECK-NEXT: ret i32
 }
 
+define i32 @test_sanitize_memtag(i32 %arg) sanitize_memtag {
+  %x1 = call i32 @noattr_callee(i32 %arg)
+  %x2 = call i32 @sanitize_memtag_callee(i32 %x1)
+  %x3 = call i32 @alwaysinline_callee(i32 %x2)
+  %x4 = call i32 @alwaysinline_sanitize_memtag_callee(i32 %x3)
+  ret i32 %x4
+; CHECK-LABEL: @test_sanitize_memtag(
+; CHECK-NEXT: @noattr_callee
+; CHECK-NEXT: ret i32
+}
+
 define i32 @test_safestack(i32 %arg) safestack {
   %x1 = call i32 @noattr_callee(i32 %arg)
   %x2 = call i32 @safestack_callee(i32 %x1)
Index: llvm/test/Bitcode/attributes.ll
===
--- llvm/test/Bitcode/attributes.ll
+++ llvm/test/Bitcode/attributes.ll
@@ -204,7 +204,7 @@
 ; CHECK: define void @f34()
 {
 call void @nobuiltin() nobuiltin
-; CHECK: call void @nobuiltin() #37
+; CHECK: call voi

[PATCH] D64164: [clang][ARM] Fix msvc arm{64} builtins to use int on LLP64 systems.

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese added a comment.

In D64164#1569442 , @efriedma wrote:

> Do the changes to BuiltinsARM.def have any practical effect?  long should be 
> 32 bits on all 32-bit ARM targets.


I don't think they do right now.  I updated them there as that's what the 
original patch did.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64164



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

[PATCH] D63889: Check possible warnings on global initializers for reachability

[Nathan Huckleberry via Phabricator via cfe-commits]

Nathan-Huckleberry updated this revision to Diff 207918.
Nathan-Huckleberry added a comment.

- Small functional and formatting changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63889

Files:
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/Sema/AnalysisBasedWarnings.h
  clang/lib/Analysis/AnalysisDeclContext.cpp
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/warn-unreachable-warning-var-decl.cpp

Index: clang/test/Sema/warn-unreachable-warning-var-decl.cpp
===
--- /dev/null
+++ clang/test/Sema/warn-unreachable-warning-var-decl.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -verify %s
+int e = 1 ? 0 : 1 / 0;
+int g = 1 ? 1 / 0 : 0; // expected-warning{{division by zero is undefined}}
+
+#define SHIFT(n) (((n) == 32) ? 0 : ((1 << (n)) - 1))
+
+int x = SHIFT(32);
+int y = SHIFT(0);
+
+// FIXME: Expressions in lambdas aren't ignored
+int z = []() {
+  return 1 ? 0 : 1 / 0; // expected-warning{{division by zero is undefined}}
+}();
+
+int f(void) {
+  int x = 1 ? 0 : 1 / 0;
+  return x;
+}
+
+template 
+struct X0 {
+  static T value;
+};
+
+template 
+struct X1 {
+  static T value;
+};
+
+template 
+T X0::value = 3.14; // expected-warning{{implicit conversion from 'double' to 'int' changes value from 3.14 to 3}}
+
+template 
+T X1::value = 1 ? 0 : 1 / 0;
+
+template struct X0; // expected-note{{in instantiation of static data member 'X0::value' requested here}}
+
+constexpr signed char c1 = 100 * 2;   // expected-warning{{changes value}}
+constexpr signed char c2 = '\x64' * '\2'; // expected-warning{{changes value}}
+constexpr int shr_32 = 0 >> 32;   // expected-error {{constant expression}} expected-note {{shift count 32 >= width of type}}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -4881,6 +4881,8 @@
"default argument expression has capturing blocks?");
   }
 
+  AnalysisWarnings.IssueWarningsForRegisteredVarDecl(Param);
+
   // We already type-checked the argument, so we know it works.
   // Just mark all of the declarations in this potentially-evaluated expression
   // as being "referenced".
@@ -1,8 +16668,8 @@
   case ExpressionEvaluationContext::PotentiallyEvaluated:
   case ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed:
 if (!Stmts.empty() && getCurFunctionOrMethodDecl()) {
-  FunctionScopes.back()->PossiblyUnreachableDiags.
-push_back(sema::PossiblyUnreachableDiag(PD, Loc, Stmts));
+  FunctionScopes.back()->PossiblyUnreachableDiags.push_back(
+  clang::sema::PossiblyUnreachableDiag(PD, Loc, Stmts));
   return true;
 }
 
@@ -16676,13 +16678,29 @@
 // but nonetheless is always required to be a constant expression, so we
 // can skip diagnosing.
 // FIXME: Using the mangling context here is a hack.
+//
+// Mangling context seems to only be defined on constexpr vardecl that
+// displayed errors.
+// This skips warnings that were already emitted as notes on errors.
 if (auto *VD = dyn_cast_or_null(
 ExprEvalContexts.back().ManglingContextDecl)) {
   if (VD->isConstexpr() ||
   (VD->isStaticDataMember() && VD->isFirstDecl() && !VD->isInline()))
 break;
-  // FIXME: For any other kind of variable, we should build a CFG for its
-  // initializer and check whether the context in question is reachable.
+}
+
+// For any other kind of variable, we should build a CFG for its
+// initializer and check whether the context in question is reachable.
+if (auto *VD = dyn_cast_or_null(CurContext->getLastDecl())) {
+  if (VD->getDefinition()) {
+VD = VD->getDefinition();
+  }
+  // FIXME: Some cases aren't picked up by path analysis currently
+  if (!Stmts.empty() && VD->isFileVarDecl()) {
+AnalysisWarnings.RegisterVarDeclWarning(
+VD, clang::sema::PossiblyUnreachableDiag(PD, Loc, Stmts));
+return true;
+  }
 }
 
 Diag(Loc, PD);
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -288,6 +288,9 @@
 UnparsedDefaultArgInstantiations.erase(InstPos);
   }
 
+  // Check for delayed warnings
+  AnalysisWarnings.IssueWarningsForRegisteredVarDecl(Param);
+
   return false;
 }
 
@@ -333,7 +336,21 @@
 return;
   }
 
+  // Temporarily change the context and add param to it.
+  // Allows DiagRuntimeBehavior to register this param with
+  // any possibly warnings.
+  // Param gets added back to context when function is added
+  // to context.
+  // FIXME: Params should probably be diagnosed after 

[PATCH] D63889: Check possible warnings on global initializers for reachability

[Nathan Huckleberry via Phabricator via cfe-commits]

Nathan-Huckleberry added inline comments.



Comment at: clang/lib/Analysis/AnalysisDeclContext.cpp:124
+if(VD->hasGlobalStorage()) {
+  return const_cast(dyn_cast(VD->getInit()));
+}

nickdesaulniers wrote:
> The `const_cast` doesn't look necessary here.  Is it?
`VD->getInit` returns a `const Expr *`. In order to remove the `const_cast` I 
would need to make `getBody()` `const` and every call to `getBody()`. The 
change required to add `const` seemed too large to be included in this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63889



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

[PATCH] D64164: [clang][ARM] Fix msvc arm{64} builtins to use int on LLP64 systems.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Do the changes to BuiltinsARM.def have any practical effect?  long should be 32 
bits on all 32-bit ARM targets.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64164



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

[PATCH] D64164: [clang][ARM] Fix msvc arm{64} builtins to use int on LLP64 systems.

[JF Bastien via Phabricator via cfe-commits]

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



Comment at: test/CodeGen/ms-intrinsics-other.c:212
+  return _InterlockedAdd(Addend, Value);
+}
+

Looks like only `cmpxchg` preserves `volatile`? That's not your patch, but 
seems bad 😖 


Repository:
  rC Clang

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

https://reviews.llvm.org/D64164



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

[PATCH] D64164: [clang][ARM] Fix msvc arm{64} builtins to use int on LLP64 systems.

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese created this revision.
Bigcheese added reviewers: bruno, rnk, bkelley.
Herald added subscribers: kristina, jfb, dexonsmith, kristof.beyls, javed.absar.
Herald added a project: clang.

The `InterlockedX_{acq,nf,rel}` functions deal with 32 bits which is long on
MSVC, but int on most other systems.

This also checks that `ReadStatusRegister` and `WriteStatusRegister` have
the correct type on aarch64-darwin.


Repository:
  rC Clang

https://reviews.llvm.org/D64164

Files:
  include/clang/Basic/BuiltinsAArch64.def
  include/clang/Basic/BuiltinsARM.def
  test/CodeGen/arm64-microsoft-status-reg.cpp
  test/CodeGen/ms-intrinsics-other.c

Index: test/CodeGen/ms-intrinsics-other.c
===
--- test/CodeGen/ms-intrinsics-other.c
+++ test/CodeGen/ms-intrinsics-other.c
@@ -4,6 +4,15 @@
 // RUN: %clang_cc1 -ffreestanding -fms-extensions \
 // RUN: -triple x86_64--linux -Oz -emit-llvm %s -o - \
 // RUN: | FileCheck %s
+// RUN: %clang_cc1 -ffreestanding -fms-extensions \
+// RUN: -triple aarch64--darwin -Oz -emit-llvm %s -o - \
+// RUN: | FileCheck %s --check-prefix=CHECK-ARM-ARM64
+// RUN: %clang_cc1 -ffreestanding -fms-extensions \
+// RUN: -triple aarch64--darwin -Oz -emit-llvm %s -o - \
+// RUN: | FileCheck %s --check-prefix=CHECK-ARM
+// RUN: %clang_cc1 -ffreestanding -fms-extensions \
+// RUN: -triple armv7--darwin -Oz -emit-llvm %s -o - \
+// RUN: | FileCheck %s --check-prefix=CHECK-ARM
 
 // LP64 targets use 'long' as 'int' for MS intrinsics (-fms-extensions)
 #ifdef __LP64__
@@ -196,3 +205,214 @@
 // CHECK:  [[RESULT:%[0-9]+]] = tail call i64 @llvm.ctpop.i64(i64 %x)
 // CHECK: ret i64 [[RESULT]]
 // CHECK: }
+
+#if defined(__aarch64__)
+LONG test_InterlockedAdd(LONG volatile *Addend, LONG Value) {
+  return _InterlockedAdd(Addend, Value);
+}
+
+// CHECK-ARM-ARM64: define{{.*}}i32 @test_InterlockedAdd(i32*{{[a-z_ ]*}}%Addend, i32 %Value) {{.*}} {
+// CHECK-ARM-ARM64: %[[OLDVAL:[0-9]+]] = atomicrmw add i32* %Addend, i32 %Value seq_cst
+// CHECK-ARM-ARM64: %[[NEWVAL:[0-9]+]] = add i32 %[[OLDVAL:[0-9]+]], %Value
+// CHECK-ARM-ARM64: ret i32 %[[NEWVAL:[0-9]+]]
+#endif
+
+#if defined(__arm__) || defined(__aarch64__)
+LONG test_InterlockedExchangeAdd_acq(LONG volatile *value, LONG mask) {
+  return _InterlockedExchangeAdd_acq(value, mask);
+}
+// CHECK-ARM: define{{.*}}i32 @test_InterlockedExchangeAdd_acq(i32*{{[a-z_ ]*}}%value, i32{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK-ARM:   [[RESULT:%[0-9]+]] = atomicrmw add i32* %value, i32 %mask acquire
+// CHECK-ARM:   ret i32 [[RESULT:%[0-9]+]]
+// CHECK-ARM: }
+LONG test_InterlockedExchangeAdd_rel(LONG volatile *value, LONG mask) {
+  return _InterlockedExchangeAdd_rel(value, mask);
+}
+// CHECK-ARM: define{{.*}}i32 @test_InterlockedExchangeAdd_rel(i32*{{[a-z_ ]*}}%value, i32{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK-ARM:   [[RESULT:%[0-9]+]] = atomicrmw add i32* %value, i32 %mask release
+// CHECK-ARM:   ret i32 [[RESULT:%[0-9]+]]
+// CHECK-ARM: }
+LONG test_InterlockedExchangeAdd_nf(LONG volatile *value, LONG mask) {
+  return _InterlockedExchangeAdd_nf(value, mask);
+}
+// CHECK-ARM: define{{.*}}i32 @test_InterlockedExchangeAdd_nf(i32*{{[a-z_ ]*}}%value, i32{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK-ARM:   [[RESULT:%[0-9]+]] = atomicrmw add i32* %value, i32 %mask monotonic
+// CHECK-ARM:   ret i32 [[RESULT:%[0-9]+]]
+// CHECK-ARM: }
+
+LONG test_InterlockedExchange_acq(LONG volatile *value, LONG mask) {
+  return _InterlockedExchange_acq(value, mask);
+}
+// CHECK-ARM: define{{.*}}i32 @test_InterlockedExchange_acq(i32*{{[a-z_ ]*}}%value, i32{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK-ARM:   [[RESULT:%[0-9]+]] = atomicrmw xchg i32* %value, i32 %mask acquire
+// CHECK-ARM:   ret i32 [[RESULT:%[0-9]+]]
+// CHECK-ARM: }
+LONG test_InterlockedExchange_rel(LONG volatile *value, LONG mask) {
+  return _InterlockedExchange_rel(value, mask);
+}
+// CHECK-ARM: define{{.*}}i32 @test_InterlockedExchange_rel(i32*{{[a-z_ ]*}}%value, i32{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK-ARM:   [[RESULT:%[0-9]+]] = atomicrmw xchg i32* %value, i32 %mask release
+// CHECK-ARM:   ret i32 [[RESULT:%[0-9]+]]
+// CHECK-ARM: }
+LONG test_InterlockedExchange_nf(LONG volatile *value, LONG mask) {
+  return _InterlockedExchange_nf(value, mask);
+}
+// CHECK-ARM: define{{.*}}i32 @test_InterlockedExchange_nf(i32*{{[a-z_ ]*}}%value, i32{{[a-z_ ]*}}%mask){{.*}}{
+// CHECK-ARM:   [[RESULT:%[0-9]+]] = atomicrmw xchg i32* %value, i32 %mask monotonic
+// CHECK-ARM:   ret i32 [[RESULT:%[0-9]+]]
+// CHECK-ARM: }
+
+LONG test_InterlockedCompareExchange_acq(LONG volatile *Destination, LONG Exchange, LONG Comperand) {
+  return _InterlockedCompareExchange_acq(Destination, Exchange, Comperand);
+}
+// CHECK-ARM: define{{.*}}i32 @test_InterlockedCompareExchange_acq(i32*{{[a-z_ ]*}}%Destination, i32{{[a-z_ ]*}}%Exchange, i32{{[a-z_ ]*}}%Comperand){{.*}}{
+// CHECK-ARM: [[TMP:%[0-9]+]] = cmpxchg volatile i32* %Destination, i3

[PATCH] D63915: [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of function calls

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

Szelethus added a comment.

This checker isn't in alpha -- did you evaluate it on LLVM? Other than that, 
looks great!




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:1119
 
-  const T *lookup(const CallEvent &Call) const {
+  Optional lookup(const CallEvent &Call) const {
 // Slow path: linear lookup.

NoQ wrote:
> Charusso wrote:
> > NoQ wrote:
> > > Charusso wrote:
> > > > Szelethus wrote:
> > > > > Charusso wrote:
> > > > > > NoQ wrote:
> > > > > > > I hope `T` never gets too expensive to copy. The ideal return 
> > > > > > > value here is `Optional` but i suspect that 
> > > > > > > `llvm::Optional`s don't support this (while C++17 
> > > > > > > `std::optional`s do). Could you double-check my vague memories 
> > > > > > > here?
> > > > > > Optional is working and used widely, I like that.
> > > > > Why do we need the optional AND the pointer? How about we just return 
> > > > > with a nullptr if we fail to find the call?
> > > > `Optional<>` stands for optional values, that is why it is made. @NoQ 
> > > > tried to avoid it, but I believe if someone does not use it for an 
> > > > optional value, that break some kind of unspoken standard.
> > > Well, that'd be the original code.
> > > 
> > > > I do not like `Optional` anymore.
> > > 
> > > @Charusso, do you still plan to undo this change?
> > Well, I am here at 2:1 against `Optional<>`, so I think it is your decision.
> I'd rather leave the original code as is and think more deeply about adding 
> support for `Optional` in the future.
Are you aware of this thread? 
http://lists.llvm.org/pipermail/cfe-dev/2018-July/058427.html


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

https://reviews.llvm.org/D63915



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

[PATCH] D63503: cmake: Add CLANG_LINK_CLANG_DYLIB option

[Tom Stellard 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 rL365092: cmake: Add CLANG_LINK_CLANG_DYLIB option (authored 
by tstellar, committed by ).
Herald added a project: LLVM.

Changed prior to commit:
  https://reviews.llvm.org/D63503?vs=205394&id=207906#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63503

Files:
  cfe/trunk/CMakeLists.txt
  cfe/trunk/cmake/modules/AddClang.cmake
  cfe/trunk/examples/AnnotateFunctions/CMakeLists.txt
  cfe/trunk/examples/PrintFunctionNames/CMakeLists.txt
  cfe/trunk/examples/clang-interpreter/CMakeLists.txt
  cfe/trunk/lib/Analysis/plugins/CheckerDependencyHandling/CMakeLists.txt
  cfe/trunk/lib/Analysis/plugins/CheckerOptionHandling/CMakeLists.txt
  cfe/trunk/lib/Analysis/plugins/SampleAnalyzer/CMakeLists.txt
  cfe/trunk/tools/arcmt-test/CMakeLists.txt
  cfe/trunk/tools/clang-check/CMakeLists.txt
  cfe/trunk/tools/clang-diff/CMakeLists.txt
  cfe/trunk/tools/clang-extdef-mapping/CMakeLists.txt
  cfe/trunk/tools/clang-format/CMakeLists.txt
  cfe/trunk/tools/clang-import-test/CMakeLists.txt
  cfe/trunk/tools/clang-offload-bundler/CMakeLists.txt
  cfe/trunk/tools/clang-refactor/CMakeLists.txt
  cfe/trunk/tools/clang-rename/CMakeLists.txt
  cfe/trunk/tools/clang-scan-deps/CMakeLists.txt
  cfe/trunk/tools/diagtool/CMakeLists.txt
  cfe/trunk/tools/driver/CMakeLists.txt
  cfe/trunk/unittests/AST/CMakeLists.txt
  cfe/trunk/unittests/ASTMatchers/CMakeLists.txt
  cfe/trunk/unittests/ASTMatchers/Dynamic/CMakeLists.txt
  cfe/trunk/unittests/Analysis/CMakeLists.txt
  cfe/trunk/unittests/Basic/CMakeLists.txt
  cfe/trunk/unittests/CodeGen/CMakeLists.txt
  cfe/trunk/unittests/CrossTU/CMakeLists.txt
  cfe/trunk/unittests/Driver/CMakeLists.txt
  cfe/trunk/unittests/Format/CMakeLists.txt
  cfe/trunk/unittests/Frontend/CMakeLists.txt
  cfe/trunk/unittests/Index/CMakeLists.txt
  cfe/trunk/unittests/Lex/CMakeLists.txt
  cfe/trunk/unittests/Rename/CMakeLists.txt
  cfe/trunk/unittests/Rewrite/CMakeLists.txt
  cfe/trunk/unittests/Sema/CMakeLists.txt
  cfe/trunk/unittests/Serialization/CMakeLists.txt
  cfe/trunk/unittests/StaticAnalyzer/CMakeLists.txt
  cfe/trunk/unittests/Tooling/CMakeLists.txt
  cfe/trunk/unittests/Tooling/Syntax/CMakeLists.txt

Index: cfe/trunk/examples/PrintFunctionNames/CMakeLists.txt
===
--- cfe/trunk/examples/PrintFunctionNames/CMakeLists.txt
+++ cfe/trunk/examples/PrintFunctionNames/CMakeLists.txt
@@ -12,10 +12,12 @@
 add_llvm_library(PrintFunctionNames MODULE PrintFunctionNames.cpp PLUGIN_TOOL clang)
 
 if(LLVM_ENABLE_PLUGINS AND (WIN32 OR CYGWIN))
-  target_link_libraries(PrintFunctionNames PRIVATE
+  set(LLVM_LINK_COMPONENTS
+Support
+  )
+  clang_target_link_libraries(PrintFunctionNames PRIVATE
 clangAST
 clangBasic
 clangFrontend
-LLVMSupport
 )
 endif()
Index: cfe/trunk/examples/clang-interpreter/CMakeLists.txt
===
--- cfe/trunk/examples/clang-interpreter/CMakeLists.txt
+++ cfe/trunk/examples/clang-interpreter/CMakeLists.txt
@@ -19,7 +19,7 @@
   clang-resource-headers
   )
 
-target_link_libraries(clang-interpreter
+clang_target_link_libraries(clang-interpreter
   PRIVATE
   clangBasic
   clangCodeGen
Index: cfe/trunk/examples/AnnotateFunctions/CMakeLists.txt
===
--- cfe/trunk/examples/AnnotateFunctions/CMakeLists.txt
+++ cfe/trunk/examples/AnnotateFunctions/CMakeLists.txt
@@ -1,11 +1,13 @@
 add_llvm_library(AnnotateFunctions MODULE AnnotateFunctions.cpp PLUGIN_TOOL clang)
 
 if(LLVM_ENABLE_PLUGINS AND (WIN32 OR CYGWIN))
-  target_link_libraries(AnnotateFunctions PRIVATE
+  set(LLVM_LINK_COMPONENTS
+Support
+  )
+  clang_target_link_libraries(AnnotateFunctions PRIVATE
 clangAST
 clangBasic
 clangFrontend
 clangLex
-LLVMSupport
 )
 endif()
Index: cfe/trunk/CMakeLists.txt
===
--- cfe/trunk/CMakeLists.txt
+++ cfe/trunk/CMakeLists.txt
@@ -323,6 +323,14 @@
 set(CLANG_PYTHON_BINDINGS_VERSIONS "" CACHE STRING
 "Python versions to install libclang python bindings for")
 
+set(CLANG_LINK_CLANG_DYLIB ${LLVM_LINK_LLVM_DYLIB} CACHE BOOL
+"Link tools against libclang_shared.so")
+
+if (NOT LLVM_LINK_LLVM_DYLIB AND CLANG_LINK_CLANG_DYLIB)
+  message(FATAL_ERROR "Cannot set CLANG_LINK_CLANG_DYLIB=ON when "
+  "LLVM_LINK_LLVM_DYLIB=OFF")
+endif()
+
 # The libdir suffix must exactly match whatever LLVM's configuration used.
 set(CLANG_LIBDIR_SUFFIX "${LLVM_LIBDIR_SUFFIX}")
 
Index: cfe/trunk/lib/Analysis/plugins/CheckerOptionHandling/CMakeLists.txt
===
--- cfe/trunk/lib/Analysis/plugins/CheckerOptionHan

r365092 - cmake: Add CLANG_LINK_CLANG_DYLIB option

[Tom Stellard via cfe-commits]

Author: tstellar
Date: Wed Jul  3 15:45:55 2019
New Revision: 365092

URL: http://llvm.org/viewvc/llvm-project?rev=365092&view=rev
Log:
cmake: Add CLANG_LINK_CLANG_DYLIB option

Summary:
Setting CLANG_LINK_CLANG_DYLIB=ON causes clang tools to link against
libclang_shared.so instead of the individual component libraries.

Reviewers: mgorny, beanz, smeenai, phosek, sylvestre.ledru

Subscribers: arphaman, cfe-commits, llvm-commits

Tags: #clang

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

Modified:
cfe/trunk/CMakeLists.txt
cfe/trunk/cmake/modules/AddClang.cmake
cfe/trunk/examples/AnnotateFunctions/CMakeLists.txt
cfe/trunk/examples/PrintFunctionNames/CMakeLists.txt
cfe/trunk/examples/clang-interpreter/CMakeLists.txt
cfe/trunk/lib/Analysis/plugins/CheckerDependencyHandling/CMakeLists.txt
cfe/trunk/lib/Analysis/plugins/CheckerOptionHandling/CMakeLists.txt
cfe/trunk/lib/Analysis/plugins/SampleAnalyzer/CMakeLists.txt
cfe/trunk/tools/arcmt-test/CMakeLists.txt
cfe/trunk/tools/clang-check/CMakeLists.txt
cfe/trunk/tools/clang-diff/CMakeLists.txt
cfe/trunk/tools/clang-extdef-mapping/CMakeLists.txt
cfe/trunk/tools/clang-format/CMakeLists.txt
cfe/trunk/tools/clang-import-test/CMakeLists.txt
cfe/trunk/tools/clang-offload-bundler/CMakeLists.txt
cfe/trunk/tools/clang-refactor/CMakeLists.txt
cfe/trunk/tools/clang-rename/CMakeLists.txt
cfe/trunk/tools/clang-scan-deps/CMakeLists.txt
cfe/trunk/tools/diagtool/CMakeLists.txt
cfe/trunk/tools/driver/CMakeLists.txt
cfe/trunk/unittests/AST/CMakeLists.txt
cfe/trunk/unittests/ASTMatchers/CMakeLists.txt
cfe/trunk/unittests/ASTMatchers/Dynamic/CMakeLists.txt
cfe/trunk/unittests/Analysis/CMakeLists.txt
cfe/trunk/unittests/Basic/CMakeLists.txt
cfe/trunk/unittests/CodeGen/CMakeLists.txt
cfe/trunk/unittests/CrossTU/CMakeLists.txt
cfe/trunk/unittests/Driver/CMakeLists.txt
cfe/trunk/unittests/Format/CMakeLists.txt
cfe/trunk/unittests/Frontend/CMakeLists.txt
cfe/trunk/unittests/Index/CMakeLists.txt
cfe/trunk/unittests/Lex/CMakeLists.txt
cfe/trunk/unittests/Rename/CMakeLists.txt
cfe/trunk/unittests/Rewrite/CMakeLists.txt
cfe/trunk/unittests/Sema/CMakeLists.txt
cfe/trunk/unittests/Serialization/CMakeLists.txt
cfe/trunk/unittests/StaticAnalyzer/CMakeLists.txt
cfe/trunk/unittests/Tooling/CMakeLists.txt
cfe/trunk/unittests/Tooling/Syntax/CMakeLists.txt

Modified: cfe/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/CMakeLists.txt?rev=365092&r1=365091&r2=365092&view=diff
==
--- cfe/trunk/CMakeLists.txt (original)
+++ cfe/trunk/CMakeLists.txt Wed Jul  3 15:45:55 2019
@@ -323,6 +323,14 @@ set(CLANG_VENDOR_UTI "org.llvm.clang" CA
 set(CLANG_PYTHON_BINDINGS_VERSIONS "" CACHE STRING
 "Python versions to install libclang python bindings for")
 
+set(CLANG_LINK_CLANG_DYLIB ${LLVM_LINK_LLVM_DYLIB} CACHE BOOL
+"Link tools against libclang_shared.so")
+
+if (NOT LLVM_LINK_LLVM_DYLIB AND CLANG_LINK_CLANG_DYLIB)
+  message(FATAL_ERROR "Cannot set CLANG_LINK_CLANG_DYLIB=ON when "
+  "LLVM_LINK_LLVM_DYLIB=OFF")
+endif()
+
 # The libdir suffix must exactly match whatever LLVM's configuration used.
 set(CLANG_LIBDIR_SUFFIX "${LLVM_LIBDIR_SUFFIX}")
 

Modified: cfe/trunk/cmake/modules/AddClang.cmake
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/modules/AddClang.cmake?rev=365092&r1=365091&r2=365092&view=diff
==
--- cfe/trunk/cmake/modules/AddClang.cmake (original)
+++ cfe/trunk/cmake/modules/AddClang.cmake Wed Jul  3 15:45:55 2019
@@ -172,3 +172,12 @@ macro(add_clang_symlink name dest)
   # Always generate install targets
   llvm_install_symlink(${name} ${dest} ALWAYS_GENERATE)
 endmacro()
+
+function(clang_target_link_libraries target type)
+  if (CLANG_LINK_CLANG_DYLIB)
+target_link_libraries(${target} ${type} clang_shared)
+  else()
+target_link_libraries(${target} ${type} ${ARGN})
+  endif()
+
+endfunction()

Modified: cfe/trunk/examples/AnnotateFunctions/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/examples/AnnotateFunctions/CMakeLists.txt?rev=365092&r1=365091&r2=365092&view=diff
==
--- cfe/trunk/examples/AnnotateFunctions/CMakeLists.txt (original)
+++ cfe/trunk/examples/AnnotateFunctions/CMakeLists.txt Wed Jul  3 15:45:55 2019
@@ -1,11 +1,13 @@
 add_llvm_library(AnnotateFunctions MODULE AnnotateFunctions.cpp PLUGIN_TOOL 
clang)
 
 if(LLVM_ENABLE_PLUGINS AND (WIN32 OR CYGWIN))
-  target_link_libraries(AnnotateFunctions PRIVATE
+  set(LLVM_LINK_COMPONENTS
+Support
+  )
+  clang_target_link_libraries(AnnotateFunctions PRIVATE
 clangAST
 clangBasic
 clangFrontend
 clangLex
-LLVMSupport
 )
 endif()

Modified:

[PATCH] D63538: [CFG] Add a new function to get the proper condition of a CFGBlock

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

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

Add one more assert to `GetExprText`.


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

https://reviews.llvm.org/D63538

Files:
  clang/include/clang/Analysis/CFG.h
  clang/lib/Analysis/CFG.cpp
  clang/unittests/Analysis/CFGTest.cpp

Index: clang/unittests/Analysis/CFGTest.cpp
===
--- clang/unittests/Analysis/CFGTest.cpp
+++ clang/unittests/Analysis/CFGTest.cpp
@@ -67,6 +67,58 @@
   expectLinear(true,  "void foo() { foo(); }"); // Recursion is not our problem.
 }
 
+TEST(CFG, ConditionExpr) {
+  const char *Code = R"(void f(bool A, bool B, bool C) {
+  if (A && B && C)
+int x;
+})";
+  BuildResult Result = BuildCFG(Code);
+  EXPECT_EQ(BuildResult::BuiltCFG, Result.getStatus());
+
+  // [B5 (ENTRY)] -> [B4] -> [B3] -> [B2] -> [B1] -> [B0 (EXIT)]
+  //   \  \   \ /
+  //--->
+
+  CFG *cfg = Result.getCFG();
+
+  auto GetBlock = [cfg] (unsigned Index) -> CFGBlock * {
+assert(Index < cfg->size());
+return *(cfg->begin() + Index);
+  };
+
+  auto GetExprText = [] (const Expr *E) -> std::string {
+// It's very awkward trying to recover the actual expression text without
+// a real source file, so use this as a workaround. We know that the
+// condition expression looks like this:
+//
+// ImplicitCastExpr 0xd07bf8 '_Bool' 
+//  `-DeclRefExpr 0xd07bd8 '_Bool' lvalue ParmVar 0xd07960 'C' '_Bool'
+
+assert(isa(E));
+assert(++E->child_begin() == E->child_end());
+const auto *D = dyn_cast(*E->child_begin());
+return D->getFoundDecl()->getNameAsString();
+  };
+
+  EXPECT_EQ(GetBlock(1)->getLastCondition(), nullptr);
+  EXPECT_EQ(GetExprText(GetBlock(4)->getLastCondition()), "A");
+  EXPECT_EQ(GetExprText(GetBlock(3)->getLastCondition()), "B");
+  EXPECT_EQ(GetExprText(GetBlock(2)->getLastCondition()), "C");
+
+  //======//
+
+  Code = R"(void foo(int x, int y) {
+  (void)(x + y);
+})";
+  Result = BuildCFG(Code);
+  EXPECT_EQ(BuildResult::BuiltCFG, Result.getStatus());
+
+  // [B2 (ENTRY)] -> [B1] -> [B0 (EXIT)]
+
+  cfg = Result.getCFG();
+  EXPECT_EQ(GetBlock(1)->getLastCondition(), nullptr);
+}
+
 } // namespace
 } // namespace analysis
 } // namespace clang
Index: clang/lib/Analysis/CFG.cpp
===
--- clang/lib/Analysis/CFG.cpp
+++ clang/lib/Analysis/CFG.cpp
@@ -5615,6 +5615,30 @@
   Out << JsonFormat(TempOut.str(), AddQuotes);
 }
 
+const Expr *CFGBlock::getLastCondition() const {
+  // If the terminator is a temporary dtor or a virtual base, etc, we can't
+  // retrieve a meaningful condition, bail out.
+  if (Terminator.getKind() != CFGTerminator::StmtBranch)
+return nullptr;
+
+  // Also, if this method was called on a block that doesn't have 2 successors,
+  // this block doesn't have retrievable condition.
+  if (succ_size() < 2)
+return nullptr;
+
+  auto StmtElem = rbegin()->getAs();
+  if (!StmtElem)
+return nullptr;
+
+  const Stmt *Cond = StmtElem->getStmt();
+  if (isa(Cond))
+return nullptr;
+
+  // Only ObjCForCollectionStmt is known not to be a non-Expr terminator, hence
+  // the cast<>.
+  return cast(Cond)->IgnoreParens();
+}
+
 Stmt *CFGBlock::getTerminatorCondition(bool StripParens) {
   Stmt *Terminator = getTerminatorStmt();
   if (!Terminator)
Index: clang/include/clang/Analysis/CFG.h
===
--- clang/include/clang/Analysis/CFG.h
+++ clang/include/clang/Analysis/CFG.h
@@ -860,6 +860,14 @@
   Stmt *getTerminatorStmt() { return Terminator.getStmt(); }
   const Stmt *getTerminatorStmt() const { return Terminator.getStmt(); }
 
+  /// \returns the last (\c rbegin()) condition, e.g. observe the following code
+  /// snippet:
+  ///   if (A && B && C)
+  /// A block would be created for \c A, \c B, and \c C. For the latter,
+  /// \c getTerminatorStmt() would retrieve the entire condition, rather than
+  /// C itself, while this method would only return C.
+  const Expr *getLastCondition() const;
+
   Stmt *getTerminatorCondition(bool StripParens = true);
 
   const Stmt *getTerminatorCondition(bool StripParens = true) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r365091 - [Bitcode] Move Bitstream to a separate library

[Francis Visoiu Mistrih via cfe-commits]

Author: thegameg
Date: Wed Jul  3 15:40:07 2019
New Revision: 365091

URL: http://llvm.org/viewvc/llvm-project?rev=365091&view=rev
Log:
[Bitcode] Move Bitstream to a separate library

This moves Bitcode/Bitstream*, Bitcode/BitCodes.h to Bitstream/.

This is needed to avoid a circular dependency when using the bitstream
code for parsing optimization remarks.

Since Bitcode uses Core for the IR part:

libLLVMRemarks -> Bitcode -> Core

and Core uses libLLVMRemarks to generate remarks (see
IR/RemarkStreamer.cpp):

Core -> libLLVMRemarks

we need to separate the Bitstream and Bitcode part.

For clang-doc, it seems that it doesn't need the whole bitcode layer, so
I updated the CMake to only use the bitstream part.

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

Modified:
cfe/trunk/include/clang/Frontend/SerializedDiagnosticPrinter.h
cfe/trunk/include/clang/Frontend/SerializedDiagnosticReader.h
cfe/trunk/include/clang/Frontend/SerializedDiagnostics.h
cfe/trunk/include/clang/Serialization/ASTBitCodes.h
cfe/trunk/include/clang/Serialization/ASTReader.h
cfe/trunk/include/clang/Serialization/ASTWriter.h
cfe/trunk/include/clang/Serialization/Module.h
cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
cfe/trunk/lib/Frontend/ASTUnit.cpp
cfe/trunk/lib/Frontend/CMakeLists.txt
cfe/trunk/lib/Frontend/SerializedDiagnosticReader.cpp
cfe/trunk/lib/Frontend/TestModuleFileExtension.cpp
cfe/trunk/lib/Frontend/TestModuleFileExtension.h
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp
cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
cfe/trunk/lib/Serialization/ASTWriterStmt.cpp
cfe/trunk/lib/Serialization/CMakeLists.txt
cfe/trunk/lib/Serialization/GeneratePCH.cpp
cfe/trunk/lib/Serialization/GlobalModuleIndex.cpp
cfe/trunk/lib/Serialization/PCHContainerOperations.cpp
cfe/trunk/tools/libclang/CXLoadedDiagnostic.cpp
cfe/trunk/unittests/Serialization/CMakeLists.txt

Modified: cfe/trunk/include/clang/Frontend/SerializedDiagnosticPrinter.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/SerializedDiagnosticPrinter.h?rev=365091&r1=365090&r2=365091&view=diff
==
--- cfe/trunk/include/clang/Frontend/SerializedDiagnosticPrinter.h (original)
+++ cfe/trunk/include/clang/Frontend/SerializedDiagnosticPrinter.h Wed Jul  3 
15:40:07 2019
@@ -11,7 +11,7 @@
 
 #include "clang/Basic/LLVM.h"
 #include "clang/Frontend/SerializedDiagnostics.h"
-#include "llvm/Bitcode/BitstreamWriter.h"
+#include "llvm/Bitstream/BitstreamWriter.h"
 
 namespace llvm {
 class raw_ostream;

Modified: cfe/trunk/include/clang/Frontend/SerializedDiagnosticReader.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/SerializedDiagnosticReader.h?rev=365091&r1=365090&r2=365091&view=diff
==
--- cfe/trunk/include/clang/Frontend/SerializedDiagnosticReader.h (original)
+++ cfe/trunk/include/clang/Frontend/SerializedDiagnosticReader.h Wed Jul  3 
15:40:07 2019
@@ -10,7 +10,7 @@
 #define LLVM_CLANG_FRONTEND_SERIALIZEDDIAGNOSTICREADER_H
 
 #include "clang/Basic/LLVM.h"
-#include "llvm/Bitcode/BitstreamReader.h"
+#include "llvm/Bitstream/BitstreamReader.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ErrorOr.h"
 #include 

Modified: cfe/trunk/include/clang/Frontend/SerializedDiagnostics.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/SerializedDiagnostics.h?rev=365091&r1=365090&r2=365091&view=diff
==
--- cfe/trunk/include/clang/Frontend/SerializedDiagnostics.h (original)
+++ cfe/trunk/include/clang/Frontend/SerializedDiagnostics.h Wed Jul  3 
15:40:07 2019
@@ -9,7 +9,7 @@
 #ifndef LLVM_CLANG_FRONTEND_SERIALIZE_DIAGNOSTICS_H_
 #define LLVM_CLANG_FRONTEND_SERIALIZE_DIAGNOSTICS_H_
 
-#include "llvm/Bitcode/BitCodes.h"
+#include "llvm/Bitstream/BitCodes.h"
 
 namespace clang {
 namespace serialized_diags {

Modified: cfe/trunk/include/clang/Serialization/ASTBitCodes.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTBitCodes.h?rev=365091&r1=365090&r2=365091&view=diff
==
--- cfe/trunk/include/clang/Serialization/ASTBitCodes.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTBitCodes.h Wed Jul  3 15:40:07 2019
@@ -23,7 +23,7 @@
 #include "clang/Basic/OperatorKinds.h"
 #include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/DenseMapInfo.h"
-#include "llvm/Bitcode/BitCodes.h"
+#include "llvm/Bitstream/BitCodes.h"
 #include 
 #include 
 

Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
URL: 
http://llvm.org/viewvc/ll

[clang-tools-extra] r365091 - [Bitcode] Move Bitstream to a separate library

[Francis Visoiu Mistrih via cfe-commits]

Author: thegameg
Date: Wed Jul  3 15:40:07 2019
New Revision: 365091

URL: http://llvm.org/viewvc/llvm-project?rev=365091&view=rev
Log:
[Bitcode] Move Bitstream to a separate library

This moves Bitcode/Bitstream*, Bitcode/BitCodes.h to Bitstream/.

This is needed to avoid a circular dependency when using the bitstream
code for parsing optimization remarks.

Since Bitcode uses Core for the IR part:

libLLVMRemarks -> Bitcode -> Core

and Core uses libLLVMRemarks to generate remarks (see
IR/RemarkStreamer.cpp):

Core -> libLLVMRemarks

we need to separate the Bitstream and Bitcode part.

For clang-doc, it seems that it doesn't need the whole bitcode layer, so
I updated the CMake to only use the bitstream part.

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

Modified:
clang-tools-extra/trunk/clang-doc/BitcodeReader.h
clang-tools-extra/trunk/clang-doc/BitcodeWriter.h
clang-tools-extra/trunk/clang-doc/CMakeLists.txt
clang-tools-extra/trunk/unittests/clang-doc/BitcodeTest.cpp
clang-tools-extra/trunk/unittests/clang-doc/CMakeLists.txt

Modified: clang-tools-extra/trunk/clang-doc/BitcodeReader.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/BitcodeReader.h?rev=365091&r1=365090&r2=365091&view=diff
==
--- clang-tools-extra/trunk/clang-doc/BitcodeReader.h (original)
+++ clang-tools-extra/trunk/clang-doc/BitcodeReader.h Wed Jul  3 15:40:07 2019
@@ -20,7 +20,7 @@
 #include "clang/AST/AST.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
-#include "llvm/Bitcode/BitstreamReader.h"
+#include "llvm/Bitstream/BitstreamReader.h"
 #include "llvm/Support/Error.h"
 
 namespace clang {

Modified: clang-tools-extra/trunk/clang-doc/BitcodeWriter.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/BitcodeWriter.h?rev=365091&r1=365090&r2=365091&view=diff
==
--- clang-tools-extra/trunk/clang-doc/BitcodeWriter.h (original)
+++ clang-tools-extra/trunk/clang-doc/BitcodeWriter.h Wed Jul  3 15:40:07 2019
@@ -20,7 +20,7 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
-#include "llvm/Bitcode/BitstreamWriter.h"
+#include "llvm/Bitstream/BitstreamWriter.h"
 #include 
 #include 
 

Modified: clang-tools-extra/trunk/clang-doc/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/CMakeLists.txt?rev=365091&r1=365090&r2=365091&view=diff
==
--- clang-tools-extra/trunk/clang-doc/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-doc/CMakeLists.txt Wed Jul  3 15:40:07 2019
@@ -1,7 +1,6 @@
 set(LLVM_LINK_COMPONENTS
   support
-  BitReader
-  BitWriter
+  BitstreamReader
   )
 
 add_clang_library(clangDoc

Modified: clang-tools-extra/trunk/unittests/clang-doc/BitcodeTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-doc/BitcodeTest.cpp?rev=365091&r1=365090&r2=365091&view=diff
==
--- clang-tools-extra/trunk/unittests/clang-doc/BitcodeTest.cpp (original)
+++ clang-tools-extra/trunk/unittests/clang-doc/BitcodeTest.cpp Wed Jul  3 
15:40:07 2019
@@ -10,8 +10,8 @@
 #include "BitcodeWriter.h"
 #include "ClangDocTest.h"
 #include "Representation.h"
-#include "llvm/Bitcode/BitstreamReader.h"
-#include "llvm/Bitcode/BitstreamWriter.h"
+#include "llvm/Bitstream/BitstreamReader.h"
+#include "llvm/Bitstream/BitstreamWriter.h"
 #include "gtest/gtest.h"
 
 namespace clang {

Modified: clang-tools-extra/trunk/unittests/clang-doc/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-doc/CMakeLists.txt?rev=365091&r1=365090&r2=365091&view=diff
==
--- clang-tools-extra/trunk/unittests/clang-doc/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/unittests/clang-doc/CMakeLists.txt Wed Jul  3 
15:40:07 2019
@@ -1,7 +1,6 @@
 set(LLVM_LINK_COMPONENTS
   support
-  BitReader
-  BitWriter
+  BitstreamReader
   )
 
 get_filename_component(CLANG_DOC_SOURCE_DIR


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

[PATCH] D64128: [CodeGen] Generate llvm.ptrmask instead of inttoptr(and(ptrtoint, C)) if possible.

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

I'm not sure I understand all the implications, and why that would / wouldn't 
be valid.

Should this be an builtin that can be called from C++ directly?




Comment at: clang/lib/CodeGen/CGExprScalar.cpp:2034
+  (AllOnes << 
llvm::Log2_64(DL.getPointerABIAlignment(static_cast(
+   CE->getSubExpr()->getType()->getPointeeType().getAddressSpace();
+  if (!BO->getRHS()->isIntegerConstantExpr(CV, Ctx) ||

This assumes pointers are max 64 bits :)
It seems better to shift `CV` up by `bit_per_byte*sizeof(void*) - 
max_pointer_size`, and then down to remove alignment, and finally checking that 
it's zero?

Which leads me to wonder: should we ever diagnose "bad" pointer masks?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64128



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

[PATCH] D63915: [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of function calls

[Artem Dergachev via Phabricator via cfe-commits]

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

I think i like it now!




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:1119
 
-  const T *lookup(const CallEvent &Call) const {
+  Optional lookup(const CallEvent &Call) const {
 // Slow path: linear lookup.

Charusso wrote:
> NoQ wrote:
> > Charusso wrote:
> > > Szelethus wrote:
> > > > Charusso wrote:
> > > > > NoQ wrote:
> > > > > > I hope `T` never gets too expensive to copy. The ideal return value 
> > > > > > here is `Optional` but i suspect that `llvm::Optional`s 
> > > > > > don't support this (while C++17 `std::optional`s do). Could you 
> > > > > > double-check my vague memories here?
> > > > > Optional is working and used widely, I like that.
> > > > Why do we need the optional AND the pointer? How about we just return 
> > > > with a nullptr if we fail to find the call?
> > > `Optional<>` stands for optional values, that is why it is made. @NoQ 
> > > tried to avoid it, but I believe if someone does not use it for an 
> > > optional value, that break some kind of unspoken standard.
> > Well, that'd be the original code.
> > 
> > > I do not like `Optional` anymore.
> > 
> > @Charusso, do you still plan to undo this change?
> Well, I am here at 2:1 against `Optional<>`, so I think it is your decision.
I'd rather leave the original code as is and think more deeply about adding 
support for `Optional` in the future.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:229
+  /// @param Cb Callback with 'BugReporterContext &, BugReport &' parameters.
+  /// @param IsPrunable Whether the note is prunable.
+  const NoteTag *getNoteTag(NoteTag::Callback &&Cb, bool IsPrunable = false) {

"Allow BugReporter to omit the note from the report if it would make the 
displayed bug path significantly shorter."



Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:88-93
+  Optional RawExpectedValue = CDM.lookup(Call);
+  if (!RawExpectedValue)
+return;
+
+  SVal ReturnV = Call.getReturnValue();
+  bool ExpectedValue = **RawExpectedValue;

Charusso wrote:
> NoQ wrote:
> > This can still be re-used by moving into `isInvariantBreak` (you can give 
> > it access to `CDM` by making it non-static).
> Well, sadly not. In both of the checks it is used inside the call, so you 
> cannot just remove it. I really wanted to make it as simple as possible, but 
> you know, "not simpler".
Aha, ok, right!



Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:95-96
+  Optional IsInvariantBreak = isInvariantBreak(ExpectedValue, ReturnV, 
C);
+  if (!IsInvariantBreak)
+return;
+

Charusso wrote:
> NoQ wrote:
> > This looks flipped to me, should probably say `if (IsInvariantBreak) 
> > return;`.
> It is the `Optional<>` checking, whether we could obtain the value. I really 
> wanted to write `!hasValue()`, but no one use that, so it is some kind of 
> unspoken standard to just `!` it.
Mmm. Aha. Ok. Indeed. Sry^^

I was thinking about simply err-ing towards "it's not a break" when we don't 
know for sure that it's a break, because in this case we have no problems with 
taking the branch that we want.

But your code seems to be more careful and i like it :)



Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:69
+  Name += Call.getCalleeIdentifier()->getName();
+  return Name;
+}

Charusso wrote:
> xazax.hun wrote:
> > `CXXMethodDecl::getQualifiedNameAsString` is not doing what you want here?
> We want to drop the namespaces for better readability.
Yeah, i think it's important to display exactly what we match for.

We might eventually do some sort of `CallDescription.explain()` (and then maybe 
even `CallDescriptionMap.explain(Call)`) for that purpose.



Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:154
+
+BR.markInteresting(SFC);
+

Hmm. It is enough to set `IsPrunable` to `false`; once you do that, there's 
actually no need to mark the stack frame as interesting. I guess this wasn't 
really necessary.


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

https://reviews.llvm.org/D63915



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

[PATCH] D63538: [CFG] Add a new function to get the proper condition of a CFGBlock

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

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

- Bail out if the actual terminator isn't a branch
- Bail out if the number of successors is less than 2
- LLVM-ify the code as suggested!
- Add some unit tests (I mean, you can kinda see how it was duct taped 
together, but it's maybe a hair better than nothing?)


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

https://reviews.llvm.org/D63538

Files:
  clang/include/clang/Analysis/CFG.h
  clang/lib/Analysis/CFG.cpp
  clang/unittests/Analysis/CFGTest.cpp

Index: clang/unittests/Analysis/CFGTest.cpp
===
--- clang/unittests/Analysis/CFGTest.cpp
+++ clang/unittests/Analysis/CFGTest.cpp
@@ -67,6 +67,57 @@
   expectLinear(true,  "void foo() { foo(); }"); // Recursion is not our problem.
 }
 
+TEST(CFG, ConditionExpr) {
+  const char *Code = R"(void f(bool A, bool B, bool C) {
+  if (A && B && C)
+int x;
+})";
+  BuildResult Result = BuildCFG(Code);
+  EXPECT_EQ(BuildResult::BuiltCFG, Result.getStatus());
+
+  // [B5 (ENTRY)] -> [B4] -> [B3] -> [B2] -> [B1] -> [B0 (EXIT)]
+  //   \  \   \ /
+  //--->
+
+  CFG *cfg = Result.getCFG();
+
+  auto GetBlock = [cfg] (unsigned Index) -> CFGBlock * {
+assert(Index < cfg->size());
+return *(cfg->begin() + Index);
+  };
+
+  auto GetExprText = [] (const Expr *E) -> std::string {
+// It's very awkward trying to recover the actual expression text without
+// a real source file, so use this as a workaround. We know that the
+// condition expression looks like this:
+//
+// ImplicitCastExpr 0xd07bf8 '_Bool' 
+//  `-DeclRefExpr 0xd07bd8 '_Bool' lvalue ParmVar 0xd07960 'C' '_Bool'
+
+assert(isa(E));
+const auto *D = dyn_cast(*E->child_begin());
+return D->getFoundDecl()->getNameAsString();
+  };
+
+  EXPECT_EQ(GetBlock(1)->getLastCondition(), nullptr);
+  EXPECT_EQ(GetExprText(GetBlock(4)->getLastCondition()), "A");
+  EXPECT_EQ(GetExprText(GetBlock(3)->getLastCondition()), "B");
+  EXPECT_EQ(GetExprText(GetBlock(2)->getLastCondition()), "C");
+
+  //======//
+
+  Code = R"(void foo(int x, int y) {
+  (void)(x + y);
+})";
+  Result = BuildCFG(Code);
+  EXPECT_EQ(BuildResult::BuiltCFG, Result.getStatus());
+
+  // [B2 (ENTRY)] -> [B1] -> [B0 (EXIT)]
+
+  cfg = Result.getCFG();
+  EXPECT_EQ(GetBlock(1)->getLastCondition(), nullptr);
+}
+
 } // namespace
 } // namespace analysis
 } // namespace clang
Index: clang/lib/Analysis/CFG.cpp
===
--- clang/lib/Analysis/CFG.cpp
+++ clang/lib/Analysis/CFG.cpp
@@ -5615,6 +5615,30 @@
   Out << JsonFormat(TempOut.str(), AddQuotes);
 }
 
+const Expr *CFGBlock::getLastCondition() const {
+  // If the terminator is a temporary dtor or a virtual base, etc, we can't
+  // retrieve a meaningful condition, bail out.
+  if (Terminator.getKind() != CFGTerminator::StmtBranch)
+return nullptr;
+
+  // Also, if this method was called on a block that doesn't have 2 successors,
+  // this block doesn't have retrievable condition.
+  if (succ_size() < 2)
+return nullptr;
+
+  auto StmtElem = rbegin()->getAs();
+  if (!StmtElem)
+return nullptr;
+
+  const Stmt *Cond = StmtElem->getStmt();
+  if (isa(Cond))
+return nullptr;
+
+  // Only ObjCForCollectionStmt is known not to be a non-Expr terminator, hence
+  // the cast<>.
+  return cast(Cond)->IgnoreParens();
+}
+
 Stmt *CFGBlock::getTerminatorCondition(bool StripParens) {
   Stmt *Terminator = getTerminatorStmt();
   if (!Terminator)
Index: clang/include/clang/Analysis/CFG.h
===
--- clang/include/clang/Analysis/CFG.h
+++ clang/include/clang/Analysis/CFG.h
@@ -860,6 +860,14 @@
   Stmt *getTerminatorStmt() { return Terminator.getStmt(); }
   const Stmt *getTerminatorStmt() const { return Terminator.getStmt(); }
 
+  /// \returns the last (\c rbegin()) condition, e.g. observe the following code
+  /// snippet:
+  ///   if (A && B && C)
+  /// A block would be created for \c A, \c B, and \c C. For the latter,
+  /// \c getTerminatorStmt() would retrieve the entire condition, rather than
+  /// C itself, while this method would only return C.
+  const Expr *getLastCondition() const;
+
   Stmt *getTerminatorCondition(bool StripParens = true);
 
   const Stmt *getTerminatorCondition(bool StripParens = true) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64149: [clang-scan-deps] use `-Wno-error` when scanning for dependencies

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

dexonsmith added a comment.

Is warning output suppressed?  If so, should we just/also disable all warnings? 
 (IIRC, the flag is `-w`.)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64149



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

[PATCH] D59980: [Attributor] Deduce memory behavior argument attributes

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a subscriber: fhahn.
jdoerfert added a comment.

@hfinkel @fhahn ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59980



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

[PATCH] D64151: Enhance abseil-faster-strsplit-delimiter to handle other non-printable characters.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp:42-43
+ const ASTContext &Context) {
+  assert(Literal->getLength() == 1);
+  assert(Literal->getCharByteWidth() == 1); // no wide char
+  std::string Result = clang::tooling::fixit::getText(*Literal, Context).str();

Assertion messages missing



Comment at: 
clang-tools-extra/test/clang-tidy/abseil-faster-strsplit-delimiter.cpp:55
+
+  absl::StrSplit("ABC", R"(A)");
+  // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit() called with a 
string literal consisting of a single character; consider using the character 
overload [abseil-faster-strsplit-delimiter]

Is there a negative test, `absl::StrSplit("ABC", R"(AA)");`?
Also what about wide chars? (the second assertion)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64151



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

[PATCH] D64151: Enhance abseil-faster-strsplit-delimiter to handle other non-printable characters.

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp:10
 #include "FasterStrsplitDelimiterCheck.h"
+
 #include "clang/AST/ASTContext.h"

Unnecessary empty line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64151



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

[PATCH] D64151: Enhance abseil-faster-strsplit-delimiter to handle other non-printable characters.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added a comment.

Sorry, the patch does not apply cleanly to current master -- could you rebase 
please?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64151



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

[PATCH] D63889: Check possible warnings on global initializers for reachability

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/include/clang/Sema/AnalysisBasedWarnings.h:101
+  void RegisterVarDeclWarning(VarDecl *VD, PossiblyUnreachableDiag
+   PossiblyUnreachableDiag);
+

`git-clang-format HEAD~`

The formal parameter should be abbreviated, maybe `PUD`?



Comment at: clang/include/clang/Sema/AnalysisBasedWarnings.h:110
 
+void emitPossiblyUnreachableDiags(Sema &S, AnalysisDeclContext &AC,
+SmallVector PossiblyUnreachableDiags);

How about making this a proper method of `AnalysisBasedWarnings` rather than a 
free floating function that's only ever called from other methods of 
`AnalysisBasedWarnings`?  That way you don't have to pass in a `Sema`.



Comment at: clang/lib/Analysis/AnalysisDeclContext.cpp:124
+if(VD->hasGlobalStorage()) {
+  return const_cast(dyn_cast(VD->getInit()));
+}

The `const_cast` doesn't look necessary here.  Is it?



Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:2012
 
+void clang::sema::emitPossiblyUnreachableDiags(Sema &S, AnalysisDeclContext 
&AC,
+SmallVector PossiblyUnreachableDiags) 
{

having the full namespace spelled out here in the definition smells funny.  Is 
there a pair of `namespace` blocks below for `clang` and `sema` where the 
definition of `emitPossiblyUnreachableDiags` could be moved into?

Actually looking at the file, I see you simply matched the existing style, but 
line 49 currently has a `using` statement that should inject the `clang` 
namespace into the current scope.  That's why you see `sema::` used in other 
places in this translation unit without the `clang` namespace prefix.  The 
whole file should remove the `clang::` prefixes or additionally replace the 
`using` statement with explicit `namespace` blocks.



Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:2260
+void clang::sema::AnalysisBasedWarnings::RegisterVarDeclWarning(VarDecl *VD,
+clang::sema::PossiblyUnreachableDiag PossiblyUnreachableDiag) {
+  VarDeclPossiblyUnreachableDiags[VD].emplace_back(PossiblyUnreachableDiag);

`PUD`



Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:2275
+
+  clang::sema::emitPossiblyUnreachableDiags(S, AC, 
VarDeclPossiblyUnreachableDiags[VD]);
+}

If you make `emitPossiblyUnreachableDiags` then it doesn't need all the 
prefixes.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:347
+  // context before the function scope or diagnostics are
+  // delayed until function scope is added
+  DeclContext *Cur = CurContext;

Use punctuation in your comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:16678
+// Mangling context seems to only be defined on constexpr vardecl that
+// displayed errors
+// This skips warnings that were already emitted as notes on errors.

Punctuation.



Comment at: clang/test/Sema/warn-unreachable-warning-var-decl.cpp:39-40
+
+constexpr signed char c1 = 100 * 2; // ok expected-warning{{changes value}}
+constexpr signed char c2 = '\x64' * '\2'; // also ok  
expected-warning{{changes value}}
+constexpr int shr_32 = 0 >> 32; // expected-error {{constant expression}} 
expected-note {{shift count 32 >= width of type}}

`ok` and `also ok` can probably be removed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63889



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

[PATCH] D40259: [libcxx] LWG2993: reference_wrapper

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

Did this ever get landed? If not, was there a reason?
Note that the synopsis at the top of `` also needs to be updated.


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

https://reviews.llvm.org/D40259



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

r365085 - [analyzer] exploded-graph-rewriter: Implement a black-and-white color scheme.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Wed Jul  3 13:48:23 2019
New Revision: 365085

URL: http://llvm.org/viewvc/llvm-project?rev=365085&view=rev
Log:
[analyzer] exploded-graph-rewriter: Implement a black-and-white color scheme.

For accessibility!

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

Modified:
cfe/trunk/test/Analysis/exploded-graph-rewriter/node_labels.dot
cfe/trunk/utils/analyzer/exploded-graph-rewriter.py

Modified: cfe/trunk/test/Analysis/exploded-graph-rewriter/node_labels.dot
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/exploded-graph-rewriter/node_labels.dot?rev=365085&r1=365084&r2=365085&view=diff
==
--- cfe/trunk/test/Analysis/exploded-graph-rewriter/node_labels.dot (original)
+++ cfe/trunk/test/Analysis/exploded-graph-rewriter/node_labels.dot Wed Jul  3 
13:48:23 2019
@@ -1,6 +1,11 @@
-// RUN: %exploded_graph_rewriter %s | FileCheck %s -check-prefixes=CHECK,LIGHT
-// RUN: %exploded_graph_rewriter %s --dark | FileCheck %s \
-// RUN: -check-prefixes CHECK,DARK
+// RUN: %exploded_graph_rewriter %s \
+// RUN: | FileCheck %s -check-prefixes=CHECK,LIGHT,COLOR
+// RUN: %exploded_graph_rewriter %s --dark \
+// RUN: | FileCheck %s -check-prefixes CHECK,DARK,COLOR
+// RUN: %exploded_graph_rewriter %s --gray \
+// RUN: | FileCheck %s -check-prefixes=CHECK,LIGHT,GRAY
+// RUN: %exploded_graph_rewriter %s --gray --dark \
+// RUN: | FileCheck %s -check-prefixes CHECK,DARK,GRAY
 
 // FIXME: Substitution doesn't seem to work on Windows.
 // UNSUPPORTED: system-windows
@@ -23,10 +28,12 @@ Node0x1 [shape=record,label=
 
 // CHECK: Node0x2 [
 // CHECK-SAME: 
-// CHECK-SAME:   Bug Report Attached
+// COLOR-SAME:   Bug Report Attached
+// GRAY-SAME:Bug Report Attached
 // CHECK-SAME: 
 // CHECK-SAME: 
-// CHECK-SAME:   Sink Node
+// COLOR-SAME:   Sink Node
+// GRAY-SAME:Sink Node
 // CHECK-SAME: 
 Node0x2 [shape=record,label=
  "{

Modified: cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/analyzer/exploded-graph-rewriter.py?rev=365085&r1=365084&r2=365085&view=diff
==
--- cfe/trunk/utils/analyzer/exploded-graph-rewriter.py (original)
+++ cfe/trunk/utils/analyzer/exploded-graph-rewriter.py Wed Jul  3 13:48:23 2019
@@ -384,24 +384,28 @@ class ExplodedGraph(object):
 # A visitor that dumps the ExplodedGraph into a DOT file with fancy HTML-based
 # syntax highlighing.
 class DotDumpVisitor(object):
-def __init__(self, do_diffs, dark_mode):
+def __init__(self, do_diffs, dark_mode, gray_mode):
 super(DotDumpVisitor, self).__init__()
 self._do_diffs = do_diffs
 self._dark_mode = dark_mode
+self._gray_mode = gray_mode
 
 @staticmethod
 def _dump_raw(s):
 print(s, end='')
 
-@staticmethod
-def _dump(s):
-print(s.replace('&', '&')
-   .replace('{', '\\{')
-   .replace('}', '\\}')
-   .replace('\\<', '<')
-   .replace('\\>', '>')
-   .replace('\\l', '')
-   .replace('|', '\\|'), end='')
+def _dump(self, s):
+s = s.replace('&', '&') \
+ .replace('{', '\\{') \
+ .replace('}', '\\}') \
+ .replace('\\<', '<') \
+ .replace('\\>', '>') \
+ .replace('\\l', '') \
+ .replace('|', '\\|')
+if self._gray_mode:
+s = re.sub(r'', '', s)
+s = re.sub(r'', '', s)
+self._dump_raw(s)
 
 @staticmethod
 def _diff_plus_minus(is_added):
@@ -835,6 +839,9 @@ def main():
 parser.add_argument('--dark', action='store_const', dest='dark',
 const=True, default=False,
 help='dark mode')
+parser.add_argument('--gray', action='store_const', dest='gray',
+const=True, default=False,
+help='black-and-white mode')
 args = parser.parse_args()
 logging.basicConfig(level=args.loglevel)
 
@@ -845,7 +852,7 @@ def main():
 graph.add_raw_line(raw_line)
 
 explorer = BasicExplorer()
-visitor = DotDumpVisitor(args.diff, args.dark)
+visitor = DotDumpVisitor(args.diff, args.dark, args.gray)
 explorer.explore(graph, visitor)
 
 


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

[PATCH] D37182: [libcxx] Special visibility macros for the experimental library

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

This is an old patch; is this still needed/desired?


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

https://reviews.llvm.org/D37182



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

[PATCH] D64153: [analyzer] exploded-graph-rewriter: Add a grayscale mode.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL365085: [analyzer] exploded-graph-rewriter: Implement a 
black-and-white color scheme. (authored by dergachev, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64153?vs=207867&id=207884#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64153

Files:
  cfe/trunk/test/Analysis/exploded-graph-rewriter/node_labels.dot
  cfe/trunk/utils/analyzer/exploded-graph-rewriter.py


Index: cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
===
--- cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
+++ cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
@@ -384,24 +384,28 @@
 # A visitor that dumps the ExplodedGraph into a DOT file with fancy HTML-based
 # syntax highlighing.
 class DotDumpVisitor(object):
-def __init__(self, do_diffs, dark_mode):
+def __init__(self, do_diffs, dark_mode, gray_mode):
 super(DotDumpVisitor, self).__init__()
 self._do_diffs = do_diffs
 self._dark_mode = dark_mode
+self._gray_mode = gray_mode
 
 @staticmethod
 def _dump_raw(s):
 print(s, end='')
 
-@staticmethod
-def _dump(s):
-print(s.replace('&', '&')
-   .replace('{', '\\{')
-   .replace('}', '\\}')
-   .replace('\\<', '<')
-   .replace('\\>', '>')
-   .replace('\\l', '')
-   .replace('|', '\\|'), end='')
+def _dump(self, s):
+s = s.replace('&', '&') \
+ .replace('{', '\\{') \
+ .replace('}', '\\}') \
+ .replace('\\<', '<') \
+ .replace('\\>', '>') \
+ .replace('\\l', '') \
+ .replace('|', '\\|')
+if self._gray_mode:
+s = re.sub(r'', '', s)
+s = re.sub(r'', '', s)
+self._dump_raw(s)
 
 @staticmethod
 def _diff_plus_minus(is_added):
@@ -835,6 +839,9 @@
 parser.add_argument('--dark', action='store_const', dest='dark',
 const=True, default=False,
 help='dark mode')
+parser.add_argument('--gray', action='store_const', dest='gray',
+const=True, default=False,
+help='black-and-white mode')
 args = parser.parse_args()
 logging.basicConfig(level=args.loglevel)
 
@@ -845,7 +852,7 @@
 graph.add_raw_line(raw_line)
 
 explorer = BasicExplorer()
-visitor = DotDumpVisitor(args.diff, args.dark)
+visitor = DotDumpVisitor(args.diff, args.dark, args.gray)
 explorer.explore(graph, visitor)
 
 
Index: cfe/trunk/test/Analysis/exploded-graph-rewriter/node_labels.dot
===
--- cfe/trunk/test/Analysis/exploded-graph-rewriter/node_labels.dot
+++ cfe/trunk/test/Analysis/exploded-graph-rewriter/node_labels.dot
@@ -1,6 +1,11 @@
-// RUN: %exploded_graph_rewriter %s | FileCheck %s -check-prefixes=CHECK,LIGHT
-// RUN: %exploded_graph_rewriter %s --dark | FileCheck %s \
-// RUN: -check-prefixes CHECK,DARK
+// RUN: %exploded_graph_rewriter %s \
+// RUN: | FileCheck %s -check-prefixes=CHECK,LIGHT,COLOR
+// RUN: %exploded_graph_rewriter %s --dark \
+// RUN: | FileCheck %s -check-prefixes CHECK,DARK,COLOR
+// RUN: %exploded_graph_rewriter %s --gray \
+// RUN: | FileCheck %s -check-prefixes=CHECK,LIGHT,GRAY
+// RUN: %exploded_graph_rewriter %s --gray --dark \
+// RUN: | FileCheck %s -check-prefixes CHECK,DARK,GRAY
 
 // FIXME: Substitution doesn't seem to work on Windows.
 // UNSUPPORTED: system-windows
@@ -23,10 +28,12 @@
 
 // CHECK: Node0x2 [
 // CHECK-SAME: 
-// CHECK-SAME:   Bug Report Attached
+// COLOR-SAME:   Bug Report Attached
+// GRAY-SAME:Bug Report Attached
 // CHECK-SAME: 
 // CHECK-SAME: 
-// CHECK-SAME:   Sink Node
+// COLOR-SAME:   Sink Node
+// GRAY-SAME:Sink Node
 // CHECK-SAME: 
 Node0x2 [shape=record,label=
  "{


Index: cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
===
--- cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
+++ cfe/trunk/utils/analyzer/exploded-graph-rewriter.py
@@ -384,24 +384,28 @@
 # A visitor that dumps the ExplodedGraph into a DOT file with fancy HTML-based
 # syntax highlighing.
 class DotDumpVisitor(object):
-def __init__(self, do_diffs, dark_mode):
+def __init__(self, do_diffs, dark_mode, gray_mode):
 super(DotDumpVisitor, self).__init__()
 self._do_diffs = do_diffs
 self._dark_mode = dark_mode
+self._gray_mode = gray_mode
 
 @staticmethod
 def _dump_raw(s):
 print(s, end='')
 
-@staticmethod
-def _dump(s):
-print(s.replace('&', '&')
-

[PATCH] D24372: [libcxx] Sprinkle constexpr over compressed_pair

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Is this patch relevant any more?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D24372



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

[PATCH] D61366: [libcxx] [test] Don't assert that moved-from containers with non-POCMA allocators are empty.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a comment.

In D61366#1569151 , @mclow.lists wrote:

> Is there a reason this hasn't been committed?


Because it needs that one change Casey requested I was going to do that next 
time we take a libcxx test harness update, and I was hoping we'd get some 
closure on what happens with https://reviews.llvm.org/D61364 before doing that. 
Would you like me to expedite this change specifically?


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

https://reviews.llvm.org/D61366



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

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam updated this revision to Diff 207882.
zahiraam marked 14 inline comments as done.
zahiraam added a comment.

Thanks for the review.
I  think and hope that I have responded to every issue you raised. Let me know 
if there are still pending issues.
Happy 4th!


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

https://reviews.llvm.org/D43576

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DeclNodes.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclBase.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/Sema/ms-uuid-1.cpp
  clang/test/Sema/ms-uuid-2.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp

Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -321,12 +321,15 @@
 OS << "\" << get" << getUpperName() << "().getAsString() << \"";
   else if (type == "ParamIdx")
 OS << "\" << get" << getUpperName() << "().getSourceIndex() << \"";
-  else
+  else if (type == "DeclSpecUuidDecl *") {
+OS << "\" << get" << getUpperName() << "() << \"";
+  } else
 OS << "\" << get" << getUpperName() << "() << \"";
 }
 
 void writeDump(raw_ostream &OS) const override {
-  if (type == "FunctionDecl *" || type == "NamedDecl *") {
+  if (type == "FunctionDecl *" || type == "NamedDecl *" ||
+  type == "DeclSpecUuidDecl *") {
 OS << "OS << \" \";\n";
 OS << "dumpBareDeclRef(SA->get" << getUpperName() << "());\n"; 
   } else if (type == "IdentifierInfo *") {
@@ -1296,6 +1299,8 @@
 Ptr = llvm::make_unique(Arg, Attr);
   else if (ArgName == "VersionArgument")
 Ptr = llvm::make_unique(Arg, Attr);
+  else if (ArgName == "DeclSpecUuidDeclArgument")
+Ptr = llvm::make_unique(Arg, Attr, "DeclSpecUuidDecl *");
 
   if (!Ptr) {
 // Search in reverse order so that the most-derived type is handled first.
Index: clang/test/Sema/ms-uuid-2.cpp
===
--- /dev/null
+++ clang/test/Sema/ms-uuid-2.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -fms-extensions -fms-compatibility -std=c++14  %s
+
+
+typedef struct _GUID
+{
+unsigned long  Data1;
+unsigned short Data2;
+unsigned short Data3;
+unsigned char  Data4[8];
+} GUID;
+
+// expected-error@+5 {{C++ requires a type specifier for all declarations}}
+// expected-error@+4 {{invalid digit 'a' in decimal constant}}
+// expected-error@+3 {{use of undeclared identifier 'def0'}}
+// expected-error@+2 {{invalid digit 'a' in decimal constant}}
+// expected-error@+1 {{expected ';' after top level declarator}}
+uuid(12345678-9abc-def0-1234-56789abcdef0) struct S2;
Index: clang/test/Sema/ms-uuid-1.cpp
===
--- /dev/null
+++ clang/test/Sema/ms-uuid-1.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -fms-extensions -fms-compatibility -std=c++14 %s
+// expected-no-diagnostics
+typedef struct _GUID {
+  int i;
+} IID;
+template 
+class A {};
+
+struct
+__declspec(uuid("{DDB47A6A-0F23-11D5-9109-00E0296B75D3}"))
+S1 {};
+
+struct
+__declspec(uuid("{DDB47A6A-0F23-11D5-9109-00E0296B75D3}"))
+S2 {};
+
+struct __declspec(dllexport)
+C1 : public A<&__uuidof(S1)> {};
+
+struct __declspec(dllexport)
+C2 : public A<&__uuidof(S2)> {};
+int printf(const char *, ...);
+int main() {
+
+  if (&__uuidof(S1) == &__uuidof(S2))
+printf("OK\n");
+  else
+printf("ERROR\n");
+
+  return 0;
+}
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -641,6 +641,11 @@
   return Inst;
 }
 
+Decl *
+TemplateDeclInstantiator::VisitDeclSpecUuidDecl(DeclSpecUuidDecl *D) {
+   llvm_unreachable("DeclSpecUuidDecl cannot be instantiated");
+}
+
 Decl *TemplateDeclInstantiator::InstantiateTypedefNameDecl(TypedefNameDecl *D,
bool IsTypeAlias) {
   bool Invalid = false;
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -628,7 +628,7 @@
   return ExprError(Diag(TypeidLoc, diag::err_uuidof_without_guid));
 if (UuidAttrs.size() > 1)
   return ExprError(Diag(TypeidLoc, diag::err_uuidof_with_multiple_guids));
-UuidStr = UuidAttrs.back()->getGuid();
+UuidStr = UuidAttrs.back()->getDecl

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added inline comments.



Comment at: include/clang/AST/Decl.h:4303
+
+   StringLiteral *getSTLUuid() { return STLUuid; }
+};

rsmith wrote:
> What does "STL" mean here?
Renamed it.



Comment at: lib/CodeGen/CodeGenModule.cpp:1071-1073
+  const auto ExistingRecord = Manglings.find(MangledName);
+  if (ExistingRecord != std::end(Manglings))
+Manglings.remove(&(*ExistingRecord));

rsmith wrote:
> Was this supposed to be included in this patch? It looks like this is 
> papering over a bug elsewhere.
This is the code that actually fixes the bug. The rest of the patch is to 
represent uuid in the AST.



Comment at: lib/CodeGen/CodeGenModule.cpp:1071-1073
+  const auto ExistingRecord = Manglings.find(MangledName);
+  if (ExistingRecord != std::end(Manglings))
+Manglings.remove(&(*ExistingRecord));

zahiraam wrote:
> rsmith wrote:
> > Was this supposed to be included in this patch? It looks like this is 
> > papering over a bug elsewhere.
> This is the code that actually fixes the bug. The rest of the patch is to 
> represent uuid in the AST.
Removed.


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

https://reviews.llvm.org/D43576



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

[PATCH] D61366: [libcxx] [test] Don't assert that moved-from containers with non-POCMA allocators are empty.

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

Is there a reason this hasn't been committed?


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

https://reviews.llvm.org/D61366



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

[PATCH] D64153: [analyzer] exploded-graph-rewriter: Add a grayscale mode.

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

Szelethus accepted this revision.
Szelethus added a comment.

BRILLIANT


Repository:
  rC Clang

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

https://reviews.llvm.org/D64153



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

[PATCH] D64156: Make joined instances of JoinedOrSeparate flags point to the unaliased args, like all other arg types do

[Nico Weber via Phabricator via cfe-commits]

thakis created this revision.
thakis added a reviewer: ruiu.
Herald added subscribers: MaskRay, aheejin, hiraditya, arichardson, sbc100, 
emaste.
Herald added a reviewer: espindola.
Herald added a project: LLVM.

This fixes an 8-year-old regression. r105763 made it so that aliases always 
refer to the unaliased option – but it missed the "joined" branch of 
JoinedOrSeparate flags. (r162231 then made the Args classes non-virtual, and 
r169344 moved them from clang to llvm.)

Back then, there was no JoinedOrSeparate flag that was an alias, so it wasn't 
observable. Now /U in CLCompatOptions is a JoinedOrSeparate alias in clang, and 
warn_slash_u_filename incorrectly used the aliased arg id (using the unaliased 
one isn't really a regression since that warning checks if the undefined macro 
contains slash or backslash and only then emits the warning – and no valid use 
will pass "-Ufoo/bar" or similar).

Also, lld has many JoinedOrSeparate aliases, and due to this bug it had to 
explicitly call `getUnaliasedOption()` in a bunch of places, even though that 
shouldn't be necessary by design. After this fix in Option, these calls really 
don't have an effect any more, so remove them.

No intended behavior change.

(I accidentally fixed this bug while working on PR29106 but then wondered why 
the warn_slash_u_filename broke. When I figured it out, I thought it would make 
sense to land this in a separate commit.)


https://reviews.llvm.org/D64156

Files:
  clang/lib/Driver/Driver.cpp
  lld/COFF/Driver.cpp
  lld/ELF/Driver.cpp
  lld/ELF/DriverUtils.cpp
  lld/MinGW/Driver.cpp
  lld/wasm/Driver.cpp
  llvm/lib/Option/Option.cpp


Index: llvm/lib/Option/Option.cpp
===
--- llvm/lib/Option/Option.cpp
+++ llvm/lib/Option/Option.cpp
@@ -207,7 +207,7 @@
 // FIXME: Avoid strlen.
 if (ArgSize != strlen(Args.getArgString(Index))) {
   const char *Value = Args.getArgString(Index) + ArgSize;
-  return new Arg(*this, Spelling, Index++, Value);
+  return new Arg(UnaliasedOption, Spelling, Index++, Value);
 }
 
 // Otherwise it must be separate.
Index: lld/wasm/Driver.cpp
===
--- lld/wasm/Driver.cpp
+++ lld/wasm/Driver.cpp
@@ -271,7 +271,7 @@
 
 void LinkerDriver::createFiles(opt::InputArgList &Args) {
   for (auto *Arg : Args) {
-switch (Arg->getOption().getUnaliasedOption().getID()) {
+switch (Arg->getOption().getID()) {
 case OPT_l:
   addLibrary(Arg->getValue());
   break;
@@ -519,7 +519,7 @@
 
   // Copy the command line to the output while rewriting paths.
   for (auto *Arg : Args) {
-switch (Arg->getOption().getUnaliasedOption().getID()) {
+switch (Arg->getOption().getID()) {
 case OPT_reproduce:
   break;
 case OPT_INPUT:
Index: lld/MinGW/Driver.cpp
===
--- lld/MinGW/Driver.cpp
+++ lld/MinGW/Driver.cpp
@@ -313,7 +313,7 @@
   StringRef Prefix = "";
   bool Static = false;
   for (auto *A : Args) {
-switch (A->getOption().getUnaliasedOption().getID()) {
+switch (A->getOption().getID()) {
 case OPT_INPUT:
   if (StringRef(A->getValue()).endswith_lower(".def"))
 Add("-def:" + StringRef(A->getValue()));
Index: lld/ELF/DriverUtils.cpp
===
--- lld/ELF/DriverUtils.cpp
+++ lld/ELF/DriverUtils.cpp
@@ -172,7 +172,7 @@
 
   // Copy the command line to the output while rewriting paths.
   for (auto *Arg : Args) {
-switch (Arg->getOption().getUnaliasedOption().getID()) {
+switch (Arg->getOption().getID()) {
 case OPT_reproduce:
   break;
 case OPT_INPUT:
Index: lld/ELF/Driver.cpp
===
--- lld/ELF/Driver.cpp
+++ lld/ELF/Driver.cpp
@@ -1110,7 +1110,7 @@
 
   // Iterate over argv to process input files and positional arguments.
   for (auto *Arg : Args) {
-switch (Arg->getOption().getUnaliasedOption().getID()) {
+switch (Arg->getOption().getID()) {
 case OPT_library:
   addLibrary(Arg->getValue());
   break;
Index: lld/COFF/Driver.cpp
===
--- lld/COFF/Driver.cpp
+++ lld/COFF/Driver.cpp
@@ -332,7 +332,7 @@
   }
 
   for (auto *Arg : Args) {
-switch (Arg->getOption().getUnaliasedOption().getID()) {
+switch (Arg->getOption().getID()) {
 case OPT_aligncomm:
   parseAligncomm(Arg->getValue());
   break;
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2180,7 +2180,7 @@
 Diag(clang::diag::err_drv_unknown_language) << A->getValue();
 InputType = types::TY_Object;
   }
-} else if (A->getOption().getID() == options::OPT__SLASH_U) {
+} else if (A->getOption().getID() == options

[clang-tools-extra] r365078 - Fixed a link in ReleaseNotes.rst (follow-up to r365007)

[Dmitri Gribenko via cfe-commits]

Author: gribozavr
Date: Wed Jul  3 13:18:34 2019
New Revision: 365078

URL: http://llvm.org/viewvc/llvm-project?rev=365078&view=rev
Log:
Fixed a link in ReleaseNotes.rst (follow-up to r365007)

Modified:
clang-tools-extra/trunk/docs/ReleaseNotes.rst

Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=365078&r1=365077&r2=365078&view=diff
==
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Wed Jul  3 13:18:34 2019
@@ -126,7 +126,7 @@ Improvements to clang-tidy
   branches in conditional operators.
 
 - New :doc:`bugprone-posix-return
-  ` check.
+  ` check.
 
   Checks if any calls to POSIX functions (except ``posix_openpt``) expect 
negative
   return values.


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

[PATCH] D64119: [PowerPC] Support constraint code "ww"

[Jinsong Ji via Phabricator via cfe-commits]

jsji added a comment.

It is great to add `ww` for compatibility. 
However if we are going to add `ww`, looks like we should update `ws` as well?




Comment at: clang/lib/Basic/Targets/PPC.h:211
+  case 's': // VSX vector register to hold scalar double data
+  case 'w': // VSX vector register to hold scalar double data
   case 'a': // Any VSX register

Add some more comments for `w` to distinguish it from `s`?

Do we want to keep compatibility with GCC? 
According to 
https://gcc.gnu.org/onlinedocs/gcc-9.1.0/gcc/Machine-Constraints.html#Machine-Constraints,
 
`ww` is `FP or VSX register to perform float operations under -mvsx or 
NO_REGS.`, 
while `ws` is `VSX vector register to hold scalar double values `. 

So `ww` can use `FP` while `ws` can NOT ?



Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:14080
 return std::make_pair(0U, &PPC::VSRCRegClass);
-  } else if (Constraint == "ws" && Subtarget.hasVSX()) {
+  } else if ((Constraint == "ws" || Constraint == "ww") && Subtarget.hasVSX()) 
{
 if (VT == MVT::f32 && Subtarget.hasP8Vector())

Should we exclude `FP` for `ws` and return `VFRCRegClass` instead of 
`VSFRCRegClass` ?



Comment at: llvm/test/CodeGen/PowerPC/inlineasm-vsx-reg.ll:42
+
+define float @test_ww(float %x, float %y) {
+  %1 = tail call float asm "xsmaxdp ${0:x}, ${1:x}, ${2:x}", 
"=^ww,^ww,^ww"(float %x, float %y)

Maybe we should add another test for ws as well? The above test is actually for 
'x' modifier?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64119



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

[PATCH] D63753: [Sema] Instead of rejecting C unions with non-trivial fields, detect attempts to destruct/initialize/copy them.

[Akira Hatanaka via Phabricator via cfe-commits]

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



Comment at: include/clang/AST/Type.h:1133
+  /// Check if this is or contains a non-trivial C struct/union type.
+  bool hasNonTrivialPrimitiveCStruct() const;
 

rjmccall wrote:
> You only want these checks to trigger on unions with non-trivial members (or 
> structs containing them), right?  How about something like 
> `hasNonTrivialPrimitiveCUnionMember()`?  Or maybe make it more descriptive 
> for the use sites, like `isPrimitiveCRestrictedType()`?
> 
> Also, it would be nice if the fast path of this could be inlined so that 
> clients usually didn't had to make a call at all.  You can write the 
> `getBaseElementTypeUnsafe()->getAs()` part in an `inline` 
> implementation at the bottom this file.
Since we don't keep track of whether a struct or union is or contains unions 
with non-trivial members, we'll have to use the visitors to detect such structs 
or unions or, to do it faster, add a bit to `RecordDeclBits` that indicates the 
presence of non-trivial unions. I guess it's okay to add another bit to 
`RecordDeclBits`?


Repository:
  rC Clang

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

https://reviews.llvm.org/D63753



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

[PATCH] D43159: Modernize: Use nullptr more.

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added inline comments.



Comment at: include/__threading_support:323
 bool __libcpp_thread_isnull(const __libcpp_thread_t *__t) {
-  return *__t == 0;
+  return *__t == nullptr;
 }

mclow.lists wrote:
> This one is wrong.
`__libcpp_thread_t` is an alias for an operating-system specific type.
On Mac OS, it is a pointer to some Darwin-specific type.
On Ubuntu, it is a `const unsigned long`.

You can't compare it to `nullptr`.



Repository:
  rCXX libc++

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

https://reviews.llvm.org/D43159



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

[PATCH] D64153: [analyzer] exploded-graph-rewriter: Add a grayscale mode.

[Csaba Dabis via Phabricator via cfe-commits]

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

Well, it is always awesome to think about the others. Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D64153



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

[PATCH] D64151: Enhance abseil-faster-strsplit-delimiter to handle other non-printable characters.

[Xiaoyi Zhang via Phabricator via cfe-commits]

zhangxy988 added a comment.

I don't think I have commit access.
Please commit it for me. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64151



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

[PATCH] D64153: [analyzer] exploded-graph-rewriter: Add a grayscale mode.

[Artem Dergachev via Phabricator via cfe-commits]

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

Accessibility! I remembered to implement accessibility.

F9454779: Screen Shot 2019-07-03 at 12.51.22 PM.png 
 F9454780: Screen Shot 2019-07-03 at 
12.51.03 PM.png 


Repository:
  rC Clang

https://reviews.llvm.org/D64153

Files:
  clang/test/Analysis/exploded-graph-rewriter/node_labels.dot
  clang/utils/analyzer/exploded-graph-rewriter.py


Index: clang/utils/analyzer/exploded-graph-rewriter.py
===
--- clang/utils/analyzer/exploded-graph-rewriter.py
+++ clang/utils/analyzer/exploded-graph-rewriter.py
@@ -384,24 +384,28 @@
 # A visitor that dumps the ExplodedGraph into a DOT file with fancy HTML-based
 # syntax highlighing.
 class DotDumpVisitor(object):
-def __init__(self, do_diffs, dark_mode):
+def __init__(self, do_diffs, dark_mode, gray_mode):
 super(DotDumpVisitor, self).__init__()
 self._do_diffs = do_diffs
 self._dark_mode = dark_mode
+self._gray_mode = gray_mode
 
 @staticmethod
 def _dump_raw(s):
 print(s, end='')
 
-@staticmethod
-def _dump(s):
-print(s.replace('&', '&')
-   .replace('{', '\\{')
-   .replace('}', '\\}')
-   .replace('\\<', '<')
-   .replace('\\>', '>')
-   .replace('\\l', '')
-   .replace('|', '\\|'), end='')
+def _dump(self, s):
+s = s.replace('&', '&') \
+ .replace('{', '\\{') \
+ .replace('}', '\\}') \
+ .replace('\\<', '<') \
+ .replace('\\>', '>') \
+ .replace('\\l', '') \
+ .replace('|', '\\|')
+if self._gray_mode:
+s = re.sub(r'', '', s)
+s = re.sub(r'', '', s)
+self._dump_raw(s)
 
 @staticmethod
 def _diff_plus_minus(is_added):
@@ -835,6 +839,9 @@
 parser.add_argument('--dark', action='store_const', dest='dark',
 const=True, default=False,
 help='dark mode')
+parser.add_argument('--gray', action='store_const', dest='gray',
+const=True, default=False,
+help='black-and-white mode')
 args = parser.parse_args()
 logging.basicConfig(level=args.loglevel)
 
@@ -845,7 +852,7 @@
 graph.add_raw_line(raw_line)
 
 explorer = BasicExplorer()
-visitor = DotDumpVisitor(args.diff, args.dark)
+visitor = DotDumpVisitor(args.diff, args.dark, args.gray)
 explorer.explore(graph, visitor)
 
 
Index: clang/test/Analysis/exploded-graph-rewriter/node_labels.dot
===
--- clang/test/Analysis/exploded-graph-rewriter/node_labels.dot
+++ clang/test/Analysis/exploded-graph-rewriter/node_labels.dot
@@ -1,6 +1,11 @@
-// RUN: %exploded_graph_rewriter %s | FileCheck %s -check-prefixes=CHECK,LIGHT
-// RUN: %exploded_graph_rewriter %s --dark | FileCheck %s \
-// RUN: -check-prefixes CHECK,DARK
+// RUN: %exploded_graph_rewriter %s \
+// RUN: | FileCheck %s -check-prefixes=CHECK,LIGHT,COLOR
+// RUN: %exploded_graph_rewriter %s --dark \
+// RUN: | FileCheck %s -check-prefixes CHECK,DARK,COLOR
+// RUN: %exploded_graph_rewriter %s --gray \
+// RUN: | FileCheck %s -check-prefixes=CHECK,LIGHT,GRAY
+// RUN: %exploded_graph_rewriter %s --gray --dark \
+// RUN: | FileCheck %s -check-prefixes CHECK,DARK,GRAY
 
 // FIXME: Substitution doesn't seem to work on Windows.
 // UNSUPPORTED: system-windows
@@ -23,10 +28,12 @@
 
 // CHECK: Node0x2 [
 // CHECK-SAME: 
-// CHECK-SAME:   Bug Report Attached
+// COLOR-SAME:   Bug Report Attached
+// GRAY-SAME:Bug Report Attached
 // CHECK-SAME: 
 // CHECK-SAME: 
-// CHECK-SAME:   Sink Node
+// COLOR-SAME:   Sink Node
+// GRAY-SAME:Sink Node
 // CHECK-SAME: 
 Node0x2 [shape=record,label=
  "{


Index: clang/utils/analyzer/exploded-graph-rewriter.py
===
--- clang/utils/analyzer/exploded-graph-rewriter.py
+++ clang/utils/analyzer/exploded-graph-rewriter.py
@@ -384,24 +384,28 @@
 # A visitor that dumps the ExplodedGraph into a DOT file with fancy HTML-based
 # syntax highlighing.
 class DotDumpVisitor(object):
-def __init__(self, do_diffs, dark_mode):
+def __init__(self, do_diffs, dark_mode, gray_mode):
 super(DotDumpVisitor, self).__init__()
 self._do_diffs = do_diffs
 self._dark_mode = dark_mode
+self._gray_mode = gray_mode
 
 @staticmethod
 def _dump_raw(s):
 print(s, end='')
 
-@staticmethod
-def _dump(s):
-print(s.replace('&', '&')
-   .replace('{'

[PATCH] D63915: [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of function calls

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

Thanks for the reviews! The remaining question is: do we want to use 
`Optional<>` in the `CallDescriptionMap::lookup()`?




Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:98-100
+// The APIModeling package is for checkers that model APIs. These checkers are
+// always turned on; this package is intended for API modeling that is not
+// controlled by the target triple.

Szelethus wrote:
> Charusso wrote:
> > NoQ wrote:
> > > Charusso wrote:
> > > > NoQ wrote:
> > > > > Szelethus wrote:
> > > > > > Charusso wrote:
> > > > > > > Szelethus wrote:
> > > > > > > > This isn't true: the user may decide to only enable 
> > > > > > > > non-pathsensitive checkers.
> > > > > > > > 
> > > > > > > > I think the comment should rather state that these whether 
> > > > > > > > these checkers are enabled shouldn't be explicitly specified, 
> > > > > > > > unless in **extreme** circumstances (causes a crash in a 
> > > > > > > > release build?).
> > > > > > > Well, I have removed it instead. Makes no sense, you are right.
> > > > > > I don't think it's a good idea -- we definitely should eventually 
> > > > > > be able to list packages with descriptions just like checkers 
> > > > > > (there actually is a FIXME in CheckerRegistry.cpp for that), but 
> > > > > > this is the next best thing that we have.
> > > > > > 
> > > > > > How about this:
> > > > > > ```
> > > > > > // The APIModeling package is for checkers that model APIs and 
> > > > > > don't perform
> > > > > > // any diagnostics. Checkers within this package are enabled by the 
> > > > > > core or
> > > > > > // through checker dependencies, so one shouldn't enable/disable 
> > > > > > them by
> > > > > > // hand (unless they cause a crash, but that will cause dependent 
> > > > > > checkers to be
> > > > > > // implicitly disabled).
> > > > > > ```
> > > > > I don't think any of these are dependencies. Most of the 
> > > > > `apiModeling` checkers are there to suppress infeasible paths 
> > > > > (exactly like this one).
> > > > > 
> > > > > I think i'd prefer to leave the comment as-is. We can always update 
> > > > > it later.
> > > > Thanks! Copy-pasted, just that patch produce diagnostics as notes.
> > > Let's change to `don't emit any warnings` then.
> > I think an APIModeling could not be too much thing, most of the stuff 
> > around it is not commented out what they do. But as @Szelethus really 
> > wanted to inject that, I cannot say no to a copy-paste.
> Some are, but saying something along the lines of "most of these are enabled 
> by default by the Driver" would've been more correct, but yea, let's leave 
> this for another day.
Well, okay. Something like that is supposed to be correct for future 
developments:
```
// Checkers within APIModeling package are model APIs and enabled by the core   
 
// or through checker dependencies, so one should not enable/disable them by
 
// hand (unless they cause a crash, but that will cause dependent checkers to   
 
// be implicitly disabled). 
 
// They do not emit any warnings, just suppress infeasible paths.
```



Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:88-93
+  Optional RawExpectedValue = CDM.lookup(Call);
+  if (!RawExpectedValue)
+return;
+
+  SVal ReturnV = Call.getReturnValue();
+  bool ExpectedValue = **RawExpectedValue;

NoQ wrote:
> This can still be re-used by moving into `isInvariantBreak` (you can give it 
> access to `CDM` by making it non-static).
Well, sadly not. In both of the checks it is used inside the call, so you 
cannot just remove it. I really wanted to make it as simple as possible, but 
you know, "not simpler".



Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:14
+
+#include "clang/Driver/DriverDiagnostic.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"

xazax.hun wrote:
> Is DriverDiagnostic used for something?
I thought, but definitely not, good catch!



Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:69
+  Name += Call.getCalleeIdentifier()->getName();
+  return Name;
+}

xazax.hun wrote:
> `CXXMethodDecl::getQualifiedNameAsString` is not doing what you want here?
We want to drop the namespaces for better readability.


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

https://reviews.llvm.org/D63915



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

[PATCH] D63915: [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of function calls

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 207862.
Charusso marked 12 inline comments as done.
Charusso added a comment.

- More fix.


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

https://reviews.llvm.org/D63915

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp
  clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  clang/test/Analysis/return-value-guaranteed.cpp
  clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp

Index: clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
===
--- clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
+++ clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
@@ -33,11 +33,11 @@
 Impl(std::move(Data)) {}
 
   const bool *lookup(const CallEvent &Call) {
-const bool *Result = Impl.lookup(Call);
+Optional Result = Impl.lookup(Call);
 // If it's a function we expected to find, remember that we've found it.
-if (Result && *Result)
+if (Result && *Result && **Result)
   ++Found;
-return Result;
+return *Result;
   }
 
   // Fail the test if we haven't found all the true-calls we were looking for.
Index: clang/test/Analysis/return-value-guaranteed.cpp
===
--- /dev/null
+++ clang/test/Analysis/return-value-guaranteed.cpp
@@ -0,0 +1,91 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,apiModeling.ReturnValue \
+// RUN:  -analyzer-output=text -verify=class %s
+
+struct Foo { int Field; };
+bool problem();
+void doSomething();
+
+// We predefined the return value of 'MCAsmParser::Error' as true and we cannot
+// take the false-branches which leads to a "garbage value" false positive.
+namespace test_classes {
+struct MCAsmParser {
+  static bool Error();
+};
+
+bool parseFoo(Foo &F) {
+  if (problem()) {
+// class-note@-1 {{Assuming the condition is false}}
+// class-note@-2 {{Taking false branch}}
+return MCAsmParser::Error();
+  }
+
+  F.Field = 0;
+  // class-note@-1 {{The value 0 is assigned to 'F.Field'}}
+  return !MCAsmParser::Error();
+  // class-note@-1 {{'MCAsmParser::Error' returns true}}
+}
+
+bool parseFile() {
+  Foo F;
+  if (parseFoo(F)) {
+// class-note@-1 {{Calling 'parseFoo'}}
+// class-note@-2 {{Returning from 'parseFoo'}}
+// class-note@-3 {{Taking false branch}}
+return true;
+  }
+
+  if (F.Field == 0) {
+// class-note@-1 {{Field 'Field' is equal to 0}}
+// class-note@-2 {{Taking true branch}}
+
+// no-warning: "The left operand of '==' is a garbage value" was here.
+doSomething();
+  }
+
+  (void)(1 / F.Field);
+  // class-warning@-1 {{Division by zero}}
+  // class-note@-2 {{Division by zero}}
+  return false;
+}
+} // namespace test_classes
+
+
+// We predefined 'MCAsmParser::Error' as returning true, but now it returns
+// false, which breaks our invariant. Test the notes.
+namespace test_break {
+struct MCAsmParser {
+  static bool Error() {
+return false; // class-note {{'MCAsmParser::Error' returns false}}
+  }
+};
+
+bool parseFoo(Foo &F) {
+  if (problem()) {
+// class-note@-1 {{Assuming the condition is false}}
+// class-note@-2 {{Taking false branch}}
+return !MCAsmParser::Error();
+  }
+
+  F.Field = 0;
+  // class-note@-1 {{The value 0 is assigned to 'F.Field'}}
+  return MCAsmParser::Error();
+  // class-note@-1 {{Calling 'MCAsmParser::Error'}}
+  // class-note@-2 {{Returning from 'MCAsmParser::Error'}}
+}
+
+bool parseFile() {
+  Foo F;
+  if (parseFoo(F)) {
+// class-note@-1 {{Calling 'parseFoo'}}
+// class-note@-2 {{Returning from 'parseFoo'}}
+// class-note@-3 {{Taking false branch}}
+return true;
+  }
+
+  (void)(1 / F.Field);
+  // class-warning@-1 {{Division by zero}}
+  // class-note@-2 {{Division by zero}}
+  return false;
+}
+} // namespace test_classes
Index: clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
===
--- clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -780,6 +780,9 @@
   NewAllocElt->getAllocatorExpr()->getBeginLoc(), SMng);
 }
 llvm_unreachable("Unexpected CFG element at front of block");
+  } else if (Optional FE = P.getAs()) {
+return PathDiagnosticLocation(FE->getStmt(), SMng,
+  FE->getLocationContext());
   } else {
 llvm_unreachable("Unexpected ProgramPoint");
   }
Index: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp
===
--- /dev/null
+++ clang/lib/StaticAnalyzer/Checkers/Return

[PATCH] D43159: Modernize: Use nullptr more.

[Louis Dionne via Phabricator via cfe-commits]

ldionne requested changes to this revision.
ldionne added a comment.

I agree with Marshall's requests.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D43159



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

[PATCH] D63915: [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of function calls

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

Szelethus added inline comments.



Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:98-100
+// The APIModeling package is for checkers that model APIs. These checkers are
+// always turned on; this package is intended for API modeling that is not
+// controlled by the target triple.

Charusso wrote:
> NoQ wrote:
> > Charusso wrote:
> > > NoQ wrote:
> > > > Szelethus wrote:
> > > > > Charusso wrote:
> > > > > > Szelethus wrote:
> > > > > > > This isn't true: the user may decide to only enable 
> > > > > > > non-pathsensitive checkers.
> > > > > > > 
> > > > > > > I think the comment should rather state that these whether these 
> > > > > > > checkers are enabled shouldn't be explicitly specified, unless in 
> > > > > > > **extreme** circumstances (causes a crash in a release build?).
> > > > > > Well, I have removed it instead. Makes no sense, you are right.
> > > > > I don't think it's a good idea -- we definitely should eventually be 
> > > > > able to list packages with descriptions just like checkers (there 
> > > > > actually is a FIXME in CheckerRegistry.cpp for that), but this is the 
> > > > > next best thing that we have.
> > > > > 
> > > > > How about this:
> > > > > ```
> > > > > // The APIModeling package is for checkers that model APIs and don't 
> > > > > perform
> > > > > // any diagnostics. Checkers within this package are enabled by the 
> > > > > core or
> > > > > // through checker dependencies, so one shouldn't enable/disable them 
> > > > > by
> > > > > // hand (unless they cause a crash, but that will cause dependent 
> > > > > checkers to be
> > > > > // implicitly disabled).
> > > > > ```
> > > > I don't think any of these are dependencies. Most of the `apiModeling` 
> > > > checkers are there to suppress infeasible paths (exactly like this one).
> > > > 
> > > > I think i'd prefer to leave the comment as-is. We can always update it 
> > > > later.
> > > Thanks! Copy-pasted, just that patch produce diagnostics as notes.
> > Let's change to `don't emit any warnings` then.
> I think an APIModeling could not be too much thing, most of the stuff around 
> it is not commented out what they do. But as @Szelethus really wanted to 
> inject that, I cannot say no to a copy-paste.
Some are, but saying something along the lines of "most of these are enabled by 
default by the Driver" would've been more correct, but yea, let's leave this 
for another day.


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

https://reviews.llvm.org/D63915



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

[PATCH] D63915: [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of function calls

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

Some nits inline, note that this was just a partial review.




Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:14
+
+#include "clang/Driver/DriverDiagnostic.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"

Is DriverDiagnostic used for something?



Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:38
+  // The pairs are in the following form: {{{class, call}}, return value}
+  CallDescriptionMap CDM = {
+  // These are known in the LLVM project: 'Error()'

Maybe this map can be const?



Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:65
+  if (const auto *MD = dyn_cast(Call.getDecl()))
+if (const auto *RD = dyn_cast(MD->getParent()))
+  Name += RD->getNameAsString() + "::";

Do you need the cast here?



Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:69
+  Name += Call.getCalleeIdentifier()->getName();
+  return Name;
+}

`CXXMethodDecl::getQualifiedNameAsString` is not doing what you want here?


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

https://reviews.llvm.org/D63915



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

[PATCH] D64151: Enhance abseil-faster-strsplit-delimiter to handle other non-printable characters.

[Dmitri Gribenko via Phabricator via cfe-commits]

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

Do you have commit access? Would you like me to commit this patch for you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64151



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

[PATCH] D63915: [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of function calls

[Csaba Dabis via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:98-100
+// The APIModeling package is for checkers that model APIs. These checkers are
+// always turned on; this package is intended for API modeling that is not
+// controlled by the target triple.

NoQ wrote:
> Charusso wrote:
> > NoQ wrote:
> > > Szelethus wrote:
> > > > Charusso wrote:
> > > > > Szelethus wrote:
> > > > > > This isn't true: the user may decide to only enable 
> > > > > > non-pathsensitive checkers.
> > > > > > 
> > > > > > I think the comment should rather state that these whether these 
> > > > > > checkers are enabled shouldn't be explicitly specified, unless in 
> > > > > > **extreme** circumstances (causes a crash in a release build?).
> > > > > Well, I have removed it instead. Makes no sense, you are right.
> > > > I don't think it's a good idea -- we definitely should eventually be 
> > > > able to list packages with descriptions just like checkers (there 
> > > > actually is a FIXME in CheckerRegistry.cpp for that), but this is the 
> > > > next best thing that we have.
> > > > 
> > > > How about this:
> > > > ```
> > > > // The APIModeling package is for checkers that model APIs and don't 
> > > > perform
> > > > // any diagnostics. Checkers within this package are enabled by the 
> > > > core or
> > > > // through checker dependencies, so one shouldn't enable/disable them by
> > > > // hand (unless they cause a crash, but that will cause dependent 
> > > > checkers to be
> > > > // implicitly disabled).
> > > > ```
> > > I don't think any of these are dependencies. Most of the `apiModeling` 
> > > checkers are there to suppress infeasible paths (exactly like this one).
> > > 
> > > I think i'd prefer to leave the comment as-is. We can always update it 
> > > later.
> > Thanks! Copy-pasted, just that patch produce diagnostics as notes.
> Let's change to `don't emit any warnings` then.
I think an APIModeling could not be too much thing, most of the stuff around it 
is not commented out what they do. But as @Szelethus really wanted to inject 
that, I cannot say no to a copy-paste.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:1119
 
-  const T *lookup(const CallEvent &Call) const {
+  Optional lookup(const CallEvent &Call) const {
 // Slow path: linear lookup.

NoQ wrote:
> Charusso wrote:
> > Szelethus wrote:
> > > Charusso wrote:
> > > > NoQ wrote:
> > > > > I hope `T` never gets too expensive to copy. The ideal return value 
> > > > > here is `Optional` but i suspect that `llvm::Optional`s 
> > > > > don't support this (while C++17 `std::optional`s do). Could you 
> > > > > double-check my vague memories here?
> > > > Optional is working and used widely, I like that.
> > > Why do we need the optional AND the pointer? How about we just return 
> > > with a nullptr if we fail to find the call?
> > `Optional<>` stands for optional values, that is why it is made. @NoQ tried 
> > to avoid it, but I believe if someone does not use it for an optional 
> > value, that break some kind of unspoken standard.
> Well, that'd be the original code.
> 
> > I do not like `Optional` anymore.
> 
> @Charusso, do you still plan to undo this change?
Well, I am here at 2:1 against `Optional<>`, so I think it is your decision.



Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:95-96
+  Optional IsInvariantBreak = isInvariantBreak(ExpectedValue, ReturnV, 
C);
+  if (!IsInvariantBreak)
+return;
+

NoQ wrote:
> This looks flipped to me, should probably say `if (IsInvariantBreak) return;`.
It is the `Optional<>` checking, whether we could obtain the value. I really 
wanted to write `!hasValue()`, but no one use that, so it is some kind of 
unspoken standard to just `!` it.


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

https://reviews.llvm.org/D63915



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

[PATCH] D53457: clang-cl: Add "/clang:" pass-through arg support.

[Nico Weber via Phabricator via cfe-commits]

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

I noticed that this causes memory errors in certain situations. 
https://bugs.llvm.org/show_bug.cgi?id=42501 has details. Can you take a look?


Repository:
  rC Clang

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

https://reviews.llvm.org/D53457



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

[PATCH] D64151: Enhance abseil-faster-strsplit-delimiter to handle other non-printable characters.

[Xiaoyi Zhang via Phabricator via cfe-commits]

zhangxy988 created this revision.
zhangxy988 added a reviewer: gribozavr.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Currently it fails on cases like '\001'.

Note: Since `StringLiteral::outputString` dumps most nonprintable
characters in octal value, the exact string literal format isn't preserved,
e.g. `"\x01"` becomes `'\001'`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64151

Files:
  clang-tools-extra/clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp
  clang-tools-extra/test/clang-tidy/abseil-faster-strsplit-delimiter.cpp


Index: clang-tools-extra/test/clang-tidy/abseil-faster-strsplit-delimiter.cpp
===
--- clang-tools-extra/test/clang-tidy/abseil-faster-strsplit-delimiter.cpp
+++ clang-tools-extra/test/clang-tidy/abseil-faster-strsplit-delimiter.cpp
@@ -44,6 +44,31 @@
   // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit() called with a 
string literal consisting of a single character; consider using the character 
overload [abseil-faster-strsplit-delimiter]
   // CHECK-FIXES: absl::StrSplit("ABC", 'A');
 
+  absl::StrSplit("ABC", "\x01");
+  // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit() called with a 
string literal consisting of a single character; consider using the character 
overload [abseil-faster-strsplit-delimiter]
+  // CHECK-FIXES: absl::StrSplit("ABC", '\x01');
+
+  absl::StrSplit("ABC", "\001");
+  // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit() called with a 
string literal consisting of a single character; consider using the character 
overload [abseil-faster-strsplit-delimiter]
+  // CHECK-FIXES: absl::StrSplit("ABC", '\001');
+
+  absl::StrSplit("ABC", R"(A)");
+  // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit() called with a 
string literal consisting of a single character; consider using the character 
overload [abseil-faster-strsplit-delimiter]
+  // CHECK-FIXES: absl::StrSplit("ABC", 'B');
+
+  absl::StrSplit("ABC", R"(')");
+  // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit() called with a 
string literal consisting of a single character; consider using the character 
overload [abseil-faster-strsplit-delimiter]
+  // CHECK-FIXES: absl::StrSplit("ABC", '\'');
+
+  absl::StrSplit("ABC", R"(
+)");
+  // CHECK-MESSAGES: [[@LINE-2]]:25: warning: absl::StrSplit() called with a 
string literal consisting of a single character; consider using the character 
overload [abseil-faster-strsplit-delimiter]
+  // CHECK-FIXES: absl::StrSplit("ABC", '\n');
+
+  absl::StrSplit("ABC", R"delimiter(A)delimiter");
+  // CHECK-MESSAGES: [[@LINE-1]]:25: warning: absl::StrSplit() called with a 
string literal consisting of a single character; consider using the character 
overload [abseil-faster-strsplit-delimiter]
+  // CHECK-FIXES: absl::StrSplit("ABC", 'A');
+
   absl::StrSplit("ABC", absl::ByAnyChar("\n"));
   // CHECK-MESSAGES: [[@LINE-1]]:41: warning: absl::StrSplit()
   // CHECK-FIXES: absl::StrSplit("ABC", '\n');
Index: clang-tools-extra/clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp
===
--- clang-tools-extra/clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp
+++ clang-tools-extra/clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp
@@ -7,8 +7,10 @@
 
//===--===//
 
 #include "FasterStrsplitDelimiterCheck.h"
+
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Tooling/FixIt.h"
 
 using namespace clang::ast_matchers;
 
@@ -35,23 +37,27 @@
   return constructExprWithArg(ClassName, constructExprWithArg(ClassName, Arg));
 }
 
-llvm::Optional makeCharacterLiteral(const StringLiteral *Literal) 
{
-  std::string Result;
-  {
+llvm::Optional makeCharacterLiteral(const StringLiteral *Literal,
+ const ASTContext &Context) {
+  assert(Literal->getLength() == 1);
+  assert(Literal->getCharByteWidth() == 1); // no wide char
+  std::string Result = clang::tooling::fixit::getText(*Literal, Context).str();
+  bool IsRawStringLiteral = StringRef(Result).startswith(R"(R")");
+  // Since raw string literal might contain unescaped non-printable characters,
+  // we normalize them using `StringLiteral::outputString`.
+  if (IsRawStringLiteral) {
+Result.clear();
 llvm::raw_string_ostream Stream(Result);
 Literal->outputString(Stream);
   }
-
   // Special case: If the string contains a single quote, we just need to 
return
   // a character of the single quote. This is a special case because we need to
   // escape it in the character literal.
   if (Result == R"("'")")
 return std::string(R"('\'')");
 
-  assert(Result.size() == 3 || (Result.size() == 4 && Result.substr(0, 2) == 
"\"\\"));
-
   // Now replace the " with '.
-  auto Pos = Result.find_first_of('"');
+  std::string::size_type Pos = Resul

[PATCH] D62611: [analyzer][Dominators] Add unittests

[Gábor Horváth via Phabricator via cfe-commits]

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

Looks good to me, some nits inline.




Comment at: clang/unittests/Analysis/CFGBuildResult.h:1
+//===- unittests/Analysis/CFGTest.cpp - CFG tests 
-===//
+//

Filename not updated?



Comment at: clang/unittests/Analysis/CFGDominatorTree.cpp:1
+//===- unittests/Analysis/CFGTest.cpp - CFG tests 
-===//
+//

Filename not updated?



Comment at: clang/unittests/Analysis/CFGDominatorTree.cpp:13
+#include "clang/Analysis/CFG.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"

Do you need the Tooling header here?


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

https://reviews.llvm.org/D62611



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

[PATCH] D64067: [X86][PPC] Support -mlong-double-64

[Andy Kaylor via Phabricator via cfe-commits]

andrew.w.kaylor added a comment.

In D64067#1568895 , @hfinkel wrote:

> One thing to realize about these flags is that they're ABI-altering flags. If 
> the user provides the flag to alter the platform defaults, this only works if 
> the user also ensures that matches the ABI of the relevant system libraries 
> that the compiler is using (e.g., because the user is explicitly linking with 
> a suitable libc).


Right. That's what I was trying to figure out. Are we relying on the users of 
this option not to shoot themselves in the foot? It sounds like yes. I believe 
that's the way we handled it in icc also, but gcc and icc do a lot of sketchy 
things that we wouldn't want to do in clang. In this case I don't object to the 
sketchiness, just so everyone realizes that's what we're doing.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64067



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

[PATCH] D63915: [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of function calls

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:98-100
+// The APIModeling package is for checkers that model APIs. These checkers are
+// always turned on; this package is intended for API modeling that is not
+// controlled by the target triple.

Charusso wrote:
> NoQ wrote:
> > Szelethus wrote:
> > > Charusso wrote:
> > > > Szelethus wrote:
> > > > > This isn't true: the user may decide to only enable non-pathsensitive 
> > > > > checkers.
> > > > > 
> > > > > I think the comment should rather state that these whether these 
> > > > > checkers are enabled shouldn't be explicitly specified, unless in 
> > > > > **extreme** circumstances (causes a crash in a release build?).
> > > > Well, I have removed it instead. Makes no sense, you are right.
> > > I don't think it's a good idea -- we definitely should eventually be able 
> > > to list packages with descriptions just like checkers (there actually is 
> > > a FIXME in CheckerRegistry.cpp for that), but this is the next best thing 
> > > that we have.
> > > 
> > > How about this:
> > > ```
> > > // The APIModeling package is for checkers that model APIs and don't 
> > > perform
> > > // any diagnostics. Checkers within this package are enabled by the core 
> > > or
> > > // through checker dependencies, so one shouldn't enable/disable them by
> > > // hand (unless they cause a crash, but that will cause dependent 
> > > checkers to be
> > > // implicitly disabled).
> > > ```
> > I don't think any of these are dependencies. Most of the `apiModeling` 
> > checkers are there to suppress infeasible paths (exactly like this one).
> > 
> > I think i'd prefer to leave the comment as-is. We can always update it 
> > later.
> Thanks! Copy-pasted, just that patch produce diagnostics as notes.
Let's change to `don't emit any warnings` then.


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

https://reviews.llvm.org/D63915



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

[PATCH] D63915: [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of function calls

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

In D63915#1568166 , @Szelethus wrote:

> This checker seems to only check LLVM functions, but doesn't check whether 
> these methods lie in the LLVM namespace. Is this intended?


Thanks for the reviews! They are not in the `llvm` namespace.




Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:98-100
+// The APIModeling package is for checkers that model APIs. These checkers are
+// always turned on; this package is intended for API modeling that is not
+// controlled by the target triple.

NoQ wrote:
> Szelethus wrote:
> > Charusso wrote:
> > > Szelethus wrote:
> > > > This isn't true: the user may decide to only enable non-pathsensitive 
> > > > checkers.
> > > > 
> > > > I think the comment should rather state that these whether these 
> > > > checkers are enabled shouldn't be explicitly specified, unless in 
> > > > **extreme** circumstances (causes a crash in a release build?).
> > > Well, I have removed it instead. Makes no sense, you are right.
> > I don't think it's a good idea -- we definitely should eventually be able 
> > to list packages with descriptions just like checkers (there actually is a 
> > FIXME in CheckerRegistry.cpp for that), but this is the next best thing 
> > that we have.
> > 
> > How about this:
> > ```
> > // The APIModeling package is for checkers that model APIs and don't perform
> > // any diagnostics. Checkers within this package are enabled by the core or
> > // through checker dependencies, so one shouldn't enable/disable them by
> > // hand (unless they cause a crash, but that will cause dependent checkers 
> > to be
> > // implicitly disabled).
> > ```
> I don't think any of these are dependencies. Most of the `apiModeling` 
> checkers are there to suppress infeasible paths (exactly like this one).
> 
> I think i'd prefer to leave the comment as-is. We can always update it later.
Thanks! Copy-pasted, just that patch produce diagnostics as notes.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:1119
 
-  const T *lookup(const CallEvent &Call) const {
+  Optional lookup(const CallEvent &Call) const {
 // Slow path: linear lookup.

Szelethus wrote:
> Charusso wrote:
> > NoQ wrote:
> > > I hope `T` never gets too expensive to copy. The ideal return value here 
> > > is `Optional` but i suspect that `llvm::Optional`s don't 
> > > support this (while C++17 `std::optional`s do). Could you double-check my 
> > > vague memories here?
> > Optional is working and used widely, I like that.
> Why do we need the optional AND the pointer? How about we just return with a 
> nullptr if we fail to find the call?
`Optional<>` stands for optional values, that is why it is made. @NoQ tried to 
avoid it, but I believe if someone does not use it for an optional value, that 
break some kind of unspoken standard.



Comment at: clang/test/Analysis/return-value-guaranteed.cpp:90
+
+// no-warning: "The left operand of '==' is a garbage value" was here.
+doSomething();

Szelethus wrote:
> Was it? I just tried it out and it doesn't seem to be the case.
Whoops, too heavy copy-pasting.


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

https://reviews.llvm.org/D63915



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

[PATCH] D63915: [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of function calls

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 207851.
Charusso marked 8 inline comments as done.
Charusso added a comment.

- Fix.
- Document `NoteTag`.


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

https://reviews.llvm.org/D63915

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp
  clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  clang/test/Analysis/return-value-guaranteed.cpp
  clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp

Index: clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
===
--- clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
+++ clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
@@ -33,11 +33,11 @@
 Impl(std::move(Data)) {}
 
   const bool *lookup(const CallEvent &Call) {
-const bool *Result = Impl.lookup(Call);
+Optional Result = Impl.lookup(Call);
 // If it's a function we expected to find, remember that we've found it.
-if (Result && *Result)
+if (Result && *Result && **Result)
   ++Found;
-return Result;
+return *Result;
   }
 
   // Fail the test if we haven't found all the true-calls we were looking for.
Index: clang/test/Analysis/return-value-guaranteed.cpp
===
--- /dev/null
+++ clang/test/Analysis/return-value-guaranteed.cpp
@@ -0,0 +1,91 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,apiModeling.ReturnValue \
+// RUN:  -analyzer-output=text -verify=class %s
+
+struct Foo { int Field; };
+bool problem();
+void doSomething();
+
+// We predefined the return value of 'MCAsmParser::Error' as true and we cannot
+// take the false-branches which leads to a "garbage value" false positive.
+namespace test_classes {
+struct MCAsmParser {
+  static bool Error();
+};
+
+bool parseFoo(Foo &F) {
+  if (problem()) {
+// class-note@-1 {{Assuming the condition is false}}
+// class-note@-2 {{Taking false branch}}
+return MCAsmParser::Error();
+  }
+
+  F.Field = 0;
+  // class-note@-1 {{The value 0 is assigned to 'F.Field'}}
+  return !MCAsmParser::Error();
+  // class-note@-1 {{'MCAsmParser::Error' returns true}}
+}
+
+bool parseFile() {
+  Foo F;
+  if (parseFoo(F)) {
+// class-note@-1 {{Calling 'parseFoo'}}
+// class-note@-2 {{Returning from 'parseFoo'}}
+// class-note@-3 {{Taking false branch}}
+return true;
+  }
+
+  if (F.Field == 0) {
+// class-note@-1 {{Field 'Field' is equal to 0}}
+// class-note@-2 {{Taking true branch}}
+
+// no-warning: "The left operand of '==' is a garbage value" was here.
+doSomething();
+  }
+
+  (void)(1 / F.Field);
+  // class-warning@-1 {{Division by zero}}
+  // class-note@-2 {{Division by zero}}
+  return false;
+}
+} // namespace test_classes
+
+
+// We predefined 'MCAsmParser::Error' as returning true, but now it returns
+// false, which breaks our invariant. Test the notes.
+namespace test_break {
+struct MCAsmParser {
+  static bool Error() {
+return false; // class-note {{'MCAsmParser::Error' returns false}}
+  }
+};
+
+bool parseFoo(Foo &F) {
+  if (problem()) {
+// class-note@-1 {{Assuming the condition is false}}
+// class-note@-2 {{Taking false branch}}
+return !MCAsmParser::Error();
+  }
+
+  F.Field = 0;
+  // class-note@-1 {{The value 0 is assigned to 'F.Field'}}
+  return MCAsmParser::Error();
+  // class-note@-1 {{Calling 'MCAsmParser::Error'}}
+  // class-note@-2 {{Returning from 'MCAsmParser::Error'}}
+}
+
+bool parseFile() {
+  Foo F;
+  if (parseFoo(F)) {
+// class-note@-1 {{Calling 'parseFoo'}}
+// class-note@-2 {{Returning from 'parseFoo'}}
+// class-note@-3 {{Taking false branch}}
+return true;
+  }
+
+  (void)(1 / F.Field);
+  // class-warning@-1 {{Division by zero}}
+  // class-note@-2 {{Division by zero}}
+  return false;
+}
+} // namespace test_classes
Index: clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
===
--- clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -780,6 +780,9 @@
   NewAllocElt->getAllocatorExpr()->getBeginLoc(), SMng);
 }
 llvm_unreachable("Unexpected CFG element at front of block");
+  } else if (Optional FE = P.getAs()) {
+return PathDiagnosticLocation(FE->getStmt(), SMng,
+  FE->getLocationContext());
   } else {
 llvm_unreachable("Unexpected ProgramPoint");
   }
Index: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp
===
--- /dev/null
+++ clang/lib/StaticAnalyzer

[PATCH] D63915: [analyzer] ReturnValueChecker: Model the guaranteed boolean return value of function calls

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:98-100
+// The APIModeling package is for checkers that model APIs. These checkers are
+// always turned on; this package is intended for API modeling that is not
+// controlled by the target triple.

Szelethus wrote:
> Charusso wrote:
> > Szelethus wrote:
> > > This isn't true: the user may decide to only enable non-pathsensitive 
> > > checkers.
> > > 
> > > I think the comment should rather state that these whether these checkers 
> > > are enabled shouldn't be explicitly specified, unless in **extreme** 
> > > circumstances (causes a crash in a release build?).
> > Well, I have removed it instead. Makes no sense, you are right.
> I don't think it's a good idea -- we definitely should eventually be able to 
> list packages with descriptions just like checkers (there actually is a FIXME 
> in CheckerRegistry.cpp for that), but this is the next best thing that we 
> have.
> 
> How about this:
> ```
> // The APIModeling package is for checkers that model APIs and don't perform
> // any diagnostics. Checkers within this package are enabled by the core or
> // through checker dependencies, so one shouldn't enable/disable them by
> // hand (unless they cause a crash, but that will cause dependent checkers to 
> be
> // implicitly disabled).
> ```
I don't think any of these are dependencies. Most of the `apiModeling` checkers 
are there to suppress infeasible paths (exactly like this one).

I think i'd prefer to leave the comment as-is. We can always update it later.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:1119
 
-  const T *lookup(const CallEvent &Call) const {
+  Optional lookup(const CallEvent &Call) const {
 // Slow path: linear lookup.

Charusso wrote:
> Szelethus wrote:
> > Charusso wrote:
> > > NoQ wrote:
> > > > I hope `T` never gets too expensive to copy. The ideal return value 
> > > > here is `Optional` but i suspect that `llvm::Optional`s 
> > > > don't support this (while C++17 `std::optional`s do). Could you 
> > > > double-check my vague memories here?
> > > Optional is working and used widely, I like that.
> > Why do we need the optional AND the pointer? How about we just return with 
> > a nullptr if we fail to find the call?
> `Optional<>` stands for optional values, that is why it is made. @NoQ tried 
> to avoid it, but I believe if someone does not use it for an optional value, 
> that break some kind of unspoken standard.
Well, that'd be the original code.

> I do not like `Optional` anymore.

@Charusso, do you still plan to undo this change?



Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:88-93
+  Optional RawExpectedValue = CDM.lookup(Call);
+  if (!RawExpectedValue)
+return;
+
+  SVal ReturnV = Call.getReturnValue();
+  bool ExpectedValue = **RawExpectedValue;

This can still be re-used by moving into `isInvariantBreak` (you can give it 
access to `CDM` by making it non-static).



Comment at: clang/lib/StaticAnalyzer/Checkers/ReturnValueChecker.cpp:95-96
+  Optional IsInvariantBreak = isInvariantBreak(ExpectedValue, ReturnV, 
C);
+  if (!IsInvariantBreak)
+return;
+

This looks flipped to me, should probably say `if (IsInvariantBreak) return;`.


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

https://reviews.llvm.org/D63915



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

[PATCH] D63538: [CFG] Add a new function to get the proper condition of a CFGBlock

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: clang/lib/Analysis/CFG.cpp:5619-5625
+  // If the terminator is a temporary dtor or a virtual base, etc, we can't
+  // retrieve a meaningful condition, bail out.
+  if (rbegin()->getKind() != CFGElement::Kind::Statement)
+return nullptr;
+
+  // This should be the condition of the terminator block.
+  const Stmt *S = rbegin()->castAs().getStmt();

NoQ wrote:
> I don't think you're looking at the terminator here, as `CFGTerminator` isn't 
> a sub-class of `CFGElement`. So this if doesn't avoid temporary dtor branches 
> or virtual base branches; instead, it avoids various other `CFGElement` 
> sub-classes such as `CFGInitializer` or `CFGImplicitDtor` (not sure how many 
> of those may appear last in a block).
> 
> Therefore the following code would be a bit more LLVM-y:
> ```lang=c++
> auto StmtElem = rbegin()->getAs();
> if (!StmtElem)
>   return nullptr;
> 
> const Stmt *S = StmtElem->getStmt();
> ```
> 
> Also, are you sure that the last expression is always a condition? Like, what 
> about
> ```lang=c++
> void foo(int x, int y) {
>   (void)(x + y);
> }
> ```
> ?
The question is, is `CFGTerminator ` is what we are looking for? I think the 
point of the method is to get the last subexpression that was evaluated before 
taking a branch. 

For the condition, would it make sense to check the number of successors 
(before pruning the trivially false branches)? If less than or equal to 1 it is 
probably not a condition. Otherwise maybe we could just rename it to last 
evaluated (sub)expression. The sub-part might depend on whether we use a 
linearized CFG.


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

https://reviews.llvm.org/D63538



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

[PATCH] D64067: [X86][PPC] Support -mlong-double-64

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In D64067#156 , @rnk wrote:

> In D64067#1568533 , @andrew.w.kaylor 
> wrote:
>
> > In this review (https://reviews.llvm.org/D6260) @rsmith mentions that this 
> > should also have an effect on name mangling.
>
>
> I'm not sure that's consistent with GCC, at least not anymore:
>  https://gcc.godbolt.org/z/eUviCd
>  Looks like you can still have an overload set with double and long double, 
> even if they both use the same representation.


It has to work that way, because they're different, standard language-level 
types.

One thing to realize about these flags is that they're ABI-altering flags. If 
the user provides the flag to alter the platform defaults, this only works if 
the user also ensures that matches the ABI of the relevant system libraries 
that the compiler is using (e.g., because the user is explicitly linking with a 
suitable libc).

> This is a backend -m flag, after all, so that seems reasonable to me.
> 
>> What will this do if the user calls a library function that expects a long 
>> double? What does gcc do in that case?
> 
> Looks like it passes according to the usual 64-bit IEEE double representation.




Repository:
  rC Clang

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

https://reviews.llvm.org/D64067



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

[PATCH] D64067: [X86][PPC] Support -mlong-double-64

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In D64067#1568533 , @andrew.w.kaylor 
wrote:

> In this review (https://reviews.llvm.org/D6260) @rsmith mentions that this 
> should also have an effect on name mangling.


I'm not sure that's consistent with GCC, at least not anymore:
https://gcc.godbolt.org/z/eUviCd
Looks like you can still have an overload set with double and long double, even 
if they both use the same representation. This is a backend -m flag, after all, 
so that seems reasonable to me.

> What will this do if the user calls a library function that expects a long 
> double? What does gcc do in that case?

Looks like it passes according to the usual 64-bit IEEE double representation.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64067



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

[PATCH] D63538: [CFG] Add a new function to get the proper condition of a CFGBlock

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/Analysis/CFG.cpp:5619-5625
+  // If the terminator is a temporary dtor or a virtual base, etc, we can't
+  // retrieve a meaningful condition, bail out.
+  if (rbegin()->getKind() != CFGElement::Kind::Statement)
+return nullptr;
+
+  // This should be the condition of the terminator block.
+  const Stmt *S = rbegin()->castAs().getStmt();

I don't think you're looking at the terminator here, as `CFGTerminator` isn't a 
sub-class of `CFGElement`. So this if doesn't avoid temporary dtor branches or 
virtual base branches; instead, it avoids various other `CFGElement` 
sub-classes such as `CFGInitializer` or `CFGImplicitDtor` (not sure how many of 
those may appear last in a block).

Therefore the following code would be a bit more LLVM-y:
```lang=c++
auto StmtElem = rbegin()->getAs();
if (!StmtElem)
  return nullptr;

const Stmt *S = StmtElem->getStmt();
```

Also, are you sure that the last expression is always a condition? Like, what 
about
```lang=c++
void foo(int x, int y) {
  (void)(x + y);
}
```
?


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

https://reviews.llvm.org/D63538



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

[PATCH] D64149: [clang-scan-deps] use `-Wno-error` when scanning for dependencies

[Alex Lorenz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL365065: [clang-scan-deps] use `-Wno-error` when scanning for 
dependencies (authored by arphaman, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64149?vs=207844&id=207847#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64149

Files:
  cfe/trunk/test/ClangScanDeps/Inputs/no-werror.json
  cfe/trunk/test/ClangScanDeps/Inputs/sys-header.h
  cfe/trunk/test/ClangScanDeps/no-werror.cpp
  cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp


Index: cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp
+++ cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -129,6 +129,7 @@
 AdjustedArgs.push_back("-Eonly");
 AdjustedArgs.push_back("-Xclang");
 AdjustedArgs.push_back("-sys-header-deps");
+AdjustedArgs.push_back("-Wno-error");
 return AdjustedArgs;
   });
 
Index: cfe/trunk/test/ClangScanDeps/no-werror.cpp
===
--- cfe/trunk/test/ClangScanDeps/no-werror.cpp
+++ cfe/trunk/test/ClangScanDeps/no-werror.cpp
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t.dir
+// RUN: rm -rf %t.cdb
+// RUN: mkdir -p %t.dir
+// RUN: cp %s %t.dir/no-werror.cpp
+// RUN: mkdir %t.dir/Inputs
+// RUN: cp %S/Inputs/sys-header.h %t.dir/Inputs/sys-header.h
+// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/no-werror.json > %t.cdb
+//
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 | FileCheck %s
+
+#define MACRO 201411L
+
+#include "sys-header.h"
+
+// CHECK: no-werror.cpp
+// CHECK-NEXT: Inputs{{/|\\}}sys-header.h
Index: cfe/trunk/test/ClangScanDeps/Inputs/sys-header.h
===
--- cfe/trunk/test/ClangScanDeps/Inputs/sys-header.h
+++ cfe/trunk/test/ClangScanDeps/Inputs/sys-header.h
@@ -0,0 +1 @@
+#define MACRO 201411
Index: cfe/trunk/test/ClangScanDeps/Inputs/no-werror.json
===
--- cfe/trunk/test/ClangScanDeps/Inputs/no-werror.json
+++ cfe/trunk/test/ClangScanDeps/Inputs/no-werror.json
@@ -0,0 +1,7 @@
+[
+{
+  "directory": "DIR",
+  "command": "clang -E DIR/no-werror.cpp -IInputs -std=c++17 -Weverything 
-Werror",
+  "file": "DIR/no-werror.cpp"
+}
+]


Index: cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp
+++ cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -129,6 +129,7 @@
 AdjustedArgs.push_back("-Eonly");
 AdjustedArgs.push_back("-Xclang");
 AdjustedArgs.push_back("-sys-header-deps");
+AdjustedArgs.push_back("-Wno-error");
 return AdjustedArgs;
   });
 
Index: cfe/trunk/test/ClangScanDeps/no-werror.cpp
===
--- cfe/trunk/test/ClangScanDeps/no-werror.cpp
+++ cfe/trunk/test/ClangScanDeps/no-werror.cpp
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t.dir
+// RUN: rm -rf %t.cdb
+// RUN: mkdir -p %t.dir
+// RUN: cp %s %t.dir/no-werror.cpp
+// RUN: mkdir %t.dir/Inputs
+// RUN: cp %S/Inputs/sys-header.h %t.dir/Inputs/sys-header.h
+// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/no-werror.json > %t.cdb
+//
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 | FileCheck %s
+
+#define MACRO 201411L
+
+#include "sys-header.h"
+
+// CHECK: no-werror.cpp
+// CHECK-NEXT: Inputs{{/|\\}}sys-header.h
Index: cfe/trunk/test/ClangScanDeps/Inputs/sys-header.h
===
--- cfe/trunk/test/ClangScanDeps/Inputs/sys-header.h
+++ cfe/trunk/test/ClangScanDeps/Inputs/sys-header.h
@@ -0,0 +1 @@
+#define MACRO 201411
Index: cfe/trunk/test/ClangScanDeps/Inputs/no-werror.json
===
--- cfe/trunk/test/ClangScanDeps/Inputs/no-werror.json
+++ cfe/trunk/test/ClangScanDeps/Inputs/no-werror.json
@@ -0,0 +1,7 @@
+[
+{
+  "directory": "DIR",
+  "command": "clang -E DIR/no-werror.cpp -IInputs -std=c++17 -Weverything -Werror",
+  "file": "DIR/no-werror.cpp"
+}
+]
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64128: [CodeGen] Generate llvm.ptrmask instead of inttoptr(and(ptrtoint, C)) if possible.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

I don't think this transform is valid, for the same reasons we don't do it in 
IR optimizations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64128



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

r365065 - [clang-scan-deps] use `-Wno-error` when scanning for dependencies

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Wed Jul  3 11:01:32 2019
New Revision: 365065

URL: http://llvm.org/viewvc/llvm-project?rev=365065&view=rev
Log:
[clang-scan-deps] use `-Wno-error` when scanning for dependencies

Warnings can be promoted to errors.
But that shouldn't prevent us from getting the dependencies!

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

Added:
cfe/trunk/test/ClangScanDeps/Inputs/no-werror.json
cfe/trunk/test/ClangScanDeps/Inputs/sys-header.h
cfe/trunk/test/ClangScanDeps/no-werror.cpp
Modified:
cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp

Added: cfe/trunk/test/ClangScanDeps/Inputs/no-werror.json
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ClangScanDeps/Inputs/no-werror.json?rev=365065&view=auto
==
--- cfe/trunk/test/ClangScanDeps/Inputs/no-werror.json (added)
+++ cfe/trunk/test/ClangScanDeps/Inputs/no-werror.json Wed Jul  3 11:01:32 2019
@@ -0,0 +1,7 @@
+[
+{
+  "directory": "DIR",
+  "command": "clang -E DIR/no-werror.cpp -IInputs -std=c++17 -Weverything 
-Werror",
+  "file": "DIR/no-werror.cpp"
+}
+]

Added: cfe/trunk/test/ClangScanDeps/Inputs/sys-header.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ClangScanDeps/Inputs/sys-header.h?rev=365065&view=auto
==
--- cfe/trunk/test/ClangScanDeps/Inputs/sys-header.h (added)
+++ cfe/trunk/test/ClangScanDeps/Inputs/sys-header.h Wed Jul  3 11:01:32 2019
@@ -0,0 +1 @@
+#define MACRO 201411

Added: cfe/trunk/test/ClangScanDeps/no-werror.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ClangScanDeps/no-werror.cpp?rev=365065&view=auto
==
--- cfe/trunk/test/ClangScanDeps/no-werror.cpp (added)
+++ cfe/trunk/test/ClangScanDeps/no-werror.cpp Wed Jul  3 11:01:32 2019
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t.dir
+// RUN: rm -rf %t.cdb
+// RUN: mkdir -p %t.dir
+// RUN: cp %s %t.dir/no-werror.cpp
+// RUN: mkdir %t.dir/Inputs
+// RUN: cp %S/Inputs/sys-header.h %t.dir/Inputs/sys-header.h
+// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/no-werror.json > %t.cdb
+//
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 | FileCheck %s
+
+#define MACRO 201411L
+
+#include "sys-header.h"
+
+// CHECK: no-werror.cpp
+// CHECK-NEXT: Inputs{{/|\\}}sys-header.h

Modified: cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp?rev=365065&r1=365064&r2=365065&view=diff
==
--- cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp (original)
+++ cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp Wed Jul  3 11:01:32 2019
@@ -129,6 +129,7 @@ int main(int argc, const char **argv) {
 AdjustedArgs.push_back("-Eonly");
 AdjustedArgs.push_back("-Xclang");
 AdjustedArgs.push_back("-sys-header-deps");
+AdjustedArgs.push_back("-Wno-error");
 return AdjustedArgs;
   });
 


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

[PATCH] D64149: [clang-scan-deps] use `-Wno-error` when scanning for dependencies

[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


Repository:
  rC Clang

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

https://reviews.llvm.org/D64149



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

[PATCH] D64149: [clang-scan-deps] use `-Wno-error` when scanning for dependencies

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.
arphaman added a reviewer: Bigcheese.
Herald added subscribers: tschuett, dexonsmith, jkorous.
Herald added a project: clang.

Warning can be promoted to errors. But that shouldn't prevent us from getting 
the dependencies!


Repository:
  rC Clang

https://reviews.llvm.org/D64149

Files:
  clang/test/ClangScanDeps/Inputs/no-werror.json
  clang/test/ClangScanDeps/Inputs/sys-header.h
  clang/test/ClangScanDeps/no-werror.cpp
  clang/tools/clang-scan-deps/ClangScanDeps.cpp


Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -129,6 +129,7 @@
 AdjustedArgs.push_back("-Eonly");
 AdjustedArgs.push_back("-Xclang");
 AdjustedArgs.push_back("-sys-header-deps");
+AdjustedArgs.push_back("-Wno-error");
 return AdjustedArgs;
   });
 
Index: clang/test/ClangScanDeps/no-werror.cpp
===
--- /dev/null
+++ clang/test/ClangScanDeps/no-werror.cpp
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t.dir
+// RUN: rm -rf %t.cdb
+// RUN: mkdir -p %t.dir
+// RUN: cp %s %t.dir/no-werror.cpp
+// RUN: mkdir %t.dir/Inputs
+// RUN: cp %S/Inputs/sys-header.h %t.dir/Inputs/sys-header.h
+// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/no-werror.json > %t.cdb
+//
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 | FileCheck %s
+
+#define MACRO 201411L
+
+#include "sys-header.h"
+
+// CHECK: no-werror.cpp
+// CHECK-NEXT: Inputs{{/|\\}}sys-header.h
Index: clang/test/ClangScanDeps/Inputs/sys-header.h
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/sys-header.h
@@ -0,0 +1 @@
+#define MACRO 201411
Index: clang/test/ClangScanDeps/Inputs/no-werror.json
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/no-werror.json
@@ -0,0 +1,7 @@
+[
+{
+  "directory": "DIR",
+  "command": "clang -E DIR/no-werror.cpp -IInputs -std=c++17 -Weverything 
-Werror",
+  "file": "DIR/no-werror.cpp"
+}
+]


Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -129,6 +129,7 @@
 AdjustedArgs.push_back("-Eonly");
 AdjustedArgs.push_back("-Xclang");
 AdjustedArgs.push_back("-sys-header-deps");
+AdjustedArgs.push_back("-Wno-error");
 return AdjustedArgs;
   });
 
Index: clang/test/ClangScanDeps/no-werror.cpp
===
--- /dev/null
+++ clang/test/ClangScanDeps/no-werror.cpp
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t.dir
+// RUN: rm -rf %t.cdb
+// RUN: mkdir -p %t.dir
+// RUN: cp %s %t.dir/no-werror.cpp
+// RUN: mkdir %t.dir/Inputs
+// RUN: cp %S/Inputs/sys-header.h %t.dir/Inputs/sys-header.h
+// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/no-werror.json > %t.cdb
+//
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 | FileCheck %s
+
+#define MACRO 201411L
+
+#include "sys-header.h"
+
+// CHECK: no-werror.cpp
+// CHECK-NEXT: Inputs{{/|\\}}sys-header.h
Index: clang/test/ClangScanDeps/Inputs/sys-header.h
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/sys-header.h
@@ -0,0 +1 @@
+#define MACRO 201411
Index: clang/test/ClangScanDeps/Inputs/no-werror.json
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/no-werror.json
@@ -0,0 +1,7 @@
+[
+{
+  "directory": "DIR",
+  "command": "clang -E DIR/no-werror.cpp -IInputs -std=c++17 -Weverything -Werror",
+  "file": "DIR/no-werror.cpp"
+}
+]
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63876: [OpenCL] Define CLK_NULL_EVENT without cast

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM! Thanks!

If you like you can also change reserve_id_t. :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D63876



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