[cfe-commits] [Patch] -Wduplicate-enum which fixes PR6343

[Ted Kremenek]

Right, but I think the amount of memory we are talking about is tiny. 
Even if an enum has 10K values, we are talking about the difference 
between 80K and (say) 160K (as a temporary increase in memory usage 
while we do the work for this warning).  I don't think that's even worth 
blinking any eye about.

David Blaikie wrote:
> Alternatively, or in addition to (so you can use the other 62 bits
> you'll get in addition to the 2 bits you'd end up using for the
> tombstone&  sentinel flags) you could just throw out any enum that
> eventually uses one of the reserved values (just like you currently
> throw out any enum with a>  64 bit representation) - though there's a
> time/space/value tradeoff (time to bit twiddle, space to store the
> extra flags/bits, value in terms of which enum types you can diagnose)
>
> On Tue, Aug 14, 2012 at 10:33 AM, Ted Kremenek<kremenek at apple.com>  wrote:
>> I can see a trivial workaround by sacrificing a bit of storage.  Wrap
>> the int64_t in a struct, with extra bits to indicate whether it is a
>> sentinel or a tombstone.  The storage requirements will likely be
>> completely unnoticeable, especially since the map is temporary.
>>
>> Jordan Rose wrote:
>>> On Aug 13, 2012, at 10:22 PM, Ted Kremenek<kremenek at apple.com>   wrote:
>>>
>>>> Hi Richard,
>>>>
>>>> Thanks for making great progress on this!  I agree with Richard Smith's suggestion about not using an std::map, but is there a reason to not just use a DenseMap to see if there are any enum collisions?  Fundamentally your algorithm is O(n*log n), not O(n).  Since you are converted everything to an int64_t, could you not just use a DenseMap<int64_t, EnumConstantDecl*>?  You could then iterate over the EnumConstantDecls in one linear pass, converting them to int64_t's on the fly for comparison purposes.  A few advantages I see are:
>>> The reason Dmitri didn't use a DenseMap for his type-tag work is because DenseMap needs to reserve an "empty" value and a "tombstone" value. I assume the same problem applies here: all possible int64_t values are possible enum constants, and we don't know until we've seen all the enums which ones are available as empty or tombstone values. It's possible there's a way around this, though.
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits