rtrieu added inline comments.
================
Comment at: lib/Frontend/DiagnosticRenderer.cpp:455
@@ -419,1 +454,3 @@
+static bool checkLocForMacroArgExpansion(SourceLocation Loc,
+ const SourceManager &SM,
----------------
rtrieu wrote:
> zhengkai wrote:
> > Because the function isMacroArgExpansion doesn't check if all locations are
> > in the same argument location,
> I still don't know what you are doing here. You have two SourceLocation's,
> Loc and PreLoc. Those are not good names for telling them apart or what they
> do. You pass the second SourceLocation in by reference, both as an input
> value, and later as something to put an output value in. This is a bad idea.
> If you need a SourceLocation to be passed back to the caller, make the
> function return a SourceLocation, or use a SourceLocation pointer as an
> argument that is only for output. The SourceLocation class is tiny so it is
> okay to copy around.
Still not answered: What is PreLoc? Why is it important to this function?
What does PreLoc even mean?
================
Comment at: lib/Frontend/DiagnosticRenderer.cpp:471
@@ -422,1 +470,3 @@
+ const SourceManager &SM,
+ SourceLocation &Loc) {
SourceLocation BegLoc = Range.getBegin(), EndLoc = Range.getEnd();
----------------
zhengkai wrote:
> rtrieu wrote:
> > What is this SourceLocation? Why is it passed by reference?
> Changed
What is Loc? Where is it pointing to? Why do you need it?
================
Comment at: test/Misc/caret-diags-macros.c:210
@@ -215,3 +209,3 @@
void f(char* pMsgBuf, char* pKeepBuf) {
-Csprintf(pMsgBuf,"\nEnter minimum anagram length (2-%1d): ",
Cstrlen(pKeepBuf));
+Csprintf(pMsgBuf,"\nEnter minimum anagram length (2-%1d): ",
strlen_test(pKeepBuf));
}
----------------
rtrieu wrote:
> Add a FIXME comment that it should be changed back to Cstrlen when the bug is
> resolved.
Use:
```
// FIXME: Change test to use 'Cstrlen' instead of 'strlen_test' when macro
printing is fixed.
```
http://reviews.llvm.org/D12379
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
