Re: [PATCH] D79504: [Clang] Wrong return type of atomic_is_lock_free

Name: Kamlesh Kumar Email: kamleshbha...@gmail.com On Mon, May 11, 2020, 6:22 PM Louis Dionne via Phabricator < revi...@reviews.llvm.org> wrote: > ldionne added a comment. > > What name and email do you want this committed with? > > > CHANGES SINCE LAST ACTION >

[PATCH] D79504: [Clang] Wrong return type of atomic_is_lock_free

This revision was automatically updated to reflect the committed changes. Closed by commit rG9aee35bcc90f: [Clang] Fix the incorrect return type of atomic_is_lock_free (authored by kamleshbhalui, committed by ldionne). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D79504: [Clang] Wrong return type of atomic_is_lock_free

ldionne added a comment. I'll commit this since I think everyone was fine with it. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79504/new/ https://reviews.llvm.org/D79504 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D79504: [Clang] Wrong return type of atomic_is_lock_free

kamleshbhalui added a comment. In D79504#2029267 , @ldionne wrote: > What name and email do you want this committed with? Kamlesh Kumar kamleshbha...@gmail.com CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79504/new/

[PATCH] D79504: [Clang] Wrong return type of atomic_is_lock_free

ldionne added a comment. What name and email do you want this committed with? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79504/new/ https://reviews.llvm.org/D79504 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D79504: [Clang] Wrong return type of atomic_is_lock_free

kamleshbhalui added a comment. I do not have commit access. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79504/new/ https://reviews.llvm.org/D79504 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D79504: [Clang] Wrong return type of atomic_is_lock_free

ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. Herald added a subscriber: dexonsmith. This change LGTM, libc++ will keep working as-is since we assume the builtins return `bool` (I checked). CHANGES SINCE LAST ACTION

[PATCH] D79504: [Clang] Wrong return type of atomic_is_lock_free

kamleshbhalui updated this revision to Diff 262518. kamleshbhalui added a comment. Addressed @rsmith concerns. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79504/new/ https://reviews.llvm.org/D79504 Files: clang/include/clang/Basic/Builtins.def clang/test/CodeGen/atomic-ops.c

[PATCH] D79504: [Clang] Wrong return type of atomic_is_lock_free

jfb added a comment. In D79504#2023274 , @rsmith wrote: > Clang's `` uses the builtin as follows: > > #define atomic_is_lock_free(obj) __c11_atomic_is_lock_free(sizeof(*(obj))) > > > ... which, combined with the builtin returning `int`, results in a

[PATCH] D79504: [Clang] Wrong return type of atomic_is_lock_free

rsmith added a comment. Clang's `` uses the builtin as follows: #define atomic_is_lock_free(obj) __c11_atomic_is_lock_free(sizeof(*(obj))) ... which, combined with the builtin returning `int`, results in a call having the wrong type. So there's definitely a bug *somewhere*. I think what

[PATCH] D79504: [Clang] Wrong return type of atomic_is_lock_free

jfb added a comment. Just a guess, but maybe the return type was intended to match what a C implementation does with `atomic_is_lock_free` as a macro: https://en.cppreference.com/w/c/atomic/atomic_is_lock_free If so, maybe the expectation is that a macro could define `atomic_is_lock_free`

[PATCH] D79504: [Clang] Wrong return type of atomic_is_lock_free

kamleshbhalui created this revision. kamleshbhalui added reviewers: aaron.ballman, rsmith. Herald added subscribers: cfe-commits, jfb. Herald added a project: clang. kamleshbhalui added a reviewer: rnk. Fixing the return type of atomic_is_lock_free as per