pgousseau 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.
----------------
zaks.anna wrote:
> Why do we not have sanity checks? What happens when there is a conflict?
I am worried that any custom sanity check I put would end up confusing the
"faux-attributes" user ... I have just noticed that Sema has some merging
capabilities, I will have a look, see if it can be reused. Thanks!
================
Comment at: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:505
@@ +504,3 @@
+// AST visitor used to merge model attributes.
+class WalkAST : public DataRecursiveASTVisitor<WalkAST> {
+ AnalysisDeclContextManager &Mgr;
----------------
zaks.anna wrote:
> This name is too generic.
Will rename it thanks!
================
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()) {
----------------
zaks.anna wrote:
> 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".)
I see, yes this can be optimized thanks!
================
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) {
----------------
zaks.anna wrote:
> Why func lost constness here?
No reason, will fix thanks!
================
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;
----------------
zaks.anna wrote:
> 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.
>
Yes, there might be cases where the model file's Decl might not match the model
file filename, causing re-parsing. Will fix thanks!
================
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
----------------
zaks.anna wrote:
> 80 col?
Will fix!
http://reviews.llvm.org/D13731
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
