Skip to content

[Clang] show attribute namespace in diagnostics #138519

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 8, 2025

Conversation

a-tarasyuk
Copy link
Member

@a-tarasyuk a-tarasyuk commented May 5, 2025

This patch enhances Clang's diagnosis of an unknown attribute by printing the attribute's namespace in the diagnostic text. e.g.,

[[foo::nodiscard]] int f(); // warning: unknown attribute 'foo::nodiscard' ignored

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules labels May 5, 2025
@llvmbot
Copy link
Member

llvmbot commented May 5, 2025

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-modules

Author: Oleksandr T. (a-tarasyuk)

Changes

This patch enhances Clang's attribute handling by diagnosing unknown attribute namespaces

[[foo::nodiscard]] int f(); // warning: unknown attribute namespace 'unknown'; attribute 'unknown::foo' ignored 

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

9 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+2)
  • (modified) clang/include/clang/Basic/AttributeCommonInfo.h (+3-1)
  • (modified) clang/include/clang/Basic/DiagnosticCommonKinds.td (+3)
  • (modified) clang/lib/Basic/Attributes.cpp (+8-1)
  • (modified) clang/lib/Sema/SemaDeclAttr.cpp (+12-7)
  • (modified) clang/test/CXX/module/module.interface/p3.cpp (+1-1)
  • (modified) clang/test/Parser/c2x-attributes.c (+1-1)
  • (modified) clang/test/Parser/cxx0x-attributes.cpp (+3-3)
  • (added) clang/test/Sema/unknown-attributes.c (+12)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a80fedebf8f89..d01fc5d1bf46f 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -385,6 +385,8 @@ related warnings within the method body.
 - Clang now disallows the use of attributes applied before an
   ``extern template`` declaration (#GH79893).
 
+- Clang now diagnoses unknown attribute namespaces
+
 Improvements to Clang's diagnostics
 -----------------------------------
 
diff --git a/clang/include/clang/Basic/AttributeCommonInfo.h b/clang/include/clang/Basic/AttributeCommonInfo.h
index 4af5a8fd1852c..ddb022c493bdf 100644
--- a/clang/include/clang/Basic/AttributeCommonInfo.h
+++ b/clang/include/clang/Basic/AttributeCommonInfo.h
@@ -67,7 +67,7 @@ class AttributeCommonInfo {
     IgnoredAttribute,
     UnknownAttribute,
   };
-  enum class Scope { NONE, CLANG, GNU, MSVC, OMP, HLSL, GSL, RISCV };
+  enum class Scope { NONE, CLANG, GNU, MSVC, OMP, HLSL, GSL, RISCV, UNKNOWN };
   enum class AttrArgsInfo {
     None,
     Optional,
@@ -234,6 +234,8 @@ class AttributeCommonInfo {
     return SyntaxUsed == AS_ContextSensitiveKeyword;
   }
 
+  bool isUnknownScopeName() const;
+
   unsigned getAttributeSpellingListIndex() const {
     assert((isAttributeSpellingListCalculated() || AttrName) &&
            "Spelling cannot be found");
diff --git a/clang/include/clang/Basic/DiagnosticCommonKinds.td b/clang/include/clang/Basic/DiagnosticCommonKinds.td
index f26c906b46447..63f4c229b2e76 100644
--- a/clang/include/clang/Basic/DiagnosticCommonKinds.td
+++ b/clang/include/clang/Basic/DiagnosticCommonKinds.td
@@ -177,6 +177,9 @@ def err_opencl_unknown_type_specifier : Error<
   "%0 does not support the '%1' "
   "%select{type qualifier|storage class specifier}2">;
 
+def warn_unknown_attribute_namespace : Warning<
+  "unknown attribute namespace '%0'; attribute '%0::%1' ignored">,
+  InGroup<UnknownAttributes>;
 def warn_unknown_attribute_ignored : Warning<
   "unknown attribute %0 ignored">, InGroup<UnknownAttributes>;
 def warn_attribute_ignored : Warning<"%0 attribute ignored">,
diff --git a/clang/lib/Basic/Attributes.cpp b/clang/lib/Basic/Attributes.cpp
index 6a070a99c8d96..c41cbf0fb61f6 100644
--- a/clang/lib/Basic/Attributes.cpp
+++ b/clang/lib/Basic/Attributes.cpp
@@ -191,7 +191,8 @@ getScopeFromNormalizedScopeName(StringRef ScopeName) {
       .Case("hlsl", AttributeCommonInfo::Scope::HLSL)
       .Case("msvc", AttributeCommonInfo::Scope::MSVC)
       .Case("omp", AttributeCommonInfo::Scope::OMP)
-      .Case("riscv", AttributeCommonInfo::Scope::RISCV);
+      .Case("riscv", AttributeCommonInfo::Scope::RISCV)
+      .Default(AttributeCommonInfo::Scope::UNKNOWN);
 }
 
 unsigned AttributeCommonInfo::calculateAttributeSpellingListIndex() const {
@@ -206,3 +207,9 @@ unsigned AttributeCommonInfo::calculateAttributeSpellingListIndex() const {
 
 #include "clang/Sema/AttrSpellingListIndex.inc"
 }
+
+bool AttributeCommonInfo::isUnknownScopeName() const {
+  return getScopeFromNormalizedScopeName(
+             normalizeAttrScopeName(getScopeName(), getSyntax())) ==
+         AttributeCommonInfo::Scope::UNKNOWN;
+}
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index bfb3ee9dcbd16..7dc5a04923946 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -6861,13 +6861,18 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
   // though they were unknown attributes.
   if (AL.getKind() == ParsedAttr::UnknownAttribute ||
       !AL.existsInTarget(S.Context.getTargetInfo())) {
-    S.Diag(AL.getLoc(),
-           AL.isRegularKeywordAttribute()
-               ? (unsigned)diag::err_keyword_not_supported_on_target
-           : AL.isDeclspecAttribute()
-               ? (unsigned)diag::warn_unhandled_ms_attribute_ignored
-               : (unsigned)diag::warn_unknown_attribute_ignored)
-        << AL << AL.getRange();
+    if (AL.isUnknownScopeName())
+      S.Diag(AL.getScopeLoc(), diag::warn_unknown_attribute_namespace)
+          << AL.getScopeName()->getName() << AL.getAttrName()->getName()
+          << SourceRange(AL.getScopeLoc(), AL.getRange().getEnd());
+    else
+      S.Diag(AL.getLoc(),
+             AL.isRegularKeywordAttribute()
+                 ? (unsigned)diag::err_keyword_not_supported_on_target
+             : AL.isDeclspecAttribute()
+                 ? (unsigned)diag::warn_unhandled_ms_attribute_ignored
+                 : (unsigned)diag::warn_unknown_attribute_ignored)
+          << AL << AL.getRange();
     return;
   }
 
diff --git a/clang/test/CXX/module/module.interface/p3.cpp b/clang/test/CXX/module/module.interface/p3.cpp
index 32819b2dccb11..18758edaf1be7 100644
--- a/clang/test/CXX/module/module.interface/p3.cpp
+++ b/clang/test/CXX/module/module.interface/p3.cpp
@@ -40,7 +40,7 @@ export { // No diagnostic after P2615R1 DR
   extern "C++" {} // No diagnostic after P2615R1 DR
 }
 export [[]]; // No diagnostic after P2615R1 DR
-export [[example::attr]]; // expected-warning {{unknown attribute 'attr'}}
+export [[example::attr]]; // expected-warning {{unknown attribute namespace 'example'; attribute 'example::attr' ignored}}
 
 // [...] shall not declare a name with internal linkage
 export static int a; // expected-error {{declaration of 'a' with internal linkage cannot be exported}}
diff --git a/clang/test/Parser/c2x-attributes.c b/clang/test/Parser/c2x-attributes.c
index be039e40f98ef..b5f502c5790d3 100644
--- a/clang/test/Parser/c2x-attributes.c
+++ b/clang/test/Parser/c2x-attributes.c
@@ -133,7 +133,7 @@ void f11(void) {
 }
 
 [[attr]] void f12(void); // expected-warning {{unknown attribute 'attr' ignored}}
-[[vendor::attr]] void f13(void); // expected-warning {{unknown attribute 'attr' ignored}}
+[[vendor::attr]] void f13(void); // expected-warning {{unknown attribute namespace 'vendor'; attribute 'vendor::attr' ignored}}
 
 // Ensure that asm statements properly handle double colons.
 void test_asm(void) {
diff --git a/clang/test/Parser/cxx0x-attributes.cpp b/clang/test/Parser/cxx0x-attributes.cpp
index fad3010c98b9c..7a0a8b989851f 100644
--- a/clang/test/Parser/cxx0x-attributes.cpp
+++ b/clang/test/Parser/cxx0x-attributes.cpp
@@ -46,7 +46,7 @@ int & [[noreturn]] ref_attr_3 = after_attr; // expected-error {{'noreturn' attri
 int && [[]] rref_attr = 0;
 int array_attr [1] [[]];
 alignas(8) int aligned_attr;
-[[test::valid(for 42 [very] **** '+' symbols went on a trip and had a "good"_time; the end.)]] int garbage_attr; // expected-warning {{unknown attribute 'valid' ignored}}
+[[test::valid(for 42 [very] **** '+' symbols went on a trip and had a "good"_time; the end.)]] int garbage_attr; // expected-warning {{unknown attribute namespace 'test'; attribute 'test::valid' ignored}}
 [[,,,static, class, namespace,, inline, constexpr, mutable,, bitand, bitor::compl(!.*_ Cx.!U^*R),,,]] int more_garbage_attr; // expected-warning {{unknown attribute 'static' ignored}} \
     // expected-warning {{unknown attribute 'class' ignored}} \
     // expected-warning {{unknown attribute 'namespace' ignored}} \
@@ -54,7 +54,7 @@ alignas(8) int aligned_attr;
     // expected-warning {{unknown attribute 'constexpr' ignored}} \
     // expected-warning {{unknown attribute 'mutable' ignored}} \
     // expected-warning {{unknown attribute 'bitand' ignored}} \
-    // expected-warning {{unknown attribute 'compl' ignored}}
+    // expected-warning {{unknown attribute namespace 'bitor'; attribute 'bitor::compl' ignored}}
 [[u8"invalid!"]] int invalid_string_attr; // expected-error {{expected ']'}}
 void fn_attr () [[]];
 void noexcept_fn_attr () noexcept [[]];
@@ -269,7 +269,7 @@ template <int... Is> void variadic_nttp() {
   void baz [[clang::no_sanitize(Is...)]] ();          // expected-error {{expected string literal as argument of 'no_sanitize' attribute}}
   void bor [[clang::annotate("A", "V" ...)]] ();      // expected-error {{pack expansion does not contain any unexpanded parameter packs}}
   void bir [[clang::annotate("B", {1, 2, 3, 4})]] (); // expected-error {{'annotate' attribute requires parameter 1 to be a constant expression}} expected-note {{subexpression not valid in a constant expression}}
-  void boo [[unknown::foo(Is...)]] ();                // expected-warning {{unknown attribute 'foo' ignored}}
+  void boo [[unknown::foo(Is...)]] ();                // expected-warning {{unknown attribute namespace 'unknown'; attribute 'unknown::foo' ignored}}
   void faz [[clang::annotate("C", (Is + ...))]] ();   // expected-warning {{pack fold expression is a C++17 extension}}
   void far [[clang::annotate("D", Is...)]] ();
   void foz [[clang::annotate("E", 1, 2, 3, Is...)]] ();
diff --git a/clang/test/Sema/unknown-attributes.c b/clang/test/Sema/unknown-attributes.c
new file mode 100644
index 0000000000000..833ac2f919ea6
--- /dev/null
+++ b/clang/test/Sema/unknown-attributes.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -Wunknown-attributes -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -Wunknown-attributes -verify %s
+
+[[foo::a]] // expected-warning {{unknown attribute namespace 'foo'; attribute 'foo::a' ignored}}
+int f1(void) {
+  return 0;
+}
+
+[[clan::deprecated]] // expected-warning {{unknown attribute namespace 'clan'; attribute 'clan::deprecated' ignored}}
+int f2(void) {
+  return 0;
+}

@a-tarasyuk a-tarasyuk removed the clang:modules C++20 modules and Clang Header Modules label May 5, 2025
Copy link
Collaborator

@erichkeane erichkeane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe our feedback on the last one was to emit the namespace in all attribute ignored diagnostics, not just if we don't know the namespace. So unknown attribute clang::whatever as well?

Otherwise, I think this is a reasonable patch that makes this diagnostic somewhat more clear.

@llvmbot llvmbot added the clang:modules C++20 modules and Clang Header Modules label May 5, 2025
@a-tarasyuk a-tarasyuk removed the clang:modules C++20 modules and Clang Header Modules label May 5, 2025
Copy link
Collaborator

@erichkeane erichkeane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I don't believe this is correct either. We want it to always say the namespace. So clang::unknown will diagnose as unknown attribute 'clang::unknown' ignored AFAIK. Right @AaronBallman ?

@AaronBallman
Copy link
Collaborator

So I don't believe this is correct either. We want it to always say the namespace. So clang::unknown will diagnose as unknown attribute 'clang::unknown' ignored AFAIK. Right @AaronBallman ?

I think what I'm seeing is correct, unless I'm missing something. Attributes without namespaces are printed as foo and attributes with namespaces are now being printed as bar::foo instead of foo.

@llvmbot llvmbot added the clang:modules C++20 modules and Clang Header Modules label May 5, 2025
@a-tarasyuk a-tarasyuk removed the clang:modules C++20 modules and Clang Header Modules label May 5, 2025
@llvmbot llvmbot added the clang:modules C++20 modules and Clang Header Modules label May 6, 2025
@a-tarasyuk a-tarasyuk removed the clang:modules C++20 modules and Clang Header Modules label May 6, 2025
@a-tarasyuk a-tarasyuk force-pushed the feat/unknown-attribute-namespaces branch from 7835772 to 8acf144 Compare May 6, 2025 21:47
@llvmbot llvmbot added clang:modules C++20 modules and Clang Header Modules clang:openmp OpenMP related changes to Clang labels May 6, 2025
@a-tarasyuk a-tarasyuk removed clang:modules C++20 modules and Clang Header Modules clang:openmp OpenMP related changes to Clang labels May 6, 2025
@a-tarasyuk
Copy link
Member Author

So I don't believe this is correct either. We want it to always say the namespace. So clang::unknown will diagnose as unknown attribute 'clang::unknown' ignored

@erichkeane @AaronBallman Thanks for the feedback. I've made changes to include the scope in the diagnostic.

@AaronBallman
Copy link
Collaborator

I think the summary and title should be updated to something more like:

[Clang] Show attribute namespace in diagnostics

This patch enhances Clang's diagnosing of an unknown attribute by also printing the namespace of the attribute in the diagnostic text. e.g.,

[[foo::nodiscard]] int f(); // warning: unknown attribute 'foo::nodiscard' ignored

@llvmbot llvmbot added clang:modules C++20 modules and Clang Header Modules clang:openmp OpenMP related changes to Clang labels May 7, 2025
@a-tarasyuk a-tarasyuk changed the title [Clang] diagnose unknown attribute namespaces [Clang] show attribute namespace in diagnostics May 7, 2025
@a-tarasyuk a-tarasyuk requested a review from AaronBallman May 7, 2025 12:02
@a-tarasyuk a-tarasyuk removed clang:modules C++20 modules and Clang Header Modules clang:openmp OpenMP related changes to Clang labels May 7, 2025
Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as the changes go, this LGTM. We can generalize in another PR (which will update some parts of this patch but not enough to be worried we're wasting effort).

@llvmbot llvmbot added clang:modules C++20 modules and Clang Header Modules clang:openmp OpenMP related changes to Clang labels May 7, 2025
@a-tarasyuk a-tarasyuk removed clang:modules C++20 modules and Clang Header Modules clang:openmp OpenMP related changes to Clang labels May 7, 2025
@a-tarasyuk a-tarasyuk merged commit 61b435e into llvm:main May 8, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants