Skip to content
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

misc: Add gen-compile-commands.py for code analysis #1756

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

honggyukim
Copy link
Collaborator

This patch adds gen-compile-commands.py script that generates compile_commands.json, which is a JSON Compilation Database Format Specification[1].

This compile_commands.json can be used by static analysis tools such as clang-tidy[2] for advanced diagnosis or lintting.

The usage is as follows.

  $ ./configure

  $ make clean    # To print build commands for all source files.
  $ ./misc/gen-compile-commands.py
  (... compile_commands.json generated ...)

  $ run-clang-tidy

[1] https://clang.llvm.org/docs/JSONCompilationDatabase.html
[2] https://clang.llvm.org/extra/clang-tidy
Signed-off-by: Honggyu Kim [email protected]

@honggyukim
Copy link
Collaborator Author

You can see that the second patch is based on the automatic fix by clang-tidy. It's just show an example. I will drop the second patch before merging this PR.

@honggyukim
Copy link
Collaborator Author

There are many useful checkers in clang-tidy.
https://clang.llvm.org/docs/analyzer/checkers.html

Copy link
Owner

@namhyung namhyung left a comment

Choose a reason for hiding this comment

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

The changes in the second commit are too many so I don't know which one is meaningful. It'd be nice if we can control clang-tidy what to report.

lines = do_dry_make()
for line in lines:
command = line
if command[:3] != 'gcc':
Copy link
Owner

Choose a reason for hiding this comment

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

Why do you need this? I guess you wanted clang as well, no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've added a comment as follows.

We're only interested in compilation commands so skip otherwise.

@honggyukim honggyukim force-pushed the check/gen-compile-commands.py branch from 3376704 to 16fcbf2 Compare August 11, 2023 15:37
@honggyukim
Copy link
Collaborator Author

The changes in the second commit are too many so I don't know which one is meaningful. It'd be nice if we can control clang-tidy what to report.

The second commit was to give you an idea what kind of changes can be made by clang-tidy. I've just dropped it and left only the compile_commands.json generation script.

You can still see what kind of warnings can be shown by running run-clang-tidy. We can set up only the checkers we're interested in at .clang-tidy.

You can take a look at the following as references.

This patch adds gen-compile-commands.py script that generates
compile_commands.json, which is a JSON Compilation Database Format
Specification[1].

This compile_commands.json can be used by static analysis tools such as
clang-tidy[2] for advanced diagnosis or lintting.

The usage is as follows.

  $ ./configure

  $ make clean    # To print build commands for all source files.
  $ ./misc/gen-compile-commands.py
  (... compile_commands.json generated ...)

  $ run-clang-tidy

[1] https://clang.llvm.org/docs/JSONCompilationDatabase.html
[2] https://clang.llvm.org/extra/clang-tidy
Signed-off-by: Honggyu Kim <[email protected]>
@honggyukim honggyukim force-pushed the check/gen-compile-commands.py branch from 16fcbf2 to f9a29d0 Compare August 15, 2023 09:34
@honggyukim
Copy link
Collaborator Author

I've added one more commit that applies readability-non-const-parameter checker for reference.

I'm fine to drop the second commit. It's just to give an idea what clang-tidy can do.

This patch applies automatic fixes by running the following clang-tidy
command.

  $ run-clang-tidy -fix -checks='-*,readability-non-const-parameter'

A few of manual modification are also applied to make const correctness.

Link: https://clang.llvm.org/extra/clang-tidy/checks/readability/non-const-parameter.html
Signed-off-by: Honggyu Kim <[email protected]>
This patch applies automatic fixes by running the following clang-tidy
command.

  $ run-clang-tidy -fix -checks='-*,readability-isolate-declaration'

Link: https://clang.llvm.org/extra/clang-tidy/checks/readability/isolate-declaration.html
Signed-off-by: Honggyu Kim <[email protected]>
This patch applies automatic fixes by running the following clang-tidy
command.

  $ run-clang-tidy -fix -checks='-*,readability-else-after-return'

Link: https://clang.llvm.org/extra/clang-tidy/checks/readability/else-after-return.html
Signed-off-by: Honggyu Kim <[email protected]>
… checker

This patch applies automatic fixes by a checker to catch inconsistent
declaration parameter name by running the following command.

  $ run-clang-tidy -fix -checks='-*,readability-inconsistent-declaration-parameter-name'

Link: https://clang.llvm.org/extra/clang-tidy/checks/readability/inconsistent-declaration-parameter-name.html
Signed-off-by: Honggyu Kim <[email protected]>
This patch applies automatic fixes by running the following clang-tidy
command.

  $ run-clang-tidy -fix -checks='-*,readability-redundant-declaration'

Link: https://clang.llvm.org/extra/clang-tidy/checks/readability/redundant-declaration.html
Signed-off-by: Honggyu Kim <[email protected]>
This patch applies automatic fixes by running the following clang-tidy
command.

  $ run-clang-tidy -fix -checks='-*,bugprone-macro-parentheses'

Link: https://clang.llvm.org/extra/clang-tidy/checks/bugprone/macro-parentheses.html
Signed-off-by: Honggyu Kim <[email protected]>
This patch applies automatic fixes by running the following clang-tidy
command.

  $ run-clang-tidy -fix -checks='-*,bugprone-suspicious-string-compare'

Link: https://clang.llvm.org/extra/clang-tidy/checks/bugprone/suspicious-string-compare.html
Signed-off-by: Honggyu Kim <[email protected]>
@honggyukim honggyukim force-pushed the check/gen-compile-commands.py branch from f9a29d0 to 87aac06 Compare August 15, 2023 12:48
@honggyukim
Copy link
Collaborator Author

I've just added a few more commits that applies each clang-tidy checkers.

Copy link
Owner

@namhyung namhyung left a comment

Choose a reason for hiding this comment

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

Is it possible to disable clang-tidy in some specific code?

@@ -1,3 +1,4 @@
Checks: >-
-*,
readability-non-const-parameter,
readability-isolate-declaration,
Copy link
Owner

Choose a reason for hiding this comment

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

Not sure if it's worth.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ack. I will remove it.

@@ -6,3 +6,4 @@ Checks: >-
readability-inconsistent-declaration-parameter-name,
readability-redundant-declaration,
bugprone-macro-parentheses,
bugprone-suspicious-string-compare,
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think it's better. :(

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ack. will remove it as well.

Copy link
Collaborator Author

@honggyukim honggyukim left a comment

Choose a reason for hiding this comment

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

Is it possible to disable clang-tidy in some specific code?

Yes, we can ignore clang-tidy warning as follows.

If a specific suppression mechanism is not available for a certain warning, or its use is not desired for some reason, clang-tidy has a generic mechanism to suppress diagnostics using NOLINT, NOLINTNEXTLINE, and NOLINTBEGIN … NOLINTEND comments.

https://clang.llvm.org/extra/clang-tidy/#suppressing-undesired-diagnostics

@@ -1,3 +1,4 @@
Checks: >-
-*,
readability-non-const-parameter,
readability-isolate-declaration,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ack. I will remove it.

@@ -6,3 +6,4 @@ Checks: >-
readability-inconsistent-declaration-parameter-name,
readability-redundant-declaration,
bugprone-macro-parentheses,
bugprone-suspicious-string-compare,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ack. will remove it as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants