[PATCH] D54169: [clang-tidy] Zircon ->
juliehockett abandoned this revision. juliehockett added a comment. After a lot of discussion, we'll do this migration internally. Thanks for your comments! https://reviews.llvm.org/D54169 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54169: [clang-tidy] Zircon ->
steveire added a comment. This is https://bugs.llvm.org//show_bug.cgi?id=32739 . I think checks not relevant to a general audience (ie including the boost directory) should be in external plugins. It's not really clear to me what has to happen for that, but it seems increasingly relevant. https://reviews.llvm.org/D54169 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54169: [clang-tidy] Zircon ->
aaron.ballman added inline comments.
Comment at: clang-tools-extra/clang-tidy/zircon/FblLimitsCheck.cpp:47
+SrcMgr::CharacteristicKind FileType) {
+ if (FileName == "fbl/limits.h") {
+unsigned End = std::strcspn(SM.getCharacterData(HashLoc), "\n") + 1;
juliehockett wrote:
> aaron.ballman wrote:
> > juliehockett wrote:
> > > aaron.ballman wrote:
> > > > juliehockett wrote:
> > > > > aaron.ballman wrote:
> > > > > > Does this also work on platforms where the path separator is `\`
> > > > > > instead of `/`? What about case insensitive file systems where it
> > > > > > may be spelled `LiMiTs.H`? Does this properly handle a case like:
> > > > > > ```
> > > > > > #define LIMITS "fbl/limits.h"
> > > > > > #include LIMITS
> > > > > > ```
> > > > > > (Should add test cases for all of these scenarios.)
> > > > > Since this is a migration for our own codebase, we know we don't have
> > > > > any code that uses any variation other than and so
> > > > > hardcoding that is acceptable to us here.
> > > > Then why should this check be a public one, rather than an internal
> > > > check?
> > > I explained this on the other one, but for completeness:
> > >
> > >
> > > So yes, this check is for a migration, and we would delete it once
> > > regressions weren't possible. We would like the suite to be in upstream,
> > > however, because we use the ToT llvm/clang/tools/etc, and don't want to
> > > have to fork just to use clang-tidy for this sort of thing. Since
> > > clang-tidy doesn't provide any way to have external checks to the tool
> > > itself, upstreaming is the most ideal option.
> > >
> > > Orthogonal to our particular build setup, it'd also be nice to have an
> > > example of this sort of migration done by clang-tidy in-tree. There has
> > > been a lot of discussion recently about doing migrations with clang-tidy,
> > > but it's always describing an internal migration that uses a forked tree
> > > and a private suite of checks that can't be released.
> > I don't think it's reasonable to expect the community to bear the
> > maintenance burden for internal-only checks for an organization. I
> > definitely understand the desire not to carry around a fork of clang-tidy
> > for this, but that doesn't seem like a good reason for us to spend cycles
> > reviewing these patches, maintaining them, and then eventually removing
> > them.
> >
> > We have some precedent in that we have clang-tidy checks for llvm coding
> > standards or google coding standards, etc but those are also used outside
> > of the particular community for which they're named. In this case, I don't
> > think the functionality is useful to anyone except Google. Is that correct?
> That makes sense in terms of investment from you and others (though I do
> appreciate the effort you've put in for reviewing it thus far), but I (and my
> team) are willing to bear the burden of reviewing/maintaining/removing them.
> If clang-tidy provided a way to tack on a set of external checks without
> forking, we'd happily do that, but that's not really possible right now.
>
> Would it be reasonable to do the review/maintenance/removal of these
> migration checks from within our team, such that we're not asking you to
> spend cycles on them?
>
>
> Would it be reasonable to do the review/maintenance/removal of these
> migration checks from within our team, such that we're not asking you to
> spend cycles on them?
Personally, I don't think so. It still impose burdens on users of clang-tidy by
having checks that are not going to be useful to anyone except people from one
company. These checks appears in clang-tidy list checks, contribute to binary
sizes, are documented in user-facing places, users may enable them needlessly
and report bugs against them, etc.
I think that we shouldn't have any checks that are only useful for
(effectively) a single company -- I don't think that scales well, and I don't
think it benefits the community of users. I think a better approach is to
introduce plugins for clang-tidy checks. It's definitely a larger project than
what you're proposing and I understand if you don't want to tackle it, but it
benefits the entire community and solves the problem you want to solve.
https://reviews.llvm.org/D54169
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54169: [clang-tidy] Zircon ->
juliehockett added inline comments.
Comment at: clang-tools-extra/clang-tidy/zircon/FblLimitsCheck.cpp:47
+SrcMgr::CharacteristicKind FileType) {
+ if (FileName == "fbl/limits.h") {
+unsigned End = std::strcspn(SM.getCharacterData(HashLoc), "\n") + 1;
aaron.ballman wrote:
> juliehockett wrote:
> > aaron.ballman wrote:
> > > juliehockett wrote:
> > > > aaron.ballman wrote:
> > > > > Does this also work on platforms where the path separator is `\`
> > > > > instead of `/`? What about case insensitive file systems where it may
> > > > > be spelled `LiMiTs.H`? Does this properly handle a case like:
> > > > > ```
> > > > > #define LIMITS "fbl/limits.h"
> > > > > #include LIMITS
> > > > > ```
> > > > > (Should add test cases for all of these scenarios.)
> > > > Since this is a migration for our own codebase, we know we don't have
> > > > any code that uses any variation other than and so
> > > > hardcoding that is acceptable to us here.
> > > Then why should this check be a public one, rather than an internal check?
> > I explained this on the other one, but for completeness:
> >
> >
> > So yes, this check is for a migration, and we would delete it once
> > regressions weren't possible. We would like the suite to be in upstream,
> > however, because we use the ToT llvm/clang/tools/etc, and don't want to
> > have to fork just to use clang-tidy for this sort of thing. Since
> > clang-tidy doesn't provide any way to have external checks to the tool
> > itself, upstreaming is the most ideal option.
> >
> > Orthogonal to our particular build setup, it'd also be nice to have an
> > example of this sort of migration done by clang-tidy in-tree. There has
> > been a lot of discussion recently about doing migrations with clang-tidy,
> > but it's always describing an internal migration that uses a forked tree
> > and a private suite of checks that can't be released.
> I don't think it's reasonable to expect the community to bear the maintenance
> burden for internal-only checks for an organization. I definitely understand
> the desire not to carry around a fork of clang-tidy for this, but that
> doesn't seem like a good reason for us to spend cycles reviewing these
> patches, maintaining them, and then eventually removing them.
>
> We have some precedent in that we have clang-tidy checks for llvm coding
> standards or google coding standards, etc but those are also used outside of
> the particular community for which they're named. In this case, I don't think
> the functionality is useful to anyone except Google. Is that correct?
That makes sense in terms of investment from you and others (though I do
appreciate the effort you've put in for reviewing it thus far), but I (and my
team) are willing to bear the burden of reviewing/maintaining/removing them. If
clang-tidy provided a way to tack on a set of external checks without forking,
we'd happily do that, but that's not really possible right now.
Would it be reasonable to do the review/maintenance/removal of these migration
checks from within our team, such that we're not asking you to spend cycles on
them?
https://reviews.llvm.org/D54169
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54169: [clang-tidy] Zircon ->
aaron.ballman added inline comments.
Comment at: clang-tools-extra/clang-tidy/zircon/FblLimitsCheck.cpp:47
+SrcMgr::CharacteristicKind FileType) {
+ if (FileName == "fbl/limits.h") {
+unsigned End = std::strcspn(SM.getCharacterData(HashLoc), "\n") + 1;
juliehockett wrote:
> aaron.ballman wrote:
> > juliehockett wrote:
> > > aaron.ballman wrote:
> > > > Does this also work on platforms where the path separator is `\`
> > > > instead of `/`? What about case insensitive file systems where it may
> > > > be spelled `LiMiTs.H`? Does this properly handle a case like:
> > > > ```
> > > > #define LIMITS "fbl/limits.h"
> > > > #include LIMITS
> > > > ```
> > > > (Should add test cases for all of these scenarios.)
> > > Since this is a migration for our own codebase, we know we don't have any
> > > code that uses any variation other than and so hardcoding
> > > that is acceptable to us here.
> > Then why should this check be a public one, rather than an internal check?
> I explained this on the other one, but for completeness:
>
>
> So yes, this check is for a migration, and we would delete it once
> regressions weren't possible. We would like the suite to be in upstream,
> however, because we use the ToT llvm/clang/tools/etc, and don't want to have
> to fork just to use clang-tidy for this sort of thing. Since clang-tidy
> doesn't provide any way to have external checks to the tool itself,
> upstreaming is the most ideal option.
>
> Orthogonal to our particular build setup, it'd also be nice to have an
> example of this sort of migration done by clang-tidy in-tree. There has been
> a lot of discussion recently about doing migrations with clang-tidy, but it's
> always describing an internal migration that uses a forked tree and a private
> suite of checks that can't be released.
I don't think it's reasonable to expect the community to bear the maintenance
burden for internal-only checks for an organization. I definitely understand
the desire not to carry around a fork of clang-tidy for this, but that doesn't
seem like a good reason for us to spend cycles reviewing these patches,
maintaining them, and then eventually removing them.
We have some precedent in that we have clang-tidy checks for llvm coding
standards or google coding standards, etc but those are also used outside of
the particular community for which they're named. In this case, I don't think
the functionality is useful to anyone except Google. Is that correct?
https://reviews.llvm.org/D54169
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54169: [clang-tidy] Zircon ->
juliehockett added inline comments.
Comment at: clang-tools-extra/clang-tidy/zircon/FblLimitsCheck.cpp:47
+SrcMgr::CharacteristicKind FileType) {
+ if (FileName == "fbl/limits.h") {
+unsigned End = std::strcspn(SM.getCharacterData(HashLoc), "\n") + 1;
aaron.ballman wrote:
> juliehockett wrote:
> > aaron.ballman wrote:
> > > Does this also work on platforms where the path separator is `\` instead
> > > of `/`? What about case insensitive file systems where it may be spelled
> > > `LiMiTs.H`? Does this properly handle a case like:
> > > ```
> > > #define LIMITS "fbl/limits.h"
> > > #include LIMITS
> > > ```
> > > (Should add test cases for all of these scenarios.)
> > Since this is a migration for our own codebase, we know we don't have any
> > code that uses any variation other than and so hardcoding
> > that is acceptable to us here.
> Then why should this check be a public one, rather than an internal check?
I explained this on the other one, but for completeness:
So yes, this check is for a migration, and we would delete it once regressions
weren't possible. We would like the suite to be in upstream, however, because
we use the ToT llvm/clang/tools/etc, and don't want to have to fork just to use
clang-tidy for this sort of thing. Since clang-tidy doesn't provide any way to
have external checks to the tool itself, upstreaming is the most ideal option.
Orthogonal to our particular build setup, it'd also be nice to have an example
of this sort of migration done by clang-tidy in-tree. There has been a lot of
discussion recently about doing migrations with clang-tidy, but it's always
describing an internal migration that uses a forked tree and a private suite of
checks that can't be released.
https://reviews.llvm.org/D54169
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54169: [clang-tidy] Zircon ->
aaron.ballman added inline comments.
Comment at: clang-tools-extra/clang-tidy/zircon/FblLimitsCheck.cpp:47
+SrcMgr::CharacteristicKind FileType) {
+ if (FileName == "fbl/limits.h") {
+unsigned End = std::strcspn(SM.getCharacterData(HashLoc), "\n") + 1;
juliehockett wrote:
> aaron.ballman wrote:
> > Does this also work on platforms where the path separator is `\` instead of
> > `/`? What about case insensitive file systems where it may be spelled
> > `LiMiTs.H`? Does this properly handle a case like:
> > ```
> > #define LIMITS "fbl/limits.h"
> > #include LIMITS
> > ```
> > (Should add test cases for all of these scenarios.)
> Since this is a migration for our own codebase, we know we don't have any
> code that uses any variation other than and so hardcoding that
> is acceptable to us here.
Then why should this check be a public one, rather than an internal check?
https://reviews.llvm.org/D54169
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54169: [clang-tidy] Zircon ->
juliehockett added inline comments.
Comment at: clang-tools-extra/clang-tidy/zircon/FblLimitsCheck.cpp:47
+SrcMgr::CharacteristicKind FileType) {
+ if (FileName == "fbl/limits.h") {
+unsigned End = std::strcspn(SM.getCharacterData(HashLoc), "\n") + 1;
aaron.ballman wrote:
> Does this also work on platforms where the path separator is `\` instead of
> `/`? What about case insensitive file systems where it may be spelled
> `LiMiTs.H`? Does this properly handle a case like:
> ```
> #define LIMITS "fbl/limits.h"
> #include LIMITS
> ```
> (Should add test cases for all of these scenarios.)
Since this is a migration for our own codebase, we know we don't have any code
that uses any variation other than and so hardcoding that is
acceptable to us here.
Comment at: clang-tools-extra/clang-tidy/zircon/FblLimitsCheck.cpp:70
+void FblLimitsCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(varDecl(hasType(cxxRecordDecl(allOf(
+ hasDeclContext(namespaceDecl(hasName("fbl"))),
aaron.ballman wrote:
> Can users specialize `fbl::numeric_limits` for custom integral types like
> they can for `std::numeric_limits`? If so, that should maybe be covered with
> a checker as well.
Technically, they could, but there are no instances of it in the codebase.
https://reviews.llvm.org/D54169
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54169: [clang-tidy] Zircon ->
juliehockett updated this revision to Diff 172891.
juliehockett marked 5 inline comments as done.
https://reviews.llvm.org/D54169
Files:
clang-tools-extra/clang-tidy/zircon/CMakeLists.txt
clang-tools-extra/clang-tidy/zircon/FblLimitsCheck.cpp
clang-tools-extra/clang-tidy/zircon/FblLimitsCheck.h
clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/zircon-fbl-limits.rst
clang-tools-extra/test/clang-tidy/Inputs/zircon/fbl/limits.h
clang-tools-extra/test/clang-tidy/Inputs/zircon/transitive.h
clang-tools-extra/test/clang-tidy/zircon-fbl-limits-transitive.cpp
clang-tools-extra/test/clang-tidy/zircon-fbl-limits.cpp
Index: clang-tools-extra/test/clang-tidy/zircon-fbl-limits.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/zircon-fbl-limits.cpp
@@ -0,0 +1,51 @@
+// RUN: %check_clang_tidy %s zircon-fbl-limits %t -- -- -isystem %S/Inputs/zircon
+
+#include
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: including fbl/limits.h is deprecated
+// CHECK-FIXES-NOT: #include
+// CHECK-FIXES: #include
+
+namespace fbl {
+
+template
+class numeric_limits {};
+
+#define SPECIALIZE_INT_FBL(type) \
+ template <>\
+ class numeric_limits {};
+
+SPECIALIZE_INT_FBL(short)
+
+numeric_limits lim;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use of fbl::numeric_limits is deprecated, use std::numeric_limits instead
+// CHECK-FIXES: std::numeric_limits lim;
+
+} // namespace fbl
+
+namespace std {
+
+template
+class numeric_limits {};
+
+#define SPECIALIZE_INT_STD(type) \
+ template <>\
+ class numeric_limits {};
+
+SPECIALIZE_INT_STD(short)
+
+numeric_limits lim;
+
+} // namespace std
+
+#define DECLARE() fbl::numeric_limits macrolim
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: use of fbl::numeric_limits is deprecated, use std::numeric_limits instead
+// CHECK-FIXES: #define DECLARE() std::numeric_limits macrolim
+
+int main() {
+ fbl::numeric_limits fbllim;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use of fbl::numeric_limits is deprecated, use std::numeric_limits instead
+ // CHECK-FIXES: std::numeric_limits fbllim;
+ std::numeric_limits stdlim;
+
+ DECLARE();
+}
Index: clang-tools-extra/test/clang-tidy/zircon-fbl-limits-transitive.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/zircon-fbl-limits-transitive.cpp
@@ -0,0 +1,11 @@
+// RUN: rm -rf %T/Headers
+// RUN: mkdir %T/Headers
+// RUN: cp -r %S/Inputs/zircon %T/Headers/zircon
+// RUN: %check_clang_tidy %s zircon-fbl-limits %t -- -header-filter=.* -- -std=c++11 -I %T/Headers/zircon
+// RUN: FileCheck -input-file=%T/Headers/zircon/transitive.h %s -check-prefix=CHECK-FIXES
+// RUN: rm -rf %T/Headers
+
+// transitive.h includes
+#include "transitive.h"
+// CHECK-NOTES: :2:1: warning: including fbl/limits.h is deprecated, transitively included from {{.*}}.
+// CHECK-FIXES-NOT: #include
Index: clang-tools-extra/test/clang-tidy/Inputs/zircon/transitive.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/Inputs/zircon/transitive.h
@@ -0,0 +1,2 @@
+// Transitively included headers.
+#include
Index: clang-tools-extra/docs/clang-tidy/checks/zircon-fbl-limits.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/zircon-fbl-limits.rst
@@ -0,0 +1,13 @@
+.. title:: clang-tidy - zircon-fbl-limits
+
+zircon-fbl-limits
+=
+
+This check is part of the migration checks for moving Zircon user code to use
+the C++ standard library.
+
+It suggests converting uses of ``fbl::numeric_limits`` to
+``std::numeric_limits``, and suggests inserting the header. It
+also suggests the removal of all uses of the header, as all
+declarations therein will be replaced by the check and the appropriate
+ replacement header recommended.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -254,4 +254,5 @@
readability-string-compare
readability-uniqueptr-delete-release
readability-uppercase-literal-suffix
+ zircon-fbl-limits
zircon-temporary-objects
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -182,6 +182,15 @@
` check does not warn
about calls inside macros anymore by default.
+- New :doc:`zircon-fbl-limits
+ ` check.
+
+ Suggests converting uses of ``fbl::numeric_limits`` to
+ ``std::numeric_limits``, and suggests
[PATCH] D54169: [clang-tidy] Zircon ->
aaron.ballman added inline comments.
Comment at: clang-tools-extra/clang-tidy/zircon/FblLimitsCheck.cpp:47
+SrcMgr::CharacteristicKind FileType) {
+ if (FileName == "fbl/limits.h") {
+unsigned End = std::strcspn(SM.getCharacterData(HashLoc), "\n") + 1;
Does this also work on platforms where the path separator is `\` instead of
`/`? What about case insensitive file systems where it may be spelled
`LiMiTs.H`? Does this properly handle a case like:
```
#define LIMITS "fbl/limits.h"
#include LIMITS
```
(Should add test cases for all of these scenarios.)
Comment at: clang-tools-extra/clang-tidy/zircon/FblLimitsCheck.cpp:70
+void FblLimitsCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(varDecl(hasType(cxxRecordDecl(allOf(
+ hasDeclContext(namespaceDecl(hasName("fbl"))),
Can users specialize `fbl::numeric_limits` for custom integral types like they
can for `std::numeric_limits`? If so, that should maybe be covered with a
checker as well.
Comment at: clang-tools-extra/clang-tidy/zircon/FblLimitsCheck.cpp:71
+ Finder->addMatcher(varDecl(hasType(cxxRecordDecl(allOf(
+ hasDeclContext(namespaceDecl(hasName("fbl"))),
+ hasName("numeric_limits")
`::fbl` instead of `fbl` to cover a situation like:
```
namespace oops {
namespace fbl {
struct numeric_limits {};
}
void foo() {
fbl::numeric_limits n;
}
}
https://reviews.llvm.org/D54169
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54169: [clang-tidy] Zircon ->
Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/zircon/FblLimitsCheck.cpp:111 + // Add in the header, since we know this file uses it. + if (auto IncludeFixit = Inserter->CreateIncludeInsertion( + SM.getFileID(V->getLocation()), "limits", Please don't use auto because type could not be deduced from statement. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:188 + + This check is part of the migration checks for moving Zircon user code to use + the C++ standard library. It suggests converting uses of declarations in the I think first statement is not necessary. Second should start from //Suggests//. Will be good idea to synchronize it with documentation. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:190 + the C++ standard library. It suggests converting uses of declarations in the + header to their std counterparts in . + Please highlight std with ``. https://reviews.llvm.org/D54169 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54169: [clang-tidy] Zircon ->
mcgrathr accepted this revision. mcgrathr added a comment. This revision is now accepted and ready to land. lgtm https://reviews.llvm.org/D54169 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54169: [clang-tidy] Zircon ->
juliehockett created this revision.
juliehockett added reviewers: aaron.ballman, alexfh, hokein.
juliehockett added a project: clang-tools-extra.
Herald added subscribers: xazax.hun, mgorny.
Adds a check to convert to std .
This check is part of a set of migration checks as we prepare to move Zircon
user code to use the C++ standard library, and should prevent regressions after
the migration is complete.
https://reviews.llvm.org/D54169
Files:
clang-tools-extra/clang-tidy/zircon/CMakeLists.txt
clang-tools-extra/clang-tidy/zircon/FblLimitsCheck.cpp
clang-tools-extra/clang-tidy/zircon/FblLimitsCheck.h
clang-tools-extra/clang-tidy/zircon/ZirconTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/zircon-fbl-limits.rst
clang-tools-extra/test/clang-tidy/Inputs/zircon/fbl/limits.h
clang-tools-extra/test/clang-tidy/Inputs/zircon/transitive.h
clang-tools-extra/test/clang-tidy/zircon-fbl-limits-transitive.cpp
clang-tools-extra/test/clang-tidy/zircon-fbl-limits.cpp
Index: clang-tools-extra/test/clang-tidy/zircon-fbl-limits.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/zircon-fbl-limits.cpp
@@ -0,0 +1,45 @@
+// RUN: %check_clang_tidy %s zircon-fbl-limits %t -- -- -isystem %S/Inputs/zircon
+
+#include
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: including fbl/limits.h is deprecated
+// CHECK-FIXES-NOT: #include
+// CHECK-FIXES: #include
+
+namespace fbl {
+
+template
+class numeric_limits {};
+
+#define SPECIALIZE_INT_FBL(type) \
+ template <>\
+ class numeric_limits {};
+
+SPECIALIZE_INT_FBL(short)
+
+numeric_limits lim;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use of fbl::numeric_limits is deprecated, use std::numeric_limits instead
+// CHECK-FIXES: std::numeric_limits lim;
+
+} // namespace fbl
+
+namespace std {
+
+template
+class numeric_limits {};
+
+#define SPECIALIZE_INT_STD(type) \
+ template <>\
+ class numeric_limits {};
+
+SPECIALIZE_INT_STD(short)
+
+numeric_limits lim;
+
+} // namespace std
+
+int main() {
+ fbl::numeric_limits fbllim;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use of fbl::numeric_limits is deprecated, use std::numeric_limits instead
+ // CHECK-FIXES: std::numeric_limits fbllim;
+ std::numeric_limits stdlim;
+}
Index: clang-tools-extra/test/clang-tidy/zircon-fbl-limits-transitive.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/zircon-fbl-limits-transitive.cpp
@@ -0,0 +1,11 @@
+// RUN: rm -rf %T/Headers
+// RUN: mkdir %T/Headers
+// RUN: cp -r %S/Inputs/zircon %T/Headers/zircon
+// RUN: %check_clang_tidy %s zircon-fbl-limits %t -- -header-filter=.* -- -std=c++11 -I %T/Headers/zircon
+// RUN: FileCheck -input-file=%T/Headers/zircon/transitive.h %s -check-prefix=CHECK-FIXES
+// RUN: rm -rf %T/Headers
+
+// transitive.h includes
+#include "transitive.h"
+// CHECK-NOTES: :2:1: warning: including fbl/limits.h is deprecated, transitively included from {{.*}}.
+// CHECK-FIXES-NOT: #include
Index: clang-tools-extra/test/clang-tidy/Inputs/zircon/transitive.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/Inputs/zircon/transitive.h
@@ -0,0 +1,2 @@
+// Transitively included headers.
+#include
Index: clang-tools-extra/docs/clang-tidy/checks/zircon-fbl-limits.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/zircon-fbl-limits.rst
@@ -0,0 +1,13 @@
+.. title:: clang-tidy - zircon-fbl-limits
+
+zircon-fbl-limits
+=
+
+This check is part of the migration checks for moving Zircon user code to use
+the C++ standard library.
+
+It suggests converting uses of ``fbl::numeric_limits`` to
+``std::numeric_limits``, and suggests inserting the header. It
+also suggests the removal of all uses of the header, as all
+declarations therein will be replaced by the check and the appropriate
+ replacement header recommended.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -254,4 +254,5 @@
readability-string-compare
readability-uniqueptr-delete-release
readability-uppercase-literal-suffix
+ zircon-fbl-limits
zircon-temporary-objects
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -182,6 +182,13 @@
` check does not warn
about calls inside macros anymore by default.
+- New :doc:`zircon-fbl-limits
+ ` check.
+
+ This check is part of the
