pgousseau marked 2 inline comments as done. pgousseau added inline comments.
================ Comment at: include/clang/Basic/Sanitizers.h:29 +class hash_code; +} ---------------- riccibruno wrote: > pgousseau wrote: > > riccibruno wrote: > > > What ? You are forward-declaring `hash_code` here and using it as a value > > > a few lines later. Just include `llvm/ADT/Hashing.h`. > > Thanks for reviewing! > > I am happy to include `Hashing.h` but I thought it was generally frown upon > > to add includes if avoidable? > > `hash_code` is only used in the return type of the `hash_value()` > > declaration here no? > > I saw StringRef.h or APInt.h also forward declare `hash_code`. > > Thanks again for reviewing and sorry for the many iterations. > Never mind, I overlooked that it is only the declaration of `llvm::hash_code > hash_value(const clang::SanitizerMask &Arg);` and not the definition. Sorry > about that. > > I believe that is it correct to have the overload of `hash_value` in the > namespace `clang`, so that it can be found by ADL since `SanitizerMask` is in > `clang`. However you can probably move the declaration of `llvm::hash_code > hash_value(const clang::SanitizerMask &Arg);` just after `SanitizerMask` so > that you don't have to forward-declare `SanitizerMask` (but keep the > forward-declaration of `hash_code` in `llvm`). Sounds good, done! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57914/new/ https://reviews.llvm.org/D57914 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits