aaron.ballman added inline comments.
================
Comment at: include/clang/Basic/Attr.td:2096
+def TriviallyRelocatable : InheritableAttr {
+ let Spellings = [CXX11<"", "trivially_relocatable", 200809>,
+ CXX11<"clang", "trivially_relocatable">];
----------------
erichkeane wrote:
> Quuxplusone wrote:
> > erichkeane wrote:
> > > This spelling is almost definitely not acceptable until it is in the
> > > standard. Also, why specify that it was added in 2008?
> > Agreed, it should be `[[clang::trivially_relocatable]]` for Clang's
> > purposes. This spelling was because this patch came from the Godbolt
> > Compiler Explorer patch where I wanted the shorter/future spelling for
> > public relations reasons. :)
> > IIUC, the appropriate fix here is to change these two lines from
> > ```
> > let Spellings = [CXX11<"", "trivially_relocatable", "200809">,
> > CXX11<"clang", "trivially_relocatable">];
> > ```
> > to
> > ```
> > let Spellings = [Clang<"trivially_relocatable">,
> > ```
> > I believe the "200809" was because I wanted it to be available in C++11
> > mode on Godbolt.
> Yep, thats the suggested fix.
> to
> `let Spellings = [Clang<"trivially_relocatable">,`
Close, but since this attribute has no meaning in C mode, it should be `let
Spellings = [Clang<"trivially_relocatable", 0>,`
================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8207
+def err_attribute_missing_on_first_decl : Error<
+ "type %0 declared %1 after its first declaration">;
----------------
erichkeane wrote:
> Quuxplusone wrote:
> > erichkeane wrote:
> > > I'm shocked that there isn't a different diagnostic to do this same
> > > thing. @aaron.ballman likely knows better... I haven't seen the usage
> > > yet, but I presume you don't necessarily want a behavior that doesn't
> > > allow forward declarations.
> > I would be very happy to see this diagnostic get into trunk separately and
> > earlier than D50119. There are some other diagnostics that could be merged
> > with this one, e.g. `[[noreturn]]` needs a version of this diagnostic, and
> > I believe `[[clang::trivial_abi]]` should have it added.
> >
> > I don't know how to link to comments on Phabricator, but Ctrl+F downward
> > for this example:
> > ```
> > struct S { S(S&&); ~S(); };
> > std::vector<S> vec;
> > struct [[trivially_relocatable]] S; // ha ha, now you have to re-do all of
> > vector's codegen!
> > ```
> > This is why it is important to diagnose and disallow "backward
> > declarations." I don't particularly care about "forward declarations"
> > (either to allow or disallow them). The attribute would end up getting used
> > only on library types where IMHO nobody should ever be forward-declaring
> > them anyway. E.g. it is not a good idea for a regular C++ programmer to
> > forward-declare `unique_ptr`. But if there's a way to allow
> > forward-declarations (when the type remains incomplete) while disallowing
> > backward-declarations (adding the attribute to an already-complete type),
> > then I will be happy to do it.
> Sure, I get that... I'm just surprised/amazed it doesn't already exist.
> Hopefully Aaron takes a look.
Yeah, I'm also surprised this doesn't already exist, but... it doesn't seem to.
We have `err_noreturn_missing_on_first_decl`,
`err_carries_dependency_missing_on_first_decl`, and
`err_internal_linkage_redeclaration` to all do the same thing.
Repository:
rC Clang
https://reviews.llvm.org/D50119
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
