aaron.ballman added a comment.

In D125919#3554233 <https://reviews.llvm.org/D125919#3554233>, @rjmccall wrote:

> In D125919#3554195 <https://reviews.llvm.org/D125919#3554195>, @aaron.ballman 
> wrote:
>
>> However, I reverted the changes in this patch in 
>> c745f2ce6c03bc6d1e59cac69cc15923d4400191 
>> <https://reviews.llvm.org/rGc745f2ce6c03bc6d1e59cac69cc15923d4400191> 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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125919/new/

https://reviews.llvm.org/D125919

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to