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

structure binding transformation is not accurate #226

Closed
viboes opened this issue Sep 27, 2019 · 4 comments
Closed

structure binding transformation is not accurate #226

viboes opened this issue Sep 27, 2019 · 4 comments

Comments

@viboes
Copy link
Author

viboes commented Sep 27, 2019

The result of

https://cppinsights.io/s/d9b470ff

could be

struct Point
{
  int x;
  int y;
  // inline Point() = default;
  // inline constexpr Point(const Point &) noexcept = default;
  // inline constexpr Point(Point &&) = default;
};

void f(int, int);

Point pt = {1, 2};

void f()
{
  Point __pt12 = Point(pt);
  #define ax = __pt12.x;
  #define ay = __pt12.y;
  f(ax, ay);
  #undef ax 
  #undef ay 
}

The use of define is stillnot accurate but IMO it conveys better the intent.
Alternatively the transformation could do the name replacement :(

//...
void f()
{
  Point __pt12 = Point(pt);
  f(__pt12.x, __pt12.y);
}

Alternatively the transformation could add a comment on the variable declaration.

  // This is a workaround to have new names. You cannot take the address of these variables
  int& ax = __pt12.x; 
  int& ay = __pt12.y;

@andreasfertig
Copy link
Owner

Hello @viboes,

thank you for bringing up that issue. I agree that it is not 100% correct due to introducing new names.

While I agree, that the #define solution would be probably the most correct one, I really really hesitate implementing it. It looks not ok. Plus introducing the #undef is not trivial.

I can look into the name replacement. I sadly had to do similar things for coroutines I'm working on, but it is not trivial and usually I like to leave as much AST parts untouched as I can.

The comment is currently the easiest way to go I think. I was thinking about a annotation mode a user can turn on or off. C++ Insights for example already adds annotations for NRVO variables and other things (implicit special member functions you also have seen).

Andreas

@viboes
Copy link
Author

viboes commented Sep 28, 2019

I agree that the macro is not welcome.

I understand that it is not easy to do the name replacement.

If the comment is possible and easy, I believe all of us could live with.

@andreasfertig
Copy link
Owner

Hello @viboes,

I just fixed another issue related to structured binding (#425). I read the wording a couple of times without seeing wording that says that the bindings are something special. I also looked at the code in Clang and couldn't find any special treatment.

Unless somebody shows me a different interpretation of the standard or code in Clang that treats BindingDecls special, I assume that the code shown is correct and close this issue.

Andreas

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

2 participants