From 0bca28d5a4fb3ddc471b478da4cc9e4646aa2060 Mon Sep 17 00:00:00 2001 From: Jakob Lykke Andersen Date: Sun, 11 Oct 2020 13:57:04 +0200 Subject: [PATCH 01/16] C, properly error on keywords as function parameters --- CHANGES | 2 ++ sphinx/domains/c.py | 3 +++ tests/test_domain_c.py | 3 +++ 3 files changed, 8 insertions(+) diff --git a/CHANGES b/CHANGES index f9f267c3a90..0e03a3f0a13 100644 --- a/CHANGES +++ b/CHANGES @@ -77,6 +77,8 @@ Bugs fixed with size explicitly set in pixels) (fixed for ``'pdflatex'/'lualatex'`` only) * #8911: C++: remove the longest matching prefix in :confval:`cpp_index_common_prefix` instead of the first that matches. +* C, properly reject function declarations when a keyword is used + as parameter name. Testing -------- diff --git a/sphinx/domains/c.py b/sphinx/domains/c.py index 061010d6651..4909844cbd1 100644 --- a/sphinx/domains/c.py +++ b/sphinx/domains/c.py @@ -2703,6 +2703,9 @@ def _parse_declarator_name_suffix( declId = None elif named == 'single': if self.match(identifier_re): + if self.matched_text in _keywords: + self.fail("Expected identifier, " + "got keyword: %s" % self.matched_text) identifier = ASTIdentifier(self.matched_text) declId = ASTNestedName([identifier], rooted=False) else: diff --git a/tests/test_domain_c.py b/tests/test_domain_c.py index 2cfcf74faf4..999c99f4d8c 100644 --- a/tests/test_domain_c.py +++ b/tests/test_domain_c.py @@ -417,6 +417,9 @@ def test_function_definitions(): check('function', 'void f(int arr[const static volatile 42])', {1: 'f'}, output='void f(int arr[static volatile const 42])') + with pytest.raises(DefinitionError): + parse('function', 'void f(int for)') + def test_nested_name(): check('struct', '{key}.A', {1: "A"}) From fb7303929a185def561b6dd0c0d3d61096a75473 Mon Sep 17 00:00:00 2001 From: Jakob Lykke Andersen Date: Sun, 11 Oct 2020 14:05:07 +0200 Subject: [PATCH 02/16] C, remove dead code --- sphinx/domains/c.py | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/sphinx/domains/c.py b/sphinx/domains/c.py index 4909844cbd1..3693e536ace 100644 --- a/sphinx/domains/c.py +++ b/sphinx/domains/c.py @@ -2693,15 +2693,9 @@ def _parse_decl_specs(self, outer: str, typed: bool = True) -> ASTDeclSpecs: def _parse_declarator_name_suffix( self, named: Union[bool, str], paramMode: str, typed: bool ) -> ASTDeclarator: + assert named in (True, False, 'single') # now we should parse the name, and then suffixes - if named == 'maybe': - pos = self.pos - try: - declId = self._parse_nested_name() - except DefinitionError: - self.pos = pos - declId = None - elif named == 'single': + if named == 'single': if self.match(identifier_re): if self.matched_text in _keywords: self.fail("Expected identifier, " @@ -2883,8 +2877,8 @@ def parser(): def _parse_type(self, named: Union[bool, str], outer: str = None) -> ASTType: """ - named=False|'maybe'|True: 'maybe' is e.g., for function objects which - doesn't need to name the arguments + named=False|'single'|True: 'single' is e.g., for function objects which + doesn't need to name the arguments, but otherwise is a single name """ if outer: # always named if outer not in ('type', 'member', 'function'): From 69d1098438de8e02734c31dc5b922971c2acabe0 Mon Sep 17 00:00:00 2001 From: Jakob Lykke Andersen Date: Sun, 11 Oct 2020 15:17:32 +0200 Subject: [PATCH 03/16] C, remove more dead code --- sphinx/domains/c.py | 26 +------------------------- 1 file changed, 1 insertion(+), 25 deletions(-) diff --git a/sphinx/domains/c.py b/sphinx/domains/c.py index 3693e536ace..79d4e145c6b 100644 --- a/sphinx/domains/c.py +++ b/sphinx/domains/c.py @@ -387,19 +387,6 @@ def describe_signature(self, signode: TextElement, mode: str, signode.append(nodes.Text('--')) -class ASTPostfixMember(ASTPostfixOp): - def __init__(self, name): - self.name = name - - def _stringify(self, transform: StringifyTransform) -> str: - return '.' + transform(self.name) - - def describe_signature(self, signode: TextElement, mode: str, - env: "BuildEnvironment", symbol: "Symbol") -> None: - signode.append(nodes.Text('.')) - self.name.describe_signature(signode, 'noneIsName', env, symbol) - - class ASTPostfixMemberOfPointer(ASTPostfixOp): def __init__(self, name): self.name = name @@ -2256,7 +2243,7 @@ def _parse_postfix_expression(self) -> ASTPostfixExpr: # | postfix "[" expression "]" # | postfix "[" braced-init-list [opt] "]" # | postfix "(" expression-list [opt] ")" - # | postfix "." id-expression + # | postfix "." id-expression // taken care of in primary by nested name # | postfix "->" id-expression # | postfix "++" # | postfix "--" @@ -2274,17 +2261,6 @@ def _parse_postfix_expression(self) -> ASTPostfixExpr: self.fail("Expected ']' in end of postfix expression.") postFixes.append(ASTPostfixArray(expr)) continue - if self.skip_string('.'): - if self.skip_string('*'): - # don't steal the dot - self.pos -= 2 - elif self.skip_string('..'): - # don't steal the dot - self.pos -= 3 - else: - name = self._parse_nested_name() - postFixes.append(ASTPostfixMember(name)) - continue if self.skip_string('->'): if self.skip_string('*'): # don't steal the arrow From 30eede5bc5036f3479879ebc8b5c4119ec31617a Mon Sep 17 00:00:00 2001 From: Jakob Lykke Andersen Date: Sun, 11 Oct 2020 15:23:11 +0200 Subject: [PATCH 04/16] C, simplify tests --- tests/roots/test-domain-c/semicolon.rst | 10 --------- tests/test_domain_c.py | 29 ++++++++++++++++++++----- 2 files changed, 24 insertions(+), 15 deletions(-) delete mode 100644 tests/roots/test-domain-c/semicolon.rst diff --git a/tests/roots/test-domain-c/semicolon.rst b/tests/roots/test-domain-c/semicolon.rst deleted file mode 100644 index 14ba177569d..00000000000 --- a/tests/roots/test-domain-c/semicolon.rst +++ /dev/null @@ -1,10 +0,0 @@ -.. c:member:: int member; -.. c:var:: int var; -.. c:function:: void f(); -.. .. c:macro:: NO_SEMICOLON; -.. c:struct:: Struct; -.. c:union:: Union; -.. c:enum:: Enum; -.. c:enumerator:: Enumerator; -.. c:type:: Type; -.. c:type:: int TypeDef; diff --git a/tests/test_domain_c.py b/tests/test_domain_c.py index 999c99f4d8c..56688a3b31f 100644 --- a/tests/test_domain_c.py +++ b/tests/test_domain_c.py @@ -526,8 +526,15 @@ def test_attributes(): # raise DefinitionError("") +def split_warnigns(warning): + ws = warning.getvalue().split("\n") + assert len(ws) >= 1 + assert ws[-1] == "" + return ws[:-1] + + def filter_warnings(warning, file): - lines = warning.getvalue().split("\n") + lines = split_warnigns(warning) res = [l for l in lines if "domain-c" in l and "{}.rst".format(file) in l and "WARNING: document isn't included in any toctree" not in l] print("Filtered warnings for file '{}':".format(file)) @@ -581,10 +588,22 @@ def test_build_domain_c_anon_dup_decl(app, status, warning): assert "WARNING: c:identifier reference target not found: @b" in ws[1] -@pytest.mark.sphinx(testroot='domain-c', confoverrides={'nitpicky': True}) -def test_build_domain_c_semicolon(app, status, warning): - app.builder.build_all() - ws = filter_warnings(warning, "semicolon") +@pytest.mark.sphinx(confoverrides={'nitpicky': True}) +def test_build_domain_c_semicolon(app, warning): + text = """ +.. c:member:: int member; +.. c:var:: int var; +.. c:function:: void f(); +.. .. c:macro:: NO_SEMICOLON; +.. c:struct:: Struct; +.. c:union:: Union; +.. c:enum:: Enum; +.. c:enumerator:: Enumerator; +.. c:type:: Type; +.. c:type:: int TypeDef; +""" + restructuredtext.parse(app, text) + ws = split_warnigns(warning) assert len(ws) == 0 From 636be706dae25a10bb6dfc7a36ab067233054314 Mon Sep 17 00:00:00 2001 From: Jakob Lykke Andersen Date: Sun, 21 Feb 2021 14:49:01 +0100 Subject: [PATCH 05/16] C, add base test for ids-vs-tags --- tests/roots/test-domain-c/ids-vs-tags0.rst | 77 ++++++++++++++++++++++ tests/test_domain_c.py | 6 +- 2 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 tests/roots/test-domain-c/ids-vs-tags0.rst diff --git a/tests/roots/test-domain-c/ids-vs-tags0.rst b/tests/roots/test-domain-c/ids-vs-tags0.rst new file mode 100644 index 00000000000..20f52d8f202 --- /dev/null +++ b/tests/roots/test-domain-c/ids-vs-tags0.rst @@ -0,0 +1,77 @@ +.. c:member:: int _member +.. c:var:: int _var +.. c:function:: void _function() +.. c:macro:: _macro +.. c:struct:: _struct +.. c:union:: _union +.. c:enum:: _enum + + .. c:enumerator:: _enumerator + +.. c:type:: _type +.. c:function:: void _functionParam(int param) + + +.. c:member:: void __member = _member + + - :any:`_member` + - :c:member:`_member` + - :c:var:`_member` + - :c:data:`_member` + +.. c:member:: void __var = _var + + - :any:`_var` + - :c:member:`_var` + - :c:var:`_var` + - :c:data:`_var` + +.. c:member:: void __function = _function + + - :any:`_function` + - :c:func:`_function` + - :c:type:`_function` + +.. c:member:: void __macro = _macro + + - :any:`_macro` + - :c:macro:`_macro` + +.. c:type:: _struct __struct + struct _struct __structTagged + + - :any:`_struct` + - :c:struct:`_struct` + - :c:type:`_struct` + +.. c:type:: _union __union + union _union __unionTagged + + - :any:`_union` + - :c:union:`_union` + - :c:type:`_union` + +.. c:type:: _enum __enum + enum _enum __enumTagged + + - :any:`_enum` + - :c:enum:`_enum` + - :c:type:`_enum` + +.. c:member:: void __enumerator = _enumerator + + - :any:`_enumerator` + - :c:enumerator:`_enumerator` + +.. c:type:: _type __type + + - :any:`_type` + - :c:type:`_type` + +.. c:member:: void __functionParam = _functionParam.param + + - :any:`_functionParam.param` + - :c:member:`_functionParam.param` + - :c:var:`_functionParam.param` + - :c:data:`_functionParam.param` + diff --git a/tests/test_domain_c.py b/tests/test_domain_c.py index 56688a3b31f..27351fbd8c1 100644 --- a/tests/test_domain_c.py +++ b/tests/test_domain_c.py @@ -587,6 +587,11 @@ def test_build_domain_c_anon_dup_decl(app, status, warning): assert "WARNING: c:identifier reference target not found: @a" in ws[0] assert "WARNING: c:identifier reference target not found: @b" in ws[1] +@pytest.mark.sphinx(testroot='domain-c', confoverrides={'nitpicky': True}) +def test_ids_vs_tags0(app, status, warning): + app.builder.build_all() + ws = filter_warnings(warning, "ids-vs-tags0") + assert len(ws) == 0 @pytest.mark.sphinx(confoverrides={'nitpicky': True}) def test_build_domain_c_semicolon(app, warning): @@ -606,7 +611,6 @@ def test_build_domain_c_semicolon(app, warning): ws = split_warnigns(warning) assert len(ws) == 0 - @pytest.mark.sphinx(testroot='domain-c', confoverrides={'nitpicky': True}) def test_build_function_param_target(app, warning): # the anchor for function parameters should be the function From 0eb2a1b2e18a07b8d5bc881c5e93a4b950361096 Mon Sep 17 00:00:00 2001 From: Jakob Lykke Andersen Date: Mon, 12 Oct 2020 14:30:44 +0200 Subject: [PATCH 06/16] C, initial introduction of tags --- sphinx/domains/c.py | 160 ++++++++++++--------- tests/roots/test-domain-c/ids-vs-tags1.rst | 10 ++ tests/test_build_html.py | 10 +- tests/test_domain_c.py | 108 +++++++++----- 4 files changed, 182 insertions(+), 106 deletions(-) create mode 100644 tests/roots/test-domain-c/ids-vs-tags1.rst diff --git a/sphinx/domains/c.py b/sphinx/domains/c.py index 79d4e145c6b..7c8c874ed66 100644 --- a/sphinx/domains/c.py +++ b/sphinx/domains/c.py @@ -9,8 +9,8 @@ """ import re -from typing import (Any, Callable, Dict, Generator, Iterator, List, Tuple, Type, TypeVar, - Union, cast) +from typing import (Any, Callable, Dict, Generator, Iterator, List, Optional, Tuple, Type, + TypeVar, Union, cast) from docutils import nodes from docutils.nodes import Element, Node, TextElement, system_message @@ -78,8 +78,8 @@ _expression_assignment_ops = ["=", "*=", "/=", "%=", "+=", "-=", ">>=", "<<=", "&=", "and_eq", "^=", "xor_eq", "|=", "or_eq"] -_max_id = 1 -_id_prefix = [None, 'c.', 'Cv2.'] +_max_id = 2 +_id_prefix = [None, 'c.', 'C2-'] # Ids are used in lookup keys which are used across pickled files, # so when _max_id changes, make sure to update the ENV_VERSION. @@ -108,31 +108,54 @@ def describe_signature(self, signode: TextElement, mode: str, ################################################################################ class ASTIdentifier(ASTBaseBase): - def __init__(self, identifier: str) -> None: + def __init__(self, identifier: str, tag: Optional[str]) -> None: + # tag: + # - len > 0: it's a struct/union/enum + # - len == 0: it's not a struct/union/enum + # - None: for matching, can be either assert identifier is not None assert len(identifier) != 0 self.identifier = identifier + self.tag = tag def __eq__(self, other: Any) -> bool: - return type(other) is ASTIdentifier and self.identifier == other.identifier + return type(other) is ASTIdentifier \ + and self.identifier == other.identifier \ + and self.tag == other.tag def is_anon(self) -> bool: return self.identifier[0] == '@' + def get_id(self, version: int) -> str: + if version <= 1: + return self.identifier + if self.tag: + assert len(self.tag) != 0 + return '-' + self.identifier + else: + return self.identifier + # and this is where we finally make a difference between __str__ and the display string def __str__(self) -> str: - return self.identifier + return self.tag + " " + self.identifier if self.tag else self.identifier def get_display_string(self) -> str: - return "[anonymous]" if self.is_anon() else self.identifier + id = "[anonymous]" if self.is_anon() else self.identifier + return self.tag + " " + id if self.tag else id def describe_signature(self, signode: TextElement, mode: str, env: "BuildEnvironment", prefix: str, symbol: "Symbol") -> None: # note: slightly different signature of describe_signature due to the prefix verify_description_mode(mode) + if self.tag: + signode += nodes.Text(self.tag) + signode += nodes.Text(' ') if mode == 'markType': - targetText = prefix + self.identifier + if self.tag: + targetText = prefix + self.tag + ' ' + self.identifier + else: + targetText = prefix + self.identifier pnode = addnodes.pending_xref('', refdomain='c', reftype='identifier', reftarget=targetText, modname=None, @@ -163,12 +186,17 @@ def __init__(self, names: List[ASTIdentifier], rooted: bool) -> None: self.names = names self.rooted = rooted + def setTagsToPattern(self) -> None: + for n in self.names: + if n.tag == '': + n.tag = None + @property def name(self) -> "ASTNestedName": return self def get_id(self, version: int) -> str: - return '.'.join(str(n) for n in self.names) + return '.'.join(n.get_id(version) for n in self.names) def _stringify(self, transform: StringifyTransform) -> str: res = '.'.join(transform(n) for n in self.names) @@ -594,8 +622,7 @@ def describe_signature(self, signode: TextElement, mode: str, class ASTTrailingTypeSpecName(ASTTrailingTypeSpec): - def __init__(self, prefix: str, nestedName: ASTNestedName) -> None: - self.prefix = prefix + def __init__(self, nestedName: ASTNestedName) -> None: self.nestedName = nestedName @property @@ -603,18 +630,10 @@ def name(self) -> ASTNestedName: return self.nestedName def _stringify(self, transform: StringifyTransform) -> str: - res = [] - if self.prefix: - res.append(self.prefix) - res.append(' ') - res.append(transform(self.nestedName)) - return ''.join(res) + return transform(self.nestedName) def describe_signature(self, signode: TextElement, mode: str, env: "BuildEnvironment", symbol: "Symbol") -> None: - if self.prefix: - signode += addnodes.desc_annotation(self.prefix, self.prefix) - signode += nodes.Text(' ') self.nestedName.describe_signature(signode, mode, env, symbol=symbol) @@ -1862,23 +1881,9 @@ def handleDuplicateDeclaration(symbol: "Symbol", candSymbol: "Symbol") -> None: candSymbol.isRedeclaration = True raise _DuplicateSymbolError(symbol, declaration) - if declaration.objectType != "function": - assert len(withDecl) <= 1 - handleDuplicateDeclaration(withDecl[0], candSymbol) - # (not reachable) - - # a function, so compare IDs - candId = declaration.get_newest_id() - if Symbol.debug_lookup: - Symbol.debug_print("candId:", candId) - for symbol in withDecl: - oldId = symbol.declaration.get_newest_id() - if Symbol.debug_lookup: - Symbol.debug_print("oldId: ", oldId) - if candId == oldId: - handleDuplicateDeclaration(symbol, candSymbol) - # (not reachable) - # no candidate symbol found with matching ID + assert len(withDecl) <= 1 + handleDuplicateDeclaration(withDecl[0], candSymbol) + # (not reachable) # if there is an empty symbol, fill that one if len(noDecl) == 0: if Symbol.debug_lookup: @@ -1960,8 +1965,12 @@ def add_declaration(self, declaration: ASTDeclaration, Symbol.debug_print("add_declaration:") assert declaration is not None assert docname is not None - assert line is not None - nestedName = declaration.name + assert declaration.name.names[-1].tag == '' + if declaration.objectType in ('struct', 'union', 'enum'): + nestedName = declaration.name.clone() + nestedName.names[-1].tag = declaration.objectType + else: + nestedName = declaration.name res = self._add_symbols(nestedName, declaration, docname, line) if Symbol.debug_lookup: Symbol.debug_indent -= 1 @@ -2089,8 +2098,6 @@ class DefinitionParser(BaseParser): '__int64', ) - _prefix_keys = ('struct', 'enum', 'union') - @property def language(self) -> str: return 'C' @@ -2182,10 +2189,9 @@ def _parse_primary_expression(self) -> ASTExpression: res = self._parse_paren_expression() if res is not None: return res - nn = self._parse_nested_name() - if nn is not None: - return ASTIdExpression(nn) - return None + nn = self._parse_nested_name(allowTags=None) + assert nn is not None + return ASTIdExpression(nn) def _parse_initializer_list(self, name: str, open: str, close: str ) -> Tuple[List[ASTExpression], bool]: @@ -2266,7 +2272,7 @@ def _parse_postfix_expression(self) -> ASTPostfixExpr: # don't steal the arrow self.pos -= 3 else: - name = self._parse_nested_name() + name = self._parse_nested_name(allowTags=None) postFixes.append(ASTPostfixMemberOfPointer(name)) continue if self.skip_string('++'): @@ -2484,15 +2490,26 @@ def _parse_expression_fallback( value = self.definition[startPos:self.pos].strip() return ASTFallbackExpr(value.strip()) - def _parse_nested_name(self) -> ASTNestedName: - names = [] # type: List[Any] + def _parse_nested_name(self, *, allowTags: Optional[str] = 'not last') -> ASTNestedName: + # A dot-separated list of identifiers, each with an optional struct/union/enum tag. + # A leading dot makes the name rooted at global scope. + + assert allowTags in (None, 'all', 'not last') + + names = [] # type: List[ASTIdentifier] - self.skip_ws() rooted = False if self.skip_string('.'): rooted = True while 1: self.skip_ws() + tag = '' + if allowTags is not None: + for k in ('struct', 'union', 'enum'): + if self.skip_word_and_ws(k): + tag = k + break + afterTagPos = self.pos if not self.match(identifier_re): self.fail("Expected identifier in nested name.") identifier = self.matched_text @@ -2500,13 +2517,19 @@ def _parse_nested_name(self) -> ASTNestedName: if identifier in _keywords: self.fail("Expected identifier in nested name, " "got keyword: %s" % identifier) - ident = ASTIdentifier(identifier) + ident = ASTIdentifier(identifier, tag) names.append(ident) self.skip_ws() if not self.skip_string('.'): break - return ASTNestedName(names, rooted) + + # post hax to explicitly forbid the last one to have a tag + if allowTags == 'not last' and names[-1].tag != '': + self.pos = afterTagPos + self.fail("Expected identifier in nested name, " + "got keyword: %s" % names[-1].tag) + return ASTNestedName(names, rooted=rooted) def _parse_trailing_type_spec(self) -> ASTTrailingTypeSpec: # fundamental types @@ -2539,16 +2562,8 @@ def _parse_trailing_type_spec(self) -> ASTTrailingTypeSpec: if len(elements) > 0: return ASTTrailingTypeSpecFundamental(' '.join(elements)) - # prefixed - prefix = None - self.skip_ws() - for k in self._prefix_keys: - if self.skip_word_and_ws(k): - prefix = k - break - - nestedName = self._parse_nested_name() - return ASTTrailingTypeSpecName(prefix, nestedName) + nestedName = self._parse_nested_name(allowTags='all') + return ASTTrailingTypeSpecName(nestedName) def _parse_parameters(self, paramMode: str) -> ASTParameters: self.skip_ws() @@ -2676,7 +2691,7 @@ def _parse_declarator_name_suffix( if self.matched_text in _keywords: self.fail("Expected identifier, " "got keyword: %s" % self.matched_text) - identifier = ASTIdentifier(self.matched_text) + identifier = ASTIdentifier(self.matched_text, '') declId = ASTNestedName([identifier], rooted=False) else: declId = None @@ -2938,7 +2953,7 @@ def _parse_macro(self) -> ASTMacro: break if not self.match(identifier_re): self.fail("Expected identifier in macro parameters.") - nn = ASTNestedName([ASTIdentifier(self.matched_text)], rooted=False) + nn = ASTNestedName([ASTIdentifier(self.matched_text, '')], rooted=False) # Allow named variadic args: # https://gcc.gnu.org/onlinedocs/cpp/Variadic-Macros.html self.skip_ws() @@ -3035,10 +3050,10 @@ def parse_declaration(self, objectType: str, directiveType: str) -> ASTDeclarati return ASTDeclaration(objectType, directiveType, declaration, semicolon) def parse_namespace_object(self) -> ASTNestedName: - return self._parse_nested_name() + return self._parse_nested_name(allowTags='all') def parse_xref_object(self) -> ASTNestedName: - name = self._parse_nested_name() + name = self._parse_nested_name(allowTags='all') # if there are '()' left, just skip them self.skip_ws() self.skip_string('()') @@ -3068,7 +3083,7 @@ def parse_expression(self) -> Union[ASTExpression, ASTType]: def _make_phony_error_name() -> ASTNestedName: - return ASTNestedName([ASTIdentifier("PhonyNameDueToError")], rooted=False) + return ASTNestedName([ASTIdentifier("PhonyNameDueToError", None)], rooted=False) class CObject(ObjectDescription[ASTDeclaration]): @@ -3849,9 +3864,18 @@ def setup(app: Sphinx) -> Dict[str, Any]: app.add_config_value("c_allow_pre_v3", False, 'env') app.add_config_value("c_warn_on_allowed_pre_v3", True, 'env') + # debug stuff + app.add_config_value("c_debug_lookup", False, '') + app.add_config_value("c_debug_show_tree", False, '') + + def setDebugFlags(app): + Symbol.debug_lookup = app.config.c_debug_lookup + Symbol.debug_show_tree = app.config.c_debug_show_tree + app.connect("builder-inited", setDebugFlags) + return { 'version': 'builtin', - 'env_version': 2, + 'env_version': 3, 'parallel_read_safe': True, 'parallel_write_safe': True, } diff --git a/tests/roots/test-domain-c/ids-vs-tags1.rst b/tests/roots/test-domain-c/ids-vs-tags1.rst new file mode 100644 index 00000000000..7778436e6b6 --- /dev/null +++ b/tests/roots/test-domain-c/ids-vs-tags1.rst @@ -0,0 +1,10 @@ +.. c:struct:: f_struct +.. c:type:: struct f_struct f_struct +.. c:union:: f_union +.. c:type:: struct f_union f_union +.. c:enum:: f_enum +.. c:type:: struct f_enum f_enum + +- :c:struct:`f_struct`, :c:type:`f_struct` +- :c:union:`f_union`, :c:type:`f_union` +- :c:enum:`f_enum`, :c:type:`f_enum` diff --git a/tests/test_build_html.py b/tests/test_build_html.py index fccf9cc6aa8..deb978b13c1 100644 --- a/tests/test_build_html.py +++ b/tests/test_build_html.py @@ -293,11 +293,11 @@ def test_html4_output(app, status, warning): (".//a[@class='reference internal'][@href='#errmod.Error']/strong", 'Error'), # C references (".//span[@class='pre']", 'CFunction()'), - (".//a[@href='#c.Sphinx_DoSomething']", ''), - (".//a[@href='#c.SphinxStruct.member']", ''), - (".//a[@href='#c.SPHINX_USE_PYTHON']", ''), - (".//a[@href='#c.SphinxType']", ''), - (".//a[@href='#c.sphinx_global']", ''), + (".//a[@href='#C2-Sphinx_DoSomething']", ''), + (".//a[@href='#C2-SphinxStruct.member']", ''), + (".//a[@href='#C2-SPHINX_USE_PYTHON']", ''), + (".//a[@href='#C2-SphinxType']", ''), + (".//a[@href='#C2-sphinx_global']", ''), # test global TOC created by toctree() (".//ul[@class='current']/li[@class='toctree-l1 current']/a[@href='#']", 'Testing object descriptions'), diff --git a/tests/test_domain_c.py b/tests/test_domain_c.py index 27351fbd8c1..635ae942ce7 100644 --- a/tests/test_domain_c.py +++ b/tests/test_domain_c.py @@ -32,6 +32,21 @@ class Config: return ast +def parse_expression(expr, allowTypeExpr=False): + class Config: + c_id_attributes = ["id_attr"] + c_paren_attributes = ["paren_attr"] + parser = DefinitionParser(expr, location=None, config=Config()) + parser.allowFallbackExpressionParsing = False + if allowTypeExpr: + ast = parser.parse_expression() + else: + ast = parser._parse_expression() + parser.skip_ws() + parser.assert_end() + return ast + + def _check(name, input, idDict, output, key, asTextOutput): if key is None: key = name @@ -113,14 +128,8 @@ def check(name, input, idDict, output=None, key=None, asTextOutput=None): def test_expressions(): - def exprCheck(expr, output=None): - class Config: - c_id_attributes = ["id_attr"] - c_paren_attributes = ["paren_attr"] - parser = DefinitionParser(expr, location=None, config=Config()) - parser.allowFallbackExpressionParsing = False - ast = parser.parse_expression() - parser.assert_end() + def exprCheck(expr, output=None, allowTypeExpr=False): + ast = parse_expression(expr, allowTypeExpr) # first a simple check of the AST if output is None: output = expr @@ -141,14 +150,14 @@ class Config: raise DefinitionError("") # type expressions - exprCheck('int*') - exprCheck('int *const*') - exprCheck('int *volatile*') - exprCheck('int *restrict*') - exprCheck('int *(*)(double)') - exprCheck('const int*') - exprCheck('__int64') - exprCheck('unsigned __int64') + exprCheck('int*', allowTypeExpr=True) + exprCheck('int *const*', allowTypeExpr=True) + exprCheck('int *volatile*', allowTypeExpr=True) + exprCheck('int *restrict*', allowTypeExpr=True) + exprCheck('int *(*)(double)', allowTypeExpr=True) + exprCheck('const int*', allowTypeExpr=True) + exprCheck('__int64', allowTypeExpr=True) + exprCheck('unsigned __int64', allowTypeExpr=True) # actual expressions @@ -422,33 +431,60 @@ def test_function_definitions(): def test_nested_name(): - check('struct', '{key}.A', {1: "A"}) - check('struct', '{key}.A.B', {1: "A.B"}) + check('struct', '{key}.A', {1: "A", 2: "-A"}) + check('struct', '{key}.A.B', {1: "A.B", 2: "A.-B"}) check('function', 'void f(.A a)', {1: "f"}) check('function', 'void f(.A.B a)', {1: "f"}) def test_struct_definitions(): - check('struct', '{key}A', {1: 'A'}) + check('struct', '{key}A', {1: 'A', 2: '-A'}) def test_union_definitions(): - check('union', '{key}A', {1: 'A'}) + check('union', '{key}A', {1: 'A', 2: '-A'}) def test_enum_definitions(): - check('enum', '{key}A', {1: 'A'}) + check('enum', '{key}A', {1: 'A', 2: '-A'}) check('enumerator', '{key}A', {1: 'A'}) check('enumerator', '{key}A = 42', {1: 'A'}) def test_anon_definitions(): - check('struct', '@a', {1: "@a"}, asTextOutput='struct [anonymous]') - check('union', '@a', {1: "@a"}, asTextOutput='union [anonymous]') - check('enum', '@a', {1: "@a"}, asTextOutput='enum [anonymous]') - check('struct', '@1', {1: "@1"}, asTextOutput='struct [anonymous]') - check('struct', '@a.A', {1: "@a.A"}, asTextOutput='struct [anonymous].A') + check('struct', '@a', {1: "@a", 2: "-@a"}, asTextOutput='struct [anonymous]') + check('union', '@a', {1: "@a", 2: "-@a"}, asTextOutput='union [anonymous]') + check('enum', '@a', {1: "@a", 2: "-@a"}, asTextOutput='enum [anonymous]') + check('struct', '@1', {1: "@1", 2: "-@1"}, asTextOutput='struct [anonymous]') + check('struct', '@a.A', {1: "@a.A", 2: "@a.-A"}, asTextOutput='struct [anonymous].A') + + +def test_tagged_names(): + for key in ('struct', 'union', 'enum'): + with pytest.raises(DefinitionError): + parse('member', "int {} A".format(key)) + with pytest.raises(DefinitionError): + parse('function', "int {} A()".format(key)) + with pytest.raises(DefinitionError): + parse('function', "int A(int {} a)".format(key)) + for objType in ('macro', 'struct', 'union', 'enum', 'enumerator'): + with pytest.raises(DefinitionError): + parse(objType, '{} A'.format(key)) + # type + with pytest.raises(DefinitionError): + parse('type', '{} A'.format(key)) + check('type', '{key1}{key2} A A'.format(key2=key, key1="{key}"), idDict={1: 'A'}, key='typedef') + # TODO: namespace, namespace-push, namespace-pop + # TODO: alias + # primary expression + for expr in ('{} a.b', 'a.{} b.c', 'a.{} b'): + with pytest.raises(DefinitionError): + parse_expression(expr.format(key)) + # postfix expression + for expr in ('a->{} b->c', 'a->{} b'): + with pytest.raises(DefinitionError): + parse_expression(expr.format(key)) def test_initializers(): @@ -593,7 +629,13 @@ def test_ids_vs_tags0(app, status, warning): ws = filter_warnings(warning, "ids-vs-tags0") assert len(ws) == 0 -@pytest.mark.sphinx(confoverrides={'nitpicky': True}) +@pytest.mark.sphinx(testroot='domain-c', confoverrides={'nitpicky': True}) +def test_ids_vs_tags1(app, warning): + app.builder.build_all() + ws = filter_warnings(warning, "ids-vs-tags1") + assert len(ws) == 0 + + def test_build_domain_c_semicolon(app, warning): text = """ .. c:member:: int member; @@ -619,8 +661,8 @@ def test_build_function_param_target(app, warning): assert len(ws) == 0 entries = extract_role_links(app, "function_param_target.html") assert entries == [ - ('c.f', 'i', 'i'), - ('c.f', 'f.i', 'f.i'), + ('C2-f', 'i', 'i'), + ('C2-f', 'f.i', 'f.i'), ] @@ -647,7 +689,7 @@ def test_cfunction(app): domain="c", objtype="function", noindex=False) entry = _get_obj(app, 'PyType_GenericAlloc') - assert entry == ('index', 'c.PyType_GenericAlloc', 'function') + assert entry == ('index', 'C2-PyType_GenericAlloc', 'function') def test_cmember(app): @@ -657,7 +699,7 @@ def test_cmember(app): domain="c", objtype="member", noindex=False) entry = _get_obj(app, 'PyTypeObject.tp_bases') - assert entry == ('index', 'c.PyTypeObject.tp_bases', 'member') + assert entry == ('index', 'C2-PyTypeObject.tp_bases', 'member') def test_cvar(app): @@ -667,7 +709,7 @@ def test_cvar(app): domain="c", objtype="var", noindex=False) entry = _get_obj(app, 'PyClass_Type') - assert entry == ('index', 'c.PyClass_Type', 'member') + assert entry == ('index', 'C2-PyClass_Type', 'member') def test_noindexentry(app): @@ -676,7 +718,7 @@ def test_noindexentry(app): " :noindexentry:\n") doctree = restructuredtext.parse(app, text) assert_node(doctree, (addnodes.index, desc, addnodes.index, desc)) - assert_node(doctree[0], addnodes.index, entries=[('single', 'f (C function)', 'c.f', '', None)]) + assert_node(doctree[0], addnodes.index, entries=[('single', 'f (C function)', 'C2-f', '', None)]) assert_node(doctree[2], addnodes.index, entries=[]) From 0dd5deee2791f6888966063bfd8010b957f818a7 Mon Sep 17 00:00:00 2001 From: Jakob Lykke Andersen Date: Mon, 12 Oct 2020 17:22:45 +0200 Subject: [PATCH 07/16] C, initial tag lookup fixes --- sphinx/domains/c.py | 5 +++ tests/roots/test-domain-c/ids-vs-tags1.rst | 27 +++++++++++++--- tests/test_domain_c.py | 37 ++++++++++++++++++++++ 3 files changed, 64 insertions(+), 5 deletions(-) diff --git a/sphinx/domains/c.py b/sphinx/domains/c.py index 7c8c874ed66..351ac8d475f 100644 --- a/sphinx/domains/c.py +++ b/sphinx/domains/c.py @@ -3797,6 +3797,11 @@ def _resolve_xref_inner(self, env: BuildEnvironment, fromdocname: str, builder: logger.warning('Unparseable C cross-reference: %r\n%s', target, e, location=node) return None, None + if typ in ('struct', 'union', 'enum'): + last = name.names[-1] + if last.tag == '': + last.tag = typ + parentKey = node.get("c:parent_key", None) # type: LookupKey rootSymbol = self.data['root_symbol'] if parentKey: diff --git a/tests/roots/test-domain-c/ids-vs-tags1.rst b/tests/roots/test-domain-c/ids-vs-tags1.rst index 7778436e6b6..6bc9b039ebf 100644 --- a/tests/roots/test-domain-c/ids-vs-tags1.rst +++ b/tests/roots/test-domain-c/ids-vs-tags1.rst @@ -1,10 +1,27 @@ +.. c:namespace:: @ids_vs_tags + .. c:struct:: f_struct .. c:type:: struct f_struct f_struct .. c:union:: f_union -.. c:type:: struct f_union f_union +.. c:type:: union f_union f_union .. c:enum:: f_enum -.. c:type:: struct f_enum f_enum +.. c:type:: enum f_enum f_enum -- :c:struct:`f_struct`, :c:type:`f_struct` -- :c:union:`f_union`, :c:type:`f_union` -- :c:enum:`f_enum`, :c:type:`f_enum` +- :c:struct:`f_struct` +- :c:struct:`struct f_struct` +- :c:type:`f_struct` +- :c:type:`struct f_struct` +- :any:`f_struct` +- :any:`struct f_struct` +- :c:union:`f_union` +- :c:union:`union f_union` +- :c:type:`f_union` +- :c:type:`union f_union` +- :any:`f_union` +- :any:`union f_union` +- :c:enum:`f_enum` +- :c:enum:`enum f_enum` +- :c:type:`f_enum` +- :c:type:`enum f_enum` +- :any:`f_enum` +- :any:`enum f_enum` diff --git a/tests/test_domain_c.py b/tests/test_domain_c.py index 635ae942ce7..b64a56643ea 100644 --- a/tests/test_domain_c.py +++ b/tests/test_domain_c.py @@ -634,6 +634,43 @@ def test_ids_vs_tags1(app, warning): app.builder.build_all() ws = filter_warnings(warning, "ids-vs-tags1") assert len(ws) == 0 + t = (app.outdir / "ids-vs-tags1.html").read_text() + lis = [l for l in t.split('\n') if l.startswith(" Date: Tue, 13 Oct 2020 11:38:31 +0200 Subject: [PATCH 08/16] C, implicit tag lookup --- sphinx/domains/c.py | 30 +++++++++++++++------- tests/roots/test-domain-c/ids-vs-tags2.rst | 12 +++++++++ tests/roots/test-domain-c/index.rst | 4 +-- tests/test_domain_c.py | 7 +++++ 4 files changed, 42 insertions(+), 11 deletions(-) create mode 100644 tests/roots/test-domain-c/ids-vs-tags2.rst diff --git a/sphinx/domains/c.py b/sphinx/domains/c.py index 351ac8d475f..52b1439eeca 100644 --- a/sphinx/domains/c.py +++ b/sphinx/domains/c.py @@ -118,11 +118,6 @@ def __init__(self, identifier: str, tag: Optional[str]) -> None: self.identifier = identifier self.tag = tag - def __eq__(self, other: Any) -> bool: - return type(other) is ASTIdentifier \ - and self.identifier == other.identifier \ - and self.tag == other.tag - def is_anon(self) -> bool: return self.identifier[0] == '@' @@ -135,6 +130,13 @@ def get_id(self, version: int) -> str: else: return self.identifier + def matches(self, other: "ASTIdentifier") -> bool: + if self.identifier != other.identifier: + return False + if self.tag is None: + return True + return self.tag == other.tag + # and this is where we finally make a difference between __str__ and the display string def __str__(self) -> str: @@ -1684,7 +1686,7 @@ def candidates() -> Generator["Symbol", None, None]: if Symbol.debug_lookup: Symbol.debug_print("candidate:") print(s.to_string(Symbol.debug_indent + 1), end="") - if s.ident == ident: + if s.ident and ident.matches(s.ident): if Symbol.debug_lookup: Symbol.debug_indent += 1 Symbol.debug_print("matches") @@ -1996,11 +1998,11 @@ def find_identifier(self, ident: ASTIdentifier, Symbol.debug_print("trying:") print(current.to_string(Symbol.debug_indent + 1), end="") Symbol.debug_indent -= 2 - if matchSelf and current.ident == ident: + if matchSelf and ident.matches(current.ident): return current children = current.children_recurse_anon if recurseInAnon else current._children for s in children: - if s.ident == ident: + if ident.matches(s.ident): return s if not searchInSiblings: break @@ -3813,10 +3815,20 @@ def _resolve_xref_inner(self, env: BuildEnvironment, fromdocname: str, builder: assert parentSymbol # should be there else: parentSymbol = rootSymbol + s = parentSymbol.find_declaration(name, typ, matchSelf=True, recurseInAnon=True) if s is None or s.declaration is None: - return None, None + if typ == 'identifier': + # these are from within declarations and should be tagged correctly + return None, None + # but those from xref roles may not have correct tagging + name.setTagsToPattern() + s = parentSymbol.find_declaration(name, typ, + matchSelf=True, recurseInAnon=True) + if s is None or s.declaration is None: + return None, None + # TODO: conditionally warn about xrefs with incorrect tagging? # TODO: check role type vs. object type diff --git a/tests/roots/test-domain-c/ids-vs-tags2.rst b/tests/roots/test-domain-c/ids-vs-tags2.rst new file mode 100644 index 00000000000..cb4c6ff269e --- /dev/null +++ b/tests/roots/test-domain-c/ids-vs-tags2.rst @@ -0,0 +1,12 @@ +.. c:namespace:: @ids_vs_tags2 + +.. c:struct:: A + + .. c:union:: @data + + .. c:member:: int a + +- :c:member:`struct A.union @data.a` +- :c:member:`A.a` +- :c:member:`A.@data.a` +- :c:member:`A.a` diff --git a/tests/roots/test-domain-c/index.rst b/tests/roots/test-domain-c/index.rst index 7e2c18be997..8b3ce1e83c4 100644 --- a/tests/roots/test-domain-c/index.rst +++ b/tests/roots/test-domain-c/index.rst @@ -8,7 +8,7 @@ directives :rtype: int -.. c:function:: MyStruct hello2(char *name) +.. c:function:: struct MyStruct hello2(char *name) :rtype: MyStruct @@ -46,7 +46,7 @@ directives - :c:expr:`unsigned int` - :c:texpr:`unsigned int` -.. c:var:: A a +.. c:var:: struct A a - :c:expr:`a->b` - :c:texpr:`a->b` diff --git a/tests/test_domain_c.py b/tests/test_domain_c.py index b64a56643ea..0b2e6302963 100644 --- a/tests/test_domain_c.py +++ b/tests/test_domain_c.py @@ -673,6 +673,13 @@ def test_ids_vs_tags1(app, warning): assert entries == expected +@pytest.mark.sphinx(testroot='domain-c', confoverrides={'nitpicky': True}) +def test_ids_vs_tags2(app, warning): + app.builder.build_all() + ws = filter_warnings(warning, "ids-vs-tags2") + assert len(ws) == 0 + + def test_build_domain_c_semicolon(app, warning): text = """ .. c:member:: int member; From baec2f9363b27e1c2e4eb6844306d4cf538704bd Mon Sep 17 00:00:00 2001 From: Jakob Lykke Andersen Date: Wed, 14 Oct 2020 11:46:37 +0200 Subject: [PATCH 09/16] C, duplicate warnings for tagged names --- sphinx/domains/c.py | 5 ++++- tests/test_domain_c.py | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/sphinx/domains/c.py b/sphinx/domains/c.py index 52b1439eeca..86895cba45b 100644 --- a/sphinx/domains/c.py +++ b/sphinx/domains/c.py @@ -135,7 +135,10 @@ def matches(self, other: "ASTIdentifier") -> bool: return False if self.tag is None: return True - return self.tag == other.tag + assert other.tag is not None + isTag = self.tag == '' + isOtherTag = other.tag == '' + return isTag == isOtherTag # and this is where we finally make a difference between __str__ and the display string diff --git a/tests/test_domain_c.py b/tests/test_domain_c.py index 0b2e6302963..0452d3f9840 100644 --- a/tests/test_domain_c.py +++ b/tests/test_domain_c.py @@ -680,6 +680,21 @@ def test_ids_vs_tags2(app, warning): assert len(ws) == 0 +def test_duplicate_tags(app, warning): + text = """ +.. c:struct:: A +.. c:union:: A +.. c:enum:: A +""" + restructuredtext.parse(app, text) + ws = split_warnigns(warning) + assert len(ws) == 4 + assert "index.rst:3: WARNING: Duplicate C declaration" in ws[0] + assert "Declaration is '.. c:union:: A'." in ws[1] + assert "index.rst:4: WARNING: Duplicate C declaration" in ws[2] + assert "Declaration is '.. c:enum:: A'." in ws[3] + + def test_build_domain_c_semicolon(app, warning): text = """ .. c:member:: int member; From b75a19e9fb8f33710696247c08269f80c64fb36a Mon Sep 17 00:00:00 2001 From: Jakob Lykke Andersen Date: Wed, 14 Oct 2020 14:46:28 +0200 Subject: [PATCH 10/16] C, warn on tag mismatch --- sphinx/domains/c.py | 16 ++++++++++++++++ tests/roots/test-domain-c/wrong-tags.rst | 14 ++++++++++++++ tests/test_domain_c.py | 20 ++++++++++++++++++++ 3 files changed, 50 insertions(+) create mode 100644 tests/roots/test-domain-c/wrong-tags.rst diff --git a/sphinx/domains/c.py b/sphinx/domains/c.py index 86895cba45b..9dee78ae3a4 100644 --- a/sphinx/domains/c.py +++ b/sphinx/domains/c.py @@ -3833,6 +3833,22 @@ def _resolve_xref_inner(self, env: BuildEnvironment, fromdocname: str, builder: return None, None # TODO: conditionally warn about xrefs with incorrect tagging? + # check if tags are used correctly + sName = s.get_full_nested_name() + assert len(name.names) <= len(sName.names) + for n, ns in zip(reversed(name.names), reversed(sName.names)): + if n.tag is None: + continue + assert ns.tag is not None + assert (n.tag == '') == (ns.tag == '') + if n.tag != ns.tag: + logger.warning( + "C '%s' cross-reference uses wrong tag:" + " reference name is '%s' but found name is '%s'." + " Full reference name is '%s'." + " Full found name is '%s'.", + typ, n, ns, name, sName, location=node) + # TODO: check role type vs. object type declaration = s.declaration diff --git a/tests/roots/test-domain-c/wrong-tags.rst b/tests/roots/test-domain-c/wrong-tags.rst new file mode 100644 index 00000000000..2fba4270202 --- /dev/null +++ b/tests/roots/test-domain-c/wrong-tags.rst @@ -0,0 +1,14 @@ +.. c:namespace:: @wrong_tag + +.. c:struct:: A + + .. c:var:: int i + +- :c:var:`A.i` +- :c:var:`union A.i` +- :c:var:`enum A.i` + +.. c:function:: void f1(union A a) +.. c:function:: void f2(enum A a) + +.. c:var:: int union A.j diff --git a/tests/test_domain_c.py b/tests/test_domain_c.py index 0452d3f9840..5be0423fd6c 100644 --- a/tests/test_domain_c.py +++ b/tests/test_domain_c.py @@ -695,6 +695,26 @@ def test_duplicate_tags(app, warning): assert "Declaration is '.. c:enum:: A'." in ws[3] +@pytest.mark.sphinx(testroot='domain-c', confoverrides={'nitpicky': True}) +def test_build_domain_c_wrong_tags(app, warning): + app.builder.build_all() + ws = filter_warnings(warning, "wrong-tags") + template = ".rst:%d: WARNING: C '%s' cross-reference uses wrong tag:"\ + " reference name is '%s' but found name is '%s'."\ + " Full reference name is '%s'."\ + " Full found name is '%s'." + expected = [ + template % (8, 'var', 'union A', 'struct A', 'union A.i', '@wrong_tag.struct A.i'), + template % (9, 'var', 'enum A', 'struct A', 'enum A.i', '@wrong_tag.struct A.i'), + template % (11, 'identifier', 'union A', 'struct A', 'union A', '@wrong_tag.struct A'), + template % (13, 'identifier', 'enum A', 'struct A', 'enum A', '@wrong_tag.struct A'), + template % (14, 'identifier', 'union A', 'struct A', 'union A', '@wrong_tag.struct A'), + ] + for i in range(len(expected)): + assert expected[i] in ws[i] + assert len(ws) == len(expected) + + def test_build_domain_c_semicolon(app, warning): text = """ .. c:member:: int member; From 6c73fb28a7271e0c730769967e508744a7995966 Mon Sep 17 00:00:00 2001 From: Jakob Lykke Andersen Date: Sat, 9 Jan 2021 17:50:42 +0100 Subject: [PATCH 11/16] C, add test --- tests/roots/test-domain-c/ids-vs-tags3.rst | 19 ++++++++++++ tests/test_domain_c.py | 34 ++++++++++++++++++++++ 2 files changed, 53 insertions(+) create mode 100644 tests/roots/test-domain-c/ids-vs-tags3.rst diff --git a/tests/roots/test-domain-c/ids-vs-tags3.rst b/tests/roots/test-domain-c/ids-vs-tags3.rst new file mode 100644 index 00000000000..4013249aeba --- /dev/null +++ b/tests/roots/test-domain-c/ids-vs-tags3.rst @@ -0,0 +1,19 @@ +.. c:namespace:: @ids_vs_tags3 + +.. c:function:: void f1(int i) + +.. c:struct:: f1 + + .. c:var:: int i + + +.. c:struct:: f2 + + .. c:var:: int i + +.. c:function:: void f2(int i) + +- :c:var:`f1.i`, resolves to the function parameter +- :c:var:`struct f1.i`, resolves to struct f1.i +- :c:var:`f2.i`, resolves to the function parameter +- :c:var:`struct f2.i`, resolves to struct f2.i diff --git a/tests/test_domain_c.py b/tests/test_domain_c.py index 5be0423fd6c..b5ce6ebabda 100644 --- a/tests/test_domain_c.py +++ b/tests/test_domain_c.py @@ -680,6 +680,40 @@ def test_ids_vs_tags2(app, warning): assert len(ws) == 0 +@pytest.mark.sphinx(testroot='domain-c', confoverrides={'nitpicky': True}) +def test_ids_vs_tags3(app, warning): + app.builder.build_all() + ws = filter_warnings(warning, "ids-vs-tags3") + assert len(ws) == 0 + t = (app.outdir / "ids-vs-tags3.html").read_text() + lis = [l for l in t.split('\n') if l.startswith(" Date: Sun, 10 Jan 2021 11:29:04 +0100 Subject: [PATCH 12/16] C, test namespace revamp --- tests/roots/test-domain-c/anon-dup-decl.rst | 2 ++ .../test-domain-c/function_param_target.rst | 2 ++ tests/roots/test-domain-c/ids-vs-tags1.rst | 2 +- tests/roots/test-domain-c/ids-vs-tags2.rst | 2 +- tests/roots/test-domain-c/ids-vs-tags3.rst | 2 +- tests/roots/test-domain-c/index.rst | 2 ++ tests/roots/test-domain-c/wrong-tags.rst | 2 +- tests/test_domain_c.py | 18 +++++++++--------- 8 files changed, 19 insertions(+), 13 deletions(-) diff --git a/tests/roots/test-domain-c/anon-dup-decl.rst b/tests/roots/test-domain-c/anon-dup-decl.rst index 5f6c3bdfe36..743ae2f6a84 100644 --- a/tests/roots/test-domain-c/anon-dup-decl.rst +++ b/tests/roots/test-domain-c/anon-dup-decl.rst @@ -1,3 +1,5 @@ +.. c:namespace:: anon_dup_decl_ns + .. c:struct:: anon_dup_decl .. c:struct:: @a.A diff --git a/tests/roots/test-domain-c/function_param_target.rst b/tests/roots/test-domain-c/function_param_target.rst index 05de01445d4..d316d7bcd16 100644 --- a/tests/roots/test-domain-c/function_param_target.rst +++ b/tests/roots/test-domain-c/function_param_target.rst @@ -1,3 +1,5 @@ +.. c:namespace:: function_param_target + .. c:function:: void f(int i) - :c:var:`i` diff --git a/tests/roots/test-domain-c/ids-vs-tags1.rst b/tests/roots/test-domain-c/ids-vs-tags1.rst index 6bc9b039ebf..177722dcd40 100644 --- a/tests/roots/test-domain-c/ids-vs-tags1.rst +++ b/tests/roots/test-domain-c/ids-vs-tags1.rst @@ -1,4 +1,4 @@ -.. c:namespace:: @ids_vs_tags +.. c:namespace:: ids_vs_tags .. c:struct:: f_struct .. c:type:: struct f_struct f_struct diff --git a/tests/roots/test-domain-c/ids-vs-tags2.rst b/tests/roots/test-domain-c/ids-vs-tags2.rst index cb4c6ff269e..3208abda7f7 100644 --- a/tests/roots/test-domain-c/ids-vs-tags2.rst +++ b/tests/roots/test-domain-c/ids-vs-tags2.rst @@ -1,4 +1,4 @@ -.. c:namespace:: @ids_vs_tags2 +.. c:namespace:: ids_vs_tags2 .. c:struct:: A diff --git a/tests/roots/test-domain-c/ids-vs-tags3.rst b/tests/roots/test-domain-c/ids-vs-tags3.rst index 4013249aeba..50d90506e22 100644 --- a/tests/roots/test-domain-c/ids-vs-tags3.rst +++ b/tests/roots/test-domain-c/ids-vs-tags3.rst @@ -1,4 +1,4 @@ -.. c:namespace:: @ids_vs_tags3 +.. c:namespace:: ids_vs_tags3 .. c:function:: void f1(int i) diff --git a/tests/roots/test-domain-c/index.rst b/tests/roots/test-domain-c/index.rst index 8b3ce1e83c4..4f2a14d6012 100644 --- a/tests/roots/test-domain-c/index.rst +++ b/tests/roots/test-domain-c/index.rst @@ -1,3 +1,5 @@ +.. c:namespace:: index + test-domain-c ============= diff --git a/tests/roots/test-domain-c/wrong-tags.rst b/tests/roots/test-domain-c/wrong-tags.rst index 2fba4270202..5ca56b8a71e 100644 --- a/tests/roots/test-domain-c/wrong-tags.rst +++ b/tests/roots/test-domain-c/wrong-tags.rst @@ -1,4 +1,4 @@ -.. c:namespace:: @wrong_tag +.. c:namespace:: wrong_tag .. c:struct:: A diff --git a/tests/test_domain_c.py b/tests/test_domain_c.py index b5ce6ebabda..a1ede902553 100644 --- a/tests/test_domain_c.py +++ b/tests/test_domain_c.py @@ -654,7 +654,7 @@ def test_ids_vs_tags1(app, warning): text = ''.join(code.itertext()) entries.append((target, title, text)) expected = [] - idPrefix = 'C2-@ids_vs_tags.' + idPrefix = 'C2-ids_vs_tags.' for tag in ['struct', 'union', 'enum']: name = 'f_' + tag tagTitle = tag + ' ' + name @@ -704,7 +704,7 @@ def test_ids_vs_tags3(app, warning): assert code.tag == 'code' text = ''.join(code.itertext()) entries.append((target, title, text)) - idPrefix = 'C2-@ids_vs_tags3.' + idPrefix = 'C2-ids_vs_tags3.' expected = [ (idPrefix + 'f1', 'f1.i', 'f1.i'), (idPrefix + '-f1.i', 'struct f1.i', 'struct f1.i'), @@ -738,11 +738,11 @@ def test_build_domain_c_wrong_tags(app, warning): " Full reference name is '%s'."\ " Full found name is '%s'." expected = [ - template % (8, 'var', 'union A', 'struct A', 'union A.i', '@wrong_tag.struct A.i'), - template % (9, 'var', 'enum A', 'struct A', 'enum A.i', '@wrong_tag.struct A.i'), - template % (11, 'identifier', 'union A', 'struct A', 'union A', '@wrong_tag.struct A'), - template % (13, 'identifier', 'enum A', 'struct A', 'enum A', '@wrong_tag.struct A'), - template % (14, 'identifier', 'union A', 'struct A', 'union A', '@wrong_tag.struct A'), + template % (8, 'var', 'union A', 'struct A', 'union A.i', 'wrong_tag.struct A.i'), + template % (9, 'var', 'enum A', 'struct A', 'enum A.i', 'wrong_tag.struct A.i'), + template % (11, 'identifier', 'union A', 'struct A', 'union A', 'wrong_tag.struct A'), + template % (13, 'identifier', 'enum A', 'struct A', 'enum A', 'wrong_tag.struct A'), + template % (14, 'identifier', 'union A', 'struct A', 'union A', 'wrong_tag.struct A'), ] for i in range(len(expected)): assert expected[i] in ws[i] @@ -774,8 +774,8 @@ def test_build_function_param_target(app, warning): assert len(ws) == 0 entries = extract_role_links(app, "function_param_target.html") assert entries == [ - ('C2-f', 'i', 'i'), - ('C2-f', 'f.i', 'f.i'), + ('C2-function_param_target.f', 'i', 'i'), + ('C2-function_param_target.f', 'f.i', 'f.i'), ] From 40baf5c8ad00c95b966ed125317e446a177cbfe2 Mon Sep 17 00:00:00 2001 From: Jakob Lykke Andersen Date: Sun, 10 Jan 2021 12:28:28 +0100 Subject: [PATCH 13/16] C, fix tag warnings for anon types --- sphinx/domains/c.py | 31 ++++++++++++++++++---- tests/roots/test-domain-c/ids-vs-tags2.rst | 22 ++++++++++----- 2 files changed, 42 insertions(+), 11 deletions(-) diff --git a/sphinx/domains/c.py b/sphinx/domains/c.py index 9dee78ae3a4..bb6e4ac2752 100644 --- a/sphinx/domains/c.py +++ b/sphinx/domains/c.py @@ -3836,18 +3836,39 @@ def _resolve_xref_inner(self, env: BuildEnvironment, fromdocname: str, builder: # check if tags are used correctly sName = s.get_full_nested_name() assert len(name.names) <= len(sName.names) - for n, ns in zip(reversed(name.names), reversed(sName.names)): - if n.tag is None: + nextName = len(sName.names) - 1 + # print("Matching '{}' to '{}'".format(name, sName)) + for xRefName in reversed(name.names): + # find the next symbol name that matches the xref name + # potentially skipping anon names + # print("\tMatching:", xRefName) + stop = False + while True: + if nextName == -1: + stop = True + break + ns = sName.names[nextName] + nextName -= 1 + if xRefName.identifier == ns.identifier: + # print("\t\tSame ident:", ns) + break + else: + # print("\t\tSkipping:", ns) + assert ns.is_anon() + if stop: + break + # print("\t\tRes(nextName={}): '{}' vs. '{}'".format(nextName, xRefName, ns)) + if xRefName.tag is None: continue assert ns.tag is not None - assert (n.tag == '') == (ns.tag == '') - if n.tag != ns.tag: + assert (xRefName.tag == '') == (ns.tag == '') + if xRefName.tag != ns.tag: logger.warning( "C '%s' cross-reference uses wrong tag:" " reference name is '%s' but found name is '%s'." " Full reference name is '%s'." " Full found name is '%s'.", - typ, n, ns, name, sName, location=node) + typ, xRefName, ns, name, sName, location=node) # TODO: check role type vs. object type diff --git a/tests/roots/test-domain-c/ids-vs-tags2.rst b/tests/roots/test-domain-c/ids-vs-tags2.rst index 3208abda7f7..822ae583b52 100644 --- a/tests/roots/test-domain-c/ids-vs-tags2.rst +++ b/tests/roots/test-domain-c/ids-vs-tags2.rst @@ -2,11 +2,21 @@ .. c:struct:: A - .. c:union:: @data + .. c:union:: @B - .. c:member:: int a + .. c:enum:: C -- :c:member:`struct A.union @data.a` -- :c:member:`A.a` -- :c:member:`A.@data.a` -- :c:member:`A.a` + .. c:enumerator:: D + +- :c:enumerator:`struct A.union @B.enum C.D` +- :c:enumerator:`A.union @B.enum C.D` +- :c:enumerator:`struct A.@B.enum C.D` +- :c:enumerator:`struct A.union @B.C.D` +- :c:enumerator:`A.@B.enum C.D` +- :c:enumerator:`A.union @B.C.D` +- :c:enumerator:`struct A.@B.C.D` +- :c:enumerator:`A.@B.C.D` +- :c:enumerator:`struct A.enum C.D` +- :c:enumerator:`A.enum C.D` +- :c:enumerator:`struct A.C.D` +- :c:enumerator:`A.C.D` From 51834d48a8eb2f3b850d52eff3fa45ada352f22b Mon Sep 17 00:00:00 2001 From: Jakob Lykke Andersen Date: Thu, 4 Feb 2021 20:01:55 +0100 Subject: [PATCH 14/16] C, update intersphinx test --- tests/test_domain_c.py | 51 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/tests/test_domain_c.py b/tests/test_domain_c.py index a1ede902553..bb6a36474b3 100644 --- a/tests/test_domain_c.py +++ b/tests/test_domain_c.py @@ -836,7 +836,7 @@ def test_noindexentry(app): @pytest.mark.sphinx(testroot='domain-c-intersphinx', confoverrides={'nitpicky': True}) -def test_intersphinx(tempdir, app, status, warning): +def test_intersphinx_v3_remote(tempdir, app, status, warning): origSource = """\ .. c:member:: int _member .. c:var:: int _var @@ -882,3 +882,52 @@ def test_intersphinx(tempdir, app, status, warning): app.builder.build_all() ws = filter_warnings(warning, "index") assert len(ws) == 0 + + +@pytest.mark.sphinx(testroot='domain-c-intersphinx', confoverrides={'nitpicky': True}) +def test_intersphinx_v4_remote(tempdir, app, status, warning): + origSource = """\ +.. c:member:: int _member +.. c:var:: int _var +.. c:function:: void _function() +.. c:macro:: _macro +.. c:struct:: _struct +.. c:union:: _union +.. c:enum:: _enum + + .. c:enumerator:: _enumerator + +.. c:type:: _type +.. c:function:: void _functionParam(int param) +""" # noqa + inv_file = tempdir / 'inventory' + inv_file.write_bytes(b'''\ +# Sphinx inventory version 2 +# Project: C Intersphinx Test +# Version: +# The remainder of this file is compressed using zlib. +''' + zlib.compress(b'''\ +_enumerator c:enumerator 1 index.html#C2--_enum.$ - +_function c:function 1 index.html#C2-$ - +_functionParam c:function 1 index.html#C2-$ - +_functionParam.param c:functionParam 1 index.html#C2-_functionParam - +_macro c:macro 1 index.html#C2-$ - +_member c:member 1 index.html#C2-$ - +_type c:type 1 index.html#C2-$ - +_var c:member 1 index.html#C2-$ - +enum _enum c:enum 1 index.html#C2--_enum - +enum _enum._enumerator c:enumerator 1 index.html#C2--_enum._enumerator - +struct _struct c:struct 1 index.html#C2--_struct - +union _union c:union 1 index.html#C2--_union - +''')) # noqa + app.config.intersphinx_mapping = { + 'https://localhost/intersphinx/c/': inv_file, + } + app.config.intersphinx_cache_limit = 0 + # load the inventory and check if it's done correctly + normalize_intersphinx_mapping(app, app.config) + load_mappings(app) + + app.builder.build_all() + ws = filter_warnings(warning, "index") + assert len(ws) == 0 From bea89e9c95324fedaaa724ea3b17da2e61c57d11 Mon Sep 17 00:00:00 2001 From: Jakob Lykke Andersen Date: Mon, 22 Feb 2021 21:26:19 +0100 Subject: [PATCH 15/16] C, allow relaxed tagging on c:identifier xrefs But warn when it happens --- sphinx/domains/c.py | 14 +++++++++----- tests/test_domain_c.py | 13 +++++++++++-- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/sphinx/domains/c.py b/sphinx/domains/c.py index bb6e4ac2752..458467e6abc 100644 --- a/sphinx/domains/c.py +++ b/sphinx/domains/c.py @@ -3822,15 +3822,19 @@ def _resolve_xref_inner(self, env: BuildEnvironment, fromdocname: str, builder: s = parentSymbol.find_declaration(name, typ, matchSelf=True, recurseInAnon=True) if s is None or s.declaration is None: - if typ == 'identifier': - # these are from within declarations and should be tagged correctly - return None, None - # but those from xref roles may not have correct tagging + # try with relaxed tagging name.setTagsToPattern() s = parentSymbol.find_declaration(name, typ, matchSelf=True, recurseInAnon=True) if s is None or s.declaration is None: return None, None + # only warn for identifiers, as they should contain the correct tagging + if typ == 'identifier': + logger.warning( + "c:%s reference has incorrect tagging:" + " Full reference name is '%s'." + " Full found name is '%s'.", + typ, name, s.get_full_nested_name(), location=node) # TODO: conditionally warn about xrefs with incorrect tagging? # check if tags are used correctly @@ -3864,7 +3868,7 @@ def _resolve_xref_inner(self, env: BuildEnvironment, fromdocname: str, builder: assert (xRefName.tag == '') == (ns.tag == '') if xRefName.tag != ns.tag: logger.warning( - "C '%s' cross-reference uses wrong tag:" + "c:%s reference uses wrong tag:" " reference name is '%s' but found name is '%s'." " Full reference name is '%s'." " Full found name is '%s'.", diff --git a/tests/test_domain_c.py b/tests/test_domain_c.py index bb6a36474b3..2289c45b3b1 100644 --- a/tests/test_domain_c.py +++ b/tests/test_domain_c.py @@ -623,11 +623,19 @@ def test_build_domain_c_anon_dup_decl(app, status, warning): assert "WARNING: c:identifier reference target not found: @a" in ws[0] assert "WARNING: c:identifier reference target not found: @b" in ws[1] + @pytest.mark.sphinx(testroot='domain-c', confoverrides={'nitpicky': True}) def test_ids_vs_tags0(app, status, warning): app.builder.build_all() ws = filter_warnings(warning, "ids-vs-tags0") - assert len(ws) == 0 + assert len(ws) == 3 + msg = "WARNING: c:identifier reference has incorrect tagging:" + msg += " Full reference name is '{}'." + msg += " Full found name is '{}'." + assert msg.format("_struct", "struct _struct") in ws[0] + assert msg.format("_union", "union _union") in ws[1] + assert msg.format("_enum", "enum _enum") in ws[2] + @pytest.mark.sphinx(testroot='domain-c', confoverrides={'nitpicky': True}) def test_ids_vs_tags1(app, warning): @@ -733,7 +741,7 @@ def test_duplicate_tags(app, warning): def test_build_domain_c_wrong_tags(app, warning): app.builder.build_all() ws = filter_warnings(warning, "wrong-tags") - template = ".rst:%d: WARNING: C '%s' cross-reference uses wrong tag:"\ + template = ".rst:%d: WARNING: c:%s reference uses wrong tag:"\ " reference name is '%s' but found name is '%s'."\ " Full reference name is '%s'."\ " Full found name is '%s'." @@ -766,6 +774,7 @@ def test_build_domain_c_semicolon(app, warning): ws = split_warnigns(warning) assert len(ws) == 0 + @pytest.mark.sphinx(testroot='domain-c', confoverrides={'nitpicky': True}) def test_build_function_param_target(app, warning): # the anchor for function parameters should be the function From 1393a983f71864d3edf00e603df1d7671db020c5 Mon Sep 17 00:00:00 2001 From: Jakob Lykke Andersen Date: Mon, 22 Feb 2021 21:47:16 +0100 Subject: [PATCH 16/16] C, update intersphinx and ids_vs_tags0 with anon --- .../roots/test-domain-c-intersphinx/index.rst | 9 +++++++++ tests/roots/test-domain-c/ids-vs-tags0.rst | 13 ++++++++++++ tests/test_domain_c.py | 20 +++++++++++++++++++ 3 files changed, 42 insertions(+) diff --git a/tests/roots/test-domain-c-intersphinx/index.rst b/tests/roots/test-domain-c-intersphinx/index.rst index 5d6d3e09891..c97cc695c0e 100644 --- a/tests/roots/test-domain-c-intersphinx/index.rst +++ b/tests/roots/test-domain-c-intersphinx/index.rst @@ -60,3 +60,12 @@ - :c:member:`_functionParam.param` - :c:var:`_functionParam.param` - :c:data:`_functionParam.param` + +- :any:`_struct.@anon.i` +- :c:member:`_struct.@anon.i` +- :c:var:`_struct.@anon.i` +- :c:data:`_struct.@anon.i` +- :any:`_struct.i` +- :c:member:`_struct.i` +- :c:var:`_struct.i` +- :c:data:`_struct.i` diff --git a/tests/roots/test-domain-c/ids-vs-tags0.rst b/tests/roots/test-domain-c/ids-vs-tags0.rst index 20f52d8f202..f00d3b79570 100644 --- a/tests/roots/test-domain-c/ids-vs-tags0.rst +++ b/tests/roots/test-domain-c/ids-vs-tags0.rst @@ -3,6 +3,11 @@ .. c:function:: void _function() .. c:macro:: _macro .. c:struct:: _struct + + .. c:union:: @anon + + .. c:var:: int i + .. c:union:: _union .. c:enum:: _enum @@ -75,3 +80,11 @@ - :c:var:`_functionParam.param` - :c:data:`_functionParam.param` +- :any:`_struct.@anon.i` +- :c:member:`_struct.@anon.i` +- :c:var:`_struct.@anon.i` +- :c:data:`_struct.@anon.i` +- :any:`_struct.i` +- :c:member:`_struct.i` +- :c:var:`_struct.i` +- :c:data:`_struct.i` diff --git a/tests/test_domain_c.py b/tests/test_domain_c.py index 2289c45b3b1..55f2af23063 100644 --- a/tests/test_domain_c.py +++ b/tests/test_domain_c.py @@ -626,6 +626,8 @@ def test_build_domain_c_anon_dup_decl(app, status, warning): @pytest.mark.sphinx(testroot='domain-c', confoverrides={'nitpicky': True}) def test_ids_vs_tags0(app, status, warning): + # this test is essentially the base case for the intersphinx tests, + # where both the primary declarations and the references are in one project app.builder.build_all() ws = filter_warnings(warning, "ids-vs-tags0") assert len(ws) == 3 @@ -846,12 +848,19 @@ def test_noindexentry(app): @pytest.mark.sphinx(testroot='domain-c-intersphinx', confoverrides={'nitpicky': True}) def test_intersphinx_v3_remote(tempdir, app, status, warning): + # a splitting of test_ids_vs_tags0 into the primary directives in a remote project, + # and then the references in the test project origSource = """\ .. c:member:: int _member .. c:var:: int _var .. c:function:: void _function() .. c:macro:: _macro .. c:struct:: _struct + + .. c:union:: @anon + + .. c:var:: int i + .. c:union:: _union .. c:enum:: _enum @@ -876,6 +885,8 @@ def test_intersphinx_v3_remote(tempdir, app, status, warning): _macro c:macro 1 index.html#c.$ - _member c:member 1 index.html#c.$ - _struct c:struct 1 index.html#c.$ - +_struct.@anon c:union 1 index.html#c.$ _struct.[anonymous] +_struct.@anon.i c:member 1 index.html#c.$ _struct.[anonymous].i _type c:type 1 index.html#c.$ - _union c:union 1 index.html#c.$ - _var c:member 1 index.html#c.$ - @@ -895,12 +906,19 @@ def test_intersphinx_v3_remote(tempdir, app, status, warning): @pytest.mark.sphinx(testroot='domain-c-intersphinx', confoverrides={'nitpicky': True}) def test_intersphinx_v4_remote(tempdir, app, status, warning): + # a splitting of test_ids_vs_tags0 into the primary directives in a remote project, + # and then the references in the test project origSource = """\ .. c:member:: int _member .. c:var:: int _var .. c:function:: void _function() .. c:macro:: _macro .. c:struct:: _struct + + .. c:union:: @anon + + .. c:var:: int i + .. c:union:: _union .. c:enum:: _enum @@ -927,6 +945,8 @@ def test_intersphinx_v4_remote(tempdir, app, status, warning): enum _enum c:enum 1 index.html#C2--_enum - enum _enum._enumerator c:enumerator 1 index.html#C2--_enum._enumerator - struct _struct c:struct 1 index.html#C2--_struct - +struct _struct.union @anon c:union 1 index.html#C2--_struct.-@anon struct _struct.union [anonymous] +struct _struct.union @anon.i c:member 1 index.html#C2--_struct.-@anon.i struct _struct.union [anonymous].i union _union c:union 1 index.html#C2--_union - ''')) # noqa app.config.intersphinx_mapping = {