Make generator work for Qt 5.15#146
Conversation
|
@jbowler - maybe you can have a look. |
e08afdd to
cef6179
Compare
| case '{': | ||
| case '[': | ||
| case '(': |
There was a problem hiding this comment.
Couldn't you just ignore other than curly braces? They shouldn't matter.
There was a problem hiding this comment.
It feels a bit uncomfortable for me to have no other checks in place, but ok. The code gets a lot simpler this way.
|
The generator made with Qt5.15.11LTS fails (aborts) with the official Qt downloads for 6.5.3 and 6.6.0:
(That's a period character.) That's the last line before the program aborts. Despite this it succeeds with my Gentoo-patched variants of the same Qt versions. The line before the above line is a MetaJavaBuilder "WARNING" about:
(I'm copying these lines by hand, so there might be small typos). Since these are the official Qt installations (for AMD64 GNU-Linux) it should be possible to repro them anywhere. |
Yes, I also found this. I haven't looked yet what the actual problem is, but the problematic template code is in template<class T>
struct QmlResolved<T, std::void_t<typename T::QmlForeignType>>
{
using Type = typename std::conditional<
QmlTypeHasMarker<T, decltype(&T::qt_qmlMarker_foreign)>::value,
typename T::QmlForeignType, T>::type;
};(after commenting this out, the parser runs through), probably somewhere where that template is instantiated. |
|
I found that after I apply a couple of patches to the generator that I made before, the generator also passes (that's under Windows), though I'm not sure yet what exactly is the problem. |
It does need a patch; the qqmlprivate.h files are identical in the Gentoo copy of the headers that succeeds and the Qt downloaad that fails. It sounds like a "use-after-free" and I've seen failures before of that type; a very minor change to the code fixes the problem as does a minor change in the headers. There is a potential use-after-free in the topo sort which should be removed on principal but that might not be the issue. |
|
I'll try the fix (well, it was a rewrite) that I came up with. |
|
The problem is the ellipsis inside a type declaration that is not expected. I added my commits to make it work, and @usiems will fix the actual type parser to allow for an ellipsis. |
|
The crash is not a real crash, but a qFatal in typeparser.cpp, because Scanner.nextToken() chokes on '.', which didn't occur in any types in Qt 5.15, but now appears in templates types because of ellipsis ("..."). So we have to correctly parse the ellipsis in templates to fix this. |
|
I can't see an ellipsis in that template; the only one outside comments in the whole file (either version) is in the template for I'll submit the fix anyway, but it's a rewrite of the function. |
461f54d to
7be0dd9
Compare
|
With those commits the crash(qFatal?) against 6.6.0 is fixed however 6.5.3 is crashing with either qFatal (I assume) or a SEGV. Here's the output: The 'gentoo' variant looks like a qFatal, the official variant shows heap corruption but the message varies; with my topo sort patch it ends up as 'corrupted size vs. prev_size'. |
7be0dd9 to
d476532
Compare
|
Maybe it's time to setup some Qt6 CI tests, at least in a branch, to catch problems with different versions... |
|
Here's the traceback of the crash. This is building the generator with gcc on Linux (gentoo) against Qt5.15.11LTS and then running it against a download of Qt6.5.3 from Qt (i.e. the official free download): Contrary to my previous report the change in #148 seems to make this go away. From the output the crash appears to be below |
Yes, I agree. I doubt that the problem has been introduced with this PR, but it certainly something to be checked and fixed. |
|
@jbowler - I think that the code is now in a good shape to be merged (for Qt 5). A few more crashes have been fixed in the latest commits, the code generation is now stable, and the code generated for Qt5.15 looks good. If you agree, I will merge this, and further changes related to Qt6 will go into separate PRs. This then can be used as base for newly generated source, for Qt <= Qt5.15. The new CI checks worked on by @YuriUfimtsev will also help to verify this. |
this way we can avoid some parser errors. This is implemented by looking for matching braces. I hope there is no case where unmatched braces are allowed (strings and comments should be handled by the tokenizer).
…peMap Previously the includes and generated methods for e.g. QItemSelection would change depending on the order in which the AbstractMetaClass entries were generated, which in turn would change which definition of QList would be seen first. I now try to mark the "main" definition containing the actual methods and such. For Qt 5.15 this seems to work at least, but I admit that I still don't understand the generator completely.
- two consecutive right angle brackets had been handled as right-shift operator - replace right-shift with two brackets in parser where the context is clear
- ignore default template parameters
This explicit new/delete, storage leaks (missing delete) and iteration over a QHash while modifying it (deleting specific keys in an iterator).
The previous code assumed that any parsing step would only add one token at a time, which isn't true. This would lead to crashes if the size doubling threshold was skipped at one point.
bd378b0 to
2aecabb
Compare
I've had the 6.5.3 crash sitting under a debugger for almost a week. I'll try the test again with the latest changes. I'm pretty certain it isn't 6.5.3 specific. It occurs in the lexical analysis somewhere low down in a QHash update but it happens with CONFIG+=DEBUG as well so it is very unlikely to be a memory overwrite. I think this needs to have been fixed before the merge. Anyway, I'll test again now there have been substantial changes. |
|
FWIW I've just discovered that the original crash disappears if I |
|
With the latest HEAD, i.e. 2aecabb all my generation tests succeed. This just means that the generator works without explicitly crashing; I don't know whether the generated_cpp is valid/usable! The change that originated the crash is now: I note that the last fix (the HEAD, as referenced above) fixes addition of two tokens at once in the lexer and the above commit does, indeed, change the code to add two rather than one token for >>. The crash is when the lexical analysis cursor is precisely here:
I.e. it is when the cursor is just before the "d" of detail, where I highlighted the text. This explains the crash to me enough for me to be happy with a merge back to the main line. |
|
Thank you - merging then! |
|
Ok, but did anyone try the headers generated by a Qt5.15.11 build using the official Qt5.11.3 header files? When I try that I get this:
The build with the 5.15.11 header files installed by Gentoo succeeds, but maybe I don't have somelike (like Qml) installed; I don't use Qml. |
|
I think it may just be a feature of how I had to set up the official 5.15.11 headers; Qt5.15.2 succeeds, but it would be a good idea for someone else with official 5.15.11 headers to check this out. |
Assorted fixes to make the generator work for Qt 5.15. It's probably best to review the commits individually.
Most of the work consisted of debugging the code paths for classes that didn't come out right and then finding a fix that works. Since I only understand part of the generator (e.g. handling of template parameters still eludes me in parts), I tried to stick to local fixes.