Azure / azure-sdk-for-cpp

This repository is for active development of the Azure SDK for C++. For consumers of the SDK we recommend visiting our versioned developer docs at https://azure.github.io/azure-sdk-for-cpp.
MIT License
181 stars 126 forks source link

link.cpp within AMQP has a compile error when the BUILD_TESTING is on. #5495

Open ahsonkhan opened 7 months ago

ahsonkhan commented 7 months ago

https://github.com/Azure/azure-sdk-for-cpp/pull/5419#discussion_r1552621435

[ 19%] Building CXX object sdk/attestation/azure-security-attestation/CMakeFiles/azure-security-attestation.dir/src/attestation_client_options.cpp.o
/mnt/vss/_work/1/s/sdk/core/azure-core-amqp/src/amqp/link.cpp: In static member function ‘static void Azure::Core::Amqp::_detail::LinkImpl::OnLinkFlowOnFn(void*)’:
/mnt/vss/_work/1/s/sdk/core/azure-core-amqp/src/amqp/link.cpp:470:42: error: cannot convert ‘Azure::Core::Amqp::_detail::Link’ to ‘const std::shared_ptr<Azure::Core::Amqp::_detail::LinkImpl>&’
  470 |       link->m_eventHandler->OnLinkFlowOn(Link{link->shared_from_this()});
      |                                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                          |
      |                                          Azure::Core::Amqp::_detail::Link
In file included from /mnt/vss/_work/1/s/sdk/core/azure-core-amqp/src/amqp/link.cpp:12:
/mnt/vss/_work/1/s/sdk/core/azure-core-amqp/src/amqp/private/link_impl.hpp:35:64: note:   initializing argument 1 of ‘virtual void Azure::Core::Amqp::_detail::LinkImplEvents::OnLinkFlowOn(const std::shared_ptr<Azure::Core::Amqp::_detail::LinkImpl>&)’
   35 |     virtual void OnLinkFlowOn(std::shared_ptr<LinkImpl> const& link) = 0;
      |                               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
LarryOsterman commented 7 months ago

What action should be taken w.r.t. this bug?

My analysis of the compiler error you reference above shows that the code as checked in is correct., the problem was an incorrect attempt at renaming the TESTING_BUILD macro.

ahsonkhan commented 7 months ago

@LarryOsterman the line of code within the ifdef with BUILD_TESTING doesn't compile (line 470 below). If it's dead code, it should be removed. It currently being unreachable doesn't mean the checked in code is correct.

That is a CMake flag and not one we use in source directly. The code in question isn't using TESTING_BUILD.

https://github.com/Azure/azure-sdk-for-cpp/blob/da1513977a4ef91a3c0f7ee9c4766c9019766ba8/sdk/core/azure-core-amqp/src/amqp/link.cpp#L469-L473

ahsonkhan commented 7 months ago

Re-opening, but not marking as release blocking, since it looks like the code that doesn't compile is unreachable.