aaron.ballman added inline comments.
================
Comment at: include/clang/Basic/Attr.td:602
def AnalyzerNoReturn : InheritableAttr {
- let Spellings = [GNU<"analyzer_noreturn">];
+ let Spellings = [Clang<"analyzer_noreturn">];
let Documentation = [Undocumented];
----------------
dcoughlin wrote:
> aaron.ballman wrote:
> > dcoughlin wrote:
> > > aaron.ballman wrote:
> > > > rsmith wrote:
> > > > > Hmm, should the clang static analyzer reuse the `clang::` namespace,
> > > > > or should it get its own?
> > > > Good question, I don't have strong opinions on the answer here, but
> > > > perhaps @dcoughlin does?
> > > >
> > > > If we want to use a separate namespace for the analyzer, would we want
> > > > to use that same namespace for any clang-tidy specific attributes? Or
> > > > should clang-tidy get its own namespace? (Do we ever plan to execute
> > > > clang-tidy through the clang driver? That might change our answer.)
> > > How would this look if we added a special namespace for the clang static
> > > analyzer? Would this lead to duplication (say,
> > > [[clang_analyzer::analyzer_noreturn]]) so that we keep the "analyzer_"
> > > prefix for __attribute__((analyzer_noreturn))? Or could we have the
> > > "analyzer_" prefix only for GNU-style attributes but not for C++ (for
> > > example, [[clang_analyzer::noreturn]])?
> > >
> > > As for clang-tidy, I think it probably makes sense for it to have its own
> > > namespace, but we should ask @alexfh.
> > > How would this look if we added a special namespace for the clang static
> > > analyzer? Would this lead to duplication (say,
> > > [[clang_analyzer::analyzer_noreturn]]) so that we keep the "analyzer_"
> > > prefix for attribute((analyzer_noreturn))? Or could we have the
> > > "analyzer_" prefix only for GNU-style attributes but not for C++ (for
> > > example, [[clang_analyzer::noreturn]])?
> >
> > We have the ability to do whatever we'd like there. Given that the
> > semantics are so similar to `[[noreturn]]`, I think it would be reasonable
> > to use `[[clang_analyzer::noreturn]]` and
> > `__attribute__((analyzer_noreturn))` if that's the direction you think is
> > best.
> >
> > > As for clang-tidy, I think it probably makes sense for it to have its own
> > > namespace, but we should ask @alexfh.
> >
> > I'm less enthusiastic about giving clang-tidy a vendor namespace that's
> > separate from the static analyzer, should the need arise. My biggest
> > concern there is that I would *really* like to see clang-tidy be more
> > tightly integrated with the clang driver (so users don't have to manually
> > execute a secondary tool). If that were to happen, then the user experience
> > would be that there are two vendor namespaces both related to analyzer
> > attributes.
> >
> > That said, I would also not be opposed to putting all of these attributes
> > under the `clang` vendor namespace and not having a separate vendor for the
> > analyzer or clang-tidy.
> I would be find with keeping all of these things under the `clang` namespace,
> too.
>
> That said, I do think there is some value in having a namespace for analyzer
> attributes separate from clang proper because the namespace would make it
> more clear that the attribute doesn't affect code generation.
I've changed this one back to the GNU spelling to give us time to decide how we
want to handle analyzer attributes.
I'm not certain "does not affect codegen" is the correct measure to use for
this, however. We have other attributes that muddy the water, such as
`annotate`, or the format specifier attributes -- these don't (really) impact
codegen in any way, but do impact more than just the analyzer. Given the
integration of the analyzer with Clang (and the somewhat fluid nature of what
is responsible for issuing diagnostics), I think we should proceed with caution
on the idea of an analyzer-specific namespace.
However, do you have a list of attributes you think might qualify as being
analyzer-only? I can make sure we leave those spellings alone in this patch.
================
Comment at: include/clang/Basic/Attr.td:649
def Availability : InheritableAttr {
- let Spellings = [GNU<"availability">];
+ let Spellings = [Clang<"availability">];
let Args = [IdentifierArgument<"platform">, VersionArgument<"introduced">,
----------------
aaron.ballman wrote:
> rsmith wrote:
> > Does the custom parsing for this work for the C++11 attribute syntax?
> Nope; I'll do this one separately (and make sure no other attributes
> similarly use custom parsing).
I've rolled back the following attributes with custom parsing to only use the
GNU spelling for right now: availability, objc_bridge_related,
argument_with_type_tag, pointer_with_type_tag, type_tag_for_datatype.
================
Comment at: include/clang/Basic/Attr.td:1218-1228
def NeonPolyVectorType : TypeAttr {
- let Spellings = [GNU<"neon_polyvector_type">];
+ let Spellings = [Clang<"neon_polyvector_type">];
let Args = [IntArgument<"NumElements">];
let Documentation = [Undocumented];
}
def NeonVectorType : TypeAttr {
----------------
aaron.ballman wrote:
> rsmith wrote:
> > I *think* these are a Clang invention rather than part of the ARM NEON
> > intrinsics specification, but perhaps you could ask someone from ARM to
> > confirm that.
> @sbaranga or @jmolloy -- do you happen to know the answer to this, or know
> someone who does?
Pinging this comment.
https://reviews.llvm.org/D40625
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
