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

Reject redefinition of Object operations #3329

Merged
merged 4 commits into from
Jan 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
59 changes: 41 additions & 18 deletions cpp/src/Slice/Parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3007,35 +3007,58 @@ Slice::InterfaceDef::createOperation(
// Check whether any base has an operation with the same name already
for (const auto& baseInterface : _bases)
{
vector<string> baseNames;
for (const auto& op : baseInterface->allOperations())
{
if (op->name() == name)
{
ostringstream os;
os << "operation '" << name << "' is already defined as an operation in a base interface";
_unit->error(os.str());
return nullptr;
}

string baseName = IceInternal::toLower(op->name());
string newName2 = IceInternal::toLower(name);
if (baseName == newName2)
{
ostringstream os;
os << "operation '" << name << "' differs only in capitalization from operation"
<< " '" << op->name() << "', which is defined in a base interface";
_unit->error(os.str());
return nullptr;
}
baseNames.push_back(op->name());
}
if (!checkBaseOperationNames(name, baseNames))
{
return nullptr;
}
}

// Check the operations of the Object pseudo-interface.
if (!checkBaseOperationNames(name, {"ice_id", "ice_ids", "ice_ping", "ice_isA"}))
{
return nullptr;
}

OperationPtr op = make_shared<Operation>(shared_from_this(), name, returnType, isOptional, tag, mode);
_unit->addContent(op);
_contents.push_back(op);
return op;
}

bool
Slice::InterfaceDef::checkBaseOperationNames(const string& name, const vector<string>& baseNames)
{
string nameInLowercase = IceInternal::toLower(name);

for (const auto& baseName : baseNames)
{
if (baseName == name)
{
ostringstream os;
os << "operation '" << name << "' is already defined as an operation in a base interface";
_unit->error(os.str());
return false;
}

string baseNameInLowercase = IceInternal::toLower(baseName);

if (baseNameInLowercase == nameInLowercase)
{
ostringstream os;
os << "operation '" << name << "' differs only in capitalization from operation"
<< " '" << baseName << "', which is defined in a base interface";
_unit->error(os.str());
return false;
}
}
return true;
}

InterfaceDeclPtr
Slice::InterfaceDef::declaration() const
{
Expand Down
3 changes: 3 additions & 0 deletions cpp/src/Slice/Parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -723,6 +723,9 @@ namespace Slice
private:
friend class Container;

// Returns true if name does not collide with any base name; otherwise, false.
bool checkBaseOperationNames(const std::string& name, const std::vector<std::string>& baseNames);

Choose a reason for hiding this comment

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

I know that we're using this to check for collisions with base names, but the implementation itself isn't specific to base names.

I think a name like checkForNameCollision would be more apt for this function.
Along with changing baseNames to otherNames or something.

Copy link
Member Author

Choose a reason for hiding this comment

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

The implementation is specific to base operation names: please read the error messages.

Choose a reason for hiding this comment

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

Yes, you're right. It should be left as-is.


InterfaceDeclPtr _declaration;
InterfaceList _bases;
};
Expand Down
97 changes: 49 additions & 48 deletions cpp/test/Slice/errorDetection/CaseInsensitive.err
Original file line number Diff line number Diff line change
@@ -1,51 +1,52 @@
CaseInsensitive.ice:11: redefinition of operation 'op' as operation 'op'
CaseInsensitive.ice:17: operation 'oP' differs only in capitalization from operation 'op'
CaseInsensitive.ice:17: redefinition of operation 'op' as operation 'oP'
CaseInsensitive.ice:24: module 'M1' is capitalized inconsistently with its previous name: 'm1'
CaseInsensitive.ice:28: module 'C1' is capitalized inconsistently with its previous name: 'c1'
CaseInsensitive.ice:33: redefinition of parameter 'aa'
CaseInsensitive.ice:34: parameter 'BB' differs only in capitalization from parameter 'bb'
CaseInsensitive.ice:34: 'BB' has changed meaning
CaseInsensitive.ice:39: operation 'I4' differs only in capitalization from enclosing interface name 'i4'
CaseInsensitive.ice:44: interface name 'i5' cannot be used as operation name
CaseInsensitive.ice:54: operation 'op' is already defined as an operation in a base interface
CaseInsensitive.ice:59: operation 'OP' differs only in capitalization from operation 'op', which is defined in a base interface
CaseInsensitive.ice:69: data member 'l' is already defined as a data member in a base class
CaseInsensitive.ice:74: data member 'L' differs only in capitalization from data member 'l', which is defined in a base class
CaseInsensitive.ice:80: redefinition of exception member 'l'
CaseInsensitive.ice:86: exception member 'L' differs only in capitalization from exception member 'l'
CaseInsensitive.ice:86: 'L' has changed meaning
CaseInsensitive.ice:106: exception member 'l' is already defined in a base exception
CaseInsensitive.ice:111: exception member 'L' differs only in capitalization from exception member 'l', which is defined in a base exception
CaseInsensitive.ice:117: redefinition of struct member 'l'
CaseInsensitive.ice:123: member 'L' differs only in capitalization from member 'l'
CaseInsensitive.ice:123: 'L' has changed meaning
CaseInsensitive.ice:139: sequence 'LS' differs only in capitalization from sequence 'ls'
CaseInsensitive.ice:140: redefinition of module 'm1' as sequence
CaseInsensitive.ice:141: sequence 'M1' differs only in capitalization from module 'm1'
CaseInsensitive.ice:144: dictionary 'D' differs only in capitalization from dictionary 'd'
CaseInsensitive.ice:145: redefinition of module 'm1' as dictionary
CaseInsensitive.ice:146: dictionary 'M1' differs only in capitalization from module 'm1'
CaseInsensitive.ice:149: enumeration 'eN1' differs only in capitalization from enumeration 'en1'
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: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
CaseInsensitive.ice:22: operation 'ice_isa' differs only in capitalization from operation 'ice_isA', which is defined in a base interface
CaseInsensitive.ice:29: module 'M1' is capitalized inconsistently with its previous name: 'm1'
CaseInsensitive.ice:33: module 'C1' is capitalized inconsistently with its previous name: 'c1'
CaseInsensitive.ice:38: redefinition of parameter 'aa'
CaseInsensitive.ice:39: parameter 'BB' differs only in capitalization from parameter 'bb'
CaseInsensitive.ice:39: 'BB' has changed meaning
CaseInsensitive.ice:44: operation 'I4' differs only in capitalization from enclosing interface name 'i4'
CaseInsensitive.ice:49: interface name 'i5' cannot be used as operation name
CaseInsensitive.ice:59: operation 'op' is already defined as an operation in a base interface
CaseInsensitive.ice:64: operation 'OP' differs only in capitalization from operation 'op', which is defined in a base interface
CaseInsensitive.ice:74: data member 'l' is already defined as a data member in a base class
CaseInsensitive.ice:79: data member 'L' differs only in capitalization from data member 'l', which is defined in a base class
CaseInsensitive.ice:85: redefinition of exception member 'l'
CaseInsensitive.ice:91: exception member 'L' differs only in capitalization from exception member 'l'
CaseInsensitive.ice:91: 'L' has changed meaning
CaseInsensitive.ice:111: exception member 'l' is already defined in a base exception
CaseInsensitive.ice:116: exception member 'L' differs only in capitalization from exception member 'l', which is defined in a base exception
CaseInsensitive.ice:122: redefinition of struct member 'l'
CaseInsensitive.ice:128: member 'L' differs only in capitalization from member 'l'
CaseInsensitive.ice:128: 'L' has changed meaning
CaseInsensitive.ice:144: sequence 'LS' differs only in capitalization from sequence 'ls'
CaseInsensitive.ice:145: redefinition of module 'm1' as sequence
CaseInsensitive.ice:146: sequence 'M1' differs only in capitalization from module 'm1'
CaseInsensitive.ice:149: dictionary 'D' differs only in capitalization from dictionary 'd'
CaseInsensitive.ice:150: redefinition of module 'm1' as dictionary
CaseInsensitive.ice:151: dictionary 'M1' differs only in capitalization from module 'm1'
CaseInsensitive.ice:154: enumeration 'eN1' differs only in capitalization from enumeration 'en1'
CaseInsensitive.ice:155: redefinition of module 'm1' as enumeration
CaseInsensitive.ice:156: enumeration 'M1' differs only in capitalization from module 'm1'
CaseInsensitive.ice:157: enumerator 'EN1' differs only in capitalization from 'en1'
CaseInsensitive.ice:172: interface name 'base' is capitalized inconsistently with its previous name: '::Test::xxx::xx::Base'
CaseInsensitive.ice:172: redefinition of interface 'Derived'
CaseInsensitive.ice:179: exception name 'E1' is capitalized inconsistently with its previous name: '::Test::xxx::xx::e1'
CaseInsensitive.ice:186: sequence name '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:188: sequence name 'xxx::XX::s1' is capitalized inconsistently with its previous name: '::Test::xxx::xx::s1'
CaseInsensitive.ice:189: sequence name 'xxx::XX::s1' is capitalized inconsistently with its previous name: '::Test::xxx::xx::s1'
CaseInsensitive.ice:195: 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
5 changes: 5 additions & 0 deletions cpp/test/Slice/errorDetection/CaseInsensitive.ice
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ interface i2
void oP();
}

interface iice
{
void ice_isa();
}

module m1
{}
module m1
Expand Down
11 changes: 6 additions & 5 deletions cpp/test/Slice/errorDetection/DerivedRedefinition.err
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
DerivedRedefinition.ice:16: operation 'op' is already defined as an operation in a base interface
DerivedRedefinition.ice:12: operation 'ice_ping' is already defined as an operation in a base interface
DerivedRedefinition.ice:17: operation 'op' is already defined as an operation in a base interface
DerivedRedefinition.ice:33: operation 'op' is already defined as an operation in a base interface
DerivedRedefinition.ice:37: ambiguous multiple inheritance: 'D3' inherits operation 'op' from two or more unrelated base interfaces
DerivedRedefinition.ice:42: data member 'l' is already defined as a data member in a base class
DerivedRedefinition.ice:44: data member 'l' is already defined as a data member in a base class
DerivedRedefinition.ice:18: operation 'op' is already defined as an operation in a base interface
DerivedRedefinition.ice:34: operation 'op' is already defined as an operation in a base interface
DerivedRedefinition.ice:38: ambiguous multiple inheritance: 'D3' inherits operation 'op' from two or more unrelated base interfaces
DerivedRedefinition.ice:43: data member 'l' is already defined as a data member in a base class
DerivedRedefinition.ice:45: data member 'l' is already defined as a data member in a base class
1 change: 1 addition & 0 deletions cpp/test/Slice/errorDetection/DerivedRedefinition.ice
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ interface Base1
{
void op();
void op2();
void ice_ping();
}

interface Derived1 extends Base1
Expand Down
Loading