Skip to content

feat: Support running Clang Tools in-process via getActionType #36

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 27 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ $Clang_DIR/bin/clang -cc1 -load <build_dir>/lib/libLACommenter.dylib -plugin LAC
```

### Run the plugin through `ct-la-commenter`
**locommenter** is a standalone tool that will run the **LACommenter** plugin,
**lacommenter** is a standalone tool that will run the **LACommenter** plugin,
but without the need of using `clang` and loading the plugin:

```bash
Expand Down Expand Up @@ -358,6 +358,32 @@ the warnings with correct source code information.
`-fcolor-diagnostics` above instructs Clang to generate color output
(unfortunately Markdown doesn't render the colors here).

### Run the plugin and compile the input

In the invocation from the previous section, Clang runs only one action - the plugin itself. This means that no output files are generated. In order to run a plugin action _and_ e.g. a compilation action, you need implement `getActionType` method (from [Clang Plugins](https://clang.llvm.org/docs/ClangPlugins.html#using-the-clang-command-line)):
> If the plugin class implements the `getActionType` method then the plugin is run automatically.
```c
// Automatically run the plugin after the main AST action
PluginASTAction::ActionType getActionType() override {
return AddAfterMainAction;
}
```

The **CodeStyleChecker** plugin does implement `getActionType` and hence can be run automatically and used during the normal compilation
process to detect errors in the code, and get the output file, for example:
```bash
$Clang_DIR/bin/clang -fplugin=libCodeStyleChecker.dylib -o file.o -c file.cpp
file.cpp:2:7: warning: Type and variable names should start with upper-case letter
class clangTutor_BadName;
^~~~~~~~~~~~~~~~~~~
ClangTutor_BadName
file.cpp:2:17: warning: `_` in names is not allowed
class clangTutor_BadName;
~~~~~~~~~~^~~~~~~~~
clangTutorBadName
2 warnings generated.
```

### Run the plugin through `ct-code-style-checker`
**ct-code-style-checker** is a standalone tool that will run the **CodeStyleChecker** plugin,
but without the need of using `clang` and loading the plugin:
Expand Down
8 changes: 8 additions & 0 deletions lib/CodeStyleChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,14 @@ class CSCASTAction : public PluginASTAction {
ros << "Help for CodeStyleChecker plugin goes here\n";
}

PluginASTAction::ActionType getActionType() override {
#ifndef TARGET_CLANG_TOOL
return AddBeforeMainAction;
#else
return CmdlineAfterMainAction;
#endif
}
Comment on lines +196 to +202
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TARGET_CLANG_TOOL -> CT_CSC_AS_TOOL (for consistency with the other CMake vars in clang-tutor)

Also, could you document this method? A link to Clang docs would be helpful. Also, what is the difference between AddBeforeMainAction and CmdlineAfterMainAction? Is it possible to test the difference? As in, to write a test that we demonstrate the difference between the two?


private:
bool MainTuOnly = true;
};
Expand Down
10 changes: 9 additions & 1 deletion test/CodeStyleCheckerAnonymous.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
// RUN: clang -cc1 -verify -load %shlibdir/libCodeStyleChecker%shlibext -plugin CSC %s 2>&1
// RUN: clang -cc1 -verify -load %shlibdir/libCodeStyleChecker%shlibext -add-plugin CSC %s 2>&1

// RUN: clang -Xclang -load -Xclang %shlibdir/libCodeStyleChecker%shlibext -Xclang -add-plugin -Xclang CSC -c %s -o %t.o
// RUN: test -f %t.o
// RUN: rm %t.o

// RUN: clang -fplugin=%shlibdir/libCodeStyleChecker%shlibext -c %s -o %t.o
// RUN: test -f %t.o
// RUN: rm %t.o

Comment on lines +3 to +9
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need these additional test lines? This question applies to all test files that you have updated :)

// 1. Verify that anonymous unions are not flagged as invalid (no name ->
// nothing to check). However, the member variables _are_ verified.
Expand Down
27 changes: 19 additions & 8 deletions test/CodeStyleCheckerConversionOp.cpp
Original file line number Diff line number Diff line change
@@ -1,11 +1,22 @@
// RUN: clang -cc1 -verify -load %shlibdir/libCodeStyleChecker%shlibext -plugin CSC %s 2>&1
// RUN: clang -cc1 -verify -load %shlibdir/libCodeStyleChecker%shlibext -add-plugin CSC %s 2>&1

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we keep -plugin instead of -add-plugin?

// Verify that conversion operators are not checked
// RUN: clang -Xclang -load -Xclang %shlibdir/libCodeStyleChecker%shlibext -Xclang -add-plugin -Xclang CSC -c %s -o %t.o
// RUN: test -f %t.o
// RUN: rm %t.o

// expected-no-diagnostics
class SomeClass {
public:
operator bool();
operator int();
operator char();
// RUN: clang -fplugin=%shlibdir/libCodeStyleChecker%shlibext -c %s -o %t.o
Comment on lines -5 to -10
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets keep these tests :)

// RUN: test -f %t.o
// RUN: rm %t.o

// Verify that function names starting with upper case are reported as invalid

// expected-warning@+1 {{Function names should start with lower-case letter}}
void ClangTutorFuncBad();

void clangTutorFuncOK();

struct ClangTutorStruct {
// expected-warning@+1 {{Function names should start with lower-case letter}}
void ClangTutorMemberMethodBad();
void clangTutorMemberMethodOK();
};
Comment on lines +13 to +21
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this duplicating tests from CodeStyleCheckerFunction.cpp?

10 changes: 9 additions & 1 deletion test/CodeStyleCheckerFunction.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
// RUN: clang -cc1 -verify -load %shlibdir/libCodeStyleChecker%shlibext -plugin CSC %s 2>&1
// RUN: clang -cc1 -verify -load %shlibdir/libCodeStyleChecker%shlibext -add-plugin CSC %s 2>&1

// RUN: clang -Xclang -load -Xclang %shlibdir/libCodeStyleChecker%shlibext -Xclang -add-plugin -Xclang CSC -c %s -o %t.o
// RUN: test -f %t.o
// RUN: rm %t.o

// RUN: clang -fplugin=%shlibdir/libCodeStyleChecker%shlibext -c %s -o %t.o
// RUN: test -f %t.o
// RUN: rm %t.o

// Verify that function names starting with upper case are reported as invalid

Expand Down
10 changes: 9 additions & 1 deletion test/CodeStyleCheckerMacro.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
// RUN: clang -cc1 -verify -load %shlibdir/libCodeStyleChecker%shlibext -plugin CSC %s 2>&1
// RUN: clang -cc1 -verify -load %shlibdir/libCodeStyleChecker%shlibext -add-plugin CSC %s 2>&1

// RUN: clang -Xclang -load -Xclang %shlibdir/libCodeStyleChecker%shlibext -Xclang -add-plugin -Xclang CSC -c %s -o %t.o
// RUN: test -f %t.o
// RUN: rm %t.o

// RUN: clang -fplugin=%shlibdir/libCodeStyleChecker%shlibext -c %s -o %t.o
// RUN: test -f %t.o
// RUN: rm %t.o

#define clang_tutor_class_ok(class_name) class ClangTutor##class_name
#define clang_tutor_class_underscore(class_name) class Clang_TutorClass##class_name
Expand Down
10 changes: 9 additions & 1 deletion test/CodeStyleCheckerTypesAndVars.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
// RUN: clang -cc1 -verify -load %shlibdir/libCodeStyleChecker%shlibext -plugin CSC %s 2>&1
// RUN: clang -cc1 -verify -load %shlibdir/libCodeStyleChecker%shlibext -add-plugin CSC %s 2>&1

// RUN: clang -Xclang -load -Xclang %shlibdir/libCodeStyleChecker%shlibext -Xclang -add-plugin -Xclang CSC -c %s -o %t.o
// RUN: test -f %t.o
// RUN: rm %t.o

// RUN: clang -fplugin=%shlibdir/libCodeStyleChecker%shlibext -c %s -o %t.o
// RUN: test -f %t.o
// RUN: rm %t.o

// Verify that type and variable names starting with lower case are reported as
// invalid
Expand Down
10 changes: 9 additions & 1 deletion test/CodeStyleCheckerUnderscore.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
// RUN: clang -cc1 -verify -load %shlibdir/libCodeStyleChecker%shlibext -plugin CSC %s 2>&1
// RUN: clang -cc1 -verify -load %shlibdir/libCodeStyleChecker%shlibext -add-plugin CSC %s 2>&1

// RUN: clang -Xclang -load -Xclang %shlibdir/libCodeStyleChecker%shlibext -Xclang -add-plugin -Xclang CSC -c %s -o %t.o
// RUN: test -f %t.o
// RUN: rm %t.o

// RUN: clang -fplugin=%shlibdir/libCodeStyleChecker%shlibext -c %s -o %t.o
// RUN: test -f %t.o
// RUN: rm %t.o

// Verify that underscare in types, variables and function names are reported
// as invalid
Expand Down
15 changes: 12 additions & 3 deletions test/CodeStyleCheckerVector.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,15 @@
// RUN: clang++ -Xclang -load -Xclang %shlibdir/libCodeStyleChecker%shlibext -Xclang -plugin -Xclang CSC -c %s
// RUN: clang++ -Xclang -load -Xclang %shlibdir/libCodeStyleChecker%shlibext -Xclang -plugin -Xclang CSC -Xclang -plugin-arg-CSC -Xclang -main-tu-only=true -c %s
// RUN: clang++ -Xclang -load -Xclang %shlibdir/libCodeStyleChecker%shlibext -Xclang -plugin -Xclang CSC -Xclang -plugin-arg-CSC -Xclang -main-tu-only=false -c %s
// RUN: clang++ -Xclang -load -Xclang %shlibdir/libCodeStyleChecker%shlibext -Xclang -add-plugin -Xclang CSC -c %s -o %t1.o 2>&1
// RUN: test -f %t1.o
// RUN: rm %t1.o

// RUN: clang++ -Xclang -load -Xclang %shlibdir/libCodeStyleChecker%shlibext -Xclang -add-plugin -Xclang CSC -Xclang -plugin-arg-CSC -Xclang -main-tu-only=true -c %s -o %t2.o 2>&1
// RUN: test -f %t2.o
// RUN: rm %t2.o

// RUN: clang++ -Xclang -load -Xclang %shlibdir/libCodeStyleChecker%shlibext -Xclang -add-plugin -Xclang CSC -Xclang -plugin-arg-CSC -Xclang -main-tu-only=false -c %s -o %t3.o 2>&1
// RUN: test -f %t3.o
// RUN: rm %t3.o


#include <vector>

Expand Down
5 changes: 5 additions & 0 deletions tools/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,9 @@ foreach( tool ${CLANG_TUTOR_TOOLS} )
${tool}
"clangTooling"
)

# ct action type should be CmdlineAfterMainAction
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to expand this comment with some explanation that will say why ct action type should be CmdlineAfterMainAction. Does this make sense?

if(${tool} STREQUAL "ct-code-style-checker")
target_compile_definitions(${tool} PRIVATE TARGET_CLANG_TOOL=1)
endif()
endforeach()
Loading