-
Notifications
You must be signed in to change notification settings - Fork 123
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes queries for lsp #349
base: master
Are you sure you want to change the base?
Conversation
- Nested typecheck for foo.bar.bas() constructions
@@ -2260,7 +2260,12 @@ struct TypeChecker { | |||
// Check if we need to do any lifetime adjustments. | |||
AdjustLifetime(n, recip, idents); | |||
// Check for queries. | |||
if (query && query->qloc == n->line) ProcessQuery(); | |||
if (query) { | |||
if ((reqret==0 && n->line == query->qloc) //reqret usually in the end of line |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reqret
indicates if there is a surrounding expression that requires a return value of the current expression, what is the effect of checking this? I don't understand the comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So can I use reqret==0
as an indicator that it is the last pass on this line?
if (query && query->qloc == n->line) ProcessQuery(); | ||
if (query) { | ||
if ((reqret==0 && n->line == query->qloc) //reqret usually in the end of line | ||
|| (n->line.line > query->qloc.line && n->line.fileidx==query->qloc.fileidx)) { //If above missed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will attempt to do the query in all lines following..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant to fix this behavior:
For example you need to know about x
here
let x = foo()
if you call ProcessQuery() on the first encounter of this line you will get undefined
.
I try to call it on the last by checking reqret==0
or on one of the next lines. Looking more sane check n->line.line == query->qloc.line+1
isn't working if the next line is an empty one. I thought reqret==0
should be enough, but still not sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I now finally understand what you're trying to do :)
reqret==0
will work under most circumstances but it is very shaky, there may be exceptions.
Just using >
for the line comparison seems more robust, and should be sufficient.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But how then will be queried the last line…) I searched for anything that would be the last line indicator, but nothing found.
As for now, reqret==0
is checked on the queried line, so there are two possible fails:
- Early
reqret==0
. A little chance query to fail (found nothing). - No
reqret==0
at all on this line. May be a problem if line is the last one. Can we have the last line withoutreqret==0
in it? If the answer is "No", thenreqret==0
is still a solution.
This one is much better than #348
Now there is only two possible breaking changes:
LookupStruct
now search insidegudttable
ProcessQuery
is now returnsbool
. Otherwise the call toProcessQuery
inCompile
was always unreachable because of theTHROW_OR_ABORT
call in the end ofProcessQuery
. So now it is required to call it manually afterProcessQuery
returnsfalse
.Also no more annoying spaces, etc. But I kept
ProcessDefinition
as a separate function because of its recursive nature.still may get some collisions (below it should be
zk.d: int
)