[cfe-commits] [PATCH] Introduces DynTypedMatcher as a new concept that replaces the UntypedBaseMatcher and TypedMatcher. Due to DynTypedNode the basic dynamically typed matcher interface can now be simplified.

[Sean Silva]

  > /// Note that we can store \c Decls and \c Stmts by pointer as they are
  > /// guaranteed be unique pointers pointing to dedicated storage in the
  > /// AST. \c QualTypes on the other hand do not have storage or unique
  > /// pointers and thus need to be stored by value.
  > /// Further info: <reference to QualType>

  This sounds more like it should be in Clang AST 101, not in the depths
  of the AST matcher implementation. Someone who doesn't know what a
  QualType is isn't going to make it this far into the file; as in all
  writing, considering your audience is important. As such, IMO this
  comment isn't necessary. I think that Manuel's original comment very
  nicely summarizes the status quo as far as "why we're storing the
  QualType by value"; namely, that this is what the AST gives us.

  However, an explanation is needed here:

  /// \brief Supported base node types.
    enum NodeTypeTag {
      NT_Decl,
      NT_Stmt,
      NT_QualType
    } Tag;

  , because QualType isn't really a "base node". Since Type/TypeLoc
  _are_ actually bases, an explanation for why and in what sense
  QualType is being used as a "base" here and not Type* or TypeLoc would
  be highly desirable. Or maybe "base" is subtly off from being the
  right concept here, which I think is more along the lines of "a handle
  for a distinct kind of entity in the AST"; it may be easier to just
  say "base" though :P.

  --Sean Silva

http://llvm-reviews.chandlerc.com/D33