Skip to content

user facing code should use not use PostAnalysis #20447

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.

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

Merged
merged 1 commit into from
Aug 19, 2025

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Aug 13, 2025

this currently causes some fun ICEs 😁

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 13, 2025
@ShoyuVanilla
Copy link
Member

diff --git a/crates/hir-ty/src/next_solver/interner.rs b/crates/hir-ty/src/next_solver/interner.rs
index 9c1bd52065..b842eeaedd 100644
--- a/crates/hir-ty/src/next_solver/interner.rs
+++ b/crates/hir-ty/src/next_solver/interner.rs
@@ -283,9 +283,7 @@ impl<'db> DbInterner<'db> {
     // FIXME(next-solver): remove this method
     pub fn conjure() -> DbInterner<'db> {
         salsa::with_attached_database(|db| DbInterner {
-            db: unsafe {
-                std::mem::transmute::<&dyn HirDatabase, &'db dyn HirDatabase>(db.as_view())
-            },
+            db: unsafe { std::mem::transmute::<&dyn salsa::Database, &'db dyn HirDatabase>(db) },
             krate: None,
             block: None,
         })

The test failures are gone with this small change. Not sure this is the correct way but I guess it wouldn't be more wrong since it's a "FIXME remove" 😅

@ChayimFriedman2
Copy link
Contributor

The test failures are gone with this small change. Not sure this is the correct way but I guess it wouldn't be more wrong since it's a "FIXME remove" 😅

That's definitely wildly incorrect, I'm not sure how the tests even succeed with this. You cannot transmute the types, the vtables are different.

@ShoyuVanilla
Copy link
Member

Then the correct way would be registering downcast view for HirDatabase

@ChayimFriedman2
Copy link
Contributor

Yeah, the problem is that no hir-ty query is being called before we call the solver. Calling any query that takes &dyn HirDatabase should work, but ugh.

@ChayimFriedman2
Copy link
Contributor

Also note that in real usage this won't ever happen, the problem is only in tests.

@ShoyuVanilla
Copy link
Member

Yeah, the problem is that no hir-ty query is being called before we call the solver. Calling any query that takes &dyn HirDatabase should work, but ugh.

I guess in real world use-case scenarios it would be called before triggering the assists by other automatic features like inlay hints but this isn't the case in the test suite. How about just calling something on test functions?

@ShoyuVanilla
Copy link
Member

Also note that in real usage this won't ever happen, the problem is only in tests.

Oh, I was typing the same thing 😅

@ShoyuVanilla
Copy link
Member

diff --git a/crates/ide-assists/src/tests.rs b/crates/ide-assists/src/tests.rs
index f4daabfe91..c179c5f20c 100644
--- a/crates/ide-assists/src/tests.rs
+++ b/crates/ide-assists/src/tests.rs
@@ -1,7 +1,7 @@
 mod generated;
 
 use expect_test::expect;
-use hir::{Semantics, setup_tracing};
+use hir::{Semantics, db::HirDatabase, setup_tracing};
 use ide_db::{
     EditionedFileId, FileRange, RootDatabase, SnippetCap,
     assists::ExprFillDefaultMode,
@@ -309,6 +309,8 @@ fn check_with_config(
     let (mut db, file_with_caret_id, range_or_offset) = RootDatabase::with_range_or_offset(before);
     db.enable_proc_attr_macros();
     let text_without_caret = db.file_text(file_with_caret_id.file_id(&db)).text(&db).to_string();
+    let krate = db.test_crate();
+    let _ = db.inherent_impls_in_crate(krate);
 
     let frange = hir::FileRange { file_id: file_with_caret_id, range: range_or_offset.into() };
 

Calling any query resolves the failures, indeed

@Veykril
Copy link
Member

Veykril commented Aug 16, 2025

#20470 should make the tests work, we can basically side step this by registering the downcaster ourselves

@Veykril Veykril enabled auto-merge August 18, 2025 21:00
@Veykril
Copy link
Member

Veykril commented Aug 18, 2025

@lcnr can you rebase this? I believe CI should turn green now

auto-merge was automatically disabled August 19, 2025 06:33

Head branch was pushed to by a user without write access

@lcnr
Copy link
Contributor Author

lcnr commented Aug 19, 2025

done

@ShoyuVanilla ShoyuVanilla enabled auto-merge August 19, 2025 06:34
@ShoyuVanilla ShoyuVanilla added this pull request to the merge queue Aug 19, 2025
Merged via the queue into rust-lang:master with commit 58bbdec Aug 19, 2025
15 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants