[cfe-commits] [PATCH] Simplify StringSwitch

Douglas Gregor dgregor at apple.com
Thu Dec 3 07:31:35 PST 2009


On Dec 3, 2009, at 4:15 AM, Sebastian Redl wrote:

> Gabor Greif wrote:
>> Hi all,
>> 
>> attached patch simplifies StringSwitch and gets rid of the
>> Copy{Constructible|Assignable} constraint on the type parameter.
>> 
> The final conversion still imposes that requirement. Perhaps you should 
> change operator T to operator const T&.


FWIW, that will break uses of StringSwitch that try to bind the result to a temporary, e.g.,

  const std::string &R = StringSwitch<std::string>("blarg").Case("blarg", "temporary");

The temporary std::string built for "temporary" only lives until the end of the initializer, so R ends up binding to it. I don't know if anyone actually uses StringSwitch that way, but they're going to get a surprise if they do.

I don't know whether this change actually makes StringSwitch any more efficient. Did anyone look at the code? We don't care about space efficiency here, because a single bool on the stack is irrelevant. As for performance, most string-switches just return an integer or an enum: taking the address of such things the way stringswitch does is likely to pessimize code. This would certainly be a win if we did string-switches with heavyweight return types (like the std::string example above), but I don't know how common that will be.

	- Doug



More information about the cfe-commits mailing list