Skip to content

Commit

Permalink
Fixed review comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
InsertCreativityHere committed Jan 14, 2025
1 parent e013361 commit 3dc8134
Show file tree
Hide file tree
Showing 16 changed files with 73 additions and 68 deletions.
4 changes: 2 additions & 2 deletions cpp/src/IceGrid/DescriptorBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ ApplicationDescriptorBuilder::isOverride(const string& name)

ServerInstanceDescriptorBuilder::ServerInstanceDescriptorBuilder(const XmlAttributesHelper& attrs)
{
_descriptor._cpp_template = attrs("template");
_descriptor.templateName = attrs("template");
_descriptor.parameterValues = attrs.asMap();
_descriptor.parameterValues.erase("template");
}
Expand Down Expand Up @@ -724,7 +724,7 @@ CommunicatorDescriptorBuilder::addProperty(PropertyDescriptorSeq& properties, co

ServiceInstanceDescriptorBuilder::ServiceInstanceDescriptorBuilder(const XmlAttributesHelper& attrs)
{
_descriptor._cpp_template = attrs("template");
_descriptor.templateName = attrs("template");
_descriptor.parameterValues = attrs.asMap();
_descriptor.parameterValues.erase("template");
}
Expand Down
38 changes: 19 additions & 19 deletions cpp/src/IceGrid/DescriptorHelper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1732,7 +1732,7 @@ ServiceInstanceHelper::ServiceInstanceHelper(ServiceInstanceDescriptor desc, boo
// descriptor must be set and contain the definition of the
// service.
//
if (_def._cpp_template.empty() && !_def.descriptor)
if (_def.templateName.empty() && !_def.descriptor)
{
throw DeploymentException("invalid service instance: no template defined");
}
Expand All @@ -1746,13 +1746,13 @@ ServiceInstanceHelper::ServiceInstanceHelper(ServiceInstanceDescriptor desc, boo
bool
ServiceInstanceHelper::operator==(const ServiceInstanceHelper& helper) const
{
if (_def._cpp_template.empty())
if (_def.templateName.empty())
{
return _service == helper._service;
}
else
{
return _def._cpp_template == helper._def._cpp_template && _def.parameterValues == helper._def.parameterValues &&
return _def.templateName == helper._def.templateName && _def.parameterValues == helper._def.parameterValues &&
_def.propertySet == helper._def.propertySet;
}
}
Expand All @@ -1770,12 +1770,12 @@ ServiceInstanceHelper::instantiate(const Resolver& resolve, const PropertySetDes
std::map<std::string, std::string> parameterValues;
if (!def.getDescriptor())
{
assert(!_def._cpp_template.empty());
TemplateDescriptor tmpl = resolve.getServiceTemplate(_def._cpp_template);
assert(!_def.templateName.empty());
TemplateDescriptor tmpl = resolve.getServiceTemplate(_def.templateName);
def = ServiceHelper(dynamic_pointer_cast<ServiceDescriptor>(tmpl.descriptor));
parameterValues = instantiateParams(
resolve,
_def._cpp_template,
_def.templateName,
_def.parameterValues,
tmpl.parameters,
tmpl.parameterDefaults);
Expand All @@ -1800,7 +1800,7 @@ ServiceInstanceHelper::instantiate(const Resolver& resolve, const PropertySetDes
// the template + parameters which would be wrong (if the template
// changed the instance also changed.)
//
// desc._cpp_template = _template;
// desc.templateName = _template;
// desc.parameterValues = _parameters;
return desc;
}
Expand Down Expand Up @@ -1828,10 +1828,10 @@ ServiceInstanceHelper::print(const shared_ptr<Ice::Communicator>& communicator,
}
else
{
assert(!_def._cpp_template.empty());
assert(!_def.templateName.empty());
out << "service instance";
out << sb;
out << nl << "template = '" << _def._cpp_template << "'";
out << nl << "template = '" << _def.templateName << "'";
out << nl << "parameters";
out << sb;
for (StringStringDict::const_iterator p = _def.parameterValues.begin(); p != _def.parameterValues.end(); ++p)
Expand Down Expand Up @@ -1868,19 +1868,19 @@ ServerInstanceHelper::init(const shared_ptr<ServerDescriptor>& definition, const
std::map<std::string, std::string> parameterValues;
if (!def)
{
if (_def._cpp_template.empty())
if (_def.templateName.empty())
{
resolve.exception("invalid server instance: template is not defined");
}

//
// Get the server definition and the template property sets.
//
TemplateDescriptor tmpl = resolve.getServerTemplate(_def._cpp_template);
TemplateDescriptor tmpl = resolve.getServerTemplate(_def.templateName);
def = dynamic_pointer_cast<ServerDescriptor>(tmpl.descriptor);
parameterValues = instantiateParams(
resolve,
_def._cpp_template,
_def.templateName,
_def.parameterValues,
tmpl.parameters,
tmpl.parameterDefaults);
Expand Down Expand Up @@ -1919,9 +1919,9 @@ ServerInstanceHelper::init(const shared_ptr<ServerDescriptor>& definition, const
// Instantiate the server instance definition (we use the server
// resolver above, so using parameters in properties is possible).
//
if (!_def._cpp_template.empty())
if (!_def.templateName.empty())
{
_instance._cpp_template = _def._cpp_template;
_instance.templateName = _def.templateName;
_instance.parameterValues = parameterValues;
_instance.propertySet = svrResolve(_def.propertySet);
for (PropertySetDescriptorDict::const_iterator p = _def.servicePropertySets.begin();
Expand All @@ -1943,13 +1943,13 @@ ServerInstanceHelper::init(const shared_ptr<ServerDescriptor>& definition, const
bool
ServerInstanceHelper::operator==(const ServerInstanceHelper& helper) const
{
if (_def._cpp_template.empty())
if (_def.templateName.empty())
{
return *_serverDefinition == *helper._serverDefinition;
}
else
{
return _def._cpp_template == helper._def._cpp_template && _def.parameterValues == helper._def.parameterValues &&
return _def.templateName == helper._def.templateName && _def.parameterValues == helper._def.parameterValues &&
_def.propertySet == helper._def.propertySet &&
_def.servicePropertySets == helper._def.servicePropertySets;
}
Expand All @@ -1970,21 +1970,21 @@ ServerInstanceHelper::getId() const
ServerInstanceDescriptor
ServerInstanceHelper::getDefinition() const
{
assert(!_def._cpp_template.empty());
assert(!_def.templateName.empty());
return _def;
}

ServerInstanceDescriptor
ServerInstanceHelper::getInstance() const
{
assert(!_def._cpp_template.empty() && !_instance._cpp_template.empty());
assert(!_def.templateName.empty() && !_instance.templateName.empty());
return _instance;
}

shared_ptr<ServerDescriptor>
ServerInstanceHelper::getServerDefinition() const
{
assert(_def._cpp_template.empty());
assert(_def.templateName.empty());
return _serverDefinition->getDescriptor();
}

Expand Down
2 changes: 1 addition & 1 deletion cpp/src/IceGrid/Parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -821,7 +821,7 @@ Parser::instantiateServerTemplate(const list<string>& args)
}

ServerInstanceDescriptor desc;
desc._cpp_template = templ;
desc.templateName = templ;
desc.parameterValues = vars;
_admin->instantiateServer(application, node, desc);
}
Expand Down
19 changes: 11 additions & 8 deletions cpp/src/Slice/Parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -581,9 +581,12 @@ Slice::Contained::scope() const
string
Slice::Contained::mappedName() const
{
const string languageName = _unit->languageName();
assert(!languageName.empty());

// First check if any 'xxx:identifier' has been applied to this element.
// If so, we return that instead of the element's Slice identifier.
const string metadata = _unit->languagePrefix() + ":identifier";
const string metadata = languageName + ":identifier";
if (auto customName = getMetadataArgs(metadata))
{
return *customName;
Expand Down Expand Up @@ -4592,23 +4595,23 @@ Slice::DataMember::DataMember(
// ----------------------------------------------------------------------

UnitPtr
Slice::Unit::createUnit(string languagePrefix, bool all, const StringList& defaultFileMetadata)
Slice::Unit::createUnit(string languageName, bool all, const StringList& defaultFileMetadata)
{
MetadataList defaultMetadata;
for (const auto& metadataString : defaultFileMetadata)
{
defaultMetadata.push_back(make_shared<Metadata>(metadataString, "<command-line>", 0));
}

UnitPtr unit{new Unit{std::move(languagePrefix), all, std::move(defaultMetadata)}};
UnitPtr unit{new Unit{std::move(languageName), all, std::move(defaultMetadata)}};
unit->_unit = unit;
return unit;
}

string
Slice::Unit::languagePrefix() const
Slice::Unit::languageName() const
{
return _languagePrefix;
return _languageName;
}

void
Expand Down Expand Up @@ -5060,16 +5063,16 @@ Slice::Unit::getTopLevelModules(const string& file) const
}
}

Slice::Unit::Unit(string languagePrefix, bool all, MetadataList defaultFileMetadata)
Slice::Unit::Unit(string languageName, bool all, MetadataList defaultFileMetadata)
: SyntaxTreeBase(nullptr),
Container(nullptr),
_languagePrefix(std::move(languagePrefix)),
_languageName(std::move(languageName)),
_all(all),
_defaultFileMetadata(std::move(defaultFileMetadata)),
_errors(0),
_currentIncludeLevel(0)
{
assert(binary_search(&languages[0], &languages[sizeof(languages) / sizeof(*languages)], _languagePrefix));
assert(binary_search(&languages[0], &languages[sizeof(languages) / sizeof(*languages)], _languageName));
}

// ----------------------------------------------------------------------
Expand Down
20 changes: 10 additions & 10 deletions cpp/src/Slice/Parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -364,18 +364,18 @@ namespace Slice
public:
[[nodiscard]] ContainerPtr container() const;

/// @return The Slice identifier of this element.
/// Returns the Slice identifier of this element.
[[nodiscard]] std::string name() const;
/// @return The Slice scope that this element is contained within (with a trailing '::').
/// Returns the Slice scope that this element is contained within (with a trailing '::').
[[nodiscard]] std::string scope() const;
/// @return The fully-scoped Slice identifier of this element (equivalent to `scope() + name()`).
/// Returns the fully-scoped Slice identifier of this element (equivalent to `scope() + name()`).
[[nodiscard]] std::string scoped() const;

/// @return The mapped identifier that this element will use in the target language.
/// Returns the mapped identifier that this element will use in the target language.
[[nodiscard]] std::string mappedName() const;
/// @return The mapped scope that this element will be generated in in the target language.
/// Returns the mapped scope that this element will be generated in in the target language.
[[nodiscard]] std::string mappedScoped() const;
/// @return The mapped fully-scoped identifier that this element will use in the target language.
/// Returns the mapped fully-scoped identifier that this element will use in the target language.
/// (equivalent to `mappedScoped() + mappedName()`).
[[nodiscard]] std::string mappedScope() const;

Expand Down Expand Up @@ -1002,9 +1002,9 @@ namespace Slice
{
public:
static UnitPtr
createUnit(std::string languagePrefix, bool all, const StringList& defaultFileMetadata = StringList());
createUnit(std::string languageName, bool all, const StringList& defaultFileMetadata = StringList());

[[nodiscard]] std::string languagePrefix() const;
[[nodiscard]] std::string languageName() const;

void setDocComment(const std::string& comment);
void addToDocComment(const std::string& comment);
Expand Down Expand Up @@ -1056,12 +1056,12 @@ namespace Slice
[[nodiscard]] std::set<std::string> getTopLevelModules(const std::string& file) const;

private:
Unit(std::string languagePrefix, bool all, MetadataList defaultFileMetadata);
Unit(std::string languageName, bool all, MetadataList defaultFileMetadata);

void pushDefinitionContext();
void popDefinitionContext();

const std::string _languagePrefix;
const std::string _languageName;
bool _all;
MetadataList _defaultFileMetadata;
int _errors;
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/ice2slice/Main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ compile(const vector<string>& argv)
}
else
{
UnitPtr p = Unit::createUnit("icerpc", false);
UnitPtr p = Unit::createUnit("", false);
int parseStatus = p->parse(*i, cppHandle, debug);

if (!icecpp->close())
Expand Down
5 changes: 3 additions & 2 deletions cpp/test/Ice/servantLocator/Test.ice
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@ interface TestIntf

void unknownExceptionWithServantException();

string impossibleException(["cpp:identifier:_cpp_throw"] bool throw) throws TestImpossibleException;
string intfUserException(["cpp:identifier:_cpp_throw"] bool throw) throws TestIntfUserException, TestImpossibleException;
// TODO rename the throw variable in all language mappings before adding more 'xxx:identifier'.
string impossibleException(["cpp:identifier:shouldThrow"] bool throw) throws TestImpossibleException;
string intfUserException(["cpp:identifier:shouldThrow"] bool throw) throws TestIntfUserException, TestImpossibleException;

void asyncResponse() throws TestIntfUserException, TestImpossibleException;
void asyncException() throws TestIntfUserException, TestImpossibleException;
Expand Down
5 changes: 3 additions & 2 deletions cpp/test/Ice/servantLocator/TestAMD.ice
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@ exception TestImpossibleException

void unknownExceptionWithServantException();

string impossibleException(["cpp:identifier:_cpp_throw"] bool throw) throws TestImpossibleException;
string intfUserException(["cpp:identifier:_cpp_throw"] bool throw) throws TestIntfUserException, TestImpossibleException;
// TODO rename the throw variable in all language mappings before adding more 'xxx:identifier'.
string impossibleException(["cpp:identifier:shouldThrow"] bool throw) throws TestImpossibleException;
string intfUserException(["cpp:identifier:shouldThrow"] bool throw) throws TestIntfUserException, TestImpossibleException;

void asyncResponse() throws TestIntfUserException, TestImpossibleException;
void asyncException() throws TestIntfUserException, TestImpossibleException;
Expand Down
8 changes: 4 additions & 4 deletions cpp/test/Ice/servantLocator/TestAMDI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,12 @@ TestAMDI::unknownExceptionWithServantExceptionAsync(

void
TestAMDI::impossibleExceptionAsync(
bool _cpp_throw,
bool shouldThrow,
function<void(string_view)> response,
function<void(exception_ptr)> error,
const Current&)
{
if (_cpp_throw)
if (shouldThrow)
{
try
{
Expand All @@ -102,12 +102,12 @@ TestAMDI::impossibleExceptionAsync(

void
TestAMDI::intfUserExceptionAsync(
bool _cpp_throw,
bool shouldThrow,
function<void(string_view)> response,
function<void(exception_ptr)> error,
const Current&)
{
if (_cpp_throw)
if (shouldThrow)
{
try
{
Expand Down
8 changes: 4 additions & 4 deletions cpp/test/Ice/servantLocator/TestI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ TestI::unknownExceptionWithServantException(const Current&)
}

string
TestI::impossibleException(bool _cpp_throw, const Current&)
TestI::impossibleException(bool shouldThrow, const Current&)
{
if (_cpp_throw)
if (shouldThrow)
{
throw Test::TestImpossibleException();
}
Expand All @@ -69,9 +69,9 @@ TestI::impossibleException(bool _cpp_throw, const Current&)
}

string
TestI::intfUserException(bool _cpp_throw, const Current&)
TestI::intfUserException(bool shouldThrow, const Current&)
{
if (_cpp_throw)
if (shouldThrow)
{
throw Test::TestIntfUserException();
}
Expand Down
2 changes: 1 addition & 1 deletion cpp/test/IceGrid/activation/AllTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,7 @@ allTests(Test::TestHelper* helper)
ostringstream id;
id << "server-" << i;
IceGrid::ServerInstanceDescriptor server;
server._cpp_template = "Server";
server.templateName = "Server";
server.parameterValues["id"] = id.str();
testApp.nodes["localnode"].serverInstances.push_back(server);
}
Expand Down
4 changes: 2 additions & 2 deletions cpp/test/IceGrid/noRestartUpdate/AllTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ allTests(Test::TestHelper* helper)
update.variables["server.dir"] = properties->getProperty("ServerDir");
update.variables["variable"] = "";
instance = ServerInstanceDescriptor();
instance._cpp_template = "ServerTemplate";
instance.templateName = "ServerTemplate";
instance.parameterValues["name"] = "Server1";
update.nodes[0].serverInstances.push_back(instance);
try
Expand Down Expand Up @@ -381,7 +381,7 @@ allTests(Test::TestHelper* helper)
update = empty;
update.serverTemplates["ServerTemplate"] = templ;
instance = ServerInstanceDescriptor();
instance._cpp_template = "ServerTemplate";
instance.templateName = "ServerTemplate";
instance.parameterValues["name"] = "Server1";
update.nodes[0].serverInstances.push_back(instance);
admin->updateApplicationWithoutRestart(update);
Expand Down
Loading

0 comments on commit 3dc8134

Please sign in to comment.