diff mbox

Building for Clang is broken for snap-types

Message ID 13220e29-4827-c2c9-5c88-0f4a88c2c244@digiware.nl (mailing list archive)
State New, archived
Headers show

Commit Message

Willem Jan Withagen Oct. 20, 2016, 12:49 a.m. UTC
On 20-10-2016 02:21, Willem Jan Withagen wrote:
> On 19-10-2016 20:58, kefu chai wrote:
>> should have been fixed in master.
> 
> Ehh,
> 
> Not really.
> Just tested by running
> cd Ceph/master/ceph
> git pull
> ./do_freebsd.sh
> 
> And I still get the same error.
> 
> Guess I'll have to start bisecting to see where it went wrong.

This is the actual commit going wrong

commit c9c5235ef7d563b92f44dab63a8ac2b694e69d4f
Author: Sage Weil <sage@redhat.com>
Date:   Wed Sep 14 13:32:20 2016 -0400

    include/object: conditional denc_traits for snapid_t

    Signed-off-by: Sage Weil <sage@redhat.com>

   if (s == CEPH_NOSNAP)
     return out << "head";

But have not figure out why.
I'm going to give it some bedrest.

> --WjW
> 
> 
>> Willem Jan Withagen <wjw@digiware.nl <mailto:wjw@digiware.nl>>于2016年10
>> 月20日 周四00:10写道:
>>
>>     I guess somebody recently modified the type of either the ::encode() or
>>     snapid_t. But Clang now really throws a fit.
>>
>>     --WjW
>>
>>     /usr/srcs/Ceph/work/ceph/src/common/snap_types.h:58:5: error: no
>>     matching function for call to 'encode'
>>         ::encode(snaps, bl);
>>         ^~~~~~~~
>>     /usr/srcs/Ceph/work/ceph/src/include/encoding.h:68:1: note: candidate
>>     function not viable: no known conversion from 'const vector<snapid_t>'
>>     to 'const __u8' (aka 'const unsigned char') for 1st argument
>>     WRITE_RAW_ENCODER(__u8)
>>     ^
>>     /usr/srcs/Ceph/work/ceph/src/include/encoding.h:65:15: note: expanded
>>     from macro 'WRITE_RAW_ENCODER'
>>       inline void encode(const type &v, bufferlist& bl, uint64_t features=0)
>>     { encode_raw(v, bl); } \
>>                   ^
>>     /usr/srcs/Ceph/work/ceph/src/include/encoding.h:70:1: note: candidate
>>     function not viable: no known conversion from 'const vector<snapid_t>'
>>     to 'const __s8' (aka 'const signed char') for 1st argument
>>     WRITE_RAW_ENCODER(__s8)
>>     ^
>>     /usr/srcs/Ceph/work/ceph/src/include/encoding.h:65:15: note: expanded
>>     from macro 'WRITE_RAW_ENCODER'
>>       inline void encode(const type &v, bufferlist& bl, uint64_t features=0)
>>     { encode_raw(v, bl); } \
>>                   ^
>>     /usr/srcs/Ceph/work/ceph/src/include/encoding.h:72:1: note: candidate
>>     function not viable: no known conversion from 'const vector<snapid_t>'
>>     to 'const char' for 1st argument
>>     WRITE_RAW_ENCODER(char)
>>     ^
>>     /usr/srcs/Ceph/work/ceph/src/include/encoding.h:65:15: note: expanded
>>     from macro 'WRITE_RAW_ENCODER'
>>       inline void encode(const type &v, bufferlist& bl, uint64_t features=0)
>>     { encode_raw(v, bl); } \
>>                   ^
>>     /usr/srcs/Ceph/work/ceph/src/include/encoding.h:73:1: note: candidate
>>     function not viable: no known conversion from 'const vector<snapid_t>'
>>     to 'const ceph_le64' for 1st argument
>>     WRITE_RAW_ENCODER(ceph_le64)
>>     ^
>>     /usr/srcs/Ceph/work/ceph/src/include/encoding.h:65:15: note: expanded
>>     from macro 'WRITE_RAW_ENCODER'
>>       inline void encode(const type &v, bufferlist& bl, uint64_t features=0)
>>     { encode_raw(v, bl); } \
>>                   ^
>>     /usr/srcs/Ceph/work/ceph/src/include/encoding.h:74:1: note: candidate
>>     function not viable: no known conversion from 'const vector<snapid_t>'
>>     to 'const ceph_le32' for 1st argument
>>     WRITE_RAW_ENCODER(ceph_le32)
>>     ^
>>     /usr/srcs/Ceph/work/ceph/src/include/encoding.h:65:15: note: expanded
>>     from macro 'WRITE_RAW_ENCODER'
>>       inline void encode(const type &v, bufferlist& bl, uint64_t features=0)
>>     { encode_raw(v, bl); } \
>>                   ^
>>     /usr/srcs/Ceph/work/ceph/src/include/encoding.h:75:1: note: candidate
>>     function not viable: no known conversion from 'const vector<snapid_t>'
>>     to 'const ceph_le16' for 1st argument
>>     WRITE_RAW_ENCODER(ceph_le16)
>>     ^
>>     /usr/srcs/Ceph/work/ceph/src/include/encoding.h:65:15: note: expanded
>>     from macro 'WRITE_RAW_ENCODER'
>>       inline void encode(const type &v, bufferlist& bl, uint64_t features=0)
>>     { encode_raw(v, bl); } \

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Willem Jan Withagen Oct. 20, 2016, 8:27 a.m. UTC | #1
On 20-10-2016 02:49, Willem Jan Withagen wrote:
> On 20-10-2016 02:21, Willem Jan Withagen wrote:
>> On 19-10-2016 20:58, kefu chai wrote:
>>> should have been fixed in master.
>>
>> Ehh,
>>
>> Not really.
>> Just tested by running
>> cd Ceph/master/ceph
>> git pull
>> ./do_freebsd.sh
>>
>> And I still get the same error.

There is now a Jenkins builder that build my WIIP tree
(on regular basis, once I get Jenkins to listen)
And where you can see the full output of a build attempt.

http://cephdev.digiware.nl:8180/jenkins/job/ceph-freebsd
(currenlty #39)

Once ceph/ceph builds and tests appropriately I'll make a jenkinst
instance for that as well.

--WjW

>> Guess I'll have to start bisecting to see where it went wrong.
> 
> This is the actual commit going wrong
> 
> commit c9c5235ef7d563b92f44dab63a8ac2b694e69d4f
> Author: Sage Weil <sage@redhat.com>
> Date:   Wed Sep 14 13:32:20 2016 -0400
> 
>     include/object: conditional denc_traits for snapid_t
> 
>     Signed-off-by: Sage Weil <sage@redhat.com>
> 
> diff --git a/src/include/object.h b/src/include/object.h
> index 74a011b..4532b78 100644
> --- a/src/include/object.h
> +++ b/src/include/object.h
> @@ -122,6 +122,22 @@ struct snapid_t {
>  inline void encode(snapid_t i, bufferlist &bl) { encode(i.val, bl); }
>  inline void decode(snapid_t &i, bufferlist::iterator &p) {
> decode(i.val, p); }
> 
> +template<>
> +struct denc_traits<snapid_t> {
> +  enum { supported = 2 };
> +  enum { featured = false };
> +  enum { bounded = true };
> +  static void bound_encode(const snapid_t o, size_t& p) {
> +    denc(o.val, p);
> +  }
> +  static void encode(const snapid_t &o,
> buffer::list::contiguous_appender& p) {
> +    denc(o.val, p);
> +  }
> +  static void decode(snapid_t& o, buffer::ptr::iterator &p) {
> +    denc(o.val, p);
> +  }
> +};
> +
>  inline ostream& operator<<(ostream& out, snapid_t s) {
>    if (s == CEPH_NOSNAP)
>      return out << "head";
> 
> But have not figure out why.
> I'm going to give it some bedrest.
> 
>> --WjW
>>
>>
>>> Willem Jan Withagen <wjw@digiware.nl <mailto:wjw@digiware.nl>>于2016年10
>>> 月20日 周四00:10写道:
>>>
>>>     I guess somebody recently modified the type of either the ::encode() or
>>>     snapid_t. But Clang now really throws a fit.
>>>
>>>     --WjW
>>>
>>>     /usr/srcs/Ceph/work/ceph/src/common/snap_types.h:58:5: error: no
>>>     matching function for call to 'encode'
>>>         ::encode(snaps, bl);
>>>         ^~~~~~~~
>>>     /usr/srcs/Ceph/work/ceph/src/include/encoding.h:68:1: note: candidate
>>>     function not viable: no known conversion from 'const vector<snapid_t>'
>>>     to 'const __u8' (aka 'const unsigned char') for 1st argument
>>>     WRITE_RAW_ENCODER(__u8)
>>>     ^
>>>     /usr/srcs/Ceph/work/ceph/src/include/encoding.h:65:15: note: expanded
>>>     from macro 'WRITE_RAW_ENCODER'
>>>       inline void encode(const type &v, bufferlist& bl, uint64_t features=0)
>>>     { encode_raw(v, bl); } \
>>>                   ^
>>>     /usr/srcs/Ceph/work/ceph/src/include/encoding.h:70:1: note: candidate
>>>     function not viable: no known conversion from 'const vector<snapid_t>'
>>>     to 'const __s8' (aka 'const signed char') for 1st argument
>>>     WRITE_RAW_ENCODER(__s8)
>>>     ^
>>>     /usr/srcs/Ceph/work/ceph/src/include/encoding.h:65:15: note: expanded
>>>     from macro 'WRITE_RAW_ENCODER'
>>>       inline void encode(const type &v, bufferlist& bl, uint64_t features=0)
>>>     { encode_raw(v, bl); } \
>>>                   ^
>>>     /usr/srcs/Ceph/work/ceph/src/include/encoding.h:72:1: note: candidate
>>>     function not viable: no known conversion from 'const vector<snapid_t>'
>>>     to 'const char' for 1st argument
>>>     WRITE_RAW_ENCODER(char)
>>>     ^
>>>     /usr/srcs/Ceph/work/ceph/src/include/encoding.h:65:15: note: expanded
>>>     from macro 'WRITE_RAW_ENCODER'
>>>       inline void encode(const type &v, bufferlist& bl, uint64_t features=0)
>>>     { encode_raw(v, bl); } \
>>>                   ^
>>>     /usr/srcs/Ceph/work/ceph/src/include/encoding.h:73:1: note: candidate
>>>     function not viable: no known conversion from 'const vector<snapid_t>'
>>>     to 'const ceph_le64' for 1st argument
>>>     WRITE_RAW_ENCODER(ceph_le64)
>>>     ^
>>>     /usr/srcs/Ceph/work/ceph/src/include/encoding.h:65:15: note: expanded
>>>     from macro 'WRITE_RAW_ENCODER'
>>>       inline void encode(const type &v, bufferlist& bl, uint64_t features=0)
>>>     { encode_raw(v, bl); } \
>>>                   ^
>>>     /usr/srcs/Ceph/work/ceph/src/include/encoding.h:74:1: note: candidate
>>>     function not viable: no known conversion from 'const vector<snapid_t>'
>>>     to 'const ceph_le32' for 1st argument
>>>     WRITE_RAW_ENCODER(ceph_le32)
>>>     ^
>>>     /usr/srcs/Ceph/work/ceph/src/include/encoding.h:65:15: note: expanded
>>>     from macro 'WRITE_RAW_ENCODER'
>>>       inline void encode(const type &v, bufferlist& bl, uint64_t features=0)
>>>     { encode_raw(v, bl); } \
>>>                   ^
>>>     /usr/srcs/Ceph/work/ceph/src/include/encoding.h:75:1: note: candidate
>>>     function not viable: no known conversion from 'const vector<snapid_t>'
>>>     to 'const ceph_le16' for 1st argument
>>>     WRITE_RAW_ENCODER(ceph_le16)
>>>     ^
>>>     /usr/srcs/Ceph/work/ceph/src/include/encoding.h:65:15: note: expanded
>>>     from macro 'WRITE_RAW_ENCODER'
>>>       inline void encode(const type &v, bufferlist& bl, uint64_t features=0)
>>>     { encode_raw(v, bl); } \
> 
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Willem Jan Withagen Oct. 20, 2016, 9:21 a.m. UTC | #2
On 20-10-2016 02:49, Willem Jan Withagen wrote:
> On 20-10-2016 02:21, Willem Jan Withagen wrote:
>> On 19-10-2016 20:58, kefu chai wrote:
>>> should have been fixed in master.
>>
>> Ehh,
>>
>> Not really.
>> Just tested by running
>> cd Ceph/master/ceph
>> git pull
>> ./do_freebsd.sh
>>
>> And I still get the same error.
>>
>> Guess I'll have to start bisecting to see where it went wrong.
> 
> This is the actual commit going wrong
> 
> commit c9c5235ef7d563b92f44dab63a8ac2b694e69d4f
> Author: Sage Weil <sage@redhat.com>
> Date:   Wed Sep 14 13:32:20 2016 -0400
> 
>     include/object: conditional denc_traits for snapid_t
> 
>     Signed-off-by: Sage Weil <sage@redhat.com>

This all was in pull #11027
Last commit that works is: dec0f05288dc4fce0f5ae2de7cf4dd8f9281fe1f

That code is changing all kind of templates... And to be honest, I not
very comfortable with that.

But obviously GCC seems to able to promote type
	'const vector<snapid_t>'
in such a way that it can find a matching function, where as Clang is
being more selective (as usual)

--WjW


--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gregory Farnum Oct. 20, 2016, 4:53 p.m. UTC | #3
On Thu, Oct 20, 2016 at 3:21 AM, Willem Jan Withagen <wjw@digiware.nl> wrote:
> On 20-10-2016 02:49, Willem Jan Withagen wrote:
>> On 20-10-2016 02:21, Willem Jan Withagen wrote:
>>> On 19-10-2016 20:58, kefu chai wrote:
>>>> should have been fixed in master.
>>>
>>> Ehh,
>>>
>>> Not really.
>>> Just tested by running
>>> cd Ceph/master/ceph
>>> git pull
>>> ./do_freebsd.sh
>>>
>>> And I still get the same error.
>>>
>>> Guess I'll have to start bisecting to see where it went wrong.
>>
>> This is the actual commit going wrong
>>
>> commit c9c5235ef7d563b92f44dab63a8ac2b694e69d4f
>> Author: Sage Weil <sage@redhat.com>
>> Date:   Wed Sep 14 13:32:20 2016 -0400
>>
>>     include/object: conditional denc_traits for snapid_t
>>
>>     Signed-off-by: Sage Weil <sage@redhat.com>
>
> This all was in pull #11027
> Last commit that works is: dec0f05288dc4fce0f5ae2de7cf4dd8f9281fe1f
>
> That code is changing all kind of templates... And to be honest, I not
> very comfortable with that.
>
> But obviously GCC seems to able to promote type
>         'const vector<snapid_t>'
> in such a way that it can find a matching function, where as Clang is
> being more selective (as usual)

That's odd; isn't snapid_t a typedef for uint64_t?
There are a few things like that which are wrapped structs with
implicit conversions, in which case maybe it needs to be defined more
clearly for Clang in this case, or just give it an explicit template
wrapper pointing at le64 (or whichever one it's supposed to be).
-Greg

>
> --WjW
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Willem Jan Withagen Oct. 21, 2016, 7:46 a.m. UTC | #4
On 20-10-2016 18:53, Gregory Farnum wrote:
> On Thu, Oct 20, 2016 at 3:21 AM, Willem Jan Withagen <wjw@digiware.nl> wrote:
>> On 20-10-2016 02:49, Willem Jan Withagen wrote:
>>> On 20-10-2016 02:21, Willem Jan Withagen wrote:
>>>> On 19-10-2016 20:58, kefu chai wrote:
>>>>> should have been fixed in master.
>>>>
>>>> Ehh,
>>>>
>>>> Not really.
>>>> Just tested by running
>>>> cd Ceph/master/ceph
>>>> git pull
>>>> ./do_freebsd.sh
>>>>
>>>> And I still get the same error.
>>>>
>>>> Guess I'll have to start bisecting to see where it went wrong.
>>>
>>> This is the actual commit going wrong
>>>
>>> commit c9c5235ef7d563b92f44dab63a8ac2b694e69d4f
>>> Author: Sage Weil <sage@redhat.com>
>>> Date:   Wed Sep 14 13:32:20 2016 -0400
>>>
>>>     include/object: conditional denc_traits for snapid_t
>>>
>>>     Signed-off-by: Sage Weil <sage@redhat.com>
>>
>> This all was in pull #11027
>> Last commit that works is: dec0f05288dc4fce0f5ae2de7cf4dd8f9281fe1f
>>
>> That code is changing all kind of templates... And to be honest, I not
>> very comfortable with that.
>>
>> But obviously GCC seems to able to promote type
>>         'const vector<snapid_t>'
>> in such a way that it can find a matching function, where as Clang is
>> being more selective (as usual)
> 
> That's odd; isn't snapid_t a typedef for uint64_t?
> There are a few things like that which are wrapped structs with
> implicit conversions, in which case maybe it needs to be defined more
> clearly for Clang in this case, or just give it an explicit template
> wrapper pointing at le64 (or whichever one it's supposed to be).

The other value I see being used in dencode is indeed ceph_le64.

Although I understand what you are saying. It is not something that I
can just pull out of my hat, since templates still are a large learning
field for me.

So a hint would be appreciated.

--WjW


--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/src/include/object.h b/src/include/object.h
index 74a011b..4532b78 100644
--- a/src/include/object.h
+++ b/src/include/object.h
@@ -122,6 +122,22 @@  struct snapid_t {
 inline void encode(snapid_t i, bufferlist &bl) { encode(i.val, bl); }
 inline void decode(snapid_t &i, bufferlist::iterator &p) {
decode(i.val, p); }

+template<>
+struct denc_traits<snapid_t> {
+  enum { supported = 2 };
+  enum { featured = false };
+  enum { bounded = true };
+  static void bound_encode(const snapid_t o, size_t& p) {
+    denc(o.val, p);
+  }
+  static void encode(const snapid_t &o,
buffer::list::contiguous_appender& p) {
+    denc(o.val, p);
+  }
+  static void decode(snapid_t& o, buffer::ptr::iterator &p) {
+    denc(o.val, p);
+  }
+};
+
 inline ostream& operator<<(ostream& out, snapid_t s) {