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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
