[clang-tidy] Add modernize-use-cpp-style-comments check #99713
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
| @llvm/pr-subscribers-clang-tidy Author: NagaChaitanya Vellanki (chaitanyav) Changesmodernize-use-cpp-style-comments check finds C style comments Fixes #24841 @LegalizeAdulthood @PiotrZSL Full diff: https://github.com/llvm/llvm-project/pull/99713.diff 7 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt index 4f68c487cac9d..04a1d04cc333e 100644 --- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt @@ -31,6 +31,7 @@ add_clang_library(clangTidyModernizeModule UseAutoCheck.cpp UseBoolLiteralsCheck.cpp UseConstraintsCheck.cpp + UseCppStyleCommentsCheck.cpp UseDefaultMemberInitCheck.cpp UseDesignatedInitializersCheck.cpp UseEmplaceCheck.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp index 1860759332063..39995a32133b3 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -32,6 +32,7 @@ #include "UseAutoCheck.h" #include "UseBoolLiteralsCheck.h" #include "UseConstraintsCheck.h" +#include "UseCppStyleCommentsCheck.h" #include "UseDefaultMemberInitCheck.h" #include "UseDesignatedInitializersCheck.h" #include "UseEmplaceCheck.h" @@ -104,6 +105,8 @@ class ModernizeModule : public ClangTidyModule { "modernize-use-bool-literals"); CheckFactories.registerCheck<UseConstraintsCheck>( "modernize-use-constraints"); + CheckFactories.registerCheck<UseCppStyleCommentsCheck>( + "modernize-use-cpp-style-comments"); CheckFactories.registerCheck<UseDefaultMemberInitCheck>( "modernize-use-default-member-init"); CheckFactories.registerCheck<UseEmplaceCheck>("modernize-use-emplace"); diff --git a/clang-tools-extra/clang-tidy/modernize/UseCppStyleCommentsCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseCppStyleCommentsCheck.cpp new file mode 100644 index 0000000000000..f4d3bbce9f121 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseCppStyleCommentsCheck.cpp @@ -0,0 +1,66 @@ +//===--- UseCppStyleCommentsCheck.cpp - clang-tidy-----------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "UseCppStyleCommentsCheck.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" +#include "clang/Lex/Preprocessor.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::modernize { + +class UseCppStyleCommentsCheck::CStyleCommentHandler : public CommentHandler { +public: + CStyleCommentHandler(UseCppStyleCommentsCheck &Check) + : Check(Check), + CStyleCommentMatch( + "^[ \t]*/\\*+[ \t\n]*(.*[ \t\n]*)*[ \t\n]*\\*+/[ \t\n]*$") {} + + bool HandleComment(Preprocessor &PP, SourceRange Range) override { + + if (Range.getBegin().isMacroID() || + PP.getSourceManager().isInSystemHeader(Range.getBegin())) + return false; + + const StringRef Text = + Lexer::getSourceText(CharSourceRange::getCharRange(Range), + PP.getSourceManager(), PP.getLangOpts()); + + SmallVector<StringRef> Matches; + if (!CStyleCommentMatch.match(Text, &Matches)) { + return false; + } + + Check.diag( + Range.getBegin(), + "use C++ style comments '//' instead of C style comments '/*...*/'") + << FixItHint::CreateRemoval(CharSourceRange::getCharRange(Range)); + + return false; + } + +private: + UseCppStyleCommentsCheck &Check; + llvm::Regex CStyleCommentMatch; +}; + +UseCppStyleCommentsCheck::UseCppStyleCommentsCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + Handler(std::make_unique<CStyleCommentHandler>(*this)) {} + +void UseCppStyleCommentsCheck::registerPPCallbacks( + const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { + PP->addCommentHandler(Handler.get()); +} + +void UseCppStyleCommentsCheck::check(const MatchFinder::MatchResult &Result) {} + +UseCppStyleCommentsCheck::~UseCppStyleCommentsCheck() = default; +} // namespace clang::tidy::modernize diff --git a/clang-tools-extra/clang-tidy/modernize/UseCppStyleCommentsCheck.h b/clang-tools-extra/clang-tidy/modernize/UseCppStyleCommentsCheck.h new file mode 100644 index 0000000000000..81b0fdeb018b5 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseCppStyleCommentsCheck.h @@ -0,0 +1,38 @@ +//===--- UseCppStyleCommentsCheck.h - clang-tidy---------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USE_CPP_STYLE_COMMENTS_CHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USE_CPP_STYLE_COMMENTS_CHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::modernize { +/// detects C-style comments and suggests to use C++ style comments instead +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/modernize/use-cpp-style-comments.html +class UseCppStyleCommentsCheck : public ClangTidyCheck { +public: + UseCppStyleCommentsCheck(StringRef Name, ClangTidyContext *Context); + ~UseCppStyleCommentsCheck() override; + + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus; + } + + void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + class CStyleCommentHandler; + std::unique_ptr<CStyleCommentHandler> Handler; +}; +} // namespace clang::tidy::modernize + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USE_CPP_STYLE_COMMENTS_CHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index a23483e6df6d2..95286962f07bb 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -172,6 +172,11 @@ New checks Detects variables and functions that can be marked as static or moved into an anonymous namespace to enforce internal linkage. +- New :doc:`modernize-use-cpp-style-comments + <clang-tidy/checks/modernize/use-cpp-style-comments>` check. + + Find C Style comments and suggests to use C++ style comments instead + - New :doc:`modernize-min-max-use-initializer-list <clang-tidy/checks/modernize/min-max-use-initializer-list>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-cpp-style-comments.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-cpp-style-comments.rst new file mode 100644 index 0000000000000..4b2f10420dd97 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-cpp-style-comments.rst @@ -0,0 +1,12 @@ +.. title:: clang-tidy - modernize-use-cpp-style-comments + +modernize-use-cpp-style-comments +======================== + +Finds C-style comments and suggests to use C++ style comments `//`. + + +.. code-block:: c++ + + memcpy(a, b, sizeof(int) * 5); /* use std::copy_n instead of memcpy */ + // warning: use C++ style comments '//' instead of C style comments '/*...*/' [modernize-use-cpp-style-comments] \ No newline at end of file diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-cpp-style-comments.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-cpp-style-comments.cpp new file mode 100644 index 0000000000000..79abf5a53fa2f --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-cpp-style-comments.cpp @@ -0,0 +1,16 @@ +// RUN: %check_clang_tidy -std=c++11 %s modernize-use-cpp-style-comments %t + +static auto PI = 3.14159265; /* value of pi upto 8 decimal places */ +// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: use C++ style comments '//' instead of C style comments '/*...*/' [modernize-use-cpp-style-comments] + +/****************************************************** +* Fancy frame comment goes here +*******************************************************/ +// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: use C++ style comments '//' instead of C style comments '/*...*/' [modernize-use-cpp-style-comments] + +/** \brief Brief description. + * Brief description continued. + * + * Detailed description starts here. (this is a doxygen comment) + */ +// CHECK-MESSAGES: :[[@LINE-5]]:1: warning: use C++ style comments '//' instead of C style comments '/*...*/' [modernize-use-cpp-style-comments] |
| @llvm/pr-subscribers-clang-tools-extra Author: NagaChaitanya Vellanki (chaitanyav) Changesmodernize-use-cpp-style-comments check finds C style comments Fixes #24841 @LegalizeAdulthood @PiotrZSL Full diff: https://github.com/llvm/llvm-project/pull/99713.diff 7 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt index 4f68c487cac9d..04a1d04cc333e 100644 --- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt @@ -31,6 +31,7 @@ add_clang_library(clangTidyModernizeModule UseAutoCheck.cpp UseBoolLiteralsCheck.cpp UseConstraintsCheck.cpp + UseCppStyleCommentsCheck.cpp UseDefaultMemberInitCheck.cpp UseDesignatedInitializersCheck.cpp UseEmplaceCheck.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp index 1860759332063..39995a32133b3 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -32,6 +32,7 @@ #include "UseAutoCheck.h" #include "UseBoolLiteralsCheck.h" #include "UseConstraintsCheck.h" +#include "UseCppStyleCommentsCheck.h" #include "UseDefaultMemberInitCheck.h" #include "UseDesignatedInitializersCheck.h" #include "UseEmplaceCheck.h" @@ -104,6 +105,8 @@ class ModernizeModule : public ClangTidyModule { "modernize-use-bool-literals"); CheckFactories.registerCheck<UseConstraintsCheck>( "modernize-use-constraints"); + CheckFactories.registerCheck<UseCppStyleCommentsCheck>( + "modernize-use-cpp-style-comments"); CheckFactories.registerCheck<UseDefaultMemberInitCheck>( "modernize-use-default-member-init"); CheckFactories.registerCheck<UseEmplaceCheck>("modernize-use-emplace"); diff --git a/clang-tools-extra/clang-tidy/modernize/UseCppStyleCommentsCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseCppStyleCommentsCheck.cpp new file mode 100644 index 0000000000000..f4d3bbce9f121 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseCppStyleCommentsCheck.cpp @@ -0,0 +1,66 @@ +//===--- UseCppStyleCommentsCheck.cpp - clang-tidy-----------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "UseCppStyleCommentsCheck.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" +#include "clang/Lex/Preprocessor.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::modernize { + +class UseCppStyleCommentsCheck::CStyleCommentHandler : public CommentHandler { +public: + CStyleCommentHandler(UseCppStyleCommentsCheck &Check) + : Check(Check), + CStyleCommentMatch( + "^[ \t]*/\\*+[ \t\n]*(.*[ \t\n]*)*[ \t\n]*\\*+/[ \t\n]*$") {} + + bool HandleComment(Preprocessor &PP, SourceRange Range) override { + + if (Range.getBegin().isMacroID() || + PP.getSourceManager().isInSystemHeader(Range.getBegin())) + return false; + + const StringRef Text = + Lexer::getSourceText(CharSourceRange::getCharRange(Range), + PP.getSourceManager(), PP.getLangOpts()); + + SmallVector<StringRef> Matches; + if (!CStyleCommentMatch.match(Text, &Matches)) { + return false; + } + + Check.diag( + Range.getBegin(), + "use C++ style comments '//' instead of C style comments '/*...*/'") + << FixItHint::CreateRemoval(CharSourceRange::getCharRange(Range)); + + return false; + } + +private: + UseCppStyleCommentsCheck &Check; + llvm::Regex CStyleCommentMatch; +}; + +UseCppStyleCommentsCheck::UseCppStyleCommentsCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + Handler(std::make_unique<CStyleCommentHandler>(*this)) {} + +void UseCppStyleCommentsCheck::registerPPCallbacks( + const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { + PP->addCommentHandler(Handler.get()); +} + +void UseCppStyleCommentsCheck::check(const MatchFinder::MatchResult &Result) {} + +UseCppStyleCommentsCheck::~UseCppStyleCommentsCheck() = default; +} // namespace clang::tidy::modernize diff --git a/clang-tools-extra/clang-tidy/modernize/UseCppStyleCommentsCheck.h b/clang-tools-extra/clang-tidy/modernize/UseCppStyleCommentsCheck.h new file mode 100644 index 0000000000000..81b0fdeb018b5 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseCppStyleCommentsCheck.h @@ -0,0 +1,38 @@ +//===--- UseCppStyleCommentsCheck.h - clang-tidy---------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USE_CPP_STYLE_COMMENTS_CHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USE_CPP_STYLE_COMMENTS_CHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::modernize { +/// detects C-style comments and suggests to use C++ style comments instead +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/modernize/use-cpp-style-comments.html +class UseCppStyleCommentsCheck : public ClangTidyCheck { +public: + UseCppStyleCommentsCheck(StringRef Name, ClangTidyContext *Context); + ~UseCppStyleCommentsCheck() override; + + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus; + } + + void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + class CStyleCommentHandler; + std::unique_ptr<CStyleCommentHandler> Handler; +}; +} // namespace clang::tidy::modernize + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USE_CPP_STYLE_COMMENTS_CHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index a23483e6df6d2..95286962f07bb 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -172,6 +172,11 @@ New checks Detects variables and functions that can be marked as static or moved into an anonymous namespace to enforce internal linkage. +- New :doc:`modernize-use-cpp-style-comments + <clang-tidy/checks/modernize/use-cpp-style-comments>` check. + + Find C Style comments and suggests to use C++ style comments instead + - New :doc:`modernize-min-max-use-initializer-list <clang-tidy/checks/modernize/min-max-use-initializer-list>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-cpp-style-comments.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-cpp-style-comments.rst new file mode 100644 index 0000000000000..4b2f10420dd97 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-cpp-style-comments.rst @@ -0,0 +1,12 @@ +.. title:: clang-tidy - modernize-use-cpp-style-comments + +modernize-use-cpp-style-comments +======================== + +Finds C-style comments and suggests to use C++ style comments `//`. + + +.. code-block:: c++ + + memcpy(a, b, sizeof(int) * 5); /* use std::copy_n instead of memcpy */ + // warning: use C++ style comments '//' instead of C style comments '/*...*/' [modernize-use-cpp-style-comments] \ No newline at end of file diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-cpp-style-comments.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-cpp-style-comments.cpp new file mode 100644 index 0000000000000..79abf5a53fa2f --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-cpp-style-comments.cpp @@ -0,0 +1,16 @@ +// RUN: %check_clang_tidy -std=c++11 %s modernize-use-cpp-style-comments %t + +static auto PI = 3.14159265; /* value of pi upto 8 decimal places */ +// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: use C++ style comments '//' instead of C style comments '/*...*/' [modernize-use-cpp-style-comments] + +/****************************************************** +* Fancy frame comment goes here +*******************************************************/ +// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: use C++ style comments '//' instead of C style comments '/*...*/' [modernize-use-cpp-style-comments] + +/** \brief Brief description. + * Brief description continued. + * + * Detailed description starts here. (this is a doxygen comment) + */ +// CHECK-MESSAGES: :[[@LINE-5]]:1: warning: use C++ style comments '//' instead of C style comments '/*...*/' [modernize-use-cpp-style-comments] |
| ✅ With the latest revision this PR passed the C/C++ code formatter. |
chaitanyav force-pushed the issue_24841 branch 2 times, most recently from 0bee2b5 to 2899eab Compare chaitanyav changed the title 
njames93 requested changes Jul 20, 2024
| .. code-block:: c++ | ||
| | ||
| memcpy(a, b, sizeof(int) * 5); /* use std::copy_n instead of memcpy */ | ||
| // warning: use C++ style comments '//' instead of C style comments '/*...*/' [modernize-use-cpp-style-comments] No newline at end of file |
| Check.diag( | ||
| Range.getBegin(), | ||
| "use C++ style comments '//' instead of C style comments '/*...*/'") | ||
| << FixItHint::CreateRemoval(CharSourceRange::getCharRange(Range)); |
There was a problem hiding this comment.
Just deleting a comment sounds like broken logic
| // RUN: %check_clang_tidy -std=c++11 %s modernize-use-cpp-style-comments %t | ||
| | ||
| static auto PI = 3.14159265; /* value of pi upto 8 decimal places */ | ||
| // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: use C++ style comments '//' instead of C style comments '/*...*/' [modernize-use-cpp-style-comments] |
There was a problem hiding this comment.
If you are going to emit fixes in a check you must demonstrate those fixes in the test files using the // CHECK-FIXES directive
| .. code-block:: c++ | ||
| | ||
| memcpy(a, b, sizeof(int) * 5); /* use std::copy_n instead of memcpy */ | ||
| // warning: use C++ style comments '//' instead of C style comments '/*...*/' [modernize-use-cpp-style-comments] No newline at end of file |
There was a problem hiding this comment.
Documentation shouldn't just print the diagnostic.
| - New :doc:`modernize-use-cpp-style-comments | ||
| <clang-tidy/checks/modernize/use-cpp-style-comments>` check. | ||
| | ||
| Find C Style comments and suggests to use C++ style comments instead |
There was a problem hiding this comment.
End sentences with a full stop.
| #include "../ClangTidyCheck.h" | ||
| | ||
| namespace clang::tidy::modernize { | ||
| /// detects C-style comments and suggests to use C++ style comments instead |
There was a problem hiding this comment.
Missing full stop at the end of a comment.
| class UseCppStyleCommentsCheck : public ClangTidyCheck { | ||
| public: | ||
| UseCppStyleCommentsCheck(StringRef Name, ClangTidyContext *Context); | ||
| ~UseCppStyleCommentsCheck() override; |
There was a problem hiding this comment.
Is there any reason to explicitly override the destructor? Given you are just defaulting it and there are no other special member functions explicitly defined, this can probably go.
There was a problem hiding this comment.
Looks to be due to private handler class
| | ||
| void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, | ||
| Preprocessor *ModuleExpanderPP) override; | ||
| void check(const ast_matchers::MatchFinder::MatchResult &Result) override; |
There was a problem hiding this comment.
This callback will never get run, so there is no need to override it.
| | ||
| private: | ||
| class CStyleCommentHandler; | ||
| std::unique_ptr<CStyleCommentHandler> Handler; |
There was a problem hiding this comment.
From a design point it could be simpler to have this class privately inherit from CommentHandler, and just override the handleComment function in this class instead.
Though I'm pretty easy sticking with how it is atm
PiotrZSL requested changes Jul 20, 2024
| Check.diag( | ||
| Range.getBegin(), | ||
| "use C++ style comments '//' instead of C style comments '/*...*/'") | ||
| << FixItHint::CreateRemoval(CharSourceRange::getCharRange(Range)); |
There was a problem hiding this comment.
modernize check shouldn't remove comments, it should transform them to proper one
| modernize-use-cpp-style-comments | ||
| ================================ | ||
| | ||
| Finds C-style comments and suggests to use C++ style comments `//`. |
There was a problem hiding this comment.
this line should be synchronized with release notes and class description
| CStyleCommentHandler(UseCppStyleCommentsCheck &Check) | ||
| : Check(Check), | ||
| CStyleCommentMatch( | ||
| "^[ \t]*/\\*+[ \t\n]*(.*[ \t\n]*)*[ \t\n]*\\*+/[ \t\n]*$") {} |
clang-tools-extra/test/clang-tidy/checkers/modernize/use-cpp-style-comments.cpp Show resolved Hide resolved
| class UseCppStyleCommentsCheck : public ClangTidyCheck { | ||
| public: | ||
| UseCppStyleCommentsCheck(StringRef Name, ClangTidyContext *Context); | ||
| ~UseCppStyleCommentsCheck() override; |
There was a problem hiding this comment.
Looks to be due to private handler class
HerrCai0907 reviewed Jul 20, 2024
| CStyleCommentHandler(UseCppStyleCommentsCheck &Check) | ||
| : Check(Check), | ||
| CStyleCommentMatch( | ||
| "^[ \t]*/\\*+[ \t\n]*(.*[ \t\n]*)*[ \t\n]*\\*+/[ \t\n]*$") {} |
There was a problem hiding this comment.
Could you explain more about the regex in the comment?
carlosgalvezp requested changes Jul 20, 2024
There was a problem hiding this comment.
Good check idea! A couple of comments:
-
This is not really a "modernize"check. Both styles are equally valid and it's a matter of taste. Modernize is typically for checks that bump from one standard to a newer one.
-
Instead I think this check fits better in "readability".
-
I think we can take the opportunity and make the check scope slightly more general, so that it enforces ether C style comments or C++ style comments. So the check could be called "readability-comment-style". Right now you can support only C->C++ conversion but can be extended in the future to do the opposite.
5chmidti reviewed Jul 20, 2024
chaitanyav force-pushed the issue_24841 branch 2 times, most recently from 2d45f81 to 670bf7a Compare 
modernize-use-cpp-style-comments check finds C style comments and suggests to use C++ style comments Fixes llvm#24841
| I am moving this under readability, i will also post some code on how am i planning to output a fix. For now i don't have a good way to handle |
There is no way to handle that and you shouldn't try. Best you could do is just emit a warning without a fix, but honestly in that situation not warning would also be acceptable |
I can see the value in having a configuration option to disable the warning in this particular case. Same for when one wants to show the name of a function argument at the call site. |
Another sticky point actually is that cpp style comments are often used when passing arguments to functions or to indicate a function parameter is unused void overriddenMethod(int /* unused_for_this */) override { fnWithSomeBools(/*ControlsA=*/ true, /*ControlsB=*/ false); }I'd say in these situations there probably isn't ever a reason to emit a warning |
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
modernize-use-cpp-style-comments check finds C style comments
and suggests to use C++ style comments
Fixes #24841
@LegalizeAdulthood @PiotrZSL