validated negative impls of generated impls (#6490)

crates/cairo-lang-lowering/src/lower/mod.rs

@@ -1800,7 +1800,7 @@ fn add_closure_call_function(
     let impl_id = ImplLongId::GeneratedImpl(
         GeneratedImplLongId {
             concrete_trait,
-            generic_args: vec![],
+            generic_params: vec![],
             impl_items: GeneratedImplItems(iter::once((ret_ty, closure_ty.ret_ty)).collect()),
         }
         .intern(semantic_db),

crates/cairo-lang-lowering/src/lower/test_data/closure

@@ -256,6 +256,27 @@ End:
   Return()
 
 
+Generated core::traits::Destruct::destruct lowering for source location:
+    let c =  || a;
+             ^^
+
+Parameters: v0: {[email protected]:6:14: 6:16}
+blk0 (root):
+Statements:
+  (v1: core::integer::u32) <- struct_destructure(v0)
+  (v2: ()) <- struct_construct()
+End:
+  Return(v2)
+
+
+Final lowering:
+Parameters: v0: {[email protected]:6:14: 6:16}
+blk0 (root):
+Statements:
+End:
+  Return()
+
+
 Generated core::ops::function::FnOnce::call lowering for source location:
     let c =  || a;
              ^^

crates/cairo-lang-semantic/src/expr/inference.rs

@@ -12,11 +12,13 @@ use cairo_lang_defs::ids::{
     LookupItemId, MemberId, ParamId, StructId, TraitConstantId, TraitFunctionId, TraitId,
     TraitImplId, TraitTypeId, VarId, VariantId,
 };
-use cairo_lang_diagnostics::{DiagnosticAdded, skip_diagnostic};
+use cairo_lang_diagnostics::{DiagnosticAdded, Maybe, skip_diagnostic};
 use cairo_lang_proc_macros::{DebugWithDb, SemanticObject};
 use cairo_lang_syntax::node::ids::SyntaxStablePtrId;
 use cairo_lang_utils::ordered_hash_map::{Entry, OrderedHashMap};
-use cairo_lang_utils::{Intern, LookupIntern, define_short_id, extract_matches};
+use cairo_lang_utils::{
+    Intern, LookupIntern, define_short_id, extract_matches, try_extract_matches,
+};
 
 use self::canonic::{CanonicalImpl, CanonicalMapping, CanonicalTrait, NoError};
 use self::solver::{Ambiguity, SolutionSet, enrich_lookup_context};
@@ -930,63 +932,93 @@ impl<'db> Inference<'db> {
         lookup_context: &ImplLookupContext,
         canonical_impl: CanonicalImpl,
     ) -> InferenceResult<SolutionSet<CanonicalImpl>> {
-        let ImplLongId::Concrete(concrete_impl) = canonical_impl.0.lookup_intern(self.db) else {
-            return Ok(SolutionSet::Unique(canonical_impl));
-        };
-        let mut rewriter = SubstitutionRewriter {
-            db: self.db,
-            substitution: &concrete_impl
-                .substitution(self.db)
-                .map_err(|diag_added| self.set_error(InferenceError::Reported(diag_added)))?,
-        };
-
-        for garg in self
-            .db
-            .impl_def_generic_params(concrete_impl.impl_def_id(self.db))
-            .map_err(|diag_added| self.set_error(InferenceError::Reported(diag_added)))?
-        {
-            let GenericParam::NegImpl(neg_impl) = garg else {
-                continue;
-            };
-
-            let concrete_trait_id = rewriter
-                .rewrite(neg_impl)
-                .map_err(|diag_added| self.set_error(InferenceError::Reported(diag_added)))?
-                .concrete_trait
-                .map_err(|diag_added| self.set_error(InferenceError::Reported(diag_added)))?;
-            for garg in concrete_trait_id.generic_args(self.db) {
-                let GenericArgumentId::Type(ty) = garg else {
-                    continue;
-                };
+        /// Validates that no solution set is found for the negative impls.
+        fn validate_no_solution_set(
+            inference: &mut Inference<'_>,
+            canonical_impl: CanonicalImpl,
+            lookup_context: &ImplLookupContext,
+            negative_impls_concrete_traits: impl Iterator<Item = Maybe<ConcreteTraitId>>,
+        ) -> InferenceResult<SolutionSet<CanonicalImpl>> {
+            for concrete_trait_id in negative_impls_concrete_traits {
+                let concrete_trait_id = concrete_trait_id.map_err(|diag_added| {
+                    inference.set_error(InferenceError::Reported(diag_added))
+                })?;
+                for garg in concrete_trait_id.generic_args(inference.db) {
+                    let GenericArgumentId::Type(ty) = garg else {
+                        continue;
+                    };
+
+                    // If the negative impl has a generic argument that is not fully
+                    // concrete we can't tell if we should rule out the candidate impl.
+                    // For example if we have -TypeEqual<S, T> we can't tell if S and
+                    // T are going to be assigned the same concrete type.
+                    // We return `SolutionSet::Ambiguous` here to indicate that more
+                    // information is needed.
+                    if !ty.is_fully_concrete(inference.db) {
+                        // TODO(ilya): Try to detect the ambiguity earlier in the
+                        // inference process.
+                        return Ok(SolutionSet::Ambiguous(
+                            Ambiguity::NegativeImplWithUnresolvedGenericArgs {
+                                impl_id: canonical_impl.0,
+                                ty,
+                            },
+                        ));
+                    }
+                }
 
-                // If the negative impl has a generic argument that is not fully
-                // concrete we can't tell if we should rule out the candidate impl.
-                // For example if we have -TypeEqual<S, T> we can't tell if S and
-                // T are going to be assigned the same concrete type.
-                // We return `SolutionSet::Ambiguous` here to indicate that more
-                // information is needed.
-                if !ty.is_fully_concrete(self.db) {
-                    // TODO(ilya): Try to detect the ambiguity earlier in the
-                    // inference process.
-                    return Ok(SolutionSet::Ambiguous(
-                        Ambiguity::NegativeImplWithUnresolvedGenericArgs {
-                            impl_id: ImplLongId::Concrete(concrete_impl).intern(self.db),
-                            ty,
-                        },
-                    ));
+                if !matches!(
+                    inference.trait_solution_set(concrete_trait_id, lookup_context.clone())?,
+                    SolutionSet::None
+                ) {
+                    // If a negative impl has an impl, then we should skip it.
+                    return Ok(SolutionSet::None);
                 }
             }
 
-            if !matches!(
-                self.trait_solution_set(concrete_trait_id, lookup_context.clone())?,
-                SolutionSet::None
-            ) {
-                // If a negative impl has an impl, then we should skip it.
-                return Ok(SolutionSet::None);
+            Ok(SolutionSet::Unique(canonical_impl))
+        }
+        match canonical_impl.0.lookup_intern(self.db) {
+            ImplLongId::Concrete(concrete_impl) => {
+                let mut rewriter = SubstitutionRewriter {
+                    db: self.db,
+                    substitution: &concrete_impl.substitution(self.db).map_err(|diag_added| {
+                        self.set_error(InferenceError::Reported(diag_added))
+                    })?,
+                };
+                let generic_params = self
+                    .db
+                    .impl_def_generic_params(concrete_impl.impl_def_id(self.db))
+                    .map_err(|diag_added| self.set_error(InferenceError::Reported(diag_added)))?;
+                let concrete_traits = generic_params
+                    .iter()
+                    .filter_map(|generic_param| {
+                        try_extract_matches!(generic_param, GenericParam::NegImpl)
+                    })
+                    .map(|generic_param| {
+                        rewriter
+                            .rewrite(*generic_param)
+                            .and_then(|generic_param| generic_param.concrete_trait)
+                    });
+                validate_no_solution_set(self, canonical_impl, lookup_context, concrete_traits)
             }
+            ImplLongId::GeneratedImpl(generated_impl) => validate_no_solution_set(
+                self,
+                canonical_impl,
+                lookup_context,
+                generated_impl
+                    .lookup_intern(self.db)
+                    .generic_params
+                    .iter()
+                    .filter_map(|generic_param| {
+                        try_extract_matches!(generic_param, GenericParam::NegImpl)
+                    })
+                    .map(|generic_param| generic_param.concrete_trait),
+            ),
+            ImplLongId::GenericParameter(_)
+            | ImplLongId::ImplVar(_)
+            | ImplLongId::ImplImpl(_)
+            | ImplLongId::TraitImpl(_) => Ok(SolutionSet::Unique(canonical_impl)),
         }
-
-        Ok(SolutionSet::Unique(canonical_impl))
     }
 
     // Error handling methods

crates/cairo-lang-semantic/src/expr/inference/infers.rs

@@ -144,14 +144,14 @@ impl InferenceEmbeddings for Inference<'_> {
             }
             UninferredImpl::GeneratedImpl(generated_impl) => {
                 let long_id = generated_impl.lookup_intern(self.db);
+
+                // Only making sure the args can be inferred - as they are unused later.
+                self.infer_generic_args(&long_id.generic_params[..], lookup_context, stable_ptr)?;
+
                 ImplLongId::GeneratedImpl(
                     GeneratedImplLongId {
                         concrete_trait: long_id.concrete_trait,
-                        generic_args: self.infer_generic_args(
-                            &long_id.generic_params[..],
-                            lookup_context,
-                            stable_ptr,
-                        )?,
+                        generic_params: long_id.generic_params,
                         impl_items: long_id.impl_items,
                     }
                     .intern(self.db),

crates/cairo-lang-semantic/src/expr/test_data/closure

@@ -626,3 +626,23 @@ error: Capture of mutable variables in a closure is not supported
  --> lib.cairo:5:9
         a + b
         ^
+
+//! > ==========================================================================
+
+//! > Not generating generating destruct impl when drop impl exists.
+
+//! > test_runner_name
+test_function_diagnostics(expect_diagnostics: false)
+
+//! > function
+fn foo() {
+    bar( || {});
+}
+
+//! > function_name
+foo
+
+//! > module_code
+fn bar<T, +Destruct<T>>(a: T) {}
+
+//! > expected_diagnostics

crates/cairo-lang-semantic/src/items/imp.rs

@@ -349,8 +349,9 @@ impl GeneratedImplId {
 #[derive(Clone, Debug, Hash, PartialEq, Eq, SemanticObject)]
 pub struct GeneratedImplLongId {
     pub concrete_trait: ConcreteTraitId,
-    /// The generic arguments used by the generated impl, typically impls and negative impls.
-    pub generic_args: Vec<GenericArgumentId>,
+    /// The generic params required for the impl. Typically impls and negative impls.
+    /// We save the params so that we can validate negative impls.
+    pub generic_params: Vec<GenericParam>,
     pub impl_items: GeneratedImplItems,
 }
 #[derive(Clone, Debug, Default, PartialEq, Eq, SemanticObject)]

crates/cairo-lang-semantic/src/items/tests/trait_impl

@@ -1239,6 +1239,7 @@ fn bar2<impl M: MyTrait<u32>>() {
 //! > expected_diagnostics
 
 //! > ==========================================================================
+
 //! > Inferring a Trait impl with generic param of a generic impl argument.
 
 //! > test_runner_name
@@ -1256,26 +1257,27 @@ trait InnerTrait<R> {
     fn foo() -> Self::X;
 }
 
-trait MyTrait<Y> {
-    impl MyImpl: InnerTrait<Y>;
+trait OuterTrait<Y> {
+    impl Inner: InnerTrait<Y>;
 }
-impl MyImpl<T, impl I: InnerTrait<T>> of MyTrait<T> {}
+impl OuterImpl<T, impl I: InnerTrait<T>> of OuterTrait<T> {}
 
 
-mod MyMod {
-    // impl of InnerTrait so MyImpl can be inferred.
-    impl MyIn<T> of super::InnerTrait<T> {
+mod my_mod {
+    // Implementing `InnerTrait` for all types.
+    impl MyInner<T> of super::InnerTrait<T> {
         type X = u16;
         fn foo() -> u16 {
             5
         }
     }
-    fn bar<F, impl A: super::MyTrait<F>>(_x: F) -> A::MyImpl::X {
-        A::MyImpl::foo()
+    fn requires_outer<T, impl O: super::OuterTrait<T>>(_x: T) -> O::Inner::X {
+        O::Inner::foo()
     }
+
     // inferring  MyTrait Impl from MyImpl with MyIn.
-    fn bar2() -> u16 {
-        bar(2_u32)
+    fn infers_outer_by_inner() -> u16 {
+        requires_outer(2_u32)
     }
 }