From 7bcc21027d3b3697611de48261f0fa562bd2ed37 Mon Sep 17 00:00:00 2001 From: "Joel E. Denny" Date: Mon, 14 May 2018 18:41:44 +0000 Subject: [PATCH] [AST] Fix -ast-print for _Bool when have diagnostics For example, given: #define bool _Bool _Bool i; void fn() { 1; } -ast-print produced: tmp.c:3:13: warning: expression result unused void fn() { 1; } ^ bool i; void fn() { 1; } That fails to compile because bool is undefined. Details: Diagnostics print _Bool as bool when the latter is defined as the former. However, diagnostics were altering the printing policy for -ast-print as well. The printed source was then invalid because the preprocessor eats the bool definition. Problematic diagnostics included suppressed warnings (e.g., add -Wno-unused-value to the above example), including those that are suppressed by default. This patch fixes this bug and cleans up some related comments. Reviewed by: aaron.ballman, rsmith Differential Revision: https://reviews.llvm.org/D45093 llvm-svn: 332275 --- clang/include/clang/Sema/Sema.h | 4 +-- clang/lib/Frontend/ASTConsumers.cpp | 7 +++-- clang/lib/Sema/Sema.cpp | 7 +++-- clang/test/Misc/ast-print-bool.c | 44 +++++++++++++++++++++++++++++ 4 files changed, 54 insertions(+), 8 deletions(-) create mode 100644 clang/test/Misc/ast-print-bool.c diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 2ba3d41d6e4c..4d9871abad99 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -2113,12 +2113,12 @@ public: void checkPartialSpecializationVisibility(SourceLocation Loc, NamedDecl *Spec); - /// Retrieve a suitable printing policy. + /// Retrieve a suitable printing policy for diagnostics. PrintingPolicy getPrintingPolicy() const { return getPrintingPolicy(Context, PP); } - /// Retrieve a suitable printing policy. + /// Retrieve a suitable printing policy for diagnostics. static PrintingPolicy getPrintingPolicy(const ASTContext &Ctx, const Preprocessor &PP); diff --git a/clang/lib/Frontend/ASTConsumers.cpp b/clang/lib/Frontend/ASTConsumers.cpp index a51d5b4a151c..b67c019baed8 100644 --- a/clang/lib/Frontend/ASTConsumers.cpp +++ b/clang/lib/Frontend/ASTConsumers.cpp @@ -87,9 +87,10 @@ namespace { << DC->getPrimaryContext() << "\n"; } else Out << "Not a DeclContext\n"; - } else if (OutputKind == Print) - D->print(Out, /*Indentation=*/0, /*PrintInstantiation=*/true); - else if (OutputKind != None) + } else if (OutputKind == Print) { + PrintingPolicy Policy(D->getASTContext().getLangOpts()); + D->print(Out, Policy, /*Indentation=*/0, /*PrintInstantiation=*/true); + } else if (OutputKind != None) D->dump(Out, OutputKind == DumpFull); } diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp index eb336ee8f0f9..c0997688b2d0 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -52,8 +52,8 @@ ModuleLoader &Sema::getModuleLoader() const { return PP.getModuleLoader(); } PrintingPolicy Sema::getPrintingPolicy(const ASTContext &Context, const Preprocessor &PP) { PrintingPolicy Policy = Context.getPrintingPolicy(); - // Our printing policy is copied over the ASTContext printing policy whenever - // a diagnostic is emitted, so recompute it. + // In diagnostics, we print _Bool as bool if the latter is defined as the + // former. Policy.Bool = Context.getLangOpts().Bool; if (!Policy.Bool) { if (const MacroInfo *BoolMacro = PP.getMacroInfo(Context.getBoolName())) { @@ -1287,7 +1287,8 @@ void Sema::EmitCurrentDiagnostic(unsigned DiagID) { } } - // Set up the context's printing policy based on our current state. + // Copy the diagnostic printing policy over the ASTContext printing policy. + // TODO: Stop doing that. See: https://reviews.llvm.org/D45093#1090292 Context.setPrintingPolicy(getPrintingPolicy()); // Emit the diagnostic. diff --git a/clang/test/Misc/ast-print-bool.c b/clang/test/Misc/ast-print-bool.c new file mode 100644 index 000000000000..05519bcd4e54 --- /dev/null +++ b/clang/test/Misc/ast-print-bool.c @@ -0,0 +1,44 @@ +// RUN: %clang_cc1 -verify -ast-print %s -xc -DDEF_BOOL_CBOOL \ +// RUN: | FileCheck %s --check-prefixes=BOOL-AS-CBOOL,CBOOL +// +// RUN: %clang_cc1 -verify -ast-print %s -xc -DDEF_BOOL_CBOOL -DDIAG \ +// RUN: | FileCheck %s --check-prefixes=BOOL-AS-CBOOL,CBOOL +// +// RUN: %clang_cc1 -verify -ast-print %s -xc -DDEF_BOOL_INT \ +// RUN: | FileCheck %s --check-prefixes=BOOL-AS-INT,CBOOL +// +// RUN: %clang_cc1 -verify -ast-print %s -xc -DDEF_BOOL_INT -DDIAG \ +// RUN: | FileCheck %s --check-prefixes=BOOL-AS-INT,CBOOL +// +// RUN: %clang_cc1 -verify -ast-print %s -xc++ \ +// RUN: | FileCheck %s --check-prefixes=BOOL-AS-BOOL +// +// RUN: %clang_cc1 -verify -ast-print %s -xc++ -DDIAG \ +// RUN: | FileCheck %s --check-prefixes=BOOL-AS-BOOL + +#if DEF_BOOL_CBOOL +# define bool _Bool +#elif DEF_BOOL_INT +# define bool int +#endif + +// BOOL-AS-CBOOL: _Bool i; +// BOOL-AS-INT: int i; +// BOOL-AS-BOOL: bool i; +bool i; + +#ifndef __cplusplus +// CBOOL: _Bool j; +_Bool j; +#endif + +// Induce a diagnostic (and verify we actually managed to do so), which used to +// permanently alter the -ast-print printing policy for _Bool. How bool is +// defined by the preprocessor is examined only once per compilation, when the +// diagnostic is emitted, and it used to affect the entirety of -ast-print, so +// test only one definition of bool per compilation. +#if DIAG +void fn() { 1; } // expected-warning {{expression result unused}} +#else +// expected-no-diagnostics +#endif