Skip to content

Commit

Permalink
[ATOM-4800] Unclear Error Message for Function Inside Struct (o3de#6)
Browse files Browse the repository at this point in the history
In order to catch the error of function inside struct
it was necessary to modify the grammar to allow function definitions
and declarations inside "struct" definitions (just like classes,
interfaces and ShaderResourceGroups), this way
the ANTLR parser would let it pass, giving the chance for AZSLc
to provide a meaningful error message.

ANTLR Syntax Error message:
"no viable alternative at input
structParallaxMapping{Texture2Dm_depthMap;floatm_depthFactor;boolm_isHeightmap;floatm_parallaxShadowFactor;float2GetParallaxUV("

New AZSLc produced Error message:
"function_declared_in_struct.azsl(14,4) : Syntax error #516: structs
cannot have member functions; consider using a class."

Signed-off-by: garrieta <[email protected]>
  • Loading branch information
galibzon authored Sep 13, 2021
1 parent 560a99d commit 5e809f4
Show file tree
Hide file tree
Showing 10 changed files with 1,482 additions and 1,370 deletions.
1 change: 1 addition & 0 deletions src/AzslcException.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ namespace AZ::ShaderCompiler
ADVANCED_SYNTAX_CONSTANT_BUFFER_ONLY_IN_SRG = 513u,
ADVANCED_SYNTAX_DOUBLE_SCOPE_RESOLUTION = 514u,
ADVANCED_RESERVED_NAME_USED = 515u,
ADVANCED_SYNTAX_FUNCTION_IN_STRUCT = 516u,
};

class AzslcException : public antlr4::RuntimeException
Expand Down
18 changes: 18 additions & 0 deletions src/AzslcListener.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,15 @@ namespace AZ::ShaderCompiler

void SemaCheckListener::enterFunctionDefinition(azslParser::FunctionDefinitionContext* ctx)
{
// Forbid function definitions inside "struct"s.
if (m_ir->m_sema.IsScopeStruct())
{
throw AzslcException(ADVANCED_SYNTAX_FUNCTION_IN_STRUCT,
"Syntax",
ctx->start,
"structs cannot have member functions; consider using a class.");
}

auto signature = ctx->hlslFunctionDefinition()->leadingTypeFunctionSignature();
auto uqName = ExtractNameFromAnyContextWithName(signature);

Expand Down Expand Up @@ -226,6 +235,15 @@ namespace AZ::ShaderCompiler

void SemaCheckListener::enterFunctionDeclaration(azslParser::FunctionDeclarationContext* ctx)
{
// Forbid function declarations inside "struct"s.
if (m_ir->m_sema.IsScopeStruct())
{
throw AzslcException(ADVANCED_SYNTAX_FUNCTION_IN_STRUCT,
"Syntax",
ctx->start,
"structs cannot have member functions; consider using a class.");
}

auto signature = ctx->hlslFunctionDeclaration()->leadingTypeFunctionSignature();
auto uqName = ExtractNameFromAnyContextWithName(signature);
auto& [id, _] = m_ir->m_sema.RegisterFunctionDeclarationAndAddSeenat(uqName, signature);
Expand Down
2 changes: 1 addition & 1 deletion src/AzslcMain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ namespace StdFs = std::filesystem;
// For large features or milestones. Minor version allows for breaking changes. Existing tests can change.
#define AZSLC_MINOR "7"
// For small features or bug fixes. They cannot introduce breaking changes. Existing tests shouldn't change.
#define AZSLC_REVISION "27" // ATOM-2430
#define AZSLC_REVISION "28" // ATOM-4800

namespace AZ::ShaderCompiler
{
Expand Down
6 changes: 6 additions & 0 deletions src/AzslcSemanticOrchestrator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1953,6 +1953,12 @@ namespace AZ::ShaderCompiler
return found;
}

bool SemanticOrchestrator::IsScopeStruct() const
{
const KindInfo& scopeKind = m_symbols->GetIdAndKindInfo(m_scope->GetNameOfCurScope())->second;
return scopeKind.IsKindOneOf(Kind::Struct);
}

bool SemanticOrchestrator::IsScopeStructClassInterface() const
{
const KindInfo& scopeKind = m_symbols->GetIdAndKindInfo(m_scope->GetNameOfCurScope())->second;
Expand Down
5 changes: 4 additions & 1 deletion src/AzslcSemanticOrchestrator.h
Original file line number Diff line number Diff line change
Expand Up @@ -351,9 +351,12 @@ namespace AZ::ShaderCompiler
// another helper to streamline what to do directly with the result from ExtractTypeNameFromAstContext function families.
auto CreateExtendedTypeInfo(const ExtractedComposedType& extractedComposed, ArrayDimensions dims, Packing::MatrixMajor mtxMajor) const -> ExtendedTypeInfo;

//! check if current scope is a structured user defined type
//! check if current scope is a structured user defined type ("struct", "class" or "interface")
bool IsScopeStructClassInterface() const;

//! check if current scope is of type "struct".
bool IsScopeStruct() const;

//! Find a concrete function from an overload-set and an argument list.
//! (adjust the shoot of a lookup to something more precise in case that it's possible)
//! if maybeOverloadSet is not an overload-set the function returns identity
Expand Down
2 changes: 2 additions & 0 deletions src/azslParser.g4
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ structDefinition:

structMemberDeclaration:
variableDeclarationStatement
| attributedFunctionDefinition //AZSL+, forbidden, but allows us to provide better error message.
| attributedFunctionDeclaration //AZSL+, forbidden, but allows us to provide better error message.
| anyStructuredTypeDefinitionStatement // AZSL+
| typeAliasingDefinitionStatement // AZSL+
;
Expand Down
Loading

0 comments on commit 5e809f4

Please sign in to comment.