Page MenuHomePhabricator

cameron314 (Cameron)
User

Projects

User does not belong to any projects.

User Details

User Since
Feb 9 2016, 1:33 PM (147 w, 6 d)

Recent Activity

Nov 5 2018

cameron314 added a comment to D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected.

Ping?

Nov 5 2018, 12:29 PM

Oct 22 2018

cameron314 added a comment to D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected.

I suppose we could but there's a few places outside of SBProcess that also use the run lock and API mutex; personally, I prefer it to be explicit which mutex is being taken first.

Oct 22 2018, 1:01 PM
cameron314 added a reviewer for D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected: bkramer.
Oct 22 2018, 6:28 AM

Oct 18 2018

cameron314 created D53412: [lldb] Fixed deadlock when SBProcess is Kill()ed and inspected.
Oct 18 2018, 2:34 PM

Mar 12 2018

cameron314 added a comment to D43453: Fix overloaded static functions for templates.

Here's the file we use in our parameter info tests locally (that don't pass with the trunk of clang, but with your changes should now theoretically pass):

Mar 12 2018, 8:02 AM

Feb 8 2018

cameron314 added a comment to D41005: Reuse preamble even if an unsaved file does not exist.

@yvvan: The clang frontend tests (PCHPreambleTest and friends) are disabled on Windows in the makefile (I think because the VFS tests depend on linux-like paths). So running the tests on Windows without failures is encouraging but not the whole story.

Feb 8 2018, 6:41 AM

Jan 15 2018

cameron314 committed rL322513: Fixed memory leak in unit test introduced in my previous commit r322503.
Fixed memory leak in unit test introduced in my previous commit r322503
Jan 15 2018, 12:39 PM
cameron314 committed rC322513: Fixed memory leak in unit test introduced in my previous commit r322503.
Fixed memory leak in unit test introduced in my previous commit r322503
Jan 15 2018, 12:38 PM
cameron314 committed rC322503: [PCH] Serialize skipped preprocessor ranges.
[PCH] Serialize skipped preprocessor ranges
Jan 15 2018, 11:15 AM
cameron314 committed rL322503: [PCH] Serialize skipped preprocessor ranges.
[PCH] Serialize skipped preprocessor ranges
Jan 15 2018, 11:15 AM
cameron314 closed D20124: [PCH] Serialize skipped preprocessor ranges.
Jan 15 2018, 11:15 AM
cameron314 added a comment to D20124: [PCH] Serialize skipped preprocessor ranges.

Excellent, I'll rebase and commit. Thanks everyone for your patience!

Jan 15 2018, 6:33 AM

Dec 14 2017

cameron314 added a comment to D20124: [PCH] Serialize skipped preprocessor ranges.

Ping?
The patch is ready to go, just needs a final approval...

Dec 14 2017, 8:14 AM

Dec 8 2017

cameron314 added a comment to D41005: Reuse preamble even if an unsaved file does not exist.

It's been a while since I was in this code, but I seem to recall the file needed to exist on disk in order to uniquely identify it (via inode). Does this break the up-to-date check?

Dec 8 2017, 7:40 AM

Dec 6 2017

cameron314 updated the summary of D20124: [PCH] Serialize skipped preprocessor ranges.
Dec 6 2017, 9:35 AM

Dec 4 2017

cameron314 added inline comments to D36390: Fix overloaded static functions in SemaCodeComplete.
Dec 4 2017, 3:16 PM
cameron314 added a comment to D39903: [libclang] Allow pretty printing declarations.

Locally we've done something similar (adding a clang_getCursorPrettyPrintedDeclaration function, though without exposing the PrintingPolicy) and overhauled DeclPrinter to produce proper pretty names. This is a hack that works well for us, but can never be upstreamed since it breaks too much existing code (and some of the printing decisions are subjective). Your way is better.

Dec 4 2017, 1:50 PM
cameron314 updated the diff for D20124: [PCH] Serialize skipped preprocessor ranges.

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).

Dec 4 2017, 6:54 AM

Dec 1 2017

cameron314 added a comment to D20124: [PCH] Serialize skipped preprocessor ranges.

Brilliant, didn't know isInPreambleFileID existed. All my tests pass for me now with that change, thanks :-)
I'll update the patch on Monday.

Dec 1 2017, 4:49 PM

Nov 20 2017

cameron314 added a comment to D20124: [PCH] Serialize skipped preprocessor ranges.

Why do we store raw source locations in PPSkippedRange? Would storing SourceLocation and using ASTWriter::AddSourceLocation and ASTReader:: ReadSourceLocation do the trick?

Nov 20 2017, 7:59 AM

Nov 16 2017

cameron314 added a comment to D20124: [PCH] Serialize skipped preprocessor ranges.

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 file. That means locations in the preamble part of the entry file can have two different file IDs depending on how they are arrived at.

Nov 16 2017, 2:33 PM
cameron314 added a comment to D20124: [PCH] Serialize skipped preprocessor ranges.

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're really one and the same in this case.

Nov 16 2017, 1:38 PM
cameron314 added a comment to D20124: [PCH] Serialize skipped preprocessor ranges.
  • 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 happening.
  • I've fixed the warning, thanks! Using MSVC on my end which doesn't emit that particular warning.
Nov 16 2017, 12:14 PM

Nov 15 2017

cameron314 updated the diff for D20124: [PCH] Serialize skipped preprocessor ranges.

Fully rebased, with a test.

Nov 15 2017, 9:33 AM

Nov 8 2017

cameron314 added inline comments to D36390: Fix overloaded static functions in SemaCodeComplete.
Nov 8 2017, 12:10 PM
cameron314 added inline comments to D36390: Fix overloaded static functions in SemaCodeComplete.
Nov 8 2017, 7:25 AM

Nov 7 2017

cameron314 added a comment to D20124: [PCH] Serialize skipped preprocessor ranges.

I'll rebase the patch and add a test. Thanks for looking at this!

Nov 7 2017, 2:14 PM

Oct 19 2017

cameron314 added a comment to D38578: [preamble] Also record the "skipping" state of the preprocessor.

@nik, pretty sure this fix I submitted ages ago fixes that. Been using it in production a couple years. Would be nice if someone reviewed it so we could finally upstream it.

Oct 19 2017, 11:21 AM

Sep 20 2017

cameron314 committed rL313802: Fixed unused variable warning introduced in r313796 causing build failure.
Fixed unused variable warning introduced in r313796 causing build failure
Sep 20 2017, 12:39 PM
cameron314 committed rL313796: [PCH] Fixed preamble breaking with BOM presence (and particularly, fluctuating….
[PCH] Fixed preamble breaking with BOM presence (and particularly, fluctuating…
Sep 20 2017, 12:05 PM
cameron314 closed D37491: [Preamble] Fixed preamble breaking with BOM presence (and particularly, fluctuating BOM presence) by committing rL313796: [PCH] Fixed preamble breaking with BOM presence (and particularly, fluctuating….
Sep 20 2017, 12:05 PM
cameron314 updated the diff for D37491: [Preamble] Fixed preamble breaking with BOM presence (and particularly, fluctuating BOM presence).

Final diff. Test passes!

Sep 20 2017, 11:51 AM

Sep 14 2017

cameron314 added inline comments to D37491: [Preamble] Fixed preamble breaking with BOM presence (and particularly, fluctuating BOM presence).
Sep 14 2017, 12:40 PM
cameron314 added inline comments to D37491: [Preamble] Fixed preamble breaking with BOM presence (and particularly, fluctuating BOM presence).
Sep 14 2017, 9:24 AM

Sep 13 2017

cameron314 updated the diff for D37491: [Preamble] Fixed preamble breaking with BOM presence (and particularly, fluctuating BOM presence).

Alright, I've changed the patch so that the preamble takes into account the BOM presence and is invalidated when it changes. This automatically fixes all clients of PrecompiledPreamble, and ensures that all SourceLocations are always consistent when using a PCH generated from the preamble.

Sep 13 2017, 1:06 PM

Sep 11 2017

cameron314 committed rL312917: [PCH] Allow VFS to be used for tests that generate PCH files.
[PCH] Allow VFS to be used for tests that generate PCH files
Sep 11 2017, 8:05 AM
cameron314 closed D37474: [PCH] Allow VFS to be used for tests that generate PCH files by committing rL312917: [PCH] Allow VFS to be used for tests that generate PCH files.
Sep 11 2017, 8:05 AM
cameron314 added inline comments to D37474: [PCH] Allow VFS to be used for tests that generate PCH files.
Sep 11 2017, 7:18 AM
cameron314 updated the diff for D37474: [PCH] Allow VFS to be used for tests that generate PCH files.

Final diff, will commit soon. Thanks!

Sep 11 2017, 7:17 AM

Sep 8 2017

cameron314 added a comment to D37491: [Preamble] Fixed preamble breaking with BOM presence (and particularly, fluctuating BOM presence).

It seems there's other users of PrecompiledPreamble that would have to be fixed, yes. If we go with my original fix of taking into account the BOM in the preamble bounds, there's no way of reusing the PCH when the BOM appears/disappears. I still maintain this is a common use case for IDE-type clients. This type of performance bug is very hard to track down.

Sep 8 2017, 12:55 PM
cameron314 updated the diff for D37474: [PCH] Allow VFS to be used for tests that generate PCH files.

The latest patch. I think this one should do the trick :-)

Sep 8 2017, 12:22 PM
cameron314 added inline comments to D37474: [PCH] Allow VFS to be used for tests that generate PCH files.
Sep 8 2017, 10:33 AM

Sep 7 2017

cameron314 updated the diff for D37491: [Preamble] Fixed preamble breaking with BOM presence (and particularly, fluctuating BOM presence).

Here's an updated patch. The code required to make it work is much simpler when the BOM is simply ignored :-)

Sep 7 2017, 12:12 PM
cameron314 added a comment to D37491: [Preamble] Fixed preamble breaking with BOM presence (and particularly, fluctuating BOM presence).

Maybe there's a third option option to remove the BOM from the buffer before passing it to clang?
Could you elaborate on your use-case a little more? Is there no way to consistently always pass buffers either with or without BOM?
Out of two options you mention discarding preamble on BOM changes seems like an easy option that is both correct and won't make a difference in performance since BOM rarely changes.
Looking at your use-case, it sounds like you'll only have 1 extra reparse of preamble, which is probably fine.

Sep 7 2017, 6:30 AM
cameron314 added a comment to D37474: [PCH] Allow VFS to be used for tests that generate PCH files.

Looking at the way remapped buffers are handled, I just remembered that they must exist on the file system (at the very least, in a directory that exists) or the remapping is not taken into account. So that pretty much rules out the other approach, I think.

Sep 7 2017, 6:07 AM

Sep 6 2017

cameron314 updated the diff for D37474: [PCH] Allow VFS to be used for tests that generate PCH files.

Here's an updated patch that allows only the PCH to be accessed from the real FS when a VSF is present. Tests still pass.

Sep 6 2017, 8:51 AM
cameron314 added a comment to D37474: [PCH] Allow VFS to be used for tests that generate PCH files.

I'll change the overlay to only allow access to the PCH.

Sep 6 2017, 7:50 AM
cameron314 added a comment to D37491: [Preamble] Fixed preamble breaking with BOM presence (and particularly, fluctuating BOM presence).

Thanks for the response!

Sep 6 2017, 7:41 AM

Sep 5 2017

cameron314 created D37491: [Preamble] Fixed preamble breaking with BOM presence (and particularly, fluctuating BOM presence).
Sep 5 2017, 12:56 PM
cameron314 added a comment to D37474: [PCH] Allow VFS to be used for tests that generate PCH files.

I suppose we could do the overlay manually in all the tests that need it; I guess it depends on what the goal of the VFS support is. To my mind, it doesn't make sense that a file is placed in the RealFS in the first place if a VFS is used, but this is quite entrenched in the current design and difficult to change. In which case, I would argue that parsing a file with a preamble and a VFS should not give a cryptic error without an explicit overlay, but rather "just work". But I agree my current patch is a little crude in that it allows access to the entire RealFS via the VFS.

Sep 5 2017, 10:05 AM
cameron314 abandoned D20338: [PCH] Fixed overridden files always invalidating preamble even when unchanged.

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 D27810 and managed to get it through. I think my test case and supporting changes are still useful, however, so I've submitted that separately in D37474.

Sep 5 2017, 6:52 AM
cameron314 created D37474: [PCH] Allow VFS to be used for tests that generate PCH files.
Sep 5 2017, 6:52 AM

Sep 1 2017

cameron314 added a comment to D27810: FileManager: mark virtual file entries as valid entries.

The IsValid = true line coincidentally fixes a tangentially related bug -- see D20338 (in which I tried to introduce the same fix almost a year earlier, but nobody accepted the review). I guess I have to maintain the test for that case out of tree, though I could try to upstream it again (however, given that it's now passing with your change, I don't have much hope of it being accepted).

Sep 1 2017, 1:08 PM

Jan 3 2017

cameron314 added a comment to D27440: clang-format-vsix: fail when clang-format outputs to stderr.

Thanks, I'll check these out! Btw, I noticed that the clang-format tests are non-Windows due to path assumptions. Is this a lost cause, or just something no one's bothered to look into yet?

No one's bothered looking into it yet.

Jan 3 2017, 7:58 AM

Nov 15 2016

cameron314 committed rL286973: [clang-format] Fixed line merging of more than two lines.
[clang-format] Fixed line merging of more than two lines
Nov 15 2016, 7:17 AM
cameron314 closed D19063: clang-format: Fixed line merging of more than two lines by committing rL286973: [clang-format] Fixed line merging of more than two lines.
Nov 15 2016, 7:17 AM

Oct 6 2016

cameron314 added a comment to D19063: clang-format: Fixed line merging of more than two lines.

I'll commit this in a few days, when I have some time to reintegrate it to the trunk. Thanks!

Oct 6 2016, 7:38 AM

Aug 18 2016

cameron314 committed rL279145: Fixed more signed/unsigned mismatch warnings introduced in my change at r279076.
Fixed more signed/unsigned mismatch warnings introduced in my change at r279076
Aug 18 2016, 2:05 PM
cameron314 committed rL279114: Removed use of 'emplace' on std::map, since not all buildbot slaves support it.
Removed use of 'emplace' on std::map, since not all buildbot slaves support it
Aug 18 2016, 11:49 AM
cameron314 committed rL279092: [libclang] Added missing entry for newly introduced….
[libclang] Added missing entry for newly introduced…
Aug 18 2016, 10:26 AM
cameron314 committed rL279085: [libclang] Fixed signed/unsigned comparison warning introduced in my revision….
[libclang] Fixed signed/unsigned comparison warning introduced in my revision…
Aug 18 2016, 9:33 AM
cameron314 committed rL279076: [libclang] Add clang_getAllSkippedRanges function.
[libclang] Add clang_getAllSkippedRanges function
Aug 18 2016, 8:52 AM
cameron314 closed D20132: [libclang] Add clang_getAllSkippedRanges function by committing rL279076: [libclang] Add clang_getAllSkippedRanges function.
Aug 18 2016, 8:52 AM
cameron314 added inline comments to D20132: [libclang] Add clang_getAllSkippedRanges function.
Aug 18 2016, 6:51 AM

Aug 2 2016

cameron314 added a comment to D20338: [PCH] Fixed overridden files always invalidating preamble even when unchanged.

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 :-)

Aug 2 2016, 9:04 AM

Jul 25 2016

cameron314 added a comment to D21328: [lldb] Fixed incorrect endianness when evaluating certain expressions.

Hmm, I don't think this is easily testable at all. We happened to see it on our (out-of-tree) big-endian architecture when debugging remotely on Windows, but only for variables backed by registers. I was thus able to trace the problem back to this code, fix it, and verify it locally. But I can't reproduce this for other architectures because they likely don't follow the same code path (plus I don't have the hardware/simulator setup required). And trying to unit test this would be... quite a challenge, I feel, considering all the layers involved.

Jul 25 2016, 1:35 PM

Jul 20 2016

cameron314 added a comment to D21328: [lldb] Fixed incorrect endianness when evaluating certain expressions.

@spyffe, do you have a minute today?

Jul 20 2016, 8:43 AM

Jul 5 2016

cameron314 added a comment to D21328: [lldb] Fixed incorrect endianness when evaluating certain expressions.

Sorry to bug you again, but @spyffe, could you have a look when you have a sec?

Jul 5 2016, 3:32 PM

Jun 28 2016

cameron314 added a comment to D21328: [lldb] Fixed incorrect endianness when evaluating certain expressions.

@spyffe, do you have a minute to look this over?

Jun 28 2016, 10:37 AM
cameron314 added a comment to D20338: [PCH] Fixed overridden files always invalidating preamble even when unchanged.

Anyone have a few minutes to look at this?

Jun 28 2016, 8:36 AM

Jun 16 2016

cameron314 added a comment to D20338: [PCH] Fixed overridden files always invalidating preamble even when unchanged.

Ping? :-)

Jun 16 2016, 8:21 AM

Jun 14 2016

cameron314 retitled D21328: [lldb] Fixed incorrect endianness when evaluating certain expressions from to [lldb] Fixed incorrect endianness when evaluating certain expressions.
Jun 14 2016, 9:55 AM
cameron314 committed rL272682: [lldb] Fixed race conditions on private state thread exit.
[lldb] Fixed race conditions on private state thread exit
Jun 14 2016, 9:29 AM
cameron314 closed D21296: [lldb] Fixed race condition on private state thread exit, take 2 by committing rL272682: [lldb] Fixed race conditions on private state thread exit.
Jun 14 2016, 9:29 AM
cameron314 added a comment to D21296: [lldb] Fixed race condition on private state thread exit, take 2.

Thanks everyone :-)

Jun 14 2016, 9:13 AM

Jun 13 2016

cameron314 added a comment to D21296: [lldb] Fixed race condition on private state thread exit, take 2.

@clayborg: Thanks for having a look! I've added Jim Ingham as a reviewer. @jingham, I'd appreciate if you could take a few minutes to look this over.

Jun 13 2016, 2:29 PM
cameron314 added a reviewer for D21296: [lldb] Fixed race condition on private state thread exit, take 2: jingham.
Jun 13 2016, 2:01 PM
cameron314 retitled D21296: [lldb] Fixed race condition on private state thread exit, take 2 from to [lldb] Fixed race condition on private state thread exit, take 2.
Jun 13 2016, 9:24 AM

Jun 9 2016

cameron314 updated the diff for D20338: [PCH] Fixed overridden files always invalidating preamble even when unchanged.

Here's the final fix (it's the line in FileManager.cpp, plus a test).

Jun 9 2016, 3:06 PM
cameron314 added inline comments to D20338: [PCH] Fixed overridden files always invalidating preamble even when unchanged.
Jun 9 2016, 3:00 PM
cameron314 updated D20129: [libclang] Properly expose linkage spec cursors.
Jun 9 2016, 11:54 AM
cameron314 added a comment to D20338: [PCH] Fixed overridden files always invalidating preamble even when unchanged.

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?

Jun 9 2016, 11:44 AM
cameron314 added a comment to D20132: [libclang] Add clang_getAllSkippedRanges function.

Can someone have a look at this now that there's a test?

Jun 9 2016, 11:43 AM

Jun 3 2016

cameron314 updated the diff for D20338: [PCH] Fixed overridden files always invalidating preamble even when unchanged.

It took some modifications to the ASTUnit to support a virtual file system with a PCH parse/reparse (preliminary VFS support had already been added in rL249410 but it did not support initial parses using PCHs, nor reparses), but I was finally able to write a test that checks that the reparse actually uses the PCH with my fix, and rejects the PCH (rereading everything and failing the test) without it.

Jun 3 2016, 10:21 AM

Jun 1 2016

cameron314 updated subscribers of rL249410: [Tooling] Reuse FileManager in ASTUnit..
Jun 1 2016, 10:39 AM
cameron314 added inline comments to rL249410: [Tooling] Reuse FileManager in ASTUnit..
Jun 1 2016, 10:37 AM

May 30 2016

cameron314 committed rL271209: [LLDB] Make sure that indexing is done before clearing DIE info.
[LLDB] Make sure that indexing is done before clearing DIE info
May 30 2016, 8:39 AM
cameron314 closed D20738: Make sure that indexing is done before clearing DIE info. by committing rL271209: [LLDB] Make sure that indexing is done before clearing DIE info.
May 30 2016, 8:39 AM

May 26 2016

cameron314 added a comment to D20338: [PCH] Fixed overridden files always invalidating preamble even when unchanged.

Thanks @bruno, I'll have a look at using a VFS for the test.

May 26 2016, 7:23 AM

May 24 2016

cameron314 added a comment to D20338: [PCH] Fixed overridden files always invalidating preamble even when unchanged.

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.

May 24 2016, 8:26 AM

May 18 2016

cameron314 added a comment to D20243: [PCH] Disable inclusion of timestamps when generating pch files on windows..

I've seen rare cases where parses using the PCH files were yielding completely invalid data, almost as if they were corrupted. I wonder now if this could be the cause. (We're on Windows too.)

May 18 2016, 2:53 PM

May 17 2016

cameron314 retitled D20338: [PCH] Fixed overridden files always invalidating preamble even when unchanged from to [PCH] Fixed overridden files always invalidating preamble even when unchanged.
May 17 2016, 2:51 PM
cameron314 committed rL269769: [PCH] Fixed bug with preamble invalidation when overridden files change.
[PCH] Fixed bug with preamble invalidation when overridden files change
May 17 2016, 7:41 AM
cameron314 closed D20137: [PCH] Fixed bugs with preamble invalidation when files change (on Windows) by committing rL269769: [PCH] Fixed bug with preamble invalidation when overridden files change.
May 17 2016, 7:41 AM

May 16 2016

cameron314 updated the diff for D20137: [PCH] Fixed bugs with preamble invalidation when files change (on Windows).

This version of the patch takes into account potential changes between filenames and their unique IDs between parses.

May 16 2016, 11:01 AM
cameron314 added inline comments to D20137: [PCH] Fixed bugs with preamble invalidation when files change (on Windows).
May 16 2016, 10:02 AM

May 13 2016

cameron314 updated the diff for D20137: [PCH] Fixed bugs with preamble invalidation when files change (on Windows).

Removed workaround for case that can no longer happen.

May 13 2016, 2:25 PM
cameron314 added inline comments to D20137: [PCH] Fixed bugs with preamble invalidation when files change (on Windows).
May 13 2016, 2:19 PM
cameron314 updated the diff for D20137: [PCH] Fixed bugs with preamble invalidation when files change (on Windows).

Updated patch to include later fixes I had made after the initial change.

May 13 2016, 11:01 AM

May 12 2016

cameron314 committed rL269366: Added missing makefile from patch D19124 (should fix the corresponding commit….
Added missing makefile from patch D19124 (should fix the corresponding commit…
May 12 2016, 3:16 PM