Bug 89502 - Compilation failure with clang - minor cleanup patch to ProfileData and Gfx
Summary: Compilation failure with clang - minor cleanup patch to ProfileData and Gfx
Status: RESOLVED WONTFIX
Alias: None
Product: poppler
Classification: Unclassified
Component: cpp frontend (show other bugs)
Version: unspecified
Hardware: All Linux (All)
: medium normal
Assignee: poppler-bugs
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-03-09 12:56 UTC by Kurt Schwehr
Modified: 2015-03-11 23:23 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Patch to poppler 0.32.0 for ProfileData.{cc,h} and Gfx.cc (1.41 KB, text/plain)
2015-03-09 12:56 UTC, Kurt Schwehr
Details
Version 2 - Only a minor cleanup of ProfileData (1.15 KB, text/plain)
2015-03-10 21:23 UTC, Kurt Schwehr
Details

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.