Building for Clang is broken for snap-types

Commit Message

Sage Weil Oct. 21, 2016, 1:53 p.m. UTC

On Fri, 21 Oct 2016, Willem Jan Withagen wrote:
> On 20-10-2016 18:53, Gregory Farnum wrote:
> > On Thu, Oct 20, 2016 at 3:21 AM, Willem Jan Withagen <[e-mail]> 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 <[e-mail]>
> >>> Date:   Wed Sep 14 13:32:20 2016 -0400
> >>>
> >>>     include/object: conditional denc_traits for snapid_t
> >>>
> >>>     Signed-off-by: Sage Weil <[e-mail]>
> >>
> >> 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.

It looks like clang is buggy.  At leasy, my fedora 23 version is

$ clang --version
clang version 3.7.0 (tags/RELEASE_370/final)
Target: x86_64-redhat-linux-gnu
Thread model: posix

This fixes the snapid_t compilation:


which, AFAICS, is nonsense... supported is either a bool or int, and in 
either case "supported" and "supported != 0" should be equivalent 
(right?).  But once I get past that clang crashes a few files later... see 
attached.

Maybe start by trying to upgrade clang?  Or submit a clang bug report? :/

(Perhaps you can use gcc for the time being?)

sage
[ 50%] Building CXX object src/CMakeFiles/common.dir/common/lockdep.cc.o
'const ceph_msg_footer' for 1stllvm::sys::PrintStackTrace(llvm::raw_ostream&)  argument
(/usr/lib64/llvm/libLLVM-3.7.so+0x510327)
#1 0x7eff3e5c7881 (/usr/lib64/llvm/libLLVM-3.7.so+0x50f881)
[ 56%] Building CXX object src/CMakeFiles/common.dir/common/hex.cc.o
#2 0x7eff3dc94a00 __restore_rt (/lib64/libpthread.so.0+0x10a00)
#3 0x560d75652070 (/usr/bin/clang+0x10ea070)
#4 0x560d756552d1 (/usr/bin/clang+0x10ed2d1)
#5 0x560d75658e66 (/usr/bin/clang+0x10f0e66)
#6 0x560d75659857 (/usr/bin/clang+0x10f1857)
#7 0x560d756546d8 WRITE_RAW_ENCODER(ceph_msg_footer)(/usr/bin/clang+0x10ec6d8)

^
#8 0x560d756512b6 (/usr/bin/clang+0x10e92b6)
#9 0x560d756516f5 (/usr/bin/clang+0x10e96f5)
#10 0x560d756579ad (/usr/bin/clang+0x10ef9ad)
#11 0x560d756584ec (/usr/bin/clang+0x10f04ec)
#12 0x560d756598c3 (/usr/bin/clang+0x10f18c3)
/home/sage/src/ceph/src/include/encoding.h#13 0x560d75654f62 :65(/usr/bin/clang+0x10ecf62):
15#14 0x560d75654dfd :(/usr/bin/clang+0x10ecdfd) 
note#15 0x560d75655118 : (/usr/bin/clang+0x10ed118)expanded
 from#16 0x560d7565a8d7  macro(/usr/bin/clang+0x10f28d7)

      'WRITE_RAW_ENCODER'#17 0x560d74b7a2a9 
clang::CodeGen::CodeGenModule::getMangledName(clang::GlobalDecl) (/usr/bin/clang+0x6122a9)
#18 0x560d74b85ec3 clang::CodeGen::CodeGenModule::EmitGlobal(clang::GlobalDecl) (/usr/bin/clang+0x61dec3)
#19 0x560d74b86918 clang::CodeGen::CodeGenModule::EmitTopLevelDecl(clang::Decl*) (/usr/bin/clang+0x61e918)
#20 0x560d74b0be0f (/usr/bin/clang+0x5a3e0f)
#21 0x560d74af9074 (/usr/bin/clang+0x591074)
#22 0x560d7513e315 clang::Sema::InstantiateFunctionDefinition(clang::SourceLocation, clang::FunctionDecl*, bool, bool) (/usr/bin/clang+0xbd6315)
#23 0x560d74f25913 clang::Sema::MarkFunctionReferenced(clang::SourceLocation, clang::FunctionDecl*, bool) (/usr/bin/clang+0x9bd913)
#24 0x560d74f2ad6e (/usr/bin/clang+0x9c2d6e)
#25 0x560d74f2af32 clang::Sema::MarkDeclRefReferenced(clang::DeclRefExpr*) (/usr/bin/clang+0x9c2f32)
#26 0x560d7503fce2 clang::Sema::FixOverloadedFunctionReference(clang::Expr*, clang::DeclAccessPair, clang::FunctionDecl*) (/usr/bin/clang+0xad7ce2)
#27 0x560d7505e51b (/usr/bin/clang+0xaf651b)
#28 0x560d7505efec   inline void encode(const type &v, bufferlist& bl, uint64_t features=0) ...
              ^
clang::Sema::BuildOverloadedCallExpr(clang::Scope*, clang::Expr*, clang::UnresolvedLookupExpr*, clang::SourceLocation, llvm::MutableArrayRef<clang::Expr*>, clang::SourceLocation, clang::Expr*, bool) (/usr/bin/clang+0xaf6fec)
#29 0x560d74f3345f clang::Sema::ActOnCallExpr(clang::Scope*, clang::Expr*, clang::SourceLocation, llvm::MutableArrayRef<clang::Expr*>, clang::SourceLocation, clang::Expr*, bool) (/usr/bin/clang+0x9cb45f)
#30 0x560d74d2bdcb clang::Parser::ParsePostfixExpressionSuffix(clang::ActionResult<clang::Expr*, true>) (/usr/bin/clang+0x7c3dcb)
#31 0x560d74d2665f clang::Parser::ParseCastExpression(bool, bool, bool&, clang::Parser::TypeCastState) (/usr/bin/clang+0x7be65f)
#32 0x560d74d2939d clang::Parser::ParseCastExpression(bool, bool, clang::Parser::TypeCastState) (/usr/bin/clang+0x7c139d)
#33 0x560d74d273d1 clang::Parser::ParseCastExpression(bool, bool, bool&, clang::Parser::TypeCastState) (/usr/bin/clang+0x7bf3d1)
#34 0x560d74d2939d clang::Parser::ParseCastExpression(bool, bool, clang::Parser::TypeCastState) (/usr/bin/clang+0x7c139d)
#35 0x560d74d2942f clang::Parser::ParseAssignmentExpression(clang::Parser::TypeCastState) (/usr/bin/clang+0x7c142f)
#36 0x560d74d2b58a clang::Parser::ParseExpressionList(llvm::SmallVectorImpl<clang::Expr*>&, llvm::SmallVectorImpl<clang::SourceLocation>&, std::function<void ()>) (/usr/bin/clang+0x7c358a)
#37 0x560d74d2c8fd clang::Parser::ParsePostfixExpressionSuffix(clang::ActionResult<clang::Expr*, true>) (/usr/bin/clang+0x7c48fd)
/home/sage/src/ceph/src/include/types.h:#38 0x560d74d2665f 268:1: note: clang::Parser::ParseCastExpression(bool, bool, bool&, clang::Parser::TypeCastState) (/usr/bin/clang+0x7be65f)candidate 
function#39 0x560d74d2939d  clang::Parser::ParseCastExpression(bool, bool, clang::Parser::TypeCastState) not
      (/usr/bin/clang+0x7c139d)viable: 
no#40 0x560d74d2942f  clang::Parser::ParseAssignmentExpression(clang::Parser::TypeCastState) known (/usr/bin/clang+0x7c142f)conversion 
from#41 0x560d74d294a9  clang::Parser::ParseExpression(clang::Parser::TypeCastState) 'const std::list<__u32>' (/usr/bin/clang+0x7c14a9)to 
#42 0x560d74d66eb9 'const
clang::Parser::ParseExprStatement()       (/usr/bin/clang+0x7feeb9)ceph_msg_footer_old' for
 #43 0x560d74d6465f 1stclang::Parser::ParseStatementOrDeclarationAfterAttributes(llvm::SmallVector<clang::Stmt*, 32u>&, bool, clang::SourceLocation*, clang::Parser::ParsedAttributesWithRange&)  argument
WRITE_RAW_ENCODER(ceph_msg_footer_old)(/usr/bin/clang+0x7fc65f)
#44 0x560d74d647f7 clang::Parser::ParseStatementOrDeclaration(llvm::SmallVector<clang::Stmt*, 32u>&, bool, clang::SourceLocation*) (/usr/bin/clang+0x7fc7f7)
#45 0x560d74d6492a clang::Parser::ParseStatement(clang::SourceLocation*) (/usr/bin/clang+0x7fc92a)
#46 0x560d74d67c35 clang::Parser::ParseIfStatement(clang::SourceLocation*) (/usr/bin/clang+0x7ffc35)
#47 0x560d74d6427b clang::Parser::ParseStatementOrDeclarationAfterAttributes(llvm::SmallVector<clang::Stmt*, 32u>&, bool, clang::SourceLocation*, clang::Parser::ParsedAttributesWithRange&) 
^(/usr/bin/clang+0x7fc27b)
/home/sage/src/ceph/src/include/encoding.h
#48 0x560d74d647f7 clang::Parser::ParseStatementOrDeclaration(llvm::SmallVector<clang::Stmt*, 32u>&, bool, clang::SourceLocation*) (/usr/bin/clang+0x7fc7f7)
#49 0x560d74d69156 clang::Parser::ParseCompoundStatementBody(bool) (/usr/bin/clang+0x801156):
65#50 0x560d74d3c7ba :15clang::Parser::ParseLambdaExpressionAfterIntroducer(clang::LambdaIntroducer&) :(/usr/bin/clang+0x7d47ba) note
: #51 0x560d74d3e42b expandedclang::Parser::ParseLambdaExpression()  from(/usr/bin/clang+0x7d642b) macro

#52 0x560d74d27a8d       clang::Parser::ParseCastExpression(bool, bool, bool&, clang::Parser::TypeCastState) 'WRITE_RAW_ENCODER'
(/usr/bin/clang+0x7bfa8d)  inline void encode(const type &v, bufferlist& bl, uint64_t features=0) ...
#53 0x560d74d2939d clang::Parser::ParseCastExpression(bool, bool, clang::Parser::TypeCastState) (/usr/bin/clang+0x7c139d)
#54 0x560d74d2942f clang::Parser::ParseAssignmentExpression(clang::Parser::TypeCastState) (/usr/bin/clang+0x7c142f)
#55 0x560d74d2b58a clang::Parser::ParseExpressionList(llvm::SmallVectorImpl<clang::Expr*>&, llvm::SmallVectorImpl<clang::SourceLocation>&, std::function<void ()>) (/usr/bin/clang+0x7c358a)
#56 0x560d74d2c8fd clang::Parser::ParsePostfixExpressionSuffix(clang::ActionResult<clang::Expr*, true>) (/usr/bin/clang+0x7c48fd)
#57 0x560d74d2665f clang::Parser::ParseCastExpression(bool, bool, bool&, clang::Parser::TypeCastState) 
              ^(/usr/bin/clang+0x7be65f)

/home/sage/src/ceph/src/include/types.h#58 0x560d74d2939d clang::Parser::ParseCastExpression(bool, bool, clang::Parser::TypeCastState) (/usr/bin/clang+0x7c139d)
#59 0x560d74d273d1 clang::Parser::ParseCastExpression(bool, bool, bool&, clang::Parser::TypeCastState) (/usr/bin/clang+0x7bf3d1)
#60 0x560d74d2939d clang::Parser::ParseCastExpression(bool, bool, clang::Parser::TypeCastState) (/usr/bin/clang+0x7c139d)
#61 0x560d74d2942f clang::Parser::ParseAssignmentExpression(clang::Parser::TypeCastState) (/usr/bin/clang+0x7c142f)
#62 0x560d74cfc7a5 clang::Parser::ParseDeclarationAfterDeclaratorAndAttributes(clang::Declarator&, clang::Parser::ParsedTemplateInfo const&, clang::Parser::ForRangeInit*) (/usr/bin/clang+0x7947a5)
#63 0x560d74d0abfa clang::Parser::ParseDeclGroup(clang::ParsingDeclSpec&, unsigned int, clang::SourceLocation*, clang::Parser::ForRangeInit*) (/usr/bin/clang+0x7a2bfa):
269#64 0x560d74d0dad4 :clang::Parser::ParseSimpleDeclaration(unsigned int, clang::SourceLocation&, clang::Parser::ParsedAttributesWithRange&, bool, clang::Parser::ForRangeInit*) 1: (/usr/bin/clang+0x7a5ad4)note: 
candidate#65 0x560d74d0dd9d  clang::Parser::ParseDeclaration(unsigned int, clang::SourceLocation&, clang::Parser::ParsedAttributesWithRange&) function (/usr/bin/clang+0x7a5d9d)not

      #66 0x560d74d645cf viable:clang::Parser::ParseStatementOrDeclarationAfterAttributes(llvm::SmallVector<clang::Stmt*, 32u>&, bool, clang::SourceLocation*, clang::Parser::ParsedAttributesWithRange&)  no known(/usr/bin/clang+0x7fc5cf) 
conversion#67 0x560d74d647f7  fromclang::Parser::ParseStatementOrDeclaration(llvm::SmallVector<clang::Stmt*, 32u>&, bool, clang::SourceLocation*)  'const std::list<__u32>'(/usr/bin/clang+0x7fc7f7) to
 #68 0x560d74d69156 'constclang::Parser::ParseCompoundStatementBody(bool) 
      (/usr/bin/clang+0x801156)ceph_mon_subscribe_item' 
for#69 0x560d74d6987e  clang::Parser::ParseCompoundStatement(bool, unsigned int) 1st (/usr/bin/clang+0x80187e)argument

WRITE_RAW_ENCODER(ceph_mon_subscribe_item)#70 0x560d74d698c2 clang::Parser::ParseCompoundStatement(bool) (/usr/bin/clang+0x8018c2)
#71 0x560d74d643da clang::Parser::ParseStatementOrDeclarationAfterAttributes(llvm::SmallVector<clang::Stmt*, 32u>&, bool, clang::SourceLocation*, clang::Parser::ParsedAttributesWithRange&) (/usr/bin/clang+0x7fc3da)
#72 0x560d74d647f7 
clang::Parser::ParseStatementOrDeclaration(llvm::SmallVector<clang::Stmt*, 32u>&, bool, clang::SourceLocation*) ^
(/usr/bin/clang+0x7fc7f7)/home/sage/src/ceph/src/include/encoding.h
#73 0x560d74d6492a clang::Parser::ParseStatement(clang::SourceLocation*) :(/usr/bin/clang+0x7fc92a)65:
15#74 0x560d74d67c35 : clang::Parser::ParseIfStatement(clang::SourceLocation*) note: (/usr/bin/clang+0x7ffc35)expanded 
from#75 0x560d74d6427b  clang::Parser::ParseStatementOrDeclarationAfterAttributes(llvm::SmallVector<clang::Stmt*, 32u>&, bool, clang::SourceLocation*, clang::Parser::ParsedAttributesWithRange&) macro
(/usr/bin/clang+0x7fc27b)      'WRITE_RAW_ENCODER'

#76 0x560d74d647f7   inline void encode(const type &v, bufferlist& bl, uint64_t features=0) ...clang::Parser::ParseStatementOrDeclaration(llvm::SmallVector<clang::Stmt*, 32u>&, bool, clang::SourceLocation*) (/usr/bin/clang+0x7fc7f7)
#77 0x560d74d69156 clang::Parser::ParseCompoundStatementBody(bool) (/usr/bin/clang+0x801156)
#78 0x560d74d6b8f6 clang::Parser::ParseFunctionStatementBody(clang::Decl*, clang::Parser::ParseScope&) (/usr/bin/clang+0x8038f6)
#79 0x560d74cf0bfe clang::Parser::ParseFunctionDefinition(clang::ParsingDeclarator&, clang::Parser::ParsedTemplateInfo const&, clang::Parser::LateParsedAttrList*) (/usr/bin/clang+0x788bfe)

#80 0x560d74d0b1c1               ^clang::Parser::ParseDeclGroup(clang::ParsingDeclSpec&, unsigned int, clang::SourceLocation*, clang::Parser::ForRangeInit*) 
/home/sage/src/ceph/src/include/types.h(/usr/bin/clang+0x7a31c1)
#81 0x560d74ced402 clang::Parser::ParseDeclOrFunctionDefInternal(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec&, clang::AccessSpecifier) (/usr/bin/clang+0x785402)
#82 0x560d74ced9c3 clang::Parser::ParseDeclarationOrFunctionDefinition(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec*, clang::AccessSpecifier) (/usr/bin/clang+0x7859c3)
:#83 0x560d74cf36cd 271clang::Parser::ParseExternalDeclaration(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec*) :1:(/usr/bin/clang+0x78b6cd) 
note: #84 0x560d74cf4052 candidateclang::Parser::ParseTopLevelDecl(clang::OpaquePtr<clang::DeclGroupRef>&)  function(/usr/bin/clang+0x78c052) not

#85 0x560d74ce931b       viable:clang::ParseAST(clang::Sema&, bool, bool)  (/usr/bin/clang+0x78131b)no 
known#86 0x560d74af9d81  clang::CodeGenAction::ExecuteAction() conversion (/usr/bin/clang+0x591d81)from 
'const std::list<__u32>'#87 0x560d749434ce  clang::FrontendAction::Execute() to
(/usr/bin/clang+0x3db4ce)      
'const ceph_mon_statfs'#88 0x560d74916381  forclang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (/usr/bin/clang+0x3ae381) 1st argument

WRITE_RAW_ENCODER(ceph_mon_statfs)#89 0x560d748fcae1 clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (/usr/bin/clang+0x394ae1)
#90 0x560d748f62b8 cc1_main(llvm::ArrayRef<char const*>, char const*, void*) (/usr/bin/clang+0x38e2b8)
#91 0x560d748f2bf2 main (/usr/bin/clang+0x38abf2)
#92 0x7eff3cb5d580 __libc_start_main (/lib64/libc.so.6+0x20580)

^#93 0x560d748f5179 
/home/sage/src/ceph/src/include/encoding.h_start (/usr/bin/clang+0x38d179)
Stack dump:
:65:15: note: expanded from macro
      'WRITE_RAW_ENCODER'
  inline void encode(const type &v, bufferlist& bl, uint64_t features=0) ...0.	Program arguments: /usr/bin/clang -cc1 -triple x86_64-redhat-linux-gnu -emit-obj -disable-free -disable-llvm-verifier -main-file-name AsyncConnection.cc
               ^-mrelocation-model
 pic/home/sage/src/ceph/src/include/types.h -pic-level 2 -mthread-model posix -relaxed-aliasing -fmath-errno -masm-verbose -mconstructor-aliases -munwind-tables -fuse-init-array -target-cpu x86-64 -target-linker-version 2.25 -momit-leaf-frame-pointer -g -dwarf-column-info -coverage-file /home/sage/src/ceph/build.clang/src/CMakeFiles/common.dir/msg/async/AsyncConnection.cc.o -resource-dir: 272/usr/bin/../lib/clang/3.7.0 :-D1 :CEPH_LIBDIR="/usr/local/lib64"  note-D:  candidateCEPH_PKGLIBDIR="/usr/local/lib64/ceph"  -Dfunction  __linux__not 
-D       viable:HAVE_CONFIG_H  no-D  known__CEPH__  -Dconversion  _REENTRANTfrom  -D'const std::list<__u32>' _THREAD_SAFE  to-D  'const__STDC_FORMAT_MACROS
       -Iceph_mon_statfs_reply'  /home/sage/src/ceph/build.clang/src/includefor  -I1st  /home/sage/src/ceph/srcargument -I
 WRITE_RAW_ENCODER(ceph_mon_statfs_reply)/home/sage/src/ceph/build.clang/include -I /home/sage/src/ceph/src/xxHash -I /usr/include/nss3 -I /usr/include/nspr4 
-internal-isystem^ /usr/bin/../lib/gcc/x86_64-redhat-linux/5.3.1/../../../../include/c++/5.3.1
/home/sage/src/ceph/src/include/encoding.h -internal-isystem /usr/bin/../lib/gcc/x86_64-redhat-linux/5.3.1/../../../../include/c++/5.3.1/x86_64-redhat-linux :-internal-isystem65 :/usr/bin/../lib/gcc/x86_64-redhat-linux/5.3.1/../../../../include/c++/5.3.1/backward15 :-internal-isystem  note/usr/local/include:  expanded-internal-isystem  from/usr/bin/../lib/clang/3.7.0/include  -internal-externc-isystemmacro 
/include       -internal-externc-isystem'WRITE_RAW_ENCODER' 
/usr/include  inline void encode(const type &v, bufferlist& bl, uint64_t features=0) ... -O2 -Wall -Wtype-limits -Wignored-qualifiers -Winit-self -Wpointer-arith -Werror=format-security -Wno-inconsistent-missing-override -Wno-mismatched-tags -Wno-unused-function -Wno-unused-local-typedef -Wno-inconsistent-missing-override -Wno-unused-private-field 
-Wno-varargs              ^ -Wno-gnu-designator
 /home/sage/src/ceph/src/include/types.h-Wno-mismatched-tags -Wno-missing-braces -Wno-parentheses -Wno-deprecated-register -Wno-invalid-offsetof -Wnon-virtual-dtor -std=c++11 -fdeprecated-macro -fdebug-compilation-dir: 298/home/sage/src/ceph/build.clang/src: 1-ftemplate-depth:  note1024 : -ferror-limitcandidate  19function -fmessage-length  not81
       viable:-mstackrealign  -fobjc-runtime=gccno  -fcxx-exceptionsknown  -fexceptionsconversion  -fdiagnostics-show-optionfrom  -vectorize-loops'const std::list<__u32>'  -vectorize-slpto 
-o       CMakeFiles/common.dir/msg/async/AsyncConnection.cc.o'const client_t'  -xfor  c++1st  /home/sage/src/ceph/src/msg/async/AsyncConnection.ccargument 

WRITE_CLASS_ENCODER(client_t)1.	/home/sage/src/ceph/src/msg/async/AsyncConnection.cc
:^1725
:/home/sage/src/ceph/src/include/encoding.h68: current parser token ')'
2:.	153:/home/sage/src/ceph/src/msg/async/AsyncConnection.cc15: :note1444: :expanded1 : fromparsing function body  'macro
      'WRITE_CLASS_ENCODER'
handle_connect_msg  inline void encode(const cl &c, bufferlist &bl, uint64_t features=0) { \'
3.	/home/sage/src/ceph/src/msg/async/AsyncConnection.cc:1444:1: in compound statement ('{}')

              ^4.	/home/sage/src/ceph/src/msg/async/AsyncConnection.cc
/home/sage/src/ceph/src/include/types.h:1640:10: in compound statement ('{}')
5.	/home/sage/src/ceph/src/msg/async/AsyncConnection.cc:1682:9: lambda expression parsing
6.	/home/sage/src/ceph/src/msg/async/AsyncConnection.cc:1682:107: in compound statement ('{}')
7.	:474:1: note: candidate/usr/bin/../lib/gcc/x86_64-redhat-linux/5.3.1/../../../../include/c++/5.3.1/bits/move.h :function101 not
      :5viable: no known conversion:  fromLLVM IR generation of declaration 'const std::list<__u32>' ' to
      'const shard_id_t' forstd:: 1st argumentmove
'
8.	/usr/bin/../lib/gcc/x86_64-redhat-linux/5.3.1/../../../../include/c++/5.3.1/bits/move.h:101:5: Mangling declaration 'std::WRITE_CLASS_ENCODER(shard_id_t)move'

...

Comments

Willem Jan Withagen Oct. 21, 2016, 2:19 p.m. UTC | #1

On 21-10-2016 15:53, Sage Weil wrote:
> On Fri, 21 Oct 2016, Willem Jan Withagen wrote:
>> On 20-10-2016 18:53, Gregory Farnum wrote:
>>> On Thu, Oct 20, 2016 at 3:21 AM, Willem Jan Withagen <[e-mail]> 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 <[e-mail]>
>>>>> Date:   Wed Sep 14 13:32:20 2016 -0400
>>>>>
>>>>>     include/object: conditional denc_traits for snapid_t
>>>>>
>>>>>     Signed-off-by: Sage Weil <[e-mail]>
>>>>
>>>> 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.
> 
> It looks like clang is buggy.  At leasy, my fedora 23 version is

Hi Sage,

I do not exclude that....
It is going to be either buggy of picky. :)

> $ clang --version
> clang version 3.7.0 (tags/RELEASE_370/final)
> Target: x86_64-redhat-linux-gnu
> Thread model: posix

I'm already at 3.8.0 because that is the default compiler for
11.0-RELEASE and futher.

> This fixes the snapid_t compilation:
> 
> diff --git a/src/include/denc.h b/src/include/denc.h
> index 59f7686..caa095b 100644
> --- a/src/include/denc.h
> +++ b/src/include/denc.h
> @@ -722,7 +722,7 @@ struct denc_traits<
>  template<typename T>
>  struct denc_traits<
>    std::vector<T>,
> -  typename std::enable_if<denc_traits<T>::supported>::type> {
> +  typename std::enable_if<denc_traits<T>::supported != 0>::type> {
>    typedef denc_traits<T> traits;
>  
>    enum { supported = true };
> @@ -831,7 +831,7 @@ struct denc_traits<
>  template<typename T>
>  struct denc_traits<
>    std::set<T>,
> -  typename std::enable_if<denc_traits<T>::supported>::type> {
> +  typename std::enable_if<denc_traits<T>::supported != 0>::type> {
>    typedef denc_traits<T> traits;
>  
>    enum { supported = true };
> 
> which, AFAICS, is nonsense... supported is either a bool or int, and in 
> either case "supported" and "supported != 0" should be equivalent 
> (right?).  But once I get past that clang crashes a few files later... see 
> attached.
> 
> Maybe start by trying to upgrade clang?  Or submit a clang bug report? :/

I'll see if I can get that somewhere reported as bug since it also
hampers 3.8.0 .... Probably one of the tool-chain people will know.

> (Perhaps you can use gcc for the time being?)

Don't need to!! 8=) since 3.8.0 takes it without crash.
Some at least something improved in this corner of the compiler.

But I owe you (lots of) beer, next time we meet...

Going to check if it now completes all the way thru.

Thanx,
--WjW

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

Willem Jan Withagen Oct. 21, 2016, 2:30 p.m. UTC | #2

On 21-10-2016 16:19, Willem Jan Withagen wrote:
> On 21-10-2016 15:53, Sage Weil wrote:
>> On Fri, 21 Oct 2016, Willem Jan Withagen wrote:
>>> On 20-10-2016 18:53, Gregory Farnum wrote:
>>>> On Thu, Oct 20, 2016 at 3:21 AM, Willem Jan Withagen <[e-mail]> 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 <[e-mail]>
>>>>>> Date:   Wed Sep 14 13:32:20 2016 -0400
>>>>>>
>>>>>>     include/object: conditional denc_traits for snapid_t
>>>>>>
>>>>>>     Signed-off-by: Sage Weil <[e-mail]>
>>>>>
>>>>> 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.
>>
>> It looks like clang is buggy.  At leasy, my fedora 23 version is
> 
> Hi Sage,
> 
> I do not exclude that....
> It is going to be either buggy of picky. :)
> 
>> $ clang --version
>> clang version 3.7.0 (tags/RELEASE_370/final)
>> Target: x86_64-redhat-linux-gnu
>> Thread model: posix
> 
> I'm already at 3.8.0 because that is the default compiler for
> 11.0-RELEASE and futher.
> 
>> This fixes the snapid_t compilation:
>>
>> diff --git a/src/include/denc.h b/src/include/denc.h
>> index 59f7686..caa095b 100644
>> --- a/src/include/denc.h
>> +++ b/src/include/denc.h
>> @@ -722,7 +722,7 @@ struct denc_traits<
>>  template<typename T>
>>  struct denc_traits<
>>    std::vector<T>,
>> -  typename std::enable_if<denc_traits<T>::supported>::type> {
>> +  typename std::enable_if<denc_traits<T>::supported != 0>::type> {
>>    typedef denc_traits<T> traits;
>>  
>>    enum { supported = true };
>> @@ -831,7 +831,7 @@ struct denc_traits<
>>  template<typename T>
>>  struct denc_traits<
>>    std::set<T>,
>> -  typename std::enable_if<denc_traits<T>::supported>::type> {
>> +  typename std::enable_if<denc_traits<T>::supported != 0>::type> {
>>    typedef denc_traits<T> traits;
>>  
>>    enum { supported = true };
>>
>> which, AFAICS, is nonsense... supported is either a bool or int, and in 
>> either case "supported" and "supported != 0" should be equivalent 
>> (right?).  But once I get past that clang crashes a few files later... see 
>> attached.
>>
>> Maybe start by trying to upgrade clang?  Or submit a clang bug report? :/
> 
> I'll see if I can get that somewhere reported as bug since it also
> hampers 3.8.0 .... Probably one of the tool-chain people will know.
> 
>> (Perhaps you can use gcc for the time being?)
> 
> Don't need to!! 8=) since 3.8.0 takes it without crash.
> Some at least something improved in this corner of the compiler.
> 
> But I owe you (lots of) beer, next time we meet...
> 
> Going to check if it now completes all the way thru.

It breaks at a different point, in a different source file, but I guess
for the same reasons. Going to grep the code for 'supported>' and
replace more of the same.

/usr/srcs/Ceph/work/ceph/src/mon/MonClient.cc:666:3: error: no matching
function for call to 'encode'
  ::encode(auth_supported->get_supported_set(), m->auth_payload);

--WjW


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

Sage Weil Oct. 21, 2016, 2:32 p.m. UTC | #3

On Fri, 21 Oct 2016, Willem Jan Withagen wrote:
> On 21-10-2016 15:53, Sage Weil wrote:
> > On Fri, 21 Oct 2016, Willem Jan Withagen wrote:
> >> On 20-10-2016 18:53, Gregory Farnum wrote:
> >>> On Thu, Oct 20, 2016 at 3:21 AM, Willem Jan Withagen <[e-mail]> 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 <[e-mail]>
> >>>>> Date:   Wed Sep 14 13:32:20 2016 -0400
> >>>>>
> >>>>>     include/object: conditional denc_traits for snapid_t
> >>>>>
> >>>>>     Signed-off-by: Sage Weil <[e-mail]>
> >>>>
> >>>> 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.
> > 
> > It looks like clang is buggy.  At leasy, my fedora 23 version is
> 
> Hi Sage,
> 
> I do not exclude that....
> It is going to be either buggy of picky. :)
> 
> > $ clang --version
> > clang version 3.7.0 (tags/RELEASE_370/final)
> > Target: x86_64-redhat-linux-gnu
> > Thread model: posix
> 
> I'm already at 3.8.0 because that is the default compiler for
> 11.0-RELEASE and futher.
> 
> > This fixes the snapid_t compilation:
> > 
> > diff --git a/src/include/denc.h b/src/include/denc.h
> > index 59f7686..caa095b 100644
> > --- a/src/include/denc.h
> > +++ b/src/include/denc.h
> > @@ -722,7 +722,7 @@ struct denc_traits<
> >  template<typename T>
> >  struct denc_traits<
> >    std::vector<T>,
> > -  typename std::enable_if<denc_traits<T>::supported>::type> {
> > +  typename std::enable_if<denc_traits<T>::supported != 0>::type> {
> >    typedef denc_traits<T> traits;
> >  
> >    enum { supported = true };
> > @@ -831,7 +831,7 @@ struct denc_traits<
> >  template<typename T>
> >  struct denc_traits<
> >    std::set<T>,
> > -  typename std::enable_if<denc_traits<T>::supported>::type> {
> > +  typename std::enable_if<denc_traits<T>::supported != 0>::type> {
> >    typedef denc_traits<T> traits;
> >  
> >    enum { supported = true };
> > 
> > which, AFAICS, is nonsense... supported is either a bool or int, and in 
> > either case "supported" and "supported != 0" should be equivalent 
> > (right?).  But once I get past that clang crashes a few files later... see 
> > attached.
> > 
> > Maybe start by trying to upgrade clang?  Or submit a clang bug report? :/
> 
> I'll see if I can get that somewhere reported as bug since it also
> hampers 3.8.0 .... Probably one of the tool-chain people will know.
> 
> > (Perhaps you can use gcc for the time being?)
> 
> Don't need to!! 8=) since 3.8.0 takes it without crash.
> Some at least something improved in this corner of the compiler.
> 
> But I owe you (lots of) beer, next time we meet...
> 
> Going to check if it now completes all the way thru.

Even if it does, we shouldn't start celebrating yet!  Those are just the 
two cases I had to change to make it get past that one error.  But if the 
template enable_if conditions aren't properly evaluating bool and/or int 
values then I doubt the code is compiling with the correct behavior.

There is some weirdness here because in some denc_traits instances 
supported = a bool and sometimes support = an int.  It doesn't seem like 
that should prevent determining if the value is true vs non-zero, 
though.  :/

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

Willem Jan Withagen Oct. 21, 2016, 2:59 p.m. UTC | #4

On 21-10-2016 16:32, Sage Weil wrote:
> On Fri, 21 Oct 2016, Willem Jan Withagen wrote:
>> On 21-10-2016 15:53, Sage Weil wrote:
>>> On Fri, 21 Oct 2016, Willem Jan Withagen wrote:
>>>> On 20-10-2016 18:53, Gregory Farnum wrote:
>>>>> On Thu, Oct 20, 2016 at 3:21 AM, Willem Jan Withagen <[e-mail]> 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 <[e-mail]>
>>>>>>> Date:   Wed Sep 14 13:32:20 2016 -0400
>>>>>>>
>>>>>>>     include/object: conditional denc_traits for snapid_t
>>>>>>>
>>>>>>>     Signed-off-by: Sage Weil <[e-mail]>
>>>>>>
>>>>>> 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.
>>>
>>> It looks like clang is buggy.  At leasy, my fedora 23 version is
>>
>> Hi Sage,
>>
>> I do not exclude that....
>> It is going to be either buggy of picky. :)
>>
>>> $ clang --version
>>> clang version 3.7.0 (tags/RELEASE_370/final)
>>> Target: x86_64-redhat-linux-gnu
>>> Thread model: posix
>>
>> I'm already at 3.8.0 because that is the default compiler for
>> 11.0-RELEASE and futher.
>>
>>> This fixes the snapid_t compilation:
>>>
>>> diff --git a/src/include/denc.h b/src/include/denc.h
>>> index 59f7686..caa095b 100644
>>> --- a/src/include/denc.h
>>> +++ b/src/include/denc.h
>>> @@ -722,7 +722,7 @@ struct denc_traits<
>>>  template<typename T>
>>>  struct denc_traits<
>>>    std::vector<T>,
>>> -  typename std::enable_if<denc_traits<T>::supported>::type> {
>>> +  typename std::enable_if<denc_traits<T>::supported != 0>::type> {
>>>    typedef denc_traits<T> traits;
>>>  
>>>    enum { supported = true };
>>> @@ -831,7 +831,7 @@ struct denc_traits<
>>>  template<typename T>
>>>  struct denc_traits<
>>>    std::set<T>,
>>> -  typename std::enable_if<denc_traits<T>::supported>::type> {
>>> +  typename std::enable_if<denc_traits<T>::supported != 0>::type> {
>>>    typedef denc_traits<T> traits;
>>>  
>>>    enum { supported = true };
>>>
>>> which, AFAICS, is nonsense... supported is either a bool or int, and in 
>>> either case "supported" and "supported != 0" should be equivalent 
>>> (right?).  But once I get past that clang crashes a few files later... see 
>>> attached.
>>>
>>> Maybe start by trying to upgrade clang?  Or submit a clang bug report? :/
>>
>> I'll see if I can get that somewhere reported as bug since it also
>> hampers 3.8.0 .... Probably one of the tool-chain people will know.
>>
>>> (Perhaps you can use gcc for the time being?)
>>
>> Don't need to!! 8=) since 3.8.0 takes it without crash.
>> Some at least something improved in this corner of the compiler.
>>
>> But I owe you (lots of) beer, next time we meet...
>>
>> Going to check if it now completes all the way thru.
> 
> Even if it does, we shouldn't start celebrating yet!  Those are just the 
> two cases I had to change to make it get past that one error.  But if the 
> template enable_if conditions aren't properly evaluating bool and/or int 
> values then I doubt the code is compiling with the correct behavior.
> 
> There is some weirdness here because in some denc_traits instances 
> supported = a bool and sometimes support = an int.  It doesn't seem like 
> that should prevent determining if the value is true vs non-zero, 
> though.  :/

Yup, found that out the "hard" way just right now.
On the other hand Clang is rather picky in parameter matching as we
found out, so I'd expect it to complain even more if it cannot find to
appropriate function signature. Not quite sure if it would then actually
compile wrong code?

Was already mailing the freebsd-toolchain guy that has been helpful thus
far to see what he suggests we do.
The ports collection contains 3.8.1 or 3.8.0_1, and clang-devel which
has tag: 3.8.d20150720, not sure how they number the revisions for
FreeBSD. And if anything like this is "fixed".

I'll get either told it is a bug and will be fixed. or to "fix the code"
if it is a GCC liberty.

For the moment I fixed the other references in denc.h that use
'supported' without comparison, and the code seems to be compiling...

I guess tests will show fast enough, since it should complete the full
testset with the exception of ceph-disk.

--WjW

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

Willem Jan Withagen Oct. 21, 2016, 9:50 p.m. UTC | #5

On 21-10-2016 16:59, Willem Jan Withagen wrote:
> On 21-10-2016 16:32, Sage Weil wrote:
>> On Fri, 21 Oct 2016, Willem Jan Withagen wrote:
>>> On 21-10-2016 15:53, Sage Weil wrote:
>>>> On Fri, 21 Oct 2016, Willem Jan Withagen wrote:
>>>>> On 20-10-2016 18:53, Gregory Farnum wrote:
>>>>>> On Thu, Oct 20, 2016 at 3:21 AM, Willem Jan Withagen <[e-mail]> 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 <[e-mail]>
>>>>>>>> Date:   Wed Sep 14 13:32:20 2016 -0400
>>>>>>>>
>>>>>>>>     include/object: conditional denc_traits for snapid_t
>>>>>>>>
>>>>>>>>     Signed-off-by: Sage Weil <[e-mail]>
>>>>>>>
>>>>>>> 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.
>>>>
>>>> It looks like clang is buggy.  At leasy, my fedora 23 version is
>>>
>>> Hi Sage,
>>>
>>> I do not exclude that....
>>> It is going to be either buggy of picky. :)
>>>
>>>> $ clang --version
>>>> clang version 3.7.0 (tags/RELEASE_370/final)
>>>> Target: x86_64-redhat-linux-gnu
>>>> Thread model: posix
>>>
>>> I'm already at 3.8.0 because that is the default compiler for
>>> 11.0-RELEASE and futher.
>>>
>>>> This fixes the snapid_t compilation:
>>>>
>>>> diff --git a/src/include/denc.h b/src/include/denc.h
>>>> index 59f7686..caa095b 100644
>>>> --- a/src/include/denc.h
>>>> +++ b/src/include/denc.h
>>>> @@ -722,7 +722,7 @@ struct denc_traits<
>>>>  template<typename T>
>>>>  struct denc_traits<
>>>>    std::vector<T>,
>>>> -  typename std::enable_if<denc_traits<T>::supported>::type> {
>>>> +  typename std::enable_if<denc_traits<T>::supported != 0>::type> {
>>>>    typedef denc_traits<T> traits;
>>>>  
>>>>    enum { supported = true };
>>>> @@ -831,7 +831,7 @@ struct denc_traits<
>>>>  template<typename T>
>>>>  struct denc_traits<
>>>>    std::set<T>,
>>>> -  typename std::enable_if<denc_traits<T>::supported>::type> {
>>>> +  typename std::enable_if<denc_traits<T>::supported != 0>::type> {
>>>>    typedef denc_traits<T> traits;
>>>>  
>>>>    enum { supported = true };
>>>>
>>>> which, AFAICS, is nonsense... supported is either a bool or int, and in 
>>>> either case "supported" and "supported != 0" should be equivalent 
>>>> (right?).  But once I get past that clang crashes a few files later... see 
>>>> attached.
>>>>
>>>> Maybe start by trying to upgrade clang?  Or submit a clang bug report? :/
>>>
>>> I'll see if I can get that somewhere reported as bug since it also
>>> hampers 3.8.0 .... Probably one of the tool-chain people will know.
>>>
>>>> (Perhaps you can use gcc for the time being?)
>>>
>>> Don't need to!! 8=) since 3.8.0 takes it without crash.
>>> Some at least something improved in this corner of the compiler.
>>>
>>> But I owe you (lots of) beer, next time we meet...
>>>
>>> Going to check if it now completes all the way thru.
>>
>> Even if it does, we shouldn't start celebrating yet!  Those are just the 
>> two cases I had to change to make it get past that one error.  But if the 
>> template enable_if conditions aren't properly evaluating bool and/or int 
>> values then I doubt the code is compiling with the correct behavior.
>>
>> There is some weirdness here because in some denc_traits instances 
>> supported = a bool and sometimes support = an int.  It doesn't seem like 
>> that should prevent determining if the value is true vs non-zero, 
>> though.  :/
> 
> Yup, found that out the "hard" way just right now.
> On the other hand Clang is rather picky in parameter matching as we
> found out, so I'd expect it to complain even more if it cannot find to
> appropriate function signature. Not quite sure if it would then actually
> compile wrong code?
> 
> Was already mailing the freebsd-toolchain guy that has been helpful thus
> far to see what he suggests we do.
> The ports collection contains 3.8.1 or 3.8.0_1, and clang-devel which
> has tag: 3.8.d20150720, not sure how they number the revisions for
> FreeBSD. And if anything like this is "fixed".
> 
> I'll get either told it is a bug and will be fixed. or to "fix the code"
> if it is a GCC liberty.
> 
> For the moment I fixed the other references in denc.h that use
> 'supported' without comparison, and the code seems to be compiling...
> 
> I guess tests will show fast enough, since it should complete the full
> testset with the exception of ceph-disk.

What I understand from the Clang guys is that libstdc++ probably has a
template that takes enable_if() to accept things other than bool. Not
quite sure why conversion is not automated. On the other hand are all
examples that I encounter really for boll results.
Even in cases where extra templates are created like:
====
template <typename>
struct is_64_bit
{
    static const bool value = sizeof(void*) == 8;
};

template <typename T>
typename enable_if<is_64_bit<T>::value, void>::type
my_memcpy(void* target, const void* source, T n)
{
    cout << "64 bit memcpy" << endl;
}
====
Perhaps that is a possible solution here as well:
	is_supported_{0,1,2}
???

Testing the fixes I currently have there is only one failure, and even
that is an expected one:
98% tests passed, 2 tests failed out of 130

Total Test time (real) = 3576.02 sec

The following tests FAILED:
         12 - run-tox-ceph-disk (Failed)
         32 - unittest_denc (SEGFAULT)
Errors while running CTest

--WjW


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

Sage Weil Oct. 21, 2016, 10:18 p.m. UTC | #6

On Fri, 21 Oct 2016, Willem Jan Withagen wrote:
> On 21-10-2016 16:59, Willem Jan Withagen wrote:
> > On 21-10-2016 16:32, Sage Weil wrote:
> >> On Fri, 21 Oct 2016, Willem Jan Withagen wrote:
> >>> On 21-10-2016 15:53, Sage Weil wrote:
> >>>> On Fri, 21 Oct 2016, Willem Jan Withagen wrote:
> >>>>> On 20-10-2016 18:53, Gregory Farnum wrote:
> >>>>>> On Thu, Oct 20, 2016 at 3:21 AM, Willem Jan Withagen <[e-mail]> 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 <[e-mail]>
> >>>>>>>> Date:   Wed Sep 14 13:32:20 2016 -0400
> >>>>>>>>
> >>>>>>>>     include/object: conditional denc_traits for snapid_t
> >>>>>>>>
> >>>>>>>>     Signed-off-by: Sage Weil <[e-mail]>
> >>>>>>>
> >>>>>>> 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.
> >>>>
> >>>> It looks like clang is buggy.  At leasy, my fedora 23 version is
> >>>
> >>> Hi Sage,
> >>>
> >>> I do not exclude that....
> >>> It is going to be either buggy of picky. :)
> >>>
> >>>> $ clang --version
> >>>> clang version 3.7.0 (tags/RELEASE_370/final)
> >>>> Target: x86_64-redhat-linux-gnu
> >>>> Thread model: posix
> >>>
> >>> I'm already at 3.8.0 because that is the default compiler for
> >>> 11.0-RELEASE and futher.
> >>>
> >>>> This fixes the snapid_t compilation:
> >>>>
> >>>> diff --git a/src/include/denc.h b/src/include/denc.h
> >>>> index 59f7686..caa095b 100644
> >>>> --- a/src/include/denc.h
> >>>> +++ b/src/include/denc.h
> >>>> @@ -722,7 +722,7 @@ struct denc_traits<
> >>>>  template<typename T>
> >>>>  struct denc_traits<
> >>>>    std::vector<T>,
> >>>> -  typename std::enable_if<denc_traits<T>::supported>::type> {
> >>>> +  typename std::enable_if<denc_traits<T>::supported != 0>::type> {
> >>>>    typedef denc_traits<T> traits;
> >>>>  
> >>>>    enum { supported = true };
> >>>> @@ -831,7 +831,7 @@ struct denc_traits<
> >>>>  template<typename T>
> >>>>  struct denc_traits<
> >>>>    std::set<T>,
> >>>> -  typename std::enable_if<denc_traits<T>::supported>::type> {
> >>>> +  typename std::enable_if<denc_traits<T>::supported != 0>::type> {
> >>>>    typedef denc_traits<T> traits;
> >>>>  
> >>>>    enum { supported = true };
> >>>>
> >>>> which, AFAICS, is nonsense... supported is either a bool or int, and in 
> >>>> either case "supported" and "supported != 0" should be equivalent 
> >>>> (right?).  But once I get past that clang crashes a few files later... see 
> >>>> attached.
> >>>>
> >>>> Maybe start by trying to upgrade clang?  Or submit a clang bug report? :/
> >>>
> >>> I'll see if I can get that somewhere reported as bug since it also
> >>> hampers 3.8.0 .... Probably one of the tool-chain people will know.
> >>>
> >>>> (Perhaps you can use gcc for the time being?)
> >>>
> >>> Don't need to!! 8=) since 3.8.0 takes it without crash.
> >>> Some at least something improved in this corner of the compiler.
> >>>
> >>> But I owe you (lots of) beer, next time we meet...
> >>>
> >>> Going to check if it now completes all the way thru.
> >>
> >> Even if it does, we shouldn't start celebrating yet!  Those are just the 
> >> two cases I had to change to make it get past that one error.  But if the 
> >> template enable_if conditions aren't properly evaluating bool and/or int 
> >> values then I doubt the code is compiling with the correct behavior.
> >>
> >> There is some weirdness here because in some denc_traits instances 
> >> supported = a bool and sometimes support = an int.  It doesn't seem like 
> >> that should prevent determining if the value is true vs non-zero, 
> >> though.  :/
> > 
> > Yup, found that out the "hard" way just right now.
> > On the other hand Clang is rather picky in parameter matching as we
> > found out, so I'd expect it to complain even more if it cannot find to
> > appropriate function signature. Not quite sure if it would then actually
> > compile wrong code?
> > 
> > Was already mailing the freebsd-toolchain guy that has been helpful thus
> > far to see what he suggests we do.
> > The ports collection contains 3.8.1 or 3.8.0_1, and clang-devel which
> > has tag: 3.8.d20150720, not sure how they number the revisions for
> > FreeBSD. And if anything like this is "fixed".
> > 
> > I'll get either told it is a bug and will be fixed. or to "fix the code"
> > if it is a GCC liberty.
> > 
> > For the moment I fixed the other references in denc.h that use
> > 'supported' without comparison, and the code seems to be compiling...
> > 
> > I guess tests will show fast enough, since it should complete the full
> > testset with the exception of ceph-disk.
> 
> What I understand from the Clang guys is that libstdc++ probably has a
> template that takes enable_if() to accept things other than bool. Not
> quite sure why conversion is not automated. On the other hand are all
> examples that I encounter really for boll results.
> Even in cases where extra templates are created like:
> ====
> template <typename>
> struct is_64_bit
> {
>     static const bool value = sizeof(void*) == 8;
> };
> 
> template <typename T>
> typename enable_if<is_64_bit<T>::value, void>::type
> my_memcpy(void* target, const void* source, T n)
> {
>     cout << "64 bit memcpy" << endl;
> }
> ====
> Perhaps that is a possible solution here as well:
> 	is_supported_{0,1,2}
> ???

That makes some sense, I guess.  For now, I think this is a minimal fix to 
avoid the issue:

	https://github.com/ceph/ceph/pull/11605

Can you try it?

(Also, I see there are a bunch of other concerning warnings we should 
fix... mostly improper use of std::move.  When I get back from ODS maybe 
I'll upgrade to fedora 24, get a less-crashy clang, and try using it for a 
while.)

sage

> 
> Testing the fixes I currently have there is only one failure, and even
> that is an expected one:
> 98% tests passed, 2 tests failed out of 130
> 
> Total Test time (real) = 3576.02 sec
> 
> The following tests FAILED:
>          12 - run-tox-ceph-disk (Failed)
>          32 - unittest_denc (SEGFAULT)
> Errors while running CTest
> 
> --WjW
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to [e-mail]
> 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 [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/src/include/denc.h b/src/include/denc.h
index 59f7686..caa095b 100644
--- a/src/include/denc.h
+++ b/src/include/denc.h
@@ -722,7 +722,7 @@  struct denc_traits<
 template<typename T>
 struct denc_traits<
   std::vector<T>,
-  typename std::enable_if<denc_traits<T>::supported>::type> {
+  typename std::enable_if<denc_traits<T>::supported != 0>::type> {
   typedef denc_traits<T> traits;
 
   enum { supported = true };
@@ -831,7 +831,7 @@  struct denc_traits<
 template<typename T>
 struct denc_traits<
   std::set<T>,
-  typename std::enable_if<denc_traits<T>::supported>::type> {
+  typename std::enable_if<denc_traits<T>::supported != 0>::type> {
   typedef denc_traits<T> traits;
 
   enum { supported = true };