Add Support for Nested Module Syntax (#2012)

cpp/include/IceUtil/ScannerConfig.h

@@ -9,6 +9,8 @@
 
 #ifdef _MSC_VER
 #    define YY_NO_UNISTD_H
+// Warning C4018: signed/unsigned mismatch
+#    pragma warning(disable : 4018)
 // Warning C4244: conversion from `int` to `_Elem`, possible loss of data
 #    pragma warning(disable : 4244)
 // warning C4267: conversion from 'size_t' to 'int', possible loss of data

cpp/src/Slice/Grammar.y

@@ -385,6 +385,83 @@ module_def
         $$ = nullptr;
     }
 }
+| ICE_MODULE ICE_SCOPED_IDENTIFIER
+{
+    auto ident = dynamic_pointer_cast<StringTok>($2);
+
+    // Reject scoped identifiers starting with "::". This is generally indicates global scope, but is invalid here.
+    size_t startPos = 0;
+    if (ident->v.find("::") == 0)
+    {
+        currentUnit->error("illegal identifier: module identifiers cannot start with '::' prefix");
+        startPos += 2; // Skip the leading "::".
+    }
+
+    // Split the scoped-identifier token into separate module names.
+    size_t endPos;
+    vector<string> modules;
+    while ((endPos = ident->v.find("::", startPos)) != string::npos)
+    {
+        modules.push_back(ident->v.substr(startPos, (endPos - startPos)));
+        startPos = endPos + 2; // Skip the "::" separator.
+    }
+    modules.push_back(ident->v.substr(startPos));
+
+    // Create the nested modules.
+    ContainerPtr cont = currentUnit->currentContainer();
+    for (size_t i = 0; i < modules.size(); i++)
+    {
+        const auto currentModuleName = modules[i];
+        ModulePtr module = cont->createModule(currentModuleName);
+        if (module)
+        {
+            cont->checkIntroduced(currentModuleName, module);
+            currentUnit->pushContainer(module);
+            $$ = cont = module;
+        }
+        else
+        {
+            // If an error occurs while creating one of the modules, we have to stop. But, to eagerly report as many
+            // errors as possible, we still 'create' any remaining modules, which will run _some_ validation on them.
+            for (size_t j = (i + 1); j < modules.size(); j++)
+            {
+                cont->createModule(modules[j]); // Dummy
+            }
+
+            // Then we roll back the chain, ie. pop the successfully-created-modules off the container stack.
+            for (; i > 0; i--)
+            {
+                currentUnit->popContainer();
+            }
+            $$ = nullptr;
+            break;
+        }
+    }
+}
+'{' definitions '}'
+{
+    if ($3)
+    {
+        // We need to pop '(N+1)' modules off the container stack, to navigate out of the nested module.
+        // Where `N` is the number of scope separators ("::").
+        size_t startPos = 0;
+        auto ident = dynamic_pointer_cast<StringTok>($2);
+        while ((startPos = ident->v.find("::", startPos)) != string::npos)
+        {
+            currentUnit->popContainer();
+            startPos += 2; // Skip the "::" separator.
+        }
+
+        // Set the 'return value' to the outer-most module, before we pop it off the stack.
+        // Whichever module we return, is the one that metadata will be applied to.
+        $$ = currentUnit->currentContainer();
+        currentUnit->popContainer();
+    }
+    else
+    {
+        $$ = nullptr;
+    }
+}
 ;
 
 // ----------------------------------------------------------------------

cpp/src/Slice/SliceUtil.cpp

@@ -483,7 +483,9 @@ Slice::checkIdentifier(const string& id)
         name = id;
     }
 
+    assert(!name.empty());
     bool isValid = true;
+
     // check the identifier for reserved suffixes
     static const string suffixBlacklist[] = {"Helper", "Holder", "Prx", "Ptr"};
     for (size_t i = 0; i < sizeof(suffixBlacklist) / sizeof(*suffixBlacklist); ++i)

cpp/test/Ice/ami/Test.ice

@@ -59,10 +59,7 @@ interface TestIntfController
     void resumeAdapter();
 }
 
-module Outer
-{
-
-module Inner
+module Outer::Inner
 {
 
 interface TestIntf
@@ -73,5 +70,3 @@ interface TestIntf
 }
 
 }
-
-}

cpp/test/Ice/scope/Test.ice

@@ -144,13 +144,7 @@ module Test
     }
 }
 
-module Inner
-{
-
-module Test
-{
-
-module Inner2
+module Inner::Test::Inner2
 {
     interface I
     {
@@ -165,7 +159,3 @@ module Inner2
         void shutdown();
     }
 }
-
-}
-
-}

cpp/test/Ice/stream/Test.ice

@@ -124,9 +124,7 @@ module Sub
 }
 }
 
-module Test2
-{
-module Sub2
+module Test2::Sub2
 {
     enum NestedEnum2
     {
@@ -153,4 +151,3 @@ module Sub2
         string str;
     }
 }
-}

cpp/test/Slice/errorDetection/CaseInsensitive.err

@@ -30,22 +30,22 @@ CaseInsensitive.ice:149: enumeration `eN1' differs only in capitalization from e
 CaseInsensitive.ice:150: redefinition of module `m1' as enumeration
 CaseInsensitive.ice:151: enumeration `M1' differs only in capitalization from module `m1'
 CaseInsensitive.ice:152: enumerator `EN1' differs only in capitalization from `en1'
-CaseInsensitive.ice:170: interface name `base' is capitalized inconsistently with its previous name: `::Test::xxx::xx::Base'
-CaseInsensitive.ice:170: redefinition of interface `Derived'
-CaseInsensitive.ice:177: exception name `E1' is capitalized inconsistently with its previous name: `::Test::xxx::xx::e1'
-CaseInsensitive.ice:184: sequence name `S1' is capitalized inconsistently with its previous name: `::Test::xxx::xx::s1'
-CaseInsensitive.ice:185: sequence name `xxx::xx::S1' is capitalized inconsistently with its previous name: `::Test::xxx::xx::s1'
-CaseInsensitive.ice:186: sequence name `xxx::XX::s1' is capitalized inconsistently with its previous name: `::Test::xxx::xx::s1'
-CaseInsensitive.ice:187: sequence name `xxx::XX::s1' is capitalized inconsistently with its previous name: `::Test::xxx::xx::s1'
-CaseInsensitive.ice:193: interface name `derived' is capitalized inconsistently with its previous name: `::Test::xxx::xx::Derived'
-CaseInsensitive.ice:202: parameter `Param' differs only in capitalization from parameter `param'
-CaseInsensitive.ice:202: `Param' has changed meaning
-CaseInsensitive.ice:204: exception name `E1' is capitalized inconsistently with its previous name: `::Test::e1'
-CaseInsensitive.ice:206: exception name `Test::xxx::xx::E1' is capitalized inconsistently with its previous name: `::Test::xxx::xx::e1'
-CaseInsensitive.ice:207: exception name `Test::xxx::XX::e1' is capitalized inconsistently with its previous name: `::Test::xxx::xx::e1'
-CaseInsensitive.ice:208: exception name `Test::XXX::xx::e1' is capitalized inconsistently with its previous name: `::Test::xxx::xx::e1'
-CaseInsensitive.ice:210: exception name `Test::xxx::xx::E1' is capitalized inconsistently with its previous name: `::Test::xxx::xx::e1'
-CaseInsensitive.ice:211: exception name `Test::xxx::XX::e1' is capitalized inconsistently with its previous name: `::Test::xxx::xx::e1'
-CaseInsensitive.ice:212: exception name `Test::XXX::xx::e1' is capitalized inconsistently with its previous name: `::Test::xxx::xx::e1'
-CaseInsensitive.ice:230: ambiguous multiple inheritance: `derived' inherits operations `op' and `OP', which differ only in capitalization, from unrelated base interfaces
-CaseInsensitive.ice:274: data member `x' differs only in capitalization from data member `X', which is defined in a base class
+CaseInsensitive.ice:167: interface name `base' is capitalized inconsistently with its previous name: `::Test::xxx::xx::Base'
+CaseInsensitive.ice:167: redefinition of interface `Derived'
+CaseInsensitive.ice:174: exception name `E1' is capitalized inconsistently with its previous name: `::Test::xxx::xx::e1'
+CaseInsensitive.ice:181: sequence name `S1' is capitalized inconsistently with its previous name: `::Test::xxx::xx::s1'
+CaseInsensitive.ice:182: sequence name `xxx::xx::S1' is capitalized inconsistently with its previous name: `::Test::xxx::xx::s1'
+CaseInsensitive.ice:183: sequence name `xxx::XX::s1' is capitalized inconsistently with its previous name: `::Test::xxx::xx::s1'
+CaseInsensitive.ice:184: sequence name `xxx::XX::s1' is capitalized inconsistently with its previous name: `::Test::xxx::xx::s1'
+CaseInsensitive.ice:190: interface name `derived' is capitalized inconsistently with its previous name: `::Test::xxx::xx::Derived'
+CaseInsensitive.ice:197: parameter `Param' differs only in capitalization from parameter `param'
+CaseInsensitive.ice:197: `Param' has changed meaning
+CaseInsensitive.ice:199: exception name `E1' is capitalized inconsistently with its previous name: `::Test::e1'
+CaseInsensitive.ice:201: exception name `Test::xxx::xx::E1' is capitalized inconsistently with its previous name: `::Test::xxx::xx::e1'
+CaseInsensitive.ice:202: exception name `Test::xxx::XX::e1' is capitalized inconsistently with its previous name: `::Test::xxx::xx::e1'
+CaseInsensitive.ice:203: exception name `Test::XXX::xx::e1' is capitalized inconsistently with its previous name: `::Test::xxx::xx::e1'
+CaseInsensitive.ice:205: exception name `Test::xxx::xx::E1' is capitalized inconsistently with its previous name: `::Test::xxx::xx::e1'
+CaseInsensitive.ice:206: exception name `Test::xxx::XX::e1' is capitalized inconsistently with its previous name: `::Test::xxx::xx::e1'
+CaseInsensitive.ice:207: exception name `Test::XXX::xx::e1' is capitalized inconsistently with its previous name: `::Test::xxx::xx::e1'
+CaseInsensitive.ice:225: ambiguous multiple inheritance: `derived' inherits operations `op' and `OP', which differ only in capitalization, from unrelated base interfaces
+CaseInsensitive.ice:269: data member `x' differs only in capitalization from data member `X', which is defined in a base class

cpp/test/Slice/errorDetection/CaseInsensitive.ice

@@ -151,10 +151,7 @@ enum m1 { green }
 enum M1 { blue }
 enum en2 { yellow, en1, EN1}
 
-module xxx
-{
-
-module xx
+module xxx::xx
 {
 
 interface Base
@@ -195,8 +192,6 @@ struct s2
 
 }
 
-}
-
 interface Foo
 {
     void op(long param, string Param);

cpp/test/Slice/errorDetection/NestedModule.err

@@ -0,0 +1,10 @@
+NestedModule.ice:9: illegal identifier `Holder': `Holder' suffix is reserved
+NestedModule.ice:11: illegal identifier `Holder': `Holder' suffix is reserved
+NestedModule.ice:15: illegal leading underscore in identifier `__Iceberg'
+NestedModule.ice:18: illegal identifier `Holder': `Holder' suffix is reserved
+NestedModule.ice:20: illegal identifier `Holder': `Holder' suffix is reserved
+NestedModule.ice:24: illegal identifier `APtr': `Ptr' suffix is reserved
+NestedModule.ice:24: illegal identifier `BPrx': `Prx' suffix is reserved
+NestedModule.ice:24: illegal identifier `CHelper': `Helper' suffix is reserved
+NestedModule.ice:27: illegal identifier: module identifiers cannot start with '::' prefix
+NestedModule.ice:27: illegal identifier `Helper': `Helper' suffix is reserved

cpp/test/Slice/errorDetection/NestedModule.ice

@@ -0,0 +1,27 @@
+//
+// Copyright (c) ZeroC, Inc. All rights reserved.
+//
+
+// Case where the module chain is all good.
+module All::Good::Here
+{
+    module Okay {}
+    module Holder {}            // Error: 'Holder' suffix is reserved
+
+    module Okay::Holder {}      // Error: 'Holder' suffix is reserved
+}
+
+// Case where the module chain is broken.
+module An::__Iceberg::Ahead      // Error: illegal leading underscore
+{
+    module Okay {}
+    module Holder {}            // Error: 'Holder' suffix is reserved
+
+    module Okay::Holder {}      // Error: 'Holder' suffix is reserved
+}
+
+// Ensure that each module segment is individually checked for errors.
+module APtr::Okay::BPrx::Fine::CHelper {}
+
+// Ensure that modules cannot start with a '::' character.
+module ::No::Leading::Helper {}

csharp/test/Ice/ami/Test.ice

@@ -60,10 +60,7 @@ interface TestIntfController
     void resumeAdapter();
 }
 
-module Outer
-{
-
-module Inner
+module Outer::Inner
 {
 
 interface TestIntf
@@ -74,5 +71,3 @@ interface TestIntf
 }
 
 }
-
-}

csharp/test/Ice/scope/Test.ice

@@ -146,13 +146,7 @@ module Test
     }
 }
 
-module Inner
-{
-
-module Test
-{
-
-module Inner2
+module Inner::Test::Inner2
 {
     interface I
     {
@@ -167,7 +161,3 @@ module Inner2
         void shutdown();
     }
 }
-
-}
-
-}

java/test/src/main/java/test/Ice/ami/Test.ice

@@ -62,10 +62,7 @@ interface TestIntfController
     void resumeAdapter();
 }
 
-module Outer
-{
-
-module Inner
+module Outer::Inner
 {
 
 interface TestIntf
@@ -76,5 +73,3 @@ interface TestIntf
 }
 
 }
-
-}

java/test/src/main/java/test/Ice/scope/Test.ice

@@ -145,13 +145,7 @@ module Test
     }
 }
 
-module Inner
-{
-
-module Test
-{
-
-module Inner2
+module Inner::Test::Inner2
 {
     interface I
     {
@@ -166,7 +160,3 @@ module Inner2
         void shutdown();
     }
 }
-
-}
-
-}

js/test/Ice/import/Demo/Canvas.ice

@@ -10,10 +10,7 @@
 #include "Demo/Circle.ice"
 #include "Glacier2/Session.ice"
 
-module Demo
-{
-
-module gx
+module Demo::gx
 {
 
 interface Canvas
@@ -29,5 +26,3 @@ interface Session extends Glacier2::Session
 }
 
 }
-
-}

js/test/Ice/import/Demo/Circle.ice

@@ -8,10 +8,7 @@
 
 #include "Demo/Point.ice"
 
-module Demo
-{
-
-module gx
+module Demo::gx
 {
 
 struct Circle
@@ -21,5 +18,3 @@ struct Circle
 }
 
 }
-
-}

js/test/Ice/import/Demo/Point.ice

@@ -6,10 +6,7 @@
 
 #pragma once
 
-module Demo
-{
-
-module gx
+module Demo::gx
 {
 
 struct Point
@@ -19,5 +16,3 @@ struct Point
 }
 
 }
-
-}