Repository: shader-slang/slang
File: source/slang/slang-emit-c-like.cpp (C++)

Code before:
            // Simple constructor call
            auto prec = getInfo(EmitOp::Prefix);
            needClose = maybeEmitParens(outerPrec, prec);

            m_writer->emit("(");
            emitType(inst->getDataType());
            m_writer->emit(")");

            emitOperand(inst->getOperand(0), rightSide(outerPrec, prec));
            break;
        }
    case kIROp_FieldExtract:
        {
            // Extract field from aggregate
            IRFieldExtract* fieldExtract = (IRFieldExtract*)inst;

            auto prec = getInfo(EmitOp::Postfix);
            needClose = maybeEmitParens(outerPrec, prec);

            auto base = fieldExtract->getBase();
            emitOperand(base, leftSide(outerPrec, prec));

            // Use -> if:
            // 1. Base type is a class type, OR
            // 2. Base type is a pointer to class type, OR
            // 3. Base is a GetAddress operation (which produces a pointer)
            if (base->getDataType()->getOp() == kIROp_ClassType ||
                isPtrToClassType(base->getDataType()) || base->getOp() == kIROp_GetAddress)
                m_writer->emit("->");
            else
                m_writer->emit(".");
            m_writer->emit(getName(fieldExtract->getField()));
            break;
        }
    case kIROp_FieldAddress:
        {
            // Extract field "address" from aggregate

            IRFieldAddress* ii = (IRFieldAddress*)inst;

            if (doesTargetSupportPtrTypes())
            {
                auto prec = getInfo(EmitOp::Prefix);
                needClose = maybeEmitParens(outerPrec, prec);
                m_writer->emit("&");
                outerPrec = rightSide(outerPrec, prec);
                auto innerPrec = getInfo(EmitOp::Postfix);
                bool innerNeedClose = maybeEmitParens(outerPrec, innerPrec);
                auto base = ii->getBase();
                if (isPtrToClassType(base->getDataType()))
                    emitDereferenceOperand(base, leftSide(outerPrec, innerPrec));

Review comment (suggestion):
This comment seems useless when the code is so straightforward.
It actually confused me when I read "Use -> if".
I had to read the implementation to understand what the comment meant, which is counter-productive.
It might be better if it was
```
// Use `->` instead of `.` if any of the following condition is true:
```

Code after:
            // Simple constructor call
            auto prec = getInfo(EmitOp::Prefix);
            needClose = maybeEmitParens(outerPrec, prec);

            m_writer->emit("(");
            emitType(inst->getDataType());
            m_writer->emit(")");

            emitOperand(inst->getOperand(0), rightSide(outerPrec, prec));
            break;
        }
    case kIROp_FieldExtract:
        {
            // Extract field from aggregate
            IRFieldExtract* fieldExtract = (IRFieldExtract*)inst;

            auto prec = getInfo(EmitOp::Postfix);
            needClose = maybeEmitParens(outerPrec, prec);

            auto base = fieldExtract->getBase();
            emitOperand(base, leftSide(outerPrec, prec));
            if (base->getDataType()->getOp() == kIROp_ClassType)
                m_writer->emit("->");
            else
                m_writer->emit(".");
            m_writer->emit(getName(fieldExtract->getField()));
            break;
        }
    case kIROp_FieldAddress:
        {
            // Extract field "address" from aggregate

            IRFieldAddress* ii = (IRFieldAddress*)inst;

            if (doesTargetSupportPtrTypes())
            {
                auto prec = getInfo(EmitOp::Prefix);
                needClose = maybeEmitParens(outerPrec, prec);
                m_writer->emit("&");
                outerPrec = rightSide(outerPrec, prec);
                auto innerPrec = getInfo(EmitOp::Postfix);
                bool innerNeedClose = maybeEmitParens(outerPrec, innerPrec);
                auto base = ii->getBase();
                if (isPtrToClassType(base->getDataType()))
                    emitDereferenceOperand(base, leftSide(outerPrec, innerPrec));
                else
                    emitOperand(base, leftSide(outerPrec, innerPrec));
                m_writer->emit("->");
                m_writer->emit(getName(ii->getField()));
                maybeCloseParens(innerNeedClose);
            }