[PATCH] D60151: [clang-tidy] Add using SmallSet to LLVM.h to fix bug in typedef in llvm checkers.

Don Hinton via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 3 08:21:23 PDT 2019


hintonda marked an inline comment as done.
hintonda added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h:21
 
-typedef llvm::SmallSet<llvm::StringRef, 5> HeaderFileExtensionsSet;
+using HeaderFileExtensionsSet = SmallSet<StringRef, 5>;
 
----------------
aaron.ballman wrote:
> I do not like that we're removing the namespace qualifier here. I would prefer to leave it as `::llvm::SmallSet<::llvm::StringRef, 5>` if there is a namespace clash.
Other than aesthetics, the reason I don't like the idea of fully scoping these types, at least without a comment, is that the error is triggered by some other code gets included first, and has nothing to do with this code -- there's nothing actually wrong with the original code.  So it could/would be confusing for a reader later on wondering why you needed to fully scope these types, and not others.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60151/new/

https://reviews.llvm.org/D60151





More information about the cfe-commits mailing list