[cfe-commits] r158771 - in /cfe/trunk: include/clang-c/ include/clang/AST/ include/clang/Basic/ include/clang/Comments/ include/clang/Lex/ include/clang/Parse/ include/clang/Sema/ include/clang/Serialization/ lib/ lib/ARCMigrate/ lib/AST/ lib/Com

[Chandler Carruth]

On Tue, Jun 19, 2012 at 5:34 PM, Dmitri Gribenko <gribozavr at gmail.com>wrote:

> Author: gribozavr
> Date: Tue Jun 19 19:34:58 2012
> New Revision: 158771
>
> URL: http://llvm.org/viewvc/llvm-project?rev=158771&view=rev
> Log:
> Structured comment parsing, first step.
>

I'm pretty excited to see this coming along, but...


>
> * Retain comments in the AST
> * Serialize/deserialize comments
> * Find comments attached to a certain Decl
> * Expose raw comment text and SourceRange via libclang
>
> Added:
>    cfe/trunk/include/clang/Comments/
>    cfe/trunk/include/clang/Comments/RawCommentList.h
>    cfe/trunk/lib/Comments/
>    cfe/trunk/lib/Comments/CMakeLists.txt
>    cfe/trunk/lib/Comments/Makefile
>    cfe/trunk/lib/Comments/RawCommentList.cpp
>

Why are we introducing an entire new library just for one class with one
source file?

This doesn't work at all -- this library depends on the AST library, and
yet the AST library uses classes defined in this library. It's a bad
layering violation, and it's not at all clear to me why it is needed.

I could see wanting the RawCommentList to be a library *below* the AST
library, and if so I would expect it to be part of the Lex library with the
preprocessor or the Basic library where the SourceManager itself is... I'm
not really enthusiastic about growing an entire separate library just for
this construct.


Anyways, the layering violation and circular dependency is a serious
problem and was breaking some builds so I've folded this code into the AST
library. You can sink it or move it around based on this discussion.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120620/dfdd455e/attachment.html>