Utilize comparison operation implemented in APInt

[SimplyDanny]

Summary

This is a fix for #53963.

[aaron.ballman]

LGTM! Can you also add a release note for the fix?

I'm happy to land this on your behalf if you'd prefer, but given that you've had several quality commits already, you might want to consider asking for your commit privileges: https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access.

[SimplyDanny]

In D122544#3411226, @aaron.ballman wrote:

LGTM! Can you also add a release note for the fix?

I'm happy to land this on your behalf if you'd prefer, but given that you've had several quality commits already, you might want to consider asking for your commit privileges: https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access.

Thank you for the review and the recommendation to become a committer! I have just been granted the privileges and have landed the change on the main branch including a mention of the fix in the release notes. I hope this is all right.

[aaron.ballman]

In D122544#3412568, @SimplyDanny wrote:

In D122544#3411226, @aaron.ballman wrote:

LGTM! Can you also add a release note for the fix?

I'm happy to land this on your behalf if you'd prefer, but given that you've had several quality commits already, you might want to consider asking for your commit privileges: https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access.

Thank you for the review and the recommendation to become a committer! I have just been granted the privileges and have landed the change on the main branch including a mention of the fix in the release notes. I hope this is all right.

Congrats!

I double-checked the commit and everything looks correct to me. Thanks for the fix!

[aaronpuchert]

Note that __int128_t is not available everywhere, generally only on 64-bit platforms.

clang-tools-extra/test/clang-tidy/checkers/bugprone-sizeof-expression.cpp

175

This causes test failures on 32-bit architectures:

/home/tcwg-buildbot/worker/clang-armv7-2stage/stage2/tools/clang/tools/extra/test/clang-tidy/checkers/Output/bugprone-sizeof-expression.cpp.tmp.cpp:175:11: error: unknown type name '__int128_t' [clang-diagnostic-error]
template <__int128_t N> 
          ^

It's probably best to wrap this in #ifdef __SIZEOF_INT128__ lest we disable the entire test on the affected platforms.

[aaron.ballman]

clang-tools-extra/test/clang-tidy/checkers/bugprone-sizeof-expression.cpp

175

Good catch! @SimplyDanny, can you fix up the test?

[SimplyDanny]

clang-tools-extra/test/clang-tidy/checkers/bugprone-sizeof-expression.cpp

175

Sure. I will fix it. Sorry for the issue ...

What is the process in this case? Push the fix directly to main or go the normal path via Phabricator review?

[aaron.ballman]

clang-tools-extra/test/clang-tidy/checkers/bugprone-sizeof-expression.cpp

175

When fixing a broken test, you can generally push directly to main. (And if the fix requires thought, it's usually best to revert the broken commit from main until you have the test fixed, then push the fixed commit back to main).

[SimplyDanny]

clang-tools-extra/test/clang-tidy/checkers/bugprone-sizeof-expression.cpp

175

Done. Fix is here.

clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp

 
 namespace clang {
 namespace tidy {
 namespace bugprone {
 
 namespace {
 
 AST_MATCHER_P(IntegerLiteral, isBiggerThan, unsigned, N) {
-  return Node.getValue().getZExtValue() > N;
+  return Node.getValue().ugt(N);
 }
 
 AST_MATCHER_P2(Expr, hasSizeOfDescendant, int, Depth,
                ast_matchers::internal::Matcher<Expr>, InnerMatcher) {
   if (Depth < 0)
     return false;
 
   const Expr *E = Node.IgnoreParenImpCasts();

clang-tools-extra/docs/ReleaseNotes.rst

 
 - Fixed a crash in :doc:`readability-const-return-type
   <clang-tidy/checks/readability-const-return-type>` when a pure virtual function
   overrided has a const return type. Removed the fix for a virtual function.
 
 - Fixed a false positive in :doc:`misc-redundant-expression <clang-tidy/checks/misc-redundant-expression>`
   involving overloaded comparison operators.
 
+- Fixed a crash in :doc:`bugprone-sizeof-expression <clang-tidy/checks/bugprone-sizeof-expression>` when
+  `sizeof(...)` is compared agains a `__int128_t`.
+
 Removed checks
 ^^^^^^^^^^^^^^
 
 Improvements to include-fixer
 -----------------------------
 
 The improvements are...
 

clang-tools-extra/test/clang-tidy/checkers/bugprone-sizeof-expression.cpp

 }
 
 template <int T>
 int Foo() { int A[T]; return sizeof(T); }
 // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: suspicious usage of 'sizeof(K)'
 template <typename T>
 int Bar() { T A[5]; return sizeof(A[0]) / sizeof(T); }
 // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'
-int Test3() { return Foo<42>() + Bar<char>(); }
+template <__int128_t N> 
+bool Baz() { return sizeof(A) < N; }
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: suspicious comparison of 'sizeof(expr)' to a constant
+int Test3() { return Foo<42>() + Bar<char>() + Baz<-1>(); }
 
 static const char* kABC = "abc";
 static const wchar_t* kDEF = L"def";
 int Test4(const char A[10]) {
   int sum = 0;
   sum += sizeof(kABC);
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(char*)'
   sum += sizeof(kDEF);