[PATCH] cpp11-migrate: Add a class to support include directives modifications

Edwin Vane edwin.vane at intel.com
Thu Aug 15 11:20:04 PDT 2013


  Just a few small things from me.

  I would remove mention of what you call X Macro Headers and just say "headers meant for multiple inclusion".

  Otherwise, the rest of the code, albeit complex, seems ok. I couldn't see any tests that were obviously missing.


================
Comment at: cpp11-migrate/Core/IncludeDirectives.h:52
@@ +51,3 @@
+
+  /// \brief Check if \p Include is included by \p File or any of the file \p
+  /// File includes.
----------------
file -> files

================
Comment at: cpp11-migrate/Core/IncludeDirectives.h:44
@@ +43,3 @@
+  ///
+  /// \return A replacement that is either null if the include already visible
+  /// by file (check with \c Replacement::isApplicable()) or non-null
----------------
I'd reword this for clarity this way:

  \returns \li A null Replacement (check using \c Replacement::isApplicable()), if the \c Include is already visible from \c File.
           \li Otherwise, a non-null Replacement that, when applied, inserts an \c #include into \c File.


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

BRANCH
  include-directives

ARCANIST PROJECT
  clang-tools-extra



More information about the cfe-commits mailing list