Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 326269 - Checker for instantiation of an abstract class
Summary: Checker for instantiation of an abstract class
Status: RESOLVED FIXED
Alias: None
Product: CDT
Classification: Tools
Component: cdt-codan (show other bugs)
Version: 8.0   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: 8.0   Edit
Assignee: Sergey Prigogin CLA
QA Contact: Elena Laskavaia CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-27 03:14 EDT by Vasiliy Dybala CLA
Modified: 2012-05-22 14:52 EDT (History)
5 users (show)

See Also:


Attachments
Checker that warns about abstract creation (26.27 KB, patch)
2011-03-21 05:08 EDT, Anton G. CLA
eclipse.sprigogin: iplog-
Details | Diff
Checker that warns about abstract creation (comment fixture) (26.30 KB, patch)
2011-03-29 04:19 EDT, Anton G. CLA
eclipse.sprigogin: iplog+
Details | Diff
A few fixes to the commited checker (3.94 KB, patch)
2011-05-02 04:33 EDT, Anton G. CLA
eclipse.sprigogin: iplog+
Details | Diff
Bug fixture for virtual destructors override (3.35 KB, patch)
2011-05-10 03:31 EDT, Anton G. CLA
eclipse.sprigogin: iplog+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Vasiliy Dybala CLA 2010-09-27 03:14:43 EDT
Build Identifier: 201009170810

class Cl
{
  Cl(){}
  virtual void pure() = 0;
};

void test()
{
  new Cl;
}

I think it would be good if codan can check that class has private constructor or private assignment operator or pure virtual functions and shows an error when user tries to create instance of such class.


Reproducible: Always

Steps to Reproduce:
1. launch eclipse
2. white example as described.
3. eclipse doesn't show any errors/warnings on this code.
Comment 1 Elena Laskavaia CLA 2010-09-29 21:45:21 EDT
Can you provide a reference why this is a problem? Is this detected by C++ compiler?
Comment 2 Marc-André Laperle CLA 2010-09-29 22:18:29 EDT
Codan will probably have a member access checker at some point so I'm not sure where to draw the line. The private constructor and private assignment operator cases could be handled by the member access checker. The pure virtual method case could be its own checker.

(In reply to comment #1)
> Can you provide a reference why this is a problem? Is this detected by C++
> compiler?

It won't compile. You can't call a private constructor from outside the class and you can instanciate an abstract class.
Comment 3 Marc-André Laperle CLA 2010-09-29 22:23:37 EDT
(In reply to comment #2)
> and you can instanciate an abstract class.

can't instanciate...
Comment 4 Sergey Prigogin CLA 2010-09-29 23:53:45 EDT
(In reply to comment #0)
Access rules in C++ are arcane. The class can be instantiated in the following scenario:

class Cl
{
  Cl(){}
  virtual void pure() = 0;
  C1* instantiator() {
    class B : public C {
      void pure() {}
    };
    return new B();
  }
};
Comment 5 Anton G. CLA 2011-03-21 05:08:00 EDT
Created attachment 191598 [details]
Checker that warns about abstract creation

I have implemented Codan checker that checks attempts to create an object of abstract class. Can somebody review and comment or commit it?

P.S. Maybe another bug with more clear title should be opened for the member access checker?
Comment 6 Sergey Prigogin CLA 2011-03-25 13:42:29 EDT
The patch looks good overall. It would be nice to eliminate the template-related limitation in ClassTypeHelper.getPureVirtualFunctions.
Comment 7 Anton G. CLA 2011-03-27 14:38:34 EDT
(In reply to comment #6)
> The patch looks good overall. It would be nice to eliminate the
> template-related limitation in ClassTypeHelper.getPureVirtualFunctions.

It is not possible for some cases. For example:

template<typename _T>
struct A : public _T
{
	void int f(_T) {}
};

Is this template class abstract or not? It depends on for which type it will be instantiated.

However, for template class instantiations it works fine. For example:
template<typename _T>
struct A
{
	void int f(_T) = 0;
};

template<typename _T>
struct B : public A<_T>
{
	void int f(float) {}
};

template<typename _T>
struct C : public _T
{
	void int f(float) {}
};

B<int> b;		// Abstract
B<float> b2;		// Ok
C< A<int> > c;		// Abstract
C< A<float> > c2;	// Ok


Certainly, it is not my merit, but CDT engine.

P.S. If I did not take into account some cases - please add them here.
Comment 8 Sergey Prigogin CLA 2011-03-27 14:58:38 EDT
(In reply to comment #7)
I guess I misunderstood the comment the ClassTypeHelper.getPureVirtualFunctions method:
* NOTE: Template parameters used as base classes are skipped without checks
* that may cause to incorrect result for some template class instantiations.

Can you give an example of a class instantiation for which the result is incorrect?
Comment 9 Anton G. CLA 2011-03-28 02:21:45 EDT
(In reply to comment #8)
> (In reply to comment #7)
> I guess I misunderstood the comment the ClassTypeHelper.getPureVirtualFunctions
> method:
> * NOTE: Template parameters used as base classes are skipped without checks
> * that may cause to incorrect result for some template class instantiations.
> 
> Can you give an example of a class instantiation for which the result is
> incorrect?

You are right, the comment is not quite accurate. Honestly, when I wrote it I was not sure that the method will work for class instantiations. But yesterday I tested it more carefully and made sure the it works properly.

The comment should be like this:
* NOTE: For some template classes with template parameters as base classes or 
* in function signatures it is impossible accurately determine the presence of
* pure virtual functions until their instantiation.
Should I fix it and submit a new patch?
Comment 10 Sergey Prigogin CLA 2011-03-28 16:16:36 EDT
(In reply to comment #9)
> The comment should be like this:
> * NOTE: For some template classes with template parameters as base classes or 
> * in function signatures it is impossible accurately determine the presence of
> * pure virtual functions until their instantiation.
> Should I fix it and submit a new patch?

The comment should say that the method produces complete results for template instantiations but doesn't take take into account base classes dependent on template parameters.

It makes sense to update the patch since it has to wait for an IP review anyway.
Comment 11 Anton G. CLA 2011-03-29 04:19:44 EDT
Created attachment 192066 [details]
Checker that warns about abstract creation (comment fixture)

Comment was fixed.
Comment 12 Anton G. CLA 2011-03-31 13:17:13 EDT
a.  I authored 100% of the content I am contributing
b.  I have the rights to donate the content to Eclipse
c.  I contribute the content under the EPL
Comment 13 Sergey Prigogin CLA 2011-04-12 19:19:28 EDT
Patch submitted.
Comment 14 CDT Genie CLA 2011-04-12 19:23:09 EDT
*** cdt cvs genie on behalf of sprigogin ***
Bug 326269 - Checker for instantiation of an abstract class. Patch by Anton Gerenkov.

[+] AbstractClassInstantiationCheckerTest.java  http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/AbstractClassInstantiationCheckerTest.java?root=Tools_Project&revision=1.1&view=markup

[*] AutomatedIntegrationSuite.java 1.19 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/test/AutomatedIntegrationSuite.java?root=Tools_Project&r1=1.18&r2=1.19

[*] plugin.xml 1.35 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/codan/org.eclipse.cdt.codan.checkers/plugin.xml?root=Tools_Project&r1=1.34&r2=1.35

[+] AbstractClassInstantiationChecker.java  http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/AbstractClassInstantiationChecker.java?root=Tools_Project&revision=1.1&view=markup

[*] bundle.properties 1.13 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/codan/org.eclipse.cdt.codan.checkers/OSGI-INF/l10n/bundle.properties?root=Tools_Project&r1=1.12&r2=1.13

[*] ClassTypeHelper.java 1.19 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.core/parser/org/eclipse/cdt/internal/core/dom/parser/cpp/ClassTypeHelper.java?root=Tools_Project&r1=1.18&r2=1.19
Comment 15 Anton G. CLA 2011-05-02 04:33:08 EDT
Created attachment 194463 [details]
A few fixes to the commited checker

Writing other checkers I have noticed a few minor things in this one:
  - resolveBinding() should be used instead of getOrResolveBinding() (the same as https://bugs.eclipse.org/bugs/show_bug.cgi?id=339795#c16);
  - this checker should have another marker type and run as you type only (the same as https://bugs.eclipse.org/bugs/show_bug.cgi?id=343274#c5).

The changes are small, IP review is not necessary. Can somebody commit them or need I to create a new bug and attach my patch there?
Comment 16 Sergey Prigogin CLA 2011-05-02 14:40:54 EDT
(In reply to comment #15)
Patch submitted. Thanks.
Comment 18 Sergey Prigogin CLA 2011-05-09 15:15:13 EDT
The following use case produces a bogus error:

class A {
 public:
  virtual ~A() = 0;
};

class B : public A {
 public:
  virtual ~B() {}
};

B b;

The error message says: "The type 'B' must implement the inherited pure virtual method 'A::~A'"
Comment 19 Anton G. CLA 2011-05-10 03:31:55 EDT
Created attachment 195177 [details]
Bug fixture for virtual destructors override

Thank you for the case. Bug was fixed, a test case was added.
Comment 20 Sergey Prigogin CLA 2011-05-10 15:46:52 EDT
(In reply to comment #19)

Thank you, Anton. The fix is applied to HEAD > 20110510.