[PATCH] D50278: [Sema] Fix for crash on conditional operation with address_space pointer

This revision was automatically updated to reflect the committed changes. Closed by commit rL339167: [Sema] Fix for crash on conditional operation with address_space pointer (authored by leonardchan, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D50278: [Sema] Fix for crash on conditional operation with address_space pointer

This revision was automatically updated to reflect the committed changes. Closed by commit rC339167: [Sema] Fix for crash on conditional operation with address_space pointer (authored by leonardchan, committed by ). Repository: rL LLVM https://reviews.llvm.org/D50278 Files:

[PATCH] D50278: [Sema] Fix for crash on conditional operation with address_space pointer

rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. LGTM. Comment at: lib/Sema/SemaExpr.cpp:6522 +bool HasDifferingLAddrSpace = LAddrSpace != ResultAddrSpace; +bool HasDifferingRAddrSpace = RAddrSpace !=

[PATCH] D50278: [Sema] Fix for crash on conditional operation with address_space pointer

rjmccall added a comment. In https://reviews.llvm.org/D50278#1190544, @ebevhan wrote: > I think the solution to a lot of diagnostic and behavior issues with address > spaces is to remove predication on OpenCL. It's a bit odd to have a language > feature that is enabled out of the box

[PATCH] D50278: [Sema] Fix for crash on conditional operation with address_space pointer

leonardchan updated this revision to Diff 159559. leonardchan marked an inline comment as done. leonardchan added a comment. - Removed checks for OpenCL in `checkConditionalPointerCompatibility`. This allows for the error `err_typecheck_op_on_nonoverlapping_address_space_pointers` to be dumped

[PATCH] D50278: [Sema] Fix for crash on conditional operation with address_space pointer

leonardchan added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6943 +def err_typecheck_incompatible_conditional_pointer_operands : Error< + "unable to find common type between %0 and %1 for conditional operation">; ebevhan wrote: >

[PATCH] D50278: [Sema] Fix for crash on conditional operation with address_space pointer

ebevhan added a comment. I think the solution to a lot of diagnostic and behavior issues with address spaces is to remove predication on OpenCL. It's a bit odd to have a language feature that is enabled out of the box regardless of langoptions (address spaces) but won't actually work properly

[PATCH] D50278: [Sema] Fix for crash on conditional operation with address_space pointer

rjmccall added inline comments. Comment at: lib/Sema/SemaExpr.cpp:6522 +bool HasDifferingLAddrSpace = LAddrSpace != ResultAddrSpace; +bool HasDifferingRAddrSpace = RAddrSpace != ResultAddrSpace; + I was going to tell you to use the predicate

[PATCH] D50278: [Sema] Fix for crash on conditional operation with address_space pointer

leonardchan updated this revision to Diff 159405. leonardchan marked an inline comment as done. leonardchan added a comment. - Replaced instances of a `pointer type mismatch` warning involving 2 conditional operands with different address spaces with a new error specifically for this situation.

[PATCH] D50278: [Sema] Fix for crash on conditional operation with address_space pointer

rjmccall added inline comments. Comment at: test/Sema/conditional-expr.c:78 + // expected-error@-1{{converting '__attribute__((address_space(2))) int *' to type 'void *' changes address space of pointer}} + //

[PATCH] D50278: [Sema] Fix for crash on conditional operation with address_space pointer

leonardchan added a comment. In https://reviews.llvm.org/D50278#1189919, @rjmccall wrote: > I would expect this to replace the existing warning, not to appear together > with it. Will do. Comment at: test/Sema/conditional-expr.c:78 + //

[PATCH] D50278: [Sema] Fix for crash on conditional operation with address_space pointer

rjmccall added inline comments. Comment at: test/Sema/conditional-expr.c:78 + // expected-error@-1{{converting '__attribute__((address_space(2))) int *' to type 'void *' changes address space of pointer}} + //

[PATCH] D50278: [Sema] Fix for crash on conditional operation with address_space pointer

rjmccall added a comment. I would expect this to replace the existing warning, not to appear together with it. Repository: rC Clang https://reviews.llvm.org/D50278 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D50278: [Sema] Fix for crash on conditional operation with address_space pointer

leonardchan updated this revision to Diff 159315. leonardchan added reviewers: ebevhan, rjmccall. leonardchan removed a subscriber: ebevhan. leonardchan added a comment. - Changed diff such that an error is dumped instead. The code shouldn't compile in the first place since it involves

[PATCH] D50278: [Sema] Fix for crash on conditional operation with address_space pointer

ebevhan added a comment. When I try the test case on our downstream (and when compiling for our target with an extra flag that enables a bunch of OpenCL-related address space code), I get: ascrash.c:5:12: error: comparison between ('__attribute__((address_space(1))) char *' and

[PATCH] D50278: [Sema] Fix for crash on conditional operation with address_space pointer

leonardchan created this revision. leonardchan added reviewers: phosek, mcgrathr, rsmith. leonardchan added a project: clang. Compiling the following causes clang to crash void cmp(char *x, __attribute__((address_space(2))) char *y) { 0 ? x : y; } with the message: "wrong cast for