Fix visibility of synthesized Differential typedefs. (#7865)

* Fix visibility of synthesized `Differential` typedefs.

* Delete incorrect test.

4 files changed, 48 insertions, 37 deletions

diff --git a/source/slang/slang-check-decl.cpp b/source/slang/slang-check-decl.cpp
index 806912abe..cebe1bb00 100644
--- a/source/slang/slang-check-decl.cpp
+++ b/source/slang/slang-check-decl.cpp
@@ -2988,6 +2988,8 @@ bool SemanticsVisitor::trySynthesizeDifferentialAssociatedTypeRequirementWitness
         assocTypeDef->type.type = context->conformingType;
         context->parentDecl->addMember(assocTypeDef);
         assocTypeDef->setCheckState(DeclCheckState::DefinitionChecked);
+        auto visibility = getDeclVisibility(context->parentDecl);
+        addVisibilityModifier(assocTypeDef, visibility);
 
         markSelfDifferentialMembersOfType(
             as<AggTypeDecl>(context->parentDecl),
@@ -3022,6 +3024,10 @@ bool SemanticsVisitor::trySynthesizeDifferentialAssociatedTypeRequirementWitness
         aggTypeDecl->nameAndLoc.name = requirementDeclRef.getName();
         aggTypeDecl->loc = context->parentDecl->nameAndLoc.loc;
 
+        // The visibility of synthesized decl should be the same of the parent decl.
+        auto thisVisibility = getDeclVisibility(context->parentDecl);
+        addVisibilityModifier(aggTypeDecl, thisVisibility);
+
         context->parentDecl->addDirectMemberDecl(aggTypeDecl);
         synth.pushScopeForContainer(aggTypeDecl);
     }
@@ -3144,7 +3150,8 @@ bool SemanticsVisitor::trySynthesizeDifferentialAssociatedTypeRequirementWitness
         assocTypeDef->nameAndLoc.name = differentialName;
         assocTypeDef->type.type = satisfyingType;
         assocTypeDef->setCheckState(DeclCheckState::DefinitionChecked);
-
+        auto visibility = getDeclVisibility(aggTypeDecl);
+        addVisibilityModifier(assocTypeDef, visibility);
         aggTypeDecl->addDirectMemberDecl(assocTypeDef);
     }
 
@@ -3176,9 +3183,6 @@ bool SemanticsVisitor::trySynthesizeDifferentialAssociatedTypeRequirementWitness
 
     addModifier(aggTypeDecl, m_astBuilder->create<SynthesizedModifier>());
 
-    // The visibility of synthesized decl should be the same of the parent decl.
-    auto thisVisibility = getDeclVisibility(context->parentDecl);
-    addVisibilityModifier(aggTypeDecl, thisVisibility);
 
     // Synthesize the rest of IDifferential method conformances by recursively checking
     // conformance on the synthesized decl.
@@ -7483,25 +7487,17 @@ bool SemanticsVisitor::findWitnessForInterfaceRequirement(
 
         if (doesMemberSatisfyRequirement(member.declRef, requiredMemberDeclRef, witnessTable))
         {
-            // The member satisfies the requirement in every other way except that
-            // it may have a lower visibility than min(parentVisibility, requirementVisibilty),
-            // in that case we will treat it as an error.
-            auto minRequiredVisibility = Math::Min(
-                getDeclVisibility(requiredMemberDeclRef.getDecl()),
-                getTypeVisibility(subType));
-            if (getDeclVisibility(member.declRef.getDecl()) < minRequiredVisibility)
-            {
-                getSink()->diagnose(
-                    member.declRef,
-                    Diagnostics::satisfyingDeclCannotHaveLowerVisibility,
-                    member.declRef);
-                getSink()->diagnose(
-                    requiredMemberDeclRef,
-                    Diagnostics::seeDeclarationOf,
-                    QualifiedDeclPath(requiredMemberDeclRef));
-                return false;
-            }
+            // The member satisfies the requirement, so we should add an `IsOverriding`
+            // modifier to the decl, to enable us to verify if a method with `override` keyword
+            // is actually overriding something.
             markOverridingDecl(context, member.declRef.getDecl(), requiredMemberDeclRef);
+
+            // Note: we do not impose any additional requirement on the visibility of the member.
+            // Specifically, it is valid for a method with lower visibility to implement an
+            // interface requirement that has higher visibility. It simply means the method is
+            // only accessible externally from the interface and not directly from the concrete
+            // type.
+            //
             return true;
         }
     }
diff --git a/tests/autodiff/differential-visibility.slang b/tests/autodiff/differential-visibility.slang
new file mode 100644
index 000000000..368155678
--- /dev/null
+++ b/tests/autodiff/differential-visibility.slang
@@ -0,0 +1,26 @@
+//TEST:SIMPLE(filecheck=CHECK): -target spirv
+// CHECK: OpEntryPoint 
+public interface IBSDF
+{
+    public float3 get_value();
+}
+
+public struct AnisotropicGGX // : IDifferentiable
+{
+    float2 alpha = {};
+    public __init() { }
+};
+
+public struct TestDiffVisibility : IBSDF, IDifferentiable
+{
+    public AnisotropicGGX D = {};
+    public __init() { }
+    public float3 get_value() { return { 0.3 }; }
+};
+
+[numthreads(1,1,1)]
+void main()
+{
+    TestDiffVisibility test = {};
+    test.get_value();
+}
diff --git a/tests/diagnostics/internal-visibility/interface-default-visibility.slang b/tests/diagnostics/internal-visibility/interface-default-visibility.slang
deleted file mode 100644
index 79df14143..000000000
--- a/tests/diagnostics/internal-visibility/interface-default-visibility.slang
+++ /dev/null
@@ -1,14 +0,0 @@
-//DIAGNOSTIC_TEST:SIMPLE(filecheck=CHECK):
-
-module m;
-
-public interface IFoo
-{
-    void foo(); // Should have public visibility by default.
-}
-
-public struct F : IFoo
-{
-    // CHECK:{{.*}}(13): error 30602:
-    void foo() {};
-}
diff --git a/tools/slang-test/slang-test-main.cpp b/tools/slang-test/slang-test-main.cpp
index 6355426f7..54ec1a842 100644
--- a/tools/slang-test/slang-test-main.cpp
+++ b/tools/slang-test/slang-test-main.cpp
@@ -657,7 +657,10 @@ static void SLANG_STDCALL _fileCheckDiagnosticCallback(
     auto& testReporter = *reinterpret_cast<TestReporter*>(data);
     testReporter.message(messageType, message);
 }
-
+struct bool2
+{
+    bool x, y;
+};
 //
 // Check some generated output with FileCheck
 //