[clang] [clang-tools-extra] [clang][nfc] Define ConstRecursiveASTVisitor twin of RecursiveASTVisitor (PR #160065)

[Arseniy Zaostrovnykh via cfe-commits]

necto wrote:

> Have you tried using `DynamicRecursiveASTVisitor`/`ConstDynamicRecursiveASTVisitor` instead? It should cover _most_ use cases (unless you need to override e.g. `WalkUpFromXY` and friends).

I have not. I am worried of the runtime performance because of the virtual dispatch overhead, but I have not benchmarked it.

> I am very much against adding this unless there is a very strong motivation for it that isn’t already covered by `ConstDynamicRecursiveASTVisitor`: Adding even a single instantiation of RAV has a measurable impact on Clang’s compilation speed (even just adding `ConstDynamicRecursiveASTVisitor`, which adds a single instantiation of RAV, came with a compile-time performance regression of about .05% iirc), to the point where we’ve been trying to get rid of uses of RAV in favour of DRAV as much as possible. We don’t exactly want people to start writing _more_ RAVs.

Is there a trend of avoiding `RecursiveASTVisitor` uses in Clang codebase? Do you think more all uses of RAVs should be converted to uses of DRAVs?

> Even if we never instantiate this new visitor upstream, the base class means there is one extra class to instantiate, and I am worried that even just parsing a huge template like this might be an issue (last time I checked, `RecursiveASTVisitor.h` proper would expand to something like 20 000 LOC because of all the macros...), so if we _really_ need this for whatever reason, it should be benchmarked first to make sure it doesn’t introduce a performance regression.

Are you speaking of the compile-time regression, or are you worried it would bloat the executable size which might slow down it at runtime?
TBH, I am leaning towards the use of static polimorfism over the dynamic one also out of performance concerns. However so far it was just a guess that I did not validate. Are you saying that use of DRAV speeds up execution rather than slowing it down?



https://github.com/llvm/llvm-project/pull/160065