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.

Sign up for GitHub

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

Jump to bottom

[C++] Ensure consistent non-specialization template argument representation #10257

Closed
wants to merge 1 commit into from
Closed

[C++] Ensure consistent non-specialization template argument representation #10257

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
109 changes: 78 additions & 31 deletions sphinx/domains/cpp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2577,6 +2577,10 @@ def name(self) -> ASTNestedName:
def name(self, name: ASTNestedName) -> None:
self.next.name = name

@property
def isPack(self) -> bool:
return self.next.isPack

@property
def function_params(self) -> List[ASTFunctionParameter]:
return self.next.function_params
Expand Down Expand Up @@ -2682,7 +2686,7 @@ def name(self, name: ASTNestedName) -> None:

@property
def isPack(self) -> bool:
return True
return self.next.isPack

@property
def function_params(self) -> List[ASTFunctionParameter]:
Expand Down Expand Up @@ -2756,6 +2760,10 @@ def function_params(self) -> List[ASTFunctionParameter]:
def trailingReturn(self) -> "ASTType":
return self.next.trailingReturn

@property
def isPack(self) -> bool:
return True

def require_space_after_declSpecs(self) -> bool:
return False

Expand Down Expand Up @@ -2812,6 +2820,10 @@ def name(self) -> ASTNestedName:
def name(self, name: ASTNestedName) -> None:
self.next.name = name

@property
def isPack(self):
return self.next.isPack

@property
def function_params(self) -> List[ASTFunctionParameter]:
return self.next.function_params
Expand Down Expand Up @@ -2909,6 +2921,10 @@ def name(self) -> ASTNestedName:
def name(self, name: ASTNestedName) -> None:
self.inner.name = name

@property
def isPack(self):
return self.inner.isPack or self.next.isPack

@property
def function_params(self) -> List[ASTFunctionParameter]:
return self.inner.function_params
Expand Down Expand Up @@ -3457,6 +3473,14 @@ def describe_signature(self, parentNode: TextElement, mode: str,
env: "BuildEnvironment", symbol: "Symbol") -> None:
raise NotImplementedError(repr(self))

@property
def isPack(self) -> bool:
raise NotImplementedError(repr(self))

@property
def name(self) -> ASTNestedName:
raise NotImplementedError(repr(self))


class ASTTemplateKeyParamPackIdDefault(ASTTemplateParam):
def __init__(self, key: str, identifier: ASTIdentifier,
Expand Down Expand Up @@ -4041,6 +4065,31 @@ def __init__(self, data: List[Tuple[ASTNestedNameElement,
self.data = data


def _is_specialization(templateParams: Union[ASTTemplateParams, ASTTemplateIntroduction],
templateArgs: ASTTemplateArgs) -> bool:
"""Checks if `templateArgs` does not exactly match `templateParams`."""
# the names of the template parameters must be given exactly as args
# and params that are packs must in the args be the name expanded
if len(templateParams.params) != len(templateArgs.args):
return True
# having no template params and no arguments is also a specialization
if len(templateParams.params) == 0:
return True
for i in range(len(templateParams.params)):
param = templateParams.params[i]
arg = templateArgs.args[i]
# TODO: doing this by string manipulation is probably not the most efficient
paramName = str(param.name)
argTxt = str(arg)
isArgPackExpansion = argTxt.endswith('...')
if param.isPack != isArgPackExpansion:
return True
argName = argTxt[:-3] if isArgPackExpansion else argTxt
if paramName != argName:
return True
return False


class Symbol:
debug_indent = 0
debug_indent_string = " "
Expand Down Expand Up @@ -4089,6 +4138,16 @@ def __init__(self, parent: "Symbol", identOrOp: Union[ASTIdentifier, ASTOperator
self.siblingAbove: Symbol = None
self.siblingBelow: Symbol = None
self.identOrOp = identOrOp
# Ensure the same symbol for `A` is created for:
#
# .. cpp:class:: template <typename T> class A
#
# and
#
# .. cpp:function:: template <typename T> int A<T>::foo()
if (templateArgs is not None and
not _is_specialization(templateParams, templateArgs)):
templateArgs = None
self.templateParams = templateParams # template<templateParams>
self.templateArgs = templateArgs # identifier<templateArgs>
self.declaration = declaration
Expand Down Expand Up @@ -4269,33 +4328,12 @@ def _find_named_symbols(self, identOrOp: Union[ASTIdentifier, ASTOperator],
Symbol.debug_print("correctPrimaryTemplateAargs:", correctPrimaryTemplateArgs)
Symbol.debug_print("searchInSiblings: ", searchInSiblings)

def isSpecialization() -> bool:
# the names of the template parameters must be given exactly as args
# and params that are packs must in the args be the name expanded
if len(templateParams.params) != len(templateArgs.args):
return True
# having no template params and no arguments is also a specialization
if len(templateParams.params) == 0:
return True
for i in range(len(templateParams.params)):
param = templateParams.params[i]
arg = templateArgs.args[i]
# TODO: doing this by string manipulation is probably not the most efficient
paramName = str(param.name)
argTxt = str(arg)
isArgPackExpansion = argTxt.endswith('...')
if param.isPack != isArgPackExpansion:
return True
argName = argTxt[:-3] if isArgPackExpansion else argTxt
if paramName != argName:
return True
return False
if correctPrimaryTemplateArgs:
if templateParams is not None and templateArgs is not None:
# If both are given, but it's not a specialization, then do lookup as if
# there is no argument list.
# For example: template<typename T> int A<T>::var;
if not isSpecialization():
if not _is_specialization(templateParams, templateArgs):
templateArgs = None

def matches(s: "Symbol") -> bool:
Expand Down Expand Up @@ -4751,14 +4789,23 @@ def unconditionalAdd(self, otherChild):
ourChild.declaration.directiveType, name)
logger.warning(msg, location=(otherChild.docname, otherChild.line))
else:
# Both have declarations, and in the same docname.
# This can apparently happen, it should be safe to
# just ignore it, right?
# Hmm, only on duplicate declarations, right?
msg = "Internal C++ domain error during symbol merging.\n"
msg += "ourChild:\n" + ourChild.to_string(1)
msg += "\notherChild:\n" + otherChild.to_string(1)
logger.warning(msg, location=otherChild.docname)
if (otherChild.declaration.objectType ==
ourChild.declaration.objectType and
otherChild.declaration.objectType in
('templateParam', 'functionParam')):
# `ourChild` was presumably just created during mergging
# by the call to `_fill_empty` on the parent and can be
# ignored.
pass
else:
# Both have declarations, and in the same docname.
# This can apparently happen, it should be safe to
# just ignore it, right?
# Hmm, only on duplicate declarations, right?
msg = "Internal C++ domain error during symbol merging.\n"
msg += "ourChild:\n" + ourChild.to_string(1)
msg += "\notherChild:\n" + otherChild.to_string(1)
logger.warning(msg, location=otherChild.docname)
ourChild.merge_with(otherChild, docnames, env)
if Symbol.debug_lookup:
Symbol.debug_indent -= 2
Expand Down
90 changes: 88 additions & 2 deletions tests/test_domain_cpp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -846,9 +846,9 @@ def test_domain_cpp_ast_templates():
check('class', 'abc::ns::foo{{id_0, id_1, ...id_2}} {key}xyz::bar',
{2: 'I00DpEXN3abc2ns3fooEI4id_04id_1sp4id_2EEN3xyz3barE'})
check('class', 'abc::ns::foo{{id_0, id_1, id_2}} {key}xyz::bar<id_0, id_1, id_2>',
{2: 'I000EXN3abc2ns3fooEI4id_04id_14id_2EEN3xyz3barI4id_04id_14id_2EE'})
{2: 'I000EXN3abc2ns3fooEI4id_04id_14id_2EEN3xyz3barE'})
check('class', 'abc::ns::foo{{id_0, id_1, ...id_2}} {key}xyz::bar<id_0, id_1, id_2...>',
{2: 'I00DpEXN3abc2ns3fooEI4id_04id_1sp4id_2EEN3xyz3barI4id_04id_1Dp4id_2EE'})
{2: 'I00DpEXN3abc2ns3fooEI4id_04id_1sp4id_2EEN3xyz3barE'})

check('class', 'template<> Concept{{U}} {key}A<int>::B', {2: 'IEI0EX7ConceptI1UEEN1AIiE1BE'})

Expand Down Expand Up @@ -1367,3 +1367,89 @@ def test_domain_cpp_parse_mix_decl_duplicate(app, warning):
assert "index.rst:3: WARNING: Duplicate C++ declaration, also defined at index:1." in ws[2]
assert "Declaration is '.. cpp:struct:: A'." in ws[3]
assert ws[4] == ""


# For some reason, using the default testroot of "root" leads to the contents of
# `test-root/objects.txt` polluting the symbol table depending on the test
# execution order. Using a testroot of "config" seems to avoid that problem.
@pytest.mark.sphinx(testroot='config')
def test_domain_cpp_normalize_unspecialized_template_args(make_app, app_params):
args, kwargs = app_params

text1 = (".. cpp:class:: template <typename T> A\n")
text2 = (".. cpp:class:: template <typename T> template <typename U> A<T>::B\n")

app1 = make_app(*args, **kwargs)
restructuredtext.parse(app=app1, text=text1, docname='text1')
root1 = app1.env.domaindata['cpp']['root_symbol']

assert root1.dump(1) == (
' ::\n'
' template<typename T> \n'
' A: template<typename T> A\t(text1)\n'
' T: typename T\t(text1)\n'
)

app2 = make_app(*args, **kwargs)
restructuredtext.parse(app=app2, text=text2, docname='text2')
root2 = app2.env.domaindata['cpp']['root_symbol']

assert root2.dump(1) == (
' ::\n'
' template<typename T> \n'
' A\n'
' T\n'
' template<typename U> \n'
' B: template<typename T> template<typename U> A<T>::B\t(text2)\n'
' U: typename U\t(text2)\n'
)

root2.merge_with(root1, ['text1'], app2.env)

assert root2.dump(1) == (
' ::\n'
' template<typename T> \n'
' A: template<typename T> A\t(text1)\n'
' T: typename T\t(text1)\n'
' template<typename U> \n'
' B: template<typename T> template<typename U> A<T>::B\t(text2)\n'
' U: typename U\t(text2)\n'
)
warning = app2._warning.getvalue()
assert 'Internal C++ domain error during symbol merging' not in warning


def parse_template_parameter(param: str):
ast = parse('type', 'template<' + param + '> X')
return ast.templatePrefix.templates[0].params[0]


@pytest.mark.parametrize(
'param,is_pack',
[('typename', False),
('typename T', False),
('typename...', True),
('typename... T', True),
('int', False),
('int N', False),
('int* N', False),
('int& N', False),
('int&... N', True),
('int*... N', True),
('int...', True),
('int... N', True),
('auto', False),
('auto...', True),
('int X::*', False),
('int X::*...', True),
('int (X::*)(bool)', False),
('int (X::*x)(bool)', False),
# TODO: the following two declarations cannot currently be parsed
# ('int (X::*)(bool)...', True),
# ('int (X::*x...)(bool)', True),
('template<typename> class', False),
('template<typename> class...', True),
])
def test_domain_cpp_template_parameters_is_pack(param: str, is_pack: bool):
ast = parse_template_parameter(param)
assert ast.isPack == is_pack