Skip to content
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

Some warnings in inlined code show up at the callsite #22790

Open
lrytz opened this issue Mar 13, 2025 · 4 comments
Open

Some warnings in inlined code show up at the callsite #22790

lrytz opened this issue Mar 13, 2025 · 4 comments
Labels
area:inline area:reporting Error reporting including formatting, implicit suggestions, etc itype:bug itype:question

Comments

@lrytz
Copy link
Member

lrytz commented Mar 13, 2025

Some warnings in inlined code show up at the callsite, some don't. It depends on the compiler phase in which the warning is issued. That seems arbitrary.

For example, this definition triggers two warnings:

scala> inline def foo = { 1; Stream(1).head }
2 warnings found
-- [E129] Potential Issue Warning: ---------------------------------------------
1 |inline def foo = { 1; Stream(1).head }
  |                   ^
  |                   A pure expression does nothing in statement position
  |
  | longer explanation available when compiling with `-explain`
-- Deprecation Warning: --------------------------------------------------------
1 |inline def foo = { 1; Stream(1).head }
  |                      ^^^^^^
  |value Stream in package scala is deprecated since 2.13.0: Use LazyList instead of Stream
def foo: Int

One of them shows up when using the inline method:

scala> foo
1 warning found
-- Deprecation Warning: --------------------------------------------------------
1 |inline def foo = { 1; Stream(1).head }
  |                      ^^^^^^
  |value Stream in package scala is deprecated since 2.13.0: Use LazyList instead of Stream
val res0: Int = 1

The recent #22682 makes sure that messages filtered by @nowarn in the inline def don't leak to the callsite.

But does it ever make sense to see warnings in inlined code? Should that be configurable with a flag?

@lrytz lrytz added area:inline area:reporting Error reporting including formatting, implicit suggestions, etc itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Mar 13, 2025
@Gedochao Gedochao added itype:question and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Mar 13, 2025
@Gedochao
Copy link
Contributor

cc @jchyb

@jchyb
Copy link
Contributor

jchyb commented Mar 13, 2025

Sometimes it makes sense to have them, especially if we consider macros (where warnings can be dynamically created, either by directly reporting a warning, or creating a specific tree, and we only get those at the inline call site). It could be argued that the deprecation warning could be useful if we are using an inlined method from another library, where we would also only get that warning at a call site (but I don't know if that's a good argument). I agree it's probably currently a bit arbitrary which ones are supported, but there were certain checks/warnings already that were turned off for inline methods.

Somewhat amusingly, I wanted to show how useful a deprecation warning could be in the context of a macro, but I ended up with an issue ticket: #22795

I don't see the point of configuring that with a flag, especially that the way the reporter works now, duplicate warnings are not reported (so outside of repl, we would not see that second one).

@lrytz
Copy link
Member Author

lrytz commented Mar 13, 2025

especially if we consider macros

Scala 2 has, for unused warnings:

Usage: -Wmacros:<mode> where <mode> choices are none, before, after, both, default (default: default).
  none     Do not inspect expansions or their original trees when generating unused symbol warnings.
  before   Only inspect unexpanded user-written code for unused symbols.
  after    Only inspect expanded trees when generating unused symbol warnings.
  both     Inspect both user-written code and expanded trees when generating unused symbol warnings.
  default  Only inspect unexpanded user-written code for unused symbols but include usages in expansions.

duplicate warnings are not reported (so outside of repl, we would not see that second one).

If we have two files and compile the definition and the usage separately, the deprecation shows when compiling the usage.

The warning position could actually be pretty confusing for users. They will see a warning pointing to a source file that they don't actually have if the inline definition is in a dependency.

@som-snytt
Copy link
Contributor

Porting -Wmacros is a TODO; there is commented code for hypothetical -Winlined.

My example for warning at call site is that implicit search can induce a warning. (The warning is context-dependent.)

I observed the "foreign position" problem but haven't given it thought.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:inline area:reporting Error reporting including formatting, implicit suggestions, etc itype:bug itype:question
Projects
None yet
Development

No branches or pull requests

4 participants