erichkeane marked an inline comment as done. erichkeane added inline comments.
================ Comment at: cfe/trunk/include/clang/Basic/AttributeCommonInfo.h:166 + ? SpellingIndex + : calculateAttributeSpellingListIndex(); + } ---------------- plotfi wrote: > erichkeane wrote: > > aheejin wrote: > > > MaskRay wrote: > > > > calculateAttributeSpellingListIndex is defined in clangSema. This can > > > > cause lib/libclangAST.so.10svn (-DBUILD_SHARED_LIBS=on) fail to link: > > > > > > > > ``` > > > > ld.lld: error: undefined symbol: > > > > clang::AttributeCommonInfo::calculateAttributeSpellingListIndex() const > > > > > > > > >>> referenced by AttributeCommonInfo.h:166 > > > > >>> (../tools/clang/include/clang/Basic/AttributeCommonInfo.h:166) > > > > >>> > > > > >>> tools/clang/lib/AST/CMakeFiles/obj.clangAST.dir/AttrImpl.cpp.o:(clang::AttributeCommonInfo::getAttributeSpellingListIndex() > > > > >>> const) > > > > ``` > > > +1 This fails builds with `-DBUILD_SHARED_LIBS=ON`. I tried to add > > > `clangSema` as a dependent library to `clangAST`, but this creates > > > several circular dependencies. > > Thanks for the heads up. The solution will just end up being moving that > > function's definition. I'll submit a patch on monday. Thanks for the > > reproducer. > Normally I’d suggest a revert, but I can see downstream some stuff with Swift > and apinotes was not completely trivial to get merged in with this patch. Is > simply moving the definition the right solution here btw? Yep, exactly. I would prefer someone to just fix it (since it IS something quite trivial) rather than revert. This ends up being a massive change for the purposes of improving diagnostics here and downstream. Typically it is considered somewhat fair to give the author time to fix a failure before reverting, and unfortunately this was discovered at 8pm on a Friday, so it seems longer than this would typically take. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67368/new/ https://reviews.llvm.org/D67368 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
