Page MenuHomePhabricator

yvvan (Ivan Donchevskii)
User

Projects

User does not belong to any projects.

User Details

User Since
May 24 2017, 5:35 AM (81 w, 2 d)

Recent Activity

Yesterday

yvvan committed rCTE349132: [clang-tidy] Remove extra config.h includes.
[clang-tidy] Remove extra config.h includes
Thu, Dec 13, 11:48 PM
yvvan committed rL349132: [clang-tidy] Remove extra config.h includes.
[clang-tidy] Remove extra config.h includes
Thu, Dec 13, 11:48 PM
yvvan added inline comments to D55595: [clang-tidy] Share the forced linking code between clang-tidy tool and plugin.
Thu, Dec 13, 11:43 PM · Restricted Project
yvvan committed rCTE349131: [clang-tidy] Share the forced linking code between clang-tidy tool and plugin.
[clang-tidy] Share the forced linking code between clang-tidy tool and plugin
Thu, Dec 13, 11:32 PM
yvvan committed rL349131: [clang-tidy] Share the forced linking code between clang-tidy tool and plugin.
[clang-tidy] Share the forced linking code between clang-tidy tool and plugin
Thu, Dec 13, 11:32 PM
yvvan added inline comments to D55595: [clang-tidy] Share the forced linking code between clang-tidy tool and plugin.
Thu, Dec 13, 11:27 PM · Restricted Project
yvvan added inline comments to D55595: [clang-tidy] Share the forced linking code between clang-tidy tool and plugin.
Thu, Dec 13, 12:08 PM · Restricted Project
yvvan added inline comments to D55595: [clang-tidy] Share the forced linking code between clang-tidy tool and plugin.
Thu, Dec 13, 12:00 PM · Restricted Project
yvvan committed rL349038: [clang-tidy] Share the forced linking code between clang-tidy tool and plugin.
[clang-tidy] Share the forced linking code between clang-tidy tool and plugin
Thu, Dec 13, 6:41 AM
yvvan committed rCTE349038: [clang-tidy] Share the forced linking code between clang-tidy tool and plugin.
[clang-tidy] Share the forced linking code between clang-tidy tool and plugin
Thu, Dec 13, 6:41 AM
yvvan closed D55595: [clang-tidy] Share the forced linking code between clang-tidy tool and plugin.
Thu, Dec 13, 6:41 AM · Restricted Project
yvvan added a comment to D55595: [clang-tidy] Share the forced linking code between clang-tidy tool and plugin.

One thing: Could you please check the utility-scripts in clang-tidy that create new checks, if they need adjustments? Not sure if we have something in there that relies on the old structure.

Thu, Dec 13, 6:18 AM · Restricted Project

Wed, Dec 12

yvvan updated the diff for D55595: [clang-tidy] Share the forced linking code between clang-tidy tool and plugin.

Add standard prologue to the new header

Wed, Dec 12, 3:06 AM · Restricted Project
yvvan updated the diff for D55595: [clang-tidy] Share the forced linking code between clang-tidy tool and plugin.
Wed, Dec 12, 2:22 AM · Restricted Project
yvvan planned changes to D55595: [clang-tidy] Share the forced linking code between clang-tidy tool and plugin.
  1. Please always upload all patches with full context.
  2. There are two places where this pattern exists - this file, and tool/ClangTidyMain.cpp. It clearly leads to such issues, Can this be reworked to be just one file that is simply included in both places?
Wed, Dec 12, 1:54 AM · Restricted Project
yvvan created D55595: [clang-tidy] Share the forced linking code between clang-tidy tool and plugin.
Wed, Dec 12, 1:40 AM · Restricted Project

Mon, Dec 10

yvvan committed rC348764: [libclang] Revert removal of tidy plugin support from libclang introduced in….
[libclang] Revert removal of tidy plugin support from libclang introduced in…
Mon, Dec 10, 8:02 AM
yvvan committed rL348764: [libclang] Revert removal of tidy plugin support from libclang introduced in….
[libclang] Revert removal of tidy plugin support from libclang introduced in…
Mon, Dec 10, 8:02 AM
yvvan closed D55415: Revert removal of tidy plugin support from libclang.
Mon, Dec 10, 8:02 AM

Fri, Dec 7

yvvan added a comment to D55415: Revert removal of tidy plugin support from libclang.

I'd be interested in hearing how this is used. I added this feature as an experiment a while back but it simply didn't work as I envisioned it to. Some checks do work but the overall latency makes it unusable in an IDE setting. People repeatedly asked me to remove it because it slows down builds while not adding value.

Fri, Dec 7, 7:15 AM
yvvan added a comment to D55415: Revert removal of tidy plugin support from libclang.

We can also add an extra variable for it if you care about build time

Fri, Dec 7, 12:46 AM
yvvan updated the diff for D55415: Revert removal of tidy plugin support from libclang.

I generated the wrong diff. This is the proper one.

Fri, Dec 7, 12:17 AM
yvvan created D55415: Revert removal of tidy plugin support from libclang.
Fri, Dec 7, 12:16 AM

Tue, Dec 4

yvvan abandoned D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap.

The fact that so many call-sites decide what to do is pretty error-prone.
There was already at least one issue when this flag was not properly set by some of the call-sites.

Tue, Dec 4, 1:05 AM

Mon, Dec 3

yvvan added a comment to D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap.

Hm. What about another way around? - We have user include paths (-I) and report them to the filesystem. This means that we have specific paths under which nothing can be mmaped and everything else can be. In particular cases we can also report -isystem there. This is quite the same logic as current isVolatile parameter but is set only once for each path.

Mon, Dec 3, 7:34 AM
yvvan added a comment to D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap.

Ok, no global option.
Why not placing your VolatileFSProvider in clang so that libclang could you it too?

Would be happy to, will need to figure out what to do with PCH and PCM files first. However if we do this on clang level, I believe we should remove the isVolatile flag from the VFS interfaces in the first place.
It would be nice to not loose out on the opportunity to avoid fully loading the PCH files, but that obviously requires passing some flags into the VFS implementation or various hacks (matching on filenames/extensions?) to find out which files are PCHs.
I actually don't know which approach to choose: on one hand, I'd really want to get rid of the isVolatile flag, on the other hand I'd really want to avoid loading large binary files into memory and that requires passing the flags.

Mon, Dec 3, 4:09 AM
yvvan planned changes to D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap.

Ok, no global option.
Why not placing your VolatileFSProvider in clang so that libclang could you it too?

Mon, Dec 3, 3:37 AM
yvvan added inline comments to D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap.
Mon, Dec 3, 1:34 AM
yvvan updated the diff for D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap.

Instead of forcing some default value let's give the client an ability to force disabled mmap if one desires.

Mon, Dec 3, 1:19 AM

Fri, Nov 30

yvvan added a comment to D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap.

@ilya-biryukov
What do you think about the global settable bool in MemoryBuffer in place of the ifdef from https://reviews.llvm.org/D35200 ? In this case the client on Windows can set it and you're safe that any MemoryBuffer call never mmaps.

Fri, Nov 30, 12:54 PM
yvvan added a comment to D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap.

No success with unmapping. Buffers are hold by PCMCache in Preprocessor (and the same one in ASTReader) and if I clean them then SourceManger crashed sooner or later on some call that gets data from external files.
So far I have no steps to reproduce the lock on user file so I don't currently know if I try something else.

Fri, Nov 30, 1:31 AM

Thu, Nov 29

yvvan added a comment to D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap.

I'm currently trying out another suggestion - which unmaps memory buffer caches after ASTUnit's Parse or Reparse and is limited to Windows only.
And my aim currently is not only clangd but any other client as well.

Thu, Nov 29, 7:10 AM
yvvan added a comment to D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap.

According to https://msdn.microsoft.com/en-us/2e9c3174-af48-4fa3-9f6a-fb62b23ed994 - "Unmapping a mapped view of a file invalidates the range occupied by the view in the address space of the process and makes the range available for other allocations".
Also as far as i understand from https://msdn.microsoft.com/en-us/library/ms810613.aspx mapped files can only be edited in other apps as mapped files opened with the same name (OpenFileMapping).

Thu, Nov 29, 1:26 AM

Wed, Nov 28

yvvan added a comment to D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap.

To clarify a little bit - i did not write my own simple app but used libclang in qt creator to reproduce the lock and track the issue. This time I hope to go further, collect flags used by clang and make a simple example.

Wed, Nov 28, 9:40 AM
yvvan added a comment to D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap.

I was able to lock files last time with mmap. The Windows API calls are CreateFileMappingW (it's in the Path.inc, line 794) and MapViewOfFile (further down). As far as I remember the second call actually creates the lock and is freed only by UnmapViewOfFile in destructor

Wed, Nov 28, 7:53 AM
yvvan added a comment to D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap.

According to https://stackoverflow.com/a/7414798 (if it's true) we can't prevent locks.

Wed, Nov 28, 7:25 AM
yvvan added a comment to D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap.

"could we figure out the exact cause of the issue?"

Wed, Nov 28, 6:54 AM
yvvan added a comment to D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap.

Hm, probably FileManager can be that adapter since it's in clang

Wed, Nov 28, 5:42 AM
yvvan added a comment to D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap.

I've realized that this patch covers too much stuff outside of clang and I have no idea how bad is to not memory map it.

Wed, Nov 28, 5:37 AM
yvvan created D54996: [libclang] Fix clang_Cursor_isAnonymous.
Wed, Nov 28, 3:38 AM
yvvan added a comment to D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap.

I have the reported evidence but didn't yet have time to investigate myself.
This is probably caused by our upgrade to Clang-7 which shows that we can't rely on the aimed fixes like the one I mention (D47460).

Wed, Nov 28, 1:38 AM
yvvan updated the diff for D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap.

VFS is moved to llvm in 8.0, update the diff

Wed, Nov 28, 12:28 AM
yvvan created D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap.
Wed, Nov 28, 12:28 AM

Tue, Nov 27

yvvan committed rL347654: [libclang] Fix clang_Cursor_getNumArguments and clang_Cursor_getArgument for….
[libclang] Fix clang_Cursor_getNumArguments and clang_Cursor_getArgument for…
Tue, Nov 27, 4:05 AM
yvvan committed rC347654: [libclang] Fix clang_Cursor_getNumArguments and clang_Cursor_getArgument for….
[libclang] Fix clang_Cursor_getNumArguments and clang_Cursor_getArgument for…
Tue, Nov 27, 4:05 AM
yvvan closed D54934: [libclang] Fix clang_Cursor_getNumArguments and clang_Cursor_getArgument for CXXConstructExpr.
Tue, Nov 27, 4:05 AM
yvvan created D54934: [libclang] Fix clang_Cursor_getNumArguments and clang_Cursor_getArgument for CXXConstructExpr.
Tue, Nov 27, 1:14 AM

Oct 31 2018

yvvan added a comment to D53866: [Preamble] Stop generating preamble for circular #includes.

@ilya-biryukov
As far as I understand the problem the same thing happens when you are in the header a.h which includes b.h and b.h includes a.h at the same time. So you get this recursion indirectly and very often because that's why include guards are there.

Oct 31 2018, 11:06 AM

Oct 19 2018

yvvan added a comment to D53072: [clang-format] Introduce the flag which allows not to shrink lines.

Do you know the better way to accomplish my aim than adding an option to libFormat? For example making a dependent library which serves a little different purpose than libFormat itself or something simpler?

Oct 19 2018, 12:36 AM

Oct 10 2018

yvvan created D53072: [clang-format] Introduce the flag which allows not to shrink lines.
Oct 10 2018, 4:35 AM

Sep 21 2018

yvvan committed rL342721: [CodeComplete] Generate completion fix-its for C code as well.
[CodeComplete] Generate completion fix-its for C code as well
Sep 21 2018, 4:25 AM
yvvan committed rC342721: [CodeComplete] Generate completion fix-its for C code as well.
[CodeComplete] Generate completion fix-its for C code as well
Sep 21 2018, 4:25 AM
yvvan closed D52261: [Sema] Generate completion fix-its for C code as well.
Sep 21 2018, 4:25 AM
yvvan updated the diff for D52261: [Sema] Generate completion fix-its for C code as well.

Provide CorrectedBase = Base for C code

Sep 21 2018, 12:22 AM
yvvan added a comment to D52261: [Sema] Generate completion fix-its for C code as well.

You're right actually. The fix is much simpler than I expected :)

Sep 21 2018, 12:16 AM

Sep 19 2018

yvvan added a comment to D52261: [Sema] Generate completion fix-its for C code as well.

I tried that first but did not I find a way just to copy an expression (we basically need the same expr for such case). Do you know how to properly generate a copy of expression or some other way to get the same expression?

Sep 19 2018, 11:11 PM
yvvan updated the diff for D52261: [Sema] Generate completion fix-its for C code as well.

Test is added

Sep 19 2018, 5:59 AM
yvvan created D52261: [Sema] Generate completion fix-its for C code as well.
Sep 19 2018, 5:31 AM

Sep 7 2018

yvvan committed rC341656: [libclang] Return the proper pointee type for 'auto' deduced to pointer.
[libclang] Return the proper pointee type for 'auto' deduced to pointer
Sep 7 2018, 6:25 AM
yvvan committed rL341656: [libclang] Return the proper pointee type for 'auto' deduced to pointer.
[libclang] Return the proper pointee type for 'auto' deduced to pointer
Sep 7 2018, 6:25 AM
yvvan closed D51281: [libclang] Return the proper pointee type for 'auto' deduced to pointer.
Sep 7 2018, 6:25 AM

Sep 5 2018

yvvan updated the diff for D51281: [libclang] Return the proper pointee type for 'auto' deduced to pointer.

Comments addressed

Sep 5 2018, 11:44 PM

Aug 28 2018

yvvan added a comment to D51281: [libclang] Return the proper pointee type for 'auto' deduced to pointer.

You are completely right! Thanks!
I did not think that -test-print-type also checks for the pointee.

Aug 28 2018, 11:15 PM

Aug 26 2018

yvvan created D51281: [libclang] Return the proper pointee type for 'auto' deduced to pointer.
Aug 26 2018, 11:42 PM

Aug 23 2018

yvvan committed rC340521: [libclang] Fix cursors for arguments of Subscript and Call operators.
[libclang] Fix cursors for arguments of Subscript and Call operators
Aug 23 2018, 2:49 AM
yvvan committed rL340521: [libclang] Fix cursors for arguments of Subscript and Call operators.
[libclang] Fix cursors for arguments of Subscript and Call operators
Aug 23 2018, 2:49 AM
yvvan closed D40481: [libclang] Fix cursors for arguments of Subscript and Call operators.
Aug 23 2018, 2:48 AM
yvvan accepted D40481: [libclang] Fix cursors for arguments of Subscript and Call operators.

Let's proceed with this one. I really see that it's going to be useful.

Aug 23 2018, 1:11 AM

Jul 31 2018

yvvan added a comment to D49794: [libclang] Allow skipping warnings from all included files.

And we already saw, that -isystem is not the best choice for that.

Are you referring to the file-locking on Windows?
Any other reasons why the -isystem trick might be non-ideal?

File locking is the first one. Another one comes with "plugin mode" of tidy.

Jul 31 2018, 6:50 AM
yvvan added a child revision for D41407: Add index-while-building support to Clang - Part 3: D49009: Add index-while-building support to Clang - non-Apple platforms.
Jul 31 2018, 6:23 AM
yvvan added a parent revision for D49009: Add index-while-building support to Clang - non-Apple platforms: D41407: Add index-while-building support to Clang - Part 3.
Jul 31 2018, 6:23 AM
yvvan added a comment to D49794: [libclang] Allow skipping warnings from all included files.

Anyways, if c++ part does not seem relevant I'm fine if we only have libclang part from D48116.

Jul 31 2018, 6:20 AM
yvvan accepted D49635: [libclang 8/8] Add support for the flag_enum attribute.
Jul 31 2018, 6:13 AM
yvvan accepted D49634: [libclang 7/8] Add support for getting property setter and getter names.
Jul 31 2018, 6:13 AM
yvvan accepted D49631: [libclang 6/8] Add support for reading implicit attributes.
Jul 31 2018, 6:12 AM
yvvan accepted D49127: [libclang 5/8] Add support for ObjC attributes without args.
Jul 31 2018, 6:12 AM
yvvan accepted D49082: [libclang 4/8] Add the clang_Type_getNullability() API.
Jul 31 2018, 6:12 AM
yvvan accepted D49081: [libclang 3/8] Add support for AttributedType.
Jul 31 2018, 6:12 AM
yvvan added a comment to D49794: [libclang] Allow skipping warnings from all included files.

And we already saw, that -isystem is not the best choice for that. And by the way - clang-tidy has this filtering in consumer which does not exist in it's plugin-mode so it spits all system diagnostics all the time...

Jul 31 2018, 6:10 AM
yvvan accepted D49066: [libclang 2/8] Add support for ObjCTypeParam.
Jul 31 2018, 6:02 AM
yvvan accepted D49063: [libclang 1/8] Add support for ObjCObjectType.
Jul 31 2018, 6:02 AM
yvvan added a comment to D49794: [libclang] Allow skipping warnings from all included files.

I already mentioned in the review inherited by this one that this way covers also loaded plugins with different consumers. So let's assume that driver loads clang-tidy and some other plugins and runs over each file in the project.
It makes sense not to include clang-tidy diagnostics coming from headers, at least for third-party headers (you can still run clang over specific headers themselves to get diagnostics for them).

Jul 31 2018, 4:48 AM
yvvan updated the diff for D49794: [libclang] Allow skipping warnings from all included files.

Restore missing tests

Jul 31 2018, 4:05 AM

Jul 25 2018

yvvan added a comment to D49063: [libclang 1/8] Add support for ObjCObjectType.

I'm mostly fine with your set of patches. Let me double check and we can get it in.

Jul 25 2018, 11:58 PM
yvvan created D49794: [libclang] Allow skipping warnings from all included files.
Jul 25 2018, 6:08 AM

Jul 24 2018

yvvan added a comment to D49010: YAML output for index-while-building.

@ilya-biryukov I would name this revision "for test purpose". It works at some extent with the current clangd version and can be used by someone else. But since we had a discussion about YAML being a temporary solution I never planned to commit this revision.
So yes, I totally agree that using a binary format would be ideal for the "index while build' use case.

Jul 24 2018, 11:15 PM

Jul 13 2018

yvvan updated the diff for D49010: YAML output for index-while-building.
Jul 13 2018, 1:31 AM

Jul 11 2018

yvvan added inline comments to D49063: [libclang 1/8] Add support for ObjCObjectType.
Jul 11 2018, 11:16 PM

Jul 6 2018

yvvan updated the summary of D49010: YAML output for index-while-building.
Jul 6 2018, 1:45 AM
yvvan created D49010: YAML output for index-while-building.
Jul 6 2018, 1:44 AM
yvvan updated subscribers of D49009: Add index-while-building support to Clang - non-Apple platforms.
Jul 6 2018, 1:42 AM
yvvan created D49009: Add index-while-building support to Clang - non-Apple platforms.
Jul 6 2018, 1:41 AM

Jul 4 2018

yvvan added a comment to D48314: [Frontend] Cache preamble-related data.

@ilya-biryukov Sorry. I didn't have time to post comments here.
The usecase that we have is a supportive translation unit for code completion. Probably you use something similar in clangd not to wait for the TU to be reparsed after a change?
The gain from this change is both performance and memory but I don't have measurements under my hand right now. And of course they are only valid for this usecase.

Jul 4 2018, 4:19 AM

Jul 3 2018

yvvan added a comment to D41407: Add index-while-building support to Clang - Part 3.

So I have that working on windows with clang-cl (probably broke something else during the porting, need to recheck.
One more commit I have in my working tree is YAML export to be able to use it for clangd. The quality of code is currently not so good so I probably need some extra time to make it ok-ish. But i hope to have it posted to reviews this week (unless I have something urgent)

Jul 3 2018, 6:07 AM

Jul 1 2018

yvvan added a comment to D41407: Add index-while-building support to Clang - Part 3.

@nathawes I've already started changing so I can hopefully share my code with you when I fix all undefined symbols :)

Jul 1 2018, 11:06 PM

Jun 29 2018

yvvan added a comment to D41407: Add index-while-building support to Clang - Part 3.

Will it work if I replace every block-related code here with llvm::function_ref<>?

Jun 29 2018, 3:36 AM
yvvan added a comment to D41407: Add index-while-building support to Clang - Part 3.

I can't really test this one because INDEXSTORE_HAS_BLOCKS is apple-specific and it's required to rewrite all block-related stuff to use that code

Jun 29 2018, 2:44 AM

Jun 27 2018

yvvan added a comment to D48116: [libclang] Allow skipping warnings from all included files.

But this one misses a way to set this flag for everything except libclang. We can probably change that (Nikolai is in vacation till the end of summer so it's probably my part now) and add some tests outside of Index for it (probably Frontend)

Jun 27 2018, 5:22 AM
yvvan added a comment to D48116: [libclang] Allow skipping warnings from all included files.

The idea was to ignore everything including notes and remarks so that only errors from system headers are still collected.

Jun 27 2018, 4:28 AM

Jun 21 2018

yvvan committed rC335220: Fix line endings in recently updated test file.
Fix line endings in recently updated test file
Jun 21 2018, 5:44 AM