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

wire: do not use global variables for all wire.Value expressions #185

Open
rogpeppe opened this issue May 25, 2019 · 5 comments · May be fixed by #397
Open

wire: do not use global variables for all wire.Value expressions #185

rogpeppe opened this issue May 25, 2019 · 5 comments · May be fixed by #397
Labels
enhancement New feature or request good first issue Good for newcomers
Milestone

Comments

@rogpeppe
Copy link

One of the specific advantages of the wire approach is that it generates code that's relatively readable compared to reflect-based equivalents. When wire.Value is used on a small by-value type, there's no need for the value to live in a global variable - the expression could instead be used literally inside the generated code, which would make the code easier to follow, and more similar to the code that one might write by hand.

@zombiezen zombiezen added this to the Unplanned milestone Jun 7, 2019
@zombiezen zombiezen added the enhancement New feature or request label Jun 7, 2019
@zombiezen
Copy link
Collaborator

The reason we have the semantics we have now for wire.Value is that we want to be confident that the expression will evaluate to the same value that was expressed at the wire.NewSet's evaluation. This keeps the behavior of wire.Value largely unsurprising, and amenable to usage in a runtime DI version if needed at some point.

It sounds like the request here is that the generated code in some circumstances (like when using a constant expression, for example) could be more efficient, since the time that the expression is evaluated wouldn't matter. That seems reasonable to me, but I think we can add this feature in a backward-compatible way later.

@rogpeppe
Copy link
Author

rogpeppe commented Jun 8, 2019

Yes, but it's not just about efficiency. One of the selling points of wire is that the generated is a readable representation of the dependency setup. This is one thing that makes the code less idiomatic/readable

@zombiezen
Copy link
Collaborator

Agreed. To be clear: I think this is a good idea, but we don't have bandwidth to fix it right now. Happy to review PRs to fix.

@zombiezen zombiezen added the good first issue Good for newcomers label Jun 11, 2019
@j4ng5y
Copy link

j4ng5y commented Jun 25, 2019

I'll take a stab at this.

@zombiezen zombiezen removed their assignment Jul 9, 2019
@ProgrammingMuffin
Copy link

ProgrammingMuffin commented Dec 15, 2023

@zombiezen A few observations here.

  1. Do we need global variables at all, if we provide that value exactly when and where the programmer wanted to provide it.
  2. Do we need backward compatibility, since the generated code is going to overwrite it anyway and wouldn't affect the code unless the programmer is using those generated global values somewhere else. If yes, do we have any ideas on how to make the backward compatibility work?

Since this issue is still open, I decided to try it out and it works but it's only a local test change (not confident if that's the solution we are looking for),

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants