Skip to content

friend function with default parameters causes "redefinition of default argument" error in templates #130917

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

Closed
wingerZYM opened this issue Mar 12, 2025 · 9 comments
Assignees
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" regression:20 Regression in 20 release

Comments

@wingerZYM
Copy link

Summary:

Starting from commit 38b3d87bd384a469a6618ec6a971352cb4f813ba in LLVM, Clang 20 reports
error: redefinition of default argument when using friend functions
in template classes, even though the default argument is only defined once.

Steps to Reproduce:

Compile the following minimal example with Clang 20:

#include <iostream>
#include <memory>

namespace Work {

namespace detail {
    class Obj;
} // namespace detail

using Ptr = std::unique_ptr<detail::Obj>;

template <int>
Ptr Create(const void* key = nullptr);  // default argument only once.

namespace detail {

class Obj {
protected:
    Obj(const void* key) {
        std::cout << "Obj::Obj\n";
    }
};

template <int>
class ObjImpl : public Obj {
public:
    ObjImpl(const void* key)
        : Obj(key) {
        std::cout << "ObjImpl::ObjImpl\n";
    }
    template <int>
    friend Ptr Work::Create(const void*);
};

} // namespace detail

template <int i>
Ptr Create(const void* key) {  // error: redefinition of default argument
    return Ptr(new detail::ObjImpl<i>(key));
}

} // namespace Work

int main() {
    auto ptr = Work::Create<192>(nullptr);
    return 0;
}

Result:

clang++ reports:

./build/bin/clang++ -std=c++20 /mnt/e/test/error.cpp
/mnt/e/test/error.cpp:38:24: error: redefinition of default argument
   13 | Ptr Create(const void* key) {
      |                        ^
/mnt/e/test/error.cpp:39:20: note: in instantiation of template class 'Work::detail::ObjImpl<192>' requested here
   39 |     return Ptr(new detail::ObjImpl<i>(key));
      |                    ^
/mnt/e/test/error.cpp:45:22: note: in instantiation of function template specialization 'Work::Create<192>' requested here
   45 |     auto ptr = Work::Create<192>(nullptr);
      |                      ^
/mnt/e/test/error.cpp:32:40: note: previous definition is here
   32 |     friend Ptr Work::Create(const void*);
      |                                        ^
1 error generated.

Workaround:

If we remove the default parameter from Ptr Create(const void* key = nullptr);, then Clang++ compiles correctly.
If we remove the friend declaration, Clang++ also compiles correctly.

Regression:

git branch: release/20.x
First bad commit: 38b3d87
This issue does not occur in earlier versions of branch.

@frederick-vs-ja
Copy link
Contributor

Slightly reduced example (Godbolt link):

template <int>
void Create(const void* = nullptr);

template <int>
struct ObjImpl {
  template <int>
  friend void ::Create(const void*);
};

template <int I>
void Create(const void*) {
  (void) ObjImpl<I>{};
}

int main() {
  Create<42>();
}

This is probably a regression since Clang 20.

@frederick-vs-ja frederick-vs-ja added clang:frontend Language frontend issues, e.g. anything involving "Sema" regression:20 Regression in 20 release and removed new issue labels Mar 12, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 12, 2025

@llvm/issue-subscribers-clang-frontend

Author: None (wingerZYM)

### Summary: Starting from commit `38b3d87bd384a469a6618ec6a971352cb4f813ba` in LLVM, Clang 20 reports `error: redefinition of default argument` when using `friend` functions in template classes, even though the default argument is only defined once.

Steps to Reproduce:

Compile the following minimal example with Clang 20:

#include &lt;iostream&gt;
#include &lt;memory&gt;

namespace Work {

namespace detail {
    class Obj;
} // namespace detail

using Ptr = std::unique_ptr&lt;detail::Obj&gt;;

template &lt;int&gt;
Ptr Create(const void* key = nullptr);  // default argument only once.

namespace detail {

class Obj {
protected:
    Obj(const void* key) {
        std::cout &lt;&lt; "Obj::Obj\n";
    }
};

template &lt;int&gt;
class ObjImpl : public Obj {
public:
    ObjImpl(const void* key)
        : Obj(key) {
        std::cout &lt;&lt; "ObjImpl::ObjImpl\n";
    }
    template &lt;int&gt;
    friend Ptr Work::Create(const void*);
};

} // namespace detail

template &lt;int i&gt;
Ptr Create(const void* key) {  // error: redefinition of default argument
    return Ptr(new detail::ObjImpl&lt;i&gt;(key));
}

} // namespace Work

int main() {
    auto ptr = Work::Create&lt;192&gt;(nullptr);
    return 0;
}

Result:

clang++ reports:

./build/bin/clang++ -std=c++20 /mnt/e/test/error.cpp
/mnt/e/test/error.cpp:38:24: error: redefinition of default argument
   13 | Ptr Create(const void* key) {
      |                        ^
/mnt/e/test/error.cpp:39:20: note: in instantiation of template class 'Work::detail::ObjImpl&lt;192&gt;' requested here
   39 |     return Ptr(new detail::ObjImpl&lt;i&gt;(key));
      |                    ^
/mnt/e/test/error.cpp:45:22: note: in instantiation of function template specialization 'Work::Create&lt;192&gt;' requested here
   45 |     auto ptr = Work::Create&lt;192&gt;(nullptr);
      |                      ^
/mnt/e/test/error.cpp:32:40: note: previous definition is here
   32 |     friend Ptr Work::Create(const void*);
      |                                        ^
1 error generated.

Workaround:

If we remove the default parameter from Ptr Create(const void* key = nullptr);, then Clang++ compiles correctly.
If we remove the friend declaration, Clang++ also compiles correctly.

Regression:

git branch: release/20.x
First bad commit: 38b3d87
This issue does not occur in earlier versions of branch.

@zyn0217
Copy link
Contributor

zyn0217 commented Mar 12, 2025

@mizvekov I suspect #125266 might be related, but I'm not quite sure

@mizvekov
Copy link
Contributor

If you look at commit which the author pointed out, it's pretty suspect of this problem: #111992

It has changes to template instantiation friend definition tracking, outside of the serialization / modules area, which look suspect and would need careful review.

CC @dmpolukhin @ChuanqiXu9 as authors and reviewer of the original commit.

@zyn0217
Copy link
Contributor

zyn0217 commented Mar 12, 2025

If you look at commit which the author pointed out, it's pretty suspect of this problem: #111992

Oops, sorry I didn't even notice that line!

@dmpolukhin
Copy link
Contributor

I'll take a look what is happening with this example and how to fix it. Unfortunately I don't know the way how to distinguish real redeclarations from artificial redeclaration that happens on AST merges.

dmpolukhin added a commit that referenced this issue Apr 3, 2025
…y a declaration during AST deserialization (#132214)

Fix for regression #130917, changes in #111992 were too broad. This change reduces scope of previous fix. Added `ExternalASTSource::wasThisDeclarationADefinition` to detect cases when FunctionDecl lost body due to declaration merges.
@dmpolukhin dmpolukhin reopened this Apr 3, 2025
@dmpolukhin
Copy link
Contributor

Should I merges this fix to clang-20?

@zyn0217
Copy link
Contributor

zyn0217 commented Apr 3, 2025

Should I merges this fix to clang-20?

I think so, yes

dmpolukhin added a commit to dmpolukhin/llvm-project that referenced this issue Apr 3, 2025
…y a declaration during AST deserialization (llvm#132214)

Fix for regression llvm#130917, changes in llvm#111992 were too broad. This change reduces scope of previous fix. Added `ExternalASTSource::wasThisDeclarationADefinition` to detect cases when FunctionDecl lost body due to declaration merges.
@dmpolukhin dmpolukhin self-assigned this Apr 3, 2025
@dmpolukhin
Copy link
Contributor

Fix cannot be cherry-picked to clang-20, see discussion in #134232 so closing this bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" regression:20 Regression in 20 release
Projects
None yet
Development

No branches or pull requests

6 participants