[PATCH] D144011: [clang]Fix warning for signed conversion on LP64

2023-02-21 Thread Yaxun Liu via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. yaxunl marked an inline comment as done. Closed by commit rG8cda128c1eff: [clang]Fix warning for signed conversion on LP64 (authored by yaxunl). Herald added a

[PATCH] D144011: [clang]Fix warning for signed conversion on LP64

2023-02-17 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl marked an inline comment as done. yaxunl added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:14296 +if (SourceBT && SourceBT->isInteger() && TargetBT && +TargetBT->isInteger() && MaskRay wrote: > Consider adding an example

[PATCH] D144011: [clang]Fix warning for signed conversion

2023-02-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. The description needs to adjusted to say that this confusing warning is for LP64 systems. It makes sense to match -m32 and GCC. If someone thinks a warning is useful, that can be a separate change. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144011/new/

[PATCH] D144011: [clang]Fix warning for signed conversion

2023-02-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Thanks! Comment at: clang/lib/Sema/SemaChecking.cpp:14296 +if (SourceBT && SourceBT->isInteger() && TargetBT && +TargetBT->isInteger() &&

[PATCH] D144011: [clang]Fix warning for signed conversion

2023-02-15 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 497703. yaxunl added a comment. revised by Fanrui's comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144011/new/ https://reviews.llvm.org/D144011 Files: clang/lib/Sema/SemaChecking.cpp clang/test/Sema/sign-conversion.c Index:

[PATCH] D144011: [clang]Fix warning for signed conversion

2023-02-15 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment. In D144011#4126853 , @MaskRay wrote: > I think it makes sense for `-Wsign-conversion` to not warn for this LP64 > case, like we don't emit a warning for `-m32`. I do not know whether we need > another diagnostic like

[PATCH] D144011: [clang]Fix warning for signed conversion

2023-02-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. I think it makes sense for `-Wsign-conversion` to not warn for this case. I do not know whether we need another diagnostic like `-Wshorten-64-to-32` for this case, but am inclined to no. `-m32` doesn't emit a warning. I wonder whether the newly added condition can be

[PATCH] D144011: [clang]Fix warning for signed conversion

2023-02-14 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment. In D144011#4126553 , @shafik wrote: > If I look at the clang docs for Wconversion > I see it > includes `-Wshorten-64-to-32` which I believe this is a case of.

[PATCH] D144011: [clang]Fix warning for signed conversion

2023-02-14 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added a comment. If I look at the clang docs for Wconversion I see it includes `-Wshorten-64-to-32` which I believe this is a case of. I think maybe the warning needs a better warning for this case? CHANGES SINCE LAST

[PATCH] D144011: [clang]Fix warning for signed conversion

2023-02-14 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl created this revision. yaxunl added reviewers: tra, rsmith, MaskRay, rtrieu. Herald added a project: All. yaxunl requested review of this revision. Currently clang emits warning with -Wconversion for the following code: long foo(long x) { return 1LL