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

Fix UIMarkdown with AuditMode #18221

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

PankajBhojwani
Copy link
Contributor

Summary of the Pull Request

Adds the UIMarkdown library to AuditMode

PR Checklist

  • Closes #xxx
  • Tests added/passed
  • Documentation updated
    • If checked, please file a pull request on our docs repo and link it here: #xxx
  • Schema updated (if necessary)

@@ -92,7 +92,7 @@ WUX::Documents::Paragraph MarkdownToXaml::_CurrentParagraph()
{
_lastParagraph.TextIndent(-WidthOfBulletPoint);
}
_lastParagraph.Margin(WUX::ThicknessHelper::FromLengths(IndentWidth * _indent, 0, 0, 0));
_lastParagraph.Margin(WUX::ThicknessHelper::FromLengths(gsl::narrow<double>(IndentWidth) * _indent, 0, 0, 0));
Copy link
Member

Choose a reason for hiding this comment

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

This probably works, but is most likely not what you actually intended.
What gsl::narrow does is something like that:

let narrowed = (TargetType)original_value;
let converted_back = (OriginalType)narrowed;
if (converted_back != original_value) {
    throw;
}
return narrowed;

In other words, this will throw if there was no accurate representation as a double. This is easy to show with floats, because if you run gsl::narrow<float>(0xffffffff) it'll throw as it'll actually turn into 4.2949673e9 which is nowhere near 4294967295.

The solution is to just use static_cast.

@@ -8,12 +8,12 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned"
template<typename T>
struct property
{
explicit constexpr property(auto&&... args) :
explicit constexpr property(auto&&... args) noexcept :
Copy link
Member

Choose a reason for hiding this comment

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

This should be something like noexcept(std::is_nothrow_constructible_v<T, decltype(args)...>, aka:
𓀀 𓁐 𓁛 𓁼 𓄿 𓆄 𓆑 𓆟 𓆣 𓆭 𓈝 𓊝 𓊩 𓊯𓋑 𓌪 𓌳 𓍯 𓎵

_value{ std::forward<decltype(args)>(args)... } {}

property& operator=(const property& other) = default;

T operator()() const noexcept
T operator()() const noexcept(noexcept(static_cast<T>(_value)))
Copy link
Member

Choose a reason for hiding this comment

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

I believe this should be std::is_nothrow_copy_constructible, but I think it would be even more ideal for us to just return const T& here and leave the unconditional noexcept.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants