Details
-
Bug
-
Resolution: Unresolved
-
P3: Somewhat important
-
6.8
-
None
-
266c20ec0 (dev)
Description
The issue started from the discovering of the following error-prone behavior, which is currently allowed by the FileLocations::addRegion API
//Let's assume we have fresh tree (with default Info) // if we do the following: FileLocations::addRegion(tree, FileLocationRegion::MainRegion, SourceLocation(0,7)); FileLocations::addRegion(tree, FileLocationRegion::MainRegion, SourceLocation(0,3)); // .fullRegion won't be properly updated after MainRegion add and will stay (0,7) tree->info().fullRegion == SourceLocation(0,7)
Another potential issue is if we:
1. Do not set fullRegion or MainRegion explicitly
2. Set for example any token region, let's say to SourceLocation(0,7). Which will indeed set fullRegion and MainRegion to SourceLocation(0,7).
3. Then reset this token region to let's say SourceLocation(0,3)
4. What value do we expect from MainRegion and .fullRegion at this point?
I think this function needs to be refined to be more specific and explicit about its usages and expectations. This should be reflected in the name, signature and also more assertive implementation.
As it was assumed that this is only used when creating a new AST -> proposed to just add an assert, this leads to some test failures, for example QQmlJS::Dom::TestDomItem::testLoadNoDep(). Which I guess links to the fact that when we reload the file, FileLocations still exist at the particular path, but are planned to be reassigned when the DOM is reconstructed? But it might be a wrong guess and this should be researched further and the function should be adjusted and documented properly.
Attachments
Gerrit Reviews
For Gerrit Dashboard: QTBUG-131288 | ||||||
---|---|---|---|---|---|---|
# | Subject | Branch | Project | Status | CR | V |
605824,3 | WIP: Adding Assert for reassigning of MainRegion leads to some failures | dev | qt/qtdeclarative | Status: NEW | -2 | 0 |
605969,1 | WIP: Adding Assert for shrinking of MainRegion leads to some failures | dev | qt/qtdeclarative | Status: NEW | -2 | 0 |
604373,10 | QmlDom improve reliability of FileLocations | dev | qt/qtdeclarative | Status: MERGED | +2 | 0 |