[PATCH] D97072: [OpenCL][Docs] Add guidelines for adding new extensions and features

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added a comment.

LGTM mostly, but would like to hear the opinion of others on the discussion about "useful".



================
Comment at: clang/docs/OpenCLSupport.rst:221
+triggers in the parsing) should not be added to clang. Moreover, the acceptable
+behavior of pragmas should provide useful functionality to the user.
+
----------------
Anastasia wrote:
> svenvh wrote:
> > Anastasia wrote:
> > > svenvh wrote:
> > > > Remove "acceptable behavior of".
> > > > 
> > > > How do you define "useful functionality"?
> > > I think this is one of those situations that is hard to define unambiguously, so I am open to suggestions. However, I do want to prevent changes that are reinventing the wheel (i.e. C/C++ already had another way to do the same thing) or functionality that doesn't add any value (i.e. requiring pragma enable to use already added types or functions). This has happened in the past multiple times so I think it is good to be specific that when new functionality is added it should have a reason for it.
> > > 
> > > I think the other statement above:
> > > 
> > > > New functionality is accepted as soon as the documentation is detailed to the level sufficient to be implemented.
> > > 
> > > is similar. It is not very easy to tell what is the detailed level of documentation. FYI it generally aligns with clang overall policy:
> > > 
> > > https://clang.llvm.org/get_involved.html
> > > 
> > > > A specification: The specification must be sufficient to understand the design of the feature as well as interpret the meaning of specific examples. The specification should be detailed enough that another compiler vendor could implement the feature.
> > > 
> > > I am just emphasizing this item here.
> > > 
> > > Regarding 'usefulness' I would even suggest adding it as a general guideline for clang but I think this is not very common. So for now I want to make sure we have it in our guidelines at least since we have encountered this.
> > I see, though I'm a bit hesitant to accept the word "useful" without any further explanation. Would adding the following make sense?
> > 
> > ... should provide useful functionality //that cannot be achieved by other means// to the user.
> I think this wording doesn't cover the case with pragmas though... where the behavior was not possible to achieve without them but it was not clear why it was needed to achieve what was achieved with the pragmas.
> 
> I find //**useful**// pretty clear here even though it is not precisely defined. I would not be able to define it precisely at this point because I don't even know all the possible ways the pragmas can be misused. I think we might have to accept the fact that human language is not as precise as a programming language. As a matter of fact, we are not publishing an algorithm but only the guidelines. Although of course, we don't want to misguide, this might not always be avoidable. For the same reasons Clang contribution policy has never defined what is a sufficiently precise documentation format for the new features. This will be left for the community to assess on a case by case basis.
> this wording doesn't cover the case with pragmas though... where the behavior was not possible to achieve without them but it was not clear why it was needed to achieve what was achieved with the pragmas.

Yes, that's a good point.

I'm still not sure about specifying "useful" without any further clarification then, but if others are happy to accept this then that's fine by me.


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

https://reviews.llvm.org/D97072