[libcxx-commits] [libcxx] c6afeda - [libc++] Reject standard attributes which are extensions in libcpp-uglify-attributes

[Nikolas Klauser via libcxx-commits]

Author: Nikolas Klauser
Date: 2023-04-07T21:10:55+02:00
New Revision: c6afeda866bf7ad6e1148e18c1bb1423ff4b2811

URL: https://github.com/llvm/llvm-project/commit/c6afeda866bf7ad6e1148e18c1bb1423ff4b2811
DIFF: https://github.com/llvm/llvm-project/commit/c6afeda866bf7ad6e1148e18c1bb1423ff4b2811.diff

LOG: [libc++] Reject standard attributes which are extensions in libcpp-uglify-attributes

This adds a list of attributes which can be pretty to be able to reject attributes which were introduced in a later C++ standard.
Fixes #61196

Reviewed By: Mordante, #libc

Spies: mikhail.ramalho, jdoerfert, libcxx-commits

Differential Revision: https://reviews.llvm.org/D145508

Added: 
    

Modified: 
    libcxx/include/__config
    libcxx/include/barrier
    libcxx/test/tools/clang_tidy_checks/uglify_attributes.cpp

Removed: 
    


################################################################################
diff  --git a/libcxx/include/__config b/libcxx/include/__config
index cf1cc9cc6dadf..671605864f998 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -879,7 +879,7 @@ _LIBCPP_BEGIN_NAMESPACE_STD _LIBCPP_END_NAMESPACE_STD
 #  endif
 
 #  if __has_cpp_attribute(nodiscard)
-#    define _LIBCPP_NODISCARD [[nodiscard]]
+#    define _LIBCPP_NODISCARD [[__nodiscard__]]
 #  else
 // We can't use GCC's [[gnu::warn_unused_result]] and
 // __attribute__((warn_unused_result)), because GCC does not silence them via
@@ -1198,7 +1198,7 @@ _LIBCPP_BEGIN_NAMESPACE_STD _LIBCPP_END_NAMESPACE_STD
 // [[msvc::no_unique_address]], this should be preferred though.
 #    define _LIBCPP_NO_UNIQUE_ADDRESS [[msvc::no_unique_address]]
 #  elif __has_cpp_attribute(no_unique_address)
-#    define _LIBCPP_NO_UNIQUE_ADDRESS [[no_unique_address]]
+#    define _LIBCPP_NO_UNIQUE_ADDRESS [[__no_unique_address__]]
 #  else
 #    define _LIBCPP_NO_UNIQUE_ADDRESS /* nothing */
 // Note that this can be replaced by #error as soon as clang-cl

diff  --git a/libcxx/include/barrier b/libcxx/include/barrier
index 41b3d8e0b7b40..ebaff8bcfeac5 100644
--- a/libcxx/include/barrier
+++ b/libcxx/include/barrier
@@ -135,7 +135,7 @@ public:
               __expected_adjustment_(0), __completion_(std::move(__completion)), __phase_(0)
     {
     }
-    [[nodiscard]] _LIBCPP_AVAILABILITY_SYNC _LIBCPP_INLINE_VISIBILITY
+    [[__nodiscard__]] _LIBCPP_AVAILABILITY_SYNC _LIBCPP_INLINE_VISIBILITY
     arrival_token arrive(ptr
diff _t __update)
     {
         auto const __old_phase = __phase_.load(memory_order_relaxed);
@@ -305,7 +305,7 @@ public:
     barrier(barrier const&) = delete;
     barrier& operator=(barrier const&) = delete;
 
-    [[nodiscard]] _LIBCPP_AVAILABILITY_SYNC _LIBCPP_INLINE_VISIBILITY
+    [[__nodiscard__]] _LIBCPP_AVAILABILITY_SYNC _LIBCPP_INLINE_VISIBILITY
     arrival_token arrive(ptr
diff _t __update = 1)
     {
         return __b_.arrive(__update);

diff  --git a/libcxx/test/tools/clang_tidy_checks/uglify_attributes.cpp b/libcxx/test/tools/clang_tidy_checks/uglify_attributes.cpp
index 03b9fe5e4d0f6..038788d94c915 100644
--- a/libcxx/test/tools/clang_tidy_checks/uglify_attributes.cpp
+++ b/libcxx/test/tools/clang_tidy_checks/uglify_attributes.cpp
@@ -12,6 +12,8 @@
 #include "uglify_attributes.hpp"
 
 #include <algorithm>
+#include <array>
+#include <span>
 #include <string_view>
 
 namespace {
@@ -23,15 +25,43 @@ bool isUgly(std::string_view str) {
   return str.find("__") != std::string_view::npos;
 }
 
+std::vector<const char*> get_standard_attributes(const clang::LangOptions& lang_opts) {
+  std::vector<const char*> attributes = {"noreturn", "carries_dependency"};
+
+  if (lang_opts.CPlusPlus14)
+    attributes.emplace_back("deprecated");
+
+  if (lang_opts.CPlusPlus17) {
+    attributes.emplace_back("fallthrough");
+    attributes.emplace_back("nodiscard");
+    attributes.emplace_back("maybe_unused");
+  }
+
+  if (lang_opts.CPlusPlus20) {
+    attributes.emplace_back("likely");
+    attributes.emplace_back("unlikely");
+    attributes.emplace_back("no_unique_address");
+  }
+
+  if (lang_opts.CPlusPlus2b) {
+    attributes.emplace_back("assume");
+  }
+
+  return attributes;
+}
+
 AST_MATCHER(clang::Attr, isPretty) {
   if (Node.isKeywordAttribute())
     return false;
-  if (Node.isCXX11Attribute() && !Node.hasScope()) // TODO: reject standard attributes that are version extensions
-    return false;
+  if (Node.isCXX11Attribute() && !Node.hasScope()) {
+    if (isUgly(Node.getAttrName()->getName()))
+      return false;
+    return !llvm::is_contained(
+        get_standard_attributes(Finder->getASTContext().getLangOpts()), Node.getAttrName()->getName());
+  }
   if (Node.hasScope())
     if (!isUgly(Node.getScopeName()->getName()))
       return true;
-
   if (Node.getAttrName())
     return !isUgly(Node.getAttrName()->getName());
 
@@ -39,9 +69,7 @@ AST_MATCHER(clang::Attr, isPretty) {
 }
 
 std::optional<std::string> getUglyfiedCXX11Attr(const clang::Attr& attr) {
-  // Don't try to fix attributes with `using` in them.
-  if (std::ranges::search(std::string_view(attr.getSpelling()), std::string_view("::")).empty())
-    return std::nullopt;
+  // TODO: Don't emit FixItHints for attributes with `using` in them or emit correct fixes.
 
   std::string attr_string;
   if (attr.isClangScope())
@@ -84,11 +112,11 @@ void uglify_attributes::registerMatchers(clang::ast_matchers::MatchFinder* finde
 }
 
 void uglify_attributes::check(const clang::ast_matchers::MatchFinder::MatchResult& result) {
-  if (const auto* call = result.Nodes.getNodeAs<clang::Attr>("normal_attribute"); call != nullptr) {
-    auto diagnostic = diag(call->getLoc(), "Non-standard attributes should use the _Ugly spelling");
-    auto uglified   = getUglified(*call);
+  if (const auto* attr = result.Nodes.getNodeAs<clang::Attr>("normal_attribute"); attr != nullptr) {
+    auto diagnostic = diag(attr->getLoc(), "Non-standard attributes should use the _Ugly spelling");
+    auto uglified   = getUglified(*attr);
     if (uglified.has_value()) {
-      diagnostic << clang::FixItHint::CreateReplacement(call->getRange(), *uglified);
+      diagnostic << clang::FixItHint::CreateReplacement(attr->getRange(), *uglified);
     }
   }
 }