[PATCH] D33625: [coroutines] Diagnose invalid result types for `await_resume` and `await_suspend` and add missing conversions.

2017-05-27 Thread Eric Fiselier via Phabricator via cfe-commits 
EricWF updated this revision to Diff 100541.
EricWF added a comment.

- Format changes


https://reviews.llvm.org/D33625

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaCoroutine.cpp
  test/SemaCXX/coroutines.cpp

Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -837,3 +837,28 @@
   // expected-error@-1 {{'bad_promise_no_return_func' must declare either 'return_value' or 'return_void'}}
   co_await a;
 }
+
+struct bad_await_suspend_return {
+  bool await_ready();
+  // expected-error@+1 {{the return type of 'await_suspend' is required to be 'void' or 'bool' (have 'char')}}
+  char await_suspend(std::experimental::coroutine_handle<>);
+  void await_resume();
+};
+struct bad_await_ready_return {
+  // expected-note@+1 {{the return type of 'await_ready' is required to be contextually convertible to 'bool'}}
+  void await_ready();
+  bool await_suspend(std::experimental::coroutine_handle<>);
+  void await_resume();
+};
+void test_bad_suspend() {
+  {
+// FIXME: The actual error emitted here is terrible, and no number of notes can save it.
+bad_await_ready_return a;
+// expected-error@+1 {{value of type 'void' is not contextually convertible to 'bool'}}
+co_await a; // expected-note {{call to 'await_ready' implicitly required by coroutine function here}}
+  }
+  {
+bad_await_suspend_return b;
+co_await b; // expected-note {{call to 'await_suspend' implicitly required by coroutine function here}}
+  }
+}
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -321,6 +321,7 @@
 }
 
 struct ReadySuspendResumeResult {
+  enum AwaitCallType { ACT_Ready, ACT_Suspend, ACT_Resume };
   Expr *Results[3];
   OpaqueValueExpr *OpaqueValue;
   bool IsInvalid;
@@ -366,7 +367,41 @@
 Calls.Results[I] = Result.get();
   }
 
+  // Assume the calls are valid; all further checking should make them invalid.
   Calls.IsInvalid = false;
+
+  using ACT = ReadySuspendResumeResult::AwaitCallType;
+  CallExpr *AwaitReady = cast(Calls.Results[ACT::ACT_Ready]);
+  if (!AwaitReady->getType()->isDependentType()) {
+// [expr.await]p3 [...]
+// — await-ready is the expression e.await_ready(), contextually converted
+// to bool.
+ExprResult Conv = S.PerformContextuallyConvertToBool(AwaitReady);
+if (Conv.isInvalid()) {
+  S.Diag(AwaitReady->getDirectCallee()->getLocStart(),
+ diag::note_await_ready_no_bool_conversion);
+  S.Diag(Loc, diag::note_coroutine_promise_call_implicitly_required)
+  << AwaitReady->getDirectCallee() << E->getSourceRange();
+  Calls.IsInvalid = true;
+}
+Calls.Results[ACT::ACT_Ready] = Conv.get();
+  }
+  CallExpr *AwaitSuspend = cast(Calls.Results[ACT::ACT_Suspend]);
+  if (!AwaitSuspend->getType()->isDependentType()) {
+// [expr.await]p3 [...]
+//   - await-suspend is the expression e.await_suspend(h), which shall be
+// a prvalue of type void or bool.
+QualType RetType = AwaitSuspend->getType();
+if (RetType != S.Context.BoolTy && RetType != S.Context.VoidTy) {
+  S.Diag(AwaitSuspend->getCalleeDecl()->getLocation(),
+ diag::err_await_suspend_invalid_return_type)
+  << RetType;
+  S.Diag(Loc, diag::note_coroutine_promise_call_implicitly_required)
+  << AwaitSuspend->getDirectCallee();
+  Calls.IsInvalid = true;
+}
+  }
+
   return Calls;
 }
 
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -8973,6 +8973,12 @@
"type declares 'get_return_object_on_allocation_failure()'">;
 def note_coroutine_promise_call_implicitly_required : Note<
   "call to %0 implicitly required by coroutine function here">;
+def err_await_suspend_invalid_return_type : Error<
+  "the return type of 'await_suspend' is required to be 'void' or 'bool' (have %0)"
+>;
+def note_await_ready_no_bool_conversion : Note<
+  "the return type of 'await_ready' is required to be contextually convertible to 'bool'"
+>;
 }
 
 let CategoryName = "Documentation Issue" in {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33625: [coroutines] Diagnose invalid result types for `await_resume` and `await_suspend` and add missing conversions.

2017-05-27 Thread Eric Fiselier via Phabricator via cfe-commits 
EricWF created this revision.

The expression `await_ready` is required to be contextually convertible to bool 
and `await_suspend` must be a prvalue of either `void` or `bool`.
This patch adds diagnostics for when those requirements are violated.

It also correctly performs the contextual conversion to bool on the result of 
`await_ready`


https://reviews.llvm.org/D33625

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Sema/SemaCoroutine.cpp
  test/SemaCXX/coroutines.cpp

Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -837,3 +837,28 @@
   // expected-error@-1 {{'bad_promise_no_return_func' must declare either 'return_value' or 'return_void'}}
   co_await a;
 }
+
+struct bad_await_suspend_return {
+  bool await_ready();
+  // expected-error@+1 {{the return type of 'await_suspend' is required to be 'void' or 'bool' (have 'char')}}
+  char await_suspend(std::experimental::coroutine_handle<>);
+  void await_resume();
+};
+struct bad_await_ready_return {
+  // expected-note@+1 {{the return type of 'await_ready' is required to be contextually convertible to 'bool'}}
+  void await_ready();
+  bool await_suspend(std::experimental::coroutine_handle<>);
+  void await_resume();
+};
+void test_bad_suspend() {
+  {
+// FIXME: The actual error emitted here is terrible, and no number of notes can save it.
+bad_await_ready_return a;
+// expected-error@+1 {{value of type 'void' is not contextually convertible to 'bool'}}
+co_await a; // expected-note {{call to 'await_ready' implicitly required by coroutine function here}}
+  }
+  {
+bad_await_suspend_return b;
+co_await b; // expected-note {{call to 'await_suspend' implicitly required by coroutine function here}}
+  }
+}
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -321,6 +321,11 @@
 }
 
 struct ReadySuspendResumeResult {
+  enum AwaitCallType {
+ACT_Ready,
+ACT_Suspend,
+ACT_Resume
+  };
   Expr *Results[3];
   OpaqueValueExpr *OpaqueValue;
   bool IsInvalid;
@@ -366,7 +371,41 @@
 Calls.Results[I] = Result.get();
   }
 
+  // Assume the calls are valid; all further checking should make them invalid.
   Calls.IsInvalid = false;
+
+
+  using ACT = ReadySuspendResumeResult::AwaitCallType;
+  CallExpr *AwaitReady = cast(Calls.Results[ACT::ACT_Ready]);
+  if (!AwaitReady->getType()->isDependentType()) {
+// [expr.await]p3 [...]
+// — await-ready is the expression e.await_ready(), contextually converted to bool.
+ExprResult Conv = S.PerformContextuallyConvertToBool(AwaitReady);
+if (Conv.isInvalid()) {
+  S.Diag(AwaitReady->getDirectCallee()->getLocStart(),
+ diag::note_await_ready_no_bool_conversion);
+  S.Diag(Loc, diag::note_coroutine_promise_call_implicitly_required)
+  << AwaitReady->getDirectCallee() << E->getSourceRange();
+  Calls.IsInvalid = true;
+}
+Calls.Results[ACT::ACT_Ready] = Conv.get();
+  }
+  CallExpr *AwaitSuspend = cast(Calls.Results[ACT::ACT_Suspend]);
+  if (!AwaitSuspend->getType()->isDependentType()) {
+// [expr.await]p3 [...]
+//   - await-suspend is the expression e.await_suspend(h), which shall be
+// a prvalue of type void or bool.
+QualType RetType = AwaitSuspend->getType();
+if (RetType != S.Context.BoolTy && RetType != S.Context.VoidTy) {
+  S.Diag(AwaitSuspend->getCalleeDecl()->getLocation(),
+ diag::err_await_suspend_invalid_return_type)
+  << RetType;
+  S.Diag(Loc, diag::note_coroutine_promise_call_implicitly_required)
+  << AwaitSuspend->getDirectCallee();
+  Calls.IsInvalid = true;
+}
+  }
+
   return Calls;
 }
 
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -2495,7 +2495,6 @@
  NestedNameSpecifier *Qualifier,
  NamedDecl *FoundDecl,
  CXXMethodDecl *Method);
-
   ExprResult PerformContextuallyConvertToBool(Expr *From);
   ExprResult PerformContextuallyConvertToObjCPointer(Expr *From);
 
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -8973,6 +8973,12 @@
"type declares 'get_return_object_on_allocation_failure()'">;
 def note_coroutine_promise_call_implicitly_required : Note<
   "call to %0 implicitly required by coroutine function here">;
+def err_await_suspend_invalid_return_type : Error<
+  "the return type of 'await_suspend' is required to be 'void' or 'bool'

[clang-tools-extra] r304082 - ClangdTests.cpp: Use "clang/Config/config.h" instead of "llvm/Config/config.h".

2017-05-27 Thread NAKAMURA Takumi via cfe-commits 
Author: chapuni
Date: Sat May 27 18:19:28 2017
New Revision: 304082

URL: http://llvm.org/viewvc/llvm-project?rev=304082=rev
Log:
ClangdTests.cpp: Use "clang/Config/config.h" instead of "llvm/Config/config.h".

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

Modified: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp?rev=304082=304081=304082=diff
==
--- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp Sat May 27 
18:19:28 2017
@@ -9,9 +9,9 @@
 
 #include "ClangdServer.h"
 #include "clang/Basic/VirtualFileSystem.h"
+#include "clang/Config/config.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
-#include "llvm/Config/config.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Regex.h"


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

[PATCH] D33624: [coroutines] Mark cxx_status.html of Coroutines TS as (SVN)

2017-05-27 Thread Gor Nishanov via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL304081: [coroutines] Mark cxx_status.html of Coroutines TS 
as (SVN) (authored by GorNishanov).

Changed prior to commit:
  https://reviews.llvm.org/D33624?vs=100538=100539#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D33624

Files:
  cfe/trunk/www/cxx_status.html


Index: cfe/trunk/www/cxx_status.html
===
--- cfe/trunk/www/cxx_status.html
+++ cfe/trunk/www/cxx_status.html
@@ -831,9 +831,9 @@
 
 
   [DRAFT TS] Coroutines
-  http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0057r7.pdf;>P0057R7
-  
-  WIP
+  https://isocpp.org/files/papers/N4663.pdf;>N4663
+  -fcoroutines-ts-stdlib=libc++
+  SVN
 
 
   [TS] Library Fundamentals, Version 1 (invocation type traits)


Index: cfe/trunk/www/cxx_status.html
===
--- cfe/trunk/www/cxx_status.html
+++ cfe/trunk/www/cxx_status.html
@@ -831,9 +831,9 @@
 
 
   [DRAFT TS] Coroutines
-  http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0057r7.pdf;>P0057R7
-  
-  WIP
+  https://isocpp.org/files/papers/N4663.pdf;>N4663
+  -fcoroutines-ts-stdlib=libc++
+  SVN
 
 
   [TS] Library Fundamentals, Version 1 (invocation type traits)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r304081 - [coroutines] Mark cxx_status.html of Coroutines TS as (SVN)

2017-05-27 Thread Gor Nishanov via cfe-commits 
Author: gornishanov
Date: Sat May 27 17:54:52 2017
New Revision: 304081

URL: http://llvm.org/viewvc/llvm-project?rev=304081=rev
Log:
[coroutines] Mark cxx_status.html of Coroutines TS as (SVN)

Summary: It is time!

Reviewers: GorNishanov, rsmith

Reviewed By: GorNishanov, rsmith

Subscribers: EricWF, rsmith, cfe-commits

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

Modified:
cfe/trunk/www/cxx_status.html

Modified: cfe/trunk/www/cxx_status.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/www/cxx_status.html?rev=304081=304080=304081=diff
==
--- cfe/trunk/www/cxx_status.html (original)
+++ cfe/trunk/www/cxx_status.html Sat May 27 17:54:52 2017
@@ -831,9 +831,9 @@ and library features that are not part o
 
 
   [DRAFT TS] Coroutines
-  http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0057r7.pdf;>P0057R7
-  
-  WIP
+  https://isocpp.org/files/papers/N4663.pdf;>N4663
+  -fcoroutines-ts-stdlib=libc++
+  SVN
 
 
   [TS] Library Fundamentals, Version 1 (invocation type traits)


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

[PATCH] D33624: [coroutines] Mark cxx_status.html of Coroutines TS as (SVN)

2017-05-27 Thread Gor Nishanov via Phabricator via cfe-commits 
GorNishanov accepted this revision.
GorNishanov added a comment.
This revision is now accepted and ready to land.

Drumroll.. Approved!


https://reviews.llvm.org/D33624



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

[PATCH] D33624: [coroutines] Mark cxx_status.html of Coroutines TS as (SVN)

2017-05-27 Thread Gor Nishanov via Phabricator via cfe-commits 
GorNishanov created this revision.

It is time!


https://reviews.llvm.org/D33624

Files:
  www/cxx_status.html


Index: www/cxx_status.html
===
--- www/cxx_status.html
+++ www/cxx_status.html
@@ -831,9 +831,9 @@
 
 
   [DRAFT TS] Coroutines
-  http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0057r7.pdf;>P0057R7
-  
-  WIP
+  https://isocpp.org/files/papers/N4663.pdf;>N4663
+  -fcoroutines-ts-stdlib=libc++
+  SVN
 
 
   [TS] Library Fundamentals, Version 1 (invocation type traits)


Index: www/cxx_status.html
===
--- www/cxx_status.html
+++ www/cxx_status.html
@@ -831,9 +831,9 @@
 
 
   [DRAFT TS] Coroutines
-  http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0057r7.pdf;>P0057R7
-  
-  WIP
+  https://isocpp.org/files/papers/N4663.pdf;>N4663
+  -fcoroutines-ts-stdlib=libc++
+  SVN
 
 
   [TS] Library Fundamentals, Version 1 (invocation type traits)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33623: Make the parser close parens for you on EOF

2017-05-27 Thread Matt Kulukundis via Phabricator via cfe-commits 
fowles created this revision.
Herald added a subscriber: klimek.

Makes the parser for clang-query auto close all open parens on EOF


https://reviews.llvm.org/D33623

Files:
  lib/ASTMatchers/Dynamic/Parser.cpp
  unittests/ASTMatchers/Dynamic/ParserTest.cpp


Index: unittests/ASTMatchers/Dynamic/ParserTest.cpp
===
--- unittests/ASTMatchers/Dynamic/ParserTest.cpp
+++ unittests/ASTMatchers/Dynamic/ParserTest.cpp
@@ -239,7 +239,6 @@
   ParseWithError("stmt(someValue)"));
   EXPECT_EQ(
   "1:1: Matcher not found: Foo\n"
-  "1:4: Error parsing matcher. Found end-of-code while looking for ')'.",
   ParseWithError("Foo("));
   EXPECT_EQ("1:1: End of code found while looking for token.",
 ParseWithError(""));
Index: lib/ASTMatchers/Dynamic/Parser.cpp
===
--- lib/ASTMatchers/Dynamic/Parser.cpp
+++ lib/ASTMatchers/Dynamic/Parser.cpp
@@ -377,8 +377,8 @@
   }
 
   if (EndToken.Kind == TokenInfo::TK_Eof) {
-Error->addError(OpenToken.Range, Error->ET_ParserNoCloseParen);
-return false;
+// Just assume they want to finish the expression immediately and did not
+// have enough trailing right parens.
   }
 
   std::string BindID;


Index: unittests/ASTMatchers/Dynamic/ParserTest.cpp
===
--- unittests/ASTMatchers/Dynamic/ParserTest.cpp
+++ unittests/ASTMatchers/Dynamic/ParserTest.cpp
@@ -239,7 +239,6 @@
   ParseWithError("stmt(someValue)"));
   EXPECT_EQ(
   "1:1: Matcher not found: Foo\n"
-  "1:4: Error parsing matcher. Found end-of-code while looking for ')'.",
   ParseWithError("Foo("));
   EXPECT_EQ("1:1: End of code found while looking for token.",
 ParseWithError(""));
Index: lib/ASTMatchers/Dynamic/Parser.cpp
===
--- lib/ASTMatchers/Dynamic/Parser.cpp
+++ lib/ASTMatchers/Dynamic/Parser.cpp
@@ -377,8 +377,8 @@
   }
 
   if (EndToken.Kind == TokenInfo::TK_Eof) {
-Error->addError(OpenToken.Range, Error->ET_ParserNoCloseParen);
-return false;
+// Just assume they want to finish the expression immediately and did not
+// have enough trailing right parens.
   }
 
   std::string BindID;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30170: Function definition may have uninstantiated body

2017-05-27 Thread Serge Pavlov via Phabricator via cfe-commits 
sepavloff updated this revision to Diff 100534.
sepavloff added a comment.

Updated patch

- Reduce number of added functions,
- Fixed some comments,
- Function `isOdrDefined` now checks uninstantiated bodies only for friend 
functions.


https://reviews.llvm.org/D30170

Files:
  include/clang/AST/Decl.h
  lib/AST/Decl.cpp
  lib/Sema/SemaDecl.cpp
  test/SemaCXX/cxx0x-cursory-default-delete.cpp
  test/SemaCXX/friend2.cpp

Index: test/SemaCXX/friend2.cpp
===
--- test/SemaCXX/friend2.cpp
+++ test/SemaCXX/friend2.cpp
@@ -101,6 +101,34 @@
   friend void func_12(int x = 0);  // expected-error{{friend declaration specifying a default argument must be the only declaration}}
 };
 
+// Friend function with uninstantiated body is still a definition.
+
+template struct C20 {
+  friend void func_20() {} // expected-note{{previous definition is here}}
+};
+C20 c20i;
+void func_20() {} // expected-error{{redefinition of 'func_20'}}
+
+template struct C21a {
+  friend void func_21() {} // expected-note{{previous definition is here}}
+};
+template struct C21b {
+  friend void func_21() {} // expected-error{{redefinition of 'func_21'}}
+};
+C21a c21ai;
+C21b c21bi; // expected-note{{in instantiation of template class 'C21b' requested here}}
+
+template struct C22a {
+  friend void func_22() {} // expected-note{{previous definition is here}}
+};
+template struct C22b {
+  friend void func_22();
+};
+C22a c22ai;
+C22b c22bi;
+void func_22() {} // expected-error{{redefinition of 'func_22'}}
+
+
 
 namespace pr22307 {
 
Index: test/SemaCXX/cxx0x-cursory-default-delete.cpp
===
--- test/SemaCXX/cxx0x-cursory-default-delete.cpp
+++ test/SemaCXX/cxx0x-cursory-default-delete.cpp
@@ -136,13 +136,13 @@
 };
 
 struct DefaultDelete {
-  DefaultDelete() = default; // expected-note {{previous declaration is here}}
+  DefaultDelete() = default; // expected-note {{previous definition is here}}
   DefaultDelete() = delete; // expected-error {{constructor cannot be redeclared}}
 
-  ~DefaultDelete() = default; // expected-note {{previous declaration is here}}
+  ~DefaultDelete() = default; // expected-note {{previous definition is here}}
   ~DefaultDelete() = delete; // expected-error {{destructor cannot be redeclared}}
 
-  DefaultDelete =(const DefaultDelete &) = default; // expected-note {{previous declaration is here}}
+  DefaultDelete =(const DefaultDelete &) = default; // expected-note {{previous definition is here}}
   DefaultDelete =(const DefaultDelete &) = delete; // expected-error {{class member cannot be redeclared}}
 };
 
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -11875,8 +11875,12 @@
SkipBodyInfo *SkipBody) {
   const FunctionDecl *Definition = EffectiveDefinition;
   if (!Definition)
-if (!FD->isDefined(Definition))
+if (!FD->isOdrDefined(Definition))
   return;
+  assert(Definition);
+
+  if (FD == Definition)
+return;
 
   if (canRedefineFunction(Definition, getLangOpts()))
 return;
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -2534,16 +2534,45 @@
 
 bool FunctionDecl::isDefined(const FunctionDecl *) const {
   for (auto I : redecls()) {
-if (I->IsDeleted || I->IsDefaulted || I->Body || I->IsLateTemplateParsed ||
-I->hasDefiningAttr()) {
+if (I->isThisDeclarationADefinition()) {
   Definition = I->IsDeleted ? I->getCanonicalDecl() : I;
   return true;
 }
   }
 
   return false;
 }
 
+bool FunctionDecl::isOdrDefined(const FunctionDecl *) const {
+  // First try to find a declaration that has existing definition.
+  if (isDefined(Definition))
+return true;
+
+  // If no existing definition exist, look for a definition which can be
+  // instantiated.
+  for (auto I : redecls()) {
+if (I->getFriendObjectKind() != Decl::FOK_None) {
+  // If this is a friend function defined in a class template, it does not
+  // have a body until it is used, nevertheless it is a definition. The
+  // following code must produce redeclaration error:
+  //
+  // template struct C20 { friend void func_20() {} };
+  // C20 c20i;
+  // void func_20() {}
+  //
+  if (FunctionDecl *Original = I->getInstantiatedFromMemberFunction()) {
+const FunctionDecl *D;
+if (Original->isOdrDefined(D)) {
+  Definition = I;
+  return true;
+}
+  }
+}
+  }
+
+  return false;
+}
+
 Stmt *FunctionDecl::getBody(const FunctionDecl *) const {
   if (!hasBody(Definition))
 return nullptr;
Index: include/clang/AST/Decl.h
===
--- include/clang/AST/Decl.h
+++ include/clang/AST/Decl.h
@@ -1778,11

[PATCH] D30170: Function definition may have uninstantiated body

2017-05-27 Thread Serge Pavlov via Phabricator via cfe-commits 
sepavloff marked 2 inline comments as done.
sepavloff added a comment.

In https://reviews.llvm.org/D30170#761342, @rsmith wrote:

> Do we really need two different notions of "definition" and "odr definition" 
> here? What goes wrong if we always treat the "instantiation of a friend 
> function definition" case as being a definition?


It requires 3 additional changes besides update of `isDefined`. All of them 
implement current behavior of the function and look like:

  @@ -3749,7 +3750,8 @@ void Sema::InstantiateFunctionDefinition(SourceLocation 
PointOfInstantiation,
bool Recursive,
bool DefinitionRequired,
bool AtEndOfTU) {
  -  if (Function->isInvalidDecl() || Function->isDefined())
  +  const FunctionDecl *Def;
  +  if (Function->isInvalidDecl() || (Function->isDefined(Def) && 
Def->isThisDeclarationADefinition()))
   return;
  
 // Never instantiate an explicit specialization except if it is a class 
scope

It make sense to replace this weird expression `Function->isDefined(Def) && 
Def->isThisDeclarationADefinition()` with a call to a new function, something 
like `isActuallyDefined`. So we anyway have to add a new function for checking 
function definition. In the case of `IsOdrDefinition` the new function is used 
in one place only, modified version of `isDefined` requires changes in 
unrelated code places.

Use of a special variant of `isDefined` dedicated to redefinition checks looks 
less invasive and more robust. If however there are reasons to use modified 
`isDefined`, it also can be implemented, there is nothing wrong with such 
solution.




Comment at: include/clang/AST/Decl.h:1829-1830
 
+  /// Returns true if the function is defined and other definitions are not
+  /// allowed in the redeclaration chain.
+  ///

rsmith wrote:
> This will not be true in the presence of modules: we will ensure that at most 
> one declaration has a body, but multiple declarations can be instantiated 
> from a defined member function of a class template, for instance. So I think 
> the constraint is that you're not allowed to add *more* such definitions.
Fixed also some comments according to this note.



Comment at: include/clang/AST/Decl.h:1848
+  return true;
+if (FunctionDecl *Original = getInstantiatedFromMemberFunction())
+  return Original->isOdrDefined();

rsmith wrote:
> I think this will do the wrong thing for an explicit specialization of a 
> member of a class template:
> 
> ```
> template struct A { void f() {}; };
> template<> void A::f();
> ```
> 
> Here, `A::f()` is "instantiated from a member function" but it's not a 
> definition, not even in the ODR sense. I think it would suffice to also check 
> whether `Original` is a friend declaration here.
Although this function is not called in this case, its name must agree with its 
functionality, so changed implementation as you recommend.



Comment at: lib/AST/Decl.cpp:2538
+if (I->isThisDeclarationADefinition()) {
+  Definition = I->IsDeleted ? I->getCanonicalDecl() : I;
+  return true;

rsmith wrote:
> It seems incorrect to me for `isThisDeclarationADefinition()` to return 
> `true` on a redeclaration of a deleted function: only the first declaration 
> should be considered to be a definition in that case. (This special case 
> should be in `isThisDeclarationADefinition()`, not in `isDefined()`.)
Indeed, if a declaration was selected to be a definition, it is strange if 
`isThisDeclarationADefinition` returns false for it.



Comment at: lib/Sema/SemaDecl.cpp:11882
+
+  if (FD == Definition)
+return;

rsmith wrote:
> This looks wrong to me: `CheckForFunctionRedefinition` is intended to be 
> called *before* marking the function in question as a definition. If you call 
> it afterwards, then the `isOdrDefined` call won't work properly because it 
> will always check to see whether `FD` is a definition before checking any 
> other declaration. So any case that reaches here with `FD` being a definition 
> seems like a bug. I expect we're only getting here in that case due to the 
> missing check for friend in `isThisDeclarationAnOdrDefinition`.
`CheckForFunctionRedefinition` is called after the declaration is put to 
redeclaration chain. At this point the declaration does not have a body, so 
problem may arise only if the provided declaration is an instantiation. It 
happens when the method is called from `PerformPendingInstantiations` -> 
`InstantiateFunctionDefinition` -> `ActOnStartOfFunctionDef` -> 
`CheckForFunctionRedefinition`. In this case function body is generated for 
declaration that is already in redeclaration chain. If the chain is in valid 
state, it does not have duplicate definitions and call to 
`CheckForFunctionRedefinition` is

[PATCH] D31830: Emit invariant.group.barrier when using union field

2017-05-27 Thread Piotr Padlewski via Phabricator via cfe-commits 
Prazek marked 2 inline comments as done.
Prazek added inline comments.



Comment at: lib/CodeGen/CGExpr.cpp:3530
+  return false;
+}
+

rjmccall wrote:
> You need to recurse into base classes (to check their fields), and if you 
> write this to take a QualType you won't have to eagerly extract RD below.
Cool, thanks for the tip


https://reviews.llvm.org/D31830



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

[PATCH] D31830: Emit invariant.group.barrier when using union field

2017-05-27 Thread Piotr Padlewski via Phabricator via cfe-commits 
Prazek updated this revision to Diff 100533.
Prazek added a comment.

- changed to QualType, now it is much cleaner


https://reviews.llvm.org/D31830

Files:
  lib/CodeGen/CGExpr.cpp
  test/CodeGenCXX/strict-vtable-pointers.cpp

Index: test/CodeGenCXX/strict-vtable-pointers.cpp
===
--- test/CodeGenCXX/strict-vtable-pointers.cpp
+++ test/CodeGenCXX/strict-vtable-pointers.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -I%S -triple=x86_64-apple-darwin10 -fstrict-vtable-pointers -disable-llvm-passes -O2 -emit-llvm -o %t.ll
+// RUN: %clang_cc1 %s -I%S -triple=x86_64-apple-darwin10 -fstrict-vtable-pointers -std=c++11 -disable-llvm-passes -O2 -emit-llvm -o %t.ll
 // RUN: FileCheck --check-prefix=CHECK-CTORS %s < %t.ll
 // RUN: FileCheck --check-prefix=CHECK-NEW %s < %t.ll
 // RUN: FileCheck --check-prefix=CHECK-DTORS %s < %t.ll
@@ -180,6 +180,82 @@
 // CHECK-CTORS-NOT: @llvm.invariant.group.barrier(
 // CHECK-CTORS-LABEL: {{^}}}
 
+struct A {
+  virtual void foo();
+};
+struct B : A {
+  virtual void foo();
+};
+
+union U {
+  A a;
+  B b;
+};
+
+void changeToB(U *u);
+void changeToA(U *u);
+
+void g2(A *a) {
+  a->foo();
+}
+// We have to guard access to union fields with invariant.group, because
+// it is very easy to skip the barrier with unions. In this example the inlined
+// g2 will produce loads with the same !invariant.group metadata, and
+// u->a and u->b would use the same pointer.
+// CHECK-NEW-LABEL: define void @_Z14UnionsBarriersP1U
+void UnionsBarriers(U *u) {
+  // CHECK-NEW: call void @_Z9changeToBP1U(
+  changeToB(u);
+  // CHECK-NEW: call i8* @llvm.invariant.group.barrier(i8*
+  // CHECK-NEW: call void @_Z2g2P1A(%struct.A*
+  g2(>b);
+  // CHECK-NEW: call void @_Z9changeToAP1U(%union.U* %6)
+  changeToA(u);
+  // CHECK-NEW: call i8* @llvm.invariant.group.barrier(i8*
+  // call void @_Z2g2P1A(%struct.A* %a)
+  g2(>a);
+  // CHECK-NEW-NOT: call i8* @llvm.invariant.group.barrier(i8*
+}
+
+struct HoldingVirtuals {
+  A a;
+};
+
+struct Empty {};
+struct AnotherEmpty {
+  Empty e;
+};
+union NoVptrs {
+  int a;
+  AnotherEmpty empty;
+};
+void take(AnotherEmpty &);
+
+// CHECK-NEW-LABEL: noBarriers
+void noBarriers(NoVptrs ) {
+  // CHECK-NEW-NOT: call i8* @llvm.invariant.group.barrier(i8*
+  // CHECK-NEW: 42
+  noVptrs.a += 42;
+  // CHECK-NEW-NOT: call i8* @llvm.invariant.group.barrier(i8*
+  // CHECK-NEW: call void @_Z4takeR12AnotherEmpty(
+  take(noVptrs.empty);
+}
+
+union U2 {
+  HoldingVirtuals h;
+  int z;
+};
+void take(HoldingVirtuals &);
+
+// CHECK-NEW-LABEL: define void @_Z15UnionsBarriers2R2U2
+void UnionsBarriers2(U2 ) {
+  // CHECK-NEW-NOT: call i8* @llvm.invariant.group.barrier(i8*
+  // CHECK-NEW: 42
+  u.z += 42;
+  // CHECK-NEW: call i8* @llvm.invariant.group.barrier(i8*
+  // CHECK-NEW: call void @_Z4takeR15HoldingVirtuals(
+  take(u.h);
+}
 
 /** DTORS **/
 // CHECK-DTORS-LABEL: define linkonce_odr void @_ZN10StaticBaseD2Ev(
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -3528,6 +3528,25 @@
   return CGF.Builder.CreateStructGEP(base, idx, offset, field->getName());
 }
 
+static bool hasAnyVptr(const QualType Type, const ASTContext ) {
+  const auto *RD = Type.getTypePtr()->getAsCXXRecordDecl();
+  if (!RD)
+return false;
+
+  if (RD->isDynamicClass())
+return true;
+
+  for (const auto  : RD->bases())
+if (hasAnyVptr(Base.getType(), Context))
+  return true;
+
+  for (const FieldDecl *Field : RD->fields())
+if (hasAnyVptr(Field->getType(), Context))
+  return true;
+
+  return false;
+}
+
 LValue CodeGenFunction::EmitLValueForField(LValue base,
const FieldDecl *field) {
   LValueBaseInfo BaseInfo = base.getBaseInfo();
@@ -3569,6 +3588,14 @@
 assert(!type->isReferenceType() && "union has reference member");
 // TODO: handle path-aware TBAA for union.
 TBAAPath = false;
+
+const auto FieldType = field->getType();
+if (CGM.getCodeGenOpts().StrictVTablePointers &&
+hasAnyVptr(FieldType, getContext()))
+  // Because unions can easily skip invariant.barriers, we need to add
+  // a barrier every time CXXRecord field with vptr is referenced.
+  addr = Address(Builder.CreateInvariantGroupBarrier(addr.getPointer()),
+ addr.getAlignment());
   } else {
 // For structs, we GEP to the field that the record layout suggests.
 addr = emitAddrOfFieldStorage(*this, addr, field);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33437: Emit available_externally vtables opportunistically

2017-05-27 Thread Piotr Padlewski via Phabricator via cfe-commits 
Prazek updated this revision to Diff 100530.
Prazek marked 3 inline comments as done.
Prazek added a comment.

- Addressing John's comments


https://reviews.llvm.org/D33437

Files:
  include/clang/AST/VTableBuilder.h
  lib/CodeGen/CGVTables.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  lib/CodeGen/ItaniumCXXABI.cpp
  test/CodeGenCXX/vtable-available-externally.cpp
  test/CodeGenCXX/vtable-linkage.cpp

Index: test/CodeGenCXX/vtable-linkage.cpp
===
--- test/CodeGenCXX/vtable-linkage.cpp
+++ test/CodeGenCXX/vtable-linkage.cpp
@@ -145,12 +145,14 @@
 // F is an explicit template instantiation declaration without a
 // key function, so its vtable should have external linkage.
 // CHECK-DAG: @_ZTV1FIiE = external unnamed_addr constant
-// CHECK-OPT-DAG: @_ZTV1FIiE = external unnamed_addr constant
+// CHECK-OPT-DAG: @_ZTV1FIiE = available_externally unnamed_addr constant
 
 // E is an explicit template instantiation declaration. It has a
 // key function is not instantiated, so we know that vtable definition
 // will be generated in TU where key function will be defined
-// so we can mark it as available_externally (only with optimizations)
+// so we can mark it as external (without optimizations) and
+// available_externally (with optimizations) because all of the inline
+// virtual functions have been emitted.
 // CHECK-DAG: @_ZTV1EIiE = external unnamed_addr constant
 // CHECK-OPT-DAG: @_ZTV1EIiE = available_externally unnamed_addr constant
 
Index: test/CodeGenCXX/vtable-available-externally.cpp
===
--- test/CodeGenCXX/vtable-available-externally.cpp
+++ test/CodeGenCXX/vtable-available-externally.cpp
@@ -12,6 +12,7 @@
 // RUN: FileCheck --check-prefix=CHECK-TEST14 %s < %t.opt
 // RUN: FileCheck --check-prefix=CHECK-TEST15 %s < %t.opt
 // RUN: FileCheck --check-prefix=CHECK-TEST16 %s < %t.opt
+// RUN: FileCheck --check-prefix=CHECK-TEST17 %s < %t.opt
 
 #include 
 
@@ -274,8 +275,8 @@
   virtual D& operator=(const D&);
 };
 
-// Cannot emit B's vtable available_externally, because we cannot create
-// a reference to the inline virtual B::operator= function.
+// Cannot emit D's vtable available_externally, because we cannot create
+// a reference to the inline virtual D::operator= function.
 // CHECK-TEST11: @_ZTVN6Test111DE = external unnamed_addr constant
 struct D : C {
   virtual void key();
@@ -391,3 +392,30 @@
 }
 }
 
+namespace Test17 {
+// This test checks if we emit vtables opportunistically.
+// CHECK-TEST17-DAG: @_ZTVN6Test171AE = available_externally
+// CHECK-TEST17-DAG: @_ZTVN6Test171BE = external
+
+struct A {
+  virtual void key();
+  virtual void bar() {}
+};
+
+// We won't gonna use deleting destructor for this type, which will disallow
+// emitting vtable as available_externally
+struct B {
+  virtual void key();
+  virtual ~B() {}
+};
+
+void testcaseA() {
+  A a;
+  a.bar(); // this forces to emit definition of bar
+}
+
+void testcaseB() {
+  B b; // This only forces emitting of complete object destructor
+}
+
+} // namespace Test17
Index: lib/CodeGen/ItaniumCXXABI.cpp
===
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -366,20 +366,30 @@
   void emitCXXStructor(const CXXMethodDecl *MD, StructorType Type) override;
 
  private:
-   bool hasAnyVirtualInlineFunction(const CXXRecordDecl *RD) const {
-const auto  =
-CGM.getItaniumVTableContext().getVTableLayout(RD);
-
-for (const auto  : VtableLayout.vtable_components()) {
-  // Skip empty slot.
-  if (!VtableComponent.isUsedFunctionPointerKind())
-continue;
-
-  const CXXMethodDecl *Method = VtableComponent.getFunctionDecl();
-  if (Method->getCanonicalDecl()->isInlined())
-return true;
-}
-return false;
+   bool hasAnyUnusedVirtualInlineFunction(const CXXRecordDecl *RD) const {
+ const auto  =
+ CGM.getItaniumVTableContext().getVTableLayout(RD);
+
+ for (const auto  : VtableLayout.vtable_components()) {
+   // Skip empty slot.
+   if (!VtableComponent.isUsedFunctionPointerKind())
+ continue;
+
+   const CXXMethodDecl *Method = VtableComponent.getFunctionDecl();
+   if (!Method->getCanonicalDecl()->isInlined())
+ continue;
+
+   StringRef Name = CGM.getMangledName(VtableComponent.getGlobalDecl());
+   auto *Entry = CGM.GetGlobalValue(Name);
+   // This checks if virtual inline function has already been emitted.
+   // Note that it is possible that this inline function would be emitted
+   // after trying to emit vtable speculatively. Because of this we do
+   // an extra pass after emitting all deferred vtables to find and emit
+   // these vtables opportunistically.
+   if (!Entry || Entry->isDeclaration())
+ return true;
+ }
+ return false;
   }
 
   bool

[PATCH] D33437: Emit available_externally vtables opportunistically

2017-05-27 Thread Piotr Padlewski via Phabricator via cfe-commits 
Prazek added a comment.

Thanks for the comments :)




Comment at: include/clang/AST/VTableBuilder.h:160
+   "GlobalDecl can be created only from virtual function");
+if (getKind() == CK_FunctionPointer)
+  return GlobalDecl(getFunctionDecl());

rjmccall wrote:
> Please use an exhaustive switch.  You can put llvm_unreachable in the other 
> cases.
Should I implement this for RTTI fields? Or maybe leave a fixme comment that it 
could also work for some other fields in vtable, but is not currently used?


https://reviews.llvm.org/D33437



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

[PATCH] D29877: Warn about unused static file scope function template declarations.

2017-05-27 Thread Vassil Vassilev via Phabricator via cfe-commits 
v.g.vassilev added a comment.

In https://reviews.llvm.org/D29877#766301, @rsmith wrote:

> In https://reviews.llvm.org/D29877#766196, @EricWF wrote:
>
> > No. But I can point you to `range-v3` which uses this pattern and I think 
> > the idiom is somewhat appealing, but that's orthogonal to Clang diagnosing 
> > it.
>
>
> I found this:
>
> https://github.com/ericniebler/range-v3/blob/a4829172c0d6c43687ba213c54f430202efd7497/include/range/v3/view/zip_with.hpp#L44
>
> This code is wrong, and creates ODR violations on lines 190 and 200.
>
> It seems to me that the warning is firing on dangerous / broken code (yay!) 
> but the warning is not sufficient to explain *why* the code is broken (boo!).


Perhaps we can complement this with a note, explaining the linkage situation if 
we fire the warning in a header file...


https://reviews.llvm.org/D29877



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

[PATCH] D29877: Warn about unused static file scope function template declarations.

2017-05-27 Thread Vassil Vassilev via Phabricator via cfe-commits 
v.g.vassilev added inline comments.



Comment at: include/clang/Basic/DiagnosticGroups.td:631
 // UnusedParameter, (matches GCC's behavior)
+// UnusedTemplate, (clean-up libc++ before enabling)
 // UnusedMemberFunction, (clean-up llvm before 
enabling)

EricWF wrote:
> Is libc++ not clean for this? I just ran the libc++ tests with 
> -Wunused-template and didn't see any errors.
I think we were waiting also for the chromium guys. Perhaps they moved, too.


https://reviews.llvm.org/D29877



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

Re: [clang-tools-extra] r303977 - [clangd] Allow to use vfs::FileSystem for file accesses.

2017-05-27 Thread NAKAMURA Takumi via cfe-commits 
Hopefully fixed in r304067.

See also; https://reviews.llvm.org/D33416#766533

On Sat, May 27, 2017 at 6:12 AM Vedant Kumar via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Hi Ilya,
>
> The unit test introduced by this commit hits an assertion failure on our
> bots. Could you please take a look at the issue?
>
> lab.llvm.org:8080/green/job/clang-stage1-cmake-RA-expensive/6733
>
> I see:
>
> [ RUN  ] ClangdVFSTest.Reparse
>
> Assertion failed: (Unit && "Unit wasn't created"), function ClangdUnit,
> file
> /Users/buildslave/jenkins/sharedspace/clang-stage1-cmake-RA_workspace/llvm/tools/clang/tools/extra/clangd/ClangdUnit.cpp,
> line 58.
>
> 0  ClangdTests  0x000102fec2f8
> llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 40
> 1  ClangdTests  0x000102fec9e6 SignalHandler(int) + 454
> 2  libsystem_platform.dylib 0x7fff97ed352a _sigtramp + 26
> 3  libsystem_platform.dylib 00 _sigtramp + 1746062064
> 4  libsystem_c.dylib0x7fff97d776df abort + 129
> 5  libsystem_c.dylib0x7fff97d3edd8 basename + 0
> 6  ClangdTests  0x0001030c95dd
> clang::clangd::ClangdUnit::ClangdUnit(llvm::StringRef, llvm::StringRef,
> std::__1::shared_ptr,
> std::__1::vector std::__1::allocator >,
> llvm::IntrusiveRefCntPtr) + 1837
> 7  ClangdTests  0x0001030c824f
> std::__1::__function::__func llvm::StringRef)::$_1,
> std::__1::allocator llvm::StringRef)::$_1>, void ()>::operator()() + 975
> 8  ClangdTests  0x0001030c6e03 void*
> std::__1::__thread_proxy
> >(void*) + 579
> 9  libsystem_pthread.dylib  0x7fff8e26099d _pthread_body + 131
> 10 libsystem_pthread.dylib  0x7fff8e26091a _pthread_body + 0
> 11 libsystem_pthread.dylib  0x7fff8e25e351 thread_start + 13
>
> 
> FAIL: Extra Tools Unit Tests ::
> clangd/ClangdTests/ClangdVFSTest.ReparseOnHeaderChange (13778 of 37955)
>  TEST 'Extra Tools Unit Tests ::
> clangd/ClangdTests/ClangdVFSTest.ReparseOnHeaderChange' FAILED
> 
> Note: Google Test filter = ClangdVFSTest.ReparseOnHeaderChange
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from ClangdVFSTest
> [ RUN  ] ClangdVFSTest.ReparseOnHeaderChange
>
> Assertion failed: (Unit && "Unit wasn't created"), function ClangdUnit,
> file
> /Users/buildslave/jenkins/sharedspace/clang-stage1-cmake-RA_workspace/llvm/tools/clang/tools/extra/clangd/ClangdUnit.cpp,
> line 58.
>
> 0  ClangdTests  0x0001005b72f8
> llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 40
> 1  ClangdTests  0x0001005b79e6 SignalHandler(int) + 454
> 2  libsystem_platform.dylib 0x7fff97ed352a _sigtramp + 26
> 3  libsystem_platform.dylib 00 _sigtramp + 1746062064
> 4  libsystem_c.dylib0x7fff97d776df abort + 129
> 5  libsystem_c.dylib0x7fff97d3edd8 basename + 0
> 6  ClangdTests  0x0001006945dd
> clang::clangd::ClangdUnit::ClangdUnit(llvm::StringRef, llvm::StringRef,
> std::__1::shared_ptr,
> std::__1::vector std::__1::allocator >,
> llvm::IntrusiveRefCntPtr) + 1837
> 7  ClangdTests  0x00010069324f
> std::__1::__function::__func llvm::StringRef)::$_1,
> std::__1::allocator llvm::StringRef)::$_1>, void ()>::operator()() + 975
> 8  ClangdTests  0x000100691e03 void*
> std::__1::__thread_proxy
> >(void*) + 579
> 9  libsystem_pthread.dylib  0x7fff8e26099d _pthread_body + 131
> 10 libsystem_pthread.dylib  0x7fff8e26091a _pthread_body + 0
> 11 libsystem_pthread.dylib  0x7fff8e25e351 thread_start + 13
>
> thanks,
> vedant
>
> On May 26, 2017, at 5:26 AM, Ilya Biryukov via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> Author: ibiryukov
> Date: Fri May 26 07:26:51 2017
> New Revision: 303977
>
> URL: http://llvm.org/viewvc/llvm-project?rev=303977=rev
> Log:
> [clangd] Allow to use vfs::FileSystem for file accesses.
>
> Summary:
> Custom vfs::FileSystem is currently used for unit tests.
> This revision depends on https://reviews.llvm.org/D33397.
>
> Reviewers: bkramer, krasimir
>
> Reviewed By: bkramer, krasimir
>
> Subscribers: klimek, cfe-commits, mgorny
>
> Differential Revision: https://reviews.llvm.org/D33416
>
> Added:
>clang-tools-extra/trunk/unittests/clangd/
>clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
>clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
> Modified:
>

[PATCH] D33416: [clangd] Allow to use vfs::FileSystem for file accesses.

2017-05-27 Thread NAKAMURA Takumi via Phabricator via cfe-commits 
chapuni added inline comments.



Comment at: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp:123
+  TmpDirs.push_back(TmpDir1.str());
+  if (TmpDir2 != TmpDir2)
+TmpDirs.push_back(TmpDir2.str());

Did you mean, "TmpDir1 != TmpDir2" ?
Fixed in r304067.


Repository:
  rL LLVM

https://reviews.llvm.org/D33416



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

[clang-tools-extra] r304067 - ClangdTests.cpp: Fix a possible typo, it was "if (TmpDir2 != TmpDir2)"

2017-05-27 Thread NAKAMURA Takumi via cfe-commits 
Author: chapuni
Date: Sat May 27 03:06:52 2017
New Revision: 304067

URL: http://llvm.org/viewvc/llvm-project?rev=304067=rev
Log:
ClangdTests.cpp: Fix a possible typo, it was "if (TmpDir2 != TmpDir2)"

It caused failures in unittests if TmpDir2 is not "/tmp" .

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

Modified: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp?rev=304067=304066=304067=diff
==
--- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp Sat May 27 
03:06:52 2017
@@ -120,7 +120,7 @@ IntrusiveRefCntPtr getT
 
   std::vector TmpDirs;
   TmpDirs.push_back(TmpDir1.str());
-  if (TmpDir2 != TmpDir2)
+  if (TmpDir1 != TmpDir2)
 TmpDirs.push_back(TmpDir2.str());
   return new vfs::FilteredFileSystem(std::move(TmpDirs),
  vfs::getRealFileSystem());


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