ilya-biryukov added inline comments.

================
Comment at: clang/lib/Index/IndexingAction.cpp:77
+    IndexCtx->getDataConsumer().setPreprocessor(PP);
+    PP->addPPCallbacks(std::make_unique<IndexPPCallbacks>(IndexCtx));
+  }
----------------
The fact that we call `addPPCallbacks` **after** creating `ASTContext` is a bit 
concerning.

This wouldn't probably cause any problems in practice, but generally changing 
preprocessor by the time `ASTContext` is already created seems dangerous (what 
if something there ever starts depending on the state of the preprocessor?)

I think this concern is easily elevated if we change Preprocessor and call 
addPPCallbacks on `CreateASTConsumer(CompilerInvocation& CI)`. That would mean 
we have a separate function that sets up the preprocessor and creates 
`IndexASTConsumer` and it should be exposed to the clients.

Have you considered this approach? Would it work?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66877/new/

https://reviews.llvm.org/D66877



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to