This revision was automatically updated to reflect the committed changes.
Closed by commit rC322503: [PCH] Serialize skipped preprocessor ranges
(authored by cameron314, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D20124?vs=125333&id=129892#toc
Repository:
rC Clang
htt
cameron314 added a comment.
Excellent, I'll rebase and commit. Thanks everyone for your patience!
https://reviews.llvm.org/D20124
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
LGTM, sorry for the delay.
https://reviews.llvm.org/D20124
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llv
erikjv added a comment.
lgtm. Ilya?
https://reviews.llvm.org/D20124
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
cameron314 added a comment.
Ping?
The patch is ready to go, just needs a final approval...
https://reviews.llvm.org/D20124
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
nik added a comment.
In https://reviews.llvm.org/D20124#943592, @cameron314 wrote:
> Here's the final patch that fixes `clang_getSkippedRegions` with regions in
> the preamble (as well as serializing the skipped regions in the PCH file).
Works fine for me, thanks! Test from the bug report is f
cameron314 updated this revision to Diff 125333.
cameron314 added a comment.
Here's the final patch that fixes `clang_getSkippedRegions` with regions in the
preamble (as well as serializing the skipped regions in the PCH file).
https://reviews.llvm.org/D20124
Files:
include/clang/Lex/Preproc
cameron314 added a comment.
Brilliant, didn't know `isInPreambleFileID` existed. All my tests pass for me
now with that change, thanks :-)
I'll update the patch on Monday.
https://reviews.llvm.org/D20124
___
cfe-commits mailing list
cfe-commits@lis
erikjv added a comment.
Maybe something like this works:
--- a/tools/libclang/CIndex.cpp
+++ b/tools/libclang/CIndex.cpp
@@ -8090,6 +8090,7 @@ CXSourceRangeList
*clang_getSkippedRanges(CXTranslationUnit TU, CXFile file) {
SourceManager &sm = Ctx.getSourceManager();
FileEntry *fil
cameron314 added a comment.
In https://reviews.llvm.org/D20124#928552, @ilya-biryukov wrote:
> Why do we store raw source locations in `PPSkippedRange`? Would storing
> `SourceLocation` and using `ASTWriter::AddSourceLocation` and `ASTReader::
> ReadSourceLocation` do the trick?
I followed t
ilya-biryukov added a comment.
In https://reviews.llvm.org/D20124#927967, @cameron314 wrote:
> Well, it seems like preamble PCH source location translation is fundamentally
> broken. The entry file has a single, positive file ID. The preamble PCH is
> treated as an imported module, so it has a
cameron314 added a comment.
Well, it seems like preamble PCH source location translation is fundamentally
broken. The entry file has a single, positive file ID. The preamble PCH is
treated as an imported module, so it has a negative file ID for the part that
overlaps the preamble of the entry f
cameron314 added a comment.
Alright, with my patch the `c-index-test` *does* correctly serialize and
restore the skipped ranges; the problem is that it searches for only ranges
matching the target file. When there's a preamble, it's seen as a different
file than the main file, even though they'
cameron314 marked an inline comment as done.
cameron314 added a comment.
- Well that's odd, because the test definitely fails for me without the patch.
I'm only a few days behind the trunk.
- I'm looking at your test case now. I can reproduce it even with the patch;
I'm investigating what's happ
nik added a comment.
Here are my observations:
- Your test case works fine for me even without having this change
applied/build. Looks like this is already fixed in current trunk. Please
confirm/test.
- Can you come up with another test case that fixes something that is not yet
addressed in tr
cameron314 updated this revision to Diff 123044.
cameron314 added a comment.
Fully rebased, with a test.
https://reviews.llvm.org/D20124
Files:
include/clang/Lex/PreprocessingRecord.h
include/clang/Serialization/ASTBitCodes.h
include/clang/Serialization/ASTReader.h
include/clang/Seriali
cameron314 added a comment.
I'll rebase the patch and add a test. Thanks for looking at this!
Repository:
rL LLVM
https://reviews.llvm.org/D20124
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listin
erikjv added a comment.
The patch has aged a bit since upload, so does it still apply? And, can you add
a testcase please?
Repository:
rL LLVM
https://reviews.llvm.org/D20124
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.l
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.
This fixes, for example, libclang's `clang_getAllSkippedRanges` returning zero
ranges after reparsing a translation unit.
19 matches
Mail list logo
