This is an archive of the discontinued LLVM Phabricator instance.

[analyzer][NFC] Refactoring BugReporter.cpp P3.: std::shared_pointer<PathDiagnosticPiece> -> PathDiagnosticPieceRef
ClosedPublic

Authored by Szelethus on Jul 28 2019, 2:51 PM.

Details

Summary

Exactly what it says on the tin!

find clang/ -type f -exec sed -i 's/std::shared_ptr<PathDiagnosticPiece>/PathDiagnosticPieceRef/g' {} \;
git diff -U3 --no-color HEAD^ | clang-format-diff -p1 -i

Diff Detail

Event Timeline

Szelethus created this revision.Jul 28 2019, 2:51 PM
Szelethus edited the summary of this revision. (Show Details)Jul 28 2019, 2:51 PM
Szelethus edited the summary of this revision. (Show Details)
NoQ accepted this revision.Jul 29 2019, 6:18 PM
find clang/ -type f -exec sed -i 's/std::shared_ptr<PathDiagnosticPiece>/PathDiagnosticPieceRef/g' {} \;
git diff -U3 --no-color HEAD^ | clang-format-diff -p1 -i

All you need to know about the state of C++ refactoring tools.

clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
449

Mmm, why do we need to define this twice?

This revision is now accepted and ready to land.Jul 29 2019, 6:18 PM

Just wondering, did you check if we actually need shared ownership for this type? If not, do not waste your time checking for now :)

clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
244

If you already touch these parts maybe you could capitalize some variable names.

clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
449

I agree, I would love to not to see it twice.

Szelethus updated this revision to Diff 212807.Aug 1 2019, 7:14 AM
Szelethus marked 5 inline comments as done.
Szelethus added inline comments.
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
244

Hmm, okay, so, what is our stance on this? Apparently, LLVM changed the coding guideline, what would be the best direction moving forward?

clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
449

This file only forward declares classes from PathDiagnostic.h, so we do need this here.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptAug 13 2019, 9:47 AM