Add FINAL procedures for ParameterLists type

[rks171]

Note that I'm getting a ton of build warnings after adding this of the variety:

Warning: Only array FINAL procedures declared for derived type ‘paramtype’ defined at (1), suggest also scalar one [-Wsurprising]

However, as far as I can tell, the procedures are scalar ones. I think it might be related to this compiler bug:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58175

Which was detected in gcc 4.9 and persists even in 6.0...

[bkochuna]

Hey Bob, what's driving this?

Overall, use of final procedures for this module would be a good thing. It would greatly cut down on memory leaks. But this will break backwards compatibility with GCC 4.8.3 and likely a bunch of other versions between 4.8.3 and 5.4.0.

I just want to make sure everyone is ok with that and aware of that before we merge (I'm ok with this).

Lastly, in all the final procedures the clear call should have the "()" after it.

Another observation, but can't you just define the final procedures as follows:

      PROCEDURE,PASS :: clear => clear_ParamType
      !> @copybrief ParameterLists::clear_ParamType
      !> @copydoc ParameterLists::clear_ParamType
      FINAL :: clear_ParamType

e.g. point to the same subroutine for clear and finalization? or does this cause a compiler error?

[rks171]

Forgetting to call clear on the ParameterLists objects has been a source of many memory leaks being flagged on CDash for me (with the most recent one just a few days ago), so it would be much better for me (and probably others) if we just had a FINAL procedure for this. If you want to keep support for 4.8.3, then certainly we cannot use this. I figured since we've upgraded to 5.4.0 as our default version a few months ago that we'd be okay making use of the FINAL procedure. I'm not sure if you can mark an existing procedure as FINAL. I'll give it a try and see if it works.

[rks171]

Okay just checked. No you cannot just make the existing clear procedures also FINAL. They need to have the passed object as polymorphic, i.e., CLASS(ParamType), INTENT(INOUT) :: this. The FINAL procedure requires the passed object is not, i.e., TYPE(ParamType), INTENT(INOUT) :: this.

[bkochuna]

Yeah, its better to not have one more thing you have to remember. Dan pointed out we could always just ifdef the GCC version around the FINAL definition. But I don't think that's necessary unless we want to consciously continue supporting 4.8.3

That sucks you can't use the same the procedure for both, but... makes sense I guess.

[rks171]

How would you like to determine if we will continue supporting 4.8.3? Open an issue for everyone to weigh in on?

[bkochuna]

That seems reasonable. This might be a good thing to discuss at the colocation this week in the SQA meetings. It likely just needs to be a decision that Shane and I agree on, and then we need to make sure the Configure Control List is up to date.

[stimpsonsg]

It seems fine to move on from 4.8.3. Does anyone know if we can still build with 4.6.3 or 4.7.2?

The only snag has been that Panacea has not been updated with 5.4.0, but that's really something that should happen anyway.

I'm still kind of in favor of ifdefing it since it might be useful down the line if we want some other compiler (intel, pgi) for some reason. This would still enable us to build in 4.8.3, but would certainly guarantee nothing about the memory performance and leaks.

[aarograh]

To me it seems pointless to add this feature if we ifdef it. We need to either always be able to rely on it to clear things, or not use it at all. If we ifdef it, we'll just immediately have a bunch of memory leaks as soon as someone builds with an old version of gfortran or some other compiler.

[bkochuna]

I'm in favor of ifdeffing for now. I didn't want to demand Bob do this extra work, if we weren't in agreement on the value of it. We can always remove later.

W.R.T 4.6.x we cannot build with this anymore, I'm almost certain of it.

The other consideration for 4.8.3 is that the latest versions of RHEL/CentOS (v7) have gcc 4.8.5 as the default compilers. All of the CASL dev boxes, UM dev boxes, and UM cluster use these linux OS's. It's easy enough to install 5.4.0, but this would ensure a backup...

Again, I think it would be helpful to discuss this week.

[stimpsonsg]

Yeah, the immediate leaks in different versions would be an issue, no doubt. I don't really foresee needing other compilers, but it's something to consider.

If 5.4.0 is the official CASL version, we should ensure any machine we're using has that.

[stimpsonsg]

Discussed this a little more with Aaron and Shane H.

I think we're in favor of not ifdefing it so that we can guarantee that users on other machines are not building with a non-official compiler version. If we really need a different compiler down the road that doesn't support FINAL, we can ifdef and fix leaks, but I'd put those odds at very low.

[bkochuna]

Ok, that's fine with me. That logic makes sense to me.