[llvm-commits] [llvm-gcc-4.2] r81586 - in /llvm-gcc-4.2/trunk/gcc: llvm-debug.cpp llvm-debug.h

[Daniel Dunbar]

Hi Devang,

On Fri, Sep 11, 2009 at 4:57 PM, Devang Patel <dpatel at apple.com> wrote:
> Author: dpatel
> Date: Fri Sep 11 18:57:27 2009
> New Revision: 81586
>
> URL: http://llvm.org/viewvc/llvm-project?rev=81586&view=rev
> Log:
> Use weak handle to keep track of MDNodes.

Does clang need the same change? It's not clear to me yet that we
should actually need a WeakVH... see below.

> ==============================================================================
> --- llvm-gcc-4.2/trunk/gcc/llvm-debug.cpp (original)
> +++ llvm-gcc-4.2/trunk/gcc/llvm-debug.cpp Fri Sep 11 18:57:27 2009
> @@ -661,7 +661,7 @@
>     return FwdDecl;
>
>   // Insert into the TypeCache so that recursive uses will find it.
> -  TypeCache[type] =  FwdDecl.getNode();
> +  TypeCache[type] =  WeakVH(FwdDecl.getNode());
>
>   // Convert all the elements.
>   llvm::SmallVector<llvm::DIDescriptor, 16> EltTys;
> @@ -772,9 +772,10 @@
>
>   DIType Ty;
>   if (tree TyDef = TYPE_NAME(type)) {
> -      std::map<tree_node *, MDNode *>::iterator I = TypeCache.find(TyDef);
> +      std::map<tree_node *, WeakVH >::iterator I = TypeCache.find(TyDef);
>       if (I != TypeCache.end())
> -        return DIType(I->second);

Can you explain this line below? Its reconstructing a type in the case
when something deleted the metadata which was in the type cache.
Should that ever happen? Can we not maintain enough invariants to
allow us to use an AssertingVH instead?

I went ahead and moved clang over to an AssertingVH in r8232{0,1}, so
that we will at least get asserts if we try something funny (which
does, indeed, assert for PR4894).

> +       if (Value *M = I->second)
> +         return DIType(cast<MDNode>(M));

 - Daniel