[PATCH] D125349: [Sema] Fix crash for C11 atomic in gnu++ mode
John McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Jun 10 06:28:21 PDT 2022
rjmccall added inline comments.
================
Comment at: clang/include/clang/AST/Type.h:277
+ return qs;
+ }
----------------
Could you go ahead and add this for `const` and `restrict` as well? I think that will give us the full set.
================
Comment at: clang/lib/Sema/SemaOverload.cpp:8196
+void forAllQualifierCombinationsImpl(
+ QualifiersAndAtomic available, QualifiersAndAtomic applied,
----------------
Extremely minor style nit: standard style is to declare these `static` rather than in an anonymous namespace. And it's probably worthwhile to say in a comment that we're currently only handling the qualifiers that are meaningful for the LHS of compound assignment overloading.
================
Comment at: clang/lib/Sema/SemaOverload.cpp:8982
// Extension: Add the binary operators =, +=, -=, *=, /= for vector types.
for (QualType Vec1Ty : CandidateTypes[0].vector_types())
----------------
jansvoboda11 wrote:
> jansvoboda11 wrote:
> > aaron.ballman wrote:
> > > Do we need to handle atomic vector types?
> > I tried those, but it appears vector types don't accept `_Atomic` element types:
> >
> > ```
> > typedef _Atomic unsigned v_a_unsigned __attribute__((__vector_size__(16)));
> > // error: invalid vector element type '_Atomic(unsigned int)'
> > ```
> >
> > And `_Atomic` vector gives this error:
> >
> > ```
> > typedef unsigned v_unsigned __attribute__((__vector_size__(16)));
> > typedef _Atomic v_unsigned a_v_unsigned;
> >
> > a_v_unsigned avu;
> >
> > void enum5() { avu += an_enum_value; }
> > void enum6() { avu |= an_enum_value; }
> > ```
> >
> > I'm not an expert in this area, so I can't say for sure this is the correct behavior. But at least we don't crash.
> > And `_Atomic` vector gives this error:
>
> ```
> error: invalid operands to binary expression
> ```
The way to get an answer here, I think, is to see if it works with something like a single scalar `int`.
Unfortunately, it probably does need to work (maybe conditionally), because at least some of our supported vector types have an implicit "splat" promotion from scalars, so you have the enum -> int -> splat conversion path. But we don't have to do that in this patch.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125349/new/
https://reviews.llvm.org/D125349
More information about the cfe-commits
mailing list