[cfe-commits] PATCH: add-override-specifier tool

Sean Silva silvas at purdue.edu
Sun Jul 15 23:59:21 PDT 2012


+//  Where <cmake-output-dir> is a CMake build directory in which a file named
+//  compile_commands.json exists (enable -DCMAKE_EXPORT_COMPILE_COMMANDS in
+//  CMake to get this output).

This doesn't seem to correctly express the intent of what is being
communicated. The CMake part is incidental. The important thing is
having a correctly structured compile_commands.json; I think the hope
is that eventually more than just CMake will be able to generate such
a file. Maybe "Where <compile-dir> is the name of a directory
containing a `compile_commands.json` file in the format understood by
JSONCompilationDatabase (see the declaration in
`include/clang/Tooling/CompilationDatabase.h` for details). Currently,
this can be achieved with CMake by passing it
`-DCMAKE_EXPORT_COMPILE_COMMANDS` when configuring."

+    // Given
+    //   class X { virtual void f(int x) {} };
+    //   class Y { virtual void f(int x) {} };
+    //   class Z { virtual void f(int x) const {}
+    //       virtual void f(int x) override {} };

Isn't there supposed to be some kind of inheritance going on between
these classes?
Also, I think you can take a bit more room to format them all pretty.

+    SourceManager& SourceManager = *Result.SourceManager;
+    const CXXMethodDecl* M = Result.Nodes.getDeclAs<CXXMethodDecl>("method");
LLVM/Clang style put the '*'/'&' on the right.

+// RUN: rm -rf %t.cpp
+// RUN: grep -Ev "//\s*[A-Z-]+:" %s > %t.cpp
+// RUN: add-override-specifier %t.cpp -- -std=c++11
+// RUN: cat "%t.cpp" | FileCheck %s
+// RUN: rm -rf %t.cpp

I don't think you need the 'recursive' option for the `rm` commands.

--Sean Silva

On Sun, Jul 15, 2012 at 10:04 AM, Philip Dunstan <phil at philipdunstan.com> wrote:
> Hi
>
> This is a patch to add a new tool to the clang tools directory to add the
> C++11 override specifier to member functions where appropriate. That is,
> member functions that override a virtual function from a base class and that
> are not already marked up with the override specifier. This tool uses the
> ASTMatchers framework (with a custom matcher) recently integrated from the
> tooling branch.
>
> I'm aware of the recent conversation on cfe-dev regarding C++11 migration
> tools. There was some talk there of possibly locating these types of tools
> separate from the main repository. If there is such a repository I can
> rebase the patch to that location.
>
> Phil
> --
> Philip Dunstan
> phil at philipdunstan.com
> www.philipdunstan.com
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>



More information about the cfe-commits mailing list