Skip to content
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

Argument Clinic duplicate module check is malfunctioning #107609

Closed
erlend-aasland opened this issue Aug 3, 2023 · 2 comments
Closed

Argument Clinic duplicate module check is malfunctioning #107609

erlend-aasland opened this issue Aug 3, 2023 · 2 comments
Labels
topic-argument-clinic type-bug An unexpected behavior, bug, or error

Comments

@erlend-aasland
Copy link
Contributor

erlend-aasland commented Aug 3, 2023

The following code is silently accepted by Argument Clinic:

/*[clinic input]
module m
module m
[clinic start generated code]*/

The duplicate module m should have been caught by Argument Clinic, but the guard is faulty:

cpython/Tools/clinic/clinic.py

Lines 4481 to 4482 in 9e6590b

if name in module.classes:
fail("Already defined module " + repr(name) + "!")

The check should be against .modules, not .classes:

diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py
index 1bcdb6b1c3..dc4a7f9318 100755
--- a/Tools/clinic/clinic.py
+++ b/Tools/clinic/clinic.py
@@ -4478,7 +4478,7 @@ def directive_module(self, name: str) -> None:
         if cls:
             fail("Can't nest a module inside a class!")
 
-        if name in module.classes:
+        if name in module.modules:
             fail("Already defined module " + repr(name) + "!")
 
         m = Module(name, module)

See also the Module class:

cpython/Tools/clinic/clinic.py

Lines 2384 to 2392 in 9e6590b

class Module:
name: str
module: Module | Clinic
def __post_init__(self) -> None:
self.parent = self.module
self.modules: ModuleDict = {}
self.classes: ClassDict = {}
self.functions: list[Function] = []

Linked PRs

@erlend-aasland erlend-aasland added type-bug An unexpected behavior, bug, or error topic-argument-clinic labels Aug 3, 2023
@erlend-aasland
Copy link
Contributor Author

Discovered while expanding the test suite to cover all directives and commands.

erlend-aasland added a commit to erlend-aasland/cpython that referenced this issue Aug 3, 2023
erlend-aasland added a commit that referenced this issue Aug 4, 2023
Also remove duplicate module def from _testcapi.
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Aug 4, 2023
…nGH-107610)

Also remove duplicate module def from _testcapi.
(cherry picked from commit a443c31)

Co-authored-by: Erlend E. Aasland <erlend@python.org>
@erlend-aasland
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-argument-clinic type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

1 participant