Skip to content
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

Constructors of inner classes which are declared in the implementation are not resolved properly when using 'new' expressions #254

Closed
ddscharfe opened this issue Jan 19, 2023 · 1 comment · Fixed by #255
Labels
language C/C++ Language Support
Milestone

Comments

@ddscharfe
Copy link
Contributor

Bug Description
Consider the following simple example (struct is used for simplicity, the problem also applies to classes):

MyClass.h

#ifndef MYCLASS_H_
#define MYCLASS_H_

struct MyClass {
	MyClass();
	struct MyInnerClass;
};

#endif /* MYCLASS_H_ */

MyClass.cpp

#include "MyClass.h"

struct MyClass::MyInnerClass {
	MyInnerClass(bool a, bool b) {
	}
};

MyClass::MyClass() {
	new MyInnerClass(true, true);
}

This is correct C++, but the constructor which is declared in the .cpp file is not resolved properly:
image
This happens only if MyInnerClass is instantiated using the new operator, e.g. MyInnerClass inner(true, true) will work fine.

Explaination
I debugged this issue and will try to provide a fix soon. My current findings are best illustrated by the following stack trace:
image
CPPSemantics::resolveAmbiguities tries to find the binding for the constructor, by analyzing all constructors. Constructors which are not declared yet, are filtered out (CPPSemantics::2228).
The constructors are indexed, hence they are IPDOMBindings. CPPSemantics::declaredBefore treats ICPPInternalBindings and IPDOMBindings differently. In the latter case the implementation will use the IIndexFileSet of the IASTTranslationUnit and check if it contains the currently analyzed constructor.
When the constructor is declared in the header file, it will be found in the index file set, but in our case it won't. As a result, the constructor candidate is filtered out and not passed to the following logic.

Fun Fact
When there is another constructor defined in the header file, which has a different signature, but the same number of arguments, the bug disappears:

#ifndef MYCLASS_H_
#define MYCLASS_H_

struct MyClass {
	MyClass();
	struct MyInnerClass {
		MyInnerClass(int a, int b) {
		}
	};
};
#endif /* MYCLASS_H_ */

This is due to the fact that CPPSemantics::resolveAmbiguities passes the list of the filtered constructors to CPPSemantics::resolveFunction, which however uses the original (unfiltered list) and only uses the filtered list to exclude constructors by the number of its arguments.

ddscharfe added a commit to ddscharfe/cdt that referenced this issue Jan 20, 2023
…perly

- add test case to reproduce eclipse-cdt#254
- add special case to detect constructors which were declared in the
index file set of the tu
@ddscharfe
Copy link
Contributor Author

Also see https://bugs.eclipse.org/bugs/show_bug.cgi?id=402498 and fbccef3 for context.

jonahgraham pushed a commit that referenced this issue Jan 28, 2023
- add test case to reproduce #254
- add special case to detect constructors which were declared in the
index file set of the tu
@jonahgraham jonahgraham added the language C/C++ Language Support label Jan 28, 2023
@jonahgraham jonahgraham added this to the 11.1.0 milestone Jan 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
language C/C++ Language Support
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants