Skip to content

Commit

Permalink
Do not crash on empty main function body. (#10990)
Browse files Browse the repository at this point in the history
Fixes #10976

https://github.com/user-attachments/assets/00b2279d-2acf-468b-8c3c-aa6885cba23d


Addressed issue of empty body block being incorrectly "repaired" into an empty group, geneating invalid `()` syntax. Appending nodes to an empty function now actually replaces its body block, instead of creating a temporary orphan body block node.

Note that empty main function is still considered an error on the engine side, but it doesn't impact the IDE in negative way. Things work again as soon as a node is inserted.

Also fixed a few issues causing hot-reloading to break. Now edited AST code properly hot-reloads all affected modules without breaking the app.
  • Loading branch information
Frizi authored Sep 6, 2024
1 parent 3420a05 commit 6bfdda3
Show file tree
Hide file tree
Showing 9 changed files with 46 additions and 27 deletions.
2 changes: 1 addition & 1 deletion app/gui2/src/components/GraphEditor/GraphNode.vue
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ watchEffect(() => {
@pointerleave="(nodeHovered = false), updateNodeHover(undefined)"
@pointermove="updateNodeHover"
>
<Teleport v-if="navigator && !edited" :to="graphNodeSelections">
<Teleport v-if="navigator && !edited && graphNodeSelections" :to="graphNodeSelections">
<GraphNodeSelection
:data-node-id="nodeId"
:nodePosition="props.node.position"
Expand Down
8 changes: 4 additions & 4 deletions app/gui2/src/composables/navigator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,20 +164,20 @@ export function useNavigator(
function panToImpl(points: Partial<Vec2>[]) {
let target = viewport.value
for (const point of points.reverse()) target = target.offsetToInclude(point) ?? target
targetCenter.value = target.center()
targetCenter.value = target.center().finiteOrZero()
}

/** Pan immediately to center the viewport at the given point, in scene coordinates. */
function scrollTo(newCenter: Vec2) {
resetTargetFollowing()
targetCenter.value = newCenter
targetCenter.value = newCenter.finiteOrZero()
center.skip()
}

/** Set viewport center point and scale value immediately, skipping animations. */
function setCenterAndScale(newCenter: Vec2, newScale: number) {
resetTargetFollowing()
targetCenter.value = newCenter
targetCenter.value = newCenter.finiteOrZero()
targetScale.value = newScale
scale.skip()
center.skip()
Expand Down Expand Up @@ -325,7 +325,7 @@ export function useNavigator(
const scenePos0 = clientToScenePos(clientPos)
const result = f()
const scenePos1 = clientToScenePos(clientPos)
targetCenter.value = center.value.add(scenePos0.sub(scenePos1))
targetCenter.value = center.value.add(scenePos0.sub(scenePos1)).finiteOrZero()
center.skip()
return result
}
Expand Down
2 changes: 1 addition & 1 deletion app/gui2/src/providers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const MISSING = Symbol('MISSING')
* [Context API]: https://vuejs.org/guide/components/provide-inject.html#provide-inject
*/
export function createContextStore<F extends (...args: any[]) => any>(name: string, factory: F) {
const provideKey = Symbol(name) as InjectionKey<ReturnType<F>>
const provideKey = Symbol.for(`contextStore-${name}`) as InjectionKey<ReturnType<F>>

/**
* Create the instance of a store and store it in the current component's context. All child
Expand Down
4 changes: 2 additions & 2 deletions app/gui2/src/stores/graph/graphDatabase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export class BindingsDb {

readFunctionAst(
func: Ast.Function,
rawFunc: RawAst.Tree.Function,
rawFunc: RawAst.Tree.Function | undefined,
moduleCode: string,
getSpan: (id: AstId) => SourceRange | undefined,
) {
Expand Down Expand Up @@ -422,7 +422,7 @@ export class GraphDb {
/** Deeply scan the function to perform alias-analysis. */
updateBindings(
functionAst_: Ast.Function,
rawFunction: RawAst.Tree.Function,
rawFunction: RawAst.Tree.Function | undefined,
moduleCode: string,
getSpan: (id: AstId) => SourceRange | undefined,
) {
Expand Down
1 change: 0 additions & 1 deletion app/gui2/src/stores/graph/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,6 @@ export const { injectFn: useGraphStore, provideFn: provideGraphStore } = createC
const methodSpan = moduleSource.getSpan(method.id)
assert(methodSpan != null)
const rawFunc = toRaw.get(sourceRangeKey(methodSpan))
assert(rawFunc != null)
const getSpan = (id: AstId) => moduleSource.getSpan(id)
db.updateBindings(method, rawFunc, moduleSource.text, getSpan)
})
Expand Down
50 changes: 33 additions & 17 deletions app/gui2/src/util/database/reactiveDb.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,15 @@ import { LazySyncEffectSet, NonReactiveView } from '@/util/reactivity'
import * as map from 'lib0/map'
import { ObservableV2 } from 'lib0/observable'
import * as set from 'lib0/set'
import { computed, effectScope, reactive, toRaw, type ComputedRef, type DebuggerOptions } from 'vue'
import {
computed,
effectScope,
onScopeDispose,
reactive,
toRaw,
type ComputedRef,
type DebuggerOptions,
} from 'vue'

export type OnDelete = (cleanupFn: () => void) => void

Expand Down Expand Up @@ -218,14 +226,18 @@ export class ReactiveIndex<K, V, IK, IV> {
constructor(db: ReactiveDb<K, V>, indexer: Indexer<K, V, IK, IV>) {
this.forward = reactive(map.create())
this.reverse = reactive(map.create())
this.effects = new LazySyncEffectSet()
db.on('entryAdded', (key, value, onDelete) => {
const stopEffect = this.effects.lazyEffect((onCleanup) => {
const keyValues = indexer(key, value)
keyValues.forEach(([key, value]) => this.writeToIndex(key, value))
onCleanup(() => keyValues.forEach(([key, value]) => this.removeFromIndex(key, value)))
const scope = effectScope()
this.effects = new LazySyncEffectSet(scope)
scope.run(() => {
const handler = db.on('entryAdded', (key, value, onDelete) => {
const stopEffect = this.effects.lazyEffect((onCleanup) => {
const keyValues = indexer(key, value)
keyValues.forEach(([key, value]) => this.writeToIndex(key, value))
onCleanup(() => keyValues.forEach(([key, value]) => this.removeFromIndex(key, value)))
})
onDelete(() => stopEffect())
})
onDelete(() => stopEffect())
onScopeDispose(() => db.off('entryAdded', handler))
})
}

Expand Down Expand Up @@ -353,16 +365,20 @@ export class ReactiveMapping<K, V, M> {
*/
constructor(db: ReactiveDb<K, V>, indexer: Mapper<K, V, M>, debugOptions?: DebuggerOptions) {
this.computed = reactive(map.create())
db.on('entryAdded', (key, value, onDelete) => {
const scope = effectScope()
const mappedValue = scope.run(() =>
computed(() => scope.run(() => indexer(key, value)), debugOptions),
)! // This non-null assertion is SAFE, as the scope is initially active.
this.computed.set(key, mappedValue)
onDelete(() => {
scope.stop()
this.computed.delete(key)
const scope = effectScope()
scope.run(() => {
const handler = db.on('entryAdded', (key, value, onDelete) => {
const scope = effectScope()
const mappedValue = scope.run(() =>
computed(() => scope.run(() => indexer(key, value)), debugOptions),
)! // This non-null assertion is SAFE, as the scope is initially active.
this.computed.set(key, mappedValue)
onDelete(() => {
scope.stop()
this.computed.delete(key)
})
})
onScopeDispose(() => db.off('entryAdded', handler))
})
}

Expand Down
3 changes: 2 additions & 1 deletion app/gui2/src/util/reactivity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,10 @@ export type StopEffect = () => void
*/
export class LazySyncEffectSet {
_dirtyRunners = new Set<() => void>()
_scope = effectScope()
_boundFlush = this.flush.bind(this)

constructor(private _scope = effectScope()) {}

/**
* Add an effect to the lazy set. The effect will run once immediately, and any subsequent runs
* will be delayed until the next flush. Only effects that were notified about a dependency change
Expand Down
2 changes: 2 additions & 0 deletions app/ydoc-shared/src/ast/parse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,8 @@ function checkSpans(expected: NodeSpanMap, encountered: NodeSpanMap, code: strin
const lostBlock = new Array<Ast>()
for (const [key, ast] of lost) {
const [start, end] = sourceRangeFromKey(key)
// Do not report lost empty body blocks, we don't want them to be considered for repair.
if (start === end && ast instanceof BodyBlock) continue
;(code.substring(start, end).match(/[\r\n]/) ? lostBlock : lostInline).push(ast)
}
return { lostInline, lostBlock }
Expand Down
1 change: 1 addition & 0 deletions app/ydoc-shared/src/ast/tree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1962,6 +1962,7 @@ export class MutableFunction extends Function implements MutableAst {
if (oldBody instanceof MutableBodyBlock) return oldBody
const newBody = BodyBlock.new([], this.module)
if (oldBody) newBody.push(oldBody.take())
this.setBody(newBody)
return newBody
}
}
Expand Down

0 comments on commit 6bfdda3

Please sign in to comment.