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

Zhongxing Xu xuzhongxing at gmail.com
Wed Aug 11 18:04:02 PDT 2010


On Thu, Aug 12, 2010 at 8:11 AM, Ted Kremenek <kremenek at apple.com> wrote:
>
> 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.

Would this over generalize this API or generalize this API too early?

>
> 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.




More information about the cfe-dev mailing list