this repo has no description
fork

Configure Feed

Select the types of activity you want to include in your feed.

Remove DCHECK in icInvalidateAttr

This DCHECK is idealistic but the world does not yet match this
invariant. See #269 and #460 for more information, but the gist is that
we do not remove dependency links up the type hierarchy, so there are
some leftover links after eviction.

Add tests.

authored by bernsteinbear.com and committed by

Max Bernstein 339c8410 9cfc1387

+58 -5
+58
runtime/ic-test.cpp
··· 2178 2178 EXPECT_TRUE(x.hasFlag(Type::Flag::kHasObjectDunderGetattribute)); 2179 2179 } 2180 2180 2181 + TEST_F(IcTest, IcInvalidateTypeHierarchy) { 2182 + ASSERT_FALSE(runFromCStr(runtime_, R"( 2183 + class A: 2184 + def __init__(self): 2185 + self.foo = 400 2186 + 2187 + class B(A): 2188 + pass 2189 + 2190 + def cache_attribute(c): 2191 + return c.foo 2192 + 2193 + def invalidate(): 2194 + A.foo = property(lambda self: 123) 2195 + 2196 + a = A() 2197 + b = B() 2198 + a_init = A.__init__ 2199 + )") 2200 + .isError()); 2201 + HandleScope scope(thread_); 2202 + Function a_init(&scope, mainModuleAt(runtime_, "a_init")); 2203 + Function cache_attribute(&scope, mainModuleAt(runtime_, "cache_attribute")); 2204 + Type type_a(&scope, mainModuleAt(runtime_, "A")); 2205 + Type type_b(&scope, mainModuleAt(runtime_, "B")); 2206 + Object obj_a(&scope, mainModuleAt(runtime_, "a")); 2207 + Object obj_b(&scope, mainModuleAt(runtime_, "b")); 2208 + Object foo_name(&scope, Runtime::internStrFromCStr(thread_, "foo")); 2209 + ValueCell foo_in_a(&scope, typeValueCellAt(*type_a, *foo_name)); 2210 + ValueCell foo_in_b(&scope, typeValueCellAt(*type_b, *foo_name)); 2211 + 2212 + // We've called __init__ already so it should be a dependent. 2213 + EXPECT_TRUE(icDependentIncluded(*a_init, foo_in_a.dependencyLink())); 2214 + EXPECT_TRUE(icDependentIncluded(*a_init, foo_in_b.dependencyLink())); 2215 + EXPECT_FALSE(icDependentIncluded(*cache_attribute, foo_in_a.dependencyLink())); 2216 + EXPECT_FALSE(icDependentIncluded(*cache_attribute, foo_in_b.dependencyLink())); 2217 + 2218 + // Cache an attribute elsewhere. 2219 + ASSERT_TRUE(isIntEqualsWord(Interpreter::call1(thread_, cache_attribute, obj_a), 400)); 2220 + ASSERT_TRUE(isIntEqualsWord(Interpreter::call1(thread_, cache_attribute, obj_b), 400)); 2221 + 2222 + // That function should be a dependent too. 2223 + EXPECT_TRUE(icDependentIncluded(*a_init, foo_in_a.dependencyLink())); 2224 + EXPECT_TRUE(icDependentIncluded(*a_init, foo_in_b.dependencyLink())); 2225 + EXPECT_TRUE(icDependentIncluded(*cache_attribute, foo_in_a.dependencyLink())); 2226 + EXPECT_TRUE(icDependentIncluded(*cache_attribute, foo_in_b.dependencyLink())); 2227 + 2228 + // Invalidate the attribute. 2229 + Function invalidate(&scope, mainModuleAt(runtime_, "invalidate")); 2230 + ASSERT_TRUE(Interpreter::call0(thread_, invalidate).isNoneType()); 2231 + 2232 + EXPECT_FALSE(icDependentIncluded(*a_init, foo_in_a.dependencyLink())); 2233 + EXPECT_FALSE(icDependentIncluded(*a_init, foo_in_b.dependencyLink())); 2234 + // TODO(#269): Change to EXPECT_FALSE. 2235 + EXPECT_TRUE(icDependentIncluded(*cache_attribute, foo_in_a.dependencyLink())); 2236 + EXPECT_TRUE(icDependentIncluded(*cache_attribute, foo_in_b.dependencyLink())); 2237 + } 2238 + 2181 2239 } // namespace testing 2182 2240 } // namespace py
-5
runtime/ic.cpp
··· 681 681 link = WeakLink::cast(*link).next(); 682 682 icEvictCache(thread, dependent, type, attr_name, attribute_kind); 683 683 } 684 - // In case is_data_descriptor is true, we shouldn't see any dependents after 685 - // caching invalidation. 686 - DCHECK(attribute_kind == AttributeKind::kNotADataDescriptor || 687 - value_cell.dependencyLink().isNoneType(), 688 - "dependencyLink must be None if is_data_descriptor is true"); 689 684 } 690 685 691 686 RawSmallInt encodeBinaryOpKey(LayoutId left_layout_id, LayoutId right_layout_id,