- User Since
- Apr 9 2020, 11:45 AM (28 w, 4 d)
Fri, Oct 23
Looks great to me; thanks!
Wed, Oct 21
There may be path separators with UNIX assumptions in the runtime I/O support library, too.
Mon, Oct 19
Why is this a fatal error, rather than a warning? If the TARGET= argument lacks both POINTER and TARGET attributes, the result will always be false, and we can warn about that.
Fri, Oct 16
Please add a comment that lays out our reasoning for the prohibition against combining PARAMETER and POINTER, since the standards do not explicitly preclude it. (The PARAMETER attribute requires a entity-decl to have an initialization that is a constant-expr, and the only form of initialization that allows a constant-expr is the one that's not a "=>" pointer initialization.) See C811, C807, & 8.5.13.
Thu, Oct 15
LGTM and thanks!
Wed, Oct 14
Please see https://reviews.llvm.org/D89435 for a proposed alternative patch.
I think a better fix would be to generalize ToUInt64 as a template elsewhere. I'll prepare a patch and add you as the reviewer.
Thu, Oct 8
There's a lot of work here and it's quite well done; thank you.
Wed, Oct 7
Tue, Oct 6
Rebased to current master branch.
Mon, Oct 5
This works, but maybe the memcpy should be in a constructor for BinaryFloatingPointNumber -- it already has a memcpy in its default constructor, after an assertion that the size is not smaller (which would be removed).
See D88752, which avoids the needless usage of the native floating types.
A much smaller work-around: extend the sole constructor of Restorer to accept a reference and a value, and then modify ScopedSet to construct its result as part of the return statement. Restorer doesn't need move construction or move assignment for its clients.
Fri, Oct 2
Thu, Oct 1
The other specialization of this template is a class, though, so maybe the best fix is to change the earlier template struct to a class so that they all are so. I've got a fix on the way that does that, I think.
This change broke the build with clang.
Wed, Sep 30
Previous update to this review had inadvertent changes to other files because I neglected to rebase after updating master; now fixed. Sorry for the scare!
Worked around clang error noticed by code reviewer, & added
a "const" suggested by the clang-tidy bot.
Tue, Sep 29
Is there no access to 80-bit extended precision at all in MSVC?
If the magic MSVC option works around this compiler bug, and perhaps others, it seems to me like a cleaner way to avoid their problems.
Your "analogous conversion" from uint32 to int64 can't lose information, though.