zaks.anna added inline comments.
================
Comment at: lib/Analysis/BodyFarm.h:43
@@ +42,3 @@
+ /// \brief Merge the attributes found in model files.
+ /// Note, this adds all attributes found in the model file without any sanity
+ /// checks.
----------------
Why do we not have sanity checks? What happens when there is a conflict?
================
Comment at: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:505
@@ +504,3 @@
+// AST visitor used to merge model attributes.
+class WalkAST : public DataRecursiveASTVisitor<WalkAST> {
+ AnalysisDeclContextManager &Mgr;
----------------
This name is too generic.
================
Comment at: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:541
@@ +540,3 @@
+ // If "faux-attributes=true" is given, merge model's attributes.
+ AnalysisDeclContextManager &ADCMgr = Mgr->getAnalysisDeclContextManager();
+ if (ADCMgr.mergeModelAttributes()) {
----------------
Should we walk the entire AST here only to get the info from the few functions
in the farm?
The AnalysisConsumer visitor tries to make sure the whole AST is not
serialized, which is very expensive when dealing with PCH files. (You an find
comments about it if you search for "PCH".)
================
Comment at: lib/StaticAnalyzer/Frontend/ModelConsumer.cpp:37
@@ +36,3 @@
+ // We are interested in definitions and declarations.
+ FunctionDecl *func = llvm::dyn_cast<FunctionDecl>(*I);
+ if (func) {
----------------
Why func lost constness here?
================
Comment at: lib/StaticAnalyzer/Frontend/ModelInjector.cpp:49
@@ -43,3 +48,3 @@
// about file name index? Mangled names may not be suitable for that either.
- if (Bodies.count(D->getName()) != 0)
+ if (Bodies.count(D->getName()) != 0 || Decls.count(D->getName()) != 0)
return;
----------------
Does ModelInjector::onBodySynthesis return immediately if the model has been
parsed but the Decl is not in the map?
If that is not the case, wouldn't it be very expensive to call onBodySynthesis
on every decl, most of which are not modeled? If I understand correctly, we
would try to parse the model file for every such decl.
================
Comment at: test/Analysis/model-attributes.cpp:2
@@ +1,3 @@
+// This is testing the 'faux-attributes' analyzer option.
+// The declaration of 'modelFileHasAttributes' found in
modelFileHasAttributes.model has 'nonnull' attributes on the 2nd and 3rd
parameter.
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -analyze
-analyzer-checker=core -analyzer-config
faux-attributes=true,model-path=%S/Inputs/Models %s -verify
----------------
80 col?
http://reviews.llvm.org/D13731
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
