erichkeane added inline comments.
================ Comment at: include/clang/Basic/DiagnosticDriverKinds.td:118 "invalid deployment target for -stdlib=libc++ (requires %0 or later)">; +def err_drv_invalid_argument_to_fmacro_prefix_map : Error< + "invalid argument '%0' to -fmacro-prefix-map">; ---------------- Since these are otherwise identical, perhaps a %select{...|...} for the flag name? ================ Comment at: include/clang/Lex/PreprocessorOptions.h:171 + /// A prefix map for __FILE__ and __BASEFILE__ + std::map<std::string, std::string> MacroPrefixMap; + ---------------- erichkeane wrote: > It seems this can be StringRefs as well. Did you miss this one? Or is there a good reason these cannot be stringrefs? ================ Comment at: lib/Driver/ToolChains/Clang.cpp:616 + } else CmdArgs.push_back(Args.MakeArgString("-fdebug-prefix-map=" + Map)); ---------------- With the continue above, 'else' is unnecessary/against coding standard. ================ Comment at: lib/Driver/ToolChains/Clang.cpp:631 + } + else + CmdArgs.push_back(Args.MakeArgString("-fmacro-prefix-map=" + Map)); ---------------- See above. ================ Comment at: lib/Lex/PPMacroExpansion.cpp:1457 +static std::string remapMacroPath(StringRef Path, + const std::map<std::string, ---------------- Did clang-format do this formatting? It looks REALLY weird... ================ Comment at: lib/Lex/PPMacroExpansion.cpp:1532 FN += PLoc.getFilename(); + // TODO remap macro prefix here + FN = remapMacroPath(FN, PPOpts->MacroPrefixMap); ---------------- First, comments end in a period. Second, isn't that what the next line does? ================ Comment at: lib/Lex/PPMacroExpansion.cpp:1528 // Escape this filename. Turn '\' -> '\\' '"' -> '\"' - SmallString<128> FN; + std::string FN; if (PLoc.isValid()) { ---------------- dankm wrote: > erichkeane wrote: > > This change shouldn't be necessary, SmallString is still likely the right > > answer here. > I tried that long ago. It didn't work, I don't remember exactly why. But I > agree that SmallString should be enough. I'll dig more. Just noting to handle this before approval. Repository: rC Clang https://reviews.llvm.org/D49466 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits