Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generic name resolution in expression analysis #778

Merged
merged 1 commit into from
Oct 10, 2019
Merged

Conversation

tskeith
Copy link
Collaborator

@tskeith tskeith commented Oct 9, 2019

Implement the basics of resolving generic names in expressions.

ExpressionAnalyzer::ResolveGeneric maps the symbol for a generic
name to the specific procedure appropriate for the actual arguments.
Extract CheckExplicitInterface out of CheckArguments so that it
can be tried for each specific procedure of the generic as part of
the test to see which is compatible.

Note that it may be there is an elemental and non-elemental specific
procedure that is compatible with the actual arguments. In that case
the generic is resolved to the non-elemental one.

Test this by using generic functions in specification expressions
that must be written to module files. Verify how the generics were
resolved by looking at the generated .mod files.

There is more work to be done in this area: the passed-object dummy
argument is not considered and in some cases generated module files
are not correct.

Copy link
Collaborator

@klausler klausler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks good. I've been working on these files myself and am in the process of moving them from one directory to another, but it shouldn't be too hard to merge our work together.

lib/evaluate/check-call.cc Outdated Show resolved Hide resolved
lib/semantics/expression.cc Outdated Show resolved Hide resolved
!symbol->owner().sourceRange().Contains(callSite)));
return symbol->has<semantics::SubprogramDetails>() &&
symbol->owner().IsGlobal() &&
!symbol->scope()->sourceRange().Contains(callSite);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not positive this condition is correct. This change fixes some incorrect warnings I was getting when a generic resolved to a specific in another module. symbol->owner() was the module and it did not contain the call site.

Copy link
Collaborator

@psteinfeld psteinfeld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

Copy link
Collaborator

@jeanPerier jeanPerier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fail to grasp the differences between some of the checks, but it will probably make more sens when the TODOs in these functions are done. Otherwise looks good to me.

for (std::size_t i{0}; i < dummies.size(); ++i) {
const characteristics::DummyArgument &dummy{dummies[i]};
const std::optional<ActualArgument> &actual{actuals[i]};
if (!actual || !CheckCompatibleArgument(isElemental, *actual, dummy)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this condition allow the actual argument to be absent if the dummy is optional ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you're right, the check for actual was backwards. I'll fix this and add a test for this case.

FoldingContext localContext{context_.foldingContext(), messages};
ActualArguments localActuals{actuals};
if (CheckExplicitInterface(*procedure, localActuals, localContext) &&
CheckCompatibleArguments(*procedure, localActuals)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not clear to me why CheckExplicitInterface does not imply CheckCompatibleArguments. CheckExplicitInterfaceArg and CheckCompatibleArgument look very similar. Obviously, if you are doing it, there is a reason. I see that CheckCompatibleArguments seems to deal with elemental aspect when CheckExplicitInterface does not seem to do so.
Could you explain a bit more what is the difference between these two checks ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The goal here is just to narrow down the specific procedures to the right one, thought it still might not perfectly match the actual arguments. This is just an initial implementation -- it may take more work to get the best error messages from bad calls to generics procedures.

Implement the basics of resolving generic names in expressions.

`ExpressionAnalyzer::ResolveGeneric` maps the symbol for a generic
name to the specific procedure appropriate for the actual arguments.
Extract `CheckExplicitInterface` out of `CheckArguments` so that it
can be tried for each specific procedure of the generic as part of
the test to see which is compatible.

Note that it may be there is an elemental and non-elemental specific
procedure that is compatible with the actual arguments. In that case
the generic is resolved to the non-elemental one.

Test this by using generic functions in specification expressions
that must be written to module files. Verify how the generics were
resolved by looking at the generated `.mod` files.

There is more work to be done in this area: the passed-object dummy
argument is not considered and in some cases generated module files
are not correct.
@tskeith tskeith merged commit 1688bef into master Oct 10, 2019
@tskeith tskeith deleted the tsk-generics branch October 10, 2019 18:47
swift-ci pushed a commit to swiftlang/llvm-project that referenced this pull request Apr 9, 2020
Implement the basics of resolving generic names in expressions.

`ExpressionAnalyzer::ResolveGeneric` maps the symbol for a generic
name to the specific procedure appropriate for the actual arguments.
Extract `CheckExplicitInterface` out of `CheckArguments` so that it
can be tried for each specific procedure of the generic as part of
the test to see which is compatible.

Note that it may be there is an elemental and non-elemental specific
procedure that is compatible with the actual arguments. In that case
the generic is resolved to the non-elemental one.

Test this by using generic functions in specification expressions
that must be written to module files. Verify how the generics were
resolved by looking at the generated `.mod` files.

There is more work to be done in this area: the passed-object dummy
argument is not considered and in some cases generated module files
are not correct.

Original-commit: flang-compiler/f18@50e4580
Reviewed-on: flang-compiler/f18#778
swift-ci pushed a commit to swiftlang/llvm-project that referenced this pull request Apr 9, 2020
…/tsk-generics

Generic name resolution in expression analysis

Original-commit: flang-compiler/f18@1688bef
Reviewed-on: flang-compiler/f18#778
mem-frob pushed a commit to draperlaboratory/hope-llvm-project that referenced this pull request Oct 7, 2022
Implement the basics of resolving generic names in expressions.

`ExpressionAnalyzer::ResolveGeneric` maps the symbol for a generic
name to the specific procedure appropriate for the actual arguments.
Extract `CheckExplicitInterface` out of `CheckArguments` so that it
can be tried for each specific procedure of the generic as part of
the test to see which is compatible.

Note that it may be there is an elemental and non-elemental specific
procedure that is compatible with the actual arguments. In that case
the generic is resolved to the non-elemental one.

Test this by using generic functions in specification expressions
that must be written to module files. Verify how the generics were
resolved by looking at the generated `.mod` files.

There is more work to be done in this area: the passed-object dummy
argument is not considered and in some cases generated module files
are not correct.

Original-commit: flang-compiler/f18@50e4580
Reviewed-on: flang-compiler/f18#778
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants