This is an archive of the discontinued LLVM Phabricator instance.

[flang] Avoid infinite recursion in common block check
ClosedPublic

Authored by luporl on Feb 2 2023, 12:43 PM.

Details

Summary

Don't call CheckCommonBlockDerivedType() recursively if the
derived type symbol is the same symbol that is already being
processed. This can happen when a component is a pointer of the
same type as its parent component, for instance.

Fixes #60230

Diff Detail

Event Timeline

luporl created this revision.Feb 2 2023, 12:43 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 2 2023, 12:43 PM
luporl requested review of this revision.Feb 2 2023, 12:43 PM
klausler accepted this revision.Feb 2 2023, 12:58 PM

Looks great to me; thanks for digging into this bug and coming up with a good fix. If you're feeling really ambitious, consider moving this whole common block checking task out of name resolution and into declaration checking (Semantics/check-declarations.cpp) which would be a more natural home for this code.

flang/lib/Semantics/resolve-names.cpp
5771–5778

Please always use braced initializers in semantics.

5772

s/method/member function/

This revision is now accepted and ready to land.Feb 2 2023, 12:58 PM
luporl added a comment.Feb 3 2023, 9:26 AM

Thanks for the quick review.

It seems that moving the common block checking task out of name resolution would require quite some work.
It's also unclear to me how to move this code out of DeclarationVisitor, that also has some functions related to name resolving.
And CheckCommonBlocks() is called by ResolveNamesVisitor::FinishSpecificationPart().
Maybe as a helper class, that would have to be exported by Semantics/check-declarations.h?

Anyway, I guess it's better to leave that for a future patch.

luporl updated this revision to Diff 494670.Feb 3 2023, 9:31 AM
luporl edited the summary of this revision. (Show Details)

Address reviewer's comments.

luporl marked 2 inline comments as done.Feb 3 2023, 9:31 AM
This revision was automatically updated to reflect the committed changes.