Re: [clang-tools-extra] r350847 - [clangd] Introduce loading of shards within auto-index

2019-01-13 Thread Amara Emerson via cfe-commits
Hi Kadir,

It seems this commit started causing failures on builds in green dragon, 
starting from: 
http://green.lab.llvm.org/green/job/clang-stage1-configure-RA/52811/ 


 Can you please take a look, and revert if the fix isn’t straight forward.

Thanks,
Amara

> On Jan 10, 2019, at 9:03 AM, Kadir Cetinkaya via cfe-commits 
>  wrote:
> 
> Author: kadircet
> Date: Thu Jan 10 09:03:04 2019
> New Revision: 350847
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=350847=rev
> Log:
> [clangd] Introduce loading of shards within auto-index
> 
> Summary:
> Whenever a change happens on a CDB, load shards associated with that
> CDB before issuing re-index actions.
> 
> Reviewers: ilya-biryukov
> 
> Reviewed By: ilya-biryukov
> 
> Subscribers: ioeric, MaskRay, jkorous, arphaman, cfe-commits
> 
> Differential Revision: https://reviews.llvm.org/D55224
> 
> Modified:
>clang-tools-extra/trunk/clangd/index/Background.cpp
>clang-tools-extra/trunk/clangd/index/Background.h
>clang-tools-extra/trunk/test/clangd/background-index.test
>clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
> 
> Modified: clang-tools-extra/trunk/clangd/index/Background.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Background.cpp?rev=350847=350846=350847=diff
> ==
> --- clang-tools-extra/trunk/clangd/index/Background.cpp (original)
> +++ clang-tools-extra/trunk/clangd/index/Background.cpp Thu Jan 10 09:03:04 
> 2019
> @@ -115,6 +115,19 @@ createFileFilter(const llvm::StringMap   };
> }
> 
> +// We cannot use vfs->makeAbsolute because Cmd.FileName is either absolute or
> +// relative to Cmd.Directory, which might not be the same as current working
> +// directory.
> +llvm::SmallString<128> getAbsolutePath(const tooling::CompileCommand ) {
> +  llvm::SmallString<128> AbsolutePath;
> +  if (llvm::sys::path::is_absolute(Cmd.Filename)) {
> +AbsolutePath = Cmd.Filename;
> +  } else {
> +AbsolutePath = Cmd.Directory;
> +llvm::sys::path::append(AbsolutePath, Cmd.Filename);
> +  }
> +  return AbsolutePath;
> +}
> } // namespace
> 
> BackgroundIndex::BackgroundIndex(
> @@ -204,40 +217,33 @@ void BackgroundIndex::enqueue(const std:
>   [this, ChangedFiles] {
> trace::Span Tracer("BackgroundIndexEnqueue");
> // We're doing this asynchronously, because we'll read shards here 
> too.
> -// FIXME: read shards here too.
> -
> log("Enqueueing {0} commands for indexing", ChangedFiles.size());
> SPAN_ATTACH(Tracer, "files", int64_t(ChangedFiles.size()));
> 
> -// We shuffle the files because processing them in a random order 
> should
> -// quickly give us good coverage of headers in the project.
> -std::vector Permutation(ChangedFiles.size());
> -std::iota(Permutation.begin(), Permutation.end(), 0);
> -std::mt19937 Generator(std::random_device{}());
> -std::shuffle(Permutation.begin(), Permutation.end(), Generator);
> -
> -for (const unsigned I : Permutation)
> -  enqueue(ChangedFiles[I]);
> +auto NeedsReIndexing = loadShards(std::move(ChangedFiles));
> +// Run indexing for files that need to be updated.
> +std::shuffle(NeedsReIndexing.begin(), NeedsReIndexing.end(),
> + std::mt19937(std::random_device{}()));
> +for (auto  : NeedsReIndexing)
> +  enqueue(std::move(Elem.first), Elem.second);
>   },
>   ThreadPriority::Normal);
> }
> 
> -void BackgroundIndex::enqueue(const std::string ) {
> -  ProjectInfo Project;
> -  if (auto Cmd = CDB.getCompileCommand(File, )) {
> -auto *Storage = IndexStorageFactory(Project.SourceRoot);
> -// Set priority to low, since background indexing is a long running
> -// task we do not want to eat up cpu when there are any other high
> -// priority threads.
> -enqueueTask(Bind(
> -[this, File, Storage](tooling::CompileCommand Cmd) {
> -  Cmd.CommandLine.push_back("-resource-dir=" + 
> ResourceDir);
> -  if (auto Error = index(std::move(Cmd), Storage))
> -log("Indexing {0} failed: {1}", File, 
> std::move(Error));
> -},
> -std::move(*Cmd)),
> -ThreadPriority::Low);
> -  }
> +void BackgroundIndex::enqueue(tooling::CompileCommand Cmd,
> +  BackgroundIndexStorage *Storage) {
> +  enqueueTask(Bind(
> +  [this, Storage](tooling::CompileCommand Cmd) {
> +Cmd.CommandLine.push_back("-resource-dir=" + 
> ResourceDir);
> +// We can't use llvm::StringRef here since we are going 
> to
> +// move from Cmd during the call below.
> +const std::string FileName = Cmd.Filename;
> 

Re: r350856 - Split -Wdelete-non-virtual-dtor into two groups

2019-01-13 Thread David Blaikie via cfe-commits
Might be handy to summarize the changes from the previous reverted version
of this patch (& mention the original commit revision and revert revision)
- in the commit message is ideal, but in a reply to the commit after the
fact will do in a pinch

On Fri, Jan 11, 2019 at 4:06 AM Erik Pilkington via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: epilk
> Date: Thu Jan 10 10:03:07 2019
> New Revision: 350856
>
> URL: http://llvm.org/viewvc/llvm-project?rev=350856=rev
> Log:
> Split -Wdelete-non-virtual-dtor into two groups
>
> This group controls two diagnostics: deleting an abstract class with
> a non-virtual dtor, which is a guaranteed crash, and deleting a
> non-abstract polymorphic class with a non-virtual dtor, which is just
> suspicious.
>
> rdar://40380564
>
> Differential revision: https://reviews.llvm.org/D56405
>
> Added:
> cfe/trunk/test/SemaCXX/delete-non-virtual-dtor.cpp
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticGroups.td
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=350856=350855=350856=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Thu Jan 10 10:03:07
> 2019
> @@ -104,7 +104,11 @@ def UndefinedFuncTemplate : DiagGroup<"u
>  def MissingNoEscape : DiagGroup<"missing-noescape">;
>
>  def DeleteIncomplete : DiagGroup<"delete-incomplete">;
> -def DeleteNonVirtualDtor : DiagGroup<"delete-non-virtual-dtor">;
> +def DeleteNonAbstractNonVirtualDtor :
> DiagGroup<"delete-non-abstract-non-virtual-dtor">;
> +def DeleteAbstractNonVirtualDtor :
> DiagGroup<"delete-abstract-non-virtual-dtor">;
> +def DeleteNonVirtualDtor : DiagGroup<"delete-non-virtual-dtor",
> + [DeleteNonAbstractNonVirtualDtor,
> +  DeleteAbstractNonVirtualDtor]>;
>  def AbstractFinalClass : DiagGroup<"abstract-final-class">;
>
>  def CXX11CompatDeprecatedWritableStr :
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=350856=350855=350856=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Jan 10
> 10:03:07 2019
> @@ -6455,12 +6455,12 @@ def warn_non_virtual_dtor : Warning<
>  def warn_delete_non_virtual_dtor : Warning<
>"%select{delete|destructor}0 called on non-final %1 that has "
>"virtual functions but non-virtual destructor">,
> -  InGroup, DefaultIgnore, ShowInSystemHeader;
> +  InGroup, DefaultIgnore,
> ShowInSystemHeader;
>  def note_delete_non_virtual : Note<
>"qualify call to silence this warning">;
>  def warn_delete_abstract_non_virtual_dtor : Warning<
>"%select{delete|destructor}0 called on %1 that is abstract but has "
> -  "non-virtual destructor">, InGroup,
> ShowInSystemHeader;
> +  "non-virtual destructor">, InGroup,
> ShowInSystemHeader;
>  def warn_overloaded_virtual : Warning<
>"%q0 hides overloaded virtual %select{function|functions}1">,
>InGroup, DefaultIgnore;
>
> Added: cfe/trunk/test/SemaCXX/delete-non-virtual-dtor.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/delete-non-virtual-dtor.cpp?rev=350856=auto
>
> ==
> --- cfe/trunk/test/SemaCXX/delete-non-virtual-dtor.cpp (added)
> +++ cfe/trunk/test/SemaCXX/delete-non-virtual-dtor.cpp Thu Jan 10 10:03:07
> 2019
> @@ -0,0 +1,30 @@
> +// RUN: %clang_cc1 %s -verify -DDIAG1
> +// RUN: %clang_cc1 %s -verify -DDIAG1 -DDIAG2 -Wdelete-non-virtual-dtor
> +// RUN: %clang_cc1 %s -verify -DDIAG1 -Wmost
> -Wno-delete-non-abstract-non-virtual-dtor
> +// RUN: %clang_cc1 %s -verify -DDIAG2 -Wmost
> -Wno-delete-abstract-non-virtual-dtor
> +// RUN: %clang_cc1 %s -verify -Wmost
> -Wno-delete-non-virtual-dtor
> +
> +#ifndef DIAG1
> +#ifndef DIAG2
> +// expected-no-diagnostics
> +#endif
> +#endif
> +
> +struct S1 {
> +  ~S1() {}
> +  virtual void abs() = 0;
> +};
> +
> +void f1(S1 *s1) { delete s1; }
> +#ifdef DIAG1
> +// expected-warning@-2 {{delete called on 'S1' that is abstract but has
> non-virtual destructor}}
> +#endif
> +
> +struct S2 {
> +  ~S2() {}
> +  virtual void real() {}
> +};
> +void f2(S2 *s2) { delete s2; }
> +#ifdef DIAG2
> +// expected-warning@-2 {{delete called on non-final 'S2' that has
> virtual functions but non-virtual destructor}}
> +#endif
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> 

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in LLVM

2019-01-13 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments.



Comment at: lib/Sema/SemaStmtAsm.cpp:470
+if (NS->isGCCAsmGoto() &&
+Exprs[ConstraintIdx]->getStmtClass() == Stmt::AddrLabelExprClass)
+  break;

jyu2 wrote:
> jyu2 wrote:
> > efriedma wrote:
> > > jyu2 wrote:
> > > > efriedma wrote:
> > > > > jyu2 wrote:
> > > > > > efriedma wrote:
> > > > > > > This looks suspicious; an AddrLabelExpr could be an input or 
> > > > > > > output, e.g. `"r"(&)`.
> > > > > > Syntax for asm goto:
> > > > > >  Syntax:
> > > > > >asm [volatile] goto ( AssemblerTemplate
> > > > > >:
> > > > > >: InputOperands
> > > > > >: Clobbers
> > > > > >: GotoLabels)
> > > > > > 
> > > > > >  Only input is allowed.  Output is not allowed
> > > > > > 
> > > > > That doesn't really address my point here... ignore the "or output" 
> > > > > part of the comment.
> > > > Sorry did not realize that.  Thank you so much for catching that.  Need 
> > > > to add other condition "ConstraintIdx > NS->getNumInputs() - 1", change 
> > > > to :
> > > > 
> > > > if (NS->isGCCAsmGoto() && ConstraintIdx > NS->getNumInputs() - 1 &&
> > > > Exprs[ConstraintIdx]->getStmtClass() == 
> > > > Stmt::AddrLabelExprClass)
> > > >   break;
> > > > 
> > > > Is this ok with you?  Thanks
> > > That's the right idea. But I still see a few issues at that point:
> > > 
> > > 1. The AddrLabelExprClass check is redundant.
> > > 2. "NS->getNumInputs() - 1" can overflow; probably should use 
> > > "ConstraintIdx >= NS->getNumInputs()".
> > > 3. "break" exits the loop completely (so it skips validating all 
> > > constraints written after the label).
> > > 4. The code needs to verify that the user correctly specified the "l" 
> > > constraint modifier.
> > Sorry not done yet.  
> For you comment 4:
> 
> The code needs to verify that the user correctly specified the "l" constraint 
> modifier.  We already emit error like following?
> 
> Do you mean, we need more checking here?  Thanks. 
> 
> n.c:4:35: error: unknown symbolic operand name in inline assembly string
>   asm goto ("frob %%r5, %1; jc %l[error]; mov (%2), %%r5"
> ~~^~~
> n.c:8:15: error: use of undeclared label 'error1'
> : error1);
> 
> Test is:
> int frob(int x)
> {
>   int y;
>   asm goto ("frob %%r5, %1; jc %l[error]; mov (%2), %%r5"
> : /* No outputs. */
> : "r"(x), "r"()
> : "memory"
> : error1);
>   return y;
> error:
>   return -1;
> }
> 
> 
I mean, there needs to be a diagnostic for the following:

```
asm goto ("jne %h0"x);
```

On a related note, there should also be a diagnostic for the following 
somewhere:

```
asm ("jne %l0"::"r"(0));
```


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

https://reviews.llvm.org/D56571



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


[PATCH] D55394: Re-order type param children of ObjC nodes

2019-01-13 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added inline comments.



Comment at: test/AST/ast-dump-decl.m:90
 // CHECK-NEXT:   -ObjCProtocol {{.+}} 'P'
+// CHECK-NEXT:   -ObjCTypeParamDecl {{.+}}  col:33 T 'id':'id'
 

steveire wrote:
> aaron.ballman wrote:
> > It seems strange to me to print out the type parameter after the superclass 
> > information given the source order. My understanding of the AST dumping 
> > order is that we try to keep the order of nodes in source order whenever 
> > possible.
> That is not really a possible thing to try to do, because the AST dump 
> doesn't relate to a single language. It should be seen as language 
> independent.
> 
> The principle I'm follow is that nodes dump themselves in entirety before 
> starting to dump their child nodes. That is a principle already followed by 
> most nodes. Changing this seems to be low cost, low impact and high benefit 
> to the code.
>That is not really a possible thing to try to do, because the AST dump doesn't 
>relate to a single language. It should be seen as language independent.

Is this particular aspect different between the different source languages 
Clang supports? (could you give examples?)


Repository:
  rC Clang

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

https://reviews.llvm.org/D55394



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


r351029 - [X86] Remove mask parameter from vpshufbitqmb intrinsics. Change result to a vXi1 vector.

2019-01-13 Thread Craig Topper via cfe-commits
Author: ctopper
Date: Sun Jan 13 16:03:55 2019
New Revision: 351029

URL: http://llvm.org/viewvc/llvm-project?rev=351029=rev
Log:
[X86] Remove mask parameter from vpshufbitqmb intrinsics. Change result to a 
vXi1 vector.

We'll do the scalar<->vXi1 conversions with bitcasts in IR.

Fixes PR40258

Modified:
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/test/CodeGen/avx512bitalg-builtins.c
cfe/trunk/test/CodeGen/avx512vlbitalg-builtins.c

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=351029=351028=351029=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Sun Jan 13 16:03:55 2019
@@ -11152,6 +11152,31 @@ Value *CodeGenFunction::EmitX86BuiltinEx
 return EmitX86MaskedCompareResult(*this, Fpclass, NumElts, MaskIn);
   }
 
+  case X86::BI__builtin_ia32_vpshufbitqmb128_mask:
+  case X86::BI__builtin_ia32_vpshufbitqmb256_mask:
+  case X86::BI__builtin_ia32_vpshufbitqmb512_mask: {
+unsigned NumElts = Ops[0]->getType()->getVectorNumElements();
+Value *MaskIn = Ops[2];
+Ops.erase([2]);
+
+Intrinsic::ID ID;
+switch (BuiltinID) {
+default: llvm_unreachable("Unsupported intrinsic!");
+case X86::BI__builtin_ia32_vpshufbitqmb128_mask:
+  ID = Intrinsic::x86_avx512_vpshufbitqmb_128;
+  break;
+case X86::BI__builtin_ia32_vpshufbitqmb256_mask:
+  ID = Intrinsic::x86_avx512_vpshufbitqmb_256;
+  break;
+case X86::BI__builtin_ia32_vpshufbitqmb512_mask:
+  ID = Intrinsic::x86_avx512_vpshufbitqmb_512;
+  break;
+}
+
+Value *Fpclass = Builder.CreateCall(CGM.getIntrinsic(ID), Ops);
+return EmitX86MaskedCompareResult(*this, Fpclass, NumElts, MaskIn);
+  }
+
   // packed comparison intrinsics
   case X86::BI__builtin_ia32_cmpeqps:
   case X86::BI__builtin_ia32_cmpeqpd:

Modified: cfe/trunk/test/CodeGen/avx512bitalg-builtins.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/avx512bitalg-builtins.c?rev=351029=351028=351029=diff
==
--- cfe/trunk/test/CodeGen/avx512bitalg-builtins.c (original)
+++ cfe/trunk/test/CodeGen/avx512bitalg-builtins.c Sun Jan 13 16:03:55 2019
@@ -42,13 +42,14 @@ __m512i test_mm512_maskz_popcnt_epi8(__m
 
 __mmask64 test_mm512_mask_bitshuffle_epi64_mask(__mmask64 __U, __m512i __A, 
__m512i __B) {
   // CHECK-LABEL: @test_mm512_mask_bitshuffle_epi64_mask
-  // CHECK: @llvm.x86.avx512.mask.vpshufbitqmb.512
+  // CHECK: @llvm.x86.avx512.vpshufbitqmb.512
+  // CHECK: and <64 x i1> %{{.*}}, %{{.*}}
   return _mm512_mask_bitshuffle_epi64_mask(__U, __A, __B);
 }
 
 __mmask64 test_mm512_bitshuffle_epi64_mask(__m512i __A, __m512i __B) {
   // CHECK-LABEL: @test_mm512_bitshuffle_epi64_mask
-  // CHECK: @llvm.x86.avx512.mask.vpshufbitqmb.512
+  // CHECK: @llvm.x86.avx512.vpshufbitqmb.512
   return _mm512_bitshuffle_epi64_mask(__A, __B);
 }
 

Modified: cfe/trunk/test/CodeGen/avx512vlbitalg-builtins.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/avx512vlbitalg-builtins.c?rev=351029=351028=351029=diff
==
--- cfe/trunk/test/CodeGen/avx512vlbitalg-builtins.c (original)
+++ cfe/trunk/test/CodeGen/avx512vlbitalg-builtins.c Sun Jan 13 16:03:55 2019
@@ -80,25 +80,27 @@ __m128i test_mm_maskz_popcnt_epi8(__mmas
 
 __mmask32 test_mm256_mask_bitshuffle_epi64_mask(__mmask32 __U, __m256i __A, 
__m256i __B) {
   // CHECK-LABEL: @test_mm256_mask_bitshuffle_epi64_mask
-  // CHECK: @llvm.x86.avx512.mask.vpshufbitqmb.256
+  // CHECK: @llvm.x86.avx512.vpshufbitqmb.256
+  // CHECK: and <32 x i1> %{{.*}}, %{{.*}}
   return _mm256_mask_bitshuffle_epi64_mask(__U, __A, __B);
 }
 
 __mmask32 test_mm256_bitshuffle_epi64_mask(__m256i __A, __m256i __B) {
   // CHECK-LABEL: @test_mm256_bitshuffle_epi64_mask
-  // CHECK: @llvm.x86.avx512.mask.vpshufbitqmb.256
+  // CHECK: @llvm.x86.avx512.vpshufbitqmb.256
   return _mm256_bitshuffle_epi64_mask(__A, __B);
 }
 
 __mmask16 test_mm_mask_bitshuffle_epi64_mask(__mmask16 __U, __m128i __A, 
__m128i __B) {
   // CHECK-LABEL: @test_mm_mask_bitshuffle_epi64_mask
-  // CHECK: @llvm.x86.avx512.mask.vpshufbitqmb.128
+  // CHECK: @llvm.x86.avx512.vpshufbitqmb.128
+  // CHECK: and <16 x i1> %{{.*}}, %{{.*}}
   return _mm_mask_bitshuffle_epi64_mask(__U, __A, __B);
 }
 
 __mmask16 test_mm_bitshuffle_epi64_mask(__m128i __A, __m128i __B) {
   // CHECK-LABEL: @test_mm_bitshuffle_epi64_mask
-  // CHECK: @llvm.x86.avx512.mask.vpshufbitqmb.128
+  // CHECK: @llvm.x86.avx512.vpshufbitqmb.128
   return _mm_bitshuffle_epi64_mask(__A, __B);
 }
 


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


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

2019-01-13 Thread Aleksei Sidorin via Phabricator via cfe-commits
a_sidorin added a comment.

Hi Rafael,
The change  looks mostly fine but I have some comments inline.




Comment at: lib/AST/ASTImporter.cpp:3243
+
+  if (R) {
+CXXDestructorDecl *ToDtor = cast(*R);

It's better to move this code to VisitFunctionDecl to keep all related stuff 
together.



Comment at: lib/AST/ASTImporter.cpp:3246
+
+auto Imp = importSeq(const_cast(D->getOperatorDelete()),
+ D->getOperatorDeleteThisArg());

Moving this code to VisitFunctionDecl will also allow us not to create a dtor 
if this import fails.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56651



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


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

2019-01-13 Thread Petr Hosek via Phabricator via cfe-commits
phosek created this revision.
phosek added reviewers: mcgrathr, jakehehrlich, juliehockett, leonardchan.
Herald added subscribers: cfe-commits, mgorny.

This reorders options between the first and second stage builds to make
them better lined up. The change also re-enables tests for first stage
which is useful e.g. for cross-compiling when we cannot run tests for
second stage directly (i.e. without emulation).


Repository:
  rC Clang

https://reviews.llvm.org/D56652

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake
  clang/cmake/caches/Fuchsia.cmake

Index: clang/cmake/caches/Fuchsia.cmake
===
--- clang/cmake/caches/Fuchsia.cmake
+++ clang/cmake/caches/Fuchsia.cmake
@@ -4,26 +4,12 @@
 
 set(PACKAGE_VENDOR Fuchsia CACHE STRING "")
 
-set(LLVM_INCLUDE_EXAMPLES OFF CACHE BOOL "")
-set(LLVM_INCLUDE_TESTS OFF CACHE BOOL "")
-set(LLVM_INCLUDE_DOCS OFF CACHE BOOL "")
 set(LLVM_ENABLE_BACKTRACES OFF CACHE BOOL "")
 set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR ON CACHE BOOL "")
 set(LLVM_ENABLE_TERMINFO OFF CACHE BOOL "")
 set(LLVM_ENABLE_ZLIB OFF CACHE BOOL "")
-set(CLANG_INCLUDE_TESTS OFF CACHE BOOL "")
-set(CLANG_PLUGIN_SUPPORT OFF CACHE BOOL "")
-
-set(ENABLE_LINKER_BUILD_ID ON CACHE BOOL "")
-set(ENABLE_X86_RELAX_RELOCATIONS ON CACHE BOOL "")
-
-set(LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
-set(CMAKE_BUILD_TYPE Release CACHE STRING "")
-
-set(BOOTSTRAP_LLVM_ENABLE_LTO ON CACHE BOOL "")
-if(NOT APPLE)
-  set(BOOTSTRAP_LLVM_ENABLE_LLD ON CACHE BOOL "")
-endif()
+set(LLVM_INCLUDE_EXAMPLES OFF CACHE BOOL "")
+set(LLVM_INCLUDE_DOCS OFF CACHE BOOL "")
 
 if(NOT APPLE)
   set(CLANG_DEFAULT_LINKER lld CACHE STRING "")
@@ -31,6 +17,13 @@
 endif()
 set(CLANG_DEFAULT_CXX_STDLIB libc++ CACHE STRING "")
 set(CLANG_DEFAULT_RTLIB compiler-rt CACHE STRING "")
+set(CLANG_PLUGIN_SUPPORT OFF CACHE BOOL "")
+
+set(ENABLE_LINKER_BUILD_ID ON CACHE BOOL "")
+set(ENABLE_X86_RELAX_RELOCATIONS ON CACHE BOOL "")
+
+set(LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
+set(CMAKE_BUILD_TYPE Release CACHE STRING "")
 
 if(APPLE)
   set(COMPILER_RT_ENABLE_IOS OFF CACHE BOOL "")
@@ -81,6 +74,11 @@
   endif()
 endif()
 
+set(BOOTSTRAP_LLVM_ENABLE_LTO ON CACHE BOOL "")
+if(NOT APPLE)
+  set(BOOTSTRAP_LLVM_ENABLE_LLD ON CACHE BOOL "")
+endif()
+
 set(CLANG_BOOTSTRAP_TARGETS
   check-all
   check-llvm
Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -1,32 +1,33 @@
-# This file sets up a CMakeCache for the second stage of a Fuchsia toolchain
-# build.
+# This file sets up a CMakeCache for the second stage of a Fuchsia toolchain build.
 
 set(LLVM_TARGETS_TO_BUILD X86;ARM;AArch64 CACHE STRING "")
 
 set(PACKAGE_VENDOR Fuchsia CACHE STRING "")
 
-set(LLVM_INCLUDE_EXAMPLES OFF CACHE BOOL "")
-set(LLVM_INCLUDE_DOCS OFF CACHE BOOL "")
 set(LLVM_ENABLE_BACKTRACES OFF CACHE BOOL "")
+if(NOT APPLE)
+  set(LLVM_ENABLE_LLD ON CACHE BOOL "")
+endif()
+set(LLVM_ENABLE_LTO ON CACHE BOOL "")
+set(LLVM_ENABLE_MODULES ON CACHE BOOL "")
 set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR ON CACHE BOOL "")
 set(LLVM_ENABLE_TERMINFO OFF CACHE BOOL "")
 set(LLVM_ENABLE_ZLIB ON CACHE BOOL "")
 set(LLVM_EXTERNALIZE_DEBUGINFO ON CACHE BOOL "")
-set(CLANG_PLUGIN_SUPPORT OFF CACHE BOOL "")
-
-set(ENABLE_LINKER_BUILD_ID ON CACHE BOOL "")
-set(ENABLE_X86_RELAX_RELOCATIONS ON CACHE BOOL "")
+set(LLVM_INCLUDE_EXAMPLES OFF CACHE BOOL "")
+set(LLVM_INCLUDE_DOCS OFF CACHE BOOL "")
 
-set(LLVM_ENABLE_LTO ON CACHE BOOL "")
 if(NOT APPLE)
-  set(LLVM_ENABLE_LLD ON CACHE BOOL "")
   set(CLANG_DEFAULT_LINKER lld CACHE STRING "")
   set(CLANG_DEFAULT_OBJCOPY llvm-objcopy CACHE STRING "")
 endif()
 set(CLANG_DEFAULT_CXX_STDLIB libc++ CACHE STRING "")
 set(CLANG_DEFAULT_RTLIB compiler-rt CACHE STRING "")
+set(CLANG_PLUGIN_SUPPORT OFF CACHE BOOL "")
+
+set(ENABLE_LINKER_BUILD_ID ON CACHE BOOL "")
+set(ENABLE_X86_RELAX_RELOCATIONS ON CACHE BOOL "")
 
-set(LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
 set(CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING "")
 set(CMAKE_C_FLAGS_RELWITHDEBINFO "-O3 -gline-tables-only -DNDEBUG" CACHE STRING "")
 set(CMAKE_CXX_FLAGS_RELWITHDEBINFO "-O3 -gline-tables-only -DNDEBUG" CACHE STRING "")
@@ -34,6 +35,10 @@
 if(APPLE)
   list(APPEND BUILTIN_TARGETS "default")
   list(APPEND RUNTIME_TARGETS "default")
+
+  set(COMPILER_RT_ENABLE_IOS OFF CACHE BOOL "")
+  set(COMPILER_RT_ENABLE_TVOS OFF CACHE BOOL "")
+  set(COMPILER_RT_ENABLE_WATCHOS OFF CACHE BOOL "")
 endif()
 
 foreach(target aarch64-linux-gnu;armv7-linux-gnueabihf;i386-linux-gnu;x86_64-linux-gnu)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2019-01-13 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment.

Didn't really upstream any non-trivial ASTImporter patches yet, so please point 
out any style errors.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56651



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


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

2019-01-13 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor created this revision.
teemperor added reviewers: shafik, martong, a_sidorin.
Herald added subscribers: cfe-commits, rnkovacs.
Herald added a reviewer: a.sidorin.

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


Repository:
  rC Clang

https://reviews.llvm.org/D56651

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


Index: test/Import/destructor/test.cpp
===
--- /dev/null
+++ test/Import/destructor/test.cpp
@@ -0,0 +1,10 @@
+// RUN: clang-import-test -dump-ast -import %S/Inputs/F.cpp -expression %s
+
+// Triggers the deserialization of B's destructor.
+B b1;
+
+// CHECK: CXXDestructorDecl
+
+// CHECK-NEXT: ~B 'void () noexcept' virtual
+// CHECK-SAME: 'void () noexcept'
+// CHECK-SAME: virtual
Index: test/Import/destructor/Inputs/F.cpp
===
--- /dev/null
+++ test/Import/destructor/Inputs/F.cpp
@@ -0,0 +1,3 @@
+struct B {
+  virtual ~B() {}
+};
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -3238,7 +3238,25 @@
 }
 
 ExpectedDecl ASTNodeImporter::VisitCXXDestructorDecl(CXXDestructorDecl *D) {
-  return VisitCXXMethodDecl(D);
+  ExpectedDecl R = VisitCXXMethodDecl(D);
+
+  if (R) {
+CXXDestructorDecl *ToDtor = cast(*R);
+
+auto Imp = importSeq(const_cast(D->getOperatorDelete()),
+ D->getOperatorDeleteThisArg());
+
+if (!Imp)
+  return Imp.takeError();
+
+FunctionDecl *ToOperatorDelete;
+Expr *ToThisArg;
+std::tie(ToOperatorDelete, ToThisArg) = *Imp;
+
+ToDtor->setOperatorDelete(ToOperatorDelete, ToThisArg);
+  }
+
+  return R;
 }
 
 ExpectedDecl ASTNodeImporter::VisitCXXConversionDecl(CXXConversionDecl *D) {


Index: test/Import/destructor/test.cpp
===
--- /dev/null
+++ test/Import/destructor/test.cpp
@@ -0,0 +1,10 @@
+// RUN: clang-import-test -dump-ast -import %S/Inputs/F.cpp -expression %s
+
+// Triggers the deserialization of B's destructor.
+B b1;
+
+// CHECK: CXXDestructorDecl
+
+// CHECK-NEXT: ~B 'void () noexcept' virtual
+// CHECK-SAME: 'void () noexcept'
+// CHECK-SAME: virtual
Index: test/Import/destructor/Inputs/F.cpp
===
--- /dev/null
+++ test/Import/destructor/Inputs/F.cpp
@@ -0,0 +1,3 @@
+struct B {
+  virtual ~B() {}
+};
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -3238,7 +3238,25 @@
 }
 
 ExpectedDecl ASTNodeImporter::VisitCXXDestructorDecl(CXXDestructorDecl *D) {
-  return VisitCXXMethodDecl(D);
+  ExpectedDecl R = VisitCXXMethodDecl(D);
+
+  if (R) {
+CXXDestructorDecl *ToDtor = cast(*R);
+
+auto Imp = importSeq(const_cast(D->getOperatorDelete()),
+ D->getOperatorDeleteThisArg());
+
+if (!Imp)
+  return Imp.takeError();
+
+FunctionDecl *ToOperatorDelete;
+Expr *ToThisArg;
+std::tie(ToOperatorDelete, ToThisArg) = *Imp;
+
+ToDtor->setOperatorDelete(ToOperatorDelete, ToThisArg);
+  }
+
+  return R;
 }
 
 ExpectedDecl ASTNodeImporter::VisitCXXConversionDecl(CXXConversionDecl *D) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

2019-01-13 Thread Michał Górny via Phabricator via cfe-commits
mgorny updated this revision to Diff 181484.
mgorny marked 7 inline comments as done.
mgorny set the repository for this revision to rLLD LLVM Linker.
mgorny added a comment.
Herald added a subscriber: fedor.sergeev.

Split target logic into D56650 , switched to 
using target to determine which paths to apply. While at it, copied the code 
from clang since it now can match exactly.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D56215

Files:
  ELF/Driver.cpp
  ELF/Driver.h


Index: ELF/Driver.h
===
--- ELF/Driver.h
+++ ELF/Driver.h
@@ -33,6 +33,7 @@
 private:
   void setTargetTriple(StringRef argv0, llvm::opt::InputArgList );
   void readConfigs(llvm::opt::InputArgList );
+  void appendDefaultSearchPaths();
   void createFiles(llvm::opt::InputArgList );
   void inferMachineType();
   template  void link(llvm::opt::InputArgList );
Index: ELF/Driver.cpp
===
--- ELF/Driver.cpp
+++ ELF/Driver.cpp
@@ -366,6 +366,56 @@
   error("unknown -z value: " + StringRef(Arg->getValue()));
 }
 
+void LinkerDriver::appendDefaultSearchPaths() {
+  if (Config->TargetTriple.isOSNetBSD()) {
+// NetBSD driver relies on the linker knowing the default search paths.
+// Please keep this in sync with clang/lib/Driver/ToolChains/NetBSD.cpp
+// (NetBSD::NetBSD constructor)
+switch (Config->TargetTriple.getArch()) {
+case llvm::Triple::x86:
+  Config->SearchPaths.push_back("=/usr/lib/i386");
+  break;
+case llvm::Triple::arm:
+case llvm::Triple::armeb:
+case llvm::Triple::thumb:
+case llvm::Triple::thumbeb:
+  switch (Config->TargetTriple.getEnvironment()) {
+  case llvm::Triple::EABI:
+  case llvm::Triple::GNUEABI:
+Config->SearchPaths.push_back("=/usr/lib/eabi");
+break;
+  case llvm::Triple::EABIHF:
+  case llvm::Triple::GNUEABIHF:
+Config->SearchPaths.push_back("=/usr/lib/eabihf");
+break;
+  default:
+Config->SearchPaths.push_back("=/usr/lib/oabi");
+break;
+  }
+  break;
+#if 0 // TODO
+case llvm::Triple::mips64:
+case llvm::Triple::mips64el:
+  if (tools::mips::hasMipsAbiArg(Args, "o32"))
+Config->SearchPaths.push_back("=/usr/lib/o32");
+  else if (tools::mips::hasMipsAbiArg(Args, "64"))
+Config->SearchPaths.push_back("=/usr/lib/64");
+  break;
+#endif
+case llvm::Triple::ppc:
+  Config->SearchPaths.push_back("=/usr/lib/powerpc");
+  break;
+case llvm::Triple::sparc:
+  Config->SearchPaths.push_back("=/usr/lib/sparc");
+  break;
+default:
+  break;
+}
+
+Config->SearchPaths.push_back("=/usr/lib");
+  }
+}
+
 void LinkerDriver::main(ArrayRef ArgsArr) {
   ELFOptTable Parser;
   opt::InputArgList Args = Parser.parse(ArgsArr.slice(1));
@@ -397,6 +447,7 @@
   setTargetTriple(ArgsArr[0], Args);
   readConfigs(Args);
   checkZOptions(Args);
+  appendDefaultSearchPaths();
 
   // Handle -v or -version.
   //


Index: ELF/Driver.h
===
--- ELF/Driver.h
+++ ELF/Driver.h
@@ -33,6 +33,7 @@
 private:
   void setTargetTriple(StringRef argv0, llvm::opt::InputArgList );
   void readConfigs(llvm::opt::InputArgList );
+  void appendDefaultSearchPaths();
   void createFiles(llvm::opt::InputArgList );
   void inferMachineType();
   template  void link(llvm::opt::InputArgList );
Index: ELF/Driver.cpp
===
--- ELF/Driver.cpp
+++ ELF/Driver.cpp
@@ -366,6 +366,56 @@
   error("unknown -z value: " + StringRef(Arg->getValue()));
 }
 
+void LinkerDriver::appendDefaultSearchPaths() {
+  if (Config->TargetTriple.isOSNetBSD()) {
+// NetBSD driver relies on the linker knowing the default search paths.
+// Please keep this in sync with clang/lib/Driver/ToolChains/NetBSD.cpp
+// (NetBSD::NetBSD constructor)
+switch (Config->TargetTriple.getArch()) {
+case llvm::Triple::x86:
+  Config->SearchPaths.push_back("=/usr/lib/i386");
+  break;
+case llvm::Triple::arm:
+case llvm::Triple::armeb:
+case llvm::Triple::thumb:
+case llvm::Triple::thumbeb:
+  switch (Config->TargetTriple.getEnvironment()) {
+  case llvm::Triple::EABI:
+  case llvm::Triple::GNUEABI:
+Config->SearchPaths.push_back("=/usr/lib/eabi");
+break;
+  case llvm::Triple::EABIHF:
+  case llvm::Triple::GNUEABIHF:
+Config->SearchPaths.push_back("=/usr/lib/eabihf");
+break;
+  default:
+Config->SearchPaths.push_back("=/usr/lib/oabi");
+break;
+  }
+  break;
+#if 0 // TODO
+case llvm::Triple::mips64:
+case llvm::Triple::mips64el:
+  if (tools::mips::hasMipsAbiArg(Args, "o32"))
+

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

2019-01-13 Thread Michał Górny via Phabricator via cfe-commits
mgorny added inline comments.



Comment at: ELF/Driver.cpp:770
+  // Start with a default initial triple
+  Config->TargetTriple = llvm::Triple(getDefaultTargetTriple());
+

krytarowski wrote:
> krytarowski wrote:
> > arichardson wrote:
> > > arichardson wrote:
> > > > If I invoke an unprefixed ld.lld on NetBSD but want to target a 
> > > > different operating system, this will cause all the NetBSD defaults to 
> > > > apply to that binary and will possibly cause it to crash at runtime.
> > > > 
> > > > I think any config changes based on a triple would need to use an 
> > > > explicit --target= flag instead. As @ruiu says, LLD's behaviour should 
> > > > not change depending on the host OS/default LLVM triple. Given the same 
> > > > input files and command line options the resulting binary should be 
> > > > identical on any host.
> > > There needs to be a way to override the target triple that is not 
> > > creating prefixed a symlink to ld.lld. Otherwise I can't use NetBSD 
> > > ld.lld to build a non-NetBSD target without giving a value for every 
> > > config option that lld supports.
> > > 
> > > I think there should be a command line option to override the triple 
> > > (e.g. --triple= or --target=).
> > > Also how will the default this interact with input files that have the 
> > > OSABI field set? I feel like the options based on the target OSABI should 
> > > be used instead of the default triple.
> > OSABI field is not reliable way to detect OS/ABI. Everybody except FreeBSD 
> > sets UNIX SystemV.
> Actually there is a FreeBSD specific hack to detect emulation name, and it 
> has suffix `fbsd`.. if it is detected it sets FreeBSD OSABI.
> 
> We don't have a chance to use a similar hack for NetBSD in other 
> configuration options.
I've addressed `--target` option in D56650.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D56215



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


[PATCH] D56650: [lld] [ELF] Support inferring target triple from filename

2019-01-13 Thread Michał Górny via Phabricator via cfe-commits
mgorny created this revision.
mgorny added reviewers: ruiu, joerg, krytarowski, arichardson.
Herald added subscribers: llvm-commits, emaste.
Herald added a reviewer: espindola.

Support inferring the target triple from linker filename (e.g.
${triple}-ld.lld), and customizing the linker behavior based on it.
If the filename does not match any known target, lld defaults
to the default target configured in LLVM.

// NB: I understand this patch isn't going to be accepted upstream. However, 
since we're probably end up living with it in NetBSD, I'd appreciate any 
suggestions if the code could be improved.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D56650

Files:
  ELF/Config.h
  ELF/Driver.cpp
  ELF/Driver.h
  ELF/Options.td

Index: ELF/Options.td
===
--- ELF/Options.td
+++ ELF/Options.td
@@ -313,6 +313,8 @@
 
 defm sysroot: Eq<"sysroot", "Set the system root">;
 
+defm target: Eq<"target", "Apply configuration defaults for a given target">;
+
 def target1_rel: F<"target1-rel">, HelpText<"Interpret R_ARM_TARGET1 as R_ARM_REL32">;
 
 def target1_abs: F<"target1-abs">, HelpText<"Interpret R_ARM_TARGET1 as R_ARM_ABS32 (default)">;
Index: ELF/Driver.h
===
--- ELF/Driver.h
+++ ELF/Driver.h
@@ -31,6 +31,7 @@
   void addLibrary(StringRef Name);
 
 private:
+  void setTargetTriple(StringRef argv0, llvm::opt::InputArgList );
   void readConfigs(llvm::opt::InputArgList );
   void createFiles(llvm::opt::InputArgList );
   void inferMachineType();
Index: ELF/Driver.cpp
===
--- ELF/Driver.cpp
+++ ELF/Driver.cpp
@@ -54,6 +54,7 @@
 #include "llvm/Support/LEB128.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/TarWriter.h"
+#include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
@@ -378,6 +379,25 @@
 return;
   }
 
+  if (const char *Path = getReproduceOption(Args)) {
+// Note that --reproduce is a debug option so you can ignore it
+// if you are trying to understand the whole picture of the code.
+Expected> ErrOrWriter =
+TarWriter::create(Path, path::stem(Path));
+if (ErrOrWriter) {
+  Tar = std::move(*ErrOrWriter);
+  Tar->append("response.txt", createResponseFile(Args));
+  Tar->append("version.txt", getLLDVersion() + "\n");
+} else {
+  error("--reproduce: " + toString(ErrOrWriter.takeError()));
+}
+  }
+
+  initLLVM();
+  setTargetTriple(ArgsArr[0], Args);
+  readConfigs(Args);
+  checkZOptions(Args);
+
   // Handle -v or -version.
   //
   // A note about "compatible with GNU linkers" message: this is a hack for
@@ -393,26 +413,11 @@
   // lot of "configure" scripts out there that are generated by old version
   // of Libtool. We cannot convince every software developer to migrate to
   // the latest version and re-generate scripts. So we have this hack.
-  if (Args.hasArg(OPT_v) || Args.hasArg(OPT_version))
+  if (Args.hasArg(OPT_v) || Args.hasArg(OPT_version)) {
 message(getLLDVersion() + " (compatible with GNU linkers)");
-
-  if (const char *Path = getReproduceOption(Args)) {
-// Note that --reproduce is a debug option so you can ignore it
-// if you are trying to understand the whole picture of the code.
-Expected> ErrOrWriter =
-TarWriter::create(Path, path::stem(Path));
-if (ErrOrWriter) {
-  Tar = std::move(*ErrOrWriter);
-  Tar->append("response.txt", createResponseFile(Args));
-  Tar->append("version.txt", getLLDVersion() + "\n");
-} else {
-  error("--reproduce: " + toString(ErrOrWriter.takeError()));
-}
+message("Target: " + Config->TargetTriple.str());
   }
 
-  readConfigs(Args);
-  checkZOptions(Args);
-
   // The behavior of -v or --version is a bit strange, but this is
   // needed for compatibility with GNU linkers.
   if (Args.hasArg(OPT_v) && !Args.hasArg(OPT_INPUT))
@@ -420,7 +425,6 @@
   if (Args.hasArg(OPT_version))
 return;
 
-  initLLVM();
   createFiles(Args);
   if (errorCount())
 return;
@@ -746,6 +750,31 @@
   error(Msg + ": " + StringRef(Err).trim());
 }
 
+void LinkerDriver::setTargetTriple(StringRef argv0, opt::InputArgList ) {
+  // Firstly, see if user specified explicit --target
+  StringRef TargetOpt = Args.getLastArgValue(OPT_target);
+  if (!TargetOpt.empty()) {
+// TODO: do we want to verify it and fail on unsupported?
+Config->TargetTriple = llvm::Triple(TargetOpt);
+return;
+  }
+
+  // Secondly, try to get it from program name prefix
+  std::string ProgName = llvm::sys::path::stem(argv0);
+  size_t LastComponent = ProgName.rfind('-');
+  if (LastComponent != std::string::npos) {
+std::string Prefix = ProgName.substr(0, LastComponent);
+std::string IgnoredError;
+if (llvm::TargetRegistry::lookupTarget(Prefix, IgnoredError)) {
+  Config->TargetTriple 

[PATCH] D56597: [clangd] Add Limit parameter for xref.

2019-01-13 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 181481.
hokein added a comment.

Fix an issue and add comments.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56597

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/XRefs.cpp
  clangd/XRefs.h
  clangd/index/Index.h
  clangd/index/MemIndex.cpp
  clangd/index/Merge.cpp
  clangd/index/dex/Dex.cpp
  unittests/clangd/DexTests.cpp
  unittests/clangd/IndexTests.cpp
  unittests/clangd/XRefsTests.cpp

Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -1219,7 +1219,7 @@
 std::vector> ExpectedLocations;
 for (const auto  : T.ranges())
   ExpectedLocations.push_back(RangeIs(R));
-EXPECT_THAT(findReferences(AST, T.point()),
+EXPECT_THAT(findReferences(AST, T.point(), 0),
 ElementsAreArray(ExpectedLocations))
 << Test;
   }
@@ -1266,7 +1266,7 @@
 std::vector> ExpectedLocations;
 for (const auto  : T.ranges())
   ExpectedLocations.push_back(RangeIs(R));
-EXPECT_THAT(findReferences(AST, T.point()),
+EXPECT_THAT(findReferences(AST, T.point(), 0),
 ElementsAreArray(ExpectedLocations))
 << Test;
   }
@@ -1281,7 +1281,7 @@
   auto AST = TU.build();
 
   // References in main file are returned without index.
-  EXPECT_THAT(findReferences(AST, Main.point(), /*Index=*/nullptr),
+  EXPECT_THAT(findReferences(AST, Main.point(), 0, /*Index=*/nullptr),
   ElementsAre(RangeIs(Main.range(;
   Annotations IndexedMain(R"cpp(
 int main() { [[f^oo]](); }
@@ -1292,13 +1292,17 @@
   IndexedTU.Code = IndexedMain.code();
   IndexedTU.Filename = "Indexed.cpp";
   IndexedTU.HeaderCode = Header;
-  EXPECT_THAT(findReferences(AST, Main.point(), IndexedTU.index().get()),
+  EXPECT_THAT(findReferences(AST, Main.point(), 0, IndexedTU.index().get()),
   ElementsAre(RangeIs(Main.range()), RangeIs(IndexedMain.range(;
 
+  EXPECT_EQ(1u, findReferences(AST, Main.point(), /*Limit*/ 1,
+   IndexedTU.index().get())
+.size());
+
   // If the main file is in the index, we don't return duplicates.
   // (even if the references are in a different location)
   TU.Code = ("\n\n" + Main.code()).str();
-  EXPECT_THAT(findReferences(AST, Main.point(), TU.index().get()),
+  EXPECT_THAT(findReferences(AST, Main.point(), 0, TU.index().get()),
   ElementsAre(RangeIs(Main.range(;
 }
 
@@ -1328,7 +1332,7 @@
 Annotations File(T.AnnotatedCode);
 RecordingIndex Rec;
 auto AST = TestTU::withCode(File.code()).build();
-findReferences(AST, File.point(), );
+findReferences(AST, File.point(), 0, );
 if (T.WantQuery)
   EXPECT_NE(Rec.RefIDs, None) << T.AnnotatedCode;
 else
Index: unittests/clangd/IndexTests.cpp
===
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -290,16 +290,24 @@
 
   RefsRequest Request;
   Request.IDs = {Foo.ID};
-  RefSlab::Builder Results;
-  Merge.refs(Request, [&](const Ref ) { Results.insert(Foo.ID, O); });
-
-  EXPECT_THAT(
-  std::move(Results).build(),
-  ElementsAre(Pair(
-  _, UnorderedElementsAre(AllOf(RefRange(Test1Code.range("Foo")),
-FileURI("unittest:///test.cc")),
-  AllOf(RefRange(Test2Code.range("Foo")),
-FileURI("unittest:///test2.cc"));
+  {
+RefSlab::Builder Results;
+Merge.refs(Request, [&](const Ref ) { Results.insert(Foo.ID, O); });
+
+EXPECT_THAT(
+std::move(Results).build(),
+ElementsAre(Pair(
+_, UnorderedElementsAre(AllOf(RefRange(Test1Code.range("Foo")),
+  FileURI("unittest:///test.cc")),
+AllOf(RefRange(Test2Code.range("Foo")),
+  FileURI("unittest:///test2.cc"));
+  }
+  {
+Request.Limit = 1;
+size_t RefsCount = 0;
+Merge.refs(Request, [&](const Ref ) { ++RefsCount; });
+EXPECT_EQ(*Request.Limit, RefsCount);
+  }
 }
 
 MATCHER_P2(IncludeHeaderWithRef, IncludeHeader, References, "") {
Index: unittests/clangd/DexTests.cpp
===
--- unittests/clangd/DexTests.cpp
+++ unittests/clangd/DexTests.cpp
@@ -670,15 +670,26 @@
   AddRef(Foo, "reffoo.h", RefKind::Reference);
   AddRef(Bar, "bar.h", RefKind::Declaration);
 
-  std::vector Files;
-  RefsRequest Req;
-  Req.IDs.insert(Foo.ID);
-  Req.Filter = RefKind::Declaration | RefKind::Definition;
-  Dex(std::vector{Foo, Bar}, Refs).refs(Req, [&](const Ref ) {
-Files.push_back(R.Location.FileURI);
-  });
-
-  

[PATCH] D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403)

2019-01-13 Thread Mateusz Maćkowski via Phabricator via cfe-commits
m4tx added a comment.

I've added the implicit access specifier check and run it on some bigger 
codebases. My findings are as below:

- Dolphin: 6 //triggers across// 856 //record types//
- OpenCV: 31 //triggers across// 3370 //record types//
- Inkscape: 39 //triggers across// 846 //record types//
- LLVM (Core + Clang + Clang Tools Extra): 128 //triggers across// 9214 
//record types//
- Blender: 948 //triggers across// 6264 //record types//

Since the check was triggered in every big project I've tested, I've decided to 
add an option (disabled by default) to enable the implicit access specifier 
check. It is included in my latest Diff, along with the rename to 
`readability-redundant-access-specifiers`.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55793



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


[PATCH] D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403)

2019-01-13 Thread Mateusz Maćkowski via Phabricator via cfe-commits
m4tx updated this revision to Diff 181474.
m4tx marked an inline comment as not done.

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55793

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/readability/RedundantAccessSpecifiersCheck.cpp
  clang-tidy/readability/RedundantAccessSpecifiersCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-redundant-access-specifiers.rst
  
test/clang-tidy/readability-redundant-access-specifiers-check-first-declaration.cpp
  test/clang-tidy/readability-redundant-access-specifiers.cpp

Index: test/clang-tidy/readability-redundant-access-specifiers.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-access-specifiers.cpp
@@ -0,0 +1,116 @@
+// RUN: %check_clang_tidy %s readability-redundant-access-specifiers %t
+
+class FooPublic {
+public:
+  int a;
+public: // comment-0
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: duplicated access specifier [readability-redundant-access-specifiers]
+  // CHECK-MESSAGES: :[[@LINE-4]]:1: note: previously declared here
+  // CHECK-FIXES: {{^}}// comment-0{{$}}
+  int b;
+private:
+  int c;
+};
+
+struct StructPublic {
+public:
+  int a;
+public: // comment-1
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: duplicated access specifier [readability-redundant-access-specifiers]
+  // CHECK-MESSAGES: :[[@LINE-4]]:1: note: previously declared here
+  // CHECK-FIXES: {{^}}// comment-1{{$}}
+  int b;
+private:
+  int c;
+};
+
+union UnionPublic {
+public:
+  int a;
+public: // comment-2
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: duplicated access specifier [readability-redundant-access-specifiers]
+  // CHECK-MESSAGES: :[[@LINE-4]]:1: note: previously declared here
+  // CHECK-FIXES: {{^}}// comment-2{{$}}
+  int b;
+private:
+  int c;
+};
+
+class FooProtected {
+protected:
+  int a;
+protected: // comment-3
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: duplicated access specifier [readability-redundant-access-specifiers]
+  // CHECK-MESSAGES: :[[@LINE-4]]:1: note: previously declared here
+  // CHECK-FIXES: {{^}}// comment-3{{$}}
+  int b;
+private:
+  int c;
+};
+
+class FooPrivate {
+private:
+  int a;
+private: // comment-4
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: duplicated access specifier [readability-redundant-access-specifiers]
+  // CHECK-MESSAGES: :[[@LINE-4]]:1: note: previously declared here
+  // CHECK-FIXES: {{^}}// comment-4{{$}}
+  int b;
+public:
+  int c;
+};
+
+class FooMacro {
+private:
+  int a;
+#if defined(ZZ)
+  public:
+  int b;
+#endif
+private: // comment-5
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: duplicated access specifier [readability-redundant-access-specifiers]
+  // CHECK-MESSAGES: :[[@LINE-8]]:1: note: previously declared here
+  // CHECK-FIXES: {{^}}// comment-5{{$}}
+  int c;
+protected:
+  int d;
+public:
+  int e;
+};
+
+class Valid {
+private:
+  int a;
+public:
+  int b;
+private:
+  int c;
+protected:
+  int d;
+public:
+  int e;
+};
+
+class ValidInnerClass {
+public:
+  int a;
+
+  class Inner {
+  public:
+int b;
+  };
+};
+
+#define MIXIN private: int b;
+
+class ValidMacro {
+private:
+  int a;
+MIXIN
+private:
+  int c;
+protected:
+  int d;
+public:
+  int e;
+};
Index: test/clang-tidy/readability-redundant-access-specifiers-check-first-declaration.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-redundant-access-specifiers-check-first-declaration.cpp
@@ -0,0 +1,43 @@
+// RUN: %check_clang_tidy %s readability-redundant-access-specifiers %t -- \
+// RUN:   -config="{CheckOptions: [{key: readability-redundant-access-specifiers.CheckFirstDeclaration, value: 1}]}" --
+
+class FooPublic {
+private: // comment-0
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant access specifier [readability-redundant-access-specifiers]
+  // CHECK-FIXES: {{^}}// comment-0{{$}}
+  int a;
+};
+
+struct StructPublic {
+public: // comment-1
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant access specifier [readability-redundant-access-specifiers]
+  // CHECK-FIXES: {{^}}// comment-1{{$}}
+  int a;
+};
+
+union UnionPublic {
+public: // comment-2
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: redundant access specifier [readability-redundant-access-specifiers]
+  // CHECK-FIXES: {{^}}// comment-2{{$}}
+  int a;
+};
+
+class FooMacro {
+#if defined(ZZ)
+private:
+#endif
+  int a;
+};
+
+class ValidInnerStruct {
+  struct Inner {
+  private:
+int b;
+  };
+};
+
+#define MIXIN private: int b;
+
+class ValidMacro {
+MIXIN
+};
Index: docs/clang-tidy/checks/readability-redundant-access-specifiers.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-redundant-access-specifiers.rst
@@ -0,0 

[PATCH] D56642: NFC: Move dump of type nodes to NodeDumper

2019-01-13 Thread Stephen Kelly via Phabricator via cfe-commits
steveire updated this revision to Diff 181475.
steveire added a comment.

Update


Repository:
  rC Clang

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

https://reviews.llvm.org/D56642

Files:
  include/clang/AST/TextNodeDumper.h
  lib/AST/ASTDumper.cpp
  lib/AST/TextNodeDumper.cpp

Index: lib/AST/TextNodeDumper.cpp
===
--- lib/AST/TextNodeDumper.cpp
+++ lib/AST/TextNodeDumper.cpp
@@ -13,6 +13,7 @@
 
 #include "clang/AST/TextNodeDumper.h"
 
+#include "clang/AST/DeclTemplate.h"
 #include "clang/AST/LocInfoType.h"
 
 using namespace clang;
@@ -170,6 +171,8 @@
 OS << " contains_unexpanded_pack";
   if (T->isFromAST())
 OS << " imported";
+
+  TypeVisitor::Visit(T);
 }
 
 void TextNodeDumper::Visit(QualType T) {
@@ -886,3 +889,161 @@
 void TextNodeDumper::VisitObjCBoolLiteralExpr(const ObjCBoolLiteralExpr *Node) {
   OS << " " << (Node->getValue() ? "__objc_yes" : "__objc_no");
 }
+
+void TextNodeDumper::VisitRValueReferenceType(const ReferenceType *T) {
+  if (T->isSpelledAsLValue())
+OS << " written as lvalue reference";
+}
+
+void TextNodeDumper::VisitArrayType(const ArrayType *T) {
+  switch (T->getSizeModifier()) {
+  case ArrayType::Normal:
+break;
+  case ArrayType::Static:
+OS << " static";
+break;
+  case ArrayType::Star:
+OS << " *";
+break;
+  }
+  OS << " " << T->getIndexTypeQualifiers().getAsString();
+}
+
+void TextNodeDumper::VisitConstantArrayType(const ConstantArrayType *T) {
+  OS << " " << T->getSize();
+  VisitArrayType(T);
+}
+
+void TextNodeDumper::VisitVariableArrayType(const VariableArrayType *T) {
+  OS << " ";
+  dumpSourceRange(T->getBracketsRange());
+  VisitArrayType(T);
+}
+
+void TextNodeDumper::VisitDependentSizedArrayType(
+const DependentSizedArrayType *T) {
+  VisitArrayType(T);
+  OS << " ";
+  dumpSourceRange(T->getBracketsRange());
+}
+
+void TextNodeDumper::VisitDependentSizedExtVectorType(
+const DependentSizedExtVectorType *T) {
+  OS << " ";
+  dumpLocation(T->getAttributeLoc());
+}
+
+void TextNodeDumper::VisitVectorType(const VectorType *T) {
+  switch (T->getVectorKind()) {
+  case VectorType::GenericVector:
+break;
+  case VectorType::AltiVecVector:
+OS << " altivec";
+break;
+  case VectorType::AltiVecPixel:
+OS << " altivec pixel";
+break;
+  case VectorType::AltiVecBool:
+OS << " altivec bool";
+break;
+  case VectorType::NeonVector:
+OS << " neon";
+break;
+  case VectorType::NeonPolyVector:
+OS << " neon poly";
+break;
+  }
+  OS << " " << T->getNumElements();
+}
+
+void TextNodeDumper::VisitFunctionType(const FunctionType *T) {
+  auto EI = T->getExtInfo();
+  if (EI.getNoReturn())
+OS << " noreturn";
+  if (EI.getProducesResult())
+OS << " produces_result";
+  if (EI.getHasRegParm())
+OS << " regparm " << EI.getRegParm();
+  OS << " " << FunctionType::getNameForCallConv(EI.getCC());
+}
+
+void TextNodeDumper::VisitFunctionProtoType(const FunctionProtoType *T) {
+  auto EPI = T->getExtProtoInfo();
+  if (EPI.HasTrailingReturn)
+OS << " trailing_return";
+  if (T->isConst())
+OS << " const";
+  if (T->isVolatile())
+OS << " volatile";
+  if (T->isRestrict())
+OS << " restrict";
+  switch (EPI.RefQualifier) {
+  case RQ_None:
+break;
+  case RQ_LValue:
+OS << " &";
+break;
+  case RQ_RValue:
+OS << " &&";
+break;
+  }
+  // FIXME: Exception specification.
+  // FIXME: Consumed parameters.
+  VisitFunctionType(T);
+}
+
+void TextNodeDumper::VisitUnresolvedUsingType(const UnresolvedUsingType *T) {
+  dumpDeclRef(T->getDecl());
+}
+
+void TextNodeDumper::VisitTypedefType(const TypedefType *T) {
+  dumpDeclRef(T->getDecl());
+}
+
+void TextNodeDumper::VisitUnaryTransformType(const UnaryTransformType *T) {
+  switch (T->getUTTKind()) {
+  case UnaryTransformType::EnumUnderlyingType:
+OS << " underlying_type";
+break;
+  }
+}
+
+void TextNodeDumper::VisitTagType(const TagType *T) {
+  dumpDeclRef(T->getDecl());
+}
+
+void TextNodeDumper::VisitTemplateTypeParmType(const TemplateTypeParmType *T) {
+  OS << " depth " << T->getDepth() << " index " << T->getIndex();
+  if (T->isParameterPack())
+OS << " pack";
+  dumpDeclRef(T->getDecl());
+}
+
+void TextNodeDumper::VisitAutoType(const AutoType *T) {
+  if (T->isDecltypeAuto())
+OS << " decltype(auto)";
+  if (!T->isDeduced())
+OS << " undeduced";
+}
+
+void TextNodeDumper::VisitTemplateSpecializationType(
+const TemplateSpecializationType *T) {
+  if (T->isTypeAlias())
+OS << " alias";
+  OS << " ";
+  T->getTemplateName().dump(OS);
+}
+
+void TextNodeDumper::VisitInjectedClassNameType(
+const InjectedClassNameType *T) {
+  dumpDeclRef(T->getDecl());
+}
+
+void TextNodeDumper::VisitObjCInterfaceType(const ObjCInterfaceType *T) {
+  dumpDeclRef(T->getDecl());
+}
+
+void TextNodeDumper::VisitPackExpansionType(const PackExpansionType *T) {
+  if (auto N = T->getNumExpansions())
+ 

[PATCH] D56644: [clang-tidy] readability-container-size-empty handle std::string length()

2019-01-13 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment.

It'll be worth to mention change in Release Notes (in changes list, in 
alphabetical order).


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56644



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


[PATCH] D56644: [clang-tidy] readability-container-size-empty handle std::string length()

2019-01-13 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments.



Comment at: clang-tidy/readability/ContainerSizeEmptyCheck.cpp:34
 return;
 
   const auto ValidContainer = qualType(hasUnqualifiedDesugaredType(

```
auto ContainerLenghtFuncNames = anyOf(hasName("size"), hasName("length"));
```
pick better naem



Comment at: clang-tidy/readability/ContainerSizeEmptyCheck.cpp:40
   isConst(), parameterCountIs(0), isPublic(),
-  hasName("size"),
+  anyOf(hasName("size"), hasName("length")),
   returns(qualType(isInteger(), unless(booleanType()

s/`anyOf(hasName("size"), hasName("length")),`/`ContainerLenghtFuncNames,`/



Comment at: clang-tidy/readability/ContainerSizeEmptyCheck.cpp:40-65
+  anyOf(hasName("size"), hasName("length")),
   returns(qualType(isInteger(), unless(booleanType()
   .bind("size")),
   has(cxxMethodDecl(isConst(), parameterCountIs(0), isPublic(),
 hasName("empty"), returns(booleanType()))
   .bind("empty")))
   .bind("container")));

lebedev.ri wrote:
> s/`anyOf(hasName("size"), hasName("length")),`/`ContainerLenghtFuncNames,`/
We want `anyOf()` in both cases, with the same list of suspects.
If it is not a `anyOf()` in the second case, then naturally it won't work 
(can't have more than one name)
If it is not a `anyOf()` in the first case, then it will e.g. work if all of 
them are present.
So we just want to ensure that we do the same thing in both cases.




Comment at: clang-tidy/readability/ContainerSizeEmptyCheck.cpp:65
   hasType(references(ValidContainer),
-callee(cxxMethodDecl(hasName("size"))), WrongUse,
+callee(cxxMethodDecl(anyOf(hasName("size"), 
hasName("length", WrongUse,
 unless(hasAncestor(cxxMethodDecl(

lebedev.ri wrote:
> This line looks too long, clang-format might be too intrusive, so at least
> ```
> callee(cxxMethodDecl(anyOf(hasName("size"), 
>hasName("length",
> WrongUse,
> 
> ```
s/`callee(cxxMethodDecl(anyOf(hasName("size"), hasName("length", 
WrongUse,`/`callee(cxxMethodDecl(ContainerLenghtFuncNames)), WrongUse,`/



Comment at: test/clang-tidy/readability-container-size-empty.cpp:19-20
   basic_string operator+(const basic_string& other) const;
   unsigned long size() const;
+  unsigned long length() const;
   bool empty() const;

Quolyk wrote:
> lebedev.ri wrote:
> > Does it still work if only one of these exists?
> It works indeed, do you suggest adding test case for this?
Hmm, actually, i think there is an easier solution, see other inline comment.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56644



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


[PATCH] D56647: [WIP] [ELF] Implement --copy-dt-needed-entries

2019-01-13 Thread Michał Górny via Phabricator via cfe-commits
mgorny created this revision.
mgorny added reviewers: ruiu, krytarowski.
Herald added subscribers: llvm-commits, jfb, arichardson, emaste.
Herald added a reviewer: espindola.

This is my proof-of-concept on making `--copy-dt-needed-entries` work. 
Apparently, i've been able to hack on it hard enough to make it actually emit 
DT_NEEDED for all symbols, recursively:

  $ clang -fuse-ld=lld  -Wl,--copy-dt-needed-entries test.c  -lgit2
  $ readelf -d a.out 
  
  Dynamic section at offset 0x3010 contains 40 entries:
TagType Name/Value
   0x0001 (NEEDED) Shared library: [libgit2.so.27]
   0x0001 (NEEDED) Shared library: [librt.so.1]
   0x0001 (NEEDED) Shared library: [libpthread.so.0]
   0x0001 (NEEDED) Shared library: [libc.so.6]
   0x0001 (NEEDED) Shared library: 
[ld-linux-x86-64.so.2]
   0x0001 (NEEDED) Shared library: [libcurl.so.4]
   0x0001 (NEEDED) Shared library: [libcares.so.2]
   0x0001 (NEEDED) Shared library: [libnghttp2.so.14]
   0x0001 (NEEDED) Shared library: [libidn2.so.4]
   0x0001 (NEEDED) Shared library: [libunistring.so.2]
   0x0001 (NEEDED) Shared library: [libssl.so.1.1]
   0x0001 (NEEDED) Shared library: [libcrypto.so.1.1]
   0x0001 (NEEDED) Shared library: [libz.so.1]
   0x0001 (NEEDED) Shared library: [libdl.so.2]
   0x0001 (NEEDED) Shared library: 
[libhttp_parser.so.2.9]
   0x0001 (NEEDED) Shared library: [libssh2.so.1]
   0x0015 (DEBUG)  0x0
   0x0007 (RELA)   0x200620
   0x0008 (RELASZ) 48 (bytes)
   0x0009 (RELAENT)24 (bytes)
   0x0017 (JMPREL) 0x200650
   0x0002 (PLTRELSZ)   72 (bytes)
   0x0003 (PLTGOT) 0x202010
   0x0014 (PLTREL) RELA
   0x0006 (SYMTAB) 0x2002b0
   0x000b (SYMENT) 24 (bytes)
   0x0005 (STRTAB) 0x200488
   0x000a (STRSZ)  404 (bytes)
   0x6ef5 (GNU_HASH)   0x200400
   0x0004 (HASH)   0x200428
   0x0019 (INIT_ARRAY) 0x203008
   0x001b (INIT_ARRAYSZ)   8 (bytes)
   0x001a (FINI_ARRAY) 0x203000
   0x001c (FINI_ARRAYSZ)   8 (bytes)
   0x000c (INIT)   0x201200
   0x000d (FINI)   0x201218
   0x6ff0 (VERSYM) 0x2003b8
   0x6ffe (VERNEED)0x2003d0
   0x6fff (VERNEEDNUM) 1
   0x (NULL)   0x0

Can't say the code is nice but before I try to make it cleanly, I'd appreciate 
some tips on whether I'm even starting in the right direction.

FWICS, GNU ld's `--copy-dt-needed-entries` adds lot less libraries — it seems 
as if it implicitly forced `--as-needed`, and `--no-as-needed` doesn't seem to 
make a difference.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D56647

Files:
  ELF/Config.h
  ELF/Driver.cpp
  ELF/InputFiles.cpp
  ELF/InputFiles.h
  ELF/Options.td
  ELF/SymbolTable.cpp

Index: ELF/SymbolTable.cpp
===
--- ELF/SymbolTable.cpp
+++ ELF/SymbolTable.cpp
@@ -16,6 +16,7 @@
 
 #include "SymbolTable.h"
 #include "Config.h"
+#include "Driver.h"
 #include "LinkerScript.h"
 #include "Symbols.h"
 #include "SyntheticSections.h"
@@ -109,6 +110,17 @@
   return;
 
 SharedFiles.push_back(F);
+if (F->CopyDtNeeded) {
+  for (std::string& DtNeeded : F->parseDtNeeded()) {
+if (Optional Path = searchLibrary(":" + DtNeeded)) {
+  // TODO: how does memory management work here?
+  if (Optional Buffer = readFile(*Path))
+addFile(make>(*Buffer, DtNeeded));
+} else {
+  // TODO: error out?
+}
+  }
+}
 F->parseRest();
 return;
   }
Index: ELF/Options.td
===
--- ELF/Options.td
+++ ELF/Options.td
@@ -88,6 +88,10 @@
   HelpText<"Use colors in diagnostics">,
   MetaVarName<"[auto,always,never]">;
 
+defm copy_dt_needed_entries: B<"copy-dt-needed-entries",
+"Recursively include all dependencies (DT_NEEDED) of linked libraries in the executable",
+"Include only DT_NEEDED entries for libraries explicity listed (default)">;
+
 defm cref: B<"cref",
 "Output cross reference table",
 "Do not output cross reference table">;
@@ -495,7 +499,6 @@
 def: F<"long-plt">;
 def: F<"no-add-needed">;
 def: 

[PATCH] D56644: [clang-tidy] readability-container-size-empty handle std::string length()

2019-01-13 Thread Dmitry Venikov via Phabricator via cfe-commits
Quolyk marked an inline comment as done.
Quolyk added inline comments.



Comment at: test/clang-tidy/readability-container-size-empty.cpp:19-20
   basic_string operator+(const basic_string& other) const;
   unsigned long size() const;
+  unsigned long length() const;
   bool empty() const;

lebedev.ri wrote:
> Does it still work if only one of these exists?
It works indeed, do you suggest adding test case for this?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56644



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


[PATCH] D56644: [clang-tidy] readability-container-size-empty handle std::string length()

2019-01-13 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

Seems to look good.




Comment at: clang-tidy/readability/ContainerSizeEmptyCheck.cpp:65
   hasType(references(ValidContainer),
-callee(cxxMethodDecl(hasName("size"))), WrongUse,
+callee(cxxMethodDecl(anyOf(hasName("size"), 
hasName("length", WrongUse,
 unless(hasAncestor(cxxMethodDecl(

This line looks too long, clang-format might be too intrusive, so at least
```
callee(cxxMethodDecl(anyOf(hasName("size"), 
   hasName("length",
WrongUse,

```



Comment at: test/clang-tidy/readability-container-size-empty.cpp:19-20
   basic_string operator+(const basic_string& other) const;
   unsigned long size() const;
+  unsigned long length() const;
   bool empty() const;

Does it still work if only one of these exists?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56644



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


[PATCH] D56644: [clang-tidy] readability-container-size-empty handle std::string length()

2019-01-13 Thread Dmitry Venikov via Phabricator via cfe-commits
Quolyk updated this revision to Diff 181467.
Quolyk added a comment.

Update tests. Handle length.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56644

Files:
  clang-tidy/readability/ContainerSizeEmptyCheck.cpp
  test/clang-tidy/readability-container-size-empty.cpp

Index: test/clang-tidy/readability-container-size-empty.cpp
===
--- test/clang-tidy/readability-container-size-empty.cpp
+++ test/clang-tidy/readability-container-size-empty.cpp
@@ -17,6 +17,7 @@
   bool operator!=(const char *) const;
   basic_string operator+(const basic_string& other) const;
   unsigned long size() const;
+  unsigned long length() const;
   bool empty() const;
 };
 
@@ -106,30 +107,42 @@
   std::string str2;
   std::wstring wstr;
   (void)(str.size() + 0);
+  (void)(str.length() + 0);
   (void)(str.size() - 0);
+  (void)(str.length() - 0);
   (void)(0 + str.size());
+  (void)(0 + str.length());
   (void)(0 - str.size());
+  (void)(0 - str.length());
   if (intSet.size() == 0)
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
   // CHECK-FIXES: {{^  }}if (intSet.empty()){{$}}
-  // CHECK-MESSAGES: :32:8: note: method 'set'::empty() defined here
+  // CHECK-MESSAGES: :33:8: note: method 'set'::empty() defined here
   if (intSet == std::set())
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness
   // CHECK-FIXES: {{^  }}if (intSet.empty()){{$}}
-  // CHECK-MESSAGES: :32:8: note: method 'set'::empty() defined here
+  // CHECK-MESSAGES: :33:8: note: method 'set'::empty() defined here
   if (s_func() == "")
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
   // CHECK-FIXES: {{^  }}if (s_func().empty()){{$}}
   if (str.size() == 0)
 ;
-  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
+  // CHECK-FIXES: {{^  }}if (str.empty()){{$}}
+  if (str.length() == 0)
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'length' [readability-container-size-empty]
   // CHECK-FIXES: {{^  }}if (str.empty()){{$}}
   if ((str + str2).size() == 0)
 ;
-  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
+  // CHECK-FIXES: {{^  }}if ((str + str2).empty()){{$}}
+  if ((str + str2).length() == 0)
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'length' [readability-container-size-empty]
   // CHECK-FIXES: {{^  }}if ((str + str2).empty()){{$}}
   if (str == "")
 ;
@@ -141,7 +154,11 @@
   // CHECK-FIXES: {{^  }}if ((str + str2).empty()){{$}}
   if (wstr.size() == 0)
 ;
-  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
+  // CHECK-FIXES: {{^  }}if (wstr.empty()){{$}}
+  if (wstr.length() == 0)
+;
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'length' [readability-container-size-empty]
   // CHECK-FIXES: {{^  }}if (wstr.empty()){{$}}
   if (wstr == "")
 ;
@@ -150,7 +167,7 @@
   std::vector vect;
   if (vect.size() == 0)
 ;
-  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
   // CHECK-FIXES: {{^  }}if (vect.empty()){{$}}
   if (vect == std::vector())
 ;
@@ -423,17 +440,17 @@
   if (templated_container.size())
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :44:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :45:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
   if (templated_container != TemplatedContainer())
 ;
   // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: the 'empty' method should be used
-  // CHECK-MESSAGES: :44:8: note: method 'TemplatedContainer'::empty() defined here
+  // CHECK-MESSAGES: :45:8: note: method 'TemplatedContainer'::empty() defined here
   // CHECK-FIXES: {{^  }}if (!templated_container.empty()){{$}}
   // CHECK-FIXES-NEXT: ;
   

[PATCH] D56646: [OpenCL] opencl-c.h: read_image*(): sampler-less, and image{1, 2}d_array_t variants are OpenCL-1.2+, mark them as such

2019-01-13 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri created this revision.
lebedev.ri added reviewers: yaxunl, Anastasia, echuraev, asavonic.
lebedev.ri added a project: clang.

Refer to `6.11.13.2 Built-in Image Functions` 
,
and `9.6.8 Image Read and Write Functions` 
 of the 
OpenCL 1.1 spec.

- There is no mention of `image1d_array_t` and `image2d_array_t` anywhere in 
the OpenCL 1.1 spec.
- All the `read_image{f,i,ui,h}()` functions, as of OpenCL 1.1 spec, have a 
second required parameter `sampler_t sampler`

Should have prevented the following regression:
https://redmine.darktable.org/issues/12493


Repository:
  rC Clang

https://reviews.llvm.org/D56646

Files:
  lib/Headers/opencl-c.h


Index: lib/Headers/opencl-c.h
===
--- lib/Headers/opencl-c.h
+++ lib/Headers/opencl-c.h
@@ -14610,6 +14610,7 @@
 uint4 __purefn __ovld read_imageui(read_only image3d_t image, sampler_t 
sampler, int4 coord);
 uint4 __purefn __ovld read_imageui(read_only image3d_t image, sampler_t 
sampler, float4 coord);
 
+#if __OPENCL_C_VERSION__ >= CL_VERSION_1_2
 float4 __purefn __ovld read_imagef(read_only image2d_array_t image_array, 
sampler_t sampler, int4 coord);
 float4 __purefn __ovld read_imagef(read_only image2d_array_t image_array, 
sampler_t sampler, float4 coord);
 
@@ -14617,6 +14618,7 @@
 int4 __purefn __ovld read_imagei(read_only image2d_array_t image_array, 
sampler_t sampler, float4 coord);
 uint4 __purefn __ovld read_imageui(read_only image2d_array_t image_array, 
sampler_t sampler, int4 coord);
 uint4 __purefn __ovld read_imageui(read_only image2d_array_t image_array, 
sampler_t sampler, float4 coord);
+#endif // __OPENCL_C_VERSION__ >= CL_VERSION_1_2
 
 float4 __purefn __ovld read_imagef(read_only image1d_t image, sampler_t 
sampler, int coord);
 float4 __purefn __ovld read_imagef(read_only image1d_t image, sampler_t 
sampler, float coord);
@@ -14626,6 +14628,7 @@
 uint4 __purefn __ovld read_imageui(read_only image1d_t image, sampler_t 
sampler, int coord);
 uint4 __purefn __ovld read_imageui(read_only image1d_t image, sampler_t 
sampler, float coord);
 
+#if __OPENCL_C_VERSION__ >= CL_VERSION_1_2
 float4 __purefn __ovld read_imagef(read_only image1d_array_t image_array, 
sampler_t sampler, int2 coord);
 float4 __purefn __ovld read_imagef(read_only image1d_array_t image_array, 
sampler_t sampler, float2 coord);
 
@@ -14633,6 +14636,7 @@
 int4 __purefn __ovld read_imagei(read_only image1d_array_t image_array, 
sampler_t sampler, float2 coord);
 uint4 __purefn __ovld read_imageui(read_only image1d_array_t image_array, 
sampler_t sampler, int2 coord);
 uint4 __purefn __ovld read_imageui(read_only image1d_array_t image_array, 
sampler_t sampler, float2 coord);
+#endif // __OPENCL_C_VERSION__ >= CL_VERSION_1_2
 
 #ifdef cl_khr_depth_images
 float __purefn __ovld read_imagef(read_only image2d_depth_t image, sampler_t 
sampler, float2 coord);
@@ -14735,6 +14739,8 @@
 #endif //cl_khr_mipmap_image
 #endif //__OPENCL_C_VERSION__ >= CL_VERSION_2_0
 
+#if __OPENCL_C_VERSION__ >= CL_VERSION_1_2
+
 /**
 * Sampler-less Image Access
 */
@@ -14768,24 +14774,31 @@
 int4 __purefn __ovld read_imagei(read_only image3d_t image, int4 coord);
 uint4 __purefn __ovld read_imageui(read_only image3d_t image, int4 coord);
 
+#endif // __OPENCL_C_VERSION__ >= CL_VERSION_1_2
+
 // Image read functions returning half4 type
 #ifdef cl_khr_fp16
 half4 __purefn __ovld read_imageh(read_only image1d_t image, sampler_t 
sampler, int coord);
 half4 __purefn __ovld read_imageh(read_only image1d_t image, sampler_t 
sampler, float coord);
-half4 __purefn __ovld read_imageh(read_only image1d_array_t image, sampler_t 
sampler, int2 coord);
-half4 __purefn __ovld read_imageh(read_only image1d_array_t image, sampler_t 
sampler, float2 coord);
 half4 __purefn __ovld read_imageh(read_only image2d_t image, sampler_t 
sampler, int2 coord);
 half4 __purefn __ovld read_imageh(read_only image2d_t image, sampler_t 
sampler, float2 coord);
 half4 __purefn __ovld read_imageh(read_only image3d_t image, sampler_t 
sampler, int4 coord);
 half4 __purefn __ovld read_imageh(read_only image3d_t image, sampler_t 
sampler, float4 coord);
+#if __OPENCL_C_VERSION__ >= CL_VERSION_1_2
+half4 __purefn __ovld read_imageh(read_only image1d_array_t image, sampler_t 
sampler, int2 coord);
+half4 __purefn __ovld read_imageh(read_only image1d_array_t image, sampler_t 
sampler, float2 coord);
 half4 __purefn __ovld read_imageh(read_only image2d_array_t image, sampler_t 
sampler, int4 coord);
 half4 __purefn __ovld read_imageh(read_only image2d_array_t image, sampler_t 
sampler, float4 coord);
+/**
+ * Sampler-less Image Access
+ */
 half4 __purefn __ovld read_imageh(read_only image1d_t image, int coord);
 half4 __purefn __ovld read_imageh(read_only image2d_t image, int2 coord);
 half4 __purefn __ovld