Page MenuHomePhabricator

[Analyzer][StreamChecker] Changed representation of stream error state - NFC.
ClosedPublic

Authored by balazske on May 15 2020, 6:48 AM.

Details

Summary

State of error flags for a stream is handled by having separate flags
that allow combination of multiple error states to be described with one
error state object.
After a failed function the error state is set in the stream state
and must not be determined later based on the last failed function
like before this change. The error state can not always be determined
from the last failed function and it was not the best design.

Diff Detail

Event Timeline

balazske created this revision.May 15 2020, 6:48 AM
Szelethus accepted this revision.May 18 2020, 5:42 AM

LGTM! Amazing job! Sorry for this dragging out for so long, but I'm very happy with how this patch and the overall direction turned out. I've left an annoying nit, and as always, feel free address it and commit without another round of review.

The error state can not always be determined from the last failed function and it was not the best design.

It was a great idea, still!

clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
40–45

I see what you mean, but in the context of the analyzer, I'd prefer to not use "there is an execution path with .*". Would this imply an a new branch in the exploded graph? Is this execution path modeled or does it only exist conceptually? I think we should avoid this ambiguity by saying something else, like "The steam may or may not have .* flag set".

This revision is now accepted and ready to land.May 18 2020, 5:42 AM
This revision was automatically updated to reflect the committed changes.

There was a problem showing up in the buildbots, it should be fixed now.