[PATCH] D26671: Replace APFloatBase static fltSemantics data members with getter functions

Stephan Bergmann via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 5 02:48:17 PST 2016


sberg added a comment.

In https://reviews.llvm.org/D26671#612164, @zturner wrote:

> I don't know anything about the code in question, but from a purely design perspective, it looks like `fltSemantics` is only forward declared in the header, and as such no external client can ever use a `fltSemantics` for anything meaningful anyway.  So it looks like it's intended to be an opaque data structure only useful within the confines of `APFloat.cpp`.
>
> If this is the case, why don't we remove the forward declarations entirely, and write something like: `typedef void* FLT_SEMANTICS`, get rid of all the getters and convert them back into data members, and have the data members declared as actual declared in `APFloat.h` as being of type `FLT_SEMANTICS` (feel free to choose a different name).
>
> Returning pointers across DLL boundaries is fine, and this way you don 't even need to export anything.


I don't see how changing those static data members from type fltSemantics to fltSemantics* or void* would change anything wrt the original MSVC export issue.  While code outside APFloat.cpp appears to indeed be only interested in opaque pointers to those objects, it still needs to obtain those pointers, and doesn't do so via the return values of existing functions.  (What /could/ be changed in the patch is to make the IEEEhalf etc. functions return fltSemantics* instead of fltSemantics&; I'm dispassionate about that.)


https://reviews.llvm.org/D26671





More information about the llvm-commits mailing list