[PATCH] D43779: [Tooling] [0/1] Refactor FrontendActionFactory::create() to return std::unique_ptr<>

2019-08-29 Thread Roman Lebedev via Phabricator via cfe-commits 
lebedev.ri abandoned this revision.
lebedev.ri added a comment.
Herald added a project: LLVM.

Superseded by D66960 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D43779



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

[PATCH] D43779: [Tooling] [0/1] Refactor FrontendActionFactory::create() to return std::unique_ptr<>

2018-04-09 Thread David Blaikie via Phabricator via cfe-commits 
dblaikie added subscribers: lebedev.ri, dblaikie.
dblaikie added a comment.

FWIW - I had some thoughts on this a while back:
https://reviews.llvm.org/D4313


Repository:
  rL LLVM

https://reviews.llvm.org/D43779



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

Re: [PATCH] D43779: [Tooling] [0/1] Refactor FrontendActionFactory::create() to return std::unique_ptr<>

2018-04-09 Thread David Blaikie via cfe-commits 
FWIW - I had some thoughts on this a while back:
https://reviews.llvm.org/D4313

On Mon, Apr 9, 2018 at 4:54 AM Roman Lebedev via Phabricator via
llvm-commits  wrote:

> lebedev.ri added a comment.
>
> In https://reviews.llvm.org/D43779#1061444, @alexfh wrote:
>
> > In https://reviews.llvm.org/D43779#1061043, @lebedev.ri wrote:
> >
> > > Hmm.
> > >  Got back to this issue.
> > >
> > > Not reproducible with gcc-4.8.5 and gcc-4.9.3 in ubuntu 16.04 chroot.
> > >  *Reproducible* with gcc-4.8.4 and gcc-4.9.2 in debian oldstable
> (Jessie) chroot.
> > >  So one might assume that it was fixed.
> > >
> > > I did try to creduce the crasher, but not not much success, since gcc
> takes 10+ sec to crash.
> >
> >
> > That's unfortunate, but it could be even worse. My experience with
> creducing crashes is that the process can take up to several days. In this
> case, however, we know what changes cause the crash and it should be
> possible to construct the test case manually instead of creducing it.
> >
> > > Should i simply try to re-commit and see if the buildbots got upgraded
> so this is no longer an issue?
>
>
>
>
> > I wouldn't expect buildbots to have been upgraded without someone doing
> this on purpose. If you have the list of buildbots that crashed, you could
> look at their recent logs to figure out which GCC version they are using
> now.
>
> Yes, it seems they weren't upgraded yet as of this time.
>
> > It *might* be fine to require a patched version of GCC, but in that case
> you would have to:
> >  0. Ask llvm-dev+cfe-dev whether anyone has concerns in raising the
> minimum required version of GCC
>
> Yep, did that right after posting that comment,
> http://lists.llvm.org/pipermail/llvm-dev/2018-April/122438.html
>
> > 1. Fix the documentation, in particular this list:
> https://llvm.org/docs/GettingStarted.html#software and these
> instructions:
> https://llvm.org/docs/GettingStarted.html#getting-a-modern-host-c-toolchain
> > 2. Get buildbot maintainers to upgrade their GCC to at least the new
> required version
> >
> >   An alternative would be to try working around the bug.
>
>
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D43779
>
>
>
> ___
> llvm-commits mailing list
> llvm-comm...@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43779: [Tooling] [0/1] Refactor FrontendActionFactory::create() to return std::unique_ptr<>

2018-04-09 Thread Roman Lebedev via Phabricator via cfe-commits 
lebedev.ri added a comment.

In https://reviews.llvm.org/D43779#1061444, @alexfh wrote:

> In https://reviews.llvm.org/D43779#1061043, @lebedev.ri wrote:
>
> > Hmm.
> >  Got back to this issue.
> >
> > Not reproducible with gcc-4.8.5 and gcc-4.9.3 in ubuntu 16.04 chroot.
> >  *Reproducible* with gcc-4.8.4 and gcc-4.9.2 in debian oldstable (Jessie) 
> > chroot.
> >  So one might assume that it was fixed.
> >
> > I did try to creduce the crasher, but not not much success, since gcc takes 
> > 10+ sec to crash.
>
>
> That's unfortunate, but it could be even worse. My experience with creducing 
> crashes is that the process can take up to several days. In this case, 
> however, we know what changes cause the crash and it should be possible to 
> construct the test case manually instead of creducing it.
>
> > Should i simply try to re-commit and see if the buildbots got upgraded so 
> > this is no longer an issue?




> I wouldn't expect buildbots to have been upgraded without someone doing this 
> on purpose. If you have the list of buildbots that crashed, you could look at 
> their recent logs to figure out which GCC version they are using now.

Yes, it seems they weren't upgraded yet as of this time.

> It *might* be fine to require a patched version of GCC, but in that case you 
> would have to:
>  0. Ask llvm-dev+cfe-dev whether anyone has concerns in raising the minimum 
> required version of GCC

Yep, did that right after posting that comment, 
http://lists.llvm.org/pipermail/llvm-dev/2018-April/122438.html

> 1. Fix the documentation, in particular this list: 
> https://llvm.org/docs/GettingStarted.html#software and these instructions: 
> https://llvm.org/docs/GettingStarted.html#getting-a-modern-host-c-toolchain
> 2. Get buildbot maintainers to upgrade their GCC to at least the new required 
> version
> 
>   An alternative would be to try working around the bug.




Repository:
  rL LLVM

https://reviews.llvm.org/D43779



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

[PATCH] D43779: [Tooling] [0/1] Refactor FrontendActionFactory::create() to return std::unique_ptr<>

2018-04-09 Thread Alexander Kornienko via Phabricator via cfe-commits 
alexfh added a comment.

In https://reviews.llvm.org/D43779#1061043, @lebedev.ri wrote:

> Hmm.
>  Got back to this issue.
>
> Not reproducible with gcc-4.8.5 and gcc-4.9.3 in ubuntu 16.04 chroot.
>  *Reproducible* with gcc-4.8.4 and gcc-4.9.2 in debian oldstable (Jessie) 
> chroot.
>  So one might assume that it was fixed.
>
> I did try to creduce the crasher, but not not much success, since gcc takes 
> 10+ sec to crash.


That's unfortunate, but it could be even worse. My experience with creducing 
crashes is that the process can take up to several days. In this case, however, 
we know what changes cause the crash and it should be possible to construct the 
test case manually instead of creducing it.

> Should i simply try to re-commit and see if the buildbots got upgraded so 
> this is no longer an issue?

I wouldn't expect buildbots to have been upgraded without someone doing this on 
purpose. If you have the list of buildbots that crashed, you could look at 
their recent logs to figure out which GCC version they are using now. It 
*might* be fine to require a patched version of GCC, but in that case you would 
have to:
0. Ask llvm-dev+cfe-dev whether anyone has concerns in raising the minimum 
required version of GCC

1. Fix the documentation, in particular this list: 
https://llvm.org/docs/GettingStarted.html#software and these instructions: 
https://llvm.org/docs/GettingStarted.html#getting-a-modern-host-c-toolchain
2. Get buildbot maintainers to upgrade their GCC to at least the new required 
version

An alternative would be to try working around the bug.


Repository:
  rL LLVM

https://reviews.llvm.org/D43779



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

[PATCH] D43779: [Tooling] [0/1] Refactor FrontendActionFactory::create() to return std::unique_ptr<>

2018-04-08 Thread Roman Lebedev via Phabricator via cfe-commits 
lebedev.ri added a comment.

Hmm.
Got back to this issue.

Not reproducible with gcc-4.8.5 and gcc-4.9.3 in ubuntu 16.04 chroot.
*Reproducible* with gcc-4.8.4 and gcc-4.9.2 in debian oldstable (Jessie) chroot.
So one might assume that it was fixed.

I did try to creduce the crasher, but not not much success, since gcc takes 10+ 
sec to crash.

Should i simply try to re-commit and see if the buildbots got upgraded so this 
is no longer an issue?


Repository:
  rL LLVM

https://reviews.llvm.org/D43779



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

[PATCH] D43779: [Tooling] [0/1] Refactor FrontendActionFactory::create() to return std::unique_ptr<>

2018-02-28 Thread Roman Lebedev via Phabricator via cfe-commits 
lebedev.ri added a comment.

In https://reviews.llvm.org/D43779#1021959, @alexfh wrote:

> Do you have a way to reproduce the gcc crashes?


Not presently.
I'm on debian sid, so gcc4.8 is a lost-for-good relic from ancient past.

I'll try to create an debian-oldstable chroot, which still has gcc4.8, maybe 
that will help.


Repository:
  rL LLVM

https://reviews.llvm.org/D43779



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

[PATCH] D43779: [Tooling] [0/1] Refactor FrontendActionFactory::create() to return std::unique_ptr<>

2018-02-28 Thread Alexander Kornienko via Phabricator via cfe-commits 
alexfh added a comment.

Do you have a way to reproduce the gcc crashes?


Repository:
  rL LLVM

https://reviews.llvm.org/D43779



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

[PATCH] D43779: [Tooling] [0/1] Refactor FrontendActionFactory::create() to return std::unique_ptr<>

2018-02-27 Thread Roman Lebedev via Phabricator via cfe-commits 
lebedev.ri reopened this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

And reverted.

This broke gcc4.8 builds, compiler just segfaults:
http://lab.llvm.org:8011/builders/clang-atom-d525-fedora-rel/builds/14909

  ...
  [72/368] Building CXX object 
tools/clang/tools/extra/clang-tidy/CMakeFiles/clangTidy.dir/ClangTidyOptions.cpp.o
  [73/368] Building CXX object 
tools/clang/tools/extra/clang-tidy/android/CMakeFiles/clangTidyAndroidModule.dir/AndroidTidyModule.cpp.o
  [74/368] Building CXX object 
tools/clang/tools/extra/clang-tidy/android/CMakeFiles/clangTidyAndroidModule.dir/CloexecAccept4Check.cpp.o
  FAILED: /bin/ccache  g++  -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE 
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
-Itools/clang/tools/clang-check 
-I/export/users/atombot/llvm/clang-atom-d525-fedora-rel/llvm/tools/clang/tools/clang-check
 
-I/export/users/atombot/llvm/clang-atom-d525-fedora-rel/llvm/tools/clang/include
 -Itools/clang/include -I/usr/include/libxml2 -Iinclude 
-I/export/users/atombot/llvm/clang-atom-d525-fedora-rel/llvm/include -fPIC 
-fvisibility-inlines-hidden -std=c++11 -Wall -W -Wno-unused-parameter 
-Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic 
-Wno-long-long -Wno-maybe-uninitialized -Wdelete-non-virtual-dtor -Wno-comment 
-ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual 
-fno-strict-aliasing -O3-UNDEBUG  -fno-exceptions -fno-rtti -MMD -MT 
tools/clang/tools/clang-check/CMakeFiles/clang-check.dir/ClangCheck.cpp.o -MF 
tools/clang/tools/clang-check/CMakeFiles/clang-check.dir/ClangCheck.cpp.o.d -o 
tools/clang/tools/clang-check/CMakeFiles/clang-check.dir/ClangCheck.cpp.o -c 
/export/users/atombot/llvm/clang-atom-d525-fedora-rel/llvm/tools/clang/tools/clang-check/ClangCheck.cpp
  In file included from 
/export/users/atombot/llvm/clang-atom-d525-fedora-rel/llvm/tools/clang/tools/clang-check/ClangCheck.cpp:28:0:
  
/export/users/atombot/llvm/clang-atom-d525-fedora-rel/llvm/tools/clang/include/clang/Tooling/Tooling.h:
 In instantiation of 'clang::tooling::newFrontendActionFactory(FactoryT*, 
clang::tooling::SourceFileCallbacks*)::FrontendActionFactoryAdapter::ConsumerFactoryAdaptor::ConsumerFactoryAdaptor(FactoryT*,
 clang::tooling::SourceFileCallbacks*) [with FactoryT = 
{anonymous}::ClangCheckActionFactory]':
  
/export/users/atombot/llvm/clang-atom-d525-fedora-rel/llvm/include/llvm/ADT/STLExtras.h:951:63:
   required from 'typename std::enable_if<(! std::is_array< 
 >::value), std::unique_ptr >::type 
llvm::make_unique(Args&& ...) [with T = 
clang::tooling::newFrontendActionFactory(FactoryT*, 
clang::tooling::SourceFileCallbacks*) [with FactoryT = 
{anonymous}::ClangCheckActionFactory]::FrontendActionFactoryAdapter::ConsumerFactoryAdaptor;
 Args = {{anonymous}::ClangCheckActionFactory*&, 
clang::tooling::SourceFileCallbacks*&}; typename std::enable_if<(! 
std::is_array<  >::value), std::unique_ptr >::type = 
std::unique_ptr
 >]'
  
/export/users/atombot/llvm/clang-atom-d525-fedora-rel/llvm/tools/clang/include/clang/Tooling/Tooling.h:390:65:
   required from 'std::unique_ptr 
clang::tooling::newFrontendActionFactory(FactoryT*, 
clang::tooling::SourceFileCallbacks*)::FrontendActionFactoryAdapter::create() 
[with FactoryT = {anonymous}::ClangCheckActionFactory]'
  
/export/users/atombot/llvm/clang-atom-d525-fedora-rel/llvm/tools/clang/include/clang/Tooling/Tooling.h:425:3:
   required from 'std::unique_ptr 
clang::tooling::newFrontendActionFactory(FactoryT*, 
clang::tooling::SourceFileCallbacks*) [with FactoryT = 
{anonymous}::ClangCheckActionFactory]'
  
/export/users/atombot/llvm/clang-atom-d525-fedora-rel/llvm/tools/clang/tools/clang-check/ClangCheck.cpp:183:61:
   required from here
  
/export/users/atombot/llvm/clang-atom-d525-fedora-rel/llvm/tools/clang/include/clang/Tooling/Tooling.h:396:7:
 internal compiler error: Segmentation fault
 ConsumerFactoryAdaptor(FactoryT *ConsumerFactory,
 ^
  Please submit a full bug report,
  with preprocessed source if appropriate.
  See  for instructions.
  Preprocessed source stored into /tmp/ccYe9Zii.out file, please attach this to 
your bugreport.
  FAILED: /bin/ccache  g++  -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE 
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
-Itools/clang/tools/extra/clang-reorder-fields/tool 
-I/export/users/atombot/llvm/clang-atom-d525-fedora-rel/llvm/tools/clang/tools/extra/clang-reorder-fields/tool

[PATCH] D43779: [Tooling] [0/1] Refactor FrontendActionFactory::create() to return std::unique_ptr<>

2018-02-27 Thread Phabricator via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL326201: [Tooling] [0/1] Refactor 
FrontendActionFactory::create() to return std… (authored by lebedevri, 
committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D43779?vs=135955=136076#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D43779

Files:
  cfe/trunk/docs/LibTooling.rst
  cfe/trunk/docs/RAVFrontendAction.rst
  cfe/trunk/include/clang/Tooling/Tooling.h
  cfe/trunk/lib/Tooling/Tooling.cpp
  cfe/trunk/tools/clang-refactor/ClangRefactor.cpp
  cfe/trunk/unittests/AST/EvaluateAsRValueTest.cpp
  cfe/trunk/unittests/CrossTU/CrossTranslationUnitTest.cpp
  cfe/trunk/unittests/Sema/CodeCompleteTest.cpp
  cfe/trunk/unittests/Sema/ExternalSemaSourceTest.cpp
  cfe/trunk/unittests/Tooling/CommentHandlerTest.cpp
  cfe/trunk/unittests/Tooling/ExecutionTest.cpp
  cfe/trunk/unittests/Tooling/RefactoringTest.cpp
  cfe/trunk/unittests/Tooling/TestVisitor.h
  cfe/trunk/unittests/Tooling/ToolingTest.cpp

Index: cfe/trunk/lib/Tooling/Tooling.cpp
===
--- cfe/trunk/lib/Tooling/Tooling.cpp
+++ cfe/trunk/lib/Tooling/Tooling.cpp
@@ -104,12 +104,12 @@
   return Invocation;
 }
 
-bool runToolOnCode(clang::FrontendAction *ToolAction, const Twine ,
-   const Twine ,
+bool runToolOnCode(std::unique_ptr ToolAction,
+   const Twine , const Twine ,
std::shared_ptr PCHContainerOps) {
-  return runToolOnCodeWithArgs(ToolAction, Code, std::vector(),
-   FileName, "clang-tool",
-   std::move(PCHContainerOps));
+  return runToolOnCodeWithArgs(std::move(ToolAction), Code,
+   std::vector(), FileName,
+   "clang-tool", std::move(PCHContainerOps));
 }
 
 static std::vector
@@ -125,7 +125,7 @@
 }
 
 bool runToolOnCodeWithArgs(
-clang::FrontendAction *ToolAction, const Twine ,
+std::unique_ptr ToolAction, const Twine ,
 const std::vector , const Twine ,
 const Twine ,
 std::shared_ptr PCHContainerOps,
@@ -143,8 +143,7 @@
   ArgumentsAdjuster Adjuster = getClangStripDependencyFileAdjuster();
   ToolInvocation Invocation(
   getSyntaxOnlyToolArgs(ToolName, Adjuster(Args, FileNameRef), FileNameRef),
-  ToolAction, Files.get(),
-  std::move(PCHContainerOps));
+  std::move(ToolAction), Files.get(), std::move(PCHContainerOps));
 
   SmallString<1024> CodeStorage;
   InMemoryFileSystem->addFile(FileNameRef, 0,
@@ -204,15 +203,18 @@
 namespace {
 
 class SingleFrontendActionFactory : public FrontendActionFactory {
-  FrontendAction *Action;
+  std::unique_ptr Action;
 
 public:
-  SingleFrontendActionFactory(FrontendAction *Action) : Action(Action) {}
+  SingleFrontendActionFactory(std::unique_ptr Action)
+  : Action(std::move(Action)) {}
 
-  FrontendAction *create() override { return Action; }
+  std::unique_ptr create() override {
+return std::move(Action);
+  }
 };
 
-}
+} // namespace
 
 ToolInvocation::ToolInvocation(
 std::vector CommandLine, ToolAction *Action,
@@ -222,12 +224,13 @@
   DiagConsumer(nullptr) {}
 
 ToolInvocation::ToolInvocation(
-std::vector CommandLine, FrontendAction *FAction,
-FileManager *Files, std::shared_ptr PCHContainerOps)
+std::vector CommandLine,
+std::unique_ptr FAction, FileManager *Files,
+std::shared_ptr PCHContainerOps)
 : CommandLine(std::move(CommandLine)),
-  Action(new SingleFrontendActionFactory(FAction)), OwnsAction(true),
-  Files(Files), PCHContainerOps(std::move(PCHContainerOps)),
-  DiagConsumer(nullptr) {}
+  Action(new SingleFrontendActionFactory(std::move(FAction))),
+  OwnsAction(true), Files(Files),
+  PCHContainerOps(std::move(PCHContainerOps)), DiagConsumer(nullptr) {}
 
 ToolInvocation::~ToolInvocation() {
   if (OwnsAction)
Index: cfe/trunk/unittests/Tooling/ToolingTest.cpp
===
--- cfe/trunk/unittests/Tooling/ToolingTest.cpp
+++ cfe/trunk/unittests/Tooling/ToolingTest.cpp
@@ -64,10 +64,10 @@
 
 TEST(runToolOnCode, FindsNoTopLevelDeclOnEmptyCode) {
   bool FoundTopLevelDecl = false;
-  EXPECT_TRUE(
-  runToolOnCode(new TestAction(llvm::make_unique(
-)),
-""));
+  EXPECT_TRUE(runToolOnCode(
+  llvm::make_unique(
+  llvm::make_unique()),
+  ""));
   EXPECT_FALSE(FoundTopLevelDecl);
 }
 
@@ -104,17 +104,17 @@
 
 TEST(runToolOnCode, FindsClassDecl) {
   bool FoundClassDeclX = false;
-  EXPECT_TRUE(
-  runToolOnCode(new TestAction(llvm::make_unique(
-)),
-"class X;"));
+  EXPECT_TRUE(runToolOnCode(
+  llvm::make_unique(
+  llvm::make_unique()),
+  "class X;"));
   EXPECT_TRUE(FoundClassDeclX);
 
   FoundClassDeclX = false;
-  EXPECT_TRUE(

[PATCH] D43779: [Tooling] [0/1] Refactor FrontendActionFactory::create() to return std::unique_ptr<>

2018-02-27 Thread Phabricator via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rC326201: [Tooling] [0/1] Refactor 
FrontendActionFactory::create() to return std… (authored by lebedevri, 
committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D43779

Files:
  docs/LibTooling.rst
  docs/RAVFrontendAction.rst
  include/clang/Tooling/Tooling.h
  lib/Tooling/Tooling.cpp
  tools/clang-refactor/ClangRefactor.cpp
  unittests/AST/EvaluateAsRValueTest.cpp
  unittests/CrossTU/CrossTranslationUnitTest.cpp
  unittests/Sema/CodeCompleteTest.cpp
  unittests/Sema/ExternalSemaSourceTest.cpp
  unittests/Tooling/CommentHandlerTest.cpp
  unittests/Tooling/ExecutionTest.cpp
  unittests/Tooling/RefactoringTest.cpp
  unittests/Tooling/TestVisitor.h
  unittests/Tooling/ToolingTest.cpp

Index: lib/Tooling/Tooling.cpp
===
--- lib/Tooling/Tooling.cpp
+++ lib/Tooling/Tooling.cpp
@@ -104,12 +104,12 @@
   return Invocation;
 }
 
-bool runToolOnCode(clang::FrontendAction *ToolAction, const Twine ,
-   const Twine ,
+bool runToolOnCode(std::unique_ptr ToolAction,
+   const Twine , const Twine ,
std::shared_ptr PCHContainerOps) {
-  return runToolOnCodeWithArgs(ToolAction, Code, std::vector(),
-   FileName, "clang-tool",
-   std::move(PCHContainerOps));
+  return runToolOnCodeWithArgs(std::move(ToolAction), Code,
+   std::vector(), FileName,
+   "clang-tool", std::move(PCHContainerOps));
 }
 
 static std::vector
@@ -125,7 +125,7 @@
 }
 
 bool runToolOnCodeWithArgs(
-clang::FrontendAction *ToolAction, const Twine ,
+std::unique_ptr ToolAction, const Twine ,
 const std::vector , const Twine ,
 const Twine ,
 std::shared_ptr PCHContainerOps,
@@ -143,8 +143,7 @@
   ArgumentsAdjuster Adjuster = getClangStripDependencyFileAdjuster();
   ToolInvocation Invocation(
   getSyntaxOnlyToolArgs(ToolName, Adjuster(Args, FileNameRef), FileNameRef),
-  ToolAction, Files.get(),
-  std::move(PCHContainerOps));
+  std::move(ToolAction), Files.get(), std::move(PCHContainerOps));
 
   SmallString<1024> CodeStorage;
   InMemoryFileSystem->addFile(FileNameRef, 0,
@@ -204,15 +203,18 @@
 namespace {
 
 class SingleFrontendActionFactory : public FrontendActionFactory {
-  FrontendAction *Action;
+  std::unique_ptr Action;
 
 public:
-  SingleFrontendActionFactory(FrontendAction *Action) : Action(Action) {}
+  SingleFrontendActionFactory(std::unique_ptr Action)
+  : Action(std::move(Action)) {}
 
-  FrontendAction *create() override { return Action; }
+  std::unique_ptr create() override {
+return std::move(Action);
+  }
 };
 
-}
+} // namespace
 
 ToolInvocation::ToolInvocation(
 std::vector CommandLine, ToolAction *Action,
@@ -222,12 +224,13 @@
   DiagConsumer(nullptr) {}
 
 ToolInvocation::ToolInvocation(
-std::vector CommandLine, FrontendAction *FAction,
-FileManager *Files, std::shared_ptr PCHContainerOps)
+std::vector CommandLine,
+std::unique_ptr FAction, FileManager *Files,
+std::shared_ptr PCHContainerOps)
 : CommandLine(std::move(CommandLine)),
-  Action(new SingleFrontendActionFactory(FAction)), OwnsAction(true),
-  Files(Files), PCHContainerOps(std::move(PCHContainerOps)),
-  DiagConsumer(nullptr) {}
+  Action(new SingleFrontendActionFactory(std::move(FAction))),
+  OwnsAction(true), Files(Files),
+  PCHContainerOps(std::move(PCHContainerOps)), DiagConsumer(nullptr) {}
 
 ToolInvocation::~ToolInvocation() {
   if (OwnsAction)
Index: unittests/Tooling/ToolingTest.cpp
===
--- unittests/Tooling/ToolingTest.cpp
+++ unittests/Tooling/ToolingTest.cpp
@@ -64,10 +64,10 @@
 
 TEST(runToolOnCode, FindsNoTopLevelDeclOnEmptyCode) {
   bool FoundTopLevelDecl = false;
-  EXPECT_TRUE(
-  runToolOnCode(new TestAction(llvm::make_unique(
-)),
-""));
+  EXPECT_TRUE(runToolOnCode(
+  llvm::make_unique(
+  llvm::make_unique()),
+  ""));
   EXPECT_FALSE(FoundTopLevelDecl);
 }
 
@@ -104,17 +104,17 @@
 
 TEST(runToolOnCode, FindsClassDecl) {
   bool FoundClassDeclX = false;
-  EXPECT_TRUE(
-  runToolOnCode(new TestAction(llvm::make_unique(
-)),
-"class X;"));
+  EXPECT_TRUE(runToolOnCode(
+  llvm::make_unique(
+  llvm::make_unique()),
+  "class X;"));
   EXPECT_TRUE(FoundClassDeclX);
 
   FoundClassDeclX = false;
-  EXPECT_TRUE(
-  runToolOnCode(new TestAction(llvm::make_unique(
-)),
-"class Y;"));
+  EXPECT_TRUE(runToolOnCode(
+  llvm::make_unique(
+  llvm::make_unique()),
+  "class Y;"));
   EXPECT_FALSE(FoundClassDeclX);
 }
 
@@ -162,8 +162,8 @@
   Args.push_back("-Idef");

[PATCH] D43779: [Tooling] [0/1] Refactor FrontendActionFactory::create() to return std::unique_ptr<>

2018-02-27 Thread Alexander Kornienko via Phabricator via cfe-commits 
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.

Awesome, thank you for this cleanup!


Repository:
  rC Clang

https://reviews.llvm.org/D43779



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

[PATCH] D43779: [Tooling] [0/1] Refactor FrontendActionFactory::create() to return std::unique_ptr<>

2018-02-26 Thread Roman Lebedev via Phabricator via cfe-commits 
lebedev.ri created this revision.
lebedev.ri added reviewers: klimek, bkramer, alexfh, pcc.
lebedev.ri added a project: clang.

Noticed during review of https://reviews.llvm.org/D41102.

I'm not sure whether there are any principal reasons why it returns raw owning 
pointer,
or it is just a old code that was not updated post-C++11.

I'm not too sure what testing i should do, because `check-all` is not error 
clean here for some reason,
but it does not //appear// asif those failures are related to these changes.

This is clang part.
Clang-tools-extra part is in the next differential.


Repository:
  rC Clang

https://reviews.llvm.org/D43779

Files:
  docs/LibTooling.rst
  docs/RAVFrontendAction.rst
  include/clang/Tooling/Tooling.h
  lib/Tooling/Tooling.cpp
  tools/clang-refactor/ClangRefactor.cpp
  unittests/AST/EvaluateAsRValueTest.cpp
  unittests/CrossTU/CrossTranslationUnitTest.cpp
  unittests/Sema/CodeCompleteTest.cpp
  unittests/Sema/ExternalSemaSourceTest.cpp
  unittests/Tooling/CommentHandlerTest.cpp
  unittests/Tooling/ExecutionTest.cpp
  unittests/Tooling/RefactoringTest.cpp
  unittests/Tooling/TestVisitor.h
  unittests/Tooling/ToolingTest.cpp

Index: unittests/Tooling/ToolingTest.cpp
===
--- unittests/Tooling/ToolingTest.cpp
+++ unittests/Tooling/ToolingTest.cpp
@@ -64,10 +64,10 @@
 
 TEST(runToolOnCode, FindsNoTopLevelDeclOnEmptyCode) {
   bool FoundTopLevelDecl = false;
-  EXPECT_TRUE(
-  runToolOnCode(new TestAction(llvm::make_unique(
-)),
-""));
+  EXPECT_TRUE(runToolOnCode(
+  llvm::make_unique(
+  llvm::make_unique()),
+  ""));
   EXPECT_FALSE(FoundTopLevelDecl);
 }
 
@@ -104,17 +104,17 @@
 
 TEST(runToolOnCode, FindsClassDecl) {
   bool FoundClassDeclX = false;
-  EXPECT_TRUE(
-  runToolOnCode(new TestAction(llvm::make_unique(
-)),
-"class X;"));
+  EXPECT_TRUE(runToolOnCode(
+  llvm::make_unique(
+  llvm::make_unique()),
+  "class X;"));
   EXPECT_TRUE(FoundClassDeclX);
 
   FoundClassDeclX = false;
-  EXPECT_TRUE(
-  runToolOnCode(new TestAction(llvm::make_unique(
-)),
-"class Y;"));
+  EXPECT_TRUE(runToolOnCode(
+  llvm::make_unique(
+  llvm::make_unique()),
+  "class Y;"));
   EXPECT_FALSE(FoundClassDeclX);
 }
 
@@ -162,8 +162,8 @@
   Args.push_back("-Idef");
   Args.push_back("-fsyntax-only");
   Args.push_back("test.cpp");
-  clang::tooling::ToolInvocation Invocation(Args, new SyntaxOnlyAction,
-Files.get());
+  clang::tooling::ToolInvocation Invocation(
+  Args, llvm::make_unique(), Files.get());
   InMemoryFileSystem->addFile(
   "test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("#include \n"));
   InMemoryFileSystem->addFile("def/abc", 0,
@@ -188,8 +188,8 @@
   Args.push_back("-Idef");
   Args.push_back("-fsyntax-only");
   Args.push_back("test.cpp");
-  clang::tooling::ToolInvocation Invocation(Args, new SyntaxOnlyAction,
-Files.get());
+  clang::tooling::ToolInvocation Invocation(
+  Args, llvm::make_unique(), Files.get());
   InMemoryFileSystem->addFile(
   "test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("#include \n"));
   InMemoryFileSystem->addFile("def/abc", 0,
@@ -259,61 +259,64 @@
   std::vector Args = {"-std=c++11"};
   std::vector Args2 = {"-fno-delayed-template-parsing"};
 
-  EXPECT_TRUE(runToolOnCode(new SkipBodyAction,
+  EXPECT_TRUE(runToolOnCode(llvm::make_unique(),
 "int skipMe() { an_error_here }"));
-  EXPECT_FALSE(runToolOnCode(new SkipBodyAction,
+  EXPECT_FALSE(runToolOnCode(llvm::make_unique(),
  "int skipMeNot() { an_error_here }"));
 
   // Test constructors with initializers
   EXPECT_TRUE(runToolOnCodeWithArgs(
-  new SkipBodyAction,
+  llvm::make_unique(),
   "struct skipMe { skipMe() : an_error() { more error } };", Args));
   EXPECT_TRUE(runToolOnCodeWithArgs(
-  new SkipBodyAction, "struct skipMe { skipMe(); };"
-  "skipMe::skipMe() : an_error([](){;}) { more error }",
+  llvm::make_unique(),
+  "struct skipMe { skipMe(); };"
+  "skipMe::skipMe() : an_error([](){;}) { more error }",
   Args));
   EXPECT_TRUE(runToolOnCodeWithArgs(
-  new SkipBodyAction, "struct skipMe { skipMe(); };"
-  "skipMe::skipMe() : an_error{[](){;}} { more error }",
+  llvm::make_unique(),
+  "struct skipMe { skipMe(); };"
+  "skipMe::skipMe() : an_error{[](){;}} { more error }",
   Args));
   EXPECT_TRUE(runToolOnCodeWithArgs(
-  new SkipBodyAction,
+  llvm::make_unique(),
   "struct skipMe { skipMe(); };"
   "skipMe::skipMe() : a>(), f{}, g() { error }",
   Args));
   EXPECT_TRUE(runToolOnCodeWithArgs(
-  new SkipBodyAction, "struct