From 0a485740a87d27e3e02076634d387c0be287e6a5 Mon Sep 17 00:00:00 2001 From: Takeshi KOMIYA Date: Sat, 25 Aug 2018 01:47:34 +0900 Subject: [PATCH 1/6] docs: Fix wrong descriptions for logging APIs --- doc/extdev/logging.rst | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/doc/extdev/logging.rst b/doc/extdev/logging.rst index d0a139321..0d96c4eb9 100644 --- a/doc/extdev/logging.rst +++ b/doc/extdev/logging.rst @@ -9,9 +9,9 @@ Logging API .. autoclass:: SphinxLoggerAdapter(logging.LoggerAdapter) - .. method:: SphinxLoggerAdapter.error(level, msg, *args, **kwargs) - .. method:: SphinxLoggerAdapter.critical(level, msg, *args, **kwargs) - .. method:: SphinxLoggerAdapter.warning(level, msg, *args, **kwargs) + .. method:: SphinxLoggerAdapter.error(msg, *args, **kwargs) + .. method:: SphinxLoggerAdapter.critical(msg, *args, **kwargs) + .. method:: SphinxLoggerAdapter.warning(msg, *args, **kwargs) Logs a message on this logger with the specified level. Basically, the arguments are as with python's logging module. @@ -33,13 +33,14 @@ Logging API logger.warning('Warning happened!', location=some_node) **color** - The color of logs. By default, warning level logs are - colored as ``"darkred"``. The others are not colored. + The color of logs. By default, error level logs are colored as + ``"darkred"``, critical level ones is not colored, and warning level + ones are colored as ``"red"``. .. method:: SphinxLoggerAdapter.log(level, msg, *args, **kwargs) - .. method:: SphinxLoggerAdapter.info(level, msg, *args, **kwargs) - .. method:: SphinxLoggerAdapter.verbose(level, msg, *args, **kwargs) - .. method:: SphinxLoggerAdapter.debug(level, msg, *args, **kwargs) + .. method:: SphinxLoggerAdapter.info(msg, *args, **kwargs) + .. method:: SphinxLoggerAdapter.verbose(msg, *args, **kwargs) + .. method:: SphinxLoggerAdapter.debug(msg, *args, **kwargs) Logs a message to this logger with the specified level. Basically, the arguments are as with python's logging module. @@ -55,9 +56,8 @@ Logging API :meth:`SphinxLoggerAdapter.warning`. **color** - The color of logs. By default, debug level logs are - colored as ``"darkgray"``, and debug2 level ones are ``"lightgray"``. - The others are not colored. + The color of logs. By default, info and verbose level logs are not colored, + and deug level ones are colored as ``"darkgray"``. .. autofunction:: pending_logging() From 7748b84cc9716f0fef3cc6696103d5e8b0c13224 Mon Sep 17 00:00:00 2001 From: Jakob Lykke Andersen Date: Sat, 25 Aug 2018 12:41:44 +0200 Subject: [PATCH 2/6] C++, fix UnboundLocalError for overload refs not being found. --- sphinx/domains/cpp.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sphinx/domains/cpp.py b/sphinx/domains/cpp.py index aeeaa2122..1d20310b3 100644 --- a/sphinx/domains/cpp.py +++ b/sphinx/domains/cpp.py @@ -6678,6 +6678,7 @@ class CPPDomain(Domain): matchSelf=True, recurseInAnon=True) else: decl = ast # type: ASTDeclaration + name = decl.name s = parentSymbol.find_declaration(decl, typ, templateShorthand=True, matchSelf=True, recurseInAnon=True) From 48427ae57a6fd088c7d30c59f3ae4436b9a80082 Mon Sep 17 00:00:00 2001 From: Takeshi KOMIYA Date: Sun, 26 Aug 2018 11:28:06 +0900 Subject: [PATCH 3/6] Improve pytest header --- sphinx/__init__.py | 3 +-- tests/conftest.py | 6 ++++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/sphinx/__init__.py b/sphinx/__init__.py index 1309996bc..9debdca07 100644 --- a/sphinx/__init__.py +++ b/sphinx/__init__.py @@ -61,8 +61,7 @@ if __version__.endswith('+'): __version__ = __version__[:-1] # remove '+' for PEP-440 version spec. try: import subprocess - p = subprocess.Popen(['git', 'show', '-s', '--pretty=format:%h', - path.join(package_dir, '..')], + p = subprocess.Popen(['git', 'show', '-s', '--pretty=format:%h'], stdout=subprocess.PIPE, stderr=subprocess.PIPE) out, err = p.communicate() if out: diff --git a/tests/conftest.py b/tests/conftest.py index eef0cb185..9f46b1868 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -11,8 +11,10 @@ import os import shutil import sys +import docutils import pytest +import sphinx from sphinx.testing.path import path pytest_plugins = 'sphinx.testing.fixtures' @@ -34,8 +36,8 @@ def rootdir(): def pytest_report_header(config): - return 'Running Sphinx test suite (with Python %s)...' % ( - sys.version.split()[0]) + return ("libraries: Sphinx-%s, docutils-%s" % + (sphinx.__display_version__, docutils.__version__)) def _initialize_test_directory(session): From d32a24ae883a129a65ab7fb25036d7e45711d6d0 Mon Sep 17 00:00:00 2001 From: Jakob Lykke Andersen Date: Sun, 26 Aug 2018 11:06:23 +0200 Subject: [PATCH 4/6] C++, properly add (overloaded) symbols and params - Consider all matching symbols when adding symbols. - Only create a new symbol if no empty symbol is present. - When filling empty symbols, also add template and function parameters. Fixes sphinx-doc/sphinx#5337 --- sphinx/domains/cpp.py | 249 ++++++++++++++++++++++++++++-------------- 1 file changed, 170 insertions(+), 79 deletions(-) diff --git a/sphinx/domains/cpp.py b/sphinx/domains/cpp.py index 1d20310b3..a3be2276f 100644 --- a/sphinx/domains/cpp.py +++ b/sphinx/domains/cpp.py @@ -3603,6 +3603,8 @@ class SymbolLookupResult(object): class Symbol(object): + debug_lookup = False + def _assert_invariants(self): # type: () -> None if not self.parent: @@ -3648,43 +3650,9 @@ class Symbol(object): self.parent._children.append(self) if self.declaration: self.declaration.symbol = self - # add symbols for the template params - # (do it after self._children has been initialised - if self.templateParams: - for p in self.templateParams.params: - if not p.get_identifier(): - continue - # only add a declaration if we our selfs are from a declaration - if declaration: - decl = ASTDeclaration('templateParam', None, None, p) - else: - decl = None - nne = ASTNestedNameElement(p.get_identifier(), None) - nn = ASTNestedName([nne], [False], rooted=False) - self._add_symbols(nn, [], decl, docname) - # add symbols for function parameters, if any - if declaration is not None and declaration.function_params is not None: - for p in declaration.function_params: - if p.arg is None: - continue - nn = p.arg.name - if nn is None: - continue - # (comparing to the template params: we have checked that we are a declaration) - decl = ASTDeclaration('functionParam', None, None, p) - assert not nn.rooted - assert len(nn.names) == 1 - identOrOp = nn.names[0].identOrOp - Symbol(parent=self, identOrOp=identOrOp, - templateParams=None, templateArgs=None, - declaration=decl, docname=docname) - def remove(self): - if self.parent is None: - return - assert self in self.parent._children - self.parent._children.remove(self) - self.parent = None + # Do symbol addition after self._children has been initialised. + self._add_template_and_function_params() def _fill_empty(self, declaration, docname): # type: (ASTDeclaration, unicode) -> None @@ -3697,6 +3665,46 @@ class Symbol(object): self.declaration.symbol = self self.docname = docname self._assert_invariants() + # and symbol addition should be done as well + self._add_template_and_function_params() + + def _add_template_and_function_params(self): + # Note: we may be called from _fill_empty, so the symbols we want + # to add may actually already be present (as empty symbols). + + # add symbols for the template params + if self.templateParams: + for p in self.templateParams.params: + if not p.get_identifier(): + continue + # only add a declaration if we our self are from a declaration + if self.declaration: + decl = ASTDeclaration('templateParam', None, None, p) + else: + decl = None + nne = ASTNestedNameElement(p.get_identifier(), None) + nn = ASTNestedName([nne], [False], rooted=False) + self._add_symbols(nn, [], decl, self.docname) + # add symbols for function parameters, if any + if self.declaration is not None and self.declaration.function_params is not None: + for p in self.declaration.function_params: + if p.arg is None: + continue + nn = p.arg.name + if nn is None: + continue + # (comparing to the template params: we have checked that we are a declaration) + decl = ASTDeclaration('functionParam', None, None, p) + assert not nn.rooted + assert len(nn.names) == 1 + self._add_symbols(nn, [], decl, self.docname) + + def remove(self): + if self.parent is None: + return + assert self in self.parent._children + self.parent._children.remove(self) + self.parent = None def clear_doc(self, docname): # type: (unicode) -> None @@ -3948,8 +3956,20 @@ class Symbol(object): # Used for adding a whole path of symbols, where the last may or may not # be an actual declaration. + if Symbol.debug_lookup: + print("_add_symbols:") + print(" tdecls:", templateDecls) + print(" nn: ", nestedName) + print(" decl: ", declaration) + print(" doc: ", docname) + def onMissingQualifiedSymbol(parentSymbol, identOrOp, templateParams, templateArgs): # type: (Symbol, Union[ASTIdentifier, ASTOperator], Any, ASTTemplateArgs) -> Symbol + if Symbol.debug_lookup: + print(" _add_symbols, onMissingQualifiedSymbol:") + print(" templateParams:", templateParams) + print(" identOrOp: ", identOrOp) + print(" templateARgs: ", templateArgs) return Symbol(parent=parentSymbol, identOrOp=identOrOp, templateParams=templateParams, templateArgs=templateArgs, declaration=None, @@ -3964,54 +3984,119 @@ class Symbol(object): recurseInAnon=True, correctPrimaryTemplateArgs=True) assert lookupResult is not None # we create symbols all the way, so that can't happen - # TODO: actually do the iteration over results, though let's find a test case first - try: - symbol = next(lookupResult.symbols) - except StopIteration: - symbol = None - - if symbol: - if not declaration: - # good, just a scope creation - return symbol - if not symbol.declaration: - # If someone first opened the scope, and then later - # declares it, e.g, - # .. namespace:: Test - # .. namespace:: nullptr - # .. class:: Test - symbol._fill_empty(declaration, docname) - return symbol - # It may simply be a function overload, so let's compare ids. - isRedeclaration = True - candSymbol = Symbol(parent=lookupResult.parentSymbol, - identOrOp=lookupResult.identOrOp, - templateParams=lookupResult.templateParams, - templateArgs=lookupResult.templateArgs, - declaration=declaration, - docname=docname) - if declaration.objectType == "function": - newId = declaration.get_newest_id() - oldId = symbol.declaration.get_newest_id() - if newId != oldId: - # we already inserted the symbol, so return the new one - symbol = candSymbol - isRedeclaration = False - if isRedeclaration: - # Redeclaration of the same symbol. - # Let the new one be there, but raise an error to the client - # so it can use the real symbol as subscope. - # This will probably result in a duplicate id warning. - candSymbol.isRedeclaration = True - raise _DuplicateSymbolError(symbol, declaration) - else: + symbols = list(lookupResult.symbols) + if len(symbols) == 0: + if Symbol.debug_lookup: + print(" _add_symbols, result, no symbol:") + print(" templateParams:", lookupResult.templateParams) + print(" identOrOp: ", lookupResult.identOrOp) + print(" templateArgs: ", lookupResult.templateArgs) + print(" declaration: ", declaration) + print(" docname: ", docname) symbol = Symbol(parent=lookupResult.parentSymbol, identOrOp=lookupResult.identOrOp, templateParams=lookupResult.templateParams, templateArgs=lookupResult.templateArgs, declaration=declaration, docname=docname) - return symbol + return symbol + + if Symbol.debug_lookup: + print(" _add_symbols, result, symbols:") + print(" number symbols:", len(symbols)) + + if not declaration: + if Symbol.debug_lookup: + print(" no delcaration") + # good, just a scope creation + # TODO: what if we have more than one symbol? + return symbols[0] + + noDecl = [] + withDecl = [] + for s in symbols: + if s.declaration is None: + noDecl.append(s) + else: + withDecl.append(s) + if Symbol.debug_lookup: + print(" #noDecl: ", len(noDecl)) + print(" #withDecl:", len(withDecl)) + # assert len(noDecl) <= 1 # we should fill in symbols when they are there + # TODO: enable assertion when we at some point find out how to do cleanup + # With partial builds we may start with a large symbol tree stripped of declarations. + + # First check if one of those with a declaration matches. + # If it's a function, we need to compare IDs, + # otherwise there should be only one symbol with a declaration. + def makeCandSymbol(): + if Symbol.debug_lookup: + print(" begin: creating candidate symbol") + symbol = Symbol(parent=lookupResult.parentSymbol, + identOrOp=lookupResult.identOrOp, + templateParams=lookupResult.templateParams, + templateArgs=lookupResult.templateArgs, + declaration=declaration, + docname=docname) + if Symbol.debug_lookup: + print(" end: creating candidate symbol") + return symbol + if len(withDecl) == 0: + candSymbol = None + else: + candSymbol = makeCandSymbol() + + def handleDuplicateDeclaration(symbol, candSymbol): + if Symbol.debug_lookup: + print(" redeclaration") + # Redeclaration of the same symbol. + # Let the new one be there, but raise an error to the client + # so it can use the real symbol as subscope. + # This will probably result in a duplicate id warning. + 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: + print(" candId:", candId) + for symbol in withDecl: + oldId = symbol.declaration.get_newest_id() + if Symbol.debug_lookup: + print(" oldId: ", oldId) + if candId == oldId: + handleDuplicateDeclaration(symbol, candSymbol) + # (not reachable) + # no candidate symbol found with matching ID + # if there is an empty symbol, fill that one + if len(noDecl) == 0: + if Symbol.debug_lookup: + print(" no match, no empty, candSybmol is not None?:", candSymbol is not None) # NOQA + if candSymbol is not None: + return candSymbol + else: + return makeCandSymbol() + else: + if Symbol.debug_lookup: + print(" no match, but fill an empty declaration, candSybmol is not None?:", candSymbol is not None) # NOQA + if candSymbol is not None: + candSymbol.remove() + # assert len(noDecl) == 1 + # TODO: enable assertion when we at some point find out how to do cleanup + # for now, just take the first one, it should work fine ... right? + symbol = noDecl[0] + # If someone first opened the scope, and then later + # declares it, e.g, + # .. namespace:: Test + # .. namespace:: nullptr + # .. class:: Test + symbol._fill_empty(declaration, docname) + return symbol def merge_with(self, other, docnames, env): # type: (Symbol, List[unicode], BuildEnvironment) -> None @@ -6598,7 +6683,7 @@ class CPPDomain(Domain): def process_doc(self, env, docname, document): # type: (BuildEnvironment, unicode, nodes.Node) -> None # just for debugging - # print(docname) + # print("process_doc:", docname) # print(self.data['root_symbol'].dump(0)) pass @@ -6608,6 +6693,12 @@ class CPPDomain(Domain): def merge_domaindata(self, docnames, otherdata): # type: (List[unicode], Dict) -> None + # print("merge_domaindata:") + # print("self") + # print(self.data['root_symbol'].dump(0)) + # print("other:") + # print(otherdata['root_symbol'].dump(0)) + self.data['root_symbol'].merge_with(otherdata['root_symbol'], docnames, self.env) ourNames = self.data['names'] From 72c135600c1e232c0fe910121bedac6093cc9f50 Mon Sep 17 00:00:00 2001 From: Jakob Lykke Andersen Date: Sun, 26 Aug 2018 11:18:40 +0200 Subject: [PATCH 5/6] Update changes --- CHANGES | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES b/CHANGES index ede52706d..e0aa26272 100644 --- a/CHANGES +++ b/CHANGES @@ -22,6 +22,8 @@ Bugs fixed * html: search box overrides to other elements if scrolled * i18n: warnings for translation catalogs have wrong line numbers (refs: #5321) * #5325: latex: cross references has been broken by multiply labeled objects +* C++, fixes for symbol addition and lookup. Lookup should no longer break + in partial builds. See also #5337. Testing -------- From 2a544b4ec3bce0bd6c11d7fda8df5a04013eca1f Mon Sep 17 00:00:00 2001 From: Jakob Lykke Andersen Date: Sun, 26 Aug 2018 13:17:53 +0200 Subject: [PATCH 6/6] C++, conditionally disable test on sys.maxunicode --- tests/test_domain_cpp.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/test_domain_cpp.py b/tests/test_domain_cpp.py index 0fe8d5c19..b8856824b 100644 --- a/tests/test_domain_cpp.py +++ b/tests/test_domain_cpp.py @@ -10,6 +10,7 @@ """ import re +import sys import pytest from six import text_type @@ -137,8 +138,9 @@ def test_expressions(): exprCheck(p + "'\\x0A'", t + "10") exprCheck(p + "'\\u0a42'", t + "2626") exprCheck(p + "'\\u0A42'", t + "2626") - exprCheck(p + "'\\U0001f34c'", t + "127820") - exprCheck(p + "'\\U0001F34C'", t + "127820") + if sys.maxunicode > 65535: + exprCheck(p + "'\\U0001f34c'", t + "127820") + exprCheck(p + "'\\U0001F34C'", t + "127820") # TODO: user-defined lit exprCheck('(... + Ns)', '(... + Ns)')