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

[clang-tidy] Ignore user-defined literals in google-runtime-int #78859

Merged
merged 2 commits into from
Jan 22, 2024

Conversation

felix642
Copy link
Contributor

User-defined literals do not accept u?intXX(_t)? variables. So the check should not emit a warning.

Fixes #54546 #25214

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 20, 2024

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

@llvm/pr-subscribers-clang-tidy

Author: Félix-Antoine Constantin (felix642)

Changes

User-defined literals do not accept u?intXX(_t)? variables. So the check should not emit a warning.

Fixes #54546 #25214


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

3 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp (+8-5)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+3)
  • (modified) clang-tools-extra/test/clang-tidy/checkers/google/runtime-int.cpp (+6-4)
diff --git a/clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp b/clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
index bb4e1de8cc4b15..b60f2d0ed63112 100644
--- a/clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
@@ -56,11 +56,14 @@ void IntegerTypesCheck::registerMatchers(MatchFinder *Finder) {
   // http://google.github.io/styleguide/cppguide.html#64-bit_Portability
   // "Where possible, avoid passing arguments of types specified by
   // bitwidth typedefs to printf-based APIs."
-  Finder->addMatcher(typeLoc(loc(isInteger()),
-                             unless(hasAncestor(callExpr(
-                                 callee(functionDecl(hasAttr(attr::Format)))))))
-                         .bind("tl"),
-                     this);
+  Finder->addMatcher(
+      typeLoc(loc(isInteger()),
+              unless(anyOf(hasAncestor(callExpr(
+                               callee(functionDecl(hasAttr(attr::Format))))),
+                           hasParent(parmVarDecl(hasAncestor(functionDecl(
+                               matchesName("operator\"\".*$"))))))))
+          .bind("tl"),
+      this);
   IdentTable = std::make_unique<IdentifierTable>(getLangOpts());
 }
 
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index a235a7d02592e8..5c857824c2b021 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -359,6 +359,9 @@ Changes in existing checks
   to ignore unused parameters when they are marked as unused and parameters of
   deleted functions and constructors.
 
+- Improved :doc:`google-runtime-int` check to ignore
+  false positives on user defined-literals.
+
 - Improved :doc:`llvm-namespace-comment
   <clang-tidy/checks/llvm/namespace-comment>` check to provide fixes for
   ``inline`` namespaces in the same format as :program:`clang-format`.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/google/runtime-int.cpp b/clang-tools-extra/test/clang-tidy/checkers/google/runtime-int.cpp
index 4bb739876b7f4c..88f0b519e9cbee 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/google/runtime-int.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/google/runtime-int.cpp
@@ -59,11 +59,13 @@ void qux() {
 // CHECK-MESSAGES: [[@LINE-1]]:3: warning: consider replacing 'short' with 'int16'
 }
 
-// FIXME: This shouldn't warn, as UD-literal operators require one of a handful
-// of types as an argument.
 struct some_value {};
-constexpr some_value operator"" _some_literal(unsigned long long int i);
-// CHECK-MESSAGES: [[@LINE-1]]:47: warning: consider replacing 'unsigned long long'
+constexpr some_value operator"" _some_literal(unsigned long long int i)
+{
+  short j;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: consider replacing 'short' with 'int16'
+  return some_value();
+}
 
 struct A { A& operator=(const A&); };
 class B { A a[0]; };

@PiotrZSL PiotrZSL linked an issue Jan 20, 2024 that may be closed by this pull request
User-defined literals do not accept u?intXX(_t)? variables.
So the check should not emit a warning.

Fixes llvm#54546 llvm#25214
Fixed Documentation
Improved ast matcher to use getLiteralIdentifier
@felix642 felix642 force-pushed the google-runtime-user-defined-literals branch from b54e033 to 5495d86 Compare January 21, 2024 15:36
@felix642
Copy link
Contributor Author

Rebased with main to fix merge conflicts.

Copy link
Member

@PiotrZSL PiotrZSL left a comment

Choose a reason for hiding this comment

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

LGTM

@felix642
Copy link
Contributor Author

@PiotrZSL would you be able to commit those changes for me please ?

@PiotrZSL PiotrZSL merged commit ee0b4d9 into llvm:main Jan 22, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants