[PATCH] D11792: MIR Parser: Simplify the token's string value handling.

Sean Silva chisophugis at gmail.com
Wed Aug 5 19:17:16 PDT 2015


silvas added a comment.

Except for removing rawStringValue this entire suggestion was pretty nitpicky anyway, so feel free to ignore if any of this doesn't seem worth it.


================
Comment at: lib/CodeGen/MIRParser/MILexer.h:113
@@ +112,3 @@
+
+  MIToken &operator=(MIToken &&Other) {
+    Kind = Other.Kind;
----------------
It seems weird to me that we are adding an operator= (and a non-obvious one at that) for a class that is never really being assigned. It seems like the complexity is coming from the use of MIToken constructor to reinitialize an MIToken. I think it would be simpler to use two explicit calls:

- resetToken(Kind, Range)
then one of
- setIntVal(APSInt &)
- setStrValNonOwned(StringRef) (or use an offset; but I like the symmetry with Owned)
- setStrValOwned(StringRef/std::string) (copies into internal buffer and sets StrVal)

Conveniently all the existing uses of the MIToken constructors like this are already broken across two lines, so this should not increase the size of the reinitialization source code (but allows deleting code duplicated across these 3 constructors).

An even simpler solution is to just use std::string/SmallString and have it always own (not sure how performance-critical this is). Once it grows large enough there should be no further need to reallocate, so it is just a small memcpy cost.


Repository:
  rL LLVM

http://reviews.llvm.org/D11792





More information about the llvm-commits mailing list