

Hello!
Why does the 'generateVisitorsDiagnostics()' function in BugReporter.cpp work backwards? It starts from the node of the error and iterate over the preds.
I have tried to make it to work forward with storing the 'const ExplodedNode *' nodes in a vector, but 'auto P = V>VisitNode(NextNode, BRC, *R);' crashes. I just found out the 'LastPiece' checking could be moved out from the whileloop, that is it for now.
Is it possible to achieve? What is could be problematic with that code snippet?
Thanks you for the suggestions, Csaba.
_______________________________________________
cfedev mailing list
[hidden email]
http://lists.llvm.org/cgibin/mailman/listinfo/cfedev


Going forward is ambiguous, as there are many leafs, but only one root.
What are you trying to achieve? Going backwards should never be a problem.
> On Nov 16, 2018, at 8:21 AM, Csaba Dabis via cfedev < [hidden email]> wrote:
>
> Hello!
>
> Why does the 'generateVisitorsDiagnostics()' function in BugReporter.cpp work backwards? It starts from the node of the error and iterate over the preds.
>
> I have tried to make it to work forward with storing the 'const ExplodedNode *' nodes in a vector, but 'auto P = V>VisitNode(NextNode, BRC, *R);' crashes. I just found out the 'LastPiece' checking could be moved out from the whileloop, that is it for now.
>
> Is it possible to achieve? What is could be problematic with that code snippet?
>
> Thanks you for the suggestions,
> Csaba.
> _______________________________________________
> cfedev mailing list
> [hidden email]
> http://lists.llvm.org/cgibin/mailman/listinfo/cfedev_______________________________________________
cfedev mailing list
[hidden email]
http://lists.llvm.org/cgibin/mailman/listinfo/cfedev


The current way of bug reporting:  1 Assuming 'i' is not equal to 1  2 Assuming 'i' is not equal to 2 3 Taking false branch 4 Taking false branch 
If I would like to hook extra information in the linear backwards way, this is going to be like the following:  1 Assuming 'i' is not equal to 1  2 Knowing 'i' is not equal to 2 3 Knowing 'i' is not equal to 1 4 Assuming 'i' is not equal to [1, 2] 
And actually the proper approach would be:  1 Assuming 'i' is not equal to 1  2 Assuming 'i' is not equal to 2 3 Knowing 'i' is not equal to [1, 2] 4 Knowing 'i' is not equal to [1, 2] 
George Karpenkov < [hidden email]> ezt írta (időpont: 2018. nov. 16., P, 19:04): Going forward is ambiguous, as there are many leafs, but only one root.
What are you trying to achieve? Going backwards should never be a problem.
> On Nov 16, 2018, at 8:21 AM, Csaba Dabis via cfedev <[hidden email]> wrote:
>
> Hello!
>
> Why does the 'generateVisitorsDiagnostics()' function in BugReporter.cpp work backwards? It starts from the node of the error and iterate over the preds.
>
> I have tried to make it to work forward with storing the 'const ExplodedNode *' nodes in a vector, but 'auto P = V>VisitNode(NextNode, BRC, *R);' crashes. I just found out the 'LastPiece' checking could be moved out from the whileloop, that is it for now.
>
> Is it possible to achieve? What is could be problematic with that code snippet?
>
> Thanks you for the suggestions,
> Csaba.
> _______________________________________________
> cfedev mailing list
> [hidden email]
> http://lists.llvm.org/cgibin/mailman/listinfo/cfedev
_______________________________________________
cfedev mailing list
[hidden email]
http://lists.llvm.org/cgibin/mailman/listinfo/cfedev


I’m still not exactly sure what are you doing, but you can store all the information you need in a vector inside the visitor, and then iterate over it in the callback for the last node. Is that what you’ve tried originally? Where does it crash?
The current way of bug reporting:  1 Assuming 'i' is not equal to 1  2 Assuming 'i' is not equal to 2 3 Taking false branch 4 Taking false branch 
If I would like to hook extra information in the linear backwards way, this is going to be like the following:  1 Assuming 'i' is not equal to 1  2 Knowing 'i' is not equal to 2 3 Knowing 'i' is not equal to 1 4 Assuming 'i' is not equal to [1, 2] 
And actually the proper approach would be:  1 Assuming 'i' is not equal to 1  2 Assuming 'i' is not equal to 2 3 Knowing 'i' is not equal to [1, 2] 4 Knowing 'i' is not equal to [1, 2] 
George Karpenkov < [hidden email]> ezt írta (időpont: 2018. nov. 16., P, 19:04): Going forward is ambiguous, as there are many leafs, but only one root.
What are you trying to achieve? Going backwards should never be a problem.
> On Nov 16, 2018, at 8:21 AM, Csaba Dabis via cfedev <[hidden email]> wrote:
>
> Hello!
>
> Why does the 'generateVisitorsDiagnostics()' function in BugReporter.cpp work backwards? It starts from the node of the error and iterate over the preds.
>
> I have tried to make it to work forward with storing the 'const ExplodedNode *' nodes in a vector, but 'auto P = V>VisitNode(NextNode, BRC, *R);' crashes. I just found out the 'LastPiece' checking could be moved out from the whileloop, that is it for now.
>
> Is it possible to achieve? What is could be problematic with that code snippet?
>
> Thanks you for the suggestions,
> Csaba.
> _______________________________________________
> cfedev mailing list
> [hidden email]
> http://lists.llvm.org/cgibin/mailman/listinfo/cfedev
_______________________________________________
cfedev mailing list
[hidden email]
http://lists.llvm.org/cgibin/mailman/listinfo/cfedev


Thanks you! This is a very good idea. I just wanted to rewrite the root of the visiting to prevent problems like that in the future, as the following (in BugReporter.cpp):
static std::unique_ptr<VisitorsDiagnosticsTy> generateVisitorsDiagnostics(BugReport *R, const ExplodedNode *ErrorNode, BugReporterContext &BRC) { typedef std::vector<const ExplodedNode *> NodePath;
auto Notes = llvm::make_unique<VisitorsDiagnosticsTy>(); BugReport::VisitorList visitors; NodePath Path;
// Run visitors on all nodes starting from the node *before* the last one. // The last node is reserved for notes generated with {@code getEndPath}. const ExplodedNode *Pred = ErrorNode; while (Pred) { Pred = Pred>getFirstPred(); if (!Pred) break;
if (!R>isValid()) break;
Path.push_back(Pred); }
for (NodePath::reverse_iterator I = Path.rbegin(); I != Path.rend(); ++I) { const ExplodedNode *NextNode = *I; // At each iteration, move all visitors from report to visitor list. for (BugReport::visitor_iterator VI = R>visitor_begin(), E = R>visitor_end(); VI != E; ++VI) { visitors.push_back(std::move(*VI)); } R>clearVisitors();
for (auto &V : visitors) { llvm::errs() << "\nbefore crash  it is written out"; auto P = V>VisitNode(NextNode, BRC, *R); llvm::errs() << "\nafter crash"; if (P) (*Notes)[NextNode].push_back(std::move(P)); }
if (!R>isValid()) break; }
std::shared_ptr<PathDiagnosticPiece> LastPiece; for (auto &V : visitors) { V>finalizeVisitor(BRC, ErrorNode, *R);
if (auto Piece = V>getEndPath(BRC, ErrorNode, *R)) { assert(!LastPiece && "There can only be one final piece in a diagnostic."); LastPiece = std::move(Piece); (*Notes)[ErrorNode].push_back(LastPiece); } }
return Notes; }
I think you are right about it I should change things locally, so I will drop that patch, because it is too difficult for me. George Karpenkov < [hidden email]> ezt írta (időpont: 2018. nov. 16., P, 19:25): I’m still not exactly sure what are you doing, but you can store all the information you need in a vector inside the visitor, and then iterate over it in the callback for the last node. Is that what you’ve tried originally? Where does it crash?
The current way of bug reporting:  1 Assuming 'i' is not equal to 1  2 Assuming 'i' is not equal to 2 3 Taking false branch 4 Taking false branch 
If I would like to hook extra information in the linear backwards way, this is going to be like the following:  1 Assuming 'i' is not equal to 1  2 Knowing 'i' is not equal to 2 3 Knowing 'i' is not equal to 1 4 Assuming 'i' is not equal to [1, 2] 
And actually the proper approach would be:  1 Assuming 'i' is not equal to 1  2 Assuming 'i' is not equal to 2 3 Knowing 'i' is not equal to [1, 2] 4 Knowing 'i' is not equal to [1, 2] 
George Karpenkov < [hidden email]> ezt írta (időpont: 2018. nov. 16., P, 19:04): Going forward is ambiguous, as there are many leafs, but only one root.
What are you trying to achieve? Going backwards should never be a problem.
> On Nov 16, 2018, at 8:21 AM, Csaba Dabis via cfedev <[hidden email]> wrote:
>
> Hello!
>
> Why does the 'generateVisitorsDiagnostics()' function in BugReporter.cpp work backwards? It starts from the node of the error and iterate over the preds.
>
> I have tried to make it to work forward with storing the 'const ExplodedNode *' nodes in a vector, but 'auto P = V>VisitNode(NextNode, BRC, *R);' crashes. I just found out the 'LastPiece' checking could be moved out from the whileloop, that is it for now.
>
> Is it possible to achieve? What is could be problematic with that code snippet?
>
> Thanks you for the suggestions,
> Csaba.
> _______________________________________________
> cfedev mailing list
> [hidden email]
> http://lists.llvm.org/cgibin/mailman/listinfo/cfedev
_______________________________________________
cfedev mailing list
[hidden email]
http://lists.llvm.org/cgibin/mailman/listinfo/cfedev


Yes, changing BugReporter.cpp is a very nontrivial task, and should generally be avoided.
Thanks you! This is a very good idea. I just wanted to rewrite the root of the visiting to prevent problems like that in the future, as the following (in BugReporter.cpp):
static std::unique_ptr<VisitorsDiagnosticsTy> generateVisitorsDiagnostics(BugReport *R, const ExplodedNode *ErrorNode, BugReporterContext &BRC) { typedef std::vector<const ExplodedNode *> NodePath;
auto Notes = llvm::make_unique<VisitorsDiagnosticsTy>(); BugReport::VisitorList visitors; NodePath Path;
// Run visitors on all nodes starting from the node *before* the last one. // The last node is reserved for notes generated with {@code getEndPath}. const ExplodedNode *Pred = ErrorNode; while (Pred) { Pred = Pred>getFirstPred(); if (!Pred) break;
if (!R>isValid()) break;
Path.push_back(Pred); }
for (NodePath::reverse_iterator I = Path.rbegin(); I != Path.rend(); ++I) { const ExplodedNode *NextNode = *I; // At each iteration, move all visitors from report to visitor list. for (BugReport::visitor_iterator VI = R>visitor_begin(), E = R>visitor_end(); VI != E; ++VI) { visitors.push_back(std::move(*VI)); } R>clearVisitors();
for (auto &V : visitors) { llvm::errs() << "\nbefore crash  it is written out"; auto P = V>VisitNode(NextNode, BRC, *R); llvm::errs() << "\nafter crash"; if (P) (*Notes)[NextNode].push_back(std::move(P)); }
if (!R>isValid()) break; }
std::shared_ptr<PathDiagnosticPiece> LastPiece; for (auto &V : visitors) { V>finalizeVisitor(BRC, ErrorNode, *R);
if (auto Piece = V>getEndPath(BRC, ErrorNode, *R)) { assert(!LastPiece && "There can only be one final piece in a diagnostic."); LastPiece = std::move(Piece); (*Notes)[ErrorNode].push_back(LastPiece); } }
return Notes; }
I think you are right about it I should change things locally, so I will drop that patch, because it is too difficult for me. George Karpenkov < [hidden email]> ezt írta (időpont: 2018. nov. 16., P, 19:25): I’m still not exactly sure what are you doing, but you can store all the information you need in a vector inside the visitor, and then iterate over it in the callback for the last node. Is that what you’ve tried originally? Where does it crash?
The current way of bug reporting:  1 Assuming 'i' is not equal to 1  2 Assuming 'i' is not equal to 2 3 Taking false branch 4 Taking false branch 
If I would like to hook extra information in the linear backwards way, this is going to be like the following:  1 Assuming 'i' is not equal to 1  2 Knowing 'i' is not equal to 2 3 Knowing 'i' is not equal to 1 4 Assuming 'i' is not equal to [1, 2] 
And actually the proper approach would be:  1 Assuming 'i' is not equal to 1  2 Assuming 'i' is not equal to 2 3 Knowing 'i' is not equal to [1, 2] 4 Knowing 'i' is not equal to [1, 2] 
George Karpenkov < [hidden email]> ezt írta (időpont: 2018. nov. 16., P, 19:04): Going forward is ambiguous, as there are many leafs, but only one root.
What are you trying to achieve? Going backwards should never be a problem.
> On Nov 16, 2018, at 8:21 AM, Csaba Dabis via cfedev <[hidden email]> wrote:
>
> Hello!
>
> Why does the 'generateVisitorsDiagnostics()' function in BugReporter.cpp work backwards? It starts from the node of the error and iterate over the preds.
>
> I have tried to make it to work forward with storing the 'const ExplodedNode *' nodes in a vector, but 'auto P = V>VisitNode(NextNode, BRC, *R);' crashes. I just found out the 'LastPiece' checking could be moved out from the whileloop, that is it for now.
>
> Is it possible to achieve? What is could be problematic with that code snippet?
>
> Thanks you for the suggestions,
> Csaba.
> _______________________________________________
> cfedev mailing list
> [hidden email]
> http://lists.llvm.org/cgibin/mailman/listinfo/cfedev
_______________________________________________
cfedev mailing list
[hidden email]
http://lists.llvm.org/cgibin/mailman/listinfo/cfedev

