-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
[IR] Changed RT Info nodes naming (compatible with an old) #25159
Conversation
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.
Provide "forward" compatibility for newly generated IRs except cases when it violates XML rules.
c0ea146
to
2cf4424
Compare
Co-authored-by: Pawel Raasz <pawel.raasz@intel.com>
Co-authored-by: Pawel Raasz <pawel.raasz@intel.com>
Co-authored-by: Pawel Raasz <pawel.raasz@intel.com>
Co-authored-by: Pawel Raasz <pawel.raasz@intel.com>
if (name.length() == 0) { | ||
return false; | ||
} | ||
if (!std::all_of(name.begin(), name.end(), [](const int c) { | ||
return std::isalnum(c) || (c == '_') || (c == '-') || (c == '.'); | ||
})) { | ||
return false; | ||
} |
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.
if (name.length() == 0) { | |
return false; | |
} | |
if (!std::all_of(name.begin(), name.end(), [](const int c) { | |
return std::isalnum(c) || (c == '_') || (c == '-') || (c == '.'); | |
})) { | |
return false; | |
} | |
if (!std::all_of(name.begin(), name.end(), [](const int c) { | |
return std::isalnum(c) || (c == '_') || (c == '-') || (c == '.'); | |
})) { | |
return false; | |
} |
For empty string std::all_of
should return false also.
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.
You are right, I just prefer explicit check :) I think I'll move it near name[0] access, just to prevent issues in case of reordering in the future. If this build fails - I'll apply, if ok - change as a separate pr
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, std::all_of
for an empty sequence returns true
.
…oolkit#25159) ### Details: - Fixed an xml serialization/deserialization in case name isn't xml-compatible ### Tickets: - 131514 --------- Co-authored-by: Pawel Raasz <pawel.raasz@intel.com>
Details:
Tickets: