[PATCH] Knowledge of GCC attribtues

Aaron Ballman aaron at aaronballman.com
Mon Jan 27 14:17:52 PST 2014


On Mon, Jan 27, 2014 at 4:19 PM, Richard Smith <metafoo at gmail.com> wrote:
> Looks great! It'd be nice for the 'isKnownToGCC' thing to look at the
> spelling that was used, but that can wait (I don't think we have attributes
> where it matters yet).

I actually went off the bit setting instead of the name of the
spelling itself. This way, if we want to support an attribute from GCC
that is not implemented as both __attribute__((ident)) and
[[gnu::ident]], we can do so. For instance, if GCC adds a C++11-only
attribute (like we did with clang::fallthrough), we can support
knowledge of it.

> +class FlattenedSpelling {
> +  std::string V, N, NS;
> +  bool K;
> +
> +public:
> +  FlattenedSpelling(const std::string &Variety, const std::string &Name,
> +                    const std::string &Namespace, bool KnownToGCC) :
> +    V(Variety), N(Name), NS(Namespace), K(KnownToGCC) {}
> +  explicit FlattenedSpelling(const Record &Spelling) :
> +    V(Spelling.getValueAsString("Variety")),
> +    N(Spelling.getValueAsString("Name")) {
>
> Maybe assert that V is not "GCC" here?
>
> +    if (V == "CXX11")
> +      NS = Spelling.getValueAsString("Namespace");
> +    bool Unset;
> +    K = Spelling.getValueAsBitOrUnset("KnownToGCC", Unset);
> +  }
> +  FlattenedSpelling(const FlattenedSpelling &RHS) : V(RHS.V), N(RHS.N),
> +    NS(RHS.NS), K(RHS.K) {}
> +
> +  FlattenedSpelling& operator=(const FlattenedSpelling &RHS) {
> +    if (&RHS != this) {
> +      V = RHS.V;
> +      N = RHS.N;
> +      NS = RHS.NS;
> +      K = RHS.K;
> +    }
> +    return *this;
> +  }
>
> Can you just use the implicit copy operations?
>
> +  const std::string& Variety() const { return V; }
> +  const std::string& Name() const { return N; }
> +  const std::string& Namespace() const { return NS; }
>
> & on the right, please, and start function names with lowercase.
>
> +  bool KnownToGCC() const { return K; }
> +};

Thanks for the rest of the review! I've made those changes and
committed in r200252.

~Aaron



More information about the cfe-commits mailing list