RFC: remove deprecation warning for @_inline_meta

[KristofferC]

This shows up a bit everywhere in PkgEval and makes things kind of noisy. I don't see a strong reason to deprecate it since it is internal so imo it is better to just have it be backwards compatible since it doesn't really cost anything.

[tkf]

This shows up a bit everywhere in PkgEval and makes things kind of noisy.

Why not just turn off deprecation in PkgEval? Was it considered/discussed before? A deprecation warning is a message primarily toward the package maintainers who can take an action based on the warning. But the main purpose of PkgEval is to detect backward-incompatible changes in julia introduced without a conscious decision (IIUC) and not to detect possible future problems in the way that the package itself is written.

But maybe a downside is that the tests are not typically run without a deprecation warning and it could cause some edge cases.

[KristofferC]

A deprecation warning is a message primarily toward the package maintainers who can take an action based on the warning.

This is an internal function (even with an underscore) so having a deprecation is strictly unnecessary in the first case. And in this case, keeping the original symbol as an alias doesn't cost anything (it is arguably cheaper). If you scroll up, you see there are similar things in this file. What is the disadvantage of keeping this name alias around?

[aviatesk]

I'm okay with this, but I think we may end up with many (essentially deprecated) aliases if we keep doing this in the future?

These macros seems to be used so often and so might be the exception though.

[tkf]

This is an internal function (even with an underscore) so having a deprecation is strictly unnecessary in the first case.

This is an argument for a strict application of public vs internal boundary. I find it strange to use it for supporting the opposite; i.e., more flexible application of the boundary.

What is the disadvantage of keeping this name alias around?

We lost an opportunity to remind package authors to acknowledge the difference between public and internal names. Given the popularity, it had a possibility of reaching a large population. Some do know the danger and some may not.

Furthermore, my comment was on fixing the problem systematically and not on this particular problem. So, another disadvantage is that this is postponing a real solution and creates future issues. Of course, it may be a good idea to try to accumulate real world problems before implementing a systematic solution.

If you scroll up, you see there are similar things in this file.

IIUC, we have cases where functions frequently used in performance-critical code. However, warning during macro expansion does not cost anything at run-time.

[KristofferC]

We lost an opportunity to remind package authors to acknowledge the difference between public and internal names.

This is a non-documented, non-exported macro that starts with an underscore. I doubt users of it somehow thought it was public. Also, the current warning message is not good enough:

┌ Warning: `@_inline_meta` is deprecated, use `@inline` instead.
│   caller = eval at boot.jl:368 [inlined]
└ @ Core ./boot.jl:368

Does not tell you where the problem is so no one will fix it. When that is fixed, it could potentially be turned back into a deprecation. Just do give another example, this is typical output from testing packages now:

     Testing Running tests...
WARNING: Wrapping `Vararg` directly in UnionAll is deprecated (wrap the tuple instead).
WARNING: Wrapping `Vararg` directly in UnionAll is deprecated (wrap the tuple instead).
WARNING: Wrapping `Vararg` directly in UnionAll is deprecated (wrap the tuple instead).
WARNING: Wrapping `Vararg` directly in UnionAll is deprecated (wrap the tuple instead).
WARNING: Wrapping `Vararg` directly in UnionAll is deprecated (wrap the tuple instead).
WARNING: Wrapping `Vararg` directly in UnionAll is deprecated (wrap the tuple instead).
WARNING: Wrapping `Vararg` directly in UnionAll is deprecated (wrap the tuple instead).
WARNING: Wrapping `Vararg` directly in UnionAll is deprecated (wrap the tuple instead).
WARNING: Wrapping `Vararg` directly in UnionAll is deprecated (wrap the tuple instead).
WARNING: Wrapping `Vararg` directly in UnionAll is deprecated (wrap the tuple instead).
WARNING: Wrapping `Vararg` directly in UnionAll is deprecated (wrap the tuple instead).
WARNING: Wrapping `Vararg` directly in UnionAll is deprecated (wrap the tuple instead).
WARNING: Wrapping `Vararg` directly in UnionAll is deprecated (wrap the tuple instead).
WARNING: Wrapping `Vararg` directly in UnionAll is deprecated (wrap the tuple instead).
WARNING: Wrapping `Vararg` directly in UnionAll is deprecated (wrap the tuple instead).
WARNING: Wrapping `Vararg` directly in UnionAll is deprecated (wrap the tuple instead).
WARNING: Wrapping `Vararg` directly in UnionAll is deprecated (wrap the tuple instead).
...

which was added over a year ago. The conclusion is that deprecaion warnings without line information is useless and will be ignored by users.

So, another disadvantage is that this is postponing a real solution and creates future issues.

This is a false dichotomy. A real solution is in no way impacted by this.

IIUC, we have cases where functions frequently used in performance-critical code.

It is not only cases for performance critical code.

[tkf]

My comment was mainly for suggesting a systematic solution at a different level and not in particular against this PR. That's what I wanted to convey with "it may be a good idea to try to accumulate real world problems." More fundamentally, I wanted to clarify our understanding of the intended recievers of the deprecation messages.

I doubt users of it somehow thought it was public.

Knowledge is not enough if they don't take action. I think it is still helpful to let the package authors know how to improve the quality of their code.

Also, the current warning message is not good enough

This is an argument for improving the deprecation of the macro. There already was a "good" example of how to add the line information:

julia/base/deprecated.jl

Lines 222 to 228 in 6e8804b

	macro get!(h, key0, default)
	f, l = __source__.file, __source__.line
	@warn "`@get!(dict, key, default)` at $f:$l is deprecated, use `get!(()->default, dict, key)` instead."
	return quote
	get!(()->$(esc(default)), $(esc(h)), $(esc(key0)))
	end
	end

("good" in the sense that the line number is included)

[timholy]

Does not tell you where the problem is so no one will fix it.

Ugh, this has been a long-standing problem. We really need to fix the location-reporting of deprecations. The main rush of this was around the Julia 0.7/1.0 release but it keeps coming up.