[PATCH] D59121: [analyzer] Fix macro names in diagnostics within bigger macros.
Csaba Dabis via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 8 07:53:27 PST 2019
Charusso requested changes to this revision.
Charusso added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1985
bool partOfParentMacro = false;
+ StringRef PName = "";
if (ParentEx->getBeginLoc().isMacroID()) {
----------------
`ParentName`?
================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1996
+ StringRef MacroName = StartName;
+ while (true) {
LocStart = BRC.getSourceManager().getImmediateMacroCallerLoc(LocStart);
----------------
I was never cool in algorithms, but I think it is over-complicated. In Hungary we are about to build a new nuclear power plant so here it is really emphasized: never-ever create an infinite loop.
What about the following?:
```
while (LocStart.isMacroID()) {
StringRef CurrentMacroName = Lexer::getImmediateMacroNameForDiagnostics(
LocStart, BRC.getSourceManager(),
BRC.getASTContext().getLangOpts());
LocStart = BRC.getSourceManager().getImmediateMacroCallerLoc(LocStart);
if (CurrentMacroName != ParentName)
MacroName = CurrentMacroName;
}
```
================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2003
+ LocStart, BRC.getSourceManager(),
+ BRC.getASTContext().getLangOpts());
+ if (NextMacroName == PName)
----------------
There is too much duplication and it is difficult to understand what is going on. May here is the time for deduplicating?
Please note that, there is a function called `getMacroName(SourceLocation, BugReporterContext &)`, may you would put your own helper-function above that.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59121/new/
https://reviews.llvm.org/D59121
More information about the cfe-commits
mailing list