When I'm new to a file/codebase, I personally find C++'s strong static type system to be a great aid. BugReporter.cpp is still painful to read however: function calls are made with mile long parameter lists, seemingly all of them taken with a non-const reference/pointer. This patch fixes nothing but this: make a few things const, and hammer it until it compiles.
Details
- Reviewers
NoQ xazax.hun Charusso dcoughlin baloghadamsoftware rnkovacs - Commits
- rGfc76d8551f54: [analyzer][NFC] Refactoring BugReporter.cpp P4.: If it can be const, make it…
rC368735: [analyzer][NFC] Refactoring BugReporter.cpp P4.: If it can be const, make it…
rL368735: [analyzer][NFC] Refactoring BugReporter.cpp P4.: If it can be const, make it…
Diff Detail
Event Timeline
It's counter-idiomatic to pass manager objects (ProgramStateManager, SValBuilder, etc.) as const-references because it tells you pretty much nothing about what you can or cannot do with them. You don't really ever semantically mutate them in the first place. There's no need to mention it every time you pass them around.
But when it comes to actual data structures such as ExplodedGraph, we should totally be doing this, yeah :)
Most managers are stateful because they also store the elements they manage (e.g. ProgramStateManager stores states, SValBuilder owns other managers such as SymbolManager that stores symbols etc.) It is also true that BugReporter should not change the existing set of states, symbols etc., just use them in a read-only way. However, I see more confusion than help in making them const here and non-const elsewhere.
That's an implementation detail, a memoization, a good use case for mutable if we go for const-correctness. For example, no observable properties of SymbolManager change when it allocates a new symbol. The interface only allows you to ask "What's the symbol for a value-that-transcends-time with a certain identity?" and it retrieves you that symbol. There's no way to find out if the symbol is freshly created or already cached, and there shouldn't be a way to find this out, because this is entirely immaterial and meaningless.