gribozavr added a comment.
> Those are already there in clang/test/SemaCXX/attr-gsl-owner-pointer.cpp.
I see. Sorry, but that seems insufficient to me -- different libraries use
different patterns. For example, libc++ wraps everything in std in an inline
namespace. I don't know how various versions of libstdc++ differ.
================
Comment at: clang/include/clang/Basic/TokenKinds.def:486
+TYPE_TRAIT_1(__is_gsl_owner, IsGslOwner, KEYCXX)
+TYPE_TRAIT_1(__is_gsl_pointer, IsGslPointer, KEYCXX)
KEYWORD(__underlying_type , KEYCXX)
----------------
I'd suggest to split type trait implementations into a separate patch.
Are these type traits being exposed only for testing? I'm not sure it is a good
idea to do that -- people will end up using them in production code -- is that
a concern? If so, maybe it would be better to test through warnings.
================
Comment at: clang/test/AST/ast-dump-attr.cpp:222
+
+class [[gsl::Pointer]] PointerWithoutArgument{};
+// CHECK: CXXRecordDecl{{.*}} class PointerWithoutArgument
----------------
This test and related code changes could be split off into a separate patch.
================
Comment at: clang/test/SemaCXX/attr-gsl-owner-pointer.cpp:86
+
+// Test builtin annotation for std types.
+namespace std {
----------------
I feel like these tests would be better off in a separate file.
It would be also good to explain which exact library we are trying to imitate
in this test. Different libraries use different coding patterns.
================
Comment at: clang/test/SemaCXX/attr-gsl-owner-pointer.cpp:103
+template <typename T>
+class set_iterator {};
+
----------------
Is it actually defined like that?
================
Comment at: clang/test/SemaCXX/attr-gsl-owner-pointer.cpp:140
+
+class thread;
+static_assert(!__is_gsl_pointer(thread), "");
----------------
Unclear what this test is testing.
If there's something special about thread (e.g., it looks very much like an
owner or a pointer, and a buggy implementation can easily declare thread to be
an owner or a pointer), please explain that in a comment.
If you're testing that some random name is not being picked up by inference
rules, I'd suggest to use an obviously-fictional name ("class
type_unknown_to_compiler;")
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64448/new/
https://reviews.llvm.org/D64448
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
