MyDeveloperDay marked 5 inline comments as done.
MyDeveloperDay added inline comments.
================
Comment at: clang/tools/clang-format/ClangFormat.cpp:310
SourceManager Sources(*Diags, Files);
FileID FileID = createInMemoryFile(AssumedFileName, Code.get(), Sources,
Files, InMemoryFileSystem.get());
----------------
thakis wrote:
> Maybe not for this change, but do you need all this machinery up here?
> SMDiagnostics takes a pointer and a file name, so yu can just pass
> Code->getBufferStart() + R.getOffset() for the pointer and you don't need the
> FileManager, the SourceManager, createInMemoryFile(), the PresumedLoc,
> translateFileLineCol.
>
> You can either try to use SourceMgr::getLineAndColumn() or compute line/col
> yourself -- either should be a lot less code than the current machinery here.
So I definitely agree here, I'm afraid I shamelessly copied this from
elsewhere.. but I see there should be a better way, let me work on that
separately and if its ok I'll send a later review your way.
For now let me reland the frontEnd removal, with your suggested change before.
================
Comment at: clang/tools/clang-format/ClangFormat.cpp:334
+ SMDiagnostic Diags(
+ llvm::SourceMgr(), SMLoc::getFromPointer(StartBuf), AssumedFileName,
+ PLoc.getLine(), PLoc.getColumn(),
----------------
thakis wrote:
> I think the idea is that you have a llvm::SourceMgr() hanging around (above
> the loop or somewhere). SMDiagnostic stores a pointer to this argument, and
> you pass in a temporary. The SourceMgr needs to outlive the Diags object. (In
> practice, the pointer is not used by anything so it happens to work, but
> having the object store a stale pointer seems like potential future trouble,
> and it's easy to prevent -- just put the SourceMgr on the stack).
>
> Also, probably "Diag", not "Diags"?
I made the suggested changes just prior to landing, hope that is ok.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69854/new/
https://reviews.llvm.org/D69854
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
