-
Notifications
You must be signed in to change notification settings - Fork 13.8k
Docs: ambiguous use of "explicitly" in [[clang::no_specializaiton]] #143839
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
base: main
Are you sure you want to change the base?
Conversation
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
@llvm/pr-subscribers-clang Author: Michael Davis (mdavis36) ChangesSummaryThis PR resolves issue #143719.
Full diff: https://github.com/llvm/llvm-project/pull/143839.diff 1 Files Affected:
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 047f51ffa59ed..508cf6d427a52 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -1216,8 +1216,8 @@ def NoSpecializationsDocs : Documentation {
let Category = DocCatDecl;
let Content = [{
``[[clang::no_specializations]]`` can be applied to function, class, or variable
-templates which should not be explicitly specialized by users. This is primarily
-used to diagnose user specializations of standard library type traits.
+templates which should not be specialized by users. This is primarily used to
+diagnose user specializations of standard library type traits.
}];
}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I've slightly modified the PR description to associate this PR with the issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems incorrect. A specialization is an instantiation or an explicit specialization according to https://eel.is/c++draft/temp.spec.general#4, which is definitely not what we mean.
Well, to be fair ‘specialized by users’ to me at least implies that we’re talking about explicit specialisations. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks fine. The meaning is clear.
(If anything, the C++ standard should have a better way to talk about "partial or explicit specializations")
Given that the whole argument to change this is that it's closer to the standard wording I don't see how the current change makes sense. It's just as technically incorrect as the old wording. |
Summary
This PR resolves #143719.
[[clang::no_specialization]]
to better match behavior of specialization as defined by the C++ standard.