[PATCH] D94952: [clangd] Take into account what is in the index (symbols, references, etc.) at indexes merge

ArcsinX added inline comments. Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:433 PreambleSymbols.update( -Uri, std::make_unique(std::move(*IF->Symbols)), +FilePath ? *FilePath : (consumeError(FilePath.takeError()), Uri), +std::make_unique(s

[PATCH] D95349: [clangd] Allow diagnostics to be suppressed with configuration

ArcsinX added inline comments. Comment at: clang-tools-extra/clangd/ParsedAST.cpp:319 -if (!CTChecks.empty()) { - ASTDiags.setLevelAdjuster([&CTContext](DiagnosticsEngine::Level DiagLevel, - const clang::Diagnostic &Info) {

[PATCH] D94952: [clangd] Take into account what is in the index (symbols, references, etc.) at indexes merge

ArcsinX updated this revision to Diff 319602. ArcsinX added a comment. - Set `IdxContents` at `FileSymbols` object breation instead of at `FileSymbols::buildIndex()` call. - Revert change of the preamble index key scheme - Add comment for `IndexContents` - `IndexDataKind` => `IndexContents` Rep

[PATCH] D94952: [clangd] Take into account what is in the index (symbols, references, etc.) at indexes merge

ArcsinX marked 4 inline comments as done. ArcsinX added inline comments. Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:433 PreambleSymbols.update( -Uri, std::make_unique(std::move(*IF->Symbols)), +FilePath ? *FilePath : (consumeError(FilePath.takeE

[PATCH] D94952: [clangd] Take into account what is in the index (symbols, references, etc.) at indexes merge

ArcsinX added inline comments. Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:433 PreambleSymbols.update( -Uri, std::make_unique(std::move(*IF->Symbols)), +FilePath ? *FilePath : (consumeError(FilePath.takeError()), Uri), +std::make_unique(s

[PATCH] D94952: [clangd] Take into account what is in the index (symbols, references, etc.) at indexes merge

ArcsinX updated this revision to Diff 321442. ArcsinX added a comment. - Remove default value of `IdxContents` in `FileSymbols` constructor. - Fix index contents for the preamble index (Symbols => Symbols|Relations). - Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D94952: [clangd] Take into account what is in the index (symbols, references, etc.) at indexes merge

ArcsinX marked 3 inline comments as done. ArcsinX added a comment. Thank you for review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94952/new/ https://reviews.llvm.org/D94952 ___ cfe-commits mailing l

[PATCH] D94952: [clangd] Take into account what is in the index (symbols, references, etc.) at indexes merge

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG91698fe45f60: [clangd] Take into account what is in the index (symbols, references, etc.) at… (authored by ArcsinX). Repository: rG LLVM Github Mo

[PATCH] D104056: [clangd][nfc] Show more information in logs when compiler instance prepare fails

ArcsinX created this revision. ArcsinX added reviewers: sammccall, kadircet. Herald added subscribers: usaxena95, arphaman. ArcsinX requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Without this patch clangd

[PATCH] D107304: [clangd][query-driver] Extract GCC version from the driver output

ArcsinX created this revision. ArcsinX added reviewers: sammccall, kadircet. Herald added subscribers: usaxena95, arphaman. ArcsinX requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Clang uses 4.2.1 as a def

[PATCH] D107304: [clangd][query-driver] Extract GCC version from the driver output

ArcsinX added a comment. In D107304#2920960 , @joerg wrote: > What do you mean with "GCC macros are correct"? Imagine that we have a project and we build it with GCC, we have `compile_commands.json` file which contains all compile command used to build

[PATCH] D107304: [clangd][query-driver] Extract GCC version from the driver output

ArcsinX added a comment. In D107304#2921588 , @kadircet wrote: > I am not sure this is a good idea. Surely we can go with this approach and > pretend to have the same GNUC version as the toolchain, but we are still > using clang underneath as @joerg poi

[PATCH] D107304: [clangd][query-driver] Extract GCC version from the driver output

ArcsinX added a comment. In D107304#2921694 , @kadircet wrote: >> The most confusing thing for me that we already have this implicitly set >> -fgnuc-version=4.2.1. So, we already have implicitly set __GNUC__ and other >> GCC macros as a default behavior

[PATCH] D107304: [clangd][query-driver] Extract GCC version from the driver output

ArcsinX added a comment. @kadircet Thanks for clarification. Now your position is clear for me. But clang was designed as a drop-in replacement for GCC: - https://clang.llvm.org/docs/LanguageExtensions.html#introduction - https://clang.llvm.org/features.html#gcccompat I do not have any proves t

[PATCH] D107304: [clangd][query-driver] Extract GCC version from the driver output

ArcsinX added a comment. In D107304#2922975 , @kadircet wrote: > But I am not convinced that changing the default behaviour here is going to > be beneficial for majority of the users. I think that majority of users doesn't use `--query-driver` at all.

[PATCH] D107304: [clangd][query-driver] Extract GCC version from the driver output

ArcsinX added a comment. In D107304#2924886 , @kadircet wrote: > Ok if you think that setting GCC version to higher values by default won't > break anything, I think that GCC version should be the same, which was used to build the project: it can be hi

[PATCH] D107304: [clangd][query-driver] Extract GCC version from the driver output

ArcsinX added a comment. In D107304#2926310 , @sammccall wrote: > Clang's current answer is "yes we are GCC >=4, but no we are not GCC >=5". > This causes the codebase to reject the compiler, because it relies on > features/absence of bugs from GCC 5. @A

[PATCH] D107304: [clangd][query-driver] Extract GCC version from the driver output

ArcsinX abandoned this revision. ArcsinX added a comment. As I can see, no chances for this to get approved. So. closing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107304/new/ https://reviews.llvm.org/D107304 __

[PATCH] D104056: [clangd][nfc] Show more information in logs when compiler instance prepare fails

ArcsinX updated this revision to Diff 355389. ArcsinX added a comment. Check ASTDiags.take() for empty instead of ASTDiags.getNumErrors() value check Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104056/new/ https://reviews.llvm.org/D104056 Files:

[PATCH] D104056: [clangd][nfc] Show more information in logs when compiler instance prepare fails

This revision was automatically updated to reflect the committed changes. Closed by commit rGa62579fc008e: [clangd][nfc] Show more information in logs when compiler instance prepare fails (authored by ArcsinX). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D97535: [clangd] Use URIs instead of paths in the index file list

ArcsinX created this revision. Herald added subscribers: usaxena95, kadircet, arphaman. ArcsinX requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang. Without this patch the file list of the index is derived from the keys, b

[PATCH] D97535: [clangd] Use URIs instead of paths in the index file list

ArcsinX added reviewers: sammccall, kadircet, hokein. ArcsinX added a comment. Initial discussion D94952#inline-892421 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97535/new/ https://reviews.llvm.or

[PATCH] D97535: [clangd] Use URIs instead of paths in the index file list

ArcsinX updated this revision to Diff 326903. ArcsinX added a comment. - Do not insert the keys into the file list - Iterate through refs and symbols to create the file list after releasing the lock Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D975

[PATCH] D97535: [clangd] Use URIs instead of paths in the index file list

ArcsinX marked an inline comment as done. ArcsinX added inline comments. Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:279 SymbolSlabs.push_back(FileAndSymbols.second); + for (const auto &S : *FileAndSymbols.second) { +if (S.Definition)

[PATCH] D97535: [clangd] Use URIs instead of paths in the index file list

ArcsinX added inline comments. Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:292 +for (const auto &Sym : *Slab) { + if (Sym.Definition) +Files.insert(Sym.Definition.FileURI); kadircet wrote: > it feels weird to choose one or the other

[PATCH] D97535: [clangd] Use URIs instead of paths in the index file list

ArcsinX added inline comments. Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:292 +for (const auto &Sym : *Slab) { + if (Sym.Definition) +Files.insert(Sym.Definition.FileURI); kadircet wrote: > ArcsinX wrote: > > kadircet wrote: > > > i

[PATCH] D97535: [clangd] Use URIs instead of paths in the index file list

ArcsinX planned changes to this revision. ArcsinX added a comment. In D97535#2596992 , @kadircet wrote: > But until that day, I suppose the best we can do is change the contract of > `FileSymbols` to sey keys are always `URIs representing the file produci

[PATCH] D97535: [clangd] Use URIs instead of paths in the index file list

ArcsinX updated this revision to Diff 327853. ArcsinX added a comment. - use URIs as keys - create the file list from the keys Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97535/new/ https://reviews.llvm.org/D97535 Files: clang-tools-extra/clan

[PATCH] D97535: [clangd] Use URIs instead of paths in the index file list

ArcsinX updated this revision to Diff 328222. ArcsinX added a comment. Call `FileIndex::updateMain()` with an absolute file path in tests to avoid test failures in debug mode. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97535/new/ https://review

[PATCH] D97535: [clangd] Use URIs instead of paths in the index file list

ArcsinX added a comment. Thank you for review! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97535/new/ https://reviews.llvm.org/D97535 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lis

[PATCH] D97535: [clangd] Use URIs instead of paths in the index file list

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc4efd04f18c7: [clangd] Use URIs instead of paths in the index file list (authored by ArcsinX). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D109239: Add support for floating-option `-ffp-eval-method` and for new `pragma clang fp eval-method`

ArcsinX added a comment. It seems for x86_32 case (e.g. with -m32 in command line) there is a difference between clang and GCC: clang: `__FLT_EVAL_METHOD__` == 0 GCC: `__FLT_EVAL_METHOD__` == 2 Example: https://godbolt.org/z/EbY8rPYGn I am not sure, is it a correct behavior from clang? CHANGE

[PATCH] D131706: [clangd][unittests][IncludeCleaner] Don't call findReferencedFiles() if the result is not used

ArcsinX created this revision. ArcsinX added reviewers: kbobyrev, sammccall, kadircet. Herald added subscribers: usaxena95, arphaman. Herald added a project: All. ArcsinX requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-

[PATCH] D131706: [clangd][unittests][IncludeCleaner] Don't call findReferencedFiles() if the result is not used

This revision was automatically updated to reflect the committed changes. Closed by commit rG42ee0d8c16f7: [clangd][unittests][IncludeCleaner] Don't call findReferencedFiles() if the… (authored by ArcsinX). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D131091: [clang][index] Index unresolved member expression as reference

This revision was automatically updated to reflect the committed changes. Closed by commit rGee648c0ce09b: [clang][index] Index unresolved member expression as reference (authored by denis-fatkulin, committed by ArcsinX). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D134379: [clangd] IncludeCleaner: handle using namespace

ArcsinX created this revision. ArcsinX added reviewers: sammccall, kbobyrev. Herald added subscribers: kadircet, arphaman. Herald added a project: All. ArcsinX requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra

[PATCH] D134379: [clangd] IncludeCleaner: handle using namespace

ArcsinX added a comment. In D134379#3806664 , @sammccall wrote: > Is test.h meaningfully used in that example? > Yes, the code is going to fail to compile without it, but it seems like the > "spirit" of IWYU would say to delete both the include and the u

[PATCH] D134379: [clangd] IncludeCleaner: handle using namespace

ArcsinX added a comment. Anyway if this is the only concern, we can handle namespace declaration as a special case inside `ReferencedLocationCrawler::add()`. something like this: diff -for (const Decl *Redecl : D->redecls()) - Result.User.insert(Redecl->getLocation()); +if (

[PATCH] D134379: [clangd] IncludeCleaner: handle using namespace

ArcsinX added a comment. In D134379#3808043 , @kadircet wrote: > In D134379#3807770 , @ArcsinX wrote: > >> Anyway if this is the only concern, we can handle namespace declaration as a >> special case inside `Refe

[PATCH] D135362: [clang] Make variables of undeduced types to have dependent alignment

ArcsinX created this revision. ArcsinX added reviewers: aaron.ballman, rsmith, mizvekov. Herald added a project: All. ArcsinX requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Without this patch `VarDecl::hasDependent()` checks only undeduced

[PATCH] D135362: [clang] Make variables of undeduced types to have dependent alignment

This revision was automatically updated to reflect the committed changes. Closed by commit rG5a42c90b778c: [clang] Make variables of undeduced types to have dependent alignment (authored by ArcsinX). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1353

[PATCH] D138546: Clangd: Preserve target flags in system includes extractor

ArcsinX added a comment. It seems this patch can break GCC toolchains support This patch expects that we never get target-related options in `CommandLine` passed to `extractSystemIncludesAndTarget()` for **GCC** toolchain. But seems this is not always true because of https://github.com/llvm/ll

[PATCH] D116966: [analyzer] Don't specify PLUGIN_TOOL for analyzer plugins

ArcsinX added a comment. Friendly ping. I am not sure that this patch is clear, so I will try to explain it again: - clang static analyzer plugins are *NOT* a typical clang plugins: instead of `llvm::Registry<>::Add` usage, these plugins provides `сlang_registerCheckers` and `clang_analyzerAPI

[PATCH] D116966: [analyzer] Don't specify PLUGIN_TOOL for analyzer plugins

ArcsinX added a comment. In D116966#3261168 , @aaron.ballman wrote: > However, the changes you've made don't look to be specific to building on > Windows; this removes `PLUGIN_TOOL` for all targets. I presume it's still > needed for non-Windows targets

[PATCH] D116966: [analyzer] Don't specify PLUGIN_TOOL for analyzer plugins

ArcsinX added a comment. In D116966#3261479 , @beanz wrote: > Have you looked at the impact on binary size? PLUGIN_TOOL _should_ cause the > plugins to link against the copy of LLVM & Clang in the tool target binary > which reduces the binary size of th

[PATCH] D116966: [analyzer] Don't specify PLUGIN_TOOL for analyzer plugins

This revision was automatically updated to reflect the committed changes. Closed by commit rG491c154677bc: [analyzer] Don't specify PLUGIN_TOOL for analyzer plugins (authored by ArcsinX). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116966/new/ htt

[PATCH] D121245: [clang][parser] Allow GNU attributes before namespace identifier

ArcsinX created this revision. ArcsinX added reviewers: aaron.ballman, rsmith. Herald added a subscriber: jdoerfert. Herald added a project: All. ArcsinX requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. GCC supports: - `namespace identifie

[PATCH] D116966: [analyzer] Don't specify PLUGIN_TOOL for analyzer plugins

ArcsinX created this revision. ArcsinX added reviewers: aaronpuchert, hintonda, NoQ, Szelethus, lebedev.ri. Herald added subscribers: manas, wenlei, steakhal, ASDenysPetrov, usaxena95, dkrupp, donat.nagy, kadircet, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun, mgorny. ArcsinX

[PATCH] D116966: [analyzer] Don't specify PLUGIN_TOOL for analyzer plugins

ArcsinX added a comment. In D116966#3242170 , @aaronpuchert wrote: > This is only observable on Windows, right? Thanks for your reply. Yes, PLUGIN_TOOL argument is used on Windows only, so this problems can't be observed on non-Windows systems and this

[PATCH] D116966: [analyzer] Don't specify PLUGIN_TOOL for analyzer plugins

ArcsinX added a comment. In D116966#3243318 , @aaron.ballman wrote: > AFAIK, we don't support plugins on Windows. See: > https://reviews.llvm.org/D16761#1441359 I don't think anything has changed in > this regard. I can say that I successfully use cla

[PATCH] D116966: [analyzer] Don't specify PLUGIN_TOOL for analyzer plugins

ArcsinX added a comment. In D116966#3243374 , @aaron.ballman wrote: > To be clear, I'm trying to figure out whether plugins are *actually* > supported on Windows or whether they just so happen to work if the stars line > up right for you. If they're su

[PATCH] D115959: [clangd] Fix undefined behavior when generating error message at rename with an invalid name

ArcsinX created this revision. ArcsinX added reviewers: sammccall, kadircet, hokein. Herald added subscribers: usaxena95, arphaman. ArcsinX requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. `Message()` lambd

[PATCH] D115959: [clangd] Fix undefined behavior when generating error message at rename with an invalid name

This revision was automatically updated to reflect the committed changes. Closed by commit rG555eacf75f21: [clangd] Fix undefined behavior when generating error message at rename with an… (authored by ArcsinX). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D93796: [clangd][fuzzyFind] Do not show stale symbols in the result.

This revision was automatically updated to reflect the committed changes. Closed by commit rG979228f120f4: [clangd][fuzzyFind] Do not show stale symbols in the result. (authored by ArcsinX). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93796/new/ h

[PATCH] D94477: [clangd] Add main file macros into the main-file index.

ArcsinX created this revision. Herald added subscribers: usaxena95, kadircet, arphaman. ArcsinX requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang. This patch is a try to fix `WorkspaceSymbols.Macros` test after D93796 <

[PATCH] D94477: [clangd] Add main file macros into the main-file index.

ArcsinX added reviewers: sammccall, kadircet. ArcsinX added a comment. I am not happy with my solution, hope to hear an advice. Initial discussion about this problem: https://reviews.llvm.org/D93683#2468842 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D94477: [clangd] Add main file macros into the main-file index.

ArcsinX updated this revision to Diff 316331. ArcsinX added a comment. - std::pair => struct MacroOccurrence - remove addressed fixme comment - assert() => cantFail() - use toSourceCode() to get the macro name Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D94477: [clangd] Add main file macros into the main-file index.

This revision was automatically updated to reflect the committed changes. Closed by commit rG2e25be0b6134: [clangd] Add main file macros into the main-file index. (authored by ArcsinX). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94477/new/ https:

[PATCH] D94952: [clangd] Take into account what is in the index (symbols, references, etc.) at indexes merge

ArcsinX created this revision. Herald added subscribers: usaxena95, kadircet, arphaman. ArcsinX requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang. Current indexes merge logic skip data from the static index if the file i

[PATCH] D94952: [clangd] Take into account what is in the index (symbols, references, etc.) at indexes merge

ArcsinX added reviewers: sammccall, kadircet. ArcsinX added a comment. Initial discussion https://reviews.llvm.org/D93683?id=313280#inline-875666 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94952/new/ https://reviews.llvm.org/D94952

[PATCH] D94952: [clangd] Take into account what is in the index (symbols, references, etc.) at indexes merge

ArcsinX updated this revision to Diff 317521. ArcsinX added a comment. Prevent crash in debug mode. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94952/new/ https://reviews.llvm.org/D94952 Files: clang-tools-extra/clangd/index/BackgroundRebuild.

<    1   2   3