Re: [PATCH] D21814: clang-rename: split existing options into two new subcommands

2016-07-29 Thread Miklos Vajna via cfe-commits
vmiklos added a comment.

Is there anything I can help with to get this accepted, please? As far as I see 
I addressed all so far mentioned concerns. Thanks.


https://reviews.llvm.org/D21814



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


Re: [PATCH] D21748: Implement tooling::Replacements as a class.

2016-07-29 Thread Eric Liu via cfe-commits
ioeric updated this revision to Diff 66093.
ioeric marked an inline comment as done.
ioeric added a comment.

- getShiftedCodePosition: do not minus 1 when there is no replacement text.


https://reviews.llvm.org/D21748

Files:
  include/clang/Tooling/Core/Replacement.h
  include/clang/Tooling/Refactoring.h
  lib/Format/Format.cpp
  lib/Format/SortJavaScriptImports.cpp
  lib/Format/TokenAnalyzer.cpp
  lib/Format/WhitespaceManager.cpp
  lib/Tooling/Core/Replacement.cpp
  lib/Tooling/Refactoring.cpp
  lib/Tooling/RefactoringCallbacks.cpp
  tools/clang-format/ClangFormat.cpp
  unittests/Format/CleanupTest.cpp
  unittests/Format/FormatTest.cpp
  unittests/Tooling/RefactoringTest.cpp
  unittests/Tooling/ReplacementTest.h
  unittests/Tooling/RewriterTest.cpp

Index: unittests/Tooling/RewriterTest.cpp
===
--- unittests/Tooling/RewriterTest.cpp
+++ unittests/Tooling/RewriterTest.cpp
@@ -39,8 +39,11 @@
 
 TEST(Rewriter, AdjacentInsertAndDelete) {
   Replacements Replaces;
-  Replaces.insert(Replacement("", 6, 6, ""));
-  Replaces.insert(Replacement("", 6, 0, "replaced\n"));
+  auto Err = Replaces.add(Replacement("", 6, 6, ""));
+  EXPECT_TRUE(!Err);
+  Replaces =
+  Replaces.merge(Replacements(Replacement("", 6, 0, "replaced\n")));
+
   auto Rewritten = applyAllReplacements("line1\nline2\nline3\nline4", Replaces);
   EXPECT_TRUE(static_cast(Rewritten));
   EXPECT_EQ("line1\nreplaced\nline3\nline4", *Rewritten);
Index: unittests/Tooling/ReplacementTest.h
===
--- /dev/null
+++ unittests/Tooling/ReplacementTest.h
@@ -0,0 +1,56 @@
+//===- unittest/Tooling/ReplacementTest.h - Replacements related test--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+//  This file defines utility class and function for Replacement related tests.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_UNITTESTS_TOOLING_REPLACEMENTTESTBASE_H
+#define LLVM_CLANG_UNITTESTS_TOOLING_REPLACEMENTTESTBASE_H
+
+#include "RewriterTestContext.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace tooling {
+
+/// \brief Converts a set of replacements to Replacements class.
+/// \return A Replacements class containing \p Replaces on success; otherwise,
+/// an empty Replacements is returned.
+static tooling::Replacements
+toReplacements(const std::set &Replaces) {
+  tooling::Replacements Result;
+  for (const auto &R : Replaces) {
+auto Err = Result.add(R);
+EXPECT_TRUE(!Err);
+if (Err) {
+  llvm::errs() << llvm::toString(std::move(Err)) << "\n";
+  return tooling::Replacements();
+}
+  }
+  return Result;
+}
+
+/// \brief A utility class for replacement related tests.
+class ReplacementTest : public ::testing::Test {
+protected:
+  tooling::Replacement createReplacement(SourceLocation Start, unsigned Length,
+ llvm::StringRef ReplacementText) {
+return tooling::Replacement(Context.Sources, Start, Length,
+ReplacementText);
+  }
+
+  RewriterTestContext Context;
+};
+
+} // namespace tooling
+} // namespace clang
+
+#endif // LLVM_CLANG_UNITTESTS_TOOLING_REPLACEMENTTESTBASE_H
Index: unittests/Tooling/RefactoringTest.cpp
===
--- unittests/Tooling/RefactoringTest.cpp
+++ unittests/Tooling/RefactoringTest.cpp
@@ -7,6 +7,7 @@
 //
 //===--===//
 
+#include "ReplacementTest.h"
 #include "RewriterTestContext.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
@@ -31,16 +32,6 @@
 namespace clang {
 namespace tooling {
 
-class ReplacementTest : public ::testing::Test {
- protected:
-  Replacement createReplacement(SourceLocation Start, unsigned Length,
-llvm::StringRef ReplacementText) {
-return Replacement(Context.Sources, Start, Length, ReplacementText);
-  }
-
-  RewriterTestContext Context;
-};
-
 TEST_F(ReplacementTest, CanDeleteAllText) {
   FileID ID = Context.createInMemoryFile("input.cpp", "text");
   SourceLocation Location = Context.getLocation(ID, 1, 1);
@@ -108,62 +99,59 @@
   EXPECT_TRUE(Replace2.getFilePath().empty());
 }
 
-TEST_F(ReplacementTest, CanApplyReplacements) {
-  FileID ID = Context.createInMemoryFile("input.cpp",
- "line1\nline2\nline3\nline4");
+TEST_F(ReplacementTest, FailAddReplacements) {
   Replacements Replaces;
-  Replaces.insert(Replacement(Context.Sources, Context.getLocation(ID, 2, 1),
-  5, "replaced"));
-  

Re: [PATCH] D21814: clang-rename: split existing options into two new subcommands

2016-07-29 Thread Kirill Bobyrev via cfe-commits
omtcyfz added a subscriber: Eugene.Zelenko.
omtcyfz added a comment.

1. Run `clang-format` or something, 80 char width limit is broken in 
`tool/ClangRename.cpp` dozen of times.
2. Only do `outs() << "abcd\n" << "efgh\n"` if you have something in between, 
which can not be predefined. I.e. if you have

  /// \brief Top level help.
  static int helpMain(int argc, const char *argv[]) {
errs() << "Usage: clang-rename {rename-at|rename-all} [OPTION]...\n\n"
   << "A tool to rename symbols in C/C++ code.\n\n"
   << "Subcommands:\n"
   << "  rename-at:  Perform rename off of a location in a file. (This 
is the default.)\n"
   << "  rename-all: Perform rename of all symbols matching one or more 
fully qualified names.\n";
return 0;
  }

Just make it a const string, isn't it better?

3. `tooling::RefactoringTool` takes care of printing version IIUC, no need to 
do that in a custom way (we don't have custom version in `clang-rename` head at 
the moment, that was something @Eugene.Zelenko sent recently).
4. `clang-rename/RenamingAction.h` -> `USRList` -> `USRs` or the other way 
around everywhere. So far single naming convention feels right to me (I 
personally prefer `*s` over `*List`). LLVM Coding Style 

 does, too, from what I understand. Unless it's `*Set`, which is pretty much 
different thing.
5. Do we really need to dispatch these functions this way? with

  enum RenameSubcommand {
RenameAt, RenameAll
  };

And many other strange things. Just pass `bool isMultipleRename` or 
`isRenameAll` to `main` routine instead of creating `enum`. We only have two 
such options here, right? I pray we won't have more.

6. Move `int main(int argc, const char **argv)` upwards, so that it's above 
dispatching routines. Otherwise when one wants to see what's going on, he sees 
subroutine first without understanding where it comes from. Other way around 
feels more intuitive to me.

Feel free do disagree with my points, I may be terribly wrong :)


https://reviews.llvm.org/D21814



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


Re: [PATCH] D22853: [clang-rename] add support for template parameter renaming

2016-07-29 Thread Kirill Bobyrev via cfe-commits
omtcyfz added a comment.

Ping. Change is quite trivial.


https://reviews.llvm.org/D22853



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


[PATCH] D22957: [ASTMatcher] Add hasTemplateArgument/hasAnyTemplateArgument support in functionDecl.

2016-07-29 Thread Haojian Wu via cfe-commits
hokein created this revision.
hokein added a reviewer: klimek.
hokein added a subscriber: cfe-commits.
Herald added a subscriber: klimek.

https://reviews.llvm.org/D22957

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  include/clang/ASTMatchers/ASTMatchersInternal.h
  unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -594,6 +594,14 @@
   "A a;",
 templateSpecializationType(hasTemplateArgument(
   1, refersToType(asString("int"));
+
+  EXPECT_TRUE(matches(
+"template void f() {};"
+  "void func() { f(); }",
+functionDecl(hasTemplateArgument(0, refersToType(asString("int"));
+  EXPECT_TRUE(notMatches(
+"template void f() {};",
+functionDecl(hasTemplateArgument(0, refersToType(asString("int"));
 }
 
 TEST(TemplateArgument, Matches) {
@@ -603,6 +611,11 @@
   EXPECT_TRUE(matches(
 "template struct C {}; C c;",
 templateSpecializationType(hasAnyTemplateArgument(templateArgument();
+
+  EXPECT_TRUE(matches(
+"template void f() {};"
+  "void func() { f(); }",
+functionDecl(hasAnyTemplateArgument(templateArgument();
 }
 
 TEST(RefersToIntegralType, Matches) {
Index: include/clang/ASTMatchers/ASTMatchersInternal.h
===
--- include/clang/ASTMatchers/ASTMatchersInternal.h
+++ include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -1638,6 +1638,13 @@
   return llvm::makeArrayRef(T.getArgs(), T.getNumArgs());
 }
 
+inline ArrayRef
+getTemplateSpecializationArgs(const FunctionDecl &FD) {
+  if (const auto* TemplateArgs = FD.getTemplateSpecializationArgs())
+return TemplateArgs->asArray();
+  return ArrayRef();
+}
+
 struct NotEqualsBoundNodePredicate {
   bool operator()(const internal::BoundNodesMap &Nodes) const {
 return Nodes.getNode(ID) != Node;
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -556,22 +556,32 @@
   return Node.isImplicit();
 }
 
-/// \brief Matches classTemplateSpecializations that have at least one
-/// TemplateArgument matching the given InnerMatcher.
+/// \brief Matches classTemplateSpecializations, templateSpecializationType and
+/// functionDecl that have at least one TemplateArgument matching the given
+/// InnerMatcher.
 ///
 /// Given
 /// \code
 ///   template class A {};
 ///   template<> class A {};
 ///   A a;
+///
+///   template f() {};
+///   void func() { f(); };
+/// \endcode
+///
 /// \endcode
 /// classTemplateSpecializationDecl(hasAnyTemplateArgument(
 /// refersToType(asString("int"
 ///   matches the specialization \c A
+///
+/// functionDecl(hasAnyTemplateArgument(refersToType(asString("int"
+///   matches the specialization \c f
 AST_POLYMORPHIC_MATCHER_P(
 hasAnyTemplateArgument,
 AST_POLYMORPHIC_SUPPORTED_TYPES(ClassTemplateSpecializationDecl,
-TemplateSpecializationType),
+TemplateSpecializationType,
+FunctionDecl),
 internal::Matcher, InnerMatcher) {
   ArrayRef List =
   internal::getTemplateSpecializationArgs(Node);
@@ -698,22 +708,29 @@
   return InnerMatcher.matches(Node.IgnoreParens(), Finder, Builder);
 }
 
-/// \brief Matches classTemplateSpecializations where the n'th TemplateArgument
-/// matches the given InnerMatcher.
+/// \brief Matches  classTemplateSpecializations, TemplateSpecializationType and
+/// FunctionDecl where the n'th TemplateArgument matches the given InnerMatcher.
 ///
 /// Given
 /// \code
 ///   template class A {};
 ///   A b;
 ///   A c;
+///
+///   template f() {};
+///   void func() { f(); };
 /// \endcode
 /// classTemplateSpecializationDecl(hasTemplateArgument(
 /// 1, refersToType(asString("int"
 ///   matches the specialization \c A
+///
+/// functionDecl(hasTemplateArgument(0, refersToType(asString("int"
+///   matches the specialization \c f
 AST_POLYMORPHIC_MATCHER_P2(
 hasTemplateArgument,
 AST_POLYMORPHIC_SUPPORTED_TYPES(ClassTemplateSpecializationDecl,
-TemplateSpecializationType),
+TemplateSpecializationType,
+FunctionDecl),
 unsigned, N, internal::Matcher, InnerMatcher) {
   ArrayRef List =
   internal::getTemplateSpecializationArgs(Node);
Index: docs/LibASTMatchersReference.html
===
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -4240,30 +4240,44 @@
 
 
 Matcher

Re: [PATCH] D22957: [ASTMatcher] Add hasTemplateArgument/hasAnyTemplateArgument support in functionDecl.

2016-07-29 Thread Manuel Klimek via cfe-commits
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.

lg


https://reviews.llvm.org/D22957



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


Re: [PATCH] D22853: [clang-rename] add support for template parameter renaming

2016-07-29 Thread Kirill Bobyrev via cfe-commits
omtcyfz updated this revision to Diff 66095.
omtcyfz added a comment.

git rebase; git resolve conflicts


https://reviews.llvm.org/D22853

Files:
  clang-rename/USRFinder.cpp
  clang-rename/USRLocFinder.cpp
  test/clang-rename/TemplateTypenameFindByTemplateParam.cpp
  test/clang-rename/TemplateTypenameFindByTypeInside.cpp

Index: test/clang-rename/TemplateTypenameFindByTypeInside.cpp
===
--- test/clang-rename/TemplateTypenameFindByTypeInside.cpp
+++ test/clang-rename/TemplateTypenameFindByTypeInside.cpp
@@ -1,8 +1,4 @@
-// RUN: clang-rename -offset=289 -new-name=U %s -- | FileCheck %s
-
-// Currently unsupported test.
-// FIXME: clang-rename should be able to rename template parameters correctly.
-// XFAIL: *
+// RUN: clang-rename -offset=153 -new-name=Bar %s -- | FileCheck %s
 
 template  // CHECK: template 
 class Foo {
Index: test/clang-rename/TemplateTypenameFindByTemplateParam.cpp
===
--- test/clang-rename/TemplateTypenameFindByTemplateParam.cpp
+++ test/clang-rename/TemplateTypenameFindByTemplateParam.cpp
@@ -1,10 +1,4 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=270 -new-name=U %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-
-// Currently unsupported test.
-// FIXME: clang-rename should be able to rename template parameters correctly.
-// XFAIL: *
+// RUN: clang-rename -offset=87 -new-name=Bar %s -- | FileCheck %s
 
 template  // CHECK: template 
 class Foo {
Index: clang-rename/USRLocFinder.cpp
===
--- clang-rename/USRLocFinder.cpp
+++ clang-rename/USRLocFinder.cpp
@@ -101,6 +101,11 @@
 if (getUSRForDecl(Loc.getType()->getAsCXXRecordDecl()) == USR) {
   checkAndAddLocation(Loc.getBeginLoc());
 }
+if (const auto TemplateTypeParm = 
dyn_cast(Loc.getType())) {
+  if (getUSRForDecl(TemplateTypeParm->getDecl()) == USR) {
+checkAndAddLocation(Loc.getBeginLoc());
+  }
+}
 return true;
   }
 
Index: clang-rename/USRFinder.cpp
===
--- clang-rename/USRFinder.cpp
+++ clang-rename/USRFinder.cpp
@@ -77,6 +77,9 @@
 const auto TypeBeginLoc = Loc.getBeginLoc();
 const auto TypeEndLoc = Lexer::getLocForEndOfToken(
 TypeBeginLoc, 0, Context.getSourceManager(), Context.getLangOpts());
+if (const auto *TemplateTypeParm = 
dyn_cast(Loc.getType())) {
+  return setResult(TemplateTypeParm->getDecl(), TypeBeginLoc, TypeEndLoc);
+}
 return setResult(Loc.getType()->getAsCXXRecordDecl(), TypeBeginLoc,
  TypeEndLoc);
   }


Index: test/clang-rename/TemplateTypenameFindByTypeInside.cpp
===
--- test/clang-rename/TemplateTypenameFindByTypeInside.cpp
+++ test/clang-rename/TemplateTypenameFindByTypeInside.cpp
@@ -1,8 +1,4 @@
-// RUN: clang-rename -offset=289 -new-name=U %s -- | FileCheck %s
-
-// Currently unsupported test.
-// FIXME: clang-rename should be able to rename template parameters correctly.
-// XFAIL: *
+// RUN: clang-rename -offset=153 -new-name=Bar %s -- | FileCheck %s
 
 template  // CHECK: template 
 class Foo {
Index: test/clang-rename/TemplateTypenameFindByTemplateParam.cpp
===
--- test/clang-rename/TemplateTypenameFindByTemplateParam.cpp
+++ test/clang-rename/TemplateTypenameFindByTemplateParam.cpp
@@ -1,10 +1,4 @@
-// RUN: cat %s > %t.cpp
-// RUN: clang-rename -offset=270 -new-name=U %t.cpp -i --
-// RUN: sed 's,//.*,,' %t.cpp | FileCheck %s
-
-// Currently unsupported test.
-// FIXME: clang-rename should be able to rename template parameters correctly.
-// XFAIL: *
+// RUN: clang-rename -offset=87 -new-name=Bar %s -- | FileCheck %s
 
 template  // CHECK: template 
 class Foo {
Index: clang-rename/USRLocFinder.cpp
===
--- clang-rename/USRLocFinder.cpp
+++ clang-rename/USRLocFinder.cpp
@@ -101,6 +101,11 @@
 if (getUSRForDecl(Loc.getType()->getAsCXXRecordDecl()) == USR) {
   checkAndAddLocation(Loc.getBeginLoc());
 }
+if (const auto TemplateTypeParm = dyn_cast(Loc.getType())) {
+  if (getUSRForDecl(TemplateTypeParm->getDecl()) == USR) {
+checkAndAddLocation(Loc.getBeginLoc());
+  }
+}
 return true;
   }
 
Index: clang-rename/USRFinder.cpp
===
--- clang-rename/USRFinder.cpp
+++ clang-rename/USRFinder.cpp
@@ -77,6 +77,9 @@
 const auto TypeBeginLoc = Loc.getBeginLoc();
 const auto TypeEndLoc = Lexer::getLocForEndOfToken(
 TypeBeginLoc, 0, Context.getSourceManager(), Context.getLangOpts());
+if (const auto *TemplateTypeParm = dyn_cast(Loc.getType())) {
+  return setResult(TemplateTypeParm-

Re: [PATCH] D21814: clang-rename: split existing options into two new subcommands

2016-07-29 Thread Miklos Vajna via cfe-commits
vmiklos updated this revision to Diff 66099.

https://reviews.llvm.org/D21814

Files:
  clang-rename/RenamingAction.cpp
  clang-rename/RenamingAction.h
  clang-rename/tool/ClangRename.cpp
  docs/clang-rename.rst
  test/clang-rename/ClassFindByName.cpp
  test/clang-rename/ClassTestMulti.cpp
  test/clang-rename/ClassTestMultiByName.cpp
  test/clang-rename/FunctionWithClassFindByName.cpp
  test/clang-rename/NoNewName.cpp

Index: test/clang-rename/NoNewName.cpp
===
--- test/clang-rename/NoNewName.cpp
+++ test/clang-rename/NoNewName.cpp
@@ -1,4 +1,4 @@
 // Check for an error while -new-name argument has not been passed to
 // clang-rename.
 // RUN: not clang-rename -offset=133 %s 2>&1 | FileCheck %s
-// CHECK: ERROR: no new name provided.
+// CHECK: clang-rename: for the -new-name option: must be specified
Index: test/clang-rename/FunctionWithClassFindByName.cpp
===
--- test/clang-rename/FunctionWithClassFindByName.cpp
+++ test/clang-rename/FunctionWithClassFindByName.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-rename -old-name=Foo -new-name=Bar %s -- | FileCheck %s
+// RUN: clang-rename rename-all -old-name=Foo -new-name=Bar %s -- | FileCheck %s
 
 void foo() {
 }
Index: test/clang-rename/ClassTestMultiByName.cpp
===
--- /dev/null
+++ test/clang-rename/ClassTestMultiByName.cpp
@@ -0,0 +1,6 @@
+// RUN: clang-rename rename-all -old-name=Cla1 -new-name=Kla1 -old-name=Cla2 -new-name=Kla2 %s -- | FileCheck %s
+class Cla1 { // CHECK: class Kla1
+};
+
+class Cla2 { // CHECK: class Kla2
+};
Index: test/clang-rename/ClassTestMulti.cpp
===
--- /dev/null
+++ test/clang-rename/ClassTestMulti.cpp
@@ -0,0 +1,6 @@
+// RUN: clang-rename rename-all -offset=113 -new-name=Kla1 -offset=151 -new-name=Kla2 %s -- | FileCheck %s
+class Cla1 { // CHECK: class Kla1
+};
+
+class Cla2 { // CHECK: class Kla2
+};
Index: test/clang-rename/ClassFindByName.cpp
===
--- test/clang-rename/ClassFindByName.cpp
+++ test/clang-rename/ClassFindByName.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-rename -old-name=Foo -new-name=Bar %s -- | FileCheck %s
+// RUN: clang-rename rename-all -old-name=Foo -new-name=Bar %s -- | FileCheck %s
 
 class Foo { // CHECK: class Bar
 };
Index: docs/clang-rename.rst
===
--- docs/clang-rename.rst
+++ docs/clang-rename.rst
@@ -42,6 +42,14 @@
   $ grep -FUbo 'foo' file.cpp
 
 
+You can also identify one or more symbols to be renamed by giving the fully qualified
+name:
+
+.. code-block:: console
+
+  $ clang-rename rename-all -old-name=foo -new-name=bar test.cpp
+
+
 The tool currently supports renaming actions inside a single Translation Unit
 only. It is planned to extend the tool's functionality to support multi-TU
 renaming actions in the future.
@@ -55,34 +63,73 @@
 .. code-block:: console
 
   $ clang-rename -help
+  Usage: clang-rename {rename-at|rename-all} [OPTION]...
+
+  A tool to rename symbols in C/C++ code.
+
+  Subcommands:
+rename-at:  Perform rename off of a location in a file. (This is the default.)
+rename-all: Perform rename of all symbols matching one or more fully qualified names.
+
+
+.. code-block:: console
+
+  $ clang-rename rename-at -help
   OVERVIEW: A tool to rename symbols in C/C++ code.
   clang-rename renames every occurrence of a symbol found at  in
   . If -i is specified, the edited files are overwritten to disk.
   Otherwise, the results are written to stdout.
-
-  USAGE: clang-rename [subcommand] [options]  [... ]
-
+  
+  USAGE: clang-rename rename-at [subcommand] [options]  [... ]
+  
   OPTIONS:
+  
+  Generic Options:
+  
+-help  - Display available options (-help-hidden for more)
+-help-list - Display list of available options (-help-list-hidden for more)
+-version   - Display the version of this program
 
-  Clang-rename options:
+  clang-rename rename-at options:
 
 -export-fixes=   - YAML file to store suggested fixes in.
 -extra-arg=- Additional argument to append to the compiler command line
 -extra-arg-before= - Additional argument to prepend to the compiler command line
 -i - Overwrite edited s.
 -new-name= - The new name to change the symbol to.
 -offset= - Locates the symbol by offset as opposed to :.
--old-name= - The fully qualified name of the symbol, if -offset is not used.
 -p=- Build path
 -pl- Print the locations affected by renaming to stderr.
 -pn- Print the found symbol's name prior to renaming to stderr.
 
+
+.. code-block:: console
+
+  $ ~/git/llvm/workdir/bin/c

Re: [PATCH] D21814: clang-rename: split existing options into two new subcommands

2016-07-29 Thread Miklos Vajna via cfe-commits
vmiklos added a subscriber: Eugene.Zelenko.
vmiklos added a comment.

> 1. Run `clang-format` or something, 80 char width limit is broken in 
> `tool/ClangRename.cpp` dozen of times.


Done. I was afraid doing that, due to the changes not related to my patch, but
the result doesn't seem to be too bad. I guess in a later patch it would be
great to run clang-format on the whole clang-rename code, then all contributors
could just run clang-format before committing in case LLVM coding style is not
in our muscle memory. :)

> 2. Only do `outs() << "abcd\n" << "efgh\n"` if you have something in between, 
> which can not be predefined. I.e. if you have ``` /// \brief Top level help. 
> static int helpMain(int argc, const char *argv[]) { errs() << "Usage: 
> clang-rename {rename-at|rename-all} [OPTION]...\n\n" << "A tool to rename 
> symbols in C/C++ code.\n\n" << "Subcommands:\n" << "  rename-at:  Perform 
> rename off of a location in a file. (This is the default.)\n" << "  
> rename-all: Perform rename of all symbols matching one or more fully 
> qualified names.\n"; return 0; } ``` Just make it a const string, isn't it 
> better?


Done. I copied llvm-cov, but I have no problems changing it.

> 3. `tooling::RefactoringTool` takes care of printing version IIUC, no need to 
> do that in a custom way (we don't have custom version in `clang-rename` head 
> at the moment, that was something @Eugene.Zelenko sent recently).


Indeed, removed.

> 4. `clang-rename/RenamingAction.h` -> `USRList` -> `USRs` or the other way 
> around everywhere. So far single naming convention feels right to me (I 
> personally prefer `*s` over `*List`). LLVM Coding Style 
> 
>  does, too, from what I understand. Unless it's `*Set`, which is pretty much 
> different thing.


I've changed NewNameList and PrevNameList.

USRList refers to a list of lists, i.e. doing one oldname->newname rename may
have to deal with multiple USRs, and when doing multiple oldname->newname
renames, you need to deal with a list of list of USRs. I used the List suffix
in this "list of list" case. I have no problem renaming
`std::vector> USRList` to USRs, but then we need a new
name for `std::vector USRs`.

> 5. Do we really need to dispatch these functions this way? with

> 

>   ``` enum RenameSubcommand { RenameAt, RenameAll }; ``` And many other 
> strange things. Just pass `bool isMultipleRename` or `isRenameAll` to `main` 
> routine instead of creating `enum`. We only have two such options here, 
> right? I pray we won't have more.


I promise I don't plan to suggest more. :) Changed the enum to a bool.

> 6. Move `int main(int argc, const char **argv)` upwards, so that it's above 
> dispatched routines. Otherwise when one wants to see what's going on, he sees 
> subroutine first without understanding where it comes from. Other way around 
> feels more intuitive to me.


Done.


https://reviews.llvm.org/D21814



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


Re: r276508 - Add -fmodules-ts flag to cc1 for the provisional C++ modules TS, and mark

2016-07-29 Thread Vassil Vassilev via cfe-commits

On 23/07/16 04:32, Richard Smith via cfe-commits wrote:

Author: rsmith
Date: Fri Jul 22 21:32:21 2016
New Revision: 276508

URL: http://llvm.org/viewvc/llvm-project?rev=276508&view=rev
Log:
Add -fmodules-ts flag to cc1 for the provisional C++ modules TS, and mark
'module' and 'import' as keywords when the flag is specified.

Added:
 cfe/trunk/test/Lexer/modules-ts.cpp
Modified:
 cfe/trunk/include/clang/Basic/LangOptions.def
 cfe/trunk/include/clang/Basic/TokenKinds.def
 cfe/trunk/include/clang/Driver/CC1Options.td
 cfe/trunk/lib/Basic/IdentifierTable.cpp
 cfe/trunk/lib/Frontend/CompilerInvocation.cpp

Modified: cfe/trunk/include/clang/Basic/LangOptions.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.def?rev=276508&r1=276507&r2=276508&view=diff
==
--- cfe/trunk/include/clang/Basic/LangOptions.def (original)
+++ cfe/trunk/include/clang/Basic/LangOptions.def Fri Jul 22 21:32:21 2016
@@ -142,6 +142,7 @@ BENIGN_LANGOPT(EmitAllDecls  , 1, 0,
  LANGOPT(MathErrno , 1, 1, "errno in math functions")
  BENIGN_LANGOPT(HeinousExtensions , 1, 0, "extensions that we really don't like and 
may be ripped out at any time")
  LANGOPT(Modules   , 1, 0, "modules extension to C")
+LANGOPT(ModulesTS , 1, 0, "C++ Modules TS")
  BENIGN_LANGOPT(CompilingModule, 1, 0, "compiling a module interface")
  COMPATIBLE_LANGOPT(ModulesDeclUse, 1, 0, "require declaration of module 
uses")
  BENIGN_LANGOPT(ModulesSearchAll  , 1, 1, "searching even non-imported modules to 
find unresolved references")

Modified: cfe/trunk/include/clang/Basic/TokenKinds.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/TokenKinds.def?rev=276508&r1=276507&r2=276508&view=diff
==
--- cfe/trunk/include/clang/Basic/TokenKinds.def (original)
+++ cfe/trunk/include/clang/Basic/TokenKinds.def Fri Jul 22 21:32:21 2016
@@ -30,6 +30,9 @@
  #ifndef CONCEPTS_KEYWORD
  #define CONCEPTS_KEYWORD(X) KEYWORD(X,KEYCONCEPTS)
  #endif
+#ifndef MODULES_KEYWORD
+#define MODULES_KEYWORD(X) KEYWORD(X,KEYMODULES)
+#endif
  #ifndef TYPE_TRAIT
  #define TYPE_TRAIT(N,I,K) KEYWORD(I,K)
  #endif
@@ -235,6 +238,8 @@ PUNCTUATOR(caretcaret,"^^")
  //   KEYCXX11 - This is a C++ keyword introduced to C++ in C++11
  //   KEYCONCEPTS - This is a keyword if the C++ extensions for concepts
  // are enabled.
+//   KEYMODULES - This is a keyword if the C++ extensions for modules
+//are enabled.
  //   KEYGNU   - This is a keyword if GNU extensions are enabled
  //   KEYMS- This is a keyword if Microsoft extensions are enabled
  //   KEYNOMS18 - This is a keyword that must never be enabled under
@@ -366,6 +371,10 @@ KEYWORD(co_await, KE
  KEYWORD(co_return   , KEYCOROUTINES)
  KEYWORD(co_yield, KEYCOROUTINES)
  
+// C++ modules TS keywords

+MODULES_KEYWORD(module)
+MODULES_KEYWORD(import)
+
  // GNU Extensions (in impl-reserved namespace)
  KEYWORD(_Decimal32  , KEYALL)
  KEYWORD(_Decimal64  , KEYALL)

Modified: cfe/trunk/include/clang/Driver/CC1Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=276508&r1=276507&r2=276508&view=diff
==
--- cfe/trunk/include/clang/Driver/CC1Options.td (original)
+++ cfe/trunk/include/clang/Driver/CC1Options.td Fri Jul 22 21:32:21 2016
@@ -388,6 +388,8 @@ def ast_dump_filter : Separate<["-"], "a
HelpText<"Use with -ast-dump or -ast-print to dump/print only AST 
declaration"
 " nodes having a certain substring in a qualified name. Use"
 " -ast-list to list all filterable declaration node names.">;
+def fmodules_ts : Flag <["-"], "fmodules-ts">, Group,
+  HelpText<"Enable support for the C++ Modules TS">;
  def fno_modules_global_index : Flag<["-"], "fno-modules-global-index">,
HelpText<"Do not automatically generate or update the global module index">;
  def fno_modules_error_recovery : Flag<["-"], "fno-modules-error-recovery">,

Modified: cfe/trunk/lib/Basic/IdentifierTable.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/IdentifierTable.cpp?rev=276508&r1=276507&r2=276508&view=diff
==
--- cfe/trunk/lib/Basic/IdentifierTable.cpp (original)
+++ cfe/trunk/lib/Basic/IdentifierTable.cpp Fri Jul 22 21:32:21 2016
@@ -113,7 +113,8 @@ namespace {
  KEYOBJC2= 0x2,
  KEYZVECTOR  = 0x4,
  KEYCOROUTINES = 0x8,
-KEYALL = (0xf & ~KEYNOMS18 &
+KEYMODULES = 0x10,
+KEYALL = (0x1f & ~KEYNOMS18 &
~KEYNOOPENCL) // KEYNOMS18 and KEYNOOPENCL are used to exclude.
};
  
@@ -1

Re: [PATCH] D21814: clang-rename: split existing options into two new subcommands

2016-07-29 Thread Kirill Bobyrev via cfe-commits
omtcyfz added a comment.

In https://reviews.llvm.org/D21814#500481, @vmiklos wrote:

> > 1. Run `clang-format` or something, 80 char width limit is broken in 
> > `tool/ClangRename.cpp` dozen of times.
>
>
> Done. I was afraid doing that, due to the changes not related to my patch, but
>  the result doesn't seem to be too bad. I guess in a later patch it would be
>  great to run clang-format on the whole clang-rename code, then all 
> contributors
>  could just run clang-format before committing in case LLVM coding style is 
> not
>  in our muscle memory. :)


Yes, but I don't think there are many locations where `clang-format` would be 
triggered. I believe I ran clang-format over most parts few times.

> 

> 

> > 2. Only do `outs() << "abcd\n" << "efgh\n"` if you have something in 
> > between, which can not be predefined. I.e. if you have ``` /// \brief Top 
> > level help. static int helpMain(int argc, const char *argv[]) { errs() << 
> > "Usage: clang-rename {rename-at|rename-all} [OPTION]...\n\n" << "A tool to 
> > rename symbols in C/C++ code.\n\n" << "Subcommands:\n" << "  rename-at:  
> > Perform rename off of a location in a file. (This is the default.)\n" << "  
> > rename-all: Perform rename of all symbols matching one or more fully 
> > qualified names.\n"; return 0; } ``` Just make it a const string, isn't it 
> > better?

> 

> 

> Done. I copied llvm-cov, but I have no problems changing it.


Cool.

> 

> 

> > 3. `tooling::RefactoringTool` takes care of printing version IIUC, no need 
> > to do that in a custom way (we don't have custom version in `clang-rename` 
> > head at the moment, that was something Eugene.Zelenko sent recently).

> 

> 

> 


Removed @; otherwise Eugene becomes subscribed :)

> Indeed, removed.

> 

> > 4. `clang-rename/RenamingAction.h` -> `USRList` -> `USRs` or the other way 
> > around everywhere. So far single naming convention feels right to me (I 
> > personally prefer `*s` over `*List`). LLVM Coding Style 
> > 
> >  does, too, from what I understand. Unless it's `*Set`, which is pretty 
> > much different thing.

> 

> 

> I've changed NewNameList and PrevNameList.

> 

> USRList refers to a list of lists, i.e. doing one oldname->newname rename may

>  have to deal with multiple USRs, and when doing multiple oldname->newname

>  renames, you need to deal with a list of list of USRs. I used the List suffix

>  in this "list of list" case. I have no problem renaming

>  `std::vector> USRList` to USRs, but then we need a 
> new

>  name for `std::vector USRs`.


Aha, I see. Yes, for `vector>` it seems reasonable. Sorry, didn't that 
it's `vector>`.

> 

> 

> > 5. Do we really need to dispatch these functions this way? with

> 

> > 

> 

> >   ``` enum RenameSubcommand { RenameAt, RenameAll }; ``` And many other 
> > strange things. Just pass `bool isMultipleRename` or `isRenameAll` to 
> > `main` routine instead of creating `enum`. We only have two such options 
> > here, right? I pray we won't have more.

> 

> 

> I promise I don't plan to suggest more. :)


//looking into your honest eyes//

> Changed the enum to a bool.

> 

> > 6. Move `int main(int argc, const char **argv)` upwards, so that it's above 
> > dispatched routines. Otherwise when one wants to see what's going on, he 
> > sees subroutine first without understanding where it comes from. Other way 
> > around feels more intuitive to me.

> 

> 

> Done.


As for help message, look at `clang-tidy`. Is there a need in `helpMain`?


https://reviews.llvm.org/D21814



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


Re: [PATCH] D21814: clang-rename: split existing options into two new subcommands

2016-07-29 Thread Kirill Bobyrev via cfe-commits
omtcyfz added a comment.

besides, let me push one thing; it's about passing a vector of USRs to the 
USRLocFinder instead of passing them 1 by 1; removes a need to write that 
`FIXME` of yours :)


https://reviews.llvm.org/D21814



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


Re: [PATCH] D21814: clang-rename: split existing options into two new subcommands

2016-07-29 Thread Kirill Bobyrev via cfe-commits
omtcyfz added a comment.

Hm, nevermind, I should check whether it doesn't break anything.


https://reviews.llvm.org/D21814



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


[clang-tools-extra] r277131 - [clang-rename] speedup RenamingAction

2016-07-29 Thread Kirill Bobyrev via cfe-commits
Author: omtcyfz
Date: Fri Jul 29 05:16:45 2016
New Revision: 277131

URL: http://llvm.org/viewvc/llvm-project?rev=277131&view=rev
Log:
[clang-rename] speedup RenamingAction

The complexity of renaming a USR is O(N) [N stands for number of nodes in
Translation Unit]. In some cases there are more than one USR for a single symbol
(see overridden functions and ctor/dtor handling), which means that the
complexity of finding all of the corresponding USRs is O(N * M) [M stands for
number of USRs corresponding to the symbols, which may be not quite small]. With
a simple tweak we can make it O(N * log(M)) by passing whole list of USRs
corresponding to the symbol to USRLocFinder.

Modified:
clang-tools-extra/trunk/clang-rename/RenamingAction.cpp
clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp
clang-tools-extra/trunk/clang-rename/USRLocFinder.h

Modified: clang-tools-extra/trunk/clang-rename/RenamingAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-rename/RenamingAction.cpp?rev=277131&r1=277130&r2=277131&view=diff
==
--- clang-tools-extra/trunk/clang-rename/RenamingAction.cpp (original)
+++ clang-tools-extra/trunk/clang-rename/RenamingAction.cpp Fri Jul 29 05:16:45 
2016
@@ -34,27 +34,21 @@ namespace rename {
 
 class RenamingASTConsumer : public ASTConsumer {
 public:
-  RenamingASTConsumer(const std::string &NewName,
-  const std::string &PrevName,
+  RenamingASTConsumer(const std::string &NewName, const std::string &PrevName,
   const std::vector &USRs,
-  tooling::Replacements &Replaces,
-  bool PrintLocations)
+  tooling::Replacements &Replaces, bool PrintLocations)
   : NewName(NewName), PrevName(PrevName), USRs(USRs), Replaces(Replaces),
-PrintLocations(PrintLocations) {
-  }
+PrintLocations(PrintLocations) {}
 
   void HandleTranslationUnit(ASTContext &Context) override {
 const auto &SourceMgr = Context.getSourceManager();
 std::vector RenamingCandidates;
 std::vector NewCandidates;
 
-for (const auto &USR : USRs) {
-  NewCandidates = getLocationsOfUSR(USR, PrevName,
-Context.getTranslationUnitDecl());
-  RenamingCandidates.insert(RenamingCandidates.end(), 
NewCandidates.begin(),
-NewCandidates.end());
-  NewCandidates.clear();
-}
+NewCandidates =
+getLocationsOfUSRs(USRs, PrevName, Context.getTranslationUnitDecl());
+RenamingCandidates.insert(RenamingCandidates.end(), NewCandidates.begin(),
+  NewCandidates.end());
 
 auto PrevNameLen = PrevName.length();
 for (const auto &Loc : RenamingCandidates) {
@@ -64,8 +58,8 @@ public:
<< ":" << FullLoc.getSpellingLineNumber() << ":"
<< FullLoc.getSpellingColumnNumber() << "\n";
   }
-  Replaces.insert(tooling::Replacement(SourceMgr, Loc, PrevNameLen,
-   NewName));
+  Replaces.insert(
+  tooling::Replacement(SourceMgr, Loc, PrevNameLen, NewName));
 }
   }
 

Modified: clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp?rev=277131&r1=277130&r2=277131&view=diff
==
--- clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp (original)
+++ clang-tools-extra/trunk/clang-rename/USRLocFinder.cpp Fri Jul 29 05:16:45 
2016
@@ -34,9 +34,11 @@ namespace {
 class USRLocFindingASTVisitor
 : public clang::RecursiveASTVisitor {
 public:
-  explicit USRLocFindingASTVisitor(StringRef USR, StringRef PrevName,
+  explicit USRLocFindingASTVisitor(const std::vector &USRs,
+   StringRef PrevName,
const ASTContext &Context)
-  : USR(USR), PrevName(PrevName), Context(Context) {}
+  : USRSet(USRs.begin(), USRs.end()), PrevName(PrevName), Context(Context) 
{
+  }
 
   // Declaration visitors:
 
@@ -47,7 +49,7 @@ public:
 continue;
   }
   if (const clang::FieldDecl *FieldDecl = Initializer->getAnyMember()) {
-if (getUSRForDecl(FieldDecl) == USR) {
+if (USRSet.find(getUSRForDecl(FieldDecl)) != USRSet.end()) {
   // The initializer refers to a field that is to be renamed.
   SourceLocation Location = Initializer->getSourceLocation();
   StringRef TokenName = Lexer::getSourceText(
@@ -65,7 +67,7 @@ public:
   }
 
   bool VisitNamedDecl(const NamedDecl *Decl) {
-if (getUSRForDecl(Decl) == USR) {
+if (USRSet.find(getUSRForDecl(Decl)) != USRSet.end()) {
   checkAndAddLocation(Decl->getLocation());
 }
 return true;
@@ -76,7 +78,7 @@ public:
   bool VisitDeclRefExpr(const DeclRefExpr *Exp

Re: [PATCH] D21814: clang-rename: split existing options into two new subcommands

2016-07-29 Thread Kirill Bobyrev via cfe-commits
omtcyfz added a comment.

Apparently it doesn't. Pushed to upstream.


https://reviews.llvm.org/D21814



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


r277134 - [GCC] Support for __final specifier

2016-07-29 Thread Andrey Bokhanko via cfe-commits
Author: asbokhan
Date: Fri Jul 29 05:42:48 2016
New Revision: 277134

URL: http://llvm.org/viewvc/llvm-project?rev=277134&view=rev
Log:
[GCC] Support for __final specifier

As reported in bug 28473, GCC supports "final" functionality in pre-C++11 code 
using the __final keyword. Clang currently supports the "final" keyword in 
accordance with the C++11 specification, however it ALSO supports it in 
pre-C++11 mode, with a warning.

This patch adds the "__final" keyword for compatibility with GCC in GCC 
Keywords mode (so it is enabled with existing flags), and issues a warning on 
its usage (suggesting switching to the C++11 keyword). This patch also adds a 
regression test for the functionality described. I believe this patch has 
minimal impact, as it simply adds a new keyword for existing behavior.

This has been validated with check-clang to avoid regressions. Patch is created 
in reference to revisions 276665.

Patch by Erich Keane.

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

Added:
cfe/trunk/test/Parser/gcc-__final-compatibility.cpp
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Parse/Parser.h
cfe/trunk/include/clang/Sema/DeclSpec.h
cfe/trunk/lib/Parse/ParseDeclCXX.cpp
cfe/trunk/lib/Sema/DeclSpec.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=277134&r1=277133&r2=277134&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Jul 29 05:42:48 
2016
@@ -8652,4 +8652,8 @@ def warn_block_literal_qualifiers_on_omi
   "'%0' qualifier on omitted return type %1 has no effect">,
   InGroup;
 
+def ext_warn_gnu_final : ExtWarn<
+  "__final is a GNU extension, consider using C++11 final">,
+  InGroup;
+
 } // end of sema component.

Modified: cfe/trunk/include/clang/Parse/Parser.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=277134&r1=277133&r2=277134&view=diff
==
--- cfe/trunk/include/clang/Parse/Parser.h (original)
+++ cfe/trunk/include/clang/Parse/Parser.h Fri Jul 29 05:42:48 2016
@@ -143,6 +143,7 @@ class Parser : public CodeCompletionHand
 
   /// C++0x contextual keywords.
   mutable IdentifierInfo *Ident_final;
+  mutable IdentifierInfo *Ident_GNU_final;
   mutable IdentifierInfo *Ident_override;
 
   // C++ type trait keywords that can be reverted to identifiers and still be

Modified: cfe/trunk/include/clang/Sema/DeclSpec.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/DeclSpec.h?rev=277134&r1=277133&r2=277134&view=diff
==
--- cfe/trunk/include/clang/Sema/DeclSpec.h (original)
+++ cfe/trunk/include/clang/Sema/DeclSpec.h Fri Jul 29 05:42:48 2016
@@ -2399,7 +2399,9 @@ public:
 VS_None = 0,
 VS_Override = 1,
 VS_Final = 2,
-VS_Sealed = 4
+VS_Sealed = 4,
+// Represents the __final keyword, which is legal for gcc in pre-C++11 
mode.
+VS_GNU_Final = 8
   };
 
   VirtSpecifiers() : Specifiers(0), LastSpecifier(VS_None) { }
@@ -2412,7 +2414,7 @@ public:
   bool isOverrideSpecified() const { return Specifiers & VS_Override; }
   SourceLocation getOverrideLoc() const { return VS_overrideLoc; }
 
-  bool isFinalSpecified() const { return Specifiers & (VS_Final | VS_Sealed); }
+  bool isFinalSpecified() const { return Specifiers & (VS_Final | VS_Sealed | 
VS_GNU_Final); }
   bool isFinalSpelledSealed() const { return Specifiers & VS_Sealed; }
   SourceLocation getFinalLoc() const { return VS_finalLoc; }
 

Modified: cfe/trunk/lib/Parse/ParseDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDeclCXX.cpp?rev=277134&r1=277133&r2=277134&view=diff
==
--- cfe/trunk/lib/Parse/ParseDeclCXX.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp Fri Jul 29 05:42:48 2016
@@ -2005,6 +2005,7 @@ void Parser::HandleMemberFunctionDeclDel
 ///   virt-specifier:
 /// override
 /// final
+/// __final
 VirtSpecifiers::Specifier Parser::isCXX11VirtSpecifier(const Token &Tok) const 
{
   if (!getLangOpts().CPlusPlus || Tok.isNot(tok::identifier))
 return VirtSpecifiers::VS_None;
@@ -2014,6 +2015,8 @@ VirtSpecifiers::Specifier Parser::isCXX1
   // Initialize the contextual keywords.
   if (!Ident_final) {
 Ident_final = &PP.getIdentifierTable().get("final");
+if (getLangOpts().GNUKeywords)
+  Ident_GNU_final = &PP.getIdentifierTable().get("__final");
 if (getLangOpts().MicrosoftExt)
   Ident_sealed = &PP.getIdentifierTable().get("sealed");
 Ident_override = &PP.getIdentifierTable().get("ov

Re: [PATCH] D22919: GCC Compatibility: Support for __final specifier

2016-07-29 Thread Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL277134: [GCC] Support for __final specifier (authored by 
asbokhan).

Changed prior to commit:
  https://reviews.llvm.org/D22919?vs=65951&id=66104#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D22919

Files:
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/include/clang/Parse/Parser.h
  cfe/trunk/include/clang/Sema/DeclSpec.h
  cfe/trunk/lib/Parse/ParseDeclCXX.cpp
  cfe/trunk/lib/Sema/DeclSpec.cpp
  cfe/trunk/test/Parser/gcc-__final-compatibility.cpp

Index: cfe/trunk/include/clang/Sema/DeclSpec.h
===
--- cfe/trunk/include/clang/Sema/DeclSpec.h
+++ cfe/trunk/include/clang/Sema/DeclSpec.h
@@ -2399,7 +2399,9 @@
 VS_None = 0,
 VS_Override = 1,
 VS_Final = 2,
-VS_Sealed = 4
+VS_Sealed = 4,
+// Represents the __final keyword, which is legal for gcc in pre-C++11 mode.
+VS_GNU_Final = 8
   };
 
   VirtSpecifiers() : Specifiers(0), LastSpecifier(VS_None) { }
@@ -2412,7 +2414,7 @@
   bool isOverrideSpecified() const { return Specifiers & VS_Override; }
   SourceLocation getOverrideLoc() const { return VS_overrideLoc; }
 
-  bool isFinalSpecified() const { return Specifiers & (VS_Final | VS_Sealed); }
+  bool isFinalSpecified() const { return Specifiers & (VS_Final | VS_Sealed | VS_GNU_Final); }
   bool isFinalSpelledSealed() const { return Specifiers & VS_Sealed; }
   SourceLocation getFinalLoc() const { return VS_finalLoc; }
 
Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8652,4 +8652,8 @@
   "'%0' qualifier on omitted return type %1 has no effect">,
   InGroup;
 
+def ext_warn_gnu_final : ExtWarn<
+  "__final is a GNU extension, consider using C++11 final">,
+  InGroup;
+
 } // end of sema component.
Index: cfe/trunk/include/clang/Parse/Parser.h
===
--- cfe/trunk/include/clang/Parse/Parser.h
+++ cfe/trunk/include/clang/Parse/Parser.h
@@ -143,6 +143,7 @@
 
   /// C++0x contextual keywords.
   mutable IdentifierInfo *Ident_final;
+  mutable IdentifierInfo *Ident_GNU_final;
   mutable IdentifierInfo *Ident_override;
 
   // C++ type trait keywords that can be reverted to identifiers and still be
Index: cfe/trunk/test/Parser/gcc-__final-compatibility.cpp
===
--- cfe/trunk/test/Parser/gcc-__final-compatibility.cpp
+++ cfe/trunk/test/Parser/gcc-__final-compatibility.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -std=c++98 -fgnu-keywords -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++11 -fgnu-keywords -fsyntax-only -verify %s
+
+struct B {
+  virtual void g();
+};
+struct D __final : B { // expected-warning {{__final is a GNU extension, consider using C++11 final}}
+  virtual void g() __final; // expected-warning {{__final is a GNU extension, consider using C++11 final}}
+};
Index: cfe/trunk/lib/Sema/DeclSpec.cpp
===
--- cfe/trunk/lib/Sema/DeclSpec.cpp
+++ cfe/trunk/lib/Sema/DeclSpec.cpp
@@ -1299,6 +1299,7 @@
   switch (VS) {
   default: llvm_unreachable("Unknown specifier!");
   case VS_Override: VS_overrideLoc = Loc; break;
+  case VS_GNU_Final:
   case VS_Sealed:
   case VS_Final:VS_finalLoc = Loc; break;
   }
@@ -1311,6 +1312,7 @@
   default: llvm_unreachable("Unknown specifier");
   case VS_Override: return "override";
   case VS_Final: return "final";
+  case VS_GNU_Final: return "__final";
   case VS_Sealed: return "sealed";
   }
 }
Index: cfe/trunk/lib/Parse/ParseDeclCXX.cpp
===
--- cfe/trunk/lib/Parse/ParseDeclCXX.cpp
+++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp
@@ -2005,6 +2005,7 @@
 ///   virt-specifier:
 /// override
 /// final
+/// __final
 VirtSpecifiers::Specifier Parser::isCXX11VirtSpecifier(const Token &Tok) const {
   if (!getLangOpts().CPlusPlus || Tok.isNot(tok::identifier))
 return VirtSpecifiers::VS_None;
@@ -2014,6 +2015,8 @@
   // Initialize the contextual keywords.
   if (!Ident_final) {
 Ident_final = &PP.getIdentifierTable().get("final");
+if (getLangOpts().GNUKeywords)
+  Ident_GNU_final = &PP.getIdentifierTable().get("__final");
 if (getLangOpts().MicrosoftExt)
   Ident_sealed = &PP.getIdentifierTable().get("sealed");
 Ident_override = &PP.getIdentifierTable().get("override");
@@ -2028,6 +2031,9 @@
   if (II == Ident_final)
 return VirtSpecifiers::VS_Final;
 
+  if (II == Ident_GNU_final)
+return VirtSpecifiers::VS_GNU_Final;
+
   return VirtSpecifiers::VS_None;
 }
 
@@ -2067,6 +2073,8 @@
 << VirtSpecifiers::getSpecifierName(Specifier);
 } else if (Specifier == Vi

Re: [PATCH] D21814: clang-rename: split existing options into two new subcommands

2016-07-29 Thread Miklos Vajna via cfe-commits
vmiklos updated this revision to Diff 66105.

https://reviews.llvm.org/D21814

Files:
  clang-rename/RenamingAction.cpp
  clang-rename/RenamingAction.h
  clang-rename/tool/ClangRename.cpp
  docs/clang-rename.rst
  test/clang-rename/ClassFindByName.cpp
  test/clang-rename/ClassTestMulti.cpp
  test/clang-rename/ClassTestMultiByName.cpp
  test/clang-rename/FunctionWithClassFindByName.cpp
  test/clang-rename/NoNewName.cpp

Index: test/clang-rename/NoNewName.cpp
===
--- test/clang-rename/NoNewName.cpp
+++ test/clang-rename/NoNewName.cpp
@@ -1,4 +1,4 @@
 // Check for an error while -new-name argument has not been passed to
 // clang-rename.
 // RUN: not clang-rename -offset=133 %s 2>&1 | FileCheck %s
-// CHECK: ERROR: no new name provided.
+// CHECK: clang-rename: for the -new-name option: must be specified
Index: test/clang-rename/FunctionWithClassFindByName.cpp
===
--- test/clang-rename/FunctionWithClassFindByName.cpp
+++ test/clang-rename/FunctionWithClassFindByName.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-rename -old-name=Foo -new-name=Bar %s -- | FileCheck %s
+// RUN: clang-rename rename-all -old-name=Foo -new-name=Bar %s -- | FileCheck %s
 
 void foo() {
 }
Index: test/clang-rename/ClassTestMultiByName.cpp
===
--- /dev/null
+++ test/clang-rename/ClassTestMultiByName.cpp
@@ -0,0 +1,6 @@
+// RUN: clang-rename rename-all -old-name=Cla1 -new-name=Kla1 -old-name=Cla2 -new-name=Kla2 %s -- | FileCheck %s
+class Cla1 { // CHECK: class Kla1
+};
+
+class Cla2 { // CHECK: class Kla2
+};
Index: test/clang-rename/ClassTestMulti.cpp
===
--- /dev/null
+++ test/clang-rename/ClassTestMulti.cpp
@@ -0,0 +1,6 @@
+// RUN: clang-rename rename-all -offset=113 -new-name=Kla1 -offset=151 -new-name=Kla2 %s -- | FileCheck %s
+class Cla1 { // CHECK: class Kla1
+};
+
+class Cla2 { // CHECK: class Kla2
+};
Index: test/clang-rename/ClassFindByName.cpp
===
--- test/clang-rename/ClassFindByName.cpp
+++ test/clang-rename/ClassFindByName.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-rename -old-name=Foo -new-name=Bar %s -- | FileCheck %s
+// RUN: clang-rename rename-all -old-name=Foo -new-name=Bar %s -- | FileCheck %s
 
 class Foo { // CHECK: class Bar
 };
Index: docs/clang-rename.rst
===
--- docs/clang-rename.rst
+++ docs/clang-rename.rst
@@ -42,6 +42,14 @@
   $ grep -FUbo 'foo' file.cpp
 
 
+You can also identify one or more symbols to be renamed by giving the fully qualified
+name:
+
+.. code-block:: console
+
+  $ clang-rename rename-all -old-name=foo -new-name=bar test.cpp
+
+
 The tool currently supports renaming actions inside a single Translation Unit
 only. It is planned to extend the tool's functionality to support multi-TU
 renaming actions in the future.
@@ -55,34 +63,73 @@
 .. code-block:: console
 
   $ clang-rename -help
+  Usage: clang-rename {rename-at|rename-all} [OPTION]...
+
+  A tool to rename symbols in C/C++ code.
+
+  Subcommands:
+rename-at:  Perform rename off of a location in a file. (This is the default.)
+rename-all: Perform rename of all symbols matching one or more fully qualified names.
+
+
+.. code-block:: console
+
+  $ clang-rename rename-at -help
   OVERVIEW: A tool to rename symbols in C/C++ code.
   clang-rename renames every occurrence of a symbol found at  in
   . If -i is specified, the edited files are overwritten to disk.
   Otherwise, the results are written to stdout.
-
-  USAGE: clang-rename [subcommand] [options]  [... ]
-
+  
+  USAGE: clang-rename rename-at [subcommand] [options]  [... ]
+  
   OPTIONS:
+  
+  Generic Options:
+  
+-help  - Display available options (-help-hidden for more)
+-help-list - Display list of available options (-help-list-hidden for more)
+-version   - Display the version of this program
 
-  Clang-rename options:
+  clang-rename rename-at options:
 
 -export-fixes=   - YAML file to store suggested fixes in.
 -extra-arg=- Additional argument to append to the compiler command line
 -extra-arg-before= - Additional argument to prepend to the compiler command line
 -i - Overwrite edited s.
 -new-name= - The new name to change the symbol to.
 -offset= - Locates the symbol by offset as opposed to :.
--old-name= - The fully qualified name of the symbol, if -offset is not used.
 -p=- Build path
 -pl- Print the locations affected by renaming to stderr.
 -pn- Print the found symbol's name prior to renaming to stderr.
 
+
+.. code-block:: console
+
+  $ ~/git/llvm/workdir/bin/c

Re: [PATCH] D21814: clang-rename: split existing options into two new subcommands

2016-07-29 Thread Miklos Vajna via cfe-commits
vmiklos added a comment.

Rebased on top of r277131 and resolved conflicts.

> As for help message, look at clang-tidy. Is there a need in helpMain?


I think so; we have this chicken-and-egg problem (see earlier comments of this
review), that the options parser wants to know the option category, but we only
know the option category after parsing options due to subcommands. This is not
a problem for clang-tidy that doesn't have subcommands (as far as I see).

So one way is the own code for handling the subcommands (that's what I did
here, and that's what e.g. llvm-cov does), an other way would be to extend
`tooling::CommonOptionsParser`, so it doesn't want a category in the ctor. That
requirement is a problem for us, since we have two categories, so we can't give
the correct one without parsing the options.

So all in all, the best seems to me is to go with a simple helpMain().

> besides, let me push one thing; it's about passing a vector of USRs to the 
> USRLocFinder instead of passing them 1 by 1; removes a need to write that 
> FIXME of yours :)


Great, I've removed it then.


https://reviews.llvm.org/D21814



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


Re: [PATCH] D21814: clang-rename: split existing options into two new subcommands

2016-07-29 Thread Kirill Bobyrev via cfe-commits
omtcyfz added a comment.

In https://reviews.llvm.org/D21814#500506, @vmiklos wrote:

> Rebased on top of r277131 and resolved conflicts.
>
> > As for help message, look at clang-tidy. Is there a need in helpMain?
>
>
> I think so; we have this chicken-and-egg problem (see earlier comments of this
>  review), that the options parser wants to know the option category, but we 
> only
>  know the option category after parsing options due to subcommands. This is 
> not
>  a problem for clang-tidy that doesn't have subcommands (as far as I see).
>
> So one way is the own code for handling the subcommands (that's what I did
>  here, and that's what e.g. llvm-cov does), an other way would be to extend
>  `tooling::CommonOptionsParser`, so it doesn't want a category in the ctor. 
> That
>  requirement is a problem for us, since we have two categories, so we can't 
> give
>  the correct one without parsing the options.
>
> So all in all, the best seems to me is to go with a simple helpMain().


Aha. I see. Well, it's not clean, but we can probably just deal with it later. 
Otherwise it gets too long and the patch is never going to be landed. Just 
write a `FIXME` then, I think I may look into that on the next week or somewhen.

> 

> 

> > besides, let me push one thing; it's about passing a vector of USRs to the 
> > USRLocFinder instead of passing them 1 by 1; removes a need to write that 
> > FIXME of yours :)

> 

> 

> Great, I've removed it then.


One more paranoid comment:

`class Cla1 { // CHECK: class Kla1`

Most of the time I use `Foo`->`Bar` renaming in tests, just so that it could be 
uniform and everyone (including myself) would typically `grep -FUbo "Foo"` 
without looking at the source.

Other than that my concerns have been addressed.

Again, I'm not happy about this approach, but if that's a use-case we expect to 
have in the future and everyone is happy about it -  let's do that.


https://reviews.llvm.org/D21814



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


Re: [PATCH] D21814: clang-rename: split existing options into two new subcommands

2016-07-29 Thread Miklos Vajna via cfe-commits
vmiklos updated this revision to Diff 66110.

https://reviews.llvm.org/D21814

Files:
  clang-rename/RenamingAction.cpp
  clang-rename/RenamingAction.h
  clang-rename/tool/ClangRename.cpp
  docs/clang-rename.rst
  test/clang-rename/ClassFindByName.cpp
  test/clang-rename/ClassTestMulti.cpp
  test/clang-rename/ClassTestMultiByName.cpp
  test/clang-rename/FunctionWithClassFindByName.cpp
  test/clang-rename/NoNewName.cpp

Index: test/clang-rename/NoNewName.cpp
===
--- test/clang-rename/NoNewName.cpp
+++ test/clang-rename/NoNewName.cpp
@@ -1,4 +1,4 @@
 // Check for an error while -new-name argument has not been passed to
 // clang-rename.
 // RUN: not clang-rename -offset=133 %s 2>&1 | FileCheck %s
-// CHECK: ERROR: no new name provided.
+// CHECK: clang-rename: for the -new-name option: must be specified
Index: test/clang-rename/FunctionWithClassFindByName.cpp
===
--- test/clang-rename/FunctionWithClassFindByName.cpp
+++ test/clang-rename/FunctionWithClassFindByName.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-rename -old-name=Foo -new-name=Bar %s -- | FileCheck %s
+// RUN: clang-rename rename-all -old-name=Foo -new-name=Bar %s -- | FileCheck %s
 
 void foo() {
 }
Index: test/clang-rename/ClassTestMultiByName.cpp
===
--- /dev/null
+++ test/clang-rename/ClassTestMultiByName.cpp
@@ -0,0 +1,6 @@
+// RUN: clang-rename rename-all -old-name=Foo1 -new-name=Bar1 -old-name=Foo2 -new-name=Bar2 %s -- | FileCheck %s
+class Foo1 { // CHECK: class Bar1
+};
+
+class Foo2 { // CHECK: class Bar2
+};
Index: test/clang-rename/ClassTestMulti.cpp
===
--- /dev/null
+++ test/clang-rename/ClassTestMulti.cpp
@@ -0,0 +1,6 @@
+// RUN: clang-rename rename-all -offset=113 -new-name=Bar1 -offset=151 -new-name=Bar2 %s -- | FileCheck %s
+class Foo1 { // CHECK: class Bar1
+};
+
+class Foo2 { // CHECK: class Bar2
+};
Index: test/clang-rename/ClassFindByName.cpp
===
--- test/clang-rename/ClassFindByName.cpp
+++ test/clang-rename/ClassFindByName.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-rename -old-name=Foo -new-name=Bar %s -- | FileCheck %s
+// RUN: clang-rename rename-all -old-name=Foo -new-name=Bar %s -- | FileCheck %s
 
 class Foo { // CHECK: class Bar
 };
Index: docs/clang-rename.rst
===
--- docs/clang-rename.rst
+++ docs/clang-rename.rst
@@ -42,6 +42,14 @@
   $ grep -FUbo 'foo' file.cpp
 
 
+You can also identify one or more symbols to be renamed by giving the fully qualified
+name:
+
+.. code-block:: console
+
+  $ clang-rename rename-all -old-name=foo -new-name=bar test.cpp
+
+
 The tool currently supports renaming actions inside a single Translation Unit
 only. It is planned to extend the tool's functionality to support multi-TU
 renaming actions in the future.
@@ -55,34 +63,73 @@
 .. code-block:: console
 
   $ clang-rename -help
+  Usage: clang-rename {rename-at|rename-all} [OPTION]...
+
+  A tool to rename symbols in C/C++ code.
+
+  Subcommands:
+rename-at:  Perform rename off of a location in a file. (This is the default.)
+rename-all: Perform rename of all symbols matching one or more fully qualified names.
+
+
+.. code-block:: console
+
+  $ clang-rename rename-at -help
   OVERVIEW: A tool to rename symbols in C/C++ code.
   clang-rename renames every occurrence of a symbol found at  in
   . If -i is specified, the edited files are overwritten to disk.
   Otherwise, the results are written to stdout.
-
-  USAGE: clang-rename [subcommand] [options]  [... ]
-
+  
+  USAGE: clang-rename rename-at [subcommand] [options]  [... ]
+  
   OPTIONS:
+  
+  Generic Options:
+  
+-help  - Display available options (-help-hidden for more)
+-help-list - Display list of available options (-help-list-hidden for more)
+-version   - Display the version of this program
 
-  Clang-rename options:
+  clang-rename rename-at options:
 
 -export-fixes=   - YAML file to store suggested fixes in.
 -extra-arg=- Additional argument to append to the compiler command line
 -extra-arg-before= - Additional argument to prepend to the compiler command line
 -i - Overwrite edited s.
 -new-name= - The new name to change the symbol to.
 -offset= - Locates the symbol by offset as opposed to :.
--old-name= - The fully qualified name of the symbol, if -offset is not used.
 -p=- Build path
 -pl- Print the locations affected by renaming to stderr.
 -pn- Print the found symbol's name prior to renaming to stderr.
 
+
+.. code-block:: console
+
+  $ ~/git/llvm/workdir/bin/c

Re: [PATCH] D21814: clang-rename: split existing options into two new subcommands

2016-07-29 Thread Miklos Vajna via cfe-commits
vmiklos added a comment.

> Just write a FIXME then, I think I may look into that on the next week or 
> somewhen.


Done.

> Most of the time I use Foo->Bar renaming in tests


Done, I've renamed ClaN->KlaN to FooN->BarN.


https://reviews.llvm.org/D21814



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


Re: [PATCH] D22668: TrailingObjects::FixedSizeStorage constexpr fixes

2016-07-29 Thread Aaron Ballman via cfe-commits
aaron.ballman added a comment.

In https://reviews.llvm.org/D22668#500340, @hubert.reinterpretcast wrote:

> In https://reviews.llvm.org/D22668#499164, @aaron.ballman wrote:
>
> > I don't suppose there's a way to test these changes, is there?
>
>
> It's a utility class (which is not even used yet). I am not aware of testing 
> for the ADTs, etc. aside from using them internally. Perhaps I should mark 
> the change as NFC?


We typically test ADTs using unit tests. Check out the ADTTests project in 
llvm. I definitely would not mark this as NFC -- it has functional changes, 
they're just not in use yet. If you can devise some tests to add to the ADT 
unit tests, that would help build confidence that this functionality is ready 
to be used, and helps protect us against accidental regressions in behavior 
later. It also exercises these code paths on all the compilers we support, 
which is probably the most important bit since these changes are a reaction to 
compiler differences in constexpr support.



Comment at: include/llvm/Support/TrailingObjects.h:149
@@ -148,1 +148,3 @@
 
+  struct RequiresRealignment {
+static const bool value =

hubert.reinterpretcast wrote:
> aaron.ballman wrote:
> > Does this need to be public?
> This is in an "Impl" class in an "internal" namespace, so I believe leaving 
> it public is reasonable.
Reasonable, sure, but the preference is always to hide implementation details 
when possible from the public interface.


https://reviews.llvm.org/D22668



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


Re: [PATCH] D22824: MathExtras.h: add LLVM_CONSTEXPR where simple

2016-07-29 Thread Aaron Ballman via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM, thank you!



Comment at: include/llvm/Support/MathExtras.h:672
@@ -669,2 +671,3 @@
 inline uint64_t alignTo(uint64_t Value, uint64_t Align, uint64_t Skew = 0) {
+  assert(Align != 0u);
   Skew %= Align;

hubert.reinterpretcast wrote:
> aaron.ballman wrote:
> > Usually preferred to put in && "rationale goes here" for asserts (here and 
> > elsewhere).
> > 
> > Were these changes expected, as they don't seem to relate to constexpr?
> Sure, I can add the "friendly text".
> 
> Also, these changes are expected. The behaviour of these functions when Align 
> is 0 is the reason why I did not modify them so that they may be marked 
> constexpr (according to the language).
Ah, okay, that makes sense to me. Thank you! Go ahead and add the friendly text 
when you commit (no need to review further).


https://reviews.llvm.org/D22824



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


r277138 - Make test not fail on hosts where the default omp library is gomp.

2016-07-29 Thread Benjamin Kramer via cfe-commits
Author: d0k
Date: Fri Jul 29 08:07:09 2016
New Revision: 277138

URL: http://llvm.org/viewvc/llvm-project?rev=277138&view=rev
Log:
Make test not fail on hosts where the default omp library is gomp.

This is the case on some linuxes, just force libomp so we get the
desired results.

Modified:
cfe/trunk/test/Driver/offloading-interoperability.c

Modified: cfe/trunk/test/Driver/offloading-interoperability.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/offloading-interoperability.c?rev=277138&r1=277137&r2=277138&view=diff
==
--- cfe/trunk/test/Driver/offloading-interoperability.c (original)
+++ cfe/trunk/test/Driver/offloading-interoperability.c Fri Jul 29 08:07:09 2016
@@ -5,7 +5,7 @@
 //
 // Verify that CUDA device commands do not get OpenMP flags.
 //
-// RUN: %clang -### -x cuda -target powerpc64le-linux-gnu -std=c++11 
--cuda-gpu-arch=sm_35 -fopenmp %s 2>&1 \
+// RUN: %clang -### -x cuda -target powerpc64le-linux-gnu -std=c++11 
--cuda-gpu-arch=sm_35 -fopenmp=libomp %s 2>&1 \
 // RUN: | FileCheck %s --check-prefix NO-OPENMP-FLAGS-FOR-CUDA-DEVICE
 //
 // NO-OPENMP-FLAGS-FOR-CUDA-DEVICE:  clang{{.*}}" "-cc1" "-triple" 
"nvptx64-nvidia-cuda"


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


Re: [PATCH] D22513: [clang-tidy] add check cppcoreguidelines-special-member-functions

2016-07-29 Thread Aaron Ballman via cfe-commits
aaron.ballman added inline comments.


Comment at: clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp:60
@@ +59,3 @@
+  }
+}
+

Btw, with MSVC, this will give you a "not all control paths return a value" 
warning. You should put an llvm_unreachable() after the switch to silence that 
diagnostic.


Comment at: clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp:63
@@ +62,3 @@
+std::string SpecialMemberFunctionsCheck::join(
+llvm::ArrayRef SMFS, llvm::StringRef AndOr) {
+

Ah, crud, good point.


https://reviews.llvm.org/D22513



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


r277142 - [ASTMatcher] Add hasTemplateArgument/hasAnyTemplateArgument support in functionDecl.

2016-07-29 Thread Haojian Wu via cfe-commits
Author: hokein
Date: Fri Jul 29 08:57:27 2016
New Revision: 277142

URL: http://llvm.org/viewvc/llvm-project?rev=277142&view=rev
Log:
[ASTMatcher] Add hasTemplateArgument/hasAnyTemplateArgument support in 
functionDecl.

Reviewers: klimek

Subscribers: klimek, cfe-commits

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

Modified:
cfe/trunk/docs/LibASTMatchersReference.html
cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Modified: cfe/trunk/docs/LibASTMatchersReference.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersReference.html?rev=277142&r1=277141&r2=277142&view=diff
==
--- cfe/trunk/docs/LibASTMatchersReference.html (original)
+++ cfe/trunk/docs/LibASTMatchersReference.html Fri Jul 29 08:57:27 2016
@@ -4240,30 +4240,44 @@ caseStmt(hasCaseConstant(integerLiteral(
 
 
 MatcherClassTemplateSpecializationDecl>hasAnyTemplateArgumentMatcherTemplateArgument>
 InnerMatcher
-Matches 
classTemplateSpecializations that have at least one
-TemplateArgument matching the given InnerMatcher.
+Matches 
classTemplateSpecializations, templateSpecializationType and
+functionDecl that have at least one TemplateArgument matching the given
+InnerMatcher.
 
 Given
   template class A {};
   template<> class A {};
   A a;
+
+  template f() {};
+  void func() { f(); };
+
 classTemplateSpecializationDecl(hasAnyTemplateArgument(
 refersToType(asString("int"
   matches the specialization A
+
+functionDecl(hasAnyTemplateArgument(refersToType(asString("int"
+  matches the specialization f
 
 
 
 MatcherClassTemplateSpecializationDecl>hasTemplateArgumentunsigned N, 
MatcherTemplateArgument>
 InnerMatcher
-Matches 
classTemplateSpecializations where the n'th TemplateArgument
-matches the given InnerMatcher.
+Matches 
classTemplateSpecializations, templateSpecializationType and
+functionDecl where the n'th TemplateArgument matches the given InnerMatcher.
 
 Given
   template class A {};
   A b;
   A c;
+
+  template f() {};
+  void func() { f(); };
 classTemplateSpecializationDecl(hasTemplateArgument(
 1, refersToType(asString("int"
   matches the specialization A
+
+functionDecl(hasTemplateArgument(0, refersToType(asString("int"
+  matches the specialization f
 
 
 
@@ -4679,6 +4693,28 @@ with hasAnyParameter(...)
 
 
 
+MatcherFunctionDecl>hasAnyTemplateArgumentMatcherTemplateArgument>
 InnerMatcher
+Matches 
classTemplateSpecializations, templateSpecializationType and
+functionDecl that have at least one TemplateArgument matching the given
+InnerMatcher.
+
+Given
+  template class A {};
+  template<> class A {};
+  A a;
+
+  template f() {};
+  void func() { f(); };
+
+classTemplateSpecializationDecl(hasAnyTemplateArgument(
+refersToType(asString("int"
+  matches the specialization A
+
+functionDecl(hasAnyTemplateArgument(refersToType(asString("int"
+  matches the specialization f
+
+
+
 MatcherFunctionDecl>hasBodyMatcherStmt> 
InnerMatcher
 Matches a 'for', 'while', 
'do while' statement or a function
 definition that has a given body.
@@ -4704,6 +4740,26 @@ with hasParameter(...)
 
 
 
+MatcherFunctionDecl>hasTemplateArgumentunsigned N, 
MatcherTemplateArgument>
 InnerMatcher
+Matches 
classTemplateSpecializations, templateSpecializationType and
+functionDecl where the n'th TemplateArgument matches the given InnerMatcher.
+
+Given
+  template class A {};
+  A b;
+  A c;
+
+  template f() {};
+  void func() { f(); };
+classTemplateSpecializationDecl(hasTemplateArgument(
+1, refersToType(asString("int"
+  matches the specialization A
+
+functionDecl(hasTemplateArgument(0, refersToType(asString("int"
+  matches the specialization f
+
+
+
 MatcherFunctionDecl>returnsMatcherQualTy

Re: [PATCH] D22957: [ASTMatcher] Add hasTemplateArgument/hasAnyTemplateArgument support in functionDecl.

2016-07-29 Thread Haojian Wu via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL277142: [ASTMatcher] Add 
hasTemplateArgument/hasAnyTemplateArgument support in… (authored by hokein).

Changed prior to commit:
  https://reviews.llvm.org/D22957?vs=66094&id=66119#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D22957

Files:
  cfe/trunk/docs/LibASTMatchersReference.html
  cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
  cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
  cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
===
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
@@ -556,22 +556,32 @@
   return Node.isImplicit();
 }
 
-/// \brief Matches classTemplateSpecializations that have at least one
-/// TemplateArgument matching the given InnerMatcher.
+/// \brief Matches classTemplateSpecializations, templateSpecializationType and
+/// functionDecl that have at least one TemplateArgument matching the given
+/// InnerMatcher.
 ///
 /// Given
 /// \code
 ///   template class A {};
 ///   template<> class A {};
 ///   A a;
+///
+///   template f() {};
+///   void func() { f(); };
+/// \endcode
+///
 /// \endcode
 /// classTemplateSpecializationDecl(hasAnyTemplateArgument(
 /// refersToType(asString("int"
 ///   matches the specialization \c A
+///
+/// functionDecl(hasAnyTemplateArgument(refersToType(asString("int"
+///   matches the specialization \c f
 AST_POLYMORPHIC_MATCHER_P(
 hasAnyTemplateArgument,
 AST_POLYMORPHIC_SUPPORTED_TYPES(ClassTemplateSpecializationDecl,
-TemplateSpecializationType),
+TemplateSpecializationType,
+FunctionDecl),
 internal::Matcher, InnerMatcher) {
   ArrayRef List =
   internal::getTemplateSpecializationArgs(Node);
@@ -698,22 +708,29 @@
   return InnerMatcher.matches(Node.IgnoreParens(), Finder, Builder);
 }
 
-/// \brief Matches classTemplateSpecializations where the n'th TemplateArgument
-/// matches the given InnerMatcher.
+/// \brief Matches classTemplateSpecializations, templateSpecializationType and
+/// functionDecl where the n'th TemplateArgument matches the given InnerMatcher.
 ///
 /// Given
 /// \code
 ///   template class A {};
 ///   A b;
 ///   A c;
+///
+///   template f() {};
+///   void func() { f(); };
 /// \endcode
 /// classTemplateSpecializationDecl(hasTemplateArgument(
 /// 1, refersToType(asString("int"
 ///   matches the specialization \c A
+///
+/// functionDecl(hasTemplateArgument(0, refersToType(asString("int"
+///   matches the specialization \c f
 AST_POLYMORPHIC_MATCHER_P2(
 hasTemplateArgument,
 AST_POLYMORPHIC_SUPPORTED_TYPES(ClassTemplateSpecializationDecl,
-TemplateSpecializationType),
+TemplateSpecializationType,
+FunctionDecl),
 unsigned, N, internal::Matcher, InnerMatcher) {
   ArrayRef List =
   internal::getTemplateSpecializationArgs(Node);
Index: cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
===
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -1638,6 +1638,13 @@
   return llvm::makeArrayRef(T.getArgs(), T.getNumArgs());
 }
 
+inline ArrayRef
+getTemplateSpecializationArgs(const FunctionDecl &FD) {
+  if (const auto* TemplateArgs = FD.getTemplateSpecializationArgs())
+return TemplateArgs->asArray();
+  return ArrayRef();
+}
+
 struct NotEqualsBoundNodePredicate {
   bool operator()(const internal::BoundNodesMap &Nodes) const {
 return Nodes.getNode(ID) != Node;
Index: cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -594,6 +594,14 @@
   "A a;",
 templateSpecializationType(hasTemplateArgument(
   1, refersToType(asString("int"));
+
+  EXPECT_TRUE(matches(
+"template void f() {};"
+  "void func() { f(); }",
+functionDecl(hasTemplateArgument(0, refersToType(asString("int"));
+  EXPECT_TRUE(notMatches(
+"template void f() {};",
+functionDecl(hasTemplateArgument(0, refersToType(asString("int"));
 }
 
 TEST(TemplateArgument, Matches) {
@@ -603,6 +611,11 @@
   EXPECT_TRUE(matches(
 "template struct C {}; C c;",
 templateSpecializationType(hasAnyTemplateArgument(templateArgument();
+
+  EXPECT_TRUE(matches(
+"template void f() {};"
+  "void func() { f(); }",
+functionDecl(hasAnyTemplateArgument(templateArgument();
 }
 
 TEST(Re

r277141 - Add missing '-no-canonical-prefixes' in test.

2016-07-29 Thread Daniel Jasper via cfe-commits
Author: djasper
Date: Fri Jul 29 08:45:03 2016
New Revision: 277141

URL: http://llvm.org/viewvc/llvm-project?rev=277141&view=rev
Log:
Add missing '-no-canonical-prefixes' in test.

Modified:
cfe/trunk/test/Driver/offloading-interoperability.c

Modified: cfe/trunk/test/Driver/offloading-interoperability.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/offloading-interoperability.c?rev=277141&r1=277140&r2=277141&view=diff
==
--- cfe/trunk/test/Driver/offloading-interoperability.c (original)
+++ cfe/trunk/test/Driver/offloading-interoperability.c Fri Jul 29 08:45:03 2016
@@ -5,7 +5,7 @@
 //
 // Verify that CUDA device commands do not get OpenMP flags.
 //
-// RUN: %clang -### -x cuda -target powerpc64le-linux-gnu -std=c++11 
--cuda-gpu-arch=sm_35 -fopenmp=libomp %s 2>&1 \
+// RUN: %clang -no-canonical-prefixes -### -x cuda -target 
powerpc64le-linux-gnu -std=c++11 --cuda-gpu-arch=sm_35 -fopenmp=libomp %s 2>&1 \
 // RUN: | FileCheck %s --check-prefix NO-OPENMP-FLAGS-FOR-CUDA-DEVICE
 //
 // NO-OPENMP-FLAGS-FOR-CUDA-DEVICE:  clang{{.*}}" "-cc1" "-triple" 
"nvptx64-nvidia-cuda"


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


Re: [PATCH] D21814: clang-rename: split existing options into two new subcommands

2016-07-29 Thread Kirill Bobyrev via cfe-commits
omtcyfz added a comment.

P.S. not sure whether we have to write `clang-rename: for the -new-name option: 
must be specified` out. We already launched `clang-rename` what else could've 
give us an error?


https://reviews.llvm.org/D21814



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


Re: [PATCH] D21814: clang-rename: split existing options into two new subcommands

2016-07-29 Thread Miklos Vajna via cfe-commits
vmiklos added a comment.

In https://reviews.llvm.org/D21814#500621, @omtcyfz wrote:

> P.S. not sure whether we have to write `clang-rename: for the -new-name 
> option: must be specified` out. We already launched `clang-rename` what else 
> could've give us an error?


You mean how is that error produced? Previously there was no `cl::Required` on 
NewName (now: NewNames), and that's why a manual check was needed, the error 
message changed, as now the option parser does this checking for us.


https://reviews.llvm.org/D21814



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


Re: r277142 - [ASTMatcher] Add hasTemplateArgument/hasAnyTemplateArgument support in functionDecl.

2016-07-29 Thread Benjamin Kramer via cfe-commits
On Fri, Jul 29, 2016 at 3:57 PM, Haojian Wu via cfe-commits
 wrote:
> Author: hokein
> Date: Fri Jul 29 08:57:27 2016
> New Revision: 277142
>
> URL: http://llvm.org/viewvc/llvm-project?rev=277142&view=rev
> Log:
> [ASTMatcher] Add hasTemplateArgument/hasAnyTemplateArgument support in 
> functionDecl.
>
> Reviewers: klimek
>
> Subscribers: klimek, cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D22957
>
> Modified:
> cfe/trunk/docs/LibASTMatchersReference.html
> cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
> cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
> cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
>
> Modified: cfe/trunk/docs/LibASTMatchersReference.html
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersReference.html?rev=277142&r1=277141&r2=277142&view=diff
> ==
> --- cfe/trunk/docs/LibASTMatchersReference.html (original)
> +++ cfe/trunk/docs/LibASTMatchersReference.html Fri Jul 29 08:57:27 2016
> @@ -4240,30 +4240,44 @@ caseStmt(hasCaseConstant(integerLiteral(
>
>
>  Matcher< href="http://clang.llvm.org/doxygen/classclang_1_1ClassTemplateSpecializationDecl.html";>ClassTemplateSpecializationDecl>  class="name" onclick="toggle('hasAnyTemplateArgument0')"> name="hasAnyTemplateArgument0Anchor">hasAnyTemplateArgumentMatcher<  
> href="http://clang.llvm.org/doxygen/classclang_1_1TemplateArgument.html";>TemplateArgument>
>  InnerMatcher
> -Matches 
> classTemplateSpecializations that have at least one
> -TemplateArgument matching the given InnerMatcher.
> +Matches 
> classTemplateSpecializations, templateSpecializationType and
> +functionDecl that have at least one TemplateArgument matching the given
> +InnerMatcher.
>
>  Given
>template class A {};
>template<> class A {};
>A a;
> +
> +  template f() {};
> +  void func() { f(); };
> +
>  classTemplateSpecializationDecl(hasAnyTemplateArgument(
>  refersToType(asString("int"
>matches the specialization A
> +
> +functionDecl(hasAnyTemplateArgument(refersToType(asString("int"
> +  matches the specialization f
>  
>
>
>  Matcher< href="http://clang.llvm.org/doxygen/classclang_1_1ClassTemplateSpecializationDecl.html";>ClassTemplateSpecializationDecl>  class="name" onclick="toggle('hasTemplateArgument0')"> name="hasTemplateArgument0Anchor">hasTemplateArgumentunsigned N, 
> Matcher< href="http://clang.llvm.org/doxygen/classclang_1_1TemplateArgument.html";>TemplateArgument>
>  InnerMatcher
> -Matches 
> classTemplateSpecializations where the n'th TemplateArgument
> -matches the given InnerMatcher.
> +Matches 
> classTemplateSpecializations, templateSpecializationType and
> +functionDecl where the n'th TemplateArgument matches the given InnerMatcher.
>
>  Given
>template class A {};
>A b;
>A c;
> +
> +  template f() {};
> +  void func() { f(); };
>  classTemplateSpecializationDecl(hasTemplateArgument(
>  1, refersToType(asString("int"
>matches the specialization A
> +
> +functionDecl(hasTemplateArgument(0, refersToType(asString("int"
> +  matches the specialization f
>  
>
>
> @@ -4679,6 +4693,28 @@ with hasAnyParameter(...)
>  
>
>
> +Matcher< href="http://clang.llvm.org/doxygen/classclang_1_1FunctionDecl.html";>FunctionDecl>  class="name" onclick="toggle('hasAnyTemplateArgument2')"> name="hasAnyTemplateArgument2Anchor">hasAnyTemplateArgumentMatcher<  
> href="http://clang.llvm.org/doxygen/classclang_1_1TemplateArgument.html";>TemplateArgument>
>  InnerMatcher
> +Matches 
> classTemplateSpecializations, templateSpecializationType and
> +functionDecl that have at least one TemplateArgument matching the given
> +InnerMatcher.
> +
> +Given
> +  template class A {};
> +  template<> class A {};
> +  A a;
> +
> +  template f() {};
> +  void func() { f(); };
> +
> +classTemplateSpecializationDecl(hasAnyTemplateArgument(
> +refersToType(asString("int"
> +  matches the specialization A
> +
> +functionDecl(hasAnyTemplateArgument(refersToType(asString("int"
> +  matches the specialization f
> +
> +
> +
>  Matcher< href="http://clang.llvm.org/doxygen/classclang_1_1FunctionDecl.html";>FunctionDecl>  class="name" onclick="toggle('hasBody4')"> name="hasBody4Anchor">hasBodyMatcher< href="http://clang.llvm.org/doxygen/classclang_1_1Stmt.html";>Stmt> 
> InnerMatcher
>  Matches a 'for', 'while', 
> 'do while' statement or a function
>  definition that has a given body.
> @@ -4704,6 +4740,26 @@ with hasParameter(...)
>  
>
>
> +Matcher< href="http://clang.llvm.org/doxygen/classclang_1_1FunctionDecl.html";>FunctionDecl>  class="name" onclick="toggle('hasTemplateArgument2')"> name="hasTemplateArgument2Anchor">hasTemplateArgumentunsigned N, 
> Matcher< href="http://clang.llvm.org/d

[PATCH] D22963: [ASTMatcher] Add templateName matcher.

2016-07-29 Thread Haojian Wu via cfe-commits
hokein created this revision.
hokein added a reviewer: klimek.
hokein added a subscriber: cfe-commits.
Herald added a subscriber: klimek.

https://reviews.llvm.org/D22963

Files:
  docs/LibASTMatchersReference.html
  include/clang/AST/ASTTypeTraits.h
  include/clang/ASTMatchers/ASTMatchers.h
  lib/AST/ASTTypeTraits.cpp
  lib/ASTMatchers/Dynamic/Registry.cpp
  unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -543,6 +543,15 @@
   asString("int"));
 }
 
+TEST(Matcher, MatchesTemplateTemplateArgument) {
+  EXPECT_TRUE(matches(
+"template class S> class X {};"
+"template class Y {};"
+  "X xi;",
+classTemplateSpecializationDecl(hasAnyTemplateArgument(
+  refersToTemplate(templateName());
+}
+
 TEST(Matcher, MatchesDeclarationReferenceTemplateArgument) {
   EXPECT_TRUE(matches(
 "struct B { int next; };"
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -391,6 +391,7 @@
   REGISTER_MATCHER(switchCase);
   REGISTER_MATCHER(switchStmt);
   REGISTER_MATCHER(templateArgument);
+  REGISTER_MATCHER(templateName);
   REGISTER_MATCHER(templateArgumentCountIs);
   REGISTER_MATCHER(templateSpecializationType);
   REGISTER_MATCHER(templateTypeParmType);
Index: lib/AST/ASTTypeTraits.cpp
===
--- lib/AST/ASTTypeTraits.cpp
+++ lib/AST/ASTTypeTraits.cpp
@@ -23,6 +23,7 @@
 const ASTNodeKind::KindInfo ASTNodeKind::AllKindInfo[] = {
   { NKI_None, "" },
   { NKI_None, "TemplateArgument" },
+  { NKI_None, "TemplateName" },
   { NKI_None, "NestedNameSpecifierLoc" },
   { NKI_None, "QualType" },
   { NKI_None, "TypeLoc" },
@@ -109,6 +110,8 @@
  const PrintingPolicy &PP) const {
   if (const TemplateArgument *TA = get())
 TA->print(PP, OS);
+  else if (const TemplateName *TN = get())
+TN->print(OS, PP);
   else if (const NestedNameSpecifier *NNS = get())
 NNS->print(OS, PP);
   else if (const NestedNameSpecifierLoc *NNSL = get())
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -446,6 +446,17 @@
 ///   matches 'int' in C.
 const internal::VariadicAllOfMatcher templateArgument;
 
+/// \brief Matches template name.
+///
+/// Given
+/// \code
+///   template  class X { };
+///   X xi;
+/// \endcode
+/// templateName()
+///   matches 'X' in X.
+const internal::VariadicAllOfMatcher templateName;
+
 /// \brief Matches non-type template parameter declarations.
 ///
 /// Given
@@ -774,6 +785,24 @@
   return InnerMatcher.matches(Node.getAsType(), Finder, Builder);
 }
 
+/// \brief Matches a TemplateArgument that refers to a certain template.
+///
+/// Given
+/// \code
+///   template class S> class X {};
+///   template class Y {};"
+///   X xi;
+/// \endcode
+/// classTemplateSpecializationDecl(hasAnyTemplateArgument(
+/// refersToTemplate(templateName(
+///   matches the specialization \c X
+AST_MATCHER_P(TemplateArgument, refersToTemplate,
+  internal::Matcher, InnerMatcher) {
+  if (Node.getKind() != TemplateArgument::Template)
+return false;
+  return InnerMatcher.matches(Node.getAsTemplate(), Finder, Builder);
+}
+
 /// \brief Matches a canonical TemplateArgument that refers to a certain
 /// declaration.
 ///
Index: include/clang/AST/ASTTypeTraits.h
===
--- include/clang/AST/ASTTypeTraits.h
+++ include/clang/AST/ASTTypeTraits.h
@@ -121,6 +121,7 @@
   enum NodeKindId {
 NKI_None,
 NKI_TemplateArgument,
+NKI_TemplateName,
 NKI_NestedNameSpecifierLoc,
 NKI_QualType,
 NKI_TypeLoc,
@@ -175,6 +176,7 @@
   };
 KIND_TO_KIND_ID(CXXCtorInitializer)
 KIND_TO_KIND_ID(TemplateArgument)
+KIND_TO_KIND_ID(TemplateName)
 KIND_TO_KIND_ID(NestedNameSpecifier)
 KIND_TO_KIND_ID(NestedNameSpecifierLoc)
 KIND_TO_KIND_ID(QualType)
@@ -472,6 +474,10 @@
 
 template <>
 struct DynTypedNode::BaseConverter<
+TemplateName, void> : public ValueConverter {};
+
+template <>
+struct DynTypedNode::BaseConverter<
 NestedNameSpecifierLoc,
 void> : public ValueConverter {};
 
Index: docs/LibASTMatchersReference.html
===
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -1318,6 +1318,17 @@
 
 
 
+MatcherTemplateName>templateNameMatcherTemplateName>...
+Matches templ

Re: [PATCH] D22963: [ASTMatcher] Add templateName matcher.

2016-07-29 Thread Manuel Klimek via cfe-commits
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.

lg



Comment at: unittests/ASTMatchers/ASTMatchersTraversalTest.cpp:550
@@ +549,3 @@
+"template class Y {};"
+  "X xi;",
+classTemplateSpecializationDecl(hasAnyTemplateArgument(

clang-format?


https://reviews.llvm.org/D22963



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


Re: [PATCH] D22963: [ASTMatcher] Add templateName matcher.

2016-07-29 Thread Haojian Wu via cfe-commits
hokein updated this revision to Diff 66129.
hokein marked an inline comment as done.
hokein added a comment.

Fix code style.


https://reviews.llvm.org/D22963

Files:
  docs/LibASTMatchersReference.html
  include/clang/AST/ASTTypeTraits.h
  include/clang/ASTMatchers/ASTMatchers.h
  lib/AST/ASTTypeTraits.cpp
  lib/ASTMatchers/Dynamic/Registry.cpp
  unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -543,6 +543,14 @@
   asString("int"));
 }
 
+TEST(Matcher, MatchesTemplateTemplateArgument) {
+  EXPECT_TRUE(matches("template class S> class X {};"
+  "template class Y {};"
+  "X xi;",
+  classTemplateSpecializationDecl(hasAnyTemplateArgument(
+  refersToTemplate(templateName());
+}
+
 TEST(Matcher, MatchesDeclarationReferenceTemplateArgument) {
   EXPECT_TRUE(matches(
 "struct B { int next; };"
Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -391,6 +391,7 @@
   REGISTER_MATCHER(switchCase);
   REGISTER_MATCHER(switchStmt);
   REGISTER_MATCHER(templateArgument);
+  REGISTER_MATCHER(templateName);
   REGISTER_MATCHER(templateArgumentCountIs);
   REGISTER_MATCHER(templateSpecializationType);
   REGISTER_MATCHER(templateTypeParmType);
Index: lib/AST/ASTTypeTraits.cpp
===
--- lib/AST/ASTTypeTraits.cpp
+++ lib/AST/ASTTypeTraits.cpp
@@ -23,6 +23,7 @@
 const ASTNodeKind::KindInfo ASTNodeKind::AllKindInfo[] = {
   { NKI_None, "" },
   { NKI_None, "TemplateArgument" },
+  { NKI_None, "TemplateName" },
   { NKI_None, "NestedNameSpecifierLoc" },
   { NKI_None, "QualType" },
   { NKI_None, "TypeLoc" },
@@ -109,6 +110,8 @@
  const PrintingPolicy &PP) const {
   if (const TemplateArgument *TA = get())
 TA->print(PP, OS);
+  else if (const TemplateName *TN = get())
+TN->print(OS, PP);
   else if (const NestedNameSpecifier *NNS = get())
 NNS->print(OS, PP);
   else if (const NestedNameSpecifierLoc *NNSL = get())
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -446,6 +446,17 @@
 ///   matches 'int' in C.
 const internal::VariadicAllOfMatcher templateArgument;
 
+/// \brief Matches template name.
+///
+/// Given
+/// \code
+///   template  class X { };
+///   X xi;
+/// \endcode
+/// templateName()
+///   matches 'X' in X.
+const internal::VariadicAllOfMatcher templateName;
+
 /// \brief Matches non-type template parameter declarations.
 ///
 /// Given
@@ -774,6 +785,24 @@
   return InnerMatcher.matches(Node.getAsType(), Finder, Builder);
 }
 
+/// \brief Matches a TemplateArgument that refers to a certain template.
+///
+/// Given
+/// \code
+///   template class S> class X {};
+///   template class Y {};"
+///   X xi;
+/// \endcode
+/// classTemplateSpecializationDecl(hasAnyTemplateArgument(
+/// refersToTemplate(templateName(
+///   matches the specialization \c X
+AST_MATCHER_P(TemplateArgument, refersToTemplate,
+  internal::Matcher, InnerMatcher) {
+  if (Node.getKind() != TemplateArgument::Template)
+return false;
+  return InnerMatcher.matches(Node.getAsTemplate(), Finder, Builder);
+}
+
 /// \brief Matches a canonical TemplateArgument that refers to a certain
 /// declaration.
 ///
Index: include/clang/AST/ASTTypeTraits.h
===
--- include/clang/AST/ASTTypeTraits.h
+++ include/clang/AST/ASTTypeTraits.h
@@ -121,6 +121,7 @@
   enum NodeKindId {
 NKI_None,
 NKI_TemplateArgument,
+NKI_TemplateName,
 NKI_NestedNameSpecifierLoc,
 NKI_QualType,
 NKI_TypeLoc,
@@ -175,6 +176,7 @@
   };
 KIND_TO_KIND_ID(CXXCtorInitializer)
 KIND_TO_KIND_ID(TemplateArgument)
+KIND_TO_KIND_ID(TemplateName)
 KIND_TO_KIND_ID(NestedNameSpecifier)
 KIND_TO_KIND_ID(NestedNameSpecifierLoc)
 KIND_TO_KIND_ID(QualType)
@@ -472,6 +474,10 @@
 
 template <>
 struct DynTypedNode::BaseConverter<
+TemplateName, void> : public ValueConverter {};
+
+template <>
+struct DynTypedNode::BaseConverter<
 NestedNameSpecifierLoc,
 void> : public ValueConverter {};
 
Index: docs/LibASTMatchersReference.html
===
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -1318,6 +1318,17 @@
 
 
 
+MatcherTemplateName>templateNameMatcher

Re: [PATCH] D22803: [clang-tidy] Fix an unused-using-decl false positive about template arguments infunction call expression.

2016-07-29 Thread Haojian Wu via cfe-commits
hokein updated this revision to Diff 66131.
hokein added a comment.

Use new ast matchers to simplify the code.

Updating D22803: [clang-tidy] Fix an unused-using-decl false positive about 
template arguments in
=

function call expression.


https://reviews.llvm.org/D22803

Files:
  clang-tidy/misc/UnusedUsingDeclsCheck.cpp
  test/clang-tidy/Inputs/unused-using-decls.h
  test/clang-tidy/misc-unused-using-decls.cpp

Index: test/clang-tidy/misc-unused-using-decls.cpp
===
--- test/clang-tidy/misc-unused-using-decls.cpp
+++ test/clang-tidy/misc-unused-using-decls.cpp
@@ -1,4 +1,5 @@
-// RUN: %check_clang_tidy %s misc-unused-using-decls %t -- -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy %s misc-unused-using-decls %t -- -- -fno-delayed-template-parsing -isystem %S/Inputs/
+
 
 // - Definitions -
 template  class vector {};
@@ -54,6 +55,16 @@
 
 }  // namespace n
 
+#include "unused-using-decls.h"
+namespace ns {
+template 
+class AA {
+  T t;
+};
+template 
+T ff() { T t; return t; }
+} // namespace ns
+
 // - Using declarations -
 // eol-comments aren't removed (yet)
 using n::A; // A
@@ -136,6 +147,9 @@
 using n::Color3;
 using n::Blue;
 
+using ns::AA;
+using ns::ff;
+
 // - Usages -
 void f(B b);
 void g() {
@@ -151,4 +165,9 @@
   Color2 color2;
   int t1 = Color3::Yellow;
   int t2 = Blue;
+
+  MyClass a;
+  int t3 = 0;
+  a.func1(&t3);
+  a.func2(t3);
 }
Index: test/clang-tidy/Inputs/unused-using-decls.h
===
--- /dev/null
+++ test/clang-tidy/Inputs/unused-using-decls.h
@@ -0,0 +1,11 @@
+class MyClass {
+public:
+  template  class S, typename T>
+  S *func1(T *a) {
+return new S();
+  }
+  template 
+  void func2(T a) {
+S();
+  }
+};
Index: clang-tidy/misc/UnusedUsingDeclsCheck.cpp
===
--- clang-tidy/misc/UnusedUsingDeclsCheck.cpp
+++ clang-tidy/misc/UnusedUsingDeclsCheck.cpp
@@ -27,7 +27,6 @@
  isa(TargetDecl) || isa(TargetDecl) ||
  isa(TargetDecl);
 }
-
 void UnusedUsingDeclsCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(usingDecl(isExpansionInMainFile()).bind("using"), this);
   auto DeclMatcher = hasDeclaration(namedDecl().bind("used"));
@@ -37,6 +36,11 @@
   Finder->addMatcher(declRefExpr().bind("used"), this);
   Finder->addMatcher(callExpr(callee(unresolvedLookupExpr().bind("used"))),
  this);
+  Finder->addMatcher(
+  callExpr(hasDeclaration(functionDecl(hasAnyTemplateArgument(
+  anyOf(refersToTemplate(templateName().bind("used")),
+refersToDeclaration(functionDecl().bind("used"))),
+  this);
 }
 
 void UnusedUsingDeclsCheck::check(const MatchFinder::MatchResult &Result) {
@@ -71,20 +75,27 @@
   Contexts.push_back(Context);
 return;
   }
-
   // Mark using declarations as used by setting FoundDecls' value to zero. As
   // the AST is walked in order, usages are only marked after a the
   // corresponding using declaration has been found.
   // FIXME: This currently doesn't look at whether the type reference is
   // actually found with the help of the using declaration.
   if (const auto *Used = Result.Nodes.getNodeAs("used")) {
-if (const auto *Specialization =
-dyn_cast(Used))
+if (const auto *FD = dyn_cast(Used)) {
+  removeFromFoundDecls(FD->getPrimaryTemplate());
+} else if (const auto *Specialization =
+dyn_cast(Used)) {
   Used = Specialization->getSpecializedTemplate();
+}
 removeFromFoundDecls(Used);
 return;
   }
 
+  if (const auto *Used = Result.Nodes.getNodeAs("used")) {
+removeFromFoundDecls(Used->getAsTemplateDecl());
+return;
+  }
+
   if (const auto *DRE = Result.Nodes.getNodeAs("used")) {
 if (const auto *FD = dyn_cast(DRE->getDecl())) {
   if (const auto *FDT = FD->getPrimaryTemplate())
@@ -109,6 +120,8 @@
 }
 
 void UnusedUsingDeclsCheck::removeFromFoundDecls(const Decl *D) {
+  if (!D)
+return;
   // FIXME: Currently, we don't handle the using-decls being used in different
   // scopes (such as different namespaces, different functions). Instead of
   // giving an incorrect message, we mark all of them as used.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: [PATCH] D22803: [clang-tidy] Fix an unused-using-decl false positive about template arguments infunction call expression.

2016-07-29 Thread Haojian Wu via cfe-commits
hokein marked an inline comment as done.


Comment at: clang-tidy/misc/UnusedUsingDeclsCheck.cpp:84-87
@@ -80,4 +83,6 @@
   if (const auto *Used = Result.Nodes.getNodeAs("used")) {
-if (const auto *Specialization =
-dyn_cast(Used))
+if (const auto *FD = dyn_cast(Used)) {
+  removeFromFoundDecls(FD->getPrimaryTemplate());
+} else if (const auto *Specialization =
+dyn_cast(Used)) {
   Used = Specialization->getSpecializedTemplate();

Have added the new matchers to clang ASTMatchers (D22963, D22957), and switched 
to use these matchers.


https://reviews.llvm.org/D22803



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


Re: r277142 - [ASTMatcher] Add hasTemplateArgument/hasAnyTemplateArgument support in functionDecl.

2016-07-29 Thread Haojian Wu via cfe-commits
No, this is not templateName patch. The templateName one is in r277155.

On Fri, Jul 29, 2016 at 4:45 PM, Benjamin Kramer 
wrote:

> On Fri, Jul 29, 2016 at 3:57 PM, Haojian Wu via cfe-commits
>  wrote:
> > Author: hokein
> > Date: Fri Jul 29 08:57:27 2016
> > New Revision: 277142
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=277142&view=rev
> > Log:
> > [ASTMatcher] Add hasTemplateArgument/hasAnyTemplateArgument support in
> functionDecl.
> >
> > Reviewers: klimek
> >
> > Subscribers: klimek, cfe-commits
> >
> > Differential Revision: https://reviews.llvm.org/D22957
> >
> > Modified:
> > cfe/trunk/docs/LibASTMatchersReference.html
> > cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
> > cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
> > cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
> >
> > Modified: cfe/trunk/docs/LibASTMatchersReference.html
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersReference.html?rev=277142&r1=277141&r2=277142&view=diff
> >
> ==
> > --- cfe/trunk/docs/LibASTMatchersReference.html (original)
> > +++ cfe/trunk/docs/LibASTMatchersReference.html Fri Jul 29 08:57:27 2016
> > @@ -4240,30 +4240,44 @@ caseStmt(hasCaseConstant(integerLiteral(
> >
> >
> >  MatcherClassTemplateSpecializationDecl> class="name" onclick="toggle('hasAnyTemplateArgument0')"> name="hasAnyTemplateArgument0Anchor">hasAnyTemplateArgumentMatcher< href="http://clang.llvm.org/doxygen/classclang_1_1TemplateArgument.html";>TemplateArgument>
> InnerMatcher
> > - id="hasAnyTemplateArgument0">Matches classTemplateSpecializations that
> have at least one
> > -TemplateArgument matching the given InnerMatcher.
> > + id="hasAnyTemplateArgument0">Matches classTemplateSpecializations,
> templateSpecializationType and
> > +functionDecl that have at least one TemplateArgument matching the given
> > +InnerMatcher.
> >
> >  Given
> >template class A {};
> >template<> class A {};
> >A a;
> > +
> > +  template f() {};
> > +  void func() { f(); };
> > +
> >  classTemplateSpecializationDecl(hasAnyTemplateArgument(
> >  refersToType(asString("int"
> >matches the specialization A
> > +
> > +functionDecl(hasAnyTemplateArgument(refersToType(asString("int"
> > +  matches the specialization f
> >  
> >
> >
> >  MatcherClassTemplateSpecializationDecl> class="name" onclick="toggle('hasTemplateArgument0')"> name="hasTemplateArgument0Anchor">hasTemplateArgumentunsigned
> N, MatcherTemplateArgument>
> InnerMatcher
> > -Matches
> classTemplateSpecializations where the n'th TemplateArgument
> > -matches the given InnerMatcher.
> > +Matches
> classTemplateSpecializations, templateSpecializationType and
> > +functionDecl where the n'th TemplateArgument matches the given
> InnerMatcher.
> >
> >  Given
> >template class A {};
> >A b;
> >A c;
> > +
> > +  template f() {};
> > +  void func() { f(); };
> >  classTemplateSpecializationDecl(hasTemplateArgument(
> >  1, refersToType(asString("int"
> >matches the specialization A
> > +
> > +functionDecl(hasTemplateArgument(0, refersToType(asString("int"
> > +  matches the specialization f
> >  
> >
> >
> > @@ -4679,6 +4693,28 @@ with hasAnyParameter(...)
> >  
> >
> >
> > +MatcherFunctionDecl> class="name" onclick="toggle('hasAnyTemplateArgument2')"> name="hasAnyTemplateArgument2Anchor">hasAnyTemplateArgumentMatcher< href="http://clang.llvm.org/doxygen/classclang_1_1TemplateArgument.html";>TemplateArgument>
> InnerMatcher
> > + id="hasAnyTemplateArgument2">Matches classTemplateSpecializations,
> templateSpecializationType and
> > +functionDecl that have at least one TemplateArgument matching the given
> > +InnerMatcher.
> > +
> > +Given
> > +  template class A {};
> > +  template<> class A {};
> > +  A a;
> > +
> > +  template f() {};
> > +  void func() { f(); };
> > +
> > +classTemplateSpecializationDecl(hasAnyTemplateArgument(
> > +refersToType(asString("int"
> > +  matches the specialization A
> > +
> > +functionDecl(hasAnyTemplateArgument(refersToType(asString("int"
> > +  matches the specialization f
> > +
> > +
> > +
> >  MatcherFunctionDecl> class="name" onclick="toggle('hasBody4')"> name="hasBody4Anchor">hasBodyMatcherStmt>
> InnerMatcher
> >  Matches a 'for',
> 'while', 'do while' statement

r277155 - [ASTMatcher] Add templateName matcher.

2016-07-29 Thread Haojian Wu via cfe-commits
Author: hokein
Date: Fri Jul 29 10:45:11 2016
New Revision: 277155

URL: http://llvm.org/viewvc/llvm-project?rev=277155&view=rev
Log:
[ASTMatcher] Add templateName matcher.

Reviewers: klimek

Subscribers: klimek, cfe-commits

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

Modified:
cfe/trunk/docs/LibASTMatchersReference.html
cfe/trunk/include/clang/AST/ASTTypeTraits.h
cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
cfe/trunk/lib/AST/ASTTypeTraits.cpp
cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Modified: cfe/trunk/docs/LibASTMatchersReference.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LibASTMatchersReference.html?rev=277155&r1=277154&r2=277155&view=diff
==
--- cfe/trunk/docs/LibASTMatchersReference.html (original)
+++ cfe/trunk/docs/LibASTMatchersReference.html Fri Jul 29 10:45:11 2016
@@ -1318,6 +1318,17 @@ templateArgument()
 
 
 
+MatcherTemplateName>templateNameMatcherTemplateName>...
+Matches template name.
+
+Given
+  template  class X { };
+  X xi;
+templateName()
+  matches 'X' in X.
+
+
+
 MatcherTypeLoc>typeLocMatcherTypeLoc>...
 Matches TypeLocs in the 
clang AST.
 
@@ -5353,6 +5364,19 @@ classTemplateSpecializationDecl(
 
 
 
+MatcherTemplateArgument>refersToTemplateMatcherTemplateName>
 InnerMatcher
+Matches a 
TemplateArgument that refers to a certain template.
+
+Given
+  template