Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow .member syntax on vector and scalars. #6424

Merged
merged 11 commits into from
Feb 27, 2025
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
166 changes: 69 additions & 97 deletions source/slang/slang-check-expr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4136,12 +4136,6 @@ Expr* SemanticsVisitor::CheckMatrixSwizzleExpr(
IntegerLiteralValue baseElementRowCount,
IntegerLiteralValue baseElementColCount)
{
MatrixSwizzleExpr* swizExpr = m_astBuilder->create<MatrixSwizzleExpr>();
swizExpr->loc = memberRefExpr->loc;
swizExpr->base = memberRefExpr->baseExpression;
swizExpr->memberOpLoc = memberRefExpr->memberOperatorLoc;
swizExpr->checked = true;

// We can have up to 4 swizzles of two elements each
MatrixCoord elementCoords[4];
int elementCount = 0;
Expand Down Expand Up @@ -4170,24 +4164,14 @@ Expr* SemanticsVisitor::CheckMatrixSwizzleExpr(
// Throw out swizzling with more than 4 output elements
if (elementCount >= 4)
{
getSink()->diagnose(
swizExpr,
Diagnostics::invalidSwizzleExpr,
swizzleText,
baseElementType->toString());
return CreateErrorExpr(memberRefExpr);
return nullptr;
}
MatrixCoord elementCoord = {0, 0};

// Check for the preceding underscore
if (*cursor++ != '_')
{
getSink()->diagnose(
swizExpr,
Diagnostics::invalidSwizzleExpr,
swizzleText,
baseElementType->toString());
return CreateErrorExpr(memberRefExpr);
return nullptr;
}

// Check for one or zero indexing
Expand All @@ -4196,12 +4180,7 @@ Expr* SemanticsVisitor::CheckMatrixSwizzleExpr(
// Can't mix one and zero indexing
if (zeroIndexOffset == 1)
{
getSink()->diagnose(
swizExpr,
Diagnostics::invalidSwizzleExpr,
swizzleText,
baseElementType->toString());
return CreateErrorExpr(memberRefExpr);
return nullptr;
}
zeroIndexOffset = 0;
// Increment the index since we saw 'm'
Expand All @@ -4212,12 +4191,7 @@ Expr* SemanticsVisitor::CheckMatrixSwizzleExpr(
// Can't mix one and zero indexing
if (zeroIndexOffset == 0)
{
getSink()->diagnose(
swizExpr,
Diagnostics::invalidSwizzleExpr,
swizzleText,
baseElementType->toString());
return CreateErrorExpr(memberRefExpr);
return nullptr;
}
zeroIndexOffset = 1;
}
Expand All @@ -4229,13 +4203,7 @@ Expr* SemanticsVisitor::CheckMatrixSwizzleExpr(

if (ch < '0' || ch > '4')
{
// An invalid character in the swizzle is an error
getSink()->diagnose(
swizExpr,
Diagnostics::invalidSwizzleExpr,
swizzleText,
baseElementType->toString());
return CreateErrorExpr(memberRefExpr);
return nullptr;
}
const int subIndex = ch - '0' - zeroIndexOffset;

Expand All @@ -4255,12 +4223,7 @@ Expr* SemanticsVisitor::CheckMatrixSwizzleExpr(
// Account for off-by-one and reject 0 if oneIndexed
if (subIndex >= elementLimit || subIndex < 0)
{
getSink()->diagnose(
swizExpr,
Diagnostics::invalidSwizzleExpr,
swizzleText,
baseElementType->toString());
return CreateErrorExpr(memberRefExpr);
return nullptr;
}
}
// Check if we've seen this index before
Expand All @@ -4275,6 +4238,12 @@ Expr* SemanticsVisitor::CheckMatrixSwizzleExpr(
elementCount++;
}

MatrixSwizzleExpr* swizExpr = m_astBuilder->create<MatrixSwizzleExpr>();
swizExpr->loc = memberRefExpr->loc;
swizExpr->base = memberRefExpr->baseExpression;
swizExpr->memberOpLoc = memberRefExpr->memberOperatorLoc;
swizExpr->checked = true;

// Store our list in the actual AST node
for (int ee = 0; ee < elementCount; ++ee)
{
Expand Down Expand Up @@ -4324,11 +4293,7 @@ Expr* SemanticsVisitor::CheckMatrixSwizzleExpr(
constantColCount->getValue());
}
}
getSink()->diagnose(
memberRefExpr,
Diagnostics::unimplemented,
"swizzle on matrix of unknown size");
return CreateErrorExpr(memberRefExpr);
return nullptr;
}

Expr* SemanticsVisitor::checkTupleSwizzleExpr(MemberExpr* memberExpr, TupleType* baseTupleType)
Expand Down Expand Up @@ -4439,26 +4404,13 @@ Expr* SemanticsVisitor::CheckSwizzleExpr(
Type* baseElementType,
IntegerLiteralValue baseElementCount)
{
SwizzleExpr* swizExpr = m_astBuilder->create<SwizzleExpr>();
swizExpr->loc = memberRefExpr->loc;
swizExpr->base = memberRefExpr->baseExpression;
swizExpr->memberOpLoc = memberRefExpr->memberOperatorLoc;
IntegerLiteralValue limitElement = baseElementCount;

ShortList<uint32_t, 4> elementIndices;

bool anyDuplicates = false;
bool anyError = false;
if (memberRefExpr->name == getSession()->getCompletionRequestTokenName())
{
auto& suggestions = getLinkage()->contentAssistInfo.completionSuggestions;
suggestions.clear();
suggestions.scopeKind = CompletionSuggestions::ScopeKind::Swizzle;
suggestions.swizzleBaseType =
memberRefExpr->baseExpression ? memberRefExpr->baseExpression->type : nullptr;
suggestions.elementCount[0] = baseElementCount;
suggestions.elementCount[1] = 0;
}

auto swizzleText = getText(memberRefExpr->name);

for (Index i = 0; i < swizzleText.getLength(); i++)
Expand Down Expand Up @@ -4518,18 +4470,18 @@ Expr* SemanticsVisitor::CheckSwizzleExpr(
elementIndices.add(elementIndex);
}

swizExpr->elementIndices = _Move(elementIndices);

if (anyError)
{
getSink()->diagnose(
swizExpr,
Diagnostics::invalidSwizzleExpr,
swizzleText,
baseElementType->toString());
return CreateErrorExpr(memberRefExpr);
return nullptr;
}
else if (swizExpr->elementIndices.getCount() == 1)

SwizzleExpr* swizExpr = m_astBuilder->create<SwizzleExpr>();
swizExpr->loc = memberRefExpr->loc;
swizExpr->base = memberRefExpr->baseExpression;
swizExpr->memberOpLoc = memberRefExpr->memberOperatorLoc;
swizExpr->elementIndices = _Move(elementIndices);

if (swizExpr->elementIndices.getCount() == 1)
{
// single-component swizzle produces a scalar
//
Expand Down Expand Up @@ -4568,11 +4520,7 @@ Expr* SemanticsVisitor::CheckSwizzleExpr(
}
else
{
getSink()->diagnose(
memberRefExpr,
Diagnostics::unimplemented,
"swizzle on vector of unknown size");
return CreateErrorExpr(memberRefExpr);
return nullptr;
}
}

Expand Down Expand Up @@ -4910,6 +4858,28 @@ Expr* SemanticsVisitor::checkGeneralMemberLookupExpr(MemberExpr* expr, Type* bas
if (expr->name == getSession()->getCompletionRequestTokenName())
{
suggestCompletionItems(CompletionSuggestions::ScopeKind::Member, lookupResult);
if (expr->baseExpression)
{
if (auto vectorType = as<VectorExpressionType>(expr->baseExpression->type))
{
auto& suggestions = getLinkage()->contentAssistInfo.completionSuggestions;
suggestions.scopeKind = CompletionSuggestions::ScopeKind::Swizzle;
suggestions.elementCount[1] = 0;
suggestions.swizzleBaseType = vectorType;
if (auto elementCount = as<ConstantIntVal>(vectorType->getElementCount()))
suggestions.elementCount[0] = elementCount->getValue();
else
suggestions.elementCount[0] = 1;
}
else if (auto scalarType = as<BasicExpressionType>(expr->baseExpression->type))
{
auto& suggestions = getLinkage()->contentAssistInfo.completionSuggestions;
suggestions.scopeKind = CompletionSuggestions::ScopeKind::Swizzle;
suggestions.elementCount[1] = 0;
suggestions.elementCount[0] = 1;
suggestions.swizzleBaseType = scalarType;
}
}
}
return createLookupResultExpr(expr->name, lookupResult, expr->baseExpression, expr->loc, expr);
}
Expand Down Expand Up @@ -4938,34 +4908,36 @@ Expr* SemanticsExprVisitor::visitMemberExpr(MemberExpr* expr)
if (auto modifiedType = as<ModifiedType>(baseType))
baseType = modifiedType->getBase();

// Note: Checking for vector types before declaration-reference types,
// because vectors are also declaration reference types...
// Try handle swizzle-able types (scalar,vector,matrix) first.
// If checking as a swizzle failed for these types,
// we will fallback to normal member lookup.
//
// Also note: the way this is done right now means that the ability
// to swizzle vectors interferes with any chance of looking up
// members via extension, for vector or scalar types.
//
if (auto baseMatrixType = as<MatrixExpressionType>(baseType))
if (auto baseScalarType = as<BasicExpressionType>(baseType))
{
return CheckMatrixSwizzleExpr(
expr,
baseMatrixType->getElementType(),
baseMatrixType->getRowCount(),
baseMatrixType->getColumnCount());
// Treat scalar like a 1-element vector when swizzling
auto swizzle = CheckSwizzleExpr(expr, baseScalarType, 1);
if (swizzle)
return swizzle;
}
if (auto baseVecType = as<VectorExpressionType>(baseType))
else if (auto baseVecType = as<VectorExpressionType>(baseType))
{
return CheckSwizzleExpr(
expr,
baseVecType->getElementType(),
baseVecType->getElementCount());
auto swizzle =
CheckSwizzleExpr(expr, baseVecType->getElementType(), baseVecType->getElementCount());
if (swizzle)
return swizzle;
}
else if (auto baseScalarType = as<BasicExpressionType>(baseType))
else if (auto baseMatrixType = as<MatrixExpressionType>(baseType))
{
// Treat scalar like a 1-element vector when swizzling
return CheckSwizzleExpr(expr, baseScalarType, 1);
auto swizzle = CheckMatrixSwizzleExpr(
expr,
baseMatrixType->getElementType(),
baseMatrixType->getRowCount(),
baseMatrixType->getColumnCount());
if (swizzle)
return swizzle;
}
else if (as<NamespaceType>(baseType))

if (as<NamespaceType>(baseType))
{
return _lookupStaticMember(expr, expr->baseExpression);
}
Expand Down
18 changes: 10 additions & 8 deletions source/slang/slang-check-overload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1316,15 +1316,17 @@ int SemanticsVisitor::CompareLookupResultItems(
// Add a special case for constructors, where we prefer the one that is not synthesized,
if (auto leftCtor = as<ConstructorDecl>(left.declRef.getDecl()))
{
auto rightCtor = as<ConstructorDecl>(right.declRef.getDecl());
bool leftIsSynthesized =
leftCtor->containsFlavor(ConstructorDecl::ConstructorFlavor::SynthesizedDefault);
bool rightIsSynthesized =
rightCtor->containsFlavor(ConstructorDecl::ConstructorFlavor::SynthesizedDefault);

if (leftIsSynthesized != rightIsSynthesized)
if (auto rightCtor = as<ConstructorDecl>(right.declRef.getDecl()))
{
return int(leftIsSynthesized) - int(rightIsSynthesized);
bool leftIsSynthesized = leftCtor->containsFlavor(
ConstructorDecl::ConstructorFlavor::SynthesizedDefault);
bool rightIsSynthesized = rightCtor->containsFlavor(
ConstructorDecl::ConstructorFlavor::SynthesizedDefault);

if (leftIsSynthesized != rightIsSynthesized)
{
return int(leftIsSynthesized) - int(rightIsSynthesized);
}
}
}

Expand Down
30 changes: 19 additions & 11 deletions source/slang/slang-language-server-completion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -512,11 +512,14 @@ LanguageServerResult<CompletionResult> CompletionContext::tryCompleteMemberAndSy

CompletionResult CompletionContext::collectMembersAndSymbols()
{
List<LanguageServerProtocol::CompletionItem> result;

auto linkage = version->linkage;
if (linkage->contentAssistInfo.completionSuggestions.scopeKind ==
CompletionSuggestions::ScopeKind::Swizzle)
{
return createSwizzleCandidates(
createSwizzleCandidates(
result,
linkage->contentAssistInfo.completionSuggestions.swizzleBaseType,
linkage->contentAssistInfo.completionSuggestions.elementCount);
}
Expand All @@ -526,12 +529,12 @@ CompletionResult CompletionContext::collectMembersAndSymbols()
{
return createCapabilityCandidates();
}
List<LanguageServerProtocol::CompletionItem> result;
bool useCommitChars = true;
bool addKeywords = false;
switch (linkage->contentAssistInfo.completionSuggestions.scopeKind)
{
case CompletionSuggestions::ScopeKind::Member:
case CompletionSuggestions::ScopeKind::Swizzle:
useCommitChars =
(commitCharacterBehavior == CommitCharacterBehavior::MembersOnly ||
commitCharacterBehavior == CommitCharacterBehavior::All);
Expand Down Expand Up @@ -698,13 +701,12 @@ CompletionResult CompletionContext::createCapabilityCandidates()
return result;
}

CompletionResult CompletionContext::createSwizzleCandidates(
void CompletionContext::createSwizzleCandidates(
List<LanguageServerProtocol::CompletionItem>& result,
Type* type,
IntegerLiteralValue elementCount[2])
{
List<LanguageServerProtocol::CompletionItem> result;
// Hard code members for vector and matrix types.
result.clear();
if (auto vectorType = as<VectorExpressionType>(type))
{
const char* memberNames[4] = {"x", "y", "z", "w"};
Expand All @@ -724,6 +726,18 @@ CompletionResult CompletionContext::createSwizzleCandidates(
result.add(item);
}
}
else if (auto scalarType = as<BasicExpressionType>(type))
{
String typeStr;
if (scalarType)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant conditional

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

typeStr = scalarType->toString();
LanguageServerProtocol::CompletionItem item;
item.data = 0;
item.detail = typeStr;
item.kind = LanguageServerProtocol::kCompletionItemKindVariable;
item.label = "x";
result.add(item);
}
else if (auto matrixType = as<MatrixExpressionType>(type))
{
Type* elementType = nullptr;
Expand Down Expand Up @@ -769,12 +783,6 @@ CompletionResult CompletionContext::createSwizzleCandidates(
result.add(item);
}
}
for (auto& item : result)
{
for (auto ch : getCommitChars())
item.commitCharacters.add(ch);
}
return result;
}

LanguageServerProtocol::CompletionItem CompletionContext::generateGUIDCompletionItem()
Expand Down
5 changes: 4 additions & 1 deletion source/slang/slang-language-server-completion.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,10 @@ struct CompletionContext


CompletionResult collectMembersAndSymbols();
CompletionResult createSwizzleCandidates(Type* baseType, IntegerLiteralValue elementCount[2]);
void createSwizzleCandidates(
List<LanguageServerProtocol::CompletionItem>& result,
Type* type,
IntegerLiteralValue elementCount[2]);
CompletionResult createCapabilityCandidates();
CompletionResult collectAttributes();
LanguageServerProtocol::CompletionItem generateGUIDCompletionItem();
Expand Down
Loading
Loading