xazax.hun added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:33-45
+ enum KindTy {
+ Opened, /// Stream is opened.
+ Closed, /// Closed stream (an invalid stream pointer after it was closed).
+ OpenFailed /// The last open operation has failed.
+ } State;
+
+ /// The error state of a stream.
----------------
baloghadamsoftware wrote:
> xazax.hun wrote:
> > Szelethus wrote:
> > > balazske wrote:
> > > > Szelethus wrote:
> > > > > Hmm, now that I think of it, could we just merge these 2 enums? Also,
> > > > > I fear that indexers would accidentally assign the comment to the
> > > > > enum after the comma:
> > > > >
> > > > > ```lang=cpp
> > > > > Opened, /// Stream is opened.
> > > > > Closed, /// Closed stream (an invalid stream pointer after it was
> > > > > closed).
> > > > > OpenFailed /// The last open operation has failed.
> > > > > ```
> > > > > ` /// Stream is opened` might be assigned to `Closed`. How about this:
> > > > > ```lang=cpp
> > > > > /// Stream is opened.
> > > > > Opened,
> > > > > /// Closed stream (an invalid stream pointer after it was closed).
> > > > > Closed,
> > > > > /// The last open operation has failed.
> > > > > OpenFailed
> > > > > ```
> > > > Probably these can be merged, it is used for a stream that is in an
> > > > indeterminate state after failed `freopen`, but practically it is
> > > > handled the same way as a closed stream. But this change would be done
> > > > in another revision.
> > > I meant to merge the two enums (`StreamState` and `ErrorKindTy`) and the
> > > fields associated with them (`State` and `ErrorState`). We could however
> > > merge `Closed` and `OpenFailed`, granted that we put a `NoteTag` to
> > > `evalFreopen`. But I agree, that should be another revision's topic :)
> > Since you mentioned that ErrorState is only applicable to Open streams, I
> > am also +1 on merging the enums. These two states are not orthogonal, no
> > reason for them to be separate.
> Not orthogonal, but rather hiearchical. That is a reason for not merging. I
> am completely against it.
In a more expressive language each enum value could have parameters and we
could have
```
Opened(ErrorKind),
Closed,
OpenFailed
```
While we do not have such an expressive language, we can simulate this using
the current constructs such as a variant. The question is, does this worth the
effort? At this point this is more like the matter of taste as long as it is
properly documented.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75682/new/
https://reviews.llvm.org/D75682
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
