aaron.ballman added a subscriber: aaron.ballman. aaron.ballman requested changes to this revision. aaron.ballman added a reviewer: aaron.ballman. aaron.ballman added a comment. This revision now requires changes to proceed.
I would like to hold off on adding the namespace attribute. There were persuasive reasons to not have attributes on namespaces that was discussed in EWG in Kona, and this is a feature we could add on later if there's sufficient design consensus. ================ Comment at: include/clang/Basic/Attr.td:2125 @@ +2124,3 @@ + +def InternalLinkage : InheritableAttr { + let Spellings = [GNU<"internal_linkage">, CXX11<"clang", "internal_linkage">]; ---------------- rsmith wrote: > `InheritableAttr` doesn't seem right for the case when this is applied to a > namespace. Long and unusual is not a problem; that's why you can manually specify the diagnostic enum. This should be using Subjects unless it's reallllly onerous (or impossible). ================ Comment at: include/clang/Basic/AttrDocs.td:1619 @@ +1618,3 @@ + let Content = [{ +The ``internal_linkage`` attribute changes the linkage type of the declaration to internal. + }]; ---------------- Some examples would be nice, as well as an explanation as to why someone might want to do this. ================ Comment at: lib/Sema/SemaDeclAttr.cpp:3306 @@ +3305,3 @@ + + if (InternalLinkageAttr *Internal = D->getAttr<InternalLinkageAttr>()) { + Diag(Range.getBegin(), diag::err_attributes_are_not_compatible) ---------------- Please use checkAttrMutualExclusion instead (and augment it with the note_conflicting_attribute). ================ Comment at: lib/Sema/SemaDeclAttr.cpp:3320 @@ +3319,3 @@ + + if (CommonAttr *Common = D->getAttr<CommonAttr>()) { + Diag(Range.getBegin(), diag::err_attributes_are_not_compatible) ---------------- Please use checkAttrMutualExclusion instead. ================ Comment at: test/SemaCXX/internal_linkage.cpp:6 @@ +5,3 @@ +public: + int x __attribute__((internal_linkage)); // expected-warning{{'internal_linkage' attribute ignored}} + static int y __attribute__((internal_linkage)); ---------------- This does not seem like a useful diagnostic as it doesn't tell the user *why* it was ignored. ================ Comment at: test/SemaCXX/internal_linkage.cpp:25 @@ +24,2 @@ +void A::f1() { +} ---------------- Missing tests for internal_linkage accepting an argument (which should diagnose). Missing tests for what happens with forward declarations and redeclarations that do not have matching attribute specifiers. Missing tests for using the C++ spelling of the attribute. Repository: rL LLVM http://reviews.llvm.org/D13925 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits