martong added a comment.
Nice and precise work! And special thanks for the unit tests. I've found some
minor nits.
================
Comment at: clang/include/clang/Analysis/MacroExpansionContext.h:77
+ /// \param MacroExpansionLoc Must be the expansion location of a macro.
+ /// \return The textual representation of the token sequence which were
+ /// substituted in place of the macro.
----------------
?
================
Comment at: clang/include/clang/Analysis/MacroExpansionContext.h:79-80
+ /// substituted in place of the macro.
+ /// If no macro was expanded at that location, returns an empty
+ /// string.
+ MacroExpansionText
----------------
I think we should have an Optional<String> as a return type, so we'd be able to
detect if no macro expansion happened. (?)
================
Comment at: clang/include/clang/Analysis/MacroExpansionContext.h:81
+ /// string.
+ MacroExpansionText
+ getExpandedMacroForLocation(SourceLocation MacroExpansionLoc) const;
----------------
Could this be `StringRef` also?
================
Comment at: clang/include/clang/Analysis/MacroExpansionContext.h:87-88
+ /// the macro expansion chain from the given location.
+ /// If no macro was expanded at that location, returns an empty
+ /// string.
+ StringRef
----------------
We should be able to distinguish between no macro expansion (1) and expansion
to an empty string (2). So, I think we should have an Optional<StringRef> as a
return type.
================
Comment at: clang/lib/Analysis/MacroExpansionContext.cpp:39
+
+ SourceLocation ExpansionEnd = [Range, &SM = SM, &MacroName] {
+ // If the range is empty, use the length of the macro.
----------------
?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93222/new/
https://reviews.llvm.org/D93222
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
