cameron314 abandoned this revision.
cameron314 added a comment.
This patch is obsolete. While waiting over a year for a review, somebody else
came across the same fix (for a different manifestation of the same bug) in
https://reviews.llvm.org/D27810 and managed to get it through. I think my
cameron314 added a comment.
Anyone have time to check this out this week?
It's a one-line fix, includes a test, and is for a fairly important bug :-)
https://reviews.llvm.org/D20338
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
cameron314 added a comment.
Anyone have a few minutes to look at this?
http://reviews.llvm.org/D20338
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
cameron314 added a comment.
Ping? :-)
http://reviews.llvm.org/D20338
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
cameron314 updated this revision to Diff 60250.
cameron314 added a comment.
Here's the final fix (it's the line in FileManager.cpp, plus a test).
http://reviews.llvm.org/D20338
Files:
include/clang/Frontend/ASTUnit.h
lib/Basic/FileManager.cpp
lib/Frontend/ASTUnit.cpp
cameron314 added inline comments.
Comment at: lib/Basic/FileManager.cpp:389
@@ -383,2 +388,3 @@
UFE->File.reset();
+ UFE->IsVirtual = true;
return UFE;
rsmith wrote:
> Yes. The `IsValid` flag is just supposed to mean that this file has actually
> been
rsmith added inline comments.
Comment at: lib/Basic/FileManager.cpp:389
@@ -383,2 +388,3 @@
UFE->File.reset();
+ UFE->IsVirtual = true;
return UFE;
Yes. The `IsValid` flag is just supposed to mean that this file has actually
been added to the
cameron314 added a comment.
This is a fairly important bug for anyone hosting clang as a library (e.g.
IDEs).
Can someone have a look at this patch when they have a free moment?
http://reviews.llvm.org/D20338
___
cfe-commits mailing list
cameron314 updated the summary for this revision.
cameron314 removed rL LLVM as the repository for this revision.
cameron314 updated this revision to Diff 59577.
cameron314 added a comment.
It took some modifications to the ASTUnit to support a virtual file system with
a PCH parse/reparse
cameron314 added a comment.
Thanks @bruno, I'll have a look at using a VFS for the test.
Comment at: lib/Basic/FileManager.cpp:389
@@ -383,2 +388,3 @@
UFE->File.reset();
+ UFE->IsVirtual = true;
return UFE;
rsmith wrote:
> Rather than adding this
rsmith added inline comments.
Comment at: lib/Basic/FileManager.cpp:389
@@ -383,2 +388,3 @@
UFE->File.reset();
+ UFE->IsVirtual = true;
return UFE;
Rather than adding this `IsVirtual` flag, could you just set `UFE->IsValid` to
`true` here? It looks like
bruno added a comment.
You can probably find a way to test this by taking a look at
unittests/Basic/VirtualFileSystemTest.cpp
Repository:
rL LLVM
http://reviews.llvm.org/D20338
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
cameron314 added a comment.
I'm not sure how to test this (originally I found this bug by stepping through
with a debugger) -- is there a way to determine if an ASTUnit used a PCH for
the preamble or not? I'd call the `getMainBufferWithPrecompiledPreamble` method
manually but it's private.
bruno added a subscriber: bruno.
bruno added a comment.
Hi Cameron,
Can you add a testcase?
Repository:
rL LLVM
http://reviews.llvm.org/D20338
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
cameron314 created this revision.
cameron314 added a reviewer: rsmith.
cameron314 added a subscriber: cfe-commits.
cameron314 set the repository for this revision to rL LLVM.
Remapped files would always cause the preamble's PCH to be invalidated (even if
they hadn't changed) because the file
15 matches
Mail list logo
