fix Issue 18528 - dmd should deduplicate identical errors #15306

WalterBright · 2023-06-10T23:26:44Z

Reboot #15303 with different approach, hopefully much less disruptive.

dlang-bot · 2023-06-10T23:26:48Z

Thanks for your pull request, @WalterBright!

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	18528	normal	dmd should deduplicate identical errors

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#15306"

WalterBright · 2023-06-11T03:05:25Z

Still an amazing number of diffs.

ibuclaw · 2023-06-11T07:16:53Z

I suppose you've already tried node-based suppression?

I think the most common entrypoint for errors are the wrapper methods e.error("I didn't pass semantic checks");. Similarly, there's an e.errors boolean field that is set by many (but not all) nodes after an error occurs.

Do we already refuse to emit errors/deprecation if the errors field is already set?

WalterBright · 2023-06-12T00:14:20Z

I suppose you've already tried node-based suppression?

A lot of that is done. The trouble is it is not a general solution, and I was looking for a general solution. But as this PR shows, I'm thinking I should look again at the node base solution, as this one is a bit too blunt.

Just adding the check to the e.error function won't work, as the supplemental messages also need to be suppressed.

WalterBright · 2023-06-12T00:35:33Z

The specific message for this bugzilla is:

/Users/timothee/git_clone/D/ggplotd/source/ggplotd/geom.d(117,24): Deprecation: module `ggplotd.aes` member hasAesField is not visible from module geom
/Users/timothee/git_clone/D/ggplotd/source/ggplotd/geom.d(117,24): Deprecation: module `ggplotd.aes` member hasAesField is not visible from module geom

is not so easy to deal with without adding complexity.

ibuclaw · 2023-06-12T06:12:13Z

I suppose you've already tried node-based suppression?

A lot of that is done. The trouble is it is not a general solution, and I was looking for a general solution. But as this PR shows, I'm thinking I should look again at the node base solution, as this one is a bit too blunt.

Just adding the check to the e.error function won't work, as the supplemental messages also need to be suppressed.

Only supplemental messages relating to the same error? Other supplemental messages should be fine to emit. Back traces being one such example where it is desired to have the same line twice.

ibuclaw · 2023-06-12T09:15:15Z

I don't see it scaling well to always wrap grouped errors up into an if block - maybe a lambda?

RazvanN7

Thanks for taking care of this. A few notes:

stop adding #line directives to test files.
please delete the newly added extra lines in the test files.
updating the failure tests can be done automatically by running ./AUTO_UPDATE = 1 ./run.d test (except for test files where there are multiple error sections)
the current implementation is going to suppress some error messages when it shouldn't given that it takes only the location of the error message. This can already be observed in some of the modified test suite files. I assume there are other situations when we prune error messages when we shouldn't. Maybe a better way to do it is to the hash of the error message?

RazvanN7 · 2023-06-12T09:59:23Z

compiler/src/dmd/errors.d

+
+/************************************************************
+ * This is a simple scheme to eliminate duplication of error
+ * messages, which we'll define has having the same location.


Suggested change

* messages, which we'll define has having the same location.

* messages, which we'll define as having the same location.

RazvanN7 · 2023-06-12T09:59:38Z

compiler/src/dmd/errors.d

+ * The complication is that messages come in groups,
+ * all with the same location. This is dealt with by having
+ * some messages trigger the start of a group, which ends only
+ * when another start is seen, at wich point the previous location


Suggested change

* when another start is seen, at wich point the previous location

* when another start is seen, at which point the previous location

RazvanN7 · 2023-06-12T10:04:13Z

compiler/test/compilable/depmsg.d

+
+
+
+


Please delete these extra lines.

RazvanN7 · 2023-06-12T10:04:26Z

compiler/test/compilable/ddoc10236b.d

 ---
 */

+


this change is unrelated to this PR

RazvanN7 · 2023-06-12T10:04:38Z

compiler/test/compilable/ddoc10236.d

 compilable/ddoc10236.d(71): Warning: Ddoc: parameter count mismatch, expected 2, got 0
 compilable/ddoc10236.d(71):        Note that the format is `param = description`
 ---
 */

+


this change is unrelated to this PR

RazvanN7 · 2023-06-12T10:08:28Z

compiler/test/compilable/sw_transition_complex.d

 compilable/sw_transition_complex.d(132): Deprecation: use of complex type `creal` is deprecated, use `std.complex.Complex!(real)` instead
 ---
 */
+


RazvanN7 · 2023-06-12T10:08:35Z

compiler/test/compilable/test21514.d

+
+
+
+


RazvanN7 · 2023-06-12T10:09:00Z

compiler/test/compilable/test23097.d

 ---
 */
+
+


RazvanN7 · 2023-06-12T10:11:17Z

compiler/test/fail_compilation/cpp_abi_tag.d

@@ -8,7 +8,6 @@ fail_compilation/cpp_abi_tag.d(102): Error: `@gnuAbiTag` at least one argument e
 fail_compilation/cpp_abi_tag.d(105): Error: `@gnuAbiTag` at least one argument expected
 fail_compilation/cpp_abi_tag.d(108): Error: `@gnuAbiTag` char `0x99` not allowed in mangling
 fail_compilation/cpp_abi_tag.d(114): Error: argument `2` to `@gnuAbiTag` cannot be `null`
-fail_compilation/cpp_abi_tag.d(114): Error: argument `3` to `@gnuAbiTag` cannot be empty


I don't think this message should be elided.

RazvanN7 · 2023-06-12T10:12:19Z

compiler/test/fail_compilation/diag10319.d

 ---
 */

+#line 100


Please, stop adding #line directives. This makes reviewing the code much more difficult.

WalterBright · 2023-07-03T02:51:48Z

Abandoning this as unworkable in favor of #15314 and #15312

dlang-bot added the Bug Fix label Jun 10, 2023

WalterBright force-pushed the fix18528-2 branch 3 times, most recently from b5d2218 to 0aa421d Compare June 11, 2023 03:05

WalterBright force-pushed the fix18528-2 branch from 0aa421d to a17f0c5 Compare June 11, 2023 03:38

fix Issue 18528 - dmd should deduplicate identical errors

a17f0c5

WalterBright mentioned this pull request Jun 11, 2023

fix Issue 18528 - dmd should deduplicate identical errors #15303

Closed

WalterBright added the WIP Work In Progress - not ready for review or pulling label Jun 11, 2023

WalterBright mentioned this pull request Jun 12, 2023

fix Issue 18528 - dmd should deduplicate identical errors #15312

Merged

RazvanN7 requested changes Jun 12, 2023

View reviewed changes

dlang-bot added the Needs Rebase label Jun 26, 2023

WalterBright closed this Jul 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix Issue 18528 - dmd should deduplicate identical errors #15306

fix Issue 18528 - dmd should deduplicate identical errors #15306

WalterBright commented Jun 10, 2023

dlang-bot commented Jun 10, 2023

WalterBright commented Jun 11, 2023

ibuclaw commented Jun 11, 2023 •

edited

WalterBright commented Jun 12, 2023

WalterBright commented Jun 12, 2023

ibuclaw commented Jun 12, 2023

ibuclaw commented Jun 12, 2023 •

edited

RazvanN7 left a comment

RazvanN7 Jun 12, 2023

RazvanN7 Jun 12, 2023

RazvanN7 Jun 12, 2023

RazvanN7 Jun 12, 2023

RazvanN7 Jun 12, 2023

RazvanN7 Jun 12, 2023

RazvanN7 Jun 12, 2023

RazvanN7 Jun 12, 2023

RazvanN7 Jun 12, 2023

RazvanN7 Jun 12, 2023

WalterBright commented Jul 3, 2023

	* messages, which we'll define has having the same location.
	* messages, which we'll define as having the same location.

	* when another start is seen, at wich point the previous location
	* when another start is seen, at which point the previous location

fix Issue 18528 - dmd should deduplicate identical errors #15306

fix Issue 18528 - dmd should deduplicate identical errors #15306

Conversation

WalterBright commented Jun 10, 2023

dlang-bot commented Jun 10, 2023

Bugzilla references

Testing this PR locally

WalterBright commented Jun 11, 2023

ibuclaw commented Jun 11, 2023 • edited

WalterBright commented Jun 12, 2023

WalterBright commented Jun 12, 2023

ibuclaw commented Jun 12, 2023

ibuclaw commented Jun 12, 2023 • edited

RazvanN7 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

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

WalterBright commented Jul 3, 2023

ibuclaw commented Jun 11, 2023 •

edited

ibuclaw commented Jun 12, 2023 •

edited