[PATCH] D54169: [clang-tidy] Zircon <fbl/limits.h> -> <limits>

[Aaron Ballman via Phabricator via cfe-commits]

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