Skip to content

Commit

Permalink
Add rmlui_static_cast to assert validity of down casts, see #514
Browse files Browse the repository at this point in the history 
Employ it around the library for extra debug checks (only with rtti enabled).
  • Loading branch information
@mikke89
mikke89 committed Sep 22, 2023
1 parent eeb380a commit 70d0d5e
Show file tree
Hide file tree
Showing 9 changed files with 33 additions and 25 deletions.
16 changes: 16 additions & 0 deletions Include/RmlUi/Core/Traits.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#define RMLUI_CORE_TRAITS_H

#include "../Config/Config.h"
#include "Debug.h"
#include "Header.h"
#include <type_traits>

Expand Down Expand Up @@ -136,6 +137,13 @@ Derived rmlui_dynamic_cast(Base base_instance)
return nullptr;
}

template <class Derived, class Base>
Derived rmlui_static_cast(Base base_instance)
{
static_assert(std::is_pointer<Derived>::value && std::is_pointer<Base>::value, "rmlui_static_cast can only cast pointer types");
return static_cast<Derived>(base_instance);
}

template <class T>
const char* rmlui_type_name(const T& /*var*/)
{
Expand All @@ -162,6 +170,14 @@ Derived rmlui_dynamic_cast(Base base_instance)
return dynamic_cast<Derived>(base_instance);
}

template <class Derived, class Base>
Derived rmlui_static_cast(Base base_instance)
{
static_assert(std::is_pointer<Derived>::value && std::is_pointer<Base>::value, "rmlui_static_cast can only cast pointer types");
RMLUI_ASSERT(dynamic_cast<Derived>(base_instance));
return static_cast<Derived>(base_instance);
}

template <class T>
const char* rmlui_type_name(const T& var)
{
Expand Down
10 changes: 5 additions & 5 deletions Samples/basic/demo/src/main.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class DemoWindow : public Rml::EventListener {
document->GetElementById("title")->SetInnerRML(title);

// Add sandbox default text.
if (auto source = static_cast<Rml::ElementFormControl*>(document->GetElementById("sandbox_rml_source")))
if (auto source = rmlui_dynamic_cast<Rml::ElementFormControl*>(document->GetElementById("sandbox_rml_source")))

This comment has been minimized.

Sign in to view

Copy link
@doctorames

doctorames Sep 25, 2023

Did you mean to change this from static to dynamic cast? Instead of rmlui_static_cast()?
Same for line 103.

This comment has been minimized.

Sign in to view

Copy link
@mikke89

mikke89 Sep 25, 2023

Author Owner

I intentionally changed it to dynamic cast. The thinking is, since we already handle non-existing ID here, we might as well handle the wrong type as well. I felt that was more consistent here, rather than just doing a the debug assertion that we would get from rmlui_static_cast.

I appreciate you looking over the changes.
{
auto value = source->GetValue();
value += "<p>Write your RML here</p>\n\n<!-- <img src=\"assets/high_scores_alien_1.tga\"/> -->";
Expand Down Expand Up @@ -100,7 +100,7 @@ class DemoWindow : public Rml::EventListener {
}

// Add sandbox style sheet text.
if (auto source = static_cast<Rml::ElementFormControl*>(document->GetElementById("sandbox_rcss_source")))
if (auto source = rmlui_dynamic_cast<Rml::ElementFormControl*>(document->GetElementById("sandbox_rcss_source")))
{
Rml::String value = "/* Write your RCSS here */\n\n/* body { color: #fea; background: #224; }\nimg { image-color: red; } */";
source->SetValue(value);
Expand Down Expand Up @@ -332,7 +332,7 @@ class DemoEventListener : public Rml::EventListener {
}
else if (value == "tween_duration")
{
float value = (float)std::atof(static_cast<Rml::ElementFormControl*>(element)->GetValue().c_str());
float value = (float)std::atof(rmlui_static_cast<Rml::ElementFormControl*>(element)->GetValue().c_str());
tweening_parameters.duration = value;
if (auto el_duration = element->GetElementById("duration"))
el_duration->SetInnerRML(CreateString(20, "%2.2f", value));
Expand Down Expand Up @@ -381,15 +381,15 @@ class DemoEventListener : public Rml::EventListener {
}
else if (value == "set_sandbox_body")
{
if (auto source = static_cast<Rml::ElementFormControl*>(element->GetElementById("sandbox_rml_source")))
if (auto source = rmlui_dynamic_cast<Rml::ElementFormControl*>(element->GetElementById("sandbox_rml_source")))
{
auto value = source->GetValue();
demo_window->SetSandboxBody(value);
}
}
else if (value == "set_sandbox_style")
{
if (auto source = static_cast<Rml::ElementFormControl*>(element->GetElementById("sandbox_rcss_source")))
if (auto source = rmlui_dynamic_cast<Rml::ElementFormControl*>(element->GetElementById("sandbox_rcss_source")))
{
auto value = source->GetValue();
demo_window->SetSandboxStylesheet(value);
Expand Down
2 changes: 1 addition & 1 deletion Source/Core/Context.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ ElementDocument* Context::LoadDocument(Stream* stream)
if (!element)
return nullptr;

ElementDocument* document = static_cast<ElementDocument*>(element.get());
ElementDocument* document = rmlui_static_cast<ElementDocument*>(element.get());

root->AppendChild(std::move(element));

Expand Down
16 changes: 5 additions & 11 deletions Source/Core/DataViewDefault.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -391,18 +391,12 @@ bool DataViewText::Update(DataModel& model)
{
if (Element* element = GetElement())
{
RMLUI_ASSERTMSG(rmlui_dynamic_cast<ElementText*>(element),
"Somehow the element type was changed from ElementText since construction of the view. Should not be possible?");
String new_text = BuildText();
String text;
if (SystemInterface* system_interface = GetSystemInterface())
system_interface->TranslateString(text, new_text);

if (ElementText* text_element = static_cast<ElementText*>(element))
{
String new_text = BuildText();

String text;
if (SystemInterface* system_interface = GetSystemInterface())
system_interface->TranslateString(text, new_text);
text_element->SetText(text);
}
rmlui_static_cast<ElementText*>(element)->SetText(text);
}
else
{
Expand Down
2 changes: 1 addition & 1 deletion Source/Core/ElementDocument.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ void ElementDocument::ReloadStyleSheet()
return;
}

SetStyleSheetContainer(static_cast<ElementDocument*>(temp_doc.get())->style_sheet_container);
SetStyleSheetContainer(rmlui_static_cast<ElementDocument*>(temp_doc.get())->style_sheet_container);
}

void ElementDocument::DirtyMediaQueries()
Expand Down
2 changes: 1 addition & 1 deletion Source/Core/ElementInstancer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ ElementPtr ElementInstancerText::InstanceElement(Element* /*parent*/, const Stri

void ElementInstancerText::ReleaseElement(Element* element)
{
pool_text_default.DestroyAndDeallocate(static_cast<ElementText*>(element));
pool_text_default.DestroyAndDeallocate(rmlui_static_cast<ElementText*>(element));
}

} // namespace Rml
2 changes: 1 addition & 1 deletion Source/Core/Layout/BlockContainer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ InlineContainer* BlockContainer::GetOpenInlineContainer()
const InlineContainer* BlockContainer::GetOpenInlineContainer() const
{
if (!child_boxes.empty() && child_boxes.back()->GetType() == Type::InlineContainer)
return static_cast<InlineContainer*>(child_boxes.back().get());
return rmlui_static_cast<InlineContainer*>(child_boxes.back().get());
return nullptr;
}

Expand Down
4 changes: 1 addition & 3 deletions Source/Core/Layout/InlineLevelBox.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -227,8 +227,6 @@ String InlineLevelBox_Text::DebugDumpNameValue() const

ElementText* InlineLevelBox_Text::GetTextElement()
{
RMLUI_ASSERT(rmlui_dynamic_cast<ElementText*>(GetElement()));

return static_cast<ElementText*>(GetElement());
return rmlui_static_cast<ElementText*>(GetElement());
}
} // namespace Rml
4 changes: 2 additions & 2 deletions Source/Core/Layout/LineBox.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ bool LineBox::AddBox(InlineLevelBox* box, InlineLayoutMode layout_mode, LayoutOv
case FragmentType::InlineBox:
{
RMLUI_ASSERT(constructor.layout_width < 0.f);
RMLUI_ASSERT(rmlui_dynamic_cast<InlineBox*>(box));
RMLUI_ASSERT(rmlui_static_cast<InlineBox*>(box));

open_fragments_leaf = fragment_index;
open_spacing_left += box->GetSpacingLeft();
Expand Down Expand Up @@ -389,7 +389,7 @@ InlineBox* LineBox::GetOpenInlineBox()
if (open_fragments_leaf == RootFragmentIndex)
return nullptr;

return static_cast<InlineBox*>(fragments[open_fragments_leaf].box);
return rmlui_static_cast<InlineBox*>(fragments[open_fragments_leaf].box);
}

bool LineBox::CanCollapseLine() const
Expand Down

0 comments on commit 70d0d5e

Please sign in to comment.