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 if you don't feel conf
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 __ha
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 confident submitting fixe
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 e
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 ).
Chan
Author: wizard
Date: Sun Apr 22 18:05:02 2018
New Revision: 330562
URL: http://llvm.org/viewvc/llvm-project?rev=330562&view=rev
Log:
update readability-identifier-naming-objc test to use interface ivar.
Implementation ivars are not supported in 32-bits OS.
Reviewers: alexfh, chandlerc
Subscribe
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 bot, but about the best y
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 about objective-c, or any
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 th
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 Y
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 O
Author: mgrang
Date: Sun Apr 22 17:49:25 2018
New Revision: 330561
URL: http://llvm.org/viewvc/llvm-project?rev=330561&view=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 hel
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
http://lab.llvm.org:8011/builders/c
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
==
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
https://reviews.llvm.org/D
Author: wizard
Date: Sun Apr 22 17:15:15 2018
New Revision: 330559
URL: http://llvm.org/viewvc/llvm-project?rev=330559&view=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.
Revi
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
test/clang-tidy/readability-identifier-naming-
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 direc
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
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
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
___
cfe-
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
http://lists.llvm.org/cgi-
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:
cfe/trunk/bindings
Author: jbcoe
Date: Sun Apr 22 13:51:05 2018
New Revision: 330557
URL: http://llvm.org/viewvc/llvm-project?rev=330557&view=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,
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
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commit
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 a
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 te
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.
> `tools/clang/
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
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 substitution
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: clang/test/CodeGenCXX/lambda-expressions-i
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 `GenericTaintC
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
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
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi
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
+
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 s
> 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 г. в 10:11,
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
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
__
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? Nothing configuration speci
40 matches
Mail list logo