[PATCH] D83508: [clangd][Hover] Don't use Decl if it is not related with tokens under cursor.

ArcsinX added a comment. In D83508#2143625 , @sammccall wrote: > Your test cases show two problems with this strategy: > > - we ignore comments and semicolons, but not preprocessor directives (or > disabled preprocessor regions). I think we can fix this b

[PATCH] D83508: [clangd][Hover] Don't use Decl if it is not related with tokens under cursor.

ArcsinX added a comment. In D83508#2155322 , @sammccall wrote: > In D83508#2155174 , @ArcsinX wrote: > > > In D83508#2143625 , @sammccall > > wrote: > > > > > Your test case

[PATCH] D83508: [clangd][Hover] Don't use Decl if it is not related with tokens under cursor.

ArcsinX updated this revision to Diff 278440. ArcsinX added a comment. Prevent tokens selection in disabled preprocessor sections. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83508/new/ https://reviews.llvm.org/D83508 Files: clang-tools-extra/

[PATCH] D83508: [clangd][Hover] Don't use Decl if it is not related with tokens under cursor.

ArcsinX added inline comments. Comment at: clang-tools-extra/clangd/Selection.cpp:227 continue; + // Ignore tokens in disabled preprocessor sections. + if (Buf.expandedTokens(SM.getMacroArgExpandedLocation(T->location())) sammccall wrote: > I t

[PATCH] D84009: [Syntax] expose API for expansions overlapping a spelled token range.

ArcsinX added inline comments. Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:252 return {}; - assert(Spelled.front().location().isFileID()); - - auto FID = sourceManager().getFileID(Spelled.front().location()); - auto It = Files.find(FID); - assert(It != Files.end());

[PATCH] D84009: [Syntax] expose API for expansions overlapping a spelled token range.

ArcsinX added a comment. Thanks. This LGTM, but I think I don't have enough experience to accept revisions and could miss something important. So, may be @kadircet could accept it if its OK for him. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84

[PATCH] D83508: [clangd][Hover] Don't use Decl if it is not related with tokens under cursor.

ArcsinX added a comment. In D83508#2157859 , @sammccall wrote: > Tried this out in D84012 /D84009 > . Works pretty well, and I think the API is > a useful and natural addition to TokenBuff

[PATCH] D84144: [clangd] Remove TokenBuffer usage in TypeHierarchy

ArcsinX created this revision. Herald added subscribers: cfe-commits, kbobyrev, usaxena95, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. This patch mostly reverts D74850 . We could not use `AST.getTokens()` here, because it do

[PATCH] D84144: [clangd] Remove TokenBuffer usage in TypeHierarchy

ArcsinX updated this revision to Diff 279168. ArcsinX added a comment. NameRange => DeclRange Simplify TypeHierarchy.Preamble test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84144/new/ https://reviews.llvm.org/D84144 Files: clang-tools-extra

[PATCH] D84144: [clangd] Remove TokenBuffer usage in TypeHierarchy

ArcsinX marked 2 inline comments as done. ArcsinX added inline comments. Comment at: clang-tools-extra/clangd/XRefs.cpp:1191 + SourceLocation EndLoc = SM.getSpellingLoc(SM.getFileLoc(ND.getEndLoc())); + const auto NameRange = + toHalfOpenFileRange(SM, Ctx.getLangOpts(), {B

[PATCH] D84144: [clangd] Remove TokenBuffer usage in TypeHierarchy

ArcsinX updated this revision to Diff 279174. ArcsinX added a comment. Check source range for Parent Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84144/new/ https://reviews.llvm.org/D84144 Files: clang-tools-extra/clangd/XRefs.cpp clang-tools

[PATCH] D84144: [clangd] Remove TokenBuffer usage in TypeHierarchy

ArcsinX marked 2 inline comments as done. ArcsinX added inline comments. Comment at: clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp:533 + + Annotations HeaderInPreambleAnnotations(R"cpp( +struct Parent { kadircet wrote: > this doesn't need to be an `A

[PATCH] D84144: [clangd] Remove TokenBuffer usage in TypeHierarchy

ArcsinX added a comment. In D84144#2161579 , @kadircet wrote: > also please let me know if I should land this for you. Yes, could you please land this for me? I do not have commit access. Aleksandr Platonov Repository: rG LLVM Github Monorepo CHANG

[PATCH] D84144: [clangd] Remove TokenBuffer usage in TypeHierarchy

ArcsinX updated this revision to Diff 279180. ArcsinX added a comment. AdditionalFiles["header_in_preamble.h"] => HeaderCode Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84144/new/ https://reviews.llvm.org/D84144 Files: clang-tools-extra/clangd

[PATCH] D84144: [clangd] Remove TokenBuffer usage in TypeHierarchy

ArcsinX marked an inline comment as done. ArcsinX added inline comments. Comment at: clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp:533 + + Annotations HeaderInPreambleAnnotations(R"cpp( +struct Parent { kadircet wrote: > ArcsinX wrote: > > kadircet w

[PATCH] D83759: [clangd] Port lit tests to Windows

ArcsinX added a comment. What do you think of this patch? I'm not sure if Windows is important OS for developers, and that some of these changes might be awkward to maintain, but maybe we can enable `background-index.test` at least? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D83759: [clangd] Port lit tests to Windows

ArcsinX marked an inline comment as done. ArcsinX added inline comments. Comment at: clang-tools-extra/clangd/test/background-index.test:18 # Test that the index is writing files in the expected location. # RUN: ls %t/.cache/clangd/index/foo.cpp.*.idx # RUN: ls %t/sub_dir/.cac

[PATCH] D83759: [clangd] Port lit tests to Windows

ArcsinX updated this revision to Diff 279482. ArcsinX added a comment. Revert tests split; Replace '\' with '/' in background-index.test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83759/new/ https://reviews.llvm.org/D83759 Files: clang-tools

[PATCH] D83759: [clangd] Fixes in lit tests

ArcsinX added a comment. In D83759#2164296 , @kadircet wrote: > as i mentioned please watch out for buildbot breakages after landing this. > (and again let me know if I should land this for you) Thanks for review. Yes, could you please land this for me

[PATCH] D83759: [clangd] Fixes in lit tests

This revision was automatically updated to reflect the committed changes. Closed by commit rGff63d6be93dc: [clangd] Fixes in lit tests (authored by ArcsinX). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83759/new/ https://reviews.llvm.org/D83759 F

[PATCH] D83759: [clangd] Fixes in lit tests

ArcsinX added a comment. In D83759#2165116 , @thakis wrote: > Looks like this breaks tests on macOS (and probably with non-gnu greps): > http://45.33.8.238/mac/17611/step_9.txt > > Please take a look, and revert while you investigate if it takes a while t

[Differential] D83759: [clangd] Port lit tests to Windows

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rGff63d6be93dc: [clangd] Fixes in lit tests (authored by ArcsinX). Changed prior to commit: https://reviews.llvm.org/D837

[PATCH] D83759: [clangd] Fixes in lit tests

ArcsinX added a comment. Seem the problem is in `-i` option. According with OSX man: `sed [-Ealn] command [file ... ]` `sed [-Ealn] [-e command] [-f command_file] [-i extension] [file ...]`. Seems on macOS `-E` is treated as an argument for `-i` in command like `sed -i -E -e ...` . Also, seems

[PATCH] D83759: [clangd] Fixes in lit tests

ArcsinX updated this revision to Diff 279754. ArcsinX added a comment. [macOS] Fix `background-index.test` failure, don't create `*-e` files during `background-index.test` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83759/new/ https://reviews.ll

[PATCH] D83759: [clangd] Fixes in lit tests

ArcsinX added a comment. I believe it's okay now on macOS. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83759/new/ https://reviews.llvm.org/D83759 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D83759: [clangd] Fixes in lit tests

ArcsinX marked an inline comment as done. ArcsinX added inline comments. Comment at: clang-tools-extra/clangd/test/background-index.test:6 +# RUN: sed -e "s|DIRECTORY|%/t|" %/t/definition.jsonrpc > %/t/definition.jsonrpc.1 +# RUN: sed -i.bak -e "s|DIRECTORY|%/t|" %/t/compile_com

[PATCH] D83759: [clangd] Fixes in lit tests

ArcsinX updated this revision to Diff 279781. ArcsinX added a comment. Get rid of `-i` `sed` option Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83759/new/ https://reviews.llvm.org/D83759 Files: clang-tools-extra/clangd/test/Inputs/background-i

[PATCH] D83759: [clangd] Fixes in lit tests

ArcsinX marked an inline comment as done. ArcsinX added inline comments. Comment at: clang-tools-extra/clangd/test/background-index.test:6 +# RUN: sed -e "s|DIRECTORY|%/t|" %/t/definition.jsonrpc > %/t/definition.jsonrpc.1 +# RUN: sed -i.bak -e "s|DIRECTORY|%/t|" %/t/compile_com

[PATCH] D83759: [clangd] Fixes in lit tests

This revision was automatically updated to reflect the committed changes. Closed by commit rG98b56c09be00: [clangd] Fixes in lit tests (authored by ArcsinX). Changed prior to commit: https://reviews.llvm.org/D83759?vs=279781&id=279789#toc Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D84513: [clangd] Collect references for externally visible main-file symbols

ArcsinX created this revision. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. Without this patch clangd does not collect references for main-file symbols if there is no public declaration in preamble. Example:

[PATCH] D84513: [clangd] Collect references for externally visible main-file symbols

ArcsinX updated this revision to Diff 280713. ArcsinX added a comment. Remove version bump Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84513/new/ https://reviews.llvm.org/D84513 Files: clang-tools-extra/clangd/index/SymbolCollector.cpp clang

[PATCH] D84513: [clangd] Collect references for externally visible main-file symbols

ArcsinX added a comment. In D84513#2172028 , @kadircet wrote: > thanks, this looks good in general, but it would be nice to make sure we are > not blowing the index memory and disk usage. > > could you grab some numbers for these two before/after this pat

[PATCH] D84513: [clangd] Collect references for externally visible main-file symbols

This revision was automatically updated to reflect the committed changes. Closed by commit rG90684d154516: [clangd] Collect references for externally visible main-file symbols (authored by ArcsinX). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84513

[PATCH] D84912: [clangd] findNearbyIdentifier(): fix the word search in the token stream.

ArcsinX created this revision. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous. Herald added a project: clang. ArcsinX requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. Without this patch the word occurrence search always returns the

[PATCH] D84912: [clangd] findNearbyIdentifier(): fix the word search in the token stream.

ArcsinX added a comment. In D84912#2184147 , @kadircet wrote: > oh and also, how did you notice the discrepancy? was it a random encounter > while reading the code or did you notice a misbehaviour? Case was like this void f1(int [[paramValue]]){ }

[PATCH] D84912: [clangd] findNearbyIdentifier(): fix the word search in the token stream.

This revision was automatically updated to reflect the committed changes. Closed by commit rG05b173466142: [clangd] findNearbyIdentifier(): fix the word search in the token stream. (authored by ArcsinX). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D84912: [clangd] findNearbyIdentifier(): fix the word search in the token stream.

ArcsinX added a comment. In D84912#2184144 , @kadircet wrote: > I think we should also cherry-pick this into release branch. I've filed > https://bugs.llvm.org/show_bug.cgi?id=46905 for cherry-picking please update > the bug > with the commit hash once y

[PATCH] D82805: [clang] Fix a crash on passing C structure of incompatible type to function with reference parameter

ArcsinX marked 3 inline comments as done. ArcsinX added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:4539 +if (CXXRecordDecl *T2RecordDecl = +dyn_cast(T2RecordType->getDecl())) { + const auto &Conversions = T2RecordDecl->getVisibleConversionFunctions(

[PATCH] D82805: [clang] Fix a crash on passing C structure of incompatible type to function with reference parameter

ArcsinX updated this revision to Diff 274789. ArcsinX added a comment. Try conversion function only in C++. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82805/new/ https://reviews.llvm.org/D82805 Files: clang/lib/Sema/SemaInit.cpp clang/test/

[PATCH] D82805: [clang] Fix a crash on passing C structure of incompatible type to function with reference parameter

ArcsinX added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82805/new/ https://reviews.llvm.org/D82805 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi

[PATCH] D82805: [clang] Fix a crash on passing C structure of incompatible type to function with reference parameter

ArcsinX added a comment. In D82805#2139243 , @riccibruno wrote: > Do you want me to commit it for you? Yes, thank you. > If so I need a name and an email for the attribution. Aleksandr Platonov Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D83508: [clangd][Hover] Don't use Decl if it is not related with tokens under cursor.

ArcsinX created this revision. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. This patch fixes redundant hover with information about Decl which is not under cursor. Repository: rG LLVM Github Monorepo ht

[PATCH] D83508: [clangd][Hover] Don't use Decl if it is not related with tokens under cursor.

ArcsinX added a comment. @kadircet Thanks for your reply. Think that you are right, because the same problem appears in GTD (with the same test cases). And as for GTD the similar workaround also does not break any regression tests except override/final. Repository: rG LLVM Github Monorepo

[PATCH] D83548: [clangd] Fix tests build for GCC5

ArcsinX created this revision. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. Build log: llvm-project/clang-tools-extra/clangd/unittests/PreambleTests.cpp: In member function ‘virtual void clang::clangd::{

[PATCH] D83548: [clangd] Fix tests build for GCC5

ArcsinX added a comment. As far as I do not have commit access, could you please commit for me? Aleksandr Platonov Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83548/new/ https://reviews.llvm.org/D83548

[PATCH] D83621: [clang][Tooling] Try to avoid file system access if there is no record for the file in compile_commads.json

ArcsinX created this revision. Herald added subscribers: cfe-commits, usaxena95, kadircet, ilya-biryukov. Herald added a project: clang. If there is no record in compile_commands.json, we try to find suitable record with `MatchTrie.findEquivalent()` call. This is very expensive operation with a l

[PATCH] D83621: [clang][Tooling] Try to avoid file system access if there is no record for the file in compile_commads.json

ArcsinX added a comment. Also I think that `FileMatchTrie` introduced in https://reviews.llvm.org/D30 is not efficient solution to fight with symlinks and for most cases `InterpolatingCompilationDatabase` will be accurate enough. But I am not sure, is it safe to completely remove `FileMatchTrie

[PATCH] D83621: [clang][Tooling] Try to avoid file system access if there is no record for the file in compile_commads.json

ArcsinX added a comment. In D83621#2146706 , @sammccall wrote: > Wow, yeah, this seems pretty bad! I'm not very familiar with this code. I'm > curious, for "it tooks more than 1 second" - what OS is this on? Windows. I faced this problem on a huge proje

[PATCH] D83621: [clang][Tooling] Try to avoid file system access if there is no record for the file in compile_commads.json

ArcsinX added a comment. In D83621#2146746 , @klimek wrote: > IIRC the symlink checking was there because some part of the system on linux > tends to give us realpaths (with CMake builds), and that leads to not finding > anything if there are symlinks in

[PATCH] D83759: [clangd] Port lit tests to Windows

ArcsinX created this revision. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, MaskRay, ilya-biryukov. Herald added a project: clang. Changes: - `background-index.test` Add Windows support. - `dependency-output.test` Split into two tests (Windows and Posix). - `did

[PATCH] D83621: [clang][Tooling] Try to avoid file system access if there is no record for the file in compile_commads.json

ArcsinX added a comment. In D83621#2151716 , @sammccall wrote: > In D83621#2146750 , @ArcsinX wrote: > > > > - don't scan for equivalences if the set of candidates exceeds some size > > > threshold (10 or so) > > >

[PATCH] D83621: [clang][Tooling] Try to avoid file system access if there is no record for the file in compile_commads.json

ArcsinX updated this revision to Diff 278161. ArcsinX added a comment. Support only directory simlinks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83621/new/ https://reviews.llvm.org/D83621 Files: clang/lib/Tooling/FileMatchTrie.cpp clang/u

[PATCH] D83621: [clang][Tooling] Try to avoid file system access if there is no record for the file in compile_commads.json

ArcsinX updated this revision to Diff 278230. ArcsinX added a comment. Update comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83621/new/ https://reviews.llvm.org/D83621 Files: clang/lib/Tooling/FileMatchTrie.cpp clang/unittests/Tooling

[PATCH] D83621: [clang][Tooling] Try to avoid file system access if there is no record for the file in compile_commads.json

ArcsinX marked 3 inline comments as done. ArcsinX added a comment. As far as I do not have commit access, could you please commit for me? Aleksandr Platonov Comment at: clang/lib/Tooling/FileMatchTrie.cpp:111 + // As far as we do not support file symlinks we compare +

[PATCH] D82805: [clang] Fix a crash on passing C structure of incompatible type to function with reference parameter

ArcsinX created this revision. ArcsinX added a reviewer: rsmith. ArcsinX added a project: clang. Herald added a subscriber: cfe-commits. ArcsinX edited the summary of this revision. `__builtin_va_*()` and `__builtin_ms_va_*()` declared as functions with reference parameters. This patch fix clang

[PATCH] D82805: [clang] Fix a crash on passing C structure of incompatible type to function with reference parameter

ArcsinX updated this revision to Diff 274250. ArcsinX added a comment. Fix format CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82805/new/ https://reviews.llvm.org/D82805 Files: clang/lib/Sema/SemaInit.cpp clang/test/Sema/init-ref-c.c Index: clang/test/Sema/init-ref-c.c =

[PATCH] D82805: [clang] Fix a crash on passing C structure of incompatible type to function with reference parameter

ArcsinX marked an inline comment as done. ArcsinX added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:4539 +if (CXXRecordDecl *T2RecordDecl = +dyn_cast(T2RecordType->getDecl())) { + const auto &Conversions = T2RecordDecl->getVisibleConversionFunctions(

[PATCH] D82805: [clang] Fix a crash on passing C structure of incompatible type to function with reference parameter

ArcsinX updated this revision to Diff 274338. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82805/new/ https://reviews.llvm.org/D82805 Files: clang/lib/Sema/SemaInit.cpp clang/test/Sema/init-ref-c.c Index: clang/test/Sema/init-ref-c.c

[PATCH] D82805: [clang] Fix a crash on passing C structure of incompatible type to function with reference parameter

ArcsinX marked an inline comment as done. ArcsinX added inline comments. Comment at: clang/test/Sema/init-ref-c.c:8 +} \ No newline at end of file riccibruno wrote: > I think it would be better to name this test `varargs-arm.c` since this bug > can only happen

[PATCH] D85193: [clang] Do not use an invalid expression to update the initializer.

ArcsinX created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. ArcsinX requested review of this revision. This patch prevents `InitListChecker::UpdateStructuredListElement()` call with `expr == nullptr` which could cause a crash. Repository: rG LLVM Gith

[PATCH] D85301: [clang-tidy] Fix crashes in bugprone-bad-signal-to-kill-thread check.

ArcsinX created this revision. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, dexonsmith, steven_wu, jkorous, hiraditya, xazax.hun. Herald added a project: clang. ArcsinX requested review of this revision. Herald added a subscriber: ilya-biryukov. This patch fixes crashes i

[PATCH] D85398: [clang-tidy] Fix bugprone-bad-signal-to-kill-thread crash when `SIGTERM` is not a literal.

ArcsinX created this revision. Herald added subscribers: cfe-commits, dexonsmith, steven_wu, hiraditya, xazax.hun. Herald added a project: clang. ArcsinX requested review of this revision. If `SIGTERM` is not a literal (e.g. `#define SIGTERM ((unsigned)15)`) bugprone-bad-signal-to-kill-thread ch

[PATCH] D85401: [clang-tidy] Fix bugprone-bad-signal-to-kill-thread crash when `SIGTERM` was undefined after definition.

ArcsinX created this revision. Herald added subscribers: cfe-commits, dexonsmith, steven_wu, hiraditya, xazax.hun. Herald added a project: clang. ArcsinX requested review of this revision. `PP->getMacroInfo()` returns nullptr for undefined macro, which leads to null-dereference at `MI->tockens()

[PATCH] D85301: [clang-tidy] Fix crashes in bugprone-bad-signal-to-kill-thread check.

ArcsinX added a comment. In D85301#2197263 , @hokein wrote: > while this patch fixes multiple issues (as described in the message), I'd > suggest splitting it. Thanks for you suggestion. > SIGTERM is not a literal D85398

[PATCH] D85401: [clang-tidy] Fix bugprone-bad-signal-to-kill-thread crash when `SIGTERM` was undefined after definition.

ArcsinX updated this revision to Diff 283503. ArcsinX added a comment. Fix test according to review comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85401/new/ https://reviews.llvm.org/D85401 Files: clang-tools-extra/clang-tidy/bugprone/Bad

[PATCH] D85401: [clang-tidy] Fix bugprone-bad-signal-to-kill-thread crash when `SIGTERM` was undefined after definition.

ArcsinX added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-undef-sigterm.cpp:1 +// RUN: clang-tidy %s --checks="-*,bugprone-bad-signal-to-kill-thread" + hokein wrote: > nit: `// RUN: clang-tidy %s -ch

[PATCH] D85398: [clang-tidy] Fix bugprone-bad-signal-to-kill-thread crash when `SIGTERM` is not a literal.

ArcsinX updated this revision to Diff 283509. ArcsinX added a comment. Fix test according to review comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85398/new/ https://reviews.llvm.org/D85398 Files: clang-tools-extra/clang-tidy/bugprone/Bad

[PATCH] D85398: [clang-tidy] Fix bugprone-bad-signal-to-kill-thread crash when `SIGTERM` is not a literal.

ArcsinX added a comment. Thank you for review! Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-bad-signal-to-kill-thread-sigterm-not-a-literal.cpp:1 +// RUN: clang-tidy %s --checks="-*,bugprone-bad-signal-to-kill-thread" + hokein wrote: > the s

[PATCH] D85401: [clang-tidy] Fix bugprone-bad-signal-to-kill-thread crash when `SIGTERM` was undefined after definition.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG86711668330c: [clang-tidy] Fix bugprone-bad-signal-to-kill-thread crash when `SIGTERM` was… (authored by ArcsinX). Repository: rG LLVM Github Mon

[PATCH] D85398: [clang-tidy] Fix bugprone-bad-signal-to-kill-thread crash when `SIGTERM` is not a literal.

This revision was automatically updated to reflect the committed changes. Closed by commit rG216ad2da74f0: [clang-tidy] Fix bugprone-bad-signal-to-kill-thread crash when `SIGTERM` is not… (authored by ArcsinX). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D85417: [clangd] Fix crash in bugprone-bad-signal-to-kill-thread clang-tidy check.

ArcsinX created this revision. Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, dexonsmith, steven_wu, jkorous, hiraditya. Herald added a project: clang. ArcsinX requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. Inside clangd, clang-tidy che

[PATCH] D85417: [clangd] Fix crash in bugprone-bad-signal-to-kill-thread clang-tidy check.

ArcsinX updated this revision to Diff 283600. ArcsinX added a comment. Test rename: ClangTidyBadSignalToKillThread => ClangTidyNoLiteralDataInMacroToken Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85417/new/ https://reviews.llvm.org/D85417 File

[PATCH] D85417: [clangd] Fix crash in bugprone-bad-signal-to-kill-thread clang-tidy check.

ArcsinX added inline comments. Comment at: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp:441 +TEST(DiagnosticTest, ClangTidyBadSignalToKillThread) { + Annotations Main(R"cpp( hokein wrote: > ClangTidyBadSignalToKillThread doesn't seen ti provide much

[PATCH] D85417: [clangd] Fix crash in bugprone-bad-signal-to-kill-thread clang-tidy check.

ArcsinX added a comment. In D85417#2199694 , @hokein wrote: > Looks like NotNullTerminatedResultCheck.cpp also has this pattern, we may > want to fix that as well. Yes, you are right. I will fix this in the next patch. Repository: rG LLVM Github Mono

[PATCH] D85417: [clangd] Fix crash in bugprone-bad-signal-to-kill-thread clang-tidy check.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9f24148b2126: [clangd] Fix crash in bugprone-bad-signal-to-kill-thread clang-tidy check. (authored by ArcsinX). Repository: rG LLVM Github Monorep

[PATCH] D85417: [clangd] Fix crash in bugprone-bad-signal-to-kill-thread clang-tidy check.

ArcsinX added a comment. In D85417#2199830 , @ArcsinX wrote: > In D85417#2199694 , @hokein wrote: > >> Looks like NotNullTerminatedResultCheck.cpp also has this pattern, we may >> want to fix that as well. > > Yes,

[PATCH] D85417: [clangd] Fix crash in bugprone-bad-signal-to-kill-thread clang-tidy check.

ArcsinX added a comment. In D85417#2201973 , @hokein wrote: > Thinking more about this, > >> Inside clangd, clang-tidy checks don't see preprocessor events in the >> preamble. >> This leads to Token::PtrData == nullptr for tokens that the macro is defined

[PATCH] D85523: [clang-tidy] Fix a crash in bugprone-not-null-terminated-result check when `__STDC_WANT_LIB_EXT1__` was undefined after definition.

ArcsinX created this revision. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. ArcsinX requested review of this revision. PP->getMacroInfo() returns nullptr for undefined macro, so we need to check this return value before dereference. Stack dump: #0 0x

[PATCH] D85525: [clang-tidy] Fix a crash in bugprone-not-null-terminated-result check when `__STDC_WANT_LIB_EXT1__` is not a literal.

ArcsinX created this revision. Herald added subscribers: cfe-commits, xazax.hun. Herald added a project: clang. ArcsinX requested review of this revision. If `__STDC_WANT_LIB_EXT1__` is not a literal (e.g. `#define __STDC_WANT_LIB_EXT1__ ((unsigned)1)`) bugprone-not-null-terminated-result check

[PATCH] D85525: [clang-tidy] Fix a crash in bugprone-not-null-terminated-result check when `__STDC_WANT_LIB_EXT1__` is not a literal.

ArcsinX added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp:806 const auto &T = MI->tokens().back(); -StringRef ValueStr = StringRef(T.getLiteralData(), T.getLength()); -llvm::APInt IntValue; -

[PATCH] D85525: [clang-tidy] Fix a crash in bugprone-not-null-terminated-result check when `__STDC_WANT_LIB_EXT1__` is not a literal.

ArcsinX added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-not-null-terminated-result-stdc-want-lib-ext1-not-a-literal.c:3 +// RUN: -- -std=c11 -I %S/Inputs/bugprone-not-null-terminated-result + +#include "not-null-terminated-result-c.h" -

[PATCH] D85523: [clang-tidy] Fix a crash in bugprone-not-null-terminated-result check when `__STDC_WANT_LIB_EXT1__` was undefined after definition.

ArcsinX updated this revision to Diff 284257. ArcsinX added a comment. newline Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85523/new/ https://reviews.llvm.org/D85523 Files: clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp

[PATCH] D85523: [clang-tidy] Fix a crash in bugprone-not-null-terminated-result check when `__STDC_WANT_LIB_EXT1__` was undefined after definition.

This revision was automatically updated to reflect the committed changes. Closed by commit rG5965fbf81b25: [clang-tidy] Fix a crash in bugprone-not-null-terminated-result check when… (authored by ArcsinX). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D85525: [clang-tidy] Fix a crash in bugprone-not-null-terminated-result check when `__STDC_WANT_LIB_EXT1__` is not a literal.

ArcsinX updated this revision to Diff 284296. ArcsinX added a comment. - Rebase - Add T.getLiteralData() check - Newline Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85525/new/ https://reviews.llvm.org/D85525 Files: clang-tools-extra/clang-tidy

[PATCH] D85525: [clang-tidy] Fix a crash in bugprone-not-null-terminated-result check when `__STDC_WANT_LIB_EXT1__` is not a literal.

This revision was automatically updated to reflect the committed changes. Closed by commit rGdcb8d3b72234: [clang-tidy] Fix a crash in bugprone-not-null-terminated-result check when… (authored by ArcsinX). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D85193: [clang] Do not use an invalid expression to update the initializer.

ArcsinX added a comment. In D85193#2204685 , @aaron.ballman wrote: > Should the `StructuredIndex` still be incremented even in the case of an > error, as done around line 1589 and 1647? Yes, it looks like we need to increment `StructuredIndex`. > Do we

[PATCH] D85193: [clang] Do not use an invalid expression to update the initializer.

ArcsinX updated this revision to Diff 284612. ArcsinX added a comment. Check for `expr == nullptr` inside `InitListChecker::UpdateStructuredListElement()` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85193/new/ https://reviews.llvm.org/D85193 Fi

[PATCH] D85193: [clang] Do not use an invalid expression to update the initializer.

ArcsinX added a comment. In D85193#2207798 , @aaron.ballman wrote: > In D85193#2207052 , @ArcsinX wrote: > >> In D85193#2204685 , @aaron.ballman >> wrote: >> >>> I sort of

[PATCH] D85193: [clang] Do not use an invalid expression to update the initializer.

ArcsinX added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:3088 // No structured initializer list to update if (!StructuredList) return; aaron.ballman wrote: > I would move the check up to here as the only time we should get a null > expre

[PATCH] D85193: [clang] Do not use an invalid expression to update the initializer.

ArcsinX updated this revision to Diff 284987. ArcsinX added a comment. Update comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85193/new/ https://reviews.llvm.org/D85193 Files: clang/lib/Sema/SemaInit.cpp clang/test/Sema/init-invalid-stru

[PATCH] D85193: [clang] Check `expr` inside `InitListChecker::UpdateStructuredListElement()`

ArcsinX updated this revision to Diff 285101. ArcsinX added a comment. Fix comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85193/new/ https://reviews.llvm.org/D85193 Files: clang/lib/Sema/SemaInit.cpp clang/test/Sema/init-invalid-struct-

[PATCH] D85193: [clang] Check `expr` inside `InitListChecker::UpdateStructuredListElement()`

ArcsinX added a comment. In D85193#2212976 , @aaron.ballman wrote: > I have a suggestion for the added comment, Fixed comment according to your suggestion. > but I also forgot to ask, do you need someone to commit on your behalf? I already have commit

[PATCH] D85193: [clang] Check `expr` inside `InitListChecker::UpdateStructuredListElement()`

This revision was automatically updated to reflect the committed changes. Closed by commit rG3fa0a039ab6f: [clang] Check `expr` inside `InitListChecker::UpdateStructuredListElement()` (authored by ArcsinX). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D133043: [clangd] Fix tests for implicit C function declaration

ArcsinX created this revision. ArcsinX added reviewers: aaron.ballman, sammccall, kadircet. Herald added a subscriber: 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

[PATCH] D133043: [clangd] Fix tests for implicit C function declaration

This revision was automatically updated to reflect the committed changes. Closed by commit rGcc4b86cfc01c: [clangd] Fix tests for implicit C function declaration (authored by ArcsinX). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133043/new/ https:

[PATCH] D133886: [clang][RecoveryExpr] Don't perform alignment check if parameter type contains errors

ArcsinX created this revision. ArcsinX added reviewers: hokein, kadircet. Herald added a project: All. ArcsinX requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This patch fixes a crash which appears because of CheckArgAlignment() call with

[PATCH] D133886: [clang][RecoveryExpr] Don't perform alignment check if parameter type contains errors

ArcsinX added a comment. This looks similar to the problem fixed in D103825 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133886/new/ https://reviews.llvm.org/D133886

[PATCH] D133886: [clang][RecoveryExpr] Don't perform alignment check if parameter type contains errors

ArcsinX added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:5779 QualType ParamTy = Proto->getParamType(ArgIdx); +if (ParamTy->containsErrors()) + continue; hokein wrote: > It looks like for the failure case the `ParamTy`

[PATCH] D133886: [clang][RecoveryExpr] Don't perform alignment check if parameter type contains errors

ArcsinX updated this revision to Diff 460358. ArcsinX added a comment. - Check for dependent type inside CheckArgAlignment() - Simplify test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133886/new/ https://reviews.llvm.org/D133886 Files: clang/

[PATCH] D133886: [clang][RecoveryExpr] Don't perform alignment check if parameter type contains errors

ArcsinX marked 3 inline comments as done. ArcsinX added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:5779 QualType ParamTy = Proto->getParamType(ArgIdx); +if (ParamTy->containsErrors()) + continue; hokein wrote: > ArcsinX

  1   2   3   >