[llvm] r185852 - StringRef: add DenseMapInfo for StringRef.

Chandler Carruth chandlerc at google.com
Tue Jul 9 14:32:10 PDT 2013


The fact that the code already existed doesn't mean you should move it into
a *much* more widely used location in the codebase without thoroughly
understanding it and making sure it works correctly in the generic context.
It might have worked fine for a narrow use case but been insufficient for
broad use. This seems like a possibility in this case.

The fundamental point remains -- we need to have unit tests and correct
generic abstractions in StringRef. =]

On Mon, Jul 8, 2013 at 1:27 PM, Manman Ren <mren at apple.com> wrote:

> The key here is a pair <StringRef and unsigned>. If having a generic
> DenseMapInfo for StringRef is not a good idea, I will introduce a
> DenseMapInfo for the pair and keep things where they were by reverting this
> patch.


I think you should do that unless you have solutions to the problems I've
described with a generic DenseMapInfo for StringRef (I'm not aware of any,
and I think any would require a change to the internals of StringRef which
probably isn't desirable).
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130709/10bf04bf/attachment.html>


More information about the llvm-commits mailing list