[PATCH] D25959: Introduce updateDiscriminator interface to DILocation to make it cleaner assigning discriminators.

Adrian Prantl via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 25 13:21:21 PDT 2016


aprantl added inline comments.


================
Comment at: include/llvm/IR/DebugInfoMetadata.h:1274
+  /// Returns a new DILocation with updated \p Discriminator.
+  inline DILocation *updateDiscriminator(unsigned Discriminator) const;
+
----------------
"update" doesn't feel quite right. Would cloneWithDiscriminator() or something along those lines be any better?


================
Comment at: include/llvm/IR/DebugInfoMetadata.h:1620
+  DIScope *Scope = getScope();
+  for (DILexicalBlockFile *LBF = dyn_cast<DILexicalBlockFile>(Scope);
+       LBF && LBF->getDiscriminator() != 0;
----------------
We usually use auto if the type is obvious from the context.
I think a while-loop may be less compact but more readable here? Or maybe just add a comment explaining what the loop does?


https://reviews.llvm.org/D25959





More information about the llvm-commits mailing list