Page MenuHomePhabricator

[Analyzer] As suggested, use value storage for BodyFarm
ClosedPublic

Authored by george.karpenkov on Oct 30 2017, 11:53 AM.

Details

Summary

I've also removed copy constructor to help clients not to shoot themselves in the foot with BodyFarm stored = Manager.getBodyFarm().

Diff Detail

Repository
rL LLVM

Event Timeline

dblaikie accepted this revision.Oct 30 2017, 11:57 AM

Looks good - thanks!

This revision is now accepted and ready to land.Oct 30 2017, 11:57 AM
xazax.hun accepted this revision.Oct 31 2017, 3:20 AM

Some minor nits otherwise LGTM!

include/clang/Analysis/AnalysisDeclContext.h
479 ↗(On Diff #120861)

The reference is on the wrong side.

include/clang/Analysis/BodyFarm.h
43 ↗(On Diff #120861)

Reference is on the wrong side.

lib/Analysis/AnalysisDeclContext.cpp
308 ↗(On Diff #120861)

Same as above.

@xazax.hun I'm really not convinced:

george@/Volumes/Transcend/code/llvm (master)≻ rg "\w+\&" tools/clang/include/clang/StaticAnalyzer
tools/clang/include/clang/StaticAnalyzer/Core/Checker.h
31:  static void _checkDecl(void *checker, const Decl *D, AnalysisManager& mgr,
50:  static void _checkBody(void *checker, const Decl *D, AnalysisManager& mgr,
67:                                         AnalysisManager& mgr,
489:raw_ostream& operator<<(raw_ostream &Out, const CheckerBase &Checker);
554:  /*implicit*/ operator bool&() { return val; }
555:  /*implicit*/ operator const bool&() const { return val; }

tools/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
170:  void runCheckersOnASTDecl(const Decl *D, AnalysisManager& mgr,
174:  void runCheckersOnASTBody(const Decl *D, AnalysisManager& mgr,
400:  typedef CheckerFn<void (const Decl *, AnalysisManager&, BugReporter &)>
469:                          AnalysisManager&, BugReporter &)>
...

On top of that, reference is part of the type, not part of the variable name,

@xazax.hun I'm really not convinced:

Unfortunately, the LLVM codebase right now does not strictly follow the style guide. But clang-format puts the references next to the variable (or function in case of return type) names. Also if you check the code snippets in the coding standard: https://llvm.org/docs/CodingStandards.html#beware-unnecessary-copies-with-auto you can see that where we officially put the references.

zaks.anna edited edge metadata.Oct 31 2017, 1:41 PM

Also if you check the code snippets in the coding standard: https://llvm.org/docs/CodingStandards.html#beware-unnecessary-copies-with-auto you can see that where we officially put the references.

Correct! The reference should not go with the type name.
George, please, address the comments before committing.

dcoughlin accepted this revision.Oct 31 2017, 5:26 PM

Looks good to me with the comments by Gabor addressed.

On top of that, reference is part of the type, not part of the variable name,

The reference is parsed as part of the declarator (associated with the variable name) rather than as part of the type.

Where this rears its ugly head is when declaring multiple variables in the same statement:

int g = 7;
void foo() {
  int& p = g, q = 7;
}

Here p has type 'int &' but q has type 'int'. The same holds for '*' for pointer types in C.

This revision was automatically updated to reflect the committed changes.