I just read this part of the code, and I found the nested ifs less readable.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
I should note that I compared this on multiple projects to the baseline version and no runtime nor reports changed.
So, this is purely a cosmetic change.
clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp | ||
---|---|---|
599–603 | Why don't just leave this snippet as it was but add const SourceLocation Loc instead of re-assigning SL? Your construction made me stumbled for a while. As this patch is meant to bring readability, I wouldn't like to see such a code-trick. I see your intention to hide SL but IMO let compiler do this for us. See suggested. |
clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp | ||
---|---|---|
599–603 | Hm, you are probably right. The main idea was to reduce the number of unrelated declarations in the scope by nesting them into the calculation of the important ones. |
Why don't just leave this snippet as it was but add const SourceLocation Loc instead of re-assigning SL? Your construction made me stumbled for a while. As this patch is meant to bring readability, I wouldn't like to see such a code-trick. I see your intention to hide SL but IMO let compiler do this for us. See suggested.