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.

Sign up for GitHub

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

Jump to bottom

suppress field expr with generics error message if it's a method #134154

Merged
merged 1 commit into from
Dec 12, 2024
Merged

suppress field expr with generics error message if it's a method #134154

merged 1 commit into from
Dec 12, 2024

Conversation

dev-ardi
Copy link
Contributor

@dev-ardi dev-ardi commented Dec 10, 2024 •
edited
Loading

Don't emit "field expressions may not have generic arguments" if it's a method call without ()

r? estebank
Fixes #67680

Is this the best way to go? It's by far the simplest I could come up with.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 10, 2024
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@dev-ardi dev-ardi force-pushed the field-expr-generics branch from 2906712 to ec5e685 Compare December 11, 2024 01:35
@rust-log-analyzer

This comment has been minimized.

Sign in to view
compiler-errors
compiler-errors approved these changes Dec 11, 2024
View reviewed changes
Copy link
Member

@compiler-errors compiler-errors left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please address the nits, then I can approve.

And also fix the test file to not mention "issue". It's redundant -- leave a comment inside saying // Regression test for <...> if you want.

compiler/rustc_errors/src/lib.rs Show resolved Hide resolved
compiler/rustc_hir_typeck/src/expr.rs Outdated Show resolved Hide resolved
compiler/rustc_parse/src/parser/expr.rs Outdated Show resolved Hide resolved
@compiler-errors
Copy link
Member

Also please give this a more descriptive error message. This is approximately the most generic message that could exist currently 😆

@dev-ardi dev-ardi force-pushed the field-expr-generics branch from ec5e685 to 1e5738b Compare December 11, 2024 14:07
@compiler-errors
Copy link
Member

Whoops I misspoke in the last comment--I meant that this needs a better PR title :)

@dev-ardi
Copy link
Contributor Author

Oh I was actually changing the error message :P

@dev-ardi dev-ardi changed the title Fix confusing error message suppress field expr with generics error message if it's a method Dec 11, 2024
@dev-ardi
Copy link
Contributor Author

Now that I'm on it, do you think that
"attempted to access {field}on type{expr_t} as a field when it's a method",
is better than
"attempted to access field {field}on type{expr_t}",
?

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@compiler-errors
Copy link
Member

I think the error message is fine as-is

@dev-ardi dev-ardi force-pushed the field-expr-generics branch from 1e5738b to 5aebe6f Compare December 11, 2024 14:50
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@dev-ardi dev-ardi force-pushed the field-expr-generics branch from 5aebe6f to 014363e Compare December 11, 2024 15:23
@compiler-errors
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Dec 11, 2024

📌 Commit 014363e has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 11, 2024
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Dec 12, 2024
@jieyouxu
Rollup merge of rust-lang#134154 - dev-ardi:field-expr-generics, r=co…
c5a1869 
…mpiler-errors

suppress field expr with generics error message if it's a method

Don't emit "field expressions may not have generic arguments" if it's a method call without `()`

r? estebank
Fixes rust-lang#67680

Is this the best way to go? It's by far the simplest I could come up with.
@jieyouxu jieyouxu mentioned this pull request Dec 12, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 12, 2024
@bors
Auto merge of rust-lang#134195 - jieyouxu:rollup-xnaenjd, r=jieyouxu
c06a6da 
Rollup of 13 pull requests

Successful merges:

 - rust-lang#122003 (link libunwind dynamically and allow controlling it via `crt-static` on gnullvm targets)
 - rust-lang#133122 (Add unpolished, experimental support for AFIDT (async fn in dyn trait))
 - rust-lang#133859 (Move some alloc tests to the alloctests crate)
 - rust-lang#134070 (Some asm! diagnostic adjustments and a papercut fix)
 - rust-lang#134095 ([CI] Use a lockfile for installing the `datadog` package)
 - rust-lang#134144 (Properly consider APITs for never type fallback ascription fix)
 - rust-lang#134152 (Simplify `rustc_mir_dataflow::abs_domain`.)
 - rust-lang#134154 (suppress field expr with generics error message if it's a method)
 - rust-lang#134155 (Forbid `unsafe_op_in_unsafe_fn` for Hurd)
 - rust-lang#134173 (allow `symbol_intern_string_literal` lint in test modules)
 - rust-lang#134178 (Stabilize the Rust 2024 prelude)
 - rust-lang#134179 (Remove outdated consteval note from `<*mut T>::align_offset` docs.)
 - rust-lang#134187 (Remove `PErr`.)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr matthiaskrgr mentioned this pull request Dec 12, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 12, 2024
@bors
Auto merge of rust-lang#134201 - matthiaskrgr:rollup-22b721y, r=matth…
4606a4d 
…iaskrgr

Rollup of 11 pull requests

Successful merges:

 - rust-lang#122003 (link libunwind dynamically and allow controlling it via `crt-static` on gnullvm targets)
 - rust-lang#133859 (Move some alloc tests to the alloctests crate)
 - rust-lang#134070 (Some asm! diagnostic adjustments and a papercut fix)
 - rust-lang#134144 (Properly consider APITs for never type fallback ascription fix)
 - rust-lang#134152 (Simplify `rustc_mir_dataflow::abs_domain`.)
 - rust-lang#134154 (suppress field expr with generics error message if it's a method)
 - rust-lang#134155 (Forbid `unsafe_op_in_unsafe_fn` for Hurd)
 - rust-lang#134173 (allow `symbol_intern_string_literal` lint in test modules)
 - rust-lang#134178 (Stabilize the Rust 2024 prelude)
 - rust-lang#134179 (Remove outdated consteval note from `<*mut T>::align_offset` docs.)
 - rust-lang#134187 (Remove `PErr`.)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 1d78422 into rust-lang:master Dec 12, 2024
6 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Dec 12, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Dec 12, 2024
@rust-timer
Unrolled build for rust-lang#134154
4008ac4 
Rollup merge of rust-lang#134154 - dev-ardi:field-expr-generics, r=compiler-errors

suppress field expr with generics error message if it's a method

Don't emit "field expressions may not have generic arguments" if it's a method call without `()`

r? estebank
Fixes rust-lang#67680

Is this the best way to go? It's by far the simplest I could come up with.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@compiler-errors compiler-errors compiler-errors approved these changes

Assignees

@compiler-errors compiler-errors

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.85.0
Development

Successfully merging this pull request may close these issues.

Confusing hint on missing parentheses after collect<>
6 participants
@dev-ardi @rust-log-analyzer @compiler-errors @bors @estebank @rustbot