[WIP] Unify AST::ExternalFunctionItem with AST::Function

[0xn4utilus]

Added support for external function in AST::Function. It can be verified it in AST validation as follows. But in line 6113 of rust-parse-impl.h, parse_external_item() needs return type ExternalItem from parse_function() which returns Function, thus we cannot directly use parse_function(). How should I proceed ?

[powerboat9]

You'd probably want to make Function inherit from ExternalItem

[0xn4utilus]

Wanted to do that, but will it be a good practice? Also it generates ambiguous functions, that can be probably tackled with virtual and override.

[P-E-P]

You're right. We'll need to inherit from ExternalItem in order to return a function from this function but it'll be probably harder than expected.

At first I thought this would cause a lot of troubles, turns out we have a dead ExternalItem class in rust-item.h that should probably be deleted.

The real ExternalItem lies in rust-ast.h. It duplicates node_id with Stmt (Function -> VisItem -> Stmt). We could make ExternalItem inherit from item instead of Visitable but this surely have some unwanted outcome I can't see right now.

[0xn4utilus]

Yes, will try out some things and see if it works.

[powerboat9]

I'm pretty sure you can fix ambiguous functions by overriding them in Function, which wouldn't be ideal, but might be the best option.

[0xn4utilus]

Yes overriding works, now I have to make corresponding modifications in Rust::HIR::ASTLoweringExternItem.

[0xn4utilus]

I am getting an unexpected bug. Where should I start looking? It is working fine for functions, and external-functions with no parameters.

extern "C"{
        fn fun(a:i32);
}

test.rs:2:25: error: unknown reference for resolved name: ‘i32’ 2 | fn fun(a:i32); | ^

[powerboat9]

It doesn't look like the types of external function parameters are being resolved. Either that, or external items in the extern "C" {} block aren't being resolved. What happens if you have an external function that has no parameters but returns an i32?

[0xn4utilus]

extern "C"{
        fn fun()->i32;
}

has the same error. So probably external function items are not being resolved

[powerboat9]

You'd also need to fix the build errors and format the commit messages

[0xn4utilus]

Requested changes made and removed AST::ExternalFunctionItem dead code.

[powerboat9]

Looks good. You might want to split off some of these commits into separate pull requests, so that they can get reviewed/merged faster -- that's what I tend to do

[0xn4utilus]

Looks good. You might want to split off some of these commits into separate pull requests, so that they can get reviewed/merged faster -- that's what I tend to do

Sure

[P-E-P]

I'm not a fan of the NodeId duplication, I'll take a deeper look in the upcoming days to be sure there's no way around.

Otherwise, it looks good.

Removing the NodeId requires reordering some inheritance and I don't feel comfortable doing that soon since I haven't got much time and we should probably focus on a stable GCC14 release.

[0xn4utilus]

Rebased, now it should be able to merge

[P-E-P]

Rebased, now it should be able to merge

Not entirely sure, sometimes the queue is doing weird things and we have to merge by hand. Let's try again.