Page MenuHomePhabricator

akyrtzi (Argyrios Kyrtzidis)
User

Projects

User does not belong to any projects.

User Details

User Since
Feb 11 2014, 3:42 PM (257 w, 1 d)

Recent Activity

Dec 6 2018

akyrtzi added inline comments to D54862: [OpenCL] Add generic AS to 'this' pointer.
Dec 6 2018, 11:50 AM

Dec 5 2018

akyrtzi added inline comments to D54862: [OpenCL] Add generic AS to 'this' pointer.
Dec 5 2018, 8:17 AM

Nov 27 2018

akyrtzi added inline comments to D54826: [Support/FileSystem] Add sub-second precision for atime/mtime of sys::fs::file_status on unix platforms.
Nov 27 2018, 2:37 PM

Nov 25 2018

akyrtzi added a comment to D54826: [Support/FileSystem] Add sub-second precision for atime/mtime of sys::fs::file_status on unix platforms.

Thanks for reviewing Kristina & Aaron, much appreciated!

Nov 25 2018, 5:05 PM
akyrtzi updated the diff for D54826: [Support/FileSystem] Add sub-second precision for atime/mtime of sys::fs::file_status on unix platforms.
  • Change #cmakedefine01 to #cmakedefine for consistency.
  • Add comments to getLastAccessedTime()/ getLastModificationTime() APIs to make sure people are aware that time resolution can differ among file systems.
  • Improve commit summary.
Nov 25 2018, 2:02 PM
akyrtzi added inline comments to D54826: [Support/FileSystem] Add sub-second precision for atime/mtime of sys::fs::file_status on unix platforms.
Nov 25 2018, 1:52 PM
akyrtzi added inline comments to D54826: [Support/FileSystem] Add sub-second precision for atime/mtime of sys::fs::file_status on unix platforms.
Nov 25 2018, 1:21 PM
akyrtzi added a comment to D54826: [Support/FileSystem] Add sub-second precision for atime/mtime of sys::fs::file_status on unix platforms.

I think that should be documented on the public API part with a mention that the resolution is expected to differ depending on the file system and OS. WDYT?

That is a good idea, the way I consider file_status is that it provides a cross-platform way to get at the file properties. It provides times in nanoseconds but leaves the precision as implementation detail of the underlying file system, which makes sense, people should not expect anything more than that.
I'll add some comments.

Nov 25 2018, 1:10 PM
akyrtzi added a comment to D54826: [Support/FileSystem] Add sub-second precision for atime/mtime of sys::fs::file_status on unix platforms.

Is it going to be an issue that the Windows side of things has a more wild idea of file timestamp resolution? NTFS has a theoretical max precision of 100ns intervals, though according to MSDN, the access time on NTFS has a resolution of 1 hour, which is better than FAT file systems, where the resolution is 1 day. It seems odd to rely on ns resolution for access time, as that seems like it's going to be highly platform and filesystem dependent.

I can't speak for the Windows side of things but what you are pointing out is a general question about applications taking into account that file properties may differ across platforms. This is orthogonal to file_status itself reporting file properties accurately.
If current file_status has 1 hour resolution on NTFS for access time, this patch is not going to make it any better or worse, nor affect behavior of applications that are querying files from NTFS.

Nov 25 2018, 12:28 PM
akyrtzi added a comment to D54826: [Support/FileSystem] Add sub-second precision for atime/mtime of sys::fs::file_status on unix platforms.

I think this is the most important bit of information here. It should have been in the description of the differential.
If the current precision was 1ms (or even 1us), then yes, going all the way down to 1ns may not be important/worthwhile.
But 1 sec precision is clearly laughably not great, and should be improved.

Ah good point, I didn't consider that people are not broadly aware that current file_status has only 'up to second' precision.

Nov 25 2018, 12:12 PM
akyrtzi added a comment to D54826: [Support/FileSystem] Add sub-second precision for atime/mtime of sys::fs::file_status on unix platforms.

On second look, if you are concerned with adding additional fields in basic_file_status, I can replace:

time_t fs_st_atime = 0;
time_t fs_st_mtime = 0;

with uint64_t's which are adequate to include nanosecond precision, while these time_t's are 64bits as well but waste space in the structure.

Nov 25 2018, 12:04 PM
akyrtzi added a comment to D54826: [Support/FileSystem] Add sub-second precision for atime/mtime of sys::fs::file_status on unix platforms.

if there's no consumer for such API (pending review) there is no point in adding it.

To make sure it's clear, this is not a new API. getLastModificationTime() already reports a a timepoint in nanoseconds, but it is always in second intervals. This patch allows it to include the nanoseconds that are part of the underlying file status.

Nov 25 2018, 11:46 AM
akyrtzi added a comment to D54826: [Support/FileSystem] Add sub-second precision for atime/mtime of sys::fs::file_status on unix platforms.

The general changes to the support libraries are usually facilitated because of missing functionality that is required somewhere else and it being the most logical place to add it. If so please attach related revisions to this one (through "Edit Related Revisions") so it's more clear why this was added

There have been enhancements to file_status that added more info without the requirement you stated (attach related revisions to this one), like:
https://reviews.llvm.org/D31110
https://reviews.llvm.org/D18456

Nov 25 2018, 11:41 AM

Nov 22 2018

akyrtzi updated the diff for D54826: [Support/FileSystem] Add sub-second precision for atime/mtime of sys::fs::file_status on unix platforms.

Moved the toTimePoint conversion function to llvm/Support/Chrono.h so it can be broadly available.

Nov 22 2018, 10:32 AM

Nov 21 2018

akyrtzi created D54826: [Support/FileSystem] Add sub-second precision for atime/mtime of sys::fs::file_status on unix platforms.
Nov 21 2018, 11:45 PM

Nov 15 2018

akyrtzi accepted D53900: [CodeComplete] Penalize inherited ObjC properties for auto-completion.
Nov 15 2018, 10:12 AM

Aug 28 2018

akyrtzi accepted D51295: Allow resetting of NumCreatedFIDsForFileID.
Aug 28 2018, 5:18 PM

Aug 1 2018

akyrtzi accepted D50160: [c-index-test] Use correct executable path to discover resource directory..

LGTM!

Aug 1 2018, 5:32 PM
akyrtzi added inline comments to D50160: [c-index-test] Use correct executable path to discover resource directory..
Aug 1 2018, 3:49 PM

Jul 19 2018

akyrtzi accepted D49476: [Index] Set OrigD before D is changed..

Good enough, thanks for looking into this!

Jul 19 2018, 2:55 PM
akyrtzi added a comment to D49476: [Index] Set OrigD before D is changed..

CXIndexDataConsumer.cpp uses ASTNode.OrigD, that code is exercised when you run c-index-test -index-file <...>. I'd recommend to at least check if that command would produce a different output for the test case that you detected originally.

Jul 19 2018, 11:52 AM

Jul 18 2018

akyrtzi added a comment to D49476: [Index] Set OrigD before D is changed..

Is it possible to add a regression test case ? I assume this is fixing some issue, we should make sure we don't regress again.

Jul 18 2018, 3:28 PM

Jun 1 2018

akyrtzi added a comment to D47445: [ASTImporter] Corrected diagnostic client handling in tests..

We could leave disableSourceFileDiagnostics off until someone finds a use case for it.

Jun 1 2018, 11:04 AM

May 31 2018

akyrtzi added a comment to D47445: [ASTImporter] Corrected diagnostic client handling in tests..

Thanks for the explanation. Please do add documentation comments for the new method so people using ASTUnit in their own code have an idea when and why they would need to call this. Something like "if you intend to emit additional diagnostics after the ASTUnit is created [...]".
Also consider making the naming more clear to match the intended purpose, like enableEmittingAdditionalDiagnostics() or something similar.
Otherwise LGTM.

May 31 2018, 12:08 PM

May 30 2018

akyrtzi added a comment to D47445: [ASTImporter] Corrected diagnostic client handling in tests..

It's not clear to me what kind of issue you are addressing, for example is the unit test hitting an assertion hit without your changes ? Or is there something else ?
Also it would be good to add some documentation comments to clarify what's the use case and when ASTUnit::beginSourceFile() should be getting called.

May 30 2018, 11:46 AM

Mar 16 2018

akyrtzi added a comment to D39050: Add index-while-building support to Clang.

That would be good. How would one go about asking Clang to generate this extra information? Would a Clang Plugin be suitable for this?

That's an interesting idea that we could explore, but I don't have much experience with that mechanism to comment on.

Mar 16 2018, 12:59 PM

Mar 14 2018

akyrtzi added a comment to D39050: Add index-while-building support to Clang.

Hey Marc,

Mar 14 2018, 1:44 PM

Jan 26 2018

akyrtzi created D42588: [index] Fix crash when indexing a C++14 PCH/module related to TemplateTemplateParmDecls of alias templates.
Jan 26 2018, 10:48 AM

Jan 10 2018

akyrtzi added a reviewer for D10833: Retrieve BinaryOperator::getOpcode and BinaryOperator::getOpcodeStr via libclang and its python interface: arphaman.
Jan 10 2018, 10:12 AM

Jan 8 2018

akyrtzi added a comment to D41575: [index] Return when DC is null in handleReference.

Ah, sorry I mislead you. To test this try using c-index-test -index-file /path/to/file, see other examples in test/Index, e.g. test/Index/index-file.cpp

Jan 8 2018, 3:43 PM
akyrtzi added a comment to D41575: [index] Return when DC is null in handleReference.

Could you add a test case for this change ? See examples in test/Index/Core.
Also for the test case code: template <template <typename> class Actor = actor>, is the actor reference in this code reported ?
See the test examples on how to print out and test how the source symbols are indexed.

Jan 8 2018, 10:04 AM

Dec 23 2017

akyrtzi accepted D41514: [Index] Reduce size of SymbolInfo struct..

LGTM!

Dec 23 2017, 9:51 AM

Dec 21 2017

akyrtzi added a comment to D41514: [Index] Reduce size of SymbolInfo struct..

Nice one! One minor change I'd suggest is to change SymbolProperty to enum class SymbolProperty : SymbolPropertySet, so that if it needs to increase we only change the type in one place.

Dec 21 2017, 3:05 PM

Dec 7 2017

akyrtzi added a comment to D39050: Add index-while-building support to Clang.

@malaperle, to clarify we are not suggesting that you write your own parser, the suggestion is to use clang in 'fast-scan' mode to get the structure of the declarations of a single file, see CXTranslationUnit_SingleFileParse (along with enabling skipping of bodies). We have found clang is super fast when you only try to get the structure of a file like this. We can make convenient APIs to provide the syntactic structure of declarations based on their location.

Dec 7 2017, 10:50 AM

Nov 16 2017

akyrtzi added a comment to D39050: Add index-while-building support to Clang.

To be honest, I want this functionality to get in as much as you do, and I'm more than happy to prioritize the code review for it :) But the current patch size makes the reviewing really hard (e.g. I would never have caught the BLOCK issues hadn't I tried running the original patch myself). I'm not sure if it's really a common practice to check in a big chunk of code without careful code review and leave potential improvements as followups. I'm sure @klimek would have thoughts about this.

Nov 16 2017, 3:35 PM

Nov 10 2017

akyrtzi added a comment to D39050: Add index-while-building support to Clang.

Hey Eric,

Nov 10 2017, 5:46 PM

Nov 1 2017

akyrtzi accepted D39524: [libclang] Add dummy libclang-headers target.

LGTM.

Nov 1 2017, 8:18 PM

Oct 23 2017

akyrtzi updated subscribers of D39217: [libclang, bindings]: add spelling location.

CC'ed @arphaman.

Oct 23 2017, 11:16 PM

Oct 17 2017

akyrtzi added a comment to D38354: Fix test by using -target instead of cc1 arguments (c-index-test goes through the driver).

FYI, I had added -target in this test in r296263 but it was still failing for some reason in some bots, that's why I then switched to triple via r296265.
I'm not sure if it's going work now or not for all bots, but it does, that is definitely better.

Oct 17 2017, 9:15 PM

Jun 27 2017

akyrtzi added a comment to D34506: Factor out a functionality from `isBeforeInTranslationUnit`.

I'd prefer to avoid including whitespace-only changes (there are a couple of lines in the diff with only whitespace change), otherwise LGTM!

Jun 27 2017, 9:35 AM

Jun 26 2017

akyrtzi added a comment to D34506: Factor out a functionality from `isBeforeInTranslationUnit`.

FullSourceLoc could be useful, it wraps the SourceManager that the SourceLocation come from.

Jun 26 2017, 1:40 PM
akyrtzi added a comment to D34506: Factor out a functionality from `isBeforeInTranslationUnit`.

Comparing SourceLocations from different translation units is not meaningful and my concern is that treating source locations like this can very easily lead to errors where by mistake the code is resolving a SourceLocation with the wrong translation unit and not the translation unit that it came from.

Jun 26 2017, 1:37 PM

Jun 19 2017

akyrtzi updated the diff for D34263: [preprocessor] When preprocessor option 'SingleFileParseMode' is enabled, parse all directive blocks if the condition uses undefined macros.

Update doc-comment for Preprocessor::EvaluateDirectiveExpression

Jun 19 2017, 9:52 AM
akyrtzi updated the diff for D34263: [preprocessor] When preprocessor option 'SingleFileParseMode' is enabled, parse all directive blocks if the condition uses undefined macros.

Provide doc-comments for struct DirectiveEvalResult.

Jun 19 2017, 9:50 AM
akyrtzi added a comment to D34263: [preprocessor] When preprocessor option 'SingleFileParseMode' is enabled, parse all directive blocks if the condition uses undefined macros.

What I find problematic in this patch is PPOpts->SingleFileParseMode checks.
Let's suppose we implement what I mentioned above => how is it going to co-exist nicely? I think code will be less understandable with both: check of flag and call to PPCallbacks.
What I propose is to move the logic from the PPOpts single flag into PPCallbacks. And from check of flag to query of PPCallbacks.
Of course when you create PPCallbacks you can consult global SingleFileParseMode to create default implementation to answer "any symbol is defined", so you get your use-case easily handled, but it also gives others more flexibility.

There is a bit of misunderstanding on what this patch is doing, the patch is not about answering "any symbol is defined". See my cfe-dev email (http://lists.llvm.org/pipermail/cfe-dev/2017-June/054254.html) that provides some more context.
The patch is doing these 2 things:

  • Keep track of whether a preprocessor directive condition used a symbol that was not resolved at all
  • If above is true then make the preprocessor parse all directive blocks, not arbitrary choose one of them.
Jun 19 2017, 9:39 AM
akyrtzi added a comment to D34263: [preprocessor] When preprocessor option 'SingleFileParseMode' is enabled, parse all directive blocks if the condition uses undefined macros.

how many patches for single file mode are coming down the road, though? I'm somewhat concerned about the overall complexity it'll add to clang.

There is no other patch for using this preprocessor option. Any related improvements down the road will be about general improvements for error recovery, for example things like this:

  • Introduce an UnresolvedTypename type instead of changing unresolved types to int.
  • For @interace A : B, don't completely drop B from the super-class list of A if it is unresolved.

These kind of improvements are not conditional.

Jun 19 2017, 9:14 AM

Jun 16 2017

akyrtzi updated the diff for D34263: [preprocessor] When preprocessor option 'SingleFileParseMode' is enabled, parse all directive blocks if the condition uses undefined macros.

Improving doc comment.

Jun 16 2017, 3:03 PM
akyrtzi updated the diff for D34263: [preprocessor] When preprocessor option 'SingleFileParseMode' is enabled, parse all directive blocks if the condition uses undefined macros.

Enhanced doc-comment for the preprocessor option, and fixed indentation on a couple of calls.

Jun 16 2017, 12:28 PM
akyrtzi added inline comments to D34263: [preprocessor] When preprocessor option 'SingleFileParseMode' is enabled, parse all directive blocks if the condition uses undefined macros.
Jun 16 2017, 11:19 AM
akyrtzi added a comment to D34263: [preprocessor] When preprocessor option 'SingleFileParseMode' is enabled, parse all directive blocks if the condition uses undefined macros.

Hey Vladimir, what you are proposing is orthogonal to this patch. You are proposing for "the client to provide the value for an undefined identifier", and the patch is about the client not knowing what the value should be so it fallbacks to parsing all tokens to get the max amount of info. Note that both of the techniques can be combined well, if the client provides the value, the preprocessor will take it into account, otherwise if it is stays unresolved it will fallback to lexing all tokens.
But what you are proposing is not a replacement for what the patch is doing.

Jun 16 2017, 8:43 AM

Jun 15 2017

akyrtzi added a reviewer for D34263: [preprocessor] When preprocessor option 'SingleFileParseMode' is enabled, parse all directive blocks if the condition uses undefined macros: benlangmuir.
Jun 15 2017, 5:59 PM
akyrtzi created D34263: [preprocessor] When preprocessor option 'SingleFileParseMode' is enabled, parse all directive blocks if the condition uses undefined macros.
Jun 15 2017, 5:09 PM

Jun 2 2017

akyrtzi added a comment to D33788: Return a canonical path from getClangResourcePath().

I retract my comment, I see that getMainExecutable on OSX calls realpath as well, so it's good to use realpath in this code to make sure they are in-sync with how the compiler will determine the path.

Jun 2 2017, 10:21 AM
akyrtzi added a comment to D33788: Return a canonical path from getClangResourcePath().

Getting the real path is notoriously slow (at least it's horrible in OSX, not sure about linux). Since this is about dropping the '/../' part, could we do some simple canonicalization of removing the dots ? Not sure if there is an existing function that does that.

Jun 2 2017, 9:36 AM

May 19 2017

akyrtzi closed D33346: Fix forward declarations interfering with USR generation of external source symbols.
May 19 2017, 9:26 PM
akyrtzi accepted D33346: Fix forward declarations interfering with USR generation of external source symbols.

Also used the new Decl:: function in a place in libclang/CIndex.cpp and committed in r303484

May 19 2017, 9:25 PM

May 18 2017

akyrtzi added a comment to D33346: Fix forward declarations interfering with USR generation of external source symbols.

I recommend to add a Decl::getExternalSourceSymbolAttr() to reuse this and avoid code duplication.

May 18 2017, 9:06 PM

Apr 21 2017

akyrtzi closed D30674: [indexer] Don't index symbol definitions marked with the new ExternalSourceSymbolAttr and namespace their USRs using the Attr's definedIn field.
Apr 21 2017, 8:22 AM
akyrtzi accepted D30674: [indexer] Don't index symbol definitions marked with the new ExternalSourceSymbolAttr and namespace their USRs using the Attr's definedIn field.
Apr 21 2017, 8:22 AM
akyrtzi added a comment to D30674: [indexer] Don't index symbol definitions marked with the new ExternalSourceSymbolAttr and namespace their USRs using the Attr's definedIn field.

Committed in pieces with r298392, r300948, r300949.

Apr 21 2017, 8:22 AM

Mar 17 2017

akyrtzi closed D30730: [indexer] Add references to the parent type where its name appears in constructor and destructor definitions and declarations.
Mar 17 2017, 5:05 PM
akyrtzi accepted D30730: [indexer] Add references to the parent type where its name appears in constructor and destructor definitions and declarations.

Committed in r298170.

Mar 17 2017, 5:05 PM

Mar 16 2017

akyrtzi added inline comments to D30730: [indexer] Add references to the parent type where its name appears in constructor and destructor definitions and declarations.
Mar 16 2017, 4:27 PM
akyrtzi closed D30907: [indexer] Add references for ObjC getter=/setter= property attributes and related property getter/setter role fixes.
Mar 16 2017, 11:39 AM
akyrtzi accepted D30907: [indexer] Add references for ObjC getter=/setter= property attributes and related property getter/setter role fixes.

Committed in r297972.

Mar 16 2017, 11:38 AM

Feb 25 2017

akyrtzi closed D30304: [indexer] Expose the logic for indexing local symbols through c-index-test and add tests for it.
Feb 25 2017, 9:57 PM
akyrtzi accepted D30304: [indexer] Expose the logic for indexing local symbols through c-index-test and add tests for it.

Committed in r296282 with a couple of changes:

  • Made it so that parameter definitions have a 'child' relation with their function/method (instead of 'containedBy'). This allows looking up parameters of a function by looking up their children.
  • Given that parameters show up in parent-child hierarchies, I think it is a bit better to make them to a separate 'SymbolKind' instead of a sub-kind. This preserves the property that 'variable' symbol kind does not ever enter a parent-child hierarchy.
Feb 25 2017, 9:56 PM

Feb 23 2017

akyrtzi accepted D28772: [Preprocessor] Fix incorrect token caching that occurs when lexing _Pragma in macro argument pre-expansion mode when skipping a function body.

LGTM!

Feb 23 2017, 5:22 PM

Feb 16 2017

akyrtzi closed D30012: [indexer] Relate properties to their backing ivars and mark implicit getters and setters with the Implicit role .
Feb 16 2017, 9:05 PM
akyrtzi accepted D30012: [indexer] Relate properties to their backing ivars and mark implicit getters and setters with the Implicit role .

Committed in r295416, thanks!

Feb 16 2017, 9:04 PM
akyrtzi added inline comments to D30012: [indexer] Relate properties to their backing ivars and mark implicit getters and setters with the Implicit role .
Feb 16 2017, 2:42 PM
akyrtzi added inline comments to D30012: [indexer] Relate properties to their backing ivars and mark implicit getters and setters with the Implicit role .
Feb 16 2017, 8:44 AM

Dec 16 2016

akyrtzi closed D26907: libclang: Restore the CXXRecordDecl path for clang_Type_getNumTemplateArguments and clang_Type_getTemplateArgumentAsType.
Dec 16 2016, 1:51 PM · Restricted Project
akyrtzi accepted D26907: libclang: Restore the CXXRecordDecl path for clang_Type_getNumTemplateArguments and clang_Type_getTemplateArgumentAsType.

Committed via r289995, thanks!

Dec 16 2016, 1:51 PM · Restricted Project

Dec 1 2016

akyrtzi added a comment to D26907: libclang: Restore the CXXRecordDecl path for clang_Type_getNumTemplateArguments and clang_Type_getTemplateArgumentAsType.

When I try just the test case (without the other changes), it doesn't fail when using r288438; so I'm not sure whether the test is actually testing the regression.

Dec 1 2016, 4:02 PM · Restricted Project
akyrtzi closed D26788: libclang: Add APIs to check the result of an integer expression in CXEvalResult without overflow..
Dec 1 2016, 3:52 PM · Restricted Project
akyrtzi accepted D26788: libclang: Add APIs to check the result of an integer expression in CXEvalResult without overflow..

LGTM, thanks! Committed in r288438.

Dec 1 2016, 3:51 PM · Restricted Project

Nov 15 2016

akyrtzi closed D26663: libclang: Generalize clang_getNumTemplateArguments and clang_getTemplateArgumentAsType to other kind of specializations..
Nov 15 2016, 1:02 PM · Restricted Project
akyrtzi accepted D26663: libclang: Generalize clang_getNumTemplateArguments and clang_getTemplateArgumentAsType to other kind of specializations..

LGTM, thanks! Committed in r287024.

Nov 15 2016, 1:01 PM · Restricted Project

Nov 9 2016

akyrtzi accepted D26446: libclang: Avoid returning an extra token in clang_tokenize.
Nov 9 2016, 4:15 PM
akyrtzi added a comment to D26446: libclang: Avoid returning an extra token in clang_tokenize.

Committed in r286421, thank you!

Nov 9 2016, 4:10 PM
akyrtzi added a comment to D26446: libclang: Avoid returning an extra token in clang_tokenize.

The fix is correct, the comment for CXSourceRange says that it "Identifies a half-open character range in the source code".
But you don't need the other changes to test this. Here's an example:

Nov 9 2016, 10:24 AM

Oct 28 2016

akyrtzi added a comment to D26054: Use `getFileLoc()` instead of `getSpellingLoc()` in the ASTImporter.

LGTM.

Oct 28 2016, 11:03 AM

Oct 13 2016

akyrtzi added a comment to D25520: [CodeCompletion] Add block placeholders when completing member access for Objective-C block property setters.

Another recommendation for follow-up. When invoking completion on the right-hand side of the assignment it should provide a block literal completion with high priority. For example, when completing like this:

Oct 13 2016, 2:11 PM
akyrtzi added a comment to D25520: [CodeCompletion] Add block placeholders when completing member access for Objective-C block property setters.
What do you think of the following possible priority heuristic
Oct 13 2016, 10:18 AM

Oct 12 2016

akyrtzi added a comment to D25520: [CodeCompletion] Add block placeholders when completing member access for Objective-C block property setters.

What if the user just wants to invoke the block, this is as common or more, like:

Oct 12 2016, 11:27 AM

Sep 28 2016

akyrtzi added a comment to D23662: [libclang] Control whether crash recovery is enabled/disabled using function argument..

I could disable crash recovery by calling clang_toggleCrashRecovery(false) after clang_createIndex has been called but this doesn't work because the right JVM handler isn't reinstalled

Sep 28 2016, 9:25 AM

Aug 31 2016

akyrtzi added a comment to rL280174: [analyzer] Use lazily created buffer in EmptyLocalizationContextChecker.

LGTM!

Aug 31 2016, 6:01 PM

Jul 22 2016

akyrtzi added a comment to D17820: Clang Code Completion Filtering .

Thanks for making the changes; I'd recommend to go ahead and commit and we can iterate post-commit if necessary.

Jul 22 2016, 3:45 PM

Mar 2 2016

akyrtzi added a comment to D17820: Clang Code Completion Filtering .

But, we currently always request code completion at a word start boundary so nothing would change for us. That said, I see how this patch could be seen as a breaking behavior change, and thus should probably only get enabled by an explicit option - if at all.

Mar 2 2016, 11:59 AM
akyrtzi added a comment to D17820: Clang Code Completion Filtering .

We should not bake-in filtering into the clang CodeComplete API. What kind of filtering to use should be on a higher level, different clients may want different filtering, e.g. prefix filtering, fuzzy filtering, etc.

Mar 2 2016, 11:50 AM

Feb 20 2016

akyrtzi added a comment to D17026: Changed ASTImporter DiagnosticsEngine from FromDiag to ToDiag for unsupported ASTNodes Import..

Could you check if you can decouple VerifyDiagnosticConsumer from depending on a particular DiagnosticsEngine ?

Feb 20 2016, 11:01 AM
akyrtzi added a comment to D17026: Changed ASTImporter DiagnosticsEngine from FromDiag to ToDiag for unsupported ASTNodes Import..

I'm not sure removing all the Importer.FromDiag() calls is the right thing. You are changing

Feb 20 2016, 10:59 AM

Feb 18 2016

akyrtzi added a comment to D17029: [AST] Implemented missing import for the Template type parameter and Injected Class Name in ASTImporter class..

If I apply just the test changes without the rest of the changes, I don't see any failures, so I'm not sure these are effective tests.

Feb 18 2016, 3:30 PM
akyrtzi added a comment to D17026: Changed ASTImporter DiagnosticsEngine from FromDiag to ToDiag for unsupported ASTNodes Import..

Doesn't this mean that _all_ of the Importer.FromDiag() calls will be ignored by VerifyDiagnosticConsumer ? Why specifically change only this two and what are we going to do with the others ?
This seems more like needing a fix higher up in the stack, not at the ASTImporter level.

Feb 18 2016, 3:23 PM
akyrtzi accepted D16923: [AST] Implemented missing VisitAccessSpecDecl function in ASTImporter class. .

LGTM, committed in r261274.

Feb 18 2016, 3:13 PM

Feb 16 2016

akyrtzi updated subscribers of D17325: Introduce llvm/ADT/OptionSet.h utility class.
Feb 16 2016, 8:29 PM
akyrtzi retitled D17325: Introduce llvm/ADT/OptionSet.h utility class from to Introduce llvm/ADT/OptionSet.h utility class.
Feb 16 2016, 8:22 PM

Jan 13 2016

akyrtzi added a comment to D10833: Retrieve BinaryOperator::getOpcode and BinaryOperator::getOpcodeStr via libclang and its python interface.

Tests need to be updated, for example:

Jan 13 2016, 12:12 PM

Dec 15 2015

akyrtzi added a comment to D15173: [Preprocessor] Fix assertion in AnnotatePreviousCachedTokens.

LGTM.

Dec 15 2015, 4:21 PM