[cfe-commits] r170467 - in /cfe/trunk: lib/AST/DeclPrinter.cpp test/Index/comment-objc-decls.m test/Index/format-comment-cdecls.c test/Index/overriding-method-comments.mm tools/libclang/CIndex.cpp tools/libclang/CMakeLists.txt tools/libclang/CXComment.cpp tools/libclang/CXTranslationUnit.h tools/libclang/Makefile tools/libclang/SimpleFormatContext.h unittests/AST/DeclPrinterTest.cpp

[Douglas Gregor]

On Dec 18, 2012, at 3:23 PM, Dmitri Gribenko <gribozavr at gmail.com> wrote:

> Hi Fariborz,
> 
> On Wed, Dec 19, 2012 at 1:03 AM, Fariborz Jahanian <fjahanian at apple.com> wrote:
>> Author: fjahanian
>> Date: Tue Dec 18 17:02:59 2012
>> New Revision: 170467
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=170467&view=rev
>> Log:
>> This is the libclang patch providing minimal API to
>> use clang's formatter. Currently, formatter is used
>> to format declaration tags for xml comments. Since formatter
>> is in flux and its change will break several of the clang comment
>> tests, only a single tests is formatted using this facility.
>> Doug has reviewed and approved it for check-in.
>> 
>> 
>> +  ASTContext &Context = FC->getDeclInfo()->CurrentDecl->getASTContext();
>>   CXTranslationUnit TU = CXC.TranslationUnit;
>>   SourceManager &SM = static_cast<ASTUnit *>(TU->TUData)->getSourceManager();
>> +
>> +  SimpleFormatContext *SFC =
>> +    static_cast<SimpleFormatContext*>(TU->FormatContext);
>> +  if (!SFC) {
>> +    SFC = new SimpleFormatContext(Context.getLangOpts());
>> +    TU->FormatContext = SFC;
>> +  }
>> +  else if ((TU->FormatInMemoryUniqueId % 10) == 0) {
> 
> Please put 'else' on the same line as '}'.
> 
>> +    delete SFC;
>> +    SFC = new SimpleFormatContext(Context.getLangOpts());
>> +    TU->FormatContext = SFC;
>> +  }
> 
> I don't understand the reason for doing this 'delete'/'new'.  Could
> you add a comment, please?  Are we holding off freeing lots of memory
> until destruction?

The SourceManager and FileManager in the SimpleFormatContext will grow with each thing we're formatting. To keep it from getting too large with stale data, we're just recreating it every once in a while. Every 10 times is probably excessive, though; it should be more like 1000.

>> +  SimpleFormatContext(LangOptions Options)
>> +      : DiagOpts(new DiagnosticOptions()),
>> +        Diagnostics(new DiagnosticsEngine(new DiagnosticIDs,
>> +                                          DiagOpts.getPtr())),
>> +        Files((FileSystemOptions())),
>> +        Sources(*Diagnostics, Files),
>> +        Rewrite(Sources, Options) {
>> +    Diagnostics->setClient(new IgnoringDiagConsumer, true);
>> +  }
>> +
>> +  ~SimpleFormatContext() { }
>> +
>> +  FileID createInMemoryFile(StringRef Name, StringRef Content) {
>> +    const llvm::MemoryBuffer *Source =
>> +      llvm::MemoryBuffer::getMemBuffer(Content);
>> +    const FileEntry *Entry =
>> +      Files.getVirtualFile(Name, Source->getBufferSize(), 0);
>> +    Sources.overrideFileContents(Entry, Source, true);
>> +    assert(Entry != NULL);
>> +    return Sources.createFileID(Entry, SourceLocation(), SrcMgr::C_User);
>> +  }
>> +
>> +  std::string getRewrittenText(FileID ID) {
>> +    std::string Result;
>> +    llvm::raw_string_ostream OS(Result);
>> +    Rewrite.getEditBuffer(ID).write(OS);
>> +    OS.flush();
>> +    return Result;
>> +  }
>> +
>> +  llvm::IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts;
>> +  llvm::IntrusiveRefCntPtr<DiagnosticsEngine> Diagnostics;
>> +  FileManager Files;
>> +  SourceManager Sources;
> 
> We already have a SourceManager.  Why not reuse it?  (I'm just
> guessing -- maybe creating an in-memory file is permanent for the
> SourceManager?)


Your guess is correct. Creating an in-memory file is permanent for both SourceManager and FileManager, and we don't want them to persist.

	- Doug