Expand supported val modifiers for @public (#2365) (#2367)

(cherry picked from commit 13fb4595b3b21beadaad5ba5c5d131138099d6e1)

Co-authored-by: Adam Izraelevitz <adam.izraelevitz@sifive.com>

2 files changed, 41 insertions, 15 deletions

diff --git a/macros/src/main/scala/chisel3/internal/InstantiableMacro.scala b/macros/src/main/scala/chisel3/internal/InstantiableMacro.scala
index d66b51ac..a001b284 100644
--- a/macros/src/main/scala/chisel3/internal/InstantiableMacro.scala
+++ b/macros/src/main/scala/chisel3/internal/InstantiableMacro.scala
@@ -16,21 +16,25 @@ private[chisel3] object instantiableMacro {
       // Note the triple `_` prefixing `module` is to avoid conflicts if a user marks a 'val module'
       //  with @public; in this case, the lookup code is ambiguous between the generated `def module`
       //  function and the argument to the generated implicit class.
-      val resultStats = stats.flatMap {
-        case x @ q"@public val $tpname: $tpe = $name" if tpname.toString() == name.toString() =>
-          extensions += atPos(x.pos)(q"def $tpname = ___module._lookup(_.$tpname)")
-          Nil
-        case x @ q"@public val $tpname: $tpe = $_" =>
-          extensions += atPos(x.pos)(q"def $tpname = ___module._lookup(_.$tpname)")
-          Seq(x)
-        case x @ q"@public val $tpname: $tpe" =>
-          extensions += atPos(x.pos)(q"def $tpname = ___module._lookup(_.$tpname)")
-          Seq(x)
-        case x @ q"@public lazy val $tpname: $tpe = $_" =>
-          extensions += atPos(x.pos)(q"def $tpname = ___module._lookup(_.$tpname)")
-          Seq(x)
-        case other =>
-          Seq(other)
+      val resultStats = stats.flatMap { stat =>
+        stat match {
+          case hasPublic: ValOrDefDef if hasPublic.mods.annotations.toString.contains("new public()") =>
+            hasPublic match {
+              case aDef: DefDef =>
+                c.error(aDef.pos, s"Cannot mark a def as @public")
+                Nil
+              // For now, we only omit protected/private vals
+              case aVal: ValDef
+                  if aVal.mods.hasFlag(c.universe.Flag.PRIVATE) || aVal.mods.hasFlag(c.universe.Flag.PROTECTED) =>
+                c.error(aVal.pos, s"Cannot mark a private or protected val as @public")
+                Nil
+              case aVal: ValDef =>
+                extensions += atPos(aVal.pos)(q"def ${aVal.name} = ___module._lookup(_.${aVal.name})")
+                if (aVal.name.toString == aVal.children.last.toString) Nil else Seq(aVal)
+              case other => Seq(other)
+            }
+          case other => Seq(other)
+        }
       }
       (resultStats, extensions)
     }
diff --git a/src/test/scala/chiselTests/experimental/hierarchy/InstanceSpec.scala b/src/test/scala/chiselTests/experimental/hierarchy/InstanceSpec.scala
index f62d1e49..45d1f85f 100644
--- a/src/test/scala/chiselTests/experimental/hierarchy/InstanceSpec.scala
+++ b/src/test/scala/chiselTests/experimental/hierarchy/InstanceSpec.scala
@@ -298,6 +298,28 @@ class InstanceSpec extends ChiselFunSpec with Utils {
       annos should contain(MarkAnnotation("~Top|Top/i:HasEither>x".rt, "xright"))
       annos should contain(MarkAnnotation("~Top|Top/i:HasEither>y".rt, "yleft"))
     }
+    it("3.12: should properly support val modifiers") {
+      class SupClass extends Module {
+        val value = 10
+        val overriddenVal = 10
+      }
+      trait SupTrait {
+        def x: Int
+        def y: Int
+      }
+      @instantiable class SubClass() extends SupClass with SupTrait {
+        // This errors
+        //@public private val privateVal = 10
+        // This errors
+        //@public protected val protectedVal = 10
+        @public override val overriddenVal = 12
+        @public final val finalVal = 12
+        @public lazy val lazyValue = 12
+        @public val value = value
+        @public final override lazy val x: Int = 3
+        @public override final lazy val y: Int = 4
+      }
+    }
   }
   describe("4: toInstance") {
     it("4.0: should work on modules") {