summaryrefslogtreecommitdiff
path: root/src/qdoc/cppcodeparser.cpp
diff options
context:
space:
mode:
authorLuca Di Sera <luca.disera@qt.io>2022-07-07 16:51:54 +0200
committerLuca Di Sera <luca.disera@qt.io>2022-07-08 14:37:55 +0200
commitcc251c688575728227f9592045eb8d819c582b47 (patch)
tree0f84dfbb8464c5d63bc647a084ac6145bde40181 /src/qdoc/cppcodeparser.cpp
parentc6f58972ec1cc3f3fbd80333d78055291462d394 (diff)
downloadqttools-cc251c688575728227f9592045eb8d819c582b47.tar.gz
QDoc: Assume that aggregates have a single meaningful include file
QDoc's `Aggregates`, which are used to internally represent "containing" elements, such as cpp classes, have a concept of "include files". "include files" represent the name of a file or similar construct from which the aggregate can be obtained the sources. For example, the name an header file declaring a cpp class. Aggregates, up to now, exposed an interface that allowed to set multiple "include file" for each aggregate. Furthermore, QDoc allows the user to manually specify include files through the use of the "\inheaderfile" command. Those include files were later used by QDoc to show information about the aggregate, In particular, the generator would show an "include requisite" to help a user understand where to import the aggregate from. The multiplicity of the include files had effects on later parts of the code that either set or read them. For example, the generators whose code required handling the multiplicity to show the required information on multiple lines. In practice, this never took effect as, at least in Qt's codebase, no aggregate has more than 1 include file. While it is not unthinkable of wanting to have more than one include file, we consider improbable for it to happen. In general, include file will mostly be used for classes, which we expect to be able to be meaningfully included from a specific file only. As such, the interface for `Aggregates` is now modified to expose a single include file per aggregate. The previous methods were removed and a singular version of them was introduced. The internal state of `Aggregate` was modified to take this into account storing a single optional include file instead of a vector of them. A singular include file was set during the construction of `HeaderNode`. The constructor is now modified to use the new interface. Include files are then generally set only during the code parsing phase. In particular, each file-name coming from an "\inheaderfile" command was appended to the vector of include files. Furthermore, each aggregate either inherited the include files of one of its parent or generated one from the aggregate's name when no sensible parent was available, unless a previous "\inheaderfile" command for the aggregate was encountered. The code is now changed to take into account the lack of multiplicity. The semantic is mostly preserved from the previous version and, in the case of the Qt's codebase, is preserved entirely due to no aggregate with more than one include file ever being produced. Some slight semantic breakages are, albeit, included with this change. Previously, if the user provided more than one "\inheaderfile" command, the arguments from all of them would be preserved and saved in the relevant aggregate. With the current change, each "\inheaderfile" after the first will overwrite the preceding value, such that only the last encountered command will be taken, effectively, into account. This breakage would currently be opaque for the user. It is later advised to better expose this behavior through changes to `DocParser`, albeit those changes are not expected to be performed right away as a more thorough inspection of `DocParser` must be made in general. Furthermore, the previous version would set include files that could be the empty string, specifically as they came from an "\inheaderfile" command. The new version ignore empty include files, such that it is ensured that, if an include file was ever provided, that include file is not an empty string. This breakage improves on the previous version, where later user of the include files had to perform possibly costly filtering operations to avoid empty strings, who are expected to have no meaning as an include file. To better support this change and partially help in exposing this semantic, the new interface for the include file exposes an std::optional and informally guarantees, but does not enforce, that if a value is ever set it is not the empty string. The changes to the code were made so as to ensure that this will be the case as for the current codebase. A more thorough enforcement was avoided as it would require deeper changes that are currently unfeasible. User of the include file, namely the generators, were modified to take into account the lack of multiplicity. The generators previously depended on `CodeMarker::markedUpIncludes` to flatten the vector of include files into a representable string. `CodeMarker::markedUpIncludes` is now supplanted with `CodeMarker::markedUpInclude`, its singular version, to take into account the lack of multiplicity. The semantics of the method is preserved and is expected to be equivalent to a call to `CodeMarker::markedUpIncludes` for vectors of include files with a single element, with the exclusion of a trailing newline. Small clean up changes were made to `HtmlGenerator::addIncludeFilesToMap`, which is the main consumer of include file for the HTML format. The method was renamed to its singular version. Some now unnecessary operations, such as the filtering of include files that were the empty string, are now removed. The `text` parameter of the method, previously a `Text*`, is now modified to a reference. The `Text` that is passed to the method is always created in its immediate scope and is never a nullptr, allowing us to remove and unnecessary check. Indeed, calling the method with a nullptr would generally be a no-op, such that there would be no meaning to it in the first place (and there was no flow-requirement for it to be a no-op). A slight change to the laying out of the table in `HtmlGenerator::generateTheTable` was included to better divide the lines of the table. The regression files for `tst_generatedOutput` were regenerated to take into account the additional newlines in the tables. Change-Id: Idebca8a74444328cd21c83be9e3f3b1706907f84 Reviewed-by: Topi Reiniƶ <topi.reinio@qt.io>
Diffstat (limited to 'src/qdoc/cppcodeparser.cpp')
-rw-r--r--src/qdoc/cppcodeparser.cpp39
1 files changed, 33 insertions, 6 deletions
diff --git a/src/qdoc/cppcodeparser.cpp b/src/qdoc/cppcodeparser.cpp
index 26d6b0f9f..6c1959503 100644
--- a/src/qdoc/cppcodeparser.cpp
+++ b/src/qdoc/cppcodeparser.cpp
@@ -479,8 +479,21 @@ void CppCodeParser::processMetaCommand(const Doc &doc, const QString &command,
{
QString arg = argPair.first;
if (command == COMMAND_INHEADERFILE) {
- if (node->isAggregate())
- static_cast<Aggregate *>(node)->addIncludeFile(arg);
+ // TODO: [incorrect-constructs][header-arg]
+ // The emptiness check for arg is required as,
+ // currently, DocParser fancies passing (without any warning)
+ // incorrect constructs doen the chain, such as an
+ // "\inheaderfile" command with no argument.
+ //
+ // As it is the case here, we require further sanity checks to
+ // preserve some of the semantic for the later phases.
+ // This generally has a ripple effect on the whole codebase,
+ // making it more complex and increasesing the surface of bugs.
+ //
+ // The following emptiness check should be removed as soon as
+ // DocParser is enhanced with correct semantics.
+ if (node->isAggregate() && !arg.isEmpty())
+ static_cast<Aggregate *>(node)->setIncludeFile(arg);
else
doc.location().warning(QStringLiteral("Ignored '\\%1'").arg(COMMAND_INHEADERFILE));
} else if (command == COMMAND_OVERLOAD) {
@@ -962,14 +975,28 @@ void CppCodeParser::processMetaCommands(NodeList &nodes, DocList &docs)
checkModuleInclusion(node);
if (node->isAggregate()) {
auto *aggregate = static_cast<Aggregate *>(node);
- if (aggregate->includeFiles().isEmpty()) {
+
+ if (!aggregate->includeFile()) {
Aggregate *parent = aggregate;
while (parent->physicalModuleName().isEmpty() && (parent->parent() != nullptr))
parent = parent->parent();
+
if (parent == aggregate)
- aggregate->addIncludeFile(aggregate->name());
- else
- aggregate->setIncludeFiles(parent->includeFiles());
+ // TODO: Understand if the name can be empty.
+ // In theory it should not be possible as
+ // there would be no aggregate to refer to
+ // such that this code is never reached.
+ //
+ // If the name can be empty, this would
+ // endanger users of the include file down the
+ // line, forcing them to ensure that, further
+ // to there being an actual include file, that
+ // include file is not an empty string, such
+ // that we would require a different way to
+ // generate the include file here.
+ aggregate->setIncludeFile(aggregate->name());
+ else if (aggregate->includeFile())
+ aggregate->setIncludeFile(*parent->includeFile());
}
}
}