Drop qualifiers from return types in C (DR423)

[aaron.ballman]

Summary

WG14 DR423 (https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2148.htm#dr_423), resolved during the C11 time frame, changed the way qualifiers are handled on function return types and in cast expressions after it was noted that these types are now directly observable via generic selection expressions. In C, the function declarator is adjusted to ignore all qualifiers (including _Atomic qualifiers).

Clang already handles the cast expression case correctly (by performing the lvalue conversion, which drops the qualifiers as well), but with these changes it will now also handle function declarations appropriately.

Fixes #39595

[erichkeane]

clang/test/Sema/warn-missing-prototypes.c

61

Hrmph, this is unfortunate...

[aaron.ballman]

Adding some Apple reviewers because this touches ObjC behavior (I would be very surprised if ObjC wanted different rules here than C though).

clang/test/Sema/warn-missing-prototypes.c

61

It is, but it didn't seem like a sufficiently common situation to warrant trying to fix it.

[rjmccall]

We definitely don't want ObjC to differ here; thanks for the CC.

[aaron.ballman]

Fixes failed AST matching unit test and speculatively fixes a CodeGen test, all found via precommit CI testing.

[aaron.ballman]

In D125919#3523441, @rjmccall wrote:

We definitely don't want ObjC to differ here; thanks for the CC.

Excellent, thank you for the confirmation!

[delcypher]

@aaron.ballman Hey I just saw this change and had questions about it. For others looking I think the resolution to DR423 is in https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1863.pdf, I found https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2148.htm#dr_423 hard to parse.

1. It looks like we're completely dropping qualifiers on the return types of function in C. If that's the case, what's even the point of having qualifiers on return types of functions?
2. Reading DR423 I see the problem but I don't understand why the desire to make _Generic work better in the presence of qualifiers means that qualifiers on function return types have to be dropped in all contexts. Couldn't this just be a thing that is done inside _Generic and no where else?

Sorry if these are silly questions and if I've misunderstood something, I saw n1863 say "functions return unqualified types" and I was very surprised.

[aaron.ballman]

In D125919#3554078, @delcypher wrote:

@aaron.ballman Hey I just saw this change and had questions about it. For others looking I think the resolution to DR423 is in https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1863.pdf, I found https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2148.htm#dr_423 hard to parse.

1. It looks like we're completely dropping qualifiers on the return types of function in C. If that's the case, what's even the point of having qualifiers on return types of functions?

As best I can tell, there is no point to having a qualified return type for a function in C because it is effectively unobservable, which is likely why qualifiers are removed from the function return type per spec. C2x 6.7.3p5 says "The properties associated with qualified types are meaningful only for expressions that are lvalues." and a function's returned value is never an lvalue in C. I say "effectively" unobservable because there can still be language extensions that make the return type more observable.

2. Reading DR423 I see the problem but I don't understand why the desire to make _Generic work better in the presence of qualifiers means that qualifiers on function return types have to be dropped in all contexts. Couldn't this just be a thing that is done inside _Generic and no where else?

The reason qualifiers are to be dropped is because of C2x 6.7.6.3p4, which was what was updated by DR423 to say: "If, in the declaration "T D1", D1 has the form D ( parameter-type-list_opt ) attribute-specifier-sequence_opt and the type specified for ident in the declaration "T D" is "derived-declarator-type-list T", then the type specified for ident is "derived-declarator-type-list function returning the unqualified version of T". Note how the type specified for the function is returning the unqualified version of T. This will impact more than just _Generic; for example __builtin_types_compatible_p() is also affected: https://godbolt.org/z/1bhcvhv3M

However, I reverted the changes in this patch in c745f2ce6c03bc6d1e59cac69cc15923d4400191 as I don't think they're correct. I've got some open questions on the WG14 reflectors regarding this function type rewriting exercise, but I think my dropping of the _Atomic qualifier is likely wrong in this patch and the fact that we're losing source fidelity in the AST is definitely an issue. The subject of https://github.com/llvm/llvm-project/issues/39595 was the behavior of the _Atomic specifier in a _Generic selection; when I realized we don't fully implement DR423, I mistakenly connected the issue with the DR. But now that I no longer think the _Atomic qualifier should be dropped as I was doing, these changes really don't address the issue in _Generic.

Sorry if these are silly questions and if I've misunderstood something, I saw n1863 say "functions return unqualified types" and I was very surprised.

These are not at all silly questions, so thank you for asking them!

[rjmccall]

In D125919#3554078, @delcypher wrote:

@aaron.ballman Hey I just saw this change and had questions about it. For others looking I think the resolution to DR423 is in https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1863.pdf, I found https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2148.htm#dr_423 hard to parse.

1. It looks like we're completely dropping qualifiers on the return types of function in C. If that's the case, what's even the point of having qualifiers on return types of functions?

I think that's the point: qualifiers don't mean anything on return types, so they shouldn't be represented in the type. If the user writes a qualifier directly on the return type, like const int twice(int x);, we should warn about that; otherwise they should be silently dropped.

2. Reading DR423 I see the problem but I don't understand why the desire to make _Generic work better in the presence of qualifiers means that qualifiers on function return types have to be dropped in all contexts. Couldn't this just be a thing that is done inside _Generic and no where else?

Sorry if these are silly questions and if I've misunderstood something, I saw n1863 say "functions return unqualified types" and I was very surprised.

Just to be clear, you understand that this is only about top-level qualifiers on the return type, right? const void *foo(); is still meaningful, it's just that const void * const foo(); isn't.

[rjmccall]

In D125919#3554195, @aaron.ballman wrote:

However, I reverted the changes in this patch in c745f2ce6c03bc6d1e59cac69cc15923d4400191 as I don't think they're correct. I've got some open questions on the WG14 reflectors regarding this function type rewriting exercise, but I think my dropping of the _Atomic qualifier is likely wrong in this patch and the fact that we're losing source fidelity in the AST is definitely an issue. The subject of https://github.com/llvm/llvm-project/issues/39595 was the behavior of the _Atomic specifier in a _Generic selection; when I realized we don't fully implement DR423, I mistakenly connected the issue with the DR. But now that I no longer think the _Atomic qualifier should be dropped as I was doing, these changes really don't address the issue in _Generic.

The source fidelity issue seems like it could be solved with AdjustedType. What's the issue with _Generic?

[aaron.ballman]

In D125919#3554233, @rjmccall wrote:

In D125919#3554195, @aaron.ballman wrote:

However, I reverted the changes in this patch in c745f2ce6c03bc6d1e59cac69cc15923d4400191 as I don't think they're correct. I've got some open questions on the WG14 reflectors regarding this function type rewriting exercise, but I think my dropping of the _Atomic qualifier is likely wrong in this patch and the fact that we're losing source fidelity in the AST is definitely an issue. The subject of https://github.com/llvm/llvm-project/issues/39595 was the behavior of the _Atomic specifier in a _Generic selection; when I realized we don't fully implement DR423, I mistakenly connected the issue with the DR. But now that I no longer think the _Atomic qualifier should be dropped as I was doing, these changes really don't address the issue in _Generic.

The source fidelity issue seems like it could be solved with AdjustedType. What's the issue with _Generic?

We get this wrong:

_Atomic int f(void);

int main(void) {
  _Generic(f(), int: 0); // Error, no matching association
}

The controlling expression has to undergo lvalue conversion, which drops the _Atomic qualifier per C2x 6.3.2.1p2, so we should match the only association rather than error. We do perform the lvalue conversion: https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaExpr.cpp#L1650 but for whatever reason, we're not matching its type to the association type.

[rjmccall]

Ah, yeah, that seems unrelated (and surprising).

You think we're *not* supposed to drop _Atomic in a return type per C2x? That seems surprising; I thought it was generally treated as a qualifier and would expect that we'd be supposed to drop it.

[aaron.ballman]

In D125919#3554310, @rjmccall wrote:

Ah, yeah, that seems unrelated (and surprising).

You think we're *not* supposed to drop _Atomic in a return type per C2x? That seems surprising; I thought it was generally treated as a qualifier and would expect that we'd be supposed to drop it.

My first inclination was that it gets dropped as well because it's a qualifier and surely "unqualified" means drop all qualifiers... but the standard makes it far less clear (alas).

6.2.5p29 (types):
"Further, there is the _Atomic qualifier. The presence of the _Atomic qualifier designates an atomic
type. The size, representation, and alignment of an atomic type need not be the same as those of
the corresponding unqualified type. Therefore, this document explicitly uses the phrase "atomic,
qualified, or unqualified type" whenever the atomic version of a type is permitted along with the
other qualified versions of a type. The phrase "qualified or unqualified type", without specific
mention of atomic, does not include the atomic types."

Isn't that a gem? :-)

6.3.2.1p2 (lvalue conversion):
"... If the lvalue has qualified type, the value has the unqualified
version of the type of the lvalue; additionally, if the lvalue has
atomic type, the value has the non-atomic version of the type of the
lvalue. ..."

Which also gives a hint that _Atomic is special in terms of the phrase "unqualified".

[rjmccall]

Ah, true, the standard doesn't describe it as a normal qualifier. From the DR, it sounds like the committee certainly expected that this reasoning would also apply to _Atomic, even if that's not quite what they've written, but that the DR submitter seems to not want that behavior. Have you been able to track down the solicited paper mentioned in that DR? In the absence of further indication, I think dropping _Atomic like anything else is the right thing to do; like the other qualifiers, it isn't meaningful for r-values.

[aaron.ballman]

In D125919#3554599, @rjmccall wrote:

Ah, true, the standard doesn't describe it as a normal qualifier. From the DR, it sounds like the committee certainly expected that this reasoning would also apply to _Atomic, even if that's not quite what they've written, but that the DR submitter seems to not want that behavior. Have you been able to track down the solicited paper mentioned in that DR? In the absence of further indication, I think dropping _Atomic like anything else is the right thing to do; like the other qualifiers, it isn't meaningful for r-values.

I think it's less clear to me that the committee expected to drop _Atomic because the standard basically makes _Atomic introduce a new type rather than a qualified use like const; it's more akin to _Complex in that regard. e.g., it's not clear to me that these are redeclarations:

int func(void);
_Atomic(int) func(void);

When I survey non-Clang C compilers which support _Atomic, it's evenly split between thinking these are valid redeclarations or not (GCC and ICC say they're not valid redeclarations, ICC and TCC say they are) but when I use const int and int as the return types, only ICC claims they're incompatible types.

Further, this seems like a gratuitous incompatibility with C++ where the qualifiers are *not* dropped, given that function call expressions explicitly drop the qualifiers anyway. (So you're right, they are effectively useless on the return type, but why should C and C++ differ in what's a valid redeclaration?)

[rjmccall]

Are you now arguing that C was wrong to drop qualifiers in return types and so we shouldn't implement that rule, or...? Because your argument is much broader than just _Atomic.

I don't understand the analogy you're making to _Complex at all. Values of type _Complex T are fundamentally different from values of type T, because inherently they have to store two of them. _Imaginary (which Clang does not implement) is more or less just a semantic recast of T, but that recast is still critically important to the nature of the type, and in no way does it act like a qualifier. Meanwhile, _Atomic is only meaningful at the top level on l-values, exactly like a qualifier, and you can assign non-atomic r-values into an atomic l-value and read them back out again, just like a qualifier.

Now, the standard has chosen not to talk about _Atomic as a qualifier, I assume because there's a fair number of rules in the standard that assume that qualifiers can be freely added to pointers and don't change layout and so on, none of which apply to _Atomic. But those rules also don't apply to a large number of other extended qualifiers, like address spaces and the ARC ownership qualifiers and __ptrauth. The committee should probably just come to terms with the fact that it's the relatively easy-come-easy-go nature of the CVR qualifiers which is the special case. I've thought for awhile that Clang should really be representing _Atomic as a qualifier instead of having to treat AtomicType as a special case in a million places.

I see absolutely no reason why anybody should ever want to declare a function as _Atomic(int) func(void);, and if the C and C++ committees decide to break people who aren't just doing it but are doing it inconsistently, they're doing the world a service.

[delcypher]

@aaron.ballman

Sorry if these are silly questions and if I've misunderstood something, I saw n1863 say "functions return unqualified types" and I was very surprised.

These are not at all silly questions, so thank you for asking them!

Thanks for taking the time to answer my questions. Your answers make sense to me apart from the derived-declarator-type-list. I'm not sure what that is, the copy of the standard I found doesn't seem seem to define what it is. I see other people have complained about this in the past (https://stackoverflow.com/questions/13779273/in-the-standard-what-is-derived-declarator-type) so I'll guess I'll stare at that for a while until it makes sense.

[aaron.ballman]

In D125919#3556319, @rjmccall wrote:

Are you now arguing that C was wrong to drop qualifiers in return types and so we shouldn't implement that rule, or...? Because your argument is much broader than just _Atomic.

That's my current thinking, but I'm still considering the ramifications. But it seems to me that dropping qualifications on lvalue to rvalue conversion, which happens for function returns anyways, is sufficient to have resolved DR423.

I'm quite peeved that:

int func(void);
const int func(void);

is valid C but invalid C++; that seems gratuitously incompatible in light of the lvalue to rvalue behavior.

I don't understand the analogy you're making to _Complex at all. Values of type _Complex T are fundamentally different from values of type T, because inherently they have to store two of them. _Imaginary (which Clang does not implement) is more or less just a semantic recast of T, but that recast is still critically important to the nature of the type, and in no way does it act like a qualifier. Meanwhile, _Atomic is only meaningful at the top level on l-values, exactly like a qualifier, and you can assign non-atomic r-values into an atomic l-value and read them back out again, just like a qualifier.

I disagree with the assessment that _Atomic behaves exactly like a qualifier. It *should* (IMHO), but it doesn't. C allows the size and alignment of an atomic type be *different* from its unqualified version, to allow for embedded locks and such. Because the size and alignment can be different, to my mind, _Atomic int and int are different types in the same way as _Complex float and float -- the object representations can (must, in the case of _Complex) be different, so they're different types. The same is not true for const, volatile, or restrict qualifiers -- those are required to have the same size and alignment as the unqualified type.

Now, the standard has chosen not to talk about _Atomic as a qualifier, I assume because there's a fair number of rules in the standard that assume that qualifiers can be freely added to pointers and don't change layout and so on, none of which apply to _Atomic. But those rules also don't apply to a large number of other extended qualifiers, like address spaces and the ARC ownership qualifiers and __ptrauth. The committee should probably just come to terms with the fact that it's the relatively easy-come-easy-go nature of the CVR qualifiers which is the special case. I've thought for awhile that Clang should really be representing _Atomic as a qualifier instead of having to treat AtomicType as a special case in a million places.

I'm not certain if we can get away with that. IIRC, Microsoft uses embedded locks on Windows in some circumstances. However, cl doesn't support _Atomic and so maybe we can get away with it on the assumption we have no other targets where the size/alignment would be different for an atomic type vs a non-atomic type?

I see absolutely no reason why anybody should ever want to declare a function as _Atomic(int) func(void);, and if the C and C++ committees decide to break people who aren't just doing it but are doing it inconsistently, they're doing the world a service.

I tend to agree -- top-level qualification on a return type is deeply strange in C. However, it's something observable in C++ due to template metaprogramming and type traits, so I don't see C++ changing the type system to disallow it as C as done. Given that C++ is unlikely to move the needle, it seems more plausible to move it in C where top-level qualification of a return type is really hard to observe except through redeclaration errors.

[delcypher]

@rjmccall

Sorry if these are silly questions and if I've misunderstood something, I saw n1863 say "functions return unqualified types" and I was very surprised.

Just to be clear, you understand that this is only about top-level qualifiers on the return type, right? const void *foo(); is still meaningful, it's just that const void * const foo(); isn't.

Oh that wasn't obvious to me at all. I had assumed "unqualified types" means remove qualifiers from everywhere in the type, not just the top level. Presumably something was meant to imply that?

[aaron.ballman]

In D125919#3556405, @delcypher wrote:

@aaron.ballman

Sorry if these are silly questions and if I've misunderstood something, I saw n1863 say "functions return unqualified types" and I was very surprised.

These are not at all silly questions, so thank you for asking them!

Thanks for taking the time to answer my questions. Your answers make sense to me apart from the derived-declarator-type-list. I'm not sure what that is, the copy of the standard I found doesn't seem seem to define what it is. I see other people have complained about this in the past (https://stackoverflow.com/questions/13779273/in-the-standard-what-is-derived-declarator-type) so I'll guess I'll stare at that for a while until it makes sense.

:-D It took me a while to find the answer when I was looking at that yesterday, but it's defined in C2x 6.2.5p26 (without the -list part, that's added because there could be multiple declarators for the same T, as in const int f(void), g(int);).

[rsmith]

In D125919#3556418, @aaron.ballman wrote:

In D125919#3556319, @rjmccall wrote:

Now, the standard has chosen not to talk about _Atomic as a qualifier, I assume because there's a fair number of rules in the standard that assume that qualifiers can be freely added to pointers and don't change layout and so on, none of which apply to _Atomic. But those rules also don't apply to a large number of other extended qualifiers, like address spaces and the ARC ownership qualifiers and __ptrauth. The committee should probably just come to terms with the fact that it's the relatively easy-come-easy-go nature of the CVR qualifiers which is the special case. I've thought for awhile that Clang should really be representing _Atomic as a qualifier instead of having to treat AtomicType as a special case in a million places.

I'm not certain if we can get away with that. IIRC, Microsoft uses embedded locks on Windows in some circumstances. However, cl doesn't support _Atomic and so maybe we can get away with it on the assumption we have no other targets where the size/alignment would be different for an atomic type vs a non-atomic type?

That assumption does not hold. Given struct A { char c[3]; };, struct A has size 3 and align 1, but _Atomic struct A has size 4 and align 4 across many (perhaps all?) of our targets. (This is an ABI divergence between GCC and Clang, which as far as I know the psABI owners have so far not succeeded in resolving.)

I saw some talk in WG14 of separating _Atomic(T) and _Atomic T so that _Atomic T would be a qualifier that doesn't affect size or alignment, only what code is generated to access the value, and _Atomic(T) would be a distinct type with potentially a different representation in order to support lock-free atomic access. Did that go anywhere? (I'd assume not, it seems to be at least a decade too late for such an idea.)

[jyknight]

In D125919#3556754, @rsmith wrote:

That assumption does not hold. Given struct A { char c[3]; };, struct A has size 3 and align 1, but _Atomic struct A has size 4 and align 4 across many (perhaps all?) of our targets. (This is an ABI divergence between GCC and Clang, which as far as I know the psABI owners have so far not succeeded in resolving.)

And while GCC indeed never changes the size (that is: it never introduces padding), it, too, increases the alignment when the size was already appropriately-aligned.

That is, given struct A { char c[4]; };, struct A has size 4 and align 1, but _Atomic struct A has size 4 and align 4, under both GCC and Clang.

[rjmccall]

I disagree with the assessment that _Atomic behaves exactly like a qualifier. It *should* (IMHO), but it doesn't. C allows the size and alignment of an atomic type be *different* from its unqualified version, to allow for embedded locks and such. Because the size and alignment can be different, to my mind, _Atomic int and int are different types in the same way as _Complex float and float -- the object representations can (must, in the case of _Complex) be different, so they're different types. The same is not true for const, volatile, or restrict qualifiers -- those are required to have the same size and alignment as the unqualified type.

I think our apparent disagreement here is rooted in something quite simple: you seem to be assuming that a qualified type by definition cannot have a different size and alignment from the unqualified type, and I think that's a property that's *guaranteed* for the CVR qualifiers and merely *happens to be true* for our current set of extended qualifiers, and only really because we don't represent _Atomic as a qualifier.

Many of our extended qualifiers completely change the ABI of the type they qualify, and they can change the basic semantic properties of types they're nested within. I don't see why we'd draw a line around type layout for what counts as a qualifier and what doesn't. For example, ObjC ARC happens to implement __weak references without using extra inline storage, but it would certainly be a reasonable ABI choice to make them larger than just a pointer — Swift does with its native weak references, for example. That ABI choice wouldn't make __weak suddenly not a qualifier. Type constructors like _Atomic, _Complex, const, and * are qualifiers or not based on the role and behavior of the constructed type in the language, not its ABI properties.

I saw some talk in WG14 of separating _Atomic(T) and _Atomic T so that _Atomic T would be a qualifier that doesn't affect size or alignment, only what code is generated to access the value, and _Atomic(T) would be a distinct type with potentially a different representation in order to support lock-free atomic access. Did that go anywhere? (I'd assume not, it seems to be at least a decade too late for such an idea.)

That seems like a terrible idea.

[aaron.ballman]

In D125919#3556754, @rsmith wrote:

In D125919#3556418, @aaron.ballman wrote:

In D125919#3556319, @rjmccall wrote:

Now, the standard has chosen not to talk about _Atomic as a qualifier, I assume because there's a fair number of rules in the standard that assume that qualifiers can be freely added to pointers and don't change layout and so on, none of which apply to _Atomic. But those rules also don't apply to a large number of other extended qualifiers, like address spaces and the ARC ownership qualifiers and __ptrauth. The committee should probably just come to terms with the fact that it's the relatively easy-come-easy-go nature of the CVR qualifiers which is the special case. I've thought for awhile that Clang should really be representing _Atomic as a qualifier instead of having to treat AtomicType as a special case in a million places.

I'm not certain if we can get away with that. IIRC, Microsoft uses embedded locks on Windows in some circumstances. However, cl doesn't support _Atomic and so maybe we can get away with it on the assumption we have no other targets where the size/alignment would be different for an atomic type vs a non-atomic type?

That assumption does not hold. Given struct A { char c[3]; };, struct A has size 3 and align 1, but _Atomic struct A has size 4 and align 4 across many (perhaps all?) of our targets. (This is an ABI divergence between GCC and Clang, which as far as I know the psABI owners have so far not succeeded in resolving.)

Good point!

I saw some talk in WG14 of separating _Atomic(T) and _Atomic T so that _Atomic T would be a qualifier that doesn't affect size or alignment, only what code is generated to access the value, and _Atomic(T) would be a distinct type with potentially a different representation in order to support lock-free atomic access. Did that go anywhere? (I'd assume not, it seems to be at least a decade too late for such an idea.)

So far that's just been talk (and I'm not convinced it'll be possible to change the semantics there without silently breaking code).

In D125919#3558062, @rjmccall wrote:

I disagree with the assessment that _Atomic behaves exactly like a qualifier. It *should* (IMHO), but it doesn't. C allows the size and alignment of an atomic type be *different* from its unqualified version, to allow for embedded locks and such. Because the size and alignment can be different, to my mind, _Atomic int and int are different types in the same way as _Complex float and float -- the object representations can (must, in the case of _Complex) be different, so they're different types. The same is not true for const, volatile, or restrict qualifiers -- those are required to have the same size and alignment as the unqualified type.

I think our apparent disagreement here is rooted in something quite simple: you seem to be assuming that a qualified type by definition cannot have a different size and alignment from the unqualified type, and I think that's a property that's *guaranteed* for the CVR qualifiers and merely *happens to be true* for our current set of extended qualifiers, and only really because we don't represent _Atomic as a qualifier.

I think you're right about the root of our disagreement. Any qualifiers other than CVR, _Atomic, and address spaces are fully implementation-defined and can do whatever they want, thus aren't really relevant to how standard qualifiers behave. Address spaces are covered by TR 18037 (which states they're extending the list of CVR qualifiers in C, thus I'd expect would follow the same rules as CVR despite predating DR423). We treat all standards-based qualifiers as a Qualifier in Clang, except for _Atomic, which I believe we treat differently because it behaves as a distinct type. I think we *want* that behavior -- these are not valid redeclarations: void func(int a); void func(_Atomic int a); while these are valid (and have utility): void func(int a); void func(const int a);.

Many of our extended qualifiers completely change the ABI of the type they qualify, and they can change the basic semantic properties of types they're nested within. I don't see why we'd draw a line around type layout for what counts as a qualifier and what doesn't. For example, ObjC ARC happens to implement __weak references without using extra inline storage, but it would certainly be a reasonable ABI choice to make them larger than just a pointer — Swift does with its native weak references, for example. That ABI choice wouldn't make __weak suddenly not a qualifier. Type constructors like _Atomic, _Complex, const, and * are qualifiers or not based on the role and behavior of the constructed type in the language, not its ABI properties.

For our implementation-defined qualifiers, I think the design space is wide open and we can do what we want. However, I think _Atomic specifically is more closely related to a new type than a qualified type for the other standards-based qualifiers. The C standard goes out of its way to segregate atomic types from other types but doesn't do anything similar for any other qualifier.

All that said, I think you can see why I'm hoping to get an answer from WG14 as to what to do. Reasonable folks are disagreeing on what the standard requires here.

[aaron.ballman]

In D125919#3560523, @aaron.ballman wrote:

All that said, I think you can see why I'm hoping to get an answer from WG14 as to what to do. Reasonable folks are disagreeing on what the standard requires here.

The discussion on the WG14 reflector seems to be settling down to a consensus position that the _Atomic qualifier is only syntactically a qualifier and its use designates an entirely new type. When the standard says "unqualified type", the _Atomic is not considered a qualification. So we should *not* be stripping the _Atomic as I was doing in this patch. (SC22WG14.22200 has most of the details spelled out nicely, if you have access to the reflectors.)

I had asked other questions in related areas that also got answers.

const void func(volatile void); -- the return type is adjusted to void; the parameter type is UB (by lack of specification) and we can do what we want here. We currently diagnose the parameter as being invalid: https://godbolt.org/z/9c8bTrerY. Our behavior with the parameter is consistent with GCC and EDG.

const int main(void) -- this is valid and equivalent to int main(void), so it should be accepted; we currently reject: https://godbolt.org/z/v43h596ev

const int func(void); int func(void) { } -- this is DR423. It is valid, the composite type is int(void); we give a conflicting types error: https://godbolt.org/z/Yb841r7Ex

[rjmccall]

In D125919#3569928, @aaron.ballman wrote:

In D125919#3560523, @aaron.ballman wrote:

All that said, I think you can see why I'm hoping to get an answer from WG14 as to what to do. Reasonable folks are disagreeing on what the standard requires here.

The discussion on the WG14 reflector seems to be settling down to a consensus position that the _Atomic qualifier is only syntactically a qualifier and its use designates an entirely new type. When the standard says "unqualified type", the _Atomic is not considered a qualification. So we should *not* be stripping the _Atomic as I was doing in this patch. (SC22WG14.22200 has most of the details spelled out nicely, if you have access to the reflectors.)

What a strange position. *All* qualifiers produce an "entirely new type". But okay, if the committee wants to pretend that _Atomic is meaningful in return values, I guess we all have to live with that.

I had asked other questions in related areas that also got answers.

const void func(volatile void); -- the return type is adjusted to void; the parameter type is UB (by lack of specification) and we can do what we want here. We currently diagnose the parameter as being invalid: https://godbolt.org/z/9c8bTrerY. Our behavior with the parameter is consistent with GCC and EDG.

const int main(void) -- this is valid and equivalent to int main(void), so it should be accepted; we currently reject: https://godbolt.org/z/v43h596ev

const int func(void); int func(void) { } -- this is DR423. It is valid, the composite type is int(void); we give a conflicting types error: https://godbolt.org/z/Yb841r7Ex

Is that only because of the type compositing rules, or are they saying that the type of the first is const int (void), which is different from int (void)?

[aaron.ballman]

In D125919#3570145, @rjmccall wrote:

In D125919#3569928, @aaron.ballman wrote:

In D125919#3560523, @aaron.ballman wrote:

All that said, I think you can see why I'm hoping to get an answer from WG14 as to what to do. Reasonable folks are disagreeing on what the standard requires here.

The discussion on the WG14 reflector seems to be settling down to a consensus position that the _Atomic qualifier is only syntactically a qualifier and its use designates an entirely new type. When the standard says "unqualified type", the _Atomic is not considered a qualification. So we should *not* be stripping the _Atomic as I was doing in this patch. (SC22WG14.22200 has most of the details spelled out nicely, if you have access to the reflectors.)

What a strange position. *All* qualifiers produce an "entirely new type". But okay, if the committee wants to pretend that _Atomic is meaningful in return values, I guess we all have to live with that.

lol, right? :-D

I had asked other questions in related areas that also got answers.

const void func(volatile void); -- the return type is adjusted to void; the parameter type is UB (by lack of specification) and we can do what we want here. We currently diagnose the parameter as being invalid: https://godbolt.org/z/9c8bTrerY. Our behavior with the parameter is consistent with GCC and EDG.

const int main(void) -- this is valid and equivalent to int main(void), so it should be accepted; we currently reject: https://godbolt.org/z/v43h596ev

const int func(void); int func(void) { } -- this is DR423. It is valid, the composite type is int(void); we give a conflicting types error: https://godbolt.org/z/Yb841r7Ex

Is that only because of the type compositing rules, or are they saying that the type of the first is const int (void), which is different from int (void)?

Quoting from the reflector question I asked,

> Question 4
> Also along the same thinking, what is the composite type of int
> f(int); and const int f(const int);? Is it before or after adjustment?
> If before adjustment, we don't seem to say anything in 6.2.7p3 about
> the parameter type but p1 then suggests the types are not compatible
> in the first place; if after adjustment, the composite type would be
> int(int). There's significant compiler divergence here: Clang, GCC,
> ppci, cc65, and TCC consider after adjusting both parameters and
> return types, while EDG, Tendra, cproc, SDCC, and MSVC consider after
> adjusting parameter types but not return types.

According to the normative text, the qualifiers on the return type never form
part of the type of the function at all.  I expect implementations doing otherwise
simply failed to implement the resolution to DR#423.

For parameter types, I think it's generally understood that qualifiers don't affect the
function type (so the types are *the same type*, for the purposes of handling the
rules on duplicate typedef declarations, not just *compatible types*), but all the
wording actually says is that the qualifiers are disregarded "In the determination of
type compatibility and of a composite type", leaving open the possibility that they
form part of the function type but don't affect type compatibility or composite types.
In any case the composite type of the two types you list must be int (int) because of
the inclusion of "composite type" in that wording, even if you could form a case for
int (const int) and int (int) being different (compatible) types.

I take this as meaning we *could* make int func(void); and const int func(void); have different types, but that seems like a pretty strange thing to do and we should probably just make them have the same types (so then there aren't special rules for compatibility and compositing for them).

[rjmccall]

Yeah, that makes sense. Thanks for following that up.

clang/docs/ReleaseNotes.rst

- The driver no longer adds ``-mignore-xcoff-visibility`` by default for AIX
   ignoring hidden visibility can silently have undesirable side effects (e.g
   when libraries depend on visibility to hide non-ABI facing entities). The
   ``-mignore-xcoff-visibility`` option can be manually specified on the
   command-line to recover the previous behavior if desired.
 
 
 C Language Changes in Clang
 ---------------------------
+- Finished implementing support for DR423. We already correctly handled
+  stripping qualifiers from cast expressions, but we did not strip qualifiers
+  on function return types. We now properly treat the function as though it
+  were declarated with an unqualified, non-atomic return type. Fixes
+  `Issue 39595 <https://github.com/llvm/llvm-project/issues/39595>`_.
 
 C2x Feature Support
 -------------------
 
 - Implemented `WG14 N2674 The noreturn attribute <http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2764.pdf>`_.
 - Implemented `WG14 N2935 Make false and true first-class language features <http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2935.pdf>`_.
 - Implemented `WG14 N2763 Adding a fundamental type for N-bit integers <http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2763.pdf>`_.
 - Implemented `WG14 N2775 Literal suffixes for bit-precise integers <http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2775.pdf>`_.

clang/include/clang/Basic/DiagnosticSemaKinds.td

 def note_exits_block_captures_weak : Note<
   "jump exits lifetime of block which weakly captures a variable">;
 def note_exits_block_captures_non_trivial_c_struct : Note<
   "jump exits lifetime of block which captures a C struct that is non-trivial "
   "to destroy">;
 def note_exits_compound_literal_scope : Note<
   "jump exits lifetime of a compound literal that is non-trivial to destruct">;
 
-def err_func_returning_qualified_void : ExtWarn<
-  "function cannot return qualified void type %0">,
-  InGroup<DiagGroup<"qualified-void-return-type">>;
 def err_func_returning_array_function : Error<
   "function cannot return %select{array|function}0 type %1">;
 def err_field_declared_as_function : Error<"field %0 declared as a function">;
 def err_field_incomplete_or_sizeless : Error<
   "field has %select{incomplete|sizeless}0 type %1">;
 def ext_variable_sized_type_in_struct : ExtWarn<
   "field %0 with variable sized type %1 not at the end of a struct or class is"
   " a GNU extension">, InGroup<GNUVariableSizedTypeNotAtEnd>;

clang/lib/Sema/SemaType.cpp

case DeclaratorChunk::Function: {
         D.setInvalidType(true);
       }
 
       // cv-qualifiers on return types are pointless except when the type is a
       // class type in C++.
       if ((T.getCVRQualifiers() || T->isAtomicType()) &&
           !(S.getLangOpts().CPlusPlus &&
             (T->isDependentType() || T->isRecordType()))) {
-        if (T->isVoidType() && !S.getLangOpts().CPlusPlus &&
-            D.getFunctionDefinitionKind() ==
-                FunctionDefinitionKind::Definition) {
-          // [6.9.1/3] qualified void return is invalid on a C
-          // function definition.  Apparently ok on declarations and
-          // in C++ though (!)
-          S.Diag(DeclType.Loc, diag::err_func_returning_qualified_void) << T;
-        } else
-          diagnoseRedundantReturnTypeQualifiers(S, T, D, chunkIndex);
+        // WG14 DR 423 updated 6.7.6.3p4 to have the function declarator drop
+        // all qualifiers from the return type.
+        if (!S.getLangOpts().CPlusPlus)
+          T = T.getAtomicUnqualifiedType();
 
         // C++2a [dcl.fct]p12:
         //   A volatile-qualified return type is deprecated
         if (T.isVolatileQualified() && S.getLangOpts().CPlusPlus20)
           S.Diag(DeclType.Loc, diag::warn_deprecated_volatile_return) << T;
       }
 
       // Objective-C ARC ownership qualifiers are ignored on the function

clang/test/CodeGen/xcore-stringtype.c

 
 // test FunctionType & Qualifiers
 // CHECK: !{{[0-9]+}} = !{void ()* @gI, !"f{0}()"}
 // CHECK: !{{[0-9]+}} = !{void (...)* @eI, !"f{0}()"}
 // CHECK: !{{[0-9]+}} = !{void ()* @gV, !"f{0}(0)"}
 // CHECK: !{{[0-9]+}} = !{void ()* @eV, !"f{0}(0)"}
 // CHECK: !{{[0-9]+}} = !{void (i32, ...)* @gVA, !"f{0}(si,va)"}
 // CHECK: !{{[0-9]+}} = !{void (i32, ...)* @eVA, !"f{0}(si,va)"}
-// CHECK: !{{[0-9]+}} = !{i32* (i32*)* @gQ, !"f{crv:p(cv:si)}(p(cv:si))"}
-// CHECK: !{{[0-9]+}} = !{i32* (i32*)* @eQ, !"f{crv:p(cv:si)}(p(cv:si))"}
+// CHECK: !{{[0-9]+}} = !{i32* (i32*)* @gQ, !"f{p(cv:si)}(p(cv:si))"}
+// CHECK: !{{[0-9]+}} = !{i32* (i32*)* @eQ, !"f{p(cv:si)}(p(cv:si))"}
 extern void eI();
 void gI() {eI();};
 extern void eV(void);
 void gV(void) {eV();}
 extern void eVA(int, ...);
 void gVA(int i, ...) {eVA(i);}
 extern const volatile int* volatile restrict const
     eQ(const volatile int * volatile restrict const);

clang/test/Sema/block-call.c

int main(void) {
 
   const int (^CICC) () = CIC;
 
 
   int * const (^IPCC) () = 0;
 
   int * const (^IPCC1) () = IPCC;
 
-  int * (^IPCC2) () = IPCC;       // expected-error {{incompatible block pointer types initializing 'int *(^)()' with an expression of type 'int *const (^)()'}}
+  int * (^IPCC2) () = IPCC;       // OK per WG14 DR 423 because the 'const' was dropped from the declarator.
 
   int (^IPCC3) (const int) = PFR;
 
   int (^IPCC4) (int, char (^CArg) (double));
 
   int (^IPCC5) (int, char (^CArg) (double)) = IPCC4;
 
   int (^IPCC6) (int, char (^CArg) (float))  = IPCC4; // expected-error {{incompatible block pointer types initializing 'int (^)(int, char (^)(float))' with an expression of type 'int (^)(int, char (^)(double))'}}
 
   IPCC2 = 0;
-  IPCC1 = 1; // expected-error {{invalid block pointer conversion assigning to 'int *const (^)()' from 'int'}}
+  IPCC1 = 1; // expected-error {{invalid block pointer conversion assigning to 'int *(^)()' from 'int'}}
   int (^x)() = 0;
   int (^y)() = 3;   // expected-error {{invalid block pointer conversion initializing 'int (^)()' with an expression of type 'int'}}
   int a = 1;
   int (^z)() = a+4;   // expected-error {{invalid block pointer conversion initializing 'int (^)()' with an expression of type 'int'}}
 }
 
 int blah(void) {
   int (^IFP) (float);
   char (^PCP)(double, double, char);
 
   IFP(1.0);
   IFP (1.0, 2.0); // expected-error {{too many arguments to block call}}
 
   char ch = PCP(1.0, 2.0, 'a');
   return PCP(1.0, 2.0);   // expected-error {{too few arguments to block}}
 }

clang/test/Sema/c89.c

 typedef CI array_of_CI[5];
 const array_of_CI mine2; /* expected-warning {{duplicate 'const' declaration specifier}} */
 
 typedef CI *array_of_pointer_to_CI[5];
 const array_of_pointer_to_CI mine3;
 
 void main(void) {} /* expected-error {{'main' must return 'int'}} */
 
-const int main(void) {} /* expected-error {{'main' must return 'int'}} */
+const int main(void) {} /* OK per DR 423 */
 
 long long ll1 = /* expected-warning {{'long long' is an extension when C99 mode is not enabled}} */
          -42LL; /* expected-warning {{'long long' is an extension when C99 mode is not enabled}} */
 unsigned long long ull1 = /* expected-warning {{'long long' is an extension when C99 mode is not enabled}} */
                    42ULL; /* expected-warning {{'long long' is an extension when C99 mode is not enabled}} */
 
 struct Test17 { int a; };
 struct Test17 test17_aux(void);
 
 void test17(int v, int w) {
   int a[2] = { v, w }; /* expected-warning {{initializer for aggregate is not a compile-time constant}} */
   struct Test17 t0 = { v }; /* expected-warning {{initializer for aggregate is not a compile-time constant}} */
   struct Test17 t1 = test17_aux(); /* this is allowed */
 }
 

clang/test/Sema/function.c

void t22(int *ptr, int (*array)[3]) {
   decays(ptr);   // expected-warning {{incompatible pointer types passing 'int *' to parameter of type 'int (*)[3]'}}
   no_decay(ptr); // expected-warning {{incompatible pointer types passing 'int *' to parameter of type 'int (*)[3]'}}
   decays(array);
   no_decay(array);
 }
 
 void const Bar (void); // ok on decl
 // PR 20146
-void const Bar (void) // expected-warning {{function cannot return qualified void type 'const void'}}
+void const Bar (void) // also okay on defn per DR 423
 {
 }

clang/test/Sema/warn-missing-prototypes.c

 const int *get_const() { // expected-warning{{no previous prototype for function 'get_const'}}
   // expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:1-[[@LINE-2]]:1}:"static "
   return (void *)0;
 }
 
 struct MyStruct {};
 
+// FIXME: because qualifiers are ignored in the return type when forming the
+// type from the declarator, we get the position incorrect for the fix-it hint.
+// It suggests 'const static struct' instead of 'static const struct'. However,
+// thanks to the awful rules of parsing in C, the effect is the same and the
+// code is valid, if a bit funny looking.
 const struct MyStruct get_struct() { // expected-warning{{no previous prototype for function 'get_struct'}}
   // expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:1-[[@LINE-2]]:1}:"static "
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:7-[[@LINE-2]]:7}:"static "
   struct MyStruct ret;
   return ret;
 }
 
 // Turn off clang-format for white-space dependent testing.
 // clang-format off
 // Two spaces between cost and struct
 const  struct MyStruct get_struct_2() {  // expected-warning{{no previous prototype for function 'get_struct_2'}}
   // expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:1-[[@LINE-2]]:1}:"static "
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:8-[[@LINE-2]]:8}:"static "
   struct MyStruct ret;
   return ret;
 }
 
 // Two spaces bewteen const and struct
 const  struct MyStruct* get_struct_ptr() {  // expected-warning{{no previous prototype for function 'get_struct_ptr'}}
   // expected-note@-1{{declare 'static' if the function is not intended to be used outside of this translation unit}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:1-[[@LINE-2]]:1}:"static "

clang/test/Sema/wg14-dr423.c

+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -ast-dump %s | FileCheck %s
+// expected-no-diagnostics
+
+void GH39595(void) {
+  // Ensure that qualifiers on function return types are dropped as part of the
+  // declaration.
+  extern const int const_int(void);
+  // CHECK: FunctionDecl {{.*}} parent {{.*}} <col:3, col:34> col:20 referenced const_int 'int (void)' extern
+  extern _Atomic int atomic(void);
+  // CHECK: FunctionDecl {{.*}} parent {{.*}} <col:3, col:33> col:22 referenced atomic 'int (void)' extern
+
+  (void)_Generic(const_int(), int : 1);
+  (void)_Generic(atomic(), int : 1);
+
+  // Make sure they're dropped from function pointers as well.
+  _Atomic int (*fp)(void);
+  (void)_Generic(fp(), int : 1);
+}
+
+void casting(void) {
+  // Ensure that qualifiers on cast operations are also dropped.
+  (void)_Generic((const int)12, int : 1);
+
+  struct S { int i; } s;
+  (void)_Generic((const struct S)s, struct S : 1);
+
+  int i;
+  __typeof__((const int)i) j;
+  j = 100; // If we didn't strip the qualifiers during the cast, this would err.
+}

clang/test/SemaObjC/block-omitted-return-type.m

- (void)test
   };
   void (^simpleBlock3)(void) = ^ _Nonnull (void) {  //expected-warning {{attribute '_Nonnull' ignored, because it cannot be applied to omitted return type}}
     return;
   };
 
   void (^simpleBlock4)(void) = ^ const { //expected-warning {{'const' qualifier on omitted return type '<dependent type>' has no effect}}
     return;
   };
-  void (^simpleBlock5)(void) = ^ const void { //expected-error {{incompatible block pointer types initializing 'void (^)(void)' with an expression of type 'const void (^)(void)'}}
-    return; // expected-warning@-1 {{function cannot return qualified void type 'const void'}}
+  void (^simpleBlock5)(void) = ^ const void { // OK after DR 423.
+    return;
   };
   void (^simpleBlock6)(void) = ^ const (void) { //expected-warning {{'const' qualifier on omitted return type '<dependent type>' has no effect}}
     return;
   };
   void (^simpleBlock7)(void) = ^ _Nonnull __attribute__((align_value(128))) _Nullable const (void) { // expected-warning {{attribute '_Nullable' ignored, because it cannot be applied to omitted return type}} \
     // expected-warning {{attribute '_Nonnull' ignored, because it cannot be applied to omitted return type}} \
     // expected-warning {{'const' qualifier on omitted return type '<dependent type>' has no effect}} \
     // expected-warning {{'align_value' attribute only applies to variables and typedefs}}

clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp

 }
 
 TEST_P(ASTMatchersTest, QualifiedTypeLocTest_BindsToConstIntVarDecl) {
   EXPECT_TRUE(matches("const int x = 0;",
                       qualifiedTypeLoc(loc(asString("const int")))));
 }
 
 TEST_P(ASTMatchersTest, QualifiedTypeLocTest_BindsToConstIntFunctionDecl) {
-  EXPECT_TRUE(matches("const int f() { return 5; }",
-                      qualifiedTypeLoc(loc(asString("const int")))));
+  StringRef Code = R"(
+    const int f() { return 5; }
+  )";
+  // In C++, the qualified return type is retained.
+  EXPECT_TRUE(matchesConditionally(
+      Code, qualifiedTypeLoc(loc(asString("const int"))), true, langAnyCxx()));
+  // In C, the qualifications on the return type are dropped, so we expect it
+  // to match 'int' rather than 'const int'.
+  EXPECT_TRUE(matchesConditionally(
+      Code, qualifiedTypeLoc(loc(asString("const int"))), false, langAnyC()));
+  EXPECT_TRUE(
+      matchesConditionally(Code, loc(asString("int")), true, langAnyC()));
 }
 
 TEST_P(ASTMatchersTest, QualifiedTypeLocTest_DoesNotBindToUnqualifiedVarDecl) {
   EXPECT_TRUE(notMatches("int x = 0;", qualifiedTypeLoc(loc(asString("int")))));
 }
 
 TEST_P(ASTMatchersTest, QualifiedTypeLocTest_IntDoesNotBindToConstIntDecl) {
   EXPECT_TRUE(

clang/unittests/ASTMatchers/ASTMatchersTest.h

if (FindResultReviewer)
       FindResultReviewer->onEndOfTranslationUnit();
   }
 
 private:
   bool *const Verified;
   const std::unique_ptr<BoundNodesCallback> FindResultReviewer;
 };
 
+inline ArrayRef<TestLanguage> langAnyC() {
+  static const TestLanguage Result[] = {Lang_C89, Lang_C99};
+  return ArrayRef<TestLanguage>(Result);
+}
+
+inline ArrayRef<TestLanguage> langAnyCxx() {
+  static const TestLanguage Result[] = {Lang_CXX03, Lang_CXX11, Lang_CXX14,
+                                        Lang_CXX17, Lang_CXX20};
+  return ArrayRef<TestLanguage>(Result);
+}
+
 inline ArrayRef<TestLanguage> langCxx11OrLater() {
   static const TestLanguage Result[] = {Lang_CXX11, Lang_CXX14, Lang_CXX17,
                                         Lang_CXX20};
   return ArrayRef<TestLanguage>(Result);
 }
 
 inline ArrayRef<TestLanguage> langCxx14OrLater() {
   static const TestLanguage Result[] = {Lang_CXX14, Lang_CXX17, Lang_CXX20};