[PATCH] D53710: [FileCheck] Warn if a prefix is only used in LABEL checks

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer created this revision.
SjoerdMeijer added reviewers: aditya_nandakumar, george.karpenkov, jdenny, samparker.
Herald added subscribers: kristof.beyls, javed.absar.

I would like FileCheck to warn for cases like this:

      
  RUN: llc -mtriple=arm-linux-gnueabi %s -o - | FileCheck %s -check-prefix=FOO
  define void @FOO() {
  entry:
  ; FOO-LABEL: FOO:
  ; DOO:       mov  pc, lr
    ret void
  }
      

Here, prefix FOO is only used in a label check FOO-LABEL. This can easily
happen when a typo is made in the actual checks: here accidentally DOO is
checked instead of FOO. The problem is that this goes unnoticed, as 
FileCheck is not producing a diagnostic for this.

      

I would like to change this behaviour, and with this patch FileCheck will now
warn like this:

  
  foo.ll:5:18: note: Prefix FOO only occurs in a LABEL check, this is probably not what you want
  ; FOO-LABEL: FOO:
                   ^


https://reviews.llvm.org/D53710

Files:
  lib/Support/FileCheck.cpp


Index: lib/Support/FileCheck.cpp
===================================================================
--- lib/Support/FileCheck.cpp
+++ lib/Support/FileCheck.cpp
@@ -737,6 +737,8 @@
   // LineNumber keeps track of the line on which CheckPrefix instances are
   // found.
   unsigned LineNumber = 1;
+  bool HasNonLabelPrefix = false;
+  StringRef MatchedPrefix;
 
   while (1) {
     Check::FileCheckType CheckTy;
@@ -746,6 +748,7 @@
                                                    CheckTy);
     if (UsedPrefix.empty())
       break;
+    MatchedPrefix = UsedPrefix;
     assert(UsedPrefix.data() == Buffer.data() &&
            "Failed to move Buffer's start forward, or pointed prefix outside "
            "of the buffer!");
@@ -763,8 +766,11 @@
       return true;
     }
 
-    // Okay, we found the prefix, yay. Remember the rest of the line, but ignore
-    // leading whitespace.
+    // Okay, we found the prefix, yay.
+    if (CheckTy != Check::CheckLabel)
+      HasNonLabelPrefix = true;
+
+    // Remember the rest of the line, but ignore leading whitespace.
     if (!(Req.NoCanonicalizeWhiteSpace && Req.MatchFullLines))
       Buffer = Buffer.substr(Buffer.find_first_not_of(" \t"));
 
@@ -840,6 +846,12 @@
     return true;
   }
 
+  if (!HasNonLabelPrefix)
+    SM.PrintMessage(SMLoc::getFromPointer(Buffer.data()),
+                    SourceMgr::DK_Note,
+                    "Prefix " + MatchedPrefix + " only occurs in a LABEL check, "
+                    "this is probably not what you want");
+
   return false;
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D53710.171115.patch
Type: text/x-patch
Size: 1546 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181025/46c2bd08/attachment.bin>