[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

[Dominic Ferreira via Phabricator via cfe-commits]

domdom updated this revision to Diff 183043.
domdom added a comment.

Formatting


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57086/new/

https://reviews.llvm.org/D57086

Files:
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/statements.c


Index: clang/test/Sema/statements.c
===
--- clang/test/Sema/statements.c
+++ clang/test/Sema/statements.c
@@ -119,3 +119,17 @@
 SIZE = sizeof(({unsigned long __ptr; __ptr;}))
   };
 }
+
+// GCC ignores empty statements at the end of compound expressions where the
+// result type is concerned.
+void test13() {
+  int a;
+  a = ({1;});
+  a = ({1;;});
+  a = ({int x = 1; (void)x;}); // expected-error {{assigning to 'int' from 
incompatible type 'void'}}
+  a = ({int x = 1; (void)x;;}); // expected-error {{assigning to 'int' from 
incompatible type 'void'}}
+}
+
+void test14() { return ({}); }
+void test15() { return ({}); }
+void test16() { return ({test:;;}); }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -13320,11 +13320,22 @@
   // More semantic analysis is needed.
 
   // If there are sub-stmts in the compound stmt, take the type of the last one
-  // as the type of the stmtexpr.
+  // as the type of the stmtexpr. For GCC compatibility this excludes trailing
+  // NullStmts
   QualType Ty = Context.VoidTy;
   bool StmtExprMayBindToTemp = false;
   if (!Compound->body_empty()) {
-Stmt *LastStmt = Compound->body_back();
+// GCC ignores empty statements at the end of compound expressions
+// i.e. ({ 5;;; })
+//   ^^ ignored
+// This code skips past these NullStmts
+Stmt *LastStmt = nullptr;
+for (Stmt *I : llvm::make_range(Compound->body_rbegin(),
+Compound->body_rend())) {
+  LastStmt = I;
+  if (!isa(LastStmt))
+break;
+}
 LabelStmt *LastLabelStmt = nullptr;
 // If LastStmt is a label, skip down through into the body.
 while (LabelStmt *Label = dyn_cast(LastStmt)) {
Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -959,10 +959,18 @@
 
 bool Parser::isExprValueDiscarded() {
   if (Actions.isCurCompoundStmtAStmtExpr()) {
-// Look to see if the next two tokens close the statement expression;
-// if so, this expression statement is the last statement in a
-// statment expression.
-return Tok.isNot(tok::r_brace) || NextToken().isNot(tok::r_paren);
+// For gcc compatibility we skip past NullStmts
+int LookAhead = 0;
+while (GetLookAheadToken(LookAhead).is(tok::semi)) {
+  LookAhead++;
+}
+
+// Then look to see if the next two tokens close the statement expression;
+// if so, this expression statement is the last statement in a statment
+// expression.
+
+return GetLookAheadToken(LookAhead).isNot(tok::r_brace) ||
+   GetLookAheadToken(LookAhead + 1).isNot(tok::r_paren);
   }
   return true;
 }


Index: clang/test/Sema/statements.c
===
--- clang/test/Sema/statements.c
+++ clang/test/Sema/statements.c
@@ -119,3 +119,17 @@
 SIZE = sizeof(({unsigned long __ptr; __ptr;}))
   };
 }
+
+// GCC ignores empty statements at the end of compound expressions where the
+// result type is concerned.
+void test13() {
+  int a;
+  a = ({1;});
+  a = ({1;;});
+  a = ({int x = 1; (void)x;}); // expected-error {{assigning to 'int' from incompatible type 'void'}}
+  a = ({int x = 1; (void)x;;}); // expected-error {{assigning to 'int' from incompatible type 'void'}}
+}
+
+void test14() { return ({}); }
+void test15() { return ({}); }
+void test16() { return ({test:;;}); }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -13320,11 +13320,22 @@
   // More semantic analysis is needed.
 
   // If there are sub-stmts in the compound stmt, take the type of the last one
-  // as the type of the stmtexpr.
+  // as the type of the stmtexpr. For GCC compatibility this excludes trailing
+  // NullStmts
   QualType Ty = Context.VoidTy;
   bool StmtExprMayBindToTemp = false;
   if (!Compound->body_empty()) {
-Stmt *LastStmt = Compound->body_back();
+// GCC ignores empty statements at the end of compound expressions
+// i.e. ({ 5;;; })
+//   ^^ ignored
+// This code skips past these NullStmts
+Stmt *LastStmt = nullptr;
+for (Stmt *I : llvm::make_range(Compound->body_rbegin(),
+Compound->body_rend())) {
+  LastStmt = I;
+  if (!isa(LastStmt))
+break;
+}
 LabelStmt *LastLabelStmt = nullptr;
 // If 

Re: r351701 - Replace llvm::isPodLike<...> by llvm::is_trivially_copyable<...>

[Axel Naumann via cfe-commits]

This got fixed in r351820.

Thanks, Serge!

Axel.

On 1/23/19 5:56 AM, Hubert Tong wrote:
> I am also hitting this. GCC 4.8 is still the minimum at this time.
>
> -- HT
>
> On Tue, Jan 22, 2019 at 8:10 AM Axel Naumann via cfe-commits
> mailto:cfe-commits@lists.llvm.org>> wrote:
>
> Hi,
>
> This broke our clang builds with
>
> $ gcc --version
> gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-36)
>
> on CentOS Linux release 7.6.1810 (Core),
>
> [ 23%] Building CXX object
> tools/clang/lib/Basic/CMakeFiles/clangBasic.dir/Module.cpp.o
>
> In file included from include/llvm/ADT/StringMap.h:20:0,
>                  from include/llvm/Support/Host.h:16,
>                  from include/llvm/ADT/Hashing.h:48,
>                  from include/llvm/ADT/ArrayRef.h:12,
>                  from include/llvm/ADT/DenseMapInfo.h:16,
>                  from include/llvm/ADT/DenseMap.h:16,
>                  from
> tools/clang/include/clang/Basic/FileManager.h:19,
>                  from tools/clang/include/clang/Basic/Module.h:18,
>                  from tools/clang/lib/Basic/Module.cpp:14:
> include/llvm/Support/PointerLikeTypeTraits.h: In instantiation of
> ‘struct llvm::PointerLikeTypeTraits’:
> /usr/include/c++/4.8.2/type_traits:1087:41:   required by substitution
> of ‘template static decltype
> (((declval<_Tp1>)()=(declval<_Up1>)(), std::__sfinae_types::__one()))
> std::__is_assignable_helper<_Tp, _Up>::__test(int) [with _Tp1 = _Tp1;
> _Up1 = _Up1; _Tp =
> llvm::detail::trivial_helper bool> >&; _Up = const
> llvm::detail::trivial_helper bool> >&] [with _Tp1 =
> llvm::detail::trivial_helper bool> >&; _Up1 = const
> llvm::detail::trivial_helper bool> >&]’
> /usr/include/c++/4.8.2/type_traits:1094:50:   required from ‘constexpr
> const bool
> 
> std::__is_assignable_helper 1u, bool> >&, const
> llvm::detail::trivial_helper bool> >&>::value’
> /usr/include/c++/4.8.2/type_traits:1099:12:   required from ‘struct
> 
> std::is_assignable 1u, bool> >&, const
> llvm::detail::trivial_helper bool> >&>’
> /usr/include/c++/4.8.2/type_traits:1112:12:   required from ‘struct
> 
> std::__is_copy_assignable_impl 1u, bool> >, false>’
> /usr/include/c++/4.8.2/type_traits:1118:12:   required from ‘struct
> 
> std::is_copy_assignable 1u, bool> > >’
> include/llvm/Support/type_traits.h:142:25:   required from ‘constexpr
> const bool
> llvm::is_trivially_copyable bool> >::has_trivial_copy_assign’
> include/llvm/Support/type_traits.h:163:32:   required from ‘constexpr
> const bool
> llvm::is_trivially_copyable bool> >::value’
> include/llvm/ADT/SmallVector.h:321:7:   required from ‘class
> llvm::SmallVectorImpl bool> >’
> include/llvm/ADT/SmallVector.h:845:7:   required from ‘class
> llvm::SmallVector, 2u>’
> tools/clang/include/clang/Basic/Module.h:290:30:   required from here
> include/llvm/Support/PointerLikeTypeTraits.h:59:8: error: invalid
> application of ‘__alignof__’ to incomplete type ‘clang::Module’
>    enum { NumLowBitsAvailable =
> detail::ConstantLog2::value };
>         ^
>
> FYI in case you wonder:
>
> $ ls -l /usr/include/c++/
> total 8
> drwxr-xr-x. 12 root root 4096 Dec 11 03:24 4.8.2
> lrwxrwxrwx.  1 root root    5 Dec 11 03:24 4.8.5 -> 4.8.2
>
>
> Are we outside the "allowed" range for GCC versions?
>
> Cheers, Axel.
>
> On 1/20/19 10:19 PM, Serge Guelton via cfe-commits wrote:
> > Author: serge_sans_paille
> > Date: Sun Jan 20 13:19:56 2019
> > New Revision: 351701
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=351701=rev
> > Log:
> > Replace llvm::isPodLike<...>  by llvm::is_trivially_copyable<...>
> >
> > As noted in https://bugs.llvm.org/show_bug.cgi?id=36651, the
> specialization for
> > isPodLike> did not match the expectation of
> > std::is_trivially_copyable which makes the memcpy optimization
> invalid.
> >
> > This patch renames the llvm::isPodLike trait into
> llvm::is_trivially_copyable.
> > Unfortunately std::is_trivially_copyable is not portable across
> compiler / STL
> > versions. So a portable version is provided too.
> >
> > Note that the following specialization were invalid:
> >
> >     std::pair
> >     llvm::Optional
> >
> > Tests have been added to assert that former specialization are
> respected by the
> > standard usage of llvm::is_trivially_copyable, and that when a
> decent version
> > of std::is_trivially_copyable is available,
> llvm::is_trivially_copyable is
> > compared to std::is_trivially_copyable.
> >
> > As of this patch, llvm::Optional is no longer considered
> trivially copyable,
> > even if T is. This is to be fixed in a later patch, as it has
>

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

[Dominic Ferreira via Phabricator via cfe-commits]

domdom created this revision.
domdom added reviewers: lattner, rsmith.
Herald added a subscriber: cfe-commits.

Ignore trailing NullStmts in compound expressions when determining the result 
type and value. This is to match the GCC behavior which ignores semicolons at 
the end of compound expressions.


Repository:
  rC Clang

https://reviews.llvm.org/D57086

Files:
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/statements.c


Index: clang/test/Sema/statements.c
===
--- clang/test/Sema/statements.c
+++ clang/test/Sema/statements.c
@@ -119,3 +119,17 @@
 SIZE = sizeof(({unsigned long __ptr; __ptr;}))
   };
 }
+
+// GCC ignores empty statements at the end of compound expressions where the
+// result type is concerned.
+void test13() {
+  int a;
+  a = ({1;});
+  a = ({1;;});
+  a = ({int x = 1; (void)x;}); // expected-error {{assigning to 'int' from 
incompatible type 'void'}}
+  a = ({int x = 1; (void)x;;}); // expected-error {{assigning to 'int' from 
incompatible type 'void'}}
+}
+
+void test14() { return ({}); }
+void test15() { return ({}); }
+void test16() { return ({test:;;}); }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -13320,11 +13320,22 @@
   // More semantic analysis is needed.
 
   // If there are sub-stmts in the compound stmt, take the type of the last one
-  // as the type of the stmtexpr.
+  // as the type of the stmtexpr. For GCC compatibility this excludes trailing
+  // NullStmts
   QualType Ty = Context.VoidTy;
   bool StmtExprMayBindToTemp = false;
   if (!Compound->body_empty()) {
-Stmt *LastStmt = Compound->body_back();
+// GCC ignores empty statements at the end of compound expressions
+// i.e. ({ 5;;; })
+//   ^^ ignored
+// This code skips past these NullStmts
+Stmt *LastStmt = nullptr;
+for (Stmt *I : llvm::make_range(Compound->body_rbegin(),
+Compound->body_rend())) {
+  LastStmt = I;
+  if (!isa(LastStmt))
+break;
+}
 LabelStmt *LastLabelStmt = nullptr;
 // If LastStmt is a label, skip down through into the body.
 while (LabelStmt *Label = dyn_cast(LastStmt)) {
Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -959,10 +959,18 @@
 
 bool Parser::isExprValueDiscarded() {
   if (Actions.isCurCompoundStmtAStmtExpr()) {
-// Look to see if the next two tokens close the statement expression;
-// if so, this expression statement is the last statement in a
-// statment expression.
-return Tok.isNot(tok::r_brace) || NextToken().isNot(tok::r_paren);
+// For gcc compatibility we skip past NullStmts
+int lookahead = 0;
+while(GetLookAheadToken(lookahead).is(tok::semi)) {
+  lookahead++;
+}
+
+// Then look to see if the next two tokens close the statement expression;
+// if so, this expression statement is the last statement in a statment
+// expression.
+
+return GetLookAheadToken(lookahead).isNot(tok::r_brace) ||
+   GetLookAheadToken(lookahead + 1).isNot(tok::r_paren);
   }
   return true;
 }


Index: clang/test/Sema/statements.c
===
--- clang/test/Sema/statements.c
+++ clang/test/Sema/statements.c
@@ -119,3 +119,17 @@
 SIZE = sizeof(({unsigned long __ptr; __ptr;}))
   };
 }
+
+// GCC ignores empty statements at the end of compound expressions where the
+// result type is concerned.
+void test13() {
+  int a;
+  a = ({1;});
+  a = ({1;;});
+  a = ({int x = 1; (void)x;}); // expected-error {{assigning to 'int' from incompatible type 'void'}}
+  a = ({int x = 1; (void)x;;}); // expected-error {{assigning to 'int' from incompatible type 'void'}}
+}
+
+void test14() { return ({}); }
+void test15() { return ({}); }
+void test16() { return ({test:;;}); }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -13320,11 +13320,22 @@
   // More semantic analysis is needed.
 
   // If there are sub-stmts in the compound stmt, take the type of the last one
-  // as the type of the stmtexpr.
+  // as the type of the stmtexpr. For GCC compatibility this excludes trailing
+  // NullStmts
   QualType Ty = Context.VoidTy;
   bool StmtExprMayBindToTemp = false;
   if (!Compound->body_empty()) {
-Stmt *LastStmt = Compound->body_back();
+// GCC ignores empty statements at the end of compound expressions
+// i.e. ({ 5;;; })
+//   ^^ ignored
+// This code skips past these NullStmts
+Stmt *LastStmt = nullptr;
+for (Stmt *I : llvm::make_range(Compound->body_rbegin(),
+  

[clang-tools-extra] r351925 - [doc] Fix svn property for bugprone-parent-virtual-call.rst

[Zinovy Nis via cfe-commits]

Author: zinovy.nis
Date: Tue Jan 22 22:46:27 2019
New Revision: 351925

URL: http://llvm.org/viewvc/llvm-project?rev=351925=rev
Log:
[doc] Fix svn property for bugprone-parent-virtual-call.rst

Modified:

clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-parent-virtual-call.rst 
  (props changed)

Propchange: 
clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-parent-virtual-call.rst
--
--- svn:executable (original)
+++ svn:executable (removed)
@@ -1 +0,0 @@
-*


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

Re: r351701 - Replace llvm::isPodLike<...> by llvm::is_trivially_copyable<...>

[Hubert Tong via cfe-commits]

I am also hitting this. GCC 4.8 is still the minimum at this time.

-- HT

On Tue, Jan 22, 2019 at 8:10 AM Axel Naumann via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Hi,
>
> This broke our clang builds with
>
> $ gcc --version
> gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-36)
>
> on CentOS Linux release 7.6.1810 (Core),
>
> [ 23%] Building CXX object
> tools/clang/lib/Basic/CMakeFiles/clangBasic.dir/Module.cpp.o
>
> In file included from include/llvm/ADT/StringMap.h:20:0,
>  from include/llvm/Support/Host.h:16,
>  from include/llvm/ADT/Hashing.h:48,
>  from include/llvm/ADT/ArrayRef.h:12,
>  from include/llvm/ADT/DenseMapInfo.h:16,
>  from include/llvm/ADT/DenseMap.h:16,
>  from tools/clang/include/clang/Basic/FileManager.h:19,
>  from tools/clang/include/clang/Basic/Module.h:18,
>  from tools/clang/lib/Basic/Module.cpp:14:
> include/llvm/Support/PointerLikeTypeTraits.h: In instantiation of
> ‘struct llvm::PointerLikeTypeTraits’:
> /usr/include/c++/4.8.2/type_traits:1087:41:   required by substitution
> of ‘template static decltype
> (((declval<_Tp1>)()=(declval<_Up1>)(), std::__sfinae_types::__one()))
> std::__is_assignable_helper<_Tp, _Up>::__test(int) [with _Tp1 = _Tp1;
> _Up1 = _Up1; _Tp =
> llvm::detail::trivial_helper bool> >&; _Up = const
> llvm::detail::trivial_helper bool> >&] [with _Tp1 =
> llvm::detail::trivial_helper bool> >&; _Up1 = const
> llvm::detail::trivial_helper bool> >&]’
> /usr/include/c++/4.8.2/type_traits:1094:50:   required from ‘constexpr
> const bool
>
> std::__is_assignable_helper 1u, bool> >&, const
> llvm::detail::trivial_helper bool> >&>::value’
> /usr/include/c++/4.8.2/type_traits:1099:12:   required from ‘struct
>
> std::is_assignable 1u, bool> >&, const
> llvm::detail::trivial_helper bool> >&>’
> /usr/include/c++/4.8.2/type_traits:1112:12:   required from ‘struct
>
> std::__is_copy_assignable_impl 1u, bool> >, false>’
> /usr/include/c++/4.8.2/type_traits:1118:12:   required from ‘struct
>
> std::is_copy_assignable 1u, bool> > >’
> include/llvm/Support/type_traits.h:142:25:   required from ‘constexpr
> const bool
> llvm::is_trivially_copyable bool> >::has_trivial_copy_assign’
> include/llvm/Support/type_traits.h:163:32:   required from ‘constexpr
> const bool
> llvm::is_trivially_copyable bool> >::value’
> include/llvm/ADT/SmallVector.h:321:7:   required from ‘class
> llvm::SmallVectorImpl >’
> include/llvm/ADT/SmallVector.h:845:7:   required from ‘class
> llvm::SmallVector, 2u>’
> tools/clang/include/clang/Basic/Module.h:290:30:   required from here
> include/llvm/Support/PointerLikeTypeTraits.h:59:8: error: invalid
> application of ‘__alignof__’ to incomplete type ‘clang::Module’
>enum { NumLowBitsAvailable = detail::ConstantLog2::value };
> ^
>
> FYI in case you wonder:
>
> $ ls -l /usr/include/c++/
> total 8
> drwxr-xr-x. 12 root root 4096 Dec 11 03:24 4.8.2
> lrwxrwxrwx.  1 root root5 Dec 11 03:24 4.8.5 -> 4.8.2
>
>
> Are we outside the "allowed" range for GCC versions?
>
> Cheers, Axel.
>
> On 1/20/19 10:19 PM, Serge Guelton via cfe-commits wrote:
> > Author: serge_sans_paille
> > Date: Sun Jan 20 13:19:56 2019
> > New Revision: 351701
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=351701=rev
> > Log:
> > Replace llvm::isPodLike<...>  by llvm::is_trivially_copyable<...>
> >
> > As noted in https://bugs.llvm.org/show_bug.cgi?id=36651, the
> specialization for
> > isPodLike> did not match the expectation of
> > std::is_trivially_copyable which makes the memcpy optimization invalid.
> >
> > This patch renames the llvm::isPodLike trait into
> llvm::is_trivially_copyable.
> > Unfortunately std::is_trivially_copyable is not portable across compiler
> / STL
> > versions. So a portable version is provided too.
> >
> > Note that the following specialization were invalid:
> >
> > std::pair
> > llvm::Optional
> >
> > Tests have been added to assert that former specialization are respected
> by the
> > standard usage of llvm::is_trivially_copyable, and that when a decent
> version
> > of std::is_trivially_copyable is available, llvm::is_trivially_copyable
> is
> > compared to std::is_trivially_copyable.
> >
> > As of this patch, llvm::Optional is no longer considered trivially
> copyable,
> > even if T is. This is to be fixed in a later patch, as it has impact on a
> > long-running bug (see r347004)
> >
> > Note that GCC warns about this UB, but this got silented by
> https://reviews.llvm.org/D50296.
> >
> > Differential Revision: https://reviews.llvm.org/D54472
> >
> >
> > Modified:
> > cfe/trunk/include/clang/AST/BaseSubobject.h
> > cfe/trunk/include/clang/AST/CharUnits.h
> > cfe/trunk/include/clang/AST/DeclAccessPair.h
> > cfe/trunk/include/clang/AST/DeclarationName.h
> > cfe/trunk/include/clang/AST/ExprObjC.h
> > cfe/trunk/include/clang/AST/GlobalDecl.h
> > 

[PATCH] D57080: [clang-tidy] Delete obsolete objc-property-declaration options ✂️

[Stephane Moore via Phabricator via cfe-commits]

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



Comment at: docs/ReleaseNotes.rst:76
 
+- The `Acronyms` and `IncludeDefaultAcronyms` options for the
+  :doc:`objc-property-declaration 
`

In https://reviews.llvm.org/D56945, I implemented the recommendation to use the 
:option: prefix for these options but the documentation generator produced a 
warning for an unknown option which I imagine is due to the deletion of the 
options. I imagine that means we can't use the :option: prefix for deleted 
options? Please let me know if you have other suggestions.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57080/new/

https://reviews.llvm.org/D57080



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

[PATCH] D57080: [clang-tidy] Delete obsolete objc-property-declaration options ✂️

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore created this revision.
Herald added subscribers: cfe-commits, xazax.hun.
stephanemoore edited the summary of this revision.

The Acronyms and IncludeDefaultAcronyms options were deprecated in
https://reviews.llvm.org/D51832. These options can be removed.

Tested by running the clang-tidy tests.

This is an amended resubmission of https://reviews.llvm.org/D56945.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D57080

Files:
  clang-tidy/objc/PropertyDeclarationCheck.cpp
  clang-tidy/objc/PropertyDeclarationCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/objc-property-declaration.rst


Index: docs/clang-tidy/checks/objc-property-declaration.rst
===
--- docs/clang-tidy/checks/objc-property-declaration.rst
+++ docs/clang-tidy/checks/objc-property-declaration.rst
@@ -40,15 +40,3 @@
@property(nonatomic, assign) int abc_lowerCamelCase;
 
 The corresponding style rule: 
https://developer.apple.com/library/content/qa/qa1908/_index.html
-
-
-Options

-
-.. option:: Acronyms
-
-   This option is deprecated and ignored.
-
-.. option:: IncludeDefaultAcronyms
-
-   This option is deprecated and ignored.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -73,6 +73,10 @@
   Checks for casts of ``absl::Duration`` conversion functions, and recommends
   the right conversion function instead.
 
+- The `Acronyms` and `IncludeDefaultAcronyms` options for the
+  :doc:`objc-property-declaration 
`
+  check have been removed.
+
 Improvements to include-fixer
 -
 
Index: clang-tidy/objc/PropertyDeclarationCheck.h
===
--- clang-tidy/objc/PropertyDeclarationCheck.h
+++ clang-tidy/objc/PropertyDeclarationCheck.h
@@ -10,8 +10,6 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_PROPERTY_DECLARATION_H
 
 #include "../ClangTidy.h"
-#include 
-#include 
 
 namespace clang {
 namespace tidy {
@@ -27,15 +25,10 @@
 /// 
http://clang.llvm.org/extra/clang-tidy/checks/objc-property-declaration.html
 class PropertyDeclarationCheck : public ClangTidyCheck {
 public:
-  PropertyDeclarationCheck(StringRef Name, ClangTidyContext *Context);
+  PropertyDeclarationCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult ) override;
-  void storeOptions(ClangTidyOptions::OptionMap ) override;
-
-private:
-  const std::vector SpecialAcronyms;
-  const bool IncludeDefaultAcronyms;
-  std::vector EscapedAcronyms;
 };
 
 } // namespace objc
Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -97,14 +97,6 @@
 }
 }  // namespace
 
-PropertyDeclarationCheck::PropertyDeclarationCheck(StringRef Name,
-   ClangTidyContext *Context)
-: ClangTidyCheck(Name, Context),
-  SpecialAcronyms(
-  utils::options::parseStringList(Options.get("Acronyms", ""))),
-  IncludeDefaultAcronyms(Options.get("IncludeDefaultAcronyms", true)),
-  EscapedAcronyms() {}
-
 void PropertyDeclarationCheck::registerMatchers(MatchFinder *Finder) {
   // this check should only be applied to ObjC sources.
   if (!getLangOpts().ObjC) return;
@@ -145,12 +137,6 @@
   << generateFixItHint(MatchedDecl, StandardProperty);
 }
 
-void PropertyDeclarationCheck::storeOptions(ClangTidyOptions::OptionMap ) 
{
-  Options.store(Opts, "Acronyms",
-utils::options::serializeStringList(SpecialAcronyms));
-  Options.store(Opts, "IncludeDefaultAcronyms", IncludeDefaultAcronyms);
-}
-
 }  // namespace objc
 }  // namespace tidy
 }  // namespace clang


Index: docs/clang-tidy/checks/objc-property-declaration.rst
===
--- docs/clang-tidy/checks/objc-property-declaration.rst
+++ docs/clang-tidy/checks/objc-property-declaration.rst
@@ -40,15 +40,3 @@
@property(nonatomic, assign) int abc_lowerCamelCase;
 
 The corresponding style rule: https://developer.apple.com/library/content/qa/qa1908/_index.html
-
-
-Options

-
-.. option:: Acronyms
-
-   This option is deprecated and ignored.
-
-.. option:: IncludeDefaultAcronyms
-
-   This option is deprecated and ignored.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -73,6 +73,10 @@
   Checks for casts of ``absl::Duration`` conversion functions, and recommends
   the right conversion function instead.
 
+- The `Acronyms` and `IncludeDefaultAcronyms` options for the
+  

[PATCH] D56945: [clang-tidy] Delete obsolete objc-property-declaration options ✂️

[Stephane Moore via Phabricator via cfe-commits]

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



Comment at: docs/ReleaseNotes.rst:248
 
+- The `Acronyms` and `IncludeDefaultAcronyms` options for the
+  :doc:`objc-property-declaration`

Eugene.Zelenko wrote:
> Please rebase from trunk and use :option: prefix for options..
Could using the :option: prefix cause issues if I am removing the option?

The documentation generation seems to emit a warning for an unknown option:
"llvm/src/tools/clang/tools/extra/docs/ReleaseNotes.rst:76: WARNING: unknown 
option: Acronyms"
http://lab.llvm.org:8011/builders/clang-tools-sphinx-docs/builds/36425/steps/docs-clang-tools-html/logs/stdio


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56945/new/

https://reviews.llvm.org/D56945



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

r351924 - [ubsan] Check the correct size when sanitizing array new.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Tue Jan 22 19:37:29 2019
New Revision: 351924

URL: http://llvm.org/viewvc/llvm-project?rev=351924=rev
Log:
[ubsan] Check the correct size when sanitizing array new.

We previously forgot to multiply the element size by the array bound.

Modified:
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/CodeGen/CGExprCXX.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=351924=351923=351924=diff
==
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Tue Jan 22 19:37:29 2019
@@ -652,7 +652,8 @@ bool CodeGenFunction::sanitizePerformTyp
 void CodeGenFunction::EmitTypeCheck(TypeCheckKind TCK, SourceLocation Loc,
 llvm::Value *Ptr, QualType Ty,
 CharUnits Alignment,
-SanitizerSet SkippedChecks) {
+SanitizerSet SkippedChecks,
+llvm::Value *ArraySize) {
   if (!sanitizePerformTypeCheck())
 return;
 
@@ -710,21 +711,27 @@ void CodeGenFunction::EmitTypeCheck(Type
   if (SanOpts.has(SanitizerKind::ObjectSize) &&
   !SkippedChecks.has(SanitizerKind::ObjectSize) &&
   !Ty->isIncompleteType()) {
-uint64_t Size = getContext().getTypeSizeInChars(Ty).getQuantity();
-
-// The glvalue must refer to a large enough storage region.
-// FIXME: If Address Sanitizer is enabled, insert dynamic instrumentation
-//to check this.
-// FIXME: Get object address space
-llvm::Type *Tys[2] = { IntPtrTy, Int8PtrTy };
-llvm::Value *F = CGM.getIntrinsic(llvm::Intrinsic::objectsize, Tys);
-llvm::Value *Min = Builder.getFalse();
-llvm::Value *NullIsUnknown = Builder.getFalse();
-llvm::Value *CastAddr = Builder.CreateBitCast(Ptr, Int8PtrTy);
-llvm::Value *LargeEnough = Builder.CreateICmpUGE(
-Builder.CreateCall(F, {CastAddr, Min, NullIsUnknown}),
-llvm::ConstantInt::get(IntPtrTy, Size));
-Checks.push_back(std::make_pair(LargeEnough, SanitizerKind::ObjectSize));
+uint64_t TySize = getContext().getTypeSizeInChars(Ty).getQuantity();
+llvm::Value *Size = llvm::ConstantInt::get(IntPtrTy, TySize);
+if (ArraySize)
+  Size = Builder.CreateMul(Size, ArraySize);
+
+// Degenerate case: new X[0] does not need an objectsize check.
+llvm::Constant *ConstantSize = dyn_cast(Size);
+if (!ConstantSize || !ConstantSize->isNullValue()) {
+  // The glvalue must refer to a large enough storage region.
+  // FIXME: If Address Sanitizer is enabled, insert dynamic instrumentation
+  //to check this.
+  // FIXME: Get object address space
+  llvm::Type *Tys[2] = { IntPtrTy, Int8PtrTy };
+  llvm::Value *F = CGM.getIntrinsic(llvm::Intrinsic::objectsize, Tys);
+  llvm::Value *Min = Builder.getFalse();
+  llvm::Value *NullIsUnknown = Builder.getFalse();
+  llvm::Value *CastAddr = Builder.CreateBitCast(Ptr, Int8PtrTy);
+  llvm::Value *LargeEnough = Builder.CreateICmpUGE(
+  Builder.CreateCall(F, {CastAddr, Min, NullIsUnknown}), Size);
+  Checks.push_back(std::make_pair(LargeEnough, SanitizerKind::ObjectSize));
+}
   }
 
   uint64_t AlignVal = 0;

Modified: cfe/trunk/lib/CodeGen/CGExprCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprCXX.cpp?rev=351924=351923=351924=diff
==
--- cfe/trunk/lib/CodeGen/CGExprCXX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprCXX.cpp Tue Jan 22 19:37:29 2019
@@ -1714,10 +1714,16 @@ llvm::Value *CodeGenFunction::EmitCXXNew
  result.getAlignment());
 
   // Emit sanitizer checks for pointer value now, so that in the case of an
-  // array it was checked only once and not at each constructor call.
+  // array it was checked only once and not at each constructor call. We may
+  // have already checked that the pointer is non-null.
+  // FIXME: If we have an array cookie and a potentially-throwing allocator,
+  // we'll null check the wrong pointer here.
+  SanitizerSet SkippedChecks;
+  SkippedChecks.set(SanitizerKind::Null, nullCheck);
   EmitTypeCheck(CodeGenFunction::TCK_ConstructorCall,
-  E->getAllocatedTypeSourceInfo()->getTypeLoc().getBeginLoc(),
-  result.getPointer(), allocType);
+E->getAllocatedTypeSourceInfo()->getTypeLoc().getBeginLoc(),
+result.getPointer(), allocType, result.getAlignment(),
+SkippedChecks, numElements);
 
   EmitNewInitializer(*this, E, allocType, elementTy, result, numElements,
  allocSizeWithoutCookie);

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
URL: 

Re: r347205 - [FileManager] getFile(open=true) after getFile(open=false) should open the file.

[Nico Weber via cfe-commits]

I don't have a reduced test case yet, but this seems to cause clang to
sometimes claim that an included file isn't found even if it's there, at
least on macOS: https://bugs.chromium.org/p/chromium/issues/detail?id=924225

On Mon, Nov 19, 2018 at 8:40 AM Sam McCall via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: sammccall
> Date: Mon Nov 19 05:37:46 2018
> New Revision: 347205
>
> URL: http://llvm.org/viewvc/llvm-project?rev=347205=rev
> Log:
> [FileManager] getFile(open=true) after getFile(open=false) should open the
> file.
>
> Summary:
> Old behavior is to just return the cached entry regardless of opened-ness.
> That feels buggy (though I guess nobody ever actually needed this).
>
> This came up in the context of clangd+clang-tidy integration: we're
> going to getFile(open=false) to replay preprocessor actions obscured by
> the preamble, but the compilation may subsequently getFile(open=true)
> for non-preamble includes.
>
> Reviewers: ilya-biryukov
>
> Subscribers: ioeric, kadircet, cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D54691
>
> Modified:
> cfe/trunk/include/clang/Basic/FileManager.h
> cfe/trunk/lib/Basic/FileManager.cpp
> cfe/trunk/unittests/Basic/FileManagerTest.cpp
>
> Modified: cfe/trunk/include/clang/Basic/FileManager.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/FileManager.h?rev=347205=347204=347205=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/FileManager.h (original)
> +++ cfe/trunk/include/clang/Basic/FileManager.h Mon Nov 19 05:37:46 2018
> @@ -70,14 +70,15 @@ class FileEntry {
>bool IsNamedPipe;
>bool InPCH;
>bool IsValid;   // Is this \c FileEntry initialized and
> valid?
> +  bool DeferredOpen;  // Created by getFile(OpenFile=0); may open
> later.
>
>/// The open file, if it is owned by the \p FileEntry.
>mutable std::unique_ptr File;
>
>  public:
>FileEntry()
> -  : UniqueID(0, 0), IsNamedPipe(false), InPCH(false), IsValid(false)
> -  {}
> +  : UniqueID(0, 0), IsNamedPipe(false), InPCH(false), IsValid(false),
> +DeferredOpen(false) {}
>
>FileEntry(const FileEntry &) = delete;
>FileEntry =(const FileEntry &) = delete;
>
> Modified: cfe/trunk/lib/Basic/FileManager.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=347205=347204=347205=diff
>
> ==
> --- cfe/trunk/lib/Basic/FileManager.cpp (original)
> +++ cfe/trunk/lib/Basic/FileManager.cpp Mon Nov 19 05:37:46 2018
> @@ -221,15 +221,21 @@ const FileEntry *FileManager::getFile(St
>*SeenFileEntries.insert(std::make_pair(Filename, nullptr)).first;
>
>// See if there is already an entry in the map.
> -  if (NamedFileEnt.second)
> -return NamedFileEnt.second == NON_EXISTENT_FILE ? nullptr
> -: NamedFileEnt.second;
> +  if (NamedFileEnt.second) {
> +if (NamedFileEnt.second == NON_EXISTENT_FILE)
> +  return nullptr;
> +// Entry exists: return it *unless* it wasn't opened and open is
> requested.
> +if (!(NamedFileEnt.second->DeferredOpen && openFile))
> +  return NamedFileEnt.second;
> +// We previously stat()ed the file, but didn't open it: do that below.
> +// FIXME: the below does other redundant work too (stats the dir and
> file).
> +  } else {
> +// By default, initialize it to invalid.
> +NamedFileEnt.second = NON_EXISTENT_FILE;
> +  }
>
>++NumFileCacheMisses;
>
> -  // By default, initialize it to invalid.
> -  NamedFileEnt.second = NON_EXISTENT_FILE;
> -
>// Get the null-terminated file name as stored as the key of the
>// SeenFileEntries map.
>StringRef InterndFileName = NamedFileEnt.first();
> @@ -267,6 +273,7 @@ const FileEntry *FileManager::getFile(St
>// It exists.  See if we have already opened a file with the same inode.
>// This occurs when one dir is symlinked to another, for example.
>FileEntry  = UniqueRealFiles[Data.UniqueID];
> +  UFE.DeferredOpen = !openFile;
>
>NamedFileEnt.second = 
>
> @@ -283,6 +290,23 @@ const FileEntry *FileManager::getFile(St
>  InterndFileName = NamedFileEnt.first().data();
>}
>
> +  // If we opened the file for the first time, record the resulting info.
> +  // Do this even if the cache entry was valid, maybe we didn't
> previously open.
> +  if (F && !UFE.File) {
> +if (auto PathName = F->getName()) {
> +  llvm::SmallString<128> AbsPath(*PathName);
> +  // This is not the same as `VFS::getRealPath()`, which resolves
> symlinks
> +  // but can be very expensive on real file systems.
> +  // FIXME: the semantic of RealPathName is unclear, and the name
> might be
> +  // misleading. We need to clean up the interface here.
> +  makeAbsolutePath(AbsPath);
> +  

[clang-tools-extra] r351922 - Revert rCTE351921 to fix documentation geneeration.

[Stephane Moore via cfe-commits]

Author: stephanemoore
Date: Tue Jan 22 18:58:59 2019
New Revision: 351922

URL: http://llvm.org/viewvc/llvm-project?rev=351922=rev
Log:
Revert rCTE351921 to fix documentation geneeration.

Original review: https://reviews.llvm.org/D56945

Modified:
clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.h
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/objc-property-declaration.rst

Modified: clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp?rev=351922=351921=351922=diff
==
--- clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp Tue 
Jan 22 18:58:59 2019
@@ -97,6 +97,14 @@ bool prefixedPropertyNameValid(llvm::Str
 }
 }  // namespace
 
+PropertyDeclarationCheck::PropertyDeclarationCheck(StringRef Name,
+   ClangTidyContext *Context)
+: ClangTidyCheck(Name, Context),
+  SpecialAcronyms(
+  utils::options::parseStringList(Options.get("Acronyms", ""))),
+  IncludeDefaultAcronyms(Options.get("IncludeDefaultAcronyms", true)),
+  EscapedAcronyms() {}
+
 void PropertyDeclarationCheck::registerMatchers(MatchFinder *Finder) {
   // this check should only be applied to ObjC sources.
   if (!getLangOpts().ObjC) return;
@@ -137,6 +145,12 @@ void PropertyDeclarationCheck::check(con
   << generateFixItHint(MatchedDecl, StandardProperty);
 }
 
+void PropertyDeclarationCheck::storeOptions(ClangTidyOptions::OptionMap ) 
{
+  Options.store(Opts, "Acronyms",
+utils::options::serializeStringList(SpecialAcronyms));
+  Options.store(Opts, "IncludeDefaultAcronyms", IncludeDefaultAcronyms);
+}
+
 }  // namespace objc
 }  // namespace tidy
 }  // namespace clang

Modified: clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.h?rev=351922=351921=351922=diff
==
--- clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.h 
(original)
+++ clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.h Tue Jan 
22 18:58:59 2019
@@ -10,6 +10,8 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_PROPERTY_DECLARATION_H
 
 #include "../ClangTidy.h"
+#include 
+#include 
 
 namespace clang {
 namespace tidy {
@@ -25,10 +27,15 @@ namespace objc {
 /// 
http://clang.llvm.org/extra/clang-tidy/checks/objc-property-declaration.html
 class PropertyDeclarationCheck : public ClangTidyCheck {
 public:
-  PropertyDeclarationCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  PropertyDeclarationCheck(StringRef Name, ClangTidyContext *Context);
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult ) override;
+  void storeOptions(ClangTidyOptions::OptionMap ) override;
+
+private:
+  const std::vector SpecialAcronyms;
+  const bool IncludeDefaultAcronyms;
+  std::vector EscapedAcronyms;
 };
 
 } // namespace objc

Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=351922=351921=351922=diff
==
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Tue Jan 22 18:58:59 2019
@@ -73,10 +73,6 @@ Improvements to clang-tidy
   Checks for casts of ``absl::Duration`` conversion functions, and recommends
   the right conversion function instead.
 
-- The :option:`Acronyms` and :option:`IncludeDefaultAcronyms` options for the
-  :doc:`objc-property-declaration 
`
-  check have been removed.
-
 Improvements to include-fixer
 -
 

Modified: 
clang-tools-extra/trunk/docs/clang-tidy/checks/objc-property-declaration.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/objc-property-declaration.rst?rev=351922=351921=351922=diff
==
--- 
clang-tools-extra/trunk/docs/clang-tidy/checks/objc-property-declaration.rst 
(original)
+++ 
clang-tools-extra/trunk/docs/clang-tidy/checks/objc-property-declaration.rst 
Tue Jan 22 18:58:59 2019
@@ -40,3 +40,15 @@ lowercase letters followed by a '_' to a
@property(nonatomic, assign) int abc_lowerCamelCase;
 
 The corresponding style rule: 
https://developer.apple.com/library/content/qa/qa1908/_index.html
+
+
+Options

[PATCH] D56945: [clang-tidy] Delete obsolete objc-property-declaration options ✂️

[Stephane Moore via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL351921: [clang-tidy] Delete obsolete 
objc-property-declaration options ✂️ (authored by stephanemoore, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D56945?vs=182660=183015#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56945/new/

https://reviews.llvm.org/D56945

Files:
  clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
  clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.h
  clang-tools-extra/trunk/docs/ReleaseNotes.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/objc-property-declaration.rst


Index: clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.h
+++ clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.h
@@ -10,8 +10,6 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_PROPERTY_DECLARATION_H
 
 #include "../ClangTidy.h"
-#include 
-#include 
 
 namespace clang {
 namespace tidy {
@@ -27,15 +25,10 @@
 /// 
http://clang.llvm.org/extra/clang-tidy/checks/objc-property-declaration.html
 class PropertyDeclarationCheck : public ClangTidyCheck {
 public:
-  PropertyDeclarationCheck(StringRef Name, ClangTidyContext *Context);
+  PropertyDeclarationCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult ) override;
-  void storeOptions(ClangTidyOptions::OptionMap ) override;
-
-private:
-  const std::vector SpecialAcronyms;
-  const bool IncludeDefaultAcronyms;
-  std::vector EscapedAcronyms;
 };
 
 } // namespace objc
Index: clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -97,14 +97,6 @@
 }
 }  // namespace
 
-PropertyDeclarationCheck::PropertyDeclarationCheck(StringRef Name,
-   ClangTidyContext *Context)
-: ClangTidyCheck(Name, Context),
-  SpecialAcronyms(
-  utils::options::parseStringList(Options.get("Acronyms", ""))),
-  IncludeDefaultAcronyms(Options.get("IncludeDefaultAcronyms", true)),
-  EscapedAcronyms() {}
-
 void PropertyDeclarationCheck::registerMatchers(MatchFinder *Finder) {
   // this check should only be applied to ObjC sources.
   if (!getLangOpts().ObjC) return;
@@ -145,12 +137,6 @@
   << generateFixItHint(MatchedDecl, StandardProperty);
 }
 
-void PropertyDeclarationCheck::storeOptions(ClangTidyOptions::OptionMap ) 
{
-  Options.store(Opts, "Acronyms",
-utils::options::serializeStringList(SpecialAcronyms));
-  Options.store(Opts, "IncludeDefaultAcronyms", IncludeDefaultAcronyms);
-}
-
 }  // namespace objc
 }  // namespace tidy
 }  // namespace clang
Index: clang-tools-extra/trunk/docs/ReleaseNotes.rst
===
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst
@@ -73,6 +73,10 @@
   Checks for casts of ``absl::Duration`` conversion functions, and recommends
   the right conversion function instead.
 
+- The :option:`Acronyms` and :option:`IncludeDefaultAcronyms` options for the
+  :doc:`objc-property-declaration 
`
+  check have been removed.
+
 Improvements to include-fixer
 -
 
Index: 
clang-tools-extra/trunk/docs/clang-tidy/checks/objc-property-declaration.rst
===
--- clang-tools-extra/trunk/docs/clang-tidy/checks/objc-property-declaration.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/objc-property-declaration.rst
@@ -40,15 +40,3 @@
@property(nonatomic, assign) int abc_lowerCamelCase;
 
 The corresponding style rule: 
https://developer.apple.com/library/content/qa/qa1908/_index.html
-
-
-Options

-
-.. option:: Acronyms
-
-   This option is deprecated and ignored.
-
-.. option:: IncludeDefaultAcronyms
-
-   This option is deprecated and ignored.


Index: clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.h
+++ clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.h
@@ -10,8 +10,6 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_PROPERTY_DECLARATION_H
 
 #include "../ClangTidy.h"
-#include 
-#include 
 
 namespace clang {
 namespace tidy {
@@ -27,15 +25,10 @@
 /// 

[PATCH] D57076: [ObjC generics] Fix applying `__kindof` to the type parameter.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai created this revision.
vsapsai added reviewers: ahatanak, erik.pilkington.
Herald added subscribers: dexonsmith, jkorous.

Fixes the warning about incompatible pointer types on assigning to a
subclass of type argument an expression of type `__kindof TypeParam`.

We already have a mechanism in `ASTContext::canAssignObjCInterfaces`
that handles `ObjCObjectType` with `__kindof`. But it wasn't triggered
because during type substitution `__kindof TypeParam` was represented as
`AttributedType` with attribute `ObjCKindOf` and equivalent type
`TypeArg`. For assignment type checking we use canonical types so
attributed type was desugared and the attribute was ignored.

The fix is in checking transformed `AttributedType` and pushing
`__kindof` down into `ObjCObjectType` when necessary.

rdar://problem/38514910


https://reviews.llvm.org/D57076

Files:
  clang/lib/AST/Type.cpp
  clang/test/SemaObjC/kindof.m

Index: clang/test/SemaObjC/kindof.m
===
--- clang/test/SemaObjC/kindof.m
+++ clang/test/SemaObjC/kindof.m
@@ -384,9 +384,17 @@
 }
 @end
 
+// ---
+// __kindof on type parameters
+// ---
+
 @interface NSGeneric : NSObject
 - (void)test:(__kindof ObjectType)T; // expected-note{{passing argument to parameter 'T' here}}
 - (void)mapUsingBlock:(id (^)(__kindof ObjectType))block;
+@property (copy) ObjectType object;
+@property (copy) __kindof ObjectType kindof_object;
+
+@property (copy) __kindof ObjectType _Nonnull nonnull_kindof_object;
 @end
 @implementation NSGeneric
 - (void)test:(id)T {
@@ -395,6 +403,11 @@
 }
 @end
 
+@interface NSDefaultGeneric : NSObject
+@property (copy) ObjectType object;
+@property (copy) __kindof ObjectType kindof_object;
+@end
+
 void testGeneric(NSGeneric *generic) {
   NSObject *NSObject_obj;
   // Assign from NSObject_obj to __kindof NSString*.
@@ -403,6 +416,38 @@
   [generic test:NSString_str];
 }
 
+void testGenericAssignment() {
+  NSMutableString *NSMutableString_str;
+  NSNumber *NSNumber_obj;
+
+  NSGeneric *generic;
+  NSMutableString_str = generic.object; // expected-warning{{incompatible pointer types}}
+  NSNumber_obj = generic.object; // expected-warning{{incompatible pointer types}}
+  NSMutableString_str = generic.kindof_object;
+  NSNumber_obj = generic.kindof_object; // expected-warning{{incompatible pointer types assigning to 'NSNumber *' from '__kindof NSString *'}}
+
+  NSGeneric<__kindof NSString*> *kindof_generic;
+  NSMutableString_str = kindof_generic.object;
+  NSNumber_obj = kindof_generic.object; // expected-warning{{incompatible pointer types assigning to 'NSNumber *' from '__kindof NSString *'}}
+  NSMutableString_str = kindof_generic.kindof_object;
+  NSNumber_obj = kindof_generic.kindof_object; // expected-warning{{incompatible pointer types assigning to 'NSNumber *' from '__kindof __kindof NSString *'}}
+
+  NSDefaultGeneric *default_generic;
+  NSMutableString_str = default_generic.object;
+  NSNumber_obj = default_generic.object; // expected-warning{{incompatible pointer types}}
+  NSMutableString_str = default_generic.kindof_object;
+  NSNumber_obj = default_generic.kindof_object; // expected-warning{{incompatible pointer types assigning to 'NSNumber *' from '__kindof __kindof NSString *'}}
+}
+
+void testKindofNonObjectType() {
+  typedef void (^BlockType)(int);
+  NSGeneric *generic;
+}
+
+void testKindofNullability(NSGeneric *generic) {
+  generic.nonnull_kindof_object = 0; // expected-warning{{null passed to a callee that requires a non-null argument}}
+}
+
 // Check that clang doesn't crash when a type parameter is illegal.
 @interface Array1 : NSObject
 @end
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -1284,6 +1284,42 @@
 
 return BaseType::VisitObjCObjectType(objcObjectType);
   }
+
+  QualType VisitAttributedType(const AttributedType *attrType) {
+QualType newType = BaseType::VisitAttributedType(attrType);
+if (newType.isNull())
+  return {};
+
+const auto *newAttrType = dyn_cast(newType.getTypePtr());
+if (newAttrType->getAttrKind() != attr::ObjCKindOf)
+  return newType;
+
+// Find out if it's an Objective-C object or object pointer type;
+QualType newEquivType = newAttrType->getEquivalentType();
+const ObjCObjectPointerType *ptrType =
+newEquivType->getAs();
+const ObjCObjectType *objType = ptrType
+? ptrType->getObjectType()
+: newEquivType->getAs();
+
+if (objType) {
+  // Rebuild the "equivalent" type, which pushes __kindof down into
+  // the object type.
+  newEquivType = Ctx.getObjCObjectType(
+  objType->getBaseType(), objType->getTypeArgsAsWritten(),
+

[PATCH] D57075: [ObjC] For type substitution in generics use a regular recursive type visitor.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai created this revision.
vsapsai added reviewers: ahatanak, erik.pilkington.
Herald added subscribers: dexonsmith, jkorous.

Switch to the inheritance-based visitor from the lambda-based visitor to
allow both preorder and postorder customizations during type
transformation. NFC intended.


https://reviews.llvm.org/D57075

Files:
  clang/lib/AST/Type.cpp

Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -722,25 +722,30 @@
   return ctx.getObjCObjectPointerType(obj)->castAs();
 }
 
-template
-static QualType simpleTransform(ASTContext , QualType type, F &);
-
 namespace {
 
-/// Visitor used by simpleTransform() to perform the transformation.
-template
-struct SimpleTransformVisitor
- : public TypeVisitor, QualType> {
+/// Visitor used to perform a simple type transformation that does not change
+/// the semantics of the type.
+template 
+struct SimpleTransformVisitor : public TypeVisitor {
   ASTContext 
-  F &
 
   QualType recurse(QualType type) {
-return simpleTransform(Ctx, type, std::move(TheFunc));
+// Split out the qualifiers from the type.
+SplitQualType splitType = type.split();
+
+// Visit the type itself.
+QualType result = static_cast(this)->Visit(splitType.Ty);
+if (result.isNull())
+  return result;
+
+// Reconstruct the transformed type by applying the local qualifiers
+// from the split type.
+return Ctx.getQualifiedType(result, splitType.Quals);
   }
 
 public:
-  SimpleTransformVisitor(ASTContext , F &)
-  : Ctx(ctx), TheFunc(std::move(f)) {}
+  explicit SimpleTransformVisitor(ASTContext ) : Ctx(ctx) {}
 
   // None of the clients of this transformation can occur where
   // there are dependent types, so skip dependent types.
@@ -1108,220 +1113,205 @@
 #undef TRIVIAL_TYPE_CLASS
 };
 
-} // namespace
+struct SubstObjCTypeArgsVisitor
+: public SimpleTransformVisitor {
+  using BaseType = SimpleTransformVisitor;
 
-/// Perform a simple type transformation that does not change the
-/// semantics of the type.
-template
-static QualType simpleTransform(ASTContext , QualType type, F &) {
-  // Transform the type. If it changed, return the transformed result.
-  QualType transformed = f(type);
-  if (transformed.getAsOpaquePtr() != type.getAsOpaquePtr())
-return transformed;
-
-  // Split out the qualifiers from the type.
-  SplitQualType splitType = type.split();
-
-  // Visit the type itself.
-  SimpleTransformVisitor visitor(ctx, std::forward(f));
-  QualType result = visitor.Visit(splitType.Ty);
-  if (result.isNull())
-return result;
-
-  // Reconstruct the transformed type by applying the local qualifiers
-  // from the split type.
-  return ctx.getQualifiedType(result, splitType.Quals);
-}
+  ArrayRef TypeArgs;
+  ObjCSubstitutionContext SubstContext;
 
-/// Substitute the given type arguments for Objective-C type
-/// parameters within the given type, recursively.
-QualType QualType::substObjCTypeArgs(
-   ASTContext ,
-   ArrayRef typeArgs,
-   ObjCSubstitutionContext context) const {
-  return simpleTransform(ctx, *this,
- [&](QualType type) -> QualType {
-SplitQualType splitType = type.split();
+  SubstObjCTypeArgsVisitor(ASTContext , ArrayRef typeArgs,
+   ObjCSubstitutionContext context)
+  : BaseType(ctx), TypeArgs(typeArgs), SubstContext(context) {}
 
+  QualType VisitObjCTypeParamType(const ObjCTypeParamType *OTPTy) {
 // Replace an Objective-C type parameter reference with the corresponding
 // type argument.
-if (const auto *OTPTy = dyn_cast(splitType.Ty)) {
-  ObjCTypeParamDecl *typeParam = OTPTy->getDecl();
-  // If we have type arguments, use them.
-  if (!typeArgs.empty()) {
-QualType argType = typeArgs[typeParam->getIndex()];
-if (OTPTy->qual_empty())
-  return ctx.getQualifiedType(argType, splitType.Quals);
-
-// Apply protocol lists if exists.
-bool hasError;
-SmallVector protocolsVec;
-protocolsVec.append(OTPTy->qual_begin(),
-OTPTy->qual_end());
-ArrayRef protocolsToApply = protocolsVec;
-QualType resultTy = ctx.applyObjCProtocolQualifiers(argType,
-protocolsToApply, hasError, true/*allowOnPointerType*/);
-
-return ctx.getQualifiedType(resultTy, splitType.Quals);
-  }
+ObjCTypeParamDecl *typeParam = OTPTy->getDecl();
+// If we have type arguments, use them.
+if (!TypeArgs.empty()) {
+  QualType argType = TypeArgs[typeParam->getIndex()];
+  if (OTPTy->qual_empty())
+return argType;
+
+  // Apply protocol lists if exists.
+  bool hasError;
+  SmallVector protocolsVec;
+  protocolsVec.append(OTPTy->qual_begin(), OTPTy->qual_end());
+  ArrayRef protocolsToApply = protocolsVec;
+  return Ctx.applyObjCProtocolQualifiers(
+   

Buildbot numbers for the last week of 01/13/2019 - 01/19/2019

[Galina Kistanova via cfe-commits]

Hello everyone,

Below are some buildbot numbers for the last week of 01/13/2019 -
01/19/2019.

Please see the same data in attached csv files:

The longest time each builder was red during the week;
"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to red or from red to green);
Count of commits by project;
Number of completed builds, failed builds and average build time for
successful builds per active builder;
Average waiting time for a revision to get build result per active builder
(response time).

Thanks

Galina


The longest time each builder was red during the week:
 buildername |  was_red
-+--
 libcxx-libcxxabi-libunwind-armv7-linux-noexceptions | 115:40:11
 libcxx-libcxxabi-libunwind-armv7-linux  | 113:05:27
 llvm-clang-x86_64-expensive-checks-win  | 45:04:12
 lld-x86_64-win7 | 44:52:27
 clang-x86_64-linux-selfhost-modules | 27:20:39
 openmp-clang-x86_64-linux-debian| 26:31:29
 openmp-gcc-x86_64-linux-debian  | 26:31:06
 clang-cmake-thumbv7-full-sh | 24:53:46
 clang-cmake-thumbv8-full-sh | 22:50:59
 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast| 22:41:37
 clang-x64-windows-msvc  | 22:38:15
 sanitizer-x86_64-linux-android  | 20:50:56
 lldb-x64-windows-ninja  | 20:04:11
 clang-lld-x86_64-2stage | 19:51:22
 clang-cmake-armv8-selfhost-neon | 19:50:31
 clang-cmake-armv7-full  | 19:11:00
 clang-s390x-linux   | 17:56:59
 clang-cmake-armv8-full  | 16:52:00
 clang-cmake-armv7-quick | 16:32:07
 clang-cmake-armv8-quick | 16:32:01
 lldb-amd64-ninja-netbsd8| 16:30:41
 clang-cmake-armv7-global-isel   | 16:28:25
 clang-cmake-aarch64-quick   | 16:20:37
 clang-cmake-aarch64-global-isel | 16:06:22
 clang-cmake-armv8-lld   | 16:03:00
 clang-cmake-x86_64-avx2-linux   | 15:56:52
 clang-cmake-armv8-global-isel   | 15:41:18
 clang-s390x-linux-lnt   | 15:24:54
 clang-cmake-x86_64-sde-avx512-linux | 15:24:17
 clang-s390x-linux-multistage| 15:10:24
 clang-cmake-armv7-selfhost  | 15:09:13
 clang-hexagon-elf   | 15:08:41
 clang-cmake-armv7-selfhost-neon | 15:06:56
 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast  | 13:25:43
 clang-ppc64le-linux-multistage  | 11:56:59
 clang-cmake-aarch64-full| 10:19:14
 clang-cmake-aarch64-lld | 09:09:12
 sanitizer-x86_64-linux  | 07:59:19
 clang-ppc64be-linux-lnt | 07:57:13
 clang-atom-d525-fedora-rel  | 07:57:01
 clang-x86_64-debian-fast| 07:52:41
 clang-ppc64be-linux-multistage  | 07:52:31
 clang-ppc64be-linux | 07:50:12
 clang-ppc64le-linux-lnt | 07:37:10
 clang-ppc64le-linux | 07:24:38
 sanitizer-x86_64-linux-fast | 07:13:18
 clang-with-thin-lto-ubuntu  | 06:51:42
 clang-cmake-armv7-lnt   | 06:43:21
 sanitizer-ppc64le-linux | 06:26:02
 sanitizer-x86_64-linux-bootstrap-msan   | 05:55:48
 sanitizer-x86_64-linux-bootstrap| 05:36:43
 clang-cmake-armv8-lnt   | 05:31:37
 sanitizer-x86_64-linux-bootstrap-ubsan  | 05:30:43
 clang-with-lto-ubuntu   | 05:27:00
 lld-x86_64-darwin13 | 04:32:02
 sanitizer-ppc64be-linux | 03:54:05
 clang-x86_64-linux-abi-test | 03:43:49
 clang-cmake-x86_64-avx2-linux-perf  | 01:52:16
 clang-cuda-build| 01:41:43
 clang-tools-sphinx-docs | 01:24:29
 polly-arm-linux | 01:03:29
 lld-x86_64-freebsd  | 01:00:32
 lld-perf-testsuite  | 00:57:52
 sanitizer-x86_64-linux-fuzzer   | 00:43:56
 llvm-hexagon-elf| 00:42:36
 lldb-x86_64-ubuntu-14.04-buildserver| 00:41:52
 

Buildbot numbers for the week of 01/06/2019 - 01/12/2019

[Galina Kistanova via cfe-commits]

Hello everyone,

Below are some buildbot numbers for the week of 01/06/2019 - 01/12/2019.

Please see the same data in attached csv files:

The longest time each builder was red during the week;
"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to red or from red to green);
Count of commits by project;
Number of completed builds, failed builds and average build time for
successful builds per active builder;
Average waiting time for a revision to get build result per active builder
(response time).

Thanks

Galina


The longest time each builder was red during the week:
  buildername  |  was_red
---+--
 clang-cmake-thumbv8-full-sh   | 105:13:50
 clang-cmake-thumbv7-full-sh   | 101:13:36
 lldb-x86_64-ubuntu-14.04-buildserver  | 77:26:59
 libcxx-libcxxabi-x86_64-linux-debian-noexceptions | 48:07:33
 libcxx-libcxxabi-x86_64-linux-debian  | 48:02:35
 libcxx-libcxxabi-singlethreaded-x86_64-linux-debian   | 48:01:48
 sanitizer-x86_64-linux-android| 39:18:47
 clang-lld-x86_64-2stage   | 22:54:40
 clang-with-thin-lto-ubuntu| 20:38:50
 clang-with-lto-ubuntu | 19:47:33
 clang-cmake-armv7-selfhost| 13:42:02
 lld-x86_64-darwin13   | 11:27:11
 clang-x86_64-linux-selfhost-modules   | 09:15:13
 clang-cmake-aarch64-lld   | 08:53:00
 clang-cmake-armv7-global-isel | 08:49:12
 llvm-hexagon-elf  | 08:09:23
 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast| 07:38:28
 clang-cmake-armv7-selfhost-neon   | 07:25:50
 lldb-amd64-ninja-netbsd8  | 07:19:50
 clang-cmake-aarch64-full  | 07:18:29
 lldb-x64-windows-ninja| 07:17:30
 sanitizer-x86_64-linux-bootstrap-msan | 07:06:08
 clang-cmake-armv8-lld | 07:03:13
 llvm-clang-x86_64-expensive-checks-win| 06:51:45
 clang-cmake-armv8-global-isel | 06:50:50
 clang-cmake-armv8-selfhost-neon   | 06:45:32
 clang-cmake-aarch64-quick | 06:37:15
 sanitizer-x86_64-linux-fast   | 06:35:29
 clang-cmake-armv8-quick   | 06:32:45
 sanitizer-x86_64-linux-bootstrap  | 06:26:16
 clang-cmake-armv7-quick   | 06:19:22
 clang-hexagon-elf | 06:18:03
 clang-cmake-armv8-full| 06:10:06
 clang-x64-windows-msvc| 06:02:24
 clang-cmake-armv7-full| 06:00:34
 clang-cmake-aarch64-global-isel   | 05:51:54
 clang-ppc64le-linux-lnt   | 04:08:01
 clang-ppc64le-linux-multistage| 03:46:44
 sanitizer-ppc64be-linux   | 03:30:38
 clang-ppc64le-linux   | 03:18:47
 sanitizer-x86_64-linux| 03:16:13
 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast  | 03:05:27
 sanitizer-ppc64le-linux   | 03:02:44
 sanitizer-x86_64-linux-autoconf   | 02:21:18
 libcxx-libcxxabi-libunwind-armv8-linux| 02:19:55
 libcxx-libcxxabi-libunwind-aarch64-linux  | 02:18:59
 libcxx-libcxxabi-libunwind-armv8-linux-noexceptions   | 02:17:25
 libcxx-libcxxabi-libunwind-aarch64-linux-noexceptions | 02:17:10
 clang-ppc64be-linux-multistage| 02:16:47
 clang-ppc64be-linux-lnt   | 02:12:18
 clang-cuda-build  | 02:12:16
 clang-s390x-linux-lnt | 01:58:14
 clang-ppc64be-linux   | 01:47:17
 clang-s390x-linux | 01:42:40
 lld-x86_64-win7   | 01:26:31
 clang-cmake-armv8-lnt | 01:23:55
 clang-cmake-x86_64-avx2-linux-perf| 01:23:04
 sanitizer-x86_64-linux-bootstrap-ubsan| 01:18:51
 polly-amd64-linux | 01:01:48
 clang-atom-d525-fedora-rel| 00:56:47
 polly-arm-linux   | 00:55:22
 clang-cmake-x86_64-avx2-linux | 00:54:49
 sanitizer-windows | 00:53:24
 clang-x86_64-linux-abi-test   | 00:51:49
 

r351911 - [Sema][ObjC] Check whether a DelayedDiagnosticPool has been pushed

[Akira Hatanaka via cfe-commits]

Author: ahatanak
Date: Tue Jan 22 16:55:48 2019
New Revision: 351911

URL: http://llvm.org/viewvc/llvm-project?rev=351911=rev
Log:
[Sema][ObjC] Check whether a DelayedDiagnosticPool has been pushed
before adding a delayed diagnostic to DelayedDiagnostics.

This fixes an assertion failure in Sema::DelayedDiagnostics::add that
was caused by the changes made in r141037.

rdar://problem/42782323

Modified:
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/test/SemaObjCXX/arc-0x.mm

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=351911=351910=351911=diff
==
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Jan 22 16:55:48 2019
@@ -12545,9 +12545,13 @@ ParmVarDecl *Sema::CheckParameter(DeclCo
 //   - otherwise, it's an error
 if (T->isArrayType()) {
   if (!T.isConstQualified()) {
-DelayedDiagnostics.add(
-sema::DelayedDiagnostic::makeForbiddenType(
-NameLoc, diag::err_arc_array_param_no_ownership, T, false));
+if (DelayedDiagnostics.shouldDelayDiagnostics())
+  DelayedDiagnostics.add(
+  sema::DelayedDiagnostic::makeForbiddenType(
+  NameLoc, diag::err_arc_array_param_no_ownership, T, false));
+else
+  Diag(NameLoc, diag::err_arc_array_param_no_ownership)
+  << TSInfo->getTypeLoc().getSourceRange();
   }
   lifetime = Qualifiers::OCL_ExplicitNone;
 } else {

Modified: cfe/trunk/test/SemaObjCXX/arc-0x.mm
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjCXX/arc-0x.mm?rev=351911=351910=351911=diff
==
--- cfe/trunk/test/SemaObjCXX/arc-0x.mm (original)
+++ cfe/trunk/test/SemaObjCXX/arc-0x.mm Tue Jan 22 16:55:48 2019
@@ -101,3 +101,13 @@ namespace rdar12078752 {
 __autoreleasing auto o3 = o;
   }
 }
+
+namespace test_err_arc_array_param_no_ownership {
+  template 
+  void func(T a) {}
+
+  void test() {
+func([](A *a[]){}); // expected-error{{must explicitly describe intended 
ownership of an object array parameter}}
+func(^(A *a[]){}); // expected-error{{must explicitly describe intended 
ownership of an object array parameter}}
+  }
+}


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

[PATCH] D56925: Do not use frame pointer by default for MSP430

[Dmitry Mikushin via Phabricator via cfe-commits]

dmikushin updated this revision to Diff 182998.
dmikushin added a comment.

@grimar Please find the updated patch with a test case.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56925/new/

https://reviews.llvm.org/D56925

Files:
  lib/Driver/ToolChains/Clang.cpp
  test/CodeGen/msp430-fp-elim.c


Index: test/CodeGen/msp430-fp-elim.c
===
--- /dev/null
+++ test/CodeGen/msp430-fp-elim.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -mdisable-fp-elim -triple msp430 -S %s -o - | FileCheck %s 
--check-prefix=FP_ENFORCED
+// RUN: %clang_cc1 -triple msp430 -S %s -o - | FileCheck %s 
--check-prefix=FP_DEFAULT
+
+// Check the frame pointer is not used on MSP430 by default, but can be 
forcibly turned on.
+
+// FP_ENFORCED: push r4
+// FP_ENFORCED: calla r4
+// FP_ENFORCED: pop r4
+// FP_DEFAULT: .globl fp_elim_check
+// FP_DEFAULT-NOT: push r4
+// FP_DEFAULT: calla r4
+// FP_DEFAULT-NOT: pop r4
+
+void fp_elim_check()
+{
+   asm volatile ("calla r4");
+}
+
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -519,10 +519,10 @@
   case llvm::Triple::xcore:
   case llvm::Triple::wasm32:
   case llvm::Triple::wasm64:
+  case llvm::Triple::msp430:
 // XCore never wants frame pointers, regardless of OS.
 // WebAssembly never wants frame pointers.
 return false;
-  case llvm::Triple::msp430:
   case llvm::Triple::riscv32:
   case llvm::Triple::riscv64:
 return !areOptimizationsEnabled(Args);


Index: test/CodeGen/msp430-fp-elim.c
===
--- /dev/null
+++ test/CodeGen/msp430-fp-elim.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -mdisable-fp-elim -triple msp430 -S %s -o - | FileCheck %s --check-prefix=FP_ENFORCED
+// RUN: %clang_cc1 -triple msp430 -S %s -o - | FileCheck %s --check-prefix=FP_DEFAULT
+
+// Check the frame pointer is not used on MSP430 by default, but can be forcibly turned on.
+
+// FP_ENFORCED: push r4
+// FP_ENFORCED: calla r4
+// FP_ENFORCED: pop r4
+// FP_DEFAULT: .globl fp_elim_check
+// FP_DEFAULT-NOT: push r4
+// FP_DEFAULT: calla r4
+// FP_DEFAULT-NOT: pop r4
+
+void fp_elim_check()
+{
+	asm volatile ("calla r4");
+}
+
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -519,10 +519,10 @@
   case llvm::Triple::xcore:
   case llvm::Triple::wasm32:
   case llvm::Triple::wasm64:
+  case llvm::Triple::msp430:
 // XCore never wants frame pointers, regardless of OS.
 // WebAssembly never wants frame pointers.
 return false;
-  case llvm::Triple::msp430:
   case llvm::Triple::riscv32:
   case llvm::Triple::riscv64:
 return !areOptimizationsEnabled(Args);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56160: [clang-tidy] modernize-use-trailing-return check

[Bernhard Manfred Gruber via Phabricator via cfe-commits]

bernhardmgruber added a comment.

Thank you again @JonasToth for all your valueable input! I could almost 
successfully run my check on the llvm/lib subfolder. I created a compilation 
database from within Visual Studio using an extension called SourceTrail 
.
 One of the issues was the following:

  struct Object { long long value; };
  class C {
int Object;
struct Object m();
  };
  Object C::m() { return {0}; }

If I rewrite this to the following

  struct Object { long long value; };
  class C {
int Object;
struct Object m();
  };
  auto C::m() -> Object { return {0}; }

a compilation error occurs afterwards, because Object now refers to the class 
member. I discovered a similar problem colliding with the name of a function 
argument. So essentially, I believe I now require a name lookup of the return 
type (if it is unqualified) in the scope of the function. In case such a name 
already exists, i have to prefix `struct/class` or perform no replacement. I 
looked at the documentation of `ASTContext`, but it seems all the good stuff is 
in `DeclContext`, which I have not available. How can I perform a name lookup 
inside the `check` member function?

Thank you for any tips! And thank you for the `decltype` hint, I will add some 
more tests.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56160/new/

https://reviews.llvm.org/D56160



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

[PATCH] D56935: [NewPM] Add support for new-PM plugins to clang

[Marco Elver via Phabricator via cfe-commits]

melver added a comment.

>> This generally looks sane. What will happen on windows though? Will it 
>> silently fa
> 
> AFAIK PassPlugin::Load uses sys::DynamicLibrary::getPermanentLibrary, which 
> uses DynamicLibrary::HandleSet::AddLibrary which works for Windows as well. 
> (The story is similar to legacy -fplugin=).

s/AddLibrary/DLOpen/ -- DLOpen and others in DynamicLibrary:: are wrappers 
around platform-specific code. On Windows the implementation is in: 
llvm/lib/Support/Windows/DynamicLibrary.inc


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56935/new/

https://reviews.llvm.org/D56935



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

[PATCH] D57072: Don't codegen an unreachable return block

[Brad Moody via Phabricator via cfe-commits]

bmoody created this revision.
bmoody added reviewers: craig.topper, dberris, rjmccall.
Herald added a subscriber: cfe-commits.

This patch adds a check at the end of CodeGenFunction::FinishFunction to delete 
the return block if it isn't reachable.

Without this patch applied the code generated for the test cases is:

define void @f1() {
entry:

  call void @abort()
  unreachable

return:   ; No predecessors!

  ret void

}

define i8* @f2() {
entry:

  %retval = alloca i8*, align 8
  call void @abort()
  unreachable

return:   ; No predecessors!

  %0 = load i8*, i8** %retval, align 8
  ret i8* %0

}

This sort-of addresses the FIXME in CodeGenFunction::EmitReturnBlock:

  // FIXME: We are at an unreachable point, there is no reason to emit the block
  // unless it has uses. However, we still need a place to put the debug
  // region.end for now.

I'm not sure if this FIXME still applies with this patch - it's not clear if 
the intent is to avoid generating the unreachable code in the first place, 
rather than generating it and then deleting it.  Seems like it would complicate 
the rest of the codegen to avoid generating it in the first place.


Repository:
  rC Clang

https://reviews.llvm.org/D57072

Files:
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/CodeGenFunction.cpp
  test/CodeGen/unreachable-ret.c
  test/OpenMP/parallel_reduction_codegen.cpp

Index: test/OpenMP/parallel_reduction_codegen.cpp
===
--- test/OpenMP/parallel_reduction_codegen.cpp
+++ test/OpenMP/parallel_reduction_codegen.cpp
@@ -622,7 +622,7 @@
 
 // CHECK-NOT: call i32 @__kmpc_reduce
 
-// CHECK: ret void
+// CHECK: }
 
 // CHECK: define {{.*}} i{{[0-9]+}} [[TMAIN_INT]]()
 // CHECK: [[TEST:%.+]] = alloca [[S_INT_TY]],
Index: test/CodeGen/unreachable-ret.c
===
--- /dev/null
+++ test/CodeGen/unreachable-ret.c
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s
+
+extern void abort() __attribute__((noreturn));
+
+void f1() {
+  abort();
+}
+// CHECK-LABEL: define void @f1()
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   call void @abort()
+// CHECK-NEXT:   unreachable
+// CHECK-NEXT: }
+
+void *f2() {
+  abort();
+  return 0;
+}
+// CHECK-LABEL: define i8* @f2()
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   call void @abort()
+// CHECK-NEXT:   unreachable
+// CHECK-NEXT: }
+
Index: lib/CodeGen/CodeGenFunction.cpp
===
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -255,6 +255,7 @@
 if (CurBB->empty() || ReturnBlock.getBlock()->use_empty()) {
   ReturnBlock.getBlock()->replaceAllUsesWith(CurBB);
   delete ReturnBlock.getBlock();
+  ReturnBlock = JumpDest();
 } else
   EmitBlock(ReturnBlock.getBlock());
 return llvm::DebugLoc();
@@ -274,6 +275,7 @@
   Builder.SetInsertPoint(BI->getParent());
   BI->eraseFromParent();
   delete ReturnBlock.getBlock();
+  ReturnBlock = JumpDest();
   return Loc;
 }
   }
@@ -448,6 +450,19 @@
   // 5. Width of vector aguments and return types for functions called by this
   //function.
   CurFn->addFnAttr("min-legal-vector-width", llvm::utostr(LargestVectorWidth));
+
+  // If we generated an unreachable return block, delete it now.
+  if (ReturnBlock.isValid() && ReturnBlock.getBlock()->use_empty()) {
+Builder.ClearInsertionPoint();
+ReturnBlock.getBlock()->eraseFromParent();
+  }
+  if (ReturnValue.isValid()) {
+auto *RetAlloca = dyn_cast(ReturnValue.getPointer());
+if (RetAlloca && RetAlloca->use_empty()) {
+  RetAlloca->eraseFromParent();
+  ReturnValue = Address::invalid();
+}
+  }
 }
 
 /// ShouldInstrumentFunction - Return true if the current function should be
Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -2896,15 +2896,6 @@
 RV = SI->getValueOperand();
 SI->eraseFromParent();
 
-// If that was the only use of the return value, nuke it as well now.
-auto returnValueInst = ReturnValue.getPointer();
-if (returnValueInst->use_empty()) {
-  if (auto alloca = dyn_cast(returnValueInst)) {
-alloca->eraseFromParent();
-ReturnValue = Address::invalid();
-  }
-}
-
   // Otherwise, we have to do a simple load.
   } else {
 RV = Builder.CreateLoad(ReturnValue);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55400: [analyzer] Move out tracking retain count for OSObjects into a separate checker

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added a comment.

> Hmmm, does this mess with options that bad? Could you please clarify?

`registerChecker` gets-or-creates a checker object. A checker name (used for 
getting the options) is set the first time it's created.
The checker which was created first "wins" and gets to name the resulting 
checker.
In practice it basically means that options and checkers reusing the same class 
do not work.
Do you have better ideas on how this could be arranged?

I think the current situation is a mess - ideally I would prefer to be able to 
access the options in the constructor, but we can't even do that,
since `registerChecker` sets the checker name and is called after the object 
has been constructed.
It seems that it would only make sense if the checker name is known at the time 
the checker is constructed: probably the function `registerXChecker` should get 
it as an argument.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55400/new/

https://reviews.llvm.org/D55400



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

[PATCH] D51866: [analyzer][UninitializedObjectChecker] New flag to ignore guarded uninitialized fields

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

If you don't mind, I'd prefer to finally get over this patch. I'll commit on 
the 31st, but will wait for any potential feedback 'til then.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D51866/new/

https://reviews.llvm.org/D51866



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

r351889 - [mips] Replace help-text for '-m{no}-relax-pic-calls'. NFC

[Vladimir Stefanovic via cfe-commits]

Author: vstefanovic
Date: Tue Jan 22 14:33:53 2019
New Revision: 351889

URL: http://llvm.org/viewvc/llvm-project?rev=351889=rev
Log:
[mips] Replace help-text for '-m{no}-relax-pic-calls'. NFC

Thanks to Simon Dardis for the new text.

Modified:
cfe/trunk/include/clang/Driver/Options.td

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=351889=351888=351889=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Tue Jan 22 14:33:53 2019
@@ -2419,12 +2419,12 @@ def mno_odd_spreg : Flag<["-"], "mno-odd
   Flags<[HelpHidden]>;
 def mrelax_pic_calls : Flag<["-"], "mrelax-pic-calls">,
   Group,
-  HelpText<"Try turning PIC calls (j{al}r{c} $25) into direct calls "
-  "(MIPS only)">, Flags<[HelpHidden]>;
+  HelpText<"Produce relaxation hints for linkers to try optimizing PIC "
+   "call sequences into direct calls (MIPS only)">, 
Flags<[HelpHidden]>;
 def mno_relax_pic_calls : Flag<["-"], "mno-relax-pic-calls">,
   Group,
-  HelpText<"Do not try turning PIC calls (j{al}r{c} $25) into direct calls "
-  "(MIPS only)">, Flags<[HelpHidden]>;
+  HelpText<"Do not produce relaxation hints for linkers to try optimizing PIC "
+   "call sequences into direct calls (MIPS only)">, 
Flags<[HelpHidden]>;
 def mglibc : Flag<["-"], "mglibc">, Group, Flags<[HelpHidden]>;
 def muclibc : Flag<["-"], "muclibc">, Group, Flags<[HelpHidden]>;
 def module_file_info : Flag<["-"], "module-file-info">, 
Flags<[DriverOption,CC1Option]>, Group,


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

[PATCH] D56984: [libunwind] Silence warnings about unused parameters

[Martin Storsjö via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rUNW351888: Silence warnings about unused parameters (authored 
by mstorsjo, committed by ).

Repository:
  rUNW libunwind

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56984/new/

https://reviews.llvm.org/D56984

Files:
  src/AddressSpace.hpp
  src/Unwind-seh.cpp
  src/UnwindCursor.hpp


Index: src/Unwind-seh.cpp
===
--- src/Unwind-seh.cpp
+++ src/Unwind-seh.cpp
@@ -52,6 +52,7 @@
 /// Exception cleanup routine used by \c _GCC_specific_handler to
 /// free foreign exceptions.
 static void seh_exc_cleanup(_Unwind_Reason_Code urc, _Unwind_Exception *exc) {
+  (void)urc;
   if (exc->exception_class != kSEHExceptionClass)
 _LIBUNWIND_ABORT("SEH cleanup called on non-SEH exception");
   free(exc);
@@ -210,6 +211,8 @@
 __libunwind_seh_personality(int version, _Unwind_Action state,
 uint64_t klass, _Unwind_Exception *exc,
 struct _Unwind_Context *context) {
+  (void)version;
+  (void)klass;
   EXCEPTION_RECORD ms_exc;
   bool phase2 = (state & (_UA_SEARCH_PHASE|_UA_CLEANUP_PHASE)) == 
_UA_CLEANUP_PHASE;
   ms_exc.ExceptionCode = STATUS_GCC_THROW;
Index: src/UnwindCursor.hpp
===
--- src/UnwindCursor.hpp
+++ src/UnwindCursor.hpp
@@ -788,6 +788,8 @@
   if (regNum >= UNW_ARM_D0 && regNum <= UNW_ARM_D31) return true;
 #elif defined(_LIBUNWIND_TARGET_AARCH64)
   if (regNum >= UNW_ARM64_D0 && regNum <= UNW_ARM64_D31) return true;
+#else
+  (void)regNum;
 #endif
   return false;
 }
@@ -815,6 +817,7 @@
 #elif defined(_LIBUNWIND_TARGET_AARCH64)
   return _msContext.V[regNum - UNW_ARM64_D0].D[0];
 #else
+  (void)regNum;
   _LIBUNWIND_ABORT("float registers unimplemented");
 #endif
 }
@@ -842,6 +845,8 @@
 #elif defined(_LIBUNWIND_TARGET_AARCH64)
   _msContext.V[regNum - UNW_ARM64_D0].D[0] = value;
 #else
+  (void)regNum;
+  (void)value;
   _LIBUNWIND_ABORT("float registers unimplemented");
 #endif
 }
Index: src/AddressSpace.hpp
===
--- src/AddressSpace.hpp
+++ src/AddressSpace.hpp
@@ -456,6 +456,8 @@
 #elif defined(_LIBUNWIND_SUPPORT_SEH_UNWIND) && defined(_WIN32)
   // Don't even bother, since Windows has functions that do all this stuff
   // for us.
+  (void)targetAddr;
+  (void)info;
   return true;
 #elif defined(_LIBUNWIND_ARM_EHABI) && defined(__BIONIC__) &&  
\
 (__ANDROID_API__ < 21)
@@ -596,6 +598,11 @@
   return true;
 }
   }
+#else
+  (void)addr;
+  (void)buf;
+  (void)bufLen;
+  (void)offset;
 #endif
   return false;
 }


Index: src/Unwind-seh.cpp
===
--- src/Unwind-seh.cpp
+++ src/Unwind-seh.cpp
@@ -52,6 +52,7 @@
 /// Exception cleanup routine used by \c _GCC_specific_handler to
 /// free foreign exceptions.
 static void seh_exc_cleanup(_Unwind_Reason_Code urc, _Unwind_Exception *exc) {
+  (void)urc;
   if (exc->exception_class != kSEHExceptionClass)
 _LIBUNWIND_ABORT("SEH cleanup called on non-SEH exception");
   free(exc);
@@ -210,6 +211,8 @@
 __libunwind_seh_personality(int version, _Unwind_Action state,
 uint64_t klass, _Unwind_Exception *exc,
 struct _Unwind_Context *context) {
+  (void)version;
+  (void)klass;
   EXCEPTION_RECORD ms_exc;
   bool phase2 = (state & (_UA_SEARCH_PHASE|_UA_CLEANUP_PHASE)) == _UA_CLEANUP_PHASE;
   ms_exc.ExceptionCode = STATUS_GCC_THROW;
Index: src/UnwindCursor.hpp
===
--- src/UnwindCursor.hpp
+++ src/UnwindCursor.hpp
@@ -788,6 +788,8 @@
   if (regNum >= UNW_ARM_D0 && regNum <= UNW_ARM_D31) return true;
 #elif defined(_LIBUNWIND_TARGET_AARCH64)
   if (regNum >= UNW_ARM64_D0 && regNum <= UNW_ARM64_D31) return true;
+#else
+  (void)regNum;
 #endif
   return false;
 }
@@ -815,6 +817,7 @@
 #elif defined(_LIBUNWIND_TARGET_AARCH64)
   return _msContext.V[regNum - UNW_ARM64_D0].D[0];
 #else
+  (void)regNum;
   _LIBUNWIND_ABORT("float registers unimplemented");
 #endif
 }
@@ -842,6 +845,8 @@
 #elif defined(_LIBUNWIND_TARGET_AARCH64)
   _msContext.V[regNum - UNW_ARM64_D0].D[0] = value;
 #else
+  (void)regNum;
+  (void)value;
   _LIBUNWIND_ABORT("float registers unimplemented");
 #endif
 }
Index: src/AddressSpace.hpp
===
--- src/AddressSpace.hpp
+++ src/AddressSpace.hpp
@@ -456,6 +456,8 @@
 #elif defined(_LIBUNWIND_SUPPORT_SEH_UNWIND) && defined(_WIN32)
   // Don't even bother, since Windows has functions that do all this stuff
   // for us.
+  (void)targetAddr;
+  (void)info;
   return true;
 #elif defined(_LIBUNWIND_ARM_EHABI) && defined(__BIONIC__) &&  \
 (__ANDROID_API__ < 21)
@@ -596,6 +598,11 @@
   return true;
 }
   }
+#else
+  

[libunwind] r351888 - Silence warnings about unused parameters

[Martin Storsjo via cfe-commits]

Author: mstorsjo
Date: Tue Jan 22 14:12:23 2019
New Revision: 351888

URL: http://llvm.org/viewvc/llvm-project?rev=351888=rev
Log:
Silence warnings about unused parameters

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

Modified:
libunwind/trunk/src/AddressSpace.hpp
libunwind/trunk/src/Unwind-seh.cpp
libunwind/trunk/src/UnwindCursor.hpp

Modified: libunwind/trunk/src/AddressSpace.hpp
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/AddressSpace.hpp?rev=351888=351887=351888=diff
==
--- libunwind/trunk/src/AddressSpace.hpp (original)
+++ libunwind/trunk/src/AddressSpace.hpp Tue Jan 22 14:12:23 2019
@@ -456,6 +456,8 @@ inline bool LocalAddressSpace::findUnwin
 #elif defined(_LIBUNWIND_SUPPORT_SEH_UNWIND) && defined(_WIN32)
   // Don't even bother, since Windows has functions that do all this stuff
   // for us.
+  (void)targetAddr;
+  (void)info;
   return true;
 #elif defined(_LIBUNWIND_ARM_EHABI) && defined(__BIONIC__) &&  
\
 (__ANDROID_API__ < 21)
@@ -596,6 +598,11 @@ inline bool LocalAddressSpace::findFunct
   return true;
 }
   }
+#else
+  (void)addr;
+  (void)buf;
+  (void)bufLen;
+  (void)offset;
 #endif
   return false;
 }

Modified: libunwind/trunk/src/Unwind-seh.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/Unwind-seh.cpp?rev=351888=351887=351888=diff
==
--- libunwind/trunk/src/Unwind-seh.cpp (original)
+++ libunwind/trunk/src/Unwind-seh.cpp Tue Jan 22 14:12:23 2019
@@ -52,6 +52,7 @@ static const uint64_t kSEHExceptionClass
 /// Exception cleanup routine used by \c _GCC_specific_handler to
 /// free foreign exceptions.
 static void seh_exc_cleanup(_Unwind_Reason_Code urc, _Unwind_Exception *exc) {
+  (void)urc;
   if (exc->exception_class != kSEHExceptionClass)
 _LIBUNWIND_ABORT("SEH cleanup called on non-SEH exception");
   free(exc);
@@ -210,6 +211,8 @@ extern "C" _Unwind_Reason_Code
 __libunwind_seh_personality(int version, _Unwind_Action state,
 uint64_t klass, _Unwind_Exception *exc,
 struct _Unwind_Context *context) {
+  (void)version;
+  (void)klass;
   EXCEPTION_RECORD ms_exc;
   bool phase2 = (state & (_UA_SEARCH_PHASE|_UA_CLEANUP_PHASE)) == 
_UA_CLEANUP_PHASE;
   ms_exc.ExceptionCode = STATUS_GCC_THROW;

Modified: libunwind/trunk/src/UnwindCursor.hpp
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/UnwindCursor.hpp?rev=351888=351887=351888=diff
==
--- libunwind/trunk/src/UnwindCursor.hpp (original)
+++ libunwind/trunk/src/UnwindCursor.hpp Tue Jan 22 14:12:23 2019
@@ -788,6 +788,8 @@ bool UnwindCursor::validFloatReg(i
   if (regNum >= UNW_ARM_D0 && regNum <= UNW_ARM_D31) return true;
 #elif defined(_LIBUNWIND_TARGET_AARCH64)
   if (regNum >= UNW_ARM64_D0 && regNum <= UNW_ARM64_D31) return true;
+#else
+  (void)regNum;
 #endif
   return false;
 }
@@ -815,6 +817,7 @@ unw_fpreg_t UnwindCursor::getFloat
 #elif defined(_LIBUNWIND_TARGET_AARCH64)
   return _msContext.V[regNum - UNW_ARM64_D0].D[0];
 #else
+  (void)regNum;
   _LIBUNWIND_ABORT("float registers unimplemented");
 #endif
 }
@@ -842,6 +845,8 @@ void UnwindCursor::setFloatReg(int
 #elif defined(_LIBUNWIND_TARGET_AARCH64)
   _msContext.V[regNum - UNW_ARM64_D0].D[0] = value;
 #else
+  (void)regNum;
+  (void)value;
   _LIBUNWIND_ABORT("float registers unimplemented");
 #endif
 }


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

[PATCH] D56878: [mips] Add '-mrelax-pic-calls', '-mno-relax-pic-calls'

[Vladimir Stefanovic via Phabricator via cfe-commits]

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



Comment at: cfe/trunk/include/clang/Driver/Options.td:2423
+  Group,
+  HelpText<"Try turning PIC calls (j{al}r{c} $25) into direct calls "
+  "(MIPS only)">, Flags<[HelpHidden]>;

sdardis wrote:
> I think this help text could be a little better. Instead of implying that the 
> compiler could turn PIC calls into direct calls, something like "Produce 
> relaxation hints for linkers to try optimizing PIC call sequences into direct 
> calls", I believe describes the optimization better. Likewise in the negative 
> sense for the -mno-relax-pic-calls.
Hi Simon, I'll replace the text with your version, it's better indeed. Thanks.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56878/new/

https://reviews.llvm.org/D56878



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

[PATCH] D56935: [NewPM] Add support for new-PM plugins to clang

[Marco Elver via Phabricator via cfe-commits]

melver added a comment.

Thanks!

In D56935#1366756 , @philip.pfaffe 
wrote:

> This generally looks sane. What will happen on windows though? Will it 
> silently fail?


AFAIK PassPlugin::Load uses sys::DynamicLibrary::getPermanentLibrary, which 
uses DynamicLibrary::HandleSet::AddLibrary which works for Windows as well. 
(The story is similar to legacy -fplugin=).




Comment at: clang/include/clang/Basic/CodeGenOptions.h:292
+  /// List of dynamic shared object files to be loaded as pass plugins.
+  std::vector PassPlugins;
+

philip.pfaffe wrote:
> This  should be SmallVector.
Not sure if this is better. getAllArgValues returns a vector, which is 
why I think the above members are also vector. And std::vector cannot 
be assigned to SmallVector, which required an extra line in 
CompilerInvocation.cpp.

Let me know what you think.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56935/new/

https://reviews.llvm.org/D56935



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

[PATCH] D56935: [NewPM] Add support for new-PM plugins to clang

[Marco Elver via Phabricator via cfe-commits]

melver updated this revision to Diff 182974.
melver marked 2 inline comments as done.
melver added a comment.

- Use SmallVector in CodeGenOptions.h


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56935/new/

https://reviews.llvm.org/D56935

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1322,6 +1322,9 @@
 
   Opts.DefaultFunctionAttrs = Args.getAllArgValues(OPT_default_function_attr);
 
+  for (auto & : Args.getAllArgValues(OPT_fpass_plugin_EQ))
+Opts.PassPlugins.push_back(std::move(V));
+
   return Success;
 }
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5066,6 +5066,13 @@
 A->claim();
   }
 
+  // Forward -fpass-plugin=name.so to -cc1.
+  for (const Arg *A : Args.filtered(options::OPT_fpass_plugin_EQ)) {
+CmdArgs.push_back(
+Args.MakeArgString(Twine("-fpass-plugin=") + A->getValue()));
+A->claim();
+  }
+
   // Setup statistics file output.
   SmallString<128> StatsFile = getStatsFileName(Args, Output, Input, D);
   if (!StatsFile.empty())
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -37,6 +37,7 @@
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/SubtargetFeature.h"
 #include "llvm/Passes/PassBuilder.h"
+#include "llvm/Passes/PassPlugin.h"
 #include "llvm/Support/BuryPointer.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/MemoryBuffer.h"
@@ -962,6 +963,16 @@
 
   PassBuilder PB(TM.get(), PGOOpt);
 
+  // Attempt to load pass plugins and register their callbacks with PB.
+  for (auto  : CodeGenOpts.PassPlugins) {
+if (auto PassPlugin = PassPlugin::Load(PluginFN)) {
+  PassPlugin->registerPassBuilderCallbacks(PB);
+} else {
+  errs() << "Failed to load passes from '" << PluginFN
+ << "'. Request ignored.\n";
+}
+  }
+
   LoopAnalysisManager LAM(CodeGenOpts.DebugPassManager);
   FunctionAnalysisManager FAM(CodeGenOpts.DebugPassManager);
   CGSCCAnalysisManager CGAM(CodeGenOpts.DebugPassManager);
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1613,6 +1613,9 @@
 def fno_rwpi : Flag<["-"], "fno-rwpi">, Group;
 def fplugin_EQ : Joined<["-"], "fplugin=">, Group, 
Flags<[DriverOption]>, MetaVarName<"">,
   HelpText<"Load the named plugin (dynamic shared object)">;
+def fpass_plugin_EQ : Joined<["-"], "fpass-plugin=">,
+  Group, Flags<[CC1Option]>, MetaVarName<"">,
+  HelpText<"Load pass plugin from a dynamic shared object file (only with new 
pass manager).">;
 def fpreserve_as_comments : Flag<["-"], "fpreserve-as-comments">, 
Group;
 def fno_preserve_as_comments : Flag<["-"], "fno-preserve-as-comments">, 
Group, Flags<[CC1Option]>,
   HelpText<"Do not preserve comments in inline assembly">;
Index: clang/include/clang/Basic/CodeGenOptions.h
===
--- clang/include/clang/Basic/CodeGenOptions.h
+++ clang/include/clang/Basic/CodeGenOptions.h
@@ -17,6 +17,7 @@
 #include "clang/Basic/DebugInfoOptions.h"
 #include "clang/Basic/Sanitizers.h"
 #include "clang/Basic/XRayInstr.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/CodeGen.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Target/TargetOptions.h"
@@ -288,6 +289,9 @@
 
   std::vector DefaultFunctionAttrs;
 
+  /// List of dynamic shared object files to be loaded as pass plugins.
+  SmallVector PassPlugins;
+
 public:
   // Define accessors/mutators for code generation options of enumeration type.
 #define CODEGENOPT(Name, Bits, Default)


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1322,6 +1322,9 @@
 
   Opts.DefaultFunctionAttrs = Args.getAllArgValues(OPT_default_function_attr);
 
+  for (auto & : Args.getAllArgValues(OPT_fpass_plugin_EQ))
+Opts.PassPlugins.push_back(std::move(V));
+
   return Success;
 }
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5066,6 +5066,13 @@
 A->claim();
   }
 
+  // Forward -fpass-plugin=name.so to -cc1.
+  for (const Arg *A : 

[PATCH] D43871: [modules] No longer include stdlib.h from mm_malloc.h.

[Raphael Isemann via Phabricator via cfe-commits]

teemperor abandoned this revision.
teemperor added a comment.

So, the idea of going back to the headers and see if we can potentially remove 
mm_malloc from the modulemap didn't work out (mostly because a lot of headers 
include it indirectly).

However, when going back to the original issue i noticed that the stdlib.h 
that's at fault is the one from stdlibc++, not the glibc one. In fact, the 
issue becomes more clear when re-adding cstdlib to my stl modulemap:

  . 
/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/8.2.1/../../../../include/c++/8.2.1/stdlib.h
  While building module 'stl11' imported from 
/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/8.2.1/../../../../include/c++/8.2.1/stdlib.h:36:
  While building module '_Builtin_intrinsics' imported from 
/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/8.2.1/../../../../include/c++/8.2.1/x86_64-pc-linux-gnu/bits/opt_random.h:34:
  In file included from :2:
  In file included from /usr/lib/clang/7.0.1/include/immintrin.h:32:
  In file included from /usr/lib/clang/7.0.1/include/xmmintrin.h:39:
  In file included from /usr/lib/clang/7.0.1/include/mm_malloc.h:27:
  
/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/8.2.1/../../../../include/c++/8.2.1/stdlib.h:36:11:
 fatal error: cyclic dependency in module 'stl11': stl11 -> _Builtin_intrinsics
-> stl11
  # include 
^
  While building module 'stl11' imported from 
/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/8.2.1/../../../../include/c++/8.2.1/stdlib.h:36:
  In file included from :52:
  In file included from 
/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/8.2.1/../../../../include/c++/8.2.1/random:50:
  
/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/8.2.1/../../../../include/c++/8.2.1/x86_64-pc-linux-gnu/bits/opt_random.h:34:10:
 fatal error: could not build module
'_Builtin_intrinsics'
  #include 
   ^
  In file included from test.cpp:2:
  
/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/8.2.1/../../../../include/c++/8.2.1/stdlib.h:36:11:
 fatal error: could not build module 'stl11'
  # include 
^
  3 errors generated.

So to summarize: including stdlib.h (or cstdlib) is triggering our stl module 
to build, which in turn causes our `random` module to trigger which includes 
mm_malloc indirectly. If we move cstdlib and stdlib.h in their own modules we 
break the cyclic dependency. It's not optimal but I think it's less hacky than 
this patch or any of the other workarounds we came up with so far.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D43871/new/

https://reviews.llvm.org/D43871



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

[PATCH] D56652: [CMake][Fuchsia] Synchronize first and second stage builds

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added inline comments.



Comment at: cfe/trunk/cmake/caches/Fuchsia-stage2.cmake:29
 
-set(LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
 set(CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING "")

Out of curiosity, how come you decided to disable assertions? 
https://reviews.llvm.org/D41471 said "Release+Asserts is fast enough. No need 
to let insanity through.", and I do think assertions end up catching a bunch of 
issues, so I was wondering if the build time penalty became too high.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56652/new/

https://reviews.llvm.org/D56652



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

[PATCH] D56723: [CodeComplete] Propagate preferred types through parser in more cases

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

rsmith added a comment.

In D56723#1366529 , @ilya-biryukov 
wrote:

> I'll try playing around with your idea, my initial plan is to store preferred 
> type alongside the current token as a member of the `Parser` class and update 
> it when advancing to next token, when the parser backtracks and in the places 
> where we care about propagating the types.


`ConsumeToken` is a fairly hot function; if you can avoid changes there that'd 
be preferable. Tracking this automatically across tentative parse / rollback 
seems like a very nice idea.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56723/new/

https://reviews.llvm.org/D56723



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

[PATCH] D56924: Handle ObjCCategoryDecl class extensions for print

[David Goldman via Phabricator via cfe-commits]

dgoldman updated this revision to Diff 182971.
dgoldman added a comment.
Herald added a subscriber: jfb.

- Add test


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56924/new/

https://reviews.llvm.org/D56924

Files:
  lib/AST/Decl.cpp
  unittests/AST/NamedDeclPrinterTest.cpp


Index: unittests/AST/NamedDeclPrinterTest.cpp
===
--- unittests/AST/NamedDeclPrinterTest.cpp
+++ unittests/AST/NamedDeclPrinterTest.cpp
@@ -115,6 +115,18 @@
  "input.cc");
 }
 
+::testing::AssertionResult
+PrintedWrittenPropertyDeclObjCMatches(StringRef Code, StringRef DeclName,
+   StringRef ExpectedPrinted) {
+  std::vector Args{"-std=c++11", "-xobjective-c++"};
+  return PrintedNamedDeclMatches(Code,
+ Args,
+ /*SuppressUnwrittenScope*/ true,
+ 
objcPropertyDecl(hasName(DeclName)).bind("id"),
+ ExpectedPrinted,
+ "input.m");
+}
+
 } // unnamed namespace
 
 TEST(NamedDeclPrinter, TestNamespace1) {
@@ -179,3 +191,17 @@
 "A",
 "X::A"));
 }
+
+TEST(NamedDeclPrinter, TestObjCClassExtension) {
+  ASSERT_TRUE(PrintedWrittenPropertyDeclObjCMatches(
+R"(
+  @interface Obj
+  @end
+
+  @interface Obj ()
+  @property(nonatomic) int property;
+  @end
+)",
+"property",
+"(class extension)::property"));
+}
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -1604,6 +1604,11 @@
 OS << *ED;
   else
 continue;
+} else if (const auto *CD = dyn_cast(DC)) {
+  if (CD->IsClassExtension())
+ OS << "(class extension)";
+  else
+OS << *CD;
 } else {
   OS << *cast(DC);
 }


Index: unittests/AST/NamedDeclPrinterTest.cpp
===
--- unittests/AST/NamedDeclPrinterTest.cpp
+++ unittests/AST/NamedDeclPrinterTest.cpp
@@ -115,6 +115,18 @@
  "input.cc");
 }
 
+::testing::AssertionResult
+PrintedWrittenPropertyDeclObjCMatches(StringRef Code, StringRef DeclName,
+   StringRef ExpectedPrinted) {
+  std::vector Args{"-std=c++11", "-xobjective-c++"};
+  return PrintedNamedDeclMatches(Code,
+ Args,
+ /*SuppressUnwrittenScope*/ true,
+ objcPropertyDecl(hasName(DeclName)).bind("id"),
+ ExpectedPrinted,
+ "input.m");
+}
+
 } // unnamed namespace
 
 TEST(NamedDeclPrinter, TestNamespace1) {
@@ -179,3 +191,17 @@
 "A",
 "X::A"));
 }
+
+TEST(NamedDeclPrinter, TestObjCClassExtension) {
+  ASSERT_TRUE(PrintedWrittenPropertyDeclObjCMatches(
+R"(
+  @interface Obj
+  @end
+
+  @interface Obj ()
+  @property(nonatomic) int property;
+  @end
+)",
+"property",
+"(class extension)::property"));
+}
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -1604,6 +1604,11 @@
 OS << *ED;
   else
 continue;
+} else if (const auto *CD = dyn_cast(DC)) {
+  if (CD->IsClassExtension())
+ OS << "(class extension)";
+  else
+OS << *CD;
 } else {
   OS << *cast(DC);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56981: [libunwind] Enable LLVM_ENABLE_WARNINGS when building standalone out of tree

[Martin Storsjö via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL351875: Enable LLVM_ENABLE_WARNINGS when building standalone 
out of tree (authored by mstorsjo, committed by ).
Herald added subscribers: llvm-commits, christof.

Changed prior to commit:
  https://reviews.llvm.org/D56981?vs=182709=182966#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56981/new/

https://reviews.llvm.org/D56981

Files:
  libunwind/trunk/CMakeLists.txt


Index: libunwind/trunk/CMakeLists.txt
===
--- libunwind/trunk/CMakeLists.txt
+++ libunwind/trunk/CMakeLists.txt
@@ -73,6 +73,8 @@
   endif()
 
   if (EXISTS ${LLVM_CMAKE_PATH})
+# Enable warnings, otherwise -w gets added to the cflags by 
HandleLLVMOptions.
+set(LLVM_ENABLE_WARNINGS ON)
 list(APPEND CMAKE_MODULE_PATH "${LLVM_CMAKE_PATH}")
 include("${LLVM_CMAKE_PATH}/AddLLVM.cmake")
 include("${LLVM_CMAKE_PATH}/HandleLLVMOptions.cmake")


Index: libunwind/trunk/CMakeLists.txt
===
--- libunwind/trunk/CMakeLists.txt
+++ libunwind/trunk/CMakeLists.txt
@@ -73,6 +73,8 @@
   endif()
 
   if (EXISTS ${LLVM_CMAKE_PATH})
+# Enable warnings, otherwise -w gets added to the cflags by HandleLLVMOptions.
+set(LLVM_ENABLE_WARNINGS ON)
 list(APPEND CMAKE_MODULE_PATH "${LLVM_CMAKE_PATH}")
 include("${LLVM_CMAKE_PATH}/AddLLVM.cmake")
 include("${LLVM_CMAKE_PATH}/HandleLLVMOptions.cmake")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56985: [libunwind] Remove an unused variable

[Martin Storsjö via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rUNW351878: Remove an unused variable (authored by mstorsjo, 
committed by ).

Repository:
  rUNW libunwind

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56985/new/

https://reviews.llvm.org/D56985

Files:
  src/Unwind-seh.cpp


Index: src/Unwind-seh.cpp
===
--- src/Unwind-seh.cpp
+++ src/Unwind-seh.cpp
@@ -68,7 +68,6 @@
 _LIBUNWIND_EXPORT EXCEPTION_DISPOSITION
 _GCC_specific_handler(PEXCEPTION_RECORD ms_exc, PVOID frame, PCONTEXT ms_ctx,
   DISPATCHER_CONTEXT *disp, __personality_routine pers) {
-  unw_context_t uc;
   unw_cursor_t cursor;
   _Unwind_Exception *exc;
   _Unwind_Action action;


Index: src/Unwind-seh.cpp
===
--- src/Unwind-seh.cpp
+++ src/Unwind-seh.cpp
@@ -68,7 +68,6 @@
 _LIBUNWIND_EXPORT EXCEPTION_DISPOSITION
 _GCC_specific_handler(PEXCEPTION_RECORD ms_exc, PVOID frame, PCONTEXT ms_ctx,
   DISPATCHER_CONTEXT *disp, __personality_routine pers) {
-  unw_context_t uc;
   unw_cursor_t cursor;
   _Unwind_Exception *exc;
   _Unwind_Action action;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56982: [libunwind] Fix warnings about printf format strings

[Martin Storsjö via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL351876: Fix warnings about printf format strings (authored 
by mstorsjo, committed by ).
Herald added subscribers: llvm-commits, christof.

Changed prior to commit:
  https://reviews.llvm.org/D56982?vs=182711=182967#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56982/new/

https://reviews.llvm.org/D56982

Files:
  libunwind/trunk/src/Unwind-seh.cpp
  libunwind/trunk/src/Unwind-sjlj.c

Index: libunwind/trunk/src/Unwind-sjlj.c
===
--- libunwind/trunk/src/Unwind-sjlj.c
+++ libunwind/trunk/src/Unwind-sjlj.c
@@ -11,6 +11,7 @@
 
 #include 
 
+#include 
 #include 
 #include 
 #include 
@@ -107,7 +108,8 @@
 static _Unwind_Reason_Code
 unwind_phase1(struct _Unwind_Exception *exception_object) {
   _Unwind_FunctionContext_t c = __Unwind_SjLj_GetTopOfFunctionStack();
-  _LIBUNWIND_TRACE_UNWINDING("unwind_phase1: initial function-context=%p", c);
+  _LIBUNWIND_TRACE_UNWINDING("unwind_phase1: initial function-context=%p",
+ (void *)c);
 
   // walk each frame looking for a place to stop
   for (bool handlerNotFound = true; handlerNotFound; c = c->prev) {
@@ -116,17 +118,18 @@
 if (c == NULL) {
   _LIBUNWIND_TRACE_UNWINDING("unwind_phase1(ex_ojb=%p): reached "
  "bottom => _URC_END_OF_STACK",
-  exception_object);
+ (void *)exception_object);
   return _URC_END_OF_STACK;
 }
 
-_LIBUNWIND_TRACE_UNWINDING("unwind_phase1: function-context=%p", c);
+_LIBUNWIND_TRACE_UNWINDING("unwind_phase1: function-context=%p", (void *)c);
 // if there is a personality routine, ask it if it will want to stop at this
 // frame
 if (c->personality != NULL) {
   _LIBUNWIND_TRACE_UNWINDING("unwind_phase1(ex_ojb=%p): calling "
-"personality function %p",
- exception_object, c->personality);
+ "personality function %p",
+ (void *)exception_object,
+ (void *)c->personality);
   _Unwind_Reason_Code personalityResult = (*c->personality)(
   1, _UA_SEARCH_PHASE, exception_object->exception_class,
   exception_object, (struct _Unwind_Context *)c);
@@ -137,12 +140,14 @@
 handlerNotFound = false;
 exception_object->private_2 = (uintptr_t) c;
 _LIBUNWIND_TRACE_UNWINDING("unwind_phase1(ex_ojb=%p): "
-   "_URC_HANDLER_FOUND", exception_object);
+   "_URC_HANDLER_FOUND",
+   (void *)exception_object);
 return _URC_NO_REASON;
 
   case _URC_CONTINUE_UNWIND:
 _LIBUNWIND_TRACE_UNWINDING("unwind_phase1(ex_ojb=%p): "
-   "_URC_CONTINUE_UNWIND", exception_object);
+   "_URC_CONTINUE_UNWIND",
+   (void *)exception_object);
 // continue unwinding
 break;
 
@@ -150,7 +155,7 @@
 // something went wrong
 _LIBUNWIND_TRACE_UNWINDING(
 "unwind_phase1(ex_ojb=%p): _URC_FATAL_PHASE1_ERROR",
-exception_object);
+(void *)exception_object);
 return _URC_FATAL_PHASE1_ERROR;
   }
 }
@@ -161,19 +166,20 @@
 
 static _Unwind_Reason_Code
 unwind_phase2(struct _Unwind_Exception *exception_object) {
-  _LIBUNWIND_TRACE_UNWINDING("unwind_phase2(ex_ojb=%p)", exception_object);
+  _LIBUNWIND_TRACE_UNWINDING("unwind_phase2(ex_ojb=%p)",
+ (void *)exception_object);
 
   // walk each frame until we reach where search phase said to stop
   _Unwind_FunctionContext_t c = __Unwind_SjLj_GetTopOfFunctionStack();
   while (true) {
 _LIBUNWIND_TRACE_UNWINDING("unwind_phase2s(ex_ojb=%p): context=%p",
-  exception_object, c);
+   (void *)exception_object, (void *)c);
 
 // check for no more frames
 if (c == NULL) {
   _LIBUNWIND_TRACE_UNWINDING("unwind_phase2(ex_ojb=%p): unw_step() reached "
-"bottom => _URC_END_OF_STACK",
- exception_object);
+ "bottom => _URC_END_OF_STACK",
+ (void *)exception_object);
   return _URC_END_OF_STACK;
 }
 
@@ -193,7 +199,7 @@
 // continue unwinding
 _LIBUNWIND_TRACE_UNWINDING(
 "unwind_phase2(ex_ojb=%p): _URC_CONTINUE_UNWIND",
-exception_object);
+(void *)exception_object);
 if ((uintptr_t) c == exception_object->private_2) {
   // phase 1 said we would stop at this frame, but we did not...
   _LIBUNWIND_ABORT("during 

[PATCH] D56984: [libunwind] Silence warnings about unused parameters

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D56984#1366297 , @ldionne wrote:

> LGTM, but would it make sense to have a macro like 
> `_LIBUNWIND_MAYBE_UNUSED(var)` instead?


I guess some macro like that could be ok as well, although I think the 
`(void)var` is a rather well-known pattern in itself.


Repository:
  rUNW libunwind

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56984/new/

https://reviews.llvm.org/D56984



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

[libunwind] r351878 - Remove an unused variable

[Martin Storsjo via cfe-commits]

Author: mstorsjo
Date: Tue Jan 22 12:50:45 2019
New Revision: 351878

URL: http://llvm.org/viewvc/llvm-project?rev=351878=rev
Log:
Remove an unused variable

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

Modified:
libunwind/trunk/src/Unwind-seh.cpp

Modified: libunwind/trunk/src/Unwind-seh.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/Unwind-seh.cpp?rev=351878=351877=351878=diff
==
--- libunwind/trunk/src/Unwind-seh.cpp (original)
+++ libunwind/trunk/src/Unwind-seh.cpp Tue Jan 22 12:50:45 2019
@@ -68,7 +68,6 @@ static void _unw_seh_set_disp_ctx(unw_cu
 _LIBUNWIND_EXPORT EXCEPTION_DISPOSITION
 _GCC_specific_handler(PEXCEPTION_RECORD ms_exc, PVOID frame, PCONTEXT ms_ctx,
   DISPATCHER_CONTEXT *disp, __personality_routine pers) {
-  unw_context_t uc;
   unw_cursor_t cursor;
   _Unwind_Exception *exc;
   _Unwind_Action action;


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

[libunwind] r351877 - Add casts to avoid warnings about implicit conversions losing precision

[Martin Storsjo via cfe-commits]

Author: mstorsjo
Date: Tue Jan 22 12:50:42 2019
New Revision: 351877

URL: http://llvm.org/viewvc/llvm-project?rev=351877=rev
Log:
Add casts to avoid warnings about implicit conversions losing precision

This fixes warnings like these:

DwarfInstructions.hpp:85:25: warning: implicit conversion
  loses integer precision: 'uint64_t' (aka 'unsigned long long') to
  'libunwind::DwarfInstructions::pint_t' (aka 'unsigned int')
  [-Wshorten-64-to-32]

DwarfInstructions.hpp:88:25: warning: implicit conversion
  loses integer precision: 'uint64_t' (aka 'unsigned long long') to
  'libunwind::DwarfInstructions::pint_t' (aka 'unsigned int')
  [-Wshorten-64-to-32]

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

Modified:
libunwind/trunk/src/DwarfInstructions.hpp

Modified: libunwind/trunk/src/DwarfInstructions.hpp
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/DwarfInstructions.hpp?rev=351877=351876=351877=diff
==
--- libunwind/trunk/src/DwarfInstructions.hpp (original)
+++ libunwind/trunk/src/DwarfInstructions.hpp Tue Jan 22 12:50:42 2019
@@ -81,12 +81,11 @@ typename A::pint_t DwarfInstructions::kRegisterInCFA:
-return addressSpace.getRegister(cfa + (pint_t)savedReg.value);
+return (pint_t)addressSpace.getRegister(cfa + (pint_t)savedReg.value);
 
   case CFI_Parser::kRegisterAtExpression:
-return addressSpace.getRegister(
-evaluateExpression((pint_t)savedReg.value, addressSpace,
-registers, cfa));
+return (pint_t)addressSpace.getRegister(evaluateExpression(
+(pint_t)savedReg.value, addressSpace, registers, cfa));
 
   case CFI_Parser::kRegisterIsExpression:
 return evaluateExpression((pint_t)savedReg.value, addressSpace,


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

[libunwind] r351876 - Fix warnings about printf format strings

[Martin Storsjo via cfe-commits]

Author: mstorsjo
Date: Tue Jan 22 12:50:39 2019
New Revision: 351876

URL: http://llvm.org/viewvc/llvm-project?rev=351876=rev
Log:
Fix warnings about printf format strings

Either adjust the format string to use a more exact type, or add casts
(for cases when printing pointers to structs/objects with a %p
format specifier).

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

Modified:
libunwind/trunk/src/Unwind-seh.cpp
libunwind/trunk/src/Unwind-sjlj.c

Modified: libunwind/trunk/src/Unwind-seh.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/Unwind-seh.cpp?rev=351876=351875=351876=diff
==
--- libunwind/trunk/src/Unwind-seh.cpp (original)
+++ libunwind/trunk/src/Unwind-seh.cpp Tue Jan 22 12:50:39 2019
@@ -77,7 +77,9 @@ _GCC_specific_handler(PEXCEPTION_RECORD
   uintptr_t retval, target;
   bool ours = false;
 
-  _LIBUNWIND_TRACE_UNWINDING("_GCC_specific_handler(%#010x(%x), %p)", 
ms_exc->ExceptionCode, ms_exc->ExceptionFlags, frame);
+  _LIBUNWIND_TRACE_UNWINDING("_GCC_specific_handler(%#010lx(%lx), %p)",
+ ms_exc->ExceptionCode, ms_exc->ExceptionFlags,
+ (void *)frame);
   if (ms_exc->ExceptionCode == STATUS_GCC_UNWIND) {
 if (IS_TARGET_UNWIND(ms_exc->ExceptionFlags)) {
   // Set up the upper return value (the lower one and the target PC
@@ -129,7 +131,10 @@ _GCC_specific_handler(PEXCEPTION_RECORD
 }
   }
 
-  _LIBUNWIND_TRACE_UNWINDING("_GCC_specific_handler() calling personality 
function %p(1, %d, %llx, %p, %p)", pers, action, exc->exception_class, exc, 
ctx);
+  _LIBUNWIND_TRACE_UNWINDING("_GCC_specific_handler() calling personality "
+ "function %p(1, %d, %llx, %p, %p)",
+ (void *)pers, action, exc->exception_class,
+ (void *)exc, (void *)ctx);
   urc = pers(1, action, exc->exception_class, exc, ctx);
   _LIBUNWIND_TRACE_UNWINDING("_GCC_specific_handler() personality returned 
%d", urc);
   switch (urc) {

Modified: libunwind/trunk/src/Unwind-sjlj.c
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/Unwind-sjlj.c?rev=351876=351875=351876=diff
==
--- libunwind/trunk/src/Unwind-sjlj.c (original)
+++ libunwind/trunk/src/Unwind-sjlj.c Tue Jan 22 12:50:39 2019
@@ -11,6 +11,7 @@
 
 #include 
 
+#include 
 #include 
 #include 
 #include 
@@ -107,7 +108,8 @@ _Unwind_SjLj_Unregister(struct _Unwind_F
 static _Unwind_Reason_Code
 unwind_phase1(struct _Unwind_Exception *exception_object) {
   _Unwind_FunctionContext_t c = __Unwind_SjLj_GetTopOfFunctionStack();
-  _LIBUNWIND_TRACE_UNWINDING("unwind_phase1: initial function-context=%p", c);
+  _LIBUNWIND_TRACE_UNWINDING("unwind_phase1: initial function-context=%p",
+ (void *)c);
 
   // walk each frame looking for a place to stop
   for (bool handlerNotFound = true; handlerNotFound; c = c->prev) {
@@ -116,17 +118,18 @@ unwind_phase1(struct _Unwind_Exception *
 if (c == NULL) {
   _LIBUNWIND_TRACE_UNWINDING("unwind_phase1(ex_ojb=%p): reached "
  "bottom => _URC_END_OF_STACK",
-  exception_object);
+ (void *)exception_object);
   return _URC_END_OF_STACK;
 }
 
-_LIBUNWIND_TRACE_UNWINDING("unwind_phase1: function-context=%p", c);
+_LIBUNWIND_TRACE_UNWINDING("unwind_phase1: function-context=%p", (void 
*)c);
 // if there is a personality routine, ask it if it will want to stop at 
this
 // frame
 if (c->personality != NULL) {
   _LIBUNWIND_TRACE_UNWINDING("unwind_phase1(ex_ojb=%p): calling "
-"personality function %p",
- exception_object, c->personality);
+ "personality function %p",
+ (void *)exception_object,
+ (void *)c->personality);
   _Unwind_Reason_Code personalityResult = (*c->personality)(
   1, _UA_SEARCH_PHASE, exception_object->exception_class,
   exception_object, (struct _Unwind_Context *)c);
@@ -137,12 +140,14 @@ unwind_phase1(struct _Unwind_Exception *
 handlerNotFound = false;
 exception_object->private_2 = (uintptr_t) c;
 _LIBUNWIND_TRACE_UNWINDING("unwind_phase1(ex_ojb=%p): "
-   "_URC_HANDLER_FOUND", exception_object);
+   "_URC_HANDLER_FOUND",
+   (void *)exception_object);
 return _URC_NO_REASON;
 
   case _URC_CONTINUE_UNWIND:
 _LIBUNWIND_TRACE_UNWINDING("unwind_phase1(ex_ojb=%p): "
-   "_URC_CONTINUE_UNWIND", exception_object);
+   "_URC_CONTINUE_UNWIND",
+

[libunwind] r351875 - Enable LLVM_ENABLE_WARNINGS when building standalone out of tree

[Martin Storsjo via cfe-commits]

Author: mstorsjo
Date: Tue Jan 22 12:50:33 2019
New Revision: 351875

URL: http://llvm.org/viewvc/llvm-project?rev=351875=rev
Log:
Enable LLVM_ENABLE_WARNINGS when building standalone out of tree

When built within the llvm runtimes directory, the runtimes
CMakeLists.txt adds the same.

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

Modified:
libunwind/trunk/CMakeLists.txt

Modified: libunwind/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/CMakeLists.txt?rev=351875=351874=351875=diff
==
--- libunwind/trunk/CMakeLists.txt (original)
+++ libunwind/trunk/CMakeLists.txt Tue Jan 22 12:50:33 2019
@@ -73,6 +73,8 @@ if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURR
   endif()
 
   if (EXISTS ${LLVM_CMAKE_PATH})
+# Enable warnings, otherwise -w gets added to the cflags by 
HandleLLVMOptions.
+set(LLVM_ENABLE_WARNINGS ON)
 list(APPEND CMAKE_MODULE_PATH "${LLVM_CMAKE_PATH}")
 include("${LLVM_CMAKE_PATH}/AddLLVM.cmake")
 include("${LLVM_CMAKE_PATH}/HandleLLVMOptions.cmake")


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

[PATCH] D57064: [Sema] Improve a -Warray-bounds diagnostic

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington created this revision.
erik.pilkington added reviewers: aaron.ballman, rsmith.
Herald added subscribers: dexonsmith, jkorous.

Fix a bug where we would compare array sizes with incompatible element types, 
and look through explicit casts.

rdar://44800168

Thanks for taking a look!


Repository:
  rC Clang

https://reviews.llvm.org/D57064

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/static-array.c

Index: clang/test/Sema/static-array.c
===
--- clang/test/Sema/static-array.c
+++ clang/test/Sema/static-array.c
@@ -1,8 +1,8 @@
-// RUN: %clang_cc1 -fsyntax-only -fblocks -verify %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 -fsyntax-only -fblocks -verify %s
 
 void cat0(int a[static 0]) {} // expected-warning {{'static' has no effect on zero-length arrays}}
 
-void cat(int a[static 3]) {} // expected-note 2 {{callee declares array parameter as static here}}
+void cat(int a[static 3]) {} // expected-note 4 {{callee declares array parameter as static here}} expected-note 2 {{passing argument to parameter 'a' here}}
 
 void vat(int i, int a[static i]) {} // expected-note {{callee declares array parameter as static here}}
 
@@ -19,6 +19,14 @@
 
   vat(1, 0); // expected-warning {{null passed to a callee that requires a non-null argument}}
   vat(3, b);
+
+  char d[4];
+  cat((int *)d); // expected-warning {{array argument is too small; is of size 4, callee requires at least 12}}
+  cat(d); // expected-warning {{array argument is too small; is of size 4, callee requires at least 12}} expected-warning{{incompatible pointer types}}
+
+  char e[12];
+  cat((int *)e);
+  cat(e); // expected-warning{{incompatible pointer types}}
 }
 
 
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -5228,15 +5228,29 @@
 return;
 
   const ConstantArrayType *ArgCAT =
-Context.getAsConstantArrayType(ArgExpr->IgnoreParenImpCasts()->getType());
+Context.getAsConstantArrayType(ArgExpr->IgnoreParenCasts()->getType());
   if (!ArgCAT)
 return;
 
-  if (ArgCAT->getSize().ult(CAT->getSize())) {
+  if (getASTContext().hasSameUnqualifiedType(CAT->getElementType(),
+ ArgCAT->getElementType())) {
+if (ArgCAT->getSize().ult(CAT->getSize())) {
+  Diag(CallLoc, diag::warn_static_array_too_small)
+  << ArgExpr->getSourceRange()
+  << (unsigned)ArgCAT->getSize().getZExtValue()
+  << (unsigned)CAT->getSize().getZExtValue() << 0;
+  DiagnoseCalleeStaticArrayParam(*this, Param);
+}
+return;
+  }
+
+  Optional ArgSize =
+  getASTContext().getTypeSizeInCharsIfKnown(ArgCAT);
+  Optional ParmSize = getASTContext().getTypeSizeInCharsIfKnown(CAT);
+  if (ArgSize && ParmSize && *ArgSize < *ParmSize) {
 Diag(CallLoc, diag::warn_static_array_too_small)
-  << ArgExpr->getSourceRange()
-  << (unsigned) ArgCAT->getSize().getZExtValue()
-  << (unsigned) CAT->getSize().getZExtValue();
+<< ArgExpr->getSourceRange() << (unsigned)ArgSize->getQuantity()
+<< (unsigned)ParmSize->getQuantity() << 1;
 DiagnoseCalleeStaticArrayParam(*this, Param);
   }
 }
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7846,7 +7846,8 @@
   "cannot mix positional and non-positional arguments in format string">,
   InGroup;
 def warn_static_array_too_small : Warning<
-  "array argument is too small; contains %0 elements, callee requires at least %1">,
+  "array argument is too small; %select{contains %0 elements|is of size %0}2,"
+  " callee requires at least %1">,
   InGroup;
 def note_callee_static_array : Note<
   "callee declares array parameter as static here">;
Index: clang/include/clang/AST/ASTContext.h
===
--- clang/include/clang/AST/ASTContext.h
+++ clang/include/clang/AST/ASTContext.h
@@ -2088,6 +2088,16 @@
   CharUnits getTypeSizeInChars(QualType T) const;
   CharUnits getTypeSizeInChars(const Type *T) const;
 
+  Optional getTypeSizeInCharsIfKnown(QualType Ty) const {
+if (Ty->isIncompleteType() || Ty->isDependentType())
+  return None;
+return getTypeSizeInChars(Ty);
+  }
+
+  Optional getTypeSizeInCharsIfKnown(const Type *Ty) const {
+return getTypeSizeInCharsIfKnown(QualType(Ty, 0));
+  }
+
   /// Return the ABI-specified alignment of a (complete) type \p T, in
   /// bits.
   unsigned getTypeAlign(QualType T) const { return getTypeInfo(T).Align; }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D57004: [docs] Add release notes for notable things I've contributed since last release

[Martin Storsjö via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL351872: [docs] Add release notes for notable things 
Ive contributed since last release (authored by mstorsjo, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57004?vs=182754=182961#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57004/new/

https://reviews.llvm.org/D57004

Files:
  cfe/branches/release_80/docs/ReleaseNotes.rst


Index: cfe/branches/release_80/docs/ReleaseNotes.rst
===
--- cfe/branches/release_80/docs/ReleaseNotes.rst
+++ cfe/branches/release_80/docs/ReleaseNotes.rst
@@ -136,6 +136,9 @@
   instrumenting for gcov-based profiling.
   See the :doc:`UsersManual` for details.
 
+- When using a custom stack alignment, the ``stackrealign`` attribute is now
+  implicitly set on the main function.
+
 - ...
 
 Deprecated Compiler Flags
@@ -179,6 +182,15 @@
   `dllexport` and `dllimport` attributes not apply to inline member functions.
   This can significantly reduce compile and link times. See the `User's Manual
   `_ for more info.
+
+- For MinGW, ``-municode`` now correctly defines ``UNICODE`` during
+  preprocessing.
+
+- For MinGW, clang now produces vtables and RTTI for dllexported classes
+  without key functions. This fixes building Qt in debug mode.
+
+- Allow using Address Sanitizer and Undefined Behaviour Sanitizer on MinGW.
+
 - ...
 
 


Index: cfe/branches/release_80/docs/ReleaseNotes.rst
===
--- cfe/branches/release_80/docs/ReleaseNotes.rst
+++ cfe/branches/release_80/docs/ReleaseNotes.rst
@@ -136,6 +136,9 @@
   instrumenting for gcov-based profiling.
   See the :doc:`UsersManual` for details.
 
+- When using a custom stack alignment, the ``stackrealign`` attribute is now
+  implicitly set on the main function.
+
 - ...
 
 Deprecated Compiler Flags
@@ -179,6 +182,15 @@
   `dllexport` and `dllimport` attributes not apply to inline member functions.
   This can significantly reduce compile and link times. See the `User's Manual
   `_ for more info.
+
+- For MinGW, ``-municode`` now correctly defines ``UNICODE`` during
+  preprocessing.
+
+- For MinGW, clang now produces vtables and RTTI for dllexported classes
+  without key functions. This fixes building Qt in debug mode.
+
+- Allow using Address Sanitizer and Undefined Behaviour Sanitizer on MinGW.
+
 - ...
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55400: [analyzer] Move out tracking retain count for OSObjects into a separate checker

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

Poor wording on my end, sorry about that. Let me clarify.

In D55400#1366684 , @george.karpenkov 
wrote:

> > Deal with the consequences of this, and just correct all plist files to now 
> > refer to the new base checker.
>
> What does it mean?


When a report is emitted in the plist output, it'll also that which checker is 
"responsible" for that warning. I vaguely remember reading that this isn't used 
that much in your application, but we use this information extensively here.

Now, what I'd propose, is register a new, `osx.RetainCountBase` checker, and 
make both of the already existing checkers depend on that. Long story short, 
this will make the name of the checker that is listed in that plist entry 
`osx.RetainCountBase`, as opposed to `osx.cocoa.RetainCount`, but this doesn't 
sound that illogical, considering that both of those checkers emit the same 
checker name anyways (meaning, that if one report originates from 
`osx.OSObjectRetainCount` and one from `osx.cocoa.RetainCount`, both reports 
will be listed as they originated from `osx.cocoa.RetainCount`).

> 
> 
>> Each time a report is emitted from these checkers, create a ProgramPointTag, 
>> and "manually" make sure the emitted checker name doesn't change.
> 
> I'm not sure what do you propose exactly, but sounds pretty bad.

It does. I think `IvarInvalidation` uses something like that, and it's both 
unsustainable and very ugly.

>> Rename osx.cocoa.RetainCount to something else. This one is clearly 
>> nonsensical, but let's put it here for the sake of completeness.
> 
> I don't think we can rename the old checker, as older Xcode versions have 
> "osx.cocoa.RetainCount" hardcoded in them.

Yea, this one makes little sense. Should've just left this one out really.

> TBH I'm not really sold on the way we "bundle" multiple checkers (from the 
> tablegen) into a single class. [...] essentially the checker name is defined 
> by the class which was registered by the tablegen first (which should not 
> even be visible, and should be an internal implementation detail)
>  Do you have better proposals on how, conceptually, grouped checkers should 
> be organized?

Yup, I've had the same thought before. I would argue strongly for registering 
them in the tblgen file, because a clear dependency graph can be easily 
established this way. What would be neat, however, is to also a new bit field 
to `Checker` in `CheckerBase.td`, which if set, will make the checker hidden in 
the `-analyzer-checker-help` list. There are many checkers that should be there 
anyways, implicit checker come to my mind first.

> For one, that means options don't work at all, and [...]

Hmmm, does this mess with options that bad? Could you please clarify?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55400/new/

https://reviews.llvm.org/D55400



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

[clang-tools-extra] r351867 - [doc] Replace 'class' with 'struct' for 'public' by default

[Zinovy Nis via cfe-commits]

Author: zinovy.nis
Date: Tue Jan 22 12:27:02 2019
New Revision: 351867

URL: http://llvm.org/viewvc/llvm-project?rev=351867=rev
Log:
[doc] Replace 'class' with 'struct' for 'public' by default

Make sample syntax correct.


Modified:

clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-parent-virtual-call.rst

Modified: 
clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-parent-virtual-call.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-parent-virtual-call.rst?rev=351867=351866=351867=diff
==
--- 
clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-parent-virtual-call.rst 
(original)
+++ 
clang-tools-extra/trunk/docs/clang-tidy/checks/bugprone-parent-virtual-call.rst 
Tue Jan 22 12:27:02 2019
@@ -8,15 +8,15 @@ to overridden parent's virtual methods.
 
 .. code-block:: c++
 
-  class A {
+  struct A {
 int virtual foo() {...}
   };
 
-  class B: public A {
+  struct B: public A {
 int foo() override {...}
   };
 
-  class C: public B {
+  struct C: public B {
 int foo() override { A::foo(); }
   // 
   // warning: qualified name A::foo refers to a member overridden in subclass; 
did you mean 'B'?  [bugprone-parent-virtual-call]


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

[PATCH] D56935: [NewPM] Add support for new-PM plugins to clang

[Philip Pfaffe via Phabricator via cfe-commits]

philip.pfaffe added a comment.

This generally looks sane. What will happen on windows though? Will it silently 
fail?




Comment at: clang/include/clang/Basic/CodeGenOptions.h:292
+  /// List of dynamic shared object files to be loaded as pass plugins.
+  std::vector PassPlugins;
+

This  should be SmallVector.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56935/new/

https://reviews.llvm.org/D56935



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

[PATCH] D57062: [analyzer] Re-enable the "System is over constrained" assertion on optimized builds.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, george.karpenkov, 
rnkovacs, mikhail.ramalho, Szelethus, baloghadamsoftware.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, a.sidorin, szepet.

This assertion was only enabled in Debug+Asserts, not on Release+Asserts, 
because it's expensive. Indeed, enabling it on Release+Asserts builds seems to 
cause varied slowdown, up to roughly ~5%. However, i think it's worth it, 
because this assertion is very important because it's very fundamental: when it 
crashes, it means that we've investigated a path that was already //obviously// 
infeasible, i.e. we could have refuted the path much earlier if we simply 
looked closer at it with existing solvers.
So, because, testing on large codebases is pretty much always done in 
Release+Asserts mode, it kinda scares me that we're missing out on such an 
important test while doing our ultimate real-world testing.

The slowdown is at most a constant factor - like, it roughly doubles the cost 
of every assume() operation, so it won't cause more than 2x slowdown, and in 
practice assume() isn't showing up in our profiles too much, so the ~5% number 
is relatively reliable.
Of course, the behavior of Clang we actually release (i.e., built without 
asserts) isn't really affected.


Repository:
  rC Clang

https://reviews.llvm.org/D57062

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h


Index: include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
@@ -94,13 +94,7 @@
 // If StTrue is infeasible, asserting the falseness of Cond is unnecessary
 // because the existing constraints already establish this.
 if (!StTrue) {
-#ifndef __OPTIMIZE__
-  // This check is expensive and should be disabled even in Release+Asserts
-  // builds.
-  // FIXME: __OPTIMIZE__ is a GNU extension that Clang implements but MSVC
-  // does not. Is there a good equivalent there?
   assert(assume(State, Cond, false) && "System is over constrained.");
-#endif
   return ProgramStatePair((ProgramStateRef)nullptr, State);
 }
 


Index: include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
@@ -94,13 +94,7 @@
 // If StTrue is infeasible, asserting the falseness of Cond is unnecessary
 // because the existing constraints already establish this.
 if (!StTrue) {
-#ifndef __OPTIMIZE__
-  // This check is expensive and should be disabled even in Release+Asserts
-  // builds.
-  // FIXME: __OPTIMIZE__ is a GNU extension that Clang implements but MSVC
-  // does not. Is there a good equivalent there?
   assert(assume(State, Cond, false) && "System is over constrained.");
-#endif
   return ProgramStatePair((ProgramStateRef)nullptr, State);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r351865 - [analyzer] Insert notes in RetainCountChecker where our dynamic cast modeling assumes 'null' output

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Tue Jan 22 11:51:00 2019
New Revision: 351865

URL: http://llvm.org/viewvc/llvm-project?rev=351865=rev
Log:
[analyzer] Insert notes in RetainCountChecker where our dynamic cast modeling 
assumes 'null' output

rdar://47397214

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

Modified:

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h
cfe/trunk/test/Analysis/osobject-retain-release.cpp

Modified: 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp?rev=351865=351864=351865=diff
==
--- 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp 
(original)
+++ 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp 
Tue Jan 22 11:51:00 2019
@@ -575,7 +575,6 @@ void RetainCountChecker::checkSummary(co
 
   // Helper tag for providing diagnostics: indicate whether dealloc was sent
   // at this location.
-  static CheckerProgramPointTag DeallocSentTag(this, DeallocTagDescription);
   bool DeallocSent = false;
 
   for (unsigned idx = 0, e = CallOrMsg.getNumArgs(); idx != e; ++idx) {
@@ -903,8 +902,7 @@ bool RetainCountChecker::evalCall(const
   // Assume that output is zero on the other branch.
   NullOutputState = NullOutputState->BindExpr(
   CE, LCtx, C.getSValBuilder().makeNull(), /*Invalidate=*/false);
-
-  C.addTransition(NullOutputState);
+  C.addTransition(NullOutputState, );
 
   // And on the original branch assume that both input and
   // output are non-zero.

Modified: 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h?rev=351865=351864=351865=diff
==
--- 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h 
(original)
+++ 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h 
Tue Jan 22 11:51:00 2019
@@ -260,9 +260,11 @@ class RetainCountChecker
   RefCountBug leakWithinFunction{this, RefCountBug::LeakWithinFunction};
   RefCountBug leakAtReturn{this, RefCountBug::LeakAtReturn};
 
+  CheckerProgramPointTag DeallocSentTag{this, "DeallocSent"};
+  CheckerProgramPointTag CastFailTag{this, "DynamicCastFail"};
+
   mutable std::unique_ptr Summaries;
 public:
-  static constexpr const char *DeallocTagDescription = "DeallocSent";
 
   /// Track Objective-C and CoreFoundation objects.
   bool TrackObjCAndCFObjects = false;
@@ -361,6 +363,14 @@ public:
  CheckerContext ,
  ExplodedNode *Pred = nullptr) const;
 
+  const CheckerProgramPointTag () const {
+return DeallocSentTag;
+  }
+
+  const CheckerProgramPointTag () const {
+return CastFailTag;
+  }
+
 private:
   /// Perform the necessary checks and state adjustments at the end of the
   /// function.

Modified: 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp?rev=351865=351864=351865=diff
==
--- 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
 (original)
+++ 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.cpp
 Tue Jan 22 11:51:00 2019
@@ -66,7 +66,7 @@ StringRef RefCountBug::getDescription()
 RefCountBug::RefCountBug(const CheckerBase *Checker, RefCountBugType BT)
 : BugType(Checker, bugTypeToName(BT), categories::MemoryRefCount,
   /*SupressOnSink=*/BT == LeakWithinFunction || BT == 
LeakAtReturn),
-  BT(BT) {}
+  BT(BT), Checker(Checker) {}
 
 static bool isNumericLiteralExpression(const Expr *E) {
   // FIXME: This set of cases was copied from SemaExprObjC.
@@ -423,6 +423,8 @@ RefCountReportVisitor::VisitNode(const E
   BugReporterContext , BugReport ) {
 
   const auto  = static_cast(BR.getBugType());
+  const auto *Checker =
+  static_cast(BT.getChecker());
 
   bool IsFreeUnowned = BT.getBugType() == RefCountBug::FreeNotOwned ||
BT.getBugType() == RefCountBug::DeallocNotOwned;
@@ -509,8 +511,12 @@ RefCountReportVisitor::VisitNode(const E
   bool DeallocSent = false;
 
   const ProgramPointTag *Tag = 

r351864 - [analyzer] Model another special-case kind of cast for OSObject RetainCountChecker

[George Karpenkov via cfe-commits]

Author: george.karpenkov
Date: Tue Jan 22 11:50:47 2019
New Revision: 351864

URL: http://llvm.org/viewvc/llvm-project?rev=351864=rev
Log:
[analyzer] Model another special-case kind of cast for OSObject 
RetainCountChecker

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

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp
cfe/trunk/test/Analysis/os_object_base.h
cfe/trunk/test/Analysis/osobject-retain-release.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h?rev=351864=351863=351864=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/RetainSummaryManager.h Tue Jan 
22 11:50:47 2019
@@ -677,6 +677,9 @@ public:
 // Function returns the first argument.
 Identity,
 
+// Function returns "this" argument.
+IdentityThis,
+
 // Function either returns zero, or the input parameter.
 IdentityOrZero
   };

Modified: 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp?rev=351864=351863=351864=diff
==
--- 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp 
(original)
+++ 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp 
Tue Jan 22 11:50:47 2019
@@ -849,7 +849,6 @@ void RetainCountChecker::processNonLeakE
 
//===--===//
 
 bool RetainCountChecker::evalCall(const CallExpr *CE, CheckerContext ) const 
{
-  // Get the callee. We're only interested in simple C functions.
   ProgramStateRef state = C.getState();
   const FunctionDecl *FD = C.getCalleeDecl(CE);
   if (!FD)
@@ -874,18 +873,27 @@ bool RetainCountChecker::evalCall(const
 
   // Bind the return value.
   if (BSmr == BehaviorSummary::Identity ||
-  BSmr == BehaviorSummary::IdentityOrZero) {
-SVal RetVal = state->getSVal(CE->getArg(0), LCtx);
+  BSmr == BehaviorSummary::IdentityOrZero ||
+  BSmr == BehaviorSummary::IdentityThis) {
+
+const Expr *BindReturnTo =
+(BSmr == BehaviorSummary::IdentityThis)
+? cast(CE)->getImplicitObjectArgument()
+: CE->getArg(0);
+SVal RetVal = state->getSVal(BindReturnTo, LCtx);
 
 // If the receiver is unknown or the function has
 // 'rc_ownership_trusted_implementation' annotate attribute, conjure a
 // return value.
+// FIXME: this branch is very strange.
 if (RetVal.isUnknown() ||
 (hasTrustedImplementationAnnotation && !ResultTy.isNull())) {
   SValBuilder  = C.getSValBuilder();
   RetVal =
   SVB.conjureSymbolVal(nullptr, CE, LCtx, ResultTy, C.blockCount());
 }
+
+// Bind the value.
 state = state->BindExpr(CE, LCtx, RetVal, /*Invalidate=*/false);
 
 if (BSmr == BehaviorSummary::IdentityOrZero) {

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp?rev=351864=351863=351864=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RetainSummaryManager.cpp Tue Jan 22 
11:50:47 2019
@@ -152,6 +152,10 @@ static bool isOSObjectDynamicCast(String
   return S == "safeMetaCast";
 }
 
+static bool isOSObjectThisCast(StringRef S) {
+  return S == "metaCast";
+}
+
 static bool isOSIteratorSubclass(const Decl *D) {
   return isSubclass(D, "OSIterator");
 }
@@ -219,13 +223,13 @@ RetainSummaryManager::getSummaryForOSObj
 const CXXRecordDecl *PD = RetTy->getPointeeType()->getAsCXXRecordDecl();
 if (PD && isOSObjectSubclass(PD)) {
   if (const IdentifierInfo *II = FD->getIdentifier()) {
-if (isOSObjectDynamicCast(II->getName()))
+StringRef FuncName = II->getName();
+if (isOSObjectDynamicCast(FuncName) || isOSObjectThisCast(FuncName))
   return getDefaultSummary();
 
 // All objects returned with functions *not* starting with
 // get, or iterators, are returned at +1.
-if ((!II->getName().startswith("get") &&
- !II->getName().startswith("Get")) ||
+if ((!FuncName.startswith("get") && !FuncName.startswith("Get")) ||
 isOSIteratorSubclass(PD)) {
   return getOSSummaryCreateRule(FD);
 } else {
@@ -703,8 

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I'll just add that we've been trying to not put things behind OpenCL guards as 
much as possible.  Most of the remaining OpenCL checks are for OpenCL-specific 
logic like inferring the default address space, and yeah, we could probably 
make that a target option or something.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55850/new/

https://reviews.llvm.org/D55850



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

[PATCH] D55676: [Modules] Fix decl order for DeclsInPrototype

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

rsmith added inline comments.



Comment at: lib/Parse/ParseDecl.cpp:6237
+llvm::sort(SortedDecls, [](const Decl *L, const Decl *R) {
+  return L->getID() < R->getID();
+});

I would prefer that we use `getID` only for debug dumping purposes. Can we 
change `Scope::DeclsInContext` to be a `SetVector` instead? (How much does that 
cost us memory-wise?)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55676/new/

https://reviews.llvm.org/D55676



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

[PATCH] D55400: [analyzer] Move out tracking retain count for OSObjects into a separate checker

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added a comment.

> Deal with the consequences of this, and just correct all plist files to now 
> refer to the new base checker.

What does it mean?

> Each time a report is emitted from these checkers, create a ProgramPointTag, 
> and "manually" make sure the emitted checker name doesn't change.

I'm not sure what do you propose exactly, but sounds pretty bad.

> Rename osx.cocoa.RetainCount to something else. This one is clearly 
> nonsensical, but let's put it here for the sake of completeness.

I don't think we can rename the old checker, as older Xcode versions have 
"osx.cocoa.RetainCount" hardcoded in them.

TBH I'm not really sold on the way we "bundle" multiple checkers (from the 
tablegen) into a single class.
For one, that means options don't work at all, and essentially the checker name 
is defined by the class which was registered by the tablegen first (which 
should not even be visible, and should be an internal implementation detail).
Do you have better proposals on how, conceptually, grouped checkers should be 
organized?

Essentially, we have a single checker class with two checks, which should be 
toggle-able orthogonally from each other.
For legacy reasons, we can't quite get rid of `osx.cocoa.RetainCount` (but even 
if we could, what then?)


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55400/new/

https://reviews.llvm.org/D55400



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

[PATCH] D56926: [Documentation] Use HTTPS whenever possible in clang-tools-extra

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE351862: [Documentation] Use HTTPS whenever possible. 
(authored by eugenezelenko, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D56926?vs=182590=182947#toc

Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56926/new/

https://reviews.llvm.org/D56926

Files:
  docs/clang-doc.rst
  docs/clang-rename.rst
  docs/clang-tidy.rst
  docs/clang-tidy/Integrations.rst
  docs/clang-tidy/checks/google-objc-avoid-throwing-exception.rst
  docs/clang-tidy/checks/google-objc-global-variable-declaration.rst
  docs/clang-tidy/checks/llvm-include-order.rst
  docs/clang-tidy/checks/llvm-namespace-comment.rst
  docs/clang-tidy/checks/modernize-pass-by-value.rst
  docs/clang-tidy/checks/modernize-use-emplace.rst
  docs/clang-tidy/checks/portability-simd-intrinsics.rst
  docs/clang-tidy/checks/readability-else-after-return.rst
  docs/clang-tidy/checks/readability-magic-numbers.rst
  docs/clang-tidy/index.rst
  docs/clangd.rst
  docs/include-fixer.rst
  docs/modularize.rst
  docs/pp-trace.rst

Index: docs/include-fixer.rst
===
--- docs/include-fixer.rst
+++ docs/include-fixer.rst
@@ -32,7 +32,7 @@
 ``compile_commands.json`` as generated by CMake does not include header files,
 so only implementation files can be handled by tools.
 
-.. _How To Setup Tooling For LLVM: http://clang.llvm.org/docs/HowToSetupToolingForLLVM.html
+.. _How To Setup Tooling For LLVM: https://clang.llvm.org/docs/HowToSetupToolingForLLVM.html
 
 Creating a Symbol Index From a Compilation Database
 ---
Index: docs/clang-tidy.rst
===
--- docs/clang-tidy.rst
+++ docs/clang-tidy.rst
@@ -3,4 +3,4 @@
 .. meta::
:http-equiv=refresh: 0;URL='clang-tidy/'
 
-clang-tidy documentation has moved here: http://clang.llvm.org/extra/clang-tidy/
+clang-tidy documentation has moved here: https://clang.llvm.org/extra/clang-tidy/
Index: docs/clang-rename.rst
===
--- docs/clang-rename.rst
+++ docs/clang-rename.rst
@@ -24,10 +24,10 @@
 ==
 
 :program:`clang-rename` is a `LibTooling
-`_-based tool, and it's easier to
+`_-based tool, and it's easier to
 work with if you set up a compile command database for your project (for an
 example of how to do this see `How To Setup Tooling For LLVM
-`_). You can also
+`_). You can also
 specify compilation options on the command line after `--`:
 
 .. code-block:: console
@@ -140,7 +140,7 @@
 You can call :program:`clang-rename` directly from Vim! To set up
 :program:`clang-rename` integration for Vim see
 `clang-rename/tool/clang-rename.py
-`_.
+`_.
 
 Please note that **you have to save all buffers, in which the replacement will
 happen before running the tool**.
@@ -157,7 +157,7 @@
 You can also use :program:`clang-rename` while using Emacs! To set up
 :program:`clang-rename` integration for Emacs see
 `clang-rename/tool/clang-rename.el
-`_.
+`_.
 
 Once installed, you can point your cursor to symbols you want to rename, press
 `M-X`, type `clang-rename` and new desired name.
Index: docs/clangd.rst
===
--- docs/clangd.rst
+++ docs/clangd.rst
@@ -31,7 +31,7 @@
 ==
 
 Packages are available for debian-based distributions, see the `LLVM packages
-page `_. :program:`Clangd` is included in the
+page `_. :program:`Clangd` is included in the
 `clang-tools` package.
 However, it is a good idea to check your distribution's packaging system first
 as it might already be available.
@@ -147,14 +147,14 @@
 ==
 
 A good place for interested contributors is the `Clangd developer mailing list
-`_. For discussions with the
+`_. For discussions with the
 broader community on topics not only related to Clangd, use
 `Clang developer mailing list
-`_.
+`_.
 If you're also interested in contributing patches 

[clang-tools-extra] r351862 - [Documentation] Use HTTPS whenever possible.

[Eugene Zelenko via cfe-commits]

Author: eugenezelenko
Date: Tue Jan 22 11:19:48 2019
New Revision: 351862

URL: http://llvm.org/viewvc/llvm-project?rev=351862=rev
Log:
[Documentation] Use HTTPS whenever possible.

Differential revision: https://reviews.llvm.org/D56926

Modified:
clang-tools-extra/trunk/docs/clang-doc.rst
clang-tools-extra/trunk/docs/clang-rename.rst
clang-tools-extra/trunk/docs/clang-tidy.rst
clang-tools-extra/trunk/docs/clang-tidy/Integrations.rst

clang-tools-extra/trunk/docs/clang-tidy/checks/google-objc-avoid-throwing-exception.rst

clang-tools-extra/trunk/docs/clang-tidy/checks/google-objc-global-variable-declaration.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/llvm-include-order.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/llvm-namespace-comment.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-pass-by-value.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-emplace.rst

clang-tools-extra/trunk/docs/clang-tidy/checks/portability-simd-intrinsics.rst

clang-tools-extra/trunk/docs/clang-tidy/checks/readability-else-after-return.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-magic-numbers.rst
clang-tools-extra/trunk/docs/clang-tidy/index.rst
clang-tools-extra/trunk/docs/clangd.rst
clang-tools-extra/trunk/docs/include-fixer.rst
clang-tools-extra/trunk/docs/modularize.rst
clang-tools-extra/trunk/docs/pp-trace.rst

Modified: clang-tools-extra/trunk/docs/clang-doc.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-doc.rst?rev=351862=351861=351862=diff
==
--- clang-tools-extra/trunk/docs/clang-doc.rst (original)
+++ clang-tools-extra/trunk/docs/clang-doc.rst Tue Jan 22 11:19:48 2019
@@ -20,10 +20,10 @@ Use
 =
 
 :program:`clang-doc` is a `LibTooling
-`_-based tool, and so requires a
+`_-based tool, and so requires a
 compile command database for your project (for an example of how to do this 
 see `How To Setup Tooling For LLVM
-`_).
+`_).
 
 The tool can be used on a single file or multiple files as defined in 
 the compile commands database:

Modified: clang-tools-extra/trunk/docs/clang-rename.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-rename.rst?rev=351862=351861=351862=diff
==
--- clang-tools-extra/trunk/docs/clang-rename.rst (original)
+++ clang-tools-extra/trunk/docs/clang-rename.rst Tue Jan 22 11:19:48 2019
@@ -24,10 +24,10 @@ Using Clang-Rename
 ==
 
 :program:`clang-rename` is a `LibTooling
-`_-based tool, and it's easier to
+`_-based tool, and it's easier to
 work with if you set up a compile command database for your project (for an
 example of how to do this see `How To Setup Tooling For LLVM
-`_). You can also
+`_). You can also
 specify compilation options on the command line after `--`:
 
 .. code-block:: console
@@ -140,7 +140,7 @@ Vim Integration
 You can call :program:`clang-rename` directly from Vim! To set up
 :program:`clang-rename` integration for Vim see
 `clang-rename/tool/clang-rename.py
-`_.
+`_.
 
 Please note that **you have to save all buffers, in which the replacement will
 happen before running the tool**.
@@ -157,7 +157,7 @@ Emacs Integration
 You can also use :program:`clang-rename` while using Emacs! To set up
 :program:`clang-rename` integration for Emacs see
 `clang-rename/tool/clang-rename.el
-`_.
+`_.
 
 Once installed, you can point your cursor to symbols you want to rename, press
 `M-X`, type `clang-rename` and new desired name.

Modified: clang-tools-extra/trunk/docs/clang-tidy.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy.rst?rev=351862=351861=351862=diff
==
--- clang-tools-extra/trunk/docs/clang-tidy.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy.rst Tue Jan 22 11:19:48 2019
@@ -3,4 +3,4 @@
 .. meta::
:http-equiv=refresh: 0;URL='clang-tidy/'
 
-clang-tidy documentation has moved here: 

[PATCH] D56900: [Fixed Point Arithmetic] Fixed Point and Integer Conversions

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 182945.
leonardchan marked 6 inline comments as done.

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56900/new/

https://reviews.llvm.org/D56900

Files:
  clang/include/clang/AST/OperationKinds.def
  clang/include/clang/Basic/FixedPoint.h
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Basic/FixedPoint.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CGExprComplex.cpp
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Edit/RewriteObjCFoundationAPI.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/test/Frontend/fixed_point_conversions.c
  clang/test/Frontend/fixed_point_errors.c
  clang/test/Frontend/fixed_point_unknown_conversions.c

Index: clang/test/Frontend/fixed_point_unknown_conversions.c
===
--- clang/test/Frontend/fixed_point_unknown_conversions.c
+++ clang/test/Frontend/fixed_point_unknown_conversions.c
@@ -22,28 +22,19 @@
   _Fract fract = accum; // ok
   _Accum *accum_ptr;
 
-  accum = b;   // expected-error{{conversion between fixed point and '_Bool' is not yet supported}}
-  accum = i;   // expected-error{{conversion between fixed point and 'int' is not yet supported}}
-  accum = i;   // expected-error{{conversion between fixed point and 'int' is not yet supported}}
   accum = f;   // expected-error{{conversion between fixed point and 'float' is not yet supported}}
   accum = d;   // expected-error{{conversion between fixed point and 'double' is not yet supported}}
   accum = dc;  // expected-error{{conversion between fixed point and '_Complex double' is not yet supported}}
   accum = ic;  // expected-error{{conversion between fixed point and '_Complex int' is not yet supported}}
   accum = s;   // expected-error{{assigning to '_Accum' from incompatible type 'struct S'}}
-  accum = e;   // expected-error{{conversion between fixed point and 'enum E' is not yet supported}}
   accum = ptr; // expected-error{{assigning to '_Accum' from incompatible type 'int *'}}
   accum_ptr = ptr; // expected-warning{{incompatible pointer types assigning to '_Accum *' from 'int *'}}
-  accum = i2;  // expected-error{{conversion between fixed point and 'int_t' (aka 'int') is not yet supported}}
 
-  c = accum;   // expected-error{{conversion between fixed point and 'char' is not yet supported}}
-  i = accum;   // expected-error{{conversion between fixed point and 'int' is not yet supported}}
   f = accum;   // expected-error{{conversion between fixed point and 'float' is not yet supported}}
   d = accum;   // expected-error{{conversion between fixed point and 'double' is not yet supported}}
   dc = accum;  // expected-error{{conversion between fixed point and '_Complex double' is not yet supported}}
   ic = accum;  // expected-error{{conversion between fixed point and '_Complex int' is not yet supported}}
   s = accum;   // expected-error{{assigning to 'struct S' from incompatible type '_Accum'}}
-  e = accum;   // expected-error{{conversion between fixed point and 'enum E' is not yet supported}}
   ptr = accum; // expected-error{{assigning to 'int *' from incompatible type '_Accum'}}
   ptr = accum_ptr; // expected-warning{{incompatible pointer types assigning to 'int *' from '_Accum *'}}
-  i2 = accum;  // expected-error{{conversion between fixed point and 'int' is not yet supported}}
 }
Index: clang/test/Frontend/fixed_point_errors.c
===
--- clang/test/Frontend/fixed_point_errors.c
+++ clang/test/Frontend/fixed_point_errors.c
@@ -233,8 +233,20 @@
  // expected-warning@-1{{type specifier missing, defaults to 'int'}}
 }
 
+// Ok conversions
+int i_const = -2.5hk;
+_Sat short _Accum sat_sa_const2 = 256.0k;
+_Sat unsigned short _Accum sat_usa_const = -1.0hk;
+short _Accum sa_const3 = 2;
+short _Accum sa_const4 = -2;
+
 // Overflow
 short _Accum sa_const = 256.0k;   // expected-warning{{implicit conversion from 256.0 cannot fit within the range of values for 'short _Accum'}}
 short _Fract sf_const = 1.0hk;// expected-warning{{implicit conversion from 1.0 cannot fit within the range of values for 'short _Fract'}}
 unsigned _Accum ua_const = -1.0k; // expected-warning{{implicit conversion from -1.0 cannot fit within the range of values for 'unsigned _Accum'}}
 short _Accum sa_const2 = 128.0k + 128.0k; // expected-warning{{implicit conversion from 256.0 cannot fit within the range of values for 'short _Accum'}}
+short s_const = 65536.0lk;// expected-warning{{implicit conversion from 65536.0 cannot fit within the range of values for 'short'}}
+unsigned u_const = -2.5hk;// expected-warning{{implicit conversion 

Re: [clang-tools-extra] r351788 - [clangd] Filter out plugin related flags and move all commandline manipulations into OverlayCDB.

[Hans Wennborg via cfe-commits]

This has been merged to the 8.0 branch in r351860. Please  let me know
if there are any follow-ups so they can be merged too.

Thanks,
Hans

On Tue, Jan 22, 2019 at 1:10 AM Kadir Cetinkaya via cfe-commits
 wrote:
>
> Author: kadircet
> Date: Tue Jan 22 01:10:20 2019
> New Revision: 351788
>
> URL: http://llvm.org/viewvc/llvm-project?rev=351788=rev
> Log:
> [clangd] Filter out plugin related flags and move all commandline 
> manipulations into OverlayCDB.
>
> Summary:
> Some projects make use of clang plugins when building, but clangd is
> not aware of those plugins therefore can't work with the same compile command
> arguments.
>
> There were multiple places clangd performed commandline manipulations,
>  this one also moves them all into OverlayCDB.
>
> Reviewers: ilya-biryukov
>
> Subscribers: klimek, sammccall, ioeric, MaskRay, jkorous, arphaman, 
> cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D56841
>
> Modified:
> clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
> clang-tools-extra/trunk/clangd/ClangdServer.cpp
> clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
> clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.h
> clang-tools-extra/trunk/clangd/index/Background.cpp
> clang-tools-extra/trunk/clangd/index/Background.h
> clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
> clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
> 
> clang-tools-extra/trunk/unittests/clangd/GlobalCompilationDatabaseTests.cpp
>
> Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=351788=351787=351788=diff
> ==
> --- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
> +++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Tue Jan 22 01:10:20 
> 2019
> @@ -289,7 +289,8 @@ void ClangdLSPServer::onInitialize(const
>if (UseDirBasedCDB)
>  BaseCDB = llvm::make_unique(
>  CompileCommandsDir);
> -  CDB.emplace(BaseCDB.get(), Params.initializationOptions.fallbackFlags);
> +  CDB.emplace(BaseCDB.get(), Params.initializationOptions.fallbackFlags,
> +  ClangdServerOpts.ResourceDir);
>Server.emplace(*CDB, FSProvider, static_cast(*this),
>   ClangdServerOpts);
>applyConfiguration(Params.initializationOptions.ConfigSettings);
>
> Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=351788=351787=351788=diff
> ==
> --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
> +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue Jan 22 01:10:20 2019
> @@ -37,11 +37,6 @@ namespace clang {
>  namespace clangd {
>  namespace {
>
> -std::string getStandardResourceDir() {
> -  static int Dummy; // Just an address in this process.
> -  return CompilerInvocation::GetResourcesPath("clangd", (void *));
> -}
> -
>  class RefactoringResultCollector final
>  : public tooling::RefactoringResultConsumer {
>  public:
> @@ -107,8 +102,6 @@ ClangdServer::ClangdServer(const GlobalC
> DiagnosticsConsumer ,
> const Options )
>  : CDB(CDB), FSProvider(FSProvider),
> -  ResourceDir(Opts.ResourceDir ? *Opts.ResourceDir
> -   : getStandardResourceDir()),
>DynamicIdx(Opts.BuildDynamicSymbolIndex
>   ? new FileIndex(Opts.HeavyweightDynamicSymbolIndex)
>   : nullptr),
> @@ -136,7 +129,7 @@ ClangdServer::ClangdServer(const GlobalC
>  AddIndex(Opts.StaticIndex);
>if (Opts.BackgroundIndex) {
>  BackgroundIdx = llvm::make_unique(
> -Context::current().clone(), ResourceDir, FSProvider, CDB,
> +Context::current().clone(), FSProvider, CDB,
>  BackgroundIndexStorage::createDiskBackedStorageFactory(),
>  Opts.BackgroundIndexRebuildPeriodMs);
>  AddIndex(BackgroundIdx.get());
> @@ -461,10 +454,6 @@ tooling::CompileCommand ClangdServer::ge
>llvm::Optional C = CDB.getCompileCommand(File);
>if (!C) // FIXME: Suppress diagnostics? Let the user know?
>  C = CDB.getFallbackCommand(File);
> -
> -  // Inject the resource dir.
> -  // FIXME: Don't overwrite it if it's already there.
> -  C->CommandLine.push_back("-resource-dir=" + ResourceDir);
>return std::move(*C);
>  }
>
>
> Modified: clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp?rev=351788=351787=351788=diff
> ==
> --- clang-tools-extra/trunk/clangd/GlobalCompilationDatabase.cpp (original)
> +++ 

[PATCH] D55676: [Modules] Fix decl order for DeclsInPrototype

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added a comment.

Ping


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55676/new/

https://reviews.llvm.org/D55676



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

[PATCH] D56946: [Documentation] Use HTTPS whenever possible in Clang

[Hans Wennborg via Phabricator via cfe-commits]

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

Seems like a good change to me.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56946/new/

https://reviews.llvm.org/D56946



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

[PATCH] D56916: Fix crash due to ObjCPropertyDecl

[David Goldman via Phabricator via cfe-commits]

dgoldman added a comment.

In D56916#1366456 , @ilya-biryukov 
wrote:

> > Unfortunately I can't get ninja check-clangd to build:
>
> Looks like `clang-tools-extra` uses an old revision. Rebasing after rL350916 
>  should do the trick.


Thanks, looks like my update script was skipping over that repo. Test case 
implemented. As I don't have commit access, can you land this for me?


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56916/new/

https://reviews.llvm.org/D56916



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

[PATCH] D56916: Fix crash due to ObjCPropertyDecl

[David Goldman via Phabricator via cfe-commits]

dgoldman updated this revision to Diff 182942.
dgoldman added a comment.
Herald added a subscriber: jfb.

- FIXME and dyn_cast
- use auto
- Add test


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56916/new/

https://reviews.llvm.org/D56916

Files:
  clangd/index/SymbolCollector.cpp
  unittests/clangd/SymbolCollectorTests.cpp


Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -437,6 +437,23 @@
   QName("MyProtocol"), QName("MyProtocol::someMethodName3:")));
 }
 
+TEST_F(SymbolCollectorTest, ObjCPropertyImpl) {
+  const std::string Header = R"(
+@interface Container
+@property(nonatomic) int magic;
+@end
+
+@implementation Container
+@end
+  )";
+  TestFileName = testPath("test.m");
+  runSymbolCollector(Header, /*Main=*/"", {"-xobjective-c++"});
+  EXPECT_THAT(Symbols,
+  UnorderedElementsAre(
+  QName("Container"), QName("Container::magic"),
+  QName("Container::_magic")));
+}
+
 TEST_F(SymbolCollectorTest, Locations) {
   Annotations Header(R"cpp(
 // Declared in header, defined in main.
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -347,19 +347,25 @@
   if (!ID)
 return true;
 
-  const NamedDecl  = *cast(ASTNode.OrigD);
+  // FIXME: ObjCPropertyDecl are not properly indexed here:
+  // - ObjCPropertyDecl may have an OrigD of ObjCPropertyImplDecl, which is
+  // not a NamedDecl.
+  auto *OriginalDecl = dyn_cast(ASTNode.OrigD);
+  if (!OriginalDecl)
+return true;
+
   const Symbol *BasicSymbol = Symbols.find(*ID);
   if (!BasicSymbol) // Regardless of role, ND is the canonical declaration.
 BasicSymbol = addDeclaration(*ND, std::move(*ID), IsMainFileOnly);
-  else if (isPreferredDeclaration(OriginalDecl, Roles))
+  else if (isPreferredDeclaration(*OriginalDecl, Roles))
 // If OriginalDecl is preferred, replace the existing canonical
 // declaration (e.g. a class forward declaration). There should be at most
 // one duplicate as we expect to see only one preferred declaration per
 // TU, because in practice they are definitions.
-BasicSymbol = addDeclaration(OriginalDecl, std::move(*ID), IsMainFileOnly);
+BasicSymbol = addDeclaration(*OriginalDecl, std::move(*ID), 
IsMainFileOnly);
 
   if (Roles & static_cast(index::SymbolRole::Definition))
-addDefinition(OriginalDecl, *BasicSymbol);
+addDefinition(*OriginalDecl, *BasicSymbol);
   return true;
 }
 


Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -437,6 +437,23 @@
   QName("MyProtocol"), QName("MyProtocol::someMethodName3:")));
 }
 
+TEST_F(SymbolCollectorTest, ObjCPropertyImpl) {
+  const std::string Header = R"(
+@interface Container
+@property(nonatomic) int magic;
+@end
+
+@implementation Container
+@end
+  )";
+  TestFileName = testPath("test.m");
+  runSymbolCollector(Header, /*Main=*/"", {"-xobjective-c++"});
+  EXPECT_THAT(Symbols,
+  UnorderedElementsAre(
+  QName("Container"), QName("Container::magic"),
+  QName("Container::_magic")));
+}
+
 TEST_F(SymbolCollectorTest, Locations) {
   Annotations Header(R"cpp(
 // Declared in header, defined in main.
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -347,19 +347,25 @@
   if (!ID)
 return true;
 
-  const NamedDecl  = *cast(ASTNode.OrigD);
+  // FIXME: ObjCPropertyDecl are not properly indexed here:
+  // - ObjCPropertyDecl may have an OrigD of ObjCPropertyImplDecl, which is
+  // not a NamedDecl.
+  auto *OriginalDecl = dyn_cast(ASTNode.OrigD);
+  if (!OriginalDecl)
+return true;
+
   const Symbol *BasicSymbol = Symbols.find(*ID);
   if (!BasicSymbol) // Regardless of role, ND is the canonical declaration.
 BasicSymbol = addDeclaration(*ND, std::move(*ID), IsMainFileOnly);
-  else if (isPreferredDeclaration(OriginalDecl, Roles))
+  else if (isPreferredDeclaration(*OriginalDecl, Roles))
 // If OriginalDecl is preferred, replace the existing canonical
 // declaration (e.g. a class forward declaration). There should be at most
 // one duplicate as we expect to see only one preferred declaration per
 // TU, because in practice they are definitions.
-BasicSymbol = addDeclaration(OriginalDecl, std::move(*ID), IsMainFileOnly);
+BasicSymbol = addDeclaration(*OriginalDecl, std::move(*ID), IsMainFileOnly);
 
   if (Roles & 

[PATCH] D57004: [docs] Add release notes for notable things I've contributed since last release

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: docs/ReleaseNotes.rst:190
+- For MinGW, clang now produces vtables and RTTI for dllexported classes
+  without key functions.
+

hans wrote:
> mstorsjo wrote:
> > This comment doesn't really say much for the casual reader, perhaps it 
> > should just be left out and regarded as general assorted bugfixes? Although 
> > the fix (D55698) is a notable step towards matching GCC better for MinGW, 
> > and fixes an actual issue (building Qt in debug mode).
> I think keeping it in is fine. If you want you could of course expand with 
> mentioning the Qt part of your comment and maybe link to a bug entry if 
> applicable.
I agree, I think the info about Qt is the kind of newsworthy thing users will 
want to know and will understand.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57004/new/

https://reviews.llvm.org/D57004



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

[PATCH] D57004: [docs] Add release notes for notable things I've contributed since last release

[Hans Wennborg via Phabricator via cfe-commits]

hans accepted this revision.
hans added inline comments.
This revision is now accepted and ready to land.



Comment at: docs/ReleaseNotes.rst:190
+- For MinGW, clang now produces vtables and RTTI for dllexported classes
+  without key functions.
+

mstorsjo wrote:
> This comment doesn't really say much for the casual reader, perhaps it should 
> just be left out and regarded as general assorted bugfixes? Although the fix 
> (D55698) is a notable step towards matching GCC better for MinGW, and fixes 
> an actual issue (building Qt in debug mode).
I think keeping it in is fine. If you want you could of course expand with 
mentioning the Qt part of your comment and maybe link to a bug entry if 
applicable.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57004/new/

https://reviews.llvm.org/D57004



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

[PATCH] D56651: [ASTImporter] Fix importing OperatorDelete from CXXConstructorDecl

[Raphael Isemann via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC351849: [ASTImporter] Fix importing OperatorDelete from 
CXXConstructorDecl (authored by teemperor, committed by ).

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56651/new/

https://reviews.llvm.org/D56651

Files:
  lib/AST/ASTImporter.cpp
  test/Import/destructor/Inputs/F.cpp
  test/Import/destructor/test.cpp


Index: test/Import/destructor/Inputs/F.cpp
===
--- test/Import/destructor/Inputs/F.cpp
+++ test/Import/destructor/Inputs/F.cpp
@@ -0,0 +1,3 @@
+struct B {
+  virtual ~B() {}
+};
Index: test/Import/destructor/test.cpp
===
--- test/Import/destructor/test.cpp
+++ test/Import/destructor/test.cpp
@@ -0,0 +1,10 @@
+// RUN: clang-import-test -dump-ast -import %S/Inputs/F.cpp -expression %s
+
+// Triggers the deserialization of B's destructor.
+B b1;
+
+// CHECK: CXXDestructorDecl
+
+// CHECK-NEXT: ~B 'void () noexcept' virtual
+// CHECK-SAME: 'void () noexcept'
+// CHECK-SAME: virtual
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -3091,12 +3091,28 @@
 FromConstructor->isExplicit(),
 D->isInlineSpecified(), D->isImplicit(), D->isConstexpr()))
   return ToFunction;
-  } else if (isa(D)) {
+  } else if (CXXDestructorDecl *FromDtor = dyn_cast(D)) {
+
+auto Imp =
+importSeq(const_cast(FromDtor->getOperatorDelete()),
+  FromDtor->getOperatorDeleteThisArg());
+
+if (!Imp)
+  return Imp.takeError();
+
+FunctionDecl *ToOperatorDelete;
+Expr *ToThisArg;
+std::tie(ToOperatorDelete, ToThisArg) = *Imp;
+
 if (GetImportedOrCreateDecl(
 ToFunction, D, Importer.getToContext(), cast(DC),
 ToInnerLocStart, NameInfo, T, TInfo, D->isInlineSpecified(),
 D->isImplicit()))
   return ToFunction;
+
+CXXDestructorDecl *ToDtor = cast(ToFunction);
+
+ToDtor->setOperatorDelete(ToOperatorDelete, ToThisArg);
   } else if (CXXConversionDecl *FromConversion =
  dyn_cast(D)) {
 if (GetImportedOrCreateDecl(


Index: test/Import/destructor/Inputs/F.cpp
===
--- test/Import/destructor/Inputs/F.cpp
+++ test/Import/destructor/Inputs/F.cpp
@@ -0,0 +1,3 @@
+struct B {
+  virtual ~B() {}
+};
Index: test/Import/destructor/test.cpp
===
--- test/Import/destructor/test.cpp
+++ test/Import/destructor/test.cpp
@@ -0,0 +1,10 @@
+// RUN: clang-import-test -dump-ast -import %S/Inputs/F.cpp -expression %s
+
+// Triggers the deserialization of B's destructor.
+B b1;
+
+// CHECK: CXXDestructorDecl
+
+// CHECK-NEXT: ~B 'void () noexcept' virtual
+// CHECK-SAME: 'void () noexcept'
+// CHECK-SAME: virtual
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -3091,12 +3091,28 @@
 FromConstructor->isExplicit(),
 D->isInlineSpecified(), D->isImplicit(), D->isConstexpr()))
   return ToFunction;
-  } else if (isa(D)) {
+  } else if (CXXDestructorDecl *FromDtor = dyn_cast(D)) {
+
+auto Imp =
+importSeq(const_cast(FromDtor->getOperatorDelete()),
+  FromDtor->getOperatorDeleteThisArg());
+
+if (!Imp)
+  return Imp.takeError();
+
+FunctionDecl *ToOperatorDelete;
+Expr *ToThisArg;
+std::tie(ToOperatorDelete, ToThisArg) = *Imp;
+
 if (GetImportedOrCreateDecl(
 ToFunction, D, Importer.getToContext(), cast(DC),
 ToInnerLocStart, NameInfo, T, TInfo, D->isInlineSpecified(),
 D->isImplicit()))
   return ToFunction;
+
+CXXDestructorDecl *ToDtor = cast(ToFunction);
+
+ToDtor->setOperatorDelete(ToOperatorDelete, ToThisArg);
   } else if (CXXConversionDecl *FromConversion =
  dyn_cast(D)) {
 if (GetImportedOrCreateDecl(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r351849 - [ASTImporter] Fix importing OperatorDelete from CXXConstructorDecl

[Raphael Isemann via cfe-commits]

Author: teemperor
Date: Tue Jan 22 09:59:45 2019
New Revision: 351849

URL: http://llvm.org/viewvc/llvm-project?rev=351849=rev
Log:
[ASTImporter] Fix importing OperatorDelete from CXXConstructorDecl

Summary:
Shafik found out that importing a CXXConstructorDecl will create a translation 
unit that
causes Clang's CodeGen to crash. The reason for that is that we don't copy the 
OperatorDelete
from the CXXConstructorDecl when importing. This patch fixes it and adds a test 
case for that.

Reviewers: shafik, martong, a_sidorin, a.sidorin

Reviewed By: martong, a_sidorin

Subscribers: rnkovacs, cfe-commits

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

Added:
cfe/trunk/test/Import/destructor/
cfe/trunk/test/Import/destructor/Inputs/
cfe/trunk/test/Import/destructor/Inputs/F.cpp
cfe/trunk/test/Import/destructor/test.cpp
Modified:
cfe/trunk/lib/AST/ASTImporter.cpp

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=351849=351848=351849=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Tue Jan 22 09:59:45 2019
@@ -3091,12 +3091,28 @@ ExpectedDecl ASTNodeImporter::VisitFunct
 FromConstructor->isExplicit(),
 D->isInlineSpecified(), D->isImplicit(), D->isConstexpr()))
   return ToFunction;
-  } else if (isa(D)) {
+  } else if (CXXDestructorDecl *FromDtor = dyn_cast(D)) {
+
+auto Imp =
+importSeq(const_cast(FromDtor->getOperatorDelete()),
+  FromDtor->getOperatorDeleteThisArg());
+
+if (!Imp)
+  return Imp.takeError();
+
+FunctionDecl *ToOperatorDelete;
+Expr *ToThisArg;
+std::tie(ToOperatorDelete, ToThisArg) = *Imp;
+
 if (GetImportedOrCreateDecl(
 ToFunction, D, Importer.getToContext(), cast(DC),
 ToInnerLocStart, NameInfo, T, TInfo, D->isInlineSpecified(),
 D->isImplicit()))
   return ToFunction;
+
+CXXDestructorDecl *ToDtor = cast(ToFunction);
+
+ToDtor->setOperatorDelete(ToOperatorDelete, ToThisArg);
   } else if (CXXConversionDecl *FromConversion =
  dyn_cast(D)) {
 if (GetImportedOrCreateDecl(

Added: cfe/trunk/test/Import/destructor/Inputs/F.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Import/destructor/Inputs/F.cpp?rev=351849=auto
==
--- cfe/trunk/test/Import/destructor/Inputs/F.cpp (added)
+++ cfe/trunk/test/Import/destructor/Inputs/F.cpp Tue Jan 22 09:59:45 2019
@@ -0,0 +1,3 @@
+struct B {
+  virtual ~B() {}
+};

Added: cfe/trunk/test/Import/destructor/test.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Import/destructor/test.cpp?rev=351849=auto
==
--- cfe/trunk/test/Import/destructor/test.cpp (added)
+++ cfe/trunk/test/Import/destructor/test.cpp Tue Jan 22 09:59:45 2019
@@ -0,0 +1,10 @@
+// RUN: clang-import-test -dump-ast -import %S/Inputs/F.cpp -expression %s
+
+// Triggers the deserialization of B's destructor.
+B b1;
+
+// CHECK: CXXDestructorDecl
+
+// CHECK-NEXT: ~B 'void () noexcept' virtual
+// CHECK-SAME: 'void () noexcept'
+// CHECK-SAME: virtual


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

Re: r351629 - Emit !callback metadata and introduce the callback attribute

[Doerfert, Johannes Rudolf via cfe-commits]

On 01/22, Chandler Carruth wrote:
> On Sat, Jan 19, 2019 at 2:18 AM Johannes Doerfert via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> 
> > Author: jdoerfert
> > Date: Fri Jan 18 21:36:54 2019
> > New Revision: 351629
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=351629=rev
> > Log:
> > Emit !callback metadata and introduce the callback attribute
> >
> >   With commit r351627, LLVM gained the ability to apply (existing) IPO
> >   optimizations on indirections through callbacks, or transitive calls.
> >   The general idea is that we use an abstraction to hide the middle man
> >   and represent the callback call in the context of the initial caller.
> >   It is described in more detail in the commit message of the LLVM patch
> >   r351627, the llvm::AbstractCallSite class description, and the
> >   language reference section on callback-metadata.
> >
> >   This commit enables clang to emit !callback metadata that is
> >   understood by LLVM. It does so in three different cases:
> > 1) For known broker functions declarations that are directly
> >generated, e.g., __kmpc_fork_call for the OpenMP pragma parallel.
> > 2) For known broker functions that are identified by their name and
> >source location through the builtin detection, e.g.,
> >pthread_create from the POSIX thread API.
> > 3) For user annotated functions that carry the "callback(callee, ...)"
> >attribute. The attribute has to include the name, or index, of
> >the callback callee and how the passed arguments can be
> >identified (as many as the callback callee has). See the callback
> >attribute documentation for detailed information.
> >
> > Differential Revision: https://reviews.llvm.org/D55483
> >
> > Added: cfe/trunk/test/Sema/attr-callback-broken.c
> > URL:
> > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/attr-callback-broken.c?rev=351629=auto
> >
> > ==
> > --- cfe/trunk/test/Sema/attr-callback-broken.c (added)
> > +++ cfe/trunk/test/Sema/attr-callback-broken.c Fri Jan 18 21:36:54 2019
> > @@ -0,0 +1,75 @@
> > +// RUN: %clang_cc1 %s -verify -fsyntax-only
> > +
> > +__attribute__((callback())) void no_callee(void (*callback)(void)); //
> > expected-error {{'callback' attribute specifies no callback callee}}
> > +
> > +__attribute__((callback(1, 1))) void too_many_args_1(void
> > (*callback)(void)) {}  // expected-error {{'callback' attribute takes
> > one argument}}
> > +__attribute__((callback(1, -1))) void too_many_args_2(double
> > (*callback)(void)); // expected-error {{'callback' attribute takes one
> > argument}}
> > +__attribute__((callback(1, 2, 2))) void too_many_args_3(void
> > (*callback)(int), int); // expected-error {{'callback' attribute requires
> > exactly 2 arguments}}
> > +
> > +__attribute__((callback(1, 2))) void too_few_args_1(void (*callback)(int,
> > int), int); // expected-error {{'callback' attribute takes one argument}}
> > +__attribute__((callback(1))) void too_few_args_2(int (*callback)(int));
> >  // expected-error {{'callback' attribute takes no arguments}}
> > +__attribute__((callback(1, -1))) void too_few_args_3(void
> > (*callback)(int, int)) {}   // expected-error {{'callback' attribute takes
> > one argument}}
> > +
> > +__attribute__((callback(-1))) void oob_args_1(void (*callback)(void));
> >  // expected-error {{'callback' attribute specifies invalid callback
> > callee}}
> > +__attribute__((callback(2))) void oob_args_2(int *(*callback)(void)) {}
> >   // expected-error {{'callback' attribute parameter 1 is out of
> > bounds}}
> > +__attribute__((callback(1, 3))) void oob_args_3(short (*callback)(int),
> > int);  // expected-error {{'callback' attribute parameter 2 is out of
> > bounds}}
> > +__attribute__((callback(-2, 2))) void oob_args_4(void *(*callback)(int),
> > int); // expected-error {{'callback' attribute parameter 1 is out of
> > bounds}}
> > +__attribute__((callback(1, -2))) void oob_args_5(void *(*callback)(int),
> > int); // expected-error {{'callback' attribute parameter 2 is out of
> > bounds}}
> > +__attribute__((callback(1, 2))) void oob_args_6(void *(*callback)(int),
> > ...);  // expected-error {{'callback' attribute parameter 2 is out of
> > bounds}}
> > +
> > +__attribute__((callback(1))) __attribute__((callback(1))) void
> > multiple_cb_1(void (*callback)(void));   //
> > expected-error {{multiple 'callback' attributes specified}}
> > +__attribute__((callback(1))) __attribute__((callback(2))) void
> > multiple_cb_2(void (*callback1)(void), void (*callback2)(void)); //
> > expected-error {{multiple 'callback' attributes specified}}
> > +
> > +#ifdef HAS_THIS
> > +__attribute__((callback(0))) void oob_args_0(void (*callback)(void)); //
> > expected-error {{'callback' attribute specifies invalid callback callee}}
> > +#else
> > +__attribute__((callback(0))) void 

[PATCH] D55447: [Sema] Fix Modified Type in address_space AttributedType

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

@rsmith Any comments on this patch before submitting?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55447/new/

https://reviews.llvm.org/D55447



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

Re: r351629 - Emit !callback metadata and introduce the callback attribute

[Doerfert, Johannes Rudolf via cfe-commits]

On 01/22, Chandler Carruth wrote:
> On Sat, Jan 19, 2019 at 2:18 AM Johannes Doerfert via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> 
> > Author: jdoerfert
> > Date: Fri Jan 18 21:36:54 2019
> > New Revision: 351629
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=351629=rev
> > Log:
> > Emit !callback metadata and introduce the callback attribute
> >
> >   With commit r351627, LLVM gained the ability to apply (existing) IPO
> >   optimizations on indirections through callbacks, or transitive calls.
> >   The general idea is that we use an abstraction to hide the middle man
> >   and represent the callback call in the context of the initial caller.
> >   It is described in more detail in the commit message of the LLVM patch
> >   r351627, the llvm::AbstractCallSite class description, and the
> >   language reference section on callback-metadata.
> >
> >   This commit enables clang to emit !callback metadata that is
> >   understood by LLVM. It does so in three different cases:
> > 1) For known broker functions declarations that are directly
> >generated, e.g., __kmpc_fork_call for the OpenMP pragma parallel.
> > 2) For known broker functions that are identified by their name and
> >source location through the builtin detection, e.g.,
> >pthread_create from the POSIX thread API.
> > 3) For user annotated functions that carry the "callback(callee, ...)"
> >attribute. The attribute has to include the name, or index, of
> >the callback callee and how the passed arguments can be
> >identified (as many as the callback callee has). See the callback
> >attribute documentation for detailed information.
> >
> > Differential Revision: https://reviews.llvm.org/D55483
> >
> > ==
> > --- cfe/trunk/test/CodeGen/callback_pthread_create.c (added)
> > +++ cfe/trunk/test/CodeGen/callback_pthread_create.c Fri Jan 18 21:36:54
> > 2019
> > @@ -0,0 +1,32 @@
> > +// RUN: %clang -O1 %s -S -c -emit-llvm -o - | FileCheck %s
> > +// RUN: %clang -O1 %s -S -c -emit-llvm -o - | opt -ipconstprop -S |
> > FileCheck --check-prefix=IPCP %s
> > +
> > +// CHECK: declare !callback ![[cid:[0-9]+]] dso_local i32 @pthread_create
> > +// CHECK: ![[cid]] = !{![[cidb:[0-9]+]]}
> > +// CHECK: ![[cidb]] = !{i64 2, i64 3, i1 false}
> > +
> > +#include 
> 
> 
> Another thing I notecide is that this code assumes the system has
> `pthread.h` -- what about systems without it? I mean, you can disable the
> test, but it seems bad to lose test coverage just because of that.
 
So far, I disabled the test with a later addition which makes sure this
test is only run under Linux. I'm unsure why we loose coverage because
of that?

> I would much prefer that you provide your own stub `pthread.h` in the
> Inputs/... tree of the test suite and use that to test this in a portable
> way.

I do not completely follow but I'm open to improving the test. Basically
I have to make sure the builtin recognition will trigger on the header
file and the contained declaration. If we can somehow do this in a
portable way I'm all for it. Is that how we test other builtin gnu extensions?

Cheers,
  Johannes


> > +
> > +const int GlobalVar = 0;
> > +
> > +static void *callee0(void *payload) {
> > +// IPCP:  define internal i8* @callee0
> > +// IPCP-NEXT:   entry:
> > +// IPCP-NEXT: ret i8* null
> > +  return payload;
> > +}
> > +
> > +static void *callee1(void *payload) {
> > +// IPCP:  define internal i8* @callee1
> > +// IPCP-NEXT:   entry:
> > +// IPCP-NEXT: ret i8* bitcast (i32* @GlobalVar to i8*)
> > +  return payload;
> > +}
> > +
> > +void foo() {
> > +  pthread_t MyFirstThread;
> > +  pthread_create(, NULL, callee0, NULL);
> > +
> > +  pthread_t MySecondThread;
> > +  pthread_create(, NULL, callee1, (void *));
> > +}
> >
> > Added: cfe/trunk/test/CodeGenCXX/attr-callback.cpp
> > URL:
> > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/attr-callback.cpp?rev=351629=auto


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

[PATCH] D56723: [CodeComplete] Propagate preferred types through parser in more cases

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Thanks for the suggestion, this should definitely work! I did struggle to 
figure out a way to do this without annotating every path with `enterUnknown` 
and failed.
I'll try playing around with your idea, my initial plan is to store preferred 
type alongside the current token as a member of the `Parser` class and update 
it when advancing to next token, when the parser backtracks and in the places 
where we care about propagating the types.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56723/new/

https://reviews.llvm.org/D56723



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

Re: r351580 - [OPENMP][DOCS] Release notes/OpenMP support updates, NFC.

[Hans Wennborg via cfe-commits]

Thanks, I've merged it to 8.0 in r351839.

Since the release notes are for 8.0 and not trunk, I've removed the
change from trunk in r351841.

On Fri, Jan 18, 2019 at 12:05 PM Kelvin Li  wrote:
>
> Hi Hans,
>
> I am not sure if it is the proper way to request the change committed
> to the release_80 branch.  Please let me know if I need to do
> something else.
>
> Thanks,
> Kelvin
>
> -- Forwarded message -
> From: Kelvin Li via cfe-commits 
> Date: Fri, Jan 18, 2019 at 3:01 PM
> Subject: r351580 - [OPENMP][DOCS] Release notes/OpenMP support updates, NFC.
> To: 
>
>
> Author: kli
> Date: Fri Jan 18 11:57:37 2019
> New Revision: 351580
>
> URL: http://llvm.org/viewvc/llvm-project?rev=351580=rev
> Log:
> [OPENMP][DOCS] Release notes/OpenMP support updates, NFC.
>
> Differential Revision: https://reviews.llvm.org/D56733
>
> Modified:
> cfe/trunk/docs/OpenMPSupport.rst
> cfe/trunk/docs/ReleaseNotes.rst
>
> Modified: cfe/trunk/docs/OpenMPSupport.rst
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/OpenMPSupport.rst?rev=351580=351579=351580=diff
> ==
> --- cfe/trunk/docs/OpenMPSupport.rst (original)
> +++ cfe/trunk/docs/OpenMPSupport.rst Fri Jan 18 11:57:37 2019
> @@ -17,60 +17,50 @@
>  OpenMP Support
>  ==
>
> -Clang fully supports OpenMP 4.5. Clang supports offloading to X86_64, 
> AArch64,
> -PPC64[LE] and has `basic support for Cuda devices`_.
> -
> -Standalone directives
> -=
> -
> -* #pragma omp [for] simd: :good:`Complete`.
> -
> -* #pragma omp declare simd: :partial:`Partial`.  We support parsing/semantic
> -  analysis + generation of special attributes for X86 target, but still
> -  missing the LLVM pass for vectorization.
> -
> -* #pragma omp taskloop [simd]: :good:`Complete`.
> -
> -* #pragma omp target [enter|exit] data: :good:`Complete`.
> -
> -* #pragma omp target update: :good:`Complete`.
> -
> -* #pragma omp target: :good:`Complete`.
> +Clang supports the following OpenMP 5.0 features
>
> -* #pragma omp declare target: :good:`Complete`.
> +* The `reduction`-based clauses in the `task` and `target`-based directives.
>
> -* #pragma omp teams: :good:`Complete`.
> +* Support relational-op != (not-equal) as one of the canonical forms of 
> random
> +  access iterator.
>
> -* #pragma omp distribute [simd]: :good:`Complete`.
> +* Support for mapping of the lambdas in target regions.
>
> -* #pragma omp distribute parallel for [simd]: :good:`Complete`.
> +* Parsing/sema analysis for the requires directive.
>
> -Combined directives
> -===
> +* Nested declare target directives.
>
> -* #pragma omp parallel for simd: :good:`Complete`.
> +* Make the `this` pointer implicitly mapped as `map(this[:1])`.
>
> -* #pragma omp target parallel: :good:`Complete`.
> +* The `close` *map-type-modifier*.
>
> -* #pragma omp target parallel for [simd]: :good:`Complete`.
> -
> -* #pragma omp target simd: :good:`Complete`.
> -
> -* #pragma omp target teams: :good:`Complete`.
> -
> -* #pragma omp teams distribute [simd]: :good:`Complete`.
> -
> -* #pragma omp target teams distribute [simd]: :good:`Complete`.
> -
> -* #pragma omp teams distribute parallel for [simd]: :good:`Complete`.
> -
> -* #pragma omp target teams distribute parallel for [simd]: :good:`Complete`.
> +Clang fully supports OpenMP 4.5. Clang supports offloading to X86_64, 
> AArch64,
> +PPC64[LE] and has `basic support for Cuda devices`_.
>
> -Clang does not support any constructs/updates from OpenMP 5.0 except
> -for `reduction`-based clauses in the `task` and `target`-based directives.
> +* #pragma omp declare simd: :partial:`Partial`.  We support parsing/semantic
> +  analysis + generation of special attributes for X86 target, but still
> +  missing the LLVM pass for vectorization.
>
>  In addition, the LLVM OpenMP runtime `libomp` supports the OpenMP Tools
> -Interface (OMPT) on x86, x86_64, AArch64, and PPC64 on Linux,
> Windows, and mac OS.
> +Interface (OMPT) on x86, x86_64, AArch64, and PPC64 on Linux,
> Windows, and macOS.
> +
> +General improvements
> +
> +- New collapse clause scheme to avoid expensive remainder operations.
> +  Compute loop index variables after collapsing a loop nest via the
> +  collapse clause by replacing the expensive remainder operation with
> +  multiplications and additions.
> +
> +- The default schedules for the `distribute` and `for` constructs in a
> +  parallel region and in SPMD mode have changed to ensure coalesced
> +  accesses. For the `distribute` construct, a static schedule is used
> +  with a chunk size equal to the number of threads per team (default
> +  value of threads or as specified by the `thread_limit` clause if
> +  present). For the `for` construct, the schedule is static with chunk
> +  size of one.
> +
> +- Simplified SPMD code generation for `distribute parallel for` when
> +  the new default schedules are applicable.
>

r351841 - ReleaseNotes: remove openmp notes from r351580

[Hans Wennborg via cfe-commits]

Author: hans
Date: Tue Jan 22 09:01:39 2019
New Revision: 351841

URL: http://llvm.org/viewvc/llvm-project?rev=351841=rev
Log:
ReleaseNotes: remove openmp notes from r351580

They were for the 8.0 branch, and have been committed there in r351839.

Modified:
cfe/trunk/docs/ReleaseNotes.rst

Modified: cfe/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=351841=351840=351841=diff
==
--- cfe/trunk/docs/ReleaseNotes.rst (original)
+++ cfe/trunk/docs/ReleaseNotes.rst Tue Jan 22 09:01:39 2019
@@ -133,36 +133,7 @@ ABI Changes in Clang
 OpenMP Support in Clang
 --
 
-- OpenMP 5.0 features
-
-  - Support relational-op != (not-equal) as one of the canonical forms of 
random
-access iterator.
-  - Added support for mapping of the lambdas in target regions.
-  - Added parsing/sema analysis for the requires directive.
-  - Support nested declare target directives.
-  - Make the `this` pointer implicitly mapped as `map(this[:1])`.
-  - Added the `close` *map-type-modifier*.
-
-- Various bugfixes and improvements.
-
-New features supported for Cuda devices:
-
-- Added support for the reductions across the teams.
-
-- Extended number of constructs that can be executed in SPMD mode.
-
-- Fixed support for lastprivate/reduction variables in SPMD constructs.
-
-- New collapse clause scheme to avoid expensive remainder operations.
-
-- New default schedule for distribute and parallel constructs.
-
-- Simplified code generation for distribute and parallel in SPMD mode.
-
-- Flag (``-fopenmp_optimistic_collapse``) for user to limit collapsed
-  loop counter width when safe to do so.
-
-- General performance improvement.
+- ...
 
 CUDA Support in Clang
 -


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

[PATCH] D56651: [ASTImporter] Fix importing OperatorDelete from CXXConstructorDecl

[Gabor Marton via Phabricator via cfe-commits]

martong accepted this revision.
martong added a comment.

The updated version looks good to me! Thank you!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56651/new/

https://reviews.llvm.org/D56651



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

[PATCH] D56850: [ASTMatchers][NFC] Add tests for assorted `CXXMemberCallExpr` matchers.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added inline comments.



Comment at: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp:558
+  int m;
+  int f(X x) { return m; }
+};

steveire wrote:
> Are we missing a matcher that would reach the type of X in this case? 
> `hasImplicitObjectExpression`, or something equivalent?
Good question. The reason we're not reaching `X` in this case is that we're 
distinguishing between `T` and `T*` (like `thisPointerType` does). That's 
separate from the difference between `on` and `onImplicitObjectArgument`.  So, 
I think we'll at least want a matcher that elides this difference, e.g.
`hasObjectType`.

I'm less inclined to add matchers that distinguish the written from the coerced 
member expression (one variant each for the expression and type matchers) given 
that I think this issue comes up far less for member expressions that don't 
involve calls. I'm just afraid that the proliferation of matchers might confuse 
beginners.

Note that I extended the test to include the hasPointerType() case (especially 
since it's now mentioned in the comments).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56850/new/

https://reviews.llvm.org/D56850



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

[PATCH] D56850: [ASTMatchers][NFC] Add tests for assorted `CXXMemberCallExpr` matchers.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 182933.
ymandel marked 3 inline comments as done.
ymandel added a comment.

Extended test of `hasObjectExpression`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56850/new/

https://reviews.llvm.org/D56850

Files:
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -470,6 +470,100 @@
 
 }
 
+TEST(MatcherCXXMemberCallExpr, On) {
+  auto Snippet1 = R"cc(
+struct Y {
+  void m();
+};
+void z(Y y) { y.m(); }
+  )cc";
+  auto Snippet2 = R"cc(
+struct Y {
+  void m();
+};
+struct X : public Y {};
+void z(X x) { x.m(); }
+  )cc";
+  auto MatchesY = cxxMemberCallExpr(on(hasType(cxxRecordDecl(hasName("Y");
+  EXPECT_TRUE(matches(Snippet1, MatchesY));
+  EXPECT_TRUE(notMatches(Snippet2, MatchesY));
+
+  auto MatchesX = cxxMemberCallExpr(on(hasType(cxxRecordDecl(hasName("X");
+  EXPECT_TRUE(matches(Snippet2, MatchesX));
+
+  // Parens are ignored.
+  auto MatchesCall = cxxMemberCallExpr(on(callExpr()));
+  EXPECT_TRUE(matches(
+  R"cc(
+struct Y {
+  void m();
+};
+Y g();
+void z(Y y) { (g()).m(); }
+  )cc",
+  MatchesCall));
+}
+
+TEST(MatcherCXXMemberCallExpr, OnImplicitObjectArgument) {
+  auto Snippet1 = R"cc(
+struct Y {
+  void m();
+};
+void z(Y y) { y.m(); }
+  )cc";
+  auto Snippet2 = R"cc(
+struct Y {
+  void m();
+};
+struct X : public Y {};
+void z(X x) { x.m(); }
+  )cc";
+  auto MatchesY = cxxMemberCallExpr(
+  onImplicitObjectArgument(hasType(cxxRecordDecl(hasName("Y");
+  EXPECT_TRUE(matches(Snippet1, MatchesY));
+  EXPECT_TRUE(matches(Snippet2, MatchesY));
+
+  auto MatchesX = cxxMemberCallExpr(
+  onImplicitObjectArgument(hasType(cxxRecordDecl(hasName("X");
+  EXPECT_TRUE(notMatches(Snippet2, MatchesX));
+
+  // Parens are not ignored.
+  auto MatchesCall = cxxMemberCallExpr(onImplicitObjectArgument(callExpr()));
+  EXPECT_TRUE(notMatches(
+  R"cc(
+struct Y {
+  void m();
+};
+Y g();
+void z(Y y) { (g()).m(); }
+  )cc",
+  MatchesCall));
+}
+
+TEST(Matcher, HasObjectExpr) {
+  auto Snippet1 = R"cc(
+struct X {
+  int m;
+  int f(X x) { return x.m; }
+};
+  )cc";
+  auto Snippet2 = R"cc(
+struct X {
+  int m;
+  int f(X x) { return m; }
+};
+  )cc";
+  auto MatchesX =
+  memberExpr(hasObjectExpression(hasType(cxxRecordDecl(hasName("X");
+  EXPECT_TRUE(matches(Snippet1, MatchesX));
+  EXPECT_TRUE(notMatches(Snippet2, MatchesX));
+
+  auto MatchesXPointer = memberExpr(
+  hasObjectExpression(hasType(pointsTo(cxxRecordDecl(hasName("X"));
+  EXPECT_TRUE(notMatches(Snippet1, MatchesXPointer));
+  EXPECT_TRUE(matches(Snippet2, MatchesXPointer));
+}
+
 TEST(ForEachArgumentWithParam, ReportsNoFalsePositives) {
   StatementMatcher ArgumentY =
 declRefExpr(to(varDecl(hasName("y".bind("arg");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56936: Fix handling of overriden methods during ASTImport

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: lib/AST/ASTImporter.cpp:3046
+if (!D->doesThisDeclarationHaveABody())
+  return cast(const_cast(FoundByLookup));
+else {

balazske wrote:
> The `cast` should not be needed here (and is not done at the other 
> return places). The `const_cast` can be omitted too (`FoundByLookup` is not 
> const now).
We must also register the decl into the map of imported decls as we do in all 
the other cases.
```
return Importer.MapImported(D, FoundByLookup);
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56936/new/

https://reviews.llvm.org/D56936



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

[PATCH] D57060: [NFC][Clang] Add driver tests for sb and predres

[Diogo N. Sampaio via Phabricator via cfe-commits]

dnsampaio created this revision.
dnsampaio added a reviewer: pbarrio.
Herald added subscribers: cfe-commits, javed.absar.

Add tests that arguments for enabling/disabling sb and predres are correctly 
being or not passed by the driver.


Repository:
  rC Clang

https://reviews.llvm.org/D57060

Files:
  test/Driver/aarch64-predres.c
  test/Driver/arm-sb.c


Index: test/Driver/arm-sb.c
===
--- /dev/null
+++ test/Driver/arm-sb.c
@@ -0,0 +1,14 @@
+// RUN: %clang -### -target arm-none-none-eabi -march=armv8a+sb %s 2>&1 | 
FileCheck %s
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8a+sb %s 2>&1 | 
FileCheck %s
+// CHECK: "-target-feature" "+sb"
+// CHECK-NOT: "-target-feature" "-sb"
+
+// RUN: %clang -### -target arm-none-none-eabi -march=armv8.5a+nosb %s 2>&1 | 
FileCheck %s --check-prefix=NOSB
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.5a+nosb %s 
2>&1 | FileCheck %s --check-prefix=NOSB
+// NOSB: "-target-feature" "-sb"
+// NOSB-NOT: "-target-feature" "+sb"
+
+// RUN: %clang -### -target arm-none-none-eabi %s 2>&1 | FileCheck %s 
--check-prefix=ABSENT
+// RUN: %clang -### -target aarch64-none-none-eabi %s 2>&1 | FileCheck %s 
--check-prefix=ABSENT
+// ABSENT-NOT: "-target-feature" "+sb"
+// ABSENT-NOT: "-target-feature" "-sb"
Index: test/Driver/aarch64-predres.c
===
--- /dev/null
+++ test/Driver/aarch64-predres.c
@@ -0,0 +1,11 @@
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8a+predres 
%s 2>&1 | FileCheck %s
+// CHECK: "-target-feature" "+predres"
+// CHECK-NOT: "-target-feature" "-predres"
+
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.5a+nopredres 
%s 2>&1 | FileCheck %s --check-prefix=NOPR
+// NOPR: "-target-feature" "-predres"
+// NOPR-NOT: "-target-feature" "+predres"
+
+// RUN: %clang -### -target aarch64-none-none-eabi   
%s 2>&1 | FileCheck %s --check-prefix=ABSENT
+// ABSENT-NOT: "-target-feature" "+predres"
+// ABSENT-NOT: "-target-feature" "-predres"


Index: test/Driver/arm-sb.c
===
--- /dev/null
+++ test/Driver/arm-sb.c
@@ -0,0 +1,14 @@
+// RUN: %clang -### -target arm-none-none-eabi -march=armv8a+sb %s 2>&1 | FileCheck %s
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8a+sb %s 2>&1 | FileCheck %s
+// CHECK: "-target-feature" "+sb"
+// CHECK-NOT: "-target-feature" "-sb"
+
+// RUN: %clang -### -target arm-none-none-eabi -march=armv8.5a+nosb %s 2>&1 | FileCheck %s --check-prefix=NOSB
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.5a+nosb %s 2>&1 | FileCheck %s --check-prefix=NOSB
+// NOSB: "-target-feature" "-sb"
+// NOSB-NOT: "-target-feature" "+sb"
+
+// RUN: %clang -### -target arm-none-none-eabi %s 2>&1 | FileCheck %s --check-prefix=ABSENT
+// RUN: %clang -### -target aarch64-none-none-eabi %s 2>&1 | FileCheck %s --check-prefix=ABSENT
+// ABSENT-NOT: "-target-feature" "+sb"
+// ABSENT-NOT: "-target-feature" "-sb"
Index: test/Driver/aarch64-predres.c
===
--- /dev/null
+++ test/Driver/aarch64-predres.c
@@ -0,0 +1,11 @@
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8a+predres %s 2>&1 | FileCheck %s
+// CHECK: "-target-feature" "+predres"
+// CHECK-NOT: "-target-feature" "-predres"
+
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.5a+nopredres %s 2>&1 | FileCheck %s --check-prefix=NOPR
+// NOPR: "-target-feature" "-predres"
+// NOPR-NOT: "-target-feature" "+predres"
+
+// RUN: %clang -### -target aarch64-none-none-eabi   %s 2>&1 | FileCheck %s --check-prefix=ABSENT
+// ABSENT-NOT: "-target-feature" "+predres"
+// ABSENT-NOT: "-target-feature" "-predres"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56936: Fix handling of overriden methods during ASTImport

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Shafik,
I have realized what's the problem with the `ctu-main` test. When we import the 
body and set the new body to the existing FunctionDecl then the parameters are 
still the old parameters so the new body does not refer to the formal 
parameters! This way the analyzer legally thinks that the parameter is not used 
inside the function because there is no DeclRef to that :(
This could be solved only if we merge *every* parts precisely, including the 
parameters, body, noexcept specifier, etc. But this would be a huge work.

Also, the test in `ctu-main` contains ODR violations. E.g, below `fcl` is first 
just a protoype, then it is defined in-class in the second TU.

  // ctu-main.cpp
  class mycls {
  public:
int fcl(int x);
//...
  
  // ctu-other.cpp
   class mycls {
   public:
int fcl(int x) {
  return x + 5;
}
//...

In the second TU it should be defined out-of-class.

So my suggestion is to

1. use out-of-class definition of functions in `ctu-other.cpp`. Since I tried 
it already i have the diff for the lit tests, attached.F7837319: 
lit_tests.patch 
2. skip the case when there is a definition in the 'from' context and let it do 
the redecl chain.

For 2) I was thinking about this:

  if (FoundByLookup) {
if (auto *MD = dyn_cast(FoundByLookup)) {
  if (D->getLexicalDeclContext() == D->getDeclContext()) {
if (!D->doesThisDeclarationHaveABody())
  return cast(const_cast(FoundByLookup));
else {
  // Let's continue and build up the redecl chain in this case.
  // FIXME Merge the functions into one decl.
}
  }
}
  }

Later, we must implement the case when we have to merge the definition into the 
prototype properly, but that should be definitely another patch. 
Actually, this case comes up mostly with class template specializations , 
because different specializations may have a function prototype in one TU, but 
a definition for that in another TU (see e.g. the test 
`MergeFieldDeclsOfClassTemplateSpecialization`).




Comment at: lib/AST/ASTImporter.cpp:3042
 
+  if (FoundByLookup) {
+if (auto *MD = dyn_cast(FoundByLookup)) {

It is not trivial why we add this block to the code, so I think a comment would 
be really appreciated here.
I was thinking about something like this:
```
+  // We do not allow more than one in-class prototype of a function.  This is
+  // because AST clients like VTableBuilder asserts on this.  VTableBuilder
+  // assumes that only one function can override a function. Building a redecl
+  // chain would result there are more than one function which override the
+  // base function (even if they are part of the same redecl chain inside the
+  // derived class.)
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56936/new/

https://reviews.llvm.org/D56936



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

[PATCH] D56916: Fix crash due to ObjCPropertyDecl

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

> Unfortunately I can't get ninja check-clangd to build:

Looks like `clang-tools-extra` uses an old revision. Rebasing after rL350916 
 should do the trick.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56916/new/

https://reviews.llvm.org/D56916



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

[PATCH] D57021: [clangd] Suggest adding missing includes for typos (like include-fixer).

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 182925.
ioeric added a comment.

- Rebase on D56903 


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57021/new/

https://reviews.llvm.org/D57021

Files:
  clangd/ClangdUnit.cpp
  clangd/IncludeFixer.cpp
  clangd/IncludeFixer.h
  unittests/clangd/DiagnosticsTests.cpp

Index: unittests/clangd/DiagnosticsTests.cpp
===
--- unittests/clangd/DiagnosticsTests.cpp
+++ unittests/clangd/DiagnosticsTests.cpp
@@ -30,6 +30,11 @@
   return Field(::Fixes, ElementsAre(FixMatcher));
 }
 
+testing::Matcher WithFix(testing::Matcher FixMatcher1,
+   testing::Matcher FixMatcher2) {
+  return Field(::Fixes, UnorderedElementsAre(FixMatcher1, FixMatcher2));
+}
+
 testing::Matcher WithNote(testing::Matcher NoteMatcher) {
   return Field(::Notes, ElementsAre(NoteMatcher));
 }
@@ -280,6 +285,25 @@
   Pair(EqualToLSPDiag(NoteInMainLSP), IsEmpty(;
 }
 
+struct SymbolWithHeader {
+  std::string QName;
+  std::string DeclaringFile;
+  std::string IncludeHeader;
+};
+
+std::unique_ptr
+buildIndexWithSymbol(llvm::ArrayRef Syms) {
+  SymbolSlab::Builder Slab;
+  for (const auto  : Syms) {
+Symbol Sym = cls(S.QName);
+Sym.Flags |= Symbol::IndexedForCodeCompletion;
+Sym.CanonicalDeclaration.FileURI = S.DeclaringFile.c_str();
+Sym.IncludeHeaders.emplace_back(S.IncludeHeader, 1);
+Slab.insert(Sym);
+  }
+  return MemIndex::build(std::move(Slab).build(), RefSlab());
+}
+
 TEST(IncludeFixerTest, IncompleteType) {
   Annotations Test(R"cpp(
 $insert[[]]namespace ns {
@@ -292,14 +316,8 @@
 }
   )cpp");
   auto TU = TestTU::withCode(Test.code());
-  Symbol Sym = cls("ns::X");
-  Sym.Flags |= Symbol::IndexedForCodeCompletion;
-  Sym.CanonicalDeclaration.FileURI = "unittest:///x.h";
-  Sym.IncludeHeaders.emplace_back("\"x.h\"", 1);
-
-  SymbolSlab::Builder Slab;
-  Slab.insert(Sym);
-  auto Index = MemIndex::build(std::move(Slab).build(), RefSlab());
+  auto Index = buildIndexWithSymbol(
+  {SymbolWithHeader{"ns::X", "unittest:///x.h", "\"x.h\""}});
   TU.ExternalIndex = Index.get();
 
   EXPECT_THAT(
@@ -314,6 +332,64 @@
 "Add include \"x.h\" for symbol ns::X");
 }
 
+TEST(IncludeFixerTest, Typo) {
+  Annotations Test(R"cpp(
+$insert[[]]namespace ns {
+void foo() {
+  $unqualified[[X]] x;
+}
+}
+void bar() {
+  ns::$qualified[[X]] x; // ns:: is valid.
+  ::$global[[Global]] glob;
+}
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  auto Index = buildIndexWithSymbol(
+  {SymbolWithHeader{"ns::X", "unittest:///x.h", "\"x.h\""},
+   SymbolWithHeader{"Global", "unittest:///global.h", "\"global.h\""}});
+  TU.ExternalIndex = Index.get();
+
+  EXPECT_THAT(
+  TU.build().getDiagnostics(),
+  UnorderedElementsAre(
+  AllOf(Diag(Test.range("unqualified"), "unknown type name 'X'"),
+WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
+"Add include \"x.h\" for symbol ns::X"))),
+  AllOf(Diag(Test.range("qualified"),
+ "no type named 'X' in namespace 'ns'"),
+WithFix(Fix(Test.range("insert"), "#include \"x.h\"\n",
+"Add include \"x.h\" for symbol ns::X"))),
+  AllOf(Diag(Test.range("global"),
+ "no type named 'Global' in the global namespace"),
+WithFix(Fix(Test.range("insert"), "#include \"global.h\"\n",
+"Add include \"global.h\" for symbol Global");
+}
+
+TEST(IncludeFixerTest, MultipleMatchedSymbols) {
+  Annotations Test(R"cpp(
+$insert[[]]namespace na {
+namespace nb {
+void foo() {
+  $unqualified[[X]] x;
+}
+}
+}
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  auto Index = buildIndexWithSymbol(
+  {SymbolWithHeader{"na::X", "unittest:///a.h", "\"a.h\""},
+   SymbolWithHeader{"na::nb::X", "unittest:///b.h", "\"b.h\""}});
+  TU.ExternalIndex = Index.get();
+
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  UnorderedElementsAre(AllOf(
+  Diag(Test.range("unqualified"), "unknown type name 'X'"),
+  WithFix(Fix(Test.range("insert"), "#include \"a.h\"\n",
+  "Add include \"a.h\" for symbol na::X"),
+  Fix(Test.range("insert"), "#include \"b.h\"\n",
+  "Add include \"b.h\" for symbol na::nb::X");
+}
 
 } // namespace
 } // namespace clangd
Index: clangd/IncludeFixer.h
===
--- clangd/IncludeFixer.h
+++ clangd/IncludeFixer.h
@@ -14,6 +14,13 @@
 #include "index/Index.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include 

[PATCH] D56903: [clangd] Suggest adding missing includes for incomplete type diagnostics.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 182922.
ioeric marked 15 inline comments as done.
ioeric added a comment.

- Address review comments


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56903/new/

https://reviews.llvm.org/D56903

Files:
  clangd/CMakeLists.txt
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/CodeComplete.cpp
  clangd/Compiler.h
  clangd/Diagnostics.cpp
  clangd/Diagnostics.h
  clangd/Headers.cpp
  clangd/Headers.h
  clangd/IncludeFixer.cpp
  clangd/IncludeFixer.h
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  clangd/tool/ClangdMain.cpp
  unittests/clangd/CMakeLists.txt
  unittests/clangd/ClangdUnitTests.cpp
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/DiagnosticsTests.cpp
  unittests/clangd/FileIndexTests.cpp
  unittests/clangd/TUSchedulerTests.cpp
  unittests/clangd/TestIndex.cpp
  unittests/clangd/TestIndex.h
  unittests/clangd/TestTU.cpp
  unittests/clangd/TestTU.h

Index: unittests/clangd/TestTU.h
===
--- unittests/clangd/TestTU.h
+++ unittests/clangd/TestTU.h
@@ -49,6 +49,8 @@
   std::vector ExtraArgs;
 
   llvm::Optional ClangTidyChecks;
+  // Index to use when building AST.
+  const SymbolIndex *ExternalIndex = nullptr;
 
   ParsedAST build() const;
   SymbolSlab headerSymbols() const;
Index: unittests/clangd/TestTU.cpp
===
--- unittests/clangd/TestTU.cpp
+++ unittests/clangd/TestTU.cpp
@@ -35,8 +35,11 @@
   Inputs.CompileCommand.Directory = testRoot();
   Inputs.Contents = Code;
   Inputs.FS = buildTestFS({{FullFilename, Code}, {FullHeaderName, HeaderCode}});
-  Inputs.ClangTidyOpts = tidy::ClangTidyOptions::getDefaults();
-  Inputs.ClangTidyOpts.Checks = ClangTidyChecks;
+  Inputs.Opts = ParseOptions();
+  Inputs.Opts.ClangTidyOpts.Checks = ClangTidyChecks;
+  Inputs.Index = ExternalIndex;
+  if (Inputs.Index)
+Inputs.Opts.EnableIncludeFixer = true;
   auto PCHs = std::make_shared();
   auto CI = buildCompilerInvocation(Inputs);
   assert(CI && "Failed to build compilation invocation.");
Index: unittests/clangd/TestIndex.h
===
--- unittests/clangd/TestIndex.h
+++ unittests/clangd/TestIndex.h
@@ -18,6 +18,19 @@
 // Creates Symbol instance and sets SymbolID to given QualifiedName.
 Symbol symbol(llvm::StringRef QName);
 
+// Helpers to produce fake index symbols with proper SymbolID.
+// USRFormat is a regex replacement string for the unqualified part of the USR.
+Symbol sym(llvm::StringRef QName, index::SymbolKind Kind,
+   llvm::StringRef USRFormat);
+// Creats a function symbol assuming no function arg.
+Symbol func(llvm::StringRef Name);
+// Creates a class symbol.
+Symbol cls(llvm::StringRef Name);
+// Creates a variable symbol.
+Symbol var(llvm::StringRef Name);
+// Creates a namespace symbol.
+Symbol ns(llvm::StringRef Name);
+
 // Create a slab of symbols with the given qualified names as IDs and names.
 SymbolSlab generateSymbols(std::vector QualifiedNames);
 
Index: unittests/clangd/TestIndex.cpp
===
--- unittests/clangd/TestIndex.cpp
+++ unittests/clangd/TestIndex.cpp
@@ -7,6 +7,8 @@
 //===--===//
 
 #include "TestIndex.h"
+#include "clang/Index/IndexSymbol.h"
+#include "llvm/Support/Regex.h"
 
 namespace clang {
 namespace clangd {
@@ -25,6 +27,58 @@
   return Sym;
 }
 
+static std::string replace(llvm::StringRef Haystack, llvm::StringRef Needle,
+   llvm::StringRef Repl) {
+  std::string Result;
+  llvm::raw_string_ostream OS(Result);
+  std::pair Split;
+  for (Split = Haystack.split(Needle); !Split.second.empty();
+   Split = Split.first.split(Needle))
+OS << Split.first << Repl;
+  Result += Split.first;
+  OS.flush();
+  return Result;
+}
+
+// Helpers to produce fake index symbols for memIndex() or completions().
+// USRFormat is a regex replacement string for the unqualified part of the USR.
+Symbol sym(llvm::StringRef QName, index::SymbolKind Kind,
+   llvm::StringRef USRFormat) {
+  Symbol Sym;
+  std::string USR = "c:"; // We synthesize a few simple cases of USRs by hand!
+  size_t Pos = QName.rfind("::");
+  if (Pos == llvm::StringRef::npos) {
+Sym.Name = QName;
+Sym.Scope = "";
+  } else {
+Sym.Name = QName.substr(Pos + 2);
+Sym.Scope = QName.substr(0, Pos + 2);
+USR += "@N@" + replace(QName.substr(0, Pos), "::", "@N@"); // ns:: -> @N@ns
+  }
+  USR += llvm::Regex("^.*$").sub(USRFormat, Sym.Name); // e.g. func -> @F@func#
+  Sym.ID = SymbolID(USR);
+  Sym.SymInfo.Kind = Kind;
+  Sym.Flags |= Symbol::IndexedForCodeCompletion;
+  Sym.Origin = SymbolOrigin::Static;
+  return Sym;
+}
+
+Symbol func(llvm::StringRef Name) { // Assumes the function has no 

[PATCH] D56903: [clangd] Suggest adding missing includes for incomplete type diagnostics.

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

In D56903#1365487 , @sammccall wrote:

> This looks pretty good! My main concern is latency (and variability of 
> latency) in practice.
> Particularly:
>
> - we should avoid paying for fuzzyfind and fetching hundreds of results when 
> we want exact-match
> - limit the damage in degenerate cases: should we limit to e.g. 5 index 
> queries per TU?
>
>   Actually, now that I think about it - if we can see a forward declaration, 
> can't we do a lookup() instead of a fuzzyFind? Is the problem that this 
> doesn't generalize to the no-declaration case (where it looks like a typo)?


I switched to use lookup requests for incomplete type diagnostics. And yes, the 
typo errors need to use FuzzyFind or lookup by names as no USR is available.

> If we had an operation that looked like lookup() but worked by qname, would 
> we design the feature the same way?
>  In particular, would we want to batch the requests to the index (lookup 
> takes a set of IDs)? This would affect the code structure a bit. But it would 
> make the feature cost basically O(1) instead of O(errs)...

As chatted offline, we decided to leave the batching optimization as a TODO in 
this patch. See inline comment for more detailed response regarding performance.




Comment at: clangd/ClangdUnit.h:91
+IntrusiveRefCntPtr VFS,
+const SymbolIndex *Index);
 

sammccall wrote:
> Index is a reasonable (if surprising) param here, but I think we need to 
> explicitly enable the include-fixing behavior. Even the very minimal 
> hard-coded list of clang-tidy checks caused issues :-( And this is going to 
> result in new network RPCs in some configs.
> 
> I'd suggest keeping the Index param, and wrapping the "use include fixer?" 
> policy along with the clang-tidy options in D55256 as some sort of 
> "diagnostic options" struct. (Though be wary of name clash with the one in 
> Diagnostics.h, sigh).
How about `ParseOptions`?



Comment at: clangd/IncludeFixer.cpp:39
+   const clang::Diagnostic ) const {
+  if (isIncompleteTypeDiag(Info.getID())) {
+// Incomplete type diagnostics should have a QualType argument for the

sammccall wrote:
> can you add a trace to this function so we know what the impact on latency is?
Added a tracer in `fixInCompleteType` to avoid noise from unsupported 
diagnostics.



Comment at: clangd/IncludeFixer.cpp:66
+  vlog("Trying to fix include for incomplete type {0}", IncompleteType);
+  FuzzyFindRequest Req;
+  Req.AnyScope = false;

ilya-biryukov wrote:
> sammccall wrote:
> > limit?
> Why do we use fuzzyFind and not `lookup` here?
> Are there cases when we can't construct `SymbolID` for the `TagDecl`?
Switched to use lookup. The typo diagnostics (i.e. undefined symbol) can't use 
lookup as there is no declaration, but we can definitely use lookup for 
incomplete types in this patch.



Comment at: clangd/IncludeFixer.cpp:74
+  llvm::Optional Matched;
+  Index.fuzzyFind(Req, [&](const Symbol ) {
+// FIXME: support multiple matched symbols.

sammccall wrote:
> so issuing a bunch of fuzzy finds in sequence is clearly not ideal from a 
> performance standpoint.
> Any ideas on what we might do better, or how we can limit the worst case?
> (not sure we need to do any better in this patch, but might be worth comments)
As you suggested, batching requests from all diagnostics would definitely help. 
SymbolIndex already supports batch lookup by IDs, but we would need to extend 
SymbolIndex to support lookup by Names for the typo errors that we are handling 
in D57021. In order to batch index requests from all diagnostics, the existing 
logic around handling and storing diagnostics might also need refactoring. I 
added a TODO for this. We can revisit if the performance turned out to be a big 
problem.

Another thing we could do is adding cache for index results. For example, for 
the code copy-paste case, one incomplete/undefined symbol can cause multiple 
errors. Caching should save us some unnecessary index queries.

For this patch, I added a limit on the number of index requests in IncludeFixer 
(5 for now), which would prevent us from sending too many index requests when 
building AST.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56903/new/

https://reviews.llvm.org/D56903



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

[PATCH] D56916: Fix crash due to ObjCPropertyDecl

[David Goldman via Phabricator via cfe-commits]

dgoldman updated this revision to Diff 182921.
dgoldman marked an inline comment as done.
dgoldman added a comment.

- use auto


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56916/new/

https://reviews.llvm.org/D56916

Files:
  clangd/index/SymbolCollector.cpp


Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -369,19 +369,25 @@
   if (!ID)
 return true;
 
-  const NamedDecl  = *cast(ASTNode.OrigD);
+  // FIXME: ObjCPropertyDecl are not properly indexed here:
+  // - ObjCPropertyDecl may have an OrigD of ObjCPropertyImplDecl, which is
+  // not a NamedDecl.
+  auto *OriginalDecl = dyn_cast(ASTNode.OrigD);
+  if (!OriginalDecl)
+return true;
+
   const Symbol *BasicSymbol = Symbols.find(*ID);
   if (!BasicSymbol) // Regardless of role, ND is the canonical declaration.
 BasicSymbol = addDeclaration(*ND, std::move(*ID));
-  else if (isPreferredDeclaration(OriginalDecl, Roles))
+  else if (isPreferredDeclaration(*OriginalDecl, Roles))
 // If OriginalDecl is preferred, replace the existing canonical
 // declaration (e.g. a class forward declaration). There should be at most
 // one duplicate as we expect to see only one preferred declaration per
 // TU, because in practice they are definitions.
-BasicSymbol = addDeclaration(OriginalDecl, std::move(*ID));
+BasicSymbol = addDeclaration(*OriginalDecl, std::move(*ID));
 
   if (Roles & static_cast(index::SymbolRole::Definition))
-addDefinition(OriginalDecl, *BasicSymbol);
+addDefinition(*OriginalDecl, *BasicSymbol);
   return true;
 }
 


Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -369,19 +369,25 @@
   if (!ID)
 return true;
 
-  const NamedDecl  = *cast(ASTNode.OrigD);
+  // FIXME: ObjCPropertyDecl are not properly indexed here:
+  // - ObjCPropertyDecl may have an OrigD of ObjCPropertyImplDecl, which is
+  // not a NamedDecl.
+  auto *OriginalDecl = dyn_cast(ASTNode.OrigD);
+  if (!OriginalDecl)
+return true;
+
   const Symbol *BasicSymbol = Symbols.find(*ID);
   if (!BasicSymbol) // Regardless of role, ND is the canonical declaration.
 BasicSymbol = addDeclaration(*ND, std::move(*ID));
-  else if (isPreferredDeclaration(OriginalDecl, Roles))
+  else if (isPreferredDeclaration(*OriginalDecl, Roles))
 // If OriginalDecl is preferred, replace the existing canonical
 // declaration (e.g. a class forward declaration). There should be at most
 // one duplicate as we expect to see only one preferred declaration per
 // TU, because in practice they are definitions.
-BasicSymbol = addDeclaration(OriginalDecl, std::move(*ID));
+BasicSymbol = addDeclaration(*OriginalDecl, std::move(*ID));
 
   if (Roles & static_cast(index::SymbolRole::Definition))
-addDefinition(OriginalDecl, *BasicSymbol);
+addDefinition(*OriginalDecl, *BasicSymbol);
   return true;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56916: Fix crash due to ObjCPropertyDecl

[David Goldman via Phabricator via cfe-commits]

dgoldman added a comment.

In D56916#1365357 , @ilya-biryukov 
wrote:

> Good point, @aaron.ballman! @dgoldman, could you please add a test case?


Unfortunately I can't get ninja check-clangd to build:

  
/Users/davg/dev/llvm/source/llvm/tools/clang/tools/extra/clang-tidy/bugprone/ParentVirtualCallCheck.cpp:54:27:
 error: no matching member function for call to 'getThisType'
  ActualMemberDecl->getThisType(ActualMemberDecl->getASTContext())
  ~~^~~
  
/Users/davg/dev/llvm/source/llvm/tools/clang/include/clang/AST/DeclCXX.h:2182:12:
 note: candidate function not viable: requires 0 arguments, but 1 was provided
QualType getThisType() const;
 ^
  
/Users/davg/dev/llvm/source/llvm/tools/clang/include/clang/AST/DeclCXX.h:2184:19:
 note: candidate function not viable: requires 2 arguments, but 1 was provided
static QualType getThisType(const FunctionProtoType *FPT,
^
  1 error generated.
  [1230/1512] Building CXX object 
tools/clang/tools/extra/clang-tidy/cert/CMakeFiles/clangTidyCERTModule.dir/CERTTidyModule.cpp.o
  ninja: build stopped: subcommand failed.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56916/new/

https://reviews.llvm.org/D56916



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

[PATCH] D57055: [RISCV] Mark TLS as supported

[Lewis Revill via Phabricator via cfe-commits]

lewis-revill updated this revision to Diff 182920.
lewis-revill added a comment.

Rely on default value rather than explicitly marking `TLSSupported` as true.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57055/new/

https://reviews.llvm.org/D57055

Files:
  lib/Basic/Targets/RISCV.h


Index: lib/Basic/Targets/RISCV.h
===
--- lib/Basic/Targets/RISCV.h
+++ lib/Basic/Targets/RISCV.h
@@ -36,7 +36,6 @@
   RISCVTargetInfo(const llvm::Triple , const TargetOptions &)
   : TargetInfo(Triple), HasM(false), HasA(false), HasF(false),
 HasD(false), HasC(false) {
-TLSSupported = false;
 LongDoubleWidth = 128;
 LongDoubleAlign = 128;
 LongDoubleFormat = ::APFloat::IEEEquad();


Index: lib/Basic/Targets/RISCV.h
===
--- lib/Basic/Targets/RISCV.h
+++ lib/Basic/Targets/RISCV.h
@@ -36,7 +36,6 @@
   RISCVTargetInfo(const llvm::Triple , const TargetOptions &)
   : TargetInfo(Triple), HasM(false), HasA(false), HasF(false),
 HasD(false), HasC(false) {
-TLSSupported = false;
 LongDoubleWidth = 128;
 LongDoubleAlign = 128;
 LongDoubleFormat = ::APFloat::IEEEquad();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56850: [ASTMatchers][NFC] Add tests for assorted `CXXMemberCallExpr` matchers.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 182919.
ymandel marked an inline comment as done.
ymandel added a comment.

Remove unnecessary expectation.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56850/new/

https://reviews.llvm.org/D56850

Files:
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -470,6 +470,96 @@
 
 }
 
+TEST(MatcherCXXMemberCallExpr, On) {
+  auto Snippet1 = R"cc(
+struct Y {
+  void m();
+};
+void z(Y y) { y.m(); }
+  )cc";
+  auto Snippet2 = R"cc(
+struct Y {
+  void m();
+};
+struct X : public Y {};
+void z(X x) { x.m(); }
+  )cc";
+  auto MatchesY = cxxMemberCallExpr(on(hasType(cxxRecordDecl(hasName("Y");
+  EXPECT_TRUE(matches(Snippet1, MatchesY));
+  EXPECT_TRUE(notMatches(Snippet2, MatchesY));
+
+  auto MatchesX = cxxMemberCallExpr(on(hasType(cxxRecordDecl(hasName("X");
+  EXPECT_TRUE(matches(Snippet2, MatchesX));
+
+  // Parens are ignored.
+  auto MatchesCall = cxxMemberCallExpr(on(callExpr()));
+  EXPECT_TRUE(matches(
+  R"cc(
+struct Y {
+  void m();
+};
+Y g();
+void z(Y y) { (g()).m(); }
+  )cc",
+  MatchesCall));
+}
+
+TEST(MatcherCXXMemberCallExpr, OnImplicitObjectArgument) {
+  auto Snippet1 = R"cc(
+struct Y {
+  void m();
+};
+void z(Y y) { y.m(); }
+  )cc";
+  auto Snippet2 = R"cc(
+struct Y {
+  void m();
+};
+struct X : public Y {};
+void z(X x) { x.m(); }
+  )cc";
+  auto MatchesY = cxxMemberCallExpr(
+  onImplicitObjectArgument(hasType(cxxRecordDecl(hasName("Y");
+  EXPECT_TRUE(matches(Snippet1, MatchesY));
+  EXPECT_TRUE(matches(Snippet2, MatchesY));
+
+  auto MatchesX = cxxMemberCallExpr(
+  onImplicitObjectArgument(hasType(cxxRecordDecl(hasName("X");
+  EXPECT_TRUE(notMatches(Snippet2, MatchesX));
+
+  // Parens are not ignored.
+  auto MatchesCall = cxxMemberCallExpr(onImplicitObjectArgument(callExpr()));
+  EXPECT_TRUE(notMatches(
+  R"cc(
+struct Y {
+  void m();
+};
+Y g();
+void z(Y y) { (g()).m(); }
+  )cc",
+  MatchesCall));
+}
+
+TEST(Matcher, HasObjectExpr) {
+  auto M = memberExpr(hasObjectExpression(hasType(cxxRecordDecl(hasName("X");
+  EXPECT_TRUE(matches(
+  R"cc(
+struct X {
+  int m;
+  int f(X x) { return x.m; }
+};
+  )cc",
+  M));
+  EXPECT_TRUE(notMatches(
+  R"cc(
+struct X {
+  int m;
+  int f(X x) { return m; }
+};
+  )cc",
+  M));
+}
+
 TEST(ForEachArgumentWithParam, ReportsNoFalsePositives) {
   StatementMatcher ArgumentY =
 declRefExpr(to(varDecl(hasName("y".bind("arg");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56849: [ASTMatchers][NFC] Update comments on assorted `CXXMemberCallExpr` matchers.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 182918.
ymandel marked an inline comment as done.
ymandel added a comment.

Use backticks instead of quotes for quoted code.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56849/new/

https://reviews.llvm.org/D56849

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h

Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -2887,14 +2887,22 @@
  InnerMatcher.matches(*UnderlyingDecl, Finder, Builder);
 }
 
-/// Matches on the implicit object argument of a member call expression.
+/// Matches on the implicit object argument of a member call expression, after
+/// stripping off any parentheses or implicit casts.
 ///
-/// Example matches y.x()
-///   (matcher = cxxMemberCallExpr(on(hasType(cxxRecordDecl(hasName("Y"))
+/// Given
 /// \code
-///   class Y { public: void x(); };
-///   void z() { Y y; y.x(); }
+///   class Y { public: void m(); };
+///   Y g();
+///   class X : public Y {};
+///   void z(Y y, X x) { y.m(); (g()).m(); x.m(); }
 /// \endcode
+/// cxxMemberCallExpr(on(hasType(cxxRecordDecl(hasName("Y")
+///   matches `y.m()` and `(g()).m()`.
+/// cxxMemberCallExpr(on(hasType(cxxRecordDecl(hasName("X")
+///   matches `x.m()`.
+/// cxxMemberCallExpr(on(callExpr()))
+///   matches `(g()).m()`.
 ///
 /// FIXME: Overload to allow directly matching types?
 AST_MATCHER_P(CXXMemberCallExpr, on, internal::Matcher,
@@ -3254,6 +3262,23 @@
   .matches(Node, Finder, Builder);
 }
 
+/// Matches on the implicit object argument of a member call expression. Unlike
+/// `on`, matches the argument directly without stripping away anything.
+///
+/// Given
+/// \code
+///   class Y { public: void m(); };
+///   Y g();
+///   class X : public Y { void g(); };
+///   void z(Y y, X x) { y.m(); x.m(); x.g(); (g()).m(); }
+/// \endcode
+/// cxxMemberCallExpr(onImplicitObjectArgument(hasType(
+/// cxxRecordDecl(hasName("Y")
+///   matches `y.m()`, `x.m()` and (g()).m(), but not `x.g()`.
+/// cxxMemberCallExpr(on(callExpr()))
+///   does not match `(g()).m()`, because the parens are not ignored.
+///
+/// FIXME: Overload to allow directly matching types?
 AST_MATCHER_P(CXXMemberCallExpr, onImplicitObjectArgument,
   internal::Matcher, InnerMatcher) {
   const Expr *ExprNode = Node.getImplicitObjectArgument();
@@ -3261,8 +3286,22 @@
   InnerMatcher.matches(*ExprNode, Finder, Builder));
 }
 
-/// Matches if the expression's type either matches the specified
-/// matcher, or is a pointer to a type that matches the InnerMatcher.
+/// Matches if the type of the expression's implicit object argument either
+/// matches the InnerMatcher, or is a pointer to a type that matches the
+/// InnerMatcher.
+///
+/// Given
+/// \code
+///   class Y { public: void m(); };
+///   class X : public Y { void g(); };
+///   void z() { Y y; y.m(); Y *p; p->m(); X x; x.m(); x.g(); }
+/// \endcode
+/// cxxMemberCallExpr(thisPointerType(hasDeclaration(
+/// cxxRecordDecl(hasName("Y")
+///   matches `y.m()`, `p->m()` and `x.m()`.
+/// cxxMemberCallExpr(thisPointerType(hasDeclaration(
+/// cxxRecordDecl(hasName("X")
+///   matches `x.g()`.
 AST_MATCHER_P_OVERLOAD(CXXMemberCallExpr, thisPointerType,
internal::Matcher, InnerMatcher, 0) {
   return onImplicitObjectArgument(
@@ -4964,18 +5003,22 @@
   return InnerMatcher.matches(*Node.getMemberDecl(), Finder, Builder);
 }
 
-/// Matches a member expression where the object expression is
-/// matched by a given matcher.
+/// Matches a member expression where the object expression is matched by a
+/// given matcher. Implicit object expressions are included; that is, it matches
+/// use of implicit `this`.
 ///
 /// Given
 /// \code
-///   struct X { int m; };
-///   void f(X x) { x.m; m; }
+///   struct X {
+/// int m;
+/// int f(X x) { x.m; return m; }
+///   };
 /// \endcode
-/// memberExpr(hasObjectExpression(hasType(cxxRecordDecl(hasName("X")))
-///   matches "x.m" and "m"
-/// with hasObjectExpression(...)
-///   matching "x" and the implicit object expression of "m" which has type X*.
+/// memberExpr(hasObjectExpression(hasType(cxxRecordDecl(hasName("X")
+///   matches `x.m`, but not `m`; however,
+/// memberExpr(hasObjectExpression(hasType(pointsTo(
+//  cxxRecordDecl(hasName("X"))
+///   matches `m` (aka. `this->m`), but not `x.m`.
 AST_POLYMORPHIC_MATCHER_P(
 hasObjectExpression,
 AST_POLYMORPHIC_SUPPORTED_TYPES(MemberExpr, UnresolvedMemberExpr,
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -4810,16 +4810,20 @@
 
 
 

[PATCH] D57057: [clangd] Log clang-tidy configuration, NFC

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric, 
ilya-biryukov.

This is used for debugging purpose.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D57057

Files:
  clangd/ClangdUnit.cpp


Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -265,6 +265,8 @@
   llvm::Optional CTContext;
   {
 trace::Span Tracer("ClangTidyInit");
+vlog("ClangTidy configuration for file {0}: {1}", MainInput.getFile(),
+ tidy::configurationAsText(ClangTidyOpts));
 tidy::ClangTidyCheckFactories CTFactories;
 for (const auto  : tidy::ClangTidyModuleRegistry::entries())
   E.instantiate()->addCheckFactories(CTFactories);


Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -265,6 +265,8 @@
   llvm::Optional CTContext;
   {
 trace::Span Tracer("ClangTidyInit");
+vlog("ClangTidy configuration for file {0}: {1}", MainInput.getFile(),
+ tidy::configurationAsText(ClangTidyOpts));
 tidy::ClangTidyCheckFactories CTFactories;
 for (const auto  : tidy::ClangTidyModuleRegistry::entries())
   E.instantiate()->addCheckFactories(CTFactories);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57055: [RISCV] Mark TLS as supported

[James Clarke via Phabricator via cfe-commits]

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



Comment at: lib/Basic/Targets/RISCV.h:39
 HasD(false), HasC(false) {
-TLSSupported = false;
+TLSSupported = true;
 LongDoubleWidth = 128;

With the exception of `SystemZTargetInfo`, all CPU target info classes rely on 
the base already initialising it to `true` by default, so you can just delete 
this line.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57055/new/

https://reviews.llvm.org/D57055



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

[PATCH] D57055: [RISCV] Mark TLS as supported

[Lewis Revill via Phabricator via cfe-commits]

lewis-revill created this revision.
lewis-revill added a reviewer: asb.
Herald added subscribers: cfe-commits, jocewei, PkmX, rkruppe, the_o, 
brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, shiva0217, 
kito-cheng, niosHD, sabuasal, apazos, simoncook, johnrusso, rbar.

Inform Clang that TLS is implemented by LLVM for RISC-V


Repository:
  rC Clang

https://reviews.llvm.org/D57055

Files:
  lib/Basic/Targets/RISCV.h


Index: lib/Basic/Targets/RISCV.h
===
--- lib/Basic/Targets/RISCV.h
+++ lib/Basic/Targets/RISCV.h
@@ -36,7 +36,7 @@
   RISCVTargetInfo(const llvm::Triple , const TargetOptions &)
   : TargetInfo(Triple), HasM(false), HasA(false), HasF(false),
 HasD(false), HasC(false) {
-TLSSupported = false;
+TLSSupported = true;
 LongDoubleWidth = 128;
 LongDoubleAlign = 128;
 LongDoubleFormat = ::APFloat::IEEEquad();


Index: lib/Basic/Targets/RISCV.h
===
--- lib/Basic/Targets/RISCV.h
+++ lib/Basic/Targets/RISCV.h
@@ -36,7 +36,7 @@
   RISCVTargetInfo(const llvm::Triple , const TargetOptions &)
   : TargetInfo(Triple), HasM(false), HasA(false), HasF(false),
 HasD(false), HasC(false) {
-TLSSupported = false;
+TLSSupported = true;
 LongDoubleWidth = 128;
 LongDoubleAlign = 128;
 LongDoubleFormat = ::APFloat::IEEEquad();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56539: [clangd] Drop documentation in static index if symbols are not indexed for completion.

[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.

Just realized I missed this review, sorry about that.
LGTM with a nit.




Comment at: clangd/index/FileIndex.cpp:51
 CollectorOpts.RefFilter = RefKind::All;
-  }else {
+CollectorOpts.StoreAllDocumentation = false;
+  } else {

NIT: maybe mention the rationale here in a comment? Something like `// comments 
for main file can always be obtained from sema, do not store them in the index.`


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56539/new/

https://reviews.llvm.org/D56539



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

[PATCH] D56540: [clangd] Clean the cache of file statuses on vscode-clangd when clangd crashes.

[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.

LGTM




Comment at: clangd/clients/clangd-vscode/src/extension.ts:127
+(fileStatus) => { status.onFileUpdated(fileStatus); });
+} else if (newState == vscodelc.State.Stopped) {
+// Clear all cached statuses when clangd crashes.

This looks scary, if we end up doing that in too many places, the code would 
become completely unreadable.

Have no idea how to make it better, but a very general observation is that we'd 
be better off with dropping all the state we have when clangd crashes and 
starting from scratch, rather than trying to keep old components and bring them 
into a sensible state when crashes happen.

No need to do anything right now, just something to keep in mind for the future.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56540/new/

https://reviews.llvm.org/D56540



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

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked an inline comment as done.
ilya-biryukov added inline comments.



Comment at: clangd/refactor/tweaks/SwapIfBranches.cpp:34
+/// After:
+///   if (foo) { continue; } else { return 10; }
+class SwapIfBranches : public Tweak {

sammccall wrote:
> The before/after is useful, we should probably have it for all tweaks if 
> possible.
> It'd also be useful to notate where the cursor can be to trigger the action. 
> Partly because it forces us to consider this!
> 
> e.g. (not sure if this actually matches the logic you want, just an example)
> ```
> Before:
>   if (foo) { return 10; } else { continue; }
>   ^^   ^
> After:
>   ...
> ```
LG, updated the comment and ranges per suggestion.
I still not sure if the action should span the whole condition, but going with 
the suggested ranges anyway. We can tweak them later if the resulting ranges 
turn out to be problematic.

However, note that I excluded `{` and `}` from available ranges, but only 
because I'm lazy and this requires declaring some more local vars, etc.



Comment at: clangd/refactor/tweaks/SwapIfBranches.cpp:46
+private:
+  tooling::Replacements Result;
+};

sammccall wrote:
> I think prepare() should just verify:
>  - cursor is in the right place
>  - else statement exists and isn't an else if
>  - both branches are compound statements (for now)
>  - (maybe) relevant locations (if keyword, else keyword, braces) are not 
> macro expansions
> and then record the relevant source locations (or just the IfStmt*)
> 
> We may be able to get away with doing all the work in `prepare()`, but it's 
> not a good model for future tweaks. (And it is at least somewhat wasteful on 
> a hot path).
Done.
The only reason I thought applying replacements was a sensible idea is because 
it would fail in some weird macro cases (only when resulting ranges overlap, 
though). And it seemed better to avoid showing the action rather than showing 
it and failing later.



Comment at: clangd/refactor/tweaks/SwapIfBranches.cpp:50
+
+class FindIfUnderCursor : public RecursiveASTVisitor {
+public:

sammccall wrote:
> (Mostly this is food for thought for later - we shouldn't try to solve 
> everything in this patch)
> 
> Two efficiency problems here:
>  - doing one traversal per tweak is wasteful (when we have >1 tweaks, but 
> worth at least *thinking* about)
>  - traversing the entire AST rather than just the nodes over the cursor is 
> bad news (though "over the cursor" may not be trivial to define)
> 
> Idea for how to "framework" this problem away:
> Add `vector SelectedNodes` to the inputs, the idea being that 
> this is the stack from the narrowest `Expr` under the cursor to the 
> `TranslationUnitDecl` at the top.
> This can be produced by a RecursiveASTVisitor (that either traverses the 
> whole AST, or just the bits under the cursor if that's simple enough).
> Tweaks would iterate over the nodes from narrowest to broadest, deciding 
> whether to select this node for processing, continue iterating, or bail out.
> 
> Matching in checks are then pretty easy to write, we haven't removed too much 
> flexibility in flow control, and it's pretty hard to write a slow check.
> 
> There are some complications:
>  - we need access to parents somehow (e.g. consider the case of adding NS 
> qualifiers, we want to match on names but then traverse up to the containing 
> declrefexpr to get the nested namespace loc)
>  - the interesting nodes may be a tree rather than a stack, because nodes 
> overlap in the source. We could store a postorder traversal... but this makes 
> e.g. "bail out when you hit a functiondecl" harder - you probably want to 
> bail out of the current branch only.
>  - things get complicated when we think about macros - depending on the 
> semantics we want, it may be hard for the framework to prune parts of the 
> tree that aren't under the cursor withouth traversing the whole AST.
I mostly agree, however I still wonder whether that would be inefficient in 
some cases. E.g. consider a corner if the input selection from LSP contains the 
whole file? I see two options there: (1) putting all nodes of a file into 
`vector`, (2) putting only the top-level `TranslationUnitDecl` 
there.

It seems easy to end up with (1) to make the interface useful, however we 
probably prefer (2) because otherwise we're back into the worst-case scenario, 
i.e. "every  tweak traverses all the nodes"

> we need access to parents somehow (e.g. consider the case of adding NS 
> qualifiers, we want to match on names but then traverse up to the containing 
> declrefexpr to get the nested namespace loc)

I wonder if it's possible to instead write the checks so that they only look at 
the children of the nodes in a vector? My bet is that almost all of the checks 
only need to look one or two levels down, so this shouldn't turn into 
inefficiency.

[PATCH] D56849: [ASTMatchers][NFC] Update comments on assorted `CXXMemberCallExpr` matchers.

[Alexander Kornienko via Phabricator via cfe-commits]

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

Thanks for tidying up the docs. LG with one nit.




Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:5018
+/// memberExpr(hasObjectExpression(hasType(cxxRecordDecl(hasName("X")
+///   matches "x.m", but not "m"; however,
+/// memberExpr(hasObjectExpression(hasType(pointsTo(

nit: Let's use backquotes here instead of double quotes. Same three lines below.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56849/new/

https://reviews.llvm.org/D56849



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

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

[Ilya Biryukov via Phabricator via cfe-commits]

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

- Fix a typo in a comment: isValidRange -> isValidFileRange
- Make action available under 'else' keywords and conditions
- Move the logic of creating replacements to apply
- Use llvm::isa instead of dyn_cast_or_null


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56611/new/

https://reviews.llvm.org/D56611

Files:
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt
  clang-tools-extra/clangd/refactor/tweaks/SwapIfBranches.cpp
  clang-tools-extra/unittests/clangd/CMakeLists.txt
  clang-tools-extra/unittests/clangd/TweakTests.cpp

Index: clang-tools-extra/unittests/clangd/TweakTests.cpp
===
--- /dev/null
+++ clang-tools-extra/unittests/clangd/TweakTests.cpp
@@ -0,0 +1,145 @@
+//===-- TweakTests.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 "SourceCode.h"
+#include "TestTU.h"
+#include "refactor/Tweak.h"
+#include "clang/AST/Expr.h"
+#include "clang/Rewrite/Core/Rewriter.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+
+using llvm::Failed;
+using llvm::HasValue;
+using llvm::Succeeded;
+using ::testing::IsEmpty;
+using ::testing::Not;
+
+namespace clang {
+namespace clangd {
+namespace {
+
+std::string markRange(llvm::StringRef Code, Range R) {
+  size_t Begin = llvm::cantFail(positionToOffset(Code, R.start));
+  size_t End = llvm::cantFail(positionToOffset(Code, R.end));
+  assert(Begin <= End);
+  if (Begin == End) // Mark a single point.
+return (Code.substr(0, Begin) + "^" + Code.substr(Begin)).str();
+  // Mark a range.
+  return (Code.substr(0, Begin) + "[[" + Code.substr(Begin, End - Begin) +
+  "]]" + Code.substr(End))
+  .str();
+}
+
+class TweakTest : public ::testing::Test {
+public:
+  /// Checks action is available at every point and range marked in \p Input.
+  void checkAvailable(TweakID ID, llvm::StringRef Input) {
+return checkAvailable(ID, Input, /*Available=*/true);
+  }
+
+  /// Same as checkAvailable, but checks the action is not available.
+  void checkNotAvailable(TweakID ID, llvm::StringRef Input) {
+return checkAvailable(ID, Input, /*Available=*/false);
+  }
+
+  llvm::Expected apply(TweakID ID, llvm::StringRef Input) {
+Annotations Code(Input);
+Range SelectionRng;
+if (Code.points().size() != 0) {
+  assert(Code.ranges().size() == 0 &&
+ "both a cursor point and a selection range were specified");
+  SelectionRng = Range{Code.point(), Code.point()};
+} else {
+  SelectionRng = Code.range();
+}
+TestTU TU;
+TU.Filename = "foo.cpp";
+TU.Code = Code.code();
+
+ParsedAST AST = TU.build();
+auto CursorLoc = llvm::cantFail(sourceLocationInMainFile(
+AST.getASTContext().getSourceManager(), SelectionRng.start));
+Tweak::Selection S = {Code.code(), AST, CursorLoc};
+
+auto T = prepareTweak(ID, S);
+if (!T)
+  return T.takeError();
+auto Replacements = (*T)->apply(S);
+if (!Replacements)
+  return Replacements.takeError();
+return applyAllReplacements(Code.code(), *Replacements);
+  }
+
+private:
+  void checkAvailable(TweakID ID, llvm::StringRef Input, bool Available) {
+Annotations Code(Input);
+ASSERT_TRUE(0 < Code.points().size() || 0 < Code.ranges().size())
+<< "no points of interest specified";
+TestTU TU;
+TU.Filename = "foo.cpp";
+TU.Code = Code.code();
+
+ParsedAST AST = TU.build();
+
+auto CheckOver = [&](Range Selection) {
+  auto CursorLoc = llvm::cantFail(sourceLocationInMainFile(
+  AST.getASTContext().getSourceManager(), Selection.start));
+  auto T = prepareTweak(ID, Tweak::Selection{Code.code(), AST, CursorLoc});
+  if (Available)
+EXPECT_THAT_EXPECTED(T, Succeeded())
+<< "code is " << markRange(Code.code(), Selection);
+  else
+EXPECT_THAT_EXPECTED(T, Failed())
+<< "code is " << markRange(Code.code(), Selection);
+};
+for (auto P : Code.points())
+  CheckOver(Range{P, P});
+for (auto R : Code.ranges())
+  CheckOver(R);
+  }
+};
+
+TEST_F(TweakTest, SwapIfBranches) {
+  llvm::StringLiteral ID = "SwapIfBranches";
+
+  checkAvailable(ID, R"cpp(
+void test() {
+  ^i^f^^(^t^r^u^e^) { return 100; } ^e^l^s^e^ { continue; }
+}
+  )cpp");
+
+  

[PATCH] D56984: [libunwind] Silence warnings about unused parameters

[Louis Dionne via Phabricator via cfe-commits]

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

LGTM, but would it make sense to have a macro like 
`_LIBUNWIND_MAYBE_UNUSED(var)` instead?


Repository:
  rUNW libunwind

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56984/new/

https://reviews.llvm.org/D56984



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

[PATCH] D57047: [clangd] Fix broken build after r351793.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

I think it is safe to use the empty one (constructed by the default 
constructor) here -- since `getDefaults` is not free, and we don't need the 
configurations in codeComplete, fixed in rL351826 
.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57047/new/

https://reviews.llvm.org/D57047



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

[clang-tools-extra] r351826 - [clangd] Followup fix of rL351818

[Haojian Wu via cfe-commits]

Author: hokein
Date: Tue Jan 22 06:48:04 2019
New Revision: 351826

URL: http://llvm.org/viewvc/llvm-project?rev=351826=rev
Log:
[clangd] Followup fix of rL351818

ClangTidyOptions::getDefaults is not free, it will initialize all
clang-tidy modules to get check-specific options, and we don't use this
information in CodeComplete, so using an empty one (constructed by
default constructor) is sufficient.

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

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=351826=351825=351826=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Tue Jan 22 06:48:04 2019
@@ -1019,9 +1019,11 @@ bool semaCodeComplete(std::unique_ptr VFS = Input.VFS;
   if (Input.Preamble && Input.Preamble->StatCache)
 VFS = Input.Preamble->StatCache->getConsumingFS(std::move(VFS));
-  auto CI = buildCompilerInvocation(
-  ParseInputs{Input.Command, VFS, Input.Contents,
-  tidy::ClangTidyOptions::getDefaults()});
+  ParseInputs PInput;
+  PInput.CompileCommand = Input.Command;
+  PInput.FS = VFS;
+  PInput.Contents = Input.Contents;
+  auto CI = buildCompilerInvocation(PInput);
   if (!CI) {
 elog("Couldn't create CompilerInvocation");
 return false;


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

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D55850#1366094 , @ebevhan wrote:

> Okay, sounds good! I'm not a C++ expert, but I'd be happy to look at the 
> patches when they're up. Haven't done much serious testing on my end so far, 
> but from what I've seen the patches so far look good!


Cool! Thanks a lot for your feedback btw. Let me know if you discover any 
issues/bugs! I do need to improve testing at some point soon as I am sure there 
are still a lot of uncaught corner cases.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55850/new/

https://reviews.llvm.org/D55850



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

[PATCH] D57047: [clangd] Fix broken build after r351793.

[Eric Liu via Phabricator via cfe-commits]

ioeric abandoned this revision.
ioeric added a comment.

nvm, Simon beat me to it ;)


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57047/new/

https://reviews.llvm.org/D57047



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

[PATCH] D57047: [clangd] Fix broken build after r351793.

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

In D57047#1366265 , @kadircet wrote:

> LGTM, but hokein might know better whether to push the default options or the 
> one in ClangdServerOpts


I'm going to land this now to unbreak build. Feel free to comment or do further 
refactoring if the default is not the best thing to pass here. Currently, this 
doesn't seem to matter as it's not used in CodeCompletion.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57047/new/

https://reviews.llvm.org/D57047



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

[PATCH] D57047: [clangd] Fix broken build after r351793.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

LGTM, but hokein might know better whether to push the default options or the 
one in ClangdServerOpts


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57047/new/

https://reviews.llvm.org/D57047



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