| Summary: | Checker for instantiation of an abstract class | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Tools] CDT | Reporter: | Vasiliy Dybala <dibalavs> | ||||||||||
| Component: | cdt-codan | Assignee: | Sergey Prigogin <eclipse.sprigogin> | ||||||||||
| Status: | RESOLVED FIXED | QA Contact: | Elena Laskavaia <elaskavaia.cdt> | ||||||||||
| Severity: | enhancement | ||||||||||||
| Priority: | P3 | CC: | cdtdoug, eclipse.sprigogin, malaperle, paedu.hofer, xgsa | ||||||||||
| Version: | 8.0 | ||||||||||||
| Target Milestone: | 8.0 | ||||||||||||
| Hardware: | All | ||||||||||||
| OS: | All | ||||||||||||
| Whiteboard: | |||||||||||||
| Attachments: |
|
||||||||||||
Can you provide a reference why this is a problem? Is this detected by C++ compiler? 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. (In reply to comment #2) > and you can instanciate an abstract class. can't instanciate... (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(); } }; 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?
The patch looks good overall. It would be nice to eliminate the template-related limitation in ClassTypeHelper.getPureVirtualFunctions. (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. (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? (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? (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. Created attachment 192066 [details]
Checker that warns about abstract creation (comment fixture)
Comment was fixed.
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 Patch submitted. *** 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 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? (In reply to comment #15) Patch submitted. Thanks. *** cdt cvs genie on behalf of sprigogin *** Bug 326269. A follow up patch by Anton Gorenkov. [*] AbstractClassInstantiationChecker.java 1.3 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&r1=1.2&r2=1.3 [*] plugin.xml 1.37 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/codan/org.eclipse.cdt.codan.checkers/plugin.xml?root=Tools_Project&r1=1.36&r2=1.37 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'"
Created attachment 195177 [details]
Bug fixture for virtual destructors override
Thank you for the case. Bug was fixed, a test case was added.
(In reply to comment #19) Thank you, Anton. The fix is applied to HEAD > 20110510. *** cdt cvs genie on behalf of sprigogin *** Bug 326269 - Checker for instantiation of an abstract class. Patch by Anton Gorenkov. [*] AbstractClassInstantiationCheckerTest.java 1.2 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&r1=1.1&r2=1.2 [*] ClassTypeHelper.java 1.20 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.19&r2=1.20 |
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.