Re: [PATCH] D13549: Added new options to ClangFormat VSIX package.

2015-10-15 Thread Zachary Turner via cfe-commits
zturner added a comment. No obvious problems, mostly just style issues. Feel free to consider them optional. Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:38 @@ +37,3 @@ +private bool sortIncludes = false; +private string style = "file";

Re: [PATCH] D13549: Added new options to ClangFormat VSIX package.

2015-10-16 Thread Zachary Turner via cfe-commits
zturner added a comment. In http://reviews.llvm.org/D13549#268701, @curdeius wrote: > Hi Zachary, just to answer your comments. I have done it on purpose not to > use enum, because clang-format style can be actually a JSON string, e.g. > `{BasedOnStyle: "LLVM", IndentWidth: 4}`, so it wouldn't

Re: [PATCH] D13549: Added new options to ClangFormat VSIX package.

2015-10-16 Thread Zachary Turner via cfe-commits
zturner added a comment. Oh I see. Makes sense then. http://reviews.llvm.org/D13549 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D13276: Don't adjust field offsets for external record layouts

2015-09-29 Thread Zachary Turner via cfe-commits
zturner created this revision. zturner added a reviewer: majnemer. zturner added a subscriber: cfe-commits. injecting the VBPtr always behaves this way, but injecting the VFPtr was not. This patch fixes that. http://reviews.llvm.org/D13276 Files: lib/AST/RecordLayoutBuilder.cpp Index:

Re: [PATCH] D13276: Don't adjust field offsets for external record layouts

2015-10-01 Thread Zachary Turner via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL249085: Don't adjust field offsets when using external record layout. (authored by zturner). Changed prior to commit: http://reviews.llvm.org/D13276?vs=36054=36310#toc Repository: rL LLVM

Re: [PATCH] D10370: clang-format: Implement AlwaysBreakAfterDeclarationReturnType.

2015-12-11 Thread Zachary Turner via cfe-commits
zturner added a subscriber: zturner. zturner added a comment. What needs to happen for this to go in? If I understand correctly, it is either: 1. Add a new option `TreatDeclarationsLikeDefinitions` 2. Merge this option into `AlwaysBreakAfterDefinitionReturnType` and make it an enum with 5

Re: [PATCH] D10370: clang-format: Implement AlwaysBreakAfterDeclarationReturnType.

2015-12-17 Thread Zachary Turner via cfe-commits
zturner added a comment. ping http://reviews.llvm.org/D10370 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D10370: clang-format: Implement AlwaysBreakAfterDeclarationReturnType.

2015-12-15 Thread Zachary Turner via cfe-commits
zturner updated this revision to Diff 42895. zturner added a comment. Attempt to address remaining issues in patch. This is my first time touching anything having to do with clang, so there's a good chance I don't know what i'm doing and this needs more work. Let me know.

Re: [PATCH] D10370: clang-format: Implement AlwaysBreakAfterDeclarationReturnType.

2015-12-14 Thread Zachary Turner via cfe-commits
zturner updated this revision to Diff 42774. zturner added a comment. This patch was very old, so it didn't apply against ToT. So this initial update is just to rebase against ToT. I will make changes in a followup patch. http://reviews.llvm.org/D10370 Files: include/clang/Format/Format.h

Re: [PATCH] D10370: clang-format: Implement AlwaysBreakAfterDeclarationReturnType.

2015-12-18 Thread Zachary Turner via cfe-commits
zturner added inline comments. Comment at: lib/Format/TokenAnnotator.h:168 @@ -158,1 +167,3 @@ + bool mustBreakForReturnType(const AnnotatedLine , + FormatToken ) const; djasper wrote: > Some comment might help. E.g. at the very

r256055 - Fix invalid enum comparison.

2015-12-18 Thread Zachary Turner via cfe-commits
Author: zturner Date: Fri Dec 18 16:58:42 2015 New Revision: 256055 URL: http://llvm.org/viewvc/llvm-project?rev=256055=rev Log: Fix invalid enum comparison. Modified: cfe/trunk/unittests/Format/FormatTest.cpp Modified: cfe/trunk/unittests/Format/FormatTest.cpp URL:

r256046 - Support AlwaysBreakAfterReturnType

2015-12-18 Thread Zachary Turner via cfe-commits
Author: zturner Date: Fri Dec 18 16:20:15 2015 New Revision: 256046 URL: http://llvm.org/viewvc/llvm-project?rev=256046=rev Log: Support AlwaysBreakAfterReturnType This changes the behavior of AlwaysBreakAfterDeclarationReturnType so that it supports breaking after declarations, definitions, or

Re: [PATCH] D21256: Improved Visual Studio 2015 visualization of SmallVectorImpl

2016-06-13 Thread Zachary Turner via cfe-commits
I agree that it can be annoying to say "hey guys, i would normally do post commit review on this, but i wanted to give the courtesy of a heads up", and then potentially waiting an indeterminate amount of time. I think that actually discourages these kind of changes going up at all, because people

Re: [PATCH] D18498: Auto-install Clang Visual Studio visualizers for VS2015 and up

2016-03-27 Thread Zachary Turner via cfe-commits
For vs2015, the files still need to be in the project right? (In the vcxproj with a tag) On Sun, Mar 27, 2016 at 9:09 AM Mike Spertus wrote: > mspertus updated this revision to Diff 51741. > mspertus added a comment. > > Apply whitespace comments from

Re: [PATCH] D18498: Auto-install Clang Visual Studio visualizers for VS2015 and up

2016-03-27 Thread Zachary Turner via cfe-commits
Code paths. For vs2013 you're installing the natvis files, can we just do that for vs2015 as well? On Sun, Mar 27, 2016 at 11:23 AM Mike Spertus wrote: > mspertus added a comment. > > What do you mean by "making both paths the same"? > > > http://reviews.llvm.org/D18498 > > > >

Re: [PATCH] D18498: Auto-install Clang Visual Studio visualizers for VS2015 and up

2016-03-27 Thread Zachary Turner via cfe-commits
Ahh that's surprising. If it works even with the none tag, i guess my concerns are not valid On Sun, Mar 27, 2016 at 12:11 PM Mike Spertus wrote: > mspertus added a comment. > > No. Testing shows that it works fine in the project with the `` tag. > VS2015 empirically looks at

Re: [PATCH] D18498: Auto-install Clang Visual Studio visualizers for VS2015 and up

2016-03-27 Thread Zachary Turner via cfe-commits
Yea, CMake doesn't actually support natvis files. You'd have to actually patch CMake to make it work. Can you make the vs2015 and vs2013 paths the same? It will still work for 2015 i think, just that 2015 also supports putting them in the project file On Sun, Mar 27, 2016 at 10:04 AM Mike Spertus

Re: [PATCH] D18498: Auto-install Clang Visual Studio visualizers for VS2015 and up

2016-03-27 Thread Zachary Turner via cfe-commits
If it's not using a natvis tag in the vcxproj (which it isn't since CMake doesn't support it during generation) then the new natvis project file support won't work right? In which case the files would need to be copied into documents\vs2015, otherwise it won't work, unless I'm misunderstanding

Re: [PATCH] D18498: Auto-install Clang Visual Studio visualizers for VS2015 and up

2016-03-27 Thread Zachary Turner via cfe-commits
Yea sounds fine On Sun, Mar 27, 2016 at 12:20 PM Mike Spertus wrote: > mspertus added a comment. > > I understand your concerns, but on balance, I don't see a better course of > action than what I've done. If I have some time, I'll submit a CMake change > so we can eventually

Re: [PATCH] D17908: Add Visual Studio Visualizers for more Clang types

2016-03-07 Thread Zachary Turner via cfe-commits
Better yet, just delete the number On Mon, Mar 7, 2016 at 6:21 AM Aaron Ballman wrote: > aaron.ballman added inline comments. > > > Comment at: clang.natvis:1-3 > @@ -1,4 +1,4 @@ > > Do you know if these visualizers will work in MSVC 2013 as well, or

Re: [PATCH] D17908: Add Visual Studio Visualizers for more Clang types

2016-03-07 Thread Zachary Turner via cfe-commits
zturner added a subscriber: zturner. zturner added a comment. Better yet, just delete the number http://reviews.llvm.org/D17908 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17908: Add Visual Studio Visualizers for more Clang types

2016-03-07 Thread Zachary Turner via cfe-commits
zturner added a comment. Natvis has hardly changed at all since it was introduced, so I'm guessing the lowest common denominator is always going to be our minimum required VS version http://reviews.llvm.org/D17908 ___ cfe-commits mailing list

Re: [PATCH] D17908: Add Visual Studio Visualizers for more Clang types

2016-03-07 Thread Zachary Turner via cfe-commits
Natvis has hardly changed at all since it was introduced, so I'm guessing the lowest common denominator is always going to be our minimum required VS version On Mon, Mar 7, 2016 at 7:09 AM Aaron Ballman wrote: > aaron.ballman added a comment. > > In

Re: [PATCH] D21946: Subject: [PATCH] [Driver] fix windows SDK detect

2016-07-28 Thread Zachary Turner via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL277005: [Driver] Fix Windows SDK Detection (authored by zturner). Changed prior to commit: https://reviews.llvm.org/D21946?vs=62570=65958#toc Repository: rL LLVM https://reviews.llvm.org/D21946

r277005 - [Driver] Fix Windows SDK Detection

2016-07-28 Thread Zachary Turner via cfe-commits
Author: zturner Date: Thu Jul 28 12:13:32 2016 New Revision: 277005 URL: http://llvm.org/viewvc/llvm-project?rev=277005=rev Log: [Driver] Fix Windows SDK Detection This fixes a couple of bugs in Windows SDK Detection. 1. `readFullStringValue` returns a bool, but was being compared with

Re: [PATCH] D21946: Subject: [PATCH] [Driver] fix windows SDK detect

2016-07-28 Thread Zachary Turner via cfe-commits
Committed in r277005 On Thu, Jul 28, 2016 at 9:38 AM Zachary Turner wrote: > Sorry for the delay, I had forgotten about this. I will check it in today. > > On Wed, Jul 27, 2016 at 5:57 PM comicfans44 wrote: > >> comicfans44 added a comment. >> >> In

Re: [PATCH] D23409: Make clang-tidy work with clang-cl

2016-08-11 Thread Zachary Turner via cfe-commits
zturner added inline comments. Comment at: lib/Tooling/JSONCompilationDatabase.cpp:119 @@ -115,1 +118,3 @@ StringRef EscapedCommandLine) { +#if defined(LLVM_ON_WIN32) + llvm::BumpPtrAllocator Alloc; rnk wrote: > It would be nice if the JSON file just told

[PATCH] D23409: Make clang-tidy work with clang-cl

2016-08-11 Thread Zachary Turner via cfe-commits
zturner created this revision. zturner added reviewers: alexfh, djasper, rnk. zturner added a subscriber: cfe-commits. Herald added a subscriber: klimek. Attempting to run clang-tidy with the clang-cl driver results in a number of warnings and errors that make the tool unusable. The two main

Re: [PATCH] D23409: Make clang-tidy work with clang-cl

2016-08-11 Thread Zachary Turner via cfe-commits
zturner added inline comments. Comment at: lib/Driver/Driver.cpp:93 @@ +92,3 @@ + ArrayRef Args) { + auto Default = ToolChain::getTargetAndModeFromProgramName(ProgramName); + StringRef DefaultMode(Default.second); rnk wrote: > Why

Re: [PATCH] D23409: Make clang-tidy work with clang-cl

2016-08-11 Thread Zachary Turner via cfe-commits
zturner added a comment. In https://reviews.llvm.org/D23409#512720, @aaron.ballman wrote: > Patch generally LGTM, though I wonder if there's a way we can add test > coverage for this. I'd imagine we can just enable the clang-tidy test suite on Windows (I'm assuming it's currently disabled).

Re: [PATCH] D23409: Make clang-tidy work with clang-cl

2016-08-11 Thread Zachary Turner via cfe-commits
zturner added a comment. In https://reviews.llvm.org/D23409#512745, @zturner wrote: > In https://reviews.llvm.org/D23409#512720, @aaron.ballman wrote: > > > Patch generally LGTM, though I wonder if there's a way we can add test > > coverage for this. > > > I'd imagine we can just enable the

Re: [PATCH] D23409: Make clang-tidy work with clang-cl

2016-08-11 Thread Zachary Turner via cfe-commits
zturner added a comment. In https://reviews.llvm.org/D23409#513017, @alexfh wrote: > May I ask you to upload patches with full diff context next time? I know, > it's not directly supported by TortoiseGit, but there are at least two other > reasonable ways of generating full diffs for

Re: [PATCH] D23409: Make clang-tidy work with clang-cl

2016-08-11 Thread Zachary Turner via cfe-commits
zturner updated this revision to Diff 67751. zturner added a comment. I added a test for the clang Driver changes. It seems I'm still having some trouble writing the clang-cl test for clang-tidy. It depends on having a different compilation database that uses clang-cl instead of clang. I

Re: [PATCH] D23455: [Tooling] Parse compilation database command lines properly on Windows

2016-08-13 Thread Zachary Turner via cfe-commits
I'm not disagreeing that it would be nice if CMake supported this. It's probably worth trying to do even. But do we want to block having a working clang-tidy for clang-cl for months because of that? Even if we can upstream the patch, how long before we up the minimum required CMake version? The

Re: [PATCH] D23455: [Tooling] Parse compilation database command lines properly on Windows

2016-08-13 Thread Zachary Turner via cfe-commits
According to the existing spec [ http://clang.llvm.org/docs/JSONCompilationDatabase.html], the command field "must be a valid command to rerun the exact compilation step for the translation unit in the environment the build system uses.". So copying compilation databases across environments with

Re: [PATCH] D23455: [Tooling] Parse compilation database command lines properly on Windows

2016-08-13 Thread Zachary Turner via cfe-commits
Also, compilation database support with CMake works correctly on Windows. It generates Windows command lines which need to be tokenized using Windows command line rules (hence why this patch makes clang-tidy work on Windows) On Sat, Aug 13, 2016 at 10:30 PM Zachary Turner

Re: [PATCH] D23455: [Tooling] Parse compilation database command lines properly on Windows

2016-08-14 Thread Zachary Turner via cfe-commits
I'll try and figure out who that was on Monday and loop them in. I'm not sure what problems the previous person ran into, but the test suite passes, i can run it on a large codebase, and all the results look fine and as if the tool is working. Hopefully the previous person has more insight into

Re: [PATCH] D23455: [Tooling] Parse compilation database command lines properly on Windows

2016-08-14 Thread Zachary Turner via cfe-commits
It occurred to me that eol detection won't work reliably. Especially for tests, someone will write a compilation database by hand and check it in. Maybe they have git set to convert newlines for them, and we don't want a test to behave differently based on your source control settings. The target

Re: [PATCH] D23434: Don't allow llvm-include-order to intermingle includes from different files.

2016-08-12 Thread Zachary Turner via cfe-commits
Neither does clang-tidy's right? Incidentally that's exactly what i was trying to add support for. Lldb has a mass reformat coming up, and as part of that we are defining an include ordering. But at the moment it won't do this, so we will be left fixing this by hand or not fixing it at all On

Re: [PATCH] D23455: [Tooling] Parse compilation database command lines properly on Windows

2016-08-12 Thread Zachary Turner via cfe-commits
zturner updated this revision to Diff 67947. zturner added a comment. After much head banging I think I finally figured this out. I'm putting this back to my original revision, unchanged. The problem is not the original patch. The original patch is perfectly fine. The problem is the way i

[PATCH] D23476: Allow manual specification of the binary directory in check_clang_tidy.py

2016-08-12 Thread Zachary Turner via cfe-commits
zturner created this revision. zturner added reviewers: alexfh, djasper. zturner added a subscriber: cfe-commits. When you run the script from lit it starts in the right directory so it can find `FileCheck` and `clang-tidy`, but when you manually run `check-clang-tidy.py` from a command line it

[PATCH] D23480: Add a test for clang-tidy using the clang cl driver

2016-08-12 Thread Zachary Turner via cfe-commits
zturner created this revision. zturner added a reviewer: alexfh. zturner added a subscriber: cfe-commits. In order to get this to work, the positional arguments must use `--driver-mode=cl `, and NOT `clang-cl `. In the latter case, clang-tidy has to try really hard to "guess" whether the first

Re: [PATCH] D23434: Don't allow llvm-include-order to intermingle includes from different files.

2016-08-12 Thread Zachary Turner via cfe-commits
one disadvantage to clang format is that you have less control over how and whether to apply fixits. Reordering across blocks has a higher risk of breaking code, and you can't tell clang format to only apply fixits which don't break code, or to not apply any fixits but just warn. Code duplication

Re: [PATCH] D23434: Don't allow llvm-include-order to intermingle includes from different files.

2016-08-12 Thread Zachary Turner via cfe-commits
That's actually the reason I think it makes more sense in clang tidy. It can be a configuration option, off by default, and since there is more control over whether to apply fixits, and it doesn't apply fixits by default, it would be easier to iterate on the experimental nature of it without

Re: [PATCH] D23434: Don't allow llvm-include-order to intermingle includes from different files.

2016-08-12 Thread Zachary Turner via cfe-commits
You and daniel both imply that clang-tidy can sort across blocks. Am I missing this somewhere? My intention was to add an option for this in a followup patch because it doesn't seem to be able to currently. On Fri, Aug 12, 2016 at 8:21 AM Alexander Kornienko wrote: > alexfh

Re: [PATCH] D23455: [Tooling] Parse compilation database command lines properly on Windows

2016-08-13 Thread Zachary Turner via cfe-commits
The json is generated by CMake, so I don't thinks we can do this without patching CMake, which is a non-starter. I don't think this will break mingw. Mingw is still Windows, and still uses Windows backslashes, quoting rules, and escaping rules. You might be thinking of cygwin, but in the case

Re: [PATCH] D23476: Allow manual specification of the binary directory in check_clang_tidy.py

2016-08-13 Thread Zachary Turner via cfe-commits
I was trying to rerun the lit commands manually to diagnose a failing test. This all works fine from lit, but if you want to run check_clang_tidy.py from the command line, this is convenient On Sat, Aug 13, 2016 at 10:27 AM Alexander Kornienko wrote: > alexfh added a comment.

Re: [PATCH] D23455: [Tooling] Parse compilation database command lines properly on Windows

2016-08-13 Thread Zachary Turner via cfe-commits
The only thing that won't work is generating a compilation database on one machine and physically copying it to another machine, but I don't think we should worry about that. We can try to submit a request to CMake to get more info in the compilation database, but something like that is months

[PATCH] D23454: [Driver] Set the default clang driver mode based on the executable

2016-08-12 Thread Zachary Turner via cfe-commits
zturner created this revision. zturner added a reviewer: rnk. zturner added a subscriber: cfe-commits. If `--driver-mode` is passed in one or more times, it will overwrite the default by this patch. This patch only makes it so that if NO `--driver-mode` is passed in, we try to do better than

Re: [PATCH] D23434: Don't allow llvm-include-order to intermingle includes from different files.

2016-08-12 Thread Zachary Turner via cfe-commits
Ahh, I see. Just to be clear, is there an LGTM to get this path in? I know alexfh@ lgtm'ed it, want to make sure you're ok with this too. On Fri, Aug 12, 2016 at 9:40 AM Daniel Jasper wrote: > The check's implementation will be replaced by a simple call to clang > tidy. It

Re: [PATCH] D23409: Make clang-tidy work with clang-cl

2016-08-12 Thread Zachary Turner via cfe-commits
zturner abandoned this revision. zturner added a comment. Once I split the JSONCompilationDatabase patch out, then the rest is strictly a driver-only patch, and the other one is strictly a tooling patch. So I will abandon this one and upload two new patches with a more targeted set of

Re: [PATCH] D23434: Don't allow llvm-include-order to intermingle includes from different files.

2016-08-12 Thread Zachary Turner via cfe-commits
Sounds good. Just to be clear, you plan to delete the code from clang-tidy, then take the code from clang-format and move it to clang-tidy, and have clang-format call clang-tidy (or otherwise share the code somehow so they both use the same implementation)? I may still try to implement

r278535 - [Driver] Set the default driver mode based on the executable.

2016-08-12 Thread Zachary Turner via cfe-commits
Author: zturner Date: Fri Aug 12 12:47:52 2016 New Revision: 278535 URL: http://llvm.org/viewvc/llvm-project?rev=278535=rev Log: [Driver] Set the default driver mode based on the executable. Currently, if --driver-mode is not passed at all, it will default to GCC style driver. This is never an

Re: [PATCH] D23454: [Driver] Set the default clang driver mode based on the executable

2016-08-12 Thread Zachary Turner via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL278535: [Driver] Set the default driver mode based on the executable. (authored by zturner). Changed prior to commit: https://reviews.llvm.org/D23454?vs=67853=67861#toc Repository: rL LLVM

[clang-tools-extra] r278546 - Analyze include order on a per-file basis.

2016-08-12 Thread Zachary Turner via cfe-commits
Author: zturner Date: Fri Aug 12 13:38:26 2016 New Revision: 278546 URL: http://llvm.org/viewvc/llvm-project?rev=278546=rev Log: Analyze include order on a per-file basis. The include order check would get notified of all include directives in a depth-first manner. This created the possibility

[clang-tools-extra] r278547 - Remove accidentally committed file.

2016-08-12 Thread Zachary Turner via cfe-commits
Author: zturner Date: Fri Aug 12 13:39:05 2016 New Revision: 278547 URL: http://llvm.org/viewvc/llvm-project?rev=278547=rev Log: Remove accidentally committed file. Removed: clang-tools-extra/trunk/test/clang-tidy/clang-cl-driver.cpp Removed:

Re: [PATCH] D23434: Don't allow llvm-include-order to intermingle includes from different files.

2016-08-12 Thread Zachary Turner via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL278546: Analyze include order on a per-file basis. (authored by zturner). Changed prior to commit: https://reviews.llvm.org/D23434?vs=6=67876#toc Repository: rL LLVM

[PATCH] D23434: Don't allow llvm-include-order to intermingle includes from different files.

2016-08-11 Thread Zachary Turner via cfe-commits
zturner created this revision. zturner added reviewers: alexfh, djasper. zturner added a subscriber: cfe-commits. See llvm.org/pr28943 for some context. I'm not sure if this is the right fix, but it seems to pass the test suite for me. https://reviews.llvm.org/D23434 Files:

Re: [PATCH] D23434: Don't allow llvm-include-order to intermingle includes from different files.

2016-08-11 Thread Zachary Turner via cfe-commits
zturner updated this revision to Diff 67772. zturner added a comment. Fixed the test for this. Confirmed that it fails before the patch and succeeds after the patch. https://reviews.llvm.org/D23434 Files: clang-tidy/llvm/IncludeOrderCheck.cpp test/clang-tidy/Inputs/Headers/cross-file-a.h

Re: [PATCH] D23434: Don't allow llvm-include-order to intermingle includes from different files.

2016-08-11 Thread Zachary Turner via cfe-commits
zturner updated this revision to Diff 6. zturner added a comment. Seems like dropping all non-main-file include directives is wrong because then it won't be able to detect errors in "non user code". Seems like the real fix is to bucket the include directives by file in which they were

Re: [PATCH] D23455: [Tooling] Parse compilation database command lines properly on Windows

2016-08-12 Thread Zachary Turner via cfe-commits
zturner updated this revision to Diff 67915. zturner added a comment. Update with changes needed to make fixed compilation database work (which also breaks many other tests) https://reviews.llvm.org/D23455 Files: include/clang/Tooling/CompilationDatabase.h

Re: [PATCH] D23455: [Tooling] Parse compilation database command lines properly on Windows

2016-08-12 Thread Zachary Turner via cfe-commits
zturner added a comment. This is starting to get fairly difficult, and I'm afraid it may require someone more knowledgable about clang-format's internals than me. I wrote a test to have it use a fixed compilation database, and I was able to make that test pass, but it breaks many other tests.

Re: [PATCH] D21946: Subject: [PATCH] [Driver] fix windows SDK detect

2016-07-01 Thread Zachary Turner via cfe-commits
zturner added a comment. In http://reviews.llvm.org/D21946#473070, @comicfans44 wrote: > I've not commited to cfe before, so I think I havn't commit access permission. If you've committed anywhere in LLVM, you should have commit access to cfe. Feel free to give it a try, as I won't be able

Re: [PATCH] D23618: [LibTooling] Allow compilation database to explicitly specify compilation database file and source root

2016-08-17 Thread Zachary Turner via cfe-commits
zturner updated this revision to Diff 68404. zturner added a comment. Added full context diff. https://reviews.llvm.org/D23618 Files: include/clang/Tooling/CompilationDatabase.h include/clang/Tooling/JSONCompilationDatabase.h lib/Tooling/CommonOptionsParser.cpp

r278964 - [Tooling] Parse compilation database command lines on Windows.

2016-08-17 Thread Zachary Turner via cfe-commits
Author: zturner Date: Wed Aug 17 15:04:35 2016 New Revision: 278964 URL: http://llvm.org/viewvc/llvm-project?rev=278964=rev Log: [Tooling] Parse compilation database command lines on Windows. When a compilation database is used on Windows, the command lines cannot be parsed using the standard

Re: [PATCH] D23455: [Tooling] Parse compilation database command lines properly on Windows

2016-08-17 Thread Zachary Turner via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL278964: [Tooling] Parse compilation database command lines on Windows. (authored by zturner). Changed prior to commit: https://reviews.llvm.org/D23455?vs=67947=68407#toc Repository: rL LLVM

Re: [PATCH] D23455: [Tooling] Parse compilation database command lines properly on Windows

2016-08-17 Thread Zachary Turner via cfe-commits
zturner added a comment. We could also provide a command line option to clang-tidy that lets the user specify the command line syntax of the compilation database. It could choose a smart default (i.e. what we do in this patch after using `llvm::sys::getProcessTriple()`) but if the command

[PATCH] D23618: [LibTooling] Allow compilation database to explicitly specify compilation database file and source root

2016-08-17 Thread Zachary Turner via cfe-commits
zturner created this revision. zturner added reviewers: djasper, alexfh, klimek. zturner added a subscriber: cfe-commits. Herald added a subscriber: klimek. Allow explicit specification of a compilation database file and source root. While trying to create a compilation database test for D23455,

[clang-tools-extra] r278968 - Add a test for clang-tidy using the clang-cl driver.

2016-08-17 Thread Zachary Turner via cfe-commits
Author: zturner Date: Wed Aug 17 15:14:10 2016 New Revision: 278968 URL: http://llvm.org/viewvc/llvm-project?rev=278968=rev Log: Add a test for clang-tidy using the clang-cl driver. Reviewed By: alexfh Differential Revision: https://reviews.llvm.org/D23480 Added:

Re: [PATCH] D23480: Add a test for clang-tidy using the clang cl driver

2016-08-17 Thread Zachary Turner via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL278968: Add a test for clang-tidy using the clang-cl driver. (authored by zturner). Changed prior to commit: https://reviews.llvm.org/D23480?vs=67948=68413#toc Repository: rL LLVM

[clang-tools-extra] r278977 - Revert "[Include-fixer] Install executables and support scripts"

2016-08-17 Thread Zachary Turner via cfe-commits
Author: zturner Date: Wed Aug 17 15:56:47 2016 New Revision: 278977 URL: http://llvm.org/viewvc/llvm-project?rev=278977=rev Log: Revert "[Include-fixer] Install executables and support scripts" This reverts commit b725a314a9b7f746c37f70909ec3c4dcb6d9f6b5. The patch that made this test work

r278976 - Revert "[Tooling] Parse compilation database command lines on Windows."

2016-08-17 Thread Zachary Turner via cfe-commits
Author: zturner Date: Wed Aug 17 15:55:35 2016 New Revision: 278976 URL: http://llvm.org/viewvc/llvm-project?rev=278976=rev Log: Revert "[Tooling] Parse compilation database command lines on Windows." This reverts commit 27a874790fc79f6391ad3703d7c790f51ac6ae1f. After the introduction of

[clang-tools-extra] r278978 - Revert "Revert "[Include-fixer] Install executables and support scripts""

2016-08-17 Thread Zachary Turner via cfe-commits
Author: zturner Date: Wed Aug 17 15:58:14 2016 New Revision: 278978 URL: http://llvm.org/viewvc/llvm-project?rev=278978=rev Log: Revert "Revert "[Include-fixer] Install executables and support scripts"" This reverts commit 2aff596257e1c45fa54baae823ecbe61a785174e. I'm having a bad day

[clang-tools-extra] r278979 - Revert "Add a test for clang-tidy using the clang-cl driver."

2016-08-17 Thread Zachary Turner via cfe-commits
Author: zturner Date: Wed Aug 17 16:01:13 2016 New Revision: 278979 URL: http://llvm.org/viewvc/llvm-project?rev=278979=rev Log: Revert "Add a test for clang-tidy using the clang-cl driver." This reverts commit fd1908ce445eba4544d64cc68b3c03249e4bf614. This should be the correct CL to revert.

Re: Upgrade and fix clang-format-vs

2016-08-18 Thread Zachary Turner via cfe-commits
The key.snk is generated when you build, the problem is the csproj file hardcodes the directory to the sdk instead of using the appropriate project system variable like $(SDKToolsDir) On Thu, Aug 18, 2016 at 7:09 PM Zachary Turner wrote: > Llvm doesn't support vs2012 anymore,

Re: Upgrade and fix clang-format-vs

2016-08-18 Thread Zachary Turner via cfe-commits
Llvm doesn't support vs2012 anymore, as long as it supports vs2013 it's fine On Thu, Aug 18, 2016 at 7:07 PM Antonio Maiorano wrote: > Hi, > > What I meant by upgrade was simply making it build in VS 2015. However, > you bring up a valid point about making sure the extension

[PATCH] D23533: [clang-tidy] Rewrite compilation database test to not require shell

2016-08-15 Thread Zachary Turner via cfe-commits
zturner created this revision. zturner added reviewers: alexfh, klimek, djasper. zturner added a subscriber: cfe-commits. This allows the test to run on windows, and also makes writing compilation database tests easier in general. See D23532 for more information.

[PATCH] D23532: [LibTooling] Allow compilation database to explicitly specify compilation database file and source root

2016-08-15 Thread Zachary Turner via cfe-commits
zturner created this revision. zturner added a reviewer: cfe-commits. zturner added subscribers: klimek, alexfh, djasper. Allow explicit specification of a compilation database file and source root. While trying to create a compilation database test for D23455, I ran into the problem that

Re: [PATCH] D23455: [Tooling] Parse compilation database command lines properly on Windows

2016-08-16 Thread Zachary Turner via cfe-commits
zturner added a comment. In https://reviews.llvm.org/D23455#516753, @alexfh wrote: > In https://reviews.llvm.org/D23455#515527, @rnk wrote: > > > In https://reviews.llvm.org/D23455#515486, @brad.king wrote: > > > > > > the feasibility of emitting 'arguments' instead of 'command' into the > > >

[PATCH] D23628: Fix unittests after windows command line parsing

2016-08-17 Thread Zachary Turner via cfe-commits
zturner created this revision. zturner added a reviewer: alexfh. zturner added a subscriber: cfe-commits. Herald added a subscriber: klimek. After submitting the windows command line parsing patch, it broke all the command line parsing unittests, because now they were trying parse gnu command

Re: [PATCH] D23618: [LibTooling] Allow compilation database to explicitly specify compilation database file and source root

2016-08-17 Thread Zachary Turner via cfe-commits
Strange, I thought i did. Will re upload On Wed, Aug 17, 2016 at 12:14 PM Alexander Kornienko wrote: > alexfh added a comment. > > Full context diffs, please ( > http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface > ). > > >

Re: [PATCH] D23848: Add a clang-tidy Visual Studio extension

2016-08-24 Thread Zachary Turner via cfe-commits
zturner added a comment. BTW, since I forgot to do so in the original review, here is a screenshot of what the UI for the property editor looks like in this extension: F2328405: clang-tidy.png Comment at:

Re: [PATCH] D23848: Add a clang-tidy Visual Studio extension

2016-08-24 Thread Zachary Turner via cfe-commits
zturner added inline comments. Comment at: clang-tidy-vs/ClangTidy/ClangTidyProperties.cs:82 @@ +81,3 @@ +public bool CERTDCL50 +{ +get { return GetInheritableProperty("CERTDCL50").Value; } Are the .rst files in the repo somewhere

Re: [PATCH] D23848: Add a clang-tidy Visual Studio extension

2016-08-24 Thread Zachary Turner via cfe-commits
zturner added inline comments. Comment at: clang-tidy-vs/ClangTidy/ClangTidyProperties.cs:82 @@ +81,3 @@ +public bool CERTDCL50 +{ +get { return GetInheritableProperty("CERTDCL50").Value; } alexfh wrote: > zturner wrote: > > Are the

Re: [PATCH] D23848: Add a clang-tidy Visual Studio extension

2016-08-31 Thread Zachary Turner via cfe-commits
zturner added a comment. I am planning to submit this today if there are no further suggestions. https://reviews.llvm.org/D23848 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [clang-tools-extra] r280839 - Resubmit "Add a test for clang-tidy using the clang-cl driver."

2016-09-08 Thread Zachary Turner via cfe-commits
un from other paths: /tmp/, /Volumes/). > Does that make sense? Any suggestions on how to fix? > > Thanks! > -Ahmed > > > On Wed, Sep 7, 2016 at 11:28 AM, Zachary Turner via cfe-commits > <cfe-commits@lists.llvm.org> wrote: > > Author: zturner > > Date:

Re: [clang-tools-extra] r280839 - Resubmit "Add a test for clang-tidy using the clang-cl driver."

2016-09-08 Thread Zachary Turner via cfe-commits
gt; My hypothesis is that the %s path ('/Users/...') is somehow > >> interpreted as a '/U' by the clang-cl driver, causing the cryptic > >> 'Error while processing /Users/.../clang-cl-driver.cpp' error (as the > >> test succeeds when run from other paths: /tmp/, /Volumes/). >

Re: Upgrade and fix clang-format-vs

2016-09-15 Thread Zachary Turner via cfe-commits
You may need to install the Visual Studio SDK. Did you do that when you initially installed VS 2015? On Thu, Sep 15, 2016 at 4:15 PM Hans Wennborg wrote: > Well, on my machine $(SDKToolsDir) doesn't work :-( I suspect the file > will need manual tweaking by whoever is trying

Re: Upgrade and fix clang-format-vs

2016-09-15 Thread Zachary Turner via cfe-commits
Strange. FWIW you can dump all the variables that are present in your environment. You need to go to Tools -> Options -> Projects and Solutions -> Build and Run and choose either Normal, Detailed, or Diagnostic for the MSBuild project build output verbosity. Then in the output window you will

Re: [llvm-dev] Upgrading phabricator

2016-09-29 Thread Zachary Turner via cfe-commits
You mentioned this was for an upgrade. Are there any major new features or bugfixes to be aware of? On Thu, Sep 29, 2016 at 9:26 PM Eric Liu via llvm-commits < llvm-comm...@lists.llvm.org> wrote: > Hi all, > > Phabricator is (finally) back online! Let me know if you have any feedback > or problem

r281998 - Add some entropy to the folder name in Driver/warning-options.cpp.

2016-09-20 Thread Zachary Turner via cfe-commits
Author: zturner Date: Tue Sep 20 13:41:19 2016 New Revision: 281998 URL: http://llvm.org/viewvc/llvm-project?rev=281998=rev Log: Add some entropy to the folder name in Driver/warning-options.cpp. It was trying to check that things behave correctly when a non-existant folder was specified for

r282004 - Fix a regex error breaking tests.

2016-09-20 Thread Zachary Turner via cfe-commits
Author: zturner Date: Tue Sep 20 14:10:56 2016 New Revision: 282004 URL: http://llvm.org/viewvc/llvm-project?rev=282004=rev Log: Fix a regex error breaking tests. Modified: cfe/trunk/test/Driver/warning-options.cpp Modified: cfe/trunk/test/Driver/warning-options.cpp URL:

[clang-tools-extra] r280839 - Resubmit "Add a test for clang-tidy using the clang-cl driver."

2016-09-07 Thread Zachary Turner via cfe-commits
Author: zturner Date: Wed Sep 7 13:28:42 2016 New Revision: 280839 URL: http://llvm.org/viewvc/llvm-project?rev=280839=rev Log: Resubmit "Add a test for clang-tidy using the clang-cl driver." This was originally reverted because the patch on the clang tooling side was reverted. That patch is

Re: [PATCH] D23848: Add a clang-tidy Visual Studio extension

2016-09-07 Thread Zachary Turner via cfe-commits
The unnecessary usings are added by Visual Studio, so I would rather leave them. For example, they often provide extension methods to containers so that containers can be used with functional paradigms, and you expect to see those methods in the Intellisense window when you type "Foo.", but

[clang-tools-extra] r280844 - Fix a few oversights in the clang-tidy VS plugin.

2016-09-07 Thread Zachary Turner via cfe-commits
Author: zturner Date: Wed Sep 7 14:41:19 2016 New Revision: 280844 URL: http://llvm.org/viewvc/llvm-project?rev=280844=rev Log: Fix a few oversights in the clang-tidy VS plugin. Over-zealous cleanup of using statements removed some that were actually needed. Also cleaned up a few warnings.

r279120 - Resubmit "[Tooling] Parse compilation database command lines on Windows."

2016-08-18 Thread Zachary Turner via cfe-commits
Author: zturner Date: Thu Aug 18 14:31:48 2016 New Revision: 279120 URL: http://llvm.org/viewvc/llvm-project?rev=279120=rev Log: Resubmit "[Tooling] Parse compilation database command lines on Windows." This patch introduced the ability to decide at runtime whether to parse JSON compilation

Re: [PATCH] D23628: Fix unittests after windows command line parsing

2016-08-18 Thread Zachary Turner via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL279120: Resubmit "[Tooling] Parse compilation database command lines on Windows." (authored by zturner). Changed prior to commit: https://reviews.llvm.org/D23628?vs=68426=68593#toc Repository: rL

r279122 - Fix json compilation database syntax on non-Windows.

2016-08-18 Thread Zachary Turner via cfe-commits
Author: zturner Date: Thu Aug 18 14:42:00 2016 New Revision: 279122 URL: http://llvm.org/viewvc/llvm-project?rev=279122=rev Log: Fix json compilation database syntax on non-Windows. Modified: cfe/trunk/lib/Tooling/JSONCompilationDatabase.cpp Modified:

Re: [PATCH] D23848: Add a clang-tidy Visual Studio extension

2016-08-25 Thread Zachary Turner via cfe-commits
zturner added a comment. In https://reviews.llvm.org/D23848#525485, @Eugene.Zelenko wrote: > I think will be good idea to mention this plugin in docs/ReleaseNotes.rst. I was planning to wait until it's "finished". This patch will give you a user interface for editing .clang-tidy files, but

Re: [PATCH] D23848: Add a clang-tidy Visual Studio extension

2016-08-24 Thread Zachary Turner via cfe-commits
zturner added a comment. I can fix the empty lines, but keep in mind that Visual Studio's C# editor is MUCH more aggressive about auto-formatting your code. So it seems like a fruitless endeavor to me, as we will constantly be fighting against their auto formatter, which does this

  1   2   3   >