[PATCH] Adding Replacement serialization support

[Manuel Klimek]

On Fri, Aug 16, 2013 at 7:32 PM, Edwin Vane <edwin.vane at intel.com> wrote:

>
>
> ================
> Comment at: include/clang/Tooling/ReplacementsYaml.h:28
> @@ +27,3 @@
> +/// \brief The top-level YAML document that contains all Replacements.
> +struct ReplacementsDocument {
> +  /// A freeform chunk of text to describe the context of the
> replacements in
> ----------------
> Manuel Klimek wrote:
> > I'd vote against calling this "Document". To me, a document is something
> I edit :)
> > Perhaps we can just call it yaml::Replacement? or YAMLReplacement?
> I'm using YAML nomenclature here. A document is everything between --- and
> ... in a YAML file where a YAML file can have multiple documents. Given
> that reasoning do you still feel strongly about renaming it?
>

I figured that that's the case. My question is, whether we really want the
strange names of the YAML domain to blend in here...


> ================
> Comment at: include/clang/Tooling/ReplacementsYaml.h:31
> @@ +30,3 @@
> +  /// this document.
> +  std::string Context;
> +
> ----------------
> Manuel Klimek wrote:
> > I'd put some stronger idea of what we want here into the comment:
> > // A chunk of text that gives context on how the replacement was
> generated (for example,
> > // the translation unit parsed, the location of a template
> instantiation, etc).
> > // This will be used for diagnostic purposes if the application of a
> replacement fails.
> >
> > Furthermore, why don't we have one of those per replacement? If it's
> useful enough, I think we should really put it into the Replacement class
> as an optional field, and provide nice libs to populate it from a
> clang-tool.
> >
> > Also, I'd perhaps call it DiagnosticInfo (but I don't really feel
> strongly here, just throwing out an idea).
> If we put Context into Replacement I'd definitely want to name it
> something that implies it's an annotation and not part of Replacement
> comparison operators. Perhaps "metadata" a la LLVM?
>

I think "metadata" basically tells me nothing. I'd want to make sure it's
intended to be used for diagnostics, and that it should include "how" the
replacement was generated.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130819/b48958f8/attachment.html>