[PATCH] D52782: [clang-tidy] Sequence statements with multiple parents correctly (PR39149)

[Martin Böhme via Phabricator via cfe-commits]

mboehme marked 3 inline comments as done.
mboehme added inline comments.


================
Comment at: clang-tidy/utils/ExprSequence.cpp:103
   for (const Stmt *Parent : getParentStmts(S, Context)) {
+    // For statements that have multiple parents, make sure we're using the
+    // parent that lies within the sub-tree under Root.
----------------
aaron.ballman wrote:
> JonasToth wrote:
> > I find the first part of the comment unclear. Does this loop handle `for` only?
> I think this means English "for" and not C `for`. Could rewrite to `If a statement has multiple parents, ` instead.
Yes, this is what I meant. Rephrased as Aaron suggested to remove the ambiguity.


================
Comment at: test/clang-tidy/bugprone-use-after-move.cpp:1198
 
+// Null statements (and some other statements) in templates may be shared
+// between the uninstantiated and instantiated versions of the template and
----------------
JonasToth wrote:
> Which other stmts? do they produce the same false positive?
I've added the two other examples I'm aware of (continue and break statements) to the description. However, I haven't been able to use these to construct an example that triggers the false positive.

In general, any statement that TemplateInstantiator leaves unchanged will have multiple parents; I'm not sure which other statements this applies to. In my experience, any statement that contains an expression will be rebuilt, but I may be missing something here.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52782