Improve checks and diagnostics around redeclarations (#1201)

* Improve checks and diagnostics around redeclarations

This change turns checking for redeclarations into a dedicated phase of semantic checking, and ensures that it applies to the main categories of declarations: functions, types, and variables.
Note that "variables" here includes function parameters and `struct` fields in addition to the more obvious global and local variables.

Some of the logic for checking redeclarations already existed for functions, and was refactored to deal with other cases of declarations. The checking for functions still needs to be special-cased because functions are much more flexible about the kinds of redeclarations that are allowed.

In addition to improving the diagnosis of redeclaration itself, this change also changes the error message that is produced when referencing a symbol that is ambiguous due to begin redeclared.
This is a small quality-of-life fix, and has the benefit of being much easier to implement than robust tracking of what variables have had redeclaration errors issued so that we can skip emitting an ambiguity error at the use site.

A new test case was added to cover the redeclaration cases for variables (but not types or functions), and the test for function parameters was updated to account for the new more universal diagnostic message (since function parameters used to have special-case redeclaration checking).

* fixup: missing file

11 files changed, 441 insertions, 207 deletions

diff --git a/source/slang/core.meta.slang b/source/slang/core.meta.slang
index 58e6e287c..85eb82576 100644
--- a/source/slang/core.meta.slang
+++ b/source/slang/core.meta.slang
@@ -359,7 +359,12 @@ for (int N = 2; N <= 4; ++N)
         sb << ", vector<T," << K << "> ";
         for (int ii = 0; ii < K; ++ii)
         {
-            sb << kComponentNames[ii];
+            // The component names for the second parameter
+            // must start at `M`, so that we get signatures like:
+            //
+            //      __init(float2 xy, float2 zw)
+            //
+            sb << kComponentNames[M + ii];
         }
         sb << ");\n";
     }
diff --git a/source/slang/core.meta.slang.h b/source/slang/core.meta.slang.h
index 201429700..b8d7d5d9c 100644
--- a/source/slang/core.meta.slang.h
+++ b/source/slang/core.meta.slang.h
@@ -380,7 +380,12 @@ for (int N = 2; N <= 4; ++N)
         sb << ", vector<T," << K << "> ";
         for (int ii = 0; ii < K; ++ii)
         {
-            sb << kComponentNames[ii];
+            // The component names for the second parameter
+            // must start at `M`, so that we get signatures like:
+            //
+            //      __init(float2 xy, float2 zw)
+            //
+            sb << kComponentNames[M + ii];
         }
         sb << ");\n";
     }
@@ -1277,7 +1282,7 @@ for (auto op : binaryOps)
         sb << "__intrinsic_op(" << int(op.opCode) << ") matrix<" << resultType << ",N,M> operator" << op.opName << "(" << leftQual << "matrix<" << leftType << ",N,M> left, " << rightType << " right);\n";
     }
 }
-SLANG_RAW("#line 1259 \"core.meta.slang\"")
+SLANG_RAW("#line 1264 \"core.meta.slang\"")
 SLANG_RAW("\n")
 SLANG_RAW("\n")
 SLANG_RAW("// Specialized function\n")
diff --git a/source/slang/slang-check-conversion.cpp b/source/slang/slang-check-conversion.cpp
index 85c508d41..4b781643e 100644
--- a/source/slang/slang-check-conversion.cpp
+++ b/source/slang/slang-check-conversion.cpp
@@ -461,7 +461,19 @@ namespace Slang
     {
         if(outToExpr)
         {
-            getSink()->diagnose(fromExpr->loc, Diagnostics::typeMismatch, toType, fromExpr->type);
+            // As a special case, if the expression we are trying to convert
+            // from is overloaded (implying an ambiguous reference), then we
+            // will try to produce a more appropriately tailored error message.
+            //
+            auto fromType = fromExpr->type.type;
+            if( as<OverloadGroupType>(fromType) )
+            {
+                diagnoseAmbiguousReference(fromExpr);
+            }
+            else
+            {
+                getSink()->diagnose(fromExpr->loc, Diagnostics::typeMismatch, toType, fromExpr->type);
+            }
         }
         return false;
     }
diff --git a/source/slang/slang-check-decl.cpp b/source/slang/slang-check-decl.cpp
index 7c8322fa7..dbc96f9b1 100644
--- a/source/slang/slang-check-decl.cpp
+++ b/source/slang/slang-check-decl.cpp
@@ -81,6 +81,27 @@ namespace Slang
         void visitAccessorDecl(AccessorDecl* decl);
     };
 
+    struct SemanticsDeclRedeclarationVisitor
+        : public SemanticsDeclVisitorBase
+        , public DeclVisitor<SemanticsDeclRedeclarationVisitor>
+    {
+        SemanticsDeclRedeclarationVisitor(SharedSemanticsContext* shared)
+            : SemanticsDeclVisitorBase(shared)
+        {}
+
+        void visitDecl(Decl*) {}
+        void visitDeclGroup(DeclGroup*) {}
+
+#define CASE(TYPE) void visit##TYPE(TYPE* decl) { checkForRedeclaration(decl); }
+
+        CASE(FuncDecl)
+        CASE(VarDeclBase)
+        CASE(SimpleTypeDecl)
+        CASE(AggTypeDecl)
+
+#undef CASE
+    };
+
     struct SemanticsDeclBasesVisitor
         : public SemanticsDeclVisitorBase
         , public DeclVisitor<SemanticsDeclBasesVisitor>
@@ -2179,214 +2200,310 @@ namespace Slang
         return subst;
     }
 
-    void SemanticsVisitor::ValidateFunctionRedeclaration(FuncDecl* funcDecl)
+    Result SemanticsVisitor::checkFuncRedeclaration(
+        FuncDecl* newDecl,
+        FuncDecl* oldDecl)
     {
-        auto parentDecl = funcDecl->ParentDecl;
-        SLANG_ASSERT(parentDecl);
-        if (!parentDecl) return;
+        // There are a few different cases that this function needs
+        // to check for:
+        //
+        // * If `newDecl` and `oldDecl` have different signatures such
+        //   that they can always be distinguished at call sites, then
+        //   they don't conflict and don't count as redeclarations.
+        //
+        // * If `newDecl` and `oldDecl` have matching signatures, but
+        //   differ in return type (or other details that would affect
+        //   compatibility), then the declarations conflict and an
+        //   error needs to be diagnosed.
+        //
+        // * If `newDecl` and `oldDecl` have matching/compatible sigantures,
+        //   but differ when it comes to target-specific overloading,
+        //   then they can co-exist.
+        //
+        // * If `newDecl` and `oldDecl` have matching/compatible signatures
+        //   and are specialized for the same target(s), then only
+        //   one can have a body (in which case the other is a forward declaration),
+        //   or else we have a redefinition error.
+
+        auto newGenericDecl = as<GenericDecl>(newDecl->ParentDecl);
+        auto oldGenericDecl = as<GenericDecl>(oldDecl->ParentDecl);
+
+        // If one declaration is a prefix/postfix operator, and the
+        // other is not a matching operator, then don't consider these
+        // to be re-declarations.
+        //
+        // Note(tfoley): Any attempt to call such an operator using
+        // ordinary function-call syntax (if we decided to allow it)
+        // would be ambiguous in such a case, of course.
+        //
+        if (newDecl->HasModifier<PrefixModifier>() != oldDecl->HasModifier<PrefixModifier>())
+            return SLANG_OK;
+        if (newDecl->HasModifier<PostfixModifier>() != oldDecl->HasModifier<PostfixModifier>())
+            return SLANG_OK;
 
-        Decl* childDecl = funcDecl;
+        // If one is generic and the other isn't, then there is no match.
+        if ((newGenericDecl != nullptr) != (oldGenericDecl != nullptr))
+            return SLANG_OK;
 
-        // If this is a generic function (that is, its parent
-        // declaration is a generic), then we need to look
-        // for sibling declarations of the parent.
-        auto genericDecl = as<GenericDecl>(parentDecl);
-        if (genericDecl)
+        // We are going to be comparing the signatures of the
+        // two functions, but if they are *generic* functions
+        // then we will need to compare them with consistent
+        // specializations in place.
+        //
+        // We'll go ahead and create some (unspecialized) declaration
+        // references here, just to be prepared.
+        //
+        DeclRef<FuncDecl> newDeclRef(newDecl, nullptr);
+        DeclRef<FuncDecl> oldDeclRef(oldDecl, nullptr);
+
+        // If we are working with generic functions, then we need to
+        // consider if their generic signatures match.
+        if(newGenericDecl)
         {
-            parentDecl = genericDecl->ParentDecl;
-            childDecl = genericDecl;
+            SLANG_ASSERT(oldGenericDecl); // already checked above
+            if(!doGenericSignaturesMatch(newGenericDecl, oldGenericDecl))
+                return SLANG_OK;
+
+            // Now we need specialize the declaration references
+            // consistently, so that we can compare.
+            //
+            // First we create a "dummy" set of substitutions that
+            // just reference the parameters of the first generic.
+            //
+            auto subst = createDummySubstitutions(newGenericDecl);
+            //
+            // Then we use those parameters to specialize the *other*
+            // generic.
+            //
+            subst->genericDecl = oldGenericDecl;
+            oldDeclRef.substitutions.substitutions = subst;
+            //
+            // One way to think about it is that if we have these
+            // declarations (ignore the name differences...):
+            //
+            //     // oldDecl:
+            //     void foo1<T>(T x);
+            //
+            //     // newDecl:
+            //     void foo2<U>(U x);
+            //
+            // Then we will compare `foo2` against `foo1<U>`.
         }
 
-        // Look at previously-declared functions with the same name,
-        // in the same container
+        // If the parameter signatures don't match, then don't worry
+        if (!doFunctionSignaturesMatch(newDeclRef, oldDeclRef))
+            return SLANG_OK;
+
+        // If we get this far, then we've got two declarations in the same
+        // scope, with the same name and signature, so they appear
+        // to be redeclarations.
         //
-        // Note: there is an assumption here that declarations that
-        // occur earlier in the program  text will be *later* in
-        // the linked list of declarations with the same name.
-        // We are also assuming/requiring that the check here is
-        // symmetric, in that it is okay to test (A,B) or (B,A),
-        // and there is no need to test both.
+        // We will track that redeclaration occured, so that we can
+        // take it into account for overload resolution.
         //
-        buildMemberDictionary(parentDecl);
-        for (auto pp = childDecl->nextInContainerWithSameName; pp; pp = pp->nextInContainerWithSameName)
+        // A huge complication that we'll need to deal with is that
+        // multiple declarations might introduce default values for
+        // (different) parameters, and we might need to merge across
+        // all of them (which could get complicated if defaults for
+        // parameters can reference earlier parameters).
+
+        // If the previous declaration wasn't already recorded
+        // as being part of a redeclaration family, then make
+        // it the primary declaration of a new family.
+        if (!oldDecl->primaryDecl)
         {
-            auto prevDecl = pp;
+            oldDecl->primaryDecl = oldDecl;
+        }
 
-            // Look through generics to the declaration underneath
-            auto prevGenericDecl = as<GenericDecl>(prevDecl);
-            if (prevGenericDecl)
-                prevDecl = prevGenericDecl->inner.Ptr();
+        // The new declaration will belong to the family of
+        // the previous one, and so it will share the same
+        // primary declaration.
+        newDecl->primaryDecl = oldDecl->primaryDecl;
+        newDecl->nextDecl = nullptr;
 
-            // We only care about previously-declared functions
-            // Note(tfoley): although we should really error out if the
-            // name is already in use for something else, like a variable...
-            auto prevFuncDecl = as<FuncDecl>(prevDecl);
-            if (!prevFuncDecl)
-                continue;
+        // Next we want to chain the new declaration onto
+        // the linked list of redeclarations.
+        auto link = &oldDecl->nextDecl;
+        while (*link)
+            link = &(*link)->nextDecl;
+        *link = newDecl;
 
-            // If one declaration is a prefix/postfix operator, and the
-            // other is not a matching operator, then don't consider these
-            // to be re-declarations.
-            //
-            // Note(tfoley): Any attempt to call such an operator using
-            // ordinary function-call syntax (if we decided to allow it)
-            // would be ambiguous in such a case, of course.
-            //
-            if (funcDecl->HasModifier<PrefixModifier>() != prevDecl->HasModifier<PrefixModifier>())
-                continue;
-            if (funcDecl->HasModifier<PostfixModifier>() != prevDecl->HasModifier<PostfixModifier>())
-                continue;
+        // Now that we've added things to a group of redeclarations,
+        // we can do some additional validation.
 
-            // If one is generic and the other isn't, then there is no match.
-            if ((genericDecl != nullptr) != (prevGenericDecl != nullptr))
-                continue;
+        // First, we will ensure that the return types match
+        // between the declarations, so that they are truly
+        // interchangeable.
+        //
+        // Note(tfoley): If we ever decide to add a beefier type
+        // system to Slang, we might allow overloads like this,
+        // so long as the desired result type can be disambiguated
+        // based on context at the call type. In that case we would
+        // consider result types earlier, as part of the signature
+        // matching step.
+        //
+        auto resultType = GetResultType(newDeclRef);
+        auto prevResultType = GetResultType(oldDeclRef);
+        if (!resultType->Equals(prevResultType))
+        {
+            // Bad redeclaration
+            getSink()->diagnose(newDecl, Diagnostics::functionRedeclarationWithDifferentReturnType, newDecl->getName(), resultType, prevResultType);
+            getSink()->diagnose(oldDecl, Diagnostics::seePreviousDeclarationOf, newDecl->getName());
 
-            // We are going to be comparing the signatures of the
-            // two functions, but if they are *generic* functions
-            // then we will need to compare them with consistent
-            // specializations in place.
-            //
-            // We'll go ahead and create some (unspecialized) declaration
-            // references here, just to be prepared.
-            DeclRef<FuncDecl> funcDeclRef(funcDecl, nullptr);
-            DeclRef<FuncDecl> prevFuncDeclRef(prevFuncDecl, nullptr);
-
-            // If we are working with generic functions, then we need to
-            // consider if their generic signatures match.
-            if (genericDecl)
-            {
-                SLANG_ASSERT(prevGenericDecl); // already checked above
-                if (!doGenericSignaturesMatch(genericDecl, prevGenericDecl))
-                    continue;
+            // Don't bother emitting other errors at this point
+            return SLANG_FAIL;
+        }
 
-                // Now we need specialize the declaration references
-                // consistently, so that we can compare.
-                //
-                // First we create a "dummy" set of substitutions that
-                // just reference the parameters of the first generic.
-                auto subst = createDummySubstitutions(genericDecl);
-                //
-                // Then we use those parameters to specialize the *other*
-                // generic.
-                //
-                subst->genericDecl = prevGenericDecl;
-                prevFuncDeclRef.substitutions.substitutions = subst;
-                //
-                // One way to think about it is that if we have these
-                // declarations (ignore the name differences...):
-                //
-                //     // prevFuncDecl:
-                //     void foo1<T>(T x);
-                //
-                //     // funcDecl:
-                //     void foo2<U>(U x);
-                //
-                // Then we will compare `foo2` against `foo1<U>`.
-            }
+        // TODO: Enforce that the new declaration had better
+        // not specify a default value for any parameter that
+        // already had a default value in a prior declaration.
 
-            // If the parameter signatures don't match, then don't worry
-            if (!doFunctionSignaturesMatch(funcDeclRef, prevFuncDeclRef))
-                continue;
+        // We are going to want to enforce that we cannot have
+        // two declarations of a function both specify bodies.
+        // Before we make that check, however, we need to deal
+        // with the case where the two function declarations
+        // might represent different target-specific versions
+        // of a function.
+        //
+        // TODO: if the two declarations are specialized for
+        // different targets, then skip the body checks below.
+        //
+        // ???: Why isn't this problem showing up in practice?
 
-            // If we get this far, then we've got two declarations in the same
-            // scope, with the same name and signature, so they appear
-            // to be redeclarations.
-            //
-            // We will track that redeclaration occured, so that we can
-            // take it into account for overload resolution.
-            //
-            // A huge complication that we'll need to deal with is that
-            // multiple declarations might introduce default values for
-            // (different) parameters, and we might need to merge across
-            // all of them (which could get complicated if defaults for
-            // parameters can reference earlier parameters).
-
-            // If the previous declaration wasn't already recorded
-            // as being part of a redeclaration family, then make
-            // it the primary declaration of a new family.
-            if (!prevFuncDecl->primaryDecl)
+        // If both of the declarations have a body, then there
+        // is trouble, because we wouldn't know which one to
+        // use during code generation.
+        if (newDecl->Body && oldDecl->Body)
+        {
+            // Redefinition
+            getSink()->diagnose(newDecl, Diagnostics::functionRedefinition, newDecl->getName());
+            getSink()->diagnose(oldDecl, Diagnostics::seePreviousDefinitionOf, newDecl->getName());
+
+            // Don't bother emitting other errors
+            return SLANG_FAIL;
+        }
+
+        // At this point we've processed the redeclaration and
+        // put it into a group, so there is no reason to keep
+        // looping and looking at prior declarations.
+        //
+        // While no diagnostics have been emitted, we return
+        // a failure result from the operation to indicate
+        // to the caller that they should stop looping over
+        // declarations at this point.
+        //
+        return SLANG_FAIL;
+    }
+
+    Result SemanticsVisitor::checkRedeclaration(Decl* newDecl, Decl* oldDecl)
+    {
+        // If either of the declarations being looked at is generic, then
+        // we want to consider the "inner" declaration instead when
+        // making decisions about what to allow or not.
+        //
+        if(auto newGenericDecl = as<GenericDecl>(newDecl))
+            newDecl = newGenericDecl->inner;
+        if(auto oldGenericDecl = as<GenericDecl>(oldDecl))
+            oldDecl = oldGenericDecl->inner;
+
+        // Functions are special in that we can have many declarations
+        // with the same name in a given scope, and it is possible
+        // for them to co-exist as overloads, or even just be multiple
+        // declarations of the same function (thanks to the inherited
+        // legacy of C forward declarations).
+        //
+        // If both declarations are functions, we will check that
+        // they are allowed to co-exist using these more nuanced rules.
+        //
+        if( auto newFuncDecl = as<FuncDecl>(newDecl) )
+        {
+            if(auto oldFuncDecl = as<FuncDecl>(oldDecl) )
             {
-                prevFuncDecl->primaryDecl = prevFuncDecl;
+                // Both new and old declarations are functions,
+                // so redeclaration may be valid.
+                return checkFuncRedeclaration(newFuncDecl, oldFuncDecl);
             }
+        }
 
-            // The new declaration will belong to the family of
-            // the previous one, and so it will share the same
-            // primary declaration.
-            funcDecl->primaryDecl = prevFuncDecl->primaryDecl;
-            funcDecl->nextDecl = nullptr;
-
-            // Next we want to chain the new declaration onto
-            // the linked list of redeclarations.
-            auto link = &prevFuncDecl->nextDecl;
-            while (*link)
-                link = &(*link)->nextDecl;
-            *link = funcDecl;
-
-            // Now that we've added things to a group of redeclarations,
-            // we can do some additional validation.
-
-            // First, we will ensure that the return types match
-            // between the declarations, so that they are truly
-            // interchangeable.
-            //
-            // Note(tfoley): If we ever decide to add a beefier type
-            // system to Slang, we might allow overloads like this,
-            // so long as the desired result type can be disambiguated
-            // based on context at the call type. In that case we would
-            // consider result types earlier, as part of the signature
-            // matching step.
+        // For all other flavors of declaration, we do not
+        // allow duplicate declarations with the same name.
+        //
+        // TODO: We might consider allowing some other cases
+        // of overloading that can be safely disambiguated:
+        //
+        // * A type and a value (function/variable/etc.) of the same name can usually
+        //   co-exist because we can distinguish which is needed by context.
+        //
+        // * Multiple generic types with the same name can co-exist
+        //   if their generic parameter lists are sufficient to
+        //   tell them apart at a use site.
+
+        // We will diagnose a redeclaration error at the new declaration,
+        // and point to the old declaration for context.
+        //
+        getSink()->diagnose(newDecl, Diagnostics::redeclaration, newDecl->getName());
+        getSink()->diagnose(oldDecl, Diagnostics::seePreviousDeclarationOf, oldDecl->getName());
+        return SLANG_FAIL;
+    }
+
+
+    void SemanticsVisitor::checkForRedeclaration(Decl* decl)
+    {
+        // We want to consider a "new" declaration in the context
+        // of some parent/container declaration, and compare it
+        // to pre-existing "old" declarations of the same name
+        // in the same container.
+        //
+        auto newDecl = decl;
+        auto parentDecl = decl->ParentDecl;
+
+        // Sanity check: there should always be a parent declaration.
+        //
+        SLANG_ASSERT(parentDecl);
+        if (!parentDecl) return;
+
+        // If the declaration is the "inner" declaration of a generic,
+        // then we actually want to look one level up, because the
+        // peers/siblings of the declaration will belong to the same
+        // parent as the generic, not to the generic.
+        //
+        if( auto genericParentDecl = as<GenericDecl>(parentDecl) )
+        {
+            // Note: we need to check here to be sure `newDecl`
+            // is the "inner" declaration and not one of the
+            // generic parameters, or else we will end up
+            // checking them at the wrong scope.
             //
-            auto resultType = GetResultType(funcDeclRef);
-            auto prevResultType = GetResultType(prevFuncDeclRef);
-            if (!resultType->Equals(prevResultType))
+            if( newDecl == genericParentDecl->inner )
             {
-                // Bad redeclaration
-                getSink()->diagnose(funcDecl, Diagnostics::functionRedeclarationWithDifferentReturnType, funcDecl->getName(), resultType, prevResultType);
-                getSink()->diagnose(prevFuncDecl, Diagnostics::seePreviousDeclarationOf, funcDecl->getName());
-
-                // Don't bother emitting other errors at this point
-                break;
+                newDecl = parentDecl;
+                parentDecl = genericParentDecl->ParentDecl;
             }
+        }
 
-            // Note(tfoley): several of the following checks should
-            // really be looping over all the previous declarations
-            // in the same group, and not just the one previous
-            // declaration we found just now.
-
-            // TODO: Enforce that the new declaration had better
-            // not specify a default value for any parameter that
-            // already had a default value in a prior declaration.
-
-            // We are going to want to enforce that we cannot have
-            // two declarations of a function both specify bodies.
-            // Before we make that check, however, we need to deal
-            // with the case where the two function declarations
-            // might represent different target-specific versions
-            // of a function.
+        // We will now look for other declarations with
+        // the same name in the same parent/container.
+        //
+        buildMemberDictionary(parentDecl);
+        for (auto oldDecl = newDecl->nextInContainerWithSameName; oldDecl; oldDecl = oldDecl->nextInContainerWithSameName)
+        {
+            // For each matching declaration, we will check
+            // whether the redeclaration should be allowed,
+            // and emit an appropriate diagnostic if not.
             //
-            // TODO: if the two declarations are specialized for
-            // different targets, then skip the body checks below.
-
-            // If both of the declarations have a body, then there
-            // is trouble, because we wouldn't know which one to
-            // use during code generation.
-            if (funcDecl->Body && prevFuncDecl->Body)
-            {
-                // Redefinition
-                getSink()->diagnose(funcDecl, Diagnostics::functionRedefinition, funcDecl->getName());
-                getSink()->diagnose(prevFuncDecl, Diagnostics::seePreviousDefinitionOf, funcDecl->getName());
+            Result checkResult = checkRedeclaration(newDecl, oldDecl);
 
-                // Don't bother emitting other errors
+            // The `checkRedeclaration` function will return a failure
+            // status (whether or not it actually emitted a diagnostic)
+            // if we should stop checking further redeclarations, because
+            // the declaration in question has been dealt with fully.
+            //
+            if(SLANG_FAILED(checkResult))
                 break;
-            }
-
-            // At this point we've processed the redeclaration and
-            // put it into a group, so there is no reason to keep
-            // looping and looking at prior declarations.
-            return;
         }
     }
 
+
     void SemanticsDeclHeaderVisitor::visitParamDecl(ParamDecl* paramDecl)
     {
         // TODO: This logic should be shared with the other cases of
@@ -2456,22 +2573,10 @@ namespace Slang
         }
         funcDecl->ReturnType = resultType;
 
-
-        HashSet<Name*> paraNames;
         for (auto & para : funcDecl->GetParameters())
         {
             ensureDecl(para, DeclCheckState::ReadyForReference);
-
-            if (paraNames.Contains(para->getName()))
-            {
-                getSink()->diagnose(para, Diagnostics::parameterAlreadyDefined, para->getName());
-            }
-            else
-                paraNames.Add(para->getName());
         }
-
-        // One last bit of validation: check if we are redeclaring an existing function
-        ValidateFunctionRedeclaration(funcDecl);
     }
 
     IntegerLiteralValue SemanticsVisitor::GetMinBound(RefPtr<IntVal> val)
@@ -2936,10 +3041,14 @@ namespace Slang
             SemanticsDeclModifiersVisitor(shared).dispatch(decl);
             break;
 
-        case DeclCheckState::ReadyForReference:
+        case DeclCheckState::SignatureChecked:
             SemanticsDeclHeaderVisitor(shared).dispatch(decl);
             break;
 
+        case DeclCheckState::ReadyForReference:
+            SemanticsDeclRedeclarationVisitor(shared).dispatch(decl);
+            break;
+
         case DeclCheckState::ReadyForLookup:
             SemanticsDeclBasesVisitor(shared).dispatch(decl);
             break;
diff --git a/source/slang/slang-check-expr.cpp b/source/slang/slang-check-expr.cpp
index e7db665ae..ebc3a3ef9 100644
--- a/source/slang/slang-check-expr.cpp
+++ b/source/slang/slang-check-expr.cpp
@@ -435,6 +435,29 @@ namespace Slang
         return result;
     }
 
+    void SemanticsVisitor::diagnoseAmbiguousReference(OverloadedExpr* overloadedExpr, LookupResult const& lookupResult)
+    {
+        getSink()->diagnose(overloadedExpr, Diagnostics::ambiguousReference, lookupResult.items[0].declRef.GetName());
+
+        for(auto item : lookupResult.items)
+        {
+            String declString = getDeclSignatureString(item);
+            getSink()->diagnose(item.declRef, Diagnostics::overloadCandidate, declString);
+        }
+    }
+
+    void SemanticsVisitor::diagnoseAmbiguousReference(Expr* expr)
+    {
+        if( auto overloadedExpr = as<OverloadedExpr>(expr) )
+        {
+            diagnoseAmbiguousReference(overloadedExpr, overloadedExpr->lookupResult2);
+        }
+        else
+        {
+            getSink()->diagnose(expr, Diagnostics::ambiguousExpression);
+        }
+    }
+
     RefPtr<Expr> SemanticsVisitor::_resolveOverloadedExprImpl(RefPtr<OverloadedExpr> overloadedExpr, LookupMask mask, DiagnosticSink* diagSink)
     {
         auto lookupResult = overloadedExpr->lookupResult2;
@@ -474,13 +497,7 @@ namespace Slang
         //
         if( diagSink )
         {
-            getSink()->diagnose(overloadedExpr, Diagnostics::ambiguousReference, lookupResult.items[0].declRef.GetName());
-
-            for(auto item : lookupResult.items)
-            {
-                String declString = getDeclSignatureString(item);
-                getSink()->diagnose(item.declRef, Diagnostics::overloadCandidate, declString);
-            }
+            diagnoseAmbiguousReference(overloadedExpr, lookupResult);
 
             // TODO(tfoley): should we construct a new ErrorExpr here?
             return CreateErrorExpr(overloadedExpr);
diff --git a/source/slang/slang-check-impl.h b/source/slang/slang-check-impl.h
index e4d49b1e6..863b5f38a 100644
--- a/source/slang/slang-check-impl.h
+++ b/source/slang/slang-check-impl.h
@@ -382,6 +382,10 @@ namespace Slang
             /// Worker reoutine for `maybeResolveOverloadedExpr` and `resolveOverloadedExpr`.
         RefPtr<Expr> _resolveOverloadedExprImpl(RefPtr<OverloadedExpr> overloadedExpr, LookupMask mask, DiagnosticSink* diagSink);
 
+        void diagnoseAmbiguousReference(OverloadedExpr* overloadedExpr, LookupResult const& lookupResult);
+        void diagnoseAmbiguousReference(Expr* overloadedExpr);
+
+
         RefPtr<Expr> ExpectATypeRepr(RefPtr<Expr> expr);
 
         RefPtr<Type> ExpectAType(RefPtr<Expr> expr);
@@ -797,7 +801,9 @@ namespace Slang
         RefPtr<GenericSubstitution> createDummySubstitutions(
             GenericDecl* genericDecl);
 
-        void ValidateFunctionRedeclaration(FuncDecl* funcDecl);
+        Result checkRedeclaration(Decl* newDecl, Decl* oldDecl);
+        Result checkFuncRedeclaration(FuncDecl* newDecl, FuncDecl* oldDecl);
+        void checkForRedeclaration(Decl* decl);
 
         RefPtr<Expr> checkPredicateExpr(Expr* expr);
 
diff --git a/source/slang/slang-diagnostic-defs.h b/source/slang/slang-diagnostic-defs.h
index cc93ccc3b..430423464 100644
--- a/source/slang/slang-diagnostic-defs.h
+++ b/source/slang/slang-diagnostic-defs.h
@@ -212,7 +212,6 @@ DIAGNOSTIC(20011, Error, unexpectedColon, "unexpected ':'.")
 // 3xxxx - Semantic analysis
 //
 
-DIAGNOSTIC(30002, Error, parameterAlreadyDefined, "parameter '$0' already defined.")
 DIAGNOSTIC(30003, Error, breakOutsideLoop, "'break' must appear inside loop constructs.")
 DIAGNOSTIC(30004, Error, continueOutsideLoop, "'continue' must appear inside loop constructs.")
 DIAGNOSTIC(30005, Error, whilePredicateTypeError, "'while': expression must evaluate to int.")
@@ -256,6 +255,7 @@ DIAGNOSTIC(30060, Error, expectedAType, "expected a type got a '$0'")
 
 DIAGNOSTIC(30100, Error, staticRefToNonStaticMember, "type '$0' cannot be used to refer to non-static member '$1'")
 
+DIAGNOSTIC(30200, Error, redeclaration, "declaration of '$0' conflicts with existing declaration")
 DIAGNOSTIC(30201, Error, functionRedefinition, "function '$0' already has a body")
 DIAGNOSTIC(30202, Error, functionRedeclarationWithDifferentReturnType, "function '$0' declared to return '$1' was previously declared to return '$2'")
 
@@ -341,6 +341,7 @@ DIAGNOSTIC(39999, Note, genericSignatureTried, "see declaration of $0")
 DIAGNOSTIC(39999, Error, expectedAnInterfaceGot, "expected an interface, got '$0'")
 
 DIAGNOSTIC(39999, Error, ambiguousReference, "ambiguous reference to '$0'");
+DIAGNOSTIC(39999, Error, ambiguousExpression, "ambiguous reference");
 
 DIAGNOSTIC(39999, Error, declarationDidntDeclareAnything, "declaration does not declare anything");
 
diff --git a/source/slang/slang-syntax.h b/source/slang/slang-syntax.h
index 35c21227c..c5160e47b 100644
--- a/source/slang/slang-syntax.h
+++ b/source/slang/slang-syntax.h
@@ -320,8 +320,7 @@ namespace Slang
             ///
         ModifiersChecked,
 
-            /// The declaration's basic signature has been checked to the point that
-            /// it is ready to be referenced in other places.
+            /// The type/signature of the declaration has been checked.
             ///
             /// For a value declaration like a variable or function, this means that
             /// the type of the declaration can be queried.
@@ -329,6 +328,15 @@ namespace Slang
             /// For a type declaration like a `struct` or `typedef` this means
             /// that a `Type` referring to that declaration can be formed.
             ///
+        SignatureChecked,
+
+            /// The declaration's basic signature has been checked to the point that
+            /// it is ready to be referenced in other places.
+            ///
+            /// For a function, this means that it has been organized into a
+            /// "redeclration group" if there are multiple functions with the
+            /// same name in a scope.
+            ///
         ReadyForReference,
 
             /// The declaration is ready for lookup operations to be performed.
diff --git a/tests/diagnostics/parameter-already-defined.slang.expected b/tests/diagnostics/parameter-already-defined.slang.expected
index ae7546a10..b748b9523 100644
--- a/tests/diagnostics/parameter-already-defined.slang.expected
+++ b/tests/diagnostics/parameter-already-defined.slang.expected
@@ -1,6 +1,7 @@
 result code = -1
 standard error = {
-tests/diagnostics/parameter-already-defined.slang(4): error 30002: parameter 'a' already defined.
+tests/diagnostics/parameter-already-defined.slang(4): error 30200: declaration of 'a' conflicts with existing declaration
+tests/diagnostics/parameter-already-defined.slang(4): note: see previous declaration of 'a'
 }
 standard output = {
 }
diff --git a/tests/diagnostics/variable-redeclaration.slang b/tests/diagnostics/variable-redeclaration.slang
new file mode 100644
index 000000000..bbd6a07c0
--- /dev/null
+++ b/tests/diagnostics/variable-redeclaration.slang
@@ -0,0 +1,54 @@
+// variable-redeclaration.slang
+
+//DIAGNOSTIC_TEST:SIMPLE:
+
+
+// This test confirms that the compiler produces
+// suitable diagnostics when variables are redeclared
+// in a given scope.
+
+// Global variables, including shader parameters
+
+static int gA;
+
+static Texture2D gA;
+
+// Local variables
+
+int testLocalRedeclaration(int x)
+{
+	int y = x;
+	int y = x;
+}
+
+int testLocalShadowing(int x)
+{
+	int y = x;
+	{
+		// Because this declaration is in an inner
+		// scope it should shadow the existing `y`
+		// rather than conflcit with it.
+		//
+		// TODO: It would be reasonable for the
+		// compiler to warn on this sort of code.
+		//
+		int y = x;
+	}
+}
+
+// Structure fields
+
+struct S
+{
+	int f;
+	float f;
+}
+
+// Function parameter list
+
+int testParameterRedeclaration(
+	int 	size,
+	float 	size)
+{
+	return size;
+}
diff --git a/tests/diagnostics/variable-redeclaration.slang.expected b/tests/diagnostics/variable-redeclaration.slang.expected
new file mode 100644
index 000000000..d49c4512f
--- /dev/null
+++ b/tests/diagnostics/variable-redeclaration.slang.expected
@@ -0,0 +1,16 @@
+result code = -1
+standard error = {
+tests/diagnostics/variable-redeclaration.slang(14): error 30200: declaration of 'gA' conflicts with existing declaration
+tests/diagnostics/variable-redeclaration.slang(12): note: see previous declaration of 'gA'
+tests/diagnostics/variable-redeclaration.slang(44): error 30200: declaration of 'f' conflicts with existing declaration
+tests/diagnostics/variable-redeclaration.slang(43): note: see previous declaration of 'f'
+tests/diagnostics/variable-redeclaration.slang(51): error 30200: declaration of 'size' conflicts with existing declaration
+tests/diagnostics/variable-redeclaration.slang(50): note: see previous declaration of 'size'
+tests/diagnostics/variable-redeclaration.slang(21): error 30200: declaration of 'y' conflicts with existing declaration
+tests/diagnostics/variable-redeclaration.slang(20): note: see previous declaration of 'y'
+tests/diagnostics/variable-redeclaration.slang(53): error 39999: ambiguous reference to 'size'
+tests/diagnostics/variable-redeclaration.slang(51): note 39999: candidate: size
+tests/diagnostics/variable-redeclaration.slang(50): note 39999: candidate: size
+}
+standard output = {
+}