# HG changeset patch # User Sebastien Jodogne # Date 1604673450 -3600 # Node ID 44b53a2c0a130ca89af388d56052cd6aa36da26a # Parent 4c91fbede7d25450e7b69b1b0b4a9d04ef1bb047 improving detection of ABI compatibility diff -r 4c91fbede7d2 -r 44b53a2c0a13 OrthancFramework/Resources/CheckOrthancFrameworkSymbols.py --- a/OrthancFramework/Resources/CheckOrthancFrameworkSymbols.py Fri Nov 06 10:00:05 2020 +0100 +++ b/OrthancFramework/Resources/CheckOrthancFrameworkSymbols.py Fri Nov 06 15:37:30 2020 +0100 @@ -79,14 +79,57 @@ f.write('#include "%s"\n' % source) -tu = index.parse(AMALGAMATION, - [ '-DORTHANC_BUILDING_FRAMEWORK_LIBRARY=1' ]) +tu = index.parse(AMALGAMATION, [ + '-DORTHANC_BUILDING_FRAMEWORK_LIBRARY=1', + '-DORTHANC_BUILD_UNIT_TESTS=0', + '-DORTHANC_SANDBOXED=0', + '-DORTHANC_ENABLE_BASE64=1', + '-DORTHANC_ENABLE_CIVETWEB=1', + '-DORTHANC_ENABLE_CURL=1', + '-DORTHANC_ENABLE_DCMTK=1', + '-DORTHANC_ENABLE_DCMTK_JPEG=1', + '-DORTHANC_ENABLE_DCMTK_NETWORKING=1', + '-DORTHANC_ENABLE_DCMTK_TRANSCODING=1', + '-DORTHANC_ENABLE_JPEG=1', + '-DORTHANC_ENABLE_LOCALE=1', + '-DORTHANC_ENABLE_LUA=1', + '-DORTHANC_ENABLE_LOGGING=1', + '-DORTHANC_ENABLE_MD5=1', + '-DORTHANC_ENABLE_PKCS11=1', + '-DORTHANC_ENABLE_PNG=1', + '-DORTHANC_ENABLE_PUGIXML=1', + '-DORTHANC_ENABLE_SSL=1', + '-DORTHANC_SQLITE_STANDALONE=0', + '-DORTHANC_ENABLE_LOGGING_STDIO=0', +]) FILES = [] COUNT = 0 +def ReportProblem(message, fqn, cursor): + global FILES, COUNT + FILES.append(os.path.normpath(str(cursor.location.file))) + COUNT += 1 + + print('%s: %s::%s()' % (message, '::'.join(fqn), cursor.spelling)) + + def ExploreClass(child, fqn): + # Safety check + if (child.kind != clang.cindex.CursorKind.CLASS_DECL and + child.kind != clang.cindex.CursorKind.STRUCT_DECL): + raise Exception() + + # Ignore forward declaration of classes + if not child.is_definition(): + return + + + ## + ## Verify that the class is publicly exported (its visibility must + ## be "default") + ## visible = False for i in child.get_children(): @@ -94,34 +137,113 @@ i.spelling == 'default'): visible = True - if visible: - isPublic = (child.kind == clang.cindex.CursorKind.STRUCT_DECL) + if not visible: + return + + + ## + ## Ignore pure abstract interfaces, by checking the following + ## criteria: + ## - It must be a C++ class (not a struct) + ## - The class name must start with "I" + ## - All its methods must be pure virtual (abstract) and public + ## - Its destructor must be public, virtual, and must do nothing + ## + + if (child.kind == clang.cindex.CursorKind.CLASS_DECL and + fqn[-1].startswith('I')): + abstract = True + isPublic = False for i in child.get_children(): - if i.kind == clang.cindex.CursorKind.CXX_ACCESS_SPEC_DECL: + if i.kind == clang.cindex.CursorKind.VISIBILITY_ATTR: # "default" + pass + elif i.kind == clang.cindex.CursorKind.CXX_ACCESS_SPEC_DECL: isPublic = (i.access_specifier == clang.cindex.AccessSpecifier.PUBLIC) + elif i.kind == clang.cindex.CursorKind.CXX_BASE_SPECIFIER: + if i.spelling != 'boost::noncopyable': + abstract = False + elif isPublic: + if i.kind == clang.cindex.CursorKind.CXX_METHOD: + if i.is_pure_virtual_method(): + pass # pure virtual is ok + elif i.is_static_method(): + # static method without an inline implementation is ok + for j in i.get_children(): + if j.kind == clang.cindex.CursorKind.COMPOUND_STMT: + abstract = False + else: + abstract = False + elif (i.kind == clang.cindex.CursorKind.DESTRUCTOR and + i.is_virtual_method()): + # The destructor must be virtual, and must do nothing + c = list(i.get_children()) + if (len(c) != 1 or + c[0].kind != clang.cindex.CursorKind.COMPOUND_STMT or + len(list(c[0].get_children())) != 0): + abstract = False + elif (i.kind == clang.cindex.CursorKind.CLASS_DECL or + i.kind == clang.cindex.CursorKind.STRUCT_DECL): + ExploreClass(i, fqn + [ i.spelling ]) + else: + abstract = False - elif (i.kind == clang.cindex.CursorKind.CLASS_DECL or - i.kind == clang.cindex.CursorKind.STRUCT_DECL): - # This is a subclass + if abstract: + print('Detected a pure interface (this is fine): %s' % ('::'.join(fqn))) + return + + + ## + ## We are facing a standard C++ class or struct + ## + + isPublic = (child.kind == clang.cindex.CursorKind.STRUCT_DECL) + + for i in child.get_children(): + if (i.kind == clang.cindex.CursorKind.VISIBILITY_ATTR or # "default" + i.kind == clang.cindex.CursorKind.CXX_BASE_SPECIFIER): # base class + pass + + elif i.kind == clang.cindex.CursorKind.CXX_ACCESS_SPEC_DECL: + isPublic = (i.access_specifier == clang.cindex.AccessSpecifier.PUBLIC) + + elif (i.kind == clang.cindex.CursorKind.CLASS_DECL or + i.kind == clang.cindex.CursorKind.STRUCT_DECL): + # This is a subclass + if isPublic: ExploreClass(i, fqn + [ i.spelling ]) - - elif (i.kind == clang.cindex.CursorKind.CXX_METHOD or - i.kind == clang.cindex.CursorKind.CONSTRUCTOR or - i.kind == clang.cindex.CursorKind.DESTRUCTOR): - if isPublic: - hasImplementation = False - for j in i.get_children(): - if j.kind == clang.cindex.CursorKind.COMPOUND_STMT: - hasImplementation = True + + elif (i.kind == clang.cindex.CursorKind.CXX_METHOD or + i.kind == clang.cindex.CursorKind.CONSTRUCTOR or + i.kind == clang.cindex.CursorKind.DESTRUCTOR): + if isPublic: + hasImplementation = False + for j in i.get_children(): + if j.kind == clang.cindex.CursorKind.COMPOUND_STMT: + hasImplementation = True + + if hasImplementation: + ReportProblem('Exported public method with an implementation', fqn, i) - if hasImplementation: - global FILES, COUNT - FILES.append(os.path.normpath(str(child.location.file))) - COUNT += 1 + elif i.kind == clang.cindex.CursorKind.VAR_DECL: + if isPublic: + ReportProblem('Exported public member variable', fqn, i) + + elif i.kind == clang.cindex.CursorKind.FUNCTION_TEMPLATE: + if isPublic: + ReportProblem('Exported public template method', fqn, i) - print('Exported public method with an implementation: %s::%s()' % - ('::'.join(fqn), i.spelling)) + elif i.kind == clang.cindex.CursorKind.FRIEND_DECL: + if isPublic: + ReportProblem('Exported public friend method', fqn, i) + + elif (i.kind == clang.cindex.CursorKind.TYPEDEF_DECL or # Allow "typedef" + i.kind == clang.cindex.CursorKind.ENUM_DECL): # Allow enums + pass + + else: + if isPublic: + raise Exception('Unsupported: %s, %s' % (i.kind, i.location)) def ExploreNamespace(node, namespace): @@ -135,6 +257,10 @@ child.kind == clang.cindex.CursorKind.STRUCT_DECL): ExploreClass(child, fqn) + + +print('') + for node in tu.cursor.get_children(): if (node.kind == clang.cindex.CursorKind.NAMESPACE and node.spelling == 'Orthanc'): @@ -143,7 +269,8 @@ print('\nTotal of possibly problematic methods: %d' % COUNT) -print('\nFiles:\n') +print('\nProblematic files:\n') for i in sorted(list(set(FILES))): print(i) - + +print('') diff -r 4c91fbede7d2 -r 44b53a2c0a13 OrthancFramework/Sources/DicomParsing/IDicomTranscoder.cpp --- a/OrthancFramework/Sources/DicomParsing/IDicomTranscoder.cpp Fri Nov 06 10:00:05 2020 +0100 +++ b/OrthancFramework/Sources/DicomParsing/IDicomTranscoder.cpp Fri Nov 06 15:37:30 2020 +0100 @@ -66,11 +66,6 @@ } - IDicomTranscoder::~IDicomTranscoder() - { - } - - std::string IDicomTranscoder::GetSopInstanceUid(DcmFileFormat& dicom) { if (dicom.getDataset() == NULL) diff -r 4c91fbede7d2 -r 44b53a2c0a13 OrthancFramework/Sources/DicomParsing/IDicomTranscoder.h --- a/OrthancFramework/Sources/DicomParsing/IDicomTranscoder.h Fri Nov 06 10:00:05 2020 +0100 +++ b/OrthancFramework/Sources/DicomParsing/IDicomTranscoder.h Fri Nov 06 15:37:30 2020 +0100 @@ -105,7 +105,9 @@ bool allowNewSopInstanceUid); public: - virtual ~IDicomTranscoder(); + virtual ~IDicomTranscoder() + { + } virtual bool Transcode(DicomImage& target, DicomImage& source /* in, "GetParsed()" possibly modified */, diff -r 4c91fbede7d2 -r 44b53a2c0a13 OrthancFramework/Sources/JobsEngine/JobsRegistry.cpp --- a/OrthancFramework/Sources/JobsEngine/JobsRegistry.cpp Fri Nov 06 10:00:05 2020 +0100 +++ b/OrthancFramework/Sources/JobsEngine/JobsRegistry.cpp Fri Nov 06 15:37:30 2020 +0100 @@ -41,11 +41,6 @@ static const char* RUNTIME = "Runtime"; - JobsRegistry::IObserver::~IObserver() - { - } - - class JobsRegistry::JobHandler : public boost::noncopyable { private: diff -r 4c91fbede7d2 -r 44b53a2c0a13 OrthancFramework/Sources/JobsEngine/JobsRegistry.h --- a/OrthancFramework/Sources/JobsEngine/JobsRegistry.h Fri Nov 06 10:00:05 2020 +0100 +++ b/OrthancFramework/Sources/JobsEngine/JobsRegistry.h Fri Nov 06 15:37:30 2020 +0100 @@ -48,7 +48,9 @@ class ORTHANC_PUBLIC IObserver : public boost::noncopyable { public: - virtual ~IObserver(); + virtual ~IObserver() + { + } virtual void SignalJobSubmitted(const std::string& jobId) = 0; diff -r 4c91fbede7d2 -r 44b53a2c0a13 OrthancFramework/Sources/JobsEngine/SetOfCommandsJob.cpp --- a/OrthancFramework/Sources/JobsEngine/SetOfCommandsJob.cpp Fri Nov 06 10:00:05 2020 +0100 +++ b/OrthancFramework/Sources/JobsEngine/SetOfCommandsJob.cpp Fri Nov 06 15:37:30 2020 +0100 @@ -32,14 +32,6 @@ namespace Orthanc { - SetOfCommandsJob::ICommand::~ICommand() - { - } - - SetOfCommandsJob::ICommandUnserializer::~ICommandUnserializer() - { - } - SetOfCommandsJob::SetOfCommandsJob() : started_(false), permissive_(false), diff -r 4c91fbede7d2 -r 44b53a2c0a13 OrthancFramework/Sources/JobsEngine/SetOfCommandsJob.h --- a/OrthancFramework/Sources/JobsEngine/SetOfCommandsJob.h Fri Nov 06 10:00:05 2020 +0100 +++ b/OrthancFramework/Sources/JobsEngine/SetOfCommandsJob.h Fri Nov 06 15:37:30 2020 +0100 @@ -36,7 +36,9 @@ class ICommand : public boost::noncopyable { public: - virtual ~ICommand(); + virtual ~ICommand() + { + } virtual bool Execute(const std::string& jobId) = 0; @@ -46,7 +48,9 @@ class ICommandUnserializer : public boost::noncopyable { public: - virtual ~ICommandUnserializer(); + virtual ~ICommandUnserializer() + { + } virtual ICommand* Unserialize(const Json::Value& source) const = 0; }; diff -r 4c91fbede7d2 -r 44b53a2c0a13 OrthancFramework/Sources/RestApi/RestApiHierarchy.cpp --- a/OrthancFramework/Sources/RestApi/RestApiHierarchy.cpp Fri Nov 06 10:00:05 2020 +0100 +++ b/OrthancFramework/Sources/RestApi/RestApiHierarchy.cpp Fri Nov 06 15:37:30 2020 +0100 @@ -167,11 +167,6 @@ } - RestApiHierarchy::IVisitor::~IVisitor() - { - } - - void RestApiHierarchy::DeleteChildren(Children& children) { for (Children::iterator it = children.begin(); diff -r 4c91fbede7d2 -r 44b53a2c0a13 OrthancFramework/Sources/RestApi/RestApiHierarchy.h --- a/OrthancFramework/Sources/RestApi/RestApiHierarchy.h Fri Nov 06 10:00:05 2020 +0100 +++ b/OrthancFramework/Sources/RestApi/RestApiHierarchy.h Fri Nov 06 15:37:30 2020 +0100 @@ -70,7 +70,9 @@ class IVisitor : public boost::noncopyable { public: - virtual ~IVisitor(); + virtual ~IVisitor() + { + } virtual bool Visit(const Resource& resource, const UriComponents& uri,