r209468 - Remove limits on the number of fix-it hints and ranges in the DiagnosticsEngine.
Alexander Kornienko
alexfh at google.com
Thu May 22 12:56:11 PDT 2014
Author: alexfh
Date: Thu May 22 14:56:11 2014
New Revision: 209468
URL: http://llvm.org/viewvc/llvm-project?rev=209468&view=rev
Log:
Remove limits on the number of fix-it hints and ranges in the DiagnosticsEngine.
Summary:
The limits on the number of fix-it hints and ranges attached to a
diagnostic are arbitrary and don't apply universally to all users of the
DiagnosticsEngine. The way the limits are enforced may lead to diagnostics
generating invalid sets of fixes. I suggest removing the limits, which will also
simplify the implementation.
Reviewers: rsmith
Reviewed By: rsmith
Subscribers: klimek, cfe-commits
Differential Revision: http://reviews.llvm.org/D3879
Modified:
cfe/trunk/include/clang/Basic/Diagnostic.h
cfe/trunk/include/clang/Basic/PartialDiagnostic.h
cfe/trunk/lib/Basic/Diagnostic.cpp
cfe/trunk/lib/Frontend/SerializedDiagnosticPrinter.cpp
cfe/trunk/lib/Frontend/TextDiagnosticPrinter.cpp
cfe/trunk/lib/Lex/LiteralSupport.cpp
cfe/trunk/lib/Lex/PPMacroExpansion.cpp
Modified: cfe/trunk/include/clang/Basic/Diagnostic.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Diagnostic.h?rev=209468&r1=209467&r2=209468&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/Diagnostic.h (original)
+++ cfe/trunk/include/clang/Basic/Diagnostic.h Thu May 22 14:56:11 2014
@@ -738,20 +738,10 @@ private:
/// diagnostic with more than that almost certainly has to be simplified
/// anyway.
MaxArguments = 10,
-
- /// \brief The maximum number of ranges we can hold.
- MaxRanges = 10,
-
- /// \brief The maximum number of ranges we can hold.
- MaxFixItHints = 10
};
/// \brief The number of entries in Arguments.
signed char NumDiagArgs;
- /// \brief The number of ranges in the DiagRanges array.
- unsigned char NumDiagRanges;
- /// \brief The number of hints in the DiagFixItHints array.
- unsigned char NumDiagFixItHints;
/// \brief Specifies whether an argument is in DiagArgumentsStr or
/// in DiagArguments.
@@ -774,11 +764,11 @@ private:
intptr_t DiagArgumentsVal[MaxArguments];
/// \brief The list of ranges added to this diagnostic.
- CharSourceRange DiagRanges[MaxRanges];
+ SmallVector<CharSourceRange, 8> DiagRanges;
/// \brief If valid, provides a hint with some code to insert, remove,
/// or modify at a particular position.
- FixItHint DiagFixItHints[MaxFixItHints];
+ SmallVector<FixItHint, 8> DiagFixItHints;
DiagnosticMappingInfo makeMappingInfo(diag::Mapping Map, SourceLocation L) {
bool isPragma = L.isValid();
@@ -875,7 +865,7 @@ public:
/// for example.
class DiagnosticBuilder {
mutable DiagnosticsEngine *DiagObj;
- mutable unsigned NumArgs, NumRanges, NumFixits;
+ mutable unsigned NumArgs;
/// \brief Status variable indicating if this diagnostic is still active.
///
@@ -890,15 +880,15 @@ class DiagnosticBuilder {
void operator=(const DiagnosticBuilder &) LLVM_DELETED_FUNCTION;
friend class DiagnosticsEngine;
-
+
DiagnosticBuilder()
- : DiagObj(nullptr), NumArgs(0), NumRanges(0), NumFixits(0), IsActive(false),
- IsForceEmit(false) { }
+ : DiagObj(nullptr), NumArgs(0), IsActive(false), IsForceEmit(false) {}
explicit DiagnosticBuilder(DiagnosticsEngine *diagObj)
- : DiagObj(diagObj), NumArgs(0), NumRanges(0), NumFixits(0), IsActive(true),
- IsForceEmit(false) {
+ : DiagObj(diagObj), NumArgs(0), IsActive(true), IsForceEmit(false) {
assert(diagObj && "DiagnosticBuilder requires a valid DiagnosticsEngine!");
+ diagObj->DiagRanges.clear();
+ diagObj->DiagFixItHints.clear();
}
friend class PartialDiagnostic;
@@ -906,8 +896,6 @@ class DiagnosticBuilder {
protected:
void FlushCounts() {
DiagObj->NumDiagArgs = NumArgs;
- DiagObj->NumDiagRanges = NumRanges;
- DiagObj->NumDiagFixItHints = NumFixits;
}
/// \brief Clear out the current diagnostic.
@@ -954,8 +942,6 @@ public:
IsForceEmit = D.IsForceEmit;
D.Clear();
NumArgs = D.NumArgs;
- NumRanges = D.NumRanges;
- NumFixits = D.NumFixits;
}
/// \brief Retrieve an empty diagnostic builder.
@@ -1001,24 +987,12 @@ public:
void AddSourceRange(const CharSourceRange &R) const {
assert(isActive() && "Clients must not add to cleared diagnostic!");
- assert(NumRanges < DiagnosticsEngine::MaxRanges &&
- "Too many arguments to diagnostic!");
- DiagObj->DiagRanges[NumRanges++] = R;
+ DiagObj->DiagRanges.push_back(R);
}
void AddFixItHint(const FixItHint &Hint) const {
assert(isActive() && "Clients must not add to cleared diagnostic!");
- assert(NumFixits < DiagnosticsEngine::MaxFixItHints &&
- "Too many arguments to diagnostic!");
- DiagObj->DiagFixItHints[NumFixits++] = Hint;
- }
-
- bool hasMaxRanges() const {
- return NumRanges == DiagnosticsEngine::MaxRanges;
- }
-
- bool hasMaxFixItHints() const {
- return NumFixits == DiagnosticsEngine::MaxFixItHints;
+ DiagObj->DiagFixItHints.push_back(Hint);
}
void addFlagValue(StringRef V) const { DiagObj->setFlagNameValue(V); }
@@ -1224,22 +1198,22 @@ public:
/// \brief Return the number of source ranges associated with this diagnostic.
unsigned getNumRanges() const {
- return DiagObj->NumDiagRanges;
+ return DiagObj->DiagRanges.size();
}
/// \pre Idx < getNumRanges()
const CharSourceRange &getRange(unsigned Idx) const {
- assert(Idx < DiagObj->NumDiagRanges && "Invalid diagnostic range index!");
+ assert(Idx < getNumRanges() && "Invalid diagnostic range index!");
return DiagObj->DiagRanges[Idx];
}
/// \brief Return an array reference for this diagnostic's ranges.
ArrayRef<CharSourceRange> getRanges() const {
- return llvm::makeArrayRef(DiagObj->DiagRanges, DiagObj->NumDiagRanges);
+ return DiagObj->DiagRanges;
}
unsigned getNumFixItHints() const {
- return DiagObj->NumDiagFixItHints;
+ return DiagObj->DiagFixItHints.size();
}
const FixItHint &getFixItHint(unsigned Idx) const {
@@ -1247,8 +1221,8 @@ public:
return DiagObj->DiagFixItHints[Idx];
}
- const FixItHint *getFixItHints() const {
- return getNumFixItHints()? DiagObj->DiagFixItHints : nullptr;
+ ArrayRef<FixItHint> getFixItHints() const {
+ return DiagObj->DiagFixItHints;
}
/// \brief Format this diagnostic into a string, substituting the
Modified: cfe/trunk/include/clang/Basic/PartialDiagnostic.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/PartialDiagnostic.h?rev=209468&r1=209467&r2=209468&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/PartialDiagnostic.h (original)
+++ cfe/trunk/include/clang/Basic/PartialDiagnostic.h Thu May 22 14:56:11 2014
@@ -36,7 +36,7 @@ public:
};
struct Storage {
- Storage() : NumDiagArgs(0), NumDiagRanges(0) { }
+ Storage() : NumDiagArgs(0) { }
enum {
/// \brief The maximum number of arguments we can hold. We
@@ -50,9 +50,6 @@ public:
/// \brief The number of entries in Arguments.
unsigned char NumDiagArgs;
- /// \brief This is the number of ranges in the DiagRanges array.
- unsigned char NumDiagRanges;
-
/// \brief Specifies for each argument whether it is in DiagArgumentsStr
/// or in DiagArguments.
unsigned char DiagArgumentsKind[MaxArguments];
@@ -69,9 +66,7 @@ public:
std::string DiagArgumentsStr[MaxArguments];
/// \brief The list of ranges added to this diagnostic.
- ///
- /// It currently only support 10 ranges, could easily be extended if needed.
- CharSourceRange DiagRanges[10];
+ SmallVector<CharSourceRange, 8> DiagRanges;
/// \brief If valid, provides a hint with some code to insert, remove, or
/// modify at a particular position.
@@ -97,7 +92,6 @@ public:
Storage *Result = FreeList[--NumFreeListEntries];
Result->NumDiagArgs = 0;
- Result->NumDiagRanges = 0;
Result->FixItHints.clear();
return Result;
}
@@ -166,10 +160,7 @@ private:
if (!DiagStorage)
DiagStorage = getStorage();
- assert(DiagStorage->NumDiagRanges <
- llvm::array_lengthof(DiagStorage->DiagRanges) &&
- "Too many arguments to diagnostic!");
- DiagStorage->DiagRanges[DiagStorage->NumDiagRanges++] = R;
+ DiagStorage->DiagRanges.push_back(R);
}
void AddFixItHint(const FixItHint &Hint) const {
@@ -308,12 +299,12 @@ public:
}
// Add all ranges.
- for (unsigned i = 0, e = DiagStorage->NumDiagRanges; i != e; ++i)
- DB.AddSourceRange(DiagStorage->DiagRanges[i]);
+ for (const CharSourceRange &Range : DiagStorage->DiagRanges)
+ DB.AddSourceRange(Range);
// Add all fix-its.
- for (unsigned i = 0, e = DiagStorage->FixItHints.size(); i != e; ++i)
- DB.AddFixItHint(DiagStorage->FixItHints[i]);
+ for (const FixItHint &Fix : DiagStorage->FixItHints)
+ DB.AddFixItHint(Fix);
}
void EmitToString(DiagnosticsEngine &Diags,
Modified: cfe/trunk/lib/Basic/Diagnostic.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Diagnostic.cpp?rev=209468&r1=209467&r2=209468&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/Diagnostic.cpp (original)
+++ cfe/trunk/lib/Basic/Diagnostic.cpp Thu May 22 14:56:11 2014
@@ -323,22 +323,19 @@ void DiagnosticsEngine::Report(const Sto
CurDiagID = storedDiag.getID();
NumDiagArgs = 0;
- NumDiagRanges = storedDiag.range_size();
- assert(NumDiagRanges < DiagnosticsEngine::MaxRanges &&
- "Too many arguments to diagnostic!");
- unsigned i = 0;
+ DiagRanges.clear();
+ DiagRanges.reserve(storedDiag.range_size());
for (StoredDiagnostic::range_iterator
RI = storedDiag.range_begin(),
RE = storedDiag.range_end(); RI != RE; ++RI)
- DiagRanges[i++] = *RI;
+ DiagRanges.push_back(*RI);
- assert(NumDiagRanges < DiagnosticsEngine::MaxFixItHints &&
- "Too many arguments to diagnostic!");
- NumDiagFixItHints = 0;
+ DiagFixItHints.clear();
+ DiagFixItHints.reserve(storedDiag.fixit_size());
for (StoredDiagnostic::fixit_iterator
FI = storedDiag.fixit_begin(),
FE = storedDiag.fixit_end(); FI != FE; ++FI)
- DiagFixItHints[NumDiagFixItHints++] = *FI;
+ DiagFixItHints.push_back(*FI);
assert(Client && "DiagnosticConsumer not set!");
Level DiagLevel = storedDiag.getLevel();
Modified: cfe/trunk/lib/Frontend/SerializedDiagnosticPrinter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/SerializedDiagnosticPrinter.cpp?rev=209468&r1=209467&r2=209468&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/SerializedDiagnosticPrinter.cpp (original)
+++ cfe/trunk/lib/Frontend/SerializedDiagnosticPrinter.cpp Thu May 22 14:56:11 2014
@@ -560,8 +560,7 @@ void SDiagsWriter::HandleDiagnostic(Diag
Renderer.emitDiagnostic(Info.getLocation(), DiagLevel,
State->diagBuf.str(),
Info.getRanges(),
- llvm::makeArrayRef(Info.getFixItHints(),
- Info.getNumFixItHints()),
+ Info.getFixItHints(),
&Info.getSourceManager(),
&Info);
}
Modified: cfe/trunk/lib/Frontend/TextDiagnosticPrinter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/TextDiagnosticPrinter.cpp?rev=209468&r1=209467&r2=209468&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/TextDiagnosticPrinter.cpp (original)
+++ cfe/trunk/lib/Frontend/TextDiagnosticPrinter.cpp Thu May 22 14:56:11 2014
@@ -154,8 +154,7 @@ void TextDiagnosticPrinter::HandleDiagno
TextDiag->emitDiagnostic(Info.getLocation(), Level, DiagMessageStream.str(),
Info.getRanges(),
- llvm::makeArrayRef(Info.getFixItHints(),
- Info.getNumFixItHints()),
+ Info.getFixItHints(),
&Info.getSourceManager());
OS.flush();
Modified: cfe/trunk/lib/Lex/LiteralSupport.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/LiteralSupport.cpp?rev=209468&r1=209467&r2=209468&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/LiteralSupport.cpp (original)
+++ cfe/trunk/lib/Lex/LiteralSupport.cpp Thu May 22 14:56:11 2014
@@ -1577,8 +1577,7 @@ bool StringLiteralParser::CopyStringFrag
Dummy.reserve(Fragment.size() * CharByteWidth);
char *Ptr = Dummy.data();
- while (!Builder.hasMaxRanges() &&
- !ConvertUTF8toWide(CharByteWidth, NextFragment, Ptr, ErrorPtrTmp)) {
+ while (!ConvertUTF8toWide(CharByteWidth, NextFragment, Ptr, ErrorPtrTmp)) {
const char *ErrorPtr = reinterpret_cast<const char *>(ErrorPtrTmp);
NextStart = resyncUTF8(ErrorPtr, Fragment.end());
Builder << MakeCharSourceRange(Features, SourceLoc, TokBegin,
Modified: cfe/trunk/lib/Lex/PPMacroExpansion.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPMacroExpansion.cpp?rev=209468&r1=209467&r2=209468&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/PPMacroExpansion.cpp (original)
+++ cfe/trunk/lib/Lex/PPMacroExpansion.cpp Thu May 22 14:56:11 2014
@@ -675,13 +675,8 @@ MacroArgs *Preprocessor::ReadFunctionLik
DiagnosticBuilder DB =
Diag(MacroName,
diag::note_init_list_at_beginning_of_macro_argument);
- for (SmallVector<SourceRange, 4>::iterator
- Range = InitLists.begin(), RangeEnd = InitLists.end();
- Range != RangeEnd; ++Range) {
- if (DB.hasMaxRanges())
- break;
- DB << *Range;
- }
+ for (const SourceRange &Range : InitLists)
+ DB << Range;
}
return nullptr;
}
@@ -689,15 +684,9 @@ MacroArgs *Preprocessor::ReadFunctionLik
return nullptr;
DiagnosticBuilder DB = Diag(MacroName, diag::note_suggest_parens_for_macro);
- for (SmallVector<SourceRange, 4>::iterator
- ParenLocation = ParenHints.begin(), ParenEnd = ParenHints.end();
- ParenLocation != ParenEnd; ++ParenLocation) {
- if (DB.hasMaxFixItHints())
- break;
- DB << FixItHint::CreateInsertion(ParenLocation->getBegin(), "(");
- if (DB.hasMaxFixItHints())
- break;
- DB << FixItHint::CreateInsertion(ParenLocation->getEnd(), ")");
+ for (const SourceRange &ParenLocation : ParenHints) {
+ DB << FixItHint::CreateInsertion(ParenLocation.getBegin(), "(");
+ DB << FixItHint::CreateInsertion(ParenLocation.getEnd(), ")");
}
ArgTokens.swap(FixedArgTokens);
NumActuals = FixedNumArgs;
More information about the cfe-commits
mailing list