On May 17, 2013, at 10:52 AM, Jordan Rose <[email protected]> wrote:
> Small comments:
>
>> @@ -119,6 +120,62 @@ PathDiagnostic::PathDiagnostic(const Dec
>> UniqueingDecl(DeclToUnique),
>> path(pathImpl) {}
>>
>> +static PathDiagnosticCallPiece *
>> +getFirstStackedCallToHeaderFile(PathDiagnosticCallPiece *CP,
>> + const SourceManager &SMgr) {
>> + assert(SMgr.isFromMainFile(CP->callEnter.asLocation()) &&
>> + "The call piece should be in the main file.");
>> +
>> + // Check if CP represents a path through a function outside of the main
>> file.
>> + if (!SMgr.isFromMainFile(CP->callEnterWithin.asLocation()))
>> + return CP;
>> +
>> + // Check if the last piece in the callee path is a call to a function
>> outside
>> + // of the main file.
>> + const PathPieces &Path = CP->path;
>> + if (PathDiagnosticCallPiece *CPInner =
>> + dyn_cast<PathDiagnosticCallPiece>(Path.back())) {
>> + return getFirstStackedCallToHeaderFile(CPInner, SMgr);
>> + }
>
> The inner path might be empty for bugs that have path pruning turned off. I
> don't think we have many of those, but it's worth checking.
>
>
>> + // Otherwise, the last piece is in the main file.
>> + return 0;
>> +}
>> +
>> +void PathDiagnostic::resetDiagnosticLocationToMainFile() {
>> + if (path.empty())
>> + return;
>> +
>> + PathDiagnosticPiece *LastP = path.back().getPtr();
>> + const SourceManager &SMgr = LastP->getLocation().getManager();
>> + assert(LastP);
>
> You're using LastP before asserting it here.
>
>
>> + // We only need to check if the report ends inside headers, if the last
>> piece
>> + // is a call piece.
>> + if (PathDiagnosticCallPiece *CP =
>> dyn_cast<PathDiagnosticCallPiece>(LastP)) {
>> + CP = getFirstStackedCallToHeaderFile(CP, SMgr);
>> + if (CP) {
>> + // Mark the piece.
>> + CP->setAsLastInMainSourceFile();
>> +
>> + // Update the path diagnostic message.
>> + const NamedDecl *ND = dyn_cast<NamedDecl>(CP->getCallee());
>> + if (ND) {
>> + SmallString<200> buf;
>> + llvm::raw_svector_ostream os(buf);
>> + os << " (within a call to " << ND->getDeclName().getAsString() <<
>> ")";
>> + appendToDesc(os.str());
>> + }
>
> Very recently this became just "<< ND->getDeclName()", which is a bit
> cleaner. This should probably be single-quoted, too, since this will look
> weird for, say, copy constructors:
>
> "(within a call to ArrayRef)"
> vs.
> "(within a call to 'ArrayRef')"
>
> ...but really we need to refactor all our function/method pretty-printing so
> that we get nice names like "copy constructor for ArrayRef" and
> "-fetchEverything:delegate:".
>
> Also, it might be worth commenting on the non-NamedDecl case of a block,
> which we don't expect to happen because people don't usually define blocks in
> header files. (But they could, so we shouldn't assert.)
We would just not print anything. We don't assert..
>
>
>> + // Reset the report containing declaration and location.
>> + DeclWithIssue = CP->getCaller();
>> + Loc = CP->getLocation();
>> +
>> + return;
>> + }
>> + }
>> +}
>> +
>> void PathDiagnosticConsumer::anchor() { }
>>
>> PathDiagnosticConsumer::~PathDiagnosticConsumer() {
>>
>> Modified: cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp?rev=182058&r1=182057&r2=182058&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp (original)
>> +++ cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp Thu May 16
>> 17:30:45 2013
>> @@ -215,13 +215,18 @@ static void ReportEvent(raw_ostream &o,
>> const SourceManager &SM,
>> const LangOptions &LangOpts,
>> unsigned indent,
>> - unsigned depth) {
>> + unsigned depth,
>> + bool isKeyEvent = false) {
>>
>> Indent(o, indent) << "<dict>\n";
>> ++indent;
>>
>> Indent(o, indent) << "<key>kind</key><string>event</string>\n";
>>
>> + if (isKeyEvent) {
>> + Indent(o, indent) << "<key>key_event</key><string>YES</string>\n";
>> + }
>
> The best-practices way to do this is <true/> rather than <string>YES</string>.
>
>
>> // Output the location.
>> FullSourceLoc L = P.getLocation().asLocation();
>>
>> @@ -270,7 +275,8 @@ static void ReportPiece(raw_ostream &o,
>> const LangOptions &LangOpts,
>> unsigned indent,
>> unsigned depth,
>> - bool includeControlFlow);
>> + bool includeControlFlow,
>> + bool isKeyEvent = false);
>>
>> static void ReportCall(raw_ostream &o,
>> const PathDiagnosticCallPiece &P,
>> @@ -283,7 +289,8 @@ static void ReportCall(raw_ostream &o,
>> P.getCallEnterEvent();
>>
>> if (callEnter)
>> - ReportPiece(o, *callEnter, FM, SM, LangOpts, indent, depth, true);
>> + ReportPiece(o, *callEnter, FM, SM, LangOpts, indent, depth, true,
>> + P.isLastInMainSourceFile());
>>
>> IntrusiveRefCntPtr<PathDiagnosticEventPiece> callEnterWithinCaller =
>> P.getCallEnterWithinCallerEvent();
>> @@ -331,7 +338,8 @@ static void ReportPiece(raw_ostream &o,
>> const LangOptions &LangOpts,
>> unsigned indent,
>> unsigned depth,
>> - bool includeControlFlow) {
>> + bool includeControlFlow,
>> + bool isKeyEvent) {
>> switch (P.getKind()) {
>> case PathDiagnosticPiece::ControlFlow:
>> if (includeControlFlow)
>> @@ -344,7 +352,7 @@ static void ReportPiece(raw_ostream &o,
>> break;
>> case PathDiagnosticPiece::Event:
>> ReportEvent(o, cast<PathDiagnosticSpotPiece>(P), FM, SM, LangOpts,
>> - indent, depth);
>> + indent, depth, isKeyEvent);
>> break;
>> case PathDiagnosticPiece::Macro:
>> ReportMacro(o, cast<PathDiagnosticMacroPiece>(P), FM, SM, LangOpts,
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
