Commit my clang plugin and fix up documentation.

Commit my clang plugin and fix up documentation.

BUG=none
TEST=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=73659

[Elliot Glaysher, 2011-02-02 19:29:11]

Nico: review
Everyone else: FYI.

[James Hawkins, 2011-02-02 19:34:15]

On 2011/02/02 19:29:11, Elliot Glaysher wrote:
> Nico: review
> Everyone else: FYI.

Yaaaaaaay!

[Nico, 2011-02-02 22:21:42]

Can you put this in tools/clang/plugins instead of tools/clang_plugins?

[Elliot Glaysher, 2011-02-02 22:41:46]

On 2011/02/02 22:21:42, Nico wrote:
> Can you put this in tools/clang/plugins instead of tools/clang_plugins?

Done.

[Nico, 2011-02-02 23:13:06]

Looks mostly good

http://codereview.chromium.org/6368055/diff/4001/tools/clang/plugins/ChromeCl...
File tools/clang/plugins/ChromeClassTester.cpp (right):

http://codereview.chromium.org/6368055/diff/4001/tools/clang/plugins/ChromeCl...
tools/clang/plugins/ChromeClassTester.cpp:34: // TODO: Let the user configure
this list of directories.
i think this todo and the one before can just be deleped?

http://codereview.chromium.org/6368055/diff/4001/tools/clang/plugins/ChromeCl...
tools/clang/plugins/ChromeClassTester.cpp:50: // TODO: Confurgre this some other
way.
"Configure"?

http://codereview.chromium.org/6368055/diff/4001/tools/clang/plugins/ChromeCl...
tools/clang/plugins/ChromeClassTester.cpp:91: // now.
TODO? Seems like inlining in test code is what's slow to build (see
gpu_unittests)

http://codereview.chromium.org/6368055/diff/4001/tools/clang/plugins/ChromeCl...
tools/clang/plugins/ChromeClassTester.cpp:169: // command line.
why?

http://codereview.chromium.org/6368055/diff/4001/tools/clang/plugins/ChromeCl...
File tools/clang/plugins/ChromeClassTester.h (right):

http://codereview.chromium.org/6368055/diff/4001/tools/clang/plugins/ChromeCl...
tools/clang/plugins/ChromeClassTester.h:20:
ChromeClassTester(clang::CompilerInstance& instance);
explicit

http://codereview.chromium.org/6368055/diff/4001/tools/clang/plugins/ChromeCl...
tools/clang/plugins/ChromeClassTester.h:30: // Emits a simple warning; this
shouldn't be used if you require pritnf-style
printf

http://codereview.chromium.org/6368055/diff/4001/tools/clang/plugins/ChromeCl...
tools/clang/plugins/ChromeClassTester.h:35: // Template method which is called
with only class that are defined in chrome
classes

"source files"?

http://codereview.chromium.org/6368055/diff/4001/tools/clang/plugins/ChromeCl...
tools/clang/plugins/ChromeClassTester.h:51: clang::Diagnostic& d_;
"d_"?

http://codereview.chromium.org/6368055/diff/4001/tools/clang/plugins/FindBadC...
File tools/clang/plugins/FindBadConstructs.cpp (right):

http://codereview.chromium.org/6368055/diff/4001/tools/clang/plugins/FindBadC...
tools/clang/plugins/FindBadConstructs.cpp:89: // the ctor warning, but if you do
*anything* else, it should. TODO: This
TODO(erg)?

http://codereview.chromium.org/6368055/diff/4001/tools/clang/plugins/FindBadC...
tools/clang/plugins/FindBadConstructs.cpp:93: dtor_score +=
(templated_base_classes * 9);
no parens? also below

http://codereview.chromium.org/6368055/diff/4001/tools/clang/plugins/README.c...
File tools/clang/plugins/README.chromium (right):

http://codereview.chromium.org/6368055/diff/4001/tools/clang/plugins/README.c...
tools/clang/plugins/README.chromium:1: Clang plugins: How to use and how to
write
I'd prefer this to be on the wiki page, and this README just to link to there.

(If you want, I can move it over later.)

http://codereview.chromium.org/6368055/diff/4001/tools/clang/plugins/README.c...
tools/clang/plugins/README.chromium:30: classes, namespaces and directories.
This should have a paragraph on "You usually don't want to write a plugin, only
do this as a last resort because clang's api here is not stable and does break
often in practice, if you write a plugin you're committed to maintaining it, all
error messages you might add should clearly call out they're originating from
your plugin, diagnostics that are useful in general should be upstreamed to
clang instead, if you want to do something else than to add errors, consider
using clang's c api instead yadda yadda yadda"

[Elliot Glaysher, 2011-02-02 23:57:49]

Addressed?

http://codereview.chromium.org/6368055/diff/4001/tools/clang/plugins/ChromeCl...
File tools/clang/plugins/ChromeClassTester.h (right):

http://codereview.chromium.org/6368055/diff/4001/tools/clang/plugins/ChromeCl...
tools/clang/plugins/ChromeClassTester.h:35: // Template method which is called
with only class that are defined in chrome
On 2011/02/02 23:13:06, Nico wrote:
> classes
> 
> "source files"?

Nope, header files. The project of deinlining all the code in cc files is even
worse of a project. (And wouldn't matter much since they're only used in one
file anyway.)

[Nico, 2011-02-02 23:59:35]

LG

[Nico, 2011-02-03 00:12:19]

…can you put the incantations how this is built somewhere? e.g. block
comment at the top of the main cc?

On Wed, Feb 2, 2011 at 3:59 PM,  <thakis@chromium.org> wrote:
> LG
>
> http://codereview.chromium.org/6368055/
>

[tfarina, 2011-02-03 02:47:53]

http://codereview.chromium.org/6368055/diff/6002/tools/clang/plugins/ChromeCl...
File tools/clang/plugins/ChromeClassTester.cpp (right):

http://codereview.chromium.org/6368055/diff/6002/tools/clang/plugins/ChromeCl...
tools/clang/plugins/ChromeClassTester.cpp:129: if (n != "") {
!n.empty() ?

http://codereview.chromium.org/6368055/diff/6002/tools/clang/plugins/ChromeCl...
File tools/clang/plugins/ChromeClassTester.h (right):

http://codereview.chromium.org/6368055/diff/6002/tools/clang/plugins/ChromeCl...
tools/clang/plugins/ChromeClassTester.h:32: void
emitWarning(clang::SourceLocation loc, const char* error);
EmitWarning?

[Elliot Glaysher, 2011-02-03 19:31:52]

On 2011/02/03 00:12:19, Nico wrote:
> …can you put the incantations how this is built somewhere? e.g. block
> comment at the top of the main cc?

There are no incantations other than "use the clang build system". I have no
idea how to build it without it.

[Nico, 2011-02-03 19:46:12]

On Thu, Feb 3, 2011 at 11:31 AM,  <erg@chromium.org> wrote:
> On 2011/02/03 00:12:19, Nico wrote:
>>
>> …can you put the incantations how this is built somewhere? e.g. block
>> comment at the top of the main cc?
>
> There are no incantations other than "use the clang build system". I have no
> idea how to build it without it.

How do you do this? Copy the plugin into a directory somewhere in the
clang tree and then run "make"?

>
> http://codereview.chromium.org/6368055/
>

[Elliot Glaysher, 2011-02-03 20:35:36]

I've added a Makefile with instructions for copying & use.

[Nico, 2011-02-03 20:38:56]

SLG

http://codereview.chromium.org/6368055/diff/22/tools/clang/plugins/Makefile
File tools/clang/plugins/Makefile (right):

http://codereview.chromium.org/6368055/diff/22/tools/clang/plugins/Makefile#n...
tools/clang/plugins/Makefile:3: # into a clang checkout:
Ugh!

Can you add a "TODO(thakis): Improve situation"?

Good enough for now tho, I suppose