Skip to content

[NFC][clang-tidy] reword diagnostic note in definitions-in-headers #106862

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

Merged

Conversation

5chmidti
Copy link
Contributor

make as inline made little sense here, so I changed the make to mark
and added the definition as well.

@llvmbot
Copy link
Member

llvmbot commented Aug 31, 2024

@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clang-tidy

Author: Julian Schmidt (5chmidti)

Changes

make as inline made little sense here, so I changed the make to mark
and added the definition as well.


Full diff: https://github.com/llvm/llvm-project/pull/106862.diff

1 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp (+1-1)
diff --git a/clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp b/clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
index 21008bc144b91a..ee869256898989 100644
--- a/clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
@@ -102,7 +102,7 @@ void DefinitionsInHeadersCheck::check(const MatchFinder::MatchResult &Result) {
     // inline is not allowed for main function.
     if (FD->isMain())
       return;
-    diag(FD->getLocation(), /*Description=*/"make as 'inline'",
+    diag(FD->getLocation(), "mark the definition as 'inline'",
          DiagnosticIDs::Note)
         << FixItHint::CreateInsertion(FD->getInnerLocStart(), "inline ");
   } else if (const auto *VD = dyn_cast<VarDecl>(ND)) {

@5chmidti
Copy link
Contributor Author

I don't think this would need a release note, but it is user-facing. WDYT?

@carlosgalvezp
Copy link
Contributor

I suppose it doesn't hurt to mention it. Don't tests need to be updated too?

@EugeneZelenko EugeneZelenko requested review from PiotrZSL, SimplyDanny and njames93 and removed request for PiotrZSL September 1, 2024 13:46
`make as inline` made little sense here, so I changed the `make` to `mark`
and added `the definition` as well.
@5chmidti 5chmidti force-pushed the clang_tidy_definitions_in_headers_note_typo branch from b3f8dc2 to 2bddf88 Compare September 16, 2024 21:19
@5chmidti 5chmidti merged commit 50320ec into llvm:main Sep 17, 2024
9 checks passed
@5chmidti 5chmidti deleted the clang_tidy_definitions_in_headers_note_typo branch September 17, 2024 08:44
tmsri pushed a commit to tmsri/llvm-project that referenced this pull request Sep 19, 2024
…lvm#106862)

`make as inline` made little sense here, so I changed the `make` to
`mark`
and added `the definition` as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants