[PATCH] D129973: [clang] Pass FoundDecl to DeclRefExpr creator for operator overloads

2022-07-29 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. In D129973#3688329 , @SimplyDanny wrote: > In D129973#3684940 , @yvvan wrote: > >> With this change we don't pass "LocInfo" directly and it seems to break the >> locations when calling

[PATCH] D129973: [clang] Pass FoundDecl to DeclRefExpr creator for operator overloads

2022-07-28 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. With this change we don't pass "LocInfo" directly and it seems to break the locations when calling "getCXXOperatorNameRange" for this DeclRefExpr later on. Please fix it. You can introduce another "Create" static method for DeclRefExpr that accepts LocInfo and passes it

[PATCH] D81263: [Sema][CodeComplete][ObjC] Don't include arrow/dot fixits

2020-06-08 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. I'm not a big objC expert here. The idea looks fine to me and won't affect my workflow. So let's take this patch if nobody comments against it here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81263/new/

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

2020-06-04 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. @DmitryPolukhin Sorry, I didn't have time recently. Thanks a lot for taking care! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63482/new/ https://reviews.llvm.org/D63482 ___ cfe-commits

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

2020-05-06 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. @mgehre I think we need to adjust `denormalize(const IO &)` method here to convert \n back properly. It seems I missed it in my patch. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63482/new/ https://reviews.llvm.org/D63482

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

2020-05-04 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. @mgehre From your comment it seems that `clang-apply-replacements` handles the YAML wrong and does not make the proper conversion back from "\n\n" to "\n" Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63482/new/

[PATCH] D74564: libclang: Add static build support for Windows

2020-02-18 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan accepted this revision. yvvan added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/include/clang-c/Platform.h:31 +#elif defined(CINDEX_EXPORTS) + #define CINDEX_LINKAGE __attribute__((visibility("default"))) +#endif

[PATCH] D74564: libclang: Add static build support for Windows

2020-02-16 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. Please, upload patches with context (-U). Comment at: clang/include/clang-c/Platform.h:31 +#elif defined(CINDEX_EXPORTS) + #define CINDEX_LINKAGE __attribute__((visibility("default"))) +#endif Is it different from just leaving

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

2019-07-03 Thread Ivan Donchevskii via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL365017: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value (authored by yvvan, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

2019-07-03 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. ok, will do it through svn. i forgot that clang repo is called "cfe" so it's there CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63482/new/ https://reviews.llvm.org/D63482 ___ cfe-commits mailing list

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

2019-07-03 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. This script does not seem to work properly on windows. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63482/new/ https://reviews.llvm.org/D63482 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

2019-07-03 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. I have a commit access but I don't understand how am I supposed to commit (haven't done that for a while). There's no clang svn repo anymore. Do you know what's the current state of repositories and where to commit? CHANGES SINCE LAST ACTION

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

2019-07-02 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan updated this revision to Diff 207483. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63482/new/ https://reviews.llvm.org/D63482 Files: clang/include/clang/Tooling/ReplacementsYaml.h clang/unittests/Tooling/ReplacementsYamlTest.cpp Index:

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

2019-07-02 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan marked an inline comment as done. yvvan added inline comments. Comment at: clang/include/clang/Tooling/ReplacementsYaml.h:35 +: FilePath(""), Offset(0), Length(0), ReplacementText("") { + size_t lineBreakPos = ReplacementText.find('\n'); + while

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

2019-07-01 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan marked an inline comment as done. yvvan added inline comments. Comment at: clang/include/clang/Tooling/ReplacementsYaml.h:35 +: FilePath(""), Offset(0), Length(0), ReplacementText("") { + size_t lineBreakPos = ReplacementText.find('\n'); + while

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

2019-06-28 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan updated this revision to Diff 207062. yvvan added a comment. Sorry for delay. Test added, redundant comments removed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63482/new/ https://reviews.llvm.org/D63482 Files: clang/include/clang/Tooling/ReplacementsYaml.h

[PATCH] D63482: [clang-tidy] Fix the YAML created for checks like modernize-pass-by-value

2019-06-18 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan created this revision. yvvan added reviewers: gribozavr, nik. Herald added a subscriber: xazax.hun. Currently this check generates the replacement with the newline in the end. The proper way to export it to YAML is to have two \n\n instead of one. Without this fix clients should

[PATCH] D48116: [libclang] Allow skipping warnings from all included files

2019-06-05 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan accepted this revision. yvvan added a comment. This revision is now accepted and ready to land. libclang part is quite small here and looks ok. I would just accept it Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D48116/new/ https://reviews.llvm.org/D48116

[PATCH] D53072: [clang-format] Create a new tool for IDEs based on clang-format

2019-05-11 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan updated this revision to Diff 199151. yvvan added a comment. Some misleading reformatting fixed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53072/new/ https://reviews.llvm.org/D53072 Files: clang/include/clang/Format/Format.h clang/lib/Format/ContinuationIndenter.cpp

[PATCH] D53072: [clang-format] Create a new tool for IDEs based on clang-format

2019-05-11 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan updated this revision to Diff 199150. yvvan added a comment. Sorry for unrelated formatting changes - that's clang-format's work :) I've removed the extra executable. I don't know how to force that behavior only for the given line (for that I need someone who can help) but in our usecase

[PATCH] D53072: [clang-format] Create a new tool for IDEs based on clang-format

2019-05-11 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. @sammccall I can't avoid adding extra formatting options because my first attempt to introduce an ordinary clang-format option faced resistance because of not fitting the clang-format purpose to format files. CHANGES SINCE LAST ACTION

[PATCH] D53072: [clang-format] Create a new tool for IDEs based on clang-format

2019-05-09 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. In D53072#1496317 , @sammccall wrote: > My feedback would be: > > - I definitely think more control over preserving line breaks would be > useful. Actually preserving *blank* lines is an important property. If you > see D60605

[PATCH] D53072: [clang-format] Create a new tool for IDEs based on clang-format

2019-05-09 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. @ilya-biryukov I don't really care how it's used from the tool side. I'm also fine to have a new option in clang-format itself. That's why this review is here - to ask for opinions. It's easy to remove that "ide" part from this patch and just add an option for

[PATCH] D60605: [clangd] Revamp textDocument/onTypeFormatting.

2019-05-08 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. @ilya-biryukov What do you think about D53072 ? It can be polished and combined with this change removing some code from here (which I assume is a good thing). The idea there is that clang-format knows that it's not allowed to remove new

[PATCH] D60678: [libclang] Forward isInline for NamespaceDecl to libclang

2019-05-03 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. Seems so. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60678/new/ https://reviews.llvm.org/D60678 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61232: [libclang] Restore old clang_Cursor_isAnonymous behaviour

2019-04-29 Thread Ivan Donchevskii via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL359448: [libclang] Restore old clang_Cursor_isAnonymous behaviour (authored by yvvan, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D61232: [libclang] Restore old clang_Cursor_isAnonymous behaviour

2019-04-28 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. Thank you! Sorry that I forgot about the required minor version increment. I will run tests tomorrow and then I can commit it for you. Comment at: include/clang-c/Index.h:35 #define CINDEX_VERSION_MAJOR 0 #define CINDEX_VERSION_MINOR 55

[PATCH] D61232: [libclang] Restore old clang_Cursor_isAnonymous behaviour

2019-04-28 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. Could you please also upload diff with the full context? It's -U for git and quite similar for svn. Otherwise looks ok. Comment at: test/Index/print-type.c:70 // CHECK: TypeRef=struct Struct:16:8 [type=struct Struct] [typekind=Record] [isPOD=1]

[PATCH] D61232: [libclang] Restore old clang_Cursor_isAnonymous behaviour

2019-04-27 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. Now we know that somebody else also uses libclang :) I the mentioned change I only wanted to follow the same logic as in TypePrinter::printTag to cover more anonymity cases. Adding the old one as an extra function seems totally fine to me. Can you please add some tests

[PATCH] D53072: [clang-format] Create a new tool for IDEs based on clang-format

2019-04-25 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. @sammccall Having a separate tool is nice because it allows the client to make it plugable. clang-format sometimes changes options quite significantly and it can be nice if you have a choice which version to pick, otherwise it might be unable to read the configuration

[PATCH] D53072: [clang-format] Create a new tool for IDEs based on clang-format

2019-04-12 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan updated this revision to Diff 194844. yvvan retitled this revision from "[clang-format] Introduce the flag which allows not to shrink lines" to "[clang-format] Create a new tool for IDEs based on clang-format". yvvan edited the summary of this revision. yvvan added a reviewer: arphaman.

[PATCH] D58501: [libclang] Fix CXTranslationUnit_KeepGoing

2019-03-07 Thread Ivan Donchevskii via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL355586: [libclang] Fix CXTranslationUnit_KeepGoing (authored by yvvan, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D57951: [Lex] Allow to set missing include error to not fatal

2019-02-11 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan planned changes to this revision. yvvan added a comment. It's probably not needed because I don't see a path which checked for the fatal errors in 8.0. So i will probably abandon this one or update if it does not cover the case I need. CHANGES SINCE LAST ACTION

[PATCH] D57951: [Lex] Allow to set missing include error to not fatal

2019-02-08 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. I've tested it with libclang. If something else is needed for proper clangd args forwarding - let me know. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57951/new/ https://reviews.llvm.org/D57951 ___ cfe-commits

[PATCH] D57951: [Lex] Allow to set missing include error to not fatal

2019-02-08 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan created this revision. yvvan added reviewers: ilya-biryukov, nik. For IDE it's a nice feature to not get a fatal error and continue preprocessing even if the file is missing. This allows include further files and get more relevant information about the current translation unit.

[PATCH] D53072: [clang-format] Introduce the flag which allows not to shrink lines

2019-01-11 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan updated this revision to Diff 181274. yvvan added a comment. The tests are improved - now they actually act differently with and without the introduced flag. Also few more cases are covered (see the second added test). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53072/new/

[PATCH] D54996: [libclang] Fix clang_Cursor_isAnonymous

2019-01-10 Thread Ivan Donchevskii via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL350805: [libclang] Fix clang_Cursor_isAnonymous (authored by yvvan, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D54996: [libclang] Fix clang_Cursor_isAnonymous

2019-01-10 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan updated this revision to Diff 181005. yvvan added a comment. Replace the absolute path with {{.*}} to be independent from the machine. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54996/new/ https://reviews.llvm.org/D54996 Files: test/Index/print-type.cpp

[PATCH] D54996: [libclang] Fix clang_Cursor_isAnonymous

2019-01-09 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. Good point :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54996/new/ https://reviews.llvm.org/D54996 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D53072: [clang-format] Introduce the flag which allows not to shrink lines

2019-01-07 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. @djasper Ok, if there's a possible way to go forward I will find time to provide a better test which behaves differently depending on this check. Also my current patch is a bit bigger than this version. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53072/new/

[PATCH] D53072: [clang-format] Introduce the flag which allows not to shrink lines

2019-01-07 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. In D53072#1348033 , @djasper wrote: > $ cat /tmp/test.cc > int foo(int a, > int b) { > f(); > } > > $ clang-format -style="{ColumnLimit: 0}" /tmp/test.cc > int foo(int a, > int b) { >

[PATCH] D53072: [clang-format] Introduce the flag which allows not to shrink lines

2019-01-07 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. In D53072#1342363 , @djasper wrote: > I don't quite understand. What you are describing should already be the > behavior with ColumnLimit=0 and I think your test should pass without the new > option. Doesn't it? As far as I see

[PATCH] D55595: [clang-tidy] Share the forced linking code between clang-tidy tool and plugin

2018-12-13 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan marked an inline comment as done. yvvan added inline comments. Comment at: clang-tidy/plugin/ClangTidyPlugin.cpp:11 #include "../ClangTidy.h" +#include "../ClangTidyForceLinker.h" #include "../ClangTidyModule.h" yvvan wrote: > lebedev.ri wrote: > > yvvan

[PATCH] D55595: [clang-tidy] Share the forced linking code between clang-tidy tool and plugin

2018-12-13 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan marked an inline comment as done. yvvan added inline comments. Comment at: clang-tidy/ClangTidyForceLinker.h:11 +#include "llvm/Support/Compiler.h" + +namespace clang { dyung wrote: > dyung wrote: > > Is there a reason that this header file does not use

[PATCH] D55595: [clang-tidy] Share the forced linking code between clang-tidy tool and plugin

2018-12-13 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan marked an inline comment as done. yvvan added inline comments. Comment at: clang-tidy/plugin/ClangTidyPlugin.cpp:11 #include "../ClangTidy.h" +#include "../ClangTidyForceLinker.h" #include "../ClangTidyModule.h" lebedev.ri wrote: > yvvan wrote: > > It

[PATCH] D55595: [clang-tidy] Share the forced linking code between clang-tidy tool and plugin

2018-12-13 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan marked an inline comment as done. yvvan added inline comments. Comment at: clang-tidy/plugin/ClangTidyPlugin.cpp:11 #include "../ClangTidy.h" +#include "../ClangTidyForceLinker.h" #include "../ClangTidyModule.h" It seems it had to go after #include

[PATCH] D55595: [clang-tidy] Share the forced linking code between clang-tidy tool and plugin

2018-12-13 Thread Ivan Donchevskii via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE349038: [clang-tidy] Share the forced linking code between clang-tidy tool and plugin (authored by yvvan, committed by ). Changed prior to commit: https://reviews.llvm.org/D55595?vs=177832=178048#toc

[PATCH] D55595: [clang-tidy] Share the forced linking code between clang-tidy tool and plugin

2018-12-13 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. In D55595#1329647 , @JonasToth wrote: > One thing: Could you please check the utility-scripts in clang-tidy that > create new checks, if they need adjustments? Not sure if we have something in > there that relies on the old

[PATCH] D55595: Share the forced linking code between clang-tidy tool and plugin

2018-12-12 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan updated this revision to Diff 177832. yvvan marked an inline comment as done. yvvan added a comment. Add standard prologue to the new header CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55595/new/ https://reviews.llvm.org/D55595 Files: clang-tidy/ClangTidyForceLinker.h

[PATCH] D55595: Share the forced linking code between clang-tidy tool and plugin

2018-12-12 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan updated this revision to Diff 177829. yvvan retitled this revision from "Add missing bugprone checks to clang-tidy plugin" to "Share the forced linking code between clang-tidy tool and plugin". yvvan edited the summary of this revision. Herald added a subscriber: srhines. CHANGES SINCE

[PATCH] D55595: Add missing bugprone checks to clang-tidy plugin

2018-12-12 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan planned changes to this revision. yvvan added a comment. In D55595#1328100 , @lebedev.ri wrote: > 1. Please always upload all patches with full context. > 2. There are two places where this pattern exists - this file, and >

[PATCH] D55595: Add missing bugprone checks to clang-tidy plugin

2018-12-12 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan created this revision. yvvan added reviewers: JonasToth, alexfh. Synchronize it with ClangTidyMain Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D55595 Files: clang-tidy/plugin/ClangTidyPlugin.cpp Index: clang-tidy/plugin/ClangTidyPlugin.cpp

[PATCH] D55415: Revert removal of tidy plugin support from libclang

2018-12-10 Thread Ivan Donchevskii via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL348764: [libclang] Revert removal of tidy plugin support from libclang introduced in… (authored by yvvan, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D55415: Revert removal of tidy plugin support from libclang

2018-12-07 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. In D55415#1323120 , @bkramer wrote: > I'd be interested in hearing how this is used. I added this feature as an > experiment a while back but it simply didn't work as I envisioned it to. Some > checks do work but the overall

[PATCH] D55415: Revert removal of tidy plugin support from libclang

2018-12-07 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. We can also add an extra variable for it if you care about build time CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55415/new/ https://reviews.llvm.org/D55415 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D55415: Revert removal of tidy plugin support from libclang

2018-12-07 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan updated this revision to Diff 177134. yvvan added a comment. Herald added a subscriber: mgorny. I generated the wrong diff. This is the proper one. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55415/new/ https://reviews.llvm.org/D55415 Files: tools/libclang/CIndex.cpp

[PATCH] D55415: Revert removal of tidy plugin support from libclang

2018-12-07 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan created this revision. yvvan added reviewers: bkramer, arphaman, nik. Herald added subscribers: kadircet, jkorous, ioeric, javed.absar, ilya-biryukov. libclang has nothing to do with clangd so I don't see why having the last one is the reason to remove features from libclang. Especially

[PATCH] D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap

2018-12-04 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan abandoned this revision. yvvan added a comment. @zturner The fact that so many call-sites decide what to do is pretty error-prone. There was already at least one issue when this flag was not properly set by some of the call-sites. CHANGES SINCE LAST ACTION

[PATCH] D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap

2018-12-03 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. @ilya-biryukov Hm. What about another way around? - We have user include paths (-I) and report them to the filesystem. This means that we have specific paths under which nothing can be mmaped and everything else can be. In particular cases we can also report -isystem

[PATCH] D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap

2018-12-03 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. In D54995#1316457 , @ilya-biryukov wrote: > In D54995#1316437 , @yvvan wrote: > > > Ok, no global option. > > Why not placing your VolatileFSProvider in clang so that libclang could > >

[PATCH] D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap

2018-12-03 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan planned changes to this revision. yvvan added a comment. Ok, no global option. Why not placing your VolatileFSProvider in clang so that libclang could you it too? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54995/new/ https://reviews.llvm.org/D54995

[PATCH] D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap

2018-12-03 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan marked an inline comment as done. yvvan added inline comments. Comment at: lib/Support/MemoryBuffer.cpp:42 +static bool MemoryMappingEnabled = true; + lebedev.ri wrote: > Such global flags are a bad idea in general, and really not great in LLVM's >

[PATCH] D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap

2018-12-03 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan updated this revision to Diff 176318. yvvan retitled this revision from "[MemoryBuffer] By default assume that all files are volatile to prevent unintended file locks" to "[MemoryBuffer] Add the setter to be able to force disabled mmap". yvvan edited the summary of this revision. yvvan

[PATCH] D54995: [MemoryBuffer] By default assume that all files are volatile to prevent unintended file locks

2018-11-30 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. @ilya-biryukov What do you think about the global settable bool in MemoryBuffer in place of the ifdef from https://reviews.llvm.org/D35200 ? In this case the client on Windows can set it and you're safe that any MemoryBuffer call never mmaps. CHANGES SINCE LAST ACTION

[PATCH] D54995: [MemoryBuffer] By default assume that all files are volatile to prevent unintended file locks

2018-11-30 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. No success with unmapping. Buffers are hold by PCMCache in Preprocessor (and the same one in ASTReader) and if I clean them then SourceManger crashed sooner or later on some call that gets data from external files. So far I have no steps to reproduce the lock on user file

[PATCH] D54995: [MemoryBuffer] By default assume that all files are volatile to prevent unintended file locks

2018-11-29 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. I'm currently trying out another suggestion - which unmaps memory buffer caches after ASTUnit's Parse or Reparse and is limited to Windows only. And my aim currently is not only clangd but any other client as well. CHANGES SINCE LAST ACTION

[PATCH] D54995: [MemoryBuffer] By default assume that all files are volatile to prevent unintended file locks

2018-11-29 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. According to https://msdn.microsoft.com/en-us/2e9c3174-af48-4fa3-9f6a-fb62b23ed994 - "Unmapping a mapped view of a file invalidates the range occupied by the view in the address space of the process and makes the range available for other allocations". Also as far as i

[PATCH] D54995: [MemoryBuffer] By default assume that all files are volatile to prevent unintended file locks

2018-11-28 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. @ilya-biryukov To clarify a little bit - i did not write my own simple app but used libclang in qt creator to reproduce the lock and track the issue. This time I hope to go further, collect flags used by clang and make a simple example. CHANGES SINCE LAST ACTION

[PATCH] D54995: [MemoryBuffer] By default assume that all files are volatile to prevent unintended file locks

2018-11-28 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. @ilya-biryukov https://bugreports.qt.io/browse/QTCREATORBUG-15449 I was able to lock files last time with mmap. The Windows API calls are CreateFileMappingW (it's in the Path.inc, line 794) and MapViewOfFile (further down). As far as I remember the second call actually

[PATCH] D54995: [MemoryBuffer] By default assume that all files are volatile to prevent unintended file locks

2018-11-28 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. @ilya-biryukov According to https://stackoverflow.com/a/7414798 (if it's true) we can't prevent locks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54995/new/ https://reviews.llvm.org/D54995 ___ cfe-commits

[PATCH] D54995: [MemoryBuffer] By default assume that all files are volatile to prevent unintended file locks

2018-11-28 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. "could we figure out the exact cause of the issue?" I'm pretty sure we can. Currently the issue is mentioned in our bugreports (https://bugreports.qt.io/secure/attachment/78582/clang-locks.png) and i plan to reproduce it and track the exact issue. And this review was not

[PATCH] D54995: [MemoryBuffer] By default assume that all files are volatile to prevent unintended file locks

2018-11-28 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. Hm, probably FileManager can be that adapter since it's in clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54995/new/ https://reviews.llvm.org/D54995 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D54995: [MemoryBuffer] By default assume that all files are volatile to prevent unintended file locks

2018-11-28 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. I've realized that this patch covers too much stuff outside of clang and I have no idea how bad is to not memory map it. "My hope is that we can get rid of this flag some day" - i'm not sure it's possible. For that we need some concept of user and system files in llvm

[PATCH] D54996: [libclang] Fix clang_Cursor_isAnonymous

2018-11-28 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan created this revision. yvvan added reviewers: nik, marcobubke. Herald added a subscriber: arphaman. Use the same logic as in TypePrinter::printTag to determine that the tag is anonymous and the separate check for namespaces. https://reviews.llvm.org/D54996 Files:

[PATCH] D54995: [MemoryBuffer] By default assume that all files are volatile to prevent unintended file locks

2018-11-28 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. @ilya-biryukov I have the reported evidence but didn't yet have time to investigate myself. This is probably caused by our upgrade to Clang-7 which shows that we can't rely on the aimed fixes like the one I mention (D47460 ). Another

[PATCH] D54995: [MemoryBuffer] By default assume that all files are volatile to prevent unintended file locks

2018-11-28 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan created this revision. yvvan added reviewers: ilya-biryukov, Dmitry.Kozhevnikov. There are reported cases of non-system files being locked by libclang on Windows (and likely by other clients as well). I tried to solve that with https://reviews.llvm.org/D47460 but it might be not the only

[PATCH] D54995: [MemoryBuffer] By default assume that all files are volatile to prevent unintended file locks

2018-11-28 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan updated this revision to Diff 175635. yvvan added a comment. VFS is moved to llvm in 8.0, update the diff CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54995/new/ https://reviews.llvm.org/D54995 Files: include/llvm/Support/MemoryBuffer.h

[PATCH] D54934: [libclang] Fix clang_Cursor_getNumArguments and clang_Cursor_getArgument for CXXConstructExpr

2018-11-27 Thread Ivan Donchevskii via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC347654: [libclang] Fix clang_Cursor_getNumArguments and clang_Cursor_getArgument for… (authored by yvvan, committed by ). Repository: rC Clang CHANGES SINCE LAST ACTION

[PATCH] D54934: [libclang] Fix clang_Cursor_getNumArguments and clang_Cursor_getArgument for CXXConstructExpr

2018-11-27 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan created this revision. yvvan added reviewers: nik, marcobubke. Herald added a subscriber: arphaman. Constructors have the same methods for arguments as call expressions. Let's provide a way to get their arguments the same way. https://reviews.llvm.org/D54934 Files:

[PATCH] D53866: [Preamble] Fix preamble for circular #includes

2018-10-31 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. @ilya-biryukov As far as I understand the problem the same thing happens when you are in the header a.h which includes b.h and b.h includes a.h at the same time. So you get this recursion indirectly and very often because that's why include guards are there.

[PATCH] D53072: [clang-format] Introduce the flag which allows not to shrink lines

2018-10-19 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. Do you know the better way to accomplish my aim than adding an option to libFormat? For example making a dependent library which serves a little different purpose than libFormat itself or something simpler? https://reviews.llvm.org/D53072

[PATCH] D53072: [clang-format] Introduce the flag which allows not to shrink lines

2018-10-10 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan created this revision. yvvan added reviewers: klimek, djasper, krasimir. Currently there's no way to prevent to lines optimization even if you have intentionally put to split the line. In general case it's fine. So I would prefer to have such option which you can enable in special cases

[PATCH] D52261: [Sema] Generate completion fix-its for C code as well

2018-09-21 Thread Ivan Donchevskii via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC342721: [CodeComplete] Generate completion fix-its for C code as well (authored by yvvan, committed by ). Repository: rC Clang https://reviews.llvm.org/D52261 Files: lib/Parse/ParseExpr.cpp

[PATCH] D52261: [Sema] Generate completion fix-its for C code as well

2018-09-21 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan updated this revision to Diff 166407. yvvan added a comment. Provide CorrectedBase = Base for C code https://reviews.llvm.org/D52261 Files: lib/Parse/ParseExpr.cpp test/CodeCompletion/member-access.c Index: test/CodeCompletion/member-access.c

[PATCH] D52261: [Sema] Generate completion fix-its for C code as well

2018-09-21 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. You're right actually. The fix is much simpler than I expected :) https://reviews.llvm.org/D52261 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52261: [Sema] Generate completion fix-its for C code as well

2018-09-20 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. I tried that first but did not I find a way just to copy an expression (we basically need the same expr for such case). Do you know how to properly generate a copy of expression or some other way to get the same expression? https://reviews.llvm.org/D52261

[PATCH] D52261: [Sema] Generate completion fix-its for C code as well

2018-09-19 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan updated this revision to Diff 166110. yvvan added a comment. Test is added https://reviews.llvm.org/D52261 Files: lib/Sema/SemaCodeComplete.cpp test/CodeCompletion/member-access.c Index: test/CodeCompletion/member-access.c

[PATCH] D52261: [Sema] Generate completion fix-its for C code as well

2018-09-19 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan created this revision. yvvan added reviewers: nik, ilya-biryukov. Current completion fix-its approach does not provide OtherOpBase for C code. But we can easily proceed in this case taking the original Base type. https://reviews.llvm.org/D52261 Files: lib/Sema/SemaCodeComplete.cpp

[PATCH] D51281: [libclang] Return the proper pointee type for 'auto' deduced to pointer

2018-09-07 Thread Ivan Donchevskii via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL341656: [libclang] Return the proper pointee type for auto deduced to pointer (authored by yvvan, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D51281: [libclang] Return the proper pointee type for 'auto' deduced to pointer

2018-09-06 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan updated this revision to Diff 164151. yvvan added a comment. Comments addressed https://reviews.llvm.org/D51281 Files: test/Index/print-type.cpp tools/libclang/CXType.cpp Index: tools/libclang/CXType.cpp === ---

[PATCH] D51281: [libclang] Return the proper pointee type for 'auto' deduced to pointer

2018-08-29 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. You are completely right! Thanks! I did not think that -test-print-type also checks for the pointee. https://reviews.llvm.org/D51281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D51281: [libclang] Return the proper pointee type for 'auto' deduced to pointer

2018-08-27 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan created this revision. yvvan added reviewers: erikjv, arphaman, michaelwu. Currently the resulting type is always invalid in such case. https://reviews.llvm.org/D51281 Files: tools/libclang/CXType.cpp Index: tools/libclang/CXType.cpp

[PATCH] D40481: [libclang] Fix cursors for arguments of Subscript and Call operators

2018-08-23 Thread Ivan Donchevskii via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL340521: [libclang] Fix cursors for arguments of Subscript and Call operators (authored by yvvan, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D40481: [libclang] Fix cursors for arguments of Subscript and Call operators

2018-08-23 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan accepted this revision. yvvan added a comment. This revision is now accepted and ready to land. Let's proceed with this one. I really see that it's going to be useful. https://reviews.llvm.org/D40481 ___ cfe-commits mailing list

[PATCH] D49794: [libclang] Allow skipping warnings from all included files

2018-07-31 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. In https://reviews.llvm.org/D49794#1182272, @ilya-biryukov wrote: > In https://reviews.llvm.org/D49794#1182220, @yvvan wrote: > > > And we already saw, that -isystem is not the best choice for that. > > > Are you referring to the file-locking on Windows? > Any other

[PATCH] D49794: [libclang] Allow skipping warnings from all included files

2018-07-31 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. Anyways, if c++ part does not seem relevant I'm fine if we only have libclang part from https://reviews.llvm.org/D48116. BTW I don't remember if you answered something to my suggestion of adding flag -ithird-party as an alternative to -isystem which does not lock files

[PATCH] D49794: [libclang] Allow skipping warnings from all included files

2018-07-31 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. And we already saw, that -isystem is not the best choice for that. And by the way - clang-tidy has this filtering in consumer which does not exist in it's plugin-mode so it spits all system diagnostics all the time... https://reviews.llvm.org/D49794

[PATCH] D49794: [libclang] Allow skipping warnings from all included files

2018-07-31 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. I already mentioned in the review inherited by this one that this way covers also loaded plugins with different consumers. So let's assume that driver loads clang-tidy and some other plugins and runs over each file in the project. It makes sense not to include clang-tidy

[PATCH] D49794: [libclang] Allow skipping warnings from all included files

2018-07-31 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan updated this revision to Diff 158211. yvvan added a comment. Restore missing tests https://reviews.llvm.org/D49794 Files: include/clang-c/Index.h include/clang/Basic/Diagnostic.h include/clang/Basic/DiagnosticOptions.def include/clang/Driver/Options.td

[PATCH] D49063: [libclang 1/8] Add support for ObjCObjectType

2018-07-26 Thread Ivan Donchevskii via Phabricator via cfe-commits
yvvan added a comment. I'm mostly fine with your set of patches. Let me double check and we can get it in. https://reviews.llvm.org/D49063 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

  1   2   3   >