[PATCH] D39220: [Analyzer] Store BodyFarm in std::unique_ptr

George Karpenkov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Oct 25 11:46:12 PDT 2017


george.karpenkov added inline comments.


================
Comment at: include/clang/Analysis/AnalysisDeclContext.h:424
   /// methods during the analysis.
-  BodyFarm *BdyFrm = nullptr;
+  std::unique_ptr<BodyFarm> BdyFrm;
 
----------------
alexfh wrote:
> george.karpenkov wrote:
> > alexfh wrote:
> > > BdyFrm is somewhat cryptic. Maybe Farm, Bodies or something else that  is not so weirdly abbreviated?
> > But that's just forced on us by the naming convention, isn't it? I think other names are worse though: they don't tell us which class we can look at to figure out what is it doing.
> LLVM Coding Standards doesn't endorse this kind of an abbreviation, on the opposite:
> "We cannot stress enough how important it is to use descriptive names. ... Avoid abbreviations unless they are well known." (http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly). Neither "Bdy" nor "Frm" are well known or otherwise clear to the reader. "Frm" could also stand for "Form" or "Frame", but in these cases it would also be confusing.
> 
> If you consider shorter names ("Bodies" or "Farm") non-informative, we can go the other direction, e.g. "FunctionBodyFarm".
What I've meant to say is that I think a good way to write it would be to use `unique_ptr<BodyFarm> bodyFarm`, or even better `unique_ptr<BodyFarm> m_bodyFarm`, which is sadly not allowed. We already have a good informative name: the name of the class, and due to the convention we have to modify it :(


Repository:
  rL LLVM

https://reviews.llvm.org/D39220





More information about the cfe-commits mailing list