Bug 89502

Summary: Compilation failure with clang - minor cleanup patch to ProfileData and Gfx
Product: poppler Reporter: Kurt Schwehr <schwehr>
Component: cpp frontendAssignee: poppler-bugs <poppler-bugs>
Status: RESOLVED WONTFIX QA Contact:
Severity: normal    
Priority: medium    
Version: unspecified   
Hardware: All   
OS: Linux (All)   
Whiteboard:
i915 platform: i915 features:
Attachments: Patch to poppler 0.32.0 for ProfileData.{cc,h} and Gfx.cc
Version 2 - Only a minor cleanup of ProfileData

Description Kurt Schwehr 2015-03-09 12:56:11 UTC
Created attachment 114163 [details]
Patch to poppler 0.32.0 for ProfileData.{cc,h} and Gfx.cc

I got a link error for multiply defined ProfileData classes.  The attached patch moves the default constructor for ProfileData from the implementation .cc to the header to fix the linking issue.  The compiler was creating a default constructor when seeing the header and that constructor then clashed with the one created by ProfileData.cc.  The patch also switches the initialization of data members to an initializer list.

While debugging this, I looked at Gfx.cc and did some minor cleanup on the use of ProfileData.

I'm using a non-standard build system and GCC 4.9.x with various patches, but this patch should be safe for any platform.

Patch is against 0.32.0.
Comment 1 Albert Astals Cid 2015-03-09 23:52:44 UTC
Why would the compiler create a default constructor when seeing the header? What's special about ProfileData? Wouldn't you have the same problems with all the other classes?
Comment 2 Kurt Schwehr 2015-03-10 04:52:01 UTC
This works around a collision with the name ProfileData coming from another part of a system.  The ultimate solution is to wrap ProfileData (and most everything else C++ in poppler) in the Poppler or other namespace.  But this patch seems like a reasonable small start.
Comment 3 Kurt Schwehr 2015-03-10 14:51:25 UTC
After looking at the issue, I need to modify my patch.  There is no need to move the definition to the header.  So the next version of the patch becomes pretty trivial cleanup and still leaves the need for overall namespace useage in poppler.
Comment 4 Kurt Schwehr 2015-03-10 21:23:44 UTC
Created attachment 114210 [details]
Version 2 - Only a minor cleanup of ProfileData
Comment 5 Albert Astals Cid 2015-03-11 22:59:58 UTC
Sorry but i don't see the need of that patch, just creates noise in the git log for no real gain.
Comment 6 Kurt Schwehr 2015-03-11 23:09:41 UTC
Totally your call.
Comment 7 Albert Astals Cid 2015-03-11 23:23:53 UTC
Closing then.

About the other patch, more than a namespace i'd suggest working on a patch to hide the symbols

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.