[PATCH] D50549: [libcxx] [test] Repair thread unsafety in thread tests

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

I was hoping my comment would summon them magically. I'll figure it out shortly.


https://reviews.llvm.org/D50549



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

[PATCH] D50549: [libcxx] [test] Repair thread unsafety in thread tests

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

BillyONeal added a comment.

In https://reviews.llvm.org/D50549#1194852, @EricWF wrote:

> We should add some TSAN folks to this review, since I think these are also 
> TSAN false negatives; perhaps correctly, perhaps not.


Do you know who that is / can you add them?


https://reviews.llvm.org/D50549



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

patch for formatter bug 38525

[Owen Pan via cfe-commits]

Hi,

Please see the attached patch file and test case for Bug 38525
 in Formatter.

Thanks,
Owen
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp	(revision 339102)
+++ lib/Format/ContinuationIndenter.cpp	(working copy)
@@ -700,7 +700,8 @@
 // Indent relative to the RHS of the expression unless this is a simple
 // assignment without binary expression on the RHS. Also indent relative to
 // unary operators and the colons of constructor initializers.
-State.Stack.back().LastSpace = State.Column;
+if (Style.BreakBeforeBinaryOperators == FormatStyle::BOS_None)
+  State.Stack.back().LastSpace = State.Column;
   } else if (Previous.is(TT_InheritanceColon)) {
 State.Stack.back().Indent = State.Column;
 State.Stack.back().LastSpace = State.Column;
bool BreakBeforeBinaryOperators(bool someVeryVeryLongConditionThatBarelyFitsOnALine, bool someOtherLongishConditionPart1, bool someOtherEvenLongerNestedConditionPart2)
{
	return someVeryVeryLongConditionThatBarelyFitsOnALine && (someOtherLongishConditionPart1 || someOtherEvenLongerNestedConditionPart2);
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r339494 - Make the section boundary checks on Windows not depend on the order as they are emitted in reverse when the compiler is built by Visual C++.

[Douglas Yung via cfe-commits]

Author: dyung
Date: Fri Aug 10 19:46:47 2018
New Revision: 339494

URL: http://llvm.org/viewvc/llvm-project?rev=339494=rev
Log:
Make the section boundary checks on Windows not depend on the order as they are 
emitted in reverse when the compiler is built by Visual C++.

Modified:
cfe/trunk/test/CodeGenObjC/gnu-init.m

Modified: cfe/trunk/test/CodeGenObjC/gnu-init.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/gnu-init.m?rev=339494=339493=339494=diff
==
--- cfe/trunk/test/CodeGenObjC/gnu-init.m (original)
+++ cfe/trunk/test/CodeGenObjC/gnu-init.m Fri Aug 10 19:46:47 2018
@@ -71,22 +71,22 @@
 
 
 // Make sure all of our section boundary variables are emitted correctly.
-// CHECK-WIN: @__start___objc_selectors = linkonce_odr hidden global 
%.objc_section_sentinel zeroinitializer, section "__objc_selectors$a", comdat, 
align 1
-// CHECK-WIN: @__stop__objc_selectors = linkonce_odr hidden global 
%.objc_section_sentinel zeroinitializer, section "__objc_selectors$z", comdat, 
align 1
-// CHECK-WIN: @__start___objc_classes = linkonce_odr hidden global 
%.objc_section_sentinel zeroinitializer, section "__objc_classes$a", comdat, 
align 1
-// CHECK-WIN: @__stop__objc_classes = linkonce_odr hidden global 
%.objc_section_sentinel zeroinitializer, section "__objc_classes$z", comdat, 
align 1
-// CHECK-WIN: @__start___objc_class_refs = linkonce_odr hidden global 
%.objc_section_sentinel zeroinitializer, section "__objc_class_refs$a", comdat, 
align 1
-// CHECK-WIN: @__stop__objc_class_refs = linkonce_odr hidden global 
%.objc_section_sentinel zeroinitializer, section "__objc_class_refs$z", comdat, 
align 1
-// CHECK-WIN: @__start___objc_cats = linkonce_odr hidden global 
%.objc_section_sentinel zeroinitializer, section "__objc_cats$a", comdat, align 
1
-// CHECK-WIN: @__stop__objc_cats = linkonce_odr hidden global 
%.objc_section_sentinel zeroinitializer, section "__objc_cats$z", comdat, align 
1
-// CHECK-WIN: @__start___objc_protocols = linkonce_odr hidden global 
%.objc_section_sentinel zeroinitializer, section "__objc_protocols$a", comdat, 
align 1
-// CHECK-WIN: @__stop__objc_protocols = linkonce_odr hidden global 
%.objc_section_sentinel zeroinitializer, section "__objc_protocols$z", comdat, 
align 1
-// CHECK-WIN: @__start___objc_protocol_refs = linkonce_odr hidden global 
%.objc_section_sentinel zeroinitializer, section "__objc_protocol_refs$a", 
comdat, align 1
-// CHECK-WIN: @__stop__objc_protocol_refs = linkonce_odr hidden global 
%.objc_section_sentinel zeroinitializer, section "__objc_protocol_refs$z", 
comdat, align 1
-// CHECK-WIN: @__start___objc_class_aliases = linkonce_odr hidden global 
%.objc_section_sentinel zeroinitializer, section "__objc_class_aliases$a", 
comdat, align 1
-// CHECK-WIN: @__stop__objc_class_aliases = linkonce_odr hidden global 
%.objc_section_sentinel zeroinitializer, section "__objc_class_aliases$z", 
comdat, align 1
-// CHECK-WIN: @__start___objc_constant_string = linkonce_odr hidden global 
%.objc_section_sentinel zeroinitializer, section "__objc_constant_string$a", 
comdat, align 1
-// CHECK-WIN: @__stop__objc_constant_string = linkonce_odr hidden global 
%.objc_section_sentinel zeroinitializer, section "__objc_constant_string$z", 
comdat, align 1
+// CHECK-WIN-DAG: @__start___objc_selectors = linkonce_odr hidden global 
%.objc_section_sentinel zeroinitializer, section "__objc_selectors$a", comdat, 
align 1
+// CHECK-WIN-DAG: @__stop__objc_selectors = linkonce_odr hidden global 
%.objc_section_sentinel zeroinitializer, section "__objc_selectors$z", comdat, 
align 1
+// CHECK-WIN-DAG: @__start___objc_classes = linkonce_odr hidden global 
%.objc_section_sentinel zeroinitializer, section "__objc_classes$a", comdat, 
align 1
+// CHECK-WIN-DAG: @__stop__objc_classes = linkonce_odr hidden global 
%.objc_section_sentinel zeroinitializer, section "__objc_classes$z", comdat, 
align 1
+// CHECK-WIN-DAG: @__start___objc_class_refs = linkonce_odr hidden global 
%.objc_section_sentinel zeroinitializer, section "__objc_class_refs$a", comdat, 
align 1
+// CHECK-WIN-DAG: @__stop__objc_class_refs = linkonce_odr hidden global 
%.objc_section_sentinel zeroinitializer, section "__objc_class_refs$z", comdat, 
align 1
+// CHECK-WIN-DAG: @__start___objc_cats = linkonce_odr hidden global 
%.objc_section_sentinel zeroinitializer, section "__objc_cats$a", comdat, align 
1
+// CHECK-WIN-DAG: @__stop__objc_cats = linkonce_odr hidden global 
%.objc_section_sentinel zeroinitializer, section "__objc_cats$z", comdat, align 
1
+// CHECK-WIN-DAG: @__start___objc_protocols = linkonce_odr hidden global 
%.objc_section_sentinel zeroinitializer, section "__objc_protocols$a", comdat, 
align 1
+// CHECK-WIN-DAG: @__stop__objc_protocols = linkonce_odr hidden global 
%.objc_section_sentinel zeroinitializer, section "__objc_protocols$z", comdat, 
align 1
+// CHECK-WIN-DAG: @__start___objc_protocol_refs = 

[PATCH] D50596: [HIP] Make __hip_gpubin_handle hidden to avoid being merged across different shared libraries

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: rjmccall, tra.

Different shared libraries contain different fat binary, which is stored in a 
global variable
`__hip_gpubin_handle`. Since different compilation units share the same fat 
binary, this
variable has linkonce linkage. However, it should not be merged across 
different shared
libraries.

This patch set the visibility of the global variable to be hidden, which will 
make it invisible
in the shared library, therefore preventing it from being merged.


https://reviews.llvm.org/D50596

Files:
  lib/CodeGen/CGCUDANV.cpp
  test/CodeGenCUDA/device-stub.cu


Index: test/CodeGenCUDA/device-stub.cu
===
--- test/CodeGenCUDA/device-stub.cu
+++ test/CodeGenCUDA/device-stub.cu
@@ -80,7 +80,7 @@
 // HIP-SAME: section ".hipFatBinSegment"
 // * variable to save GPU binary handle after initialization
 // CUDANORDC: @__[[PREFIX]]_gpubin_handle = internal global i8** null
-// HIP: @__[[PREFIX]]_gpubin_handle = linkonce global i8** null
+// HIP: @__[[PREFIX]]_gpubin_handle = linkonce hidden global i8** null
 // * constant unnamed string with NVModuleID
 // RDC: [[MODULE_ID_GLOBAL:@.*]] = private constant
 // CUDARDC-SAME: c"[[MODULE_ID:.+]]\00", section "__nv_module_id", align 32
Index: lib/CodeGen/CGCUDANV.cpp
===
--- lib/CodeGen/CGCUDANV.cpp
+++ lib/CodeGen/CGCUDANV.cpp
@@ -459,6 +459,8 @@
 /*Initializer=*/llvm::ConstantPointerNull::get(VoidPtrPtrTy),
 "__hip_gpubin_handle");
 GpuBinaryHandle->setAlignment(CGM.getPointerAlign().getQuantity());
+// Prevent the weak symbol in different shared libraries being merged.
+GpuBinaryHandle->setVisibility(llvm::GlobalValue::HiddenVisibility);
 Address GpuBinaryAddr(
 GpuBinaryHandle,
 CharUnits::fromQuantity(GpuBinaryHandle->getAlignment()));


Index: test/CodeGenCUDA/device-stub.cu
===
--- test/CodeGenCUDA/device-stub.cu
+++ test/CodeGenCUDA/device-stub.cu
@@ -80,7 +80,7 @@
 // HIP-SAME: section ".hipFatBinSegment"
 // * variable to save GPU binary handle after initialization
 // CUDANORDC: @__[[PREFIX]]_gpubin_handle = internal global i8** null
-// HIP: @__[[PREFIX]]_gpubin_handle = linkonce global i8** null
+// HIP: @__[[PREFIX]]_gpubin_handle = linkonce hidden global i8** null
 // * constant unnamed string with NVModuleID
 // RDC: [[MODULE_ID_GLOBAL:@.*]] = private constant
 // CUDARDC-SAME: c"[[MODULE_ID:.+]]\00", section "__nv_module_id", align 32
Index: lib/CodeGen/CGCUDANV.cpp
===
--- lib/CodeGen/CGCUDANV.cpp
+++ lib/CodeGen/CGCUDANV.cpp
@@ -459,6 +459,8 @@
 /*Initializer=*/llvm::ConstantPointerNull::get(VoidPtrPtrTy),
 "__hip_gpubin_handle");
 GpuBinaryHandle->setAlignment(CGM.getPointerAlign().getQuantity());
+// Prevent the weak symbol in different shared libraries being merged.
+GpuBinaryHandle->setVisibility(llvm::GlobalValue::HiddenVisibility);
 Address GpuBinaryAddr(
 GpuBinaryHandle,
 CharUnits::fromQuantity(GpuBinaryHandle->getAlignment()));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r339493 - [analyzer] Fix keyboard navigation for .msgNote events

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Fri Aug 10 18:47:41 2018
New Revision: 339493

URL: http://llvm.org/viewvc/llvm-project?rev=339493=rev
Log:
[analyzer] Fix keyboard navigation for .msgNote events

Does not go to msgNote's.

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp?rev=339493=339492=339493=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp Fri Aug 10 18:47:41 
2018
@@ -997,7 +997,8 @@ var numToId = function(num) {
 };
 
 var navigateTo = function(up) {
-  var numItems = document.querySelectorAll(".line > .msg").length;
+  var numItems = document.querySelectorAll(
+  ".line > .msgEvent, .line > .msgControl").length;
   var currentSelected = findNum();
   var newSelected = move(currentSelected, up, numItems);
   var newEl = numToId(newSelected, numItems);


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

[PATCH] D50595: [analyzer] Fix keyboard navigation for .msgNote events

[George Karpenkov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC339493: [analyzer] Fix keyboard navigation for .msgNote 
events (authored by george.karpenkov, committed by ).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D50595?vs=160220=160223#toc

Repository:
  rC Clang

https://reviews.llvm.org/D50595

Files:
  lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp


Index: lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
===
--- lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -997,7 +997,8 @@
 };
 
 var navigateTo = function(up) {
-  var numItems = document.querySelectorAll(".line > .msg").length;
+  var numItems = document.querySelectorAll(
+  ".line > .msgEvent, .line > .msgControl").length;
   var currentSelected = findNum();
   var newSelected = move(currentSelected, up, numItems);
   var newEl = numToId(newSelected, numItems);


Index: lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
===
--- lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -997,7 +997,8 @@
 };
 
 var navigateTo = function(up) {
-  var numItems = document.querySelectorAll(".line > .msg").length;
+  var numItems = document.querySelectorAll(
+  ".line > .msgEvent, .line > .msgControl").length;
   var currentSelected = findNum();
   var newSelected = move(currentSelected, up, numItems);
   var newEl = numToId(newSelected, numItems);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41217: [Concepts] Concept Specialization Expressions

[Saar Raz via Phabricator via cfe-commits]

saar.raz updated this revision to Diff 160216.
saar.raz added a comment.

Address Richard's CR comments. Among other things:

- CSEs overhauled and now store both source and converted template arguments 
(latter are tail-allocated), template KW location and NNS.
- CSEs no longer violate layering - satisfaction check now moved back to 
CheckConceptTemplateId.
- CodeSynthesisContexts created for both the act of checking the associated 
constraints of a template or the constraint expression of a concept 
(non-SFINAE), and for the act of substituting template arguments into a(n 
atomic) constraint expression (SFINAE).
- Added more tests for cases where substitution leads to a non-SFINAE failure 
and emits diagnostics
- Fixed and added test for incorrect conversion of instantiated CSE argument 
list.


Repository:
  rC Clang

https://reviews.llvm.org/D41217

Files:
  include/clang/AST/DeclTemplate.h
  include/clang/AST/ExprCXX.h
  include/clang/AST/RecursiveASTVisitor.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/StmtNodes.td
  include/clang/Sema/Sema.h
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/Expr.cpp
  lib/AST/ExprCXX.cpp
  lib/AST/ExprClassification.cpp
  lib/AST/ExprConstant.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/StmtPrinter.cpp
  lib/AST/StmtProfile.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/Parse/ParseExpr.cpp
  lib/Parse/ParseTemplate.cpp
  lib/Sema/CMakeLists.txt
  lib/Sema/SemaConcept.cpp
  lib/Sema/SemaExceptionSpec.cpp
  lib/Sema/SemaTemplate.cpp
  lib/Sema/SemaTemplateInstantiate.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/CXX/concepts-ts/expr/expr.prim/expr.prim.id/p3.cpp
  test/Parser/cxx-concept-declaration.cpp
  tools/libclang/CXCursor.cpp

Index: tools/libclang/CXCursor.cpp
===
--- tools/libclang/CXCursor.cpp
+++ tools/libclang/CXCursor.cpp
@@ -231,6 +231,7 @@
   case Stmt::TypeTraitExprClass:
   case Stmt::CoroutineBodyStmtClass:
   case Stmt::CoawaitExprClass:
+  case Stmt::ConceptSpecializationExprClass:
   case Stmt::DependentCoawaitExprClass:
   case Stmt::CoreturnStmtClass:
   case Stmt::CoyieldExprClass:
Index: test/Parser/cxx-concept-declaration.cpp
===
--- test/Parser/cxx-concept-declaration.cpp
+++ test/Parser/cxx-concept-declaration.cpp
@@ -9,8 +9,6 @@
 
 template concept D1 = true; // expected-error {{expected template parameter}}
 
-template concept C2 = 0.f; // expected-error {{constraint expression must be 'bool'}}
-
 struct S1 {
   template concept C1 = true; // expected-error {{concept declarations may only appear in global or namespace scope}}
 };
@@ -29,3 +27,22 @@
 
 // TODO: Add test to prevent explicit specialization, partial specialization
 // and explicit instantiation of concepts.
+
+template concept C7 = 2; // expected-error {{atomic constraint must be of type 'bool' (found 'int')}}
+template concept C8 = 2 && x; // expected-error {{atomic constraint must be of type 'bool' (found 'int')}}
+template concept C9 = x || 2 || x; // expected-error {{atomic constraint must be of type 'bool' (found 'int')}}
+template concept C10 = 8ull && x || x; // expected-error {{atomic constraint must be of type 'bool' (found 'unsigned long long')}}
+template concept C11 = sizeof(T); // expected-error {{atomic constraint must be of type 'bool' (found 'unsigned long')}}
+template concept C12 = T{};
+template concept C13 = (bool&&)true;
+template concept C14 = (const bool&)true;
+template concept C15 = (const bool)true;
+
+template
+struct integral_constant { static constexpr T value = v; };
+
+template  concept C16 = integral_constant::value && true;
+template  concept C17 = integral_constant::value;
+
+bool a = C16;
+bool b = C17;
Index: test/CXX/concepts-ts/expr/expr.prim/expr.prim.id/p3.cpp
===
--- test/CXX/concepts-ts/expr/expr.prim/expr.prim.id/p3.cpp
+++ test/CXX/concepts-ts/expr/expr.prim/expr.prim.id/p3.cpp
@@ -1,5 +1,136 @@
 // RUN:  %clang_cc1 -std=c++2a -fconcepts-ts -verify %s
-// expected-no-diagnostics
 
-template concept C = true;
-static_assert(C);
+template concept C1 = true;
+static_assert(C1);
+
+template concept C2 = sizeof(T) == 4;
+static_assert(C2);
+static_assert(!C2);
+static_assert(C2);
+static_assert(!C2);
+
+template concept C3 = sizeof(*T{}) == 4;
+static_assert(C3);
+static_assert(!C3);
+
+struct A {
+  static constexpr int add(int a, int b) {
+return a + b;
+  }
+};
+struct B {
+  static int add(int a, int b) {
+return a + b;
+  }
+};
+template
+concept C4 = U::add(1, 2) == 3; // expected-error {{substitution into constraint expression resulted in a non-constant expression}}
+static_assert(C4);
+static_assert(!C4); // expected-note {{while checking the satisfaction of concept 'C4' requested here}}
+
+template

r339489 - [analyzer] InnerPointerChecker: improve warning messages and notes.

[Reka Kovacs via cfe-commits]

Author: rkovacs
Date: Fri Aug 10 16:56:57 2018
New Revision: 339489

URL: http://llvm.org/viewvc/llvm-project?rev=339489=rev
Log:
[analyzer] InnerPointerChecker: improve warning messages and notes.

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/AllocationState.h
cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
cfe/trunk/test/Analysis/inner-pointer.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/AllocationState.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/AllocationState.h?rev=339489=339488=339489=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/AllocationState.h (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/AllocationState.h Fri Aug 10 16:56:57 
2018
@@ -26,6 +26,11 @@ ProgramStateRef markReleased(ProgramStat
 /// AF_InnerBuffer symbols.
 std::unique_ptr getInnerPointerBRVisitor(SymbolRef Sym);
 
+/// 'Sym' represents a pointer to the inner buffer of a container object.
+/// This function looks up the memory region of that object in
+/// DanglingInternalBufferChecker's program state map.
+const MemRegion *getContainerObjRegion(ProgramStateRef State, SymbolRef Sym);
+
 } // end namespace allocation_state
 
 } // end namespace ento

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp?rev=339489=339488=339489=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp Fri Aug 10 
16:56:57 2018
@@ -279,6 +279,28 @@ void InnerPointerChecker::checkDeadSymbo
   C.addTransition(State);
 }
 
+namespace clang {
+namespace ento {
+namespace allocation_state {
+
+std::unique_ptr getInnerPointerBRVisitor(SymbolRef Sym) {
+  return llvm::make_unique(Sym);
+}
+
+const MemRegion *getContainerObjRegion(ProgramStateRef State, SymbolRef Sym) {
+  RawPtrMapTy Map = State->get();
+  for (const auto Entry : Map) {
+if (Entry.second.contains(Sym)) {
+  return Entry.first;
+}
+  }
+  return nullptr;
+}
+
+} // end namespace allocation_state
+} // end namespace ento
+} // end namespace clang
+
 std::shared_ptr
 InnerPointerChecker::InnerPointerBRVisitor::VisitNode(const ExplodedNode *N,
   const ExplodedNode 
*PrevN,
@@ -292,27 +314,21 @@ InnerPointerChecker::InnerPointerBRVisit
   if (!S)
 return nullptr;
 
+  const MemRegion *ObjRegion =
+  allocation_state::getContainerObjRegion(N->getState(), PtrToBuf);
+  const auto *TypedRegion = cast(ObjRegion);
+  QualType ObjTy = TypedRegion->getValueType();
+
   SmallString<256> Buf;
   llvm::raw_svector_ostream OS(Buf);
-  OS << "Dangling inner pointer obtained here";
+  OS << "Pointer to inner buffer of '" << ObjTy.getAsString()
+ << "' obtained here";
   PathDiagnosticLocation Pos(S, BRC.getSourceManager(),
  N->getLocationContext());
   return std::make_shared(Pos, OS.str(), true,
 nullptr);
 }
 
-namespace clang {
-namespace ento {
-namespace allocation_state {
-
-std::unique_ptr getInnerPointerBRVisitor(SymbolRef Sym) {
-  return llvm::make_unique(Sym);
-}
-
-} // end namespace allocation_state
-} // end namespace ento
-} // end namespace clang
-
 void ento::registerInnerPointerChecker(CheckerManager ) {
   registerInnerPointerCheckerAux(Mgr);
   Mgr.registerChecker();

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp?rev=339489=339488=339489=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Fri Aug 10 16:56:57 
2018
@@ -1996,15 +1996,20 @@ void MallocChecker::ReportUseAfterFree(C
   BT_UseFree[*CheckKind].reset(new BugType(
   CheckNames[*CheckKind], "Use-after-free", categories::MemoryError));
 
+AllocationFamily AF =
+C.getState()->get(Sym)->getAllocationFamily();
+
 auto R = llvm::make_unique(*BT_UseFree[*CheckKind],
- "Use of memory after it is freed", N);
+AF == AF_InnerBuffer
+  ? "Inner pointer of container used after re/deallocation"
+  : "Use of memory after it is freed",
+N);
 
 R->markInteresting(Sym);
 R->addRange(Range);
 R->addVisitor(llvm::make_unique(Sym));
 
-const RefState *RS = C.getState()->get(Sym);
-if (RS->getAllocationFamily() == AF_InnerBuffer)
+if 

[PATCH] D49570: [analyzer] Improve warning messages and notes of InnerPointerChecker

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL339489: [analyzer] InnerPointerChecker: improve warning 
messages and notes. (authored by rkovacs, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D49570?vs=160193=160213#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D49570

Files:
  cfe/trunk/lib/StaticAnalyzer/Checkers/AllocationState.h
  cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  cfe/trunk/test/Analysis/inner-pointer.cpp

Index: cfe/trunk/lib/StaticAnalyzer/Checkers/AllocationState.h
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/AllocationState.h
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/AllocationState.h
@@ -26,6 +26,11 @@
 /// AF_InnerBuffer symbols.
 std::unique_ptr getInnerPointerBRVisitor(SymbolRef Sym);
 
+/// 'Sym' represents a pointer to the inner buffer of a container object.
+/// This function looks up the memory region of that object in
+/// DanglingInternalBufferChecker's program state map.
+const MemRegion *getContainerObjRegion(ProgramStateRef State, SymbolRef Sym);
+
 } // end namespace allocation_state
 
 } // end namespace ento
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
@@ -279,6 +279,28 @@
   C.addTransition(State);
 }
 
+namespace clang {
+namespace ento {
+namespace allocation_state {
+
+std::unique_ptr getInnerPointerBRVisitor(SymbolRef Sym) {
+  return llvm::make_unique(Sym);
+}
+
+const MemRegion *getContainerObjRegion(ProgramStateRef State, SymbolRef Sym) {
+  RawPtrMapTy Map = State->get();
+  for (const auto Entry : Map) {
+if (Entry.second.contains(Sym)) {
+  return Entry.first;
+}
+  }
+  return nullptr;
+}
+
+} // end namespace allocation_state
+} // end namespace ento
+} // end namespace clang
+
 std::shared_ptr
 InnerPointerChecker::InnerPointerBRVisitor::VisitNode(const ExplodedNode *N,
   const ExplodedNode *PrevN,
@@ -292,27 +314,21 @@
   if (!S)
 return nullptr;
 
+  const MemRegion *ObjRegion =
+  allocation_state::getContainerObjRegion(N->getState(), PtrToBuf);
+  const auto *TypedRegion = cast(ObjRegion);
+  QualType ObjTy = TypedRegion->getValueType();
+
   SmallString<256> Buf;
   llvm::raw_svector_ostream OS(Buf);
-  OS << "Dangling inner pointer obtained here";
+  OS << "Pointer to inner buffer of '" << ObjTy.getAsString()
+ << "' obtained here";
   PathDiagnosticLocation Pos(S, BRC.getSourceManager(),
  N->getLocationContext());
   return std::make_shared(Pos, OS.str(), true,
 nullptr);
 }
 
-namespace clang {
-namespace ento {
-namespace allocation_state {
-
-std::unique_ptr getInnerPointerBRVisitor(SymbolRef Sym) {
-  return llvm::make_unique(Sym);
-}
-
-} // end namespace allocation_state
-} // end namespace ento
-} // end namespace clang
-
 void ento::registerInnerPointerChecker(CheckerManager ) {
   registerInnerPointerCheckerAux(Mgr);
   Mgr.registerChecker();
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -1996,15 +1996,20 @@
   BT_UseFree[*CheckKind].reset(new BugType(
   CheckNames[*CheckKind], "Use-after-free", categories::MemoryError));
 
+AllocationFamily AF =
+C.getState()->get(Sym)->getAllocationFamily();
+
 auto R = llvm::make_unique(*BT_UseFree[*CheckKind],
- "Use of memory after it is freed", N);
+AF == AF_InnerBuffer
+  ? "Inner pointer of container used after re/deallocation"
+  : "Use of memory after it is freed",
+N);
 
 R->markInteresting(Sym);
 R->addRange(Range);
 R->addVisitor(llvm::make_unique(Sym));
 
-const RefState *RS = C.getState()->get(Sym);
-if (RS->getAllocationFamily() == AF_InnerBuffer)
+if (AF == AF_InnerBuffer)
   R->addVisitor(allocation_state::getInnerPointerBRVisitor(Sym));
 
 C.emitReport(std::move(R));
@@ -2944,13 +2949,22 @@
 case AF_CXXNewArray:
 case AF_IfNameIndex:
   Msg = "Memory is released";
+  StackHint = new StackHintGeneratorForSymbol(Sym,
+  "Returning; memory was released");
   break;
 case AF_InnerBuffer: {
-  OS << "Inner pointer invalidated by call to ";
+  const MemRegion *ObjRegion =
+  

[PATCH] D50580: [clang-tidy] Abseil: no namespace check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Check documentation is missing.




Comment at: clang-tidy/abseil/NoNamespaceCheck.cpp:21
+void NoNamespaceCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus) return;
+

Please place return in separate line.



Comment at: clang-tidy/abseil/NoNamespaceCheck.h:25
+class NoNamespaceCheck : public ClangTidyCheck {
+ public:
+  NoNamespaceCheck(StringRef Name, ClangTidyContext *Context)

Please run Clang-format.



Comment at: docs/ReleaseNotes.rst:60
 
 The improvements are...
 

Please remove placeholder.



Comment at: docs/ReleaseNotes.rst:65
+
+  Checks to ensure user did not open namespace absl as that
+  violates abseil's compatibility guidelines.

Please reformulate to refer to code, not user. Please enclose absl in ``.



Comment at: docs/ReleaseNotes.rst:66
+  Checks to ensure user did not open namespace absl as that
+  violates abseil's compatibility guidelines.
+

abseil ->Abseil.


https://reviews.llvm.org/D50580



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

[PATCH] D50526: Model type attributes as regular Attrs

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

rsmith added inline comments.



Comment at: include/clang/Basic/Attr.td:1510
+  let Spellings = [Keyword<"__unsafe_unretained">];
+  let Documentation = [Undocumented];
+}

aaron.ballman wrote:
> I don't suppose you can throw in some quick docs for this keyword? Or is this 
> not really user-facing? If it's not user-facing, perhaps it should have no 
> spellings and only ever be created implicitly?
This isn't actually the primary representation of `__unsafe_unretained`, this 
is an internal placeholder for "the user wrote `__unsafe_unretained` but we're 
not in ARC mode so it's meaningless (hence "inert")". So I don't think this is 
where we should attach the documentation (the right place is the 
`ObjCOwnership` attribute, which is also currently `Undocumented`, sadly). I'll 
at least add a comment to the .td file to explain that.



Comment at: lib/AST/Type.cpp:1624
+  const Type *Cur = this;
+  while (auto *AT = Cur->getAs()) {
+if (AT->getAttrKind() == AK)

aaron.ballman wrote:
> `const auto *`
Done, but...

The pointee type is deduced as `const` anyway, so the `const` doesn't give any 
extra type safety. So the only benefit would be to the reader, and I don't 
think a reader of this code would care whether `*AT` is mutable, so the `const` 
seems like a distraction to me (and hence the explicit `const` is a minor harm 
to readability rather than a minor improvement). I can see how a reader trained 
to think that absence-of-`const` implies that mutation is intended would find 
the explicit `const` clearer, but (for better or probably worse) that's not the 
style we generally use in Clang (for instance, I didn't mark the `AK` parameter 
as `const`, but there is no implication that I intend to modify it).

That said, I don't feel strongly about this, and I certainly have no objection 
to making the change here. If we generally want to move to a style where `auto` 
is always accompanied by an explicit `const` when `const` would be deduced (in 
much the same way that we already expect that `auto` be accompanied by an 
explicit `*` when a pointer type would be deduced), I'd be OK with that -- but 
we should discuss that somewhere more visible than this review thread and 
include it in our general coding guidelines.



Comment at: lib/AST/Type.cpp:3215
   }
-  llvm_unreachable("bad attributed type kind");
 }

aaron.ballman wrote:
> Probably need to keep this to prevent MSVC from barking about not all control 
> paths returning a value.
There's a `default:` case now, so that shouldn't be a problem. (Removing the 
exhaustive list is intended to be temporary; I'd like to move to generating 
this with TableGen sooner rather than later.)



Comment at: lib/Sema/SemaType.cpp:178-180
+// their TypeLocs makes it hard to correctly assign these. We use the
+// keep the attributes in creation order as an attempt to make them
+// line up properly.

aaron.ballman wrote:
> "We use the keep the attributes" isn't grammatical. Should it be, "We keep 
> the attributes in creation order" instead?
Hah, oops, yes.



Comment at: lib/Sema/SemaType.cpp:181
+// line up properly.
+using TypeAttr = std::pair;
+SmallVector TypeAttrs;

aaron.ballman wrote:
> It's unfortunate to use the name `TypeAttr` here -- unqualified uses of the 
> type name, like below, makes it look like it might be the `TypeAttr` from 
> Attr.h
Good point. I went with `TypeAttrPair` here, and `AttrsForTypes` in the vector.



Comment at: lib/Sema/SemaType.cpp:3884
 
+template
+static AttrT *createSimpleAttr(ASTContext , ParsedAttr ) {

aaron.ballman wrote:
> Did clang-format do this? It seems like it's not formatted how I'd expect.
How would you expect it? clang-format puts a space after the `template` 
keyword, unfortunately, and IIRC can't be configured to not do so. Though as a 
consequence, it looks like the space is now more common in clang code by a 2:1 
ratio despite being "clearly wrong" ;-(



Comment at: lib/Sema/SemaType.cpp:6360
+  }
+}
+

aaron.ballman wrote:
> MSVC may complain about not all control paths returning a value here.
I'm confident that this pattern is fine; we use it all over the place. MSVC 
knows that control flow cannot leave an `llvm_unreachable(...)`.



Comment at: lib/Serialization/ASTReaderDecl.cpp:2707
 void ASTReader::ReadAttributes(ASTRecordReader , AttrVec ) {
-  for (unsigned i = 0, e = Record.readInt(); i != e; ++i) {
-Attr *New = nullptr;
-auto Kind = (attr::Kind)Record.readInt();
-SourceRange Range = Record.readSourceRange();
-ASTContext  = getContext();
-
-#include "clang/Serialization/AttrPCHRead.inc"
-
-assert(New && "Unable to decode attribute?");
-Attrs.push_back(New);
-  }
+  for (unsigned i = 0, e = Record.readInt(); i != e; 

[PATCH] D50526: Model type attributes as regular Attrs

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

rsmith updated this revision to Diff 160207.
rsmith marked 10 inline comments as done.

https://reviews.llvm.org/D50526

Files:
  include/clang/AST/ASTContext.h
  include/clang/AST/Attr.h
  include/clang/AST/Type.h
  include/clang/AST/TypeLoc.h
  include/clang/Basic/Attr.td
  include/clang/Sema/Sema.h
  include/clang/Serialization/ASTReader.h
  include/clang/Serialization/ASTWriter.h
  lib/ARCMigrate/TransGCAttrs.cpp
  lib/ARCMigrate/Transforms.cpp
  lib/AST/ASTContext.cpp
  lib/AST/Type.cpp
  lib/AST/TypeLoc.cpp
  lib/AST/TypePrinter.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaInit.cpp
  lib/Sema/SemaObjCProperty.cpp
  lib/Sema/SemaType.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriter.cpp
  lib/StaticAnalyzer/Core/CheckerHelpers.cpp
  utils/TableGen/ClangAttrEmitter.cpp

Index: utils/TableGen/ClangAttrEmitter.cpp
===
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -2470,8 +2470,10 @@
 
 static const AttrClassDescriptor AttrClassDescriptors[] = {
   { "ATTR", "Attr" },
+  { "TYPE_ATTR", "TypeAttr" },
   { "STMT_ATTR", "StmtAttr" },
   { "INHERITABLE_ATTR", "InheritableAttr" },
+  { "DECL_OR_TYPE_ATTR", "DeclOrTypeAttr" },
   { "INHERITABLE_PARAM_ATTR", "InheritableParamAttr" },
   { "PARAMETER_ABI_ATTR", "ParameterABIAttr" }
 };
Index: lib/StaticAnalyzer/Core/CheckerHelpers.cpp
===
--- lib/StaticAnalyzer/Core/CheckerHelpers.cpp
+++ lib/StaticAnalyzer/Core/CheckerHelpers.cpp
@@ -103,9 +103,9 @@
   const auto *AttrType = Type->getAs();
   if (!AttrType)
 return Nullability::Unspecified;
-  if (AttrType->getAttrKind() == AttributedType::attr_nullable)
+  if (AttrType->getAttrKind() == attr::TypeNullable)
 return Nullability::Nullable;
-  else if (AttrType->getAttrKind() == AttributedType::attr_nonnull)
+  else if (AttrType->getAttrKind() == attr::TypeNonNull)
 return Nullability::Nonnull;
   return Nullability::Unspecified;
 }
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -770,19 +770,7 @@
 }
 
 void TypeLocWriter::VisitAttributedTypeLoc(AttributedTypeLoc TL) {
-  Record.AddSourceLocation(TL.getAttrNameLoc());
-  if (TL.hasAttrOperand()) {
-SourceRange range = TL.getAttrOperandParensRange();
-Record.AddSourceLocation(range.getBegin());
-Record.AddSourceLocation(range.getEnd());
-  }
-  if (TL.hasAttrExprOperand()) {
-Expr *operand = TL.getAttrExprOperand();
-Record.push_back(operand ? 1 : 0);
-if (operand) Record.AddStmt(operand);
-  } else if (TL.hasAttrEnumOperand()) {
-Record.AddSourceLocation(TL.getAttrEnumOperandLoc());
-  }
+  Record.AddAttr(TL.getAttr());
 }
 
 void TypeLocWriter::VisitTemplateTypeParmTypeLoc(TemplateTypeParmTypeLoc TL) {
@@ -4481,16 +4469,21 @@
 // General Serialization Routines
 //===--===//
 
-/// Emit the list of attributes to the specified record.
-void ASTRecordWriter::AddAttributes(ArrayRef Attrs) {
+void ASTRecordWriter::AddAttr(const Attr *A) {
   auto  = *this;
-  Record.push_back(Attrs.size());
-  for (const auto *A : Attrs) {
-Record.push_back(A->getKind()); // FIXME: stable encoding, target attrs
-Record.AddSourceRange(A->getRange());
+  if (!A)
+return Record.push_back(0);
+  Record.push_back(A->getKind() + 1); // FIXME: stable encoding, target attrs
+  Record.AddSourceRange(A->getRange());
 
 #include "clang/Serialization/AttrPCHWrite.inc"
-  }
+}
+
+/// Emit the list of attributes to the specified record.
+void ASTRecordWriter::AddAttributes(ArrayRef Attrs) {
+  push_back(Attrs.size());
+  for (const auto *A : Attrs)
+AddAttr(A);
 }
 
 void ASTWriter::AddToken(const Token , RecordDataImpl ) {
Index: lib/Serialization/ASTReaderDecl.cpp
===
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -2642,19 +2642,72 @@
 // Attribute Reading
 //===--===//
 
+namespace {
+class AttrReader {
+  ModuleFile *F;
+  ASTReader *Reader;
+  const ASTReader::RecordData 
+  unsigned 
+
+public:
+  AttrReader(ModuleFile , ASTReader ,
+ const ASTReader::RecordData , unsigned )
+  : F(), Reader(), Record(Record), Idx(Idx) {}
+
+  const uint64_t () { return Record[Idx++]; }
+
+  SourceRange readSourceRange() {
+return Reader->ReadSourceRange(*F, Record, Idx);
+  }
+
+  Expr *readExpr() { return Reader->ReadExpr(*F); }
+
+  std::string readString() {
+return Reader->ReadString(Record, Idx);
+  }
+
+  TypeSourceInfo *getTypeSourceInfo() {
+return Reader->GetTypeSourceInfo(*F, Record, 

[PATCH] D49570: [analyzer] Improve warning messages and notes of InnerPointerChecker

[Artem Dergachev via Phabricator via cfe-commits]

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

I think this is way easier to understand :)


https://reviews.llvm.org/D49570



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

[PATCH] D49570: [analyzer] Improve warning messages and notes of DanglingInternalBufferChecker

[Reka Kovacs via Phabricator via cfe-commits]

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



Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:253
+  allocation_state::getContainerObjRegion(N->getState(), PtrToBuf);
+  const auto *TypedRegion = dyn_cast(ObjRegion);
+  QualType ObjTy = TypedRegion->getValueType();

NoQ wrote:
> `dyn_cast` may fail by returning a null pointer. This either needs to be 
> changed to `cast` or there needs to be a check for a null pointer before use. 
> I guess it should be a `cast` because you're only acting on typed regions in 
> the checker itself.
I hope that in a few more reviews I'll learn to use all of these properly.



Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2931-2932
+OS << "deallocated by call to destructor";
+StackHint = new StackHintGeneratorForSymbol(Sym,
+  "Returning; inner buffer was 
deallocated");
   } else {

NoQ wrote:
> Cool stuff!
:)



Comment at: test/Analysis/dangling-internal-buffer.cpp:63
 // expected-note@-4 {{Taking false branch}}
-consume(c); // expected-warning {{Use of memory after it is freed}}
-// expected-note@-1 {{Use of memory after it is freed}}
+consume(c); // expected-warning {{Deallocated pointer returned to the 
caller}}
+// expected-note@-1 {{Deallocated pointer returned to the caller}}

NoQ wrote:
> Mm, nono, there's no `return` statement here, so we shouldn't say that our 
> pointer is returned to the caller. Whether it should say "returned to the 
> caller" or not, should not depend on the allocation family, but on the kind 
> of "use" we encounter "after" "free".
I don't even know how this went so off, sorry.


https://reviews.llvm.org/D49570



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

[PATCH] D49570: [analyzer] Improve warning messages and notes of DanglingInternalBufferChecker

[Reka Kovacs via Phabricator via cfe-commits]

rnkovacs updated this revision to Diff 160193.
rnkovacs marked 3 inline comments as done.
rnkovacs added a comment.

Address comments & rebase.


https://reviews.llvm.org/D49570

Files:
  lib/StaticAnalyzer/Checkers/AllocationState.h
  lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  test/Analysis/inner-pointer.cpp

Index: test/Analysis/inner-pointer.cpp
===
--- test/Analysis/inner-pointer.cpp
+++ test/Analysis/inner-pointer.cpp
@@ -65,106 +65,106 @@
   const char *c, *d;
   {
 std::string s;
-c = s.c_str(); // expected-note {{Dangling inner pointer obtained here}}
-d = s.data();  // expected-note {{Dangling inner pointer obtained here}}
-  }// expected-note {{Inner pointer invalidated by call to destructor}}
-  // expected-note@-1 {{Inner pointer invalidated by call to destructor}}
+c = s.c_str(); // expected-note {{Pointer to inner buffer of 'std::string' obtained here}}
+d = s.data();  // expected-note {{Pointer to inner buffer of 'std::string' obtained here}}
+  }// expected-note {{Inner buffer of 'std::string' deallocated by call to destructor}}
+  // expected-note@-1 {{Inner buffer of 'std::string' deallocated by call to destructor}}
   std::string s;
   const char *c2 = s.c_str();
   if (cond) {
 // expected-note@-1 {{Assuming 'cond' is not equal to 0}}
 // expected-note@-2 {{Taking true branch}}
 // expected-note@-3 {{Assuming 'cond' is 0}}
 // expected-note@-4 {{Taking false branch}}
-consume(c); // expected-warning {{Use of memory after it is freed}}
-// expected-note@-1 {{Use of memory after it is freed}}
+consume(c); // expected-warning {{Inner pointer of container used after re/deallocation}}
+// expected-note@-1 {{Inner pointer of container used after re/deallocation}}
   } else {
-consume(d); // expected-warning {{Use of memory after it is freed}}
-// expected-note@-1 {{Use of memory after it is freed}}
+consume(d); // expected-warning {{Inner pointer of container used after re/deallocation}}
+// expected-note@-1 {{Inner pointer of container used after re/deallocation}}
   }
 }
 
 void deref_after_scope_char_data_non_const() {
   char *c;
   {
 std::string s;
-c = s.data(); // expected-note {{Dangling inner pointer obtained here}}
-  }   // expected-note {{Inner pointer invalidated by call to destructor}}
+c = s.data(); // expected-note {{Pointer to inner buffer of 'std::string' obtained here}}
+  }   // expected-note {{Inner buffer of 'std::string' deallocated by call to destructor}}
   std::string s;
   char *c2 = s.data();
-  consume(c); // expected-warning {{Use of memory after it is freed}}
-  // expected-note@-1 {{Use of memory after it is freed}}
+  consume(c); // expected-warning {{Inner pointer of container used after re/deallocation}}
+  // expected-note@-1 {{Inner pointer of container used after re/deallocation}}
 }
 
 void deref_after_scope_wchar_t(bool cond) {
   const wchar_t *c, *d;
   {
 std::wstring s;
-c = s.c_str(); // expected-note {{Dangling inner pointer obtained here}}
-d = s.data();  // expected-note {{Dangling inner pointer obtained here}}
-  }// expected-note {{Inner pointer invalidated by call to destructor}}
-  // expected-note@-1 {{Inner pointer invalidated by call to destructor}}
+c = s.c_str(); // expected-note {{Pointer to inner buffer of 'std::wstring' obtained here}}
+d = s.data();  // expected-note {{Pointer to inner buffer of 'std::wstring' obtained here}}
+  }// expected-note {{Inner buffer of 'std::wstring' deallocated by call to destructor}}
+  // expected-note@-1 {{Inner buffer of 'std::wstring' deallocated by call to destructor}}
   std::wstring s;
   const wchar_t *c2 = s.c_str();
   if (cond) {
 // expected-note@-1 {{Assuming 'cond' is not equal to 0}}
 // expected-note@-2 {{Taking true branch}}
 // expected-note@-3 {{Assuming 'cond' is 0}}
 // expected-note@-4 {{Taking false branch}}
-consume(c); // expected-warning {{Use of memory after it is freed}}
-// expected-note@-1 {{Use of memory after it is freed}}
+consume(c); // expected-warning {{Inner pointer of container used after re/deallocation}}
+// expected-note@-1 {{Inner pointer of container used after re/deallocation}}
   } else {
-consume(d); // expected-warning {{Use of memory after it is freed}}
-// expected-note@-1 {{Use of memory after it is freed}}
+consume(d); // expected-warning {{Inner pointer of container used after re/deallocation}}
+// expected-note@-1 {{Inner pointer of container used after re/deallocation}}
   }
 }
 
 void deref_after_scope_char16_t_cstr() {
   const char16_t *c16;
   {
 std::u16string s16;
-c16 = s16.c_str(); // expected-note {{Dangling inner pointer obtained here}}
-  }// expected-note 

[PATCH] D50564: Add support for SEH unwinding on Windows.

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: src/libunwind_ext.h:43
+  #if defined(__x86_64__) && !defined(__MINGW64__)
+typedef struct _DISPATCHER_CONTEXT {
+  ULONG64 ControlPc;

cdavis5x wrote:
> mstorsjo wrote:
> > What's this all about? winnt.h (from both MSVC and mingw-w64) should define 
> > this struct, no?
> Not my copy of `` from the Win7 SDK. Dunno about WIn8 and WIn10.
If we have to pick between SDK versions to support, I'd much prefer to support 
8+.


Repository:
  rUNW libunwind

https://reviews.llvm.org/D50564



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

[PATCH] D50564: Add support for SEH unwinding on Windows.

[Charles Davis via Phabricator via cfe-commits]

cdavis5x added a comment.

In https://reviews.llvm.org/D50564#1195794, @mstorsjo wrote:

> > Special thanks to KJK::Hyperion for his excellent series of articles on how 
> > EH works on x86-64 Windows. (Seriously, check it out. It's awesome.)
>
> Can you give some links to it? A brief googling didn't turn up much else than 
> the PSEH library.


I can't seem to find it either. I wonder if it's vanished from the Internet. 
Maybe the IA will have something.


Repository:
  rUNW libunwind

https://reviews.llvm.org/D50564



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

[PATCH] D48687: [clangd] Avoid duplicates in findDefinitions response

[Simon Marchi via Phabricator via cfe-commits]

simark added inline comments.



Comment at: clangd/SourceCode.h:65
 
 /// Get the absolute file path of a given file entry.
 TextEdit toTextEdit(const FixItHint , const SourceManager ,

hokein wrote:
> nit: this comment is not needed.
Woops, merge mistake.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48687



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

[PATCH] D48687: [clangd] Avoid duplicates in findDefinitions response

[Simon Marchi via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE339483: [clangd] Avoid duplicates in findDefinitions 
response (authored by simark, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D48687?vs=160175=160197#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48687

Files:
  clangd/FindSymbols.cpp
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  clangd/XRefs.cpp
  unittests/clangd/TestFS.cpp
  unittests/clangd/TestFS.h
  unittests/clangd/XRefsTests.cpp

Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -312,27 +312,65 @@
 }
 
 TEST(GoToDefinition, RelPathsInCompileCommand) {
+  // The source is in "/clangd-test/src".
+  // We build in "/clangd-test/build".
+
   Annotations SourceAnnotations(R"cpp(
+#include "header_in_preamble.h"
 int [[foo]];
-int baz = f^oo;
+#include "header_not_in_preamble.h"
+int baz = f$p1^oo + bar_pre$p2^amble + bar_not_pre$p3^amble;
+)cpp");
+
+  Annotations HeaderInPreambleAnnotations(R"cpp(
+int [[bar_preamble]];
+)cpp");
+
+  Annotations HeaderNotInPreambleAnnotations(R"cpp(
+int [[bar_not_preamble]];
 )cpp");
 
+  // Make the compilation paths appear as ../src/foo.cpp in the compile
+  // commands.
+  SmallString<32> RelPathPrefix("..");
+  llvm::sys::path::append(RelPathPrefix, "src");
+  std::string BuildDir = testPath("build");
+  MockCompilationDatabase CDB(BuildDir, RelPathPrefix);
+
   IgnoreDiagnostics DiagConsumer;
-  MockCompilationDatabase CDB(/*UseRelPaths=*/true);
   MockFSProvider FS;
   ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
 
-  auto FooCpp = testPath("foo.cpp");
+  // Fill the filesystem.
+  auto FooCpp = testPath("src/foo.cpp");
   FS.Files[FooCpp] = "";
+  auto HeaderInPreambleH = testPath("src/header_in_preamble.h");
+  FS.Files[HeaderInPreambleH] = HeaderInPreambleAnnotations.code();
+  auto HeaderNotInPreambleH = testPath("src/header_not_in_preamble.h");
+  FS.Files[HeaderNotInPreambleH] = HeaderNotInPreambleAnnotations.code();
 
-  Server.addDocument(FooCpp, SourceAnnotations.code());
   runAddDocument(Server, FooCpp, SourceAnnotations.code());
+
+  // Go to a definition in main source file.
   auto Locations =
-  runFindDefinitions(Server, FooCpp, SourceAnnotations.point());
+  runFindDefinitions(Server, FooCpp, SourceAnnotations.point("p1"));
   EXPECT_TRUE(bool(Locations)) << "findDefinitions returned an error";
-
   EXPECT_THAT(*Locations, ElementsAre(Location{URIForFile{FooCpp},
SourceAnnotations.range()}));
+
+  // Go to a definition in header_in_preamble.h.
+  Locations = runFindDefinitions(Server, FooCpp, SourceAnnotations.point("p2"));
+  EXPECT_TRUE(bool(Locations)) << "findDefinitions returned an error";
+  EXPECT_THAT(*Locations,
+  ElementsAre(Location{URIForFile{HeaderInPreambleH},
+   HeaderInPreambleAnnotations.range()}));
+
+  // Go to a definition in header_not_in_preamble.h.
+  Locations = runFindDefinitions(Server, FooCpp, SourceAnnotations.point("p3"));
+  EXPECT_TRUE(bool(Locations)) << "findDefinitions returned an error";
+  EXPECT_THAT(*Locations,
+  ElementsAre(Location{URIForFile{HeaderNotInPreambleH},
+   HeaderNotInPreambleAnnotations.range()}));
 }
 
 TEST(Hover, All) {
Index: unittests/clangd/TestFS.cpp
===
--- unittests/clangd/TestFS.cpp
+++ unittests/clangd/TestFS.cpp
@@ -32,22 +32,36 @@
   return MemFS;
 }
 
-MockCompilationDatabase::MockCompilationDatabase(bool UseRelPaths)
-: ExtraClangFlags({"-ffreestanding"}), UseRelPaths(UseRelPaths) {
+MockCompilationDatabase::MockCompilationDatabase(StringRef Directory,
+ StringRef RelPathPrefix)
+: ExtraClangFlags({"-ffreestanding"}), Directory(Directory),
+  RelPathPrefix(RelPathPrefix) {
   // -ffreestanding avoids implicit stdc-predef.h.
 }
 
 Optional
 MockCompilationDatabase::getCompileCommand(PathRef File) const {
   if (ExtraClangFlags.empty())
 return None;
 
-  auto CommandLine = ExtraClangFlags;
   auto FileName = sys::path::filename(File);
+
+  // Build the compile command.
+  auto CommandLine = ExtraClangFlags;
   CommandLine.insert(CommandLine.begin(), "clang");
-  CommandLine.insert(CommandLine.end(), UseRelPaths ? FileName : File);
-  return {tooling::CompileCommand(sys::path::parent_path(File), FileName,
-  std::move(CommandLine), "")};
+  if (RelPathPrefix.empty()) {
+// Use the absolute path in the compile command.
+CommandLine.push_back(File);
+  } else {
+// Build a relative path using RelPathPrefix.
+SmallString<32> RelativeFilePath(RelPathPrefix);
+llvm::sys::path::append(RelativeFilePath, FileName);

[clang-tools-extra] r339483 - [clangd] Avoid duplicates in findDefinitions response

[Simon Marchi via cfe-commits]

Author: simark
Date: Fri Aug 10 15:27:53 2018
New Revision: 339483

URL: http://llvm.org/viewvc/llvm-project?rev=339483=rev
Log:
[clangd] Avoid duplicates in findDefinitions response

Summary:
When compile_commands.json contains some source files expressed as
relative paths, we can get duplicate responses to findDefinitions.  The
responses only differ by the URI, which are different versions of the
same file:

"result": [
{
...
"uri": 
"file:///home/emaisin/src/ls-interact/cpp-test/build/../src/first.h"
},
{
...
"uri": "file:///home/emaisin/src/ls-interact/cpp-test/src/first.h"
}
]

In getAbsoluteFilePath, we try to obtain the realpath of the FileEntry
by calling tryGetRealPathName.  However, this can fail and return an
empty string.  It may be bug a bug in clang, but in any case we should
fall back to computing it ourselves if it happens.

I changed getAbsoluteFilePath so that if tryGetRealPathName succeeds, we
return right away (a real path is always absolute).  Otherwise, we try
to build an absolute path, as we did before, but we also call
VFS->getRealPath to make sure to get the canonical path (e.g. without
any ".." in it).

Reviewers: malaperle

Subscribers: hokein, ilya-biryukov, ioeric, MaskRay, jkorous, cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/FindSymbols.cpp
clang-tools-extra/trunk/clangd/SourceCode.cpp
clang-tools-extra/trunk/clangd/SourceCode.h
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/unittests/clangd/TestFS.cpp
clang-tools-extra/trunk/unittests/clangd/TestFS.h
clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp

Modified: clang-tools-extra/trunk/clangd/FindSymbols.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/FindSymbols.cpp?rev=339483=339482=339483=diff
==
--- clang-tools-extra/trunk/clangd/FindSymbols.cpp (original)
+++ clang-tools-extra/trunk/clangd/FindSymbols.cpp Fri Aug 10 15:27:53 2018
@@ -193,7 +193,7 @@ public:
 const FileEntry *F = SM.getFileEntryForID(SM.getMainFileID());
 if (!F)
   return;
-auto FilePath = getAbsoluteFilePath(F, SM);
+auto FilePath = getRealPath(F, SM);
 if (FilePath)
   MainFileUri = URIForFile(*FilePath);
   }

Modified: clang-tools-extra/trunk/clangd/SourceCode.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SourceCode.cpp?rev=339483=339482=339483=diff
==
--- clang-tools-extra/trunk/clangd/SourceCode.cpp (original)
+++ clang-tools-extra/trunk/clangd/SourceCode.cpp Fri Aug 10 15:27:53 2018
@@ -185,18 +185,34 @@ std::vector replacementsToEdit
   return Edits;
 }
 
-llvm::Optional
-getAbsoluteFilePath(const FileEntry *F, const SourceManager ) {
-  SmallString<64> FilePath = F->tryGetRealPathName();
-  if (FilePath.empty())
-FilePath = F->getName();
-  if (!llvm::sys::path::is_absolute(FilePath)) {
-if (!SourceMgr.getFileManager().makeAbsolutePath(FilePath)) {
-  log("Could not turn relative path to absolute: {0}", FilePath);
+llvm::Optional getRealPath(const FileEntry *F,
+const SourceManager ) {
+  // Ideally, we get the real path from the FileEntry object.
+  SmallString<128> FilePath = F->tryGetRealPathName();
+  if (!FilePath.empty()) {
+return FilePath.str().str();
+  }
+
+  // Otherwise, we try to compute ourselves.
+  vlog("FileEntry for {0} did not contain the real path.", F->getName());
+
+  llvm::SmallString<128> Path = F->getName();
+
+  if (!llvm::sys::path::is_absolute(Path)) {
+if (!SourceMgr.getFileManager().makeAbsolutePath(Path)) {
+  log("Could not turn relative path to absolute: {0}", Path);
   return llvm::None;
 }
   }
-  return FilePath.str().str();
+
+  llvm::SmallString<128> RealPath;
+  if (SourceMgr.getFileManager().getVirtualFileSystem()->getRealPath(
+  Path, RealPath)) {
+log("Could not compute real path: {0}", Path);
+return Path.str().str();
+  }
+
+  return RealPath.str().str();
 }
 
 TextEdit toTextEdit(const FixItHint , const SourceManager ,

Modified: clang-tools-extra/trunk/clangd/SourceCode.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SourceCode.h?rev=339483=339482=339483=diff
==
--- clang-tools-extra/trunk/clangd/SourceCode.h (original)
+++ clang-tools-extra/trunk/clangd/SourceCode.h Fri Aug 10 15:27:53 2018
@@ -62,13 +62,20 @@ TextEdit replacementToEdit(StringRef Cod
 std::vector replacementsToEdits(StringRef Code,
   const tooling::Replacements );
 
-/// Get the absolute file path of a given file entry.
-llvm::Optional getAbsoluteFilePath(const 

[PATCH] D50124: [analyzer] Record nullability implications on getting items from NSDictionary

[George Karpenkov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC339482: [analyzer] Record nullability implications on 
getting items from NSDictionary (authored by george.karpenkov, committed by ).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D50124?vs=160188=160196#toc

Repository:
  rC Clang

https://reviews.llvm.org/D50124

Files:
  lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp
  test/Analysis/Inputs/system-header-simulator-for-nullability.h
  test/Analysis/trustnonnullchecker_test.m

Index: test/Analysis/Inputs/system-header-simulator-for-nullability.h
===
--- test/Analysis/Inputs/system-header-simulator-for-nullability.h
+++ test/Analysis/Inputs/system-header-simulator-for-nullability.h
@@ -9,6 +9,8 @@
 NS_ASSUME_NONNULL_BEGIN
 
 typedef struct _NSZone NSZone;
+typedef unsigned long NSUInteger;
+@class NSCoder, NSEnumerator;
 
 @protocol NSObject
 + (instancetype)alloc;
@@ -24,6 +26,22 @@
 - (id)mutableCopyWithZone:(nullable NSZone *)zone;
 @end
 
+@protocol NSCoding
+- (void)encodeWithCoder:(NSCoder *)aCoder;
+@end
+
+@protocol NSSecureCoding 
+@required
++ (BOOL)supportsSecureCoding;
+@end
+
+typedef struct {
+  unsigned long state;
+  id *itemsPtr;
+  unsigned long *mutationsPtr;
+  unsigned long extra[5];
+} NSFastEnumerationState;
+
 __attribute__((objc_root_class))
 @interface
 NSObject
@@ -52,3 +70,36 @@
 @end
 
 NS_ASSUME_NONNULL_END
+
+@interface NSDictionary : NSObject 
+
+- (NSUInteger)count;
+- (id)objectForKey:(id)aKey;
+- (NSEnumerator *)keyEnumerator;
+- (id)objectForKeyedSubscript:(id)aKey;
+
+@end
+
+@interface NSDictionary (NSDictionaryCreation)
+
++ (id)dictionary;
++ (id)dictionaryWithObject:(id)object forKey:(id )key;
++ (instancetype)dictionaryWithObjects:(const id [])objects forKeys:(const id  [])keys count:(NSUInteger)cnt;
+
+@end
+
+@interface NSMutableDictionary : NSDictionary
+
+- (void)removeObjectForKey:(id)aKey;
+- (void)setObject:(id)anObject forKey:(id )aKey;
+
+@end
+
+@interface NSMutableDictionary (NSExtendedMutableDictionary)
+
+- (void)addEntriesFromDictionary:(NSDictionary *)otherDictionary;
+- (void)removeAllObjects;
+- (void)setDictionary:(NSDictionary *)otherDictionary;
+- (void)setObject:(id)obj forKeyedSubscript:(id )key __attribute__((availability(macosx,introduced=10.8)));
+
+@end
Index: test/Analysis/trustnonnullchecker_test.m
===
--- test/Analysis/trustnonnullchecker_test.m
+++ test/Analysis/trustnonnullchecker_test.m
@@ -1,7 +1,9 @@
-// RUN: %clang_analyze_cc1 -fblocks -analyze -analyzer-checker=core,nullability,apiModeling  -verify %s
+// RUN: %clang_analyze_cc1 -fblocks -analyze -analyzer-checker=core,nullability,apiModeling,debug.ExprInspection  -verify %s
 
 #include "Inputs/system-header-simulator-for-nullability.h"
 
+void clang_analyzer_warnIfReached();
+
 NSString* _Nonnull trust_nonnull_framework_annotation() {
   NSString* out = [NSString generateString];
   if (out) {}
@@ -67,3 +69,104 @@
   return out; // expected-warning{{}}
 }
 
+// If the return value is non-nil, the index is non-nil.
+NSString *_Nonnull retImpliesIndex(NSString *s,
+   NSDictionary *dic) {
+  id obj = dic[s];
+  if (s) {}
+  if (obj)
+return s; // no-warning
+  return @"foo";
+}
+
+NSString *_Nonnull retImpliesIndexOtherMethod(NSString *s,
+   NSDictionary *dic) {
+  id obj = [dic objectForKey:s];
+  if (s) {}
+  if (obj)
+return s; // no-warning
+  return @"foo";
+}
+
+NSString *_Nonnull retImpliesIndexOnRHS(NSString *s,
+NSDictionary *dic) {
+  id obj = dic[s];
+  if (s) {}
+  if (nil != obj)
+return s; // no-warning
+  return @"foo";
+}
+
+NSString *_Nonnull retImpliesIndexReverseCheck(NSString *s,
+   NSDictionary *dic) {
+  id obj = dic[s];
+  if (s) {}
+  if (!obj)
+return @"foo";
+  return s; // no-warning
+}
+
+NSString *_Nonnull retImpliesIndexReverseCheckOnRHS(NSString *s,
+NSDictionary *dic) {
+  id obj = dic[s];
+  if (s) {}
+  if (nil == obj)
+return @"foo";
+  return s; // no-warning
+}
+
+NSString *_Nonnull retImpliesIndexWrongBranch(NSString *s,
+  NSDictionary *dic) {
+  id obj = dic[s];
+  if (s) {}
+  if (!obj)
+return s; // expected-warning{{}}
+  return @"foo";
+}
+
+NSString *_Nonnull retImpliesIndexWrongBranchOnRHS(NSString *s,
+   NSDictionary *dic) {
+  id obj = dic[s];
+  if (s) {}
+  if (nil == obj)
+return s; // expected-warning{{}}
+  return @"foo";
+}
+
+// The return value could still be nil for a non-nil index.
+NSDictionary *_Nonnull indexDoesNotImplyRet(NSString *s,
+

r339482 - [analyzer] Record nullability implications on getting items from NSDictionary

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Fri Aug 10 15:27:04 2018
New Revision: 339482

URL: http://llvm.org/viewvc/llvm-project?rev=339482=rev
Log:
[analyzer] Record nullability implications on getting items from NSDictionary

If we get an item from a dictionary, we know that the item is non-null
if and only if the key is non-null.

This patch is a rather hacky way to record this implication, because
some logic needs to be duplicated from the solver.
And yet, it's pretty simple, performant, and works.

Other possible approaches:

 - Record the implication, in future rely on Z3 to pick it up.
 - Generalize the current code and move it to the constraint manager.

rdar://34990742

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp
cfe/trunk/test/Analysis/Inputs/system-header-simulator-for-nullability.h
cfe/trunk/test/Analysis/trustnonnullchecker_test.m

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp?rev=339482=339481=339482=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp Fri Aug 10 
15:27:04 2018
@@ -1,4 +1,4 @@
-//== TrustNonnullChecker.cpp - Checker for trusting annotations -*- C++ 
-*--==//
+//== TrustNonnullChecker.cpp - API nullability modeling -*- C++ 
-*--==//
 //
 // The LLVM Compiler Infrastructure
 //
@@ -7,12 +7,20 @@
 //
 
//===--===//
 //
-// This checker adds an assumption that methods annotated with _Nonnull
+// This checker adds nullability-related assumptions:
+//
+// 1. Methods annotated with _Nonnull
 // which come from system headers actually return a non-null pointer.
 //
+// 2. NSDictionary key is non-null after the keyword subscript operation
+// on read if and only if the resulting expression is non-null.
+//
+// 3. NSMutableDictionary index is non-null after a write operation.
+//
 
//===--===//
 
 #include "ClangSACheckers.h"
+#include "SelectorExtras.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
@@ -22,10 +30,129 @@
 using namespace clang;
 using namespace ento;
 
+/// Records implications between symbols.
+/// The semantics is:
+///(antecedent != 0) => (consequent != 0)
+/// These implications are then read during the evaluation of the assumption,
+/// and the appropriate antecedents are applied.
+REGISTER_MAP_WITH_PROGRAMSTATE(NonNullImplicationMap, SymbolRef, SymbolRef)
+
+/// The semantics is:
+///(antecedent == 0) => (consequent == 0)
+REGISTER_MAP_WITH_PROGRAMSTATE(NullImplicationMap, SymbolRef, SymbolRef)
+
 namespace {
 
-class TrustNonnullChecker : public Checker {
+class TrustNonnullChecker : public Checker {
+  // Do not try to iterate over symbols with higher complexity.
+  static unsigned constexpr ComplexityThreshold = 10;
+  Selector ObjectForKeyedSubscriptSel;
+  Selector ObjectForKeySel;
+  Selector SetObjectForKeyedSubscriptSel;
+  Selector SetObjectForKeySel;
+
+public:
+  TrustNonnullChecker(ASTContext )
+  : ObjectForKeyedSubscriptSel(
+getKeywordSelector(Ctx, "objectForKeyedSubscript")),
+ObjectForKeySel(getKeywordSelector(Ctx, "objectForKey")),
+SetObjectForKeyedSubscriptSel(
+getKeywordSelector(Ctx, "setObject", "forKeyedSubscript")),
+SetObjectForKeySel(getKeywordSelector(Ctx, "setObject", "forKey")) {}
+
+  ProgramStateRef evalAssume(ProgramStateRef State,
+ SVal Cond,
+ bool Assumption) const {
+const SymbolRef CondS = Cond.getAsSymbol();
+if (!CondS || CondS->computeComplexity() > ComplexityThreshold)
+  return State;
+
+for (auto B=CondS->symbol_begin(), E=CondS->symbol_end(); B != E; ++B) {
+  const SymbolRef Antecedent = *B;
+  State = addImplication(Antecedent, State, true);
+  State = addImplication(Antecedent, State, false);
+}
+
+return State;
+  }
+
+  void checkPostCall(const CallEvent , CheckerContext ) const {
+// Only trust annotations for system headers for non-protocols.
+if (!Call.isInSystemHeader())
+  return;
+
+ProgramStateRef State = C.getState();
+
+if (isNonNullPtr(Call, C))
+  if (auto L = Call.getReturnValue().getAs())
+State = State->assume(*L, /*Assumption=*/true);
+
+C.addTransition(State);
+  }
+
+  void checkPostObjCMessage(const ObjCMethodCall ,
+CheckerContext ) const {
+const ObjCInterfaceDecl *ID = Msg.getReceiverInterface();
+if (!ID)

[PATCH] D50564: Add support for SEH unwinding on Windows.

[Charles Davis via Phabricator via cfe-commits]

cdavis5x updated this revision to Diff 160195.
cdavis5x added a comment.

- Fix outdated comment.
- Make preprocessor conditional more consistent.
- Make some private functions used only in a single file static.


Repository:
  rUNW libunwind

https://reviews.llvm.org/D50564

Files:
  include/__libunwind_config.h
  src/AddressSpace.hpp
  src/CMakeLists.txt
  src/Unwind-seh.cpp
  src/UnwindCursor.hpp
  src/UnwindLevel1-gcc-ext.c
  src/UnwindLevel1.c
  src/config.h
  src/libunwind_ext.h

Index: src/libunwind_ext.h
===
--- src/libunwind_ext.h
+++ src/libunwind_ext.h
@@ -17,6 +17,11 @@
 #include 
 #include 
 
+#if defined(_LIBUNWIND_SUPPORT_SEH_UNWIND)
+  #include 
+  #include 
+#endif
+
 #define UNW_STEP_SUCCESS 1
 #define UNW_STEP_END 0
 
@@ -33,6 +38,40 @@
 extern void _unw_add_dynamic_fde(unw_word_t fde);
 extern void _unw_remove_dynamic_fde(unw_word_t fde);
 
+#if defined(_LIBUNWIND_SUPPORT_SEH_UNWIND)
+  #if defined(__x86_64__) && !defined(__MINGW64__)
+typedef struct _DISPATCHER_CONTEXT {
+  ULONG64 ControlPc;
+  ULONG64 ImageBase;
+  PRUNTIME_FUNCTION FunctionEntry;
+  ULONG64 EstablisherFrame;
+  ULONG64 TargetIp;
+  PCONTEXT ContextRecord;
+  PEXCEPTION_ROUTINE LanguageHandler;
+  PVOID HandlerData;
+  PUNWIND_HISTORY_TABLE HistoryTable;
+  ULONG ScopeIndex;
+  ULONG Fill0;
+} DISPATCHER_CONTEXT;
+  #elif defined(__arm__)
+typedef struct _DISPATCHER_CONTEXT {
+  ULONG ControlPc;
+  ULONG ImageBase;
+  PRUNTIME_FUNCTION FunctionEntry;
+  ULONG EstablisherFrame;
+  ULONG TargetIp;
+  PCONTEXT ContextRecord;
+  PEXCEPTION_ROUTINE LanguageHandler;
+  PVOID HandlerData;
+  PUNWIND_HISTORY_TABLE HistoryTable;
+  ULONG ScopeIndex;
+  ULONG ControlPcIsUnwound;
+  PKNONVOLATILE_CONTEXT_POINTERS NonVolatileRegisters;
+  ULONG VirtualVfpHead;
+} DISPATCHER_CONTEXT;
+  #endif
+#endif
+
 #if defined(_LIBUNWIND_ARM_EHABI)
 extern const uint32_t* decode_eht_entry(const uint32_t*, size_t*, size_t*);
 extern _Unwind_Reason_Code _Unwind_VRS_Interpret(_Unwind_Context *context,
Index: src/config.h
===
--- src/config.h
+++ src/config.h
@@ -38,7 +38,11 @@
 #define _LIBUNWIND_SUPPORT_DWARF_UNWIND   1
   #endif
 #elif defined(_WIN32)
-  #define _LIBUNWIND_SUPPORT_DWARF_UNWIND 1
+  #ifdef __SEH__
+#define _LIBUNWIND_SUPPORT_SEH_UNWIND 1
+  #else
+#define _LIBUNWIND_SUPPORT_DWARF_UNWIND 1
+  #endif
 #else
   #if defined(__ARM_DWARF_EH__) || !defined(__arm__)
 #define _LIBUNWIND_SUPPORT_DWARF_UNWIND 1
Index: src/UnwindLevel1.c
===
--- src/UnwindLevel1.c
+++ src/UnwindLevel1.c
@@ -32,6 +32,8 @@
 
 #if !defined(_LIBUNWIND_ARM_EHABI) && !defined(__USING_SJLJ_EXCEPTIONS__)
 
+#ifndef _LIBUNWIND_SUPPORT_SEH_UNWIND
+
 static _Unwind_Reason_Code
 unwind_phase1(unw_context_t *uc, unw_cursor_t *cursor, _Unwind_Exception *exception_object) {
   unw_init_local(cursor, uc);
@@ -449,6 +451,7 @@
   return result;
 }
 
+#endif // !_LIBUNWIND_SUPPORT_SEH_UNWIND
 
 /// Called by personality handler during phase 2 if a foreign exception
 // is caught.
Index: src/UnwindLevel1-gcc-ext.c
===
--- src/UnwindLevel1-gcc-ext.c
+++ src/UnwindLevel1-gcc-ext.c
@@ -25,6 +25,10 @@
 
 #if defined(_LIBUNWIND_BUILD_ZERO_COST_APIS)
 
+#if defined(_LIBUNWIND_SUPPORT_SEH_UNWIND)
+#define private_1 private_[0]
+#endif
+
 ///  Called by __cxa_rethrow().
 _LIBUNWIND_EXPORT _Unwind_Reason_Code
 _Unwind_Resume_or_Rethrow(_Unwind_Exception *exception_object) {
Index: src/UnwindCursor.hpp
===
--- src/UnwindCursor.hpp
+++ src/UnwindCursor.hpp
@@ -18,18 +18,40 @@
 #include 
 #include 
 
+#ifdef _WIN32
+  #include 
+#endif
 #ifdef __APPLE__
   #include 
 #endif
 
+#if defined(_LIBUNWIND_SUPPORT_SEH_UNWIND)
+#  ifdef _LIBUNWIND_TARGET_X86_64
+struct UNWIND_INFO {
+  BYTE Version : 3;
+  BYTE Flags : 5;
+  BYTE SizeOfProlog;
+  BYTE CountOfCodes;
+  BYTE FrameRegister : 4;
+  BYTE FrameOffset : 4;
+  WORD UnwindCodes[2];
+};
+#  endif
+
+extern "C" _Unwind_Reason_Code __libunwind_seh_personality(
+int, _Unwind_Action, _Unwind_Exception_Class, _Unwind_Exception *,
+struct _Unwind_Context *);
+
+#endif
+
 #include "config.h"
 
 #include "AddressSpace.hpp"
 #include "CompactUnwinder.hpp"
 #include "config.h"
 #include "DwarfInstructions.hpp"
 #include "EHHeaderParser.hpp"
-#include "libunwind.h"
+#include "libunwind_ext.h"
 #include "Registers.hpp"
 #include "RWMutex.hpp"
 #include "Unwind-EHABI.h"
@@ -412,6 +434,525 @@
 #endif
 };
 
+#if defined(_LIBUNWIND_SUPPORT_SEH_UNWIND)
+
+/// \c UnwindCursor contains all state (including all register values) during
+/// an unwind.  This is normally stack-allocated inside a unw_cursor_t.
+template 
+class UnwindCursor : public AbstractUnwindCursor {
+  typedef typename A::pint_t pint_t;
+public:

[PATCH] D50564: Add support for SEH unwinding on Windows.

[Charles Davis via Phabricator via cfe-commits]

cdavis5x added inline comments.



Comment at: src/Unwind-seh.cpp:53
+
+/// Exception cleanup routine used by \c __libunwind_frame_consolidate to
+/// regain control after handling an SEH exception.

mstorsjo wrote:
> I don't see any `__libunwind_frame_consolidate` anywhere, is this comment 
> outdated?
Why yes it is.



Comment at: src/UnwindLevel1.c:35
 
+#if !_LIBUNWIND_SUPPORT_SEH_UNWIND
+

mstorsjo wrote:
> This probably works, but isn't `#if !defined(_LIBUNWIND_SUPPORT_SEH_UNWIND)` 
> more of the common form?
I think I wrote this before the change to make that so took place, and I forgot 
to update this. Good catch.



Comment at: src/libunwind_ext.h:73
+  #endif
+extern int _unw_init_seh(unw_cursor_t *cursor, CONTEXT *ctx);
+extern DISPATCHER_CONTEXT *_unw_seh_get_disp_ctx(unw_cursor_t *cursor);

mstorsjo wrote:
> These are all both defined and called from Unwind-seh.cpp, so couldn't they 
> just be static functions within there?
Probably. Done.


Repository:
  rUNW libunwind

https://reviews.llvm.org/D50564



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

[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers marked 3 inline comments as done.
nickdesaulniers added a comment.

Thanks for the info, I found 
https://gcc.gnu.org/onlinedocs/cpp/Preprocessor-Output.html helpful.  How does 
this look?


Repository:
  rC Clang

https://reviews.llvm.org/D50467



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

[PATCH] D49240: [libc++] Introduce _LIBCPP_HIDE_FROM_ABI to replace _LIBCPP_INLINE_VISIBILITY

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In https://reviews.llvm.org/D49240#1195733, @ldionne wrote:

> Ah, thanks a lot for taking a look! Yes, this makes a lot of sense, since now 
> we're not inlining everything anymore. So the code size is actually smaller 
> (-9.8%), but there's more symbols because more functions are emitted. In this 
> case, I would say this is expected, if unfortunate. Also, a similar effect 
> would probably be witnessed if Chromium were to change their standard library 
> to libstdc++, for example, since libstdc++ does not abuse inlining like 
> libc++ used to.


I think if we used libstdc++, the situation would be much better because the 
inline functions would be linkonce_odr, and there would be far fewer symbols in 
the symbol table. We'd get code size wins from deduplicating the code, and 
symbol table size wins from dropping duplicate long mangled names.


Repository:
  rCXX libc++

https://reviews.llvm.org/D49240



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

[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 160191.
nickdesaulniers added a comment.

- fix up system macro case and add test coverage for that case.


Repository:
  rC Clang

https://reviews.llvm.org/D50467

Files:
  lib/Sema/SemaChecking.cpp
  test/SemaCXX/warn-float-conversion.cpp

Index: test/SemaCXX/warn-float-conversion.cpp
===
--- test/SemaCXX/warn-float-conversion.cpp
+++ test/SemaCXX/warn-float-conversion.cpp
@@ -41,6 +41,32 @@
   l = ld;  //expected-warning{{conversion}}
 }
 
+void CompoundAssignment() {
+  int x = 3;
+
+  x += 1.234;  //expected-warning{{conversion}}
+  x -= -0.0;  //expected-warning{{conversion}}
+  x *= 1.1f;  //expected-warning{{conversion}}
+  x /= -2.2f;  //expected-warning{{conversion}}
+
+  int y = x += 1.4f;  //expected-warning{{conversion}}
+
+  float z = 1.1f;
+  double w = -2.2;
+
+  y += z + w;  //expected-warning{{conversion}}
+}
+
+# 1 "foo.h" 3
+//  ^ the following text comes from a system header file.
+#define SYSTEM_MACRO_FLOAT(x) do { (x) += 1.1; } while(0)
+# 1 "warn-float-conversion.cpp" 1
+//  ^ start of a new file.
+void SystemMacro() {
+  float x = 0.0f;
+  SYSTEM_MACRO_FLOAT(x);
+}
+
 void Test() {
   int a1 = 10.0/2.0;  //expected-warning{{conversion}}
   int a2 = 1.0/2.0;  //expected-warning{{conversion}}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -10292,33 +10292,6 @@
   DiagnoseImpCast(S, E, E->getType(), T, CContext, diag, pruneControlFlow);
 }
 
-/// Analyze the given compound assignment for the possible losing of
-/// floating-point precision.
-static void AnalyzeCompoundAssignment(Sema , BinaryOperator *E) {
-  assert(isa(E) &&
- "Must be compound assignment operation");
-  // Recurse on the LHS and RHS in here
-  AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc());
-  AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc());
-
-  // Now check the outermost expression
-  const auto *ResultBT = E->getLHS()->getType()->getAs();
-  const auto *RBT = cast(E)
-->getComputationResultType()
-->getAs();
-
-  // If both source and target are floating points.
-  if (ResultBT && ResultBT->isFloatingPoint() && RBT && RBT->isFloatingPoint())
-// Builtin FP kinds are ordered by increasing FP rank.
-if (ResultBT->getKind() < RBT->getKind())
-  // We don't want to warn for system macro.
-  if (!S.SourceMgr.isInSystemMacro(E->getOperatorLoc()))
-// warn about dropping FP rank.
-DiagnoseImpCast(S, E->getRHS(), E->getLHS()->getType(),
-E->getOperatorLoc(),
-diag::warn_impcast_float_result_precision);
-}
-
 /// Diagnose an implicit cast from a floating point value to an integer value.
 static void DiagnoseFloatingImpCast(Sema , Expr *E, QualType T,
 SourceLocation CContext) {
@@ -10421,6 +10394,39 @@
   }
 }
 
+/// Analyze the given compound assignment for the possible losing of
+/// floating-point precision.
+static void AnalyzeCompoundAssignment(Sema , BinaryOperator *E) {
+  assert(isa(E) &&
+ "Must be compound assignment operation");
+  // Recurse on the LHS and RHS in here
+  AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc());
+  AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc());
+
+  // Now check the outermost expression
+  const auto *ResultBT = E->getLHS()->getType()->getAs();
+  const auto *RBT = cast(E)
+->getComputationResultType()
+->getAs();
+
+  // The below checks assume source is floating point.
+  if (!ResultBT || !RBT || !RBT->isFloatingPoint()) return;
+
+  // If source is floating point but target is not.
+  if (!ResultBT->isFloatingPoint())
+return DiagnoseFloatingImpCast(S, E, E->getRHS()->getType(),
+   E->getExprLoc());
+
+  // If both source and target are floating points.
+  // Builtin FP kinds are ordered by increasing FP rank.
+  if (ResultBT->getKind() < RBT->getKind() &&
+  // We don't want to warn for system macro.
+  !S.SourceMgr.isInSystemMacro(E->getOperatorLoc()))
+// warn about dropping FP rank.
+DiagnoseImpCast(S, E->getRHS(), E->getLHS()->getType(), E->getOperatorLoc(),
+diag::warn_impcast_float_result_precision);
+}
+
 static std::string PrettyPrintInRange(const llvm::APSInt ,
   IntRange Range) {
   if (!Range.Width) return "0";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50564: Add support for SEH unwinding on Windows.

[Charles Davis via Phabricator via cfe-commits]

cdavis5x added inline comments.



Comment at: src/libunwind_ext.h:43
+  #if defined(__x86_64__) && !defined(__MINGW64__)
+typedef struct _DISPATCHER_CONTEXT {
+  ULONG64 ControlPc;

mstorsjo wrote:
> What's this all about? winnt.h (from both MSVC and mingw-w64) should define 
> this struct, no?
Not my copy of `` from the Win7 SDK. Dunno about WIn8 and WIn10.


Repository:
  rUNW libunwind

https://reviews.llvm.org/D50564



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

r339476 - [analyzer] Fix tracking expressions through negation operator

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Fri Aug 10 14:42:19 2018
New Revision: 339476

URL: http://llvm.org/viewvc/llvm-project?rev=339476=rev
Log:
[analyzer] Fix tracking expressions through negation operator

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=339476=339475=339476=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Fri Aug 10 
14:42:19 2018
@@ -1560,6 +1560,10 @@ static const Expr *peelOffOuterExpr(cons
 if (const Expr *SubEx = peelOffPointerArithmetic(BO))
   return peelOffOuterExpr(SubEx, N);
 
+  if (auto *UO = dyn_cast(Ex))
+if (UO->getOpcode() == UO_LNot)
+  return peelOffOuterExpr(UO->getSubExpr(), N);
+
   return Ex;
 }
 

Modified: cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.cpp?rev=339476=339475=339476=diff
==
--- cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.cpp (original)
+++ cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.cpp Fri Aug 10 
14:42:19 2018
@@ -357,3 +357,18 @@ int forceElementRegionApperence() {
   return ((HasFieldB*))->x; // expected-warning{{Undefined or garbage value 
returned to caller}}
   // expected-note@-1{{Undefined or garbage value 
returned to caller}}
 }
+
+
+
+struct HasForgottenField {
+  int x;
+  HasForgottenField() {} // expected-note{{Returning without writing to 
'this->x'}}
+};
+
+// Test that tracking across exclamation mark works.
+bool tracksThroughExclamationMark() {
+  HasForgottenField a; // expected-note{{Calling default constructor for 
'HasForgottenField'}}
+   // expected-note@-1{{Returning from default constructor 
for 'HasForgottenField'}}
+  return !a.x; // expected-warning{{Undefined or garbage value returned to 
caller}}
+   // expected-note@-1{{Undefined or garbage value returned to 
caller}}
+}


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

[PATCH] D50545: [analyzer] [NFC] [tests] Move plist-based diagnostics tests to separate files, use diff instead of a FileCheck

[George Karpenkov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC339475: [analyzer] [NFC] [tests] Move plist-based 
diagnostics tests to separate files… (authored by george.karpenkov, committed 
by ).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D50545?vs=160139=160182#toc

Repository:
  rC Clang

https://reviews.llvm.org/D50545

Files:
  test/Analysis/ExpectedOutputs/plists/conditional-path-notes.c.plist
  
test/Analysis/ExpectedOutputs/plists/copypaste/plist-diagnostics-notes-as-events.cpp.plist
  test/Analysis/ExpectedOutputs/plists/copypaste/plist-diagnostics.cpp.plist
  test/Analysis/ExpectedOutputs/plists/cstring-plist.c.plist
  test/Analysis/ExpectedOutputs/plists/cxx-for-range.cpp.plist
  
test/Analysis/ExpectedOutputs/plists/diagnostics/deref-track-symbolic-region.c.plist
  test/Analysis/ExpectedOutputs/plists/diagnostics/undef-value-caller.c.plist
  test/Analysis/ExpectedOutputs/plists/diagnostics/undef-value-param.c.plist
  test/Analysis/ExpectedOutputs/plists/diagnostics/undef-value-param.m.plist
  test/Analysis/ExpectedOutputs/plists/edges-new.mm.plist
  test/Analysis/ExpectedOutputs/plists/generics.m.plist
  test/Analysis/ExpectedOutputs/plists/inline-plist.c.plist
  test/Analysis/ExpectedOutputs/plists/inline-unique-reports.c.plist
  
test/Analysis/ExpectedOutputs/plists/inlining/eager-reclamation-path-notes.c.plist
  
test/Analysis/ExpectedOutputs/plists/inlining/eager-reclamation-path-notes.cpp.plist
  test/Analysis/ExpectedOutputs/plists/inlining/path-notes.c.plist
  test/Analysis/ExpectedOutputs/plists/inlining/path-notes.cpp.plist
  test/Analysis/ExpectedOutputs/plists/inlining/path-notes.m.plist
  test/Analysis/ExpectedOutputs/plists/model-file.cpp.plist
  test/Analysis/ExpectedOutputs/plists/null-deref-path-notes.m.plist
  test/Analysis/ExpectedOutputs/plists/nullability-notes.m.plist
  test/Analysis/ExpectedOutputs/plists/objc-arc.m.plist
  test/Analysis/ExpectedOutputs/plists/objc-radar17039661.m.plist
  test/Analysis/ExpectedOutputs/plists/plist-output.m.plist
  test/Analysis/ExpectedOutputs/plists/retain-release-path-notes-gc.m.plist
  test/Analysis/ExpectedOutputs/plists/retain-release-path-notes.m.plist
  test/Analysis/ExpectedOutputs/plists/unix-fns.c.plist
  test/Analysis/ExpectedOutputs/plists/yaccignore.c.plist
  test/Analysis/conditional-path-notes.c
  test/Analysis/copypaste/plist-diagnostics-notes-as-events.cpp
  test/Analysis/copypaste/plist-diagnostics.cpp
  test/Analysis/cxx-for-range.cpp
  test/Analysis/diagnostics/deref-track-symbolic-region.c
  test/Analysis/diagnostics/undef-value-caller.c
  test/Analysis/diagnostics/undef-value-param.c
  test/Analysis/diagnostics/undef-value-param.m
  test/Analysis/edges-new.mm
  test/Analysis/generics.m
  test/Analysis/inline-plist.c
  test/Analysis/inline-unique-reports.c
  test/Analysis/inlining/eager-reclamation-path-notes.c
  test/Analysis/inlining/eager-reclamation-path-notes.cpp
  test/Analysis/inlining/path-notes.c
  test/Analysis/inlining/path-notes.cpp
  test/Analysis/inlining/path-notes.m
  test/Analysis/model-file.cpp
  test/Analysis/null-deref-path-notes.m
  test/Analysis/nullability-notes.m
  test/Analysis/objc-arc.m
  test/Analysis/plist-output.m
  test/Analysis/retain-release-path-notes-gc.m
  test/Analysis/retain-release-path-notes.m



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

[PATCH] D50534: [libc++] Fix handling of negated character classes in regex

[Tim Shen via Phabricator via cfe-commits]

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

That looks more correct to me, thanks! Although I'm still puzzled by the empty 
check at all, it's clearly an improvement.


Repository:
  rCXX libc++

https://reviews.llvm.org/D50534



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

[PATCH] D49722: [CStringSyntaxChecker] Check strlcat sizeof check

[George Karpenkov via Phabricator via cfe-commits]

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



Comment at: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp:164
+  // - sizeof(dst)
+  if (Append && isSizeof(LenArg, DstArg))
+return true;

george.karpenkov wrote:
> george.karpenkov wrote:
> > I am confused on what is happening in this branch and why is it bad. Could 
> > we add a commend?
> Sorry I'm still confused about this branch: could you clarify?
Ah, OK, I see it needs one more byte due to null-termination.



Comment at: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp:199
 uint64_t BufferLen = C.getTypeSize(Buffer) / 8;
-if ((BufferLen - DstOff) < ILRawVal)
-  return true;
+auto RemainingBufferLen = BufferLen - DstOff;
+if (Append) {

Probably better expressed as:

```
if (Append)
  RemainingBufferLen -= 1;
```

Then you don't need branching.


https://reviews.llvm.org/D49722



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

[PATCH] D49722: [CStringSyntaxChecker] Check strlcat sizeof check

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp:164
+  // - sizeof(dst)
+  if (Append && isSizeof(LenArg, DstArg))
+return true;

george.karpenkov wrote:
> I am confused on what is happening in this branch and why is it bad. Could we 
> add a commend?
Sorry I'm still confused about this branch: could you clarify?


https://reviews.llvm.org/D49722



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

[PATCH] D48687: [clangd] Avoid duplicates in findDefinitions response

[Simon Marchi via Phabricator via cfe-commits]

simark updated this revision to Diff 160175.
simark marked 4 inline comments as done.
simark added a comment.

Latest update.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48687

Files:
  clangd/FindSymbols.cpp
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  clangd/XRefs.cpp
  unittests/clangd/TestFS.cpp
  unittests/clangd/TestFS.h
  unittests/clangd/XRefsTests.cpp

Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -312,27 +312,65 @@
 }
 
 TEST(GoToDefinition, RelPathsInCompileCommand) {
+  // The source is in "/clangd-test/src".
+  // We build in "/clangd-test/build".
+
   Annotations SourceAnnotations(R"cpp(
+#include "header_in_preamble.h"
 int [[foo]];
-int baz = f^oo;
+#include "header_not_in_preamble.h"
+int baz = f$p1^oo + bar_pre$p2^amble + bar_not_pre$p3^amble;
+)cpp");
+
+  Annotations HeaderInPreambleAnnotations(R"cpp(
+int [[bar_preamble]];
+)cpp");
+
+  Annotations HeaderNotInPreambleAnnotations(R"cpp(
+int [[bar_not_preamble]];
 )cpp");
 
+  // Make the compilation paths appear as ../src/foo.cpp in the compile
+  // commands.
+  SmallString<32> RelPathPrefix("..");
+  llvm::sys::path::append(RelPathPrefix, "src");
+  std::string BuildDir = testPath("build");
+  MockCompilationDatabase CDB(BuildDir, RelPathPrefix);
+
   IgnoreDiagnostics DiagConsumer;
-  MockCompilationDatabase CDB(/*UseRelPaths=*/true);
   MockFSProvider FS;
   ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
 
-  auto FooCpp = testPath("foo.cpp");
+  // Fill the filesystem.
+  auto FooCpp = testPath("src/foo.cpp");
   FS.Files[FooCpp] = "";
+  auto HeaderInPreambleH = testPath("src/header_in_preamble.h");
+  FS.Files[HeaderInPreambleH] = HeaderInPreambleAnnotations.code();
+  auto HeaderNotInPreambleH = testPath("src/header_not_in_preamble.h");
+  FS.Files[HeaderNotInPreambleH] = HeaderNotInPreambleAnnotations.code();
 
-  Server.addDocument(FooCpp, SourceAnnotations.code());
   runAddDocument(Server, FooCpp, SourceAnnotations.code());
+
+  // Go to a definition in main source file.
   auto Locations =
-  runFindDefinitions(Server, FooCpp, SourceAnnotations.point());
+  runFindDefinitions(Server, FooCpp, SourceAnnotations.point("p1"));
   EXPECT_TRUE(bool(Locations)) << "findDefinitions returned an error";
-
   EXPECT_THAT(*Locations, ElementsAre(Location{URIForFile{FooCpp},
SourceAnnotations.range()}));
+
+  // Go to a definition in header_in_preamble.h.
+  Locations = runFindDefinitions(Server, FooCpp, SourceAnnotations.point("p2"));
+  EXPECT_TRUE(bool(Locations)) << "findDefinitions returned an error";
+  EXPECT_THAT(*Locations,
+  ElementsAre(Location{URIForFile{HeaderInPreambleH},
+   HeaderInPreambleAnnotations.range()}));
+
+  // Go to a definition in header_not_in_preamble.h.
+  Locations = runFindDefinitions(Server, FooCpp, SourceAnnotations.point("p3"));
+  EXPECT_TRUE(bool(Locations)) << "findDefinitions returned an error";
+  EXPECT_THAT(*Locations,
+  ElementsAre(Location{URIForFile{HeaderNotInPreambleH},
+   HeaderNotInPreambleAnnotations.range()}));
 }
 
 TEST(Hover, All) {
Index: unittests/clangd/TestFS.h
===
--- unittests/clangd/TestFS.h
+++ unittests/clangd/TestFS.h
@@ -40,15 +40,22 @@
 // A Compilation database that returns a fixed set of compile flags.
 class MockCompilationDatabase : public GlobalCompilationDatabase {
 public:
-  /// When \p UseRelPaths is true, uses relative paths in compile commands.
-  /// When \p UseRelPaths is false, uses absoulte paths.
-  MockCompilationDatabase(bool UseRelPaths = false);
+  /// If \p Directory is not empty, use that as the Directory field of the
+  /// CompileCommand.
+  ///
+  /// If \p RelPathPrefix is not empty, use that as a prefix in front of the
+  /// source file name, instead of using an absolute path.
+  MockCompilationDatabase(StringRef Directory = StringRef(),
+  StringRef RelPathPrefix = StringRef());
 
   llvm::Optional
   getCompileCommand(PathRef File) const override;
 
   std::vector ExtraClangFlags;
-  const bool UseRelPaths;
+
+private:
+  StringRef Directory;
+  StringRef RelPathPrefix;
 };
 
 // Returns an absolute (fake) test directory for this OS.
Index: unittests/clangd/TestFS.cpp
===
--- unittests/clangd/TestFS.cpp
+++ unittests/clangd/TestFS.cpp
@@ -32,22 +32,36 @@
   return MemFS;
 }
 
-MockCompilationDatabase::MockCompilationDatabase(bool UseRelPaths)
-: ExtraClangFlags({"-ffreestanding"}), UseRelPaths(UseRelPaths) {
+MockCompilationDatabase::MockCompilationDatabase(StringRef Directory,
+ StringRef RelPathPrefix)
+: 

[PATCH] D48687: [clangd] Avoid duplicates in findDefinitions response

[Simon Marchi via Phabricator via cfe-commits]

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

In https://reviews.llvm.org/D48687#1195840, @hokein wrote:

> Looks good with a few nits. Looks like you didn't update the patch correctly. 
> You have marked comments done, but your code doesn't get changed accordingly. 
> Please double check with it.
>
> I tried your patch and it did fix the duplicated issue I encountered before. 
> Thanks for fixing it!


Ah sorry, I applied the fixes locally, but was waiting for Ilya's feedback for 
the function comment.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48687



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

[PATCH] D48687: [clangd] Avoid duplicates in findDefinitions response

[Haojian Wu via Phabricator via cfe-commits]

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

Looks good with a few nits. Looks like you didn't update the patch correctly. 
You have marked comments done, but your code doesn't get changed accordingly. 
Please double check with it.

I tried your patch and it did fix the duplicated issue I encountered before. 
Thanks for fixing it!




Comment at: clangd/SourceCode.h:65
 /// Get the absolute file path of a given file entry.
-llvm::Optional getAbsoluteFilePath(const FileEntry *F,
-const SourceManager 
);
+llvm::Optional getRealPath(const FileEntry *F,
+const SourceManager );

simark wrote:
> hokein wrote:
> > Why rename this function?
> > 
> >  Is it guaranteed that `RealPath` is always an absolute path?
> Before, it only made sure that the path was absolute, now it goes a step 
> further and makes it a "real path", resolving symlinks and removing `.` and 
> `..`.  When we talk about a "real path", it refers to the unix realpath 
> syscall:
> 
> http://man7.org/linux/man-pages/man3/realpath.3.html
> 
> So yes, the result is guaranteed to be absolute too.
That makes sense, thanks for the explanation and the useful link!



Comment at: clangd/SourceCode.h:65
 
 /// Get the absolute file path of a given file entry.
 TextEdit toTextEdit(const FixItHint , const SourceManager ,

nit: this comment is not needed.



Comment at: clangd/SourceCode.h:69
 
+llvm::Optional getRealPath(const FileEntry *F,
+const SourceManager );

simark wrote:
> ilya-biryukov wrote:
> > This function looks like a good default choice for normalizing paths before 
> > putting them into LSP structs, ClangdServer responses, etc.
> > I suggest we add a small comment here with a guideline for everyone to 
> > attempt using it whenever possible. WDYT?
> What about this:
> 
> ```
> /// Get the real/canonical path of \p F.  This means:
> ///
> ///   - Absolute path
> ///   - Symlinks resolved
> ///   - No "." or ".." component
> ///   - No duplicate or trailing directory separator
> ///
> /// This function should be used when sending paths to clients, so that paths
> /// are normalized as much as possible.
> ```
SG.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48687



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

Re: r339428 - Add Windows support for the GNUstep Objective-C ABI V2.

[David Chisnall via cfe-commits]

Hi Tom,

I’ll take a look over the weekend and see if I can reproduce locally.  It’s odd 
that a test for a Windows triple would be behaving differently on a Windows 
host.

David

> On 10 Aug 2018, at 18:01, Tom Weaver  wrote:
> 
> Hi David,
> 
> revision 339428 seems to have caused failing tests on a couple of windows 
> build bots, any chance you can take a look please?
> 
> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/18985
> 
> 
> Failing Tests (1):
> Clang :: CodeGenObjC/gnu-init.m
> 
>   Expected Passes: 30627
>   Expected Failures  : 65
>   Unsupported Tests  : 12223
>   Unexpected Failures: 1
> 
> 
> Thanks
> 
> Tom Weaver
> 
> On 10 August 2018 at 13:53, David Chisnall via cfe-commits 
>  wrote:
> Author: theraven
> Date: Fri Aug 10 05:53:13 2018
> New Revision: 339428
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=339428=rev
> Log:
> Add Windows support for the GNUstep Objective-C ABI V2.
> 
> Summary:
> Introduces funclet-based unwinding for Objective-C and fixes an issue
> where global blocks can't have their isa pointers initialised on
> Windows.
> 
> After discussion with Dustin, this changes the name mangling of
> Objective-C types to prevent a C++ catch statement of type struct X*
> from catching an Objective-C object of type X*.
> 
> Reviewers: rjmccall, DHowett-MSFT
> 
> Reviewed By: rjmccall, DHowett-MSFT
> 
> Subscribers: mgrang, mstorsjo, smeenai, cfe-commits
> 
> Differential Revision: https://reviews.llvm.org/D50144
> 
> Modified:
> cfe/trunk/include/clang/Driver/Options.td
> cfe/trunk/lib/AST/MicrosoftMangle.cpp
> cfe/trunk/lib/CodeGen/CGException.cpp
> cfe/trunk/lib/CodeGen/CGObjCGNU.cpp
> cfe/trunk/lib/CodeGen/CGObjCRuntime.cpp
> cfe/trunk/lib/CodeGen/CGObjCRuntime.h
> cfe/trunk/lib/CodeGen/CodeGenFunction.h
> cfe/trunk/lib/Driver/ToolChains/Clang.cpp
> cfe/trunk/test/CodeGenObjC/gnu-init.m
> cfe/trunk/test/CodeGenObjC/gnustep2-proto.m
> cfe/trunk/test/CodeGenObjCXX/arc-marker-funclet.mm
> cfe/trunk/test/CodeGenObjCXX/microsoft-abi-arc-param-order.mm
> cfe/trunk/test/CodeGenObjCXX/msabi-objc-extensions.mm
> cfe/trunk/test/CodeGenObjCXX/msabi-objc-types.mm
> 
> Modified: cfe/trunk/include/clang/Driver/Options.td
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=339428=339427=339428=diff
> ==
> --- cfe/trunk/include/clang/Driver/Options.td (original)
> +++ cfe/trunk/include/clang/Driver/Options.td Fri Aug 10 05:53:13 2018
> @@ -1488,7 +1488,7 @@ def fobjc_weak : Flag<["-"], "fobjc-weak
>HelpText<"Enable ARC-style weak references in Objective-C">;
> 
>  // Objective-C ABI options.
> -def fobjc_runtime_EQ : Joined<["-"], "fobjc-runtime=">, Group, 
> Flags<[CC1Option]>,
> +def fobjc_runtime_EQ : Joined<["-"], "fobjc-runtime=">, Group, 
> Flags<[CC1Option, CoreOption]>,
>HelpText<"Specify the target Objective-C runtime kind and version">;
>  def fobjc_abi_version_EQ : Joined<["-"], "fobjc-abi-version=">, 
> Group;
>  def fobjc_nonfragile_abi_version_EQ : Joined<["-"], 
> "fobjc-nonfragile-abi-version=">, Group;
> 
> Modified: cfe/trunk/lib/AST/MicrosoftMangle.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/MicrosoftMangle.cpp?rev=339428=339427=339428=diff
> ==
> --- cfe/trunk/lib/AST/MicrosoftMangle.cpp (original)
> +++ cfe/trunk/lib/AST/MicrosoftMangle.cpp Fri Aug 10 05:53:13 2018
> @@ -445,7 +445,7 @@ void MicrosoftCXXNameMangler::mangle(con
>  mangleFunctionEncoding(FD, Context.shouldMangleDeclName(FD));
>else if (const VarDecl *VD = dyn_cast(D))
>  mangleVariableEncoding(VD);
> -  else
> +  else if (!isa(D))
>  llvm_unreachable("Tried to mangle unexpected NamedDecl!");
>  }
> 
> @@ -1884,13 +1884,13 @@ void MicrosoftCXXNameMangler::mangleType
>  llvm_unreachable("placeholder types shouldn't get to name mangling");
> 
>case BuiltinType::ObjCId:
> -mangleArtificalTagType(TTK_Struct, "objc_object");
> +mangleArtificalTagType(TTK_Struct, ".objc_object");
>  break;
>case BuiltinType::ObjCClass:
> -mangleArtificalTagType(TTK_Struct, "objc_class");
> +mangleArtificalTagType(TTK_Struct, ".objc_class");
>  break;
>case BuiltinType::ObjCSel:
> -mangleArtificalTagType(TTK_Struct, "objc_selector");
> +mangleArtificalTagType(TTK_Struct, ".objc_selector");
>  break;
> 
>  #define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) \
> @@ -2570,9 +2570,10 @@ void MicrosoftCXXNameMangler::mangleType
> 
>  void MicrosoftCXXNameMangler::mangleType(const ObjCInterfaceType *T, 
> Qualifiers,
>   SourceRange) {
> -  // ObjC interfaces have structs underlying them.
> +  // ObjC interfaces are mangled as if they were structs with a name that is
> +  

[PATCH] D50580: [clang-tidy] Abseil: no namespace check

[Deanna Garcia via Phabricator via cfe-commits]

deannagarcia created this revision.
deannagarcia added reviewers: alexfh, hokein.
deannagarcia added a project: clang-tools-extra.
Herald added subscribers: xazax.hun, mgorny.

This check ensures that users of Abseil do not open namespace absl in their 
code, as that violates our compatibility guidelines.

We are aware that this test will cause warnings on users code through their 
dependencies on abseil. However, from what we know it seems like these warnings 
are normally suppressed. If anyone has a good idea on how to avoid this/has 
insight on whether this will be a problem for the average user, please let me 
know!


https://reviews.llvm.org/D50580

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/NoNamespaceCheck.cpp
  clang-tidy/abseil/NoNamespaceCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-no-namespace.cpp

Index: test/clang-tidy/abseil-no-namespace.cpp
===
--- test/clang-tidy/abseil-no-namespace.cpp
+++ test/clang-tidy/abseil-no-namespace.cpp
@@ -0,0 +1,17 @@
+// RUN: %check_clang_tidy %s abseil-no-namespace %t
+  
+namespace absl {}
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: namespace 'absl' is reserved for
+// implementation of the Abseil library and should not be opened in the user
+// code [abseil-no-namespace]
+
+namespace absl {
+int i = 5;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:11: warning: namespace 'absl' is reserved for
+// implementation of the Abseil library and should not be opened in the user
+// code
+
+// Things that shouldn't trigger the check
+int i = 5;
+namespace std {}
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-no-namespace
abseil-string-find-startswith
android-cloexec-accept
android-cloexec-accept4
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -59,6 +59,12 @@
 
 The improvements are...
 
+- New :doc:`abseil-no-namespace
+  ` check.
+
+  Checks to ensure user did not open namespace absl as that
+  violates abseil's compatibility guidelines.
+
 Improvements to include-fixer
 -
 
Index: clang-tidy/abseil/NoNamespaceCheck.h
===
--- clang-tidy/abseil/NoNamespaceCheck.h
+++ clang-tidy/abseil/NoNamespaceCheck.h
@@ -0,0 +1,37 @@
+//===--- NoNamespaceCheck.h - clang-tidy-*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_NONAMESPACECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_NONAMESPACECHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// This test ensures users don't open namespace absl, as that violates
+/// our compatibility guidelines.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-no-namespace.html
+class NoNamespaceCheck : public ClangTidyCheck {
+ public:
+  NoNamespaceCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+}  // namespace abseil
+}  // namespace tidy
+}  // namespace clang
+
+#endif  // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_NONAMESPACECHECK_H
+
Index: clang-tidy/abseil/NoNamespaceCheck.cpp
===
--- clang-tidy/abseil/NoNamespaceCheck.cpp
+++ clang-tidy/abseil/NoNamespaceCheck.cpp
@@ -0,0 +1,38 @@
+//===--- NoNamespaceCheck.cpp - clang-tidy-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "NoNamespaceCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+void NoNamespaceCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus) return;
+
+  Finder->addMatcher(namespaceDecl(hasName("absl")).bind("absl_namespace"),
+ this);
+}
+
+void NoNamespaceCheck::check(const MatchFinder::MatchResult ) {
+  

[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:10424
+  // We don't want to warn for system macro.
+  S.SourceMgr.isInSystemMacro(E->getOperatorLoc()))
+// warn about dropping FP rank.

nickdesaulniers wrote:
> aaron.ballman wrote:
> > This looks incorrect to me -- this is testing the rank difference and that 
> > it is in a system macro (the `!` was dropped). If this didn't cause any 
> > tests to fail, we should add a test that would fail for it.
> Thanks for the code review!
> 
> Sorry, I don't understand what you mean by "rank difference"? F16 vs F32 vs 
> F64 vs F128 etc?
> 
> When you say "this looks incorrect," are you commenting on the code I added 
> (Lines 10415 to 10418) or the prexisting code that I moved (Lines 
> 10420-10427)?
> 
> Good catch on dropping the `!`, that was definitely not intentional (and now 
> I'm curious if `dw` in vim will delete `(!` together), will add back.
> 
> I'm happy to add a test for the system macro, but I also don't know what that 
> is.  Can you explain?
Sorry, my explanation may have been confusing. A better way to phrase it would 
have been "this isn't doing what the old code used to do, and I don't think you 
intended to change it." I was talking about the `!` that disappeared.

I think you can use linemarker directives to get this to happen:
```
# 1 "systemheader.h" 1 3
#define MACRO

# 1 "test_file.c" 2
// Your code that uses MACRO.
```


Repository:
  rC Clang

https://reviews.llvm.org/D50467



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

[PATCH] D50447: [clang-tidy] Omit cases where loop variable is not used in loop body in performance-for-range-copy.

[Shuai Wang via Phabricator via cfe-commits]

shuaiwang added a comment.

In https://reviews.llvm.org/D50447#1194967, @JonasToth wrote:

> Do you think it is a bad idea? If the variable is not used it is ok to
>  ignore it in this particular circumstance. Other warnings/check should
>  deal with such a situation IMHO.
>
> Am 10.08.2018 um 10:29 schrieb Roman Lebedev via Phabricator:
>
> > lebedev.ri added a comment.
> > 
> > It seems this ended up silently being a catch-all, with no option to 
> > control this behavior, and i don't see any comments discussing this..
> > 
> > Repository:
> > 
> >   rL LLVM
> > 
> > https://reviews.llvm.org/D50447


Not sure whether hokein have done that already but I did run through our code 
base and AFAICT there's no false negative.


Repository:
  rL LLVM

https://reviews.llvm.org/D50447



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

[PATCH] D45444: [clang-tidy] implement new check for const-correctness

[Shuai Wang via Phabricator via cfe-commits]

shuaiwang added a comment.

In https://reviews.llvm.org/D45444#1191874, @JonasToth wrote:

> > Could you give a concrete example of this?
>
> vi llvm/lib/Demangle/ItaniumDemangle.cpp +1762
>
> /home/jonas/opt/llvm/lib/Demangle/ItaniumDemangle.cpp:1762:7: warning:
>  variable 'num' of type 'char [FloatData::max_demangled_size]' can
>  be declared 'const' [cppcoreguidelines-const-correctness]
>    char num[FloatData::max_demangled_size] = {0};
>   ^
>  /home/jonas/opt/llvm/lib/Demangle/ItaniumDemangle.cpp:1763:7: warning:
>  variable 'n' of type 'int' can be declared 'const'
>  [cppcoreguidelines-const-correctness]
>        int n = snprintf(num, sizeof(num), FloatData::spec, value);
>   ^


Looks like related to template.
the exact type of `num` depends on `Float`
and the `CallExpr` is not even resolved because we don't know the type of the 
arguments yet (adl, overload resolution, etc.)
So the AST doesn't contain any array to pointer decay.

I guess we should suppress the check in such cases.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45444



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

[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:10424
+  // We don't want to warn for system macro.
+  S.SourceMgr.isInSystemMacro(E->getOperatorLoc()))
+// warn about dropping FP rank.

aaron.ballman wrote:
> This looks incorrect to me -- this is testing the rank difference and that it 
> is in a system macro (the `!` was dropped). If this didn't cause any tests to 
> fail, we should add a test that would fail for it.
Thanks for the code review!

Sorry, I don't understand what you mean by "rank difference"? F16 vs F32 vs F64 
vs F128 etc?

When you say "this looks incorrect," are you commenting on the code I added 
(Lines 10415 to 10418) or the prexisting code that I moved (Lines 10420-10427)?

Good catch on dropping the `!`, that was definitely not intentional (and now 
I'm curious if `dw` in vim will delete `(!` together), will add back.

I'm happy to add a test for the system macro, but I also don't know what that 
is.  Can you explain?


Repository:
  rC Clang

https://reviews.llvm.org/D50467



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

[PATCH] D50559: [gnu-objc] Make selector order deterministic.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

You can't test that there's no non-determinism, but you can certainly test that 
we emit selectors in sorted order as opposed to the order in which they're 
used.  I imagine a function with a bunch of `@selector` expressions should be 
good enough for that.


Repository:
  rC Clang

https://reviews.llvm.org/D50559



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

[PATCH] D50564: Add support for SEH unwinding on Windows.

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

> I've tested this implementation on x86-64 to ensure that it works. All 
> libc++abi tests pass, as do all libc++ exception-related tests.

I tested it now as well, and seems to work for my simple testcase.

> ARM still remains to be implemented (@compnerd?).

LLVM doesn't generate SEH unwind data for ARM at all yet. ARM64 implementation 
is ongoing at the moment though, see https://reviews.llvm.org/D50166 and 
https://reviews.llvm.org/D50288.

> Special thanks to KJK::Hyperion for his excellent series of articles on how 
> EH works on x86-64 Windows. (Seriously, check it out. It's awesome.)

Can you give some links to it? A brief googling didn't turn up much else than 
the PSEH library.

> I'm actually not sure if this should go in as is. I particularly don't like 
> that I duplicated the UnwindCursor class for this special case.

As I'm not totally familiar with how it works, I can only give cursory comments 
and compare to the libgcc implementation when trying to wrap my head around it, 
but this does seem way, way more complex than the libgcc version of the same.

As for the duplicated UnwindCursor, I'm thinking if it'd become more 
straightforward by avoiding touching those parts altogether, and just 
reimplementing all the other public functions, `_Unwind_G/SetGR/IP`, 
`_Unwind_Backtrace` etc, directly in Unwind-seh.cpp, and not care about using 
the libunwind.h APIs (`unw_*`) inbetween altogether? Or perhaps my libgcc 
comparison is making me biased.




Comment at: src/Unwind-seh.cpp:53
+
+/// Exception cleanup routine used by \c __libunwind_frame_consolidate to
+/// regain control after handling an SEH exception.

I don't see any `__libunwind_frame_consolidate` anywhere, is this comment 
outdated?



Comment at: src/UnwindLevel1.c:35
 
+#if !_LIBUNWIND_SUPPORT_SEH_UNWIND
+

This probably works, but isn't `#if !defined(_LIBUNWIND_SUPPORT_SEH_UNWIND)` 
more of the common form?



Comment at: src/libunwind_ext.h:43
+  #if defined(__x86_64__) && !defined(__MINGW64__)
+typedef struct _DISPATCHER_CONTEXT {
+  ULONG64 ControlPc;

What's this all about? winnt.h (from both MSVC and mingw-w64) should define 
this struct, no?



Comment at: src/libunwind_ext.h:73
+  #endif
+extern int _unw_init_seh(unw_cursor_t *cursor, CONTEXT *ctx);
+extern DISPATCHER_CONTEXT *_unw_seh_get_disp_ctx(unw_cursor_t *cursor);

These are all both defined and called from Unwind-seh.cpp, so couldn't they 
just be static functions within there?


Repository:
  rUNW libunwind

https://reviews.llvm.org/D50564



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

[PATCH] D50507: [CodeGen][ARM] Coerce FP16 vectors to integer vectors when needed

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: lib/CodeGen/TargetInfo.cpp:5788
+  llvm::Type *Ty = llvm::ArrayType::get(NewVecTy, Members);
+  return ABIArgInfo::getDirect(Ty, 0, nullptr, false);
+}

Do we need equivalent code in classifyReturnType?


https://reviews.llvm.org/D50507



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

[PATCH] D50559: [gnu-objc] Make selector order deterministic.

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang added a comment.

In https://reviews.llvm.org/D50559#1195787, @bmwiedemann wrote:

>   got a build failure with this patch added onto 6.0.1


You may have to add:

  #include "llvm/ADT/STLExtras.h"


Repository:
  rC Clang

https://reviews.llvm.org/D50559



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

[PATCH] D50559: [gnu-objc] Make selector order deterministic.

[Bernhard M. Wiedemann via Phabricator via cfe-commits]

bmwiedemann added a comment.

got a build failure with this patch added onto 6.0.1




Comment at: lib/CodeGen/CGObjCGNU.cpp:3547
+  allSelectors.push_back(entry.first);
+llvm::sort(allSelectors.begin(), allSelectors.end());
 

compilation failed here:
../tools/clang/lib/CodeGen/CGObjCGNU.cpp:2444:11: error: 'sort' is not a member 
of 'llvm'

it suggested std::sort


Repository:
  rC Clang

https://reviews.llvm.org/D50559



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

[PATCH] D50341: [libcxx] Mark aligned allocation tests as XFAIL on old OSX versions

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

I still think we should go forward with this change since the tests _are_ 
expected to fail on the provided OS X versions, which do not contain the 
required operators.


Repository:
  rCXX libc++

https://reviews.llvm.org/D50341



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

[PATCH] D50517: [clangd] Generate incomplete trigrams for the Dex index

[Eric Liu via Phabricator via cfe-commits]

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

lg! Thanks for the changes!




Comment at: clang-tools-extra/clangd/index/dex/Trigram.cpp:147
+  if (Chars.size() == 3) {
+const auto Trigram =
+Token(Token::Kind::Trigram, std::string(begin(Chars), end(Chars)));

nit: inline this variable? You don't need to `count` below as `insert` 
duplicates for you already.


https://reviews.llvm.org/D50517



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

[PATCH] D50534: [libc++] Fix handling of negated character classes in regex

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

In https://reviews.llvm.org/D50534#1194664, @timshen wrote:

> I'm not fully equipped with the context right now, but something doesn't add 
> up. if `__neg_chars_.empty()` check is removed, the `(__neg_mask_ == 0)` 
> above should be removed too. They have to be consistent.
>
> However, there is more weirdness in it. The comment above describes the 
> intention:
>
>   union(complement(union(__neg_chars_, __neg_mask_)), other cases...)
>   
>
> With the `__neg_chars_.empty()` and `(__neg_mask_ == 0)` removed, I believe 
> that the code exactly matches the comment. Let's see what happens when users 
> don't specify any negative class or chars. `__neg_chars_` and `__neg_mask_` 
> will be empty sets, and `union(complement(union(__neg_chars_, __neg_mask_)), 
> other cases...)` always evaluate to full set, which means it always matches 
> all characters. This can't be right.
>
> It's likely that the comment description doesn't fully describe the intended 
> behavior. I think we need to figure that out first.


Yes, I think you were right. I think what was missing is that the comment does 
not apply when there's no `neg_mask` and no `neg_chars`. Otherwise we get into 
that situation where we're taking the complement of an empty set, and we match 
everything, which is not what we want.


Repository:
  rCXX libc++

https://reviews.llvm.org/D50534



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

[PATCH] D50534: [libc++] Fix handling of negated character classes in regex

[Louis Dionne via Phabricator via cfe-commits]

ldionne updated this revision to Diff 160169.
ldionne added a comment.

Rewrite the patch in light of timshen's comment, and add tests.


Repository:
  rCXX libc++

https://reviews.llvm.org/D50534

Files:
  libcxx/include/regex
  libcxx/test/std/re/re.alg/re.alg.match/inverted_character_classes.pass.cpp
  libcxx/test/std/re/re.alg/re.alg.search/invert_neg_word_search.pass.cpp


Index: libcxx/test/std/re/re.alg/re.alg.search/invert_neg_word_search.pass.cpp
===
--- libcxx/test/std/re/re.alg/re.alg.search/invert_neg_word_search.pass.cpp
+++ libcxx/test/std/re/re.alg/re.alg.search/invert_neg_word_search.pass.cpp
@@ -18,7 +18,7 @@
 
 #include 
 #include 
-#include "test_macros.h"
+
 
 // PR34310
 int main()
Index: 
libcxx/test/std/re/re.alg/re.alg.match/inverted_character_classes.pass.cpp
===
--- /dev/null
+++ libcxx/test/std/re/re.alg/re.alg.match/inverted_character_classes.pass.cpp
@@ -0,0 +1,44 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// 
+// UNSUPPORTED: c++98, c++03
+
+// Make sure that we correctly match inverted character classes.
+
+#include 
+#include 
+
+
+int main() {
+assert(std::regex_match("X", std::regex("[X]")));
+assert(std::regex_match("X", std::regex("[XY]")));
+assert(!std::regex_match("X", std::regex("[^X]")));
+assert(!std::regex_match("X", std::regex("[^XY]")));
+
+assert(std::regex_match("X", std::regex("[\\S]")));
+assert(!std::regex_match("X", std::regex("[^\\S]")));
+
+assert(!std::regex_match("X", std::regex("[\\s]")));
+assert(std::regex_match("X", std::regex("[^\\s]")));
+
+assert(std::regex_match("X", std::regex("[\\s\\S]")));
+assert(std::regex_match("X", std::regex("[^Y\\s]")));
+assert(!std::regex_match("X", std::regex("[^X\\s]")));
+
+assert(std::regex_match("X", std::regex("[\\w]")));
+assert(std::regex_match("_", std::regex("[\\w]")));
+assert(!std::regex_match("X", std::regex("[^\\w]")));
+assert(!std::regex_match("_", std::regex("[^\\w]")));
+
+assert(!std::regex_match("X", std::regex("[\\W]")));
+assert(!std::regex_match("_", std::regex("[\\W]")));
+assert(std::regex_match("X", std::regex("[^\\W]")));
+assert(std::regex_match("_", std::regex("[^\\W]")));
+}
Index: libcxx/include/regex
===
--- libcxx/include/regex
+++ libcxx/include/regex
@@ -2414,20 +2414,17 @@
 goto __exit;
 }
 }
-// set of "__found" chars =
+// When there's at least one of __neg_chars_ and __neg_mask_, the set
+// of "__found" chars is
 //   union(complement(union(__neg_chars_, __neg_mask_)),
 // other cases...)
 //
-// __neg_chars_ and __neg_mask_'d better be handled together, as there
-// are no short circuit opportunities.
-//
-// In addition, when __neg_mask_/__neg_chars_ is empty, they should be
-// treated as all ones/all chars.
+// It doesn't make sense to check this when there are no __neg_chars_
+// and no __neg_mask_.
+if (!(__neg_mask_ == 0 && __neg_chars_.empty()))
 {
-  const bool __in_neg_mask = (__neg_mask_ == 0) ||
-  __traits_.isctype(__ch, __neg_mask_);
+const bool __in_neg_mask = __traits_.isctype(__ch, __neg_mask_);
   const bool __in_neg_chars =
-  __neg_chars_.empty() ||
   std::find(__neg_chars_.begin(), __neg_chars_.end(), __ch) !=
   __neg_chars_.end();
   if (!(__in_neg_mask || __in_neg_chars))


Index: libcxx/test/std/re/re.alg/re.alg.search/invert_neg_word_search.pass.cpp
===
--- libcxx/test/std/re/re.alg/re.alg.search/invert_neg_word_search.pass.cpp
+++ libcxx/test/std/re/re.alg/re.alg.search/invert_neg_word_search.pass.cpp
@@ -18,7 +18,7 @@
 
 #include 
 #include 
-#include "test_macros.h"
+
 
 // PR34310
 int main()
Index: libcxx/test/std/re/re.alg/re.alg.match/inverted_character_classes.pass.cpp
===
--- /dev/null
+++ libcxx/test/std/re/re.alg/re.alg.match/inverted_character_classes.pass.cpp
@@ -0,0 +1,44 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 160168.
hugoeg added a comment.

corrections from comments applied


https://reviews.llvm.org/D50542

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/NoInternalDepsCheck.cpp
  clang-tidy/abseil/NoInternalDepsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-no-internal-deps.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-no-internal-deps.cpp

Index: test/clang-tidy/abseil-no-internal-deps.cpp
===
--- test/clang-tidy/abseil-no-internal-deps.cpp
+++ test/clang-tidy/abseil-no-internal-deps.cpp
@@ -0,0 +1,62 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t
+
+
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+} // namepsace std
+
+namespace absl {
+std::string StringsFunction (std::string s1){
+  return s1;
+}
+class SomeContainer{
+
+};
+
+namespace strings_internal{
+  
+  void InternalFunction(){}
+
+  template 
+  P InternalTemplateFunction (P a) {}
+} // namespace strings_internal
+
+namespace container_internal{
+  struct InternalStruct{};
+
+} // namespace container_internal
+} // namespace absl
+
+void foo(){
+  absl::strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: depends upon internal implementation details, which violates the Abseil compatibilty guidelines. See https://abseil.io/about/compatibility
+
+  absl::strings_internal::InternalTemplateFunction ("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: depends upon internal implementation details, which violates the Abseil compatibilty guidelines. See https://abseil.io/about/compatibility
+}
+
+class foo2{
+  friend struct absl::container_internal::InternalStruct;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: depends upon internal implementation details, which violates the Abseil compatibilty guidelines. See https://abseil.io/about/compatibility
+};
+
+namespace absl{
+  void foo3(){
+strings_internal::InternalFunction();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: depends upon internal implementation details, which violates the Abseil compatibilty guidelines. See https://abseil.io/about/compatibility
+  }
+} // namespace absl
+
+// should not trigger warnings
+void foo4(){
+  std::string str = absl::StringsFunction("a");
+  absl::SomeContainer b;
+}
+
+namespace absl {
+  SomeContainer b;
+  std::string str = absl::StringsFunction("a");
+} // namespace absl
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -5,6 +5,7 @@
 
 .. toctree::
abseil-string-find-startswith
+   abseil-no-internal-deps
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-no-internal-deps.rst
===
--- docs/clang-tidy/checks/abseil-no-internal-deps.rst
+++ docs/clang-tidy/checks/abseil-no-internal-deps.rst
@@ -0,0 +1,16 @@
+subl.. title:: clang-tidy - abseil-no-internal-deps
+
+abseil-no-internal-deps
+===
+
+Gives a warning if code using Abseil depends on internal details. If something is in a namespace or filename/path that includes the word “internal”, code is not allowed to depend upon it beaucse it’s an implementation detail. They cannot friend it, include it, you mention it or refer to it in any way. Doing so violtaes Abseil's compatibility guidelines and may result in breakage.
+
+The folowing cases will result in warnings:
+
+.. code-block:: c++
+
+absl::strings_internal::foo();
+class foo{
+  friend struct absl::container_internal::faa;
+};
+absl::memory_internal::MakeUniqueResult();
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -52,7 +52,10 @@
 Improvements to clang-rename
 
 
-The improvements are...
+- New :doc:`abseil-no-internal-deps
+  ` check.
+  
+  Gives a warning if code using Abseil depends on internal details.
 
 Improvements to clang-tidy
 --
Index: clang-tidy/abseil/NoInternalDepsCheck.h
===
--- clang-tidy/abseil/NoInternalDepsCheck.h
+++ clang-tidy/abseil/NoInternalDepsCheck.h
@@ -0,0 +1,37 @@
+//===--- NoInternalDepsCheck.h - clang-tidy--*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_NOINTERNALDEPSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_NOINTERNALDEPSCHECK_H
+

[PATCH] D49240: [libc++] Introduce _LIBCPP_HIDE_FROM_ABI to replace _LIBCPP_INLINE_VISIBILITY

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

In https://reviews.llvm.org/D49240#1195723, @rnk wrote:

> In https://reviews.llvm.org/D49240#1195237, @ldionne wrote:
>
> > In https://reviews.llvm.org/D49240#1195125, @thakis wrote:
> >
> > > When we updated out clang bundle in chromium (which includes libc++ 
> > > headers), our ios simulator bots regressed debug info size by ~50% due to 
> > > this commit 
> > > (https://bugs.chromium.org/p/chromium/issues/detail?id=872926#c13). Is 
> > > that expected?
> >
> >
> > No, this is quite surprising. What happened with the size of the Release 
> > builds? We should investigate, perhaps this change exposed something in 
> > Clang's debug info generation.
>
>
> It looks like the increase is entirely from more symbols in the symbol table. 
> It's not a problem with clang's debug info. It would help a lot if ld64 
> de-duplicated strings in the symbol table, if that's possible.
>
> [...]


Ah, thanks a lot for taking a look! Yes, this makes a lot of sense, since now 
we're not inlining everything anymore. So the code size is actually smaller 
(-9.8%), but there's more symbols because more functions are emitted. In this 
case, I would say this is expected, if unfortunate. Also, a similar effect 
would probably be witnessed if Chromium were to change their standard library 
to libstdc++, for example, since libstdc++ does not abuse inlining like libc++ 
used to.


Repository:
  rCXX libc++

https://reviews.llvm.org/D49240



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

[PATCH] D49511: [Sema/Attribute] Check for noderef attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: lib/Sema/SemaExpr.cpp:14251-14253
+if (const auto *Ptr = dyn_cast(Ty))
+  Inner = Ptr->getPointeeType();
+else if (const auto *Arr = dyn_cast(Ty))

I don't think this strips off sugar from the type, so I suspect it won't 
properly handle a typedef to a pointer type, for instance, or a type including 
parens. You should add tests for these cases.


Repository:
  rC Clang

https://reviews.llvm.org/D49511



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

[PATCH] D49511: [Sema/Attribute] Check for noderef attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Oops, I hit "Send" too soon. I was going to say that you should also keep an 
eye on https://reviews.llvm.org/D50526 because that may impact this patch (or 
vice versa if this one lands first).


Repository:
  rC Clang

https://reviews.llvm.org/D49511



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

[PATCH] D49240: [libc++] Introduce _LIBCPP_HIDE_FROM_ABI to replace _LIBCPP_INLINE_VISIBILITY

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In https://reviews.llvm.org/D49240#1195237, @ldionne wrote:

> In https://reviews.llvm.org/D49240#1195125, @thakis wrote:
>
> > When we updated out clang bundle in chromium (which includes libc++ 
> > headers), our ios simulator bots regressed debug info size by ~50% due to 
> > this commit 
> > (https://bugs.chromium.org/p/chromium/issues/detail?id=872926#c13). Is that 
> > expected?
>
>
> No, this is quite surprising. What happened with the size of the Release 
> builds? We should investigate, perhaps this change exposed something in 
> Clang's debug info generation.


It looks like the increase is entirely from more symbols in the symbol table. 
It's not a problem with clang's debug info. It would help a lot if ld64 
de-duplicated strings in the symbol table, if that's possible.

  $ bloaty after_base_unittests -- before_base_unittests 
   VM SIZE FILE SIZE
   --   --
+103% +51.4Mi String Table  +51.4Mi  +103%
+105% +17.6Mi Symbol Table  +17.6Mi  +105%
 +49%  +827Ki Code Signature +827Ki   +49%
 +92%  +263Ki Function Start Addresses   +263Ki   +92%
 +15% +4.50Ki __TEXT,__unwind_info  +4.50Ki   +15%
 +74% +3.52Ki Table of Non-instructions +3.52Ki   +74%
-0.0%-128 __TEXT,__const   -128  -0.0%
-2.0%-136 [__TEXT] -136  -2.0%
[ = ]   0 [Unmapped]   -480  -3.4%
-1.0%-516 __TEXT,__gcc_except_tab  -516  -1.0%
   -39.3% -1.07Ki [__LINKEDIT]   +4   +33%
-9.8% -5.61Mi __TEXT,__text -5.61Mi  -9.8%
 +50% +64.5Mi TOTAL +64.5Mi   +50%


Repository:
  rCXX libc++

https://reviews.llvm.org/D49240



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

[PATCH] D50509: [analyzer][UninitializedObjectChecker] Refactoring p6.: Move dereferencing to a function

[George Karpenkov via Phabricator via cfe-commits]

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



Comment at: lib/StaticAnalyzer/Checkers/UninitializedPointee.cpp:78
+/// If for whatever reason dereferencing fails, returns with false.
+static bool dereference(ProgramStateRef State, SVal , QualType );
+

In general, using return values is better than out-parameters.
Could you instead return `Optional>` ?


https://reviews.llvm.org/D50509



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

[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis

[Aaron Ballman via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaChecking.cpp:10424
+  // We don't want to warn for system macro.
+  S.SourceMgr.isInSystemMacro(E->getOperatorLoc()))
+// warn about dropping FP rank.

This looks incorrect to me -- this is testing the rank difference and that it 
is in a system macro (the `!` was dropped). If this didn't cause any tests to 
fail, we should add a test that would fail for it.


Repository:
  rC Clang

https://reviews.llvm.org/D50467



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

[PATCH] D50506: [analyzer][UninitializedObjectChecker] Refactoring p4.: Wrap FieldRegions and reduce weight on FieldChainInfo

[George Karpenkov via Phabricator via cfe-commits]

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

OK. I'm not a fan of inheritance hierarchies in general (especially if you only 
have two cases) but OK if it makes your life easier.


Repository:
  rC Clang

https://reviews.llvm.org/D50506



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

[PATCH] D50526: Model type attributes as regular Attrs

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for working on this -- the approach you've taken looks good, and I 
mostly have little nits here and there. I think this is a great refactoring 
overall though -- much needed!




Comment at: include/clang/AST/Type.h:1856
+
+  /// Was this type written with the special inert-in-MRC __unsafe_unretained
+  /// qualifier?

Typo: MRC should be ARC (typo was present in the original code).



Comment at: include/clang/Basic/Attr.td:1510
+  let Spellings = [Keyword<"__unsafe_unretained">];
+  let Documentation = [Undocumented];
+}

I don't suppose you can throw in some quick docs for this keyword? Or is this 
not really user-facing? If it's not user-facing, perhaps it should have no 
spellings and only ever be created implicitly?



Comment at: lib/AST/Type.cpp:1624
+  const Type *Cur = this;
+  while (auto *AT = Cur->getAs()) {
+if (AT->getAttrKind() == AK)

`const auto *`



Comment at: lib/AST/Type.cpp:3215
   }
-  llvm_unreachable("bad attributed type kind");
 }

Probably need to keep this to prevent MSVC from barking about not all control 
paths returning a value.



Comment at: lib/AST/Type.cpp:3653
+Optional
+Type::getNullability(const ASTContext ) const {
   QualType type(this, 0);

Update the identifiers for coding standards since you're basically rewriting 
the function?



Comment at: lib/AST/Type.cpp:3655
   QualType type(this, 0);
-  do {
+  while (auto *AT = type->getAs()) {
 // Check whether this is an attributed type with nullability

`const auto *`



Comment at: lib/AST/TypeLoc.cpp:408
   if (auto attributedLoc = getAs()) {
-if (attributedLoc.getAttrKind() == AttributedType::attr_nullable ||
-attributedLoc.getAttrKind() == AttributedType::attr_nonnull ||
-attributedLoc.getAttrKind() == AttributedType::attr_null_unspecified)
-  return attributedLoc.getAttrNameLoc();
+auto *A = attributedLoc.getAttr();
+if (A && (isa(A) || isa(A) ||

Might as well go with `const Attr *` here, the `auto` gets us nothing.



Comment at: lib/Sema/SemaDecl.cpp:6024
   // by applying it to the function type.
-  if (ATL.getAttrKind() == AttributedType::attr_lifetimebound) {
+  if (auto *A = ATL.getAttrAs()) {
 const auto *MD = dyn_cast(FD);

`const auto *`



Comment at: lib/Sema/SemaType.cpp:178-180
+// their TypeLocs makes it hard to correctly assign these. We use the
+// keep the attributes in creation order as an attempt to make them
+// line up properly.

"We use the keep the attributes" isn't grammatical. Should it be, "We keep the 
attributes in creation order" instead?



Comment at: lib/Sema/SemaType.cpp:181
+// line up properly.
+using TypeAttr = std::pair;
+SmallVector TypeAttrs;

It's unfortunate to use the name `TypeAttr` here -- unqualified uses of the 
type name, like below, makes it look like it might be the `TypeAttr` from Attr.h



Comment at: lib/Sema/SemaType.cpp:3884
 
+template
+static AttrT *createSimpleAttr(ASTContext , ParsedAttr ) {

Did clang-format do this? It seems like it's not formatted how I'd expect.



Comment at: lib/Sema/SemaType.cpp:6360
+  }
+}
+

MSVC may complain about not all control paths returning a value here.



Comment at: lib/Serialization/ASTReaderDecl.cpp:2695
+  Attr *New = nullptr;
+  auto Kind = (attr::Kind)(V - 1);
+  SourceRange Range = Record.readSourceRange();

This could use a comment to explain why the -1 is required. Also, do we have a 
preference for C-style vs static_cast here?



Comment at: lib/Serialization/ASTReaderDecl.cpp:2707
 void ASTReader::ReadAttributes(ASTRecordReader , AttrVec ) {
-  for (unsigned i = 0, e = Record.readInt(); i != e; ++i) {
-Attr *New = nullptr;
-auto Kind = (attr::Kind)Record.readInt();
-SourceRange Range = Record.readSourceRange();
-ASTContext  = getContext();
-
-#include "clang/Serialization/AttrPCHRead.inc"
-
-assert(New && "Unable to decode attribute?");
-Attrs.push_back(New);
-  }
+  for (unsigned i = 0, e = Record.readInt(); i != e; ++i)
+Attrs.push_back(Record.readAttr());

Identifiers don't meet coding style rules.



Comment at: lib/Serialization/ASTWriter.cpp:4485-4486
+  push_back(Attrs.size());
+  for (const auto *A : Attrs)
+AddAttr(A);
 }

`llvm::for_each(Attrs, AddAttr);` ?


Repository:
  rC Clang

https://reviews.llvm.org/D50526



___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D50467: [SEMA] add more -Wfloat-conversion to compound assigment analysis

[Arnaud Coomans via Phabricator via cfe-commits]

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

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D50467



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

[PATCH] D50504: [analyzer][UninitializedObjectChecker] Refactoring p2.: Moving pointer chasing to a separate file

[George Karpenkov via Phabricator via cfe-commits]

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

Overall: great! Thank you for following this direction!

However, to avoid clutter, could we put all the checker files into a separate 
folder, like MPI-Checker?


Repository:
  rC Clang

https://reviews.llvm.org/D50504



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

[PATCH] D50576: [clangd] Allow consumption of DocIDs without overhead

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev created this revision.
kbobyrev added reviewers: ioeric, ilya-biryukov.
Herald added subscribers: arphaman, jkorous, MaskRay.

This patch allows processing DocIDs from iterator using callback so that they 
are not stored in a vector if actual DocIDs are not needed.

Such overhead is the case for https://reviews.llvm.org/D50337 patch: 
`fuzzyFindLongQuery` stores `SymbolDocIDs` but they are thrown away later, 
because what the index really needs is `std::vector> Scores;` and it can be filled on-the-fly in `matchSymbols`.


https://reviews.llvm.org/D50576

Files:
  clang-tools-extra/clangd/index/dex/Iterator.cpp
  clang-tools-extra/clangd/index/dex/Iterator.h


Index: clang-tools-extra/clangd/index/dex/Iterator.h
===
--- clang-tools-extra/clangd/index/dex/Iterator.h
+++ clang-tools-extra/clangd/index/dex/Iterator.h
@@ -101,8 +101,14 @@
   virtual llvm::raw_ostream (llvm::raw_ostream ) const = 0;
 };
 
-/// Advances the iterator until it is either exhausted or the number of
-/// requested items is reached. The result contains sorted DocumentIDs.
+/// Advances given iterator until it is exhausted or the requested number of
+/// symbols is already seen while using callback on each processed DocID.
+void matchSymbols(Iterator ,
+  llvm::function_ref Callback,
+  size_t Limit = std::numeric_limits::max());
+
+/// Advances given iterator until it is either exhausted or the number of
+/// requested items is reached and returns all seen DocIDs.
 std::vector consume(Iterator ,
size_t Limit = std::numeric_limits::max());
 
Index: clang-tools-extra/clangd/index/dex/Iterator.cpp
===
--- clang-tools-extra/clangd/index/dex/Iterator.cpp
+++ clang-tools-extra/clangd/index/dex/Iterator.cpp
@@ -220,10 +220,16 @@
 
 std::vector consume(Iterator , size_t Limit) {
   std::vector Result;
+  matchSymbols(It, [&](const DocID ) { Result.push_back(ID); }, Limit);
+  return Result;
+}
+
+void matchSymbols(Iterator ,
+  llvm::function_ref Callback,
+  size_t Limit) {
   for (size_t Retrieved = 0; !It.reachedEnd() && Retrieved < Limit;
It.advance(), ++Retrieved)
-Result.push_back(It.peek());
-  return Result;
+Callback(It.peek());
 }
 
 std::unique_ptr create(PostingListRef Documents) {


Index: clang-tools-extra/clangd/index/dex/Iterator.h
===
--- clang-tools-extra/clangd/index/dex/Iterator.h
+++ clang-tools-extra/clangd/index/dex/Iterator.h
@@ -101,8 +101,14 @@
   virtual llvm::raw_ostream (llvm::raw_ostream ) const = 0;
 };
 
-/// Advances the iterator until it is either exhausted or the number of
-/// requested items is reached. The result contains sorted DocumentIDs.
+/// Advances given iterator until it is exhausted or the requested number of
+/// symbols is already seen while using callback on each processed DocID.
+void matchSymbols(Iterator ,
+  llvm::function_ref Callback,
+  size_t Limit = std::numeric_limits::max());
+
+/// Advances given iterator until it is either exhausted or the number of
+/// requested items is reached and returns all seen DocIDs.
 std::vector consume(Iterator ,
size_t Limit = std::numeric_limits::max());
 
Index: clang-tools-extra/clangd/index/dex/Iterator.cpp
===
--- clang-tools-extra/clangd/index/dex/Iterator.cpp
+++ clang-tools-extra/clangd/index/dex/Iterator.cpp
@@ -220,10 +220,16 @@
 
 std::vector consume(Iterator , size_t Limit) {
   std::vector Result;
+  matchSymbols(It, [&](const DocID ) { Result.push_back(ID); }, Limit);
+  return Result;
+}
+
+void matchSymbols(Iterator ,
+  llvm::function_ref Callback,
+  size_t Limit) {
   for (size_t Retrieved = 0; !It.reachedEnd() && Retrieved < Limit;
It.advance(), ++Retrieved)
-Result.push_back(It.peek());
-  return Result;
+Callback(It.peek());
 }
 
 std::unique_ptr create(PostingListRef Documents) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r339459 - Invalidate static locals when escaping lambdas

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Fri Aug 10 11:28:04 2018
New Revision: 339459

URL: http://llvm.org/viewvc/llvm-project?rev=339459=rev
Log:
Invalidate static locals when escaping lambdas

Lambdas can affect static locals even without an explicit capture.

rdar://39537031

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
cfe/trunk/test/Analysis/lambdas.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=339459=339458=339459=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Fri Aug 10 11:28:04 2018
@@ -17,6 +17,7 @@
 
 #include "clang/AST/Attr.h"
 #include "clang/AST/CharUnits.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Analysis/Analyses/LiveVariables.h"
 #include "clang/Analysis/AnalysisDeclContext.h"
 #include "clang/Basic/TargetInfo.h"
@@ -1033,6 +1034,32 @@ void invalidateRegionsWorker::VisitClust
   B = B.remove(baseR);
   }
 
+  if (const auto *TO = dyn_cast(baseR)) {
+if (const auto *RD = TO->getValueType()->getAsCXXRecordDecl()) {
+
+  // Lambdas can affect all static local variables without explicitly
+  // capturing those.
+  // We invalidate all static locals referenced inside the lambda body.
+  if (RD->isLambda() && RD->getLambdaCallOperator()->getBody()) {
+using namespace ast_matchers;
+
+const char *DeclBind = "DeclBind";
+StatementMatcher RefToStatic = stmt(hasDescendant(declRefExpr(
+  to(varDecl(hasStaticStorageDuration()).bind(DeclBind);
+auto Matches =
+match(RefToStatic, *RD->getLambdaCallOperator()->getBody(),
+  RD->getASTContext());
+
+for (BoundNodes  : Matches) {
+  auto *VD = Match.getNodeAs(DeclBind);
+  const VarRegion *ToInvalidate =
+  RM.getRegionManager().getVarRegion(VD, LCtx);
+  AddToWorkList(ToInvalidate);
+}
+  }
+}
+  }
+
   // BlockDataRegion?  If so, invalidate captured variables that are passed
   // by reference.
   if (const BlockDataRegion *BR = dyn_cast(baseR)) {

Modified: cfe/trunk/test/Analysis/lambdas.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/lambdas.cpp?rev=339459=339458=339459=diff
==
--- cfe/trunk/test/Analysis/lambdas.cpp (original)
+++ cfe/trunk/test/Analysis/lambdas.cpp Fri Aug 10 11:28:04 2018
@@ -1,10 +1,26 @@
 // RUN: %clang_analyze_cc1 -std=c++11 
-analyzer-checker=core,deadcode,debug.ExprInspection -analyzer-config 
inline-lambdas=true -verify %s
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core -analyzer-config 
inline-lambdas=false -DNO_INLINING=1 -verify %s
 // RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,debug.DumpCFG 
-analyzer-config inline-lambdas=true %s > %t 2>&1
 // RUN: FileCheck --input-file=%t %s
 
 void clang_analyzer_warnIfReached();
 void clang_analyzer_eval(int);
 
+#ifdef NO_INLINING
+
+// expected-no-diagnostics
+
+int& invalidate_static_on_unknown_lambda() {
+  static int* z;
+  auto f = [] {
+z = nullptr;
+  }; // should invalidate "z" when inlining is disabled.
+  f();
+  return *z; // no-warning
+}
+
+#else
+
 struct X { X(const X&); };
 void f(X x) { (void) [x]{}; }
 
@@ -348,6 +364,18 @@ void testCapturedConstExprFloat() {
   lambda();
 }
 
+void escape(void*);
+
+int& invalidate_static_on_unknown_lambda() {
+  static int* z;
+  auto lambda = [] {
+static float zz;
+z = new int(120);
+  };
+  escape();
+  return *z; // no-warning
+}
+
 
 static int b = 0;
 
@@ -365,6 +393,8 @@ int f() {
   return 0;
 }
 
+#endif
+
 // CHECK: [B2 (ENTRY)]
 // CHECK:   Succs (1): B1
 // CHECK: [B1]


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

[PATCH] D50517: [clangd] Generate incomplete trigrams for the Dex index

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 160154.
kbobyrev marked 7 inline comments as done.
kbobyrev added a comment.

Address a round of comments.


https://reviews.llvm.org/D50517

Files:
  clang-tools-extra/clangd/index/dex/Iterator.h
  clang-tools-extra/clangd/index/dex/Trigram.cpp
  clang-tools-extra/clangd/index/dex/Trigram.h
  clang-tools-extra/unittests/clangd/DexIndexTests.cpp

Index: clang-tools-extra/unittests/clangd/DexIndexTests.cpp
===
--- clang-tools-extra/unittests/clangd/DexIndexTests.cpp
+++ clang-tools-extra/unittests/clangd/DexIndexTests.cpp
@@ -250,45 +250,57 @@
 }
 
 TEST(DexIndexTrigrams, IdentifierTrigrams) {
-  EXPECT_THAT(generateIdentifierTrigrams("X86"), trigramsAre({"x86"}));
+  EXPECT_THAT(generateIdentifierTrigrams("X86"),
+  trigramsAre({"x86", "x$$", "x8$", "$$$"}));
 
-  EXPECT_THAT(generateIdentifierTrigrams("nl"), trigramsAre({}));
+  EXPECT_THAT(generateIdentifierTrigrams("nl"),
+  trigramsAre({"nl$", "n$$", "$$$"}));
+
+  EXPECT_THAT(generateIdentifierTrigrams("n"), trigramsAre({"n$$", "$$$"}));
 
   EXPECT_THAT(generateIdentifierTrigrams("clangd"),
-  trigramsAre({"cla", "lan", "ang", "ngd"}));
+  trigramsAre({"c$$", "cl$", "cla", "lan", "ang", "ngd", "$$$"}));
 
   EXPECT_THAT(generateIdentifierTrigrams("abc_def"),
-  trigramsAre({"abc", "abd", "ade", "bcd", "bde", "cde", "def"}));
+  trigramsAre({"a$$", "abc", "abd", "ade", "bcd", "bde", "cde",
+   "def", "ab$", "ad$", "$$$"}));
 
-  EXPECT_THAT(
-  generateIdentifierTrigrams("a_b_c_d_e_"),
-  trigramsAre({"abc", "abd", "acd", "ace", "bcd", "bce", "bde", "cde"}));
+  EXPECT_THAT(generateIdentifierTrigrams("a_b_c_d_e_"),
+  trigramsAre({"a$$", "a_$", "a_b", "abc", "abd", "acd", "ace",
+   "bcd", "bce", "bde", "cde", "ab$", "$$$"}));
 
-  EXPECT_THAT(
-  generateIdentifierTrigrams("unique_ptr"),
-  trigramsAre({"uni", "unp", "upt", "niq", "nip", "npt", "iqu", "iqp",
-   "ipt", "que", "qup", "qpt", "uep", "ept", "ptr"}));
+  EXPECT_THAT(generateIdentifierTrigrams("unique_ptr"),
+  trigramsAre({"u$$", "uni", "unp", "upt", "niq", "nip", "npt",
+   "iqu", "iqp", "ipt", "que", "qup", "qpt", "uep",
+   "ept", "ptr", "un$", "up$", "$$$"}));
 
   EXPECT_THAT(generateIdentifierTrigrams("TUDecl"),
-  trigramsAre({"tud", "tde", "ude", "dec", "ecl"}));
+  trigramsAre({"t$$", "tud", "tde", "ude", "dec", "ecl", "tu$",
+   "td$", "$$$"}));
 
   EXPECT_THAT(generateIdentifierTrigrams("IsOK"),
-  trigramsAre({"iso", "iok", "sok"}));
-
-  EXPECT_THAT(generateIdentifierTrigrams("abc_defGhij__klm"),
-  trigramsAre({
-  "abc", "abd", "abg", "ade", "adg", "adk", "agh", "agk", "bcd",
-  "bcg", "bde", "bdg", "bdk", "bgh", "bgk", "cde", "cdg", "cdk",
-  "cgh", "cgk", "def", "deg", "dek", "dgh", "dgk", "dkl", "efg",
-  "efk", "egh", "egk", "ekl", "fgh", "fgk", "fkl", "ghi", "ghk",
-  "gkl", "hij", "hik", "hkl", "ijk", "ikl", "jkl", "klm",
-  }));
+  trigramsAre({"i$$", "iso", "iok", "sok", "is$", "io$", "$$$"}));
+
+  EXPECT_THAT(
+  generateIdentifierTrigrams("abc_defGhij__klm"),
+  trigramsAre({"a$$", "abc", "abd", "abg", "ade", "adg", "adk", "agh",
+   "agk", "bcd", "bcg", "bde", "bdg", "bdk", "bgh", "bgk",
+   "cde", "cdg", "cdk", "cgh", "cgk", "def", "deg", "dek",
+   "dgh", "dgk", "dkl", "efg", "efk", "egh", "egk", "ekl",
+   "fgh", "fgk", "fkl", "ghi", "ghk", "gkl", "hij", "hik",
+   "hkl", "ijk", "ikl", "jkl", "klm", "ab$", "ad$", "$$$"}));
 }
 
 TEST(DexIndexTrigrams, QueryTrigrams) {
-  EXPECT_THAT(generateQueryTrigrams("X86"), trigramsAre({"x86"}));
+  EXPECT_THAT(generateQueryTrigrams("c"), trigramsAre({"c$$"}));
+  EXPECT_THAT(generateQueryTrigrams("cl"), trigramsAre({"cl$"}));
+  EXPECT_THAT(generateQueryTrigrams("cla"), trigramsAre({"cla"}));
 
-  EXPECT_THAT(generateQueryTrigrams("nl"), trigramsAre({}));
+  EXPECT_THAT(generateQueryTrigrams("_"), trigramsAre({"_$$"}));
+  EXPECT_THAT(generateQueryTrigrams("__"), trigramsAre({"__$"}));
+  EXPECT_THAT(generateQueryTrigrams("___"), trigramsAre({"___"}));
+
+  EXPECT_THAT(generateQueryTrigrams("X86"), trigramsAre({"x86"}));
 
   EXPECT_THAT(generateQueryTrigrams("clangd"),
   trigramsAre({"cla", "lan", "ang", "ngd"}));
Index: clang-tools-extra/clangd/index/dex/Trigram.h
===
--- clang-tools-extra/clangd/index/dex/Trigram.h
+++ clang-tools-extra/clangd/index/dex/Trigram.h
@@ -36,14 +36,20 @@
 /// First, given Identifier (unqualified symbol name) is segmented using
 /// 

[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like keys

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang added a comment.

Thanks for all your review comments. I will try to address them soon.

Meanwhile, I have started an email thread on the general problem of writing 
checkers/sanitizers to detect non-deterministic behaviors. See 
http://lists.llvm.org/pipermail/llvm-dev/2018-August/125191.html.

The background for this is that I had already added support in LLVM to uncover 
2 types of non-deterministic behaviors: iteration of unordered containers and 
sorting of keys with same values. But that is only for LLVM code, not user 
code. I now wanted to write checkers so that we could detect non-deterministic 
behavior in user code. It would be great if the reviewers here could chip-in 
with comments/suggestions for this. Thanks!


Repository:
  rC Clang

https://reviews.llvm.org/D50488



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

[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like keys

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

> I'm only a beginner, but here are some things that caught my eye. I really 
> like the idea! :)

Thanks, i appreciate help with reviews greatly.




Comment at: lib/StaticAnalyzer/Checkers/PointerSortingChecker.cpp:93
+  const RecordDecl *RD =
+IterTy->getUnqualifiedDesugaredType()->getAsCXXRecordDecl();
+

Szelethus wrote:
> Is there a reason for not directly acquiring the record declaration?
I suspect it's about typedefs. We usually use `getCanonicalType()` for this 
purpose.



Comment at: test/Analysis/ptr-sort.cpp:1
+// RUN: %clang_analyze_cc1 
-analyzer-checker=alpha.nondeterminism.PointerSorting %s -analyzer-output=text 
-verify
+

Szelethus wrote:
> Always run the core checkers too.
> `-analyzer-checker=core,alpha.nondeterminism.PointerSorting`
This is a good discipline, but it's not very useful in case of AST-based 
checkers because they don't interact at all with each other.


Repository:
  rC Clang

https://reviews.llvm.org/D50488



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

[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like keys

[George Karpenkov via Phabricator via cfe-commits]

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

All the comments above + if anything, should use ASTMatchers.


Repository:
  rC Clang

https://reviews.llvm.org/D50488



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

[PATCH] D50337: [clangd] DexIndex implementation prototype

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 160146.
kbobyrev marked 2 inline comments as done.
kbobyrev added a comment.

Store symbol qualities (so that it's not computed each time when requested 
which might be expensive). Use `operator[]` to construct the value for inverted 
index when key is not inserted yet.


https://reviews.llvm.org/D50337

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/dex/DexIndex.cpp
  clang-tools-extra/clangd/index/dex/DexIndex.h
  clang-tools-extra/clangd/index/dex/Token.h
  clang-tools-extra/unittests/clangd/CMakeLists.txt
  clang-tools-extra/unittests/clangd/DexIndexTests.cpp
  clang-tools-extra/unittests/clangd/IndexTests.cpp
  clang-tools-extra/unittests/clangd/TestIndexOperations.cpp
  clang-tools-extra/unittests/clangd/TestIndexOperations.h

Index: clang-tools-extra/unittests/clangd/TestIndexOperations.h
===
--- /dev/null
+++ clang-tools-extra/unittests/clangd/TestIndexOperations.h
@@ -0,0 +1,57 @@
+//===-- IndexHelpers.h --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_INDEXTESTCOMMON_H
+#define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_INDEXTESTCOMMON_H
+
+#include "index/Index.h"
+#include "index/Merge.h"
+#include "index/dex/DexIndex.h"
+#include "index/dex/Iterator.h"
+#include "index/dex/Token.h"
+#include "index/dex/Trigram.h"
+
+namespace clang {
+namespace clangd {
+
+Symbol symbol(llvm::StringRef QName);
+
+struct SlabAndPointers {
+  SymbolSlab Slab;
+  std::vector Pointers;
+};
+
+// Create a slab of symbols with the given qualified names as both IDs and
+// names. The life time of the slab is managed by the returned shared pointer.
+// If \p WeakSymbols is provided, it will be pointed to the managed object in
+// the returned shared pointer.
+std::shared_ptr>
+generateSymbols(std::vector QualifiedNames,
+std::weak_ptr *WeakSymbols = nullptr);
+
+// Create a slab of symbols with IDs and names [Begin, End], otherwise identical
+// to the `generateSymbols` above.
+std::shared_ptr>
+generateNumSymbols(int Begin, int End,
+   std::weak_ptr *WeakSymbols = nullptr);
+
+std::string getQualifiedName(const Symbol );
+
+std::vector match(const SymbolIndex ,
+   const FuzzyFindRequest ,
+   bool *Incomplete = nullptr);
+
+// Returns qualified names of symbols with any of IDs in the index.
+std::vector lookup(const SymbolIndex ,
+llvm::ArrayRef IDs);
+
+} // namespace clangd
+} // namespace clang
+
+#endif
Index: clang-tools-extra/unittests/clangd/TestIndexOperations.cpp
===
--- /dev/null
+++ clang-tools-extra/unittests/clangd/TestIndexOperations.cpp
@@ -0,0 +1,89 @@
+//===-- IndexHelpers.cpp *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "TestIndexOperations.h"
+
+namespace clang {
+namespace clangd {
+
+Symbol symbol(llvm::StringRef QName) {
+  Symbol Sym;
+  Sym.ID = SymbolID(QName.str());
+  size_t Pos = QName.rfind("::");
+  if (Pos == llvm::StringRef::npos) {
+Sym.Name = QName;
+Sym.Scope = "";
+  } else {
+Sym.Name = QName.substr(Pos + 2);
+Sym.Scope = QName.substr(0, Pos + 2);
+  }
+  return Sym;
+}
+
+// Create a slab of symbols with the given qualified names as both IDs and
+// names. The life time of the slab is managed by the returned shared pointer.
+// If \p WeakSymbols is provided, it will be pointed to the managed object in
+// the returned shared pointer.
+std::shared_ptr>
+generateSymbols(std::vector QualifiedNames,
+std::weak_ptr *WeakSymbols) {
+  SymbolSlab::Builder Slab;
+  for (llvm::StringRef QName : QualifiedNames)
+Slab.insert(symbol(QName));
+
+  auto Storage = std::make_shared();
+  Storage->Slab = std::move(Slab).build();
+  for (const auto  : Storage->Slab)
+Storage->Pointers.push_back();
+  if (WeakSymbols)
+*WeakSymbols = Storage;
+  auto *Pointers = >Pointers;
+  return {std::move(Storage), Pointers};
+}
+
+// Create a slab of symbols with IDs and names [Begin, End], otherwise identical
+// to the `generateSymbols` above.
+std::shared_ptr>
+generateNumSymbols(int Begin, int End,
+   std::weak_ptr *WeakSymbols) {
+  std::vector Names;
+  for (int i = Begin; i <= End; i++)
+

[PATCH] D50571: [clangd] add an extension field to LSP to transfer the diagnostic's category

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.
arphaman added reviewers: jkorous, ilya-biryukov, sammccall.
Herald added subscribers: dexonsmith, MaskRay, ioeric.

This patch adds a 'category' extension field to the LSP diagnostic that's sent 
by Clangd. This extension is always on by default.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50571

Files:
  clangd/ClangdLSPServer.cpp
  clangd/Diagnostics.cpp
  clangd/Diagnostics.h
  clangd/Protocol.h
  test/clangd/compile-commands-path-in-initialize.test
  test/clangd/compile-commands-path.test
  test/clangd/diagnostics.test
  test/clangd/did-change-configuration-params.test
  test/clangd/execute-command.test
  test/clangd/extra-flags.test
  test/clangd/fixits.test

Index: test/clangd/fixits.test
===
--- test/clangd/fixits.test
+++ test/clangd/fixits.test
@@ -6,6 +6,7 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"diagnostics": [
 # CHECK-NEXT:  {
+# CHECK-NEXT:"category": "Semantic Issue",
 # CHECK-NEXT:"message": "Using the result of an assignment as a condition without parentheses",
 # CHECK-NEXT:"range": {
 # CHECK-NEXT:  "end": {
Index: test/clangd/extra-flags.test
===
--- test/clangd/extra-flags.test
+++ test/clangd/extra-flags.test
@@ -6,6 +6,7 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"diagnostics": [
 # CHECK-NEXT:  {
+# CHECK-NEXT:"category": "Semantic Issue",
 # CHECK-NEXT:"message": "Variable 'i' is uninitialized when used here",
 # CHECK-NEXT:"range": {
 # CHECK-NEXT:  "end": {
@@ -28,6 +29,7 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"diagnostics": [
 # CHECK-NEXT:  {
+# CHECK-NEXT:"category": "Semantic Issue",
 # CHECK-NEXT:"message": "Variable 'i' is uninitialized when used here",
 # CHECK-NEXT:"range": {
 # CHECK-NEXT:  "end": {
Index: test/clangd/execute-command.test
===
--- test/clangd/execute-command.test
+++ test/clangd/execute-command.test
@@ -6,6 +6,7 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"diagnostics": [
 # CHECK-NEXT:  {
+# CHECK-NEXT:"category": "Semantic Issue",
 # CHECK-NEXT:"message": "Using the result of an assignment as a condition without parentheses",
 # CHECK-NEXT:"range": {
 # CHECK-NEXT:  "end": {
Index: test/clangd/did-change-configuration-params.test
===
--- test/clangd/did-change-configuration-params.test
+++ test/clangd/did-change-configuration-params.test
@@ -24,6 +24,7 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"diagnostics": [
 # CHECK-NEXT:  {
+# CHECK-NEXT:"category": "Semantic Issue",
 # CHECK-NEXT:"message": "Variable 'i' is uninitialized when used here",
 # CHECK-NEXT:"range": {
 # CHECK-NEXT:  "end": {
Index: test/clangd/diagnostics.test
===
--- test/clangd/diagnostics.test
+++ test/clangd/diagnostics.test
@@ -6,6 +6,7 @@
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:"diagnostics": [
 # CHECK-NEXT:  {
+# CHECK-NEXT:"category": "Semantic Issue",
 # CHECK-NEXT:"message": "Return type of 'main' is not 'int'",
 # CHECK-NEXT:"range": {
 # CHECK-NEXT:  "end": {
Index: test/clangd/compile-commands-path.test
===
--- test/clangd/compile-commands-path.test
+++ test/clangd/compile-commands-path.test
@@ -23,20 +23,23 @@
 # CHECK-NEXT:   "params": {
 # CHECK-NEXT: "diagnostics": [
 # CHECK-NEXT:   {
+# CHECK-NEXT: "category": "#pragma message Directive",
 # CHECK-NEXT: "message": "MACRO is not defined",
 ---
 {"jsonrpc":"2.0","id":0,"method":"workspace/didChangeConfiguration","params":{"settings":{"compilationDatabasePath":"INPUT_DIR/build-1"}}}
 # CHECK:   "method": "textDocument/publishDiagnostics",
 # CHECK-NEXT:   "params": {
 # CHECK-NEXT: "diagnostics": [
 # CHECK-NEXT:   {
+# CHECK-NEXT: "category": "#pragma message Directive",
 # CHECK-NEXT: "message": "MACRO is one",
 ---
 {"jsonrpc":"2.0","id":0,"method":"workspace/didChangeConfiguration","params":{"settings":{"compilationDatabasePath":"INPUT_DIR/build-2"}}}
 # CHECK:   "method": "textDocument/publishDiagnostics",
 # CHECK-NEXT:   "params": {
 # CHECK-NEXT: "diagnostics": [
 # CHECK-NEXT:   {
+# CHECK-NEXT: "category": "#pragma message Directive",
 # CHECK-NEXT: "message": "MACRO is two",
 ---
 {"jsonrpc":"2.0","id":1,"method":"shutdown"}
Index: test/clangd/compile-commands-path-in-initialize.test
===
--- test/clangd/compile-commands-path-in-initialize.test
+++ 

r339456 - Allow relockable scopes with thread safety attributes.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Fri Aug 10 10:33:47 2018
New Revision: 339456

URL: http://llvm.org/viewvc/llvm-project?rev=339456=rev
Log:
Allow relockable scopes with thread safety attributes.

Patch by Aaron Puchert

Modified:
cfe/trunk/lib/Analysis/ThreadSafety.cpp
cfe/trunk/test/SemaCXX/warn-thread-safety-analysis.cpp

Modified: cfe/trunk/lib/Analysis/ThreadSafety.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ThreadSafety.cpp?rev=339456=339455=339456=diff
==
--- cfe/trunk/lib/Analysis/ThreadSafety.cpp (original)
+++ cfe/trunk/lib/Analysis/ThreadSafety.cpp Fri Aug 10 10:33:47 2018
@@ -86,8 +86,8 @@ static void warnInvalidLock(ThreadSafety
 
 namespace {
 
-/// A set of CapabilityInfo objects, which are compiled from the
-/// requires attributes on a function.
+/// A set of CapabilityExpr objects, which are compiled from thread safety
+/// attributes on a function.
 class CapExprSet : public SmallVector {
 public:
   /// Push M onto list, but discard duplicates.
@@ -944,6 +944,23 @@ public:
 if (FullyRemove)
   FSet.removeLock(FactMan, Cp);
   }
+
+  void Relock(FactSet , FactManager , LockKind LK,
+  SourceLocation RelockLoc, ThreadSafetyHandler ,
+  StringRef DiagKind) const {
+for (const auto *UnderlyingMutex : UnderlyingMutexes) {
+  CapabilityExpr UnderCp(UnderlyingMutex, false);
+
+  // We're relocking the underlying mutexes. Warn on double locking.
+  if (FSet.findLock(FactMan, UnderCp))
+Handler.handleDoubleLock(DiagKind, UnderCp.toString(), RelockLoc);
+  else {
+FSet.removeLock(FactMan, !UnderCp);
+FSet.addLock(FactMan, llvm::make_unique(UnderCp, LK,
+   RelockLoc));
+  }
+}
+  }
 };
 
 /// Class which implements the core thread safety analysis routines.
@@ -974,6 +991,9 @@ public:
   void removeLock(FactSet , const CapabilityExpr ,
   SourceLocation UnlockLoc, bool FullyRemove, LockKind Kind,
   StringRef DiagKind);
+  void relockScopedLock(FactSet , const CapabilityExpr ,
+SourceLocation RelockLoc, LockKind Kind,
+StringRef DiagKind);
 
   template 
   void getMutexIDs(CapExprSet , AttrType *Attr, Expr *Exp,
@@ -1285,6 +1305,27 @@ void ThreadSafetyAnalyzer::removeLock(Fa
  DiagKind);
 }
 
+void ThreadSafetyAnalyzer::relockScopedLock(FactSet ,
+const CapabilityExpr ,
+SourceLocation RelockLoc,
+LockKind Kind, StringRef DiagKind) 
{
+  if (Cp.shouldIgnore())
+return;
+
+  const FactEntry *LDat = FSet.findLock(FactMan, Cp);
+  if (!LDat) {
+// FIXME: It's possible to manually destruct the scope and then relock it.
+// Should that be a separate warning? For now we pretend nothing happened.
+// It's undefined behavior, so not related to thread safety.
+return;
+  }
+
+  // We should only land here if Cp is a scoped capability, so if we have any
+  // fact, it must be a ScopedLockableFactEntry.
+  const auto *SLDat = static_cast(LDat);
+  SLDat->Relock(FSet, FactMan, Kind, RelockLoc, Handler, DiagKind);
+}
+
 /// Extract the list of mutexIDs from the attribute on an expression,
 /// and push them onto Mtxs, discarding any duplicates.
 template 
@@ -1726,14 +1767,19 @@ void BuildLockset::handleCall(Expr *Exp,
   StringRef CapDiagKind = "mutex";
 
   // Figure out if we're constructing an object of scoped lockable class
-  bool isScopedVar = false;
+  bool isScopedConstructor = false, isScopedMemberCall = false;
   if (VD) {
 if (const auto *CD = dyn_cast(D)) {
   const CXXRecordDecl* PD = CD->getParent();
   if (PD && PD->hasAttr())
-isScopedVar = true;
+isScopedConstructor = true;
 }
   }
+  if (const auto *MCD = dyn_cast(Exp)) {
+const CXXRecordDecl *PD = MCD->getRecordDecl();
+if (PD && PD->hasAttr())
+  isScopedMemberCall = true;
+  }
 
   for(const Attr *At : D->attrs()) {
 switch (At->getKind()) {
@@ -1813,7 +1859,7 @@ void BuildLockset::handleCall(Expr *Exp,
  POK_FunctionCall, ClassifyDiagnostic(A),
  Exp->getExprLoc());
   // use for adopting a lock
-  if (isScopedVar) {
+  if (isScopedConstructor) {
 Analyzer->getMutexIDs(A->isShared() ? ScopedSharedReqs
 : ScopedExclusiveReqs,
   A, Exp, D, VD);
@@ -1846,16 +1892,24 @@ void BuildLockset::handleCall(Expr *Exp,
 Analyzer->removeLock(FSet, M, Loc, Dtor, LK_Generic, CapDiagKind);
 
   // Add locks.
-  for (const auto  : ExclusiveLocksToAdd)
-Analyzer->addLock(FSet, llvm::make_unique(
-M, 

[PATCH] D49885: Thread safety analysis: Allow relockable scopes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

Committed in r339456; if @delesley has concerns with the commit, they can be 
addressed post-commit. Thank you for the patch!


Repository:
  rC Clang

https://reviews.llvm.org/D49885



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

[PATCH] D50517: [clangd] Generate incomplete trigrams for the Dex index

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clang-tools-extra/clangd/index/dex/Trigram.cpp:83
+
+  bool FoundFirstHead = false;
+  // Iterate through valid seqneces of three characters Fuzzy Matcher can

ioeric wrote:
> It's probably unclear what `FoundFirstHead` and `FoundSecondHead` are for to 
> readers without context (i.e. we are looking first two HEADs). I think it's 
> would be cleaner if we handle this out side of the look e.g. record the first 
> head in the `Next` initialization loop above.
Sorry, full of typos here: 

I think it would be cleaner if we handle this outside of the loop e.g. record 
the first head in the `Next` initialization loop above.




https://reviews.llvm.org/D50517



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

[PATCH] D50517: [clangd] Generate incomplete trigrams for the Dex index

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clang-tools-extra/clangd/index/dex/Trigram.cpp:31
+/// use it as the special symbol.
+const auto END_MARKER = '$';
+

nit: s/auto/char/

Maybe just use `static` instead of an anonymous namespace just for this.



Comment at: clang-tools-extra/clangd/index/dex/Trigram.cpp:83
+
+  bool FoundFirstHead = false;
+  // Iterate through valid seqneces of three characters Fuzzy Matcher can

It's probably unclear what `FoundFirstHead` and `FoundSecondHead` are for to 
readers without context (i.e. we are looking first two HEADs). I think it's 
would be cleaner if we handle this out side of the look e.g. record the first 
head in the `Next` initialization loop above.



Comment at: clang-tools-extra/clangd/index/dex/Trigram.cpp:138
   DenseSet UniqueTrigrams;
   std::deque Chars;
 

nit: `Chars` is only used in the else branch?



Comment at: clang-tools-extra/clangd/index/dex/Trigram.cpp:143
+  if (ValidSymbolsCount < 3) {
+std::string Symbols = {{END_MARKER, END_MARKER, END_MARKER}};
+  if (LowercaseQuery.size() > 0)

I think this can be simplified as:
```
std::string Chars = LowercaseQuery.substr(std::min(3, LowercaseQuery.size()));
Chars.append(END_MARKER, 3-Chars.size());
UniqueTrigrams.insert(Token(Token::Kind::Trigram, Chars));
```

also nit: avoid using the term "symbol" here.



Comment at: clang-tools-extra/clangd/index/dex/Trigram.cpp:167
 
-Chars.push_back(LowercaseQuery[I]);
+if (Chars.front() == END_MARKER)
+  auto Trigram = Token(Token::Kind::Trigram,

When would this happen? And why are we reversing the trigram and throwing it 
away?



Comment at: clang-tools-extra/clangd/index/dex/Trigram.h:59
+/// * Bigram with the first two symbols (if availible), same logic applies.
+///
 /// Note: the returned list of trigrams does not have duplicates, if any 
trigram

ioeric wrote:
> I think the comment can be simplified a bit:
> ```
> This also generates incomplete trigrams for short query scenarios:
>   * Empty trigram: "$$$"
>   * Unigram: the first character of the identifier.
>   * Bigrams: a 2-char prefix of the identifier and a bigram of the first two 
> HEAD characters (if it exists).
> ```
nit: and the previous paragraph `Special kind of trigrams ...` is redundant now 
IMO.


https://reviews.llvm.org/D50517



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

[PATCH] D50559: [gnu-objc] Make selector order deterministic.

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang added a comment.

Does the error show up if you build llvm with -DLLVM_REVERSE_ITERATION:ON?

  LLVM_REVERSE_ITERATION:BOOL
  If enabled, all supported unordered llvm containers would be iterated in 
reverse order. This is useful for uncovering non-determinism caused by 
iteration of unordered containers.


Repository:
  rC Clang

https://reviews.llvm.org/D50559



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

[PATCH] D50055: Update the coding standard about NFC changes and whitespace

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman marked 2 inline comments as done.
aaron.ballman added a comment.

Thank you for the review and discussion -- I've commit in r339455.


https://reviews.llvm.org/D50055



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

[PATCH] D50415: [clangd] extend the publishDiagnostics response to send back fixits to the client directly as well (if requested)

[Alex Lorenz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
arphaman marked an inline comment as done.
Closed by commit rCTE339454: [clangd] extend the publishDiagnostics response to 
send back fixits to the (authored by arphaman, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D50415?vs=160007=160137#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50415

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

Index: clangd/ClangdLSPServer.h
===
--- clangd/ClangdLSPServer.h
+++ clangd/ClangdLSPServer.h
@@ -155,6 +155,8 @@
   RealFileSystemProvider FSProvider;
   /// Options used for code completion
   clangd::CodeCompleteOptions CCOpts;
+  /// Options used for diagnostics.
+  ClangdDiagnosticOptions DiagOpts;
   /// The supported kinds of the client.
   SymbolKindBitset SupportedSymbolKinds;
 
Index: clangd/Diagnostics.h
===
--- clangd/Diagnostics.h
+++ clangd/Diagnostics.h
@@ -23,6 +23,12 @@
 namespace clang {
 namespace clangd {
 
+struct ClangdDiagnosticOptions {
+  /// If true, Clangd uses an LSP extension to embed the fixes with the
+  /// diagnostics that are sent to the client.
+  bool EmbedFixesInDiagnostics = false;
+};
+
 /// Contains basic information about a diagnostic.
 struct DiagBase {
   std::string Message;
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -178,6 +178,15 @@
   return true;
 }
 
+bool fromJSON(const llvm::json::Value ,
+  PublishDiagnosticsClientCapabilities ) {
+  json::ObjectMapper O(Params);
+  if (!O)
+return false;
+  O.map("clangdFixSupport", R.clangdFixSupport);
+  return true;
+}
+
 bool fromJSON(const json::Value , SymbolKind ) {
   if (auto T = E.getAsInteger()) {
 if (*T < static_cast(SymbolKind::File) ||
@@ -240,6 +249,7 @@
   if (!O)
 return false;
   O.map("completion", R.completion);
+  O.map("publishDiagnostics", R.publishDiagnostics);
   return true;
 }
 
Index: clangd/ClangdLSPServer.cpp
===
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -82,6 +82,8 @@
 
   CCOpts.EnableSnippets =
   Params.capabilities.textDocument.completion.completionItem.snippetSupport;
+  DiagOpts.EmbedFixesInDiagnostics =
+  Params.capabilities.textDocument.publishDiagnostics.clangdFixSupport;
 
   if (Params.capabilities.workspace && Params.capabilities.workspace->symbol &&
   Params.capabilities.workspace->symbol->symbolKind) {
@@ -486,11 +488,25 @@
   DiagnosticToReplacementMap LocalFixIts; // Temporary storage
   for (auto  : Diagnostics) {
 toLSPDiags(Diag, [&](clangd::Diagnostic Diag, llvm::ArrayRef Fixes) {
-  DiagnosticsJSON.push_back(json::Object{
+  json::Object LSPDiag({
   {"range", Diag.range},
   {"severity", Diag.severity},
   {"message", Diag.message},
   });
+  // LSP extension: embed the fixes in the diagnostic.
+  if (DiagOpts.EmbedFixesInDiagnostics && !Fixes.empty()) {
+json::Array ClangdFixes;
+for (const auto  : Fixes) {
+  WorkspaceEdit WE;
+  URIForFile URI{File};
+  WE.changes = {{URI.uri(), std::vector(Fix.Edits.begin(),
+  Fix.Edits.end())}};
+  ClangdFixes.push_back(
+  json::Object{{"edit", toJSON(WE)}, {"title", Fix.Message}});
+}
+LSPDiag["clangd_fixes"] = std::move(ClangdFixes);
+  }
+  DiagnosticsJSON.push_back(std::move(LSPDiag));
 
   auto  = LocalFixIts[Diag];
   std::copy(Fixes.begin(), Fixes.end(),
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -247,6 +247,18 @@
 };
 bool fromJSON(const llvm::json::Value &, CompletionClientCapabilities &);
 
+struct PublishDiagnosticsClientCapabilities {
+  // Whether the client accepts diagnostics with related information.
+  // NOTE: not used by clangd at the moment.
+  // bool relatedInformation;
+
+  /// Whether the client accepts diagnostics with fixes attached using the
+  /// "clangd_fixes" extension.
+  bool clangdFixSupport = false;
+};
+bool fromJSON(const llvm::json::Value &,
+  PublishDiagnosticsClientCapabilities &);
+
 /// A symbol kind.
 enum class SymbolKind {
   File = 1,
@@ -313,6 +325,9 @@
 struct TextDocumentClientCapabilities {
   /// Capabilities specific to the `textDocument/completion`
   CompletionClientCapabilities completion;
+
+  /// Capabilities specific to the 'textDocument/publishDiagnostics'
+  PublishDiagnosticsClientCapabilities publishDiagnostics;
 };
 bool fromJSON(const llvm::json::Value &, 

[clang-tools-extra] r339454 - [clangd] extend the publishDiagnostics response to send back fixits to the

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Fri Aug 10 10:25:07 2018
New Revision: 339454

URL: http://llvm.org/viewvc/llvm-project?rev=339454=rev
Log:
[clangd] extend the publishDiagnostics response to send back fixits to the
client if the client supports this extension

This commit extends the 'textDocument/publishDiagnostics' notification sent from
Clangd to the client.  When it's enabled, Clangd sends out the fixits associated
with the appropriate diagnostic in the body of the 'publishDiagnostics'
notification. The client can enable this extension by setting 'clangdFixSupport'
to true in the textDocument capabilities during initialization.

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

Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdLSPServer.h
clang-tools-extra/trunk/clangd/Diagnostics.h
clang-tools-extra/trunk/clangd/Protocol.cpp
clang-tools-extra/trunk/clangd/Protocol.h

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=339454=339453=339454=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Fri Aug 10 10:25:07 2018
@@ -82,6 +82,8 @@ void ClangdLSPServer::onInitialize(Initi
 
   CCOpts.EnableSnippets =
   
Params.capabilities.textDocument.completion.completionItem.snippetSupport;
+  DiagOpts.EmbedFixesInDiagnostics =
+  Params.capabilities.textDocument.publishDiagnostics.clangdFixSupport;
 
   if (Params.capabilities.workspace && Params.capabilities.workspace->symbol &&
   Params.capabilities.workspace->symbol->symbolKind) {
@@ -486,11 +488,25 @@ void ClangdLSPServer::onDiagnosticsReady
   DiagnosticToReplacementMap LocalFixIts; // Temporary storage
   for (auto  : Diagnostics) {
 toLSPDiags(Diag, [&](clangd::Diagnostic Diag, llvm::ArrayRef Fixes) {
-  DiagnosticsJSON.push_back(json::Object{
+  json::Object LSPDiag({
   {"range", Diag.range},
   {"severity", Diag.severity},
   {"message", Diag.message},
   });
+  // LSP extension: embed the fixes in the diagnostic.
+  if (DiagOpts.EmbedFixesInDiagnostics && !Fixes.empty()) {
+json::Array ClangdFixes;
+for (const auto  : Fixes) {
+  WorkspaceEdit WE;
+  URIForFile URI{File};
+  WE.changes = {{URI.uri(), std::vector(Fix.Edits.begin(),
+  Fix.Edits.end())}};
+  ClangdFixes.push_back(
+  json::Object{{"edit", toJSON(WE)}, {"title", Fix.Message}});
+}
+LSPDiag["clangd_fixes"] = std::move(ClangdFixes);
+  }
+  DiagnosticsJSON.push_back(std::move(LSPDiag));
 
   auto  = LocalFixIts[Diag];
   std::copy(Fixes.begin(), Fixes.end(),

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=339454=339453=339454=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Fri Aug 10 10:25:07 2018
@@ -155,6 +155,8 @@ private:
   RealFileSystemProvider FSProvider;
   /// Options used for code completion
   clangd::CodeCompleteOptions CCOpts;
+  /// Options used for diagnostics.
+  ClangdDiagnosticOptions DiagOpts;
   /// The supported kinds of the client.
   SymbolKindBitset SupportedSymbolKinds;
 

Modified: clang-tools-extra/trunk/clangd/Diagnostics.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Diagnostics.h?rev=339454=339453=339454=diff
==
--- clang-tools-extra/trunk/clangd/Diagnostics.h (original)
+++ clang-tools-extra/trunk/clangd/Diagnostics.h Fri Aug 10 10:25:07 2018
@@ -23,6 +23,12 @@
 namespace clang {
 namespace clangd {
 
+struct ClangdDiagnosticOptions {
+  /// If true, Clangd uses an LSP extension to embed the fixes with the
+  /// diagnostics that are sent to the client.
+  bool EmbedFixesInDiagnostics = false;
+};
+
 /// Contains basic information about a diagnostic.
 struct DiagBase {
   std::string Message;

Modified: clang-tools-extra/trunk/clangd/Protocol.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.cpp?rev=339454=339453=339454=diff
==
--- clang-tools-extra/trunk/clangd/Protocol.cpp (original)
+++ clang-tools-extra/trunk/clangd/Protocol.cpp Fri Aug 10 10:25:07 2018
@@ -178,6 +178,15 @@ bool fromJSON(const json::Value ,
   return true;
 }
 
+bool fromJSON(const llvm::json::Value ,
+  PublishDiagnosticsClientCapabilities ) {
+  json::ObjectMapper O(Params);
+  if 

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/ReleaseNotes.rst:58
+  
+  Warns Abseil users if they attempt to depend on internal details.
 

I think will be good idea to omit user, and just refer to code which depend on 
internal details. Please synchronize with documentations.



Comment at: test/clang-tidy/abseil-no-internal-deps.cpp:15
+}
+class SomeContainer{};
+

Please separate with empty line.



Comment at: test/clang-tidy/abseil-no-internal-deps.cpp:51
+
+
+

Please remove two empty lines


https://reviews.llvm.org/D50542



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

[PATCH] D50543: [libcxx] Mark charconv tests as failing for previous libcxx versions.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Thanks for the review.


Repository:
  rL LLVM

https://reviews.llvm.org/D50543



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

[PATCH] D50517: [clangd] Generate incomplete trigrams for the Dex index

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 160133.
kbobyrev marked 7 inline comments as done.
kbobyrev added a comment.

Address issues we have discussed with Eric.


https://reviews.llvm.org/D50517

Files:
  clang-tools-extra/clangd/index/dex/Iterator.h
  clang-tools-extra/clangd/index/dex/Trigram.cpp
  clang-tools-extra/clangd/index/dex/Trigram.h
  clang-tools-extra/unittests/clangd/DexIndexTests.cpp

Index: clang-tools-extra/unittests/clangd/DexIndexTests.cpp
===
--- clang-tools-extra/unittests/clangd/DexIndexTests.cpp
+++ clang-tools-extra/unittests/clangd/DexIndexTests.cpp
@@ -250,45 +250,57 @@
 }
 
 TEST(DexIndexTrigrams, IdentifierTrigrams) {
-  EXPECT_THAT(generateIdentifierTrigrams("X86"), trigramsAre({"x86"}));
+  EXPECT_THAT(generateIdentifierTrigrams("X86"),
+  trigramsAre({"x86", "x$$", "x8$", "$$$"}));
 
-  EXPECT_THAT(generateIdentifierTrigrams("nl"), trigramsAre({}));
+  EXPECT_THAT(generateIdentifierTrigrams("nl"),
+  trigramsAre({"nl$", "n$$", "$$$"}));
+
+  EXPECT_THAT(generateIdentifierTrigrams("n"), trigramsAre({"n$$", "$$$"}));
 
   EXPECT_THAT(generateIdentifierTrigrams("clangd"),
-  trigramsAre({"cla", "lan", "ang", "ngd"}));
+  trigramsAre({"c$$", "cl$", "cla", "lan", "ang", "ngd", "$$$"}));
 
   EXPECT_THAT(generateIdentifierTrigrams("abc_def"),
-  trigramsAre({"abc", "abd", "ade", "bcd", "bde", "cde", "def"}));
+  trigramsAre({"a$$", "abc", "abd", "ade", "bcd", "bde", "cde",
+   "def", "ab$", "ad$", "$$$"}));
 
-  EXPECT_THAT(
-  generateIdentifierTrigrams("a_b_c_d_e_"),
-  trigramsAre({"abc", "abd", "acd", "ace", "bcd", "bce", "bde", "cde"}));
+  EXPECT_THAT(generateIdentifierTrigrams("a_b_c_d_e_"),
+  trigramsAre({"a$$", "a_$", "a_b", "abc", "abd", "acd", "ace",
+   "bcd", "bce", "bde", "cde", "ab$", "$$$"}));
 
-  EXPECT_THAT(
-  generateIdentifierTrigrams("unique_ptr"),
-  trigramsAre({"uni", "unp", "upt", "niq", "nip", "npt", "iqu", "iqp",
-   "ipt", "que", "qup", "qpt", "uep", "ept", "ptr"}));
+  EXPECT_THAT(generateIdentifierTrigrams("unique_ptr"),
+  trigramsAre({"u$$", "uni", "unp", "upt", "niq", "nip", "npt",
+   "iqu", "iqp", "ipt", "que", "qup", "qpt", "uep",
+   "ept", "ptr", "un$", "up$", "$$$"}));
 
   EXPECT_THAT(generateIdentifierTrigrams("TUDecl"),
-  trigramsAre({"tud", "tde", "ude", "dec", "ecl"}));
+  trigramsAre({"t$$", "tud", "tde", "ude", "dec", "ecl", "tu$",
+   "td$", "$$$"}));
 
   EXPECT_THAT(generateIdentifierTrigrams("IsOK"),
-  trigramsAre({"iso", "iok", "sok"}));
-
-  EXPECT_THAT(generateIdentifierTrigrams("abc_defGhij__klm"),
-  trigramsAre({
-  "abc", "abd", "abg", "ade", "adg", "adk", "agh", "agk", "bcd",
-  "bcg", "bde", "bdg", "bdk", "bgh", "bgk", "cde", "cdg", "cdk",
-  "cgh", "cgk", "def", "deg", "dek", "dgh", "dgk", "dkl", "efg",
-  "efk", "egh", "egk", "ekl", "fgh", "fgk", "fkl", "ghi", "ghk",
-  "gkl", "hij", "hik", "hkl", "ijk", "ikl", "jkl", "klm",
-  }));
+  trigramsAre({"i$$", "iso", "iok", "sok", "is$", "io$", "$$$"}));
+
+  EXPECT_THAT(
+  generateIdentifierTrigrams("abc_defGhij__klm"),
+  trigramsAre({"a$$", "abc", "abd", "abg", "ade", "adg", "adk", "agh",
+   "agk", "bcd", "bcg", "bde", "bdg", "bdk", "bgh", "bgk",
+   "cde", "cdg", "cdk", "cgh", "cgk", "def", "deg", "dek",
+   "dgh", "dgk", "dkl", "efg", "efk", "egh", "egk", "ekl",
+   "fgh", "fgk", "fkl", "ghi", "ghk", "gkl", "hij", "hik",
+   "hkl", "ijk", "ikl", "jkl", "klm", "ab$", "ad$", "$$$"}));
 }
 
 TEST(DexIndexTrigrams, QueryTrigrams) {
-  EXPECT_THAT(generateQueryTrigrams("X86"), trigramsAre({"x86"}));
+  EXPECT_THAT(generateQueryTrigrams("c"), trigramsAre({"c$$"}));
+  EXPECT_THAT(generateQueryTrigrams("cl"), trigramsAre({"cl$"}));
+  EXPECT_THAT(generateQueryTrigrams("cla"), trigramsAre({"cla"}));
 
-  EXPECT_THAT(generateQueryTrigrams("nl"), trigramsAre({}));
+  EXPECT_THAT(generateQueryTrigrams("_"), trigramsAre({"_$$"}));
+  EXPECT_THAT(generateQueryTrigrams("__"), trigramsAre({"__$"}));
+  EXPECT_THAT(generateQueryTrigrams("___"), trigramsAre({"___"}));
+
+  EXPECT_THAT(generateQueryTrigrams("X86"), trigramsAre({"x86"}));
 
   EXPECT_THAT(generateQueryTrigrams("clangd"),
   trigramsAre({"cla", "lan", "ang", "ngd"}));
Index: clang-tools-extra/clangd/index/dex/Trigram.h
===
--- clang-tools-extra/clangd/index/dex/Trigram.h
+++ clang-tools-extra/clangd/index/dex/Trigram.h
@@ -36,14 +36,26 @@
 /// First, given Identifier (unqualified symbol name) is 

r339452 - Make changes to the check strings so that the test I modified in r339438

[Akira Hatanaka via cfe-commits]

Author: ahatanak
Date: Fri Aug 10 10:07:27 2018
New Revision: 339452

URL: http://llvm.org/viewvc/llvm-project?rev=339452=rev
Log:
Make changes to the check strings so that the test I modified in r339438
passes on 32-bit targets.

Modified:
cfe/trunk/test/CodeGenCXX/block-byref-cxx-objc.cpp

Modified: cfe/trunk/test/CodeGenCXX/block-byref-cxx-objc.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/block-byref-cxx-objc.cpp?rev=339452=339451=339452=diff
==
--- cfe/trunk/test/CodeGenCXX/block-byref-cxx-objc.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/block-byref-cxx-objc.cpp Fri Aug 10 10:07:27 2018
@@ -26,12 +26,12 @@ int testA() {
 // CHECK-LABEL: define internal void @__Block_byref_object_dispose_
 // CHECK: call {{.*}} @_ZN1AD1Ev
 
-// CHECK-LABEL: define linkonce_odr hidden void 
@__copy_helper_block_e8_32rc40rc(
+// CHECK-LABEL: define linkonce_odr hidden void 
@__copy_helper_block_e{{4|8}}_{{20|32}}rc{{24|40}}rc(
 // CHECK: call void @_Block_object_assign(
 // CHECK: invoke void @_Block_object_assign(
 // CHECK: call void @_Block_object_dispose({{.*}}) #[[NOUNWIND_ATTR:[0-9]+]]
 
-// CHECK-LABEL: define linkonce_odr hidden void 
@__destroy_helper_block_e8_32rd40rd(
+// CHECK-LABEL: define linkonce_odr hidden void 
@__destroy_helper_block_e{{4|8}}_{{20|32}}rd{{24|40}}rd(
 // CHECK: invoke void @_Block_object_dispose(
 // CHECK: call void @_Block_object_dispose(
 // CHECK: invoke void @_Block_object_dispose(
@@ -48,7 +48,7 @@ int testB() {
 // CHECK: call {{.*}} @_ZN1BD1Ev
 
 // CHECK-NOT: define{{.*}}@__copy_helper_block
-// CHECK: define linkonce_odr hidden void @__destroy_helper_block_e8_32r40r(
+// CHECK: define linkonce_odr hidden void 
@__destroy_helper_block_e{{4|8}}_{{20|32}}r{{24|40}}r(
 
 // CHECK: attributes #[[NOUNWIND_ATTR]] = {{{.*}}nounwind{{.*}}}
 


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

[PATCH] D50543: [libcxx] Mark charconv tests as failing for previous libcxx versions.

[Volodymyr Sapsai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL339451: [libcxx] Mark charconv tests as failing for previous 
libcxx versions. (authored by vsapsai, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D50543?vs=160041=160132#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D50543

Files:
  libcxx/trunk/test/std/utilities/charconv/charconv.from.chars/integral.pass.cpp
  libcxx/trunk/test/std/utilities/charconv/charconv.to.chars/integral.pass.cpp


Index: 
libcxx/trunk/test/std/utilities/charconv/charconv.from.chars/integral.pass.cpp
===
--- 
libcxx/trunk/test/std/utilities/charconv/charconv.from.chars/integral.pass.cpp
+++ 
libcxx/trunk/test/std/utilities/charconv/charconv.from.chars/integral.pass.cpp
@@ -8,6 +8,15 @@
 
//===--===//
 
 // UNSUPPORTED: c++98, c++03, c++11
+
+// XFAIL: with_system_cxx_lib=macosx10.13
+// XFAIL: with_system_cxx_lib=macosx10.12
+// XFAIL: with_system_cxx_lib=macosx10.11
+// XFAIL: with_system_cxx_lib=macosx10.10
+// XFAIL: with_system_cxx_lib=macosx10.9
+// XFAIL: with_system_cxx_lib=macosx10.8
+// XFAIL: with_system_cxx_lib=macosx10.7
+
 // 
 
 // from_chars_result from_chars(const char* first, const char* last,
Index: 
libcxx/trunk/test/std/utilities/charconv/charconv.to.chars/integral.pass.cpp
===
--- libcxx/trunk/test/std/utilities/charconv/charconv.to.chars/integral.pass.cpp
+++ libcxx/trunk/test/std/utilities/charconv/charconv.to.chars/integral.pass.cpp
@@ -8,6 +8,15 @@
 
//===--===//
 
 // UNSUPPORTED: c++98, c++03, c++11
+
+// XFAIL: with_system_cxx_lib=macosx10.13
+// XFAIL: with_system_cxx_lib=macosx10.12
+// XFAIL: with_system_cxx_lib=macosx10.11
+// XFAIL: with_system_cxx_lib=macosx10.10
+// XFAIL: with_system_cxx_lib=macosx10.9
+// XFAIL: with_system_cxx_lib=macosx10.8
+// XFAIL: with_system_cxx_lib=macosx10.7
+
 // 
 
 // to_chars_result to_chars(char* first, char* last, Integral value,


Index: libcxx/trunk/test/std/utilities/charconv/charconv.from.chars/integral.pass.cpp
===
--- libcxx/trunk/test/std/utilities/charconv/charconv.from.chars/integral.pass.cpp
+++ libcxx/trunk/test/std/utilities/charconv/charconv.from.chars/integral.pass.cpp
@@ -8,6 +8,15 @@
 //===--===//
 
 // UNSUPPORTED: c++98, c++03, c++11
+
+// XFAIL: with_system_cxx_lib=macosx10.13
+// XFAIL: with_system_cxx_lib=macosx10.12
+// XFAIL: with_system_cxx_lib=macosx10.11
+// XFAIL: with_system_cxx_lib=macosx10.10
+// XFAIL: with_system_cxx_lib=macosx10.9
+// XFAIL: with_system_cxx_lib=macosx10.8
+// XFAIL: with_system_cxx_lib=macosx10.7
+
 // 
 
 // from_chars_result from_chars(const char* first, const char* last,
Index: libcxx/trunk/test/std/utilities/charconv/charconv.to.chars/integral.pass.cpp
===
--- libcxx/trunk/test/std/utilities/charconv/charconv.to.chars/integral.pass.cpp
+++ libcxx/trunk/test/std/utilities/charconv/charconv.to.chars/integral.pass.cpp
@@ -8,6 +8,15 @@
 //===--===//
 
 // UNSUPPORTED: c++98, c++03, c++11
+
+// XFAIL: with_system_cxx_lib=macosx10.13
+// XFAIL: with_system_cxx_lib=macosx10.12
+// XFAIL: with_system_cxx_lib=macosx10.11
+// XFAIL: with_system_cxx_lib=macosx10.10
+// XFAIL: with_system_cxx_lib=macosx10.9
+// XFAIL: with_system_cxx_lib=macosx10.8
+// XFAIL: with_system_cxx_lib=macosx10.7
+
 // 
 
 // to_chars_result to_chars(char* first, char* last, Integral value,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r339451 - [libcxx] Mark charconv tests as failing for previous libcxx versions.

[Volodymyr Sapsai via cfe-commits]

Author: vsapsai
Date: Fri Aug 10 10:03:47 2018
New Revision: 339451

URL: http://llvm.org/viewvc/llvm-project?rev=339451=rev
Log:
[libcxx] Mark charconv tests as failing for previous libcxx versions.

 was added in r338479. Previous libcxx versions don't have
this functionality and corresponding tests should be failing.

Reviewers: mclow.lists, ldionne, EricWF

Reviewed By: ldionne

Subscribers: christof, dexonsmith, lichray, EricWF, cfe-commits

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

Modified:

libcxx/trunk/test/std/utilities/charconv/charconv.from.chars/integral.pass.cpp
libcxx/trunk/test/std/utilities/charconv/charconv.to.chars/integral.pass.cpp

Modified: 
libcxx/trunk/test/std/utilities/charconv/charconv.from.chars/integral.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/charconv/charconv.from.chars/integral.pass.cpp?rev=339451=339450=339451=diff
==
--- 
libcxx/trunk/test/std/utilities/charconv/charconv.from.chars/integral.pass.cpp 
(original)
+++ 
libcxx/trunk/test/std/utilities/charconv/charconv.from.chars/integral.pass.cpp 
Fri Aug 10 10:03:47 2018
@@ -8,6 +8,15 @@
 
//===--===//
 
 // UNSUPPORTED: c++98, c++03, c++11
+
+// XFAIL: with_system_cxx_lib=macosx10.13
+// XFAIL: with_system_cxx_lib=macosx10.12
+// XFAIL: with_system_cxx_lib=macosx10.11
+// XFAIL: with_system_cxx_lib=macosx10.10
+// XFAIL: with_system_cxx_lib=macosx10.9
+// XFAIL: with_system_cxx_lib=macosx10.8
+// XFAIL: with_system_cxx_lib=macosx10.7
+
 // 
 
 // from_chars_result from_chars(const char* first, const char* last,

Modified: 
libcxx/trunk/test/std/utilities/charconv/charconv.to.chars/integral.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/charconv/charconv.to.chars/integral.pass.cpp?rev=339451=339450=339451=diff
==
--- 
libcxx/trunk/test/std/utilities/charconv/charconv.to.chars/integral.pass.cpp 
(original)
+++ 
libcxx/trunk/test/std/utilities/charconv/charconv.to.chars/integral.pass.cpp 
Fri Aug 10 10:03:47 2018
@@ -8,6 +8,15 @@
 
//===--===//
 
 // UNSUPPORTED: c++98, c++03, c++11
+
+// XFAIL: with_system_cxx_lib=macosx10.13
+// XFAIL: with_system_cxx_lib=macosx10.12
+// XFAIL: with_system_cxx_lib=macosx10.11
+// XFAIL: with_system_cxx_lib=macosx10.10
+// XFAIL: with_system_cxx_lib=macosx10.9
+// XFAIL: with_system_cxx_lib=macosx10.8
+// XFAIL: with_system_cxx_lib=macosx10.7
+
 // 
 
 // to_chars_result to_chars(char* first, char* last, Integral value,


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

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Thank you for your first contribution to LLVM btw :)




Comment at: clang-tidy/abseil/NoInternalDepsCheck.cpp:24
+
+  TODO(hugoeg): refactor matcher to be configurable or just match on any 
internal access from outside the enclosing namespace.
+  

Missing //



Comment at: clang-tidy/abseil/NoInternalDepsCheck.cpp:35
+void NoInternalDepsCheck::check(const MatchFinder::MatchResult ) {
+  const auto *intr_dep =
+  Result.Nodes.getNodeAs("internal_dep");

Please follow the LLVM naming convention (s/initr_dep/InternalDependency/ or 
similar)

https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
for reference


https://reviews.llvm.org/D50542



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

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 160124.

https://reviews.llvm.org/D50542

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/NoInternalDepsCheck.cpp
  clang-tidy/abseil/NoInternalDepsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-no-internal-deps.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-no-internal-deps.cpp

Index: test/clang-tidy/abseil-no-internal-deps.cpp
===
--- test/clang-tidy/abseil-no-internal-deps.cpp
+++ test/clang-tidy/abseil-no-internal-deps.cpp
@@ -0,0 +1,62 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t
+
+
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+} // namepsace std
+
+namespace absl {
+std::string StringsFunction (std::string s1){
+  return s1;
+}
+class SomeContainer{};
+
+namespace strings_internal{
+  
+  void InternalFunction(){}
+
+  template 
+  P InternalTemplateFunction (P a) {}
+} // namespace strings_internal
+
+namespace container_internal{
+  struct InternalStruct{};
+
+} // namespace container_internal
+} // namespace absl
+
+void foo(){
+  absl::strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: depends upon internal implementation details, which violates the Abseil compatibilty guidelines. See https://abseil.io/about/compatibility
+
+  absl::strings_internal::InternalTemplateFunction ("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: depends upon internal implementation details, which violates the Abseil compatibilty guidelines. See https://abseil.io/about/compatibility
+}
+
+class foo2{
+  friend struct absl::container_internal::InternalStruct;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: depends upon internal implementation details, which violates the Abseil compatibilty guidelines. See https://abseil.io/about/compatibility
+};
+
+namespace absl{
+  void foo3(){
+strings_internal::InternalFunction();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: depends upon internal implementation details, which violates the Abseil compatibilty guidelines. See https://abseil.io/about/compatibility
+  }
+} // namespace absl
+
+
+
+// should not trigger warnings
+void foo4(){
+  std::string str = absl::StringsFunction("a");
+  absl::SomeContainer b;
+}
+
+namespace absl {
+  SomeContainer b;
+  std::string str = absl::StringsFunction("a");
+} // namespace absl
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -5,6 +5,7 @@
 
 .. toctree::
abseil-string-find-startswith
+   abseil-no-internal-deps
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-no-internal-deps.rst
===
--- docs/clang-tidy/checks/abseil-no-internal-deps.rst
+++ docs/clang-tidy/checks/abseil-no-internal-deps.rst
@@ -0,0 +1,16 @@
+subl.. title:: clang-tidy - abseil-no-internal-deps
+
+abseil-no-internal-deps
+===
+
+Warns users if they attempt to depend on internal details. If something is in a namespace or filename/path that includes the word “internal”, users are not allowed to depend upon it beaucse it’s an implementation detail. They cannot friend it, include it, you mention it or refer to it in any way. Doing so violtaes Abseil's compatibility guidelines and may result in breakage.
+
+The folowing cases will result in warnings:
+
+.. code-block:: c++
+
+absl::strings_internal::foo();
+class foo{
+  friend struct absl::container_internal::faa;
+};
+absl::memory_internal::MakeUniqueResult();
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -52,7 +52,10 @@
 Improvements to clang-rename
 
 
-The improvements are...
+- New :doc:`abseil-no-internal-deps
+  ` check.
+  
+  Warns Abseil users if they attempt to depend on internal details.
 
 Improvements to clang-tidy
 --
Index: clang-tidy/abseil/NoInternalDepsCheck.h
===
--- clang-tidy/abseil/NoInternalDepsCheck.h
+++ clang-tidy/abseil/NoInternalDepsCheck.h
@@ -0,0 +1,37 @@
+//===--- NoInternalDepsCheck.h - clang-tidy--*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_NOINTERNALDEPSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_NOINTERNALDEPSCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/abseil/NoInternalDepsCheck.cpp:24
+
+  Finder->addMatcher(
+  nestedNameSpecifierLoc(loc(specifiesNamespace(namespaceDecl(

hugoeg wrote:
> JonasToth wrote:
> > Actually that one is generally useful. Accessing the `foo::internal` from 
> > outside of `foo` is always a problem. Maybe this matcher can become 
> > configurable or just match on any `internal` access from outside the 
> > enclosing namespace.
> That's a good idea. While we agree, right now our efforts are focused on 
> releasing abseil specific functions. Perhaps we can refactor this check at a 
> later time. 
Ok. Could you please add a `TODO` or `FIXME` that it is not forgotten?


https://reviews.llvm.org/D50542



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

[PATCH] D50389: [clang-tidy] new check for Abseil

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/abseil/DurationDivisionCheck.cpp:50
+ *result.SourceManager, result.Context->getLangOpts()),
+ ")");
+}

deannagarcia wrote:
> JonasToth wrote:
> > This line looks odd, does it come from clang-format?
> I have run clang-format on this file and it's still formatted like this.
Leave as is :)


https://reviews.llvm.org/D50389



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

[PATCH] D50507: [CodeGen][ARM] Coerce FP16 vectors to integer vectors when needed

[Mikhail Maltsev via Phabricator via cfe-commits]

miyuki updated this revision to Diff 160121.
miyuki edited the summary of this revision.
miyuki added a comment.

Fix handling of homogeneous aggregates of FP16 vectors


https://reviews.llvm.org/D50507

Files:
  lib/CodeGen/TargetInfo.cpp
  test/CodeGen/arm-vfp16-arguments.c
  test/CodeGen/arm_neon_intrinsics.c

Index: test/CodeGen/arm_neon_intrinsics.c
===
--- test/CodeGen/arm_neon_intrinsics.c
+++ test/CodeGen/arm_neon_intrinsics.c
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -triple thumbv7s-apple-darwin -target-abi apcs-gnu\
-// RUN:  -target-cpu swift -fallow-half-arguments-and-returns -ffreestanding \
+// RUN:  -target-cpu swift -fallow-half-arguments-and-returns \
+// RUN:  -target-feature +fullfp16 -ffreestanding \
 // RUN:  -disable-O0-optnone -emit-llvm -o - %s \
 // RUN:  | opt -S -mem2reg | FileCheck %s
 
@@ -3896,9 +3897,8 @@
 
 // CHECK-LABEL: @test_vld1q_f16(
 // CHECK:   [[TMP0:%.*]] = bitcast half* %a to i8*
-// CHECK:   [[VLD1:%.*]] = call <8 x i16> @llvm.arm.neon.vld1.v8i16.p0i8(i8* [[TMP0]], i32 2)
-// CHECK:   [[TMP1:%.*]] = bitcast <8 x i16> [[VLD1]] to <8 x half>
-// CHECK:   ret <8 x half> [[TMP1]]
+// CHECK:   [[VLD1:%.*]] = call <8 x half> @llvm.arm.neon.vld1.v8f16.p0i8(i8* [[TMP0]], i32 2)
+// CHECK:   ret <8 x half> [[VLD1]]
 float16x8_t test_vld1q_f16(float16_t const * a) {
   return vld1q_f16(a);
 }
@@ -3990,9 +3990,8 @@
 
 // CHECK-LABEL: @test_vld1_f16(
 // CHECK:   [[TMP0:%.*]] = bitcast half* %a to i8*
-// CHECK:   [[VLD1:%.*]] = call <4 x i16> @llvm.arm.neon.vld1.v4i16.p0i8(i8* [[TMP0]], i32 2)
-// CHECK:   [[TMP1:%.*]] = bitcast <4 x i16> [[VLD1]] to <4 x half>
-// CHECK:   ret <4 x half> [[TMP1]]
+// CHECK:   [[VLD1:%.*]] = call <4 x half> @llvm.arm.neon.vld1.v4f16.p0i8(i8* [[TMP0]], i32 2)
+// CHECK:   ret <4 x half> [[VLD1]]
 float16x4_t test_vld1_f16(float16_t const * a) {
   return vld1_f16(a);
 }
@@ -4106,12 +4105,11 @@
 
 // CHECK-LABEL: @test_vld1q_dup_f16(
 // CHECK:   [[TMP0:%.*]] = bitcast half* %a to i8*
-// CHECK:   [[TMP1:%.*]] = bitcast i8* [[TMP0]] to i16*
-// CHECK:   [[TMP2:%.*]] = load i16, i16* [[TMP1]], align 2
-// CHECK:   [[TMP3:%.*]] = insertelement <8 x i16> undef, i16 [[TMP2]], i32 0
-// CHECK:   [[LANE:%.*]] = shufflevector <8 x i16> [[TMP3]], <8 x i16> [[TMP3]], <8 x i32> zeroinitializer
-// CHECK:   [[TMP4:%.*]] = bitcast <8 x i16> [[LANE]] to <8 x half>
-// CHECK:   ret <8 x half> [[TMP4]]
+// CHECK:   [[TMP1:%.*]] = bitcast i8* [[TMP0]] to half*
+// CHECK:   [[TMP2:%.*]] = load half, half* [[TMP1]], align 2
+// CHECK:   [[TMP3:%.*]] = insertelement <8 x half> undef, half [[TMP2]], i32 0
+// CHECK:   [[LANE:%.*]] = shufflevector <8 x half> [[TMP3]], <8 x half> [[TMP3]], <8 x i32> zeroinitializer
+// CHECK:   ret <8 x half> [[LANE]]
 float16x8_t test_vld1q_dup_f16(float16_t const * a) {
   return vld1q_dup_f16(a);
 }
@@ -4233,12 +4231,11 @@
 
 // CHECK-LABEL: @test_vld1_dup_f16(
 // CHECK:   [[TMP0:%.*]] = bitcast half* %a to i8*
-// CHECK:   [[TMP1:%.*]] = bitcast i8* [[TMP0]] to i16*
-// CHECK:   [[TMP2:%.*]] = load i16, i16* [[TMP1]], align 2
-// CHECK:   [[TMP3:%.*]] = insertelement <4 x i16> undef, i16 [[TMP2]], i32 0
-// CHECK:   [[LANE:%.*]] = shufflevector <4 x i16> [[TMP3]], <4 x i16> [[TMP3]], <4 x i32> zeroinitializer
-// CHECK:   [[TMP4:%.*]] = bitcast <4 x i16> [[LANE]] to <4 x half>
-// CHECK:   ret <4 x half> [[TMP4]]
+// CHECK:   [[TMP1:%.*]] = bitcast i8* [[TMP0]] to half*
+// CHECK:   [[TMP2:%.*]] = load half, half* [[TMP1]], align 2
+// CHECK:   [[TMP3:%.*]] = insertelement <4 x half> undef, half [[TMP2]], i32 0
+// CHECK:   [[LANE:%.*]] = shufflevector <4 x half> [[TMP3]], <4 x half> [[TMP3]], <4 x i32> zeroinitializer
+// CHECK:   ret <4 x half> [[LANE]]
 float16x4_t test_vld1_dup_f16(float16_t const * a) {
   return vld1_dup_f16(a);
 }
@@ -4365,12 +4362,11 @@
 // CHECK-LABEL: @test_vld1q_lane_f16(
 // CHECK:   [[TMP0:%.*]] = bitcast half* %a to i8*
 // CHECK:   [[TMP1:%.*]] = bitcast <8 x half> %b to <16 x i8>
-// CHECK:   [[TMP2:%.*]] = bitcast <16 x i8> [[TMP1]] to <8 x i16>
-// CHECK:   [[TMP3:%.*]] = bitcast i8* [[TMP0]] to i16*
-// CHECK:   [[TMP4:%.*]] = load i16, i16* [[TMP3]], align 2
-// CHECK:   [[VLD1_LANE:%.*]] = insertelement <8 x i16> [[TMP2]], i16 [[TMP4]], i32 7
-// CHECK:   [[TMP5:%.*]] = bitcast <8 x i16> [[VLD1_LANE]] to <8 x half>
-// CHECK:   ret <8 x half> [[TMP5]]
+// CHECK:   [[TMP2:%.*]] = bitcast <16 x i8> [[TMP1]] to <8 x half>
+// CHECK:   [[TMP3:%.*]] = bitcast i8* [[TMP0]] to half*
+// CHECK:   [[TMP4:%.*]] = load half, half* [[TMP3]], align 2
+// CHECK:   [[VLD1_LANE:%.*]] = insertelement <8 x half> [[TMP2]], half [[TMP4]], i32 7
+// CHECK:   ret <8 x half> [[VLD1_LANE]]
 float16x8_t test_vld1q_lane_f16(float16_t const * a, float16x8_t b) {
   return vld1q_lane_f16(a, b, 7);
 }
@@ -4498,12 +4494,11 @@
 // CHECK-LABEL: @test_vld1_lane_f16(
 // CHECK:   [[TMP0:%.*]] = bitcast half* %a to i8*
 // CHECK:   [[TMP1:%.*]] = bitcast <4 x 

[PATCH] D50413: [libunwind][include] Add some missing definitions to .

[Charles Davis via Phabricator via cfe-commits]

cdavis5x added a comment.

In https://reviews.llvm.org/D50413#1195101, @mstorsjo wrote:

> @cdavis5x I presume the fact that this one turned out tricky is blocking 
> submitting the SEH unwinding patch.
>
> Would it be worth to rework that patch to just use the basic types just like 
> libunwind does today, e.g. having `_Unwind_GetRegionStart` return plain 
> `uintptr_t` instead of `_Unwind_Ptr`, which also would be consistent with the 
> existing entry points in Unwind-EHABI.cpp, Unwind-sjlj.c and UnwindLevel1.c, 
> in order to be able to submit it before this one gets sorted out?


Sounds reasonable.


Repository:
  rUNW libunwind

https://reviews.llvm.org/D50413



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

[PATCH] D50564: Add support for SEH unwinding on Windows.

[Charles Davis via Phabricator via cfe-commits]

cdavis5x created this revision.
cdavis5x added reviewers: mstorsjo, rnk, compnerd, smeenai.
Herald added subscribers: cfe-commits, chrib, christof, kristof.beyls, mgorny.
Herald added a reviewer: javed.absar.

I've tested this implementation on x86-64 to ensure that it works. All
`libc++abi` tests pass, as do all `libc++` exception-related tests. ARM
still remains to be implemented (@compnerd?).

Special thanks to KJK::Hyperion for his excellent series of articles on
how EH works on x86-64 Windows. (Seriously, check it out. It's awesome.)

I'm actually not sure if this should go in as is. I particularly don't
like that I duplicated the UnwindCursor class for this special case.


Repository:
  rUNW libunwind

https://reviews.llvm.org/D50564

Files:
  include/__libunwind_config.h
  src/AddressSpace.hpp
  src/CMakeLists.txt
  src/Unwind-seh.cpp
  src/UnwindCursor.hpp
  src/UnwindLevel1-gcc-ext.c
  src/UnwindLevel1.c
  src/config.h
  src/libunwind_ext.h

Index: src/libunwind_ext.h
===
--- src/libunwind_ext.h
+++ src/libunwind_ext.h
@@ -17,6 +17,11 @@
 #include 
 #include 
 
+#if defined(_LIBUNWIND_SUPPORT_SEH_UNWIND)
+  #include 
+  #include 
+#endif
+
 #define UNW_STEP_SUCCESS 1
 #define UNW_STEP_END 0
 
@@ -33,6 +38,43 @@
 extern void _unw_add_dynamic_fde(unw_word_t fde);
 extern void _unw_remove_dynamic_fde(unw_word_t fde);
 
+#if defined(_LIBUNWIND_SUPPORT_SEH_UNWIND)
+  #if defined(__x86_64__) && !defined(__MINGW64__)
+typedef struct _DISPATCHER_CONTEXT {
+  ULONG64 ControlPc;
+  ULONG64 ImageBase;
+  PRUNTIME_FUNCTION FunctionEntry;
+  ULONG64 EstablisherFrame;
+  ULONG64 TargetIp;
+  PCONTEXT ContextRecord;
+  PEXCEPTION_ROUTINE LanguageHandler;
+  PVOID HandlerData;
+  PUNWIND_HISTORY_TABLE HistoryTable;
+  ULONG ScopeIndex;
+  ULONG Fill0;
+} DISPATCHER_CONTEXT;
+  #elif defined(__arm__)
+typedef struct _DISPATCHER_CONTEXT {
+  ULONG ControlPc;
+  ULONG ImageBase;
+  PRUNTIME_FUNCTION FunctionEntry;
+  ULONG EstablisherFrame;
+  ULONG TargetIp;
+  PCONTEXT ContextRecord;
+  PEXCEPTION_ROUTINE LanguageHandler;
+  PVOID HandlerData;
+  PUNWIND_HISTORY_TABLE HistoryTable;
+  ULONG ScopeIndex;
+  ULONG ControlPcIsUnwound;
+  PKNONVOLATILE_CONTEXT_POINTERS NonVolatileRegisters;
+  ULONG VirtualVfpHead;
+} DISPATCHER_CONTEXT;
+  #endif
+extern int _unw_init_seh(unw_cursor_t *cursor, CONTEXT *ctx);
+extern DISPATCHER_CONTEXT *_unw_seh_get_disp_ctx(unw_cursor_t *cursor);
+extern void _unw_seh_set_disp_ctx(unw_cursor_t *cursor, DISPATCHER_CONTEXT *disp);
+#endif
+
 #if defined(_LIBUNWIND_ARM_EHABI)
 extern const uint32_t* decode_eht_entry(const uint32_t*, size_t*, size_t*);
 extern _Unwind_Reason_Code _Unwind_VRS_Interpret(_Unwind_Context *context,
Index: src/config.h
===
--- src/config.h
+++ src/config.h
@@ -38,7 +38,11 @@
 #define _LIBUNWIND_SUPPORT_DWARF_UNWIND   1
   #endif
 #elif defined(_WIN32)
-  #define _LIBUNWIND_SUPPORT_DWARF_UNWIND 1
+  #ifdef __SEH__
+#define _LIBUNWIND_SUPPORT_SEH_UNWIND 1
+  #else
+#define _LIBUNWIND_SUPPORT_DWARF_UNWIND 1
+  #endif
 #else
   #if defined(__ARM_DWARF_EH__) || !defined(__arm__)
 #define _LIBUNWIND_SUPPORT_DWARF_UNWIND 1
Index: src/UnwindLevel1.c
===
--- src/UnwindLevel1.c
+++ src/UnwindLevel1.c
@@ -32,6 +32,8 @@
 
 #if !defined(_LIBUNWIND_ARM_EHABI) && !defined(__USING_SJLJ_EXCEPTIONS__)
 
+#if !_LIBUNWIND_SUPPORT_SEH_UNWIND
+
 static _Unwind_Reason_Code
 unwind_phase1(unw_context_t *uc, unw_cursor_t *cursor, _Unwind_Exception *exception_object) {
   unw_init_local(cursor, uc);
@@ -449,6 +451,7 @@
   return result;
 }
 
+#endif // !_LIBUNWIND_SUPPORT_SEH_UNWIND
 
 /// Called by personality handler during phase 2 if a foreign exception
 // is caught.
Index: src/UnwindLevel1-gcc-ext.c
===
--- src/UnwindLevel1-gcc-ext.c
+++ src/UnwindLevel1-gcc-ext.c
@@ -25,6 +25,10 @@
 
 #if defined(_LIBUNWIND_BUILD_ZERO_COST_APIS)
 
+#if defined(_LIBUNWIND_SUPPORT_SEH_UNWIND)
+#define private_1 private_[0]
+#endif
+
 ///  Called by __cxa_rethrow().
 _LIBUNWIND_EXPORT _Unwind_Reason_Code
 _Unwind_Resume_or_Rethrow(_Unwind_Exception *exception_object) {
Index: src/UnwindCursor.hpp
===
--- src/UnwindCursor.hpp
+++ src/UnwindCursor.hpp
@@ -18,18 +18,40 @@
 #include 
 #include 
 
+#ifdef _WIN32
+  #include 
+#endif
 #ifdef __APPLE__
   #include 
 #endif
 
+#if defined(_LIBUNWIND_SUPPORT_SEH_UNWIND)
+#  ifdef _LIBUNWIND_TARGET_X86_64
+struct UNWIND_INFO {
+  BYTE Version : 3;
+  BYTE Flags : 5;
+  BYTE SizeOfProlog;
+  BYTE CountOfCodes;
+  BYTE FrameRegister : 4;
+  BYTE FrameOffset : 4;
+  WORD UnwindCodes[2];
+};
+#  endif
+
+extern "C" _Unwind_Reason_Code __libunwind_seh_personality(
+int, _Unwind_Action, _Unwind_Exception_Class, 

[PATCH] D50563: Fixed frontend clang tests in windows read-only container

[Justice Adams via Phabricator via cfe-commits]

justice_adams created this revision.
justice_adams added a project: clang.

When mounting LLVM source into a windows container in read-only mode, certain 
tests fail. Ideally, we want all these tests to pass so that developers can 
mount the same source folder into multiple (windows) containers simultaneously, 
allowing them to build/test the same source code using various different 
configurations simultaneously.

**Fix**: I've found that when attempting to open a file for writing on windows, 
if you don't have the correct permissions (trying to open a file for writing in 
a read-only folder), you get Access is denied 
.
 In llvm, we map this error message to a linux based error, see: 
https://github.com/llvm-mirror/llvm/blob/master/lib/Support/ErrorHandling.cpp

This is why we see "Permission denied" in our output as opposed to the expected 
"No such file or directory", thus causing the tests to fail.

I've changed the test locally to instead point to the root drive so that they 
can successfully bypass the Access is denied error when o is mounted in as a 
read-only directory. This way, the test operate exactly the same, but we can 
get around the windows-complications of what error to expect in a read-only 
directory.


https://reviews.llvm.org/D50563

Files:
  test/Frontend/output-failures.c
  test/Frontend/stats-file.c
  test/lit.cfg.py


Index: test/lit.cfg.py
===
--- test/lit.cfg.py
+++ test/lit.cfg.py
@@ -49,6 +49,7 @@
 
 config.substitutions.append(('%PATH%', config.environment['PATH']))
 
+config.substitutions.append(('%ROOT%', os.path.abspath(os.sep)))
 
 # For each occurrence of a clang tool name, replace it with the full path to
 # the build directory holding that tool.  We explicitly specify the directories
Index: test/Frontend/stats-file.c
===
--- test/Frontend/stats-file.c
+++ test/Frontend/stats-file.c
@@ -4,5 +4,5 @@
 //  ... here come some json values ...
 // CHECK: }
 
-// RUN: %clang_cc1 -emit-llvm -o %t -stats-file=%S/doesnotexist/bla %s 2>&1 | 
FileCheck -check-prefix=OUTPUTFAIL %s
+// RUN: %clang_cc1 -emit-llvm -o %t -stats-file=%ROOT%/doesnotexist/bla %s 
2>&1 | FileCheck -check-prefix=OUTPUTFAIL %s
 // OUTPUTFAIL: warning: unable to open statistics output file 
'{{.*}}doesnotexist{{.}}bla': '{{[Nn]}}o such file or directory'
Index: test/Frontend/output-failures.c
===
--- test/Frontend/output-failures.c
+++ test/Frontend/output-failures.c
@@ -1,4 +1,3 @@
-// RUN: not %clang_cc1 -emit-llvm -o %S/doesnotexist/somename %s 2> %t
-// RUN: FileCheck -check-prefix=OUTPUTFAIL -input-file=%t %s
+// RUN: not %clang_cc1 -emit-llvm -o %ROOT%/doesnotexist/somename %s 2>&1 | 
FileCheck -check-prefix=OUTPUTFAIL %s
 
-// OUTPUTFAIL: error: unable to open output file 
'{{.*}}{{[/\\]}}test{{[/\\]}}Frontend{{[/\\]}}doesnotexist{{[/\\]}}somename': 
'{{[nN]}}o such file or directory'
+// OUTPUTFAIL: error: unable to open output file 
'{{.*}}doesnotexist{{.}}somename': '{{[nN]}}o such file or directory'
\ No newline at end of file


Index: test/lit.cfg.py
===
--- test/lit.cfg.py
+++ test/lit.cfg.py
@@ -49,6 +49,7 @@
 
 config.substitutions.append(('%PATH%', config.environment['PATH']))
 
+config.substitutions.append(('%ROOT%', os.path.abspath(os.sep)))
 
 # For each occurrence of a clang tool name, replace it with the full path to
 # the build directory holding that tool.  We explicitly specify the directories
Index: test/Frontend/stats-file.c
===
--- test/Frontend/stats-file.c
+++ test/Frontend/stats-file.c
@@ -4,5 +4,5 @@
 //  ... here come some json values ...
 // CHECK: }
 
-// RUN: %clang_cc1 -emit-llvm -o %t -stats-file=%S/doesnotexist/bla %s 2>&1 | FileCheck -check-prefix=OUTPUTFAIL %s
+// RUN: %clang_cc1 -emit-llvm -o %t -stats-file=%ROOT%/doesnotexist/bla %s 2>&1 | FileCheck -check-prefix=OUTPUTFAIL %s
 // OUTPUTFAIL: warning: unable to open statistics output file '{{.*}}doesnotexist{{.}}bla': '{{[Nn]}}o such file or directory'
Index: test/Frontend/output-failures.c
===
--- test/Frontend/output-failures.c
+++ test/Frontend/output-failures.c
@@ -1,4 +1,3 @@
-// RUN: not %clang_cc1 -emit-llvm -o %S/doesnotexist/somename %s 2> %t
-// RUN: FileCheck -check-prefix=OUTPUTFAIL -input-file=%t %s
+// RUN: not %clang_cc1 -emit-llvm -o %ROOT%/doesnotexist/somename %s 2>&1 | FileCheck -check-prefix=OUTPUTFAIL %s
 
-// OUTPUTFAIL: error: unable to open output file '{{.*}}{{[/\\]}}test{{[/\\]}}Frontend{{[/\\]}}doesnotexist{{[/\\]}}somename': '{{[nN]}}o such file or directory'
+// OUTPUTFAIL: error: unable to 

[PATCH] D50389: [clang-tidy] new check for Abseil

[Deanna Garcia via Phabricator via cfe-commits]

deannagarcia added inline comments.



Comment at: clang-tidy/abseil/DurationDivisionCheck.cpp:50
+ *result.SourceManager, result.Context->getLangOpts()),
+ ")");
+}

JonasToth wrote:
> This line looks odd, does it come from clang-format?
I have run clang-format on this file and it's still formatted like this.


https://reviews.llvm.org/D50389



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

[PATCH] D50389: [clang-tidy] new check for Abseil

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

LGTM with only the formatting question.

But don't commit before any of the reviewers accepts (@alexfh @aaron.ballman 
usually have the last word)




Comment at: clang-tidy/abseil/DurationDivisionCheck.cpp:50
+ *result.SourceManager, result.Context->getLangOpts()),
+ ")");
+}

This line looks odd, does it come from clang-format?


https://reviews.llvm.org/D50389



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

[PATCH] D48687: [clangd] Avoid duplicates in findDefinitions response

[Simon Marchi via Phabricator via cfe-commits]

simark marked 8 inline comments as done.
simark added inline comments.



Comment at: clangd/SourceCode.h:69
 
+llvm::Optional getRealPath(const FileEntry *F,
+const SourceManager );

ilya-biryukov wrote:
> This function looks like a good default choice for normalizing paths before 
> putting them into LSP structs, ClangdServer responses, etc.
> I suggest we add a small comment here with a guideline for everyone to 
> attempt using it whenever possible. WDYT?
What about this:

```
/// Get the real/canonical path of \p F.  This means:
///
///   - Absolute path
///   - Symlinks resolved
///   - No "." or ".." component
///   - No duplicate or trailing directory separator
///
/// This function should be used when sending paths to clients, so that paths
/// are normalized as much as possible.
```


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48687



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

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

[Justin Hibbits via Phabricator via cfe-commits]

jhibbits added a comment.

Hi Vit,

Thanks for the feedback.  I can add the -mspe=yes/no, that shouldn't be hard.  
I didn't include it because it's been deprecated by GCC already as well.  I can 
add the -mcpu=8548 option as well.  I use -mcpu=8540 on FreeBSD for the 
powerpcspe target anyway (GCC based).

Regarding the stack overwriting, that's very peculiar, because I explicitly 
mark the immediate as needing to be an 8-bit multiple-of-8 value, so the 
codegen logic should take that into account.  I'm surprised it's not.  I've 
reproduced it as well myself, so I can take a closer look.


Repository:
  rC Clang

https://reviews.llvm.org/D49754



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

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg added inline comments.



Comment at: clang-tidy/abseil/NoInternalDepsCheck.cpp:24
+
+  Finder->addMatcher(
+  nestedNameSpecifierLoc(loc(specifiesNamespace(namespaceDecl(

JonasToth wrote:
> Actually that one is generally useful. Accessing the `foo::internal` from 
> outside of `foo` is always a problem. Maybe this matcher can become 
> configurable or just match on any `internal` access from outside the 
> enclosing namespace.
That's a good idea. While we agree, right now our efforts are focused on 
releasing abseil specific functions. Perhaps we can refactor this check at a 
later time. 


https://reviews.llvm.org/D50542



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

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 160112.
hugoeg marked 10 inline comments as done.
hugoeg added a comment.

Applied corrections from first round comments


https://reviews.llvm.org/D50542

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/NoInternalDepsCheck.cpp
  clang-tidy/abseil/NoInternalDepsCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-no-internal-deps.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-no-internal-deps.cpp

Index: test/clang-tidy/abseil-no-internal-deps.cpp
===
--- test/clang-tidy/abseil-no-internal-deps.cpp
+++ test/clang-tidy/abseil-no-internal-deps.cpp
@@ -0,0 +1,62 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-deps %t
+
+
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+} // namepsace std
+
+namespace absl {
+std::string StringsFunction (std::string s1){
+  return s1;
+}
+class SomeContainer{};
+
+namespace strings_internal{
+  
+  void InternalFunction(){}
+
+  template 
+  P InternalTemplateFunction (P a) {}
+} // namespace strings_internal
+
+namespace container_internal{
+  struct InternalStruct{};
+
+} // namespace container_internal
+} // namespace absl
+
+void foo(){
+  absl::strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: depends upon internal implementation details, which violates the Abseil compatibilty guidelines. See https://abseil.io/about/compatibility
+
+  absl::strings_internal::InternalTemplateFunction ("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: depends upon internal implementation details, which violates the Abseil compatibilty guidelines. See https://abseil.io/about/compatibility
+}
+
+class foo2{
+  friend struct absl::container_internal::InternalStruct;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: depends upon internal implementation details, which violates the Abseil compatibilty guidelines. See https://abseil.io/about/compatibility
+};
+
+namespace absl{
+  void foo3(){
+strings_internal::InternalFunction();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: depends upon internal implementation details, which violates the Abseil compatibilty guidelines. See https://abseil.io/about/compatibility
+  }
+} // namespace absl
+
+
+
+// should not trigger warnings
+void foo4(){
+  std::string str = absl::StringsFunction("a");
+  absl::SomeContainer b;
+}
+
+namespace absl {
+  SomeContainer b;
+  std::string str = absl::StringsFunction("a");
+} // namespace absl
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -5,6 +5,7 @@
 
 .. toctree::
abseil-string-find-startswith
+   abseil-no-internal-deps
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-no-internal-deps.rst
===
--- docs/clang-tidy/checks/abseil-no-internal-deps.rst
+++ docs/clang-tidy/checks/abseil-no-internal-deps.rst
@@ -0,0 +1,16 @@
+subl.. title:: clang-tidy - abseil-no-internal-deps
+
+abseil-no-internal-deps
+===
+
+Warns users if they attempt to depend on internal details. If something is in a namespace or filename/path that includes the word “internal”, users are not allowed to depend upon it beaucse it’s an implementation detail. They cannot friend it, include it, you mention it or refer to it in any way. Doing so violtaes Abseil's compatibility guidelines and may result in breakage.
+
+The folowing cases will result in warnings:
+
+.. code-block:: c++
+
+absl::strings_internal::foo();
+class foo{
+  friend struct absl::container_internal::faa;
+};
+absl::memory_internal::MakeUniqueResult();
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -52,7 +52,10 @@
 Improvements to clang-rename
 
 
-The improvements are...
+- New :doc:`abseil-no-internal-deps
+  ` check.
+  
+  Warns Abseil users if they attempt to depend on internal details.
 
 Improvements to clang-tidy
 --
Index: clang-tidy/abseil/NoInternalDepsCheck.h
===
--- clang-tidy/abseil/NoInternalDepsCheck.h
+++ clang-tidy/abseil/NoInternalDepsCheck.h
@@ -0,0 +1,37 @@
+//===--- NoInternalDepsCheck.h - clang-tidy--*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_NOINTERNALDEPSCHECK_H
+#define 

[PATCH] D50152: [CodeGen] Merge equivalent block copy/helper functions

[Akira Hatanaka via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC339438: [CodeGen] Merge equivalent block copy/helper 
functions. (authored by ahatanak, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D50152?vs=160053=160110#toc

Repository:
  rC Clang

https://reviews.llvm.org/D50152

Files:
  include/clang/AST/ASTContext.h
  lib/AST/ASTContext.cpp
  lib/CodeGen/CGBlocks.cpp
  lib/CodeGen/CGBlocks.h
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CGNonTrivialStruct.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Sema/SemaDecl.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriterDecl.cpp
  test/CodeGen/blocks-1.c
  test/CodeGen/blocks.c
  test/CodeGen/sanitize-thread-no-checking-at-run-time.m
  test/CodeGenCXX/block-byref-cxx-objc.cpp
  test/CodeGenCXX/blocks.cpp
  test/CodeGenCXX/cxx-block-objects.cpp
  test/CodeGenObjC/arc-blocks.m
  test/CodeGenObjC/debug-info-block-helper.m
  test/CodeGenObjC/debug-info-blocks.m
  test/CodeGenObjC/mrc-weak.m
  test/CodeGenObjC/strong-in-c-struct.m
  test/CodeGenObjCXX/arc-blocks.mm
  test/CodeGenObjCXX/lambda-to-block.mm
  test/CodeGenObjCXX/mrc-weak.mm
  test/PCH/block-helpers.cpp
  test/PCH/block-helpers.h

Index: test/PCH/block-helpers.h
===
--- test/PCH/block-helpers.h
+++ test/PCH/block-helpers.h
@@ -0,0 +1,12 @@
+struct S0 {
+  S0();
+  S0(const S0 &) noexcept(false);
+  int a;
+};
+
+struct S {
+  void m() {
+__block S0 x, y;
+^{ (void)x; (void)y; };
+  }
+};
Index: test/PCH/block-helpers.cpp
===
--- test/PCH/block-helpers.cpp
+++ test/PCH/block-helpers.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -x c++-header -triple x86_64-apple-darwin11 -emit-pch -fblocks -fexceptions -o %t %S/block-helpers.h
+// RUN: %clang_cc1 -triple x86_64-apple-darwin11 -include-pch %t -emit-llvm -fblocks -fexceptions -o - %s | FileCheck %s
+
+// The second call to block_object_assign should be an invoke.
+
+// CHECK-LABEL: define linkonce_odr hidden void @__copy_helper_block_e8_32rc40rc(
+// CHECK: call void @_Block_object_assign(
+// CHECK: invoke void @_Block_object_assign(
+// CHECK: call void @_Block_object_dispose(
+
+// CHECK-LABEL: define linkonce_odr hidden void @__destroy_helper_block_e8_32r40r(
+void test() {
+  S s;
+  s.m();
+}
Index: test/CodeGen/sanitize-thread-no-checking-at-run-time.m
===
--- test/CodeGen/sanitize-thread-no-checking-at-run-time.m
+++ test/CodeGen/sanitize-thread-no-checking-at-run-time.m
@@ -35,7 +35,7 @@
 void test2(id x) {
   extern void test2_helper(id (^)(void));
   test2_helper(^{ return x; });
-// TSAN: define internal void @__destroy_helper_block_(i8*) [[ATTR:#[0-9]+]]
+// TSAN: define linkonce_odr hidden void @__destroy_helper_block_8_32o(i8*) unnamed_addr [[ATTR:#[0-9]+]]
 }
 
 // TSAN: attributes [[ATTR]] = { noinline nounwind {{.*}} "sanitize_thread_no_checking_at_run_time" {{.*}} }
Index: test/CodeGen/blocks.c
===
--- test/CodeGen/blocks.c
+++ test/CodeGen/blocks.c
@@ -1,4 +1,9 @@
 // RUN: %clang_cc1 -triple i386-unknown-unknown %s -emit-llvm -o - -fblocks | FileCheck %s
+
+// CHECK: %[[STRUCT_BLOCK_DESCRIPTOR:.*]] = type { i32, i32 }
+
+// CHECK: @[[BLOCK_DESCRIPTOR_TMP21:.*]] = internal constant { i32, i32, void (i8*, i8*)*, void (i8*)*, i8*, i8* } { i32 0, i32 24, void (i8*, i8*)* @__copy_helper_block_4_20r, void (i8*)* @__destroy_helper_block_4_20r, i8* getelementptr inbounds ([6 x i8], [6 x i8]* @.str, i32 0, i32 0), i8* null }, align 4
+
 void (^f)(void) = ^{};
 
 // rdar://6768379
@@ -27,6 +32,32 @@
   ^ { i = 1; }();
 };
 
+// CHECK-LABEL: define linkonce_odr hidden void @__copy_helper_block_4_20r(i8*, i8*) unnamed_addr
+// CHECK: %[[_ADDR:.*]] = alloca i8*, align 4
+// CHECK-NEXT: %[[_ADDR1:.*]] = alloca i8*, align 4
+// CHECK-NEXT: store i8* %0, i8** %[[_ADDR]], align 4
+// CHECK-NEXT: store i8* %1, i8** %[[_ADDR1]], align 4
+// CHECK-NEXT: %[[V2:.*]] = load i8*, i8** %[[_ADDR1]], align 4
+// CHECK-NEXT: %[[BLOCK_SOURCE:.*]] = bitcast i8* %[[V2]] to <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8* }>*
+// CHECK-NEXT: %[[V3:.*]] = load i8*, i8** %[[_ADDR]], align 4
+// CHECK-NEXT: %[[BLOCK_DEST:.*]] = bitcast i8* %[[V3]] to <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8* }>*
+// CHECK-NEXT: %[[V4:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8* }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8* }>* %[[BLOCK_SOURCE]], i32 0, i32 5
+// CHECK-NEXT: %[[V5:.*]] = getelementptr inbounds <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8* }>, <{ i8*, i32, i32, i8*, %[[STRUCT_BLOCK_DESCRIPTOR]]*, i8* }>* %[[BLOCK_DEST]], i32 0, i32 5
+// CHECK-NEXT: %[[BLOCKCOPY_SRC:.*]] = load i8*, i8** %[[V4]], align 4
+// CHECK-NEXT: