[PATCH] D39638: [NVPTX] Implement __nvvm_atom_add_gen_d builtin.
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.
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
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
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
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
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)
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
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
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
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
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
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
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
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
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
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
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()
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
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
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
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()
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
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
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
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
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
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
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
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
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
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
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.
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
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
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
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
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
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
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
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
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
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