QuickTypes.jl
QuickTypes.jl copied to clipboard
Add Revise tests
Start with some basic Revise tests, that currently pass on v1.9.1.
then https://github.com/cstjean/QuickTypes.jl/pull/27/commits/b459c6a0279b2d66b02f0fba3e32b4e960c20e92 leads to
The culprit seems to be in https://github.com/cstjean/QuickTypes.jl/commit/2ed4967e966e7249043795423286c0ddc48730c4. Reverting it makes the test/revise.jl
suite pass.
Thank you for the bug report! 121bab4 makes the revise tests pass, at the cost of breaking some other tests. Revise doesn't seem to like __source__
interpolation.
Is Revise really broken on your side? Maybe the Revise test failures are just an artifact of the way the revise tests are performed, and it's the tests themselves that need updating.
I appreciate the test suite, and it was quite useful to me. That said, it would be a bit sad to add this kind of maintenance burden on all packages that use macros, for what is arguably a Revise failure (heroic as Revise is).
I think once this is fixed, I'd merge the test suite, but wouldn't hesitate to drop it if it becomes a burden.
Maybe the Revise test failures are just an artifact of the way the revise tests are performed, and it's the tests themselves that need updating.
No, the tests are working as expected. We can replicate the
MethodError: Cannot `convert` an object of type Nothing to an object of type Symbol
errors in real life scenarios.
MacroTools.@q
seems to be part of the problem.
With
diff --git a/test/assets/within-macro1.jl b/test/assets/within-macro1.jl
index 36e468b..f639abd 100644
--- a/test/assets/within-macro1.jl
+++ b/test/assets/within-macro1.jl
@@ -3,7 +3,7 @@ using MacroTools: @q, @capture
macro mymodel(def)
@capture(def, model_(; params__))
- esc(@q begin
+ esc(quote
$QuickTypes.@qstruct_fp $model(; $(params...))
end)
end
diff --git a/test/assets/within-macro2.jl b/test/assets/within-macro2.jl
index 76da376..d27b4a6 100644
--- a/test/assets/within-macro2.jl
+++ b/test/assets/within-macro2.jl
@@ -3,7 +3,7 @@ using MacroTools: @q, @capture
macro mymodel(def)
@capture(def, model_(; params__))
- esc(@q begin
+ esc(quote
$QuickTypes.@qstruct_fp $model(; $(params...))
end)
end
the Revise error goes away.
Yes, @q
strips away all line numbers, which Revise doesn't seem to like. I would try:
res = esc(@q begin
$QuickTypes.@qstruct_fp $model(; $(params...))
end)
res.args[1].args[2] = __source__
return res
The problem with normal quote
is that the line numbers point to the macro definition code, which isn't great. Eg. before my latest PR, @qstruct Foo(); @which Foo()
would point to the QuickTypes code, whereas we'd like it to point to the module where @qstruct Foo()
is written. That's what the @q
+ res.args[1].args[2] = __source__
combo achieves.
Come to think of it, maybe the @q
isn't necessary... But still, I don't think it hurts. quote
puts a lot of LineNumberNode everywhere, and almost none of them are desirable from my POV.