[PATCH] Template Instantiation Observer + a few other templight-related changes

[Sean Silva]

On Wed, Jan 7, 2015 at 8:38 PM, Nick Lewycky <nlewycky at google.com> wrote:

> There's a few minor cosmetic issues that apply across multiple files:
>
> - Some if-statements have extra spaces inside the parentheses. "if (foo)"
> not "if ( foo )".
> - Some files are inconsistent about * and & placement in types. We usually
> place them next to the variable names.
> - include/clang/Sema/ActiveTemplateInst.h has extra blank lines.
>
> You may find it simplest to use clang-format-diff.py.
>
> I think we'll want a way to test out this interface; it should probably
> come with a command-line tool that has a very simple template observer that
> prints out the callbacks as they're called, and a test which runs this tool
> to exercise it. This might also be a FrontendAction in clang, similar to
> -ast-dump, instead of a separate tool. (This is not a trivial amount of
> work, you may want to wait for another clang developer to chime in before
> spending time on it.)
>

Other than -ast-dump, there is also pp-trace which is in clang-tools-extra.
However, we don't depend on pp-trace for the actual testing. It seems we
test PPCallbacks with a unittest.

Personally, I would prefer something FileCheck-able though.


(also, minor bikeshed, but naming this *Callbacks instead of *Observer to
match with PPCallbacks would be nice)

-- Sean Silva


>
> Overall I'm happy with the design!
>
>
> ================
> Comment at: include/clang/Sema/TemplateInstObserver.h:25-26
> @@ +24,4 @@
> +
> +  class Sema;
> +  class ActiveTemplateInstantiation;
> +
> ----------------
> Alphabetize.
>
> ================
> Comment at: include/clang/Sema/TemplateInstObserver.h:28
> @@ +27,3 @@
> +
> +/// \brief This is a base class for an observer that will be notified
> (callback) at every template instantiation.
> +class TemplateInstantiationObserver {
> ----------------
> What about a callback when clang transitions from parsing the TU to
> actOnEndOfTranslationUnit? Template instantiation is different before and
> after we reach the end-of-TU.
>
> ================
> Comment at: lib/Sema/SemaType.cpp:5202
> @@ -5200,1 +5201,3 @@
>      }
> +    else if (Def && TemplateInstObserverChain) {
> +      ActiveTemplateInstantiation TempInst;
> ----------------
> Is there a guarantee that this is a template? Offhand it looks like this
> could fire on any struct, couldn't it?
>
> http://reviews.llvm.org/D5767
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150108/1cc78ada/attachment.html>