Skip to content

Add -allow-rewrite-failures option to downgrade rewrite errors to warnings. #461

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions clang/include/clang/3C/3C.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ struct _3COptions {

bool DumpUnwritableChanges;
bool AllowUnwritableChanges;

bool AllowRewriteFailures;
};

// The main interface exposed by the 3C to interact with the tool.
Expand Down
1 change: 1 addition & 0 deletions clang/include/clang/3C/3CGlobalOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ extern bool WarnRootCause;
extern bool WarnAllRootCause;
extern bool DumpUnwritableChanges;
extern bool AllowUnwritableChanges;
extern bool AllowRewriteFailures;

#ifdef FIVE_C
extern bool RemoveItypes;
Expand Down
2 changes: 2 additions & 0 deletions clang/lib/3C/3C.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ std::set<std::string> FilePaths;
bool VerifyDiagnosticOutput;
bool DumpUnwritableChanges;
bool AllowUnwritableChanges;
bool AllowRewriteFailures;

#ifdef FIVE_C
bool RemoveItypes;
Expand Down Expand Up @@ -235,6 +236,7 @@ _3CInterface::_3CInterface(const struct _3COptions &CCopt,
VerifyDiagnosticOutput = CCopt.VerifyDiagnosticOutput;
DumpUnwritableChanges = CCopt.DumpUnwritableChanges;
AllowUnwritableChanges = CCopt.AllowUnwritableChanges;
AllowRewriteFailures = CCopt.AllowRewriteFailures;

#ifdef FIVE_C
RemoveItypes = CCopt.RemoveItypes;
Expand Down
57 changes: 40 additions & 17 deletions clang/lib/3C/RewriteUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -162,13 +162,26 @@ void rewriteSourceRange(Rewriter &R, const CharSourceRange &Range,
// crashing with an assert fail.
if (!RewriteSuccess) {
clang::DiagnosticsEngine &DE = R.getSourceMgr().getDiagnostics();
unsigned ErrorId =
DE.getCustomDiagID(
ErrFail ? DiagnosticsEngine::Error : DiagnosticsEngine::Warning,
"Unable to rewrite converted source range. Intended rewriting: \"%0\"");
auto ErrorBuilder = DE.Report(Range.getBegin(), ErrorId);
ErrorBuilder.AddSourceRange(R.getSourceMgr().getExpansionRange(Range));
ErrorBuilder.AddString(NewText);
bool ReportError = ErrFail && !AllowRewriteFailures;
{
// Put this in a block because Clang only allows one DiagnosticBuilder to
// exist at a time.
unsigned ErrorId = DE.getCustomDiagID(
ReportError ? DiagnosticsEngine::Error : DiagnosticsEngine::Warning,
"Unable to rewrite converted source range. Intended rewriting: \"%0\"");
auto ErrorBuilder = DE.Report(Range.getBegin(), ErrorId);
ErrorBuilder.AddSourceRange(R.getSourceMgr().getExpansionRange(Range));
ErrorBuilder.AddString(NewText);
}
if (ReportError) {
unsigned NoteId = DE.getCustomDiagID(
DiagnosticsEngine::Note,
"you can use the -allow-rewrite-failures option to temporarily "
"downgrade this error to a warning");
// If we pass the location here, the macro call stack gets dumped again,
// which looks silly.
DE.Report(NoteId);
}
}
}

Expand All @@ -187,18 +200,28 @@ static void emit(Rewriter &R, ASTContext &C) {
DiagnosticsEngine::Level UnwritableChangeDiagnosticLevel =
AllowUnwritableChanges ? DiagnosticsEngine::Warning
: DiagnosticsEngine::Error;
auto MaybeDumpUnwritableChange = [&]() {
auto PrintExtraUnwritableChangeInfo = [&]() {
DiagnosticsEngine &DE = C.getDiagnostics();
// With -dump-unwritable-changes and not -allow-unwritable-changes, we
// want the -allow-unwritable-changes note before the dump.
if (!DumpUnwritableChanges) {
unsigned DumpNoteId = DE.getCustomDiagID(
DiagnosticsEngine::Note,
"use the -dump-unwritable-changes option to see the new version "
"of the file");
DE.Report(DumpNoteId);
}
if (!AllowUnwritableChanges) {
unsigned AllowNoteId = DE.getCustomDiagID(
DiagnosticsEngine::Note,
"you can use the -allow-unwritable-changes option to temporarily "
"downgrade this error to a warning");
DE.Report(AllowNoteId);
}
if (DumpUnwritableChanges) {
errs() << "=== Beginning of new version of " << FE->getName() << " ===\n";
Buffer->second.write(errs());
errs() << "=== End of new version of " << FE->getName() << " ===\n";
} else {
DiagnosticsEngine &DE = C.getDiagnostics();
unsigned ID = DE.getCustomDiagID(
DiagnosticsEngine::Note,
"use the -dump-unwritable-changes option to see the new version "
"of the file");
DE.Report(SM.translateFileLineCol(FE, 1, 1), ID);
}
};

Expand All @@ -213,7 +236,7 @@ static void emit(Rewriter &R, ASTContext &C) {
"is not allowed to write to the file "
"(https://github.com/correctcomputation/checkedc-clang/issues/387)");
DE.Report(SM.translateFileLineCol(FE, 1, 1), ID);
MaybeDumpUnwritableChange();
PrintExtraUnwritableChangeInfo();
continue;
}

Expand All @@ -230,7 +253,7 @@ static void emit(Rewriter &R, ASTContext &C) {
"but is not the main file and thus cannot be written in stdout "
"mode");
DE.Report(SM.translateFileLineCol(FE, 1, 1), ID);
MaybeDumpUnwritableChange();
PrintExtraUnwritableChangeInfo();
}
continue;
}
Expand Down
3 changes: 2 additions & 1 deletion clang/test/3C/base_subdir/canwrite_constraints_symlink.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
// RUN: cd %t.base && 3c -addcr -verify canwrite_constraints_symlink.c --

// expected-error@base_subdir_partial_defn.h:1 {{3C internal error: 3C generated changes to this file even though it is not allowed to write to the file}}
// expected-note@base_subdir_partial_defn.h:1 {{-dump-unwritable-changes}}
// expected-note@*:* {{-dump-unwritable-changes}}
// expected-note@*:* {{-allow-unwritable-changes}}

void
#include "base_subdir_partial_defn.h"
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
// RUN: cd %S && 3c -addcr -verify %s --

// expected-error@../base_subdir_partial_defn.h:1 {{3C internal error: 3C generated changes to this file even though it is not allowed to write to the file}}
// expected-note@../base_subdir_partial_defn.h:1 {{-dump-unwritable-changes}}
// expected-note@*:* {{-dump-unwritable-changes}}
// expected-note@*:* {{-allow-unwritable-changes}}

// The "../base_subdir_partial_defn.h" path is testing two former base dir
// matching bugs from
Expand Down
2 changes: 2 additions & 0 deletions clang/test/3C/macro_rewrite_error.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
#define args ();
typedef int (*a) args // expected-error {{Unable to rewrite converted source range. Intended rewriting: "typedef _Ptr<int (void)> a"}}
a b;
// expected-note@*:* {{-allow-rewrite-failures}}

#define MIDDLE x; int *
int MIDDLE y; // expected-error {{Unable to rewrite converted source range. Intended rewriting: "_Ptr<int> y = ((void *)0)"}}
// expected-note@*:* {{-allow-rewrite-failures}}
3 changes: 2 additions & 1 deletion clang/test/3C/stdout_mode_write_other.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
// RUN: 3c -base-dir=%S -addcr -verify %s --

// expected-error@base_subdir_partial_defn.h:1 {{3C generated changes to this file, which is under the base dir but is not the main file and thus cannot be written in stdout mode}}
// expected-note@base_subdir_partial_defn.h:1 {{-dump-unwritable-changes}}
// expected-note@*:* {{-dump-unwritable-changes}}
// expected-note@*:* {{-allow-unwritable-changes}}

void
#include "base_subdir_partial_defn.h"
15 changes: 14 additions & 1 deletion clang/tools/3c/3CStandalone.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,18 @@ static cl::opt<bool> OptAllowUnwritableChanges(
"may be removed in the future."),
cl::init(false), cl::cat(_3CCategory));

static cl::opt<bool> OptAllowRewriteFailures(
"allow-rewrite-failures",
cl::desc("When 3c fails to make a rewrite to a source file (typically "
"because of macros), issue a warning instead of an error. This "
"option is intended to be used temporarily until you change your "
"code to allow 3c to work or you report the problem to the 3C "
"team to get it fixed; the option may be removed in the future. "
"Note that some kinds of rewrite failures currently generate "
"warnings regardless of this option, due to known bugs that "
"affect common use cases."),
cl::init(false), cl::cat(_3CCategory));

#ifdef FIVE_C
static cl::opt<bool> OptRemoveItypes(
"remove-itypes",
Expand Down Expand Up @@ -272,6 +284,7 @@ int main(int argc, const char **argv) {
CcOptions.VerifyDiagnosticOutput = OptVerifyDiagnosticOutput;
CcOptions.DumpUnwritableChanges = OptDumpUnwritableChanges;
CcOptions.AllowUnwritableChanges = OptAllowUnwritableChanges;
CcOptions.AllowRewriteFailures = OptAllowRewriteFailures;

#ifdef FIVE_C
CcOptions.RemoveItypes = OptRemoveItypes;
Expand Down Expand Up @@ -335,7 +348,7 @@ int main(int argc, const char **argv) {

// Write all the converted files back.
if (!_3CInterface.writeAllConvertedFilesToDisk()) {
errs() << "Failure occurred while trying to rewrite converted files back."
errs() << "Failure occurred while trying to rewrite converted files back. "
"Exiting.\n";
return 1;
}
Expand Down