Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Range initialization expression evaluated twice after range-based for transformation #3

Open
muggenhor opened this issue Mar 8, 2018 · 0 comments

Comments

@muggenhor
Copy link

muggenhor commented Mar 8, 2018

Looking at the code at ReplaceForRange.inc:69 you seem to be jumping through extra hoops to eliminate the intermediate __range variable from being emitted.

This is non-conformant for two reasons:

  1. You're no longer applying lifetime extension to the object returned by the range-init expression (section 12.2 [class.temporary], paragraph 5) and thus not ensuring that said object is still alive to iterate over after having extracted its iterators
  2. You're now evaluating the range-init expression twice, which is a problem if that expression is not idempotent. I.e. it might be possible for the second evaluation of the range-init expression to return a different object, and thus the two iterators would be part of different ranges and comparing them be meaningless. This is true for mostly every range-init expression that's an prvalue (e.g. any function returning a newly constructed range object).

This could be solved by emitting the equivalent of the following instead (C++17 spec, 6.5.4 [stmt.ranged]):

{
  auto&& __range = <for-range-initializer>;
  auto __begin = &__range[0] <or> __range.begin() <or> begin(__range);
  auto __end  = &__range[sizeof(__range)/sizeof(__range[0])] <or> __range.end() <or> end(__range);
  for (; __begin != __end; ++__begin)
  {
    <for-range-declaration> = *__begin;
    <for-statement>
  }
}

Note that you're almost there already. I believe, from code inspection (haven't yet had the time to try), that all you need to do is to emit the C++03 equivalent of auto&& __range = <for-range-initializer> preceding its use instead of replacing the uses of __range with <for-range-initializer>.

Edit: if a universal reference (auto&&) is a problem: due to the above description of the usage of __range I believe an l-value reference (auto& __range = ...) would suffice as well, because no std::forward<decltype(__range)>-like constructs are used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant