sammccall added inline comments.
================
Comment at: clang/include/clang/Lex/PPCallbacks.h:346
+
+ virtual void setPreprocessor(Preprocessor *preprocessor) {
+ preprocessor_ = preprocessor;
----------------
alexfh wrote:
> sammccall wrote:
> > This seems pretty tangly from a layering point of view, giving each object
> > a reference to the other. I always find it hard to reason about ownership
> > in such cases.
> >
> > What about lifecycle methods `beginPreprocessing(Preprocessor*)` and
> > `endPreprocessing(Preprocessor*)`? It will look similar in practice:
> > subclasses that need it can still track the PP instance, but the circular
> > references will only be there when needed, and will be explicit.
> We can do that as well. However, adding another couple of overrides to each
> PPCallbacks implementation will result in quite some boilerplate. How would
> you look at creating an intermediate class in the `PPCallbacks` -> <users'
> PPCallbacks handlers> hierarchy that will handle
> `beginPreprocessing`/`endPreprocessing` and expose the preprocessor pointer
> via a protected `getPreprocessor()` method?
I think that would probably obscure the intent, I really think this is subtle
enough to be explicit, and would be only about as boilerplatey as today (the PP
pointer is set by a method rather than in the constructor).
But I'm more concerned about the interfaces in Lex/ than the specific callsites
in clang-tidy, so if such a helper class were private to clang-tidy, that seems
ok.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58278/new/
https://reviews.llvm.org/D58278
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
