From b42b865ac7ce144561fa17743616e550eeae7102 Mon Sep 17 00:00:00 2001 From: Tim Foley Date: Wed, 5 Feb 2020 13:26:39 -0800 Subject: Improve behavior when undefined identifier is a contextual keyword (#1200) The HLSL language has keywords with very common names like `triangle`, and Slang doesn't want to preclude users from using such names for their variables/functions/etc. In addition, Slang adds new keywords on top of HLSL (like `extension`) and we don't want those to prevent us from compiling existing code. As a result, almost all keywords in Slang are contextual keywords, and they can be shadowed by user varaibles. The down-side to making all keywords contextual is that in a case like this: ``` int test() { return triangle; } ``` The identifier `triangle` is *not* undefined as far as lookup (it is defined as a modifier keyword), so the existing "undefined identifier" logic gets bypassed, and instead we ran into an internal compiler error trying to construct an expression that refers to a modifier keyword. Fortunately, the internal compiler error in that case was overkill, and the compiler already had defensive logic to produce an expression with an error type if it couldn't figure out what the type of a declaration reference should be. The main fix here is thus to emit an "undefined identifier" error instead of an internal compiler error at the point where we see an attempt to reference a declaration that shouldn't be available in an expression context. In order to improve the quality of the diagnostic, the code for constructing declaration references was updated to pass along a source location to be used in error messages. --- source/slang/slang-check-decl.cpp | 27 +++++++++++++++++++++------ source/slang/slang-check-expr.cpp | 2 +- source/slang/slang-check-impl.h | 2 +- source/slang/slang-diagnostic-defs.h | 1 - source/slang/slang-lookup.cpp | 3 ++- source/slang/slang-lookup.h | 6 ++++-- 6 files changed, 29 insertions(+), 12 deletions(-) (limited to 'source') diff --git a/source/slang/slang-check-decl.cpp b/source/slang/slang-check-decl.cpp index 027ddc70f..7c8322fa7 100644 --- a/source/slang/slang-check-decl.cpp +++ b/source/slang/slang-check-decl.cpp @@ -252,7 +252,8 @@ namespace Slang SemanticsVisitor* sema, DiagnosticSink* sink, DeclRef declRef, - RefPtr* outTypeResult) + RefPtr* outTypeResult, + SourceLoc loc) { if( sema ) { @@ -386,17 +387,30 @@ namespace Slang } if( sink ) { - sink->diagnose(declRef, Diagnostics::unimplemented, "cannot form reference to this kind of declaration"); + // The compiler is trying to form a reference to a declaration + // that doesn't appear to be usable as an expression or type. + // + // In practice, this arises when user code has an undefined-identifier + // error, but the name that was undefined in context also matches + // a contextual keyword. Rather than confuse the user with the + // details of contextual keywords in the compiler, we will diagnose + // this as an undefined identifier. + // + // TODO: This code could break if we ever go down this path with + // an identifier that doesn't have a name. + // + sink->diagnose(loc, Diagnostics::undefinedIdentifier2, declRef.GetName()); } return QualType(session->getErrorType()); } QualType getTypeForDeclRef( Session* session, - DeclRef declRef) + DeclRef declRef, + SourceLoc loc) { RefPtr typeResult; - return getTypeForDeclRef(session, nullptr, nullptr, declRef, &typeResult); + return getTypeForDeclRef(session, nullptr, nullptr, declRef, &typeResult, loc); } DeclRef ApplyExtensionToType( @@ -2832,7 +2846,7 @@ namespace Slang return extDeclRef; } - QualType SemanticsVisitor::GetTypeForDeclRef(DeclRef declRef) + QualType SemanticsVisitor::GetTypeForDeclRef(DeclRef declRef, SourceLoc loc) { RefPtr typeResult; return getTypeForDeclRef( @@ -2840,7 +2854,8 @@ namespace Slang this, getSink(), declRef, - &typeResult); + &typeResult, + loc); } void SemanticsVisitor::importModuleIntoScope(Scope* scope, ModuleDecl* moduleDecl) diff --git a/source/slang/slang-check-expr.cpp b/source/slang/slang-check-expr.cpp index f1c1b9a43..e7db665ae 100644 --- a/source/slang/slang-check-expr.cpp +++ b/source/slang/slang-check-expr.cpp @@ -177,7 +177,7 @@ namespace Slang { // Compute the type that this declaration reference will have in context. // - auto type = GetTypeForDeclRef(declRef); + auto type = GetTypeForDeclRef(declRef, loc); // Construct an appropriate expression based on the structured of // the declaration reference. diff --git a/source/slang/slang-check-impl.h b/source/slang/slang-check-impl.h index 8cb691f4a..e4d49b1e6 100644 --- a/source/slang/slang-check-impl.h +++ b/source/slang/slang-check-impl.h @@ -1256,7 +1256,7 @@ namespace Slang RefPtr CheckInvokeExprWithCheckedOperands(InvokeExpr *expr); // Get the type to use when referencing a declaration - QualType GetTypeForDeclRef(DeclRef declRef); + QualType GetTypeForDeclRef(DeclRef declRef, SourceLoc loc); // // diff --git a/source/slang/slang-diagnostic-defs.h b/source/slang/slang-diagnostic-defs.h index 6d0f01354..cc93ccc3b 100644 --- a/source/slang/slang-diagnostic-defs.h +++ b/source/slang/slang-diagnostic-defs.h @@ -228,7 +228,6 @@ DIAGNOSTIC(30012, Error, noApplicationUnaryOperator, "no overload found for oper DIAGNOSTIC(30012, Error, noOverloadFoundForBinOperatorOnTypes, "no overload found for operator $0 ($1, $2).") DIAGNOSTIC(30013, Error, subscriptNonArray, "no subscript operation found for type '$0'") DIAGNOSTIC(30014, Error, subscriptIndexNonInteger, "index expression must evaluate to int.") -DIAGNOSTIC(30015, Error, undefinedIdentifier, "'$0': undefined identifier.") DIAGNOSTIC(30015, Error, undefinedIdentifier2, "undefined identifier '$0'.") DIAGNOSTIC(30017, Error, componentNotAccessibleFromShader, "component '$0' is not accessible from shader '$1'.") DIAGNOSTIC(30019, Error, typeMismatch, "expected an expression of type '$0', got '$1'") diff --git a/source/slang/slang-lookup.cpp b/source/slang/slang-lookup.cpp index 2a99d8e53..e860d91d1 100644 --- a/source/slang/slang-lookup.cpp +++ b/source/slang/slang-lookup.cpp @@ -223,7 +223,8 @@ void DoMemberLookupImpl( { auto baseType = getTypeForDeclRef( session, - baseDeclRef); + baseDeclRef, + SourceLoc()); return DoMemberLookupImpl( session, name, baseType, request, ioResult, breadcrumbs); diff --git a/source/slang/slang-lookup.h b/source/slang/slang-lookup.h index d1675015d..eaf51f1e5 100644 --- a/source/slang/slang-lookup.h +++ b/source/slang/slang-lookup.h @@ -48,11 +48,13 @@ QualType getTypeForDeclRef( SemanticsVisitor* sema, DiagnosticSink* sink, DeclRef declRef, - RefPtr* outTypeResult); + RefPtr* outTypeResult, + SourceLoc loc); QualType getTypeForDeclRef( Session* session, - DeclRef declRef); + DeclRef declRef, + SourceLoc loc); /// Add a found item to a lookup result void AddToLookupResult( -- cgit v1.2.3