RFR; 8077420: Build failure with SS12u4

[David Holmes]

Hi Stefan,

On 13/04/2015 10:52 PM, Stefan Karlsson wrote:
> Hi David,
>
> On 2015-04-13 14:25, David Holmes wrote:
>> Hi Stefan,
>>
>> On 13/04/2015 9:57 PM, Stefan Karlsson wrote:
>>> Hi,
>>>
>>> Please review this fix for a build failure that was encountered during
>>> the Solaris Studio compiler upgrade.
>>
>> So what was it about the compiler upgrade that exposed this ??
>
> I'm not sure, but somehow the function wasn't made available to the
> compiler/linker. Maybe there's a combination of the inline keyword and
> template functions that used to work, but doesn't anymore? I don't have
> access to a SS12u4 setup, so Erik J had to compile with the patch.

Ok. I'm seeing some other weird stuff coming out in relation to SS12u4. :)

>>
>>> Two files were missing an include of stack.inline.hpp, which is needed
>>> to get the definition of default_segment_size(). This function is used
>>> in the stack.hpp, to provide the default value to the constructor. This
>>> forces all users of Stacks with the default segment size, to have to
>>> include stack.inline.hpp. The default_segment_size() implementation
>>> always evaluates to the same value, given the template parameters, so I
>>> chose to setup a constant in the header instead of adding the missing
>>> stack.inline.hpp files.
>>>
>>> http://cr.openjdk.java.net/~stefank/8077420/webrev.01/
>>> https://bugs.openjdk.java.net/browse/JDK-8077420
>>
>> This seems okay but I have to say I remain very confused about the
>> "right" way to deal with .hpp and .inline.hpp files in general.
>
> OK. One rule to honor, as far as possible, is to _not_ use functions in
> inline.hpp files from .hpp files. In this case stack.hpp used a function
> in stack.inline.hpp.

That seems reasonable given that if you have a .inline.hpp I would 
expect that all defined functions should be in there. Having some 
functions in the .hpp and some in the .inline.hpp seems a recipe for 
total confusion. :)

> Maybe there's a more authoritative place that describes the "right way",
> but I think this post is worth reading and describes it well enough:
> http://www.cplusplus.com/forum/articles/10627/

It doesn't really address the .inline.hpp, as it never states when/where 
to include the .inline.hpp file - there is even some discussion in the 
comments about the problems relating to wanting functions inlined if not 
in the included header, but no resolution. He also says that including 
the .hpp in the .inline.hpp is harmless but unnecessary - which suggest 
he expected client code to include both of them (whereas I think what 
has been advocated in hotspot lately is for client code to include the 
.inline.hpp and for the the .inline.hpp to include the .hpp).

I did like the simple rules regarding forward declarations versus 
include dependencies. My C++ introduction was in an environment where 
forward declarations were seen as a hack to deal with circular 
dependencies, rather than being a first class mechanism for introducing 
type names.

Cheers,
David

> Thanks,
> StefanK
>
>>
>> Thanks,
>> David
>>
>>> Tested with JPRT, and built with SS12u4.
>>>
>>> Thanks,
>>> StefanK
>