Re: [clang-tools-extra] r330559 - update test to use ivar in implementation instead of class extension

Did not see any failure related to https://reviews.llvm.org/D45936 yet after submission. Best regards Yan Zhang > On Apr 22, 2018, at 18:13, Chandler Carruth wrote: > > I won't be back at a computer for a while and I really don't know anything > about objective-c... But

[PATCH] D30882: Add a callback for __has_include and use it for dependency scanning

dexonsmith requested changes to this revision. dexonsmith added a comment. This revision now requires changes to proceed. Herald added a subscriber: kbarton. I don't think this is quite right. I know at least `make`-based incremental builds wouldn't deal well with this. Given t.cpp: #if

Re: [clang-tools-extra] r330559 - update test to use ivar in implementation instead of class extension

Sure. Will do. The change I am trying is pretty trivial and only limited to the test itself. On Sun, Apr 22, 2018 at 6:13 PM Chandler Carruth wrote: > I won't be back at a computer for a while and I really don't know anything > about objective-c... But if you don't feel

Re: [clang-tools-extra] r330559 - update test to use ivar in implementation instead of class extension

I won't be back at a computer for a while and I really don't know anything about objective-c... But if you don't feel confident submitting fixes with post commit review, you should probably just revert If the fix is trivial and/or you can find ways to test it a similar suggested in my earlier

[PATCH] D45936: update readability-identifier-naming-objc test to use interface ivar. Implementation ivars are not supported in 32-bits OS.

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rCTE330562: update readability-identifier-naming-objc test to use interface ivar. (authored by Wizard, committed by ).

[clang-tools-extra] r330562 - update readability-identifier-naming-objc test to use interface ivar. Implementation ivars are not supported in 32-bits OS.

Author: wizard Date: Sun Apr 22 18:05:02 2018 New Revision: 330562 URL: http://llvm.org/viewvc/llvm-project?rev=330562=rev Log: update readability-identifier-naming-objc test to use interface ivar. Implementation ivars are not supported in 32-bits OS. Reviewers: alexfh, chandlerc Subscribers:

Re: [clang-tools-extra] r330559 - update test to use ivar in implementation instead of class extension

Need a accept for that revision. Can you accept it? On Sun, Apr 22, 2018 at 6:02 PM Chandler Carruth wrote: > See my other email -- you can compile code targeting other platforms > regardless of the platform you develop on. Not exactly as good as > reproducing it with a

Re: [clang-tools-extra] r330559 - update test to use ivar in implementation instead of class extension

See my other email -- you can compile code targeting other platforms regardless of the platform you develop on. Not exactly as good as reproducing it with a bot, but about the best you have. On Sun, Apr 22, 2018 at 6:01 PM Chandler Carruth wrote: > I don't know anything

Re: [clang-tools-extra] r330559 - update test to use ivar in implementation instead of class extension

I don't know anything about objective-c, or anything about OSX. However, I guarantee these bots aren't using 32-bit OSX. ;] Look at the bot names. They're running Linux in various flavors: ppc64be, ppc64le, armv8, etc. My suspicion is that this is a linux-specific issue. But you can reproduce

Re: [clang-tools-extra] r330559 - update test to use ivar in implementation instead of class extension

btw due to the lack of way to testing it on bot environments, I am not sure if specify fobjc-abo-version=2 could solve the problem (could it break builds on 32-bit machines or just skip them). So I played safe to use the old way of declaring ivars in the revision. On Sun, Apr 22, 2018 at 5:55 PM

Re: [clang-tools-extra] r330559 - update test to use ivar in implementation instead of class extension

I am running tests locally with "ninja check-clang-tools" and I am sure it is running this test because I could get error message when I debug it. The problem (according to the error message) is all caused by different architecture. It seems a lot of ObjC features are not supported in old 32-bit

r330561 - [XRay] Change std::sort to llvm::sort in response to r327219

Author: mgrang Date: Sun Apr 22 17:49:25 2018 New Revision: 330561 URL: http://llvm.org/viewvc/llvm-project?rev=330561=rev Log: [XRay] Change std::sort to llvm::sort in response to r327219 r327219 added wrappers to std::sort which randomly shuffle the container before sorting. This will help in

Re: [clang-tools-extra] r330559 - update test to use ivar in implementation instead of class extension

The commit log here no longer reflects the commit. This is not just updating the test, this is a complete re-application of the original patch in r330492. =[ Also, the bots are still complaining: http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/17830

[PATCH] D45936: update readability-identifier-naming-objc test to use interface ivar. Implementation ivars are not supported in 32-bits OS.

Wizard created this revision. Herald added subscribers: cfe-commits, klimek. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45936 Files: test/clang-tidy/readability-identifier-naming-objc.m Index: test/clang-tidy/readability-identifier-naming-objc.m

[PATCH] D45912: update test to use ivar in implementation instead of class extension

This revision was automatically updated to reflect the committed changes. Closed by commit rL330559: update test to use ivar in implementation instead of class extension (authored by Wizard, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM

[clang-tools-extra] r330559 - update test to use ivar in implementation instead of class extension

Author: wizard Date: Sun Apr 22 17:15:15 2018 New Revision: 330559 URL: http://llvm.org/viewvc/llvm-project?rev=330559=rev Log: update test to use ivar in implementation instead of class extension Summary: using ivar in class extension is not supported in 32-bit architecture of MacOS.

[PATCH] D45912: update test to use ivar in implementation instead of class extension

Wizard updated this revision to Diff 143494. Wizard added a comment. add back objc update for identifier naming check. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45912 Files: clang-tidy/readability/IdentifierNamingCheck.cpp

[PATCH] D45505: [WIP] [GCC] Match a GCC version with a patch suffix without a third version component

chandlerc added a comment. In https://reviews.llvm.org/D45505#1074747, @mstorsjo wrote: > In https://reviews.llvm.org/D45505#1074744, @chandlerc wrote: > > > Definitely need tests here. > > > Do you have any suggestion on where to create them and what kind? Just a > normal test with a fake

[PATCH] D45766: [Sema] Add -Wno-self-assign-overloaded

lebedev.ri added a comment. Ping. At least one of these needs to land. Repository: rC Clang https://reviews.llvm.org/D45766 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45505: [WIP] [GCC] Match a GCC version with a patch suffix without a third version component

mstorsjo added a comment. In https://reviews.llvm.org/D45505#1074744, @chandlerc wrote: > Definitely need tests here. Do you have any suggestion on where to create them and what kind? Just a normal test with a fake directory tree, or some sort of unit test for the gcc version number matching

[PATCH] D45505: [WIP] [GCC] Match a GCC version with a patch suffix without a third version component

chandlerc requested changes to this revision. chandlerc added a comment. This revision now requires changes to proceed. Definitely need tests here. But no concern w/ the idea of this. Repository: rC Clang https://reviews.llvm.org/D45505 ___

[PATCH] D45505: [WIP] [GCC] Match a GCC version with a patch suffix without a third version component

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

[PATCH] D45671: [python bindings] Fix Cursor.result_type for ObjC method declarations - Bug 36677

This revision was automatically updated to reflect the committed changes. Closed by commit rL330557: [python bindings] Fix Cursor.result_type for ObjC method declarations - Bug… (authored by jbcoe, committed by ). Repository: rL LLVM https://reviews.llvm.org/D45671 Files:

r330557 - [python bindings] Fix Cursor.result_type for ObjC method declarations - Bug 36677

Author: jbcoe Date: Sun Apr 22 13:51:05 2018 New Revision: 330557 URL: http://llvm.org/viewvc/llvm-project?rev=330557=rev Log: [python bindings] Fix Cursor.result_type for ObjC method declarations - Bug 36677 Summary: In cindex.py, Cursor.result_type called into the wrong libclang function,

[PATCH] D45671: [python bindings] Fix Cursor.result_type for ObjC method declarations - Bug 36677

jbcoe added a comment. Congrats on your first patch! I can commit this for you. Repository: rC Clang https://reviews.llvm.org/D45671 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45932: [clang-tidy][modernize-raw-string-literal] Don't replace upper ASCII with raw literals

zinovy.nis added inline comments. Comment at: test/clang-tidy/modernize-raw-string-literal.cpp:42 char const *const HexNonPrintable("\\\x03"); char const *const Delete("\\\177"); +char const *const MultibyteSnowman("\xE2\x98\x83"); By the way, AFAIK the lines

[PATCH] D45932: [clang-tidy][modernize-raw-string-literal] Don't replace upper ASCII with raw literals

zinovy.nis created this revision. zinovy.nis added reviewers: xazax.hun, LegalizeAdulthood. zinovy.nis added a project: clang-tools-extra. Herald added subscribers: cfe-commits, rnkovacs. It's useless and not safe to replace `"\xE2\x98\x83"` with `"☃"` (snowman) Especially there's an explicit

[PATCH] D45718: [analyzer] Allow registering custom statically-linked analyzer checkers

alexfh added a comment. In https://reviews.llvm.org/D45718#1074611, @george.karpenkov wrote: > Actually, we do have unittests in `tools/clang/unittest/Analysis`. > @alexfh would you be able to write a unittest in there? It should be fairly > easy following the structure of e.g. >

[PATCH] D45931: [ASTMatchers] Don't garble the profiling output when multiple TU's are processed

lebedev.ri created this revision. lebedev.ri added reviewers: sbenza, alexfh, klimek. https://reviews.llvm.org/D5911 added support for profiling clang-tidy checks. It works nice, the tabulated report generated by `clang-tidy -enable-check-profile` is readable. Unfortunately, it gets complicated

[PATCH] D45451: [ItaniumMangle] Undeduced auto type doesn't belong in the substitution table

erik.pilkington added inline comments. Comment at: clang/lib/AST/ItaniumMangle.cpp:2342 + if (isa(Ty)) +return false; return true; rjmccall wrote: > rjmccall wrote: > > rjmccall wrote: > > > I agree with your analysis that this shouldn't be a

[PATCH] D45451: [ItaniumMangle] Undeduced auto type doesn't belong in the substitution table

erik.pilkington updated this revision to Diff 143469. erik.pilkington added a comment. Add a comment. https://reviews.llvm.org/D45451 Files: clang/lib/AST/ItaniumMangle.cpp clang/test/CodeGenCXX/lambda-expressions-inside-auto-functions.cpp Index:

[PATCH] D45682: [analyzer] Move `TaintBugVisitor` from `GenericTaintChecker.cpp` to `BugReporterVisitors.h`.

george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. > can add extra notes like Ah right, than it's "can rely on" rather than "rely on". > it is necessary to explicitly include I've meant adding the include to

[PATCH] D45682: [analyzer] Move `TaintBugVisitor` from `GenericTaintChecker.cpp` to `BugReporterVisitors.h`.

MTC added a comment. In https://reviews.llvm.org/D45682#1074407, @george.karpenkov wrote: > I'm new to the taint visitor, but I am quite confused by your change > description. > > > and many checkers rely on it > > How can other checkers rely on it if it's private to the taint checker? Thanks

[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

george.karpenkov added a comment. `antipatterns` or `security` could be another potential category name. https://reviews.llvm.org/D45532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

george.karpenkov added a comment. The code looks good apart from a few minor nits. I think I would prefer a new category created for this checker instead of using `alpha`; let's see what @NoQ has to say. Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:305 +

[PATCH] D45718: [analyzer] Allow registering custom statically-linked analyzer checkers

george.karpenkov requested changes to this revision. george.karpenkov added a comment. This revision now requires changes to proceed. Actually, we do have unittests in `tools/clang/unittest/Analysis`. @alexfh would you be able to write a unittest in there? It should be fairly easy following the

Re: [PATCH] D45927: [clang-tidy] [modernize-use-auto] Correct way to calculate a type name length for multi-token types

> I think spaces that will be removed should be counted - long long is 9. I thought about it, but what about "long long int ** * *"? Is it still 9? I think it's a business of clang-format to remove excessive spaces, not of clang-tidy. вс, 22 апр. 2018 г. в

[PATCH] D45927: [clang-tidy] [modernize-use-auto] Correct way to calculate a type name length for multi-token types

zinovy.nis added a subscriber: malcolm.parsons. zinovy.nis added a comment. > I think spaces that will be removed should be counted - long long is 9. I thought about it, but what about "long long int - * * *"? Is it still 9? I think it's a business of clang-format to

[PATCH] D45927: [clang-tidy] [modernize-use-auto] Correct way to calculate a type name length for multi-token types

malcolm.parsons added a comment. I think spaces that will be removed should be counted - long long is 9. OTOH, MinTypeNameLength isn't likely to be set high enough for that to matter. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45927

Re: [clang-tools-extra] r330492 - [clang-tidy] add new check to find out objc ivars which do not have prefix '_'

Btw is there any way to test on bots before submission? Ninja tests already works on my local machine Best regards Yan Zhang > On Apr 21, 2018, at 18:15, Chandler Carruth wrote: > > Should be able to reproduce it by patching it in and running the tests > yourself?

[PATCH] D45927: [clang-tidy] [modernize-use-auto] Correct way to calculate a type name length for multi-token types

zinovy.nis created this revision. zinovy.nis added reviewers: malcolm.parsons, alexfh. zinovy.nis added a project: clang-tools-extra. Herald added subscribers: cfe-commits, xazax.hun. This patch is a fix for https://reviews.llvm.org/D45405 where spaces were also considered as a part of a type