Re: [PATCH] D23160: [Coverage] Prevent creating a redundant counter if a nested body ends with a macro.

[Igor Kudrin via cfe-commits]

ikudrin added a comment.

In https://reviews.llvm.org/D23160#506061, @vsk wrote:

> I guess it never makes sense to have two regions with the exact same 
> start/end loc, and different counters. Do you think we should add assertions 
> in llvm (either in llvm-cov, or in the coverage reader) which guard against 
> this?


Right now we can have several ranges with the same start and end locations in 
case of macro fully expanded into another macro. We consider this situation 
possible and handle it in class `SegmentBuilder` in 
llvm/lib/ProfileData/Coverage/CoverageMapping.cpp. Maybe something has to be 
fixed here too.

By the way, thinking about your words, I found another example which results in 
spawning not only redundant but also wrong counters. I'm postponing landing 
this patch until I finish the investigation.

  4|1|void dummy() {}
   |2|
  4|3|#define M_INT dummy()
   |4|
 11|5|#define MACRO M_INT
   |6|
   |7|int main()
  1|8|{
  1|9|  int i = 0;
 11|   10|  while (i++ < 10)
 10|   11|if (i < 5)
 10|   12|  MACRO;
  --
  |  | 11|5|#define MACRO M_INT
  |  |  --
  |  |  |  |  4|3|#define M_INT dummy()
  |  |  --
  --
  1|   13|  return 0;
  1|   14|}


https://reviews.llvm.org/D23160



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

r277797 - Fix crash in template type diffing.

[Richard Trieu via cfe-commits]

Author: rtrieu
Date: Thu Aug  4 22:16:36 2016
New Revision: 277797

URL: http://llvm.org/viewvc/llvm-project?rev=277797=rev
Log:
Fix crash in template type diffing.

When the type being diffed is a type alias, and the orginal type is not a
templated type, then there will be no unsugared TemplateSpecializationType.
When this happens, exit early from the constructor.  Also add assertions to
the other iterator accessor to prevent the iterator from being used.

Modified:
cfe/trunk/lib/AST/ASTDiagnostic.cpp
cfe/trunk/test/Misc/diag-template-diffing.cpp

Modified: cfe/trunk/lib/AST/ASTDiagnostic.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDiagnostic.cpp?rev=277797=277796=277797=diff
==
--- cfe/trunk/lib/AST/ASTDiagnostic.cpp (original)
+++ cfe/trunk/lib/AST/ASTDiagnostic.cpp Thu Aug  4 22:16:36 2016
@@ -916,6 +916,8 @@ class TemplateDiff {
   /// template argument.
   InternalIterator(const TemplateSpecializationType *TST)
   : TST(TST), Index(0), CurrentTA(nullptr), EndTA(nullptr) {
+if (!TST) return;
+
 if (isEnd()) return;
 
 // Set to first template argument.  If not a parameter pack, done.
@@ -936,11 +938,13 @@ class TemplateDiff {
 
   /// isEnd - Returns true if the iterator is one past the end.
   bool isEnd() const {
+assert(TST && "InternalalIterator is invalid with a null TST.");
 return Index >= TST->getNumArgs();
   }
 
   /// ++ - Increment the iterator to the next template argument.
   InternalIterator ++() {
+assert(TST && "InternalalIterator is invalid with a null TST.");
 if (isEnd()) {
   return *this;
 }
@@ -976,6 +980,7 @@ class TemplateDiff {
 
   /// operator* - Returns the appropriate TemplateArgument.
   reference operator*() const {
+assert(TST && "InternalalIterator is invalid with a null TST.");
 assert(!isEnd() && "Index exceeds number of arguments.");
 if (CurrentTA == EndTA)
   return TST->getArg(Index);
@@ -985,6 +990,7 @@ class TemplateDiff {
 
   /// operator-> - Allow access to the underlying TemplateArgument.
   pointer operator->() const {
+assert(TST && "InternalalIterator is invalid with a null TST.");
 return *();
   }
 };

Modified: cfe/trunk/test/Misc/diag-template-diffing.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/diag-template-diffing.cpp?rev=277797=277796=277797=diff
==
--- cfe/trunk/test/Misc/diag-template-diffing.cpp (original)
+++ cfe/trunk/test/Misc/diag-template-diffing.cpp Thu Aug  4 22:16:36 2016
@@ -1455,7 +1455,37 @@ void run() {
 }
 // CHECK-ELIDE-NOTREE: error: no matching function for call to 'D'
 // CHECK-ELIDE-NOTREE: note: candidate function [with x = TypeAlias::X::X1] 
not viable: no known conversion from 'VectorType' to 'const 
VectorType<(TypeAlias::X)0>' for 1st argument
+}
+
+namespace TypeAlias2 {
+template 
+class A {};
+
+template 
+using A_reg = A;
+void take_reg(A_reg);
+
+template 
+using A_ptr = A *;
+void take_ptr(A_ptr);
 
+template 
+using A_ref = const A &;
+void take_ref(A_ref);
+
+void run(A_reg reg, A_ptr ptr, A_ref ref) {
+  take_reg(reg);
+// CHECK-ELIDE-NOTREE: error: no matching function for call to 'take_reg'
+// CHECK-ELIDE-NOTREE: note: candidate function not viable: no known 
conversion from 'A_reg' to 'A_reg' for 1st argument
+
+  take_ptr(ptr);
+// CHECK-ELIDE-NOTREE: error: no matching function for call to 'take_ptr'
+// CHECK-ELIDE-NOTREE: note: candidate function not viable: no known 
conversion from 'A_ptr' to 'A_ptr' for 1st argument
+
+  take_ref(ref);
+// CHECK-ELIDE-NOTREE: error: no matching function for call to 'take_ref'
+// CHECK-ELIDE-NOTREE: note: candidate function not viable: no known 
conversion from 'const A' to 'const A' for 1st argument
+}
 }
 
 // CHECK-ELIDE-NOTREE: {{[0-9]*}} errors generated.


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

r277796 - Allow -1 to assign max value to unsigned bitfields.

[Richard Trieu via cfe-commits]

Author: rtrieu
Date: Thu Aug  4 21:39:30 2016
New Revision: 277796

URL: http://llvm.org/viewvc/llvm-project?rev=277796=rev
Log:
Allow -1 to assign max value to unsigned bitfields.

Silence the -Wbitfield-constant-conversion warning for when -1 or other
negative values are assigned to unsigned bitfields, provided that the bitfield
is wider than the minimum number of bits needed to encode the negative value.

Modified:
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/Sema/bitfield.c
cfe/trunk/test/Sema/constant-conversion.c

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=277796=277795=277796=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Aug  4 21:39:30 2016
@@ -7827,6 +7827,12 @@ bool AnalyzeBitFieldAssignment(Sema ,
   unsigned OriginalWidth = Value.getBitWidth();
   unsigned FieldWidth = Bitfield->getBitWidthValue(S.Context);
 
+  if (Value.isSigned() && Value.isNegative())
+if (UnaryOperator *UO = dyn_cast(OriginalInit))
+  if (UO->getOpcode() == UO_Minus)
+if (isa(UO->getSubExpr()))
+  OriginalWidth = Value.getMinSignedBits();
+
   if (OriginalWidth <= FieldWidth)
 return false;
 

Modified: cfe/trunk/test/Sema/bitfield.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/bitfield.c?rev=277796=277795=277796=diff
==
--- cfe/trunk/test/Sema/bitfield.c (original)
+++ cfe/trunk/test/Sema/bitfield.c Thu Aug  4 21:39:30 2016
@@ -64,7 +64,7 @@ typedef signed Signed;
 
 struct Test5 { unsigned n : 2; } t5;
 // Bitfield is unsigned
-struct Test5 sometest5 = {-1}; // expected-warning {{implicit truncation from 
'int' to bitfield changes value from -1 to 3}}
+struct Test5 sometest5 = {-1};
 typedef __typeof__(+t5.n) Signed;  // ... but promotes to signed.
 
 typedef __typeof__(t5.n + 0) Signed; // Arithmetic promotes.

Modified: cfe/trunk/test/Sema/constant-conversion.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/constant-conversion.c?rev=277796=277795=277796=diff
==
--- cfe/trunk/test/Sema/constant-conversion.c (original)
+++ cfe/trunk/test/Sema/constant-conversion.c Thu Aug  4 21:39:30 2016
@@ -113,3 +113,15 @@ void test9() {
 
   char array_init[] = { 255, 127, 128, 129, 0 };
 }
+
+void test10() {
+  struct S {
+unsigned a : 4;
+  } s;
+  s.a = -1;
+  s.a = 15;
+  s.a = -8;
+
+  s.a = -9;  // expected-warning{{implicit truncation from 'int' to bitfield 
changes value from -9 to 7}}
+  s.a = 16;  // expected-warning{{implicit truncation from 'int' to bitfield 
changes value from 16 to 0}}
+}


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

Buildbot numbers for the last week of 7/24/2016 - 7/30/2016

[Galina Kistanova via cfe-commits]

Hello everyone,

Below are some buildbot numbers for the last week of 7/24/2016 - 7/30/2016.

Please see the same data in attached csv files:

The longest time each builder was red during the last 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 last week:

 buildername|  was_red
+---
 clang-x86-win2008-selfhost | 65:04:13
 clang-native-aarch64-full  | 32:17:43
 perf-x86_64-penryn-O3  | 32:06:30
 clang-x64-ninja-win7   | 30:35:37
 libcxx-libcxxabi-x86_64-linux-debian   | 29:08:09
 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast   | 20:44:14
 lld-x86_64-win7| 20:31:04
 libcxx-libcxxabi-x86_64-linux-ubuntu-gcc49-cxx11   | 19:17:28
 llvm-clang-lld-x86_64-debian-fast  | 18:23:22
 lld-x86_64-freebsd | 18:10:27
 libcxx-libcxxabi-x86_64-linux-ubuntu-cxx03 | 17:57:58
 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast | 17:53:22
 lldb-x86_64-ubuntu-14.04-android   | 15:26:47
 libcxx-libcxxabi-x86_64-linux-debian-noexceptions  | 15:20:21
 lldb-x86_64-darwin-13.4| 15:07:43
 lldb-windows7-android  | 12:59:23
 clang-ppc64le-linux-multistage | 10:30:19
 sanitizer-x86_64-linux | 09:40:30
 perf-x86_64-penryn-O3-polly-fast   | 09:28:05
 llvm-mips-linux| 09:26:47
 clang-atom-d525-fedora-rel | 08:53:20
 clang-cmake-armv7-a15-selfhost | 07:09:52
 clang-cmake-thumbv7-a15-full-sh| 07:05:54
 clang-ppc64le-linux-lnt| 06:51:03
 clang-cmake-armv7-a15-selfhost-neon| 06:05:33
 clang-cmake-armv7-a15  | 05:59:25
 clang-cmake-thumbv7-a15| 05:58:57
 sanitizer-ppc64le-linux| 05:43:18
 clang-cmake-armv7-a15-full | 05:41:43
 sanitizer-ppc64be-linux| 05:00:52
 clang-cmake-mips   | 04:51:16
 clang-cmake-aarch64-quick  | 04:45:22
 clang-hexagon-elf  | 04:44:38
 clang-cmake-aarch64-42vma  | 04:28:43
 clang-ppc64be-linux-multistage | 04:03:31
 clang-ppc64le-linux| 03:58:07
 polly-amd64-linux  | 03:57:58
 clang-cmake-aarch64-full   | 03:41:03
 clang-native-arm-lnt   | 03:37:04
 clang-ppc64be-linux-lnt| 03:32:31
 sanitizer-x86_64-linux-bootstrap   | 03:15:29
 sanitizer-windows  | 03:02:02
 clang-ppc64be-linux| 02:40:31
 clang-s390x-linux  | 02:37:17
 sanitizer-x86_64-linux-fuzzer  | 02:00:06
 sanitizer-x86_64-linux-autoconf| 01:42:53
 perf-x86_64-penryn-O3-polly-unprofitable   | 01:42:25
 clang-3stage-ubuntu| 01:41:38
 lldb-amd64-ninja-netbsd7   | 01:37:19
 lld-x86_64-darwin13| 01:37:01
 lldb-x86_64-ubuntu-14.04-cmake | 01:36:00
 clang-x86_64-debian-fast   | 01:35:57
 llvm-sphinx-docs   | 01:34:55
 clang-sphinx-docs  | 01:34:55
 lld-sphinx-docs| 01:34:54
 sanitizer-x86_64-linux-fast| 01:32:42
 lldb-x86_64-ubuntu-14.04-buildserver   | 01:26:20
 libcxx-libcxxabi-x86_64-linux-ubuntu-unstable-abi  | 01:24:14
 perf-x86_64-penryn-O3-polly-before-vectorizer-unprofitable | 01:19:03
 

Re: [PATCH] D22525: [Sema] Add sizeof diagnostics for bzero

[Bruno Cardoso Lopes via cfe-commits]

bruno closed this revision.
bruno added a comment.

Thanks! Committed in r277787


https://reviews.llvm.org/D22525



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

r277787 - [Sema] Add sizeof diagnostics for bzero

[Bruno Cardoso Lopes via cfe-commits]

Author: bruno
Date: Thu Aug  4 18:55:22 2016
New Revision: 277787

URL: http://llvm.org/viewvc/llvm-project?rev=277787=rev
Log:
[Sema] Add sizeof diagnostics for bzero

For memset (and others) we can get diagnostics like:

  struct stat { int x; };
  void foo(struct stat *stamps) {
bzero(stamps, sizeof(stamps));
memset(stamps, 0, sizeof(stamps));
  }

  t.c:7:28: warning: 'memset' call operates on objects of type 'struct stat' 
while the size is based on a different type 'struct stat *' 
[-Wsizeof-pointer-memaccess]
memset(stamps, 0, sizeof(stamps));
   ~~^~
  t.c:7:28: note: did you mean to dereference the argument to 'sizeof' (and 
multiply it by the number of elements)?
memset(stamps, 0, sizeof(stamps));
 ^~

This patch implements the same class of warnings for bzero.

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

rdar://problem/18963514

Modified:
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof.cpp

Modified: cfe/trunk/lib/AST/Decl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=277787=277786=277787=diff
==
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Thu Aug  4 18:55:22 2016
@@ -3408,6 +3408,10 @@ unsigned FunctionDecl::getMemoryFunction
   case Builtin::BIstrlen:
 return Builtin::BIstrlen;
 
+  case Builtin::BI__builtin_bzero:
+  case Builtin::BIbzero:
+return Builtin::BIbzero;
+
   default:
 if (isExternC()) {
   if (FnInfo->isStr("memset"))
@@ -3430,6 +3434,8 @@ unsigned FunctionDecl::getMemoryFunction
 return Builtin::BIstrndup;
   else if (FnInfo->isStr("strlen"))
 return Builtin::BIstrlen;
+  else if (FnInfo->isStr("bzero"))
+return Builtin::BIbzero;
 }
 break;
   }

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=277787=277786=277787=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Aug  4 18:55:22 2016
@@ -6179,13 +6179,15 @@ void Sema::CheckMemaccessArguments(const
 
   // It is possible to have a non-standard definition of memset.  Validate
   // we have enough arguments, and if not, abort further checking.
-  unsigned ExpectedNumArgs = (BId == Builtin::BIstrndup ? 2 : 3);
+  unsigned ExpectedNumArgs =
+  (BId == Builtin::BIstrndup || Builtin::BIbzero ? 2 : 3);
   if (Call->getNumArgs() < ExpectedNumArgs)
 return;
 
-  unsigned LastArg = (BId == Builtin::BImemset ||
+  unsigned LastArg = (BId == Builtin::BImemset || BId == Builtin::BIbzero ||
   BId == Builtin::BIstrndup ? 1 : 2);
-  unsigned LenArg = (BId == Builtin::BIstrndup ? 1 : 2);
+  unsigned LenArg =
+  (BId == Builtin::BIbzero || BId == Builtin::BIstrndup ? 1 : 2);
   const Expr *LenExpr = Call->getArg(LenArg)->IgnoreParenImpCasts();
 
   if (CheckMemorySizeofForComparison(*this, LenExpr, FnName,

Modified: cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof.cpp?rev=277787=277786=277787=diff
==
--- cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof.cpp Thu Aug  4 18:55:22 2016
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wno-sizeof-array-argument %s
 //
+extern "C" void *bzero(void *, unsigned);
 extern "C" void *memset(void *, int, unsigned);
 extern "C" void *memmove(void *s1, const void *s2, unsigned n);
 extern "C" void *memcpy(void *s1, const void *s2, unsigned n);
@@ -47,6 +48,19 @@ void f(Mat m, const Foo& const_foo, char
   memset(heap_buffer, 0, sizeof(heap_buffer));  // \
   // expected-warning {{'memset' call operates on objects of type 'char' 
while the size is based on a different type 'char *'}} expected-note{{did you 
mean to provide an explicit length?}}
 
+  bzero(, sizeof());  // \
+  // expected-warning {{'bzero' call operates on objects of type 'S' while 
the size is based on a different type 'S *'}} expected-note{{did you mean to 
remove the addressof in the argument to 'sizeof' (and multiply it by the number 
of elements)?}}
+  bzero(ps, sizeof(ps));  // \
+  // expected-warning {{'bzero' call operates on objects of type 'S' while 
the size is based on a different type 'S *'}} expected-note{{did you mean to 
dereference the argument to 'sizeof' (and multiply it by the number of 
elements)?}}
+  bzero(ps2, sizeof(ps2));  // \
+  // expected-warning {{'bzero' call operates on objects of type 'S' while 
the size is based on a different type 'PS' (aka 'S *')}} expected-note{{did you 
mean 

Re: [PATCH] D23147: [ADT] Migrate DepthFirstIterator to use NodeRef

[Tim Shen via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL277783: [ADT] Migrate DepthFirstIterator to use NodeRef 
(authored by timshen).

Changed prior to commit:
  https://reviews.llvm.org/D23147?vs=66745=66872#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D23147

Files:
  cfe/trunk/include/clang/AST/StmtGraphTraits.h
  cfe/trunk/include/clang/Analysis/Analyses/Dominators.h
  cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h

Index: cfe/trunk/include/clang/Analysis/Analyses/Dominators.h
===
--- cfe/trunk/include/clang/Analysis/Analyses/Dominators.h
+++ cfe/trunk/include/clang/Analysis/Analyses/Dominators.h
@@ -168,6 +168,7 @@
 namespace llvm {
 template <> struct GraphTraits< ::clang::DomTreeNode* > {
   typedef ::clang::DomTreeNode NodeType;
+  typedef ::clang::DomTreeNode *NodeRef;
   typedef NodeType::iterator  ChildIteratorType;
 
   static NodeType *getEntryNode(NodeType *N) {
Index: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
@@ -451,6 +451,7 @@
 namespace llvm {
   template<> struct GraphTraits {
 typedef clang::ento::ExplodedNode NodeType;
+typedef clang::ento::ExplodedNode *NodeRef;
 typedef NodeType::succ_iterator  ChildIteratorType;
 typedef llvm::df_iterator  nodes_iterator;
 
@@ -477,6 +478,7 @@
 
   template<> struct GraphTraits {
 typedef const clang::ento::ExplodedNode NodeType;
+typedef const clang::ento::ExplodedNode *NodeRef;
 typedef NodeType::const_succ_iterator   ChildIteratorType;
 typedef llvm::df_iterator   nodes_iterator;
 
Index: cfe/trunk/include/clang/AST/StmtGraphTraits.h
===
--- cfe/trunk/include/clang/AST/StmtGraphTraits.h
+++ cfe/trunk/include/clang/AST/StmtGraphTraits.h
@@ -26,6 +26,7 @@
 
 template <> struct GraphTraits {
   typedef clang::Stmt   NodeType;
+  typedef clang::Stmt * NodeRef;
   typedef clang::Stmt::child_iterator   ChildIteratorType;
   typedef llvm::df_iterator   nodes_iterator;
 
@@ -53,6 +54,7 @@
 
 template <> struct GraphTraits {
   typedef const clang::Stmt   NodeType;
+  typedef const clang::Stmt * NodeRef;
   typedef clang::Stmt::const_child_iterator   ChildIteratorType;
   typedef llvm::df_iterator   nodes_iterator;
 


Index: cfe/trunk/include/clang/Analysis/Analyses/Dominators.h
===
--- cfe/trunk/include/clang/Analysis/Analyses/Dominators.h
+++ cfe/trunk/include/clang/Analysis/Analyses/Dominators.h
@@ -168,6 +168,7 @@
 namespace llvm {
 template <> struct GraphTraits< ::clang::DomTreeNode* > {
   typedef ::clang::DomTreeNode NodeType;
+  typedef ::clang::DomTreeNode *NodeRef;
   typedef NodeType::iterator  ChildIteratorType;
 
   static NodeType *getEntryNode(NodeType *N) {
Index: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
@@ -451,6 +451,7 @@
 namespace llvm {
   template<> struct GraphTraits {
 typedef clang::ento::ExplodedNode NodeType;
+typedef clang::ento::ExplodedNode *NodeRef;
 typedef NodeType::succ_iterator  ChildIteratorType;
 typedef llvm::df_iterator  nodes_iterator;
 
@@ -477,6 +478,7 @@
 
   template<> struct GraphTraits {
 typedef const clang::ento::ExplodedNode NodeType;
+typedef const clang::ento::ExplodedNode *NodeRef;
 typedef NodeType::const_succ_iterator   ChildIteratorType;
 typedef llvm::df_iterator   nodes_iterator;
 
Index: cfe/trunk/include/clang/AST/StmtGraphTraits.h
===
--- cfe/trunk/include/clang/AST/StmtGraphTraits.h
+++ cfe/trunk/include/clang/AST/StmtGraphTraits.h
@@ -26,6 +26,7 @@
 
 template <> struct GraphTraits {
   typedef clang::Stmt   NodeType;
+  typedef clang::Stmt * NodeRef;
   typedef clang::Stmt::child_iterator   ChildIteratorType;
   typedef llvm::df_iterator   nodes_iterator;
 
@@ -53,6 +54,7 @@
 
 template <> struct GraphTraits {
   typedef const clang::Stmt   NodeType;
+  typedef const clang::Stmt * NodeRef;
   typedef clang::Stmt::const_child_iterator   ChildIteratorType;
   

r277783 - [ADT] Migrate DepthFirstIterator to use NodeRef

[Tim Shen via cfe-commits]

Author: timshen
Date: Thu Aug  4 18:03:44 2016
New Revision: 277783

URL: http://llvm.org/viewvc/llvm-project?rev=277783=rev
Log:
[ADT] Migrate DepthFirstIterator to use NodeRef

Summary: The corresponding LLVM change is D23146.

Reviewers: dblaikie, chandlerc

Subscribers: cfe-commits

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

Modified:
cfe/trunk/include/clang/AST/StmtGraphTraits.h
cfe/trunk/include/clang/Analysis/Analyses/Dominators.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h

Modified: cfe/trunk/include/clang/AST/StmtGraphTraits.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/StmtGraphTraits.h?rev=277783=277782=277783=diff
==
--- cfe/trunk/include/clang/AST/StmtGraphTraits.h (original)
+++ cfe/trunk/include/clang/AST/StmtGraphTraits.h Thu Aug  4 18:03:44 2016
@@ -26,6 +26,7 @@ namespace llvm {
 
 template <> struct GraphTraits {
   typedef clang::Stmt   NodeType;
+  typedef clang::Stmt * NodeRef;
   typedef clang::Stmt::child_iterator   ChildIteratorType;
   typedef llvm::df_iterator   nodes_iterator;
 
@@ -53,6 +54,7 @@ template <> struct GraphTraits struct GraphTraits {
   typedef const clang::Stmt   NodeType;
+  typedef const clang::Stmt * NodeRef;
   typedef clang::Stmt::const_child_iterator   ChildIteratorType;
   typedef llvm::df_iterator   nodes_iterator;
 

Modified: cfe/trunk/include/clang/Analysis/Analyses/Dominators.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/Dominators.h?rev=277783=277782=277783=diff
==
--- cfe/trunk/include/clang/Analysis/Analyses/Dominators.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/Dominators.h Thu Aug  4 18:03:44 
2016
@@ -168,6 +168,7 @@ private:
 namespace llvm {
 template <> struct GraphTraits< ::clang::DomTreeNode* > {
   typedef ::clang::DomTreeNode NodeType;
+  typedef ::clang::DomTreeNode *NodeRef;
   typedef NodeType::iterator  ChildIteratorType;
 
   static NodeType *getEntryNode(NodeType *N) {

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h?rev=277783=277782=277783=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h 
Thu Aug  4 18:03:44 2016
@@ -451,6 +451,7 @@ public:
 namespace llvm {
   template<> struct GraphTraits {
 typedef clang::ento::ExplodedNode NodeType;
+typedef clang::ento::ExplodedNode *NodeRef;
 typedef NodeType::succ_iterator  ChildIteratorType;
 typedef llvm::df_iterator  nodes_iterator;
 
@@ -477,6 +478,7 @@ namespace llvm {
 
   template<> struct GraphTraits {
 typedef const clang::ento::ExplodedNode NodeType;
+typedef const clang::ento::ExplodedNode *NodeRef;
 typedef NodeType::const_succ_iterator   ChildIteratorType;
 typedef llvm::df_iterator   nodes_iterator;
 


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

Re: [PATCH] D23160: [Coverage] Prevent creating a redundant counter if a nested body ends with a macro.

[Vedant Kumar via cfe-commits]

vsk added a subscriber: hans.
vsk added a comment.

(@hans If there are no objections, and if it's still possible, it would be 
great to see this merged into 3.9 once it has landed.)


https://reviews.llvm.org/D23160



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

Re: [PATCH] D22045: [X86] Support of no_caller_saved_registers attribute (Clang part)

[Erich Keane via cfe-commits]

erichkeane added inline comments.


Comment at: include/clang/CodeGen/CGFunctionInfo.h:479
@@ +478,3 @@
+  /// Whether this function saved caller registers.
+  unsigned NoCallerSavedRegs : 1;
+

majnemer wrote:
> aaron.ballman wrote:
> > erichkeane wrote:
> > > aaron.ballman wrote:
> > > > erichkeane wrote:
> > > > > aaron.ballman wrote:
> > > > > > This is unfortunate as it will bump the bit-field length to 33 
> > > > > > bits, which seems rather wasteful. Are there any bits we can steal 
> > > > > > to bring this back down to a 32-bit bit-field?
> > > > > I implemented this additional patch, but don't really know a TON 
> > > > > about this area, so I have a few ideas but would like to get 
> > > > > direction on it if possible.  I had the following ideas of where to 
> > > > > get a few more bits:
> > > > > 
> > > > > CallingConvention/EffectiveCallingConvention/ASTCallingConvention:  
> > > > > This structure stores a pre-converted calling convention to the 
> > > > > llvm::CallingConv enum (llvm/include/llvm/IR/CallingConv.h).  I'll 
> > > > > note that the legal values for this go up to 1023 (so 8 bits isn't 
> > > > > enough anyway!), though only up to 91 are currently used.  
> > > > > 
> > > > > HOWEVER, the clang CallingConv (include/clang/Basic/Specifiers.h 
> > > > > :233) only has 16 items in it.  If we were instead to store THAT and 
> > > > > convert upon access (a simple switch statement, already used 
> > > > > constructing this value, see ClangCallConvToLLVMCallConv), we could 
> > > > > get away with 6 or 7 bits each, saving this 3-6 bits total, yet have 
> > > > > way more than enough room for expansion.
> > > > > 
> > > > > HasRegParm: This field might be possible to eliminate.  According to 
> > > > > the GCC RegParm docs (I don't see a clang one?) here 
> > > > > (https://gcc.gnu.org/onlinedocs/gcc/x86-Function-Attributes.html), ", 
> > > > > the regparm attribute causes the compiler to pass arguments number 
> > > > > one to number if they are of integral type in registers EAX, EDX, and 
> > > > > ECX instead of on the stack".
> > > > > 
> > > > > It seems that 0 for "RegParm" should be illegal, so I wonder if we 
> > > > > can treat "RegParm==0" as "HasRegParm==false" and eliminate storing 
> > > > > that.
> > > > > 
> > > > > In my opinion, the 1st one gives us way more room for the future and 
> > > > > corrects a possible future bug.  The 2nd is likely a lower touch, 
> > > > > though it could possibly change behavior (see discussion here 
> > > > > https://www.coreboot.org/pipermail/coreboot/2008-October/040406.html) 
> > > > > as regparm(0) is seemingly accepted by both compilers.  I DO note 
> > > > > that this comment notes that 'regparm 0' is the default behavior, so 
> > > > > I'm not sure what change that would do.
> > > > > 
> > > > > Either way, I suspect this change should be a separate commit (though 
> > > > > I would figure making it a pre-req for this patch would be the right 
> > > > > way to go).  If you give some guidance as to which you think would be 
> > > > > better, I can put a patch together.
> > > > > 
> > > > > -Erich
> > > > > 
> > > > I think that `unsigned ASTCallingConvention : 8;` can be safely 
> > > > reduced. This tracks a `clang::CallingConv` value, the maximum of which 
> > > > is 15. So I think the way to do this is to reduce ASTCallingConvention 
> > > > from 8 to 7 bits and then pack yours in as well.
> > > Ah! I missed that this was the case.  That said, it could likely be 
> > > reduced to 6 if we really wished (currently 16 items, 6 gives us room for 
> > > 32).  Perhaps something to keep in our pocket for the next time someone 
> > > needs a bit or two here.
> > > 
> > > 
> > > I'll update the diff for Amjad if possible.
> > I'm on the fence about 6 vs 7 and see no harm in reducing it to either 
> > value, but have a *very* slight preference for 7 so that the bit-field 
> > grouping continues to add up to 32-bits. However, it's your call.
> I'd go with 6 and have another bitfield, `unsigned UnusedBits : 1;`
> 
> We use this paradigm elsewhere.
That seems like a solid solution.  I'll add it to another diff and give it to 
Amjad to update this patch.  Not sure if he has the ability to (or if it is too 
late), but I don't have permissions to alter this commit for him.


https://reviews.llvm.org/D22045



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

Re: [PATCH] D22045: [X86] Support of no_caller_saved_registers attribute (Clang part)

[David Majnemer via cfe-commits]

majnemer added a subscriber: majnemer.


Comment at: include/clang/CodeGen/CGFunctionInfo.h:479
@@ +478,3 @@
+  /// Whether this function saved caller registers.
+  unsigned NoCallerSavedRegs : 1;
+

aaron.ballman wrote:
> erichkeane wrote:
> > aaron.ballman wrote:
> > > erichkeane wrote:
> > > > aaron.ballman wrote:
> > > > > This is unfortunate as it will bump the bit-field length to 33 bits, 
> > > > > which seems rather wasteful. Are there any bits we can steal to bring 
> > > > > this back down to a 32-bit bit-field?
> > > > I implemented this additional patch, but don't really know a TON about 
> > > > this area, so I have a few ideas but would like to get direction on it 
> > > > if possible.  I had the following ideas of where to get a few more bits:
> > > > 
> > > > CallingConvention/EffectiveCallingConvention/ASTCallingConvention:  
> > > > This structure stores a pre-converted calling convention to the 
> > > > llvm::CallingConv enum (llvm/include/llvm/IR/CallingConv.h).  I'll note 
> > > > that the legal values for this go up to 1023 (so 8 bits isn't enough 
> > > > anyway!), though only up to 91 are currently used.  
> > > > 
> > > > HOWEVER, the clang CallingConv (include/clang/Basic/Specifiers.h :233) 
> > > > only has 16 items in it.  If we were instead to store THAT and convert 
> > > > upon access (a simple switch statement, already used constructing this 
> > > > value, see ClangCallConvToLLVMCallConv), we could get away with 6 or 7 
> > > > bits each, saving this 3-6 bits total, yet have way more than enough 
> > > > room for expansion.
> > > > 
> > > > HasRegParm: This field might be possible to eliminate.  According to 
> > > > the GCC RegParm docs (I don't see a clang one?) here 
> > > > (https://gcc.gnu.org/onlinedocs/gcc/x86-Function-Attributes.html), ", 
> > > > the regparm attribute causes the compiler to pass arguments number one 
> > > > to number if they are of integral type in registers EAX, EDX, and ECX 
> > > > instead of on the stack".
> > > > 
> > > > It seems that 0 for "RegParm" should be illegal, so I wonder if we can 
> > > > treat "RegParm==0" as "HasRegParm==false" and eliminate storing that.
> > > > 
> > > > In my opinion, the 1st one gives us way more room for the future and 
> > > > corrects a possible future bug.  The 2nd is likely a lower touch, 
> > > > though it could possibly change behavior (see discussion here 
> > > > https://www.coreboot.org/pipermail/coreboot/2008-October/040406.html) 
> > > > as regparm(0) is seemingly accepted by both compilers.  I DO note that 
> > > > this comment notes that 'regparm 0' is the default behavior, so I'm not 
> > > > sure what change that would do.
> > > > 
> > > > Either way, I suspect this change should be a separate commit (though I 
> > > > would figure making it a pre-req for this patch would be the right way 
> > > > to go).  If you give some guidance as to which you think would be 
> > > > better, I can put a patch together.
> > > > 
> > > > -Erich
> > > > 
> > > I think that `unsigned ASTCallingConvention : 8;` can be safely reduced. 
> > > This tracks a `clang::CallingConv` value, the maximum of which is 15. So 
> > > I think the way to do this is to reduce ASTCallingConvention from 8 to 7 
> > > bits and then pack yours in as well.
> > Ah! I missed that this was the case.  That said, it could likely be reduced 
> > to 6 if we really wished (currently 16 items, 6 gives us room for 32).  
> > Perhaps something to keep in our pocket for the next time someone needs a 
> > bit or two here.
> > 
> > 
> > I'll update the diff for Amjad if possible.
> I'm on the fence about 6 vs 7 and see no harm in reducing it to either value, 
> but have a *very* slight preference for 7 so that the bit-field grouping 
> continues to add up to 32-bits. However, it's your call.
I'd go with 6 and have another bitfield, `unsigned UnusedBits : 1;`

We use this paradigm elsewhere.


https://reviews.llvm.org/D22045



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

Re: [PATCH] D23168: emit_DW_AT_noreturn flag

[Adrian Prantl via cfe-commits]

aprantl added a comment.

Please add separate testcases for C11 and C++11 (and possibly Objective-C?).


https://reviews.llvm.org/D23168



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

r277757 - [analyzer] Make CloneDetector recognize different variable patterns.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Thu Aug  4 14:37:00 2016
New Revision: 277757

URL: http://llvm.org/viewvc/llvm-project?rev=277757=rev
Log:
[analyzer] Make CloneDetector recognize different variable patterns.

CloneDetector should be able to detect clones with renamed variables.
However, if variables are referenced multiple times around the code sample,
the usage patterns need to be recognized.

For example, (x < y ? y : x) and (y < x ? y : x) are no longer clones,
however (a < b ? b : a) is still a clone of the former.

Variable patterns are computed and compared during a separate filtering pass.

Patch by Raphael Isemann!

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

Removed:
cfe/trunk/test/Analysis/copypaste/false-positives.cpp
Modified:
cfe/trunk/lib/Analysis/CloneDetection.cpp
cfe/trunk/test/Analysis/copypaste/functions.cpp

Modified: cfe/trunk/lib/Analysis/CloneDetection.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CloneDetection.cpp?rev=277757=277756=277757=diff
==
--- cfe/trunk/lib/Analysis/CloneDetection.cpp (original)
+++ cfe/trunk/lib/Analysis/CloneDetection.cpp Thu Aug  4 14:37:00 2016
@@ -80,6 +80,102 @@ SourceLocation StmtSequence::getStartLoc
 SourceLocation StmtSequence::getEndLoc() const { return back()->getLocEnd(); }
 
 namespace {
+
+/// \brief Analyzes the pattern of the referenced variables in a statement.
+class VariablePattern {
+
+  /// \brief Describes an occurence of a variable reference in a statement.
+  struct VariableOccurence {
+/// The index of the associated VarDecl in the Variables vector.
+size_t KindID;
+
+VariableOccurence(size_t KindID) : KindID(KindID) {}
+  };
+
+  /// All occurences of referenced variables in the order of appearance.
+  std::vector Occurences;
+  /// List of referenced variables in the order of appearance.
+  /// Every item in this list is unique.
+  std::vector Variables;
+
+  /// \brief Adds a new variable referenced to this pattern.
+  /// \param VarDecl The declaration of the variable that is referenced.
+  void addVariableOccurence(const VarDecl *VarDecl) {
+// First check if we already reference this variable
+for (size_t KindIndex = 0; KindIndex < Variables.size(); ++KindIndex) {
+  if (Variables[KindIndex] == VarDecl) {
+// If yes, add a new occurence that points to the existing entry in
+// the Variables vector.
+Occurences.emplace_back(KindIndex);
+return;
+  }
+}
+// If this variable wasn't already referenced, add it to the list of
+// referenced variables and add a occurence that points to this new entry.
+Occurences.emplace_back(Variables.size());
+Variables.push_back(VarDecl);
+  }
+
+  /// \brief Adds each referenced variable from the given statement.
+  void addVariables(const Stmt *S) {
+// Sometimes we get a nullptr (such as from IfStmts which often have 
nullptr
+// children). We skip such statements as they don't reference any
+// variables.
+if (!S)
+  return;
+
+// Check if S is a reference to a variable. If yes, add it to the pattern.
+if (auto D = dyn_cast(S)) {
+  if (auto VD = dyn_cast(D->getDecl()->getCanonicalDecl()))
+addVariableOccurence(VD);
+}
+
+// Recursively check all children of the given statement.
+for (const Stmt *Child : S->children()) {
+  addVariables(Child);
+}
+  }
+
+public:
+  /// \brief Creates an VariablePattern object with information about the given
+  ///StmtSequence.
+  VariablePattern(const StmtSequence ) {
+for (const Stmt *S : Sequence)
+  addVariables(S);
+  }
+
+  /// \brief Compares this pattern with the given one.
+  /// \param Other The given VariablePattern to compare with.
+  /// \return Returns true if and only if the references variables in this
+  /// object follow the same pattern than the ones in the given
+  /// VariablePattern.
+  ///
+  /// For example, the following statements all have the same pattern:
+  ///
+  ///   if (a < b) return a; return b;
+  ///   if (x < y) return x; return y;
+  ///   if (u2 < u1) return u2; return u1;
+  ///
+  /// but the following statement has a different pattern (note the changed
+  /// variables in the return statements).
+  ///
+  ///   if (a < b) return b; return a;
+  ///
+  /// This function should only be called if the related statements of the 
given
+  /// pattern and the statements of this objects are clones of each other.
+  bool comparePattern(const VariablePattern ) {
+assert(Other.Occurences.size() == Occurences.size());
+for (unsigned i = 0; i < Occurences.size(); ++i) {
+  if (Occurences[i].KindID != Other.Occurences[i].KindID) {
+return false;
+  }
+}
+return true;
+  }
+};
+}
+
+namespace {
 /// \brief Collects the data of a single Stmt.
 ///
 /// This class defines what a code clone is: If it collects for two 

Re: [PATCH] D22982: [CloneDetector] No longer reporting clones that don't have a common referenced variable pattern.

[Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL277757: [analyzer] Make CloneDetector recognize different 
variable patterns. (authored by dergachev).

Changed prior to commit:
  https://reviews.llvm.org/D22982?vs=66839=66842#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D22982

Files:
  cfe/trunk/lib/Analysis/CloneDetection.cpp
  cfe/trunk/test/Analysis/copypaste/false-positives.cpp
  cfe/trunk/test/Analysis/copypaste/functions.cpp

Index: cfe/trunk/test/Analysis/copypaste/functions.cpp
===
--- cfe/trunk/test/Analysis/copypaste/functions.cpp
+++ cfe/trunk/test/Analysis/copypaste/functions.cpp
@@ -37,14 +37,22 @@
   return b;
 }
 
-
+// No clone because of the different comparison operator.
 int min1(int a, int b) { // no-warning
   log();
   if (a < b)
 return a;
   return b;
 }
 
+// No clone because of the different pattern in which the variables are used.
+int min2(int a, int b) { // no-warning
+  log();
+  if (a > b)
+return b;
+  return a;
+}
+
 int foo(int a, int b) { // no-warning
   return a + b;
 }
Index: cfe/trunk/test/Analysis/copypaste/false-positives.cpp
===
--- cfe/trunk/test/Analysis/copypaste/false-positives.cpp
+++ cfe/trunk/test/Analysis/copypaste/false-positives.cpp
@@ -1,21 +0,0 @@
-// RUN: %clang_cc1 -analyze -std=c++11 -analyzer-checker=alpha.clone.CloneChecker -verify %s
-
-// This test contains false-positive reports from the CloneChecker that need to
-// be fixed.
-
-void log();
-
-int max(int a, int b) { // expected-warning{{Detected code clone.}}
-  log();
-  if (a > b)
-return a;
-  return b;
-}
-
-// FIXME: Detect different variable patterns.
-int min2(int a, int b) { // expected-note{{Related code clone is here.}}
-  log();
-  if (b > a)
-return a;
-  return b;
-}
Index: cfe/trunk/lib/Analysis/CloneDetection.cpp
===
--- cfe/trunk/lib/Analysis/CloneDetection.cpp
+++ cfe/trunk/lib/Analysis/CloneDetection.cpp
@@ -80,6 +80,102 @@
 SourceLocation StmtSequence::getEndLoc() const { return back()->getLocEnd(); }
 
 namespace {
+
+/// \brief Analyzes the pattern of the referenced variables in a statement.
+class VariablePattern {
+
+  /// \brief Describes an occurence of a variable reference in a statement.
+  struct VariableOccurence {
+/// The index of the associated VarDecl in the Variables vector.
+size_t KindID;
+
+VariableOccurence(size_t KindID) : KindID(KindID) {}
+  };
+
+  /// All occurences of referenced variables in the order of appearance.
+  std::vector Occurences;
+  /// List of referenced variables in the order of appearance.
+  /// Every item in this list is unique.
+  std::vector Variables;
+
+  /// \brief Adds a new variable referenced to this pattern.
+  /// \param VarDecl The declaration of the variable that is referenced.
+  void addVariableOccurence(const VarDecl *VarDecl) {
+// First check if we already reference this variable
+for (size_t KindIndex = 0; KindIndex < Variables.size(); ++KindIndex) {
+  if (Variables[KindIndex] == VarDecl) {
+// If yes, add a new occurence that points to the existing entry in
+// the Variables vector.
+Occurences.emplace_back(KindIndex);
+return;
+  }
+}
+// If this variable wasn't already referenced, add it to the list of
+// referenced variables and add a occurence that points to this new entry.
+Occurences.emplace_back(Variables.size());
+Variables.push_back(VarDecl);
+  }
+
+  /// \brief Adds each referenced variable from the given statement.
+  void addVariables(const Stmt *S) {
+// Sometimes we get a nullptr (such as from IfStmts which often have nullptr
+// children). We skip such statements as they don't reference any
+// variables.
+if (!S)
+  return;
+
+// Check if S is a reference to a variable. If yes, add it to the pattern.
+if (auto D = dyn_cast(S)) {
+  if (auto VD = dyn_cast(D->getDecl()->getCanonicalDecl()))
+addVariableOccurence(VD);
+}
+
+// Recursively check all children of the given statement.
+for (const Stmt *Child : S->children()) {
+  addVariables(Child);
+}
+  }
+
+public:
+  /// \brief Creates an VariablePattern object with information about the given
+  ///StmtSequence.
+  VariablePattern(const StmtSequence ) {
+for (const Stmt *S : Sequence)
+  addVariables(S);
+  }
+
+  /// \brief Compares this pattern with the given one.
+  /// \param Other The given VariablePattern to compare with.
+  /// \return Returns true if and only if the references variables in this
+  /// object follow the same pattern than the ones in the given
+  /// VariablePattern.
+  ///
+  /// For example, the following statements all have the same pattern:
+  ///
+  ///   if (a < b) return a; return b;
+  /// 

r277756 - [OpenCL] Add the lit test for image size which was omitted by r277647.

[Yaxun Liu via cfe-commits]

Author: yaxunl
Date: Thu Aug  4 14:35:17 2016
New Revision: 277756

URL: http://llvm.org/viewvc/llvm-project?rev=277756=rev
Log:
[OpenCL] Add the lit test for image size which was omitted by r277647.

Added:
cfe/trunk/test/CodeGenOpenCL/cast_image.cl

Added: cfe/trunk/test/CodeGenOpenCL/cast_image.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/cast_image.cl?rev=277756=auto
==
--- cfe/trunk/test/CodeGenOpenCL/cast_image.cl (added)
+++ cfe/trunk/test/CodeGenOpenCL/cast_image.cl Thu Aug  4 14:35:17 2016
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -emit-llvm -o - -triple amdgcn--amdhsa %s | FileCheck 
--check-prefix=AMDGCN %s
+// RUN: %clang_cc1 -emit-llvm -o - -triple spir-unknown-unknown %s | FileCheck 
--check-prefix=SPIR %s
+
+#ifdef __AMDGCN__
+
+constant int* convert(image2d_t img) {
+  // AMDGCN: bitcast %opencl.image2d_ro_t addrspace(2)* %img to i32 
addrspace(2)*
+  return __builtin_astype(img, constant int*);
+}
+
+#else
+
+global int* convert(image2d_t img) {
+  // SPIR: bitcast %opencl.image2d_ro_t addrspace(1)* %img to i32 addrspace(1)*
+  return __builtin_astype(img, global int*);
+}
+
+#endif


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

r277754 - [OpenCL] Remove extra native_ functions from opencl-c.h

[Yaxun Liu via cfe-commits]

Author: yaxunl
Date: Thu Aug  4 14:30:54 2016
New Revision: 277754

URL: http://llvm.org/viewvc/llvm-project?rev=277754=rev
Log:
[OpenCL] Remove extra native_ functions from opencl-c.h

There should be no native_ builtin functions with double type arguments.

Patch by Aaron En Ye Shi.

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

Modified:
cfe/trunk/lib/Headers/opencl-c.h

Modified: cfe/trunk/lib/Headers/opencl-c.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/opencl-c.h?rev=277754=277753=277754=diff
==
--- cfe/trunk/lib/Headers/opencl-c.h (original)
+++ cfe/trunk/lib/Headers/opencl-c.h Thu Aug  4 14:30:54 2016
@@ -9810,14 +9810,6 @@ float3 __ovld __cnfn native_cos(float3 x
 float4 __ovld __cnfn native_cos(float4 x);
 float8 __ovld __cnfn native_cos(float8 x);
 float16 __ovld __cnfn native_cos(float16 x);
-#ifdef cl_khr_fp64
-double __ovld __cnfn native_cos(double x);
-double2 __ovld __cnfn native_cos(double2 x);
-double3 __ovld __cnfn native_cos(double3 x);
-double4 __ovld __cnfn native_cos(double4 x);
-double8 __ovld __cnfn native_cos(double8 x);
-double16 __ovld __cnfn native_cos(double16 x);
-#endif //cl_khr_fp64
 
 /**
  * Compute x / y over an implementation-defined range.
@@ -9829,14 +9821,6 @@ float3 __ovld __cnfn native_divide(float
 float4 __ovld __cnfn native_divide(float4 x, float4 y);
 float8 __ovld __cnfn native_divide(float8 x, float8 y);
 float16 __ovld __cnfn native_divide(float16 x, float16 y);
-#ifdef cl_khr_fp64
-double __ovld __cnfn native_divide(double x, double y);
-double2 __ovld __cnfn native_divide(double2 x, double2 y);
-double3 __ovld __cnfn native_divide(double3 x, double3 y);
-double4 __ovld __cnfn native_divide(double4 x, double4 y);
-double8 __ovld __cnfn native_divide(double8 x, double8 y);
-double16 __ovld __cnfn native_divide(double16 x, double16 y);
-#endif //cl_khr_fp64
 
 /**
  * Compute the base- e exponential of x over an
@@ -9849,14 +9833,6 @@ float3 __ovld __cnfn native_exp(float3 x
 float4 __ovld __cnfn native_exp(float4 x);
 float8 __ovld __cnfn native_exp(float8 x);
 float16 __ovld __cnfn native_exp(float16 x);
-#ifdef cl_khr_fp64
-double __ovld __cnfn native_exp(double x);
-double2 __ovld __cnfn native_exp(double2 x);
-double3 __ovld __cnfn native_exp(double3 x);
-double4 __ovld __cnfn native_exp(double4 x);
-double8 __ovld __cnfn native_exp(double8 x);
-double16 __ovld __cnfn native_exp(double16 x);
-#endif //cl_khr_fp64
 
 /**
  * Compute the base- 2 exponential of x over an
@@ -9869,14 +9845,6 @@ float3 __ovld __cnfn native_exp2(float3
 float4 __ovld __cnfn native_exp2(float4 x);
 float8 __ovld __cnfn native_exp2(float8 x);
 float16 __ovld __cnfn native_exp2(float16 x);
-#ifdef cl_khr_fp64
-double __ovld __cnfn native_exp2(double x);
-double2 __ovld __cnfn native_exp2(double2 x);
-double3 __ovld __cnfn native_exp2(double3 x);
-double4 __ovld __cnfn native_exp2(double4 x);
-double8 __ovld __cnfn native_exp2(double8 x);
-double16 __ovld __cnfn native_exp2(double16 x);
-#endif //cl_khr_fp64
 
 /**
  * Compute the base- 10 exponential of x over an
@@ -9889,14 +9857,6 @@ float3 __ovld __cnfn native_exp10(float3
 float4 __ovld __cnfn native_exp10(float4 x);
 float8 __ovld __cnfn native_exp10(float8 x);
 float16 __ovld __cnfn native_exp10(float16 x);
-#ifdef cl_khr_fp64
-double __ovld __cnfn native_exp10(double x);
-double2 __ovld __cnfn native_exp10(double2 x);
-double3 __ovld __cnfn native_exp10(double3 x);
-double4 __ovld __cnfn native_exp10(double4 x);
-double8 __ovld __cnfn native_exp10(double8 x);
-double16 __ovld __cnfn native_exp10(double16 x);
-#endif //cl_khr_fp64
 
 /**
  * Compute natural logarithm over an implementationdefined
@@ -9909,14 +9869,6 @@ float3 __ovld __cnfn native_log(float3 x
 float4 __ovld __cnfn native_log(float4 x);
 float8 __ovld __cnfn native_log(float8 x);
 float16 __ovld __cnfn native_log(float16 x);
-#ifdef cl_khr_fp64
-double __ovld __cnfn native_log(double x);
-double2 __ovld __cnfn native_log(double2 x);
-double3 __ovld __cnfn native_log(double3 x);
-double4 __ovld __cnfn native_log(double4 x);
-double8 __ovld __cnfn native_log(double8 x);
-double16 __ovld __cnfn native_log(double16 x);
-#endif //cl_khr_fp64
 
 /**
  * Compute a base 2 logarithm over an implementationdefined
@@ -9928,14 +9880,6 @@ float3 __ovld __cnfn native_log2(float3
 float4 __ovld __cnfn native_log2(float4 x);
 float8 __ovld __cnfn native_log2(float8 x);
 float16 __ovld __cnfn native_log2(float16 x);
-#ifdef cl_khr_fp64
-double __ovld __cnfn native_log2(double x);
-double2 __ovld __cnfn native_log2(double2 x);
-double3 __ovld __cnfn native_log2(double3 x);
-double4 __ovld __cnfn native_log2(double4 x);
-double8 __ovld __cnfn native_log2(double8 x);
-double16 __ovld __cnfn native_log2(double16 x);
-#endif //cl_khr_fp64
 
 /**
  * Compute a base 10 logarithm over an implementationdefined
@@ -9947,14 +9891,6 @@ float3 __ovld __cnfn native_log10(float3

Re: [PATCH] D22982: [CloneDetector] No longer reporting clones that don't have a common referenced variable pattern.

[Raphael Isemann via cfe-commits]

teemperor updated this revision to Diff 66839.
teemperor marked 3 inline comments as done.
teemperor added a comment.

- Patch should no longer cause merge conflicts.
- Improved comments and tests in functions.cpp.


https://reviews.llvm.org/D22982

Files:
  lib/Analysis/CloneDetection.cpp
  test/Analysis/copypaste/false-positives.cpp
  test/Analysis/copypaste/functions.cpp

Index: test/Analysis/copypaste/functions.cpp
===
--- test/Analysis/copypaste/functions.cpp
+++ test/Analysis/copypaste/functions.cpp
@@ -37,14 +37,22 @@
   return b;
 }
 
-
+// No clone because of the different comparison operator.
 int min1(int a, int b) { // no-warning
   log();
   if (a < b)
 return a;
   return b;
 }
 
+// No clone because of the different pattern in which the variables are used.
+int min2(int a, int b) { // no-warning
+  log();
+  if (a > b)
+return b;
+  return a;
+}
+
 int foo(int a, int b) { // no-warning
   return a + b;
 }
Index: test/Analysis/copypaste/false-positives.cpp
===
--- test/Analysis/copypaste/false-positives.cpp
+++ /dev/null
@@ -1,21 +0,0 @@
-// RUN: %clang_cc1 -analyze -std=c++11 -analyzer-checker=alpha.clone.CloneChecker -verify %s
-
-// This test contains false-positive reports from the CloneChecker that need to
-// be fixed.
-
-void log();
-
-int max(int a, int b) { // expected-warning{{Detected code clone.}}
-  log();
-  if (a > b)
-return a;
-  return b;
-}
-
-// FIXME: Detect different variable patterns.
-int min2(int a, int b) { // expected-note{{Related code clone is here.}}
-  log();
-  if (b > a)
-return a;
-  return b;
-}
Index: lib/Analysis/CloneDetection.cpp
===
--- lib/Analysis/CloneDetection.cpp
+++ lib/Analysis/CloneDetection.cpp
@@ -80,6 +80,102 @@
 SourceLocation StmtSequence::getEndLoc() const { return back()->getLocEnd(); }
 
 namespace {
+
+/// \brief Analyzes the pattern of the referenced variables in a statement.
+class VariablePattern {
+
+  /// \brief Describes an occurence of a variable reference in a statement.
+  struct VariableOccurence {
+/// The index of the associated VarDecl in the Variables vector.
+size_t KindID;
+
+VariableOccurence(size_t KindID) : KindID(KindID) {}
+  };
+
+  /// All occurences of referenced variables in the order of appearance.
+  std::vector Occurences;
+  /// List of referenced variables in the order of appearance.
+  /// Every item in this list is unique.
+  std::vector Variables;
+
+  /// \brief Adds a new variable referenced to this pattern.
+  /// \param VarDecl The declaration of the variable that is referenced.
+  void addVariableOccurence(const VarDecl *VarDecl) {
+// First check if we already reference this variable
+for (size_t KindIndex = 0; KindIndex < Variables.size(); ++KindIndex) {
+  if (Variables[KindIndex] == VarDecl) {
+// If yes, add a new occurence that points to the existing entry in
+// the Variables vector.
+Occurences.emplace_back(KindIndex);
+return;
+  }
+}
+// If this variable wasn't already referenced, add it to the list of
+// referenced variables and add a occurence that points to this new entry.
+Occurences.emplace_back(Variables.size());
+Variables.push_back(VarDecl);
+  }
+
+  /// \brief Adds each referenced variable from the given statement.
+  void addVariables(const Stmt *S) {
+// Sometimes we get a nullptr (such as from IfStmts which often have nullptr
+// children). We skip such statements as they don't reference any
+// variables.
+if (!S)
+  return;
+
+// Check if S is a reference to a variable. If yes, add it to the pattern.
+if (auto D = dyn_cast(S)) {
+  if (auto VD = dyn_cast(D->getDecl()->getCanonicalDecl()))
+addVariableOccurence(VD);
+}
+
+// Recursively check all children of the given statement.
+for (const Stmt *Child : S->children()) {
+  addVariables(Child);
+}
+  }
+
+public:
+  /// \brief Creates an VariablePattern object with information about the given
+  ///StmtSequence.
+  VariablePattern(const StmtSequence ) {
+for (const Stmt *S : Sequence)
+  addVariables(S);
+  }
+
+  /// \brief Compares this pattern with the given one.
+  /// \param Other The given VariablePattern to compare with.
+  /// \return Returns true if and only if the references variables in this
+  /// object follow the same pattern than the ones in the given
+  /// VariablePattern.
+  ///
+  /// For example, the following statements all have the same pattern:
+  ///
+  ///   if (a < b) return a; return b;
+  ///   if (x < y) return x; return y;
+  ///   if (u2 < u1) return u2; return u1;
+  ///
+  /// but the following statement has a different pattern (note the changed
+  /// variables in the return statements).
+  ///
+  ///   if (a < b) 

Re: [PATCH] D22374: [analyzer] Copy and move constructors - ExprEngine extended for "almost trivial" copy and move constructors

[Artem Dergachev via cfe-commits]

NoQ added a comment.

In https://reviews.llvm.org/D22374#505672, @baloghadamsoftware wrote:

> In https://reviews.llvm.org/D22374#504855, @NoQ wrote:
>
> > Hmm. I suggest:
> >
> > 1. Change this test's constructor so that it was no longer almost-trivial. 
> > Because it isn't significant for this test if the constructor is 
> > almost-trivial or not. The test would pass.
>
>
> It is significant, because I want to test here the almost-trivial 
> constructors.


I mean, the positive in `Inner::Inner()`, which has regressed, was written long 
before the notion of "almost-trivial constructor" was introduced by you. So it 
shouldn't be a problem if we modify this test to let the constructor become 
non-almost-trivial; that wasn't the purpose of that particular test.

And we could create another test with almost-trivial constructor elsewhere, 
which would prove the usefulness of your patch.

> > 2. Add a FIXME-test for this checker, in which a completely undefined 
> > structure is being copied. Perhaps somebody implements this feature some 
> > day.

> 

> 

> So it would be a new function with the same structure, and commented out 
> beginning with a FIXME?


Yeah, that's what i thought - not completely commented out, just having an 
opposite expected-warning <=> no-warning, and a "FIXME: we should (not) warn 
here because *some hand-waving*". There are a few such tests around.

> > 3. Leave out the situation that the current test checks as a grey-area. I'm 
> > still not convinced that this situation (copying a partially-initialized 
> > almost-trivial structure) deserves a warning from this checker.

> 

> 

> This is what I originally did here in the patch. Maybe a FIXME could be 
> written there a well?


I think this test should still test what it used to test - i've a feeling it's 
ok if we modify the part of it that was not related to what it used to test, 
but i'm feeling bad about removing it completely.

Essentially, this plan of mine has the sole purpose of keeping my (and perhaps 
somebody else's) conscience from biting me for removing the test; it seems that 
simpler approaches seem to carry more of the "no-no, this shouldn't happen" 
feeling. I guess i could post a patch-over-a-patch if what i'm expressing isn't 
clear.


https://reviews.llvm.org/D22374



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

Re: [PATCH] D22982: [CloneDetector] No longer reporting clones that don't have a common referenced variable pattern.

[Artem Dergachev via cfe-commits]

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

Whoops, because of a merge conflict while applying the patch, i noticed a few 
problems in the tests, they seem obvious, but could you have a quick look?



Comment at: test/Analysis/copypaste/functions.cpp:21
@@ -20,3 +20,3 @@
 
-// Functions below are not clones and should not be reported.
+// Different variable patterns, therefore different clones.
 

Not sure, do "different clones" mean "different code, should not be reported" 
or "different clones of the same code, should be reported"?


Comment at: test/Analysis/copypaste/functions.cpp:30
@@ +29,3 @@
+
+int minClone(int x, int y) { // expected-note{{Related code clone is here.}}
+  log();

This code is exactly identical, did you mean 'x > y' in any of those places?


https://reviews.llvm.org/D22982



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

Re: [PATCH] D23003: [ObjC Availability] Warn upon unguarded use of partially available declaration

[Manman Ren via cfe-commits]

manmanren added a comment.

Hi Erik,

Thanks for working on this! It is great to see these patches coming.

Manman



Comment at: include/clang/Sema/Sema.h:9608
@@ -9604,1 +9607,3 @@
 
+  /// \brief Whether we should emit an availability diagnostic for \c D.
+  bool ShouldDiagnoseAvailabilityOfDecl(

Can you explain the inputs and outputs?


Comment at: lib/Sema/SemaDeclAttr.cpp:6517
@@ +6516,3 @@
+
+/// \brief This class implements -Wunguarded-availability.
+class DiagnoseUnguardedAvailability

Can you add a high level description of what this class is doing to issue 
unguarded diagnostics?


Comment at: lib/Sema/SemaDeclAttr.cpp:6540
@@ +6539,3 @@
+
+  bool VisitTypeLoc(TypeLoc TL);
+

Do we cover all usage of decls with the above three member functions?


Comment at: lib/Sema/SemaDeclAttr.cpp:6552
@@ +6551,3 @@
+
+  if (SemaRef.ShouldDiagnoseAvailabilityOfDecl(D, nullptr, false, ObjCPDecl,
+   ContextVersion, nullptr, AD)) {

It is easier to read if you add comment for the boolean argument.


Comment at: lib/Sema/SemaDeclAttr.cpp:6554
@@ +6553,3 @@
+   ContextVersion, nullptr, AD)) {
+// All other diagnostic kinds have already been handled.
+if (AD != Sema::AD_Partial)

Can you mention where they are handled?


Comment at: lib/Sema/SemaExpr.cpp:110
@@ -113,1 +109,3 @@
+VersionTuple ContextVersion, std::string *Message,
+Sema::AvailabilityDiagnostic ) {
 

This looks like a refactoring of DiagnoseAvailabilityOfDecl. Is it possible to 
commit as a NFC patch?


Comment at: test/SemaObjC/unguarded-availability.m:93
@@ +92,3 @@
+  label2:
+goto label1; // expected-error{{cannot jump from this goto statement to 
its label}}
+  }

Should we emit error here? Or should we just ignore the tighter availability 
scope?


https://reviews.llvm.org/D23003



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

Re: [PATCH] D22045: [X86] Support of no_caller_saved_registers attribute (Clang part)

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.


Comment at: include/clang/CodeGen/CGFunctionInfo.h:479
@@ +478,3 @@
+  /// Whether this function saved caller registers.
+  unsigned NoCallerSavedRegs : 1;
+

erichkeane wrote:
> aaron.ballman wrote:
> > erichkeane wrote:
> > > aaron.ballman wrote:
> > > > This is unfortunate as it will bump the bit-field length to 33 bits, 
> > > > which seems rather wasteful. Are there any bits we can steal to bring 
> > > > this back down to a 32-bit bit-field?
> > > I implemented this additional patch, but don't really know a TON about 
> > > this area, so I have a few ideas but would like to get direction on it if 
> > > possible.  I had the following ideas of where to get a few more bits:
> > > 
> > > CallingConvention/EffectiveCallingConvention/ASTCallingConvention:  This 
> > > structure stores a pre-converted calling convention to the 
> > > llvm::CallingConv enum (llvm/include/llvm/IR/CallingConv.h).  I'll note 
> > > that the legal values for this go up to 1023 (so 8 bits isn't enough 
> > > anyway!), though only up to 91 are currently used.  
> > > 
> > > HOWEVER, the clang CallingConv (include/clang/Basic/Specifiers.h :233) 
> > > only has 16 items in it.  If we were instead to store THAT and convert 
> > > upon access (a simple switch statement, already used constructing this 
> > > value, see ClangCallConvToLLVMCallConv), we could get away with 6 or 7 
> > > bits each, saving this 3-6 bits total, yet have way more than enough room 
> > > for expansion.
> > > 
> > > HasRegParm: This field might be possible to eliminate.  According to the 
> > > GCC RegParm docs (I don't see a clang one?) here 
> > > (https://gcc.gnu.org/onlinedocs/gcc/x86-Function-Attributes.html), ", the 
> > > regparm attribute causes the compiler to pass arguments number one to 
> > > number if they are of integral type in registers EAX, EDX, and ECX 
> > > instead of on the stack".
> > > 
> > > It seems that 0 for "RegParm" should be illegal, so I wonder if we can 
> > > treat "RegParm==0" as "HasRegParm==false" and eliminate storing that.
> > > 
> > > In my opinion, the 1st one gives us way more room for the future and 
> > > corrects a possible future bug.  The 2nd is likely a lower touch, though 
> > > it could possibly change behavior (see discussion here 
> > > https://www.coreboot.org/pipermail/coreboot/2008-October/040406.html) as 
> > > regparm(0) is seemingly accepted by both compilers.  I DO note that this 
> > > comment notes that 'regparm 0' is the default behavior, so I'm not sure 
> > > what change that would do.
> > > 
> > > Either way, I suspect this change should be a separate commit (though I 
> > > would figure making it a pre-req for this patch would be the right way to 
> > > go).  If you give some guidance as to which you think would be better, I 
> > > can put a patch together.
> > > 
> > > -Erich
> > > 
> > I think that `unsigned ASTCallingConvention : 8;` can be safely reduced. 
> > This tracks a `clang::CallingConv` value, the maximum of which is 15. So I 
> > think the way to do this is to reduce ASTCallingConvention from 8 to 7 bits 
> > and then pack yours in as well.
> Ah! I missed that this was the case.  That said, it could likely be reduced 
> to 6 if we really wished (currently 16 items, 6 gives us room for 32).  
> Perhaps something to keep in our pocket for the next time someone needs a bit 
> or two here.
> 
> 
> I'll update the diff for Amjad if possible.
I'm on the fence about 6 vs 7 and see no harm in reducing it to either value, 
but have a *very* slight preference for 7 so that the bit-field grouping 
continues to add up to 32-bits. However, it's your call.


https://reviews.llvm.org/D22045



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

Re: [PATCH] D22045: [X86] Support of no_caller_saved_registers attribute (Clang part)

[Erich Keane via cfe-commits]

erichkeane added a comment.

Response on CGFucntionInfo.



Comment at: include/clang/CodeGen/CGFunctionInfo.h:479
@@ +478,3 @@
+  /// Whether this function saved caller registers.
+  unsigned NoCallerSavedRegs : 1;
+

aaron.ballman wrote:
> erichkeane wrote:
> > aaron.ballman wrote:
> > > This is unfortunate as it will bump the bit-field length to 33 bits, 
> > > which seems rather wasteful. Are there any bits we can steal to bring 
> > > this back down to a 32-bit bit-field?
> > I implemented this additional patch, but don't really know a TON about this 
> > area, so I have a few ideas but would like to get direction on it if 
> > possible.  I had the following ideas of where to get a few more bits:
> > 
> > CallingConvention/EffectiveCallingConvention/ASTCallingConvention:  This 
> > structure stores a pre-converted calling convention to the 
> > llvm::CallingConv enum (llvm/include/llvm/IR/CallingConv.h).  I'll note 
> > that the legal values for this go up to 1023 (so 8 bits isn't enough 
> > anyway!), though only up to 91 are currently used.  
> > 
> > HOWEVER, the clang CallingConv (include/clang/Basic/Specifiers.h :233) only 
> > has 16 items in it.  If we were instead to store THAT and convert upon 
> > access (a simple switch statement, already used constructing this value, 
> > see ClangCallConvToLLVMCallConv), we could get away with 6 or 7 bits each, 
> > saving this 3-6 bits total, yet have way more than enough room for 
> > expansion.
> > 
> > HasRegParm: This field might be possible to eliminate.  According to the 
> > GCC RegParm docs (I don't see a clang one?) here 
> > (https://gcc.gnu.org/onlinedocs/gcc/x86-Function-Attributes.html), ", the 
> > regparm attribute causes the compiler to pass arguments number one to 
> > number if they are of integral type in registers EAX, EDX, and ECX instead 
> > of on the stack".
> > 
> > It seems that 0 for "RegParm" should be illegal, so I wonder if we can 
> > treat "RegParm==0" as "HasRegParm==false" and eliminate storing that.
> > 
> > In my opinion, the 1st one gives us way more room for the future and 
> > corrects a possible future bug.  The 2nd is likely a lower touch, though it 
> > could possibly change behavior (see discussion here 
> > https://www.coreboot.org/pipermail/coreboot/2008-October/040406.html) as 
> > regparm(0) is seemingly accepted by both compilers.  I DO note that this 
> > comment notes that 'regparm 0' is the default behavior, so I'm not sure 
> > what change that would do.
> > 
> > Either way, I suspect this change should be a separate commit (though I 
> > would figure making it a pre-req for this patch would be the right way to 
> > go).  If you give some guidance as to which you think would be better, I 
> > can put a patch together.
> > 
> > -Erich
> > 
> I think that `unsigned ASTCallingConvention : 8;` can be safely reduced. This 
> tracks a `clang::CallingConv` value, the maximum of which is 15. So I think 
> the way to do this is to reduce ASTCallingConvention from 8 to 7 bits and 
> then pack yours in as well.
Ah! I missed that this was the case.  That said, it could likely be reduced to 
6 if we really wished (currently 16 items, 6 gives us room for 32).  Perhaps 
something to keep in our pocket for the next time someone needs a bit or two 
here.


I'll update the diff for Amjad if possible.


https://reviews.llvm.org/D22045



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

Re: [PATCH] D23160: [Coverage] Prevent creating a redundant counter if a nested body ends with a macro.

[Vedant Kumar via cfe-commits]

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

Thanks for catching this! I couldn't really reduce your test case any further. 
This LGTM.

I guess it never makes sense to have two regions with the exact same start/end 
loc, and different counters. Do you think we should add assertions in llvm 
(either in llvm-cov, or in the coverage reader) which guard against this?


https://reviews.llvm.org/D23160



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

Re: [PATCH] D22725: [clang-tidy] Add check 'modernize-use-algorithm'

[Jonas Devlieghere via cfe-commits]

JDevlieghere updated this revision to Diff 66835.
JDevlieghere marked 9 inline comments as done.
JDevlieghere added a comment.

- Added function pointer test case
- Used placeholders for diagnostics

I extended the matchers to include `::memcpy` and `::memset` as well because 
the check otherwise does not work on my mac because the `cstring` header that 
ships with Xcode is implemented as shown below.

  _LIBCPP_BEGIN_NAMESPACE_STD
  using ::size_t;
  using ::memcpy;
  using ::memset;
  ...
  _LIBCPP_END_NAMESPACE_STD

This is annoying as I have no way to discriminate functions that have the same 
signature. Unless there's a better solution, this seems like a reasonable 
trade-off to me. Of course I'm open to suggestions!

In https://reviews.llvm.org/D22725#505739, @aaron.ballman wrote:

> The tests added mostly cover them -- I elaborated on the function pointer 
> case, which I don't *think* will go wrong with this check, but should have a 
> pathological test case for just to be sure.


I added the test case. The call is indeed replaced, which I guess is fine?

In https://reviews.llvm.org/D22725#505476, @Prazek wrote:

> Did you manage to see what was wrong in the transformation that you did on 
> LLVM?


Not yet, thought it seemed to be related to `std::copy` rather than 
`std::fill`. I'm still trying to pinpoint the culprit but it's extremely time 
consuming as I have to recompile LLVM completely in order to run the tests...


Repository:
  rL LLVM

https://reviews.llvm.org/D22725

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/UseAlgorithmCheck.cpp
  clang-tidy/modernize/UseAlgorithmCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-use-algorithm.rst
  test/clang-tidy/modernize-use-algorithm.cpp

Index: test/clang-tidy/modernize-use-algorithm.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-algorithm.cpp
@@ -0,0 +1,139 @@
+// RUN: %check_clang_tidy %s modernize-use-algorithm %t
+// CHECK-FIXES: #include 
+
+namespace std {
+typedef unsigned int size_t;
+void *memcpy(void *dest, const void *src, std::size_t count);
+void *memset(void *dest, int ch, std::size_t count);
+
+template 
+OutputIt copy(InputIt first, InputIt last, OutputIt d_first);
+
+template 
+OutputIt move(InputIt first, InputIt last, OutputIt d_first);
+
+template 
+void fill(ForwardIt first, ForwardIt last, const T );
+}
+
+namespace awful {
+void memcpy(int, int, int);
+}
+
+typedef unsigned int size_t;
+void *memcpy(void *dest, const void *source, size_t size);
+
+void a() {
+  char foo[] = "foo", bar[4];
+  std::memcpy(bar, foo, sizeof bar);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm]
+  // CHECK-FIXES: std::copy(foo, foo + sizeof bar, bar);
+
+  void *baz = bar;
+  std::memcpy(baz, foo, sizeof bar);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm]
+
+  memcpy(bar, foo, sizeof bar);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm]
+  // CHECK-FIXES: std::copy(foo, foo + sizeof bar, bar);
+
+  std::copy(foo, foo + sizeof bar, bar);
+}
+
+void b() {
+  char foo[] = "foobar";
+  std::memset(foo, 'a', 3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memset' reduces type-safety, consider using 'std::fill' instead [modernize-use-algorithm]
+  // CHECK-FIXES: std::fill(foo, foo + 3, 'a');
+
+  std::memset(foo, 1, 3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memset' reduces type-safety, consider using 'std::fill' instead [modernize-use-algorithm]
+
+  std::fill(foo, foo + 2, 'a');
+}
+
+#define MEMCPY(dest, src) std::memcpy((dest), (src), sizeof(dest))
+void c() {
+  char foo[] = "foo", bar[3];
+  MEMCPY(bar, foo);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm]
+}
+
+void d() {
+  typedef char foo_t;
+  typedef char bar_t;
+  foo_t foo[] = "foo";
+  bar_t bar[4];
+  std::memcpy(bar, foo, sizeof bar);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm]
+  // CHECK-FIXES: std::copy(foo, foo + sizeof bar, bar);
+}
+
+void e() {
+  typedef const char *foo_t;
+  typedef const char *bar_t;
+  foo_t foo[] = {"foo", "bar"};
+  bar_t bar[2];
+  std::memcpy(bar, foo, sizeof bar);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'memcpy' reduces type-safety, consider using 'std::copy' instead [modernize-use-algorithm]
+  // CHECK-FIXES: std::copy(foo, foo + sizeof bar, bar);
+}
+
+void f() {
+  typedef int some_type;
+  typedef some_type *const *volatile *foo_ptr;
+
+  typedef int *const some_other_type;
+  typedef 

Re: [PATCH] D22045: [X86] Support of no_caller_saved_registers attribute (Clang part)

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.


Comment at: include/clang/CodeGen/CGFunctionInfo.h:479
@@ +478,3 @@
+  /// Whether this function saved caller registers.
+  unsigned NoCallerSavedRegs : 1;
+

erichkeane wrote:
> aaron.ballman wrote:
> > This is unfortunate as it will bump the bit-field length to 33 bits, which 
> > seems rather wasteful. Are there any bits we can steal to bring this back 
> > down to a 32-bit bit-field?
> I implemented this additional patch, but don't really know a TON about this 
> area, so I have a few ideas but would like to get direction on it if 
> possible.  I had the following ideas of where to get a few more bits:
> 
> CallingConvention/EffectiveCallingConvention/ASTCallingConvention:  This 
> structure stores a pre-converted calling convention to the llvm::CallingConv 
> enum (llvm/include/llvm/IR/CallingConv.h).  I'll note that the legal values 
> for this go up to 1023 (so 8 bits isn't enough anyway!), though only up to 91 
> are currently used.  
> 
> HOWEVER, the clang CallingConv (include/clang/Basic/Specifiers.h :233) only 
> has 16 items in it.  If we were instead to store THAT and convert upon access 
> (a simple switch statement, already used constructing this value, see 
> ClangCallConvToLLVMCallConv), we could get away with 6 or 7 bits each, saving 
> this 3-6 bits total, yet have way more than enough room for expansion.
> 
> HasRegParm: This field might be possible to eliminate.  According to the GCC 
> RegParm docs (I don't see a clang one?) here 
> (https://gcc.gnu.org/onlinedocs/gcc/x86-Function-Attributes.html), ", the 
> regparm attribute causes the compiler to pass arguments number one to number 
> if they are of integral type in registers EAX, EDX, and ECX instead of on the 
> stack".
> 
> It seems that 0 for "RegParm" should be illegal, so I wonder if we can treat 
> "RegParm==0" as "HasRegParm==false" and eliminate storing that.
> 
> In my opinion, the 1st one gives us way more room for the future and corrects 
> a possible future bug.  The 2nd is likely a lower touch, though it could 
> possibly change behavior (see discussion here 
> https://www.coreboot.org/pipermail/coreboot/2008-October/040406.html) as 
> regparm(0) is seemingly accepted by both compilers.  I DO note that this 
> comment notes that 'regparm 0' is the default behavior, so I'm not sure what 
> change that would do.
> 
> Either way, I suspect this change should be a separate commit (though I would 
> figure making it a pre-req for this patch would be the right way to go).  If 
> you give some guidance as to which you think would be better, I can put a 
> patch together.
> 
> -Erich
> 
I think that `unsigned ASTCallingConvention : 8;` can be safely reduced. This 
tracks a `clang::CallingConv` value, the maximum of which is 15. So I think the 
way to do this is to reduce ASTCallingConvention from 8 to 7 bits and then pack 
yours in as well.


https://reviews.llvm.org/D22045



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

Re: [PATCH] D23120: [OpenCL] Added underscores to the names of 'to_addr' OpenCL built-ins.

[Alexey Bader via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL277743: [OpenCL] Added underscores to the names of 'to_addr' 
OpenCL built-ins. (authored by bader).

Changed prior to commit:
  https://reviews.llvm.org/D23120?vs=66653=66832#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D23120

Files:
  cfe/trunk/lib/CodeGen/CGBuiltin.cpp
  cfe/trunk/test/CodeGenOpenCL/to_addr_builtin.cl

Index: cfe/trunk/test/CodeGenOpenCL/to_addr_builtin.cl
===
--- cfe/trunk/test/CodeGenOpenCL/to_addr_builtin.cl
+++ cfe/trunk/test/CodeGenOpenCL/to_addr_builtin.cl
@@ -14,74 +14,74 @@
   generic int *gen;
 
   //CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(1)* %{{.*}} to i8 addrspace(4)*
-  //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8 addrspace(4)* %[[ARG]])
+  //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @__to_global(i8 addrspace(4)* %[[ARG]])
   //CHECK: %{{.*}} = bitcast i8 addrspace(1)* %[[RET]] to i32 addrspace(1)*
   glob = to_global(glob);
   
   //CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(3)* %{{.*}} to i8 addrspace(4)*
-  //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8 addrspace(4)* %[[ARG]])
+  //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @__to_global(i8 addrspace(4)* %[[ARG]])
   //CHECK: %{{.*}} = bitcast i8 addrspace(1)* %[[RET]] to i32 addrspace(1)*
   glob = to_global(loc);
  
   //CHECK: %[[ARG:.*]] = addrspacecast i32* %{{.*}} to i8 addrspace(4)*
-  //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8 addrspace(4)* %[[ARG]])
+  //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @__to_global(i8 addrspace(4)* %[[ARG]])
   //CHECK: %{{.*}} = bitcast i8 addrspace(1)* %[[RET]] to i32 addrspace(1)*
   glob = to_global(priv);
  
   //CHECK: %[[ARG:.*]] = bitcast i32 addrspace(4)* %{{.*}} to i8 addrspace(4)*
-  //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8 addrspace(4)* %[[ARG]])
+  //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @__to_global(i8 addrspace(4)* %[[ARG]])
   //CHECK: %{{.*}} = bitcast i8 addrspace(1)* %[[RET]] to i32 addrspace(1)*
   glob = to_global(gen);
   
   //CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(1)* %{{.*}} to i8 addrspace(4)*
-  //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @to_local(i8 addrspace(4)* %[[ARG]])
+  //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @__to_local(i8 addrspace(4)* %[[ARG]])
   //CHECK: %{{.*}} = bitcast i8 addrspace(3)* %[[RET]] to i32 addrspace(3)*
   loc = to_local(glob);
 
   //CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(3)* %{{.*}} to i8 addrspace(4)*
-  //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @to_local(i8 addrspace(4)* %[[ARG]])
+  //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @__to_local(i8 addrspace(4)* %[[ARG]])
   //CHECK: %{{.*}} = bitcast i8 addrspace(3)* %[[RET]] to i32 addrspace(3)*
   loc = to_local(loc);
 
   //CHECK: %[[ARG:.*]] = addrspacecast i32* %{{.*}} to i8 addrspace(4)*
-  //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @to_local(i8 addrspace(4)* %[[ARG]])
+  //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @__to_local(i8 addrspace(4)* %[[ARG]])
   //CHECK: %{{.*}} = bitcast i8 addrspace(3)* %[[RET]] to i32 addrspace(3)*
   loc = to_local(priv);
 
   //CHECK: %[[ARG:.*]] = bitcast i32 addrspace(4)* %{{.*}} to i8 addrspace(4)*
-  //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @to_local(i8 addrspace(4)* %[[ARG]])
+  //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @__to_local(i8 addrspace(4)* %[[ARG]])
   //CHECK: %{{.*}} = bitcast i8 addrspace(3)* %[[RET]] to i32 addrspace(3)*
   loc = to_local(gen);
 
   //CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(1)* %{{.*}} to i8 addrspace(4)*
-  //CHECK: %[[RET:.*]] = call i8* @to_private(i8 addrspace(4)* %[[ARG]])
+  //CHECK: %[[RET:.*]] = call i8* @__to_private(i8 addrspace(4)* %[[ARG]])
   //CHECK: %{{.*}} = bitcast i8* %[[RET]] to i32*
   priv = to_private(glob);
 
   //CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(3)* %{{.*}} to i8 addrspace(4)*
-  //CHECK: %[[RET:.*]] = call i8* @to_private(i8 addrspace(4)* %[[ARG]])
+  //CHECK: %[[RET:.*]] = call i8* @__to_private(i8 addrspace(4)* %[[ARG]])
   //CHECK: %{{.*}} = bitcast i8* %[[RET]] to i32*
   priv = to_private(loc);
 
   //CHECK: %[[ARG:.*]] = addrspacecast i32* %{{.*}} to i8 addrspace(4)*
-  //CHECK: %[[RET:.*]] = call i8* @to_private(i8 addrspace(4)* %[[ARG]])
+  //CHECK: %[[RET:.*]] = call i8* @__to_private(i8 addrspace(4)* %[[ARG]])
   //CHECK: %{{.*}} = bitcast i8* %[[RET]] to i32*
   priv = to_private(priv);
 
   //CHECK: %[[ARG:.*]] = bitcast i32 addrspace(4)* %{{.*}} to i8 addrspace(4)*
-  //CHECK: %[[RET:.*]] = call i8* @to_private(i8 addrspace(4)* %[[ARG]])
+  //CHECK: %[[RET:.*]] = call i8* @__to_private(i8 addrspace(4)* %[[ARG]])
   //CHECK: %{{.*}} = bitcast i8* %[[RET]] to i32*
   priv = to_private(gen);
 
   //CHECK: %[[ARG:.*]] = addrspacecast %[[A]]* %{{.*}} to i8 addrspace(4)*
-  //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8 addrspace(4)* %[[ARG]])
+  //CHECK: 

r277743 - [OpenCL] Added underscores to the names of 'to_addr' OpenCL built-ins.

[Alexey Bader via cfe-commits]

Author: bader
Date: Thu Aug  4 13:06:27 2016
New Revision: 277743

URL: http://llvm.org/viewvc/llvm-project?rev=277743=rev
Log:
[OpenCL] Added underscores to the names of 'to_addr' OpenCL built-ins.

Summary:
In order to re-define OpenCL built-in functions
'to_{private,local,global}' in OpenCL run-time library LLVM names must
be different from the clang built-in function names.

Reviewers: yaxunl, Anastasia

Subscribers: cfe-commits

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

Modified:
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/test/CodeGenOpenCL/to_addr_builtin.cl

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=277743=277742=277743=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Thu Aug  4 13:06:27 2016
@@ -2209,8 +2209,9 @@ RValue CodeGenFunction::EmitBuiltinExpr(
   NewArg = Builder.CreateAddrSpaceCast(Arg0, NewArgT);
 else
   NewArg = Builder.CreateBitOrPointerCast(Arg0, NewArgT);
-auto NewCall = Builder.CreateCall(CGM.CreateRuntimeFunction(FTy,
-  E->getDirectCallee()->getName()), {NewArg});
+auto NewName = std::string("__") + E->getDirectCallee()->getName().str();
+auto NewCall =
+Builder.CreateCall(CGM.CreateRuntimeFunction(FTy, NewName), {NewArg});
 return RValue::get(Builder.CreateBitOrPointerCast(NewCall,
   ConvertType(E->getType(;
   }

Modified: cfe/trunk/test/CodeGenOpenCL/to_addr_builtin.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/to_addr_builtin.cl?rev=277743=277742=277743=diff
==
--- cfe/trunk/test/CodeGenOpenCL/to_addr_builtin.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/to_addr_builtin.cl Thu Aug  4 13:06:27 2016
@@ -14,74 +14,74 @@ void test(void) {
   generic int *gen;
 
   //CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(1)* %{{.*}} to i8 
addrspace(4)*
-  //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8 addrspace(4)* 
%[[ARG]])
+  //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @__to_global(i8 addrspace(4)* 
%[[ARG]])
   //CHECK: %{{.*}} = bitcast i8 addrspace(1)* %[[RET]] to i32 addrspace(1)*
   glob = to_global(glob);
   
   //CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(3)* %{{.*}} to i8 
addrspace(4)*
-  //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8 addrspace(4)* 
%[[ARG]])
+  //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @__to_global(i8 addrspace(4)* 
%[[ARG]])
   //CHECK: %{{.*}} = bitcast i8 addrspace(1)* %[[RET]] to i32 addrspace(1)*
   glob = to_global(loc);
  
   //CHECK: %[[ARG:.*]] = addrspacecast i32* %{{.*}} to i8 addrspace(4)*
-  //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8 addrspace(4)* 
%[[ARG]])
+  //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @__to_global(i8 addrspace(4)* 
%[[ARG]])
   //CHECK: %{{.*}} = bitcast i8 addrspace(1)* %[[RET]] to i32 addrspace(1)*
   glob = to_global(priv);
  
   //CHECK: %[[ARG:.*]] = bitcast i32 addrspace(4)* %{{.*}} to i8 addrspace(4)*
-  //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @to_global(i8 addrspace(4)* 
%[[ARG]])
+  //CHECK: %[[RET:.*]] = call i8 addrspace(1)* @__to_global(i8 addrspace(4)* 
%[[ARG]])
   //CHECK: %{{.*}} = bitcast i8 addrspace(1)* %[[RET]] to i32 addrspace(1)*
   glob = to_global(gen);
   
   //CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(1)* %{{.*}} to i8 
addrspace(4)*
-  //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @to_local(i8 addrspace(4)* 
%[[ARG]])
+  //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @__to_local(i8 addrspace(4)* 
%[[ARG]])
   //CHECK: %{{.*}} = bitcast i8 addrspace(3)* %[[RET]] to i32 addrspace(3)*
   loc = to_local(glob);
 
   //CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(3)* %{{.*}} to i8 
addrspace(4)*
-  //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @to_local(i8 addrspace(4)* 
%[[ARG]])
+  //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @__to_local(i8 addrspace(4)* 
%[[ARG]])
   //CHECK: %{{.*}} = bitcast i8 addrspace(3)* %[[RET]] to i32 addrspace(3)*
   loc = to_local(loc);
 
   //CHECK: %[[ARG:.*]] = addrspacecast i32* %{{.*}} to i8 addrspace(4)*
-  //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @to_local(i8 addrspace(4)* 
%[[ARG]])
+  //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @__to_local(i8 addrspace(4)* 
%[[ARG]])
   //CHECK: %{{.*}} = bitcast i8 addrspace(3)* %[[RET]] to i32 addrspace(3)*
   loc = to_local(priv);
 
   //CHECK: %[[ARG:.*]] = bitcast i32 addrspace(4)* %{{.*}} to i8 addrspace(4)*
-  //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @to_local(i8 addrspace(4)* 
%[[ARG]])
+  //CHECK: %[[RET:.*]] = call i8 addrspace(3)* @__to_local(i8 addrspace(4)* 
%[[ARG]])
   //CHECK: %{{.*}} = bitcast i8 addrspace(3)* %[[RET]] to i32 addrspace(3)*
   loc = to_local(gen);
 
   //CHECK: %[[ARG:.*]] = addrspacecast i32 addrspace(1)* %{{.*}} to i8 
addrspace(4)*
-  

Re: [PATCH] D23086: [OpenCL] Generate concrete struct type for ndrange_t

[Yaxun Liu via cfe-commits]

yaxunl added inline comments.


Comment at: lib/CodeGen/CGOpenCLRuntime.cpp:43
@@ +42,3 @@
+
+  return llvm::StructType::create(EleTypes, "ndrange_t");
+}

Anastasia wrote:
> yaxunl wrote:
> > yaxunl wrote:
> > > struct name should be "struct.ndrange_t" to allow library code to access 
> > > it.
> > Sorry, should be "struct.__ndrange_t" to avoid conflict with builtin type 
> > ndrange_t.
> Is there any conflict really? I think it should be Ok to keep 
> "struct.ndrange_t", since we transform it to a struct and don't declare as 
> struct.
ndrange_t is defined as a builtin type in Clang, so library developer cannot 
implement it as a concrete type, but they can implement `__ndrange_t`. This is 
similar to the case of `as_type`.


Repository:
  rL LLVM

https://reviews.llvm.org/D23086



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

Re: [PATCH] D23120: [OpenCL] Added underscores to the names of 'to_addr' OpenCL built-ins.

[Anastasia Stulova via cfe-commits]

Anastasia accepted this revision.
Anastasia added a comment.

LGTM! Thanks!


https://reviews.llvm.org/D23120



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

Re: [PATCH] D22045: [X86] Support of no_caller_saved_registers attribute (Clang part)

[Erich Keane via cfe-commits]

erichkeane added a comment.

Response on CGFucntionInfo.



Comment at: include/clang/CodeGen/CGFunctionInfo.h:479
@@ +478,3 @@
+  /// Whether this function saved caller registers.
+  unsigned NoCallerSavedRegs : 1;
+

aaron.ballman wrote:
> This is unfortunate as it will bump the bit-field length to 33 bits, which 
> seems rather wasteful. Are there any bits we can steal to bring this back 
> down to a 32-bit bit-field?
I implemented this additional patch, but don't really know a TON about this 
area, so I have a few ideas but would like to get direction on it if possible.  
I had the following ideas of where to get a few more bits:

CallingConvention/EffectiveCallingConvention/ASTCallingConvention:  This 
structure stores a pre-converted calling convention to the llvm::CallingConv 
enum (llvm/include/llvm/IR/CallingConv.h).  I'll note that the legal values for 
this go up to 1023 (so 8 bits isn't enough anyway!), though only up to 91 are 
currently used.  

HOWEVER, the clang CallingConv (include/clang/Basic/Specifiers.h :233) only has 
16 items in it.  If we were instead to store THAT and convert upon access (a 
simple switch statement, already used constructing this value, see 
ClangCallConvToLLVMCallConv), we could get away with 6 or 7 bits each, saving 
this 3-6 bits total, yet have way more than enough room for expansion.

HasRegParm: This field might be possible to eliminate.  According to the GCC 
RegParm docs (I don't see a clang one?) here 
(https://gcc.gnu.org/onlinedocs/gcc/x86-Function-Attributes.html), ", the 
regparm attribute causes the compiler to pass arguments number one to number if 
they are of integral type in registers EAX, EDX, and ECX instead of on the 
stack".

It seems that 0 for "RegParm" should be illegal, so I wonder if we can treat 
"RegParm==0" as "HasRegParm==false" and eliminate storing that.

In my opinion, the 1st one gives us way more room for the future and corrects a 
possible future bug.  The 2nd is likely a lower touch, though it could possibly 
change behavior (see discussion here 
https://www.coreboot.org/pipermail/coreboot/2008-October/040406.html) as 
regparm(0) is seemingly accepted by both compilers.  I DO note that this 
comment notes that 'regparm 0' is the default behavior, so I'm not sure what 
change that would do.

Either way, I suspect this change should be a separate commit (though I would 
figure making it a pre-req for this patch would be the right way to go).  If 
you give some guidance as to which you think would be better, I can put a patch 
together.

-Erich



https://reviews.llvm.org/D22045



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

Re: [PATCH] D23168: emit_DW_AT_noreturn flag

[Victor via cfe-commits]

vleschuk updated this revision to Diff 66825.
vleschuk added a comment.

More context. Slight formatting changes.


https://reviews.llvm.org/D23168

Files:
  lib/AST/Decl.cpp
  lib/CodeGen/CGDebugInfo.cpp

Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -2641,6 +2641,9 @@
   llvm::DIScope *Mod = getParentModuleOrNull(RDecl);
   FDContext = getContextDescriptor(RDecl, Mod ? Mod : TheCU);
 }
+// Check if it is a noreturn-marked function
+if (FD->isNoReturn())
+  Flags |= llvm::DINode::FlagNoReturn;
 // Collect template parameters.
 TParamsArray = CollectFunctionTemplateParams(FD, Unit);
   }
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -2653,9 +2653,13 @@
 }
 
 bool FunctionDecl::isNoReturn() const {
-  return hasAttr() || hasAttr() ||
- hasAttr() ||
- getType()->getAs()->getNoReturnAttr();
+  bool HasNoReturnAttr = hasAttr() || 
hasAttr()
+ || hasAttr();
+  const auto *FuncType = getType()->getAs();
+  bool TypeHasNoReturnAttr = false;
+  if (FuncType)
+TypeHasNoReturnAttr = FuncType->getNoReturnAttr();
+  return HasNoReturnAttr || TypeHasNoReturnAttr;
 }
 
 void


Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -2641,6 +2641,9 @@
   llvm::DIScope *Mod = getParentModuleOrNull(RDecl);
   FDContext = getContextDescriptor(RDecl, Mod ? Mod : TheCU);
 }
+// Check if it is a noreturn-marked function
+if (FD->isNoReturn())
+  Flags |= llvm::DINode::FlagNoReturn;
 // Collect template parameters.
 TParamsArray = CollectFunctionTemplateParams(FD, Unit);
   }
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -2653,9 +2653,13 @@
 }
 
 bool FunctionDecl::isNoReturn() const {
-  return hasAttr() || hasAttr() ||
- hasAttr() ||
- getType()->getAs()->getNoReturnAttr();
+  bool HasNoReturnAttr = hasAttr() || hasAttr()
+ || hasAttr();
+  const auto *FuncType = getType()->getAs();
+  bool TypeHasNoReturnAttr = false;
+  if (FuncType)
+TypeHasNoReturnAttr = FuncType->getNoReturnAttr();
+  return HasNoReturnAttr || TypeHasNoReturnAttr;
 }
 
 void
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D23167: emit_DW_AT_noreturn flag

[Adrian Prantl via cfe-commits]

aprantl added a comment.

Thanks for working in this!
Technically this is a DWARF 5 feature, but unknown attributes can be ignored by 
consumers, so there's no reason to emit it conditionally.

Could you please also add a test for llvm-dwarfdump?
Could you please also add a textual LLVM IR parser testcase?



Comment at: include/llvm/IR/DebugInfoMetadata.h:1424
@@ +1423,3 @@
+  ///
+  /// Return true if this subprogram is C++11 noreturn or C11 _Noreturn
+  unsigned isNoReturn() const {

The \brief is redundant. We compile doxygen with autobrief now.
Also, there's a missing . at the end.


https://reviews.llvm.org/D23167



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

Re: [PATCH] D22045: [X86] Support of no_caller_saved_registers attribute (Clang part)

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.


Comment at: include/clang/Basic/Attr.td:1674
@@ +1673,3 @@
+   TargetSpecificAttr {
+  let Spellings = [GNU<"no_caller_saved_registers">];
+  let Subjects = SubjectList<[FunctionLike], WarnDiag, "ExpectedFunction">;

Yes, though interrupt should be handled in a separate patch since it's a 
logically separate change.


Comment at: include/clang/Basic/AttrDocs.td:2205
@@ +2204,3 @@
+Use this attribute to indicate that the specified function has no
+caller-saved registers.  That is, all registers are callee-saved.
+The compiler generates proper function entry and exit sequences to

> Please, correct me if I am mistaken, but calling convention determine not 
> only what the callee and the caller function saved registers are, but also 
> how we move parameters to the called function, right?

Correct.

> The idea behind no_caller_saved_registers attribute, is that it can be 
> combined with any other calling convention, and it only override the 
> callee/caller saved register list, but not the way parameters are passed to 
> the called function.
>
> So, using a calling convention for this attribute will not be right.

Okay, that makes sense to me, but we should document how this interacts with 
calling convention attributes more clearly. Specifically, we should call out 
that this does not affect the calling convention and give an example of what 
gets generated for a function that combines a caller-saved calling convention 
with this attribute.


Comment at: include/clang/CodeGen/CGFunctionInfo.h:479
@@ +478,3 @@
+  /// Whether this function saved caller registers.
+  unsigned NoCallerSavedRegs : 1;
+

This is unfortunate as it will bump the bit-field length to 33 bits, which 
seems rather wasteful. Are there any bits we can steal to bring this back down 
to a 32-bit bit-field?


https://reviews.llvm.org/D22045



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

Re: [PATCH] D23168: emit_DW_AT_noreturn flag

[David Majnemer via cfe-commits]

majnemer added a subscriber: majnemer.
majnemer requested changes to this revision.
majnemer added a reviewer: majnemer.
majnemer added a comment.
This revision now requires changes to proceed.

This has no tests.


https://reviews.llvm.org/D23168



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

Re: [PATCH] D23158: [clang-rename] merge tests when possible

[Kirill Bobyrev via cfe-commits]

omtcyfz updated this revision to Diff 66818.
omtcyfz added a comment.

Make testset great again.


https://reviews.llvm.org/D23158

Files:
  test/clang-rename/ClassAsTemplateArgument.cpp
  test/clang-rename/ClassAsTemplateArgumentFindByClass.cpp
  test/clang-rename/ClassAsTemplateArgumentFindByTemplateArgument.cpp
  test/clang-rename/ClassFindByName.cpp
  test/clang-rename/ClassNameInFunctionDefinition.cpp
  test/clang-rename/ClassSimpleRenaming.cpp
  test/clang-rename/ClassTestMulti.cpp
  test/clang-rename/ClassTestMultiByName.cpp
  test/clang-rename/ComplexFunctionOverride.cpp
  test/clang-rename/ComplicatedClassType.cpp
  test/clang-rename/ConstCastExpr.cpp
  test/clang-rename/ConstructExpr.cpp
  test/clang-rename/Ctor.cpp
  test/clang-rename/CtorFindByDeclaration.cpp
  test/clang-rename/CtorFindByDefinition.cpp
  test/clang-rename/CtorInitializer.cpp
  test/clang-rename/DeclRefExpr.cpp
  test/clang-rename/Dtor.cpp
  test/clang-rename/DtorDeclaration.cpp
  test/clang-rename/DtorDefinition.cpp
  test/clang-rename/DynamicCastExpr.cpp
  test/clang-rename/Field.cpp
  test/clang-rename/FunctionMacro.cpp
  test/clang-rename/FunctionOverride.cpp
  test/clang-rename/FunctionWithClassFindByName.cpp
  test/clang-rename/MemberExprMacro.cpp
  test/clang-rename/Namespace.cpp
  test/clang-rename/NoNewName.cpp
  test/clang-rename/ReinterpretCastExpr.cpp
  test/clang-rename/StaticCastExpr.cpp
  test/clang-rename/TemplateClassInstantiation.cpp
  test/clang-rename/TemplateFunction.cpp
  test/clang-rename/TemplateFunctionFindByDeclaration.cpp
  test/clang-rename/TemplateFunctionFindByUse.cpp
  test/clang-rename/TemplateTypename.cpp
  test/clang-rename/TemplateTypenameFindByTemplateParam.cpp
  test/clang-rename/TemplateTypenameFindByTypeInside.cpp
  test/clang-rename/UserDefinedConversion.cpp
  test/clang-rename/UserDefinedConversionFindByTypeDeclaration.cpp
  test/clang-rename/Variable.cpp
  test/clang-rename/VariableMacro.cpp

Index: test/clang-rename/VariableMacro.cpp
===
--- test/clang-rename/VariableMacro.cpp
+++ test/clang-rename/VariableMacro.cpp
@@ -1,18 +1,15 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=208 -new-name=Z %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-
-#define Y X // CHECK: #define Y Z
+#define Baz Foo // CHECK: #define Baz Bar
 
 void foo(int value) {}
 
 void macro() {
-  int X;// CHECK: int Z;
-  X = 42;   // CHECK: Z = 42;
-  Y -= 0;
-  foo(X);   // CHECK: foo(Z);
-  foo(Y);
+  int Foo;// CHECK: int Bar;
+  Foo = 42;   // CHECK: Bar = 42;
+  Baz -= 0;
+  foo(Foo);   // CHECK: foo(Bar);
+  foo(Baz);
 }
 
-// Use grep -FUbo 'foo;'  to get the correct offset of foo when changing
-// this file.
+// RUN: clang-rename -offset=88 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// RUN: clang-rename -offset=117 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// RUN: clang-rename -offset=167 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
Index: test/clang-rename/Variable.cpp
===
--- test/clang-rename/Variable.cpp
+++ test/clang-rename/Variable.cpp
@@ -1,7 +1,3 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=148 -new-name=Bar %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-
 namespace A {
 int Foo;// CHECK: int Bar;
 }
@@ -23,5 +19,7 @@
   Foo = b.Foo;  // Foo = b.Foo;
 }
 
-// Use grep -FUbo 'Foo'  to get the correct offset of foo when changing
-// this file.
+// RUN: clang-rename -offset=18 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// RUN: clang-rename -offset=164 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// RUN: clang-rename -offset=487 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// RUN: clang-rename -offset=535 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
Index: test/clang-rename/UserDefinedConversionFindByTypeDeclaration.cpp
===
--- test/clang-rename/UserDefinedConversionFindByTypeDeclaration.cpp
+++ /dev/null
@@ -1,26 +0,0 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=136 -new-name=Bar %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-
-class Foo { // CHECK: class Bar {
-//^ offset must be here
-public:
-  Foo() {}  // CHECK: Bar() {}
-};
-
-class Baz {
-public:
-  operator Foo() const {// CHECK: operator Bar() const {
-Foo foo;// CHECK: Bar foo;
-return foo;
-  }
-};
-
-int main() {
-  Baz boo;
-  Foo foo = static_cast(boo);  // CHECK: Bar foo = static_cast(boo);
-  return 0;
-}
-
-// Use grep -FUbo 'Foo'  to get the correct offset of Cla when changing
-// this file.
Index: test/clang-rename/UserDefinedConversion.cpp
===
--- test/clang-rename/UserDefinedConversion.cpp
+++ 

Re: [PATCH] D23004: [ASTMatchers] Add matchers canReferToDecl() and hasUnderlyingDecl()

[Martin Böhme via cfe-commits]

mboehme updated this revision to Diff 66815.
mboehme added a comment.

Removed superfluous parentheses


https://reviews.llvm.org/D23004

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  lib/ASTMatchers/Dynamic/Registry.cpp
  unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -241,6 +241,21 @@
 hasDeclaration(namedDecl(hasName("A";
 }
 
+TEST(HasUnderlyingDecl, Matches) {
+  EXPECT_TRUE(matches("namespace N { template  void f(T t); }"
+  "template  void g() { using N::f; f(T()); }",
+  unresolvedLookupExpr(canReferToDecl(
+  namedDecl(hasUnderlyingDecl(hasName("::N::f")));
+  EXPECT_TRUE(matches(
+  "namespace N { template  void f(T t); }"
+  "template  void g() { N::f(T()); }",
+  unresolvedLookupExpr(canReferToDecl(namedDecl(hasName("::N::f"));
+  EXPECT_TRUE(notMatches(
+  "namespace N { template  void f(T t); }"
+  "template  void g() { using N::f; f(T()); }",
+  unresolvedLookupExpr(canReferToDecl(namedDecl(hasName("::N::f"));
+}
+
 TEST(HasType, TakesQualTypeMatcherAndMatchesExpr) {
   TypeMatcher ClassX = hasDeclaration(recordDecl(hasName("X")));
   EXPECT_TRUE(
@@ -2072,5 +2087,24 @@
   EXPECT_TRUE(notMatches(Code2, ForEachOverriddenInClass("A1")));
 }
 
+TEST(Matcher, CanReferToDecl) {
+  std::string Fragment = "void foo(int p1);"
+ "void foo(int *p2);"
+ "void bar(int p3);"
+ "template  void baz(T t) { foo(t); }";
+
+  EXPECT_TRUE(
+  matches(Fragment, unresolvedLookupExpr(canReferToDecl(functionDecl(
+hasParameter(0, parmVarDecl(hasName("p1";
+  EXPECT_TRUE(
+  matches(Fragment, unresolvedLookupExpr(canReferToDecl(functionDecl(
+hasParameter(0, parmVarDecl(hasName("p2";
+  EXPECT_TRUE(
+  notMatches(Fragment, unresolvedLookupExpr(canReferToDecl(functionDecl(
+   hasParameter(0, parmVarDecl(hasName("p3";
+  EXPECT_TRUE(notMatches(Fragment, unresolvedLookupExpr(canReferToDecl(
+   functionDecl(hasName("bar"));
+}
+
 } // namespace ast_matchers
 } // namespace clang
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -115,6 +115,7 @@
   REGISTER_MATCHER(breakStmt);
   REGISTER_MATCHER(builtinType);
   REGISTER_MATCHER(callExpr);
+  REGISTER_MATCHER(canReferToDecl);
   REGISTER_MATCHER(caseStmt);
   REGISTER_MATCHER(castExpr);
   REGISTER_MATCHER(characterLiteral);
@@ -265,6 +266,7 @@
   REGISTER_MATCHER(hasTypeLoc);
   REGISTER_MATCHER(hasUnaryOperand);
   REGISTER_MATCHER(hasUnarySelector);
+  REGISTER_MATCHER(hasUnderlyingDecl);
   REGISTER_MATCHER(hasValueType);
   REGISTER_MATCHER(ifStmt);
   REGISTER_MATCHER(ignoringImplicit);
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -2468,6 +2468,25 @@
   void(internal::HasDeclarationSupportedTypes)>(InnerMatcher);
 }
 
+/// \brief Matches a \c NamedDecl whose underlying declaration matches the given
+/// matcher.
+///
+/// Given
+/// \code
+///   namespace N { template void f(T t); }
+///   template  void g() { using N::f; f(T()); }
+/// \endcode
+/// \c unresolvedLookupExpr(canReferToDecl(
+/// namedDecl(hasUnderlyingDecl(hasName("::N::f")
+///   matches the use of \c f in \c g() .
+AST_MATCHER_P(NamedDecl, hasUnderlyingDecl, internal::Matcher,
+  InnerMatcher) {
+  const NamedDecl *UnderlyingDecl = Node.getUnderlyingDecl();
+
+  return UnderlyingDecl != nullptr &&
+ InnerMatcher.matches(*UnderlyingDecl, Finder, Builder);
+}
+
 /// \brief Matches on the implicit object argument of a member call expression.
 ///
 /// Example matches y.x()
@@ -2823,6 +2842,26 @@
   return false;
 }
 
+/// \brief Matches an \c OverloadExpr if any of the declarations in the set of
+/// overloads matches the given matcher.
+///
+/// Given
+/// \code
+///   template  void foo(T);
+///   template  void bar(T);
+///   template  void baz(T t) {
+/// foo(t);
+/// bar(t);
+///   }
+/// \endcode
+/// unresolvedLookupExpr(canReferToDecl(functionTemplateDecl(hasName("foo"
+///   matches \c foo in \c foo(t); but not \c bar in \c bar(t);
+AST_MATCHER_P(OverloadExpr, canReferToDecl, internal::Matcher,
+  InnerMatcher) {
+  return matchesFirstInPointerRange(InnerMatcher, Node.decls_begin(),
+   

[PATCH] D23168: emit_DW_AT_noreturn flag

[Victor via cfe-commits]

vleschuk created this revision.
vleschuk added a reviewer: asl.
vleschuk added a subscriber: cfe-commits.

Emit DWARF DW_AT_noreturn for C++ [[ noreturn ]] and C _Noreturn specifiers.

Corresponding LLVM patch: https://reviews.llvm.org/D23167


https://reviews.llvm.org/D23168

Files:
  lib/AST/Decl.cpp
  lib/CodeGen/CGDebugInfo.cpp

Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -2641,6 +2641,9 @@
   llvm::DIScope *Mod = getParentModuleOrNull(RDecl);
   FDContext = getContextDescriptor(RDecl, Mod ? Mod : TheCU);
 }
+// Check if it is a noreturn-marked function
+if (FD->isNoReturn())
+  Flags |= llvm::DINode::FlagNoReturn;
 // Collect template parameters.
 TParamsArray = CollectFunctionTemplateParams(FD, Unit);
   }
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -2653,9 +2653,12 @@
 }
 
 bool FunctionDecl::isNoReturn() const {
-  return hasAttr() || hasAttr() ||
- hasAttr() ||
- getType()->getAs()->getNoReturnAttr();
+  bool HasNoReturnAttr = hasAttr() || 
hasAttr() || hasAttr();
+  const auto *FuncType = getType()->getAs();
+  bool TypeHasNoReturnAttr = false;
+  if (FuncType)
+TypeHasNoReturnAttr = FuncType->getNoReturnAttr();
+  return HasNoReturnAttr || TypeHasNoReturnAttr;
 }
 
 void


Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -2641,6 +2641,9 @@
   llvm::DIScope *Mod = getParentModuleOrNull(RDecl);
   FDContext = getContextDescriptor(RDecl, Mod ? Mod : TheCU);
 }
+// Check if it is a noreturn-marked function
+if (FD->isNoReturn())
+  Flags |= llvm::DINode::FlagNoReturn;
 // Collect template parameters.
 TParamsArray = CollectFunctionTemplateParams(FD, Unit);
   }
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -2653,9 +2653,12 @@
 }
 
 bool FunctionDecl::isNoReturn() const {
-  return hasAttr() || hasAttr() ||
- hasAttr() ||
- getType()->getAs()->getNoReturnAttr();
+  bool HasNoReturnAttr = hasAttr() || hasAttr() || hasAttr();
+  const auto *FuncType = getType()->getAs();
+  bool TypeHasNoReturnAttr = false;
+  if (FuncType)
+TypeHasNoReturnAttr = FuncType->getNoReturnAttr();
+  return HasNoReturnAttr || TypeHasNoReturnAttr;
 }
 
 void
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D23004: [ASTMatchers] Add matchers canReferToDecl() and hasUnderlyingDecl()

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.


Comment at: include/clang/ASTMatchers/ASTMatchers.h:2855
@@ +2854,3 @@
+/// bar(t);
+///   }
+/// \endcode

alexfh wrote:
> mboehme wrote:
> > > The documentation doesn't describe what the matcher does (can you please 
> > > clarify the docs?).
> > 
> > I've rephrased the description of the matcher -- is it clearer now?
> > 
> > > The implementation suggests that this is looking to see if the given decl 
> > > exists in the overload expression set, which makes me wonder why this 
> > > isn't implemented on the hasDeclaration() traversal matcher rather than 
> > > adding a new matcher name?
> > 
> > As klimek notes, the difference is that hasDeclaration() is currently used 
> > only for nodes that have exactly one associated declaration.
> > 
> > My proposal would be to stay with canReferToDecl -- thoughts?
> +1 to `canReferToDecl`
`canReferToDecl` doesn't make it any more clear that there could be multiple 
candidates, and I find its usage to read really strangely. However, it is a 
good point about `hasDeclaration` usage being singular currently. My preference 
is for `hasAnyDeclaration`, and the precedence is `hasAnyName`, 
`hasAnyConstructorInitializer`, `hasAnyArgument`, etc.

The documentation does read better now, thank you!


https://reviews.llvm.org/D23004



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

Re: [PATCH] D22045: [X86] Support of no_caller_saved_registers attribute (Clang part)

[Amjad Aboud via cfe-commits]

aaboud updated this revision to Diff 66810.
aaboud added a comment.

Made "no_caller_saved_registers" part of function prototype.
This will allow Clang to yell a warning when there is a mismatch between 
function pointer and assigned function value.

Thanks to Erich for implementing this addition change.


https://reviews.llvm.org/D22045

Files:
  include/clang/AST/Type.h
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/CodeGen/CGFunctionInfo.h
  include/clang/Sema/Sema.h
  lib/AST/ASTContext.cpp
  lib/AST/TypePrinter.cpp
  lib/CodeGen/CGCall.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  lib/Sema/SemaType.cpp
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  test/CodeGenCXX/attr-x86-no_caller_saved_registers.cpp
  test/SemaCXX/attr-x86-no_caller_saved_registers.cpp

Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -221,6 +221,7 @@
   // FIXME: need to stabilize encoding of calling convention...
   Record.push_back(C.getCC());
   Record.push_back(C.getProducesResult());
+  Record.push_back(C.getNoCallerSavedRegs());
 
   if (C.getHasRegParm() || C.getRegParm() || C.getProducesResult())
 AbbrevToUse = 0;
@@ -731,6 +732,7 @@
   Abv->Add(BitCodeAbbrevOp(0)); // RegParm
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 4)); // CC
   Abv->Add(BitCodeAbbrevOp(0)); // ProducesResult
+  Abv->Add(BitCodeAbbrevOp(0)); // NoCallerSavedRegs
   // FunctionProtoType
   Abv->Add(BitCodeAbbrevOp(0)); // IsVariadic
   Abv->Add(BitCodeAbbrevOp(0)); // HasTrailingReturn
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -5368,13 +5368,14 @@
   }
 
   case TYPE_FUNCTION_NO_PROTO: {
-if (Record.size() != 6) {
+if (Record.size() != 7) {
   Error("incorrect encoding of no-proto function type");
   return QualType();
 }
 QualType ResultType = readType(*Loc.F, Record, Idx);
 FunctionType::ExtInfo Info(Record[1], Record[2], Record[3],
-   (CallingConv)Record[4], Record[5]);
+   (CallingConv)Record[4], Record[5],
+   Record[6]);
 return Context.getFunctionNoProtoType(ResultType, Info);
   }
 
@@ -5386,9 +5387,10 @@
 /*hasregparm*/ Record[2],
 /*regparm*/ Record[3],
 static_cast(Record[4]),
-/*produces*/ Record[5]);
+/*produces*/ Record[5],
+/*nocallersavedregs*/ Record[6]);
 
-unsigned Idx = 6;
+unsigned Idx = 7;
 
 EPI.Variadic = Record[Idx++];
 EPI.HasTrailingReturn = Record[Idx++];
Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -746,6 +746,7 @@
   FI->ChainCall = chainCall;
   FI->NoReturn = info.getNoReturn();
   FI->ReturnsRetained = info.getProducesResult();
+  FI->NoCallerSavedRegs = info.getNoCallerSavedRegs();
   FI->Required = required;
   FI->HasRegParm = info.getHasRegParm();
   FI->RegParm = info.getRegParm();
@@ -1673,6 +1674,8 @@
   RetAttrs.addAttribute(llvm::Attribute::NoAlias);
 if (TargetDecl->hasAttr())
   RetAttrs.addAttribute(llvm::Attribute::NonNull);
+if (TargetDecl->hasAttr())
+  FuncAttrs.addAttribute("no_caller_saved_registers");
 
 HasAnyX86InterruptAttr = TargetDecl->hasAttr();
 HasOptnone = TargetDecl->hasAttr();
Index: lib/AST/ASTContext.cpp
===
--- lib/AST/ASTContext.cpp
+++ lib/AST/ASTContext.cpp
@@ -7564,6 +7564,8 @@
 
   if (lbaseInfo.getProducesResult() != rbaseInfo.getProducesResult())
 return QualType();
+  if (lbaseInfo.getNoCallerSavedRegs() != rbaseInfo.getNoCallerSavedRegs())
+return QualType();
 
   // FIXME: some uses, e.g. conditional exprs, really want this to be 'both'.
   bool NoReturn = lbaseInfo.getNoReturn() || rbaseInfo.getNoReturn();
Index: lib/AST/TypePrinter.cpp
===
--- lib/AST/TypePrinter.cpp
+++ lib/AST/TypePrinter.cpp
@@ -745,6 +745,8 @@
   if (Info.getRegParm())
 OS << " __attribute__((regparm ("
<< Info.getRegParm() << ")))";
+  if (Info.getNoCallerSavedRegs())
+OS << "__attribute__((no_caller_saved_registers))";
 
   if (unsigned quals = T->getTypeQuals()) {
 OS << ' ';
Index: lib/Sema/SemaType.cpp
===
--- 

Re: [PATCH] D23004: [ASTMatchers] Add matchers canReferToDecl() and hasUnderlyingDecl()

[Martin Böhme via cfe-commits]

mboehme marked an inline comment as done.


Comment at: include/clang/ASTMatchers/ASTMatchers.h:2486
@@ +2485,3 @@
+
+  return (UnderlyingDecl != nullptr &&
+  InnerMatcher.matches(*UnderlyingDecl, Finder, Builder));

I was trying to match the style of the next matcher below -- or do you think I 
should establish a precedent / preference for emitting the superfluous 
parentheses?


Comment at: include/clang/ASTMatchers/ASTMatchers.h:2855
@@ +2854,3 @@
+/// bar(t);
+///   }
+/// \endcode

> The documentation doesn't describe what the matcher does (can you please 
> clarify the docs?).

I've rephrased the description of the matcher -- is it clearer now?

> The implementation suggests that this is looking to see if the given decl 
> exists in the overload expression set, which makes me wonder why this isn't 
> implemented on the hasDeclaration() traversal matcher rather than adding a 
> new matcher name?

As klimek notes, the difference is that hasDeclaration() is currently used only 
for nodes that have exactly one associated declaration.

My proposal would be to stay with canReferToDecl -- thoughts?


https://reviews.llvm.org/D23004



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

Re: [PATCH] D23004: [ASTMatchers] Add matchers canReferToDecl() and hasUnderlyingDecl()

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.


Comment at: include/clang/ASTMatchers/ASTMatchers.h:2486
@@ +2485,3 @@
+
+  return (UnderlyingDecl != nullptr &&
+  InnerMatcher.matches(*UnderlyingDecl, Finder, Builder));

mboehme wrote:
> I was trying to match the style of the next matcher below -- or do you think 
> I should establish a precedent / preference for emitting the superfluous 
> parentheses?
Yes, I consider parentheses with `return` are confusing - I'm immediately 
trying to find a reason, why they are needed.


Comment at: include/clang/ASTMatchers/ASTMatchers.h:2855
@@ +2854,3 @@
+/// bar(t);
+///   }
+/// \endcode

mboehme wrote:
> > The documentation doesn't describe what the matcher does (can you please 
> > clarify the docs?).
> 
> I've rephrased the description of the matcher -- is it clearer now?
> 
> > The implementation suggests that this is looking to see if the given decl 
> > exists in the overload expression set, which makes me wonder why this isn't 
> > implemented on the hasDeclaration() traversal matcher rather than adding a 
> > new matcher name?
> 
> As klimek notes, the difference is that hasDeclaration() is currently used 
> only for nodes that have exactly one associated declaration.
> 
> My proposal would be to stay with canReferToDecl -- thoughts?
+1 to `canReferToDecl`


https://reviews.llvm.org/D23004



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

Re: [PATCH] D23004: [ASTMatchers] Add matchers canReferToDecl() and hasUnderlyingDecl()

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

In https://reviews.llvm.org/D23004#505854, @mboehme wrote:

> I assume this means I should avoid doing this (rebasing to a newer revision)?


No, rebasing is totally fine. But if some files changed in the meantime, 
comparing with older revisions will contain unrelated changes, that's expected, 
I think.


https://reviews.llvm.org/D23004



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

Re: [PATCH] D23004: [ASTMatchers] Add matchers canReferToDecl() and hasUnderlyingDecl()

[Martin Böhme via cfe-commits]

mboehme added a comment.

I've rebased the patch to a newer head revision, which seems to confuse 
Phabricator when diffing against earlier versions of the patch. Apologies. I 
assume this means I should avoid doing this (rebasing to a newer revision)?


https://reviews.llvm.org/D23004



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

Re: [PATCH] D23004: [ASTMatchers] Add matchers canReferToDecl() and hasUnderlyingDecl()

[Martin Böhme via cfe-commits]

mboehme updated this revision to Diff 66808.

https://reviews.llvm.org/D23004

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  lib/ASTMatchers/Dynamic/Registry.cpp
  unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -241,6 +241,21 @@
 hasDeclaration(namedDecl(hasName("A";
 }
 
+TEST(HasUnderlyingDecl, Matches) {
+  EXPECT_TRUE(matches("namespace N { template  void f(T t); }"
+  "template  void g() { using N::f; f(T()); }",
+  unresolvedLookupExpr(canReferToDecl(
+  namedDecl(hasUnderlyingDecl(hasName("::N::f")));
+  EXPECT_TRUE(matches(
+  "namespace N { template  void f(T t); }"
+  "template  void g() { N::f(T()); }",
+  unresolvedLookupExpr(canReferToDecl(namedDecl(hasName("::N::f"));
+  EXPECT_TRUE(notMatches(
+  "namespace N { template  void f(T t); }"
+  "template  void g() { using N::f; f(T()); }",
+  unresolvedLookupExpr(canReferToDecl(namedDecl(hasName("::N::f"));
+}
+
 TEST(HasType, TakesQualTypeMatcherAndMatchesExpr) {
   TypeMatcher ClassX = hasDeclaration(recordDecl(hasName("X")));
   EXPECT_TRUE(
@@ -2072,5 +2087,24 @@
   EXPECT_TRUE(notMatches(Code2, ForEachOverriddenInClass("A1")));
 }
 
+TEST(Matcher, CanReferToDecl) {
+  std::string Fragment = "void foo(int p1);"
+ "void foo(int *p2);"
+ "void bar(int p3);"
+ "template  void baz(T t) { foo(t); }";
+
+  EXPECT_TRUE(
+  matches(Fragment, unresolvedLookupExpr(canReferToDecl(functionDecl(
+hasParameter(0, parmVarDecl(hasName("p1";
+  EXPECT_TRUE(
+  matches(Fragment, unresolvedLookupExpr(canReferToDecl(functionDecl(
+hasParameter(0, parmVarDecl(hasName("p2";
+  EXPECT_TRUE(
+  notMatches(Fragment, unresolvedLookupExpr(canReferToDecl(functionDecl(
+   hasParameter(0, parmVarDecl(hasName("p3";
+  EXPECT_TRUE(notMatches(Fragment, unresolvedLookupExpr(canReferToDecl(
+   functionDecl(hasName("bar"));
+}
+
 } // namespace ast_matchers
 } // namespace clang
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -115,6 +115,7 @@
   REGISTER_MATCHER(breakStmt);
   REGISTER_MATCHER(builtinType);
   REGISTER_MATCHER(callExpr);
+  REGISTER_MATCHER(canReferToDecl);
   REGISTER_MATCHER(caseStmt);
   REGISTER_MATCHER(castExpr);
   REGISTER_MATCHER(characterLiteral);
@@ -265,6 +266,7 @@
   REGISTER_MATCHER(hasTypeLoc);
   REGISTER_MATCHER(hasUnaryOperand);
   REGISTER_MATCHER(hasUnarySelector);
+  REGISTER_MATCHER(hasUnderlyingDecl);
   REGISTER_MATCHER(hasValueType);
   REGISTER_MATCHER(ifStmt);
   REGISTER_MATCHER(ignoringImplicit);
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -2468,6 +2468,25 @@
   void(internal::HasDeclarationSupportedTypes)>(InnerMatcher);
 }
 
+/// \brief Matches a \c NamedDecl whose underlying declaration matches the given
+/// matcher.
+///
+/// Given
+/// \code
+///   namespace N { template void f(T t); }
+///   template  void g() { using N::f; f(T()); }
+/// \endcode
+/// \c unresolvedLookupExpr(canReferToDecl(
+/// namedDecl(hasUnderlyingDecl(hasName("::N::f")
+///   matches the use of \c f in \c g() .
+AST_MATCHER_P(NamedDecl, hasUnderlyingDecl, internal::Matcher,
+  InnerMatcher) {
+  const NamedDecl *UnderlyingDecl = Node.getUnderlyingDecl();
+
+  return (UnderlyingDecl != nullptr &&
+  InnerMatcher.matches(*UnderlyingDecl, Finder, Builder));
+}
+
 /// \brief Matches on the implicit object argument of a member call expression.
 ///
 /// Example matches y.x()
@@ -2823,6 +2842,26 @@
   return false;
 }
 
+/// \brief Matches an \c OverloadExpr if any of the declarations in the set of
+/// overloads matches the given matcher.
+///
+/// Given
+/// \code
+///   template  void foo(T);
+///   template  void bar(T);
+///   template  void baz(T t) {
+/// foo(t);
+/// bar(t);
+///   }
+/// \endcode
+/// unresolvedLookupExpr(canReferToDecl(functionTemplateDecl(hasName("foo"
+///   matches \c foo in \c foo(t); but not \c bar in \c bar(t);
+AST_MATCHER_P(OverloadExpr, canReferToDecl, internal::Matcher,
+  InnerMatcher) {
+  return matchesFirstInPointerRange(InnerMatcher, Node.decls_begin(),
+Node.decls_end(), Finder, 

Re: [PATCH] D23158: [clang-rename] merge tests when possible

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

Much better now, please also add instruction on how to find offsets.


https://reviews.llvm.org/D23158



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

Re: [libcxx] r273034 - Add Filesystem TS -- Complete

[Adrian Prantl via cfe-commits]

> On Aug 3, 2016, at 1:56 PM, Bruno Cardoso Lopes  
> wrote:
> 
> Hi Eric,
> 
> After we upgraded our green dragon bots to El Captain (10.11), the
> test below started to fail on some of our machines:
> 
> -- 
> test/std/experimental/filesystem/fs.op.funcs/fs.op.hard_lk_ct/hard_link_count.pass.cpp
> 
>  TEST_CASE(hard_link_count_for_directory)
>  {
>  uintmax_t DirExpect = 3;
>  uintmax_t Dir3Expect = 2;
>  #if defined(__APPLE__)
>  DirExpect += 2;
>  Dir3Expect += 1;
>  #endif

Just as a general code review comment: When committing a platform-specific 
workaround, I would expect there to be a comment explaining what 
bug/peculiarity of the platform is being worked around :-)

thanks,
Adrian

>  TEST_CHECK(hard_link_count(StaticEnv::Dir) == DirExpect);
>  TEST_CHECK(hard_link_count(StaticEnv::Dir3) == Dir3Expect);
> 
>  std::error_code ec;
>  TEST_CHECK(hard_link_count(StaticEnv::Dir, ec) == DirExpect);
>  TEST_CHECK(hard_link_count(StaticEnv::Dir3, ec) == Dir3Expect);
>  }
> 
> You can see the issue in every recent (past 10 days) run on
> http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA-expensive/433
> 
> I noticed that commenting out the snippet around "#if
> defined(__APPLE__)" would make it work. What's the rationale behind
> the "#if defined(__APPLE__)" above?
> 
> Thanks,
> 
> 
> On Tue, Jun 21, 2016 at 3:03 PM, Eric Fiselier via cfe-commits
>  wrote:
>> The issue should be fixed in r273323.
>> 
>> Thanks for the report and for your patience.
>> 
>> /Eric
>> 
>> On Mon, Jun 20, 2016 at 11:27 PM, Eric Fiselier  wrote:
>>> 
>>> Hi Artem,
>>> 
>>> Sorry for the delay, I've been busy with school all day.
>>> I'll check in a fix tomorrow morning.
>>> 
>>> Sorry again,
>>> 
>>> /Eric
>>> 
>>> On Mon, Jun 20, 2016 at 2:31 PM, Eric Fiselier  wrote:
 
 Oh shoot, I definitely didn't take that into account. I'll put together a
 fix.
 
 /Eric
 
 
 
 On Mon, Jun 20, 2016 at 2:27 PM, Artem Belevich  wrote:
> 
> Eric,
> 
> Some tests appear to fail if the path to the tests' current directory
> has some symlinks in it.
> In my case source and build tree are in directory 'work' that's
> symlinked to from my home directory:
> /usr/local/home/tra/work -> /work/tra
> 
> This causes multiple failures in libcxx tests. One example:
> 
> 
> projects/libcxx/test/std/experimental/filesystem/fs.op.funcs/fs.op.rename/rename.pass.cpp:121
> 121TEST_CHECK(read_symlink(bad_sym_dest) == dne);
> 
> dne:
> 
> /usr/local/home/tra/work/llvm/build/gpu/debug/projects/libcxx/test/filesystem/Output/dynamic_env/test.529143/dne
> bad_sym_dest:
> 
> /usr/local/home/tra/work/llvm/build/gpu/debug/projects/libcxx/test/filesystem/Output/dynamic_env/test.529143/bad_sym2
> 
> These are the paths that traverse the 'work' symlink in my home
> directory. However, the symlink that we're reading contains physical path 
> to
> the directory. While it does link to the right place, it's not the path 
> the
> test expects.
> 
> #readlink
> /usr/local/home/tra/work/llvm/build/gpu/debug/projects/libcxx/test/filesystem/Output/dynamic_env/test.529143/bad_sym2
> 
> /work/tra/llvm/build/gpu/debug/projects/libcxx/test/filesystem/Output/dynamic_env/test.529143/dne
> 
> I think we need to normalize paths before we compare them.
> 
> --Artem
> 
> 
> On Sat, Jun 18, 2016 at 12:03 PM, Eric Fiselier via cfe-commits
>  wrote:
>> 
>>> I assume the correct way to fix this is to disable
>>> -Wcovered-switch-default while compiling
>>> libcxx/src/experimental/filesystem/operations.cpp
>> 
>> Agreed. Disabled in r273092.
>> 
>> Thanks for your patience with this latest change,
>> 
>> /Eric
>> 
>> On Sat, Jun 18, 2016 at 12:54 PM, Adrian Prantl 
>> wrote:
>>> 
>>> Hello Eric,
>>> 
>>> this commit causes new warnings on our bots:
>>> 
>>> clang/src/projects/libcxx/include/fstream:816:5: warning: default
>>> label in switch which covers all enumeration values
>>> [-Wcovered-switch-default]
>>>default:
>>> 
>>> The problem is with this defensive default statement in fstream:
>>> 
>>> 
>>> template 
>>> 0792 typename basic_filebuf<_CharT, _Traits>::pos_type
>>> 0793 basic_filebuf<_CharT, _Traits>::seekoff(off_type __off,
>>> ios_base::seekdir __way,
>>> 0794 ios_base::openmode)
>>> 0795 {
>>> 0796 #ifndef _LIBCPP_NO_EXCEPTIONS
>>> 0797 if (!__cv_)
>>> 0798 throw bad_cast();
>>> 0799 #endif
>>> 0800 int __width = __cv_->encoding();
>>> 0801 if (__file_ == 0 || (__width <= 0 && 

Re: [PATCH] D23135: [clang-tidy] misc-argument-comment non-strict mode

[Alexander Kornienko via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL277729: [clang-tidy] misc-argument-comment non-strict mode 
(authored by alexfh).

Changed prior to commit:
  https://reviews.llvm.org/D23135?vs=66806=66807#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D23135

Files:
  clang-tools-extra/trunk/clang-tidy/ClangTidy.h
  clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.cpp
  clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.h
  clang-tools-extra/trunk/docs/clang-tidy/checks/misc-argument-comment.rst
  clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment-strict.cpp
  clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment.cpp
  clang-tools-extra/trunk/unittests/clang-tidy/MiscModuleTest.cpp

Index: clang-tools-extra/trunk/unittests/clang-tidy/MiscModuleTest.cpp
===
--- clang-tools-extra/trunk/unittests/clang-tidy/MiscModuleTest.cpp
+++ clang-tools-extra/trunk/unittests/clang-tidy/MiscModuleTest.cpp
@@ -23,10 +23,10 @@
 TEST(ArgumentCommentCheckTest, OtherEditDistanceAboveThreshold) {
   EXPECT_EQ("void f(int xxx, int yyy); void g() { f(/*xxx=*/0, 0); }",
 runCheckOnCode(
-"void f(int xxx, int yyy); void g() { f(/*Xxx=*/0, 0); }"));
+"void f(int xxx, int yyy); void g() { f(/*Zxx=*/0, 0); }"));
   EXPECT_EQ("struct C { C(int xxx, int yyy); }; C c(/*xxx=*/0, 0);",
 runCheckOnCode(
-"struct C { C(int xxx, int yyy); }; C c(/*Xxx=*/0, 0);"));
+"struct C { C(int xxx, int yyy); }; C c(/*Zxx=*/0, 0);"));
 }
 
 TEST(ArgumentCommentCheckTest, OtherEditDistanceBelowThreshold) {
Index: clang-tools-extra/trunk/clang-tidy/ClangTidy.h
===
--- clang-tools-extra/trunk/clang-tidy/ClangTidy.h
+++ clang-tools-extra/trunk/clang-tidy/ClangTidy.h
@@ -73,6 +73,23 @@
 return Result;
   }
 
+  /// \brief Read a named option from the ``Context`` and parse it as an
+  /// integral type ``T``.
+  ///
+  /// Reads the option with the check-local name \p LocalName from local or
+  /// global ``CheckOptions``. Gets local option first. If local is not present,
+  /// falls back to get global option. If global option is not present either,
+  /// returns Default.
+  template 
+  typename std::enable_if::type
+  getLocalOrGlobal(StringRef LocalName, T Default) const {
+std::string Value = getLocalOrGlobal(LocalName, "");
+T Result = Default;
+if (!Value.empty())
+  StringRef(Value).getAsInteger(10, Result);
+return Result;
+  }
+
   /// \brief Stores an option with the check-local name \p LocalName with string
   /// value \p Value to \p Options.
   void store(ClangTidyOptions::OptionMap , StringRef LocalName,
Index: clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.cpp
@@ -22,16 +22,21 @@
 ArgumentCommentCheck::ArgumentCommentCheck(StringRef Name,
ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
+  StrictMode(Options.getLocalOrGlobal("StrictMode", 0) != 0),
   IdentRE("^(/\\* *)([_A-Za-z][_A-Za-z0-9]*)( *= *\\*/)$") {}
 
+void ArgumentCommentCheck::storeOptions(ClangTidyOptions::OptionMap ) {
+  Options.store(Opts, "StrictMode", StrictMode);
+}
+
 void ArgumentCommentCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
   callExpr(unless(cxxOperatorCallExpr()),
// NewCallback's arguments relate to the pointed function, don't
// check them against NewCallback's parameter names.
// FIXME: Make this configurable.
-   unless(hasDeclaration(functionDecl(anyOf(
-   hasName("NewCallback"), hasName("NewPermanentCallback"))
+   unless(hasDeclaration(functionDecl(
+   hasAnyName("NewCallback", "NewPermanentCallback")
   .bind("expr"),
   this);
   Finder->addMatcher(cxxConstructExpr().bind("expr"), this);
@@ -78,12 +83,13 @@
   return Comments;
 }
 
-bool
-ArgumentCommentCheck::isLikelyTypo(llvm::ArrayRef Params,
-   StringRef ArgName, unsigned ArgIndex) {
-  std::string ArgNameLower = ArgName.lower();
+bool ArgumentCommentCheck::isLikelyTypo(llvm::ArrayRef Params,
+StringRef ArgName, unsigned ArgIndex) {
+  std::string ArgNameLowerStr = ArgName.lower();
+  StringRef ArgNameLower = ArgNameLowerStr;
+  // The threshold is arbitrary.
   unsigned UpperBound = (ArgName.size() + 2) / 3 + 1;
-  unsigned ThisED = StringRef(ArgNameLower).edit_distance(
+  unsigned ThisED = ArgNameLower.edit_distance(
   

[clang-tools-extra] r277729 - [clang-tidy] misc-argument-comment non-strict mode

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Thu Aug  4 09:54:54 2016
New Revision: 277729

URL: http://llvm.org/viewvc/llvm-project?rev=277729=rev
Log:
[clang-tidy] misc-argument-comment non-strict mode

Summary:
The misc-argument-comment check now ignores leading and trailing underscores and
case. The new `StrictMode` local/global option can be used to switch back to
strict checking.

Add getLocalOrGlobal version for integral types, minor cleanups.

Reviewers: hokein, aaron.ballman

Subscribers: aaron.ballman, Prazek, cfe-commits

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

Added:
clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment-strict.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/ClangTidy.h
clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-argument-comment.rst
clang-tools-extra/trunk/test/clang-tidy/misc-argument-comment.cpp
clang-tools-extra/trunk/unittests/clang-tidy/MiscModuleTest.cpp

Modified: clang-tools-extra/trunk/clang-tidy/ClangTidy.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidy.h?rev=277729=277728=277729=diff
==
--- clang-tools-extra/trunk/clang-tidy/ClangTidy.h (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidy.h Thu Aug  4 09:54:54 2016
@@ -73,6 +73,23 @@ public:
 return Result;
   }
 
+  /// \brief Read a named option from the ``Context`` and parse it as an
+  /// integral type ``T``.
+  ///
+  /// Reads the option with the check-local name \p LocalName from local or
+  /// global ``CheckOptions``. Gets local option first. If local is not 
present,
+  /// falls back to get global option. If global option is not present either,
+  /// returns Default.
+  template 
+  typename std::enable_if::type
+  getLocalOrGlobal(StringRef LocalName, T Default) const {
+std::string Value = getLocalOrGlobal(LocalName, "");
+T Result = Default;
+if (!Value.empty())
+  StringRef(Value).getAsInteger(10, Result);
+return Result;
+  }
+
   /// \brief Stores an option with the check-local name \p LocalName with 
string
   /// value \p Value to \p Options.
   void store(ClangTidyOptions::OptionMap , StringRef LocalName,

Modified: clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.cpp?rev=277729=277728=277729=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/ArgumentCommentCheck.cpp Thu Aug  4 
09:54:54 2016
@@ -22,16 +22,21 @@ namespace misc {
 ArgumentCommentCheck::ArgumentCommentCheck(StringRef Name,
ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
+  StrictMode(Options.getLocalOrGlobal("StrictMode", 0) != 0),
   IdentRE("^(/\\* *)([_A-Za-z][_A-Za-z0-9]*)( *= *\\*/)$") {}
 
+void ArgumentCommentCheck::storeOptions(ClangTidyOptions::OptionMap ) {
+  Options.store(Opts, "StrictMode", StrictMode);
+}
+
 void ArgumentCommentCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
   callExpr(unless(cxxOperatorCallExpr()),
// NewCallback's arguments relate to the pointed function, don't
// check them against NewCallback's parameter names.
// FIXME: Make this configurable.
-   unless(hasDeclaration(functionDecl(anyOf(
-   hasName("NewCallback"), hasName("NewPermanentCallback"))
+   unless(hasDeclaration(functionDecl(
+   hasAnyName("NewCallback", "NewPermanentCallback")
   .bind("expr"),
   this);
   Finder->addMatcher(cxxConstructExpr().bind("expr"), this);
@@ -78,12 +83,13 @@ getCommentsInRange(ASTContext *Ctx, Char
   return Comments;
 }
 
-bool
-ArgumentCommentCheck::isLikelyTypo(llvm::ArrayRef Params,
-   StringRef ArgName, unsigned ArgIndex) {
-  std::string ArgNameLower = ArgName.lower();
+bool ArgumentCommentCheck::isLikelyTypo(llvm::ArrayRef Params,
+StringRef ArgName, unsigned ArgIndex) {
+  std::string ArgNameLowerStr = ArgName.lower();
+  StringRef ArgNameLower = ArgNameLowerStr;
+  // The threshold is arbitrary.
   unsigned UpperBound = (ArgName.size() + 2) / 3 + 1;
-  unsigned ThisED = StringRef(ArgNameLower).edit_distance(
+  unsigned ThisED = ArgNameLower.edit_distance(
   Params[ArgIndex]->getIdentifier()->getName().lower(),
   /*AllowReplacements=*/true, UpperBound);
   if (ThisED >= UpperBound)
@@ -100,9 +106,9 @@ ArgumentCommentCheck::isLikelyTypo(llvm:
 // Other parameters must be an edit distance at 

Re: [PATCH] D23135: [clang-tidy] misc-argument-comment non-strict mode

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

hokein: I'm committing the patch; if you have more comments, I'm happy to 
address them in a follow-up.


https://reviews.llvm.org/D23135



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

Re: [PATCH] D23130: [Clang-tidy] Add a check for definitions in the global namespace.

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.


Comment at: test/clang-tidy/google-global-names.cpp:13-14
@@ +12,4 @@
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'i' declared in the global 
namespace
+extern int ii = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 'ii' declared in the global 
namespace
+

bkramer wrote:
> aaron.ballman wrote:
> > This strikes me as being intentional enough to warrant not diagnosing 
> > because of the `extern` keyword.
> The only case I see where this pattern is valuable is interfacing with C 
> code. Not sure yet if we want to allow that or enforce extern "C" instead. 
> Ideas?
> 
> an extern global in the global namespace still feels like something we should 
> warn on :|
Yet externs in the global namespace do happen for valid reasons (such as not 
breaking ABIs by putting the extern definition into a namespace or changing the 
language linkage) -- I'm trying to think of ways we can allow the user to 
silence this diagnostic in those cases. I feel like in cases where the user 
writes "extern", they're explicitly specifying their intent and that doesn't 
seem like a case to warn them about, in some regards. It would give us two ways 
to silence the diagnostic (well, three, but two are morally close enough):

1) Put it into a namespace
2) Slap `extern` on it if it is global for C++ compatibility (such as ABIs)
3) Slap `extern "C"` on it if it global for C compatibility

I suppose we could require `extern "C++"` instead of `extern`, but I don't 
think that's a particularly common use of the language linkage specifier?


https://reviews.llvm.org/D23130



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

Re: [PATCH] D23135: [clang-tidy] misc-argument-comment non-strict mode

[Aaron Ballman via cfe-commits]

On Thu, Aug 4, 2016 at 10:45 AM, Alexander Kornienko  wrote:
> alexfh added inline comments.
>
> 
> Comment at: clang-tidy/misc/ArgumentCommentCheck.cpp:124
> @@ +123,3 @@
> +  InDecl = InDecl.trim('_');
> +  return InComment.compare_lower(InDecl) == 0;
> +}
> 
> aaron.ballman wrote:
>> alexfh wrote:
>> > aaron.ballman wrote:
>> > > Correct, which means this won't behave properly in some locales with 
>> > > UTF-8 identifiers. Consider Turkish, where İ (U+0130 “Latin Capital 
>> > > Letter I With Dot Above”) is the uppercase form of ı (U+0131 “Latin 
>> > > Small Letter Dotless I”). If the comment contains one version while the 
>> > > identifier contains the other, the comparison will currently fail, while 
>> > > a locale-aware comparison would succeed. You run into similar things 
>> > > with SS vs ß in German as well, where the uppercase form is two 
>> > > characters while the lowercase is only a single character.
>> > Interesting, though it looks like there's now an official capital ẞ 
>> > https://en.wikipedia.org/wiki/Capital_%E1%BA%9E (which is not frequently 
>> > needed anyway, I guess).
>> >
>> > At the end of the day, what we get is that the non-strict mode is 
>> > currently somewhat stricter for non-ascii characters. Similar will happen 
>> > with all other parts in LLVM that rely on `StringRef::compare_lower`. I 
>> > don't think we need a separate test for this _here_, since it's a problem 
>> > on a completely different level. And I guess the use non-ascii identifiers 
>> > in C++ will cause much more serious problems than a slightly stricter 
>> > clang-tidy warning ;]
>> We may just have different testing philosophies -- I would advocate for a 
>> test because we know of a use case that's broken with this particular use of 
>> `compare_lower`. Not all uses of `compare_lower` are problematic, after all. 
>> However, I'm not going to fight for that test case too hard because this is 
>> hopefully an edge case that is low-impact. A FIXME would also suffice.
> I'm reluctant to add a case, since the cost of making it work and maintaining 
> on both linux and windows is higher than the value of it, IMO (it's my take 
> out from writing clang-format's limited support for Unicode).

I am totally okay with that line of reasoning. I was mostly looking
for some marker that says "if this acts funky, it's expected, not
accidental." The FIXME scratches that itch for me, so thank you!

~Aaron

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

Re: [PATCH] D23135: [clang-tidy] misc-argument-comment non-strict mode

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.


Comment at: clang-tidy/misc/ArgumentCommentCheck.cpp:124
@@ +123,3 @@
+  InDecl = InDecl.trim('_');
+  return InComment.compare_lower(InDecl) == 0;
+}

aaron.ballman wrote:
> alexfh wrote:
> > aaron.ballman wrote:
> > > Correct, which means this won't behave properly in some locales with 
> > > UTF-8 identifiers. Consider Turkish, where İ (U+0130 “Latin Capital 
> > > Letter I With Dot Above”) is the uppercase form of ı (U+0131 “Latin Small 
> > > Letter Dotless I”). If the comment contains one version while the 
> > > identifier contains the other, the comparison will currently fail, while 
> > > a locale-aware comparison would succeed. You run into similar things with 
> > > SS vs ß in German as well, where the uppercase form is two characters 
> > > while the lowercase is only a single character.
> > Interesting, though it looks like there's now an official capital ẞ 
> > https://en.wikipedia.org/wiki/Capital_%E1%BA%9E (which is not frequently 
> > needed anyway, I guess).
> > 
> > At the end of the day, what we get is that the non-strict mode is currently 
> > somewhat stricter for non-ascii characters. Similar will happen with all 
> > other parts in LLVM that rely on `StringRef::compare_lower`. I don't think 
> > we need a separate test for this _here_, since it's a problem on a 
> > completely different level. And I guess the use non-ascii identifiers in 
> > C++ will cause much more serious problems than a slightly stricter 
> > clang-tidy warning ;]
> We may just have different testing philosophies -- I would advocate for a 
> test because we know of a use case that's broken with this particular use of 
> `compare_lower`. Not all uses of `compare_lower` are problematic, after all. 
> However, I'm not going to fight for that test case too hard because this is 
> hopefully an edge case that is low-impact. A FIXME would also suffice.
I'm reluctant to add a case, since the cost of making it work and maintaining 
on both linux and windows is higher than the value of it, IMO (it's my take out 
from writing clang-format's limited support for Unicode).


https://reviews.llvm.org/D23135



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

Re: [PATCH] D23135: [clang-tidy] misc-argument-comment non-strict mode

[Aaron Ballman via cfe-commits]

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

LGTM!


https://reviews.llvm.org/D23135



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

Re: [PATCH] D23130: [Clang-tidy] Add a check for definitions in the global namespace.

[Benjamin Kramer via cfe-commits]

bkramer added inline comments.


Comment at: test/clang-tidy/google-global-names.cpp:13-14
@@ +12,4 @@
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'i' declared in the global 
namespace
+extern int ii = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 'ii' declared in the global 
namespace
+

aaron.ballman wrote:
> This strikes me as being intentional enough to warrant not diagnosing because 
> of the `extern` keyword.
The only case I see where this pattern is valuable is interfacing with C code. 
Not sure yet if we want to allow that or enforce extern "C" instead. Ideas?

an extern global in the global namespace still feels like something we should 
warn on :|


https://reviews.llvm.org/D23130



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

Re: [PATCH] D23135: [clang-tidy] misc-argument-comment non-strict mode

[Alexander Kornienko via cfe-commits]

alexfh updated this revision to Diff 66806.
alexfh added a comment.

- Added a FIXME.


https://reviews.llvm.org/D23135

Files:
  clang-tidy/ClangTidy.h
  clang-tidy/misc/ArgumentCommentCheck.cpp
  clang-tidy/misc/ArgumentCommentCheck.h
  docs/clang-tidy/checks/misc-argument-comment.rst
  test/clang-tidy/misc-argument-comment-strict.cpp
  test/clang-tidy/misc-argument-comment.cpp
  unittests/clang-tidy/MiscModuleTest.cpp

Index: unittests/clang-tidy/MiscModuleTest.cpp
===
--- unittests/clang-tidy/MiscModuleTest.cpp
+++ unittests/clang-tidy/MiscModuleTest.cpp
@@ -23,10 +23,10 @@
 TEST(ArgumentCommentCheckTest, OtherEditDistanceAboveThreshold) {
   EXPECT_EQ("void f(int xxx, int yyy); void g() { f(/*xxx=*/0, 0); }",
 runCheckOnCode(
-"void f(int xxx, int yyy); void g() { f(/*Xxx=*/0, 0); }"));
+"void f(int xxx, int yyy); void g() { f(/*Zxx=*/0, 0); }"));
   EXPECT_EQ("struct C { C(int xxx, int yyy); }; C c(/*xxx=*/0, 0);",
 runCheckOnCode(
-"struct C { C(int xxx, int yyy); }; C c(/*Xxx=*/0, 0);"));
+"struct C { C(int xxx, int yyy); }; C c(/*Zxx=*/0, 0);"));
 }
 
 TEST(ArgumentCommentCheckTest, OtherEditDistanceBelowThreshold) {
Index: test/clang-tidy/misc-argument-comment.cpp
===
--- test/clang-tidy/misc-argument-comment.cpp
+++ test/clang-tidy/misc-argument-comment.cpp
@@ -36,13 +36,18 @@
 
 void templates() {
   variadic(/*xxx=*/0, /*yyy=*/1);
-  variadic2(/*zzZ=*/0, /*xxx=*/1, /*yyy=*/2);
-  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: argument name 'zzZ' in comment does not match parameter name 'zzz'
+  variadic2(/*zzU=*/0, /*xxx=*/1, /*yyy=*/2);
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: argument name 'zzU' in comment does not match parameter name 'zzz'
   // CHECK-FIXES: variadic2(/*zzz=*/0, /*xxx=*/1, /*yyy=*/2);
 }
 
 #define FALSE 0
 void qqq(bool aaa);
 void f() { qqq(/*bbb=*/FALSE); }
 // CHECK-MESSAGES: [[@LINE-1]]:16: warning: argument name 'bbb' in comment does not match parameter name 'aaa'
 // CHECK-FIXES: void f() { qqq(/*bbb=*/FALSE); }
+
+void f(bool _with_underscores_);
+void ignores_underscores() {
+  f(/*With_Underscores=*/false);
+}
Index: test/clang-tidy/misc-argument-comment-strict.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-argument-comment-strict.cpp
@@ -0,0 +1,19 @@
+// RUN: %check_clang_tidy %s misc-argument-comment %t -- \
+// RUN:   -config="{CheckOptions: [{key: StrictMode, value: 1}]}" --
+
+void f(int _with_underscores_);
+void g(int x_);
+void ignores_underscores() {
+  f(/*With_Underscores=*/0);
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument name 'With_Underscores' in comment does not match parameter name '_with_underscores_'
+// CHECK-FIXES: f(/*_with_underscores_=*/0);
+  f(/*with_underscores=*/1);
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument name 'with_underscores' in comment does not match parameter name '_with_underscores_'
+// CHECK-FIXES: f(/*_with_underscores_=*/1);
+  f(/*_With_Underscores_=*/2);
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument name '_With_Underscores_' in comment does not match parameter name '_with_underscores_'
+// CHECK-FIXES: f(/*_with_underscores_=*/2);
+  g(/*X=*/3);
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument name 'X' in comment does not match parameter name 'x_'
+// CHECK-FIXES: g(/*x_=*/3);
+}
Index: docs/clang-tidy/checks/misc-argument-comment.rst
===
--- docs/clang-tidy/checks/misc-argument-comment.rst
+++ docs/clang-tidy/checks/misc-argument-comment.rst
@@ -18,3 +18,7 @@
   // warning: argument name 'bar' in comment does not match parameter name 'foo'
 
 The check tries to detect typos and suggest automated fixes for them.
+
+Supported options:
+  - `StrictMode` (local or global): when non-zero, the check will ignore leading
+and trailing underscores and case when comparing parameter names.
Index: clang-tidy/misc/ArgumentCommentCheck.h
===
--- clang-tidy/misc/ArgumentCommentCheck.h
+++ clang-tidy/misc/ArgumentCommentCheck.h
@@ -37,8 +37,10 @@
 
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult ) override;
+  void storeOptions(ClangTidyOptions::OptionMap& Opts) override;
 
 private:
+  const bool StrictMode;
   llvm::Regex IdentRE;
 
   bool isLikelyTypo(llvm::ArrayRef Params, StringRef ArgName,
Index: clang-tidy/misc/ArgumentCommentCheck.cpp
===
--- clang-tidy/misc/ArgumentCommentCheck.cpp
+++ clang-tidy/misc/ArgumentCommentCheck.cpp
@@ -22,16 +22,21 @@
 ArgumentCommentCheck::ArgumentCommentCheck(StringRef Name,

Re: [PATCH] D22943: [Driver] Add FIXME's where we can't use effective triples (NFC)

[Joerg Sonnenberger via cfe-commits]

On Mon, Aug 01, 2016 at 08:50:59AM -0700, Vedant Kumar wrote:
> 
> > On Jul 30, 2016, at 12:59 PM, Joerg Sonnenberger via cfe-commits 
> >  wrote:
> > 
> > On Thu, Jul 28, 2016 at 10:11:05PM +, Vedant Kumar wrote:
> >>  - test/Driver/netbsd.c
> >> 
> >>We see -mcpu=arm1022e instead of arm926ej-s in a single run.
> > 
> > Which one exactly?
> 
> Here is the test (test/Driver/netbsd.c @ Line 175):
> 
> > // ARM: clang{{.*}}" "-cc1" "-triple" "armv5e--netbsd-eabi"
> > // ARM: as{{.*}}" "-mcpu=arm926ej-s" "-o"
> > // ARM: ld{{.*}}" "--eh-frame-hdr" "-dynamic-linker" "/libexec/ld.elf_so"
> > // ARM: "-m" "armelf_nbsd_eabi"
> > // ARM: "-o" "a.out" "{{.*}}/usr/lib{{/|}}crt0.o"
> > // ARM: "{{.*}}/usr/lib{{/|}}eabi{{/|}}crti.o"
> > // ARM: "{{.*}}/usr/lib{{/|}}crtbegin.o" "{{.*}}.o" "-lc"
> > // ARM: "{{.*}}/usr/lib{{/|}}crtend.o" "{{.*}}/usr/lib{{/|}}crtn.o"
> 
> And here is the test output:
> 
> > /Users/vk/Desktop/llvm/tools/clang/test/Driver/netbsd.c:175:9: error: 
> > expected string not found in input
> > // ARM: as{{.*}}" "-mcpu=arm926ej-s" "-o"
> > ^
> > :5:81: note: scanning from here
> >  "/Users/vk/Desktop/llvm/DA/./bin/clang" "-cc1" "-triple" 
> > "armv5e--netbsd-eabi" "-S" "-disable-free" "-main-file-name" "netbsd.c" 
> > "-mrelocation-model" "static" "-mthread-model" "posix" "-mdisable-fp-elim" 
> > "-masm-verbose" "-no-integrated-as" "-mconstructor-aliases" 
> > "-munwind-tables" "-target-cpu" "arm1022e" "-target-feature" 
> > "+soft-float-abi" "-target-feature" "+strict-align" "-target-abi" "aapcs" 
> > "-mfloat-abi" "soft" "-target-linker-version" "272" "-dwarf-column-info" 
> > "-debugger-tuning=gdb" "-resource-dir" 
> > "/Users/vk/Desktop/llvm/DA/./bin/../lib/clang/4.0.0" "-isysroot" 
> > "/Users/vk/Desktop/llvm/tools/clang/test/Driver/Inputs/basic_netbsd_tree" 
> > "-fno-dwarf-directory-asm" "-fdebug-compilation-dir" 
> > "/Users/vk/Desktop/llvm/build/tools/clang/test/Driver" "-ferror-limit" "19" 
> > "-fmessage-length" "0" "-fallow-half-arguments-and-returns" 
> > "-fno-signed-char" "-fobjc-runtime=gnustep" "-fdiagnostics-show-option" 
> > "-o" "/var/folders/j5/4k_dcg5j5kl7z4fd3gp1pjp4gn/T/netbsd-0d4ff3.s" 
> > "-x" "c" "/Users/vk/Desktop/llvm/tools/clang/test/Driver/netbsd.c"
> > 
> > ^
> > :6:6: note: possible intended match here
> >  "/usr/bin/as" "-mcpu=arm1022e" "-o" 
> > "/var/folders/j5/4k_dcg5j5kl7z4fd3gp1pjp4gn/T/netbsd-82d94c.o" 
> > "/var/folders/j5/4k_dcg5j5kl7z4fd3gp1pjp4gn/T/netbsd-0d4ff3.s"
> 
> For context, the inconsistency arises when the driver uses
> ToolChain::ComputeLLVMTriple() instead of the default toolchain triple.

Hm. So we are hitting the no-architecture fall-through in
getARMCPUForArch for this test case before, but not after? That looks
suspicious. I'm not saying the new result is wrong, but that we should
investigate where the difference comes from. Sadly, I'm currently on
vacation, so I don't have time for that.

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

Re: [PATCH] D23135: [clang-tidy] misc-argument-comment non-strict mode

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.


Comment at: clang-tidy/misc/ArgumentCommentCheck.cpp:124
@@ +123,3 @@
+  InDecl = InDecl.trim('_');
+  return InComment.compare_lower(InDecl) == 0;
+}

alexfh wrote:
> aaron.ballman wrote:
> > Correct, which means this won't behave properly in some locales with UTF-8 
> > identifiers. Consider Turkish, where İ (U+0130 “Latin Capital Letter I With 
> > Dot Above”) is the uppercase form of ı (U+0131 “Latin Small Letter Dotless 
> > I”). If the comment contains one version while the identifier contains the 
> > other, the comparison will currently fail, while a locale-aware comparison 
> > would succeed. You run into similar things with SS vs ß in German as well, 
> > where the uppercase form is two characters while the lowercase is only a 
> > single character.
> Interesting, though it looks like there's now an official capital ẞ 
> https://en.wikipedia.org/wiki/Capital_%E1%BA%9E (which is not frequently 
> needed anyway, I guess).
> 
> At the end of the day, what we get is that the non-strict mode is currently 
> somewhat stricter for non-ascii characters. Similar will happen with all 
> other parts in LLVM that rely on `StringRef::compare_lower`. I don't think we 
> need a separate test for this _here_, since it's a problem on a completely 
> different level. And I guess the use non-ascii identifiers in C++ will cause 
> much more serious problems than a slightly stricter clang-tidy warning ;]
We may just have different testing philosophies -- I would advocate for a test 
because we know of a use case that's broken with this particular use of 
`compare_lower`. Not all uses of `compare_lower` are problematic, after all. 
However, I'm not going to fight for that test case too hard because this is 
hopefully an edge case that is low-impact. A FIXME would also suffice.


https://reviews.llvm.org/D23135



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

Re: [PATCH] D23135: [clang-tidy] misc-argument-comment non-strict mode

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.


Comment at: clang-tidy/misc/ArgumentCommentCheck.cpp:124
@@ +123,3 @@
+  InDecl = InDecl.trim('_');
+  return InComment.compare_lower(InDecl) == 0;
+}

aaron.ballman wrote:
> Correct, which means this won't behave properly in some locales with UTF-8 
> identifiers. Consider Turkish, where İ (U+0130 “Latin Capital Letter I With 
> Dot Above”) is the uppercase form of ı (U+0131 “Latin Small Letter Dotless 
> I”). If the comment contains one version while the identifier contains the 
> other, the comparison will currently fail, while a locale-aware comparison 
> would succeed. You run into similar things with SS vs ß in German as well, 
> where the uppercase form is two characters while the lowercase is only a 
> single character.
Interesting, though it looks like there's now an official capital ẞ 
https://en.wikipedia.org/wiki/Capital_%E1%BA%9E (which is not frequently needed 
anyway, I guess).

At the end of the day, what we get is that the non-strict mode is currently 
somewhat stricter for non-ascii characters. Similar will happen with all other 
parts in LLVM that rely on `StringRef::compare_lower`. I don't think we need a 
separate test for this _here_, since it's a problem on a completely different 
level. And I guess the use non-ascii identifiers in C++ will cause much more 
serious problems than a slightly stricter clang-tidy warning ;]


https://reviews.llvm.org/D23135



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

Re: [PATCH] D23130: [Clang-tidy] Add a check for definitions in the global namespace.

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

For cases where the external linkage is desired, how would you go about 
silencing this diagnostic?



Comment at: test/clang-tidy/google-global-names.cpp:13-14
@@ +12,4 @@
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'i' declared in the global 
namespace
+extern int ii = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 'ii' declared in the global 
namespace
+

This strikes me as being intentional enough to warrant not diagnosing because 
of the `extern` keyword.


https://reviews.llvm.org/D23130



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

Re: [PATCH] D23158: [clang-rename] merge tests when possible

[Kirill Bobyrev via cfe-commits]

omtcyfz updated this revision to Diff 66803.
omtcyfz added a comment.

Rebased on master. Removed unneeded file copying etc since clang-rename can 
output to the stdout.


https://reviews.llvm.org/D23158

Files:
  test/clang-rename/ClassAsTemplateArgument.cpp
  test/clang-rename/ClassAsTemplateArgumentFindByClass.cpp
  test/clang-rename/ClassAsTemplateArgumentFindByTemplateArgument.cpp
  test/clang-rename/ConstCastExpr.cpp
  test/clang-rename/FunctionWithClassFindByName.cpp
  test/clang-rename/TemplateFunction.cpp
  test/clang-rename/TemplateFunctionFindByDeclaration.cpp
  test/clang-rename/TemplateFunctionFindByUse.cpp
  test/clang-rename/TemplateTypename.cpp
  test/clang-rename/TemplateTypenameFindByTemplateParam.cpp
  test/clang-rename/TemplateTypenameFindByTypeInside.cpp
  test/clang-rename/UserDefinedConversion.cpp
  test/clang-rename/UserDefinedConversionFindByTypeDeclaration.cpp

Index: test/clang-rename/UserDefinedConversionFindByTypeDeclaration.cpp
===
--- test/clang-rename/UserDefinedConversionFindByTypeDeclaration.cpp
+++ /dev/null
@@ -1,26 +0,0 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=136 -new-name=Bar %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-
-class Foo { // CHECK: class Bar {
-//^ offset must be here
-public:
-  Foo() {}  // CHECK: Bar() {}
-};
-
-class Baz {
-public:
-  operator Foo() const {// CHECK: operator Bar() const {
-Foo foo;// CHECK: Bar foo;
-return foo;
-  }
-};
-
-int main() {
-  Baz boo;
-  Foo foo = static_cast(boo);  // CHECK: Bar foo = static_cast(boo);
-  return 0;
-}
-
-// Use grep -FUbo 'Foo'  to get the correct offset of Cla when changing
-// this file.
Index: test/clang-rename/UserDefinedConversion.cpp
===
--- test/clang-rename/UserDefinedConversion.cpp
+++ test/clang-rename/UserDefinedConversion.cpp
@@ -1,13 +1,22 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=205 -new-name=Bar %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-
-class Foo {}; // CHECK: class Bar {};
+class Foo { // CHECK: class Bar {
+public:
+  Foo() {}  // CHECK: Bar() {}
+};
 
 class Baz {
-  operator Foo() const {  // CHECK: operator Bar() const {
-// offset  ^
-Foo foo;  // CHECK: Bar foo;
+public:
+  operator Foo() const {// CHECK: operator Bar() const {
+Foo foo;// CHECK: Bar foo;
 return foo;
   }
 };
+
+int main() {
+  Baz boo;
+  Foo foo = static_cast(boo);  // CHECK: Bar foo = static_cast(boo);
+  return 0;
+}
+
+// RUN: clang-rename -offset=7 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+
+// RUN: clang-rename -offset=156 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
Index: test/clang-rename/TemplateTypenameFindByTypeInside.cpp
===
--- test/clang-rename/TemplateTypenameFindByTypeInside.cpp
+++ /dev/null
@@ -1,18 +0,0 @@
-template  // CHECK: template 
-class Foo {
-T foo(T arg, T& ref, T* ptr) {// CHECK: U foo(U arg, U& ref, U* ptr) {
-  T value;// CHECK: U value;
-  int number = 42;
-  value = (T)number;  // CHECK: value = (U)number;
-  value = static_cast(number); // CHECK: value = static_cast(number);
-  return value;
-}
-
-static void foo(T value) {}   // CHECK: static void foo(U value) {}
-
-T member; // CHECK: U member;
-};
-
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=99 -new-name=U %t.cpp -i -- -fno-delayed-template-parsing
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
Index: test/clang-rename/TemplateTypenameFindByTemplateParam.cpp
===
--- test/clang-rename/TemplateTypenameFindByTemplateParam.cpp
+++ /dev/null
@@ -1,18 +0,0 @@
-template  // CHECK: template 
-class Foo {
-T foo(T arg, T& ref, T* ptr) {// CHECK: U foo(U arg, U& ref, U* ptr) {
-  T value;// CHECK: U value;
-  int number = 42;
-  value = (T)number;  // CHECK: value = (U)number;
-  value = static_cast(number); // CHECK: value = static_cast(number);
-  return value;
-}
-
-static void foo(T value) {}   // CHECK: static void foo(U value) {}
-
-T member; // CHECK: U member;
-};
-
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=19 -new-name=U %t.cpp -i -- -fno-delayed-template-parsing
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
Index: test/clang-rename/TemplateTypename.cpp
===
--- test/clang-rename/TemplateTypename.cpp
+++ test/clang-rename/TemplateTypename.cpp
@@ -1,12 +1,18 @@
-// Currently unsupported test.
-// RUN: cat %s > %t.cpp
-// FIXME: 

Re: [PATCH] D23130: [Clang-tidy] Add a check for definitions in the global namespace.

[Benjamin Kramer via cfe-commits]

bkramer added a comment.

In https://reviews.llvm.org/D23130#505769, @alexfh wrote:

> Could you run the check on LLVM and post a summary of results?


I have a full list at https://reviews.llvm.org/P7085. It's huge.


https://reviews.llvm.org/D23130



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

Re: [PATCH] D23130: [Clang-tidy] Add a check for definitions in the global namespace.

[Benjamin Kramer via cfe-commits]

bkramer updated this revision to Diff 66800.
bkramer added a comment.

Address review comments.


https://reviews.llvm.org/D23130

Files:
  clang-tidy/google/CMakeLists.txt
  clang-tidy/google/GlobalNamesCheck.cpp
  clang-tidy/google/GlobalNamesCheck.h
  clang-tidy/google/GlobalNamesInHeadersCheck.cpp
  clang-tidy/google/GlobalNamesInHeadersCheck.h
  clang-tidy/google/GoogleTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/google-global-names-in-headers.rst
  docs/clang-tidy/checks/google-global-names.rst
  test/clang-tidy/google-global-names.cpp
  unittests/clang-tidy/GoogleModuleTest.cpp

Index: unittests/clang-tidy/GoogleModuleTest.cpp
===
--- unittests/clang-tidy/GoogleModuleTest.cpp
+++ unittests/clang-tidy/GoogleModuleTest.cpp
@@ -1,6 +1,6 @@
 #include "ClangTidyTest.h"
 #include "google/ExplicitConstructorCheck.h"
-#include "google/GlobalNamesInHeadersCheck.h"
+#include "google/GlobalNamesCheck.h"
 #include "gtest/gtest.h"
 
 using namespace clang::tidy::google;
@@ -56,7 +56,7 @@
   "A(Foo);"));
 }
 
-class GlobalNamesInHeadersCheckTest : public ::testing::Test {
+class GlobalNamesCheckTest : public ::testing::Test {
 protected:
   bool runCheckOnCode(const std::string , const std::string ) {
 static const char *const Header = "namespace std {\n"
@@ -69,7 +69,7 @@
 if (!StringRef(Filename).endswith(".cpp")) {
   Args.emplace_back("-xc++-header");
 }
-test::runCheckOnCode(
+test::runCheckOnCode(
 Header + Code, , Filename, Args);
 if (Errors.empty())
   return false;
@@ -81,7 +81,7 @@
   }
 };
 
-TEST_F(GlobalNamesInHeadersCheckTest, UsingDeclarations) {
+TEST_F(GlobalNamesCheckTest, UsingDeclarations) {
   EXPECT_TRUE(runCheckOnCode("using std::string;", "foo.h"));
   EXPECT_FALSE(runCheckOnCode("using std::string;", "foo.cpp"));
   EXPECT_FALSE(runCheckOnCode("namespace my_namespace {\n"
@@ -91,7 +91,7 @@
   EXPECT_FALSE(runCheckOnCode("SOME_MACRO(std::string);", "foo.h"));
 }
 
-TEST_F(GlobalNamesInHeadersCheckTest, UsingDirectives) {
+TEST_F(GlobalNamesCheckTest, UsingDirectives) {
   EXPECT_TRUE(runCheckOnCode("using namespace std;", "foo.h"));
   EXPECT_FALSE(runCheckOnCode("using namespace std;", "foo.cpp"));
   EXPECT_FALSE(runCheckOnCode("namespace my_namespace {\n"
@@ -101,7 +101,7 @@
   EXPECT_FALSE(runCheckOnCode("SOME_MACRO(namespace std);", "foo.h"));
 }
 
-TEST_F(GlobalNamesInHeadersCheckTest, RegressionAnonymousNamespace) {
+TEST_F(GlobalNamesCheckTest, RegressionAnonymousNamespace) {
   EXPECT_FALSE(runCheckOnCode("namespace {}", "foo.h"));
 }
 
Index: test/clang-tidy/google-global-names.cpp
===
--- /dev/null
+++ test/clang-tidy/google-global-names.cpp
@@ -0,0 +1,45 @@
+// RUN: %check_clang_tidy %s google-global-names %t
+
+void f();
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'f' declared in the global namespace
+void f() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: 'f' declared in the global namespace
+class F;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'F' declared in the global namespace
+class F {};
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'F' declared in the global namespace
+int i;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: 'i' declared in the global namespace
+extern int ii = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 'ii' declared in the global namespace
+
+// No warnings below.
+extern "C" void g();
+extern "C" void h() {}
+
+#define VAR(v) v
+VAR(int m);
+
+extern "C" {
+void j() {}
+}
+
+struct Clike {
+  int i;
+};
+
+extern "C" int ik;
+extern "C" { int il; }
+
+void *operator new(__SIZE_TYPE__, int) { return 0; }
+void operator delete(void *, int) {}
+
+static void l() {}
+namespace {
+void m() {}
+}
+namespace x {
+void n() {}
+}
+
+int main() {}
Index: docs/clang-tidy/checks/google-global-names.rst
===
--- docs/clang-tidy/checks/google-global-names.rst
+++ docs/clang-tidy/checks/google-global-names.rst
@@ -1,10 +1,12 @@
-.. title:: clang-tidy - google-global-names-in-headers
+.. title:: clang-tidy - google-global-names
 
-google-global-names-in-headers
-==
+google-global-names
+===
 
 Flag global namespace pollution in header files.
-Right now it only triggers on ``using`` declarations and directives.
+Right now it only triggers on using declarations and directives in header files
+and declarations and definitions of functions, classes and variables in the
+global namespace.
 
 The check supports these options:
   - `HeaderFileExtensions`: a comma-separated list of filename extensions
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -59,6 +59,12 @@
 Improvements to clang-tidy
 --
 
+- `google-global-names
+  

Re: [PATCH] D23158: [clang-rename] merge tests when possible

[Kirill Bobyrev via cfe-commits]

omtcyfz updated this revision to Diff 66801.
omtcyfz marked an inline comment as done.

https://reviews.llvm.org/D23158

Files:
  clang-rename/USRFindingAction.cpp
  clang-rename/tool/ClangRename.cpp
  test/clang-rename/ClassAsTemplateArgument.cpp
  test/clang-rename/ClassAsTemplateArgumentFindByClass.cpp
  test/clang-rename/ClassAsTemplateArgumentFindByTemplateArgument.cpp
  test/clang-rename/ConstCastExpr.cpp
  test/clang-rename/FunctionWithClassFindByName.cpp
  test/clang-rename/TemplateFunction.cpp
  test/clang-rename/TemplateFunctionFindByDeclaration.cpp
  test/clang-rename/TemplateFunctionFindByUse.cpp
  test/clang-rename/TemplateTypename.cpp
  test/clang-rename/TemplateTypenameFindByTemplateParam.cpp
  test/clang-rename/TemplateTypenameFindByTypeInside.cpp
  test/clang-rename/UserDefinedConversion.cpp
  test/clang-rename/UserDefinedConversionFindByTypeDeclaration.cpp

Index: test/clang-rename/UserDefinedConversionFindByTypeDeclaration.cpp
===
--- test/clang-rename/UserDefinedConversionFindByTypeDeclaration.cpp
+++ /dev/null
@@ -1,26 +0,0 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=136 -new-name=Bar %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-
-class Foo { // CHECK: class Bar {
-//^ offset must be here
-public:
-  Foo() {}  // CHECK: Bar() {}
-};
-
-class Baz {
-public:
-  operator Foo() const {// CHECK: operator Bar() const {
-Foo foo;// CHECK: Bar foo;
-return foo;
-  }
-};
-
-int main() {
-  Baz boo;
-  Foo foo = static_cast(boo);  // CHECK: Bar foo = static_cast(boo);
-  return 0;
-}
-
-// Use grep -FUbo 'Foo'  to get the correct offset of Cla when changing
-// this file.
Index: test/clang-rename/UserDefinedConversion.cpp
===
--- test/clang-rename/UserDefinedConversion.cpp
+++ test/clang-rename/UserDefinedConversion.cpp
@@ -1,13 +1,22 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=205 -new-name=Bar %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-
-class Foo {}; // CHECK: class Bar {};
+class Foo { // CHECK: class Bar {
+public:
+  Foo() {}  // CHECK: Bar() {}
+};
 
 class Baz {
-  operator Foo() const {  // CHECK: operator Bar() const {
-// offset  ^
-Foo foo;  // CHECK: Bar foo;
+public:
+  operator Foo() const {// CHECK: operator Bar() const {
+Foo foo;// CHECK: Bar foo;
 return foo;
   }
 };
+
+int main() {
+  Baz boo;
+  Foo foo = static_cast(boo);  // CHECK: Bar foo = static_cast(boo);
+  return 0;
+}
+
+// RUN: clang-rename -offset=7 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+
+// RUN: clang-rename -offset=156 -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
Index: test/clang-rename/TemplateTypenameFindByTypeInside.cpp
===
--- test/clang-rename/TemplateTypenameFindByTypeInside.cpp
+++ /dev/null
@@ -1,18 +0,0 @@
-template  // CHECK: template 
-class Foo {
-T foo(T arg, T& ref, T* ptr) {// CHECK: U foo(U arg, U& ref, U* ptr) {
-  T value;// CHECK: U value;
-  int number = 42;
-  value = (T)number;  // CHECK: value = (U)number;
-  value = static_cast(number); // CHECK: value = static_cast(number);
-  return value;
-}
-
-static void foo(T value) {}   // CHECK: static void foo(U value) {}
-
-T member; // CHECK: U member;
-};
-
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=99 -new-name=U %t.cpp -i -- -fno-delayed-template-parsing
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
Index: test/clang-rename/TemplateTypenameFindByTemplateParam.cpp
===
--- test/clang-rename/TemplateTypenameFindByTemplateParam.cpp
+++ /dev/null
@@ -1,18 +0,0 @@
-template  // CHECK: template 
-class Foo {
-T foo(T arg, T& ref, T* ptr) {// CHECK: U foo(U arg, U& ref, U* ptr) {
-  T value;// CHECK: U value;
-  int number = 42;
-  value = (T)number;  // CHECK: value = (U)number;
-  value = static_cast(number); // CHECK: value = static_cast(number);
-  return value;
-}
-
-static void foo(T value) {}   // CHECK: static void foo(U value) {}
-
-T member; // CHECK: U member;
-};
-
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=19 -new-name=U %t.cpp -i -- -fno-delayed-template-parsing
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
Index: test/clang-rename/TemplateTypename.cpp
===
--- test/clang-rename/TemplateTypename.cpp
+++ test/clang-rename/TemplateTypename.cpp
@@ -1,12 +1,18 @@
-// Currently unsupported test.
-// RUN: cat %s > %t.cpp
-// FIXME: clang-rename should 

Re: [PATCH] D23130: [Clang-tidy] Add a check for definitions in the global namespace.

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

Could you run the check on LLVM and post a summary of results?



Comment at: clang-tidy/google/GlobalNamesCheck.cpp:90
@@ +89,3 @@
+// extern "C" globals need to be in the global namespace.
+if (VDecl->isExternC())
+  return;

Is this already filtered-out by the matcher?


Comment at: clang-tidy/google/GlobalNamesCheck.cpp:114
@@ +113,3 @@
+  // definitions into a global namespace or make them static with a FixIt.
+  diag(D->getLocation(), "%0 declared in the global namespace") << D;
+}

The warning explains, what's wrong, but doesn't tell why. We don't need the 
level of details of the documentation here, but maybe expand the message a bit 
to cover some of the possible effects (e.g. "this can cause ODR violations" or 
something like this)?


Comment at: docs/ReleaseNotes.rst:62
@@ -61,1 +61,3 @@
 
+- `google-global-names
+  `_ 
check

Please mention the old check name as well.


Comment at: docs/clang-tidy/checks/google-global-names.rst:4
@@ -4,2 +3,3 @@
+google-global-names
 ==
 

Cut extra =s.


https://reviews.llvm.org/D23130



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

Re: [PATCH] D22997: [cxx1z-constexpr-lambda] Make conversion function constexpr, and teach the expression-evaluator to evaluate the static-invoker.

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

Some small nits.



Comment at: lib/AST/ExprConstant.cpp:4409
@@ +4408,3 @@
+  } else if (MD && MD->isLambdaStaticInvoker()) {
+   
+// Map the static invoker for the lambda back to the call operator.

Nit: remove new line.


Comment at: lib/AST/ExprConstant.cpp:4415
@@ +4414,3 @@
+const CXXRecordDecl *ClosureClass = MD->getParent();
+ // number of captures better be zero.
+assert(std::distance(ClosureClass->captures_begin(),

Nit: Capitalize Number.


Comment at: lib/AST/ExprConstant.cpp:4416-4417
@@ +4415,4 @@
+ // number of captures better be zero.
+assert(std::distance(ClosureClass->captures_begin(),
+ ClosureClass->captures_end()) == 0);
+const CXXMethodDecl *LambdaCallOp = 
ClosureClass->getLambdaCallOperator();

Asserts should come with explanatory text &&'ed in.


Comment at: lib/AST/ExprConstant.cpp:4425
@@ +4424,3 @@
+if (ClosureClass->isGenericLambda()) {
+  assert(MD->isFunctionTemplateSpecialization());
+  const TemplateArgumentList *TAL = 
MD->getTemplateSpecializationArgs();

Same here as above (and elsewhere).


Repository:
  rL LLVM

https://reviews.llvm.org/D22997



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

Re: [PATCH] D22766: Handle -mlong-calls on Hexagon

[Krzysztof Parzyszek via cfe-commits]

kparzysz added a comment.

In https://reviews.llvm.org/D22766#505512, @echristo wrote:

> You haven't removed the custom handling?


The one from the driver?  If I remove it, it won't be passed to the compiler 
(and the testcase will fail).

I forgot to move the no-long-calls from the ARM group to the m group, so I will 
need to post another patch, but I'd like to know what specific changes you are 
expecting.  I think I was wrong in my earlier comment about the pre-existing 
handling of long-calls, since it only happens in the ARM target.


Repository:
  rL LLVM

https://reviews.llvm.org/D22766



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

Re: [PATCH] D23135: [clang-tidy] misc-argument-comment non-strict mode

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.


Comment at: clang-tidy/misc/ArgumentCommentCheck.cpp:124
@@ +123,3 @@
+  InDecl = InDecl.trim('_');
+  return InComment.compare_lower(InDecl) == 0;
+}

Correct, which means this won't behave properly in some locales with UTF-8 
identifiers. Consider Turkish, where İ (U+0130 “Latin Capital Letter I With Dot 
Above”) is the uppercase form of ı (U+0131 “Latin Small Letter Dotless I”). If 
the comment contains one version while the identifier contains the other, the 
comparison will currently fail, while a locale-aware comparison would succeed. 
You run into similar things with SS vs ß in German as well, where the uppercase 
form is two characters while the lowercase is only a single character.


https://reviews.llvm.org/D23135



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

Re: [PATCH] D23158: [clang-rename] merge tests when possible

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

Awesome! A few improvements are still possible though. See inline comments.



Comment at: test/clang-rename/ClassAsTemplateArgument.cpp:15
@@ +14,3 @@
+
+// RUN: cat %s > %t_0.cpp
+// RUN: clang-rename -offset=7 -new-name=Bar %t_0.cpp -i --

Since clang-rename can output the result to stdout, it makes sense to avoid 
copying files and switch to a single pipeline for each test case.


Comment at: test/clang-rename/ClassAsTemplateArgument.cpp:16
@@ +15,3 @@
+// RUN: cat %s > %t_0.cpp
+// RUN: clang-rename -offset=7 -new-name=Bar %t_0.cpp -i --
+// RUN: sed 's,//.*,,' %t_0.cpp | FileCheck %s

Please add instruction on how to find offsets (something along the lines of 
"n-th line of grep -b Foo test-file.cpp").


https://reviews.llvm.org/D23158



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

Re: [PATCH] D23135: [clang-tidy] misc-argument-comment non-strict mode

[Alexander Kornienko via cfe-commits]

alexfh updated this revision to Diff 66796.
alexfh marked 2 inline comments as done.
alexfh added a comment.

- Documented the StrictMode option, added a couple of consts.


https://reviews.llvm.org/D23135

Files:
  clang-tidy/ClangTidy.h
  clang-tidy/misc/ArgumentCommentCheck.cpp
  clang-tidy/misc/ArgumentCommentCheck.h
  docs/clang-tidy/checks/misc-argument-comment.rst
  test/clang-tidy/misc-argument-comment-strict.cpp
  test/clang-tidy/misc-argument-comment.cpp
  unittests/clang-tidy/MiscModuleTest.cpp

Index: unittests/clang-tidy/MiscModuleTest.cpp
===
--- unittests/clang-tidy/MiscModuleTest.cpp
+++ unittests/clang-tidy/MiscModuleTest.cpp
@@ -23,10 +23,10 @@
 TEST(ArgumentCommentCheckTest, OtherEditDistanceAboveThreshold) {
   EXPECT_EQ("void f(int xxx, int yyy); void g() { f(/*xxx=*/0, 0); }",
 runCheckOnCode(
-"void f(int xxx, int yyy); void g() { f(/*Xxx=*/0, 0); }"));
+"void f(int xxx, int yyy); void g() { f(/*Zxx=*/0, 0); }"));
   EXPECT_EQ("struct C { C(int xxx, int yyy); }; C c(/*xxx=*/0, 0);",
 runCheckOnCode(
-"struct C { C(int xxx, int yyy); }; C c(/*Xxx=*/0, 0);"));
+"struct C { C(int xxx, int yyy); }; C c(/*Zxx=*/0, 0);"));
 }
 
 TEST(ArgumentCommentCheckTest, OtherEditDistanceBelowThreshold) {
Index: test/clang-tidy/misc-argument-comment.cpp
===
--- test/clang-tidy/misc-argument-comment.cpp
+++ test/clang-tidy/misc-argument-comment.cpp
@@ -36,13 +36,18 @@
 
 void templates() {
   variadic(/*xxx=*/0, /*yyy=*/1);
-  variadic2(/*zzZ=*/0, /*xxx=*/1, /*yyy=*/2);
-  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: argument name 'zzZ' in comment does not match parameter name 'zzz'
+  variadic2(/*zzU=*/0, /*xxx=*/1, /*yyy=*/2);
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: argument name 'zzU' in comment does not match parameter name 'zzz'
   // CHECK-FIXES: variadic2(/*zzz=*/0, /*xxx=*/1, /*yyy=*/2);
 }
 
 #define FALSE 0
 void qqq(bool aaa);
 void f() { qqq(/*bbb=*/FALSE); }
 // CHECK-MESSAGES: [[@LINE-1]]:16: warning: argument name 'bbb' in comment does not match parameter name 'aaa'
 // CHECK-FIXES: void f() { qqq(/*bbb=*/FALSE); }
+
+void f(bool _with_underscores_);
+void ignores_underscores() {
+  f(/*With_Underscores=*/false);
+}
Index: test/clang-tidy/misc-argument-comment-strict.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-argument-comment-strict.cpp
@@ -0,0 +1,19 @@
+// RUN: %check_clang_tidy %s misc-argument-comment %t -- \
+// RUN:   -config="{CheckOptions: [{key: StrictMode, value: 1}]}" --
+
+void f(int _with_underscores_);
+void g(int x_);
+void ignores_underscores() {
+  f(/*With_Underscores=*/0);
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument name 'With_Underscores' in comment does not match parameter name '_with_underscores_'
+// CHECK-FIXES: f(/*_with_underscores_=*/0);
+  f(/*with_underscores=*/1);
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument name 'with_underscores' in comment does not match parameter name '_with_underscores_'
+// CHECK-FIXES: f(/*_with_underscores_=*/1);
+  f(/*_With_Underscores_=*/2);
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument name '_With_Underscores_' in comment does not match parameter name '_with_underscores_'
+// CHECK-FIXES: f(/*_with_underscores_=*/2);
+  g(/*X=*/3);
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: argument name 'X' in comment does not match parameter name 'x_'
+// CHECK-FIXES: g(/*x_=*/3);
+}
Index: docs/clang-tidy/checks/misc-argument-comment.rst
===
--- docs/clang-tidy/checks/misc-argument-comment.rst
+++ docs/clang-tidy/checks/misc-argument-comment.rst
@@ -18,3 +18,7 @@
   // warning: argument name 'bar' in comment does not match parameter name 'foo'
 
 The check tries to detect typos and suggest automated fixes for them.
+
+Supported options:
+  - `StrictMode` (local or global): when non-zero, the check will ignore leading
+and trailing underscores and case when comparing parameter names.
Index: clang-tidy/misc/ArgumentCommentCheck.h
===
--- clang-tidy/misc/ArgumentCommentCheck.h
+++ clang-tidy/misc/ArgumentCommentCheck.h
@@ -37,8 +37,10 @@
 
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult ) override;
+  void storeOptions(ClangTidyOptions::OptionMap& Opts) override;
 
 private:
+  const bool StrictMode;
   llvm::Regex IdentRE;
 
   bool isLikelyTypo(llvm::ArrayRef Params, StringRef ArgName,
Index: clang-tidy/misc/ArgumentCommentCheck.cpp
===
--- clang-tidy/misc/ArgumentCommentCheck.cpp
+++ clang-tidy/misc/ArgumentCommentCheck.cpp
@@ -22,16 +22,21 @@
 

Re: [PATCH] D23135: [clang-tidy] misc-argument-comment non-strict mode

[Alexander Kornienko via cfe-commits]

alexfh marked an inline comment as done.


Comment at: clang-tidy/misc/ArgumentCommentCheck.cpp:124
@@ +123,3 @@
+  InDecl = InDecl.trim('_');
+  return InComment.compare_lower(InDecl) == 0;
+}

aaron.ballman wrote:
> I think this is going to do the wrong thing for non-ASCII identifiers 
> containing characters for which lowercase comparisons make no sense. I'm okay 
> with that behavior (I don't think it should be a common occurrence), but 
> would like to see a test demonstrating a failure case with a FIXME.
compare_lower will only modify ascii characters, all other characters will be 
compared as is.

As a purely theoretical question: are there characters that have lower-case 
variants, but it doesn't make sense to compare the lower-case variants? Can you 
give an example?


https://reviews.llvm.org/D23135



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

Re: [PATCH] D22725: [clang-tidy] Add check 'modernize-use-algorithm'

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D22725#505020, @JDevlieghere wrote:

> Addresses comments from Aaron Ballman
>
> @aaron.ballman Thanks for the thorough review! Can you check whether the 
> tests I added address your concerns? Could you also elaborate on the case 
> with the C-function pointer? Unless I explicitly cast it to void* the 
> compiler rejects will reject it as an argument to memcpy. Am I missing a case 
> where this could go wrong? I still added it to the pointer arithmetic check 
> though, just to be sure.


The tests added mostly cover them -- I elaborated on the function pointer case, 
which I don't *think* will go wrong with this check, but should have a 
pathological test case for just to be sure.



Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:26
@@ +25,3 @@
+static QualType getStrippedType(QualType T) {
+  while (const auto *PtrType = T->getAs())
+T = PtrType->getPointeeType();

Consider a pathological case like:
```
#include 

void f();

int main() {
  typedef void (__cdecl *fp)();
  
  fp f1 = f;
  fp f2;
  memcpy(, , sizeof(fp)); // transforms into std::copy(, , ); ?
}
```
The calling convention is important in the test because `getStrippedType()` 
isn't looking through attributed types, which I *think* that declaration would 
rely on. I don't think it will cause problems in this case, but the test will 
ensure it.


Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:53-54
@@ +52,4 @@
+  const StringRef Arg2) {
+  Twine Replacement = Function + "(" + Arg0 + ", " + Arg1 + ", " + Arg2 + ")";
+  return Replacement.str();
+}

A more idiomatic way to write this is: `return (Twine(Function) + "(" + 
Arg0...).str();`


Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:106
@@ +105,3 @@
+  const SourceLocation Loc = MatchedExpr->getExprLoc();
+  auto Diag = diag(Loc, "'" + MatchedName.str() +
+"' reduces type-safety, consider using '" +

Instead of manually composing the diagnostic, it should use placeholders. e.g.,
```
diag(Loc, "%0 reduces type-safety, consider using '%1' instead) << Callee << 
ReplacedName;
```
(This makes it easier if we ever decide to localize our diagnostics.) Note that 
there's no quoting required around %0 because we're passing in a NamedDecl 
argument and the diagnostics engine understands how to display that.


Repository:
  rL LLVM

https://reviews.llvm.org/D22725



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

Re: [PATCH] D23160: [Coverage] Prevent creating a redundant counter if a nested body ends with a macro.

[Igor Kudrin via cfe-commits]

ikudrin added a comment.

The motivation sample:

  $ cat > test.cpp << EOF
  void dummy() {}
  
  #define MACRO dummy()
  
  int main()
  {
int i = 0;
while (i++ < 10)
  if (i < 5)
MACRO;
return 0;
  }
  EOF
  $ clang++ -fprofile-instr-generate -fcoverage-mapping dummy.cpp test.cpp
  $ ./a.out
  $ llvm-profdata merge -o default.profdata default.profraw
  $ llvm-cov show a.out -instr-profile --show-expansions default.profdata
  test.cpp:
4|1|void dummy() {}
 |2|
   14|3|#define MACRO dummy()
 |4|
 |5|int main()
1|6|{
1|7|  int i = 0;
   11|8|  while (i++ < 10)
   10|9|if (i < 5)
4|   10|  MACRO;
--
|  | 14|3|#define MACRO dummy()
--
1|   11|  return 0;
1|   12|}

After applying this patch the counter for MACRO shows a reasonable value:

  $ llvm-cov show a.out -instr-profile --show-expansions default.profdata
  test.cpp:
4|1|void dummy() {}
 |2|
4|3|#define MACRO dummy()
 |4|
 |5|int main()
1|6|{
1|7|  int i = 0;
   11|8|  while (i++ < 10)
   10|9|if (i < 5)
4|   10|  MACRO;
--
|  |  4|3|#define MACRO dummy()
--
1|   11|  return 0;
1|   12|}


https://reviews.llvm.org/D23160



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

Re: [PATCH] D23130: [Clang-tidy] Add a check for definitions in the global namespace.

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.


Comment at: clang-tidy/misc/GlobalNamespaceCheck.cpp:72
@@ +71,3 @@
+   "move it into a namespace or give it "
+   "internal linkage to avoid ODR conflicts")
+  << MatchedDecl;

ODR violations instead of ODR conflicts?


Comment at: docs/clang-tidy/checks/misc-global-namespace.rst:7
@@ +6,2 @@
+Finds definitions in the global namespace. Those definitions are prone to ODR
+conflicts.

violations instead of conflicts. Also, expanding the documentation a bit more 
would be useful -- not everyone understands what an ODR violation is, why it 
would be bad, etc. It can even be as simple as adding in an external link for 
more info.


Comment at: test/clang-tidy/misc-global-namespace.cpp:13
@@ +12,3 @@
+
+extern "C" void h() {}
+

Add a case for `extern "C" int i;`?

Also, isn't this a definition with external linkage in C++ (or is it only in 
C)? `extern int i = 30;` Assuming it's a definition, should this also trigger 
the diagnostic?


Repository:
  rL LLVM

https://reviews.llvm.org/D23130



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

Re: [PATCH] D21678: Fix For pr28288 - Error message in shift of vector values

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.


Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8597
@@ -8596,4 +8596,3 @@
 ///by a scalar or vector shift amount.
-static QualType checkOpenCLVectorShift(Sema ,
-   ExprResult , ExprResult ,
-   SourceLocation Loc, bool IsCompAssign) {
+static QualType checkVectorShift(Sema , ExprResult , ExprResult ,
+ SourceLocation Loc, bool IsCompAssign) {

Why does this drop the mention of OpenCL in the function name?


Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8638
@@ -8638,4 +8637,3 @@
   if (RHSVecTy) {
-// OpenCL v1.1 s6.3.j says that for vector types, the operators
-// are applied component-wise. So if RHS is a vector, then ensure
-// that the number of elements is the same as LHS...
+// For vector types, the operators are applied component-wise. So if RHS is
+// a vector, then ensure that the number of elements is the same as LHS...

It's good to keep language references in the comments, so I would leave the 
reference in even though this is being expanded for non-OpenCL vector types 
(unless the reference is incorrect).


Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8681-8683
@@ -8680,5 +8675,3 @@
 }
-return CheckVectorOperands(LHS, RHS, Loc, IsCompAssign,
-   /*AllowBothBool*/true,
-   /*AllowBoolConversions*/false);
   }

Why is it correct to remove semantic checking for vector operands?


Comment at: llvm/tools/clang/test/Sema/shift.c:75
@@ +74,3 @@
+void
+vect_shift_1 (vec16 *x)
+{

Please clang-format the test case.


https://reviews.llvm.org/D21678



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

[PATCH] D23160: [Coverage] Prevent creating a redundant counter if a nested body ends with a macro.

[Igor Kudrin via cfe-commits]

ikudrin created this revision.
ikudrin added reviewers: vsk, davidxl, bogner.
ikudrin added a subscriber: cfe-commits.

If there were several nested statements arranged in a way that all of them end 
up with the same macro,
then the expansion of this macro was assigned with all the corresponding 
counters of these statements.
As a result, the wrong counter value was shown for the macro in llvm-cov.

This patch fixes the issue by preventing adding a counter for an expanded 
source range
if it already has an assigned counter, which is expected to come from the most 
specific statement.

https://reviews.llvm.org/D23160

Files:
  lib/CodeGen/CoverageMappingGen.cpp
  test/CoverageMapping/macros.c

Index: test/CoverageMapping/macros.c
===
--- test/CoverageMapping/macros.c
+++ test/CoverageMapping/macros.c
@@ -36,8 +36,20 @@
 // CHECK-NEXT: File 1, 4:17 -> 4:22 = #0
 // CHECK-NEXT: File 2, 4:17 -> 4:22 = #0
 
+// CHECK-NEXT: func4
+void func4() { // CHECK-NEXT: File 0, [[@LINE]]:14 -> [[@LINE+6]]:2 = #0
+  int i = 0;
+  while (i++ < 10) // CHECK-NEXT: File 0, [[@LINE]]:10 -> [[@LINE]]:18 = (#0 + 
#1)
+if (i < 5) // CHECK-NEXT: File 0, [[@LINE]]:5 -> [[@LINE+2]]:14 = #1
+   // CHECK-NEXT: File 0, [[@LINE-1]]:9 -> [[@LINE-1]]:14 = #1
+  MACRO_2; // CHECK-NEXT: Expansion,File 0, [[@LINE]]:7 -> [[@LINE]]:14 = 
#2
+}
+// CHECK-NEXT: File 1, 4:17 -> 4:22 = #2
+// CHECK-NOT: File 1
+
 int main(int argc, const char *argv[]) {
   func();
   func2();
   func3();
+  func4();
 }
Index: lib/CodeGen/CoverageMappingGen.cpp
===
--- lib/CodeGen/CoverageMappingGen.cpp
+++ lib/CodeGen/CoverageMappingGen.cpp
@@ -432,7 +432,8 @@
   SourceLocation NestedLoc = getStartOfFileOrMacro(EndLoc);
   assert(SM.isWrittenInSameFile(NestedLoc, EndLoc));
 
-  SourceRegions.emplace_back(Region.getCounter(), NestedLoc, EndLoc);
+  if (!isRegionAlreadyAdded(NestedLoc, EndLoc))
+SourceRegions.emplace_back(Region.getCounter(), NestedLoc, EndLoc);
 
   EndLoc = getPreciseTokenLocEnd(getIncludeOrExpansionLoc(EndLoc));
   if (EndLoc.isInvalid())


Index: test/CoverageMapping/macros.c
===
--- test/CoverageMapping/macros.c
+++ test/CoverageMapping/macros.c
@@ -36,8 +36,20 @@
 // CHECK-NEXT: File 1, 4:17 -> 4:22 = #0
 // CHECK-NEXT: File 2, 4:17 -> 4:22 = #0
 
+// CHECK-NEXT: func4
+void func4() { // CHECK-NEXT: File 0, [[@LINE]]:14 -> [[@LINE+6]]:2 = #0
+  int i = 0;
+  while (i++ < 10) // CHECK-NEXT: File 0, [[@LINE]]:10 -> [[@LINE]]:18 = (#0 + #1)
+if (i < 5) // CHECK-NEXT: File 0, [[@LINE]]:5 -> [[@LINE+2]]:14 = #1
+   // CHECK-NEXT: File 0, [[@LINE-1]]:9 -> [[@LINE-1]]:14 = #1
+  MACRO_2; // CHECK-NEXT: Expansion,File 0, [[@LINE]]:7 -> [[@LINE]]:14 = #2
+}
+// CHECK-NEXT: File 1, 4:17 -> 4:22 = #2
+// CHECK-NOT: File 1
+
 int main(int argc, const char *argv[]) {
   func();
   func2();
   func3();
+  func4();
 }
Index: lib/CodeGen/CoverageMappingGen.cpp
===
--- lib/CodeGen/CoverageMappingGen.cpp
+++ lib/CodeGen/CoverageMappingGen.cpp
@@ -432,7 +432,8 @@
   SourceLocation NestedLoc = getStartOfFileOrMacro(EndLoc);
   assert(SM.isWrittenInSameFile(NestedLoc, EndLoc));
 
-  SourceRegions.emplace_back(Region.getCounter(), NestedLoc, EndLoc);
+  if (!isRegionAlreadyAdded(NestedLoc, EndLoc))
+SourceRegions.emplace_back(Region.getCounter(), NestedLoc, EndLoc);
 
   EndLoc = getPreciseTokenLocEnd(getIncludeOrExpansionLoc(EndLoc));
   if (EndLoc.isInvalid())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D23135: [clang-tidy] misc-argument-comment non-strict mode

[Aaron Ballman via cfe-commits]

aaron.ballman added a subscriber: aaron.ballman.


Comment at: clang-tidy/misc/ArgumentCommentCheck.cpp:124
@@ +123,3 @@
+  InDecl = InDecl.trim('_');
+  return InComment.compare_lower(InDecl) == 0;
+}

I think this is going to do the wrong thing for non-ASCII identifiers 
containing characters for which lowercase comparisons make no sense. I'm okay 
with that behavior (I don't think it should be a common occurrence), but would 
like to see a test demonstrating a failure case with a FIXME.


https://reviews.llvm.org/D23135



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

Re: [PATCH] D23135: [clang-tidy] misc-argument-comment non-strict mode

[Haojian Wu via cfe-commits]

hokein added inline comments.


Comment at: clang-tidy/misc/ArgumentCommentCheck.cpp:29
@@ +28,3 @@
+void ArgumentCommentCheck::storeOptions(ClangTidyOptions::OptionMap ) {
+  Options.store(Opts, "StrictMode", StrictMode);
+}

I think we should add a `StringMode` description in `ArgumentCommentCheck`'s 
document. 


Comment at: clang-tidy/misc/ArgumentCommentCheck.cpp:180
@@ -165,3 +179,3 @@
   const Expr *E = Result.Nodes.getNodeAs("expr");
-  if (auto Call = dyn_cast(E)) {
+  if (auto *Call = dyn_cast(E)) {
 const FunctionDecl *Callee = Call->getDirectCallee();

`const auto *`


Comment at: clang-tidy/misc/ArgumentCommentCheck.cpp:188
@@ -173,3 +187,3 @@
   } else {
-auto Construct = cast(E);
+auto *Construct = cast(E);
 checkCallArgs(

`const auto *`


https://reviews.llvm.org/D23135



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

Re: [PATCH] D21678: Fix For pr28288 - Error message in shift of vector values

[Vladimir Yakovlev via cfe-commits]

vbyakovl added a comment.

Someone, please review this.


https://reviews.llvm.org/D21678



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

Re: [PATCH] D23153: Run clang-format on clang-rename code

[Miklos Vajna via cfe-commits]

vmiklos added a comment.

Thanks. Yes, I used the Linux-distro-provided clang-format-3.7, I didn't notice 
that the trunk version now also sorts includes.


Repository:
  rL LLVM

https://reviews.llvm.org/D23153



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

Re: [PATCH] D22374: [analyzer] Copy and move constructors - ExprEngine extended for "almost trivial" copy and move constructors

[Balogh , Ádám via cfe-commits]

baloghadamsoftware added a comment.

In https://reviews.llvm.org/D22374#504855, @NoQ wrote:

> Hmm. I suggest:
>
> 1. Change this test's constructor so that it was no longer almost-trivial. 
> Because it isn't significant for this test if the constructor is 
> almost-trivial or not. The test would pass.


It is significant, because I want to test here the almost-trivial constructors.

> 

> 

> 2. Add a FIXME-test for this checker, in which a completely undefined 
> structure is being copied. Perhaps somebody implements this feature some day.


So it would be a new function with the same structure, and commented out 
beginning with a FIXME?

> 3. Leave out the situation that the current test checks as a grey-area. I'm 
> still not convinced that this situation (copying a partially-initialized 
> almost-trivial structure) deserves a warning from this checker.


This is what I originally did here in the patch. Maybe a FIXME could be written 
there a well?


https://reviews.llvm.org/D22374



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

Re: [PATCH] D23130: [Clang-tidy] Add a check for definitions in the global namespace.

[Benjamin Kramer via cfe-commits]

bkramer added a comment.

In https://reviews.llvm.org/D23130#505290, @alexfh wrote:

> And the second difference is that it is limited to definitions, but I don't 
> yet understand, why it is important, since declarations in the global 
> namespace (except for using declarations, typedefs, etc.) will have a 
> corresponding definition in the global namespace. I guess, these checks, if 
> somewhat different, may be similar enough to share some implementation.


My reasoning was to avoid duplicated warnings (on both declaration and 
definition) and false positives on headers importing C symbols. I think that 
the latter would trigger an explosion in false positives :(



Comment at: clang-tidy/misc/GlobalNamespaceCheck.cpp:46
@@ +45,3 @@
+// extern "C" globals need to be in the global namespace.
+if (VDecl->isExternC())
+  return;

Prazek wrote:
> I think it would be better to check it in matcher.
> I see that there is isExternC, but it only works for FunctionDecl, but I 
> think that adding isExternC for VarDecl would be good
I'll fix this once we figured out where to put this check.


Comment at: clang-tidy/misc/GlobalNamespaceCheck.cpp:67
@@ +66,3 @@
+  }
+
+  // FIXME: If we want to be really fancy we could move declaration-less

Prazek wrote:
> What about macros? I think you should check isMacroId on location here (don't 
> do fixit when it is in macro, but print warning)
> 
> Also please add tests for it.
The FixIt isn't actually implemented. Adding FixIts for this check is something 
I'm planning to do, but it's not part of this patch and doesn't make sense to 
implement before we know that this check is actually working properly.


Repository:
  rL LLVM

https://reviews.llvm.org/D23130



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

r277712 - Make isExternC work on VarDecls too.

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Thu Aug  4 05:02:03 2016
New Revision: 277712

URL: http://llvm.org/viewvc/llvm-project?rev=277712=rev
Log:
Make isExternC work on VarDecls too.

Modified:
cfe/trunk/docs/LibASTMatchersReference.html
cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Modified: cfe/trunk/docs/LibASTMatchersReference.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersReference.html?rev=277712=277711=277712=diff
==
--- cfe/trunk/docs/LibASTMatchersReference.html (original)
+++ cfe/trunk/docs/LibASTMatchersReference.html Thu Aug  4 05:02:03 2016
@@ -3441,6 +3441,18 @@ Usable as: Matcherhttp://cl
 
 
 
+Matcherhttp://clang.llvm.org/doxygen/classclang_1_1VarDecl.html;>VarDeclisExternC
+Matches extern "C" 
function declarations.
+
+Given:
+  extern "C" void f() {}
+  extern "C" { void g() {} }
+  void h() {}
+functionDecl(isExternC())
+  matches the declaration of f and g, but not the declaration h
+
+
+
 Matcherhttp://clang.llvm.org/doxygen/classclang_1_1VarDecl.html;>VarDeclisTemplateInstantiation
 Matches 
template instantiations of function, class, or static
 member variable template instantiations.

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h?rev=277712=277711=277712=diff
==
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h Thu Aug  4 05:02:03 2016
@@ -3342,7 +3342,8 @@ AST_MATCHER_P(FunctionDecl, returns,
 /// \endcode
 /// functionDecl(isExternC())
 ///   matches the declaration of f and g, but not the declaration h
-AST_MATCHER(FunctionDecl, isExternC) {
+AST_POLYMORPHIC_MATCHER(isExternC, 
AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl,
+   VarDecl)) {
   return Node.isExternC();
 }
 

Modified: cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp?rev=277712=277711=277712=diff
==
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp (original)
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp Thu Aug  4 
05:02:03 2016
@@ -842,6 +842,12 @@ TEST(IsExternC, MatchesExternCFunctionDe
   EXPECT_TRUE(notMatches("void f() {}", functionDecl(isExternC(;
 }
 
+TEST(IsExternC, MatchesExternCVariableDeclarations) {
+  EXPECT_TRUE(matches("extern \"C\" int i;", varDecl(isExternC(;
+  EXPECT_TRUE(matches("extern \"C\" { int i; }", varDecl(isExternC(;
+  EXPECT_TRUE(notMatches("int i;", varDecl(isExternC(;
+}
+
 TEST(IsDefaulted, MatchesDefaultedFunctionDeclarations) {
   EXPECT_TRUE(notMatches("class A { ~A(); };",
  functionDecl(hasName("~A"), isDefaulted(;


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

Re: [PATCH] D23153: Run clang-format on clang-rename code

[Kirill Bobyrev via cfe-commits]

omtcyfz added a comment.

Actually few changes `clang-format` would produce weren't there. It's either 
you used old `clang-format` or you missed them, but there were header 
reordering issues. Pushed quick fix.


Repository:
  rL LLVM

https://reviews.llvm.org/D23153



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

[clang-tools-extra] r277709 - [clang-rename] add missing clang-format improvements

[Kirill Bobyrev via cfe-commits]

Author: omtcyfz
Date: Thu Aug  4 04:23:30 2016
New Revision: 277709

URL: http://llvm.org/viewvc/llvm-project?rev=277709=rev
Log:
[clang-rename] add missing clang-format improvements

r277702 introduced clang-format changes so that later commits wouldn't introduce
non-functional changes while running clang-format before commiting. Though,
few changes by clang-format weren't in the patch.

Modified:
clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp
clang-tools-extra/trunk/clang-rename/tool/ClangRename.cpp

Modified: clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp?rev=277709=277708=277709=diff
==
--- clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp (original)
+++ clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp Thu Aug  4 
04:23:30 2016
@@ -29,8 +29,8 @@
 #include "clang/Tooling/Refactoring.h"
 #include "clang/Tooling/Tooling.h"
 #include 
-#include 
 #include 
+#include 
 #include 
 
 using namespace llvm;

Modified: clang-tools-extra/trunk/clang-rename/tool/ClangRename.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-rename/tool/ClangRename.cpp?rev=277709=277708=277709=diff
==
--- clang-tools-extra/trunk/clang-rename/tool/ClangRename.cpp (original)
+++ clang-tools-extra/trunk/clang-rename/tool/ClangRename.cpp Thu Aug  4 
04:23:30 2016
@@ -13,8 +13,8 @@
 ///
 
//===--===//
 
-#include "../USRFindingAction.h"
 #include "../RenamingAction.h"
+#include "../USRFindingAction.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/FileManager.h"
@@ -31,8 +31,8 @@
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
-#include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/YAMLTraits.h"
+#include "llvm/Support/raw_ostream.h"
 #include 
 #include 
 #include 


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

[PATCH] D23158: [clang-rename] merge tests when possible

[Kirill Bobyrev via cfe-commits]

omtcyfz created this revision.
omtcyfz added reviewers: alexfh, klimek.
omtcyfz added a subscriber: cfe-commits.

The only difference between some tests is `-offset` passed to `clang-rename`. 
It makes sense to merge them into a single file and add multiple tool 
invocations.

https://reviews.llvm.org/D23158

Files:
  test/clang-rename/ClassAsTemplateArgument.cpp
  test/clang-rename/ClassAsTemplateArgumentFindByClass.cpp
  test/clang-rename/ClassAsTemplateArgumentFindByTemplateArgument.cpp
  test/clang-rename/FunctionWithClassFindByName.cpp
  test/clang-rename/TemplateFunction.cpp
  test/clang-rename/TemplateFunctionFindByDeclaration.cpp
  test/clang-rename/TemplateFunctionFindByUse.cpp
  test/clang-rename/TemplateTypename.cpp
  test/clang-rename/TemplateTypenameFindByTemplateParam.cpp
  test/clang-rename/TemplateTypenameFindByTypeInside.cpp
  test/clang-rename/UserDefinedConversion.cpp
  test/clang-rename/UserDefinedConversionFindByTypeDeclaration.cpp

Index: test/clang-rename/UserDefinedConversionFindByTypeDeclaration.cpp
===
--- test/clang-rename/UserDefinedConversionFindByTypeDeclaration.cpp
+++ /dev/null
@@ -1,26 +0,0 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=136 -new-name=Bar %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-
-class Foo { // CHECK: class Bar {
-//^ offset must be here
-public:
-  Foo() {}  // CHECK: Bar() {}
-};
-
-class Baz {
-public:
-  operator Foo() const {// CHECK: operator Bar() const {
-Foo foo;// CHECK: Bar foo;
-return foo;
-  }
-};
-
-int main() {
-  Baz boo;
-  Foo foo = static_cast(boo);  // CHECK: Bar foo = static_cast(boo);
-  return 0;
-}
-
-// Use grep -FUbo 'Foo'  to get the correct offset of Cla when changing
-// this file.
Index: test/clang-rename/UserDefinedConversion.cpp
===
--- test/clang-rename/UserDefinedConversion.cpp
+++ test/clang-rename/UserDefinedConversion.cpp
@@ -1,13 +1,26 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=205 -new-name=Bar %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-
-class Foo {}; // CHECK: class Bar {};
+class Foo { // CHECK: class Bar {
+public:
+  Foo() {}  // CHECK: Bar() {}
+};
 
 class Baz {
-  operator Foo() const {  // CHECK: operator Bar() const {
-// offset  ^
-Foo foo;  // CHECK: Bar foo;
+public:
+  operator Foo() const {// CHECK: operator Bar() const {
+Foo foo;// CHECK: Bar foo;
 return foo;
   }
 };
+
+int main() {
+  Baz boo;
+  Foo foo = static_cast(boo);  // CHECK: Bar foo = static_cast(boo);
+  return 0;
+}
+
+// RUN: cat %s > %t_0.cpp
+// RUN: clang-rename -offset=7 -new-name=Bar %t_0.cpp -i --
+// RUN: sed 's,//.*,,' %t_0.cpp | FileCheck %s
+
+// RUN: cat %s > %t_1.cpp
+// RUN: clang-rename -offset=156 -new-name=Bar %t_1.cpp -i --
+// RUN: sed 's,//.*,,' %t_1.cpp | FileCheck %s
Index: test/clang-rename/TemplateTypenameFindByTypeInside.cpp
===
--- test/clang-rename/TemplateTypenameFindByTypeInside.cpp
+++ /dev/null
@@ -1,18 +0,0 @@
-template  // CHECK: template 
-class Foo {
-T foo(T arg, T& ref, T* ptr) {// CHECK: U foo(U arg, U& ref, U* ptr) {
-  T value;// CHECK: U value;
-  int number = 42;
-  value = (T)number;  // CHECK: value = (U)number;
-  value = static_cast(number); // CHECK: value = static_cast(number);
-  return value;
-}
-
-static void foo(T value) {}   // CHECK: static void foo(U value) {}
-
-T member; // CHECK: U member;
-};
-
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=99 -new-name=U %t.cpp -i -- -fno-delayed-template-parsing
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
Index: test/clang-rename/TemplateTypenameFindByTemplateParam.cpp
===
--- test/clang-rename/TemplateTypenameFindByTemplateParam.cpp
+++ /dev/null
@@ -1,18 +0,0 @@
-template  // CHECK: template 
-class Foo {
-T foo(T arg, T& ref, T* ptr) {// CHECK: U foo(U arg, U& ref, U* ptr) {
-  T value;// CHECK: U value;
-  int number = 42;
-  value = (T)number;  // CHECK: value = (U)number;
-  value = static_cast(number); // CHECK: value = static_cast(number);
-  return value;
-}
-
-static void foo(T value) {}   // CHECK: static void foo(U value) {}
-
-T member; // CHECK: U member;
-};
-
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=19 -new-name=U %t.cpp -i -- -fno-delayed-template-parsing
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
Index: test/clang-rename/TemplateTypename.cpp
===
--- 

Re: [PATCH] D22774: [MSVC] Add ARM support to intrin.h for MSVC compatibility

[Martin Storsjö via cfe-commits]

mstorsjo added a comment.

Ping - any reply from @compnerd?

In any case, feel free to formulate the ifdefs whichever way you want on commit 
as well (since I don't have commit access), as long as the included test passes.


https://reviews.llvm.org/D22774



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

Re: [PATCH] D23153: Run clang-format on clang-rename code

[Miklos Vajna via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL277702: Run clang-format on clang-rename code (authored by 
vmiklos).

Changed prior to commit:
  https://reviews.llvm.org/D23153?vs=66766=66769#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D23153

Files:
  clang-tools-extra/trunk/clang-rename/RenamingAction.h
  clang-tools-extra/trunk/clang-rename/USRFinder.h
  clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp

Index: clang-tools-extra/trunk/clang-rename/RenamingAction.h
===
--- clang-tools-extra/trunk/clang-rename/RenamingAction.h
+++ clang-tools-extra/trunk/clang-rename/RenamingAction.h
@@ -41,7 +41,6 @@
   std::map 
   bool PrintLocations;
 };
-
 }
 }
 
Index: clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp
===
--- clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp
+++ clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp
@@ -33,7 +33,6 @@
 #include 
 #include 
 
-
 using namespace llvm;
 
 namespace clang {
@@ -47,8 +46,8 @@
 class AdditionalUSRFinder : public RecursiveASTVisitor {
 public:
   explicit AdditionalUSRFinder(const Decl *FoundDecl, ASTContext ,
- std::vector *USRs)
-: FoundDecl(FoundDecl), Context(Context), USRs(USRs) {}
+   std::vector *USRs)
+  : FoundDecl(FoundDecl), Context(Context), USRs(USRs) {}
 
   void Find() {
 // Fill OverriddenMethods and PartialSpecs storages.
@@ -63,7 +62,7 @@
 } else if (const auto *RecordDecl = dyn_cast(FoundDecl)) {
   handleCXXRecordDecl(RecordDecl);
 } else if (const auto *TemplateDecl =
-   dyn_cast(FoundDecl)) {
+   dyn_cast(FoundDecl)) {
   handleClassTemplateDecl(TemplateDecl);
 } else {
   USRSet.insert(getUSRForDecl(FoundDecl));
@@ -87,8 +86,8 @@
 private:
   void handleCXXRecordDecl(const CXXRecordDecl *RecordDecl) {
 RecordDecl = RecordDecl->getDefinition();
-if (const auto *ClassTemplateSpecDecl
-= dyn_cast(RecordDecl)) {
+if (const auto *ClassTemplateSpecDecl =
+dyn_cast(RecordDecl)) {
   handleClassTemplateDecl(ClassTemplateSpecDecl->getSpecializedTemplate());
 }
 addUSRsOfCtorDtors(RecordDecl);
@@ -137,8 +136,8 @@
   ASTContext 
   std::vector *USRs;
   std::set USRSet;
-  std::vector OverriddenMethods;
-  std::vector PartialSpecs;
+  std::vector OverriddenMethods;
+  std::vector PartialSpecs;
 };
 } // namespace
 
Index: clang-tools-extra/trunk/clang-rename/USRFinder.h
===
--- clang-tools-extra/trunk/clang-rename/USRFinder.h
+++ clang-tools-extra/trunk/clang-rename/USRFinder.h
@@ -49,7 +49,7 @@
 class NestedNameSpecifierLocFinder : public MatchFinder::MatchCallback {
 public:
   explicit NestedNameSpecifierLocFinder(ASTContext )
-: Context(Context) {}
+  : Context(Context) {}
 
   std::vector getNestedNameSpecifierLocations() {
 addMatchers();
@@ -65,16 +65,15 @@
   }
 
   virtual void run(const MatchFinder::MatchResult ) {
-const auto *NNS =
-
Result.Nodes.getNodeAs("nestedNameSpecifierLoc");
+const auto *NNS = Result.Nodes.getNodeAs(
+"nestedNameSpecifierLoc");
 Locations.push_back(*NNS);
   }
 
   ASTContext 
   std::vector Locations;
   MatchFinder Finder;
 };
-
 }
 }
 


Index: clang-tools-extra/trunk/clang-rename/RenamingAction.h
===
--- clang-tools-extra/trunk/clang-rename/RenamingAction.h
+++ clang-tools-extra/trunk/clang-rename/RenamingAction.h
@@ -41,7 +41,6 @@
   std::map 
   bool PrintLocations;
 };
-
 }
 }
 
Index: clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp
===
--- clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp
+++ clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp
@@ -33,7 +33,6 @@
 #include 
 #include 
 
-
 using namespace llvm;
 
 namespace clang {
@@ -47,8 +46,8 @@
 class AdditionalUSRFinder : public RecursiveASTVisitor {
 public:
   explicit AdditionalUSRFinder(const Decl *FoundDecl, ASTContext ,
- std::vector *USRs)
-: FoundDecl(FoundDecl), Context(Context), USRs(USRs) {}
+   std::vector *USRs)
+  : FoundDecl(FoundDecl), Context(Context), USRs(USRs) {}
 
   void Find() {
 // Fill OverriddenMethods and PartialSpecs storages.
@@ -63,7 +62,7 @@
 } else if (const auto *RecordDecl = dyn_cast(FoundDecl)) {
   handleCXXRecordDecl(RecordDecl);
 } else if (const auto *TemplateDecl =
-   dyn_cast(FoundDecl)) {
+   dyn_cast(FoundDecl)) {
   handleClassTemplateDecl(TemplateDecl);
 } else {
   

[clang-tools-extra] r277702 - Run clang-format on clang-rename code

[Miklos Vajna via cfe-commits]

Author: vmiklos
Date: Thu Aug  4 02:43:29 2016
New Revision: 277702

URL: http://llvm.org/viewvc/llvm-project?rev=277702=rev
Log:
Run clang-format on clang-rename code

So that later commits don't introduce non-functional changes when
running clang-format before committing.

Reviewers: klimek

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

Modified:
clang-tools-extra/trunk/clang-rename/RenamingAction.h
clang-tools-extra/trunk/clang-rename/USRFinder.h
clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp

Modified: clang-tools-extra/trunk/clang-rename/RenamingAction.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-rename/RenamingAction.h?rev=277702=277701=277702=diff
==
--- clang-tools-extra/trunk/clang-rename/RenamingAction.h (original)
+++ clang-tools-extra/trunk/clang-rename/RenamingAction.h Thu Aug  4 02:43:29 
2016
@@ -41,7 +41,6 @@ private:
   std::map 
   bool PrintLocations;
 };
-
 }
 }
 

Modified: clang-tools-extra/trunk/clang-rename/USRFinder.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-rename/USRFinder.h?rev=277702=277701=277702=diff
==
--- clang-tools-extra/trunk/clang-rename/USRFinder.h (original)
+++ clang-tools-extra/trunk/clang-rename/USRFinder.h Thu Aug  4 02:43:29 2016
@@ -49,7 +49,7 @@ std::string getUSRForDecl(const Decl *De
 class NestedNameSpecifierLocFinder : public MatchFinder::MatchCallback {
 public:
   explicit NestedNameSpecifierLocFinder(ASTContext )
-: Context(Context) {}
+  : Context(Context) {}
 
   std::vector getNestedNameSpecifierLocations() {
 addMatchers();
@@ -65,8 +65,8 @@ private:
   }
 
   virtual void run(const MatchFinder::MatchResult ) {
-const auto *NNS =
-
Result.Nodes.getNodeAs("nestedNameSpecifierLoc");
+const auto *NNS = Result.Nodes.getNodeAs(
+"nestedNameSpecifierLoc");
 Locations.push_back(*NNS);
   }
 
@@ -74,7 +74,6 @@ private:
   std::vector Locations;
   MatchFinder Finder;
 };
-
 }
 }
 

Modified: clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp?rev=277702=277701=277702=diff
==
--- clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp (original)
+++ clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp Thu Aug  4 
02:43:29 2016
@@ -33,7 +33,6 @@
 #include 
 #include 
 
-
 using namespace llvm;
 
 namespace clang {
@@ -47,8 +46,8 @@ namespace {
 class AdditionalUSRFinder : public RecursiveASTVisitor {
 public:
   explicit AdditionalUSRFinder(const Decl *FoundDecl, ASTContext ,
- std::vector *USRs)
-: FoundDecl(FoundDecl), Context(Context), USRs(USRs) {}
+   std::vector *USRs)
+  : FoundDecl(FoundDecl), Context(Context), USRs(USRs) {}
 
   void Find() {
 // Fill OverriddenMethods and PartialSpecs storages.
@@ -63,7 +62,7 @@ public:
 } else if (const auto *RecordDecl = dyn_cast(FoundDecl)) {
   handleCXXRecordDecl(RecordDecl);
 } else if (const auto *TemplateDecl =
-   dyn_cast(FoundDecl)) {
+   dyn_cast(FoundDecl)) {
   handleClassTemplateDecl(TemplateDecl);
 } else {
   USRSet.insert(getUSRForDecl(FoundDecl));
@@ -87,8 +86,8 @@ public:
 private:
   void handleCXXRecordDecl(const CXXRecordDecl *RecordDecl) {
 RecordDecl = RecordDecl->getDefinition();
-if (const auto *ClassTemplateSpecDecl
-= dyn_cast(RecordDecl)) {
+if (const auto *ClassTemplateSpecDecl =
+dyn_cast(RecordDecl)) {
   handleClassTemplateDecl(ClassTemplateSpecDecl->getSpecializedTemplate());
 }
 addUSRsOfCtorDtors(RecordDecl);
@@ -137,8 +136,8 @@ private:
   ASTContext 
   std::vector *USRs;
   std::set USRSet;
-  std::vector OverriddenMethods;
-  std::vector PartialSpecs;
+  std::vector OverriddenMethods;
+  std::vector PartialSpecs;
 };
 } // namespace
 


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

Re: [PATCH] D23153: Run clang-format on clang-rename code

[Manuel Klimek via cfe-commits]

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

lg


https://reviews.llvm.org/D23153



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

[PATCH] D23153: Run clang-format on clang-rename code

[Miklos Vajna via cfe-commits]

vmiklos created this revision.
vmiklos added reviewers: klimek, omtcyfz.
vmiklos added a subscriber: cfe-commits.

So that later commits don't introduce non-functional changes when
running clang-format before committing.

https://reviews.llvm.org/D23153

Files:
  clang-rename/RenamingAction.h
  clang-rename/USRFinder.h
  clang-rename/USRFindingAction.cpp

Index: clang-rename/USRFindingAction.cpp
===
--- clang-rename/USRFindingAction.cpp
+++ clang-rename/USRFindingAction.cpp
@@ -33,7 +33,6 @@
 #include 
 #include 
 
-
 using namespace llvm;
 
 namespace clang {
@@ -47,8 +46,8 @@
 class AdditionalUSRFinder : public RecursiveASTVisitor {
 public:
   explicit AdditionalUSRFinder(const Decl *FoundDecl, ASTContext ,
- std::vector *USRs)
-: FoundDecl(FoundDecl), Context(Context), USRs(USRs) {}
+   std::vector *USRs)
+  : FoundDecl(FoundDecl), Context(Context), USRs(USRs) {}
 
   void Find() {
 // Fill OverriddenMethods and PartialSpecs storages.
@@ -63,7 +62,7 @@
 } else if (const auto *RecordDecl = dyn_cast(FoundDecl)) {
   handleCXXRecordDecl(RecordDecl);
 } else if (const auto *TemplateDecl =
-   dyn_cast(FoundDecl)) {
+   dyn_cast(FoundDecl)) {
   handleClassTemplateDecl(TemplateDecl);
 } else {
   USRSet.insert(getUSRForDecl(FoundDecl));
@@ -87,8 +86,8 @@
 private:
   void handleCXXRecordDecl(const CXXRecordDecl *RecordDecl) {
 RecordDecl = RecordDecl->getDefinition();
-if (const auto *ClassTemplateSpecDecl
-= dyn_cast(RecordDecl)) {
+if (const auto *ClassTemplateSpecDecl =
+dyn_cast(RecordDecl)) {
   handleClassTemplateDecl(ClassTemplateSpecDecl->getSpecializedTemplate());
 }
 addUSRsOfCtorDtors(RecordDecl);
@@ -137,8 +136,8 @@
   ASTContext 
   std::vector *USRs;
   std::set USRSet;
-  std::vector OverriddenMethods;
-  std::vector PartialSpecs;
+  std::vector OverriddenMethods;
+  std::vector PartialSpecs;
 };
 } // namespace
 
Index: clang-rename/USRFinder.h
===
--- clang-rename/USRFinder.h
+++ clang-rename/USRFinder.h
@@ -49,7 +49,7 @@
 class NestedNameSpecifierLocFinder : public MatchFinder::MatchCallback {
 public:
   explicit NestedNameSpecifierLocFinder(ASTContext )
-: Context(Context) {}
+  : Context(Context) {}
 
   std::vector getNestedNameSpecifierLocations() {
 addMatchers();
@@ -65,16 +65,15 @@
   }
 
   virtual void run(const MatchFinder::MatchResult ) {
-const auto *NNS =
-
Result.Nodes.getNodeAs("nestedNameSpecifierLoc");
+const auto *NNS = Result.Nodes.getNodeAs(
+"nestedNameSpecifierLoc");
 Locations.push_back(*NNS);
   }
 
   ASTContext 
   std::vector Locations;
   MatchFinder Finder;
 };
-
 }
 }
 
Index: clang-rename/RenamingAction.h
===
--- clang-rename/RenamingAction.h
+++ clang-rename/RenamingAction.h
@@ -41,7 +41,6 @@
   std::map 
   bool PrintLocations;
 };
-
 }
 }
 


Index: clang-rename/USRFindingAction.cpp
===
--- clang-rename/USRFindingAction.cpp
+++ clang-rename/USRFindingAction.cpp
@@ -33,7 +33,6 @@
 #include 
 #include 
 
-
 using namespace llvm;
 
 namespace clang {
@@ -47,8 +46,8 @@
 class AdditionalUSRFinder : public RecursiveASTVisitor {
 public:
   explicit AdditionalUSRFinder(const Decl *FoundDecl, ASTContext ,
- std::vector *USRs)
-: FoundDecl(FoundDecl), Context(Context), USRs(USRs) {}
+   std::vector *USRs)
+  : FoundDecl(FoundDecl), Context(Context), USRs(USRs) {}
 
   void Find() {
 // Fill OverriddenMethods and PartialSpecs storages.
@@ -63,7 +62,7 @@
 } else if (const auto *RecordDecl = dyn_cast(FoundDecl)) {
   handleCXXRecordDecl(RecordDecl);
 } else if (const auto *TemplateDecl =
-   dyn_cast(FoundDecl)) {
+   dyn_cast(FoundDecl)) {
   handleClassTemplateDecl(TemplateDecl);
 } else {
   USRSet.insert(getUSRForDecl(FoundDecl));
@@ -87,8 +86,8 @@
 private:
   void handleCXXRecordDecl(const CXXRecordDecl *RecordDecl) {
 RecordDecl = RecordDecl->getDefinition();
-if (const auto *ClassTemplateSpecDecl
-= dyn_cast(RecordDecl)) {
+if (const auto *ClassTemplateSpecDecl =
+dyn_cast(RecordDecl)) {
   handleClassTemplateDecl(ClassTemplateSpecDecl->getSpecializedTemplate());
 }
 addUSRsOfCtorDtors(RecordDecl);
@@ -137,8 +136,8 @@
   ASTContext 
   std::vector *USRs;
   std::set USRSet;
-  std::vector OverriddenMethods;
-  std::vector PartialSpecs;
+  std::vector OverriddenMethods;
+  std::vector PartialSpecs;
 };
 } // namespace
 
Index: clang-rename/USRFinder.h

Re: [PATCH] D22945: [Driver] Replace more uses of default triples with effective triples (NFCI)

[Eric Christopher via cfe-commits]

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

OK.

-eric


https://reviews.llvm.org/D22945



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

Re: [PATCH] D22944: [Driver] Replace one-off uses of default triples with effective triples (NFCI)

[Eric Christopher via cfe-commits]

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

OK, please go back and document in ToolChain.h the difference between the 
Triple and the EffectiveTriple - at length, as a follow on patch.

-eric


https://reviews.llvm.org/D22944



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

Re: [PATCH] D22943: [Driver] Add FIXME's where we can't use effective triples (NFC)

[Eric Christopher via cfe-commits]

echristo added a comment.

Seems reasonable to fix the tests?


https://reviews.llvm.org/D22943



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

Re: [PATCH] D22729: MPIBufferDerefCheck for Clang-Tidy

[Alexander Droste via cfe-commits]

Alexander_Droste updated this revision to Diff 66761.
Alexander_Droste added a comment.

- add needed extra line for docs


https://reviews.llvm.org/D22729

Files:
  clang-tidy/mpi/BufferDerefCheck.cpp
  clang-tidy/mpi/BufferDerefCheck.h
  clang-tidy/mpi/CMakeLists.txt
  clang-tidy/mpi/MPITidyModule.cpp
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/mpi-buffer-deref.rst
  test/clang-tidy/Inputs/mpi-type-mismatch/mpimock.h
  test/clang-tidy/mpi-buffer-deref.cpp

Index: test/clang-tidy/mpi-buffer-deref.cpp
===
--- /dev/null
+++ test/clang-tidy/mpi-buffer-deref.cpp
@@ -0,0 +1,50 @@
+// RUN: %check_clang_tidy %s mpi-buffer-deref %t -- -- -I %S/Inputs/mpi-type-mismatch
+
+#include "mpimock.h"
+
+void negativeTests() {
+  char *buf;
+  MPI_Send(, 1, MPI_CHAR, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer is insufficiently dereferenced: pointer->pointer [mpi-buffer-deref]
+
+  unsigned **buf2;
+  MPI_Send(buf2, 1, MPI_UNSIGNED, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer is insufficiently dereferenced: pointer->pointer
+
+  short buf3[1][1];
+  MPI_Send(buf3, 1, MPI_SHORT, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer is insufficiently dereferenced: array->array
+
+  long double _Complex *buf4[1];
+  MPI_Send(buf4, 1, MPI_C_LONG_DOUBLE_COMPLEX, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer is insufficiently dereferenced: pointer->array
+
+  std::complex *buf5[1][1];
+  MPI_Send(, 1, MPI_CXX_FLOAT_COMPLEX, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer is insufficiently dereferenced: pointer->array->array->pointer
+}
+
+void positiveTests() {
+  char buf;
+  MPI_Send(, 1, MPI_CHAR, 0, 0, MPI_COMM_WORLD);
+
+  unsigned *buf2;
+  MPI_Send(buf2, 1, MPI_UNSIGNED, 0, 0, MPI_COMM_WORLD);
+
+  short buf3[1][1];
+  MPI_Send(buf3[0], 1, MPI_SHORT, 0, 0, MPI_COMM_WORLD);
+
+  long double _Complex *buf4[1];
+  MPI_Send(*buf4, 1, MPI_C_LONG_DOUBLE_COMPLEX, 0, 0, MPI_COMM_WORLD);
+
+  long double _Complex buf5[1];
+  MPI_Send(buf5, 1, MPI_C_LONG_DOUBLE_COMPLEX, 0, 0, MPI_COMM_WORLD);
+
+  std::complex *buf6[1][1];
+  MPI_Send(*buf6[0], 1, MPI_CXX_FLOAT_COMPLEX, 0, 0, MPI_COMM_WORLD);
+
+  // Referencing an array with '&' is valid, as this also points to the
+  // beginning of the array.
+  long double _Complex buf7[1];
+  MPI_Send(, 1, MPI_C_LONG_DOUBLE_COMPLEX, 0, 0, MPI_COMM_WORLD);
+}
Index: test/clang-tidy/Inputs/mpi-type-mismatch/mpimock.h
===
--- test/clang-tidy/Inputs/mpi-type-mismatch/mpimock.h
+++ test/clang-tidy/Inputs/mpi-type-mismatch/mpimock.h
@@ -24,13 +24,15 @@
 #define MPI_DATATYPE_NULL 0
 #define MPI_CHAR 0
 #define MPI_BYTE 0
+#define MPI_SHORT 0
 #define MPI_INT 0
 #define MPI_LONG 0
 #define MPI_LONG_DOUBLE 0
 #define MPI_UNSIGNED 0
 #define MPI_INT8_T 0
 #define MPI_UINT8_T 0
 #define MPI_UINT16_T 0
+#define MPI_C_FLOAT_COMPLEX 0
 #define MPI_C_LONG_DOUBLE_COMPLEX 0
 #define MPI_FLOAT 0
 #define MPI_DOUBLE 0
Index: docs/clang-tidy/checks/mpi-buffer-deref.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/mpi-buffer-deref.rst
@@ -0,0 +1,25 @@
+.. title:: clang-tidy - mpi-buffer-deref
+
+mpi-buffer-deref
+
+
+This check verifies if a buffer passed to an MPI (Message Passing Interface)
+function is sufficiently dereferenced. Buffers should be passed as a single
+pointer or array. As MPI function signatures specify ``void *`` for their buffer
+types, insufficiently dereferenced buffers can be passed, like for example as
+double pointers or multidimensional arrays, without a compiler warning emitted.
+
+Examples:
+.. code:: c++
+
+  // A double pointer is passed to the MPI function.
+  char *buf;
+  MPI_Send(, 1, MPI_CHAR, 0, 0, MPI_COMM_WORLD);
+
+  // A multidimensional array is passed to the MPI function.
+  short buf[1][1];
+  MPI_Send(buf, 1, MPI_SHORT, 0, 0, MPI_COMM_WORLD);
+
+  // A pointer to an array is passed to the MPI function.
+  short *buf[1];
+  MPI_Send(buf, 1, MPI_SHORT, 0, 0, MPI_COMM_WORLD);
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -110,6 +110,7 @@
modernize-use-nullptr
modernize-use-override
modernize-use-using
+   mpi-buffer-deref
mpi-type-mismatch
performance-faster-string-find
performance-for-range-copy
Index: clang-tidy/mpi/MPITidyModule.cpp
===
--- clang-tidy/mpi/MPITidyModule.cpp
+++ clang-tidy/mpi/MPITidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "BufferDerefCheck.h"
 #include 

Re: [PATCH] D22729: MPIBufferDerefCheck for Clang-Tidy

[Alexander Droste via cfe-commits]

Alexander_Droste updated this revision to Diff 66760.
Alexander_Droste added a comment.

- clarify lambda argument with comment
- explicit cast to `int` for the number of indirections in for loop


https://reviews.llvm.org/D22729

Files:
  clang-tidy/mpi/BufferDerefCheck.cpp
  clang-tidy/mpi/BufferDerefCheck.h
  clang-tidy/mpi/CMakeLists.txt
  clang-tidy/mpi/MPITidyModule.cpp
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/mpi-buffer-deref.rst
  test/clang-tidy/Inputs/mpi-type-mismatch/mpimock.h
  test/clang-tidy/mpi-buffer-deref.cpp

Index: test/clang-tidy/mpi-buffer-deref.cpp
===
--- /dev/null
+++ test/clang-tidy/mpi-buffer-deref.cpp
@@ -0,0 +1,50 @@
+// RUN: %check_clang_tidy %s mpi-buffer-deref %t -- -- -I %S/Inputs/mpi-type-mismatch
+
+#include "mpimock.h"
+
+void negativeTests() {
+  char *buf;
+  MPI_Send(, 1, MPI_CHAR, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer is insufficiently dereferenced: pointer->pointer [mpi-buffer-deref]
+
+  unsigned **buf2;
+  MPI_Send(buf2, 1, MPI_UNSIGNED, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer is insufficiently dereferenced: pointer->pointer
+
+  short buf3[1][1];
+  MPI_Send(buf3, 1, MPI_SHORT, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer is insufficiently dereferenced: array->array
+
+  long double _Complex *buf4[1];
+  MPI_Send(buf4, 1, MPI_C_LONG_DOUBLE_COMPLEX, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer is insufficiently dereferenced: pointer->array
+
+  std::complex *buf5[1][1];
+  MPI_Send(, 1, MPI_CXX_FLOAT_COMPLEX, 0, 0, MPI_COMM_WORLD);
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: buffer is insufficiently dereferenced: pointer->array->array->pointer
+}
+
+void positiveTests() {
+  char buf;
+  MPI_Send(, 1, MPI_CHAR, 0, 0, MPI_COMM_WORLD);
+
+  unsigned *buf2;
+  MPI_Send(buf2, 1, MPI_UNSIGNED, 0, 0, MPI_COMM_WORLD);
+
+  short buf3[1][1];
+  MPI_Send(buf3[0], 1, MPI_SHORT, 0, 0, MPI_COMM_WORLD);
+
+  long double _Complex *buf4[1];
+  MPI_Send(*buf4, 1, MPI_C_LONG_DOUBLE_COMPLEX, 0, 0, MPI_COMM_WORLD);
+
+  long double _Complex buf5[1];
+  MPI_Send(buf5, 1, MPI_C_LONG_DOUBLE_COMPLEX, 0, 0, MPI_COMM_WORLD);
+
+  std::complex *buf6[1][1];
+  MPI_Send(*buf6[0], 1, MPI_CXX_FLOAT_COMPLEX, 0, 0, MPI_COMM_WORLD);
+
+  // Referencing an array with '&' is valid, as this also points to the
+  // beginning of the array.
+  long double _Complex buf7[1];
+  MPI_Send(, 1, MPI_C_LONG_DOUBLE_COMPLEX, 0, 0, MPI_COMM_WORLD);
+}
Index: test/clang-tidy/Inputs/mpi-type-mismatch/mpimock.h
===
--- test/clang-tidy/Inputs/mpi-type-mismatch/mpimock.h
+++ test/clang-tidy/Inputs/mpi-type-mismatch/mpimock.h
@@ -24,13 +24,15 @@
 #define MPI_DATATYPE_NULL 0
 #define MPI_CHAR 0
 #define MPI_BYTE 0
+#define MPI_SHORT 0
 #define MPI_INT 0
 #define MPI_LONG 0
 #define MPI_LONG_DOUBLE 0
 #define MPI_UNSIGNED 0
 #define MPI_INT8_T 0
 #define MPI_UINT8_T 0
 #define MPI_UINT16_T 0
+#define MPI_C_FLOAT_COMPLEX 0
 #define MPI_C_LONG_DOUBLE_COMPLEX 0
 #define MPI_FLOAT 0
 #define MPI_DOUBLE 0
Index: docs/clang-tidy/checks/mpi-buffer-deref.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/mpi-buffer-deref.rst
@@ -0,0 +1,24 @@
+.. title:: clang-tidy - mpi-buffer-deref
+
+mpi-buffer-deref
+
+
+This check verifies if a buffer passed to an MPI (Message Passing Interface)
+function is sufficiently dereferenced. Buffers should be passed as a single
+pointer or array. As MPI function signatures specify ``void *`` for their buffer
+types, insufficiently dereferenced buffers can be passed, like for example as
+double pointers or multidimensional arrays, without a compiler warning emitted.
+
+Examples:
+.. code:: c++
+  // A double pointer is passed to the MPI function.
+  char *buf;
+  MPI_Send(, 1, MPI_CHAR, 0, 0, MPI_COMM_WORLD);
+
+  // A multidimensional array is passed to the MPI function.
+  short buf[1][1];
+  MPI_Send(buf, 1, MPI_SHORT, 0, 0, MPI_COMM_WORLD);
+
+  // A pointer to an array is passed to the MPI function.
+  short *buf[1];
+  MPI_Send(buf, 1, MPI_SHORT, 0, 0, MPI_COMM_WORLD);
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -110,6 +110,7 @@
modernize-use-nullptr
modernize-use-override
modernize-use-using
+   mpi-buffer-deref
mpi-type-mismatch
performance-faster-string-find
performance-for-range-copy
Index: clang-tidy/mpi/MPITidyModule.cpp
===
--- clang-tidy/mpi/MPITidyModule.cpp
+++ clang-tidy/mpi/MPITidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include 

r277696 - After PR28761 use -Wall with -Werror in builtins tests to identify

[Eric Christopher via cfe-commits]

Author: echristo
Date: Thu Aug  4 01:02:50 2016
New Revision: 277696

URL: http://llvm.org/viewvc/llvm-project?rev=277696=rev
Log:
After PR28761 use -Wall with -Werror in builtins tests to identify
possible problems in headers.

Modified:
cfe/trunk/test/CodeGen/3dnow-builtins.c
cfe/trunk/test/CodeGen/avx-builtins.c
cfe/trunk/test/CodeGen/avx2-builtins.c
cfe/trunk/test/CodeGen/avx512bw-builtins.c
cfe/trunk/test/CodeGen/avx512cdintrin.c
cfe/trunk/test/CodeGen/avx512dq-builtins.c
cfe/trunk/test/CodeGen/avx512er-builtins.c
cfe/trunk/test/CodeGen/avx512f-builtins.c
cfe/trunk/test/CodeGen/avx512ifma-builtins.c
cfe/trunk/test/CodeGen/avx512ifmavl-builtins.c
cfe/trunk/test/CodeGen/avx512pf-builtins.c
cfe/trunk/test/CodeGen/avx512vbmi-builtins.c
cfe/trunk/test/CodeGen/avx512vbmivl-builtin.c
cfe/trunk/test/CodeGen/avx512vl-builtins.c
cfe/trunk/test/CodeGen/avx512vlbw-builtins.c
cfe/trunk/test/CodeGen/avx512vlcd-builtins.c
cfe/trunk/test/CodeGen/avx512vldq-builtins.c
cfe/trunk/test/CodeGen/bmi-builtins.c
cfe/trunk/test/CodeGen/builtin-clflushopt.c
cfe/trunk/test/CodeGen/f16c-builtins.c
cfe/trunk/test/CodeGen/fma4-builtins.c
cfe/trunk/test/CodeGen/mmx-builtins.c
cfe/trunk/test/CodeGen/pku.c
cfe/trunk/test/CodeGen/sse-builtins.c
cfe/trunk/test/CodeGen/sse2-builtins.c
cfe/trunk/test/CodeGen/sse3-builtins.c
cfe/trunk/test/CodeGen/sse41-builtins.c
cfe/trunk/test/CodeGen/sse42-builtins.c
cfe/trunk/test/CodeGen/sse4a-builtins.c
cfe/trunk/test/CodeGen/ssse3-builtins.c
cfe/trunk/test/CodeGen/x86_32-xsave.c
cfe/trunk/test/CodeGen/x86_64-xsave.c
cfe/trunk/test/CodeGen/xop-builtins.c

Modified: cfe/trunk/test/CodeGen/3dnow-builtins.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/3dnow-builtins.c?rev=277696=277695=277696=diff
==
--- cfe/trunk/test/CodeGen/3dnow-builtins.c (original)
+++ cfe/trunk/test/CodeGen/3dnow-builtins.c Thu Aug  4 01:02:50 2016
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 %s -triple=x86_64-unknown-unknown -target-feature +3dnowa 
-emit-llvm -o - -Werror | FileCheck %s -check-prefix=GCC -check-prefix=CHECK
-// RUN: %clang_cc1 %s -triple=x86_64-scei-ps4 -target-feature +3dnowa 
-emit-llvm -o - -Werror | FileCheck %s -check-prefix=PS4 -check-prefix=CHECK
+// RUN: %clang_cc1 %s -triple=x86_64-unknown-unknown -target-feature +3dnowa 
-emit-llvm -o - -Wall -Werror | FileCheck %s -check-prefix=GCC 
-check-prefix=CHECK
+// RUN: %clang_cc1 %s -triple=x86_64-scei-ps4 -target-feature +3dnowa 
-emit-llvm -o - -Wall -Werror | FileCheck %s -check-prefix=PS4 
-check-prefix=CHECK
 
 // Don't include mm_malloc.h, it's system specific.
 #define __MM_MALLOC_H

Modified: cfe/trunk/test/CodeGen/avx-builtins.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/avx-builtins.c?rev=277696=277695=277696=diff
==
--- cfe/trunk/test/CodeGen/avx-builtins.c (original)
+++ cfe/trunk/test/CodeGen/avx-builtins.c Thu Aug  4 01:02:50 2016
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin -target-feature +avx 
-emit-llvm -o - -Werror | FileCheck %s
-// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin -target-feature +avx 
-fno-signed-char -emit-llvm -o - -Werror | FileCheck %s
+// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin -target-feature +avx 
-emit-llvm -o - -Wall -Werror | FileCheck %s
+// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin -target-feature +avx 
-fno-signed-char -emit-llvm -o - -Wall -Werror | FileCheck %s
 
 // Don't include mm_malloc.h, it's system specific.
 #define __MM_MALLOC_H

Modified: cfe/trunk/test/CodeGen/avx2-builtins.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/avx2-builtins.c?rev=277696=277695=277696=diff
==
--- cfe/trunk/test/CodeGen/avx2-builtins.c (original)
+++ cfe/trunk/test/CodeGen/avx2-builtins.c Thu Aug  4 01:02:50 2016
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin -target-feature +avx2 
-emit-llvm -o - -Werror | FileCheck %s
-// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin -target-feature +avx2 
-fno-signed-char -emit-llvm -o - -Werror | FileCheck %s
+// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin -target-feature +avx2 
-emit-llvm -o - -Wall -Werror | FileCheck %s
+// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin -target-feature +avx2 
-fno-signed-char -emit-llvm -o - -Wall -Werror | FileCheck %s
 
 // Don't include mm_malloc.h, it's system specific.
 #define __MM_MALLOC_H

Modified: cfe/trunk/test/CodeGen/avx512bw-builtins.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/avx512bw-builtins.c?rev=277696=277695=277696=diff
==
--- cfe/trunk/test/CodeGen/avx512bw-builtins.c 

Re: [PATCH] D22729: MPIBufferDerefCheck for Clang-Tidy

[Alexander Droste via cfe-commits]

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

https://reviews.llvm.org/D22729



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