r211411 - Driver: Record that we're in crashdump and push flags to ConstructJob
Justin Bogner
mail at justinbogner.com
Mon Jun 23 15:18:50 PDT 2014
Jordan Rose <jordan_rose at apple.com> 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 <mail at justinbogner.com> 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
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list