[PATCH] MC: Use StringRef to avoid strcmp/strlen on non-null-term'd strings.

[Benjamin Kramer]

On 13.10.2013, at 05:52, Will Dietz <w at wdtz.org> wrote:

> See attached, explanation from commit message follows.
> 
> ----------------------------------
> 
> This can happen when processing command line arguments, which
> are often stored as std::string's and later turned into
> StringRef's via std::string::data().  Unfortunately this
> is not guaranteed to return a null-terminated string
> until C++11, causing breakage on platforms that don't do this.
> 
> Use of StringRef also simplifies the code a bit, as a bonus.
> 
> Note that the use of ".data()" is okay when printing the
> lists of recognized -mcpu options and similar code, since
> the tables themselves are statically populated with C-strings
> that are guaranteed to be null-terminated.
> 
> ----------------------------------
> 
> Two concerns I have with this change:
> 
> * Resulting increase in size/memory use of the tables due to storing
> size explicitly
> * Possible initialization time increases.
> 
> Are either concerns something to be sufficiently concerned about that
> they should be measured, and if so any recommenedations on how I might
> do so?

I would prefer not doing this.

* StringRef in a compile-time initialized array adds static initializers because the struct is no longer POD.
* It adds data to the table that's not really needed. The code using the tables is cold.
* StringRef with format strings is ugly.

Can't we just adapt the binary search with a custom comparator that constructs the StringRefs on the fly?

- Ben

> 
> Regarding initialization time, I tried running llvm-mc before and
> after assembling a small file, which showed no ignificant difference
> across many runs.  I'm not confident in the validity of this test,
> however.
> 
> If these are valid concerns, an alternative is to just construct a
> null-terminated
> string on-the-fly when querying the tables (since these are the strings that are
> potentially lacking the terminators).  Let me know if this sounds preferrable.
> 
> ~Will
> <0001-MC-Use-StringRef-to-avoid-strcmp-strlen-on-non-null-.patch>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits