[analyzer]: null issues when running Zapp (which embeds the analysis server on a web client) #54468
Comments
|
Right so I got it 'working' with some patches to the following (highlighted code = added by me):
That fixed the above issue (for some reason
As you can see with the above 2 patches the analyzer is now 'working' (doesn't analyze anything else except dart built-ins, e.g. Flutter doesn't exist/resolve) :
These 2 patches are obviously huge code-smells and working around a problem elsewhere which I still don't know what it is, but putting patches here incase it maybe gives some indication of what the underlying problem is. Note: there's still something not working here, as above it now doesn't analyze anything else except dart core builtins, e.g. Flutter doesn't exist/resolve, seeing some weird paths in the analyzer, e.g. compare this build broken paths:
... with paths correct on Dart 2.19.2 build:
... maybe paths/libraries missing issue? |
|
There's a lot of information here, but thanks for collecting it all, with relevant source and all, as well as for Zapp :D Since there are two primary issues here, I would consider splitting this into two issues if you could. That way the relevant parties for each can more easily identify what's relevant to them and the discussion can be more focused. That would also allow for a more clear resolution/stopping point for each issue as well. |
lrhn
added
web-dart2js
area-web
|
FWIW, dart2js has this: sdk/pkg/compiler/lib/src/js_model/records.dart Lines 490 to 499 in a7e1e26 Your stack trace shows that this function can indeed be called via |
Bug: #54468 Change-Id: Idb7b794a53c878285d00308394cb1206b0136826 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/344380 Reviewed-by: Stephen Adams <sra@google.com> Commit-Queue: Mayank Patke <fishythefish@google.com>
|
Thanks That fix landed in ff9e863, so now it seems like the next issues are some Your mentioned |
|
It seems that there is too much context. |
@scheglov Sorry I know it's a lot of information 😓 to clarify it crashes on any code analyzed from a Flutter app to even a basic print hello world Dart main with no imports. My assumption (maybe incorrect) is that type inference is failing on Dart core libraries as they're returning
Hey @parlough! Thanks for taking a look (it was great chatting in Prague too albeit briefly sorry!), good idea - since that first issue seems to have a fixed merged now should I maybe just edit original issue to remove context around that? I am working on fully open sourcing Zapp & SDK builds in the meantime since it's probably the best way to provide a repro, but may take a few days since it's quite tightly coupled to a few things that would still make it difficult for anyone to clone and run even if open sourced. |
|
Well, given that the analyzer when running in Dart VM can analyze "hello world", it might be:
The specific code change in |
You may be right here, looking at I'm debugging to confirm path issues, will report back.
|
|
Ok so after @scheglov's comment I went debugging element hashCodes, the
...note in the screenshot that the element we're trying to find has an extra Edit: I patched to remove the empty components (just as a test) and the hashCodes are matching now, however
Edit 2: Patching to resolve elements by
So most likely I've not found root cause of overall issue yet |
|
I'm not 100% sure, but the extra empty component in Given the warning with |
I got side tracked here and patched to see if I could get further, I did and things got to the 'kinda working' state (including packages!): but of course it wasn't a valid fix or anything and me just debugging things and trying to understand what's going on better.
Yep, I'm pretty sure it's Element and |
sigmundch
added
area-analyzer
sigmundch
changed the title
RecordGetterData.forEachParameter
|
FYI - since the dart2js issue is complete, I've updated this bug title and labels. |
I'm pretty much out of ideas now 😞 I also tried reverting a bunch of commits around JS interop/runtime changes between 2.19.2 and 3.0.0 in the following files:
but no joy. Open to suggestions |
|
Update: Tried Flutter The analyzer issue above still persists though and I've also noticed our compiler is now broken too on this latest Dart version (not investigated yet) so I feel like I'm fighting a losing battle here and not sure whether to continue with this anymore |
|
I don't think we can really help with this unless you provide some straightforward reproduction, e.g. extract the problematic code into a minimal project which could easily be run locally. That would at least make it possible for us to debug it - rather than look at screenshots and try to guess what is going wrong in such a huge code base (the answer is: anything and everything could have gone wrong). If you depend on running analyzer compiled to JS (which, I must stress, is not something we directly support) the best way of approach this and save yourself a lot of headache in the future would be to invest in making analyzer test suite run compiled to JS. Then you could at least have a chance to catch and debug breakages on a smaller scale. |
|
Looking forward to a fully open sourced Zapp & SDK build. That will make it much easier for us to reproduce and debug the issue. |
This comment was marked as off-topic.
This comment was marked as off-topic.
|
There is no reason to leave off-topic comments on this issue and spam people. We can't do much unless we have open-source reproduction of the problem. |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
|
I played around with getting the devcompiler to run when compiled with dart2js. I fixed this in a patch, shared here: I didn't really try to land it in the SDK. But given how cheap it is, I don't see any harm to doing so. Though we might have to ask someone who owns this part of the code. In particular the problem is probably: And the behavior is probably very hard to debug by any other means than simply doing a bisection 🤣 |
|
The patch I have above is based on 4722e4d, it's possible that other issues have arised since then. |
|
Interesting, why is it for |
|
@jonasfj nice detective work! makes sense. @scheglov in JavaScript bitwise operations are performed on 32-bit integers, but the rest of the arithmetic is performed on doubles. This means you can use 53 mantissa bits to represent integers precisely - giving range |
…elements. Bug: #54468 Change-Id: I04a221526dcc7e380e7e9c84f8bb8b2fed52f4eb Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/355823 Reviewed-by: Brian Wilkerson <brianwilkerson@google.com> Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
|
@mraleph informed me that And indeed, here is statistics for running over I think we don't want to pay the price of using |
I think you don't need to store it in 2 int (2x 30 bits) to avoid a JS issue. You just need to use 2 int (2x 30 bits) while you perform the bitwise operations (in the JS implementation) and then store it using a normal int, which will be 64 bits or 53 bits depending on the platform. I believe that a limit of 53 constants (53 bits) is enough and won't change the current number of objects or memory utilization. Note that the heap utilization increases because a Record is a new object/struct with 2 ints + a reference to it. The current implementation uses just an int, which has the same cost as a reference to an object. |
…p to 60 elements." This reverts commit 492e407. Reason for revert: too expensive Original change's description: > Issue 54468. Use (int, int) as backing storage for EnumSet, up to 60 elements. > > Bug: #54468 > Change-Id: I04a221526dcc7e380e7e9c84f8bb8b2fed52f4eb > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/355823 > Reviewed-by: Brian Wilkerson <brianwilkerson@google.com> > Commit-Queue: Konstantin Shcheglov <scheglov@google.com> Bug: #54468 Change-Id: I9f8b6755c8f8fce4e23dcb8533ef680eb8516dba Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/355885 Auto-Submit: Konstantin Shcheglov <scheglov@google.com> Reviewed-by: Phil Quitslund <pquitslund@google.com> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Commit-Queue: Phil Quitslund <pquitslund@google.com>
srawlins
added
the
type-bug
|
Dirty hack to only use https://dart-review.googlesource.com/c/sdk/+/366822 We could probably make it even simpler by using Another option is to make the whole But conditional imports is not that bad either. |
|
Fixed in 160b9ea |
Hello 👋
Firstly, I understand our use case is probably not one the Dart team will directly support but
zapp.runhas been working for some time on lots of Dart versions <= 2.19.2 - but something fundamentally has changed either in dart2js, JS interop or Analyzer LSP since Dart 3.I've spent months trying to figure out why and try to boil it down to one small reproducible example but I've been unable to do so so I'm creating this issue with what I have currently and hoping someone can help out here either directly or suggesting patches I can apply to the Dart SDK that may fix it (I'm fully setup to patch and compile the Dart SDK locally).
Context: Zapp is a free-for-all open IDE to try out and easily share Dart / Flutter code with ~30k users and we'd love to keep it going but we're blocked and don't see any way forward, please help 🙏
Current issues:
The code that's being compiled isn't doing anything detailed itself or using records directly, this is for zapp.run (online Flutter/Dart Sandbox IDE) and historically this has worked, since Dart 3 we've been unable to compile our web version of the Dart analyzer anymore (we're still stuck on Dart 2.19.2 😭 ) with various issues relating to dart2js internal changes, this being one of them.
Our code in
zapp_analyzer.dartis a thin wrapper aroundanalysis_server, it imports the following:...and starting the LSP client with something like:
...and we're compiling as follows:
...which errors with the above. However; if we then try compile with
-O1optimization level it then compiles to JS fine, but then we get a strange runtime issue once the analyzer has read all the files and starts analyzing:If we debug/inspect the line it errors in this JS version of
DefaultValueResolver.executable()then we can seet1is null:and this is the SDK source relating to that code above (
./pkg/analyzer/lib/src/summary2/default_value_resolver.dart):Inspecting the element we see it relates to
_Completerconstructor fromfuture_impl.dartfromdart:asyncseemingly not being able to be resolved by this portion of the analyzer:Side note: if I comment out the throw in
RecordGetterData.forEachParameterit then compiles as expected but we get back to the same runtime JS error as above - so these are most likely two issues. I've also tried at -O4 optimization level and levels in between and we get what looks to be the same runtime null error (albeit minified and harder to read):I've also tried compiler flags like
--disable-native-live-type-analysis&--no-frequency-based-minification- on their own, together, etc and same output/issue.Side note 2: the online compiler for Zapp is also built in the same way (a thin Dart wrapper on
dev_compiler&front_endthen compiled to JS via dart2js and ran in a web worker) - this is still working without issue in Dart v3+.Side note 3: I've been trying this on practically every stable Dart version that has come out since and always facing the same issue, I have also been trying the betas but I'm unable to try the very latest Dart betas since there's an issue with the build tools so it can't be compiled at the moment:
dart info)Current version info below but any version really since v3.
N/A
N/A
The text was updated successfully, but these errors were encountered: