xazax.hun added a comment.
Thank you for working on this!
I have two concerns:
- The logic when should we import the initializer of a VarDecl is duplicated. I
think to have better maintainability it would be better to have only one
implementation of the decision in a function and call it multiple times.
- While the static analyzer is currently the only user of CTU lib it might
change in the future. Only importing initializers of const variables is an
analyzer specific decision which might not be good for other potential users of
the CTU library. Maybe it would be better to be able to configure the
ClangExtDefMapGen somehow, so it is able to both export only vardecls that are
required by the analyzer or anything?
================
Comment at: lib/CrossTU/CrossTranslationUnit.cpp:163
+static bool hasBodyOrInit(const VarDecl *D, const VarDecl *&DefD) {
+ return D->getAnyInitializer(DefD) != nullptr;
+}
----------------
The `!= nullptr` part is usually not written in LLVM codebase.
================
Comment at: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:357
+ return true;
+ if (!RTy->hasConstFields())
+ return true;
----------------
I wonder what would happen with types that has const fields and user written
constructors. In case we will not simulate the effect of the constructor and
will not be able to set the const fields maybe we should exclude those as well?
================
Comment at: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:362
+ // Only import if const.
+ if (!Ctx->getCanonicalType(VD->getType()).isConstQualified())
+ return true;
----------------
Maybe this check could be hoisted so we do not need to repeat in both branches?
================
Comment at: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:369
+
+ if (VD->getAnyInitializer() != nullptr)
+ return true;
----------------
Redundant `!= nullptr`.
================
Comment at: tools/clang-extdef-mapping/ClangExtDefMapGen.cpp:68
+ QualType VTy = VD->getType();
+ bool containsConst = VTy.isConstQualified();
+ if (!containsConst && !VTy.isNull())
----------------
Variables should start with a capital letter.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D46421/new/
https://reviews.llvm.org/D46421
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
