[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

jhuber6 added a comment. In D58346#4359631 , @ebevhan wrote: > In D58346#4359291 , @jhuber6 wrote: > >> should C++ really be limited by OpenCL here? > > It probably shouldn't. There's many places in the codebase whe

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

ebevhan added a comment. In D58346#4359291 , @jhuber6 wrote: > should C++ really be limited by OpenCL here? It probably shouldn't. There's many places in the codebase where OpenCL flags restrict generic address space behavior. I have a patch at D62574 <

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

jhuber6 added a comment. Herald added a subscriber: arichardson. Herald added a project: All. Ran into this change trying to decay a qualifier pointer to a generic address space, e.g. https://godbolt.org/z/3dEd4TxjW. I understand that `addrspace_cast` was added to replace this functionality, but

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

david-salinas added a comment. This change has caused a regression for generic C++. Should this affect OpenCL only? The following simple test causes the diagnostic to be emitted: #define ATTR_GLOBAL __attribute__((address_space(1))) int calc(int ATTR_GLOBAL *ip) { int i = *ip; return i+1

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

This revision was automatically updated to reflect the committed changes. Closed by commit rL355609: [Sema] Change addr space diagnostics in casts to follow C++ style. (authored by stulova, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to com

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Thanks, LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58346/new/ https://reviews.llvm.org/D58346 ___ cfe-commits mailing list

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

Anastasia updated this revision to Diff 189474. Anastasia added a comment. Restructure code with early return. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58346/new/ https://reviews.llvm.org/D58346 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaCast.cpp test/Cod

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

rjmccall added inline comments. Comment at: lib/Sema/SemaCast.cpp:2293 + // define overlapping address spaces currently. + if (Self.getLangOpts().OpenCL) { +auto SrcType = SrcExpr.get()->getType(); Please structure this as an early exit. CHANGES SINCE LAS

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

Anastasia added a comment. In D58346#1417983 , @rjmccall wrote: > In the abstract, it would be nice to warn about casts that always have UB > because the address spaces don't overlap; however, I'm worried about how > people might be using address spaces

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

Anastasia updated this revision to Diff 189361. Anastasia added a comment. Updates FiXME explaining why C++ mode is more permissive. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58346/new/ https://reviews.llvm.org/D58346 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/S

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

rjmccall added a comment. In the abstract, it would be nice to warn about casts that always have UB because the address spaces don't overlap; however, I'm worried about how people might be using address spaces in strange ways today, knowing that they *do* overlap, only in ways that the compiler

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

Anastasia added a comment. In D58346#1413863 , @rjmccall wrote: > Hmm. Yeah, Embedded C allows these casts, so contra my previous comment, I > think we can't make them ill-formed outside of OpenCL mode. Ok, would these rules apply in regular C99 mode t

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

rjmccall added a comment. Hmm. Yeah, Embedded C allows these casts, so contra my previous comment, I think we can't make them ill-formed outside of OpenCL mode. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58346/new/ https://reviews.llvm.org/D58346

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

Anastasia marked an inline comment as done. Anastasia added inline comments. Comment at: lib/Sema/SemaCast.cpp:2309 +auto DestPointeeTypeWithoutAS = Self.Context.removeAddrSpaceQualType( +DestPointeeType.getCanonicalType()); +return Self.Context.hasSameType(SrcPoi

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

ebevhan added inline comments. Comment at: lib/Sema/SemaCast.cpp:2309 +auto DestPointeeTypeWithoutAS = Self.Context.removeAddrSpaceQualType( +DestPointeeType.getCanonicalType()); +return Self.Context.hasSameType(SrcPointeeTypeWithoutAS, Anastasia

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

Anastasia marked an inline comment as done. Anastasia added inline comments. Comment at: lib/Sema/SemaCast.cpp:2309 +auto DestPointeeTypeWithoutAS = Self.Context.removeAddrSpaceQualType( +DestPointeeType.getCanonicalType()); +return Self.Context.hasSameType(SrcPoi

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

Anastasia updated this revision to Diff 188577. Anastasia added a comment. - Added a comment to explain OpenCL check. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58346/new/ https://reviews.llvm.org/D58346 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaCast.cpp t

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

rjmccall added a comment. One comment, but otherwise LGTM. Comment at: lib/Sema/SemaCast.cpp:2309 +auto DestPointeeTypeWithoutAS = Self.Context.removeAddrSpaceQualType( +DestPointeeType.getCanonicalType()); +return Self.Context.hasSameType(SrcPointeeTypeWithoutA

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

Anastasia updated this revision to Diff 187573. Anastasia added a comment. Added a CodeGen test to cover address space of reference in `reinterpret_cast`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58346/new/ https://reviews.llvm.org/D58346 Files: include/clang/Basic/DiagnosticSem

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

ebevhan added inline comments. Comment at: lib/Sema/SemaCast.cpp:2224 + } else if (IsLValueCast) { Kind = CK_LValueBitCast; } else if (DestType->isObjCObjectPointerType()) { Anastasia wrote: > ebevhan wrote: > > Anastasia wrote: > > > Anastasia wrote: >

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

Anastasia marked an inline comment as done. Anastasia added inline comments. Comment at: lib/Sema/SemaCast.cpp:2224 + } else if (IsLValueCast) { Kind = CK_LValueBitCast; } else if (DestType->isObjCObjectPointerType()) { ebevhan wrote: > Anastasia wrote:

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

Anastasia marked an inline comment as done. Anastasia added inline comments. Comment at: lib/Sema/SemaCast.cpp:2224 + } else if (IsLValueCast) { Kind = CK_LValueBitCast; } else if (DestType->isObjCObjectPointerType()) { Anastasia wrote: > ebevhan wrote:

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

ebevhan added inline comments. Comment at: lib/Sema/SemaCast.cpp:2224 + } else if (IsLValueCast) { Kind = CK_LValueBitCast; } else if (DestType->isObjCObjectPointerType()) { Anastasia wrote: > ebevhan wrote: > > Anastasia wrote: > > > ebevhan wrote: > >

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

Anastasia marked an inline comment as done. Anastasia added inline comments. Comment at: lib/Sema/SemaCast.cpp:2309 +auto DestPointeeTypeWithoutAS = Self.Context.removeAddrSpaceQualType( +DestPointeeType.getCanonicalType()); +return Self.Context.hasSameType(SrcPoi

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

Anastasia marked an inline comment as done. Anastasia added inline comments. Comment at: lib/Sema/SemaCast.cpp:2224 + } else if (IsLValueCast) { Kind = CK_LValueBitCast; } else if (DestType->isObjCObjectPointerType()) { ebevhan wrote: > Anastasia wrote:

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

ebevhan added inline comments. Comment at: lib/Sema/SemaCast.cpp:2224 + } else if (IsLValueCast) { Kind = CK_LValueBitCast; } else if (DestType->isObjCObjectPointerType()) { Anastasia wrote: > ebevhan wrote: > > This might not be applicable to this patch

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

Anastasia marked 2 inline comments as done. Anastasia added inline comments. Comment at: lib/Sema/SemaCast.cpp:2224 + } else if (IsLValueCast) { Kind = CK_LValueBitCast; } else if (DestType->isObjCObjectPointerType()) { ebevhan wrote: > This might not be

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

ebevhan added inline comments. Comment at: lib/Sema/SemaCast.cpp:2224 + } else if (IsLValueCast) { Kind = CK_LValueBitCast; } else if (DestType->isObjCObjectPointerType()) { This might not be applicable to this patch, but just something I noticed. So `r

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

Anastasia created this revision. Anastasia added reviewers: rjmccall, ebevhan. Herald added a subscriber: jdoerfert. This patch adds a new diagnostic for mismatching address spaces to be used for C++ casts (TODO: only enabled in C style cast for now, the rest will follow!). This change extends C