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

2017-02-23 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi accepted this revision. akyrtzi added a comment. This revision is now accepted and ready to land. LGTM! Repository: rL LLVM https://reviews.llvm.org/D28772 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D34263: [preprocessor] When preprocessor option 'SingleFileParseMode' is enabled, parse all directive blocks if the condition uses undefined macros

2017-06-19 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 103066. akyrtzi added a comment. Provide doc-comments for `struct DirectiveEvalResult`. https://reviews.llvm.org/D34263 Files: include/clang/Lex/Preprocessor.h include/clang/Lex/PreprocessorOptions.h lib/Lex/PPDirectives.cpp

[PATCH] D34263: [preprocessor] When preprocessor option 'SingleFileParseMode' is enabled, parse all directive blocks if the condition uses undefined macros

2017-06-19 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. In https://reviews.llvm.org/D34263#783694, @klimek wrote: > 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.

[PATCH] D34263: [preprocessor] When preprocessor option 'SingleFileParseMode' is enabled, parse all directive blocks if the condition uses undefined macros

2017-06-19 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. In https://reviews.llvm.org/D34263#783770, @voskresensky.vladimir wrote: > 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

[PATCH] D34263: [preprocessor] When preprocessor option 'SingleFileParseMode' is enabled, parse all directive blocks if the condition uses undefined macros

2017-06-19 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 103067. akyrtzi added a comment. Update doc-comment for `Preprocessor::EvaluateDirectiveExpression` https://reviews.llvm.org/D34263 Files: include/clang/Lex/Preprocessor.h include/clang/Lex/PreprocessorOptions.h lib/Lex/PPDirectives.cpp

[PATCH] D34506: Relax an assert in the comparison of source locations

2017-06-26 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. Comparing SourceLocations from different translation units is not meaningful and my concern is that treating source locations like this can very easy lead to errors where by mistake the code is resolving a SourceLocation with the wrong translation unit and not the

[PATCH] D34506: Relax an assert in the comparison of source locations

2017-06-26 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. `FullSourceLoc` could be useful, it wraps the SourceManager that the SourceLocation come from. Repository: rL LLVM https://reviews.llvm.org/D34506 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D34506: Factor out a functionality from `isBeforeInTranslationUnit`

2017-06-27 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. I'd prefer to avoid including whitespace-only changes (there are a couple of lines in the diff with only whitespace change), otherwise LGTM! https://reviews.llvm.org/D34506 ___ cfe-commits mailing list

[PATCH] D33788: Return a canonical path from getClangResourcePath()

2017-06-02 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. 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.

[PATCH] D33788: Return a canonical path from getClangResourcePath()

2017-06-02 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. 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. https://reviews.llvm.org/D33788

[PATCH] D34263: [preprocessor] When preprocessor option 'SingleFileParseMode' is enabled, parse all directive blocks if the condition uses undefined macros

2017-06-16 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added inline comments. Comment at: lib/Lex/PPDirectives.cpp:2774 +// the directive blocks. +CurPPLexer->pushConditionalLevel(CI.IfLoc, /*wasskip*/false, + /*foundnonskip*/true, /*foundelse*/true); benlangmuir

[PATCH] D34263: [preprocessor] When preprocessor option 'SingleFileParseMode' is enabled, parse all directive blocks if the condition uses undefined macros

2017-06-16 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 102859. akyrtzi added a comment. Enhanced doc-comment for the preprocessor option, and fixed indentation on a couple of calls. https://reviews.llvm.org/D34263 Files: include/clang/Lex/Preprocessor.h include/clang/Lex/PreprocessorOptions.h

[PATCH] D34263: [preprocessor] When preprocessor option 'SingleFileParseMode' is enabled, parse all directive blocks if the condition uses undefined macros

2017-06-16 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 102885. akyrtzi added a comment. Improving doc comment. https://reviews.llvm.org/D34263 Files: include/clang/Lex/Preprocessor.h include/clang/Lex/PreprocessorOptions.h lib/Lex/PPDirectives.cpp lib/Lex/PPExpressions.cpp

[PATCH] D34263: [preprocessor] When preprocessor option 'SingleFileParseMode' is enabled, parse all directive blocks if the condition uses undefined macros

2017-06-15 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision. This is useful for being able to parse the preprocessor directive blocks even the header that defined the macro that they check for hasn't been included. https://reviews.llvm.org/D34263 Files: include/clang/Lex/Preprocessor.h lib/Lex/PPDirectives.cpp

[PATCH] D34263: [preprocessor] When preprocessor option 'SingleFileParseMode' is enabled, parse all directive blocks if the condition uses undefined macros

2017-06-16 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. 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

[PATCH] D38354: Fix test by using -target instead of cc1 arguments (c-index-test goes through the driver)

2017-10-17 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. 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.

[PATCH] D39050: Add index-while-building support to Clang

2017-11-16 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. > 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

[PATCH] D39050: Add index-while-building support to Clang

2017-11-10 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. Hey Eric, In https://reviews.llvm.org/D39050#921748, @ioeric wrote: > >> - I think the implementation of the index output logic (e.g. > >> `IndexUnitWriter` and bit format file) can be abstracted away (and split > >> into separate patches) so that you can unit-test

[PATCH] D39050: Add index-while-building support to Clang

2017-12-07 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. @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).

[PATCH] D47445: [ASTImporter] Corrected diagnostic client handling in tests.

2018-05-30 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. 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

[PATCH] D47445: [ASTImporter] Corrected diagnostic client handling in tests.

2018-05-31 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. 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

[PATCH] D47445: [ASTImporter] Corrected diagnostic client handling in tests.

2018-06-01 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. We could leave `disableSourceFileDiagnostics` off until someone finds a use case for it. Repository: rC Clang https://reviews.llvm.org/D47445 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D41514: [Index] Reduce size of SymbolInfo struct.

2017-12-21 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. 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. Repository: rC Clang https://reviews.llvm.org/D41514

[PATCH] D41514: [Index] Reduce size of SymbolInfo struct.

2017-12-23 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi accepted this revision. akyrtzi added a comment. This revision is now accepted and ready to land. LGTM! Repository: rC Clang https://reviews.llvm.org/D41514 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D41575: [index] Return when DC is null in handleReference

2018-01-08 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. 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` Repository: rL LLVM https://reviews.llvm.org/D41575 ___

[PATCH] D41575: [index] Return when DC is null in handleReference

2018-01-08 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. Could you add a test case for this change ? See examples in `test/Index/Core`. Also for the test case code: `template 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

[PATCH] D42588: [index] Fix crash when indexing a C++14 PCH/module related to TemplateTemplateParmDecls of alias templates

2018-01-26 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision. akyrtzi added reviewers: cfe-commits, nathawes. Herald added subscribers: kristof.beyls, aemerson. TemplateTemplateParmDecls of alias templates ended-up serialized as 'file-level decls' which was causing a crash while trying to index a PCH/module file that

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

2018-08-01 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added inline comments. Comment at: clang/tools/c-index-test/core_main.cpp:210 + void *P = (void*) (intptr_t) indextest_core_main; + std::string Executable = llvm::sys::fs::getMainExecutable(ProgName, P); SmallVector ArgsWithProgName; Could you move

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

2018-08-01 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi accepted this revision. akyrtzi added a comment. This revision is now accepted and ready to land. LGTM! https://reviews.llvm.org/D50160 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

2018-07-18 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. 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. Repository: rC Clang https://reviews.llvm.org/D49476 ___ cfe-commits mailing list

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

2018-07-19 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. `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. Repository: rC

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

2018-07-19 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi accepted this revision. akyrtzi added a comment. This revision is now accepted and ready to land. Good enough, thanks for looking into this! Repository: rC Clang https://reviews.llvm.org/D49476 ___ cfe-commits mailing list

[PATCH] D39050: Add index-while-building support to Clang

2018-03-14 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. Hey Marc, > The fact that both clang and clangd have to agree on the format so that > index-while-building can be used seems to make it inherently not possible to > extend I don't think "not possible to extend" is quite correct, we can make it so that the format

[PATCH] D39050: Add index-while-building support to Clang

2018-03-16 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. > 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. > Only the

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

2018-12-05 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added inline comments. Comment at: lib/Index/USRGeneration.cpp:274 +if (unsigned quals = MD->getTypeQualifiers().getCVRUQualifiers()) Out << (char)('0' + quals); switch (MD->getRefQualifier()) { rjmccall wrote: > Paging @akyrtzi here.

[PATCH] D54862: [OpenCL] Add generic AS to 'this' pointer

2018-12-06 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added inline comments. Comment at: lib/Index/USRGeneration.cpp:274 +if (unsigned quals = MD->getTypeQualifiers().getCVRUQualifiers()) Out << (char)('0' + quals); switch (MD->getRefQualifier()) { rjmccall wrote: > akyrtzi wrote: > >

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

2019-02-23 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added inline comments. Comment at: clang/lib/DirectoryWatcher/DirectoryWatcher-linux.inc.h:135 +if (!statusOpt.hasValue()) + K = DirectoryWatcher::EventKind::Removed; + } mgorny wrote: > akyrtzi wrote: > > mgorny wrote: > > >

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

2019-02-23 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added inline comments. Comment at: clang/lib/DirectoryWatcher/DirectoryWatcher-linux.inc.h:135 +if (!statusOpt.hasValue()) + K = DirectoryWatcher::EventKind::Removed; + } mgorny wrote: > akyrtzi wrote: > > mgorny wrote: > > >

[PATCH] D39050: Add index-while-building support to Clang

2019-03-06 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added inline comments. Comment at: include/clang/Frontend/FrontendOptions.h:380 + /// Whether to ignore system files when writing out index data + unsigned IndexIgnoreSystemSymbols : 1; + /// Whether to include the codegen name of symbols in the index data

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

2019-02-23 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added inline comments. Comment at: clang/lib/DirectoryWatcher/DirectoryWatcher-linux.inc.h:135 +if (!statusOpt.hasValue()) + K = DirectoryWatcher::EventKind::Removed; + } mgorny wrote: > jkorous wrote: > > mgorny wrote: > > > Why? I

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

2019-02-23 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added inline comments. Comment at: clang/lib/DirectoryWatcher/DirectoryWatcher-linux.inc.h:135 +if (!statusOpt.hasValue()) + K = DirectoryWatcher::EventKind::Removed; + } mgorny wrote: > akyrtzi wrote: > > mgorny wrote: > > >

[PATCH] D65846: Improve error message from FrontendAction

2019-08-07 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. Sorry, it's not clear to me what is the issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65846/new/ https://reviews.llvm.org/D65846 ___ cfe-commits mailing list

[PATCH] D65846: Improve error message from FrontendAction

2019-08-07 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. In D65846#1619645 , @jfb wrote: > My current guess is that this part of the test: > > c-index-test -write-pch %t.h.pch %s -fmodules -fmodules-cache-path=%t.mcp > -Xclang -triple -Xclang x86_64-apple-darwin > > > Is expected to

[PATCH] D65846: Improve error message from FrontendAction

2019-08-07 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. In D65846#1619752 , @bruno wrote: > > `clang -fmodules -fmodules-cache-path=...` is supposed to create the > > directory for the cache path, including the parent directories, AFAIK. If > > this doesn't happen it is a behavior

[PATCH] D89024: [AST] Fix crashes caused by redeclarations in hidden prototypes

2020-10-08 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGfbb499ef255b: [AST] Fix crashes caused by redeclarations in hidden prototypes (authored by bnbarham, committed by akyrtzi). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D89453: Fix hidden-redecls.m test for some environments

2020-10-15 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. Thank you! Are you able to commit it by yourself? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89453/new/ https://reviews.llvm.org/D89453 ___ cfe-commits mailing list

[PATCH] D89024: [AST] Fix crashes caused by redeclarations in hidden prototypes

2020-10-07 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi accepted this revision. akyrtzi added a comment. This revision is now accepted and ready to land. Good catch! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89024/new/ https://reviews.llvm.org/D89024

[PATCH] D60193: [OpenCL] Added addrspace_cast operator

2020-10-21 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added inline comments. Herald added a subscriber: dexonsmith. Comment at: clang/include/clang-c/Index.h:2057 + */ + CXCursor_CXXAddrspaceCastExpr = 129, + Hi Anastasia, apologies for not catching this earlier, but libclang is intended to keep a

[PATCH] D91580: [Frontend] Add flag to allow PCM generation despite compiler errors

2020-11-17 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. I'd like if we only had to use one flag (`-fallow-pcm-with-compiler-errors`) and have it handle both modules and PCH. Could you make the flag also work for PCH and/or add a test that verifies it works? You may only have to change Opts.AllowPCHWithCompilerErrors =

[PATCH] D91580: [Frontend] Add flag to allow PCM generation despite compiler errors

2020-11-17 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG5834996fefc9: [Frontend] Add flag to allow PCM generation despite compiler errors (authored by bnbarham, committed by akyrtzi). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D95159: [ASTReader] Allow controlling separately whether validation should be disabled for a PCH vs a module file

2021-01-21 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGb0e89906f5b7: [ASTReader] Allow controlling separately whether validation should be disabled… (authored by akyrtzi). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D95159: [ASTReader] Allow controlling separately whether validation should be disabled for a PCH vs a module file

2021-01-21 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 318335. akyrtzi edited the summary of this revision. akyrtzi added a comment. Fix typo in commit message, 'state' -> 'stale' Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95159/new/

[PATCH] D95159: [ASTReader] Allow controlling separately whether validation should be disabled for a PCH vs a module file

2021-01-21 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 318314. akyrtzi added a comment. clang-format changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95159/new/ https://reviews.llvm.org/D95159 Files: clang/include/clang/Driver/Options.td

[PATCH] D95159: [ASTReader] Allow controlling separately whether validation should be disabled for a PCH vs a module file

2021-01-21 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 318333. akyrtzi added a comment. Use `getValueOr` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95159/new/ https://reviews.llvm.org/D95159 Files: clang/include/clang/Driver/Options.td

[PATCH] D95159: [ASTReader] Allow controlling separately whether validation should be disabled for a PCH vs a module file

2021-01-21 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision. Herald added subscribers: dang, arphaman. Herald added a reviewer: jansvoboda11. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This addresses an issue with how the PCH preable works, specifically:

[PATCH] D95989: [ASTReader] Always rebuild a cached module that has errors

2021-02-03 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa2c1054c303f: [ASTReader] Always rebuild a cached module that has errors (authored by bnbarham, committed by akyrtzi). Repository: rG LLVM Github

[PATCH] D102159: [index][analyzer][ctu] Eliminate white spaces in the CTU lookup name.

2021-05-10 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. Maybe we could also handle this kind of type instead of leaving it 'unhandled'? What `Type` is it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102159/new/ https://reviews.llvm.org/D102159

[PATCH] D102614: [index] Add support for type of pointers to class members

2021-05-19 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added inline comments. Comment at: clang/lib/Index/USRGeneration.cpp:897 +if (const MemberPointerType *MPT = T->getAs()) { + VisitType(QualType(MPT->getClass(), 0)); + Out << "::*"; A bit better to do `VisitTagDecl(MPT->getClass())`, what

[PATCH] D102159: [index][analyzer][ctu] Eliminate white spaces in the CTU lookup name.

2021-05-11 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. To clarify, I was suggesting that //in addition// to removing the space from unhandled types, we also handle the member function pointer type and not leave it in this fallback case. Types should have unique USR characters so that overloaded functions (overloaded on the

[PATCH] D102001: [Index] Ignore nullptr decls for indexing

2021-05-06 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGa3a8a1a15b52: [Index] Ignore nullptr decls for indexing (authored by ahoppen, committed by akyrtzi). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

2021-04-23 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added inline comments. Comment at: clang/lib/Index/USRGeneration.cpp:708 c = 'd'; break; +case BuiltinType::Ibm128: // FIXME: Need separate tag case BuiltinType::LongDouble: rjmccall wrote: > @akyrtzi @benlangmuir We can just

[PATCH] D101645: [clang] RecursiveASTVisitor visits ObjCPropertyRefExpr's class receiver

2021-05-03 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added inline comments. Comment at: clang/test/Index/Core/index-source.m:410 // CHECK-NEXT: RelCont | classReceivers | c:@F@classReceivers +// CHECK: [[@LINE-3]]:3 | class/ObjC | ClassReceivers | c:objc(cs)ClassReceivers | _OBJC_CLASS_$_ClassReceivers | Ref,RelCont |

[PATCH] D96246: Make sure a module file with errors produced via '-fallow-pcm-with-compiler-errors' can be loaded when using implicit modules

2021-02-08 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. A module with errors would be marked as out-of-date, then the `compilerModule` action would produce it, but due to the error it would be treated as

[PATCH] D96246: Make sure a module file with errors produced via '-fallow-pcm-with-compiler-errors' can be loaded when using implicit modules

2021-02-08 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 322166. akyrtzi added a comment. clang-format change Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96246/new/ https://reviews.llvm.org/D96246 Files: clang/include/clang/Serialization/ASTReader.h

[PATCH] D96246: Make sure a module file with errors produced via '-fallow-pcm-with-compiler-errors' can be loaded when using implicit modules

2021-02-08 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa8cb39bab04c: Make sure a module file with errors produced via -fallow-pcm-with-compiler… (authored by akyrtzi). Repository: rG LLVM Github

[PATCH] D97204: [RFC] Clang 64-bit source locations

2021-02-24 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. In D97204#2586111 , @rsmith wrote: > Can we avoid a libclang ABI break if we don't allow the use of 64-bit source > locations for builds with 32-bit pointers? To @rsmith's point, the simplest option may be to avoid building

[PATCH] D97204: [RFC] Clang 64-bit source locations

2021-02-22 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. Hi @miyuki, > A major thing worth noting is that 64-bit source locations will > require an ABI breakage in libclang. This patch changes the bit width > in libclang unconditionally, rather than making it configurable. Is it possible to make the libclang change

[PATCH] D100619: [ASTReader] Only mark module out of date if not already compiled

2021-04-16 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added inline comments. Comment at: clang/test/Modules/Inputs/error/error.h:1 +#pragma mark mark + Is this pragma relevant for the test? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100619/new/

[PATCH] D100619: [ASTReader] Only mark module out of date if not already compiled

2021-04-16 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG1206b95e0703: [ASTReader] Only mark module out of date if not already compiled (authored by bnbarham, committed by akyrtzi). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D112591: [clang] [Objective C] Inclusive language: use objcmt-allowlist-dir-path= instead of objcmt-white-list-dir-path=

2021-10-27 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi accepted this revision. akyrtzi added a comment. This revision is now accepted and ready to land. Do you intend to rename `clang/test/ARCMT/whitelisted` directory as a follow-up? Otherwise LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D121181: [DNM] Testing for premerge builds

2022-03-07 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D121181 Files: clang/lib/Sema/TreeTransform.h Index:

[PATCH] D121184: Try code change

2022-03-07 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D121184 Files: clang/lib/Sema/TreeTransform.h Index:

[PATCH] D120426: [Sema] Mark the referenced destructor during transformation of a `CXXBindTemporaryExpr`

2022-03-08 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf2b24905bfed: [Sema] Mark the referenced destructor during transformation of a… (authored by akyrtzi). Herald added a project: All. Repository:

[PATCH] D120426: [Sema] Mark the referenced destructor during transformation of a `CXXBindTemporaryExpr`

2022-02-24 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 411185. akyrtzi added a comment. Make sure to check the destructor decl pointer for nil before passing to `Sema::MarkFunctionReferenced()` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120426/new/

[PATCH] D120426: [Sema] Mark the referenced destructor during transformation of a `CXXBindTemporaryExpr`

2022-02-24 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 411186. akyrtzi added a comment. Move the `const_cast` to the call site. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120426/new/ https://reviews.llvm.org/D120426 Files: clang/lib/Sema/TreeTransform.h

[PATCH] D120426: [Sema] Mark the referenced destructor during transformation of a `CXXBindTemporaryExpr`

2022-02-24 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 411219. akyrtzi added a comment. Add a target triple to the test, so it can pass on windows. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120426/new/ https://reviews.llvm.org/D120426 Files:

[PATCH] D102614: [index] Add support for type of pointers to class members

2022-02-15 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. > What do you think about this patch? Can it be landed now? Or I should debug > the crash in the Windows version detected with the previous version of my > patch. Is your change introducing the crash or is the crash triggered with the test file without your changes as

[PATCH] D120426: [Sema] Mark the referenced destructor during transformation of a `CXXBindTemporaryExpr`

2022-02-23 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Otherwise we will fail to generate the definition of a defaulted destructor, if the only reference was in a templated temporary. rdar://89366678

[PATCH] D123100: [Support/Hash functions] Change the `final()` and `result()` of the hashing functions to return an array of bytes

2022-04-04 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 420359. akyrtzi added a comment. Adjust `ASTFileSignature` to accept only the array hash bytes and directly from the `final()` call. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123100/new/

[PATCH] D123100: [Support/Hash functions] Change the `final()` and `result()` of the hashing functions to return an array of bytes

2022-04-04 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 420377. akyrtzi added a comment. - Move `BLAKE3` back to templated sizes for `final()` and `result()` and add `TruncatedBLAKE3` that has the size parameter on the class. - Make `MD5Result` inherit from `std::array` which both simplifies its API and makes it

[PATCH] D123100: [Support/Hash functions] Change the `final()` and `result()` of the hashing functions to return an array of bytes

2022-04-04 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added inline comments. Comment at: llvm/include/llvm/Support/BLAKE3.h:38 +/// A class that wraps the BLAKE3 algorithm. +template class BLAKE3 { public: dexonsmith wrote: > The visual noise of `BLAKE3<>` everywhere is a bit unfortunate. > > Another

[PATCH] D123100: [Support/Hash functions] Change the `final()` and `result()` of the hashing functions to return an array of bytes

2022-04-04 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision. Herald added subscribers: ayermolo, sdasgup3, wenzhicui, wrengr, Chia-hungDuan, dcaballe, cota, teijeong, dexonsmith, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, mgester, arpith-jacob, antiagainst, shauheen, rriddle,

[PATCH] D123100: [Support/Hash functions] Change the `final()` and `result()` of the hashing functions to return an array of bytes

2022-04-04 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 420378. akyrtzi added a comment. Also revert the `README` changes to the previous version of `BLAKE3` class. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123100/new/ https://reviews.llvm.org/D123100 Files:

[PATCH] D123100: [Support/Hash functions] Change the `final()` and `result()` of the hashing functions to return an array of bytes

2022-04-05 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 420545. akyrtzi added a comment. Expand type instead of using `auto` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123100/new/ https://reviews.llvm.org/D123100 Files: bolt/lib/Core/DebugData.cpp

[PATCH] D123100: [Support/Hash functions] Change the `final()` and `result()` of the hashing functions to return an array of bytes

2022-04-05 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added inline comments. Comment at: bolt/lib/Core/DebugData.cpp:823 Hasher.update(AbbrevData); -StringRef Key = Hasher.final(); +auto Hash = Hasher.final(); +StringRef Key((const char *)Hash.data(), Hash.size()); jhenderson wrote: > Amir

[PATCH] D123100: [Support/Hash functions] Change the `final()` and `result()` of the hashing functions to return an array of bytes

2022-04-05 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG330268ba346b: [Support/Hash functions] Change the `final()` and `result()` of the hashing… (authored by akyrtzi). Repository: rG LLVM Github

[PATCH] D125487: [Tooling/DependencyScanning] Refactor dependency scanning to produce pre-lexed preprocessor directive tokens, instead of minimized sources

2022-05-15 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 429580. akyrtzi added a comment. Fix issue where an empty '#' in a line was causing the immediately following preprocessor directive to be skipped. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125487/new/

[PATCH] D125488: [Preprocessor] Make the special lexing for dependency scanning a first-class feature of the `Preprocessor` and `Lexer`

2022-05-21 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 431151. akyrtzi added a comment. Update due to source change in the previous patch (https://reviews.llvm.org/D125487) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125488/new/ https://reviews.llvm.org/D125488

[PATCH] D125487: [Tooling/DependencyScanning] Refactor dependency scanning to produce pre-lexed preprocessor directive tokens, instead of minimized sources

2022-05-21 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 431150. akyrtzi added a comment. Remove `DependencyScanningFilesystem::disableDirectivesScanning()` function. Unlike source minimization which changes the source contents size and needed to be disabled in certain situations, directive lexing keeps the same

[PATCH] D125487: [Tooling/DependencyScanning] Refactor dependency scanning to produce pre-lexed preprocessor directive tokens, instead of minimized sources

2022-05-21 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi marked an inline comment as done. akyrtzi added inline comments. Comment at: clang/unittests/Lex/DependencyDirectivesScannerTest.cpp:175 "#define MACRO con tent ", Out)); - EXPECT_STREQ("#define MACRO con tent\n", Out.data()); + EXPECT_STREQ("#define MACRO

[PATCH] D125936: [Sema] Relax an assertion in BuildStmtExpr

2022-05-26 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. The assertion was assuming "the expression doesn't need cleanups", have you considered adding a test that checks that the destructor of the temporary inside the asm statement is called, to ensure these temporaries are properly handled? Repository: rG LLVM Github

[PATCH] D125484: [Tooling/DependencyScanning] Rename refactorings towards transitioning dependency scanning to use pre-lexed preprocessor directive tokens

2022-05-26 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. akyrtzi marked an inline comment as done. Closed by commit rGb58a420ff4f9: [Tooling/DependencyScanning] Rename refactorings towards transitioning… (authored by akyrtzi). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D125486: [Tooling/DependencyScanning] Remove `ExcludedPreprocessorDirectiveSkipMapping` and related functionality

2022-05-26 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb4c83a13f664: [Tooling/DependencyScanning Preprocessor] Refactor dependency scanning to… (authored by akyrtzi). Changed prior to commit:

[PATCH] D125488: [Preprocessor] Make the special lexing for dependency scanning a first-class feature of the `Preprocessor` and `Lexer`

2022-05-26 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi closed this revision. akyrtzi added a comment. b4c83a13f664582015ea22924b9a0c6290d41f5b Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125488/new/

[PATCH] D125487: [Tooling/DependencyScanning] Refactor dependency scanning to produce pre-lexed preprocessor directive tokens, instead of minimized sources

2022-05-26 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi closed this revision. akyrtzi marked an inline comment as done. akyrtzi added a comment. b4c83a13f664582015ea22924b9a0c6290d41f5b Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D125487: [Tooling/DependencyScanning] Refactor dependency scanning to produce pre-lexed preprocessor directive tokens, instead of minimized sources

2022-05-12 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Depends on D125486 This is 3/4 of a series of patches for making the special lexing for

[PATCH] D125484: [Tooling/DependencyScanning] Rename refactorings towards transitioning dependency scanning to use pre-lexed preprocessor directive tokens

2022-05-12 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision. Herald added a subscriber: mgorny. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This is 1/4 of a series of patches for making the special lexing for dependency

[PATCH] D125488: [Preprocessor] Make the special lexing for dependency scanning a first-class feature of the `Preprocessor` and `Lexer`

2022-05-12 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Depends on D125487 This is 4/4 of a series of patches, bringing the following benefits:

[PATCH] D125486: [Tooling/DependencyScanning] Remove `ExcludedPreprocessorDirectiveSkipMapping` and related functionality

2022-05-12 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Depends on D125484 This is 2/4 of a series of patches for making the special lexing for

[PATCH] D124687: [Tooling/DependencyScanning & Preprocessor] Refactor dependency scanning to record and use pre-lexed preprocessor directive tokens, instead of minimized sources

2022-05-12 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment. I've split this in smaller patches: https://reviews.llvm.org/D125484 - NFC rename refactorings https://reviews.llvm.org/D125486 - Remove `ExcludedPreprocessorDirectiveSkipMapping` https://reviews.llvm.org/D125487 - Change to producing pre-lexed directive tokens instead

  1   2   3   >