[PATCH] D54077: [clangd] Implemented DraftFileSystem

sammccall added a comment. I don't think either point 1 or 2 above have been addressed, and so I'm not comfortable moving forward with this change. > I think it would be reasonable to say that a large portion of C++ users are > used to the behavior that this patch introduces. I agree, the new

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

ebevhan added a comment. In https://reviews.llvm.org/D53738#1288333, @rjmccall wrote: > In https://reviews.llvm.org/D53738#1287123, @ebevhan wrote: > > > In https://reviews.llvm.org/D53738#1284213, @rjmccall wrote: > > > > > Not out of line with other features that significantly break with what's

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

JonasToth added inline comments. Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:20 + +static const char LoopName[] = "forLoopName"; +static const char loopVarName[] = "loopVar"; JonasToth wrote: > ztamas wrote: > > JonasToth wrote: > > > Please mov

r346216 - T was unused on assertion disabled builds.

Author: kadircet Date: Tue Nov 6 00:59:25 2018 New Revision: 346216 URL: http://llvm.org/viewvc/llvm-project?rev=346216&view=rev Log: T was unused on assertion disabled builds. Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp URL: http://llvm.org/

[PATCH] D54077: [clangd] Implemented DraftFileSystem

sammccall added a comment. Someone mentioned to me that the interaction-between-features argument wasn't clear here: - we **don't** currently update diagnostics for `A.cc` when `A.h` is edited - we should, this seems more obvious & important than what we do with drafts - this interacts badly wit

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

Szelethus added inline comments. Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:20 + +static const char LoopName[] = "forLoopName"; +static const char loopVarName[] = "loopVar"; JonasToth wrote: > JonasToth wrote: > > ztamas wrote: > > > JonasToth

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

lebedev.ri added inline comments. Comment at: clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp:20 + +static const char LoopName[] = "forLoopName"; +static const char loopVarName[] = "loopVar"; Szelethus wrote: > JonasToth wrote: > > JonasToth wrote: > > > ztamas

[PATCH] D54061: [clang-tidy] run() doesn't update the SourceManager.

sammccall added a comment. In https://reviews.llvm.org/D54061#1286956, @JonasToth wrote: > > Theoretically, we could replace `ClangTidyCheck::check` with > > `ClangTidyCheck::run`, but I'm not sure it is worth, > > `ClangTidyCheck::check` is a public API, and is widely-used (for all > > clang-

[PATCH] D54061: [clang-tidy] run() doesn't update the SourceManager.

JonasToth added a comment. In https://reviews.llvm.org/D54061#1288395, @sammccall wrote: > In https://reviews.llvm.org/D54061#1286956, @JonasToth wrote: > > > > Theoretically, we could replace `ClangTidyCheck::check` with > > > `ClangTidyCheck::run`, but I'm not sure it is worth, > > > `ClangTi

[PATCH] D52795: [analyzer][PlistMacroExpansion] Part 3.: Macro arguments are expanded

xazax.hun added a comment. I would love to see a test with deeper macro in macro expansion and larger number of arguments, with some of the arguments unused. Some minor nits inline, otherwise looks good. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:831 const Ma

[clang-tools-extra] r346219 - [clang-tidy] run() doesn't update the SourceManager.

Author: sammccall Date: Tue Nov 6 01:28:23 2018 New Revision: 346219 URL: http://llvm.org/viewvc/llvm-project?rev=346219&view=rev Log: [clang-tidy] run() doesn't update the SourceManager. Summary: By now the context's SourceManager is now initialized everywhere that ClangTidyCheck::registerMatch

[PATCH] D54061: [clang-tidy] run() doesn't update the SourceManager.

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE346219: [clang-tidy] run() doesn't update the SourceManager. (authored by sammccall, committed by ). Changed prior to commit: https://reviews.llvm.org/D54061?vs=172471&id=172719#toc Repository: rCT

[PATCH] D54077: [clangd] Implemented DraftFileSystem

ioeric added a comment. In https://reviews.llvm.org/D54077#1288387, @sammccall wrote: > Someone mentioned to me that the interaction-between-features argument wasn't > clear here: > > - we **don't** currently update diagnostics for `A.cc` when `A.h` is edited > - we should, this seems more obvio

[PATCH] D54077: [clangd] Implemented DraftFileSystem

klimek added a comment. In https://reviews.llvm.org/D54077#1288404, @ioeric wrote: > In https://reviews.llvm.org/D54077#1288387, @sammccall wrote: > > > Someone mentioned to me that the interaction-between-features argument > > wasn't clear here: > > > > - we **don't** currently update diagnosti

[PATCH] D54077: [clangd] Implemented DraftFileSystem

nik added a comment. In https://reviews.llvm.org/D54077#1288404, @ioeric wrote: > I would be very happy if `A.cc` can see the unsaved `A.h` when I am editing > `A.cc`. We have that in Qt Creator (with libclang) and that's quite handy as it can save you some build cycles. > Not sure if I want

[PATCH] D54077: [clangd] Implemented DraftFileSystem

nik added a comment. In https://reviews.llvm.org/D54077#1288413, @nik wrote: > If it helps (and the LSP allows it): In case A.h is edited, we flag it dirty. > If the user makes some file depending on it visible (or the current file), we > trigger a reparse for that. Not sure whether the LSP has

[PATCH] D52967: Extend shelf-life by 70 years

bmwiedemann added a comment. I tested now, that it still works on i586 (in addition to x86_64) Repository: rC Clang https://reviews.llvm.org/D52967 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/list

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth created this revision. JonasToth added reviewers: alexfh, aaron.ballman, hokein, sammccall, lebedev.ri. Herald added subscribers: cfe-commits, xazax.hun, mgorny. `run-clang-tidy.py` is the parallel executor for `clang-tidy`. Due to the common header-inclusion problem in C++/C diagnostics

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth updated this revision to Diff 172726. JonasToth added a comment. - spurious change in my git Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54141 Files: clang-tidy/tool/run-clang-tidy.py clang-tidy/tool/run_clang_tidy.py clang-tidy/tool/test_input/out_csa_cmake.

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth added inline comments. Comment at: clang-tidy/tool/run_clang_tidy.py:1 +run-clang-tidy.py This simlink is required for my unittests, I don't know how to add the added tests in the `lit` test-suite so there is no change there yet. A bit of guidance the

[PATCH] D54077: [clangd] Implemented DraftFileSystem

LutsenkoDanil added a comment. @sammccall Thank you for detailed explanation! In https://reviews.llvm.org/D54077#1288387, @sammccall wrote: > Someone mentioned to me that the interaction-between-features argument wasn't > clear here: > > - we **don't** currently update diagnostics for `A.cc` wh

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

ioeric updated this revision to Diff 172732. ioeric added a comment. - Rebase Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53433 Files: clangd/index/Background.cpp clangd/index/Background.h clangd/index/FileIndex.cpp clangd/index/FileIndex.h clangd/index/SymbolColle

[clang-tools-extra] r346221 - [clangd] auto-index stores symbols per-file instead of per-TU.

Author: ioeric Date: Tue Nov 6 02:55:21 2018 New Revision: 346221 URL: http://llvm.org/viewvc/llvm-project?rev=346221&view=rev Log: [clangd] auto-index stores symbols per-file instead of per-TU. Summary: This allows us to deduplicate header symbols across TUs. File digests are collects when coll

[PATCH] D53433: [clangd] auto-index stores symbols per-file instead of per-TU.

This revision was automatically updated to reflect the committed changes. Closed by commit rL346221: [clangd] auto-index stores symbols per-file instead of per-TU. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D53433

[PATCH] D53933: [clangd] Get rid of QueryScopes.empty() == AnyScope special case.

ioeric updated this revision to Diff 172735. ioeric added a comment. - rebase Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53933 Files: clangd/FindSymbols.cpp clangd/index/Index.h clangd/index/MemIndex.cpp clangd/index/dex/Dex.cpp unittests/clangd/DexTests.cpp uni

[clang-tools-extra] r346223 - [clangd] Get rid of QueryScopes.empty() == AnyScope special case.

Author: ioeric Date: Tue Nov 6 03:08:17 2018 New Revision: 346223 URL: http://llvm.org/viewvc/llvm-project?rev=346223&view=rev Log: [clangd] Get rid of QueryScopes.empty() == AnyScope special case. Reviewers: sammccall Reviewed By: sammccall Subscribers: ilya-biryukov, MaskRay, jkorous, arpham

[PATCH] D53933: [clangd] Get rid of QueryScopes.empty() == AnyScope special case.

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE346223: [clangd] Get rid of QueryScopes.empty() == AnyScope special case. (authored by ioeric, committed by ). Changed prior to commit: https://reviews.llvm.org/D53933?vs=172735&id=172737#toc Reposit

[PATCH] D54106: [clangd] Limit the index-returned results in dexp.

hokein updated this revision to Diff 172736. hokein added a comment. Update based on the offline discussion. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54106 Files: clangd/index/dex/dexp/Dexp.cpp Index: clangd/index/dex/dexp/Dexp.cpp

[PATCH] D54105: [clangd] Deduplicate query scopes.

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54105 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D54111: [clang-format] Do not treat the asm clobber [ as ObjCExpr

krasimir updated this revision to Diff 172738. krasimir added a comment. - Address review comments Repository: rC Clang https://reviews.llvm.org/D54111 Files: lib/Format/TokenAnnotator.cpp unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp ==

[PATCH] D54106: [clangd] Limit the index-returned results in dexp.

ioeric added inline comments. Comment at: clangd/index/dex/dexp/Dexp.cpp:166 + cl::init(10), + cl::desc("Max results to display. This flag is only meaningful when -name" + " is set."), Maybe `The max number of symbols with the same name b

[PATCH] D52795: [analyzer][PlistMacroExpansion] Part 3.: Macro arguments are expanded

whisperity added inline comments. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:1018 +auto It = CurrExpArgTokens.begin(); +while (It != CurrExpArgTokens.end()) { + if (It->isNot(tok::identifier)) { xazax.hun wrote: > Maybe a for loop mor n

[clang-tools-extra] r346224 - [clangd] Deduplicate query scopes.

Author: ioeric Date: Tue Nov 6 03:17:40 2018 New Revision: 346224 URL: http://llvm.org/viewvc/llvm-project?rev=346224&view=rev Log: [clangd] Deduplicate query scopes. Summary: For example, when anonymous namespace is present, duplicated namespaces might be generated for the enclosing namespace.

[PATCH] D54077: [clangd] Implemented DraftFileSystem

ilya-biryukov added a comment. In https://reviews.llvm.org/D54077#1287289, @klimek wrote: > don't most IDEs show whether a file is saved or just modified? They do, but whenever you run the build from them, they will save all modified files before actually running it. In https://reviews.llvm.o

[PATCH] D54105: [clangd] Deduplicate query scopes.

This revision was automatically updated to reflect the committed changes. Closed by commit rL346224: [clangd] Deduplicate query scopes. (authored by ioeric, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D54105 Files: clang-tools-extra/

[PATCH] D53334: [Frontend/Modules] Show diagnostics on prebuilt module configuration mismatch too

whisperity planned changes to this revision. whisperity added a comment. In https://reviews.llvm.org/D53334#1288057, @dblaikie wrote: > In https://reviews.llvm.org/D53334#1273877, @whisperity wrote: > > > @dblaikie I have created a test, but unfortunately `%clang_cpp` in LIT > > invokes `clang -

[PATCH] D52984: [analyzer] Checker reviewer's checklist

Szelethus added a comment. We probably should also add an entry about some code conventions we use here, for example, the use of `auto` was debated recently when used with `SVal::getAs`, maybe something like this: - As an LLVM subproject, the code in the Static Analyzer should too follow the h

[PATCH] D52984: [analyzer] Checker reviewer's checklist

Szelethus added a comment. Also, context is missing :) https://reviews.llvm.org/D52984 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r346167 - [Driver] Reland again again: Default Android toolchains to libc++.

Just fyi: this broke Chrome's build of compiler-rt using tot Clang against the sysroot generated by NDK r16's (I think) make_standalong_toolchain.py, see https://crbug.com/902270 It looks like we'll just hack around it though. On Mon, Nov 5, 2018 at 9:57 PM, Dan Albert via cfe-commits wrote: > A

[PATCH] D51729: [Tooling] JSONCompilationDatabasePlugin infers compile commands for missing files

Lekensteyn added a comment. In https://reviews.llvm.org/D51729#1288360, @sammccall wrote: > > I'm not entirely sure what to do here. The old behavior works great in > > cases where a complete database is available (produced by CMake). The new > > behavior might work better for clangd (?), but i

[PATCH] D53457: clang-cl: Add "/clang:" pass-through arg support.

hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. Okay, looks good to me (with a small nit). Comment at: docs/UsersManual.rst:2852 /Brepro Emit an object file which can be reproduced over time + /cla

Re: r345591 - [CodeGen] Disable the machine verifier on a ThinLTO test

Thanks for the suggestion, I think it’s reasonable since it’s all due to: * ICALL_BRANCH_FUNNEL: Bad machine code: Explicit definition marked as use https://bugs.llvm.org/show_bug.cgi?id=39436 . I’ll look into it. > On 5 Nov 2018, at 19:13, David Bl

[PATCH] D54109: [clang-query] continue querying even if files are skipped

Lekensteyn updated this revision to Diff 172747. Lekensteyn retitled this revision from "[clang-query] continue even if files are skipped" to "[clang-query] continue querying even if files are skipped". Lekensteyn added a comment. Changes: - Return 1 (instead of 0) if none of the files could be

[PATCH] D54149: [Analyzer] [WIP] Standard C++ library functions checker for the std::find() family

baloghadamsoftware created this revision. baloghadamsoftware added a reviewer: NoQ. baloghadamsoftware added a project: clang. Herald added subscribers: donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, whisperity. Herald added a reviewer: george.karpenkov. Repository: rC Cla

[PATCH] D54148: [NFC][Clang][Aarch64] Add missing test file

dnsampaio created this revision. dnsampaio added a reviewer: olista01. Herald added subscribers: cfe-commits, kristof.beyls, javed.absar. The commit https://reviews.llvm.org/rL345273 by @LukeCheeseman has a missing test file, see https://reviews.llvm.org/D51429. This patch adds the missing test f

[PATCH] D52034: [Clang] Add options -Xclang -coverage-filter and -Xclang -coverage-exclude to filter the files to instrument with gcov

calixte updated this revision to Diff 172752. calixte added a comment. Change options names to -fprofile-exclude-files & -fprofile-filter-files and add some doc in UserManual. Repository: rC Clang https://reviews.llvm.org/D52034 Files: docs/UsersManual.rst include/clang/Driver/Options.t

[PATCH] D53701: [Analyzer] Record and process comparison of symbols instead of iterator positions in interator checkers

baloghadamsoftware added a comment. In https://reviews.llvm.org/D53701#1288258, @NoQ wrote: > Mmm, is it possible to detect adapters and inline them as an exception from > the rule? You can foresee a pretty complicated system of rules and exceptions > if we go down this path, but i believe that

[PATCH] D54152: [OpenCL] Fix diagnostic message about overload candidates

AlexeySachkov created this revision. AlexeySachkov added reviewers: asavonic, Anastasia. Herald added a subscriber: yaxunl. I wonder if there are some extension which need to be disabled to get overloadable candidate available. Repository: rC Clang https://reviews.llvm.org/D54152 Files: in

[PATCH] D52034: [Clang] Add options -fprofile-filter-files and -fprofile-exclude-files to filter the files to instrument with gcov

calixte updated this revision to Diff 172761. calixte added a comment. Fix plural form of regex and just use ';' as separator. Repository: rC Clang https://reviews.llvm.org/D52034 Files: docs/UsersManual.rst include/clang/Driver/Options.td include/clang/Frontend/CodeGenOptions.h lib/

[PATCH] D41005: Reuse preamble even if an unsaved file does not exist

nik added a comment. Coming back to this one, I see a failing test: PCHPreambleTest.ReparseWithOverriddenFileDoesNotInvalidatePreamble Note that PCHPreambleTest.ReparseWithOverriddenFileDoesNotInvalidatePreamble references the header paths in different ways ("//./header1.h" vs "//./foo/../head

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

hokein added a comment. Thanks for the patch and nice improvements. Some initial thoughts: - The output of clang-tidy diagnostic is YAML, and YAML is not an space-efficient format (just for human readability). If you want to save space further, we might consider using some compressed formats,

[PATCH] D52998: [benchmark] Disable exceptions in Microsoft STL

This revision was automatically updated to reflect the committed changes. Closed by commit rL346237: [benchmark] Disable exceptions in Microsoft STL (authored by eandrews, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D52998?vs=172171

[PATCH] D54156: [CodeComplete] Do not complete self-initializations

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added a subscriber: arphaman. Removes references to initialized variable from the following completions: int x = ^; Handles only the trivial cases where the variable name is completed immediately at the star

[PATCH] D54156: [CodeComplete] Do not complete self-initializations

ilya-biryukov updated this revision to Diff 172773. ilya-biryukov added a comment. - Remove std::move, the target is const ref, so it does nothing (thanks, clang-tidy!) Repository: rC Clang https://reviews.llvm.org/D54156 Files: lib/Sema/SemaCodeComplete.cpp test/CodeCompletion/ordinary

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

steveire added a comment. This feature seems like a good idea. I started writing it too some months ago, but then I changed tactic and worked on distributing the refactor over the network instead. As far as I know, your deduplication would not work with a distributed environment. However, it s

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth added a comment. > - The output of clang-tidy diagnostic is YAML, and YAML is not an > space-efficient format (just for human readability). If you want to save > space further, we might consider using some compressed formats, e.g. > llvm::bitcode. Given the reduced YAML result (5.4MB)

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth added a comment. Thank you for the comment! In https://reviews.llvm.org/D54141#1288809, @steveire wrote: > This feature seems like a good idea. I started writing it too some months > ago, but then I changed tactic and worked on distributing the refactor over > the network instead. As

[PATCH] D54152: [OpenCL] Fix diagnostic message about overload candidates

Anastasia accepted this revision. Anastasia added a comment. This revision is now accepted and ready to land. LGTM! Thanks! Repository: rC Clang https://reviews.llvm.org/D54152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

steveire added a comment. In https://reviews.llvm.org/D54141#1288818, @JonasToth wrote: > Thank you for the comment! > > In https://reviews.llvm.org/D54141#1288809, @steveire wrote: > > > This feature seems like a good idea. I started writing it too some months > > ago, but then I changed tactic

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth added a comment. > Maybe my suggestion was not clear. The yaml file generated by clang-tidy > contains not only replacements, but all diagnostics, even without a fixit. > > So, running `clang-apply-replacements --issue-diags the_new_file.yaml` would > issue the warnings/fixit hints by

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

gchatelet updated this revision to Diff 172778. gchatelet marked 7 inline comments as done. gchatelet added a comment. - Addressing comments and enhancing diagnostics Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53488 Files: clang-tidy/cppcoreguidelines/NarrowingConversions

[PATCH] D54157: [clangd] [NFC] Fix clang-tidy warnings.

kadircet created this revision. kadircet added reviewers: ioeric, sammccall, ilya-biryukov, hokein. Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54157 Files: clangd/index/FileIndex.cpp unittests/clangd/Backg

[PATCH] D54077: [clangd] Implemented DraftFileSystem

sammccall added a comment. In https://reviews.llvm.org/D54077#1288455, @LutsenkoDanil wrote: > I already made a patch which introduces such behavior (not uploaded here > yet), and looks like it works well with draft fs: diagnostics updates for > depended files in 'real-time' on typing for opene

[PATCH] D54156: [CodeComplete] Do not complete self-initializations

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. :-) Repository: rC Clang https://reviews.llvm.org/D54156 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/c

[PATCH] D54157: [clangd] [NFC] Fix clang-tidy warnings.

JonasToth accepted this revision. JonasToth added a comment. This revision is now accepted and ready to land. Fixing such warnings is ok without prior review. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54157 ___ cfe-commits mail

[PATCH] D52421: [Sema] Diagnose parameter names that shadow inherited field names

lebedev.ri added a comment. And now i'm having concerns. struct Base { void* f; }; struct Inherit : Base { static void func(void* f) { // <- does 'f' *actually* shadow the 'f' in the 'Base'? You can't access that non-static member variable from static function. } };

[PATCH] D53488: [clang-tidy] Catching narrowing from double to float.

JonasToth added a comment. I think the check is really close. If the other reviewers want to take a look at it again, now is a good moment :) Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:178 + return; +// Conversions to unsigned integer are we

[PATCH] D52421: [Sema] Diagnose parameter names that shadow inherited field names

aaron.ballman added a comment. In https://reviews.llvm.org/D52421#1288910, @lebedev.ri wrote: > And now i'm having concerns. > > struct Base { > void* f; > }; > > struct Inherit : Base { > static void func(void* f) { // <- does 'f' *actually* shadow the 'f' in > the 'Base'? Y

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

steveire added a comment. In https://reviews.llvm.org/D54141#1288851, @JonasToth wrote: > > So, running `clang-apply-replacements --issue-diags the_new_file.yaml` > > would issue the warnings/fixit hints by processing the yaml and issuing the > > diagnostics the way clang-tidy would have done (

Re: r342827 - Fix modules build with shared library.

Shuai - have you had a chance to look at this? On Mon, Oct 22, 2018 at 4:43 PM Richard Smith wrote: > On Mon, 22 Oct 2018 at 14:57, David Blaikie via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Richard - any further thoughts here (re: layering/dependencies, etc)? >> Would love to get

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth added a comment. > Do you understand the proposal now? Yes better, I was under the impression that `clang-apply-replaments` is run on the end and the YAMLs are kept until then. Now its clear. I assume `--issue-diags` produce the same result as the normal diagnostic engine. That could

[PATCH] D52421: [Sema] Diagnose parameter names that shadow inherited field names

lebedev.ri added a comment. In https://reviews.llvm.org/D52421#1288917, @aaron.ballman wrote: > In https://reviews.llvm.org/D52421#1288910, @lebedev.ri wrote: > > > ... > > > ... Ok, thank you for reassuring me that it is working as intended. https://reviews.llvm.org/D52421 ___

Re: r342827 - Fix modules build with shared library.

Sorry for the unresponsiveness, I've been fire fighting for other stuffs. I'll look into this and try to get it fixed this week. On Tue, Nov 6, 2018 at 9:52 AM David Blaikie wrote: > Shuai - have you had a chance to look at this? > > On Mon, Oct 22, 2018 at 4:43 PM Richard Smith > wrote: > >> O

[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

rjmccall added a comment. In https://reviews.llvm.org/D53738#1288372, @ebevhan wrote: > In https://reviews.llvm.org/D53738#1288333, @rjmccall wrote: > > > In https://reviews.llvm.org/D53738#1287123, @ebevhan wrote: > > > > > In https://reviews.llvm.org/D53738#1284213, @rjmccall wrote: > > > > > >

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

Eugene.Zelenko added a comment. Reducing log file size is good idea, but I think will be also good idea to count duplicates. This will allow to concentrate clean-up efforts on place where most of warnings originate. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54141 _

[PATCH] D54161: [AST] Pack DeclRefExpr, UnaryOperator, MemberExpr and BinaryOperator

riccibruno created this revision. riccibruno added a reviewer: rsmith. riccibruno added a project: clang. Herald added a subscriber: cfe-commits. Move some data to the newly available space in the bit-fields of `Stmt`. This cuts the size of `DeclRefExpr`, `UnaryOperator`, `MemberExpr` and `BinaryO

[PATCH] D54162: OpenCL: Don't warn on v printf modifier

arsenm created this revision. arsenm added a reviewer: Anastasia. Herald added subscribers: yaxunl, wdng. This avoids spurious warnings, but could use a lot of work. For example the number of vector elements is not verified, and the passed value type is not checked. https://reviews.llvm.org/D541

[PATCH] D54162: OpenCL: Don't warn on v printf modifier

Anastasia accepted this revision. Anastasia added a comment. This revision is now accepted and ready to land. LGTM! Thanks! Seems like a reasonable start. https://reviews.llvm.org/D54162 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:/

[PATCH] D54166: [AST] Store the string data in StringLiteral in a trailing array of chars

riccibruno created this revision. riccibruno added a reviewer: rsmith. riccibruno added a project: clang. Herald added a subscriber: cfe-commits. Use the newly available space in the bit-fields of `Stmt` and store the string data in a trailing array of `char`s after the trailing array of `SourceLo

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth added a comment. In https://reviews.llvm.org/D54141#1288993, @Eugene.Zelenko wrote: > Reducing log file size is good idea, but I think will be also good idea to > count duplicates. This will allow to concentrate clean-up efforts on place > where most of warnings originate. Places th

[PATCH] D54168: [clang-tidy] Zircon fbl::move -> std::move

juliehockett created this revision. juliehockett added reviewers: aaron.ballman, alexfh, hokein. juliehockett added a project: clang-tools-extra. Herald added subscribers: xazax.hun, mgorny. Adds a check to convert fbl::move to std::move. This check is part of a set of migration checks as we prep

[PATCH] D54169: [clang-tidy] Zircon ->

juliehockett created this revision. juliehockett added reviewers: aaron.ballman, alexfh, hokein. juliehockett added a project: clang-tools-extra. Herald added subscribers: xazax.hun, mgorny. Adds a check to convert to std . This check is part of a set of migration checks as we prepare to move Zi

[PATCH] D54168: [clang-tidy] Zircon fbl::move -> std::move

JonasToth added a comment. I think this check is ok in the current form, but does it really need to be in upstream clang-tidy? You said its for migration only, so it is not valuable for you for a long time either? Comment at: clang-tools-extra/clang-tidy/zircon/FblMoveCheck.h

[PATCH] D54171: [MS] Zero out ECX in __cpuid in intrin.h

rnk created this revision. rnk added reviewers: craig.topper, hans. Some CPUID leafs depend on the value of ECX as well as EAX, but we left it uninitialized. Originally reported as https://crbug.com/901547 https://reviews.llvm.org/D54171 Files: clang/lib/Headers/intrin.h clang/test/CodeGen

[PATCH] D54172: [AST] Pack ArraySubscriptExpr and CallExpr

riccibruno created this revision. riccibruno added a reviewer: rsmith. riccibruno added a project: clang. Herald added a subscriber: cfe-commits. Use the newly available space in the bit-fields of `Stmt` to store some data from `ArraySubscriptExpr` and `CallExpr`. This saves a pointer for each of

[PATCH] D54169: [clang-tidy] Zircon ->

mcgrathr accepted this revision. mcgrathr added a comment. This revision is now accepted and ready to land. lgtm https://reviews.llvm.org/D54169 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/c

[PATCH] D49736: [Basic] Emit warning flag suggestion only in case there's existing flag *similar* to the unknown one

jkorous requested review of this revision. jkorous added a comment. I tried to move the getNearestOption() to it's only client - EmitUnknownDiagWarning() but it turned out to be a significant change because of clang/Basic/DiagnosticGroups.inc use in DiagnosticIDs.cpp. I suggest we leave that for

[PATCH] D54171: [MS] Zero out ECX in __cpuid in intrin.h

craig.topper accepted this revision. craig.topper added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D54171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/li

r346265 - [MS] Zero out ECX in __cpuid in intrin.h

Author: rnk Date: Tue Nov 6 12:45:26 2018 New Revision: 346265 URL: http://llvm.org/viewvc/llvm-project?rev=346265&view=rev Log: [MS] Zero out ECX in __cpuid in intrin.h Summary: Some CPUID leafs depend on the value of ECX as well as EAX, but we left it uninitialized. Originally reported as htt

[PATCH] D54171: [MS] Zero out ECX in __cpuid in intrin.h

This revision was automatically updated to reflect the committed changes. Closed by commit rL346265: [MS] Zero out ECX in __cpuid in intrin.h (authored by rnk, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D54171?vs=172811&id=172831#t

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

steveire added a comment. In https://reviews.llvm.org/D54141#1288930, @JonasToth wrote: > > Do you understand the proposal now? > > Yes better, I was under the impression that `clang-apply-replaments` is run > on the end and the YAMLs are kept until then. Now its clear. > I assume `--issue-diag

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth added a comment. In https://reviews.llvm.org/D54141#1288930, @JonasToth wrote: > > Do you understand the proposal now? > > Yes better, I was under the impression that `clang-apply-replaments` is run > on the end and the YAMLs are kept until then. Now its clear. > I assume `--issue-dia

[PATCH] D53329: Generate DIFile with main program if source is not available

dblaikie added a comment. In https://reviews.llvm.org/D53329#1270035, @yonghong-song wrote: > Sure. Let me provide a little bit more context and what I want to achieve: > > . I have a tool, called bcc (https://github.com/iovisor/bcc) which uses > clang CompilerInvocation interface and > MC

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

JonasToth added a comment. > At least the clang-tidy quiet mode is trivial to implement. Maybe instead of > `--quiet` we could have `--stdout=` where `output_format` can > be one of `none`, `diag`, `yaml` and in the future possibly `json` (requested > here: http://lists.llvm.org/pipermail/cfe-d

r346266 - Don't use std::next() on an input iterator; NFC.

Author: aaronballman Date: Tue Nov 6 13:12:44 2018 New Revision: 346266 URL: http://llvm.org/viewvc/llvm-project?rev=346266&view=rev Log: Don't use std::next() on an input iterator; NFC. Instead, advance the old-fashioned way, as std::next() cannot be used on an input iterator until C++17. Mod

[PATCH] D54169: [clang-tidy] Zircon ->

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/zircon/FblLimitsCheck.cpp:111 + // Add in the header, since we know this file uses it. + if (auto IncludeFixit = Inserter->CreateIncludeInsertion( + SM.getFileID(V->getLocation()), "limits"

[PATCH] D54168: [clang-tidy] Zircon fbl::move -> std::move

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/zircon/FblMoveCheck.cpp:61 +// Add in the header. +if (auto IncludeFixit = +Inserter->CreateIncludeInsertion(SM.getFileID(Start), "utility", Please don't use auto

[PATCH] D54169: [clang-tidy] Zircon ->

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/zircon/FblLimitsCheck.cpp:47 +SrcMgr::CharacteristicKind FileType) { + if (FileName == "fbl/limits.h") { +unsigned End = std::strcspn(SM.getCharacterData(HashLoc), "\n") + 1; D

[PATCH] D53153: [OpenCL] Mark kernel functions with default visibility

scott.linder added a comment. I don't believe that is currently the case (the unrestricted linking of OCL code to OCL code via a dynamic linker), but we do have the notion of a static link step, followed by dynamic linking at runtime. The static link step is currently via IR, but we plan to sup

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

steveire added a comment. In https://reviews.llvm.org/D54141#1289326, @JonasToth wrote: > > That said, would you agree to have the parser-based deduplication as an > developer-only optin solution for now? :) If you're suggesting proceeding with this regex based solution, I don't think that

  1   2   >