Skip to content
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

Send info about function values to the IDE #6957

Closed
JaroslavTulach opened this issue Jun 6, 2023 · 12 comments · Fixed by #7168
Closed

Send info about function values to the IDE #6957

JaroslavTulach opened this issue Jun 6, 2023 · 12 comments · Fixed by #7168

Comments

@JaroslavTulach
Copy link
Member

JaroslavTulach commented Jun 6, 2023

Based on language server API discussion, analysis while working on bug 6903 and request for changes in #6939 I am proposing following path forward:

ProgramExecutionSupport.scala needs to do following when composing sendExpressionUpdate:

  • when the value.getType is Standard.Builtins.Main.Function
  • accompany the information with a MethodPointer to the function definition
  • use (value.value : runtime.Function).getSchema() to obtain hasPreApplied array and send it to the IDE somehow

This fixes problems with partially applied constructors and functions and lets the IDE know what arguments have already been applied sooner when Function value arrives. The IDE however needs to process this additional information somehow.

@4e6
Copy link
Contributor

4e6 commented Jun 6, 2023

The #6939 is already doing what you are proposing:

if (constructorCalls.contains(nodeId) && result instanceof Function function) {
FunctionCallInfo call = calls.get(nodeId);
return new FunctionCallInfo(call, function);

  • (line 328) When the value is a function
  • (line 329) Accompany the information with a MethodPointer (it is represented as a FunctionCallInfo)
  • (line 330) use function.getSchema() to obtain hasPreApplied array. The FunctionCallInfo constructor does exactly this:
    private static int[] collectNotAppliedArguments(Function function) {
    Object[] preAppliedArguments = function.getPreAppliedArguments();
    int[] notAppliedArgs = new int[preAppliedArguments.length];
    int notAppliedArgsSize = 0;
    for (int i = 0; i < preAppliedArguments.length; i++) {
    if (preAppliedArguments[i] == null) {
    notAppliedArgs[notAppliedArgsSize] = i;
    notAppliedArgsSize += 1;
    }
    }

@JaroslavTulach
Copy link
Member Author

JaroslavTulach commented Jun 7, 2023

The #6939 is already doing what you are proposing:

Kind of, but not exactly:

Each result instanceof Function shall be sent to the IDE as FunctionCallInfo and it doesn't matter where it is originating from - whether it is a result of applying a constructor or calling a function. Such detailed information about the result instanceof Function has to be delivered as soon as possible without forcing the IDE to perform any textual edits. Particularly interesting test case is:

text1 = "  hello ! "
operator4 = Text.trim
operator5 = operator4 where=Location.Start
operator7 = operator5 text1

Partially applied Text.trim function

Will the code currently proposed in #6939 deliver FunctionCallInfo information about operator5 to the IDE? Will the IDE not display where attribute when invoking operator5?

@4e6
Copy link
Contributor

4e6 commented Jun 7, 2023

This issue was addressed in #6867 by sending the list of not applied arguments together with the method pointer. #6939 is a follow-up to that PR and is only supposed to send the correct arguments in case of constructor calls.

@JaroslavTulach
Copy link
Member Author

JaroslavTulach commented Jun 7, 2023

One more example that illustrates the problem with function value vs. the last executed function:

Pair.second

The program "hides" the Text.trim function inside of Pair and then obtains it as Pair.second later. It is still the same Text.trim function - e.g. the IDE shall offer list of not yet applied arguments. It should offer where and what in the last node, in my opinion.

@4e6
Copy link
Contributor

4e6 commented Jun 7, 2023

A follow-up to the call when we were discussing the case when the function is read from the variable

main =
    x1_1 = T.func1
    x1_2 = x1_1
    x1 = x1_2 1 2
    x1

type T
    A

    func1 x y = x + y

x1_1 expression (ReadLocalVariableNode) is not wrapped in the FunctionCallInstrumentationNodeWrapper (only function applications are) and does not have the method pointer associated with it.

Tracking PR with a test case #6986

@JaroslavTulach
Copy link
Member Author

This case shall continue to work the same way it works now:
obrazek

@JaroslavTulach
Copy link
Member Author

Think about ... operator:
obrazek

@4e6
Copy link
Contributor

4e6 commented Jun 12, 2023

Summary of the discussion with @Frizi and @JaroslavTulach

IDE needs additional information to display the placeholders for the arguments of partially applied functions.

  • When the return value is a function, the Payload of ExpressionUpdate should provide additional MethodCall information (MethodPointer + argument infos)
  • In case of the complex expression
      Pair.Value 1 2
    When the Pair.Value subexpression is annotated, it returns an ExpressionUpdate with a function return type and a payload containing the method call information of the returned function

@hubertp hubertp moved this from 📤 Backlog to 🔧 Implementation in Issues Board Jun 12, 2023
@hubertp hubertp removed their assignment Jun 16, 2023
@enso-bot
Copy link

enso-bot bot commented Jun 27, 2023

Dmitry Bushev reports a new STANDUP for yesterday (2023-06-26):

Progress: Started working on the issue. Updated the API types. Updated the language server part up to the runtime connector. Started working on the engine It should be finished by 2023-06-29.

Next Day: Next day I will be working on the #6957 task. Continue working on the task

@enso-bot
Copy link

enso-bot bot commented Jun 27, 2023

Dmitry Bushev reports a new STANDUP for today (2023-06-27):

Progress: Continue working on the issue. Updated the polyglot-api types. Created a test reproducing the scenario. with returned function. Started updating the instrumentation to obtain the method call information from the returned function value. It should be finished by 2023-06-29.

Next Day: Next day I will be working on the #6957 task. Continue working on the task

@enso-bot
Copy link

enso-bot bot commented Jun 29, 2023

Dmitry Bushev reports a new STANDUP for yesterday (2023-06-28):

Progress: Continue working on the issue. Added logic to extract the function schema from the return function. Found an issue with a type constructor. Started looking for a workaround It should be finished by 2023-06-29.

Next Day: Next day I will be working on the #6957 task. Continue working on the task

@enso-bot
Copy link

enso-bot bot commented Jun 29, 2023

Dmitry Bushev reports a new STANDUP for today (2023-06-29):

Progress: Continue working on the issue. Started creating the function schema only for the return value. Updated the runtime tests. Updated the language server logic. Updated the docs. Tested in the IDE It should be finished by 2023-06-29.

Next Day: Next day I will be working on the #6957 task. Continue working on the task

@mergify mergify bot closed this as completed in #7168 Jun 30, 2023
mergify bot pushed a commit that referenced this issue Jun 30, 2023
close #6957

Extend `ExpressionUpdate` message and send a function schema if the returned value is a function.
@github-project-automation github-project-automation bot moved this from 🔧 Implementation to 🟢 Accepted in Issues Board Jun 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants