Skip to content

Commit c0acedd

Browse files
jhonabreulclaude
andcommitted
Use a miss-only __getattr__ hook for AttributeError suggestions
Move the suggestion logic for regular reflected types off the hot attribute-access path. Previously ClassObject overrode tp_getattro (__getattribute__), so every successful attribute access paid a managed round-trip (~17 ns/access measured). Instead, install a shared __getattr__ on each reflected type, which CPython only invokes on a miss via slot_tp_getattr_hook; hits go straight through the native generic getattr with no managed transition. pythonnet's metatype does not run CPython's slot-fixup when attributes are set on a type, so AttributeErrorHint wires tp_getattro to the hook manually (the hook address is read from a probe class). Only types still using the native generic getattr are redirected, so dynamic objects, modules and interfaces (which have their own tp_getattro) are untouched; DynamicClassObject keeps enriching on its own miss path. Benchmark (System.Version, best-of-N, ns/access): hit path: ~109 ns (baseline ~105; tp_getattro was ~122) miss path: ~11 us (rare; reflection + Levenshtein, as before) The hot-path overhead is effectively eliminated; the extra miss-path cost only applies when an attribute is actually absent. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent c9fabdc commit c0acedd

5 files changed

Lines changed: 182 additions & 26 deletions

File tree

src/runtime/AttributeErrorHint.cs

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
using System;
2+
3+
using Python.Runtime.Native;
4+
5+
namespace Python.Runtime
6+
{
7+
/// <summary>
8+
/// Installs a miss-only <c>__getattr__</c> hook on reflected .NET types so that an
9+
/// <c>AttributeError</c> raised for a missing attribute is enriched with suggestions
10+
/// of similarly-named members — without adding any cost to the (common) successful
11+
/// attribute-access path.
12+
/// </summary>
13+
/// <remarks>
14+
/// CPython only invokes <c>__getattr__</c> after the normal attribute lookup fails,
15+
/// via the native <c>slot_tp_getattr_hook</c>: on a hit it calls the generic getattr
16+
/// directly (no managed transition); only on a miss does it call our <c>__getattr__</c>.
17+
/// pythonnet's metatype does not run CPython's slot-fixup machinery when an attribute
18+
/// is set on a type, so simply adding <c>__getattr__</c> to the type dict would not
19+
/// rewire the slot — we therefore wire <c>tp_getattro</c> to the hook manually.
20+
/// </remarks>
21+
internal static class AttributeErrorHint
22+
{
23+
// The shared __getattr__ function object installed on every eligible type.
24+
private static PyObject? _getAttr;
25+
// The managed message builder exposed to Python, kept alive for _getAttr's globals.
26+
private static PyObject? _messageBuilder;
27+
// Address of CPython's slot_tp_getattr_hook (extracted from a probe type).
28+
private static IntPtr _hookSlot;
29+
// Address of PyObject_GenericGetAttr, used to detect types we may safely redirect.
30+
private static IntPtr _genericGetAttr;
31+
32+
private static bool IsReady => _getAttr is not null && _hookSlot != IntPtr.Zero;
33+
34+
internal static void Initialize()
35+
{
36+
try
37+
{
38+
_genericGetAttr = Util.ReadIntPtr(Runtime.PyBaseObjectType, TypeOffset.tp_getattro);
39+
40+
Func<PyObject, string, string> builder = ClassBase.BuildMissingAttributeMessage;
41+
_messageBuilder = builder.ToPython();
42+
43+
using var globals = new PyDict();
44+
Runtime.PyDict_SetItemString(globals.Reference, "__builtins__", Runtime.PyEval_GetBuiltins());
45+
globals["__clr_attr_msg__"] = _messageBuilder;
46+
47+
// Define the shared hook, plus a probe class whose tp_getattro is
48+
// slot_tp_getattr_hook so we can read that function pointer.
49+
PythonEngine.Exec(
50+
"def __clr_getattr__(self, name):\n" +
51+
" raise AttributeError(__clr_attr_msg__(self, name))\n" +
52+
"class __clr_getattr_probe__:\n" +
53+
" def __getattr__(self, name):\n" +
54+
" raise AttributeError(name)\n",
55+
globals);
56+
57+
_getAttr = globals["__clr_getattr__"];
58+
using var probe = globals["__clr_getattr_probe__"];
59+
_hookSlot = Util.ReadIntPtr(probe.Reference, TypeOffset.tp_getattro);
60+
}
61+
catch (Exception e)
62+
{
63+
// Degrade gracefully: without the hook, AttributeError messages are simply
64+
// not enriched. Never let this break interpreter initialization.
65+
DebugUtil.Print($"AttributeErrorHint.Initialize failed: {e}");
66+
Shutdown();
67+
}
68+
}
69+
70+
/// <summary>
71+
/// Wires the miss-only hook onto <paramref name="type"/> if it still uses the
72+
/// native generic getattr. Types with a custom <c>tp_getattro</c> (dynamic
73+
/// objects, modules, interfaces, ...) handle misses themselves and are left
74+
/// untouched; derived types that inherit an already-hooked base are likewise
75+
/// skipped, since they inherit the behavior through the MRO.
76+
/// </summary>
77+
internal static void Install(BorrowedReference type)
78+
{
79+
if (!IsReady)
80+
{
81+
return;
82+
}
83+
84+
if (Util.ReadIntPtr(type, TypeOffset.tp_getattro) != _genericGetAttr)
85+
{
86+
return;
87+
}
88+
89+
if (Runtime.PyObject_SetAttrString(type, "__getattr__", _getAttr!.Reference) != 0)
90+
{
91+
Exceptions.Clear();
92+
return;
93+
}
94+
95+
Util.WriteIntPtr(type, TypeOffset.tp_getattro, _hookSlot);
96+
Runtime.PyType_Modified(type);
97+
}
98+
99+
internal static void Shutdown()
100+
{
101+
_getAttr?.Dispose();
102+
_getAttr = null;
103+
_messageBuilder?.Dispose();
104+
_messageBuilder = null;
105+
_hookSlot = IntPtr.Zero;
106+
_genericGetAttr = IntPtr.Zero;
107+
}
108+
}
109+
}

src/runtime/PythonEngine.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,10 @@ public static void Initialize(IEnumerable<string> args, bool setSysArgv = true,
263263
}
264264

265265
ImportHook.UpdateCLRModuleDict();
266+
267+
// Set up the miss-only __getattr__ hook used to enrich AttributeError
268+
// messages on reflected .NET types with member-name suggestions.
269+
AttributeErrorHint.Initialize();
266270
}
267271

268272
static BorrowedReference DefineModule(string name)
@@ -369,6 +373,9 @@ public static void Shutdown()
369373
AppDomain.CurrentDomain.ProcessExit -= OnProcessExit;
370374

371375
ExecuteShutdownHandlers();
376+
377+
AttributeErrorHint.Shutdown();
378+
372379
// Remember to shut down the runtime.
373380
Runtime.Shutdown();
374381

src/runtime/TypeManager.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,10 @@ internal static void InitializeClass(PyType type, ClassBase impl, Type clrType)
303303

304304
Runtime.PyType_Modified(type.Reference);
305305

306+
// Enrich AttributeError messages for missing attributes with member-name
307+
// suggestions, via a miss-only __getattr__ hook (no hot-path cost).
308+
AttributeErrorHint.Install(type.Reference);
309+
306310
//DebugUtil.DumpType(type);
307311
}
308312

src/runtime/Types/ClassBase.cs

Lines changed: 62 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -628,20 +628,13 @@ internal static void AppendAttributeErrorSuggestions(BorrowedReference ob, Borro
628628
}
629629

630630
var name = Runtime.GetManagedString(key);
631-
// Skip empty and dunder names: the latter are probed internally by CPython
632-
// (e.g. __iter__, __len__) and are never user-facing typos worth helping with.
633-
if (string.IsNullOrEmpty(name) || name.StartsWith("__", StringComparison.Ordinal))
631+
if (string.IsNullOrEmpty(name))
634632
{
635633
return;
636634
}
637635

638-
if (GetManagedObject(ob) is not CLRObject clrObj || clrObj.inst is null)
639-
{
640-
return;
641-
}
642-
643-
var suggestions = GetSimilarMemberNames(clrObj.inst.GetType(), name);
644-
if (suggestions.Count == 0)
636+
var hint = GetSuggestionHint(ob, name);
637+
if (hint.Length == 0)
645638
{
646639
return;
647640
}
@@ -651,7 +644,6 @@ internal static void AppendAttributeErrorSuggestions(BorrowedReference ob, Borro
651644
try
652645
{
653646
var baseMessage = GetErrorMessage(errValue.BorrowNullable(), name);
654-
var hint = " Did you mean: " + string.Join(", ", suggestions.Select(s => $"'{s}'")) + "?";
655647
Exceptions.SetError(Exceptions.AttributeError, baseMessage + hint);
656648
}
657649
finally
@@ -662,6 +654,65 @@ internal static void AppendAttributeErrorSuggestions(BorrowedReference ob, Borro
662654
}
663655
}
664656

657+
/// <summary>
658+
/// Builds the full message for an <c>AttributeError</c> raised for a missing
659+
/// attribute on a .NET object, including any "Did you mean ...?" hint. Used by
660+
/// the miss-only <c>__getattr__</c> hook installed on reflected types (see
661+
/// <see cref="AttributeErrorHint"/>), where the original error has already been
662+
/// cleared, so the base message is reconstructed here.
663+
/// </summary>
664+
internal static string BuildMissingAttributeMessage(PyObject self, string name)
665+
{
666+
var typeName = "object";
667+
try
668+
{
669+
using var pyType = self.GetPythonType();
670+
typeName = pyType.Name;
671+
}
672+
catch
673+
{
674+
// fall back to the generic type name
675+
}
676+
677+
var message = $"'{typeName}' object has no attribute '{name}'";
678+
try
679+
{
680+
return message + GetSuggestionHint(self.Reference, name);
681+
}
682+
catch
683+
{
684+
// never let suggestion building turn into a different exception
685+
return message;
686+
}
687+
}
688+
689+
/// <summary>
690+
/// Returns " Did you mean: 'x', 'y'?" listing similarly-named members of the
691+
/// managed object, or an empty string when there is nothing to suggest. Dunder
692+
/// names are skipped: they are probed internally by CPython (e.g. __iter__,
693+
/// __len__) and are never user-facing typos worth helping with.
694+
/// </summary>
695+
private static string GetSuggestionHint(BorrowedReference ob, string name)
696+
{
697+
if (string.IsNullOrEmpty(name) || name.StartsWith("__", StringComparison.Ordinal))
698+
{
699+
return string.Empty;
700+
}
701+
702+
if (GetManagedObject(ob) is not CLRObject clrObj || clrObj.inst is null)
703+
{
704+
return string.Empty;
705+
}
706+
707+
var suggestions = GetSimilarMemberNames(clrObj.inst.GetType(), name);
708+
if (suggestions.Count == 0)
709+
{
710+
return string.Empty;
711+
}
712+
713+
return " Did you mean: " + string.Join(", ", suggestions.Select(s => $"'{s}'")) + "?";
714+
}
715+
665716
private static string GetErrorMessage(BorrowedReference value, string fallbackName)
666717
{
667718
if (value != null)

src/runtime/Types/ClassObject.cs

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -167,21 +167,6 @@ public override void InitializeSlots(BorrowedReference pyType, SlotsHolder slots
167167
protected virtual NewReference NewObjectToPython(object obj, BorrowedReference tp)
168168
=> CLRObject.GetReference(obj, tp);
169169

170-
/// <summary>
171-
/// Type __getattro__ implementation. Delegates to the generic CLR attribute
172-
/// lookup, but enriches the AttributeError raised for a missing attribute with
173-
/// suggestions of similarly-named members of the managed type.
174-
/// </summary>
175-
public static NewReference tp_getattro(BorrowedReference ob, BorrowedReference key)
176-
{
177-
var result = Runtime.PyObject_GenericGetAttr(ob, key);
178-
if (result.IsNull())
179-
{
180-
AppendAttributeErrorSuggestions(ob, key);
181-
}
182-
return result;
183-
}
184-
185170
private static NewReference NewEnum(Type type, BorrowedReference args, BorrowedReference tp)
186171
{
187172
nint argCount = Runtime.PyTuple_Size(args);

0 commit comments

Comments
 (0)