Skip to content
This repository was archived by the owner on Aug 8, 2023. It is now read-only.

Relax typing for comparison operators - #12537

Merged
anandthakker merged 3 commits into
masterfrom
relax-equality-typing
Aug 2, 2018
Merged

anandthakker merged 3 commits into
masterfrom
relax-equality-typing

Conversation

@anandthakker

Copy link
Copy Markdown
Contributor

@jfirebaugh jfirebaugh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎉

void CollatorComparison::eachChild(const std::function<void(const Expression&)>& visit) const {
visit(*lhs);
visit(*rhs);
if (collator) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is the null check needed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah, nope, it's leftover

if (e.getKind() == Kind::Comparison) {
auto comp = static_cast<const CollatorComparison*>(&e);
return comp->op == op &&
comp->collator && *comp->collator == *collator &&

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is the null check needed?

bool gteqCollate(std::string a, std::string b, Collator c) { return c.compare(a, b) >= 0; }

static BasicComparison::CompareFunctionType getBasicCompareFunction(const std::string& op) {
static std::unordered_map<std::string, const BasicComparison::CompareFunctionType> map {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

unordered_map is somewhat expensive for binary size, and it's probably faster anyway to just cascade if statements.

std::unique_ptr<Expression> lhs_,
std::unique_ptr<Expression> rhs_)
: Expression(Kind::Comparison, type::Boolean),
op(op_),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

std::move(op_)

@anandthakker
anandthakker merged commit d25e1a1 into master Aug 2, 2018
@anandthakker
anandthakker deleted the relax-equality-typing branch August 2, 2018 17:48
@friedbunny friedbunny added Core The cross-platform C++ core, aka mbgl needs changelog Indicates PR needs a changelog entry prior to merging. labels Aug 16, 2018
@1ec5 1ec5 added this to the ios-v4.3.0 milestone Aug 16, 2018
@friedbunny friedbunny removed needs changelog Indicates PR needs a changelog entry prior to merging. labels Sep 20, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Core The cross-platform C++ core, aka mbgl

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants