[clang-tools-extra] [clangd] Do not collect macros when clang-tidy checks call into the preprocessor (PR #106329)

[kadir çetinkaya via cfe-commits]

================
@@ -702,6 +704,10 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
     log("Execute() failed when building AST for {0}: {1}", MainInput.getFile(),
         toString(std::move(Err)));
 
+  // Disable the macro collector for the remainder of this function, e.g.
+  // clang-tidy checkers.
+  MacroCollectorPtr->doneParse();
----------------
kadircet wrote:

thanks it's unfortunate but i guess makes sense.

in theory, we can have similar issues with all the others PPCallbcks that we installed. Moreover if others were to use clang-tidy as a library, they'll run into similar pitfalls. As the flow is:
- Inject PPCallbacks
- Build AST
- Pass AST to clang-ast-consumer (in this case clang-tidy)
- Invoke EndOfMainFile

So there might be some value in injecting an extra callback between `Build AST` and `Pass AST to consumer`. We could properly reset our PPCallbacks to recognize leaving main file for such situations. It's unfortunate that both `FileChanged` and `LexedFileChanged` are designed to operate with a contract that hints "new file/location" will be valid. It makes such a semantic possibly breaking. Leaving that idea here in case you want to follow up on that (I'd be happy to review), but I can see that it's much more involved, possibly without anything breaking (and if it does we can always ask people to turn that check off until we fix the issue).

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