[PATCH] D125349: [Sema] Fix crash for C11 atomic in gnu++ mode

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

> It doesn't look like the patch handles `volatile _Atomic` correctly, though.

That should be fixed in the new revision.

> I know we do a lot of candidate prefiltering, and that that can be difficult because of uesr-defined conversions.  How do those things interact with qualifiers?

I don't know.

> Like, I notice the existing code is adding both `(T &, T)` and `(volatile T &, T)` when the LHS is `volatile`; is there a reason that's necessary?

Looking at Git history, that was always the case. I don't see any particular reason we do that eagerly.

> Because we can't actually convert the LHS to remove those qualifiers, and adding combinatoric candidates could get very expensive if we start doing it for arbitrary extended qualifiers, which it feels like we ought to.  (You could certainly have e.g. a `volatile _Atomic __myaddrspace` l-value.)  If this is only necessary when the LHS has a user-defined conversion, then maybe we could at least not do it in the normal case just because the RHS is an overloaded type.

Yup, seems like we could be more lazy/minimal about that. I can look into that as a follow-up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125349