fix: emit E0600 when unary !/- is applied to unsupported type - #23147
Conversation
681dfbb to
3c8ef6d
Compare
|
@ChayimFriedman2 while I was thoroughly testing the alternative solutions I hit a subtle interaction and wanted to check with you before pushing. The problem
When those regions are active without
Fixture-only fix I triedRewriting What I have workingA guard inside
Test results
FramingYour suggestion was a fixture-only fix: add minicore directives and either implement primitive traits in minicore or switch tests to user types. I did the first half (directives + primitive impls in The guard I have handles cases the trait mechanism cannot decide because the trait itself is not in scope, which feels like a different problem from what your original guidance was ruling out (avoiding a caller-side workaround for missing primitive impls). Wanted to check that framing with you before pushing. |
|
Did you use AI for this? If yes, please read our AI policy. In particular, using AI to write GitHub comments, including PR descriptions, is forbidden. |
| // contain an error do not represent a real user mistake | ||
| // here, and would otherwise fire on incomplete code and on | ||
| // macro expansions that resolve to `{unknown}`. | ||
| if !operand_ty.is_ty_var() && !operand_ty.references_error() { |
There was a problem hiding this comment.
This is not the place. They should be filtered in resolve_diagnostics().
|
Hi @ChayimFriedman2, yes, I used AI to help me understand the codebase more deeply and generate alternative solutions, which I understand is allowed by the policy. I reviewed and tested every change thoroughly on my Windows machine. I've implemented the solution like you said. The diagnostic is now filtered in One small note: |
9e331e2 to
55fe0b0
Compare
| } | ||
| Err(_errors) => { | ||
| // FIXME: Report diagnostic. | ||
| // The diagnostic is filtered in `resolve_diagnostics` when the |
There was a problem hiding this comment.
This comment is redundant. this is true for all diagnostics containing types. Please remove it.
| //! async_fn: fn, tuple, future, copy | ||
| //! bool_impl: option, fn | ||
| //! builtin_impls: | ||
| //! builtin_impls: index, slice |
There was a problem hiding this comment.
builtin_impls shouldn't depend on anything. If some code needs additional flags, it should also be guarded by those flags, e.g.:
// region:builtin_impls
// region:index
...
// endregion:index
// endregion:builtin_implsAddresses the FIXME in `hir-ty/src/infer/op.rs` inside
`infer_user_unop`, which previously silently discarded operator method
resolution failures for `!x` and `-x` expressions.
When the operand's type does not implement `std::ops::Not` (for `!`) or
`std::ops::Neg` (for `-`), rust-analyzer now reports the same E0600
error that rustc produces:
cannot apply unary operator `!` to type `Question`
Wired through the standard inference diagnostic pipeline: new
`InferenceDiagnostic::UnaryOperatorCannotBeApplied` variant in hir-ty,
matching `UnaryOperatorCannotBeApplied` struct plus conversion in hir,
and a handler in ide-diagnostics using
`DiagnosticCode::RustcHardError("E0600")`.
Filtering for unresolved / error-typed operands is done in
`resolve_diagnostics()` (crates/hir-ty/src/infer/unify.rs) alongside
the existing `references_non_lt_error()` filter chain for other
diagnostics that carry a type. This keeps `infer_user_unop` free of
callsite guards and lets the natural inference pipeline suppress
spurious reports on incomplete code and on macro expansions that
infer to `{unknown}`.
The `unary_ops` region of `test-utils/src/minicore.rs` also gains
builtin `Not` and `Neg` impls, mirroring how `add_impl!` provides them
in the `add` region. Without these, the diagnostic test fixture would
incorrectly flag `!true`, `!0i32` and similar builtin uses as errors,
because `lookup_op_method` would find no impl in the minicore fixture
even though real `core` has one. With the impls present, primitives
resolve normally and only genuinely unsupported operators trigger the
diagnostic. This also lets us correctly report `-1u32` as E0600, since
real `core` does not implement `Neg` for unsigned integers.
Because the new `not_impl!` / `neg_impl!` blocks live in a nested
`region:builtin_impls` inside `region:unary_ops`, the new tests opt
into both flags via `//- minicore: unary_ops, builtin_impls`. The
existing `legacy_const_generics` test in `mismatched_arg_count` uses
`-1i32` / `-1i8` inline and now needs the same directive so that
`core::ops::Neg` is in scope for its operands.
Minicore `region:eq` and `region:float_consts` now depend on
`unary_ops, builtin_impls` so their smoke tests resolve `Not`/`Neg`
without per-callsite guards. The `Clone for [T; 1]` impl inside
`region:builtin_impls` uses `self[0]`, so it is scoped to a nested
`region:index` and only compiles when `index` is also enabled.
The `UnaryOp::Deref` case is left unchanged; it is already handled by
the `CannotBeDereferenced` diagnostic (E0614) and `infer_user_unop` is
never called for `Deref`.
Part of #22140.
55fe0b0 to
e6f2eb5
Compare
|
Thanks, I've done the changes.
|
|
I noticed an error on the Windows CI runner. All tests ran successfully locally on my Windows computer. I believe a transient glitch caused |
Addresses the FIXME in
infer_user_unop, which previously discarded operator method resolution failures for!xand-xexpressions.When the operand's type does not implement
std::ops::Not(for!) orstd::ops::Neg(for-), rust-analyzer now reports the same E0600 error that rustc produces:Wired through the standard inference diagnostic pipeline: new
InferenceDiagnostic::UnaryOperatorCannotBeAppliedvariant in hir-ty, matchingUnaryOperatorCannotBeAppliedstruct plus conversion in hir, and a handler in ide-diagnostics usingDiagnosticCode::RustcHardError("E0600").The diagnostic is suppressed when the operand is an unresolved type variable or already references an error, so that incomplete code and macro expansions that infer to
{unknown}do not produce spurious E0600 reports.The
unary_opsregion ofcrates/test-utils/src/minicore.rsalso gains builtinNotandNegimpls, mirroring howadd_impl!provides them in theaddregion. Without these, the diagnostic test fixture would incorrectly flag!true,!0i32and similar builtin uses as errors, becauselookup_op_methodwould find no impl in the minicore fixture even though realcorehas one. With the impls present, primitives resolve normally and only genuinely unsupported operators trigger the diagnostic. This also lets us correctly report-1u32as E0600, since realcoredoes not implementNegfor unsigned integers.Because the new
not_impl!/neg_impl!blocks live in a nestedregion:builtin_implsinsideregion:unary_ops, the new tests opt into both flags via//- minicore: unary_ops, builtin_impls. The existinglegacy_const_genericstest inmismatched_arg_countuses-1i32/-1i8inline and now needs the same directive so thatcore::ops::Negis in scope for its operands.The
UnaryOp::Derefcase is left unchanged; it is already handled by theCannotBeDereferenceddiagnostic (E0614) andinfer_user_unopis never called forDeref.Refs #22140
r? @ChayimFriedman2