[cfe-commits] r152708 - in /cfe/trunk: include/clang/Basic/Diagnostic.h include/clang/Sema/Sema.h lib/Sema/Sema.cpp
Daniel Dunbar
daniel at zuster.org
Wed Mar 14 02:49:33 PDT 2012
Author: ddunbar
Date: Wed Mar 14 04:49:32 2012
New Revision: 152708
URL: http://llvm.org/viewvc/llvm-project?rev=152708&view=rev
Log:
[Sema] Fix SemaDiagnosticBuilder to be inline.
- As with DiagnosticBuilder, it is very important that SemaDiagnosticBuilder be
completely inline to ensure that the compiler can rip it apart and sink it to
registers.
This is good for another 30k reduction in code size.
Modified:
cfe/trunk/include/clang/Basic/Diagnostic.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/Sema.cpp
Modified: cfe/trunk/include/clang/Basic/Diagnostic.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Diagnostic.h?rev=152708&r1=152707&r2=152708&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/Diagnostic.h (original)
+++ cfe/trunk/include/clang/Basic/Diagnostic.h Wed Mar 14 04:49:32 2012
@@ -700,9 +700,24 @@
return Diags->ProcessDiag(*this);
}
+ /// @name Diagnostic Emission
+ /// @{
+protected:
+ // Sema requires access to the following functions because the current design
+ // of SFINAE requires it to use its own SemaDiagnosticBuilder, which needs to
+ // access us directly to ensure we minimize the emitted code for the common
+ // Sema::Diag() patterns.
+ friend class Sema;
+
/// \brief Emit the current diagnostic and clear the diagnostic state.
bool EmitCurrentDiagnostic();
+ unsigned getCurrentDiagID() const { return CurDiagID; }
+
+ SourceLocation getCurrentDiagLoc() const { return CurDiagLoc; }
+
+ /// @}
+
friend class ASTReader;
friend class ASTWriter;
};
@@ -779,19 +794,6 @@
/// isActive - Determine whether this diagnostic is still active.
bool isActive() const { return DiagObj != 0; }
- /// \brief Retrieve the active diagnostic ID.
- ///
- /// \pre \c isActive()
- unsigned getDiagID() const {
- assert(isActive() && "DiagnosticsEngine is inactive");
- return DiagObj->CurDiagID;
- }
-
- /// \brief Retrieve the active diagnostic's location.
- ///
- /// \pre \c isActive()
- SourceLocation getLocation() const { return DiagObj->CurDiagLoc; }
-
/// \brief Force the diagnostic builder to emit the diagnostic now.
///
/// Once this function has been called, the DiagnosticBuilder object
@@ -816,7 +818,6 @@
return Result;
}
-
public:
/// Copy constructor. When copied, this "takes" the diagnostic info from the
@@ -829,8 +830,7 @@
NumFixits = D.NumFixits;
}
- /// Destructor - The dtor emits the diagnostic if it hasn't already
- /// been emitted.
+ /// Destructor - The dtor emits the diagnostic.
~DiagnosticBuilder() {
Emit();
}
Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=152708&r1=152707&r2=152708&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Wed Mar 14 04:49:32 2012
@@ -735,6 +735,12 @@
/// Private Helper predicate to check for 'self'.
bool isSelfExpr(Expr *RExpr);
+
+ /// \brief Cause the active diagnostic on the DiagosticsEngine to be
+ /// emitted. This is closely coupled to the SemaDiagnosticBuilder class and
+ /// should not be used elsewhere.
+ void EmitCurrentDiagnostic(unsigned DiagID);
+
public:
Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer,
TranslationUnitKind TUKind = TU_Complete,
@@ -776,11 +782,32 @@
SemaDiagnosticBuilder(DiagnosticBuilder &DB, Sema &SemaRef, unsigned DiagID)
: DiagnosticBuilder(DB), SemaRef(SemaRef), DiagID(DiagID) { }
- ~SemaDiagnosticBuilder();
+ ~SemaDiagnosticBuilder() {
+ // If we aren't active, there is nothing to do.
+ if (!isActive()) return;
+
+ // Otherwise, we need to emit the diagnostic. First flush the underlying
+ // DiagnosticBuilder data, and clear the diagnostic builder itself so it
+ // won't emit the diagnostic in its own destructor.
+ //
+ // This seems wasteful, in that as written the DiagnosticBuilder dtor will
+ // do its own needless checks to see if the diagnostic needs to be
+ // emitted. However, because we take care to ensure that the builder
+ // objects never escape, a sufficiently smart compiler will be able to
+ // eliminate that code.
+ FlushCounts();
+ Clear();
+
+ // Dispatch to Sema to emit the diagnostic.
+ SemaRef.EmitCurrentDiagnostic(DiagID);
+ }
};
/// \brief Emit a diagnostic.
- SemaDiagnosticBuilder Diag(SourceLocation Loc, unsigned DiagID);
+ SemaDiagnosticBuilder Diag(SourceLocation Loc, unsigned DiagID) {
+ DiagnosticBuilder DB = Diags.Report(Loc, DiagID);
+ return SemaDiagnosticBuilder(DB, *this, DiagID);
+ }
/// \brief Emit a partial diagnostic.
SemaDiagnosticBuilder Diag(SourceLocation Loc, const PartialDiagnostic& PD);
Modified: cfe/trunk/lib/Sema/Sema.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=152708&r1=152707&r2=152708&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/Sema.cpp (original)
+++ cfe/trunk/lib/Sema/Sema.cpp Wed Mar 14 04:49:32 2012
@@ -673,12 +673,17 @@
return 0;
}
-Sema::SemaDiagnosticBuilder::~SemaDiagnosticBuilder() {
- if (!isActive())
- return;
-
- if (llvm::Optional<TemplateDeductionInfo*> Info = SemaRef.isSFINAEContext()) {
- switch (DiagnosticIDs::getDiagnosticSFINAEResponse(getDiagID())) {
+void Sema::EmitCurrentDiagnostic(unsigned DiagID) {
+ // FIXME: It doesn't make sense to me that DiagID is an incoming argument here
+ // and yet we also use the current diag ID on the DiagnosticsEngine. This has
+ // been made more painfully obvious by the refactor that introduced this
+ // function, but it is possible that the incoming argument can be
+ // eliminnated. If it truly cannot be (for example, there is some reentrancy
+ // issue I am not seeing yet), then there should at least be a clarifying
+ // comment somewhere.
+ if (llvm::Optional<TemplateDeductionInfo*> Info = isSFINAEContext()) {
+ switch (DiagnosticIDs::getDiagnosticSFINAEResponse(
+ Diags.getCurrentDiagID())) {
case DiagnosticIDs::SFINAE_Report:
// We'll report the diagnostic below.
break;
@@ -686,10 +691,9 @@
case DiagnosticIDs::SFINAE_SubstitutionFailure:
// Count this failure so that we know that template argument deduction
// has failed.
- ++SemaRef.NumSFINAEErrors;
- SemaRef.Diags.setLastDiagnosticIgnored();
- SemaRef.Diags.Clear();
- Clear();
+ ++NumSFINAEErrors;
+ Diags.setLastDiagnosticIgnored();
+ Diags.Clear();
return;
case DiagnosticIDs::SFINAE_AccessControl: {
@@ -697,52 +701,47 @@
// Additionally, the AccessCheckingSFINAE flag can be used to temporarily
// make access control a part of SFINAE for the purposes of checking
// type traits.
- if (!SemaRef.AccessCheckingSFINAE &&
- !SemaRef.getLangOpts().CPlusPlus0x)
+ if (!AccessCheckingSFINAE && !getLangOpts().CPlusPlus0x)
break;
- SourceLocation Loc = getLocation();
+ SourceLocation Loc = Diags.getCurrentDiagLoc();
// Suppress this diagnostic.
- ++SemaRef.NumSFINAEErrors;
- SemaRef.Diags.setLastDiagnosticIgnored();
- SemaRef.Diags.Clear();
- Clear();
+ ++NumSFINAEErrors;
+ Diags.setLastDiagnosticIgnored();
+ Diags.Clear();
// Now the diagnostic state is clear, produce a C++98 compatibility
// warning.
- SemaRef.Diag(Loc, diag::warn_cxx98_compat_sfinae_access_control);
+ Diag(Loc, diag::warn_cxx98_compat_sfinae_access_control);
// The last diagnostic which Sema produced was ignored. Suppress any
// notes attached to it.
- SemaRef.Diags.setLastDiagnosticIgnored();
+ Diags.setLastDiagnosticIgnored();
return;
}
case DiagnosticIDs::SFINAE_Suppress:
// Make a copy of this suppressed diagnostic and store it with the
// template-deduction information;
- FlushCounts();
- Diagnostic DiagInfo(&SemaRef.Diags);
+ Diagnostic DiagInfo(&Diags);
if (*Info)
(*Info)->addSuppressedDiagnostic(DiagInfo.getLocation(),
- PartialDiagnostic(DiagInfo,
- SemaRef.Context.getDiagAllocator()));
+ PartialDiagnostic(DiagInfo,Context.getDiagAllocator()));
// Suppress this diagnostic.
- SemaRef.Diags.setLastDiagnosticIgnored();
- SemaRef.Diags.Clear();
- Clear();
+ Diags.setLastDiagnosticIgnored();
+ Diags.Clear();
return;
}
}
// Set up the context's printing policy based on our current state.
- SemaRef.Context.setPrintingPolicy(SemaRef.getPrintingPolicy());
+ Context.setPrintingPolicy(getPrintingPolicy());
// Emit the diagnostic.
- if (!this->Emit())
+ if (!Diags.EmitCurrentDiagnostic())
return;
// If this is not a note, and we're in a template instantiation
@@ -750,20 +749,14 @@
// we emitted an error, print a template instantiation
// backtrace.
if (!DiagnosticIDs::isBuiltinNote(DiagID) &&
- !SemaRef.ActiveTemplateInstantiations.empty() &&
- SemaRef.ActiveTemplateInstantiations.back()
- != SemaRef.LastTemplateInstantiationErrorContext) {
- SemaRef.PrintInstantiationStack();
- SemaRef.LastTemplateInstantiationErrorContext
- = SemaRef.ActiveTemplateInstantiations.back();
+ !ActiveTemplateInstantiations.empty() &&
+ ActiveTemplateInstantiations.back()
+ != LastTemplateInstantiationErrorContext) {
+ PrintInstantiationStack();
+ LastTemplateInstantiationErrorContext = ActiveTemplateInstantiations.back();
}
}
-Sema::SemaDiagnosticBuilder Sema::Diag(SourceLocation Loc, unsigned DiagID) {
- DiagnosticBuilder DB = Diags.Report(Loc, DiagID);
- return SemaDiagnosticBuilder(DB, *this, DiagID);
-}
-
Sema::SemaDiagnosticBuilder
Sema::Diag(SourceLocation Loc, const PartialDiagnostic& PD) {
SemaDiagnosticBuilder Builder(Diag(Loc, PD.getDiagID()));
More information about the cfe-commits
mailing list