[PATCH] D80961: WIP: Ignore template instantiations if not in AsIs mode

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D80961#2073049 , @klimek wrote:

> Without jumping into the discussion whether it should be the default, I think 
> we should be able to control template instantiation visitation separately 
> from other implicit nodes.
>  Having to put AsIs on a matcher every time you need to match template 
> instantiations is a rather big change (suddenly you have to change all the 
> matchers you've written so far).


I think that's the intended meaning of `AsIs` though. Template instantiations 
are not source code the user wrote, they're source code the compiler stamped 
out from code the user wrote. I hope `IgnoreUnlessSpelledInSource` isn't a 
misnomer.

> I love the idea of being able to control visitation of template instantiation.
>  I am somewhat torn on whether it should be the default, and would like to 
> see more data.
>  I feel more strongly about needing AsIs when I want to match template 
> instantiations.

FWIW, my experience in clang-tidy has been that template instantiations are 
ignored far more often than they're desired. In fact, instantiations tend to be 
a source of bugs for us because they're easy to forget about when writing 
matchers without keeping templates in mind. The times when template 
instantiations become important to *not* ignore within the checks is when the 
check is specific to template behavior, but that's a minority of the public 
checks thus far.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80961



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80961: WIP: Ignore template instantiations if not in AsIs mode

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

In D80961#2074915 , @steveire wrote:

> Hmm, `IgnoreUnlessSpelledInSource` is designed and named to fill that role.


In other words: to me this change is a bug fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80961



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80961: WIP: Ignore template instantiations if not in AsIs mode

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

In D80961#2073049 , @klimek wrote:

> Without jumping into the discussion whether it should be the default, I think 
> we should be able to control template instantiation visitation separately 
> from other implicit nodes.


Hmm, `IgnoreUnlessSpelledInSource` is designed and named to fill that role. 
After this patch is in, I intend to make it also control whether the implicit 
node here is dumped/matchable: https://godbolt.org/z/V_KCFm

> Having to put AsIs on a matcher every time you need to match template 
> instantiations is a rather big change (suddenly you have to change all the 
> matchers you've written so far).

I don't think that's true. You have to change the matchers you've written which 
deliberately match the instantiation, but you can also (optionally) simplify 
the others to remove `unless(isInTemplateInstantiation())` from them. The third 
category of matchers are the ones where you haven't used 
`unless(isInTemplateInstantiation())` even though you should have and you have 
a bug that you don't know about yet. This change fixes those ones.

> I love the idea of being able to control visitation of template instantiation.
>  I am somewhat torn on whether it should be the default, and would like to 
> see more data.
>  I feel more strongly about needing AsIs when I want to match template 
> instantiations.

I feel strongly that the default should not change code in a known-wrong way, 
as the unit test demonstrates. It's not a novice-friendly default.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80961



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80961: WIP: Ignore template instantiations if not in AsIs mode

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

Without jumping into the discussion whether it should be the default, I think 
we should be able to control template instantiation visitation separately from 
other implicit nodes.
Having to put AsIs on a matcher every time you need to match template 
instantiations is a rather big change (suddenly you have to change all the 
matchers you've written so far).
I love the idea of being able to control visitation of template instantiation.
I am somewhat torn on whether it should be the default, and would like to see 
more data.
I feel more strongly about needing AsIs when I want to match template 
instantiations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80961



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80961: WIP: Ignore template instantiations if not in AsIs mode

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

In D80961#2068865 , @ymandel wrote:

> Thank you for bringing up this issue. I think it highlights an underlying 
> problem -- editing templates is quite difficult -- that neither setting will 
> address, as Dmitri expanded on above. Given the parallel to macros, I'd say 
> your change is better than the status quo. Most clang tidies and other 
> rewriting tools that I've encountered simply skip code in macro expansions, 
> rather than reason about how to update the macro definition or whatnot. So, 
> by that reasoning, we should skip template instantations.


I've personally written quite a few tools that changed macro definitions. Do 
you have numbers on how many of the library team's tools generally want to 
match template instantiations?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80961



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80961: WIP: Ignore template instantiations if not in AsIs mode

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

In D80961#2068902 , @aaron.ballman 
wrote:

> My experience with clang-tidy has been that template instantiations are a 
> double-edged sword. The instantiation is the only place at which you have 
> sufficient information to perform many kinds of analyses, but it's also often 
> not plausible to modify the templated code because it's not clear from the 
> instantiation context how changes may impact other instantiations. The result 
> is that most of the clang-tidy checks wind up punting on template 
> instantiations similar to what they do for macros. Based on that experience, 
> I think it makes sense to continue in the proposed direction.


Great. I also think that given that the default has just been changed to 
`TK_IgnoreUnlessSpelledInSource`, we should change this soon. The facts would 
still be the same if we do this at some future date, but it would be needlessly 
more disruptive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80961



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80961: WIP: Ignore template instantiations if not in AsIs mode

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

In D80961#2067638 , @gribozavr2 wrote:

> If IgnoreUnlessSpelledInSource is indeed for novice users (and not to be 
> strictly interpreted as "it does what it says") we should think about whether 
> it more useful to ignore instantiations or to match in instantiations.


Tools should generally follow the Hippocratic: First do no harm.

As I demonstrated, simple tools currently accidentally make unintended and 
incorrect changes to code. That is wrong.

Do you have a standing objection to this being changed?

I can't really work on this if I know or assume that you're going to prevent it 
going in (ever or for a significant time which results in this being harder 
than it should be).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80961



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80961: WIP: Ignore template instantiations if not in AsIs mode

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

In D80961#2068865 , @ymandel wrote:

> Thank you for bringing up this issue. I think it highlights an underlying 
> problem -- editing templates is quite difficult -- that neither setting will 
> address, as Dmitri expanded on above. Given the parallel to macros, I'd say 
> your change is better than the status quo. Most clang tidies and other 
> rewriting tools that I've encountered simply skip code in macro expansions, 
> rather than reason about how to update the macro definition or whatnot. So, 
> by that reasoning, we should skip template instantations.


Yes, it's generally true for tools that given the choice of

1. possibly making a change which is definitely incorrect (as is currently done 
by default and as demonstrated in the Transformer test case)
2. not making a particular change meaning the overall change is incomplete (as 
in Dmitris response)

number (2) would always be way to go. I don't think Dmitris objection makes any 
sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80961



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80961: WIP: Ignore template instantiations if not in AsIs mode

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

My experience with clang-tidy has been that template instantiations are a 
double-edged sword. The instantiation is the only place at which you have 
sufficient information to perform many kinds of analyses, but it's also often 
not plausible to modify the templated code because it's not clear from the 
instantiation context how changes may impact other instantiations. The result 
is that most of the clang-tidy checks wind up punting on template 
instantiations similar to what they do for macros. Based on that experience, I 
think it makes sense to continue in the proposed direction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80961



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80961: WIP: Ignore template instantiations if not in AsIs mode

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

Thank you for bringing up this issue. I think it highlights an underlying 
problem -- editing templates is quite difficult -- that neither setting will 
address, as Dmitri expanded on above. Given the parallel to macros, I'd say 
your change is better than the status quo. Most clang tidies and other 
rewriting tools that I've encountered simply skip code in macro expansions, 
rather than reason about how to update the macro definition or whatnot. So, by 
that reasoning, we should skip template instantations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80961



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80961: WIP: Ignore template instantiations if not in AsIs mode

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a comment.

I think it makes sense to align the documentation and the definition of 
IgnoreUnlessSpelledInSource, I'm just not sure which component we should change 
-- the documentation or the implementation.

Making IgnoreUnlessSpelledInSource not match template instantiations is a nice 
and simple mental model. However, I'm not sure it is more useful in practice, 
and here's why. Processing non-instantiated templates is generally very 
difficult to do correctly: major parts of AST semantics are still unknown, and 
the parts that can be known are not reliably available. Clang does not provide 
guarantees about what parts of the template AST are type checked and what are 
not (I think there are only aspirations), so AST matchers that work in simple 
tests (where Clang decides to type check more expressions) could fail in more 
complex templates (where Clang decides to type check less). Generally, 
processing non-instantiated templates is difficult for the same reasons that 
make analyzing macro definitions difficult. Analyzing macro definitions is so 
difficult that tools generally don't do it at all and analyze expansions 
instead. Analyzing template definitions is more tractable (at least we have 
some AST nodes), but still, in practice, I'd generally recommend people to 
analyze template instantiations instead and if they need to perform a 
refactoring (like renaming a method), trying to cross-reference information 
across multiple template instantiations.

If IgnoreUnlessSpelledInSource is indeed for novice users (and not to be 
strictly interpreted as "it does what it says") we should think about whether 
it more useful to ignore instantiations or to match in instantiations.

For example, imagine someone who is trying to find usages of a certain function 
in their codebase by writing a matcher (for example so that they can rename it, 
or delete as dead code or whatever -- "find usages" is a very common first 
step). A matcher in IgnoreUnlessSpelledInSource mode  that ignores 
instantiations could find no matches when in fact there are usages from 
templates. The user deletes the function, and in the best case they get a 
compilation error (and frustration because the AST matchers lied to them), or 
in the worst case due to the magic of overload resolution and ADL the code 
still compiles, but calls a different function that was in the overload set.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80961



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80961: WIP: Ignore template instantiations if not in AsIs mode

[Stephen Kelly via Phabricator via cfe-commits]

steveire updated this revision to Diff 267750.
steveire added a comment.

Update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80961

Files:
  clang-tools-extra/clang-tidy/abseil/TimeSubtractionCheck.cpp
  clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UndefinedMemoryManipulationCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/VirtualNearMissCheck.cpp
  clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.cpp
  clang-tools-extra/clang-tidy/hicpp/ExceptionBaseclassCheck.cpp
  clang-tools-extra/clang-tidy/modernize/DeprecatedIosBaseAliasesCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseUncaughtExceptionsCheck.cpp
  clang-tools-extra/clang-tidy/performance/FasterStringFindCheck.cpp
  clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
  clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp
  
clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
  clang-tools-extra/clang-tidy/zircon/TemporaryObjectsCheck.cpp
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/lib/AST/ASTDumper.cpp
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/unittests/AST/ASTTraverserTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -695,6 +695,70 @@
   EXPECT_EQ(ErrorCount, 0);
 }
 
+TEST_F(TransformerTest, TemplateInstantiation) {
+
+  std::string NonTemplatesInput = R"cpp(
+struct S {
+  int m_i;
+};
+)cpp";
+  std::string NonTemplatesExpected = R"cpp(
+struct S {
+  safe_int m_i;
+};
+)cpp";
+
+  std::string TemplatesInput = R"cpp(
+template
+struct TemplStruct {
+  TemplStruct() {}
+  ~TemplStruct() {}
+
+private:
+  T m_t;
+};
+
+void instantiate()
+{
+  TemplStruct ti;
+}
+)cpp";
+
+  auto MatchedField = fieldDecl(hasType(asString("int"))).bind("theField");
+
+  // Changes the 'int' in 'S', but not the 'T' in 'TemplStruct':
+  testRule(makeRule(traverse(TK_IgnoreUnlessSpelledInSource, MatchedField),
+changeTo(cat("safe_int ", name("theField",
+   NonTemplatesInput + TemplatesInput,
+   NonTemplatesExpected + TemplatesInput);
+
+  // In AsIs mode, template instantiations are modified, which is
+  // often not desired:
+
+  std::string IncorrectTemplatesExpected = R"cpp(
+template
+struct TemplStruct {
+  TemplStruct() {}
+  ~TemplStruct() {}
+
+private:
+  safe_int m_t;
+};
+
+void instantiate()
+{
+  TemplStruct ti;
+}
+)cpp";
+
+  // Changes the 'int' in 'S', and (incorrectly) the 'T' in 'TemplStruct':
+  testRule(makeRule(traverse(TK_AsIs, MatchedField),
+changeTo(cat("safe_int ", name("theField",
+
+   NonTemplatesInput + TemplatesInput,
+   NonTemplatesExpected + IncorrectTemplatesExpected);
+}
+
 // Transformation of macro source text when the change encompasses the entirety
 // of the expanded text.
 TEST_F(TransformerTest, SimpleMacro) {
Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -1890,7 +1890,60 @@
substNonTypeTemplateParmExpr(has(integerLiteral());
 
   EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource,
- staticAssertDecl(has(integerLiteral());
+ staticAssertDecl(has(declRefExpr());
+
+  Code = R"cpp(
+
+template
+struct TemplStruct {
+  TemplStruct() {}
+  ~TemplStruct() {}
+
+private:
+  T m_t;
+};
+
+template
+T timesTwo(T input)
+{
+  return input * 2;
+}
+
+void instantiate()
+{
+  TemplStruct ti;
+  TemplStruct td;
+  (void)timesTwo(2);
+  (void)timesTwo(2);
+}
+
+)cpp";
+  {
+auto M = cxxRecordDecl(hasName("TemplStruct"),
+   has(fieldDecl(hasType(asString("int");
+EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+EXPECT_FALSE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+  }
+  {
+auto M = cxxRecordDecl(hasName("TemplStruct"),
+   has(fieldDecl(hasType(asString("double");
+

[PATCH] D80961: WIP: Ignore template instantiations if not in AsIs mode

[Stephen Kelly via Phabricator via cfe-commits]

steveire created this revision.
steveire added reviewers: sammccall, aaron.ballman, gribozavr2, ymandel, klimek.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
steveire updated this revision to Diff 267750.
steveire added a comment.

Update


IgnoreUnlessSpelledInSource mode should ignore these because they are
not written in the source.  This matters for example when trying to
replace types or values which are templated.  The new test in
TransformerTest.cpp in this commit demonstrates the problem.

In existing matcher code, users can write
`unless(isInTemplateInstantiation())` or `unless(isInstantiated())` (the
user must know which to use).  The point of the
TK_IgnoreUnlessSpelledInSource mode is to allow the novice to avoid such
details.  This patch changes the IgnoreUnlessSpelledInSource mode to
skip over implicit template instantiations.

This patch does not change the TK_AsIs mode. Adjust existing clang-tidy
matchers which explicitly desire to match implicit template
instantiations.

Note: An obvious attempt at an alternative implementation would simply
change the shouldVisitTemplateInstantiations() in ASTMatchFinder.cpp to
return something conditional on the operational TraversalKind.  That
does not work because shouldVisitTemplateInstantiations() is called
before a possible top-level traverse() matcher changes the operational
TraversalKind.

WIP: This change is WIP because I have not yet adjusted unit tests which
currently rely on the current behavior.  I am soliciting feedback to
determine whether there is mood to proceed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80961

Files:
  clang-tools-extra/clang-tidy/abseil/TimeSubtractionCheck.cpp
  clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UndefinedMemoryManipulationCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/VirtualNearMissCheck.cpp
  clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.cpp
  clang-tools-extra/clang-tidy/hicpp/ExceptionBaseclassCheck.cpp
  clang-tools-extra/clang-tidy/modernize/DeprecatedIosBaseAliasesCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseUncaughtExceptionsCheck.cpp
  clang-tools-extra/clang-tidy/performance/FasterStringFindCheck.cpp
  clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
  clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp
  
clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp
  clang-tools-extra/clang-tidy/zircon/TemporaryObjectsCheck.cpp
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/lib/AST/ASTDumper.cpp
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/unittests/AST/ASTTraverserTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -695,6 +695,70 @@
   EXPECT_EQ(ErrorCount, 0);
 }
 
+TEST_F(TransformerTest, TemplateInstantiation) {
+
+  std::string NonTemplatesInput = R"cpp(
+struct S {
+  int m_i;
+};
+)cpp";
+  std::string NonTemplatesExpected = R"cpp(
+struct S {
+  safe_int m_i;
+};
+)cpp";
+
+  std::string TemplatesInput = R"cpp(
+template
+struct TemplStruct {
+  TemplStruct() {}
+  ~TemplStruct() {}
+
+private:
+  T m_t;
+};
+
+void instantiate()
+{
+  TemplStruct ti;
+}
+)cpp";
+
+  auto MatchedField = fieldDecl(hasType(asString("int"))).bind("theField");
+
+  // Changes the 'int' in 'S', but not the 'T' in 'TemplStruct':
+  testRule(makeRule(traverse(TK_IgnoreUnlessSpelledInSource, MatchedField),
+changeTo(cat("safe_int ", name("theField",
+   NonTemplatesInput + TemplatesInput,
+   NonTemplatesExpected + TemplatesInput);
+
+  // In AsIs mode, template instantiations are modified, which is
+  // often not desired:
+
+  std::string IncorrectTemplatesExpected = R"cpp(
+template
+struct TemplStruct {
+  TemplStruct() {}
+  ~TemplStruct() {}
+
+private:
+  safe_int m_t;
+};
+
+void instantiate()
+{
+  TemplStruct ti;
+}
+)cpp";
+
+  // Changes the 'int' in 'S', and (incorrectly) the 'T' in 'TemplStruct':
+  testRule(makeRule(traverse(TK_AsIs, MatchedField),
+changeTo(cat("safe_int ", name("theField",
+
+   NonTemplatesInput + TemplatesInput,
+   NonTemplatesExpected + IncorrectTemplatesExpected);
+}
+
 // Transformation of