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

Edwin Vane edwin.vane at intel.com
Fri Aug 9 06:40:20 PDT 2013

  So do you have solutions for the problem of headers with header guards but no includes? I'm fine documenting the //X Macro header// problem as a known shortcoming for now. I'd place a FIXME somewhere if there's an appropriate spot. Also log a bug in JIRA.

  How easy is it to detect if a header doesn't have header guards? You could use that as a test to identify these //X Macro headers// which have the common feature of being intended for multiple inclusion. Once you can identify such inclusions you can disregard considering them as insertion points for new includes.

Comment at: cpp11-migrate/Core/IncludeDirectives.h:58
@@ +57,3 @@
+  friend class IncludeDirectivesPPCallback;
Why do we even need friendship here? I know you have `InclusionDirective()` as private but according to docs, this is actually a public member of the clang::PPCallbacks class. You should probably also make it public. At this point, friendship is no longer necessary and then you can put the callbacks class into an anonymous namespace.

Comment at: unittests/cpp11-migrate/IncludeDirectivesTest.cpp:165-167
@@ +164,5 @@
+TEST(IncludeDirectivesTest, indirectIncludes) {
+      "#include </virtual/foo.h>\n",
+      addIncludeInCode("/virtual/foo-inner.h", "#include </virtual/foo.h>\n"));
I'm still not convinced this EXPECT is testing what you think. I understand the point of this test is to test indirect includes but this particular EXPECT is not using `makeIndirectTestsAction()` so as far as I see, it's identical to the avoidDuplicates() test. What am I missing?


More information about the cfe-commits mailing list