r212396 - PlistSupport.h: avoid gcc 'defined but not used' warning

[Alp Toker]

On 08/07/2014 00:05, David Blaikie wrote:
> On Sun, Jul 6, 2014 at 12:59 AM, Alp Toker <alp at nuanti.com> wrote:
>> Author: alp
>> Date: Sun Jul  6 02:59:14 2014
>> New Revision: 212396
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=212396&view=rev
>> Log:
>> PlistSupport.h: avoid gcc 'defined but not used' warning
> Is this warning helpful? Should we just disable it? I assume Clang's
> warning here is smart enough not to warn for things in headers?

I like this GCC warning and think clang's being a bit too clever here 
actually.

>
>> Modified:
>>      cfe/trunk/include/clang/Basic/PlistSupport.h
>>      cfe/trunk/lib/ARCMigrate/PlistReporter.cpp
>>      cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
>>
>> Modified: cfe/trunk/include/clang/Basic/PlistSupport.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/PlistSupport.h?rev=212396&r1=212395&r2=212396&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/Basic/PlistSupport.h (original)
>> +++ cfe/trunk/include/clang/Basic/PlistSupport.h Sun Jul  6 02:59:14 2014
>> @@ -19,12 +19,6 @@ namespace clang {
>>   namespace markup {
>>   typedef llvm::DenseMap<FileID, unsigned> FIDMap;
>>
>> -static const char *PlistHeader =
>> -    "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
>> -    "<!DOCTYPE plist PUBLIC \"-//Apple Computer//DTD PLIST 1.0//EN\" "
>> -    "\"http://www.apple.com/DTDs/PropertyList-1.0.dtd\">\n"
>> -    "<plist version=\"1.0\">\n";
>> -
>>   static inline void AddFID(FIDMap &FIDs, SmallVectorImpl<FileID> &V,
>>                             const SourceManager &SM, SourceLocation L) {
>>     FileID FID = SM.getFileID(SM.getExpansionLoc(L));
>> @@ -49,6 +43,15 @@ static inline raw_ostream &Indent(raw_os
>>     return o;
>>   }
>>
>> +static inline raw_ostream &EmitPlistHeader(raw_ostream &o) {
> I don't think this function needs to be static, does it? If it's just
> "inline" then it can be deduped by the linker, etc. (if it happens to
> not get inlined)

Addressed r212494.
>
> Also, it might be simpler just to have the function return the string
> for use, rather than making it so the string can only be used for
> ostream emission like this.

The encapsulation is intentional. Without conscious effort this is 
moving towards and efficient key-value emission interface that might 
some day build C API dictionaries etc. instead of plist strings.

In the meantime, wrapping ostream in this way encourages escaping which 
the old code often got wrong while attempting to pipe raw strings into 
the output file.

> Or should we just do the obvious fix and
> declare the string in the header and define it in the implementation
> file, as usual?

I'd like to keep this facility purely header-based. It's just a 
collection of standalone functions that used to be copy-and-pasted, and 
not a pattern I'm prepared to formalize just yet.

On the other hand, it's currently mislayered for modules and needs 
moving away out of include/Basic. I'm tracking this.

Alp.


>
>> +  static const char *PlistHeader =
>> +      "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
>> +      "<!DOCTYPE plist PUBLIC \"-//Apple Computer//DTD PLIST 1.0//EN\" "
>> +      "\"http://www.apple.com/DTDs/PropertyList-1.0.dtd\">\n"
>> +      "<plist version=\"1.0\">\n";
>> +  return o << PlistHeader;
>> +}
>> +
>>   static inline raw_ostream &EmitInteger(raw_ostream &o, int64_t value) {
>>     o << "<integer>";
>>     o << value;
>>
>> Modified: cfe/trunk/lib/ARCMigrate/PlistReporter.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ARCMigrate/PlistReporter.cpp?rev=212396&r1=212395&r2=212396&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/ARCMigrate/PlistReporter.cpp (original)
>> +++ cfe/trunk/lib/ARCMigrate/PlistReporter.cpp Sun Jul  6 02:59:14 2014
>> @@ -63,8 +63,7 @@ void arcmt::writeARCDiagsToPlist(const s
>>       return;
>>     }
>>
>> -  // Write the plist header.
>> -  o << PlistHeader;
>> +  EmitPlistHeader(o);
>>
>>     // Write the root object: a <dict> containing...
>>     //  - "files", an <array> mapping from FIDs to file names
>>
>> Modified: cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp?rev=212396&r1=212395&r2=212396&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp (original)
>> +++ cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp Sun Jul  6 02:59:14 2014
>> @@ -345,8 +345,7 @@ void PlistDiagnostics::FlushDiagnosticsI
>>       return;
>>     }
>>
>> -  // Write the plist header.
>> -  o << PlistHeader;
>> +  EmitPlistHeader(o);
>>
>>     // Write the root object: a <dict> containing...
>>     //  - "clang_version", the string representation of clang version
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

-- 
http://www.nuanti.com
the browser experts