[PATCH] D41228: [ObjC] Enable __strong pointers in structs under ARC

2017-12-20 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: include/clang/AST/Type.h:1152
+NTFK_Struct,  // non-trivial C struct.
+NTFK_Array// array that has non-trivial elements.
+  };

ahatanak wrote:
> rjmccall wrote:
> > We don't actually distinguish arrays in DestructionKind.  Is it important 
> > here?  You can't actually do anything with that information.
> > 
> > Regarding your naming question, I wonder if it would be useful to just 
> > invent a new term here, something meaning "non-trivial" but without the C++ 
> > baggage.  Maybe just "PrimitiveCopyKind", with the understanding that C++ 
> > can never do a "primitive" copy?  And for consistency with DestructionKind, 
> > maybe you should lowercase the second words.
> > 
> > I'm not sure how isNonTrivialToCopy is supposed to be used.  Where does the 
> > function come from?  And why is a context required when it isn't required 
> > for isDestructedType?
> Enum NonTrivialCopyKind and isNonTrivialToCopy() are used just to distinguish 
> the different types of fields in a C struct. Currently, there are four types 
> that are handled by visitField functions in CGNonTrivialStruct.cpp:
> 
> - struct field
> - array field
> - __strong field
> - trivial field that can be copied using memcpy
> 
> I thought using enums would make the code easier to read, but of course it's 
> possible to remove them and instead just check the field types calling 
> functions like getAsArrayType or getAs. ASTContext is passed to 
> isNonTrivialToCopy so that it can call ASTContext::getAsArrayType to 
> determine whether the type is an array. Maybe there is another way to detect 
> an array that doesn't require ASTContext?
> 
> As for the naming, PrimitiveCopyKind seems find if we want to distinguish it 
> from C++ non-trivial classes (I don't have a better name). If we are going to 
> call non-trivial C structs primitiveCopyKind, what should we call the C 
> structs that are trivial (those that can be copied using memcpy)?
I think "trivial" is a reasonable kind of primitive-copy semantics.  Basically, 
we're saying that all complete object types other than non-trivial C++ types 
have some sort of primitive-copy semantics, and this enum tells us what those 
semantics are.

DestructionKind has to deal with arrays as well, but it does so by just 
reporting the appropriate value for the underlying element type without 
mentioning that it's specifically an *array* of the type.  I think that's a 
reasonable design, since most places do not need to distinguish arrays from 
non-arrays.  IRGen does, but only when you're actually finally emitting code; 
prior to that (when e.g. deciding that a field is non-trivial to copy) you can 
just use the enum value.

You can do without getAsArrayType the same way that isDestructedType() does: 
the more complex operation is only necessary in order to preserve qualifiers on 
the element type, but that's unnecessary for this operation because the array 
type itself is considered to have the same qualifiers as its element type.  
That is, you can ask the original type whether it's __strong/__weak-qualified 
without having to specifically handle array types, and once you've done all of 
those queries, you can use the "unsafe" operations to drill down to the element 
type because you no longer care about qualification.


https://reviews.llvm.org/D41228



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


[PATCH] D41103: [CMake] Allow passing extra CMake arguments to custom libc++

2017-12-20 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment.

ping?


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D41103



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


[PATCH] D40705: Diagnose invalid decl-specifiers in non-type template parameter declarations (original author miyuki!)

2017-12-20 Thread Faisal Vali via Phabricator via cfe-commits
faisalv updated this revision to Diff 127828.
faisalv retitled this revision from "[Parser] Diagnose storage classes in 
template parameter declarations" to "Diagnose invalid decl-specifiers in 
non-type template parameter declarations (original author miyuki!)".
faisalv edited the summary of this revision.
faisalv set the repository for this revision to rC Clang.
faisalv added projects: clang, clang-c.
faisalv added a comment.

Miyuki - please take a look at the patch and let me know if you agree with the 
changes - or have any concerns...

Thank you!

P.S. While I can see the argument for having these syntactical checks be done 
by the Parser - since they could be described as context sensitive syntax 
analysis, moving the changes to Sema also seems quite appropriate.


Repository:
  rC Clang

https://reviews.llvm.org/D40705

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaTemplate.cpp
  test/CXX/temp/temp.param/p2.cpp

Index: test/CXX/temp/temp.param/p2.cpp
===
--- test/CXX/temp/temp.param/p2.cpp
+++ test/CXX/temp/temp.param/p2.cpp
@@ -1,5 +1,6 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
-// expected-no-diagnostics
+// RUN: %clang_cc1 -fsyntax-only -verify %s 
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s -DCPP11
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify %s -DCPP17
 
 // There is no semantic difference between class and typename in a
 // template-parameter. typename followed by an unqualified-id names a
@@ -13,8 +14,32 @@
 template::type Value> struct Y1;
 
 // A storage class shall not be specified in a template-parameter declaration.
-template struct Z; // FIXME: expect an error
+template struct Z; //expected-error{{invalid declaration specifier}}
+template struct Z0; //expected-error{{invalid declaration specifier}}
+template struct Z0; //expected-error2{{invalid declaration specifier}}
+template struct Z0; //expected-error{{invalid declaration specifier}}
+template struct Z0; //expected-error{{invalid declaration specifier}}
+template struct Z0; //expected-error{{invalid declaration specifier}}
+template struct Z0; //expected-error{{invalid declaration specifier}}
+template struct Z0;  //expected-error{{invalid declaration specifier}}
+template struct Z0; //expected-error{{invalid declaration specifier}}
+template struct Z0; //expected-error{{invalid declaration specifier}}
 
+template struct Z0; // OK 
+template struct Z0; // OK
+
+
+
+#ifdef CPP11
+template struct Z0; //expected-error{{invalid declaration specifier}}
+template struct Z0; //expected-error{{invalid declaration specifier}}
+
+#endif
+
+#ifdef CPP17
+template struct Z1; // OK
+#endif
+
 // Make sure that we properly disambiguate non-type template parameters that
 // start with 'class'.
 class X1 { };
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -929,6 +929,60 @@
   Expr *Default) {
   TypeSourceInfo *TInfo = GetTypeForDeclarator(D, S);
 
+  // Check that we have valid decl-specifiers specified.
+  auto CheckValidDeclSpecifiers = [this, ] {
+// C++ [temp.param]
+// p1 
+//   template-parameter:
+// ...
+// parameter-declaration
+// p2 
+//   ... A storage class shall not be specified in a template-parameter
+//   declaration.
+// [dcl.typedef]p1: 
+//   The typedef specifier [...] shall not be used in the decl-specifier-seq
+//   of a parameter-declaration
+const DeclSpec  = D.getDeclSpec();
+auto EmitDiag = [this](SourceLocation Loc) {
+  Diag(Loc, diag::err_invalid_decl_specifier_in_nontype_parm)
+  << FixItHint::CreateRemoval(Loc);
+};
+if (DS.getStorageClassSpec() != DeclSpec::SCS_unspecified)
+  EmitDiag(DS.getStorageClassSpecLoc());
+
+if (DeclSpec::TSCS TSCS = DS.getThreadStorageClassSpec())
+  EmitDiag(DS.getThreadStorageClassSpecLoc());
+
+// [dcl.inline]p1: 
+//   The inline specifier can be applied only to the declaration or 
+//   definition of a variable or function.
+
+if (DS.isInlineSpecified())
+  EmitDiag(DS.getInlineSpecLoc());
+
+// [dcl.constexpr]p1:
+//   The constexpr specifier shall be applied only to the definition of a 
+//   variable or variable template or the declaration of a function or 
+//   function template.
+
+if (DS.isConstexprSpecified())
+  EmitDiag(DS.getConstexprSpecLoc());
+
+// [dcl.fct.spec]p1:
+//   Function-specifiers can be used only in function declarations.
+
+if (DS.isVirtualSpecified())
+  EmitDiag(DS.getVirtualSpecLoc());
+
+if (DS.isExplicitSpecified())
+  EmitDiag(DS.getExplicitSpecLoc());
+
+if (DS.isNoreturnSpecified())
+  EmitDiag(DS.getNoreturnSpecLoc());
+  };
+
+  CheckValidDeclSpecifiers();
+  
   if 

[PATCH] D41228: [ObjC] Enable __strong pointers in structs under ARC

2017-12-20 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added inline comments.



Comment at: include/clang/AST/Type.h:1152
+NTFK_Struct,  // non-trivial C struct.
+NTFK_Array// array that has non-trivial elements.
+  };

rjmccall wrote:
> We don't actually distinguish arrays in DestructionKind.  Is it important 
> here?  You can't actually do anything with that information.
> 
> Regarding your naming question, I wonder if it would be useful to just invent 
> a new term here, something meaning "non-trivial" but without the C++ baggage. 
>  Maybe just "PrimitiveCopyKind", with the understanding that C++ can never do 
> a "primitive" copy?  And for consistency with DestructionKind, maybe you 
> should lowercase the second words.
> 
> I'm not sure how isNonTrivialToCopy is supposed to be used.  Where does the 
> function come from?  And why is a context required when it isn't required for 
> isDestructedType?
Enum NonTrivialCopyKind and isNonTrivialToCopy() are used just to distinguish 
the different types of fields in a C struct. Currently, there are four types 
that are handled by visitField functions in CGNonTrivialStruct.cpp:

- struct field
- array field
- __strong field
- trivial field that can be copied using memcpy

I thought using enums would make the code easier to read, but of course it's 
possible to remove them and instead just check the field types calling 
functions like getAsArrayType or getAs. ASTContext is passed to 
isNonTrivialToCopy so that it can call ASTContext::getAsArrayType to determine 
whether the type is an array. Maybe there is another way to detect an array 
that doesn't require ASTContext?

As for the naming, PrimitiveCopyKind seems find if we want to distinguish it 
from C++ non-trivial classes (I don't have a better name). If we are going to 
call non-trivial C structs primitiveCopyKind, what should we call the C structs 
that are trivial (those that can be copied using memcpy)?


https://reviews.llvm.org/D41228



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


[PATCH] D41399: [CodeGen] Represent array members in new-format TBAA type descriptors

2017-12-20 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel accepted this revision.
hfinkel added a comment.
This revision is now accepted and ready to land.

LGTM. The array accesses here are just being represented as their scalar-access 
types. In the future, we should update this to represent them as fields in 
their parent structs, but this is fine for now.


https://reviews.llvm.org/D41399



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


[PATCH] D41452: [CodeGen] Fix access sizes in new-format TBAA tags

2017-12-20 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel accepted this revision.
hfinkel added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D41452#961710, @rjmccall wrote:

> LGTM.


Me too.


Repository:
  rL LLVM

https://reviews.llvm.org/D41452



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


[PATCH] D41471: [CMake][Fuchsia] Enable assertions

2017-12-20 Thread Petr Hosek via Phabricator via cfe-commits
phosek accepted this revision.
phosek added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D41471



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


[PATCH] D41478: [analyzer] Fix zero-initialization of stack VLAs under ARC.

2017-12-20 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision.
NoQ added reviewers: dcoughlin, george.karpenkov.
Herald added subscribers: cfe-commits, a.sidorin, szepet, xazax.hun.

https://developer.apple.com/library/content/releasenotes/ObjectiveC/RN-TransitioningToARC/Introduction/Introduction.html#//apple_ref/doc/uid/TP40011226-CH1-SW5
 :

> Using ARC, strong, weak, and autoreleasing stack variables are now implicitly 
> initialized with nil.

This includes variable-length arrays of Objective-C object pointers. However, 
in the analyzer we don't zero-initialize them. We used to, but it accidentally 
regressed after r289618.

Under ARC, the array variable's initializer within `DeclStmt` is an 
`ImplicitValueInitExpr`. Environment doesn't maintain any bindings for this 
expression kind - instead it always knows that it's a known constant (0 in our 
case), so it just returns the known value by calling 
`SValBuilder::makeZeroVal()` (see `EnvironmentManager::getSVal()`. Commit 
r289618 had introduced reasonable behavior of `SValBuilder::makeZeroVal()` for 
the arrays, which produces a zero-length `compoundVal{}`. When such value is 
bound to arrays, in `RegionStoreManager::bindArray()` "remaining" items in the 
array are default-initialized with zero, as in 
`RegionStoreManager::setImplicitDefaultValue()`. The similar mechanism works 
when an array is initialized by an initializer list that is too short, eg. `int 
a[3] = { 1, 2 };` would result in `a[2]` initialized with `0`. However, in case 
of variable-length arrays it didn't know if any more items need to be added, 
because, well, the length is variable.

Add the default binding anyway, regardless of how many actually need to be 
added. We don't really care, because the default binding covers the whole array 
anyway.


Repository:
  rC Clang

https://reviews.llvm.org/D41478

Files:
  lib/StaticAnalyzer/Core/RegionStore.cpp
  test/Analysis/arc-zero-init.m


Index: test/Analysis/arc-zero-init.m
===
--- /dev/null
+++ test/Analysis/arc-zero-init.m
@@ -0,0 +1,46 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify -fobjc-arc %s
+
+#if __has_feature(objc_arc)
+// expected-no-diagnostics
+#endif
+
+@interface SomeClass
+@end
+
+void simpleStrongPointerValue() {
+  SomeClass *x;
+  if (x) {}
+#if !__has_feature(objc_arc)
+// expected-warning@-2{{Branch condition evaluates to a garbage value}}
+#endif
+}
+
+void simpleArray() {
+  SomeClass *vlaArray[5];
+
+  if (vlaArray[0]) {}
+#if !__has_feature(objc_arc)
+// expected-warning@-2{{Branch condition evaluates to a garbage value}}
+#endif
+}
+
+void variableLengthArray() {
+   int count = 1;
+   SomeClass * vlaArray[count];
+
+   if (vlaArray[0]) {}
+#if !__has_feature(objc_arc)
+  // expected-warning@-2{{Branch condition evaluates to a garbage value}}
+#endif
+}
+
+void variableLengthArrayWithExplicitStrongAttribute() {
+   int count = 1;
+   __attribute__((objc_ownership(strong))) SomeClass * vlaArray[count];
+
+   if (vlaArray[0]) {}
+#if !__has_feature(objc_arc)
+  // expected-warning@-2{{Branch condition evaluates to a garbage value}}
+#endif
+}
Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2132,9 +2132,10 @@
   NewB = bind(NewB, loc::MemRegionVal(ER), *VI);
   }
 
-  // If the init list is shorter than the array length, set the
-  // array default value.
-  if (Size.hasValue() && i < Size.getValue())
+  // If the init list is shorter than the array length (or the array has
+  // variable length), set the array default value. Values that are already set
+  // are not overwritten.
+  if (!Size.hasValue() || i < Size.getValue())
 NewB = setImplicitDefaultValue(NewB, R, ElementTy);
 
   return NewB;


Index: test/Analysis/arc-zero-init.m
===
--- /dev/null
+++ test/Analysis/arc-zero-init.m
@@ -0,0 +1,46 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify -fobjc-arc %s
+
+#if __has_feature(objc_arc)
+// expected-no-diagnostics
+#endif
+
+@interface SomeClass
+@end
+
+void simpleStrongPointerValue() {
+  SomeClass *x;
+  if (x) {}
+#if !__has_feature(objc_arc)
+// expected-warning@-2{{Branch condition evaluates to a garbage value}}
+#endif
+}
+
+void simpleArray() {
+  SomeClass *vlaArray[5];
+
+  if (vlaArray[0]) {}
+#if !__has_feature(objc_arc)
+// expected-warning@-2{{Branch condition evaluates to a garbage value}}
+#endif
+}
+
+void variableLengthArray() {
+   int count = 1;
+   SomeClass * vlaArray[count];
+
+   if (vlaArray[0]) {}
+#if !__has_feature(objc_arc)
+  // expected-warning@-2{{Branch condition evaluates to a garbage value}}
+#endif
+}
+
+void variableLengthArrayWithExplicitStrongAttribute() {
+   int 

[PATCH] D41228: [ObjC] Enable __strong pointers in structs under ARC

2017-12-20 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: include/clang/AST/Type.h:1152
+NTFK_Struct,  // non-trivial C struct.
+NTFK_Array// array that has non-trivial elements.
+  };

We don't actually distinguish arrays in DestructionKind.  Is it important here? 
 You can't actually do anything with that information.

Regarding your naming question, I wonder if it would be useful to just invent a 
new term here, something meaning "non-trivial" but without the C++ baggage.  
Maybe just "PrimitiveCopyKind", with the understanding that C++ can never do a 
"primitive" copy?  And for consistency with DestructionKind, maybe you should 
lowercase the second words.

I'm not sure how isNonTrivialToCopy is supposed to be used.  Where does the 
function come from?  And why is a context required when it isn't required for 
isDestructedType?


https://reviews.llvm.org/D41228



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


[PATCH] D24933: Enable configuration files in clang

2017-12-20 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel accepted this revision.
hfinkel added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: lib/Driver/Driver.cpp:1508
+if (!SystemConfigDir.empty())
+  llvm::errs() << "System configuration file directory: "
+   << SystemConfigDir << "\n";

configuration file directory -> configuration-file directory



Comment at: lib/Driver/Driver.cpp:1511
+if (!UserConfigDir.empty())
+  llvm::errs() << "User configuration file directory: "
+   << UserConfigDir << "\n";

configuration file directory -> configuration-file directory


https://reviews.llvm.org/D24933



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


[PATCH] D41452: [CodeGen] Fix access sizes in new-format TBAA tags

2017-12-20 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

LGTM.


Repository:
  rL LLVM

https://reviews.llvm.org/D41452



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


[PATCH] D41394: [CodeGen] Support generation of TBAA info in the new format

2017-12-20 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

You can pass multiple -check-prefix arguments to FileCheck and it'll match all 
of them.  You can use that to make your test change simpler: make the existing 
RUN check for both PATH and OLD-PATH and the new RUN check for both PATH and 
NEW-PATH, then change all the existing metadata matches to OLD-PATH.


https://reviews.llvm.org/D41394



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


[PATCH] D41311: [CodeGen] Fix crash when a function taking transparent union is redeclared.

2017-12-20 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

Okay, I think that's reasonable enough.  LGTM.


https://reviews.llvm.org/D41311



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


[PATCH] D41228: [ObjC] Enable __strong pointers in structs under ARC

2017-12-20 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak updated this revision to Diff 127818.
ahatanak added a comment.

- Improved IRGen for copying trivial fields in a non-trivial C struct. IRGen in 
CGNonTrivialStruct.cpp now calls a single memcpy if there are consecutive 
trivial fields in a struct (it does something similar to what FieldMemcpyizer 
does)

- IRGen for structs containing bitfields was not correct, so I fixed that too.


https://reviews.llvm.org/D41228

Files:
  docs/LanguageExtensions.rst
  include/clang/AST/Decl.h
  include/clang/AST/Type.h
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/AST/ASTContext.cpp
  lib/AST/Decl.cpp
  lib/AST/Type.cpp
  lib/CodeGen/CGBlocks.cpp
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CGDeclCXX.cpp
  lib/CodeGen/CGExprAgg.cpp
  lib/CodeGen/CGNonTrivialStruct.cpp
  lib/CodeGen/CMakeLists.txt
  lib/CodeGen/CodeGenFunction.h
  lib/Lex/PPMacroExpansion.cpp
  lib/Sema/JumpDiagnostics.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  test/ARCMT/checking.m
  test/CodeGenObjC/strong-in-c-struct.m
  test/Lexer/has_feature_objc_arc.m
  test/SemaObjC/arc-decls.m
  test/SemaObjC/arc-system-header.m
  test/SemaObjC/strong-in-c-struct.m

Index: test/SemaObjC/strong-in-c-struct.m
===
--- /dev/null
+++ test/SemaObjC/strong-in-c-struct.m
@@ -0,0 +1,56 @@
+// RUN: %clang_cc1 -triple arm64-apple-ios11 -fobjc-arc -fblocks  -fobjc-runtime=ios-11.0 -fsyntax-only -verify %s
+
+typedef struct {
+  id a;
+} Strong;
+
+void callee_variadic(const char *, ...);
+
+void test_variadic(void) {
+  Strong t;
+  callee_variadic("s", t); // expected-error {{cannot pass non-trivial C object of type 'Strong' by value to variadic function}}
+}
+
+void test_jump0(int cond) {
+  switch (cond) {
+  case 0:
+;
+Strong x; // expected-note {{jump bypasses initialization of variable of non-trivial C struct type}}
+break;
+  case 1: // expected-error {{cannot jump from switch statement to this case label}}
+x.a = 0;
+break;
+  }
+}
+
+void test_jump1(void) {
+  static void *ips[] = { & };
+L0:  // expected-note {{possible target of indirect goto}}
+  ;
+  Strong x; // expected-note {{jump exits scope of variable with non-trivial destructor}}
+  goto *ips; // expected-error {{cannot jump}}
+}
+
+typedef void (^BlockTy)(void);
+void func(BlockTy);
+void func2(Strong);
+
+void test_block_scope0(int cond) {
+  Strong x; // expected-note {{jump enters lifetime of block which captures a C struct that is non-trivial to destroy}}
+  switch (cond) {
+  case 0:
+func(^{ func2(x); });
+break;
+  default: // expected-error {{cannot jump from switch statement to this case label}}
+break;
+  }
+}
+
+void test_block_scope1(void) {
+  static void *ips[] = { & };
+L0:  // expected-note {{possible target of indirect goto}}
+  ;
+  Strong x; // expected-note {{jump exits scope of variable with non-trivial destructor}} expected-note {{jump exits lifetime of block which captures a C struct that is non-trivial to destroy}}
+  func(^{ func2(x); });
+  goto *ips; // expected-error {{cannot jump}}
+}
Index: test/SemaObjC/arc-system-header.m
===
--- test/SemaObjC/arc-system-header.m
+++ test/SemaObjC/arc-system-header.m
@@ -23,8 +23,7 @@
 }
 
 void test5(struct Test5 *p) {
-  p->field = 0; // expected-error {{'field' is unavailable in ARC}}
-// expected-note@arc-system-header.h:25 {{field has non-trivial ownership qualification}}
+  p->field = 0;
 }
 
 id test6() {
@@ -49,8 +48,7 @@
 
 extern void doSomething(Test9 arg);
 void test9() {
-Test9 foo2 = {0, 0}; // expected-error {{'field' is unavailable in ARC}}
- // expected-note@arc-system-header.h:56 {{field has non-trivial ownership qualification}}
+Test9 foo2 = {0, 0};
 doSomething(foo2);
 }
 #endif
Index: test/SemaObjC/arc-decls.m
===
--- test/SemaObjC/arc-decls.m
+++ test/SemaObjC/arc-decls.m
@@ -3,7 +3,7 @@
 // rdar://8843524
 
 struct A {
-id x; // expected-error {{ARC forbids Objective-C objects in struct}}
+id x;
 };
 
 union u {
@@ -13,7 +13,7 @@
 @interface I {
struct A a; 
struct B {
-id y[10][20]; // expected-error {{ARC forbids Objective-C objects in struct}}
+id y[10][20];
 id z;
} b;
 
@@ -23,7 +23,7 @@
 
 // rdar://10260525
 struct r10260525 {
-  id (^block) (); // expected-error {{ARC forbids blocks in struct}}
+  id (^block) ();
 };
 
 struct S { 
Index: test/Lexer/has_feature_objc_arc.m
===
--- test/Lexer/has_feature_objc_arc.m
+++ test/Lexer/has_feature_objc_arc.m
@@ -13,8 +13,16 @@
 void no_objc_arc_weak_feature();
 #endif
 
+#if __has_feature(objc_arc_fields)
+void has_objc_arc_fields();
+#else
+void no_objc_arc_fields();
+#endif
+
 // CHECK-ARC: void has_objc_arc_feature();
 // CHECK-ARC: void 

[PATCH] D40560: [analyzer] Get construction into `operator new` running in simple cases.

2017-12-20 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

A slower explanation of the approach in '3.' in the previous message:

(1) Evaluate operator new() aka the allocator call as usual.
(2) Take the return value of (1) as usual.
(3) Take `CXXConstructExpr` which is the child of the `CXXNewExpr` that 
triggered the allocator call on (1).
(4) Construct a `StackFrameContext` with `CXXConstructExpr` from (3).
(5) //**Don't**// put the newly constructed `StackFrameContext` on the location 
context stack.
(6) Construct the `StackArgumentsSpaceRegion` for the `StackFrameContext` from 
(4).
(7) Construct the `CXXThisRegion` for the `StackArgumentsSpaceRegion` from (6).
(8) Bind the return value from (2) to `CXXThisRegion` from (7) in the Store.
(9) Put the node with the state from (8) to the worklist as usual.
(10) `CoreEngine` says it's time to evaluate `CXXConstructExpr` from (3) as 
usual.
(11) Make sure that the binding we made in (8) survives garbage collection*.
(11) Construct `StackFrameContext` for the `CXXConstructExpr` from (3) as usual.
(12) `LocationContextManager` ensures that on (4) and on (11) we get //the 
same// `StackFrameContext`.
(13) //**Don't**// bind `CXXThisRegion` while entering the stack frame - it was 
already done in (8).
(14) Finally enter the stack frame we've constructed twice on (4) and on (11), 
as usual.
(15) Evaluate the constructor, as usual.
(16) Bind this-value to `CXXConstructExpr` after evaluation (as usual? not 
sure).
(17) Allow the binding in the Store we made on (8) to be garbage-colllected as 
usual.
(18) When evaluating `CXXNewExpr`, take value of `CXXConstructExpr` and bind it 
to `CXXNewExpr`.

__
*We  may modify `SymbolReaper::isLiveRegion()` for this purpose. Sounds easy.


https://reviews.llvm.org/D40560



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


[PATCH] D40560: [analyzer] Get construction into `operator new` running in simple cases.

2017-12-20 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

1. Devin suggested a fantastic idea: it may be great to have a new 
`LocationContext` for whatever happens within the big-new-expression. This 
would help immensely with CFG look-aheads and look-behinds and reduce parent 
map usage - not only for operator new, but for other constructors (into 
initializer lists, probably into function arguments). Let's call it 
`ConstructionTriggerContext` for now. We can also store our "`_this`" value in 
a program state map from `ConstructionTriggerContext` into `SVal`.

2. My reaction was that we can instead store our "`_this`" value in some sort 
of "`CXX_ThisRegion`" within the `StackLocalsSpaceRegion` that corresponds to 
`ConstructionTriggerContext`, and then copy it over to `CXXThisRegion` that 
corresponds to the `StackFrameContext` of the constructor. This would kinda 
make sense given the pseudocode that we're imagine here for the new-expression. 
However, this sounds a bit over-engineered because we're using the Store to 
just temporarily stash the value. Well, right now we're using Environment for 
that, but it's equally dirty.

3. Now, it might actually be better to store "`_this`" value directly into 
`CXXThisRegion` that corresponds to the `StackFrameContext` of the constructor 
(rather than stash it in another place and eventually copy), even if that stack 
frame has not been entered yet. Because the stack frame would anyway be entered 
almost immediately, we already know the parameters of the stack frame (parent 
location context, call site, block count, block id, element id), and location 
contexts  are uniqued, so we can add the store binding now to the stack frame 
of the future, and whenever we actually enter the stack frame, when formerly 
we'd have bound the value to `CXXThisRegion`, we'd see that the value is 
already there. In this case we don't immediately need 
`ConstructionTriggerContext` - we can still add it later to refactor 
look-aheads and stuff. The binding would anyway be garbage-collected once the 
constructor exits.

I'd be glad to implement approach 3. instead of the stack of values if it 
sounds reasonable.


https://reviews.llvm.org/D40560



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


Re: r321239 - Fix for PR32990

2017-12-20 Thread Hal Finkel via cfe-commits


On 12/20/2017 08:07 PM, Erich Keane via cfe-commits wrote:

Author: erichkeane
Date: Wed Dec 20 18:07:46 2017
New Revision: 321239

URL: http://llvm.org/viewvc/llvm-project?rev=321239=rev
Log:
Fix for PR32990

This fixes the bug in https://bugs.llvm.org/show_bug.cgi?id=32990.


Too late now, but "Fix for PR32990" is not a useful subject, and only 
the bug reference is not a useful commit message. Commits should 
independently describe the problem and the solution. "Fixes PR32990." 
should be only a part of the message.


Thanks again,
Hal



Patch By: zahiraam
Differential Revision: https://reviews.llvm.org/D39063

Added:
 cfe/trunk/test/CodeGenCXX/dllimport-virtual-base.cpp
 cfe/trunk/test/CodeGenCXX/external-linkage.cpp
Modified:
 cfe/trunk/lib/CodeGen/CodeGenModule.cpp
 cfe/trunk/test/CodeGenCXX/dllimport-dtor-thunks.cpp
 cfe/trunk/test/CodeGenCXX/dllimport-members.cpp
 cfe/trunk/test/CodeGenCXX/dllimport.cpp

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=321239=321238=321239=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Wed Dec 20 18:07:46 2017
@@ -855,14 +855,25 @@ CodeGenModule::getFunctionLinkage(Global
GVALinkage Linkage = getContext().GetGVALinkageForFunction(D);
  
if (isa(D) &&

-  getCXXABI().useThunkForDtorVariant(cast(D),
- GD.getDtorType())) {
-// Destructor variants in the Microsoft C++ ABI are always internal or
-// linkonce_odr thunks emitted on an as-needed basis.
-return Linkage == GVA_Internal ? llvm::GlobalValue::InternalLinkage
-   : llvm::GlobalValue::LinkOnceODRLinkage;
+  Context.getTargetInfo().getCXXABI().isMicrosoft()) {
+switch (GD.getDtorType()) {
+case CXXDtorType::Dtor_Base:
+  break;
+case CXXDtorType::Dtor_Comdat:
+case CXXDtorType::Dtor_Complete:
+  if (D->hasAttr() &&
+ (cast(D)->getParent()->getNumVBases() ||
+  (Linkage == GVA_AvailableExternally ||
+   Linkage == GVA_StrongExternal)))
+   return llvm::Function::AvailableExternallyLinkage;
+  else
+return Linkage == GVA_Internal ? llvm::GlobalValue::InternalLinkage
+   : llvm::GlobalValue::LinkOnceODRLinkage;
+case CXXDtorType::Dtor_Deleting:
+  return Linkage == GVA_Internal ? llvm::GlobalValue::InternalLinkage
+ : llvm::GlobalValue::LinkOnceODRLinkage;
+}
}
-
if (isa(D) &&
cast(D)->isInheritingConstructor() &&
Context.getTargetInfo().getCXXABI().isMicrosoft()) {
@@ -878,12 +889,25 @@ CodeGenModule::getFunctionLinkage(Global
  void CodeGenModule::setFunctionDLLStorageClass(GlobalDecl GD, llvm::Function 
*F) {
const auto *FD = cast(GD.getDecl());
  
-  if (const auto *Dtor = dyn_cast_or_null(FD)) {

-if (getCXXABI().useThunkForDtorVariant(Dtor, GD.getDtorType())) {
+  if (dyn_cast_or_null(FD)) {
+switch (GD.getDtorType()) {
+case CXXDtorType::Dtor_Comdat:
+case CXXDtorType::Dtor_Deleting: {
// Don't dllexport/import destructor thunks.
F->setDLLStorageClass(llvm::GlobalValue::DefaultStorageClass);
return;
  }
+case CXXDtorType::Dtor_Complete:
+  if (FD->hasAttr())
+F->setDLLStorageClass(llvm::GlobalVariable::DLLImportStorageClass);
+  else if (FD->hasAttr())
+F->setDLLStorageClass(llvm::GlobalVariable::DLLExportStorageClass);
+  else
+F->setDLLStorageClass(llvm::GlobalVariable::DefaultStorageClass);
+  return;
+case CXXDtorType::Dtor_Base:
+  break;
+}
}
  
if (FD->hasAttr())


Modified: cfe/trunk/test/CodeGenCXX/dllimport-dtor-thunks.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/dllimport-dtor-thunks.cpp?rev=321239=321238=321239=diff
==
--- cfe/trunk/test/CodeGenCXX/dllimport-dtor-thunks.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/dllimport-dtor-thunks.cpp Wed Dec 20 18:07:46 2017
@@ -1,4 +1,5 @@
  // RUN: %clang_cc1 -mconstructor-aliases %s -triple x86_64-windows-msvc 
-fms-extensions -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -mconstructor-aliases %s -triple x86_64-windows-msvc 
-fms-extensions -emit-llvm -O1 -disable-llvm-passes -o - | FileCheck 
--check-prefix=MO1 %s
  
  // FIXME: We should really consider removing -mconstructor-aliases for MS C++

  // ABI. The risk of bugs introducing ABI incompatibility under
@@ -23,9 +24,7 @@ struct __declspec(dllimport) ImportOverr
virtual ~ImportOverrideVDtor() {}
  };
  
-// Virtually inherits from a non-dllimport base class. This time we need to call

-// the complete destructor and emit it inline. It's not exported from the DLL,
-// 

r321239 - Fix for PR32990

2017-12-20 Thread Erich Keane via cfe-commits
Author: erichkeane
Date: Wed Dec 20 18:07:46 2017
New Revision: 321239

URL: http://llvm.org/viewvc/llvm-project?rev=321239=rev
Log:
Fix for PR32990

This fixes the bug in https://bugs.llvm.org/show_bug.cgi?id=32990.

Patch By: zahiraam
Differential Revision: https://reviews.llvm.org/D39063

Added:
cfe/trunk/test/CodeGenCXX/dllimport-virtual-base.cpp
cfe/trunk/test/CodeGenCXX/external-linkage.cpp
Modified:
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/test/CodeGenCXX/dllimport-dtor-thunks.cpp
cfe/trunk/test/CodeGenCXX/dllimport-members.cpp
cfe/trunk/test/CodeGenCXX/dllimport.cpp

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=321239=321238=321239=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Wed Dec 20 18:07:46 2017
@@ -855,14 +855,25 @@ CodeGenModule::getFunctionLinkage(Global
   GVALinkage Linkage = getContext().GetGVALinkageForFunction(D);
 
   if (isa(D) &&
-  getCXXABI().useThunkForDtorVariant(cast(D),
- GD.getDtorType())) {
-// Destructor variants in the Microsoft C++ ABI are always internal or
-// linkonce_odr thunks emitted on an as-needed basis.
-return Linkage == GVA_Internal ? llvm::GlobalValue::InternalLinkage
-   : llvm::GlobalValue::LinkOnceODRLinkage;
+  Context.getTargetInfo().getCXXABI().isMicrosoft()) {
+switch (GD.getDtorType()) {
+case CXXDtorType::Dtor_Base:
+  break;
+case CXXDtorType::Dtor_Comdat:
+case CXXDtorType::Dtor_Complete:
+  if (D->hasAttr() &&
+ (cast(D)->getParent()->getNumVBases() ||
+  (Linkage == GVA_AvailableExternally ||
+   Linkage == GVA_StrongExternal)))
+   return llvm::Function::AvailableExternallyLinkage;
+  else
+return Linkage == GVA_Internal ? llvm::GlobalValue::InternalLinkage
+   : llvm::GlobalValue::LinkOnceODRLinkage;
+case CXXDtorType::Dtor_Deleting:
+  return Linkage == GVA_Internal ? llvm::GlobalValue::InternalLinkage
+ : llvm::GlobalValue::LinkOnceODRLinkage;
+}
   }
-
   if (isa(D) &&
   cast(D)->isInheritingConstructor() &&
   Context.getTargetInfo().getCXXABI().isMicrosoft()) {
@@ -878,12 +889,25 @@ CodeGenModule::getFunctionLinkage(Global
 void CodeGenModule::setFunctionDLLStorageClass(GlobalDecl GD, llvm::Function 
*F) {
   const auto *FD = cast(GD.getDecl());
 
-  if (const auto *Dtor = dyn_cast_or_null(FD)) {
-if (getCXXABI().useThunkForDtorVariant(Dtor, GD.getDtorType())) {
+  if (dyn_cast_or_null(FD)) {
+switch (GD.getDtorType()) {
+case CXXDtorType::Dtor_Comdat:
+case CXXDtorType::Dtor_Deleting: {
   // Don't dllexport/import destructor thunks.
   F->setDLLStorageClass(llvm::GlobalValue::DefaultStorageClass);
   return;
 }
+case CXXDtorType::Dtor_Complete:
+  if (FD->hasAttr())
+F->setDLLStorageClass(llvm::GlobalVariable::DLLImportStorageClass);
+  else if (FD->hasAttr())
+F->setDLLStorageClass(llvm::GlobalVariable::DLLExportStorageClass);
+  else
+F->setDLLStorageClass(llvm::GlobalVariable::DefaultStorageClass);
+  return;
+case CXXDtorType::Dtor_Base:
+  break;
+}
   }
 
   if (FD->hasAttr())

Modified: cfe/trunk/test/CodeGenCXX/dllimport-dtor-thunks.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/dllimport-dtor-thunks.cpp?rev=321239=321238=321239=diff
==
--- cfe/trunk/test/CodeGenCXX/dllimport-dtor-thunks.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/dllimport-dtor-thunks.cpp Wed Dec 20 18:07:46 2017
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -mconstructor-aliases %s -triple x86_64-windows-msvc 
-fms-extensions -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -mconstructor-aliases %s -triple x86_64-windows-msvc 
-fms-extensions -emit-llvm -O1 -disable-llvm-passes -o - | FileCheck 
--check-prefix=MO1 %s
 
 // FIXME: We should really consider removing -mconstructor-aliases for MS C++
 // ABI. The risk of bugs introducing ABI incompatibility under
@@ -23,9 +24,7 @@ struct __declspec(dllimport) ImportOverr
   virtual ~ImportOverrideVDtor() {}
 };
 
-// Virtually inherits from a non-dllimport base class. This time we need to 
call
-// the complete destructor and emit it inline. It's not exported from the DLL,
-// and it must be emitted.
+// Virtually inherits from a non-dllimport base class. Emit the vbase 
destructor.
 struct __declspec(dllimport) ImportVBaseOverrideVDtor
 : public virtual BaseClass {
   virtual ~ImportVBaseOverrideVDtor() {}
@@ -41,9 +40,11 @@ extern "C" void testit() {
 // needs the complete destructor (_D).
 // CHECK-LABEL: define void @testit()
 

r321237 - Reverting a file that snuck in with r321229 by accident.

2017-12-20 Thread Aaron Ballman via cfe-commits
Author: aaronballman
Date: Wed Dec 20 17:34:46 2017
New Revision: 321237

URL: http://llvm.org/viewvc/llvm-project?rev=321237=rev
Log:
Reverting a file that snuck in with r321229 by accident.

Modified:
cfe/trunk/test/Misc/ast-dump-color.cpp

Modified: cfe/trunk/test/Misc/ast-dump-color.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/ast-dump-color.cpp?rev=321237=321236=321237=diff
==
--- cfe/trunk/test/Misc/ast-dump-color.cpp (original)
+++ cfe/trunk/test/Misc/ast-dump-color.cpp Wed Dec 20 17:34:46 2017
@@ -43,7 +43,7 @@ struct Invalid {
 //CHECK: {{^}}[[Blue]]| 
|-[[RESET]][[Blue]]HTMLEndTagComment[[RESET]][[Yellow]] 
0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:13[[RESET]], 
[[Yellow]]col:16[[RESET]]> Name="a"{{$}}
 //CHECK: {{^}}[[Blue]]| |-[[RESET]][[Blue]]TextComment[[RESET]][[Yellow]] 
0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]line:5:4[[RESET]]> Text=" "{{$}}
 //CHECK: {{^}}[[Blue]]| 
`-[[RESET]][[Blue]]HTMLStartTagComment[[RESET]][[Yellow]] 
0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:5[[RESET]], 
[[Yellow]]col:8[[RESET]]> Name="br" SelfClosing{{$}}
-//CHECK: {{^}}[[Blue]]|-[[RESET]][[GREEN]]FunctionDecl[[RESET]][[Yellow]] 
0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]line:9:1[[RESET]], 
[[Yellow]]line:16:1[[RESET]]> [[Yellow]]line:9:6[[RESET]][[CYAN]] 
TestAttributedStmt[[RESET]] [[Green]]'void ()'[[RESET]]{{$}}
+//CHECK: {{^}}[[Blue]]|-[[RESET]][[GREEN]]FunctionDecl[[RESET]][[Yellow]] 
0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]line:9:1[[RESET]], 
[[Yellow]]line:16:1[[RESET]]> [[Yellow]]line:9:6[[RESET]][[CYAN]] 
TestAttributedStmt[[RESET]] [[Green]]'void (void)'[[RESET]]{{$}}
 //CHECK: {{^}}[[Blue]]| 
|-[[RESET]][[MAGENTA:.\[0;1;35m]]CompoundStmt[[RESET]][[Yellow]] 
0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:27[[RESET]], 
[[Yellow]]line:16:1[[RESET]]>{{$}}
 //CHECK: {{^}}[[Blue]]| | `-[[RESET]][[MAGENTA]]SwitchStmt[[RESET]][[Yellow]] 
0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]line:10:3[[RESET]], 
[[Yellow]]line:15:3[[RESET]]>{{$}}
 //CHECK: {{^}}[[Blue]]| |   
|-[[RESET]][[Blue:.\[0;34m]]<<>>[[RESET]]{{$}}


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


LLVM buildmaster will be updated and restarted tonight

2017-12-20 Thread Galina Kistanova via cfe-commits
Hello everyone,

LLVM buildmaster will be updated and restarted after 8 PM Pacific time.

Thanks

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


[PATCH] D41363: [clang-tidy] Adding Fuchsia checker for overloaded operators

2017-12-20 Thread Julie Hockett via Phabricator via cfe-commits
juliehockett updated this revision to Diff 127806.
juliehockett added a comment.

Updating matcher to include overloaded operators outside classes.


https://reviews.llvm.org/D41363

Files:
  clang-tidy/fuchsia/CMakeLists.txt
  clang-tidy/fuchsia/FuchsiaTidyModule.cpp
  clang-tidy/fuchsia/OverloadedOperatorCheck.cpp
  clang-tidy/fuchsia/OverloadedOperatorCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/fuchsia-overloaded-operator.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/fuchsia-overloaded-operator.cpp

Index: test/clang-tidy/fuchsia-overloaded-operator.cpp
===
--- /dev/null
+++ test/clang-tidy/fuchsia-overloaded-operator.cpp
@@ -0,0 +1,18 @@
+// RUN: %check_clang_tidy %s fuchsia-overloaded-operator %t
+
+class A {
+public:
+  int operator+(int);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: cannot overload 'operator+' [fuchsia-overloaded-operator]
+};
+
+class B {
+public:
+  B =(const B& other);
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:3: warning: cannot overload 'operator=' [fuchsia-overloaded-operator]
+  B =(B &);
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:3: warning: cannot overload 'operator=' [fuchsia-overloaded-operator]
+};
+
+A operator-(const A& a, const A& b);
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: cannot overload 'operator-' [fuchsia-overloaded-operator]
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -69,6 +69,7 @@
cppcoreguidelines-slicing
cppcoreguidelines-special-member-functions
fuchsia-default-arguments
+   fuchsia-overloaded-operator
fuchsia-virtual-inheritance
google-build-explicit-make-pair
google-build-namespaces
Index: docs/clang-tidy/checks/fuchsia-overloaded-operator.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/fuchsia-overloaded-operator.rst
@@ -0,0 +1,18 @@
+.. title:: clang-tidy - fuchsia-overloaded-operator
+
+fuchsia-overloaded-operator
+===
+
+Warns if an operator is overloaded, except for the assignment (copy and move) 
+operators.
+
+For example:
+
+.. code-block:: c++
+
+  int operator+(int); // Warning
+
+  B =(const B& other);  // No warning
+  B =(B &) // No warning
+
+See the features disallowed in Fuchsia at https://fuchsia.googlesource.com/zircon/+/master/docs/cxx.md
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -134,7 +134,12 @@
   `_ check
 
   Warns if a function or method is declared or called with default arguments.
-  
+
+- New `fuchsia-overloaded-operator
+  `_ check
+
+  Warns if an operator is overloaded, except for the assignment (copy and move) operators.
+
 - New `fuchsia-virtual-inheritance
   `_ check
 
Index: clang-tidy/fuchsia/OverloadedOperatorCheck.h
===
--- /dev/null
+++ clang-tidy/fuchsia/OverloadedOperatorCheck.h
@@ -0,0 +1,35 @@
+//===--- OverloadedOperatorCheck.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_FUCHSIA_OVERLOADED_OPERATOR_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_OVERLOADED_OPERATOR_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace fuchsia {
+
+/// Overloading operators is disallowed by the Fuchsia coding standard.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/fuchsia-overloaded-operator.html
+class OverloadedOperatorCheck : public ClangTidyCheck {
+public:
+  OverloadedOperatorCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace fuchsia
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_OVERLOADED_OPERATOR_H
Index: clang-tidy/fuchsia/OverloadedOperatorCheck.cpp
===
--- /dev/null
+++ clang-tidy/fuchsia/OverloadedOperatorCheck.cpp
@@ -0,0 +1,39 @@
+//===--- OverloadedOperatorCheck.cpp - clang-tidy--===//
+//
+// The LLVM Compiler Infrastructure
+//

[PATCH] D41471: [CMake][Fuchsia] Enable assertions

2017-12-20 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr created this revision.
mcgrathr added a reviewer: phosek.
Herald added a subscriber: mgorny.

Enable assertions in both stages.
Release+Asserts is fast enough.
No need to let insanity through.


Repository:
  rC Clang

https://reviews.llvm.org/D41471

Files:
  cmake/caches/Fuchsia-stage2.cmake
  cmake/caches/Fuchsia.cmake


Index: cmake/caches/Fuchsia.cmake
===
--- cmake/caches/Fuchsia.cmake
+++ cmake/caches/Fuchsia.cmake
@@ -13,6 +13,7 @@
 set(LLVM_ENABLE_ZLIB OFF CACHE BOOL "")
 set(CLANG_PLUGIN_SUPPORT OFF CACHE BOOL "")
 
+set(LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
 set(CMAKE_BUILD_TYPE Release CACHE STRING "")
 
 set(BOOTSTRAP_LLVM_ENABLE_LTO ON CACHE BOOL "")
Index: cmake/caches/Fuchsia-stage2.cmake
===
--- cmake/caches/Fuchsia-stage2.cmake
+++ cmake/caches/Fuchsia-stage2.cmake
@@ -23,6 +23,7 @@
   set(LLDB_CODESIGN_IDENTITY "" CACHE STRING "")
 endif()
 
+set(LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
 set(CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING "")
 set(CMAKE_C_FLAGS_RELWITHDEBINFO "-O3 -gline-tables-only -DNDEBUG" CACHE 
STRING "")
 set(CMAKE_CXX_FLAGS_RELWITHDEBINFO "-O3 -gline-tables-only -DNDEBUG" CACHE 
STRING "")


Index: cmake/caches/Fuchsia.cmake
===
--- cmake/caches/Fuchsia.cmake
+++ cmake/caches/Fuchsia.cmake
@@ -13,6 +13,7 @@
 set(LLVM_ENABLE_ZLIB OFF CACHE BOOL "")
 set(CLANG_PLUGIN_SUPPORT OFF CACHE BOOL "")
 
+set(LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
 set(CMAKE_BUILD_TYPE Release CACHE STRING "")
 
 set(BOOTSTRAP_LLVM_ENABLE_LTO ON CACHE BOOL "")
Index: cmake/caches/Fuchsia-stage2.cmake
===
--- cmake/caches/Fuchsia-stage2.cmake
+++ cmake/caches/Fuchsia-stage2.cmake
@@ -23,6 +23,7 @@
   set(LLDB_CODESIGN_IDENTITY "" CACHE STRING "")
 endif()
 
+set(LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
 set(CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING "")
 set(CMAKE_C_FLAGS_RELWITHDEBINFO "-O3 -gline-tables-only -DNDEBUG" CACHE STRING "")
 set(CMAKE_CXX_FLAGS_RELWITHDEBINFO "-O3 -gline-tables-only -DNDEBUG" CACHE STRING "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D41363: [clang-tidy] Adding Fuchsia checker for overloaded operators

2017-12-20 Thread Julie Hockett via Phabricator via cfe-commits
juliehockett added inline comments.



Comment at: docs/clang-tidy/checks/fuchsia-overloaded-operator.rst:17
+
+See the features disallowed in Fuchsia at 
https://fuchsia.googlesource.com/zircon/+/master/docs/cxx.md

JonasToth wrote:
> Could you make the link clickable? 
Doesn't rST parse standalone links this into hyperlinks without additional 
markup?


https://reviews.llvm.org/D41363



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


[PATCH] D41363: [clang-tidy] Adding Fuchsia checker for overloaded operators

2017-12-20 Thread Julie Hockett via Phabricator via cfe-commits
juliehockett updated this revision to Diff 127799.
juliehockett marked 7 inline comments as done.
juliehockett added a comment.

Fixing comments


https://reviews.llvm.org/D41363

Files:
  clang-tidy/fuchsia/CMakeLists.txt
  clang-tidy/fuchsia/FuchsiaTidyModule.cpp
  clang-tidy/fuchsia/OverloadedOperatorCheck.cpp
  clang-tidy/fuchsia/OverloadedOperatorCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/fuchsia-overloaded-operator.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/fuchsia-overloaded-operator.cpp

Index: test/clang-tidy/fuchsia-overloaded-operator.cpp
===
--- /dev/null
+++ test/clang-tidy/fuchsia-overloaded-operator.cpp
@@ -0,0 +1,15 @@
+// RUN: %check_clang_tidy %s fuchsia-overloaded-operator %t
+
+class A {
+public:
+  int operator+(int);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: cannot overload 'operator+' [fuchsia-overloaded-operator]
+};
+
+class B {
+public:
+  B =(const B& other);
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:3: warning: cannot overload 'operator=' [fuchsia-overloaded-operator]
+  B =(B &);
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:3: warning: cannot overload 'operator=' [fuchsia-overloaded-operator]
+};
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -69,6 +69,7 @@
cppcoreguidelines-slicing
cppcoreguidelines-special-member-functions
fuchsia-default-arguments
+   fuchsia-overloaded-operator
fuchsia-virtual-inheritance
google-build-explicit-make-pair
google-build-namespaces
Index: docs/clang-tidy/checks/fuchsia-overloaded-operator.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/fuchsia-overloaded-operator.rst
@@ -0,0 +1,18 @@
+.. title:: clang-tidy - fuchsia-overloaded-operator
+
+fuchsia-overloaded-operator
+===
+
+Warns if an operator is overloaded, except for the assignment (copy and move) 
+operators.
+
+For example:
+
+.. code-block:: c++
+
+  int operator+(int); // Warning
+
+  B =(const B& other);  // No warning
+  B =(B &) // No warning
+
+See the features disallowed in Fuchsia at https://fuchsia.googlesource.com/zircon/+/master/docs/cxx.md
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -134,7 +134,12 @@
   `_ check
 
   Warns if a function or method is declared or called with default arguments.
-  
+
+- New `fuchsia-overloaded-operator
+  `_ check
+
+  Warns if an operator is overloaded, except for the assignment (copy and move) operators.
+
 - New `fuchsia-virtual-inheritance
   `_ check
 
Index: clang-tidy/fuchsia/OverloadedOperatorCheck.h
===
--- /dev/null
+++ clang-tidy/fuchsia/OverloadedOperatorCheck.h
@@ -0,0 +1,35 @@
+//===--- OverloadedOperatorCheck.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_FUCHSIA_OVERLOADED_OPERATOR_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_OVERLOADED_OPERATOR_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace fuchsia {
+
+/// Overloading operators is disallowed by the Fuchsia coding standard.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/fuchsia-overloaded-operator.html
+class OverloadedOperatorCheck : public ClangTidyCheck {
+public:
+  OverloadedOperatorCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace fuchsia
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_FUCHSIA_OVERLOADED_OPERATOR_H
Index: clang-tidy/fuchsia/OverloadedOperatorCheck.cpp
===
--- /dev/null
+++ clang-tidy/fuchsia/OverloadedOperatorCheck.cpp
@@ -0,0 +1,35 @@
+//===--- OverloadedOperatorCheck.cpp - clang-tidy--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//

[PATCH] D40700: [ubsan] Diagnose noreturn functions which return (compiler-rt)

2017-12-20 Thread Phabricator via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rCRT321232: [ubsan] Diagnose noreturn functions which return 
(compiler-rt) (authored by vedantk, committed by ).
Herald added subscribers: Sanitizers, llvm-commits.

Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D40700

Files:
  lib/ubsan/ubsan_handlers.cc
  test/ubsan/TestCases/Misc/Inputs/returns-unexpectedly.c
  test/ubsan/TestCases/Misc/unreachable.cpp


Index: test/ubsan/TestCases/Misc/unreachable.cpp
===
--- test/ubsan/TestCases/Misc/unreachable.cpp
+++ test/ubsan/TestCases/Misc/unreachable.cpp
@@ -1,6 +1,25 @@
-// RUN: %clangxx -fsanitize=unreachable %s -O3 -o %t && not %run %t 2>&1 | 
FileCheck %s
+// RUN: %clang %S/Inputs/returns-unexpectedly.c -O3 -c -o %t.ru.o
+// RUN: %clangxx -fsanitize=unreachable -O3 -o %t %s %t.ru.o
+// RUN: not %run %t builtin 2>&1 | FileCheck %s -check-prefix=BUILTIN
+// RUN: not %run %t noreturn-callee-marked 2>&1 | FileCheck %s 
-check-prefix=NORETURN1
+// RUN: not %run %t noreturn-caller-marked 2>&1 | FileCheck %s 
-check-prefix=NORETURN2
+
+#include 
+
+void __attribute__((noreturn)) callee_marked_noreturn() {
+  // NORETURN1: unreachable.cpp:[[@LINE+1]]:1: runtime error: execution 
reached an unreachable program point
+}
+
+extern "C" void __attribute__((noreturn)) returns_unexpectedly();
 
 int main(int, char **argv) {
-  // CHECK: unreachable.cpp:5:3: runtime error: execution reached a 
__builtin_unreachable() call
-  __builtin_unreachable();
+  if (strcmp(argv[1], "builtin") == 0)
+// BUILTIN: unreachable.cpp:[[@LINE+1]]:5: runtime error: execution 
reached an unreachable program point
+__builtin_unreachable();
+  else if (strcmp(argv[1], "noreturn-callee-marked") == 0)
+callee_marked_noreturn();
+  else if (strcmp(argv[1], "noreturn-caller-marked") == 0)
+// NORETURN2: unreachable.cpp:[[@LINE+1]]:5: runtime error: execution 
reached an unreachable program point
+returns_unexpectedly();
+  return 0;
 }
Index: test/ubsan/TestCases/Misc/Inputs/returns-unexpectedly.c
===
--- test/ubsan/TestCases/Misc/Inputs/returns-unexpectedly.c
+++ test/ubsan/TestCases/Misc/Inputs/returns-unexpectedly.c
@@ -0,0 +1 @@
+void returns_unexpectedly() {}
Index: lib/ubsan/ubsan_handlers.cc
===
--- lib/ubsan/ubsan_handlers.cc
+++ lib/ubsan/ubsan_handlers.cc
@@ -297,7 +297,7 @@
 static void handleBuiltinUnreachableImpl(UnreachableData *Data,
  ReportOptions Opts) {
   ScopedReport R(Opts, Data->Loc, ErrorType::UnreachableCall);
-  Diag(Data->Loc, DL_Error, "execution reached a __builtin_unreachable() 
call");
+  Diag(Data->Loc, DL_Error, "execution reached an unreachable program point");
 }
 
 void __ubsan::__ubsan_handle_builtin_unreachable(UnreachableData *Data) {


Index: test/ubsan/TestCases/Misc/unreachable.cpp
===
--- test/ubsan/TestCases/Misc/unreachable.cpp
+++ test/ubsan/TestCases/Misc/unreachable.cpp
@@ -1,6 +1,25 @@
-// RUN: %clangxx -fsanitize=unreachable %s -O3 -o %t && not %run %t 2>&1 | FileCheck %s
+// RUN: %clang %S/Inputs/returns-unexpectedly.c -O3 -c -o %t.ru.o
+// RUN: %clangxx -fsanitize=unreachable -O3 -o %t %s %t.ru.o
+// RUN: not %run %t builtin 2>&1 | FileCheck %s -check-prefix=BUILTIN
+// RUN: not %run %t noreturn-callee-marked 2>&1 | FileCheck %s -check-prefix=NORETURN1
+// RUN: not %run %t noreturn-caller-marked 2>&1 | FileCheck %s -check-prefix=NORETURN2
+
+#include 
+
+void __attribute__((noreturn)) callee_marked_noreturn() {
+  // NORETURN1: unreachable.cpp:[[@LINE+1]]:1: runtime error: execution reached an unreachable program point
+}
+
+extern "C" void __attribute__((noreturn)) returns_unexpectedly();
 
 int main(int, char **argv) {
-  // CHECK: unreachable.cpp:5:3: runtime error: execution reached a __builtin_unreachable() call
-  __builtin_unreachable();
+  if (strcmp(argv[1], "builtin") == 0)
+// BUILTIN: unreachable.cpp:[[@LINE+1]]:5: runtime error: execution reached an unreachable program point
+__builtin_unreachable();
+  else if (strcmp(argv[1], "noreturn-callee-marked") == 0)
+callee_marked_noreturn();
+  else if (strcmp(argv[1], "noreturn-caller-marked") == 0)
+// NORETURN2: unreachable.cpp:[[@LINE+1]]:5: runtime error: execution reached an unreachable program point
+returns_unexpectedly();
+  return 0;
 }
Index: test/ubsan/TestCases/Misc/Inputs/returns-unexpectedly.c
===
--- test/ubsan/TestCases/Misc/Inputs/returns-unexpectedly.c
+++ test/ubsan/TestCases/Misc/Inputs/returns-unexpectedly.c
@@ -0,0 +1 @@
+void returns_unexpectedly() {}
Index: lib/ubsan/ubsan_handlers.cc

[PATCH] D40700: [ubsan] Diagnose noreturn functions which return (compiler-rt)

2017-12-20 Thread Phabricator via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL321232: [ubsan] Diagnose noreturn functions which return 
(compiler-rt) (authored by vedantk, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D40700?vs=125220=127796#toc

Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D40700

Files:
  compiler-rt/trunk/lib/ubsan/ubsan_handlers.cc
  compiler-rt/trunk/test/ubsan/TestCases/Misc/Inputs/returns-unexpectedly.c
  compiler-rt/trunk/test/ubsan/TestCases/Misc/unreachable.cpp


Index: compiler-rt/trunk/lib/ubsan/ubsan_handlers.cc
===
--- compiler-rt/trunk/lib/ubsan/ubsan_handlers.cc
+++ compiler-rt/trunk/lib/ubsan/ubsan_handlers.cc
@@ -297,7 +297,7 @@
 static void handleBuiltinUnreachableImpl(UnreachableData *Data,
  ReportOptions Opts) {
   ScopedReport R(Opts, Data->Loc, ErrorType::UnreachableCall);
-  Diag(Data->Loc, DL_Error, "execution reached a __builtin_unreachable() 
call");
+  Diag(Data->Loc, DL_Error, "execution reached an unreachable program point");
 }
 
 void __ubsan::__ubsan_handle_builtin_unreachable(UnreachableData *Data) {
Index: compiler-rt/trunk/test/ubsan/TestCases/Misc/Inputs/returns-unexpectedly.c
===
--- compiler-rt/trunk/test/ubsan/TestCases/Misc/Inputs/returns-unexpectedly.c
+++ compiler-rt/trunk/test/ubsan/TestCases/Misc/Inputs/returns-unexpectedly.c
@@ -0,0 +1 @@
+void returns_unexpectedly() {}
Index: compiler-rt/trunk/test/ubsan/TestCases/Misc/unreachable.cpp
===
--- compiler-rt/trunk/test/ubsan/TestCases/Misc/unreachable.cpp
+++ compiler-rt/trunk/test/ubsan/TestCases/Misc/unreachable.cpp
@@ -1,6 +1,25 @@
-// RUN: %clangxx -fsanitize=unreachable %s -O3 -o %t && not %run %t 2>&1 | 
FileCheck %s
+// RUN: %clang %S/Inputs/returns-unexpectedly.c -O3 -c -o %t.ru.o
+// RUN: %clangxx -fsanitize=unreachable -O3 -o %t %s %t.ru.o
+// RUN: not %run %t builtin 2>&1 | FileCheck %s -check-prefix=BUILTIN
+// RUN: not %run %t noreturn-callee-marked 2>&1 | FileCheck %s 
-check-prefix=NORETURN1
+// RUN: not %run %t noreturn-caller-marked 2>&1 | FileCheck %s 
-check-prefix=NORETURN2
+
+#include 
+
+void __attribute__((noreturn)) callee_marked_noreturn() {
+  // NORETURN1: unreachable.cpp:[[@LINE+1]]:1: runtime error: execution 
reached an unreachable program point
+}
+
+extern "C" void __attribute__((noreturn)) returns_unexpectedly();
 
 int main(int, char **argv) {
-  // CHECK: unreachable.cpp:5:3: runtime error: execution reached a 
__builtin_unreachable() call
-  __builtin_unreachable();
+  if (strcmp(argv[1], "builtin") == 0)
+// BUILTIN: unreachable.cpp:[[@LINE+1]]:5: runtime error: execution 
reached an unreachable program point
+__builtin_unreachable();
+  else if (strcmp(argv[1], "noreturn-callee-marked") == 0)
+callee_marked_noreturn();
+  else if (strcmp(argv[1], "noreturn-caller-marked") == 0)
+// NORETURN2: unreachable.cpp:[[@LINE+1]]:5: runtime error: execution 
reached an unreachable program point
+returns_unexpectedly();
+  return 0;
 }


Index: compiler-rt/trunk/lib/ubsan/ubsan_handlers.cc
===
--- compiler-rt/trunk/lib/ubsan/ubsan_handlers.cc
+++ compiler-rt/trunk/lib/ubsan/ubsan_handlers.cc
@@ -297,7 +297,7 @@
 static void handleBuiltinUnreachableImpl(UnreachableData *Data,
  ReportOptions Opts) {
   ScopedReport R(Opts, Data->Loc, ErrorType::UnreachableCall);
-  Diag(Data->Loc, DL_Error, "execution reached a __builtin_unreachable() call");
+  Diag(Data->Loc, DL_Error, "execution reached an unreachable program point");
 }
 
 void __ubsan::__ubsan_handle_builtin_unreachable(UnreachableData *Data) {
Index: compiler-rt/trunk/test/ubsan/TestCases/Misc/Inputs/returns-unexpectedly.c
===
--- compiler-rt/trunk/test/ubsan/TestCases/Misc/Inputs/returns-unexpectedly.c
+++ compiler-rt/trunk/test/ubsan/TestCases/Misc/Inputs/returns-unexpectedly.c
@@ -0,0 +1 @@
+void returns_unexpectedly() {}
Index: compiler-rt/trunk/test/ubsan/TestCases/Misc/unreachable.cpp
===
--- compiler-rt/trunk/test/ubsan/TestCases/Misc/unreachable.cpp
+++ compiler-rt/trunk/test/ubsan/TestCases/Misc/unreachable.cpp
@@ -1,6 +1,25 @@
-// RUN: %clangxx -fsanitize=unreachable %s -O3 -o %t && not %run %t 2>&1 | FileCheck %s
+// RUN: %clang %S/Inputs/returns-unexpectedly.c -O3 -c -o %t.ru.o
+// RUN: %clangxx -fsanitize=unreachable -O3 -o %t %s %t.ru.o
+// RUN: not %run %t builtin 2>&1 | FileCheck %s -check-prefix=BUILTIN
+// RUN: not %run %t noreturn-callee-marked 2>&1 | FileCheck %s 

[PATCH] D40698: [ubsan] Diagnose noreturn functions which return

2017-12-20 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC321231: [ubsan] Diagnose noreturn functions which return 
(authored by vedantk, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D40698

Files:
  docs/UndefinedBehaviorSanitizer.rst
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGExprCXX.cpp
  lib/CodeGen/CodeGenFunction.h
  test/CodeGen/ubsan-noreturn.c
  test/CodeGenCXX/ubsan-unreachable.cpp

Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -1432,14 +1432,7 @@
   case Builtin::BI__debugbreak:
 return RValue::get(EmitTrapCall(Intrinsic::debugtrap));
   case Builtin::BI__builtin_unreachable: {
-if (SanOpts.has(SanitizerKind::Unreachable)) {
-  SanitizerScope SanScope(this);
-  EmitCheck(std::make_pair(static_cast(Builder.getFalse()),
-   SanitizerKind::Unreachable),
-SanitizerHandler::BuiltinUnreachable,
-EmitCheckSourceLocation(E->getExprLoc()), None);
-} else
-  Builder.CreateUnreachable();
+EmitUnreachable(E->getExprLoc());
 
 // We do need to preserve an insertion point.
 EmitBlock(createBasicBlock("unreachable.cont"));
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -3288,11 +3288,15 @@
   /// LLVM arguments and the types they were derived from.
   RValue EmitCall(const CGFunctionInfo , const CGCallee ,
   ReturnValueSlot ReturnValue, const CallArgList ,
-  llvm::Instruction **callOrInvoke = nullptr);
-
+  llvm::Instruction **callOrInvoke, SourceLocation Loc);
+  RValue EmitCall(const CGFunctionInfo , const CGCallee ,
+  ReturnValueSlot ReturnValue, const CallArgList ,
+  llvm::Instruction **callOrInvoke = nullptr) {
+return EmitCall(CallInfo, Callee, ReturnValue, Args, callOrInvoke,
+SourceLocation());
+  }
   RValue EmitCall(QualType FnType, const CGCallee , const CallExpr *E,
-  ReturnValueSlot ReturnValue,
-  llvm::Value *Chain = nullptr);
+  ReturnValueSlot ReturnValue, llvm::Value *Chain = nullptr);
   RValue EmitCallExpr(const CallExpr *E,
   ReturnValueSlot ReturnValue = ReturnValueSlot());
   RValue EmitSimpleCallExpr(const CallExpr *E, ReturnValueSlot ReturnValue);
@@ -3747,6 +3751,10 @@
 llvm::ConstantInt *TypeId, llvm::Value *Ptr,
 ArrayRef StaticArgs);
 
+  /// Emit a reached-unreachable diagnostic if \p Loc is valid and runtime
+  /// checking is enabled. Otherwise, just emit an unreachable instruction.
+  void EmitUnreachable(SourceLocation Loc);
+
   /// \brief Create a basic block that will call the trap intrinsic, and emit a
   /// conditional branch to it, for the -ftrapv checks.
   void EmitTrapCheck(llvm::Value *Checked);
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -3076,6 +3076,17 @@
   CGM.addUsedGlobal(F);
 }
 
+void CodeGenFunction::EmitUnreachable(SourceLocation Loc) {
+  if (SanOpts.has(SanitizerKind::Unreachable)) {
+SanitizerScope SanScope(this);
+EmitCheck(std::make_pair(static_cast(Builder.getFalse()),
+ SanitizerKind::Unreachable),
+  SanitizerHandler::BuiltinUnreachable,
+  EmitCheckSourceLocation(Loc), None);
+  }
+  Builder.CreateUnreachable();
+}
+
 void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked) {
   llvm::BasicBlock *Cont = createBasicBlock("cont");
 
@@ -4616,7 +4627,7 @@
 Callee.setFunctionPointer(CalleePtr);
   }
 
-  return EmitCall(FnInfo, Callee, ReturnValue, Args);
+  return EmitCall(FnInfo, Callee, ReturnValue, Args, nullptr, E->getExprLoc());
 }
 
 LValue CodeGenFunction::
Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -2758,6 +2758,12 @@
 void CodeGenFunction::EmitFunctionEpilog(const CGFunctionInfo ,
  bool EmitRetDbgLoc,
  SourceLocation EndLoc) {
+  if (FI.isNoReturn()) {
+// Noreturn functions don't return.
+EmitUnreachable(EndLoc);
+return;
+  }
+
   if (CurCodeDecl && CurCodeDecl->hasAttr()) {
 // Naked functions don't have epilogues.
 Builder.CreateUnreachable();
@@ -3718,7 +3724,8 @@
  const CGCallee ,
  ReturnValueSlot ReturnValue,
  const CallArgList ,
- llvm::Instruction 

r321230 - [Driver] Ensure no overlap between trapping & recoverable sanitizers. NFC.

2017-12-20 Thread Vedant Kumar via cfe-commits
Author: vedantk
Date: Wed Dec 20 16:10:24 2017
New Revision: 321230

URL: http://llvm.org/viewvc/llvm-project?rev=321230=rev
Log:
[Driver] Ensure no overlap between trapping & recoverable sanitizers. NFC.

This is NFC because in EmitCheck(), -fsanitize-trap=X overrides
-fsanitize-recover=X.

Modified:
cfe/trunk/lib/Driver/SanitizerArgs.cpp
cfe/trunk/test/Driver/fsanitize.c

Modified: cfe/trunk/lib/Driver/SanitizerArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/SanitizerArgs.cpp?rev=321230=321229=321230=diff
==
--- cfe/trunk/lib/Driver/SanitizerArgs.cpp (original)
+++ cfe/trunk/lib/Driver/SanitizerArgs.cpp Wed Dec 20 16:10:24 2017
@@ -440,6 +440,7 @@ SanitizerArgs::SanitizerArgs(const ToolC
   RecoverableKinds &= ~Unrecoverable;
 
   TrappingKinds &= Kinds;
+  RecoverableKinds &= ~TrappingKinds;
 
   // Setup blacklist files.
   // Add default blacklist from resource directory.
@@ -683,6 +684,8 @@ SanitizerArgs::SanitizerArgs(const ToolC
   Sanitizers.Mask |= Kinds;
   RecoverableSanitizers.Mask |= RecoverableKinds;
   TrapSanitizers.Mask |= TrappingKinds;
+  assert(!(RecoverableKinds & TrappingKinds) &&
+ "Overlap between recoverable and trapping sanitizers");
 }
 
 static std::string toString(const clang::SanitizerSet ) {

Modified: cfe/trunk/test/Driver/fsanitize.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/fsanitize.c?rev=321230=321229=321230=diff
==
--- cfe/trunk/test/Driver/fsanitize.c (original)
+++ cfe/trunk/test/Driver/fsanitize.c Wed Dec 20 16:10:24 2017
@@ -3,6 +3,7 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=undefined 
-fsanitize-undefined-trap-on-error %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-UNDEFINED-TRAP
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=undefined-trap 
-fsanitize-undefined-trap-on-error %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-UNDEFINED-TRAP
 // RUN: %clang -target x86_64-linux-gnu -fsanitize-undefined-trap-on-error 
-fsanitize=undefined-trap %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-UNDEFINED-TRAP
+// CHECK-UNDEFINED-TRAP-NOT: -fsanitize-recover
 // CHECK-UNDEFINED-TRAP: 
"-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|shift-base|shift-exponent|unreachable|return|vla-bound|alignment|null|pointer-overflow|float-cast-overflow|array-bounds|enum|bool|builtin|returns-nonnull-attribute|nonnull-attribute|function),?){19}"}}
 // CHECK-UNDEFINED-TRAP: 
"-fsanitize-trap=alignment,array-bounds,bool,builtin,enum,float-cast-overflow,float-divide-by-zero,function,integer-divide-by-zero,nonnull-attribute,null,pointer-overflow,return,returns-nonnull-attribute,shift-base,shift-exponent,signed-integer-overflow,unreachable,vla-bound"
 // CHECK-UNDEFINED-TRAP2: 
"-fsanitize-trap=alignment,array-bounds,bool,builtin,enum,float-cast-overflow,float-divide-by-zero,function,integer-divide-by-zero,nonnull-attribute,null,pointer-overflow,return,returns-nonnull-attribute,shift-base,shift-exponent,unreachable,vla-bound"


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


r321231 - [ubsan] Diagnose noreturn functions which return

2017-12-20 Thread Vedant Kumar via cfe-commits
Author: vedantk
Date: Wed Dec 20 16:10:25 2017
New Revision: 321231

URL: http://llvm.org/viewvc/llvm-project?rev=321231=rev
Log:
[ubsan] Diagnose noreturn functions which return

Diagnose 'unreachable' UB when a noreturn function returns.

  1. Insert a check at the end of functions marked noreturn.

  2. A decl may be marked noreturn in the caller TU, but not marked in
 the TU where it's defined. To diagnose this scenario, strip away the
 noreturn attribute on the callee and insert check after calls to it.

Testing: check-clang, check-ubsan, check-ubsan-minimal, D40700

rdar://33660464

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

Added:
cfe/trunk/test/CodeGen/ubsan-noreturn.c
cfe/trunk/test/CodeGenCXX/ubsan-unreachable.cpp
Modified:
cfe/trunk/docs/UndefinedBehaviorSanitizer.rst
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/CodeGen/CGExprCXX.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h

Modified: cfe/trunk/docs/UndefinedBehaviorSanitizer.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/UndefinedBehaviorSanitizer.rst?rev=321231=321230=321231=diff
==
--- cfe/trunk/docs/UndefinedBehaviorSanitizer.rst (original)
+++ cfe/trunk/docs/UndefinedBehaviorSanitizer.rst Wed Dec 20 16:10:25 2017
@@ -124,8 +124,8 @@ Available checks are:
   -  ``-fsanitize=signed-integer-overflow``: Signed integer overflow,
  including all the checks added by ``-ftrapv``, and checking for
  overflow in signed division (``INT_MIN / -1``).
-  -  ``-fsanitize=unreachable``: If control flow reaches
- ``__builtin_unreachable``.
+  -  ``-fsanitize=unreachable``: If control flow reaches an unreachable
+ program point.
   -  ``-fsanitize=unsigned-integer-overflow``: Unsigned integer
  overflows. Note that unlike signed integer overflow, unsigned integer
  is not undefined behavior. However, while it has well-defined semantics,

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=321231=321230=321231=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Wed Dec 20 16:10:25 2017
@@ -1432,14 +1432,7 @@ RValue CodeGenFunction::EmitBuiltinExpr(
   case Builtin::BI__debugbreak:
 return RValue::get(EmitTrapCall(Intrinsic::debugtrap));
   case Builtin::BI__builtin_unreachable: {
-if (SanOpts.has(SanitizerKind::Unreachable)) {
-  SanitizerScope SanScope(this);
-  EmitCheck(std::make_pair(static_cast(Builder.getFalse()),
-   SanitizerKind::Unreachable),
-SanitizerHandler::BuiltinUnreachable,
-EmitCheckSourceLocation(E->getExprLoc()), None);
-} else
-  Builder.CreateUnreachable();
+EmitUnreachable(E->getExprLoc());
 
 // We do need to preserve an insertion point.
 EmitBlock(createBasicBlock("unreachable.cont"));

Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=321231=321230=321231=diff
==
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Wed Dec 20 16:10:25 2017
@@ -2758,6 +2758,12 @@ static llvm::StoreInst *findDominatingSt
 void CodeGenFunction::EmitFunctionEpilog(const CGFunctionInfo ,
  bool EmitRetDbgLoc,
  SourceLocation EndLoc) {
+  if (FI.isNoReturn()) {
+// Noreturn functions don't return.
+EmitUnreachable(EndLoc);
+return;
+  }
+
   if (CurCodeDecl && CurCodeDecl->hasAttr()) {
 // Naked functions don't have epilogues.
 Builder.CreateUnreachable();
@@ -3718,7 +3724,8 @@ RValue CodeGenFunction::EmitCall(const C
  const CGCallee ,
  ReturnValueSlot ReturnValue,
  const CallArgList ,
- llvm::Instruction **callOrInvoke) {
+ llvm::Instruction **callOrInvoke,
+ SourceLocation Loc) {
   // FIXME: We no longer need the types from CallArgs; lift up and simplify.
 
   assert(Callee.isOrdinary());
@@ -4241,7 +4248,15 @@ RValue CodeGenFunction::EmitCall(const C
   EmitLifetimeEnd(llvm::ConstantInt::get(Int64Ty, UnusedReturnSize),
   SRetPtr.getPointer());
 
-Builder.CreateUnreachable();
+// Strip away the noreturn attribute to better diagnose unreachable UB.
+if (SanOpts.has(SanitizerKind::Unreachable)) {
+  if (auto *F = CS.getCalledFunction())
+F->removeFnAttr(llvm::Attribute::NoReturn);
+  

r321228 - Silence a -Wreorder warning from r321223.

2017-12-20 Thread Aaron Ballman via cfe-commits
Author: aaronballman
Date: Wed Dec 20 15:11:05 2017
New Revision: 321228

URL: http://llvm.org/viewvc/llvm-project?rev=321228=rev
Log:
Silence a -Wreorder warning from r321223.

Modified:
cfe/trunk/lib/AST/ASTDumper.cpp

Modified: cfe/trunk/lib/AST/ASTDumper.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDumper.cpp?rev=321228=321227=321228=diff
==
--- cfe/trunk/lib/AST/ASTDumper.cpp (original)
+++ cfe/trunk/lib/AST/ASTDumper.cpp Wed Dec 20 15:11:05 2017
@@ -219,8 +219,8 @@ namespace  {
 ASTDumper(raw_ostream , const CommandTraits *Traits,
   const SourceManager *SM, bool ShowColors,
   const PrintingPolicy )
-: OS(OS), Traits(Traits), SM(SM), ShowColors(ShowColors),
-  PrintPolicy(PrintPolicy) {}
+: OS(OS), Traits(Traits), SM(SM), PrintPolicy(PrintPolicy),
+  ShowColors(ShowColors) {}
 
 void setDeserialize(bool D) { Deserialize = D; }
 


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


[PATCH] D41044: Implementation of -fextend-lifetimes and -fextend-this-ptr to aid with debugging of optimized code

2017-12-20 Thread Vedant Kumar via Phabricator via cfe-commits
vsk added a comment.

I have some results from the development build of our kernel ('-O2 -g -flto'). 
According to dwarfdump -statistics, when compiled with -fextend-lifetimes, the 
percentage of covered scope bytes increases from 62% to 69%. The number of 
inlined scopes decreases by 4%, and (I think relatedly) the size of the binary 
increases by 14%. There is a small increase in the number of unique source 
variables (under 1%). I'd be happy to report back on any other suggested 
quantitative measures.

My qualitative feedback based on spot-checking a few frames is  that 
-fextend-lifetimes noticeably improves the overall debugging experience. More 
argument values and local variables tend to be available. I'm not sure how best 
to put a number to this.


https://reviews.llvm.org/D41044



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


[PATCH] D41458: WIP: [libc++][C++17] Elementary string conversions for integral types

2017-12-20 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment.

I've got an implementation for this, too - at 
https://github.com/mclow/snippets/blob/master/to_chars.cpp

I'll compare them.


Repository:
  rCXX libc++

https://reviews.llvm.org/D41458



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


[PATCH] D40698: [ubsan] Diagnose noreturn functions which return

2017-12-20 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D40698



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


[clang-tools-extra] r321226 - [clangd-fuzzer] Update ClangdLSPServer constructor call.

2017-12-20 Thread Matt Morehouse via cfe-commits
Author: morehouse
Date: Wed Dec 20 14:29:23 2017
New Revision: 321226

URL: http://llvm.org/viewvc/llvm-project?rev=321226=rev
Log:
[clangd-fuzzer] Update ClangdLSPServer constructor call.

Build was broken by r321092.

Modified:
clang-tools-extra/trunk/clangd/fuzzer/ClangdFuzzer.cpp

Modified: clang-tools-extra/trunk/clangd/fuzzer/ClangdFuzzer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/fuzzer/ClangdFuzzer.cpp?rev=321226=321225=321226=diff
==
--- clang-tools-extra/trunk/clangd/fuzzer/ClangdFuzzer.cpp (original)
+++ clang-tools-extra/trunk/clangd/fuzzer/ClangdFuzzer.cpp Wed Dec 20 14:29:23 
2017
@@ -25,7 +25,8 @@ extern "C" int LLVMFuzzerTestOneInput(ui
   // Initialize and run ClangdLSPServer.
   clang::clangd::ClangdLSPServer LSPServer(
   Out, clang::clangd::getDefaultAsyncThreadsCount(),
-  /*StorePreamblesInMemory=*/false, CCOpts, llvm::None, llvm::None);
+  /*StorePreamblesInMemory=*/false, CCOpts, llvm::None, llvm::None,
+  /*BuildDynamicSymbolIndex=*/false);
 
   std::istringstream In(std::string(reinterpret_cast(data), size));
   LSPServer.run(In);


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


[PATCH] D40448: Add a printing policy for AST dumping

2017-12-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman closed this revision.
aaron.ballman marked an inline comment as done.
aaron.ballman added a comment.

Thanks, Hal! Committed in r321223.




Comment at: lib/AST/ASTDumper.cpp:218
+: ASTDumper(OS, Traits, SM, ShowColors, LangOptions()) {}
+//ASTDumper(raw_ostream , const CommandTraits *Traits,
+//  const SourceManager *SM, const PrintingPolicy )

hfinkel wrote:
> Remove commented-out code.
Sorry about that -- I spotted it when my rebase failed and have removed it.


https://reviews.llvm.org/D40448



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


[PATCH] D40448: Add a printing policy for AST dumping

2017-12-20 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel accepted this revision.
hfinkel added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D40448#961398, @aaron.ballman wrote:

> P-p-p-power ping! :-)


LGTM




Comment at: lib/AST/ASTDumper.cpp:218
+: ASTDumper(OS, Traits, SM, ShowColors, LangOptions()) {}
+//ASTDumper(raw_ostream , const CommandTraits *Traits,
+//  const SourceManager *SM, const PrintingPolicy )

Remove commented-out code.


https://reviews.llvm.org/D40448



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


[PATCH] D41345: [clangd] Add more symbol information for code completion.

2017-12-20 Thread Eric Liu via Phabricator via cfe-commits
ioeric added inline comments.



Comment at: clangd/index/Index.h:105
+  /// What to insert when completing this symbol (plain text version).
+  std::string CompletionPlainInsertText;
+  /// What to insert when completing this symbol (snippet version).

sammccall wrote:
> insert texts can be in details I think? They're not required for completion 
> until after resolve.
Quote from 
https://github.com/Microsoft/language-server-protocol/blob/master/protocol.md#completion-request
```
The request can delay the computation od the detail and documentation 
properties. However, properties that are needed for the inital sorting and 
filtering, like sortText, filterText, insertText, and textEdit must be provided 
in the textDocument/completion request and must not be changed during resolve.
```



Comment at: clangd/index/Index.h:122
+
+  llvm::Optional Detail;
+

sammccall wrote:
> I think you probably want a raw pointer rather than optional:
>  - reduce the size of the struct when it's absent
>  - make it inheritance-friendly so we can hang index-specific info off it
> (raw pointer rather than unique_ptr because it's owned by a slab not by 
> malloc, but unique_ptr is ok for now)
> 
This is not easy for now with `unique_ptr` because of this line :( 
https://github.com/llvm-mirror/clang-tools-extra/blob/3565d1a1a692fc9f5c21e634b470535da2bb4d25/clangd/index/SymbolYAML.cpp#L141).
 

This shouldn't be an issue when we have the optimized symbol slab, where we 
store raw pointers. And we would probably want to serialize the whole slab 
instead of the individual symbols anyway.

> reduce the size of the struct when it's absent
`llvm::Optional` doesn't take much more space, so the size should be fine.

> make it inheritance-friendly so we can hang index-specific info off it
Could you elaborate on `index-specific info`? It's unclear to me how this would 
be used.



Comment at: clangd/index/SymbolCollector.cpp:65
+  CodeCompletionResult SymbolCompletion(ND, 0);
+  auto Allocator = std::make_shared();
+  CodeCompletionTUInfo TUInfo(Allocator);

sammccall wrote:
> this doesn't seem like the kind of thing we should be allocating per-symbol!
> You can use a single one and Reset() it, I think
> 
> also why globalcodecompletionallocator, vs just codecompletionallocator?
> You can use a single one and Reset() it, I think
It's unclear how reset would affect `CodeCompletionTUInfo`, but we can simply 
maintain one allocator for each TU.

> also why globalcodecompletionallocator, vs just codecompletionallocator?
They are actually the same thing, but `CodeCompletionTUInfo` requires a global 
one. I don't really know why...


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41345



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


[PATCH] D41345: [clangd] Add more symbol information for code completion.

2017-12-20 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 127781.
ioeric marked an inline comment as done.
ioeric added a comment.

- Merged with origin/master
- Addressed some more comments.
- Add new fields to YAML.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41345

Files:
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/CodeComplete.cpp
  clangd/index/FileIndex.cpp
  clangd/index/Index.h
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h
  clangd/index/SymbolYAML.cpp
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/SymbolCollectorTests.cpp

Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -9,7 +9,6 @@
 
 #include "index/SymbolCollector.h"
 #include "index/SymbolYAML.h"
-#include "clang/Index/IndexingAction.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/FileSystemOptions.h"
 #include "clang/Basic/VirtualFileSystem.h"
@@ -26,11 +25,25 @@
 #include 
 #include 
 
+using testing::AllOf;
 using testing::Eq;
 using testing::Field;
 using testing::UnorderedElementsAre;
 
 // GMock helpers for matching Symbol.
+MATCHER_P(Labeled, Label, "") { return arg.second.CompletionLabel == Label; }
+MATCHER_P(Detail, D, "") {
+  return arg.second.Detail && arg.second.Detail->CompletionDetail == D;
+}
+MATCHER_P(Doc, D, "") {
+  return arg.second.Detail && arg.second.Detail->Documentation == D;
+}
+MATCHER_P(Plain, Text, "") {
+  return arg.second.CompletionPlainInsertText == Text;
+}
+MATCHER_P(Snippet, S, "") {
+  return arg.second.CompletionSnippetInsertText == S;
+}
 MATCHER_P(QName, Name, "") {
   return (arg.second.Scope + (arg.second.Scope.empty() ? "" : "::") +
   arg.second.Name) == Name;
@@ -110,6 +123,38 @@
 QName("f1"), QName("f2")));
 }
 
+TEST_F(SymbolCollectorTest, SymbolWithDocumentation) {
+  const std::string Main = R"(
+namespace nx {
+/// Foo comment.
+int ff(int x, double y) { return 0; }
+}
+  )";
+  runSymbolCollector(/*Header=*/"", Main);
+  EXPECT_THAT(Symbols,
+  UnorderedElementsAre(QName("nx"),
+   AllOf(QName("nx::ff"),
+ Labeled("ff(int x, double y)"),
+ Detail("int"), Doc("Foo comment.";
+}
+
+TEST_F(SymbolCollectorTest, PlainAndSnippet) {
+  const std::string Main = R"(
+namespace nx {
+void f() {}
+int ff(int x, double y) { return 0; }
+}
+  )";
+  runSymbolCollector(/*Header=*/"", Main);
+  EXPECT_THAT(
+  Symbols,
+  UnorderedElementsAre(
+  QName("nx"),
+  AllOf(QName("nx::f"), Labeled("f()"), Plain("f"), Snippet("f()")),
+  AllOf(QName("nx::ff"), Labeled("ff(int x, double y)"), Plain("ff"),
+Snippet("ff(${1:int x}, ${2:double y})";
+}
+
 TEST_F(SymbolCollectorTest, YAMLConversions) {
   const std::string YAML1 = R"(
 ---
@@ -123,6 +168,13 @@
   StartOffset: 0
   EndOffset:   1
   FilePath:/path/foo.h
+CompletionLabel:'Foo1-label'
+CompletionFilterText:'filter'
+CompletionPlainInsertText:'plain'
+CompletionSnippetInsertText:'snippet'
+Detail:
+  Documentation:'Foo doc'
+  CompletionDetail:'int'
 ...
 )";
   const std::string YAML2 = R"(
@@ -137,15 +189,22 @@
   StartOffset: 10
   EndOffset:   12
   FilePath:/path/foo.h
+CompletionLabel:'Foo2-label'
+CompletionFilterText:'filter'
+CompletionPlainInsertText:'plain'
+CompletionSnippetInsertText:'snippet'
 ...
 )";
 
   auto Symbols1 = SymbolFromYAML(YAML1);
-  EXPECT_THAT(Symbols1,
-  UnorderedElementsAre(QName("clang::Foo1")));
+  EXPECT_THAT(Symbols1, UnorderedElementsAre(
+AllOf(QName("clang::Foo1"), Labeled("Foo1-label"),
+  Doc("Foo doc"), Detail("int";
+  EXPECT_TRUE(Symbols1.begin()->second.Detail);
   auto Symbols2 = SymbolFromYAML(YAML2);
-  EXPECT_THAT(Symbols2,
-  UnorderedElementsAre(QName("clang::Foo2")));
+  EXPECT_THAT(Symbols2, UnorderedElementsAre(AllOf(QName("clang::Foo2"),
+   Labeled("Foo2-label";
+  EXPECT_FALSE(Symbols2.begin()->second.Detail);
 
   std::string ConcatenatedYAML =
   SymbolToYAML(Symbols1) + SymbolToYAML(Symbols2);
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -67,6 +67,8 @@
 MATCHER_P(Labeled, Label, "") { return arg.label == Label; }
 MATCHER_P(Kind, K, "") { return arg.kind == K; }
 MATCHER_P(Filter, F, "") { return arg.filterText == F; }
+MATCHER_P(Doc, D, "") { return arg.documentation == D; }
+MATCHER_P(Detail, D, "") { return arg.detail == D; }
 

[PATCH] D41455: [ASTMatchers] Add isNoReturn() match narrower for FunctionDeclarations

2017-12-20 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri updated this revision to Diff 127780.
lebedev.ri marked 2 inline comments as done.
lebedev.ri added a comment.

- Added C tests
- Cleaned-up spurious semicolons
- Docs are still not regenerated, somehow that script results in a huge diff 
for me.


Repository:
  rC Clang

https://reviews.llvm.org/D41455

Files:
  include/clang/ASTMatchers/ASTMatchers.h
  lib/ASTMatchers/Dynamic/Registry.cpp
  unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1770,6 +1770,84 @@
 functionDecl(isExplicitTemplateSpecialization(;
 }
 
+TEST(TypeMatching, MatchesNoReturn) {
+  EXPECT_TRUE(notMatches("void func();", functionDecl(isNoReturn(;
+  EXPECT_TRUE(notMatches("void func() {}", functionDecl(isNoReturn(;
+
+  EXPECT_TRUE(notMatchesC("void func();", functionDecl(isNoReturn(;
+  EXPECT_TRUE(notMatchesC("void func() {}", functionDecl(isNoReturn(;
+
+  EXPECT_TRUE(
+  notMatches("struct S { void func(); };", functionDecl(isNoReturn(;
+  EXPECT_TRUE(
+  notMatches("struct S { void func() {} };", functionDecl(isNoReturn(;
+
+  EXPECT_TRUE(notMatches("struct S { static void func(); };",
+ functionDecl(isNoReturn(;
+  EXPECT_TRUE(notMatches("struct S { static void func() {} };",
+ functionDecl(isNoReturn(;
+
+  EXPECT_TRUE(notMatches("struct S { S(); };", functionDecl(isNoReturn(;
+  EXPECT_TRUE(notMatches("struct S { S() {} };", functionDecl(isNoReturn(;
+
+  // ---
+
+  EXPECT_TRUE(matches("[[noreturn]] void func();", functionDecl(isNoReturn(;
+  EXPECT_TRUE(
+  matches("[[noreturn]] void func() {}", functionDecl(isNoReturn(;
+
+  EXPECT_TRUE(matches("struct S { [[noreturn]] void func(); };",
+  functionDecl(isNoReturn(;
+  EXPECT_TRUE(matches("struct S { [[noreturn]] void func() {} };",
+  functionDecl(isNoReturn(;
+
+  EXPECT_TRUE(matches("struct S { [[noreturn]] static void func(); };",
+  functionDecl(isNoReturn(;
+  EXPECT_TRUE(matches("struct S { [[noreturn]] static void func() {} };",
+  functionDecl(isNoReturn(;
+
+  EXPECT_TRUE(
+  matches("struct S { [[noreturn]] S(); };", functionDecl(isNoReturn(;
+  EXPECT_TRUE(matches("struct S { [[noreturn]] S() {} };",
+  functionDecl(isNoReturn(;
+
+  // ---
+
+  EXPECT_TRUE(matches("__attribute__((noreturn)) void func();",
+  functionDecl(isNoReturn(;
+  EXPECT_TRUE(matches("__attribute__((noreturn)) void func() {}",
+  functionDecl(isNoReturn(;
+
+  EXPECT_TRUE(matches("struct S { __attribute__((noreturn)) void func(); };",
+  functionDecl(isNoReturn(;
+  EXPECT_TRUE(matches("struct S { __attribute__((noreturn)) void func() {} };",
+  functionDecl(isNoReturn(;
+
+  EXPECT_TRUE(
+  matches("struct S { __attribute__((noreturn)) static void func(); };",
+  functionDecl(isNoReturn(;
+  EXPECT_TRUE(
+  matches("struct S { __attribute__((noreturn)) static void func() {} };",
+  functionDecl(isNoReturn(;
+
+  EXPECT_TRUE(matches("struct S { __attribute__((noreturn)) S(); };",
+  functionDecl(isNoReturn(;
+  EXPECT_TRUE(matches("struct S { __attribute__((noreturn)) S() {} };",
+  functionDecl(isNoReturn(;
+
+  // ---
+
+  EXPECT_TRUE(matchesC("__attribute__((noreturn)) void func();",
+  functionDecl(isNoReturn(;
+  EXPECT_TRUE(matchesC("__attribute__((noreturn)) void func() {}",
+  functionDecl(isNoReturn(;
+
+  EXPECT_TRUE(matchesC("_Noreturn void func();",
+  functionDecl(isNoReturn(;
+  EXPECT_TRUE(matchesC("_Noreturn void func() {}",
+  functionDecl(isNoReturn(;
+}
+
 TEST(TypeMatching, MatchesBool) {
   EXPECT_TRUE(matches("struct S { bool func(); };",
   cxxMethodDecl(returns(booleanType();
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -354,6 +354,7 @@
   REGISTER_MATCHER(isMemberInitializer);
   REGISTER_MATCHER(isMoveAssignmentOperator);
   REGISTER_MATCHER(isMoveConstructor);
+  REGISTER_MATCHER(isNoReturn);
   REGISTER_MATCHER(isNoThrow);
   REGISTER_MATCHER(isOverride);
   REGISTER_MATCHER(isPrivate);
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -3567,6 +3567,42 @@
   return 

[PATCH] D18768: Refactoring attribute subject diagnostics

2017-12-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman abandoned this revision.
aaron.ballman added a comment.

Closing this via "abandon" so that Richard doesn't have to accept it in order 
for me to close it. It's already been committed.


https://reviews.llvm.org/D18768



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


[PATCH] D40448: Add a printing policy for AST dumping

2017-12-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

P-p-p-power ping! :-)


https://reviews.llvm.org/D40448



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


[PATCH] D41311: [CodeGen] Fix crash when a function taking transparent union is redeclared.

2017-12-20 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai updated this revision to Diff 127779.
vsapsai added a comment.

- Address review comment, compare ArgI type with Arg type. Update tests to 
account for coerced store when types don't match.


https://reviews.llvm.org/D41311

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGen/kr-func-promote.c
  clang/test/CodeGen/transparent-union-redecl.c
  clang/test/CodeGenCXX/microsoft-abi-multiple-nonvirtual-inheritance.cpp
  clang/test/CodeGenCXX/microsoft-abi-virtual-inheritance-vtordisps.cpp
  clang/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
  clang/test/Sema/transparent-union.c

Index: clang/test/Sema/transparent-union.c
===
--- clang/test/Sema/transparent-union.c
+++ clang/test/Sema/transparent-union.c
@@ -43,6 +43,35 @@
 void fvpp(TU); // expected-note{{previous declaration is here}}
 void fvpp(void **v) {} // expected-error{{conflicting types}}
 
+/* Test redeclaring a function taking a transparent_union arg more than twice.
+   Merging different declarations depends on their order, vary order too. */
+
+void f_triple0(TU tu) {}
+void f_triple0(int *); // expected-note{{previous declaration is here}}
+void f_triple0(float *f); // expected-error{{conflicting types}}
+
+void f_triple1(int *);
+void f_triple1(TU tu) {} // expected-note{{previous definition is here}}
+void f_triple1(float *f); // expected-error{{conflicting types}}
+
+void f_triple2(int *); // expected-note{{previous declaration is here}}
+void f_triple2(float *f); // expected-error{{conflicting types}}
+void f_triple2(TU tu) {}
+
+/* Test calling redeclared function taking a transparent_union arg. */
+
+void f_callee(TU);
+void f_callee(int *i) {} // expected-note{{passing argument to parameter 'i' here}}
+
+void caller(void) {
+  TU tu;
+  f_callee(tu); // expected-error{{passing 'TU' to parameter of incompatible type 'int *'}}
+
+  int *i;
+  f_callee(i);
+}
+
+
 /* FIXME: we'd like to just use an "int" here and align it differently
from the normal "int", but if we do so we lose the alignment
information from the typedef within the compiler. */
Index: clang/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
===
--- clang/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
+++ clang/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
@@ -115,9 +115,15 @@
 // B::foo gets 'this' cast to VBase* in ECX (i.e. this+8) so we
 // need to adjust 'this' before use.
 //
-// Store initial this:
+// Coerce this to correct type:
+// CHECK:   %[[THIS_STORE:.*]] = alloca %struct.B*
 // CHECK:   %[[THIS_ADDR:.*]] = alloca %struct.B*
-// CHECK:   store %struct.B* %{{.*}}, %struct.B** %[[THIS_ADDR]], align 4
+// CHECK:   %[[COERCE_VAL:.*]] = bitcast i8* %{{.*}} to %struct.B*
+// CHECK:   store %struct.B* %[[COERCE_VAL]], %struct.B** %[[THIS_STORE]], align 4
+//
+// Store initial this:
+// CHECK:   %[[THIS_INIT:.*]] = load %struct.B*, %struct.B** %[[THIS_STORE]]
+// CHECK:   store %struct.B* %[[THIS_INIT]], %struct.B** %[[THIS_ADDR]], align 4
 //
 // Reload and adjust the this parameter:
 // CHECK:   %[[THIS_RELOAD:.*]] = load %struct.B*, %struct.B** %[[THIS_ADDR]]
Index: clang/test/CodeGenCXX/microsoft-abi-virtual-inheritance-vtordisps.cpp
===
--- clang/test/CodeGenCXX/microsoft-abi-virtual-inheritance-vtordisps.cpp
+++ clang/test/CodeGenCXX/microsoft-abi-virtual-inheritance-vtordisps.cpp
@@ -25,6 +25,7 @@
 
 // CHECK-LABEL: define linkonce_odr x86_thiscallcc void @"\01?f@D@@$4PPPM@A@AEXXZ"
 // Note that the vtordisp is applied before really adjusting to D*.
+// CHECK: %[[COERCE_LOAD:.*]] = load %struct.D*, %struct.D** %{{.*}}
 // CHECK: %[[ECX:.*]] = load %struct.D*, %struct.D** %{{.*}}
 // CHECK: %[[ECX_i8:.*]] = bitcast %struct.D* %[[ECX]] to i8*
 // CHECK: %[[VTORDISP_PTR_i8:.*]] = getelementptr inbounds i8, i8* %[[ECX_i8]], i32 -4
@@ -36,6 +37,7 @@
 // CHECK: ret void
 
 // CHECK-LABEL: define linkonce_odr x86_thiscallcc void @"\01?f@D@@$4PPPI@3AEXXZ"
+// CHECK: %[[COERCE_LOAD:.*]] = load %struct.D*, %struct.D** %{{.*}}
 // CHECK: %[[ECX:.*]] = load %struct.D*, %struct.D** %{{.*}}
 // CHECK: %[[ECX_i8:.*]] = bitcast %struct.D* %[[ECX]] to i8*
 // CHECK: %[[VTORDISP_PTR_i8:.*]] = getelementptr inbounds i8, i8* %[[ECX_i8]], i32 -8
@@ -64,7 +66,8 @@
 
 G::G() {}  // Forces vftable emission.
 
-// CHECK-LABEL: define linkonce_odr x86_thiscallcc void @"\01?f@E@@$R4BA@M@PPPM@7AEXXZ"(i8*)
+// CHECK-LABEL: define linkonce_odr x86_thiscallcc void @"\01?f@E@@$R4BA@M@PPPM@7AEXXZ"(i8*
+// CHECK: %[[COERCE_LOAD:.*]] = load %struct.E*, %struct.E** %{{.*}}
 // CHECK: %[[ECX:.*]] = load %struct.E*, %struct.E** %{{.*}}
 // CHECK: %[[ECX_i8:.*]] = bitcast %struct.E* %[[ECX]] to i8*
 // CHECK: %[[VTORDISP_PTR_i8:.*]] = getelementptr inbounds i8, i8* %[[ECX_i8]], i32 -4
Index: 

[PATCH] D41405: Fix an assertion failure regression in isDesignatorAtObjectEnd for __builtin_object_size with incomplete array type in struct

2017-12-20 Thread Alex Lorenz via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
arphaman marked 2 inline comments as done.
Closed by commit rC321222: Fix an assertion failure regression in 
isDesignatorAtObjectEnd for (authored by arphaman, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D41405?vs=127438=127778#toc

Repository:
  rC Clang

https://reviews.llvm.org/D41405

Files:
  lib/AST/ExprConstant.cpp
  test/Sema/builtin-object-size.c


Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -7420,7 +7420,10 @@
 // If we don't know the array bound, conservatively assume we're looking at
 // the final array element.
 ++I;
-BaseType = BaseType->castAs()->getPointeeType();
+if (BaseType->isIncompleteArrayType())
+  BaseType = Ctx.getAsArrayType(BaseType)->getElementType();
+else
+  BaseType = BaseType->castAs()->getPointeeType();
   }
 
   for (unsigned E = LVal.Designator.Entries.size(); I != E; ++I) {
Index: test/Sema/builtin-object-size.c
===
--- test/Sema/builtin-object-size.c
+++ test/Sema/builtin-object-size.c
@@ -91,3 +91,22 @@
 
   return n;
 }
+
+typedef struct {
+  char string[512];
+} NestedArrayStruct;
+
+typedef struct {
+  int x;
+  NestedArrayStruct session[];
+} IncompleteArrayStruct;
+
+void rd36094951_IAS_builtin_object_size_assertion(IncompleteArrayStruct *p) {
+#define rd36094951_CHECK(mode) 
\
+  __builtin___strlcpy_chk(p->session[0].string, "ab", 2,   
\
+  __builtin_object_size(p->session[0].string, mode))
+  rd36094951_CHECK(0);
+  rd36094951_CHECK(1);
+  rd36094951_CHECK(2);
+  rd36094951_CHECK(3);
+}


Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -7420,7 +7420,10 @@
 // If we don't know the array bound, conservatively assume we're looking at
 // the final array element.
 ++I;
-BaseType = BaseType->castAs()->getPointeeType();
+if (BaseType->isIncompleteArrayType())
+  BaseType = Ctx.getAsArrayType(BaseType)->getElementType();
+else
+  BaseType = BaseType->castAs()->getPointeeType();
   }
 
   for (unsigned E = LVal.Designator.Entries.size(); I != E; ++I) {
Index: test/Sema/builtin-object-size.c
===
--- test/Sema/builtin-object-size.c
+++ test/Sema/builtin-object-size.c
@@ -91,3 +91,22 @@
 
   return n;
 }
+
+typedef struct {
+  char string[512];
+} NestedArrayStruct;
+
+typedef struct {
+  int x;
+  NestedArrayStruct session[];
+} IncompleteArrayStruct;
+
+void rd36094951_IAS_builtin_object_size_assertion(IncompleteArrayStruct *p) {
+#define rd36094951_CHECK(mode) \
+  __builtin___strlcpy_chk(p->session[0].string, "ab", 2,   \
+  __builtin_object_size(p->session[0].string, mode))
+  rd36094951_CHECK(0);
+  rd36094951_CHECK(1);
+  rd36094951_CHECK(2);
+  rd36094951_CHECK(3);
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r321222 - Fix an assertion failure regression in isDesignatorAtObjectEnd for

2017-12-20 Thread Alex Lorenz via cfe-commits
Author: arphaman
Date: Wed Dec 20 13:03:38 2017
New Revision: 321222

URL: http://llvm.org/viewvc/llvm-project?rev=321222=rev
Log:
Fix an assertion failure regression in isDesignatorAtObjectEnd for
__builtin_object_size with incomplete array type in struct

The commit r316245 introduced a regression that causes an assertion failure when
Clang tries to cast an IncompleteArrayType to a PointerType when evaluating
__builtin_object_size.

rdar://36094951

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

Modified:
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/test/Sema/builtin-object-size.c

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=321222=321221=321222=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Wed Dec 20 13:03:38 2017
@@ -7420,7 +7420,10 @@ static bool isDesignatorAtObjectEnd(cons
 // If we don't know the array bound, conservatively assume we're looking at
 // the final array element.
 ++I;
-BaseType = BaseType->castAs()->getPointeeType();
+if (BaseType->isIncompleteArrayType())
+  BaseType = Ctx.getAsArrayType(BaseType)->getElementType();
+else
+  BaseType = BaseType->castAs()->getPointeeType();
   }
 
   for (unsigned E = LVal.Designator.Entries.size(); I != E; ++I) {

Modified: cfe/trunk/test/Sema/builtin-object-size.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/builtin-object-size.c?rev=321222=321221=321222=diff
==
--- cfe/trunk/test/Sema/builtin-object-size.c (original)
+++ cfe/trunk/test/Sema/builtin-object-size.c Wed Dec 20 13:03:38 2017
@@ -91,3 +91,22 @@ int pr31843() {
 
   return n;
 }
+
+typedef struct {
+  char string[512];
+} NestedArrayStruct;
+
+typedef struct {
+  int x;
+  NestedArrayStruct session[];
+} IncompleteArrayStruct;
+
+void rd36094951_IAS_builtin_object_size_assertion(IncompleteArrayStruct *p) {
+#define rd36094951_CHECK(mode) 
\
+  __builtin___strlcpy_chk(p->session[0].string, "ab", 2,   
\
+  __builtin_object_size(p->session[0].string, mode))
+  rd36094951_CHECK(0);
+  rd36094951_CHECK(1);
+  rd36094951_CHECK(2);
+  rd36094951_CHECK(3);
+}


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


[PATCH] D41405: Fix an assertion failure regression in isDesignatorAtObjectEnd for __builtin_object_size with incomplete array type in struct

2017-12-20 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added a comment.

In https://reviews.llvm.org/D41405#960008, @george.burgess.iv wrote:

> LGTM assuming my nit gets addressed.
>
> Thank you!
>
> > Maybe someone who's more familiar with this builtin could point to the 
> > cause of this discrepancy
>
> Yeah, the documentation for this builtin is pretty sparse. My guess: clang 
> assumes that the array's size is unknown because it's both an array and at 
> the end of a struct. This exists because code will do clever things like
>
>   struct string {
> size_t len;
> char buf[1]; // actual string is accessed from here; `string` just gets 
> overallocated to hold it all.
>   };
>
>
> in FreeBSD-land, they also recommend overallocation with sockaddrs, which 
> have a 14-length trailing element: 
> https://www.freebsd.org/doc/en/books/developers-handbook/sockets-essential-functions.html
>
> ...So, the best compatible heuristic we've been able to settle on here, 
> sadly, is "are we touching the final element in a struct, and is that element 
> an array?" On the bright side, clang failing just means LLVM gets to try to 
> figure it out, so only some hope of getting a useful answer is lost. :)
>
> It's interesting that GCC tries to do better here, since AIUI it has a 
> similar heuristic meant to cope with code like the above.


Thanks!




Comment at: test/Sema/builtin-object-size.c:105
+void rd36094951_IAS_builtin_object_size_assertion(IncompleteArrayStruct* p) {
+   __builtin___strlcpy_chk (p->session[0].string, "ab", 2, 
__builtin_object_size(p->session[0].string, 1));
+}

george.burgess.iv wrote:
> vsapsai wrote:
> > Do we execute significantly different code paths when the second 
> > `__builtin_object_size` argument is 0, 2, 3? I think it is worth checking 
> > locally if these values aren't causing an assertion. Not sure about having 
> > such tests permanently, I'll leave it to you as you are more familiar with 
> > the code.
> In this case, only 1 and 3 should be touching the buggy codepath, and they 
> should execute it identically. If 0 and 2 reach there, we have bigger 
> problems, since they shouldn't really be poking around in the designator of 
> the given LValue.
> 
> Since it's presumably only ~10 seconds of copy-pasting, I'd be happy if we 
> added sanity checks for other modes, as well. :)
Sure, I'll test the other modes as well.


Repository:
  rC Clang

https://reviews.llvm.org/D41405



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


[PATCH] D39239: [AST] Incorrectly qualified unscoped enumeration as template actual parameter.

2017-12-20 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a project: debug-info.
probinson accepted this revision.
probinson added a comment.
This revision is now accepted and ready to land.

With the GDB test results and LLDB able to handle it, this LGTM.
Carlos, do you have commit access?


https://reviews.llvm.org/D39239



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


[PATCH] D23934: Add a -ffixed-date-time= flag that sets the initial value of __DATE__, __TIME__, __TIMESTAMP__

2017-12-20 Thread Ed Maste via Phabricator via cfe-commits
emaste added a comment.

Ping?


https://reviews.llvm.org/D23934



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


[PATCH] D41456: [clang-tidy] readability-else-after-return: also diagnose noreturn function calls.

2017-12-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang-tidy/readability/ElseAfterReturnCheck.cpp:43
+  for (const auto *BindingName :
+   {"return", "continue", "break", "throw", "noreturn call"})
 if (Result.Nodes.getNodeAs(BindingName))

You've spotted the downside to this clever construct -- it is hard to make the 
wording not terrible here. :-)

I think the diagnostic should read `do no use 'else' after a call to a function 
that does not return`. This is trickier because the current `%0` is always 
quoted.



Comment at: docs/ReleaseNotes.rst:189
 
+- The 'readability-else-after-return' check was adjusted to handle throw 
expressions that require cleanups, calls to functions marked with noreturn 
attribute.
+

This seems to be > 80 cols, so please reformat. Also, it should be "that 
require cleanups and calls to functions marked with the noreturn attribute" 
(fixes a few grammatical issues at once).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41456



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


[PATCH] D41455: [ASTMatchers] Add isNoReturn() match narrower for FunctionDeclarations

2017-12-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

You regenerate the HTML by running `clang/docs/tools/dump_ast_matchers.py`.




Comment at: unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp:1775
+  EXPECT_TRUE(notMatches("void func();", functionDecl(isNoReturn(;
+  EXPECT_TRUE(notMatches("void func() {};", functionDecl(isNoReturn(;
+

Spurious semicolon, which happens elsewhere in the tests as well -- please 
clean them all.



Comment at: unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp:1834
+  functionDecl(isNoReturn(;
+}
+

For fun, I'd like to see a `matchesC` test for `_Noreturn` as well, just to be 
extra sure. :-)


Repository:
  rC Clang

https://reviews.llvm.org/D41455



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


[PATCH] D41311: [CodeGen] Fix crash when a function taking transparent union is redeclared.

2017-12-20 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:2321
+  !isa(ConvertType(Arg->getType())) &&
   ArgI.getCoerceToType() == ConvertType(Ty) &&
   ArgI.getDirectOffset() == 0) {

vsapsai wrote:
> rjmccall wrote:
> > I think the right fix is to change the second clause to 
> > ArgI.getCoerceToType() == ConvertType(Arg->getType()).  The point of this 
> > special case is to catch the common case that the natural way that IRGen 
> > wants to emit the argument expression is by producing a single scalar of 
> > exactly the right IR type; it seems to me that that condition would capture 
> > that correctly.
> I've tried that and it almost works. There are 4 failing tests
> Clang :: CodeGen/kr-func-promote.c
> Clang :: CodeGenCXX/microsoft-abi-multiple-nonvirtual-inheritance.cpp
> Clang :: CodeGenCXX/microsoft-abi-virtual-inheritance-vtordisps.cpp
> Clang :: CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
> 
> In a couple of places the problem is that expected function `i32 @a(i32)` but 
> received `i32 @a(i32 %x.coerce)`.
> 
> As for me, parameter naming is not really a problem and can be fixed by 
> making tests less specific. What is more interesting, with the change we 
> started adding extra store and load. For example, for 
> CodeGen/kr-func-promote.c IR was
> 
> ```
> define i32 @a(i32) #0 {
>   %x.addr = alloca i16, align 2
>   %x = trunc i32 %0 to i16
>   store i16 %x, i16* %x.addr, align 2
>   %2 = load i16, i16* %x.addr, align 2
>   %conv = sext i16 %2 to i32
>   ret i32 %conv
> }
> ```
> 
> and with type comparison change IR becomes
> ```
> define i32 @a(i32 %x.coerce) #0 {
> entry:
>   %x = alloca i16, align 2
>   %x.addr = alloca i16, align 2
>   %coerce.val.ii = trunc i32 %x.coerce to i16
>   store i16 %coerce.val.ii, i16* %x, align 2
>   %x1 = load i16, i16* %x, align 2
>   store i16 %x1, i16* %x.addr, align 2
>   %0 = load i16, i16* %x.addr, align 2
>   %conv = sext i16 %0 to i32
>   ret i32 %conv
> }
> ```
> 
> The same situation is with "microsoft-abi-*" tests where instead of
> ```
>   %this = bitcast i8* %0 to %struct.D*
>   store %struct.D* %this, %struct.D** %this.addr, align 4
>   %this1 = load %struct.D*, %struct.D** %this.addr, align 4
>   %2 = bitcast %struct.D* %this1 to i8*
> ```
> we have
> ```
>   %coerce.val = bitcast i8* %this.coerce to %struct.D*
>   store %struct.D* %coerce.val, %struct.D** %this, align 4
>   %this1 = load %struct.D*, %struct.D** %this, align 4
>   store %struct.D* %this1, %struct.D** %this.addr, align 4
>   %this2 = load %struct.D*, %struct.D** %this.addr, align 4
>   %0 = bitcast %struct.D* %this2 to i8*
> ```
> 
> I'll check where and why we are adding extra store/load.
The last store comes from `CodeGenFunction::EmitParmDecl` where for direct 
arguments we have
```lang=c++
  if (Arg.isIndirect()) {
//...
  } else {
// Otherwise, create a temporary to hold the value.
DeclPtr = CreateMemTemp(Ty, getContext().getDeclAlign(),
D.getName() + ".addr");
DoStore = true;
  }
```
We have this store before and after my change, and it doesn't depend on 
comparing `ArgI.getCoerceToType()` to other types.

The extra store+load comes from `CreateCoercedStore` and `EmitLoadOfScalar` a 
few lines lower. I've tried to avoid this store+load for cases where we can use 
only `emitArgumentDemotion` and `CreateBitCast`. But it doesn't look to be 
particularly simple and starts to look like incomplete version of "trivial 
case, handle it with no muss and fuss".

Knowing the cause of the extra store+load I don't think it is crucial to avoid 
it. I'll update the patch with `ArgI.getCoerceToType() == 
ConvertType(Arg->getType())` comparison and fixed tests.



Comment at: clang/lib/CodeGen/CGCall.cpp:2461
 AI->setName(Arg->getName() + ".coerce");
 CreateCoercedStore(AI, Ptr, /*DestIsVolatile=*/false, *this);
   }

Here is where we add the extra store.


https://reviews.llvm.org/D41311



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


[PATCH] D40295: -fsanitize=vptr warnings on bad static types in dynamic_cast and typeid

2017-12-20 Thread Vedant Kumar via Phabricator via cfe-commits
vsk added reviewers: vsk, thakis.
vsk accepted this revision.
vsk added a comment.
This revision is now accepted and ready to land.

Thanks, lgtm. Could you wait a day or two before committing? IIRC Richard or 
Nico have a -fsanitize=vptr Chromium bot, and they may have further comments.


https://reviews.llvm.org/D40295



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


[PATCH] D41405: Fix an assertion failure regression in isDesignatorAtObjectEnd for __builtin_object_size with incomplete array type in struct

2017-12-20 Thread George Burgess IV via Phabricator via cfe-commits
george.burgess.iv added inline comments.



Comment at: test/Sema/builtin-object-size.c:105
+void rd36094951_IAS_builtin_object_size_assertion(IncompleteArrayStruct* p) {
+   __builtin___strlcpy_chk (p->session[0].string, "ab", 2, 
__builtin_object_size(p->session[0].string, 1));
+}

vsapsai wrote:
> Do we execute significantly different code paths when the second 
> `__builtin_object_size` argument is 0, 2, 3? I think it is worth checking 
> locally if these values aren't causing an assertion. Not sure about having 
> such tests permanently, I'll leave it to you as you are more familiar with 
> the code.
In this case, only 1 and 3 should be touching the buggy codepath, and they 
should execute it identically. If 0 and 2 reach there, we have bigger problems, 
since they shouldn't really be poking around in the designator of the given 
LValue.

Since it's presumably only ~10 seconds of copy-pasting, I'd be happy if we 
added sanity checks for other modes, as well. :)


Repository:
  rC Clang

https://reviews.llvm.org/D41405



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


[PATCH] D41406: [analyzer] Add a new checker callback, check::NewAllocator.

2017-12-20 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

In https://reviews.llvm.org/D41406#960824, @xazax.hun wrote:

> Maybe `debug.AnalysisOrder` could be used to test the callback order 
> explicitly. This way the test could also serve as a documentation for the 
> callback order.


Yep, totally, will do.


https://reviews.llvm.org/D41406



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


[PATCH] D41408: [analyzer] NFC: Fix nothrow operator new definition in a test.

2017-12-20 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: test/Analysis/NewDelete-custom.cpp:7
 
-#if !LEAKS
+#if !(LEAKS && !ALLOCATOR_INLINING)
 // expected-no-diagnostics

a.sidorin wrote:
> Double negation can be simplified a bit: `#if !LEAKS || ALLOCATOR_INLINING`
The rest of the `#if`s in this file look like `#if LEAKS && 
!ALLOCATOR_INLINING`, so i wanted this to look similarly.


Repository:
  rC Clang

https://reviews.llvm.org/D41408



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


[PATCH] D41458: WIP: [libc++][C++17] Elementary string conversions for integral types

2017-12-20 Thread Zhihao Yuan via Phabricator via cfe-commits
lichray created this revision.
lichray added reviewers: mclow.lists, EricWF.
Herald added a subscriber: mgorny.

Progress: std::to_chars for integers
Missing: std::from_chars

References:
 https://wg21.link/p0067r5
 https://wg21.link/p0682r1


Repository:
  rCXX libc++

https://reviews.llvm.org/D41458

Files:
  .gitignore
  include/charconv
  include/support/itoa/
  include/support/itoa/itoa.h
  lib/CMakeLists.txt
  src/support/itoa/
  src/support/itoa/itoa.cpp
  test/std/utilities/charconv/
  test/std/utilities/charconv/charconv.to.chars/
  test/std/utilities/charconv/charconv.to.chars/integral.bool.fail.cpp
  test/std/utilities/charconv/charconv.to.chars/integral.pass.cpp

Index: test/std/utilities/charconv/charconv.to.chars/integral.pass.cpp
===
--- /dev/null
+++ test/std/utilities/charconv/charconv.to.chars/integral.pass.cpp
@@ -0,0 +1,87 @@
+//===--===//
+//
+// 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
+// 
+
+// to_chars_result to_chars(char* first, char* last, Integral value,
+//  int base = 10)
+
+#include "charconv_test_helpers.h"
+
+template 
+struct test_basics : to_chars_test_base
+{
+using to_chars_test_base::test;
+using to_chars_test_base::test_value;
+
+void operator()()
+{
+
+test(0, "0");
+test(42, "42");
+test(32768, "32768");
+test(0, "0", 10);
+test(42, "42", 10);
+test(32768, "32768", 10);
+test(0xf, "f", 16);
+test(0xdeadbeaf, "deadbeaf", 16);
+test(0755, "755", 8);
+
+for (int b = 2; b < 37; ++b)
+{
+using xl = std::numeric_limits;
+
+test_value(1, b);
+test_value(xl::lowest(), b);
+test_value((xl::max)(), b);
+test_value((xl::max)() / 2, b);
+}
+}
+};
+
+template 
+struct test_signed : to_chars_test_base
+{
+using to_chars_test_base::test;
+using to_chars_test_base::test_value;
+
+void operator()()
+{
+test(-1, "-1");
+test(-12, "-12");
+test(-1, "-1", 10);
+test(-12, "-12", 10);
+test(-21734634, "-21734634", 10);
+test(-2647, "-101001010111", 2);
+test(-0xcc1, "-cc1", 16);
+
+for (int b = 2; b < 37; ++b)
+{
+using xl = std::numeric_limits;
+
+test_value(0, b);
+test_value(xl::lowest(), b);
+test_value((xl::max)(), b);
+}
+}
+};
+
+int
+main()
+{
+auto all_signed =
+type_list();
+auto all_unsigned = type_list();
+auto integrals = concat(all_signed, all_unsigned);
+
+run(integrals);
+run(all_signed);
+}
Index: test/std/utilities/charconv/charconv.to.chars/integral.bool.fail.cpp
===
--- /dev/null
+++ test/std/utilities/charconv/charconv.to.chars/integral.bool.fail.cpp
@@ -0,0 +1,30 @@
+//===--===//
+//
+// 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
+// 
+
+// In
+//
+// to_chars_result to_chars(char* first, char* last, Integral value,
+//  int base = 10)
+//
+// Integral cannot be bool.
+
+#include 
+
+int main()
+{
+using std::to_chars;
+char buf[10];
+bool lv = true;
+
+to_chars(buf, buf + sizeof(buf), false);   // expected-error {{call to deleted function}}
+to_chars(buf, buf + sizeof(buf), lv, 16);  // expected-error {{call to deleted function}}
+}
Index: src/support/itoa/itoa.cpp
===
--- /dev/null
+++ src/support/itoa/itoa.cpp
@@ -0,0 +1,296 @@
+// Tencent is pleased to support the open source community by making RapidJSON available.
+//
+// Copyright (C) 2015 THL A29 Limited, a Tencent company, and Milo Yip. All rights reserved.
+//
+// Licensed under the MIT License (the "License"); you may not use this file except
+// in compliance with the License. You may obtain a copy of the License at
+//
+// http://opensource.org/licenses/MIT
+//
+// Unless required by applicable law or agreed to in writing, software distributed
+// under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR
+// CONDITIONS OF ANY KIND, either express or implied. See the License for the
+// 

[PATCH] D41417: [hwasan] Implement -fsanitize-recover=hwaddress.

2017-12-20 Thread Evgenii Stepanov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC321203: [hwasan] Implement -fsanitize-recover=hwaddress. 
(authored by eugenis, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D41417?vs=127642=127760#toc

Repository:
  rC Clang

https://reviews.llvm.org/D41417

Files:
  lib/CodeGen/BackendUtil.cpp


Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -239,7 +239,11 @@
 
 static void addHWAddressSanitizerPasses(const PassManagerBuilder ,
 legacy::PassManagerBase ) {
-  PM.add(createHWAddressSanitizerPass());
+  const PassManagerBuilderWrapper  =
+  static_cast(Builder);
+  const CodeGenOptions  = BuilderWrapper.getCGOpts();
+  bool Recover = CGOpts.SanitizeRecover.has(SanitizerKind::HWAddress);
+  PM.add(createHWAddressSanitizerPass(Recover));
 }
 
 static void addMemorySanitizerPass(const PassManagerBuilder ,


Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -239,7 +239,11 @@
 
 static void addHWAddressSanitizerPasses(const PassManagerBuilder ,
 legacy::PassManagerBase ) {
-  PM.add(createHWAddressSanitizerPass());
+  const PassManagerBuilderWrapper  =
+  static_cast(Builder);
+  const CodeGenOptions  = BuilderWrapper.getCGOpts();
+  bool Recover = CGOpts.SanitizeRecover.has(SanitizerKind::HWAddress);
+  PM.add(createHWAddressSanitizerPass(Recover));
 }
 
 static void addMemorySanitizerPass(const PassManagerBuilder ,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r321203 - [hwasan] Implement -fsanitize-recover=hwaddress.

2017-12-20 Thread Evgeniy Stepanov via cfe-commits
Author: eugenis
Date: Wed Dec 20 11:05:44 2017
New Revision: 321203

URL: http://llvm.org/viewvc/llvm-project?rev=321203=rev
Log:
[hwasan] Implement -fsanitize-recover=hwaddress.

Summary: Very similar to AddressSanitizer, with the exception of the error type 
encoding.

Reviewers: kcc, alekseyshl

Subscribers: cfe-commits, kubamracek, llvm-commits, hiraditya

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

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

Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=321203=321202=321203=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Wed Dec 20 11:05:44 2017
@@ -239,7 +239,11 @@ static void addKernelAddressSanitizerPas
 
 static void addHWAddressSanitizerPasses(const PassManagerBuilder ,
 legacy::PassManagerBase ) {
-  PM.add(createHWAddressSanitizerPass());
+  const PassManagerBuilderWrapper  =
+  static_cast(Builder);
+  const CodeGenOptions  = BuilderWrapper.getCGOpts();
+  bool Recover = CGOpts.SanitizeRecover.has(SanitizerKind::HWAddress);
+  PM.add(createHWAddressSanitizerPass(Recover));
 }
 
 static void addMemorySanitizerPass(const PassManagerBuilder ,


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


[PATCH] D41303: Add support for ObjectFormat to TargetSpecificAttr

2017-12-20 Thread Erich Keane via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC321201: Add support for ObjectFormat to TargetSpecificAttr 
(authored by erichkeane, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D41303?vs=127159=127757#toc

Repository:
  rC Clang

https://reviews.llvm.org/D41303

Files:
  include/clang/Basic/Attr.td
  lib/Sema/SemaDeclAttr.cpp
  test/Sema/attr-ifunc.c
  utils/TableGen/ClangAttrEmitter.cpp

Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -1844,12 +1844,6 @@
 S.Diag(Attr.getLoc(), diag::err_alias_is_definition) << FD << 1;
 return;
   }
-  // FIXME: it should be handled as a target specific attribute.
-  if (S.Context.getTargetInfo().getTriple().getObjectFormat() !=
-  llvm::Triple::ELF) {
-S.Diag(Attr.getLoc(), diag::warn_attribute_ignored) << Attr.getName();
-return;
-  }
 
   D->addAttr(::new (S.Context) IFuncAttr(Attr.getRange(), S.Context, Str,
  Attr.getAttributeSpellingListIndex()));
Index: utils/TableGen/ClangAttrEmitter.cpp
===
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -2622,6 +2622,31 @@
   OS << "  }\n";
 }
 
+// Helper function for GenerateTargetSpecificAttrChecks that alters the 'Test'
+// parameter with only a single check type, if applicable.
+static void GenerateTargetSpecificAttrCheck(const Record *R, std::string ,
+std::string *FnName,
+StringRef ListName,
+StringRef CheckAgainst,
+StringRef Scope) {
+  if (!R->isValueUnset(ListName)) {
+Test += " && (";
+std::vector Items = R->getValueAsListOfStrings(ListName);
+for (auto I = Items.begin(), E = Items.end(); I != E; ++I) {
+  StringRef Part = *I;
+  Test += CheckAgainst;
+  Test += " == ";
+  Test += Scope;
+  Test += Part;
+  if (I + 1 != E)
+Test += " || ";
+  if (FnName)
+*FnName += Part;
+}
+Test += ")";
+  }
+}
+
 // Generate a conditional expression to check if the current target satisfies
 // the conditions for a TargetSpecificAttr record, and append the code for
 // those checks to the Test string. If the FnName string pointer is non-null,
@@ -2635,29 +2660,15 @@
   // named "T" and a TargetInfo object named "Target" within
   // scope that can be used to determine whether the attribute exists in
   // a given target.
-  Test += "(";
-
-  for (auto I = Arches.begin(), E = Arches.end(); I != E; ++I) {
-StringRef Part = *I;
-Test += "T.getArch() == llvm::Triple::";
-Test += Part;
-if (I + 1 != E)
-  Test += " || ";
-if (FnName)
-  *FnName += Part;
-  }
-  Test += ")";
-
-  // If the attribute is specific to particular OSes, check those.
-  if (!R->isValueUnset("OSes")) {
-// We know that there was at least one arch test, so we need to and in the
-// OS tests.
+  Test += "true";
+  // If one or more architectures is specified, check those.  Arches are handled
+  // differently because GenerateTargetRequirements needs to combine the list
+  // with ParseKind.
+  if (!Arches.empty()) {
 Test += " && (";
-std::vector OSes = R->getValueAsListOfStrings("OSes");
-for (auto I = OSes.begin(), E = OSes.end(); I != E; ++I) {
+for (auto I = Arches.begin(), E = Arches.end(); I != E; ++I) {
   StringRef Part = *I;
-
-  Test += "T.getOS() == llvm::Triple::";
+  Test += "T.getArch() == llvm::Triple::";
   Test += Part;
   if (I + 1 != E)
 Test += " || ";
@@ -2667,21 +2678,17 @@
 Test += ")";
   }
 
+  // If the attribute is specific to particular OSes, check those.
+  GenerateTargetSpecificAttrCheck(R, Test, FnName, "OSes", "T.getOS()",
+  "llvm::Triple::");
+
   // If one or more CXX ABIs are specified, check those as well.
-  if (!R->isValueUnset("CXXABIs")) {
-Test += " && (";
-std::vector CXXABIs = R->getValueAsListOfStrings("CXXABIs");
-for (auto I = CXXABIs.begin(), E = CXXABIs.end(); I != E; ++I) {
-  StringRef Part = *I;
-  Test += "Target.getCXXABI().getKind() == TargetCXXABI::";
-  Test += Part;
-  if (I + 1 != E)
-Test += " || ";
-  if (FnName)
-*FnName += Part;
-}
-Test += ")";
-  }
+  GenerateTargetSpecificAttrCheck(R, Test, FnName, "CXXABIs",
+  "Target.getCXXABI().getKind()",
+  "TargetCXXABI::");
+  // If one or more object formats is specified, check those.
+  GenerateTargetSpecificAttrCheck(R, Test, FnName, "ObjectFormats",
+  "T.getObjectFormat()", "llvm::Triple::");
 }
 
 static void 

r321201 - Add support for ObjectFormat to TargetSpecificAttr

2017-12-20 Thread Erich Keane via cfe-commits
Author: erichkeane
Date: Wed Dec 20 10:51:08 2017
New Revision: 321201

URL: http://llvm.org/viewvc/llvm-project?rev=321201=rev
Log:
Add support for ObjectFormat to TargetSpecificAttr

Looking through the code, I saw a FIXME on IFunc to switch it
to a target specific attribute. In looking through it, i saw that
the no-longer-appropriately-named TargetArch didn't support ObjectFormat
checking.

This patch changes the name of TargetArch to TargetSpecific
(since it checks much more than just Arch), makes "Arch" optional, adds
support for ObjectFormat, better documents the TargetSpecific type, and
changes IFunc over to a TargetSpecificAttr.

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

Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/test/Sema/attr-ifunc.c
cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=321201=321200=321201=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Wed Dec 20 10:51:08 2017
@@ -267,13 +267,24 @@ def RenderScript : LangOpt<"RenderScript
 def ObjC : LangOpt<"ObjC1">;
 def BlocksSupported : LangOpt<"Blocks">;
 
-// Defines targets for target-specific attributes. The list of strings should
-// specify architectures for which the target applies, based off the ArchType
-// enumeration in Triple.h.
-class TargetArch {
-  list Arches = arches;
+// Defines targets for target-specific attributes. Empty lists are unchecked.
+class TargetSpec {
+  // Specifies Architectures for which the target applies, based off the
+  // ArchType enumeration in Triple.h.
+  list Arches = [];
+  // Specifies Operating Systems for which the target applies, based off the
+  // OSType enumeration in Triple.h
   list OSes;
+  // Specifies the C++ ABIs for which the target applies, based off the
+  // TargetCXXABI::Kind in TargetCXXABI.h.
   list CXXABIs;
+  // Specifies Object Formats for which the target applies, based off the
+  // ObjectFormatType enumeration in Triple.h
+  list ObjectFormats;
+}
+
+class TargetArch : TargetSpec {
+  let Arches = arches;
 }
 def TargetARM : TargetArch<["arm", "thumb", "armeb", "thumbeb"]>;
 def TargetAVR : TargetArch<["avr"]>;
@@ -288,6 +299,9 @@ def TargetWindows : TargetArch<["x86", "
 def TargetMicrosoftCXXABI : TargetArch<["x86", "x86_64", "arm", "thumb", 
"aarch64"]> {
   let CXXABIs = ["Microsoft"];
 }
+def TargetELF : TargetSpec {
+  let ObjectFormats = ["ELF"];
+}
 
 // Attribute subject match rules that are used for #pragma clang attribute.
 //
@@ -465,8 +479,8 @@ class InheritableAttr : Attr;
 
 /// A target-specific attribute.  This class is meant to be used as a mixin
 /// with InheritableAttr or Attr depending on the attribute's needs.
-class TargetSpecificAttr {
-  TargetArch Target = target;
+class TargetSpecificAttr {
+  TargetSpec Target = target;
   // Attributes are generally required to have unique spellings for their names
   // so that the parser can determine what kind of attribute it has parsed.
   // However, target-specific attributes are special in that the attribute only
@@ -1121,7 +1135,7 @@ def IBOutletCollection : InheritableAttr
   let Documentation = [Undocumented];
 }
 
-def IFunc : Attr {
+def IFunc : Attr, TargetSpecificAttr {
   let Spellings = [GCC<"ifunc">];
   let Args = [StringArgument<"Resolver">];
   let Subjects = SubjectList<[Function]>;

Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=321201=321200=321201=diff
==
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Wed Dec 20 10:51:08 2017
@@ -1844,12 +1844,6 @@ static void handleIFuncAttr(Sema , Dec
 S.Diag(Attr.getLoc(), diag::err_alias_is_definition) << FD << 1;
 return;
   }
-  // FIXME: it should be handled as a target specific attribute.
-  if (S.Context.getTargetInfo().getTriple().getObjectFormat() !=
-  llvm::Triple::ELF) {
-S.Diag(Attr.getLoc(), diag::warn_attribute_ignored) << Attr.getName();
-return;
-  }
 
   D->addAttr(::new (S.Context) IFuncAttr(Attr.getRange(), S.Context, Str,
  
Attr.getAttributeSpellingListIndex()));

Modified: cfe/trunk/test/Sema/attr-ifunc.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/attr-ifunc.c?rev=321201=321200=321201=diff
==
--- cfe/trunk/test/Sema/attr-ifunc.c (original)
+++ cfe/trunk/test/Sema/attr-ifunc.c Wed Dec 20 10:51:08 2017
@@ -5,7 +5,7 @@
 #if defined(_WIN32)
 void foo() {}
 void bar() __attribute__((ifunc("foo")));
-//expected-warning@-1 

[PATCH] D41363: [clang-tidy] Adding Fuchsia checker for overloaded operators

2017-12-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang-tidy/fuchsia/OverloadedOperatorCheck.cpp:18
+
+AST_MATCHER(CXXMethodDecl, hasOverloadedOperator) {
+  if (Node.isCopyAssignmentOperator() || Node.isMoveAssignmentOperator())

JonasToth wrote:
> I think `isOverloadedOperator` is a better name.
I think `isFuchsiaOverloadedOperator()` is even better because of the extra 
work done by the checker (otherwise, it might be tempting to put this into 
ASTMatchers.h).



Comment at: clang-tidy/fuchsia/OverloadedOperatorCheck.cpp:30
+  if (const auto *D = Result.Nodes.getNodeAs("decl"))
+diag(D->getLocStart(), "operator overloading is disallowed");
+}

I think this could be better stated as "cannot overload %0" and pass in `D` 
(which should expand to something reasonable).



Comment at: test/clang-tidy/fuchsia-overloaded-operator.cpp:11
+public:
+  B =(B other);
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:3: warning: operator overloading is 
disallowed [fuchsia-overloaded-operator]

While this is an overloaded assignment operator, it's grating for it not to 
accept a `const B&`.


https://reviews.llvm.org/D41363



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


[PATCH] D41303: Add support for ObjectFormat to TargetSpecificAttr

2017-12-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

Aside from some minor nits, LGTM. Thanks!




Comment at: include/clang/Basic/Attr.td:281
   list CXXABIs;
+  // Specifies Object Formats  for which the target applies, based off the
+  // ObjectFormatType enumeration in Triple.h

Extra space after "Formats"



Comment at: utils/TableGen/ClangAttrEmitter.cpp:2665
+  // If one or more architectures is specified, check those.  Arches are 
handled
+  // differently because GenerateTargetRequiresments needs to combine the list
+  // with ParseKind.

Typo: GenerateTargetRequiresments 


Repository:
  rC Clang

https://reviews.llvm.org/D41303



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


[PATCH] D38639: [clangd] #include statements support for Open definition

2017-12-20 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov requested changes to this revision.
ilya-biryukov added a comment.
This revision now requires changes to proceed.

Another round of review




Comment at: clangd/ClangdUnit.cpp:113
 
+  CppFilePreambleCallbacks(IncludeReferenceMap )
+  : IRM(IRM) {}

ilya-biryukov wrote:
> Let's create a new empty map inside this class and have a 
> `takeIncludeReferences` method (similar to `TopLevelDeclIDs` and 
> `takeTopLevelDeclIDs`)
This comment is not addressed yet, but marked as done.



Comment at: clangd/ClangdUnit.cpp:147
+  IncludeReferenceMap 
+  std::vector> 
TempPreambleIncludeLocations;
 };

Nebiroth wrote:
> ilya-biryukov wrote:
> > We should have `SourceMgr` at all the proper places now, let's store 
> > `IncludeReferenceMap` directly
> The idea was to pass a single map around to fill with every reference instead 
> of having separate maps being merged together.
You copy the map for preamble and then append to it in 
`CppFilePreambleCallbacks`? That also LG, we should not have many references 
there anyway.



Comment at: clangd/ClangdUnit.cpp:281
+ IntrusiveRefCntPtr VFS,
+ IncludeReferenceMap IRM) {
 

Nebiroth wrote:
> ilya-biryukov wrote:
> > What reference does this `IncludeReferenceMap` contain? Includes from the 
> > preamble? 
> This should contain every reference available for one file.
Thanks for clarifying, LG.



Comment at: clangd/ClangdUnit.cpp:141
+  std::unique_ptr createPPCallbacks() {
+return llvm::make_unique(*SourceMgr, IRM);
+  }

Maybe add `assert(SourceMgr && "SourceMgr must be set at this point")` here?



Comment at: clangd/ClangdUnit.cpp:281
+std::shared_ptr PCHs,
+IntrusiveRefCntPtr VFS, IncludeReferenceMap IRM) {
 

Don't we already store the map we need in `PreambleData`? Why do we need an 
extra `IRM` parameter?



Comment at: clangd/ClangdUnit.h:276
+std::vector
+findDefinitions(const Context , ParsedAST , Position Pos);
+

ilya-biryukov wrote:
> This function moved without need and lost a comment.
Now we have duplicated definitions here. Bad merge?



Comment at: clangd/CodeComplete.cpp:328
   unsigned NumResults) override final {
-if (auto SS = Context.getCXXScopeSpecifier())
-  CompletedName.SSInfo = extraCompletionScope(S, **SS);
+//if (auto SS = Context.getCXXScopeSpecifier())
+//  CompletedName.SSInfo = extraCompletionScope(S, **SS);

Accidental change?



Comment at: clangd/CodeComplete.cpp:807
   // Enable index-based code completion when Index is provided.
-  Result.IncludeNamespaceLevelDecls = !Index;
+  // Result.IncludeNamespaceLevelDecls = !Index;
 

Accidental change?



Comment at: clangd/XRefs.cpp:41
 
+  std::vector takeLocations() {
+// Don't keep the same location multiple times.

We don't need locations anymore. Remove them.



Comment at: clangd/XRefs.cpp:177
 
+  std::vector IRMResult;
+  if (!AST.getIRM().empty()) {

Probably a good place for a comment. Also, maybe rename local var to something 
easier to understand like `IncludeDefinitions`
```
/// Process targets for paths inside #include directive.
std::vector IncludeTargets;
```



Comment at: clangd/XRefs.cpp:178
+  std::vector IRMResult;
+  if (!AST.getIRM().empty()) {
+for (auto  : AST.getIRM()) {

No need to special case empty maps, remove the if altogether.



Comment at: clangd/XRefs.cpp:183
+  unsigned CharNumber = SourceMgr.getSpellingColumnNumber(
+  DeclMacrosFinder->getSearchedLocation());
+

Replace with `DeclMacrosFinder->getSearchedLocation()` `SourceLocationBeg`, it 
makes the code easier to read.



Comment at: clangd/XRefs.cpp:185
+
+  if ((unsigned)R.start.line ==
+  SourceMgr.getSpellingLineNumber(

why do we need to convert to unsigned? To slience compiler warnings?



Comment at: clangd/XRefs.cpp:188
+  DeclMacrosFinder->getSearchedLocation()) &&
+  ((unsigned)R.start.character >= CharNumber &&
+   CharNumber <= (unsigned)R.end.character)) {

this should be `R.start.charater <= CharNumber && CharNumber <= R.end.character`


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D38639



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


[PATCH] D41456: [clang-tidy] readability-else-after-return: also diagnose noreturn function calls.

2017-12-20 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

Depends on https://reviews.llvm.org/D41455.
I'm open to suggestions re diagnostic wording.
The current `'noreturn call'` seems lazy, and is basically a placeholder.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41456



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


[PATCH] D41456: [clang-tidy] readability-else-after-return: also diagnose noreturn function calls.

2017-12-20 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri created this revision.
lebedev.ri added reviewers: aaron.ballman, alexfh, djasper, malcolm.parsons.
lebedev.ri added a project: clang-tools-extra.
Herald added a subscriber: xazax.hun.

The readability-else-after-return check was not warning about  an else after a 
call to the function that will not return. In particular, marker with a 
noreturn attribute, be it either C++11 `[[noreturn]]`, or the 
`__attribute__((noreturn))`.
This differential adds support to diagnose normal function calls, and calls to 
member functions; but not constructors, destructors and special member 
functions.

A follow-up for https://reviews.llvm.org/D40505.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41456

Files:
  clang-tidy/readability/ElseAfterReturnCheck.cpp
  docs/ReleaseNotes.rst
  test/clang-tidy/readability-else-after-return.cpp

Index: test/clang-tidy/readability-else-after-return.cpp
===
--- test/clang-tidy/readability-else-after-return.cpp
+++ test/clang-tidy/readability-else-after-return.cpp
@@ -11,6 +11,28 @@
   my_exception(const std::string );
 };
 
+__attribute__((noreturn)) void my_noreturn();
+[[noreturn]] void my_noreturn2();
+[[noreturn]] __attribute__((noreturn)) void my_noreturn3();
+struct my_struct {
+  [[noreturn]] static void test();
+  [[noreturn]] void test_2();
+};
+struct my_assign {
+  my_assign();
+  [[noreturn]] my_assign& operator=(const my_assign& c);
+};
+struct my_copy {
+  my_copy();
+  [[noreturn]] my_copy(const my_copy& c);
+};
+struct noreturn_ctor {
+  [[noreturn]] noreturn_ctor();
+};
+struct noreturn_dtor {
+  [[noreturn]] noreturn_dtor();
+};
+
 void f(int a) {
   if (a > 0)
 return;
@@ -103,5 +125,69 @@
 // CHECK-FIXES: {{^}}} // comment-10
   x++;
 }
+if (x) {
+  my_noreturn();
+} else { // comment-11
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'noreturn call'
+// CHECK-FIXES: {{^}}} // comment-11
+  x++;
+}
+if (x) {
+  my_noreturn2();
+} else { // comment-12
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'noreturn call'
+// CHECK-FIXES: {{^}}} // comment-12
+  x++;
+}
+if (x) {
+  my_noreturn3();
+} else { // comment-13
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'noreturn call'
+// CHECK-FIXES: {{^}}} // comment-13
+  x++;
+}
+if (x) {
+  my_struct::test();
+} else { // comment-14
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'noreturn call'
+// CH1ECK-FIXES: {{^}}} // comment-14
+  x++;
+}
+if (x) {
+  my_struct s;
+  s.test_2();
+} else { // comment-15
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'noreturn call'
+// CHECK-FIXES: {{^}}} // comment-15
+  x++;
+}
+if (x) {
+  my_assign a0;
+  my_assign a1;
+  a1 = a0;
+} else { // comment-16
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use 'else' after 'noreturn call'
+// CHECK-FIXES: {{^}}} // comment-16
+  x++;
+}
+if (x) {
+  // FIXME
+  my_copy c0;
+  my_copy c1(c0);
+} else { // comment-17
+  x++;
+}
+if (x) {
+  // FIXME
+  noreturn_ctor n;
+} else { // comment-18
+  x++;
+}
+if (x) {
+  // FIXME
+  noreturn_dtor n;
+} else { // comment-19
+  x++;
+}
   }
 }
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -186,6 +186,8 @@
   Finds property declarations in Objective-C files that do not follow the
   pattern of property names in Apple's programming guide.
 
+- The 'readability-else-after-return' check was adjusted to handle throw expressions that require cleanups, calls to functions marked with noreturn attribute.
+
 - New `readability-static-accessed-through-instance
   `_ check
 
Index: clang-tidy/readability/ElseAfterReturnCheck.cpp
===
--- clang-tidy/readability/ElseAfterReturnCheck.cpp
+++ clang-tidy/readability/ElseAfterReturnCheck.cpp
@@ -22,7 +22,9 @@
   const auto ControlFlowInterruptorMatcher =
   stmt(anyOf(returnStmt().bind("return"), continueStmt().bind("continue"),
  breakStmt().bind("break"),
- expr(ignoringImplicit(cxxThrowExpr().bind("throw");
+ expr(ignoringImplicit(cxxThrowExpr().bind("throw"))),
+ callExpr(hasDeclaration(functionDecl(isNoReturn(
+ .bind("noreturn call")));
   Finder->addMatcher(
   compoundStmt(forEach(
   ifStmt(hasThen(stmt(
@@ -37,7 +39,8 @@
   const auto *If = Result.Nodes.getNodeAs("if");
   

[PATCH] D40705: [Parser] Diagnose storage classes in template parameter declarations

2017-12-20 Thread Faisal Vali via Phabricator via cfe-commits
faisalv added a comment.

Sounds good - if I don't get this done over the next seven days - would you 
mind just pinging me!

Thanks!


https://reviews.llvm.org/D40705



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


[PATCH] D41455: [ASTMatchers] Add isNoReturn() match narrower for FunctionDeclarations

2017-12-20 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri created this revision.
lebedev.ri added a reviewer: aaron.ballman.
lebedev.ri added a project: clang.
Herald added a subscriber: klimek.

How to regenerate LibASTMatchersReference.html ?


Repository:
  rC Clang

https://reviews.llvm.org/D41455

Files:
  include/clang/ASTMatchers/ASTMatchers.h
  lib/ASTMatchers/Dynamic/Registry.cpp
  unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1770,6 +1770,69 @@
 functionDecl(isExplicitTemplateSpecialization(;
 }
 
+TEST(TypeMatching, MatchesNoReturn) {
+  EXPECT_TRUE(notMatches("void func();", functionDecl(isNoReturn(;
+  EXPECT_TRUE(notMatches("void func() {};", functionDecl(isNoReturn(;
+
+  EXPECT_TRUE(
+  notMatches("struct S { void func(); };", functionDecl(isNoReturn(;
+  EXPECT_TRUE(
+  notMatches("struct S { void func() {}; };", functionDecl(isNoReturn(;
+
+  EXPECT_TRUE(notMatches("struct S { static void func(); };",
+ functionDecl(isNoReturn(;
+  EXPECT_TRUE(notMatches("struct S { static void func() {}; };",
+ functionDecl(isNoReturn(;
+
+  EXPECT_TRUE(notMatches("struct S { S(); };", functionDecl(isNoReturn(;
+  EXPECT_TRUE(notMatches("struct S { S() {}; };", functionDecl(isNoReturn(;
+
+  // ---
+
+  EXPECT_TRUE(matches("[[noreturn]] void func();", functionDecl(isNoReturn(;
+  EXPECT_TRUE(
+  matches("[[noreturn]] void func() {};", functionDecl(isNoReturn(;
+
+  EXPECT_TRUE(matches("struct S { [[noreturn]] void func(); };",
+  functionDecl(isNoReturn(;
+  EXPECT_TRUE(matches("struct S { [[noreturn]] void func() {}; };",
+  functionDecl(isNoReturn(;
+
+  EXPECT_TRUE(matches("struct S { [[noreturn]] static void func(); };",
+  functionDecl(isNoReturn(;
+  EXPECT_TRUE(matches("struct S { [[noreturn]] static void func() {}; };",
+  functionDecl(isNoReturn(;
+
+  EXPECT_TRUE(
+  matches("struct S { [[noreturn]] S(); };", functionDecl(isNoReturn(;
+  EXPECT_TRUE(matches("struct S { [[noreturn]] S() {}; };",
+  functionDecl(isNoReturn(;
+
+  // ---
+
+  EXPECT_TRUE(matches("__attribute__((noreturn)) void func();",
+  functionDecl(isNoReturn(;
+  EXPECT_TRUE(matches("__attribute__((noreturn)) void func() {};",
+  functionDecl(isNoReturn(;
+
+  EXPECT_TRUE(matches("struct S { __attribute__((noreturn)) void func(); };",
+  functionDecl(isNoReturn(;
+  EXPECT_TRUE(matches("struct S { __attribute__((noreturn)) void func() {}; };",
+  functionDecl(isNoReturn(;
+
+  EXPECT_TRUE(
+  matches("struct S { __attribute__((noreturn)) static void func(); };",
+  functionDecl(isNoReturn(;
+  EXPECT_TRUE(
+  matches("struct S { __attribute__((noreturn)) static void func() {}; };",
+  functionDecl(isNoReturn(;
+
+  EXPECT_TRUE(matches("struct S { __attribute__((noreturn)) S(); };",
+  functionDecl(isNoReturn(;
+  EXPECT_TRUE(matches("struct S { __attribute__((noreturn)) S() {}; };",
+  functionDecl(isNoReturn(;
+}
+
 TEST(TypeMatching, MatchesBool) {
   EXPECT_TRUE(matches("struct S { bool func(); };",
   cxxMethodDecl(returns(booleanType();
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -354,6 +354,7 @@
   REGISTER_MATCHER(isMemberInitializer);
   REGISTER_MATCHER(isMoveAssignmentOperator);
   REGISTER_MATCHER(isMoveConstructor);
+  REGISTER_MATCHER(isNoReturn);
   REGISTER_MATCHER(isNoThrow);
   REGISTER_MATCHER(isOverride);
   REGISTER_MATCHER(isPrivate);
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -3567,6 +3567,32 @@
   return Node.getNumParams() == N;
 }
 
+/// \brief Matches \c FunctionDecls that have a noreturn attribute.
+///
+/// Given
+/// \code
+///   void nope();
+///   [[noreturn]] void a0();
+///   [[noreturn]] void a1() {};
+///   __attribute__((noreturn)) void a2();
+///   __attribute__((noreturn)) void a3() {};
+///   struct b0 { [[noreturn]] b0(); };
+///   struct b1 { [[noreturn]] b1() {}; };
+///   struct b2 { __attribute__((noreturn)) b2(); };
+///   struct b3 { __attribute__((noreturn)) b3() {}; };
+///   struct c0 { [[noreturn]] int A(); };
+///   struct c1 { [[noreturn]] int A() {}; };
+///   struct c2 { __attribute__((noreturn)) int A(); };
+///   struct c3 { 

[PATCH] D41399: [CodeGen] Represent array members in new-format TBAA type descriptors

2017-12-20 Thread Ivan Kosarev via Phabricator via cfe-commits
kosarev updated this revision to Diff 127742.
kosarev added a comment.

- Fixed the access size.
- Added the suggested tests.


https://reviews.llvm.org/D41399

Files:
  lib/CodeGen/CodeGenTBAA.cpp
  test/CodeGen/tbaa-array.cpp


Index: test/CodeGen/tbaa-array.cpp
===
--- test/CodeGen/tbaa-array.cpp
+++ test/CodeGen/tbaa-array.cpp
@@ -1,18 +1,52 @@
 // RUN: %clang_cc1 -triple x86_64-linux -O1 -disable-llvm-passes %s \
 // RUN: -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux -O1 -disable-llvm-passes %s \
+// RUN: -new-struct-path-tbaa -emit-llvm -o - | \
+// RUN: FileCheck -check-prefix=CHECK-NEW %s
 //
 // Check that we generate correct TBAA information for accesses to array
 // elements.
 
 struct A { int i; };
 struct B { A a[1]; };
+struct C { int i; int x[3]; };
 
 int foo(B *b) {
 // CHECK-LABEL: _Z3fooP1B
 // CHECK: load i32, {{.*}}, !tbaa [[TAG_A_i:!.*]]
+// CHECK-NEW-LABEL: _Z3fooP1B
+// CHECK-NEW: load i32, {{.*}}, !tbaa [[TAG_A_i:!.*]]
   return b->a->i;
 }
 
+// Check that members of array types are represented correctly.
+int bar(C *c) {
+// CHECK-NEW-LABEL: _Z3barP1C
+// CHECK-NEW: load i32, {{.*}}, !tbaa [[TAG_C_i:!.*]]
+  return c->i;
+}
+
+int bar2(C *c) {
+// CHECK-NEW-LABEL: _Z4bar2P1C
+// CHECK-NEW: load i32, {{.*}}, !tbaa [[TAG_int:!.*]]
+  return c->x[2];
+}
+
+int bar3(C *c, int j) {
+// CHECK-NEW-LABEL: _Z4bar3P1Ci
+// CHECK-NEW: load i32, {{.*}}, !tbaa [[TAG_int:!.*]]
+  return c->x[j];
+}
+
 // CHECK-DAG: [[TAG_A_i]] = !{[[TYPE_A:!.*]], [[TYPE_int:!.*]], i64 0}
 // CHECK-DAG: [[TYPE_A]] = !{!"_ZTS1A", !{{.*}}, i64 0}
 // CHECK-DAG: [[TYPE_int]] = !{!"int", !{{.*}}, i64 0}
+
+// CHECK-NEW-DAG: [[TYPE_char:!.*]] = !{{{.*}}, i64 1, !"omnipotent char"}
+// CHECK-NEW-DAG: [[TYPE_int:!.*]] = !{[[TYPE_char]], i64 4, !"int"}
+// CHECK-NEW-DAG: [[TAG_int]] = !{[[TYPE_int]], [[TYPE_int]], i64 0, i64 4}
+// CHECK-NEW-DAG: [[TYPE_pointer:!.*]] = !{[[TYPE_char]], i64 8, !"any 
pointer"}
+// CHECK-NEW-DAG: [[TYPE_A:!.*]] = !{[[TYPE_char]], i64 4, !"_ZTS1A", 
[[TYPE_int]], i64 0, i64 4}
+// CHECK-NEW-DAG: [[TAG_A_i]] = !{[[TYPE_A]], [[TYPE_int]], i64 0, i64 4}
+// CHECK-NEW-DAG: [[TYPE_C:!.*]] = !{[[TYPE_char]], i64 16, !"_ZTS1C", 
[[TYPE_int]], i64 0, i64 4, [[TYPE_int]], i64 4, i64 12}
+// CHECK-NEW-DAG: [[TAG_C_i]] = !{[[TYPE_C:!.*]], [[TYPE_int:!.*]], i64 0, i64 
4}
Index: lib/CodeGen/CodeGenTBAA.cpp
===
--- lib/CodeGen/CodeGenTBAA.cpp
+++ lib/CodeGen/CodeGenTBAA.cpp
@@ -161,6 +161,10 @@
   if (Ty->isPointerType() || Ty->isReferenceType())
 return createScalarTypeNode("any pointer", getChar(), Size);
 
+  // Accesses to arrays are accesses to objects of their element types.
+  if (CodeGenOpts.NewStructPathTBAA && Ty->isArrayType())
+return getTypeInfo(cast(Ty)->getElementType());
+
   // Enum types are distinct types. In C++ they have "underlying types",
   // however they aren't related for TBAA.
   if (const EnumType *ETy = dyn_cast(Ty)) {


Index: test/CodeGen/tbaa-array.cpp
===
--- test/CodeGen/tbaa-array.cpp
+++ test/CodeGen/tbaa-array.cpp
@@ -1,18 +1,52 @@
 // RUN: %clang_cc1 -triple x86_64-linux -O1 -disable-llvm-passes %s \
 // RUN: -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux -O1 -disable-llvm-passes %s \
+// RUN: -new-struct-path-tbaa -emit-llvm -o - | \
+// RUN: FileCheck -check-prefix=CHECK-NEW %s
 //
 // Check that we generate correct TBAA information for accesses to array
 // elements.
 
 struct A { int i; };
 struct B { A a[1]; };
+struct C { int i; int x[3]; };
 
 int foo(B *b) {
 // CHECK-LABEL: _Z3fooP1B
 // CHECK: load i32, {{.*}}, !tbaa [[TAG_A_i:!.*]]
+// CHECK-NEW-LABEL: _Z3fooP1B
+// CHECK-NEW: load i32, {{.*}}, !tbaa [[TAG_A_i:!.*]]
   return b->a->i;
 }
 
+// Check that members of array types are represented correctly.
+int bar(C *c) {
+// CHECK-NEW-LABEL: _Z3barP1C
+// CHECK-NEW: load i32, {{.*}}, !tbaa [[TAG_C_i:!.*]]
+  return c->i;
+}
+
+int bar2(C *c) {
+// CHECK-NEW-LABEL: _Z4bar2P1C
+// CHECK-NEW: load i32, {{.*}}, !tbaa [[TAG_int:!.*]]
+  return c->x[2];
+}
+
+int bar3(C *c, int j) {
+// CHECK-NEW-LABEL: _Z4bar3P1Ci
+// CHECK-NEW: load i32, {{.*}}, !tbaa [[TAG_int:!.*]]
+  return c->x[j];
+}
+
 // CHECK-DAG: [[TAG_A_i]] = !{[[TYPE_A:!.*]], [[TYPE_int:!.*]], i64 0}
 // CHECK-DAG: [[TYPE_A]] = !{!"_ZTS1A", !{{.*}}, i64 0}
 // CHECK-DAG: [[TYPE_int]] = !{!"int", !{{.*}}, i64 0}
+
+// CHECK-NEW-DAG: [[TYPE_char:!.*]] = !{{{.*}}, i64 1, !"omnipotent char"}
+// CHECK-NEW-DAG: [[TYPE_int:!.*]] = !{[[TYPE_char]], i64 4, !"int"}
+// CHECK-NEW-DAG: [[TAG_int]] = !{[[TYPE_int]], [[TYPE_int]], i64 0, i64 4}
+// CHECK-NEW-DAG: [[TYPE_pointer:!.*]] = !{[[TYPE_char]], i64 8, !"any pointer"}
+// CHECK-NEW-DAG: [[TYPE_A:!.*]] = !{[[TYPE_char]], i64 4, !"_ZTS1A", [[TYPE_int]], i64 0, i64 4}
+// 

[PATCH] D41444: [ASTImporterTest] Make testing under '-fdelayed-template-parsing' mandatory

2017-12-20 Thread Aleksei Sidorin via Phabricator via cfe-commits
a.sidorin updated this revision to Diff 127748.
a.sidorin added a comment.

Test both with and without '-fdelayed-template-parsing' in C++ mode.


Repository:
  rC Clang

https://reviews.llvm.org/D41444

Files:
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -22,38 +22,50 @@
 namespace clang {
 namespace ast_matchers {
 
-typedef std::vector StringVector;
+typedef std::vector ArgVector;
+typedef std::vector RunOptions;
 
-void getLangArgs(Language Lang, StringVector ) {
+static bool isCXX(Language Lang) {
+  return Lang == Lang_CXX || Lang == Lang_CXX11;
+}
+
+static RunOptions getRunOptionsForLanguage(Language Lang) {
+  ArgVector BasicArgs;
+  // Test with basic arguments.
   switch (Lang) {
   case Lang_C:
-Args.insert(Args.end(), { "-x", "c", "-std=c99" });
+BasicArgs = {"-x", "c", "-std=c99"};
 break;
   case Lang_C89:
-Args.insert(Args.end(), { "-x", "c", "-std=c89" });
+BasicArgs = {"-x", "c", "-std=c89"};
 break;
   case Lang_CXX:
-Args.push_back("-std=c++98");
+BasicArgs = {"-std=c++98"};
 break;
   case Lang_CXX11:
-Args.push_back("-std=c++11");
+BasicArgs = {"-std=c++11"};
 break;
   case Lang_OpenCL:
   case Lang_OBJCXX:
-break;
+llvm_unreachable("Not implemented yet!");
   }
+
+  // For C++, test with "-fdelayed-template-parsing" enabled to handle MSVC
+  // default behaviour.
+  if (isCXX(Lang)) {
+ArgVector ArgsForDelayedTemplateParse = BasicArgs;
+ArgsForDelayedTemplateParse.emplace_back("-fdelayed-template-parsing");
+return {BasicArgs, ArgsForDelayedTemplateParse};
+  }
+
+  return {BasicArgs};
 }
 
 template
 testing::AssertionResult
-testImport(const std::string , Language FromLang,
-   const std::string , Language ToLang,
-   MatchVerifier ,
-   const MatcherType ) {
-  StringVector FromArgs, ToArgs;
-  getLangArgs(FromLang, FromArgs);
-  getLangArgs(ToLang, ToArgs);
-
+testImport(const ArgVector , const ArgVector ,
+   const std::string , const std::string ,
+   MatchVerifier , const MatcherType ) {
   const char *const InputFileName = "input.cc";
   const char *const OutputFileName = "output.cc";
 
@@ -92,7 +104,7 @@
 return testing::AssertionFailure() << "Import failed, nullptr returned!";
 
   // This should dump source locations and assert if some source locations
-  // were not imported
+  // were not imported.
   SmallString<1024> ImportChecker;
   llvm::raw_svector_ostream ToNothing(ImportChecker);
   ToCtx.getTranslationUnitDecl()->print(ToNothing);
@@ -104,148 +116,154 @@
   return Verifier.match(Imported, AMatcher);
 }
 
+template
+void testImport(const std::string , Language FromLang,
+const std::string , Language ToLang,
+MatchVerifier ,
+const MatcherType ) {
+  auto RunOptsFrom = getRunOptionsForLanguage(FromLang);
+  auto RunOptsTo = getRunOptionsForLanguage(ToLang);
+  for (const auto  : RunOptsFrom)
+for (const auto  : RunOptsTo)
+  EXPECT_TRUE(testImport(FromArgs, ToArgs, FromCode, ToCode,
+ Verifier, AMatcher));
+}
+
+
 TEST(ImportExpr, ImportStringLiteral) {
   MatchVerifier Verifier;
-  EXPECT_TRUE(testImport("void declToImport() { \"foo\"; }",
- Lang_CXX, "", Lang_CXX, Verifier,
- functionDecl(
-   hasBody(
- compoundStmt(
-   has(
- stringLiteral(
-   hasType(
- asString("const char [4]");
-  EXPECT_TRUE(testImport("void declToImport() { L\"foo\"; }",
- Lang_CXX, "", Lang_CXX, Verifier,
- functionDecl(
-   hasBody(
- compoundStmt(
-   has(
- stringLiteral(
-   hasType(
- asString("const wchar_t [4]");
-  EXPECT_TRUE(testImport("void declToImport() { \"foo\" \"bar\"; }",
- Lang_CXX, "", Lang_CXX, Verifier,
- functionDecl(
-   hasBody(
- compoundStmt(
-   has(
- stringLiteral(
-   hasType(
- asString("const char [7]");
+  testImport("void declToImport() { \"foo\"; }",
+ Lang_CXX, "", Lang_CXX, Verifier,
+ functionDecl(
+   hasBody(
+ compoundStmt(
+   has(
+ stringLiteral(
+   hasType(
+   

[PATCH] D41345: [clangd] Add more symbol information for code completion.

2017-12-20 Thread Sam McCall via Phabricator via cfe-commits
sammccall added inline comments.



Comment at: clangd/index/Index.h:105
+  /// What to insert when completing this symbol (plain text version).
+  std::string CompletionPlainInsertText;
+  /// What to insert when completing this symbol (snippet version).

insert texts can be in details I think? They're not required for completion 
until after resolve.



Comment at: clangd/index/Index.h:122
+
+  llvm::Optional Detail;
+

I think you probably want a raw pointer rather than optional:
 - reduce the size of the struct when it's absent
 - make it inheritance-friendly so we can hang index-specific info off it
(raw pointer rather than unique_ptr because it's owned by a slab not by malloc, 
but unique_ptr is ok for now)




Comment at: clangd/index/SymbolCollector.cpp:65
+  CodeCompletionResult SymbolCompletion(ND, 0);
+  auto Allocator = std::make_shared();
+  CodeCompletionTUInfo TUInfo(Allocator);

this doesn't seem like the kind of thing we should be allocating per-symbol!
You can use a single one and Reset() it, I think

also why globalcodecompletionallocator, vs just codecompletionallocator?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41345



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


[PATCH] D41450: [clangd] Pull CodeCompletionString handling logic into its own file and add unit test.

2017-12-20 Thread Eric Liu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
ioeric marked 7 inline comments as done.
Closed by commit rCTE321193: [clangd] Pull CodeCompletionString handling logic 
into its own file and add… (authored by ioeric, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D41450?vs=127718=127745#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41450

Files:
  clangd/CMakeLists.txt
  clangd/CodeComplete.cpp
  clangd/CodeCompletionStrings.cpp
  clangd/CodeCompletionStrings.h
  unittests/clangd/CMakeLists.txt
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/CodeCompletionStringsTests.cpp

Index: clangd/CodeCompletionStrings.cpp
===
--- clangd/CodeCompletionStrings.cpp
+++ clangd/CodeCompletionStrings.cpp
@@ -0,0 +1,188 @@
+//===--- CodeCompletionStrings.cpp ---*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===-===//
+
+#include "CodeCompletionStrings.h"
+#include 
+
+namespace clang {
+namespace clangd {
+
+namespace {
+
+bool isInformativeQualifierChunk(CodeCompletionString::Chunk const ) {
+  return Chunk.Kind == CodeCompletionString::CK_Informative &&
+ StringRef(Chunk.Text).endswith("::");
+}
+
+void processPlainTextChunks(const CodeCompletionString ,
+std::string *LabelOut, std::string *InsertTextOut) {
+  std::string  = *LabelOut;
+  std::string  = *InsertTextOut;
+  for (const auto  : CCS) {
+// Informative qualifier chunks only clutter completion results, skip
+// them.
+if (isInformativeQualifierChunk(Chunk))
+  continue;
+
+switch (Chunk.Kind) {
+case CodeCompletionString::CK_ResultType:
+case CodeCompletionString::CK_Optional:
+  break;
+case CodeCompletionString::CK_TypedText:
+  InsertText += Chunk.Text;
+  Label += Chunk.Text;
+  break;
+default:
+  Label += Chunk.Text;
+  break;
+}
+  }
+}
+
+void appendEscapeSnippet(const llvm::StringRef Text, std::string *Out) {
+  for (const auto Character : Text) {
+if (Character == '$' || Character == '}' || Character == '\\')
+  Out->push_back('\\');
+Out->push_back(Character);
+  }
+}
+
+void processSnippetChunks(const CodeCompletionString ,
+  std::string *LabelOut, std::string *InsertTextOut) {
+  std::string  = *LabelOut;
+  std::string  = *InsertTextOut;
+
+  unsigned ArgCount = 0;
+  for (const auto  : CCS) {
+// Informative qualifier chunks only clutter completion results, skip
+// them.
+if (isInformativeQualifierChunk(Chunk))
+  continue;
+
+switch (Chunk.Kind) {
+case CodeCompletionString::CK_TypedText:
+case CodeCompletionString::CK_Text:
+  Label += Chunk.Text;
+  InsertText += Chunk.Text;
+  break;
+case CodeCompletionString::CK_Optional:
+  // FIXME: Maybe add an option to allow presenting the optional chunks?
+  break;
+case CodeCompletionString::CK_Placeholder:
+  ++ArgCount;
+  InsertText += "${" + std::to_string(ArgCount) + ':';
+  appendEscapeSnippet(Chunk.Text, );
+  InsertText += '}';
+  Label += Chunk.Text;
+  break;
+case CodeCompletionString::CK_Informative:
+  // For example, the word "const" for a const method, or the name of
+  // the base class for methods that are part of the base class.
+  Label += Chunk.Text;
+  // Don't put the informative chunks in the insertText.
+  break;
+case CodeCompletionString::CK_ResultType:
+  // This is retrieved as detail.
+  break;
+case CodeCompletionString::CK_CurrentParameter:
+  // This should never be present while collecting completion items,
+  // only while collecting overload candidates.
+  llvm_unreachable("Unexpected CK_CurrentParameter while collecting "
+   "CompletionItems");
+  break;
+case CodeCompletionString::CK_LeftParen:
+case CodeCompletionString::CK_RightParen:
+case CodeCompletionString::CK_LeftBracket:
+case CodeCompletionString::CK_RightBracket:
+case CodeCompletionString::CK_LeftBrace:
+case CodeCompletionString::CK_RightBrace:
+case CodeCompletionString::CK_LeftAngle:
+case CodeCompletionString::CK_RightAngle:
+case CodeCompletionString::CK_Comma:
+case CodeCompletionString::CK_Colon:
+case CodeCompletionString::CK_SemiColon:
+case CodeCompletionString::CK_Equal:
+case CodeCompletionString::CK_HorizontalSpace:
+  InsertText += Chunk.Text;
+  Label += Chunk.Text;
+  break;
+case CodeCompletionString::CK_VerticalSpace:
+  InsertText += Chunk.Text;
+  // Don't even add a space to the label.
+  break;
+}
+  }
+}
+
+} // 

[clang-tools-extra] r321193 - [clangd] Pull CodeCompletionString handling logic into its own file and add unit test.

2017-12-20 Thread Eric Liu via cfe-commits
Author: ioeric
Date: Wed Dec 20 09:24:31 2017
New Revision: 321193

URL: http://llvm.org/viewvc/llvm-project?rev=321193=rev
Log:
[clangd] Pull CodeCompletionString handling logic into its own file and add 
unit test.

Reviewers: sammccall

Subscribers: klimek, mgorny, ilya-biryukov, cfe-commits

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

Added:
clang-tools-extra/trunk/clangd/CodeCompletionStrings.cpp
clang-tools-extra/trunk/clangd/CodeCompletionStrings.h
clang-tools-extra/trunk/unittests/clangd/CodeCompletionStringsTests.cpp
Modified:
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=321193=321192=321193=diff
==
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Wed Dec 20 09:24:31 2017
@@ -8,6 +8,7 @@ add_clang_library(clangDaemon
   ClangdUnit.cpp
   ClangdUnitStore.cpp
   CodeComplete.cpp
+  CodeCompletionStrings.cpp
   Context.cpp
   Compiler.cpp
   DraftStore.cpp

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=321193=321192=321193=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Wed Dec 20 09:24:31 2017
@@ -15,6 +15,7 @@
 //===-===//
 
 #include "CodeComplete.h"
+#include "CodeCompletionStrings.h"
 #include "Compiler.h"
 #include "Logger.h"
 #include "index/Index.h"
@@ -144,46 +145,6 @@ CompletionItemKind toCompletionItemKind(
   llvm_unreachable("Unhandled clang::index::SymbolKind.");
 }
 
-std::string escapeSnippet(const llvm::StringRef Text) {
-  std::string Result;
-  Result.reserve(Text.size()); // Assume '$', '}' and '\\' are rare.
-  for (const auto Character : Text) {
-if (Character == '$' || Character == '}' || Character == '\\')
-  Result.push_back('\\');
-Result.push_back(Character);
-  }
-  return Result;
-}
-
-std::string getDocumentation(const CodeCompletionString ) {
-  // Things like __attribute__((nonnull(1,3))) and [[noreturn]]. Present this
-  // information in the documentation field.
-  std::string Result;
-  const unsigned AnnotationCount = CCS.getAnnotationCount();
-  if (AnnotationCount > 0) {
-Result += "Annotation";
-if (AnnotationCount == 1) {
-  Result += ": ";
-} else /* AnnotationCount > 1 */ {
-  Result += "s: ";
-}
-for (unsigned I = 0; I < AnnotationCount; ++I) {
-  Result += CCS.getAnnotation(I);
-  Result.push_back(I == AnnotationCount - 1 ? '\n' : ' ');
-}
-  }
-  // Add brief documentation (if there is any).
-  if (CCS.getBriefComment() != nullptr) {
-if (!Result.empty()) {
-  // This means we previously added annotations. Add an extra newline
-  // character to make the annotations stand out.
-  Result.push_back('\n');
-}
-Result += CCS.getBriefComment();
-  }
-  return Result;
-}
-
 /// Get the optional chunk as a string. This function is possibly recursive.
 ///
 /// The parameter info for each parameter is appended to the Parameters.
@@ -320,7 +281,8 @@ public:
  /*OutputIsBinary=*/false),
 ClangdOpts(CodeCompleteOpts), Items(Items),
 Allocator(std::make_shared()),
-CCTUInfo(Allocator), CompletedName(CompletedName) {}
+CCTUInfo(Allocator), CompletedName(CompletedName),
+EnableSnippets(CodeCompleteOpts.EnableSnippets) {}
 
   void ProcessCodeCompleteResults(Sema , CodeCompletionContext Context,
   CodeCompletionResult *Results,
@@ -402,14 +364,16 @@ private:
 // Adjust this to InsertTextFormat::Snippet iff we encounter a
 // CK_Placeholder chunk in SnippetCompletionItemsCollector.
 CompletionItem Item;
-Item.insertTextFormat = InsertTextFormat::PlainText;
 
 Item.documentation = getDocumentation(CCS);
 Item.sortText = Candidate.sortText();
 
-// Fill in the label, detail, insertText and filterText fields of the
-// CompletionItem.
-ProcessChunks(CCS, Item);
+Item.detail = getDetail(CCS);
+Item.filterText = getFilterText(CCS);
+getLabelAndInsertText(CCS, , , EnableSnippets);
+
+Item.insertTextFormat = EnableSnippets ? InsertTextFormat::Snippet
+   : InsertTextFormat::PlainText;
 
 // Fill in the kind field of the CompletionItem.
 Item.kind = toCompletionItemKind(Candidate.Result->Kind,
@@ 

[clang-tools-extra] r321192 - [clangd] Remove an unused lambda capture.

2017-12-20 Thread Eric Liu via cfe-commits
Author: ioeric
Date: Wed Dec 20 09:22:56 2017
New Revision: 321192

URL: http://llvm.org/viewvc/llvm-project?rev=321192=rev
Log:
[clangd] Remove an unused lambda capture.

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

Modified: clang-tools-extra/trunk/unittests/clangd/Annotations.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/Annotations.cpp?rev=321192=321191=321192=diff
==
--- clang-tools-extra/trunk/unittests/clangd/Annotations.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/Annotations.cpp Wed Dec 20 
09:22:56 2017
@@ -24,7 +24,7 @@ static void require(bool Assertion, cons
 
 Annotations::Annotations(StringRef Text) {
   auto Here = [this] { return offsetToPosition(Code, Code.size()); };
-  auto Require = [this, Text](bool Assertion, const char *Msg) {
+  auto Require = [Text](bool Assertion, const char *Msg) {
 require(Assertion, Msg, Text);
   };
   Optional Name;


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


[PATCH] D41454: [clangd] Add ClangdUnit diagnostics tests using annotated code.

2017-12-20 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision.
sammccall added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, mgorny, klimek.

This adds checks that our diagnostics emit correct ranges in a bunch of cases,
as promised in https://reviews.llvm.org/D41118.

The diagnostics-preamble test is also converted and extended to be a little more
precise.

diagnostics.test stays around as the smoke test for this feature.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41454

Files:
  clangd/Protocol.cpp
  clangd/Protocol.h
  test/clangd/diagnostics-preamble.test
  unittests/clangd/CMakeLists.txt
  unittests/clangd/ClangdUnitTests.cpp

Index: unittests/clangd/ClangdUnitTests.cpp
===
--- /dev/null
+++ unittests/clangd/ClangdUnitTests.cpp
@@ -0,0 +1,124 @@
+//===-- ClangdUnitTests.cpp - ClangdUnit tests --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "ClangdUnit.h"
+#include "Annotations.h"
+#include "TestFS.h"
+#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/PCHContainerOperations.h"
+#include "clang/Frontend/Utils.h"
+#include "llvm/Support/ScopedPrinter.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+using namespace llvm;
+void PrintTo(const DiagWithFixIts , std::ostream *O) {
+  llvm::raw_os_ostream OS(*O);
+  OS << D.Diag;
+  if (!D.FixIts.empty()) {
+OS << " {";
+const char *Sep = "";
+for (const auto  : D.FixIts) {
+  OS << Sep << F;
+  Sep = ", ";
+}
+OS << "}";
+  }
+}
+
+namespace {
+using testing::ElementsAre;
+
+// FIXME: this is duplicated with FileIndexTests. Share it.
+ParsedAST build(StringRef Code, std::vector Flags = {}) {
+  std::vector Cmd = {"clang", "main.cpp"};
+  Cmd.insert(Cmd.begin() + 1, Flags.begin(), Flags.end());
+  auto CI = createInvocationFromCommandLine(Cmd);
+  auto Buf = MemoryBuffer::getMemBuffer(Code);
+  auto AST = ParsedAST::Build(
+  Context::empty(), std::move(CI), nullptr, std::move(Buf),
+  std::make_shared(), vfs::getRealFileSystem());
+  assert(AST.hasValue());
+  return std::move(*AST);
+}
+
+MATCHER_P2(Diag, Range, Message,
+   "Diagnostic at " + llvm::to_string(Range) + " = [" + Message + "]") {
+  return arg.Diag.range == Range && arg.Diag.message == Message &&
+ arg.FixIts.empty();
+}
+
+MATCHER_P3(Fix, Range, Replacement, Message,
+   "Fix " + llvm::to_string(Range) + " => " +
+   testing::PrintToString(Replacement) + " = [" + Message + "]") {
+  return arg.Diag.range == Range && arg.Diag.message == Message &&
+ arg.FixIts.size() == 1 && arg.FixIts[0].range == Range &&
+ arg.FixIts[0].newText == Replacement;
+}
+
+TEST(DiagnosticsTest, DiagnosticRanges) {
+  // Check we report correct ranges, including those that are zero-width, at
+  // the end of a line, or span multiple lines.
+  Annotations Test(R"cpp(
+void $decl[[foo]]();
+int main() {
+  $typo[[go\
+o]]();
+  foo()$semicolon[[]]
+}
+  )cpp");
+  llvm::errs() << Test.code();
+  EXPECT_THAT(
+  build(Test.code()).getDiagnostics(),
+  ElementsAre(
+  Fix(Test.range("typo"), "foo",
+  "use of undeclared identifier 'goo'; did you mean 'foo'?"),
+  Diag(Test.range("decl"), "'foo' declared here"),
+  Fix(Test.range("semicolon"), ";",
+  "expected ';' after expression")));
+}
+
+TEST(DiagnosticsTest, FlagsMatter) {
+  Annotations Test("[[void]] main() {}");
+  EXPECT_THAT(
+  build(Test.code()).getDiagnostics(),
+  ElementsAre(Fix(Test.range(), "int", "'main' must return 'int'")));
+  // Same code built as C gets different diagnostics.
+  EXPECT_THAT(
+  build(Test.code(), {"-x", "c"}).getDiagnostics(),
+  ElementsAre(
+  // FIXME: ideally this would be one diagnostic with a named FixIt.
+  Diag(Test.range(), "return type of 'main' is not 'int'"),
+  Fix(Test.range(), "int", "change return type to 'int'")));
+}
+
+TEST(DiagnosticsTest, Preprocessor) {
+  // This looks like a preamble, but there's an #else in the middle!
+  // Check that:
+  //  - the #else doesn't generate diagnostics (we had this bug)
+  //  - we get diagnostics from the taken branch
+  //  - we get no diagnostics from the not taken branch
+  Annotations Test(R"cpp(
+#ifndef FOO
+#define FOO
+  int a = [[b]];
+#else
+  int x = y;
+#endif
+)cpp");
+  EXPECT_THAT(
+  build(Test.code()).getDiagnostics(),
+  ElementsAre(Diag(Test.range(), "use of undeclared identifier 'b'")));
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: 

[PATCH] D41399: [CodeGen] Represent array members in new-format TBAA type descriptors

2017-12-20 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added inline comments.



Comment at: test/CodeGen/tbaa-array.cpp:24
+// CHECK-DAG: [[TAG_A_i]] = !{[[TYPE_A:!.*]], [[TYPE_int:!.*]], i64 0, i64 4}
+// CHECK-DAG: [[TAG_C_i]] = !{[[TYPE_C:!.*]], [[TYPE_int:!.*]], i64 0, i64 16}
+// CHECK-DAG: [[TYPE_A]] = !{[[TYPE_char:!.*]], i64 4, !"_ZTS1A", 
[[TYPE_int]], i64 0, i64 4}

kosarev wrote:
> kosarev wrote:
> > hfinkel wrote:
> > > Shouldn't this access have a size of 4, and an access for c->x[2] have a 
> > > size of 4 and a specific offset and c->x[j] have a size of 12 and an 
> > > offset of zero? Why does this list a size of 16?
> > > 
> > > In any case, please add tests for:
> > > 
> > >   int *bar2(C *c) {
> > > return c->x;
> > >   }
> > > 
> > >   int bar3(C *c) {
> > > return c->x[2];
> > >   }
> > > 
> > >   int bar4(C *c, int j) {
> > > return c->x[j];
> > >   }
> > > 
> > Indeed, the access size is wrong as we mistakenly inherit it from the base 
> > type. D41452 fixes this. Thanks for catching.
> Hal, in bar2() we don't really access memory. What do we want to check with 
> it?
> Hal, in bar2() we don't really access memory. What do we want to check with 
> it?

Ah, good point. Ignore that one.


Repository:
  rL LLVM

https://reviews.llvm.org/D41399



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


[PATCH] D41451: Make DiagnosticIDs::getAllDiagnostics use std::vector

2017-12-20 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun closed this revision.
xazax.hun added a comment.

Committed in https://reviews.llvm.org/rL321190


https://reviews.llvm.org/D41451



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


r321190 - Make DiagnosticIDs::getAllDiagnostics use std::vector. NFC.

2017-12-20 Thread Gabor Horvath via cfe-commits
Author: xazax
Date: Wed Dec 20 08:55:41 2017
New Revision: 321190

URL: http://llvm.org/viewvc/llvm-project?rev=321190=rev
Log:
Make DiagnosticIDs::getAllDiagnostics use std::vector. NFC.

The size of the result vector is currently around 4600 with
Flavor::WarningOrError, which makes std::vector a better candidate than
llvm::SmallVector.

Patch by: Andras Leitereg!

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

Modified:
cfe/trunk/include/clang/Basic/DiagnosticIDs.h
cfe/trunk/lib/Basic/Diagnostic.cpp
cfe/trunk/lib/Basic/DiagnosticIDs.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticIDs.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticIDs.h?rev=321190=321189=321190=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticIDs.h (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticIDs.h Wed Dec 20 08:55:41 2017
@@ -297,7 +297,7 @@ public:
 
   /// \brief Get the set of all diagnostic IDs.
   static void getAllDiagnostics(diag::Flavor Flavor,
-SmallVectorImpl );
+std::vector );
 
   /// \brief Get the diagnostic option with the closest edit distance to the
   /// given group name.

Modified: cfe/trunk/lib/Basic/Diagnostic.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Diagnostic.cpp?rev=321190=321189=321190=diff
==
--- cfe/trunk/lib/Basic/Diagnostic.cpp (original)
+++ cfe/trunk/lib/Basic/Diagnostic.cpp Wed Dec 20 08:55:41 2017
@@ -363,7 +363,7 @@ void DiagnosticsEngine::setSeverityForAl
   diag::Severity Map,
   SourceLocation Loc) {
   // Get all the diagnostics.
-  SmallVector AllDiags;
+  std::vector AllDiags;
   DiagnosticIDs::getAllDiagnostics(Flavor, AllDiags);
 
   // Set the mapping.

Modified: cfe/trunk/lib/Basic/DiagnosticIDs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/DiagnosticIDs.cpp?rev=321190=321189=321190=diff
==
--- cfe/trunk/lib/Basic/DiagnosticIDs.cpp (original)
+++ cfe/trunk/lib/Basic/DiagnosticIDs.cpp Wed Dec 20 08:55:41 2017
@@ -583,7 +583,7 @@ DiagnosticIDs::getDiagnosticsInGroup(dia
 }
 
 void DiagnosticIDs::getAllDiagnostics(diag::Flavor Flavor,
-  SmallVectorImpl ) {
+  std::vector ) {
   for (unsigned i = 0; i != StaticDiagInfoSize; ++i)
 if (StaticDiagInfo[i].getFlavor() == Flavor)
   Diags.push_back(StaticDiagInfo[i].DiagID);


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


r321189 - [clang] Add BeforeExecute method to PrecompiledPreamble

2017-12-20 Thread Ilya Biryukov via cfe-commits
Author: ibiryukov
Date: Wed Dec 20 08:48:56 2017
New Revision: 321189

URL: http://llvm.org/viewvc/llvm-project?rev=321189=rev
Log:
[clang] Add BeforeExecute method to PrecompiledPreamble

Summary: Adds BeforeExecute method to PrecompiledPreamble to be called
before Execute(). This method can be overriden.

Patch by William Enright.

Reviewers: malaperle, ilya-biryukov

Reviewed By: ilya-biryukov

Subscribers: arphaman, cfe-commits, ilya-biryukov

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

Modified:
cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h
cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp

Modified: cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h?rev=321189=321188=321189=diff
==
--- cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h (original)
+++ cfe/trunk/include/clang/Frontend/PrecompiledPreamble.h Wed Dec 20 08:48:56 
2017
@@ -244,6 +244,10 @@ class PreambleCallbacks {
 public:
   virtual ~PreambleCallbacks() = default;
 
+  /// Called before FrontendAction::BeginSourceFile.
+  /// Can be used to store references to various CompilerInstance fields
+  /// (e.g. SourceManager) that may be interesting to the consumers of other 
callbacks.
+  virtual void BeforeExecute(CompilerInstance );
   /// Called after FrontendAction::Execute(), but before
   /// FrontendAction::EndSourceFile(). Can be used to transfer ownership of
   /// various CompilerInstance fields before they are destroyed.

Modified: cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp?rev=321189=321188=321189=diff
==
--- cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp (original)
+++ cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp Wed Dec 20 08:48:56 2017
@@ -333,6 +333,7 @@ llvm::ErrorOr Preco
   std::unique_ptr Act;
   Act.reset(new PrecompilePreambleAction(
   StoreInMemory ? ().Data : nullptr, Callbacks));
+  Callbacks.BeforeExecute(*Clang);
   if (!Act->BeginSourceFile(*Clang.get(), Clang->getFrontendOpts().Inputs[0]))
 return BuildPreambleError::BeginSourceFileFailed;
 
@@ -694,6 +695,7 @@ void PrecompiledPreamble::setupPreambleS
   }
 }
 
+void PreambleCallbacks::BeforeExecute(CompilerInstance ) {}
 void PreambleCallbacks::AfterExecute(CompilerInstance ) {}
 void PreambleCallbacks::AfterPCHEmitted(ASTWriter ) {}
 void PreambleCallbacks::HandleTopLevelDecl(DeclGroupRef DG) {}


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


[PATCH] D41365: [clang] Add BeforeExecute method to PrecompiledPreamble

2017-12-20 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC321189: [clang] Add BeforeExecute method to 
PrecompiledPreamble (authored by ibiryukov, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D41365?vs=127715=127740#toc

Repository:
  rC Clang

https://reviews.llvm.org/D41365

Files:
  include/clang/Frontend/PrecompiledPreamble.h
  lib/Frontend/PrecompiledPreamble.cpp


Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -333,6 +333,7 @@
   std::unique_ptr Act;
   Act.reset(new PrecompilePreambleAction(
   StoreInMemory ? ().Data : nullptr, Callbacks));
+  Callbacks.BeforeExecute(*Clang);
   if (!Act->BeginSourceFile(*Clang.get(), Clang->getFrontendOpts().Inputs[0]))
 return BuildPreambleError::BeginSourceFileFailed;
 
@@ -694,6 +695,7 @@
   }
 }
 
+void PreambleCallbacks::BeforeExecute(CompilerInstance ) {}
 void PreambleCallbacks::AfterExecute(CompilerInstance ) {}
 void PreambleCallbacks::AfterPCHEmitted(ASTWriter ) {}
 void PreambleCallbacks::HandleTopLevelDecl(DeclGroupRef DG) {}
Index: include/clang/Frontend/PrecompiledPreamble.h
===
--- include/clang/Frontend/PrecompiledPreamble.h
+++ include/clang/Frontend/PrecompiledPreamble.h
@@ -244,6 +244,10 @@
 public:
   virtual ~PreambleCallbacks() = default;
 
+  /// Called before FrontendAction::BeginSourceFile.
+  /// Can be used to store references to various CompilerInstance fields
+  /// (e.g. SourceManager) that may be interesting to the consumers of other 
callbacks.
+  virtual void BeforeExecute(CompilerInstance );
   /// Called after FrontendAction::Execute(), but before
   /// FrontendAction::EndSourceFile(). Can be used to transfer ownership of
   /// various CompilerInstance fields before they are destroyed.


Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -333,6 +333,7 @@
   std::unique_ptr Act;
   Act.reset(new PrecompilePreambleAction(
   StoreInMemory ? ().Data : nullptr, Callbacks));
+  Callbacks.BeforeExecute(*Clang);
   if (!Act->BeginSourceFile(*Clang.get(), Clang->getFrontendOpts().Inputs[0]))
 return BuildPreambleError::BeginSourceFileFailed;
 
@@ -694,6 +695,7 @@
   }
 }
 
+void PreambleCallbacks::BeforeExecute(CompilerInstance ) {}
 void PreambleCallbacks::AfterExecute(CompilerInstance ) {}
 void PreambleCallbacks::AfterPCHEmitted(ASTWriter ) {}
 void PreambleCallbacks::HandleTopLevelDecl(DeclGroupRef DG) {}
Index: include/clang/Frontend/PrecompiledPreamble.h
===
--- include/clang/Frontend/PrecompiledPreamble.h
+++ include/clang/Frontend/PrecompiledPreamble.h
@@ -244,6 +244,10 @@
 public:
   virtual ~PreambleCallbacks() = default;
 
+  /// Called before FrontendAction::BeginSourceFile.
+  /// Can be used to store references to various CompilerInstance fields
+  /// (e.g. SourceManager) that may be interesting to the consumers of other callbacks.
+  virtual void BeforeExecute(CompilerInstance );
   /// Called after FrontendAction::Execute(), but before
   /// FrontendAction::EndSourceFile(). Can be used to transfer ownership of
   /// various CompilerInstance fields before they are destroyed.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D41399: [CodeGen] Represent array members in new-format TBAA type descriptors

2017-12-20 Thread Ivan Kosarev via Phabricator via cfe-commits
kosarev added inline comments.



Comment at: test/CodeGen/tbaa-array.cpp:24
+// CHECK-DAG: [[TAG_A_i]] = !{[[TYPE_A:!.*]], [[TYPE_int:!.*]], i64 0, i64 4}
+// CHECK-DAG: [[TAG_C_i]] = !{[[TYPE_C:!.*]], [[TYPE_int:!.*]], i64 0, i64 16}
+// CHECK-DAG: [[TYPE_A]] = !{[[TYPE_char:!.*]], i64 4, !"_ZTS1A", 
[[TYPE_int]], i64 0, i64 4}

kosarev wrote:
> hfinkel wrote:
> > Shouldn't this access have a size of 4, and an access for c->x[2] have a 
> > size of 4 and a specific offset and c->x[j] have a size of 12 and an offset 
> > of zero? Why does this list a size of 16?
> > 
> > In any case, please add tests for:
> > 
> >   int *bar2(C *c) {
> > return c->x;
> >   }
> > 
> >   int bar3(C *c) {
> > return c->x[2];
> >   }
> > 
> >   int bar4(C *c, int j) {
> > return c->x[j];
> >   }
> > 
> Indeed, the access size is wrong as we mistakenly inherit it from the base 
> type. D41452 fixes this. Thanks for catching.
Hal, in bar2() we don't really access memory. What do we want to check with it?


Repository:
  rL LLVM

https://reviews.llvm.org/D41399



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


[PATCH] D41384: [analyzer] Suppress false positive warnings form security.insecureAPI.strcpy

2017-12-20 Thread András Leitereg via Phabricator via cfe-commits
leanil updated this revision to Diff 127739.
leanil added a comment.

Move negative test next to the positive ones, because the necessary macros and 
run-lines are already defined there.


https://reviews.llvm.org/D41384

Files:
  lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
  test/Analysis/security-syntax-checks.m


Index: test/Analysis/security-syntax-checks.m
===
--- test/Analysis/security-syntax-checks.m
+++ test/Analysis/security-syntax-checks.m
@@ -146,6 +146,11 @@
   strcpy(x, y); //expected-warning{{Call to function 'strcpy' is insecure as 
it does not provide bounding of the memory buffer. Replace unbounded copy 
functions with analogous functions that support length arguments such as 
'strlcpy'. CWE-119}}
 }
 
+void test_strcpy_safe() {
+  char x[5];
+  strcpy(x, "abcd");
+}
+
 //===--===
 // strcat()
 //===--===
Index: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
===
--- lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -510,6 +510,18 @@
   if (!checkCall_strCommon(CE, FD))
 return;
 
+  int ArraySize = -1, StrLen = -1;
+  const auto *Target = CE->getArg(0)->IgnoreImpCasts(),
+ *Source = CE->getArg(1)->IgnoreImpCasts();
+  if (const auto *DeclRef = dyn_cast(Target))
+if (const auto *Array = dyn_cast(
+DeclRef->getDecl()->getType().getTypePtr()))
+  ArraySize = Array->getSize().getLimitedValue();
+  if (const auto *String = dyn_cast(Source))
+StrLen = String->getLength();
+  if (StrLen != -1 && ArraySize >= StrLen + 1)
+return;
+
   // Issue a warning.
   PathDiagnosticLocation CELoc =
 PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);


Index: test/Analysis/security-syntax-checks.m
===
--- test/Analysis/security-syntax-checks.m
+++ test/Analysis/security-syntax-checks.m
@@ -146,6 +146,11 @@
   strcpy(x, y); //expected-warning{{Call to function 'strcpy' is insecure as it does not provide bounding of the memory buffer. Replace unbounded copy functions with analogous functions that support length arguments such as 'strlcpy'. CWE-119}}
 }
 
+void test_strcpy_safe() {
+  char x[5];
+  strcpy(x, "abcd");
+}
+
 //===--===
 // strcat()
 //===--===
Index: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
===
--- lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -510,6 +510,18 @@
   if (!checkCall_strCommon(CE, FD))
 return;
 
+  int ArraySize = -1, StrLen = -1;
+  const auto *Target = CE->getArg(0)->IgnoreImpCasts(),
+ *Source = CE->getArg(1)->IgnoreImpCasts();
+  if (const auto *DeclRef = dyn_cast(Target))
+if (const auto *Array = dyn_cast(
+DeclRef->getDecl()->getType().getTypePtr()))
+  ArraySize = Array->getSize().getLimitedValue();
+  if (const auto *String = dyn_cast(Source))
+StrLen = String->getLength();
+  if (StrLen != -1 && ArraySize >= StrLen + 1)
+return;
+
   // Issue a warning.
   PathDiagnosticLocation CELoc =
 PathDiagnosticLocation::createBegin(CE, BR.getSourceManager(), AC);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2017-12-20 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments.



Comment at: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp:407
+  if (!NaiveCTU.hasValue()) {
+NaiveCTU = getBooleanOption("experimental-enable-naive-ctu-analysis",
+/*Default=*/false);

This option might not be the most readable but this way experimental is a 
prefix. Do you prefer this way or something like 
`enable-experimental-naive-ctu-analysis`?


https://reviews.llvm.org/D30691



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


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

2017-12-20 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 127525.
xazax.hun marked an inline comment as not done.
xazax.hun added a comment.

- Address some review comments
- Rebased on ToT


https://reviews.llvm.org/D30691

Files:
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/CMakeLists.txt
  lib/StaticAnalyzer/Core/CallEvent.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  lib/StaticAnalyzer/Frontend/CMakeLists.txt
  test/Analysis/Inputs/ctu-chain.cpp
  test/Analysis/Inputs/ctu-other.cpp
  test/Analysis/Inputs/externalFnMap.txt
  test/Analysis/ctu-main.cpp
  tools/scan-build-py/libscanbuild/__init__.py
  tools/scan-build-py/libscanbuild/analyze.py
  tools/scan-build-py/libscanbuild/arguments.py
  tools/scan-build-py/libscanbuild/clang.py
  tools/scan-build-py/libscanbuild/report.py
  tools/scan-build-py/tests/unit/test_analyze.py
  tools/scan-build-py/tests/unit/test_clang.py

Index: tools/scan-build-py/tests/unit/test_clang.py
===
--- tools/scan-build-py/tests/unit/test_clang.py
+++ tools/scan-build-py/tests/unit/test_clang.py
@@ -92,3 +92,15 @@
 self.assertEqual('Checker One description', result.get('checker.one'))
 self.assertTrue('checker.two' in result)
 self.assertEqual('Checker Two description', result.get('checker.two'))
+
+
+class ClangIsCtuCapableTest(unittest.TestCase):
+def test_ctu_not_found(self):
+is_ctu = sut.is_ctu_capable('not-found-clang-func-mapping')
+self.assertFalse(is_ctu)
+
+
+class ClangGetTripleArchTest(unittest.TestCase):
+def test_arch_is_not_empty(self):
+arch = sut.get_triple_arch(['clang', '-E', '-'], '.')
+self.assertTrue(len(arch) > 0)
Index: tools/scan-build-py/tests/unit/test_analyze.py
===
--- tools/scan-build-py/tests/unit/test_analyze.py
+++ tools/scan-build-py/tests/unit/test_analyze.py
@@ -4,12 +4,12 @@
 # This file is distributed under the University of Illinois Open Source
 # License. See LICENSE.TXT for details.
 
-import libear
-import libscanbuild.analyze as sut
 import unittest
 import re
 import os
 import os.path
+import libear
+import libscanbuild.analyze as sut
 
 
 class ReportDirectoryTest(unittest.TestCase):
@@ -333,3 +333,83 @@
 
 def test_method_exception_not_caught(self):
 self.assertRaises(Exception, method_exception_from_inside, dict())
+
+
+class PrefixWithTest(unittest.TestCase):
+
+def test_gives_empty_on_empty(self):
+res = sut.prefix_with(0, [])
+self.assertFalse(res)
+
+def test_interleaves_prefix(self):
+res = sut.prefix_with(0, [1, 2, 3])
+self.assertListEqual([0, 1, 0, 2, 0, 3], res)
+
+
+class MergeCtuMapTest(unittest.TestCase):
+
+def test_no_map_gives_empty(self):
+pairs = sut.create_global_ctu_function_map([])
+self.assertFalse(pairs)
+
+def test_multiple_maps_merged(self):
+concat_map = ['c:@F@fun1#I# ast/fun1.c.ast',
+  'c:@F@fun2#I# ast/fun2.c.ast',
+  'c:@F@fun3#I# ast/fun3.c.ast']
+pairs = sut.create_global_ctu_function_map(concat_map)
+self.assertTrue(('c:@F@fun1#I#', 'ast/fun1.c.ast') in pairs)
+self.assertTrue(('c:@F@fun2#I#', 'ast/fun2.c.ast') in pairs)
+self.assertTrue(('c:@F@fun3#I#', 'ast/fun3.c.ast') in pairs)
+self.assertEqual(3, len(pairs))
+
+def test_not_unique_func_left_out(self):
+concat_map = ['c:@F@fun1#I# ast/fun1.c.ast',
+  'c:@F@fun2#I# ast/fun2.c.ast',
+  'c:@F@fun1#I# ast/fun7.c.ast']
+pairs = sut.create_global_ctu_function_map(concat_map)
+self.assertFalse(('c:@F@fun1#I#', 'ast/fun1.c.ast') in pairs)
+self.assertFalse(('c:@F@fun1#I#', 'ast/fun7.c.ast') in pairs)
+self.assertTrue(('c:@F@fun2#I#', 'ast/fun2.c.ast') in pairs)
+self.assertEqual(1, len(pairs))
+
+def test_duplicates_are_kept(self):
+concat_map = ['c:@F@fun1#I# ast/fun1.c.ast',
+  'c:@F@fun2#I# ast/fun2.c.ast',
+  'c:@F@fun1#I# ast/fun1.c.ast']
+pairs = sut.create_global_ctu_function_map(concat_map)
+self.assertTrue(('c:@F@fun1#I#', 'ast/fun1.c.ast') in pairs)
+self.assertTrue(('c:@F@fun2#I#', 'ast/fun2.c.ast') in pairs)
+self.assertEqual(2, len(pairs))
+
+def test_space_handled_in_source(self):
+concat_map = ['c:@F@fun1#I# ast/f un.c.ast']
+pairs = sut.create_global_ctu_function_map(concat_map)
+self.assertTrue(('c:@F@fun1#I#', 'ast/f un.c.ast') in pairs)
+self.assertEqual(1, len(pairs))
+
+
+class FuncMapSrcToAstTest(unittest.TestCase):
+
+def 

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

2017-12-20 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked 6 inline comments as done.
xazax.hun added inline comments.



Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:372
+
+  cross_tu::CrossTranslationUnitContext  =
+  Engine->getCrossTranslationUnitContext();

dcoughlin wrote:
> Can this logic be moved to AnalysisDeclContext->getBody()?
> 
> CallEvent::getRuntimeDefinition() is really all about modeling function 
> dispatch at run time. It seems odd to have the cross-translation-unit loading 
> (which is about compiler book-keeping rather than semantics) here.
I just tried that and unfortunately, that introduces cyclic dependencies. 
CrossTU depends on Frontend. Frontend depends on Sema. Sema depends on 
Analysis. Making Analysis depending on CrossTU introduces the cycle.



Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:382
+[&](const cross_tu::IndexError ) {
+  CTUCtx.emitCrossTUDiagnostics(IE);
+});

dcoughlin wrote:
> I don't think it makes sense to diagnose index errors here.
> 
> Doing it when during analysis means that, for example, the parse error could 
> be emitted or not emitted depending on whether the analyzer thinks a 
> particular call site is reached.
> 
> It would be better to validate/parse the index before starting analysis 
> rather than during analysis itself.
> 
> 
While I agree, right now this validation is not the part of the analyzer but 
the responsibility of the "driver" script for example CodeChecker. It is useful 
to have this diagnostics to catch bugs in the driver. 


https://reviews.llvm.org/D30691



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


[PATCH] D38171: [clang-tidy] Implement clang-tidy check aliases

2017-12-20 Thread András Leitereg via Phabricator via cfe-commits
leanil marked 2 inline comments as done.
leanil added a comment.

Does anyone have any more thoughts about this?


https://reviews.llvm.org/D38171



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


[libcxx] r321188 - Fix the definitions of 'reference' and 'pointer' in string_view that no one uses :-). Thanks to K-ballo for the catch.

2017-12-20 Thread Marshall Clow via cfe-commits
Author: marshall
Date: Wed Dec 20 08:31:40 2017
New Revision: 321188

URL: http://llvm.org/viewvc/llvm-project?rev=321188=rev
Log:
Fix the definitions of 'reference' and 'pointer' in string_view that no one 
uses :-). Thanks to K-ballo for the catch.

Added:
libcxx/trunk/test/std/strings/string.view/types.pass.cpp
Modified:
libcxx/trunk/include/string_view

Modified: libcxx/trunk/include/string_view
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/string_view?rev=321188=321187=321188=diff
==
--- libcxx/trunk/include/string_view (original)
+++ libcxx/trunk/include/string_view Wed Dec 20 08:31:40 2017
@@ -196,9 +196,9 @@ public:
 // types
 typedef _Traitstraits_type;
 typedef _CharT value_type;
-typedef const _CharT*  pointer;
+typedef _CharT*pointer;
 typedef const _CharT*  const_pointer;
-typedef const _CharT&  reference;
+typedef _CharT&reference;
 typedef const _CharT&  const_reference;
 typedef const_pointer  const_iterator; // See 
[string.view.iterators]
 typedef const_iterator iterator;

Added: libcxx/trunk/test/std/strings/string.view/types.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/strings/string.view/types.pass.cpp?rev=321188=auto
==
--- libcxx/trunk/test/std/strings/string.view/types.pass.cpp (added)
+++ libcxx/trunk/test/std/strings/string.view/types.pass.cpp Wed Dec 20 
08:31:40 2017
@@ -0,0 +1,77 @@
+//===--===//
+//
+// 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.
+//
+//===--===//
+
+// 
+
+// Test nested types and default template args:
+
+// template>
+// {
+// public:
+// // types:
+// using traits_type   = traits;
+// using value_type= charT;
+// using pointer   = value_type*;
+// using const_pointer = const value_type*;
+// using reference = value_type&;
+// using const_reference   = const value_type&;
+// using const_iterator= implementation-defined ; // see 
24.4.2.2
+// using iterator  = const_iterator;
+// using const_reverse_iterator= reverse_iterator;
+// using iterator  = const_reverse_iterator;
+// using size_type = size_t;
+// using difference_type   = ptrdiff_t;
+// static constexpr size_type npos = size_type(-1);
+// 
+// };
+
+#include 
+#include 
+#include 
+
+#include "test_macros.h"
+
+template 
+void
+test()
+{
+typedef std::basic_string_view S;
+
+static_assert((std::is_same::value), 
"");
+static_assert((std::is_same::value), "");
+static_assert((std::is_same::value), "");
+static_assert((std::is_same::value), "");
+static_assert((std::is_same::value), "");
+static_assert((std::is_same::value), "");
+static_assert((std::is_same::value), "");
+static_assert((std::is_same::value), "");
+static_assert((std::is_same<
+typename std::iterator_traits::iterator_category,
+std::random_access_iterator_tag>::value), "");
+static_assert((std::is_same<
+typename std::iterator_traits::iterator_category,
+std::random_access_iterator_tag>::value), "");
+static_assert((std::is_same<
+typename S::reverse_iterator,
+std::reverse_iterator >::value), "");
+static_assert((std::is_same<
+typename S::const_reverse_iterator,
+std::reverse_iterator >::value), "");
+static_assert(S::npos == -1, "");
+static_assert((std::is_same::value), "");
+static_assert((std::is_same::value), "");
+}
+
+int main()
+{
+test();
+test();
+static_assert((std::is_same::value), "");
+}


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


[PATCH] D38171: [clang-tidy] Implement clang-tidy check aliases

2017-12-20 Thread András Leitereg via Phabricator via cfe-commits
leanil updated this revision to Diff 127736.
leanil added a comment.

Remove redundant empty lines.
Make list-clang-diagnostics test less strict.
Update getAllDiagnostics to use std::vector.


https://reviews.llvm.org/D38171

Files:
  clang-tidy/ClangTidy.cpp
  clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tidy/ClangTidyModule.h
  clang-tidy/cert/CERTTidyModule.cpp
  clang-tidy/tool/ClangTidyMain.cpp
  test/clang-tidy/cert-exp59-cpp.cpp
  test/clang-tidy/custom-diagnostics.cpp
  test/clang-tidy/diagnostic.cpp
  test/clang-tidy/list-clang-diagnostics.cpp
  test/clang-tidy/misc-suspicious-semicolon-fail.cpp
  test/clang-tidy/validate-check-names.cpp
  test/clang-tidy/warning-check-aliases.cpp
  test/clang-tidy/werrors-diagnostics.cpp

Index: test/clang-tidy/werrors-diagnostics.cpp
===
--- test/clang-tidy/werrors-diagnostics.cpp
+++ test/clang-tidy/werrors-diagnostics.cpp
@@ -1,11 +1,10 @@
-// RUN: clang-tidy %s -checks='-*,llvm-namespace-comment,clang-diagnostic*' \
-// RUN:   -- -Wunused-variable 2>&1 \
+// RUN: clang-tidy %s -checks='-*,llvm-namespace-comment,clang-diagnostic-unused-variable' -- 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-WARN -implicit-check-not='{{warning|error}}:'
-// RUN: not clang-tidy %s -checks='-*,llvm-namespace-comment,clang-diagnostic*' \
-// RUN:   -warnings-as-errors='clang-diagnostic*' -- -Wunused-variable 2>&1 \
+// RUN: not clang-tidy %s -checks='-*,llvm-namespace-comment,clang-diagnostic-unused-variable' \
+// RUN:   -warnings-as-errors='clang-diagnostic*' -- 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-WERR -implicit-check-not='{{warning|error}}:'
-// RUN: not clang-tidy %s -checks='-*,llvm-namespace-comment,clang-diagnostic*' \
-// RUN:   -warnings-as-errors='clang-diagnostic*' -quiet -- -Wunused-variable 2>&1 \
+// RUN: not clang-tidy %s -checks='-*,llvm-namespace-comment,clang-diagnostic-unused-variable' \
+// RUN:   -warnings-as-errors='clang-diagnostic*' -quiet -- 2>&1 \
 // RUN:   | FileCheck %s --check-prefix=CHECK-WERR-QUIET -implicit-check-not='{{warning|error}}:'
 
 void f() { int i; }
Index: test/clang-tidy/warning-check-aliases.cpp
===
--- /dev/null
+++ test/clang-tidy/warning-check-aliases.cpp
@@ -0,0 +1,19 @@
+// RUN: clang-tidy %s -checks='-*,clang-diagnostic-exceptions' -- 2>&1 | FileCheck -implicit-check-not='{{warning:|error:}}' %s
+// RUN: clang-tidy %s -checks='-*,cert-err54-cpp' -- 2>&1 | FileCheck -implicit-check-not='{{warning:|error:}}' -check-prefix=CHECK2 %s
+// RUN: clang-tidy %s -- 2>&1 | FileCheck -allow-empty -implicit-check-not='{{warning:|error:}}' -check-prefix=CHECK3 %s
+
+class B {};
+class D : public B {};
+
+void f() {
+  try {
+  } catch (B ) {
+  } catch (D ) {
+  }
+}
+
+//CHECK: :11:12: warning: exception of type 'D &' will be caught by earlier handler [clang-diagnostic-exceptions]
+//CHECK: :10:12: note: for type 'B &'
+
+//CHECK2: :11:12: warning: exception of type 'D &' will be caught by earlier handler [cert-err54-cpp]
+//CHECK2: :10:12: note: for type 'B &'
Index: test/clang-tidy/validate-check-names.cpp
===
--- test/clang-tidy/validate-check-names.cpp
+++ test/clang-tidy/validate-check-names.cpp
@@ -1,2 +1,2 @@
 // Check names may only contain alphanumeric characters, '-', '_', and '.'.
-// RUN: clang-tidy -checks=* -list-checks | grep '^' | cut -b5- | not grep -v '^[a-zA-Z0-9_.\-]\+$'
+// RUN: clang-tidy -checks=*,-clang-diagnostic* -list-checks | grep '^' | cut -b5- | not grep -v '^[a-zA-Z0-9_.\-]\+$'
Index: test/clang-tidy/misc-suspicious-semicolon-fail.cpp
===
--- test/clang-tidy/misc-suspicious-semicolon-fail.cpp
+++ test/clang-tidy/misc-suspicious-semicolon-fail.cpp
@@ -1,8 +1,8 @@
 // RUN: clang-tidy %s -checks="-*,misc-suspicious-semicolon" -- -DERROR 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=CHECK-ERROR \
 // RUN:   -implicit-check-not="{{warning|error}}:"
-// RUN: clang-tidy %s -checks="-*,misc-suspicious-semicolon,clang-diagnostic*" \
-// RUN:-- -DWERROR -Wno-everything -Werror=unused-variable 2>&1 \
+// RUN: not clang-tidy %s -checks="-*,misc-suspicious-semicolon,clang-diagnostic-unused-variable" \
+// RUN:   -warnings-as-errors=clang-diagnostic-unused-variable -- -DWERROR 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=CHECK-WERROR \
 // RUN:   -implicit-check-not="{{warning|error}}:"
 
@@ -19,7 +19,7 @@
   // CHECK-ERROR: :[[@LINE-1]]:8: error: expected ';' at end of declaration [clang-diagnostic-error]
 #elif WERROR
   int a;
-  // CHECK-WERROR: :[[@LINE-1]]:7: error: unused variable 'a' [clang-diagnostic-unused-variable]
+  // CHECK-WERROR: :[[@LINE-1]]:7: error: unused variable 'a' [clang-diagnostic-unused-variable,-warnings-as-errors]
 #else
 #error "One of ERROR or 

[PATCH] D41444: [ASTImporterTest] Make testing under '-fdelayed-template-parsing' mandatory

2017-12-20 Thread Zachary Turner via Phabricator via cfe-commits
zturner added a comment.

Can you just have `getLangArgs` return a vector of vectors, and then in 
`testImport` run it in a loop over all sets of args?


Repository:
  rC Clang

https://reviews.llvm.org/D41444



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


[PATCH] D41450: [clangd] Pull CodeCompletionString handling logic into its own file and add unit test.

2017-12-20 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.



Comment at: clangd/CompletionString.cpp:51
+  std::string Result;
+  Result.reserve(Text.size()); // Assume '$', '}' and '\\' are rare.
+  for (const auto Character : Text) {

if you actually care about performance, escapeSnippet should take the string to 
append to as a parameter



Comment at: clangd/CompletionString.cpp:67
+  for (const auto  : CCS) {
+// Informative qualifier chunks only clutter completion results, skip
+// them.

these comments are just copies of the enum documentation, mind removing them 
while here?



Comment at: clangd/CompletionString.cpp:118
+  break;
+case CodeCompletionString::CK_LeftParen:
+  // A left parenthesis ('(').

you've grouped these into a default case above - do the same here?



Comment at: clangd/CompletionString.h:1
+//===--- CompletionString.h --*- 
C++-*-===//
+//

nit: `CodeCompletionStrings`?

plural because it doesn't define the type, but operations on it.
Full name because we have too many ways to spell things already...



Comment at: clangd/CompletionString.h:25
+///
+/// If \p IsSnippet is not nullptr, this will try to use snippet for the insert
+/// text and sets `IsSnippet` to true when a snippet is created. Otherwise, the

This IsSnippet signature is clever, and matches the existing behavior (even if 
snippets are supported, we send plaintext if possible).

However that doesn't seem important to preserve - either snippets are supported 
or they're not, and the logic is simpler if we just tell getLabelAndInsertText 
what style we want.



Comment at: clangd/CompletionString.h:28
+/// insert text will always be plain text.
+std::pair
+getLabelAndInsertText(const CodeCompletionString ,

nit: std::pair is always hard to remember - can we take two out-params so 
code-completion can help us?



Comment at: clangd/CompletionString.h:34
+/// a class declaration.
+std::string getDocumentation(const CodeCompletionString );
+

I'm skeptical that CodeCompletionString is actually where we want to be 
generating documentation in the long run, vs something like Decl which will 
give us more flexibility. But this matches what we currently do and seems fine 
for now.



Comment at: clangd/CompletionString.h:37
+/// Gets detail to be used as the detail field in an LSP completion item. This
+/// is usually the result of a function.
+std::string getDetail(const CodeCompletionString );

nit: result -> return type



Comment at: unittests/clangd/CompletionStringTests.cpp:89
+
+TEST_F(CompletionStringTest, FunctionSnippet) {
+  Builder.AddResultTypeChunk("result no no");

I like the fine-grained tests of the features, but I'd also like to be able to 
see how these strings compare for more typical examples, like the one in this 
test.

Could you have a test (maybe this one or maybe a new one) where you build a 
typical function CCS, and then assert all the strings: label, insert text, 
filter text, with and without snippets...


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41450



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


[PATCH] D41365: [clang] Add BeforeExecute method to PrecompiledPreamble

2017-12-20 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LG


Repository:
  rC Clang

https://reviews.llvm.org/D41365



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


[PATCH] D41444: [ASTImporterTest] Make testing under '-fdelayed-template-parsing' mandatory

2017-12-20 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment.

In https://reviews.llvm.org/D41444#960999, @a.sidorin wrote:

> Also, I still think we should rather not apply template-related patches until 
> this testing is implemented. Gabor, Peter, do you agree?


Sure, I am fine with that.


Repository:
  rC Clang

https://reviews.llvm.org/D41444



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


[PATCH] D41345: [clangd] Add more symbol information for code completion.

2017-12-20 Thread Eric Liu via Phabricator via cfe-commits
ioeric added a comment.

Thanks for the review!

Logic around CodeCompletionString is pulled into a separate file in 
https://reviews.llvm.org/D41450.

I also propagate the new completion info to completion code. I am happy to 
split it out if it adds too much noise for the review.




Comment at: clangd/index/Index.h:92
 
+  // Documentation including comment for the symbol declaration.
+  std::string Documentation;

sammccall wrote:
> AFAIK this information isn't needed for retrieval/scoring, just for display.
> 
> LSP has `completionItem/resolve` that adds additional info to a completion 
> item. This allows us to avoid sending a bunch of bulky comments, most of 
> which will never be looked at.
> 
> In practice, there's nothing we particularly want to do differently for the 
> memory index: we have to load the data into memory, and so including a 
> pointer to it right away is no extra work.
> 
> However Symbol has two roles, and being the in-memory representation for 
> MemIndex is only secondary. Its primary role is defining the protocol between 
> indexes and clangd, including remote indexes where returning all 
> documentation *is* expensive.
> 
> One option is to have Symbol just have the "core stuff" that's suitable for 
> returning in bulk, and have an index method to retrieve more details that 
> would be a point lookup only. (Maybe this is just the getSymbol method we've 
> thought about). I'm not sure what it means for the data structure. OK if we 
> discuss offline?
As discussed offline, putting non-core stuff in an optional structure.



Comment at: clangd/index/Index.h:100
+  std::string CompletionDetail;
+  // The placeholder text for function parameters in order.
+  std::vector Params;

sammccall wrote:
> How are you planning to use this?
> 
> This seems to be related to the completion text/edits. We had some early 
> discussions about whether we'd encode something like CodeCompletionString, or 
> LSP snippets, or something else entirely.
> Honestly it would be great to have a doc describing this mapping between 
> source -> index -> LSP for completion data.
As discussed offline, we now store the whole snippet in the insertion text.



Comment at: clangd/index/SymbolCollector.cpp:61
+
+std::string getDocumentation(const CodeCompletionString ) {
+  // Things like __attribute__((nonnull(1,3))) and [[noreturn]]. Present this

sammccall wrote:
> it seems we'll want to share the(some?) doc logic between hover, AST-based 
> complete, and indexing... See D35894 (which is ... complicated, no idea if 
> it'll land soon).
> 
> Among other things:
>  - we may not want to make the logic too elaborate until we're able to merge 
> interfaces
>  - we may want to consume AST nodes rather than CCS in the long run
I pulled `CodeCompletionString` handling logic into a separate file in `D41450`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41345



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


[PATCH] D41444: [ASTImporterTest] Make testing under '-fdelayed-template-parsing' mandatory

2017-12-20 Thread Aleksei Sidorin via Phabricator via cfe-commits
a.sidorin added a comment.

In case of `-fdelayed-template-parsing`, this code won't be even visible to the 
Importer. In my opinion, we shouldn't care about code we're not going to 
import. If we want to import it, we should make it visible and instantiate it. 
In this case it will not compile.
However, I completely agree with the statement that testing of two options is 
better. The question is how to design unit testing for different command line 
options. I'll make a try and update the patch. Unfortunately, the new version 
is much bigger than the source patch. I'm not also sure that new design is 
scalable if we want to introduce more options in future. Any suggestions on 
this are welcome.
Also, I still think we should rather not apply template-related patches until 
this testing is implemented. Gabor, Peter, do you agree?


Repository:
  rC Clang

https://reviews.llvm.org/D41444



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


[PATCH] D41399: [CodeGen] Represent array members in new-format TBAA type descriptors

2017-12-20 Thread Ivan Kosarev via Phabricator via cfe-commits
kosarev added inline comments.



Comment at: test/CodeGen/tbaa-array.cpp:24
+// CHECK-DAG: [[TAG_A_i]] = !{[[TYPE_A:!.*]], [[TYPE_int:!.*]], i64 0, i64 4}
+// CHECK-DAG: [[TAG_C_i]] = !{[[TYPE_C:!.*]], [[TYPE_int:!.*]], i64 0, i64 16}
+// CHECK-DAG: [[TYPE_A]] = !{[[TYPE_char:!.*]], i64 4, !"_ZTS1A", 
[[TYPE_int]], i64 0, i64 4}

hfinkel wrote:
> Shouldn't this access have a size of 4, and an access for c->x[2] have a size 
> of 4 and a specific offset and c->x[j] have a size of 12 and an offset of 
> zero? Why does this list a size of 16?
> 
> In any case, please add tests for:
> 
>   int *bar2(C *c) {
> return c->x;
>   }
> 
>   int bar3(C *c) {
> return c->x[2];
>   }
> 
>   int bar4(C *c, int j) {
> return c->x[j];
>   }
> 
Indeed, the access size is wrong as we mistakenly inherit it from the base 
type. D41452 fixes this. Thanks for catching.


Repository:
  rL LLVM

https://reviews.llvm.org/D41399



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


[PATCH] D41345: [clangd] Add more symbol information for code completion.

2017-12-20 Thread Eric Liu via Phabricator via cfe-commits
ioeric updated this revision to Diff 127730.
ioeric marked 4 inline comments as done.
ioeric added a comment.

- Merge with origin/master
- Fixed an error in merge
- Make documentation etc optional in symbols
- Merge remote-tracking branch 'origin/master' into symbol
- Merge branch 'index-completion' into symbol
- Address review comments; merge with https://reviews.llvm.org/D41450.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41345

Files:
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/CodeComplete.cpp
  clangd/index/FileIndex.cpp
  clangd/index/Index.h
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/SymbolCollectorTests.cpp

Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -9,7 +9,6 @@
 
 #include "index/SymbolCollector.h"
 #include "index/SymbolYAML.h"
-#include "clang/Index/IndexingAction.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/FileSystemOptions.h"
 #include "clang/Basic/VirtualFileSystem.h"
@@ -26,11 +25,25 @@
 #include 
 #include 
 
+using testing::AllOf;
 using testing::Eq;
 using testing::Field;
 using testing::UnorderedElementsAre;
 
 // GMock helpers for matching Symbol.
+MATCHER_P(Labeled, Label, "") { return arg.second.CompletionLabel == Label; }
+MATCHER_P(Detail, D, "") {
+  return arg.second.Detail && arg.second.Detail->CompletionDetail == D;
+}
+MATCHER_P(Doc, D, "") {
+  return arg.second.Detail && arg.second.Detail->Documentation == D;
+}
+MATCHER_P(Plain, Text, "") {
+  return arg.second.CompletionPlainInsertText == Text;
+}
+MATCHER_P(Snippet, S, "") {
+  return arg.second.CompletionSnippetInsertText == S;
+}
 MATCHER_P(QName, Name, "") {
   return (arg.second.Scope + (arg.second.Scope.empty() ? "" : "::") +
   arg.second.Name) == Name;
@@ -110,6 +123,38 @@
 QName("f1"), QName("f2")));
 }
 
+TEST_F(SymbolCollectorTest, SymbolWithDocumentation) {
+  const std::string Main = R"(
+namespace nx {
+/// Foo comment.
+int ff(int x, double y) { return 0; }
+}
+  )";
+  runSymbolCollector(/*Header=*/"", Main);
+  EXPECT_THAT(Symbols,
+  UnorderedElementsAre(QName("nx"),
+   AllOf(QName("nx::ff"),
+ Labeled("ff(int x, double y)"),
+ Detail("int"), Doc("Foo comment.";
+}
+
+TEST_F(SymbolCollectorTest, PlainAndSnippet) {
+  const std::string Main = R"(
+namespace nx {
+void f() {}
+int ff(int x, double y) { return 0; }
+}
+  )";
+  runSymbolCollector(/*Header=*/"", Main);
+  EXPECT_THAT(
+  Symbols,
+  UnorderedElementsAre(
+  QName("nx"),
+  AllOf(QName("nx::f"), Labeled("f()"), Plain("f()"), Snippet("")),
+  AllOf(QName("nx::ff"), Labeled("ff(int x, double y)"), Plain("ff"),
+Snippet("ff(${1:int x}, ${2:double y})";
+}
+
 TEST_F(SymbolCollectorTest, YAMLConversions) {
   const std::string YAML1 = R"(
 ---
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -86,6 +86,8 @@
 MATCHER_P(Labeled, Label, "") { return arg.label == Label; }
 MATCHER_P(Kind, K, "") { return arg.kind == K; }
 MATCHER_P(Filter, F, "") { return arg.filterText == F; }
+MATCHER_P(Doc, D, "") { return arg.documentation == D; }
+MATCHER_P(Detail, D, "") { return arg.detail == D; }
 MATCHER_P(PlainText, Text, "") {
   return arg.insertTextFormat == clangd::InsertTextFormat::PlainText &&
  arg.insertText == Text;
@@ -480,6 +482,7 @@
   Sym.Name = QName.substr(Pos + 2);
   Sym.Scope = QName.substr(0, Pos);
 }
+Sym.CompletionPlainInsertText = Sym.Name;
 Sym.SymInfo.Kind = Pair.second;
 Snap->Slab.insert(std::move(Sym));
   }
@@ -530,7 +533,7 @@
   void f() { ns::x^ }
   )cpp",
  Opts);
-  EXPECT_THAT(Results.items, Contains(AllOf(Named("XYZ"), Filter("x";
+  EXPECT_THAT(Results.items, Contains(Named("XYZ")));
   EXPECT_THAT(Results.items, Not(Has("foo")));
 }
 
@@ -568,13 +571,17 @@
 
   Server
   .addDocument(Context::empty(), getVirtualTestFilePath("foo.cpp"), R"cpp(
-  namespace ns { class XYZ {}; void foo() {} }
+  namespace ns { class XYZ {}; void foo(int x) {} }
   )cpp")
   .wait();
 
   auto File = getVirtualTestFilePath("bar.cpp");
   auto Test = parseTextMarker(R"cpp(
-  namespace ns { class XXX {}; void fo() {} }
+  namespace ns {
+  class XXX {};
+  /// Doooc
+  void fo() {}
+  }
   void f() { ns::^ }
   )cpp");
   Server.addDocument(Context::empty(), File, Test.Text).wait();
@@ -587,7 +594,9 @@
   

[PATCH] D41432: [clangd] Switch xrefs and documenthighlight to annotated-code unit tests. NFC

2017-12-20 Thread Sam McCall via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
sammccall marked 2 inline comments as done.
Closed by commit rL321184: [clangd] Switch xrefs and documenthighlight to 
annotated-code unit tests. NFC (authored by sammccall, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D41432?vs=127674=127728#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D41432

Files:
  clang-tools-extra/trunk/test/clangd/definitions.test
  clang-tools-extra/trunk/test/clangd/documenthighlight.test
  clang-tools-extra/trunk/test/clangd/xrefs.test
  clang-tools-extra/trunk/unittests/clangd/Annotations.cpp
  clang-tools-extra/trunk/unittests/clangd/Annotations.h
  clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
  clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
  clang-tools-extra/trunk/unittests/clangd/Matchers.h
  clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp

Index: clang-tools-extra/trunk/test/clangd/xrefs.test
===
--- clang-tools-extra/trunk/test/clangd/xrefs.test
+++ clang-tools-extra/trunk/test/clangd/xrefs.test
@@ -0,0 +1,67 @@
+# RUN: clangd -pretty -run-synchronously < %s | FileCheck -strict-whitespace %s
+# It is absolutely vital that this file has CRLF line endings.
+#
+Content-Length: 125
+
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+
+Content-Length: 165
+
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///main.cpp","languageId":"cpp","version":1,"text":"int x = 0;\nint y = x;"}}}
+
+Content-Length: 148
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/definition","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":1,"character":8}}}
+#  CHECK:  "id": 1,
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT:  "character": 9,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT:  "character": 0,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:}
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
+# CHECK-NEXT:}
+# CHECK-NEXT:  ]
+Content-Length: 155
+
+{"jsonrpc":"2.0","id":1,"method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":1,"character":8}}}
+#  CHECK: "id": 1
+# CHECK-NEXT: "jsonrpc": "2.0",
+# CHECK-NEXT: "result": [
+# CHECK-NEXT:   {
+# CHECK-NEXT: "kind": 1,
+# CHECK-NEXT: "range": {
+# CHECK-NEXT:   "end": {
+# CHECK-NEXT: "character": 5,
+# CHECK-NEXT: "line": 0
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "start": {
+# CHECK-NEXT: "character": 4,
+# CHECK-NEXT: "line": 0
+# CHECK-NEXT:   }
+# CHECK-NEXT: }
+# CHECK-NEXT:   },
+# CHECK-NEXT:   {
+# CHECK-NEXT: "kind": 2,
+# CHECK-NEXT: "range": {
+# CHECK-NEXT:   "end": {
+# CHECK-NEXT: "character": 9,
+# CHECK-NEXT: "line": 1
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "start": {
+# CHECK-NEXT: "character": 8,
+# CHECK-NEXT: "line": 1
+# CHECK-NEXT:   }
+# CHECK-NEXT: }
+# CHECK-NEXT:   }
+# CHECK-NEXT: ]
+Content-Length: 48
+
+{"jsonrpc":"2.0","id":1,"method":"shutdown"}
Index: clang-tools-extra/trunk/unittests/clangd/Matchers.h
===
--- clang-tools-extra/trunk/unittests/clangd/Matchers.h
+++ clang-tools-extra/trunk/unittests/clangd/Matchers.h
@@ -13,6 +13,7 @@
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_MATCHERS_H
 #define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_MATCHERS_H
+#include "Protocol.h"
 #include "gmock/gmock.h"
 
 namespace clang {
Index: clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
@@ -0,0 +1,218 @@
+//===-- XRefsTests.cpp  ---*- C++ -*--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+#include "Annotations.h"
+#include "ClangdUnit.h"
+#include "Matchers.h"
+#include "XRefs.h"
+#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/PCHContainerOperations.h"
+#include "clang/Frontend/Utils.h"
+#include "llvm/Support/Path.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+using namespace llvm;
+
+void PrintTo(const DocumentHighlight , std::ostream *O) {
+  llvm::raw_os_ostream OS(*O);
+  OS << 

[clang-tools-extra] r321184 - [clangd] Switch xrefs and documenthighlight to annotated-code unit tests. NFC

2017-12-20 Thread Sam McCall via cfe-commits
Author: sammccall
Date: Wed Dec 20 08:06:05 2017
New Revision: 321184

URL: http://llvm.org/viewvc/llvm-project?rev=321184=rev
Log:
[clangd] Switch xrefs and documenthighlight to annotated-code unit tests. NFC

Summary:
The goal here is again to make it easier to read and write the tests.

I've extracted `parseTextMarker` from CodeCompleteTests into an `Annotations`
class, adding features to it:
  - as well as points `^s` it allows ranges `[[...]]`
  - multiple points and ranges are supported
  - points and ranges may be named: `$name^` and `$name[[...]]`

These features are used for the xrefs tests. This also paves the way for
replacing the lit diagnostics.test with more readable unit tests, using named
ranges.

Alternative considered: `TestSelectionRange` in clang-refactor/TestSupport
Main problems were:
 - delimiting the end of ranges is awkward, requiring counting
 - comment syntax is long and at least as cryptic for most cases
 - no separate syntax for point vs range, which keeps xrefs tests concise
 - Still need to convert to Position everywhere
 - Still need helpers for common case of expecting exactly one point/range

(I'll probably promote the extra `PrintTo`s from some of the core Protocol types
into `operator<<` in `Protocol.h` itself in a separate, prior patch...)

Reviewers: ioeric

Subscribers: klimek, mgorny, ilya-biryukov, cfe-commits

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

Added:
clang-tools-extra/trunk/test/clangd/xrefs.test
clang-tools-extra/trunk/unittests/clangd/Annotations.cpp
clang-tools-extra/trunk/unittests/clangd/Annotations.h
clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
Removed:
clang-tools-extra/trunk/test/clangd/definitions.test
clang-tools-extra/trunk/test/clangd/documenthighlight.test
Modified:
clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
clang-tools-extra/trunk/unittests/clangd/Matchers.h

Removed: clang-tools-extra/trunk/test/clangd/definitions.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/definitions.test?rev=321183=auto
==
--- clang-tools-extra/trunk/test/clangd/definitions.test (original)
+++ clang-tools-extra/trunk/test/clangd/definitions.test (removed)
@@ -1,421 +0,0 @@
-# RUN: clangd -pretty -run-synchronously < %s | FileCheck -strict-whitespace %s
-# It is absolutely vital that this file has CRLF line endings.
-#
-Content-Length: 125
-
-{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
-
-Content-Length: 172
-
-{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file:///main.cpp","languageId":"cpp","version":1,"text":"int
 main() {\nint a;\na;\n}\n"}}}
-
-Content-Length: 148
-
-{"jsonrpc":"2.0","id":1,"method":"textDocument/definition","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":2,"character":0}}}
-# Go to local variable
-#  CHECK:  "id": 1,
-# CHECK-NEXT:  "jsonrpc": "2.0",
-# CHECK-NEXT:  "result": [
-# CHECK-NEXT:{
-# CHECK-NEXT:  "range": {
-# CHECK-NEXT:"end": {
-# CHECK-NEXT:  "character": 5,
-# CHECK-NEXT:  "line": 1
-# CHECK-NEXT:},
-# CHECK-NEXT:"start": {
-# CHECK-NEXT:  "character": 0,
-# CHECK-NEXT:  "line": 1
-# CHECK-NEXT:}
-# CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
-# CHECK-NEXT:}
-# CHECK-NEXT:  ]
-Content-Length: 148
-
-{"jsonrpc":"2.0","id":1,"method":"textDocument/definition","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":2,"character":1}}}
-# Go to local variable, end of token
-#  CHECK:  "id": 1,
-# CHECK-NEXT:  "jsonrpc": "2.0",
-# CHECK-NEXT:  "result": [
-# CHECK-NEXT:{
-# CHECK-NEXT:  "range": {
-# CHECK-NEXT:"end": {
-# CHECK-NEXT:  "character": 5,
-# CHECK-NEXT:  "line": 1
-# CHECK-NEXT:},
-# CHECK-NEXT:"start": {
-# CHECK-NEXT:  "character": 0,
-# CHECK-NEXT:  "line": 1
-# CHECK-NEXT:}
-# CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file:///{{([A-Z]:/)?}}main.cpp"
-# CHECK-NEXT:}
-# CHECK-NEXT:  ]
-Content-Length: 214
-
-{"jsonrpc":"2.0","method":"textDocument/didChange","params":{"textDocument":{"uri":"file:///main.cpp","version":2},"contentChanges":[{"text":"struct
 Foo {\nint x;\n};\nint main() {\n  Foo bar = { x : 1 };\n}\n"}]}}
-
-Content-Length: 149
-
-{"jsonrpc":"2.0","id":1,"method":"textDocument/definition","params":{"textDocument":{"uri":"file:///main.cpp"},"position":{"line":4,"character":14}}}
-# Go to field, GNU old-style field designator 
-#  CHECK:  "id": 1,
-# CHECK-NEXT:  "jsonrpc": "2.0",
-# CHECK-NEXT:  "result": [
-# CHECK-NEXT:{
-# CHECK-NEXT:  "range": {
-# CHECK-NEXT:"end": {
-# CHECK-NEXT:

[PATCH] D41452: [CodeGen] Fix access sizes in new-format TBAA tags

2017-12-20 Thread Ivan Kosarev via Phabricator via cfe-commits
kosarev created this revision.
kosarev added reviewers: rjmccall, hfinkel.
kosarev added a project: clang.

The new format requires to specify both the type of the access and its size. 
This patch fixes setting access sizes for TBAA tags that denote accesses to 
structure members. This fix affects all future TBAA metadata tests for the new 
format, so I guess we don't need any special tests for this fix.


Repository:
  rL LLVM

https://reviews.llvm.org/D41452

Files:
  lib/CodeGen/CGExpr.cpp


Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -3790,8 +3790,10 @@
   FieldTBAAInfo.Offset +=
   Layout.getFieldOffset(field->getFieldIndex()) / CharWidth;
 
-// Update the final access type.
+// Update the final access type and size.
 FieldTBAAInfo.AccessType = CGM.getTBAATypeInfo(FieldType);
+FieldTBAAInfo.Size =
+getContext().getTypeSizeInChars(FieldType).getQuantity();
   }
 
   Address addr = base.getAddress();


Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -3790,8 +3790,10 @@
   FieldTBAAInfo.Offset +=
   Layout.getFieldOffset(field->getFieldIndex()) / CharWidth;
 
-// Update the final access type.
+// Update the final access type and size.
 FieldTBAAInfo.AccessType = CGM.getTBAATypeInfo(FieldType);
+FieldTBAAInfo.Size =
+getContext().getTypeSizeInChars(FieldType).getQuantity();
   }
 
   Address addr = base.getAddress();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D41432: [clangd] Switch xrefs and documenthighlight to annotated-code unit tests. NFC

2017-12-20 Thread Sam McCall via Phabricator via cfe-commits
sammccall marked 2 inline comments as done.
sammccall added inline comments.



Comment at: unittests/clangd/Annotations.h:12
+//
+//Annotations Example(R"cpp(
+//   int complete() { x.pri^ }  // ^ indicates a point

ioeric wrote:
> Does this support overlapping annotations like (named) points in (named) 
> ranges?
Yes - ranges can be nested, but not "overlapping" like `[a(b]c)`.
Added a comment.



Comment at: unittests/clangd/Annotations.h:45
+  // Crashes if there isn't exactly one.
+  Position point(llvm::StringRef Name = "") const;
+  // Returns the position of all points marked by ^ (or $name^) in the text.

ioeric wrote:
> Is this the position in the stripped code or the original text?
In the stripped code, which is what you'll feed to the compiler. Added a 
comment.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41432



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


[PATCH] D41451: Make DiagnosticIDs::getAllDiagnostics use std::vector

2017-12-20 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.

LGTM!


https://reviews.llvm.org/D41451



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


[PATCH] D41451: Make DiagnosticIDs::getAllDiagnostics use std::vector

2017-12-20 Thread András Leitereg via Phabricator via cfe-commits
leanil created this revision.
leanil added reviewers: rsmith, xazax.hun.
Herald added a subscriber: rnkovacs.

The size of the result vector is currently around 4600 with 
Flavor::WarningOrError, which makes std::vector a better candidate than 
llvm::SmallVector.


https://reviews.llvm.org/D41451

Files:
  include/clang/Basic/DiagnosticIDs.h
  lib/Basic/Diagnostic.cpp
  lib/Basic/DiagnosticIDs.cpp


Index: lib/Basic/DiagnosticIDs.cpp
===
--- lib/Basic/DiagnosticIDs.cpp
+++ lib/Basic/DiagnosticIDs.cpp
@@ -583,7 +583,7 @@
 }
 
 void DiagnosticIDs::getAllDiagnostics(diag::Flavor Flavor,
-  SmallVectorImpl ) {
+  std::vector ) {
   for (unsigned i = 0; i != StaticDiagInfoSize; ++i)
 if (StaticDiagInfo[i].getFlavor() == Flavor)
   Diags.push_back(StaticDiagInfo[i].DiagID);
Index: lib/Basic/Diagnostic.cpp
===
--- lib/Basic/Diagnostic.cpp
+++ lib/Basic/Diagnostic.cpp
@@ -363,7 +363,7 @@
   diag::Severity Map,
   SourceLocation Loc) {
   // Get all the diagnostics.
-  SmallVector AllDiags;
+  std::vector AllDiags;
   DiagnosticIDs::getAllDiagnostics(Flavor, AllDiags);
 
   // Set the mapping.
Index: include/clang/Basic/DiagnosticIDs.h
===
--- include/clang/Basic/DiagnosticIDs.h
+++ include/clang/Basic/DiagnosticIDs.h
@@ -297,7 +297,7 @@
 
   /// \brief Get the set of all diagnostic IDs.
   static void getAllDiagnostics(diag::Flavor Flavor,
-SmallVectorImpl );
+std::vector );
 
   /// \brief Get the diagnostic option with the closest edit distance to the
   /// given group name.


Index: lib/Basic/DiagnosticIDs.cpp
===
--- lib/Basic/DiagnosticIDs.cpp
+++ lib/Basic/DiagnosticIDs.cpp
@@ -583,7 +583,7 @@
 }
 
 void DiagnosticIDs::getAllDiagnostics(diag::Flavor Flavor,
-  SmallVectorImpl ) {
+  std::vector ) {
   for (unsigned i = 0; i != StaticDiagInfoSize; ++i)
 if (StaticDiagInfo[i].getFlavor() == Flavor)
   Diags.push_back(StaticDiagInfo[i].DiagID);
Index: lib/Basic/Diagnostic.cpp
===
--- lib/Basic/Diagnostic.cpp
+++ lib/Basic/Diagnostic.cpp
@@ -363,7 +363,7 @@
   diag::Severity Map,
   SourceLocation Loc) {
   // Get all the diagnostics.
-  SmallVector AllDiags;
+  std::vector AllDiags;
   DiagnosticIDs::getAllDiagnostics(Flavor, AllDiags);
 
   // Set the mapping.
Index: include/clang/Basic/DiagnosticIDs.h
===
--- include/clang/Basic/DiagnosticIDs.h
+++ include/clang/Basic/DiagnosticIDs.h
@@ -297,7 +297,7 @@
 
   /// \brief Get the set of all diagnostic IDs.
   static void getAllDiagnostics(diag::Flavor Flavor,
-SmallVectorImpl );
+std::vector );
 
   /// \brief Get the diagnostic option with the closest edit distance to the
   /// given group name.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


  1   2   >