Page MenuHomePhabricator

[Sema] Allow creating types with multiple of the same addrspace.
ClosedPublic

Authored by ebevhan on Jun 1 2018, 6:18 AM.

Details

Summary

The comment with the OpenCL clause about this clearly
says: "No type shall be qualified by qualifiers for
two or more different address spaces."

This must mean that two or more qualifiers for the
_same_ address space is allowed. However, it is
likely unintended by the programmer, so emit a
warning.

For dependent address space types, reject them like
before since we cannot know what the address space
will be.

Diff Detail

Repository
rC Clang

Event Timeline

ebevhan created this revision.Jun 1 2018, 6:18 AM
bader added a subscriber: bader.Jun 1 2018, 7:24 AM
bader added inline comments.
test/Sema/address_spaces.c
17

I think it might be valuable to give a warning or remark to user.
Using the same address space qualifier multiple times is not something OpenCL C developers are supposed to do.

ebevhan added inline comments.Jun 1 2018, 8:06 AM
test/Sema/address_spaces.c
17

The test is obviously a bit contrived, but it could happen by mistake, or as a result of some typedef or macro combination. It also cannot go wrong, so there's no harm in it happening.

I see your point, though. A warning feels like a bit much, so I'm not sure what else to use. A note?

Anastasia added inline comments.Jun 5 2018, 9:12 AM
test/Sema/address_spaces.c
17

Just checked for const qualifier we get a warning:

warning: duplicate 'const' declaration specifier [-Wduplicate-decl-specifier]

We could do the same... not sure if we could try to share the diagnostic as well.

ebevhan added inline comments.Jun 7 2018, 2:04 AM
test/Sema/address_spaces.c
17

I have a patch ready that adds a warning in the same warning group and uses that. I'm not sure about reusing the other one since address spaces don't have to be declaration specifiers.

The warning is 'multiple identical address spaces specified for type', similar to the error. Is that acceptable?

bader added inline comments.Jun 7 2018, 9:18 AM
test/Sema/address_spaces.c
17

Sounds good to me. Could you share your patch, please?

ebevhan updated this revision to Diff 150683.Jun 11 2018, 12:49 AM
ebevhan edited the summary of this revision. (Show Details)

Added a warning for identical address spaces.

Anastasia accepted this revision.Jun 15 2018, 9:23 AM

LGTM! Thanks!

This revision is now accepted and ready to land.Jun 15 2018, 9:23 AM

Thanks! I do not have commit access, so it would be great if someone could commit this.

This revision was automatically updated to reflect the committed changes.