feature(fix): function parameter tracking #5483

liuly0322 · 2024-04-22T14:37:58Z

This PR contains:

Are tests included?

yes (bugfixes and features will not be merged without tests)
no

Breaking Changes?

yes (breaking changes will not be merged unless absolutely necessary)
no

List any relevant issue numbers:

resolves TypeError: Cannot read properties of null (reading 'render') #5480

Description

This PR is to point out a problem with 4.16, and discuss if we should revert it first if the work to do is heavy.

Unused node should not be included, otherwise:

logicalExpression1 -- logicalExpression2 -- LogicalExpression3
                                         -- LogicalExpression4
                   -- logicalExpression5 -- LogicalExpression6
                                         -- LogicalExpression7

In the first visit, assuming no optimization info for node1, so node2 and node5 will both get included. and node6 may get not included.

However, if in the second visit, node5 is excluded, becaused it is still mark included, in render stage renderer will try to render node5 and find node6 or node 7 error.

This PR can fix #5480. (no tests because the condition is a bit harsh and I have not reproduced a smaller one)

While it is fixable, It actually reveals: we need a way to revert include. This can be done in two ways to make sure no problems:

explicitly set included=false in every include function that may only include some children (or deoptimizePath like in the PR, it actually makes no difference because deoptimizeCache will request one include, but place it in include may be better because there may be other functions causing children included change (in the future?) and request a include)
in render function, we don't use included to check, but only uses usedBranch or other stuffs like that. I guess it's not how it is designed.

This PR only achives a part of the first way. I'll be off for next 12 or more hours, so if it's too heavy to do, maybe we should revert version 4.16 ...

vercel · 2024-04-22T14:39:20Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
rollup	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 26, 2024 4:04am

codecov · 2024-04-22T14:47:45Z

Codecov Report

Attention: Patch coverage is 99.02913% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 98.81%. Comparing base (1c404fa) to head (133cade).
Report is 2 commits behind head on master.

Files	Patch %	Lines
src/ast/nodes/shared/FunctionBase.ts	95.65%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #5483   +/-   ##
=======================================
  Coverage   98.81%   98.81%           
=======================================
  Files         238      238           
  Lines        9451     9540   +89     
  Branches     2408     2436   +28     
=======================================
+ Hits         9339     9427   +88     
- Misses         47       48    +1     
  Partials       65       65

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

lukastaegert · 2024-04-22T15:05:15Z

Not sure I fully get the underlying connections, but at first glance I feel very uneasy about reverting includes. It has been a reliable behavior for tree-shaking that things only ever get included, and it could even make the algorithm non-deterministic if we change that. In the worst case, we include something unnecessarily, but I would rather have that than have the algorithm not reliably converge.

liuly0322 · 2024-04-22T15:14:44Z

Not sure I fully get the underlying connections, but at first glance I feel very uneasy about reverting includes. It has been a reliable behavior for tree-shaking that things only ever get included, and it could even make the algorithm non-deterministic if we change that. In the worst case, we include something unnecessarily, but I would rather have that than have the algorithm not reliably converge.

Wow, yes, the real problem here is not don't include anything unnecessary, but make sure the children include has the latest state. I'll rethink it this way.

lukastaegert · 2024-04-22T15:18:33Z

I think the problem is that branch resolution should never go from analyzed to not-analyzed but rather from analyzed to unoptimized as it used to be. That ensures that it stays deterministic. We CAN re-analyze the resolution, but if the result is that then the other branch is picked, we should instead go for not-analyzed IMO. Not sure I will manage to do a rollback now, but maybe it is ok to leave the issue open for a day.

liuly0322 · 2024-04-23T03:03:05Z

I think the problem is that branch resolution should never go from analyzed to not-analyzed but rather from analyzed to unoptimized as it used to be. That ensures that it stays deterministic. We CAN re-analyze the resolution, but if the result is that then the other branch is picked, we should instead go for not-analyzed IMO. Not sure I will manage to do a rollback now, but maybe it is ok to leave the issue open for a day.

I think the problem is not restricted to picking the other branch, a short example (still I have not reproduced, so only an example):

function foo(l, r) {
  // l and r may be a complex expression
  console.log(l && r)
}

foo(l, r)

If in the first pass, l and r are both unknown, both l and r will be included. However, the value of r is unknown does not mean nothing happens inside, there is probably something happens in r, and request a tree-shaking to update include.
If in the second pass, function parameters updated, so only l is picked. the tree-shaking update of include of r is then missing!

I force pushed a commit, to keep the idea with "include", make sure if one branch is included, then it will always be included in next tree-shakings.

Also I think it may be fine to update branchResolutionAnalyzed, since it only indicated "isUsedBranchCached"?

Still this fix may not be complete, I'm checking if there are other situations a node "included" but include function not called.

lukastaegert · 2024-04-23T04:51:13Z

I'm still not super happy about this. The idea that an unused branch could be included seems wrong to me. Why not go back to the original implementation for now where something that is unknown at one point remains unknown? We would still keep most of the function parameter optimization but this part would be on more solid footing?

lukastaegert · 2024-04-23T04:55:33Z

Looking at your example, why is the function call not known during the first pass when the function is included? After all, it is only included because of the function call.

liuly0322 · 2024-04-23T05:11:02Z

Looking at your example, why is the function call not known during the first pass when the function is included? After all, it is only included because of the function call.

I believe it is a cache problem, the function gets included because of the function call but before that it seems it has an cached value of unoptimized. (expression cache test (which fails if I don't set isBranchResolutionAnalyzed=false))

One other test with a conditional expression will fail too, but yes, we still have most of the optimizations, and don't include unused branch is reasonable, so I'll remove isBranchResolutionAnalyzed=false and update tests now.

EDIT: I just found IfStatement has the same problem. In 4.16, this.testValue = unset in deoptimizeCache might introduce a state transition from Unknown to Known. If we change it to this.testValue = UnknownValue, all optimizations would fail.

Reason: For ConditionalExpression, LogicalExpression and IfStatement, hasEffects is called first so they have a cached branch value. It's quite early, probably in module.hasEffects, at that time no function arguments can be got, so it's before we can optimize the parameters.

Consider:

include () {
  // in the first include we clear the branch cache
  if (!this.included) this.testValue = unset
  // original code ...
}

apply this change to ConditionalExpression, LogicalExpression and IfStatement, then most optimizations will still work, but this way looks not very good too (actually only branch cache infected by function parameters update needs update).

lukastaegert · 2024-04-23T09:05:05Z

Maybe we just revert the change to IfStatement for now as well, i.e.

deoptimizeCache(): void {
  this.testValue = UnknownValue;
}

as it was before, and we can then think about a better long-term approach after this is released?

liuly0322 · 2024-04-23T09:22:15Z

Maybe we just revert the change to IfStatement for now as well, i.e.
deoptimizeCache(): void {
  this.testValue = UnknownValue;
}
as it was before, and we can then think about a better long-term approach after this is released?

It almost discards all of the parameter optimizations, so maybe we revert main branch to 4.15 code, and move all commits after that to a new development branch?

lukastaegert · 2024-04-23T12:56:07Z

I created #5487 to revert for now. While this is not as nice as rewinding master back to an earlier version, it ensures that master is kept in a releasable state while maintaining the release history and avoiding force pushes.

Revert "Revert function parameter tracking logic for now (rollup#5487)" This reverts commit 74da135.

liuly0322 · 2024-04-24T05:19:27Z

So I just reverted #5487 in this PR, we can collect and fix bugs for function parameters tracking here.

Some known issues:

cache logic
feat: improve tree-shaking by propagate const parameter #5443 (comment)
Maybe we first make it an experimental feature to see if it's stable for some time?

Would you be able to add an option separate from treeshake for this feature (like experimental_treeshake?), which is disabled by default

lukastaegert · 2024-04-24T07:00:44Z

Ok, I have some thoughts about the problems we were facing. As you correctly saw, the function parameters are collected far too late. The problem is not the collection, though: This happens on applyDeoptimizations for the CallExpression. The problem is that the parameter values are updated imperatively only on include, which is far too late.

Here is a proposal how this can be fixed:
Instead of imperatively storing the argument values on the ParameterVariable on FunctionBase.include, we could instead directly store the argument values on FunctionBase.deoptimizeArgumentsOnInteractionAtPath on the ParameterVariables. This process would

directly consolidate new values with the existing values for each parameter
make sure that we can only change from no value to one value to unknown value
deoptimize caches if we transition to "unknown", see below

Beyond that, any functions like ParameterVariable.getLiteralValueAtPath would get the current parameter value but also store the origin parameter in its cache so that we the known value changes, we can reactively deoptimize any other part that depends on the value.

As the CallExpression should be traversed before the function body is traversed, the call values should usually be present. However, if we overlook anything and the ParameterVariable is queried before it has a call value (or before we know there is a usage that we do not track), we could just set the value to Unknown and keep it like that. But I do not expect this to happen.

What do you think?

liuly0322 · 2024-04-24T13:38:42Z

Ok, I have some thoughts about the problems we were facing. As you correctly saw, the function parameters are collected far too late. The problem is not the collection, though: This happens on applyDeoptimizations for the CallExpression. The problem is that the parameter values are updated imperatively only on include, which is far too late.

Here is a proposal how this can be fixed: Instead of imperatively storing the argument values on the ParameterVariable on FunctionBase.include, we could instead directly store the argument values on FunctionBase.deoptimizeArgumentsOnInteractionAtPath on the ParameterVariables. This process would

directly consolidate new values with the existing values for each parameter

make sure that we can only change from no value to one value to unknown value

deoptimize caches if we transition to "unknown", see below

Beyond that, any functions like ParameterVariable.getLiteralValueAtPath would get the current parameter value but also store the origin parameter in its cache so that we the known value changes, we can reactively deoptimize any other part that depends on the value.

As the CallExpression should be traversed before the function body is traversed, the call values should usually be present. However, if we overlook anything and the ParameterVariable is queried before it has a call value (or before we know there is a usage that we do not track), we could just set the value to Unknown and keep it like that. But I do not expect this to happen.

What do you think?

Yes I thought of include too late yesterday, thanks for reminding me of FunctionBase.deoptimizeArgumentsOnInteractionAtPath

There are one problem and some thoughs I'd like to share now:

One problem:

In the first call of a function, everything starts from CallExpression.hasEffects

hasEffects(context: HasEffectsContext): boolean {
	try {
		for (const argument of this.arguments) {
			if (argument.hasEffects(context)) return true;
		}
		if (this.annotationPure) {
			return false;
		}
		return (
			this.callee.hasEffects(context) ||
			this.callee.hasEffectsOnInteractionAtPath(EMPTY_PATH, this.interaction, context)
		);
	} finally {
		if (!this.deoptimized) this.applyDeoptimizations();
	}
}

this.applyDeoptimizations is called after this.callee.hasEffects, so still, the branch cache is first generated, then FunctionBase.deoptimizeArgumentsOnInteractionAtPath gets called.

So I wonder (sorry for not understanding the logic here well_:

Why this.applyDeoptimizations() is not placed at the entry of the function, but the exit?
Should we create a new method to pass this.interaction?

Some thoughts

Only known and unknown states are needed

In traditional constant propagation optimization, we must init a top state for every variables. This is because we have no idea of what constant value a variable can be at the beginning. However things have changed for tree-shaking, as tree-shaking only includes nodes needed, it naturally generates a "call tree/graph", so we actually only need two state: from known to unknown, which is similar to other parts of tree-shaking, really cool. See code below:

function foo1(a) {
  // ...
}

foo1(0)
foo2(0)

function foo2(a) {
  // ...
}

We are pretty sure the parameter value is 1 or unknown, from the first "include". And in multi-level function call like code below:

function foo1(a) {
  // a is 1 here
  foo2(a)
}

function foo2(a) {
  // ...
}

foo1(1)

foo2 is included while foo1 calls it, where the value of a is known! While in traditional compiler, everything is included and unknown from the beginning.

Update states in every include?

In previous implemention, I update function parameters in every include to make sure converge. However, it seems we don't have to do so. Making an Data-flow analysis optimization converge usually has two ways:

In every loop, iterate over all nodes. that's what we used to do.
use a work-list algorithm. Push node-b to work-list if node-a updates and node-b depends on node-a.

I found deoptimizeCache a good tool to achieve similar effects like work-list algorithm. We can make a handler array and pass handlerArray[position] when getting literal value of an argument, so we know the this.params[position] needs update. and since we only have two states now, we can simply set it to unknown.

liuly0322 · 2024-04-24T15:07:49Z

So I update my code:

reverted changes to IfStatement, ConditionalExpression and LogicalExpression, and other changes to cache design.
refactor ParameterVariable and FunctionBase
1. Now isReassigned also indicates if it's a bottom state. (I found old implementation bottom state confusing).
2. Bottom state means "no optimization" as other parts, so we can use EMPTY_ARRAY again.
3. add deoptimizeCache for ParameterVariable

The CallExpression.hasEffects may not be perfect, so I call it a draft.

It's interesting that we have a big refactor which reduces 60 lines of code because of bugs😀

liuly0322 · 2024-04-25T00:31:32Z

As code gets smaller, I can take more time to see where the problems might be and I found one.

function foo(a) {
  arguments[0] = 0
  eval("arguments[0] = 0")
  if (a) {
    console.log(1)
  } else {
    console.log(0)
  }
}

foo(1)

While I assumed functions parameters/arguments to be static, It seems not. Luckily it's the only thing I found break the rule. Function.prototype.arguments is not standard and deprecated. (Since we definately cannot access a local variable without name (or most minifiers shouldn't work), arguments is the only hole. And arguments itself is a local variable, we must access it from name (or eval))

From my perspective, we need to make eval and arguments deoptimize function parameters...?

EDIT: I found in strict mode there is "No syncing between parameters and arguments indices", so this should not be a problem.

lukastaegert · 2024-04-25T05:04:55Z

Why this.applyDeoptimizations() is not placed at the entry of the function, but the exit?

Honestly, I am not sure, I guess the order just did not matter. Try switching the order around, if no test fails it should be fine.

Making an Data-flow analysis optimization converge usually has two ways:
In every loop, iterate over all nodes. that's what we used to do.
use a work-list algorithm. Push node-b to work-list if node-a updates and node-b depends on node-a.

Interesting that you mention this. In the far future, I would envision Rollup to transfer most of the tree-shaking logic to what you call a work-list algorithm. So one would only have a single full pass and then maintain a list of "dirty" nodes that need to be re-evaluated. For myself, I always called this a "reactive" algorithm, but work-list probably fits better. If done right, this should improve performance quite a bit. However, it would require to know all dependencies of all nodes and this feels quite complicated to implement.

I think I will have a look at your changes by tomorrow.

From my perspective, we need to make eval and arguments deoptimize function parameters...?

eval is usually ignored by Rollup as you get a big warning when it is used. It can just break in unexpected ways as variables are renamed dynamically. However, using arguments should deoptimize parameters. There IS logic to track the arguments variable to make sure all parameters are included when it is used, look for argumentsVariable in the code.

lukastaegert · 2024-04-25T05:08:47Z

Ah but you are right, reassignments of parameters themselves should not be tracked in arguments.

liuly0322 · 2024-04-25T05:38:44Z

Try switching the order around, if no test fails it should be fine.

No test fails, I find #4148 related. I will have a look today.

If done right, this should improve performance quite a bit. However, it would require to know all dependencies of all nodes and this feels quite complicated to implement.

Really cool, in LLVM IR it's easy because it is trivial to build a use-def chain for op src1 src2 dst, but not sure how it can be done with AST.

Ah but you are right, reassignments of parameters themselves should not be tracked in arguments.

I'm just a little worried because I'm not familiar with how people would use rollup, and maybe there are some legacy code depends on this behavior. The fix is relatively easy, though.

export default class FunctionNode extends FunctionBase {
                super.include(context, includeChildrenRecursively);
                this.id?.include();
                const hasArguments = this.scope.argumentsVariable.included;
+               if (hasArguments) {
+                       this.deoptimizeParameters();
+               }
                // ...
}

style: move deoptimizeCache logic style: naming style: naming

style: move deoptimizeCache logic style: naming style: naming remove useless comment style: small update

This is to make sure if onlyFunctionCallUsed becomes false, there is a new tree-shaking pass to deoptimize it

lukastaegert

This looks quite good now. I wonder if we should try another release :)

src/ast/nodes/Identifier.ts

liuly0322 · 2024-04-26T06:03:18Z

This looks quite good now. I wonder if we should try another release :)

Thanks, I think it's definitely worth trying.

github-actions · 2024-04-27T11:30:28Z

This PR has been released as part of rollup@4.17.0. You can test it via npm install rollup.

kleinfreund · 2024-04-29T08:34:57Z

Apologies for not providing a concise bug report in the form of a new issue, but I can’t really share the actual reproducible code (private repository). If I find some time, I’ll try working on a proper bug report (edit: someone else one: #5502).

I’m pretty sure 4.17.0 has the same regression I’ve seen in rollup >=4.16.0 <4.16.4. The following kind of code (I obfuscated the real code but kept, hopefully, the syntax and logic sufficiently accurately):

import { exportA, exportB } from 'package'

export function fn(a, b, c) {
  return async (d, e) => {
    const { result } = await exportB(d)
    if (!b.prop1?.prop2?.prop3) {
      c.error = new Error('', { cause: { prop: a.fn('string') } })
      c.dispatchEvent(new ErrorEvent('error', { error: c.error }))

      return true
    }
    if (!result) {
      if (!exportA) {
        c.error = new Error('', { cause: { prop: a.fn('string') } })
        c.dispatchEvent(new ErrorEvent('error', { error: c.error }))
      }

      return true
    }
  }
}

becomes, functionally, in the output:

import { exportA, exportB } from 'package'

export function fn(a, b, c) {
  return async (d, e) => {
    await exportB(d)
    c.error = new Error('', { cause: { prop: a.fn('string') } })
    c.dispatchEvent(new ErrorEvent('error', { error: c.error }))
    return true
  }
}

I think the second if statement isn’t relevant. What seems to happen is that if (!b.prop1?.prop2?.prop3) { gets "optimized" away making the first return statement return from the async function unconditionally and then, following this optimization logically, the second if statement doesn't matter because it became dead code.

lukastaegert · 2024-04-29T19:13:11Z

If you want to help us track this down, we need to know what all places where the function fn is called look like, especially where it would take the second branch:

what do the parameters look like?
what does the line of code look like where the call is made?

For some reason, it can neither track the call nor can I detect that it "lost track" of the function. It could also be of interest if the function is passed to other functions, assigned to object properties or global variables etc.

kleinfreund · 2024-04-30T08:29:44Z

Of course, but first, let me start by saying that 4.17.2 (issue, PR) fixes the issue I describe in #5483 (comment).

we need to know what all places where the function fn is called look like, especially where it would take the second branch:

what do the parameters look like?

what does the line of code look like where the call is made?

fn is called exactly once and the parameters are provided as fn(...args). The line containing the call is the sole body of an arrow function:

      prop: (...args) => {
        return [fn(...args)]
      },

It could also be of interest if the function is passed to other functions, assigned to object properties or global variables etc.

In the file that contains the sole call of fn, it is accessed through a direct, named import from the file fn is exported from. It’s not referenced or called anywhere else.

Looking at https://github.com/rollup/rollup/pull/5503/files, it seems my example might be exactly what’s fixed in that PR. If you think there could be more to this, I’d be happy to provide more information of course.

lukastaegert · 2024-04-30T16:00:58Z

Thanks for taking the time to respond, this gives me some confidence that @liuly0322 got to the root of the problem here. Lets see if anything else pops up in the next days, but so far it is looking quite good!

vercel bot deployed to Preview April 22, 2024 14:39 View deployment

liuly0322 force-pushed the fix-include branch from 62e2207 to 716867b Compare April 23, 2024 03:02

vercel bot deployed to Preview April 23, 2024 03:03 View deployment

liuly0322 changed the title ~~[Don't merge now] unused branch should not be included~~ fix: include children Apr 23, 2024

liuly0322 force-pushed the fix-include branch from 716867b to 3d9e077 Compare April 23, 2024 05:29

vercel bot deployed to Preview April 23, 2024 05:31 View deployment

vercel bot deployed to Preview April 23, 2024 05:39 View deployment

feat: re-init function parameter tracking

62c4f51

Revert "Revert function parameter tracking logic for now (rollup#5487)" This reverts commit 74da135.

liuly0322 force-pushed the fix-include branch from a5ba2ee to 62c4f51 Compare April 24, 2024 05:13

vercel bot deployed to Preview April 24, 2024 05:14 View deployment

liuly0322 changed the title ~~fix: include children~~ feature(fix): function parameter tracking Apr 24, 2024

liuly0322 added a commit to liuly0322/_rollup that referenced this pull request Apr 24, 2024

refactor(draft): see rollup#5483 comments

ac9aaf8

vercel bot deployed to Preview April 24, 2024 15:04 View deployment

vercel bot deployed to Preview April 24, 2024 16:55 View deployment

vercel bot deployed to Preview April 24, 2024 23:45 View deployment

liuly0322 added a commit to liuly0322/_rollup that referenced this pull request Apr 25, 2024

refactor: see rollup#5483 comments. squashed with some style commits

f736ba6

style: move deoptimizeCache logic style: naming style: naming

liuly0322 force-pushed the fix-include branch from 7f2a7b9 to f736ba6 Compare April 25, 2024 07:26

vercel bot deployed to Preview April 25, 2024 07:27 View deployment

vercel bot deployed to Preview April 25, 2024 13:56 View deployment

vercel bot deployed to Preview April 25, 2024 15:17 View deployment

vercel bot deployed to Preview April 25, 2024 22:48 View deployment

liuly0322 added 3 commits April 26, 2024 11:53

refactor: see rollup#5483 comments. squashed with some style commits

c2c8f52

style: move deoptimizeCache logic style: naming style: naming remove useless comment style: small update

make sure every addUsedPlace has a requestTreeshakingPass

e662bed

This is to make sure if onlyFunctionCallUsed becomes false, there is a new tree-shaking pass to deoptimize it

fix: missing reassigned guard

133cade

liuly0322 force-pushed the fix-include branch from ea0f793 to 133cade Compare April 26, 2024 04:03

vercel bot deployed to Preview April 26, 2024 04:04 View deployment

lukastaegert approved these changes Apr 26, 2024

View reviewed changes

src/ast/nodes/Identifier.ts Show resolved Hide resolved

lukastaegert added this pull request to the merge queue Apr 27, 2024

Merged via the queue into rollup:master with commit d98e59d Apr 27, 2024
36 checks passed

github-actions bot mentioned this pull request Apr 27, 2024

TypeError: Cannot read properties of null (reading 'render') #5480

Closed

liuly0322 deleted the fix-include branch April 28, 2024 04:19

lukastaegert mentioned this pull request Apr 30, 2024

fix: rest element should deoptimize parameter values #5503

Merged

9 tasks

lukastaegert mentioned this pull request May 3, 2024

feat: improve tree-shaking by propagate const parameter #5443

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature(fix): function parameter tracking #5483

feature(fix): function parameter tracking #5483

liuly0322 commented Apr 22, 2024 •

edited

vercel bot commented Apr 22, 2024 •

edited

codecov bot commented Apr 22, 2024 •

edited

lukastaegert commented Apr 22, 2024

liuly0322 commented Apr 22, 2024

lukastaegert commented Apr 22, 2024

liuly0322 commented Apr 23, 2024 •

edited

lukastaegert commented Apr 23, 2024

lukastaegert commented Apr 23, 2024

liuly0322 commented Apr 23, 2024 •

edited

lukastaegert commented Apr 23, 2024

liuly0322 commented Apr 23, 2024

lukastaegert commented Apr 23, 2024 •

edited

liuly0322 commented Apr 24, 2024

lukastaegert commented Apr 24, 2024

liuly0322 commented Apr 24, 2024 •

edited

liuly0322 commented Apr 24, 2024 •

edited

liuly0322 commented Apr 25, 2024 •

edited

lukastaegert commented Apr 25, 2024

lukastaegert commented Apr 25, 2024

liuly0322 commented Apr 25, 2024

lukastaegert left a comment

liuly0322 commented Apr 26, 2024

github-actions bot commented Apr 27, 2024

kleinfreund commented Apr 29, 2024 •

edited

lukastaegert commented Apr 29, 2024

kleinfreund commented Apr 30, 2024 •

edited

lukastaegert commented Apr 30, 2024

feature(fix): function parameter tracking #5483

feature(fix): function parameter tracking #5483

Conversation

liuly0322 commented Apr 22, 2024 • edited

Description

vercel bot commented Apr 22, 2024 • edited

codecov bot commented Apr 22, 2024 • edited

Codecov Report

lukastaegert commented Apr 22, 2024

liuly0322 commented Apr 22, 2024

lukastaegert commented Apr 22, 2024

liuly0322 commented Apr 23, 2024 • edited

lukastaegert commented Apr 23, 2024

lukastaegert commented Apr 23, 2024

liuly0322 commented Apr 23, 2024 • edited

lukastaegert commented Apr 23, 2024

liuly0322 commented Apr 23, 2024

lukastaegert commented Apr 23, 2024 • edited

liuly0322 commented Apr 24, 2024

lukastaegert commented Apr 24, 2024

liuly0322 commented Apr 24, 2024 • edited

One problem:

Some thoughts

Only known and unknown states are needed

Update states in every include?

liuly0322 commented Apr 24, 2024 • edited

liuly0322 commented Apr 25, 2024 • edited

lukastaegert commented Apr 25, 2024

lukastaegert commented Apr 25, 2024

liuly0322 commented Apr 25, 2024

lukastaegert left a comment

Choose a reason for hiding this comment

liuly0322 commented Apr 26, 2024

github-actions bot commented Apr 27, 2024

kleinfreund commented Apr 29, 2024 • edited

lukastaegert commented Apr 29, 2024

kleinfreund commented Apr 30, 2024 • edited

lukastaegert commented Apr 30, 2024

liuly0322 commented Apr 22, 2024 •

edited

vercel bot commented Apr 22, 2024 •

edited

codecov bot commented Apr 22, 2024 •

edited

liuly0322 commented Apr 23, 2024 •

edited

liuly0322 commented Apr 23, 2024 •

edited

lukastaegert commented Apr 23, 2024 •

edited

liuly0322 commented Apr 24, 2024 •

edited

liuly0322 commented Apr 24, 2024 •

edited

liuly0322 commented Apr 25, 2024 •

edited

kleinfreund commented Apr 29, 2024 •

edited

kleinfreund commented Apr 30, 2024 •

edited