steakhal added inline comments.
================ Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:504 + + OrigStdin = findStdStream("stdin", C); + OrigStdout = findStdStream("stdout", C); ---------------- balazske wrote: > steakhal wrote: > > balazske wrote: > > > steakhal wrote: > > > > martong wrote: > > > > > We should be careful, to cache the results (either here, or deeper in > > > > > the call stack). > > > > > I mean, we certainly don't want to do a lookup of "stdin" every time > > > > > a function is evaluated. We should do this lazily, only once. > > > > I agree. We should do this only for the first top-level function, > > > > instead of doing this for every top-level function. > > > I am not sure if the `SymbolRef` values are safe to store between > > > top-level function analyses. The `FILE` type and std stream declarations > > > are safe to cache, but the SymbolRef values of these variables probably > > > not. > > I think it should be safe. But place there an assert and run the test > > suite. If it won't trigger, that means that we have high confidentiality > > that this is safe. I know it's not 100%, but pretty close. > I tried to check if these `SymbolRef`'s are the same at `checkBeginFunction` > after initialized once. The assertion for equality failed sometimes, or other > crash happened when `dump` was called on the value. So it looks not safe to > cache these. And should we add assumptions about that these `StdinSym`, > `StdoutSym`, `StderrSym` are not equal to each other? Good to know. I don't think it's necessary to add assertions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106644/new/ https://reviews.llvm.org/D106644 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits