Skip to content

Conversation

@Barthelemy
Copy link
Collaborator

No description provided.

@Barthelemy
Copy link
Collaborator Author

(I hope boost frees the memory returned by what())

Copy link
Contributor

@matthiasrichter matthiasrichter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking into this.

I would not restrict this to errinfo_object_name as information type. Or if it is restricted to that type, all other types should be caught by compile time error. E.g. if one simply uses

ObjectNotFoundError() << "object1"

the current implementation will pretend that the object name is not available, thus ignoring intention of the source code. Though I have not been working with boot::get_error_info, maybe this is handled transparently.

{
return "Object not found error";
std::string message = "Object not found: ";
auto* errinfo = boost::get_error_info<errinfo_object_name>(*this);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this implies that one can only write errinfo_object_name as additional exception info, correct. If this fails, maybe try to extract string?

@Barthelemy
Copy link
Collaborator Author

Passing a string to the exception with << is not possible at the moment, it will trigger a compilation error.

I think that using a errinfo is the correct way of doing it with boost::exception. Of course the extra information has to be provided, under the correct name. I have checked and we do it wherever we use this type of exception in the QC framework.

With the proposed fix, the name of the missing object will be printed in the use case you had in the original PR.

What will definitely help is catching the exception before it bubbles up outside the framework, print the exception with diagnostic_information(), then raise again if it is fatal. This way we are sure to print all the extra information added.

@MichaelLettrich
Copy link

MichaelLettrich commented May 14, 2020

Hi, it seems that there is a problem with building O2 on macOS and arrow/libgandiva... can anyone with a mac check with the lastest alidist if this is a problem with the CI configuration or if this is a genuine problem? @Barthelemy , @ktf

CMake Error at dependencies/Findarrow.cmake:24 (set_target_properties):
  set_target_properties Can not find target to add properties to:
  gandiva_shared
Call Stack (most recent call first):
  dependencies/O2Dependencies.cmake:39 (find_package)
  dependencies/CMakeLists.txt:11 (include)
  CMakeLists.txt:45 (include)


CMake Error at dependencies/Findarrow.cmake:25 (add_library):
  add_library cannot create ALIAS target "arrow::gandiva_shared" because
  target "gandiva_shared" does not already exist.
Call Stack (most recent call first):
  dependencies/O2Dependencies.cmake:39 (find_package)
  dependencies/CMakeLists.txt:11 (include)
  CMakeLists.txt:45 (include)

@Barthelemy
Copy link
Collaborator Author

I am trying to build on my mac but it decided to rebuild Clang 😠

@MichaelLettrich
Copy link

@Barthelemy: @ktf told me that a new tag for O2 should fix it. Let's see if it does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

5 participants