[cfe-commits] [PATCH] narrowing in AltiVec literals in C++0x

Douglas Gregor dgregor at apple.com
Tue Oct 18 23:34:06 PDT 2011

On Sep 29, 2011, at 9:43 AM, Alex_Rosenberg at PlayStation.Sony.com wrote:

> On Sep 25, 2011, at 3:35 AM, Sebastian Redl wrote:
>> On 24.09.2011, at 01:30, alex_rosenberg at playstation.sony.com wrote:
>>> [dcl.init.list] dictates errors for narrowing conversions, which triggers even for trivial AltiVec literals such as those inside <altivec.h>.
>>> Attached is a big hammer written by a manager.
>> I probably invalidated this patch with my recent commits.
>> In any case, though, I don't think this is the right approach. First, C++11 specifies that narrowing conversions are errors in list-initialization, unless the source is a compile-time known value which fits into the target type. I don't see why an exception should be made for AltiVec literals. Second, if <altivec.h> triggers the error with literals, doesn't that mean the header is actually buggy? Third, there may be more places where this C++11 behavior may trigger errors in 3rd party headers, so I think the right approach is to disable the error in system headers, not for AltiVec literals.
> AltiVec literals were defined using both paren and braces syntaxes long before Bjarne and Gaby wrote N1919, which appears to be the first place they decided to propose banning narrowing. The historical perspective presented there is pretty clear on the reasons they decided to ban narrowing. The paren syntax used for AltiVec literals wasn't in C/C++ at all at the time it was defined. The brace syntax appeared solely because the GCC community didn't like the paren syntax and elected to make up their own approach. In both cases, those legacy syntaxes now happen to match C++11 features (and OpenCL, etc.).
> While we can certainly address the issues in <altivec.h> via several means, I'm concerned that lots of other valid AltiVec code outside that header will want to otherwise move forward to C++11 and won't want to introduce casts everywhere (16 per literal!).
> I fully accept that this may not be the right way/place to fix this, but I'm pretty sure that I don't accept that AltiVec literals should be retoactively absorbed into C++11 syntax like this.

I can certainly imagine that AltiVec wouldn't want to complain about these conversions. AltiVec's been a bit loose with typing forever, and AltiVec vectors are one place where you would expect narrowing to happen often (and expect the programmer to know that this is happening). So, I'm okay with this change not to warn about narrowing for AltiVec vectors. We're dealing with an extension here, and I think the history of the extension is more important than C++11's possibly-ill-fated clamping down on narrowing.

Alex, did you perchance check how GCC handled this? They added the narrowing diagnostics a while ago, and have supported AltiVec for a very long time.

Index: llvm/tools/clang/lib/Sema/SemaInit.cpp
--- llvm/tools/clang/lib/Sema/SemaInit.cpp	(revision 140355)
+++ llvm/tools/clang/lib/Sema/SemaInit.cpp	(working copy)
@@ -5317,8 +5317,14 @@
   bool Constant = false;
   APValue Result;
+  const InitializedEntity *Parent;
+  const VectorType *VecTy;
   if (TopLevelOfInitList &&
-      Seq.endsWithNarrowing(Context, InitE, &Constant, &Result)) {
+      Seq.endsWithNarrowing(Context, InitE, &Constant, &Result) &&
+      !((Parent = Entity.getParent()) &&
+        (VecTy = Parent->getType().getTypePtr()->getAs<VectorType>()) &&
+        VecTy->isAnyAltiVecKind())) {
     DiagnoseNarrowingInInitList(*this, Entity.getType(), InitE,
                                 Constant, Result);

Those assignments in the conditional are a bit on the… clever… side. Please factor the am-I-an-element-of-a-vector check out into a separate routine somewhere in SemaInit.cpp.

Also… test case?

	- Doug

More information about the cfe-commits mailing list