Ticket #2345 (closed: fixed)

Opened 10 years ago

Last modified 5 years ago

Sort out DLLExport/DLLImport usage

Reported by: Martyn Gigg Owned by: Martyn Gigg
Priority: minor Milestone: Iteration 29
Component: Mantid Keywords:
Cc: Blocked By:
Blocking: Tester: Peter Peterson

Description

Our usage of DLLExport/DLLImport is in inconsistent and in some places plain wrong.

System.h defines DLLExport and DLLImport but it is the responsibility of each package to set an additional macro, EXPORT_OPT_IN_API for example in API/DLLExport.h, to switch between DLLExport/DLLImport based on whether that package is under compilation or being linked against.

Each class should then be marked as (with API as an example)

class EXPORT_OPT_IN_API IAlgorithm 
...

and NOT

class DLLExport IAlgorithm 
...

as this second declaration causes in an incorrect import when the library is used and in some circumstances will prevent symbols from being visible.

This was the case recently with API::IDataFileChecker that was marked as DLLExport. When the Nexus library tried to see some static data members it couldn't because they weren't imported correctly as the macro hadn't switched to DLLImport.

Change History

comment:1 Changed 10 years ago by Nick Draper

  • Milestone changed from Iteration 27 to Iteration 28

Bulk move of tickets at the end of iteration 27

comment:2 Changed 9 years ago by Nick Draper

  • Status changed from new to assigned
  • Owner set to Martyn Gigg

comment:3 Changed 9 years ago by Nick Draper

  • Milestone changed from Iteration 28 to Iteration 29

Bulk move of tickets at the end of iteration 28

comment:4 Changed 9 years ago by Martyn Gigg

(In [12406]) Refs #3157 Sort our System.h. It was becoming a dumping ground. It should only be touched when absolutely necessary as it affects every file in Mantid. Also combined this with Refs #2345 to sort out DLLExport/DLLImport usage. It's done for Kernel, Geometry and API but stuff in the plugins could do with updating as well.

comment:5 Changed 9 years ago by Martyn Gigg

(In [12407]) Fix for compiler directive Re #2345"

comment:6 Changed 9 years ago by Martyn Gigg

  • Status changed from assigned to accepted

comment:7 Changed 9 years ago by Martyn Gigg

  • Status changed from accepted to verify
  • Resolution set to fixed

comment:8 Changed 9 years ago by Peter Peterson

  • Status changed from verify to verifying
  • Tester set to Peter Peterson

comment:9 Changed 9 years ago by Peter Peterson

  • Status changed from verifying to closed

This is a ton of changes that are really hard to declare victory with. The fact that the code compiles/d on a all of the build servers is all of the verification that I think we can ever hope for.

comment:10 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 3192

Note: See TracTickets for help on using tickets.