jdoerfert added a comment.
Updated according to your comments.
================
Comment at: lib/Sema/SemaDeclAttr.cpp:3481
+ llvm::StringMap<int> NameIdxMapping;
+ NameIdxMapping["__"] = -1;
+
----------------
aaron.ballman wrote:
> This doesn't match the documentation -- I assume you switched from `?` to
> `__` because `__` at least parses as a valid identifier, whereas `?` would
> require extra parsing support? If so, that's fine by me.
Yes, `__`, and `__this` where chosen because they work without lexer/parser
changes and are in the implementation namespace. I forgot to update the
documentation though. Will be fixed.
================
Comment at: lib/Sema/SemaDeclAttr.cpp:3483
+
+ NameIdxMapping["__this"] = 0;
+
----------------
aaron.ballman wrote:
> This doesn't match the documentation either, but I'm less clear on why the
> double underscores are used.
If you use `this`, the lexer will generate the special "this" token. That one
is checked explicitly to be only used inside of non-static class methods. If
you have an idea how to avoid this check or make it consider uses in the
attribute as OK, please let me know.
================
Comment at: lib/Sema/SemaDeclAttr.cpp:3492
+ SmallVector<int, 8> EncodingIndices;
+ for (unsigned u = 0, e = AL.getNumArgs(); u < e; u++) {
+
----------------
aaron.ballman wrote:
> Identifiers don't match the usual naming conventions. Prefer `++U` as well.
OK.
> Prefer ++U as well.
Out of curiosity, why?
================
Comment at: lib/Sema/SemaDeclAttr.cpp:3493
+ for (unsigned u = 0, e = AL.getNumArgs(); u < e; u++) {
+
+ SourceRange SR;
----------------
aaron.ballman wrote:
> Spurious newline
That was intentional but OK.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55483/new/
https://reviews.llvm.org/D55483
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
