[PATCH] D157572: [clang] Add `[[clang::library_extension]]` attribute

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

I think I like this attribute.

However, I am concerned that this would make it seem more acceptable to add extensions to the library, when our policy has been that we don't (and for good reasons, we ran into big issues with e.g. `std::string_view` and we learned from those). Therefore, I would really support this (and obviously using it in libc++), but I would like us to write down (and justify) our policy about library extensions so that we can keep applying it firmly going forward. Then, I think it's a win-win situation for everyone. Users get diagnostics when they use extensions, libc++ gets a useful tool, and we don't start making bad decisions w.r.t. extensions that will bite us.



================
Comment at: clang/include/clang/Basic/AttrDocs.td:1741
+    template <class CharT, class Traits = char_traits<CharT>>
+    class [[clang::extension("C++17")]] basic_string_view { /*...*/ };
+
----------------
This should say `clang::library_extension`


================
Comment at: clang/include/clang/Basic/AttrDocs.td:1746
+
+  // Clang produces the warning "std::string_view is a C++17 extension"
+  void func(std::string_view);
----------------
ldionne wrote:
> 
Perhaps the diagnostic should be `std::string_view is a C++17 library extension`?


================
Comment at: clang/include/clang/Basic/AttrDocs.td:1746-1747
+
+  // Clang produces the warning "std::string_view is a C++17 extension"
+  void func(std::string_view);
+
----------------



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157572/new/

https://reviews.llvm.org/D157572