aaron.ballman added inline comments.
================ Comment at: clang-tidy/abseil/QualifiedAliasesCheck.cpp:29 + // Finds the nested-name-specifier location. + const NestedNameSpecifierLoc QualifiedLoc = MatchedDecl->getQualifierLoc(); + const SourceLocation FrontLoc = QualifiedLoc.getBeginLoc(); ---------------- Drop the top-level `const` qualifier (here and elsewhere). ================ Comment at: clang-tidy/abseil/QualifiedAliasesCheck.cpp:34 + const SourceManager *SM = Result.SourceManager; + CharSourceRange FrontRange = CharSourceRange(); + FrontRange.setBegin(FrontLoc); ---------------- No need for the assignment. ================ Comment at: clang-tidy/abseil/QualifiedAliasesCheck.cpp:39 + + // If the using declaration is fully qualified, don't produce a warning. + if (Beg.startswith("::")) ---------------- This is missing a check for whether the referenced name is within the current namespace. ================ Comment at: clang-tidy/abseil/QualifiedAliasesCheck.cpp:43-44 + + diag(FrontLoc, "using declaration is not fully qualified: see " + "https://abseil.io/tips/119"); +} ---------------- Do not put HTML links into the diagnostic -- the wording should stand on its own. I would probably phrase it "using declaration should use a fully qualified name" or something along those lines. ================ Comment at: clang-tidy/abseil/QualifiedAliasesCheck.h:19 + +/// FIXME: Write a short description. +/// ---------------- This should be fixed. ================ Comment at: test/clang-tidy/abseil-qualified-aliases.cpp:22-23 + + using innermost::Color; +// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: using declaration is not fully qualified: see https://abseil.io/tips/119 [abseil-qualified-aliases] + ---------------- I don't think this should warn, per the tip. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55346/new/ https://reviews.llvm.org/D55346 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits