Re: [PATCH] D21104: [CodeGen][ObjC] Block captures should inherit the type of the captured field in the enclosing lambda or block

[John McCall via cfe-commits]

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

LGTM.


https://reviews.llvm.org/D21104



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

Re: [PATCH] D24461: CodeGen: Cast llvm.flt.rounds result to match __builtin_flt_rounds

[John McCall via cfe-commits]

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

LGTM.


https://reviews.llvm.org/D24461



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

Re: [PATCH] D24383: Add addOrMerge interface to tooling::Replacements.

[Eric Liu via cfe-commits]

ioeric abandoned this revision.
ioeric added a comment.

Abandon in favor of https://reviews.llvm.org/D24515


https://reviews.llvm.org/D24383



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

Re: [PATCH] D24526: [Release notes] Mention readability-container-size-empty improvements

[Alexander Kornienko via cfe-commits]

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

LG with a nit.



Comment at: docs/ReleaseNotes.rst:94
@@ +93,3 @@
+  
`_
 check
+  supports arbitrary containers with ``empty()`` and ``size()`` methods.
+

The wording is a bit too broad. We should somehow hint that the methods are not 
just arbitrary. Maybe "with suitable ``size()`` and ``empty()`` methods"?


Repository:
  rL LLVM

https://reviews.llvm.org/D24526



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

Re: [PATCH] D22452: [libcxx] Fix last_write_time tests for filesystems that don't support negative and very large times.

[Jonas Hahnfeld via cfe-commits]

Hahnfeld added a comment.

In addition to this patch, my local environment would require the following 
changes:

  diff --git 
a/test/std/experimental/filesystem/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp
 
b/test/std/experimental/filesystem/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp
  index 01b3a67..a01b386 100644
  --- 
a/test/std/experimental/filesystem/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp
  +++ 
b/test/std/experimental/filesystem/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp
  @@ -86,13 +86,7 @@ bool TestSupportsNegativeTimes() {
   fs::last_write_time(file, tp, ec);
   new_write_time = LastWriteTime(file);
   }
  -if (ec) {
  -assert(old_write_time == new_write_time);
  -return false;
  -} else {
  -assert(new_write_time <= -5);
  -return true;
  -}
  +return !ec && new_write_time <= -5;
   }
   
   bool TestSupportsMaxTime() {
  @@ -110,13 +104,7 @@ bool TestSupportsMaxTime() {
   fs::last_write_time(file, tp, ec);
   new_write_time = LastWriteTime(file);
   }
  -if (ec) {
  -assert(new_write_time == old_write_time);
  -return false;
  -} else {
  -assert(new_write_time > old_write_time);
  -return true;
  -}
  +return !ec && new_write_time > max_sec - 1;
   }
   
   static const bool SupportsNegativeTimes = TestSupportsNegativeTimes();
  @@ -133,8 +121,9 @@ inline bool 
TimeIsRepresentableByFilesystem(file_time_type tp) {
   using namespace std::chrono;
   using Lim = std::numeric_limits;
   auto sec = duration_cast(tp.time_since_epoch()).count();
  +auto microsec = 
duration_cast(tp.time_since_epoch()).count();
   if (sec < Lim::min() || sec > Lim::max())   return false;
  -else if (sec < 0 && !SupportsNegativeTimes) return false;
  +else if (microsec < 0 && !SupportsNegativeTimes) return false;
   else if (tp == file_time_type::max() && !SupportsMaxTime) return false;
   return true;
   }
  @@ -273,15 +262,17 @@ TEST_CASE(set_last_write_time_dynamic_env_test)
   
   file_time_type  got_time = last_write_time(TC.p);
   
  -TEST_CHECK(got_time != old_time);
  -if (TC.new_time < epoch_time) {
  -TEST_CHECK(got_time <= TC.new_time);
  -TEST_CHECK(got_time > TC.new_time - Sec(1));
  -} else {
  -TEST_CHECK(got_time <= TC.new_time + Sec(1));
  -TEST_CHECK(got_time >= TC.new_time - Sec(1));
  +if (TimeIsRepresentableByFilesystem(TC.new_time)) {
  +TEST_CHECK(got_time != old_time);
  +if (TC.new_time < epoch_time) {
  +TEST_CHECK(got_time <= TC.new_time);
  +TEST_CHECK(got_time > TC.new_time - Sec(1));
  +} else {
  +TEST_CHECK(got_time <= TC.new_time + Sec(1));
  +TEST_CHECK(got_time >= TC.new_time - Sec(1));
  +}
  +TEST_CHECK(LastAccessTime(TC.p) == old_times.first);
   }
  -TEST_CHECK(LastAccessTime(TC.p) == old_times.first);
   }
   }
   
  @@ -334,11 +325,7 @@ TEST_CASE(test_write_min_time)
   last_write_time(p, new_time, ec);
   file_time_type tt = last_write_time(p);
   
  -if (!TimeIsRepresentableByFilesystem(new_time)) {
  -TEST_CHECK(ec);
  -TEST_CHECK(ec != GetTestEC());
  -TEST_CHECK(tt == last_time);
  -} else {
  +if (TimeIsRepresentableByFilesystem(new_time)) {
   TEST_CHECK(!ec);
   TEST_CHECK(tt >= new_time);
   TEST_CHECK(tt < new_time + Sec(1));
  @@ -353,11 +340,7 @@ TEST_CASE(test_write_min_time)
   last_write_time(p, new_time, ec);
   tt = last_write_time(p);
   
  -if (!TimeIsRepresentableByFilesystem(new_time)) {
  -TEST_CHECK(ec);
  -TEST_CHECK(ec != GetTestEC());
  -TEST_CHECK(tt == last_time);
  -} else {
  +if (TimeIsRepresentableByFilesystem(new_time)) {
   TEST_CHECK(!ec);
   TEST_CHECK(tt >= new_time);
   TEST_CHECK(tt < new_time + Sec(1));
  @@ -383,11 +366,7 @@ TEST_CASE(test_write_min_max_time)
   last_write_time(p, new_time, ec);
   file_time_type tt = last_write_time(p);
   
  -if (!TimeIsRepresentableByFilesystem(new_time)) {
  -TEST_CHECK(ec);
  -TEST_CHECK(ec != GetTestEC());
  -TEST_CHECK(tt == last_time);
  -} else {
  +if (TimeIsRepresentableByFilesystem(new_time)) {
   TEST_CHECK(!ec);
   TEST_CHECK(tt > new_time - Sec(1));
   TEST_CHECK(tt <= new_time);

However, this disables some of the tests if the filesystem behaves wrongly...


https://reviews.llvm.org/D22452



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

Re: [PATCH] D24461: CodeGen: Cast llvm.flt.rounds result to match __builtin_flt_rounds

[Edward Jones via cfe-commits]

edward-jones added a comment.

Awesome, is it possible for someone to commit this on my behalf?


https://reviews.llvm.org/D24461



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

Re: [PATCH] D22439: Add missing includes.

[Vassil Vassilev via cfe-commits]

v.g.vassilev added a comment.

The 'atomic' bug was fixed I assume as part of 
https://llvm.org/bugs/show_bug.cgi?id=28794

Other includes landed in r281450.


https://reviews.llvm.org/D22439



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

r281452 - Revert "[modules] When merging one definition into another, propagate the list of re-exporting modules from the discarded definition to the retained definition."

[Eric Liu via cfe-commits]

Author: ioeric
Date: Wed Sep 14 05:05:10 2016
New Revision: 281452

URL: http://llvm.org/viewvc/llvm-project?rev=281452&view=rev
Log:
Revert "[modules] When merging one definition into another, propagate the list 
of re-exporting modules from the discarded definition to the retained 
definition."

This reverts commit r281429.

Modified:
cfe/trunk/include/clang/AST/ASTContext.h
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/a.h
cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/b.h
cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/c.h
cfe/trunk/test/Modules/merge-template-pattern-visibility.cpp

Modified: cfe/trunk/include/clang/AST/ASTContext.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=281452&r1=281451&r2=281452&view=diff
==
--- cfe/trunk/include/clang/AST/ASTContext.h (original)
+++ cfe/trunk/include/clang/AST/ASTContext.h Wed Sep 14 05:05:10 2016
@@ -308,18 +308,10 @@ class ASTContext : public RefCountedBase
   /// merged into.
   llvm::DenseMap MergedDecls;
 
-  /// The modules into which a definition has been merged, or a map from a
-  /// merged definition to its canonical definition. This is really a union of
-  /// a NamedDecl* and a vector of Module*.
-  struct MergedModulesOrCanonicalDef {
-llvm::TinyPtrVector MergedModules;
-NamedDecl *CanonicalDef = nullptr;
-  };
-
   /// \brief A mapping from a defining declaration to a list of modules (other
   /// than the owning module of the declaration) that contain merged
   /// definitions of that entity.
-  llvm::DenseMap MergedDefModules;
+  llvm::DenseMap> MergedDefModules;
 
   /// \brief Initializers for a module, in order. Each Decl will be either
   /// something that has a semantic effect on startup (such as a variable with
@@ -902,7 +894,6 @@ public:
   /// and should be visible whenever \p M is visible.
   void mergeDefinitionIntoModule(NamedDecl *ND, Module *M,
  bool NotifyListeners = true);
-  void mergeDefinitionIntoModulesOf(NamedDecl *ND, NamedDecl *Other);
   /// \brief Clean up the merged definition list. Call this if you might have
   /// added duplicates into the list.
   void deduplicateMergedDefinitonsFor(NamedDecl *ND);
@@ -913,9 +904,7 @@ public:
 auto MergedIt = MergedDefModules.find(Def);
 if (MergedIt == MergedDefModules.end())
   return None;
-if (auto *CanonDef = MergedIt->second.CanonicalDef)
-  return getModulesWithMergedDefinition(CanonDef);
-return MergedIt->second.MergedModules;
+return MergedIt->second;
   }
 
   /// Add a declaration to the list of declarations that are initialized

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=281452&r1=281451&r2=281452&view=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Wed Sep 14 05:05:10 2016
@@ -890,59 +890,10 @@ void ASTContext::mergeDefinitionIntoModu
 if (auto *Listener = getASTMutationListener())
   Listener->RedefinedHiddenDefinition(ND, M);
 
-  if (getLangOpts().ModulesLocalVisibility) {
-auto *Merged = &MergedDefModules[ND];
-if (auto *CanonDef = Merged->CanonicalDef)
-  Merged = &MergedDefModules[CanonDef];
-Merged->MergedModules.push_back(M);
-  } else {
-auto MergedIt = MergedDefModules.find(ND);
-if (MergedIt != MergedDefModules.end() && MergedIt->second.CanonicalDef)
-  ND = MergedIt->second.CanonicalDef;
+  if (getLangOpts().ModulesLocalVisibility)
+MergedDefModules[ND].push_back(M);
+  else
 ND->setHidden(false);
-  }
-}
-
-void ASTContext::mergeDefinitionIntoModulesOf(NamedDecl *Def,
-  NamedDecl *Other) {
-  // We need to know the owning module of the merge source.
-  assert(Other->isFromASTFile() && "merge of non-imported decl not supported");
-  assert(Def != Other && "merging definition into itself");
-
-  if (!getLangOpts().ModulesLocalVisibility && !Other->isHidden()) {
-Def->setHidden(false);
-return;
-  }
-  assert(Other->getImportedOwningModule() &&
- "hidden, imported declaration has no owning module");
-
-  // Mark Def as the canonical definition of merged definition Other.
-  {
-auto &OtherMerged = MergedDefModules[Other];
-assert((!OtherMerged.CanonicalDef || OtherMerged.CanonicalDef == Def) &&
-   "mismatched canonical definitions for declaration");
-OtherMerged.CanonicalDef = Def;
-  }
-
-  auto &Merged = MergedDefModules[Def];
-  // Grab this again, we potentially just invalidated our reference.
-  auto &OtherMerged = MergedDefModules[Other];
-
-  if (Module *M = Other->getImportedOwningModule())
-M

Re: [PATCH] D24513: [AMDGPU] Expose flat work group size, register and wave control attributes

[Konstantin Zhuravlyov via cfe-commits]

kzhuravl updated the summary for this revision.
kzhuravl updated this revision to Diff 71311.
kzhuravl added a comment.

Update docs in AttrDocs.td


https://reviews.llvm.org/D24513

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/CodeGen/TargetInfo.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGenOpenCL/amdgpu-attrs.cl
  test/CodeGenOpenCL/amdgpu-num-gpr-attr.cl
  test/SemaCUDA/amdgpu-attrs.cu
  test/SemaCUDA/amdgpu-num-gpr-attr.cu
  test/SemaOpenCL/amdgpu-attrs.cl
  test/SemaOpenCL/amdgpu-num-register-attrs.cl

Index: test/SemaOpenCL/amdgpu-num-register-attrs.cl
===
--- test/SemaOpenCL/amdgpu-num-register-attrs.cl
+++ test/SemaOpenCL/amdgpu-num-register-attrs.cl
@@ -1,40 +0,0 @@
-// RUN: %clang_cc1 -triple r600-- -verify -fsyntax-only %s
-
-typedef __attribute__((amdgpu_num_vgpr(128))) struct FooStruct { // expected-error {{'amdgpu_num_vgpr' attribute only applies to kernel functions}}
-  int x;
-  float y;
-} FooStruct;
-
-
-__attribute__((amdgpu_num_vgpr("ABC"))) kernel void foo2() {} // expected-error {{'amdgpu_num_vgpr' attribute requires an integer constant}}
-__attribute__((amdgpu_num_sgpr("ABC"))) kernel void foo3() {} // expected-error {{'amdgpu_num_sgpr' attribute requires an integer constant}}
-
-
-__attribute__((amdgpu_num_vgpr(40))) void foo4() {} // expected-error {{'amdgpu_num_vgpr' attribute only applies to kernel functions}}
-__attribute__((amdgpu_num_sgpr(64))) void foo5() {} // expected-error {{'amdgpu_num_sgpr' attribute only applies to kernel functions}}
-
-__attribute__((amdgpu_num_vgpr(40))) kernel void foo7() {}
-__attribute__((amdgpu_num_sgpr(64))) kernel void foo8() {}
-__attribute__((amdgpu_num_vgpr(40), amdgpu_num_sgpr(64))) kernel void foo9() {}
-
-// Check 0 VGPR is accepted.
-__attribute__((amdgpu_num_vgpr(0))) kernel void foo10() {}
-
-// Check 0 SGPR is accepted.
-__attribute__((amdgpu_num_sgpr(0))) kernel void foo11() {}
-
-// Check both 0 SGPR and VGPR is accepted.
-__attribute__((amdgpu_num_vgpr(0), amdgpu_num_sgpr(0))) kernel void foo12() {}
-
-// Too large VGPR value.
-__attribute__((amdgpu_num_vgpr(4294967296))) kernel void foo13() {} // expected-error {{integer constant expression evaluates to value 4294967296 that cannot be represented in a 32-bit unsigned integer type}}
-
-__attribute__((amdgpu_num_sgpr(4294967296))) kernel void foo14() {} // expected-error {{integer constant expression evaluates to value 4294967296 that cannot be represented in a 32-bit unsigned integer type}}
-
-__attribute__((amdgpu_num_sgpr(4294967296), amdgpu_num_vgpr(4294967296))) kernel void foo15() {} // expected-error 2 {{integer constant expression evaluates to value 4294967296 that cannot be represented in a 32-bit unsigned integer type}}
-
-
-// Make sure it is accepted with kernel keyword before the attribute.
-kernel __attribute__((amdgpu_num_vgpr(40))) void foo16() {}
-
-kernel __attribute__((amdgpu_num_sgpr(40))) void foo17() {}
Index: test/SemaOpenCL/amdgpu-attrs.cl
===
--- test/SemaOpenCL/amdgpu-attrs.cl
+++ test/SemaOpenCL/amdgpu-attrs.cl
@@ -0,0 +1,64 @@
+// RUN: %clang_cc1 -triple amdgcn-- -verify -fsyntax-only %s
+
+typedef __attribute__((amdgpu_flat_work_group_size(32, 64))) struct struct_flat_work_group_size_32_64 { // expected-error {{'amdgpu_flat_work_group_size' attribute only applies to kernel functions}}
+  int x;
+  float y;
+} struct_flat_work_group_size_32_64;
+typedef __attribute__((amdgpu_waves_per_eu(2))) struct struct_waves_per_eu_2 { // expected-error {{'amdgpu_waves_per_eu' attribute only applies to kernel functions}}
+  int x;
+  float y;
+} struct_waves_per_eu_2;
+typedef __attribute__((amdgpu_waves_per_eu(2, 4))) struct struct_waves_per_eu_2_4 { // expected-error {{'amdgpu_waves_per_eu' attribute only applies to kernel functions}}
+  int x;
+  float y;
+} struct_waves_per_eu_2_4;
+typedef __attribute__((amdgpu_num_sgpr(32))) struct struct_num_sgpr_32 { // expected-error {{'amdgpu_num_sgpr' attribute only applies to kernel functions}}
+  int x;
+  float y;
+} struct_num_sgpr_32;
+typedef __attribute__((amdgpu_num_vgpr(64))) struct struct_num_vgpr_64 { // expected-error {{'amdgpu_num_vgpr' attribute only applies to kernel functions}}
+  int x;
+  float y;
+} struct_num_vgpr_64;
+
+__attribute__((amdgpu_flat_work_group_size(32, 64))) void func_flat_work_group_size_32_64() {} // expected-error {{'amdgpu_flat_work_group_size' attribute only applies to kernel functions}}
+__attribute__((amdgpu_waves_per_eu(2))) void func_waves_per_eu_2() {} // expected-error {{'amdgpu_waves_per_eu' attribute only applies to kernel functions}}
+__attribute__((amdgpu_waves_per_eu(2, 4))) void func_waves_per_eu_2_4() {} // expected-error {{'amdgpu_waves_per_eu' attribute only applies to kernel functions}}
+__attribute__((amdgpu_num_sgpr(32))) void func_num_sgpr_32() {} // expected-error 

Re: [PATCH] D23353: [clang-tidy] Add check 'misc-use-after-move'

[Martin Böhme via cfe-commits]

mboehme updated this revision to Diff 71312.
mboehme marked 6 inline comments as done.
mboehme added a comment.
Herald added subscribers: mgorny, beanz.

Responses to reviewer comments.


https://reviews.llvm.org/D23353

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/UseAfterMoveCheck.cpp
  clang-tidy/misc/UseAfterMoveCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-use-after-move.rst
  test/clang-tidy/misc-use-after-move.cpp

Index: test/clang-tidy/misc-use-after-move.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-use-after-move.cpp
@@ -0,0 +1,1039 @@
+// RUN: %check_clang_tidy %s misc-use-after-move %t
+
+typedef decltype(nullptr) nullptr_t;
+
+namespace std {
+typedef unsigned size_t;
+
+template 
+struct unique_ptr {
+  unique_ptr();
+  T *get() const;
+};
+
+template 
+struct shared_ptr {
+  shared_ptr();
+  T *get() const;
+};
+
+#define DECLARE_STANDARD_CONTAINER(name) \
+  template   \
+  struct name {  \
+name();  \
+void clear();\
+bool empty();\
+  }
+
+#define DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(name) \
+  template   \
+  struct name {  \
+name();  \
+void clear();\
+bool empty();\
+void assign(size_t, const T &);  \
+  }
+
+DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(basic_string);
+DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(vector);
+DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(deque);
+DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(forward_list);
+DECLARE_STANDARD_CONTAINER_WITH_ASSIGN(list);
+DECLARE_STANDARD_CONTAINER(set);
+DECLARE_STANDARD_CONTAINER(map);
+DECLARE_STANDARD_CONTAINER(multiset);
+DECLARE_STANDARD_CONTAINER(multimap);
+DECLARE_STANDARD_CONTAINER(unordered_set);
+DECLARE_STANDARD_CONTAINER(unordered_map);
+DECLARE_STANDARD_CONTAINER(unordered_multiset);
+DECLARE_STANDARD_CONTAINER(unordered_multimap);
+
+typedef basic_string string;
+
+template 
+struct remove_reference;
+
+template 
+struct remove_reference {
+  typedef _Tp type;
+};
+
+template 
+struct remove_reference<_Tp &> {
+  typedef _Tp type;
+};
+
+template 
+struct remove_reference<_Tp &&> {
+  typedef _Tp type;
+};
+
+template 
+constexpr typename std::remove_reference<_Tp>::type &&move(_Tp &&__t) noexcept {
+  return static_cast::type &&>(__t);
+}
+
+} // namespace std
+
+class A {
+public:
+  A();
+  A(const A &);
+  A(A &&);
+
+  A &operator=(const A &);
+  A &operator=(A &&);
+
+  void foo() const;
+  int getInt() const;
+
+  operator bool() const;
+
+  int i;
+};
+
+
+// General tests.
+
+// Simple case.
+void simple() {
+  A a;
+  a.foo();
+  A other_a = std::move(a);
+  a.foo();
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: 'a' used after it was moved
+  // CHECK-MESSAGES: [[@LINE-3]]:15: note: move occurred here
+}
+
+// A warning should only be emitted for one use-after-move.
+void onlyFlagOneUseAfterMove() {
+  A a;
+  a.foo();
+  A other_a = std::move(a);
+  a.foo();
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: 'a' used after it was moved
+  // CHECK-MESSAGES: [[@LINE-3]]:15: note: move occurred here
+  a.foo();
+}
+
+void moveAfterMove() {
+  // Move-after-move also counts as a use.
+  {
+A a;
+std::move(a);
+std::move(a);
+// CHECK-MESSAGES: [[@LINE-1]]:15: warning: 'a' used after it was moved
+// CHECK-MESSAGES: [[@LINE-3]]:5: note: move occurred here
+  }
+  // This is also true if the move itself turns into the use on the second loop
+  // iteration.
+  {
+A a;
+for (int i = 0; i < 10; ++i) {
+  std::move(a);
+  // CHECK-MESSAGES: [[@LINE-1]]:17: warning: 'a' used after it was moved
+  // CHECK-MESSAGES: [[@LINE-2]]:7: note: move occurred here
+}
+  }
+}
+
+// Checks also works on function parameters that have a use-after move.
+void parameters(A a) {
+  std::move(a);
+  a.foo();
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: 'a' used after it was moved
+  // CHECK-MESSAGES: [[@LINE-3]]:3: note: move occurred here
+}
+
+void uniquePtrAndSharedPtr() {
+  // Use-after-moves on std::unique_ptr<> or std::shared_ptr<> aren't flagged.
+  {
+std::unique_ptr ptr;
+std::move(ptr);
+ptr.get();
+  }
+  {
+std::shared_ptr ptr;
+std::move(ptr);
+ptr.get();
+  }
+  // This is also true if the std::unique_ptr<> or std::shared_ptr<> is wrapped
+  // in a typedef.
+  {
+typedef std::unique_ptr PtrToA;
+PtrToA ptr;
+std::move(ptr);
+ptr.get();
+  }
+  {
+typedef std::shared_ptr PtrToA;
+PtrToA ptr;
+std::move(ptr);
+ptr.get();
+  }
+  // And it's also true if the template 

Re: [PATCH] D23353: [clang-tidy] Add check 'misc-use-after-move'

[Martin Böhme via cfe-commits]

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

https://reviews.llvm.org/D23353



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

Re: [PATCH] D23353: [clang-tidy] Add check 'misc-use-after-move'

[Martin Böhme via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL281453: [clang-tidy] Add check 'misc-use-after-move' 
(authored by mboehme).

Changed prior to commit:
  https://reviews.llvm.org/D23353?vs=71312&id=71313#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D23353

Files:
  clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.cpp
  clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.h
  clang-tools-extra/trunk/docs/ReleaseNotes.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/misc-use-after-move.rst
  clang-tools-extra/trunk/test/clang-tidy/misc-use-after-move.cpp

Index: clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.h
+++ clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.h
@@ -0,0 +1,36 @@
+//===--- UseAfterMoveCheck.h - clang-tidy -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_USEAFTERMOVECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_USEAFTERMOVECHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// The check warns if an object is used after it has been moved, without an
+/// intervening reinitialization.
+///
+/// For details, see the user-facing documentation:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-use-after-move.html
+class UseAfterMoveCheck : public ClangTidyCheck {
+public:
+  UseAfterMoveCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_USEAFTERMOVECHECK_H
Index: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
===
--- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
+++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
@@ -43,6 +43,7 @@
   UnusedParametersCheck.cpp
   UnusedRAIICheck.cpp
   UnusedUsingDeclsCheck.cpp
+  UseAfterMoveCheck.cpp
   VirtualNearMissCheck.cpp
 
   LINK_LIBS
Index: clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.cpp
@@ -0,0 +1,643 @@
+//===--- UseAfterMoveCheck.cpp - clang-tidy ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "UseAfterMoveCheck.h"
+
+#include "clang/Analysis/CFG.h"
+#include "clang/Lex/Lexer.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/ADT/SmallVector.h"
+
+#include 
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+namespace {
+
+/// Provides information about the evaluation order of (sub-)expressions within
+/// a `CFGBlock`.
+///
+/// While a `CFGBlock` does contain individual `CFGElement`s for some
+/// sub-expressions, the order in which those `CFGElement`s appear reflects
+/// only one possible order in which the sub-expressions may be evaluated.
+/// However, we want to warn if any of the potential evaluation orders can lead
+/// to a use-after-move, not just the one contained in the `CFGBlock`.
+///
+/// This class implements only a simplified version of the C++ sequencing rules
+/// that is, however, sufficient for the purposes of this check. The main
+/// limitation is that we do not distinguish between value computation and side
+/// effect -- see the "Implementation" section for more details.
+///
+/// Note: `SequenceChecker` from SemaChecking.cpp does a similar job (and much
+/// more thoroughly), but using it would require
+/// - Pulling `SequenceChecker` out into a header file (i.e. making it part of
+///   the API),
+/// - Removing the dependency of `SequenceChecker` on `Sema`, and
+/// - (Probably) modifying `SequenceChecker` to make it suitable to be used in
+///   this context.
+/// For the moment, it seems preferable to re-implement our own v

[clang-tools-extra] r281453 - [clang-tidy] Add check 'misc-use-after-move'

[Martin Bohme via cfe-commits]

Author: mboehme
Date: Wed Sep 14 05:29:32 2016
New Revision: 281453

URL: http://llvm.org/viewvc/llvm-project?rev=281453&view=rev
Log:
[clang-tidy] Add check 'misc-use-after-move'

Summary:
The check warns if an object is used after it has been moved, without an
intervening reinitialization.

See user-facing documentation for details.

Reviewers: sbenza, Prazek, alexfh

Subscribers: beanz, mgorny, shadeware, omtcyfz, Eugene.Zelenko, Prazek, fowles, 
ioeric, cfe-commits

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

Added:
clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-use-after-move.rst
clang-tools-extra/trunk/test/clang-tidy/misc-use-after-move.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt?rev=281453&r1=281452&r2=281453&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt Wed Sep 14 05:29:32 
2016
@@ -43,6 +43,7 @@ add_clang_library(clangTidyMiscModule
   UnusedParametersCheck.cpp
   UnusedRAIICheck.cpp
   UnusedUsingDeclsCheck.cpp
+  UseAfterMoveCheck.cpp
   VirtualNearMissCheck.cpp
 
   LINK_LIBS

Modified: clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp?rev=281453&r1=281452&r2=281453&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp Wed Sep 14 
05:29:32 2016
@@ -51,6 +51,7 @@
 #include "UnusedParametersCheck.h"
 #include "UnusedRAIICheck.h"
 #include "UnusedUsingDeclsCheck.h"
+#include "UseAfterMoveCheck.h"
 #include "VirtualNearMissCheck.h"
 
 namespace clang {
@@ -139,6 +140,7 @@ public:
 CheckFactories.registerCheck("misc-unused-raii");
 CheckFactories.registerCheck(
 "misc-unused-using-decls");
+CheckFactories.registerCheck("misc-use-after-move");
 CheckFactories.registerCheck(
 "misc-virtual-near-miss");
   }

Added: clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.cpp?rev=281453&view=auto
==
--- clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/misc/UseAfterMoveCheck.cpp Wed Sep 14 
05:29:32 2016
@@ -0,0 +1,643 @@
+//===--- UseAfterMoveCheck.cpp - clang-tidy 
---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "UseAfterMoveCheck.h"
+
+#include "clang/Analysis/CFG.h"
+#include "clang/Lex/Lexer.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/ADT/SmallVector.h"
+
+#include 
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+namespace {
+
+/// Provides information about the evaluation order of (sub-)expressions within
+/// a `CFGBlock`.
+///
+/// While a `CFGBlock` does contain individual `CFGElement`s for some
+/// sub-expressions, the order in which those `CFGElement`s appear reflects
+/// only one possible order in which the sub-expressions may be evaluated.
+/// However, we want to warn if any of the potential evaluation orders can lead
+/// to a use-after-move, not just the one contained in the `CFGBlock`.
+///
+/// This class implements only a simplified version of the C++ sequencing rules
+/// that is, however, sufficient for the purposes of this check. The main
+/// limitation is that we do not distinguish between value computation and side
+/// effect -- see the "Implementation" section for more details.
+///
+/// Note: `SequenceChecker` from SemaChecking.cpp does a similar job (and much
+/// more thoroughly), but using it would require
+/// - Pulling `SequenceChecker` out into a header file (i.e. making it part of
+///   the API),
+/// - Removing the dependency of `SequenceChecker` on `Sema`, and
+/// - (Probably) modifying `SequenceChecker` to make it suitable to be used in
+///   this context.
+/// For the moment, it se

Re: [clang-tools-extra] r281453 - [clang-tidy] Add check 'misc-use-after-move'

[Chandler Carruth via cfe-commits]

On Wed, Sep 14, 2016 at 3:38 AM Martin Bohme via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: mboehme
> Date: Wed Sep 14 05:29:32 2016
> New Revision: 281453
>
> URL: http://llvm.org/viewvc/llvm-project?rev=281453&view=rev
> Log:
> [clang-tidy] Add check 'misc-use-after-move'
>
> Summary:
> The check warns if an object is used after it has been moved, without an
> intervening reinitialization.
>

While I'm excited to see this go in anywhere, I have to say I'm a bit sad
it isn't going in as a warning and instead inside clang-tidy. This has been
a much requested warning from Clang for many years.

Is there a concise description of why this design was chosen? Are there
specific problems that make it infeasible to be a warnings? Is there maybe
a plan to move it to a warning after some set of issues are addressed?


>
> See user-facing documentation for details.
>

In general, I suspect at least some more context should be provided in
change descriptions. =]
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24508: PR28752: Do not instantiate var decls which are not visible.

[Vassil Vassilev via cfe-commits]

v.g.vassilev removed rL LLVM as the repository for this revision.
v.g.vassilev updated this revision to Diff 71317.
v.g.vassilev marked 4 inline comments as done.
v.g.vassilev added a comment.

Address comments.

I still find the regression test a bit clumsy. I will try to add it to 
`test/Modules/submodules-merge-defs.cpp`, would this be the right place for it?


https://reviews.llvm.org/D24508

Files:
  lib/Sema/SemaTemplate.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Sema/SemaType.cpp
  test/Modules/Inputs/PR28752/Subdir1/b.h
  test/Modules/Inputs/PR28752/Subdir1/c.h
  test/Modules/Inputs/PR28752/Subdir1/module.modulemap
  test/Modules/Inputs/PR28752/a.h
  test/Modules/Inputs/PR28752/module.modulemap
  test/Modules/Inputs/PR28752/vector
  test/Modules/pr28752.cpp

Index: test/Modules/pr28752.cpp
===
--- /dev/null
+++ test/Modules/pr28752.cpp
@@ -0,0 +1,19 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -std=c++11 -nostdsysteminc -I%S/Inputs/PR28752 -verify %s
+// RUN: %clang_cc1 -std=c++11 -nostdsysteminc -fmodules -fmodule-map-file=%S/Inputs/PR28752/Subdir1/module.modulemap -fmodule-map-file=%S/Inputs/PR28752/module.modulemap -fmodules-cache-path=%t -I%S/Inputs/PR28752 -I%S/Inputs/PR28752/Subdir1 -verify %s
+
+#include "a.h"
+#include "Subdir1/c.h"
+#include 
+
+class TClingClassInfo {
+  std::vector fIterStack;
+};
+
+TClingClassInfo *a;
+class TClingBaseClassInfo {
+  TClingBaseClassInfo() { new TClingClassInfo(*a); }
+};
+
+// expected-no-diagnostics
+
Index: test/Modules/Inputs/PR28752/vector
===
--- /dev/null
+++ test/Modules/Inputs/PR28752/vector
@@ -0,0 +1,28 @@
+#ifndef VECTOR
+#define VECTOR
+template  struct B;
+template  struct B { typedef _Tp type; };
+namespace std {
+template  struct D {
+
+  template  struct F {
+static const bool value = 0;
+  };
+
+  template 
+  typename B::value, _Alloc2>::type _S_select(_Alloc2);
+  template 
+  static
+  typename B::value, _Alloc2>::type _S_select(_Alloc2);
+};
+template 
+template 
+const bool D<_Alloc>::F<_Alloc2>::value;
+
+template  class vector {
+public:
+  vector(int);
+  vector(vector &) : vector(D::_S_select((bool)0)) {}
+};
+}
+#endif // VECTOR
\ No newline at end of file
Index: test/Modules/Inputs/PR28752/module.modulemap
===
--- /dev/null
+++ test/Modules/Inputs/PR28752/module.modulemap
@@ -0,0 +1 @@
+module a { header "a.h" export * }
Index: test/Modules/Inputs/PR28752/a.h
===
--- /dev/null
+++ test/Modules/Inputs/PR28752/a.h
@@ -0,0 +1 @@
+#include 
Index: test/Modules/Inputs/PR28752/Subdir1/module.modulemap
===
--- /dev/null
+++ test/Modules/Inputs/PR28752/Subdir1/module.modulemap
@@ -0,0 +1,5 @@
+module b {
+  module "b.h" { header "b.h" export * }
+  module "c.h" { header "c.h" export * }
+  export *
+}
Index: test/Modules/Inputs/PR28752/Subdir1/b.h
===
--- /dev/null
+++ test/Modules/Inputs/PR28752/Subdir1/b.h
@@ -0,0 +1 @@
+#include 
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -6894,6 +6894,12 @@
 if (auto *Pattern = FD->getTemplateInstantiationPattern())
   FD = Pattern;
 D = FD->getDefinition();
+  } else if (auto *VD = dyn_cast(D)) {
+// FIXME: Handle the case where D is a VarTemplateSpecializationDecl, i.e. D
+// is not a static data member.
+if (auto *Pattern = VD->getInstantiatedFromStaticDataMember())
+  VD = Pattern;
+D = VD->getDefinition();
   }
   assert(D && "missing definition for pattern of instantiated definition");
 
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4067,6 +4067,10 @@
   PrettyDeclStackTraceEntry CrashInfo(*this, Var, SourceLocation(),
   "instantiating variable initializer");
 
+  // The instantiation is visible here, even if it was first declared in an
+  // unimported module.
+  Var->setHidden(false);
+
   // If we're performing recursive template instantiation, create our own
   // queue of pending implicit instantiations that we will instantiate
   // later, while we're still within our own instantiation context.
@@ -4115,47 +4119,38 @@
 Def = PatternDecl->getDefinition();
   }
 
-  // FIXME: Check that the definition is visible before trying to instantiate
-  // it. This requires us to track the instantiation stack in order to know
-  // which definitions should be visible.
+  TemplateSpecializationKind TSK = Var->getTemplateSpecializationKind(

[PATCH] D24550: [clang-tidy] Make test for misc-use-after-move pass under Windows

[Martin Böhme via cfe-commits]

mboehme created this revision.
mboehme added a reviewer: alexfh.
mboehme added a subscriber: cfe-commits.

Adds -fno-delayed-template-parsing

https://reviews.llvm.org/D24550

Files:
  test/clang-tidy/misc-use-after-move.cpp

Index: test/clang-tidy/misc-use-after-move.cpp
===
--- test/clang-tidy/misc-use-after-move.cpp
+++ test/clang-tidy/misc-use-after-move.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s misc-use-after-move %t
+// RUN: %check_clang_tidy %s misc-use-after-move %t -- -- -std=c++11 
-fno-delayed-template-parsing
 
 typedef decltype(nullptr) nullptr_t;
 


Index: test/clang-tidy/misc-use-after-move.cpp
===
--- test/clang-tidy/misc-use-after-move.cpp
+++ test/clang-tidy/misc-use-after-move.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s misc-use-after-move %t
+// RUN: %check_clang_tidy %s misc-use-after-move %t -- -- -std=c++11 -fno-delayed-template-parsing
 
 typedef decltype(nullptr) nullptr_t;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24550: [clang-tidy] Make test for misc-use-after-move pass under Windows

[Martin Böhme via cfe-commits]

mboehme added a comment.

Single-line trivial change, will submit without review. Please let me know if 
you have any objections.


https://reviews.llvm.org/D24550



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

[clang-tools-extra] r281455 - [clang-tidy] Make test for misc-use-after-move pass under Windows

[Martin Bohme via cfe-commits]

Author: mboehme
Date: Wed Sep 14 07:22:35 2016
New Revision: 281455

URL: http://llvm.org/viewvc/llvm-project?rev=281455&view=rev
Log:
[clang-tidy] Make test for misc-use-after-move pass under Windows

Summary: Adds -fno-delayed-template-parsing

Reviewers: alexfh

Subscribers: cfe-commits

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

Modified:
clang-tools-extra/trunk/test/clang-tidy/misc-use-after-move.cpp

Modified: clang-tools-extra/trunk/test/clang-tidy/misc-use-after-move.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-use-after-move.cpp?rev=281455&r1=281454&r2=281455&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/misc-use-after-move.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-use-after-move.cpp Wed Sep 14 
07:22:35 2016
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s misc-use-after-move %t
+// RUN: %check_clang_tidy %s misc-use-after-move %t -- -- -std=c++11 
-fno-delayed-template-parsing
 
 typedef decltype(nullptr) nullptr_t;
 


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

Re: [PATCH] D24550: [clang-tidy] Make test for misc-use-after-move pass under Windows

[Martin Böhme via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL281455: [clang-tidy] Make test for misc-use-after-move pass 
under Windows (authored by mboehme).

Changed prior to commit:
  https://reviews.llvm.org/D24550?vs=71318&id=71319#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24550

Files:
  clang-tools-extra/trunk/test/clang-tidy/misc-use-after-move.cpp

Index: clang-tools-extra/trunk/test/clang-tidy/misc-use-after-move.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/misc-use-after-move.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/misc-use-after-move.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s misc-use-after-move %t
+// RUN: %check_clang_tidy %s misc-use-after-move %t -- -- -std=c++11 
-fno-delayed-template-parsing
 
 typedef decltype(nullptr) nullptr_t;
 


Index: clang-tools-extra/trunk/test/clang-tidy/misc-use-after-move.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/misc-use-after-move.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/misc-use-after-move.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s misc-use-after-move %t
+// RUN: %check_clang_tidy %s misc-use-after-move %t -- -- -std=c++11 -fno-delayed-template-parsing
 
 typedef decltype(nullptr) nullptr_t;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24518: Correct assert text in DeclGroup::getSingleDecl()

[Alexander Kornienko via cfe-commits]

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

LG


https://reviews.llvm.org/D24518



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

Re: [PATCH] D24500: [clang-tidy] Bugfix for readability-redundant-control-flow check

[Alexander Kornienko via cfe-commits]

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

LG. If there's a related buganizer item, please mention it in the commit 
message.


https://reviews.llvm.org/D24500



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

Re: [PATCH] D24500: [clang-tidy] Bugfix for readability-redundant-control-flow check

[Malcolm Parsons via cfe-commits]

malcolm.parsons added a comment.

I didn't report a bug for this issue, and there isn't an existing one.


https://reviews.llvm.org/D24500



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

Re: [PATCH] D24339: [clang-tidy] Add check 'readability-redundant-member-init'

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

In https://reviews.llvm.org/D24339#537488, @malcolm.parsons wrote:

> How do I add FixIt hints?
>  They should be simple removals, but how do I decide whether to remove the 
> following comma, preceding comma or preceding colon?


Just remove the entry and leave the comma and the colon alone. Clang-tidy needs 
to learn to clean up the code after such replacements using clang-format's 
cleanupAroundReplacements. Filed https://llvm.org/bugs/show_bug.cgi?id=30379


https://reviews.llvm.org/D24339



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

Re: [PATCH] D24224: [clang-rename] Merge rename-{ at | all } and optimise USRFindingAction.

[Alexander Kornienko via cfe-commits]

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

LG with a nit.



Comment at: clang-rename/USRFindingAction.cpp:182
@@ -161,1 +181,3 @@
+return false;
   } else {
+unsigned CouldNotFindSymbolNamed = Engine.getCustomDiagID(

No `else` after `return`.


https://reviews.llvm.org/D24224



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

Re: [PATCH] D24515: Supports adding insertion around non-insertion replacements.

[Daniel Jasper via cfe-commits]

djasper added inline comments.


Comment at: include/clang/Tooling/Core/Replacement.h:172
@@ +171,3 @@
+  ///   - They do not overlap.
+  ///   - One replacement is insertion, and the other is a replacement with
+  /// length > 0, and the insertion is adjecent to but not contained in the

I think technically this is also not an overlap and so the first condition 
would hold. How about phrasing this the other way, i.e.:

Two replacements are considered order independent if they:
- they don't overlap (being directly adjacent is fine) and
- aren't both inserts at the same code location


https://reviews.llvm.org/D24515



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

Re: [PATCH] D24515: Supports adding insertion around non-insertion replacements.

[Eric Liu via cfe-commits]

ioeric updated this revision to Diff 71332.
ioeric marked an inline comment as done.
ioeric added a comment.

- Addressed comment.


https://reviews.llvm.org/D24515

Files:
  include/clang/Tooling/Core/Replacement.h
  lib/Tooling/Core/Replacement.cpp
  unittests/Tooling/RefactoringTest.cpp

Index: unittests/Tooling/RefactoringTest.cpp
===
--- unittests/Tooling/RefactoringTest.cpp
+++ unittests/Tooling/RefactoringTest.cpp
@@ -115,24 +115,26 @@
   llvm::consumeError(std::move(Err));
 }
 
-TEST_F(ReplacementTest, FailAddOverlappingInsertions) {
+TEST_F(ReplacementTest, AddAdjacentInsertionAndReplacement) {
   Replacements Replaces;
   // Test adding an insertion at the offset of an existing replacement.
   auto Err = Replaces.add(Replacement("x.cc", 10, 3, "replace"));
   EXPECT_TRUE(!Err);
   llvm::consumeError(std::move(Err));
   Err = Replaces.add(Replacement("x.cc", 10, 0, "insert"));
-  EXPECT_TRUE((bool)Err);
+  EXPECT_TRUE(!Err);
   llvm::consumeError(std::move(Err));
+  EXPECT_EQ(Replaces.size(), 2u);
 
   Replaces.clear();
   // Test overlap with an existing insertion.
   Err = Replaces.add(Replacement("x.cc", 10, 0, "insert"));
   EXPECT_TRUE(!Err);
   llvm::consumeError(std::move(Err));
   Err = Replaces.add(Replacement("x.cc", 10, 3, "replace"));
-  EXPECT_TRUE((bool)Err);
+  EXPECT_TRUE(!Err);
   llvm::consumeError(std::move(Err));
+  EXPECT_EQ(Replaces.size(), 2u);
 }
 
 TEST_F(ReplacementTest, FailAddRegression) {
@@ -157,14 +159,24 @@
   llvm::consumeError(std::move(Err));
 }
 
-TEST_F(ReplacementTest, FailAddInsertAtOffsetOfReplacement) {
+TEST_F(ReplacementTest, InsertAtOffsetOfReplacement) {
   Replacements Replaces;
   auto Err = Replaces.add(Replacement("x.cc", 10, 2, ""));
   EXPECT_TRUE(!Err);
   llvm::consumeError(std::move(Err));
   Err = Replaces.add(Replacement("x.cc", 10, 0, ""));
-  EXPECT_TRUE((bool)Err);
+  EXPECT_TRUE(!Err);
+  llvm::consumeError(std::move(Err));
+  EXPECT_EQ(Replaces.size(), 2u);
+
+  Replaces.clear();
+  Err = Replaces.add(Replacement("x.cc", 10, 0, ""));
+  EXPECT_TRUE(!Err);
+  llvm::consumeError(std::move(Err));
+  Err = Replaces.add(Replacement("x.cc", 10, 2, ""));
+  EXPECT_TRUE(!Err);
   llvm::consumeError(std::move(Err));
+  EXPECT_EQ(Replaces.size(), 2u);
 }
 
 TEST_F(ReplacementTest, FailAddInsertAtOtherInsert) {
@@ -175,6 +187,38 @@
   Err = Replaces.add(Replacement("x.cc", 10, 0, "b"));
   EXPECT_TRUE((bool)Err);
   llvm::consumeError(std::move(Err));
+
+  Replaces.clear();
+  Err = Replaces.add(Replacement("x.cc", 10, 0, ""));
+  EXPECT_TRUE(!Err);
+  llvm::consumeError(std::move(Err));
+  Err = Replaces.add(Replacement("x.cc", 10, 0, ""));
+  EXPECT_TRUE((bool)Err);
+  llvm::consumeError(std::move(Err));
+
+  Replaces.clear();
+  Err = Replaces.add(Replacement("x.cc", 10, 0, ""));
+  EXPECT_TRUE(!Err);
+  llvm::consumeError(std::move(Err));
+  Err = Replaces.add(Replacement("x.cc", 10, 3, ""));
+  EXPECT_TRUE(!Err);
+  llvm::consumeError(std::move(Err));
+  Err = Replaces.add(Replacement("x.cc", 10, 0, ""));
+  EXPECT_TRUE((bool)Err);
+  llvm::consumeError(std::move(Err));
+}
+
+TEST_F(ReplacementTest, InsertBetweenAdjacentReplacements) {
+  Replacements Replaces;
+  auto Err = Replaces.add(Replacement("x.cc", 10, 5, "a"));
+  EXPECT_TRUE(!Err);
+  llvm::consumeError(std::move(Err));
+  Err = Replaces.add(Replacement("x.cc", 8, 2, "a"));
+  EXPECT_TRUE(!Err);
+  llvm::consumeError(std::move(Err));
+  Err = Replaces.add(Replacement("x.cc", 10, 0, "b"));
+  EXPECT_TRUE(!Err);
+  llvm::consumeError(std::move(Err));
 }
 
 TEST_F(ReplacementTest, CanApplyReplacements) {
@@ -189,6 +233,29 @@
   EXPECT_EQ("line1\nreplaced\nother\nline4", Context.getRewrittenText(ID));
 }
 
+// Verifies that replacement/deletion is applied before insertion at the same
+// offset.
+TEST_F(ReplacementTest, InsertAndDelete) {
+  FileID ID = Context.createInMemoryFile("input.cpp",
+ "line1\nline2\nline3\nline4");
+  Replacements Replaces = toReplacements(
+  {Replacement(Context.Sources, Context.getLocation(ID, 2, 1), 6, ""),
+   Replacement(Context.Sources, Context.getLocation(ID, 2, 1), 0,
+   "other\n")});
+  EXPECT_TRUE(applyAllReplacements(Replaces, Context.Rewrite));
+  EXPECT_EQ("line1\nother\nline3\nline4", Context.getRewrittenText(ID));
+}
+
+TEST_F(ReplacementTest, AdjacentReplacements) {
+  FileID ID = Context.createInMemoryFile("input.cpp",
+ "ab");
+  Replacements Replaces = toReplacements(
+  {Replacement(Context.Sources, Context.getLocation(ID, 1, 1), 1, "x"),
+   Replacement(Context.Sources, Context.getLocation(ID, 1, 2), 1, "y")});
+  EXPECT_TRUE(applyAllReplacements(Replaces, Context.Rewrite));
+  EXPECT_EQ("xy", Context.getRewrittenText(ID));
+}
+
 TEST_F(ReplacementTest, SkipsDuplicateReplacements) {
   FileID ID = Context.createInMemoryFile("input.cpp",

Re: [PATCH] D24224: [clang-rename] Merge rename-{ at | all } and optimise USRFindingAction.

[Kirill Bobyrev via cfe-commits]

omtcyfz updated this revision to Diff 71334.
omtcyfz marked an inline comment as done.
omtcyfz added a comment.

Nit.


https://reviews.llvm.org/D24224

Files:
  clang-rename/USRFindingAction.cpp
  clang-rename/USRFindingAction.h
  clang-rename/tool/ClangRename.cpp
  docs/clang-rename.rst
  test/clang-rename/ClassFindByName.cpp
  test/clang-rename/ClassTestMulti.cpp
  test/clang-rename/ClassTestMultiByName.cpp
  test/clang-rename/ClassTestMultiByNameYAML.cpp
  test/clang-rename/FunctionWithClassFindByName.cpp
  test/clang-rename/Inputs/ClassTestMultiByNameYAMLRenameAll.yaml
  test/clang-rename/Inputs/ClassTestMultiByNameYAMLRenameAt.yaml
  test/clang-rename/InvalidOldName.cpp
  test/clang-rename/NoNewName.cpp

Index: test/clang-rename/NoNewName.cpp
===
--- test/clang-rename/NoNewName.cpp
+++ test/clang-rename/NoNewName.cpp
@@ -1,4 +1,4 @@
 // Check for an error while -new-name argument has not been passed to
 // clang-rename.
 // RUN: not clang-rename -offset=133 %s 2>&1 | FileCheck %s
-// CHECK: clang-rename: for the -new-name option: must be specified
+// CHECK: clang-rename: -new-name must be specified.
Index: test/clang-rename/InvalidOldName.cpp
===
--- test/clang-rename/InvalidOldName.cpp
+++ /dev/null
@@ -1,2 +0,0 @@
-// RUN: not clang-rename rename-all -new-name=Foo -old-name=Bar %s -- 2>&1 | FileCheck %s
-// CHECK: clang-rename: could not find symbol Bar.
Index: test/clang-rename/Inputs/ClassTestMultiByNameYAMLRenameAt.yaml
===
--- test/clang-rename/Inputs/ClassTestMultiByNameYAMLRenameAt.yaml
+++ /dev/null
@@ -1,6 +0,0 @@

-- Offset: 6
-  NewName:Bar1
-- Offset: 44
-  NewName:Bar2
-...
Index: test/clang-rename/Inputs/ClassTestMultiByNameYAMLRenameAll.yaml
===
--- test/clang-rename/Inputs/ClassTestMultiByNameYAMLRenameAll.yaml
+++ /dev/null
@@ -1,6 +0,0 @@

-- OldName:Foo1
-  NewName:Bar1
-- OldName:Foo2
-  NewName:Bar2
-...
Index: test/clang-rename/FunctionWithClassFindByName.cpp
===
--- test/clang-rename/FunctionWithClassFindByName.cpp
+++ test/clang-rename/FunctionWithClassFindByName.cpp
@@ -9,4 +9,4 @@
   return 0;
 }
 
-// RUN: clang-rename rename-all -old-name=Foo -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// RUN: clang-rename -qualified-name=Foo -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
Index: test/clang-rename/ClassTestMultiByNameYAML.cpp
===
--- test/clang-rename/ClassTestMultiByNameYAML.cpp
+++ /dev/null
@@ -1,10 +0,0 @@
-class Foo1 { // CHECK: class Bar1
-};
-
-class Foo2 { // CHECK: class Bar2
-};
-
-// Test 1.
-// RUN: clang-rename rename-all -input %S/Inputs/ClassTestMultiByNameYAMLRenameAll.yaml %s -- | sed 's,//.*,,' | FileCheck %s
-// Test 2.
-// RUN: clang-rename rename-all -input %S/Inputs/ClassTestMultiByNameYAMLRenameAt.yaml %s -- | sed 's,//.*,,' | FileCheck %s
Index: test/clang-rename/ClassTestMultiByName.cpp
===
--- test/clang-rename/ClassTestMultiByName.cpp
+++ test/clang-rename/ClassTestMultiByName.cpp
@@ -5,4 +5,4 @@
 };
 
 // Test 1.
-// RUN: clang-rename rename-all -old-name=Foo1 -new-name=Bar1 -old-name=Foo2 -new-name=Bar2 %s -- | sed 's,//.*,,' | FileCheck %s
+// RUN: clang-rename -qualified-name=Foo1 -new-name=Bar1 -qualified-name=Foo2 -new-name=Bar2 %s -- | sed 's,//.*,,' | FileCheck %s
Index: test/clang-rename/ClassTestMulti.cpp
===
--- test/clang-rename/ClassTestMulti.cpp
+++ test/clang-rename/ClassTestMulti.cpp
@@ -5,7 +5,7 @@
 };
 
 // Test 1.
-// RUN: clang-rename rename-all -offset=6 -new-name=Bar1 -offset=76 -new-name=Bar2 %s -- | sed 's,//.*,,' | FileCheck %s
+// RUN: clang-rename -offset=6 -new-name=Bar1 -offset=76 -new-name=Bar2 %s -- | sed 's,//.*,,' | FileCheck %s
 
 // To find offsets after modifying the file, use:
 //   grep -Ubo 'Foo.*' 
Index: test/clang-rename/ClassFindByName.cpp
===
--- test/clang-rename/ClassFindByName.cpp
+++ test/clang-rename/ClassFindByName.cpp
@@ -7,4 +7,4 @@
 }
 
 // Test 1.
-// RUN: clang-rename rename-all -old-name=Foo -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// RUN: clang-rename -qualified-name=Foo -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
Index: docs/clang-rename.rst
===
--- docs/clang-rename.rst
+++ docs/clang-rename.rst
@@ -52,21 +52,29 @@
 Although a command line interface exists, it is highly recommended to use the
 text editor interface instead for better experience.
 
-You can

Re: [PATCH] D24515: Supports adding insertion around non-insertion replacements.

[Daniel Jasper via cfe-commits]

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

Looks good.



Comment at: include/clang/Tooling/Core/Replacement.h:172
@@ -169,1 +171,3 @@
+  ///   - aren't both inserts at the same code location.
+  /// Note: two insertions at the same offset are considered order-dependent.
   /// Replacements with offset UINT_MAX are special - we do not detect 
conflicts

Remove the Note line and just add "(would be order-dependent)" to the line 
above.


https://reviews.llvm.org/D24515



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

[clang-tools-extra] r281456 - [clang-rename] Merge rename-{at|all} & optimize.

[Kirill Bobyrev via cfe-commits]

Author: omtcyfz
Date: Wed Sep 14 08:00:36 2016
New Revision: 281456

URL: http://llvm.org/viewvc/llvm-project?rev=281456&view=rev
Log:
[clang-rename] Merge rename-{at|all} & optimize.

Having both rename-at and rename-all both seems confusing and introduces
unneeded difficulties. Allowing to use both -qualified-name and -offset at once
while performing efficient renamings seems like a feature, too. Maintaining main
function wrappers and custom help becomes redundant while CLI becomes less
confusing.

Reviewers: alexfh

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


Modified:
clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp
clang-tools-extra/trunk/clang-rename/USRFindingAction.h
clang-tools-extra/trunk/clang-rename/tool/ClangRename.cpp
clang-tools-extra/trunk/docs/clang-rename.rst
clang-tools-extra/trunk/test/clang-rename/ClassFindByName.cpp
clang-tools-extra/trunk/test/clang-rename/ClassTestMulti.cpp
clang-tools-extra/trunk/test/clang-rename/ClassTestMultiByName.cpp
clang-tools-extra/trunk/test/clang-rename/FunctionWithClassFindByName.cpp
clang-tools-extra/trunk/test/clang-rename/NoNewName.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=281456&r1=281455&r2=281456&view=diff
==
--- clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp (original)
+++ clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp Wed Sep 14 
08:00:36 2016
@@ -28,6 +28,7 @@
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Refactoring.h"
 #include "clang/Tooling/Tooling.h"
+
 #include 
 #include 
 #include 
@@ -45,11 +46,10 @@ namespace {
 // to virtual method.
 class AdditionalUSRFinder : public RecursiveASTVisitor {
 public:
-  explicit AdditionalUSRFinder(const Decl *FoundDecl, ASTContext &Context,
-   std::vector *USRs)
-  : FoundDecl(FoundDecl), Context(Context), USRs(USRs) {}
+  AdditionalUSRFinder(const Decl *FoundDecl, ASTContext &Context)
+  : FoundDecl(FoundDecl), Context(Context) {}
 
-  void Find() {
+  std::vector Find() {
 // Fill OverriddenMethods and PartialSpecs storages.
 TraverseDecl(Context.getTranslationUnitDecl());
 if (const auto *MethodDecl = dyn_cast(FoundDecl)) {
@@ -66,7 +66,7 @@ public:
 } else {
   USRSet.insert(getUSRForDecl(FoundDecl));
 }
-USRs->insert(USRs->end(), USRSet.begin(), USRSet.end());
+return std::vector(USRSet.begin(), USRSet.end());
   }
 
   bool VisitCXXMethodDecl(const CXXMethodDecl *MethodDecl) {
@@ -129,69 +129,98 @@ private:
 
   const Decl *FoundDecl;
   ASTContext &Context;
-  std::vector *USRs;
   std::set USRSet;
   std::vector OverriddenMethods;
   std::vector PartialSpecs;
 };
 } // namespace
 
-struct NamedDeclFindingConsumer : public ASTConsumer {
-  void HandleTranslationUnit(ASTContext &Context) override {
-const SourceManager &SourceMgr = Context.getSourceManager();
-// The file we look for the USR in will always be the main source file.
+class NamedDeclFindingConsumer : public ASTConsumer {
+public:
+  NamedDeclFindingConsumer(ArrayRef SymbolOffsets,
+   ArrayRef QualifiedNames,
+   std::vector &SpellingNames,
+   std::vector> &USRList,
+   bool &ErrorOccurred)
+  : SymbolOffsets(SymbolOffsets), QualifiedNames(QualifiedNames),
+SpellingNames(SpellingNames), USRList(USRList),
+ErrorOccurred(ErrorOccurred) {}
+
+private:
+  bool FindSymbol(ASTContext &Context, const SourceManager &SourceMgr,
+  unsigned SymbolOffset, const std::string &QualifiedName) {
+DiagnosticsEngine &Engine = Context.getDiagnostics();
+
 const SourceLocation Point =
 SourceMgr.getLocForStartOfFile(SourceMgr.getMainFileID())
 .getLocWithOffset(SymbolOffset);
-if (!Point.isValid())
-  return;
-const NamedDecl *FoundDecl = nullptr;
-if (OldName.empty())
-  FoundDecl = getNamedDeclAt(Context, Point);
-else
-  FoundDecl = getNamedDeclFor(Context, OldName);
+
+if (!Point.isValid()) {
+  ErrorOccurred = true;
+  unsigned InvalidOffset = Engine.getCustomDiagID(
+  DiagnosticsEngine::Error,
+  "SourceLocation in file %0 at offset %1 is invalid");
+  Engine.Report(Point, InvalidOffset) << SourceMgr.getFilename(Point)
+  << SymbolOffset;
+  return false;
+}
+
+const NamedDecl *FoundDecl = QualifiedName.empty()
+ ? getNamedDeclAt(Context, Point)
+ : getNamedDeclFor(Context, QualifiedName);
+
 if (FoundDecl == nullptr) {
-  if (OldName.empty()) {
+  if (QualifiedName.empty()) {
 FullSourceLoc FullLoc(

Re: [PATCH] D24515: Supports adding insertion around non-insertion replacements.

[Eric Liu via cfe-commits]

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

- Updated comment.


https://reviews.llvm.org/D24515

Files:
  include/clang/Tooling/Core/Replacement.h
  lib/Tooling/Core/Replacement.cpp
  unittests/Tooling/RefactoringTest.cpp

Index: unittests/Tooling/RefactoringTest.cpp
===
--- unittests/Tooling/RefactoringTest.cpp
+++ unittests/Tooling/RefactoringTest.cpp
@@ -115,24 +115,26 @@
   llvm::consumeError(std::move(Err));
 }
 
-TEST_F(ReplacementTest, FailAddOverlappingInsertions) {
+TEST_F(ReplacementTest, AddAdjacentInsertionAndReplacement) {
   Replacements Replaces;
   // Test adding an insertion at the offset of an existing replacement.
   auto Err = Replaces.add(Replacement("x.cc", 10, 3, "replace"));
   EXPECT_TRUE(!Err);
   llvm::consumeError(std::move(Err));
   Err = Replaces.add(Replacement("x.cc", 10, 0, "insert"));
-  EXPECT_TRUE((bool)Err);
+  EXPECT_TRUE(!Err);
   llvm::consumeError(std::move(Err));
+  EXPECT_EQ(Replaces.size(), 2u);
 
   Replaces.clear();
   // Test overlap with an existing insertion.
   Err = Replaces.add(Replacement("x.cc", 10, 0, "insert"));
   EXPECT_TRUE(!Err);
   llvm::consumeError(std::move(Err));
   Err = Replaces.add(Replacement("x.cc", 10, 3, "replace"));
-  EXPECT_TRUE((bool)Err);
+  EXPECT_TRUE(!Err);
   llvm::consumeError(std::move(Err));
+  EXPECT_EQ(Replaces.size(), 2u);
 }
 
 TEST_F(ReplacementTest, FailAddRegression) {
@@ -157,14 +159,24 @@
   llvm::consumeError(std::move(Err));
 }
 
-TEST_F(ReplacementTest, FailAddInsertAtOffsetOfReplacement) {
+TEST_F(ReplacementTest, InsertAtOffsetOfReplacement) {
   Replacements Replaces;
   auto Err = Replaces.add(Replacement("x.cc", 10, 2, ""));
   EXPECT_TRUE(!Err);
   llvm::consumeError(std::move(Err));
   Err = Replaces.add(Replacement("x.cc", 10, 0, ""));
-  EXPECT_TRUE((bool)Err);
+  EXPECT_TRUE(!Err);
+  llvm::consumeError(std::move(Err));
+  EXPECT_EQ(Replaces.size(), 2u);
+
+  Replaces.clear();
+  Err = Replaces.add(Replacement("x.cc", 10, 0, ""));
+  EXPECT_TRUE(!Err);
+  llvm::consumeError(std::move(Err));
+  Err = Replaces.add(Replacement("x.cc", 10, 2, ""));
+  EXPECT_TRUE(!Err);
   llvm::consumeError(std::move(Err));
+  EXPECT_EQ(Replaces.size(), 2u);
 }
 
 TEST_F(ReplacementTest, FailAddInsertAtOtherInsert) {
@@ -175,6 +187,38 @@
   Err = Replaces.add(Replacement("x.cc", 10, 0, "b"));
   EXPECT_TRUE((bool)Err);
   llvm::consumeError(std::move(Err));
+
+  Replaces.clear();
+  Err = Replaces.add(Replacement("x.cc", 10, 0, ""));
+  EXPECT_TRUE(!Err);
+  llvm::consumeError(std::move(Err));
+  Err = Replaces.add(Replacement("x.cc", 10, 0, ""));
+  EXPECT_TRUE((bool)Err);
+  llvm::consumeError(std::move(Err));
+
+  Replaces.clear();
+  Err = Replaces.add(Replacement("x.cc", 10, 0, ""));
+  EXPECT_TRUE(!Err);
+  llvm::consumeError(std::move(Err));
+  Err = Replaces.add(Replacement("x.cc", 10, 3, ""));
+  EXPECT_TRUE(!Err);
+  llvm::consumeError(std::move(Err));
+  Err = Replaces.add(Replacement("x.cc", 10, 0, ""));
+  EXPECT_TRUE((bool)Err);
+  llvm::consumeError(std::move(Err));
+}
+
+TEST_F(ReplacementTest, InsertBetweenAdjacentReplacements) {
+  Replacements Replaces;
+  auto Err = Replaces.add(Replacement("x.cc", 10, 5, "a"));
+  EXPECT_TRUE(!Err);
+  llvm::consumeError(std::move(Err));
+  Err = Replaces.add(Replacement("x.cc", 8, 2, "a"));
+  EXPECT_TRUE(!Err);
+  llvm::consumeError(std::move(Err));
+  Err = Replaces.add(Replacement("x.cc", 10, 0, "b"));
+  EXPECT_TRUE(!Err);
+  llvm::consumeError(std::move(Err));
 }
 
 TEST_F(ReplacementTest, CanApplyReplacements) {
@@ -189,6 +233,29 @@
   EXPECT_EQ("line1\nreplaced\nother\nline4", Context.getRewrittenText(ID));
 }
 
+// Verifies that replacement/deletion is applied before insertion at the same
+// offset.
+TEST_F(ReplacementTest, InsertAndDelete) {
+  FileID ID = Context.createInMemoryFile("input.cpp",
+ "line1\nline2\nline3\nline4");
+  Replacements Replaces = toReplacements(
+  {Replacement(Context.Sources, Context.getLocation(ID, 2, 1), 6, ""),
+   Replacement(Context.Sources, Context.getLocation(ID, 2, 1), 0,
+   "other\n")});
+  EXPECT_TRUE(applyAllReplacements(Replaces, Context.Rewrite));
+  EXPECT_EQ("line1\nother\nline3\nline4", Context.getRewrittenText(ID));
+}
+
+TEST_F(ReplacementTest, AdjacentReplacements) {
+  FileID ID = Context.createInMemoryFile("input.cpp",
+ "ab");
+  Replacements Replaces = toReplacements(
+  {Replacement(Context.Sources, Context.getLocation(ID, 1, 1), 1, "x"),
+   Replacement(Context.Sources, Context.getLocation(ID, 1, 2), 1, "y")});
+  EXPECT_TRUE(applyAllReplacements(Replaces, Context.Rewrite));
+  EXPECT_EQ("xy", Context.getRewrittenText(ID));
+}
+
 TEST_F(ReplacementTest, SkipsDuplicateReplacements) {
   FileID ID = Context.createInMemoryFile("input.cpp",
  "line1\nline2\nline3\nline4")

Re: [PATCH] D24224: [clang-rename] Merge rename-{ at | all } and optimise USRFindingAction.

[Kirill Bobyrev via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL281456: [clang-rename] Merge rename-{at|all} & optimize. 
(authored by omtcyfz).

Changed prior to commit:
  https://reviews.llvm.org/D24224?vs=71334&id=71336#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24224

Files:
  clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp
  clang-tools-extra/trunk/clang-rename/USRFindingAction.h
  clang-tools-extra/trunk/clang-rename/tool/ClangRename.cpp
  clang-tools-extra/trunk/docs/clang-rename.rst
  clang-tools-extra/trunk/test/clang-rename/ClassFindByName.cpp
  clang-tools-extra/trunk/test/clang-rename/ClassTestMulti.cpp
  clang-tools-extra/trunk/test/clang-rename/ClassTestMultiByName.cpp
  clang-tools-extra/trunk/test/clang-rename/FunctionWithClassFindByName.cpp
  clang-tools-extra/trunk/test/clang-rename/NoNewName.cpp

Index: clang-tools-extra/trunk/clang-rename/tool/ClangRename.cpp
===
--- clang-tools-extra/trunk/clang-rename/tool/ClangRename.cpp
+++ clang-tools-extra/trunk/clang-rename/tool/ClangRename.cpp
@@ -38,28 +38,12 @@
 #include 
 
 using namespace llvm;
-
 using namespace clang;
 
-cl::OptionCategory ClangRenameAtCategory("clang-rename rename-at options");
-cl::OptionCategory ClangRenameAllCategory("clang-rename rename-all options");
-
-const char RenameAtUsage[] = "A tool to rename symbols in C/C++ code.\n\
-clang-rename renames every occurrence of a symbol found at  in\n\
-. If -i is specified, the edited files are overwritten to disk.\n\
-Otherwise, the results are written to stdout.\n";
-
-const char RenameAllUsage[] = "A tool to rename symbols in C/C++ code.\n\
-clang-rename performs renaming given pairs {offset | old-name} -> new-name.\n";
-
-static int renameAtMain(int argc, const char *argv[]);
-static int renameAllMain(int argc, const char *argv[]);
-static int helpMain(int argc, const char *argv[]);
-
 /// \brief An oldname -> newname rename.
 struct RenameAllInfo {
-  std::string OldName;
   unsigned Offset = 0;
+  std::string QualifiedName;
   std::string NewName;
 };
 
@@ -72,81 +56,51 @@
 /// (de)serialized.
 template <> struct MappingTraits {
   static void mapping(IO &IO, RenameAllInfo &Info) {
-IO.mapOptional("OldName", Info.OldName);
 IO.mapOptional("Offset", Info.Offset);
+IO.mapOptional("QualifiedName", Info.QualifiedName);
 IO.mapRequired("NewName", Info.NewName);
   }
 };
 
 } // end namespace yaml
 } // end namespace llvm
 
-int main(int argc, const char **argv) {
-  if (argc > 1) {
-using MainFunction = std::function;
-MainFunction Func = StringSwitch(argv[1])
-.Case("rename-at", renameAtMain)
-.Case("rename-all", renameAllMain)
-.Cases("-help", "--help", helpMain)
-.Default(nullptr);
-
-if (Func) {
-  std::string Invocation = std::string(argv[0]) + " " + argv[1];
-  argv[1] = Invocation.c_str();
-  return Func(argc - 1, argv + 1);
-} else {
-  return renameAtMain(argc, argv);
-}
-  }
+static cl::OptionCategory ClangRenameOptions("clang-rename common options");
 
-  helpMain(argc, argv);
-  return 1;
-}
+static cl::list SymbolOffsets(
+"offset",
+cl::desc("Locates the symbol by offset as opposed to :."),
+cl::ZeroOrMore, cl::cat(ClangRenameOptions));
+static cl::opt Inplace("i", cl::desc("Overwrite edited s."),
+ cl::cat(ClangRenameOptions));
+static cl::list
+QualifiedNames("qualified-name",
+   cl::desc("The fully qualified name of the symbol."),
+   cl::ZeroOrMore, cl::cat(ClangRenameOptions));
+
+static cl::list
+NewNames("new-name", cl::desc("The new name to change the symbol to."),
+ cl::ZeroOrMore, cl::cat(ClangRenameOptions));
+static cl::opt PrintName(
+"pn",
+cl::desc("Print the found symbol's name prior to renaming to stderr."),
+cl::cat(ClangRenameOptions));
+static cl::opt PrintLocations(
+"pl", cl::desc("Print the locations affected by renaming to stderr."),
+cl::cat(ClangRenameOptions));
+static cl::opt
+ExportFixes("export-fixes",
+cl::desc("YAML file to store suggested fixes in."),
+cl::value_desc("filename"), cl::cat(ClangRenameOptions));
+static cl::opt
+Input("input", cl::desc("YAML file to load oldname-newname pairs from."),
+  cl::Optional, cl::cat(ClangRenameOptions));
 
-int subcommandMain(bool isRenameAll, int argc, const char **argv) {
-  cl::OptionCategory *Category = nullptr;
-  const char *Usage = nullptr;
-  if (isRenameAll) {
-Category = &ClangRenameAllCategory;
-Usage = RenameAllUsage;
-  } else {
-Category = &ClangRenameAtCategory;
-Usage = RenameAtUsage;
-  }
-
-  cl::list NewNames(
-  "new-name", cl::desc("The new name to change the symbol to."),
-  (isRenameAll ? cl::ZeroOrMore : cl::Requ

Re: r281452 - Revert "[modules] When merging one definition into another, propagate the list of re-exporting modules from the discarded definition to the retained definition."

[Nico Weber via cfe-commits]

When reverting something, please say why in the commit message.

On Sep 14, 2016 6:13 AM, "Eric Liu via cfe-commits" <
cfe-commits@lists.llvm.org> wrote:

> Author: ioeric
> Date: Wed Sep 14 05:05:10 2016
> New Revision: 281452
>
> URL: http://llvm.org/viewvc/llvm-project?rev=281452&view=rev
> Log:
> Revert "[modules] When merging one definition into another, propagate the
> list of re-exporting modules from the discarded definition to the retained
> definition."
>
> This reverts commit r281429.
>
> Modified:
> cfe/trunk/include/clang/AST/ASTContext.h
> cfe/trunk/lib/AST/ASTContext.cpp
> cfe/trunk/lib/Serialization/ASTReader.cpp
> cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/a.h
> cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/b.h
> cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/c.h
> cfe/trunk/test/Modules/merge-template-pattern-visibility.cpp
>
> Modified: cfe/trunk/include/clang/AST/ASTContext.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/AST/ASTContext.h?rev=281452&r1=281451&r2=281452&view=diff
> 
> ==
> --- cfe/trunk/include/clang/AST/ASTContext.h (original)
> +++ cfe/trunk/include/clang/AST/ASTContext.h Wed Sep 14 05:05:10 2016
> @@ -308,18 +308,10 @@ class ASTContext : public RefCountedBase
>/// merged into.
>llvm::DenseMap MergedDecls;
>
> -  /// The modules into which a definition has been merged, or a map from a
> -  /// merged definition to its canonical definition. This is really a
> union of
> -  /// a NamedDecl* and a vector of Module*.
> -  struct MergedModulesOrCanonicalDef {
> -llvm::TinyPtrVector MergedModules;
> -NamedDecl *CanonicalDef = nullptr;
> -  };
> -
>/// \brief A mapping from a defining declaration to a list of modules
> (other
>/// than the owning module of the declaration) that contain merged
>/// definitions of that entity.
> -  llvm::DenseMap
> MergedDefModules;
> +  llvm::DenseMap>
> MergedDefModules;
>
>/// \brief Initializers for a module, in order. Each Decl will be either
>/// something that has a semantic effect on startup (such as a variable
> with
> @@ -902,7 +894,6 @@ public:
>/// and should be visible whenever \p M is visible.
>void mergeDefinitionIntoModule(NamedDecl *ND, Module *M,
>   bool NotifyListeners = true);
> -  void mergeDefinitionIntoModulesOf(NamedDecl *ND, NamedDecl *Other);
>/// \brief Clean up the merged definition list. Call this if you might
> have
>/// added duplicates into the list.
>void deduplicateMergedDefinitonsFor(NamedDecl *ND);
> @@ -913,9 +904,7 @@ public:
>  auto MergedIt = MergedDefModules.find(Def);
>  if (MergedIt == MergedDefModules.end())
>return None;
> -if (auto *CanonDef = MergedIt->second.CanonicalDef)
> -  return getModulesWithMergedDefinition(CanonDef);
> -return MergedIt->second.MergedModules;
> +return MergedIt->second;
>}
>
>/// Add a declaration to the list of declarations that are initialized
>
> Modified: cfe/trunk/lib/AST/ASTContext.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/
> ASTContext.cpp?rev=281452&r1=281451&r2=281452&view=diff
> 
> ==
> --- cfe/trunk/lib/AST/ASTContext.cpp (original)
> +++ cfe/trunk/lib/AST/ASTContext.cpp Wed Sep 14 05:05:10 2016
> @@ -890,59 +890,10 @@ void ASTContext::mergeDefinitionIntoModu
>  if (auto *Listener = getASTMutationListener())
>Listener->RedefinedHiddenDefinition(ND, M);
>
> -  if (getLangOpts().ModulesLocalVisibility) {
> -auto *Merged = &MergedDefModules[ND];
> -if (auto *CanonDef = Merged->CanonicalDef)
> -  Merged = &MergedDefModules[CanonDef];
> -Merged->MergedModules.push_back(M);
> -  } else {
> -auto MergedIt = MergedDefModules.find(ND);
> -if (MergedIt != MergedDefModules.end() &&
> MergedIt->second.CanonicalDef)
> -  ND = MergedIt->second.CanonicalDef;
> +  if (getLangOpts().ModulesLocalVisibility)
> +MergedDefModules[ND].push_back(M);
> +  else
>  ND->setHidden(false);
> -  }
> -}
> -
> -void ASTContext::mergeDefinitionIntoModulesOf(NamedDecl *Def,
> -  NamedDecl *Other) {
> -  // We need to know the owning module of the merge source.
> -  assert(Other->isFromASTFile() && "merge of non-imported decl not
> supported");
> -  assert(Def != Other && "merging definition into itself");
> -
> -  if (!getLangOpts().ModulesLocalVisibility && !Other->isHidden()) {
> -Def->setHidden(false);
> -return;
> -  }
> -  assert(Other->getImportedOwningModule() &&
> - "hidden, imported declaration has no owning module");
> -
> -  // Mark Def as the canonical definition of merged definition Other.
> -  {
> -auto &OtherMerged = MergedDefModules[Other];
> -assert((!OtherMerged

Re: [PATCH] D24515: Supports adding insertion around non-insertion replacements.

[Eric Liu via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL281457: Supports adding insertion around non-insertion 
replacements. (authored by ioeric).

Changed prior to commit:
  https://reviews.llvm.org/D24515?vs=71335&id=71337#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24515

Files:
  cfe/trunk/include/clang/Tooling/Core/Replacement.h
  cfe/trunk/lib/Tooling/Core/Replacement.cpp
  cfe/trunk/unittests/Tooling/RefactoringTest.cpp

Index: cfe/trunk/include/clang/Tooling/Core/Replacement.h
===
--- cfe/trunk/include/clang/Tooling/Core/Replacement.h
+++ cfe/trunk/include/clang/Tooling/Core/Replacement.h
@@ -158,14 +158,18 @@
 
   /// \brief Adds a new replacement \p R to the current set of replacements.
   /// \p R must have the same file path as all existing replacements.
-  /// Returns true if the replacement is successfully inserted; otherwise,
+  /// Returns `success` if the replacement is successfully inserted; otherwise,
   /// it returns an llvm::Error, i.e. there is a conflict between R and the
-  /// existing replacements or R's file path is different from the filepath of
-  /// existing replacements. Callers must explicitly check the Error returned.
-  /// This prevents users from adding order-dependent replacements. To control
-  /// the order in which order-dependent replacements are applied, use
-  /// merge({R}) with R referring to the changed code after applying all
-  /// existing replacements.
+  /// existing replacements (i.e. they are order-dependent) or R's file path is
+  /// different from the filepath of existing replacements. Callers must
+  /// explicitly check the Error returned. This prevents users from adding
+  /// order-dependent replacements. To control the order in which
+  /// order-dependent replacements are applied, use merge({R}) with R referring
+  /// to the changed code after applying all existing replacements.
+  /// Two replacements are considered order-independent if they:
+  ///   - don't overlap (being directly adjacent is fine) and
+  ///   - aren't both inserts at the same code location (would be
+  /// order-dependent).
   /// Replacements with offset UINT_MAX are special - we do not detect conflicts
   /// for such replacements since users may add them intentionally as a special
   /// category of replacements.
Index: cfe/trunk/lib/Tooling/Core/Replacement.cpp
===
--- cfe/trunk/lib/Tooling/Core/Replacement.cpp
+++ cfe/trunk/lib/Tooling/Core/Replacement.cpp
@@ -137,6 +137,14 @@
 ReplacementText);
 }
 
+llvm::Error makeConflictReplacementsError(const Replacement &New,
+  const Replacement &Existing) {
+  return llvm::make_error(
+  "New replacement:\n" + New.toString() +
+  "\nconflicts with existing replacement:\n" + Existing.toString(),
+  llvm::inconvertibleErrorCode());
+}
+
 llvm::Error Replacements::add(const Replacement &R) {
   // Check the file path.
   if (!Replaces.empty() && R.getFilePath() != Replaces.begin()->getFilePath())
@@ -163,11 +171,22 @@
   // entries that start at the end can still be conflicting if R is an
   // insertion.
   auto I = Replaces.lower_bound(AtEnd);
-  // If it starts at the same offset as R (can only happen if R is an
-  // insertion), we have a conflict.  In that case, increase I to fall through
-  // to the conflict check.
-  if (I != Replaces.end() && R.getOffset() == I->getOffset())
-++I;
+  // If `I` starts at the same offset as `R`, `R` must be an insertion.
+  if (I != Replaces.end() && R.getOffset() == I->getOffset()) {
+assert(R.getLength() == 0);
+// `I` is also an insertion, `R` and `I` conflict.
+if (I->getLength() == 0)
+  return makeConflictReplacementsError(R, *I);
+// Insertion `R` is adjacent to a non-insertion replacement `I`, so they
+// are order-independent. It is safe to assume that `R` will not conflict
+// with any replacement before `I` since all replacements before `I` must
+// either end before `R` or end at `R` but has length > 0 (if the
+// replacement before `I` is an insertion at `R`, it would have been `I`
+// since it is a lower bound of `AtEnd` and ordered before the current `I`
+// in the set).
+Replaces.insert(R);
+return llvm::Error::success();
+  }
 
   // I is the smallest iterator whose entry cannot overlap.
   // If that is begin(), there are no overlaps.
@@ -178,16 +197,19 @@
   --I;
   // If the previous entry does not overlap, we know that entries before it
   // can also not overlap.
-  if (R.getOffset() != I->getOffset() &&
-  !Range(R.getOffset(), R.getLength())
+  if (!Range(R.getOffset(), R.getLength())
.overlapsWith(Range(I->getOffset(), I->getLength( {
+// If `R` and `I` do not have the same offset, it is safe to add `R` since
+// it must com

r281457 - Supports adding insertion around non-insertion replacements.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Wed Sep 14 08:04:51 2016
New Revision: 281457

URL: http://llvm.org/viewvc/llvm-project?rev=281457&view=rev
Log:
Supports adding insertion around non-insertion replacements.

Summary:
Extend `tooling::Replacements::add()` to support adding order-independent 
replacements.

Two replacements are considered order-independent if one of the following 
conditions is true:
  - They do not overlap. (This is already supported.)
  - One replacement is insertion, and the other is a replacement with
length > 0, and the insertion is adjecent to but not contained in the
other replacement. In this case, the replacement should always change
the original code instead of the inserted text.

Reviewers: klimek, djasper

Subscribers: cfe-commits, klimek

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

Modified:
cfe/trunk/include/clang/Tooling/Core/Replacement.h
cfe/trunk/lib/Tooling/Core/Replacement.cpp
cfe/trunk/unittests/Tooling/RefactoringTest.cpp

Modified: cfe/trunk/include/clang/Tooling/Core/Replacement.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Core/Replacement.h?rev=281457&r1=281456&r2=281457&view=diff
==
--- cfe/trunk/include/clang/Tooling/Core/Replacement.h (original)
+++ cfe/trunk/include/clang/Tooling/Core/Replacement.h Wed Sep 14 08:04:51 2016
@@ -158,14 +158,18 @@ class Replacements {
 
   /// \brief Adds a new replacement \p R to the current set of replacements.
   /// \p R must have the same file path as all existing replacements.
-  /// Returns true if the replacement is successfully inserted; otherwise,
+  /// Returns `success` if the replacement is successfully inserted; otherwise,
   /// it returns an llvm::Error, i.e. there is a conflict between R and the
-  /// existing replacements or R's file path is different from the filepath of
-  /// existing replacements. Callers must explicitly check the Error returned.
-  /// This prevents users from adding order-dependent replacements. To control
-  /// the order in which order-dependent replacements are applied, use
-  /// merge({R}) with R referring to the changed code after applying all
-  /// existing replacements.
+  /// existing replacements (i.e. they are order-dependent) or R's file path is
+  /// different from the filepath of existing replacements. Callers must
+  /// explicitly check the Error returned. This prevents users from adding
+  /// order-dependent replacements. To control the order in which
+  /// order-dependent replacements are applied, use merge({R}) with R referring
+  /// to the changed code after applying all existing replacements.
+  /// Two replacements are considered order-independent if they:
+  ///   - don't overlap (being directly adjacent is fine) and
+  ///   - aren't both inserts at the same code location (would be
+  /// order-dependent).
   /// Replacements with offset UINT_MAX are special - we do not detect 
conflicts
   /// for such replacements since users may add them intentionally as a special
   /// category of replacements.

Modified: cfe/trunk/lib/Tooling/Core/Replacement.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Core/Replacement.cpp?rev=281457&r1=281456&r2=281457&view=diff
==
--- cfe/trunk/lib/Tooling/Core/Replacement.cpp (original)
+++ cfe/trunk/lib/Tooling/Core/Replacement.cpp Wed Sep 14 08:04:51 2016
@@ -137,6 +137,14 @@ void Replacement::setFromSourceRange(con
 ReplacementText);
 }
 
+llvm::Error makeConflictReplacementsError(const Replacement &New,
+  const Replacement &Existing) {
+  return llvm::make_error(
+  "New replacement:\n" + New.toString() +
+  "\nconflicts with existing replacement:\n" + Existing.toString(),
+  llvm::inconvertibleErrorCode());
+}
+
 llvm::Error Replacements::add(const Replacement &R) {
   // Check the file path.
   if (!Replaces.empty() && R.getFilePath() != Replaces.begin()->getFilePath())
@@ -163,11 +171,22 @@ llvm::Error Replacements::add(const Repl
   // entries that start at the end can still be conflicting if R is an
   // insertion.
   auto I = Replaces.lower_bound(AtEnd);
-  // If it starts at the same offset as R (can only happen if R is an
-  // insertion), we have a conflict.  In that case, increase I to fall through
-  // to the conflict check.
-  if (I != Replaces.end() && R.getOffset() == I->getOffset())
-++I;
+  // If `I` starts at the same offset as `R`, `R` must be an insertion.
+  if (I != Replaces.end() && R.getOffset() == I->getOffset()) {
+assert(R.getLength() == 0);
+// `I` is also an insertion, `R` and `I` conflict.
+if (I->getLength() == 0)
+  return makeConflictReplacementsError(R, *I);
+// Insertion `R` is adjacent to a non-insertion replacement `I`, so they
+// are order-independent. It i

Re: [clang-tools-extra] r281453 - [clang-tidy] Add check 'misc-use-after-move'

[Martin Böhme via cfe-commits]

>
> While I'm excited to see this go in anywhere, I have to say I'm a bit sad
> it isn't going in as a warning and instead inside clang-tidy. This has been
> a much requested warning from Clang for many years.
>
> Is there a concise description of why this design was chosen? Are there
> specific problems that make it infeasible to be a warnings? Is there maybe
> a plan to move it to a warning after some set of issues are addressed?
>

Yes, my hope is that this can be moved to a warning, and others have
expressed the same thought to me.

There are two main issues that need to be addressed before this can be made
a warning:

   - I suspect (though I haven't measured) that the implementation isn't
   currently efficient enough to be made a Clang warning.
   - There are a number of scenarios in which the check warns when arguably
   it should not. These are:
  - User-defined types that make guarantees about the state of a
  moved-from object (in the way that std::unique_ptr and
std::shared_ptr do).
  Some form of annotation could be used to indicate that this is the case.
  - User-defined types that provide a member function to reinitialize
  the entire object (the way that clear() does on the standard container
  classes). These could either again be annotated in some way, or we could
  simply assume that any non-const member function reinitializes the object.
  - Tests for move constructors / move assignment operators. Often,
  these tests check that a moved-from object is left in a certain
state, even
  if the class in question does not provide such a guarantee to
clients (the
  intent being to check that the move constructor / move
assignment operator
  do not simply perform a copy).

All of these scenarios are probably somewhat controversial, but I don't
think there's a broad consensus that any of them should be prohibited, so
there needs to be a way to silence the warning in these cases.


I do hope to resolve these issues and make this check into a warning, but
in the meantime, having it available as a clang-tidy check seems like a
good way to gather feedback and gain experience with it.

Since you say this is a much-requested warning: Any pointers to previous
discussions that I should be aware of?


See user-facing documentation for details.
>>
>
> In general, I suspect at least some more context should be provided in
> change descriptions. =]
>

Thanks -- still new to this.

The intent was not to repeat myself, but I evidently went a bit far with
that. ;) Is it of value if I provide a short summary on this thread, or is
this just something I should keep in mind for future patches?
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r281459 - reverting r281456

[Kirill Bobyrev via cfe-commits]

Author: omtcyfz
Date: Wed Sep 14 08:23:14 2016
New Revision: 281459

URL: http://llvm.org/viewvc/llvm-project?rev=281459&view=rev
Log:
reverting r281456

Modified:
clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp
clang-tools-extra/trunk/clang-rename/USRFindingAction.h
clang-tools-extra/trunk/clang-rename/tool/ClangRename.cpp
clang-tools-extra/trunk/docs/clang-rename.rst
clang-tools-extra/trunk/test/clang-rename/ClassFindByName.cpp
clang-tools-extra/trunk/test/clang-rename/ClassTestMulti.cpp
clang-tools-extra/trunk/test/clang-rename/ClassTestMultiByName.cpp
clang-tools-extra/trunk/test/clang-rename/FunctionWithClassFindByName.cpp
clang-tools-extra/trunk/test/clang-rename/NoNewName.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=281459&r1=281458&r2=281459&view=diff
==
--- clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp (original)
+++ clang-tools-extra/trunk/clang-rename/USRFindingAction.cpp Wed Sep 14 
08:23:14 2016
@@ -28,7 +28,6 @@
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Refactoring.h"
 #include "clang/Tooling/Tooling.h"
-
 #include 
 #include 
 #include 
@@ -46,10 +45,11 @@ namespace {
 // to virtual method.
 class AdditionalUSRFinder : public RecursiveASTVisitor {
 public:
-  AdditionalUSRFinder(const Decl *FoundDecl, ASTContext &Context)
-  : FoundDecl(FoundDecl), Context(Context) {}
+  explicit AdditionalUSRFinder(const Decl *FoundDecl, ASTContext &Context,
+   std::vector *USRs)
+  : FoundDecl(FoundDecl), Context(Context), USRs(USRs) {}
 
-  std::vector Find() {
+  void Find() {
 // Fill OverriddenMethods and PartialSpecs storages.
 TraverseDecl(Context.getTranslationUnitDecl());
 if (const auto *MethodDecl = dyn_cast(FoundDecl)) {
@@ -66,7 +66,7 @@ public:
 } else {
   USRSet.insert(getUSRForDecl(FoundDecl));
 }
-return std::vector(USRSet.begin(), USRSet.end());
+USRs->insert(USRs->end(), USRSet.begin(), USRSet.end());
   }
 
   bool VisitCXXMethodDecl(const CXXMethodDecl *MethodDecl) {
@@ -129,98 +129,69 @@ private:
 
   const Decl *FoundDecl;
   ASTContext &Context;
+  std::vector *USRs;
   std::set USRSet;
   std::vector OverriddenMethods;
   std::vector PartialSpecs;
 };
 } // namespace
 
-class NamedDeclFindingConsumer : public ASTConsumer {
-public:
-  NamedDeclFindingConsumer(ArrayRef SymbolOffsets,
-   ArrayRef QualifiedNames,
-   std::vector &SpellingNames,
-   std::vector> &USRList,
-   bool &ErrorOccurred)
-  : SymbolOffsets(SymbolOffsets), QualifiedNames(QualifiedNames),
-SpellingNames(SpellingNames), USRList(USRList),
-ErrorOccurred(ErrorOccurred) {}
-
-private:
-  bool FindSymbol(ASTContext &Context, const SourceManager &SourceMgr,
-  unsigned SymbolOffset, const std::string &QualifiedName) {
-DiagnosticsEngine &Engine = Context.getDiagnostics();
-
+struct NamedDeclFindingConsumer : public ASTConsumer {
+  void HandleTranslationUnit(ASTContext &Context) override {
+const SourceManager &SourceMgr = Context.getSourceManager();
+// The file we look for the USR in will always be the main source file.
 const SourceLocation Point =
 SourceMgr.getLocForStartOfFile(SourceMgr.getMainFileID())
 .getLocWithOffset(SymbolOffset);
-
-if (!Point.isValid()) {
-  ErrorOccurred = true;
-  unsigned InvalidOffset = Engine.getCustomDiagID(
-  DiagnosticsEngine::Error,
-  "SourceLocation in file %0 at offset %1 is invalid");
-  Engine.Report(Point, InvalidOffset) << SourceMgr.getFilename(Point)
-  << SymbolOffset;
-  return false;
-}
-
-const NamedDecl *FoundDecl = QualifiedName.empty()
- ? getNamedDeclAt(Context, Point)
- : getNamedDeclFor(Context, QualifiedName);
-
+if (!Point.isValid())
+  return;
+const NamedDecl *FoundDecl = nullptr;
+if (OldName.empty())
+  FoundDecl = getNamedDeclAt(Context, Point);
+else
+  FoundDecl = getNamedDeclFor(Context, OldName);
 if (FoundDecl == nullptr) {
-  if (QualifiedName.empty()) {
+  if (OldName.empty()) {
 FullSourceLoc FullLoc(Point, SourceMgr);
-unsigned CouldNotFindSymbolAt = Engine.getCustomDiagID(
-DiagnosticsEngine::Error,
-"clang-rename could not find symbol (offset %0)");
-Engine.Report(Point, CouldNotFindSymbolAt) << SymbolOffset;
-ErrorOccurred = true;
-return false;
+errs() << "clang-rename: could not find symbol at "
+   << SourceMgr.getFilename(Point) << ":"
+ 

Re: r281452 - Revert "[modules] When merging one definition into another, propagate the list of re-exporting modules from the discarded definition to the retained definition."

[Eric Liu via cfe-commits]

This revision caused internal build breakage, but unfortunately I'm not
sure what exactly in this revision caused the breakage. I have reported the
issue to @rsmith - the revision author.

On Wed, Sep 14, 2016 at 3:10 PM Nico Weber  wrote:

> When reverting something, please say why in the commit message.
>
> On Sep 14, 2016 6:13 AM, "Eric Liu via cfe-commits" <
> cfe-commits@lists.llvm.org> wrote:
>
> Author: ioeric
> Date: Wed Sep 14 05:05:10 2016
> New Revision: 281452
>
> URL: http://llvm.org/viewvc/llvm-project?rev=281452&view=rev
> Log:
> Revert "[modules] When merging one definition into another, propagate the
> list of re-exporting modules from the discarded definition to the retained
> definition."
>
> This reverts commit r281429.
>
> Modified:
> cfe/trunk/include/clang/AST/ASTContext.h
> cfe/trunk/lib/AST/ASTContext.cpp
> cfe/trunk/lib/Serialization/ASTReader.cpp
> cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/a.h
> cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/b.h
> cfe/trunk/test/Modules/Inputs/merge-template-pattern-visibility/c.h
> cfe/trunk/test/Modules/merge-template-pattern-visibility.cpp
>
> Modified: cfe/trunk/include/clang/AST/ASTContext.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=281452&r1=281451&r2=281452&view=diff
>
> ==
> --- cfe/trunk/include/clang/AST/ASTContext.h (original)
> +++ cfe/trunk/include/clang/AST/ASTContext.h Wed Sep 14 05:05:10 2016
> @@ -308,18 +308,10 @@ class ASTContext : public RefCountedBase
>/// merged into.
>llvm::DenseMap MergedDecls;
>
> -  /// The modules into which a definition has been merged, or a map from a
> -  /// merged definition to its canonical definition. This is really a
> union of
> -  /// a NamedDecl* and a vector of Module*.
> -  struct MergedModulesOrCanonicalDef {
> -llvm::TinyPtrVector MergedModules;
> -NamedDecl *CanonicalDef = nullptr;
> -  };
> -
>/// \brief A mapping from a defining declaration to a list of modules
> (other
>/// than the owning module of the declaration) that contain merged
>/// definitions of that entity.
> -  llvm::DenseMap
> MergedDefModules;
> +  llvm::DenseMap>
> MergedDefModules;
>
>/// \brief Initializers for a module, in order. Each Decl will be either
>/// something that has a semantic effect on startup (such as a variable
> with
> @@ -902,7 +894,6 @@ public:
>/// and should be visible whenever \p M is visible.
>void mergeDefinitionIntoModule(NamedDecl *ND, Module *M,
>   bool NotifyListeners = true);
> -  void mergeDefinitionIntoModulesOf(NamedDecl *ND, NamedDecl *Other);
>/// \brief Clean up the merged definition list. Call this if you might
> have
>/// added duplicates into the list.
>void deduplicateMergedDefinitonsFor(NamedDecl *ND);
> @@ -913,9 +904,7 @@ public:
>  auto MergedIt = MergedDefModules.find(Def);
>  if (MergedIt == MergedDefModules.end())
>return None;
> -if (auto *CanonDef = MergedIt->second.CanonicalDef)
> -  return getModulesWithMergedDefinition(CanonDef);
> -return MergedIt->second.MergedModules;
> +return MergedIt->second;
>}
>
>/// Add a declaration to the list of declarations that are initialized
>
> Modified: cfe/trunk/lib/AST/ASTContext.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=281452&r1=281451&r2=281452&view=diff
>
> ==
> --- cfe/trunk/lib/AST/ASTContext.cpp (original)
> +++ cfe/trunk/lib/AST/ASTContext.cpp Wed Sep 14 05:05:10 2016
> @@ -890,59 +890,10 @@ void ASTContext::mergeDefinitionIntoModu
>  if (auto *Listener = getASTMutationListener())
>Listener->RedefinedHiddenDefinition(ND, M);
>
> -  if (getLangOpts().ModulesLocalVisibility) {
> -auto *Merged = &MergedDefModules[ND];
> -if (auto *CanonDef = Merged->CanonicalDef)
> -  Merged = &MergedDefModules[CanonDef];
> -Merged->MergedModules.push_back(M);
> -  } else {
> -auto MergedIt = MergedDefModules.find(ND);
> -if (MergedIt != MergedDefModules.end() &&
> MergedIt->second.CanonicalDef)
> -  ND = MergedIt->second.CanonicalDef;
> +  if (getLangOpts().ModulesLocalVisibility)
> +MergedDefModules[ND].push_back(M);
> +  else
>  ND->setHidden(false);
> -  }
> -}
> -
> -void ASTContext::mergeDefinitionIntoModulesOf(NamedDecl *Def,
> -  NamedDecl *Other) {
> -  // We need to know the owning module of the merge source.
> -  assert(Other->isFromASTFile() && "merge of non-imported decl not
> supported");
> -  assert(Def != Other && "merging definition into itself");
> -
> -  if (!getLangOpts().ModulesLocalVisibility && !Other->isHidden()) {
> -Def->setHidden(false);
> -return;
> -  }
> -  assert(Other->getImportedOw

[PATCH] D24561: [clang-tidy] Add dependency on clangAnalysis to clangTidyMiscModule

[Martin Böhme via cfe-commits]

mboehme created this revision.
mboehme added a subscriber: cfe-commits.
Herald added subscribers: mgorny, beanz.

This is needed for the recently submitted misc-use-after-move check (rL281453).
For some reason, this still built under Linux, but it caused the PPC build bot
to fail.

https://reviews.llvm.org/D24561

Files:
  clang-tidy/misc/CMakeLists.txt

Index: clang-tidy/misc/CMakeLists.txt
===
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -47,6 +47,7 @@
   VirtualNearMissCheck.cpp
 
   LINK_LIBS
+  clangAnalysis
   clangAST
   clangASTMatchers
   clangBasic


Index: clang-tidy/misc/CMakeLists.txt
===
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -47,6 +47,7 @@
   VirtualNearMissCheck.cpp
 
   LINK_LIBS
+  clangAnalysis
   clangAST
   clangASTMatchers
   clangBasic
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24561: [clang-tidy] Add dependency on clangAnalysis to clangTidyMiscModule

[Martin Böhme via cfe-commits]

mboehme added a comment.

Trivial build fix, will submit without review.


https://reviews.llvm.org/D24561



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

Re: [PATCH] D24561: [clang-tidy] Add dependency on clangAnalysis to clangTidyMiscModule

[Aaron Ballman via cfe-commits]

aaron.ballman added a subscriber: aaron.ballman.
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 -- for these sort of post-commit fixes that are trivial and make failing 
bots happy, you can go ahead and commit them and handle further review once the 
bots go back to green.


https://reviews.llvm.org/D24561



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

Re: [PATCH] D24561: [clang-tidy] Add dependency on clangAnalysis to clangTidyMiscModule

[Martin Böhme via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL281460: [clang-tidy] Add dependency on clangAnalysis to 
clangTidyMiscModule (authored by mboehme).

Changed prior to commit:
  https://reviews.llvm.org/D24561?vs=71345&id=71346#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24561

Files:
  clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt

Index: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
===
--- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
+++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
@@ -47,6 +47,7 @@
   VirtualNearMissCheck.cpp
 
   LINK_LIBS
+  clangAnalysis
   clangAST
   clangASTMatchers
   clangBasic


Index: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
===
--- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
+++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
@@ -47,6 +47,7 @@
   VirtualNearMissCheck.cpp
 
   LINK_LIBS
+  clangAnalysis
   clangAST
   clangASTMatchers
   clangBasic
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r281460 - [clang-tidy] Add dependency on clangAnalysis to clangTidyMiscModule

[Martin Bohme via cfe-commits]

Author: mboehme
Date: Wed Sep 14 08:33:11 2016
New Revision: 281460

URL: http://llvm.org/viewvc/llvm-project?rev=281460&view=rev
Log:
[clang-tidy] Add dependency on clangAnalysis to clangTidyMiscModule

Summary:
This is needed for the recently submitted misc-use-after-move check (rL281453).
For some reason, this still built under Linux, but it caused the PPC build bot
to fail.

Subscribers: beanz, cfe-commits, mgorny

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

Modified:
clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt

Modified: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt?rev=281460&r1=281459&r2=281460&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt Wed Sep 14 08:33:11 
2016
@@ -47,6 +47,7 @@ add_clang_library(clangTidyMiscModule
   VirtualNearMissCheck.cpp
 
   LINK_LIBS
+  clangAnalysis
   clangAST
   clangASTMatchers
   clangBasic


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

Re: [PATCH] D24005: [compiler-rt cmake] Support overriding llvm-config query results

[Michał Górny via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL281461: [cmake] Support overriding llvm-config query results 
(authored by mgorny).

Changed prior to commit:
  https://reviews.llvm.org/D24005?vs=71040&id=71348#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24005

Files:
  compiler-rt/trunk/cmake/Modules/CompilerRTUtils.cmake

Index: compiler-rt/trunk/cmake/Modules/CompilerRTUtils.cmake
===
--- compiler-rt/trunk/cmake/Modules/CompilerRTUtils.cmake
+++ compiler-rt/trunk/cmake/Modules/CompilerRTUtils.cmake
@@ -197,10 +197,15 @@
 message(FATAL_ERROR "llvm-config failed with status ${HAD_ERROR}")
   endif()
   string(REGEX REPLACE "[ \t]*[\r\n]+[ \t]*" ";" CONFIG_OUTPUT 
${CONFIG_OUTPUT})
-  list(GET CONFIG_OUTPUT 0 LLVM_BINARY_DIR)
-  list(GET CONFIG_OUTPUT 1 LLVM_TOOLS_BINARY_DIR)
-  list(GET CONFIG_OUTPUT 2 LLVM_LIBRARY_DIR)
-  list(GET CONFIG_OUTPUT 3 LLVM_MAIN_SRC_DIR)
+  list(GET CONFIG_OUTPUT 0 BINARY_DIR)
+  list(GET CONFIG_OUTPUT 1 TOOLS_BINARY_DIR)
+  list(GET CONFIG_OUTPUT 2 LIBRARY_DIR)
+  list(GET CONFIG_OUTPUT 3 MAIN_SRC_DIR)
+
+  set(LLVM_BINARY_DIR ${BINARY_DIR} CACHE PATH "Path to LLVM build tree")
+  set(LLVM_TOOLS_BINARY_DIR ${TOOLS_BINARY_DIR} CACHE PATH "Path to llvm/bin")
+  set(LLVM_LIBRARY_DIR ${LIBRARY_DIR} CACHE PATH "Path to llvm/lib")
+  set(LLVM_MAIN_SRC_DIR ${MAIN_SRC_DIR} CACHE PATH "Path to LLVM source tree")
 
   # Make use of LLVM CMake modules.
   file(TO_CMAKE_PATH ${LLVM_BINARY_DIR} LLVM_BINARY_DIR_CMAKE_STYLE)


Index: compiler-rt/trunk/cmake/Modules/CompilerRTUtils.cmake
===
--- compiler-rt/trunk/cmake/Modules/CompilerRTUtils.cmake
+++ compiler-rt/trunk/cmake/Modules/CompilerRTUtils.cmake
@@ -197,10 +197,15 @@
 message(FATAL_ERROR "llvm-config failed with status ${HAD_ERROR}")
   endif()
   string(REGEX REPLACE "[ \t]*[\r\n]+[ \t]*" ";" CONFIG_OUTPUT ${CONFIG_OUTPUT})
-  list(GET CONFIG_OUTPUT 0 LLVM_BINARY_DIR)
-  list(GET CONFIG_OUTPUT 1 LLVM_TOOLS_BINARY_DIR)
-  list(GET CONFIG_OUTPUT 2 LLVM_LIBRARY_DIR)
-  list(GET CONFIG_OUTPUT 3 LLVM_MAIN_SRC_DIR)
+  list(GET CONFIG_OUTPUT 0 BINARY_DIR)
+  list(GET CONFIG_OUTPUT 1 TOOLS_BINARY_DIR)
+  list(GET CONFIG_OUTPUT 2 LIBRARY_DIR)
+  list(GET CONFIG_OUTPUT 3 MAIN_SRC_DIR)
+
+  set(LLVM_BINARY_DIR ${BINARY_DIR} CACHE PATH "Path to LLVM build tree")
+  set(LLVM_TOOLS_BINARY_DIR ${TOOLS_BINARY_DIR} CACHE PATH "Path to llvm/bin")
+  set(LLVM_LIBRARY_DIR ${LIBRARY_DIR} CACHE PATH "Path to llvm/lib")
+  set(LLVM_MAIN_SRC_DIR ${MAIN_SRC_DIR} CACHE PATH "Path to LLVM source tree")
 
   # Make use of LLVM CMake modules.
   file(TO_CMAKE_PATH ${LLVM_BINARY_DIR} LLVM_BINARY_DIR_CMAKE_STYLE)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24561: [clang-tidy] Add dependency on clangAnalysis to clangTidyMiscModule

[Martin Böhme via cfe-commits]

mboehme added a comment.

Thanks!


Repository:
  rL LLVM

https://reviews.llvm.org/D24561



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

Re: [PATCH] D24005: [compiler-rt cmake] Support overriding llvm-config query results

[Michał Górny via cfe-commits]

mgorny added a comment.

Thanks again. I've left them as-is and committed.


Repository:
  rL LLVM

https://reviews.llvm.org/D24005



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

[PATCH] D24562: [libcxx] Recover no-exceptions XFAILs

[Asiri Rathnayake via cfe-commits]

rmaprath created this revision.
rmaprath added reviewers: EricWF, mclow.lists.
rmaprath added a subscriber: cfe-commits.

When we added support for the no-exceptions build of `libc++`, all the tests 
that used exceptions got a blanket XFAIL for the no-exceptions variety.

Previously, we tried to fix-up these tests with an elaborate setjmp/longjmp 
hackery (see D14653), which was rejected on the grounds of being too 
complicated / obtrusive.

The current patch is a representative fix that does the most obvious thing 
instead: use pre-processor macros to exclude just the bits of the test that use 
exceptions. Unlike in the previous patch, this means we are not trying to 
associate / verify a particular behaviour of the no-exceptions library with 
respect to exceptional situations (like calling abort). Here we simply save the 
parts of the test that has nothing to do with exceptions, I think this is a 
fair compromise.

We could also split the test file into two; one that uses exceptions and one 
that does not, though I can't think of any added benefits of that approach.

If this patch is approved I'll go ahead and update the rest of the tests (~150) 
to follow the same.

https://reviews.llvm.org/D24562

Files:
  test/std/re/re.alg/re.alg.search/grep.pass.cpp

Index: test/std/re/re.alg/re.alg.search/grep.pass.cpp
===
--- test/std/re/re.alg/re.alg.search/grep.pass.cpp
+++ test/std/re/re.alg/re.alg.search/grep.pass.cpp
@@ -7,7 +7,6 @@
 //
 
//===--===//
 
-// XFAIL: libcpp-no-exceptions
 // 
 
 // template 
@@ -23,6 +22,7 @@
 #include "test_macros.h"
 #include "test_iterators.h"
 
+#ifndef TEST_HAS_NO_EXCEPTIONS
 extern "C" void LLVMFuzzerTestOneInput(const char *data)
 {
 size_t size = strlen(data);
@@ -47,6 +47,7 @@
 
LLVMFuzzerTestOneInput(R"XX(Õ)_%()()((\8'_%()_%()_%()_%(()_%()_%()_%(.t;)()¥f()_%()(.)_%;)()!¥f)()XX");
 #endif
 }
+#endif
 
 int main()
 {
@@ -82,5 +83,7 @@
 assert(m.position(0) == 0);
 assert(m.str(0) == "");
 }
+#ifndef TEST_HAS_NO_EXCEPTIONS
 fuzz_tests();
+#endif
 }


Index: test/std/re/re.alg/re.alg.search/grep.pass.cpp
===
--- test/std/re/re.alg/re.alg.search/grep.pass.cpp
+++ test/std/re/re.alg/re.alg.search/grep.pass.cpp
@@ -7,7 +7,6 @@
 //
 //===--===//
 
-// XFAIL: libcpp-no-exceptions
 // 
 
 // template 
@@ -23,6 +22,7 @@
 #include "test_macros.h"
 #include "test_iterators.h"
 
+#ifndef TEST_HAS_NO_EXCEPTIONS
 extern "C" void LLVMFuzzerTestOneInput(const char *data)
 {
 size_t size = strlen(data);
@@ -47,6 +47,7 @@
 LLVMFuzzerTestOneInput(R"XX(Õ)_%()()((\8'_%()_%()_%()_%(()_%()_%()_%(.t;)()¥f()_%()(.)_%;)()!¥f)()XX");
 #endif
 }
+#endif
 
 int main()
 {
@@ -82,5 +83,7 @@
 assert(m.position(0) == 0);
 assert(m.str(0) == "");
 }
+#ifndef TEST_HAS_NO_EXCEPTIONS
 fuzz_tests();
+#endif
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24562: [libcxx] Recover no-exceptions XFAILs

[Asiri Rathnayake via cfe-commits]

rmaprath updated this revision to Diff 71349.
rmaprath added a comment.

(Added more context)


https://reviews.llvm.org/D24562

Files:
  test/std/re/re.alg/re.alg.search/grep.pass.cpp

Index: test/std/re/re.alg/re.alg.search/grep.pass.cpp
===
--- test/std/re/re.alg/re.alg.search/grep.pass.cpp
+++ test/std/re/re.alg/re.alg.search/grep.pass.cpp
@@ -7,7 +7,6 @@
 //
 
//===--===//
 
-// XFAIL: libcpp-no-exceptions
 // 
 
 // template 
@@ -23,6 +22,7 @@
 #include "test_macros.h"
 #include "test_iterators.h"
 
+#ifndef TEST_HAS_NO_EXCEPTIONS
 extern "C" void LLVMFuzzerTestOneInput(const char *data)
 {
 size_t size = strlen(data);
@@ -47,6 +47,7 @@
 
LLVMFuzzerTestOneInput(R"XX(Õ)_%()()((\8'_%()_%()_%()_%(()_%()_%()_%(.t;)()¥f()_%()(.)_%;)()!¥f)()XX");
 #endif
 }
+#endif
 
 int main()
 {
@@ -82,5 +83,7 @@
 assert(m.position(0) == 0);
 assert(m.str(0) == "");
 }
+#ifndef TEST_HAS_NO_EXCEPTIONS
 fuzz_tests();
+#endif
 }


Index: test/std/re/re.alg/re.alg.search/grep.pass.cpp
===
--- test/std/re/re.alg/re.alg.search/grep.pass.cpp
+++ test/std/re/re.alg/re.alg.search/grep.pass.cpp
@@ -7,7 +7,6 @@
 //
 //===--===//
 
-// XFAIL: libcpp-no-exceptions
 // 
 
 // template 
@@ -23,6 +22,7 @@
 #include "test_macros.h"
 #include "test_iterators.h"
 
+#ifndef TEST_HAS_NO_EXCEPTIONS
 extern "C" void LLVMFuzzerTestOneInput(const char *data)
 {
 size_t size = strlen(data);
@@ -47,6 +47,7 @@
 LLVMFuzzerTestOneInput(R"XX(Õ)_%()()((\8'_%()_%()_%()_%(()_%()_%()_%(.t;)()¥f()_%()(.)_%;)()!¥f)()XX");
 #endif
 }
+#endif
 
 int main()
 {
@@ -82,5 +83,7 @@
 assert(m.position(0) == 0);
 assert(m.str(0) == "");
 }
+#ifndef TEST_HAS_NO_EXCEPTIONS
 fuzz_tests();
+#endif
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r281469 - Fix documentation of MemberExpr::getMemberDecl

[Stephan Bergmann via cfe-commits]

Author: sberg
Date: Wed Sep 14 09:03:50 2016
New Revision: 281469

URL: http://llvm.org/viewvc/llvm-project?rev=281469&view=rev
Log:
Fix documentation of MemberExpr::getMemberDecl

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

Modified:
cfe/trunk/include/clang/AST/Expr.h

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=281469&r1=281468&r2=281469&view=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Wed Sep 14 09:03:50 2016
@@ -2406,8 +2406,8 @@ public:
 
   /// \brief Retrieve the member declaration to which this expression refers.
   ///
-  /// The returned declaration will either be a FieldDecl or (in C++)
-  /// a CXXMethodDecl.
+  /// The returned declaration will be a FieldDecl or (in C++) a VarDecl (for
+  /// static data members), a CXXMethodDecl, or an EnumConstantDecl.
   ValueDecl *getMemberDecl() const { return MemberDecl; }
   void setMemberDecl(ValueDecl *D) { MemberDecl = D; }
 


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

Re: [PATCH] D23907: Fix documentation of MemberExpr::getMemberDecl

[Stephan Bergmann via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL281469: Fix documentation of MemberExpr::getMemberDecl 
(authored by sberg).

Changed prior to commit:
  https://reviews.llvm.org/D23907?vs=69317&id=71357#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D23907

Files:
  cfe/trunk/include/clang/AST/Expr.h

Index: cfe/trunk/include/clang/AST/Expr.h
===
--- cfe/trunk/include/clang/AST/Expr.h
+++ cfe/trunk/include/clang/AST/Expr.h
@@ -2406,8 +2406,8 @@
 
   /// \brief Retrieve the member declaration to which this expression refers.
   ///
-  /// The returned declaration will either be a FieldDecl or (in C++)
-  /// a CXXMethodDecl.
+  /// The returned declaration will be a FieldDecl or (in C++) a VarDecl (for
+  /// static data members), a CXXMethodDecl, or an EnumConstantDecl.
   ValueDecl *getMemberDecl() const { return MemberDecl; }
   void setMemberDecl(ValueDecl *D) { MemberDecl = D; }
 


Index: cfe/trunk/include/clang/AST/Expr.h
===
--- cfe/trunk/include/clang/AST/Expr.h
+++ cfe/trunk/include/clang/AST/Expr.h
@@ -2406,8 +2406,8 @@
 
   /// \brief Retrieve the member declaration to which this expression refers.
   ///
-  /// The returned declaration will either be a FieldDecl or (in C++)
-  /// a CXXMethodDecl.
+  /// The returned declaration will be a FieldDecl or (in C++) a VarDecl (for
+  /// static data members), a CXXMethodDecl, or an EnumConstantDecl.
   ValueDecl *getMemberDecl() const { return MemberDecl; }
   void setMemberDecl(ValueDecl *D) { MemberDecl = D; }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r281475 - [libcxx] Enable building and testing of libcxx with ThreadSanitizer on OS X

[Kuba Brecka via cfe-commits]

Author: kuba.brecka
Date: Wed Sep 14 09:12:50 2016
New Revision: 281475

URL: http://llvm.org/viewvc/llvm-project?rev=281475&view=rev
Log:
[libcxx] Enable building and testing of libcxx with ThreadSanitizer on OS X

This patch enables building and testing libcxx under ThreadSanitizer on OS X. 
CMake builds that have -DLLVM_USE_SANITIZER=Thread will automatically build 
libcxx with -fsanitize=thread and testing via lit then runs under TSan.

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


Modified:
libcxx/trunk/lib/CMakeLists.txt

Modified: libcxx/trunk/lib/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/lib/CMakeLists.txt?rev=281475&r1=281474&r2=281475&view=diff
==
--- libcxx/trunk/lib/CMakeLists.txt (original)
+++ libcxx/trunk/lib/CMakeLists.txt Wed Sep 14 09:12:50 2016
@@ -42,6 +42,8 @@ if (APPLE AND LLVM_USE_SANITIZER)
 set(LIBFILE "libclang_rt.asan_osx_dynamic.dylib")
   elseif("${LLVM_USE_SANITIZER}" STREQUAL "Undefined")
 set(LIBFILE "libclang_rt.ubsan_osx_dynamic.dylib")
+  elseif("${LLVM_USE_SANITIZER}" STREQUAL "Thread")
+set(LIBFILE "libclang_rt.tsan_osx_dynamic.dylib")
   else()
 message(WARNING "LLVM_USE_SANITIZER=${LLVM_USE_SANITIZER} is not supported 
on OS X")
   endif()


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

[libcxx] r281476 - [libcxx] Fix a typo in test/libcxx/test/target_info.py that prevents running tests on Darwin with sanitizers

[Kuba Brecka via cfe-commits]

Author: kuba.brecka
Date: Wed Sep 14 09:13:50 2016
New Revision: 281476

URL: http://llvm.org/viewvc/llvm-project?rev=281476&view=rev
Log:
[libcxx] Fix a typo in test/libcxx/test/target_info.py that prevents running 
tests on Darwin with sanitizers

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


Modified:
libcxx/trunk/test/libcxx/test/target_info.py

Modified: libcxx/trunk/test/libcxx/test/target_info.py
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/libcxx/test/target_info.py?rev=281476&r1=281475&r2=281476&view=diff
==
--- libcxx/trunk/test/libcxx/test/target_info.py (original)
+++ libcxx/trunk/test/libcxx/test/target_info.py Wed Sep 14 09:13:50 2016
@@ -115,7 +115,7 @@ class DarwinLocalTI(DefaultTargetInfo):
 return False
 
 def add_sanitizer_features(self, sanitizer_type, features):
-if san == 'Undefined':
+if sanitizer_type == 'Undefined':
 features.add('sanitizer-new-delete')
 
 


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

[libcxx] r281477 - [libcxx] Add a TSan regression test for a data race in call_once

[Kuba Brecka via cfe-commits]

Author: kuba.brecka
Date: Wed Sep 14 09:15:42 2016
New Revision: 281477

URL: http://llvm.org/viewvc/llvm-project?rev=281477&view=rev
Log:
[libcxx] Add a TSan regression test for a data race in call_once

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


Added:

libcxx/trunk/test/std/thread/thread.mutex/thread.once/thread.once.callonce/race.pass.cpp

Added: 
libcxx/trunk/test/std/thread/thread.mutex/thread.once/thread.once.callonce/race.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/thread/thread.mutex/thread.once/thread.once.callonce/race.pass.cpp?rev=281477&view=auto
==
--- 
libcxx/trunk/test/std/thread/thread.mutex/thread.once/thread.once.callonce/race.pass.cpp
 (added)
+++ 
libcxx/trunk/test/std/thread/thread.mutex/thread.once/thread.once.callonce/race.pass.cpp
 Wed Sep 14 09:15:42 2016
@@ -0,0 +1,48 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// UNSUPPORTED: libcpp-has-no-threads
+
+// 
+
+// struct once_flag;
+
+// template
+//   void call_once(once_flag& flag, Callable&& func, Args&&... args);
+
+// This test is supposed to be run with ThreadSanitizer and verifies that
+// call_once properly synchronizes user state, a data race that was fixed
+// in r280621.
+
+#include 
+#include 
+#include 
+
+std::once_flag flg0;
+long global = 0;
+
+void init0()
+{
+++global;
+}
+
+void f0()
+{
+std::call_once(flg0, init0);
+assert(global == 1);
+}
+
+int main()
+{
+std::thread t0(f0);
+std::thread t1(f0);
+t0.join();
+t1.join();
+assert(global == 1);
+}


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

r281487 - CodeGen: simplify the logic a slight bit

[Saleem Abdulrasool via cfe-commits]

Author: compnerd
Date: Wed Sep 14 10:17:46 2016
New Revision: 281487

URL: http://llvm.org/viewvc/llvm-project?rev=281487&view=rev
Log:
CodeGen: simplify the logic a slight bit

Move the definition of `getTriple()` into the header.  It would just call
`getTarget().getTriple()`.  Inline the definition to allow the compiler to see
the same amount of the layout as previously.  Remove the more verbose
`getTarget().getTriple()` in favour of `getTriple()`.

Modified:
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.h
cfe/trunk/lib/CodeGen/TargetInfo.cpp

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=281487&r1=281486&r2=281487&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Wed Sep 14 10:17:46 2016
@@ -189,8 +189,7 @@ void CodeGenModule::createOpenCLRuntime(
 void CodeGenModule::createOpenMPRuntime() {
   // Select a specialized code generation class based on the target, if any.
   // If it does not exist use the default implementation.
-  switch (getTarget().getTriple().getArch()) {
-
+  switch (getTriple().getArch()) {
   case llvm::Triple::nvptx:
   case llvm::Triple::nvptx64:
 assert(getLangOpts().OpenMPIsDevice &&
@@ -471,7 +470,7 @@ void CodeGenModule::Release() {
 getModule().addModuleFlag(llvm::Module::Override, "Cross-DSO CFI", 1);
   }
 
-  if (LangOpts.CUDAIsDevice && getTarget().getTriple().isNVPTX()) {
+  if (LangOpts.CUDAIsDevice && getTriple().isNVPTX()) {
 // Indicate whether __nvvm_reflect should be configured to flush denormal
 // floating point values to 0.  (This corresponds to its "__CUDA_FTZ"
 // property.)
@@ -1059,8 +1058,7 @@ void CodeGenModule::SetFunctionAttribute
   // where substantial code, including the libstdc++ dylib, was compiled with
   // GCC and does not actually return "this".
   if (!IsThunk && getCXXABI().HasThisReturn(GD) &&
-  !(getTarget().getTriple().isiOS() &&
-getTarget().getTriple().isOSVersionLT(6))) {
+  !(getTriple().isiOS() && getTriple().isOSVersionLT(6))) {
 assert(!F->arg_empty() &&
F->arg_begin()->getType()
  ->canLosslesslyBitCastTo(F->getReturnType()) &&
@@ -2201,7 +2199,7 @@ CodeGenModule::GetOrCreateLLVMGlobal(Str
 }
 
 // Handle XCore specific ABI requirements.
-if (getTarget().getTriple().getArch() == llvm::Triple::xcore &&
+if (getTriple().getArch() == llvm::Triple::xcore &&
 D->getLanguageLinkage() == CLanguageLinkage &&
 D->getType().isConstant(Context) &&
 isExternallyVisible(D->getLinkageAndVisibility().getLinkage()))
@@ -3187,7 +3185,7 @@ CodeGenModule::GetAddrOfConstantCFString
 llvm::Constant *GV =
 CreateRuntimeVariable(Ty, "__CFConstantStringClassReference");
 
-if (getTarget().getTriple().isOSBinFormatCOFF()) {
+if (getTriple().isOSBinFormatCOFF()) {
   IdentifierInfo &II = getContext().Idents.get(GV->getName());
   TranslationUnitDecl *TUDecl = getContext().getTranslationUnitDecl();
   DeclContext *DC = TranslationUnitDecl::castToDeclContext(TUDecl);
@@ -3253,7 +3251,7 @@ CodeGenModule::GetAddrOfConstantCFString
   // FIXME: We set the section explicitly to avoid a bug in ld64 224.1.
   // Without it LLVM can merge the string with a non unnamed_addr one during
   // LTO.  Doing that changes the section it ends in, which surprises ld64.
-  if (getTarget().getTriple().isOSBinFormatMachO())
+  if (getTriple().isOSBinFormatMachO())
 GV->setSection(isUTF16 ? "__TEXT,__ustring"
: "__TEXT,__cstring,cstring_literals");
 
@@ -3277,7 +3275,7 @@ CodeGenModule::GetAddrOfConstantCFString
 llvm::GlobalVariable::PrivateLinkage, C,
 "_unnamed_cfstring_");
   GV->setAlignment(Alignment.getQuantity());
-  switch (getTarget().getTriple().getObjectFormat()) {
+  switch (getTriple().getObjectFormat()) {
   case llvm::Triple::UnknownObjectFormat:
 llvm_unreachable("unknown file format");
   case llvm::Triple::COFF:

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.h?rev=281487&r1=281486&r2=281487&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenModule.h (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.h Wed Sep 14 10:17:46 2016
@@ -614,7 +614,7 @@ public:
 return TheModule.getDataLayout();
   }
   const TargetInfo &getTarget() const { return Target; }
-  const llvm::Triple &getTriple() const;
+  const llvm::Triple &getTriple() const { return Target.getTriple(); }
   bool supportsCOMDAT() const;
   void maybeSetTrivialComdat(const Decl &D, llvm::GlobalObject &GO);
 

Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
UR

Re: [PATCH] D24245: [ARM] ARM-specific attributes should be accepted for big-endian

[Diana Picus via cfe-commits]

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

Looks like a sensible thing to do.


Repository:
  rL LLVM

https://reviews.llvm.org/D24245



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

[PATCH] D24567: [clang-rename] Merge rename-{at|all} & optimise.

[Kirill Bobyrev via cfe-commits]

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

Having both rename-at and rename-all both seems confusing and introduces 
unneeded difficulties. After merging rename-at and rename-all maintaining main 
function wrappers and custom help becomes redundant while CLI becomes less 
confusing.

D24224 (which was the original patch causing buildbot failures) wasn't aware of 
bugs caused by passing both `-offset` and `-qualified-name`.

D24224 was reverted and this patch previous diff to disallow both options 
present within single invocation.

Old and unmaintaned clang-rename unittest is deleted.

https://reviews.llvm.org/D24567

Files:
  clang-rename/USRFindingAction.cpp
  clang-rename/USRFindingAction.h
  clang-rename/tool/ClangRename.cpp
  docs/clang-rename.rst
  test/clang-rename/ClassFindByName.cpp
  test/clang-rename/ClassTestMulti.cpp
  test/clang-rename/ClassTestMultiByName.cpp
  test/clang-rename/ClassTestMultiByNameYAML.cpp
  test/clang-rename/FunctionWithClassFindByName.cpp
  test/clang-rename/Inputs/ClassTestMultiByNameYAMLRenameAll.yaml
  test/clang-rename/Inputs/ClassTestMultiByNameYAMLRenameAt.yaml
  test/clang-rename/Inputs/OffsetToNewName.yaml
  test/clang-rename/Inputs/QualifiedNameToNewName.yaml
  test/clang-rename/InvalidOldName.cpp
  test/clang-rename/NoNewName.cpp
  test/clang-rename/YAMLInput.cpp

Index: test/clang-rename/YAMLInput.cpp
===
--- /dev/null
+++ test/clang-rename/YAMLInput.cpp
@@ -0,0 +1,10 @@
+class Foo1 { // CHECK: class Bar1
+};
+
+class Foo2 { // CHECK: class Bar2
+};
+
+// Test 1.
+// RUN: clang-rename -input %S/Inputs/OffsetToNewName.yaml %s -- | sed 's,//.*,,' | FileCheck %s
+// Test 2.
+// RUN: clang-rename -input %S/Inputs/QualifiedNameToNewName.yaml %s -- | sed 's,//.*,,' | FileCheck %s
Index: test/clang-rename/NoNewName.cpp
===
--- test/clang-rename/NoNewName.cpp
+++ test/clang-rename/NoNewName.cpp
@@ -1,4 +1,4 @@
 // Check for an error while -new-name argument has not been passed to
 // clang-rename.
 // RUN: not clang-rename -offset=133 %s 2>&1 | FileCheck %s
-// CHECK: clang-rename: for the -new-name option: must be specified
+// CHECK: clang-rename: -new-name must be specified.
Index: test/clang-rename/InvalidOldName.cpp
===
--- test/clang-rename/InvalidOldName.cpp
+++ /dev/null
@@ -1,2 +0,0 @@
-// RUN: not clang-rename rename-all -new-name=Foo -old-name=Bar %s -- 2>&1 | FileCheck %s
-// CHECK: clang-rename: could not find symbol Bar.
Index: test/clang-rename/Inputs/QualifiedNameToNewName.yaml
===
--- test/clang-rename/Inputs/QualifiedNameToNewName.yaml
+++ test/clang-rename/Inputs/QualifiedNameToNewName.yaml
@@ -1,6 +1,6 @@
 ---
-- OldName:Foo1
+- QualifiedName:  Foo1
   NewName:Bar1
-- OldName:Foo2
+- QualifiedName:  Foo2
   NewName:Bar2
 ...
Index: test/clang-rename/FunctionWithClassFindByName.cpp
===
--- test/clang-rename/FunctionWithClassFindByName.cpp
+++ test/clang-rename/FunctionWithClassFindByName.cpp
@@ -9,4 +9,4 @@
   return 0;
 }
 
-// RUN: clang-rename rename-all -old-name=Foo -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
+// RUN: clang-rename -qualified-name=Foo -new-name=Bar %s -- | sed 's,//.*,,' | FileCheck %s
Index: test/clang-rename/ClassTestMultiByNameYAML.cpp
===
--- test/clang-rename/ClassTestMultiByNameYAML.cpp
+++ /dev/null
@@ -1,10 +0,0 @@
-class Foo1 { // CHECK: class Bar1
-};
-
-class Foo2 { // CHECK: class Bar2
-};
-
-// Test 1.
-// RUN: clang-rename rename-all -input %S/Inputs/ClassTestMultiByNameYAMLRenameAll.yaml %s -- | sed 's,//.*,,' | FileCheck %s
-// Test 2.
-// RUN: clang-rename rename-all -input %S/Inputs/ClassTestMultiByNameYAMLRenameAt.yaml %s -- | sed 's,//.*,,' | FileCheck %s
Index: test/clang-rename/ClassTestMultiByName.cpp
===
--- test/clang-rename/ClassTestMultiByName.cpp
+++ test/clang-rename/ClassTestMultiByName.cpp
@@ -5,4 +5,4 @@
 };
 
 // Test 1.
-// RUN: clang-rename rename-all -old-name=Foo1 -new-name=Bar1 -old-name=Foo2 -new-name=Bar2 %s -- | sed 's,//.*,,' | FileCheck %s
+// RUN: clang-rename -qualified-name=Foo1 -new-name=Bar1 -qualified-name=Foo2 -new-name=Bar2 %s -- | sed 's,//.*,,' | FileCheck %s
Index: test/clang-rename/ClassTestMulti.cpp
===
--- test/clang-rename/ClassTestMulti.cpp
+++ test/clang-rename/ClassTestMulti.cpp
@@ -5,7 +5,7 @@
 };
 
 // Test 1.
-// RUN: clang-rename rename-all -offset=6 -new-name=Bar1 -offset=76 -new-name=Bar2 %s -- | sed 's,//.*,,' | FileCheck %s
+// RUN: clang-rename -off

Re: [PATCH] D24562: [libcxx] Recover no-exceptions XFAILs

[Marshall Clow via cfe-commits]

mclow.lists added a comment.

What an awful test.  I wonder who wrote such a steaming pile.  Probably me.


https://reviews.llvm.org/D24562



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

Re: [PATCH] D24562: [libcxx] Recover no-exceptions XFAILs

[Asiri Rathnayake via cfe-commits]

rmaprath added a comment.

In https://reviews.llvm.org/D24562#542630, @mclow.lists wrote:

> What an awful test.  I wonder who wrote such a steaming pile.  Probably me.


I think I've thrown quite a lot of XFAILs like this in the past, just to get 
the no-exceptions build passing. Now I've got to clean them up one by one :)

Are you OK with this way of fixing them?

Cheers,

/ Asiri


https://reviews.llvm.org/D24562



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

Re: [PATCH] D24467: Fix an error after D21678

[Vladimir Yakovlev via cfe-commits]

vbyakovlcl added inline comments.


Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8733
@@ -8731,2 +8732,3 @@
   if (!IsCompAssign) {
-LHS = S.UsualUnaryConversions(LHS.get());
+if (S.LangOpts.OpenCL || S.LangOpts.ZVector)
+  LHS = S.UsualUnaryConversions(LHS.get());

ahatanak wrote:
> It wasn't clear to me why we have to call 
> DefaultFunctionArrayLvalueConversion instead of UsualUnaryConversions. Is it 
> possible to come up with a test case that would fail without this change, and 
> if it is, can you add it to vecshift.c?
I cannot remember why I did this change. Removing it doesn't cause any new 
fails. 


Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8747
@@ -8742,3 +8746,3 @@
 
   // Note that RHS might not be a vector.
   QualType RHSType = RHS.get()->getType();

ahatanak wrote:
> We no longer error out when LHS is not a vector, so it should mention that 
> either the LHS or the RHS might not be a vector.
I added a comment.


Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8787
@@ -8770,1 +8786,3 @@
 }
+if (!S.LangOpts.OpenCL && !S.LangOpts.ZVector) {
+  const BuiltinType *LHSBT = LHSEleType->getAs();

ahatanak wrote:
> Is it possible to split this out to another patch? The first patch would fix 
> handling of (scalar << vector) and the second patch would make clang reject 
> vector shifts if element sizes of the LHS and RHS are different. It's not 
> clear whether it's correct to reject the latter case, so perhaps you can 
> discuss it in a separate patch? 
Ok. I removed this and will fail a new review after committing of this review.


Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8847
@@ -8774,3 +8846,3 @@
   S.Context.getExtVectorType(RHSEleType, LHSVecTy->getNumElements());
 RHS = S.ImpCastExprToType(RHS.get(), VecTy, CK_VectorSplat);
   }

ahatanak wrote:
> When the LHS is a scalar, we check the type of the LHS and the element type 
> of the RHS, and if necessary, cast the LHS to the element type of the RHS. 
> What's the reason we don't do the same here?
No need, because CodeGen inserts a conversion.

define <8 x i16> @foo1(<8 x i16> %n, i32 %m) #0 {
entry:
  %n.addr = alloca <8 x i16>, align 16
  %m.addr = alloca i32, align 4
  store <8 x i16> %n, <8 x i16>* %n.addr, align 16
  store i32 %m, i32* %m.addr, align 4
  %0 = load <8 x i16>, <8 x i16>* %n.addr, align 16
  %1 = load i32, i32* %m.addr, align 4
  %splat.splatinsert = insertelement <8 x i32> undef, i32 %1, i32 0
  %splat.splat = shufflevector <8 x i32> %splat.splatinsert, <8 x i32> undef, 
<8 x i32> zeroinitializer
  %sh_prom = trunc <8 x i32> %splat.splat to <8 x i16>
  %shl = shl <8 x i16> %0, %sh_prom
  ret <8 x i16> %shl

We need insert a conversion of scalar LHS to the RHS element type because we 
must return a vector of the RHS type.


https://reviews.llvm.org/D24467



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

Re: [PATCH] D21506: [analyzer] Block in critical section

[Zoltán Dániel Török via cfe-commits]

zdtorok updated this revision to Diff 71375.
zdtorok added a comment.
Herald added subscribers: mgorny, beanz.

Fixed possible integer-underflow in case of unexpected unlocking function. Also 
added test for this.


Repository:
  rL LLVM

https://reviews.llvm.org/D21506

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  test/Analysis/block-in-critical-section.cpp

Index: test/Analysis/block-in-critical-section.cpp
===
--- /dev/null
+++ test/Analysis/block-in-critical-section.cpp
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.unix.BlockInCriticalSection -std=c++11 -verify %s
+
+void sleep(int x) {}
+
+namespace std {
+struct mutex {
+  void lock() {}
+  void unlock() {}
+};
+}
+
+void testBlockInCriticalSection() {
+  std::mutex m;
+  m.lock();
+  sleep(3); // expected-warning {{A blocking function %s is called inside a critical section}}
+  m.unlock();
+}
+
+void testBlockInCriticalSectionWithNestedMutexes() {
+  std::mutex m, n, k;
+  m.lock();
+  n.lock();
+  k.lock();
+  sleep(3); // expected-warning {{A blocking function %s is called inside a critical section}}
+  k.unlock();
+  sleep(5); // expected-warning {{A blocking function %s is called inside a critical section}}
+  n.unlock();
+  sleep(3); // expected-warning {{A blocking function %s is called inside a critical section}}
+  m.unlock();
+  sleep(3); // no-warning
+}
+
+void f() {
+  sleep(1000); // expected-warning {{A blocking function %s is called inside a critical section}}
+}
+
+void testBlockInCriticalSectionInterProcedural() {
+  std::mutex m;
+  m.lock();
+  f();
+  m.unlock();
+}
+
+void testBlockInCriticalSectionUnexpectedUnlock() {
+  std::mutex m;
+  m.unlock();
+  sleep(1); // no-warning
+  m.lock();
+  sleep(1); // expected-warning {{A blocking function %s is called inside a critical section}}
+}
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
===
--- lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -9,6 +9,7 @@
   ArrayBoundChecker.cpp
   ArrayBoundCheckerV2.cpp
   BasicObjCFoundationChecks.cpp
+  BlockInCriticalSectionChecker.cpp
   BoolAssignmentChecker.cpp
   BuiltinFunctionChecker.cpp
   CStringChecker.cpp
Index: lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
===
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
@@ -0,0 +1,109 @@
+//===-- BlockInCriticalSectionChecker.cpp ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// Defines a checker for blocks in critical sections. This checker should find
+// the calls to blocking functions (for example: sleep, getc, fgets, read,
+// recv etc.) inside a critical section. When sleep(x) is called while a mutex
+// is held, other threades cannot lock the same mutex. This might take some
+// time, leading to bad performance or even deadlock.
+//
+//===--===//
+
+#include "ClangSACheckers.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+
+class BlockInCriticalSectionChecker : public Checker {
+
+  CallDescription LockFn, UnlockFn, SleepFn, GetcFn, FgetsFn, ReadFn, RecvFn;
+
+  std::unique_ptr BlockInCritSectionBugType;
+
+  void reportBlockInCritSection(SymbolRef FileDescSym,
+const CallEvent &call,
+CheckerContext &C) const;
+
+public:
+  BlockInCriticalSectionChecker();
+
+  void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
+
+  /// Process unlock.
+  /// Process lock.
+  /// Process blocking functions (sleep, getc, fgets, read, recv)
+  void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
+
+};
+
+} // end anonymous namespace
+
+REGISTER_TRAIT_WITH_PROGRAMSTATE(MutexCounter, unsigned)
+
+BlockInCriticalSectionChecker::BlockInCriticalSectionChecker()
+: LockFn("lock"), UnlockFn("unlock"), SleepFn("sleep"), GetcFn("getc"),
+  FgetsFn("fgets"), ReadFn("read"), RecvFn("recv") {
+  // Initialize the bug type.
+  BlockInCritSectionBugType.reset(
+  new BugType(this, "Call to blocking function in critical section",
+"Blocking Error"));
+}
+
+void BlockInCriticalSection

Re: [PATCH] D21506: [analyzer] Block in critical section

[Zoltán Dániel Török via cfe-commits]

zdtorok added a comment.

You are right NoQ, thank you for the remark, I've upload a new diff containing 
a fix for the integer underflow case.


Repository:
  rL LLVM

https://reviews.llvm.org/D21506



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

[PATCH] D24571: [CUDA] Disallow overloading destructors.

[Justin Lebar via cfe-commits]

jlebar created this revision.
jlebar added a reviewer: rsmith.
jlebar added subscribers: tra, cfe-commits.

We'd attempted to allow this, but turns out we were doing a very bad
job.  :)

Making this work properly would be a giant change in clang.  For
example, we'd need to make CXXRecordDecl::getDestructor()
context-sensitive, because the destructor you end up with depends on
where you're calling it from.

For now (and hopefully for ever), just disallow overloading of
destructors in CUDA.

https://reviews.llvm.org/D24571

Files:
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CodeGenCUDA/function-overload.cu
  clang/test/SemaCUDA/call-overloaded-destructor.cu
  clang/test/SemaCUDA/function-overload.cu
  clang/test/SemaCUDA/no-destructor-overload.cu

Index: clang/test/SemaCUDA/no-destructor-overload.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/no-destructor-overload.cu
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fcuda-is-device -fsyntax-only -verify %s
+
+#include "Inputs/cuda.h"
+
+// We don't allow destructors to be overloaded.  Making this work would be a
+// giant change to clang, and the use cases seem quite limited.
+
+struct A {
+  ~A() {} // expected-note {{previous declaration is here}}
+  __device__ ~A() {} // expected-error {{destructor cannot be redeclared}}
+};
+
+struct B {
+  __host__ ~B() {} // expected-note {{previous declaration is here}}
+  __host__ __device__ ~B() {} // expected-error {{destructor cannot be redeclared}}
+};
+
+struct C {
+  __host__ __device__ ~C() {} // expected-note {{previous declaration is here}}
+  __host__ ~C() {} // expected-error {{destructor cannot be redeclared}}
+};
+
+struct D {
+  __device__ ~D() {} // expected-note {{previous declaration is here}}
+  __host__ __device__ ~D() {} // expected-error {{destructor cannot be redeclared}}
+};
+
+struct E {
+  __host__ __device__ ~E() {} // expected-note {{previous declaration is here}}
+  __device__ ~E() {} // expected-error {{destructor cannot be redeclared}}
+};
+
Index: clang/test/SemaCUDA/function-overload.cu
===
--- clang/test/SemaCUDA/function-overload.cu
+++ clang/test/SemaCUDA/function-overload.cu
@@ -210,44 +210,11 @@
   __host__ ~d_h() {} // expected-error {{destructor cannot be redeclared}}
 };
 
-// H/D overloading is OK
-struct d_dh {
-  __device__ ~d_dh() {}
-  __host__ ~d_dh() {}
-};
-
 // HD is OK
 struct d_hd {
   __host__ __device__ ~d_hd() {}
 };
 
-// Mixing H/D and HD is not allowed.
-struct d_dhhd {
-  __device__ ~d_dhhd() {}
-  __host__ ~d_dhhd() {} // expected-note {{previous declaration is here}}
-  __host__ __device__ ~d_dhhd() {} // expected-error {{destructor cannot be redeclared}}
-};
-
-struct d_hhd {
-  __host__ ~d_hhd() {} // expected-note {{previous declaration is here}}
-  __host__ __device__ ~d_hhd() {} // expected-error {{destructor cannot be redeclared}}
-};
-
-struct d_hdh {
-  __host__ __device__ ~d_hdh() {} // expected-note {{previous declaration is here}}
-  __host__ ~d_hdh() {} // expected-error {{destructor cannot be redeclared}}
-};
-
-struct d_dhd {
-  __device__ ~d_dhd() {} // expected-note {{previous declaration is here}}
-  __host__ __device__ ~d_dhd() {} // expected-error {{destructor cannot be redeclared}}
-};
-
-struct d_hdd {
-  __host__ __device__ ~d_hdd() {} // expected-note {{previous declaration is here}}
-  __device__ ~d_hdd() {} // expected-error {{destructor cannot be redeclared}}
-};
-
 // Test overloading of member functions
 struct m_h {
   void operator delete(void *ptr); // expected-note {{previous declaration is here}}
Index: clang/test/SemaCUDA/call-overloaded-destructor.cu
===
--- clang/test/SemaCUDA/call-overloaded-destructor.cu
+++ /dev/null
@@ -1,17 +0,0 @@
-// expected-no-diagnostics
-
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only -verify %s
-// RUN: %clang_cc1 -triple nvptx64-nvidia-cuda -fsyntax-only -fcuda-is-device -verify %s
-
-#include "Inputs/cuda.h"
-
-struct S {
-  __host__ ~S() {}
-  __device__ ~S() {}
-};
-
-__host__ __device__ void test() {
-  S s;
-  // This should not crash clang.
-  s.~S();
-}
Index: clang/test/CodeGenCUDA/function-overload.cu
===
--- clang/test/CodeGenCUDA/function-overload.cu
+++ clang/test/CodeGenCUDA/function-overload.cu
@@ -16,8 +16,6 @@
 struct s_cd_dh {
   __host__ s_cd_dh() { x = 11; }
   __device__ s_cd_dh() { x = 12; }
-  __host__ ~s_cd_dh() { x = 21; }
-  __device__ ~s_cd_dh() { x = 22; }
 };
 
 struct s_cd_hd {
@@ -38,7 +36,6 @@
   // CHECK-BOTH: call void @_ZN7s_cd_hdC1Ev
 
   // CHECK-BOTH: call void @_ZN7s_cd_hdD1Ev(
-  // CHECK-BOTH: call void @_ZN7s_cd_dhD1Ev(
 }
 // CHECK-BOTH: ret void
 
@@ -56,8 +53,3 @@
 // CHECK-BOTH: define linkonce_odr void @_ZN7s_cd_hdD2Ev(
 // CHECK-BOTH: store i

[PATCH] D24572: [clang-tidy] Clean up code after applying replacements.

[Alexander Kornienko via cfe-commits]

alexfh created this revision.
alexfh added reviewers: ioeric, hokein.
alexfh added a subscriber: cfe-commits.

Remove empty namespaces and initializer list commas / colons in
affected ranges. A strawman patch: proper options for enabling the cleanup and
specifying the format style are needed.

https://reviews.llvm.org/D24572

Files:
  clang-tidy/ClangTidy.cpp
  clang-tidy/ClangTidyDiagnosticConsumer.cpp
  test/clang-tidy/clean-up-code.cpp
  test/clang-tidy/fix.cpp
  test/clang-tidy/google-readability-namespace-comments.cpp

Index: test/clang-tidy/google-readability-namespace-comments.cpp
===
--- test/clang-tidy/google-readability-namespace-comments.cpp
+++ test/clang-tidy/google-readability-namespace-comments.cpp
@@ -4,7 +4,7 @@
 namespace n2 {
 
 
-
+void f(); // So that the namespace isn't empty.
 
 
 // CHECK-MESSAGES: :[[@LINE+4]]:2: warning: namespace 'n2' not terminated with a closing comment [google-readability-namespace-comments]
Index: test/clang-tidy/fix.cpp
===
--- test/clang-tidy/fix.cpp
+++ test/clang-tidy/fix.cpp
@@ -5,6 +5,7 @@
 // RUN: FileCheck -input-file=%t.yaml -check-prefix=CHECK-YAML %s
 
 namespace i {
+void f(); // So that the namespace isn't empty.
 }
 // CHECK: } // namespace i
 // CHECK-MESSAGES: note: FIX-IT applied suggested code changes
Index: test/clang-tidy/clean-up-code.cpp
===
--- /dev/null
+++ test/clang-tidy/clean-up-code.cpp
@@ -0,0 +1,12 @@
+// RUN: %check_clang_tidy %s misc-unused-using-decls %t
+namespace a { class A {}; }
+namespace b {
+using a::A;
+}
+namespace c {}
+// CHECK-MESSAGES: :[[@LINE-3]]:10: warning: using decl 'A' is unused [misc-unused-using-decls]
+// CHECK-FIXES: {{^namespace a { class A {}; }$}}
+// CHECK-FIXES-NOT: namespace
+// CHECK-FIXES: {{^namespace c {}$}}
+// FIXME: cleanupAroundReplacements leaves whitespace. Otherwise we could just
+// check the next line.
Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -79,12 +79,13 @@
 
   tooling::Replacement Replacement(SM, Range, FixIt.CodeToInsert);
   llvm::Error Err = Error.Fix[Replacement.getFilePath()].add(Replacement);
-  // FIXME: better error handling.
+  // FIXME: better error handling (at least, don't let other replacements be
+  // applied).
   if (Err) {
 llvm::errs() << "Fix conflicts with existing fix! "
 << llvm::toString(std::move(Err)) << "\n";
+assert(false && "Fix conflicts with existing fix!");
   }
-  assert(!Err && "Fix conflicts with existing fix!");
 }
   }
 
Index: clang-tidy/ClangTidy.cpp
===
--- clang-tidy/ClangTidy.cpp
+++ clang-tidy/ClangTidy.cpp
@@ -22,6 +22,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Format/Format.h"
 #include "clang/Frontend/ASTConsumers.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendActions.h"
@@ -101,9 +102,8 @@
 DiagPrinter(new TextDiagnosticPrinter(llvm::outs(), &*DiagOpts)),
 Diags(IntrusiveRefCntPtr(new DiagnosticIDs), &*DiagOpts,
   DiagPrinter),
-SourceMgr(Diags, Files), Rewrite(SourceMgr, LangOpts),
-ApplyFixes(ApplyFixes), TotalFixes(0), AppliedFixes(0),
-WarningsAsErrors(0) {
+SourceMgr(Diags, Files), ApplyFixes(ApplyFixes), TotalFixes(0),
+AppliedFixes(0), WarningsAsErrors(0) {
 DiagOpts->ShowColors = llvm::sys::Process::StandardOutHasColors();
 DiagPrinter->BeginSourceFile(LangOpts);
   }
@@ -127,31 +127,56 @@
   auto Diag = Diags.Report(Loc, Diags.getCustomDiagID(Level, "%0 [%1]"))
   << Message.Message << Name;
   for (const auto &FileAndReplacements : Error.Fix) {
-for (const auto &Replacement : FileAndReplacements.second) {
+for (const auto &Repl : FileAndReplacements.second) {
   // Retrieve the source range for applicable fixes. Macro definitions
   // on the command line have locations in a virtual buffer and don't
   // have valid file paths and are therefore not applicable.
   SourceRange Range;
   SourceLocation FixLoc;
-  if (Replacement.isApplicable()) {
-SmallString<128> FixAbsoluteFilePath = Replacement.getFilePath();
+  ++TotalFixes;
+  bool CanBeApplied = false;
+  if (Repl.isApplicable()) {
+SmallString<128> FixAbsoluteFilePath = Repl.getFilePath();
 Files.makeAbsolutePath(FixAbsoluteFilePath);
-FixLoc = getLocation(FixAbsoluteFilePath, Replacement.getOffset());
+if (Appl

[PATCH] D24573: [CUDA] Do a better job at detecting wrong-side calls.

[Justin Lebar via cfe-commits]

jlebar created this revision.
jlebar added a reviewer: rsmith.
jlebar added subscribers: tra, cfe-commits.

Move CheckCUDACall from ActOnCallExpr and BuildDeclRefExpr to
DiagnoseUseOfDecl.  This lets us catch some edge cases we were missing,
specifically around class operators.

This necessitates a few other changes:

 - Avoid emitting duplicate deferred diags in CheckCUDACall.

   Previously we'd carefully placed our call to CheckCUDACall such that
   it would only ever run once for a particular callsite.  But now this
   isn't the case.

 - Emit deferred diagnostics from a template
   specialization/instantiation's primary template, in addition to from
   the specialization/instantiation itself.  DiagnoseUseOfDecl ends up
   putting the deferred diagnostics on the template, rather than the
   specialization, so we need to check both.

https://reviews.llvm.org/D24573

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/SemaCUDA.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCUDA/Inputs/cuda.h
  clang/test/SemaCUDA/call-host-fn-from-device.cu

Index: clang/test/SemaCUDA/call-host-fn-from-device.cu
===
--- clang/test/SemaCUDA/call-host-fn-from-device.cu
+++ clang/test/SemaCUDA/call-host-fn-from-device.cu
@@ -12,6 +12,9 @@
 // expected-note@-4 {{'host_fn' declared here}}
 // expected-note@-5 {{'host_fn' declared here}}
 // expected-note@-6 {{'host_fn' declared here}}
+// expected-note@-7 {{'host_fn' declared here}}
+
+struct Dummy {};
 
 struct S {
   S() {}
@@ -34,6 +37,15 @@
 
   void h() {}
   // expected-note@-1 {{'h' declared here}}
+
+  void operator+();
+  // expected-note@-1 {{'operator+' declared here}}
+
+  void operator-(const T&) {}
+  // expected-note@-1 {{'operator-' declared here}}
+
+  operator Dummy() { return Dummy(); }
+  // expected-note@-1 {{'operator Dummy' declared here}}
 };
 
 __host__ __device__ void T::hd3() {
@@ -92,3 +104,30 @@
 __host__ __device__ void fn_ptr_template() {
   auto* ptr = &host_fn;  // Not an error because the template isn't instantiated.
 }
+
+__host__ __device__ void unaryOp() {
+  T t;
+  (void) +t; // expected-error {{reference to __host__ function 'operator+' in __host__ __device__ function}}
+}
+
+__host__ __device__ void binaryOp() {
+  T t;
+  (void) (t - t); // expected-error {{reference to __host__ function 'operator-' in __host__ __device__ function}}
+}
+
+__host__ __device__ void implicitConversion() {
+  T t;
+  Dummy d = t; // expected-error {{reference to __host__ function 'operator Dummy' in __host__ __device__ function}}
+}
+
+template 
+struct TmplStruct {
+  template  __host__ __device__ void fn() {}
+};
+
+template <>
+template <>
+__host__ __device__ void TmplStruct::fn() { host_fn(); }
+// expected-error@-1 {{reference to __host__ function 'host_fn' in __host__ __device__ function}}
+
+__device__ void double_specialization() { TmplStruct().fn(); }
Index: clang/test/SemaCUDA/Inputs/cuda.h
===
--- clang/test/SemaCUDA/Inputs/cuda.h
+++ clang/test/SemaCUDA/Inputs/cuda.h
@@ -22,7 +22,9 @@
 int cudaConfigureCall(dim3 gridSize, dim3 blockSize, size_t sharedSize = 0,
   cudaStream_t stream = 0);
 
-// Device-side placement new overloads.
+// Host- and device-side placement new overloads.
+void *operator new(__SIZE_TYPE__, void *p) { return p; }
+void *operator new[](__SIZE_TYPE__, void *p) { return p; }
 __device__ void *operator new(__SIZE_TYPE__, void *p) { return p; }
 __device__ void *operator new[](__SIZE_TYPE__, void *p) { return p; }
 
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -374,6 +374,9 @@
 if (getLangOpts().CPlusPlus14 && FD->getReturnType()->isUndeducedType() &&
 DeduceReturnType(FD, Loc))
   return true;
+
+if (getLangOpts().CUDA && !CheckCUDACall(Loc, FD))
+  return true;
   }
 
   // [OpenMP 4.0], 2.15 declare reduction Directive, Restrictions
@@ -1743,11 +1746,6 @@
const DeclarationNameInfo &NameInfo,
const CXXScopeSpec *SS, NamedDecl *FoundD,
const TemplateArgumentListInfo *TemplateArgs) {
-  if (getLangOpts().CUDA)
-if (FunctionDecl *Callee = dyn_cast(D))
-  if (!CheckCUDACall(NameInfo.getLoc(), Callee))
-return ExprError();
-
   bool RefersToCapturedVariable =
   isa(D) &&
   NeedToCaptureVariable(cast(D), NameInfo.getLoc());
@@ -5140,35 +5138,36 @@
   return Callee->getMinRequiredArguments() <= NumArgs;
 }
 
-static ExprResult ActOnCallExprImpl(Sema &S, Scope *Scope, Expr *Fn,
-SourceLocation LParenLoc,
-MultiExprArg ArgExprs,
-SourceLocation RParenLoc, Expr *ExecConfig,
-   

Re: [PATCH] D24513: [AMDGPU] Expose flat work group size, register and wave control attributes

[Konstantin Zhuravlyov via cfe-commits]

kzhuravl updated this revision to Diff 71382.
kzhuravl added a comment.

Fix minor typos


https://reviews.llvm.org/D24513

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/CodeGen/TargetInfo.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGenOpenCL/amdgpu-attrs.cl
  test/CodeGenOpenCL/amdgpu-num-gpr-attr.cl
  test/SemaCUDA/amdgpu-attrs.cu
  test/SemaCUDA/amdgpu-num-gpr-attr.cu
  test/SemaOpenCL/amdgpu-attrs.cl
  test/SemaOpenCL/amdgpu-num-register-attrs.cl

Index: test/SemaOpenCL/amdgpu-num-register-attrs.cl
===
--- test/SemaOpenCL/amdgpu-num-register-attrs.cl
+++ test/SemaOpenCL/amdgpu-num-register-attrs.cl
@@ -1,40 +0,0 @@
-// RUN: %clang_cc1 -triple r600-- -verify -fsyntax-only %s
-
-typedef __attribute__((amdgpu_num_vgpr(128))) struct FooStruct { // expected-error {{'amdgpu_num_vgpr' attribute only applies to kernel functions}}
-  int x;
-  float y;
-} FooStruct;
-
-
-__attribute__((amdgpu_num_vgpr("ABC"))) kernel void foo2() {} // expected-error {{'amdgpu_num_vgpr' attribute requires an integer constant}}
-__attribute__((amdgpu_num_sgpr("ABC"))) kernel void foo3() {} // expected-error {{'amdgpu_num_sgpr' attribute requires an integer constant}}
-
-
-__attribute__((amdgpu_num_vgpr(40))) void foo4() {} // expected-error {{'amdgpu_num_vgpr' attribute only applies to kernel functions}}
-__attribute__((amdgpu_num_sgpr(64))) void foo5() {} // expected-error {{'amdgpu_num_sgpr' attribute only applies to kernel functions}}
-
-__attribute__((amdgpu_num_vgpr(40))) kernel void foo7() {}
-__attribute__((amdgpu_num_sgpr(64))) kernel void foo8() {}
-__attribute__((amdgpu_num_vgpr(40), amdgpu_num_sgpr(64))) kernel void foo9() {}
-
-// Check 0 VGPR is accepted.
-__attribute__((amdgpu_num_vgpr(0))) kernel void foo10() {}
-
-// Check 0 SGPR is accepted.
-__attribute__((amdgpu_num_sgpr(0))) kernel void foo11() {}
-
-// Check both 0 SGPR and VGPR is accepted.
-__attribute__((amdgpu_num_vgpr(0), amdgpu_num_sgpr(0))) kernel void foo12() {}
-
-// Too large VGPR value.
-__attribute__((amdgpu_num_vgpr(4294967296))) kernel void foo13() {} // expected-error {{integer constant expression evaluates to value 4294967296 that cannot be represented in a 32-bit unsigned integer type}}
-
-__attribute__((amdgpu_num_sgpr(4294967296))) kernel void foo14() {} // expected-error {{integer constant expression evaluates to value 4294967296 that cannot be represented in a 32-bit unsigned integer type}}
-
-__attribute__((amdgpu_num_sgpr(4294967296), amdgpu_num_vgpr(4294967296))) kernel void foo15() {} // expected-error 2 {{integer constant expression evaluates to value 4294967296 that cannot be represented in a 32-bit unsigned integer type}}
-
-
-// Make sure it is accepted with kernel keyword before the attribute.
-kernel __attribute__((amdgpu_num_vgpr(40))) void foo16() {}
-
-kernel __attribute__((amdgpu_num_sgpr(40))) void foo17() {}
Index: test/SemaOpenCL/amdgpu-attrs.cl
===
--- test/SemaOpenCL/amdgpu-attrs.cl
+++ test/SemaOpenCL/amdgpu-attrs.cl
@@ -0,0 +1,64 @@
+// RUN: %clang_cc1 -triple amdgcn-- -verify -fsyntax-only %s
+
+typedef __attribute__((amdgpu_flat_work_group_size(32, 64))) struct struct_flat_work_group_size_32_64 { // expected-error {{'amdgpu_flat_work_group_size' attribute only applies to kernel functions}}
+  int x;
+  float y;
+} struct_flat_work_group_size_32_64;
+typedef __attribute__((amdgpu_waves_per_eu(2))) struct struct_waves_per_eu_2 { // expected-error {{'amdgpu_waves_per_eu' attribute only applies to kernel functions}}
+  int x;
+  float y;
+} struct_waves_per_eu_2;
+typedef __attribute__((amdgpu_waves_per_eu(2, 4))) struct struct_waves_per_eu_2_4 { // expected-error {{'amdgpu_waves_per_eu' attribute only applies to kernel functions}}
+  int x;
+  float y;
+} struct_waves_per_eu_2_4;
+typedef __attribute__((amdgpu_num_sgpr(32))) struct struct_num_sgpr_32 { // expected-error {{'amdgpu_num_sgpr' attribute only applies to kernel functions}}
+  int x;
+  float y;
+} struct_num_sgpr_32;
+typedef __attribute__((amdgpu_num_vgpr(64))) struct struct_num_vgpr_64 { // expected-error {{'amdgpu_num_vgpr' attribute only applies to kernel functions}}
+  int x;
+  float y;
+} struct_num_vgpr_64;
+
+__attribute__((amdgpu_flat_work_group_size(32, 64))) void func_flat_work_group_size_32_64() {} // expected-error {{'amdgpu_flat_work_group_size' attribute only applies to kernel functions}}
+__attribute__((amdgpu_waves_per_eu(2))) void func_waves_per_eu_2() {} // expected-error {{'amdgpu_waves_per_eu' attribute only applies to kernel functions}}
+__attribute__((amdgpu_waves_per_eu(2, 4))) void func_waves_per_eu_2_4() {} // expected-error {{'amdgpu_waves_per_eu' attribute only applies to kernel functions}}
+__attribute__((amdgpu_num_sgpr(32))) void func_num_sgpr_32() {} // expected-error {{'amdgpu_num_sgpr' attribute only applies to kernel functi

[PATCH] D24574: clang-format: [JS] ASI insertion after boolean literals.

[Martin Probst via cfe-commits]

mprobst created this revision.
mprobst added a reviewer: djasper.
mprobst added a subscriber: cfe-commits.
Herald added a subscriber: klimek.

Before when a semicolon was missing after a boolean literal:
a = true
return 1;

clang-format would parse this as one line and format as:
a = true return 1;

It turns out that C++ does not consider `true` and `false` to be literals, we
have to check for that explicitly.

https://reviews.llvm.org/D24574

Files:
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTestJS.cpp

Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -747,6 +747,14 @@
   "String");
   verifyFormat("function f(@Foo bar) {}", "function f(@Foo\n"
   "  bar) {}");
+  verifyFormat("a = true\n"
+   "return 1",
+   "a = true\n"
+   "  return   1");
+  verifyFormat("a = 'true'\n"
+   "return 1",
+   "a = 'true'\n"
+   "  return   1");
 }
 
 TEST_F(FormatTestJS, ClosureStyleCasts) {
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -681,7 +681,9 @@
 
 static bool mustBeJSIdentOrValue(const AdditionalKeywords &Keywords,
  const FormatToken *FormatTok) {
-  return FormatTok->Tok.isLiteral() || mustBeJSIdent(Keywords, FormatTok);
+  return FormatTok->Tok.isLiteral() ||
+ FormatTok->isOneOf(tok::kw_true, tok::kw_false) ||
+ mustBeJSIdent(Keywords, FormatTok);
 }
 
 // isJSDeclOrStmt returns true if |FormatTok| starts a declaration or statement


Index: unittests/Format/FormatTestJS.cpp
===
--- unittests/Format/FormatTestJS.cpp
+++ unittests/Format/FormatTestJS.cpp
@@ -747,6 +747,14 @@
   "String");
   verifyFormat("function f(@Foo bar) {}", "function f(@Foo\n"
   "  bar) {}");
+  verifyFormat("a = true\n"
+   "return 1",
+   "a = true\n"
+   "  return   1");
+  verifyFormat("a = 'true'\n"
+   "return 1",
+   "a = 'true'\n"
+   "  return   1");
 }
 
 TEST_F(FormatTestJS, ClosureStyleCasts) {
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -681,7 +681,9 @@
 
 static bool mustBeJSIdentOrValue(const AdditionalKeywords &Keywords,
  const FormatToken *FormatTok) {
-  return FormatTok->Tok.isLiteral() || mustBeJSIdent(Keywords, FormatTok);
+  return FormatTok->Tok.isLiteral() ||
+ FormatTok->isOneOf(tok::kw_true, tok::kw_false) ||
+ mustBeJSIdent(Keywords, FormatTok);
 }
 
 // isJSDeclOrStmt returns true if |FormatTok| starts a declaration or statement
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r281509 - Convert finite to builtin

[Dehao Chen via cfe-commits]

Author: dehao
Date: Wed Sep 14 12:34:14 2016
New Revision: 281509

URL: http://llvm.org/viewvc/llvm-project?rev=281509&view=rev
Log:
Convert finite to builtin

Summary: This patch converts finite/__finite to builtin functions so that it 
will be inlined by compiler.

Reviewers: hfinkel, davidxl, efriedma

Subscribers: efriedma, llvm-commits

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

Modified:
cfe/trunk/include/clang/Basic/Builtins.def
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/test/CodeGen/builtins.c

Modified: cfe/trunk/include/clang/Basic/Builtins.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=281509&r1=281508&r2=281509&view=diff
==
--- cfe/trunk/include/clang/Basic/Builtins.def (original)
+++ cfe/trunk/include/clang/Basic/Builtins.def Wed Sep 14 12:34:14 2016
@@ -937,6 +937,14 @@ LIBBUILTIN(fabs, "dd", "fnc", "math.h",
 LIBBUILTIN(fabsf, "ff", "fnc", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(fabsl, "LdLd", "fnc", "math.h", ALL_LANGUAGES)
 
+LIBBUILTIN(finite, "id", "fnc", "math.h", GNU_LANG)
+LIBBUILTIN(finitef, "if", "fnc", "math.h", GNU_LANG)
+LIBBUILTIN(finitel, "iLd", "fnc", "math.h", GNU_LANG)
+// glibc's math.h generates calls to __finite
+LIBBUILTIN(__finite, "id", "fnc", "math.h", ALL_LANGUAGES)
+LIBBUILTIN(__finitef, "if", "fnc", "math.h", ALL_LANGUAGES)
+LIBBUILTIN(__finitel, "iLd", "fnc", "math.h", ALL_LANGUAGES)
+
 LIBBUILTIN(fmod, "ddd", "fne", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(fmodf, "fff", "fne", "math.h", ALL_LANGUAGES)
 LIBBUILTIN(fmodl, "LdLdLd", "fne", "math.h", ALL_LANGUAGES)

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=281509&r1=281508&r2=281509&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Wed Sep 14 12:34:14 2016
@@ -903,6 +903,12 @@ RValue CodeGenFunction::EmitBuiltinExpr(
 return RValue::get(Builder.CreateZExt(V, ConvertType(E->getType(;
   }
 
+  case Builtin::BIfinite:
+  case Builtin::BI__finite:
+  case Builtin::BIfinitef:
+  case Builtin::BI__finitef:
+  case Builtin::BIfinitel:
+  case Builtin::BI__finitel:
   case Builtin::BI__builtin_isinf:
   case Builtin::BI__builtin_isfinite: {
 // isinf(x)--> fabs(x) == infinity

Modified: cfe/trunk/test/CodeGen/builtins.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/builtins.c?rev=281509&r1=281508&r2=281509&view=diff
==
--- cfe/trunk/test/CodeGen/builtins.c (original)
+++ cfe/trunk/test/CodeGen/builtins.c Wed Sep 14 12:34:14 2016
@@ -220,6 +220,10 @@ void test_float_builtins(float F, double
   // CHECK: call float @llvm.fabs.f32(float
   // CHECK: fcmp one float {{.*}}, 0x7FF0
 
+  res = finite(D);
+  // CHECK: call double @llvm.fabs.f64(double
+  // CHECK: fcmp one double {{.*}}, 0x7FF0
+
   res = __builtin_isnormal(F);
   // CHECK: fcmp oeq float
   // CHECK: call float @llvm.fabs.f32(float


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

Re: [PATCH] D24513: [AMDGPU] Expose flat work group size, register and wave control attributes

[Aaron Ballman via cfe-commits]

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


Comment at: include/clang/Basic/Attr.td:1054-1056
@@ -1058,3 +1053,5 @@
+//
 // FIXME: This should be for OpenCLKernelFunction, but is not to
 // workaround needing to see kernel attribute before others to know if
 // this should be rejected on non-kernels.
+

This FIXME is now detached from what "this" is referring to. Can you clarify by 
mentioning that the Subjects list is what should be modified?


Comment at: include/clang/Basic/Attr.td:1067
@@ +1066,3 @@
+  let Spellings = [GNU<"amdgpu_waves_per_eu">];
+  let Args = [UnsignedArgument<"Min">, VariadicUnsignedArgument<"Max">];
+  let Documentation = [AMDGPUWavesPerEUDocs];

Looking at the documentation, are you sure this should be a 
`VariadicUnsignedArgument`? It seems like this should be an `UnsignedArgument` 
with the optional bit set. Or can you pass multiple Max values?


Comment at: include/clang/Basic/AttrDocs.td:1138
@@ +1137,3 @@
+ wavefronts are able to fit within the resources of an EU. Requesting
+more wavefronts can  hide memory latency but limits available registers which
+can result in spilling. Requesting fewer wavefronts can help reduce cache

There's an extra space between "can" and "hide".


Comment at: include/clang/Basic/AttrDocs.td:1144
@@ +1143,3 @@
+to ensure it is capable of meeting the requested values. However, when the
+kernel is executed there may be other reasons that prevent meeting the request,
+for example, there may be wavefronts from other kernels executing on the EU.

Comma after "executed".


Comment at: include/clang/Basic/AttrDocs.td:1147
@@ +1146,3 @@
+
+The error will be given if:
+  - Specified values violate subtarget specifications;

An error instead of The error.


Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2385
@@ -2384,1 +2384,3 @@
+def err_attribute_argument_invalid : Error<
+  "%0 attribute parameter is invalid: %1">;
 def err_attribute_argument_is_zero : Error<

I think this should say the attribute argument is invalid, rather than the 
attribute parameter.


Comment at: lib/CodeGen/TargetInfo.cpp:6958
@@ +6957,3 @@
+
+  if (const auto Attr = FD->getAttr()) {
+unsigned Min = Attr->getMin();

Should be `const auto *Attr`.


Comment at: lib/CodeGen/TargetInfo.cpp:6967-6969
@@ +6966,5 @@
+  F->addFnAttr("amdgpu-flat-work-group-size", AttrVal);
+} else {
+  assert(Max == 0 && "Max must be zero");
+}
+  }

Elide braces?


Comment at: lib/CodeGen/TargetInfo.cpp:6972
@@ +6971,3 @@
+
+  if (const auto Attr = FD->getAttr()) {
+unsigned Min = Attr->getMin();

`const auto *` here as well.


Comment at: lib/CodeGen/TargetInfo.cpp:6983-6985
@@ -6961,1 +6982,5 @@
+  F->addFnAttr("amdgpu-waves-per-eu", AttrVal);
+} else {
+  assert(Max == 0 && "Max must be zero");
+}
   }

Elide braces?


Comment at: lib/CodeGen/TargetInfo.cpp:6995
@@ +6994,3 @@
+
+  if (const auto Attr = FD->getAttr()) {
+uint32_t NumVGPR = Attr->getNumVGPR();

`const auto *` here as well.


Comment at: lib/Sema/SemaDeclAttr.cpp:4947
@@ +4946,3 @@
+  uint32_t Min = 0;
+  Expr *MinExpr = static_cast(Attr.getArgAsExpr(0));
+  if (!checkUInt32Argument(S, Attr, MinExpr, Min))

I don't think this case is required (here, or elsewhere). `getArgAsExpr()` 
already returns an `Expr *`.


Comment at: lib/Sema/SemaDeclAttr.cpp:4959-4960
@@ +4958,4 @@
+  << Attr.getName()
+  << "maximum flat work-group size must be zero since minimum flat "
+ "work-group size is zero";
+return;

Please include this text in the diagnostic rather than hard-coding it here. You 
can use `%select` to differentiate between different text snippets in the 
diagnostic, and the diagnostic doesn't need to continue to try to be generic. 
Same comment applies elsewhere.


Comment at: lib/Sema/SemaDeclAttr.cpp:4988
@@ +4987,3 @@
+  return;
+  }
+

This will change if you use the optional bit rather than a variadic argument, 
but this silently eats attributes that have more than two arguments, which is 
not a good user experience.


Comment at: lib/Sema/SemaDeclAttr.cpp:6048
@@ -5976,2 +6047,3 @@
   D->setInvalidDecl();
-} else if (Attr *A = D->getAttr()) {
+} else if (Attr *A = D->getAttr()) {
+  Diag(D->getLocation(), diag::err_attribute_wrong_decl_type)

This list is getting to the point where we really need to start handling this 
in Attr.td soon. Are you planning to work on more AMDGPU attributes in the near 
future?

=

Re: [PATCH] D24526: [Release notes] Mention readability-container-size-empty improvements

[Eugene Zelenko via cfe-commits]

Eugene.Zelenko added inline comments.


Comment at: docs/ReleaseNotes.rst:94
@@ +93,3 @@
+  
`_
 check
+  supports arbitrary containers with ``empty()`` and ``size()`` methods.
+

alexfh wrote:
> The wording is a bit too broad. We should somehow hint that the methods are 
> not just arbitrary. Maybe "with suitable ``size()`` and ``empty()`` methods"?
Will fix on commit. **Suitable** sounds good enough since link to documentation 
is provided, where more details could be found.


Repository:
  rL LLVM

https://reviews.llvm.org/D24526



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

[clang-tools-extra] r281510 - [Release notes] Mention readability-container-size-empty improvements.

[Eugene Zelenko via cfe-commits]

Author: eugenezelenko
Date: Wed Sep 14 12:41:51 2016
New Revision: 281510

URL: http://llvm.org/viewvc/llvm-project?rev=281510&view=rev
Log:
[Release notes] Mention readability-container-size-empty improvements.

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

Modified:
clang-tools-extra/trunk/docs/ReleaseNotes.rst

Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=281510&r1=281509&r2=281510&view=diff
==
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Wed Sep 14 12:41:51 2016
@@ -95,6 +95,11 @@ Improvements to clang-tidy
   Warns about the performance overhead arising from concatenating strings using
   the ``operator+``, instead of ``operator+=``.
 
+- `readability-container-size-empty
+  
`_
 check
+  supports arbitrary containers with with suitable ``empty()`` and ``size()``
+  methods.
+
 - New `readability-misplaced-array-index
   
`_
 check
 


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

Re: [PATCH] D24526: [Release notes] Mention readability-container-size-empty improvements

[Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL281510: [Release notes] Mention 
readability-container-size-empty improvements. (authored by eugenezelenko).

Changed prior to commit:
  https://reviews.llvm.org/D24526?vs=71250&id=71394#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24526

Files:
  clang-tools-extra/trunk/docs/ReleaseNotes.rst

Index: clang-tools-extra/trunk/docs/ReleaseNotes.rst
===
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst
@@ -95,6 +95,11 @@
   Warns about the performance overhead arising from concatenating strings using
   the ``operator+``, instead of ``operator+=``.
 
+- `readability-container-size-empty
+  
`_
 check
+  supports arbitrary containers with with suitable ``empty()`` and ``size()``
+  methods.
+
 - New `readability-misplaced-array-index
   
`_
 check
 


Index: clang-tools-extra/trunk/docs/ReleaseNotes.rst
===
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst
@@ -95,6 +95,11 @@
   Warns about the performance overhead arising from concatenating strings using
   the ``operator+``, instead of ``operator+=``.
 
+- `readability-container-size-empty
+  `_ check
+  supports arbitrary containers with with suitable ``empty()`` and ``size()``
+  methods.
+
 - New `readability-misplaced-array-index
   `_ check
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22346: [Clang-tidy] CERT-MSC50-CPP (std:rand() )

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.


Comment at: clang-tidy/cert/LimitedRandomnessCheck.cpp:22-23
@@ +21,4 @@
+  Finder->addMatcher(
+  declRefExpr(hasDeclaration(functionDecl(namedDecl(hasName("::rand")),
+  parameterCountIs(0
+  .bind("randomGenerator"),

falho wrote:
> xazax.hun wrote:
> > aaron.ballman wrote:
> > > xazax.hun wrote:
> > > > aaron.ballman wrote:
> > > > > xazax.hun wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > Prazek wrote:
> > > > > > > > aaron.ballman wrote:
> > > > > > > > > Prazek wrote:
> > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > This should be looking at a callExpr() rather than a 
> > > > > > > > > > > declRefExpr(), should it not?
> > > > > > > > > > I was also thinking about this, but this is actually 
> > > > > > > > > > better, because it will also match with binding rand with 
> > > > > > > > > > function pointer.
> > > > > > > > > True, but a DeclRefExpr doesn't mean it's a function call. 
> > > > > > > > > Binding the function is not contrary to the CERT rule, just 
> > > > > > > > > calling it. For instance, the following pathological case 
> > > > > > > > > will be caught by this check:
> > > > > > > > > ```
> > > > > > > > > if (std::rand) {}
> > > > > > > > > ```
> > > > > > > > > The behavior of this check should be consistent with 
> > > > > > > > > cert-env33-c, which only looks at calls. (If we really care 
> > > > > > > > > about bound functions, we'd need flow control analysis, and I 
> > > > > > > > > think that's overkill for both of those checks, but wouldn't 
> > > > > > > > > be opposed to someone writing the flow analysis if they 
> > > > > > > > > really wanted to.)
> > > > > > > > It would indeed fire on this pathological case, but I don't 
> > > > > > > > think we should care about cases like this, because no one is 
> > > > > > > > writing code like this (and if he would then it would probably 
> > > > > > > > be a bug).
> > > > > > > > I don't think that there is much code that binds pointer to 
> > > > > > > > std::rand either, but I think it would be good to display 
> > > > > > > > warning for this, because even if the function would be never 
> > > > > > > > called, then it means that this is a bug, and if it would be 
> > > > > > > > called then it would be nice to tell user that rand might be 
> > > > > > > > used here.
> > > > > > > > 
> > > > > > > > Anyway I don't oppose for changing it to callExpr, but I think 
> > > > > > > > it is better this way.
> > > > > > > > It would indeed fire on this pathological case, but I don't 
> > > > > > > > think we should care about cases like this, because no one is 
> > > > > > > > writing code like this (and if he would then it would probably 
> > > > > > > > be a bug).
> > > > > > > 
> > > > > > > It would be a known false-positive for a check designed to 
> > > > > > > conform to a particular coding standard. When deviations have 
> > > > > > > come up in the past for various coding standards, we've added an 
> > > > > > > option to enable the additional functionality, which I don't 
> > > > > > > think would be reasonable in this case. Alternatively, the CERT 
> > > > > > > guideline could be modified, but that is unlikely to occur 
> > > > > > > because binding the function pointer is not a security concern 
> > > > > > > (only calling the function).
> > > > > > In case you let binding to function pointer you introduce potential 
> > > > > > false negatives which is worse in this case in my opinion. 
> > > > > Basically: this half-measure is sufficient for the CERT coding rule, 
> > > > > but isn't ideal. The ideal check isn't likely to uncover many more 
> > > > > cases than the half-measure, which is why it was not implemented in 
> > > > > the past. If someone wants to implement the whole-measure, that's 
> > > > > great! But implementing a half, half-measure that isn't consistent 
> > > > > with other, similar checks is the wrong thing to do.
> > > > You can not implement an ideal checker. In general, it is undecidable 
> > > > whether std::rand is called or not. (You can easily create an example 
> > > > where you would need to solve the halting problem in order to decide 
> > > > whether std::rand is called.)
> > > > 
> > > > Since the ideal checker is infeasible the question is whether you are 
> > > > OK with false positives or false negatives. The approach you are 
> > > > suggesting result in false negatives. The current approach results in 
> > > > false positives. I think, for this security checker, a false positive 
> > > > is much less serious to have than a false negative. Moreover, I doubt 
> > > > that people write code where such false positives are intended and the 
> > > > code should not be changed. But in case you can think of an example, 
> > > > please let us know.
> > > > You can not implement an ideal checker. In general, it is undecidable 
> > > > whether s

r281516 - [analyzer] scan-build-py: Remove relative path hack for SATestsBuild.py

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Wed Sep 14 13:14:11 2016
New Revision: 281516

URL: http://llvm.org/viewvc/llvm-project?rev=281516&view=rev
Log:
[analyzer] scan-build-py: Remove relative path hack for SATestsBuild.py

Remove the relative path hack in scan-build-py that converts a fully qualified
directory name and a fully qualified file path to a relative path before running
the analyzer on a file.

This hack is not needed: the bad interaction with SATestsBuild.py it was
intended to address is actually the same underlying problem that r280768 fixed.
Further, because the hack would always relativize paths, it caused
SATestBuild.py to be unable to properly line up issues when the build system
changed directory and then built a source file in a child directory but used a
fully-qualified path for the source file.

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

Modified:
cfe/trunk/tools/scan-build-py/libscanbuild/runner.py
cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py

Modified: cfe/trunk/tools/scan-build-py/libscanbuild/runner.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/scan-build-py/libscanbuild/runner.py?rev=281516&r1=281515&r2=281516&view=diff
==
--- cfe/trunk/tools/scan-build-py/libscanbuild/runner.py (original)
+++ cfe/trunk/tools/scan-build-py/libscanbuild/runner.py Wed Sep 14 13:14:11 
2016
@@ -205,19 +205,8 @@ def filter_debug_flags(opts, continuatio
 return continuation(opts)
 
 
-@require(['file', 'directory'])
-def set_file_path_relative(opts, continuation=filter_debug_flags):
-""" Set source file path to relative to the working directory.
-
-The only purpose of this function is to pass the SATestBuild.py tests. """
-
-opts.update({'file': os.path.relpath(opts['file'], opts['directory'])})
-
-return continuation(opts)
-
-
 @require(['language', 'compiler', 'file', 'flags'])
-def language_check(opts, continuation=set_file_path_relative):
+def language_check(opts, continuation=filter_debug_flags):
 """ Find out the language from command line parameters or file name
 extension. The decision also influenced by the compiler invocation. """
 

Modified: cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py?rev=281516&r1=281515&r2=281516&view=diff
==
--- cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py (original)
+++ cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py Wed Sep 14 13:14:11 
2016
@@ -219,20 +219,6 @@ class AnalyzerTest(unittest.TestCase):
 self.assertEqual(['-DNDEBUG', '-UNDEBUG'], test(['-DNDEBUG']))
 self.assertEqual(['-DSomething', '-UNDEBUG'], test(['-DSomething']))
 
-def test_set_file_relative_path(self):
-def test(expected, input):
-spy = Spy()
-self.assertEqual(spy.success,
- sut.set_file_path_relative(input, spy.call))
-self.assertEqual(expected, spy.arg['file'])
-
-test('source.c',
- {'file': '/home/me/source.c', 'directory': '/home/me'})
-test('me/source.c',
- {'file': '/home/me/source.c', 'directory': '/home'})
-test('../home/me/source.c',
- {'file': '/home/me/source.c', 'directory': '/tmp'})
-
 def test_set_language_fall_through(self):
 def language(expected, input):
 spy = Spy()


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

Re: [PATCH] D24470: [analyzer] scan-build-py: Remove relative path hack for SATestsBuild.py

[Devin Coughlin via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL281516: [analyzer] scan-build-py: Remove relative path hack 
for SATestsBuild.py (authored by dcoughlin).

Changed prior to commit:
  https://reviews.llvm.org/D24470?vs=71046&id=71401#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24470

Files:
  cfe/trunk/tools/scan-build-py/libscanbuild/runner.py
  cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py

Index: cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py
===
--- cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py
+++ cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py
@@ -219,20 +219,6 @@
 self.assertEqual(['-DNDEBUG', '-UNDEBUG'], test(['-DNDEBUG']))
 self.assertEqual(['-DSomething', '-UNDEBUG'], test(['-DSomething']))
 
-def test_set_file_relative_path(self):
-def test(expected, input):
-spy = Spy()
-self.assertEqual(spy.success,
- sut.set_file_path_relative(input, spy.call))
-self.assertEqual(expected, spy.arg['file'])
-
-test('source.c',
- {'file': '/home/me/source.c', 'directory': '/home/me'})
-test('me/source.c',
- {'file': '/home/me/source.c', 'directory': '/home'})
-test('../home/me/source.c',
- {'file': '/home/me/source.c', 'directory': '/tmp'})
-
 def test_set_language_fall_through(self):
 def language(expected, input):
 spy = Spy()
Index: cfe/trunk/tools/scan-build-py/libscanbuild/runner.py
===
--- cfe/trunk/tools/scan-build-py/libscanbuild/runner.py
+++ cfe/trunk/tools/scan-build-py/libscanbuild/runner.py
@@ -205,19 +205,8 @@
 return continuation(opts)
 
 
-@require(['file', 'directory'])
-def set_file_path_relative(opts, continuation=filter_debug_flags):
-""" Set source file path to relative to the working directory.
-
-The only purpose of this function is to pass the SATestBuild.py tests. """
-
-opts.update({'file': os.path.relpath(opts['file'], opts['directory'])})
-
-return continuation(opts)
-
-
 @require(['language', 'compiler', 'file', 'flags'])
-def language_check(opts, continuation=set_file_path_relative):
+def language_check(opts, continuation=filter_debug_flags):
 """ Find out the language from command line parameters or file name
 extension. The decision also influenced by the compiler invocation. """
 


Index: cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py
===
--- cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py
+++ cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py
@@ -219,20 +219,6 @@
 self.assertEqual(['-DNDEBUG', '-UNDEBUG'], test(['-DNDEBUG']))
 self.assertEqual(['-DSomething', '-UNDEBUG'], test(['-DSomething']))
 
-def test_set_file_relative_path(self):
-def test(expected, input):
-spy = Spy()
-self.assertEqual(spy.success,
- sut.set_file_path_relative(input, spy.call))
-self.assertEqual(expected, spy.arg['file'])
-
-test('source.c',
- {'file': '/home/me/source.c', 'directory': '/home/me'})
-test('me/source.c',
- {'file': '/home/me/source.c', 'directory': '/home'})
-test('../home/me/source.c',
- {'file': '/home/me/source.c', 'directory': '/tmp'})
-
 def test_set_language_fall_through(self):
 def language(expected, input):
 spy = Spy()
Index: cfe/trunk/tools/scan-build-py/libscanbuild/runner.py
===
--- cfe/trunk/tools/scan-build-py/libscanbuild/runner.py
+++ cfe/trunk/tools/scan-build-py/libscanbuild/runner.py
@@ -205,19 +205,8 @@
 return continuation(opts)
 
 
-@require(['file', 'directory'])
-def set_file_path_relative(opts, continuation=filter_debug_flags):
-""" Set source file path to relative to the working directory.
-
-The only purpose of this function is to pass the SATestBuild.py tests. """
-
-opts.update({'file': os.path.relpath(opts['file'], opts['directory'])})
-
-return continuation(opts)
-
-
 @require(['language', 'compiler', 'file', 'flags'])
-def language_check(opts, continuation=set_file_path_relative):
+def language_check(opts, continuation=filter_debug_flags):
 """ Find out the language from command line parameters or file name
 extension. The decision also influenced by the compiler invocation. """
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24330: Add some MS aliases for existing intrinsics

[Albert Gutowski via cfe-commits]

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

https://reviews.llvm.org/D24330



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

Re: [PATCH] D24330: Add some MS aliases for existing intrinsics

[Albert Gutowski via cfe-commits]

agutowski updated this revision to Diff 71404.
agutowski added a comment.

Fix doxygen comment


https://reviews.llvm.org/D24330

Files:
  include/clang/Basic/Builtins.def
  include/clang/Basic/Builtins.h
  include/clang/Basic/BuiltinsX86.def
  lib/Basic/Targets.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Headers/emmintrin.h
  lib/Headers/ia32intrin.h
  lib/Headers/intrin.h
  lib/Headers/xmmintrin.h
  lib/Sema/SemaDecl.cpp
  test/CodeGen/builtins-x86.c
  test/Headers/x86intrin.cpp
  test/Sema/implicit-intel-builtin-decl.c
  test/Sema/implicit-ms-builtin-decl.c

Index: lib/Headers/ia32intrin.h
===
--- lib/Headers/ia32intrin.h
+++ lib/Headers/ia32intrin.h
@@ -60,12 +60,6 @@
   return __builtin_ia32_rdpmc(__A);
 }
 
-/* __rdtsc */
-static __inline__ unsigned long long __attribute__((__always_inline__, __nodebug__))
-__rdtsc(void) {
-  return __builtin_ia32_rdtsc();
-}
-
 /* __rdtscp */
 static __inline__ unsigned long long __attribute__((__always_inline__, __nodebug__))
 __rdtscp(unsigned int *__A) {
Index: lib/Headers/emmintrin.h
===
--- lib/Headers/emmintrin.h
+++ lib/Headers/emmintrin.h
@@ -2447,6 +2447,10 @@
 }
 #endif
 
+#if defined(__cplusplus)
+extern "C" {
+#endif
+
 /// \brief The cache line containing __p is flushed and invalidated from all
 ///caches in the coherency domain.
 ///
@@ -2457,11 +2461,7 @@
 /// \param __p
 ///A pointer to the memory location used to identify the cache line to be
 ///flushed.
-static __inline__ void __DEFAULT_FN_ATTRS
-_mm_clflush(void const *__p)
-{
-  __builtin_ia32_clflush(__p);
-}
+void _mm_clflush(void const *);
 
 /// \brief Forces strong memory ordering (serialization) between load
 ///instructions preceding this instruction and load instructions following
@@ -2472,11 +2472,7 @@
 ///
 /// This intrinsic corresponds to the \c LFENCE instruction.
 ///
-static __inline__ void __DEFAULT_FN_ATTRS
-_mm_lfence(void)
-{
-  __builtin_ia32_lfence();
-}
+void _mm_lfence(void);
 
 /// \brief Forces strong memory ordering (serialization) between load and store
 ///instructions preceding this instruction and load and store instructions
@@ -2487,12 +2483,12 @@
 ///
 /// This intrinsic corresponds to the \c MFENCE instruction.
 ///
-static __inline__ void __DEFAULT_FN_ATTRS
-_mm_mfence(void)
-{
-  __builtin_ia32_mfence();
-}
+void _mm_mfence(void);
 
+#if defined(__cplusplus)
+} // extern "C"
+#endif
+
 /// \brief Converts 16-bit signed integers from both 128-bit integer vector
 ///operands into 8-bit signed integers, and packs the results into the
 ///destination. Positive values greater than 0x7F are saturated to 0x7F.
@@ -3213,11 +3209,10 @@
 ///
 /// This intrinsic corresponds to the \c PAUSE instruction.
 ///
-static __inline__ void __DEFAULT_FN_ATTRS
-_mm_pause(void)
-{
-  __builtin_ia32_pause();
-}
+#if defined(__cplusplus)
+extern "C"
+#endif
+void _mm_pause(void);
 
 #undef __DEFAULT_FN_ATTRS
 
Index: lib/Headers/intrin.h
===
--- lib/Headers/intrin.h
+++ lib/Headers/intrin.h
@@ -463,14 +463,6 @@
   *_Index = 31 - __builtin_clzl(_Mask);
   return 1;
 }
-static __inline__ unsigned short __DEFAULT_FN_ATTRS
-__popcnt16(unsigned short _Value) {
-  return __builtin_popcount((int)_Value);
-}
-static __inline__ unsigned int __DEFAULT_FN_ATTRS
-__popcnt(unsigned int _Value) {
-  return __builtin_popcount(_Value);
-}
 static __inline__ unsigned char __DEFAULT_FN_ATTRS
 _bittest(long const *_BitBase, long _BitPos) {
   return (*_BitBase >> _BitPos) & 1;
@@ -513,11 +505,6 @@
   *_Index = 63 - __builtin_clzll(_Mask);
   return 1;
 }
-static __inline__
-unsigned __int64 __DEFAULT_FN_ATTRS
-__popcnt64(unsigned __int64 _Value) {
-  return __builtin_popcountll(_Value);
-}
 static __inline__ unsigned char __DEFAULT_FN_ATTRS
 _bittest64(__int64 const *_BitBase, __int64 _BitPos) {
   return (*_BitBase >> _BitPos) & 1;
@@ -546,63 +533,63 @@
   __atomic_fetch_or(_BitBase, 1ll << _BitPos, __ATOMIC_SEQ_CST);
   return (_PrevVal >> _BitPos) & 1;
 }
-/**\
-|* Interlocked Exchange Add
-\**/
-static __inline__ __int64 __DEFAULT_FN_ATTRS
-_InterlockedExchangeAdd64(__int64 volatile *_Addend, __int64 _Value) {
-	return __atomic_fetch_add(_Addend, _Value, __ATOMIC_SEQ_CST);
-}
-/**\
-|* Interlocked Exchange Sub
-\**/
-static __inline__ __int64 __DEFAULT_FN_ATTRS
-_InterlockedExchangeSub64(__int64 volatile *_Subend, __int64 _Value) {
-	return __atomic_fetch_sub(_Subend, _Value, __ATOMIC_SEQ_CST);
-}
-/**\
-|* Interl

Re: [PATCH] D24467: Fix an error after D21678

[Akira Hatanaka via cfe-commits]

ahatanak added inline comments.


Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8751
@@ -8747,3 +8750,3 @@
 
   // OpenCL v1.1 s6.3.j says that the operands need to be integers.
   if (!LHSEleType->isIntegerType()) {

Should we mention that any vectors used as shift operands have to have integer 
element types? This comment gives the impression that only OpenCL vectors need 
to be integers.


Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8790
@@ -8774,3 +8789,3 @@
   S.Context.getExtVectorType(RHSEleType, LHSVecTy->getNumElements());
 RHS = S.ImpCastExprToType(RHS.get(), VecTy, CK_VectorSplat);
   }

OK. So the rule is that the type of the scalar operand is normally converted to 
the vector element type of the other operand before being transformed to a 
vector, but we don't have to do this when it's used as the RHS (shift amount) 
because IRGen generates the correct IR without the conversion?


Comment at: llvm/tools/clang/test/Sema/vecshift.c:56
@@ +55,3 @@
+
+  vc4 = vc4 << vc8; // expected-error {{vector operands do not have the same 
number of elements}}
+  vi4 = vi4 << vuc8; // expected-error {{vector operands do not have the same 
number of elements}}

I don't think this patch was necessary to have clang print diagnostic "vector 
operands do not have the same number of elements"? If that's the case, I think 
it's better to add these lines in a separate commit.


https://reviews.llvm.org/D24467



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

Re: [PATCH] D21506: [analyzer] Block in critical section

[Anna Zaks via cfe-commits]

zaks.anna added a comment.

Do you have commit access or should we commit?


Repository:
  rL LLVM

https://reviews.llvm.org/D21506



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

Re: [PATCH] D21506: [analyzer] Block in critical section

[Anna Zaks via cfe-commits]

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

LGTM! Thanks!

Future steps:
How do we plan to bring this checker out of alpha? Have you evaluated it on 
large codebases?


Repository:
  rL LLVM

https://reviews.llvm.org/D21506



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

Re: [PATCH] D24574: clang-format: [JS] ASI insertion after boolean literals.

[Daniel Jasper via cfe-commits]

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

Interesting.. Would have expected true/false to be literals as well, but ok :).


https://reviews.llvm.org/D24574



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

Re: [PATCH] D24467: Fix an error after D21678

[Vladimir Yakovlev via cfe-commits]

vbyakovlcl added inline comments.


Comment at: llvm/tools/clang/lib/Sema/SemaExpr.cpp:8790
@@ -8774,3 +8789,3 @@
   S.Context.getExtVectorType(RHSEleType, LHSVecTy->getNumElements());
 RHS = S.ImpCastExprToType(RHS.get(), VecTy, CK_VectorSplat);
   }

ahatanak wrote:
> OK. So the rule is that the type of the scalar operand is normally converted 
> to the vector element type of the other operand before being transformed to a 
> vector, but we don't have to do this when it's used as the RHS (shift amount) 
> because IRGen generates the correct IR without the conversion?
Yes. I think a CodeGen test should be added for checking of IR correctness. 


Comment at: llvm/tools/clang/test/Sema/vecshift.c:56
@@ +55,3 @@
+
+  vc4 = vc4 << vc8; // expected-error {{vector operands do not have the same 
number of elements}}
+  vi4 = vi4 << vuc8; // expected-error {{vector operands do not have the same 
number of elements}}

ahatanak wrote:
> I don't think this patch was necessary to have clang print diagnostic "vector 
> operands do not have the same number of elements"? If that's the case, I 
> think it's better to add these lines in a separate commit.
Do you propose to delete lines with this diagnostic?


https://reviews.llvm.org/D24467



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

Re: [PATCH] D24518: Correct assert text in DeclGroup::getSingleDecl()

[Ben Taylor via cfe-commits]

brtaylor92 added a comment.

I don't have commit access. @alexfh could you please land the patch?


https://reviews.llvm.org/D24518



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

Re: [PATCH] D23820: Do not warn about format strings that are indexed string literals.

[Meike Baumgärtner via cfe-commits]

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

Thanks for reviewing my patch! It would be great if someone could submit this 
patch for me.


https://reviews.llvm.org/D23820



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

Re: [PATCH] D22452: [libcxx] Fix last_write_time tests for filesystems that don't support negative and very large times.

[Eric Fiselier via cfe-commits]

EricWF added a comment.

Thanks. I'll update this patch today.


https://reviews.llvm.org/D22452



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

r281525 - Correct assert text in DeclGroup::getSingleDecl()

[Kirill Bobyrev via cfe-commits]

Author: omtcyfz
Date: Wed Sep 14 14:59:26 2016
New Revision: 281525

URL: http://llvm.org/viewvc/llvm-project?rev=281525&view=rev
Log:
Correct assert text in DeclGroup::getSingleDecl()

Assert text for getSingleDecl() is inaccurate. Appears to have been copy pasted
from getDeclGroup().

Patch by Ben Taylor!

Reviewers: alexfh

Subscribers: cfe-commits

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

Modified:
cfe/trunk/include/clang/AST/DeclGroup.h

Modified: cfe/trunk/include/clang/AST/DeclGroup.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclGroup.h?rev=281525&r1=281524&r2=281525&view=diff
==
--- cfe/trunk/include/clang/AST/DeclGroup.h (original)
+++ cfe/trunk/include/clang/AST/DeclGroup.h Wed Sep 14 14:59:26 2016
@@ -84,7 +84,7 @@ public:
   bool isDeclGroup() const { return getKind() == DeclGroupKind; }
 
   Decl *getSingleDecl() {
-assert(isSingleDecl() && "Isn't a declgroup");
+assert(isSingleDecl() && "Isn't a single decl");
 return D;
   }
   const Decl *getSingleDecl() const {


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

Re: [PATCH] D24518: Correct assert text in DeclGroup::getSingleDecl()

[Kirill Bobyrev via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL281525: Correct assert text in DeclGroup::getSingleDecl() 
(authored by omtcyfz).

Changed prior to commit:
  https://reviews.llvm.org/D24518?vs=71188&id=71414#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24518

Files:
  cfe/trunk/include/clang/AST/DeclGroup.h

Index: cfe/trunk/include/clang/AST/DeclGroup.h
===
--- cfe/trunk/include/clang/AST/DeclGroup.h
+++ cfe/trunk/include/clang/AST/DeclGroup.h
@@ -84,7 +84,7 @@
   bool isDeclGroup() const { return getKind() == DeclGroupKind; }
 
   Decl *getSingleDecl() {
-assert(isSingleDecl() && "Isn't a declgroup");
+assert(isSingleDecl() && "Isn't a single decl");
 return D;
   }
   const Decl *getSingleDecl() const {


Index: cfe/trunk/include/clang/AST/DeclGroup.h
===
--- cfe/trunk/include/clang/AST/DeclGroup.h
+++ cfe/trunk/include/clang/AST/DeclGroup.h
@@ -84,7 +84,7 @@
   bool isDeclGroup() const { return getKind() == DeclGroupKind; }
 
   Decl *getSingleDecl() {
-assert(isSingleDecl() && "Isn't a declgroup");
+assert(isSingleDecl() && "Isn't a single decl");
 return D;
   }
   const Decl *getSingleDecl() const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D23820: Do not warn about format strings that are indexed string literals.

[Stephen Hines via cfe-commits]

srhines added a comment.

I will take care of submitting this. Thanks for the reviews everybody!


https://reviews.llvm.org/D23820



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

Re: [PATCH] D24372: [libcxx] Sprinkle constexpr over compressed_pair

[Eric Fiselier via cfe-commits]

static_cast(t) also works as a constexpr C++11 forward.

On Sep 13, 2016 4:07 PM, "Keno Fischer" 
wrote:

> loladiro added inline comments.
>
> 
> Comment at: include/memory:2137
> @@ -2132,3 +2136,3 @@
>
> -_LIBCPP_INLINE_VISIBILITY
> +_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
>  __libcpp_compressed_pair_imp& operator=(__libcpp_compressed_pair_imp&&
> __p)
> 
> mclow.lists wrote:
> > loladiro wrote:
> > > mclow.lists wrote:
> > > > Have you tested this on C++11?
> > > > I suspect that some of these need to be
> `_LIBCPP_CONSTEXPR_AFTER_CXX11`
> > > Well, as I mentioned there are other problems on C++11, namely forward
> not being constexpr (so even the simple constructors below can't be
> constant initialized in C++11). I would REALLY like for unique_ptr to be
> constant initialized even in C++11 mode. I feel like arguably the standard
> requires it by marking the constructor `constexpr`.
> > One way to deal with that is to define a call `__forward()` that is the
> same as `forward()` except that it's constexpr in C++11, and have
> compressed_pair call that.
> >
> I was worried that something in the definition of forward required C++14
> constexpr semantics? Is that not the case. If so this sounds like a good
> solution, and I can update the patch accordingly.
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D24372
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D23820: Do not warn about format strings that are indexed string literals.

[Stephen Hines via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL281527: Do not warn about format strings that are indexed 
string literals. (authored by srhines).

Changed prior to commit:
  https://reviews.llvm.org/D23820?vs=71286&id=71416#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D23820

Files:
  cfe/trunk/lib/Sema/SemaChecking.cpp
  cfe/trunk/test/Sema/format-strings.c

Index: cfe/trunk/test/Sema/format-strings.c
===
--- cfe/trunk/test/Sema/format-strings.c
+++ cfe/trunk/test/Sema/format-strings.c
@@ -652,3 +652,38 @@
   // expected-note@-1{{treat the string as an argument to avoid this}}
 }
 #pragma GCC diagnostic warning "-Wformat-nonliteral"
+
+void test_char_pointer_arithmetic(int b) {
+  const char s1[] = "string";
+  const char s2[] = "%s string";
+
+  printf(s1 - 1);  // expected-warning {{format string is not a string literal (potentially insecure)}}
+  // expected-note@-1{{treat the string as an argument to avoid this}}
+
+  printf(s1 + 2);  // no-warning
+  printf(s2 + 2);  // no-warning
+
+  const char s3[] = "%s string";
+  printf((s3 + 2) - 2);  // expected-warning{{more '%' conversions than data arguments}}
+  // expected-note@-2{{format string is defined here}}
+  printf(2 + s2); // no-warning
+  printf(6 + s2 - 2); // no-warning
+  printf(2 + (b ? s1 : s2));  // no-warning
+
+  const char s5[] = "string %s";
+  printf(2 + (b ? s2 : s5));  // expected-warning{{more '%' conversions than data arguments}}
+  // expected-note@-2{{format string is defined here}}
+  printf(2 + (b ? s2 : s5), "");  // no-warning
+  printf(2 + (b ? s1 : s2 - 2), "");  // no-warning
+
+  const char s6[] = "%s string";
+  printf(2 + (b ? s1 : s6 - 2));  // expected-warning{{more '%' conversions than data arguments}}
+  // expected-note@-2{{format string is defined here}}
+  printf(1 ? s2 + 2 : s2);  // no-warning
+  printf(0 ? s2 : s2 + 2);  // no-warning
+  printf(2 + s2 + 5 * 3 - 16, "");  // expected-warning{{data argument not used}}
+
+  const char s7[] = "%s string %s %s";
+  printf(s7 + 3, "");  // expected-warning{{more '%' conversions than data arguments}}
+  // expected-note@-2{{format string is defined here}}
+}
Index: cfe/trunk/lib/Sema/SemaChecking.cpp
===
--- cfe/trunk/lib/Sema/SemaChecking.cpp
+++ cfe/trunk/lib/Sema/SemaChecking.cpp
@@ -3841,7 +3841,95 @@
 };
 } // end anonymous namespace
 
-static void CheckFormatString(Sema &S, const StringLiteral *FExpr,
+static void sumOffsets(llvm::APSInt &Offset, llvm::APSInt Addend,
+ BinaryOperatorKind BinOpKind,
+ bool AddendIsRight) {
+  unsigned BitWidth = Offset.getBitWidth();
+  unsigned AddendBitWidth = Addend.getBitWidth();
+  // There might be negative interim results.
+  if (Addend.isUnsigned()) {
+Addend = Addend.zext(++AddendBitWidth);
+Addend.setIsSigned(true);
+  }
+  // Adjust the bit width of the APSInts.
+  if (AddendBitWidth > BitWidth) {
+Offset = Offset.sext(AddendBitWidth);
+BitWidth = AddendBitWidth;
+  } else if (BitWidth > AddendBitWidth) {
+Addend = Addend.sext(BitWidth);
+  }
+
+  bool Ov = false;
+  llvm::APSInt ResOffset = Offset;
+  if (BinOpKind == BO_Add)
+ResOffset = Offset.sadd_ov(Addend, Ov);
+  else {
+assert(AddendIsRight && BinOpKind == BO_Sub &&
+   "operator must be add or sub with addend on the right");
+ResOffset = Offset.ssub_ov(Addend, Ov);
+  }
+
+  // We add an offset to a pointer here so we should support an offset as big as
+  // possible.
+  if (Ov) {
+assert(BitWidth <= UINT_MAX / 2 && "index (intermediate) result too big");
+Offset.sext(2 * BitWidth);
+sumOffsets(Offset, Addend, BinOpKind, AddendIsRight);
+return;
+  }
+
+  Offset = ResOffset;
+}
+
+namespace {
+// This is a wrapper class around StringLiteral to support offsetted string
+// literals as format strings. It takes the offset into account when returning
+// the string and its length or the source locations to display notes correctly.
+class FormatStringLiteral {
+  const StringLiteral *FExpr;
+  int64_t Offset;
+
+ public:
+  FormatStringLiteral(const StringLiteral *fexpr, int64_t Offset = 0)
+  : FExpr(fexpr), Offset(Offset) {}
+
+  StringRef getString() const {
+return FExpr->getString().drop_front(Offset);
+  }
+
+  unsigned getByteLength() const {
+return FExpr->getByteLength() - getCharByteWidth() * Offset;
+  }
+  unsigned getLength() const { return FExpr->getLength() - Offset; }
+  unsigned getCharByteWidth() const { return FExpr->getCharByteWidth(); }
+
+  StringLiteral::StringKind getKind() const { return FExpr->getKind(); }
+
+  QualType getType() const { return FExpr->getType(); }
+
+  bool isAscii() const { return FExpr->isAscii(); }
+  bool isWide() const { return FExpr->isWide(); }
+  bool isUTF8() const { retu

r281527 - Do not warn about format strings that are indexed string literals.

[Stephen Hines via cfe-commits]

Author: srhines
Date: Wed Sep 14 15:05:20 2016
New Revision: 281527

URL: http://llvm.org/viewvc/llvm-project?rev=281527&view=rev
Log:
Do not warn about format strings that are indexed string literals.

Summary:
The warning for a format string not being a sting literal and therefore
being potentially insecure is overly strict for indecies into sting
literals. This fix checks if the index into the string literal is
precomputable. If thats the case it will check if the suffix of that
sting literal is a valid format string string literal. It will still
issue the aforementioned warning for out of range indecies into the
string literal.

Reviewers: rsmith

Subscribers: srhines, cfe-commits

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

Modified:
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/Sema/format-strings.c

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=281527&r1=281526&r2=281527&view=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Wed Sep 14 15:05:20 2016
@@ -3841,7 +3841,95 @@ enum StringLiteralCheckType {
 };
 } // end anonymous namespace
 
-static void CheckFormatString(Sema &S, const StringLiteral *FExpr,
+static void sumOffsets(llvm::APSInt &Offset, llvm::APSInt Addend,
+ BinaryOperatorKind BinOpKind,
+ bool AddendIsRight) {
+  unsigned BitWidth = Offset.getBitWidth();
+  unsigned AddendBitWidth = Addend.getBitWidth();
+  // There might be negative interim results.
+  if (Addend.isUnsigned()) {
+Addend = Addend.zext(++AddendBitWidth);
+Addend.setIsSigned(true);
+  }
+  // Adjust the bit width of the APSInts.
+  if (AddendBitWidth > BitWidth) {
+Offset = Offset.sext(AddendBitWidth);
+BitWidth = AddendBitWidth;
+  } else if (BitWidth > AddendBitWidth) {
+Addend = Addend.sext(BitWidth);
+  }
+
+  bool Ov = false;
+  llvm::APSInt ResOffset = Offset;
+  if (BinOpKind == BO_Add)
+ResOffset = Offset.sadd_ov(Addend, Ov);
+  else {
+assert(AddendIsRight && BinOpKind == BO_Sub &&
+   "operator must be add or sub with addend on the right");
+ResOffset = Offset.ssub_ov(Addend, Ov);
+  }
+
+  // We add an offset to a pointer here so we should support an offset as big 
as
+  // possible.
+  if (Ov) {
+assert(BitWidth <= UINT_MAX / 2 && "index (intermediate) result too big");
+Offset.sext(2 * BitWidth);
+sumOffsets(Offset, Addend, BinOpKind, AddendIsRight);
+return;
+  }
+
+  Offset = ResOffset;
+}
+
+namespace {
+// This is a wrapper class around StringLiteral to support offsetted string
+// literals as format strings. It takes the offset into account when returning
+// the string and its length or the source locations to display notes 
correctly.
+class FormatStringLiteral {
+  const StringLiteral *FExpr;
+  int64_t Offset;
+
+ public:
+  FormatStringLiteral(const StringLiteral *fexpr, int64_t Offset = 0)
+  : FExpr(fexpr), Offset(Offset) {}
+
+  StringRef getString() const {
+return FExpr->getString().drop_front(Offset);
+  }
+
+  unsigned getByteLength() const {
+return FExpr->getByteLength() - getCharByteWidth() * Offset;
+  }
+  unsigned getLength() const { return FExpr->getLength() - Offset; }
+  unsigned getCharByteWidth() const { return FExpr->getCharByteWidth(); }
+
+  StringLiteral::StringKind getKind() const { return FExpr->getKind(); }
+
+  QualType getType() const { return FExpr->getType(); }
+
+  bool isAscii() const { return FExpr->isAscii(); }
+  bool isWide() const { return FExpr->isWide(); }
+  bool isUTF8() const { return FExpr->isUTF8(); }
+  bool isUTF16() const { return FExpr->isUTF16(); }
+  bool isUTF32() const { return FExpr->isUTF32(); }
+  bool isPascal() const { return FExpr->isPascal(); }
+
+  SourceLocation getLocationOfByte(
+  unsigned ByteNo, const SourceManager &SM, const LangOptions &Features,
+  const TargetInfo &Target, unsigned *StartToken = nullptr,
+  unsigned *StartTokenByteOffset = nullptr) const {
+return FExpr->getLocationOfByte(ByteNo + Offset, SM, Features, Target,
+StartToken, StartTokenByteOffset);
+  }
+
+  SourceLocation getLocStart() const LLVM_READONLY {
+return FExpr->getLocStart().getLocWithOffset(Offset);
+  }
+  SourceLocation getLocEnd() const LLVM_READONLY { return FExpr->getLocEnd(); }
+};
+}  // end anonymous namespace
+
+static void CheckFormatString(Sema &S, const FormatStringLiteral *FExpr,
   const Expr *OrigFormatExpr,
   ArrayRef Args,
   bool HasVAListArg, unsigned format_idx,
@@ -3862,8 +3950,11 @@ checkFormatStringExpr(Sema &S, const Exp
   unsigned firstDataArg, Sema::FormatStringType Type,
   Sema::Vari

[PATCH] D24579: Revert "Do not warn about format strings that are indexed string literals."

[Stephen Hines via cfe-commits]

srhines created this revision.
srhines added a subscriber: cfe-commits.

This reverts r281527 because I messed up the attribution.

https://reviews.llvm.org/D24579

Files:
  lib/Sema/SemaChecking.cpp
  test/Sema/format-strings.c

Index: test/Sema/format-strings.c
===
--- test/Sema/format-strings.c
+++ test/Sema/format-strings.c
@@ -652,38 +652,3 @@
   // expected-note@-1{{treat the string as an argument to avoid this}}
 }
 #pragma GCC diagnostic warning "-Wformat-nonliteral"
-
-void test_char_pointer_arithmetic(int b) {
-  const char s1[] = "string";
-  const char s2[] = "%s string";
-
-  printf(s1 - 1);  // expected-warning {{format string is not a string literal (potentially insecure)}}
-  // expected-note@-1{{treat the string as an argument to avoid this}}
-
-  printf(s1 + 2);  // no-warning
-  printf(s2 + 2);  // no-warning
-
-  const char s3[] = "%s string";
-  printf((s3 + 2) - 2);  // expected-warning{{more '%' conversions than data arguments}}
-  // expected-note@-2{{format string is defined here}}
-  printf(2 + s2); // no-warning
-  printf(6 + s2 - 2); // no-warning
-  printf(2 + (b ? s1 : s2));  // no-warning
-
-  const char s5[] = "string %s";
-  printf(2 + (b ? s2 : s5));  // expected-warning{{more '%' conversions than data arguments}}
-  // expected-note@-2{{format string is defined here}}
-  printf(2 + (b ? s2 : s5), "");  // no-warning
-  printf(2 + (b ? s1 : s2 - 2), "");  // no-warning
-
-  const char s6[] = "%s string";
-  printf(2 + (b ? s1 : s6 - 2));  // expected-warning{{more '%' conversions than data arguments}}
-  // expected-note@-2{{format string is defined here}}
-  printf(1 ? s2 + 2 : s2);  // no-warning
-  printf(0 ? s2 : s2 + 2);  // no-warning
-  printf(2 + s2 + 5 * 3 - 16, "");  // expected-warning{{data argument not used}}
-
-  const char s7[] = "%s string %s %s";
-  printf(s7 + 3, "");  // expected-warning{{more '%' conversions than data arguments}}
-  // expected-note@-2{{format string is defined here}}
-}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -3841,95 +3841,7 @@
 };
 } // end anonymous namespace
 
-static void sumOffsets(llvm::APSInt &Offset, llvm::APSInt Addend,
- BinaryOperatorKind BinOpKind,
- bool AddendIsRight) {
-  unsigned BitWidth = Offset.getBitWidth();
-  unsigned AddendBitWidth = Addend.getBitWidth();
-  // There might be negative interim results.
-  if (Addend.isUnsigned()) {
-Addend = Addend.zext(++AddendBitWidth);
-Addend.setIsSigned(true);
-  }
-  // Adjust the bit width of the APSInts.
-  if (AddendBitWidth > BitWidth) {
-Offset = Offset.sext(AddendBitWidth);
-BitWidth = AddendBitWidth;
-  } else if (BitWidth > AddendBitWidth) {
-Addend = Addend.sext(BitWidth);
-  }
-
-  bool Ov = false;
-  llvm::APSInt ResOffset = Offset;
-  if (BinOpKind == BO_Add)
-ResOffset = Offset.sadd_ov(Addend, Ov);
-  else {
-assert(AddendIsRight && BinOpKind == BO_Sub &&
-   "operator must be add or sub with addend on the right");
-ResOffset = Offset.ssub_ov(Addend, Ov);
-  }
-
-  // We add an offset to a pointer here so we should support an offset as big as
-  // possible.
-  if (Ov) {
-assert(BitWidth <= UINT_MAX / 2 && "index (intermediate) result too big");
-Offset.sext(2 * BitWidth);
-sumOffsets(Offset, Addend, BinOpKind, AddendIsRight);
-return;
-  }
-
-  Offset = ResOffset;
-}
-
-namespace {
-// This is a wrapper class around StringLiteral to support offsetted string
-// literals as format strings. It takes the offset into account when returning
-// the string and its length or the source locations to display notes correctly.
-class FormatStringLiteral {
-  const StringLiteral *FExpr;
-  int64_t Offset;
-
- public:
-  FormatStringLiteral(const StringLiteral *fexpr, int64_t Offset = 0)
-  : FExpr(fexpr), Offset(Offset) {}
-
-  StringRef getString() const {
-return FExpr->getString().drop_front(Offset);
-  }
-
-  unsigned getByteLength() const {
-return FExpr->getByteLength() - getCharByteWidth() * Offset;
-  }
-  unsigned getLength() const { return FExpr->getLength() - Offset; }
-  unsigned getCharByteWidth() const { return FExpr->getCharByteWidth(); }
-
-  StringLiteral::StringKind getKind() const { return FExpr->getKind(); }
-
-  QualType getType() const { return FExpr->getType(); }
-
-  bool isAscii() const { return FExpr->isAscii(); }
-  bool isWide() const { return FExpr->isWide(); }
-  bool isUTF8() const { return FExpr->isUTF8(); }
-  bool isUTF16() const { return FExpr->isUTF16(); }
-  bool isUTF32() const { return FExpr->isUTF32(); }
-  bool isPascal() const { return FExpr->isPascal(); }
-
-  SourceLocation getLocationOfByte(
-  unsigned ByteNo, const SourceManager &SM, const LangOptions &Features,
-  const T

Re: [PATCH] D24508: PR28752: Do not instantiate var decls which are not visible.

[Richard Smith via cfe-commits]

rsmith added a comment.

I think you'll also need to update the `ASTDeclReader` to merge `VarDecl` 
definitions together if it reads a definition and there already is one in the 
AST. I note that right now `Sema::AddInitializerToDecl` permits multiple 
definitions of a `VarDecl`, which doesn't seem like what we want here; we'll 
probably want to merge those as we parse them, too.

Now, we have a problem here: unlike with classes and functions, we can't always 
convert a variable definition to a declaration by just dropping the 
initializer. Perhaps we can add a flag to `VarDecl` to indicate "this is just a 
declaration, even though it looks like a definition", to handle that case? 
(This would also be useful for `VarTemplateSpecializationDecl`s, where we 
currently reject valid code such as "template int n; int k = n;" 
because we have no way to represent the difference between a mere declaration 
and a definition of `n`.)



Comment at: lib/Sema/SemaTemplate.cpp:505
@@ -499,3 +504,3 @@
 Instantiation->setInvalidDecl();
   } else if (InstantiatedFromMember) {
 if (isa(Instantiation)) {

Why do we not issue a diagnostic in this case for a `VarDecl` when `Complain` 
is true and no definition is available? It seems like we should either be 
diagnosing this or asserting that it can't happen.


Comment at: lib/Sema/SemaTemplate.cpp:528
@@ +527,3 @@
+  Note = diag::note_template_decl_here;
+} else if (isa(Instantiation)) {
+  if (isa(Instantiation)) {

`else` + `assert` would make more sense here. No other kind of declaration 
should get here.


Comment at: lib/Sema/SemaType.cpp:6898-6899
@@ +6897,4 @@
+  } else if (auto *VD = dyn_cast(D)) {
+// FIXME: Handle the case where D is a VarTemplateSpecializationDecl, i.e. 
D
+// is not a static data member.
+if (auto *Pattern = VD->getInstantiatedFromStaticDataMember())

We should add a `getTemplateInstantiationPattern()` to `VarDecl` and use it 
from here.


https://reviews.llvm.org/D24508



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

r281530 - Revert "Do not warn about format strings that are indexed string literals."

[Stephen Hines via cfe-commits]

Author: srhines
Date: Wed Sep 14 15:20:14 2016
New Revision: 281530

URL: http://llvm.org/viewvc/llvm-project?rev=281530&view=rev
Log:
Revert "Do not warn about format strings that are indexed string literals."

Summary: This reverts r281527 because I messed up the attribution.

Reviewers: srhines

Subscribers: cfe-commits

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

Modified:
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/Sema/format-strings.c

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=281530&r1=281529&r2=281530&view=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Wed Sep 14 15:20:14 2016
@@ -3841,95 +3841,7 @@ enum StringLiteralCheckType {
 };
 } // end anonymous namespace
 
-static void sumOffsets(llvm::APSInt &Offset, llvm::APSInt Addend,
- BinaryOperatorKind BinOpKind,
- bool AddendIsRight) {
-  unsigned BitWidth = Offset.getBitWidth();
-  unsigned AddendBitWidth = Addend.getBitWidth();
-  // There might be negative interim results.
-  if (Addend.isUnsigned()) {
-Addend = Addend.zext(++AddendBitWidth);
-Addend.setIsSigned(true);
-  }
-  // Adjust the bit width of the APSInts.
-  if (AddendBitWidth > BitWidth) {
-Offset = Offset.sext(AddendBitWidth);
-BitWidth = AddendBitWidth;
-  } else if (BitWidth > AddendBitWidth) {
-Addend = Addend.sext(BitWidth);
-  }
-
-  bool Ov = false;
-  llvm::APSInt ResOffset = Offset;
-  if (BinOpKind == BO_Add)
-ResOffset = Offset.sadd_ov(Addend, Ov);
-  else {
-assert(AddendIsRight && BinOpKind == BO_Sub &&
-   "operator must be add or sub with addend on the right");
-ResOffset = Offset.ssub_ov(Addend, Ov);
-  }
-
-  // We add an offset to a pointer here so we should support an offset as big 
as
-  // possible.
-  if (Ov) {
-assert(BitWidth <= UINT_MAX / 2 && "index (intermediate) result too big");
-Offset.sext(2 * BitWidth);
-sumOffsets(Offset, Addend, BinOpKind, AddendIsRight);
-return;
-  }
-
-  Offset = ResOffset;
-}
-
-namespace {
-// This is a wrapper class around StringLiteral to support offsetted string
-// literals as format strings. It takes the offset into account when returning
-// the string and its length or the source locations to display notes 
correctly.
-class FormatStringLiteral {
-  const StringLiteral *FExpr;
-  int64_t Offset;
-
- public:
-  FormatStringLiteral(const StringLiteral *fexpr, int64_t Offset = 0)
-  : FExpr(fexpr), Offset(Offset) {}
-
-  StringRef getString() const {
-return FExpr->getString().drop_front(Offset);
-  }
-
-  unsigned getByteLength() const {
-return FExpr->getByteLength() - getCharByteWidth() * Offset;
-  }
-  unsigned getLength() const { return FExpr->getLength() - Offset; }
-  unsigned getCharByteWidth() const { return FExpr->getCharByteWidth(); }
-
-  StringLiteral::StringKind getKind() const { return FExpr->getKind(); }
-
-  QualType getType() const { return FExpr->getType(); }
-
-  bool isAscii() const { return FExpr->isAscii(); }
-  bool isWide() const { return FExpr->isWide(); }
-  bool isUTF8() const { return FExpr->isUTF8(); }
-  bool isUTF16() const { return FExpr->isUTF16(); }
-  bool isUTF32() const { return FExpr->isUTF32(); }
-  bool isPascal() const { return FExpr->isPascal(); }
-
-  SourceLocation getLocationOfByte(
-  unsigned ByteNo, const SourceManager &SM, const LangOptions &Features,
-  const TargetInfo &Target, unsigned *StartToken = nullptr,
-  unsigned *StartTokenByteOffset = nullptr) const {
-return FExpr->getLocationOfByte(ByteNo + Offset, SM, Features, Target,
-StartToken, StartTokenByteOffset);
-  }
-
-  SourceLocation getLocStart() const LLVM_READONLY {
-return FExpr->getLocStart().getLocWithOffset(Offset);
-  }
-  SourceLocation getLocEnd() const LLVM_READONLY { return FExpr->getLocEnd(); }
-};
-}  // end anonymous namespace
-
-static void CheckFormatString(Sema &S, const FormatStringLiteral *FExpr,
+static void CheckFormatString(Sema &S, const StringLiteral *FExpr,
   const Expr *OrigFormatExpr,
   ArrayRef Args,
   bool HasVAListArg, unsigned format_idx,
@@ -3950,11 +3862,8 @@ checkFormatStringExpr(Sema &S, const Exp
   unsigned firstDataArg, Sema::FormatStringType Type,
   Sema::VariadicCallType CallType, bool InFunctionCall,
   llvm::SmallBitVector &CheckedVarArgs,
-  UncoveredArgHandler &UncoveredArg,
-  llvm::APSInt Offset) {
+  UncoveredArgHandler &UncoveredArg) {
  tryAgain:
-  assert(Offset.isSigned() && "invalid offset");
-
   if (E->isTypeDependent() || E->is

Re: [PATCH] D24579: Revert "Do not warn about format strings that are indexed string literals."

[Stephen Hines via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL281530: Revert "Do not warn about format strings that are 
indexed string literals." (authored by srhines).

Changed prior to commit:
  https://reviews.llvm.org/D24579?vs=71417&id=71419#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24579

Files:
  cfe/trunk/lib/Sema/SemaChecking.cpp
  cfe/trunk/test/Sema/format-strings.c

Index: cfe/trunk/lib/Sema/SemaChecking.cpp
===
--- cfe/trunk/lib/Sema/SemaChecking.cpp
+++ cfe/trunk/lib/Sema/SemaChecking.cpp
@@ -3841,95 +3841,7 @@
 };
 } // end anonymous namespace
 
-static void sumOffsets(llvm::APSInt &Offset, llvm::APSInt Addend,
- BinaryOperatorKind BinOpKind,
- bool AddendIsRight) {
-  unsigned BitWidth = Offset.getBitWidth();
-  unsigned AddendBitWidth = Addend.getBitWidth();
-  // There might be negative interim results.
-  if (Addend.isUnsigned()) {
-Addend = Addend.zext(++AddendBitWidth);
-Addend.setIsSigned(true);
-  }
-  // Adjust the bit width of the APSInts.
-  if (AddendBitWidth > BitWidth) {
-Offset = Offset.sext(AddendBitWidth);
-BitWidth = AddendBitWidth;
-  } else if (BitWidth > AddendBitWidth) {
-Addend = Addend.sext(BitWidth);
-  }
-
-  bool Ov = false;
-  llvm::APSInt ResOffset = Offset;
-  if (BinOpKind == BO_Add)
-ResOffset = Offset.sadd_ov(Addend, Ov);
-  else {
-assert(AddendIsRight && BinOpKind == BO_Sub &&
-   "operator must be add or sub with addend on the right");
-ResOffset = Offset.ssub_ov(Addend, Ov);
-  }
-
-  // We add an offset to a pointer here so we should support an offset as big as
-  // possible.
-  if (Ov) {
-assert(BitWidth <= UINT_MAX / 2 && "index (intermediate) result too big");
-Offset.sext(2 * BitWidth);
-sumOffsets(Offset, Addend, BinOpKind, AddendIsRight);
-return;
-  }
-
-  Offset = ResOffset;
-}
-
-namespace {
-// This is a wrapper class around StringLiteral to support offsetted string
-// literals as format strings. It takes the offset into account when returning
-// the string and its length or the source locations to display notes correctly.
-class FormatStringLiteral {
-  const StringLiteral *FExpr;
-  int64_t Offset;
-
- public:
-  FormatStringLiteral(const StringLiteral *fexpr, int64_t Offset = 0)
-  : FExpr(fexpr), Offset(Offset) {}
-
-  StringRef getString() const {
-return FExpr->getString().drop_front(Offset);
-  }
-
-  unsigned getByteLength() const {
-return FExpr->getByteLength() - getCharByteWidth() * Offset;
-  }
-  unsigned getLength() const { return FExpr->getLength() - Offset; }
-  unsigned getCharByteWidth() const { return FExpr->getCharByteWidth(); }
-
-  StringLiteral::StringKind getKind() const { return FExpr->getKind(); }
-
-  QualType getType() const { return FExpr->getType(); }
-
-  bool isAscii() const { return FExpr->isAscii(); }
-  bool isWide() const { return FExpr->isWide(); }
-  bool isUTF8() const { return FExpr->isUTF8(); }
-  bool isUTF16() const { return FExpr->isUTF16(); }
-  bool isUTF32() const { return FExpr->isUTF32(); }
-  bool isPascal() const { return FExpr->isPascal(); }
-
-  SourceLocation getLocationOfByte(
-  unsigned ByteNo, const SourceManager &SM, const LangOptions &Features,
-  const TargetInfo &Target, unsigned *StartToken = nullptr,
-  unsigned *StartTokenByteOffset = nullptr) const {
-return FExpr->getLocationOfByte(ByteNo + Offset, SM, Features, Target,
-StartToken, StartTokenByteOffset);
-  }
-
-  SourceLocation getLocStart() const LLVM_READONLY {
-return FExpr->getLocStart().getLocWithOffset(Offset);
-  }
-  SourceLocation getLocEnd() const LLVM_READONLY { return FExpr->getLocEnd(); }
-};
-}  // end anonymous namespace
-
-static void CheckFormatString(Sema &S, const FormatStringLiteral *FExpr,
+static void CheckFormatString(Sema &S, const StringLiteral *FExpr,
   const Expr *OrigFormatExpr,
   ArrayRef Args,
   bool HasVAListArg, unsigned format_idx,
@@ -3950,11 +3862,8 @@
   unsigned firstDataArg, Sema::FormatStringType Type,
   Sema::VariadicCallType CallType, bool InFunctionCall,
   llvm::SmallBitVector &CheckedVarArgs,
-  UncoveredArgHandler &UncoveredArg,
-  llvm::APSInt Offset) {
+  UncoveredArgHandler &UncoveredArg) {
  tryAgain:
-  assert(Offset.isSigned() && "invalid offset");
-
   if (E->isTypeDependent() || E->isValueDependent())
 return SLCT_NotALiteral;
 
@@ -3988,28 +3897,23 @@
 CheckLeft = false;
 }
 
-// We need to maintain the offsets for the right and the left hand side
-// separately to check if every possible indexed expression is a valid
-// string literal. The

[PATCH] D24581: [CUDA] Add test checking our ability to take a function pointer to a __global__ function on the host side.

[Justin Lebar via cfe-commits]

jlebar created this revision.
jlebar added a reviewer: tra.
jlebar added a subscriber: cfe-commits.

This functionality is used by Thrust.

https://reviews.llvm.org/D24581

Files:
  clang/test/SemaCUDA/reference-to-kernel-fn.cu

Index: clang/test/SemaCUDA/reference-to-kernel-fn.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/reference-to-kernel-fn.cu
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++11 -fcuda-is-device -fsyntax-only -verify -DDEVICE 
%s
+
+// Check that we can reference (get a function pointer to) a __global__
+// function from the host side, but not the device side.  (We don't yet support
+// device-side kernel launches.)
+
+#include "Inputs/cuda.h"
+
+struct Dummy {};
+
+__global__ void kernel() {}
+// expected-note@-1 {{declared here}}
+#ifdef DEVICE
+// expected-note@-3 {{declared here}}
+#endif
+
+typedef void (*fn_ptr_t)();
+
+__host__ __device__ fn_ptr_t get_ptr_hd() {
+  return kernel;
+#ifdef DEVICE
+  // expected-error@-2 {{reference to __global__ function}}
+#endif
+}
+__host__ fn_ptr_t get_ptr_h() {
+  return kernel;
+}
+__device__ fn_ptr_t get_ptr_d() {
+  return kernel;  // expected-error {{reference to __global__ function}}
+}


Index: clang/test/SemaCUDA/reference-to-kernel-fn.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/reference-to-kernel-fn.cu
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++11 -fcuda-is-device -fsyntax-only -verify -DDEVICE %s
+
+// Check that we can reference (get a function pointer to) a __global__
+// function from the host side, but not the device side.  (We don't yet support
+// device-side kernel launches.)
+
+#include "Inputs/cuda.h"
+
+struct Dummy {};
+
+__global__ void kernel() {}
+// expected-note@-1 {{declared here}}
+#ifdef DEVICE
+// expected-note@-3 {{declared here}}
+#endif
+
+typedef void (*fn_ptr_t)();
+
+__host__ __device__ fn_ptr_t get_ptr_hd() {
+  return kernel;
+#ifdef DEVICE
+  // expected-error@-2 {{reference to __global__ function}}
+#endif
+}
+__host__ fn_ptr_t get_ptr_h() {
+  return kernel;
+}
+__device__ fn_ptr_t get_ptr_d() {
+  return kernel;  // expected-error {{reference to __global__ function}}
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24467: Fix an error after D21678

[Akira Hatanaka via cfe-commits]

ahatanak added inline comments.


Comment at: llvm/tools/clang/test/Sema/vecshift.c:56
@@ +55,3 @@
+
+  vc4 = vc4 << vc8; // expected-error {{vector operands do not have the same 
number of elements}}
+  vi4 = vi4 << vuc8; // expected-error {{vector operands do not have the same 
number of elements}}

vbyakovlcl wrote:
> ahatanak wrote:
> > I don't think this patch was necessary to have clang print diagnostic 
> > "vector operands do not have the same number of elements"? If that's the 
> > case, I think it's better to add these lines in a separate commit.
> Do you propose to delete lines with this diagnostic?
Yes, I think you can move them to a separate patch. Doing so will help people 
who read the patch understand what problems you are trying to fix with this 
patch.


https://reviews.llvm.org/D24467



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

Re: [PATCH] D21803: [libcxxabi] Provide a fallback __cxa_thread_atexit() implementation

[Eric Fiselier via cfe-commits]

EricWF added a comment.

LGTM after addressing the inline comments.



Comment at: src/cxa_thread_atexit.cpp:70
@@ +69,3 @@
+while (auto head = dtors) {
+  dtors = head->next;
+  head->dtor(head->obj);

tavianator wrote:
> EricWF wrote:
> > tavianator wrote:
> > > EricWF wrote:
> > > > There is a bug here. If `head->next == nullptr` and if 
> > > > `head->dtor(head->obj))` creates a TL variable in the destructor then 
> > > > that destructor will not be invoked.
> > > > 
> > > > Here's an updated test case which catches the bug: 
> > > > https://gist.github.com/EricWF/3bb50d4f28b91aa28d2adefea0e94a0e
> > > I can't reproduce that failure here, your exact test case passes (even 
> > > with `#undef HAVE___CXA_THREAD_ATEXIT_IMPL` and the weak symbol test 
> > > commented out).
> > > 
> > > Tracing the implementation logic, it seems correct.  If `head->next == 
> > > nullptr` then this line does `dtors = nullptr`.  Then if 
> > > `head->dtor(head->obj)` registers a new `thread_local`, 
> > > `__cxa_thread_atexit()` does `head = malloc(...); ... dtors = head;`.  
> > > Then the next iteration of the loop `while (auto head = dtors) {` picks 
> > > up that new node.
> > > 
> > > Have I missed something?
> > I can't reproduce this morning either, I must have been doing something 
> > funny. I'll look at this with a fresh head tomorrow. If I can't find 
> > anything this will be good to go. Thanks for working on this. 
> No problem!  I can integrate your updated test case anyway if you want.
Yeah I would like to see the upgraded test case applied. At least that way 
we're testing the case in question.

So I agree with your above analysis of what happens, and that all destructors 
are correctly called during the first iteration of pthread key destruction. My 
one issues is that we still register a new non-null key which forces pthread to 
run the destructor for the key again. I would like to see this fixed.


https://reviews.llvm.org/D21803



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

Re: [PATCH] D24581: [CUDA] Add test checking our ability to take a function pointer to a __global__ function on the host side.

[Artem Belevich via cfe-commits]

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

LGTM.


https://reviews.llvm.org/D24581



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

[PATCH] D24584: Do not warn about format strings that are indexed string literals.

[Meike Baumgärtner via cfe-commits]

meikeb created this revision.
meikeb added a reviewer: rsmith.
meikeb added subscribers: cfe-commits, srhines.

The warning for a format string not being a sting literal and therefore
being potentially insecure is overly strict for indecies into sting
literals. This fix checks if the index into the string literal is
precomputable. If thats the case it will check if the suffix of that
sting literal is a valid format string string literal. It will still
issue the aforementioned warning for out of range indecies into the
string literal.

Patch by Meike Baumgärtner (meikeb)

https://reviews.llvm.org/D24584

Files:
  lib/Sema/SemaChecking.cpp
  test/Sema/format-strings.c

Index: test/Sema/format-strings.c
===
--- test/Sema/format-strings.c
+++ test/Sema/format-strings.c
@@ -652,3 +652,38 @@
   // expected-note@-1{{treat the string as an argument to avoid this}}
 }
 #pragma GCC diagnostic warning "-Wformat-nonliteral"
+
+void test_char_pointer_arithmetic(int b) {
+  const char s1[] = "string";
+  const char s2[] = "%s string";
+
+  printf(s1 - 1);  // expected-warning {{format string is not a string literal (potentially insecure)}}
+  // expected-note@-1{{treat the string as an argument to avoid this}}
+
+  printf(s1 + 2);  // no-warning
+  printf(s2 + 2);  // no-warning
+
+  const char s3[] = "%s string";
+  printf((s3 + 2) - 2);  // expected-warning{{more '%' conversions than data arguments}}
+  // expected-note@-2{{format string is defined here}}
+  printf(2 + s2); // no-warning
+  printf(6 + s2 - 2); // no-warning
+  printf(2 + (b ? s1 : s2));  // no-warning
+
+  const char s5[] = "string %s";
+  printf(2 + (b ? s2 : s5));  // expected-warning{{more '%' conversions than data arguments}}
+  // expected-note@-2{{format string is defined here}}
+  printf(2 + (b ? s2 : s5), "");  // no-warning
+  printf(2 + (b ? s1 : s2 - 2), "");  // no-warning
+
+  const char s6[] = "%s string";
+  printf(2 + (b ? s1 : s6 - 2));  // expected-warning{{more '%' conversions than data arguments}}
+  // expected-note@-2{{format string is defined here}}
+  printf(1 ? s2 + 2 : s2);  // no-warning
+  printf(0 ? s2 : s2 + 2);  // no-warning
+  printf(2 + s2 + 5 * 3 - 16, "");  // expected-warning{{data argument not used}}
+
+  const char s7[] = "%s string %s %s";
+  printf(s7 + 3, "");  // expected-warning{{more '%' conversions than data arguments}}
+  // expected-note@-2{{format string is defined here}}
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -3839,7 +3839,95 @@
 };
 } // end anonymous namespace
 
-static void CheckFormatString(Sema &S, const StringLiteral *FExpr,
+static void sumOffsets(llvm::APSInt &Offset, llvm::APSInt Addend,
+ BinaryOperatorKind BinOpKind,
+ bool AddendIsRight) {
+  unsigned BitWidth = Offset.getBitWidth();
+  unsigned AddendBitWidth = Addend.getBitWidth();
+  // There might be negative interim results.
+  if (Addend.isUnsigned()) {
+Addend = Addend.zext(++AddendBitWidth);
+Addend.setIsSigned(true);
+  }
+  // Adjust the bit width of the APSInts.
+  if (AddendBitWidth > BitWidth) {
+Offset = Offset.sext(AddendBitWidth);
+BitWidth = AddendBitWidth;
+  } else if (BitWidth > AddendBitWidth) {
+Addend = Addend.sext(BitWidth);
+  }
+
+  bool Ov = false;
+  llvm::APSInt ResOffset = Offset;
+  if (BinOpKind == BO_Add)
+ResOffset = Offset.sadd_ov(Addend, Ov);
+  else {
+assert(AddendIsRight && BinOpKind == BO_Sub &&
+   "operator must be add or sub with addend on the right");
+ResOffset = Offset.ssub_ov(Addend, Ov);
+  }
+
+  // We add an offset to a pointer here so we should support an offset as big as
+  // possible.
+  if (Ov) {
+assert(BitWidth <= UINT_MAX / 2 && "index (intermediate) result too big");
+Offset.sext(2 * BitWidth);
+sumOffsets(Offset, Addend, BinOpKind, AddendIsRight);
+return;
+  }
+
+  Offset = ResOffset;
+}
+
+namespace {
+// This is a wrapper class around StringLiteral to support offsetted string
+// literals as format strings. It takes the offset into account when returning
+// the string and its length or the source locations to display notes correctly.
+class FormatStringLiteral {
+  const StringLiteral *FExpr;
+  int64_t Offset;
+
+ public:
+  FormatStringLiteral(const StringLiteral *fexpr, int64_t Offset = 0)
+  : FExpr(fexpr), Offset(Offset) {}
+
+  StringRef getString() const {
+return FExpr->getString().drop_front(Offset);
+  }
+
+  unsigned getByteLength() const {
+return FExpr->getByteLength() - getCharByteWidth() * Offset;
+  }
+  unsigned getLength() const { return FExpr->getLength() - Offset; }
+  unsigned getCharByteWidth() const { return FExpr->getCharByteWidth(); }
+
+  StringLiteral::StringKind getKind() const { return FExpr->getKind

Re: [PATCH] D24330: Add some MS aliases for existing intrinsics

[Albert Gutowski via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL281540: Add some MS aliases for existing intrinsics 
(authored by agutowski).

Changed prior to commit:
  https://reviews.llvm.org/D24330?vs=71404&id=71437#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24330

Files:
  cfe/trunk/include/clang/Basic/Builtins.def
  cfe/trunk/include/clang/Basic/Builtins.h
  cfe/trunk/include/clang/Basic/BuiltinsX86.def
  cfe/trunk/lib/Basic/Targets.cpp
  cfe/trunk/lib/CodeGen/CGBuiltin.cpp
  cfe/trunk/lib/Headers/emmintrin.h
  cfe/trunk/lib/Headers/ia32intrin.h
  cfe/trunk/lib/Headers/intrin.h
  cfe/trunk/lib/Headers/xmmintrin.h
  cfe/trunk/lib/Sema/SemaDecl.cpp
  cfe/trunk/test/CodeGen/builtins-x86.c
  cfe/trunk/test/Headers/x86intrin.cpp
  cfe/trunk/test/Sema/implicit-intel-builtin-decl.c
  cfe/trunk/test/Sema/implicit-ms-builtin-decl.c

Index: cfe/trunk/include/clang/Basic/BuiltinsX86.def
===
--- cfe/trunk/include/clang/Basic/BuiltinsX86.def
+++ cfe/trunk/include/clang/Basic/BuiltinsX86.def
@@ -23,6 +23,10 @@
 #   define TARGET_BUILTIN(ID, TYPE, ATTRS, FEATURE) BUILTIN(ID, TYPE, ATTRS)
 #endif
 
+#if defined(BUILTIN) && !defined(TARGET_HEADER_BUILTIN)
+#  define TARGET_HEADER_BUILTIN(ID, TYPE, ATTRS, HEADER, LANG, FEATURE) BUILTIN(ID, TYPE, ATTRS)
+#endif
+
 // FIXME: Are these nothrow/const?
 
 // Miscellaneous builtin for checking x86 cpu features.
@@ -301,15 +305,18 @@
 TARGET_BUILTIN(__builtin_ia32_pabsd128, "V4iV4i", "", "ssse3")
 
 TARGET_BUILTIN(__builtin_ia32_ldmxcsr, "vUi", "", "sse")
+TARGET_HEADER_BUILTIN(_mm_setcsr, "vUi", "h","xmmintrin.h", ALL_LANGUAGES, "sse")
 TARGET_BUILTIN(__builtin_ia32_stmxcsr, "Ui", "", "sse")
+TARGET_HEADER_BUILTIN(_mm_getcsr, "Ui", "h", "xmmintrin.h", ALL_LANGUAGES, "sse")
 TARGET_BUILTIN(__builtin_ia32_cvtss2si, "iV4f", "", "sse")
 TARGET_BUILTIN(__builtin_ia32_cvttss2si, "iV4f", "", "sse")
 TARGET_BUILTIN(__builtin_ia32_cvtss2si64, "LLiV4f", "", "sse")
 TARGET_BUILTIN(__builtin_ia32_cvttss2si64, "LLiV4f", "", "sse")
 TARGET_BUILTIN(__builtin_ia32_storehps, "vV2i*V4f", "", "sse")
 TARGET_BUILTIN(__builtin_ia32_storelps, "vV2i*V4f", "", "sse")
 TARGET_BUILTIN(__builtin_ia32_movmskps, "iV4f", "", "sse")
 TARGET_BUILTIN(__builtin_ia32_sfence, "v", "", "sse")
+TARGET_HEADER_BUILTIN(_mm_sfence, "v", "h", "xmmintrin.h", ALL_LANGUAGES, "sse")
 TARGET_BUILTIN(__builtin_ia32_rcpps, "V4fV4f", "", "sse")
 TARGET_BUILTIN(__builtin_ia32_rcpss, "V4fV4f", "", "sse")
 TARGET_BUILTIN(__builtin_ia32_rsqrtps, "V4fV4f", "", "sse")
@@ -337,9 +344,13 @@
 TARGET_BUILTIN(__builtin_ia32_cvtps2dq, "V4iV4f", "", "sse2")
 TARGET_BUILTIN(__builtin_ia32_cvttps2dq, "V4iV4f", "", "sse2")
 TARGET_BUILTIN(__builtin_ia32_clflush, "vvC*", "", "sse2")
+TARGET_HEADER_BUILTIN(_mm_clflush, "vvC*", "h", "emmintrin.h", ALL_LANGUAGES, "sse2")
 TARGET_BUILTIN(__builtin_ia32_lfence, "v", "", "sse2")
+TARGET_HEADER_BUILTIN(_mm_lfence, "v", "h", "emmintrin.h", ALL_LANGUAGES, "sse2")
 TARGET_BUILTIN(__builtin_ia32_mfence, "v", "", "sse2")
+TARGET_HEADER_BUILTIN(_mm_mfence, "v", "h", "emmintrin.h", ALL_LANGUAGES, "sse2")
 TARGET_BUILTIN(__builtin_ia32_pause, "v", "", "sse2")
+TARGET_HEADER_BUILTIN(_mm_pause, "v", "h", "emmintrin.h", ALL_LANGUAGES, "sse2")
 TARGET_BUILTIN(__builtin_ia32_pmuludq128, "V2LLiV4iV4i", "", "sse2")
 TARGET_BUILTIN(__builtin_ia32_psraw128, "V8sV8sV8s", "", "sse2")
 TARGET_BUILTIN(__builtin_ia32_psrad128, "V4iV4iV4i", "", "sse2")
@@ -894,6 +905,7 @@
 
 BUILTIN(__builtin_ia32_rdpmc, "ULLii", "")
 BUILTIN(__builtin_ia32_rdtsc, "ULLi", "")
+BUILTIN(__rdtsc, "ULLi", "")
 BUILTIN(__builtin_ia32_rdtscp, "ULLiUi*", "")
 // PKU
 TARGET_BUILTIN(__builtin_ia32_rdpkru, "Ui", "", "pku")
@@ -2059,3 +2071,4 @@
 
 #undef BUILTIN
 #undef TARGET_BUILTIN
+#undef TARGET_HEADER_BUILTIN
Index: cfe/trunk/include/clang/Basic/Builtins.def
===
--- cfe/trunk/include/clang/Basic/Builtins.def
+++ cfe/trunk/include/clang/Basic/Builtins.def
@@ -74,6 +74,7 @@
 //  f -> this is a libc/libm function without the '__builtin_' prefix. It can
 //   be followed by ':headername:' to state which header this function
 //   comes from.
+//  h -> this function requires a specific header or an explicit declaration.
 //  i -> this is a runtime library implemented function without the
 //   '__builtin_' prefix. It will be implemented in compiler-rt or libgcc.
 //  p:N: -> this is a printf-like function whose Nth argument is the format
@@ -708,6 +709,9 @@
 // Microsoft builtins.  These are only active with -fms-extensions.
 LANGBUILTIN(_alloca,  "v*z", "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(__assume, "vb",  "n", ALL_MS_LANGUAGES)
+LIBBUILTIN(_byteswap_ushort, "UsUs", "fnc", "stdlib.h", ALL_MS_LANGUAGES)
+LIBBUILTIN(_byteswap_ulong,  "ULiULi",   "fnc", "stdlib.h", ALL_MS_LANGUAGES)
+LIBBUILTIN(_byteswap_uint64, "ULLiULLi", "fnc", "stdlib.h", ALL_MS_LA

r281540 - Add some MS aliases for existing intrinsics

[Albert Gutowski via cfe-commits]

Author: agutowski
Date: Wed Sep 14 16:19:43 2016
New Revision: 281540

URL: http://llvm.org/viewvc/llvm-project?rev=281540&view=rev
Log:
Add some MS aliases for existing intrinsics

Reviewers: thakis, compnerd, majnemer, rsmith, rnk

Subscribers: alexshap, cfe-commits

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

Added:
cfe/trunk/test/Headers/x86intrin.cpp
cfe/trunk/test/Sema/implicit-intel-builtin-decl.c
cfe/trunk/test/Sema/implicit-ms-builtin-decl.c
Modified:
cfe/trunk/include/clang/Basic/Builtins.def
cfe/trunk/include/clang/Basic/Builtins.h
cfe/trunk/include/clang/Basic/BuiltinsX86.def
cfe/trunk/lib/Basic/Targets.cpp
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/Headers/emmintrin.h
cfe/trunk/lib/Headers/ia32intrin.h
cfe/trunk/lib/Headers/intrin.h
cfe/trunk/lib/Headers/xmmintrin.h
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/test/CodeGen/builtins-x86.c

Modified: cfe/trunk/include/clang/Basic/Builtins.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=281540&r1=281539&r2=281540&view=diff
==
--- cfe/trunk/include/clang/Basic/Builtins.def (original)
+++ cfe/trunk/include/clang/Basic/Builtins.def Wed Sep 14 16:19:43 2016
@@ -74,6 +74,7 @@
 //  f -> this is a libc/libm function without the '__builtin_' prefix. It can
 //   be followed by ':headername:' to state which header this function
 //   comes from.
+//  h -> this function requires a specific header or an explicit declaration.
 //  i -> this is a runtime library implemented function without the
 //   '__builtin_' prefix. It will be implemented in compiler-rt or libgcc.
 //  p:N: -> this is a printf-like function whose Nth argument is the format
@@ -708,6 +709,9 @@ BUILTIN(__builtin_rindex, "c*cC*i", "Fn"
 // Microsoft builtins.  These are only active with -fms-extensions.
 LANGBUILTIN(_alloca,  "v*z", "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(__assume, "vb",  "n", ALL_MS_LANGUAGES)
+LIBBUILTIN(_byteswap_ushort, "UsUs", "fnc", "stdlib.h", ALL_MS_LANGUAGES)
+LIBBUILTIN(_byteswap_ulong,  "ULiULi",   "fnc", "stdlib.h", ALL_MS_LANGUAGES)
+LIBBUILTIN(_byteswap_uint64, "ULLiULLi", "fnc", "stdlib.h", ALL_MS_LANGUAGES)
 LANGBUILTIN(__debugbreak, "v",   "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(__exception_code, "ULi", "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(_exception_code,  "ULi", "n", ALL_MS_LANGUAGES)
@@ -745,6 +749,9 @@ LANGBUILTIN(_InterlockedXor8,  "ccD*c",
 LANGBUILTIN(_InterlockedXor16, "ssD*s",   "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(_InterlockedXor,   "LiLiD*Li","n", ALL_MS_LANGUAGES)
 LANGBUILTIN(__noop,   "i.",  "n", ALL_MS_LANGUAGES)
+LANGBUILTIN(__popcnt16, "UsUs", "nc", ALL_MS_LANGUAGES)
+LANGBUILTIN(__popcnt,   "UiUi", "nc", ALL_MS_LANGUAGES)
+LANGBUILTIN(__popcnt64, "ULLiULLi", "nc", ALL_MS_LANGUAGES)
 LANGBUILTIN(__readfsdword,"ULiULi", "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(_rotl8,  "UcUcUc","n", ALL_MS_LANGUAGES)
 LANGBUILTIN(_rotl16, "UsUsUc","n", ALL_MS_LANGUAGES)

Modified: cfe/trunk/include/clang/Basic/Builtins.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.h?rev=281540&r1=281539&r2=281540&view=diff
==
--- cfe/trunk/include/clang/Basic/Builtins.h (original)
+++ cfe/trunk/include/clang/Basic/Builtins.h Wed Sep 14 16:19:43 2016
@@ -139,6 +139,13 @@ public:
 return strchr(getRecord(ID).Attributes, 'f') != nullptr;
   }
 
+  /// \brief Returns true if this builtin requires appropriate header in other
+  /// compilers. In Clang it will work even without including it, but we can 
emit
+  /// a warning about missing header.
+  bool isHeaderDependentFunction(unsigned ID) const {
+return strchr(getRecord(ID).Attributes, 'h') != nullptr;
+  }
+
   /// \brief Determines whether this builtin is a predefined compiler-rt/libgcc
   /// function, such as "__clear_cache", where we know the signature a
   /// priori.

Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=281540&r1=281539&r2=281540&view=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Wed Sep 14 16:19:43 2016
@@ -23,6 +23,10 @@
 #   define TARGET_BUILTIN(ID, TYPE, ATTRS, FEATURE) BUILTIN(ID, TYPE, ATTRS)
 #endif
 
+#if defined(BUILTIN) && !defined(TARGET_HEADER_BUILTIN)
+#  define TARGET_HEADER_BUILTIN(ID, TYPE, ATTRS, HEADER, LANG, FEATURE) 
BUILTIN(ID, TYPE, ATTRS)
+#endif
+
 // FIXME: Are these nothrow/const?
 
 // Miscellaneous builtin for checking x86 cpu features.
@@ -301,7 +305,9 @@ TARGET_BUILTIN(__builtin_ia32_pabsw128,
 TARGET_BUILTIN(__builtin_ia32_pabsd128, "V4iV4i", "", "ssse3")
 
 TARGET_BU