Skip to content

Sonalmahajan15/fix non literal access in maps#5

Open
lizard-boy wants to merge 5 commits into
mainfrom
sonalmahajan15/fix-non-literal-access-in-maps
Open

Sonalmahajan15/fix non literal access in maps#5
lizard-boy wants to merge 5 commits into
mainfrom
sonalmahajan15/fix-non-literal-access-in-maps

Conversation

@lizard-boy

Copy link
Copy Markdown

No description provided.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

This PR refines nil check propagation and control flow in the assertion tree by introducing an early exit in fixed point iterations and updating negative nil check production.

• In /assertion/function/assertiontree/preprocess_blocks.go, the propagation loop now consults checkCFGFixedPointRuntime to break early, preventing excessive iterations.
• In /assertion/function/assertiontree/backprop.go, the direct panic has been replaced with a conditional check for early termination, coordinating with the new runtime limit.
• In /assertion/function/assertiontree/util.go, produceNegativeNilCheck is refactored to update triggers via string comparison, which requires careful testing to ensure robustness.

3 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile

Comment on lines 260 to +305
produceNegativeNilCheck := func(expr ast.Expr) RootFunc {
return produceExprByTrigger(expr, &annotation.NegativeNilCheck{ProduceTriggerNever: &annotation.ProduceTriggerNever{}})
return func(self *RootAssertionNode) {
trigger := &annotation.NegativeNilCheck{ProduceTriggerNever: &annotation.ProduceTriggerNever{}}

self.AddProduction(&annotation.ProduceTrigger{
Annotation: trigger,
Expr: expr,
})

// Iterate over already created triggers and update the producer, if necessary.
// Here we can safely match on consumer expressions since we are looking at adding negative nil checks,
// and they rightly should be applied to only consumers with the same expression as the one in the nil check.
if _, ok := expr.(*ast.IndexExpr); ok {
for i := range self.triggers {
s1, _ := asthelper.PrintExpr(self.triggers[i].Consumer.Expr, self.Pass(), false)
s2, _ := asthelper.PrintExpr(expr, self.Pass(), false)
if s1 == s2 {
self.triggers[i] = annotation.FullTrigger{
Producer: &annotation.ProduceTrigger{
Annotation: trigger,
Expr: expr,
},
Consumer: self.triggers[i].Consumer,
}
}
}
}

// if e, ok := expr.(*ast.IndexExpr); ok {
// if _, ok := e.X.(*ast.Ident); ok {
// if _, ok := e.Index.(*ast.Ident); ok {
// for i := range self.triggers {
// if self.eqStableTemp(self.triggers[i].Consumer.Expr, expr) {
// self.triggers[i] = annotation.FullTrigger{
// Producer: &annotation.ProduceTrigger{
// Annotation: trigger,
// Expr: expr,
// },
// Consumer: self.triggers[i].Consumer,
// }
// }
// }
// }
// }
// }
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: In produceNegativeNilCheck, string comparison via asthelper.PrintExpr is used to update triggers. Ensure this comparison remains robust against AST formatting changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants