ilya-biryukov added inline comments.
================
Comment at: clangd/ClangdUnit.cpp:129
+class ReplayPreamble : public PPCallbacks {
+ const IncludeStructure &Includes;
+ PPCallbacks *Delegate;
----------------
Maybe move fields and the private function to the end of the class?
We definitely don't have a strict rule about this in clangd, but that's what
most of the code does. The upside is that the public interface of the class is
the first thing that readers of the code see.
That's a stylistic thing, so definitely feel free to disagree :-)
================
Comment at: clangd/ClangdUnit.cpp:159
+
+ PP.getPPCallbacks()->InclusionDirective(
+ HashTok.getLocation(), IncludeTok, WrittenFilename, Angled,
----------------
This should be `Delegate->` instead of `PP.getCallbacks()`.
Maybe add a test that catches it?
================
Comment at: clangd/ClangdUnit.cpp:165
+ if (File)
+ PP.getPPCallbacks()->FileSkipped(*File, FilenameTok, Inc.FileKind);
+ else {
----------------
Another one: should this be `Delegate->`?
================
Comment at: clangd/ClangdUnit.cpp:279
+ Clang->getPreprocessor().addPPCallbacks(llvm::make_unique<ReplayPreamble>(
+ Preamble->Includes, Clang->getPreprocessor().getPPCallbacks(),
+ Clang->getSourceManager(), Clang->getPreprocessor(),
----------------
Maybe assert that the pointer returned from `getPPCallbacks()` actually changes
after the call to `addPPCallbacks` call here?
A potential breakage might occur if the implementation of adding a callback
changes and uses an inheritor of `PPCallbacks` that stores a list internally
and adds new callbacks to a list instead of wrapping an existing one.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54694
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
