[cfe-commits] Unique'ing VariableArrayType
Ted Kremenek
kremenek at apple.com
Wed Oct 8 17:21:50 PDT 2008
On Oct 8, 2008, at 4:20 PM, Argiris Kirtzidis wrote:
> After the subtle bug that was uncovered here:
> http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20081006/008095.html
> I think that VariableArrayTypes should be unique'd based on the
> expression that represents the size.
>
> The comments of over VariableArrayType class mention:
>
>> /// Note: VariableArrayType's aren't uniqued (since the expressions
>> aren't) and
>> /// should not be: two lexically equivalent variable array types
>> could
>> mean
>> /// different things, for example, these variables do not have the
>> same type
>> /// dynamically:
>> ///
>> /// void foo(int x) {
>> /// int Y[x];
>> /// ++x;
>> /// int Z[x];
>> /// }
>
> But those two '[x]' expressions are different DeclRefExpr objects.
> Not unique'ing the VariableArrayTypes doesn't seem to have any
> benefit,
> since it's a bug to create two VariableArrayTypes with the same Expr*
> (they will both try to destroy it).
> And in general it's unintuitive to have Sema::GetTypeForDeclarator
> create different Type* objects each time it gets called with the exact
> same declarator.
Hi Argiris,
Your point makes a lot of sense. By definition the types will be
unique since they refer to distinct expressions.
I think there are a few issues worth thinking about. The first one
has to do with tying the functionality of the VariableArrayType object
to the lifetime of its referenced Expr. If the rewriter (or some
other refactoring) decides to change the Expr referenced by the
VariableArrayType, what happens to the VariableArrayType? If the
VariableArrayType is uniqued by using the Expr* value, then if that
Expr is replaced then all of a sudden the VariableArrayType might be
incorrectly placed in the FoldingSet that we use to unique types
(within ASTContext). A refactoring client would have to get this
corner case right when making changes to an AST, potentially by
removing the VariableArrayType from the foldingset before changing
it's underlying Expr and then reinserting it into the FoldingSet after
changing the underlying Expr. Requiring this would be strangely
burdensome on clients changing the AST, nor do we currently expose the
FoldingSet implementation for uniquing types in the public interface
of ASTContext (and my belief is that we shouldn't expose this
implementation detail).
Thoughts?
Ted
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20081008/a077ab34/attachment.html>
More information about the cfe-commits
mailing list