[PATCH] D14744: PR10235: support for vector mode attributes + warning

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

In http://reviews.llvm.org/D14744#291663, @DmitryPolukhin wrote:

> Added new warning group as suggested.
>
> I see thousands of vector types in headers on GitHub https://github.com/search?p=1&q=%22__attribute__+mode+v4sf%22+extension%3Ah&ref=searchresults&type=Code&utf8=%E2%9C%93
>
> liboil from freedesktop.org uses vector types in headers.


The vast majority of those headers all appear to be GCC's vector-defs.h, so I'm still not convinced this is actually something we need to solve. However, I also don't see the harm.


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2828
@@ +2827,3 @@
+def warn_vector_mode_deprecated : Warning<
+  "specifying vector types with __attribute__ ((mode)) is deprecated, "
+  "use __attribute__ ((vector_size)) instead">,
----------------
Should be reworded slightly to be consistent with other attribute diagnostics:

"specifying vector types with the 'mode' attribute is deprecated; use the 'vector_size' attribute instead"

================
Comment at: lib/Sema/SemaDeclAttr.cpp:3239
@@ +3238,3 @@
+/// attribute.
+static void parseModeAttr(Sema &S, StringRef Str, unsigned &DestWidth,
+                          bool &IntegerMode, bool &ComplexMode) {
----------------
parseModeAttrArg() please.

================
Comment at: lib/Sema/SemaDeclAttr.cpp:3315
@@ +3314,3 @@
+  llvm::APInt VectorSize(64, 0);
+  if (Str.size() >= 4 && Str[0] == 'V') {
+    // Minimal length of vector mode is 4: 'V' + NUMBER(>=1) + TYPE(>=2).
----------------
Can you have a vector of vectors with mode? I'm wondering why this isn't part of parseModeAttr(), since this is parsing part of the mode attribute.

================
Comment at: lib/Sema/SemaDeclAttr.cpp:3318
@@ +3317,3 @@
+    int VectorStringLength = 0;
+    while (isdigit(Str[VectorStringLength + 1]))
+      ++VectorStringLength;
----------------
This will cause a buffer overrun on pathological code; should also constrain based on the size of Str.


http://reviews.llvm.org/D14744