-
-
Notifications
You must be signed in to change notification settings - Fork 217
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
Calling a function with a literal and a variable requires manually writing .borrow() #492
Comments
(I was already writing a comment in #490 before you posted this one, so I'll just post my final comment on this issue instead) This was an intentional change, and it was done in PR #423. It changed so literals (and anything ending in The issue was that it was impossible to call functions that didn't expect a reference, e.g. you wouldn't be able to call The compromise was to change so literals were no longer implicitly borrowed for e.g. function calls. Because (as you already figured out) it's possible to call In short, if you really want a referenced literal, then Yes it's annoying having to do |
I know, that function always took a Another problem is that I usually only find the exact problem and the workaround by looking at the generated code and possibly compiling that to get the exact error location. Error messages like this one, for longer templates, don't really tell you much. Unfortunately, I have no idea how this could be improved.
|
Just to be clear, the "you" you're referring to in this case is @djc. I don't think I've voiced my opinion on it. But I can definitely see the pros and cons of both view points :) One thing though, if borrowing was added, then technically your example in the issue would still fail, as you'd still need to do Personally, I'm open to and probably fine with any solution, as long as it doesn't e.g. minimize the amount of function calls it would be possible to make :)
This is something we're well aware about. It personally bugs me a lot as well, as it sometimes confuses me a lot too. I've been looking into feasible ways to improve this, especially in relation to how all of it plays together with Related issue: #310 |
Sorry, my bad.
I don't think the error message I posted above can be improved by nom at all. Neither nom nor askama's derive macro have enough information to know the parameters of the function I'm calling. Unless I understood something completely incorrectly, this error message has to always come from the rust compiler, and the most askama can do is hint the compiler to the "real" source of the error. That being said, improving the error messages for invalid template syntax is of course also important :) |
Ahh, yeah, you're right. For some reason I was confusing the short error with parser errors :) |
I'm okay with the way passing a literal here doesn't work any more. It's definitely not enough to make me reconsider my stance on allowing |
I'm not sure, how does #500 improve the situation here? |
It doesn't, we referenced the wrong issue there (since corrected). |
Using the latest release, we could write code like this (untested, from memory):
Using the code currently on the main branch,
foo.bar(1)
gets translated toself.foo.bar(1)
, whereasfoo.bar(value)
gets translated toself.foo.bar(&self.value)
. This makes it impossible to write such a function without introducing a trait that is implemented for bothu32
and&u32
, as the function now needs to accept both values and references.The only other workaround I could find was to use
{{ foo.bar(1.borrow()) }}
in the template code, which requires an explicituse std::borrow::Borrow
in the file that invokes the derive macro. Still less code than defining my own trait.The text was updated successfully, but these errors were encountered: