[clang] Change "bad" to "unsupported" in register type error #111550
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is maybe a personal take but I expect "bad" to either mean: * Allowed but not ideal, like a "bad" memory alignment might work but it is slow. * The tool won't allow it but is going to tell me why it didn't. The current error doesn't elaborate so I think it's best we just say "unsupported" instead. This is clear that the type used is not allowed at all.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: David Spickett (DavidSpickett) ChangesThis is maybe a personal take but I expect "bad" to either mean:
The current error doesn't elaborate so I think it's best we just say "unsupported" instead. This is clear that the type used is not allowed at all. Full diff: https://github.com/llvm/llvm-project/pull/111550.diff 4 Files Affected:
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 583475327c5227..82588cea4155c1 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9386,7 +9386,8 @@ let CategoryName = "Inline Assembly Issue" in {
"global register variables on this target">;
def err_asm_register_size_mismatch : Error<"size of register '%0' does not "
"match variable size">;
- def err_asm_bad_register_type : Error<"bad type for named register variable">;
+ def err_asm_unsupported_register_type : Error<
+ "unsupported type for named register variable">;
def err_asm_invalid_input_size : Error<
"invalid input size for constraint '%0'">;
def err_asm_invalid_output_size : Error<
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 83d71913f8635e..072f43d360ee1c 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -7961,7 +7961,8 @@ NamedDecl *Sema::ActOnVariableDeclarator(
}
if (!R->isIntegralType(Context) && !R->isPointerType()) {
- Diag(TInfo->getTypeLoc().getBeginLoc(), diag::err_asm_bad_register_type)
+ Diag(TInfo->getTypeLoc().getBeginLoc(),
+ diag::err_asm_unsupported_register_type)
<< TInfo->getTypeLoc().getSourceRange();
NewVD->setInvalidDecl(true);
}
diff --git a/clang/test/Sema/asm.c b/clang/test/Sema/asm.c
index 630a5e85dd9131..6cd95c71604d44 100644
--- a/clang/test/Sema/asm.c
+++ b/clang/test/Sema/asm.c
@@ -191,8 +191,8 @@ void iOutputConstraint(int x){
struct foo {
int a;
};
-register struct foo bar asm("esp"); // expected-error {{bad type for named register variable}}
-register float baz asm("esp"); // expected-error {{bad type for named register variable}}
+register struct foo bar asm("esp"); // expected-error {{unsupported type for named register variable}}
+register float baz asm("esp"); // expected-error {{unsupported type for named register variable}}
register int r0 asm ("edi"); // expected-error {{register 'edi' unsuitable for global register variables on this target}}
register long long r1 asm ("esp"); // expected-error {{size of register 'esp' does not match variable size}}
diff --git a/clang/test/Sema/caret-diags-register-variable.cpp b/clang/test/Sema/caret-diags-register-variable.cpp
index 24f5061d4b4d2c..c2d2fbe0c581ae 100644
--- a/clang/test/Sema/caret-diags-register-variable.cpp
+++ b/clang/test/Sema/caret-diags-register-variable.cpp
@@ -4,7 +4,7 @@ struct foo {
int a;
};
-//CHECK: {{.*}}: error: bad type for named register variable
+//CHECK: {{.*}}: error: unsupported type for named register variable
//CHECK-NEXT: {{^}}register struct foo bar asm("esp");
//CHECK-NEXT: {{^}} ^~~~~~~~~~{{$}}
register struct foo bar asm("esp");
|
Sirraide
approved these changes
Oct 8, 2024
|
I think we also might want to add a release note for this since this is technically improving a diagnostic, if only in a very minor way... I’m not sure if we have a clear-cut policy for that when it comes to changes that only change the wording of a diagnostic. |
|
Added a release note. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/123/builds/7202 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/18/builds/5139 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is maybe a personal take but I expect "bad" to either mean:
The current error doesn't elaborate so I think it's best we just say "unsupported" instead. This is clear that the type used is not allowed at all.