[PATCH] D70368: [clang-tidy] Rewrite modernize-avoid-bind check

2019-11-21 Thread Zachary Turner via Phabricator via cfe-commits
zturner updated this revision to Diff 230495. zturner added a comment. - Updated documentation for this check - Incorporated additional suggestions from @aaron.ballman - Fixed an invalid transformation that was generated when binding a member function and the second argument of `bind` (the object

[PATCH] D70368: [clang-tidy] Rewrite modernize-avoid-bind check

2019-11-22 Thread Zachary Turner via Phabricator via cfe-commits
zturner updated this revision to Diff 230661. zturner added a comment. Addressed suggestions from @Eugene.Zelenko CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70368/new/ https://reviews.llvm.org/D70368 Files: clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp clang-tools-ex

[PATCH] D70368: [clang-tidy] Rewrite modernize-avoid-bind check

2019-12-02 Thread Zachary Turner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG64f74bf72eb4: [clang-tidy] Rewrite modernize-avoid-bind check. (authored by zturner). Changed prior to commit: https://reviews.llvm.org/D70368?vs=230661&id=231793#toc Repository: rG LLVM Github Monor

[PATCH] D70553: [clang-apply-replacements] Add command line option to overwrite readonly files.

2019-12-06 Thread Zachary Turner via Phabricator via cfe-commits
zturner added a comment. In D70553#1757862 , @aaron.ballman wrote: > Can you add a test case for this functionality? The reason there's no test case is because it seems like that wouldn't really be any different than testing the functionality of `llvm:

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

2020-03-06 Thread Zachary Turner via Phabricator via cfe-commits
zturner added a comment. If the current listed reviewers on this patch are not the best people for reviewing this, would one of them please suggest a more appropriate reviewer so we can get some traction on this? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69585/new/ https://reviews

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

2020-01-27 Thread Zachary Turner via Phabricator via cfe-commits
zturner added a comment. Adding a ping since it's been a week with no additional feedback for the author. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69585/new/ https://reviews.llvm.org/D69585 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D44778: [clang-format] Wildcard expansion on Windows.

2018-03-22 Thread Zachary Turner via Phabricator via cfe-commits
zturner added subscribers: alexfh, zturner. zturner added a comment. Never seen this PURE_WINDOWS CMake variable. How is it different than MSVC? Repository: rC Clang https://reviews.llvm.org/D44778 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D36347: New lldb python module for adding diagnostic breakpoints

2017-10-26 Thread Zachary Turner via Phabricator via cfe-commits
zturner added a comment. Do I understand correctly that this will insert breakpoints on *all* clang diagnostics? That's not necessarily bad, but I was under the impression originally that it would let you pick the diagnostics you wanted to insert breakpoints on. Also, What is the workflow for

[PATCH] D39994: Loosen MSVC 2017 path requirements

2017-11-15 Thread Zachary Turner via Phabricator via cfe-commits
zturner added a comment. I'm not suuuper opposed, but at the same time if this code is bothering people (and it is, consistently), I don't changing the requirements from "confusing rule A" to "confusing rule B" is going to solve the long term burden that people keep running into. Not asking yo

[PATCH] D30991: [Driver] Fix cross compiling with Visual Studio 2017

2017-03-15 Thread Zachary Turner via Phabricator via cfe-commits
zturner requested changes to this revision. zturner added inline comments. This revision now requires changes to proceed. Comment at: lib/Driver/ToolChains/MSVC.cpp:48-50 + // Undefine this macro so we can call the ANSI version of the function. + #undef GetEnvironmentStrings +

[PATCH] D30991: [Driver] Fix cross compiling with Visual Studio 2017

2017-03-15 Thread Zachary Turner via Phabricator via cfe-commits
zturner added inline comments. Comment at: lib/Driver/Job.cpp:312 SmallVector Argv; + auto Envp = Environment.size() > 1 +? const_cast(Environment.data()) Can you add `assert(Environment.back() == nullptr);`? Comment at: lib

[PATCH] D30991: [Driver] Fix cross compiling with Visual Studio 2017

2017-03-15 Thread Zachary Turner via Phabricator via cfe-commits
zturner added inline comments. Comment at: lib/Driver/ToolChains/MSVC.cpp:517 + } +} + SkipSettingEnvironment: zturner wrote: > I think you need to push 1 more null terminator onto the end here to > terminate the block. Actually if you use the `std::ti

[PATCH] D30991: [Driver] Fix cross compiling with Visual Studio 2017

2017-03-16 Thread Zachary Turner via Phabricator via cfe-commits
zturner added a comment. In https://reviews.llvm.org/D30991#702965, @thakis wrote: > When you say "cross-compiling", you mean targeting Windows while running on > non-Windows, right? How do dlls get loaded there at all? > > Also, when does clang invoke link.exe? Normally on Windows the linker is

[PATCH] D30991: [Driver] Fix cross compiling with Visual Studio 2017

2017-03-16 Thread Zachary Turner via Phabricator via cfe-commits
zturner added a comment. In https://reviews.llvm.org/D30991#702966, @zturner wrote: > In https://reviews.llvm.org/D30991#702965, @thakis wrote: > > > When you say "cross-compiling", you mean targeting Windows while running on > > non-Windows, right? How do dlls get loaded there at all? > > > > A

[PATCH] D30991: [Driver] Fix cross compiling with Visual Studio 2017

2017-03-16 Thread Zachary Turner via Phabricator via cfe-commits
zturner added a comment. Looks good with one more suggested fix. Comment at: include/clang/Driver/Job.h:129 + /// the given vector is to be copied in as opposed to moved. + void setEnvironment(const std::vector &NewEnvironment); + Since it's just a vector of

[PATCH] D30991: [Driver] Fix cross compiling with Visual Studio 2017

2017-03-16 Thread Zachary Turner via Phabricator via cfe-commits
zturner accepted this revision. zturner added a comment. This revision is now accepted and ready to land. lgtm, do you have commit access? https://reviews.llvm.org/D30991 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/

[PATCH] D30991: [Driver] Fix cross compiling with Visual Studio 2017

2017-03-17 Thread Zachary Turner via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL298098: [clang-cl] Fix cross-compilation with MSVC 2017. (authored by zturner). Changed prior to commit: https://reviews.llvm.org/D30991?vs=92050&id=92154#toc Repository: rL LLVM https://reviews.llv

[PATCH] D31696: Automatically add include-what-you-use for when building in tree

2017-04-04 Thread Zachary Turner via Phabricator via cfe-commits
zturner created this revision. Herald added subscribers: mgorny, rengolin, aemerson. It's a little bit annoying to have to manually edit files and then deal with git thinking that you've got untracked files, and thusly deleting them when you run `git clean -fd`. Although this is not an official

[PATCH] D31697: Check for null before using TUScope

2017-04-04 Thread Zachary Turner via Phabricator via cfe-commits
zturner created this revision. To be honest I don't really understand anything about this code. I encountered a situation when running `include-what-you-use` against LLVM where `TUScope` was null here, triggering a segfault. Some surrounding comments and code suggests that this variable is sp

[PATCH] D31696: Automatically add include-what-you-use for when building in tree

2017-04-06 Thread Zachary Turner via Phabricator via cfe-commits
zturner added reviewers: beanz, rnk, chandlerc. zturner added a comment. Not really sure who to add as a reviewer here, so + a few random people. BTW, kimgr@, is there any particular reason you haven't tried to upstream the tool? Maybe even as not a standalone tool but as a set of checks in `c

[PATCH] D34907: fix ODR violations due to abuse of LLVM_YAML_IS_(FLOW_)?SEQUENCE_VECTOR

2017-06-30 Thread Zachary Turner via Phabricator via cfe-commits
zturner accepted this revision. zturner added a comment. This revision is now accepted and ready to land. I don't have any comments, this seems fine to me. Long term I think the best solution is to get rid of these global specializations entirely, and instead provide adapters called `flow` and

<    1   2