Configurable macros namespace #3944
Open
+341
−217
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Does your PR solve an issue?
Closes #3943 .
Is this a breaking change?
Nope. Nothing should change for SQLx itself.
Approach
The exposed functions from
sqlx-macros-core
were extended to take acrate_name
argument which would refer to the namespace that module resolution is done at. Right nowsqlx
is hardcoded, which is more than understandable, but changing it would be beneficial for 3rd party driver developers.This might not be ideal, so I'm open to suggestions. I'll list a couple of things at the top of my head:
Function pollution
Not sure if there's a better way to deal with this other than the extra function argument. It's verbose, but that could be a good thing since once this is introduced it would ideally be used in all future proc macro development.
Moreover, because the goal is to have external drivers and
sqlx
coexist, this can't just be an environment variable or something like that. By using the function argument external drivers can redefine the macros (which they have to anyway forquery!
and friends so a customQueryDriver
is used).Testing
Testing is still incomplete.
query!
& co. are not used because they would need a live database connection or some cached queries. Also, the location might not be ideal and it might be worth moving this to the maintests
directory. I'd love to hear a second opinion on how to tackle this.Note that moving the tests someplace else might be a little more trouble since the
#[cfg(test)]
directive is only going to apply to the proc-macrosqlx-macros
crate when it gets compiled in test mode. Using it as a dependency will most likely still result in the proc macros getting compiled in the non-test scenario, resulting in the hardcodedsqlx
namespace being used. This could be circumvented with an internal feature or some custom attribute passed as a flag to the compiler.@abonander what's you're take on this? I'd really like to get this in before the
sqlx 0.9.0
release.Extra
This PR also makes the
MatchBorrow
invocation in the query proc macrosmutable
. The reason for that is because this can allow another level of autoref specialization, since resolution will first attempt the owned type trait impl, then the referenced type trait impl and finally the mutable type trait impl. While not useful forSQLx
right now, it could be in the future and it would also allow external drivers to make use of it right away.