Jordan Rose <[email protected]> writes: > Hm. .i isn't really correct for something that still has macros in > it. IIRC Clang happens to expand them again, but if we've merely > rewritten includes I would expect the resulting file to have a .c > extension.
It's a bit tricky to get this right consistently in a crashdump context, due to the way the logic is split between the driver and cc1. Notably, I can fairly easily special case the driver here to use a .c extension for crashdumps, but as of r211421 that isn't always right. We don't really know if we're going to specify rewrite-includes or not at the time that we decide the filename. That said, the trade off is a matter of which is worse: 1. Crashdump files named .i/.mi even though they still have macros when -fmodules isn't present, or 2. Crashdump files named .c/.m even though they're fully preprocessed when -fmodules is present. I suppose that, given we'd like to make rewrite-includes and module crashdumps work together in the long term, (2) is a better trade off. It also avoids changing the status quo. Agree? > On Jun 20, 2014, at 15:16 , Justin Bogner <[email protected]> wrote: > >> Author: bogner >> Date: Fri Jun 20 17:16:00 2014 >> New Revision: 211411 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=211411&view=rev> Log: >> Driver: Record that we're in crashdump and push flags to ConstructJob >> >> It's more flexible and arguably better layering to set flags to modify >> compiling for diagnostics in the CC1 job themselves, rather than >> tweaking the driver flags and letting them propagate. >> >> There is one visible change this causes: crash report files will now >> get preprocessed names (.i and friends). >> >> Modified: >> cfe/trunk/include/clang/Driver/Compilation.h >> cfe/trunk/lib/Driver/Compilation.cpp >> cfe/trunk/lib/Driver/Driver.cpp >> cfe/trunk/lib/Driver/Tools.cpp >> cfe/trunk/test/Driver/crash-report.c >> >> Modified: cfe/trunk/include/clang/Driver/Compilation.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Compilation.h?rev=211411&r1=211410&r2=211411&view=diff> >> >> ============================================================================== >> --- cfe/trunk/include/clang/Driver/Compilation.h (original) >> +++ cfe/trunk/include/clang/Driver/Compilation.h Fri Jun 20 17:16:00 2014 >> @@ -69,6 +69,9 @@ class Compilation { >> /// Redirection for stdout, stderr, etc. >> const StringRef **Redirects; >> >> + /// Whether we're compiling for diagnostic purposes. >> + bool ForDiagnostics; >> + >> public: >> Compilation(const Driver &D, const ToolChain &DefaultToolChain, >> llvm::opt::InputArgList *Args, >> @@ -173,6 +176,9 @@ public: >> /// so compilation can be reexecuted to generate additional diagnostic >> /// information (e.g., preprocessed source(s)). >> void initCompilationForDiagnostics(); >> + >> + /// Return true if we're compiling for diagnostics. >> + bool isForDiagnostics() { return ForDiagnostics; } >> }; >> >> } // end namespace driver >> >> Modified: cfe/trunk/lib/Driver/Compilation.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Compilation.cpp?rev=211411&r1=211410&r2=211411&view=diff> >> >> ============================================================================== >> --- cfe/trunk/lib/Driver/Compilation.cpp (original) >> +++ cfe/trunk/lib/Driver/Compilation.cpp Fri Jun 20 17:16:00 2014 >> @@ -26,9 +26,9 @@ using namespace llvm::opt; >> >> Compilation::Compilation(const Driver &D, const ToolChain &_DefaultToolChain, >> InputArgList *_Args, DerivedArgList >> *_TranslatedArgs) >> - : TheDriver(D), DefaultToolChain(_DefaultToolChain), Args(_Args), >> - TranslatedArgs(_TranslatedArgs), Redirects(nullptr) { >> -} >> + : TheDriver(D), DefaultToolChain(_DefaultToolChain), Args(_Args), >> + TranslatedArgs(_TranslatedArgs), Redirects(nullptr), >> + ForDiagnostics(false) {} >> >> Compilation::~Compilation() { >> delete TranslatedArgs; >> @@ -211,6 +211,8 @@ void Compilation::ExecuteJob(const Job & >> } >> >> void Compilation::initCompilationForDiagnostics() { >> + ForDiagnostics = true; >> + >> // Free actions and jobs. >> DeleteContainerPointers(Actions); >> Jobs.clear(); >> >> Modified: cfe/trunk/lib/Driver/Driver.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=211411&r1=211410&r2=211411&view=diff> >> >> ============================================================================== >> --- cfe/trunk/lib/Driver/Driver.cpp (original) >> +++ cfe/trunk/lib/Driver/Driver.cpp Fri Jun 20 17:16:00 2014 >> @@ -419,8 +419,6 @@ void Driver::generateCompilationDiagnost >> // Suppress driver output and emit preprocessor output to temp file. >> Mode = CPPMode; >> CCGenDiagnostics = true; >> - C.getArgs().AddFlagArg(nullptr, >> - Opts->getOption(options::OPT_frewrite_includes)); >> >> // Save the original job command(s). >> std::string Cmd; >> >> Modified: cfe/trunk/lib/Driver/Tools.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=211411&r1=211410&r2=211411&view=diff> >> >> ============================================================================== >> --- cfe/trunk/lib/Driver/Tools.cpp (original) >> +++ cfe/trunk/lib/Driver/Tools.cpp Fri Jun 20 17:16:00 2014 >> @@ -3339,10 +3339,6 @@ void Clang::ConstructJob(Compilation &C, >> if (Args.getLastArg(options::OPT_fapple_kext)) >> CmdArgs.push_back("-fapple-kext"); >> >> - if (Args.hasFlag(options::OPT_frewrite_includes, >> - options::OPT_fno_rewrite_includes, false)) >> - CmdArgs.push_back("-frewrite-includes"); >> - >> Args.AddLastArg(CmdArgs, options::OPT_fobjc_sender_dependent_dispatch); >> Args.AddLastArg(CmdArgs, >> options::OPT_fdiagnostics_print_source_range_info); >> Args.AddLastArg(CmdArgs, options::OPT_fdiagnostics_parseable_fixits); >> @@ -4037,6 +4033,11 @@ void Clang::ConstructJob(Compilation &C, >> } >> #endif >> >> + if (Args.hasFlag(options::OPT_frewrite_includes, >> + options::OPT_fno_rewrite_includes, false) || >> + C.isForDiagnostics()) >> + CmdArgs.push_back("-frewrite-includes"); >> + >> // Only allow -traditional or -traditional-cpp outside in preprocessing >> modes. >> if (Arg *A = Args.getLastArg(options::OPT_traditional, >> options::OPT_traditional_cpp)) { >> >> Modified: cfe/trunk/test/Driver/crash-report.c >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/crash-report.c?rev=211411&r1=211410&r2=211411&view=diff> >> >> ============================================================================== >> --- cfe/trunk/test/Driver/crash-report.c (original) >> +++ cfe/trunk/test/Driver/crash-report.c Fri Jun 20 17:16:00 2014 >> @@ -7,11 +7,11 @@ >> // RUN: -Xclang -internal-isystem -Xclang /tmp/ \ >> // RUN: -Xclang -internal-externc-isystem -Xclang /tmp/ \ >> // RUN: -DFOO=BAR 2>&1 | FileCheck %s >> -// RUN: cat %t/crash-report-*.c | FileCheck --check-prefix=CHECKSRC %s >> +// RUN: cat %t/crash-report-*.i | FileCheck --check-prefix=CHECKSRC %s >> // RUN: cat %t/crash-report-*.sh | FileCheck --check-prefix=CHECKSH %s >> // REQUIRES: crash-recovery >> >> -// because of the glob (*.c, *.sh) >> +// because of the glob (*.i, *.sh) >> // REQUIRES: shell >> >> // RUN: not env FORCE_CLANG_DIAGNOSTICS_CRASH=1 %clang -fsyntax-only -x c >> /dev/null -lstdc++ 2>&1 | FileCheck %s >> @@ -21,7 +21,7 @@ >> >> #pragma clang __debug parser_crash >> // CHECK: Preprocessed source(s) and associated run script(s) are located at: >> -// CHECK-NEXT: note: diagnostic msg: {{.*}}.c >> +// CHECK-NEXT: note: diagnostic msg: {{.*}}.i >> FOO >> // CHECKSRC: FOO >> // CHECKSH: -cc1 >> >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
