Skip to content

[clang] Accept recursive non-dependent calls to functions with deduced return type #75456

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

Merged
merged 4 commits into from
Jan 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -692,6 +692,9 @@ Bug Fixes in This Version
Fixes (`#64347 <https://github.com/llvm/llvm-project/issues/64347>`_)
- Fix crash when using C++ only tokens like ``::`` in C compiler clang.
Fixes (`#73559 <https://github.com/llvm/llvm-project/issues/73559>`_)
- Clang now accepts recursive non-dependent calls to functions with deduced
return type.
Fixes (`#71015 <https://github.com/llvm/llvm-project/issues/71015>`_)


Bug Fixes to Compiler Builtins
Expand Down
2 changes: 2 additions & 0 deletions clang/lib/AST/ComputeDependence.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -603,6 +603,8 @@ ExprDependence clang::computeDependence(PredefinedExpr *E) {
ExprDependence clang::computeDependence(CallExpr *E,
llvm::ArrayRef<Expr *> PreArgs) {
auto D = E->getCallee()->getDependence();
if (E->getType()->isDependentType())
D |= ExprDependence::Type;
for (auto *A : llvm::ArrayRef(E->getArgs(), E->getNumArgs())) {
if (A)
D |= A->getDependence();
Expand Down
18 changes: 18 additions & 0 deletions clang/lib/Sema/SemaOverload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13994,6 +13994,24 @@ ExprResult Sema::BuildOverloadedCallExpr(Scope *S, Expr *Fn,
OverloadCandidateSet::iterator Best;
OverloadingResult OverloadResult =
CandidateSet.BestViableFunction(*this, Fn->getBeginLoc(), Best);
FunctionDecl *FDecl = Best->Function;

// Model the case with a call to a templated function whose definition
// encloses the call and whose return type contains a placeholder type as if
// the UnresolvedLookupExpr was type-dependent.
if (OverloadResult == OR_Success && FDecl &&
FDecl->isTemplateInstantiation() &&
FDecl->getReturnType()->isUndeducedType()) {
if (auto TP = FDecl->getTemplateInstantiationPattern(false)) {
if (TP->willHaveBody()) {
CallExpr *CE =
CallExpr::Create(Context, Fn, Args, Context.DependentTy, VK_PRValue,
RParenLoc, CurFPFeatureOverrides());
result = CE;
return result;
}
}
}

return FinishOverloadedCallExpr(*this, S, Fn, ULE, LParenLoc, Args, RParenLoc,
ExecConfig, &CandidateSet, &Best,
Expand Down
46 changes: 46 additions & 0 deletions clang/test/SemaCXX/deduced-return-type-cxx14.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -640,3 +640,49 @@ namespace PR46637 {
template<typename T> struct Y { T x; };
Y<auto() -> auto> y; // expected-error {{'auto' not allowed in template argument}}
}

namespace GH71015 {

// Check that there is no error in case a templated function is recursive and
// has a placeholder return type.
struct Node {
int value;
Node* left;
Node* right;
};

bool parse(const char*);
Node* parsePrimaryExpr();

auto parseMulExpr(auto node) { // cxx14-error {{'auto' not allowed in function prototype}} \
// cxx14-note {{not viable}}
if (node == nullptr) node = parsePrimaryExpr();
if (!parse("*")) return node;
return parseMulExpr(new Node{.left = node, .right = parsePrimaryExpr()});
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be great to add a test that this is rejected without the previous line retuen node;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added the test.

A note here is that It will only be rejected if the function instantiated because clang doesn't take into account return statements if the function is templated due to this part:

// C++1y [dcl.spec.auto]p12:

gcc rejects even if the function is not instantiated. msvc doesn't. https://compiler-explorer.com/z/hGc8M1esT

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that's fine, and it's what I expected. While we could try to reject in the template definition if we see a recursive call prior to the end of the first return statement, I don't think it's worth the added complexity.

I think what GCC's doing here is that it's treating the recursive call as dependent in much the same way that Clang does with this patch, but then performing an instantiation of the function template specialization parseMulExpr<Node*> after it finishes processing the template definition -- so it's also not eagerly diagnosing the problem. (Clang suppresses performing recursive non-dependent function template instantiations until the template has a point of instantiation outside itself, which GCC doesn't seem to do.) But I could be wrong.

}

template <typename T>
auto parseMulExpr2(T node) {
if (node == nullptr) node = parsePrimaryExpr();
if (!parse("*")) return node;
return parseMulExpr2(new Node{.left = node, .right = parsePrimaryExpr()});
}

template <typename T>
auto parseMulExpr3(T node) { // expected-note {{declared here}}
if (node == nullptr) node = parsePrimaryExpr();
return parseMulExpr3(new Node{.left = node, .right = parsePrimaryExpr()}); // expected-error {{cannot be used before it is defined}}
}

void foo() {
parseMulExpr(new Node{}); // cxx14-error {{no matching function}}
parseMulExpr2(new Node{});
parseMulExpr3(new Node{}); // expected-note {{in instantiation}}
}

auto f(auto x) { // cxx14-error {{'auto' not allowed in function prototype}}
if (x == 0) return 0;
return f(1) + 1;
}

}