Fix pack expansion for array elements

[i-garrison]

When parameter pack contains array type the EvalCompositeAccess.getType() will attempt to return type of array element.

Fix this by providing EvalPackAccess which returns pack elements as is.

[jonahgraham]

Added call for reviewers on cdt-dev. If none are forthcoming we'll need a plan-B (me learning this area?)

[i-garrison]

Added call for reviewers on cdt-dev. If none are forthcoming we'll need a plan-B (me learning this area?)

Well I can try to share what I have learned of CDT if you need help.. Evaluating stuff does not look particularly hard in CDT but there is a lot of code - fortunately most implementations are quite similar in structure, and there are lots of tests too.

[ddscharfe]

LGTM.

There might be another way implementing this. Instead of adding the new class EvalPackAccess you could add another case to EvalCompositeAccess::getType:

	@Override
	public IType getType() {
		IType type = getParent().getType();
		type = SemanticUtil.getNestedType(type, TDEF | REF | CVTYPE);

		if (type instanceof IArrayType) {
			IArrayType arrayType = (IArrayType) type;
			return arrayType.getType();
		}
		...

Add case for detecting packs:

	@Override
	public IType getType() {
		IType type = getParent().getType();

		type = SemanticUtil.getNestedType(type, TDEF | REF | CVTYPE);

		if (parent instanceof EvalBinding parentBinding && parentBinding.getBinding() instanceof ICPPSpecialization spec
				&& spec.getSpecializedBinding() instanceof IVariable v
				&& v.getType() instanceof ICPPParameterPackType) {
			return type;
		} else if (type instanceof IArrayType) {
			IArrayType arrayType = (IArrayType) type;
			return arrayType.getType();
		}
		...

I'm not sure what's the better solution, yours involves more changes but might be more solid because it doesn't need the additional detection logic.

[i-garrison]

I'm not sure what's the better solution, yours involves more changes but might be more solid because it doesn't need the additional detection logic.

Yes to my mind EvalPackAccess subclass looks cleaner since this is just an eval helper to access pack elements. I would not be concerned with more boilerplate changes here in exchange for not adding yet another role to EvalCompositeAccess.

[jonahgraham]

I have kicked off the build, assuming it all goes well this looks good to go.

Once your first PR gets merged, future PRs will run the build automatically.

[github-actions]

Test Results

     593 files       593 suites   17m 10s ⏱️
10 136 tests 10 113 ✔️ 22 💤 1 ❌
10 152 runs  10 129 ✔️ 22 💤 1 ❌

For more details on these failures, see this check.

Results for commit 58116bd.

♻️ This comment has been updated with latest results.

[jonahgraham]

Pretty sure failing test is unrelated. I'll need to check that it is in he to do list when I get back to my desk.

[jonahgraham]

Pretty sure failing test is unrelated. I'll need to check that it is in he to do list when I get back to my desk.

The failing test is:

testDCOM_BDFG (org.eclipse.cdt.ui.tests.text.doctools.DocCommentHighlightingTest)

with this output;

expected:<[offset: 149, length: 17, offset: 184, length: 18, offset: 223, length: 16, offset: 241, length: 17, offset: 258, length: 16, offset: 345, length: 16]> but was:<[]>
junit.framework.AssertionFailedError: expected:<[offset: 149, length: 17, offset: 184, length: 18, offset: 223, length: 16, offset: 241, length: 17, offset: 258, length: 16, offset: 345, length: 16]> but was:<[]>
	at org.eclipse.cdt.ui.tests.text.doctools.DocCommentHighlightingTest.testDCOM_BDFG(DocCommentHighlightingTest.java:211)

which doesn't appear in any of my previous marked releng labelled issues. I have kicked off the build fresh.

[i-garrison]

Pretty sure failing test is unrelated. I'll need to check that it is in he to do list when I get back to my desk.

The failing test is:

testDCOM_BDFG (org.eclipse.cdt.ui.tests.text.doctools.DocCommentHighlightingTest)

with this output;

expected:<[offset: 149, length: 17, offset: 184, length: 18, offset: 223, length: 16, offset: 241, length: 17, offset: 258, length: 16, offset: 345, length: 16]> but was:<[]>
junit.framework.AssertionFailedError: expected:<[offset: 149, length: 17, offset: 184, length: 18, offset: 223, length: 16, offset: 241, length: 17, offset: 258, length: 16, offset: 345, length: 16]> but was:<[]>
	at org.eclipse.cdt.ui.tests.text.doctools.DocCommentHighlightingTest.testDCOM_BDFG(DocCommentHighlightingTest.java:211)

which doesn't appear in any of my previous marked releng labelled issues. I have kicked off the build fresh.

Note this failure is the same as in PR #255 and we were not able reproduce it locally yet https://github.com/eclipse-cdt/cdt/pull/255/checks?check_run_id=10773221361

[jonahgraham]

Note this failure is the same as in PR #255 and we were not able reproduce it locally yet https://github.com/eclipse-cdt/cdt/pull/255/checks?check_run_id=10773221361

Thanks - I have created the issue for the flaky test, #259