dexonsmith closed this revision.
dexonsmith marked an inline comment as done.
dexonsmith added a comment.
Thanks for the reviews! Committed in r298165.
https://reviews.llvm.org/D28299
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
aprantl added a comment.
Drive-by comment.
Comment at: clang/lib/Basic/MemoryBufferCache.cpp:18
+ std::unique_ptr Buffer) {
+ auto Insertion = Buffers.insert(std::make_pair(
+ Filename, BufferEntry{std::move(Buffer), NextIndex++}));
dexonsmith added a comment.
> ! In https://reviews.llvm.org/D28299#701194, @dexonsmith wrote:
> Rebased again, and cleaned up tests, and a ping... now that dependencies are
> resolved.
>
> Richard and/or Ben, can you take a look?
(Or anyone else that feels comfortable confirming that the
dexonsmith updated this revision to Diff 91799.
dexonsmith added a comment.
Rebased again, and cleaned up tests, and a ping... now that dependencies are
resolved.
Richard and/or Ben, can you take a look?
https://reviews.llvm.org/D28299
Files:
dexonsmith requested review of this revision.
dexonsmith added a comment.
This has changed enough it needs another review. Richard or Ben, could you
take a(nother) look?
https://reviews.llvm.org/D28299
___
cfe-commits mailing list
dexonsmith updated this revision to Diff 87257.
dexonsmith added a comment.
Herald added subscribers: mgorny, nemanjai.
Rebased on top of the current https://reviews.llvm.org/D27689. This also has
some substantive changes:
- Renamed the data structure PCMCache to MemoryBufferCache, and split
According to the comment at line 239:
if (LoadedSuccessfully.count(*victim) == 0) {
// Before removing the module file, check if it was validated in an
// ancestor thread, if yes, throw a hard error instead of causing
// use-after-free in the ancestor thread.
bool
bruno added inline comments.
Comment at: lib/Serialization/ASTReader.cpp:3692
+ValidationConflicts);
+for (auto N : ValidationConflicts)
+ Diag(diag::err_module_ancestor_conflict) << N;
This should honor `ARR_OutOfDate`, so
benlangmuir accepted this revision.
benlangmuir added a comment.
This revision is now accepted and ready to land.
Sorry for the delay, I though you were still iterating with @aprantl for some
reason. LGTM
https://reviews.llvm.org/D28299
___
manmanren added a comment.
Ping :]
Hoping to wrap this up this week.
Cheers,
Manman
https://reviews.llvm.org/D28299
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
manmanren updated this revision to Diff 84137.
manmanren added a comment.
Addressing review comments.
https://reviews.llvm.org/D28299
Files:
include/clang/Basic/DiagnosticSerializationKinds.td
include/clang/Basic/FileManager.h
include/clang/Serialization/ASTReader.h
benlangmuir added inline comments.
Comment at: include/clang/Basic/FileManager.h:176
+ /// Manage memory buffers associated with pcm files.
+ std::unique_ptr BufferMgr;
+
manmanren wrote:
> benlangmuir wrote:
> > Why is this inside the FileManager? It isn't
manmanren added inline comments.
Comment at: include/clang/Basic/DiagnosticSerializationKinds.td:131
+ "diagnostic options now it is a non-system module">,
+ InGroup;
+
aprantl wrote:
> Is this better?
>
> "module file '%0' was validated as a system module
manmanren updated this revision to Diff 83273.
manmanren added a comment.
Add testing case.
https://reviews.llvm.org/D28299
Files:
include/clang/Basic/DiagnosticSerializationKinds.td
include/clang/Basic/FileManager.h
include/clang/Serialization/ASTReader.h
manmanren added a comment.
I forgot to upload the testing case, I will add it now.
Manman
Comment at: include/clang/Basic/FileManager.h:176
+ /// Manage memory buffers associated with pcm files.
+ std::unique_ptr BufferMgr;
+
benlangmuir wrote:
> Why is
benlangmuir added a comment.
Can we test the already-validated diagnostics?
Comment at: include/clang/Basic/FileManager.h:176
+ /// Manage memory buffers associated with pcm files.
+ std::unique_ptr BufferMgr;
+
Why is this inside the FileManager? It isn't
aprantl added inline comments.
Comment at: include/clang/Basic/DiagnosticSerializationKinds.td:131
+ "diagnostic options now it is a non-system module">,
+ InGroup;
+
Is this better?
"module file '%0' was validated as a system module and is now being imported
manmanren created this revision.
manmanren added reviewers: rsmith, benlangmuir, dexonsmith.
manmanren added subscribers: cfe-commits, bruno.
We create a PCMCache class to manage memory buffers associated with pcm files.
With implicit modules, we currently use lock files to make
sure we are
18 matches
Mail list logo
