codemzs added a comment.
@tahonermann I would like to understand your concern better on unordered
floating point types as the callers of `getFloatingTypeOrder` handle this
result as per the C++23 proposal, for example there is a test case that
exercises this scenario: _Float16 f16_val_1 = 1.0bf16; // expected-error
{{cannot initialize a variable of type '_Float16' with an rvalue of type
'__bf16'}}
Perhaps if you can please give me an example that further illustrates your
concerns it would help me understand and assuage your concerns. Thank you.
================
Comment at: clang/lib/AST/ASTContext.cpp:191
+ FloatingRankCompareResult::FRCR_Greater,
+ FloatingRankCompareResult::FRCR_Equal}}}};
+
----------------
tahonermann wrote:
> I just realized that none of these comparisons results in
> `FRCR_Equal_Lesser_Subrank` or `FRCR_Equal_Greater_Subrank`. I guess those
> won't actually be used until support for `std::float32_t` and
> `std::float64_t` are added. That means that we don't have a way to test code
> that performs subrank checks though.
That is correct, my plan was to add support for these types after this change
is committed.
================
Comment at: clang/lib/Frontend/InitPreprocessor.cpp:461
+ Builder.defineMacro("__STDCPP_FLOAT16_T__", "1");
+ Builder.defineMacro("__STDCPP_BFLOAT16_T__", "1");
+ }
----------------
tahonermann wrote:
> Should the definition of `__STDCPP_BFLOAT16_T__` be conditional on something
> like `Ctx.getTargetInfo().hasFullBFloat16Type()`?
Based on [our
discussion](https://discourse.llvm.org/t/rfc-c-23-p1467r9-extended-floating-point-types-and-standard-names/70033/34?u=codemzs)
on the RFC it was determined that representing `bfloat16` using `fp32` is ok
as per the author of the proposal, in that case do you believe we should be
making this macro conditional on the target natively supporting `bfloat16`?
================
Comment at: clang/lib/Sema/SemaChecking.cpp:10451
return ICE->getCastKind() == CK_FloatingCast &&
- S.Context.getFloatingTypeOrder(From, To) < 0;
+ S.Context.getFloatingTypeOrder(From, To) == FRCR_Lesser;
}
----------------
tahonermann wrote:
> I'm not sure this is right. If I understand correctly, the C++23 extended FP
> types don't participate in argument promotions. Perhaps they should be
> excluded here.
Rules for
[promotion](https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p1467r9.html#promotion)
are unchanged as per the proposal. This is just using the new enumeration to
represent a smaller conversion rank.
================
Comment at: clang/test/Sema/cxx23-fp-ext-std-names-p1467r9.cpp:3-6
+_Float16 f16_val_1 = 1.0bf16; // expected-error {{cannot initialize a variable
of type '_Float16' with an rvalue of type '__bf16'}}
+_Float16 f16_val_2 = 1.0f; // expected-error {{cannot initialize a variable of
type '_Float16' with an rvalue of type 'float'}}
+_Float16 f16_val_3 = 1.0; // expected-error {{cannot initialize a variable of
type '_Float16' with an rvalue of type 'double'}}
+_Float16 f16_val_4 = 1.0l; // expected-error {{cannot initialize a variable of
type '_Float16' with an rvalue of type 'long double'}}
----------------
tahonermann wrote:
> Are these errors correct? Since the source is a constant expression that
> specifies a value that is within the representable range of the type, I think
> these initializations are expected to be allowed.
I believe this should be an error unless my understanding is incorrect, below
is what the proposal
[says](https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p1467r9.html#implicit-constant)
for constant expressions:
"A drawback of this part of the proposal is that constant values don’t get any
special treatment. As a result, this code:
`std::float16_t x = 1.0;`
would be ill-formed. The constant 1.0 has type double, which cannot be
implicitly converted to type std::float16_t, even though the value 1.0 can be
represented exactly in both types. To compile, the code must have an explicit
cast, or, preferably, use a literal suffix:
`std::float16_t x = 1.0f16;`
...."
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D149573/new/
https://reviews.llvm.org/D149573
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
