Skip to content

Commit 2ecb40f

Browse files
committed
Simplify circular dependency detection
- Used topological sort for all entries, not just constants - Removed checks during instantiation, relying on instantiation-time check only. - Updated tests and documentation.
1 parent d2d0a7a commit 2ecb40f

23 files changed

Lines changed: 335 additions & 417 deletions

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ Objects are instantiated only when accessed, using `__getattr__` to intercept at
2828
Strings like `{name}` in spec values are converted to `_WiredRef` markers during parsing, then resolved to actual objects at instantiation time. See `constants.py` for delimiter patterns.
2929

3030
**Thread Safety Model:**
31-
Uses optimistic per-attribute locking (`dict[str, threading.Lock]`) with a global fallback lock. Thread-local state tracks the resolving stack for circular dependency detection. See `threads.py` for `ThreadSafeMixin` implementation.
31+
Uses optimistic per-attribute locking (`dict[str, threading.Lock]`) with a global fallback lock. Thread-local state stores lock-related per-thread state (mode and held locks) used by the `ThreadSafeMixin`; see `threads.py` for implementation details.
3232

3333
**Compilation Equivalence:**
3434
Generated code must behave identically to runtime `Wiring`. The compiler generates accessor methods that replicate lazy loading, caching, and optional async/thread-safe behavior.

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ reuse:
1919
--license ISC \
2020
--recursive .
2121

22-
format: reuse
22+
format:
2323
uv run black apywire tests
2424
uv run isort apywire tests
2525

apywire/generator.py

Lines changed: 21 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,9 @@
2323
from apywire.constants import (
2424
PLACEHOLDER_END,
2525
PLACEHOLDER_START,
26-
SPEC_KEY_DELIMITER,
2726
)
28-
from apywire.exceptions import CircularWiringError
2927
from apywire.runtime import _Constructor
30-
from apywire.wiring import Spec, _ConstantValue, _SpecMapping
28+
from apywire.wiring import Spec, SpecParser, _ConstantValue, _SpecMapping
3129

3230
# Parameter sentinel values
3331
_PARAM_EMPTY: object = inspect.Parameter.empty # type: ignore[misc]
@@ -37,7 +35,7 @@
3735
_VAR_KEYWORD: object = inspect.Parameter.VAR_KEYWORD # type: ignore[misc]
3836

3937

40-
class Generator:
38+
class Generator(SpecParser):
4139
"""Generates apywire specs from class introspection.
4240
4341
This class inspects class constructors to automatically generate
@@ -73,7 +71,6 @@ def generate(cls, *entries: str) -> Spec:
7371
7472
Raises:
7573
ValueError: If entry format is invalid
76-
CircularWiringError: If circular dependencies detected
7774
7875
Example:
7976
>>> spec = Generator.generate("datetime.datetime now")
@@ -96,9 +93,13 @@ def _process_entry(cls, entry: str, spec: Spec, visited: set[str]) -> None:
9693
spec: Spec dict to populate
9794
visited: Set of already-visited class keys to detect cycles
9895
"""
99-
module_name, class_name, instance_name, factory_method = (
100-
cls._parse_entry(entry)
101-
)
96+
parsed = cls._parse_request_string(entry)
97+
if parsed is None:
98+
raise ValueError(
99+
f"Invalid entry format '{entry}': "
100+
f"expected 'module.Class name'"
101+
)
102+
module_name, class_name, instance_name, factory_method = parsed
102103

103104
# Build the spec key
104105
type_path = f"{module_name}.{class_name}"
@@ -108,9 +109,9 @@ def _process_entry(cls, entry: str, spec: Spec, visited: set[str]) -> None:
108109

109110
# Check for circular dependency
110111
if spec_key in visited:
111-
raise CircularWiringError(
112-
f"Circular dependency detected: '{spec_key}'"
113-
)
112+
# Cycle detected: stop recursion. The runtime container will
113+
# handle the cycle (or raise error) when resolving.
114+
return
114115
visited.add(spec_key)
115116

116117
# Import the class
@@ -136,8 +137,10 @@ def _process_entry(cls, entry: str, spec: Spec, visited: set[str]) -> None:
136137
else:
137138
spec[spec_key] = {}
138139
return
139-
except (ValueError, TypeError):
140-
# Some built-in classes don't have inspectable signatures
140+
except (ValueError, TypeError, RecursionError):
141+
# Some built-in classes (or pathological annotations) don't have
142+
# inspectable signatures; fall back to an empty entry to avoid
143+
# infinite recursion.
141144
spec[spec_key] = {}
142145
return
143146

@@ -174,60 +177,13 @@ def _process_entry(cls, entry: str, spec: Spec, visited: set[str]) -> None:
174177

175178
if not is_dependency:
176179
# For non-dependency params, set a constant value
177-
if has_default and cls._is_constant(param_default):
180+
if has_default and cls._is_spec_constant(param_default):
178181
spec[const_name] = cast(_ConstantValue, param_default)
179182
else:
180183
spec[const_name] = default_value
181184

182185
spec[spec_key] = cast(_SpecMapping, params)
183186

184-
@classmethod
185-
def _parse_entry(cls, entry: str) -> tuple[str, str, str, str | None]:
186-
"""Parse an entry string into components.
187-
188-
Args:
189-
entry: Entry string like "module.Class name" or
190-
"module.Class name.factoryMethod"
191-
192-
Returns:
193-
Tuple of (module_name, class_name, instance_name, factory_method)
194-
195-
Raises:
196-
ValueError: If entry format is invalid
197-
"""
198-
if SPEC_KEY_DELIMITER not in entry:
199-
raise ValueError(
200-
f"Invalid entry format '{entry}': "
201-
f"expected 'module.Class name'"
202-
)
203-
204-
type_str, name_part = entry.rsplit(SPEC_KEY_DELIMITER, 1)
205-
parts = type_str.split(".")
206-
207-
if len(parts) < 2:
208-
raise ValueError(
209-
f"Invalid entry format '{entry}': "
210-
f"missing module qualification"
211-
)
212-
213-
module_name = ".".join(parts[:-1])
214-
class_name = parts[-1]
215-
216-
# Check for factory method
217-
factory_method: str | None = None
218-
if "." in name_part:
219-
instance_name, factory_method = name_part.split(".", 1)
220-
else:
221-
instance_name = name_part
222-
223-
if factory_method and "." in factory_method:
224-
raise ValueError(
225-
f"invalid generator '{name_part}': nested factory methods "
226-
f"are not supported."
227-
)
228-
229-
return module_name, class_name, instance_name, factory_method
230-
231187
@classmethod
232188
def _get_default_for_type(
233189
cls,
@@ -338,33 +294,10 @@ def _generate_dependency(
338294

339295
# Recursively process (will add to spec)
340296
try:
341-
cls._process_entry(entry, spec, visited.copy())
342-
except (CircularWiringError, AttributeError, ModuleNotFoundError):
343-
# If circular or can't find module, just reference it
297+
# Propagate the same visited set so that cycles are detected
298+
cls._process_entry(entry, spec, visited)
299+
except (AttributeError, ModuleNotFoundError):
300+
# If can't find module, just reference it
344301
pass
345302

346303
return None, True
347-
348-
@classmethod
349-
def _is_constant(cls, value: object) -> bool:
350-
"""Check if a value is a valid constant for a spec.
351-
352-
Args:
353-
value: Value to check
354-
355-
Returns:
356-
True if value can be stored as a spec constant
357-
"""
358-
if value is None or value is ...:
359-
return True
360-
if isinstance(value, (str, bytes, bool, int, float, complex)):
361-
return True
362-
if isinstance(value, (list, tuple, dict)):
363-
# Check contents recursively
364-
if isinstance(value, dict):
365-
return all(
366-
cls._is_constant(k) and cls._is_constant(v)
367-
for k, v in value.items()
368-
)
369-
return all(cls._is_constant(v) for v in value)
370-
return False

apywire/runtime.py

Lines changed: 42 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
from apywire.constants import PLACEHOLDER_REGEX, SYNTHETIC_CONST
1818
from apywire.exceptions import (
19-
CircularWiringError,
2019
UnknownPlaceholderError,
2120
WiringError,
2221
)
@@ -94,12 +93,6 @@ def _init_thread_safety(
9493
self, max_lock_attempts, lock_retry_sleep
9594
)
9695

97-
def _get_resolving_stack(self) -> list[str]:
98-
"""Return the resolving stack for the current context."""
99-
if self._thread_safe:
100-
return ThreadSafeMixin._get_resolving_stack(self)
101-
return self._resolving_stack
102-
10396
def __getattr__(self, name: str) -> Accessor:
10497
"""Return a callable accessor for the named wired object."""
10598
# If the name is in our parsed spec or constants, return an accessor.
@@ -185,71 +178,54 @@ def _instantiate_impl(self, name: str) -> _RuntimeValue:
185178
"module.Class instance.from_date"), the factory method is called
186179
instead of the class constructor.
187180
"""
188-
# Check for circular dependencies
189-
stack = self._get_resolving_stack()
190-
if name in stack:
191-
raise CircularWiringError(
192-
f"Circular wiring dependency detected: "
193-
f"{' -> '.join(stack)} -> {name}"
181+
if name in self._values:
182+
return self._values[name]
183+
184+
if name not in self._parsed:
185+
# Should have been caught by __getattr__ or _resolve_runtime,
186+
# but just in case.
187+
raise UnknownPlaceholderError(
188+
f"Unknown placeholder '{name}' referenced."
194189
)
195190

196-
stack.append(name)
197-
try:
198-
if name in self._values:
199-
return self._values[name]
191+
entry = self._parsed[name]
200192

201-
if name not in self._parsed:
202-
# Should have been caught by __getattr__ or _resolve_runtime,
203-
# but just in case.
204-
raise UnknownPlaceholderError(
205-
f"Unknown placeholder '{name}' referenced."
206-
)
193+
# Check for synthetic auto-promoted constant
194+
if entry.module_name == SYNTHETIC_CONST and entry.class_name == "str":
195+
# This is an auto-promoted constant with string interpolation
196+
value = self._format_string_constant(entry.data, context=name)
197+
self._values[name] = value
198+
return value
207199

208-
entry = self._parsed[name]
209-
210-
# Check for synthetic auto-promoted constant
211-
if (
212-
entry.module_name == SYNTHETIC_CONST
213-
and entry.class_name == "str"
214-
):
215-
# This is an auto-promoted constant with string interpolation
216-
value = self._format_string_constant(entry.data, context=name)
217-
self._values[name] = value
218-
return value
219-
220-
module = importlib.import_module(entry.module_name)
221-
cls = cast(_Constructor, getattr(module, entry.class_name))
222-
223-
# If a factory method is specified, get it from the class
224-
if entry.factory_method:
225-
constructor = cast(
226-
_Constructor, getattr(cls, entry.factory_method)
227-
)
228-
else:
229-
constructor = cls
200+
module = importlib.import_module(entry.module_name)
201+
cls = cast(_Constructor, getattr(module, entry.class_name))
230202

231-
# Resolve arguments
232-
kwargs = self._resolve_runtime(entry.data, context=name)
203+
# If a factory method is specified, get it from the class
204+
if entry.factory_method:
205+
constructor = cast(
206+
_Constructor, getattr(cls, entry.factory_method)
207+
)
208+
else:
209+
constructor = cls
233210

234-
try:
235-
if isinstance(kwargs, dict):
236-
# Separate positional args (int keys) from keyword args
237-
# (str keys)
238-
pos_args, kwargs_dict = self._separate_args_kwargs(kwargs)
239-
instance = constructor(*pos_args, **kwargs_dict)
240-
elif isinstance(kwargs, list):
241-
# All positional arguments
242-
instance = constructor(*kwargs)
243-
else:
244-
instance = constructor(kwargs)
245-
except Exception as e:
246-
raise WiringError(
247-
f"failed to instantiate '{name}': {e}"
248-
) from e
249-
250-
return instance
251-
finally:
252-
stack.pop()
211+
# Resolve arguments
212+
kwargs = self._resolve_runtime(entry.data, context=name)
213+
214+
try:
215+
if isinstance(kwargs, dict):
216+
# Separate positional args (int keys) from keyword args
217+
# (str keys)
218+
pos_args, kwargs_dict = self._separate_args_kwargs(kwargs)
219+
instance = constructor(*pos_args, **kwargs_dict)
220+
elif isinstance(kwargs, list):
221+
# All positional arguments
222+
instance = constructor(*kwargs)
223+
else:
224+
instance = constructor(kwargs)
225+
except Exception as e:
226+
raise WiringError(f"failed to instantiate '{name}': {e}") from e
227+
228+
return instance
253229

254230
def _resolve_runtime(
255231
self,

apywire/threads.py

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,12 @@ class _ThreadLocalState(threading.local):
2323
"""Thread-local storage for wiring resolution state.
2424
2525
Provides typed attributes initialized per-thread:
26-
- resolving_stack: Stack for circular dependency detection
2726
- mode: Current instantiation mode ('optimistic', 'global', or None)
2827
- held_locks: Locks held by this thread
2928
"""
3029

3130
def __init__(self) -> None:
3231
super().__init__()
33-
self.resolving_stack: list[str] = []
3432
self.mode: Literal["optimistic", "global"] | None = None
3533
self.held_locks: list[threading.RLock] = []
3634

@@ -71,9 +69,6 @@ def _get_attribute_lock(self, name: str) -> threading.RLock:
7169
self._attr_locks[name] = threading.RLock()
7270
return self._attr_locks[name]
7371

74-
def _get_resolving_stack(self) -> list[str]:
75-
return self._local.resolving_stack
76-
7772
def _get_held_locks(self) -> list[threading.RLock]:
7873
return self._local.held_locks
7974

@@ -121,12 +116,11 @@ def _wrap_instantiation_error(self, e: Exception, name: str) -> NoReturn:
121116
This method always raises an exception and never returns.
122117
"""
123118
from apywire.exceptions import (
124-
CircularWiringError,
125119
UnknownPlaceholderError,
126120
WiringError,
127121
)
128122

129-
if isinstance(e, (UnknownPlaceholderError, CircularWiringError)):
123+
if isinstance(e, UnknownPlaceholderError):
130124
raise
131125
# All other exceptions (including WiringError) are wrapped
132126
raise WiringError(f"failed to instantiate '{name}'") from e

0 commit comments

Comments
 (0)