fix Issue 18528 - dmd should deduplicate identical errors #15303
Conversation
|
Thanks for your pull request, @WalterBright! Bugzilla references
Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub run digger -- build "master + dmd#15303" |
WalterBright
force-pushed
the
fix18528
branch
6 times, most recently
from
bee349d
to
d08b644
Compare
|
Egad, an astounding number of diffs in the error messages. |
|
I'm going to try a different approach. This is too disruptive. |
There was a problem hiding this comment.
Some of the changes in diagnostic are dubious. A good chunk of them seem to stem from supplemental diagnostic messages being recorded and used to gag future genuine found errors. I suspect verrorPrint should be more capable to distinguish between diagnostica and supplementary information. That or having gag logic in verrorPrint is the wrong place to do so.
| fail_compilation/dtor_attributes.d(209): Error: `pure` function `dtor_attributes.test2` cannot call impure destructor `dtor_attributes.StrictClass.~this` | ||
| fail_compilation/dtor_attributes.d(204): generated `StrictClass.~this` is impure because of the following field's destructors: | ||
| fail_compilation/dtor_attributes.d(203): - HasDtor member | ||
| fail_compilation/dtor_attributes.d(103): impure `HasDtor.~this` is declared here | ||
| fail_compilation/dtor_attributes.d(209): Error: `pure` function `fail_compilation/dtor_attributes.test2` cannot call impure destructor `fail_compilation/dtor_attributes.StrictClass.~this` | ||
| fail_compilation/dtor_attributes.d(209): generated `StrictClass.~this` at fail_compilation/dtor_attributes.d(204) is impure because of the following field's destructors: | ||
| fail_compilation/dtor_attributes.d(209): field at fail_compilation/dtor_attributes.d(203) - HasDtor member | ||
| fail_compilation/dtor_attributes.d(209): impure `HasDtor.~this` is declared at fail_compilation/dtor_attributes.d(103) |
There was a problem hiding this comment.
Embedding file/line information in the message instead of cross-referencing the line is not an improvement, and makes diagnostic messages seriously worse for vcontext (enabled by default in at least one downstream).
| --- | ||
| */ | ||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
I don't see the point in wasting time adding line padding. If it's really important to keep line numbers in TEST_OUTPUT stable, it's simpler to just use #line.
| fail_compilation/test13786.d(16): Deprecation: `debug = <integer>` is deprecated, use debug identifiers instead | ||
| fail_compilation/test13786.d(18): Deprecation: `version = <integer>` is deprecated, use version identifiers instead | ||
| fail_compilation/test13786.d(16): Error: debug `123` level declaration must be at module level | ||
| fail_compilation/test13786.d(17): Error: debug `abc` declaration must be at module level | ||
| fail_compilation/test13786.d(18): Error: version `123` level declaration must be at module level | ||
| fail_compilation/test13786.d(19): Error: version `abc` declaration must be at module level | ||
| fail_compilation/test13786.d(22): Error: template instance `test13786.T!()` error instantiating |
There was a problem hiding this comment.
Deprecations should never hide/gag errors. Does that also mean you won't see the error when compiling with '-dw`?
| fail_compilation/parse13361.d(14): Error: empty attribute list is not allowed | ||
| fail_compilation/parse13361.d(14): Error: use `@(attributes)` instead of `[attributes]` |
There was a problem hiding this comment.
Isn't this just hiding the more important error?
| fail_compilation/issue20704.d(17): Error: `S(0)` is not an lvalue and cannot be modified | ||
| fail_compilation/issue20704.d(36): Error: template instance `issue20704.f2!(S)` error instantiating | ||
| fail_compilation/issue20704.d(17): Error: `null` is not an lvalue and cannot be modified | ||
| fail_compilation/issue20704.d(38): Error: template instance `issue20704.f2!(C)` error instantiating |
There was a problem hiding this comment.
So this change would mean you would get error instantiating messages, but the reason for failure is suppressed? How is that useful?
| fail_compilation/ice18753.d(23): Error: template instance `ice18753.isInputRange!(Group)` error instantiating | ||
| fail_compilation/ice18753.d(18): instantiated from here: `isForwardRange!(Group)` | ||
| fail_compilation/ice18753.d(23): instantiated from: `isForwardRange!(Group)` at fail_compilation/ice17853.d(18) | ||
| fail_compilation/ice18753.d(18): while evaluating: `static assert(isForwardRange!(Group))` |
There was a problem hiding this comment.
Embedding file/line in the message instead of cross referencing is not an improvement. Instantiation backtraces would suffer the most for this when vcontext is enabled.
| fail_compilation/fail10254.d(20): Error: `pure` function `fail10254.foo` cannot call impure constructor `fail10254.C.this` | ||
| fail_compilation/fail10254.d(20): Error: `@safe` function `fail10254.foo` cannot call `@system` constructor `fail10254.C.this` | ||
| fail_compilation/fail10254.d(15): `fail10254.C.this` is declared here | ||
| fail_compilation/fail10254.d(21): Error: `pure` function `fail10254.foo` cannot call impure constructor `fail10254.S.this` | ||
| fail_compilation/fail10254.d(21): Error: `@safe` function `fail10254.foo` cannot call `@system` constructor `fail10254.S.this` | ||
| fail_compilation/fail10254.d(16): `fail10254.S.this` is declared here |
There was a problem hiding this comment.
Isn't it better to diagnose both pure and @safe violations at the same time?
| compilable/sw_transition_complex.d(128): Deprecation: use of complex type `creal*` is deprecated, use `std.complex.Complex!(real)` instead | ||
| compilable/sw_transition_complex.d(128): Deprecation: use of imaginary type `ireal` is deprecated, use `real` instead |
There was a problem hiding this comment.
Deprecation message is gagged for no good reason.
|
@ibuclaw I agree with much of your comments, but I've already decided to redo it: "I'm going to try a different approach. This is too disruptive." |
|
Replaced with #15306 |
No description provided.