-
Notifications
You must be signed in to change notification settings - Fork 23
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
Fix empty streams as result #582
Conversation
Aqua version is v0.0.1-fix-empty-streams-47aca0f-72-1.0To install it run: npm login --registry https://npm.fluence.dev
npm i @fluencelabs/aqua@=v0.0.1-fix-empty-streams-47aca0f-72-1.0 --registry=https://npm.fluence.dev |
LNG-83 Push to stream doesn't work with assigned streams
Aqua:
Air:
there is a mess with variable names LNG-84 Function with only stream creation generate incorrect air
aqua:
air:
there is no |
@@ -179,8 +179,14 @@ object TagInliner extends Logging { | |||
} | |||
|
|||
case AssignmentTag(value, assignTo) => | |||
// add name to CollectionRaw for streams to resolve them |
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 can't get this message
) | ||
streamName <- | ||
raw.boxType match { | ||
case _: StreamType => raw.assignToName.map(s => State.pure(s)).getOrElse(Mangler[S].findAndForbidName("stream-inline")) |
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 CollectionRawInliner
has a very specific callsite where you already have this assignToName
in scope, I suppose
case _ => false | ||
} | ||
|
||
_ = println("out decl streams: " + outsideDeclaredStreams) |
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.
Plz remove debug output
_ = println("out decl streams: " + outsideDeclaredStreams) | ||
|
||
// Function's internal variables will not be available outside, hence the scope | ||
result <- Exports[S].scope( |
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.
Maybe
Exports[S].exports
.map(_.collect{case a@(_, VarModel(_, StreamType(_), _)) => a})
.flatMap(outsideDeclaredStreams =>
Exports[S].scope...
)
would be more readable than a for loop?
(value match { | ||
// if we assign collection to a stream, we must use it's name, because it is already created with 'new' | ||
case c@CollectionRaw(_, _: StreamType) => | ||
collectionToModel(c, Some(assignTo)) |
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.
Do you ever call collectionToModel(..., None)
?
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.
yes, in unfold
Handle returning streams in ArrowSem, because they will be processed and canonicalized later in inliners
fixes LNG-83
fixes LNG-84