PiotrZSL added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/portability/NonPortableIntegerConstantCheck.cpp:92
+void NonPortableIntegerConstantCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(integerLiteral().bind("integer"), this);
+}
----------------
consider excluding system headers, or even exclude things like "equals(0)" to
speed up check.
================
Comment at:
clang-tools-extra/clang-tidy/portability/NonPortableIntegerConstantCheck.cpp:103
+ QualType IntegerLiteralType = MatchedInt->getType();
+ auto LiteralBitWidth = Result.Context->getTypeSize( IntegerLiteralType );
+
----------------
use type instead of auto
================
Comment at:
clang-tools-extra/clang-tidy/portability/NonPortableIntegerConstantCheck.cpp:146
+ diag(MatchedInt->getBeginLoc(),
+ "non-portable integer literal: hardcoded platform-specific maximum
value");
+ } else if (IsMin && !RepresentsZero) {
----------------
add here some hint to developer what to do, in cpp suggest using
std::numeric_limits, in C some other MAX macro.
Same for other.
================
Comment at:
clang-tools-extra/clang-tidy/portability/NonPortableIntegerConstantCheck.h:20
+///
http://clang.llvm.org/extra/clang-tidy/checks/portability/non-portable-integer-constant.html
+class NonPortableIntegerConstantCheck : public ClangTidyCheck {
+public:
----------------
this check is already in portability group, no need to duplicate "portable".
Maybe just portability-integer-constant
================
Comment at:
clang-tools-extra/docs/clang-tidy/checks/portability/non-portable-integer-constant.rst:22
+ return x ^ 0xFFFFFFFF;
+ // The right way to write this would be ULONG_MAX, or -1.
+ }
----------------
well, not, right way would be using std::numeric_limits<std::uint32_t>::max().
And unsigned long can be 4 bytes or 8 bytes or anything compiler decide.
So this example got more issues., and 0XFFF.. is a last of them, and -1 is not
valid in such case.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/portability/non-portable-integer-constant.cpp:1
+// RUN: %check_clang_tidy %s -std=c++17-or-later
portability-non-portable-integer-constant %t
+
----------------
no need to limit this check to c++17 or later.
when it's targeting all versions of C and C++
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/portability/non-portable-integer-constant.cpp:229
+ FULL_LITERAL;
+ // --CHECK-MESSAGES: :[[@LINE-1]]:3: warning: non-portable integer literal
+ // --CHECK-MESSAGES: :[[@LINE-3]]:22: note: expanded from macro
----------------
exclude system macros, otherwise it may warn for things like ULONG_MAX
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146712/new/
https://reviews.llvm.org/D146712
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
