[PATCH] D40205: [msan] Don't sanitize "nosanitize" instructions

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka added inline comments.


================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:1344
   // ------------------- Visitors.
+  using InstVisitor<MemorySanitizerVisitor>::visit;
+  void visit(Instruction &I) {
----------------
eugenis wrote:
> what's the point of this using statement?
> 
> I think it would be more readable to do this in runOnFunction (see the depth_first loop over basic blocks).
InstVisitor has other overloads of visit()
c++ stops looking for overloads if it found something in the current class
if I add mine here "visit(basic_block)" will try to call mine and fail.
"using" inherit all overloads from parent class, so for basic block still will be able to call the same overload as before.

seems like this visitor exists for things exactly like this, and I see similar usage in other places. moving check into runOnFunction will make us to copy loop over instructions which does not look more readable to me. I'd prefer to keep as much as possible logic in InstVisitor. I guess we have own look for bblocks in runOnFunction only to have depth_first.


https://reviews.llvm.org/D40205