[PATCH] D19843: Use the name of the file on disk to issue a new diagnostic about non-portable #include and #import paths.

Eric Niebler via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 9 16:20:42 PDT 2016


eric_niebler added inline comments.

================
Comment at: lib/Lex/PPDirectives.cpp:33
@@ -28,2 +32,2 @@
 #include "llvm/Support/Path.h"
 #include "llvm/Support/SaveAndRestore.h"
----------------
rsmith wrote:
> eric_niebler wrote:
> > You mean, instead of the `StringSet` below? Looks like `StringSwitch` just turns into cascading `memcmp`'s. Bet I can tell you how that performs versus a hash set. :-) Also, with the `StringSet`, I get to initialize it once and reuse it many times. I expect that will be pretty darn quick at runtime, but I'm looking forward to @bruno's results.
> Right, I'm not suggesting `StringSwitch` will be faster; it's preferable for other reasons (it avoids the memory and shutdown costs of the static local set). We should stick with what you have if the performance advantage is measurable; otherwise my preference would be to use `StringSwitch`. But it's only a slight preference -- if you'd rather not, I won't complain.
> 
> [`StringSwitch` isn't /quite/ as bad as you're suggesting: it always first compares on length, and it typically compiles into a switch on string length followed by memcmps. Moreover, the code should be "obvious" enough that compilers can (at least in principle) optimize those memcmps very aggressively, right down into the equivalent of an unrolled DFA or a perfect hash function, but I'm not at all confident that LLVM will actually do that =)]
@rsmith wrote:
> We should stick with what you have if the performance advantage is measurable; otherwise my preference would be to use `StringSwitch`.

I tried `StringSwitch` on Windows.h where there are lots of wrongly-cased #includes. I couldn't measure the difference. I'll update the diff. 


http://reviews.llvm.org/D19843





More information about the cfe-commits mailing list