r206882 - Comment parsing: in the generated XML file, mark HTML that is safe to pass

[Dmitri Gribenko]

On Tue, Apr 22, 2014 at 12:45 PM, Alp Toker <alp at nuanti.com> wrote:
>
> On 22/04/2014 11:59, Dmitri Gribenko wrote:
>>
>> +// The list is intentionally organized as one item per line to make it
>> easier
>> +// to compare with the HTML spec.
>> +foreach AttrName = [
>> +    "onabort",
>> +    "onblur",
>> +    "oncancel",
>> +    "oncanplay",
>> +    "oncanplaythrough",
>> +    "onchange",
>> +    "onclick",
>> +    "onclose",
>> +    "oncuechange",
>> +    "ondblclick",
>> +    "ondrag",
>> +    "ondragend",
>> +    "ondragenter",
>> +    "ondragexit",
>> +    "ondragleave",
>> +    "ondragover",
>> +    "ondragstart",
>> +    "ondrop",
>> +    "ondurationchange",
>> +    "onemptied",
>> +    "onended",
>> +    "onerror",
>> +    "onfocus",
>> +    "oninput",
>> +    "oninvalid",
>> +    "onkeydown",
>> +    "onkeypress",
>> +    "onkeyup",
>> +    "onload",
>> +    "onloadeddata",
>> +    "onloadedmetadata",
>> +    "onloadstart",
>> +    "onmousedown",
>> +    "onmouseenter",
>> +    "onmouseleave",
>> +    "onmousemove",
>> +    "onmouseout",
>> +    "onmouseover",
>> +    "onmouseup",
>> +    "onmousewheel",
>> +    "onpause",
>> +    "onplay",
>> +    "onplaying",
>> +    "onprogress",
>> +    "onratechange",
>> +    "onreset",
>> +    "onresize",
>> +    "onscroll",
>> +    "onseeked",
>> +    "onseeking",
>> +    "onselect",
>> +    "onshow",
>> +    "onstalled",
>> +    "onsubmit",
>> +    "onsuspend",
>> +    "ontimeupdate",
>> +    "ontoggle",
>> +    "onvolumechange",
>> +    "onwaiting"
>> +  ] in {
>> +  def Attr#AttrName : EventHandlerContentAttribute<AttrName>;
>> +}
>
>
> Hi Dmitri,
>
> I've been meaning to bring this up earlier but I really don't think this
> kind of functionality belongs in the core of the clang AST/Basic.
>
> We often ask contributors to add even useful features like warnings and
> checks to clang-tools-extra or third-party plugins where it might keep
> things trim. So it's surprising to me that at the same time there's a
> full-featured HTML5 generator and XML parser/tree being incrementally
> hard-coded into clang/AST.

Hi Alp,

Please note that HTML and XML *generation* is in libIndex, not in
libAST.  Also, no piece of comment parsing (except for the diagnostic
.td file) lives in libBasic.

Comment parsing, including semantic analysis for HTML embedded in
comments, is, indeed, in libAST.  Actually, at first, I put it into
libComment library, because I had the same feeling -- it did not
belong into libAST, but I had to merge it into libAST because of a
dependency cycle: comment parsing depends on libAST in order to
inspect declarations, and libAST (specifically, ASTContext) depends on
libComment in order to parse comments.

If you do feel strongly about this, we could bounce off an abstract
base class that is declared in libAST and the only implementation of
which lives in libComment.  Then libComment would depend on libAST,
and something in libFrontend, which would depend on both libAST and
libComment, would instantiate the comment parser appropriately and
configure ASTContext with the instance.  But I don't think that it is
a significant cleanup of libAST, it introduced even more complexity.
All of lib/AST/Comment* is just 3.5 KLoC, include/clang/AST/Comment*
(including all supporting tables) is less than 3 KLoC.  Compared to
libAST as a whole, with over 100 KLoC, I don't think it adds much
value to factor this small piece into a separate library.

Dmitri

-- 
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/