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

Code before:
    // the modules that were directly or
    // indirectly referenced.
    //
    Scope* scope = _getOrCreateScopeForLegacyLookup(astBuilder);

    auto linkage = getLinkage();

    SLANG_AST_BUILDER_RAII(linkage->getASTBuilder());

    Expr* expr = linkage->parseTermString(name, scope);

    SemanticsContext context(linkage->getSemanticsForReflection());
    context = context.allowStaticReferenceToNonStaticMember().withSink(sink);

    SemanticsVisitor visitor(context);

    auto checkedExpr = visitor.CheckTerm(expr);

    if (as<DeclRefExpr>(checkedExpr) || as<OverloadedExpr>(checkedExpr))
    {
        result = checkedExpr;
    }
    else if (auto overloadExpr2 = as<OverloadedExpr2>(checkedExpr))
    {
        result = convertOverloadExpr2(overloadExpr2, astBuilder);
    }

    m_decls[name] = result;
    return result;
}

static bool _isSimpleName(String const& name)
{
    for (char c : name)
    {
        if (!CharUtil::isAlphaOrDigit(c) && c != '_' && c != '$')
            return false;
    }
    return true;
}

Expr* ComponentType::findDeclFromStringInType(
    Type* type,
    String const& name,
    LookupMask mask,
    DiagnosticSink* sink)
{
    // Only look up in the type if it is a DeclRefType
    if (!as<DeclRefType>(type))
        return nullptr;


Review comment (refactor):
It might be good to define a subroutine along the lines of:

    Expr* maybeSimplifyExprForReflectionAPIUsage(Expr* originalExpr, ASTBuilder* astBuilder);

That function would be a catch-all to try to simplify the given `originalExpr` in ways that may make it more useful as a result to expose via the reflection API, and return either a simplified expression or the original expression.

That subroutine could handle the conditional check for `OverloadedExpr2`, so that this and the other use site would just become calls like:

    result = maybeSimplifyExprForReflectionAPIUsage(result, astBuilder);

That refactor would keep the messy details out of the flow here, and also make it more clear *why* this logic is being invoked in the first place.

Code after:
Expr* ComponentType::findDeclFromString(String const& name, DiagnosticSink* sink)
{
    // If we've looked up this type name before,
    // then we can re-use it.
    //
    Expr* result = nullptr;
    if (m_decls.tryGetValue(name, result))
        return result;


    // TODO(JS): For now just used the linkages ASTBuilder to keep on scope
    //
    // The parseTermString uses the linkage ASTBuilder for it's parsing.
    //
    // It might be possible to just create a temporary ASTBuilder - the worry though is
    // that the parsing sets a member variable in AST node to one of these scopes, and then
    // it become a dangling pointer. So for now we go with the linkages.
    auto astBuilder = getLinkage()->getASTBuilder();

    // Otherwise, we need to start looking in
    // the modules that were directly or
    // indirectly referenced.
    //
    Scope* scope = _getOrCreateScopeForLegacyLookup(astBuilder);

    auto linkage = getLinkage();

    SLANG_AST_BUILDER_RAII(linkage->getASTBuilder());

    Expr* expr = linkage->parseTermString(name, scope);

    SemanticsContext context(linkage->getSemanticsForReflection());
    context = context.allowStaticReferenceToNonStaticMember().withSink(sink);

    SemanticsVisitor visitor(context);

    auto checkedExpr = visitor.CheckTerm(expr);

    if (as<DeclRefExpr>(checkedExpr) || as<OverloadedExpr>(checkedExpr))
    {
        result = checkedExpr;
    }
    result = maybeSimplifyExprForReflectionAPIUsage(checkedExpr, astBuilder);

    m_decls[name] = result;
    return result;
}

static bool _isSimpleName(String const& name)
{
    for (char c : name)