[PATCH] D88859: APINotes: add APINotesYAMLCompiler

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd marked an inline comment as done.
compnerd added a comment.

> I'd like to have maximum flexibility from the submitter by willing to change the details of the implementation

I don't think that is a fair expectation - there are plenty of times where this is technical disagreement on functionality, and it doesn't get immediately resolved.  A high quality implementation doesn't require that everything be changed immediately, nor does it mean that everything must be completely flexible.  There are trade-offs, and the goal is to strike a balance between simplicity and the needs of the project.



================
Comment at: clang/include/clang/APINotes/Types.h:25
+/// auditing.
+enum class EnumExtensibilityKind {
+  None,
----------------
martong wrote:
> compnerd wrote:
> > martong wrote:
> > > This seems a bit redundant to `Attrs.td`. I'd prefer to have the structure of an attribute defined only in one place. Why can't we directly reuse the types in the generated `Attrs.inc`? In this case
> > > ```
> > > class EnumExtensibilityAttr : public InheritableAttr {
> > > public:
> > >   enum Kind {
> > >     Closed,
> > >     Open
> > >   };
> > > ```
> > > could perfectly fit, wouldn't it?
> > The none-case here is not the same as missing - it tracks the explicitly audited case.  I suppose we can change the internal enum case from `None` to `Audited` if you like.
> I am not sure I can follow. Could you please elaborate what is an "explicit y audited" case?
> https://clang.llvm.org/docs/AttributeReference.html#enum-extensibility mentions only open/closed ...
There are three states consider:

```
enum Unaudited {
};
enum __attribute__((__enum_extensibility__(open))) Open {
};
enum __attribute__((__enum_extensibility__(closed))) Closed {
};
```

The optionality of the value indicates whether the value is present or not in the APINotes, not the tri-state nature of the attribute.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88859/new/

https://reviews.llvm.org/D88859