[Development] Is this QML parser behaviour correct?

[Simon Hausmann]

On Monday 21. October 2013 13.33.43 Martin Smith wrote:
> The QML parser has callbacks for processing the Import list and for
> processing individual imports. In qdoc, I used the one that gets the entire
> import list:
> 
>  virtual bool visit(UiImportList *) { return true; }
>  virtual bool visit(UiImport *) { return true; }
> 
> I assumed it would bet called once, when the entire import list had been
> parsed. My import list has three imports in it.
> 
> import QtQuick 2.1
> import QtQuick.Controls 1.1
> import QtQuick.Controls.Private 1.0
> 
> The import list visit() function is indeed called with a pointer to the
> first entry in the list of three imports, and qdoc processes all three
> imports.
> 
> But then it is called again pointing at the second import in the list, and
> qdoc processes the last two imports again. And then it is called a third
> time pointing at the third entry in the list, and qdoc processes the last
> entry a third time.
> 
> Is that behaviour correct, or should the parser only visit the list once
> when it is complete?

I think something in your analysis doesn't match up, because the first
overload of visit that you mentioned, the one that takes a UiImportList*
doesn't exist anymore. So nobody should be calling it - it was removed
in https://codereview.qt-project.org/#change,63591

... unless you're not using the current grammar from qmldevtools but a copy of it?

However it makes sense that the second overload you mentioned is called three times.

Given that the imports in the grammar do not produce a recursive data structure and
that this part of the grammar is unlikely to be subject to drastic changes, you could also
just process the imports directly without a visitor, with a loop like this:

   for (QQmlJS::AST::UiHeaderItemList *headerItemIt = uiProgram->headers; headerItemIt; headerItemIt = headerItemIt->next) {
        QQmlJS::AST::UiImport *importNode = QQmlJS::AST::cast<QQmlJS::AST::UiImport *>(headerItemIt->headerItem);
        if (!importNode)
            continue;
    ...
}

That said, I see why UiImportList* with the old grammar is called three times,
as the default accept implementation doesn't do the traversal itself but
simply recursively calls accept(next). For other list types, accept tends to do
the traversal itself in accept iteratively. 

I don't know the reason for the inconsistency here but I think it makes sense
to make it consistent and thus fix the behaviour to match your expectations.


Simon