[iwami4438]

Ref: #3311

Before↓

const ErrorSet = error{A} || error{B}
(type = error{A,B})

const ErrorSetWithUnknown = error{A} || undefined.Unknown
(unknown)

const ErrorSetWithUnknown2 = undefined.Unknown || error{A}
(unknown)

const ErrorSetWithUnknown3 = undefined.Unknown || error{A} || error{B}
(unknown)

const AllUnknown = undefined.Unknown || undefined.Unknown
(unknown)

After: ↓

const ErrorSet = error{A} || error{B}
(type = error{A,B})

const ErrorSetWithUnknown = error{A} || undefined.Unknown
(type = error{A, ... })

const ErrorSetWithUnknown2 = undefined.Unknown || error{B}
(type = error{B, ... })

const ErrorSetWithUnknown3 = undefined.Unknown || error{A} || error{B}
(type = error{A,B, ... })

const AllUnknown = undefined.Unknown || undefined.Unknown
(type = error{ ... })

[iwami4438]

There was an error in the test (it has been fixed—the number of ^ symbols was insufficient). Could you please re-run the CI?

[neoto]

I won't comment on the implementation since I'm not familiar with the codebase, but I tested the PR with these test cases and they all crash the server:

const AnyerrorUnknown = anyerror || Unknown;

const UnknownAnyerror = Unknown || anyerror;

const AnyerrorPartiallyUnknown = anyerror || error{A} || Unknown;

At the very least, I think this should now be lifted up from the else branch to the start of the function:

        // Anything merged with anyerror is anyerror.
        if (a_ty == .anyerror_type or b_ty == .anyerror_type) {
            return .anyerror_type;
        }

[neoto]

Is there a reason why you haven't just used a bool for this?

[iwami4438]

I considered using a bool for has_unknown, but that would have required additional implementation work, so I decided to use something like Index.bool_true instead.
It is a place like the one described below.

zls/src/analyser/InternPool.zig

Line 1584 in 066658e

fn extraData(ip: *const InternPool, comptime T: type, index: u32) T {

[neoto]

Could probably use the same Unknown defined at the top, no?

[iwami4438]

You are quite right, so I will revise the test.

[iwami4438]

I won't comment on the implementation since I'm not familiar with the codebase, but I tested the PR with these test cases and they all crash the server:

const AnyerrorUnknown = anyerror || Unknown;

const UnknownAnyerror = Unknown || anyerror;

const AnyerrorPartiallyUnknown = anyerror || error{A} || Unknown;

At the very least, I think this should now be lifted up from the else branch to the start of the function:

        // Anything merged with anyerror is anyerror.
        if (a_ty == .anyerror_type or b_ty == .anyerror_type) {
            return .anyerror_type;
        }

Thank you for your feedback.
anyerror was overlooked—my apologies.
I will add a test.

[techatrix]

@iwami4438 Did use LLMs or AI to author anything here? Let it be the source code, commit descriptions or your comments/replies. Also counting AI translation tools.

[iwami4438]

@iwami4438 Did use LLMs or AI to author anything here? Let it be the source code, commit descriptions or your comments/replies. Also counting AI translation tools.

I am using Google Translate for the English text; please excuse any awkward phrasing. I have not used any AI.