aaron.ballman added inline comments.
================
Comment at: clang/include/clang/AST/Type.h:617
+struct QualifiersAndAtomic {
+ Qualifiers Quals;
----------------
The set of operations here feel a bit weird. We have `withVolatile` and
`withAtomic` but not `withConst` or `withRestrict`. We have `hasVolatile` and
`hasRestrict` but no `hasConst` or `hasAtomic`. And we have `addConst` but no
other add methods for the other qualifiers.
Should these sets be rounded out to cover all the qualifiers? I know we don't
need them for the functionality you're fixing up, but this seems like a pretty
generic interface in a header file that's used all over the place.
================
Comment at: clang/lib/Sema/SemaOverload.cpp:8982
// Extension: Add the binary operators =, +=, -=, *=, /= for vector types.
for (QualType Vec1Ty : CandidateTypes[0].vector_types())
----------------
rjmccall wrote:
> 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.
> But we don't have to do that in this patch.
+1, if it's involved, it can definitely be done in a follow-up.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125349/new/
https://reviews.llvm.org/D125349
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
