steakhal added a comment.
Really good progress. Nicestuff. I really appreciate the unittest.
However Ive got some minor comments there :D
================
Comment at: clang/test/Analysis/fixed-point.c:9
+
+enum en_t { en_0 = 1 };
+
----------------
I would suggest 'Kind' or something similar. So by reading it would feel
natural that is an enum.
Its probably a personal preference though. Disregard if you want.
================
Comment at: clang/unittests/StaticAnalyzer/APSIntTypeTest.cpp:1
+//===- unittest/StaticAnalyzer/AnalyzerOptionsTest.cpp - SA Options test
--===//
+//
----------------
Copypaste typo: SA Options?. Check the rest.
================
Comment at: clang/unittests/StaticAnalyzer/APSIntTypeTest.cpp:13
+#include "clang/Basic/TargetInfo.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h"
----------------
Do we really need checkers here? Check the rest of the includes. It will reduce
compile times inthe long run.
================
Comment at: clang/unittests/StaticAnalyzer/APSIntTypeTest.cpp:20
+
+namespace clang {
+namespace ento {
----------------
Should the content really be inside the ento namespace?
I would probably advocate for anonymous namespaces instead.
================
Comment at: clang/unittests/StaticAnalyzer/APSIntTypeTest.cpp:23
+
+TEST(getAPSIntTypeTest, foo) {
+ std::unique_ptr<ASTUnit> AST = tooling::buildASTFromCode("");
----------------
Give a meaningful name for this.
================
Comment at: clang/unittests/StaticAnalyzer/APSIntTypeTest.cpp:29
+
+ APSIntType ty = BVF.getAPSIntType(Context.LongAccumTy);
+ EXPECT_TRUE(ty.getBitWidth() == Context.getTargetInfo().getLongAccumWidth());
----------------
We use Upper Camel Case for variables as per llvm. In addition we usually refer
to QualTypes by 'Ty'.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D139759/new/
https://reviews.llvm.org/D139759
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
