Merge lp:~zorba-coders/zorba/clang-friend into lp:zorba

Proposed by David Graf
Status: Rejected
Rejected by: David Graf
Proposed branch: lp:~zorba-coders/zorba/clang-friend
Merge into: lp:zorba
Diff against target: 58 lines (+3/-13)
2 files modified
src/runtime/collections/collections_impl.cpp (+2/-12)
src/util/string/rstring.h (+1/-1)
To merge this branch: bzr merge lp:~zorba-coders/zorba/clang-friend
Reviewer Review Type Date Requested Status
Till Westmann Needs Information
David Graf (community) Approve
Paul J. Lucas Pending
Review via email: mp+101518@code.launchpad.net

This proposal supersedes a proposal from 2012-04-05.

Description of the change

Added __llvm__ test because Apple LLVM compiler also does not recognize the + operator as a friend of zorba::rstring.
Additionally, fixed two warnings shown when compiling zorba with clang.

To post a comment you must log in.
Revision history for this message
Zorba Build Bot (zorba-buildbot) wrote : Posted in a previous version of this proposal

No approved revision specified.

Revision history for this message
Zorba Build Bot (zorba-buildbot) wrote : Posted in a previous version of this proposal
Revision history for this message
Zorba Build Bot (zorba-buildbot) wrote : Posted in a previous version of this proposal

Validation queue job clang-friend-2012-04-05T14-39-06.813Z is finished. The final status was:

All tests succeeded!

Revision history for this message
Zorba Build Bot (zorba-buildbot) wrote : Posted in a previous version of this proposal

Voting does not meet specified criteria. Required: Approve > 1, Disapprove < 1, Needs Fixing < 1, Pending < 1. Got: 2 Pending.

Revision history for this message
Till Westmann (tillw) : Posted in a previous version of this proposal
review: Approve
Revision history for this message
David Graf (davidagraf) : Posted in a previous version of this proposal
review: Approve
Revision history for this message
Zorba Build Bot (zorba-buildbot) wrote : Posted in a previous version of this proposal
Revision history for this message
Zorba Build Bot (zorba-buildbot) wrote : Posted in a previous version of this proposal
Revision history for this message
Zorba Build Bot (zorba-buildbot) wrote : Posted in a previous version of this proposal

Validation queue job clang-friend-2012-04-10T14-30-00.688Z is finished. The final status was:

All tests succeeded!

Revision history for this message
Zorba Build Bot (zorba-buildbot) wrote : Posted in a previous version of this proposal

Voting does not meet specified criteria. Required: Approve > 1, Disapprove < 1, Needs Fixing < 1, Pending < 1. Got: 2 Approve, 1 Pending.

Revision history for this message
David Graf (davidagraf) wrote : Posted in a previous version of this proposal

Added fixes to get rid of another two warnings when compiling zorba with clang.

Revision history for this message
David Graf (davidagraf) :
review: Approve
Revision history for this message
Till Westmann (tillw) wrote :

The changes look ok, but I'm not sure how to test.
I tried to build this branch using clang on MacOS X, but it doesn't build for me.
What exactly should work now that did not work before?

review: Needs Information
Revision history for this message
David Graf (davidagraf) wrote :

> The changes look ok, but I'm not sure how to test.
> I tried to build this branch using clang on MacOS X, but it doesn't build for
> me.
> What exactly should work now that did not work before?

Hello Till

Sorry for not answering earlier. The 'defined __llvm__' condition makes Zorba compiling in XCode with clang/llvm. The other stuff fixes warnings.

Revision history for this message
Till Westmann (tillw) wrote :

I tried to build on the command line by replacing /usr/bin/c++ with /usr/bin/llvm-g++ (and the same for cc), but that didn't work.
To build with XCode, is it sufficient to use CMake's XCode project generator?

Revision history for this message
David Graf (davidagraf) wrote :

Ghislain used cmake's xcode generator, yet. But it should also work if you just replace gcc with clang. I did this on linux.

Revision history for this message
Paul J. Lucas (paul-lucas) wrote :

If this merge is about __llvm__, why is there apparently unrelated stuff about collectionDecl in it?

Unmerged revisions

10747. By David Graf

fixed clang warnings by removing unused variables

10746. By Ghislain Fourny

Forgot defined keyword.

10745. By Ghislain Fourny

XCode's LLVM compiler also does not recognize the + operator as a zorba::rstring friend.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/runtime/collections/collections_impl.cpp'
--- src/runtime/collections/collections_impl.cpp 2012-04-08 21:39:25 +0000
+++ src/runtime/collections/collections_impl.cpp 2012-04-11 09:35:24 +0000
@@ -1627,7 +1627,6 @@
1627 PlanState& planState) const1627 PlanState& planState) const
1628{1628{
1629 store::Collection_t collection;1629 store::Collection_t collection;
1630 const StaticallyKnownCollection* collectionDecl;
1631 store::Item_t collectionName;1630 store::Item_t collectionName;
1632 store::Item_t numNodesItem;1631 store::Item_t numNodesItem;
1633 xs_integer numNodes = 1;1632 xs_integer numNodes = 1;
@@ -1640,11 +1639,7 @@
1640 if (!consumeNext(collectionName, theChildren[0].getp(), planState))1639 if (!consumeNext(collectionName, theChildren[0].getp(), planState))
1641 ZORBA_ASSERT(false);1640 ZORBA_ASSERT(false);
16421641
1643 collectionDecl = getCollection(1642 (void)getCollection(theSctx, collectionName, loc, theDynamicCollection, collection);
1644 theSctx, collectionName, loc, theDynamicCollection, collection);
1645
1646 /* added just to remove an unused variable warning in CMake */
1647 (void*)collectionDecl;
16481643
1649 if (theChildren.size() > 1)1644 if (theChildren.size() > 1)
1650 {1645 {
@@ -1751,7 +1746,6 @@
1751 PlanState& planState) const1746 PlanState& planState) const
1752{1747{
1753 store::Collection_t collection;1748 store::Collection_t collection;
1754 const StaticallyKnownCollection* collectionDecl;
1755 store::Item_t collectionName;1749 store::Item_t collectionName;
1756 store::Item_t numNodesItem;1750 store::Item_t numNodesItem;
1757 xs_integer numNodes = 1;1751 xs_integer numNodes = 1;
@@ -1763,11 +1757,7 @@
17631757
1764 consumeNext(collectionName, theChildren[0].getp(), planState);1758 consumeNext(collectionName, theChildren[0].getp(), planState);
17651759
1766 collectionDecl = getCollection(1760 (void)getCollection(theSctx, collectionName, loc, theDynamicCollection, collection);
1767 theSctx, collectionName, loc, theDynamicCollection, collection);
1768
1769 /* added just to remove an unused variable warning in CMake */
1770 (void*)collectionDecl;
17711761
1772 if (theChildren.size() > 1)1762 if (theChildren.size() > 1)
1773 {1763 {
17741764
=== modified file 'src/util/string/rstring.h'
--- src/util/string/rstring.h 2012-03-28 05:19:57 +0000
+++ src/util/string/rstring.h 2012-04-11 09:35:24 +0000
@@ -2051,7 +2051,7 @@
20512051
2052 string_data data_;2052 string_data data_;
20532053
2054#if defined _MSC_VER || defined CLANG2054#if defined _MSC_VER || defined CLANG || defined __llvm__
2055//2055//
2056// Microsoft's Visual Studio C++ compiler doesn't consider the operator+()2056// Microsoft's Visual Studio C++ compiler doesn't consider the operator+()
2057// functions as friends even though they're declared as such.2057// functions as friends even though they're declared as such.

Subscribers

People subscribed via source and target branches