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

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. https://reviews.ll

[PATCH] D39217: [libclang, bindings]: add spelling location

akyrtzi added a subscriber: arphaman. akyrtzi added a comment. CC'ed @arphaman. https://reviews.llvm.org/D39217 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

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 Clan

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

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 cfe-commits@lis

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

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 t

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

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 http://lists.llvm.org/cgi-bin/mailman/listinfo/cf

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

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

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

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

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 lib/Le

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

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 the

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

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 wrot

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

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 lib/Lex/PPD

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

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 test/Index/singe-file-parse.

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

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

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

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 less

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

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 lib/Lex/PPExpressions.cp

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

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 lib/Lex/PPEx

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

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.

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 http://lists.llvm.org/cgi

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

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 in

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

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 ___ cfe-

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

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 allow

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

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 l

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

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 `ASTUni

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

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 create

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

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 http://lists.llvm.org/

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

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

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

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

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

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

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

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

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

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

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

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

[PATCH] D65846: Improve error message from FrontendAction

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 cfe-commits

[PATCH] D65846: Improve error message from FrontendAction

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 g

[PATCH] D65846: Improve error message from FrontendAction

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 cha

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

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 cfe-commits

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

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

[PATCH] D132801: [driver] Additional ignoring of module-map related flags, if modules are disabled

akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D132801 Files: clang/lib/Driver/ToolChains/Clang.

[PATCH] D132801: [driver] Additional ignoring of module-map related flags, if modules are disabled

akyrtzi updated this revision to Diff 456483. akyrtzi added a comment. Merge the new `RUN` line together with the prior 2. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132801/new/ https://reviews.llvm.org/D132801 Files: clang/lib/Driver/ToolCha

[PATCH] D132801: [driver] Additional ignoring of module-map related flags, if modules are disabled

akyrtzi marked an inline comment as done. akyrtzi added inline comments. Comment at: clang/test/Driver/modules.m:81 // RUN: %clang -fno-modules -fmodules-validate-system-headers -### %s 2>&1 | FileCheck -check-prefix=VALIDATE_SYSTEM_FLAG %s // VALIDATE_SYSTEM_FLAG-NOT: -fmod

[PATCH] D132801: [driver] Additional ignoring of module-map related flags, if modules are disabled

This revision was automatically updated to reflect the committed changes. akyrtzi marked an inline comment as done. Closed by commit rG33162a81d4c9: [driver] Additional ignoring of module-map related flags, if modules are… (authored by akyrtzi). Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D132801: [driver] Additional ignoring of module-map related flags, if modules are disabled

akyrtzi added a comment. In D132801#3760014 , @rsmith wrote: > This doesn't look right to me -- we still use module maps when modules are > disabled to enforce layering checking, and when > `-fmodules-local-submodule-visibility` is enabled but `-fmodule

[PATCH] D133229: [driver] Prune module-map related flags, if they are not going to be needed

akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. This is follow-up from https://reviews.llvm.org/D132801, but taking into account the conditions where the module-ma

[PATCH] D132801: [driver] Additional ignoring of module-map related flags, if modules are disabled

akyrtzi added a comment. In D132801#3761253 , @akyrtzi wrote: > In D132801#3760014 , @rsmith wrote: > >> This doesn't look right to me -- we still use module maps when modules are >> disabled to enforce layering

[PATCH] D133229: [driver] Prune module-map related flags, if they are not going to be needed

akyrtzi added a comment. In D133229#3768101 , @rsmith wrote: > I think the approach you're taking here is probably doomed -- too many things > in Clang depend on whether we've read module map files, and it seems unlikely > to me that you'll be able to c

[PATCH] D133357: [Lex/DependencyDirectivesScanner] Keep track of the presence of tokens between the last scanned directive and EOF

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. Directive `dependency_directives_scan::tokens_present_before_eof` is introduced to indic

[PATCH] D133357: [Lex/DependencyDirectivesScanner] Keep track of the presence of tokens between the last scanned directive and EOF

akyrtzi updated this revision to Diff 458173. akyrtzi added a comment. Remove a couple of unused local variables. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133357/new/ https://reviews.llvm.org/D133357 Files: clang/include/clang/Lex/Dependenc

[PATCH] D133357: [Lex/DependencyDirectivesScanner] Keep track of the presence of tokens between the last scanned directive and EOF

akyrtzi added inline comments. Comment at: clang/include/clang/Lex/DependencyDirectivesScanner.h:131 +/// \p dependency_directives_scan::tokens_present_before_eof, otherwise this +/// directive will be ignored. /// benlangmuir wrote: > Why would you want to prin

[PATCH] D133357: [Lex/DependencyDirectivesScanner] Keep track of the presence of tokens between the last scanned directive and EOF

akyrtzi added inline comments. Comment at: clang/include/clang/Lex/DependencyDirectivesScanner.h:131 +/// \p dependency_directives_scan::tokens_present_before_eof, otherwise this +/// directive will be ignored. /// benlangmuir wrote: > benlangmuir wrote: > > aky

[PATCH] D133357: [Lex/DependencyDirectivesScanner] Keep track of the presence of tokens between the last scanned directive and EOF

akyrtzi updated this revision to Diff 458360. akyrtzi added a comment. Always print `tokens_present_before_eof` and print it as "" for clarity. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133357/new/ https://reviews.llvm.org/D133357 Files: cl

[PATCH] D133357: [Lex/DependencyDirectivesScanner] Keep track of the presence of tokens between the last scanned directive and EOF

akyrtzi marked 4 inline comments as done. akyrtzi added a comment. @benlangmuir see latest diff. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133357/new/ https://reviews.llvm.org/D133357 ___ cfe-commits

[PATCH] D133357: [Lex/DependencyDirectivesScanner] Keep track of the presence of tokens between the last scanned directive and EOF

akyrtzi updated this revision to Diff 458499. akyrtzi added a comment. Remove leftover doc-comment parameter. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133357/new/ https://reviews.llvm.org/D133357 Files: clang/include/clang/Lex/DependencyDir

[PATCH] D133357: [Lex/DependencyDirectivesScanner] Keep track of the presence of tokens between the last scanned directive and EOF

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGaa484c90cf59: [Lex/DependencyDirectivesScanner] Keep track of the presence of tokens between… (authored by akyrtzi). Repository: rG LLVM Github Mo

[PATCH] D133674: [Lex/DependencyDirectivesScanner] Handle the case where the source line starts with a `tok::hashhash`

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/D133674 Files: clang/lib/Lex/DependencyDirectivesScanner.

[PATCH] D133674: [Lex/DependencyDirectivesScanner] Handle the case where the source line starts with a `tok::hashhash`

akyrtzi added a comment. In D133674#3784602 , @jansvoboda11 wrote: > Could you explain why this is necessary and even correct? I'd expect Clang to > give an error when seeing `##` in this position, and I'd expect the scanner > to do the same. `##` is

[PATCH] D133674: [Lex/DependencyDirectivesScanner] Handle the case where the source line starts with a `tok::hashhash`

akyrtzi updated this revision to Diff 459888. akyrtzi added a comment. Add comment to clarify why we skip if `tok::hashhash` is encountered. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133674/new/ https://reviews.llvm.org/D133674 Files: clang/

[PATCH] D133674: [Lex/DependencyDirectivesScanner] Handle the case where the source line starts with a `tok::hashhash`

akyrtzi added a comment. In D133674#3784710 , @akyrtzi wrote: > In D133674#3784602 , @jansvoboda11 > wrote: > >> Could you explain why this is necessary and even correct? I'd expect Clang >> to give an error whe

[PATCH] D133674: [Lex/DependencyDirectivesScanner] Handle the case where the source line starts with a `tok::hashhash`

This revision was automatically updated to reflect the committed changes. Closed by commit rGb340c5ae4221: [Lex/DependencyDirectivesScanner] Handle the case where the source line starts… (authored by akyrtzi). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D128772: [Lex] Make sure to notify `MultipleIncludeOpt` for "read tokens" during fast dependency directive lexing

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. Otherwise a header may be erroneously marked as having a header macro guard and won't get re-included. Repository: rG L

[PATCH] D128772: [Lex] Make sure to notify `MultipleIncludeOpt` for "read tokens" during fast dependency directive lexing

akyrtzi added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:4251 DepDirectives.front().Tokens[NextDepDirectiveTokenIndex++]; + if (NextDepDirectiveTokenIndex > 1 || DDTok.Kind != tok::hash) { +// Read something other than a preprocessor directive hash.

[PATCH] D128772: [Lex] Make sure to notify `MultipleIncludeOpt` for "read tokens" during fast dependency directive lexing

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc68b8c84eb17: [Lex] Make sure to notify `MultipleIncludeOpt` for "read tokens" during fast… (authored by akyrtzi). Changed prior to commit: https:

[PATCH] D128947: [Lex] Introduce `PPCallbacks::LexedFileChanged()` preprocessor callback

akyrtzi created this revision. Herald added subscribers: shchenz, kbarton, nemanjai. Herald added a project: All. akyrtzi requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This is a preprocessor callback focused on the lexed file changing, wi

[PATCH] D128947: [Lex] Introduce `PPCallbacks::LexedFileChanged()` preprocessor callback

akyrtzi added inline comments. Comment at: clang/lib/Basic/SourceManager.cpp:1016 +return *Name; return StringRef(); } benlangmuir wrote: > Just a suggestion: `value_or` can be a nice way to express simple cases like > this: > > ``` > getFilename(getFil

[PATCH] D128947: [Lex] Introduce `PPCallbacks::LexedFileChanged()` preprocessor callback

akyrtzi updated this revision to Diff 441551. akyrtzi added a comment. Use `Optional::value_or()` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128947/new/ https://reviews.llvm.org/D128947 Files: clang/include/clang/Basic/SourceManager.h clang

[PATCH] D128947: [Lex] Introduce `PPCallbacks::LexedFileChanged()` preprocessor callback

akyrtzi updated this revision to Diff 441556. akyrtzi added a comment. Pass a value for `PrevFID` for `FileChanged()` callback as well, for `PPCallbacks::EnterFile` reason. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128947/new/ https://reviews.

[PATCH] D128947: [Lex] Introduce `PPCallbacks::LexedFileChanged()` preprocessor callback

akyrtzi updated this revision to Diff 441587. akyrtzi added a comment. Herald added a project: clang-tools-extra. Update `clang-tools-extra/test/pp-trace/pp-trace-include.cpp` to accomodate for `PrevFID` getting a value and preserve using `getFileEntryForID()` for the `SourceManager::getFilename(

[PATCH] D128947: [Lex] Introduce `PPCallbacks::LexedFileChanged()` preprocessor callback

akyrtzi updated this revision to Diff 441700. akyrtzi added a comment. Avoid changing the `SourceManager::getFilename()` implementation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128947/new/ https://reviews.llvm.org/D128947 Files: clang-tool

[PATCH] D128947: [Lex] Introduce `PPCallbacks::LexedFileChanged()` preprocessor callback

akyrtzi added inline comments. Comment at: clang/lib/Lex/PPLexerChange.cpp:136 PPCallbacks::EnterFile, FileType); +FileID PrevFID; +SourceLocation EnterLoc; akyrtzi wrote: > benlangmuir wrote: > > Why does `LexedFileChanged` ha

[PATCH] D128947: [Lex] Introduce `PPCallbacks::LexedFileChanged()` preprocessor callback

This revision was automatically updated to reflect the committed changes. Closed by commit rG0d3a2b4c6601: [Lex] Introduce `PPCallbacks::LexedFileChanged()` preprocessor callback (authored by akyrtzi). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D12

[PATCH] D129030: [Driver] Ignore the clang modules validation-related flags if clang modules are not enabled

akyrtzi created this revision. Herald added a project: All. akyrtzi requested review of this revision. Herald added subscribers: cfe-commits, MaskRay. Herald added a project: clang. If clang modules are not enabled it becomes unnecessary to read the session timestamp file in order to pass `-fbuil

[PATCH] D129030: [Driver] Ignore the clang modules validation-related flags if clang modules are not enabled

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG93d6fdfc232c: [Driver] Ignore the clang modules validation-related flags if clang modules are… (authored by akyrtzi). Repository: rG LLVM Github M

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

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, m

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

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/ https://reviews.ll

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

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

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 opti

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

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

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 clang/incl

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

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 w

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

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 Monor

[PATCH] D130443: [CGDebugInfo] Access the current working directory from the `VFS`

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG8dfaecc4c244: [CGDebugInfo] Access the current working directory from the `VFS` (authored by akyrtzi). Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D130443: [CGDebugInfo] Access the current working directory from the `VFS`

akyrtzi added a comment. In D130443#3680753 , @thakis wrote: > Looks like this doesn't build: http://45.33.8.238/linux/82380/step_4.txt Sorry about that, fixed here: https://github.com/llvm/llvm-project/commit/c5ddacb3b6afe2fd507b5f4a10c32ec00ffb245e

[PATCH] D130710: [ASTWriter] Provide capability to output a PCM/PCH file that does not write out information about its output path

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. This is useful to enable sharing of the same PCH file even when it's intended for a different output path. The only inform

[PATCH] D130710: [ASTWriter] Provide capability to output a PCM/PCH file that does not write out information about its output path

akyrtzi added a subscriber: v.g.vassilev. akyrtzi added a comment. @v.g.vassilev is the functionality of "write `ORIGINAL_PCH_DIR` and resolve headers relative to it if PCH file and headers moved together" used by `Cling`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D130710: [ASTWriter] Provide capability to output a PCM/PCH file that does not write out information about its output path

akyrtzi added a subscriber: rmaz. akyrtzi added a comment. Also pinging @rmaz who made a related change , is this used by Facebook? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130710/new/ https://reviews.llvm.or

[PATCH] D130710: [ASTWriter] Provide capability to output a PCM/PCH file that does not write out information about its output path

akyrtzi added a comment. In D130710#3685509 , @v.g.vassilev wrote: > In D130710#3685470 , @akyrtzi wrote: > >> @v.g.vassilev is the functionality of "write `ORIGINAL_PCH_DIR` and resolve >> headers relative to i

[PATCH] D130710: [ASTWriter] Provide capability to output a PCM/PCH file that does not write out information about its output path

akyrtzi updated this revision to Diff 448715. akyrtzi added a comment. Add `FIXME` comment to consider either removing `ORIGINAL_PCH_DIR` or changing the default. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130710/new/ https://reviews.llvm.org/D

[PATCH] D130710: [ASTWriter] Provide capability to output a PCM/PCH file that does not write out information about its output path

akyrtzi added a comment. In D130710#3685436 , @benlangmuir wrote: > Is the functionality provided by `ORIGINAL_PCH_DIR` still useful for making > it possible to move a PCH and its referenced headers? It's not completely > clear to me when this feature

[PATCH] D130710: [ASTWriter] Provide capability to output a PCM/PCH file that does not write out information about its output path

This revision was automatically updated to reflect the committed changes. Closed by commit rG944a86de7c50: [ASTWriter] Provide capability to output a PCM/PCH file that does not write out… (authored by akyrtzi). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D131124: [Serialization] Remove `ORIGINAL_PCH_DIR` record

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. Use of `ORIGINAL_PCH_DIR` record has been superseeded by making PCH/PCM files with relocatable paths at write time. Removin

[PATCH] D131124: [Serialization] Remove `ORIGINAL_PCH_DIR` record

This revision was automatically updated to reflect the committed changes. Closed by commit rG6635f48e4aba: [Serialization] Remove `ORIGINAL_PCH_DIR` record (authored by akyrtzi). Changed prior to commit: https://reviews.llvm.org/D131124?vs=449808&id=450439#toc Repository: rG LLVM Github Mono

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

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] D124558: [Tooling/DependencyScanning] Make skipping excluded PP ranges during dependency scanning the default

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. This is to improve maintenance a bit and remove need to maintain the additional option and related code-paths. Repository

[PATCH] D124558: [Tooling/DependencyScanning] Make skipping excluded PP ranges during dependency scanning the default

akyrtzi updated this revision to Diff 425833. akyrtzi added a comment. Change APIs to accept a reference of `ExcludedPreprocessorDirectiveSkipMapping` instead of a pointer, since it is required now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124

[PATCH] D124558: [Tooling/DependencyScanning] Make skipping excluded PP ranges during dependency scanning the default

akyrtzi added a comment. @jansvoboda11 thanks for reviewing! I've changed APIs to use a reference instead of a pointer and removed the unnecessary check and heap allocations. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124558/new/ https://review

[PATCH] D124558: [Tooling/DependencyScanning] Make skipping excluded PP ranges during dependency scanning the default

This revision was automatically updated to reflect the committed changes. Closed by commit rG42823beb1d71: [Tooling/DependencyScanning] Make skipping excluded PP ranges during dependency… (authored by akyrtzi). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

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

akyrtzi added inline comments. Comment at: clang/include/clang/Lex/PreprocessorOptions.h:214 + FileEntryRef)> + DependencyDirectivesForFile; jansvoboda11 wrote: > To be honest, I'm not a fan of using `PreprocessorOptions` to carry state > between com

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

akyrtzi added a comment. In D124687#3485710 , @tschuett wrote: > Could you split this into smaller patches? I'll split up the renames to a separate patch so that it is easier to see the code that affects functionality. Not sure if it can be broken furth

[PATCH] D124715: Use QoS class Utility for ThreadPriority::Low on Mac

akyrtzi added a comment. My recommendation is that indexing belongs to 'utility', as @stefanhaller mentioned the user is actively depending on functionality coming from the index. That said, you may want to consider dynamically switching to background if running on laptop with battery, or other

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

akyrtzi added a comment. In D124687#3486473 , @akyrtzi wrote: > In D124687#3485710 , @tschuett > wrote: > >> Is there overhead in the non dependency scanning mode? > > Good suggestion, I'll do some measurements a

  1   2   3   >