Skip to content

Commit

Permalink
All class/interface metadata is stored on the 'Decl' instead of split…
Browse files Browse the repository at this point in the history 
… between 'Decl' and 'Def'.
  • Loading branch information
@InsertCreativityHere
InsertCreativityHere committed Dec 3, 2024
1 parent b463378 commit ac61730
Show file tree
Hide file tree
Showing 15 changed files with 165 additions and 127 deletions.
14 changes: 7 additions & 7 deletions cpp/src/Slice/Grammar.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1791,7 +1791,7 @@ YYLTYPE yylloc = yyloc_default;
auto contained = dynamic_pointer_cast<Contained>(yyvsp[0]);
if (contained && !metadata->v.empty())
{
contained->setMetadata(std::move(metadata->v));
contained->appendMetadata(std::move(metadata->v));
}
}
#line 1798 "src/Slice/Grammar.cpp"
Expand Down Expand Up @@ -2670,7 +2670,7 @@ YYLTYPE yylloc = yyloc_default;
auto contained = dynamic_pointer_cast<Contained>(yyvsp[-2]);
if (contained && !metadata->v.empty())
{
contained->setMetadata(std::move(metadata->v));
contained->appendMetadata(std::move(metadata->v));
}
}
#line 2677 "src/Slice/Grammar.cpp"
Expand Down Expand Up @@ -3221,7 +3221,7 @@ YYLTYPE yylloc = yyloc_default;
auto contained = dynamic_pointer_cast<Contained>(yyvsp[-2]);
if (contained && !metadata->v.empty())
{
contained->setMetadata(std::move(metadata->v));
contained->appendMetadata(std::move(metadata->v));
}
}
#line 3228 "src/Slice/Grammar.cpp"
Expand Down Expand Up @@ -3435,7 +3435,7 @@ YYLTYPE yylloc = yyloc_default;
auto enumerator = dynamic_pointer_cast<Enumerator>(yyvsp[-2]);
if (enumerator && !metadata->v.empty())
{
enumerator->setMetadata(std::move(metadata->v));
enumerator->appendMetadata(std::move(metadata->v));
}
auto enumeratorList = dynamic_pointer_cast<EnumeratorListTok>(yyvsp[0]);
enumeratorList->v.push_front(enumerator);
Expand All @@ -3451,7 +3451,7 @@ YYLTYPE yylloc = yyloc_default;
auto enumerator = dynamic_pointer_cast<Enumerator>(yyvsp[0]);
if (enumerator && !metadata->v.empty())
{
enumerator->setMetadata(std::move(metadata->v));
enumerator->appendMetadata(std::move(metadata->v));
}
auto enumeratorList = make_shared<EnumeratorListTok>();
enumeratorList->v.push_front(enumerator);
Expand Down Expand Up @@ -3599,7 +3599,7 @@ YYLTYPE yylloc = yyloc_default;
auto metadata = dynamic_pointer_cast<MetadataListTok>(yyvsp[-1]);
if (!metadata->v.empty())
{
pd->setMetadata(std::move(metadata->v));
pd->appendMetadata(std::move(metadata->v));
}
}
}
Expand All @@ -3619,7 +3619,7 @@ YYLTYPE yylloc = yyloc_default;
auto metadata = dynamic_pointer_cast<MetadataListTok>(yyvsp[-1]);
if (!metadata->v.empty())
{
pd->setMetadata(std::move(metadata->v));
pd->appendMetadata(std::move(metadata->v));
}
}
}
Expand Down
14 changes: 7 additions & 7 deletions cpp/src/Slice/Grammar.y
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ definitions
auto contained = dynamic_pointer_cast<Contained>($3);
if (contained && !metadata->v.empty())
{
contained->setMetadata(std::move(metadata->v));
contained->appendMetadata(std::move(metadata->v));
}
}
| %empty
Expand Down Expand Up @@ -989,7 +989,7 @@ data_members
auto contained = dynamic_pointer_cast<Contained>($2);
if (contained && !metadata->v.empty())
{
contained->setMetadata(std::move(metadata->v));
contained->appendMetadata(std::move(metadata->v));
}
}
| error ';' data_members
Expand Down Expand Up @@ -1470,7 +1470,7 @@ operations
auto contained = dynamic_pointer_cast<Contained>($2);
if (contained && !metadata->v.empty())
{
contained->setMetadata(std::move(metadata->v));
contained->appendMetadata(std::move(metadata->v));
}
}
| error ';' operations
Expand Down Expand Up @@ -1648,7 +1648,7 @@ enumerator_list
auto enumerator = dynamic_pointer_cast<Enumerator>($2);
if (enumerator && !metadata->v.empty())
{
enumerator->setMetadata(std::move(metadata->v));
enumerator->appendMetadata(std::move(metadata->v));
}
auto enumeratorList = dynamic_pointer_cast<EnumeratorListTok>($4);
enumeratorList->v.push_front(enumerator);
Expand All @@ -1660,7 +1660,7 @@ enumerator_list
auto enumerator = dynamic_pointer_cast<Enumerator>($2);
if (enumerator && !metadata->v.empty())
{
enumerator->setMetadata(std::move(metadata->v));
enumerator->appendMetadata(std::move(metadata->v));
}
auto enumeratorList = make_shared<EnumeratorListTok>();
enumeratorList->v.push_front(enumerator);
Expand Down Expand Up @@ -1788,7 +1788,7 @@ parameters
auto metadata = dynamic_pointer_cast<MetadataListTok>($2);
if (!metadata->v.empty())
{
pd->setMetadata(std::move(metadata->v));
pd->appendMetadata(std::move(metadata->v));
}
}
}
Expand All @@ -1804,7 +1804,7 @@ parameters
auto metadata = dynamic_pointer_cast<MetadataListTok>($4);
if (!metadata->v.empty())
{
pd->setMetadata(std::move(metadata->v));
pd->appendMetadata(std::move(metadata->v));
}
}
}
Expand Down
63 changes: 35 additions & 28 deletions cpp/src/Slice/MetadataValidation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,13 @@ namespace
public:
MetadataVisitor(string_view language, map<string, MetadataInfo> knownMetadata);

// We don't visit `ClassDef` and `InterfaceDef` because their metadata is always stored on their corresponding
// `ClassDecl` and `InterfaceDecl` types. So just visiting the `Decl` types is sufficient to check everything.

bool visitUnitStart(const UnitPtr&) final;
bool visitModuleStart(const ModulePtr&) final;
void visitClassDecl(const ClassDeclPtr&) final;
bool visitClassDefStart(const ClassDefPtr&) final;
void visitInterfaceDecl(const InterfaceDeclPtr&) final;
bool visitInterfaceDefStart(const InterfaceDefPtr&) final;
bool visitExceptionStart(const ExceptionPtr&) final;
bool visitStructStart(const StructPtr&) final;
void visitOperation(const OperationPtr&) final;
Expand All @@ -40,7 +41,7 @@ namespace

string_view _language;
map<string, MetadataInfo> _knownMetadata;
set<string> _seenDirectives;
map<string, MetadataPtr> _seenDirectives;
};
}

Expand Down Expand Up @@ -73,17 +74,15 @@ Slice::validateMetadata(const UnitPtr& p, string_view prefix, map<string, Metada

// "amd"
MetadataInfo amdInfo = {
{typeid(InterfaceDef), typeid(Operation)},
{typeid(InterfaceDecl), typeid(Operation)},
MetadataArgumentKind::NoArguments,
};
knownMetadata.emplace("amd", std::move(amdInfo));

// "deprecated"
MetadataInfo deprecatedInfo = {
{typeid(InterfaceDecl),
typeid(InterfaceDef),
typeid(ClassDecl),
typeid(ClassDef),
typeid(Operation),
typeid(Exception),
typeid(Struct),
Expand All @@ -95,26 +94,27 @@ Slice::validateMetadata(const UnitPtr& p, string_view prefix, map<string, Metada
typeid(DataMember)},
MetadataArgumentKind::OptionalTextArgument,
};
knownMetadata.emplace("deprecate", std::move(deprecatedInfo)); // Kept as an alias for 'deprecated'.
knownMetadata.emplace("deprecated", std::move(deprecatedInfo));

// "format"
MetadataInfo formatInfo = {
{typeid(InterfaceDef), typeid(Operation)},
{typeid(InterfaceDecl), typeid(Operation)},
MetadataArgumentKind::SingleArgument,
{{"compact", "sliced", "default"}},
};
knownMetadata.emplace("format", std::move(formatInfo));

// "marshaled-result"
MetadataInfo marshaledResultInfo = {
{typeid(InterfaceDef), typeid(Operation)},
{typeid(InterfaceDecl), typeid(Operation)},
MetadataArgumentKind::NoArguments,
};
knownMetadata.emplace("marshaled-result", std::move(marshaledResultInfo));

// "protected"
MetadataInfo protectedInfo = {
{typeid(ClassDef), typeid(Slice::Exception), typeid(Struct), typeid(DataMember)},
{typeid(ClassDecl), typeid(Slice::Exception), typeid(Struct), typeid(DataMember)},
MetadataArgumentKind::NoArguments,
};
knownMetadata.emplace("protected", std::move(protectedInfo));
Expand Down Expand Up @@ -171,26 +171,12 @@ MetadataVisitor::visitClassDecl(const ClassDeclPtr& p)
p->setMetadata(validate(p->getMetadata(), p));
}

bool
MetadataVisitor::visitClassDefStart(const ClassDefPtr& p)
{
p->setMetadata(validate(p->getMetadata(), p));
return true;
}

void
MetadataVisitor::visitInterfaceDecl(const InterfaceDeclPtr& p)
{
p->setMetadata(validate(p->getMetadata(), p));
}

bool
MetadataVisitor::visitInterfaceDefStart(const InterfaceDefPtr& p)
{
p->setMetadata(validate(p->getMetadata(), p));
return true;
}

bool
MetadataVisitor::visitExceptionStart(const ExceptionPtr& p)
{
Expand Down Expand Up @@ -441,13 +427,34 @@ MetadataVisitor::isMetadataValid(const MetadataPtr& metadata, const SyntaxTreeBa
// Fifth we check if this metadata is a duplicate, i.e. have we already seen this metadata in this context?
if (info.mustBeUnique)
{
// 'insert' only returns `true` if the value wasn't already present in the set.
bool wasInserted = _seenDirectives.insert(directive).second;
// 'emplace' only returns `true` if the value wasn't already present in the set.
bool wasInserted = _seenDirectives.emplace(directive, metadata).second;
if (!wasInserted)
{
auto msg = "ignoring duplicate metadata: '" + directive + "' has already been applied in this context";
p->unit()->warning(metadata->file(), metadata->line(), InvalidMetadata, msg);
isValid = false;
// We make a special exception to uniqueness for metadata on forward declarations, since there can be
// multiple forward declarations for the same entity, but they all share a single backing `MetadataList`.
// So for these types, even 'unique' metadata to appear multiple times, but we require them to be identical.
if (dynamic_pointer_cast<ClassDecl>(p) || dynamic_pointer_cast<InterfaceDecl>(p))
{
const MetadataPtr& previousMetadata = _seenDirectives[directive];
if (arguments != previousMetadata->arguments())
{
ostringstream msg;
msg << "ignoring duplicate metadata: '" << *metadata
<< "' does not match previously applied metadata of '" << *previousMetadata << "'";
p->unit()->warning(metadata->file(), metadata->line(), InvalidMetadata, msg.str());
}

// Even in this special case, we want to remove the metadata. At worst it was invalid, and at best,
// it's an exact duplicate of some other metadata. Either way, there's no reason to keep it around.
isValid = false;
}
else
{
auto msg = "ignoring duplicate metadata: '" + directive + "' has already been applied in this context";
p->unit()->warning(metadata->file(), metadata->line(), InvalidMetadata, msg);
isValid = false;
}
}
}

Expand Down
3 changes: 3 additions & 0 deletions cpp/src/Slice/MetadataValidation.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ namespace Slice
/// If this list is empty we don't perform any automatic checking of whether this metadata is validly applied.
/// Usually, this is because determining validity isn't as straightforward as matching against a list,
/// and requires a more complex approach, which is achieved through providing an `extraValidation` function.
///
/// Note that `ClassDef` and `InterfaceDef` should never appear in this list, since class & interface metadata
/// is always stored on the corresponding `ClassDecl` and `InterfaceDecl` types, which should be used instead.
std::list<std::reference_wrapper<const std::type_info>> validOn;

/// Specifies how many, and what kinds of arguments, this metadata accepts.
Expand Down
Loading

0 comments on commit ac61730

Please sign in to comment.