If we make CommentOptions a member of LanguageOptions, most of this patch
vanishes.
================
Comment at: include/clang/AST/ASTContext.h:442
@@ -436,1 +441,3 @@
+ const CommentOptions& getCommentOpts() const { return CommentOpts; }
+
----------------
The space should be before the ampersand.
================
Comment at: lib/AST/CommentCommandTraits.cpp:21
@@ -19,2 +20,3 @@
NextID(llvm::array_lengthof(Commands)), Allocator(Allocator)
-{ }
+{
+ registerBlockCommands(CommentOptions.BlockCommandNames);
----------------
The brace should be on the previous line.
================
Comment at: lib/AST/CommentCommandTraits.cpp:37
@@ -33,3 +36,3 @@
-const CommandInfo *CommandTraits::registerUnknownCommand(StringRef
CommandName) {
+CommandInfo *CommandTraits::CreateCommandInfoWithName(StringRef CommandName) {
char *Name = Allocator.Allocate<char>(CommandName.size() + 1);
----------------
According to coding guidelines, function name should start with a lowercase
letter.
================
Comment at: include/clang/Basic/CommentOptions.h:25
@@ +24,3 @@
+struct CommentOptions {
+ typedef llvm::SmallVector<llvm::StringRef, 4> BlockCommandNamesTy;
+
----------------
'4' can be safely changed to '1', since most of the time there will be no
commands here.
Or even use a std::vector since otherwise it is not clear who owns memory for
StringRefs (when CommentOptions is created, ASTContext is not yet created, so
we can not allocate memory there).
================
Comment at: include/clang/Basic/CommentOptions.h:27
@@ +26,3 @@
+
+ /// \brief Strings to treat as names of block commands in comments.
+ BlockCommandNamesTy BlockCommandNames;
----------------
This comment might suggest that we should put a complete command here,
including the backslash.
A better one: command names to treat as ...
================
Comment at: lib/AST/CommentCommandTraits.cpp:60
@@ +59,3 @@
+void CommandTraits::registerBlockCommands(
+ const CommentOptions::BlockCommandNamesTy &BlockCommandNames) {
+ for (CommentOptions::BlockCommandNamesTy::const_iterator I =
----------------
ArrayRef, please.
Actually, I think that a cleaner interface would be to register only one
command. There is only one caller of this that needs this 'register in bulk'
functionality, and we can move the loop there.
================
Comment at: lib/Frontend/CompilerInvocation.cpp:306-308
@@ +305,5 @@
+ Args.getAllArgValues(OPT_fcomment_block_commands);
+ std::copy(
+ BlockCommandNames.begin(), BlockCommandNames.end(),
+ std::back_inserter(Opts.BlockCommandNames));
+}
----------------
This saves StringRefs to std::strings that are going to be destroyed on the
next line, doesn't it?
================
Comment at: lib/AST/ASTContext.cpp:605-606
@@ -604,2 +604,4 @@
-ASTContext::ASTContext(LangOptions& LOpts, SourceManager &SM,
+ASTContext::ASTContext(LangOptions& LOpts,
+ CommentOptions& COpts,
+ SourceManager &SM,
----------------
Space should be before the ampersand.
================
Comment at: unittests/AST/CommentLexer.cpp:457
@@ +456,3 @@
+// Custom registered block command.
+TEST_F(CommentLexerTest, DoxygenCommentRegisterBlockCommand) {
+ const char *Source = "/// \\NewBlockCommand Aaa.\n";
----------------
Since this is not testing Doxygen-specific features,
s/DoxygenCommentRegisterBlockCommand/RegisterCustomBlockCommand/
The comment can be deleted then. (It is always better to have explanations in
the names in code than in comments)
================
Comment at: unittests/AST/CommentLexer.cpp:484
@@ +483,3 @@
+// Multiple custom registered block commands.
+TEST_F(CommentLexerTest, DoxygenCommentRegisterMultipleBlockCommands) {
+ const char *Source =
----------------
s/DoxygenCommentRegisterMultipleBlockCommands/RegisterMultipleCustomBlockCommands/
This test becomes redundant if we change CommentTraits to register only a
single command. Or maybe not, I don't mind if you leave it. This has an
interesting case of two custom commands following each other. This is not
different from builtin commands at this moment, but might change in future.
http://llvm-reviews.chandlerc.com/D272
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
