Skip to content

Fix SFINAE calling a function template with deduced return type and pending definition - #322

Open
cpplearner wants to merge 3 commits into
edgcpp:mainfrom
cpplearner:auto-pending-inst
Open

cpplearner wants to merge 3 commits into
edgcpp:mainfrom
cpplearner:auto-pending-inst

Conversation

@cpplearner

Copy link
Copy Markdown
Contributor

With modules, it is possible that a function template definition exists in a module but the definition is not yet loaded. In such a case, routine->defined is false, despite the existence of a definition. Previously, substitution failed if such a function template is called in a SFINAE context and the function template has a deduced return type. This is fixed in this PR.

See #242 for an example.

Fixes #242

@cpplearner

Copy link
Copy Markdown
Contributor Author
  • Call has_pending_template_definition_from_module only if routine refers to a template instance.
  • Drive by: simplify the neighboring friend_defined_in_instantiation check. The routine pointer was assigned routine_sym->variant.routine.ptr.

@daveedvdv-nvidia daveedvdv-nvidia left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for taking this on!

I've created issue #453 from the discussion.

The fix looks fine to me (I made a note of one stylistic nit). To be complete we need a few more things:

  1. A src/pending_changes/... entry explaining the improvement (referring to [GH #453])
  2. One or more regressions tests
  3. Rebase and collapse the commits into one, with a single-line commit message ending with [GH #453]

Comment thread src/exprutil.c
tsym = prototype_template_if_template_symbol(tip->template_sym);
if (tsym->defined) {
could_be_instantiated = TRUE;
} else if (has_pending_template_definition_from_module(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd slightly prefer a single if (tsym->defined || has_pending... ) ... statement here.

This branch has not been deployed

No deployments
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.

2 participants