[PATCH] D39638: [NVPTX] Implement __nvvm_atom_add_gen_d builtin.

2017-11-05 Thread Justin Lebar via Phabricator via cfe-commits
jlebar created this revision.
Herald added subscribers: hiraditya, sanjoy, jholewinski.

This just seems to have been an oversight.  We already supported the f64
atomic add with an explicit scope (e.g. "cta"), but not the scopeless
version.


https://reviews.llvm.org/D39638

Files:
  clang/include/clang/Basic/BuiltinsNVPTX.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-nvptx-ptx50.cu
  llvm/include/llvm/IR/IntrinsicsNVVM.td
  llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
  llvm/lib/Target/NVPTX/NVPTXIntrinsics.td
  llvm/test/CodeGen/NVPTX/atomics-sm60.ll

Index: llvm/test/CodeGen/NVPTX/atomics-sm60.ll
===
--- /dev/null
+++ llvm/test/CodeGen/NVPTX/atomics-sm60.ll
@@ -0,0 +1,19 @@
+; RUN: llc < %s -march=nvptx -mcpu=sm_60 | FileCheck %s
+; RUN: llc < %s -march=nvptx64 -mcpu=sm_60 | FileCheck %s
+
+; CHECK-LABEL .func test(
+define void @test(double* %dp0, double addrspace(1)* %dp1, double addrspace(3)* %dp3, double %d) {
+; CHECK: atom.add.f64
+  %r1 = call double @llvm.nvvm.atomic.load.add.f64.p0f64(double* %dp0, double %d)
+; CHECK: atom.add.f64
+  %r2 = call double @llvm.nvvm.atomic.load.add.f64.p1f64(double addrspace(1)* %dp1, double %d)
+; CHECK: atom.add.f64
+  %ret = call double @llvm.nvvm.atomic.load.add.f64.p3f64(double addrspace(3)* %dp3, double %d)
+  ret void
+}
+
+declare double @llvm.nvvm.atomic.load.add.f64.p0f64(double* nocapture, double) #1
+declare double @llvm.nvvm.atomic.load.add.f64.p1f64(double addrspace(1)* nocapture, double) #1
+declare double @llvm.nvvm.atomic.load.add.f64.p3f64(double addrspace(3)* nocapture, double) #1
+
+attributes #1 = { argmemonly nounwind }
Index: llvm/lib/Target/NVPTX/NVPTXIntrinsics.td
===
--- llvm/lib/Target/NVPTX/NVPTXIntrinsics.td
+++ llvm/lib/Target/NVPTX/NVPTXIntrinsics.td
@@ -1095,6 +1095,12 @@
   (int_nvvm_atomic_load_add_f32 node:$a, node:$b)>;
 def atomic_load_add_f32_gen: ATOMIC_GENERIC_CHK<(ops node:$a, node:$b),
   (int_nvvm_atomic_load_add_f32 node:$a, node:$b)>;
+def atomic_load_add_f64_g: ATOMIC_GLOBAL_CHK<(ops node:$a, node:$b),
+  (int_nvvm_atomic_load_add_f64 node:$a, node:$b)>;
+def atomic_load_add_f64_s: ATOMIC_SHARED_CHK<(ops node:$a, node:$b),
+  (int_nvvm_atomic_load_add_f64 node:$a, node:$b)>;
+def atomic_load_add_f64_gen: ATOMIC_GENERIC_CHK<(ops node:$a, node:$b),
+  (int_nvvm_atomic_load_add_f64 node:$a, node:$b)>;
 
 defm INT_PTX_ATOM_ADD_G_32 : F_ATOMIC_2;
@@ -1121,6 +1127,13 @@
 defm INT_PTX_ATOM_ADD_GEN_F32 : F_ATOMIC_2;
 
+defm INT_PTX_ATOM_ADD_G_F64 : F_ATOMIC_2;
+defm INT_PTX_ATOM_ADD_S_F64 : F_ATOMIC_2;
+defm INT_PTX_ATOM_ADD_GEN_F64 : F_ATOMIC_2;
+
 // atom_sub
 
 def atomic_load_sub_32_g: ATOMIC_GLOBAL_CHK<(ops node:$a, node:$b),
Index: llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
===
--- llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
+++ llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
@@ -3449,6 +3449,7 @@
   }
 
   case Intrinsic::nvvm_atomic_load_add_f32:
+  case Intrinsic::nvvm_atomic_load_add_f64:
   case Intrinsic::nvvm_atomic_load_inc_32:
   case Intrinsic::nvvm_atomic_load_dec_32:
 
Index: llvm/include/llvm/IR/IntrinsicsNVVM.td
===
--- llvm/include/llvm/IR/IntrinsicsNVVM.td
+++ llvm/include/llvm/IR/IntrinsicsNVVM.td
@@ -683,10 +683,15 @@
   Intrinsic<[llvm_i64_ty], [llvm_double_ty], [IntrNoMem]>;
 
 
-// Atomic not available as an llvm intrinsic.
+// Atomics not available as llvm intrinsics.
   def int_nvvm_atomic_load_add_f32 : Intrinsic<[llvm_float_ty],
   [LLVMAnyPointerType, llvm_float_ty],
   [IntrArgMemOnly, NoCapture<0>]>;
+  // Atomic add of f64 requires sm_60.
+  def int_nvvm_atomic_load_add_f64 : Intrinsic<[llvm_double_ty],
+  [LLVMAnyPointerType, llvm_double_ty],
+  [IntrArgMemOnly, NoCapture<0>]>;
+
   def int_nvvm_atomic_load_inc_32 : Intrinsic<[llvm_i32_ty],
   [LLVMAnyPointerType, llvm_i32_ty],
   [IntrArgMemOnly, NoCapture<0>]>;
Index: clang/test/CodeGen/builtins-nvptx-ptx50.cu
===
--- /dev/null
+++ clang/test/CodeGen/builtins-nvptx-ptx50.cu
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -triple nvptx64-unknown-unknown -target-cpu sm_60 \
+// RUN:-fcuda-is-device -target-feature +ptx50 \
+// RUN:-S -emit-llvm -o - -x cuda %s \
+// RUN:   | FileCheck -check-prefix=CHECK %s
+//
+// RUN: %clang_cc1 -triple nvptx-unknown-unknown -target-cpu sm_60 \
+// RUN:   -fcuda-is-device -S -o /dev/null -x cuda -verify %s
+
+#define __device__ __attribute__((device))
+#define __global__ __attribute__((global))
+#define __shared__ __attribute__((shared))
+#define __constant__ __attribute__((constant))
+
+// We have to keep all builtins that depe

[PATCH] D39638: [NVPTX] Implement __nvvm_atom_add_gen_d builtin.

2017-11-05 Thread Justin Lebar via Phabricator via cfe-commits
jlebar updated this revision to Diff 121620.
jlebar added a comment.

Fix tests.


https://reviews.llvm.org/D39638

Files:
  clang/include/clang/Basic/BuiltinsNVPTX.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-nvptx-ptx50.cu
  llvm/include/llvm/IR/IntrinsicsNVVM.td
  llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
  llvm/lib/Target/NVPTX/NVPTXIntrinsics.td
  llvm/test/CodeGen/NVPTX/atomics-sm60.ll

Index: llvm/test/CodeGen/NVPTX/atomics-sm60.ll
===
--- /dev/null
+++ llvm/test/CodeGen/NVPTX/atomics-sm60.ll
@@ -0,0 +1,19 @@
+; RUN: llc < %s -march=nvptx -mcpu=sm_60 | FileCheck %s
+; RUN: llc < %s -march=nvptx64 -mcpu=sm_60 | FileCheck %s
+
+; CHECK-LABEL .func test(
+define void @test(double* %dp0, double addrspace(1)* %dp1, double addrspace(3)* %dp3, double %d) {
+; CHECK: atom.add.f64
+  %r1 = call double @llvm.nvvm.atomic.load.add.f64.p0f64(double* %dp0, double %d)
+; CHECK: atom.global.add.f64
+  %r2 = call double @llvm.nvvm.atomic.load.add.f64.p1f64(double addrspace(1)* %dp1, double %d)
+; CHECK: atom.shared.add.f64
+  %ret = call double @llvm.nvvm.atomic.load.add.f64.p3f64(double addrspace(3)* %dp3, double %d)
+  ret void
+}
+
+declare double @llvm.nvvm.atomic.load.add.f64.p0f64(double* nocapture, double) #1
+declare double @llvm.nvvm.atomic.load.add.f64.p1f64(double addrspace(1)* nocapture, double) #1
+declare double @llvm.nvvm.atomic.load.add.f64.p3f64(double addrspace(3)* nocapture, double) #1
+
+attributes #1 = { argmemonly nounwind }
Index: llvm/lib/Target/NVPTX/NVPTXIntrinsics.td
===
--- llvm/lib/Target/NVPTX/NVPTXIntrinsics.td
+++ llvm/lib/Target/NVPTX/NVPTXIntrinsics.td
@@ -1095,6 +1095,12 @@
   (int_nvvm_atomic_load_add_f32 node:$a, node:$b)>;
 def atomic_load_add_f32_gen: ATOMIC_GENERIC_CHK<(ops node:$a, node:$b),
   (int_nvvm_atomic_load_add_f32 node:$a, node:$b)>;
+def atomic_load_add_f64_g: ATOMIC_GLOBAL_CHK<(ops node:$a, node:$b),
+  (int_nvvm_atomic_load_add_f64 node:$a, node:$b)>;
+def atomic_load_add_f64_s: ATOMIC_SHARED_CHK<(ops node:$a, node:$b),
+  (int_nvvm_atomic_load_add_f64 node:$a, node:$b)>;
+def atomic_load_add_f64_gen: ATOMIC_GENERIC_CHK<(ops node:$a, node:$b),
+  (int_nvvm_atomic_load_add_f64 node:$a, node:$b)>;
 
 defm INT_PTX_ATOM_ADD_G_32 : F_ATOMIC_2;
@@ -1121,6 +1127,13 @@
 defm INT_PTX_ATOM_ADD_GEN_F32 : F_ATOMIC_2;
 
+defm INT_PTX_ATOM_ADD_G_F64 : F_ATOMIC_2;
+defm INT_PTX_ATOM_ADD_S_F64 : F_ATOMIC_2;
+defm INT_PTX_ATOM_ADD_GEN_F64 : F_ATOMIC_2;
+
 // atom_sub
 
 def atomic_load_sub_32_g: ATOMIC_GLOBAL_CHK<(ops node:$a, node:$b),
Index: llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
===
--- llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
+++ llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
@@ -3449,6 +3449,7 @@
   }
 
   case Intrinsic::nvvm_atomic_load_add_f32:
+  case Intrinsic::nvvm_atomic_load_add_f64:
   case Intrinsic::nvvm_atomic_load_inc_32:
   case Intrinsic::nvvm_atomic_load_dec_32:
 
Index: llvm/include/llvm/IR/IntrinsicsNVVM.td
===
--- llvm/include/llvm/IR/IntrinsicsNVVM.td
+++ llvm/include/llvm/IR/IntrinsicsNVVM.td
@@ -683,10 +683,15 @@
   Intrinsic<[llvm_i64_ty], [llvm_double_ty], [IntrNoMem]>;
 
 
-// Atomic not available as an llvm intrinsic.
+// Atomics not available as llvm intrinsics.
   def int_nvvm_atomic_load_add_f32 : Intrinsic<[llvm_float_ty],
   [LLVMAnyPointerType, llvm_float_ty],
   [IntrArgMemOnly, NoCapture<0>]>;
+  // Atomic add of f64 requires sm_60.
+  def int_nvvm_atomic_load_add_f64 : Intrinsic<[llvm_double_ty],
+  [LLVMAnyPointerType, llvm_double_ty],
+  [IntrArgMemOnly, NoCapture<0>]>;
+
   def int_nvvm_atomic_load_inc_32 : Intrinsic<[llvm_i32_ty],
   [LLVMAnyPointerType, llvm_i32_ty],
   [IntrArgMemOnly, NoCapture<0>]>;
Index: clang/test/CodeGen/builtins-nvptx-ptx50.cu
===
--- /dev/null
+++ clang/test/CodeGen/builtins-nvptx-ptx50.cu
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple nvptx64-unknown-unknown -target-cpu sm_60 \
+// RUN:-fcuda-is-device -S -emit-llvm -o - -x cuda %s \
+// RUN:   | FileCheck -check-prefix=CHECK %s
+//
+// RUN: %clang_cc1 -triple nvptx-unknown-unknown -target-cpu sm_50 \
+// RUN:   -fcuda-is-device -S -o /dev/null -x cuda -verify %s
+
+#define __device__ __attribute__((device))
+#define __global__ __attribute__((global))
+#define __shared__ __attribute__((shared))
+#define __constant__ __attribute__((constant))
+
+// We have to keep all builtins that depend on particular target feature in the
+// same function, because the codegen will stop after the very first function
+// that encounters an error, so -verify will not be able to find errors 

[PATCH] D39631: [X86] Fix the spelling of 3dnow and 3dnowa in isValidFeatureName

2017-11-05 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo updated this revision to Diff 121623.
mstorsjo retitled this revision from "[X86] Add 3dnow and 3dnowa to the list of 
valid target features" to "[X86] Fix the spelling of 3dnow and 3dnowa in 
isValidFeatureName".
mstorsjo edited the summary of this revision.
mstorsjo added a comment.

Updated based on Craig's comments.


https://reviews.llvm.org/D39631

Files:
  lib/Basic/Targets/X86.cpp
  test/Headers/mm3dnow.c


Index: test/Headers/mm3dnow.c
===
--- /dev/null
+++ test/Headers/mm3dnow.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -ffreestanding %s -verify
+// RUN: %clang_cc1 -fsyntax-only -ffreestanding -x c++ %s -verify
+// expected-no-diagnostics
+
+#if defined(i386) || defined(__x86_64__)
+#include 
+
+int __attribute__((__target__(("3dnow" foo(int a) {
+  _m_femms();
+  return 4;
+}
+
+__m64 __attribute__((__target__(("3dnowa" bar(__m64 a) {
+  return _m_pf2iw(a);
+}
+#endif
Index: lib/Basic/Targets/X86.cpp
===
--- lib/Basic/Targets/X86.cpp
+++ lib/Basic/Targets/X86.cpp
@@ -1121,6 +1121,8 @@
 
 bool X86TargetInfo::isValidFeatureName(StringRef Name) const {
   return llvm::StringSwitch(Name)
+  .Case("3dnow", true)
+  .Case("3dnowa", true)
   .Case("aes", true)
   .Case("avx", true)
   .Case("avx2", true)
@@ -1147,8 +1149,6 @@
   .Case("fxsr", true)
   .Case("lwp", true)
   .Case("lzcnt", true)
-  .Case("mm3dnow", true)
-  .Case("mm3dnowa", true)
   .Case("mmx", true)
   .Case("movbe", true)
   .Case("mpx", true)


Index: test/Headers/mm3dnow.c
===
--- /dev/null
+++ test/Headers/mm3dnow.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -ffreestanding %s -verify
+// RUN: %clang_cc1 -fsyntax-only -ffreestanding -x c++ %s -verify
+// expected-no-diagnostics
+
+#if defined(i386) || defined(__x86_64__)
+#include 
+
+int __attribute__((__target__(("3dnow" foo(int a) {
+  _m_femms();
+  return 4;
+}
+
+__m64 __attribute__((__target__(("3dnowa" bar(__m64 a) {
+  return _m_pf2iw(a);
+}
+#endif
Index: lib/Basic/Targets/X86.cpp
===
--- lib/Basic/Targets/X86.cpp
+++ lib/Basic/Targets/X86.cpp
@@ -1121,6 +1121,8 @@
 
 bool X86TargetInfo::isValidFeatureName(StringRef Name) const {
   return llvm::StringSwitch(Name)
+  .Case("3dnow", true)
+  .Case("3dnowa", true)
   .Case("aes", true)
   .Case("avx", true)
   .Case("avx2", true)
@@ -1147,8 +1149,6 @@
   .Case("fxsr", true)
   .Case("lwp", true)
   .Case("lzcnt", true)
-  .Case("mm3dnow", true)
-  .Case("mm3dnowa", true)
   .Case("mmx", true)
   .Case("movbe", true)
   .Case("mpx", true)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r317434 - [clang-diff] NFC: format

2017-11-05 Thread Johannes Altmanninger via cfe-commits
Author: krobelus
Date: Sun Nov  5 03:53:18 2017
New Revision: 317434

URL: http://llvm.org/viewvc/llvm-project?rev=317434&view=rev
Log:
[clang-diff] NFC: format

Modified:
cfe/trunk/tools/clang-diff/ClangDiff.cpp

Modified: cfe/trunk/tools/clang-diff/ClangDiff.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-diff/ClangDiff.cpp?rev=317434&r1=317433&r2=317434&view=diff
==
--- cfe/trunk/tools/clang-diff/ClangDiff.cpp (original)
+++ cfe/trunk/tools/clang-diff/ClangDiff.cpp Sun Nov  5 03:53:18 2017
@@ -33,9 +33,9 @@ static cl::opt ASTDumpJson(
 cl::desc("Print the internal representation of the AST as JSON."),
 cl::init(false), cl::cat(ClangDiffCategory));
 
-static cl::opt
-PrintMatches("dump-matches", cl::desc("Print the matched nodes."),
- cl::init(false), cl::cat(ClangDiffCategory));
+static cl::opt PrintMatches("dump-matches",
+  cl::desc("Print the matched nodes."),
+  cl::init(false), cl::cat(ClangDiffCategory));
 
 static cl::opt HtmlDiff("html",
   cl::desc("Output a side-by-side diff in HTML."),


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


[PATCH] D39611: [CodeGen] make cbrt and fma constant (never set errno); document complex calls as always constant

2017-11-05 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment.

In https://reviews.llvm.org/D39611#915812, @hfinkel wrote:

> In the C specification, 7.12 specifies the requirements for functions in 
> math.h. For those functions, 7.12.1 (Treatment of error conditions) says that 
> overflows do set ERANGE, and that it's implementation defined if the same is 
> true for underflows. That's true for functions in math.h in general. 7.3 
> specifies the requirements for functions in complex.h. Here, 7.3.2 says, "An 
> implementation may set errno but is not required to." That, as I understand 
> it, applies to all functions in complex.h (unless otherwise noted, I 
> presume). However, because setting errno is not required by C for functions 
> in complex.h, when POSIX says "No errors are defined." that constrains the 
> choice (POSIX is constraining the implementation choice allowed by C; this is 
> not a contraction with the C specification). As a result, under POSIX, the 
> functions in complex.h don't set errno (some, as you've noted, may raise FP 
> exceptions, but that's another matter).
>
> Regarding glibc-specific behavior, I do want us to be careful only to model 
> this behavior when the target triple has -gnu. Otherwise, we should stick to 
> the standard behavior.


I'll split 'complex' off into its own patch then. We'll need a new attribute 
code for those functions. Something like:
//  E -> const when -fmath-errno=0 or in a GNU (or a POSIX-compliant?) 
environment

I think we can detect GNU-ness with:
Context.getTargetInfo().getTriple().isGNUEnvironment()

But is that correct? If this is actually a POSIX-compliance-based constraint of 
the choice to not set errno, then GNU is not wide enough to cover other 
platforms like macOS or AIX that are (mostly?) POSIX-compliant. Also, does GNU 
guarantee POSIX-compliance?


https://reviews.llvm.org/D39611



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


[PATCH] D39640: [lit] Set shlibpath_var on Solaris

2017-11-05 Thread Rainer Orth via Phabricator via cfe-commits
ro created this revision.
Herald added a subscriber: fedor.sergeev.

During make check-all on Solaris, lit complains

llvm-lit: /vol/gcc/src/llvm/llvm/dist/tools/clang/test/Unit/lit.cfg.py:57: 
warning: unable to inject shared library path on 'SunOS'

The following patch avoids this: Solaris uses LD_LIBRARY_PATH like several 
other targets.

In theory, once could also handle LD_LIBRARY_PATH_{32,64} which take precedence 
over
LD_LIBRARY_PATH if set, but let's cross that bridge when we get there.


https://reviews.llvm.org/D39640

Files:
  test/Unit/lit.cfg.py


Index: test/Unit/lit.cfg.py
===
--- test/Unit/lit.cfg.py
+++ test/Unit/lit.cfg.py
@@ -36,7 +36,7 @@
 config.environment[symbolizer] = os.environ[symbolizer]
 
 def find_shlibpath_var():
-if platform.system() in ['Linux', 'FreeBSD', 'NetBSD']:
+if platform.system() in ['Linux', 'FreeBSD', 'NetBSD', 'SunOS']:
 yield 'LD_LIBRARY_PATH'
 elif platform.system() == 'Darwin':
 yield 'DYLD_LIBRARY_PATH'


Index: test/Unit/lit.cfg.py
===
--- test/Unit/lit.cfg.py
+++ test/Unit/lit.cfg.py
@@ -36,7 +36,7 @@
 config.environment[symbolizer] = os.environ[symbolizer]
 
 def find_shlibpath_var():
-if platform.system() in ['Linux', 'FreeBSD', 'NetBSD']:
+if platform.system() in ['Linux', 'FreeBSD', 'NetBSD', 'SunOS']:
 yield 'LD_LIBRARY_PATH'
 elif platform.system() == 'Darwin':
 yield 'DYLD_LIBRARY_PATH'
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D39641: [CodeGen] make cbrt and fma constant (never set errno)

2017-11-05 Thread Sanjay Patel via Phabricator via cfe-commits
spatel created this revision.
Herald added a subscriber: mcrosier.

Splitting cbrt and fma off from https://reviews.llvm.org/D39611, so we can deal 
with complex.h separately.

I've also gone ahead with a 'fix' for the fma case in 
CodeGenFunction::EmitBuiltinExpr(). But as noted previously, there's no test 
change because we were ignoring the const-ness of something that might have 
been non-const. Now, we're correctly checking that attribute even though 
there's no way it can be non-const. So if we change our minds about the fma 
decision, we'll do it in the right place. :)

Copying from https://reviews.llvm.org/D39611:

1. Cube root

We're going to dismiss POSIX language like this:
"On successful completion, cbrt() returns the cube root of x. If x is NaN, 
cbrt() returns NaN and errno may be set to [EDOM]. "
http://pubs.opengroup.org/onlinepubs/7908799/xsh/cbrt.html

And favor an interpretation based on the C standard that says:
"Functions with a NaN argument return a NaN result and raise no floating-point 
exception, except where stated otherwise."

2. Floating multiply-add

The C standard is silent?
http://pubs.opengroup.org/onlinepubs/9699919799/functions/fma.html - clearly 
says "...then errno shall be set..."
but:
https://linux.die.net/man/3/fma - "These functions do not set errno."

Let's opt for the - hopefully common - implementation where errno is not set. 
Note that there's no test change because as noted in the earlier discussion, 
we're ignoring the const attr in CodeGenFunction::EmitBuiltinExpr(). I'll look 
at what needs fixing in that function next.


https://reviews.llvm.org/D39641

Files:
  include/clang/Basic/Builtins.def
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGen/builtin-errno.c
  test/CodeGen/libcalls-errno.c

Index: test/CodeGen/libcalls-errno.c
===
--- test/CodeGen/libcalls-errno.c
+++ test/CodeGen/libcalls-errno.c
@@ -153,9 +153,9 @@
 // NO__ERRNO: declare double @cbrt(double) [[READNONE]]
 // NO__ERRNO: declare float @cbrtf(float) [[READNONE]]
 // NO__ERRNO: declare x86_fp80 @cbrtl(x86_fp80) [[READNONE]]
-// HAS_ERRNO: declare double @cbrt(double) [[NOT_READNONE]]
-// HAS_ERRNO: declare float @cbrtf(float) [[NOT_READNONE]]
-// HAS_ERRNO: declare x86_fp80 @cbrtl(x86_fp80) [[NOT_READNONE]]
+// HAS_ERRNO: declare double @cbrt(double) [[READNONE]]
+// HAS_ERRNO: declare float @cbrtf(float) [[READNONE]]
+// HAS_ERRNO: declare x86_fp80 @cbrtl(x86_fp80) [[READNONE]]
 
   ceil(f);   ceilf(f);  ceill(f);
 
Index: test/CodeGen/builtin-errno.c
===
--- test/CodeGen/builtin-errno.c
+++ test/CodeGen/builtin-errno.c
@@ -198,9 +198,9 @@
 // NO__ERRNO: declare double @cbrt(double) [[READNONE]]
 // NO__ERRNO: declare float @cbrtf(float) [[READNONE]]
 // NO__ERRNO: declare x86_fp80 @cbrtl(x86_fp80) [[READNONE]]
-// HAS_ERRNO: declare double @cbrt(double) [[NOT_READNONE]]
-// HAS_ERRNO: declare float @cbrtf(float) [[NOT_READNONE]]
-// HAS_ERRNO: declare x86_fp80 @cbrtl(x86_fp80) [[NOT_READNONE]]
+// HAS_ERRNO: declare double @cbrt(double) [[READNONE]]
+// HAS_ERRNO: declare float @cbrtf(float) [[READNONE]]
+// HAS_ERRNO: declare x86_fp80 @cbrtl(x86_fp80) [[READNONE]]
 
   __builtin_ceil(f);   __builtin_ceilf(f);  __builtin_ceill(f);
 
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -2109,15 +2109,11 @@
   case Builtin::BIfmal:
   case Builtin::BI__builtin_fma:
   case Builtin::BI__builtin_fmaf:
-  case Builtin::BI__builtin_fmal: {
-// Rewrite fma to intrinsic.
-Value *FirstArg = EmitScalarExpr(E->getArg(0));
-llvm::Type *ArgType = FirstArg->getType();
-Value *F = CGM.getIntrinsic(Intrinsic::fma, ArgType);
-return RValue::get(
-Builder.CreateCall(F, {FirstArg, EmitScalarExpr(E->getArg(1)),
-   EmitScalarExpr(E->getArg(2))}));
-  }
+  case Builtin::BI__builtin_fmal:
+// A constant libcall or builtin is equivalent to the LLVM intrinsic.
+if (FD->hasAttr())
+  return RValue::get(emitTernaryBuiltin(*this, E, Intrinsic::fma));
+break;
 
   case Builtin::BI__builtin_signbit:
   case Builtin::BI__builtin_signbitf:
Index: include/clang/Basic/Builtins.def
===
--- include/clang/Basic/Builtins.def
+++ include/clang/Basic/Builtins.def
@@ -165,9 +165,9 @@
 BUILTIN(__builtin_atanh , "dd", "Fne")
 BUILTIN(__builtin_atanhf, "ff", "Fne")
 BUILTIN(__builtin_atanhl, "LdLd", "Fne")
-BUILTIN(__builtin_cbrt , "dd", "Fne")
-BUILTIN(__builtin_cbrtf, "ff", "Fne")
-BUILTIN(__builtin_cbrtl, "LdLd", "Fne")
+BUILTIN(__builtin_cbrt , "dd", "Fnc")
+BUILTIN(__builtin_cbrtf, "ff", "Fnc")
+BUILTIN(__builtin_cbrtl, "LdLd", "Fnc")
 BUILTIN(__builtin_ceil , "dd"  , "Fnc")
 BUILTIN(__builtin_ceilf, "ff"  , "Fnc")
 BUILTIN(__builtin_ceill, "LdLd", "F

[PATCH] D39633: Remove \brief from doxygen comments in PrettyPrinter.h

2017-11-05 Thread Adrian Prantl via Phabricator via cfe-commits
aprantl accepted this revision.
aprantl added a comment.
This revision is now accepted and ready to land.

Thanks!


Repository:
  rL LLVM

https://reviews.llvm.org/D39633



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


[PATCH] D39640: [lit] Set shlibpath_var on Solaris

2017-11-05 Thread Rainer Orth via Phabricator via cfe-commits
ro added a comment.

Thanks.  Could someone please commit it for me?


https://reviews.llvm.org/D39640



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


[PATCH] D39644: [clang-diff] Use references to Node instead of NodeId

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision.
Herald added a subscriber: klimek.

This adds to each node a reference to its syntax tree. As a result,
instead of passing around the tree plus the node ID, we just use a
reference to the node. This removes some potential for errors. Users
will almost always use node references and are oblivious of their IDs.

Iterating through lists of Nodes is provided by NodeRefIterator


https://reviews.llvm.org/D39644

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  include/clang/Tooling/ASTDiff/ASTDiffInternal.h
  lib/Tooling/ASTDiff/ASTDiff.cpp
  tools/clang-diff/ClangDiff.cpp

Index: tools/clang-diff/ClangDiff.cpp
===
--- tools/clang-diff/ClangDiff.cpp
+++ tools/clang-diff/ClangDiff.cpp
@@ -265,30 +265,31 @@
 }
 
 static unsigned printHtmlForNode(raw_ostream &OS, const diff::ASTDiff &Diff,
- diff::SyntaxTree &Tree, bool IsLeft,
- diff::NodeId Id, unsigned Offset) {
-  const diff::Node &Node = Tree.getNode(Id);
+ bool IsLeft, const diff::Node &Node,
+ unsigned Offset) {
   char MyTag, OtherTag;
   diff::NodeId LeftId, RightId;
-  diff::NodeId TargetId = Diff.getMapped(Tree, Id);
+  diff::SyntaxTree &Tree = Node.getTree();
+  const diff::Node *Target = Diff.getMapped(Tree, Node);
+  diff::NodeId TargetId = Target ? Target->getId() : diff::NodeId();
   if (IsLeft) {
 MyTag = 'L';
 OtherTag = 'R';
-LeftId = Id;
+LeftId = Node.getId();
 RightId = TargetId;
   } else {
 MyTag = 'R';
 OtherTag = 'L';
 LeftId = TargetId;
-RightId = Id;
+RightId = Node.getId();
   }
   unsigned Begin, End;
   std::tie(Begin, End) = Tree.getSourceRangeOffsets(Node);
-  const SourceManager &SrcMgr = Tree.getASTContext().getSourceManager();
-  auto Code = SrcMgr.getBuffer(SrcMgr.getMainFileID())->getBuffer();
+  const SourceManager &SM = Tree.getASTContext().getSourceManager();
+  auto Code = SM.getBuffer(SM.getMainFileID())->getBuffer();
   for (; Offset < Begin; ++Offset)
 printHtml(OS, Code[Offset]);
-  OS << "";
 
-  for (diff::NodeId Child : Node.Children)
-Offset = printHtmlForNode(OS, Diff, Tree, IsLeft, Child, Offset);
+  for (const diff::Node &Child : Node)
+Offset = printHtmlForNode(OS, Diff, IsLeft, Child, Offset);
 
   for (; Offset < End; ++Offset)
 printHtml(OS, Code[Offset]);
-  if (Id == Tree.getRootId()) {
+  if (&Node == &Tree.getRoot()) {
 End = Code.size();
 for (; Offset < End; ++Offset)
   printHtml(OS, Code[Offset]);
@@ -343,28 +344,26 @@
 }
 
 static void printNodeAttributes(raw_ostream &OS, diff::SyntaxTree &Tree,
-diff::NodeId Id) {
-  const diff::Node &N = Tree.getNode(Id);
-  OS << R"("id":)" << int(Id);
-  OS << R"(,"type":")" << N.getTypeLabel() << '"';
-  auto Offsets = Tree.getSourceRangeOffsets(N);
+const diff::Node &Node) {
+  OS << R"("id":)" << int(Node.getId());
+  OS << R"(,"type":")" << Node.getTypeLabel() << '"';
+  auto Offsets = Tree.getSourceRangeOffsets(Node);
   OS << R"(,"begin":)" << Offsets.first;
   OS << R"(,"end":)" << Offsets.second;
-  std::string Value = Tree.getNodeValue(N);
+  std::string Value = Tree.getNodeValue(Node);
   if (!Value.empty()) {
 OS << R"(,"value":")";
 printJsonString(OS, Value);
 OS << '"';
   }
 }
 
 static void printNodeAsJson(raw_ostream &OS, diff::SyntaxTree &Tree,
-diff::NodeId Id) {
-  const diff::Node &N = Tree.getNode(Id);
+const diff::Node &Node) {
   OS << "{";
-  printNodeAttributes(OS, Tree, Id);
-  auto Identifier = N.getIdentifier();
-  auto QualifiedIdentifier = N.getQualifiedIdentifier();
+  printNodeAttributes(OS, Tree, Node);
+  auto Identifier = Node.getIdentifier();
+  auto QualifiedIdentifier = Node.getQualifiedIdentifier();
   if (Identifier) {
 OS << R"(,"identifier":")";
 printJsonString(OS, *Identifier);
@@ -376,66 +375,65 @@
 }
   }
   OS << R"(,"children":[)";
-  if (N.Children.size() > 0) {
-printNodeAsJson(OS, Tree, N.Children[0]);
-for (size_t I = 1, E = N.Children.size(); I < E; ++I) {
+  auto ChildBegin = Node.begin(), ChildEnd = Node.end();
+  if (ChildBegin != ChildEnd) {
+printNodeAsJson(OS, Tree, *ChildBegin);
+for (++ChildBegin; ChildBegin != ChildEnd; ++ChildBegin) {
   OS << ",";
-  printNodeAsJson(OS, Tree, N.Children[I]);
+  printNodeAsJson(OS, Tree, *ChildBegin);
 }
   }
   OS << "]}";
 }
 
 static void printNode(raw_ostream &OS, diff::SyntaxTree &Tree,
-  diff::NodeId Id) {
-  if (Id.isInvalid()) {
-OS << "None";
-return;
-  }
-  OS << Tree.getNode(Id).getTypeLabel();
-  std::string Value = Tree.getNodeValue(Id);
+  const diff::Node &Node) {
+  OS << Node.getTypeLabel();
+  std::string Value = Tree.getNodeValue(Node);

[PATCH] D39645: [clang-diff] NFC: replace const Node & with a typedef'd NodeRef

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision.
Herald added a subscriber: klimek.

https://reviews.llvm.org/D39645

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  lib/Tooling/ASTDiff/ASTDiff.cpp
  tools/clang-diff/ClangDiff.cpp

Index: tools/clang-diff/ClangDiff.cpp
===
--- tools/clang-diff/ClangDiff.cpp
+++ tools/clang-diff/ClangDiff.cpp
@@ -265,7 +265,7 @@
 }
 
 static unsigned printHtmlForNode(raw_ostream &OS, const diff::ASTDiff &Diff,
- bool IsLeft, const diff::Node &Node,
+ bool IsLeft,  diff::NodeRef Node,
  unsigned Offset) {
   char MyTag, OtherTag;
   diff::NodeId LeftId, RightId;
@@ -304,7 +304,7 @@
 OS << " class='" << getChangeKindAbbr(Node.Change) << "'";
   OS << ">";
 
-  for (const diff::Node &Child : Node)
+  for (diff::NodeRef Child : Node)
 Offset = printHtmlForNode(OS, Diff, IsLeft, Child, Offset);
 
   for (; Offset < End; ++Offset)
@@ -344,7 +344,7 @@
 }
 
 static void printNodeAttributes(raw_ostream &OS, diff::SyntaxTree &Tree,
-const diff::Node &Node) {
+diff::NodeRef Node) {
   OS << R"("id":)" << int(Node.getId());
   OS << R"(,"type":")" << Node.getTypeLabel() << '"';
   auto Offsets = Tree.getSourceRangeOffsets(Node);
@@ -359,7 +359,7 @@
 }
 
 static void printNodeAsJson(raw_ostream &OS, diff::SyntaxTree &Tree,
-const diff::Node &Node) {
+diff::NodeRef Node) {
   OS << "{";
   printNodeAttributes(OS, Tree, Node);
   auto Identifier = Node.getIdentifier();
@@ -387,16 +387,16 @@
 }
 
 static void printNode(raw_ostream &OS, diff::SyntaxTree &Tree,
-  const diff::Node &Node) {
+  diff::NodeRef Node) {
   OS << Node.getTypeLabel();
   std::string Value = Tree.getNodeValue(Node);
   if (!Value.empty())
 OS << ": " << Value;
   OS << "(" << Node.getId() << ")";
 }
 
 static void printTree(raw_ostream &OS, diff::SyntaxTree &Tree) {
-  for (const diff::Node &Node : Tree) {
+  for (diff::NodeRef Node : Tree) {
 for (int I = 0; I < Node.Depth; ++I)
   OS << " ";
 printNode(OS, Tree, Node);
@@ -406,7 +406,7 @@
 
 static void printDstChange(raw_ostream &OS, diff::ASTDiff &Diff,
diff::SyntaxTree &SrcTree, diff::SyntaxTree &DstTree,
-   const diff::Node &Dst) {
+   diff::NodeRef Dst) {
   const diff::Node *Src = Diff.getMapped(DstTree, Dst);
   switch (Dst.Change) {
   case diff::None:
@@ -511,7 +511,7 @@
 return 0;
   }
 
-  for (const diff::Node &Dst : DstTree) {
+  for (diff::NodeRef Dst : DstTree) {
 const diff::Node *Src = Diff.getMapped(DstTree, Dst);
 if (PrintMatches && Src) {
   llvm::outs() << "Match ";
@@ -522,7 +522,7 @@
 }
 printDstChange(llvm::outs(), Diff, SrcTree, DstTree, Dst);
   }
-  for (const diff::Node &Src : SrcTree) {
+  for (diff::NodeRef Src : SrcTree) {
 if (!Diff.getMapped(SrcTree, Src)) {
   llvm::outs() << "Delete ";
   printNode(llvm::outs(), SrcTree, Src);
Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -69,29 +69,28 @@
   void computeChangeKinds(Mapping &M);
 
   const Node *getMapped(const std::unique_ptr &Tree,
-const Node &N) const;
+NodeRef N) const;
 
 private:
   // Returns true if the two subtrees are identical.
-  bool identical(const Node &N1, const Node &N2) const;
+  bool identical(NodeRef N1, NodeRef N2) const;
 
   // Returns false if the nodes must not be mached.
-  bool isMatchingPossible(const Node &N1, const Node &N2) const;
+  bool isMatchingPossible(NodeRef N1, NodeRef N2) const;
 
   // Returns true if the nodes' parents are matched.
-  bool haveSameParents(const Mapping &M, const Node &N1, const Node &N2) const;
+  bool haveSameParents(const Mapping &M, NodeRef N1, NodeRef N2) const;
 
   // Uses an optimal albeit slow algorithm to compute a mapping between two
   // subtrees, but only if both have fewer nodes than MaxSize.
-  void addOptimalMapping(Mapping &M, const Node &N1, const Node &N2) const;
+  void addOptimalMapping(Mapping &M, NodeRef N1, NodeRef N2) const;
 
   // Computes the ratio of common descendants between the two nodes.
   // Descendants are only considered to be equal when they are mapped.
-  double getJaccardSimilarity(const Mapping &M, const Node &N1,
-  const Node &N2) const;
+  double getJaccardSimilarity(const Mapping &M, NodeRef N1, NodeRef N2) const;
 
   // Returns the node that has the highest degree of similarity.
-  const Node *findCandidate(const Mapping &M, const Node &N1) const;
+  const Node *findCandidate(const Mapping &M, NodeRef N1) const;
 
   // Returns a map

[PATCH] D39646: [clang-diff] Rename ChangeKind::None to NoChange

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision.
Herald added a subscriber: klimek.

https://reviews.llvm.org/D39646

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  tools/clang-diff/ClangDiff.cpp


Index: tools/clang-diff/ClangDiff.cpp
===
--- tools/clang-diff/ClangDiff.cpp
+++ tools/clang-diff/ClangDiff.cpp
@@ -248,7 +248,7 @@
 
 static std::string getChangeKindAbbr(diff::ChangeKind Kind) {
   switch (Kind) {
-  case diff::None:
+  case diff::NoChange:
 return "";
   case diff::Delete:
 return "d";
@@ -300,7 +300,7 @@
 printHtml(OS, Value);
   }
   OS << "'";
-  if (Node.Change != diff::None)
+  if (Node.Change != diff::NoChange)
 OS << " class='" << getChangeKindAbbr(Node.Change) << "'";
   OS << ">";
 
@@ -409,7 +409,7 @@
diff::NodeRef Dst) {
   const diff::Node *Src = Diff.getMapped(DstTree, Dst);
   switch (Dst.Change) {
-  case diff::None:
+  case diff::NoChange:
 break;
   case diff::Delete:
 llvm_unreachable("The destination tree can't have deletions.");
Index: include/clang/Tooling/ASTDiff/ASTDiff.h
===
--- include/clang/Tooling/ASTDiff/ASTDiff.h
+++ include/clang/Tooling/ASTDiff/ASTDiff.h
@@ -26,7 +26,7 @@
 namespace diff {
 
 enum ChangeKind {
-  None,
+  NoChange,
   Delete,// (Src): delete node Src.
   Update,// (Src, Dst): update the value of node Src to match Dst.
   Insert,// (Src, Dst, Pos): insert Src as child of Dst at offset Pos.
@@ -95,7 +95,7 @@
   int Depth, Height, Shift = 0;
   ast_type_traits::DynTypedNode ASTNode;
   SmallVector Children;
-  ChangeKind Change = None;
+  ChangeKind Change = NoChange;
   Node(SyntaxTree::Impl &Tree) : Tree(Tree), Children() {}
 
   NodeId getId() const;


Index: tools/clang-diff/ClangDiff.cpp
===
--- tools/clang-diff/ClangDiff.cpp
+++ tools/clang-diff/ClangDiff.cpp
@@ -248,7 +248,7 @@
 
 static std::string getChangeKindAbbr(diff::ChangeKind Kind) {
   switch (Kind) {
-  case diff::None:
+  case diff::NoChange:
 return "";
   case diff::Delete:
 return "d";
@@ -300,7 +300,7 @@
 printHtml(OS, Value);
   }
   OS << "'";
-  if (Node.Change != diff::None)
+  if (Node.Change != diff::NoChange)
 OS << " class='" << getChangeKindAbbr(Node.Change) << "'";
   OS << ">";
 
@@ -409,7 +409,7 @@
diff::NodeRef Dst) {
   const diff::Node *Src = Diff.getMapped(DstTree, Dst);
   switch (Dst.Change) {
-  case diff::None:
+  case diff::NoChange:
 break;
   case diff::Delete:
 llvm_unreachable("The destination tree can't have deletions.");
Index: include/clang/Tooling/ASTDiff/ASTDiff.h
===
--- include/clang/Tooling/ASTDiff/ASTDiff.h
+++ include/clang/Tooling/ASTDiff/ASTDiff.h
@@ -26,7 +26,7 @@
 namespace diff {
 
 enum ChangeKind {
-  None,
+  NoChange,
   Delete,// (Src): delete node Src.
   Update,// (Src, Dst): update the value of node Src to match Dst.
   Insert,// (Src, Dst, Pos): insert Src as child of Dst at offset Pos.
@@ -95,7 +95,7 @@
   int Depth, Height, Shift = 0;
   ast_type_traits::DynTypedNode ASTNode;
   SmallVector Children;
-  ChangeKind Change = None;
+  ChangeKind Change = NoChange;
   Node(SyntaxTree::Impl &Tree) : Tree(Tree), Children() {}
 
   NodeId getId() const;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D39647: [clang-diff] NFC: remove Mapping

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision.
Herald added a subscriber: klimek.

Store it within ASTDiff::Impl instead.


https://reviews.llvm.org/D39647

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  lib/Tooling/ASTDiff/ASTDiff.cpp
  tools/clang-diff/ClangDiff.cpp

Index: tools/clang-diff/ClangDiff.cpp
===
--- tools/clang-diff/ClangDiff.cpp
+++ tools/clang-diff/ClangDiff.cpp
@@ -270,7 +270,7 @@
   char MyTag, OtherTag;
   diff::NodeId LeftId, RightId;
   diff::SyntaxTree &Tree = Node.getTree();
-  const diff::Node *Target = Diff.getMapped(Tree, Node);
+  const diff::Node *Target = Diff.getMapped(Node);
   diff::NodeId TargetId = Target ? Target->getId() : diff::NodeId();
   if (IsLeft) {
 MyTag = 'L';
@@ -407,7 +407,7 @@
 static void printDstChange(raw_ostream &OS, diff::ASTDiff &Diff,
diff::SyntaxTree &SrcTree, diff::SyntaxTree &DstTree,
diff::NodeRef Dst) {
-  const diff::Node *Src = Diff.getMapped(DstTree, Dst);
+  const diff::Node *Src = Diff.getMapped(Dst);
   switch (Dst.Change) {
   case diff::NoChange:
 break;
@@ -512,7 +512,7 @@
   }
 
   for (diff::NodeRef Dst : DstTree) {
-const diff::Node *Src = Diff.getMapped(DstTree, Dst);
+const diff::Node *Src = Diff.getMapped(Dst);
 if (PrintMatches && Src) {
   llvm::outs() << "Match ";
   printNode(llvm::outs(), SrcTree, *Src);
@@ -523,7 +523,7 @@
 printDstChange(llvm::outs(), Diff, SrcTree, DstTree, Dst);
   }
   for (diff::NodeRef Src : SrcTree) {
-if (!Diff.getMapped(SrcTree, Src)) {
+if (!Diff.getMapped(Src)) {
   llvm::outs() << "Delete ";
   printNode(llvm::outs(), SrcTree, Src);
   llvm::outs() << "\n";
Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -27,76 +27,57 @@
 namespace clang {
 namespace diff {
 
-namespace {
-/// Maps nodes of the left tree to ones on the right, and vice versa.
-class Mapping {
-public:
-  Mapping() = default;
-  Mapping(Mapping &&Other) = default;
-  Mapping &operator=(Mapping &&Other) = default;
-
-  Mapping(size_t Size) {
-SrcToDst = llvm::make_unique(Size);
-DstToSrc = llvm::make_unique(Size);
-  }
-
-  void link(NodeId Src, NodeId Dst) {
-SrcToDst[Src] = Dst, DstToSrc[Dst] = Src;
-  }
-
-  NodeId getDst(NodeId Src) const { return SrcToDst[Src]; }
-  NodeId getSrc(NodeId Dst) const { return DstToSrc[Dst]; }
-  bool hasSrc(NodeId Src) const { return getDst(Src).isValid(); }
-  bool hasDst(NodeId Dst) const { return getSrc(Dst).isValid(); }
-
+class ASTDiff::Impl {
 private:
   std::unique_ptr SrcToDst, DstToSrc;
-};
-} // end anonymous namespace
 
-class ASTDiff::Impl {
 public:
   SyntaxTree::Impl &T1, &T2;
-  Mapping TheMapping;
 
   Impl(SyntaxTree::Impl &T1, SyntaxTree::Impl &T2,
const ComparisonOptions &Options);
 
   /// Matches nodes one-by-one based on their similarity.
   void computeMapping();
 
   // Compute Change for each node based on similarity.
-  void computeChangeKinds(Mapping &M);
+  void computeChangeKinds();
 
-  const Node *getMapped(const std::unique_ptr &Tree,
-NodeRef N) const;
+  const Node *getMapped(NodeRef N) const;
 
 private:
+  // Adds a mapping between two nodes.
+  void link(NodeRef N1, NodeRef N2);
+
+  // Returns the mapped node, or null.
+  const Node *getDst(NodeRef N1) const;
+  const Node *getSrc(NodeRef N2) const;
+
   // Returns true if the two subtrees are identical.
   bool identical(NodeRef N1, NodeRef N2) const;
 
   // Returns false if the nodes must not be mached.
   bool isMatchingPossible(NodeRef N1, NodeRef N2) const;
 
   // Returns true if the nodes' parents are matched.
-  bool haveSameParents(const Mapping &M, NodeRef N1, NodeRef N2) const;
+  bool haveSameParents(NodeRef N1, NodeRef N2) const;
 
   // Uses an optimal albeit slow algorithm to compute a mapping between two
   // subtrees, but only if both have fewer nodes than MaxSize.
-  void addOptimalMapping(Mapping &M, NodeRef N1, NodeRef N2) const;
+  void addOptimalMapping(NodeRef N1, NodeRef N2);
 
   // Computes the ratio of common descendants between the two nodes.
   // Descendants are only considered to be equal when they are mapped.
-  double getJaccardSimilarity(const Mapping &M, NodeRef N1, NodeRef N2) const;
+  double getJaccardSimilarity(NodeRef N1, NodeRef N2) const;
 
   // Returns the node that has the highest degree of similarity.
-  const Node *findCandidate(const Mapping &M, NodeRef N1) const;
+  const Node *findCandidate(NodeRef N1) const;
 
   // Returns a mapping of identical subtrees.
-  Mapping matchTopDown();
+  void matchTopDown();
 
   // Tries to match any yet unmapped nodes, in a bottom-up fashion.
-  void matchBottomUp(Mapping &M) const;
+  void matchBottomUp();
 
   const ComparisonOptions &Options;
 
@@ -803,34 +784,31 @@
   return Options.isMa

[PATCH] D39648: [clang-diff] NFC: organise header

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision.
Herald added a subscriber: klimek.

https://reviews.llvm.org/D39648

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  lib/Tooling/ASTDiff/ASTDiff.cpp


Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -27,6 +27,10 @@
 namespace clang {
 namespace diff {
 
+bool ComparisonOptions::isMatchingAllowed(NodeRef N1, NodeRef N2) const {
+  return N1.getType().isSame(N2.getType());
+}
+
 class ASTDiff::Impl {
 private:
   std::unique_ptr SrcToDst, DstToSrc;
Index: include/clang/Tooling/ASTDiff/ASTDiff.h
===
--- include/clang/Tooling/ASTDiff/ASTDiff.h
+++ include/clang/Tooling/ASTDiff/ASTDiff.h
@@ -36,6 +36,24 @@
 
 using NodeRef = const Node &;
 
+struct ComparisonOptions {
+  /// During top-down matching, only consider nodes of at least this height.
+  int MinHeight = 2;
+
+  /// During bottom-up matching, match only nodes with at least this value as
+  /// the ratio of their common descendants.
+  double MinSimilarity = 0.5;
+
+  /// Whenever two subtrees are matched in the bottom-up phase, the optimal
+  /// mapping is computed, unless the size of either subtrees exceeds this.
+  int MaxSize = 100;
+
+  bool StopAfterTopDown = false;
+
+  /// Returns false if the nodes should never be matched.
+  bool isMatchingAllowed(NodeRef N1, NodeRef N2) const;
+};
+
 class ASTDiff {
 public:
   ASTDiff(SyntaxTree &Src, SyntaxTree &Dst, const ComparisonOptions &Options);
@@ -123,26 +141,6 @@
   bool operator!=(const NodeRefIterator &Other) const;
 };
 
-struct ComparisonOptions {
-  /// During top-down matching, only consider nodes of at least this height.
-  int MinHeight = 2;
-
-  /// During bottom-up matching, match only nodes with at least this value as
-  /// the ratio of their common descendants.
-  double MinSimilarity = 0.5;
-
-  /// Whenever two subtrees are matched in the bottom-up phase, the optimal
-  /// mapping is computed, unless the size of either subtrees exceeds this.
-  int MaxSize = 100;
-
-  bool StopAfterTopDown = false;
-
-  /// Returns false if the nodes should never be matched.
-  bool isMatchingAllowed(NodeRef N1, NodeRef N2) const {
-return N1.getType().isSame(N2.getType());
-  }
-};
-
 } // end namespace diff
 } // end namespace clang
 


Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -27,6 +27,10 @@
 namespace clang {
 namespace diff {
 
+bool ComparisonOptions::isMatchingAllowed(NodeRef N1, NodeRef N2) const {
+  return N1.getType().isSame(N2.getType());
+}
+
 class ASTDiff::Impl {
 private:
   std::unique_ptr SrcToDst, DstToSrc;
Index: include/clang/Tooling/ASTDiff/ASTDiff.h
===
--- include/clang/Tooling/ASTDiff/ASTDiff.h
+++ include/clang/Tooling/ASTDiff/ASTDiff.h
@@ -36,6 +36,24 @@
 
 using NodeRef = const Node &;
 
+struct ComparisonOptions {
+  /// During top-down matching, only consider nodes of at least this height.
+  int MinHeight = 2;
+
+  /// During bottom-up matching, match only nodes with at least this value as
+  /// the ratio of their common descendants.
+  double MinSimilarity = 0.5;
+
+  /// Whenever two subtrees are matched in the bottom-up phase, the optimal
+  /// mapping is computed, unless the size of either subtrees exceeds this.
+  int MaxSize = 100;
+
+  bool StopAfterTopDown = false;
+
+  /// Returns false if the nodes should never be matched.
+  bool isMatchingAllowed(NodeRef N1, NodeRef N2) const;
+};
+
 class ASTDiff {
 public:
   ASTDiff(SyntaxTree &Src, SyntaxTree &Dst, const ComparisonOptions &Options);
@@ -123,26 +141,6 @@
   bool operator!=(const NodeRefIterator &Other) const;
 };
 
-struct ComparisonOptions {
-  /// During top-down matching, only consider nodes of at least this height.
-  int MinHeight = 2;
-
-  /// During bottom-up matching, match only nodes with at least this value as
-  /// the ratio of their common descendants.
-  double MinSimilarity = 0.5;
-
-  /// Whenever two subtrees are matched in the bottom-up phase, the optimal
-  /// mapping is computed, unless the size of either subtrees exceeds this.
-  int MaxSize = 100;
-
-  bool StopAfterTopDown = false;
-
-  /// Returns false if the nodes should never be matched.
-  bool isMatchingAllowed(NodeRef N1, NodeRef N2) const {
-return N1.getType().isSame(N2.getType());
-  }
-};
-
 } // end namespace diff
 } // end namespace clang
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D39649: [clang-diff] Split findPositionInParent

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision.
Herald added a subscriber: klimek.

This makes findPositionInParent() a member of Node.  Additionally, the
function that computes the new position that is dependent on
insertions and deletions of siblings, is now a member of
ASTDiff::Impl.


https://reviews.llvm.org/D39649

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  lib/Tooling/ASTDiff/ASTDiff.cpp
  tools/clang-diff/ClangDiff.cpp

Index: tools/clang-diff/ClangDiff.cpp
===
--- tools/clang-diff/ClangDiff.cpp
+++ tools/clang-diff/ClangDiff.cpp
@@ -434,7 +434,7 @@
   OS << "None";
 else
   printNode(OS, DstTree, *Dst.getParent());
-OS << " at " << DstTree.findPositionInParent(Dst) << "\n";
+OS << " at " << Dst.findPositionInParent() << "\n";
 break;
   }
 }
Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -83,6 +83,8 @@
   // Tries to match any yet unmapped nodes, in a bottom-up fashion.
   void matchBottomUp();
 
+  int findNewPosition(NodeRef N) const;
+
   const ComparisonOptions &Options;
 
   friend class ZhangShashaMatcher;
@@ -141,7 +143,6 @@
   Node &getMutableNode(NodeRef N) { return getMutableNode(N.getId()); }
   int getNumberOfDescendants(NodeRef N) const;
   bool isInSubtree(NodeRef N, NodeRef SubtreeRoot) const;
-  int findPositionInParent(NodeRef Id, bool Shifted = false) const;
 
   std::string getRelativeName(const NamedDecl *ND,
   const DeclContext *Context) const;
@@ -339,23 +340,6 @@
  N.getId() <= SubtreeRoot.RightMostDescendant;
 }
 
-int SyntaxTree::Impl::findPositionInParent(NodeRef N, bool Shifted) const {
-  if (!N.getParent())
-return 0;
-  NodeRef Parent = *N.getParent();
-  const auto &Siblings = Parent.Children;
-  int Position = 0;
-  for (size_t I = 0, E = Siblings.size(); I < E; ++I) {
-if (Shifted)
-  Position += getNode(Siblings[I]).Shift;
-if (Siblings[I] == N.getId()) {
-  Position += I;
-  return Position;
-}
-  }
-  llvm_unreachable("Node not found in parent's children.");
-}
-
 // Returns the qualified name of ND. If it is subordinate to Context,
 // then the prefix of the latter is removed from the returned value.
 std::string
@@ -724,6 +708,14 @@
   return {&Tree, isLeaf() ? nullptr : &Children[0] + Children.size()};
 }
 
+int Node::findPositionInParent() const {
+  if (!getParent())
+return 0;
+  const ArrayRef &Siblings = getParent()->Children;
+  return std::find(Siblings.begin(), Siblings.end(), getId()) -
+ Siblings.begin();
+}
+
 namespace {
 // Compares nodes by their depth.
 struct HeightLess {
@@ -950,8 +942,8 @@
 if (!getDst(N1))
   continue;
 NodeRef N2 = *getDst(N1);
-if (!haveSameParents(N1, N2) || T1.findPositionInParent(N1, true) !=
-T2.findPositionInParent(N2, true)) {
+if (!haveSameParents(N1, N2) ||
+findNewPosition(N1) != findNewPosition(N2)) {
   T1.getMutableNode(N1).Shift -= 1;
   T2.getMutableNode(N2).Shift -= 1;
 }
@@ -962,8 +954,8 @@
 NodeRef N1 = *getSrc(N2);
 Node &MutableN1 = T1.getMutableNode(N1);
 Node &MutableN2 = T2.getMutableNode(N2);
-if (!haveSameParents(N1, N2) || T1.findPositionInParent(N1, true) !=
-T2.findPositionInParent(N2, true)) {
+if (!haveSameParents(N1, N2) ||
+findNewPosition(N1) != findNewPosition(N2)) {
   MutableN1.Change = MutableN2.Change = Move;
 }
 if (T1.getNodeValue(N1) != T2.getNodeValue(N2)) {
@@ -995,6 +987,18 @@
 : nullptr;
 }
 
+int ASTDiff::Impl::findNewPosition(NodeRef N) const {
+  if (!N.getParent())
+return 0;
+  int Position = N.findPositionInParent();
+  for (NodeRef Sibling : *N.getParent()) {
+Position += Sibling.Shift;
+if (&Sibling == &N)
+  return Position;
+  }
+  llvm_unreachable("Node not found amongst parent's children.");
+}
+
 ASTDiff::ASTDiff(SyntaxTree &T1, SyntaxTree &T2,
  const ComparisonOptions &Options)
 : DiffImpl(llvm::make_unique(*T1.TreeImpl, *T2.TreeImpl, Options)) {}
@@ -1022,10 +1026,6 @@
 }
 SyntaxTree::PreorderIterator SyntaxTree::end() const { return TreeImpl->end(); }
 
-int SyntaxTree::findPositionInParent(NodeRef N) const {
-  return TreeImpl->findPositionInParent(N);
-}
-
 std::pair
 SyntaxTree::getSourceRangeOffsets(NodeRef N) const {
   const SourceManager &SM = TreeImpl->AST.getSourceManager();
Index: include/clang/Tooling/ASTDiff/ASTDiff.h
===
--- include/clang/Tooling/ASTDiff/ASTDiff.h
+++ include/clang/Tooling/ASTDiff/ASTDiff.h
@@ -90,7 +90,6 @@
   PreorderIterator end() const;
 
   NodeRef getNode(NodeId Id) const;
-  int findPositionInParent(NodeRef Node) const;
 
   // Returns the start

[PATCH] D39650: [clang-diff] Make getSourceRangeOffsets a member of Node

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision.
Herald added a subscriber: klimek.

https://reviews.llvm.org/D39650

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  lib/Tooling/ASTDiff/ASTDiff.cpp
  tools/clang-diff/ClangDiff.cpp


Index: tools/clang-diff/ClangDiff.cpp
===
--- tools/clang-diff/ClangDiff.cpp
+++ tools/clang-diff/ClangDiff.cpp
@@ -284,7 +284,7 @@
 RightId = Node.getId();
   }
   unsigned Begin, End;
-  std::tie(Begin, End) = Tree.getSourceRangeOffsets(Node);
+  std::tie(Begin, End) = Node.getSourceRangeOffsets();
   const SourceManager &SM = Tree.getASTContext().getSourceManager();
   auto Code = SM.getBuffer(SM.getMainFileID())->getBuffer();
   for (; Offset < Begin; ++Offset)
@@ -347,7 +347,7 @@
 diff::NodeRef Node) {
   OS << R"("id":)" << int(Node.getId());
   OS << R"(,"type":")" << Node.getTypeLabel() << '"';
-  auto Offsets = Tree.getSourceRangeOffsets(Node);
+  auto Offsets = Node.getSourceRangeOffsets();
   OS << R"(,"begin":)" << Offsets.first;
   OS << R"(,"end":)" << Offsets.second;
   std::string Value = Tree.getNodeValue(Node);
Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -716,6 +716,21 @@
  Siblings.begin();
 }
 
+std::pair Node::getSourceRangeOffsets() const {
+  const SourceManager &SM = Tree.AST.getSourceManager();
+  SourceRange Range = ASTNode.getSourceRange();
+  SourceLocation BeginLoc = Range.getBegin();
+  SourceLocation EndLoc = Lexer::getLocForEndOfToken(
+  Range.getEnd(), /*Offset=*/0, SM, Tree.AST.getLangOpts());
+  if (auto *ThisExpr = ASTNode.get()) {
+if (ThisExpr->isImplicit())
+  EndLoc = BeginLoc;
+  }
+  unsigned Begin = SM.getFileOffset(SM.getExpansionLoc(BeginLoc));
+  unsigned End = SM.getFileOffset(SM.getExpansionLoc(EndLoc));
+  return {Begin, End};
+}
+
 namespace {
 // Compares nodes by their depth.
 struct HeightLess {
@@ -1026,22 +1041,6 @@
 }
 SyntaxTree::PreorderIterator SyntaxTree::end() const { return TreeImpl->end(); 
}
 
-std::pair
-SyntaxTree::getSourceRangeOffsets(NodeRef N) const {
-  const SourceManager &SM = TreeImpl->AST.getSourceManager();
-  SourceRange Range = N.ASTNode.getSourceRange();
-  SourceLocation BeginLoc = Range.getBegin();
-  SourceLocation EndLoc = Lexer::getLocForEndOfToken(
-  Range.getEnd(), /*Offset=*/0, SM, TreeImpl->AST.getLangOpts());
-  if (auto *ThisExpr = N.ASTNode.get()) {
-if (ThisExpr->isImplicit())
-  EndLoc = BeginLoc;
-  }
-  unsigned Begin = SM.getFileOffset(SM.getExpansionLoc(BeginLoc));
-  unsigned End = SM.getFileOffset(SM.getExpansionLoc(EndLoc));
-  return {Begin, End};
-}
-
 std::string SyntaxTree::getNodeValue(NodeRef N) const {
   return TreeImpl->getNodeValue(N);
 }
Index: include/clang/Tooling/ASTDiff/ASTDiff.h
===
--- include/clang/Tooling/ASTDiff/ASTDiff.h
+++ include/clang/Tooling/ASTDiff/ASTDiff.h
@@ -91,9 +91,6 @@
 
   NodeRef getNode(NodeId Id) const;
 
-  // Returns the starting and ending offset of the node in its source file.
-  std::pair getSourceRangeOffsets(NodeRef N) const;
-
   /// Serialize the node attributes to a string representation. This should
   /// uniquely distinguish nodes of the same kind. Note that this function
   /// just
@@ -129,6 +126,9 @@
   NodeRefIterator end() const;
 
   int findPositionInParent() const;
+
+  // Returns the starting and ending offset of the node in its source file.
+  std::pair getSourceRangeOffsets() const;
 };
 
 struct NodeRefIterator {


Index: tools/clang-diff/ClangDiff.cpp
===
--- tools/clang-diff/ClangDiff.cpp
+++ tools/clang-diff/ClangDiff.cpp
@@ -284,7 +284,7 @@
 RightId = Node.getId();
   }
   unsigned Begin, End;
-  std::tie(Begin, End) = Tree.getSourceRangeOffsets(Node);
+  std::tie(Begin, End) = Node.getSourceRangeOffsets();
   const SourceManager &SM = Tree.getASTContext().getSourceManager();
   auto Code = SM.getBuffer(SM.getMainFileID())->getBuffer();
   for (; Offset < Begin; ++Offset)
@@ -347,7 +347,7 @@
 diff::NodeRef Node) {
   OS << R"("id":)" << int(Node.getId());
   OS << R"(,"type":")" << Node.getTypeLabel() << '"';
-  auto Offsets = Tree.getSourceRangeOffsets(Node);
+  auto Offsets = Node.getSourceRangeOffsets();
   OS << R"(,"begin":)" << Offsets.first;
   OS << R"(,"end":)" << Offsets.second;
   std::string Value = Tree.getNodeValue(Node);
Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -716,6 +716,21 @@
  Siblings.begin();
 }
 
+std::pair Node::getSourceRangeOffsets() const {
+  const SourceManager &SM = Tree.AST.getSourceManager();
+  SourceRange Range = ASTNode.

[PATCH] D39651: [clang-diff] NFC: renames, moves

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision.
Herald added a subscriber: klimek.

https://reviews.llvm.org/D39651

Files:
  lib/Tooling/ASTDiff/ASTDiff.cpp


Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -141,8 +141,6 @@
   NodeRef getNode(NodeId Id) const { return Nodes[Id]; }
   Node &getMutableNode(NodeId Id) { return Nodes[Id]; }
   Node &getMutableNode(NodeRef N) { return getMutableNode(N.getId()); }
-  int getNumberOfDescendants(NodeRef N) const;
-  bool isInSubtree(NodeRef N, NodeRef SubtreeRoot) const;
 
   std::string getRelativeName(const NamedDecl *ND,
   const DeclContext *Context) const;
@@ -331,11 +329,11 @@
   }
 }
 
-int SyntaxTree::Impl::getNumberOfDescendants(NodeRef N) const {
+static int getNumberOfDescendants(NodeRef N) {
   return N.RightMostDescendant - N.getId() + 1;
 }
 
-bool SyntaxTree::Impl::isInSubtree(NodeRef N, NodeRef SubtreeRoot) const {
+static bool isInSubtree(NodeRef N, NodeRef SubtreeRoot) {
   return N.getId() >= SubtreeRoot.getId() &&
  N.getId() <= SubtreeRoot.RightMostDescendant;
 }
@@ -802,7 +800,7 @@
 }
 
 void ASTDiff::Impl::addOptimalMapping(NodeRef N1, NodeRef N2) {
-  if (std::max(T1.getNumberOfDescendants(N1), T2.getNumberOfDescendants(N2)) >
+  if (std::max(getNumberOfDescendants(N1), getNumberOfDescendants(N2)) >
   Options.MaxSize)
 return;
   ZhangShashaMatcher Matcher(*this, T1, T2, N1.getId(), N2.getId());
@@ -821,12 +819,12 @@
   for (NodeId Src = N1.getId() + 1; Src <= N1.RightMostDescendant; ++Src) {
 const Node *Dst = getDst(T1.getNode(Src));
 if (Dst)
-  CommonDescendants += T2.isInSubtree(*Dst, N2);
+  CommonDescendants += isInSubtree(*Dst, N2);
   }
   // We need to subtract 1 to get the number of descendants excluding the
   // root.
-  double Denominator = T1.getNumberOfDescendants(N1) - 1 +
-   T2.getNumberOfDescendants(N2) - 1 - CommonDescendants;
+  double Denominator = getNumberOfDescendants(N1) - 1 +
+   getNumberOfDescendants(N2) - 1 - CommonDescendants;
   // CommonDescendants is less than the size of one subtree.
   assert(Denominator >= 0 && "Expected non-negative denominator.");
   if (Denominator == 0)
@@ -906,7 +904,7 @@
 for (NodeRef N1 : H1) {
   for (NodeRef N2 : H2) {
 if (identical(N1, N2) && !getDst(N1) && !getSrc(N2)) {
-  for (int I = 0, E = T1.getNumberOfDescendants(N1); I < E; ++I) {
+  for (int I = 0, E = getNumberOfDescendants(N1); I < E; ++I) {
 link(T1.getNode(N1.getId() + I), T2.getNode(N2.getId() + I));
   }
 }


Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -141,8 +141,6 @@
   NodeRef getNode(NodeId Id) const { return Nodes[Id]; }
   Node &getMutableNode(NodeId Id) { return Nodes[Id]; }
   Node &getMutableNode(NodeRef N) { return getMutableNode(N.getId()); }
-  int getNumberOfDescendants(NodeRef N) const;
-  bool isInSubtree(NodeRef N, NodeRef SubtreeRoot) const;
 
   std::string getRelativeName(const NamedDecl *ND,
   const DeclContext *Context) const;
@@ -331,11 +329,11 @@
   }
 }
 
-int SyntaxTree::Impl::getNumberOfDescendants(NodeRef N) const {
+static int getNumberOfDescendants(NodeRef N) {
   return N.RightMostDescendant - N.getId() + 1;
 }
 
-bool SyntaxTree::Impl::isInSubtree(NodeRef N, NodeRef SubtreeRoot) const {
+static bool isInSubtree(NodeRef N, NodeRef SubtreeRoot) {
   return N.getId() >= SubtreeRoot.getId() &&
  N.getId() <= SubtreeRoot.RightMostDescendant;
 }
@@ -802,7 +800,7 @@
 }
 
 void ASTDiff::Impl::addOptimalMapping(NodeRef N1, NodeRef N2) {
-  if (std::max(T1.getNumberOfDescendants(N1), T2.getNumberOfDescendants(N2)) >
+  if (std::max(getNumberOfDescendants(N1), getNumberOfDescendants(N2)) >
   Options.MaxSize)
 return;
   ZhangShashaMatcher Matcher(*this, T1, T2, N1.getId(), N2.getId());
@@ -821,12 +819,12 @@
   for (NodeId Src = N1.getId() + 1; Src <= N1.RightMostDescendant; ++Src) {
 const Node *Dst = getDst(T1.getNode(Src));
 if (Dst)
-  CommonDescendants += T2.isInSubtree(*Dst, N2);
+  CommonDescendants += isInSubtree(*Dst, N2);
   }
   // We need to subtract 1 to get the number of descendants excluding the
   // root.
-  double Denominator = T1.getNumberOfDescendants(N1) - 1 +
-   T2.getNumberOfDescendants(N2) - 1 - CommonDescendants;
+  double Denominator = getNumberOfDescendants(N1) - 1 +
+   getNumberOfDescendants(N2) - 1 - CommonDescendants;
   // CommonDescendants is less than the size of one subtree.
   assert(Denominator >= 0 && "Expected non-negative denominator.");
   if (Denominator == 0)
@@ -906,7 +904,7 @@
 for (NodeRef N1 : H1) {
   for (NodeRef N2 : H2) {
 

[PATCH] D39652: [clang-diff] NFC: factor out getCompilationDatabase()

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision.

https://reviews.llvm.org/D39652

Files:
  tools/clang-diff/ClangDiff.cpp


Index: tools/clang-diff/ClangDiff.cpp
===
--- tools/clang-diff/ClangDiff.cpp
+++ tools/clang-diff/ClangDiff.cpp
@@ -84,27 +84,33 @@
   Compilations = std::move(AdjustingCompilations);
 }
 
-static std::unique_ptr
-getAST(const std::unique_ptr &CommonCompilations,
-   const StringRef Filename) {
+static std::unique_ptr
+getCompilationDatabase(StringRef Filename) {
   std::string ErrorMessage;
-  std::unique_ptr Compilations;
-  if (!CommonCompilations) {
-Compilations = CompilationDatabase::autoDetectFromSource(
-BuildPath.empty() ? Filename : BuildPath, ErrorMessage);
-if (!Compilations) {
-  llvm::errs()
-  << "Error while trying to load a compilation database, running "
- "without flags.\n"
-  << ErrorMessage;
-  Compilations =
-  llvm::make_unique(
-  ".", std::vector());
-}
+  std::unique_ptr Compilations =
+  CompilationDatabase::autoDetectFromSource(
+  BuildPath.empty() ? Filename : BuildPath, ErrorMessage);
+  if (!Compilations) {
+llvm::errs()
+<< "Error while trying to load a compilation database, running "
+   "without flags.\n"
+<< ErrorMessage;
+Compilations = llvm::make_unique(
+".", std::vector());
   }
   addExtraArgs(Compilations);
+  return Compilations;
+}
+
+static std::unique_ptr
+getAST(const std::unique_ptr &CommonCompilations,
+   const StringRef Filename) {
   std::array Files = {{Filename}};
-  ClangTool Tool(Compilations ? *Compilations : *CommonCompilations, Files);
+  std::unique_ptr FileCompilations;
+  if (!CommonCompilations)
+FileCompilations = getCompilationDatabase(Filename);
+  ClangTool Tool(CommonCompilations ? *CommonCompilations : *FileCompilations,
+ Files);
   std::vector> ASTs;
   Tool.buildASTs(ASTs);
   if (ASTs.size() != Files.size())


Index: tools/clang-diff/ClangDiff.cpp
===
--- tools/clang-diff/ClangDiff.cpp
+++ tools/clang-diff/ClangDiff.cpp
@@ -84,27 +84,33 @@
   Compilations = std::move(AdjustingCompilations);
 }
 
-static std::unique_ptr
-getAST(const std::unique_ptr &CommonCompilations,
-   const StringRef Filename) {
+static std::unique_ptr
+getCompilationDatabase(StringRef Filename) {
   std::string ErrorMessage;
-  std::unique_ptr Compilations;
-  if (!CommonCompilations) {
-Compilations = CompilationDatabase::autoDetectFromSource(
-BuildPath.empty() ? Filename : BuildPath, ErrorMessage);
-if (!Compilations) {
-  llvm::errs()
-  << "Error while trying to load a compilation database, running "
- "without flags.\n"
-  << ErrorMessage;
-  Compilations =
-  llvm::make_unique(
-  ".", std::vector());
-}
+  std::unique_ptr Compilations =
+  CompilationDatabase::autoDetectFromSource(
+  BuildPath.empty() ? Filename : BuildPath, ErrorMessage);
+  if (!Compilations) {
+llvm::errs()
+<< "Error while trying to load a compilation database, running "
+   "without flags.\n"
+<< ErrorMessage;
+Compilations = llvm::make_unique(
+".", std::vector());
   }
   addExtraArgs(Compilations);
+  return Compilations;
+}
+
+static std::unique_ptr
+getAST(const std::unique_ptr &CommonCompilations,
+   const StringRef Filename) {
   std::array Files = {{Filename}};
-  ClangTool Tool(Compilations ? *Compilations : *CommonCompilations, Files);
+  std::unique_ptr FileCompilations;
+  if (!CommonCompilations)
+FileCompilations = getCompilationDatabase(Filename);
+  ClangTool Tool(CommonCompilations ? *CommonCompilations : *FileCompilations,
+ Files);
   std::vector> ASTs;
   Tool.buildASTs(ASTs);
   if (ASTs.size() != Files.size())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D36687: [clang-diff] Match nodes with the same parent and same value

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 121641.
johannes added a comment.

update


https://reviews.llvm.org/D36687

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  lib/Tooling/ASTDiff/ASTDiff.cpp
  test/Tooling/clang-diff-bottomup.cpp
  test/Tooling/clang-diff-heuristics.cpp
  test/Tooling/clang-diff-opt.cpp
  tools/clang-diff/ClangDiff.cpp

Index: tools/clang-diff/ClangDiff.cpp
===
--- tools/clang-diff/ClangDiff.cpp
+++ tools/clang-diff/ClangDiff.cpp
@@ -496,8 +496,10 @@
   if (!StopAfter.empty()) {
 if (StopAfter == "topdown")
   Options.StopAfterTopDown = true;
-else if (StopAfter != "bottomup") {
-  llvm::errs() << "Error: Invalid argument for -stop-after\n";
+else if (StopAfter == "bottomup")
+  Options.StopAfterBottomUp = true;
+else {
+  llvm::errs() << "Error: Invalid argument for -stop-diff-after\n";
   return 1;
 }
   }
Index: test/Tooling/clang-diff-opt.cpp
===
--- test/Tooling/clang-diff-opt.cpp
+++ test/Tooling/clang-diff-opt.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -E %s > %t.src.cpp
 // RUN: %clang_cc1 -E %s > %t.dst.cpp -DDEST
-// RUN: clang-diff -dump-matches -s=10 %t.src.cpp %t.dst.cpp -- | FileCheck %s
+// RUN: clang-diff -dump-matches -s=10 -stop-diff-after=bottomup %t.src.cpp %t.dst.cpp -- | FileCheck %s
 //
 // Test the behaviour of the matching according to the optimal tree edit
 // distance, implemented with Zhang and Shasha's algorithm.
Index: test/Tooling/clang-diff-heuristics.cpp
===
--- /dev/null
+++ test/Tooling/clang-diff-heuristics.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -E %s > %t.src.cpp
+// RUN: %clang_cc1 -E %s > %t.dst.cpp -DDEST
+// RUN: clang-diff -dump-matches -s=0 %t.src.cpp %t.dst.cpp -- | FileCheck %s
+//
+// Test the heuristics, with maxsize set to 0, so that the optimal matching will never be applied.
+
+#ifndef DEST
+
+void f1() {;}
+
+void f2(int) {;}
+
+#else
+
+// same parents, same value
+// CHECK: Match FunctionDecl: f1(void ())(1) to FunctionDecl: f1(void ())(1)
+// CHECK: Match CompoundStmt
+void f1() {}
+
+// same parents, same identifier
+// CHECK: Match FunctionDecl: f2(void (int))(4) to FunctionDecl: f2(void ())(3)
+// CHECK: Match CompoundStmt
+void f2() {}
+
+#endif
Index: test/Tooling/clang-diff-bottomup.cpp
===
--- test/Tooling/clang-diff-bottomup.cpp
+++ test/Tooling/clang-diff-bottomup.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -E %s > %t.src.cpp
 // RUN: %clang_cc1 -E %s > %t.dst.cpp -DDEST
-// RUN: clang-diff -dump-matches -s=0 %t.src.cpp %t.dst.cpp -- | FileCheck %s
+// RUN: clang-diff -dump-matches -s=0 -stop-diff-after=bottomup %t.src.cpp %t.dst.cpp -- | FileCheck %s
 //
 // Test the bottom-up matching, with maxsize set to 0, so that the optimal matching will never be applied.
 
Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -74,15 +74,22 @@
   // Descendants are only considered to be equal when they are mapped.
   double getJaccardSimilarity(NodeRef N1, NodeRef N2) const;
 
+  double getNodeSimilarity(NodeRef N1, NodeRef N2) const;
+
   // Returns the node that has the highest degree of similarity.
   const Node *findCandidate(NodeRef N1) const;
 
+  const Node *findCandidateFromChildren(NodeRef N1, NodeRef P2) const;
+
   // Returns a mapping of identical subtrees.
   void matchTopDown();
 
   // Tries to match any yet unmapped nodes, in a bottom-up fashion.
   void matchBottomUp();
 
+  // Matches nodes, whose parents are matched.
+  void matchChildren();
+
   int findNewPosition(NodeRef N) const;
 
   const ComparisonOptions &Options;
@@ -832,6 +839,20 @@
   return CommonDescendants / Denominator;
 }
 
+double ASTDiff::Impl::getNodeSimilarity(NodeRef N1, NodeRef N2) const {
+  auto Ident1 = N1.getIdentifier(), Ident2 = N2.getIdentifier();
+
+  bool SameValue = T1.getNodeValue(N1) == T2.getNodeValue(N2);
+  auto SameIdent = Ident1 && Ident2 && *Ident1 == *Ident2;
+
+  double NodeSimilarity = 0;
+  NodeSimilarity += SameValue;
+  NodeSimilarity += SameIdent;
+
+  assert(haveSameParents(N1, N2));
+  return NodeSimilarity * Options.MinSimilarity;
+}
+
 const Node *ASTDiff::Impl::findCandidate(NodeRef N1) const {
   const Node *Candidate = nullptr;
   double HighestSimilarity = 0.0;
@@ -849,6 +870,25 @@
   return Candidate;
 }
 
+const Node *ASTDiff::Impl::findCandidateFromChildren(NodeRef N1,
+ NodeRef P2) const {
+  const Node *Candidate = nullptr;
+  double HighestSimilarity = 0.0;
+  for (NodeRef N2 : P2) {
+if (!isMatchingPossible(N1, N2))
+  continue;
+if (getSrc(N2))
+  continue;
+double Similarity = getJaccardSimilarity(N1, N2);
+Simila

[PATCH] D36997: [clang-diff] Honor macros

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 121642.
johannes added a comment.

update


https://reviews.llvm.org/D36997

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  lib/Tooling/ASTDiff/ASTDiff.cpp
  test/Tooling/Inputs/clang-diff-basic-src.cpp
  test/Tooling/clang-diff-ast.cpp
  test/Tooling/clang-diff-basic.cpp
  tools/clang-diff/ClangDiff.cpp

Index: tools/clang-diff/ClangDiff.cpp
===
--- tools/clang-diff/ClangDiff.cpp
+++ tools/clang-diff/ClangDiff.cpp
@@ -467,7 +467,7 @@
 std::unique_ptr AST = getAST(CommonCompilations, SourcePath);
 if (!AST)
   return 1;
-diff::SyntaxTree Tree(AST->getASTContext());
+diff::SyntaxTree Tree(*AST);
 if (ASTDump) {
   printTree(llvm::outs(), Tree);
   return 0;
@@ -503,8 +503,8 @@
   return 1;
 }
   }
-  diff::SyntaxTree SrcTree(Src->getASTContext());
-  diff::SyntaxTree DstTree(Dst->getASTContext());
+  diff::SyntaxTree SrcTree(*Src);
+  diff::SyntaxTree DstTree(*Dst);
   diff::ASTDiff Diff(SrcTree, DstTree, Options);
 
   if (HtmlDiff) {
Index: test/Tooling/clang-diff-basic.cpp
===
--- test/Tooling/clang-diff-basic.cpp
+++ test/Tooling/clang-diff-basic.cpp
@@ -53,5 +53,20 @@
 void f1() {{ (void) __func__;;; }}
 }
 
+// macros are always compared by their full textual value
+
+#define M1 return 1 + 1
+#define M2 return 2 * 2
+#define F(a, b) return a + b;
+
+int f2() {
+  // CHECK: Match Macro: M1{{.*}} to Macro: M1
+  M1;
+  // CHECK: Update Macro: M1{{.*}} to M2
+  M2;
+  // CHECK: Update Macro: F(1, 1){{.*}} to F(1, /*b=*/1)
+  F(1, /*b=*/1);
+}
+
 // CHECK: Delete AccessSpecDecl: public
 // CHECK: Delete CXXMethodDecl
Index: test/Tooling/clang-diff-ast.cpp
===
--- test/Tooling/clang-diff-ast.cpp
+++ test/Tooling/clang-diff-ast.cpp
@@ -72,12 +72,14 @@
 };
 
 #define M (void)1
-#define MA(a, b) (void)a, b
-// CHECK: FunctionDecl
-// CHECK-NEXT: CompoundStmt
+#define F(a, b) (void)a, b
 void macros() {
+  // CHECK: Macro: M(
   M;
-  MA(1, 2);
+  // two expressions, therefore it occurs twice
+  // CHECK-NEXT: Macro: F(1, 2)(
+  // CHECK-NEXT: Macro: F(1, 2)(
+  F(1, 2);
 }
 
 #ifndef GUARD
Index: test/Tooling/Inputs/clang-diff-basic-src.cpp
===
--- test/Tooling/Inputs/clang-diff-basic-src.cpp
+++ test/Tooling/Inputs/clang-diff-basic-src.cpp
@@ -31,3 +31,13 @@
 int um = 1 * 2 + 3;
 
 void f1() {{ (void) __func__;;; }}
+
+#define M1 return 1 + 1
+#define M2 return 1 + 2
+#define F(a, b) return a + b;
+
+int f2() {
+  M1;
+  M1;
+  F(1, 1);
+}
Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -28,7 +28,7 @@
 namespace diff {
 
 bool ComparisonOptions::isMatchingAllowed(NodeRef N1, NodeRef N2) const {
-  return N1.getType().isSame(N2.getType());
+  return (N1.isMacro() && N2.isMacro()) || N1.getType().isSame(N2.getType());
 }
 
 class ASTDiff::Impl {
@@ -114,23 +114,23 @@
 /// Represents the AST of a TranslationUnit.
 class SyntaxTree::Impl {
 public:
-  Impl(SyntaxTree *Parent, ASTContext &AST);
+  Impl(SyntaxTree *Parent, ASTUnit &AST);
   /// Constructs a tree from an AST node.
-  Impl(SyntaxTree *Parent, Decl *N, ASTContext &AST);
-  Impl(SyntaxTree *Parent, Stmt *N, ASTContext &AST);
+  Impl(SyntaxTree *Parent, Decl *N, ASTUnit &AST);
+  Impl(SyntaxTree *Parent, Stmt *N, ASTUnit &AST);
   template 
   Impl(SyntaxTree *Parent,
typename std::enable_if::value, T>::type *Node,
-   ASTContext &AST)
+   ASTUnit &AST)
   : Impl(Parent, dyn_cast(Node), AST) {}
   template 
   Impl(SyntaxTree *Parent,
typename std::enable_if::value, T>::type *Node,
-   ASTContext &AST)
+   ASTUnit &AST)
   : Impl(Parent, dyn_cast(Node), AST) {}
 
   SyntaxTree *Parent;
-  ASTContext &AST;
+  ASTUnit &AST;
   PrintingPolicy TypePP;
   /// Nodes in preorder.
   std::vector Nodes;
@@ -181,21 +181,40 @@
   return !I->isWritten();
 }
 
-template  static bool isNodeExcluded(const SourceManager &SM, T *N) {
+template  static bool isNodeExcluded(ASTUnit &AST, T *N) {
+  const SourceManager &SM = AST.getSourceManager();
   if (!N)
 return true;
   SourceLocation SLoc = N->getSourceRange().getBegin();
   if (SLoc.isValid()) {
 // Ignore everything from other files.
 if (!SM.isInMainFile(SLoc))
   return true;
-// Ignore macros.
-if (SLoc != SM.getSpellingLoc(SLoc))
+const Preprocessor &PP = AST.getPreprocessor();
+if (SLoc.isMacroID() && !PP.isAtStartOfMacroExpansion(SLoc))
   return true;
   }
   return isSpecializedNodeExcluded(N);
 }
 
+static SourceRange getSourceRange(const ASTUnit &AST, const DynTypedNode &DTN) {
+  const SourceManager &SM = AST.getSourceManager();
+  SourceRange Range = DTN.getSource

[PATCH] D39653: [clang-diff] Add utility functions, forbid copying Node

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision.
Herald added a subscriber: klimek.

https://reviews.llvm.org/D39653

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  lib/Tooling/ASTDiff/ASTDiff.cpp


Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -1119,14 +1119,23 @@
 
 ASTUnit &SyntaxTree::getASTUnit() const { return TreeImpl->AST; }
 
+SourceManager &SyntaxTree::getSourceManager() const {
+  return TreeImpl->AST.getSourceManager();
+}
+
+const LangOptions &SyntaxTree::getLangOpts() const {
+  return TreeImpl->AST.getLangOpts();
+}
+
 const ASTContext &SyntaxTree::getASTContext() const {
   return TreeImpl->AST.getASTContext();
 }
 
 NodeRef SyntaxTree::getNode(NodeId Id) const { return TreeImpl->getNode(Id); }
 
 int SyntaxTree::getSize() const { return TreeImpl->getSize(); }
 NodeRef SyntaxTree::getRoot() const { return TreeImpl->getRoot(); }
+NodeId SyntaxTree::getRootId() const { return TreeImpl->getRootId(); }
 SyntaxTree::PreorderIterator SyntaxTree::begin() const {
   return TreeImpl->begin();
 }
Index: include/clang/Tooling/ASTDiff/ASTDiff.h
===
--- include/clang/Tooling/ASTDiff/ASTDiff.h
+++ include/clang/Tooling/ASTDiff/ASTDiff.h
@@ -80,14 +80,18 @@
   SyntaxTree(T *Node, ASTUnit &AST)
   : TreeImpl(llvm::make_unique(this, Node, AST)) {}
   SyntaxTree(SyntaxTree &&Other) = default;
+  SyntaxTree &operator=(SyntaxTree &&Other) = default;
   ~SyntaxTree();
 
   ASTUnit &getASTUnit() const;
   const ASTContext &getASTContext() const;
+  SourceManager &getSourceManager() const;
+  const LangOptions &getLangOpts() const;
   StringRef getFilename() const;
 
   int getSize() const;
   NodeRef getRoot() const;
+  NodeId getRootId() const;
   using PreorderIterator = const Node *;
   PreorderIterator begin() const;
   PreorderIterator end() const;
@@ -113,6 +117,10 @@
   SmallVector Children;
   ChangeKind Change = NoChange;
   Node(SyntaxTree::Impl &Tree) : Tree(Tree), Children() {}
+  Node(NodeRef Other) = delete;
+  explicit Node(Node &&Other) = default;
+  Node &operator=(NodeRef Other) = delete;
+  Node &operator=(Node &&Other) = default;
 
   NodeId getId() const;
   SyntaxTree &getTree() const;


Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -1119,14 +1119,23 @@
 
 ASTUnit &SyntaxTree::getASTUnit() const { return TreeImpl->AST; }
 
+SourceManager &SyntaxTree::getSourceManager() const {
+  return TreeImpl->AST.getSourceManager();
+}
+
+const LangOptions &SyntaxTree::getLangOpts() const {
+  return TreeImpl->AST.getLangOpts();
+}
+
 const ASTContext &SyntaxTree::getASTContext() const {
   return TreeImpl->AST.getASTContext();
 }
 
 NodeRef SyntaxTree::getNode(NodeId Id) const { return TreeImpl->getNode(Id); }
 
 int SyntaxTree::getSize() const { return TreeImpl->getSize(); }
 NodeRef SyntaxTree::getRoot() const { return TreeImpl->getRoot(); }
+NodeId SyntaxTree::getRootId() const { return TreeImpl->getRootId(); }
 SyntaxTree::PreorderIterator SyntaxTree::begin() const {
   return TreeImpl->begin();
 }
Index: include/clang/Tooling/ASTDiff/ASTDiff.h
===
--- include/clang/Tooling/ASTDiff/ASTDiff.h
+++ include/clang/Tooling/ASTDiff/ASTDiff.h
@@ -80,14 +80,18 @@
   SyntaxTree(T *Node, ASTUnit &AST)
   : TreeImpl(llvm::make_unique(this, Node, AST)) {}
   SyntaxTree(SyntaxTree &&Other) = default;
+  SyntaxTree &operator=(SyntaxTree &&Other) = default;
   ~SyntaxTree();
 
   ASTUnit &getASTUnit() const;
   const ASTContext &getASTContext() const;
+  SourceManager &getSourceManager() const;
+  const LangOptions &getLangOpts() const;
   StringRef getFilename() const;
 
   int getSize() const;
   NodeRef getRoot() const;
+  NodeId getRootId() const;
   using PreorderIterator = const Node *;
   PreorderIterator begin() const;
   PreorderIterator end() const;
@@ -113,6 +117,10 @@
   SmallVector Children;
   ChangeKind Change = NoChange;
   Node(SyntaxTree::Impl &Tree) : Tree(Tree), Children() {}
+  Node(NodeRef Other) = delete;
+  explicit Node(Node &&Other) = default;
+  Node &operator=(NodeRef Other) = delete;
+  Node &operator=(Node &&Other) = default;
 
   NodeId getId() const;
   SyntaxTree &getTree() const;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D39655: [clang-diff] Expose Node::getSourceRange()

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision.
Herald added a subscriber: klimek.

https://reviews.llvm.org/D39655

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  lib/Tooling/ASTDiff/ASTDiff.cpp
  test/Tooling/clang-diff-json.cpp

Index: test/Tooling/clang-diff-json.cpp
===
--- test/Tooling/clang-diff-json.cpp
+++ test/Tooling/clang-diff-json.cpp
@@ -2,10 +2,10 @@
 // RUN: | %python -c 'import json, sys; json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
 // RUN: | FileCheck %s
 
-// CHECK: "begin": 299,
-// CHECK: "type": "FieldDecl",
-// CHECK: "end": 319,
-// CHECK: "type": "CXXRecordDecl",
+// CHECK: "begin": 297,
+// CHECK: "type": "FieldDecl"
+// CHECK: "end": 318,
+// CHECK: "type": "CXXRecordDecl"
 class A {
   int x;
 };
Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -197,24 +197,6 @@
   return isSpecializedNodeExcluded(N);
 }
 
-static SourceRange getSourceRange(const ASTUnit &AST, const DynTypedNode &DTN) {
-  const SourceManager &SM = AST.getSourceManager();
-  SourceRange Range = DTN.getSourceRange();
-  SourceLocation BeginLoc = Range.getBegin();
-  SourceLocation EndLoc;
-  if (BeginLoc.isMacroID())
-EndLoc = SM.getExpansionRange(BeginLoc).second;
-  else
-EndLoc = Range.getEnd();
-  EndLoc =
-  Lexer::getLocForEndOfToken(EndLoc, /*Offset=*/0, SM, AST.getLangOpts());
-  if (auto *ThisExpr = DTN.get()) {
-if (ThisExpr->isImplicit())
-  EndLoc = BeginLoc;
-  }
-  return {SM.getExpansionLoc(BeginLoc), SM.getExpansionLoc(EndLoc)};
-}
-
 namespace {
 // Sets Height, Parent and Children for each node.
 struct PreorderVisitor : public RecursiveASTVisitor {
@@ -420,8 +402,7 @@
   assert(&N.Tree == this);
   const DynTypedNode &DTN = N.ASTNode;
   if (N.isMacro()) {
-CharSourceRange Range(getSourceRange(AST, N.ASTNode), false);
-return Lexer::getSourceText(Range, AST.getSourceManager(),
+return Lexer::getSourceText(N.getSourceRange(), AST.getSourceManager(),
 AST.getLangOpts());
   }
   if (auto *S = DTN.get())
@@ -756,9 +737,59 @@
  Siblings.begin();
 }
 
+static SourceRange getSourceRangeImpl(NodeRef N) {
+  const DynTypedNode &DTN = N.ASTNode;
+  SyntaxTree::Impl &Tree = N.Tree;
+  SourceManager &SM = Tree.AST.getSourceManager();
+  const LangOptions &LangOpts = Tree.AST.getLangOpts();
+  auto EndOfToken = [&](SourceLocation Loc) {
+return Lexer::getLocForEndOfToken(Loc, /*Offset=*/0, SM, LangOpts);
+  };
+  auto TokenToCharRange = [&](SourceRange Range) -> SourceRange {
+return {Range.getBegin(), EndOfToken(Range.getEnd())};
+  };
+  SourceRange Range = DTN.getSourceRange();
+  if (N.isMacro()) {
+SourceLocation BeginLoc = Range.getBegin();
+SourceLocation End = SM.getExpansionRange(BeginLoc).second;
+End = EndOfToken(End);
+return {SM.getExpansionLoc(BeginLoc), SM.getExpansionLoc(End)};
+  }
+  if (auto *ThisExpr = DTN.get())
+if (ThisExpr->isImplicit())
+  return {Range.getBegin(), Range.getBegin()};
+  if (auto *CE = DTN.get()) {
+if (!isa(CE))
+  return CE->getParenOrBraceRange();
+  } else if (DTN.get()) {
+return TokenToCharRange(Range);
+  } else if (DTN.get() || DTN.get() ||
+ DTN.get() ||
+ (DTN.get() &&
+  N.getParent()->ASTNode.get()) ||
+ (DTN.get() &&
+  !DTN.get()->isThisDeclarationADefinition()) ||
+ DTN.get() || DTN.get() ||
+ DTN.get()) {
+SourceLocation End = Range.getEnd();
+if (DTN.get())
+  End = End.getLocWithOffset(-1);
+SourceLocation SemicolonLoc = Lexer::findLocationAfterToken(
+End, tok::semi, SM, LangOpts,
+/*SkipTrailingWhitespaceAndNewLine=*/false);
+Range.setEnd(SemicolonLoc);
+return Range;
+  }
+  return TokenToCharRange(Range);
+}
+
+CharSourceRange Node::getSourceRange() const {
+  return CharSourceRange::getCharRange(getSourceRangeImpl(*this));
+}
+
 std::pair Node::getSourceRangeOffsets() const {
   const SourceManager &SM = Tree.AST.getSourceManager();
-  SourceRange Range = getSourceRange(Tree.AST, ASTNode);
+  CharSourceRange Range = getSourceRange();
   unsigned Begin = SM.getFileOffset(Range.getBegin());
   unsigned End = SM.getFileOffset(Range.getEnd());
   return {Begin, End};
Index: include/clang/Tooling/ASTDiff/ASTDiff.h
===
--- include/clang/Tooling/ASTDiff/ASTDiff.h
+++ include/clang/Tooling/ASTDiff/ASTDiff.h
@@ -139,7 +139,21 @@
 
   int findPositionInParent() const;
 
-  // Returns the starting and ending offset of the node in its source file.
+  /// Returns the range that contains the text that is associated with this
+  /// node. This is based on DynTypedNode::getSourceRange() with a couple of
+  /// workarounds.
+  ///
+  /// The ra

[PATCH] D37001: [clang-diff] Use data collectors for node comparison

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 121647.
johannes added a comment.

use raw source code of owned tokens instead


https://reviews.llvm.org/D37001

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  lib/Tooling/ASTDiff/ASTDiff.cpp
  test/Tooling/clang-diff-basic.cpp
  test/Tooling/clang-diff-html.test
  test/Tooling/clang-diff-topdown.cpp

Index: test/Tooling/clang-diff-topdown.cpp
===
--- test/Tooling/clang-diff-topdown.cpp
+++ test/Tooling/clang-diff-topdown.cpp
@@ -35,8 +35,6 @@
   int x2 = ::x + 1;
 }
 
-class A { int x = 1 + 1; void f() { int x1 = x; } };
-
 #else
 
 
@@ -66,18 +64,4 @@
   int x2 = ::x + 1;
 }
 
-class B {
-  // Only the class name changed; it is not included in the field value,
-  // therefore there is no update.
-  // CHECK: Match FieldDecl: :x(int)(24) to FieldDecl: :x(int)(29)
-  // CHECK-NOT: Update FieldDecl: :x(int)(24)
-  int x = 1+1;
-  void f() {
-// CHECK: Match MemberExpr: :x(32) to MemberExpr: :x(37)
-// CHECK-NOT: Update MemberExpr: :x(32)
-int x1 = B::x;
-  }
-
-};
-
 #endif
Index: test/Tooling/clang-diff-html.test
===
--- test/Tooling/clang-diff-html.test
+++ test/Tooling/clang-diff-html.test
@@ -1,7 +1,7 @@
 RUN: clang-diff -html %S/Inputs/clang-diff-basic-src.cpp %S/clang-diff-basic.cpp -- | FileCheck %s
 
 CHECK: 
+CHECK-NEXT: 0 -> 0' class='u'>
 
 match, update
 CHECK: "Bar"
 
 comments
-CHECK: // CHECK: Insert IfStmt{{.*}} into IfStmt
 CHECK: // CHECK: Delete AccessSpecDecl: public
Index: test/Tooling/clang-diff-basic.cpp
===
--- test/Tooling/clang-diff-basic.cpp
+++ test/Tooling/clang-diff-basic.cpp
@@ -30,10 +30,10 @@
 
 class X {
   const char *foo(int i) {
+// CHECK: Insert IfStmt(29) into CompoundStmt(28)
 if (i == 0)
   return "Bar";
-// CHECK: Insert IfStmt{{.*}} into IfStmt
-// CHECK: Insert BinaryOperator: =={{.*}} into IfStmt
+// CHECK: Move IfStmt(35) into IfStmt
 else if (i == -1)
   return "foo";
 return 0;
@@ -64,7 +64,7 @@
   M1;
   // CHECK: Update Macro: M1{{.*}} to M2
   M2;
-  // CHECK: Update Macro: F(1, 1){{.*}} to F(1, /*b=*/1)
+  // CHECK: Match Macro: F(1, 1)(74)
   F(1, /*b=*/1);
 }
 
Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -16,6 +16,7 @@
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/PriorityQueue.h"
+#include "llvm/Support/MD5.h"
 
 #include 
 #include 
@@ -111,6 +112,8 @@
 };
 } // end anonymous namespace
 
+using HashType = std::array;
+
 /// Represents the AST of a TranslationUnit.
 class SyntaxTree::Impl {
 public:
@@ -463,6 +466,30 @@
   return "";
 }
 
+static HashType hashNode(NodeRef N) {
+  llvm::MD5 Hash;
+  SourceManager &SM = N.getTree().getSourceManager();
+  const LangOptions &LangOpts = N.getTree().getLangOpts();
+  Token Tok;
+  for (auto TokenLocation : N.getOwnedTokens()) {
+bool Failure = Lexer::getRawToken(TokenLocation, Tok, SM, LangOpts,
+  /*IgnoreWhiteSpace=*/true);
+assert(!Failure);
+auto Range = CharSourceRange::getCharRange(TokenLocation, Tok.getEndLoc());
+// This is here to make CompoundStmt nodes compare equal, to make the tests
+// pass. It should be changed to include changes to comments.
+if (!Tok.isOneOf(tok::comment, tok::semi))
+  Hash.update(Lexer::getSourceText(Range, SM, LangOpts));
+  }
+  llvm::MD5::MD5Result HashResult;
+  Hash.final(HashResult);
+  return HashResult;
+}
+
+static bool areNodesDifferent(NodeRef N1, NodeRef N2) {
+  return hashNode(N1) != hashNode(N2);
+}
+
 /// Identifies a node in a subtree by its postorder offset, starting at 1.
 struct SNodeId {
   int Id = 0;
@@ -637,7 +664,7 @@
 NodeRef N1 = S1.getNode(Id1), N2 = S2.getNode(Id2);
 if (!DiffImpl.isMatchingPossible(N1, N2))
   return std::numeric_limits::max();
-return S1.getNodeValue(Id1) != S2.getNodeValue(Id2);
+return areNodesDifferent(S1.getNode(Id1), S2.getNode(Id2));
   }
 
   void computeTreeDist() {
@@ -795,6 +822,91 @@
   return {Begin, End};
 }
 
+static bool onlyWhitespace(StringRef Str) {
+  return std::all_of(Str.begin(), Str.end(),
+ [](char C) { return std::isspace(C); });
+}
+
+SmallVector Node::getOwnedSourceRanges() const {
+  SmallVector SourceRanges;
+  SourceManager &SM = getTree().getSourceManager();
+  const LangOptions &LangOpts = getTree().getLangOpts();
+  CharSourceRange Range = getSourceRange();
+  SourceLocation Offset = Range.getBegin();
+  BeforeThanCompare Less(SM);
+  auto AddSegment = [&](SourceLocation Until) {
+if (Offset.isValid() && Until.isValid() && Less(Offset, Until)) {
+  CharSourceRange R = CharSourceRange::getCharRange({Offset, Until});
+  S

[PATCH] D39656: [clang-diff] Remove getNodeValue

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision.
Herald added a subscriber: klimek.

https://reviews.llvm.org/D39656

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  lib/Tooling/ASTDiff/ASTDiff.cpp
  test/Tooling/clang-diff-ast.cpp
  test/Tooling/clang-diff-basic.cpp
  test/Tooling/clang-diff-bottomup.cpp
  test/Tooling/clang-diff-heuristics.cpp
  test/Tooling/clang-diff-html.test
  test/Tooling/clang-diff-json.cpp
  test/Tooling/clang-diff-topdown.cpp
  tools/clang-diff/ClangDiff.cpp

Index: tools/clang-diff/ClangDiff.cpp
===
--- tools/clang-diff/ClangDiff.cpp
+++ tools/clang-diff/ClangDiff.cpp
@@ -299,13 +299,7 @@
  << "tid='" << OtherTag << TargetId << "' ";
   OS << "title='";
   printHtml(OS, Node.getTypeLabel());
-  OS << "\n" << LeftId << " -> " << RightId;
-  std::string Value = Tree.getNodeValue(Node);
-  if (!Value.empty()) {
-OS << "\n";
-printHtml(OS, Value);
-  }
-  OS << "'";
+  OS << "\n" << LeftId << " -> " << RightId << "'";
   if (Node.Change != diff::NoChange)
 OS << " class='" << getChangeKindAbbr(Node.Change) << "'";
   OS << ">";
@@ -356,12 +350,6 @@
   auto Offsets = Node.getSourceRangeOffsets();
   OS << R"(,"begin":)" << Offsets.first;
   OS << R"(,"end":)" << Offsets.second;
-  std::string Value = Tree.getNodeValue(Node);
-  if (!Value.empty()) {
-OS << R"(,"value":")";
-printJsonString(OS, Value);
-OS << '"';
-  }
 }
 
 static void printNodeAsJson(raw_ostream &OS, diff::SyntaxTree &Tree,
@@ -395,9 +383,6 @@
 static void printNode(raw_ostream &OS, diff::SyntaxTree &Tree,
   diff::NodeRef Node) {
   OS << Node.getTypeLabel();
-  std::string Value = Tree.getNodeValue(Node);
-  if (!Value.empty())
-OS << ": " << Value;
   OS << "(" << Node.getId() << ")";
 }
 
@@ -422,7 +407,7 @@
   case diff::Update:
 OS << "Update ";
 printNode(OS, SrcTree, *Src);
-OS << " to " << DstTree.getNodeValue(Dst) << "\n";
+OS << "\n";
 break;
   case diff::Insert:
   case diff::Move:
Index: test/Tooling/clang-diff-topdown.cpp
===
--- test/Tooling/clang-diff-topdown.cpp
+++ test/Tooling/clang-diff-topdown.cpp
@@ -58,9 +58,9 @@
 
 namespace dst {
   int x;
-  // CHECK: Match DeclRefExpr: :x(17) to DeclRefExpr: :x(22)
+  // CHECK: Match DeclRefExpr(17) to DeclRefExpr(22)
   int x1 = x + 1;
-  // CHECK: Match DeclRefExpr: x(21) to DeclRefExpr: x(26)
+  // CHECK: Match DeclRefExpr(21) to DeclRefExpr(26)
   int x2 = ::x + 1;
 }
 
Index: test/Tooling/clang-diff-json.cpp
===
--- test/Tooling/clang-diff-json.cpp
+++ test/Tooling/clang-diff-json.cpp
@@ -19,9 +19,5 @@
 // CHECK-NEXT: "type": "CharacterLiteral"
 // CHECK-NEXT: }
 // CHECK: ]
-// CHECK: "type": "VarDecl",
+// CHECK: "type": "VarDecl"
 char nl = '\n';
-
-// CHECK: "value": "abc \n\t\u\u001f"
-char s[] = "abc \n\t\0\x1f";
-
Index: test/Tooling/clang-diff-html.test
===
--- test/Tooling/clang-diff-html.test
+++ test/Tooling/clang-diff-html.test
@@ -5,31 +5,26 @@
 
 match, update
 CHECK: namespace src {
+CHECK-NEXT: [[L]] -> [[R]]' class='u'>namespace src {
 
 match, move
 CHECK: void foo()
+CHECK-NEXT: [[L]] -> [[R]]' class='m'>void foo()
 
 match
 CHECK: void main()
+CHECK-NEXT: [[L]] -> [[R]]'>void main()
 
 deletion
 CHECK: 4
+CHECK-NEXT: [[L]] -> -1' class='d'>4
 
 update + move
-CHECK: 2' class='u m'>2
+CHECK: class='u m'>2
 
 insertion
 CHECK: "Bar"
+CHECK-NEXT: -1 -> [[R]]' class='i'>"Bar"
 
 comments
-CHECK: // CHECK: Delete AccessSpecDecl: public
+CHECK: // CHECK: Delete AccessSpecDecl
Index: test/Tooling/clang-diff-heuristics.cpp
===
--- test/Tooling/clang-diff-heuristics.cpp
+++ test/Tooling/clang-diff-heuristics.cpp
@@ -13,12 +13,12 @@
 #else
 
 // same parents, same value
-// CHECK: Match FunctionDecl: f1(void ())(1) to FunctionDecl: f1(void ())(1)
+// CHECK: Match FunctionDecl(1) to FunctionDecl(1)
 // CHECK: Match CompoundStmt
 void f1() {}
 
 // same parents, same identifier
-// CHECK: Match FunctionDecl: f2(void (int))(4) to FunctionDecl: f2(void ())(3)
+// CHECK: Match FunctionDecl(4) to FunctionDecl(3)
 // CHECK: Match CompoundStmt
 void f2() {}
 
Index: test/Tooling/clang-diff-bottomup.cpp
===
--- test/Tooling/clang-diff-bottomup.cpp
+++ test/Tooling/clang-diff-bottomup.cpp
@@ -16,7 +16,7 @@
 void f1() {
 // CompoundStmt: 3 matched descendants, subtree sizes 4 and 5
 // Jaccard similarity = 3 / (4 + 5 - 3) = 3 / 6 >= 0.5
-// CHECK: Match FunctionDecl: f1(void ())(1) to FunctionDecl: f1(void ())(1)
+// CHECK: Match FunctionDecl(1) to FunctionDecl(1)
 // CHECK: Match CompoundStmt(2) to CompoundStmt(2)
 // CHECK: Match CompoundStmt(4) to CompoundStmt(3)
 // CHECK: Match CompoundStmt(5) to CompoundStmt

[PATCH] D36688: [clang-diff] Fix matching for unnamed NamedDecs

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 121649.
johannes added a comment.

update


https://reviews.llvm.org/D36688

Files:
  lib/Tooling/ASTDiff/ASTDiff.cpp
  test/Tooling/clang-diff-heuristics.cpp


Index: test/Tooling/clang-diff-heuristics.cpp
===
--- test/Tooling/clang-diff-heuristics.cpp
+++ test/Tooling/clang-diff-heuristics.cpp
@@ -10,6 +10,8 @@
 
 void f2(int) {;}
 
+class C3 { C3(); };
+
 #else
 
 // same parents, same value
@@ -22,4 +24,8 @@
 // CHECK: Match CompoundStmt
 void f2() {}
 
+// same parents, same identifier
+// CHECK: Match CXXConstructorDecl(9) to CXXConstructorDecl(6)
+class C3 { C3(int); };
+
 #endif
Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -607,6 +607,8 @@
   if (auto *ND = ASTNode.get()) {
 if (ND->getDeclName().isIdentifier())
   return ND->getQualifiedNameAsString();
+else
+  return std::string();
   }
   return llvm::None;
 }
@@ -617,6 +619,8 @@
   if (auto *ND = ASTNode.get()) {
 if (ND->getDeclName().isIdentifier())
   return ND->getName();
+else
+  return StringRef();
   }
   return llvm::None;
 }


Index: test/Tooling/clang-diff-heuristics.cpp
===
--- test/Tooling/clang-diff-heuristics.cpp
+++ test/Tooling/clang-diff-heuristics.cpp
@@ -10,6 +10,8 @@
 
 void f2(int) {;}
 
+class C3 { C3(); };
+
 #else
 
 // same parents, same value
@@ -22,4 +24,8 @@
 // CHECK: Match CompoundStmt
 void f2() {}
 
+// same parents, same identifier
+// CHECK: Match CXXConstructorDecl(9) to CXXConstructorDecl(6)
+class C3 { C3(int); };
+
 #endif
Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -607,6 +607,8 @@
   if (auto *ND = ASTNode.get()) {
 if (ND->getDeclName().isIdentifier())
   return ND->getQualifiedNameAsString();
+else
+  return std::string();
   }
   return llvm::None;
 }
@@ -617,6 +619,8 @@
   if (auto *ND = ASTNode.get()) {
 if (ND->getDeclName().isIdentifier())
   return ND->getName();
+else
+  return StringRef();
   }
   return llvm::None;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D37003: [clang-diff] Support templates

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 121650.
johannes added a comment.

update


https://reviews.llvm.org/D37003

Files:
  lib/Tooling/ASTDiff/ASTDiff.cpp
  test/Tooling/Inputs/clang-diff-basic-src.cpp
  test/Tooling/clang-diff-ast.cpp
  test/Tooling/clang-diff-basic.cpp
  test/Tooling/clang-diff-html.test

Index: test/Tooling/clang-diff-html.test
===
--- test/Tooling/clang-diff-html.test
+++ test/Tooling/clang-diff-html.test
@@ -1,4 +1,4 @@
-RUN: clang-diff -html %S/Inputs/clang-diff-basic-src.cpp %S/clang-diff-basic.cpp -- | FileCheck %s
+RUN: clang-diff -html %S/Inputs/clang-diff-basic-src.cpp %S/clang-diff-basic.cpp -- -std=c++11 | FileCheck %s
 
 CHECK: 
Index: test/Tooling/clang-diff-basic.cpp
===
--- test/Tooling/clang-diff-basic.cpp
+++ test/Tooling/clang-diff-basic.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-diff -dump-matches %S/Inputs/clang-diff-basic-src.cpp %s -- | FileCheck %s
+// RUN: clang-diff -dump-matches %S/Inputs/clang-diff-basic-src.cpp %s -- -std=c++11 | FileCheck %s
 
 // CHECK: Match TranslationUnitDecl(0) to TranslationUnitDecl(0)
 // CHECK: Match NamespaceDecl(1) to NamespaceDecl(1)
@@ -68,5 +68,18 @@
   F(1, /*b=*/1);
 }
 
+// CHECK: Match TemplateTypeParmDecl(77)
+template 
+U visit(Type &t) {
+  int x = t;
+  return U();
+}
+
+void tmp() {
+  long x;
+  // CHECK: Match TemplateArgument(93)
+  visit(x);
+}
+
 // CHECK: Delete AccessSpecDecl(39)
 // CHECK: Delete CXXMethodDecl(42)
Index: test/Tooling/clang-diff-ast.cpp
===
--- test/Tooling/clang-diff-ast.cpp
+++ test/Tooling/clang-diff-ast.cpp
@@ -93,3 +93,16 @@
 // CHECK-NEXT: FunctionDecl(64)
 void sentinel();
 #endif
+
+// CHECK-NEXT: ClassTemplateDecl(65)
+// CHECK-NEXT: TemplateTypeParmDecl(66)
+// CHECK-NEXT: CXXRecordDecl(67)
+template  class C {
+  // CHECK-NEXT: FieldDecl(68)
+  T t;
+};
+
+// CHECK-NEXT: CXXRecordDecl(69)
+// CHECK-NEXT: TemplateName(70)
+// CHECK-NEXT: TemplateArgument(71)
+class I : C {};
Index: test/Tooling/Inputs/clang-diff-basic-src.cpp
===
--- test/Tooling/Inputs/clang-diff-basic-src.cpp
+++ test/Tooling/Inputs/clang-diff-basic-src.cpp
@@ -41,3 +41,16 @@
   M1;
   F(1, 1);
 }
+
+template 
+U visit(T &t) {
+  int x = t;
+  return U();
+}
+
+void tmp() {
+  int x;
+  visit(x);
+}
+
+int x = 1;
Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -13,7 +13,7 @@
 
 #include "clang/Tooling/ASTDiff/ASTDiff.h"
 
-#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/LexicallyOrderedRecursiveASTVisitor.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/PriorityQueue.h"
 #include "llvm/Support/MD5.h"
@@ -141,6 +141,7 @@
   // Maps preorder indices to postorder ones.
   std::vector PostorderIds;
   NodeList NodesBfs;
+  std::map TemplateArgumentLocations;
 
   int getSize() const { return Nodes.size(); }
   NodeRef getRoot() const { return getNode(getRootId()); }
@@ -175,6 +176,13 @@
 static bool isSpecializedNodeExcluded(CXXCtorInitializer *I) {
   return !I->isWritten();
 }
+static bool isSpecializedNodeExcluded(const TemplateArgumentLoc *S) {
+  return false;
+}
+
+static bool isNodeExcluded(ASTUnit &AST, TemplateName *Template) {
+  return false;
+}
 
 template  static bool isNodeExcluded(ASTUnit &AST, T *N) {
   const SourceManager &SM = AST.getSourceManager();
@@ -194,20 +202,24 @@
 
 namespace {
 // Sets Height, Parent and Children for each node.
-struct PreorderVisitor : public RecursiveASTVisitor {
+struct PreorderVisitor
+: public LexicallyOrderedRecursiveASTVisitor {
+  using BaseType = LexicallyOrderedRecursiveASTVisitor;
+
   int Id = 0, Depth = 0;
   NodeId Parent;
   SyntaxTree::Impl &Tree;
 
-  PreorderVisitor(SyntaxTree::Impl &Tree) : Tree(Tree) {}
+  PreorderVisitor(SyntaxTree::Impl &Tree)
+  : BaseType(Tree.AST.getSourceManager()), Tree(Tree) {}
 
-  template  std::tuple PreTraverse(T *ASTNode) {
+  template  std::tuple PreTraverse(const T &ASTNode) {
 NodeId MyId = Id;
 Tree.Nodes.emplace_back(Tree);
 Node &N = Tree.getMutableNode(MyId);
 N.Parent = Parent;
 N.Depth = Depth;
-N.ASTNode = DynTypedNode::create(*ASTNode);
+N.ASTNode = DynTypedNode::create(ASTNode);
 assert(!N.ASTNode.getNodeKind().isNone() &&
"Expected nodes to have a valid kind.");
 if (Parent.isValid()) {
@@ -239,27 +251,44 @@
   bool TraverseDecl(Decl *D) {
 if (isNodeExcluded(Tree.AST, D))
   return true;
-auto SavedState = PreTraverse(D);
-RecursiveASTVisitor::TraverseDecl(D);
+auto SavedState = PreTraverse(*D);
+BaseType::TraverseDecl(D);
 PostTraverse(SavedState);
 return true;
   }
   bool TraverseStmt(Stmt *S) {
 if (S)
   S = S->IgnoreI

[PATCH] D39658: [clang-diff] Treat QualType / TypeLoc as a node

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision.
Herald added a subscriber: klimek.

https://reviews.llvm.org/D39658

Files:
  lib/Tooling/ASTDiff/ASTDiff.cpp
  test/Tooling/clang-diff-ast.cpp
  test/Tooling/clang-diff-basic.cpp
  test/Tooling/clang-diff-bottomup.cpp
  test/Tooling/clang-diff-heuristics.cpp
  test/Tooling/clang-diff-html.test
  test/Tooling/clang-diff-json.cpp
  test/Tooling/clang-diff-opt.cpp
  test/Tooling/clang-diff-topdown.cpp

Index: test/Tooling/clang-diff-topdown.cpp
===
--- test/Tooling/clang-diff-topdown.cpp
+++ test/Tooling/clang-diff-topdown.cpp
@@ -9,9 +9,10 @@
 void f1()
 {
   // Match some subtree of height greater than 2.
-  // CHECK: Match CompoundStmt(3) to CompoundStmt(3)
-  // CHECK: Match CompoundStmt(4) to CompoundStmt(4)
-  // CHECK: Match NullStmt(5) to NullStmt(5)
+  // CHECK: Match CompoundStmt(5) to CompoundStmt(5)
+  // CHECK-NEXT: Move CompoundStmt
+  // CHECK-NEXT: Match CompoundStmt
+  // CHECK-NEXT: Match NullStmt({{.*}}) to NullStmt({{.*}})
   {{;}}
 
   // Don't match subtrees that are smaller.
@@ -21,9 +22,10 @@
 
   // Greedy approach - use the first matching subtree when there are multiple
   // identical subtrees.
-  // CHECK: Match CompoundStmt(8) to CompoundStmt(8)
-  // CHECK: Match CompoundStmt(9) to CompoundStmt(9)
-  // CHECK: Match NullStmt(10) to NullStmt(10)
+  // CHECK: Match CompoundStmt(10) to CompoundStmt(10)
+  // CHECK-NEXT: Move CompoundStmt
+  // CHECK-NEXT: Match CompoundStmt({{.*}}) to CompoundStmt({{.*}})
+  // CHECK-NEXT: Match NullStmt({{.*}}) to NullStmt({{.*}})
   {{;;}}
 }
 
@@ -45,22 +47,22 @@
   {;}
 
   {{;;}}
-  // CHECK-NOT: Match {{.*}} to CompoundStmt(11)
-  // CHECK-NOT: Match {{.*}} to CompoundStmt(12)
-  // CHECK-NOT: Match {{.*}} to NullStmt(13)
+  // CHECK-NOT: Match {{.*}} to CompoundStmt(15)
+  // CHECK-NOT: Match {{.*}} to CompoundStmt(16)
+  // CHECK-NOT: Match {{.*}} to NullStmt(17)
   {{;;}}
 
-  // CHECK-NOT: Match {{.*}} to NullStmt(14)
+  // CHECK-NOT: Match {{.*}} to NullStmt(18)
   ;
 }
 
 int x;
 
 namespace dst {
   int x;
-  // CHECK: Match DeclRefExpr(17) to DeclRefExpr(22)
+  // CHECK: Match DeclRefExpr(22) to DeclRefExpr(27)
   int x1 = x + 1;
-  // CHECK: Match DeclRefExpr(21) to DeclRefExpr(26)
+  // CHECK: Match DeclRefExpr
   int x2 = ::x + 1;
 }
 
Index: test/Tooling/clang-diff-opt.cpp
===
--- test/Tooling/clang-diff-opt.cpp
+++ test/Tooling/clang-diff-opt.cpp
@@ -20,25 +20,25 @@
 void f1() {
 // Jaccard similarity = 3 / (5 + 4 - 3) = 3 / 6 >= 0.5
 // The optimal matching algorithm should move the ; into the outer block
-// CHECK: Match CompoundStmt(2) to CompoundStmt(2)
+// CHECK: Match CompoundStmt(4) to CompoundStmt(4)
 // CHECK-NOT: Match CompoundStmt(3)
-// CHECK-NEXT: Match NullStmt(4) to NullStmt(3)
+// CHECK-NEXT: Match NullStmt(6) to NullStmt(5)
   ; {{;}}
 }
 
 void f2() {
   // Jaccard similarity = 7 / (10 + 10 - 7) >= 0.5
   // As none of the subtrees is bigger than 10 nodes, the optimal algorithm
   // will be run.
-  // CHECK: Match NullStmt(11) to NullStmt(9)
+  // CHECK: Match NullStmt(15) to NullStmt(13)
   ;; {{;}}
 }
 
 void f3() {
   // Jaccard similarity = 8 / (11 + 11 - 8) >= 0.5
   // As the subtrees are bigger than 10 nodes, the optimal algorithm will not
   // be run.
-  // CHECK: Delete NullStmt(22)
+  // CHECK: Delete NullStmt(28)
   ;; {{;;}}
 }
  
Index: test/Tooling/clang-diff-json.cpp
===
--- test/Tooling/clang-diff-json.cpp
+++ test/Tooling/clang-diff-json.cpp
@@ -16,8 +16,8 @@
 // CHECK-NEXT: "children": []
 // CHECK-NEXT: "end":
 // CHECK-NEXT: "id":
-// CHECK-NEXT: "type": "CharacterLiteral"
-// CHECK-NEXT: }
+// CHECK-NEXT: "type": "TypeLoc"
+// CHECK: }
 // CHECK: ]
 // CHECK: "type": "VarDecl"
 char nl = '\n';
Index: test/Tooling/clang-diff-html.test
===
--- test/Tooling/clang-diff-html.test
+++ test/Tooling/clang-diff-html.test
@@ -1,4 +1,4 @@
-RUN: clang-diff -html %S/Inputs/clang-diff-basic-src.cpp %S/clang-diff-basic.cpp -- -std=c++11 | FileCheck %s
+RUN: clang-diff -html %S/Inputs/clang-diff-basic-src.cpp %S/clang-diff-basic.cpp -s=200 -- -std=c++11 | FileCheck %s
 
 CHECK: 
@@ -9,11 +9,15 @@
 
 match, move
 CHECK: void foo()
+CHECK-NEXT: [[L]] -> [[R]]' class='m'>{{.*}} title='TypeLoc
+CHECK-NEXT: TypeLoc
+CHECK-NEXT: void foo()
 
 match
 CHECK: void main()
+CHECK-NEXT: TypeLoc
+CHECK-NEXT: TypeLoc
+CHECK-NEXT: void main()
 
 deletion
 CHECK: 

[PATCH] D39659: [clang-diff] Store changes within ASTDiff::Impl

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision.
Herald added a subscriber: klimek.

This way we avoid modifying SyntaxTree::Impl except during
construction.


https://reviews.llvm.org/D39659

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  lib/Tooling/ASTDiff/ASTDiff.cpp
  tools/clang-diff/ClangDiff.cpp

Index: tools/clang-diff/ClangDiff.cpp
===
--- tools/clang-diff/ClangDiff.cpp
+++ tools/clang-diff/ClangDiff.cpp
@@ -298,10 +298,11 @@
   OS << "";
 
   for (diff::NodeRef Child : Node)
@@ -399,7 +400,8 @@
diff::SyntaxTree &SrcTree, diff::SyntaxTree &DstTree,
diff::NodeRef Dst) {
   const diff::Node *Src = Diff.getMapped(Dst);
-  switch (Dst.Change) {
+  diff::ChangeKind Change = Diff.getNodeChange(Dst);
+  switch (Change) {
   case diff::NoChange:
 break;
   case diff::Delete:
@@ -412,11 +414,11 @@
   case diff::Insert:
   case diff::Move:
   case diff::UpdateMove:
-if (Dst.Change == diff::Insert)
+if (Change == diff::Insert)
   OS << "Insert";
-else if (Dst.Change == diff::Move)
+else if (Change == diff::Move)
   OS << "Move";
-else if (Dst.Change == diff::UpdateMove)
+else if (Change == diff::UpdateMove)
   OS << "Update and Move";
 OS << " ";
 printNode(OS, DstTree, Dst);
Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -32,12 +32,22 @@
   return (N1.isMacro() && N2.isMacro()) || N1.getType().isSame(N2.getType());
 }
 
+namespace {
+struct NodeChange {
+  ChangeKind Change = NoChange;
+  int Shift = 0;
+  NodeChange() = default;
+  NodeChange(ChangeKind Change, int Shift) : Change(Change), Shift(Shift) {}
+};
+} // end anonymous namespace
+
 class ASTDiff::Impl {
 private:
   std::unique_ptr SrcToDst, DstToSrc;
 
 public:
   SyntaxTree::Impl &T1, &T2;
+  std::map ChangesT1, ChangesT2;
 
   Impl(SyntaxTree::Impl &T1, SyntaxTree::Impl &T2,
const ComparisonOptions &Options);
@@ -50,6 +60,8 @@
 
   const Node *getMapped(NodeRef N) const;
 
+  ChangeKind getNodeChange(NodeRef N) const;
+
 private:
   // Adds a mapping between two nodes.
   void link(NodeRef N1, NodeRef N2);
@@ -150,8 +162,8 @@
   PreorderIterator end() const { return begin() + getSize(); }
 
   NodeRef getNode(NodeId Id) const { return Nodes[Id]; }
-  Node &getMutableNode(NodeId Id) { return Nodes[Id]; }
-  Node &getMutableNode(NodeRef N) { return getMutableNode(N.getId()); }
+  Node &getMutableNode(NodeRef N) { return Nodes[N.getId()]; }
+  Node &getMutableNode(NodeId Id) { return getMutableNode(getNode(Id)); }
 
 private:
   void initTree();
@@ -1091,40 +1103,35 @@
 
 void ASTDiff::Impl::computeChangeKinds() {
   for (NodeRef N1 : T1) {
-if (!getDst(N1)) {
-  T1.getMutableNode(N1).Change = Delete;
-  T1.getMutableNode(N1).Shift -= 1;
-}
+if (!getDst(N1))
+  ChangesT1.emplace(N1.getId(), NodeChange(Delete, -1));
   }
   for (NodeRef N2 : T2) {
-if (!getSrc(N2)) {
-  T2.getMutableNode(N2).Change = Insert;
-  T2.getMutableNode(N2).Shift -= 1;
-}
+if (!getSrc(N2))
+  ChangesT2.emplace(N2.getId(), NodeChange(Insert, -1));
   }
   for (NodeRef N1 : T1.NodesBfs) {
 if (!getDst(N1))
   continue;
 NodeRef N2 = *getDst(N1);
 if (!haveSameParents(N1, N2) ||
 findNewPosition(N1) != findNewPosition(N2)) {
-  T1.getMutableNode(N1).Shift -= 1;
-  T2.getMutableNode(N2).Shift -= 1;
+  ChangesT1[N1.getId()].Shift -= 1;
+  ChangesT2[N2.getId()].Shift -= 1;
 }
   }
   for (NodeRef N2 : T2.NodesBfs) {
 if (!getSrc(N2))
   continue;
 NodeRef N1 = *getSrc(N2);
-Node &MutableN1 = T1.getMutableNode(N1);
-Node &MutableN2 = T2.getMutableNode(N2);
 if (!haveSameParents(N1, N2) ||
 findNewPosition(N1) != findNewPosition(N2)) {
-  MutableN1.Change = MutableN2.Change = Move;
+  ChangesT1[N1.getId()].Change = ChangesT2[N2.getId()].Change = Move;
 }
 if (areNodesDifferent(N1, N2)) {
-  MutableN1.Change = MutableN2.Change =
-  (N1.Change == Move ? UpdateMove : Update);
+  bool Moved = ChangesT1[N1.getId()].Change == Move;
+  ChangesT1[N1.getId()].Change = ChangesT2[N2.getId()].Change =
+  (Moved ? UpdateMove : Update);
 }
   }
 }
@@ -1152,17 +1159,37 @@
 }
 
 int ASTDiff::Impl::findNewPosition(NodeRef N) const {
+  const std::map *Changes;
+  if (&N.Tree == &T1)
+Changes = &ChangesT1;
+  else
+Changes = &ChangesT2;
+
   if (!N.getParent())
 return 0;
   int Position = N.findPositionInParent();
   for (NodeRef Sibling : *N.getParent()) {
-Position += Sibling.Shift;
+if (Changes->count(Sibling.getId()))
+  Position += Changes->at(Sibling.getId()).Shift;
 if (&Sibling == &N)
   return Position;
   }
   llvm_unreachable("Node not found amongst parent's children.");
 }
 
+ChangeKind ASTD

[PATCH] D39660: [clang-diff] Don't ignore nodes from other files

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision.
Herald added a subscriber: klimek.

https://reviews.llvm.org/D39660

Files:
  lib/Tooling/ASTDiff/ASTDiff.cpp
  test/Tooling/clang-diff-ast.cpp
  tools/clang-diff/ClangDiff.cpp


Index: tools/clang-diff/ClangDiff.cpp
===
--- tools/clang-diff/ClangDiff.cpp
+++ tools/clang-diff/ClangDiff.cpp
@@ -276,6 +276,10 @@
   char MyTag, OtherTag;
   diff::NodeId LeftId, RightId;
   diff::SyntaxTree &Tree = Node.getTree();
+  const SourceManager &SM = Tree.getASTContext().getSourceManager();
+  SourceLocation SLoc = Node.getSourceRange().getBegin();
+  if (SLoc.isValid() && !SM.isInMainFile(SLoc))
+return Offset;
   const diff::Node *Target = Diff.getMapped(Node);
   diff::NodeId TargetId = Target ? Target->getId() : diff::NodeId();
   if (IsLeft) {
@@ -291,7 +295,6 @@
   }
   unsigned Begin, End;
   std::tie(Begin, End) = Node.getSourceRangeOffsets();
-  const SourceManager &SM = Tree.getASTContext().getSourceManager();
   auto Code = SM.getBuffer(SM.getMainFileID())->getBuffer();
   for (; Offset < Begin; ++Offset)
 printHtml(OS, Code[Offset]);
Index: test/Tooling/clang-diff-ast.cpp
===
--- test/Tooling/clang-diff-ast.cpp
+++ test/Tooling/clang-diff-ast.cpp
@@ -95,14 +95,14 @@
 #define GUARD
 // CHECK-NEXT: NamespaceDecl(93)
 namespace world {
-// nodes from other files are excluded, there should be no output here
+// CHECK-NEXT: NamespaceDecl
 #include "clang-diff-ast.cpp"
 }
-// CHECK-NEXT: FunctionDecl(94)
+// CHECK: FunctionDecl(197)
 void sentinel();
 #endif
 
-// CHECK: ClassTemplateDecl(97)
+// CHECK: ClassTemplateDecl(200)
 // CHECK-NEXT: TemplateTypeParmDecl
 // CHECK-NEXT: QualType
 // CHECK-NEXT: CXXRecordDecl
Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -203,8 +203,7 @@
 return true;
   SourceLocation SLoc = N->getSourceRange().getBegin();
   if (SLoc.isValid()) {
-// Ignore everything from other files.
-if (!SM.isInMainFile(SLoc))
+if (SM.isInSystemHeader(SLoc))
   return true;
 const Preprocessor &PP = AST.getPreprocessor();
 if (SLoc.isMacroID() && !PP.isAtStartOfMacroExpansion(SLoc))


Index: tools/clang-diff/ClangDiff.cpp
===
--- tools/clang-diff/ClangDiff.cpp
+++ tools/clang-diff/ClangDiff.cpp
@@ -276,6 +276,10 @@
   char MyTag, OtherTag;
   diff::NodeId LeftId, RightId;
   diff::SyntaxTree &Tree = Node.getTree();
+  const SourceManager &SM = Tree.getASTContext().getSourceManager();
+  SourceLocation SLoc = Node.getSourceRange().getBegin();
+  if (SLoc.isValid() && !SM.isInMainFile(SLoc))
+return Offset;
   const diff::Node *Target = Diff.getMapped(Node);
   diff::NodeId TargetId = Target ? Target->getId() : diff::NodeId();
   if (IsLeft) {
@@ -291,7 +295,6 @@
   }
   unsigned Begin, End;
   std::tie(Begin, End) = Node.getSourceRangeOffsets();
-  const SourceManager &SM = Tree.getASTContext().getSourceManager();
   auto Code = SM.getBuffer(SM.getMainFileID())->getBuffer();
   for (; Offset < Begin; ++Offset)
 printHtml(OS, Code[Offset]);
Index: test/Tooling/clang-diff-ast.cpp
===
--- test/Tooling/clang-diff-ast.cpp
+++ test/Tooling/clang-diff-ast.cpp
@@ -95,14 +95,14 @@
 #define GUARD
 // CHECK-NEXT: NamespaceDecl(93)
 namespace world {
-// nodes from other files are excluded, there should be no output here
+// CHECK-NEXT: NamespaceDecl
 #include "clang-diff-ast.cpp"
 }
-// CHECK-NEXT: FunctionDecl(94)
+// CHECK: FunctionDecl(197)
 void sentinel();
 #endif
 
-// CHECK: ClassTemplateDecl(97)
+// CHECK: ClassTemplateDecl(200)
 // CHECK-NEXT: TemplateTypeParmDecl
 // CHECK-NEXT: QualType
 // CHECK-NEXT: CXXRecordDecl
Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -203,8 +203,7 @@
 return true;
   SourceLocation SLoc = N->getSourceRange().getBegin();
   if (SLoc.isValid()) {
-// Ignore everything from other files.
-if (!SM.isInMainFile(SLoc))
+if (SM.isInSystemHeader(SLoc))
   return true;
 const Preprocessor &PP = AST.getPreprocessor();
 if (SLoc.isMacroID() && !PP.isAtStartOfMacroExpansion(SLoc))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D39661: [clang-diff] Move printing of changes to the ASTDiff library

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision.
Herald added a subscriber: klimek.

https://reviews.llvm.org/D39661

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  lib/Tooling/ASTDiff/ASTDiff.cpp
  tools/clang-diff/ClangDiff.cpp

Index: tools/clang-diff/ClangDiff.cpp
===
--- tools/clang-diff/ClangDiff.cpp
+++ tools/clang-diff/ClangDiff.cpp
@@ -384,57 +384,15 @@
   OS << "]}";
 }
 
-static void printNode(raw_ostream &OS, diff::SyntaxTree &Tree,
-  diff::NodeRef Node) {
-  OS << Node.getTypeLabel();
-  OS << "(" << Node.getId() << ")";
-}
-
 static void printTree(raw_ostream &OS, diff::SyntaxTree &Tree) {
   for (diff::NodeRef Node : Tree) {
 for (int I = 0; I < Node.Depth; ++I)
   OS << " ";
-printNode(OS, Tree, Node);
+Node.dump(OS);
 OS << "\n";
   }
 }
 
-static void printDstChange(raw_ostream &OS, diff::ASTDiff &Diff,
-   diff::SyntaxTree &SrcTree, diff::SyntaxTree &DstTree,
-   diff::NodeRef Dst) {
-  const diff::Node *Src = Diff.getMapped(Dst);
-  diff::ChangeKind Change = Diff.getNodeChange(Dst);
-  switch (Change) {
-  case diff::NoChange:
-break;
-  case diff::Delete:
-llvm_unreachable("The destination tree can't have deletions.");
-  case diff::Update:
-OS << "Update ";
-printNode(OS, SrcTree, *Src);
-OS << "\n";
-break;
-  case diff::Insert:
-  case diff::Move:
-  case diff::UpdateMove:
-if (Change == diff::Insert)
-  OS << "Insert";
-else if (Change == diff::Move)
-  OS << "Move";
-else if (Change == diff::UpdateMove)
-  OS << "Update and Move";
-OS << " ";
-printNode(OS, DstTree, Dst);
-OS << " into ";
-if (!Dst.getParent())
-  OS << "None";
-else
-  printNode(OS, DstTree, *Dst.getParent());
-OS << " at " << Dst.findPositionInParent() << "\n";
-break;
-  }
-}
-
 int main(int argc, const char **argv) {
   std::string ErrorMessage;
   std::unique_ptr CommonCompilations =
@@ -509,24 +467,7 @@
 return 0;
   }
 
-  for (diff::NodeRef Dst : DstTree) {
-const diff::Node *Src = Diff.getMapped(Dst);
-if (PrintMatches && Src) {
-  llvm::outs() << "Match ";
-  printNode(llvm::outs(), SrcTree, *Src);
-  llvm::outs() << " to ";
-  printNode(llvm::outs(), DstTree, Dst);
-  llvm::outs() << "\n";
-}
-printDstChange(llvm::outs(), Diff, SrcTree, DstTree, Dst);
-  }
-  for (diff::NodeRef Src : SrcTree) {
-if (!Diff.getMapped(Src)) {
-  llvm::outs() << "Delete ";
-  printNode(llvm::outs(), SrcTree, Src);
-  llvm::outs() << "\n";
-}
-  }
+  Diff.dumpChanges(llvm::outs(), PrintMatches);
 
   return 0;
 }
Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -31,6 +31,27 @@
 bool ComparisonOptions::isMatchingAllowed(NodeRef N1, NodeRef N2) const {
   return (N1.isMacro() && N2.isMacro()) || N1.getType().isSame(N2.getType());
 }
+void printChangeKind(raw_ostream &OS, ChangeKind Kind) {
+  switch (Kind) {
+  case NoChange:
+break;
+  case Delete:
+OS << "Delete";
+break;
+  case Update:
+OS << "Update";
+break;
+  case Insert:
+OS << "Insert";
+break;
+  case Move:
+OS << "Move";
+break;
+  case UpdateMove:
+OS << "Update and Move";
+break;
+  }
+}
 
 namespace {
 struct NodeChange {
@@ -62,6 +83,8 @@
 
   ChangeKind getNodeChange(NodeRef N) const;
 
+  void dumpChanges(raw_ostream &OS, bool DumpMatches) const;
+
 private:
   // Adds a mapping between two nodes.
   void link(NodeRef N1, NodeRef N2);
@@ -1203,6 +1226,62 @@
   return DiffImpl->getNodeChange(N);
 }
 
+static void dumpDstChange(raw_ostream &OS, const ASTDiff::Impl &Diff,
+  SyntaxTree::Impl &SrcTree, SyntaxTree::Impl &DstTree,
+  NodeRef Dst) {
+  const Node *Src = Diff.getMapped(Dst);
+  ChangeKind Change = Diff.getNodeChange(Dst);
+  printChangeKind(OS, Change);
+  switch (Change) {
+  case NoChange:
+break;
+  case Delete:
+llvm_unreachable("The destination tree can't have deletions.");
+  case Update:
+OS << " ";
+Src->dump(OS);
+OS << "\n";
+break;
+  case Insert:
+  case Move:
+  case UpdateMove:
+OS << " ";
+Dst.dump(OS);
+OS << " into ";
+if (!Dst.getParent())
+  OS << "None";
+else
+  Dst.getParent()->dump(OS);
+OS << " at " << Dst.findPositionInParent() << "\n";
+break;
+  }
+}
+
+void ASTDiff::Impl::dumpChanges(raw_ostream &OS, bool DumpMatches) const {
+  for (NodeRef N2 : T2) {
+const Node *N1 = getMapped(N2);
+if (DumpMatches && N1) {
+  OS << "Match ";
+  N1->dump(OS);
+  OS << " to ";
+  N2.dump(OS);
+  OS << "\n";
+}
+dumpDstChange(OS, *this, T1, T2, N2);
+  }
+  for (NodeRef N1 : T1) {
+if (!getMapped(N1)) {
+  OS << "Delete ";
+  N1.dump

[PATCH] D39662: [clang-diff] Enable postorder traversal

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision.
Herald added a subscriber: klimek.

https://reviews.llvm.org/D39662

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  lib/Tooling/ASTDiff/ASTDiff.cpp

Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -173,16 +173,21 @@
   /// Nodes in preorder.
   std::vector Nodes;
   NodeList Leaves;
-  // Maps preorder indices to postorder ones.
-  std::vector PostorderIds;
+  std::vector PreorderToPostorderId;
   NodeList NodesBfs;
+  NodeList NodesPostorder;
   std::map TemplateArgumentLocations;
 
   int getSize() const { return Nodes.size(); }
   NodeRef getRoot() const { return getNode(getRootId()); }
   NodeId getRootId() const { return 0; }
   PreorderIterator begin() const { return &getRoot(); }
   PreorderIterator end() const { return begin() + getSize(); }
+  PostorderIterator postorder_begin() const { return NodesPostorder.begin(); }
+  PostorderIterator postorder_end() const { return NodesPostorder.end(); }
+  llvm::iterator_range postorder() const {
+return {postorder_begin(), postorder_end()};
+  }
 
   NodeRef getNode(NodeId Id) const { return Nodes[Id]; }
   Node &getMutableNode(NodeRef N) { return Nodes[N.getId()]; }
@@ -199,13 +204,23 @@
 NodeRefIterator &NodeRefIterator::operator+(int Offset) {
   return IdPointer += Offset, *this;
 }
+NodeRefIterator::difference_type NodeRefIterator::
+operator-(const NodeRefIterator &Other) const {
+  assert(Tree == Other.Tree &&
+ "Cannot subtract two iterators of different trees.");
+  return IdPointer - Other.IdPointer;
+}
 
 bool NodeRefIterator::operator!=(const NodeRefIterator &Other) const {
   assert(Tree == Other.Tree &&
  "Cannot compare two iterators of different trees.");
   return IdPointer != Other.IdPointer;
 }
 
+bool NodeRefIterator::operator==(const NodeRefIterator &Other) const {
+  return !(*this != Other);
+}
+
 static bool isSpecializedNodeExcluded(const Decl *D) { return D->isImplicit(); }
 static bool isSpecializedNodeExcluded(const Stmt *S) { return false; }
 static bool isSpecializedNodeExcluded(CXXCtorInitializer *I) {
@@ -345,7 +360,7 @@
 
 SyntaxTree::Impl::Impl(SyntaxTree *Parent, ASTUnit &AST)
 : Parent(Parent), AST(AST), TypePP(AST.getLangOpts()), Leaves(*this),
-  NodesBfs(*this) {
+  NodesBfs(*this), NodesPostorder(*this) {
   TypePP.AnonymousTagLocations = false;
 }
 
@@ -363,17 +378,13 @@
   initTree();
 }
 
-static std::vector getSubtreePostorder(SyntaxTree::Impl &Tree,
-   NodeId Root) {
-  std::vector Postorder;
-  std::function Traverse = [&](NodeId Id) {
-NodeRef N = Tree.getNode(Id);
-for (NodeId Child : N.Children)
+static void getSubtreePostorder(NodeList &Ids, NodeRef Root) {
+  std::function Traverse = [&](NodeRef N) {
+for (NodeRef Child : N)
   Traverse(Child);
-Postorder.push_back(Id);
+Ids.push_back(N.getId());
   };
   Traverse(Root);
-  return Postorder;
 }
 
 static void getSubtreeBfs(NodeList &Ids, NodeRef Root) {
@@ -387,15 +398,16 @@
 void SyntaxTree::Impl::initTree() {
   setLeftMostDescendants();
   int PostorderId = 0;
-  PostorderIds.resize(getSize());
+  PreorderToPostorderId.resize(getSize());
   std::function PostorderTraverse = [&](NodeRef N) {
 for (NodeRef Child : N)
   PostorderTraverse(Child);
-PostorderIds[N.getId()] = PostorderId;
+PreorderToPostorderId[N.getId()] = PostorderId;
 ++PostorderId;
   };
   PostorderTraverse(getRoot());
   getSubtreeBfs(NodesBfs, getRoot());
+  getSubtreePostorder(NodesPostorder, getRoot());
 }
 
 void SyntaxTree::Impl::setLeftMostDescendants() {
@@ -459,32 +471,29 @@
 private:
   /// The parent tree.
   SyntaxTree::Impl &Tree;
-  /// Maps SNodeIds to original ids.
-  std::vector RootIds;
-  /// Maps subtree nodes to their leftmost descendants wtihin the subtree.
+  NodeRef Root;
+  /// Maps subtree nodes to their leftmost descendants wihin the subtree.
   std::vector LeftMostDescendants;
 
 public:
   std::vector KeyRoots;
 
-  Subtree(SyntaxTree::Impl &Tree, NodeId SubtreeRoot) : Tree(Tree) {
-RootIds = getSubtreePostorder(Tree, SubtreeRoot);
+  Subtree(NodeRef Root) : Tree(Root.Tree), Root(Root) {
 int NumLeaves = setLeftMostDescendants();
 computeKeyRoots(NumLeaves);
   }
-  int getSize() const { return RootIds.size(); }
+  int getSize() const { return getNumberOfDescendants(Root); }
   NodeId getIdInRoot(SNodeId Id) const {
-assert(Id > 0 && Id <= getSize() && "Invalid subtree node index.");
-return RootIds[Id - 1];
+return Tree.NodesPostorder[getPostorderIdInRoot(Id)].getId();
   }
   NodeRef getNode(SNodeId Id) const { return Tree.getNode(getIdInRoot(Id)); }
   SNodeId getLeftMostDescendant(SNodeId Id) const {
 assert(Id > 0 && Id <= getSize() && "Invalid subtree node index.");
 return LeftMostDescendants[Id - 1];
   }
-  /// Returns the postorder 

[PATCH] D37005: [clang-diff] Initial implementation of patching

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes updated this revision to Diff 121656.
johannes added a comment.

update


https://reviews.llvm.org/D37005

Files:
  include/clang/Tooling/ASTDiff/ASTDiff.h
  include/clang/Tooling/ASTDiff/ASTPatch.h
  lib/Tooling/ASTDiff/ASTDiff.cpp
  lib/Tooling/ASTDiff/ASTPatch.cpp
  lib/Tooling/ASTDiff/CMakeLists.txt
  test/Tooling/clang-diff-patch.test
  tools/clang-diff/CMakeLists.txt
  tools/clang-diff/ClangDiff.cpp
  unittests/Tooling/ASTPatchTest.cpp
  unittests/Tooling/CMakeLists.txt

Index: unittests/Tooling/CMakeLists.txt
===
--- unittests/Tooling/CMakeLists.txt
+++ unittests/Tooling/CMakeLists.txt
@@ -11,6 +11,7 @@
 endif()
 
 add_clang_unittest(ToolingTests
+  ASTPatchTest.cpp
   ASTSelectionTest.cpp
   CastExprTest.cpp
   CommentHandlerTest.cpp
@@ -45,4 +46,5 @@
   clangTooling
   clangToolingCore
   clangToolingRefactor
+  clangToolingASTDiff
   )
Index: unittests/Tooling/ASTPatchTest.cpp
===
--- /dev/null
+++ unittests/Tooling/ASTPatchTest.cpp
@@ -0,0 +1,265 @@
+//===- unittest/Tooling/ASTPatchTest.cpp --===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/Tooling/ASTDiff/ASTPatch.h"
+#include "clang/Tooling/ASTDiff/ASTDiff.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Program.h"
+#include "gtest/gtest.h"
+#include 
+
+using namespace clang;
+using namespace tooling;
+
+std::string ReadShellCommand(const Twine &Command) {
+  char Buffer[128];
+  std::string Result;
+  std::shared_ptr Pipe(popen(Command.str().data(), "r"), pclose);
+  if (!Pipe)
+return Result;
+  while (!feof(Pipe.get())) {
+if (fgets(Buffer, 128, Pipe.get()) != nullptr)
+  Result += Buffer;
+  }
+  return Result;
+}
+
+class ASTPatchTest : public ::testing::Test {
+  llvm::SmallString<256> TargetFile, ExpectedFile;
+  std::array TargetFileArray;
+
+public:
+  void SetUp() override {
+std::string Suffix = "cpp";
+ASSERT_FALSE(llvm::sys::fs::createTemporaryFile(
+"clang-libtooling-patch-target", Suffix, TargetFile));
+ASSERT_FALSE(llvm::sys::fs::createTemporaryFile(
+"clang-libtooling-patch-expected", Suffix, ExpectedFile));
+TargetFileArray[0] = TargetFile.str();
+  }
+  void TearDown() override {
+llvm::sys::fs::remove(TargetFile);
+llvm::sys::fs::remove(ExpectedFile);
+  }
+
+  void WriteFile(StringRef Filename, StringRef Contents) {
+std::ofstream OS(Filename);
+OS << Contents.str();
+assert(OS.good());
+  }
+
+  std::string ReadFile(StringRef Filename) {
+std::ifstream IS(Filename);
+std::stringstream OS;
+OS << IS.rdbuf();
+assert(IS.good());
+return OS.str();
+  }
+
+  std::string formatExpected(StringRef Code) {
+WriteFile(ExpectedFile, Code);
+return ReadShellCommand("clang-format " + ExpectedFile);
+  }
+
+  llvm::Expected patchResult(const char *SrcCode,
+  const char *DstCode,
+  const char *TargetCode) {
+std::unique_ptr SrcAST = buildASTFromCode(SrcCode),
+ DstAST = buildASTFromCode(DstCode);
+if (!SrcAST || !DstAST) {
+  if (!SrcAST)
+llvm::errs() << "Failed to build AST from code:\n" << SrcCode << "\n";
+  if (!DstAST)
+llvm::errs() << "Failed to build AST from code:\n" << DstCode << "\n";
+  return llvm::make_error(
+  diff::patching_error::failed_to_build_AST);
+}
+
+diff::SyntaxTree Src(*SrcAST);
+diff::SyntaxTree Dst(*DstAST);
+
+WriteFile(TargetFile, TargetCode);
+FixedCompilationDatabase Compilations(".", std::vector());
+RefactoringTool TargetTool(Compilations, TargetFileArray);
+diff::ComparisonOptions Options;
+
+if (auto Err = diff::patch(TargetTool, Src, Dst, Options, /*Debug=*/false))
+  return std::move(Err);
+return ReadShellCommand("clang-format " + TargetFile);
+  }
+
+#define APPEND_NEWLINE(x) x "\n"
+// use macros for this to make test failures have proper line numbers
+#define PATCH(Src, Dst, Target, ExpectedResult)\
+  {\
+llvm::Expected Result = patchResult(  \
+APPEND_NEWLINE(Src), APPEND_NEWLINE(Dst), APPEND_NEWLINE(Target)); \
+ASSERT_TRUE(bool(Result)); \
+EXPECT_EQ(Result.get(), formatExpected(APPEND_NEWLINE(ExpectedResult)));   \
+  }
+#define PATCH_ERROR(Src, Dst, Target, ErrorCode)   \
+  {

[PATCH] D39663: [clang-diff] Split source ranges at points where nodes are inserted.

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision.
Herald added a subscriber: klimek.

This enables inserting into empty CompoundStmt nodes, for example.
Previously the insertion would occur only after the closing parenthesis.


https://reviews.llvm.org/D39663

Files:
  lib/Tooling/ASTDiff/ASTPatch.cpp
  unittests/Tooling/ASTPatchTest.cpp


Index: unittests/Tooling/ASTPatchTest.cpp
===
--- unittests/Tooling/ASTPatchTest.cpp
+++ unittests/Tooling/ASTPatchTest.cpp
@@ -262,4 +262,8 @@
 R"(void f() { })",
 R"(void f() { { int x = 2; } })",
 R"(void f() {  })");
+  PATCH(R"(void f() {;;} namespace {})",
+R"(namespace { void f() {;;} })",
+R"(void g() {;;} namespace {})",
+R"(namespace { void g() {;;} })");
 }
Index: lib/Tooling/ASTDiff/ASTPatch.cpp
===
--- lib/Tooling/ASTDiff/ASTPatch.cpp
+++ lib/Tooling/ASTDiff/ASTPatch.cpp
@@ -79,12 +79,19 @@
 
   void addInsertion(PatchedTreeNode &PatchedNode, SourceLocation InsertionLoc) 
{
 addChildAt(PatchedNode, InsertionLoc);
+SplitAt.emplace_back(InsertionLoc);
   }
   void addChild(PatchedTreeNode &PatchedNode) {
 SourceLocation InsertionLoc = PatchedNode.getSourceRange().getBegin();
 addChildAt(PatchedNode, InsertionLoc);
   }
 
+  // This returns an array of source ranges identical to the input,
+  // except whenever a SourceRange contains any of the locations in
+  // SplitAt, it is split up in two ranges at that location.
+  SmallVector
+  splitSourceRanges(ArrayRef SourceRanges);
+
 private:
   void addChildAt(PatchedTreeNode &PatchedNode, SourceLocation InsertionLoc) {
 auto Less = makeTolerantLess(getTree().getSourceManager());
@@ -94,6 +101,8 @@
 Children.insert(Children.begin() + Offset, &PatchedNode);
 ChildrenLocations.insert(It, InsertionLoc);
   }
+
+  SmallVector SplitAt;
 };
 } // end anonymous namespace
 
@@ -501,7 +510,7 @@
   unsigned ChildIndex = 0;
   auto MySourceRanges = PatchedNode.getOwnedSourceRanges();
   BeforeThanCompare MyLess(Tree.getSourceManager());
-  for (auto &MySubRange : MySourceRanges) {
+  for (auto &MySubRange : PatchedNode.splitSourceRanges(MySourceRanges)) {
 SourceLocation ChildBegin;
 SourceLocation InsertionBegin;
 while (ChildIndex < NumChildren &&
@@ -555,6 +564,36 @@
   return {-1, true};
 }
 
+static bool onlyWhitespace(StringRef Str) {
+  return std::all_of(Str.begin(), Str.end(),
+ [](char C) { return std::isspace(C); });
+}
+
+SmallVector
+PatchedTreeNode::splitSourceRanges(ArrayRef SourceRanges) {
+  SourceManager &SM = getTree().getSourceManager();
+  const LangOptions &LangOpts = getTree().getLangOpts();
+  SmallVector Result;
+  BeforeThanCompare Less(SM);
+  std::sort(SplitAt.begin(), SplitAt.end(), Less);
+  for (auto &Range : SourceRanges) {
+SourceLocation Begin = Range.getBegin(), End = Range.getEnd();
+for (auto SplitPoint : SplitAt) {
+  if (SM.isPointWithin(SplitPoint, Begin, End)) {
+auto SplitRange = CharSourceRange::getCharRange(Begin, SplitPoint);
+StringRef Text = Lexer::getSourceText(SplitRange, SM, LangOpts);
+if (onlyWhitespace(Text))
+  continue;
+Result.emplace_back(SplitRange);
+Begin = SplitPoint;
+  }
+}
+if (Less(Begin, End))
+  Result.emplace_back(CharSourceRange::getCharRange(Begin, End));
+  }
+  return Result;
+}
+
 Error patch(RefactoringTool &TargetTool, SyntaxTree &Src, SyntaxTree &Dst,
 const ComparisonOptions &Options, bool Debug) {
   std::vector> TargetASTs;


Index: unittests/Tooling/ASTPatchTest.cpp
===
--- unittests/Tooling/ASTPatchTest.cpp
+++ unittests/Tooling/ASTPatchTest.cpp
@@ -262,4 +262,8 @@
 R"(void f() { })",
 R"(void f() { { int x = 2; } })",
 R"(void f() {  })");
+  PATCH(R"(void f() {;;} namespace {})",
+R"(namespace { void f() {;;} })",
+R"(void g() {;;} namespace {})",
+R"(namespace { void g() {;;} })");
 }
Index: lib/Tooling/ASTDiff/ASTPatch.cpp
===
--- lib/Tooling/ASTDiff/ASTPatch.cpp
+++ lib/Tooling/ASTDiff/ASTPatch.cpp
@@ -79,12 +79,19 @@
 
   void addInsertion(PatchedTreeNode &PatchedNode, SourceLocation InsertionLoc) {
 addChildAt(PatchedNode, InsertionLoc);
+SplitAt.emplace_back(InsertionLoc);
   }
   void addChild(PatchedTreeNode &PatchedNode) {
 SourceLocation InsertionLoc = PatchedNode.getSourceRange().getBegin();
 addChildAt(PatchedNode, InsertionLoc);
   }
 
+  // This returns an array of source ranges identical to the input,
+  // except whenever a SourceRange contains any of the locations in
+  // SplitAt, it is split up in two ranges at that location.
+  SmallVector
+  splitSourceRanges(ArrayRef SourceRanges);
+
 private:
   void addChildAt(PatchedTreeNode &Patc

[PATCH] D39664: [clang-diff] patching: implement updates

2017-11-05 Thread Johannes Altmanninger via Phabricator via cfe-commits
johannes created this revision.
Herald added a subscriber: klimek.

This allows to patch nodes that have been updated. We use a LCS
algorithm to determine which tokens changed, and update accordingly.


https://reviews.llvm.org/D39664

Files:
  lib/Tooling/ASTDiff/ASTPatch.cpp
  unittests/Tooling/ASTPatchTest.cpp

Index: unittests/Tooling/ASTPatchTest.cpp
===
--- unittests/Tooling/ASTPatchTest.cpp
+++ unittests/Tooling/ASTPatchTest.cpp
@@ -115,6 +115,32 @@
   }
 };
 
+TEST_F(ASTPatchTest, Constructors) {
+  PATCH(R"(class C { C() {} };)",
+R"(class C { int b; C(int b) : b(b) {} };)",
+R"(class D { D() {} };)",
+R"(class D { int b; D(int b) : b(b) {} };)");
+  PATCH(R"(class C { C() {} };)",
+R"(class C { int b; C(int b) {} };)",
+R"(class D { D() {} };)",
+R"(class D { int b;D(int b) {} };)");
+  PATCH(R"(struct C { C() {} };)",
+R"(struct C { C(int b) {} };)",
+R"(struct C { C() {} };)",
+R"(struct C { C(int b) {} };)");
+  PATCH(R"(struct C { C(int a) {} };)",
+R"(struct C { C(int a, int b) {} };)",
+R"(struct C { C(int a) {} };)",
+R"(struct C { C(int a, int b) {} };)");
+  PATCH(R"(struct C { C(int a) {} };)",
+R"(struct C { C(int b, int a) {} };)",
+R"(struct C { C(int a) {} };)",
+R"(struct C { C(int b, int a) {} };)");
+  PATCH(R"(struct C { C(int) {} };)",
+R"(struct C { C(int) {int x;} };)",
+R"(struct C { C(int) {} };)",
+R"(struct C { C(int) {int x;} };)");
+}
 TEST_F(ASTPatchTest, Delete) {
   PATCH(R"(void f() { { int x = 1; } })",
 R"(void f() { })",
@@ -130,14 +156,64 @@
 R"(void foo(...); void test1() { foo ( ); })",
 R"(void foo(...); void test2() { foo ( 1 + 1 ); })",
 R"(void foo(...); void test2() { foo (  ); })");
+  PATCH(R"(void foo(...); void test1() { foo (1, 2 + 2); })",
+R"(void foo(...); void test1() { foo (2 + 2); })",
+R"(void foo(...); void test2() { foo (/*L*/ 0 /*R*/ , 2 + 2); })",
+R"(void foo(...); void test2() { foo (/*L*/ /*R*/ 2 + 2); })");
+  PATCH(R"(void foo(...); void test1() { foo (1, 2); })",
+R"(void foo(...); void test1() { foo (1); })",
+R"(void foo(...); void test2() { foo (0, /*L*/ 0 /*R*/); })",
+R"(void foo(...); void test2() { foo (0 /*L*/ /*R*/); })");
+  PATCH(
+  R"(void printf(...); void foo(int x) { printf("%d", x, x); })",
+  R"(void printf(...); void foo(int x) { printf("%d", x); })",
+  R"(void printf(...); void foo(int x) { printf("different string %d", x, x); })",
+  R"(void printf(...); void foo(int x) { printf("different string %d", x); })");
+  PATCH(
+  R"(void printf(...); void foo(int xx) { printf("%d", xx, xx); })",
+  R"(void printf(...); void foo(int xx) { printf("%d", xx); })",
+  R"(void printf(...); void foo(int xx) { printf("different string %d", xx, xx); })",
+  R"(void printf(...); void foo(int xx) { printf("different string %d", xx); })");
 }
 TEST_F(ASTPatchTest, DeleteParmVarDecl) {
+  PATCH(R"(void f(int a, int b, int c);)",
+R"(void f(int a, int b);)",
+R"(void b(int o, int b, int c);)",
+R"(void b(int o, int b);)");
+  PATCH(R"(void f(int a, int b, int c);)",
+R"(void f(int a, int b);)",
+R"(void f(int a, int b, int c);)",
+R"(void f(int a, int b);)");
   PATCH(R"(void foo(int a);)",
 R"(void foo();)",
 R"(void bar(int x);)",
 R"(void bar();)");
+  PATCH(R"(void foo(int a);)",
+R"(void foo();)",
+R"(void bar(int x, int y);)",
+R"(void bar(int y);)");
+  PATCH(R"(void foo(int a, int b);)",
+R"(void foo(int a);)",
+R"(void bar(int x, int y);)",
+R"(void bar(int x);)");
+  PATCH(R"(void foo(int a, int b, int c);)",
+R"(void foo(int a, int b);)",
+R"(void bar(int a, int b, int c);)",
+R"(void bar(int a, int b);)");
 }
 TEST_F(ASTPatchTest, Insert) {
+  PATCH(R"()",
+R"(int x;)",
+R"()",
+R"(int x;)");
+  PATCH(R"()",
+R"(namespace a {  })",
+R"()",
+R"(namespace a {  })");
+  PATCH(R"(class C {   };)",
+R"(class C { int b;};)",
+R"(class C { int c;};)",
+R"(class C { int c;int b;  };)");
   PATCH(R"(class C {  C() {} };)",
 R"(class C { int b;   C() {} };)",
 R"(class C { int c;   C() {} };)",
@@ -158,6 +234,10 @@
 R"(class C { int x;int b;};)",
 R"(class C { int x;int c;};)",
 R"(class C { int x;int b;int c;  };)");
+  PATCH(R"(class X { void f() { } };   )",
+R"(class X { void f() { int x; } };)",
+R"(class Y { void g() { } };   )",
+R"(class Y { void g() { int x; }}; )");
   PATCH(R"(int a;)",
 R"(int a; int x();)",
 R"(int a;)",
@@ -174,14 +254,22 @@

r317443 - Remove \brief from doxygen comments in PrettyPrinter.h

2017-11-05 Thread Adrian Prantl via cfe-commits
Author: adrian
Date: Sun Nov  5 13:52:36 2017
New Revision: 317443

URL: http://llvm.org/viewvc/llvm-project?rev=317443&view=rev
Log:
Remove \brief from doxygen comments in PrettyPrinter.h

Patch by @xsga!

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

Modified:
cfe/trunk/include/clang/AST/PrettyPrinter.h

Modified: cfe/trunk/include/clang/AST/PrettyPrinter.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/PrettyPrinter.h?rev=317443&r1=317442&r2=317443&view=diff
==
--- cfe/trunk/include/clang/AST/PrettyPrinter.h (original)
+++ cfe/trunk/include/clang/AST/PrettyPrinter.h Sun Nov  5 13:52:36 2017
@@ -30,8 +30,8 @@ public:
   virtual bool handledStmt(Stmt* E, raw_ostream& OS) = 0;
 };
 
-/// \brief Describes how types, statements, expressions, and
-/// declarations should be printed.
+/// Describes how types, statements, expressions, and declarations should be
+/// printed.
 ///
 /// This type is intended to be small and suitable for passing by value.
 /// It is very frequently copied.
@@ -53,20 +53,20 @@ struct PrintingPolicy {
   IncludeNewlines(true), MSVCFormatting(false),
   ConstantsAsWritten(false), SuppressImplicitBase(false) { }
 
-  /// \brief Adjust this printing policy for cases where it's known that
-  /// we're printing C++ code (for instance, if AST dumping reaches a
-  /// C++-only construct). This should not be used if a real LangOptions
-  /// object is available.
+  /// Adjust this printing policy for cases where it's known that we're
+  /// printing C++ code (for instance, if AST dumping reaches a C++-only
+  /// construct). This should not be used if a real LangOptions object is
+  /// available.
   void adjustForCPlusPlus() {
 SuppressTagKeyword = true;
 Bool = true;
 UseVoidForZeroParams = false;
   }
 
-  /// \brief The number of spaces to use to indent each line.
+  /// The number of spaces to use to indent each line.
   unsigned Indentation : 8;
 
-  /// \brief Whether we should suppress printing of the actual specifiers for
+  /// Whether we should suppress printing of the actual specifiers for
   /// the given type or declaration.
   ///
   /// This flag is only used when we are printing declarators beyond
@@ -82,7 +82,7 @@ struct PrintingPolicy {
   /// "const int" type specifier and instead only print the "*y".
   bool SuppressSpecifiers : 1;
 
-  /// \brief Whether type printing should skip printing the tag keyword.
+  /// Whether type printing should skip printing the tag keyword.
   ///
   /// This is used when printing the inner type of elaborated types,
   /// (as the tag keyword is part of the elaborated type):
@@ -92,7 +92,7 @@ struct PrintingPolicy {
   /// \endcode
   bool SuppressTagKeyword : 1;
 
-  /// \brief When true, include the body of a tag definition.
+  /// When true, include the body of a tag definition.
   ///
   /// This is used to place the definition of a struct
   /// in the middle of another declaration as with:
@@ -102,14 +102,14 @@ struct PrintingPolicy {
   /// \endcode
   bool IncludeTagDefinition : 1;
 
-  /// \brief Suppresses printing of scope specifiers.
+  /// Suppresses printing of scope specifiers.
   bool SuppressScope : 1;
 
-  /// \brief Suppress printing parts of scope specifiers that don't need
+  /// Suppress printing parts of scope specifiers that don't need
   /// to be written, e.g., for inline or anonymous namespaces.
   bool SuppressUnwrittenScope : 1;
   
-  /// \brief Suppress printing of variable initializers.
+  /// Suppress printing of variable initializers.
   ///
   /// This flag is used when printing the loop variable in a for-range
   /// statement. For example, given:
@@ -122,8 +122,8 @@ struct PrintingPolicy {
   /// internal initializer constructed for x will not be printed.
   bool SuppressInitializers : 1;
 
-  /// \brief Whether we should print the sizes of constant array expressions
-  /// as written in the sources.
+  /// Whether we should print the sizes of constant array expressions as 
written
+  /// in the sources.
   ///
   /// This flag determines whether array types declared as
   ///
@@ -140,69 +140,67 @@ struct PrintingPolicy {
   /// \endcode
   bool ConstantArraySizeAsWritten : 1;
   
-  /// \brief When printing an anonymous tag name, also print the location of
-  /// that entity (e.g., "enum "). Otherwise, just 
-  /// prints "(anonymous)" for the name.
+  /// When printing an anonymous tag name, also print the location of that
+  /// entity (e.g., "enum "). Otherwise, just prints
+  /// "(anonymous)" for the name.
   bool AnonymousTagLocations : 1;
   
-  /// \brief When true, suppress printing of the __strong lifetime qualifier in
-  /// ARC.
+  /// When true, suppress printing of the __strong lifetime qualifier in ARC.
   unsigned SuppressStrongLifetime : 1;
   
-  /// \brief When true, suppress printing of lifetime qualifier in
-  /// ARC.
+  /// When true, suppress prin

[PATCH] D39633: Remove \brief from doxygen comments in PrettyPrinter.h

2017-11-05 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL317443: Remove \brief from doxygen comments in 
PrettyPrinter.h (authored by adrian).

Changed prior to commit:
  https://reviews.llvm.org/D39633?vs=121603&id=121663#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39633

Files:
  cfe/trunk/include/clang/AST/PrettyPrinter.h

Index: cfe/trunk/include/clang/AST/PrettyPrinter.h
===
--- cfe/trunk/include/clang/AST/PrettyPrinter.h
+++ cfe/trunk/include/clang/AST/PrettyPrinter.h
@@ -30,8 +30,8 @@
   virtual bool handledStmt(Stmt* E, raw_ostream& OS) = 0;
 };
 
-/// \brief Describes how types, statements, expressions, and
-/// declarations should be printed.
+/// Describes how types, statements, expressions, and declarations should be
+/// printed.
 ///
 /// This type is intended to be small and suitable for passing by value.
 /// It is very frequently copied.
@@ -53,20 +53,20 @@
   IncludeNewlines(true), MSVCFormatting(false),
   ConstantsAsWritten(false), SuppressImplicitBase(false) { }
 
-  /// \brief Adjust this printing policy for cases where it's known that
-  /// we're printing C++ code (for instance, if AST dumping reaches a
-  /// C++-only construct). This should not be used if a real LangOptions
-  /// object is available.
+  /// Adjust this printing policy for cases where it's known that we're
+  /// printing C++ code (for instance, if AST dumping reaches a C++-only
+  /// construct). This should not be used if a real LangOptions object is
+  /// available.
   void adjustForCPlusPlus() {
 SuppressTagKeyword = true;
 Bool = true;
 UseVoidForZeroParams = false;
   }
 
-  /// \brief The number of spaces to use to indent each line.
+  /// The number of spaces to use to indent each line.
   unsigned Indentation : 8;
 
-  /// \brief Whether we should suppress printing of the actual specifiers for
+  /// Whether we should suppress printing of the actual specifiers for
   /// the given type or declaration.
   ///
   /// This flag is only used when we are printing declarators beyond
@@ -82,7 +82,7 @@
   /// "const int" type specifier and instead only print the "*y".
   bool SuppressSpecifiers : 1;
 
-  /// \brief Whether type printing should skip printing the tag keyword.
+  /// Whether type printing should skip printing the tag keyword.
   ///
   /// This is used when printing the inner type of elaborated types,
   /// (as the tag keyword is part of the elaborated type):
@@ -92,7 +92,7 @@
   /// \endcode
   bool SuppressTagKeyword : 1;
 
-  /// \brief When true, include the body of a tag definition.
+  /// When true, include the body of a tag definition.
   ///
   /// This is used to place the definition of a struct
   /// in the middle of another declaration as with:
@@ -102,14 +102,14 @@
   /// \endcode
   bool IncludeTagDefinition : 1;
 
-  /// \brief Suppresses printing of scope specifiers.
+  /// Suppresses printing of scope specifiers.
   bool SuppressScope : 1;
 
-  /// \brief Suppress printing parts of scope specifiers that don't need
+  /// Suppress printing parts of scope specifiers that don't need
   /// to be written, e.g., for inline or anonymous namespaces.
   bool SuppressUnwrittenScope : 1;
   
-  /// \brief Suppress printing of variable initializers.
+  /// Suppress printing of variable initializers.
   ///
   /// This flag is used when printing the loop variable in a for-range
   /// statement. For example, given:
@@ -122,8 +122,8 @@
   /// internal initializer constructed for x will not be printed.
   bool SuppressInitializers : 1;
 
-  /// \brief Whether we should print the sizes of constant array expressions
-  /// as written in the sources.
+  /// Whether we should print the sizes of constant array expressions as written
+  /// in the sources.
   ///
   /// This flag determines whether array types declared as
   ///
@@ -140,69 +140,67 @@
   /// \endcode
   bool ConstantArraySizeAsWritten : 1;
   
-  /// \brief When printing an anonymous tag name, also print the location of
-  /// that entity (e.g., "enum "). Otherwise, just 
-  /// prints "(anonymous)" for the name.
+  /// When printing an anonymous tag name, also print the location of that
+  /// entity (e.g., "enum "). Otherwise, just prints
+  /// "(anonymous)" for the name.
   bool AnonymousTagLocations : 1;
   
-  /// \brief When true, suppress printing of the __strong lifetime qualifier in
-  /// ARC.
+  /// When true, suppress printing of the __strong lifetime qualifier in ARC.
   unsigned SuppressStrongLifetime : 1;
   
-  /// \brief When true, suppress printing of lifetime qualifier in
-  /// ARC.
+  /// When true, suppress printing of lifetime qualifier in ARC.
   unsigned SuppressLifetimeQualifiers : 1;
 
   /// When true, suppresses printing template arguments in names of C++
   /// constructors.
   unsigned SuppressTemplateArgsInCXXConstructors : 1;
 
-  /// \brief Whether we can use 'bool' rather

[PATCH] D39665: Support __has_c_attribute

2017-11-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman created this revision.

We have the __has_cpp_attribute builtin macro to determine when an attribute is 
supported with C++ [[]] syntax, and this patch adds the analogous 
__has_c_attribute builtin macro so that users can test whether a given [[]] 
attribute is available in C when -fdouble-square-bracket-attributes is 
specified.


https://reviews.llvm.org/D39665

Files:
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/test/Preprocessor/has_c_attribute.c


Index: clang/test/Preprocessor/has_c_attribute.c
===
--- clang/test/Preprocessor/has_c_attribute.c
+++ clang/test/Preprocessor/has_c_attribute.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fdouble-square-bracket-attributes -std=c11 -E %s -o - | 
FileCheck %s
+
+// CHECK: has_fallthrough
+#if __has_c_attribute(fallthrough)
+  int has_fallthrough();
+#endif
+
+// CHECK: does_not_have_selectany
+#if !__has_c_attribute(selectany)
+  int does_not_have_selectany();
+#endif
+
Index: clang/lib/Lex/PPMacroExpansion.cpp
===
--- clang/lib/Lex/PPMacroExpansion.cpp
+++ clang/lib/Lex/PPMacroExpansion.cpp
@@ -369,6 +369,7 @@
   Ident__has_extension= RegisterBuiltinMacro(*this, "__has_extension");
   Ident__has_builtin  = RegisterBuiltinMacro(*this, "__has_builtin");
   Ident__has_attribute= RegisterBuiltinMacro(*this, "__has_attribute");
+  Ident__has_c_attribute  = RegisterBuiltinMacro(*this, "__has_c_attribute");
   Ident__has_declspec = RegisterBuiltinMacro(*this, 
"__has_declspec_attribute");
   Ident__has_include  = RegisterBuiltinMacro(*this, "__has_include");
   Ident__has_include_next = RegisterBuiltinMacro(*this, "__has_include_next");
@@ -1775,30 +1776,34 @@
 return II ? hasAttribute(AttrSyntax::Declspec, nullptr, II,
  getTargetInfo(), getLangOpts()) : 0;
   });
-  } else if (II == Ident__has_cpp_attribute) {
-EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this,
-  [this](Token &Tok, bool &HasLexedNextToken) -> int {
-IdentifierInfo *ScopeII = nullptr;
-IdentifierInfo *II = ExpectFeatureIdentifierInfo(Tok, *this,
-   diag::err_feature_check_malformed);
-if (!II)
-  return false;
+  } else if (II == Ident__has_cpp_attribute ||
+ II == Ident__has_c_attribute) {
+bool IsCXX = II == Ident__has_cpp_attribute;
+EvaluateFeatureLikeBuiltinMacro(
+OS, Tok, II, *this, [&](Token &Tok, bool &HasLexedNextToken) -> int {
+  IdentifierInfo *ScopeII = nullptr;
+  IdentifierInfo *II = ExpectFeatureIdentifierInfo(
+  Tok, *this, diag::err_feature_check_malformed);
+  if (!II)
+return false;
 
-// It is possible to receive a scope token.  Read the "::", if it is
-// available, and the subsequent identifier.
-LexUnexpandedToken(Tok);
-if (Tok.isNot(tok::coloncolon))
-  HasLexedNextToken = true;
-else {
-  ScopeII = II;
+  // It is possible to receive a scope token.  Read the "::", if it is
+  // available, and the subsequent identifier.
   LexUnexpandedToken(Tok);
-  II = ExpectFeatureIdentifierInfo(Tok, *this,
-   diag::err_feature_check_malformed);
-}
+  if (Tok.isNot(tok::coloncolon))
+HasLexedNextToken = true;
+  else {
+ScopeII = II;
+LexUnexpandedToken(Tok);
+II = ExpectFeatureIdentifierInfo(Tok, *this,
+ 
diag::err_feature_check_malformed);
+  }
 
-return II ? hasAttribute(AttrSyntax::CXX, ScopeII, II,
- getTargetInfo(), getLangOpts()) : 0;
-  });
+  AttrSyntax Syntax = IsCXX ? AttrSyntax::CXX : AttrSyntax::C;
+  return II ? hasAttribute(Syntax, ScopeII, II, getTargetInfo(),
+   getLangOpts())
+: 0;
+});
   } else if (II == Ident__has_include ||
  II == Ident__has_include_next) {
 // The argument to these two builtins should be a parenthesized
Index: clang/include/clang/Lex/Preprocessor.h
===
--- clang/include/clang/Lex/Preprocessor.h
+++ clang/include/clang/Lex/Preprocessor.h
@@ -144,6 +144,7 @@
   IdentifierInfo *Ident__building_module;  // __building_module
   IdentifierInfo *Ident__MODULE__; // __MODULE__
   IdentifierInfo *Ident__has_cpp_attribute;// __has_cpp_attribute
+  IdentifierInfo *Ident__has_c_attribute;  // __has_c_attribute
   IdentifierInfo *Ident__has_declspec; // __has_declspec_attribute
 
   SourceLocation DATELoc, TIMELoc;


Index: clang/test/Preprocessor/has_c_attribute.c
===

[PATCH] D39502: [Driver] Make clang/cc conforms to UNIX standard

2017-11-05 Thread Justin Lebar via Phabricator via cfe-commits
jlebar requested changes to this revision.
jlebar added a comment.
This revision now requires changes to proceed.

> Note the host clang side has the inputs "test.cu" and "test-c3378c.fatbin". 
> Which means if the device side compilation failed, the host side will not 
> compile because InputOk will return false in this case

The patch handles this case correctly, thank you.

Unfortunately it's a bit more complicated than this.  You can pass 
`--cuda-gpu-arch=XXX` multiple times when invoking clang, and that will result 
in multiple compilations for different GPU architectures.  In this case we want 
to fail when the first one fails, on the theory that (most of the time) all 
compilations will have the same error.

You can test this with e.g.

  $ clang -x cuda test.cu -nocudalib -nocudainc --cuda-gpu-arch=sm_35 
--cuda-gpu-arch=sm_60

In the testcase above, this outputs the errors twice, once for sm_35 and once 
for sm_60.

> I don't know what is the best way to write test cases for CUDA driver.

Thanks for offering to write tests for this.  See `ls clang/test/Driver | grep 
'\.cu$'`, and let me know if you need help crafting a testcase.


https://reviews.llvm.org/D39502



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


[PATCH] D39505: [OpenMP] Show error if VLAs are not supported

2017-11-05 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: include/clang/Basic/TargetInfo.h:944
+  /// \brief Whether target supports variable-length arrays.
+  bool isVLASupported() const { return VLASupported; }
+

Hahnfeld wrote:
> rjmccall wrote:
> > Hahnfeld wrote:
> > > rjmccall wrote:
> > > > The way you've written this makes it sound like "does the target 
> > > > support VLAs?", but the actual semantic checks treat it as "do OpenMP 
> > > > devices on this target support VLAs?"  Maybe there should be a more 
> > > > specific way to query things about OpenMP devices instead of setting a 
> > > > global flag for the target?
> > > Actually, the NVPTX and SPIR targets never support VLAs. So I felt like 
> > > it would be more correct to make this a global property of the target.
> > > 
> > > The difference is that the other programming models (OpenCL and CUDA) 
> > > error out immediatelyand regardless of the target because this limitation 
> > > is reflected in the standards that disallow VLAs (see SemaType.cpp). For 
> > > OpenMP we might have target devices that support VLA so we shouldn't 
> > > error out for those.
> > If you want to make it a global property of the target, that's fine, but 
> > then I don't understand why your diagnostic only fires when 
> > (S.isInOpenMPDeclareTargetContext() || 
> > S.isInOpenMPTargetExecutionDirective()).
> That is because of how OpenMP offloading works and how it is implemented in 
> Clang. Consider the following snippet from the added test case:
> ```lang=c
> int vla[arg];
> 
> #pragma omp target map(vla[0:arg])
> {
>// more code here...
> }
> ```
> 
> Clang will take the following steps to compile this into a working binary for 
> a GPU:
> 1. Parse and (semantically) analyze the code as-is for the host and produce 
> LLVM Bitcode.
> 2. Parse and analyze again the code as-is and generate code for the 
> offloading target, the GPU in this case.
> 3. Take LLVM Bitcode from 1., generate host binary and embed target binary 
> from 3.
> 
> `OpenMPIsDevice` will be true for 2., but the complete source code is 
> analyzed. So to not throw errors for the host code, we have to make sure that 
> we are actually generating code for the target device. This is either in a 
> `target` directive or in a `declare target` region.
> Note that this is quite similar to what CUDA does, only they have 
> `CUDADiagIfDeviceCode` for this logic. If you want me to add something of 
> that kind for OpenMP target devices, I'm fine with that. However for the 
> given case, it's a bit different because this error should only be thrown for 
> target devices that don't support VLAs...
I see.  So the entire translation unit is re-parsed and re-Sema'ed from scratch 
for the target?  Which means you need to avoid generating errors about things 
in the outer translation unit that aren't part of the target directive that you 
actually want to compile.  I would've expected there to be some existing 
mechanism for that, to be honest, as opposed to explicitly trying to suppress 
target-specific diagnostics one by one.


https://reviews.llvm.org/D39505



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


r317456 - lowering broadcastm

2017-11-05 Thread Jina Nahias via cfe-commits
Author: jina.nahias
Date: Sun Nov  5 23:04:12 2017
New Revision: 317456

URL: http://llvm.org/viewvc/llvm-project?rev=317456&view=rev
Log:
lowering broadcastm

Change-Id: I0661abea3e3742860e0a03ff9e4fcdc367eff7db

Modified:
cfe/trunk/include/clang/Basic/BuiltinsX86.def
cfe/trunk/lib/Headers/avx512cdintrin.h
cfe/trunk/lib/Headers/avx512vlcdintrin.h
cfe/trunk/test/CodeGen/avx512cdintrin.c
cfe/trunk/test/CodeGen/avx512vlcd-builtins.c

Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=317456&r1=317455&r2=317456&view=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Sun Nov  5 23:04:12 2017
@@ -1580,12 +1580,6 @@ TARGET_BUILTIN(__builtin_ia32_cvtmask2q1
 TARGET_BUILTIN(__builtin_ia32_cvtmask2q256, "V4LLiUc","","avx512dq,avx512vl")
 TARGET_BUILTIN(__builtin_ia32_cvtq2mask128, "UcV2LLi","","avx512dq,avx512vl")
 TARGET_BUILTIN(__builtin_ia32_cvtq2mask256, "UcV4LLi","","avx512dq,avx512vl")
-TARGET_BUILTIN(__builtin_ia32_broadcastmb512, "V8LLiUc","","avx512cd")
-TARGET_BUILTIN(__builtin_ia32_broadcastmw512, "V16iUs","","avx512cd")
-TARGET_BUILTIN(__builtin_ia32_broadcastmb128, "V2LLiUc","","avx512cd,avx512vl")
-TARGET_BUILTIN(__builtin_ia32_broadcastmb256, "V4LLiUc","","avx512cd,avx512vl")
-TARGET_BUILTIN(__builtin_ia32_broadcastmw128, "V4iUs","","avx512cd,avx512vl")
-TARGET_BUILTIN(__builtin_ia32_broadcastmw256, "V8iUs","","avx512cd,avx512vl")
 TARGET_BUILTIN(__builtin_ia32_pmovsdb512_mask, "V16cV16iV16cUs","","avx512f")
 TARGET_BUILTIN(__builtin_ia32_pmovsdb512mem_mask, "vV16c*V16iUs","","avx512f")
 TARGET_BUILTIN(__builtin_ia32_pmovswb512mem_mask, "vV32c*V32sUi","","avx512bw")

Modified: cfe/trunk/lib/Headers/avx512cdintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/avx512cdintrin.h?rev=317456&r1=317455&r2=317456&view=diff
==
--- cfe/trunk/lib/Headers/avx512cdintrin.h (original)
+++ cfe/trunk/lib/Headers/avx512cdintrin.h Sun Nov  5 23:04:12 2017
@@ -130,13 +130,14 @@ _mm512_maskz_lzcnt_epi64 (__mmask8 __U,
 static __inline__ __m512i __DEFAULT_FN_ATTRS
 _mm512_broadcastmb_epi64 (__mmask8 __A)
 {
-  return (__m512i) __builtin_ia32_broadcastmb512 (__A);
+  return (__m512i) _mm512_set1_epi64((long long) __A);
 }
 
 static __inline__ __m512i __DEFAULT_FN_ATTRS
 _mm512_broadcastmw_epi32 (__mmask16 __A)
 {
-  return (__m512i) __builtin_ia32_broadcastmw512 (__A);
+  return (__m512i) _mm512_set1_epi32((int) __A);
+
 }
 
 #undef __DEFAULT_FN_ATTRS

Modified: cfe/trunk/lib/Headers/avx512vlcdintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/avx512vlcdintrin.h?rev=317456&r1=317455&r2=317456&view=diff
==
--- cfe/trunk/lib/Headers/avx512vlcdintrin.h (original)
+++ cfe/trunk/lib/Headers/avx512vlcdintrin.h Sun Nov  5 23:04:12 2017
@@ -33,26 +33,26 @@
 
 static __inline__ __m128i __DEFAULT_FN_ATTRS
 _mm_broadcastmb_epi64 (__mmask8 __A)
-{
-  return (__m128i) __builtin_ia32_broadcastmb128 (__A);
+{ 
+  return (__m128i) _mm_set1_epi64x((long long) __A);
 }
 
 static __inline__ __m256i __DEFAULT_FN_ATTRS
 _mm256_broadcastmb_epi64 (__mmask8 __A)
 {
-  return (__m256i) __builtin_ia32_broadcastmb256 (__A);
+  return (__m256i) _mm256_set1_epi64x((long long)__A);
 }
 
 static __inline__ __m128i __DEFAULT_FN_ATTRS
 _mm_broadcastmw_epi32 (__mmask16 __A)
 {
-  return (__m128i) __builtin_ia32_broadcastmw128 (__A);
+  return (__m128i) _mm_set1_epi32((int)__A);
 }
 
 static __inline__ __m256i __DEFAULT_FN_ATTRS
 _mm256_broadcastmw_epi32 (__mmask16 __A)
 {
-  return (__m256i) __builtin_ia32_broadcastmw256 (__A);
+  return (__m256i) _mm256_set1_epi32((int)__A);
 }
 
 

Modified: cfe/trunk/test/CodeGen/avx512cdintrin.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/avx512cdintrin.c?rev=317456&r1=317455&r2=317456&view=diff
==
--- cfe/trunk/test/CodeGen/avx512cdintrin.c (original)
+++ cfe/trunk/test/CodeGen/avx512cdintrin.c Sun Nov  5 23:04:12 2017
@@ -68,14 +68,40 @@ __m512i test_mm512_maskz_lzcnt_epi64(__m
   return _mm512_maskz_lzcnt_epi64(__U,__A); 
 }
 
-__m512i test_mm512_broadcastmb_epi64(__mmask8 __A) {
+__m512i test_mm512_broadcastmb_epi64(__m512i a, __m512i b) {
   // CHECK-LABEL: @test_mm512_broadcastmb_epi64
-  // CHECK: @llvm.x86.avx512.broadcastmb.512
-  return _mm512_broadcastmb_epi64(__A); 
+  // CHECK: icmp eq <8 x i64> %{{.*}}, %{{.*}}
+  // CHECK: zext i8 %{{.*}} to i64
+  // CHECK: insertelement <8 x i64> undef, i64 %{{.*}}, i32 0
+  // CHECK: insertelement <8 x i64> %{{.*}}, i64 %{{.*}}, i32 1
+  // CHECK: insertelement <8 x i64> %{{.*}}, i64 %{{.*}}, i32 2
+  // CHECK: insertelement 

[PATCH] D39673: Toolchain: Normalize dwarf, sjlj and seh eh

2017-11-05 Thread Martell Malone via Phabricator via cfe-commits
martell created this revision.
Herald added a subscriber: aprantl.

Adds `-fseh-exceptions` and `-fdwarf-exceptions` to compliment 
`-fsjlj-exceptions`.

This makes the exception personality configurable at runtime rather then just 
compile time.

If nothing is passed to cc1 we default to `-fdwarf-exceptions`
The clang frontend will first check if the user has specified one of the above 
flags,
if not then it will check the target default and attach either 
`-fseh-exceptions` or
`-fsjlj-exceptions`. Attaching `-fdwarf-exceptions` is unnecessary because it 
is the
default and some of tests check for `-NOT: "exception"` when passing 
`-fno-exceptions`

The general rule of thumb here is `-fno-exceptions` has a higher priority then 
specifying
the exception type.

Other notable changes:

Move `__SEH__` macro definition out of Targets and into `InitPreprocessor` 
making
it dependant on `-fseh-exceptions`

Remove UseSEHExceptions from the MinGW Driver that doesn't actually do anything.


Repository:
  rL LLVM

https://reviews.llvm.org/D39673

Files:
  include/clang/Basic/LangOptions.def
  include/clang/Driver/Options.td
  include/clang/Driver/ToolChain.h
  lib/Basic/Targets/X86.h
  lib/CodeGen/BackendUtil.cpp
  lib/CodeGen/CGException.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/MinGW.cpp
  lib/Driver/ToolChains/MinGW.h
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/InitPreprocessor.cpp
  test/CodeGenCXX/mingw-w64-exceptions.c
  test/CodeGenCXX/mingw-w64-seh-exceptions.cpp

Index: test/CodeGenCXX/mingw-w64-seh-exceptions.cpp
===
--- test/CodeGenCXX/mingw-w64-seh-exceptions.cpp
+++ test/CodeGenCXX/mingw-w64-seh-exceptions.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 %s -fexceptions -emit-llvm -triple x86_64-w64-windows-gnu -o - | FileCheck %s --check-prefix=X64
-// RUN: %clang_cc1 %s -fexceptions -emit-llvm -triple i686-w64-windows-gnu -o - | FileCheck %s --check-prefix=X86
+// RUN: %clang_cc1 %s -fexceptions -fseh-exceptions -emit-llvm -triple x86_64-w64-windows-gnu -o - | FileCheck %s --check-prefix=X64
+// RUN: %clang_cc1 %s -fexceptions -fdwarf-exceptions -emit-llvm -triple i686-w64-windows-gnu -o - | FileCheck %s --check-prefix=X86
 
 extern "C" void foo();
 extern "C" void bar();
Index: test/CodeGenCXX/mingw-w64-exceptions.c
===
--- /dev/null
+++ test/CodeGenCXX/mingw-w64-exceptions.c
@@ -0,0 +1,22 @@
+// RUN: %clang -target x86_64-windows-gnu -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SEH
+// RUN: %clang -target i686-windows-gnu -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-DWARF
+
+// RUN: %clang -target x86_64-windows-gnu -fsjlj-exceptions -c %s -### 2>&1 | \
+// RUN: FileCheck %s --check-prefix=CHECK-SJLJ
+
+// RUN: %clang -target x86_64-windows-gnu -fdwarf-exceptions -c %s -### 2>&1 | \
+// RUN: FileCheck %s --check-prefix=CHECK-DWARF
+
+// RUN: %clang -target x86_64-windows-gnu -fsjlj-exceptions -fseh-exceptions -c %s -### 2>&1 | \
+// RUN: FileCheck %s --check-prefix=CHECK-SEH
+
+// RUN: %clang -target x86_64-windows-gnu -fseh-exceptions -fsjlj-exceptions -c %s -### 2>&1 | \
+// RUN: FileCheck %s --check-prefix=CHECK-SJLJ
+
+// RUN: %clang -target x86_64-windows-gnu -fseh-exceptions -fdwarf-exceptions -c %s -### 2>&1 | \
+// RUN: FileCheck %s --check-prefix=CHECK-DWARF
+
+// CHECK-SEH: "-fseh-exceptions"
+// CHECK-SJLJ: "-fsjlj-exceptions"
+// CHECK-DWARF-NOT: "-fsjlj-exceptions"
+// CHECK-DWARF-NOT: "-fseh-exceptions"
Index: lib/Frontend/InitPreprocessor.cpp
===
--- lib/Frontend/InitPreprocessor.cpp
+++ lib/Frontend/InitPreprocessor.cpp
@@ -679,6 +679,8 @@
 Builder.defineMacro("__GXX_RTTI");
   if (LangOpts.SjLjExceptions)
 Builder.defineMacro("__USING_SJLJ_EXCEPTIONS__");
+  if (LangOpts.SEHExceptions)
+Builder.defineMacro("__SEH__");
 
   if (LangOpts.Deprecated)
 Builder.defineMacro("__DEPRECATED");
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2133,7 +2133,18 @@
   Opts.Exceptions = Args.hasArg(OPT_fexceptions);
   Opts.ObjCExceptions = Args.hasArg(OPT_fobjc_exceptions);
   Opts.CXXExceptions = Args.hasArg(OPT_fcxx_exceptions);
-  Opts.SjLjExceptions = Args.hasArg(OPT_fsjlj_exceptions);
+
+  // Handle exception personalities
+  Arg *A = Args.getLastArg(options::OPT_fsjlj_exceptions,
+   options::OPT_fseh_exceptions,
+   options::OPT_fdwarf_exceptions);
+  if (A) {
+const Option &Opt = A->getOption();
+Opts.SjLjExceptions = Opt.matches(options::OPT_fsjlj_exceptions);
+Opts.SEHExceptions = Opt.matches(options::OPT_fseh_exceptions);
+Opts.DWARFExceptions = Opt.matches(options::OPT_fdwarf_exceptions);
+  }
+
   Opts.ExternCNoUnwind = Args.hasArg(OPT_fext

[PATCH] D38683: [X86][AVX512] lowering broadcastm intrinsic - clang part

2017-11-05 Thread jina via Phabricator via cfe-commits
jina.nahias added a comment.

commit in https://reviews.llvm.org/rL317456


https://reviews.llvm.org/D38683



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