[PATCH] D75286: [clangd] Handle clang-tidy suppression comments for diagnostics inside macro expansions

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Sorry for taking a while to come back to this again. A bit chaotic right now :-)

Looks good, some further simplification possible.



================
Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:298
 
+static const char *getCharacterData(const SourceManager &SM, SourceLocation Loc,
+                                    bool *Invalid, bool AvoidIO) {
----------------
This seems more complicated than necessary - e.g. it replicates the "invalid" handling from SourceManager::getCharacterData, when we just bail out in this case. And keeping the raw char* interfaces seems unfortunate if we're going to the trouble of reimplementing most of this anyway.

What about implementing `llvm::Optional<StringRef> getBuffer(FileID File, bool AllowIO)`?
Where AllowIO determines whether you call getbuffer or getrawbuffer.

This is simpler than the current function, and the caller can be a bit more expressive/simpler like:
```
unsigned Offset = SM.getFileOffset(Loc);
StringRef RestOfLine = Data.substr(Offset).split('\n').first;
StringRef PrevLine = Data.substr(0, Offset).rsplit('\n').first.rsplit('\n').second;
```



================
Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h:220
                               const Diagnostic &Info, ClangTidyContext &Context,
-                              bool CheckMacroExpansion = true);
+                              bool AvoidIO = false);
 
----------------
nit: I'd use `AllowIO=true` to avoid the negative sense, up to you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75286