Repository: shader-slang/slang
File: source/slang/slang-linkable.cpp (C++)

Code before:
    SharedSemanticsContext sharedSemanticsContext(linkage, nullptr, sink);
    SemanticsVisitor visitor(&sharedSemanticsContext);
    type = visitor.TranslateTypeNode(typeExpr);
    auto typeOut = visitor.tryCoerceToProperType(TypeExp(type));
    if (typeOut.type)
        type = typeOut.type;

    if (type)
    {
        m_types[typeStr] = type;
    }
    return type;
}

Expr* ComponentType::tryResolveOverloadedExpr(Expr* exprIn)
{
    auto linkage = getLinkage();
    SemanticsContext context(linkage->getSemanticsForReflection());
    SemanticsVisitor visitor(context);
    return visitor.maybeResolveOverloadedExpr(exprIn, LookupMask::Function, nullptr);
}

// We perform a conservative conversion from OverloadedExpr2 to OverloadedExpr here, it will
// only convert the expression when all candidates are DeclRefExpr and share the same originalExpr
// as that will be what OverloadedExpr expects
static OverloadedExpr* convertOverloadExpr2(
    OverloadedExpr2* overloadedExpr2,
    ASTBuilder* astBuilder)
{
    if (!overloadedExpr2->candidateExprs.getCount())
        return nullptr;

    auto overloadedExpr = astBuilder->create<OverloadedExpr>();

    Expr* originalExpr = nullptr;

    for (auto candidate : overloadedExpr2->candidateExprs)
    {
        if (auto declRefExpr = as<DeclRefExpr>(candidate))
        {
            LookupResultItem item;
            item.declRef = declRefExpr->declRef;
            overloadedExpr->lookupResult2.items.add(item);

            if (!originalExpr)
            {
                originalExpr = declRefExpr->originalExpr;
            }
            else if (originalExpr != declRefExpr->originalExpr)
            {
                return nullptr;

Review comment (bug):
This comment is a good start, but I would like to see a more detailed explanation of the problem this is solving, the differences between the two overloaded expression representations (in terms of how they store the candidates), and a discussion of the strategy being used to attempt conversion.

It would also be good to document that this function returns `nullptr` on failure. Along those lines, it might be best to name this `tryConvertOverloadExpr2ToOverloadExpr` to both be more explicit about what conversion it performs and also that it only *tries* to perform the conversion, and can fail.

Code after:
    SharedSemanticsContext sharedSemanticsContext(linkage, nullptr, sink);
    SemanticsVisitor visitor(&sharedSemanticsContext);
    type = visitor.TranslateTypeNode(typeExpr);
    auto typeOut = visitor.tryCoerceToProperType(TypeExp(type));
    if (typeOut.type)
        type = typeOut.type;

    if (type)
    {
        m_types[typeStr] = type;
    }
    return type;
}

Expr* ComponentType::tryResolveOverloadedExpr(Expr* exprIn)
{
    auto linkage = getLinkage();
    SemanticsContext context(linkage->getSemanticsForReflection());
    SemanticsVisitor visitor(context);
    return visitor.maybeResolveOverloadedExpr(exprIn, LookupMask::Function, nullptr);
}

// This function tries to simplify an overloaded expr into OverloadedExpr for reflection API usage.
// There are two kinds of overloaded expr in the AST: OverloadedExpr and OverloadedExpr2.
//
// OverloadedExpr stores candidates in LookupResult, where a list of `DeclRef<Decl>` hold
// the properly-specialized reference to the declaration that was found. And all the candidates
// must share a same base (if it is coming from a member-reference), and same orignalExpr.
//
// While OverloadedExpr2 stores candidates in a list of Expr, which is not necessary to be
// DeclRefExpr.
//
// When the input orignalExpr is already OverloadedExpr, we can directly return it. But when
// the input orignalExpr is OverloadedExpr2, we need to simplify it by converting it into
// OverloadedExpr. The conversion routine conservatively performs the conversion when each Expr
// candidates of OverloadedExpr2 is DeclRefExpr and all the candidates DeclRefExpr share the same
// orignalExpr. If such condition is not met, it will return nullptr to indicated failed conversion.
static Expr* maybeSimplifyExprForReflectionAPIUsage(Expr* originalExpr, ASTBuilder* astBuilder)
{
    // return directly if it is already OverloadedExpr
    if (as<OverloadedExpr>(originalExpr))
        return originalExpr;

    OverloadedExpr2* overloadedExpr2 = as<OverloadedExpr2>(originalExpr);
    // Don't perform any conversion if it is not OverloadedExpr2
    if (!overloadedExpr2)
        return originalExpr;

    if (!overloadedExpr2->candidateExprs.getCount())
        return nullptr;