-Wundefined-func-template warns that no definition is available for a pure virtual function

[ahatanak]

The warning seems incorrect as B::bar is pure virtual and its definition isn't needed.

$ cat test.cpp

template <typename T> class B {
public:
  constexpr void foo(const T &) { bar(1); }
  virtual constexpr void bar(unsigned int) = 0;
};

template <typename T> class D : public B<T> {
public:
  constexpr void bar(unsigned int) override {}
};

void test() {
  auto t = D<int>();
  t.foo(0);
}

$ clang++ -std=c++20 test.cpp -Wundefined-func-template -c
test.cpp:4:35: warning: instantiation of function 'B<int>::bar' required here, but no definition is available [-Wundefined-func-template]
  constexpr void foo(const T &) { bar(1); }
                                  ^
test.cpp:5:26: note: forward declaration of template entity is here
  virtual constexpr void bar(unsigned int) = 0;
                         ^
test.cpp:4:35: note: add an explicit instantiation declaration to suppress this warning if 'B<int>::bar' is explicitly instantiated in another translation unit
  constexpr void foo(const T &) { bar(1); }
                                  ^
1 warning generated.

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: Akira Hatanaka (ahatanak)

The warning seems incorrect as `B::bar` is pure virtual and its definition isn't needed.

$ cat test.cpp

template <typename T> class B {
public:
  constexpr void foo(const T &) { bar(1); }
  virtual constexpr void bar(unsigned int) = 0;
};

template <typename T> class D : public B<T> {
public:
  constexpr void bar(unsigned int) override {}
};

void test() {
  auto t = D<int>();
  t.foo(0);
}

$ clang++ -std=c++20 test.cpp -Wundefined-func-template -c
test.cpp:4:35: warning: instantiation of function 'B<int>::bar' required here, but no definition is available [-Wundefined-func-template]
constexpr void foo(const T &) { bar(1); }
^
test.cpp:5:26: note: forward declaration of template entity is here
virtual constexpr void bar(unsigned int) = 0;
^
test.cpp:4:35: note: add an explicit instantiation declaration to suppress this warning if 'B<int>::bar' is explicitly instantiated in another translation unit
constexpr void foo(const T &) { bar(1); }
^
1 warning generated.

[shafik]

Confirmed: https://godbolt.org/z/6a3vvveEG

[llvmbot]

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

1. In the comments of the issue, request for it to be assigned to you.
2. Fix the issue locally.
3. Run the test suite locally. Remember that the subdirectories under test/ create fine-grained testing targets, so you can e.g. use make check-clang-ast to only run Clang's AST tests.
4. Create a Git commit.
5. Run git clang-format HEAD~1 to format your changes.
6. Open a pull request to the upstream repository on GitHub. Detailed instructions can be found in GitHub's documentation.

If you have any further questions about this issue, don't hesitate to ask via a comment in the thread below.

[llvmbot]

@llvm/issue-subscribers-good-first-issue

Author: Akira Hatanaka (ahatanak)

The warning seems incorrect as `B::bar` is pure virtual and its definition isn't needed.

$ cat test.cpp

template <typename T> class B {
public:
  constexpr void foo(const T &) { bar(1); }
  virtual constexpr void bar(unsigned int) = 0;
};

template <typename T> class D : public B<T> {
public:
  constexpr void bar(unsigned int) override {}
};

void test() {
  auto t = D<int>();
  t.foo(0);
}

$ clang++ -std=c++20 test.cpp -Wundefined-func-template -c
test.cpp:4:35: warning: instantiation of function 'B<int>::bar' required here, but no definition is available [-Wundefined-func-template]
constexpr void foo(const T &) { bar(1); }
^
test.cpp:5:26: note: forward declaration of template entity is here
virtual constexpr void bar(unsigned int) = 0;
^
test.cpp:4:35: note: add an explicit instantiation declaration to suppress this warning if 'B<int>::bar' is explicitly instantiated in another translation unit
constexpr void foo(const T &) { bar(1); }
^
1 warning generated.

[charmitro]

I'd like to pick this up. I'm sharing my thoughts so we can discuss them.

Where does this happen?

The warning is coming from

llvm-project/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

Lines 4954 to 4958 in bc51a36

	if (AtEndOfTU && !getDiagnostics().hasErrorOccurred() &&
	!getSourceManager().isInSystemHeader(PatternDecl->getBeginLoc())) {
	Diag(PointOfInstantiation, diag::warn_func_template_missing)
	<< Function;
	Diag(PatternDecl->getLocation(), diag::note_forward_template_decl);

We do not check if the function is a pure virtual function via FunctionDecl::isVirtualAsWritten().

Issue fix

IMO we should just add another condition in the if statement checking for this, such as:

      if (AtEndOfTU && !getDiagnostics().hasErrorOccurred() &&
          !getSourceManager().isInSystemHeader(PatternDecl->getBeginLoc()) &&
          !Function->isVirtualAsWritten()) { ... }

Finally we should add a test-case for this under clang/test/SemaTemplate/ called instantiate-virtual-function.cpp (I'm not good at naming).

if all these sound good, I'll start implementing it.

[dwblaikie]

if all these sound good, I'll start implementing it.

Sounds about right/plausible to me, thanks!

[ahatanaka]

We still want to emit the warning if B::bar(1) was called instead of bar(1) in B::foo, right?

The patch silences the warning.

[charmitro]

We still want to emit the warning if B::bar(1) was called instead of bar(1) in B::foo, right?

The patch silences the warning.

Sorry I didn't get this. Are we talking about the following?

  constexpr void foo(const T &) { B::bar(1); }

instead of:

  constexpr void foo(const T &) { bar(1); }

[ahatanak]

Yes, that's what I meant.

[charmitro]

Yes, that's what I meant.

This is what we get with the patch:

$ ./bin/clang++ main.cc -Wundefined-func-template 
main.cc:10:26: warning: inline function 'B<int>::bar' is not defined [-Wundefined-inline]
   10 |   virtual constexpr void bar(unsigned int) = 0;
      |                          ^
main.cc:9:38: note: used here
    9 |   constexpr void foo(const T &) { B::bar(1); }
      |                                      ^
1 warning generated.
/usr/bin/ld: /tmp/main-c89dda.o: in function `B<int>::foo(int const&)':
main.cc:(.text._ZN1BIiE3fooERKi[_ZN1BIiE3fooERKi]+0x1a): undefined reference to `B<int>::bar(unsigned int)'
clang++: error: linker command failed with exit code 1 (use -v to see invocation)

[ahatanak]

I meant the patch was silencing the -Wundefined-func-template warning.

Note that ToT clang emits the warning for the following code, but with the patch, it doesn't emit any warnings:

template <typename T> class B {
public:
  const void foo(const T &) { B::bar(1); }
  virtual const void bar(unsigned int) = 0;
};

template <typename T> class D : public B<T> {
public:
  const void bar(unsigned int) override {}
};

void test() {
  auto t = D<int>();
  t.foo(0);
}

[charmitro]

I meant the patch was silencing the -Wundefined-func-template warning.

Note that ToT clang emits the warning for the following code, but with the patch, it doesn't emit any warnings:

template <typename T> class B {
public:
  const void foo(const T &) { B::bar(1); }
  virtual const void bar(unsigned int) = 0;
};

template <typename T> class D : public B<T> {
public:
  const void bar(unsigned int) override {}
};

void test() {
  auto t = D<int>();
  t.foo(0);
}

Thanks for pinging me about this @ahatanak.

I revisited the patch based on your recommendation (thanks) and added a test case for it.
Please check!