Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Phabricator shutdown timeline

[Clang] Check for abstract parameters only when functions are defined.

Authored by cor3ntin on Jun 4 2023, 8:16 AM.



The C++ standard allows abstract parameters in deleted functions
and in function declarations

The type of a parameter or the return type for a function definition
shall not be a (possibly cv-qualified) class type that is
incomplete or abstract within the function body
unless the function is deleted.


Diff Detail

Event Timeline

cor3ntin created this revision.Jun 4 2023, 8:16 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 4 2023, 8:16 AM
cor3ntin requested review of this revision.Jun 4 2023, 8:16 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 4 2023, 8:16 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
cor3ntin added a reviewer: Restricted Project.Jun 4 2023, 8:16 AM
aaron.ballman added inline comments.

This doesn't seem quite right -- we skip bodies for various reasons:

Which test case needed this change?


Whole lotta trailing semi colons that should be removed (you should get all of them, not just theses).


FIXME can be removed now? Do we need a DR test case for this?


Is this fixme still accurate? Should we be changing the resolution from partial to full?


A mildly horrifying test case to consider:

struct S {
  virtual void func() = 0;
void S::func() {}


struct T {
  void func(S) = delete;
  void other(S);
  void yet_another(S) {}

We currently don't do good things...

cor3ntin added inline comments.Jun 6 2023, 12:42 PM

nah, cf below (L714)

cor3ntin updated this revision to Diff 529045.Jun 6 2023, 2:23 PM
cor3ntin marked 2 inline comments as done.

Address Aaron's comments.

Initially these changes were tracked in

I fixed the specific github issue but more work is needed to acertain conformance.

cor3ntin marked 3 inline comments as done.Jun 7 2023, 4:26 AM
aaron.ballman accepted this revision.Jun 7 2023, 9:35 AM
aaron.ballman added a subscriber: rjmccall.

LGTM, though please wait a day or so to land this in case @rjmccall disagrees with my assessment of the Objective-C changes.


I think this change makes sense for Objective-C as well (this is a declaration and not a definition, at least as I understand things), but pinging @rjmccall just in case I'm wrong.

This revision is now accepted and ready to land.Jun 7 2023, 9:35 AM
rjmccall added inline comments.Jun 7 2023, 12:18 PM

No, that's right, methods in @interfaces and @protocols are just declarations.

Consistency with the base standard seems like the right way to go here, even those I personally find this rule rather silly. (It's one thing to allow this for deleted methods, but allowing it for declarations?)