date:20171023

[PATCH] D38773: [Sema] Add support for flexible array members in Obj-C.

2017-10-23 Thread John McCall via Phabricator via cfe-commits

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

LGTM.


https://reviews.llvm.org/D38773



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

[PATCH] D38857: [OpenCL] Improve printing and semantic check related to implicit addr space

2017-10-23 Thread John McCall via Phabricator via cfe-commits

rjmccall added inline comments.



Comment at: include/clang/AST/Type.h:337
+Mask = (Mask & ~ImplicitAddrSpaceMask) |
+   (((uint32_t)Value) << ImplicitAddrSpaceShift);
+  }

This is probably cleaner as:

  Mask = (Value ? (Mask | ImplicitAddrSpaceMask) : (Mask & 
~ImplicitAddrSpaceMask));



Comment at: lib/AST/ASTContext.cpp:2290
+  if (CanT.getAddressSpace() == AddressSpace &&
+  CanT.getQualifiers().isAddressSpaceImplicit() == ImplicitFlag)
 return T;

It looks like your changes here are making implicitness part of the canonical 
type, which is wrong, because implicitly- and explicitly-qualified types are 
not actually different types.

That is fixable, but I'm going to ask you to investigate whether you can solve 
this problem with AttributedType before you introduce this complexity into the 
qualifier system.


https://reviews.llvm.org/D38857



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

[PATCH] D39174: [analyzer] Fix handling of labels in getLValueElement

2017-10-23 Thread Artem Dergachev via Phabricator via cfe-commits

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

Great. Probably one more reason to turn all `Loc`s into regions.


Repository:
  rL LLVM

https://reviews.llvm.org/D39174



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

[PATCH] D39008: [CodeGen] Propagate may-alias'ness of lvalues with TBAA info

2017-10-23 Thread John McCall via Phabricator via cfe-commits

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

Well, I've been agreeing so far that it makes sense to propagate TBAA 
information separately from LValueBaseInfo, and this seems like a logical part 
of that, so okay.


Repository:
  rL LLVM

https://reviews.llvm.org/D39008



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

[PATCH] D39057: [clangd][WIP] Integrate the refactoring actions into clangd

2017-10-23 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added inline comments.



Comment at: clangd/ClangdServer.cpp:500
+if (!RefactoringClient)
+  RefactoringClient = 
llvm::make_unique();
+Results = clangd::findAvailableRefactoringCommands(*RefactoringClient, 
*AST,

Maybe initialize `RefactoringClient` in constructor?



Comment at: clangd/ClangdServer.cpp:514
+  std::shared_ptr Resources = Units.getFile(File);
+  assert(Resources && "executeCommand getCommands on non-added file");
+

s/getCommands//



Comment at: clangd/ClangdServer.cpp:526
+clangd::performRefactoringCommand(*RefactoringClient, CommandName, 
*AST,
+
+  SelectionRange, Logger);

NIT: remove empty line. 



Comment at: clangd/Protocol.cpp:992
+   clangd::Logger &Logger) {
+  CommandArgument Result;
+  for (auto &NextKeyValue : *Params) {

Maybe use `llvm::Optional` instead of `CommandArgument`?
If the loop body below won't get executed (i.e., `Params` is empty), we'll be 
returning uninitialized local variable.



Comment at: clangd/Protocol.h:540
+  enum Kind { SelectionRangeKind, TextDocumentIdentifierKind };
+  Kind ArgumentKind;
+

Could we make `ArgumentKind`, `union` members and default constructor private  
(`makeDocumentID` , `makeSelectionRange` and `unparse` would be the only way to 
construct `CommandArgument`)?

This would give an interface that's much harder to misuse.


Repository:
  rL LLVM

https://reviews.llvm.org/D39057



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

[PATCH] D39127: Fix template parameter default args missed if redecled

2017-10-23 Thread Martin Storsjö via Phabricator via cfe-commits

mstorsjo added inline comments.



Comment at: lib/Sema/SemaTemplate.cpp:4811
+  TemplateParameterList *Params =
+  cast(Template->getMostRecentDecl())
+  ->getTemplateParameters();

erichkeane wrote:
> mstorsjo wrote:
> > erichkeane wrote:
> > > mstorsjo wrote:
> > > > How does this work if there's another forward declaration missing the 
> > > > parameter later? Is the logic with "most recent" ok, or should it be 
> > > > "most qualified/complete" instead?
> > > I'm not sure the case you mean.  Any future declarations with fewer 
> > > parameters would be aspecialization and a different decl, right?
> > Sorry, I meant a declaration with fewer defaults. E.g. the original forward 
> > declaration without defaults again after the actual definition.
> > 
> > E.g. by adding
> > 
> > ```
> > namespace llvm {
> >   template struct StringSet;
> > }
> > ```
> > before the last `namespace lld`.
> > 
> > 
> I think thats OK.  The code propagates those forward.  The problem here is 
> that it does NOT propagate them backwards.
Ok then, then I guess it sounds fine to me, although I'm very much not familiar 
with this code. So I'm not comfortable with giving it a formal approval.


https://reviews.llvm.org/D39127



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

[PATCH] D38126: Make TBAA information to be part of LValueBaseInfo

2017-10-23 Thread John McCall via Phabricator via cfe-commits

rjmccall added a comment.

Hmm.  I'm not sure I like the design of merging TBAAAccessInfo into 
LValueBaseInfo.  LValueBaseInfo is currently the set of information that's 
generally preserved across l-value manipulations.  It was extracted from LValue 
specifically to create an encapsulated entity that can just be mindlessly 
propagated in most of that code.  But if you merge TBAA information into it, 
then l-value manipulations need to be aware of its contents, in the same way 
they have to be aware of everything else they're storing in the new LValue.  
The fact that code needs to be aware of it suggests it should continue to be an 
explicit argument, or at the very least be combined with something else that 
l-value manipulations generally already need to be aware of, like the type.


https://reviews.llvm.org/D38126



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

[PATCH] D39069: CodeGen: Fix missing debug loc due to alloca

2017-10-23 Thread John McCall via Phabricator via cfe-commits

rjmccall added a comment.

If this is something we generally need to be doing in all the places we 
temporarily save and restore the insertion point, we should fix the basic 
behavior of saveIP instead of adding explicit code to a bunch of separate 
places.  Can we just override saveIP() on CGBuilder to return a struct that 
also includes the current debug location?


https://reviews.llvm.org/D39069



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

[PATCH] D38618: Do not add a colon chunk to the code completion of class inheritance access modifiers

2017-10-23 Thread Ivan Donchevskii via Phabricator via cfe-commits

yvvan added a comment.

ping


https://reviews.llvm.org/D38618



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

[PATCH] D39177: [CodeGen] Generate TBAA info for reference loads

2017-10-23 Thread Ivan Kosarev via Phabricator via cfe-commits

kosarev created this revision.
kosarev added a project: clang.

Repository:
  rL LLVM

https://reviews.llvm.org/D39177

Files:
  lib/CodeGen/CGBlocks.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGStmtOpenMP.cpp
  lib/CodeGen/CodeGenFunction.h
  test/CodeGen/tbaa-reference.cpp

Index: test/CodeGen/tbaa-reference.cpp
===
--- test/CodeGen/tbaa-reference.cpp
+++ test/CodeGen/tbaa-reference.cpp
@@ -6,24 +6,32 @@
 
 struct B {
   S &s;
-  B(S &s) : s(s) {}
-  void bar();
+  B(S &s);
+  S &get();
 };
 
-void foo(S &s) {
-  B b(s);
-  b.bar();
-}
-
-// CHECK-LABEL: _Z3fooR1S
-// Check initialization of the reference parameter in foo().
-// CHECK: store %struct.S* {{.*}}, %struct.S** {{.*}}, !tbaa [[TAG_pointer:!.*]]
-//
+B::B(S &s) : s(s) {
 // CHECK-LABEL: _ZN1BC2ER1S
-// TODO: Check loading of the reference parameter in B::B(S&).
-// Check initialization of B::s in B::B(S&).
+// Check initialization of the reference parameter.
+// CHECK: store %struct.S* {{.*}}, %struct.S** {{.*}}, !tbaa [[TAG_pointer:!.*]]
+
+// Check loading of the reference parameter.
+// CHECK: load %struct.S*, %struct.S** {{.*}}, !tbaa [[TAG_pointer]]
+
+// Check initialization of the reference member.
 // CHECK: store %struct.S* {{.*}}, %struct.S** {{.*}}, !tbaa [[TAG_pointer]]
-//
+}
+
+S &B::get() {
+// CHECK-LABEL: _ZN1B3getEv
+// Check that we access the reference as a structure member.
+// CHECK: load %struct.S*, %struct.S** {{.*}}, !tbaa [[TAG_B_s:!.*]]
+  return s;
+}
+
 // CHECK-DAG: [[TAG_pointer]] = !{[[TYPE_pointer:!.*]], [[TYPE_pointer]], i64 0}
+// CHECK-DAG: [[TAG_B_s]] = !{[[TYPE_B:!.*]], [[TYPE_pointer]], i64 0}
+//
+// CHECK-DAG: [[TYPE_B]] = !{!"_ZTS1B", [[TYPE_pointer]], i64 0}
 // CHECK-DAG: [[TYPE_pointer]] = !{!"any pointer", [[TYPE_char:!.*]], i64 0}
 // CHECK-DAG: [[TYPE_char]] = !{!"omnipotent char", {{!.*}}, i64 0}
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -1952,10 +1952,25 @@
LValueBaseInfo *BaseInfo = nullptr,
TBAAAccessInfo *TBAAInfo = nullptr);
 
-  Address EmitLoadOfReference(Address Ref, const ReferenceType *RefTy,
-  LValueBaseInfo *BaseInfo = nullptr,
-  TBAAAccessInfo *TBAAInfo = nullptr);
-  LValue EmitLoadOfReferenceLValue(Address Ref, const ReferenceType *RefTy);
+  Address EmitLoadOfReference(Address ReferenceAddr,
+  const ReferenceType *ReferenceTy,
+  LValueBaseInfo ReferenceBaseInfo,
+  TBAAAccessInfo ReferenceTBAAInfo,
+  bool IsVolatile = false,
+  LValueBaseInfo *ReferenceeBaseInfo = nullptr,
+  TBAAAccessInfo *ReferenceeTBAAInfo = nullptr);
+  LValue EmitLoadOfReferenceLValue(Address ReferenceAddr,
+   const ReferenceType *ReferenceTy,
+   LValueBaseInfo ReferenceBaseInfo,
+   TBAAAccessInfo ReferenceTBAAInfo);
+  LValue EmitLoadOfReferenceLValue(Address ReferenceAddr,
+   const ReferenceType *ReferenceTy,
+   AlignmentSource Source =
+   AlignmentSource::Type) {
+return EmitLoadOfReferenceLValue(
+ReferenceAddr, ReferenceTy, LValueBaseInfo(Source),
+CGM.getTBAAAccessInfo(QualType(ReferenceTy, 0)));
+  }
 
   Address EmitLoadOfPointer(Address Ptr, const PointerType *PtrTy,
 LValueBaseInfo *BaseInfo = nullptr,
Index: lib/CodeGen/CGStmtOpenMP.cpp
===
--- lib/CodeGen/CGStmtOpenMP.cpp
+++ lib/CodeGen/CGStmtOpenMP.cpp
@@ -412,7 +412,8 @@
   if (!VarTy->isReferenceType()) {
 if (ArgLVal.getType()->isLValueReferenceType()) {
   ArgAddr = CGF.EmitLoadOfReference(
-  ArgAddr, ArgLVal.getType()->castAs());
+  ArgAddr, ArgLVal.getType()->castAs(),
+  ArgLVal.getBaseInfo(), ArgLVal.getTBAAInfo());
 } else if (!VarTy->isVariablyModifiedType() || !VarTy->isPointerType()) {
   assert(ArgLVal.getType()->isPointerType());
   ArgAddr = CGF.EmitLoadOfPointer(
Index: lib/CodeGen/CGOpenMPRuntime.cpp
===
--- lib/CodeGen/CGOpenMPRuntime.cpp
+++ lib/CodeGen/CGOpenMPRuntime.cpp
@@ -1039,7 +1039,9 @@
   BaseLV = CGF.EmitLoadOfPointerLValue(BaseLV.getAddress(), PtrTy);
 else {
   BaseLV = CGF.EmitLoadOfReferenceLValue(BaseLV.getAddress(),
- BaseTy->castAs());
+

[PATCH] D39177: [CodeGen] Generate TBAA info for reference loads

2017-10-23 Thread Ivan Kosarev via Phabricator via cfe-commits

kosarev added inline comments.



Comment at: test/CodeGen/tbaa-reference.cpp:1
 // RUN: %clang_cc1 -triple x86_64-linux -O1 -disable-llvm-passes %s -emit-llvm 
-o - | FileCheck %s
 //

Changes for this test look somewhat complicated, so here's a short summary:
* We now test loading of reference parameter in B::B() and the constructor 
itself got an out-of-line definition, so we don't need the foo() function 
anymore. B::bar() is removed for the same reason.
* Added method B::get() to check that we load member references properly.


Repository:
  rL LLVM

https://reviews.llvm.org/D39177



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

[PATCH] D38939: [clangd] Handle exit notification (proper shutdown)

2017-10-23 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added inline comments.



Comment at: clangd/ClangdLSPServer.cpp:59
 void ClangdLSPServer::onShutdown(Ctx C, ShutdownParams &Params) {
-  IsDone = true;
+  // Before we reply, we could serialize the preambles to disk. For now, let's
+  // just say we're ready to exit.

The comment is a bit misleading. I'd simply say we don't do need to do anything 
on `shutdown` at this point.

offtopic:
Preambles is not something we'd want to serialise (they are rather fast to 
build, and we only need them to make code completion fast), properly saving 
state of various indexes when we'll have them is something we'll definitely 
need to do on shutdown, though.



Comment at: clangd/tool/ClangdMain.cpp:117
 ResourceDirRef, CompileCommandsDirPath);
-  LSPServer.run(std::cin);
+  return LSPServer.run(std::cin) ? 0 : 1;
 }

Maybe add a `const NoShutdownRequestErrorCode = 1;` and  use it here instead of 
`1`?
Or add a comment on why we exit with error code. (It's not entirely obvious to 
someone who hasn't read this part of LSP spec)



Comment at: test/clangd/protocol.test:1
-# RUN: clangd -run-synchronously < %s | FileCheck %s
-# RUN: clangd -run-synchronously < %s 2>&1 | FileCheck -check-prefix=STDERR %s
+# RUN: not clangd -run-synchronously < %s | FileCheck %s
+# RUN: not clangd -run-synchronously < %s 2>&1 | FileCheck 
-check-prefix=STDERR %s

Maybe add a comment why we use `not` here?
It's not obvious for someone who hasn't read the whole test file. 


https://reviews.llvm.org/D38939



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

r316314 - [rename] Don't overwrite the template argument when renaming a template function.

2017-10-23 Thread Haojian Wu via cfe-commits

Author: hokein
Date: Mon Oct 23 01:58:50 2017
New Revision: 316314

URL: http://llvm.org/viewvc/llvm-project?rev=316314&view=rev
Log:
[rename] Don't overwrite the template argument when renaming a template 
function.

Reviewers: ioeric

Reviewed By: ioeric

Subscribers: cierpuchaw, cfe-commits, klimek

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

Modified:
cfe/trunk/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
cfe/trunk/unittests/Rename/RenameFunctionTest.cpp

Modified: cfe/trunk/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp?rev=316314&r1=316313&r2=316314&view=diff
==
--- cfe/trunk/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp (original)
+++ cfe/trunk/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp Mon Oct 23 
01:58:50 2017
@@ -221,7 +221,12 @@ public:
 }
 
 auto StartLoc = Expr->getLocStart();
-auto EndLoc = Expr->getLocEnd();
+// For template function call expressions like `foo()`, we want to
+// restrict the end of location to just before the `<` character.
+SourceLocation EndLoc = Expr->hasExplicitTemplateArgs()
+? Expr->getLAngleLoc().getLocWithOffset(-1)
+: Expr->getLocEnd();
+
 // In case of renaming an enum declaration, we have to explicitly handle
 // unscoped enum constants referenced in expressions (e.g.
 // "auto r = ns1::ns2::Green" where Green is an enum constant of an 
unscoped

Modified: cfe/trunk/unittests/Rename/RenameFunctionTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Rename/RenameFunctionTest.cpp?rev=316314&r1=316313&r2=316314&view=diff
==
--- cfe/trunk/unittests/Rename/RenameFunctionTest.cpp (original)
+++ cfe/trunk/unittests/Rename/RenameFunctionTest.cpp Mon Oct 23 01:58:50 2017
@@ -220,6 +220,25 @@ TEST_F(RenameFunctionTest, RenameFunctio
   CompareSnippets(Expected, After);
 }
 
+TEST_F(RenameFunctionTest, RenameTemplateFunctions) {
+  std::string Before = R"(
+  namespace na {
+  template T X();
+  }
+  namespace na { void f() { X(); } }
+  namespace nb { void g() { na::X  (); } }
+  )";
+  std::string Expected = R"(
+  namespace na {
+  template T Y();
+  }
+  namespace na { void f() { nb::Y(); } }
+  namespace nb { void g() { Y(); } }
+  )";
+  std::string After = runClangRenameOnCode(Before, "na::X", "nb::Y");
+  CompareSnippets(Expected, After);
+}
+
 TEST_F(RenameFunctionTest, RenameOutOfLineFunctionDecls) {
   std::string Before = R"(
   namespace na {


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

[PATCH] D39120: [rename] Don't overwrite the template argument when renaming a template function.

2017-10-23 Thread Haojian Wu via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL316314: [rename] Don't overwrite the template argument when 
renaming a template… (authored by hokein).

Repository:
  rL LLVM

https://reviews.llvm.org/D39120

Files:
  cfe/trunk/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
  cfe/trunk/unittests/Rename/RenameFunctionTest.cpp


Index: cfe/trunk/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
===
--- cfe/trunk/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
+++ cfe/trunk/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
@@ -221,7 +221,12 @@
 }
 
 auto StartLoc = Expr->getLocStart();
-auto EndLoc = Expr->getLocEnd();
+// For template function call expressions like `foo()`, we want to
+// restrict the end of location to just before the `<` character.
+SourceLocation EndLoc = Expr->hasExplicitTemplateArgs()
+? Expr->getLAngleLoc().getLocWithOffset(-1)
+: Expr->getLocEnd();
+
 // In case of renaming an enum declaration, we have to explicitly handle
 // unscoped enum constants referenced in expressions (e.g.
 // "auto r = ns1::ns2::Green" where Green is an enum constant of an 
unscoped
Index: cfe/trunk/unittests/Rename/RenameFunctionTest.cpp
===
--- cfe/trunk/unittests/Rename/RenameFunctionTest.cpp
+++ cfe/trunk/unittests/Rename/RenameFunctionTest.cpp
@@ -220,6 +220,25 @@
   CompareSnippets(Expected, After);
 }
 
+TEST_F(RenameFunctionTest, RenameTemplateFunctions) {
+  std::string Before = R"(
+  namespace na {
+  template T X();
+  }
+  namespace na { void f() { X(); } }
+  namespace nb { void g() { na::X  (); } }
+  )";
+  std::string Expected = R"(
+  namespace na {
+  template T Y();
+  }
+  namespace na { void f() { nb::Y(); } }
+  namespace nb { void g() { Y(); } }
+  )";
+  std::string After = runClangRenameOnCode(Before, "na::X", "nb::Y");
+  CompareSnippets(Expected, After);
+}
+
 TEST_F(RenameFunctionTest, RenameOutOfLineFunctionDecls) {
   std::string Before = R"(
   namespace na {


Index: cfe/trunk/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
===
--- cfe/trunk/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
+++ cfe/trunk/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
@@ -221,7 +221,12 @@
 }
 
 auto StartLoc = Expr->getLocStart();
-auto EndLoc = Expr->getLocEnd();
+// For template function call expressions like `foo()`, we want to
+// restrict the end of location to just before the `<` character.
+SourceLocation EndLoc = Expr->hasExplicitTemplateArgs()
+? Expr->getLAngleLoc().getLocWithOffset(-1)
+: Expr->getLocEnd();
+
 // In case of renaming an enum declaration, we have to explicitly handle
 // unscoped enum constants referenced in expressions (e.g.
 // "auto r = ns1::ns2::Green" where Green is an enum constant of an unscoped
Index: cfe/trunk/unittests/Rename/RenameFunctionTest.cpp
===
--- cfe/trunk/unittests/Rename/RenameFunctionTest.cpp
+++ cfe/trunk/unittests/Rename/RenameFunctionTest.cpp
@@ -220,6 +220,25 @@
   CompareSnippets(Expected, After);
 }
 
+TEST_F(RenameFunctionTest, RenameTemplateFunctions) {
+  std::string Before = R"(
+  namespace na {
+  template T X();
+  }
+  namespace na { void f() { X(); } }
+  namespace nb { void g() { na::X  (); } }
+  )";
+  std::string Expected = R"(
+  namespace na {
+  template T Y();
+  }
+  namespace na { void f() { nb::Y(); } }
+  namespace nb { void g() { Y(); } }
+  )";
+  std::string After = runClangRenameOnCode(Before, "na::X", "nb::Y");
+  CompareSnippets(Expected, After);
+}
+
 TEST_F(RenameFunctionTest, RenameOutOfLineFunctionDecls) {
   std::string Before = R"(
   namespace na {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39120: [rename] Don't overwrite the template argument when renaming a template function.

2017-10-23 Thread Haojian Wu via Phabricator via cfe-commits

hokein added inline comments.



Comment at: unittests/Rename/RenameFunctionTest.cpp:232
+  std::string Expected = R"(
+  namespace na {
+  template T Y();

cierpuchaw wrote:
> Shouldn't this be `namespace nb {`?
It is intended. We don't change the namespace here, only the name of the 
definition will be changed.


Repository:
  rL LLVM

https://reviews.llvm.org/D39120



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

[PATCH] D37554: [libclang] Allow crash recovery with LIBCLANG_NOTHREADS

2017-10-23 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik updated this revision to Diff 119823.
nik added a comment.

Rebased and took over better wording/description from Ilya.


https://reviews.llvm.org/D37554

Files:
  tools/libclang/CIndex.cpp
  tools/libclang/CIndexCodeCompletion.cpp
  tools/libclang/Indexing.cpp


Index: tools/libclang/Indexing.cpp
===
--- tools/libclang/Indexing.cpp
+++ tools/libclang/Indexing.cpp
@@ -880,11 +880,6 @@
 TU_options);
   };
 
-  if (getenv("LIBCLANG_NOTHREADS")) {
-IndexSourceFileImpl();
-return result;
-  }
-
   llvm::CrashRecoveryContext CRC;
 
   if (!RunSafely(CRC, IndexSourceFileImpl)) {
@@ -934,11 +929,6 @@
 index_options, TU);
   };
 
-  if (getenv("LIBCLANG_NOTHREADS")) {
-IndexTranslationUnitImpl();
-return result;
-  }
-
   llvm::CrashRecoveryContext CRC;
 
   if (!RunSafely(CRC, IndexTranslationUnitImpl)) {
Index: tools/libclang/CIndexCodeCompletion.cpp
===
--- tools/libclang/CIndexCodeCompletion.cpp
+++ tools/libclang/CIndexCodeCompletion.cpp
@@ -806,11 +806,6 @@
 llvm::makeArrayRef(unsaved_files, num_unsaved_files), options);
   };
 
-  if (getenv("LIBCLANG_NOTHREADS")) {
-CodeCompleteAtImpl();
-return result;
-  }
-
   llvm::CrashRecoveryContext CRC;
 
   if (!RunSafely(CRC, CodeCompleteAtImpl)) {
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -3508,11 +3508,6 @@
 llvm::makeArrayRef(unsaved_files, num_unsaved_files), options, out_TU);
   };
 
-  if (getenv("LIBCLANG_NOTHREADS")) {
-ParseTranslationUnitImpl();
-return result;
-  }
-
   llvm::CrashRecoveryContext CRC;
 
   if (!RunSafely(CRC, ParseTranslationUnitImpl)) {
@@ -3921,8 +3916,7 @@
 result = clang_saveTranslationUnit_Impl(TU, FileName, options);
   };
 
-  if (!CXXUnit->getDiagnostics().hasUnrecoverableErrorOccurred() ||
-  getenv("LIBCLANG_NOTHREADS")) {
+  if (!CXXUnit->getDiagnostics().hasUnrecoverableErrorOccurred()) {
 SaveTranslationUnitImpl();
 
 if (getenv("LIBCLANG_RESOURCE_USAGE"))
@@ -4045,11 +4039,6 @@
 TU, llvm::makeArrayRef(unsaved_files, num_unsaved_files), options);
   };
 
-  if (getenv("LIBCLANG_NOTHREADS")) {
-ReparseTranslationUnitImpl();
-return result;
-  }
-
   llvm::CrashRecoveryContext CRC;
 
   if (!RunSafely(CRC, ReparseTranslationUnitImpl)) {
@@ -8164,7 +8153,7 @@
unsigned Size) {
   if (!Size)
 Size = GetSafetyThreadStackSize();
-  if (Size)
+  if (Size && !getenv("LIBCLANG_NOTHREADS"))
 return CRC.RunSafelyOnThread(Fn, Size);
   return CRC.RunSafely(Fn);
 }


Index: tools/libclang/Indexing.cpp
===
--- tools/libclang/Indexing.cpp
+++ tools/libclang/Indexing.cpp
@@ -880,11 +880,6 @@
 TU_options);
   };
 
-  if (getenv("LIBCLANG_NOTHREADS")) {
-IndexSourceFileImpl();
-return result;
-  }
-
   llvm::CrashRecoveryContext CRC;
 
   if (!RunSafely(CRC, IndexSourceFileImpl)) {
@@ -934,11 +929,6 @@
 index_options, TU);
   };
 
-  if (getenv("LIBCLANG_NOTHREADS")) {
-IndexTranslationUnitImpl();
-return result;
-  }
-
   llvm::CrashRecoveryContext CRC;
 
   if (!RunSafely(CRC, IndexTranslationUnitImpl)) {
Index: tools/libclang/CIndexCodeCompletion.cpp
===
--- tools/libclang/CIndexCodeCompletion.cpp
+++ tools/libclang/CIndexCodeCompletion.cpp
@@ -806,11 +806,6 @@
 llvm::makeArrayRef(unsaved_files, num_unsaved_files), options);
   };
 
-  if (getenv("LIBCLANG_NOTHREADS")) {
-CodeCompleteAtImpl();
-return result;
-  }
-
   llvm::CrashRecoveryContext CRC;
 
   if (!RunSafely(CRC, CodeCompleteAtImpl)) {
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -3508,11 +3508,6 @@
 llvm::makeArrayRef(unsaved_files, num_unsaved_files), options, out_TU);
   };
 
-  if (getenv("LIBCLANG_NOTHREADS")) {
-ParseTranslationUnitImpl();
-return result;
-  }
-
   llvm::CrashRecoveryContext CRC;
 
   if (!RunSafely(CRC, ParseTranslationUnitImpl)) {
@@ -3921,8 +3916,7 @@
 result = clang_saveTranslationUnit_Impl(TU, FileName, options);
   };
 
-  if (!CXXUnit->getDiagnostics().hasUnrecoverableErrorOccurred() ||
-  getenv("LIBCLANG_NOTHREADS")) {
+  if (!CXXUnit->getDiagnostics().hasUnrecoverableErrorOccurred()) {
 SaveTranslationUnitImpl();
 
 if (getenv("LIBCLANG_RESOURCE_USAGE"))
@@ -4045,11 +4039,6 @@
 TU, llvm::makeArrayRef(unsaved_files, num_unsaved_files), options);
   };
 
-  if (getenv("LIBCLANG_NOTHREADS")) {
-ReparseTranslationUnitImpl();
-return result;
-  }
-
   llvm::CrashRecoveryContext CRC;
 
   if (!RunSafely(CRC, ReparseTranslationUnitImpl)) {
@@ -8164

[PATCH] D37554: [libclang] Allow crash recovery with LIBCLANG_NOTHREADS

2017-10-23 Thread Nikolai Kosjar via Phabricator via cfe-commits

nik added a comment.

Hmm, apparently "arc diff --update https://reviews.llvm.org/D37554"; did not 
take the new commit message into account. Changed it manually with the web 
interface.

Ilya, I hope it's OK if I take your description :)

> That said, I am not familiar with the code you're changing, so can't really 
> LGTM this. Hopefully, someone else can do that.

So who would be the appropriate reviewer here? Manuel, any idea?


https://reviews.llvm.org/D37554



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

[PATCH] D38819: [libunwind] Add support for dwarf unwinding on windows on x86_64

2017-10-23 Thread Martin Storsjö via Phabricator via cfe-commits

mstorsjo added inline comments.

Comment at: src/UnwindRegistersRestore.S:98
+  # skip fs
+  # skip gs
+  movq  56(%rcx), %rsp  # cut back rsp to new location

mstorsjo wrote:
> compnerd wrote:
> > Doesn't Win64 ABI require some of the MMX registers be saved/restored too?
> Right, yes, xmm6-xmm15 should be backed up and restored. I'll try to amend 
> this with such a change.
Actually, such a change doesn't necessarily make much sense on its own.

As long as the dwarf encoding itself doesn't describe how to restore those 
registers (and on unix platforms you don't need to, so it probably isn't even 
specified), you'd just end up backing up the xmm registers on entry when 
throwing the exception, and restoring the exactly same ones again - it only 
guards against changes within libcxxabi/libunwind and the unwinding machinery 
itself, not against changes further down in the call stack between the thrower 
and catcher of the exception.

So with that, I guess this patch is futile unless planning to extend the x86_64 
dwarf handling in llvm to include those registers as well - and that's a little 
out of scope of what I intended to do here...

https://reviews.llvm.org/D38819

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

[PATCH] D39178: [rename] support renaming class member.

2017-10-23 Thread Haojian Wu via Phabricator via cfe-commits

hokein created this revision.
Herald added subscribers: mgorny, klimek.

https://reviews.llvm.org/D39178

Files:
  lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
  unittests/Rename/CMakeLists.txt
  unittests/Rename/RenameMemberTest.cpp

Index: unittests/Rename/RenameMemberTest.cpp
===
--- /dev/null
+++ unittests/Rename/RenameMemberTest.cpp
@@ -0,0 +1,224 @@
+//===-- ClangMemberTests.cpp - unit tests for renaming class members --===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "ClangRenameTest.h"
+
+namespace clang {
+namespace clang_rename {
+namespace test {
+namespace {
+
+class RenameMemberTest : public ClangRenameTest {
+public:
+  RenameMemberTest() {
+AppendToHeader(R"(
+struct NA {
+  void Foo();
+  void NotFoo();
+  static void SFoo();
+  static void SNotFoo();
+  int Moo;
+};
+struct A {
+  virtual void Foo();
+  void NotFoo();
+  static void SFoo();
+  static void SNotFoo();
+  int Moo;
+  int NotMoo;
+};
+struct B : public A {
+  void Foo() override;
+};
+template  struct TA {
+  T* Foo();
+  T* NotFoo();
+  static T* SFoo();
+  static T* NotSFoo();
+};
+template  struct TB : public TA {};
+namespace ns {
+  template  struct TA {
+T* Foo();
+T* NotFoo();
+static T* SFoo();
+static T* NotSFoo();
+  };
+  template  struct TB : public TA {};
+  struct A {
+void Foo();
+void NotFoo();
+static void SFoo();
+static void SNotFoo();
+  };
+  struct B : public A {};
+  struct C {
+template 
+void SFoo(const T& t) {}
+template 
+void Foo() {}
+  };
+})");
+  }
+};
+
+INSTANTIATE_TEST_CASE_P(
+DISABLED_RenameTemplateMethodTest, RenameMemberTest,
+testing::ValuesIn(std::vector({
+// FIXME: support renaming static template methods for template classs.
+//
+// A template static class method of a template class could have more
+// than one AST node (each class specialization has its own instantiated
+// method). The current rename library only finds the NamedDecl of the
+// template method for given qualified name. To support this, we need
+// to record all NamedDecls (including the instantiated ones) which the
+// static class method corresponds to.
+{"void f() { TA::SFoo(); }", "void f() { TA::SBar(); }",
+ "TA::SFoo", "TA::SBar"},
+{"void f() { ns::TA::SFoo(); }",
+ "void f() { ns::TA::SBar(); }", "ns::TA::SFoo", "ns::TA::SBar"},
+{"void f() { TB::SFoo(); }", "void f() { TA::SBar(); }",
+ "TA::SFoo", "TA::SBar"},
+{"void f() { ns::TB::SFoo(); }",
+ "void f() { ns::TA::SBar(); }", "ns::TA::SFoo", "ns::TA::SBar"},
+})), );
+
+INSTANTIATE_TEST_CASE_P(
+RenameMemberTest, RenameMemberTest,
+testing::ValuesIn(std::vector({
+// Normal methods and fields.
+{"void f() { A a; a.Foo(); }", "void f() { A a; a.Bar(); }", "A::Foo",
+ "A::Bar"},
+{"void f() { ns::A a; a.Foo(); }", "void f() { ns::A a; a.Bar(); }",
+ "ns::A::Foo", "ns::A::Bar"},
+{"void f() { A a; int x = a.Moo; }", "void f() { A a; int x = a.Meh; }",
+ "A::Moo", "A::Meh"},
+{"void f() { B b; b.Foo(); }", "void f() { B b; b.Bar(); }", "B::Foo",
+ "B::Bar"},
+{"void f() { ns::B b; b.Foo(); }", "void f() { ns::B b; b.Bar(); }",
+ "ns::A::Foo", "ns::A::Bar"},
+{"void f() { B b; int x = b.Moo; }", "void f() { B b; int x = b.Meh; }",
+ "A::Moo", "A::Meh"},
+
+// Static methods.
+{"void f() { A::SFoo(); }", "void f() { A::SBar(); }", "A::SFoo",
+ "A::SBar"},
+{"void f() { ns::A::SFoo(); }", "void f() { ns::A::SBar(); }",
+ "ns::A::SFoo", "ns::A::SBar"},
+
+// Templated methods.
+{"void f() { TA a; a.Foo(); }", "void f() { TA a; a.Bar(); }",
+ "TA::Foo", "TA::Bar"},
+{"void f() { ns::TA a; a.Foo(); }",
+ "void f() { ns::TA a; a.Bar(); }", "ns::TA::Foo", "ns::TA::Bar"},
+{"void f() { TB b; b.Foo(); }", "void f() { TB b; b.Bar(); }",
+ "TA::Foo", "TA::Bar"},
+{"void f() { ns::TB b; b.Foo(); }",
+ "void f() { ns::TB b; b.Bar(); }", "ns::TA::Foo", "ns::TA::Bar"},
+{"void f() { ns::C c; int x; c.SFoo(x); }",
+ "void f() { ns::C c; int x; c.SBar(x); }", "ns::C::SFoo",
+

Re: r316268 - [Sema] Fixes for enum handling for tautological comparison diagnostics

2017-10-23 Thread Hans Wennborg via cfe-commits

This seems to have had the side effect of introducing a new warning in
Chromium builds:

../../third_party/expat/files/lib/xmlparse.c(2429,24):  error:
comparison of integers of different signs: 'enum XML_Error' and
'unsigned int' [-Werror,-Wsign-compare]
  if (code > 0 && code < sizeof(message)/sizeof(message[0]))
   ^ ~~

I'm not sure if this was intentional or not.

The warning seems technically correct here, though not very useful in
this specific case.

On Sat, Oct 21, 2017 at 6:44 PM, Roman Lebedev via cfe-commits
 wrote:
> Author: lebedevri
> Date: Sat Oct 21 09:44:03 2017
> New Revision: 316268
>
> URL: http://llvm.org/viewvc/llvm-project?rev=316268&view=rev
> Log:
> [Sema] Fixes for enum handling for tautological comparison diagnostics
>
> Summary:
> As Mattias Eriksson has reported in PR35009, in C, for enums, the underlying 
> type should
> be used when checking for the tautological comparison, unlike C++, where the 
> enumerator
> values define the value range. So if not in CPlusPlus mode, use the enum 
> underlying type.
>
> Also, i have discovered a problem (a crash) when evaluating tautological-ness 
> of the following comparison:
> ```
> enum A { A_a = 0 };
> if (a < 0) // expected-warning {{comparison of unsigned enum expression < 0 
> is always false}}
> return 0;
> ```
> This affects both the C and C++, but after the first fix, only C++ code was 
> affected.
> That was also fixed, while preserving (i think?) the proper diagnostic output.
>
> And while there, attempt to enhance the test coverage.
> Yes, some tests got moved around, sorry about that :)
>
> Fixes PR35009
>
> Reviewers: aaron.ballman, rsmith, rjmccall
>
> Reviewed By: aaron.ballman
>
> Subscribers: Rakete, efriedma, materi, cfe-commits
>
> Tags: #clang
>
> Differential Revision: https://reviews.llvm.org/D39122
>
> Added:
> cfe/trunk/test/Sema/outof-range-enum-constant-compare.c
> cfe/trunk/test/Sema/tautological-constant-enum-compare.c
> Modified:
> cfe/trunk/lib/Sema/SemaChecking.cpp
> cfe/trunk/test/Sema/tautological-unsigned-enum-zero-compare.c
> cfe/trunk/test/Sema/tautological-unsigned-enum-zero-compare.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=316268&r1=316267&r2=316268&view=diff
> ==
> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Sat Oct 21 09:44:03 2017
> @@ -8181,8 +8181,12 @@ struct IntRange {
>  if (const AtomicType *AT = dyn_cast(T))
>T = AT->getValueType().getTypePtr();
>
> -// For enum types, use the known bit width of the enumerators.
> -if (const EnumType *ET = dyn_cast(T)) {
> +if (!C.getLangOpts().CPlusPlus) {
> +  // For enum types in C code, use the underlying datatype.
> +  if (const EnumType *ET = dyn_cast(T))
> +T = ET->getDecl()->getIntegerType().getDesugaredType(C).getTypePtr();
> +} else if (const EnumType *ET = dyn_cast(T)) {
> +  // For enum types in C++, use the known bit width of the enumerators.
>EnumDecl *Enum = ET->getDecl();
>// In C++11, enums without definitions can have an explicitly specified
>// underlying type.  Use this type to compute the range.
> @@ -8584,8 +8588,10 @@ bool isNonBooleanUnsignedValue(Expr *E)
>  }
>
>  enum class LimitType {
> -  Max, // e.g. 32767 for short
> -  Min  // e.g. -32768 for short
> +  Max = 1U << 0U,  // e.g. 32767 for short
> +  Min = 1U << 1U,  // e.g. -32768 for short
> +  Both = Max | Min // When the value is both the Min and the Max limit at the
> +   // same time; e.g. in C++, A::a in enum A { a = 0 };
>  };
>
>  /// Checks whether Expr 'Constant' may be the
> @@ -8608,6 +8614,10 @@ llvm::Optional IsTypeLimit(Se
>
>IntRange OtherRange = IntRange::forValueOfType(S.Context, OtherT);
>
> +  // Special-case for C++ for enum with one enumerator with value of 0.
> +  if (OtherRange.Width == 0)
> +return Value == 0 ? LimitType::Both : llvm::Optional();
> +
>if (llvm::APSInt::isSameValue(
>llvm::APSInt::getMaxValue(OtherRange.Width,
>  OtherT->isUnsignedIntegerType()),
> @@ -8620,7 +8630,7 @@ llvm::Optional IsTypeLimit(Se
>Value))
>  return LimitType::Min;
>
> -  return llvm::Optional();
> +  return llvm::None;
>  }
>
>  bool HasEnumType(Expr *E) {
> @@ -8655,9 +8665,12 @@ bool CheckTautologicalComparison(Sema &S
>
>bool ConstIsLowerBound = (Op == BO_LT || Op == BO_LE) ^ RhsConstant;
>bool ResultWhenConstEqualsOther = (Op == BO_LE || Op == BO_GE);
> -  bool ResultWhenConstNeOther =
> -  ConstIsLowerBound ^ (ValueType == LimitType::Max);
> -  if (ResultWhenConstEqualsOther != ResultWhenConstNeOther)
> +  if (ValueType != LimitType::Both) {
> +bool ResultWhenConstNeOther =
> +Const

[PATCH] D38954: [Sema] -Wzero-as-null-pointer-constant: don't warn for system macros other than NULL.

2017-10-23 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri updated this revision to Diff 119833.
lebedev.ri retitled this revision from "[Sema] -Wzero-as-null-pointer-constant: 
don't warn for system macros." to "[Sema] -Wzero-as-null-pointer-constant: 
don't warn for system macros other than NULL.".
lebedev.ri edited the summary of this revision.
lebedev.ri added a reviewer: aaron.ballman.
lebedev.ri added a comment.

Ping.
Rebased, added release notes note.


Repository:
  rL LLVM

https://reviews.llvm.org/D38954

Files:
  docs/ReleaseNotes.rst
  lib/Sema/Sema.cpp
  test/SemaCXX/Inputs/warn-zero-nullptr.h
  test/SemaCXX/warn-zero-nullptr.cpp

Index: test/SemaCXX/warn-zero-nullptr.cpp
===
--- test/SemaCXX/warn-zero-nullptr.cpp
+++ test/SemaCXX/warn-zero-nullptr.cpp
@@ -1,4 +1,9 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s -Wzero-as-null-pointer-constant -std=c++11
+// RUN: %clang_cc1 -fsyntax-only -verify %s -isystem %S/Inputs -Wzero-as-null-pointer-constant -std=c++11
+
+#include 
+
+#define MACRO (0)
+#define MCRO(x) (x)
 
 struct S {};
 
@@ -15,13 +20,60 @@
 void (*fp2)() = __null; // expected-warning{{zero as null pointer constant}}
 int (S::*mp2) = __null; // expected-warning{{zero as null pointer constant}}
 
-void f0(void* v = 0); // expected-warning{{zero as null pointer constant}}
-void f1(void* v);
+void f0(void* v = MACRO); // expected-warning{{zero as null pointer constant}}
+void f1(void* v = NULL); // expected-warning{{zero as null pointer constant}}
+void f2(void* v = MCRO(0)); // expected-warning{{zero as null pointer constant}}
+void f3(void* v = MCRO(NULL)); // expected-warning{{zero as null pointer constant}}
+void f4(void* v = 0); // expected-warning{{zero as null pointer constant}}
+void f5(void* v);
 
 void g() {
   f1(0); // expected-warning{{zero as null pointer constant}}
 }
 
 // Warn on these too. Matches gcc and arguably makes sense.
 void* pp = (decltype(nullptr))0; // expected-warning{{zero as null pointer constant}}
 void* pp2 = static_cast(0); // expected-warning{{zero as null pointer constant}}
+
+template  void TmplFunc0(T var) {}
+void Func0Test() {
+  TmplFunc0(0);
+  TmplFunc0(0); // expected-warning {{zero as null pointer constant}}
+  TmplFunc0(0); // expected-warning {{zero as null pointer constant}}
+}
+
+// FIXME: this one should *NOT* warn.
+template  void TmplFunc1(int a, T default_value = 0) {} // expected-warning{{zero as null pointer constant}} expected-warning{{zero as null pointer constant}}
+void FuncTest() {
+  TmplFunc1(0);
+  TmplFunc1(0); // expected-note {{in instantiation of default function argument expression for 'TmplFunc1' required here}}
+  TmplFunc1(0);  // expected-note {{in instantiation of default function argument expression for 'TmplFunc1' required here}}
+}
+
+template
+class TemplateClass0 {
+ public:
+  explicit TemplateClass0(T var) {}
+};
+void TemplateClass0Test() {
+  TemplateClass0 a(0);
+  TemplateClass0 b(0); // expected-warning {{zero as null pointer constant}}
+  TemplateClass0 c(0); // expected-warning {{zero as null pointer constant}}
+}
+
+template
+class TemplateClass1 {
+ public:
+// FIXME: this one should *NOT* warn.
+  explicit TemplateClass1(int a, T default_value = 0) {} // expected-warning{{zero as null pointer constant}} expected-warning{{zero as null pointer constant}}
+};
+void IgnoreSubstTemplateType1() {
+  TemplateClass1 a(1);
+  TemplateClass1 b(1); // expected-note {{in instantiation of default function argument expression for 'TemplateClass1' required here}}
+  TemplateClass1 c(1); // expected-note {{in instantiation of default function argument expression for 'TemplateClass1' required here}}
+}
+
+// Do not warn on *any* other macros from system headers, even if they
+// expand to/their expansion contains NULL.
+void* sys_init = SYSTEM_MACRO;
+void* sys_init2 = OTHER_SYSTEM_MACRO;
Index: test/SemaCXX/Inputs/warn-zero-nullptr.h
===
--- /dev/null
+++ test/SemaCXX/Inputs/warn-zero-nullptr.h
@@ -0,0 +1,3 @@
+#define NULL (0)
+#define SYSTEM_MACRO (0)
+#define OTHER_SYSTEM_MACRO (NULL)
Index: lib/Sema/Sema.cpp
===
--- lib/Sema/Sema.cpp
+++ lib/Sema/Sema.cpp
@@ -440,6 +440,14 @@
 return;
   if (E->getType()->isNullPtrType())
 return;
+
+  // If it is a macro from system header, and if the macro name is not "NULL",
+  // do not warn.
+  SourceLocation MaybeMacroLoc = E->getLocStart();
+  if (SourceMgr.isInSystemMacro(E->getLocStart()) &&
+  !findMacroSpelling(MaybeMacroLoc, "NULL"))
+return;
+
   // nullptr only exists from C++11 on, so don't warn on its absence earlier.
   if (!getLangOpts().CPlusPlus11)
 return;
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -87,6 +87,9 @@
   offset is nonzero. It also now warns about arithmetic on a null pointer

Re: r316268 - [Sema] Fixes for enum handling for tautological comparison diagnostics

2017-10-23 Thread Roman Lebedev via cfe-commits

On Mon, Oct 23, 2017 at 2:13 PM, Hans Wennborg  wrote:
Hi.

> This seems to have had the side effect of introducing a new warning in
> Chromium builds:
>
> ../../third_party/expat/files/lib/xmlparse.c(2429,24):  error:
> comparison of integers of different signs: 'enum XML_Error' and
> 'unsigned int' [-Werror,-Wsign-compare]
>   if (code > 0 && code < sizeof(message)/sizeof(message[0]))
>    ^ ~~
(I guess this is on windows)

> I'm not sure if this was intentional or not.
>
> The warning seems technically correct here, though not very useful in
> this specific case.
I *believe* that was caused by the fix for IntRange::forValueOfCanonicalType()
to use enum's underlying type for C code. That fix was absolutely intentional.

However that new -Wsign-compare diagnostic (and i suspect there may be
more repercussions) was not really intentional.
However as you have said, and i think i agree, the diagnostic valid.

So perhaps i simply should add a test and release notes entry?

Roman.

> On Sat, Oct 21, 2017 at 6:44 PM, Roman Lebedev via cfe-commits
>  wrote:
>> Author: lebedevri
>> Date: Sat Oct 21 09:44:03 2017
>> New Revision: 316268
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=316268&view=rev
>> Log:
>> [Sema] Fixes for enum handling for tautological comparison diagnostics
>>
>> Summary:
>> As Mattias Eriksson has reported in PR35009, in C, for enums, the underlying 
>> type should
>> be used when checking for the tautological comparison, unlike C++, where the 
>> enumerator
>> values define the value range. So if not in CPlusPlus mode, use the enum 
>> underlying type.
>>
>> Also, i have discovered a problem (a crash) when evaluating 
>> tautological-ness of the following comparison:
>> ```
>> enum A { A_a = 0 };
>> if (a < 0) // expected-warning {{comparison of unsigned enum expression < 0 
>> is always false}}
>> return 0;
>> ```
>> This affects both the C and C++, but after the first fix, only C++ code was 
>> affected.
>> That was also fixed, while preserving (i think?) the proper diagnostic 
>> output.
>>
>> And while there, attempt to enhance the test coverage.
>> Yes, some tests got moved around, sorry about that :)
>>
>> Fixes PR35009
>>
>> Reviewers: aaron.ballman, rsmith, rjmccall
>>
>> Reviewed By: aaron.ballman
>>
>> Subscribers: Rakete, efriedma, materi, cfe-commits
>>
>> Tags: #clang
>>
>> Differential Revision: https://reviews.llvm.org/D39122
>>
>> Added:
>> cfe/trunk/test/Sema/outof-range-enum-constant-compare.c
>> cfe/trunk/test/Sema/tautological-constant-enum-compare.c
>> Modified:
>> cfe/trunk/lib/Sema/SemaChecking.cpp
>> cfe/trunk/test/Sema/tautological-unsigned-enum-zero-compare.c
>> cfe/trunk/test/Sema/tautological-unsigned-enum-zero-compare.cpp
>>
>> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=316268&r1=316267&r2=316268&view=diff
>> ==
>> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Sat Oct 21 09:44:03 2017
>> @@ -8181,8 +8181,12 @@ struct IntRange {
>>  if (const AtomicType *AT = dyn_cast(T))
>>T = AT->getValueType().getTypePtr();
>>
>> -// For enum types, use the known bit width of the enumerators.
>> -if (const EnumType *ET = dyn_cast(T)) {
>> +if (!C.getLangOpts().CPlusPlus) {
>> +  // For enum types in C code, use the underlying datatype.
>> +  if (const EnumType *ET = dyn_cast(T))
>> +T = 
>> ET->getDecl()->getIntegerType().getDesugaredType(C).getTypePtr();
>> +} else if (const EnumType *ET = dyn_cast(T)) {
>> +  // For enum types in C++, use the known bit width of the enumerators.
>>EnumDecl *Enum = ET->getDecl();
>>// In C++11, enums without definitions can have an explicitly 
>> specified
>>// underlying type.  Use this type to compute the range.
>> @@ -8584,8 +8588,10 @@ bool isNonBooleanUnsignedValue(Expr *E)
>>  }
>>
>>  enum class LimitType {
>> -  Max, // e.g. 32767 for short
>> -  Min  // e.g. -32768 for short
>> +  Max = 1U << 0U,  // e.g. 32767 for short
>> +  Min = 1U << 1U,  // e.g. -32768 for short
>> +  Both = Max | Min // When the value is both the Min and the Max limit at 
>> the
>> +   // same time; e.g. in C++, A::a in enum A { a = 0 };
>>  };
>>
>>  /// Checks whether Expr 'Constant' may be the
>> @@ -8608,6 +8614,10 @@ llvm::Optional IsTypeLimit(Se
>>
>>IntRange OtherRange = IntRange::forValueOfType(S.Context, OtherT);
>>
>> +  // Special-case for C++ for enum with one enumerator with value of 0.
>> +  if (OtherRange.Width == 0)
>> +return Value == 0 ? LimitType::Both : llvm::Optional();
>> +
>>if (llvm::APSInt::isSameValue(
>>llvm::APSInt::getMaxValue(OtherRange.Width,
>>  OtherT->isUnsignedIntegerType()),
>> @@ -8620,7 +8

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

2017-10-23 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added inline comments.



Comment at: clangd/ClangdUnit.cpp:103
 
+  void AfterExecute(CompilerInstance &CI) override {
+const SourceManager &SM = CI.getSourceManager();

Nebiroth wrote:
> ilya-biryukov wrote:
> > There's a much better public API to get all includes that were encountered 
> > by the `Preprocessor`: we need to override `PPCallbacks 
> > ::InclusionDirective`.
> > 
> > 
> > `PrecompiledPreamble` does not currently expose this callbacks, but could 
> > you add to `PreambleCallbacks` in a separate commit?
> > 
> If I were to use InclusionDirective , how would that callback be called 
> automatically? As far as I know, it wouldn't be called automatically for 
> every file that gets indexed the same way AfterExecute would be.
It will be called for each `#include` directive that Preprocessor encountered 
while building the preamble.
A separate instance of `CppFilePreambleCallbacks` is created every time we 
build a preamble for file.

Does that answer your question?


https://reviews.llvm.org/D38639



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

[PATCH] D39179: [AArch64] Fix PR34625 -mtune without -mcpu should not set -target-cpu

2017-10-23 Thread Peter Smith via Phabricator via cfe-commits

peter.smith created this revision.
Herald added subscribers: javed.absar, rengolin, aemerson.

When -mtune is used on AArch64 the -target-cpu is passed the value of the cpu 
given to -mtune. As well as setting micro-architectural features of the -mtune 
cpu, this will also add the architectural features such as support for 
instructions. This can result in the backend using instructions that are 
supported in the -mtune cpu but not supported in the target architecture. For 
example use of the v8.1-a LSE extensions with -march=v8.

  

This change removes the setting of -target-cpu for -mtune, the -mcpu must be 
used to set -target-cpu. This has the effect of removing all non-hard coded 
benefits of mtune but it does produce correct output when -mtune cpu with a 
later architecture than v8 is used. I've kept the hard-coded behaviour of 
-mtune for some Apple CPU codenames and have added in a call to check the 
-mtune cpu as there are some tests that depend on it.

Further work will be needed to implement -mtune properly, so that it affects 
only micro-architectural features.

fixes PR34625


https://reviews.llvm.org/D39179

Files:
  lib/Driver/ToolChains/Arch/AArch64.cpp
  test/Driver/aarch64-cpus.c

Index: test/Driver/aarch64-cpus.c
===
--- test/Driver/aarch64-cpus.c
+++ test/Driver/aarch64-cpus.c
@@ -21,182 +21,205 @@
 // RUN: %clang -target aarch64 -mcpu=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35 %s
 // RUN: %clang -target aarch64 -mlittle-endian -mcpu=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35 %s
 // RUN: %clang -target aarch64_be -mlittle-endian -mcpu=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35 %s
-// RUN: %clang -target aarch64 -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35 %s
-// RUN: %clang -target aarch64 -mlittle-endian -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35 %s
-// RUN: %clang -target aarch64_be -mlittle-endian -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35 %s
+// RUN: %clang -target aarch64 -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35-TUNE %s
+// RUN: %clang -target aarch64 -mlittle-endian -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35-TUNE %s
+// RUN: %clang -target aarch64_be -mlittle-endian -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=CA35-TUNE %s
 // CA35: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "cortex-a35"
+// CA35-TUNE: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic"
 
 // RUN: %clang -target arm64 -mcpu=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA35 %s
 // RUN: %clang -target arm64 -mlittle-endian -mcpu=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA35 %s
-// RUN: %clang -target arm64 -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA35 %s
-// RUN: %clang -target arm64 -mlittle-endian -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA35 %s
+// RUN: %clang -target arm64 -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA35-TUNE %s
+// RUN: %clang -target arm64 -mlittle-endian -mtune=cortex-a35 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA35-TUNE %s
 // ARM64-CA35: "-cc1"{{.*}} "-triple" "arm64{{.*}}" "-target-cpu" "cortex-a35"
-
+// ARM64-CA35-TUNE: "-cc1"{{.*}} "-triple" "arm64{{.*}}" "-target-cpu" "generic"
 
 // RUN: %clang -target aarch64 -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53 %s
 // RUN: %clang -target aarch64 -mlittle-endian -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53 %s
 // RUN: %clang -target aarch64_be -mlittle-endian -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53 %s
-// RUN: %clang -target aarch64 -mtune=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53 %s
-// RUN: %clang -target aarch64_be -mlittle-endian -mtune=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53 %s
+// RUN: %clang -target aarch64 -mtune=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53-TUNE %s
+// RUN: %clang -target aarch64_be -mlittle-endian -mtune=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=CA53-TUNE %s
 // CA53: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "cortex-a53"
+// CA53-TUNE: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic"
 
 // RUN: %clang -target arm64 -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA53 %s
 // RUN: %clang -target arm64 -mlittle-endian -mcpu=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA53 %s
-// RUN: %clang -target arm64 -mtune=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA53 %s
-// RUN: %clang -target arm64 -mlittle-endian -mtune=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA53 %s
+// RUN: %clang -target arm64 -mtune=cortex-a53 -### -c %s 2>&1 | FileCheck -check-prefix=ARM64-CA53-TUNE %s
+// RUN: %clang -target arm64 -mlittle-endian -mtune=cortex-a53 -### -c %s 2>&1

[PATCH] D38939: [clangd] Handle exit notification (proper shutdown)

2017-10-23 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.

Oh, and other than those NITs, LGTM.

@rwols, you have commit access now, right?)


https://reviews.llvm.org/D38939



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

Re: [clang-tools-extra] r315323 - [clangd] Added forgotten return in UniqueFunction.

2017-10-23 Thread Ilya Biryukov via cfe-commits

Sorry for late response, was on vacation.

The following commit (r315325) adds code coverage for this case.
But you are correct, we were missing usages of the class with non-void
return type before that.


On Mon, Oct 16, 2017 at 7:12 PM, David Blaikie  wrote:

> Is there missing test coverage for this?
>
> On Tue, Oct 10, 2017 at 9:12 AM Ilya Biryukov via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: ibiryukov
>> Date: Tue Oct 10 09:12:47 2017
>> New Revision: 315323
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=315323&view=rev
>> Log:
>> [clangd] Added forgotten return in UniqueFunction.
>>
>> This hasn't bitten us because we only used functions returning
>> 'void'.
>>
>> Modified:
>> clang-tools-extra/trunk/clangd/Function.h
>>
>> Modified: clang-tools-extra/trunk/clangd/Function.h
>> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/
>> trunk/clangd/Function.h?rev=315323&r1=315322&r2=315323&view=diff
>> 
>> ==
>> --- clang-tools-extra/trunk/clangd/Function.h (original)
>> +++ clang-tools-extra/trunk/clangd/Function.h Tue Oct 10 09:12:47 2017
>> @@ -46,7 +46,7 @@ public:
>>
>>Ret operator()(Args... As) {
>>  assert(CallablePtr);
>> -CallablePtr->Call(std::forward(As)...);
>> +return CallablePtr->Call(std::forward(As)...);
>>}
>>
>>  private:
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>


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

Re: Attribute spelling policy

2017-10-23 Thread Hal Finkel via cfe-commits



On 10/21/2017 10:14 AM, Aaron Ballman via cfe-commits wrote:

Attributes come with multiple spelling flavors, but when it comes to
adding new attributes that are not present in other compiler tools
such as GCC or MSVC, we have done a poor job of being consistent with
which spelling flavors we adopt the attributes under. Some of our
attributes are specified with only an __attribute__ spelling (about
100), while others are specified with both __attribute__ and
[[clang::XXX]] (about 30), and still others are specified as only
[[clang::XXX]] attributes (only 1). This puts additional burden on
developers to remember which attributes are spelled with what syntax
and the various rules surrounding how to write attributes with that
spelling.

I am proposing that we take a more principled approach when adding new
attributes so that we provide a better user experience. Specifically,
when adding an attribute that other vendors do not support, the
attribute should be given an __attribute__ and [[clang::]] spelling
unless there's good reason not to. This is not a novel proposal -- GCC
supports all of their documented __attribute__ spellings under a
[[gnu::XXX]] spelling, and I am proposing we do the same with our
vendor namespace.


For attributes that both Clang and GCC support, where GCC provides a 
[[gnu::X]] syntax, do you propose that our policy will be to support the 
same?




Assuming this approach is reasonable to the community,
  I will add a
CLANG spelling that behaves similar to the GCC spelling in that it
automatically provides both the GNU and CXX11 spellings as
appropriate. There are some attributes for which a [[clang::XXX]]
spelling is not appropriate:
   * attributes that appertain to function declarations but require
accessing the function parameters, such as disable_if or
requires_capability


Is this restriction related to the change that p0542 proposes to make to 
the interpretation of attributes that appear after functions as part of 
the contracts proposal?


Thanks again,
Hal


   * attributes with GNU spellings whose use is discouraged or
deprecated, such as no_sanitize_memory
   * attributes that are part of other vendor specifications, like CUDA or 
OpenCL
These deviations are reasonable, but should be documented in Attr.td
near the Spelling definition for the attribute so that it's explicitly
understood why the spelling differs.

Additionally, I intend for the proposed CLANG spelling to be extended
in the future to more easily expose [[clang::XXX]] spellings for
attributes intended to be used in C (with
-fdouble-square-bracket-attributes) as well as C++.

As always, feedback is welcome!

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


--
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory

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

[PATCH] D35894: [clangd] Code hover for Clangd

2017-10-23 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

In https://reviews.llvm.org/D35894#895023, @Nebiroth wrote:

> I would like some feedback/suggestions on whether it's possible to "append" 
> information to a MarkedString to be displayed on client?

You mean append **after** returning `MarkedString` to the client? I don't think 
that's possible.
Under what circumstances this could be useful? In general, it seems perfectly 
fine if we send the whole description+documentation right away.

https://reviews.llvm.org/D35894

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

[PATCH] D38425: [clangd] Document highlights for clangd

2017-10-23 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

> Another note: current implementation does not seem to handle macros at all 
> (it will only highlight definition of a macro, not its usages).

Current implementation still doesn't highlight macro references, only 
definitions. I'd either disable `documentHighlight` for macros for the initial 
version or come up with a way to implement `documentHighlight` for them.
I don't think `Preprocessor` currently stores information about all macro 
references, so we'll need to either store a separate index for that or rerun 
preprocessor to get this information.




Comment at: clangd/ClangdUnit.cpp:1017
+
+  auto DeclLocationsFinder = std::make_shared(
+  llvm::errs(), SourceLocationBeg, AST.getASTContext(),

Nebiroth wrote:
> ilya-biryukov wrote:
> > I wonder if we really need to rerun indexer twice here. Probably ok for the 
> > first version, but we should definitely think about a faster way to get the 
> > `Decl` under cursor than visiting the whole AST. Not sure if it's easy to 
> > do with clang's AST, though, maybe we'll need a separate index for that.
> Yeah I realise it's definitely not the fastest way to go about it. Maybe 
> there is a way to stop handling occurrences once the highlighted declaration 
> is found. This would be the most straightforward way of improving the 
> performance of the feature without re-writing a new AST indexer.
That only helps if we get lucky, average case and worst case would still be 
slow.
Anyway, this commit can certainly go in without this change.



Comment at: clangd/ClangdUnit.cpp:879
 public:
+  std::vector Decls;
+  std::vector MacroInfos;

Let's not store  `DeclarationLocations` anymore. If we have `Decl`s and 
`MacroInfo`s, we can move the code getting their `Location`s out of the 
`DeclarationLocationsFinder`.



Comment at: clangd/ClangdUnit.cpp:880
+  std::vector Decls;
+  std::vector MacroInfos;
   DeclarationLocationsFinder(raw_ostream &OS,

Please make the fields private, e.g. put them before `SearchedLocations`.



Comment at: clangd/ClangdUnit.cpp:976
+public:
+  std::vector Decls;
+  std::vector MacroInfos;

Make these fields private.
Don't copy `std::vector`s, store references to them instead.



Comment at: clangd/ClangdUnit.cpp:1003
+const SourceManager &SourceMgr = AST.getSourceManager();
+if (std::find(Decls.begin(), Decls.end(), D) != Decls.end()) {
+  SourceLocation Begin, End;

Please rewrite 
```
if (cond) {
  // long code
}
return true;
```
to 
```
if (!cond)
  return true;

// long code
```

This reduces nesting and makes code easier to read.
See [[ 
https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code
 | style guide ]].



Comment at: clangd/Protocol.h:431
+ */
+struct DocumentHighlight{
+  /**

ilya-biryukov wrote:
> There seems to be a missing brace before `{`. I generally run this simple 
> script before submission to make sure my code is always formatted:
> ```
> find "$CLANG_TOOLS_EXTRA_PATH/clangd" -name "*.cpp" -or -name "*.h" -exec 
> clang-format -i --style=LLVM {} \;
> find "$CLANG_TOOLS_EXTRA_PATH/unittests/clangd" -name "*.cpp" -or -name "*.h" 
> -exec clang-format -i --style=LLVM {} \ ;
> ```
BTW, the script had an error. (missing some parens)
```
find "$CLANG_TOOLS_EXTRA_PATH/clangd" \( -name "*.cpp" -or -name "*.h" \) -exec 
clang-format -i --style=LLVM {} \;
find "$CLANG_TOOLS_EXTRA_PATH/unittests/clangd" \( -name "*.cpp" -or -name 
"*.h" \) -exec clang-format -i --style=LLVM {} \ ;

```



Comment at: test/clangd/initialize-params.test:24
 {"jsonrpc":"2.0","id":3,"method":"shutdown"}
+
+

Redundant newlines at the end of the file.


https://reviews.llvm.org/D38425



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

[clang-tools-extra] r316323 - [clangd] Updated outdated test comment. NFC.

2017-10-23 Thread Ilya Biryukov via cfe-commits

Author: ibiryukov
Date: Mon Oct 23 07:08:52 2017
New Revision: 316323

URL: http://llvm.org/viewvc/llvm-project?rev=316323&view=rev
Log:
[clangd] Updated outdated test comment. NFC.

Modified:
clang-tools-extra/trunk/test/clangd/input-mirror.test

Modified: clang-tools-extra/trunk/test/clangd/input-mirror.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/input-mirror.test?rev=316323&r1=316322&r2=316323&view=diff
==
--- clang-tools-extra/trunk/test/clangd/input-mirror.test (original)
+++ clang-tools-extra/trunk/test/clangd/input-mirror.test Mon Oct 23 07:08:52 
2017
@@ -1,5 +1,5 @@
 # RUN: clangd -run-synchronously -input-mirror-file %t < %s
-# Note that we have to use '-Z' as -input-mirror-file does not have a newline 
at the end of file.
+# Note that we have to use '-b' as -input-mirror-file does not have a newline 
at the end of file.
 # RUN: diff -b %t %s
 # It is absolutely vital that this file has CRLF line endings.
 #


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

Re: [clang-tools-extra] r315287 - Revert "Revert r315214 since diff -Z isn't portable, this is breaking:"

2017-10-23 Thread Ilya Biryukov via cfe-commits

Missed that, will do, thanks.

On Wed, Oct 11, 2017 at 1:39 AM, Bruno Cardoso Lopes <
bruno.card...@gmail.com> wrote:

> On Tue, Oct 10, 2017 at 2:08 AM, Ilya Biryukov via cfe-commits
>  wrote:
> > Author: ibiryukov
> > Date: Tue Oct 10 02:08:47 2017
> > New Revision: 315287
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=315287&view=rev
> > Log:
> > Revert "Revert r315214 since diff -Z isn't portable, this is breaking:"
> >
> > This reverts commit r315242 and restores r315214.
> >
> > To fix original failure, replaced non-portable `diff -Z` with portable
> > alternative: `diff -b`.
> >
> > Added:
> > clang-tools-extra/trunk/test/clangd/input-mirror.test
> > Modified:
> > clang-tools-extra/trunk/clangd/JSONRPCDispatcher.cpp
> > clang-tools-extra/trunk/clangd/JSONRPCDispatcher.h
> > clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
> >
> > Modified: clang-tools-extra/trunk/clangd/JSONRPCDispatcher.cpp
> > URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/
> JSONRPCDispatcher.cpp?rev=315287&r1=315286&r2=315287&view=diff
> > 
> ==
> > --- clang-tools-extra/trunk/clangd/JSONRPCDispatcher.cpp (original)
> > +++ clang-tools-extra/trunk/clangd/JSONRPCDispatcher.cpp Tue Oct 10
> 02:08:47 2017
> > @@ -37,6 +37,14 @@ void JSONOutput::log(const Twine &Messag
> >Logs.flush();
> >  }
> >
> > +void JSONOutput::mirrorInput(const Twine &Message) {
> > +  if (!InputMirror)
> > +return;
> > +
> > +  *InputMirror << Message;
> > +  InputMirror->flush();
> > +}
> > +
> >  void Handler::handleMethod(llvm::yaml::MappingNode *Params, StringRef
> ID) {
> >Output.log("Method ignored.\n");
> >// Return that this method is unsupported.
> > @@ -147,6 +155,14 @@ void clangd::runLanguageServerLoop(std::
> >  continue;
> >}
> >
> > +  Out.mirrorInput(Line);
> > +  // Mirror '\n' that gets consumed by std::getline, but is not
> included in
> > +  // the resulting Line.
> > +  // Note that '\r' is part of Line, so we don't need to mirror it
> > +  // separately.
> > +  if (!In.eof())
> > +Out.mirrorInput("\n");
> > +
> >llvm::StringRef LineRef(Line);
> >
> >// We allow YAML-style comments in headers. Technically this
> isn't part
> > @@ -163,9 +179,8 @@ void clangd::runLanguageServerLoop(std::
> >if (LineRef.consume_front("Content-Length: ")) {
> >  if (ContentLength != 0) {
> >Out.log("Warning: Duplicate Content-Length header received. "
> > -  "The previous value for this message ("
> > -  + std::to_string(ContentLength)
> > -  + ") was ignored.\n");
> > +  "The previous value for this message (" +
> > +  std::to_string(ContentLength) + ") was ignored.\n");
> >  }
> >
> >  llvm::getAsUnsignedInteger(LineRef.trim(), 0, ContentLength);
> > @@ -185,15 +200,13 @@ void clangd::runLanguageServerLoop(std::
> >// parser.
> >std::vector JSON(ContentLength + 1, '\0');
> >In.read(JSON.data(), ContentLength);
> > +  Out.mirrorInput(StringRef(JSON.data(), In.gcount()));
> >
> >// If the stream is aborted before we read ContentLength bytes, In
> >// will have eofbit and failbit set.
> >if (!In) {
> > -Out.log("Input was aborted. Read only "
> > -+ std::to_string(In.gcount())
> > -+ " bytes of expected "
> > -+ std::to_string(ContentLength)
> > -+ ".\n");
> > +Out.log("Input was aborted. Read only " +
> std::to_string(In.gcount()) +
> > +" bytes of expected " + std::to_string(ContentLength) +
> ".\n");
> >  break;
> >}
> >
> > @@ -209,8 +222,8 @@ void clangd::runLanguageServerLoop(std::
> >if (IsDone)
> >  break;
> >  } else {
> > -  Out.log( "Warning: Missing Content-Length header, or message has
> zero "
> > -   "length.\n" );
> > +  Out.log("Warning: Missing Content-Length header, or message has
> zero "
> > +  "length.\n");
> >  }
> >}
> >  }
> >
> > Modified: clang-tools-extra/trunk/clangd/JSONRPCDispatcher.h
> > URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/
> JSONRPCDispatcher.h?rev=315287&r1=315286&r2=315287&view=diff
> > 
> ==
> > --- clang-tools-extra/trunk/clangd/JSONRPCDispatcher.h (original)
> > +++ clang-tools-extra/trunk/clangd/JSONRPCDispatcher.h Tue Oct 10
> 02:08:47 2017
> > @@ -24,8 +24,9 @@ namespace clangd {
> >  /// them.
> >  class JSONOutput : public Logger {
> >  public:
> > -  JSONOutput(llvm::raw_ostream &Outs, llvm::raw_ostream &Logs)
> > -  : Outs(Outs), Logs(Logs) {}
> > +  JSONOutput(llvm::raw_ostream &Outs, llvm::raw_ostream &Logs,
> > + llvm::raw_ostream *InputMirro

[PATCH] D38538: Avoid printing some redundant name qualifiers in completion

2017-10-23 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

Friendly ping. 
Would it be okay to make the new behaviour optional? (Leaving old behaviour to 
be the default).


https://reviews.llvm.org/D38538



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

[PATCH] D35894: [clangd] Code hover for Clangd

2017-10-23 Thread Marc-Andre Laperle via Phabricator via cfe-commits

malaperle added a comment.

In https://reviews.llvm.org/D35894#903542, @ilya-biryukov wrote:

> In https://reviews.llvm.org/D35894#895023, @Nebiroth wrote:
>
> > I would like some feedback/suggestions on whether it's possible to "append" 
> > information to a MarkedString to be displayed on client?
>
>
> You mean append **after** returning `MarkedString` to the client? I don't 
> think that's possible.
>  Under what circumstances this could be useful? In general, it seems 
> perfectly fine if we send the whole description+documentation right away.


I think he meant to have multiple sections in the hover, one C/C++ and one not. 
But we noticed you can have an array of MarkedString in Hover so it should be 
fine.


https://reviews.llvm.org/D35894



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

Re: Attribute spelling policy

2017-10-23 Thread Aaron Ballman via cfe-commits

On Mon, Oct 23, 2017 at 9:48 AM, Hal Finkel  wrote:
>
> On 10/21/2017 10:14 AM, Aaron Ballman via cfe-commits wrote:
>>
>> Attributes come with multiple spelling flavors, but when it comes to
>> adding new attributes that are not present in other compiler tools
>> such as GCC or MSVC, we have done a poor job of being consistent with
>> which spelling flavors we adopt the attributes under. Some of our
>> attributes are specified with only an __attribute__ spelling (about
>> 100), while others are specified with both __attribute__ and
>> [[clang::XXX]] (about 30), and still others are specified as only
>> [[clang::XXX]] attributes (only 1). This puts additional burden on
>> developers to remember which attributes are spelled with what syntax
>> and the various rules surrounding how to write attributes with that
>> spelling.
>>
>> I am proposing that we take a more principled approach when adding new
>> attributes so that we provide a better user experience. Specifically,
>> when adding an attribute that other vendors do not support, the
>> attribute should be given an __attribute__ and [[clang::]] spelling
>> unless there's good reason not to. This is not a novel proposal -- GCC
>> supports all of their documented __attribute__ spellings under a
>> [[gnu::XXX]] spelling, and I am proposing we do the same with our
>> vendor namespace.
>
>
> For attributes that both Clang and GCC support, where GCC provides a
> [[gnu::X]] syntax, do you propose that our policy will be to support the
> same?

Yes; we should use the [[gnu::X]] spelling instead of adding the same
attribute under [[clang::X]] unless we think our semantics need to
significantly differ from GCC's.

>> Assuming this approach is reasonable to the community,
>>   I will add a
>> CLANG spelling that behaves similar to the GCC spelling in that it
>> automatically provides both the GNU and CXX11 spellings as
>> appropriate. There are some attributes for which a [[clang::XXX]]
>> spelling is not appropriate:
>>* attributes that appertain to function declarations but require
>> accessing the function parameters, such as disable_if or
>> requires_capability
>
>
> Is this restriction related to the change that p0542 proposes to make to the
> interpretation of attributes that appear after functions as part of the
> contracts proposal?

Of sorts. P0542 describes the same problem we're running into with
disable_if and others, but its solution is highly concerning because
it removes the ability to specify any attributes on the function type
and it introduces an irregularity in the way attributes are specified.
I really don't like the direction taken in P0542, but I agree with the
problem and think it needs to be solved.

~Aaron

>
> Thanks again,
> Hal
>
>>* attributes with GNU spellings whose use is discouraged or
>> deprecated, such as no_sanitize_memory
>>* attributes that are part of other vendor specifications, like CUDA or
>> OpenCL
>> These deviations are reasonable, but should be documented in Attr.td
>> near the Spelling definition for the attribute so that it's explicitly
>> understood why the spelling differs.
>>
>> Additionally, I intend for the proposed CLANG spelling to be extended
>> in the future to more easily expose [[clang::XXX]] spellings for
>> attributes intended to be used in C (with
>> -fdouble-square-bracket-attributes) as well as C++.
>>
>> As always, feedback is welcome!
>>
>> ~Aaron
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
> --
> Hal Finkel
> Lead, Compiler Technology and Programming Languages
> Leadership Computing Facility
> Argonne National Laboratory
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r316327 - [clangd] Allow to pass code completion opts to ClangdServer.

2017-10-23 Thread Ilya Biryukov via cfe-commits

Author: ibiryukov
Date: Mon Oct 23 07:46:48 2017
New Revision: 316327

URL: http://llvm.org/viewvc/llvm-project?rev=316327&view=rev
Log:
[clangd] Allow to pass code completion opts to ClangdServer.

Reviewers: bkramer, krasimir, sammccall

Reviewed By: krasimir

Subscribers: klimek, cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/ClangdUnit.h
clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=316327&r1=316326&r2=316327&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Mon Oct 23 07:46:48 2017
@@ -193,7 +193,9 @@ ClangdLSPServer::ClangdLSPServer(JSONOut
  llvm::Optional CompileCommandsDir)
 : Out(Out), CDB(/*Logger=*/Out, std::move(CompileCommandsDir)),
   Server(CDB, /*DiagConsumer=*/*this, FSProvider, AsyncThreadsCount,
- SnippetCompletions, /*Logger=*/Out, ResourceDir) {}
+ clangd::CodeCompleteOptions(
+ /*EnableSnippetsAndCodePatterns=*/SnippetCompletions),
+ /*Logger=*/Out, ResourceDir) {}
 
 void ClangdLSPServer::run(std::istream &In) {
   assert(!IsDone && "Run was called before");

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=316327&r1=316326&r2=316327&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Mon Oct 23 07:46:48 2017
@@ -144,15 +144,15 @@ ClangdScheduler::~ClangdScheduler() {
 ClangdServer::ClangdServer(GlobalCompilationDatabase &CDB,
DiagnosticsConsumer &DiagConsumer,
FileSystemProvider &FSProvider,
-   unsigned AsyncThreadsCount, bool SnippetCompletions,
+   unsigned AsyncThreadsCount,
+   clangd::CodeCompleteOptions CodeCompleteOpts,
clangd::Logger &Logger,
llvm::Optional ResourceDir)
 : Logger(Logger), CDB(CDB), DiagConsumer(DiagConsumer),
   FSProvider(FSProvider),
   ResourceDir(ResourceDir ? ResourceDir->str() : getStandardResourceDir()),
   PCHs(std::make_shared()),
-  SnippetCompletions(SnippetCompletions), WorkScheduler(AsyncThreadsCount) 
{
-}
+  CodeCompleteOpts(CodeCompleteOpts), WorkScheduler(AsyncThreadsCount) {}
 
 void ClangdServer::setRootPath(PathRef RootPath) {
   std::string NewRootPath = llvm::sys::path::convert_to_slash(
@@ -237,7 +237,7 @@ ClangdServer::codeComplete(PathRef File,
 std::vector Result = clangd::codeComplete(
 File, Resources->getCompileCommand(),
 Preamble ? &Preamble->Preamble : nullptr, Contents, Pos, 
TaggedFS.Value,
-PCHs, SnippetCompletions, Logger);
+PCHs, CodeCompleteOpts, Logger);
 return make_tagged(std::move(Result), std::move(TaggedFS.Tag));
   });
 

Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=316327&r1=316326&r2=316327&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Mon Oct 23 07:46:48 2017
@@ -209,7 +209,8 @@ public:
   ClangdServer(GlobalCompilationDatabase &CDB,
DiagnosticsConsumer &DiagConsumer,
FileSystemProvider &FSProvider, unsigned AsyncThreadsCount,
-   bool SnippetCompletions, clangd::Logger &Logger,
+   clangd::CodeCompleteOptions CodeCompleteOpts,
+   clangd::Logger &Logger,
llvm::Optional ResourceDir = llvm::None);
 
   /// Set the root path of the workspace.
@@ -309,7 +310,7 @@ private:
   // If set, this represents the workspace path.
   llvm::Optional RootPath;
   std::shared_ptr PCHs;
-  bool SnippetCompletions;
+  clangd::CodeCompleteOptions CodeCompleteOpts;
   /// Used to serialize diagnostic callbacks.
   /// FIXME(ibiryukov): get rid of an extra map and put all version counters
   /// into CppFile.

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=316327&r1=316326&r2=316327&view=diff

[PATCH] D38731: [clangd] Allow to pass code completion opts to ClangdServer.

2017-10-23 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

In https://reviews.llvm.org/D38731#898539, @krasimir wrote:

> Great! I like the unit testing approach a lot!


Thanks!


Repository:
  rL LLVM

https://reviews.llvm.org/D38731



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

[PATCH] D38731: [clangd] Allow to pass code completion opts to ClangdServer.

2017-10-23 Thread Phabricator via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL316327: [clangd] Allow to pass code completion opts to 
ClangdServer. (authored by ibiryukov).

Changed prior to commit:
  https://reviews.llvm.org/D38731?vs=118385&id=119849#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38731

Files:
  clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
  clang-tools-extra/trunk/clangd/ClangdServer.cpp
  clang-tools-extra/trunk/clangd/ClangdServer.h
  clang-tools-extra/trunk/clangd/ClangdUnit.cpp
  clang-tools-extra/trunk/clangd/ClangdUnit.h
  clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp

Index: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
@@ -135,6 +135,33 @@
 namespace clangd {
 namespace {
 
+struct StringWithPos {
+  std::string Text;
+  clangd::Position MarkerPos;
+};
+
+/// Returns location of "{mark}" substring in \p Text and removes it from \p
+/// Text. Note that \p Text must contain exactly one occurence of "{mark}".
+///
+/// Marker name can be configured using \p MarkerName parameter.
+StringWithPos parseTextMarker(StringRef Text, StringRef MarkerName = "mark") {
+  SmallString<16> Marker;
+  Twine("{" + MarkerName + "}").toVector(/*ref*/ Marker);
+
+  std::size_t MarkerOffset = Text.find(Marker);
+  assert(MarkerOffset != StringRef::npos && "{mark} wasn't found in Text.");
+
+  std::string WithoutMarker;
+  WithoutMarker += Text.take_front(MarkerOffset);
+  WithoutMarker += Text.drop_front(MarkerOffset + Marker.size());
+  assert(StringRef(WithoutMarker).find(Marker) == StringRef::npos &&
+ "There were multiple occurences of {mark} inside Text");
+
+  clangd::Position MarkerPos =
+  clangd::offsetToPosition(WithoutMarker, MarkerOffset);
+  return {std::move(WithoutMarker), MarkerPos};
+}
+
 // Don't wait for async ops in clangd test more than that to avoid blocking
 // indefinitely in case of bugs.
 static const std::chrono::seconds DefaultFutureTimeout =
@@ -305,7 +332,7 @@
 ErrorCheckingDiagConsumer DiagConsumer;
 MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
 ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
-/*SnippetCompletions=*/false,
+clangd::CodeCompleteOptions(),
 EmptyLogger::getInstance());
 for (const auto &FileWithContents : ExtraFiles)
   FS.Files[getVirtualTestFilePath(FileWithContents.first)] =
@@ -369,7 +396,8 @@
   ErrorCheckingDiagConsumer DiagConsumer;
   MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
   ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
-  /*SnippetCompletions=*/false, EmptyLogger::getInstance());
+  clangd::CodeCompleteOptions(),
+  EmptyLogger::getInstance());
 
   const auto SourceContents = R"cpp(
 #include "foo.h"
@@ -414,7 +442,8 @@
   MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
 
   ClangdServer Server(CDB, DiagConsumer, FS, getDefaultAsyncThreadsCount(),
-  /*SnippetCompletions=*/false, EmptyLogger::getInstance());
+  clangd::CodeCompleteOptions(),
+  EmptyLogger::getInstance());
 
   const auto SourceContents = R"cpp(
 #include "foo.h"
@@ -461,7 +490,7 @@
   MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
   // Run ClangdServer synchronously.
   ClangdServer Server(CDB, DiagConsumer, FS,
-  /*AsyncThreadsCount=*/0, /*SnippetCompletions=*/false,
+  /*AsyncThreadsCount=*/0, clangd::CodeCompleteOptions(),
   EmptyLogger::getInstance());
 
   auto FooCpp = getVirtualTestFilePath("foo.cpp");
@@ -495,7 +524,7 @@
   "-stdlib=libstdc++"});
   // Run ClangdServer synchronously.
   ClangdServer Server(CDB, DiagConsumer, FS,
-  /*AsyncThreadsCount=*/0, /*SnippetCompletions=*/false,
+  /*AsyncThreadsCount=*/0, clangd::CodeCompleteOptions(),
   EmptyLogger::getInstance());
 
   // Just a random gcc version string
@@ -544,7 +573,7 @@
   ErrorCheckingDiagConsumer DiagConsumer;
   MockCompilationDatabase CDB(/*AddFreestandingFlag=*/true);
   ClangdServer Server(CDB, DiagConsumer, FS,
-  /*AsyncThreadsCount=*/0, /*SnippetCompletions=*/false,
+  /*AsyncThreadsCount=*/0, clangd::CodeCompleteOptions(),
   EmptyLogger::getInstance());
   // No need to sync reparses, because reparses are performed on the calling
   // thread to true.
@@ -589,22 +618,32 @@
 
 class ClangdCompletionTest : public ClangdVFSTest {
 protected:
-  bool ContainsItem(std::vector const &Items, StringRef Name) {
+  template 
+  bool

[PATCH] D37554: [libclang] Allow crash recovery with LIBCLANG_NOTHREADS

2017-10-23 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

In https://reviews.llvm.org/D37554#903401, @nik wrote:

> Ilya, I hope it's OK if I take your description :)


Sure, no problem.


https://reviews.llvm.org/D37554



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

[PATCH] D39178: [rename] support renaming class member.

2017-10-23 Thread Eric Liu via Phabricator via cfe-commits

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

Lgtm. Nice!




Comment at: lib/Tooling/Refactoring/Rename/USRLocFinder.cpp:251
+  // Ignore implicit initializers.
+  if (!Initializer->isWritten())
+continue;

Is there a test case for this?



Comment at: unittests/Rename/RenameMemberTest.cpp:206
+
+  X::X():a() {}
+  )";

Maybe add one more field in the test?


https://reviews.llvm.org/D39178



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

[PATCH] D35894: [clangd] Code hover for Clangd

2017-10-23 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

In https://reviews.llvm.org/D35894#903552, @malaperle wrote:

> I think he meant to have multiple sections in the hover, one C/C++ and one 
> not. But we noticed you can have an array of MarkedString in Hover so it 
> should be fine.

I totally misunderstood the question. Good to know it all works without 
changing the LSP.
Just to make sure I got it right this time: the use-case is to, e.g., return 
both a code snippet (having `"language": cpp`) and a documentation string 
(without `language`) as a result of hover request?

https://reviews.llvm.org/D35894

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

Re: Attribute spelling policy

2017-10-23 Thread Arthur O'Dwyer via cfe-commits

On Mon, Oct 23, 2017 at 7:33 AM, Aaron Ballman via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On Mon, Oct 23, 2017 at 9:48 AM, Hal Finkel  wrote:
> > On 10/21/2017 10:14 AM, Aaron Ballman via cfe-commits wrote:
> >>
> >> Attributes come with multiple spelling flavors, but when it comes to
> >> adding new attributes that are not present in other compiler tools
> >> such as GCC or MSVC, we have done a poor job of being consistent with
> >> which spelling flavors we adopt the attributes under. Some of our
> >> attributes are specified with only an __attribute__ spelling (about
> >> 100), while others are specified with both __attribute__ and
> >> [[clang::XXX]] (about 30), and still others are specified as only
> >> [[clang::XXX]] attributes (only 1). This puts additional burden on
> >> developers to remember which attributes are spelled with what syntax
> >> and the various rules surrounding how to write attributes with that
> >> spelling.
> >>
> >> I am proposing that we take a more principled approach when adding new
> >> attributes so that we provide a better user experience. Specifically,
> >> when adding an attribute that other vendors do not support, the
> >> attribute should be given an __attribute__ and [[clang::]] spelling
> >> unless there's good reason not to. This is not a novel proposal -- GCC
> >> supports all of their documented __attribute__ spellings under a
> >> [[gnu::XXX]] spelling, and I am proposing we do the same with our
> >> vendor namespace.
> >
> > For attributes that both Clang and GCC support, where GCC provides a
> > [[gnu::X]] syntax, do you propose that our policy will be to support the
> > same?
>
> Yes; we should use the [[gnu::X]] spelling instead of adding the same
> attribute under [[clang::X]] unless we think our semantics need to
> significantly differ from GCC's.
>

This seems like a departure from what you wrote originally, unless you just
misspoke or I misinterpreted.
You originally said, "[New attributes] should be given an __attribute__ and
[[clang::]] spelling unless there's good reason not to."  I would 100%
agree with that policy, from a user's point of view. It would be awesome to
know exactly how an attribute is spelled without having to look it up in
the manual.
But when you used the word "instead" just now, you implied that the policy
would be more like "New attributes should be given an __attribute__ and
[[clang::]] spelling unless there's good reason not to, *or unless Clang is
copying work originally done by the GNU project*, in which case the new
attribute should be given __attribute__ and [[gnu::]] spellings but not a
[[clang::]] spelling." Is that really what you meant, or was your original
statement closer to what you meant?

As a user, I would strongly prefer to write all my attributes in a
consistent way. If that way has to be __attribute__((foo)), then okay. But
in C++11-and-later, it would be nice to write all my attributes as
[[clang::foo]], and not have to keep consulting the manual to find out
which of [[foo]], [[clang::foo]], and [[gnu::foo]] is the proper spelling
for this particular attribute.

Of course the worst possible outcome would be where [[clang::foo]] and
[[gnu::foo]] both exist and have different semantics. I hope that doesn't
happen!  But if it has already happened, then it becomes even more
important that a user of Clang never need to use the [[gnu::]] spelling for
any Clang attribute (because accidentally using [[gnu::]] on the wrong
attribute could cause bugs, in the hypothetical case that [[clang::foo]]
and [[gnu::foo]] have different semantics).

Basically, as a user, I would like to see:
- Attribute names never deliberately collide with other vendors
- Every attribute supported by Clang is available under __attribute__
- Every attribute supported by Clang is available under [[clang::]]
- Any attribute that mimics a GNU feature is available under [[gnu::]] (and
also under the two above)
- Any attribute that mimics an MSVC feature is available under [[msvc::]]
(and also under the first two above)
- Any attribute that mimics a GNU-and-MSVC feature (thus, a feature
available on all three platforms) is available under all of [[gnu::]],
[[msvc::]], and [[clang::]] (and also under __attribute__)

Then, as a user, I don't need to care about the historical accident of
which project's employees invented a feature first; I can just use the
[[xxx::]] prefix corresponding to my primary compiler, and either it will
work or it will error out (in which case I'll know that my primary compiler
does not support that feature).

my $.02,
–Arthur
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35894: [clangd] Code hover for Clangd

2017-10-23 Thread Marc-Andre Laperle via Phabricator via cfe-commits

malaperle added a comment.

In https://reviews.llvm.org/D35894#903594, @ilya-biryukov wrote:

> In https://reviews.llvm.org/D35894#903552, @malaperle wrote:
>
> > I think he meant to have multiple sections in the hover, one C/C++ and one 
> > not. But we noticed you can have an array of MarkedString in Hover so it 
> > should be fine.
>
>
> I totally misunderstood the question. Good to know it all works without 
> changing the LSP.
>  Just to make sure I got it right this time: the use-case is to, e.g., return 
> both a code snippet (having `"language": cpp`) and a documentation string 
> (without `language`) as a result of hover request?


Exactly! Although the without-language part, maybe it'll be "scope" information 
first. Maybe documentation/comments in a second patch.


https://reviews.llvm.org/D35894



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

[PATCH] D35894: [clangd] Code hover for Clangd

2017-10-23 Thread William Enright via Phabricator via cfe-commits

Nebiroth added a comment.

In https://reviews.llvm.org/D35894#903594, @ilya-biryukov wrote:

> In https://reviews.llvm.org/D35894#903552, @malaperle wrote:
>
> > I think he meant to have multiple sections in the hover, one C/C++ and one 
> > not. But we noticed you can have an array of MarkedString in Hover so it 
> > should be fine.
>
>
> I totally misunderstood the question. Good to know it all works without 
> changing the LSP.
>  Just to make sure I got it right this time: the use-case is to, e.g., return 
> both a code snippet (having `"language": cpp`) and a documentation string 
> (without `language`) as a result of hover request?

That's about it.

I'm sitting on a code hover patch that does more or less what you just 
described. One MarkedString with it's language field set to "C++" returns the 
appropriate code snippet for the code hover and another MarkedString with it's 
language field set to "markdown" returns the associated scope.

https://reviews.llvm.org/D35894

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

[PATCH] D35894: [clangd] Code hover for Clangd

2017-10-23 Thread Ilya Biryukov via Phabricator via cfe-commits

ilya-biryukov added a comment.

In https://reviews.llvm.org/D35894#903607, @malaperle wrote:

> Exactly! Although the without-language part, maybe it'll be "scope" 
> information first. Maybe documentation/comments in a second patch.


Sure, whatever works/looks best.

In https://reviews.llvm.org/D35894#903611, @Nebiroth wrote:

> I'm sitting on a code hover patch that does more or less what you just 
> described. One MarkedString with it's language field set to "C++" returns the 
> appropriate code snippet for the code hover and another MarkedString with 
> it's language field set to "markdown" returns the associated scope.


Sounds cool, looking forward to reviewing it.


https://reviews.llvm.org/D35894



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

r316335 - clang-cl: Expose --version.

2017-10-23 Thread Nico Weber via cfe-commits

Author: nico
Date: Mon Oct 23 08:54:44 2017
New Revision: 316335

URL: http://llvm.org/viewvc/llvm-project?rev=316335&view=rev
Log:
clang-cl: Expose --version.

This is for consistency with lld-link, see https://reviews.llvm.org/D38972
Also give --version a help text so it shows up in --help / /? output (for
both clang-cl and regular clang).

Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/test/Driver/cl-options.c

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=316335&r1=316334&r2=316335&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Mon Oct 23 08:54:44 2017
@@ -2469,7 +2469,8 @@ def _rtlib : Separate<["--"], "rtlib">,
 def _serialize_diags : Separate<["-", "--"], "serialize-diagnostics">, 
Flags<[DriverOption]>,
   HelpText<"Serialize compiler diagnostics to a file">;
 // We give --version different semantics from -version.
-def _version : Flag<["--"], "version">,  Flags<[CC1Option]>;
+def _version : Flag<["--"], "version">, Flags<[CoreOption, CC1Option]>,
+  HelpText<"Print version information">;
 def _signed_char : Flag<["--"], "signed-char">, Alias;
 def _std : Separate<["--"], "std">, Alias;
 def _stdlib : Separate<["--"], "stdlib">, Alias;

Modified: cfe/trunk/test/Driver/cl-options.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-options.c?rev=316335&r1=316334&r2=316335&view=diff
==
--- cfe/trunk/test/Driver/cl-options.c (original)
+++ cfe/trunk/test/Driver/cl-options.c Mon Oct 23 08:54:44 2017
@@ -574,6 +574,7 @@
 // RUN: -fstandalone-debug \
 // RUN: -flimit-debug-info \
 // RUN: -flto \
+// RUN: --version \
 // RUN: -Werror /Zs -- %s 2>&1
 
 


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

[PATCH] D39184: CodeGen: Fix invalid bitcast in partial initialization of automatic arrary variable

2017-10-23 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl created this revision.

https://reviews.llvm.org/D39184

Files:
  lib/CodeGen/CGDecl.cpp
  test/CodeGenOpenCL/amdgcn-automatic-variable.cl


Index: test/CodeGenOpenCL/amdgcn-automatic-variable.cl
===
--- test/CodeGenOpenCL/amdgcn-automatic-variable.cl
+++ test/CodeGenOpenCL/amdgcn-automatic-variable.cl
@@ -58,3 +58,11 @@
   const int lvc = 4;
   lv1 = lvc;
 }
+
+// CHECK-LABEL: define void @func3()
+// CHECK: %a = alloca [16 x [1 x float]], align 4, addrspace(5)
+// CHECK: %[[CAST:.+]] = bitcast [16 x [1 x float]] addrspace(5)* %a to i8 
addrspace(5)*
+// CHECK: call void @llvm.memset.p5i8.i64(i8 addrspace(5)* %[[CAST]], i8 0, 
i64 64, i32 4, i1 false)
+void func3(void) {
+  float a[16][1] = {{0.}};
+}
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -1266,7 +1266,7 @@
 llvm::ConstantInt::get(IntPtrTy,

getContext().getTypeSizeInChars(type).getQuantity());
 
-  llvm::Type *BP = Int8PtrTy;
+  llvm::Type *BP = AllocaInt8PtrTy;
   if (Loc.getType() != BP)
 Loc = Builder.CreateBitCast(Loc, BP);
 


Index: test/CodeGenOpenCL/amdgcn-automatic-variable.cl
===
--- test/CodeGenOpenCL/amdgcn-automatic-variable.cl
+++ test/CodeGenOpenCL/amdgcn-automatic-variable.cl
@@ -58,3 +58,11 @@
   const int lvc = 4;
   lv1 = lvc;
 }
+
+// CHECK-LABEL: define void @func3()
+// CHECK: %a = alloca [16 x [1 x float]], align 4, addrspace(5)
+// CHECK: %[[CAST:.+]] = bitcast [16 x [1 x float]] addrspace(5)* %a to i8 addrspace(5)*
+// CHECK: call void @llvm.memset.p5i8.i64(i8 addrspace(5)* %[[CAST]], i8 0, i64 64, i32 4, i1 false)
+void func3(void) {
+  float a[16][1] = {{0.}};
+}
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -1266,7 +1266,7 @@
 llvm::ConstantInt::get(IntPtrTy,
getContext().getTypeSizeInChars(type).getQuantity());
 
-  llvm::Type *BP = Int8PtrTy;
+  llvm::Type *BP = AllocaInt8PtrTy;
   if (Loc.getType() != BP)
 Loc = Builder.CreateBitCast(Loc, BP);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: Attribute spelling policy

2017-10-23 Thread Aaron Ballman via cfe-commits

On Mon, Oct 23, 2017 at 11:16 AM, Arthur O'Dwyer
 wrote:
> On Mon, Oct 23, 2017 at 7:33 AM, Aaron Ballman via cfe-commits
>  wrote:
>>
>> On Mon, Oct 23, 2017 at 9:48 AM, Hal Finkel  wrote:
>> > On 10/21/2017 10:14 AM, Aaron Ballman via cfe-commits wrote:
>> >>
>> >> Attributes come with multiple spelling flavors, but when it comes to
>> >> adding new attributes that are not present in other compiler tools
>> >> such as GCC or MSVC, we have done a poor job of being consistent with
>> >> which spelling flavors we adopt the attributes under. Some of our
>> >> attributes are specified with only an __attribute__ spelling (about
>> >> 100), while others are specified with both __attribute__ and
>> >> [[clang::XXX]] (about 30), and still others are specified as only
>> >> [[clang::XXX]] attributes (only 1). This puts additional burden on
>> >> developers to remember which attributes are spelled with what syntax
>> >> and the various rules surrounding how to write attributes with that
>> >> spelling.
>> >>
>> >> I am proposing that we take a more principled approach when adding new
>> >> attributes so that we provide a better user experience. Specifically,
>> >> when adding an attribute that other vendors do not support, the
>> >> attribute should be given an __attribute__ and [[clang::]] spelling
>> >> unless there's good reason not to. This is not a novel proposal -- GCC
>> >> supports all of their documented __attribute__ spellings under a
>> >> [[gnu::XXX]] spelling, and I am proposing we do the same with our
>> >> vendor namespace.
>> >
>> > For attributes that both Clang and GCC support, where GCC provides a
>> > [[gnu::X]] syntax, do you propose that our policy will be to support the
>> > same?
>>
>> Yes; we should use the [[gnu::X]] spelling instead of adding the same
>> attribute under [[clang::X]] unless we think our semantics need to
>> significantly differ from GCC's.
>
>
> This seems like a departure from what you wrote originally, unless you just
> misspoke or I misinterpreted.
> You originally said, "[New attributes] should be given an __attribute__ and
> [[clang::]] spelling unless there's good reason not to."  I would 100% agree
> with that policy, from a user's point of view. It would be awesome to know
> exactly how an attribute is spelled without having to look it up in the
> manual.

Agreed.

> But when you used the word "instead" just now, you implied that the policy
> would be more like "New attributes should be given an __attribute__ and
> [[clang::]] spelling unless there's good reason not to, or unless Clang is
> copying work originally done by the GNU project, in which case the new
> attribute should be given __attribute__ and [[gnu::]] spellings but not a
> [[clang::]] spelling." Is that really what you meant, or was your original
> statement closer to what you meant?

That is really what I meant, but to be extra-double-clear:

* If we're adding a new attribute that has never existed under a
previous [[vendor::attr]] spelling, it should be __attribute__ and
[[clang::attr]].
* If we're adding a new-to-clang attribute that has existed under a
previous [[vendor::attr]] spelling:
  * if we intend for the attribute semantics to match the other
vendor, it should be [[vendor::attr]].
  * if we intend for the attribute semantics to differ from the other
vendor, it should be [[clang::attr]].
  * whether it's also __attribute__ may depend on the attribute being
added; for instance, we may or may not want to add an __attribute__
spelling for a [[gsl::attr]].

> As a user, I would strongly prefer to write all my attributes in a
> consistent way. If that way has to be __attribute__((foo)), then okay. But
> in C++11-and-later, it would be nice to write all my attributes as
> [[clang::foo]], and not have to keep consulting the manual to find out which
> of [[foo]], [[clang::foo]], and [[gnu::foo]] is the proper spelling for this
> particular attribute.

You will always have to figure out whether it's [[foo]] or
[[vendor::foo]] because [[foo]] is provided by the standard. However,
the point still stands of it being difficult to remember [[gnu::foo]]
vs [[clang::foo]]. However, I think that's a momentary bit of
confusion compared to the confusion of "where do I write this
attribute so that it works" that you get with __attribute__.

> Of course the worst possible outcome would be where [[clang::foo]] and
> [[gnu::foo]] both exist and have different semantics. I hope that doesn't
> happen!  But if it has already happened, then it becomes even more important
> that a user of Clang never need to use the [[gnu::]] spelling for any Clang
> attribute (because accidentally using [[gnu::]] on the wrong attribute could
> cause bugs, in the hypothetical case that [[clang::foo]] and [[gnu::foo]]
> have different semantics).
>
> Basically, as a user, I would like to see:
> - Attribute names never deliberately collide with other vendors

While that would be ideal, it also seems unlikely to be enforceable.
One point

[PATCH] D39086: Performance tracing facility for clangd.

2017-10-23 Thread Sam McCall via Phabricator via cfe-commits

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

Comment at: clangd/Trace.cpp:87
+
+static Tracer* T = nullptr;
+} // namespace

ilya-biryukov wrote:
> Maybe use `unique_ptr` (or even `llvm::Optional`)? Otherwise 
> we leak memory and don't flush the stream if users of tracing API forget to 
> call `stop()`. (I think I'm on the same page with @ioeric here  about 
> forgetting a call to `stop()`).
>  
> Having a static global of non-pod type is a no-go, but we could use static 
> local variables:
> 
> ```
> llvm::Optional& getTracerInstance() {
>  static llvm::Optional Tracer;
>  return Tracer;
> }
> ```
So Eric's solution of a scoped object that would be owned by main() makes sense 
to me, and I'm happy to add it (though personally not sure it carries its 
weight).

However I don't understand *your* suggestion :-)
> Having a static global of non-pod type is a no-go, but we could use static 
> local variables:
> llvm::Optional& getTracerInstance() {
>   static llvm::Optional Tracer;
>   return Tracer;
> }
So it's not global, which means we're not calling Optional() on startup, which 
would actually be totally safe (I guess the constructor should be marked as 
`constexpr`).

But we *are* calling `~Optional()` and therefore `~Tracer()` at shutdown. And 
maybe `~Tracer()` calls something that logs, and llvm::errs() has already been 
destroyed... *this* is why avoid static objects, right?

> Otherwise we leak memory and don't flush the stream if users of tracing API 
> forget to call stop()
The intended time to call `stop()` is at the end of the program. If you forget 
to call it, we won't be leaking memory :-) We indeed won't flush the stream, 
but that wouldn't be safe (see above).

The other option is the user is calling `start()` and `stop()` multiple times, 
and they call `start()` twice without stopping. We already assert on this, and 
anyone who doesn't reproducibly hit that assert is well into racy UB-land 
(logging methods may be called concurrently with each other, but everything 
else must be synchronized).

Overall I'm not really seeing what scenario we can detect and safely do 
something about.

https://reviews.llvm.org/D39086

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

[PATCH] D39086: Performance tracing facility for clangd.

2017-10-23 Thread Sam McCall via Phabricator via cfe-commits

sammccall updated this revision to Diff 119864.
sammccall added a comment.

Address FIXME now that r316330 is in.


https://reviews.llvm.org/D39086

Files:
  clangd/CMakeLists.txt
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/JSONRPCDispatcher.cpp
  clangd/ProtocolHandlers.cpp
  clangd/Trace.cpp
  clangd/Trace.h
  clangd/tool/ClangdMain.cpp
  test/clangd/trace.test
  unittests/clangd/CMakeLists.txt
  unittests/clangd/TraceTests.cpp

Index: unittests/clangd/TraceTests.cpp
===
--- /dev/null
+++ unittests/clangd/TraceTests.cpp
@@ -0,0 +1,117 @@
+//===-- TraceTests.cpp - Tracing unit tests -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "Trace.h"
+
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/SourceMgr.h"
+#include "llvm/Support/YAMLParser.h"
+#include "gtest/gtest.h"
+#include "gmock/gmock.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+using namespace llvm;
+
+MATCHER_P(StringNode, Val, "") {
+  if (arg->getType() != yaml::Node::NK_Scalar) {
+*result_listener << "is a " << arg->getVerbatimTag();
+return false;
+  }
+  SmallString<32> S;
+  return Val == static_cast(arg)->getValue(S);
+}
+
+// Checks that N is a Mapping (JS object) with the expected scalar properties.
+// The object must have all the Expected properties, but may have others.
+bool VerifyObject(yaml::Node &N, std::map Expected) {
+  auto* M = dyn_cast(&N);
+  if (!M) {
+ADD_FAILURE() << "Not an object";
+return false;
+  }
+  bool Match = true;
+  SmallString<32> Tmp;
+  for (auto Prop : *M) {
+auto* K = dyn_cast_or_null(Prop.getKey());
+if (!K) continue;
+std::string KS = K->getValue(Tmp).str();
+auto I = Expected.find(KS);
+if (I == Expected.end()) continue;  // Ignore properties with no assertion.
+
+auto* V = dyn_cast_or_null(Prop.getValue());
+if (!V) {
+  ADD_FAILURE() << KS << " is not a string";
+  Match = false;
+}
+std::string VS = V->getValue(Tmp).str();
+if (VS != I->second) {
+  ADD_FAILURE() << KS << " expected " << I->second << " but actual " << VS;
+  Match = false;
+}
+Expected.erase(I);
+  }
+  for (const auto& P : Expected) {
+ADD_FAILURE() << P.first << " missing, expected " << P.second;
+Match = false;
+  }
+  return Match;
+}
+
+TEST(TraceTest, SmokeTest) {
+  // Capture some events.
+  std::string JSON;
+  raw_string_ostream OS(JSON);
+  trace::start(OS);
+  {
+trace::Span S("A");
+trace::log("B");
+  }
+  trace::stop();
+
+  // Get the root JSON object using the YAML parser.
+  SourceMgr SM;
+  yaml::Stream Stream(JSON, SM);
+  auto Doc = Stream.begin();
+  ASSERT_NE(Doc, Stream.end());
+  auto* Root = dyn_cast_or_null(Doc->getRoot());
+  ASSERT_NE(Root, nullptr) << "Root should be an object";
+
+  // We expect in order:
+  //   displayTimeUnit: "ns"
+  //   traceEvents: [process name, thread name, start span, log, end span]
+  // (The order doesn't matter, but the YAML parser is awkward to use otherwise)
+  auto Prop = Root->begin();
+  ASSERT_NE(Prop, Root->end()) << "Expected displayTimeUnit property";
+  ASSERT_THAT(Prop->getKey(), StringNode("displayTimeUnit"));
+  EXPECT_THAT(Prop->getValue(), StringNode("ns"));
+  ASSERT_NE(++Prop, Root->end()) << "Expected traceEvents property";
+  EXPECT_THAT(Prop->getKey(), StringNode("traceEvents"));
+  auto* Events = dyn_cast_or_null(Prop->getValue());
+  ASSERT_NE(Events, nullptr) << "traceEvents should be an array";
+  auto Event = Events->begin();
+  ASSERT_NE(Event, Events->end()) << "Expected process name";
+  EXPECT_TRUE(VerifyObject(*Event, {{"ph", "M"}, {"name", "process_name"}}));
+  ASSERT_NE(++Event, Events->end()) << "Expected thread name";
+  EXPECT_TRUE(VerifyObject(*Event, {{"ph", "M"}, {"name", "thread_name"}}));
+  ASSERT_NE(++Event, Events->end()) << "Expected span start";
+  EXPECT_TRUE(VerifyObject(*Event, {{"ph", "B"}, {"name", "A"}}));
+  ASSERT_NE(++Event, Events->end()) << "Expected log message";
+  EXPECT_TRUE(VerifyObject(*Event, {{"ph", "i"}, {"name", "B"}}));
+  ASSERT_NE(++Event, Events->end()) << "Expected span end";
+  EXPECT_TRUE(VerifyObject(*Event, {{"ph", "E"}}));
+  ASSERT_EQ(++Event, Events->end());
+  ASSERT_EQ(++Prop, Root->end());
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: unittests/clangd/CMakeLists.txt
===
--- unittests/clangd/CMakeLists.txt
+++ unittests/clangd/CMakeLists.txt
@@ -10,6 +10,7 @@
 
 add_extra_unittest(ClangdTests
   ClangdTests.cpp
+  TraceTests.cpp
   )
 
 target_link_libraries(ClangdTests
Index: test/clangd/trace.test
===

[PATCH] D38704: [libunwind] Abstract rwlocks into a class, provide a SRW lock implementation for windows

2017-10-23 Thread Zachary Turner via Phabricator via cfe-commits

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

This looks much better than the previous solution.  Thanks!


https://reviews.llvm.org/D38704



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

r316338 - Pull X86 "CPUKind" checking into .cpp file. [NFC]

2017-10-23 Thread Erich Keane via cfe-commits

Author: erichkeane
Date: Mon Oct 23 09:20:15 2017
New Revision: 316338

URL: http://llvm.org/viewvc/llvm-project?rev=316338&view=rev
Log:
Pull X86 "CPUKind" checking into .cpp file. [NFC]

Preparing to do a refactor of CPU/feature checking, this
patch pulls the one CPU implementation from the .h file
to the .cpp file.

Modified:
cfe/trunk/lib/Basic/Targets/X86.cpp
cfe/trunk/lib/Basic/Targets/X86.h

Modified: cfe/trunk/lib/Basic/Targets/X86.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/X86.cpp?rev=316338&r1=316337&r2=316338&view=diff
==
--- cfe/trunk/lib/Basic/Targets/X86.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/X86.cpp Mon Oct 23 09:20:15 2017
@@ -1522,6 +1522,78 @@ std::string X86TargetInfo::convertConstr
   }
 }
 
+bool X86TargetInfo::checkCPUKind(CPUKind Kind) const {
+  // Perform any per-CPU checks necessary to determine if this CPU is
+  // acceptable.
+  // FIXME: This results in terrible diagnostics. Clang just says the CPU is
+  // invalid without explaining *why*.
+  switch (Kind) {
+  case CK_Generic:
+// No processor selected!
+return false;
+
+  case CK_i386:
+  case CK_i486:
+  case CK_WinChipC6:
+  case CK_WinChip2:
+  case CK_C3:
+  case CK_i586:
+  case CK_Pentium:
+  case CK_PentiumMMX:
+  case CK_i686:
+  case CK_PentiumPro:
+  case CK_Pentium2:
+  case CK_Pentium3:
+  case CK_PentiumM:
+  case CK_Yonah:
+  case CK_C3_2:
+  case CK_Pentium4:
+  case CK_Lakemont:
+  case CK_Prescott:
+  case CK_K6:
+  case CK_K6_2:
+  case CK_K6_3:
+  case CK_Athlon:
+  case CK_AthlonXP:
+  case CK_Geode:
+// Only accept certain architectures when compiling in 32-bit mode.
+if (getTriple().getArch() != llvm::Triple::x86)
+  return false;
+
+LLVM_FALLTHROUGH;
+  case CK_Nocona:
+  case CK_Core2:
+  case CK_Penryn:
+  case CK_Bonnell:
+  case CK_Silvermont:
+  case CK_Goldmont:
+  case CK_Nehalem:
+  case CK_Westmere:
+  case CK_SandyBridge:
+  case CK_IvyBridge:
+  case CK_Haswell:
+  case CK_Broadwell:
+  case CK_SkylakeClient:
+  case CK_SkylakeServer:
+  case CK_Cannonlake:
+  case CK_KNL:
+  case CK_KNM:
+  case CK_K8:
+  case CK_K8SSE3:
+  case CK_AMDFAM10:
+  case CK_BTVER1:
+  case CK_BTVER2:
+  case CK_BDVER1:
+  case CK_BDVER2:
+  case CK_BDVER3:
+  case CK_BDVER4:
+  case CK_ZNVER1:
+  case CK_x86_64:
+return true;
+  }
+  llvm_unreachable("Unhandled CPU kind");
+}
+
 X86TargetInfo::CPUKind X86TargetInfo::getCPUKind(StringRef CPU) const {
   return llvm::StringSwitch(CPU)
   .Case("i386", CK_i386)

Modified: cfe/trunk/lib/Basic/Targets/X86.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/X86.h?rev=316338&r1=316337&r2=316338&view=diff
==
--- cfe/trunk/lib/Basic/Targets/X86.h (original)
+++ cfe/trunk/lib/Basic/Targets/X86.h Mon Oct 23 09:20:15 2017
@@ -270,77 +270,7 @@ class LLVM_LIBRARY_VISIBILITY X86TargetI
 //@}
   } CPU = CK_Generic;
 
-  bool checkCPUKind(CPUKind Kind) const {
-// Perform any per-CPU checks necessary to determine if this CPU is
-// acceptable.
-// FIXME: This results in terrible diagnostics. Clang just says the CPU is
-// invalid without explaining *why*.
-switch (Kind) {
-case CK_Generic:
-  // No processor selected!
-  return false;
-
-case CK_i386:
-case CK_i486:
-case CK_WinChipC6:
-case CK_WinChip2:
-case CK_C3:
-case CK_i586:
-case CK_Pentium:
-case CK_PentiumMMX:
-case CK_i686:
-case CK_PentiumPro:
-case CK_Pentium2:
-case CK_Pentium3:
-case CK_PentiumM:
-case CK_Yonah:
-case CK_C3_2:
-case CK_Pentium4:
-case CK_Lakemont:
-case CK_Prescott:
-case CK_K6:
-case CK_K6_2:
-case CK_K6_3:
-case CK_Athlon:
-case CK_AthlonXP:
-case CK_Geode:
-  // Only accept certain architectures when compiling in 32-bit mode.
-  if (getTriple().getArch() != llvm::Triple::x86)
-return false;
-
-  LLVM_FALLTHROUGH;
-case CK_Nocona:
-case CK_Core2:
-case CK_Penryn:
-case CK_Bonnell:
-case CK_Silvermont:
-case CK_Goldmont:
-case CK_Nehalem:
-case CK_Westmere:
-case CK_SandyBridge:
-case CK_IvyBridge:
-case CK_Haswell:
-case CK_Broadwell:
-case CK_SkylakeClient:
-case CK_SkylakeServer:
-case CK_Cannonlake:
-case CK_KNL:
-case CK_KNM:
-case CK_K8:
-case CK_K8SSE3:
-case CK_AMDFAM10:
-case CK_BTVER1:
-case CK_BTVER2:
-case CK_BDVER1:
-case CK_BDVER2:
-case CK_BDVER3:
-case CK_BDVER4:
-case CK_ZNVER1:
-case CK_x86_64:
-  return true;
-}
-llvm_unreachable("Unhandled CPU kind");
-  }
+  bool checkCPUKind(CPUKind Kind) const;
 
   CPUKind getCPUKind(StringRef CPU) const;
 


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.ll

[PATCH] D39057: [clangd][WIP] Integrate the refactoring actions into clangd

2017-10-23 Thread Sam McCall via Phabricator via cfe-commits

sammccall added a comment.

As noted on the other patch, I'm not sure EditorCommands is a useful 
abstraction.

This shows up one of the problems: clangd is mostly decoupled from the actual 
behavior/semantics of the commands by the EditorCommands abstraction. However 
LSP doesn't say what the structure of ExecuteCommandParams.arguments is - it 
depends on the individual command. But clangd needs to parse those arguments 
out of the JSON! So we're left with all the EditorCommands needing to take the 
same set of arguments, which then get hardcoded in clangd.

Coupling clangd more closely to the refactorings seems simpler and more 
flexible - what would we be losing?

Other than the EditorCommands question, implementation looks fine!




Comment at: clangd/Protocol.h:535
+struct CommandArgument {
+  union {
+llvm::AlignedCharArrayUnion SelectionRange;

Why a union-of-unions instead of AlignedCharArrayUnion?


Repository:
  rL LLVM

https://reviews.llvm.org/D39057



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

[PATCH] D38954: [Sema] -Wzero-as-null-pointer-constant: don't warn for system macros other than NULL.

2017-10-23 Thread Nicolas Lesser via Phabricator via cfe-commits

Rakete added inline comments.



Comment at: lib/Sema/Sema.cpp:445
+  // If it is a macro from system header, and if the macro name is not "NULL",
+  // do not warn.
+  SourceLocation MaybeMacroLoc = E->getLocStart();

That comment doesn't really add anything IMO. It just says what the code just 
below says.



Comment at: lib/Sema/Sema.cpp:447
+  SourceLocation MaybeMacroLoc = E->getLocStart();
+  if (SourceMgr.isInSystemMacro(E->getLocStart()) &&
+  !findMacroSpelling(MaybeMacroLoc, "NULL"))

Please use `MaybeMacroLoc` there too.


Repository:
  rL LLVM

https://reviews.llvm.org/D38954



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

[libcxx] r316343 - Fix misguided error message in debug mode. No functional change. Fixes PR#34966

2017-10-23 Thread Marshall Clow via cfe-commits

Author: marshall
Date: Mon Oct 23 09:46:44 2017
New Revision: 316343

URL: http://llvm.org/viewvc/llvm-project?rev=316343&view=rev
Log:
Fix misguided error message in debug mode. No functional change. Fixes PR#34966

Modified:
libcxx/trunk/include/list

Modified: libcxx/trunk/include/list
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/list?rev=316343&r1=316342&r2=316343&view=diff
==
--- libcxx/trunk/include/list (original)
+++ libcxx/trunk/include/list Mon Oct 23 09:46:44 2017
@@ -481,7 +481,7 @@ public:
 {
 #if _LIBCPP_DEBUG_LEVEL >= 2
 _LIBCPP_ASSERT(__get_const_db()->__dereferenceable(this),
-   "Attempted to dereference a non-dereferenceable 
list::iterator");
+   "Attempted to dereference a non-dereferenceable 
list::const_iterator");
 #endif
 return 
pointer_traits::pointer_to(__ptr_->__as_node()->__value_);
 }


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

[PATCH] D38704: [libunwind] Abstract rwlocks into a class, provide a SRW lock implementation for windows

2017-10-23 Thread David Majnemer via Phabricator via cfe-commits

majnemer accepted this revision.
majnemer added a comment.

LGTM


https://reviews.llvm.org/D38704



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

r316344 - [ASTMatchers] Expose forEachOverriden in dynamic AST matchers.

2017-10-23 Thread Benjamin Kramer via cfe-commits

Author: d0k
Date: Mon Oct 23 09:48:46 2017
New Revision: 316344

URL: http://llvm.org/viewvc/llvm-project?rev=316344&view=rev
Log:
[ASTMatchers] Expose forEachOverriden in dynamic AST matchers.

Modified:
cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp

Modified: cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp?rev=316344&r1=316343&r2=316344&view=diff
==
--- cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp (original)
+++ cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp Mon Oct 23 09:48:46 2017
@@ -196,6 +196,7 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(forEachArgumentWithParam);
   REGISTER_MATCHER(forEachConstructorInitializer);
   REGISTER_MATCHER(forEachDescendant);
+  REGISTER_MATCHER(forEachOverridden);
   REGISTER_MATCHER(forEachSwitchCase);
   REGISTER_MATCHER(forField);
   REGISTER_MATCHER(forFunction);


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

[PATCH] D39184: CodeGen: Fix invalid bitcast in partial initialization of automatic arrary variable

2017-10-23 Thread John McCall via Phabricator via cfe-commits

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

LGTM.


https://reviews.llvm.org/D39184



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

[PATCH] D38954: [Sema] -Wzero-as-null-pointer-constant: don't warn for system macros other than NULL.

2017-10-23 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri updated this revision to Diff 119892.
lebedev.ri marked an inline comment as done.
lebedev.ri added a comment.

Use `MaybeMacroLoc` variable in the other place too.


Repository:
  rL LLVM

https://reviews.llvm.org/D38954

Files:
  docs/ReleaseNotes.rst
  lib/Sema/Sema.cpp
  test/SemaCXX/Inputs/warn-zero-nullptr.h
  test/SemaCXX/warn-zero-nullptr.cpp

Index: test/SemaCXX/warn-zero-nullptr.cpp
===
--- test/SemaCXX/warn-zero-nullptr.cpp
+++ test/SemaCXX/warn-zero-nullptr.cpp
@@ -1,4 +1,9 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s -Wzero-as-null-pointer-constant -std=c++11
+// RUN: %clang_cc1 -fsyntax-only -verify %s -isystem %S/Inputs -Wzero-as-null-pointer-constant -std=c++11
+
+#include 
+
+#define MACRO (0)
+#define MCRO(x) (x)
 
 struct S {};
 
@@ -15,13 +20,60 @@
 void (*fp2)() = __null; // expected-warning{{zero as null pointer constant}}
 int (S::*mp2) = __null; // expected-warning{{zero as null pointer constant}}
 
-void f0(void* v = 0); // expected-warning{{zero as null pointer constant}}
-void f1(void* v);
+void f0(void* v = MACRO); // expected-warning{{zero as null pointer constant}}
+void f1(void* v = NULL); // expected-warning{{zero as null pointer constant}}
+void f2(void* v = MCRO(0)); // expected-warning{{zero as null pointer constant}}
+void f3(void* v = MCRO(NULL)); // expected-warning{{zero as null pointer constant}}
+void f4(void* v = 0); // expected-warning{{zero as null pointer constant}}
+void f5(void* v);
 
 void g() {
   f1(0); // expected-warning{{zero as null pointer constant}}
 }
 
 // Warn on these too. Matches gcc and arguably makes sense.
 void* pp = (decltype(nullptr))0; // expected-warning{{zero as null pointer constant}}
 void* pp2 = static_cast(0); // expected-warning{{zero as null pointer constant}}
+
+template  void TmplFunc0(T var) {}
+void Func0Test() {
+  TmplFunc0(0);
+  TmplFunc0(0); // expected-warning {{zero as null pointer constant}}
+  TmplFunc0(0); // expected-warning {{zero as null pointer constant}}
+}
+
+// FIXME: this one should *NOT* warn.
+template  void TmplFunc1(int a, T default_value = 0) {} // expected-warning{{zero as null pointer constant}} expected-warning{{zero as null pointer constant}}
+void FuncTest() {
+  TmplFunc1(0);
+  TmplFunc1(0); // expected-note {{in instantiation of default function argument expression for 'TmplFunc1' required here}}
+  TmplFunc1(0);  // expected-note {{in instantiation of default function argument expression for 'TmplFunc1' required here}}
+}
+
+template
+class TemplateClass0 {
+ public:
+  explicit TemplateClass0(T var) {}
+};
+void TemplateClass0Test() {
+  TemplateClass0 a(0);
+  TemplateClass0 b(0); // expected-warning {{zero as null pointer constant}}
+  TemplateClass0 c(0); // expected-warning {{zero as null pointer constant}}
+}
+
+template
+class TemplateClass1 {
+ public:
+// FIXME: this one should *NOT* warn.
+  explicit TemplateClass1(int a, T default_value = 0) {} // expected-warning{{zero as null pointer constant}} expected-warning{{zero as null pointer constant}}
+};
+void IgnoreSubstTemplateType1() {
+  TemplateClass1 a(1);
+  TemplateClass1 b(1); // expected-note {{in instantiation of default function argument expression for 'TemplateClass1' required here}}
+  TemplateClass1 c(1); // expected-note {{in instantiation of default function argument expression for 'TemplateClass1' required here}}
+}
+
+// Do not warn on *any* other macros from system headers, even if they
+// expand to/their expansion contains NULL.
+void* sys_init = SYSTEM_MACRO;
+void* sys_init2 = OTHER_SYSTEM_MACRO;
Index: test/SemaCXX/Inputs/warn-zero-nullptr.h
===
--- /dev/null
+++ test/SemaCXX/Inputs/warn-zero-nullptr.h
@@ -0,0 +1,3 @@
+#define NULL (0)
+#define SYSTEM_MACRO (0)
+#define OTHER_SYSTEM_MACRO (NULL)
Index: lib/Sema/Sema.cpp
===
--- lib/Sema/Sema.cpp
+++ lib/Sema/Sema.cpp
@@ -440,6 +440,14 @@
 return;
   if (E->getType()->isNullPtrType())
 return;
+
+  // If it is a macro from system header, and if the macro name is not "NULL",
+  // do not warn.
+  SourceLocation MaybeMacroLoc = E->getLocStart();
+  if (SourceMgr.isInSystemMacro(MaybeMacroLoc) &&
+  !findMacroSpelling(MaybeMacroLoc, "NULL"))
+return;
+
   // nullptr only exists from C++11 on, so don't warn on its absence earlier.
   if (!getLangOpts().CPlusPlus11)
 return;
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -87,6 +87,9 @@
   offset is nonzero. It also now warns about arithmetic on a null pointer
   treated as a cast from integer to pointer (GNU extension).
 
+- ``-Wzero-as-null-pointer-constant`` was adjusted not to warn on null pointer
+  constants that originate from system macros, except ``NULL`` macro.
+
 Non-comprehensive list o

[PATCH] D36347: Add new script to launch lldb and set breakpoints for diagnostics all diagnostics seen.

2017-10-23 Thread Don Hinton via Phabricator via cfe-commits

hintonda added a comment.

Thanks for all the feedback.  I'll report back once I've addressed all your 
suggestions.

Thanks again...


https://reviews.llvm.org/D36347



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

[PATCH] D38954: [Sema] -Wzero-as-null-pointer-constant: don't warn for system macros other than NULL.

2017-10-23 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added inline comments.



Comment at: lib/Sema/Sema.cpp:445
+  // If it is a macro from system header, and if the macro name is not "NULL",
+  // do not warn.
+  SourceLocation MaybeMacroLoc = E->getLocStart();

Rakete wrote:
> That comment doesn't really add anything IMO. It just says what the code just 
> below says.
I don't disagree. It seems to be the common pattern for the code in this file.
Having both the code and the comment might help convey the idea better.


Repository:
  rL LLVM

https://reviews.llvm.org/D38954



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

[PATCH] D38704: [libunwind] Abstract rwlocks into a class, provide a SRW lock implementation for windows

2017-10-23 Thread Shoaib Meenai via Phabricator via cfe-commits

smeenai added a comment.

BTW LLVM requires a minimum of Windows 7 according to 
https://releases.llvm.org/3.8.0/docs/ReleaseNotes.html, so you can rely on SRW 
locks always being present. If LLVM still has a fallback path for when SRW 
locks are absent, that could be cleaned up.


https://reviews.llvm.org/D38704



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

[PATCH] D30946: [ScopePrinting] Added support to print full scopes of types and declarations.

2017-10-23 Thread Simon Schroeder via Phabricator via cfe-commits

schroedersi added a comment.

Ping


https://reviews.llvm.org/D30946



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

r316353 - CodeGen: Fix invalid bitcast in partial initialization of automatic arrary variable

2017-10-23 Thread Yaxun Liu via cfe-commits

Author: yaxunl
Date: Mon Oct 23 10:49:26 2017
New Revision: 316353

URL: http://llvm.org/viewvc/llvm-project?rev=316353&view=rev
Log:
CodeGen: Fix invalid bitcast in partial initialization of automatic arrary 
variable

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

Modified:
cfe/trunk/lib/CodeGen/CGDecl.cpp
cfe/trunk/test/CodeGenOpenCL/amdgcn-automatic-variable.cl

Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=316353&r1=316352&r2=316353&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDecl.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDecl.cpp Mon Oct 23 10:49:26 2017
@@ -1266,7 +1266,7 @@ void CodeGenFunction::EmitAutoVarInit(co
 llvm::ConstantInt::get(IntPtrTy,

getContext().getTypeSizeInChars(type).getQuantity());
 
-  llvm::Type *BP = Int8PtrTy;
+  llvm::Type *BP = AllocaInt8PtrTy;
   if (Loc.getType() != BP)
 Loc = Builder.CreateBitCast(Loc, BP);
 

Modified: cfe/trunk/test/CodeGenOpenCL/amdgcn-automatic-variable.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/amdgcn-automatic-variable.cl?rev=316353&r1=316352&r2=316353&view=diff
==
--- cfe/trunk/test/CodeGenOpenCL/amdgcn-automatic-variable.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/amdgcn-automatic-variable.cl Mon Oct 23 
10:49:26 2017
@@ -58,3 +58,11 @@ void func2(void) {
   const int lvc = 4;
   lv1 = lvc;
 }
+
+// CHECK-LABEL: define void @func3()
+// CHECK: %a = alloca [16 x [1 x float]], align 4, addrspace(5)
+// CHECK: %[[CAST:.+]] = bitcast [16 x [1 x float]] addrspace(5)* %a to i8 
addrspace(5)*
+// CHECK: call void @llvm.memset.p5i8.i64(i8 addrspace(5)* %[[CAST]], i8 0, 
i64 64, i32 4, i1 false)
+void func3(void) {
+  float a[16][1] = {{0.}};
+}


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

[PATCH] D39184: CodeGen: Fix invalid bitcast in partial initialization of automatic arrary variable

2017-10-23 Thread Yaxun Liu via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL316353: CodeGen: Fix invalid bitcast in partial 
initialization of automatic arrary… (authored by yaxunl).

Changed prior to commit:
  https://reviews.llvm.org/D39184?vs=119861&id=119900#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39184

Files:
  cfe/trunk/lib/CodeGen/CGDecl.cpp
  cfe/trunk/test/CodeGenOpenCL/amdgcn-automatic-variable.cl


Index: cfe/trunk/lib/CodeGen/CGDecl.cpp
===
--- cfe/trunk/lib/CodeGen/CGDecl.cpp
+++ cfe/trunk/lib/CodeGen/CGDecl.cpp
@@ -1266,7 +1266,7 @@
 llvm::ConstantInt::get(IntPtrTy,

getContext().getTypeSizeInChars(type).getQuantity());
 
-  llvm::Type *BP = Int8PtrTy;
+  llvm::Type *BP = AllocaInt8PtrTy;
   if (Loc.getType() != BP)
 Loc = Builder.CreateBitCast(Loc, BP);
 
Index: cfe/trunk/test/CodeGenOpenCL/amdgcn-automatic-variable.cl
===
--- cfe/trunk/test/CodeGenOpenCL/amdgcn-automatic-variable.cl
+++ cfe/trunk/test/CodeGenOpenCL/amdgcn-automatic-variable.cl
@@ -58,3 +58,11 @@
   const int lvc = 4;
   lv1 = lvc;
 }
+
+// CHECK-LABEL: define void @func3()
+// CHECK: %a = alloca [16 x [1 x float]], align 4, addrspace(5)
+// CHECK: %[[CAST:.+]] = bitcast [16 x [1 x float]] addrspace(5)* %a to i8 
addrspace(5)*
+// CHECK: call void @llvm.memset.p5i8.i64(i8 addrspace(5)* %[[CAST]], i8 0, 
i64 64, i32 4, i1 false)
+void func3(void) {
+  float a[16][1] = {{0.}};
+}


Index: cfe/trunk/lib/CodeGen/CGDecl.cpp
===
--- cfe/trunk/lib/CodeGen/CGDecl.cpp
+++ cfe/trunk/lib/CodeGen/CGDecl.cpp
@@ -1266,7 +1266,7 @@
 llvm::ConstantInt::get(IntPtrTy,
getContext().getTypeSizeInChars(type).getQuantity());
 
-  llvm::Type *BP = Int8PtrTy;
+  llvm::Type *BP = AllocaInt8PtrTy;
   if (Loc.getType() != BP)
 Loc = Builder.CreateBitCast(Loc, BP);
 
Index: cfe/trunk/test/CodeGenOpenCL/amdgcn-automatic-variable.cl
===
--- cfe/trunk/test/CodeGenOpenCL/amdgcn-automatic-variable.cl
+++ cfe/trunk/test/CodeGenOpenCL/amdgcn-automatic-variable.cl
@@ -58,3 +58,11 @@
   const int lvc = 4;
   lv1 = lvc;
 }
+
+// CHECK-LABEL: define void @func3()
+// CHECK: %a = alloca [16 x [1 x float]], align 4, addrspace(5)
+// CHECK: %[[CAST:.+]] = bitcast [16 x [1 x float]] addrspace(5)* %a to i8 addrspace(5)*
+// CHECK: call void @llvm.memset.p5i8.i64(i8 addrspace(5)* %[[CAST]], i8 0, i64 64, i32 4, i1 false)
+void func3(void) {
+  float a[16][1] = {{0.}};
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39179: [AArch64] Fix PR34625 -mtune without -mcpu should not set -target-cpu

2017-10-23 Thread Eric Christopher via Phabricator via cfe-commits

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

This should be fine for now. Thanks!

-eric


https://reviews.llvm.org/D39179



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

Re: [PATCH] D39069: CodeGen: Fix missing debug loc due to alloca

2017-10-23 Thread David Blaikie via cfe-commits

What John said, but also a narrower test would be good - checking that the
appropriate call instruction gets a debug location, rather than checking
that a bunch of inlining doesn't cause the assertion/verifier failure,
would be good. (Clang tests should, as much as possible, not rely on or run
the LLVM optimization passes - but check the IR coming directly from Clang
before any of that)

On Wed, Oct 18, 2017 at 2:15 PM Yaxun Liu via Phabricator via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> yaxunl created this revision.
>
> Builder save/restores insertion pointer when emitting addr space cast
> for alloca, but does not save/restore debug loc, which causes verifier
> failure for certain call instructions.
>
> This patch fixes that.
>
>
> https://reviews.llvm.org/D39069
>
> Files:
>   lib/CodeGen/CGExpr.cpp
>   test/CodeGenOpenCL/func-call-dbg-loc.cl
>
>
> Index: test/CodeGenOpenCL/func-call-dbg-loc.cl
> ===
> --- /dev/null
> +++ test/CodeGenOpenCL/func-call-dbg-loc.cl
> @@ -0,0 +1,34 @@
> +// RUN: %clang_cc1 -triple amdgcn---amdgizcl -debug-info-kind=limited
> -dwarf-version=2 -debugger-tuning=gdb -O0 -emit-llvm -o - %s | FileCheck %s
> +// Checks the file compiles without verifier error: inlinable function
> call in a function with debug info must have a !dbg location.
> +
> +typedef struct
> +{
> +float m_max;
> +} Struct;
> +
> +typedef struct
> +{
> +Struct m_volume;
> +} Node;
> +
> +
> +Struct buzz(Node node)
> +{
> +return node.m_volume;
> +}
> +
> +__attribute__((always_inline))
> +float bar(Struct aabb)
> +{
> +return 0.0f;
> +}
> +
> +__attribute__((used))
> +void foo()
> +{
> +Node node;
> +// CHECK: store float 0.00e+00, float addrspace(5)* %f, align 4,
> !dbg !{{[0-9]+}}
> +float f = bar(buzz(node));
> +}
> +
> +
> Index: lib/CodeGen/CGExpr.cpp
> ===
> --- lib/CodeGen/CGExpr.cpp
> +++ lib/CodeGen/CGExpr.cpp
> @@ -75,11 +75,13 @@
>if (CastToDefaultAddrSpace && getASTAllocaAddressSpace() !=
> LangAS::Default) {
>  auto DestAddrSpace =
> getContext().getTargetAddressSpace(LangAS::Default);
>  auto CurIP = Builder.saveIP();
> +auto DbgLoc = Builder.getCurrentDebugLocation();
>  Builder.SetInsertPoint(AllocaInsertPt);
>  V = getTargetHooks().performAddrSpaceCast(
>  *this, V, getASTAllocaAddressSpace(), LangAS::Default,
>  Ty->getPointerTo(DestAddrSpace), /*non-null*/ true);
>  Builder.restoreIP(CurIP);
> +Builder.SetCurrentDebugLocation(DbgLoc);
>}
>
>return Address(V, Align);
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

RE: [PATCH] D39069: CodeGen: Fix missing debug loc due to alloca

2017-10-23 Thread Liu, Yaxun (Sam) via cfe-commits

I will simplify the test. Thanks.

Sam

From: David Blaikie [mailto:dblai...@gmail.com]
Sent: Monday, October 23, 2017 2:08 PM
To: reviews+d39069+public+8da79e110d303...@reviews.llvm.org; Yaxun Liu via 
Phabricator ; Liu, Yaxun (Sam) ; 
rjmcc...@gmail.com
Cc: cfe-commits@lists.llvm.org
Subject: Re: [PATCH] D39069: CodeGen: Fix missing debug loc due to alloca

What John said, but also a narrower test would be good - checking that the 
appropriate call instruction gets a debug location, rather than checking that a 
bunch of inlining doesn't cause the assertion/verifier failure, would be good. 
(Clang tests should, as much as possible, not rely on or run the LLVM 
optimization passes - but check the IR coming directly from Clang before any of 
that)
On Wed, Oct 18, 2017 at 2:15 PM Yaxun Liu via Phabricator via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
yaxunl created this revision.

Builder save/restores insertion pointer when emitting addr space cast
for alloca, but does not save/restore debug loc, which causes verifier
failure for certain call instructions.

This patch fixes that.


https://reviews.llvm.org/D39069

Files:
  lib/CodeGen/CGExpr.cpp
  test/CodeGenOpenCL/func-call-dbg-loc.cl


Index: test/CodeGenOpenCL/func-call-dbg-loc.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/func-call-dbg-loc.cl
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -triple amdgcn---amdgizcl -debug-info-kind=limited 
-dwarf-version=2 -debugger-tuning=gdb -O0 -emit-llvm -o - %s | FileCheck %s
+// Checks the file compiles without verifier error: inlinable function call in 
a function with debug info must have a !dbg location.
+
+typedef struct
+{
+float m_max;
+} Struct;
+
+typedef struct
+{
+Struct m_volume;
+} Node;
+
+
+Struct buzz(Node node)
+{
+return node.m_volume;
+}
+
+__attribute__((always_inline))
+float bar(Struct aabb)
+{
+return 0.0f;
+}
+
+__attribute__((used))
+void foo()
+{
+Node node;
+// CHECK: store float 0.00e+00, float addrspace(5)* %f, align 4, !dbg 
!{{[0-9]+}}
+float f = bar(buzz(node));
+}
+
+
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -75,11 +75,13 @@
   if (CastToDefaultAddrSpace && getASTAllocaAddressSpace() != LangAS::Default) 
{
 auto DestAddrSpace = getContext().getTargetAddressSpace(LangAS::Default);
 auto CurIP = Builder.saveIP();
+auto DbgLoc = Builder.getCurrentDebugLocation();
 Builder.SetInsertPoint(AllocaInsertPt);
 V = getTargetHooks().performAddrSpaceCast(
 *this, V, getASTAllocaAddressSpace(), LangAS::Default,
 Ty->getPointerTo(DestAddrSpace), /*non-null*/ true);
 Builder.restoreIP(CurIP);
+Builder.SetCurrentDebugLocation(DbgLoc);
   }

   return Address(V, Align);


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

[PATCH] D39069: CodeGen: Fix missing debug loc due to alloca

2017-10-23 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl added a comment.

In https://reviews.llvm.org/D39069#903344, @rjmccall wrote:

> If this is something we generally need to be doing in all the places we 
> temporarily save and restore the insertion point, we should fix the basic 
> behavior of saveIP instead of adding explicit code to a bunch of separate 
> places.  Can we just override saveIP() on CGBuilder to return a struct that 
> also includes the current debug location?


IRBuilderBase::InsertPointGuard does that. Will use it instead.


https://reviews.llvm.org/D39069



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

[PATCH] D27607: [ubsan] Treat ObjC's BOOL as if its range is always {0, 1}

2017-10-23 Thread Vedant Kumar via Phabricator via cfe-commits

vsk added a comment.

In https://reviews.llvm.org/D27607#901882, @fjricci wrote:

> On platforms where `BOOL` == `signed char`, is it actually undefined behavior 
> (or is it just bad programming practice) to store a value other than 0 or 1 
> in your `BOOL`? I can't find any language specs suggesting that it is, and 
> given that it's just a typedef for a `signed char`, I don't see why it would 
> be.

Treating BOOL as a regular 'signed char' creates bad interactions with 
bitfields. For example, this code calls panic:

  struct S {
BOOL b1 : 1;
  };

  S s;
  s.b1 = YES;
  if (s.b1 != YES)
panic();

> If it's not actually undefined behavior, could we make it controllable via a 
> separate fsanitize switch (like we have for unsigned integer overflow, which 
> is also potentially bad practice but not actually undefined behavior).

The only defined values for BOOL are 'YES' and 'NO' {1, 0}. We've documented 
that it's UB to load values outside of this range from a BOOL here:
https://developer.apple.com/documentation/code_diagnostics/undefined_behavior_sanitizer/invalid_boolean

Repository:
  rL LLVM

https://reviews.llvm.org/D27607

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

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

2017-10-23 Thread Sriraman Tallam via Phabricator via cfe-commits

tmsriram added a comment.

Ping.


https://reviews.llvm.org/D39079



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

r316362 - [OpenMP] Avoid VLAs for some reductions on array sections

2017-10-23 Thread Jonas Hahnfeld via cfe-commits

Author: hahnfeld
Date: Mon Oct 23 12:01:35 2017
New Revision: 316362

URL: http://llvm.org/viewvc/llvm-project?rev=316362&view=rev
Log:
[OpenMP] Avoid VLAs for some reductions on array sections

In some cases the compiler can deduce the length of an array section
as constants. With this information, VLAs can be avoided in place of
a constant sized array or even a scalar value if the length is 1.
Example:
int a[4], b[2];
pragma omp parallel reduction(+: a[1:2], b[1:1])
{ }

For chained array sections, this optimization is restricted to cases
where all array sections except the last have a constant length 1.
This trivially guarantees that there are no holes in the memory region
that needs to be privatized.
Example:
int c[3][4];
pragma omp parallel reduction(+: c[1:1][1:2])
{ }

This relands commit r316229 that I reverted in r316235 because it
failed on some bots. During investigation I found that this was because
Clang and GCC evaluate the two arguments to emplace_back() in
ReductionCodeGen::emitSharedLValue() in a different order, hence
leading to a different order of generated instructions in the final
LLVM IR. Fix this by passing in the arguments from temporary variables
that are evaluated in a defined order.

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

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/for_reduction_codegen.cpp
cfe/trunk/test/OpenMP/for_reduction_codegen_UDR.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=316362&r1=316361&r2=316362&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Mon Oct 23 12:01:35 2017
@@ -916,8 +916,9 @@ ReductionCodeGen::ReductionCodeGen(Array
 void ReductionCodeGen::emitSharedLValue(CodeGenFunction &CGF, unsigned N) {
   assert(SharedAddresses.size() == N &&
  "Number of generated lvalues must be exactly N.");
-  SharedAddresses.emplace_back(emitSharedLValue(CGF, ClausesData[N].Ref),
-   emitSharedLValueUB(CGF, ClausesData[N].Ref));
+  LValue First = emitSharedLValue(CGF, ClausesData[N].Ref);
+  LValue Second = emitSharedLValueUB(CGF, ClausesData[N].Ref);
+  SharedAddresses.emplace_back(First, Second);
 }
 
 void ReductionCodeGen::emitAggregateType(CodeGenFunction &CGF, unsigned N) {
@@ -925,7 +926,7 @@ void ReductionCodeGen::emitAggregateType
   cast(cast(ClausesData[N].Private)->getDecl());
   QualType PrivateType = PrivateVD->getType();
   bool AsArraySection = isa(ClausesData[N].Ref);
-  if (!AsArraySection && !PrivateType->isVariablyModifiedType()) {
+  if (!PrivateType->isVariablyModifiedType()) {
 Sizes.emplace_back(
 CGF.getTypeSize(
 SharedAddresses[N].first.getType().getNonReferenceType()),
@@ -963,10 +964,9 @@ void ReductionCodeGen::emitAggregateType
   auto *PrivateVD =
   cast(cast(ClausesData[N].Private)->getDecl());
   QualType PrivateType = PrivateVD->getType();
-  bool AsArraySection = isa(ClausesData[N].Ref);
-  if (!AsArraySection && !PrivateType->isVariablyModifiedType()) {
+  if (!PrivateType->isVariablyModifiedType()) {
 assert(!Size && !Sizes[N].second &&
-   "Size should be nullptr for non-variably modified redution "
+   "Size should be nullptr for non-variably modified reduction "
"items.");
 return;
   }
@@ -994,8 +994,7 @@ void ReductionCodeGen::emitInitializatio
CGF.ConvertTypeForMem(SharedType)),
   SharedType, SharedAddresses[N].first.getBaseInfo(),
   CGF.CGM.getTBAAAccessInfo(SharedType));
-  if (isa(ClausesData[N].Ref) ||
-  CGF.getContext().getAsArrayType(PrivateVD->getType())) {
+  if (CGF.getContext().getAsArrayType(PrivateVD->getType())) {
 emitAggregateInitialization(CGF, N, PrivateAddr, SharedLVal, DRD);
   } else if (DRD && (DRD->getInitializer() || !PrivateVD->hasInit())) {
 emitInitWithReductionInitializer(CGF, DRD, ClausesData[N].ReductionOp,

Modified: cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp?rev=316362&r1=316361&r2=316362&view=diff
==
--- cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp Mon Oct 23 12:01:35 2017
@@ -996,7 +996,9 @@ void CodeGenFunction::EmitOMPReductionCl
 
 auto *LHSVD = cast(cast(*ILHS)->getDecl());
 auto *RHSVD = cast(cast(*IRHS)->getDecl());
-if (isa(IRef)) {
+QualType Type = PrivateVD->getType();
+bool isaOMPArraySectionExpr = isa(IRef);
+if (isaOMPArraySectionExpr && Type->isVariablyModifiedType()) {
   // Store the address of the original variable associ

[PATCH] D39136: [OpenMP] Avoid VLAs for some reductions on array sections

2017-10-23 Thread Jonas Hahnfeld via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL316362: [OpenMP] Avoid VLAs for some reductions on array 
sections (authored by Hahnfeld).

Changed prior to commit:
  https://reviews.llvm.org/D39136?vs=119689&id=119909#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39136

Files:
  cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
  cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
  cfe/trunk/lib/Sema/SemaOpenMP.cpp
  cfe/trunk/test/OpenMP/for_reduction_codegen.cpp
  cfe/trunk/test/OpenMP/for_reduction_codegen_UDR.cpp

Index: cfe/trunk/test/OpenMP/for_reduction_codegen.cpp
===
--- cfe/trunk/test/OpenMP/for_reduction_codegen.cpp
+++ cfe/trunk/test/OpenMP/for_reduction_codegen.cpp
@@ -27,15 +27,16 @@
 // CHECK-DAG: [[REDUCTION_LOC:@.+]] = private unnamed_addr constant %{{.+}} { i32 0, i32 18, i32 0, i32 0, i8*
 // CHECK-DAG: [[REDUCTION_LOCK:@.+]] = common global [8 x i32] zeroinitializer
 
-template 
+template 
 T tmain() {
   T t;
   S test;
   T t_var = T(), t_var1;
   T vec[] = {1, 2};
   S s_arr[] = {1, 2};
   S &var = test;
   S var1;
+  S arr[length];
 #pragma omp parallel
 #pragma omp for reduction(+:t_var) reduction(&:var) reduction(&& : var1) reduction(min: t_var1) nowait
   for (int i = 0; i < 2; ++i) {
@@ -48,6 +49,12 @@
 vec[i] = t_var;
 s_arr[i] = var;
   }
+#pragma omp parallel
+#pragma omp for reduction(+ : arr[1:length-2])
+  for (int i = 0; i < 2; ++i) {
+vec[i] = t_var;
+s_arr[i] = var;
+  }
   return T();
 }
 
@@ -180,12 +187,12 @@
   S test;
   float t_var = 0, t_var1;
   int vec[] = {1, 2};
-  S s_arr[] = {1, 2};
+  S s_arr[] = {1, 2, 3, 4};
   S &var = test;
   S var1, arrs[10][4];
   S **var2 = foo();
-  S vvar2[2];
-  S (&var3)[2] = s_arr;
+  S vvar2[5];
+  S (&var3)[4] = s_arr;
 #pragma omp parallel
 #pragma omp for reduction(+:t_var) reduction(&:var) reduction(&& : var1) reduction(min: t_var1)
   for (int i = 0; i < 2; ++i) {
@@ -205,36 +212,62 @@
   for (int i = 0; i < 10; ++i)
 ;
 #pragma omp parallel
+#pragma omp for reduction(& : var2[1][1 : 6])
+  for (int i = 0; i < 10; ++i)
+;
+#pragma omp parallel
+#pragma omp for reduction(& : var2[1 : 1][1 : 6])
+  for (int i = 0; i < 10; ++i)
+;
+#pragma omp parallel
+#pragma omp for reduction(& : var2[1 : 1][1])
+  for (int i = 0; i < 10; ++i)
+;
+#pragma omp parallel
 #pragma omp for reduction(& : vvar2[0 : 5])
   for (int i = 0; i < 10; ++i)
 ;
 #pragma omp parallel
 #pragma omp for reduction(& : var3[1 : 2])
   for (int i = 0; i < 10; ++i)
 ;
 #pragma omp parallel
+#pragma omp for reduction(& : var3[ : 2])
+  for (int i = 0; i < 10; ++i)
+;
+  // TODO: The compiler should also be able to generate a constant sized array in this case!
+#pragma omp parallel
+#pragma omp for reduction(& : var3[2 : ])
+  for (int i = 0; i < 10; ++i)
+;
+#pragma omp parallel
 #pragma omp for reduction(& : var3)
   for (int i = 0; i < 10; ++i)
 ;
-  return tmain();
+  return tmain();
 #endif
 }
 
 // CHECK: define {{.*}}i{{[0-9]+}} @main()
 // CHECK: [[TEST:%.+]] = alloca [[S_FLOAT_TY]],
 // CHECK: call {{.*}} [[S_FLOAT_TY_CONSTR:@.+]]([[S_FLOAT_TY]]* [[TEST]])
-// CHECK: call void (%{{.+}}*, i{{[0-9]+}}, void (i{{[0-9]+}}*, i{{[0-9]+}}*, ...)*, ...) @__kmpc_fork_call(%{{.+}}* @{{.+}}, i{{[0-9]+}} 6, void (i{{[0-9]+}}*, i{{[0-9]+}}*, ...)* bitcast (void (i{{[0-9]+}}*, i{{[0-9]+}}*, float*, [[S_FLOAT_TY]]*, [[S_FLOAT_TY]]*, float*, [2 x i32]*, [2 x [[S_FLOAT_TY]]]*)* [[MAIN_MICROTASK:@.+]] to void
+// CHECK: call void (%{{.+}}*, i{{[0-9]+}}, void (i{{[0-9]+}}*, i{{[0-9]+}}*, ...)*, ...) @__kmpc_fork_call(%{{.+}}* @{{.+}}, i{{[0-9]+}} 6, void (i{{[0-9]+}}*, i{{[0-9]+}}*, ...)* bitcast (void (i{{[0-9]+}}*, i{{[0-9]+}}*, float*, [[S_FLOAT_TY]]*, [[S_FLOAT_TY]]*, float*, [2 x i32]*, [4 x [[S_FLOAT_TY]]]*)* [[MAIN_MICROTASK:@.+]] to void
 // CHECK: call void (%{{.+}}*, i{{[0-9]+}}, void (i{{[0-9]+}}*, i{{[0-9]+}}*, ...)*, ...) @__kmpc_fork_call(%{{.+}}* @{{.+}}, i{{[0-9]+}} 5, void (i{{[0-9]+}}*, i{{[0-9]+}}*, ...)* bitcast (void (i{{[0-9]+}}*, i{{[0-9]+}}*, i64, i64, i32*, [2 x i32]*, [10 x [4 x [[S_FLOAT_TY*)* [[MAIN_MICROTASK1:@.+]] to void
 // CHECK: call void (%{{.+}}*, i{{[0-9]+}}, void (i{{[0-9]+}}*, i{{[0-9]+}}*, ...)*, ...) @__kmpc_fork_call(%{{.+}}* @{{.+}}, i{{[0-9]+}} 4, void (i{{[0-9]+}}*, i{{[0-9]+}}*, ...)* bitcast (void (i{{[0-9]+}}*, i{{[0-9]+}}*, i64, i64, i32*, [10 x [4 x [[S_FLOAT_TY*)* [[MAIN_MICROTASK2:@.+]] to void
 // CHECK: call void (%{{.+}}*, i{{[0-9]+}}, void (i{{[0-9]+}}*, i{{[0-9]+}}*, ...)*, ...) @__kmpc_fork_call(%{{.+}}* @{{.+}}, i{{[0-9]+}} 1, void (i{{[0-9]+}}*, i{{[0-9]+}}*, ...)* bitcast (void (i{{[0-9]+}}*, i{{[0-9]+}}*, [[S_FLOAT_TY]]***)* [[MAIN_MICROTASK3:@.+]] to void
-// CHECK: call void (%{{.+}}*, i{{[0-9]+}}, void (i{{[0-9]+}}*, i{{[0-9]+}}*, ...)*, ...) @__kmpc_fork_call(%{{.+}}* @{{.+}}, i{{[0-9]+}} 1, void (i{{[0-9]+}}*, i{{[0-9]+}}*, ...)* bitcast (void (i{{[0-

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

2017-10-23 Thread Roman Lebedev via Phabricator via cfe-commits

lebedev.ri added a comment.

No tests?


https://reviews.llvm.org/D39079



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

[PATCH] D27607: [ubsan] Treat ObjC's BOOL as if its range is always {0, 1}

2017-10-23 Thread Francis Ricci via Phabricator via cfe-commits

fjricci added a comment.

Awesome, good to know. Thanks!


Repository:
  rL LLVM

https://reviews.llvm.org/D27607



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

[PATCH] D39201: [Analyzer] Handle implicit function reference in bodyfarming std::call_once

2017-10-23 Thread George Karpenkov via Phabricator via cfe-commits

george.karpenkov created this revision.
Herald added subscribers: szepet, kristof.beyls, xazax.hun, javed.absar, 
aemerson.

https://reviews.llvm.org/D39201

Files:
  lib/Analysis/BodyFarm.cpp
  test/Analysis/call_once.cpp


Index: test/Analysis/call_once.cpp
===
--- test/Analysis/call_once.cpp
+++ test/Analysis/call_once.cpp
@@ -290,3 +290,16 @@
   std::call_once(flag, &fail_mutator, a);
   clang_analyzer_eval(a == 42); // expected-warning{{FALSE}}
 }
+
+// Function is implicitly treated as a function pointer
+// even when an ampersand is not explicitly set.
+void callbackn(int ¶m) {
+  param = 42;
+};
+void test_implicit_funcptr() {
+  int x = 0;
+  static std::once_flag flagn;
+
+  std::call_once(flagn, callbackn, x);
+  clang_analyzer_eval(x == 42); // expected-warning{{TRUE}}
+}
Index: lib/Analysis/BodyFarm.cpp
===
--- lib/Analysis/BodyFarm.cpp
+++ lib/Analysis/BodyFarm.cpp
@@ -253,13 +253,23 @@
const ParmVarDecl *Callback,
ArrayRef CallArgs) {
 
-  return new (C) CallExpr(
-  /*ASTContext=*/C,
-  /*StmtClass=*/M.makeLvalueToRvalue(/*Expr=*/Callback),
-  /*args=*/CallArgs,
-  /*QualType=*/C.VoidTy,
-  /*ExprValueType=*/VK_RValue,
-  /*SourceLocation=*/SourceLocation());
+  QualType Ty = Callback->getType();
+  DeclRefExpr *Call = M.makeDeclRefExpr(Callback);
+  CastKind CK;
+  if (Ty->isRValueReferenceType()) {
+CK = CK_LValueToRValue;
+  } else {
+assert(Ty->isLValueReferenceType());
+CK = CK_FunctionToPointerDecay;
+Ty = C.getPointerType(Ty.getNonReferenceType());
+  }
+
+  return new (C)
+  CallExpr(C, M.makeImplicitCast(Call, Ty.getNonReferenceType(), CK),
+   /*args=*/CallArgs,
+   /*QualType=*/C.VoidTy,
+   /*ExprValueType=*/VK_RValue,
+   /*SourceLocation=*/SourceLocation());
 }
 
 static CallExpr *create_call_once_lambda_call(ASTContext &C, ASTMaker M,
@@ -366,9 +376,11 @@
 CallbackFunctionType = CallbackRecordDecl->getLambdaCallOperator()
->getType()
->getAs();
-  } else {
+  } else if (!CallbackType->getPointeeType().isNull()) {
 CallbackFunctionType =
 CallbackType->getPointeeType()->getAs();
+  } else {
+CallbackFunctionType = CallbackType->getAs();
   }
 
   if (!CallbackFunctionType)


Index: test/Analysis/call_once.cpp
===
--- test/Analysis/call_once.cpp
+++ test/Analysis/call_once.cpp
@@ -290,3 +290,16 @@
   std::call_once(flag, &fail_mutator, a);
   clang_analyzer_eval(a == 42); // expected-warning{{FALSE}}
 }
+
+// Function is implicitly treated as a function pointer
+// even when an ampersand is not explicitly set.
+void callbackn(int ¶m) {
+  param = 42;
+};
+void test_implicit_funcptr() {
+  int x = 0;
+  static std::once_flag flagn;
+
+  std::call_once(flagn, callbackn, x);
+  clang_analyzer_eval(x == 42); // expected-warning{{TRUE}}
+}
Index: lib/Analysis/BodyFarm.cpp
===
--- lib/Analysis/BodyFarm.cpp
+++ lib/Analysis/BodyFarm.cpp
@@ -253,13 +253,23 @@
const ParmVarDecl *Callback,
ArrayRef CallArgs) {
 
-  return new (C) CallExpr(
-  /*ASTContext=*/C,
-  /*StmtClass=*/M.makeLvalueToRvalue(/*Expr=*/Callback),
-  /*args=*/CallArgs,
-  /*QualType=*/C.VoidTy,
-  /*ExprValueType=*/VK_RValue,
-  /*SourceLocation=*/SourceLocation());
+  QualType Ty = Callback->getType();
+  DeclRefExpr *Call = M.makeDeclRefExpr(Callback);
+  CastKind CK;
+  if (Ty->isRValueReferenceType()) {
+CK = CK_LValueToRValue;
+  } else {
+assert(Ty->isLValueReferenceType());
+CK = CK_FunctionToPointerDecay;
+Ty = C.getPointerType(Ty.getNonReferenceType());
+  }
+
+  return new (C)
+  CallExpr(C, M.makeImplicitCast(Call, Ty.getNonReferenceType(), CK),
+   /*args=*/CallArgs,
+   /*QualType=*/C.VoidTy,
+   /*ExprValueType=*/VK_RValue,
+   /*SourceLocation=*/SourceLocation());
 }
 
 static CallExpr *create_call_once_lambda_call(ASTContext &C, ASTMaker M,
@@ -366,9 +376,11 @@
 CallbackFunctionType = CallbackRecordDecl->getLambdaCallOperator()
->getType()
->getAs();
-  } else {
+  } else if (!CallbackType->getPointeeType().isNull()) {
 CallbackFunctionType =
 CallbackType->getPointeeType()->getAs();
+  } else {
+CallbackFunctionType = CallbackType->getAs();
   }
 
   if (!CallbackFunctionType)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/list

[libunwind] r316364 - Abstract rwlocks into a class, provide a SRW lock implementation for windows

2017-10-23 Thread Martin Storsjo via cfe-commits

Author: mstorsjo
Date: Mon Oct 23 12:29:36 2017
New Revision: 316364

URL: http://llvm.org/viewvc/llvm-project?rev=316364&view=rev
Log:
Abstract rwlocks into a class, provide a SRW lock implementation for windows

This requires _WIN32_WINNT >= 0x0600.

If someone wants to spend effort on supporting earlier versions,
one can easily add another fallback implementation based on
critical sections, or try to load SRW lock functions dynamically.

This makes sure that the FDE cache is thread safe on windows.

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

Added:
libunwind/trunk/src/RWMutex.hpp
Modified:
libunwind/trunk/src/CMakeLists.txt
libunwind/trunk/src/UnwindCursor.hpp
libunwind/trunk/src/config.h

Modified: libunwind/trunk/src/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/CMakeLists.txt?rev=316364&r1=316363&r2=316364&view=diff
==
--- libunwind/trunk/src/CMakeLists.txt (original)
+++ libunwind/trunk/src/CMakeLists.txt Mon Oct 23 12:29:36 2017
@@ -30,6 +30,7 @@ set(LIBUNWIND_HEADERS
 DwarfParser.hpp
 libunwind_ext.h
 Registers.hpp
+RWMutex.hpp
 UnwindCursor.hpp
 ${CMAKE_CURRENT_SOURCE_DIR}/../include/libunwind.h
 ${CMAKE_CURRENT_SOURCE_DIR}/../include/unwind.h)

Added: libunwind/trunk/src/RWMutex.hpp
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/RWMutex.hpp?rev=316364&view=auto
==
--- libunwind/trunk/src/RWMutex.hpp (added)
+++ libunwind/trunk/src/RWMutex.hpp Mon Oct 23 12:29:36 2017
@@ -0,0 +1,77 @@
+//===- Registers.hpp 
--===//
+//
+// 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.
+//
+//
+// Abstract interface to shared reader/writer log, hiding platform and
+// configuration differences.
+//
+//===--===//
+
+#ifndef __RWMUTEX_HPP__
+#define __RWMUTEX_HPP__
+
+#if defined(_WIN32)
+#include 
+#elif !defined(_LIBUNWIND_HAS_NO_THREADS)
+#include 
+#endif
+
+namespace libunwind {
+
+#if defined(_LIBUNWIND_HAS_NO_THREADS)
+
+class _LIBUNWIND_HIDDEN RWMutex {
+public:
+  bool lock_shared() { return true; }
+  bool unlock_shared() { return true; }
+  bool lock() { return true; }
+  bool unlock() { return true; }
+};
+
+#elif defined(_WIN32)
+
+class _LIBUNWIND_HIDDEN RWMutex {
+public:
+  bool lock_shared() {
+AcquireSRWLockShared(&_lock);
+return true;
+  }
+  bool unlock_shared() {
+ReleaseSRWLockShared(&_lock);
+return true;
+  }
+  bool lock() {
+AcquireSRWLockExclusive(&_lock);
+return true;
+  }
+  bool unlock() {
+ReleaseSRWLockExclusive(&_lock);
+return true;
+  }
+
+private:
+  SRWLOCK _lock = SRWLOCK_INIT;
+};
+
+#else
+
+class _LIBUNWIND_HIDDEN RWMutex {
+public:
+  bool lock_shared() { return pthread_rwlock_rdlock(&_lock) == 0; }
+  bool unlock_shared() { return pthread_rwlock_unlock(&_lock) == 0; }
+  bool lock() { return pthread_rwlock_wrlock(&_lock) == 0; }
+  bool unlock() { return pthread_rwlock_unlock(&_lock) == 0; }
+
+private:
+  pthread_rwlock_t _lock = PTHREAD_RWLOCK_INITIALIZER;
+};
+
+#endif
+
+} // namespace libunwind
+
+#endif // __RWMUTEX_HPP__

Modified: libunwind/trunk/src/UnwindCursor.hpp
URL: 
http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/UnwindCursor.hpp?rev=316364&r1=316363&r2=316364&view=diff
==
--- libunwind/trunk/src/UnwindCursor.hpp (original)
+++ libunwind/trunk/src/UnwindCursor.hpp Mon Oct 23 12:29:36 2017
@@ -16,9 +16,6 @@
 #include 
 #include 
 #include 
-#ifndef _LIBUNWIND_HAS_NO_THREADS
-  #include 
-#endif
 #include 
 
 #ifdef __APPLE__
@@ -34,6 +31,7 @@
 #include "EHHeaderParser.hpp"
 #include "libunwind.h"
 #include "Registers.hpp"
+#include "RWMutex.hpp"
 #include "Unwind-EHABI.h"
 
 namespace libunwind {
@@ -62,9 +60,7 @@ private:
 
   // These fields are all static to avoid needing an initializer.
   // There is only one instance of this class per process.
-#ifndef _LIBUNWIND_HAS_NO_THREADS
-  static pthread_rwlock_t _lock;
-#endif
+  static RWMutex _lock;
 #ifdef __APPLE__
   static void dyldUnloadHook(const struct mach_header *mh, intptr_t slide);
   static bool _registeredForDyldUnloads;
@@ -91,10 +87,8 @@ DwarfFDECache::_bufferEnd = &_initial
 template 
 typename DwarfFDECache::entry DwarfFDECache::_initialBuffer[64];
 
-#ifndef _LIBUNWIND_HAS_NO_THREADS
 template 
-pthread_rwlock_t DwarfFDECache::_lock = PTHREAD_RWLOCK_INITIALIZER;
-#endif
+RWMutex DwarfFDECache::_lock;
 
 #ifdef __APPLE__
 template 
@@ -104,7 +98,7 @@ bool DwarfFDECache::_registeredForDyl
 template 
 typename A::pint_t DwarfFDECache::findFDE(pint_t mh, pint_t pc) {
   pin

[PATCH] D38704: [libunwind] Abstract rwlocks into a class, provide a SRW lock implementation for windows

2017-10-23 Thread Martin Storsjö via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL316364: Abstract rwlocks into a class, provide a SRW lock 
implementation for windows (authored by mstorsjo).

Changed prior to commit:
  https://reviews.llvm.org/D38704?vs=119094&id=119914#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38704

Files:
  libunwind/trunk/src/CMakeLists.txt
  libunwind/trunk/src/RWMutex.hpp
  libunwind/trunk/src/UnwindCursor.hpp
  libunwind/trunk/src/config.h

Index: libunwind/trunk/src/CMakeLists.txt
===
--- libunwind/trunk/src/CMakeLists.txt
+++ libunwind/trunk/src/CMakeLists.txt
@@ -30,6 +30,7 @@
 DwarfParser.hpp
 libunwind_ext.h
 Registers.hpp
+RWMutex.hpp
 UnwindCursor.hpp
 ${CMAKE_CURRENT_SOURCE_DIR}/../include/libunwind.h
 ${CMAKE_CURRENT_SOURCE_DIR}/../include/unwind.h)
Index: libunwind/trunk/src/RWMutex.hpp
===
--- libunwind/trunk/src/RWMutex.hpp
+++ libunwind/trunk/src/RWMutex.hpp
@@ -0,0 +1,77 @@
+//===- Registers.hpp --===//
+//
+// 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.
+//
+//
+// Abstract interface to shared reader/writer log, hiding platform and
+// configuration differences.
+//
+//===--===//
+
+#ifndef __RWMUTEX_HPP__
+#define __RWMUTEX_HPP__
+
+#if defined(_WIN32)
+#include 
+#elif !defined(_LIBUNWIND_HAS_NO_THREADS)
+#include 
+#endif
+
+namespace libunwind {
+
+#if defined(_LIBUNWIND_HAS_NO_THREADS)
+
+class _LIBUNWIND_HIDDEN RWMutex {
+public:
+  bool lock_shared() { return true; }
+  bool unlock_shared() { return true; }
+  bool lock() { return true; }
+  bool unlock() { return true; }
+};
+
+#elif defined(_WIN32)
+
+class _LIBUNWIND_HIDDEN RWMutex {
+public:
+  bool lock_shared() {
+AcquireSRWLockShared(&_lock);
+return true;
+  }
+  bool unlock_shared() {
+ReleaseSRWLockShared(&_lock);
+return true;
+  }
+  bool lock() {
+AcquireSRWLockExclusive(&_lock);
+return true;
+  }
+  bool unlock() {
+ReleaseSRWLockExclusive(&_lock);
+return true;
+  }
+
+private:
+  SRWLOCK _lock = SRWLOCK_INIT;
+};
+
+#else
+
+class _LIBUNWIND_HIDDEN RWMutex {
+public:
+  bool lock_shared() { return pthread_rwlock_rdlock(&_lock) == 0; }
+  bool unlock_shared() { return pthread_rwlock_unlock(&_lock) == 0; }
+  bool lock() { return pthread_rwlock_wrlock(&_lock) == 0; }
+  bool unlock() { return pthread_rwlock_unlock(&_lock) == 0; }
+
+private:
+  pthread_rwlock_t _lock = PTHREAD_RWLOCK_INITIALIZER;
+};
+
+#endif
+
+} // namespace libunwind
+
+#endif // __RWMUTEX_HPP__
Index: libunwind/trunk/src/config.h
===
--- libunwind/trunk/src/config.h
+++ libunwind/trunk/src/config.h
@@ -93,20 +93,15 @@
   fprintf(stderr, "libunwind: " msg "\n", __VA_ARGS__)
 #endif
 
-#if defined(_LIBUNWIND_HAS_NO_THREADS)
-  // only used with pthread calls, not needed for the single-threaded builds
-  #define _LIBUNWIND_LOG_NON_ZERO(x)
+#if defined(NDEBUG)
+  #define _LIBUNWIND_LOG_IF_FALSE(x) x
 #else
-  #if defined(NDEBUG)
-#define _LIBUNWIND_LOG_NON_ZERO(x) x
-  #else
-#define _LIBUNWIND_LOG_NON_ZERO(x) \
-  do { \
-int _err = x;  \
-if (_err != 0) \
-  _LIBUNWIND_LOG("" #x "=%d in %s", _err, __FUNCTION__);   \
-  } while (0)
-  #endif
+  #define _LIBUNWIND_LOG_IF_FALSE(x)   \
+do {   \
+  bool _ret = x;   \
+  if (!_ret)   \
+_LIBUNWIND_LOG("" #x " failed in %s", __FUNCTION__);   \
+} while (0)
 #endif
 
 // Macros that define away in non-Debug builds
Index: libunwind/trunk/src/UnwindCursor.hpp
===
--- libunwind/trunk/src/UnwindCursor.hpp
+++ libunwind/trunk/src/UnwindCursor.hpp
@@ -16,9 +16,6 @@
 #include 
 #include 
 #include 
-#ifndef _LIBUNWIND_HAS_NO_THREADS
-  #include 
-#endif
 #include 
 
 #ifdef __APPLE__
@@ -34,6 +31,7 @@
 #include "EHHeaderParser.hpp"
 #include "libunwind.h"
 #include "Registers.hpp"
+#include "RWMutex.hpp"
 #include "Unwind-EHABI.h"
 
 namespace libunwind {
@@ -62,9 +60,7 @@
 
   // These fields are all static to avoid needing an initializer.
   // There is only one instance

[PATCH] D39069: CodeGen: Fix missing debug loc due to alloca

2017-10-23 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl updated this revision to Diff 119918.
yaxunl added a comment.

Use InsertPointGuard and simplify test.


https://reviews.llvm.org/D39069

Files:
  lib/CodeGen/CGExpr.cpp
  test/CodeGenOpenCL/func-call-dbg-loc.cl


Index: test/CodeGenOpenCL/func-call-dbg-loc.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/func-call-dbg-loc.cl
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple amdgcn---amdgizcl -debug-info-kind=limited -O0 
-emit-llvm -o - %s | FileCheck %s
+
+typedef struct
+{
+int a;
+} Struct;
+
+Struct func1();
+
+void func2(Struct S);
+
+void func3()
+{
+// CHECK: call i32 @func1() #{{[0-9]+}}, !dbg !{{[0-9]+}}
+// CHECK: call void @func2(i32 %{{[0-9]+}}) #{{[0-9]+}}, !dbg !{{[0-9]+}}
+func2(func1());
+}
+
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -74,12 +74,11 @@
   // cast alloca to the default address space when necessary.
   if (CastToDefaultAddrSpace && getASTAllocaAddressSpace() != LangAS::Default) 
{
 auto DestAddrSpace = getContext().getTargetAddressSpace(LangAS::Default);
-auto CurIP = Builder.saveIP();
+llvm::IRBuilderBase::InsertPointGuard IPG(Builder);
 Builder.SetInsertPoint(AllocaInsertPt);
 V = getTargetHooks().performAddrSpaceCast(
 *this, V, getASTAllocaAddressSpace(), LangAS::Default,
 Ty->getPointerTo(DestAddrSpace), /*non-null*/ true);
-Builder.restoreIP(CurIP);
   }
 
   return Address(V, Align);


Index: test/CodeGenOpenCL/func-call-dbg-loc.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/func-call-dbg-loc.cl
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple amdgcn---amdgizcl -debug-info-kind=limited -O0 -emit-llvm -o - %s | FileCheck %s
+
+typedef struct
+{
+int a;
+} Struct;
+
+Struct func1();
+
+void func2(Struct S);
+
+void func3()
+{
+// CHECK: call i32 @func1() #{{[0-9]+}}, !dbg !{{[0-9]+}}
+// CHECK: call void @func2(i32 %{{[0-9]+}}) #{{[0-9]+}}, !dbg !{{[0-9]+}}
+func2(func1());
+}
+
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -74,12 +74,11 @@
   // cast alloca to the default address space when necessary.
   if (CastToDefaultAddrSpace && getASTAllocaAddressSpace() != LangAS::Default) {
 auto DestAddrSpace = getContext().getTargetAddressSpace(LangAS::Default);
-auto CurIP = Builder.saveIP();
+llvm::IRBuilderBase::InsertPointGuard IPG(Builder);
 Builder.SetInsertPoint(AllocaInsertPt);
 V = getTargetHooks().performAddrSpaceCast(
 *this, V, getASTAllocaAddressSpace(), LangAS::Default,
 Ty->getPointerTo(DestAddrSpace), /*non-null*/ true);
-Builder.restoreIP(CurIP);
   }
 
   return Address(V, Align);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39176: [Driver] Use ld.lld directly for Fuchsia rather than passing flavor

2017-10-23 Thread Petr Hosek via Phabricator via cfe-commits

phosek updated this revision to Diff 119923.

Repository:
  rL LLVM

https://reviews.llvm.org/D39176

Files:
  lib/Driver/ToolChains/Fuchsia.cpp
  lib/Driver/ToolChains/Fuchsia.h
  test/Driver/fuchsia.c
  test/Driver/fuchsia.cpp


Index: test/Driver/fuchsia.cpp
===
--- test/Driver/fuchsia.cpp
+++ test/Driver/fuchsia.cpp
@@ -1,13 +1,12 @@
 // RUN: %clangxx %s -### -no-canonical-prefixes 
--target=x86_64-unknown-fuchsia \
-// RUN: --sysroot=%S/platform 2>&1 -fuse-ld=ld | FileCheck %s
+// RUN: --sysroot=%S/platform 2>&1 | FileCheck %s
 // CHECK: {{.*}}clang{{.*}}" "-cc1"
 // CHECK: "-triple" "x86_64-fuchsia"
 // CHECK: "-fuse-init-array"
 // CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK: "-internal-isystem" 
"{{.*[/\\]}}x86_64-fuchsia{{/|}}include{{/|}}c++{{/|}}v1"
 // CHECK: "-internal-externc-isystem" "[[SYSROOT]]{{/|}}include"
-// CHECK: {{.*}}lld{{.*}}" "-flavor" "gnu"
-// CHECK: "-z" "rodynamic"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "rodynamic"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
Index: test/Driver/fuchsia.c
===
--- test/Driver/fuchsia.c
+++ test/Driver/fuchsia.c
@@ -1,16 +1,15 @@
 // RUN: %clang %s -### -no-canonical-prefixes --target=x86_64-unknown-fuchsia \
-// RUN: --sysroot=%S/platform -fuse-ld=ld 2>&1 \
+// RUN: --sysroot=%S/platform 2>&1 \
 // RUN: | FileCheck -check-prefixes=CHECK,CHECK-X86_64 %s
 // RUN: %clang %s -### -no-canonical-prefixes --target=aarch64-unknown-fuchsia 
\
-// RUN: --sysroot=%S/platform -fuse-ld=ld 2>&1 \
+// RUN: --sysroot=%S/platform 2>&1 \
 // RUN: | FileCheck -check-prefixes=CHECK,CHECK-AARCH64 %s
 // CHECK: {{.*}}clang{{.*}}" "-cc1"
 // CHECK: "-munwind-tables"
 // CHECK: "-fuse-init-array"
 // CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK: "-internal-externc-isystem" "[[SYSROOT]]{{/|}}include"
-// CHECK: {{.*}}lld{{.*}}" "-flavor" "gnu"
-// CHECK: "-z" "rodynamic"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "rodynamic"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
Index: lib/Driver/ToolChains/Fuchsia.h
===
--- lib/Driver/ToolChains/Fuchsia.h
+++ lib/Driver/ToolChains/Fuchsia.h
@@ -82,7 +82,7 @@
llvm::opt::ArgStringList &CmdArgs) const override;
 
   const char *getDefaultLinker() const override {
-return "lld";
+return "ld.lld";
   }
 
 protected:
Index: lib/Driver/ToolChains/Fuchsia.cpp
===
--- lib/Driver/ToolChains/Fuchsia.cpp
+++ lib/Driver/ToolChains/Fuchsia.cpp
@@ -44,10 +44,8 @@
   Args.ClaimAllArgs(options::OPT_w);
 
   const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
-  if (llvm::sys::path::stem(Exec).equals_lower("lld")) {
-CmdArgs.push_back("-flavor");
-CmdArgs.push_back("gnu");
-
+  if (llvm::sys::path::filename(Exec).equals_lower("ld.lld") ||
+  llvm::sys::path::stem(Exec).equals_lower("ld.lld")) {
 CmdArgs.push_back("-z");
 CmdArgs.push_back("rodynamic");
   }


Index: test/Driver/fuchsia.cpp
===
--- test/Driver/fuchsia.cpp
+++ test/Driver/fuchsia.cpp
@@ -1,13 +1,12 @@
 // RUN: %clangxx %s -### -no-canonical-prefixes --target=x86_64-unknown-fuchsia \
-// RUN: --sysroot=%S/platform 2>&1 -fuse-ld=ld | FileCheck %s
+// RUN: --sysroot=%S/platform 2>&1 | FileCheck %s
 // CHECK: {{.*}}clang{{.*}}" "-cc1"
 // CHECK: "-triple" "x86_64-fuchsia"
 // CHECK: "-fuse-init-array"
 // CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK: "-internal-isystem" "{{.*[/\\]}}x86_64-fuchsia{{/|}}include{{/|}}c++{{/|}}v1"
 // CHECK: "-internal-externc-isystem" "[[SYSROOT]]{{/|}}include"
-// CHECK: {{.*}}lld{{.*}}" "-flavor" "gnu"
-// CHECK: "-z" "rodynamic"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "rodynamic"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
Index: test/Driver/fuchsia.c
===
--- test/Driver/fuchsia.c
+++ test/Driver/fuchsia.c
@@ -1,16 +1,15 @@
 // RUN: %clang %s -### -no-canonical-prefixes --target=x86_64-unknown-fuchsia \
-// RUN: --sysroot=%S/platform -fuse-ld=ld 2>&1 \
+// RUN: --sysroot=%S/platform 2>&1 \
 // RUN: | FileCheck -check-prefixes=CHECK,CHECK-X86_64 %s
 // RUN: %clang %s -### -no-canonical-prefixes --target=aarch64-unknown-fuchsia \
-// RUN: --sysroot=%S/platform -fuse-ld=ld 2>&1 \
+// RUN: --sysroot=%S/platform 2>&1 \
 // RUN: | FileCheck -check-prefixes=CHECK,CHECK-AARCH64 %s
 // CHECK: {{.*}}clang{{.*}}" "-cc1"
 // CHECK: "-munwind-tables"
 // CHECK: "-fuse-init-array"
 // CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK: "-internal-externc-isystem" "[[SYSROOT]]{{/|}}include"
-// CHECK: {{.*}}lld{{.*}}" "-flavor

[PATCH] D39176: [Driver] Use ld.lld directly for Fuchsia rather than passing flavor

2017-10-23 Thread Roland McGrath via Phabricator via cfe-commits

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

This seems like how it always should have been, since -fuse-ld=lld sets it to 
ld.lld, not lld.
Looks to me like the existing MipsLinux and WebAssembly implementations are 
wrong too and only BareMetal is right.


Repository:
  rL LLVM

https://reviews.llvm.org/D39176



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

[PATCH] D39160: [CodeGen] __builtin_sqrt should map to the compiler's intrinsic sqrt function

2017-10-23 Thread Hal Finkel via Phabricator via cfe-commits

hfinkel added a comment.

In https://reviews.llvm.org/D39160#902918, @spatel wrote:

> Oops - I wrongly made llvm-commits a subscriber rather than cfe-commits. Let 
> me know if I should reopen under a new thread to get the patch to hit the 
> right mailing list.


Yes, please reopen.


https://reviews.llvm.org/D39160



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

[PATCH] D39204: [CodeGen] __builtin_sqrt should map to the compiler's intrinsic sqrt function

2017-10-23 Thread Sanjay Patel via Phabricator via cfe-commits

spatel created this revision.
Herald added a subscriber: mcrosier.

This patch is intended to answer a question raised in PR27435:
https://bugs.llvm.org/show_bug.cgi?id=27435

Is a programmer using __builtin_sqrt() invoking the compiler's intrinsic 
definition of sqrt or the mathlib definition of sqrt?

I know there are follow-up bugs that still need to be solved 
(https://reviews.llvm.org/D28335), but I think we should answer this first. 
Also if this patch is correct, it is enough to produce the hoped-for result in 
the PR27435 examples because we'll get this IR at -O2:

$ ./clang -O2 27435.c -S -o - -emit-llvm | grep llvm.sqrt

  %2 = call <2 x double> @llvm.sqrt.v2f64(<2 x double> %1)
  %2 = call <4 x float> @llvm.sqrt.v4f32(<4 x float> %1)


https://reviews.llvm.org/D39204

Files:
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGen/builtins.c


Index: test/CodeGen/builtins.c
===
--- test/CodeGen/builtins.c
+++ test/CodeGen/builtins.c
@@ -318,13 +318,13 @@
   // CHECK: call x86_fp80 @llvm.floor.f80
 
   resf = __builtin_sqrtf(F);
-  // CHECK: call float @sqrtf(
+  // CHECK: call float @llvm.sqrt.f32
 
   resd = __builtin_sqrt(D);
-  // CHECK: call double @sqrt(
+  // CHECK: call double @llvm.sqrt.f64
 
   resld = __builtin_sqrtl(LD);
-  // CHECK: call x86_fp80 @sqrtl(
+  // CHECK: call x86_fp80 @llvm.sqrt.f80
 
   resf = __builtin_truncf(F);
   // CHECK: call float @llvm.trunc.f32
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -941,6 +941,12 @@
   case Builtin::BI__builtin_roundl: {
 return RValue::get(emitUnaryBuiltin(*this, E, Intrinsic::round));
   }
+
+  case Builtin::BI__builtin_sqrt:
+  case Builtin::BI__builtin_sqrtf:
+  case Builtin::BI__builtin_sqrtl:
+return RValue::get(emitUnaryBuiltin(*this, E, Intrinsic::sqrt));
+
   case Builtin::BI__builtin_fmin:
   case Builtin::BI__builtin_fminf:
   case Builtin::BI__builtin_fminl: {


Index: test/CodeGen/builtins.c
===
--- test/CodeGen/builtins.c
+++ test/CodeGen/builtins.c
@@ -318,13 +318,13 @@
   // CHECK: call x86_fp80 @llvm.floor.f80
 
   resf = __builtin_sqrtf(F);
-  // CHECK: call float @sqrtf(
+  // CHECK: call float @llvm.sqrt.f32
 
   resd = __builtin_sqrt(D);
-  // CHECK: call double @sqrt(
+  // CHECK: call double @llvm.sqrt.f64
 
   resld = __builtin_sqrtl(LD);
-  // CHECK: call x86_fp80 @sqrtl(
+  // CHECK: call x86_fp80 @llvm.sqrt.f80
 
   resf = __builtin_truncf(F);
   // CHECK: call float @llvm.trunc.f32
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -941,6 +941,12 @@
   case Builtin::BI__builtin_roundl: {
 return RValue::get(emitUnaryBuiltin(*this, E, Intrinsic::round));
   }
+
+  case Builtin::BI__builtin_sqrt:
+  case Builtin::BI__builtin_sqrtf:
+  case Builtin::BI__builtin_sqrtl:
+return RValue::get(emitUnaryBuiltin(*this, E, Intrinsic::sqrt));
+
   case Builtin::BI__builtin_fmin:
   case Builtin::BI__builtin_fminf:
   case Builtin::BI__builtin_fminl: {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39160: [CodeGen] __builtin_sqrt should map to the compiler's intrinsic sqrt function

2017-10-23 Thread Sanjay Patel via Phabricator via cfe-commits

spatel abandoned this revision.
spatel added a comment.

Reincarnated as https://reviews.llvm.org/D39204.


https://reviews.llvm.org/D39160



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

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

2017-10-23 Thread Reid Kleckner via Phabricator via cfe-commits

rnk added a comment.

In https://reviews.llvm.org/D39079#904050, @lebedev.ri wrote:

> No tests?


+1, there should be an -emit-llvm test in clang/test/CodeGen/.




Comment at: lib/CodeGen/CGCall.cpp:1859
+if (auto *Fn = dyn_cast(TargetDecl)) {
+  if (!Fn->isDefined() && !AttrOnCallSite) {
+FuncAttrs.addAttribute(llvm::Attribute::NonLazyBind);

Remind me what happens when the definition is within the current DSO after 
linking. I seem to recall that the call through memory is 6 bytes and the 
direct pcrel call is 5 bytes, and the linker is required to know to rewrite the 
indirect call to a nop. Is that accurate?


https://reviews.llvm.org/D39079



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

[PATCH] D38618: Do not add a colon chunk to the code completion of class inheritance access modifiers

2017-10-23 Thread Alex Lorenz via Phabricator via cfe-commits

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

Overall LGTM, just a couple of comments:




Comment at: include/clang/Sema/Scope.h:131
+
+/// We are between inheritance colon and the real class/struct definition 
scope
+ClassInheritanceScope = 0x80,

Nit: Add '.' to the end of the comment + clang-format.



Comment at: lib/Parse/ParseDeclCXX.cpp:3198
   if (Tok.is(tok::colon)) {
+ParseScope InheritanceScope(this, Scope::ClassScope | Scope::DeclScope |
+  Scope::ClassInheritanceScope);

Might be better to use `getCurScope()->getFlags() | 
Scope::ClassInheritanceScope` to avoid dealing with any future scoping rule 
changes if such changes will occur.



Comment at: lib/Sema/SemaCodeComplete.cpp:1661
 
+bool IsNotInheritanceScope = !(S->getFlags() & 
Scope::ClassInheritanceScope);
 // public:

80 columns violation, please clang-format


https://reviews.llvm.org/D38618



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

[PATCH] D38819: [libunwind] Add support for dwarf unwinding on windows on x86_64

2017-10-23 Thread Reid Kleckner via Phabricator via cfe-commits

rnk added inline comments.



Comment at: src/UnwindRegistersRestore.S:98
+  # skip fs
+  # skip gs
+  movq  56(%rcx), %rsp  # cut back rsp to new location

mstorsjo wrote:
> mstorsjo wrote:
> > compnerd wrote:
> > > Doesn't Win64 ABI require some of the MMX registers be saved/restored too?
> > Right, yes, xmm6-xmm15 should be backed up and restored. I'll try to amend 
> > this with such a change.
> Actually, such a change doesn't necessarily make much sense on its own.
> 
> As long as the dwarf encoding itself doesn't describe how to restore those 
> registers (and on unix platforms you don't need to, so it probably isn't even 
> specified), you'd just end up backing up the xmm registers on entry when 
> throwing the exception, and restoring the exactly same ones again - it only 
> guards against changes within libcxxabi/libunwind and the unwinding machinery 
> itself, not against changes further down in the call stack between the 
> thrower and catcher of the exception.
> 
> So with that, I guess this patch is futile unless planning to extend the 
> x86_64 dwarf handling in llvm to include those registers as well - and that's 
> a little out of scope of what I intended to do here...
If we have XMM values in the register context, we might as well reload them 
here. I assume libunwind will switch away from DWARF and over to UNWIND_INFO 
opcodes in the near future, and that will give us an accurate register context.


https://reviews.llvm.org/D38819



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

[PATCH] D39176: [Driver] Use ld.lld directly for Fuchsia rather than passing flavor

2017-10-23 Thread Petr Hosek via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL316379: [Driver] Use ld.lld directly for Fuchsia rather than 
passing flavor (authored by phosek).

Changed prior to commit:
  https://reviews.llvm.org/D39176?vs=119923&id=119938#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39176

Files:
  cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp
  cfe/trunk/lib/Driver/ToolChains/Fuchsia.h
  cfe/trunk/test/Driver/fuchsia.c
  cfe/trunk/test/Driver/fuchsia.cpp


Index: cfe/trunk/test/Driver/fuchsia.c
===
--- cfe/trunk/test/Driver/fuchsia.c
+++ cfe/trunk/test/Driver/fuchsia.c
@@ -1,16 +1,15 @@
 // RUN: %clang %s -### -no-canonical-prefixes --target=x86_64-unknown-fuchsia \
-// RUN: --sysroot=%S/platform -fuse-ld=ld 2>&1 \
+// RUN: --sysroot=%S/platform 2>&1 \
 // RUN: | FileCheck -check-prefixes=CHECK,CHECK-X86_64 %s
 // RUN: %clang %s -### -no-canonical-prefixes --target=aarch64-unknown-fuchsia 
\
-// RUN: --sysroot=%S/platform -fuse-ld=ld 2>&1 \
+// RUN: --sysroot=%S/platform 2>&1 \
 // RUN: | FileCheck -check-prefixes=CHECK,CHECK-AARCH64 %s
 // CHECK: {{.*}}clang{{.*}}" "-cc1"
 // CHECK: "-munwind-tables"
 // CHECK: "-fuse-init-array"
 // CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK: "-internal-externc-isystem" "[[SYSROOT]]{{/|}}include"
-// CHECK: {{.*}}lld{{.*}}" "-flavor" "gnu"
-// CHECK: "-z" "rodynamic"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "rodynamic"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
Index: cfe/trunk/test/Driver/fuchsia.cpp
===
--- cfe/trunk/test/Driver/fuchsia.cpp
+++ cfe/trunk/test/Driver/fuchsia.cpp
@@ -1,13 +1,12 @@
 // RUN: %clangxx %s -### -no-canonical-prefixes 
--target=x86_64-unknown-fuchsia \
-// RUN: --sysroot=%S/platform 2>&1 -fuse-ld=ld | FileCheck %s
+// RUN: --sysroot=%S/platform 2>&1 | FileCheck %s
 // CHECK: {{.*}}clang{{.*}}" "-cc1"
 // CHECK: "-triple" "x86_64-fuchsia"
 // CHECK: "-fuse-init-array"
 // CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK: "-internal-isystem" 
"{{.*[/\\]}}x86_64-fuchsia{{/|}}include{{/|}}c++{{/|}}v1"
 // CHECK: "-internal-externc-isystem" "[[SYSROOT]]{{/|}}include"
-// CHECK: {{.*}}lld{{.*}}" "-flavor" "gnu"
-// CHECK: "-z" "rodynamic"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "rodynamic"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
Index: cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp
@@ -44,10 +44,8 @@
   Args.ClaimAllArgs(options::OPT_w);
 
   const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
-  if (llvm::sys::path::stem(Exec).equals_lower("lld")) {
-CmdArgs.push_back("-flavor");
-CmdArgs.push_back("gnu");
-
+  if (llvm::sys::path::filename(Exec).equals_lower("ld.lld") ||
+  llvm::sys::path::stem(Exec).equals_lower("ld.lld")) {
 CmdArgs.push_back("-z");
 CmdArgs.push_back("rodynamic");
   }
Index: cfe/trunk/lib/Driver/ToolChains/Fuchsia.h
===
--- cfe/trunk/lib/Driver/ToolChains/Fuchsia.h
+++ cfe/trunk/lib/Driver/ToolChains/Fuchsia.h
@@ -82,7 +82,7 @@
llvm::opt::ArgStringList &CmdArgs) const override;
 
   const char *getDefaultLinker() const override {
-return "lld";
+return "ld.lld";
   }
 
 protected:


Index: cfe/trunk/test/Driver/fuchsia.c
===
--- cfe/trunk/test/Driver/fuchsia.c
+++ cfe/trunk/test/Driver/fuchsia.c
@@ -1,16 +1,15 @@
 // RUN: %clang %s -### -no-canonical-prefixes --target=x86_64-unknown-fuchsia \
-// RUN: --sysroot=%S/platform -fuse-ld=ld 2>&1 \
+// RUN: --sysroot=%S/platform 2>&1 \
 // RUN: | FileCheck -check-prefixes=CHECK,CHECK-X86_64 %s
 // RUN: %clang %s -### -no-canonical-prefixes --target=aarch64-unknown-fuchsia \
-// RUN: --sysroot=%S/platform -fuse-ld=ld 2>&1 \
+// RUN: --sysroot=%S/platform 2>&1 \
 // RUN: | FileCheck -check-prefixes=CHECK,CHECK-AARCH64 %s
 // CHECK: {{.*}}clang{{.*}}" "-cc1"
 // CHECK: "-munwind-tables"
 // CHECK: "-fuse-init-array"
 // CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK: "-internal-externc-isystem" "[[SYSROOT]]{{/|}}include"
-// CHECK: {{.*}}lld{{.*}}" "-flavor" "gnu"
-// CHECK: "-z" "rodynamic"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "rodynamic"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
Index: cfe/trunk/test/Driver/fuchsia.cpp
===
--- cfe/trunk/test/Driver/fuchsia.cpp
+++ cfe/trunk/test/Driver/fuchsia.cpp
@@ -1,13 +1,12 @@
 // RUN: %clangxx %s -### -no-canonical-prefixes --target=x86_64-unknown-fuchsia \
-// RUN: --sysroot=%S/

r316379 - [Driver] Use ld.lld directly for Fuchsia rather than passing flavor

2017-10-23 Thread Petr Hosek via cfe-commits

Author: phosek
Date: Mon Oct 23 14:31:05 2017
New Revision: 316379

URL: http://llvm.org/viewvc/llvm-project?rev=316379&view=rev
Log:
[Driver] Use ld.lld directly for Fuchsia rather than passing flavor

Passing a flavor to LLD requires command line argument, but if these
are being passed through a response file, this will fail because LLD
needs to know which driver to use before processing the response file.
Use ld.lld directly instead to avoid this issue.

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

Modified:
cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp
cfe/trunk/lib/Driver/ToolChains/Fuchsia.h
cfe/trunk/test/Driver/fuchsia.c
cfe/trunk/test/Driver/fuchsia.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp?rev=316379&r1=316378&r2=316379&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp Mon Oct 23 14:31:05 2017
@@ -44,10 +44,8 @@ void fuchsia::Linker::ConstructJob(Compi
   Args.ClaimAllArgs(options::OPT_w);
 
   const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
-  if (llvm::sys::path::stem(Exec).equals_lower("lld")) {
-CmdArgs.push_back("-flavor");
-CmdArgs.push_back("gnu");
-
+  if (llvm::sys::path::filename(Exec).equals_lower("ld.lld") ||
+  llvm::sys::path::stem(Exec).equals_lower("ld.lld")) {
 CmdArgs.push_back("-z");
 CmdArgs.push_back("rodynamic");
   }

Modified: cfe/trunk/lib/Driver/ToolChains/Fuchsia.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Fuchsia.h?rev=316379&r1=316378&r2=316379&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Fuchsia.h (original)
+++ cfe/trunk/lib/Driver/ToolChains/Fuchsia.h Mon Oct 23 14:31:05 2017
@@ -82,7 +82,7 @@ public:
llvm::opt::ArgStringList &CmdArgs) const override;
 
   const char *getDefaultLinker() const override {
-return "lld";
+return "ld.lld";
   }
 
 protected:

Modified: cfe/trunk/test/Driver/fuchsia.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/fuchsia.c?rev=316379&r1=316378&r2=316379&view=diff
==
--- cfe/trunk/test/Driver/fuchsia.c (original)
+++ cfe/trunk/test/Driver/fuchsia.c Mon Oct 23 14:31:05 2017
@@ -1,16 +1,15 @@
 // RUN: %clang %s -### -no-canonical-prefixes --target=x86_64-unknown-fuchsia \
-// RUN: --sysroot=%S/platform -fuse-ld=ld 2>&1 \
+// RUN: --sysroot=%S/platform 2>&1 \
 // RUN: | FileCheck -check-prefixes=CHECK,CHECK-X86_64 %s
 // RUN: %clang %s -### -no-canonical-prefixes --target=aarch64-unknown-fuchsia 
\
-// RUN: --sysroot=%S/platform -fuse-ld=ld 2>&1 \
+// RUN: --sysroot=%S/platform 2>&1 \
 // RUN: | FileCheck -check-prefixes=CHECK,CHECK-AARCH64 %s
 // CHECK: {{.*}}clang{{.*}}" "-cc1"
 // CHECK: "-munwind-tables"
 // CHECK: "-fuse-init-array"
 // CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK: "-internal-externc-isystem" "[[SYSROOT]]{{/|}}include"
-// CHECK: {{.*}}lld{{.*}}" "-flavor" "gnu"
-// CHECK: "-z" "rodynamic"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "rodynamic"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"

Modified: cfe/trunk/test/Driver/fuchsia.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/fuchsia.cpp?rev=316379&r1=316378&r2=316379&view=diff
==
--- cfe/trunk/test/Driver/fuchsia.cpp (original)
+++ cfe/trunk/test/Driver/fuchsia.cpp Mon Oct 23 14:31:05 2017
@@ -1,13 +1,12 @@
 // RUN: %clangxx %s -### -no-canonical-prefixes 
--target=x86_64-unknown-fuchsia \
-// RUN: --sysroot=%S/platform 2>&1 -fuse-ld=ld | FileCheck %s
+// RUN: --sysroot=%S/platform 2>&1 | FileCheck %s
 // CHECK: {{.*}}clang{{.*}}" "-cc1"
 // CHECK: "-triple" "x86_64-fuchsia"
 // CHECK: "-fuse-init-array"
 // CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK: "-internal-isystem" 
"{{.*[/\\]}}x86_64-fuchsia{{/|}}include{{/|}}c++{{/|}}v1"
 // CHECK: "-internal-externc-isystem" "[[SYSROOT]]{{/|}}include"
-// CHECK: {{.*}}lld{{.*}}" "-flavor" "gnu"
-// CHECK: "-z" "rodynamic"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "rodynamic"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"


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

[PATCH] D38819: [libunwind] Add support for dwarf unwinding on windows on x86_64

2017-10-23 Thread Martin Storsjö via Phabricator via cfe-commits

mstorsjo added inline comments.



Comment at: src/UnwindRegistersRestore.S:98
+  # skip fs
+  # skip gs
+  movq  56(%rcx), %rsp  # cut back rsp to new location

rnk wrote:
> mstorsjo wrote:
> > mstorsjo wrote:
> > > compnerd wrote:
> > > > Doesn't Win64 ABI require some of the MMX registers be saved/restored 
> > > > too?
> > > Right, yes, xmm6-xmm15 should be backed up and restored. I'll try to 
> > > amend this with such a change.
> > Actually, such a change doesn't necessarily make much sense on its own.
> > 
> > As long as the dwarf encoding itself doesn't describe how to restore those 
> > registers (and on unix platforms you don't need to, so it probably isn't 
> > even specified), you'd just end up backing up the xmm registers on entry 
> > when throwing the exception, and restoring the exactly same ones again - it 
> > only guards against changes within libcxxabi/libunwind and the unwinding 
> > machinery itself, not against changes further down in the call stack 
> > between the thrower and catcher of the exception.
> > 
> > So with that, I guess this patch is futile unless planning to extend the 
> > x86_64 dwarf handling in llvm to include those registers as well - and 
> > that's a little out of scope of what I intended to do here...
> If we have XMM values in the register context, we might as well reload them 
> here. I assume libunwind will switch away from DWARF and over to UNWIND_INFO 
> opcodes in the near future, and that will give us an accurate register 
> context.
Yeah, although to do anything useful with the XMM registers, the llvm backend 
would have to output dwarf opcodes for callee saved xmm registers, which it 
currently doesn't. Otherwise it'll just reload them back to the same values as 
they already are. I've managed to hack LLVM to do that (without fully 
understanding what I'm doing though), and I'll see if I manage to update 
libunwind to use that as well. 


https://reviews.llvm.org/D38819



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

Re: r316379 - [Driver] Use ld.lld directly for Fuchsia rather than passing flavor

2017-10-23 Thread Rui Ueyama via cfe-commits

Nice. Dispatching based on argv[0] is more common and probably a
recommended way now, so this patch aligned with that.

On Mon, Oct 23, 2017 at 2:31 PM, Petr Hosek via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: phosek
> Date: Mon Oct 23 14:31:05 2017
> New Revision: 316379
>
> URL: http://llvm.org/viewvc/llvm-project?rev=316379&view=rev
> Log:
> [Driver] Use ld.lld directly for Fuchsia rather than passing flavor
>
> Passing a flavor to LLD requires command line argument, but if these
> are being passed through a response file, this will fail because LLD
> needs to know which driver to use before processing the response file.
> Use ld.lld directly instead to avoid this issue.
>
> Differential Revision: https://reviews.llvm.org/D39176
>
> Modified:
> cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp
> cfe/trunk/lib/Driver/ToolChains/Fuchsia.h
> cfe/trunk/test/Driver/fuchsia.c
> cfe/trunk/test/Driver/fuchsia.cpp
>
> Modified: cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/
> ToolChains/Fuchsia.cpp?rev=316379&r1=316378&r2=316379&view=diff
> 
> ==
> --- cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp (original)
> +++ cfe/trunk/lib/Driver/ToolChains/Fuchsia.cpp Mon Oct 23 14:31:05 2017
> @@ -44,10 +44,8 @@ void fuchsia::Linker::ConstructJob(Compi
>Args.ClaimAllArgs(options::OPT_w);
>
>const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
> -  if (llvm::sys::path::stem(Exec).equals_lower("lld")) {
> -CmdArgs.push_back("-flavor");
> -CmdArgs.push_back("gnu");
> -
> +  if (llvm::sys::path::filename(Exec).equals_lower("ld.lld") ||
> +  llvm::sys::path::stem(Exec).equals_lower("ld.lld")) {
>  CmdArgs.push_back("-z");
>  CmdArgs.push_back("rodynamic");
>}
>
> Modified: cfe/trunk/lib/Driver/ToolChains/Fuchsia.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/
> ToolChains/Fuchsia.h?rev=316379&r1=316378&r2=316379&view=diff
> 
> ==
> --- cfe/trunk/lib/Driver/ToolChains/Fuchsia.h (original)
> +++ cfe/trunk/lib/Driver/ToolChains/Fuchsia.h Mon Oct 23 14:31:05 2017
> @@ -82,7 +82,7 @@ public:
> llvm::opt::ArgStringList &CmdArgs) const
> override;
>
>const char *getDefaultLinker() const override {
> -return "lld";
> +return "ld.lld";
>}
>
>  protected:
>
> Modified: cfe/trunk/test/Driver/fuchsia.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/
> fuchsia.c?rev=316379&r1=316378&r2=316379&view=diff
> 
> ==
> --- cfe/trunk/test/Driver/fuchsia.c (original)
> +++ cfe/trunk/test/Driver/fuchsia.c Mon Oct 23 14:31:05 2017
> @@ -1,16 +1,15 @@
>  // RUN: %clang %s -### -no-canonical-prefixes --target=x86_64-unknown-fuchsia
> \
> -// RUN: --sysroot=%S/platform -fuse-ld=ld 2>&1 \
> +// RUN: --sysroot=%S/platform 2>&1 \
>  // RUN: | FileCheck -check-prefixes=CHECK,CHECK-X86_64 %s
>  // RUN: %clang %s -### -no-canonical-prefixes 
> --target=aarch64-unknown-fuchsia
> \
> -// RUN: --sysroot=%S/platform -fuse-ld=ld 2>&1 \
> +// RUN: --sysroot=%S/platform 2>&1 \
>  // RUN: | FileCheck -check-prefixes=CHECK,CHECK-AARCH64 %s
>  // CHECK: {{.*}}clang{{.*}}" "-cc1"
>  // CHECK: "-munwind-tables"
>  // CHECK: "-fuse-init-array"
>  // CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
>  // CHECK: "-internal-externc-isystem" "[[SYSROOT]]{{/|}}include"
> -// CHECK: {{.*}}lld{{.*}}" "-flavor" "gnu"
> -// CHECK: "-z" "rodynamic"
> +// CHECK: {{.*}}ld.lld{{.*}}" "-z" "rodynamic"
>  // CHECK: "--sysroot=[[SYSROOT]]"
>  // CHECK: "-pie"
>  // CHECK: "--build-id"
>
> Modified: cfe/trunk/test/Driver/fuchsia.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/
> fuchsia.cpp?rev=316379&r1=316378&r2=316379&view=diff
> 
> ==
> --- cfe/trunk/test/Driver/fuchsia.cpp (original)
> +++ cfe/trunk/test/Driver/fuchsia.cpp Mon Oct 23 14:31:05 2017
> @@ -1,13 +1,12 @@
>  // RUN: %clangxx %s -### -no-canonical-prefixes 
> --target=x86_64-unknown-fuchsia
> \
> -// RUN: --sysroot=%S/platform 2>&1 -fuse-ld=ld | FileCheck %s
> +// RUN: --sysroot=%S/platform 2>&1 | FileCheck %s
>  // CHECK: {{.*}}clang{{.*}}" "-cc1"
>  // CHECK: "-triple" "x86_64-fuchsia"
>  // CHECK: "-fuse-init-array"
>  // CHECK: "-isysroot" "[[SYSROOT:[^"]+]]"
>  // CHECK: "-internal-isystem" "{{.*[/\\]}}x86_64-fuchsia{{/|
> }}include{{/|}}c++{{/|}}v1"
>  // CHECK: "-internal-externc-isystem" "[[SYSROOT]]{{/|}}include"
> -// CHECK: {{.*}}lld{{.*}}" "-flavor" "gnu"
> -// CHECK: "-z" "rodynamic"
> +// CHECK: {{.*}}ld.lld{{.*}}" "-z" "rodynamic"
>  // CHECK: "--sysroot=[[SYSROOT]]"
>  // CHECK: "-pie"
>  // CHECK: "--build-id"
>
>
>

[PATCH] D39206: [libunwind] Add missing checks for register number

2017-10-23 Thread Martin Storsjö via Phabricator via cfe-commits

mstorsjo created this revision.
Herald added a subscriber: JDevlieghere.

Most other cases that touch `savedRegisters[reg]` have got this check, but 
these three seemed to lack it.


https://reviews.llvm.org/D39206

Files:
  src/DwarfParser.hpp


Index: src/DwarfParser.hpp
===
--- src/DwarfParser.hpp
+++ src/DwarfParser.hpp
@@ -605,6 +605,11 @@
   break;
 case DW_CFA_val_offset:
   reg = addressSpace.getULEB128(p, instructionsEnd);
+  if (reg > kMaxRegisterNumber) {
+fprintf(stderr,
+"malformed DW_CFA_val_offset DWARF unwind, reg too big\n");
+return false;
+  }
   offset = (int64_t)addressSpace.getULEB128(p, instructionsEnd)
 * cieInfo.dataAlignFactor;
   results->savedRegisters[reg].location = kRegisterOffsetFromCFA;
@@ -668,6 +673,11 @@
   switch (opcode & 0xC0) {
   case DW_CFA_offset:
 reg = operand;
+if (reg > kMaxRegisterNumber) {
+  fprintf(stderr,
+  "malformed DW_CFA_offset DWARF unwind, reg too big\n");
+  return false;
+}
 offset = (int64_t)addressSpace.getULEB128(p, instructionsEnd)
 * cieInfo.dataAlignFactor;
 results->savedRegisters[reg].location = kRegisterInCFA;
@@ -682,6 +692,11 @@
 break;
   case DW_CFA_restore:
 reg = operand;
+if (reg > kMaxRegisterNumber) {
+  fprintf(stderr,
+  "malformed DW_CFA_restore DWARF unwind, reg too big\n");
+  return false;
+}
 results->savedRegisters[reg] = initialState.savedRegisters[reg];
 _LIBUNWIND_TRACE_DWARF("DW_CFA_restore(reg=%" PRIu64 ")\n",
static_cast(operand));


Index: src/DwarfParser.hpp
===
--- src/DwarfParser.hpp
+++ src/DwarfParser.hpp
@@ -605,6 +605,11 @@
   break;
 case DW_CFA_val_offset:
   reg = addressSpace.getULEB128(p, instructionsEnd);
+  if (reg > kMaxRegisterNumber) {
+fprintf(stderr,
+"malformed DW_CFA_val_offset DWARF unwind, reg too big\n");
+return false;
+  }
   offset = (int64_t)addressSpace.getULEB128(p, instructionsEnd)
 * cieInfo.dataAlignFactor;
   results->savedRegisters[reg].location = kRegisterOffsetFromCFA;
@@ -668,6 +673,11 @@
   switch (opcode & 0xC0) {
   case DW_CFA_offset:
 reg = operand;
+if (reg > kMaxRegisterNumber) {
+  fprintf(stderr,
+  "malformed DW_CFA_offset DWARF unwind, reg too big\n");
+  return false;
+}
 offset = (int64_t)addressSpace.getULEB128(p, instructionsEnd)
 * cieInfo.dataAlignFactor;
 results->savedRegisters[reg].location = kRegisterInCFA;
@@ -682,6 +692,11 @@
 break;
   case DW_CFA_restore:
 reg = operand;
+if (reg > kMaxRegisterNumber) {
+  fprintf(stderr,
+  "malformed DW_CFA_restore DWARF unwind, reg too big\n");
+  return false;
+}
 results->savedRegisters[reg] = initialState.savedRegisters[reg];
 _LIBUNWIND_TRACE_DWARF("DW_CFA_restore(reg=%" PRIu64 ")\n",
static_cast(operand));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38939: [clangd] Handle exit notification (proper shutdown)

2017-10-23 Thread Raoul Wols via Phabricator via cfe-commits

rwols updated this revision to Diff 119941.
rwols added a comment.

- Clarify why we invert the tests in protocol.test,
- Use a variable name to clarify why we exit with status code 1 (when run() 
returns true),
- Reword misleading comment in onShutdown method.


https://reviews.llvm.org/D38939

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h
  clangd/tool/ClangdMain.cpp
  test/clangd/authority-less-uri.test
  test/clangd/completion-priorities.test
  test/clangd/completion-qualifiers.test
  test/clangd/completion-snippet.test
  test/clangd/completion.test
  test/clangd/definitions.test
  test/clangd/diagnostics-preamble.test
  test/clangd/diagnostics.test
  test/clangd/did-change-watch-files.test
  test/clangd/extra-flags.test
  test/clangd/fixits.test
  test/clangd/formatting.test
  test/clangd/initialize-params-invalid.test
  test/clangd/initialize-params.test
  test/clangd/input-mirror.test
  test/clangd/protocol.test
  test/clangd/shutdown-with-exit.test
  test/clangd/shutdown-without-exit.test
  test/clangd/signature-help.test
  test/clangd/unsupported-method.test

Index: test/clangd/unsupported-method.test
===
--- test/clangd/unsupported-method.test
+++ test/clangd/unsupported-method.test
@@ -17,3 +17,7 @@
 Content-Length: 44
 
 {"jsonrpc":"2.0","id":2,"method":"shutdown"}
+# CHECK: {"jsonrpc":"2.0","id":2,"result":null}
+Content-Length: 33
+
+{"jsonrpc":"2.0":"method":"exit"}
Index: test/clangd/signature-help.test
===
--- test/clangd/signature-help.test
+++ test/clangd/signature-help.test
@@ -40,3 +40,7 @@
 Content-Length: 49
 
 {"jsonrpc":"2.0","id":10,"method":"shutdown"}
+# CHECK: {"jsonrpc":"2.0","id":10,"result":null}
+Content-Length: 33
+
+{"jsonrpc":"2.0":"method":"exit"}
Index: test/clangd/shutdown-without-exit.test
===
--- /dev/null
+++ test/clangd/shutdown-without-exit.test
@@ -0,0 +1,6 @@
+# RUN: not clangd -run-synchronously < %s
+# vim: fileformat=dos
+# It is absolutely vital that this file has CRLF line endings.
+Content-Length: 33
+
+{"jsonrpc":"2.0":"method":"exit"}
Index: test/clangd/shutdown-with-exit.test
===
--- /dev/null
+++ test/clangd/shutdown-with-exit.test
@@ -0,0 +1,9 @@
+# RUN: clangd -run-synchronously < %s
+# vim: fileformat=dos
+# It is absolutely vital that this file has CRLF line endings.
+Content-Length: 44
+
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+Content-Length: 33
+
+{"jsonrpc":"2.0":"method":"exit"}
Index: test/clangd/protocol.test
===
--- test/clangd/protocol.test
+++ test/clangd/protocol.test
@@ -1,8 +1,10 @@
-# RUN: clangd -run-synchronously < %s | FileCheck %s
-# RUN: clangd -run-synchronously < %s 2>&1 | FileCheck -check-prefix=STDERR %s
+# RUN: not clangd -run-synchronously < %s | FileCheck %s
+# RUN: not clangd -run-synchronously < %s 2>&1 | FileCheck -check-prefix=STDERR %s
 # vim: fileformat=dos
 # It is absolutely vital that this file has CRLF line endings.
 #
+# Note that we invert the test because we intent to let clangd exit prematurely.
+#
 # Test protocol parsing
 Content-Length: 125
 Content-Type: application/vscode-jsonrpc; charset-utf-8
Index: test/clangd/input-mirror.test
===
--- test/clangd/input-mirror.test
+++ test/clangd/input-mirror.test
@@ -152,3 +152,7 @@
 Content-Length: 44
 
 {"jsonrpc":"2.0","id":3,"method":"shutdown"}
+# CHECK: {"jsonrpc":"2.0","id":3,"result":null}
+Content-Length: 33
+
+{"jsonrpc":"2.0":"method":"exit"}
Index: test/clangd/initialize-params.test
===
--- test/clangd/initialize-params.test
+++ test/clangd/initialize-params.test
@@ -20,3 +20,7 @@
 Content-Length: 44
 
 {"jsonrpc":"2.0","id":3,"method":"shutdown"}
+# CHECK: {"jsonrpc":"2.0","id":3,"result":null}
+Content-Length: 33
+
+{"jsonrpc":"2.0":"method":"exit"}
Index: test/clangd/initialize-params-invalid.test
===
--- test/clangd/initialize-params-invalid.test
+++ test/clangd/initialize-params-invalid.test
@@ -20,3 +20,7 @@
 Content-Length: 44
 
 {"jsonrpc":"2.0","id":3,"method":"shutdown"}
+# CHECK: {"jsonrpc":"2.0","id":3,"result":null}
+Content-Length: 33
+
+{"jsonrpc":"2.0":"method":"exit"}
Index: test/clangd/formatting.test
===
--- test/clangd/formatting.test
+++ test/clangd/formatting.test
@@ -65,3 +65,7 @@
 Content-Length: 44
 
 {"jsonrpc":"2.0","id":6,"method":"shutdown"}
+# CHECK: {"jsonrpc":"2.0","id":6,"result":null}
+Content-Length: 33
+
+{"jsonrpc":"2.0":"method":"exit"}
Index:

[PATCH] D39204: [CodeGen] __builtin_sqrt should map to the compiler's intrinsic sqrt function

2017-10-23 Thread Eli Friedman via Phabricator via cfe-commits

efriedma added a comment.

The gcc documentation says "GCC includes built-in versions of many of the 
functions in the standard C library. These functions come in two forms: one 
whose names start with the `__builtin_` prefix, and the other without. Both 
forms have the same type (including prototype), the same address (when their 
address is taken), and the same meaning as the C library functions".  And gcc 
specifically preserves the stated semantics.  Given that, I'm not sure it makes 
sense for us to try to redefine `__builtin_sqrt()` just because it's convenient.

Note that this reasoning only applies if the user hasn't specified any 
fast-math flags; under -ffinite-math-only, we can assume the result isn't a 
NaN, and therefore we can use `llvm.sqrt.*`. (The definition of `llvm.sqrt.*` 
changed in https://reviews.llvm.org/D28797; I don't think we ever updated clang 
to take advantage of this).

If we really need a name for the never-sets-errno sqrt, we should probably use 
a different name, e.g. `__builtin_ieee_sqrt()`.


https://reviews.llvm.org/D39204



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

[PATCH] D38939: [clangd] Handle exit notification (proper shutdown)

2017-10-23 Thread Raoul Wols via Phabricator via cfe-commits

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

> you have commit access now, right?

I'll have to think about if I want that responsibility!


https://reviews.llvm.org/D38939



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

r316381 - [Sema] Add support for flexible array members in Obj-C.

2017-10-23 Thread Volodymyr Sapsai via cfe-commits

Author: vsapsai
Date: Mon Oct 23 15:01:41 2017
New Revision: 316381

URL: http://llvm.org/viewvc/llvm-project?rev=316381&view=rev
Log:
[Sema] Add support for flexible array members in Obj-C.

Allow Obj-C ivars with incomplete array type but only as the last ivar.
Also add a requirement for ivars that contain a flexible array member to
be at the end of class too. It is possible to add in a subclass another
ivar at the end but we'll emit a warning in this case. Also we'll emit a
warning if a variable sized ivar is declared in class extension or in
implementation because subclasses won't know they should avoid adding
new ivars.

In ARC incomplete array objects are treated as __unsafe_unretained so
require them to be marked as such.

Prohibit synthesizing ivars with flexible array members because order of
synthesized ivars is not obvious and tricky to control. Spelling out
ivar explicitly gives control to developers and helps to avoid surprises
with unexpected ivar ordering.

For C and C++ changed diagnostic to tell explicitly a field is not the
last one and point to the next field. It is not as useful as in Obj-C
but it is an improvement and it is consistent with Obj-C. For C for
unions emit more specific err_flexible_array_union instead of generic
err_field_incomplete.

rdar://problem/21054495

Reviewers: rjmccall, theraven

Reviewed By: rjmccall

Subscribers: cfe-commits

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


Added:
cfe/trunk/test/SemaObjC/flexible-array-arc.m
cfe/trunk/test/SemaObjC/flexible-array.m
cfe/trunk/test/SemaObjCXX/flexible-array.mm
Modified:
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaDeclObjC.cpp
cfe/trunk/lib/Sema/SemaObjCProperty.cpp
cfe/trunk/test/Sema/transparent-union.c
cfe/trunk/test/SemaCXX/flexible-array-test.cpp
cfe/trunk/test/SemaObjC/ivar-sem-check-1.m

Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=316381&r1=316380&r2=316381&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Mon Oct 23 15:01:41 2017
@@ -374,6 +374,7 @@ def ObjCRootClass : DiagGroup<"objc-root
 def ObjCPointerIntrospectPerformSelector : 
DiagGroup<"deprecated-objc-pointer-introspection-performSelector">;
 def ObjCPointerIntrospect : DiagGroup<"deprecated-objc-pointer-introspection", 
[ObjCPointerIntrospectPerformSelector]>;
 def ObjCMultipleMethodNames : DiagGroup<"objc-multiple-method-names">;
+def ObjCFlexibleArray : DiagGroup<"objc-flexible-array">;
 def OpenCLUnsupportedRGBA: DiagGroup<"opencl-unsupported-rgba">;
 def DeprecatedObjCIsaUsage : DiagGroup<"deprecated-objc-isa-usage">;
 def ExplicitInitializeCall : DiagGroup<"explicit-initialize-call">;
@@ -750,6 +751,7 @@ def Most : DiagGroup<"most", [
 VolatileRegisterVar,
 ObjCMissingSuperCalls,
 ObjCDesignatedInit,
+ObjCFlexibleArray,
 OverloadedVirtual,
 PrivateExtern,
 SelTypeCast,

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=316381&r1=316380&r2=316381&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Oct 23 15:01:41 
2017
@@ -5222,6 +5222,28 @@ def ext_flexible_array_empty_aggregate_g
 def ext_flexible_array_union_gnu : Extension<
   "flexible array member %0 in a union is a GNU extension">, 
InGroup;
 
+def err_flexible_array_not_at_end : Error<
+  "flexible array member %0 with type %1 is not at the end of"
+  " %select{struct|interface|union|class|enum}2">;
+def err_objc_variable_sized_type_not_at_end : Error<
+  "field %0 with variable sized type %1 is not at the end of class">;
+def note_next_field_declaration : Note<
+  "next field declaration is here">;
+def note_next_ivar_declaration : Note<
+  "next %select{instance variable declaration|synthesized instance variable}0"
+  " is here">;
+def err_synthesize_variable_sized_ivar : Error<
+  "synthesized property with variable size type %0"
+  " requires an existing instance variable">;
+def err_flexible_array_arc_retainable : Error<
+  "ARC forbids flexible array members with retainable object type">;
+def warn_variable_sized_ivar_visibility : Warning<
+  "field %0 with variable sized type %1 is not visible to subclasses and"
+  " can conflict with their instance variables">, InGroup;
+def warn_superclass_variable_sized_type_not_at_end : Warning<
+  "field %0 can overwrite instance variable %1 with variable sized type %2"
+  " in superclass %3">, InGroup;
+
 let C

[PATCH] D38773: [Sema] Add support for flexible array members in Obj-C.

2017-10-23 Thread Volodymyr Sapsai via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL316381: [Sema] Add support for flexible array members in 
Obj-C. (authored by vsapsai).

Changed prior to commit:
  https://reviews.llvm.org/D38773?vs=118990&id=119943#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38773

Files:
  cfe/trunk/include/clang/Basic/DiagnosticGroups.td
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/lib/Sema/SemaDecl.cpp
  cfe/trunk/lib/Sema/SemaDeclObjC.cpp
  cfe/trunk/lib/Sema/SemaObjCProperty.cpp
  cfe/trunk/test/Sema/transparent-union.c
  cfe/trunk/test/SemaCXX/flexible-array-test.cpp
  cfe/trunk/test/SemaObjC/flexible-array-arc.m
  cfe/trunk/test/SemaObjC/flexible-array.m
  cfe/trunk/test/SemaObjC/ivar-sem-check-1.m
  cfe/trunk/test/SemaObjCXX/flexible-array.mm

Index: cfe/trunk/lib/Sema/SemaObjCProperty.cpp
===
--- cfe/trunk/lib/Sema/SemaObjCProperty.cpp
+++ cfe/trunk/lib/Sema/SemaObjCProperty.cpp
@@ -1290,6 +1290,14 @@
 // An abstract type is as bad as an incomplete type.
 CompleteTypeErr = true;
   }
+  if (!CompleteTypeErr) {
+const RecordType *RecordTy = PropertyIvarType->getAs();
+if (RecordTy && RecordTy->getDecl()->hasFlexibleArrayMember()) {
+  Diag(PropertyIvarLoc, diag::err_synthesize_variable_sized_ivar)
+<< PropertyIvarType;
+  CompleteTypeErr = true; // suppress later diagnostics about the ivar
+}
+  }
   if (CompleteTypeErr)
 Ivar->setInvalidDecl();
   ClassImpDecl->addDecl(Ivar);
Index: cfe/trunk/lib/Sema/SemaDecl.cpp
===
--- cfe/trunk/lib/Sema/SemaDecl.cpp
+++ cfe/trunk/lib/Sema/SemaDecl.cpp
@@ -14997,67 +14997,78 @@
 //   possibly recursively, a member that is such a structure)
 //   shall not be a member of a structure or an element of an
 //   array.
+bool IsLastField = (i + 1 == Fields.end());
 if (FDTy->isFunctionType()) {
   // Field declared as a function.
   Diag(FD->getLocation(), diag::err_field_declared_as_function)
 << FD->getDeclName();
   FD->setInvalidDecl();
   EnclosingDecl->setInvalidDecl();
   continue;
-} else if (FDTy->isIncompleteArrayType() && Record &&
-   ((i + 1 == Fields.end() && !Record->isUnion()) ||
-((getLangOpts().MicrosoftExt ||
-  getLangOpts().CPlusPlus) &&
- (i + 1 == Fields.end() || Record->isUnion() {
-  // Flexible array member.
-  // Microsoft and g++ is more permissive regarding flexible array.
-  // It will accept flexible array in union and also
-  // as the sole element of a struct/class.
-  unsigned DiagID = 0;
-  if (Record->isUnion())
-DiagID = getLangOpts().MicrosoftExt
- ? diag::ext_flexible_array_union_ms
- : getLangOpts().CPlusPlus
-   ? diag::ext_flexible_array_union_gnu
-   : diag::err_flexible_array_union;
-  else if (NumNamedMembers < 1)
-DiagID = getLangOpts().MicrosoftExt
- ? diag::ext_flexible_array_empty_aggregate_ms
- : getLangOpts().CPlusPlus
-   ? diag::ext_flexible_array_empty_aggregate_gnu
-   : diag::err_flexible_array_empty_aggregate;
-
-  if (DiagID)
-Diag(FD->getLocation(), DiagID) << FD->getDeclName()
-<< Record->getTagKind();
-  // While the layout of types that contain virtual bases is not specified
-  // by the C++ standard, both the Itanium and Microsoft C++ ABIs place
-  // virtual bases after the derived members.  This would make a flexible
-  // array member declared at the end of an object not adjacent to the end
-  // of the type.
-  if (CXXRecordDecl *RD = dyn_cast(Record))
-if (RD->getNumVBases() != 0)
-  Diag(FD->getLocation(), diag::err_flexible_array_virtual_base)
+} else if (FDTy->isIncompleteArrayType() &&
+   (Record || isa(EnclosingDecl))) {
+  if (Record) {
+// Flexible array member.
+// Microsoft and g++ is more permissive regarding flexible array.
+// It will accept flexible array in union and also
+// as the sole element of a struct/class.
+unsigned DiagID = 0;
+if (!Record->isUnion() && !IsLastField) {
+  Diag(FD->getLocation(), diag::err_flexible_array_not_at_end)
+<< FD->getDeclName() << FD->getType() << Record->getTagKind();
+  Diag((*(i + 1))->getLocation(), diag::note_next_field_declaration);
+  FD->setInvalidDecl();
+  EnclosingDecl->setInvalidDecl();
+  continue;
+} else if (Record->isUnion())
+  DiagID = getLangOpts().MicrosoftExt
+   ? diag::ext_fle

[PATCH] D39201: [Analyzer] Handle implicit function reference in bodyfarming std::call_once

2017-10-23 Thread Devin Coughlin via Phabricator via cfe-commits

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

LGTM!


https://reviews.llvm.org/D39201



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

[PATCH] D39069: CodeGen: Fix missing debug loc due to alloca

2017-10-23 Thread John McCall via Phabricator via cfe-commits

rjmccall added a comment.

The code looks fine to me.  The test seems very vague to me, but I'd like Dave 
to weigh in on that, because I'm not sure it's reasonable to test the exact 
pattern here.

Also, Dave, I don't know what the IR invariants around debug info are.  Is this 
something we should be updating all of our uses of saveIP() to do if there's a 
possibility of emitting arbitrary code while the IP is saved?


https://reviews.llvm.org/D39069



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

[PATCH] D38774: [CodeGen] Add support for IncompleteArrayType in Obj-C ivars.

2017-10-23 Thread Volodymyr Sapsai via Phabricator via cfe-commits

vsapsai added inline comments.



Comment at: clang/lib/CodeGen/CGObjCMac.cpp:5095
+fieldType = fieldType->getAsArrayTypeUnsafe()->getElementType();
+  }
+

rjmccall wrote:
> You can't just use isa<> here; there can be typedefs of incomplete array type.
Thanks for catching it.


https://reviews.llvm.org/D38774



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

[PATCH] D39079: New clang option -fno-plt to avoid PLT for external calls

2017-10-23 Thread Sriraman Tallam via Phabricator via cfe-commits

tmsriram updated this revision to Diff 119950.
tmsriram added a comment.

Added test test/CodeGen/noplt.c


https://reviews.llvm.org/D39079

Files:
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/CGCall.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/noplt.c


Index: test/CodeGen/noplt.c
===
--- test/CodeGen/noplt.c
+++ test/CodeGen/noplt.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -emit-llvm -fno-plt %s -o - | FileCheck %s 
-check-prefix=CHECK-NOPLT
+
+// CHECK-NOPLT: Function Attrs: nonlazybind
+// CHECK-NOPLT-NEXT: declare i32 @foo
+int foo();
+
+int bar() {
+  return foo();
+}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -650,6 +650,7 @@
   Args.hasArg(OPT_mincremental_linker_compatible);
   Opts.PIECopyRelocations =
   Args.hasArg(OPT_mpie_copy_relocations);
+  Opts.NoPLT = Args.hasArg(OPT_fno_plt);
   Opts.OmitLeafFramePointer = Args.hasArg(OPT_momit_leaf_frame_pointer);
   Opts.SaveTempLabels = Args.hasArg(OPT_msave_temp_labels);
   Opts.NoDwarfDirectoryAsm = Args.hasArg(OPT_fno_dwarf_directory_asm);
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3386,6 +3386,10 @@
 CmdArgs.push_back("-mpie-copy-relocations");
   }
 
+  if (Args.hasFlag(options::OPT_fno_plt, options::OPT_fplt, false)) {
+CmdArgs.push_back("-fno-plt");
+  }
+
   // -fhosted is default.
   // TODO: Audit uses of KernelOrKext and see where it'd be more appropriate to
   // use Freestanding.
Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -1852,6 +1852,16 @@
   !(TargetDecl && TargetDecl->hasAttr()))
 FuncAttrs.addAttribute("split-stack");
 
+  // Add NonLazyBind attribute to function declarations when -fno-plt
+  // is used.
+  if (TargetDecl && CodeGenOpts.NoPLT) {
+if (auto *Fn = dyn_cast(TargetDecl)) {
+  if (!Fn->isDefined() && !AttrOnCallSite) {
+FuncAttrs.addAttribute(llvm::Attribute::NonLazyBind);
+  }
+}
+  }
+
   if (!AttrOnCallSite) {
 bool DisableTailCalls =
 CodeGenOpts.DisableTailCalls ||
Index: include/clang/Frontend/CodeGenOptions.def
===
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -294,6 +294,8 @@
 /// Whether to emit .debug_gnu_pubnames section instead of .debug_pubnames.
 CODEGENOPT(GnuPubnames, 1, 0)
 
+CODEGENOPT(NoPLT, 1, 0)
+
 #undef CODEGENOPT
 #undef ENUM_CODEGENOPT
 #undef VALUE_CODEGENOPT
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1363,6 +1363,10 @@
 def fno_pic : Flag<["-"], "fno-pic">, Group;
 def fpie : Flag<["-"], "fpie">, Group;
 def fno_pie : Flag<["-"], "fno-pie">, Group;
+def fplt : Flag<["-"], "fplt">, Group, Flags<[CC1Option]>,
+  HelpText<"Use the PLT to make function calls">;
+def fno_plt : Flag<["-"], "fno-plt">, Group, Flags<[CC1Option]>,
+  HelpText<"Do not use the PLT to make function calls">;
 def fropi : Flag<["-"], "fropi">, Group;
 def fno_ropi : Flag<["-"], "fno-ropi">, Group;
 def frwpi : Flag<["-"], "frwpi">, Group;


Index: test/CodeGen/noplt.c
===
--- test/CodeGen/noplt.c
+++ test/CodeGen/noplt.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -emit-llvm -fno-plt %s -o - | FileCheck %s -check-prefix=CHECK-NOPLT
+
+// CHECK-NOPLT: Function Attrs: nonlazybind
+// CHECK-NOPLT-NEXT: declare i32 @foo
+int foo();
+
+int bar() {
+  return foo();
+}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -650,6 +650,7 @@
   Args.hasArg(OPT_mincremental_linker_compatible);
   Opts.PIECopyRelocations =
   Args.hasArg(OPT_mpie_copy_relocations);
+  Opts.NoPLT = Args.hasArg(OPT_fno_plt);
   Opts.OmitLeafFramePointer = Args.hasArg(OPT_momit_leaf_frame_pointer);
   Opts.SaveTempLabels = Args.hasArg(OPT_msave_temp_labels);
   Opts.NoDwarfDirectoryAsm = Args.hasArg(OPT_fno_dwarf_directory_asm);
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3386,6 +3386,10 @@
 CmdArgs.push_back("-mpie-copy-relocations");
   }
 
+  if (Args.hasFlag(options::OPT_fno_plt, options::OPT_fplt, false)) {
+CmdArgs.push_back("-fno-plt");
+  }
+
   // -fhos

1 2 >

1 - 100 of 165 matches

Mail list logo