[GitHub] [daffodil] stevedlawrence commented on a diff in pull request #1071: Remove support for dfdlx:repType/Values from unions

["stevedlawrence (via GitHub)" <gi...@apache.org>]

stevedlawrence commented on code in PR #1071:
URL: https://github.com/apache/daffodil/pull/1071#discussion_r1308761957


##########
daffodil-core/src/main/scala/org/apache/daffodil/core/dsom/SimpleTypes.scala:
##########
@@ -405,90 +386,39 @@ abstract class SimpleTypeDefBase(xml: Node, lexicalParent: SchemaComponent)
       })
   }.value
 
-  private lazy val optRepTypeFromSelf: Option[SimpleTypeBase with NamedMixin] =
-    LV('optRepTypeFromSelf) {
-      val optRepTypeDef = optRepTypeQName.flatMap(schemaSet.getGlobalSimpleTypeDef(_))
-      val optRepPrimType = optRepTypeQName.flatMap(schemaSet.getPrimitiveType(_))
-      Assert.invariant(!(optRepPrimType.isDefined && optRepTypeDef.isDefined))
-      if (optRepTypeQName.isDefined) {
-        schemaDefinitionUnless(
-          optRepTypeDef.isDefined || optRepPrimType.isDefined,
-          s"Cannot find reptype ${optRepTypeQNameString.get}",
-        )
-      }
-      optRepTypeDef.orElse(optRepPrimType)
-    }.value
-
-  private lazy val optRepTypeFromUnion: Option[SimpleTypeBase with NamedMixin] =
-    LV('optRepTypeFromUnion) {
-      optUnion.flatMap(union => {
-        val repTypes = union.unionMemberTypes.map(_.optRepType)
-        // check that all repTypes are the same
-        // Because of how we inline types, we do not expect to see structural equality,
-        // so we rely on the xml qname instead
-        val numRepTypes = repTypes.map(_.map(_.namedQName)).toSet.size
-        if (numRepTypes > 1) {
-          SDE("If any child type of a union has a repType, they all must have the same repType")
-        }
-        if (numRepTypes == 0) {
-          None
-        } else {
-          repTypes.head
-        }
-      })
-    }.value
-
-  /*
-   * We don't really need the NamedMixin. It is only used for detecting duplicates
-   * However, since only named types can be a repType, there is no problem
-   * in requiring them to be named
-   */
   override lazy val optRepType: Option[SimpleTypeBase with NamedMixin] = LV('optRepType) {
-    /*
-     * Note that there is no fromRestriction option here
-     * In theory, we could consider every restriction type without an explicit repType to be
-     * either a restriction or identity transform.
-     * In practice, this would introduce the overhead of a transform to almost every derived type.
-     * Instead, when a user needs a restriction transform, they must simply provide the reptype explitly,
-     * which is arguably a good design decision from a readability standpoint of the schema as well.
-     */
-    optRepTypeFromSelf.orElse(optRepTypeFromUnion)
+    PrimitiveType

Review Comment:
   This `PrimitiveType` line shouldn't be here. I'm not sure where it came from, maybe a weird copy/paste issue? But it doesn't do anything and should be removed.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org