search

boostorg / spirit

Boost.org spirit module
http://boost.org/libs/spirit
393 stars 161 forks source link

X3: attr parser should not move elements from itself #473

Closed Kojoley closed 5 years ago

Kojoley commented 5 years ago

The second and subsequent calls to it will produce garbage on complex types https://github.com/boostorg/spirit/blob/32537028f5aad270db4df86e4e82b5ffa7d42027/include/boost/spirit/home/x3/auxiliary/attr.hpp#L47 https://github.com/boostorg/spirit/blob/32537028f5aad270db4df86e4e82b5ffa7d42027/include/boost/spirit/home/x3/auxiliary/attr.hpp#L83

djowel commented 5 years ago

I'm not sure if I am following here. Do you have some context and maybe an example?

Xeverous commented 5 years ago 
    auto const rule_def = (string | '_' >> x3::attr(std::string("default"))) % ','

text:

"abc", _, "def", _, "ghi"

First occurence of _: places "default" into the attribute

Second occurence of _: places ??? becase string object supplied to attr was moved-from

Kojoley commented 5 years ago

I overlooked that std::move will return const&&, so copying will be performed instead.

@Xeverous do you really observe this? This should not happen at least because the string is short for small string optimization to be used and as a result there is nothing to move out from such string.

djowel commented 5 years ago

Perhaps the semantics of 'move_to' has changed since then, and the comment: // $$$ Change to copy_to once we have it $$$ is telling. So yes, this should be copy, not move, technically.

Xeverous commented 5 years ago

@Kojoley No, I just gaved this as an example.