search

CASL / Futility

VERA Fortran Utilities
Other
44 stars 20 forks source link

Fixed compilation issue with FutilityComputingEnvironment for gcc 4.8.5 #156

Closed apfitzge closed 4 years ago

apfitzge commented 5 years ago

In routines adding or removing timers, changed TYPE(TimerType) to CLASS(TimerType) in order to match the TimerPtrArray specification.

When compiling with gcc 4.8.5 the following error was found, along with similar errors in other routines.

/home/apfitzge/MPACT/Futility/src/FutilityComputingEnvironment.f90:143.2:

  timer => this%getTimer(name)
  1
Error: Different types in pointer assignment at (1); attempted assignment of CLASS(timertype) to TYPE(timertype)

this change allows compilation to succeed and all unit-tests pass.

stimpsonsg commented 5 years ago

Are any downstream changes necessary in MPACT?

bkochuna commented 5 years ago

A little scary 4.8.5 catches this bug and 5.4.0 does not.

apfitzge commented 5 years ago

I think what is happening in 5.4.0 is that it realizes that there is only a single type: TimerType If there were derived types, it'd be important to distinguish between class and pointer, but there aren't. 5.4.0 realizes there is only a single type, 4.8.5 does not. I think using class pointers is generally better here, if we were to add derived types everything would suddenly break.

No downstream changes are necessary for this pull request to work, but MPACT won't compile with 4.8.5 anymore:

There are a few similar bugs like this with the XSMesh type (class vs type pointers) that are simple fixes. I didn't merge this in because there was an internal compiler error in EditsBase with 4.8.5. I didn't find a solution for that though, it's unclear to me whether or not we should even address this since we've moved on to using 5.4.0 as our standard.

Should I create an issue on the gitlab repo for that internal compiler error?

stimpsonsg commented 5 years ago

I think we should merge this (and the XSMesh issues) for the reason that you mentioned.

We could address the ICE in EditsBase, but my understanding is that we're moving on from 4.8.3/4.8.5, without intent to support it (but I could be wrong). As such, it's probably not worth the time.

What builds or where are you trying to build that still needs 4.8.X?

apfitzge commented 5 years ago

Alright, I can make a merge request on the MPACT side of things tomorrow.

I don't need 4.8.X anymore, I was working with an old build on highpoint, but I've switched to 5.4.0 now. I must have missed the standup when we officially switched off of 4.8.

apfitzge commented 5 years ago

I've submitted a merge request on GitLab for 4.8.5 compilation issues in MPACT.

aarograh commented 4 years ago

Just updated this with master. I ran all the unit tests in a debug MPACT build and encountered no issues (5.4.0 on ROSS). No conflicts due to the small amount of changes. Even though this issue isn't really causing problems with 5.4, it's more correct code and should be merged.

@stimpsonsg can you merge if you're good with this?

stimpsonsg commented 4 years ago

yeah, I will after the CI finishes