Page MenuHomePhabricator

kousikk (Kousik Kumar)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 10 2019, 10:42 AM (28 w, 2 h)

Recent Activity

Nov 22 2019

kousikk committed rG26fa9e31f58a: Add support to find out resource dir and add it as compilation args (authored by kousikk).
Add support to find out resource dir and add it as compilation args
Nov 22 2019, 7:44 AM
kousikk closed D69122: Add support to find out resource dir and add it as compilation args.
Nov 22 2019, 7:44 AM · Restricted Project

Nov 21 2019

kousikk added inline comments to D70351: [clang][WIP][clang-scan-deps] Add an experimental C API..
Nov 21 2019, 6:45 PM · Restricted Project, Restricted Project
kousikk reopened D69122: Add support to find out resource dir and add it as compilation args.
Nov 21 2019, 1:26 PM · Restricted Project
kousikk closed D69122: Add support to find out resource dir and add it as compilation args.
Nov 21 2019, 1:17 PM · Restricted Project
kousikk added inline comments to D69122: Add support to find out resource dir and add it as compilation args.
Nov 21 2019, 12:31 PM · Restricted Project
kousikk updated the diff for D69122: Add support to find out resource dir and add it as compilation args.

Address review comments

Nov 21 2019, 12:31 PM · Restricted Project

Nov 12 2019

kousikk added a comment to D69122: Add support to find out resource dir and add it as compilation args.

Ping

Nov 12 2019, 4:26 PM · Restricted Project

Nov 2 2019

kousikk updated the diff for D69122: Add support to find out resource dir and add it as compilation args.

Forgot to run clang-format

Nov 2 2019, 10:13 PM · Restricted Project
kousikk updated the diff for D69122: Add support to find out resource dir and add it as compilation args.

@Bigcheese - thanks, all the three points make sense!

Nov 2 2019, 10:10 PM · Restricted Project

Oct 28 2019

kousikk added a comment to D69122: Add support to find out resource dir and add it as compilation args.

I discussed with @klimek about this. The conclusion we arrived at was:

Oct 28 2019, 6:33 AM · Restricted Project

Oct 25 2019

kousikk added a comment to D69420: [clang][clang-scan-deps] Add support for extracting full module dependencies..

A few nit comments

Oct 25 2019, 11:11 AM · Restricted Project

Oct 21 2019

kousikk committed rGfb042b094fda: Refactor DependencyScanningTool to its own file (authored by kousikk).
Refactor DependencyScanningTool to its own file
Oct 21 2019, 10:05 PM
kousikk committed rL375483: Refactor DependencyScanningTool to its own file.
Refactor DependencyScanningTool to its own file
Oct 21 2019, 10:04 PM
kousikk closed D69186: Refactor DependencyScanningTool to its own file.
Oct 21 2019, 10:04 PM · Restricted Project
kousikk updated the diff for D69186: Refactor DependencyScanningTool to its own file.

Run clang format

Oct 21 2019, 9:55 PM · Restricted Project
kousikk abandoned D69090: [Try 2] Include sanitize blacklist and other extra deps as part of scan-deps output.
Oct 21 2019, 1:08 PM · Restricted Project
kousikk added a comment to D69090: [Try 2] Include sanitize blacklist and other extra deps as part of scan-deps output.

I was able to test this on a Windows machine. The ExtraDeps are already being added to the dependency output (this code does that - https://github.com/llvm/llvm-project/blob/master/clang/lib/Frontend/DependencyFile.cpp#L188). My suspicion for why I wasn't seeing them previously was that these ExtraDeps are added as relative paths and my consumer code (which integrates with scan-deps) was ignoring relative paths. I can handle that in the consumer code - I'll leave the behaviour of the tool as it is.

Oct 21 2019, 12:59 PM · Restricted Project

Oct 18 2019

kousikk updated the diff for D69186: Refactor DependencyScanningTool to its own file.

Lint fixes

Oct 18 2019, 11:07 AM · Restricted Project
kousikk created D69186: Refactor DependencyScanningTool to its own file.
Oct 18 2019, 11:07 AM · Restricted Project

Oct 17 2019

kousikk added a comment to D69090: [Try 2] Include sanitize blacklist and other extra deps as part of scan-deps output.

Thanks for the comment @jkorous.

Oct 17 2019, 7:07 PM · Restricted Project
kousikk added a comment to D69122: Add support to find out resource dir and add it as compilation args.

I haven't looked into this in detail but it feels kind of wasteful to start a process just to get this value.

Right, that's why I cache this information. So multiple compile commands sharing the same compiler process will trigger at 1 subprocess and then subsequently use the cached information.

Oct 17 2019, 5:27 PM · Restricted Project
kousikk updated the summary of D69122: Add support to find out resource dir and add it as compilation args.
Oct 17 2019, 11:02 AM · Restricted Project
kousikk updated the summary of D69122: Add support to find out resource dir and add it as compilation args.
Oct 17 2019, 11:02 AM · Restricted Project
kousikk created D69122: Add support to find out resource dir and add it as compilation args.
Oct 17 2019, 11:02 AM · Restricted Project
kousikk updated the diff for D69090: [Try 2] Include sanitize blacklist and other extra deps as part of scan-deps output.

Fix tests now that output is in ascending order

Oct 17 2019, 1:44 AM · Restricted Project
kousikk updated the summary of D69090: [Try 2] Include sanitize blacklist and other extra deps as part of scan-deps output.
Oct 17 2019, 12:30 AM · Restricted Project
kousikk created D69090: [Try 2] Include sanitize blacklist and other extra deps as part of scan-deps output.
Oct 17 2019, 12:30 AM · Restricted Project

Oct 16 2019

kousikk committed rG9e7e36d4c260: Revert "Include sanitize blacklist and other extra deps as part of scan-deps… (authored by kousikk).
Revert "Include sanitize blacklist and other extra deps as part of scan-deps…
Oct 16 2019, 9:52 PM
kousikk added a reverting change for rG962ca076e51c: Include sanitize blacklist and other extra deps as part of scan-deps output: rG9e7e36d4c260: Revert "Include sanitize blacklist and other extra deps as part of scan-deps….
Oct 16 2019, 9:52 PM
kousikk committed rL375079: Revert "Include sanitize blacklist and other extra deps as part of scan-deps….
Revert "Include sanitize blacklist and other extra deps as part of scan-deps…
Oct 16 2019, 9:52 PM
kousikk closed D69079: Revert "Include sanitize blacklist and other extra deps as part of scan-deps output" This test is failing on Windows bots, revert for now (will check the right fix and retry the patch)..
Oct 16 2019, 9:52 PM · Restricted Project
kousikk created D69079: Revert "Include sanitize blacklist and other extra deps as part of scan-deps output" This test is failing on Windows bots, revert for now (will check the right fix and retry the patch)..
Oct 16 2019, 8:37 PM · Restricted Project
kousikk added a comment to D69079: Revert "Include sanitize blacklist and other extra deps as part of scan-deps output" This test is failing on Windows bots, revert for now (will check the right fix and retry the patch)..

Failure - http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/11583

Oct 16 2019, 8:37 PM · Restricted Project
kousikk added a reverting change for rG962ca076e51c: Include sanitize blacklist and other extra deps as part of scan-deps output: D69079: Revert "Include sanitize blacklist and other extra deps as part of scan-deps output" This test is failing on Windows bots, revert for now (will check the right fix and retry the patch)..
Oct 16 2019, 8:37 PM
kousikk committed rG962ca076e51c: Include sanitize blacklist and other extra deps as part of scan-deps output (authored by kousikk).
Include sanitize blacklist and other extra deps as part of scan-deps output
Oct 16 2019, 7:14 PM
kousikk committed rL375074: Include sanitize blacklist and other extra deps as part of scan-deps output.
Include sanitize blacklist and other extra deps as part of scan-deps output
Oct 16 2019, 7:14 PM
kousikk closed D69017: Include sanitize blacklist and other extra deps as part of scan-deps output.
Oct 16 2019, 7:14 PM · Restricted Project

Oct 15 2019

kousikk updated the diff for D69017: Include sanitize blacklist and other extra deps as part of scan-deps output.

Lint fixes

Oct 15 2019, 8:52 PM · Restricted Project
kousikk created D69017: Include sanitize blacklist and other extra deps as part of scan-deps output.
Oct 15 2019, 8:52 PM · Restricted Project

Oct 10 2019

kousikk committed rG4abac5330277: In openFileForRead don't cache erroneous entries if the error relates to them… (authored by kousikk).
In openFileForRead don't cache erroneous entries if the error relates to them…
Oct 10 2019, 8:31 AM
kousikk committed rL374366: In openFileForRead don't cache erroneous entries if the error relates to them….
In openFileForRead don't cache erroneous entries if the error relates to them…
Oct 10 2019, 8:31 AM
kousikk closed D68193: In openFileForRead don't cache erroneous entries if the error relates to them being directories. Add tests..
Oct 10 2019, 8:31 AM · Restricted Project
kousikk committed rL374360: Request commit access for kousikk.
Request commit access for kousikk
Oct 10 2019, 7:35 AM

Oct 9 2019

kousikk updated the diff for D68193: In openFileForRead don't cache erroneous entries if the error relates to them being directories. Add tests..

Remove getError() function that's now unused

Oct 9 2019, 6:32 PM · Restricted Project
kousikk updated the diff for D68193: In openFileForRead don't cache erroneous entries if the error relates to them being directories. Add tests..
  • Revert changes to createFileEntry since that may result in a race condition if stat is called before file open
Oct 9 2019, 5:54 PM · Restricted Project
kousikk added a comment to D68193: In openFileForRead don't cache erroneous entries if the error relates to them being directories. Add tests..

@arphaman I don't mind changing this if there are race conditions as you say, but isn't the assumption of the tool that the filesystem remains unchanged for a single run of the tool? If so, should we actually throw error conditions instead of crashing in those cases?

Oct 9 2019, 7:01 AM · Restricted Project

Oct 4 2019

kousikk added a comment to D68193: In openFileForRead don't cache erroneous entries if the error relates to them being directories. Add tests..

@dexonsmith when you get a chance, can you take another look at this? Thanks!

Oct 4 2019, 7:09 AM · Restricted Project
kousikk accepted D68436: [clang-scan-deps] Improve string/character literal skipping.
Oct 4 2019, 6:06 AM · Restricted Project

Oct 3 2019

kousikk added inline comments to D68436: [clang-scan-deps] Improve string/character literal skipping.
Oct 3 2019, 7:40 PM · Restricted Project

Oct 2 2019

kousikk updated the diff for D68193: In openFileForRead don't cache erroneous entries if the error relates to them being directories. Add tests..

Update diff to include all commits

Oct 2 2019, 7:16 PM · Restricted Project
kousikk added a comment to D68193: In openFileForRead don't cache erroneous entries if the error relates to them being directories. Add tests..

I don't quite understand the specific issue you're hitting, as it sounds that the logic right now should handle it:

It seems that when the CachingFileSystem is first given a file to open that is actually a directory, it incorrectly
caches that path to be errenous and throws an error when subsequently a directory open >call is made for the same
path.

In this case, right now (without this patch) createFileEntry will return std::errc::is_a_directory. DependencyScanningWorkerFilesystem::openFileForRead will then invalidate the entry in the global cache:

Oct 2 2019, 6:55 PM · Restricted Project

Oct 1 2019

kousikk updated the diff for D68193: In openFileForRead don't cache erroneous entries if the error relates to them being directories. Add tests..

Add missed const

Oct 1 2019, 12:07 PM · Restricted Project
kousikk added a comment to D68193: In openFileForRead don't cache erroneous entries if the error relates to them being directories. Add tests..

Sorry for bouncing you around, but I just had a look at the other user of createFileEntry and I think the right thing to do is to somehow share the code between DependencyScanningWorkerFilesystem::status and this. I suggest splitting a function out of status (called getOrCreateFileSystemEntry?) that returns a CachedFileSystemEntry (or an error).

The fix I just asked you to make (to only call stat once) could be done on getOrCreateFileSystemEntry as a follow-up, using std::unique_ptr<llvm::vfs::File> and changing getFileEntry to take that instead of a filename.

Oct 1 2019, 11:58 AM · Restricted Project
kousikk updated the diff for D68193: In openFileForRead don't cache erroneous entries if the error relates to them being directories. Add tests..

Address code-reorg comment

Oct 1 2019, 11:50 AM · Restricted Project

Sep 30 2019

kousikk added inline comments to D68193: In openFileForRead don't cache erroneous entries if the error relates to them being directories. Add tests..
Sep 30 2019, 12:19 PM · Restricted Project
kousikk updated the diff for D68193: In openFileForRead don't cache erroneous entries if the error relates to them being directories. Add tests..

Avoid the extra stat() call

Sep 30 2019, 12:17 PM · Restricted Project

Sep 29 2019

kousikk created D68193: In openFileForRead don't cache erroneous entries if the error relates to them being directories. Add tests..
Sep 29 2019, 7:54 AM · Restricted Project

Sep 20 2019

kousikk added a comment to D67635: Fix for stringized function-macro args continued across lines.

I think you should obtain commit access for your future patches/commits. You can follow the instructions here: https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access

Sep 20 2019, 5:58 AM · Restricted Project, Restricted Project

Sep 19 2019

kousikk added a comment to D67635: Fix for stringized function-macro args continued across lines.

Thanks! I will need you to merge this one too!

Sep 19 2019, 1:19 PM · Restricted Project, Restricted Project

Sep 18 2019

kousikk added a comment to D67635: Fix for stringized function-macro args continued across lines.

Ping on the review for this :)

Sep 18 2019, 5:17 PM · Restricted Project, Restricted Project

Sep 16 2019

kousikk updated the summary of D67635: Fix for stringized function-macro args continued across lines.
Sep 16 2019, 4:42 PM · Restricted Project, Restricted Project
kousikk updated the summary of D67635: Fix for stringized function-macro args continued across lines.
Sep 16 2019, 4:42 PM · Restricted Project, Restricted Project
kousikk created D67635: Fix for stringized function-macro args continued across lines.
Sep 16 2019, 2:26 PM · Restricted Project, Restricted Project

Sep 13 2019

kousikk added a comment to D67091: Fix for headers having the same name as a directory.

I don't have commit access - do you mind submitting this for me? Thanks!

Sep 13 2019, 12:55 PM · Restricted Project, Restricted Project

Sep 12 2019

kousikk added a comment to D67091: Fix for headers having the same name as a directory.

I was unable to reproduce the failures when I moved the check to createFile. What kind of failures did you see? Did you move it right to the start of the function? I would recommend rebasing the patch and retrying.

I saw an assertion error - but I realize now that it was because it was not at the top of the function. I had put it after Entry->getContents() which was throwing an assertion. I now realize that for getContents() to work, the entry needs to be a file. Thanks for correcting me!

Sep 12 2019, 12:42 PM · Restricted Project, Restricted Project
kousikk updated the diff for D67091: Fix for headers having the same name as a directory.
  • Add validation inside the createFile() function
Sep 12 2019, 12:42 PM · Restricted Project, Restricted Project
kousikk updated the diff for D67091: Fix for headers having the same name as a directory.
  • Add validation inside the createFile() function
Sep 12 2019, 12:41 PM · Restricted Project, Restricted Project
kousikk added a comment to D67091: Fix for headers having the same name as a directory.

Sorry about the delay on this - I was OOO (back now).

Sep 12 2019, 7:45 AM · Restricted Project, Restricted Project
kousikk updated the diff for D67091: Fix for headers having the same name as a directory.
  • Add validation inside the createFile() function
Sep 12 2019, 7:43 AM · Restricted Project, Restricted Project
kousikk updated the diff for D67091: Fix for headers having the same name as a directory.
  • Add tests and remove the fix inside createFile since it fails in other cases
Sep 12 2019, 7:42 AM · Restricted Project, Restricted Project
kousikk updated the diff for D67091: Fix for headers having the same name as a directory.
  • Add validation inside the createFile() function
Sep 12 2019, 5:17 AM · Restricted Project, Restricted Project
kousikk updated the diff for D67091: Fix for headers having the same name as a directory.
  • Add validation inside the createFile() function
Sep 12 2019, 5:17 AM · Restricted Project, Restricted Project

Sep 3 2019

kousikk added a comment to D67091: Fix for headers having the same name as a directory.

Sure I'll add a test case!

Sep 3 2019, 7:45 PM · Restricted Project, Restricted Project
kousikk created D67091: Fix for headers having the same name as a directory.
Sep 3 2019, 3:50 AM · Restricted Project, Restricted Project

Jul 10 2019

Herald updated subscribers of D53354: [WIP][NOT FOR COMMIT][PROTOTYPE] clang-scan-deps: dependency scanning tool rough prototype.
Jul 10 2019, 1:59 PM · Restricted Project