[clang][Sema] Bad register variable type error should point to the type

[DavidSpickett]

...not the register keyword. Fixes #109776.

Until now the error was only tested in clang/test/Sema/asm.c, where you can't check for the "^" character. I've added a new caret test file as I see has been done for other error types.

[llvmbot]

@llvm/pr-subscribers-clang

Author: David Spickett (DavidSpickett)

Changes

...not the register keyword. Fixes #109776.

Until now the error was only tested in clang/test/Sema/asm.c, where you can't check for the "^" character. I've added a new caret test file as I see has been done for other error types.

---

Full diff: https://github.com/llvm/llvm-project/pull/110239.diff

2 Files Affected:

• (modified) clang/lib/Sema/SemaDecl.cpp (+2-1)
• (added) clang/test/Sema/caret-diags-register-variable.cpp (+20)

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 1bf0e800a36228..048ac9c22eef50 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -7949,7 +7949,8 @@ NamedDecl *Sema::ActOnVariableDeclarator(
       }
 
       if (!R->isIntegralType(Context) && !R->isPointerType()) {
-        Diag(D.getBeginLoc(), diag::err_asm_bad_register_type);
+        Diag(TInfo->getTypeLoc().getBeginLoc(),
+             diag::err_asm_bad_register_type);
         NewVD->setInvalidDecl(true);
       }
     }
diff --git a/clang/test/Sema/caret-diags-register-variable.cpp b/clang/test/Sema/caret-diags-register-variable.cpp
new file mode 100644
index 00000000000000..09893341717c8e
--- /dev/null
+++ b/clang/test/Sema/caret-diags-register-variable.cpp
@@ -0,0 +1,20 @@
+// RUN: not %clang_cc1 -triple i386-pc-linux-gnu -std=c++11 -fsyntax-only -fno-diagnostics-show-line-numbers -fcaret-diagnostics-max-lines=5 %s 2>&1 | FileCheck %s -strict-whitespace
+
+struct foo {
+  int a;
+};
+
+//CHECK: {{.*}}: error: bad type for named register variable
+//CHECK-NEXT: {{^}}register struct foo bar asm("esp");
+//CHECK-NEXT: {{^}}         ^{{$}}
+register struct foo bar asm("esp"); // expected-error {{bad type for named register variable}}
+
+//CHECK: {{.*}}: error: register 'edi' unsuitable for global register variables on this target
+//CHECK-NEXT: {{^}}register int r0 asm ("edi");
+//CHECK-NEXT: {{^}}                     ^{{$}}
+register int r0 asm ("edi");
+
+//CHECK: {{.*}}: error: size of register 'esp' does not match variable size
+//CHECK-NEXT: {{^}}register long long r1 asm ("esp");
+//CHECK-NEXT: {{^}}                           ^{{$}}
+register long long r1 asm ("esp");

[DavidSpickett]

I also think that "bad" is a confusing word to use here, "unsupported" would be better but I will address that in a follow up PR.

[Sirraide]

This is still missing a release note, and adding the source range too would be nice.

[Sirraide]

I also think that "bad" is a confusing word to use here, "unsupported" would be better but I will address that in a follow up PR.

I don’t have a problem w/ ‘bad’, but ‘unsupported’ is clearer, yeah

[DavidSpickett]

This is still missing a release note, and adding the source range too would be nice.

Both done.

[Sirraide]

Looks like CI is still mad at you for some reason so that still needs to be looked into, but other than that this lgtm.