[PATCH] D39562: [CodeGen][ObjC] Fix an assertion failure caused by copy elision

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 136895.
ahatanak added a comment.

Add a flag to OpaqeuValueExpr that indicates whether it is a unique reference 
to its subexpression. If the flag is set, IRGen will avoid binding the OVE and 
copying the result to the destination and instead just visit the OVE's 
sub-expression.

The flag is set only when the PseudoObjectExpr is an assignment and the RHS is 
an rvalue of a C++ class type, which is sufficient to fix the assertion failure 
this patch is trying to fix.


https://reviews.llvm.org/D39562

Files:
  include/clang/AST/Expr.h
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGExprAgg.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Sema/SemaPseudoObject.cpp
  test/CodeGenObjCXX/property-dot-copy-elision.mm
  test/CodeGenObjCXX/property-objects.mm

Index: test/CodeGenObjCXX/property-objects.mm
===
--- test/CodeGenObjCXX/property-objects.mm
+++ test/CodeGenObjCXX/property-objects.mm
@@ -72,7 +72,7 @@
 
 // rdar://8379892
 // CHECK-LABEL: define void @_Z1fP1A
-// CHECK: call void @_ZN1XC1Ev(%struct.X* [[LVTEMP:%[a-zA-Z0-9\.]+]])
+// CHECK: call void @_ZN1XC1Ev(%struct.X* [[LVTEMP:%.+]])
 // CHECK: call void @_ZN1XC1ERKS_(%struct.X* [[AGGTMP:%[a-zA-Z0-9\.]+]], %struct.X* dereferenceable({{[0-9]+}}) [[LVTEMP]])
 // CHECK: call void bitcast (i8* (i8*, i8*, ...)* @objc_msgSend to void (i8*, i8*, %struct.X*)*)({{.*}} %struct.X* [[AGGTMP]])
 struct X {
Index: test/CodeGenObjCXX/property-dot-copy-elision.mm
===
--- /dev/null
+++ test/CodeGenObjCXX/property-dot-copy-elision.mm
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -std=c++1z -fobjc-arc -o - %s | FileCheck %s
+
+struct S0 {
+  id f;
+};
+
+struct S1 {
+  S1();
+  S1(S0);
+  id f;
+};
+
+@interface C
+@property S1 f;
+@end
+@implementation C
+@end
+
+// CHECK-LABEL: define void @_Z5test0P1C(
+// CHECK: %{{.*}} = alloca %
+// CHECK: %[[AGG_TMP:.*]] = alloca %[[STRUCT_S1:.*]], align
+// CHECK: %[[AGG_TMP_1:.*]] = alloca %[[STRUCT_S0:.*]], align
+// CHECK: call void @_ZN2S0C1Ev(%[[STRUCT_S0]]* %[[AGG_TMP_1]])
+// CHECK: call void @_ZN2S1C1E2S0(%[[STRUCT_S1]]* %[[AGG_TMP]], %[[STRUCT_S0]]* %[[AGG_TMP_1]])
+// CHECK: call void bitcast (i8* (i8*, i8*, ...)* @objc_msgSend to void (i8*, i8*, %[[STRUCT_S1]]*)*)(i8* %{{.*}}, i8* %{{.*}}, %[[STRUCT_S1]]* %[[AGG_TMP]])
+
+void test0(C *c) {
+  c.f = S0();
+}
+
+// CHECK: define void @_Z5test1P1C(
+// CHECK: %{{.*}} = alloca %
+// CHECK: %[[TEMP_LVALUE:.*]] = alloca %[[STRUCT_S1:.*]], align
+// CHECK: call void @_ZN2S1C1Ev(%[[STRUCT_S1]]* %[[TEMP_LVALUE]])
+// CHECK: call void bitcast (i8* (i8*, i8*, ...)* @objc_msgSend to void (i8*, i8*, %[[STRUCT_S1]]*)*)(i8* %{{.*}}, i8* %{{.*}}, %[[STRUCT_S1]]* %[[TEMP_LVALUE]])
+
+void test1(C *c) {
+  c.f = S1();
+}
Index: lib/Sema/SemaPseudoObject.cpp
===
--- lib/Sema/SemaPseudoObject.cpp
+++ lib/Sema/SemaPseudoObject.cpp
@@ -428,6 +428,9 @@
   Expr *syntacticLHS = rebuildAndCaptureObject(LHS);
   OpaqueValueExpr *capturedRHS = capture(RHS);
 
+  if (capturedRHS->getType()->getAsCXXRecordDecl() && capturedRHS->isRValue())
+capturedRHS->setIsUnique();
+
   // In some very specific cases, semantic analysis of the RHS as an
   // expression may require it to be rewritten.  In these cases, we
   // cannot safely keep the OVE around.  Fortunately, we don't really
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -2124,14 +2124,7 @@
 
   /// getOpaqueLValueMapping - Given an opaque value expression (which
   /// must be mapped to an l-value), return its mapping.
-  const LValue (const OpaqueValueExpr *e) {
-assert(OpaqueValueMapping::shouldBindAsLValue(e));
-
-llvm::DenseMap::iterator
-  it = OpaqueLValues.find(e);
-assert(it != OpaqueLValues.end() && "no mapping for opaque value!");
-return it->second;
-  }
+  LValue getOpaqueLValueMapping(const OpaqueValueExpr *e);
 
   /// getOpaqueRValueMapping - Given an opaque value expression (which
   /// must be mapped to an r-value), return its mapping.
Index: lib/CodeGen/CGExprAgg.cpp
===
--- lib/CodeGen/CGExprAgg.cpp
+++ lib/CodeGen/CGExprAgg.cpp
@@ -606,7 +606,11 @@
 }
 
 void AggExprEmitter::VisitOpaqueValueExpr(OpaqueValueExpr *e) {
-  EmitFinalDestCopy(e->getType(), CGF.getOpaqueLValueMapping(e));
+  // Avoid copying if OVE is a unique reference to its source expression.
+  if (e->isUnique())
+Visit(e->getSourceExpr());
+  else
+EmitFinalDestCopy(e->getType(), CGF.getOpaqueLValueMapping(e));
 }
 
 void
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -4170,6 +4170,18 

[PATCH] D44064: [Driver] Automatically disable incompatible default sanitizers

[Petr Hosek via Phabricator via cfe-commits]

phosek created this revision.
phosek added reviewers: kcc, vitalybuka, mcgrathr.
Herald added subscribers: cfe-commits, cryptoad.
phosek added a dependent revision: D44065: [Driver] Enable SafeStack by default 
on Fuchsia.

When a sanitizer incompatible with one of the default sanitizers
is explicitly enabled, automatically disable all the conflicting
default sanitizers.


Repository:
  rC Clang

https://reviews.llvm.org/D44064

Files:
  clang/lib/Driver/SanitizerArgs.cpp


Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -332,8 +332,30 @@
 }
   }
 
+  std::pair IncompatibleGroups[] = {
+  std::make_pair(Address, Thread | Memory),
+  std::make_pair(Thread, Memory),
+  std::make_pair(Leak, Thread | Memory),
+  std::make_pair(KernelAddress, Address | Leak | Thread | Memory),
+  std::make_pair(HWAddress, Address | Thread | Memory | KernelAddress),
+  std::make_pair(Efficiency, Address | HWAddress | Leak | Thread | Memory |
+ KernelAddress),
+  std::make_pair(Scudo, Address | HWAddress | Leak | Thread | Memory |
+KernelAddress | Efficiency),
+  std::make_pair(SafeStack, Address | HWAddress | Leak | Thread | Memory |
+KernelAddress | Efficiency)};
+
   // Enable toolchain specific default sanitizers if not explicitly disabled.
-  Kinds |= TC.getDefaultSanitizers() & ~AllRemove;
+  SanitizerMask Default = TC.getDefaultSanitizers() & ~AllRemove;
+
+  // Disable default sanitizers that are incompatible with the default ones.
+  for (auto G : IncompatibleGroups) {
+SanitizerMask Group = G.first;
+if ((Default & Group) && (Kinds & G.second))
+  Default &= ~Group;
+  }
+
+  Kinds |= Default;
 
   // We disable the vptr sanitizer if it was enabled by group expansion but 
RTTI
   // is disabled.
@@ -369,18 +391,6 @@
   }
 
   // Warn about incompatible groups of sanitizers.
-  std::pair IncompatibleGroups[] = {
-  std::make_pair(Address, Thread | Memory),
-  std::make_pair(Thread, Memory),
-  std::make_pair(Leak, Thread | Memory),
-  std::make_pair(KernelAddress, Address | Leak | Thread | Memory),
-  std::make_pair(HWAddress, Address | Thread | Memory | KernelAddress),
-  std::make_pair(Efficiency, Address | HWAddress | Leak | Thread | Memory |
- KernelAddress),
-  std::make_pair(Scudo, Address | HWAddress | Leak | Thread | Memory |
-KernelAddress | Efficiency),
-  std::make_pair(SafeStack, Address | HWAddress | Leak | Thread | Memory |
-KernelAddress | Efficiency)};
   for (auto G : IncompatibleGroups) {
 SanitizerMask Group = G.first;
 if (Kinds & Group) {


Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -332,8 +332,30 @@
 }
   }
 
+  std::pair IncompatibleGroups[] = {
+  std::make_pair(Address, Thread | Memory),
+  std::make_pair(Thread, Memory),
+  std::make_pair(Leak, Thread | Memory),
+  std::make_pair(KernelAddress, Address | Leak | Thread | Memory),
+  std::make_pair(HWAddress, Address | Thread | Memory | KernelAddress),
+  std::make_pair(Efficiency, Address | HWAddress | Leak | Thread | Memory |
+ KernelAddress),
+  std::make_pair(Scudo, Address | HWAddress | Leak | Thread | Memory |
+KernelAddress | Efficiency),
+  std::make_pair(SafeStack, Address | HWAddress | Leak | Thread | Memory |
+KernelAddress | Efficiency)};
+
   // Enable toolchain specific default sanitizers if not explicitly disabled.
-  Kinds |= TC.getDefaultSanitizers() & ~AllRemove;
+  SanitizerMask Default = TC.getDefaultSanitizers() & ~AllRemove;
+
+  // Disable default sanitizers that are incompatible with the default ones.
+  for (auto G : IncompatibleGroups) {
+SanitizerMask Group = G.first;
+if ((Default & Group) && (Kinds & G.second))
+  Default &= ~Group;
+  }
+
+  Kinds |= Default;
 
   // We disable the vptr sanitizer if it was enabled by group expansion but RTTI
   // is disabled.
@@ -369,18 +391,6 @@
   }
 
   // Warn about incompatible groups of sanitizers.
-  std::pair IncompatibleGroups[] = {
-  std::make_pair(Address, Thread | Memory),
-  std::make_pair(Thread, Memory),
-  std::make_pair(Leak, Thread | Memory),
-  std::make_pair(KernelAddress, Address | Leak | Thread | Memory),
-  std::make_pair(HWAddress, Address | Thread | Memory | KernelAddress),
-  

[PATCH] D44065: [Driver] Enable SafeStack by default on Fuchsia

[Petr Hosek via Phabricator via cfe-commits]

phosek created this revision.
phosek added reviewers: mcgrathr, kcc, vitalybuka.
Herald added subscribers: cfe-commits, cryptoad.
phosek added a dependency: D44064: [Driver] Automatically disable incompatible 
default sanitizers.

This is already used throughout the entire system, so make it a default.


Repository:
  rC Clang

https://reviews.llvm.org/D44065

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/lib/Driver/ToolChains/Fuchsia.h
  clang/test/Driver/fuchsia.c


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -10,6 +10,7 @@
 // CHECK: "-fuse-init-array"
 // CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK: "-internal-externc-isystem" "[[SYSROOT]]{{/|}}include"
+// CHECK: "-fsanitize=safe-stack"
 // CHECK: "-fno-common"
 // CHECK: {{.*}}ld.lld{{.*}}" "-z" "rodynamic"
 // CHECK: "--sysroot=[[SYSROOT]]"
@@ -84,31 +85,31 @@
 // RUN: %clang %s -### --target=x86_64-fuchsia \
 // RUN: -fsanitize=fuzzer 2>&1 \
 // RUN: | FileCheck %s -check-prefix=CHECK-FUZZER-X86
-// CHECK-FUZZER-X86: "-fsanitize=fuzzer,fuzzer-no-link"
+// CHECK-FUZZER-X86: "-fsanitize=fuzzer,fuzzer-no-link,safe-stack"
 // CHECK-FUZZER-X86: "{{.*[/\\]}}libclang_rt.fuzzer-x86_64.a"
 
 // RUN: %clang %s -### --target=aarch64-fuchsia \
 // RUN: -fsanitize=fuzzer 2>&1 \
 // RUN: | FileCheck %s -check-prefix=CHECK-FUZZER-AARCH64
-// CHECK-FUZZER-AARCH64: "-fsanitize=fuzzer,fuzzer-no-link"
+// CHECK-FUZZER-AARCH64: "-fsanitize=fuzzer,fuzzer-no-link,safe-stack"
 // CHECK-FUZZER-AARCH64: "{{.*[/\\]}}libclang_rt.fuzzer-aarch64.a"
 
 // RUN: %clang %s -### --target=x86_64-fuchsia \
 // RUN: -fsanitize=scudo 2>&1 \
 // RUN: | FileCheck %s -check-prefix=CHECK-SCUDO-X86
-// CHECK-SCUDO-X86: "-fsanitize=scudo"
+// CHECK-SCUDO-X86: "-fsanitize=safe-stack,scudo"
 // CHECK-SCUDO-X86: "-pie"
 // CHECK-SCUDO-X86: "{{.*[/\\]}}libclang_rt.scudo-x86_64.so"
 
 // RUN: %clang %s -### --target=aarch64-fuchsia \
 // RUN: -fsanitize=scudo 2>&1 \
 // RUN: | FileCheck %s -check-prefix=CHECK-SCUDO-AARCH64
-// CHECK-SCUDO-AARCH64: "-fsanitize=scudo"
+// CHECK-SCUDO-AARCH64: "-fsanitize=safe-stack,scudo"
 // CHECK-SCUDO-AARCH64: "-pie"
 // CHECK-SCUDO-AARCH64: "{{.*[/\\]}}libclang_rt.scudo-aarch64.so"
 
 // RUN: %clang %s -### --target=x86_64-fuchsia \
 // RUN: -fsanitize=scudo -fPIC -shared 2>&1 \
 // RUN: | FileCheck %s -check-prefix=CHECK-SCUDO-SHARED
-// CHECK-SCUDO-SHARED: "-fsanitize=scudo"
+// CHECK-SCUDO-SHARED: "-fsanitize=safe-stack,scudo"
 // CHECK-SCUDO-SHARED: "{{.*[/\\]}}libclang_rt.scudo-x86_64.so"
Index: clang/lib/Driver/ToolChains/Fuchsia.h
===
--- clang/lib/Driver/ToolChains/Fuchsia.h
+++ clang/lib/Driver/ToolChains/Fuchsia.h
@@ -64,6 +64,7 @@
   types::ID InputType) const override;
 
   SanitizerMask getSupportedSanitizers() const override;
+  SanitizerMask getDefaultSanitizers() const override;
 
   RuntimeLibType
   GetRuntimeLibType(const llvm::opt::ArgList ) const override;
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -284,3 +284,7 @@
   Res |= SanitizerKind::Scudo;
   return Res;
 }
+
+SanitizerMask Fuchsia::getDefaultSanitizers() const {
+  return SanitizerKind::SafeStack;
+}


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -10,6 +10,7 @@
 // CHECK: "-fuse-init-array"
 // CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK: "-internal-externc-isystem" "[[SYSROOT]]{{/|}}include"
+// CHECK: "-fsanitize=safe-stack"
 // CHECK: "-fno-common"
 // CHECK: {{.*}}ld.lld{{.*}}" "-z" "rodynamic"
 // CHECK: "--sysroot=[[SYSROOT]]"
@@ -84,31 +85,31 @@
 // RUN: %clang %s -### --target=x86_64-fuchsia \
 // RUN: -fsanitize=fuzzer 2>&1 \
 // RUN: | FileCheck %s -check-prefix=CHECK-FUZZER-X86
-// CHECK-FUZZER-X86: "-fsanitize=fuzzer,fuzzer-no-link"
+// CHECK-FUZZER-X86: "-fsanitize=fuzzer,fuzzer-no-link,safe-stack"
 // CHECK-FUZZER-X86: "{{.*[/\\]}}libclang_rt.fuzzer-x86_64.a"
 
 // RUN: %clang %s -### --target=aarch64-fuchsia \
 // RUN: -fsanitize=fuzzer 2>&1 \
 // RUN: | FileCheck %s -check-prefix=CHECK-FUZZER-AARCH64
-// CHECK-FUZZER-AARCH64: "-fsanitize=fuzzer,fuzzer-no-link"
+// CHECK-FUZZER-AARCH64: "-fsanitize=fuzzer,fuzzer-no-link,safe-stack"
 // CHECK-FUZZER-AARCH64: "{{.*[/\\]}}libclang_rt.fuzzer-aarch64.a"
 
 // RUN: %clang %s -### --target=x86_64-fuchsia \
 // RUN: -fsanitize=scudo 2>&1 \
 // RUN: | FileCheck %s -check-prefix=CHECK-SCUDO-X86
-// CHECK-SCUDO-X86: "-fsanitize=scudo"
+// CHECK-SCUDO-X86: "-fsanitize=safe-stack,scudo"
 // CHECK-SCUDO-X86: "-pie"
 // 

[PATCH] D43423: [SimplifyCFG] Create flag to disable simplifyCFG.

[Vitaly Buka via Phabricator via cfe-commits]

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

I assume this is going to be abandoned in favor of 
https://reviews.llvm.org/D44057


https://reviews.llvm.org/D43423



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

[PATCH] D44051: [CFG] [analyzer] Add construction context for implicit constructor conversions.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.

Implicit constructor conversions such as `A a = B()` are represented by 
surrounding the constructor for `B()` with an `ImplicitCastExpr` of 
`CK_ConstructorConversion` kind, similarly to how explicit constructor 
conversions are surrounded by a `CXXFunctionalCastExpr`. Support this syntax 
pattern in when understanding the construction context for the implicit 
constructor that performs the conversion.


Repository:
  rC Clang

https://reviews.llvm.org/D44051

Files:
  lib/Analysis/CFG.cpp
  lib/Analysis/ConstructionContext.cpp
  test/Analysis/cfg-rich-constructors.cpp
  test/Analysis/temporaries.cpp

Index: test/Analysis/temporaries.cpp
===
--- test/Analysis/temporaries.cpp
+++ test/Analysis/temporaries.cpp
@@ -940,3 +940,48 @@
 }
 } // namespace temporary_list_crash
 #endif // C++11
+
+namespace implicit_constructor_conversion {
+struct S {
+  int x;
+  S(int x) : x(x) {}
+  ~S() {}
+};
+
+class C {
+  int x;
+
+public:
+  C(const S ) : x(s.x) {}
+  ~C() {}
+  int getX() const { return x; }
+};
+
+void test() {
+  const C  = S(10);
+  clang_analyzer_eval(c1.getX() == 10);
+#ifdef TEMPORARY_DTORS
+  // expected-warning@-2{{TRUE}}
+#else
+  // expected-warning@-4{{UNKNOWN}}
+#endif
+
+  S s = 20;
+  clang_analyzer_eval(s.x == 20);
+#ifdef TEMPORARY_DTORS
+  // expected-warning@-2{{TRUE}}
+#else
+  // expected-warning@-4{{UNKNOWN}}
+#endif
+
+  C c2 = s;
+  clang_analyzer_eval(c2.getX() == 20);
+#ifdef TEMPORARY_DTORS
+  // expected-warning@-2{{TRUE}}
+#else
+  // expected-warning@-4{{UNKNOWN}}
+#endif
+}
+} // end namespace implicit_constructor_conversion
+
+
Index: test/Analysis/cfg-rich-constructors.cpp
===
--- test/Analysis/cfg-rich-constructors.cpp
+++ test/Analysis/cfg-rich-constructors.cpp
@@ -498,20 +498,70 @@
   ~B() {}
 };
 
-// FIXME: Find construction context for the implicit constructor conversion.
+// CHECK: void implicitConstructionConversionFromTemporary()
+// CHECK:  1: implicit_constructor_conversion::A() (CXXConstructExpr, [B1.3], class implicit_constructor_conversion::A)
+// CHECK-NEXT: 2: [B1.1] (ImplicitCastExpr, NoOp, const class implicit_constructor_conversion::A)
+// CHECK-NEXT: 3: [B1.2]
+// CHECK-NEXT: 4: [B1.3] (CXXConstructExpr, [B1.6], [B1.8], class implicit_constructor_conversion::B)
+// CHECK-NEXT: 5: [B1.4] (ImplicitCastExpr, ConstructorConversion, class implicit_constructor_conversion::B)
+// CHECK-NEXT: 6: [B1.5] (BindTemporary)
+// CHECK-NEXT: 7: [B1.6] (ImplicitCastExpr, NoOp, const class implicit_constructor_conversion::B)
+// CHECK-NEXT: 8: [B1.7]
+// CHECK-NEXT: 9: [B1.8] (CXXConstructExpr, [B1.10], class implicit_constructor_conversion::B)
+// CHECK-NEXT:10: implicit_constructor_conversion::B b = implicit_constructor_conversion::A();
+// CHECK-NEXT:11: ~implicit_constructor_conversion::B() (Temporary object destructor)
+// CHECK-NEXT:12: [B1.10].~B() (Implicit destructor)
+void implicitConstructionConversionFromTemporary() {
+  B b = A();
+}
+
 // CHECK: void implicitConstructionConversionFromFunctionValue()
 // CHECK:  1: get
+// CHECK-NEXT: 2: [B1.1] (ImplicitCastExpr, FunctionToPointerDecay, class implicit_constructor_conversion::A (*)(void))
+// CHECK-NEXT: 3: [B1.2]()
+// CHECK-NEXT: 4: [B1.3] (ImplicitCastExpr, NoOp, const class implicit_constructor_conversion::A)
+// CHECK-NEXT: 5: [B1.4]
+// CHECK-NEXT: 6: [B1.5] (CXXConstructExpr, [B1.8], [B1.10], class implicit_constructor_conversion::B)
+// CHECK-NEXT: 7: [B1.6] (ImplicitCastExpr, ConstructorConversion, class implicit_constructor_conversion::B)
+// CHECK-NEXT: 8: [B1.7] (BindTemporary)
+// CHECK-NEXT: 9: [B1.8] (ImplicitCastExpr, NoOp, const class implicit_constructor_conversion::B)
+// CHECK-NEXT:10: [B1.9]
+// CHECK-NEXT:11: [B1.10] (CXXConstructExpr, [B1.12], class implicit_constructor_conversion::B)
+// CHECK-NEXT:12: implicit_constructor_conversion::B b = get();
+// CHECK-NEXT:13: ~implicit_constructor_conversion::B() (Temporary object destructor)
+// CHECK-NEXT:14: [B1.12].~B() (Implicit destructor)
+void implicitConstructionConversionFromFunctionValue() {
+  B b = get();
+}
+
+// CHECK: void implicitConstructionConversionFromTemporaryWithLifetimeExtension()
+// CHECK:  1: implicit_constructor_conversion::A() (CXXConstructExpr, [B1.3], class implicit_constructor_conversion::A)
+// CHECK-NEXT: 2: [B1.1] (ImplicitCastExpr, NoOp, const class implicit_constructor_conversion::A)
+// CHECK-NEXT: 3: [B1.2]
+// CHECK-NEXT: 4: [B1.3] (CXXConstructExpr, [B1.7], class implicit_constructor_conversion::B)
+// CHECK-NEXT: 5: [B1.4] (ImplicitCastExpr, ConstructorConversion, class 

Re: r313729 - Implement C++ [basic.link]p8.

[Alex L via cfe-commits]

Hi Richard,

This commit has caused a second regression in Clang which triggers a new
invalid -Wunused-function warning. I filed
https://bugs.llvm.org/show_bug.cgi?id=36584 which contains the minimized
test case for this issue. Do you mind taking a look at it?

Thanks,
Alex

On 19 December 2017 at 11:14, Alex L  wrote:

> Hi Richard,
>
> This commit has caused a new regression in Clang which causes an assertion
> failure for extern "C" function definitions whose return type is a pointer
> to a record. I filed https://bugs.llvm.org/show_bug.cgi?id=35697 which
> contains the minimized test case for this issue. Do you mind taking a look
> at it?
>
> Cheers,
> Alex
>
> On 20 September 2017 at 00:22, Richard Smith via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: rsmith
>> Date: Wed Sep 20 00:22:00 2017
>> New Revision: 313729
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=313729=rev
>> Log:
>> Implement C++ [basic.link]p8.
>>
>> If a function or variable has a type with no linkage (and is not extern
>> "C"),
>> any use of it requires a definition within the same translation unit; the
>> idea
>> is that it is not possible to define the entity elsewhere, so any such
>> use is
>> necessarily an error.
>>
>> There is an exception, though: some types formally have no linkage but
>> nonetheless can be referenced from other translation units (for example,
>> this
>> happens to anonymous structures defined within inline functions). For
>> entities
>> with those types, we suppress the diagnostic except under -pedantic.
>>
>> Added:
>> cfe/trunk/test/CXX/basic/basic.link/p8.cpp
>> Modified:
>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> cfe/trunk/include/clang/Sema/Sema.h
>> cfe/trunk/include/clang/Sema/SemaInternal.h
>> cfe/trunk/lib/AST/Decl.cpp
>> cfe/trunk/lib/Sema/Sema.cpp
>> cfe/trunk/lib/Sema/SemaDecl.cpp
>> cfe/trunk/lib/Sema/SemaExpr.cpp
>> cfe/trunk/test/Analysis/malloc.mm
>> cfe/trunk/test/Analysis/reference.cpp
>> cfe/trunk/test/CodeGenCXX/debug-info-method.cpp
>> cfe/trunk/test/CodeGenCXX/mangle-ms-cxx11.cpp
>> cfe/trunk/test/CodeGenCXX/mangle.cpp
>> cfe/trunk/test/PCH/cxx11-lambdas.mm
>> cfe/trunk/test/SemaCXX/warn-unreachable.cpp
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/
>> Basic/DiagnosticSemaKinds.td?rev=313729=313728=313729=diff
>> 
>> ==
>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Sep 20
>> 00:22:00 2017
>> @@ -4707,6 +4707,14 @@ def note_deleted_assign_field : Note<
>>  def warn_undefined_internal : Warning<
>>"%select{function|variable}0 %q1 has internal linkage but is not
>> defined">,
>>InGroup>;
>> +def err_undefined_internal_type : Error<
>> +  "%select{function|variable}0 %q1 is used but not defined in this "
>> +  "translation unit, and cannot be defined in any other translation unit
>> "
>> +  "because its type does not have linkage">;
>> +def ext_undefined_internal_type : Extension<
>> +  "ISO C++ requires a definition in this translation unit for "
>> +  "%select{function|variable}0 %q1 because its type does not have
>> linkage">,
>> +  InGroup>;
>>  def warn_undefined_inline : Warning<"inline function %q0 is not
>> defined">,
>>InGroup>;
>>  def err_undefined_inline_var : Error<"inline variable %q0 is not
>> defined">;
>>
>> Modified: cfe/trunk/include/clang/Sema/Sema.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/
>> Sema/Sema.h?rev=313729=313728=313729=diff
>> 
>> ==
>> --- cfe/trunk/include/clang/Sema/Sema.h (original)
>> +++ cfe/trunk/include/clang/Sema/Sema.h Wed Sep 20 00:22:00 2017
>> @@ -1086,6 +1086,11 @@ public:
>>/// definition in this translation unit.
>>llvm::MapVector UndefinedButUsed;
>>
>> +  /// Determine if VD, which must be a variable or function, is an
>> external
>> +  /// symbol that nonetheless can't be referenced from outside this
>> translation
>> +  /// unit because its type has no linkage and it's not extern "C".
>> +  bool isExternalWithNoLinkageType(ValueDecl *VD);
>> +
>>/// Obtain a sorted list of functions that are undefined but ODR-used.
>>void getUndefinedButUsed(
>>SmallVectorImpl
>> );
>>
>> Modified: cfe/trunk/include/clang/Sema/SemaInternal.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/
>> Sema/SemaInternal.h?rev=313729=313728=313729=diff
>> 
>> ==
>> --- cfe/trunk/include/clang/Sema/SemaInternal.h (original)
>> +++ 

[PATCH] D43995: Do not generate calls to fentry with __attribute__((no_instrument_function))

[Manoj Gupta via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC326639: Do not generate calls to fentry with 
__attribute__((no_instrument_function)) (authored by manojgupta, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D43995?vs=136670=136874#toc

Repository:
  rC Clang

https://reviews.llvm.org/D43995

Files:
  lib/CodeGen/CodeGenFunction.cpp
  test/CodeGen/fentry.c


Index: test/CodeGen/fentry.c
===
--- test/CodeGen/fentry.c
+++ test/CodeGen/fentry.c
@@ -7,5 +7,12 @@
   return 0;
 }
 
-//CHECK: attributes #{{[0-9]+}} = { {{.*}}"fentry-call"="true"{{.*}} }
-//NOPG-NOT: attributes #{{[0-9]+}} = { {{.*}}"fentry-call"{{.*}} }
+int __attribute__((no_instrument_function)) no_instrument(void) {
+  return foo();
+}
+
+//CHECK: attributes #0 = { {{.*}}"fentry-call"="true"{{.*}} }
+//CHECK: attributes #1 = { {{.*}} }
+//CHECK-NOT: attributes #1 = { {{.*}}"fentry-call"="true"{{.*}} }
+//NOPG-NOT: attributes #0 = { {{.*}}"fentry-call"{{.*}} }
+//NOPG-NOT: attributes #1 = { {{.*}}"fentry-call"{{.*}} }
Index: lib/CodeGen/CodeGenFunction.cpp
===
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -1016,10 +1016,12 @@
   // The attribute "counting-function" is set to mcount function name which is
   // architecture dependent.
   if (CGM.getCodeGenOpts().InstrumentForProfiling) {
-if (CGM.getCodeGenOpts().CallFEntry)
-  Fn->addFnAttr("fentry-call", "true");
-else {
-  if (!CurFuncDecl || !CurFuncDecl->hasAttr()) {
+// Calls to fentry/mcount should not be generated if function has
+// the no_instrument_function attribute.
+if (!CurFuncDecl || !CurFuncDecl->hasAttr()) {
+  if (CGM.getCodeGenOpts().CallFEntry)
+Fn->addFnAttr("fentry-call", "true");
+  else {
 Fn->addFnAttr("instrument-function-entry-inlined",
   getTarget().getMCountName());
   }


Index: test/CodeGen/fentry.c
===
--- test/CodeGen/fentry.c
+++ test/CodeGen/fentry.c
@@ -7,5 +7,12 @@
   return 0;
 }
 
-//CHECK: attributes #{{[0-9]+}} = { {{.*}}"fentry-call"="true"{{.*}} }
-//NOPG-NOT: attributes #{{[0-9]+}} = { {{.*}}"fentry-call"{{.*}} }
+int __attribute__((no_instrument_function)) no_instrument(void) {
+  return foo();
+}
+
+//CHECK: attributes #0 = { {{.*}}"fentry-call"="true"{{.*}} }
+//CHECK: attributes #1 = { {{.*}} }
+//CHECK-NOT: attributes #1 = { {{.*}}"fentry-call"="true"{{.*}} }
+//NOPG-NOT: attributes #0 = { {{.*}}"fentry-call"{{.*}} }
+//NOPG-NOT: attributes #1 = { {{.*}}"fentry-call"{{.*}} }
Index: lib/CodeGen/CodeGenFunction.cpp
===
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -1016,10 +1016,12 @@
   // The attribute "counting-function" is set to mcount function name which is
   // architecture dependent.
   if (CGM.getCodeGenOpts().InstrumentForProfiling) {
-if (CGM.getCodeGenOpts().CallFEntry)
-  Fn->addFnAttr("fentry-call", "true");
-else {
-  if (!CurFuncDecl || !CurFuncDecl->hasAttr()) {
+// Calls to fentry/mcount should not be generated if function has
+// the no_instrument_function attribute.
+if (!CurFuncDecl || !CurFuncDecl->hasAttr()) {
+  if (CGM.getCodeGenOpts().CallFEntry)
+Fn->addFnAttr("fentry-call", "true");
+  else {
 Fn->addFnAttr("instrument-function-entry-inlined",
   getTarget().getMCountName());
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r326639 - Do not generate calls to fentry with __attribute__((no_instrument_function))

[Manoj Gupta via cfe-commits]

Author: manojgupta
Date: Fri Mar  2 15:52:44 2018
New Revision: 326639

URL: http://llvm.org/viewvc/llvm-project?rev=326639=rev
Log:
Do not generate calls to fentry with __attribute__((no_instrument_function))

Summary:
Currently only calls to mcount were suppressed with
no_instrument_function attribute.
Linux kernel requires that calls to fentry should also not be
generated.
This is an extended fix for PR PR33515.

Reviewers: hfinkel, rengolin, srhines, rnk, rsmith, rjmccall, hans

Reviewed By: rjmccall

Subscribers: cfe-commits

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

Modified:
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/test/CodeGen/fentry.c

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=326639=326638=326639=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Fri Mar  2 15:52:44 2018
@@ -1016,10 +1016,12 @@ void CodeGenFunction::StartFunction(Glob
   // The attribute "counting-function" is set to mcount function name which is
   // architecture dependent.
   if (CGM.getCodeGenOpts().InstrumentForProfiling) {
-if (CGM.getCodeGenOpts().CallFEntry)
-  Fn->addFnAttr("fentry-call", "true");
-else {
-  if (!CurFuncDecl || !CurFuncDecl->hasAttr()) {
+// Calls to fentry/mcount should not be generated if function has
+// the no_instrument_function attribute.
+if (!CurFuncDecl || !CurFuncDecl->hasAttr()) {
+  if (CGM.getCodeGenOpts().CallFEntry)
+Fn->addFnAttr("fentry-call", "true");
+  else {
 Fn->addFnAttr("instrument-function-entry-inlined",
   getTarget().getMCountName());
   }

Modified: cfe/trunk/test/CodeGen/fentry.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/fentry.c?rev=326639=326638=326639=diff
==
--- cfe/trunk/test/CodeGen/fentry.c (original)
+++ cfe/trunk/test/CodeGen/fentry.c Fri Mar  2 15:52:44 2018
@@ -7,5 +7,12 @@ int foo(void) {
   return 0;
 }
 
-//CHECK: attributes #{{[0-9]+}} = { {{.*}}"fentry-call"="true"{{.*}} }
-//NOPG-NOT: attributes #{{[0-9]+}} = { {{.*}}"fentry-call"{{.*}} }
+int __attribute__((no_instrument_function)) no_instrument(void) {
+  return foo();
+}
+
+//CHECK: attributes #0 = { {{.*}}"fentry-call"="true"{{.*}} }
+//CHECK: attributes #1 = { {{.*}} }
+//CHECK-NOT: attributes #1 = { {{.*}}"fentry-call"="true"{{.*}} }
+//NOPG-NOT: attributes #0 = { {{.*}}"fentry-call"{{.*}} }
+//NOPG-NOT: attributes #1 = { {{.*}}"fentry-call"{{.*}} }


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

[PATCH] D43911: [AMDGPU] Clean up old address space mapping and fix constant address space value

[Tony Tye via Phabricator via cfe-commits]

t-tye accepted this revision.
t-tye added a comment.
This revision is now accepted and ready to land.

LGTM except minor comment.




Comment at: lib/Basic/Targets/AMDGPU.cpp:41-49
 0, // Default
 1, // opencl_global
 3, // opencl_local
 4, // opencl_constant
 5, // opencl_private
 0, // opencl_generic
 1, // cuda_device

Could the values be the AddrSpace enumerators?



Comment at: lib/Basic/Targets/AMDGPU.cpp:53-61
 5, // Default
 1, // opencl_global
 3, // opencl_local
 4, // opencl_constant
 5, // opencl_private
 0, // opencl_generic
 1, // cuda_device

Could the values be the AddrSpace enumerators?



Comment at: lib/Basic/Targets/AMDGPU.cpp:254
   }
-  auto IsGenericZero = isGenericZero(Triple);
   resetDataLayout(getTriple().getArch() == llvm::Triple::amdgcn
+  ? DataLayoutStringAMDGCN

To be consistent should this be:

```
!isAMDGCN(getTriple())
```


https://reviews.llvm.org/D43911



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

r326633 - [StaticAnalyzer] Fix some Clang-tidy modernize and Include What You Use warnings; other minor fixes (NFC).

[Eugene Zelenko via cfe-commits]

Author: eugenezelenko
Date: Fri Mar  2 15:11:49 2018
New Revision: 326633

URL: http://llvm.org/viewvc/llvm-project?rev=326633=rev
Log:
[StaticAnalyzer] Fix some Clang-tidy modernize and Include What You Use 
warnings; other minor fixes (NFC).

Modified:

cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/StoreRef.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h
cfe/trunk/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
cfe/trunk/lib/StaticAnalyzer/Core/CoreEngine.cpp
cfe/trunk/lib/StaticAnalyzer/Core/Store.cpp

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h?rev=326633=326632=326633=diff
==
--- 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h 
(original)
+++ 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h 
Fri Mar  2 15:11:49 2018
@@ -1,4 +1,4 @@
-//=== BasicValueFactory.h - Basic values for Path Sens analysis --*- C++ 
-*---//
+//==- BasicValueFactory.h - Basic values for Path Sens analysis --*- C++ 
-*-==//
 //
 // The LLVM Compiler Infrastructure
 //
@@ -17,12 +17,26 @@
 #define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_BASICVALUEFACTORY_H
 
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Type.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/APSIntType.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/StoreRef.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
+#include "llvm/ADT/APSInt.h"
+#include "llvm/ADT/FoldingSet.h"
+#include "llvm/ADT/ImmutableList.h"
+#include "llvm/ADT/iterator_range.h"
+#include "llvm/Support/Allocator.h"
+#include 
+#include 
+#include 
 
 namespace clang {
+
+class CXXBaseSpecifier;
+class DeclaratorDecl;
+
 namespace ento {
 
 class CompoundValData : public llvm::FoldingSetNode {
@@ -34,7 +48,8 @@ public:
 assert(NonLoc::isCompoundType(t));
   }
 
-  typedef llvm::ImmutableList::iterator iterator;
+  using iterator = llvm::ImmutableList::iterator;
+
   iterator begin() const { return L.begin(); }
   iterator end() const { return L.end(); }
 
@@ -47,6 +62,7 @@ public:
 class LazyCompoundValData : public llvm::FoldingSetNode {
   StoreRef store;
   const TypedValueRegion *region;
+
 public:
   LazyCompoundValData(const StoreRef , const TypedValueRegion *r)
   : store(st), region(r) {
@@ -63,16 +79,17 @@ public:
   void Profile(llvm::FoldingSetNodeID& ID) { Profile(ID, store, region); }
 };
 
-class PointerToMemberData: public llvm::FoldingSetNode {
+class PointerToMemberData : public llvm::FoldingSetNode {
   const DeclaratorDecl *D;
   llvm::ImmutableList L;
 
 public:
   PointerToMemberData(const DeclaratorDecl *D,
   llvm::ImmutableList L)
-: D(D), L(L) {}
+  : D(D), L(L) {}
+
+  using iterator = llvm::ImmutableList::iterator;
 
-  typedef llvm::ImmutableList::iterator iterator;
   iterator begin() const { return L.begin(); }
   iterator end() const { return L.end(); }
 
@@ -81,24 +98,25 @@ public:
 
   void Profile(llvm::FoldingSetNodeID& ID) { Profile(ID, D, L); }
   const DeclaratorDecl *getDeclaratorDecl() const {return D;}
+
   llvm::ImmutableList getCXXBaseList() const {
 return L;
   }
 };
 
 class BasicValueFactory {
-  typedef llvm::FoldingSet
-  APSIntSetTy;
+  using APSIntSetTy =
+  llvm::FoldingSet;
 
   ASTContext 
   llvm::BumpPtrAllocator& BPAlloc;
 
-  APSIntSetTy   APSIntSet;
-  void *PersistentSVals;
-  void *PersistentSValPairs;
+  APSIntSetTy APSIntSet;
+  void *PersistentSVals = nullptr;
+  void *PersistentSValPairs = nullptr;
 
   llvm::ImmutableList::Factory SValListFactory;
-  llvm::ImmutableList::Factory CXXBaseListFactory;
+  llvm::ImmutableList::Factory CXXBaseListFactory;
   llvm::FoldingSet  CompoundValDataSet;
   llvm::FoldingSet LazyCompoundValDataSet;
   llvm::FoldingSet PointerToMemberDataSet;
@@ -109,9 +127,8 @@ class BasicValueFactory {
 
 public:
   BasicValueFactory(ASTContext , llvm::BumpPtrAllocator )
-: Ctx(ctx), BPAlloc(Alloc), PersistentSVals(nullptr),
-  PersistentSValPairs(nullptr), SValListFactory(Alloc),
-  CXXBaseListFactory(Alloc) {}
+  : Ctx(ctx), BPAlloc(Alloc), SValListFactory(Alloc),
+CXXBaseListFactory(Alloc) {}
 
   ~BasicValueFactory();
 
@@ -147,57 +164,57 @@ public:
 return getValue(TargetType.convert(From));
   }
 
-  const llvm::APSInt& getIntValue(uint64_t 

[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:207
+  llvm::DenseMap>
+  GroupedReplacements;
+

jdemeule wrote:
> ioeric wrote:
> > I don't think we need to do the deduplication here anymore. AtomicChange 
> > handles duplicates for you. I think all you need to do here is to group 
> > replacements by files and later convert replacements to atomic changes.
> I think I wrongly use AtomicChange somewhere because it doesn't deduplicate 
> same replacement automatically.
> For exemple, in the test suite, basic test defines 2 time the same 
> replacement (adding 'override ' at offset 148) and I do not manage to avoid 
> AtomicChange to add 'override override '. This is why I have kept the 
> deduplicate step.
`AtomicChange` does deduplicate identical replacements but not insertions, 
because it wouldn't know whether users really want the insertions to be 
deduplicated or not (e.g. imagine a tool wants to insert two `)` at the same 
location). So it doesn't support that test case intentionally. In general, 
users (i.e. tools generating changes) are responsible for ensuring changes are 
deduplicated/applied in the expected way by using the correct interface (e.g. 
`replace`, `insert` etc).  I think it would probably make more sense to change 
the test to deduplicate identical non-insertion replacements. It would also 
make sense to add another test case where identical insertions are both applied.

For more semantics of conflicting/duplicated replacements, see 
https://github.com/llvm-mirror/clang/blob/master/include/clang/Tooling/Core/Replacement.h#L217
 



Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:279
+  if (!NewCode) {
+errs() << "Failed to apply fixes on " << File << "\n";
+return false;

jdemeule wrote:
> ioeric wrote:
> > You should handle the error in `llvm::Expected`. You could convert it to 
> > string and add to the error message with 
> > `llvm::toString(NewCode.takeError())`. It would be nice if we could have a 
> > test case for such cases.
> I will use `llvm::Expected` as you suggest.
> Do you have some ideas to made a test failed at this level?
> I will use llvm::Expected as you suggest.
I think `NewCode` is already `llvm::Expected`. You would only need 
to explicitly handle the error.

> Do you have some ideas to made a test failed at this level?
That's a good question. I think we would really need unit tests for the 
`ApplyReplacements` library in order to get better coverage. But it's probably 
out of the scope of this patch, so I'd also be fine without the test. Up to you 
:)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43764



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

[PATCH] D43995: Do not generate calls to fentry with __attribute__((no_instrument_function))

[John McCall via Phabricator via cfe-commits]

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

Seems reasonable to me.


Repository:
  rC Clang

https://reviews.llvm.org/D43995



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

[PATCH] D43741: [Analyzer] More accurate modeling about the increment operator of the operand with type bool.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Nice catch, thanks!




Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:1078-1080
+// The use of an operand of type bool with the ++ operators is deprecated
+// but valid untill C++17. And if the operand of the increment operator is
+// of type bool, it is set to true untill C++17.

`untill` seems to be deprecated in favor of `until`.



Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:1082
+if (U->getType()->isBooleanType() && U->isIncrementOp() &&
+AMgr.getLangOpts().CPlusPlus)
+  Result = svalBuilder.makeTruthVal(true, U->getType());

Doesn't `isBooleanType()` imply `CPlusPlus`? I guess we need to see if it works 
in Objective-C++ as well.



Comment at: test/Analysis/bool.cpp:65
+bool b = -10;
+clang_analyzer_dump(b); // expected-warning{{1 U1b}}
+  }

`dump()` exposes too much internals, we try to only use it for debugging, not 
for tests.

Would eg. `clang_analyzer_eval(b == 1)` be enough?


Repository:
  rC Clang

https://reviews.llvm.org/D43741



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

r326624 - PR36581: Support data recursion over Stmts in AST matchers.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Fri Mar  2 13:55:03 2018
New Revision: 326624

URL: http://llvm.org/viewvc/llvm-project?rev=326624=rev
Log:
PR36581: Support data recursion over Stmts in AST matchers.

Modified:
cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp

Modified: cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp?rev=326624=326623=326624=diff
==
--- cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp (original)
+++ cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp Fri Mar  2 13:55:03 2018
@@ -145,17 +145,22 @@ public:
 ScopedIncrement ScopedDepth();
 return (DeclNode == nullptr) || traverse(*DeclNode);
   }
-  bool TraverseStmt(Stmt *StmtNode) {
+  bool TraverseStmt(Stmt *StmtNode, DataRecursionQueue *Queue = nullptr) {
+// If we need to keep track of the depth, we can't perform data recursion.
+if (CurrentDepth == 0 || (CurrentDepth <= MaxDepth && MaxDepth < INT_MAX))
+  Queue = nullptr;
+
 ScopedIncrement ScopedDepth();
-const Stmt *StmtToTraverse = StmtNode;
-if (Traversal ==
-ASTMatchFinder::TK_IgnoreImplicitCastsAndParentheses) {
-  const Expr *ExprNode = dyn_cast_or_null(StmtNode);
-  if (ExprNode) {
+Stmt *StmtToTraverse = StmtNode;
+if (Traversal == ASTMatchFinder::TK_IgnoreImplicitCastsAndParentheses) {
+  if (Expr *ExprNode = dyn_cast_or_null(StmtNode))
 StmtToTraverse = ExprNode->IgnoreParenImpCasts();
-  }
 }
-return (StmtToTraverse == nullptr) || traverse(*StmtToTraverse);
+if (!StmtToTraverse)
+  return true;
+if (!match(*StmtToTraverse))
+  return false;
+return VisitorBase::TraverseStmt(StmtToTraverse, Queue);
   }
   // We assume that the QualType and the contained type are on the same
   // hierarchy level. Thus, we try to match either of them.
@@ -378,7 +383,7 @@ public:
   }
 
   bool TraverseDecl(Decl *DeclNode);
-  bool TraverseStmt(Stmt *StmtNode);
+  bool TraverseStmt(Stmt *StmtNode, DataRecursionQueue *Queue = nullptr);
   bool TraverseType(QualType TypeNode);
   bool TraverseTypeLoc(TypeLoc TypeNode);
   bool TraverseNestedNameSpecifier(NestedNameSpecifier *NNS);
@@ -841,12 +846,12 @@ bool MatchASTVisitor::TraverseDecl(Decl
   return RecursiveASTVisitor::TraverseDecl(DeclNode);
 }
 
-bool MatchASTVisitor::TraverseStmt(Stmt *StmtNode) {
+bool MatchASTVisitor::TraverseStmt(Stmt *StmtNode, DataRecursionQueue *Queue) {
   if (!StmtNode) {
 return true;
   }
   match(*StmtNode);
-  return RecursiveASTVisitor::TraverseStmt(StmtNode);
+  return RecursiveASTVisitor::TraverseStmt(StmtNode, Queue);
 }
 
 bool MatchASTVisitor::TraverseType(QualType TypeNode) {


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

[PATCH] D44032: Remove -i command line option, add -imultilib

[Erich Keane via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL326623: Remove -i command line option, add -imultilib 
(authored by erichkeane, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D44032?vs=136845=136847#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D44032

Files:
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/test/Driver/unknown-arg.c


Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -1764,7 +1764,7 @@
   Flags<[CC1Option]>;
 def ivfsoverlay : JoinedOrSeparate<["-"], "ivfsoverlay">, 
Group, Flags<[CC1Option]>,
   HelpText<"Overlay the virtual filesystem described by file over the real 
file system">;
-def i : Joined<["-"], "i">, Group;
+def imultilib : Separate<["-"], "imultilib">, Group;
 def keep__private__externs : Flag<["-"], "keep_private_externs">;
 def l : JoinedOrSeparate<["-"], "l">, Flags<[LinkerInput, RenderJoined]>,
 Group;
Index: cfe/trunk/test/Driver/unknown-arg.c
===
--- cfe/trunk/test/Driver/unknown-arg.c
+++ cfe/trunk/test/Driver/unknown-arg.c
@@ -1,5 +1,7 @@
-// RUN: not %clang %s -cake-is-lie -%0 -%d - -munknown-to-clang-option 
-print-stats -funknown-to-clang-option -### 2>&1 | \
+// RUN: not %clang %s -cake-is-lie -%0 -%d - -munknown-to-clang-option 
-print-stats -funknown-to-clang-option -ifoo -imultilib dir -### 2>&1 | \
 // RUN: FileCheck %s
+// RUN: %clang %s -imultilib dir -### 2>&1 | \
+// RUN: FileCheck %s --check-prefix=MULTILIB
 // RUN: not %clang %s -stdlibs=foo -hell -version -### 2>&1 | \
 // RUN: FileCheck %s --check-prefix=DID-YOU-MEAN
 // RUN: %clang_cl -cake-is-lie -%0 -%d - -munknown-to-clang-option 
-print-stats -funknown-to-clang-option -### -c -- %s 2>&1 | \
@@ -24,6 +26,8 @@
 // CHECK: error: unknown argument: '-munknown-to-clang-option'
 // CHECK: error: unknown argument: '-print-stats'
 // CHECK: error: unknown argument: '-funknown-to-clang-option'
+// CHECK: error: unknown argument: '-ifoo'
+// MULTILIB: warning: argument unused during compilation: '-imultilib dir'
 // DID-YOU-MEAN: error: unknown argument '-stdlibs=foo', did you mean 
'-stdlib=foo'?
 // DID-YOU-MEAN: error: unknown argument '-hell', did you mean '-help'?
 // DID-YOU-MEAN: error: unknown argument '-version', did you mean '--version'?


Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -1764,7 +1764,7 @@
   Flags<[CC1Option]>;
 def ivfsoverlay : JoinedOrSeparate<["-"], "ivfsoverlay">, Group, Flags<[CC1Option]>,
   HelpText<"Overlay the virtual filesystem described by file over the real file system">;
-def i : Joined<["-"], "i">, Group;
+def imultilib : Separate<["-"], "imultilib">, Group;
 def keep__private__externs : Flag<["-"], "keep_private_externs">;
 def l : JoinedOrSeparate<["-"], "l">, Flags<[LinkerInput, RenderJoined]>,
 Group;
Index: cfe/trunk/test/Driver/unknown-arg.c
===
--- cfe/trunk/test/Driver/unknown-arg.c
+++ cfe/trunk/test/Driver/unknown-arg.c
@@ -1,5 +1,7 @@
-// RUN: not %clang %s -cake-is-lie -%0 -%d - -munknown-to-clang-option -print-stats -funknown-to-clang-option -### 2>&1 | \
+// RUN: not %clang %s -cake-is-lie -%0 -%d - -munknown-to-clang-option -print-stats -funknown-to-clang-option -ifoo -imultilib dir -### 2>&1 | \
 // RUN: FileCheck %s
+// RUN: %clang %s -imultilib dir -### 2>&1 | \
+// RUN: FileCheck %s --check-prefix=MULTILIB
 // RUN: not %clang %s -stdlibs=foo -hell -version -### 2>&1 | \
 // RUN: FileCheck %s --check-prefix=DID-YOU-MEAN
 // RUN: %clang_cl -cake-is-lie -%0 -%d - -munknown-to-clang-option -print-stats -funknown-to-clang-option -### -c -- %s 2>&1 | \
@@ -24,6 +26,8 @@
 // CHECK: error: unknown argument: '-munknown-to-clang-option'
 // CHECK: error: unknown argument: '-print-stats'
 // CHECK: error: unknown argument: '-funknown-to-clang-option'
+// CHECK: error: unknown argument: '-ifoo'
+// MULTILIB: warning: argument unused during compilation: '-imultilib dir'
 // DID-YOU-MEAN: error: unknown argument '-stdlibs=foo', did you mean '-stdlib=foo'?
 // DID-YOU-MEAN: error: unknown argument '-hell', did you mean '-help'?
 // DID-YOU-MEAN: error: unknown argument '-version', did you mean '--version'?
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r326623 - Remove -i command line option, add -imultilib

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Fri Mar  2 13:53:25 2018
New Revision: 326623

URL: http://llvm.org/viewvc/llvm-project?rev=326623=rev
Log:
Remove -i command line option, add -imultilib

I discovered that '-i' is a command line option for the driver,
however it actually does not do anything and is not supported by any
other compiler. In fact, it is completely undocumented for Clang.

I found a couple of instances of people confusing it with one of
the variety of other command line options that control the driver.
Because of this, we should delete this option so that it is clear
that it isn't valid.

HOWEVER, I found that GCC DOES support -imultilib, which the -i
was hiding our lack of support for. We currently only use imultilib
for the purpose of forwarding to gfortran (in a specific test written
  by chandlerc for this purpose).

  imultilib is a rarely used (if ever?) feature that I could find no
  references to on the internet, and in fact, my company's massive test
  suite has zero references to it ever being used.

  SO, this patch removes the -i option so that we will now give an error
  on its usage (so that it won't be confused with -I), and replaces it with
  -imultilib, which is now specified as a gfortran_group option.

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

Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/test/Driver/unknown-arg.c

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=326623=326622=326623=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Fri Mar  2 13:53:25 2018
@@ -1764,7 +1764,7 @@ def iwithsysroot : JoinedOrSeparate<["-"
   Flags<[CC1Option]>;
 def ivfsoverlay : JoinedOrSeparate<["-"], "ivfsoverlay">, 
Group, Flags<[CC1Option]>,
   HelpText<"Overlay the virtual filesystem described by file over the real 
file system">;
-def i : Joined<["-"], "i">, Group;
+def imultilib : Separate<["-"], "imultilib">, Group;
 def keep__private__externs : Flag<["-"], "keep_private_externs">;
 def l : JoinedOrSeparate<["-"], "l">, Flags<[LinkerInput, RenderJoined]>,
 Group;

Modified: cfe/trunk/test/Driver/unknown-arg.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/unknown-arg.c?rev=326623=326622=326623=diff
==
--- cfe/trunk/test/Driver/unknown-arg.c (original)
+++ cfe/trunk/test/Driver/unknown-arg.c Fri Mar  2 13:53:25 2018
@@ -1,5 +1,7 @@
-// RUN: not %clang %s -cake-is-lie -%0 -%d - -munknown-to-clang-option 
-print-stats -funknown-to-clang-option -### 2>&1 | \
+// RUN: not %clang %s -cake-is-lie -%0 -%d - -munknown-to-clang-option 
-print-stats -funknown-to-clang-option -ifoo -imultilib dir -### 2>&1 | \
 // RUN: FileCheck %s
+// RUN: %clang %s -imultilib dir -### 2>&1 | \
+// RUN: FileCheck %s --check-prefix=MULTILIB
 // RUN: not %clang %s -stdlibs=foo -hell -version -### 2>&1 | \
 // RUN: FileCheck %s --check-prefix=DID-YOU-MEAN
 // RUN: %clang_cl -cake-is-lie -%0 -%d - -munknown-to-clang-option 
-print-stats -funknown-to-clang-option -### -c -- %s 2>&1 | \
@@ -24,6 +26,8 @@
 // CHECK: error: unknown argument: '-munknown-to-clang-option'
 // CHECK: error: unknown argument: '-print-stats'
 // CHECK: error: unknown argument: '-funknown-to-clang-option'
+// CHECK: error: unknown argument: '-ifoo'
+// MULTILIB: warning: argument unused during compilation: '-imultilib dir'
 // DID-YOU-MEAN: error: unknown argument '-stdlibs=foo', did you mean 
'-stdlib=foo'?
 // DID-YOU-MEAN: error: unknown argument '-hell', did you mean '-help'?
 // DID-YOU-MEAN: error: unknown argument '-version', did you mean '--version'?


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

[PATCH] D44032: Remove -i command line option, add -imultilib

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


https://reviews.llvm.org/D44032



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

[PATCH] D44032: Remove -i command line option, add -imultilib

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 136845.
erichkeane added a comment.

Talked to Aaron and came up with a way of covering with a testcase.


https://reviews.llvm.org/D44032

Files:
  include/clang/Driver/Options.td
  test/Driver/unknown-arg.c


Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1764,7 +1764,7 @@
   Flags<[CC1Option]>;
 def ivfsoverlay : JoinedOrSeparate<["-"], "ivfsoverlay">, 
Group, Flags<[CC1Option]>,
   HelpText<"Overlay the virtual filesystem described by file over the real 
file system">;
-def i : Joined<["-"], "i">, Group;
+def imultilib : Separate<["-"], "imultilib">, Group;
 def keep__private__externs : Flag<["-"], "keep_private_externs">;
 def l : JoinedOrSeparate<["-"], "l">, Flags<[LinkerInput, RenderJoined]>,
 Group;
Index: test/Driver/unknown-arg.c
===
--- test/Driver/unknown-arg.c
+++ test/Driver/unknown-arg.c
@@ -1,5 +1,7 @@
-// RUN: not %clang %s -cake-is-lie -%0 -%d - -munknown-to-clang-option 
-print-stats -funknown-to-clang-option -### 2>&1 | \
+// RUN: not %clang %s -cake-is-lie -%0 -%d - -munknown-to-clang-option 
-print-stats -funknown-to-clang-option -ifoo -imultilib dir -### 2>&1 | \
 // RUN: FileCheck %s
+// RUN: %clang %s -imultilib dir -### 2>&1 | \
+// RUN: FileCheck %s --check-prefix=MULTILIB
 // RUN: not %clang %s -stdlibs=foo -hell -version -### 2>&1 | \
 // RUN: FileCheck %s --check-prefix=DID-YOU-MEAN
 // RUN: %clang_cl -cake-is-lie -%0 -%d - -munknown-to-clang-option 
-print-stats -funknown-to-clang-option -### -c -- %s 2>&1 | \
@@ -24,6 +26,8 @@
 // CHECK: error: unknown argument: '-munknown-to-clang-option'
 // CHECK: error: unknown argument: '-print-stats'
 // CHECK: error: unknown argument: '-funknown-to-clang-option'
+// CHECK: error: unknown argument: '-ifoo'
+// MULTILIB: warning: argument unused during compilation: '-imultilib dir'
 // DID-YOU-MEAN: error: unknown argument '-stdlibs=foo', did you mean 
'-stdlib=foo'?
 // DID-YOU-MEAN: error: unknown argument '-hell', did you mean '-help'?
 // DID-YOU-MEAN: error: unknown argument '-version', did you mean '--version'?


Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1764,7 +1764,7 @@
   Flags<[CC1Option]>;
 def ivfsoverlay : JoinedOrSeparate<["-"], "ivfsoverlay">, Group, Flags<[CC1Option]>,
   HelpText<"Overlay the virtual filesystem described by file over the real file system">;
-def i : Joined<["-"], "i">, Group;
+def imultilib : Separate<["-"], "imultilib">, Group;
 def keep__private__externs : Flag<["-"], "keep_private_externs">;
 def l : JoinedOrSeparate<["-"], "l">, Flags<[LinkerInput, RenderJoined]>,
 Group;
Index: test/Driver/unknown-arg.c
===
--- test/Driver/unknown-arg.c
+++ test/Driver/unknown-arg.c
@@ -1,5 +1,7 @@
-// RUN: not %clang %s -cake-is-lie -%0 -%d - -munknown-to-clang-option -print-stats -funknown-to-clang-option -### 2>&1 | \
+// RUN: not %clang %s -cake-is-lie -%0 -%d - -munknown-to-clang-option -print-stats -funknown-to-clang-option -ifoo -imultilib dir -### 2>&1 | \
 // RUN: FileCheck %s
+// RUN: %clang %s -imultilib dir -### 2>&1 | \
+// RUN: FileCheck %s --check-prefix=MULTILIB
 // RUN: not %clang %s -stdlibs=foo -hell -version -### 2>&1 | \
 // RUN: FileCheck %s --check-prefix=DID-YOU-MEAN
 // RUN: %clang_cl -cake-is-lie -%0 -%d - -munknown-to-clang-option -print-stats -funknown-to-clang-option -### -c -- %s 2>&1 | \
@@ -24,6 +26,8 @@
 // CHECK: error: unknown argument: '-munknown-to-clang-option'
 // CHECK: error: unknown argument: '-print-stats'
 // CHECK: error: unknown argument: '-funknown-to-clang-option'
+// CHECK: error: unknown argument: '-ifoo'
+// MULTILIB: warning: argument unused during compilation: '-imultilib dir'
 // DID-YOU-MEAN: error: unknown argument '-stdlibs=foo', did you mean '-stdlib=foo'?
 // DID-YOU-MEAN: error: unknown argument '-hell', did you mean '-help'?
 // DID-YOU-MEAN: error: unknown argument '-version', did you mean '--version'?
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r326622 - Don't claim that va_start has special semantic checks

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Fri Mar  2 13:41:08 2018
New Revision: 326622

URL: http://llvm.org/viewvc/llvm-project?rev=326622=rev
Log:
Don't claim that va_start has special semantic checks

We don't have special checks for BI_va_start in
Sema::CheckBuiltinFunctionCall, so setting the 't' flag for va_start in
Builtins.def disables semantic checking for it. That's not desired, and
IRGen crashes when it tries to generate a call to va_start that doesn't
have at least one argument.

Follow-up to r322573

Fixes PR36565

Modified:
cfe/trunk/include/clang/Basic/Builtins.def
cfe/trunk/test/Sema/varargs.c

Modified: cfe/trunk/include/clang/Basic/Builtins.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=326622=326621=326622=diff
==
--- cfe/trunk/include/clang/Basic/Builtins.def (original)
+++ cfe/trunk/include/clang/Basic/Builtins.def Fri Mar  2 13:41:08 2018
@@ -803,7 +803,7 @@ LIBBUILTIN(_setjmpex, "iJ", "fj",   "set
 
 // C99 library functions
 // C99 stdarg.h
-LIBBUILTIN(va_start, "vA.",   "fnt",   "stdarg.h", ALL_LANGUAGES)
+LIBBUILTIN(va_start, "vA.",   "fn","stdarg.h", ALL_LANGUAGES)
 LIBBUILTIN(va_end, "vA",  "fn","stdarg.h", ALL_LANGUAGES)
 LIBBUILTIN(va_copy, "vAA","fn","stdarg.h", ALL_LANGUAGES)
 // C99 stdlib.h

Modified: cfe/trunk/test/Sema/varargs.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/varargs.c?rev=326622=326621=326622=diff
==
--- cfe/trunk/test/Sema/varargs.c (original)
+++ cfe/trunk/test/Sema/varargs.c Fri Mar  2 13:41:08 2018
@@ -112,3 +112,12 @@ void f13(enum E1 e, ...) {
 #endif
   __builtin_va_end(va);
 }
+
+void f14(int e, ...) {
+  // expected-warning@+3 {{implicitly declaring library function 'va_start'}}
+  // expected-note@+2 {{include the header }}
+  // expected-error@+1 {{too few arguments to function call}}
+  va_start();
+  __builtin_va_list va;
+  va_start(va, e);
+}


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

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

[James Chou via Phabricator via cfe-commits]

uohcsemaj added a comment.

Ping!


https://reviews.llvm.org/D40988



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

[PATCH] D44044: [analyzer] Don't throw NSNumberObjectConversion warning on object initialization in if-expression

[George Karpenkov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC326619: [analyzer] Dont throw NSNumberObjectConversion 
warning on object… (authored by george.karpenkov, committed by ).
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D44044

Files:
  lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp
  test/Analysis/number-object-conversion.mm


Index: test/Analysis/number-object-conversion.mm
===
--- test/Analysis/number-object-conversion.mm
+++ test/Analysis/number-object-conversion.mm
@@ -0,0 +1,13 @@
+// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -fblocks -fobjc-arc -w 
-analyzer-checker=osx.NumberObjectConversion -analyzer-config 
osx.NumberObjectConversion:Pedantic=true %s -verify
+
+#include "Inputs/system-header-simulator-objc.h"
+
+NSNumber* generateNumber();
+
+// expected-no-diagnostics
+int test_initialization_in_ifstmt() { // Don't warn on initialization in guard.
+  if (NSNumber* number = generateNumber()) { // no-warning
+return 0;
+  }
+  return 1;
+}
Index: lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp
+++ lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp
@@ -270,8 +270,10 @@
hasRHS(SuspiciousNumberObjectExprM)));
 
   auto ConversionThroughBranchingM =
-  ifStmt(hasCondition(SuspiciousNumberObjectExprM))
-  .bind("pedantic");
+  ifStmt(allOf(
+  hasCondition(SuspiciousNumberObjectExprM),
+  unless(hasConditionVariableStatement(declStmt())
+  ))).bind("pedantic");
 
   auto ConversionThroughCallM =
   callExpr(hasAnyArgument(allOf(hasType(SuspiciousScalarTypeM),


Index: test/Analysis/number-object-conversion.mm
===
--- test/Analysis/number-object-conversion.mm
+++ test/Analysis/number-object-conversion.mm
@@ -0,0 +1,13 @@
+// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -fblocks -fobjc-arc -w -analyzer-checker=osx.NumberObjectConversion -analyzer-config osx.NumberObjectConversion:Pedantic=true %s -verify
+
+#include "Inputs/system-header-simulator-objc.h"
+
+NSNumber* generateNumber();
+
+// expected-no-diagnostics
+int test_initialization_in_ifstmt() { // Don't warn on initialization in guard.
+  if (NSNumber* number = generateNumber()) { // no-warning
+return 0;
+  }
+  return 1;
+}
Index: lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp
+++ lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp
@@ -270,8 +270,10 @@
hasRHS(SuspiciousNumberObjectExprM)));
 
   auto ConversionThroughBranchingM =
-  ifStmt(hasCondition(SuspiciousNumberObjectExprM))
-  .bind("pedantic");
+  ifStmt(allOf(
+  hasCondition(SuspiciousNumberObjectExprM),
+  unless(hasConditionVariableStatement(declStmt())
+  ))).bind("pedantic");
 
   auto ConversionThroughCallM =
   callExpr(hasAnyArgument(allOf(hasType(SuspiciousScalarTypeM),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r326619 - [analyzer] Don't throw NSNumberObjectConversion warning on object initialization in if-expression

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Fri Mar  2 13:34:24 2018
New Revision: 326619

URL: http://llvm.org/viewvc/llvm-project?rev=326619=rev
Log:
[analyzer] Don't throw NSNumberObjectConversion warning on object 
initialization in if-expression

```
if (NSNumber* x = ...)
```
is a reasonable pattern in objc++, we should not warn on it.

rdar://35152234

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

Added:
cfe/trunk/test/Analysis/number-object-conversion.mm
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp

Modified: 
cfe/trunk/lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp?rev=326619=326618=326619=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp 
(original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp Fri 
Mar  2 13:34:24 2018
@@ -270,8 +270,10 @@ void NumberObjectConversionChecker::chec
hasRHS(SuspiciousNumberObjectExprM)));
 
   auto ConversionThroughBranchingM =
-  ifStmt(hasCondition(SuspiciousNumberObjectExprM))
-  .bind("pedantic");
+  ifStmt(allOf(
+  hasCondition(SuspiciousNumberObjectExprM),
+  unless(hasConditionVariableStatement(declStmt())
+  ))).bind("pedantic");
 
   auto ConversionThroughCallM =
   callExpr(hasAnyArgument(allOf(hasType(SuspiciousScalarTypeM),

Added: cfe/trunk/test/Analysis/number-object-conversion.mm
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/number-object-conversion.mm?rev=326619=auto
==
--- cfe/trunk/test/Analysis/number-object-conversion.mm (added)
+++ cfe/trunk/test/Analysis/number-object-conversion.mm Fri Mar  2 13:34:24 2018
@@ -0,0 +1,13 @@
+// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -fblocks -fobjc-arc -w 
-analyzer-checker=osx.NumberObjectConversion -analyzer-config 
osx.NumberObjectConversion:Pedantic=true %s -verify
+
+#include "Inputs/system-header-simulator-objc.h"
+
+NSNumber* generateNumber();
+
+// expected-no-diagnostics
+int test_initialization_in_ifstmt() { // Don't warn on initialization in guard.
+  if (NSNumber* number = generateNumber()) { // no-warning
+return 0;
+  }
+  return 1;
+}


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

[PATCH] D44032: Remove -i command line option, add -imultilib

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

testcase 


Repository:
  rC Clang

https://reviews.llvm.org/D44032



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

[PATCH] D43847: [clang-tidy] Add check: replace string::find(...) == 0 with absl::StartsWith

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/absl/StringFindStartswithCheck.cpp:17-19
+  auto stringClassMatcher = anyOf(cxxRecordDecl(hasName("string")),
+  cxxRecordDecl(hasName("__versa_string")),
+  cxxRecordDecl(hasName("basic_string")));

niko wrote:
> hokein wrote:
> > aaron.ballman wrote:
> > > These should be using elaborated type specifiers to ensure we get the 
> > > correct class, not some class that happens to have the same name. Also, 
> > > this list should be configurable so that users can add their own entries 
> > > to it.
> > +1, we could add a `StringLikeClasses` option.
> I've made the list configurable. Can you clarify what I would need to add in 
> terms of type specifiers?
You should be looking for a record decl with the name `::std::string` so that 
you don't run into issues with things like: `namespace terrible_person { class 
string; }`



Comment at: clang-tidy/absl/StringFindStartswithCheck.cpp:44-47
+  const auto *expr = result.Nodes.getNodeAs("expr");
+  const auto *needle = result.Nodes.getNodeAs("needle");
+  const auto *haystack = result.Nodes.getNodeAs("findexpr")
+ ->getImplicitObjectArgument();

niko wrote:
> aaron.ballman wrote:
> > Btw, these can use `auto` because the type is spelled out in the 
> > initialization.
> Thanks for the clarification. Is this true even for `Haystack` (as 
> `->getImplicitObjectArgument()`'s type is Expr)?
It's not true for `haystack`, I think I highlighted one too many lines there. 
:-)



Comment at: clang-tidy/absl/StringFindStartswithCheck.cpp:72
+   .str())
+  << FixItHint::CreateReplacement(expr->getSourceRange(),
+  (startswith_str + "(" +

niko wrote:
> aaron.ballman wrote:
> > This fixit should be guarded in case it lands in the middle of a macro for 
> > some reason.
> Sorry, could you elaborate?
We generally don't issue fixit hints if the source location for the fixit is in 
a macro. You can test that with `expr->getLocStart().isMacroID()`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43847



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

[PATCH] D43911: [AMDGPU] Clean up old address space mapping and fix constant address space value

[Brian Sumner via Phabricator via cfe-commits]

b-sumner added a comment.

Looks fine to me.


https://reviews.llvm.org/D43911



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

r326607 - Range-ify a for loop. NFC

[George Burgess IV via cfe-commits]

Author: gbiv
Date: Fri Mar  2 12:10:38 2018
New Revision: 326607

URL: http://llvm.org/viewvc/llvm-project?rev=326607=rev
Log:
Range-ify a for loop. NFC

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

Modified: cfe/trunk/lib/CodeGen/CGBlocks.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBlocks.cpp?rev=326607=326606=326607=diff
==
--- cfe/trunk/lib/CodeGen/CGBlocks.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBlocks.cpp Fri Mar  2 12:10:38 2018
@@ -713,11 +713,8 @@ static void enterBlockScope(CodeGenFunct
 /// kind of cleanup object is a BlockDecl*.
 void CodeGenFunction::enterNonTrivialFullExpression(const ExprWithCleanups *E) 
{
   assert(E->getNumObjects() != 0);
-  ArrayRef cleanups = E->getObjects();
-  for (ArrayRef::iterator
- i = cleanups.begin(), e = cleanups.end(); i != e; ++i) {
-enterBlockScope(*this, *i);
-  }
+  for (const ExprWithCleanups::CleanupObject  : E->getObjects())
+enterBlockScope(*this, C);
 }
 
 /// Find the layout for the given block in a linked list and remove it.


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

[PATCH] D43847: [clang-tidy] Add check: replace string::find(...) == 0 with absl::StartsWith

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

In https://reviews.llvm.org/D43847#1025833, @niko wrote:

> Thanks everyone for your comments! I renamed the namespace and filenames to 
> 'abseil'.
>
> @Eugene.Zelenko, definitely interested in extending this to a C++20 modernize 
> check and adding `absl::EndsWith()` support,  would it be OK though to do 
> this in a later patch?


I think will be better to do this now, since you'll need to create base class 
and specializations for Abseil and C++17.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43847



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

[PATCH] D44039: [Sema] Make getCurFunction() return null outside function parsing

[Reid Kleckner via Phabricator via cfe-commits]

rnk created this revision.
rnk added a reviewer: rsmith.

Before this patch, Sema pre-allocated a FunctionScopeInfo and kept it in
the first, always present element of the FunctionScopes stack. This
meant that Sema::getCurFunction would return a pointer to this
pre-allocated object when parsing code outside a function body. This is
pretty much always a bug, so this patch moves the pre-allocated object
into a separate unique_ptr. This should make bugs like PR36536 a lot
more obvious.

As you can see from this patch, there were a number of places that
unconditionally assumed they were always called inside a function.
However, there are also many places that null checked the result of
getCurFunction(), so I think this is a reasonable direction.


https://reviews.llvm.org/D44039

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExprCXX.cpp

Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -1114,8 +1114,9 @@
 
   assert((!ByCopy || Explicit) && "cannot implicitly capture *this by value");
 
-  const unsigned MaxFunctionScopesIndex = FunctionScopeIndexToStopAt ?
-*FunctionScopeIndexToStopAt : FunctionScopes.size() - 1;
+  const int MaxFunctionScopesIndex = FunctionScopeIndexToStopAt
+ ? *FunctionScopeIndexToStopAt
+ : FunctionScopes.size() - 1;
 
   // Check that we can capture the *enclosing object* (referred to by '*this')
   // by the capturing-entity/closure (lambda/block/etc) at
@@ -1141,7 +1142,7 @@
 
 
   unsigned NumCapturingClosures = 0;
-  for (unsigned idx = MaxFunctionScopesIndex; idx != 0; idx--) {
+  for (int idx = MaxFunctionScopesIndex; idx >= 0; idx--) {
 if (CapturingScopeInfo *CSI =
 dyn_cast(FunctionScopes[idx])) {
   if (CSI->CXXThisCaptureIndex != 0) {
@@ -1196,8 +1197,8 @@
   // FIXME: We need to delay this marking in PotentiallyPotentiallyEvaluated
   // contexts.
   QualType ThisTy = getCurrentThisType();
-  for (unsigned idx = MaxFunctionScopesIndex; NumCapturingClosures;
-  --idx, --NumCapturingClosures) {
+  for (int idx = MaxFunctionScopesIndex; NumCapturingClosures;
+   --idx, --NumCapturingClosures) {
 CapturingScopeInfo *CSI = cast(FunctionScopes[idx]);
 Expr *ThisExpr = nullptr;
 
@@ -7176,9 +7177,6 @@
 
   const bool IsFullExprInstantiationDependent = FE->isInstantiationDependent();
 
-  ArrayRef FunctionScopesArrayRef(
-  S.FunctionScopes.data(), S.FunctionScopes.size());
-
   // All the potentially captureable variables in the current nested
   // lambda (within a generic outer lambda), must be captured by an
   // outer lambda that is enclosed within a non-dependent context.
@@ -7207,7 +7205,7 @@
 // capture the variable in that lambda (and all its enclosing lambdas).
 if (const Optional Index =
 getStackIndexOfNearestEnclosingCaptureCapableLambda(
-FunctionScopesArrayRef, Var, S)) {
+S.FunctionScopes, Var, S)) {
   const unsigned FunctionScopeIndexOfCapturableLambda = Index.getValue();
   MarkVarDeclODRUsed(Var, VarExpr->getExprLoc(), S,
  );
@@ -7243,7 +7241,7 @@
 // 'this' in that lambda (and all its enclosing lambdas).
 if (const Optional Index =
 getStackIndexOfNearestEnclosingCaptureCapableLambda(
-FunctionScopesArrayRef, /*0 is 'this'*/ nullptr, S)) {
+S.FunctionScopes, /*0 is 'this'*/ nullptr, S)) {
   const unsigned FunctionScopeIndexOfCapturableLambda = Index.getValue();
   S.CheckCXXThisCapture(CurrentLSI->PotentialThisCaptureLocation,
 /*Explicit*/ false, /*BuildAndDiagnose*/ true,
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -5747,7 +5747,8 @@
   TypeSourceInfo *TInfo = NewTD->getTypeSourceInfo();
   QualType T = TInfo->getType();
   if (T->isVariablyModifiedType()) {
-getCurFunction()->setHasBranchProtectedScope();
+if (getCurFunction())
+  getCurFunction()->setHasBranchProtectedScope();
 
 if (S->getFnParent() == nullptr) {
   bool SizeIsNegative;
@@ -7407,7 +7408,8 @@
   bool isVM = T->isVariablyModifiedType();
   if (isVM || NewVD->hasAttr() ||
   NewVD->hasAttr())
-getCurFunction()->setHasBranchProtectedScope();
+if (getCurFunction())
+  getCurFunction()->setHasBranchProtectedScope();
 
   if ((isVM && NewVD->hasLinkage()) ||
   (T->isVariableArrayType() && NewVD->hasGlobalStorage())) {
@@ -10643,7 +10645,7 @@
   return;
 }
 
-if (VDecl->hasLocalStorage())
+if (VDecl->hasLocalStorage() && getCurFunction())
   

[PATCH] D43847: [clang-tidy] Add check: replace string::find(...) == 0 with absl::StartsWith

[Niko Weh via Phabricator via cfe-commits]

niko added a comment.

Thanks everyone for your comments! I renamed the namespace and filenames to 
'abseil'.

@Eugene.Zelenko, definitely interested in extending this to a C++20 modernize 
check and adding `absl::EndsWith()` support,  would it be OK though to do this 
in a later patch?




Comment at: clang-tidy/absl/StringFindStartswithCheck.cpp:17-19
+  auto stringClassMatcher = anyOf(cxxRecordDecl(hasName("string")),
+  cxxRecordDecl(hasName("__versa_string")),
+  cxxRecordDecl(hasName("basic_string")));

hokein wrote:
> aaron.ballman wrote:
> > These should be using elaborated type specifiers to ensure we get the 
> > correct class, not some class that happens to have the same name. Also, 
> > this list should be configurable so that users can add their own entries to 
> > it.
> +1, we could add a `StringLikeClasses` option.
I've made the list configurable. Can you clarify what I would need to add in 
terms of type specifiers?



Comment at: clang-tidy/absl/StringFindStartswithCheck.cpp:44-47
+  const auto *expr = result.Nodes.getNodeAs("expr");
+  const auto *needle = result.Nodes.getNodeAs("needle");
+  const auto *haystack = result.Nodes.getNodeAs("findexpr")
+ ->getImplicitObjectArgument();

aaron.ballman wrote:
> Btw, these can use `auto` because the type is spelled out in the 
> initialization.
Thanks for the clarification. Is this true even for `Haystack` (as 
`->getImplicitObjectArgument()`'s type is Expr)?



Comment at: clang-tidy/absl/StringFindStartswithCheck.cpp:72
+   .str())
+  << FixItHint::CreateReplacement(expr->getSourceRange(),
+  (startswith_str + "(" +

aaron.ballman wrote:
> This fixit should be guarded in case it lands in the middle of a macro for 
> some reason.
Sorry, could you elaborate?



Comment at: clang-tidy/absl/StringFindStartswithCheck.cpp:81
+  auto include_hint = include_inserter_->CreateIncludeInsertion(
+  source.getFileID(expr->getLocStart()), 
"third_party/absl/strings/match.h",
+  false);

hokein wrote:
> The include path maybe different in different project.
> 
> I think we also need an option for the inserting header, and make it default 
> to `absl/strings/match.h`.
Instead of having a "AbseilStringsMatchHeader" option, does it make sense to 
have a "AbseilIncludeDir" option here & default that to `absl`? (esp. if there 
are going to be other abseil-checks in the future...)



Comment at: docs/clang-tidy/checks/absl-string-find-startswith.rst:18
+``if (absl::StartsWith(s, "Hello World")) { /* do something */ };``
+

Eugene.Zelenko wrote:
> Is there any online documentation about such usage? If so please refer to in 
> at. See other guidelines as example.
I don't believe there is documentation for this yet. The [comment in the 
header](https://github.com/abseil/abseil-cpp/blob/9850abf25d8efcdc1ff752f1eeef13b640c4ead4/absl/strings/match.h#L50)
 explains what it does but doesn't have an explicit usage example. (If that 
qualifies I'll obviously include it?)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43847



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

[PATCH] D43847: [clang-tidy] Add check: replace string::find(...) == 0 with absl::StartsWith

[Niko Weh via Phabricator via cfe-commits]

niko updated this revision to Diff 136821.
niko marked 23 inline comments as done.
niko added a comment.

Renamed 'absl' to 'abseil', applied reviewer suggestions


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43847

Files:
  clang-tidy/CMakeLists.txt
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/StringFindStartswithCheck.cpp
  clang-tidy/abseil/StringFindStartswithCheck.h
  clang-tidy/plugin/CMakeLists.txt
  clang-tidy/tool/CMakeLists.txt
  clang-tidy/tool/ClangTidyMain.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-string-find-startswith.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-string-find-startswith.cpp

Index: test/clang-tidy/abseil-string-find-startswith.cpp
===
--- test/clang-tidy/abseil-string-find-startswith.cpp
+++ test/clang-tidy/abseil-string-find-startswith.cpp
@@ -0,0 +1,54 @@
+// RUN: %check_clang_tidy %s abseil-string-find-startswith %t
+// -std=c++11
+
+namespace std {
+template  class allocator {};
+template  class char_traits {};
+template ,
+  typename A = std::allocator>
+struct basic_string {
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const C *, const A  = A());
+  ~basic_string();
+  int find(basic_string s, int pos = 0);
+  int find(const char *s, int pos = 0);
+};
+typedef basic_string string;
+typedef basic_string wstring;
+} // namespace std
+
+std::string foo(std::string);
+std::string bar();
+
+void tests(std::string s) {
+  s.find("a") == 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StartsWith instead of find() == 0 [abseil-string-find-startswith]
+  // CHECK-FIXES: {{^[[:space:]]*}}absl::StartsWith(s, "a");{{$}}
+
+  s.find(s) == 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use absl::StartsWith instead of find() == 0 [abseil-string-find-startswith]
+  // CHECK-FIXES: {{^[[:space:]]*}}absl::StartsWith(s, s);{{$}}
+
+  s.find("aaa") != 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StartsWith instead of find() != 0 [abseil-string-find-startswith]
+  // CHECK-FIXES: {{^[[:space:]]*}}!absl::StartsWith(s, "aaa");{{$}}
+
+  s.find(foo(foo(bar( != 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StartsWith instead of find() != 0 [abseil-string-find-startswith]
+  // CHECK-FIXES: {{^[[:space:]]*}}!absl::StartsWith(s, foo(foo(bar(;{{$}}
+
+  if (s.find("") == 0) { /* do something */ }
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use absl::StartsWith instead of find() == 0 [abseil-string-find-startswith]
+  // CHECK-FIXES: {{^[[:space:]]*}}if (absl::StartsWith(s, "")) { /* do something */ }{{$}}
+
+  0 != s.find("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use !absl::StartsWith instead of find() != 0 [abseil-string-find-startswith]
+  // CHECK-FIXES: {{^[[:space:]]*}}!absl::StartsWith(s, "a");{{$}}
+
+
+  // expressions that don't trigger the check are here.
+  s.find("a", 1) == 0;
+  s.find("a", 1) == 1;
+  s.find("a") == 1;
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -4,6 +4,7 @@
 =
 
 .. toctree::
+   abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-string-find-startswith.rst
===
--- docs/clang-tidy/checks/abseil-string-find-startswith.rst
+++ docs/clang-tidy/checks/abseil-string-find-startswith.rst
@@ -0,0 +1,41 @@
+.. title:: clang-tidy - abseil-string-find-startswith
+
+abseil-string-find-startswith
+==
+
+Checks whether a ``std::string::find()`` result is compared with 0, and
+suggests replacing with ``absl::StartsWith()``. This is both a readability and
+performance issue.
+
+.. code-block:: c++
+
+  string s = "...";
+  if (s.find("Hello World") == 0) { /* do something */ }
+
+becomes
+
+
+.. code-block:: c++
+
+  string s = "...";
+  if (absl::StartsWith(s, "Hello World")) { /* do something */ }
+
+
+Options
+---
+
+.. option:: StringLikeClasses
+
+   Semicolon-separated list of names of string-like classes. By default only
+   ``std::basic_string`` is considered. The list of methods to consired is
+   fixed.
+
+.. option:: IncludeStyle
+
+   A string specifying which include-style is used, `llvm` or `google`. Default
+   is `llvm`.
+
+.. option:: AbseilStringsMatchHeader
+
+   The location of Abseil's ``strings/match.h``. Defaults to
+   ``absl/strings/match.h``.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -75,6 +75,15 @@
 
   Warns if a class inherits from multiple classes that are not pure virtual.
 
+- New `abseil` module for checks 

[PATCH] D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D40737#1025777, @lebedev.ri wrote:

> In https://reviews.llvm.org/D40737#1024120, @JonasToth wrote:
>
> > After long inactivity (sorry!) i had a chance to look at it again:
> >
> >   switch(i) {
> >   case 0:;
> >   case 1:;
> >   case 2:;
> >   ...
> >   }
> >
> >
> > does *NOT* lead to the stack overflow. This is most likely an issue in the 
> > AST:
> >  https://godbolt.org/g/vZw2BD
> >
> > Empty case labels do nest, an empty statement prevents this. The nesting 
> > leads most likely to the deep recursion. I will file a bug for it.
>
>
> FWIW here are my 5 cent: this is a preexisting bug. Your testcase just 
> happened to expose it.
>  I'd file the bug, and then simply adjust the testcases here not to trigger 
> it and re-land this diff.
>
> I'm not sure what is to be gained by not doing that.
>  Of course, the bug is a bug, and should  be fixed, but it exists regardless 
> of this differential...


Just because a particular check triggers an issue elsewhere doesn't mean we 
should gloss over the issue in the check. That just kicks the can farther down 
the road so that our users find the issue. In this case, the check is likely to 
push users *towards* discovering that AST bug rather than away from it which is 
why I'm giving the push-back.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40737



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

[PATCH] D43911: [AMDGPU] Clean up old address space mapping and fix constant address space value

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

Ping


https://reviews.llvm.org/D43911



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

[PATCH] D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D40737#1024120, @JonasToth wrote:

> After long inactivity (sorry!) i had a chance to look at it again:
>
>   switch(i) {
>   case 0:;
>   case 1:;
>   case 2:;
>   ...
>   }
>
>
> does *NOT* lead to the stack overflow. This is most likely an issue in the 
> AST:
>  https://godbolt.org/g/vZw2BD
>
> Empty case labels do nest, an empty statement prevents this. The nesting 
> leads most likely to the deep recursion. I will file a bug for it.


FWIW here are my 5 cent: this is a preexisting bug. Your testcase just happened 
to expose it.
I'd file the bug, and then simply adjust the testcases here not to trigger it 
and re-land this diff.

I'm not sure what is to be gained by not doing that.
Of course, the bug is a bug, and should  be fixed, but it exists regardless of 
this differential...


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40737



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

[clang-tools-extra] r326605 - Adds a clang-tidy test for a failing assertion in the misc-misplaced-const check.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Fri Mar  2 11:14:34 2018
New Revision: 326605

URL: http://llvm.org/viewvc/llvm-project?rev=326605=rev
Log:
Adds a clang-tidy test for a failing assertion in the misc-misplaced-const 
check.

The test case previously triggered an assertion in the AST matchers because the 
QualType being matched is invalid. That is no longer the case after r326604.

Added:
clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-const-cxx17.cpp

Added: clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-const-cxx17.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-const-cxx17.cpp?rev=326605=auto
==
--- clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-const-cxx17.cpp 
(added)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-const-cxx17.cpp Fri 
Mar  2 11:14:34 2018
@@ -0,0 +1,15 @@
+// RUN: %check_clang_tidy %s misc-misplaced-const %t -- -- -std=c++17
+
+// This test previously would cause a failed assertion because the structured
+// binding declaration had no valid type associated with it. This ensures the
+// expected clang diagnostic is generated instead.
+// CHECK-MESSAGES: :[[@LINE+1]]:6: error: decomposition declaration '[x]' 
requires an initializer [clang-diagnostic-error]
+auto [x];
+
+struct S { int a; };
+S f();
+
+int main() {
+  auto [x] = f();
+}
+


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

r326604 - Fix the hasType() AST matcher to not assert when the QualType is invalid.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Fri Mar  2 11:14:21 2018
New Revision: 326604

URL: http://llvm.org/viewvc/llvm-project?rev=326604=rev
Log:
Fix the hasType() AST matcher to not assert when the QualType is invalid.

There's not a particularly good way to test this with the AST matchers unit 
tests because the only way to get an invalid type (that I can devise) involves 
creating parse errors, which the test harness always treats as a failure. 
Instead, a clang-tidy test case will be added in a follow-up commit based on 
the original bug report.

Modified:
cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=326604=326603=326604=diff
==
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Fri Mar  2 11:14:21 2018
@@ -2843,8 +2843,10 @@ AST_MATCHER_P_OVERLOAD(CallExpr, callee,
 AST_POLYMORPHIC_MATCHER_P_OVERLOAD(
 hasType, AST_POLYMORPHIC_SUPPORTED_TYPES(Expr, TypedefNameDecl, ValueDecl),
 internal::Matcher, InnerMatcher, 0) {
-  return InnerMatcher.matches(internal::getUnderlyingType(Node),
-  Finder, Builder);
+  QualType QT = internal::getUnderlyingType(Node);
+  if (!QT.isNull())
+return InnerMatcher.matches(QT, Finder, Builder);
+  return false;
 }
 
 /// \brief Overloaded to match the declaration of the expression's or value


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

[PATCH] D43995: Do not generate calls to fentry with __attribute__((no_instrument_function))

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

This looks right to me, but we should check with @hans since I think he touched 
this code last.


Repository:
  rC Clang

https://reviews.llvm.org/D43995



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

r326603 - [Attr] Use -fsyntax-only in test

[Joel E. Denny via cfe-commits]

Author: jdenny
Date: Fri Mar  2 11:03:27 2018
New Revision: 326603

URL: http://llvm.org/viewvc/llvm-project?rev=326603=rev
Log:
[Attr] Use -fsyntax-only in test

Suggested at: https://reviews.llvm.org/D43248

Modified:
cfe/trunk/test/Sema/attr-ownership.c

Modified: cfe/trunk/test/Sema/attr-ownership.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/attr-ownership.c?rev=326603=326602=326603=diff
==
--- cfe/trunk/test/Sema/attr-ownership.c (original)
+++ cfe/trunk/test/Sema/attr-ownership.c Fri Mar  2 11:03:27 2018
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -verify
+// RUN: %clang_cc1 %s -verify -fsyntax-only
 
 void f1(void) __attribute__((ownership_takes("foo"))); // expected-error 
{{'ownership_takes' attribute requires parameter 1 to be an identifier}}
 void *f2(void) __attribute__((ownership_returns(foo, 1, 2)));  // 
expected-error {{'ownership_returns' attribute takes no more than 1 argument}}


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

r326602 - [Attr] Fix parameter indexing for several attributes

[Joel E. Denny via cfe-commits]

Author: jdenny
Date: Fri Mar  2 11:03:22 2018
New Revision: 326602

URL: http://llvm.org/viewvc/llvm-project?rev=326602=rev
Log:
[Attr] Fix parameter indexing for several attributes

The patch fixes a number of bugs related to parameter indexing in
attributes:

* Parameter indices in some attributes (argument_with_type_tag,
  pointer_with_type_tag, nonnull, ownership_takes, ownership_holds,
  and ownership_returns) are specified in source as one-origin
  including any C++ implicit this parameter, were stored as
  zero-origin excluding any this parameter, and were erroneously
  printing (-ast-print) and confusingly dumping (-ast-dump) as the
  stored values.

* For alloc_size, the C++ implicit this parameter was not subtracted
  correctly in Sema, leading to assert failures or to silent failures
  of __builtin_object_size to compute a value.

* For argument_with_type_tag, pointer_with_type_tag, and
  ownership_returns, the C++ implicit this parameter was not added
  back to parameter indices in some diagnostics.

This patch fixes the above bugs and aims to prevent similar bugs in
the future by introducing careful mechanisms for handling parameter
indices in attributes.  ParamIdx stores a parameter index and is
designed to hide the stored encoding while providing accessors that
require each use (such as printing) to make explicit the encoding that
is needed.  Attribute declarations declare parameter index arguments
as [Variadic]ParamIdxArgument, which are exposed as ParamIdx[*].  This
patch rewrites all attribute arguments that are processed by
checkFunctionOrMethodParameterIndex in SemaDeclAttr.cpp to be declared
as [Variadic]ParamIdxArgument.  The only exception is xray_log_args's
argument, which is encoded as a count not an index.

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

Added:
cfe/trunk/test/Sema/attr-ownership.cpp
Modified:
cfe/trunk/include/clang/AST/Attr.h
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
cfe/trunk/test/CodeGenCXX/alloc-size.cpp
cfe/trunk/test/Misc/ast-dump-attr.cpp
cfe/trunk/test/Sema/attr-print.cpp
cfe/trunk/test/Sema/error-type-safety.cpp
cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp

Modified: cfe/trunk/include/clang/AST/Attr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Attr.h?rev=326602=326601=326602=diff
==
--- cfe/trunk/include/clang/AST/Attr.h (original)
+++ cfe/trunk/include/clang/AST/Attr.h Fri Mar  2 11:03:22 2018
@@ -195,6 +195,120 @@ public:
}
 };
 
+/// A single parameter index whose accessors require each use to make explicit
+/// the parameter index encoding needed.
+class ParamIdx {
+  // Idx is exposed only via accessors that specify specific encodings.
+  unsigned Idx : 30;
+  unsigned HasThis : 1;
+  unsigned IsValid : 1;
+
+  void assertComparable(const ParamIdx ) const {
+assert(isValid() && I.isValid() &&
+   "ParamIdx must be valid to be compared");
+// It's possible to compare indices from separate functions, but so far
+// it's not proven useful.  Moreover, it might be confusing because a
+// comparison on the results of getASTIndex might be inconsistent with a
+// comparison on the ParamIdx objects themselves.
+assert(HasThis == I.HasThis &&
+   "ParamIdx must be for the same function to be compared");
+  }
+
+public:
+  /// Construct an invalid parameter index (\c isValid returns false and
+  /// accessors fail an assert).
+  ParamIdx() : Idx(0), HasThis(false), IsValid(false) {}
+
+  /// \param Idx is the parameter index as it is normally specified in
+  /// attributes in the source: one-origin including any C++ implicit this
+  /// parameter.
+  ///
+  /// \param D is the declaration containing the parameters.  It is used to
+  /// determine if there is a C++ implicit this parameter.
+  ParamIdx(unsigned Idx, const Decl *D)
+  : Idx(Idx), HasThis(false), IsValid(true) {
+if (const auto *FD = dyn_cast(D))
+  HasThis = FD->isCXXInstanceMember();
+  }
+
+  /// \param Idx is the parameter index as it is normally specified in
+  /// attributes in the source: one-origin including any C++ implicit this
+  /// parameter.
+  ///
+  /// \param HasThis specifies whether the function has a C++ implicit this
+  /// parameter.
+  ParamIdx(unsigned Idx, bool HasThis)
+  : Idx(Idx), HasThis(HasThis), IsValid(true) {}
+
+  /// Is this parameter index valid?
+  bool isValid() const { return IsValid; }
+
+  /// Is there a C++ implicit this parameter?
+  bool hasThis() const {
+assert(isValid() && "ParamIdx 

[PATCH] D43248: [Attr] Fix parameter indexing for attributes

[Joel E. Denny via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL326602: [Attr] Fix parameter indexing for several attributes 
(authored by jdenny, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D43248?vs=136624=136811#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D43248

Files:
  cfe/trunk/include/clang/AST/Attr.h
  cfe/trunk/include/clang/Basic/Attr.td
  cfe/trunk/lib/AST/ExprConstant.cpp
  cfe/trunk/lib/CodeGen/CGCall.cpp
  cfe/trunk/lib/Sema/SemaChecking.cpp
  cfe/trunk/lib/Sema/SemaDecl.cpp
  cfe/trunk/lib/Sema/SemaDeclAttr.cpp
  cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
  cfe/trunk/test/CodeGenCXX/alloc-size.cpp
  cfe/trunk/test/Misc/ast-dump-attr.cpp
  cfe/trunk/test/Sema/attr-ownership.cpp
  cfe/trunk/test/Sema/attr-print.cpp
  cfe/trunk/test/Sema/error-type-safety.cpp
  cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp

Index: cfe/trunk/include/clang/AST/Attr.h
===
--- cfe/trunk/include/clang/AST/Attr.h
+++ cfe/trunk/include/clang/AST/Attr.h
@@ -195,6 +195,120 @@
}
 };
 
+/// A single parameter index whose accessors require each use to make explicit
+/// the parameter index encoding needed.
+class ParamIdx {
+  // Idx is exposed only via accessors that specify specific encodings.
+  unsigned Idx : 30;
+  unsigned HasThis : 1;
+  unsigned IsValid : 1;
+
+  void assertComparable(const ParamIdx ) const {
+assert(isValid() && I.isValid() &&
+   "ParamIdx must be valid to be compared");
+// It's possible to compare indices from separate functions, but so far
+// it's not proven useful.  Moreover, it might be confusing because a
+// comparison on the results of getASTIndex might be inconsistent with a
+// comparison on the ParamIdx objects themselves.
+assert(HasThis == I.HasThis &&
+   "ParamIdx must be for the same function to be compared");
+  }
+
+public:
+  /// Construct an invalid parameter index (\c isValid returns false and
+  /// accessors fail an assert).
+  ParamIdx() : Idx(0), HasThis(false), IsValid(false) {}
+
+  /// \param Idx is the parameter index as it is normally specified in
+  /// attributes in the source: one-origin including any C++ implicit this
+  /// parameter.
+  ///
+  /// \param D is the declaration containing the parameters.  It is used to
+  /// determine if there is a C++ implicit this parameter.
+  ParamIdx(unsigned Idx, const Decl *D)
+  : Idx(Idx), HasThis(false), IsValid(true) {
+if (const auto *FD = dyn_cast(D))
+  HasThis = FD->isCXXInstanceMember();
+  }
+
+  /// \param Idx is the parameter index as it is normally specified in
+  /// attributes in the source: one-origin including any C++ implicit this
+  /// parameter.
+  ///
+  /// \param HasThis specifies whether the function has a C++ implicit this
+  /// parameter.
+  ParamIdx(unsigned Idx, bool HasThis)
+  : Idx(Idx), HasThis(HasThis), IsValid(true) {}
+
+  /// Is this parameter index valid?
+  bool isValid() const { return IsValid; }
+
+  /// Is there a C++ implicit this parameter?
+  bool hasThis() const {
+assert(isValid() && "ParamIdx must be valid");
+return HasThis;
+  }
+
+  /// Get the parameter index as it would normally be encoded for attributes at
+  /// the source level of representation: one-origin including any C++ implicit
+  /// this parameter.
+  ///
+  /// This encoding thus makes sense for diagnostics, pretty printing, and
+  /// constructing new attributes from a source-like specification.
+  unsigned getSourceIndex() const {
+assert(isValid() && "ParamIdx must be valid");
+return Idx;
+  }
+
+  /// Get the parameter index as it would normally be encoded at the AST level
+  /// of representation: zero-origin not including any C++ implicit this
+  /// parameter.
+  ///
+  /// This is the encoding primarily used in Sema.  However, in diagnostics,
+  /// Sema uses \c getSourceIndex instead.
+  unsigned getASTIndex() const {
+assert(isValid() && "ParamIdx must be valid");
+assert(Idx >= 1 + HasThis &&
+   "stored index must be base-1 and not specify C++ implicit this");
+return Idx - 1 - HasThis;
+  }
+
+  /// Get the parameter index as it would normally be encoded at the LLVM level
+  /// of representation: zero-origin including any C++ implicit this parameter.
+  ///
+  /// This is the encoding primarily used in CodeGen.
+  unsigned getLLVMIndex() const {
+assert(isValid() && "ParamIdx must be valid");
+assert(Idx >= 1 && "stored index must be base-1");
+return Idx - 1;
+  }
+
+  bool operator==(const ParamIdx ) const {
+assertComparable(I);
+return Idx == I.Idx;
+  }
+  bool operator!=(const ParamIdx ) const {
+assertComparable(I);
+return Idx != I.Idx;
+  }
+  bool operator<(const 

[PATCH] D42938: [Sema] Emit -Winteger-overflow for arguments in function calls, ObjC messages.

[Jan Korous via Phabricator via cfe-commits]

jkorous-apple added a comment.

Sorry, my bad, I tried just older clang version which didn't produce any error.

With reasonably up to date clang I get these:

  > cat tmpl_overflow.hpp



  template struct foo { short a; };
  
  template<> struct foo<100 * 10240> { bool a; };
  
  int main() {
  foo<100 * 10240> a;
  }



  > ~/src/oss/llvm@master/build/bin/clang++ tmpl_overflow.hpp
  tmpl_overflow.hpp:3:23: error: non-type template argument is not a constant 
expression
  template<> struct foo<100 * 10240> { bool a; };
^~~
  tmpl_overflow.hpp:3:43: note: value 1024000 is outside the 
range of representable values of type 'long'
  template<> struct foo<100 * 10240> { bool a; };
^
  tmpl_overflow.hpp:6:9: error: non-type template argument is not a constant 
expression
  foo<100 * 10240> a;
  ^~~
  tmpl_overflow.hpp:6:29: note: value 1024000 is outside the 
range of representable values of type 'long'
  foo<100 * 10240> a;
  ^
  2 errors generated.

I am not actually sure those error messages are correct (not constant? type 
'long'?) but I am not sure it's still within the scope of your patch either.

What is interesting is that by using lower value (which is still to big for 
short) I probably hit different diagnostic check and get different error:

  > cat tmpl_overflow.hpp

  template struct foo { short a; };
  
  template<> struct foo<100 * 10240> { bool a; };
  
  int main() {
  foo<100 * 10240> a;
  }






  
jankorous @ jans-imac /Users/jankorous/tmp
  > ~/src/oss/llvm@master/build/bin/clang++ tmpl_overflow.hpp
  tmpl_overflow.hpp:3:23: error: non-type template argument evaluates to 
1024000, which cannot be narrowed to type 'int' [-Wc++11-narrowing]
  template<> struct foo<100 * 10240> { bool a; };
^
  tmpl_overflow.hpp:6:9: error: non-type template argument evaluates to 
1024000, which cannot be narrowed to type 'int' [-Wc++11-narrowing]
  foo<100 * 10240> a;
  ^
  2 errors generated.


https://reviews.llvm.org/D42938



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

[PATCH] D41102: Setup clang-doc frontend framework

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-doc/Representation.h:117
+  bool IsDefinition = false;
+  llvm::Optional DefLoc;
+  llvm::SmallVector Loc;

lebedev.ri wrote:
> I meant that `IsDefinition` controls whether `DefLoc` will be set/used or not.
> So with `llvm::Optional DefLoc`, you don't need the `bool 
> IsDefinition`.
That...makes so much sense. Oops. Thank you!


https://reviews.llvm.org/D41102



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

[PATCH] D41102: Setup clang-doc frontend framework

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 136809.
juliehockett marked an inline comment as done.
juliehockett added a comment.

Removing IsDefinition field.


https://reviews.llvm.org/D41102

Files:
  CMakeLists.txt
  clang-doc/BitcodeWriter.cpp
  clang-doc/BitcodeWriter.h
  clang-doc/CMakeLists.txt
  clang-doc/ClangDoc.h
  clang-doc/Mapper.cpp
  clang-doc/Mapper.h
  clang-doc/Representation.h
  clang-doc/Serialize.cpp
  clang-doc/Serialize.h
  clang-doc/tool/CMakeLists.txt
  clang-doc/tool/ClangDocMain.cpp
  docs/clang-doc.rst
  test/CMakeLists.txt
  test/clang-doc/mapper-class-in-class.cpp
  test/clang-doc/mapper-class-in-function.cpp
  test/clang-doc/mapper-class.cpp
  test/clang-doc/mapper-comments.cpp
  test/clang-doc/mapper-enum.cpp
  test/clang-doc/mapper-function.cpp
  test/clang-doc/mapper-method.cpp
  test/clang-doc/mapper-namespace.cpp
  test/clang-doc/mapper-struct.cpp
  test/clang-doc/mapper-union.cpp

Index: test/clang-doc/mapper-union.cpp
===
--- /dev/null
+++ test/clang-doc/mapper-union.cpp
@@ -0,0 +1,27 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "" > %t/compile_flags.txt
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-doc --dump --omit-filenames -doxygen -p %t %t/test.cpp -output=%t/docs
+// RUN: llvm-bcanalyzer %t/docs/c:@u...@d.bc --dump | FileCheck %s
+
+union D { int X; int Y; };
+// CHECK: 
+// CHECK: 
+  // CHECK: 
+// CHECK: 
+// CHECK: 
+  // CHECK:  blob data = 'c:@U@D'
+  // CHECK:  blob data = 'D'
+  // CHECK: 
+  // CHECK: 
+// CHECK:  blob data = 'int'
+// CHECK:  blob data = 'D::X'
+// CHECK: 
+  // CHECK: 
+  // CHECK: 
+// CHECK:  blob data = 'int'
+// CHECK:  blob data = 'D::Y'
+// CHECK: 
+  // CHECK: 
+// CHECK: 
Index: test/clang-doc/mapper-struct.cpp
===
--- /dev/null
+++ test/clang-doc/mapper-struct.cpp
@@ -0,0 +1,21 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "" > %t/compile_flags.txt
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-doc --dump --omit-filenames -doxygen -p %t %t/test.cpp -output=%t/docs
+// RUN: llvm-bcanalyzer %t/docs/c:@s...@c.bc --dump | FileCheck %s
+
+struct C { int i; };
+// CHECK: 
+// CHECK: 
+  // CHECK: 
+// CHECK: 
+// CHECK: 
+  // CHECK:  blob data = 'c:@S@C'
+  // CHECK:  blob data = 'C'
+  // CHECK: 
+// CHECK:  blob data = 'int'
+// CHECK:  blob data = 'C::i'
+// CHECK: 
+  // CHECK: 
+// CHECK: 
Index: test/clang-doc/mapper-namespace.cpp
===
--- /dev/null
+++ test/clang-doc/mapper-namespace.cpp
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "" > %t/compile_flags.txt
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-doc --dump --omit-filenames -doxygen -p %t %t/test.cpp -output=%t/docs
+// RUN: llvm-bcanalyzer %t/docs/c:@n...@a.bc --dump | FileCheck %s
+
+namespace A {}
+// CHECK: 
+// CHECK: 
+  // CHECK: 
+// CHECK: 
+// CHECK: 
+  // CHECK:  blob data = 'c:@N@A'
+  // CHECK:  blob data = 'A'
+// CHECK: 
Index: test/clang-doc/mapper-method.cpp
===
--- /dev/null
+++ test/clang-doc/mapper-method.cpp
@@ -0,0 +1,42 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "" > %t/compile_flags.txt
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-doc --dump --omit-filenames -doxygen -p %t %t/test.cpp -output=%t/docs
+// RUN: llvm-bcanalyzer %t/docs/c:@S@G@F@Method#I#.bc --dump | FileCheck %s --check-prefix CHECK-G-F
+// RUN: llvm-bcanalyzer %t/docs/c:@s...@g.bc --dump | FileCheck %s --check-prefix CHECK-G
+
+class G {
+public: 
+	int Method(int param) { return param; }
+};
+
+// CHECK-G: 
+// CHECK-G: 
+  // CHECK-G: 
+// CHECK-G: 
+// CHECK-G: 
+  // CHECK-G:  blob data = 'c:@S@G'
+  // CHECK-G:  blob data = 'G'
+  // CHECK-G: 
+// CHECK-G: 
+
+
+// CHECK-G-F: 
+// CHECK-G-F: 
+  // CHECK-G-F: 
+// CHECK-G-F: 
+// CHECK-G-F: 
+  // CHECK-G-F:  blob data = 'c:@S@G@F@Method#I#'
+  // CHECK-G-F:  blob data = 'Method'
+  // CHECK-G-F:  blob data = 'c:@S@G'
+  // CHECK-G-F: 
+  // CHECK-G-F:  blob data = 'c:@S@G'
+  // CHECK-G-F: 
+// CHECK-G-F:  blob data = 'int'
+  // CHECK-G-F: 
+  // CHECK-G-F: 
+// CHECK-G-F:  blob data = 'int'
+// CHECK-G-F:  blob data = 'param'
+  // CHECK-G-F: 
+// CHECK-G-F: 
Index: test/clang-doc/mapper-function.cpp
===
--- /dev/null
+++ test/clang-doc/mapper-function.cpp
@@ -0,0 +1,23 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "" > %t/compile_flags.txt
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-doc --dump --omit-filenames -doxygen -p %t %t/test.cpp -output=%t/docs
+// RUN: llvm-bcanalyzer %t/docs/c:@F@F#I#.bc --dump | FileCheck %s
+
+int F(int param) { return param; }
+// CHECK: 
+// CHECK: 
+  // CHECK: 
+// CHECK: 
+// CHECK: 
+  // CHECK:  blob data = 'c:@F@F#I#'
+  // CHECK:  blob data = 'F'
+  // CHECK: 
+   

[PATCH] D43818: Better OpenBSD frontend support

[David CARLIER via Phabricator via cfe-commits]

devnexen updated this revision to Diff 136804.
devnexen added a comment.

backtrace on OpenBSD is not base library but a package. Plus not needed for the 
UBsan subset.


https://reviews.llvm.org/D43818

Files:
  lib/Driver/ToolChain.cpp
  lib/Driver/ToolChains/CommonArgs.cpp
  lib/Driver/ToolChains/Gnu.cpp


Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -242,11 +242,13 @@
 ArgStringList ) {
   CmdArgs.push_back("--no-as-needed");
   CmdArgs.push_back("-lpthread");
-  CmdArgs.push_back("-lrt");
+  if (TC.getTriple().getOS() != llvm::Triple::OpenBSD)
+CmdArgs.push_back("-lrt");
   CmdArgs.push_back("-lm");
 
   if (TC.getTriple().getOS() != llvm::Triple::FreeBSD &&
-  TC.getTriple().getOS() != llvm::Triple::NetBSD)
+  TC.getTriple().getOS() != llvm::Triple::NetBSD &&
+  TC.getTriple().getOS() != llvm::Triple::OpenBSD)
 CmdArgs.push_back("-ldl");
 }
 
Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -542,12 +542,14 @@
   // There's no libpthread or librt on RTEMS.
   if (TC.getTriple().getOS() != llvm::Triple::RTEMS) {
 CmdArgs.push_back("-lpthread");
-CmdArgs.push_back("-lrt");
+if (TC.getTriple().getOS() != llvm::Triple::OpenBSD)
+  CmdArgs.push_back("-lrt");
   }
   CmdArgs.push_back("-lm");
   // There's no libdl on all OSes.
   if (TC.getTriple().getOS() != llvm::Triple::FreeBSD &&
   TC.getTriple().getOS() != llvm::Triple::NetBSD &&
+  TC.getTriple().getOS() != llvm::Triple::OpenBSD &&
   TC.getTriple().getOS() != llvm::Triple::RTEMS)
 CmdArgs.push_back("-ldl");
   // Required for backtrace on some OSes
Index: lib/Driver/ToolChain.cpp
===
--- lib/Driver/ToolChain.cpp
+++ lib/Driver/ToolChain.cpp
@@ -329,6 +329,8 @@
 return "freebsd";
   case llvm::Triple::NetBSD:
 return "netbsd";
+  case llvm::Triple::OpenBSD:
+return "openbsd";
   case llvm::Triple::Solaris:
 return "sunos";
   default:


Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -242,11 +242,13 @@
 ArgStringList ) {
   CmdArgs.push_back("--no-as-needed");
   CmdArgs.push_back("-lpthread");
-  CmdArgs.push_back("-lrt");
+  if (TC.getTriple().getOS() != llvm::Triple::OpenBSD)
+CmdArgs.push_back("-lrt");
   CmdArgs.push_back("-lm");
 
   if (TC.getTriple().getOS() != llvm::Triple::FreeBSD &&
-  TC.getTriple().getOS() != llvm::Triple::NetBSD)
+  TC.getTriple().getOS() != llvm::Triple::NetBSD &&
+  TC.getTriple().getOS() != llvm::Triple::OpenBSD)
 CmdArgs.push_back("-ldl");
 }
 
Index: lib/Driver/ToolChains/CommonArgs.cpp
===
--- lib/Driver/ToolChains/CommonArgs.cpp
+++ lib/Driver/ToolChains/CommonArgs.cpp
@@ -542,12 +542,14 @@
   // There's no libpthread or librt on RTEMS.
   if (TC.getTriple().getOS() != llvm::Triple::RTEMS) {
 CmdArgs.push_back("-lpthread");
-CmdArgs.push_back("-lrt");
+if (TC.getTriple().getOS() != llvm::Triple::OpenBSD)
+  CmdArgs.push_back("-lrt");
   }
   CmdArgs.push_back("-lm");
   // There's no libdl on all OSes.
   if (TC.getTriple().getOS() != llvm::Triple::FreeBSD &&
   TC.getTriple().getOS() != llvm::Triple::NetBSD &&
+  TC.getTriple().getOS() != llvm::Triple::OpenBSD &&
   TC.getTriple().getOS() != llvm::Triple::RTEMS)
 CmdArgs.push_back("-ldl");
   // Required for backtrace on some OSes
Index: lib/Driver/ToolChain.cpp
===
--- lib/Driver/ToolChain.cpp
+++ lib/Driver/ToolChain.cpp
@@ -329,6 +329,8 @@
 return "freebsd";
   case llvm::Triple::NetBSD:
 return "netbsd";
+  case llvm::Triple::OpenBSD:
+return "openbsd";
   case llvm::Triple::Solaris:
 return "sunos";
   default:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44032: Remove -i command line option, add -imultilib

[Erich Keane via Phabricator via cfe-commits]

erichkeane created this revision.
erichkeane added reviewers: rsmith, chandlerc, aaron.ballman, echristo.

I discovered that '-i' is a command line option for the driver, 
however it actually does not do anything and is not supported by any
other compiler.  In fact, it is completely undocumented for Clang.

I found a couple of instances of people confusing it with one of 
the variety of other command line options that control the driver.
Because of this, we should delete this option so that it is clear
that it isn't valid.

HOWEVER, I found that GCC DOES support -imultilib, which the -i
was hiding our lack of support for.  We currently only use imultilib
for the purpose of forwarding to gfortran (in a specific test written
by chandlerc for this purpose).

imultilib is a rarely used (if ever?) feature that I could find no
references to on the internet, and in fact, my company's massive test
suite has zero references to it ever being used.

SO, this patch removes the -i option so that we will now give an error
on its usage (so that it won't be confused with -I), and replaces it with
-imultilib, which is now specified as a gfortran_group option.


Repository:
  rC Clang

https://reviews.llvm.org/D44032

Files:
  include/clang/Driver/Options.td


Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1764,7 +1764,7 @@
   Flags<[CC1Option]>;
 def ivfsoverlay : JoinedOrSeparate<["-"], "ivfsoverlay">, 
Group, Flags<[CC1Option]>,
   HelpText<"Overlay the virtual filesystem described by file over the real 
file system">;
-def i : Joined<["-"], "i">, Group;
+def imultilib : Separate<["-"], "imultilib">, Group;
 def keep__private__externs : Flag<["-"], "keep_private_externs">;
 def l : JoinedOrSeparate<["-"], "l">, Flags<[LinkerInput, RenderJoined]>,
 Group;


Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1764,7 +1764,7 @@
   Flags<[CC1Option]>;
 def ivfsoverlay : JoinedOrSeparate<["-"], "ivfsoverlay">, Group, Flags<[CC1Option]>,
   HelpText<"Overlay the virtual filesystem described by file over the real file system">;
-def i : Joined<["-"], "i">, Group;
+def imultilib : Separate<["-"], "imultilib">, Group;
 def keep__private__externs : Flag<["-"], "keep_private_externs">;
 def l : JoinedOrSeparate<["-"], "l">, Flags<[LinkerInput, RenderJoined]>,
 Group;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41102: Setup clang-doc frontend framework

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang-doc/Representation.h:117
+  bool IsDefinition = false;
+  llvm::Optional DefLoc;
+  llvm::SmallVector Loc;

I meant that `IsDefinition` controls whether `DefLoc` will be set/used or not.
So with `llvm::Optional DefLoc`, you don't need the `bool 
IsDefinition`.


https://reviews.llvm.org/D41102



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

[PATCH] D43341: [clang-doc] Implement reducer portion of the frontend framework

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 136793.
juliehockett added a comment.

Updating for parent diff & refactoring reader.


https://reviews.llvm.org/D43341

Files:
  clang-doc/BitcodeReader.cpp
  clang-doc/BitcodeReader.h
  clang-doc/BitcodeWriter.cpp
  clang-doc/BitcodeWriter.h
  clang-doc/CMakeLists.txt
  clang-doc/Reducer.cpp
  clang-doc/Reducer.h
  clang-doc/Representation.cpp
  clang-doc/Representation.h
  clang-doc/tool/ClangDocMain.cpp
  test/clang-doc/comment-bc.cpp
  test/clang-doc/mapper-class-in-class.cpp
  test/clang-doc/mapper-class-in-function.cpp
  test/clang-doc/mapper-class.cpp
  test/clang-doc/mapper-comments.cpp
  test/clang-doc/mapper-enum.cpp
  test/clang-doc/mapper-function.cpp
  test/clang-doc/mapper-method.cpp
  test/clang-doc/mapper-namespace.cpp
  test/clang-doc/mapper-struct.cpp
  test/clang-doc/mapper-union.cpp
  test/clang-doc/namespace-bc.cpp
  test/clang-doc/record-bc.cpp

Index: test/clang-doc/record-bc.cpp
===
--- /dev/null
+++ test/clang-doc/record-bc.cpp
@@ -0,0 +1,133 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "" > %t/compile_flags.txt
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-doc --dump --omit-filenames -doxygen -p %t %t/test.cpp -output=%t/docs
+// RUN: llvm-bcanalyzer %t/docs/docs.bc --dump | FileCheck %s
+
+union A { int X; int Y; };
+
+enum B { X, Y };
+
+struct C { int i; };
+
+class D {};
+
+class E {
+public:
+  E() {}
+  ~E() {}
+
+protected:
+  void ProtectedMethod();
+};
+
+void E::ProtectedMethod() {}
+
+class F : virtual private D, public E {};
+
+// CHECK: 
+// CHECK: 
+  // CHECK: 
+// CHECK: 
+// CHECK: 
+  // CHECK:  blob data = 'c:@S@E@F@E#'
+  // CHECK:  blob data = 'E'
+  // CHECK:  blob data = 'c:@S@E'
+  // CHECK: 
+  // CHECK: 
+  // CHECK:  blob data = 'c:@S@E'
+  // CHECK: 
+// CHECK:  blob data = 'void'
+  // CHECK: 
+// CHECK: 
+// CHECK: 
+  // CHECK:  blob data = 'c:@S@E@F@~E#'
+  // CHECK:  blob data = '~E'
+  // CHECK:  blob data = 'c:@S@E'
+  // CHECK: 
+  // CHECK: 
+  // CHECK:  blob data = 'c:@S@E'
+  // CHECK: 
+// CHECK:  blob data = 'void'
+  // CHECK: 
+// CHECK: 
+// CHECK: 
+  // CHECK:  blob data = 'c:@S@E@F@ProtectedMethod#'
+  // CHECK:  blob data = 'ProtectedMethod'
+  // CHECK:  blob data = 'c:@S@E'
+  // CHECK: 
+  // CHECK:  blob data = 'c:@S@E'
+  // CHECK: 
+// CHECK:  blob data = 'void'
+  // CHECK: 
+// CHECK: 
+// CHECK: 
+  // CHECK:  blob data = 'c:@S@E@F@ProtectedMethod#'
+  // CHECK:  blob data = 'ProtectedMethod'
+  // CHECK:  blob data = 'c:@S@E'
+  // CHECK: 
+  // CHECK: 
+  // CHECK:  blob data = 'c:@S@E'
+  // CHECK: 
+// CHECK:  blob data = 'void'
+  // CHECK: 
+// CHECK: 
+// CHECK: 
+  // CHECK:  blob data = 'c:@U@A'
+  // CHECK:  blob data = 'A'
+  // CHECK: 
+  // CHECK: 
+  // CHECK: 
+// CHECK:  blob data = 'int'
+// CHECK:  blob data = 'A::X'
+// CHECK: 
+  // CHECK: 
+  // CHECK: 
+// CHECK:  blob data = 'int'
+// CHECK:  blob data = 'A::Y'
+// CHECK: 
+  // CHECK: 
+// CHECK: 
+// CHECK: 
+  // CHECK:  blob data = 'c:@S@C'
+  // CHECK:  blob data = 'C'
+  // CHECK: 
+  // CHECK: 
+  // CHECK: 
+// CHECK:  blob data = 'int'
+// CHECK:  blob data = 'C::i'
+// CHECK: 
+  // CHECK: 
+// CHECK: 
+// CHECK: 
+  // CHECK:  blob data = 'c:@S@D'
+  // CHECK:  blob data = 'D'
+  // CHECK: 
+  // CHECK: 
+// CHECK: 
+// CHECK: 
+  // CHECK:  blob data = 'c:@S@E'
+  // CHECK:  blob data = 'E'
+  // CHECK: 
+  // CHECK: 
+// CHECK: 
+// CHECK: 
+  // CHECK:  blob data = 'c:@S@F'
+  // CHECK:  blob data = 'F'
+  // CHECK: 
+  // CHECK: 
+  // CHECK:  blob data = 'c:@S@E'
+  // CHECK:  blob data = 'c:@S@D'
+// CHECK: 
+// CHECK: 
+  // CHECK:  blob data = 'c:@E@B'
+  // CHECK:  blob data = 'B'
+  // CHECK: 
+  // CHECK: 
+// CHECK:  blob data = 'X'
+  // CHECK: 
+  // CHECK: 
+// CHECK:  blob data = 'Y'
+  // CHECK: 
+// CHECK: 
Index: test/clang-doc/namespace-bc.cpp
===
--- /dev/null
+++ test/clang-doc/namespace-bc.cpp
@@ -0,0 +1,74 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "" > %t/compile_flags.txt
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-doc --dump --omit-filenames -doxygen -p %t %t/test.cpp -output=%t/docs
+// RUN: llvm-bcanalyzer %t/docs/docs.bc --dump | FileCheck %s
+
+namespace A {
+
+void f();
+void f() {};
+
+} // A
+
+namespace A {
+namespace B {
+
+enum E { X };
+
+E func(int i) { 
+  return X;
+}
+
+}
+}
+
+// CHECK: 
+// CHECK: 
+  // CHECK: 
+// CHECK: 
+// CHECK: 
+  // CHECK:  blob data = 'c:@N@A'
+  // CHECK:  blob data = 'A'
+// CHECK: 
+// CHECK: 
+  // CHECK:  blob data = 'c:@N@A@N@B'
+  // CHECK:  blob data = 'B'
+  // CHECK:  blob data = 'c:@N@A'
+// CHECK: 
+// CHECK: 
+  // CHECK:  blob data = 'c:@N@A@F@f#'
+  // CHECK:  blob data = 'f'
+  // CHECK:  blob data = 'c:@N@A'
+  // CHECK: 
+  // CHECK: 
+// CHECK:  blob data = 'void'
+  // CHECK: 
+// CHECK: 
+// 

Re: [clang-tools-extra] r326546 - [clangd] Debounce streams of updates.

[Ilya Biryukov via cfe-commits]

The TUSchedulerTest::Debounce breaks on Windows buildbots. Probably because
the timeouts of 50ms/10ms/40ms are too low to be scheduled properly.
Increased the timeouts in r326598 to 1s/200ms/2s, hopefully that would
unbreak the buildbots.

We could tweak them back to lower values that work on Monday :-)


On Fri, Mar 2, 2018 at 9:58 AM Sam McCall via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: sammccall
> Date: Fri Mar  2 00:56:37 2018
> New Revision: 326546
>
> URL: http://llvm.org/viewvc/llvm-project?rev=326546=rev
> Log:
> [clangd] Debounce streams of updates.
>
> Summary:
> Don't actually start building ASTs for new revisions until either:
> - 500ms have passed since the last revision, or
> - we actually need the revision for something (or to unblock the queue)
>
> In practice, this avoids the "first keystroke results in diagnostics"
> problem.
> This is kind of awkward to test, and the test is pretty bad.
> It can be observed nicely by capturing a trace, though.
>
> Reviewers: hokein, ilya-biryukov
>
> Subscribers: klimek, jkorous-apple, ioeric, cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D43648
>
> Modified:
> clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
> clang-tools-extra/trunk/clangd/ClangdServer.cpp
> clang-tools-extra/trunk/clangd/ClangdServer.h
> clang-tools-extra/trunk/clangd/TUScheduler.cpp
> clang-tools-extra/trunk/clangd/TUScheduler.h
> clang-tools-extra/trunk/clangd/Threading.cpp
> clang-tools-extra/trunk/clangd/Threading.h
> clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp
>
> Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=326546=326545=326546=diff
>
> ==
> --- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
> +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Fri Mar  2 00:56:37
> 2018
> @@ -405,7 +405,7 @@ ClangdLSPServer::ClangdLSPServer(JSONOut
>  : Out(Out), CDB(std::move(CompileCommandsDir)), CCOpts(CCOpts),
>Server(CDB, /*DiagConsumer=*/*this, FSProvider, AsyncThreadsCount,
>   StorePreamblesInMemory, BuildDynamicSymbolIndex, StaticIdx,
> - ResourceDir) {}
> + ResourceDir,
> /*UpdateDebounce=*/std::chrono::milliseconds(500)) {}
>
>  bool ClangdLSPServer::run(std::istream , JSONStreamStyle InputStyle) {
>assert(!IsDone && "Run was called before");
>
> Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=326546=326545=326546=diff
>
> ==
> --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
> +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Fri Mar  2 00:56:37
> 2018
> @@ -76,7 +76,8 @@ ClangdServer::ClangdServer(GlobalCompila
> unsigned AsyncThreadsCount,
> bool StorePreamblesInMemory,
> bool BuildDynamicSymbolIndex, SymbolIndex
> *StaticIdx,
> -   llvm::Optional ResourceDir)
> +   llvm::Optional ResourceDir,
> +   std::chrono::steady_clock::duration
> UpdateDebounce)
>  : CompileArgs(CDB,
>ResourceDir ? ResourceDir->str() :
> getStandardResourceDir()),
>DiagConsumer(DiagConsumer), FSProvider(FSProvider),
> @@ -91,7 +92,8 @@ ClangdServer::ClangdServer(GlobalCompila
>  FileIdx
>  ? [this](PathRef Path,
>   ParsedAST *AST) { FileIdx->update(Path,
> AST); }
> -: ASTParsedCallback()) {
> +: ASTParsedCallback(),
> +UpdateDebounce) {
>if (FileIdx && StaticIdx) {
>  MergedIndex = mergeIndex(FileIdx.get(), StaticIdx);
>  Index = MergedIndex.get();
>
> Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
> URL:
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=326546=326545=326546=diff
>
> ==
> --- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
> +++ clang-tools-extra/trunk/clangd/ClangdServer.h Fri Mar  2 00:56:37 2018
> @@ -125,6 +125,8 @@ public:
>/// \p DiagConsumer. Note that a callback to \p DiagConsumer happens on
> a
>/// worker thread. Therefore, instances of \p DiagConsumer must properly
>/// synchronize access to shared state.
> +  /// UpdateDebounce determines how long to wait after a new version of
> the file
> +  /// before starting to compute diagnostics.
>///
>/// \p StorePreamblesInMemory defines whether the Preambles generated by
>/// clangd are stored 

[PATCH] D41102: Setup clang-doc frontend framework

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 136791.
juliehockett marked 11 inline comments as done.
juliehockett added a comment.

Addressing comments


https://reviews.llvm.org/D41102

Files:
  CMakeLists.txt
  clang-doc/BitcodeWriter.cpp
  clang-doc/BitcodeWriter.h
  clang-doc/CMakeLists.txt
  clang-doc/ClangDoc.h
  clang-doc/Mapper.cpp
  clang-doc/Mapper.h
  clang-doc/Representation.h
  clang-doc/Serialize.cpp
  clang-doc/Serialize.h
  clang-doc/tool/CMakeLists.txt
  clang-doc/tool/ClangDocMain.cpp
  docs/clang-doc.rst
  test/CMakeLists.txt
  test/clang-doc/mapper-class-in-class.cpp
  test/clang-doc/mapper-class-in-function.cpp
  test/clang-doc/mapper-class.cpp
  test/clang-doc/mapper-comments.cpp
  test/clang-doc/mapper-enum.cpp
  test/clang-doc/mapper-function.cpp
  test/clang-doc/mapper-method.cpp
  test/clang-doc/mapper-namespace.cpp
  test/clang-doc/mapper-struct.cpp
  test/clang-doc/mapper-union.cpp

Index: test/clang-doc/mapper-union.cpp
===
--- /dev/null
+++ test/clang-doc/mapper-union.cpp
@@ -0,0 +1,28 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "" > %t/compile_flags.txt
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-doc --dump --omit-filenames -doxygen -p %t %t/test.cpp -output=%t/docs
+// RUN: llvm-bcanalyzer %t/docs/c:@u...@d.bc --dump | FileCheck %s
+
+union D { int X; int Y; };
+// CHECK: 
+// CHECK: 
+  // CHECK: 
+// CHECK: 
+// CHECK: 
+  // CHECK:  blob data = 'c:@U@D'
+  // CHECK:  blob data = 'D'
+  // CHECK: 
+  // CHECK: 
+  // CHECK: 
+// CHECK:  blob data = 'int'
+// CHECK:  blob data = 'D::X'
+// CHECK: 
+  // CHECK: 
+  // CHECK: 
+// CHECK:  blob data = 'int'
+// CHECK:  blob data = 'D::Y'
+// CHECK: 
+  // CHECK: 
+// CHECK: 
Index: test/clang-doc/mapper-struct.cpp
===
--- /dev/null
+++ test/clang-doc/mapper-struct.cpp
@@ -0,0 +1,22 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "" > %t/compile_flags.txt
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-doc --dump --omit-filenames -doxygen -p %t %t/test.cpp -output=%t/docs
+// RUN: llvm-bcanalyzer %t/docs/c:@s...@c.bc --dump | FileCheck %s
+
+struct C { int i; };
+// CHECK: 
+// CHECK: 
+  // CHECK: 
+// CHECK: 
+// CHECK: 
+  // CHECK:  blob data = 'c:@S@C'
+  // CHECK:  blob data = 'C'
+  // CHECK: 
+  // CHECK: 
+// CHECK:  blob data = 'int'
+// CHECK:  blob data = 'C::i'
+// CHECK: 
+  // CHECK: 
+// CHECK: 
Index: test/clang-doc/mapper-namespace.cpp
===
--- /dev/null
+++ test/clang-doc/mapper-namespace.cpp
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "" > %t/compile_flags.txt
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-doc --dump --omit-filenames -doxygen -p %t %t/test.cpp -output=%t/docs
+// RUN: llvm-bcanalyzer %t/docs/c:@n...@a.bc --dump | FileCheck %s
+
+namespace A {}
+// CHECK: 
+// CHECK: 
+  // CHECK: 
+// CHECK: 
+// CHECK: 
+  // CHECK:  blob data = 'c:@N@A'
+  // CHECK:  blob data = 'A'
+// CHECK: 
Index: test/clang-doc/mapper-method.cpp
===
--- /dev/null
+++ test/clang-doc/mapper-method.cpp
@@ -0,0 +1,44 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "" > %t/compile_flags.txt
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-doc --dump --omit-filenames -doxygen -p %t %t/test.cpp -output=%t/docs
+// RUN: llvm-bcanalyzer %t/docs/c:@S@G@F@Method#I#.bc --dump | FileCheck %s --check-prefix CHECK-G-F
+// RUN: llvm-bcanalyzer %t/docs/c:@s...@g.bc --dump | FileCheck %s --check-prefix CHECK-G
+
+class G {
+public: 
+	int Method(int param) { return param; }
+};
+
+// CHECK-G: 
+// CHECK-G: 
+  // CHECK-G: 
+// CHECK-G: 
+// CHECK-G: 
+  // CHECK-G:  blob data = 'c:@S@G'
+  // CHECK-G:  blob data = 'G'
+  // CHECK-G: 
+  // CHECK-G: 
+// CHECK-G: 
+
+
+// CHECK-G-F: 
+// CHECK-G-F: 
+  // CHECK-G-F: 
+// CHECK-G-F: 
+// CHECK-G-F: 
+  // CHECK-G-F:  blob data = 'c:@S@G@F@Method#I#'
+  // CHECK-G-F:  blob data = 'Method'
+  // CHECK-G-F:  blob data = 'c:@S@G'
+  // CHECK-G-F: 
+  // CHECK-G-F: 
+  // CHECK-G-F:  blob data = 'c:@S@G'
+  // CHECK-G-F: 
+// CHECK-G-F:  blob data = 'int'
+  // CHECK-G-F: 
+  // CHECK-G-F: 
+// CHECK-G-F:  blob data = 'int'
+// CHECK-G-F:  blob data = 'param'
+  // CHECK-G-F: 
+// CHECK-G-F: 
Index: test/clang-doc/mapper-function.cpp
===
--- /dev/null
+++ test/clang-doc/mapper-function.cpp
@@ -0,0 +1,24 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "" > %t/compile_flags.txt
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-doc --dump --omit-filenames -doxygen -p %t %t/test.cpp -output=%t/docs
+// RUN: llvm-bcanalyzer %t/docs/c:@F@F#I#.bc --dump | FileCheck %s
+
+int F(int param) { return param; }
+// CHECK: 
+// CHECK: 
+  // CHECK: 
+// CHECK: 
+// CHECK: 
+  // CHECK:  blob data = 

[PATCH] D42938: [Sema] Emit -Winteger-overflow for arguments in function calls, ObjC messages.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

In https://reviews.llvm.org/D42938#1025151, @jkorous-apple wrote:

> Maybe a stupid idea but in case it makes sense to add these expression types 
> could we also add integer template arguments?


The idea isn't stupid but I think it is already handled. For

  template void f() {}
  template void g() {}
  
  void foo() {
  f<100 * 10240>();
  g();
  }

I got errors

  template-args.cpp:2:18: error: non-type template argument is not a constant 
expression
  template void g() {}
   ^~~
  template-args.cpp:2:26: note: value 1024000 is outside the range of 
representable values of type 'int'
  template void g() {}
   ^
  template-args.cpp:5:5: error: no matching function for call to 'f'
  f<100 * 10240>();
  ^~
  template-args.cpp:1:22: note: candidate template ignored: invalid 
explicitly-specified argument for template parameter 'N'
  template void f() {}
   ^
  template-args.cpp:6:5: error: no matching function for call to 'g'
  g();
  ^
  template-args.cpp:2:40: note: candidate template ignored: couldn't infer 
template argument 'N'
  template void g() {}
 ^
  3 errors generated.

Do you have some other template arguments usage in mind?


https://reviews.llvm.org/D42938



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

[clang-tools-extra] r326598 - [clangd] Use higher timoout values in TUSchedulerTest::Debounce

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Fri Mar  2 10:23:41 2018
New Revision: 326598

URL: http://llvm.org/viewvc/llvm-project?rev=326598=rev
Log:
[clangd] Use higher timoout values in TUSchedulerTest::Debounce

Should unbreak windows buildbots.

Modified:
clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp

Modified: clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp?rev=326598=326597=326598=diff
==
--- clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp Fri Mar  2 
10:23:41 2018
@@ -127,16 +127,17 @@ TEST_F(TUSchedulerTests, Debounce) {
 TUScheduler S(getDefaultAsyncThreadsCount(),
   /*StorePreamblesInMemory=*/true,
   /*ASTParsedCallback=*/nullptr,
-  /*UpdateDebounce=*/std::chrono::milliseconds(50));
+  /*UpdateDebounce=*/std::chrono::seconds(1));
+// FIXME: we could probably use timeouts lower than 1 second here.
 auto Path = testPath("foo.cpp");
 S.update(Path, getInputs(Path, "auto (debounced)"), WantDiagnostics::Auto,
  [&](std::vector Diags) {
ADD_FAILURE() << "auto should have been debounced and canceled";
  });
-std::this_thread::sleep_for(std::chrono::milliseconds(10));
+std::this_thread::sleep_for(std::chrono::milliseconds(200));
 S.update(Path, getInputs(Path, "auto (timed out)"), WantDiagnostics::Auto,
  [&](std::vector Diags) { ++CallbackCount; });
-std::this_thread::sleep_for(std::chrono::milliseconds(60));
+std::this_thread::sleep_for(std::chrono::seconds(2));
 S.update(Path, getInputs(Path, "auto (shut down)"), WantDiagnostics::Auto,
  [&](std::vector Diags) { ++CallbackCount; });
   }


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

r326594 - [OPENMP] Scan all redeclarations looking for `declare simd` attribute.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Fri Mar  2 10:07:00 2018
New Revision: 326594

URL: http://llvm.org/viewvc/llvm-project?rev=326594=rev
Log:
[OPENMP] Scan all redeclarations looking for `declare simd` attribute.

Patch fixes the problem with the functions marked as `declare simd`. If
the canonical declaration does not have associated `declare simd`
construct, we may not generate required code even if other
redeclarations are marked as `declare simd`.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/test/OpenMP/declare_simd_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=326594=326593=326594=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Fri Mar  2 10:07:00 2018
@@ -7861,7 +7861,7 @@ emitX86DeclareSimdFunction(const Functio
 void CGOpenMPRuntime::emitDeclareSimdFunction(const FunctionDecl *FD,
   llvm::Function *Fn) {
   ASTContext  = CGM.getContext();
-  FD = FD->getCanonicalDecl();
+  FD = FD->getMostRecentDecl();
   // Map params to their positions in function decl.
   llvm::DenseMap ParamPositions;
   if (isa(FD))
@@ -7871,80 +7871,84 @@ void CGOpenMPRuntime::emitDeclareSimdFun
 ParamPositions.insert({P->getCanonicalDecl(), ParamPos});
 ++ParamPos;
   }
-  for (auto *Attr : FD->specific_attrs()) {
-llvm::SmallVector ParamAttrs(ParamPositions.size());
-// Mark uniform parameters.
-for (auto *E : Attr->uniforms()) {
-  E = E->IgnoreParenImpCasts();
-  unsigned Pos;
-  if (isa(E))
-Pos = ParamPositions[FD];
-  else {
-auto *PVD = cast(cast(E)->getDecl())
-->getCanonicalDecl();
-Pos = ParamPositions[PVD];
-  }
-  ParamAttrs[Pos].Kind = Uniform;
-}
-// Get alignment info.
-auto NI = Attr->alignments_begin();
-for (auto *E : Attr->aligneds()) {
-  E = E->IgnoreParenImpCasts();
-  unsigned Pos;
-  QualType ParmTy;
-  if (isa(E)) {
-Pos = ParamPositions[FD];
-ParmTy = E->getType();
-  } else {
-auto *PVD = cast(cast(E)->getDecl())
-->getCanonicalDecl();
-Pos = ParamPositions[PVD];
-ParmTy = PVD->getType();
+  while (FD) {
+for (auto *Attr : FD->specific_attrs()) {
+  llvm::SmallVector ParamAttrs(ParamPositions.size());
+  // Mark uniform parameters.
+  for (auto *E : Attr->uniforms()) {
+E = E->IgnoreParenImpCasts();
+unsigned Pos;
+if (isa(E))
+  Pos = ParamPositions[FD];
+else {
+  auto *PVD = cast(cast(E)->getDecl())
+  ->getCanonicalDecl();
+  Pos = ParamPositions[PVD];
+}
+ParamAttrs[Pos].Kind = Uniform;
   }
-  ParamAttrs[Pos].Alignment =
-  (*NI) ? (*NI)->EvaluateKnownConstInt(C)
+  // Get alignment info.
+  auto NI = Attr->alignments_begin();
+  for (auto *E : Attr->aligneds()) {
+E = E->IgnoreParenImpCasts();
+unsigned Pos;
+QualType ParmTy;
+if (isa(E)) {
+  Pos = ParamPositions[FD];
+  ParmTy = E->getType();
+} else {
+  auto *PVD = cast(cast(E)->getDecl())
+  ->getCanonicalDecl();
+  Pos = ParamPositions[PVD];
+  ParmTy = PVD->getType();
+}
+ParamAttrs[Pos].Alignment =
+(*NI)
+? (*NI)->EvaluateKnownConstInt(C)
 : llvm::APSInt::getUnsigned(
   
C.toCharUnitsFromBits(C.getOpenMPDefaultSimdAlign(ParmTy))
   .getQuantity());
-  ++NI;
-}
-// Mark linear parameters.
-auto SI = Attr->steps_begin();
-auto MI = Attr->modifiers_begin();
-for (auto *E : Attr->linears()) {
-  E = E->IgnoreParenImpCasts();
-  unsigned Pos;
-  if (isa(E))
-Pos = ParamPositions[FD];
-  else {
-auto *PVD = cast(cast(E)->getDecl())
-->getCanonicalDecl();
-Pos = ParamPositions[PVD];
+++NI;
   }
-  auto  = ParamAttrs[Pos];
-  ParamAttr.Kind = Linear;
-  if (*SI) {
-if (!(*SI)->EvaluateAsInt(ParamAttr.StrideOrArg, C,
-  Expr::SE_AllowSideEffects)) {
-  if (auto *DRE = cast((*SI)->IgnoreParenImpCasts())) {
-if (auto *StridePVD = cast(DRE->getDecl())) {
-  ParamAttr.Kind = LinearWithVarStride;
-  ParamAttr.StrideOrArg = llvm::APSInt::getUnsigned(
-  ParamPositions[StridePVD->getCanonicalDecl()]);
+  // Mark linear parameters.
+  auto SI = Attr->steps_begin();
+  auto MI = Attr->modifiers_begin();
+  for (auto *E : Attr->linears()) {
+E = 

[PATCH] D43814: [x86][CET] Introduce _get_ssp, _inc_ssp intrinsics

[Craig Topper via Phabricator via cfe-commits]

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

LGTM




Comment at: lib/Headers/cetintrin.h:45
+static __inline__ void __DEFAULT_FN_ATTRS _inc_ssp(unsigned int __a) {
+  __builtin_ia32_incsspq(__a);
+}

GBuella wrote:
> craig.topper wrote:
> > Where is the zeroing behavior for older CPUs coming from? This 
> > implementation looks identical to _incsspq?
> Do we need some zeroing behaviour for _inc_ssp?
> I know we need it for _get_ssp.
Nevermind. I mixed up the instructions, and didn't realize that we already 
modeled rdsspq/rdsspd as taking a value as input. I assumed we would have to do 
more work. Should have looked more closely. Sorry.


https://reviews.llvm.org/D43814



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

[PATCH] D43248: [Attr] Fix parameter indexing for attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D43248#1025495, @jdenny wrote:

> Aaron, thanks for the review.  I've applied your suggestions and am ready to 
> commit.


You're correct that we have a lot of freedom with the commit message, but the 
important piece is in describing what's changed and why (if it's not 
immediately obvious). Both commit messages look good to me.


https://reviews.llvm.org/D43248



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

Re: r326590 - [OPENMP] Treat local variables in CUDA mode as thread local.

[Jonas Hahnfeld via cfe-commits]

Hi Alexey,

Am 2018-03-02 18:17, schrieb Alexey Bataev via cfe-commits:

Author: abataev
Date: Fri Mar  2 09:17:12 2018
New Revision: 326590

URL: http://llvm.org/viewvc/llvm-project?rev=326590=rev
Log:
[OPENMP] Treat local variables in CUDA mode as thread local.

In CUDA mode all local variables are actually thread
local|threadprivate, not private, and, thus, they cannot be shared
between threads|lanes.

Added:
cfe/trunk/test/OpenMP/nvptx_target_cuda_mode_messages.cpp
Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/Sema/SemaOpenMP.cpp

Modified: cfe/trunk/include/clang/Driver/Options.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=326590=326589=326590=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Fri Mar  2 09:17:12 2018
@@ -1427,7 +1427,7 @@ def fopenmp_simd : Flag<["-"], "fopenmp-
   HelpText<"Emit OpenMP code only for SIMD-based constructs.">;
 def fno_openmp_simd : Flag<["-"], "fno-openmp-simd">, Group,
Flags<[CC1Option, NoArgumentUnused]>;
 def fopenmp_cuda_mode : Flag<["-"], "fopenmp-cuda-mode">,
Group, Flags<[CC1Option, NoArgumentUnused]>;
-def fno_openmp_cuda_mode : Flag<["-"], "fno-openmp-cuda-mode">,
Group, Flags<[CC1Option, NoArgumentUnused]>;
+def fno_openmp_cuda_mode : Flag<["-"], "fno-openmp-cuda-mode">,
Group, Flags<[NoArgumentUnused]>;


Did you remove CC1Option intentionally? I think we need this with the 
method Carlo chose.
Btw unless I muss something OpenMPCUDAMode is not the default? If I 
remember correctly we discussed this on Phabricator?


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

r326590 - [OPENMP] Treat local variables in CUDA mode as thread local.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Fri Mar  2 09:17:12 2018
New Revision: 326590

URL: http://llvm.org/viewvc/llvm-project?rev=326590=rev
Log:
[OPENMP] Treat local variables in CUDA mode as thread local.

In CUDA mode all local variables are actually thread
local|threadprivate, not private, and, thus, they cannot be shared
between threads|lanes.

Added:
cfe/trunk/test/OpenMP/nvptx_target_cuda_mode_messages.cpp
Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/Sema/SemaOpenMP.cpp

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=326590=326589=326590=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Fri Mar  2 09:17:12 2018
@@ -1427,7 +1427,7 @@ def fopenmp_simd : Flag<["-"], "fopenmp-
   HelpText<"Emit OpenMP code only for SIMD-based constructs.">;
 def fno_openmp_simd : Flag<["-"], "fno-openmp-simd">, Group, 
Flags<[CC1Option, NoArgumentUnused]>;
 def fopenmp_cuda_mode : Flag<["-"], "fopenmp-cuda-mode">, Group, 
Flags<[CC1Option, NoArgumentUnused]>;
-def fno_openmp_cuda_mode : Flag<["-"], "fno-openmp-cuda-mode">, 
Group, Flags<[CC1Option, NoArgumentUnused]>;
+def fno_openmp_cuda_mode : Flag<["-"], "fno-openmp-cuda-mode">, 
Group, Flags<[NoArgumentUnused]>;
 def fno_optimize_sibling_calls : Flag<["-"], "fno-optimize-sibling-calls">, 
Group;
 def foptimize_sibling_calls : Flag<["-"], "foptimize-sibling-calls">, 
Group;
 def fno_escaping_block_tail_calls : Flag<["-"], 
"fno-escaping-block-tail-calls">, Group, Flags<[CC1Option]>;

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=326590=326589=326590=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Fri Mar  2 09:17:12 2018
@@ -936,10 +936,11 @@ DSAStackTy::getTopMostTaskgroupReduction
 
 bool DSAStackTy::isOpenMPLocal(VarDecl *D, StackTy::reverse_iterator Iter) {
   D = D->getCanonicalDecl();
-  if (!isStackEmpty() && Stack.back().first.size() > 1) {
+  if (!isStackEmpty()) {
 reverse_iterator I = Iter, E = Stack.back().first.rend();
 Scope *TopScope = nullptr;
-while (I != E && !isParallelOrTaskRegion(I->Directive))
+while (I != E && !isParallelOrTaskRegion(I->Directive) &&
+   !isOpenMPTargetExecutionDirective(I->Directive))
   ++I;
 if (I == E)
   return false;
@@ -956,20 +957,7 @@ DSAStackTy::DSAVarData DSAStackTy::getTo
   D = getCanonicalDecl(D);
   DSAVarData DVar;
 
-  // OpenMP [2.9.1.1, Data-sharing Attribute Rules for Variables Referenced
-  // in a Construct, C/C++, predetermined, p.1]
-  //  Variables appearing in threadprivate directives are threadprivate.
   auto *VD = dyn_cast(D);
-  if ((VD && VD->getTLSKind() != VarDecl::TLS_None &&
-   !(VD->hasAttr() &&
- SemaRef.getLangOpts().OpenMPUseTLS &&
- SemaRef.getASTContext().getTargetInfo().isTLSSupported())) ||
-  (VD && VD->getStorageClass() == SC_Register &&
-   VD->hasAttr() && !VD->isLocalVarDecl())) {
-addDSA(D, buildDeclRefExpr(SemaRef, VD, D->getType().getNonReferenceType(),
-   D->getLocation()),
-   OMPC_threadprivate);
-  }
   auto TI = Threadprivates.find(D);
   if (TI != Threadprivates.end()) {
 DVar.RefExpr = TI->getSecond().RefExpr.getPointer();
@@ -981,6 +969,62 @@ DSAStackTy::DSAVarData DSAStackTy::getTo
 VD->getAttr()->getLocation());
 DVar.CKind = OMPC_threadprivate;
 addDSA(D, DVar.RefExpr, OMPC_threadprivate);
+return DVar;
+  }
+  // OpenMP [2.9.1.1, Data-sharing Attribute Rules for Variables Referenced
+  // in a Construct, C/C++, predetermined, p.1]
+  //  Variables appearing in threadprivate directives are threadprivate.
+  if ((VD && VD->getTLSKind() != VarDecl::TLS_None &&
+   !(VD->hasAttr() &&
+ SemaRef.getLangOpts().OpenMPUseTLS &&
+ SemaRef.getASTContext().getTargetInfo().isTLSSupported())) ||
+  (VD && VD->getStorageClass() == SC_Register &&
+   VD->hasAttr() && !VD->isLocalVarDecl())) {
+DVar.RefExpr = buildDeclRefExpr(
+SemaRef, VD, D->getType().getNonReferenceType(), D->getLocation());
+DVar.CKind = OMPC_threadprivate;
+addDSA(D, DVar.RefExpr, OMPC_threadprivate);
+return DVar;
+  }
+  if (SemaRef.getLangOpts().OpenMPCUDAMode && VD &&
+  VD->isLocalVarDeclOrParm() && !isStackEmpty() &&
+  !isLoopControlVariable(D).first) {
+auto IterTarget =
+std::find_if(Stack.back().first.rbegin(), Stack.back().first.rend(),
+ [](const SharingMapTy ) {
+   return isOpenMPTargetExecutionDirective(Data.Directive);
+ });
+if (IterTarget != Stack.back().first.rend()) {
+  auto 

[PATCH] D43248: [Attr] Fix parameter indexing for attributes

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

Aaron, thanks for the review.  I've applied your suggestions and am ready to 
commit.

I've noticed a variety of styles in commit logs, I've read the coding 
standards, and it seems there's a lot of freedom here.  Below are the two 
commit logs I'm planning to use.  Please let me know if you have any 
objections.  Thanks.

  [Attr] Fix parameter indexing for several attributes
  
  The patch fixes a number of bugs related to parameter indexing in
  attributes:
  
  * Parameter indices in some attributes (argument_with_type_tag,
pointer_with_type_tag, nonnull, ownership_takes, ownership_holds,
and ownership_returns) are specified in source as one-origin
including any C++ implicit this parameter, were stored as
zero-origin excluding any this parameter, and were erroneously
printing (-ast-print) and confusingly dumping (-ast-dump) as the
stored values.
  
  * For alloc_size, the C++ implicit this parameter was not subtracted
correctly in Sema, leading to assert failures or to silent failures
of __builtin_object_size to compute a value.
  
  * For argument_with_type_tag, pointer_with_type_tag, and
ownership_returns, the C++ implicit this parameter was not added
back to parameter indices in some diagnostics.
  
  This patch fixes the above bugs and aims to prevent similar bugs in
  the future by introducing careful mechanisms for handling parameter
  indices in attributes.  ParamIdx stores a parameter index and is
  designed to hide the stored encoding while providing accessors that
  require each use (such as printing) to make explicit the encoding that
  is needed.  Attribute declarations declare parameter index arguments
  as [Variadic]ParamIdxArgument, which are exposed as ParamIdx[*].  This
  patch rewrites all attribute arguments that are processed by
  checkFunctionOrMethodParameterIndex in SemaDeclAttr.cpp to be declared
  as [Variadic]ParamIdxArgument.  The only exception is xray_log_args's
  argument, which is encoded as a count not an index.
  
  Differential Revision: https://reviews.llvm.org/D43248

For the test/Sema/attr-ownership.c change:

  [Attr] Use -fsyntax-only in test
  
  Suggested at: https://reviews.llvm.org/D43248


https://reviews.llvm.org/D43248



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

[PATCH] D43248: [Attr] Fix parameter indexing for attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: test/Sema/attr-ownership.cpp:1
+// RUN: %clang_cc1 %s -verify
+

jdenny wrote:
> aaron.ballman wrote:
> > Please pass `-fsyntax-only` as well.
> Sure.  Would you like me to change test/Sema/attr-ownership.c while we're 
> thinking about it?
Because it's unrelated to this patch, that can be done in a separate commit (no 
review required, you can just make the change and commit it).


https://reviews.llvm.org/D43248



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

[PATCH] D43248: [Attr] Fix parameter indexing for attributes

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added inline comments.



Comment at: test/Sema/attr-ownership.cpp:1
+// RUN: %clang_cc1 %s -verify
+

aaron.ballman wrote:
> Please pass `-fsyntax-only` as well.
Sure.  Would you like me to change test/Sema/attr-ownership.c while we're 
thinking about it?


https://reviews.llvm.org/D43248



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

[PATCH] D43281: [AMDGPU] fixes for lds f32 builtins

[Daniil Fukalov via Phabricator via cfe-commits]

dfukalov updated this revision to Diff 136752.
dfukalov edited the summary of this revision.
dfukalov set the repository for this revision to rC Clang.
dfukalov added a comment.

addrspace specifications are kept in descriptions strings


Repository:
  rC Clang

https://reviews.llvm.org/D43281

Files:
  include/clang/Basic/BuiltinsAMDGPU.def
  lib/CodeGen/CGBuiltin.cpp
  test/SemaOpenCL/builtins-amdgcn-error.cl


Index: test/SemaOpenCL/builtins-amdgcn-error.cl
===
--- test/SemaOpenCL/builtins-amdgcn-error.cl
+++ test/SemaOpenCL/builtins-amdgcn-error.cl
@@ -102,3 +102,20 @@
   *out = __builtin_amdgcn_mov_dpp(a, 0, 0, 0, e); // expected-error {{argument 
to '__builtin_amdgcn_mov_dpp' must be a constant integer}}
 }
 
+void test_ds_fadd(__attribute__((address_space(3))) float *out, float src, int 
a) {
+  *out = __builtin_amdgcn_ds_fadd(out, src, a, 0, false); // expected-error 
{{argument to '__builtin_amdgcn_ds_fadd' must be a constant integer}}
+  *out = __builtin_amdgcn_ds_fadd(out, src, 0, a, false); // expected-error 
{{argument to '__builtin_amdgcn_ds_fadd' must be a constant integer}}
+  *out = __builtin_amdgcn_ds_fadd(out, src, 0, 0, a); // expected-error 
{{argument to '__builtin_amdgcn_ds_fadd' must be a constant integer}}
+}
+
+void test_ds_fmin(__attribute__((address_space(3))) float *out, float src, int 
a) {
+  *out = __builtin_amdgcn_ds_fmin(out, src, a, 0, false); // expected-error 
{{argument to '__builtin_amdgcn_ds_fmin' must be a constant integer}}
+  *out = __builtin_amdgcn_ds_fmin(out, src, 0, a, false); // expected-error 
{{argument to '__builtin_amdgcn_ds_fmin' must be a constant integer}}
+  *out = __builtin_amdgcn_ds_fmin(out, src, 0, 0, a); // expected-error 
{{argument to '__builtin_amdgcn_ds_fmin' must be a constant integer}}
+}
+
+void test_ds_fmax(__attribute__((address_space(3))) float *out, float src, int 
a) {
+  *out = __builtin_amdgcn_ds_fmax(out, src, a, 0, false); // expected-error 
{{argument to '__builtin_amdgcn_ds_fmax' must be a constant integer}}
+  *out = __builtin_amdgcn_ds_fmax(out, src, 0, a, false); // expected-error 
{{argument to '__builtin_amdgcn_ds_fmax' must be a constant integer}}
+  *out = __builtin_amdgcn_ds_fmax(out, src, 0, 0, a); // expected-error 
{{argument to '__builtin_amdgcn_ds_fmax' must be a constant integer}}
+}
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -9860,6 +9860,49 @@
 CI->setConvergent();
 return CI;
   }
+  case AMDGPU::BI__builtin_amdgcn_ds_fadd:
+  case AMDGPU::BI__builtin_amdgcn_ds_fmin:
+  case AMDGPU::BI__builtin_amdgcn_ds_fmax: {
+llvm::SmallVector Args;
+for (unsigned I = 0; I != 5; ++I)
+  Args.push_back(EmitScalarExpr(E->getArg(I)));
+const llvm::Type *PtrTy = Args[0]->getType();
+// check pointer parameter
+if (!PtrTy->isPointerTy() ||
+LangAS::opencl_local != E->getArg(0)
+->getType()
+->getPointeeType()
+.getQualifiers()
+.getAddressSpace() ||
+!PtrTy->getPointerElementType()->isFloatTy()) {
+  CGM.Error(E->getArg(0)->getLocStart(),
+"parameter should have type \"local float*\"");
+  return nullptr;
+}
+// check float parameter
+if (!Args[1]->getType()->isFloatTy()) {
+  CGM.Error(E->getArg(1)->getLocStart(),
+"parameter should have type \"float\"");
+  return nullptr;
+}
+
+Intrinsic::ID ID;
+switch (BuiltinID) {
+case AMDGPU::BI__builtin_amdgcn_ds_fadd:
+  ID = Intrinsic::amdgcn_ds_fadd;
+  break;
+case AMDGPU::BI__builtin_amdgcn_ds_fmin:
+  ID = Intrinsic::amdgcn_ds_fmin;
+  break;
+case AMDGPU::BI__builtin_amdgcn_ds_fmax:
+  ID = Intrinsic::amdgcn_ds_fmax;
+  break;
+default:
+  llvm_unreachable("Unknown BuiltinID");
+}
+Value *F = CGM.getIntrinsic(ID);
+return Builder.CreateCall(F, Args);
+  }
 
   // amdgcn workitem
   case AMDGPU::BI__builtin_amdgcn_workitem_id_x:
Index: include/clang/Basic/BuiltinsAMDGPU.def
===
--- include/clang/Basic/BuiltinsAMDGPU.def
+++ include/clang/Basic/BuiltinsAMDGPU.def
@@ -93,9 +93,9 @@
 BUILTIN(__builtin_amdgcn_readfirstlane, "ii", "nc")
 BUILTIN(__builtin_amdgcn_readlane, "iii", "nc")
 BUILTIN(__builtin_amdgcn_fmed3f, "", "nc")
-BUILTIN(__builtin_amdgcn_ds_fadd, "ff*3fiib", "n")
-BUILTIN(__builtin_amdgcn_ds_fmin, "ff*3fiib", "n")
-BUILTIN(__builtin_amdgcn_ds_fmax, "ff*3fiib", "n")
+BUILTIN(__builtin_amdgcn_ds_fadd, "ff*3fIiIiIb", "n")
+BUILTIN(__builtin_amdgcn_ds_fmin, "ff*3fIiIiIb", "n")
+BUILTIN(__builtin_amdgcn_ds_fmax, "ff*3fIiIiIb", "n")
 
 

[PATCH] D41102: Setup clang-doc frontend framework

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Could some other people please review this differential, too?
I'm sure i have missed things.

---

Some more nitpicking.

For this differential as standalone, i'we mostly run out of things to nitpick.
Some things can probably be done better (the blockid/recordid stuff could 
probably be nicer if tablegen-ed, but that is for later).

I'll try to look at the next differential, and at them combined.




Comment at: clang-doc/BitcodeWriter.cpp:120
+BlockIdNameMap[Init.first] = Init.second;
+assert((BlockIdNameMap[Init.first].size() + 1) <=
+   BitCodeConstants::RecordSize);

We don't actually push these strings to the `Record` (but instead output them 
directly), so this assertion is not really meaningful, i think?



Comment at: clang-doc/BitcodeWriter.h:21
+#include "clang/AST/AST.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/Bitcode/BitstreamWriter.h"

+DenseMap



Comment at: clang-doc/BitcodeWriter.h:21
+#include "clang/AST/AST.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/Bitcode/BitstreamWriter.h"

lebedev.ri wrote:
> +DenseMap
+StringRef



Comment at: clang-doc/BitcodeWriter.h:197
+: Stream(Stream_) {
+  Stream.EnterSubblock(ID, BitCodeConstants::SubblockIDSize);
+}

Humm, you could avoid this constant, and conserve a few bits, if you move the 
init-list out of `emitBlockInfoBlock()` to somewhere e.g. after the `enum 
RecordId`, and then since the `BlockId ID` is already passed, you could compute 
it on-the-fly the same way the `BitCodeConstants::SubblockIDSize` is asserted 
in `emitBlockInfo*()`.
Not sure if it's worth doing though. Maybe just add it as a `NOTE` here.



Comment at: clang-doc/BitcodeWriter.h:249
+///
+/// \param WriteBlockInfo
+/// For serializing a single info (as in the mapper

Stale comment



Comment at: clang-doc/Representation.h:60
+  InfoType RefType = InfoType::IT_default;
+  Info *Ref;
+};

`Info *Ref;` isn't used anywhere



Comment at: clang-doc/Representation.h:117
+  bool IsDefinition = false;
+  Location DefLoc;
+  llvm::SmallVector Loc;

`llvm::Optional DefLoc;`  ?


https://reviews.llvm.org/D41102



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

[PATCH] D42787: clang-format: do not add extra indent when wrapping last parameter

[Daniel Jasper via Phabricator via cfe-commits]

djasper added a comment.

We have talked about that and none of us agree.


Repository:
  rC Clang

https://reviews.llvm.org/D42787



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

[PATCH] D42787: clang-format: do not add extra indent when wrapping last parameter

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

> Also double-checked with Richard Smith and he agrees that the current 
> behavior is preferable. For comma and plus this doesn't seem overly 
> important, but making it:
> 
>   aa(b + ccc *
>  d);
>
> 
> seems really bad to him as this suggests that we are adding both ccc 
> and d.
> 
>   aa(b + ccc *
>  d);
>
> 
> seems clearer.

And I fully agree with this!
But I don't think that would be affected by my patch... I hope it does not at 
least, and I'll try to double check (and add a test).

> And for consistency reasons, we should not treat those two cases differently.

The consistency is related only to the implementation of clang-format: for a 
language (and its users) they are not the same things, comma separating 
arguments are not operators and are used to separate different expressions.


Repository:
  rC Clang

https://reviews.llvm.org/D42787



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

[PATCH] D42787: clang-format: do not add extra indent when wrapping last parameter

[Daniel Jasper via Phabricator via cfe-commits]

djasper added a comment.

In https://reviews.llvm.org/D42787#1025117, @Typz wrote:

> If people don't care about this case, we might as well merge this :-) Just 
> kidding.
>
> The tweak matches both our expectation, the auto-indent behaviour of IDE (not 
> to be trusted, but still probably of 'default' behaviour for many people, 
> esp. when you don't yet use a formatter), and its seems our code base is 
> generally formatted like this. I think it is quite more frequent than 50 
> times in whole LLVM/Clang, but I have no actual numbers; do you have a magic 
> tool to search for such specific "code pattern", or just run clang-format 
> with one option then the next and count the differences ?


I just tweaked a search based on regular expressions. Fundamentally something 
like a line with an open paren and a comma that doesn't end in a comma gives a 
reasonable first approximation. But yes, first formatting with one option, then 
reformatting and looking at numbers of diffs would be interesting. And I would 
bet that even in your codebase diffs are few.

Also double-checked with Richard Smith and he agrees that the current behavior 
is preferable. For comma and plus this doesn't seem overly important, but 
making it:

  aa(b + ccc *
 d);

seems really bad to him as this suggests that we are adding both ccc 
and d.

  aa(b + ccc *
 d);

seems clearer. And for consistency reasons, we should not treat those two cases 
differently.


Repository:
  rC Clang

https://reviews.llvm.org/D42787



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

[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

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

xazax.hun abandoned this revision.
xazax.hun added a comment.

Resubmitted in https://reviews.llvm.org/rL326439

Phabricator did not display the mailing list conversation. The point is, the 
circular dependency does not exist in the upstream version of clang. The reason 
is that CMake does not track the header includes as dependencies. There might 
be some layering violations with the header includes but those are independent 
of this patch and need to be fixed separately.


Repository:
  rC Clang

https://reviews.llvm.org/D30691



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

[PATCH] D43847: [clang-tidy] Add check: replace string::find(...) == 0 with absl::StartsWith

[ahedberg via Phabricator via cfe-commits]

ahedberg added a comment.

In https://reviews.llvm.org/D43847#1024018, @aaron.ballman wrote:

> In https://reviews.llvm.org/D43847#1023452, @hokein wrote:
>
> > In https://reviews.llvm.org/D43847#1021967, @aaron.ballman wrote:
> >
> > > I need a bit more context because I'm unfamiliar with `absl`. What is 
> > > this module's intended use?
> >
> >
> > As `absl` has been open-sourced (https://github.com/abseil/abseil-cpp), I 
> > think there will be more absl-related checks contributed from google or 
> > external contributors in the future, so it make sense to create a new 
> > module.
>
>
> Ah, so this is for abseil, good to know. Yeah, I think we should have a new 
> module for it but perhaps with the full name instead of this shortened 
> version?


+1 to the name `abseil`. Some folks from the Abseil team discussed 
yesterday--we want to keep other projects from using nested absl namespaces to 
avoid ever having to type ::absl. So it would be nice to replace that nested 
namespace with something else; maybe clang::tidy::abseil?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43847



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

[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch

[Daniel Jasper via Phabricator via cfe-commits]

djasper added a comment.

Got two responses so far.

1. Having to closing braces in the same column is weird, but not that weird. 
Consider



  namespace n {
  void f() {
...
  }
  }

2. Richard Smith (code owner of Clang) says that he doesn't really like the two 
closing braces in the same column, but he always cheats by removing the braces 
for the last case label / default. You never need them. In any case he'd 
strongly prefer the current behavior over adding an extra break and more 
indentation.

In conclusion, I don't think we want to move forward with making clang-format do

  switch (x) {
  case 1:
{
  doSomething();
}
  }

even with IndentCaseLabels: false.


Repository:
  rC Clang

https://reviews.llvm.org/D43183



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

[PATCH] D37057: [clang] Require address space to be specified when creating functions (3/3)

[Dylan McKay via Phabricator via cfe-commits]

dylanmckay updated this revision to Diff 136734.
dylanmckay added a comment.

Rebase


Repository:
  rC Clang

https://reviews.llvm.org/D37057

Files:
  lib/CodeGen/CGBlocks.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CGCUDANV.cpp
  lib/CodeGen/CGDeclCXX.cpp
  lib/CodeGen/CGException.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGGPUBuiltin.cpp
  lib/CodeGen/CGNonTrivialStruct.cpp
  lib/CodeGen/CGObjC.cpp
  lib/CodeGen/CGObjCGNU.cpp
  lib/CodeGen/CGObjCMac.cpp
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CGStmt.cpp
  lib/CodeGen/CGStmtOpenMP.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/CodeGen/MicrosoftCXXABI.cpp
  lib/CodeGen/TargetInfo.cpp

Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -9137,7 +9137,7 @@
   std::string Name = Invoke->getName().str() + "_kernel";
   auto *FT = llvm::FunctionType::get(llvm::Type::getVoidTy(C), ArgTys, false);
   auto *F = llvm::Function::Create(FT, llvm::GlobalValue::InternalLinkage, Name,
-   ());
+   CGF.CGM.getModule());
   auto IP = CGF.Builder.saveIP();
   auto *BB = llvm::BasicBlock::Create(C, "entry", F);
   auto  = CGF.Builder;
@@ -9195,7 +9195,7 @@
   std::string Name = Invoke->getName().str() + "_kernel";
   auto *FT = llvm::FunctionType::get(llvm::Type::getVoidTy(C), ArgTys, false);
   auto *F = llvm::Function::Create(FT, llvm::GlobalValue::InternalLinkage, Name,
-   ());
+   CGF.CGM.getModule());
   F->addFnAttr("enqueued-block");
   auto IP = CGF.Builder.saveIP();
   auto *BB = llvm::BasicBlock::Create(C, "entry", F);
Index: lib/CodeGen/MicrosoftCXXABI.cpp
===
--- lib/CodeGen/MicrosoftCXXABI.cpp
+++ lib/CodeGen/MicrosoftCXXABI.cpp
@@ -1948,7 +1948,7 @@
   llvm::FunctionType *ThunkTy = CGM.getTypes().GetFunctionType(FnInfo);
   llvm::Function *ThunkFn =
   llvm::Function::Create(ThunkTy, llvm::Function::ExternalLinkage,
- ThunkName.str(), ());
+ ThunkName.str(), CGM.getModule());
   assert(ThunkFn->getName() == ThunkName && "name was uniqued!");
 
   ThunkFn->setLinkage(MD->isExternallyVisible()
@@ -3876,7 +3876,7 @@
   const CXXRecordDecl *RD = CD->getParent();
   QualType RecordTy = getContext().getRecordType(RD);
   llvm::Function *ThunkFn = llvm::Function::Create(
-  ThunkTy, getLinkageForRTTI(RecordTy), ThunkName.str(), ());
+  ThunkTy, getLinkageForRTTI(RecordTy), ThunkName.str(), CGM.getModule());
   ThunkFn->setCallingConv(static_cast(
   FnInfo.getEffectiveCallingConvention()));
   if (ThunkFn->isWeakForLinker())
Index: lib/CodeGen/ItaniumCXXABI.cpp
===
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -2287,7 +2287,7 @@
   llvm::FunctionType *FnTy = CGM.getTypes().GetFunctionType(FI);
   llvm::Function *Wrapper =
   llvm::Function::Create(FnTy, getThreadLocalWrapperLinkage(VD, CGM),
- WrapperName.str(), ());
+ WrapperName.str(), CGM.getModule());
 
   CGM.SetLLVMFunctionAttributes(nullptr, FI, Wrapper);
 
@@ -2394,7 +2394,7 @@
   llvm::FunctionType *FnTy = llvm::FunctionType::get(CGM.VoidTy, false);
   Init = llvm::Function::Create(FnTy,
 llvm::GlobalVariable::ExternalWeakLinkage,
-InitFnName.str(), ());
+InitFnName.str(), CGM.getModule());
   const CGFunctionInfo  = CGM.getTypes().arrangeNullaryFunction();
   CGM.SetLLVMFunctionAttributes(nullptr, FI, cast(Init));
 }
Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -2446,7 +2446,7 @@
 
   llvm::Function *F =
   llvm::Function::Create(FTy, llvm::Function::ExternalLinkage,
- Entry ? StringRef() : MangledName, ());
+ Entry ? StringRef() : MangledName, getModule());
 
   // If we already created a function with the same mangled name (but different
   // type) before, take its name and add it to the list of functions to be
Index: lib/CodeGen/CGStmtOpenMP.cpp
===
--- lib/CodeGen/CGStmtOpenMP.cpp
+++ lib/CodeGen/CGStmtOpenMP.cpp
@@ -452,7 +452,7 @@
 
   llvm::Function *F =
   llvm::Function::Create(FuncLLVMTy, llvm::GlobalValue::InternalLinkage,
- FO.FunctionName, ());
+ FO.FunctionName, CGM.getModule());
   CGM.SetInternalFunctionAttributes(CD, F, FuncInfo);
   if (CD->isNothrow())

[PATCH] D43928: [analyzer] Correctly measure array size in security.insecureAPI.strcpy

[András Leitereg via Phabricator via cfe-commits]

leanil added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:517
 if (const auto *Array = dyn_cast(DeclRef->getType())) {
-  uint64_t ArraySize = BR.getContext().getTypeSize(Array) / 8;
+  auto ArraySize = BR.getContext().getTypeSizeInChars(Array).getQuantity();
   if (const auto *String = dyn_cast(Source)) {

NoQ wrote:
> I suggest not using `auto` here, because it makes it harder to understand 
> integer promotions (potential overflows or sign extensions) in the comparison.
That's true. Should it be `CharUnits::QuantityType` then?


https://reviews.llvm.org/D43928



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

[PATCH] D42983: [clang-tidy] Add readability-simd-intrinsics check.

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

A late comment here: should this check start a new "portability" module? This 
seems to be the main focus of the check rather than making code more readable.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42983



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

r326551 - [Sema] Improve test coverage of narrowing conversion diagnostics

[Mikhail Maltsev via cfe-commits]

Author: miyuki
Date: Fri Mar  2 02:03:02 2018
New Revision: 326551

URL: http://llvm.org/viewvc/llvm-project?rev=326551=rev
Log:
[Sema] Improve test coverage of narrowing conversion diagnostics

Summary:
This patch adds tests of narrowing conversion diagnostics for the
'unscoped enum -> integer' case.

Reviewers: faisalv, rsmith, rogfer01

Reviewed By: rogfer01

Subscribers: cfe-commits, rogfer01

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

Modified:
cfe/trunk/test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp

Modified: cfe/trunk/test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp?rev=326551=326550=326551=diff
==
--- cfe/trunk/test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp (original)
+++ cfe/trunk/test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp Fri Mar  2 
02:03:02 2018
@@ -147,8 +147,10 @@ void int_to_float() {
 void shrink_int() {
   // Not a constant expression.
   short s = 1;
+  UnscopedEnum e = EnumVal;
   unsigned short us = 1;
   Agg c1 = {s};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}}
+  Agg c2 = {e};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}}
   Agg s1 = {s};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}}
   Agg s2 = {us};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}}
 
@@ -158,16 +160,19 @@ void shrink_int() {
   // long).
   long l1 = 1;
   Agg i1 = {l1};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}}
+  Agg i2 = {e};  // OK
   long long ll = 1;
   Agg l2 = {ll};  // OK
 
   // Constants.
-  Agg c2 = {127};  // OK
-  Agg c3 = {300};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}} expected-warning {{changes value}}
+  Agg c3 = {127};  // OK
+  Agg c4 = {300};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}} expected-warning {{changes value}}
+  Agg c5 = {EnumVal};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}} expected-warning {{changes value}}
 
-  Agg i2 = {0x7FFFU};  // OK
-  Agg i3 = {0x8000U};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}}
-  Agg i4 = {-0x8000L};  // expected-error {{ cannot be 
narrowed }} expected-note {{silence}}
+  Agg i3 = {0x7FFFU};  // OK
+  Agg i4 = {EnumVal};  // OK
+  Agg i5 = {0x8000U};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}}
+  Agg i6 = {-0x8000L};  // expected-error {{ cannot be 
narrowed }} expected-note {{silence}}
 
   // Bool is also an integer type, but conversions to it are a different AST
   // node.


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

[PATCH] D42938: [Sema] Emit -Winteger-overflow for arguments in function calls, ObjC messages.

[Jan Korous via Phabricator via cfe-commits]

jkorous-apple added a comment.

Maybe a stupid idea but in case it makes sense to add these expression types 
could we also add integer template arguments?


https://reviews.llvm.org/D42938



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

r326566 - Add possibility to specify output stream for CompilerInstance

[via cfe-commits]

Author: AlexeySotkin
Date: Fri Mar  2 04:11:40 2018
New Revision: 326566

URL: http://llvm.org/viewvc/llvm-project?rev=326566=rev
Log:
Add possibility to specify output stream for CompilerInstance

Patch by: krisb

Reviewers: teemperor

Reviewed By: teemperor

Subscribers: klimek, mgorny, cfe-commits

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

Added:
cfe/trunk/unittests/Frontend/OutputStreamTest.cpp
Modified:
cfe/trunk/include/clang/Frontend/CompilerInstance.h
cfe/trunk/lib/CodeGen/CodeGenAction.cpp
cfe/trunk/unittests/Frontend/CMakeLists.txt

Modified: cfe/trunk/include/clang/Frontend/CompilerInstance.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CompilerInstance.h?rev=326566=326565=326566=diff
==
--- cfe/trunk/include/clang/Frontend/CompilerInstance.h (original)
+++ cfe/trunk/include/clang/Frontend/CompilerInstance.h Fri Mar  2 04:11:40 2018
@@ -183,6 +183,9 @@ class CompilerInstance : public ModuleLo
   /// The list of active output files.
   std::list OutputFiles;
 
+  /// Force an output buffer.
+  std::unique_ptr OutputStream;
+
   CompilerInstance(const CompilerInstance &) = delete;
   void operator=(const CompilerInstance &) = delete;
 public:
@@ -773,6 +776,14 @@ public:
 
   /// }
 
+  void setOutputStream(std::unique_ptr OutStream) {
+OutputStream = std::move(OutStream);
+  }
+
+  std::unique_ptr takeOutputStream() {
+return std::move(OutputStream);
+  }
+
   // Create module manager.
   void createModuleManager();
 

Modified: cfe/trunk/lib/CodeGen/CodeGenAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenAction.cpp?rev=326566=326565=326566=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenAction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenAction.cpp Fri Mar  2 04:11:40 2018
@@ -846,7 +846,10 @@ GetOutputStream(CompilerInstance , St
 std::unique_ptr
 CodeGenAction::CreateASTConsumer(CompilerInstance , StringRef InFile) {
   BackendAction BA = static_cast(Act);
-  std::unique_ptr OS = GetOutputStream(CI, InFile, BA);
+  std::unique_ptr OS = CI.takeOutputStream();
+  if (!OS)
+OS = GetOutputStream(CI, InFile, BA);
+
   if (BA != Backend_EmitNothing && !OS)
 return nullptr;
 

Modified: cfe/trunk/unittests/Frontend/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Frontend/CMakeLists.txt?rev=326566=326565=326566=diff
==
--- cfe/trunk/unittests/Frontend/CMakeLists.txt (original)
+++ cfe/trunk/unittests/Frontend/CMakeLists.txt Fri Mar  2 04:11:40 2018
@@ -9,6 +9,7 @@ add_clang_unittest(FrontendTests
   CodeGenActionTest.cpp
   ParsedSourceLocationTest.cpp
   PCHPreambleTest.cpp
+  OutputStreamTest.cpp
   )
 target_link_libraries(FrontendTests
   PRIVATE
@@ -18,4 +19,5 @@ target_link_libraries(FrontendTests
   clangLex
   clangSema
   clangCodeGen
+  clangFrontendTool
   )

Added: cfe/trunk/unittests/Frontend/OutputStreamTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Frontend/OutputStreamTest.cpp?rev=326566=auto
==
--- cfe/trunk/unittests/Frontend/OutputStreamTest.cpp (added)
+++ cfe/trunk/unittests/Frontend/OutputStreamTest.cpp Fri Mar  2 04:11:40 2018
@@ -0,0 +1,46 @@
+//===- unittests/Frontend/OutputStreamTest.cpp --- FrontendAction tests --===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/CodeGen/BackendUtil.h"
+#include "clang/CodeGen/CodeGenAction.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/FrontendTool/Utils.h"
+#include "clang/Lex/PreprocessorOptions.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace clang;
+using namespace clang::frontend;
+
+namespace {
+
+TEST(FrontendOutputTests, TestOutputStream) {
+  auto Invocation = std::make_shared();
+  Invocation->getPreprocessorOpts().addRemappedFile(
+  "test.cc", MemoryBuffer::getMemBuffer("").release());
+  Invocation->getFrontendOpts().Inputs.push_back(
+  FrontendInputFile("test.cc", InputKind::CXX));
+  Invocation->getFrontendOpts().ProgramAction = EmitBC;
+  Invocation->getTargetOpts().Triple = "i386-unknown-linux-gnu";
+  CompilerInstance Compiler;
+
+  SmallVector IRBuffer;
+  std::unique_ptr IRStream(
+  new raw_svector_ostream(IRBuffer));
+
+  Compiler.setOutputStream(std::move(IRStream));
+  Compiler.setInvocation(std::move(Invocation));
+  Compiler.createDiagnostics();
+
+  bool Success = ExecuteCompilerInvocation();
+  EXPECT_TRUE(Success);
+  EXPECT_TRUE(!IRBuffer.empty());
+  

[PATCH] D43248: [Attr] Fix parameter indexing for attributes

[Aaron Ballman via Phabricator via cfe-commits]

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

Aside from two minor nits, this LGTM. Thank you for working on it!




Comment at: test/Sema/attr-ownership.cpp:1
+// RUN: %clang_cc1 %s -verify
+

Please pass `-fsyntax-only` as well.



Comment at: utils/TableGen/ClangAttrEmitter.cpp:763
+ << "assert(HasThis == Idx.hasThis() && "
+"\"HasThis must be consistent\");\n"
+ << "  }\n"

Given how oddly this is wrapped, might as well make this a stream argument 
rather than a string literal concatenation.


https://reviews.llvm.org/D43248



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

[PATCH] D44003: [clangd] Fix unintentionally loose fuzzy matching, and the tests masking it.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 136719.
sammccall added a comment.

test tweaks


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44003

Files:
  clangd/FuzzyMatch.cpp
  clangd/FuzzyMatch.h
  unittests/clangd/FuzzyMatchTests.cpp
  unittests/clangd/IndexTests.cpp

Index: unittests/clangd/IndexTests.cpp
===
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -116,14 +116,6 @@
   EXPECT_TRUE(Symbols.expired());
 }
 
-TEST(MemIndexTest, MemIndexMatchSubstring) {
-  MemIndex I;
-  I.build(generateNumSymbols(5, 25));
-  FuzzyFindRequest Req;
-  Req.Query = "5";
-  EXPECT_THAT(match(I, Req), UnorderedElementsAre("5", "15", "25"));
-}
-
 TEST(MemIndexTest, MemIndexDeduplicate) {
   auto Symbols = generateNumSymbols(0, 10);
 
@@ -166,46 +158,46 @@
 
 TEST(MemIndexTest, MatchQualifiedNamesWithoutSpecificScope) {
   MemIndex I;
-  I.build(generateSymbols({"a::xyz", "b::yz", "yz"}));
+  I.build(generateSymbols({"a::y1", "b::y2", "y3"}));
   FuzzyFindRequest Req;
   Req.Query = "y";
-  EXPECT_THAT(match(I, Req), UnorderedElementsAre("a::xyz", "b::yz", "yz"));
+  EXPECT_THAT(match(I, Req), UnorderedElementsAre("a::y1", "b::y2", "y3"));
 }
 
 TEST(MemIndexTest, MatchQualifiedNamesWithGlobalScope) {
   MemIndex I;
-  I.build(generateSymbols({"a::xyz", "b::yz", "yz"}));
+  I.build(generateSymbols({"a::y1", "b::y2", "y3"}));
   FuzzyFindRequest Req;
   Req.Query = "y";
   Req.Scopes = {""};
-  EXPECT_THAT(match(I, Req), UnorderedElementsAre("yz"));
+  EXPECT_THAT(match(I, Req), UnorderedElementsAre("y3"));
 }
 
 TEST(MemIndexTest, MatchQualifiedNamesWithOneScope) {
   MemIndex I;
-  I.build(generateSymbols({"a::xyz", "a::yy", "a::xz", "b::yz", "yz"}));
+  I.build(generateSymbols({"a::y1", "a::y2", "a::x", "b::y2", "y3"}));
   FuzzyFindRequest Req;
   Req.Query = "y";
   Req.Scopes = {"a"};
-  EXPECT_THAT(match(I, Req), UnorderedElementsAre("a::xyz", "a::yy"));
+  EXPECT_THAT(match(I, Req), UnorderedElementsAre("a::y1", "a::y2"));
 }
 
 TEST(MemIndexTest, MatchQualifiedNamesWithMultipleScopes) {
   MemIndex I;
-  I.build(generateSymbols({"a::xyz", "a::yy", "a::xz", "b::yz", "yz"}));
+  I.build(generateSymbols({"a::y1", "a::y2", "a::x", "b::y3", "y3"}));
   FuzzyFindRequest Req;
   Req.Query = "y";
   Req.Scopes = {"a", "b"};
-  EXPECT_THAT(match(I, Req), UnorderedElementsAre("a::xyz", "a::yy", "b::yz"));
+  EXPECT_THAT(match(I, Req), UnorderedElementsAre("a::y1", "a::y2", "b::y3"));
 }
 
 TEST(MemIndexTest, NoMatchNestedScopes) {
   MemIndex I;
-  I.build(generateSymbols({"a::xyz", "a::b::yy"}));
+  I.build(generateSymbols({"a::y1", "a::b::y2"}));
   FuzzyFindRequest Req;
   Req.Query = "y";
   Req.Scopes = {"a"};
-  EXPECT_THAT(match(I, Req), UnorderedElementsAre("a::xyz"));
+  EXPECT_THAT(match(I, Req), UnorderedElementsAre("a::y1"));
 }
 
 TEST(MemIndexTest, IgnoreCases) {
Index: unittests/clangd/FuzzyMatchTests.cpp
===
--- unittests/clangd/FuzzyMatchTests.cpp
+++ unittests/clangd/FuzzyMatchTests.cpp
@@ -20,16 +20,27 @@
 using testing::Not;
 
 struct ExpectedMatch {
-  ExpectedMatch(StringRef Annotated) : Word(Annotated), Annotated(Annotated) {
+  // Annotations are optional, and will not be asserted if absent.
+  ExpectedMatch(StringRef Match) : Word(Match), Annotated(Match) {
 for (char C : "[]")
   Word.erase(std::remove(Word.begin(), Word.end(), C), Word.end());
+if (Word.size() == Annotated->size())
+  Annotated = None;
+  }
+  bool accepts(StringRef ActualAnnotated) const {
+return !Annotated || ActualAnnotated == *Annotated;
   }
   std::string Word;
-  StringRef Annotated;
+
+  friend raw_ostream <<(raw_ostream , const ExpectedMatch ) {
+return OS << "'" << M.Word;
+if (M.Annotated)
+  OS << "' as " << *M.Annotated;
+  }
+
+private:
+  Optional Annotated;
 };
-raw_ostream <<(raw_ostream , const ExpectedMatch ) {
-  return OS << "'" << M.Word << "' as " << M.Annotated;
-}
 
 struct MatchesMatcher : public testing::MatcherInterface {
   ExpectedMatch Candidate;
@@ -47,7 +58,7 @@
 FuzzyMatcher Matcher(Pattern);
 auto Result = Matcher.match(Candidate.Word);
 auto AnnotatedMatch = Matcher.dumpLast(*OS << "\n");
-return Result && AnnotatedMatch == Candidate.Annotated;
+return Result && Candidate.accepts(AnnotatedMatch);
   }
 };
 
@@ -122,6 +133,7 @@
   EXPECT_THAT("sl", matches("[S]Visual[L]oggerLogsList"));
   EXPECT_THAT("s", matches("[S]Visua[lL]ogger[L]ogs[L]ist"));
   EXPECT_THAT("Three", matches("H[T]ML[HRE]l[e]ment"));
+  EXPECT_THAT("b", Not(matches("NDEBUG")));
   EXPECT_THAT("Three", matches("[Three]"));
   EXPECT_THAT("fo", Not(matches("barfoo")));
   EXPECT_THAT("fo", matches("bar_[fo]o"));
@@ -151,8 +163,9 @@
   EXPECT_THAT("g", matches("zz[G]roup"));
 
   EXPECT_THAT("", matches("_a_[]")); // Prefer consecutive.
-  EXPECT_THAT("printf", matches("s[printf]"));
-  

[PATCH] D44003: [clangd] Fix unintentionally loose fuzzy matching, and the tests masking it.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
Herald added subscribers: cfe-commits, ioeric, jkorous-apple, ilya-biryukov, 
klimek.
sammccall updated this revision to Diff 136719.
sammccall added a comment.

test tweaks


The intent was that [ar] doesn't match "FooBar"; the first character must match
a Head character (hard requirement, not just a low score).
This matches VSCode, and was "tested" but the tests were defective.

The tests expected matches("FooBar") to fail for lack of a match. But instead
it fails because the string should be annotated - matches("FooB[ar]").
This patch makes matches("FooBar") ignore annotations, as was intended.

Fixing the code to reject weak matches for the first char causes problems:

- [bre] no longer matches "HTMLBRElement". We allow matching against an 
uppercase char even if we don't think it's head. Only do this if there's at 
least one lowercase, to avoid triggering on MACROS
- [print] no longer matches "sprintf". This is hard to fix without false 
positives (e.g. [int] vs "sprintf"]) This patch leaves this case broken. A 
future patch will add a dictionary providing custom segmentation to common 
names from the standard library.

Fixed a couple of index tests that indirectly relied on broken fuzzy matching.
Added const in a couple of missing places for consistency with new code.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44003

Files:
  clangd/FuzzyMatch.cpp
  clangd/FuzzyMatch.h
  unittests/clangd/FuzzyMatchTests.cpp
  unittests/clangd/IndexTests.cpp

Index: unittests/clangd/IndexTests.cpp
===
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -116,14 +116,6 @@
   EXPECT_TRUE(Symbols.expired());
 }
 
-TEST(MemIndexTest, MemIndexMatchSubstring) {
-  MemIndex I;
-  I.build(generateNumSymbols(5, 25));
-  FuzzyFindRequest Req;
-  Req.Query = "5";
-  EXPECT_THAT(match(I, Req), UnorderedElementsAre("5", "15", "25"));
-}
-
 TEST(MemIndexTest, MemIndexDeduplicate) {
   auto Symbols = generateNumSymbols(0, 10);
 
@@ -166,46 +158,46 @@
 
 TEST(MemIndexTest, MatchQualifiedNamesWithoutSpecificScope) {
   MemIndex I;
-  I.build(generateSymbols({"a::xyz", "b::yz", "yz"}));
+  I.build(generateSymbols({"a::y1", "b::y2", "y3"}));
   FuzzyFindRequest Req;
   Req.Query = "y";
-  EXPECT_THAT(match(I, Req), UnorderedElementsAre("a::xyz", "b::yz", "yz"));
+  EXPECT_THAT(match(I, Req), UnorderedElementsAre("a::y1", "b::y2", "y3"));
 }
 
 TEST(MemIndexTest, MatchQualifiedNamesWithGlobalScope) {
   MemIndex I;
-  I.build(generateSymbols({"a::xyz", "b::yz", "yz"}));
+  I.build(generateSymbols({"a::y1", "b::y2", "y3"}));
   FuzzyFindRequest Req;
   Req.Query = "y";
   Req.Scopes = {""};
-  EXPECT_THAT(match(I, Req), UnorderedElementsAre("yz"));
+  EXPECT_THAT(match(I, Req), UnorderedElementsAre("y3"));
 }
 
 TEST(MemIndexTest, MatchQualifiedNamesWithOneScope) {
   MemIndex I;
-  I.build(generateSymbols({"a::xyz", "a::yy", "a::xz", "b::yz", "yz"}));
+  I.build(generateSymbols({"a::y1", "a::y2", "a::x", "b::y2", "y3"}));
   FuzzyFindRequest Req;
   Req.Query = "y";
   Req.Scopes = {"a"};
-  EXPECT_THAT(match(I, Req), UnorderedElementsAre("a::xyz", "a::yy"));
+  EXPECT_THAT(match(I, Req), UnorderedElementsAre("a::y1", "a::y2"));
 }
 
 TEST(MemIndexTest, MatchQualifiedNamesWithMultipleScopes) {
   MemIndex I;
-  I.build(generateSymbols({"a::xyz", "a::yy", "a::xz", "b::yz", "yz"}));
+  I.build(generateSymbols({"a::y1", "a::y2", "a::x", "b::y3", "y3"}));
   FuzzyFindRequest Req;
   Req.Query = "y";
   Req.Scopes = {"a", "b"};
-  EXPECT_THAT(match(I, Req), UnorderedElementsAre("a::xyz", "a::yy", "b::yz"));
+  EXPECT_THAT(match(I, Req), UnorderedElementsAre("a::y1", "a::y2", "b::y3"));
 }
 
 TEST(MemIndexTest, NoMatchNestedScopes) {
   MemIndex I;
-  I.build(generateSymbols({"a::xyz", "a::b::yy"}));
+  I.build(generateSymbols({"a::y1", "a::b::y2"}));
   FuzzyFindRequest Req;
   Req.Query = "y";
   Req.Scopes = {"a"};
-  EXPECT_THAT(match(I, Req), UnorderedElementsAre("a::xyz"));
+  EXPECT_THAT(match(I, Req), UnorderedElementsAre("a::y1"));
 }
 
 TEST(MemIndexTest, IgnoreCases) {
Index: unittests/clangd/FuzzyMatchTests.cpp
===
--- unittests/clangd/FuzzyMatchTests.cpp
+++ unittests/clangd/FuzzyMatchTests.cpp
@@ -20,16 +20,27 @@
 using testing::Not;
 
 struct ExpectedMatch {
-  ExpectedMatch(StringRef Annotated) : Word(Annotated), Annotated(Annotated) {
+  // Annotations are optional, and will not be asserted if absent.
+  ExpectedMatch(StringRef Match) : Word(Match), Annotated(Match) {
 for (char C : "[]")
   Word.erase(std::remove(Word.begin(), Word.end(), C), Word.end());
+if (Word.size() == Annotated->size())
+  Annotated = None;
+  }
+  bool accepts(StringRef ActualAnnotated) const {
+return !Annotated || ActualAnnotated == *Annotated;
   }
   std::string Word;
-  StringRef Annotated;
+
+  friend raw_ostream 

Re: r326542 - [Frontend] Avoid including default system header paths on Fuchsia

[Aaron Ballman via cfe-commits]

On Fri, Mar 2, 2018 at 2:19 AM, Petr Hosek via cfe-commits
 wrote:
> Author: phosek
> Date: Thu Mar  1 23:19:42 2018
> New Revision: 326542
>
> URL: http://llvm.org/viewvc/llvm-project?rev=326542=rev
> Log:
> [Frontend] Avoid including default system header paths on Fuchsia
>
> These paths aren't used and don't make sense on Fuchsia.
>
> Differential Revision: https://reviews.llvm.org/D43992
>
> Modified:
> cfe/trunk/lib/Frontend/InitHeaderSearch.cpp

Please add test cases to verify the behavior and help ensure we don't
regress it later.

~Aaron

>
> Modified: cfe/trunk/lib/Frontend/InitHeaderSearch.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/InitHeaderSearch.cpp?rev=326542=326541=326542=diff
> ==
> --- cfe/trunk/lib/Frontend/InitHeaderSearch.cpp (original)
> +++ cfe/trunk/lib/Frontend/InitHeaderSearch.cpp Thu Mar  1 23:19:42 2018
> @@ -216,6 +216,7 @@ void InitHeaderSearch::AddDefaultCInclud
>  case llvm::Triple::NaCl:
>  case llvm::Triple::PS4:
>  case llvm::Triple::ELFIAMCU:
> +case llvm::Triple::Fuchsia:
>break;
>  case llvm::Triple::Win32:
>if (triple.getEnvironment() != llvm::Triple::Cygnus)
> @@ -322,6 +323,7 @@ void InitHeaderSearch::AddDefaultCInclud
>case llvm::Triple::RTEMS:
>case llvm::Triple::NaCl:
>case llvm::Triple::ELFIAMCU:
> +  case llvm::Triple::Fuchsia:
>  break;
>case llvm::Triple::PS4: {
>  //  gets prepended later in AddPath().
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44000: [clang] Fix use-after-free on code completion

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL326569: [clang] Fix use-after-free on code completion 
(authored by ibiryukov, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D44000

Files:
  clang-tools-extra/trunk/clangd/CodeComplete.cpp

Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp
===
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp
@@ -429,13 +429,19 @@
 // It filters out ignored results (but doesn't apply fuzzy-filtering yet).
 // It doesn't do scoring or conversion to CompletionItem yet, as we want to
 // merge with index results first.
+// Generally the fields and methods of this object should only be used from
+// within the callback.
 struct CompletionRecorder : public CodeCompleteConsumer {
-  CompletionRecorder(const CodeCompleteOptions )
+  CompletionRecorder(const CodeCompleteOptions ,
+ UniqueFunction ResultsCallback)
   : CodeCompleteConsumer(Opts.getClangCompleteOpts(),
  /*OutputIsBinary=*/false),
 CCContext(CodeCompletionContext::CCC_Other), Opts(Opts),
 CCAllocator(std::make_shared()),
-CCTUInfo(CCAllocator) {}
+CCTUInfo(CCAllocator), ResultsCallback(std::move(ResultsCallback)) {
+assert(this->ResultsCallback);
+  }
+
   std::vector Results;
   CodeCompletionContext CCContext;
   Sema *CCSema = nullptr; // Sema that created the results.
@@ -466,6 +472,7 @@
 continue;
   Results.push_back(Result);
 }
+ResultsCallback();
   }
 
   CodeCompletionAllocator () override { return *CCAllocator; }
@@ -503,6 +510,7 @@
   CodeCompleteOptions Opts;
   std::shared_ptr CCAllocator;
   CodeCompletionTUInfo CCTUInfo;
+  UniqueFunction ResultsCallback;
 };
 
 // Tracks a bounded number of candidates with the best scores.
@@ -657,12 +665,10 @@
 };
 
 // Invokes Sema code completion on a file.
-// Callback will be invoked once completion is done, but before cleaning up.
 bool semaCodeComplete(std::unique_ptr Consumer,
   const clang::CodeCompleteOptions ,
-  const SemaCompleteInput ,
-  llvm::function_ref Callback = nullptr) {
-  auto Tracer = llvm::make_unique("Sema completion");
+  const SemaCompleteInput ) {
+  trace::Span Tracer("Sema completion");
   std::vector ArgStrs;
   for (const auto  : Input.Command.CommandLine)
 ArgStrs.push_back(S.c_str());
@@ -729,12 +735,6 @@
 log("Execute() failed when running codeComplete for " + Input.FileName);
 return false;
   }
-  Tracer.reset();
-
-  if (Callback)
-Callback();
-
-  Tracer = llvm::make_unique("Sema completion cleanup");
   Action.EndSourceFile();
 
   return true;
@@ -813,7 +813,7 @@
 // other arenas, which must stay alive for us to build CompletionItems.
 //   - we may get duplicate results from Sema and the Index, we need to merge.
 //
-// So we start Sema completion first, but defer its cleanup until we're done.
+// So we start Sema completion first, and do all our work in its callback.
 // We use the Sema context information to query the index.
 // Then we merge the two result sets, producing items that are Sema/Index/Both.
 // These items are scored, and the top N are synthesized into the LSP response.
@@ -834,34 +834,32 @@
   PathRef FileName;
   const CodeCompleteOptions 
   // Sema takes ownership of Recorder. Recorder is valid until Sema cleanup.
-  std::unique_ptr RecorderOwner;
-  CompletionRecorder 
+  CompletionRecorder *Recorder = nullptr;
   int NSema = 0, NIndex = 0, NBoth = 0; // Counters for logging.
   bool Incomplete = false; // Would more be available with a higher limit?
   llvm::Optional Filter; // Initialized once Sema runs.
 
 public:
   // A CodeCompleteFlow object is only useful for calling run() exactly once.
   CodeCompleteFlow(PathRef FileName, const CodeCompleteOptions )
-  : FileName(FileName), Opts(Opts),
-RecorderOwner(new CompletionRecorder(Opts)), Recorder(*RecorderOwner) {}
+  : FileName(FileName), Opts(Opts) {}
 
   CompletionList run(const SemaCompleteInput ) && {
 trace::Span Tracer("CodeCompleteFlow");
 // We run Sema code completion first. It builds an AST and calculates:
-//   - completion results based on the AST. These are saved for merging.
+//   - completion results based on the AST.
 //   - partial identifier and context. We need these for the index query.
 CompletionList Output;
+auto RecorderOwner = llvm::make_unique(Opts, [&]() {
+  assert(Recorder && "Recorder is not set");
+  Output = runWithSema();
+  SPAN_ATTACH(Tracer, "sema_completion_kind",
+  getCompletionKindString(Recorder->CCContext.getKind()));
+});
+
+Recorder = RecorderOwner.get();
 

[PATCH] D42787: clang-format: do not add extra indent when wrapping last parameter

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc added a comment.

In https://reviews.llvm.org/D42787#1000687, @krasimir wrote:

> We could adapt the single-argument version instead, turning:
>
>   foo(bb +
>   c);
>
>
> into:
>
>   foo(bb +
>   c);
>


I have a mild preference for this, and it seems more consistent. But I don't 
have a strong preference.

However, for each of the other cases shown in the thread so far, i strongly 
prefer clang-format's current behavior.


Repository:
  rC Clang

https://reviews.llvm.org/D42787



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

[clang-tools-extra] r326569 - [clang] Fix use-after-free on code completion

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Fri Mar  2 04:28:27 2018
New Revision: 326569

URL: http://llvm.org/viewvc/llvm-project?rev=326569=rev
Log:
[clang] Fix use-after-free on code completion

Summary:
Found by asan. Fiddling with code completion AST after
FrontendAction::Exceute can lead to errors.
Calling the callback in ProcessCodeCompleteResults to make sure we
don't access uninitialized state.

This particular issue comes from the fact that Sema::TUScope is
deleted when destructor of ~Parser runs, but still present in
Sema::TUScope and accessed when building completion items.

I'm still struggling to come up with a small repro. The relevant
stackframes reported by asan are:
ERROR: AddressSanitizer: heap-use-after-free on address
READ of size 8 at 0x61400020d090 thread T175
#0 0x5632dff7821b in llvm::SmallPtrSetImplBase::isSmall() const 
include/llvm/ADT/SmallPtrSet.h:195:33
#1 0x5632e0335901 in llvm::SmallPtrSetImplBase::insert_imp(void const*) 
include/llvm/ADT/SmallPtrSet.h:127:9
#2 0x5632e067347d in 
llvm::SmallPtrSetImpl::insert(clang::Decl*) 
include/llvm/ADT/SmallPtrSet.h:372:14
#3 0x5632e065df80 in clang::Scope::AddDecl(clang::Decl*) 
tools/clang/include/clang/Sema/Scope.h:287:18
#4 0x5632e0623eea in 
clang::ASTReader::pushExternalDeclIntoScope(clang::NamedDecl*, 
clang::DeclarationName) clang/lib/Serialization/ASTReader.cpp
#5 0x5632e062ce74 in clang::ASTReader::finishPendingActions() 
tools/clang/lib/Serialization/ASTReader.cpp:9164:9

#30 0x5632e02009c4 in clang::index::generateUSRForDecl(clang::Decl const*, 
llvm::SmallVectorImpl&) tools/clang/lib/Index/USRGeneration.cpp:1037:6
#31 0x5632dff73eab in clang::clangd::(anonymous 
namespace)::getSymbolID(clang::CodeCompletionResult const&) 
tools/clang/tools/extra/clangd/CodeComplete.cpp:326:20
#32 0x5632dff6fe91 in 
clang::clangd::CodeCompleteFlow::mergeResults(std::vector const&, 
clang::clangd::SymbolSlab const&)::'lambda'(clang::CodeCompletionResult 
const&)::operator()(clang::CodeCompletionResult const&) 
tools/clang/tools/extra/clangd/CodeComplete.cpp:938:24
#33 0x5632dff6e426 in 
clang::clangd::CodeCompleteFlow::mergeResults(std::vector const&, 
clang::clangd::SymbolSlab const&) 
third_party/llvm/llvm/tools/clang/tools/extra/clangd/CodeComplete.cpp:949:38
#34 0x5632dff7a34d in clang::clangd::CodeCompleteFlow::runWithSema() 
llvm/tools/clang/tools/extra/clangd/CodeComplete.cpp:894:16
#35 0x5632dff6df6a in 
clang::clangd::CodeCompleteFlow::run(clang::clangd::(anonymous 
namespace)::SemaCompleteInput const&) &&::'lambda'()::operator()() const 
third_party/llvm/llvm/tools/clang/tools/extra/clangd/CodeComplete.cpp:858:35
#36 0x5632dff6cd42 in clang::clangd::(anonymous 
namespace)::semaCodeComplete(std::unique_ptr, clang::CodeCompleteOptions 
const&, clang::clangd::(anonymous namespace)::SemaCompleteInput const&, 
llvm::function_ref) 
tools/clang/tools/extra/clangd/CodeComplete.cpp:735:5

0x61400020d090 is located 80 bytes inside of 432-byte region 
[0x61400020d040,0x61400020d1f0)
freed by thread T175  here:
#0 0x5632df74e115 in operator delete(void*, unsigned long) 
projects/compiler-rt/lib/asan/asan_new_delete.cc:161:3
#1 0x5632e0b06973 in clang::Parser::~Parser() 
tools/clang/lib/Parse/Parser.cpp:410:3
#2 0x5632e0b06ddd in clang::Parser::~Parser() 
clang/lib/Parse/Parser.cpp:408:19
#3 0x5632e0b03286 in std::unique_ptr::~unique_ptr() .../bits/unique_ptr.h:236:4
#4 0x5632e0b021c4 in clang::ParseAST(clang::Sema&, bool, bool) 
tools/clang/lib/Parse/ParseAST.cpp:182:1
#5 0x5632e0726544 in clang::FrontendAction::Execute() 
tools/clang/lib/Frontend/FrontendAction.cpp:904:8
#6 0x5632dff6cd05 in clang::clangd::(anonymous 
namespace)::semaCodeComplete(std::unique_ptr, clang::CodeCompleteOptions 
const&, clang::clangd::(anonymous namespace)::SemaCompleteInput const&, 
llvm::function_ref) 
tools/clang/tools/extra/clangd/CodeComplete.cpp:728:15

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: klimek, jkorous-apple, cfe-commits, ioeric

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

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

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=326569=326568=326569=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Fri Mar  2 04:28:27 2018
@@ -429,13 +429,19 @@ std::vector getQueryScopes(
 // It filters out ignored results (but doesn't apply fuzzy-filtering yet).
 // It doesn't do scoring or conversion to CompletionItem 

[PATCH] D43809: Add possibility to specify output stream for CompilerInstance

[Alexey Sotkin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC326566: Add possibility to specify output stream for 
CompilerInstance (authored by AlexeySotkin, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D43809?vs=136252=136715#toc

Repository:
  rC Clang

https://reviews.llvm.org/D43809

Files:
  include/clang/Frontend/CompilerInstance.h
  lib/CodeGen/CodeGenAction.cpp
  unittests/Frontend/CMakeLists.txt
  unittests/Frontend/OutputStreamTest.cpp

Index: lib/CodeGen/CodeGenAction.cpp
===
--- lib/CodeGen/CodeGenAction.cpp
+++ lib/CodeGen/CodeGenAction.cpp
@@ -846,7 +846,10 @@
 std::unique_ptr
 CodeGenAction::CreateASTConsumer(CompilerInstance , StringRef InFile) {
   BackendAction BA = static_cast(Act);
-  std::unique_ptr OS = GetOutputStream(CI, InFile, BA);
+  std::unique_ptr OS = CI.takeOutputStream();
+  if (!OS)
+OS = GetOutputStream(CI, InFile, BA);
+
   if (BA != Backend_EmitNothing && !OS)
 return nullptr;
 
Index: unittests/Frontend/OutputStreamTest.cpp
===
--- unittests/Frontend/OutputStreamTest.cpp
+++ unittests/Frontend/OutputStreamTest.cpp
@@ -0,0 +1,46 @@
+//===- unittests/Frontend/OutputStreamTest.cpp --- FrontendAction tests --===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/CodeGen/BackendUtil.h"
+#include "clang/CodeGen/CodeGenAction.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/FrontendTool/Utils.h"
+#include "clang/Lex/PreprocessorOptions.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace clang;
+using namespace clang::frontend;
+
+namespace {
+
+TEST(FrontendOutputTests, TestOutputStream) {
+  auto Invocation = std::make_shared();
+  Invocation->getPreprocessorOpts().addRemappedFile(
+  "test.cc", MemoryBuffer::getMemBuffer("").release());
+  Invocation->getFrontendOpts().Inputs.push_back(
+  FrontendInputFile("test.cc", InputKind::CXX));
+  Invocation->getFrontendOpts().ProgramAction = EmitBC;
+  Invocation->getTargetOpts().Triple = "i386-unknown-linux-gnu";
+  CompilerInstance Compiler;
+
+  SmallVector IRBuffer;
+  std::unique_ptr IRStream(
+  new raw_svector_ostream(IRBuffer));
+
+  Compiler.setOutputStream(std::move(IRStream));
+  Compiler.setInvocation(std::move(Invocation));
+  Compiler.createDiagnostics();
+
+  bool Success = ExecuteCompilerInvocation();
+  EXPECT_TRUE(Success);
+  EXPECT_TRUE(!IRBuffer.empty());
+  EXPECT_TRUE(StringRef(IRBuffer.data()).startswith("BC"));
+}
+}
Index: unittests/Frontend/CMakeLists.txt
===
--- unittests/Frontend/CMakeLists.txt
+++ unittests/Frontend/CMakeLists.txt
@@ -9,6 +9,7 @@
   CodeGenActionTest.cpp
   ParsedSourceLocationTest.cpp
   PCHPreambleTest.cpp
+  OutputStreamTest.cpp
   )
 target_link_libraries(FrontendTests
   PRIVATE
@@ -18,4 +19,5 @@
   clangLex
   clangSema
   clangCodeGen
+  clangFrontendTool
   )
Index: include/clang/Frontend/CompilerInstance.h
===
--- include/clang/Frontend/CompilerInstance.h
+++ include/clang/Frontend/CompilerInstance.h
@@ -183,6 +183,9 @@
   /// The list of active output files.
   std::list OutputFiles;
 
+  /// Force an output buffer.
+  std::unique_ptr OutputStream;
+
   CompilerInstance(const CompilerInstance &) = delete;
   void operator=(const CompilerInstance &) = delete;
 public:
@@ -773,6 +776,14 @@
 
   /// }
 
+  void setOutputStream(std::unique_ptr OutStream) {
+OutputStream = std::move(OutStream);
+  }
+
+  std::unique_ptr takeOutputStream() {
+return std::move(OutputStream);
+  }
+
   // Create module manager.
   void createModuleManager();
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44000: [clang] Fix use-after-free on code completion

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 136714.
ilya-biryukov marked 7 inline comments as done.
ilya-biryukov added a comment.

Addressed review comments


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44000

Files:
  clangd/CodeComplete.cpp

Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -429,13 +429,19 @@
 // It filters out ignored results (but doesn't apply fuzzy-filtering yet).
 // It doesn't do scoring or conversion to CompletionItem yet, as we want to
 // merge with index results first.
+// Generally the fields and methods of this object should only be used from
+// within the callback.
 struct CompletionRecorder : public CodeCompleteConsumer {
-  CompletionRecorder(const CodeCompleteOptions )
+  CompletionRecorder(const CodeCompleteOptions ,
+ UniqueFunction ResultsCallback)
   : CodeCompleteConsumer(Opts.getClangCompleteOpts(),
  /*OutputIsBinary=*/false),
 CCContext(CodeCompletionContext::CCC_Other), Opts(Opts),
 CCAllocator(std::make_shared()),
-CCTUInfo(CCAllocator) {}
+CCTUInfo(CCAllocator), ResultsCallback(std::move(ResultsCallback)) {
+assert(this->ResultsCallback);
+  }
+
   std::vector Results;
   CodeCompletionContext CCContext;
   Sema *CCSema = nullptr; // Sema that created the results.
@@ -466,6 +472,7 @@
 continue;
   Results.push_back(Result);
 }
+ResultsCallback();
   }
 
   CodeCompletionAllocator () override { return *CCAllocator; }
@@ -503,6 +510,7 @@
   CodeCompleteOptions Opts;
   std::shared_ptr CCAllocator;
   CodeCompletionTUInfo CCTUInfo;
+  UniqueFunction ResultsCallback;
 };
 
 // Tracks a bounded number of candidates with the best scores.
@@ -657,12 +665,10 @@
 };
 
 // Invokes Sema code completion on a file.
-// Callback will be invoked once completion is done, but before cleaning up.
 bool semaCodeComplete(std::unique_ptr Consumer,
   const clang::CodeCompleteOptions ,
-  const SemaCompleteInput ,
-  llvm::function_ref Callback = nullptr) {
-  auto Tracer = llvm::make_unique("Sema completion");
+  const SemaCompleteInput ) {
+  trace::Span Tracer("Sema completion");
   std::vector ArgStrs;
   for (const auto  : Input.Command.CommandLine)
 ArgStrs.push_back(S.c_str());
@@ -729,12 +735,6 @@
 log("Execute() failed when running codeComplete for " + Input.FileName);
 return false;
   }
-  Tracer.reset();
-
-  if (Callback)
-Callback();
-
-  Tracer = llvm::make_unique("Sema completion cleanup");
   Action.EndSourceFile();
 
   return true;
@@ -813,7 +813,7 @@
 // other arenas, which must stay alive for us to build CompletionItems.
 //   - we may get duplicate results from Sema and the Index, we need to merge.
 //
-// So we start Sema completion first, but defer its cleanup until we're done.
+// So we start Sema completion first, and do all our work in its callback.
 // We use the Sema context information to query the index.
 // Then we merge the two result sets, producing items that are Sema/Index/Both.
 // These items are scored, and the top N are synthesized into the LSP response.
@@ -834,34 +834,32 @@
   PathRef FileName;
   const CodeCompleteOptions 
   // Sema takes ownership of Recorder. Recorder is valid until Sema cleanup.
-  std::unique_ptr RecorderOwner;
-  CompletionRecorder 
+  CompletionRecorder *Recorder = nullptr;
   int NSema = 0, NIndex = 0, NBoth = 0; // Counters for logging.
   bool Incomplete = false; // Would more be available with a higher limit?
   llvm::Optional Filter; // Initialized once Sema runs.
 
 public:
   // A CodeCompleteFlow object is only useful for calling run() exactly once.
   CodeCompleteFlow(PathRef FileName, const CodeCompleteOptions )
-  : FileName(FileName), Opts(Opts),
-RecorderOwner(new CompletionRecorder(Opts)), Recorder(*RecorderOwner) {}
+  : FileName(FileName), Opts(Opts) {}
 
   CompletionList run(const SemaCompleteInput ) && {
 trace::Span Tracer("CodeCompleteFlow");
 // We run Sema code completion first. It builds an AST and calculates:
-//   - completion results based on the AST. These are saved for merging.
+//   - completion results based on the AST.
 //   - partial identifier and context. We need these for the index query.
 CompletionList Output;
+auto RecorderOwner = llvm::make_unique(Opts, [&]() {
+  assert(Recorder && "Recorder is not set");
+  Output = runWithSema();
+  SPAN_ATTACH(Tracer, "sema_completion_kind",
+  getCompletionKindString(Recorder->CCContext.getKind()));
+});
+
+Recorder = RecorderOwner.get();
 semaCodeComplete(std::move(RecorderOwner), Opts.getClangCompleteOpts(),
- SemaCCInput, [&] {
-   if (Recorder.CCSema) {
- 

[PATCH] D42787: clang-format: do not add extra indent when wrapping last parameter

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

If people don't care about this case, we might as well merge this :-) Just 
kidding.

The tweak matches both our expectation, the auto-indent behaviour of IDE (not 
to be trusted, but still probably of 'default' behaviour for many people, esp. 
when you don't yet use a formatter), and its seems our code base is generally 
formatted like this. I think it is quite more frequent than 50 times in whole 
LLVM/Clang, but I have no actual numbers; do you have a magic tool to search 
for such specific "code pattern", or just run clang-format with one option then 
the next and count the differences ?


Repository:
  rC Clang

https://reviews.llvm.org/D42787



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

[libunwind] r326565 - Creating release candidate final from release_600 branch

[Hans Wennborg via cfe-commits]

Author: hans
Date: Fri Mar  2 04:08:51 2018
New Revision: 326565

URL: http://llvm.org/viewvc/llvm-project?rev=326565=rev
Log:
Creating release candidate final from release_600 branch

Added:
libunwind/tags/RELEASE_600/final/
  - copied from r326564, libunwind/branches/release_60/

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

[libcxx] r326558 - Creating release candidate final from release_600 branch

[Hans Wennborg via cfe-commits]

Author: hans
Date: Fri Mar  2 04:07:59 2018
New Revision: 326558

URL: http://llvm.org/viewvc/llvm-project?rev=326558=rev
Log:
Creating release candidate final from release_600 branch

Added:
libcxx/tags/RELEASE_600/final/   (props changed)
  - copied from r326557, libcxx/branches/release_60/

Propchange: libcxx/tags/RELEASE_600/final/
--
--- svn:mergeinfo (added)
+++ svn:mergeinfo Fri Mar  2 04:07:59 2018
@@ -0,0 +1,2 @@
+/libcxx/branches/apple:136569-137939
+/libcxx/trunk:321963,324153,324855,325147


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

[libcxxabi] r326559 - Creating release candidate final from release_600 branch

[Hans Wennborg via cfe-commits]

Author: hans
Date: Fri Mar  2 04:08:05 2018
New Revision: 326559

URL: http://llvm.org/viewvc/llvm-project?rev=326559=rev
Log:
Creating release candidate final from release_600 branch

Added:
libcxxabi/tags/RELEASE_600/final/
  - copied from r326558, libcxxabi/branches/release_60/

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

[PATCH] D44000: [clang] Fix use-after-free on code completion

[Sam McCall via Phabricator via cfe-commits]

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

This feels like a bug in the underlying clang libraries, but since none of the 
lifetimes are documented and everyone just does it this way...




Comment at: clangd/CodeComplete.cpp:431
 // It doesn't do scoring or conversion to CompletionItem yet, as we want to
 // merge with index results first.
 struct CompletionRecorder : public CodeCompleteConsumer {

maybe comment:  'generally the fields/methods should only be used from within 
the callback'



Comment at: clangd/CodeComplete.cpp:470
 }
+if (ResultsCallback)
+  ResultsCallback();

I think this check is dead - remove?



Comment at: clangd/CodeComplete.cpp:660
 // Invokes Sema code completion on a file.
 // Callback will be invoked once completion is done, but before cleaning up.
 bool semaCodeComplete(std::unique_ptr Consumer,

remove this comment



Comment at: clangd/CodeComplete.cpp:665
-  llvm::function_ref Callback = nullptr) {
+  const SemaCompleteInput ) {
   auto Tracer = llvm::make_unique("Sema completion");
   std::vector ArgStrs;

this can be stack-allocated, as we don't need to reset it



Comment at: clangd/CodeComplete.cpp:737
-
   Tracer = llvm::make_unique("Sema completion cleanup");
   Action.EndSourceFile();

this tracer isn't needed anymore as discussed



Comment at: clangd/CodeComplete.cpp:816
 //
 // So we start Sema completion first, but defer its cleanup until we're done.
 // We use the Sema context information to query the index.

this comment should probably be ", and do all our work in its callback"



Comment at: clangd/CodeComplete.cpp:852
 // We run Sema code completion first. It builds an AST and calculates:
 //   - completion results based on the AST. These are saved for merging.
 //   - partial identifier and context. We need these for the index query.

remove "these are saved for merging"



Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44000



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

[PATCH] D44000: [clang] Fix use-after-free on code completion

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 136710.
ilya-biryukov added a comment.

Remove a trace for "sema cleanup", it is not very informative now that we run 
the callback under the "sema completion" trace


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44000

Files:
  clangd/CodeComplete.cpp

Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -430,12 +430,13 @@
 // It doesn't do scoring or conversion to CompletionItem yet, as we want to
 // merge with index results first.
 struct CompletionRecorder : public CodeCompleteConsumer {
-  CompletionRecorder(const CodeCompleteOptions )
+  CompletionRecorder(const CodeCompleteOptions ,
+ UniqueFunction ResultsCallback)
   : CodeCompleteConsumer(Opts.getClangCompleteOpts(),
  /*OutputIsBinary=*/false),
 CCContext(CodeCompletionContext::CCC_Other), Opts(Opts),
 CCAllocator(std::make_shared()),
-CCTUInfo(CCAllocator) {}
+CCTUInfo(CCAllocator), ResultsCallback(std::move(ResultsCallback)) {}
   std::vector Results;
   CodeCompletionContext CCContext;
   Sema *CCSema = nullptr; // Sema that created the results.
@@ -466,6 +467,8 @@
 continue;
   Results.push_back(Result);
 }
+if (ResultsCallback)
+  ResultsCallback();
   }
 
   CodeCompletionAllocator () override { return *CCAllocator; }
@@ -503,6 +506,7 @@
   CodeCompleteOptions Opts;
   std::shared_ptr CCAllocator;
   CodeCompletionTUInfo CCTUInfo;
+  UniqueFunction ResultsCallback;
 };
 
 // Tracks a bounded number of candidates with the best scores.
@@ -660,9 +664,8 @@
 // Callback will be invoked once completion is done, but before cleaning up.
 bool semaCodeComplete(std::unique_ptr Consumer,
   const clang::CodeCompleteOptions ,
-  const SemaCompleteInput ,
-  llvm::function_ref Callback = nullptr) {
-  auto Tracer = llvm::make_unique("Sema completion");
+  const SemaCompleteInput ) {
+  trace::Span Tracer("Sema completion");
   std::vector ArgStrs;
   for (const auto  : Input.Command.CommandLine)
 ArgStrs.push_back(S.c_str());
@@ -729,12 +732,6 @@
 log("Execute() failed when running codeComplete for " + Input.FileName);
 return false;
   }
-  Tracer.reset();
-
-  if (Callback)
-Callback();
-
-  Tracer = llvm::make_unique("Sema completion cleanup");
   Action.EndSourceFile();
 
   return true;
@@ -834,34 +831,32 @@
   PathRef FileName;
   const CodeCompleteOptions 
   // Sema takes ownership of Recorder. Recorder is valid until Sema cleanup.
-  std::unique_ptr RecorderOwner;
-  CompletionRecorder 
+  CompletionRecorder *Recorder = nullptr;
   int NSema = 0, NIndex = 0, NBoth = 0; // Counters for logging.
   bool Incomplete = false; // Would more be available with a higher limit?
   llvm::Optional Filter; // Initialized once Sema runs.
 
 public:
   // A CodeCompleteFlow object is only useful for calling run() exactly once.
   CodeCompleteFlow(PathRef FileName, const CodeCompleteOptions )
-  : FileName(FileName), Opts(Opts),
-RecorderOwner(new CompletionRecorder(Opts)), Recorder(*RecorderOwner) {}
+  : FileName(FileName), Opts(Opts) {}
 
   CompletionList run(const SemaCompleteInput ) && {
 trace::Span Tracer("CodeCompleteFlow");
 // We run Sema code completion first. It builds an AST and calculates:
 //   - completion results based on the AST. These are saved for merging.
 //   - partial identifier and context. We need these for the index query.
 CompletionList Output;
+auto RecorderOwner = llvm::make_unique(Opts, [&]() {
+  assert(Recorder && "Recorder is not set");
+  Output = runWithSema();
+  SPAN_ATTACH(Tracer, "sema_completion_kind",
+  getCompletionKindString(Recorder->CCContext.getKind()));
+});
+
+Recorder = RecorderOwner.get();
 semaCodeComplete(std::move(RecorderOwner), Opts.getClangCompleteOpts(),
- SemaCCInput, [&] {
-   if (Recorder.CCSema) {
- Output = runWithSema();
- SPAN_ATTACH(
- Tracer, "sema_completion_kind",
- getCompletionKindString(Recorder.CCContext.getKind()));
-   } else
- log("Code complete: no Sema callback, 0 results");
- });
+ SemaCCInput);
 
 SPAN_ATTACH(Tracer, "sema_results", NSema);
 SPAN_ATTACH(Tracer, "index_results", NIndex);
@@ -883,15 +878,15 @@
   // Sema data structures are torn down. It does all the real work.
   CompletionList runWithSema() {
 Filter = FuzzyMatcher(
-Recorder.CCSema->getPreprocessor().getCodeCompletionFilter());
+

[PATCH] D44000: [clang] Fix use-after-free on code completion

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 136707.
ilya-biryukov added a comment.

Remove code refactoring from the patch.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44000

Files:
  clangd/CodeComplete.cpp

Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -430,12 +430,13 @@
 // It doesn't do scoring or conversion to CompletionItem yet, as we want to
 // merge with index results first.
 struct CompletionRecorder : public CodeCompleteConsumer {
-  CompletionRecorder(const CodeCompleteOptions )
+  CompletionRecorder(const CodeCompleteOptions ,
+ UniqueFunction ResultsCallback)
   : CodeCompleteConsumer(Opts.getClangCompleteOpts(),
  /*OutputIsBinary=*/false),
 CCContext(CodeCompletionContext::CCC_Other), Opts(Opts),
 CCAllocator(std::make_shared()),
-CCTUInfo(CCAllocator) {}
+CCTUInfo(CCAllocator), ResultsCallback(std::move(ResultsCallback)) {}
   std::vector Results;
   CodeCompletionContext CCContext;
   Sema *CCSema = nullptr; // Sema that created the results.
@@ -466,6 +467,8 @@
 continue;
   Results.push_back(Result);
 }
+if (ResultsCallback)
+  ResultsCallback();
   }
 
   CodeCompletionAllocator () override { return *CCAllocator; }
@@ -503,6 +506,7 @@
   CodeCompleteOptions Opts;
   std::shared_ptr CCAllocator;
   CodeCompletionTUInfo CCTUInfo;
+  UniqueFunction ResultsCallback;
 };
 
 // Tracks a bounded number of candidates with the best scores.
@@ -660,8 +664,7 @@
 // Callback will be invoked once completion is done, but before cleaning up.
 bool semaCodeComplete(std::unique_ptr Consumer,
   const clang::CodeCompleteOptions ,
-  const SemaCompleteInput ,
-  llvm::function_ref Callback = nullptr) {
+  const SemaCompleteInput ) {
   auto Tracer = llvm::make_unique("Sema completion");
   std::vector ArgStrs;
   for (const auto  : Input.Command.CommandLine)
@@ -729,11 +732,6 @@
 log("Execute() failed when running codeComplete for " + Input.FileName);
 return false;
   }
-  Tracer.reset();
-
-  if (Callback)
-Callback();
-
   Tracer = llvm::make_unique("Sema completion cleanup");
   Action.EndSourceFile();
 
@@ -834,34 +832,32 @@
   PathRef FileName;
   const CodeCompleteOptions 
   // Sema takes ownership of Recorder. Recorder is valid until Sema cleanup.
-  std::unique_ptr RecorderOwner;
-  CompletionRecorder 
+  CompletionRecorder *Recorder = nullptr;
   int NSema = 0, NIndex = 0, NBoth = 0; // Counters for logging.
   bool Incomplete = false; // Would more be available with a higher limit?
   llvm::Optional Filter; // Initialized once Sema runs.
 
 public:
   // A CodeCompleteFlow object is only useful for calling run() exactly once.
   CodeCompleteFlow(PathRef FileName, const CodeCompleteOptions )
-  : FileName(FileName), Opts(Opts),
-RecorderOwner(new CompletionRecorder(Opts)), Recorder(*RecorderOwner) {}
+  : FileName(FileName), Opts(Opts) {}
 
   CompletionList run(const SemaCompleteInput ) && {
 trace::Span Tracer("CodeCompleteFlow");
 // We run Sema code completion first. It builds an AST and calculates:
 //   - completion results based on the AST. These are saved for merging.
 //   - partial identifier and context. We need these for the index query.
 CompletionList Output;
+auto RecorderOwner = llvm::make_unique(Opts, [&]() {
+  assert(Recorder && "Recorder is not set");
+  Output = runWithSema();
+  SPAN_ATTACH(Tracer, "sema_completion_kind",
+  getCompletionKindString(Recorder->CCContext.getKind()));
+});
+
+Recorder = RecorderOwner.get();
 semaCodeComplete(std::move(RecorderOwner), Opts.getClangCompleteOpts(),
- SemaCCInput, [&] {
-   if (Recorder.CCSema) {
- Output = runWithSema();
- SPAN_ATTACH(
- Tracer, "sema_completion_kind",
- getCompletionKindString(Recorder.CCContext.getKind()));
-   } else
- log("Code complete: no Sema callback, 0 results");
- });
+ SemaCCInput);
 
 SPAN_ATTACH(Tracer, "sema_results", NSema);
 SPAN_ATTACH(Tracer, "index_results", NIndex);
@@ -883,15 +879,15 @@
   // Sema data structures are torn down. It does all the real work.
   CompletionList runWithSema() {
 Filter = FuzzyMatcher(
-Recorder.CCSema->getPreprocessor().getCodeCompletionFilter());
+Recorder->CCSema->getPreprocessor().getCodeCompletionFilter());
 // Sema provides the needed context to query the index.
 // FIXME: in addition to querying for extra/overlapping symbols, we should
 //explicitly request 

[PATCH] D43741: [Analyzer] More accurate modeling about the increment operator of the operand with type bool.

[Henry Wong via Phabricator via cfe-commits]

MTC added a comment.

In https://reviews.llvm.org/D43741#1024949, @alexshap wrote:

> i see, but just in case - what about the decrement operator ?


@alexshap, If I'm not wrong, decrement operation is not allowed on bool type in 
C++98 and C++14.

> 5.2.6 Increment and decrement [expr.post.incr]
>  The operand of postfix -- is decremented analogously to the postfix ++ 
> operator, except that the operand
>  shall not be of type bool. [ Note: For prefix increment and decrement, see 
> 5.3.2. — end note ]




Repository:
  rC Clang

https://reviews.llvm.org/D43741



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

[PATCH] D44000: [clang] Fix use-after-free on code completion

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: sammccall.
Herald added subscribers: ioeric, jkorous-apple, klimek.

Found by asan. Fiddling with code completion AST after
FrontendAction::Exceute can lead to errors.
Calling the callback in ProcessCodeCompleteResults to make sure we
don't access uninitialized state.

This particular issue comes from the fact that Sema::TUScope is
deleted when destructor of ~Parser runs, but still present in
Sema::TUScope and accessed when building completion items.

I'm still struggling to come up with a small repro. The relevant
stackframes reported by asan are:
ERROR: AddressSanitizer: heap-use-after-free on address
READ of size 8 at 0x61400020d090 thread T175

  #0 0x5632dff7821b in llvm::SmallPtrSetImplBase::isSmall() const 
include/llvm/ADT/SmallPtrSet.h:195:33
  #1 0x5632e0335901 in llvm::SmallPtrSetImplBase::insert_imp(void const*) 
include/llvm/ADT/SmallPtrSet.h:127:9
  #2 0x5632e067347d in 
llvm::SmallPtrSetImpl::insert(clang::Decl*) 
include/llvm/ADT/SmallPtrSet.h:372:14
  #3 0x5632e065df80 in clang::Scope::AddDecl(clang::Decl*) 
tools/clang/include/clang/Sema/Scope.h:287:18
  #4 0x5632e0623eea in 
clang::ASTReader::pushExternalDeclIntoScope(clang::NamedDecl*, 
clang::DeclarationName) clang/lib/Serialization/ASTReader.cpp
  #5 0x5632e062ce74 in clang::ASTReader::finishPendingActions() 
tools/clang/lib/Serialization/ASTReader.cpp:9164:9
  
  #30 0x5632e02009c4 in clang::index::generateUSRForDecl(clang::Decl const*, 
llvm::SmallVectorImpl&) tools/clang/lib/Index/USRGeneration.cpp:1037:6
  #31 0x5632dff73eab in clang::clangd::(anonymous 
namespace)::getSymbolID(clang::CodeCompletionResult const&) 
tools/clang/tools/extra/clangd/CodeComplete.cpp:326:20
  #32 0x5632dff6fe91 in 
clang::clangd::CodeCompleteFlow::mergeResults(std::vector const&, 
clang::clangd::SymbolSlab const&)::'lambda'(clang::CodeCompletionResult 
const&)::operator()(clang::CodeCompletionResult const&) 
tools/clang/tools/extra/clangd/CodeComplete.cpp:938:24
  #33 0x5632dff6e426 in 
clang::clangd::CodeCompleteFlow::mergeResults(std::vector const&, 
clang::clangd::SymbolSlab const&) 
third_party/llvm/llvm/tools/clang/tools/extra/clangd/CodeComplete.cpp:949:38
  #34 0x5632dff7a34d in clang::clangd::CodeCompleteFlow::runWithSema() 
llvm/tools/clang/tools/extra/clangd/CodeComplete.cpp:894:16
  #35 0x5632dff6df6a in 
clang::clangd::CodeCompleteFlow::run(clang::clangd::(anonymous 
namespace)::SemaCompleteInput const&) &&::'lambda'()::operator()() const 
third_party/llvm/llvm/tools/clang/tools/extra/clangd/CodeComplete.cpp:858:35
  #36 0x5632dff6cd42 in clang::clangd::(anonymous 
namespace)::semaCodeComplete(std::unique_ptr, clang::CodeCompleteOptions 
const&, clang::clangd::(anonymous namespace)::SemaCompleteInput const&, 
llvm::function_ref) 
tools/clang/tools/extra/clangd/CodeComplete.cpp:735:5

0x61400020d090 is located 80 bytes inside of 432-byte region 
[0x61400020d040,0x61400020d1f0)
freed by thread T175  here:

  #0 0x5632df74e115 in operator delete(void*, unsigned long) 
projects/compiler-rt/lib/asan/asan_new_delete.cc:161:3
  #1 0x5632e0b06973 in clang::Parser::~Parser() 
tools/clang/lib/Parse/Parser.cpp:410:3
  #2 0x5632e0b06ddd in clang::Parser::~Parser() 
clang/lib/Parse/Parser.cpp:408:19
  #3 0x5632e0b03286 in std::unique_ptr::~unique_ptr() .../bits/unique_ptr.h:236:4
  #4 0x5632e0b021c4 in clang::ParseAST(clang::Sema&, bool, bool) 
tools/clang/lib/Parse/ParseAST.cpp:182:1
  #5 0x5632e0726544 in clang::FrontendAction::Execute() 
tools/clang/lib/Frontend/FrontendAction.cpp:904:8
  #6 0x5632dff6cd05 in clang::clangd::(anonymous 
namespace)::semaCodeComplete(std::unique_ptr, clang::CodeCompleteOptions 
const&, clang::clangd::(anonymous namespace)::SemaCompleteInput const&, 
llvm::function_ref) 
tools/clang/tools/extra/clangd/CodeComplete.cpp:728:15


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44000

Files:
  clangd/CodeComplete.cpp

Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -425,30 +425,32 @@
   return Info.scopesForIndexQuery();
 }
 
+struct SemaCtx {
+  Sema 
+  CodeCompletionContext 
+  GlobalCodeCompletionAllocator 
+  CodeCompletionTUInfo 
+};
+
 // The CompletionRecorder captures Sema code-complete output, including context.
 // It filters out ignored results (but doesn't apply fuzzy-filtering yet).
 // It doesn't do scoring or conversion to CompletionItem yet, as we want to
 // merge with index results first.
 struct CompletionRecorder : public CodeCompleteConsumer {
-  CompletionRecorder(const CodeCompleteOptions )
+  using ResultsFn =
+  UniqueFunction

[PATCH] D43814: [x86][CET] Introduce _get_ssp, _inc_ssp intrinsics

[Gabor Buella via Phabricator via cfe-commits]

GBuella updated this revision to Diff 136703.

https://reviews.llvm.org/D43814

Files:
  lib/Headers/cetintrin.h
  test/CodeGen/cetintrin.c


Index: test/CodeGen/cetintrin.c
===
--- test/CodeGen/cetintrin.c
+++ test/CodeGen/cetintrin.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -ffreestanding %s -triple=i386-apple-darwin -target-feature 
+shstk -emit-llvm -o - -Wall -Werror | FileCheck %s
-// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-apple-darwin 
-target-feature +shstk  -emit-llvm -o - -Wall -Werror | FileCheck %s 
--check-prefix=X86_64
+// RUN: %clang_cc1 -ffreestanding %s -triple=i386-apple-darwin -target-feature 
+shstk -emit-llvm -o - -Wall -Werror | FileCheck %s --check-prefix=I386 
--check-prefix=CHECK
+// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-apple-darwin 
-target-feature +shstk  -emit-llvm -o - -Wall -Werror | FileCheck %s 
--check-prefix=X86_64  --check-prefix=CHECK
 
 #include 
 
@@ -15,6 +15,20 @@
   // X86_64:   call void @llvm.x86.incsspq(i64 %{{[a-z0-9.]+}})
   _incsspq(a);
 }
+
+void test_inc_ssp(unsigned int a) {
+  // X86_64-LABEL: @test_inc_ssp
+  // X86_64:   call void @llvm.x86.incsspq(i64 %{{[a-z0-9.]+}})
+  _inc_ssp(a);
+}
+#else
+
+void test_inc_ssp(unsigned int a) {
+  // I386-LABEL: @test_inc_ssp
+  // I386:   call void @llvm.x86.incsspd(i32 %{{[0-9]+}})
+  _inc_ssp(a);
+}
+
 #endif
 
 unsigned int test_rdsspd(unsigned int a) {
@@ -29,6 +43,21 @@
   // X86_64:   call i64 @llvm.x86.rdsspq(i64 %{{[a-z0-9.]+}})
   return _rdsspq(a);
 }
+
+unsigned long long test_get_ssp(void) {
+  // X86_64-LABEL: @test_get_ssp
+  // X86_64:   call i64 @llvm.x86.rdsspq(i64 0)
+  return _get_ssp();
+}
+
+#else
+
+unsigned int test_get_ssp(void) {
+  // I386-LABEL: @test_get_ssp
+  // I386:   call i32 @llvm.x86.rdsspd(i32 0)
+  return _get_ssp();
+}
+
 #endif
 
 void  test_saveprevssp() {
Index: lib/Headers/cetintrin.h
===
--- lib/Headers/cetintrin.h
+++ lib/Headers/cetintrin.h
@@ -42,6 +42,16 @@
 }
 #endif /* __x86_64__ */
 
+#ifdef __x86_64__
+static __inline__ void __DEFAULT_FN_ATTRS _inc_ssp(unsigned int __a) {
+  __builtin_ia32_incsspq(__a);
+}
+#else /* __x86_64__ */
+static __inline__ void __DEFAULT_FN_ATTRS _inc_ssp(unsigned int __a) {
+  __builtin_ia32_incsspd((int)__a);
+}
+#endif /* __x86_64__ */
+
 static __inline__ unsigned int __DEFAULT_FN_ATTRS _rdsspd(unsigned int __a) {
   return __builtin_ia32_rdsspd(__a);
 }
@@ -52,6 +62,16 @@
 }
 #endif /* __x86_64__ */
 
+#ifdef __x86_64__
+static __inline__ unsigned long long __DEFAULT_FN_ATTRS _get_ssp(void) {
+  return __builtin_ia32_rdsspq(0);
+}
+#else /* __x86_64__ */
+static __inline__ unsigned int __DEFAULT_FN_ATTRS _get_ssp(void) {
+  return __builtin_ia32_rdsspd(0);
+}
+#endif /* __x86_64__ */
+
 static __inline__ void __DEFAULT_FN_ATTRS _saveprevssp() {
   __builtin_ia32_saveprevssp();
 }


Index: test/CodeGen/cetintrin.c
===
--- test/CodeGen/cetintrin.c
+++ test/CodeGen/cetintrin.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -ffreestanding %s -triple=i386-apple-darwin -target-feature +shstk -emit-llvm -o - -Wall -Werror | FileCheck %s
-// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-apple-darwin -target-feature +shstk  -emit-llvm -o - -Wall -Werror | FileCheck %s --check-prefix=X86_64
+// RUN: %clang_cc1 -ffreestanding %s -triple=i386-apple-darwin -target-feature +shstk -emit-llvm -o - -Wall -Werror | FileCheck %s --check-prefix=I386 --check-prefix=CHECK
+// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-apple-darwin -target-feature +shstk  -emit-llvm -o - -Wall -Werror | FileCheck %s --check-prefix=X86_64  --check-prefix=CHECK
 
 #include 
 
@@ -15,6 +15,20 @@
   // X86_64:   call void @llvm.x86.incsspq(i64 %{{[a-z0-9.]+}})
   _incsspq(a);
 }
+
+void test_inc_ssp(unsigned int a) {
+  // X86_64-LABEL: @test_inc_ssp
+  // X86_64:   call void @llvm.x86.incsspq(i64 %{{[a-z0-9.]+}})
+  _inc_ssp(a);
+}
+#else
+
+void test_inc_ssp(unsigned int a) {
+  // I386-LABEL: @test_inc_ssp
+  // I386:   call void @llvm.x86.incsspd(i32 %{{[0-9]+}})
+  _inc_ssp(a);
+}
+
 #endif
 
 unsigned int test_rdsspd(unsigned int a) {
@@ -29,6 +43,21 @@
   // X86_64:   call i64 @llvm.x86.rdsspq(i64 %{{[a-z0-9.]+}})
   return _rdsspq(a);
 }
+
+unsigned long long test_get_ssp(void) {
+  // X86_64-LABEL: @test_get_ssp
+  // X86_64:   call i64 @llvm.x86.rdsspq(i64 0)
+  return _get_ssp();
+}
+
+#else
+
+unsigned int test_get_ssp(void) {
+  // I386-LABEL: @test_get_ssp
+  // I386:   call i32 @llvm.x86.rdsspd(i32 0)
+  return _get_ssp();
+}
+
 #endif
 
 void  test_saveprevssp() {
Index: lib/Headers/cetintrin.h
===
--- lib/Headers/cetintrin.h
+++ lib/Headers/cetintrin.h
@@ -42,6 +42,16 @@
 }
 #endif /* __x86_64__ */
 
+#ifdef __x86_64__
+static __inline__ void 

[PATCH] D43814: [x86][CET] Introduce _get_ssp, _inc_ssp intrinsics

[Gabor Buella via Phabricator via cfe-commits]

GBuella added inline comments.



Comment at: lib/Headers/cetintrin.h:45
+static __inline__ void __DEFAULT_FN_ATTRS _inc_ssp(unsigned int __a) {
+  __builtin_ia32_incsspq(__a);
+}

craig.topper wrote:
> Where is the zeroing behavior for older CPUs coming from? This implementation 
> looks identical to _incsspq?
Do we need some zeroing behaviour for _inc_ssp?
I know we need it for _get_ssp.


Repository:
  rC Clang

https://reviews.llvm.org/D43814



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

[PATCH] D43995: Do not generate calls to fentry with __attribute__((no_instrument_function))

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc edited reviewers, added: rnk, rsmith, rjmccall; removed: chandlerc.
chandlerc added a comment.

Adding more Clang CodeGen folks to this review rather than myself...


Repository:
  rC Clang

https://reviews.llvm.org/D43995



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

[PATCH] D43572: [Sema] Improve test coverage of narrowing conversion diagnostics

[Mikhail Maltsev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC326551: [Sema] Improve test coverage of narrowing conversion 
diagnostics (authored by miyuki, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D43572

Files:
  test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp


Index: test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
===
--- test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
+++ test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
@@ -147,8 +147,10 @@
 void shrink_int() {
   // Not a constant expression.
   short s = 1;
+  UnscopedEnum e = EnumVal;
   unsigned short us = 1;
   Agg c1 = {s};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}}
+  Agg c2 = {e};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}}
   Agg s1 = {s};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}}
   Agg s2 = {us};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}}
 
@@ -158,16 +160,19 @@
   // long).
   long l1 = 1;
   Agg i1 = {l1};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}}
+  Agg i2 = {e};  // OK
   long long ll = 1;
   Agg l2 = {ll};  // OK
 
   // Constants.
-  Agg c2 = {127};  // OK
-  Agg c3 = {300};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}} expected-warning {{changes value}}
-
-  Agg i2 = {0x7FFFU};  // OK
-  Agg i3 = {0x8000U};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}}
-  Agg i4 = {-0x8000L};  // expected-error {{ cannot be 
narrowed }} expected-note {{silence}}
+  Agg c3 = {127};  // OK
+  Agg c4 = {300};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}} expected-warning {{changes value}}
+  Agg c5 = {EnumVal};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}} expected-warning {{changes value}}
+
+  Agg i3 = {0x7FFFU};  // OK
+  Agg i4 = {EnumVal};  // OK
+  Agg i5 = {0x8000U};  // expected-error {{ cannot be narrowed }} 
expected-note {{silence}}
+  Agg i6 = {-0x8000L};  // expected-error {{ cannot be 
narrowed }} expected-note {{silence}}
 
   // Bool is also an integer type, but conversions to it are a different AST
   // node.


Index: test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
===
--- test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
+++ test/CXX/dcl.decl/dcl.init/dcl.init.list/p7-0x.cpp
@@ -147,8 +147,10 @@
 void shrink_int() {
   // Not a constant expression.
   short s = 1;
+  UnscopedEnum e = EnumVal;
   unsigned short us = 1;
   Agg c1 = {s};  // expected-error {{ cannot be narrowed }} expected-note {{silence}}
+  Agg c2 = {e};  // expected-error {{ cannot be narrowed }} expected-note {{silence}}
   Agg s1 = {s};  // expected-error {{ cannot be narrowed }} expected-note {{silence}}
   Agg s2 = {us};  // expected-error {{ cannot be narrowed }} expected-note {{silence}}
 
@@ -158,16 +160,19 @@
   // long).
   long l1 = 1;
   Agg i1 = {l1};  // expected-error {{ cannot be narrowed }} expected-note {{silence}}
+  Agg i2 = {e};  // OK
   long long ll = 1;
   Agg l2 = {ll};  // OK
 
   // Constants.
-  Agg c2 = {127};  // OK
-  Agg c3 = {300};  // expected-error {{ cannot be narrowed }} expected-note {{silence}} expected-warning {{changes value}}
-
-  Agg i2 = {0x7FFFU};  // OK
-  Agg i3 = {0x8000U};  // expected-error {{ cannot be narrowed }} expected-note {{silence}}
-  Agg i4 = {-0x8000L};  // expected-error {{ cannot be narrowed }} expected-note {{silence}}
+  Agg c3 = {127};  // OK
+  Agg c4 = {300};  // expected-error {{ cannot be narrowed }} expected-note {{silence}} expected-warning {{changes value}}
+  Agg c5 = {EnumVal};  // expected-error {{ cannot be narrowed }} expected-note {{silence}} expected-warning {{changes value}}
+
+  Agg i3 = {0x7FFFU};  // OK
+  Agg i4 = {EnumVal};  // OK
+  Agg i5 = {0x8000U};  // expected-error {{ cannot be narrowed }} expected-note {{silence}}
+  Agg i6 = {-0x8000L};  // expected-error {{ cannot be narrowed }} expected-note {{silence}}
 
   // Bool is also an integer type, but conversions to it are a different AST
   // node.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r326548 - [clangd:vscode] Bump clangd-vscode version to 0.0.4.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Fri Mar  2 01:26:17 2018
New Revision: 326548

URL: http://llvm.org/viewvc/llvm-project?rev=326548=rev
Log:
[clangd:vscode] Bump clangd-vscode version to 0.0.4.

Modified:
clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json

Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json?rev=326548=326547=326548=diff
==
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json (original)
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/package.json Fri Mar  
2 01:26:17 2018
@@ -2,7 +2,7 @@
 "name": "vscode-clangd",
 "displayName": "vscode-clangd",
 "description": "Clang Language Server",
-"version": "0.0.3",
+"version": "0.0.4",
 "publisher": "llvm-vs-code-extensions",
 "homepage": "https://clang.llvm.org/extra/clangd.html;,
 "engines": {


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

[clang-tools-extra] r326547 - [clangd:vscode] check empty/null string correctly.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Fri Mar  2 01:21:41 2018
New Revision: 326547

URL: http://llvm.org/viewvc/llvm-project?rev=326547=rev
Log:
[clangd:vscode] check empty/null string correctly.

Modified:
clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts

Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts?rev=326547=326546=326547=diff
==
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts 
(original)
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts Fri 
Mar  2 01:21:41 2018
@@ -23,7 +23,7 @@ export function activate(context: vscode
 args: getConfig('arguments')
 };
 const traceFile = getConfig('trace');
-if (traceFile != '') {
+if (!!traceFile) {
   const trace = {CLANGD_TRACE : traceFile};
   clangd.options = {env : {...process.env, ...trace}};
 }


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

[PATCH] D43898: Preliminary refactoring in service of -Wreturn-std-move. NFC.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

@rtrieu please take a look?

Also, I would need someone to commit this for me, as I don't have commit privs.


Repository:
  rC Clang

https://reviews.llvm.org/D43898



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

[PATCH] D43648: [clangd] Debounce streams of updates.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL326546: [clangd] Debounce streams of updates. (authored by 
sammccall, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D43648

Files:
  clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
  clang-tools-extra/trunk/clangd/ClangdServer.cpp
  clang-tools-extra/trunk/clangd/ClangdServer.h
  clang-tools-extra/trunk/clangd/TUScheduler.cpp
  clang-tools-extra/trunk/clangd/TUScheduler.h
  clang-tools-extra/trunk/clangd/Threading.cpp
  clang-tools-extra/trunk/clangd/Threading.h
  clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp

Index: clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp
@@ -42,7 +42,8 @@
 TEST_F(TUSchedulerTests, MissingFiles) {
   TUScheduler S(getDefaultAsyncThreadsCount(),
 /*StorePreamblesInMemory=*/true,
-/*ASTParsedCallback=*/nullptr);
+/*ASTParsedCallback=*/nullptr,
+/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero());
 
   auto Added = testPath("added.cpp");
   Files[Added] = "";
@@ -94,9 +95,11 @@
 // To avoid a racy test, don't allow tasks to actualy run on the worker
 // thread until we've scheduled them all.
 Notification Ready;
-TUScheduler S(getDefaultAsyncThreadsCount(),
-  /*StorePreamblesInMemory=*/true,
-  /*ASTParsedCallback=*/nullptr);
+TUScheduler S(
+getDefaultAsyncThreadsCount(),
+/*StorePreamblesInMemory=*/true,
+/*ASTParsedCallback=*/nullptr,
+/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero());
 auto Path = testPath("foo.cpp");
 S.update(Path, getInputs(Path, ""), WantDiagnostics::Yes,
  [&](std::vector) { Ready.wait(); });
@@ -118,6 +121,28 @@
   EXPECT_EQ(2, CallbackCount);
 }
 
+TEST_F(TUSchedulerTests, Debounce) {
+  std::atomic CallbackCount(0);
+  {
+TUScheduler S(getDefaultAsyncThreadsCount(),
+  /*StorePreamblesInMemory=*/true,
+  /*ASTParsedCallback=*/nullptr,
+  /*UpdateDebounce=*/std::chrono::milliseconds(50));
+auto Path = testPath("foo.cpp");
+S.update(Path, getInputs(Path, "auto (debounced)"), WantDiagnostics::Auto,
+ [&](std::vector Diags) {
+   ADD_FAILURE() << "auto should have been debounced and canceled";
+ });
+std::this_thread::sleep_for(std::chrono::milliseconds(10));
+S.update(Path, getInputs(Path, "auto (timed out)"), WantDiagnostics::Auto,
+ [&](std::vector Diags) { ++CallbackCount; });
+std::this_thread::sleep_for(std::chrono::milliseconds(60));
+S.update(Path, getInputs(Path, "auto (shut down)"), WantDiagnostics::Auto,
+ [&](std::vector Diags) { ++CallbackCount; });
+  }
+  EXPECT_EQ(2, CallbackCount);
+}
+
 TEST_F(TUSchedulerTests, ManyUpdates) {
   const int FilesCount = 3;
   const int UpdatesPerFile = 10;
@@ -131,7 +156,8 @@
   {
 TUScheduler S(getDefaultAsyncThreadsCount(),
   /*StorePreamblesInMemory=*/true,
-  /*ASTParsedCallback=*/nullptr);
+  /*ASTParsedCallback=*/nullptr,
+  /*UpdateDebounce=*/std::chrono::milliseconds(50));
 
 std::vector Files;
 for (int I = 0; I < FilesCount; ++I) {
Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
@@ -405,7 +405,7 @@
 : Out(Out), CDB(std::move(CompileCommandsDir)), CCOpts(CCOpts),
   Server(CDB, /*DiagConsumer=*/*this, FSProvider, AsyncThreadsCount,
  StorePreamblesInMemory, BuildDynamicSymbolIndex, StaticIdx,
- ResourceDir) {}
+ ResourceDir, /*UpdateDebounce=*/std::chrono::milliseconds(500)) {}
 
 bool ClangdLSPServer::run(std::istream , JSONStreamStyle InputStyle) {
   assert(!IsDone && "Run was called before");
Index: clang-tools-extra/trunk/clangd/TUScheduler.h
===
--- clang-tools-extra/trunk/clangd/TUScheduler.h
+++ clang-tools-extra/trunk/clangd/TUScheduler.h
@@ -17,6 +17,7 @@
 
 namespace clang {
 namespace clangd {
+
 /// Returns a number of a default async threads to use for TUScheduler.
 /// Returned value is always >= 1 (i.e. will not cause requests to be processed
 /// synchronously).
@@ -46,10 +47,12 @@
 /// and scheduling tasks.
 /// Callbacks are run on a threadpool and it's appropriate to do slow work in
 /// them. Each task has a name, used for tracing (should be UpperCamelCase).
+/// FIXME(sammccall): pull out a scheduler options 

[clang-tools-extra] r326546 - [clangd] Debounce streams of updates.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Fri Mar  2 00:56:37 2018
New Revision: 326546

URL: http://llvm.org/viewvc/llvm-project?rev=326546=rev
Log:
[clangd] Debounce streams of updates.

Summary:
Don't actually start building ASTs for new revisions until either:
- 500ms have passed since the last revision, or
- we actually need the revision for something (or to unblock the queue)

In practice, this avoids the "first keystroke results in diagnostics" problem.
This is kind of awkward to test, and the test is pretty bad.
It can be observed nicely by capturing a trace, though.

Reviewers: hokein, ilya-biryukov

Subscribers: klimek, jkorous-apple, ioeric, cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/clangd/TUScheduler.cpp
clang-tools-extra/trunk/clangd/TUScheduler.h
clang-tools-extra/trunk/clangd/Threading.cpp
clang-tools-extra/trunk/clangd/Threading.h
clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=326546=326545=326546=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Fri Mar  2 00:56:37 2018
@@ -405,7 +405,7 @@ ClangdLSPServer::ClangdLSPServer(JSONOut
 : Out(Out), CDB(std::move(CompileCommandsDir)), CCOpts(CCOpts),
   Server(CDB, /*DiagConsumer=*/*this, FSProvider, AsyncThreadsCount,
  StorePreamblesInMemory, BuildDynamicSymbolIndex, StaticIdx,
- ResourceDir) {}
+ ResourceDir, /*UpdateDebounce=*/std::chrono::milliseconds(500)) {}
 
 bool ClangdLSPServer::run(std::istream , JSONStreamStyle InputStyle) {
   assert(!IsDone && "Run was called before");

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=326546=326545=326546=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Fri Mar  2 00:56:37 2018
@@ -76,7 +76,8 @@ ClangdServer::ClangdServer(GlobalCompila
unsigned AsyncThreadsCount,
bool StorePreamblesInMemory,
bool BuildDynamicSymbolIndex, SymbolIndex 
*StaticIdx,
-   llvm::Optional ResourceDir)
+   llvm::Optional ResourceDir,
+   std::chrono::steady_clock::duration UpdateDebounce)
 : CompileArgs(CDB,
   ResourceDir ? ResourceDir->str() : getStandardResourceDir()),
   DiagConsumer(DiagConsumer), FSProvider(FSProvider),
@@ -91,7 +92,8 @@ ClangdServer::ClangdServer(GlobalCompila
 FileIdx
 ? [this](PathRef Path,
  ParsedAST *AST) { FileIdx->update(Path, AST); 
}
-: ASTParsedCallback()) {
+: ASTParsedCallback(),
+UpdateDebounce) {
   if (FileIdx && StaticIdx) {
 MergedIndex = mergeIndex(FileIdx.get(), StaticIdx);
 Index = MergedIndex.get();

Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=326546=326545=326546=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Fri Mar  2 00:56:37 2018
@@ -125,6 +125,8 @@ public:
   /// \p DiagConsumer. Note that a callback to \p DiagConsumer happens on a
   /// worker thread. Therefore, instances of \p DiagConsumer must properly
   /// synchronize access to shared state.
+  /// UpdateDebounce determines how long to wait after a new version of the 
file
+  /// before starting to compute diagnostics.
   ///
   /// \p StorePreamblesInMemory defines whether the Preambles generated by
   /// clangd are stored in-memory or on disk.
@@ -135,13 +137,17 @@ public:
   ///
   /// If \p StaticIdx is set, ClangdServer uses the index for global code
   /// completion.
+  /// FIXME(sammccall): pull out an options struct.
   ClangdServer(GlobalCompilationDatabase ,
DiagnosticsConsumer ,
FileSystemProvider , unsigned AsyncThreadsCount,
bool StorePreamblesInMemory,
bool BuildDynamicSymbolIndex = false,
SymbolIndex *StaticIdx = nullptr,
-   llvm::Optional ResourceDir = llvm::None);
+   llvm::Optional