Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Support for cpp:identifier Metadata, and the Groundwork for xxx:identifier Metadata in General #3292

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
6787ddb
The base structure is down.
InsertCreativityHere Dec 16, 2024
87e3dde
Implemented 'cpp:identifier'.
InsertCreativityHere Dec 17, 2024
11ac464
Turns out we actually needed this in our own code...
InsertCreativityHere Dec 17, 2024
78529c0
Fixing all the merge conflicts from clang-tidy...
InsertCreativityHere Dec 17, 2024
ce37faf
Enable the 'cpp:identifier' metadata in the validator.
InsertCreativityHere Dec 17, 2024
9cda559
Cleanup and formatting.
InsertCreativityHere Dec 17, 2024
b2e8390
Finish fixing the tests.
InsertCreativityHere Dec 18, 2024
5427090
Added a way to inject language prefix into the parser's core machinery.
InsertCreativityHere Jan 8, 2025
1cb67c5
Switch APIs to take bools instead of strings.
InsertCreativityHere Jan 8, 2025
329aeae
Expose the injected prefix with an accessor function.
InsertCreativityHere Jan 8, 2025
a3444f1
Need to change the dynamic compilation code in Ruby & Python.
InsertCreativityHere Jan 8, 2025
2c3b573
Remove support for putting this on modules.
InsertCreativityHere Jan 8, 2025
dca41be
Get the escape test working now that we can't fix module names anymore.
InsertCreativityHere Jan 8, 2025
3f54c9d
Self review stuff.
InsertCreativityHere Jan 8, 2025
382482e
Formatting...
InsertCreativityHere Jan 8, 2025
2e8b08d
Use separate functions instead of bools.
InsertCreativityHere Jan 13, 2025
3053c04
Missed a couple.
InsertCreativityHere Jan 13, 2025
c4b3647
Merge branch 'main' into xxx-identifier-metadata
InsertCreativityHere Jan 13, 2025
8a904fe
Lost one among the merge fixes.
InsertCreativityHere Jan 13, 2025
6eeb7ac
Review fixes and formatting.
InsertCreativityHere Jan 13, 2025
4084958
Overzealous.
InsertCreativityHere Jan 13, 2025
e013361
Formatting.
InsertCreativityHere Jan 14, 2025
3dc8134
Fixed review comments.
InsertCreativityHere Jan 14, 2025
951a19b
Merge branch 'main' into xxx-identifier-metadata
InsertCreativityHere Jan 15, 2025
3f92679
Merge conflict artifacts.
InsertCreativityHere Jan 15, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -1729,7 +1729,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 @@ -1743,13 +1743,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 @@ -1767,12 +1767,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 @@ -1797,7 +1797,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 @@ -1825,10 +1825,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 (const auto& parameterValue : _def.parameterValues)
Expand Down Expand Up @@ -1865,19 +1865,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 @@ -1916,9 +1916,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 (const auto& servicePropertySet : _def.servicePropertySets)
Expand All @@ -1939,13 +1939,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 @@ -1966,21 +1966,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
4 changes: 2 additions & 2 deletions cpp/src/Slice/MetadataValidation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,9 @@ Slice::validateMetadata(const UnitPtr& p, string_view prefix, map<string, Metada
MetadataInfo deprecatedInfo = {
.validOn =
{typeid(InterfaceDecl),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just swapped the order here to be consistent with other lists.

typeid(ClassDecl),
typeid(Operation),
typeid(Exception),
typeid(ClassDecl),
typeid(Slice::Exception),
typeid(Struct),
typeid(Sequence),
typeid(Dictionary),
Expand Down
74 changes: 52 additions & 22 deletions cpp/src/Slice/Parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ compareTag(const T& lhs, const T& rhs)
return lhs->tag() < rhs->tag();
}

// NOTE: It is important that this list is kept in alphabetical order!
constexpr string_view languages[] = {"cpp", "cs", "java", "js", "matlab", "php", "python", "ruby", "swift"};

// Forward declare things from Bison and Flex the parser can use.
extern int slice_parse();
extern int slice_lineno;
Expand Down Expand Up @@ -104,8 +107,6 @@ Slice::Metadata::Metadata(string rawMetadata, string file, int line) : GrammarBa
if (firstColonPos != string::npos)
{
// Check if the metadata starts with a language prefix.
// NOTE: It is important that this list is kept in alphabetical order!
constexpr string_view languages[] = {"cpp", "cs", "java", "js", "matlab", "php", "python", "rb", "swift"};
string prefix = rawMetadata.substr(0, firstColonPos);
bool hasLangPrefix = binary_search(&languages[0], &languages[sizeof(languages) / sizeof(*languages)], prefix);
if (hasLangPrefix)
Expand Down Expand Up @@ -562,27 +563,52 @@ Slice::Contained::name() const
string
Slice::Contained::scoped() const
{
return _scoped;
return scope() + name();
}

string
Slice::Contained::scope() const
{
string::size_type idx = _scoped.rfind("::");
assert(idx != string::npos);
return string(_scoped, 0, idx + 2);
string scoped;
if (auto container = dynamic_pointer_cast<Contained>(_container))
{
scoped = container->scoped();
}
return scoped + "::";
}

string
Slice::Contained::flattenedScope() const
Slice::Contained::mappedName() const
{
string s = scope();
string::size_type pos = 0;
while ((pos = s.find("::", pos)) != string::npos)
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 = languageName + ":identifier";
if (auto customName = getMetadataArgs(metadata))
{
s.replace(pos, 2, "_");
return *customName;
}
return s;

return _name;
}

string
Slice::Contained::mappedScoped() const
{
return mappedScope() + mappedName();
}

string
Slice::Contained::mappedScope() const
{
string scoped;
if (auto container = dynamic_pointer_cast<Contained>(_container))
{
scoped = container->mappedScoped();
}
return scoped + "::";
}

string
Expand Down Expand Up @@ -1050,12 +1076,6 @@ Slice::Contained::Contained(const ContainerPtr& container, string name)
_container(container),
_name(std::move(name))
{
ContainedPtr cont = dynamic_pointer_cast<Contained>(_container);
if (cont)
{
_scoped = cont->scoped();
}
_scoped += "::" + _name;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There isn't much point to _scoped now.

Most of the calls to scoped() and scope() are for mapped identifiers, and now we have to compute their values on the fly. So, there's even less point to caching the _scoped name up-front. So, I removed it.

assert(_unit);
_file = _unit->currentFile();
_line = _unit->currentLine();
Expand Down Expand Up @@ -4544,19 +4564,25 @@ Slice::DataMember::DataMember(
// ----------------------------------------------------------------------
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the code below this is to allow the compilers to inject their languagePrefix into libSlice.


UnitPtr
Slice::Unit::createUnit(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{all, std::move(defaultMetadata)}};
UnitPtr unit{new Unit{std::move(languageName), all, std::move(defaultMetadata)}};
unit->_unit = unit;
return unit;
}

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

void
Slice::Unit::setDocComment(const string& comment)
{
Expand Down Expand Up @@ -5006,15 +5032,19 @@ Slice::Unit::getTopLevelModules(const string& file) const
}
}

Slice::Unit::Unit(bool all, MetadataList defaultFileMetadata)
Slice::Unit::Unit(string languageName, bool all, MetadataList defaultFileMetadata)
: SyntaxTreeBase(nullptr),
Container(nullptr),
_languageName(std::move(languageName)),
_all(all),
_defaultFileMetadata(std::move(defaultFileMetadata)),
_errors(0),
_currentIncludeLevel(0)

{
if (!languageName.empty())
{
assert(binary_search(&languages[0], &languages[sizeof(languages) / sizeof(*languages)], _languageName));
}
}

// ----------------------------------------------------------------------
Expand Down
Loading
Loading