benlangmuir added inline comments.
================
Comment at: include/clang/Basic/FileManager.h:176
+ /// Manage memory buffers associated with pcm files.
+ std::unique_ptr<PCMCache> BufferMgr;
+
----------------
manmanren wrote:
> benlangmuir wrote:
> > Why is this inside the FileManager? It isn't used by the FileManager.
> I initially had another patch that puts PCMCache object at the top level (i.e
> whenever we create a FileManager, we create a PCMCache object). The initial
> patch is much bigger than this patch. PCMCache is cacheing the content of a
> PCM File, and is shared across threads that we spawned to build modules
> implicitly, just like FileManager. I had some discussions with Bruno and
> Adrian, we felt FileManager is a better place. But if you have other
> suggestions, please let me know!
I see. It's not ideal, but I see why it would be convenient to use the
FileManager this way, since you want to share it in all the same places the
FileManager is shared. Since I don't have any better answer, I guess I'm okay
with this...
================
Comment at: include/clang/Basic/FileManager.h:308
+ /// a thread, we pop a ThreadContext.
+ struct ThreadContext {
+ /// Keep track of all module files that have been validated in this thread
----------------
manmanren wrote:
> benlangmuir wrote:
> > Can we call this something like "ModuleLoadContext" or maybe
> > "ModuleCompilationContext"? The word thread is misleading, since the
> > threads are not run concurrently, and are only an implementation detail of
> > our crash recovery.
> >
> > Also, can you explain why it is only necessary to keep information about
> > the the current stack of contexts? In a situation like
> >
> > A imports B imports C
> > A imports D imports C
> >
> > We would no longer have information about C being validated, right? What
> > happens if there's a mismatch there? Can this never happen?
> Sure, we can call it ModuleCompilationContext because we are spawning threads
> to build modules implicitly.
>
> The issue we are trying to detect is use-after-free, i.e we don't want a
> thread to hold on to an invalid FileEntry. We error out before trying to
> delete the FileEntry that is live in another thread.
>
> In a situation like
> A imports B imports C
> A imports D imports C
>
> A will start a thread to build B, and B will start a thread to build C, C
> will be validated in B's compilation thread and A's compilation thread. Later
> on, if D tries to invalidate C, D knows that an ancestor thread A has
> validated C, so the compiler will error out. And it is okay for A to
> invalidate C in A's compilation thread, because we can clean up the
> references to C's FileEntry.
>
Okay
https://reviews.llvm.org/D28299
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
