aaron.ballman added a comment.
I have some suggested changes for the way we're checking and diagnosing
unsuitable types; I've not tried my suggestions out though, so if you run into
problems, please let me know.
================
Comment at: clang/include/clang/AST/Type.h:2159
bool isEnumeralType() const;
+ bool isPureIntegerType() const;
----------------
I'd rather put this logic into SemaChecking.cpp than expose it in Type.h. This
isn't a type predicate most other code should need to use.
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8616-8625
def err_overflow_builtin_must_be_int : Error<
"operand argument to overflow builtin must be an integer (%0 invalid)">;
def err_overflow_builtin_must_be_ptr_int : Error<
"result argument to overflow builtin must be a pointer "
"to a non-const integer (%0 invalid)">;
def err_overflow_builtin_bit_int_max_size : Error<
"__builtin_mul_overflow does not support 'signed _BitInt' operands of more "
----------------
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8624-8625
"than %0 bits">;
+def warn_overflow_builtin_can_not_be_short : Warning<
+ "'short' may not be suitable to hold the result of operating two 'int's ">;
def err_expected_struct_pointer_argument : Error<
----------------
There's a few things that aren't correct here, so I'm suggesting a different
approach, but for your own understanding:
* All warning diagnostics need to be added to a warning group, using
`InGroup<>`.
* The warning text is very specific to only one problematic situation; it
should be generalized to cover as many code patterns as it can.
* We can likely reuse the existing error (`err_overflow_builtin_must_be_int`)
instead of making a new one.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:388
+ }
+
// First two arguments should be integers.
----------------
================
Comment at: clang/lib/Sema/SemaChecking.cpp:396-401
+ bool IsInt = CkdOperation ? Ty->isPureIntegerType() : Ty->isIntegerType();
+ if (!IsInt) {
S.Diag(Arg.get()->getBeginLoc(), diag::err_overflow_builtin_must_be_int)
<< Ty << Arg.get()->getSourceRange();
return true;
}
----------------
================
Comment at: clang/lib/Sema/SemaChecking.cpp:414-431
if (!PtrTy ||
!PtrTy->getPointeeType()->isIntegerType() ||
+ (!PtrTy->getPointeeType()->isPureIntegerType() && CkdOperation) ||
PtrTy->getPointeeType().isConstQualified()) {
S.Diag(Arg.get()->getBeginLoc(),
diag::err_overflow_builtin_must_be_ptr_int)
<< Ty << Arg.get()->getSourceRange();
----------------
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157331/new/
https://reviews.llvm.org/D157331
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
