[cfe-dev] Allowing checkers to mark symbols as live

[Ted Kremenek]

On Aug 10, 2010, at 9:51 PM, Jordy Rose wrote:

> What I'm not so happy about here is that the checker-writer could still
> arbitrarily markLive() random symbols, instead of being restricted to
> markInUse() (which is deliberately limited to metadata right now).
> Admittedly, that was the /only/ choice before, so this is probably still an

Hi Jordy,

I think this can be simplified a bit.  How about changing:

  for (CStringLength::EntryMap::iterator I = Entries.begin(), E = Entries.end();
       I != E; ++I) {
    SVal Len = I.getData();
    if (const SymbolMetadata *Sym = dyn_cast<SymbolMetadata>(Len.getAsSymbol()))
      SR.markInUse(Sym);
  }

to:

  for (CStringLength::EntryMap::iterator I = Entries.begin(), E = Entries.end();
       I != E; ++I) {
    if (const SymbolRef *Sym = Len.getAsSymbol())
      SR.markInUse(Sym);

and then let SymbolManager decide if it needs to do anything special for the SymbolRef passed to 'markInUse'.  That seems cleaner from an API perspective, as it leaves SymbolReaper in the business of deciding when a symbol is live (and how it decides when it is live).  In other words, just move the dyn_cast<SymbolMetadata> to markInUse.

Minor nit:

+  static void Profile(llvm::FoldingSetNodeID& profile, const MemRegion *R,
+                      const Stmt *S, QualType T, unsigned Count,
+                      const void *Tag) {
+    profile.AddInteger((unsigned) ExtentKind);

The kind is not 'ExtentKind'.

For:

 
+class SymbolMetadata : public SymbolData {
+  const MemRegion* R;
+  const Stmt* S;
+  QualType T;
+  unsigned Count;
+  const void* Tag;
+public:

Please add a doxygen comment above SymbolMetadata that describes its intended purpose.  I know the other classes aren't well documented; but we need to start somewhere.