[PATCH] D29610: Fix BucketT Handling in DenseMap.

Daniel Berlin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 6 18:04:04 PST 2017


dberlin requested changes to this revision.
dberlin added a comment.
This revision now requires changes to proceed.

As Dean said,

1. You need a very clear testcase demonstrating the issue you are trying to solve.
2. A clear description how the change solves the problem.

What it looks like now is "a change people are having trouble understanding the reasoning for" and "an argument that world is broken right now".

That is leaving people confused.

Saying "I am just making it less bad. Really we need to initialize the buckets correctly, however this requires more work than I can spare." doesn't help.

It's okay if you don't have time to fix it right.
File a bug, with a reproducible testcase demonstrating what's wrong, and someone who does have time will probably take a stab at it.

In the meantime, we probably don't want to hack up one of the most used datastructures in LLVM with an incompletely solution to a problem people don't understand yet.


https://reviews.llvm.org/D29610





More information about the llvm-commits mailing list