martong marked 3 inline comments as done.
martong added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:851
+ } getPointerTy(ACtx);
+ class {
+ public:
----------------
balazske wrote:
> Why has this class different layout than `GetPointerTy` and `GetRestrictTy`
> (beneath that here is no `ASTContext`)? It would be better if all these
> classes look the same way: First is the operator with `QualType`, then
> operator with `Optional` that calls the other version of the operator. And
> all of these can be "unnamed" classes?
I'd had the same thoughts before, and was thinking about that maybe a class
template would suffice for all these, but then I realized the following:
To make a type const, we don't need the ASTContext, that is the reason of the
difference.
However, to make a type restricted or a pointer, we need the ASTContext. This
is a legacy non-symmetry from the AST.
> And all of these can be "unnamed" classes?
GetPointerTy and GetRestrictTy need the ASTContext in their constructor. And
you cannot define a constructor to an unnamed class because you cannot write
down the nonexistent name.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:890
- const QualType VoidPtrTy = ACtx.VoidPtrTy; // void *
- const QualType IntPtrTy = ACtx.getPointerType(IntTy); // int *
+ const QualType VoidPtrTy = getPointerTy(VoidTy); // void *
+ const QualType IntPtrTy = getPointerTy(IntTy); // int *
----------------
balazske wrote:
> Is it better to use `ACtx.VoidPtrTy`?
I'd like to use the `ACtx` as less as possible: just to get the basic types.
And from that on we can use our convenience API (GetConstTy, GetPointerTy, etc)
to define all of the derived types... and we can do that in a declarative form
(like ASTMatchters).
Also, the declarative definition of these types now look the same regardless
the type is "derived" from a looked-up type or from a built-in type.
================
Comment at:
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1321
- Optional<QualType> Mode_tTy = lookupType("mode_t", ACtx);
+ Optional<QualType> Mode_tTy = lookupTy("mode_t");
----------------
balazske wrote:
> It is better to get every type at the start before adding functions, or group
> the functions and get some types at the start of these groups but mark the
> groups at least with comments.
Well, with looked-up types I followed the usual convention to define a variable
right before using it. This means that we lookup a type just before we try to
add the function which first uses that type.
However, builtin types are defined at the beginning, because they are used very
often.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86531/new/
https://reviews.llvm.org/D86531
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
