majnemer added inline comments.
================
Comment at: lib/AST/ItaniumMangle.cpp:217-218
@@ -216,2 +216,4 @@
raw_ostream &Out;
+ bool NullOut = false;
+ bool DisableDerivedAbiTags = false;
----------------
Please add a comment describe this state.
================
Comment at: lib/AST/ItaniumMangle.cpp:329
@@ +328,3 @@
+
+ bool LinkActive{ false };
+
----------------
Please use = instead of list initialization.
================
Comment at: lib/AST/ItaniumMangle.cpp:332
@@ +331,3 @@
+ public:
+ explicit AbiTagState(AbiTagState *&linkHead) : LinkHead(linkHead) {
+ Parent = LinkHead;
----------------
Variables start with an uppercase letter.
================
Comment at: lib/AST/ItaniumMangle.cpp:339-340
@@ +338,4 @@
+ // no copy, no move
+ AbiTagState(AbiTagState const &) = delete;
+ AbiTagState &operator=(AbiTagState const &) = delete;
+
----------------
const typically goes before the type.
================
Comment at: lib/AST/ItaniumMangle.cpp:363
@@ +362,3 @@
+
+ if (dyn_cast<FunctionDecl>(ND) || dyn_cast<VarDecl>(ND)) {
+ // assert(AdditionalAbiTags && "function and variables need a list of
----------------
Don't use dyn_cast if you aren't going to use the value, use isa instead.
================
Comment at: lib/AST/ItaniumMangle.cpp:364-365
@@ +363,4 @@
+ if (dyn_cast<FunctionDecl>(ND) || dyn_cast<VarDecl>(ND)) {
+ // assert(AdditionalAbiTags && "function and variables need a list of
+ // additional abi tags");
+ } else {
----------------
commented out code?
================
Comment at: lib/AST/ItaniumMangle.cpp:394
@@ +393,3 @@
+ if (std::find(TagList.begin(), TagList.end(), Tag) == TagList.end())
{
+ // don't insert duplicates
+ TagList.push_back(Tag);
----------------
Sentences in LLVM comments start with a capital letter and end with a period.
================
Comment at: lib/AST/ItaniumMangle.cpp:427
@@ +426,3 @@
+ AbiTagState *AbiTags = nullptr;
+ AbiTagState AbiTagsRoot{ AbiTags };
+
----------------
Is this clang-format'd?
================
Comment at: lib/AST/ItaniumMangle.cpp:670-671
@@ +669,4 @@
+ const AbiTagList *AdditionalAbiTags) {
+ assert(AbiTags && "require AbiTagState");
+ if (AbiTags)
+ AbiTags->write(Out, ND,
----------------
Why do you have an assert and check for that the pointer is not null?
================
Comment at: lib/AST/ItaniumMangle.cpp:824
@@ +823,3 @@
+ if (!ExcludeUnqualifiedName) {
+ if (const VarDecl *VD = dyn_cast<VarDecl>(ND)) {
+ AbiTagList VariableAdditionalAbiTags = makeAdditionalTagsForVariable(VD);
----------------
Use `const auto *` if the type is obvious from the RHS.
================
Comment at: lib/AST/ItaniumMangle.cpp:1447
@@ +1446,3 @@
+ {
+ AbiTagState localAbiTags(AbiTags);
+
----------------
Variables start with an upper case letter.
================
Comment at: lib/AST/ItaniumMangle.cpp:4641-4642
@@ -4218,2 +4640,4 @@
CXXNameMangler Mangler(*this, Out);
+ // GCC 5.3.0 doesn't emit derived abi tags for but that seems to be a bug
+ // that is fixed in trunk.
Mangler.getStream() << "_ZGV";
----------------
I think a noun is missing between "for" and "but".
http://reviews.llvm.org/D18035
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
