lgtm, seems like cleaner code this way anyhow.

But that sounds like a compiler bug if it wasn't constructing the static variable properly. Just out of curiosity, which compiler was it with?

I'm not completely sure which one the build system picked up, but it was probably either g++ (Ubuntu 4.8.4-2ubuntu1~14.04.3) 4.8.4 or clang version 4.0.0 (http://llvm.org/git/clang.git 7759c7d28c3c5ad00d2e3310384c5e7073de1f97). If it's using the git version of clang, that seems like a pretty likely culprit for the compiler bug.

It looks like asan is reporting memory leaks with this new change. I'm not sure why this should be worse, because the old version had absolutely no way to ever free the StringSet.

It's possible the change makes ASan capable of finding more leaks, because the leaks seem to be a bunch of strings, and not the StringSet itself.

See the // XXX leaked comment in the middle of your diff. We allocate there and never free it, since it needs to live til the process exits anyhow. Perhaps we should change that, although we'd need to look at perf.

Is this still active, or should we close it?

We might as well close it. I tried a few ways to fix the leaks, but that just exposed more of the problems I was trying to avoid and I got distracted. It'll be easy enough to revisit in the future if needed.