-
Notifications
You must be signed in to change notification settings - Fork 592
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
Use local static const / constexpr for a strings #1840
Use local static const / constexpr for a strings #1840
Conversation
C << nl << "::std::pair<const ::std::string*, const ::std::string*> r = " | ||
<< "::std::equal_range(" << flatName << ", " << flatName << " + " << allOpNames.size() | ||
C << sp; | ||
C << nl << "static constexpr ::std::string_view allOperations[] = "; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's nicer to use a constexpr string_view since it gets initialized at a compile-time. Lots of statics increase start-up time.
@@ -107,10 +107,6 @@ class Gen | |||
bool visitClassDefStart(const ClassDefPtr&) final; | |||
bool visitExceptionStart(const ExceptionPtr&) final; | |||
|
|||
// TODO: temporary - move to InterfaceVisitor. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing these functions was the main goal of this PR.
@@ -2334,11 +2286,14 @@ Slice::Gen::ProxyVisitor::visitOperation(const OperationPtr& p) | |||
C << inParamsImplDecl << ("const " + getUnqualified("::Ice::Context&", interfaceScope) + " context"); | |||
C << epar << " const"; | |||
C << sb; | |||
// TODO: switch to string_view and constexpr. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switching to string_view requires some work in OutgoingAsync. Maybe it could use a string_view too.
C.spar("{ "); | ||
for (auto typeId : p->ids()) | ||
{ | ||
C << '"' + typeId + '"'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be clear to use <<
instead of +
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this doesn't work. Each "<<" inserts a comma.
cpp/src/slice2cpp/Gen.cpp
Outdated
for (auto opName : allOpNames) | ||
{ | ||
C << '"' + opName + '"'; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
This PR updates the generated code to use local static const / constexpr for the list of operations and ids used by the generated code.
It also updates the server-side implementation of ice_isA: there is now a single implementation (in Ice::Object) that calls ice_ids(). It's less efficient than the old implementation but simpler - and if the efficiency of ice_isA matters to your app, something is very wrong with this app.