jkorous-apple added a comment.
Replied and going to delete the end delimiter.
================
Comment at: lib/Index/USRGeneration.cpp:819
}
+ if (const ArrayType *const AT = dyn_cast<ArrayType>(T)) {
+ Out << "{";
----------------
arphaman wrote:
> You can use `const auto *` here.
That's what I wanted to do in the first place but decided against it because of
consistency - see other if conditions around. Do you still advice to use auto?
================
Comment at: lib/Index/USRGeneration.cpp:826
+ }
+ if (const ConstantArrayType* const CAT = dyn_cast<ConstantArrayType>(T))
{
+ Out << CAT->getSize();
----------------
arphaman wrote:
> Ditto. Also, the braces are redundant.
Braces might be redundant yet I use them intentionally even for such cases
(think the infamous goto bug). But I can definitely delete those if you insist.
BTW Is there some consensus about this?
================
Comment at: lib/Index/USRGeneration.cpp:829
+ }
+ Out << "}";
+ T = AT->getElementType();
----------------
arphaman wrote:
> I don't think we need the terminating character.
I agree, going to delete that.
================
Comment at: test/Index/USR/array-type.cpp:1
+// RUN: c-index-test core -print-source-symbols -- %s | grep
"function(Gen,TS)/C++" | grep foo | cut -s -d "|" -f 4 | uniq | wc -l | grep 3
+
----------------
arphaman wrote:
> Please use `FileCheck` and verify the exact USR strings. This way you'll know
> that they're unique without actually trying to verify if they're unique in
> the test.
Since the USR format doesn't seem to be really stable I wanted to avoid
hard-coding USR values in tests. Wouldn't those tests be unnecessary brittle in
the sense that hard-coded values would have to be modified in case of future
changes?
https://reviews.llvm.org/D38643
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
