r212191 - ARCMigrate: simplify diagnostic handling

[David Blaikie]

On Wed, Jul 2, 2014 at 10:08 AM, Alp Toker <alp at nuanti.com> wrote:
> Author: alp
> Date: Wed Jul  2 12:08:00 2014
> New Revision: 212191
>
> URL: http://llvm.org/viewvc/llvm-project?rev=212191&view=rev
> Log:
> ARCMigrate: simplify diagnostic handling
>
> Recent enhancements in the diagnostics engine mean that
> TransformActions::report() no longer needs to duplicate this suppression logic.
>
> That's great because the old code was flawed and would have attached notes to
> the wrong primary diagnostic in non-trivial use.

Any value in adding a test for this? (probably not, now that the
codepaths are shared and presumably the surviving/existing codepath
was already well/better tested - but just checking)

>
> With these changes it becomes safe to use reportNote() freely in the migration
> tool.
>
> Modified:
>     cfe/trunk/include/clang/Basic/Diagnostic.td
>     cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td
>     cfe/trunk/lib/ARCMigrate/Internals.h
>     cfe/trunk/lib/ARCMigrate/TransformActions.cpp
>
> Modified: cfe/trunk/include/clang/Basic/Diagnostic.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Diagnostic.td?rev=212191&r1=212190&r2=212191&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/Diagnostic.td (original)
> +++ cfe/trunk/include/clang/Basic/Diagnostic.td Wed Jul  2 12:08:00 2014
> @@ -89,6 +89,10 @@ class ShowInSystemHeader {
>    bit ShowInSystemHeader = 1;
>  }
>
> +class SuppressInSystemHeader {
> +  bit ShowInSystemHeader = 0;
> +}
> +
>  // FIXME: ExtWarn and Extension should also be SFINAEFailure by default.
>  class Error<string str>     : Diagnostic<str, CLASS_ERROR, SEV_Error>, SFINAEFailure {
>    bit ShowInSystemHeader = 1;
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td?rev=212191&r1=212190&r2=212191&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td Wed Jul  2 12:08:00 2014
> @@ -138,7 +138,7 @@ def err_module_file_conflict : Error<"mo
>
>  // TransformActions
>  // TODO: Use a custom category name to distinguish rewriter errors.
> -def err_mt_message : Error<"[rewriter] %0">;
> +def err_mt_message : Error<"[rewriter] %0">, SuppressInSystemHeader;
>  def warn_mt_message : Warning<"[rewriter] %0">;
>  def note_mt_message : Note<"[rewriter] %0">;
>
>
> Modified: cfe/trunk/lib/ARCMigrate/Internals.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ARCMigrate/Internals.h?rev=212191&r1=212190&r2=212191&view=diff
> ==============================================================================
> --- cfe/trunk/lib/ARCMigrate/Internals.h (original)
> +++ cfe/trunk/lib/ARCMigrate/Internals.h Wed Jul  2 12:08:00 2014
> @@ -48,7 +48,6 @@ void writeARCDiagsToPlist(const std::str
>  class TransformActions {
>    DiagnosticsEngine &Diags;
>    CapturedDiagList &CapturedDiags;
> -  bool ReportedErrors;
>    void *Impl; // TransformActionsImpl.
>
>  public:
> @@ -104,7 +103,9 @@ public:
>    void reportNote(StringRef note, SourceLocation loc,
>                    SourceRange range = SourceRange());
>
> -  bool hasReportedErrors() const { return ReportedErrors; }
> +  bool hasReportedErrors() const {
> +    return Diags.hasUnrecoverableErrorOccurred();
> +  }
>
>    class RewriteReceiver {
>    public:
>
> Modified: cfe/trunk/lib/ARCMigrate/TransformActions.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ARCMigrate/TransformActions.cpp?rev=212191&r1=212190&r2=212191&view=diff
> ==============================================================================
> --- cfe/trunk/lib/ARCMigrate/TransformActions.cpp (original)
> +++ cfe/trunk/lib/ARCMigrate/TransformActions.cpp Wed Jul  2 12:08:00 2014
> @@ -601,7 +601,7 @@ TransformActions::RewriteReceiver::~Rewr
>  TransformActions::TransformActions(DiagnosticsEngine &diag,
>                                     CapturedDiagList &capturedDiags,
>                                     ASTContext &ctx, Preprocessor &PP)
> -  : Diags(diag), CapturedDiags(capturedDiags), ReportedErrors(false) {
> +    : Diags(diag), CapturedDiags(capturedDiags) {
>    Impl = new TransformActionsImpl(capturedDiags, ctx, PP);
>  }
>
> @@ -677,17 +677,6 @@ DiagnosticBuilder TransformActions::repo
>                                             SourceRange range) {
>    assert(!static_cast<TransformActionsImpl *>(Impl)->isInTransaction() &&
>           "Errors should be emitted out of a transaction");
> -
> -  SourceManager &SM = static_cast<TransformActionsImpl *>(Impl)
> -                          ->getASTContext()
> -                          .getSourceManager();
> -  DiagnosticsEngine::Level L = Diags.getDiagnosticLevel(diagId, loc);
> -  // TODO: Move this check to the caller to ensure consistent note attachments.
> -  if (L == DiagnosticsEngine::Ignored ||
> -      SM.isInSystemHeader(SM.getExpansionLoc(loc)))
> -    return DiagnosticBuilder::getEmpty();
> -  if (L >= DiagnosticsEngine::Error)
> -    ReportedErrors = true;
>    return Diags.Report(loc, diagId) << range;
>  }
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits