aaron.ballman added inline comments.
================
Comment at: include/clang/AST/DeclBase.h:563
@@ +562,3 @@
+ /// \brief Return true if this declaration is a definition of alias or ifunc.
+ bool hasDefiningAttr() const;
+
----------------
I think this function and getDefiningAttr() can be defined in the header
instead of split into the source file. The implementations are short enough
that inlining may be nice to allow.
================
Comment at: include/clang/AST/DeclBase.h:565
@@ +564,3 @@
+
+ /// \brief Returns AliasAttr or IFuncAttr if any or nullptr.
+ Attr* getDefiningAttr() const;
----------------
rjmccall wrote:
> "Return this declaration's defining attribute if it has one."
>
> Also, please put the * next to the method name rather than the type.
Also, since this is a const method, I'd prefer if it returned a `const Attr *`
if possible.
================
Comment at: lib/AST/DeclBase.cpp:369
@@ +368,3 @@
+
+Attr* Decl::getDefiningAttr() const {
+ if (AliasAttr *AA = getAttr<AliasAttr>())
----------------
Attr* -> Attr *
================
Comment at: lib/Sema/SemaDeclAttr.cpp:1538
@@ +1537,3 @@
+ // Aliases should be on declarations, not definitions.
+ if (const auto *FD = dyn_cast<FunctionDecl>(D)) {
+ if (FD->isThisDeclarationADefinition()) {
----------------
Can use cast<> instead of dyn_cast<> and remove the if.
================
Comment at: lib/Sema/SemaDeclAttr.cpp:1543
@@ +1542,3 @@
+ }
+ if (S.Context.getTargetInfo().getTriple().getObjectFormat() !=
+ llvm::Triple::ELF) {
----------------
I think that this should be codified in Attr.td as a target-specific attribute;
though that may require a bit of work for the attribute emitter to handle. I
don't think this needs to be done in this patch, but it should at least have a
FIXME or be handled in a follow-up patch.
================
Comment at: lib/Sema/SemaDeclAttr.cpp:1549
@@ +1548,3 @@
+ } else
+ llvm_unreachable("ifunc must be used for function declaration");
+
----------------
Can remove the else clause entirely; common attribute handling already takes
care of this and by switching to use cast<> instead of dyn_cast<> you already
get an assert if the type is incorrect.
http://reviews.llvm.org/D15524
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
