[MSVC] Automatically add atlmfc include and lib directories as system paths.

[zturner]

Summary

This matches the behavior of cl.

[amccarth]

This matches the behavior of cl.

Are you sure? In a bare environment, cl.exe doesn't include any system paths, not even to the standard library. It actually uses the INCLUDE environment variable for those paths. Granted, VCVARSALL sets that (and other environment variables), but it's not innate behavior of cl.exe.

[zturner]

In D68736#1702482, @amccarth wrote:

This matches the behavior of cl.

Are you sure? In a bare environment, cl.exe doesn't include any system paths, not even to the standard library. It actually uses the INCLUDE environment variable for those paths. Granted, VCVARSALL sets that (and other environment variables), but it's not innate behavior of cl.exe.

Right, sorry. I guess what I mean is that this matches the behavior of running from a cl command prompt, which is what we do elsewhere. This is already in the codepath that is trying to build up a consistent WindowsSdk + CRT environment, we were just missing the atlmfc part.

[rnk]

lgtm

I think the old VS Express editions used to exclude atlmfc, so there's the possibility that we'll be searching non-existent directories, but I think that's OK. Whoever wrote this code was probably basing it on what VS express did. That edition is long gone, and we're better off with this new logic.

clang/lib/Driver/ToolChains/MSVC.h

public:
   }
 
   enum class SubDirectoryType {
     Bin,
     Include,
     Lib,
   };
   std::string getSubDirectoryPath(SubDirectoryType Type,
+                                  llvm::StringRef SubdirParent,
                                   llvm::Triple::ArchType TargetArch) const;
 
   // Convenience overload.
   // Uses the current target arch.
-  std::string getSubDirectoryPath(SubDirectoryType Type) const {
-    return getSubDirectoryPath(Type, getArch());
+  std::string getSubDirectoryPath(SubDirectoryType Type,
+                                  llvm::StringRef SubdirParent = "") const {
+    return getSubDirectoryPath(Type, SubdirParent, getArch());
   }
 
   enum class ToolsetLayout {
     OlderVS,
     VS2017OrNewer,
     DevDivInternal,
   };
   bool getIsVS2017OrNewer() const { return VSLayout == ToolsetLayout::VS2017OrNewer; }

clang/lib/Driver/ToolChains/MSVC.cpp

if (!llvm::sys::Process::GetEnv("LIB")) {
     // did not run vcvarsall), try to build a consistent link environment.  If
     // the environment variable is set however, assume the user knows what
     // they're doing.
     CmdArgs.push_back(Args.MakeArgString(
         Twine("-libpath:") +
         TC.getSubDirectoryPath(
             toolchains::MSVCToolChain::SubDirectoryType::Lib)));
 
+    CmdArgs.push_back(Args.MakeArgString(
+        Twine("-libpath:") +
+        TC.getSubDirectoryPath(toolchains::MSVCToolChain::SubDirectoryType::Lib,
+                               "atlmfc")));
+
     if (TC.useUniversalCRT()) {
       std::string UniversalCRTLibPath;
       if (TC.getUniversalCRTLibraryPath(UniversalCRTLibPath))
         CmdArgs.push_back(
             Args.MakeArgString(Twine("-libpath:") + UniversalCRTLibPath));
     }
 
     std::string WindowsSdkLibPath;
if (TC.getIsVS2017OrNewer() &&
         llvm::StringRef EnvVar(Cursor);
         if (EnvVar.startswith_lower("path=")) {
           using SubDirectoryType = toolchains::MSVCToolChain::SubDirectoryType;
           constexpr size_t PrefixLen = 5; // strlen("path=")
           Environment.push_back(Args.MakeArgString(
               EnvVar.substr(0, PrefixLen) +
               TC.getSubDirectoryPath(SubDirectoryType::Bin) +
               llvm::Twine(llvm::sys::EnvPathSeparator) +
-              TC.getSubDirectoryPath(SubDirectoryType::Bin, HostArch) +
+              TC.getSubDirectoryPath(SubDirectoryType::Bin, "", HostArch) +
               (EnvVar.size() > PrefixLen
                    ? llvm::Twine(llvm::sys::EnvPathSeparator) +
                          EnvVar.substr(PrefixLen)
                    : "")));
         } else {
           Environment.push_back(Args.MakeArgString(EnvVar));
         }
         Cursor += EnvVar.size() + 1 /*null-terminator*/;

 }
 
 // Get the path to a specific subdirectory in the current toolchain for
 // a given target architecture.
 // VS2017 changed the VC toolchain layout, so this should be used instead
 // of hardcoding paths.
 std::string
 MSVCToolChain::getSubDirectoryPath(SubDirectoryType Type,
+                                   llvm::StringRef SubdirParent,
                                    llvm::Triple::ArchType TargetArch) const {
   const char *SubdirName;
   const char *IncludeName;
   switch (VSLayout) {
   case ToolsetLayout::OlderVS:
     SubdirName = llvmArchToLegacyVCArch(TargetArch);
     IncludeName = "include";
     break;
   case ToolsetLayout::VS2017OrNewer:
     SubdirName = llvmArchToWindowsSDKArch(TargetArch);
     IncludeName = "include";
     break;
   case ToolsetLayout::DevDivInternal:
     SubdirName = llvmArchToDevDivInternalArch(TargetArch);
     IncludeName = "inc";
     break;
   }
 
   llvm::SmallString<256> Path(VCToolChainPath);
+  if (!SubdirParent.empty())
+    llvm::sys::path::append(Path, SubdirParent);
+
   switch (Type) {
   case SubDirectoryType::Bin:
     if (VSLayout == ToolsetLayout::VS2017OrNewer) {
       const bool HostIsX64 =
           llvm::Triple(llvm::sys::getProcessTriple()).isArch64Bit();
       const char *const HostName = HostIsX64 ? "Hostx64" : "Hostx86";
       llvm::sys::path::append(Path, "bin", HostName, SubdirName);
     } else { // OlderVS or DevDivInternal
if (!Dirs.empty())
       return;
   }
 
   // When built with access to the proper Windows APIs, try to actually find
   // the correct include paths first.
   if (!VCToolChainPath.empty()) {
     addSystemInclude(DriverArgs, CC1Args,
                      getSubDirectoryPath(SubDirectoryType::Include));
+    addSystemInclude(DriverArgs, CC1Args,
+                     getSubDirectoryPath(SubDirectoryType::Include, "atlmfc"));
 
     if (useUniversalCRT()) {
       std::string UniversalCRTSdkPath;
       std::string UCRTVersion;
       if (getUniversalCRTSdkDir(UniversalCRTSdkPath, UCRTVersion)) {
         AddSystemIncludeWithSubfolder(DriverArgs, CC1Args, UniversalCRTSdkPath,
                                       "Include", UCRTVersion, "ucrt");
       }