Support src filter in -WConf (Closes #17635) #18783

povder · 2023-10-28T21:42:49Z

Fixes #18782

povder · 2023-10-28T22:26:00Z

Related to #18782 #17634 #17635

som-snytt · 2023-10-28T22:31:28Z

added the f- word to the comment, which links the ticket; I hope that was intended.

lrytz

Thanks @povder!

compiler/src/dotty/tools/dotc/reporting/WConf.scala

austek · 2023-10-30T13:21:59Z

compiler/src/dotty/tools/dotc/reporting/WConf.scala

    case None => false

  case Any, Deprecated, Feature, Unchecked, None
  case MessagePattern(pattern: Regex)
  case MessageID(errorId: ErrorMessageID)
+  case SourcePattern(pattern: Regex)


Old code had a cache, shouldn't that be provided too?

cache added

lrytz · 2023-10-31T08:27:38Z

compiler/src/dotty/tools/dotc/reporting/WConf.scala


 enum MessageFilter:
+
+  private val sourcePatternCache = mutable.Map.empty[SourceFile, Boolean]


The cache should probably be cleared on each compiler run (dotc.Compiler.newRun). What utilities does dotty have to manage caches?

There is the PerRun in Definitions.

Or we could make it a Map of (pattern, SourceFile) -> Boolean. I think there is no state to influence the check if a source file matches a pattern.

Multiple compiler instances running at the same time might concurrently try to access and update this cache, so if this really needs to be a global map (seems like it should be tied to the Context instead?) it needs to be a ConcurrentHashMap (I'm surprised this wasn't caught by https://github.com/lampepfl/dotty/blob/main/compiler/src/dotty/tools/dotc/transform/CheckReentrant.scala)

Not knowing how this works exactly from the concurrency perspective I attempted porting over Scala2 implementation which was a plain mutable map. Given concerns about the concurrency though, would it be acceptable to not do caching in this PR? We could always make an optimization later without holding back the feature

TBH, it was me who mentioned the cache. I can withdraw my comment

cache removed

charego · 2023-11-30T18:45:19Z

compiler/src/dotty/tools/dotc/config/ScalaSettings.scala

@@ -266,6 +269,7 @@ private sealed trait WarningSettings:
         |Examples:
         |  - change every warning into an error: -Wconf:any:error
         |  - silence deprecations: -Wconf:cat=deprecation:s
+         |  - silence warnings in src_managed directory: -Wconf:src=src_managed/.*


Shouldn't this example end with :s or :silent?

You're correct, fixed https://github.com/lampepfl/dotty/pull/18783/files#diff-05e59408426c08d96c341addfb7f7a06353bd762027e856e8d4c1e8719e5c6bdR277

szymon-rd · 2023-12-04T14:31:21Z

compiler/test/dotty/tools/dotc/config/ScalaSettingsTests.scala

@@ -96,5 +101,52 @@ class ScalaSettingsTests:
    assertEquals(Action.Silent, sut.action(depr))


+  private def wconfSrcFilterTest(argsStr: String, source: util.SourceFile, expectedAction: reporting.Action): Unit =


Please add a test with malformed regex and add an assertion on the error.

Add also a test that mixes it with different wconf flags

Please add a test with malformed regex and add an assertion on the error.

Added

d65e8f8#diff-f4b356d10a0ec15d5030bed4c51b0fc839753cc43c5854475ed9701e6bc5148bR170

Add also a test that mixes it with different wconf flags

Added

d65e8f8#diff-f4b356d10a0ec15d5030bed4c51b0fc839753cc43c5854475ed9701e6bc5148bR187

lrytz · 2024-03-05T13:30:42Z

compiler/test/dotty/tools/dotc/config/ScalaSettingsTests.scala

+      )
+    )
+
+  @Test def `WConf src filter can be mixed with other filters with rightmost taking precedence`: Unit =


Hmm, in Scala 2 the leftmost takes precedence. Also -Wconf:help (on Scala 3) says otherwise.

https://github.com/lampepfl/dotty/blob/a095115b43398feeac3ee4329edb87f87da00012/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala#L268-L269

So this is a bug that should be fixed. I think a separate PR is better, so I filed #19885.

tOverney · 2024-04-15T08:47:58Z

It's really awesome to see this PR and all the efforts done by @povder !

What is holding this up at the moment? Is it the "change requested" review from @szymon-rd from December?

It would be amazing to get this feature back in Scala3

povder · 2024-04-22T16:02:01Z

@szymon-rd @Kordyjan I've addressed the comments quite a while ago, could I have another review or possibly this could be merged now?

szymon-rd

LGTM now - But I am no longer in the team maintaining this repo, so probably better to tag someone else to merge it to main @Kordyjan
edit: Oh, I see you already tagged him

Linyxus

Looks great to me!

mkurz · 2024-04-25T11:41:22Z

Would it be possible to backport this to 3.3.x?

Conflict between scala#18783 and scala#19766

Kordyjan · 2024-04-25T11:53:02Z

Would it be possible to backport this to 3.3.x?

I think it will be backported. We are trying to have all reporting and lining improvements on LTS.

mkurz · 2024-04-25T11:58:43Z

Would it be possible to backport this to 3.3.x?

I think it will be backported. We are trying to have all reporting and lining improvements on LTS.

Awesome! Should I open a backport PR? Or will you do this?

Conflict between #18783 and #19766. See `val sets = ScalaSettings` in the test just above.

Fixes scala#18782

mkurz · 2024-04-29T07:11:17Z

I opened a backport PR:

Backport "Support src filter in -WConf (Closes #17635) (#18783)" #20282

Fixes scala#18782

Fixes #18782 [Cherry-picked 86e2fe5][modified]

Backports #18783 to the LTS branch. PR submitted by the release tooling. [skip ci]

povder force-pushed the wconf-src branch from cf47ee1 to ccf5ac0 Compare October 28, 2023 21:44

SethTisue requested a review from lrytz October 28, 2023 22:23

lrytz reviewed Oct 30, 2023

View reviewed changes

compiler/src/dotty/tools/dotc/reporting/WConf.scala Outdated Show resolved Hide resolved

austek reviewed Oct 30, 2023

View reviewed changes

Kordyjan requested a review from szymon-rd October 30, 2023 15:14

Kordyjan assigned szymon-rd Oct 30, 2023

povder force-pushed the wconf-src branch 2 times, most recently from ad9fd2a to dd2e03b Compare October 31, 2023 03:25

povder requested review from austek and lrytz October 31, 2023 05:18

lrytz reviewed Oct 31, 2023

View reviewed changes

povder force-pushed the wconf-src branch from dd2e03b to 0000177 Compare November 4, 2023 20:34

povder requested a review from lrytz November 5, 2023 06:05

austek approved these changes Nov 8, 2023

View reviewed changes

povder requested a review from smarter November 14, 2023 01:59

charego reviewed Nov 30, 2023

View reviewed changes

szymon-rd suggested changes Dec 4, 2023

View reviewed changes

mkurz mentioned this pull request Dec 19, 2023

route file compilation warning ("-Ywarn-unused"): local val in method at is never used playframework/playframework#6302

Open

armanbilge mentioned this pull request Jan 8, 2024

sdk-exporter-trace: add OtlpHttpSpanExporter typelevel/otel4s#421

Merged

povder force-pushed the wconf-src branch 2 times, most recently from 0cb7d2e to 4f40ecd Compare March 4, 2024 02:03

Support src filter in -WConf (Closes scala#18782)

d65e8f8

povder force-pushed the wconf-src branch from 4f40ecd to d65e8f8 Compare March 4, 2024 02:45

povder requested review from szymon-rd and charego March 4, 2024 06:31

lrytz approved these changes Mar 5, 2024

View reviewed changes

szymon-rd approved these changes Apr 22, 2024

View reviewed changes

Gedochao assigned Linyxus and unassigned szymon-rd Apr 23, 2024

Gedochao requested review from Linyxus and removed request for charego April 23, 2024 10:04

Linyxus approved these changes Apr 23, 2024

View reviewed changes

Gedochao merged commit 86e2fe5 into scala:main Apr 25, 2024
19 checks passed

Gedochao added the backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. label Apr 25, 2024

nicolasstucki added a commit to dotty-staging/dotty that referenced this pull request Apr 25, 2024

Fix conflict with new ScalaSettings

48bfe81

Conflict between scala#18783 and scala#19766

nicolasstucki mentioned this pull request Apr 25, 2024

Fix conflict with new ScalaSettings #20264

Merged

Gedochao removed the backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. label Apr 25, 2024

nicolasstucki added a commit that referenced this pull request Apr 25, 2024

Fix conflict with new ScalaSettings (#20264)

1cf779c

Conflict between #18783 and #19766. See `val sets = ScalaSettings` in the test just above.

mkurz pushed a commit to mkurz/dotty that referenced this pull request Apr 29, 2024

Support src filter in -WConf (Closes scala#17635) (scala#18783)

aa9861a

Fixes scala#18782

mkurz mentioned this pull request Apr 29, 2024

Backport "Support src filter in -WConf (Closes #17635) (#18783)" #20282

Closed

mkurz pushed a commit to mkurz/dotty that referenced this pull request Apr 30, 2024

Support src filter in -WConf (Closes scala#17635) (scala#18783)

89d5479

Fixes scala#18782

Kordyjan added this to the 3.5.0 milestone May 10, 2024

WojciechMazur added a commit that referenced this pull request Jul 6, 2024

Support src filter in -WConf (Closes #17635) (#18783)

6397a0d

Fixes #18782 [Cherry-picked 86e2fe5][modified]

WojciechMazur mentioned this pull request Jul 6, 2024

Backport "Support src filter in -WConf (Closes #17635)" to LTS #21087

Merged

WojciechMazur added a commit that referenced this pull request Jul 6, 2024

Backport "Support src filter in -WConf (Closes #17635)" to LTS (#21087)

4f326c6

Backports #18783 to the LTS branch. PR submitted by the release tooling. [skip ci]

mkurz mentioned this pull request Jul 8, 2024

Unused import warning in routes (Play 2.6.0-RC1 and Scala 2.12.2) playframework/playframework#7382

Open

lenguyenthanh mentioned this pull request Aug 17, 2024

Is it possible to disable the unused warnings for generated src code in target/src_manged? typelevel/sbt-tpolecat#125

Open

mpro7 mentioned this pull request Sep 2, 2024

chore: Remove unused imports from TWIRL templates and silence false positives dasch-swiss/dsp-api#3351

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support src filter in -WConf (Closes #17635) #18783

Support src filter in -WConf (Closes #17635) #18783

povder commented Oct 28, 2023 •

edited by som-snytt

Loading

povder commented Oct 28, 2023 •

edited

Loading

som-snytt commented Oct 28, 2023

lrytz left a comment

austek Oct 30, 2023

povder Oct 31, 2023

lrytz Oct 31, 2023

szymon-rd Oct 31, 2023 •

edited

Loading

smarter Oct 31, 2023

povder Oct 31, 2023

austek Nov 2, 2023

povder Nov 4, 2023

charego Nov 30, 2023

povder Mar 4, 2024

szymon-rd Dec 4, 2023

szymon-rd Dec 4, 2023

povder Mar 4, 2024 •

edited

Loading

povder Mar 4, 2024 •

edited

Loading

lrytz Mar 5, 2024

tOverney commented Apr 15, 2024

povder commented Apr 22, 2024

szymon-rd left a comment •

edited

Loading

Linyxus left a comment

mkurz commented Apr 25, 2024

Kordyjan commented Apr 25, 2024

mkurz commented Apr 25, 2024

mkurz commented Apr 29, 2024


		enum MessageFilter:

		private val sourcePatternCache = mutable.Map.empty[SourceFile, Boolean]

		@@ -96,5 +101,52 @@ class ScalaSettingsTests:
		assertEquals(Action.Silent, sut.action(depr))


		private def wconfSrcFilterTest(argsStr: String, source: util.SourceFile, expectedAction: reporting.Action): Unit =

Support src filter in -WConf (Closes #17635) #18783

Support src filter in -WConf (Closes #17635) #18783

Conversation

povder commented Oct 28, 2023 • edited by som-snytt Loading

povder commented Oct 28, 2023 • edited Loading

som-snytt commented Oct 28, 2023

lrytz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

szymon-rd Oct 31, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

povder Mar 4, 2024 • edited Loading

Choose a reason for hiding this comment

povder Mar 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tOverney commented Apr 15, 2024

povder commented Apr 22, 2024

szymon-rd left a comment • edited Loading

Choose a reason for hiding this comment

Linyxus left a comment

Choose a reason for hiding this comment

mkurz commented Apr 25, 2024

Kordyjan commented Apr 25, 2024

mkurz commented Apr 25, 2024

mkurz commented Apr 29, 2024

povder commented Oct 28, 2023 •

edited by som-snytt

Loading

povder commented Oct 28, 2023 •

edited

Loading

szymon-rd Oct 31, 2023 •

edited

Loading

povder Mar 4, 2024 •

edited

Loading

povder Mar 4, 2024 •

edited

Loading

szymon-rd left a comment •

edited

Loading