[PATCH] D17794: [modules] addHeaderInclude() can't fail.

Davide Italiano via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 1 19:37:16 PST 2016


davide created this revision.
davide added reviewers: silvas, rsmith, christof.
davide added a subscriber: cfe-commits.

I noticed that addHeaderInclude() returns bool although it can never fail. 
I find this a little bit weird from a semantic point of view. My best guess is that the 'bool' return type was put there 'just in case'.
I'd like to change its return type to void until real evidence that this function can fail is provided, unless, of course, I'm missing something.


http://reviews.llvm.org/D17794

Files:
  lib/Frontend/FrontendActions.cpp

Index: lib/Frontend/FrontendActions.cpp
===================================================================
--- lib/Frontend/FrontendActions.cpp
+++ lib/Frontend/FrontendActions.cpp
@@ -152,10 +152,10 @@
   return Includes;
 }
 
-static std::error_code addHeaderInclude(StringRef HeaderName,
-                                        SmallVectorImpl<char> &Includes,
-                                        const LangOptions &LangOpts,
-                                        bool IsExternC) {
+static void addHeaderInclude(StringRef HeaderName,
+                             SmallVectorImpl<char> &Includes,
+                             const LangOptions &LangOpts,
+                             bool IsExternC) {
   if (IsExternC && LangOpts.CPlusPlus)
     Includes += "extern \"C\" {\n";
   if (LangOpts.ObjC1)
@@ -168,7 +168,6 @@
   Includes += "\"\n";
   if (IsExternC && LangOpts.CPlusPlus)
     Includes += "}\n";
-  return std::error_code();
 }
 
 /// \brief Collect the set of header includes needed to construct the given 
@@ -194,22 +193,17 @@
       // file relative to the module build directory (the directory containing
       // the module map file) so this will find the same file that we found
       // while parsing the module map.
-      if (std::error_code Err = addHeaderInclude(H.NameAsWritten, Includes,
-                                                 LangOpts, Module->IsExternC))
-        return Err;
+      addHeaderInclude(H.NameAsWritten, Includes, LangOpts, Module->IsExternC);
     }
   }
   // Note that Module->PrivateHeaders will not be a TopHeader.
 
   if (Module::Header UmbrellaHeader = Module->getUmbrellaHeader()) {
     Module->addTopHeader(UmbrellaHeader.Entry);
-    if (Module->Parent) {
+    if (Module->Parent)
       // Include the umbrella header for submodules.
-      if (std::error_code Err = addHeaderInclude(UmbrellaHeader.NameAsWritten,
-                                                 Includes, LangOpts,
-                                                 Module->IsExternC))
-        return Err;
-    }
+      addHeaderInclude(UmbrellaHeader.NameAsWritten, Includes, LangOpts,
+                       Module->IsExternC);
   } else if (Module::DirectoryName UmbrellaDir = Module->getUmbrellaDir()) {
     // Add all of the headers we find in this subdirectory.
     std::error_code EC;
@@ -248,9 +242,7 @@
 
       // Include this header as part of the umbrella directory.
       Module->addTopHeader(Header);
-      if (std::error_code Err = addHeaderInclude(RelativeHeader, Includes,
-                                                 LangOpts, Module->IsExternC))
-        return Err;
+      addHeaderInclude(RelativeHeader, Includes, LangOpts, Module->IsExternC);
     }
 
     if (EC)
@@ -356,10 +348,9 @@
   SmallString<256> HeaderContents;
   std::error_code Err = std::error_code();
   if (Module::Header UmbrellaHeader = Module->getUmbrellaHeader())
-    Err = addHeaderInclude(UmbrellaHeader.NameAsWritten, HeaderContents,
-                           CI.getLangOpts(), Module->IsExternC);
-  if (!Err)
-    Err = collectModuleHeaderIncludes(
+    addHeaderInclude(UmbrellaHeader.NameAsWritten, HeaderContents,
+                     CI.getLangOpts(), Module->IsExternC);
+  Err = collectModuleHeaderIncludes(
         CI.getLangOpts(), FileMgr,
         CI.getPreprocessor().getHeaderSearchInfo().getModuleMap(), Module,
         HeaderContents);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D17794.49578.patch
Type: text/x-patch
Size: 3416 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160302/85d21503/attachment.bin>


More information about the cfe-commits mailing list