revert broken commit - add test

[t-bltg]

@timholy, the added test breaks on 1.0.2 and not on 1.0.1.

So I've reverted the changes of 1.0.2 and we can start working on a fix.

I'm not sure if the precompilation scenario is valid code in RecipesBase (this is clearly a niche involving meta-programming), and I'd be happy to change some - considered invalid - but working statements in https://github.com/JuliaPlots/Plots.jl/blob/master/RecipesBase/src/RecipesBase.jl#L600-L619.

[timholy]

Since SnoopPrecompile 1.0 supports putting a method inside @precompile_all_calls, we have to support that at least until SnoopPrecompile 2.0. This test is extremely useful, thanks!

[t-bltg]

Let me know if you need something else, I'm not really into the precompilation internals but if I can help in any way :)

[timholy]

Continuing from #317, the main issue is how to ensure that some of the workload in the @precompile_all_calls doesn't get compiled before inference starts tracking it. xref JuliaLang/julia#47902. That issue details potential solutions. If you're wanting to make sure that the macro-expansion itself gets fully precompiled, my sense is the @eval solution might be the right one. But I think that would require you to use $__RecipesBasePrecompileType anywhere you current use it without the $ currently. That of course is itself a breaking change.

[codecov]

Codecov Report

Base: 84.01% // Head: 84.08% // Increases project coverage by +0.07% 🎉

Coverage data is based on head (0c73c34) compared to base (d11c578).
Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #318      +/-   ##
==========================================
+ Coverage   84.01%   84.08%   +0.07%     
==========================================
  Files          17       17              
  Lines        2165     2162       -3     
==========================================
- Hits         1819     1818       -1     
+ Misses        346      344       -2     

Impacted Files	Coverage Δ
SnoopPrecompile/src/SnoopPrecompile.jl	87.50% <0.00%> (+4.64%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[timholy]

Let me know if you need something else, I'm not really into the precompilation internals but if I can help in any way :)

Basically is your goal to ensure everything needed by @recipe gets precompiled? AFAICT that only helps reduce package precompilation times, it doesn't affect actual usage of the packages that use RecipesBase.

[timholy]

I'm guessing there's another let introduction in 1.0.2 that needs to be reverted...

[t-bltg]

Basically is your goal to ensure everything needed by @recipe gets precompiled? AFAICT that only helps reduce package precompilation times, it doesn't affect actual usage of the packages that use RecipesBase.

Yeah, that's probably an overkill to use SnoopPrecompile there ...

[timholy]

I wouldn't necessarily call it overkill, I totally see why you'd do that. Just trying to come up with a solution that is as transparent to users about what will and what won't work. There's a real expectation that lines will be executed in the order in which they appear, but without special precautions that's not actually true.

[t-bltg]

Hum, the pseudo RecipeBase precompilation code might not even be valid in julia versions <= 1.4 ...
When I added the SnoopPrecompile dep to RecipesBase, we restricted the julia version to >= 1.6.
Interesting ...

[timholy]

I think it would be OK to restrict that test to Julia versions where it works. In my mind the point is to make sure we catch potential regressions; if it never worked in any SnoopPrecompile release, then we don't have to figure out how to make it work.

[t-bltg]

nightly failure seems unrelated (JET failure).

[timholy]

Yep, see #300

[timholy]

Thanks for a very important contribution!