[cfe-commits] [PATCH] Support for pragma include_alias

Richard Smith richard at metafoo.co.uk
Wed Feb 29 22:21:51 PST 2012


Hi,

> On Sun, Feb 19, 2012 at 5:31 PM, Aaron Ballman <aaron at aaronballman.com>
> wrote:
> >> One of the MSVC pragmas is the ability to remap include filenames, as
> >> described in MSDN:
> >> http://msdn.microsoft.com/en-us/library/wbeh5h91.aspx
> >>
> >> The long and short of it is: you can use this pragma to map the source
> >> filename to a replacement filename (usually for supporting the old 8.3
> >> filenames on FAT systems, but can be used arbitrarily).  For instance:
> >>
> >> #pragma include_alias( "FoobarIsReallyLong.h", "FOOBAR~1.H" )
> >> #include "FoobarIsReallyLong.h"  // Actually opens up FOOBAR~1.H
> >>
> >> #pragma include_alias( <foo.h>, <bar.h> )
> >> #include <foo.h>  // Actually includes <bar.h>
> >> #include "foo.h"  // Still includes "foo.h"
> >>
> >> This patch implements that pragma, and closes Bug 10705.
>

Sorry for the delay!

> Index: include/clang/Basic/DiagnosticLexKinds.td
> ===================================================================
> --- include/clang/Basic/DiagnosticLexKinds.td (revision 150897)
> +++ include/clang/Basic/DiagnosticLexKinds.td (working copy)
> @@ -292,6 +292,15 @@
>     ExtWarn<"__has_warning expected option name (e.g. \"-Wundef\")">,
>     InGroup<MalformedWarningCheck>;
>
> +def warn_pragma_include_alias_mismatch :
> +   ExtWarn<"pragma include_alias requires matching include directives "
> +   "(e.g include_alias(\"foo.h\", \"bar.h\") or "
> +   "include_alias(<foo.h>, <bar.h>))">,
> +   InGroup<UnknownPragmas>;

Can we make this diagnostic clearer? I'd prefer for us to produce different
text depending on whether the source include was a "include" or a
<include>. As a first pass:

angle-bracketed include <%0> cannot be aliased to double-quoted include "%1"
double-quoted include "%0" cannot be aliased to angle-bracketed include <%1>

> +def warn_pragma_include_alias_expected :
> +   ExtWarn<"pragma include_alias expected '%0'">,
> +   InGroup<UnknownPragmas>;
> +
>  def err__Pragma_malformed : Error<
>    "_Pragma takes a parenthesized string literal">;
>  def err_pragma_comment_malformed : Error<
> Index: include/clang/Lex/HeaderSearch.h
> ===================================================================
> --- include/clang/Lex/HeaderSearch.h (revision 150897)
> +++ include/clang/Lex/HeaderSearch.h (working copy)
> @@ -154,6 +154,9 @@
>    llvm::StringMap<const DirectoryEntry *, llvm::BumpPtrAllocator>
>      FrameworkMap;
>
> +  llvm::StringMap<std::pair<StringRef, bool>, llvm::BumpPtrAllocator>
> +    IncludeAliasMap;

This map doesn't appear to be sufficient: the Microsoft documentation
indicates that "foo" and <foo> can be aliased to different includes.

Also, the StringRef in the value type here will refer to a string whose
lifetime has ended.

> +
>    /// HeaderMaps - This is a mapping from FileEntry -> HeaderMap,
uniquing
>    /// headermaps.  This vector owns the headermap.
>    std::vector<std::pair<const FileEntry*, const HeaderMap*> > HeaderMaps;
> @@ -217,6 +220,25 @@
>      SystemDirIdx++;
>    }
>
> +  /// AddHeaderMapping -- Map the source include name to the dest
include name
> +  void AddHeaderMapping(const StringRef& Source, const StringRef& Dest,

Generally, please write " &" not "& ". Also, StringRefs are usually passed
by value.

> +                        bool IsAngled) {
> +    IncludeAliasMap[Source] = std::make_pair(Dest, IsAngled);
> +  }
> +
> +  StringRef MapHeader(const StringRef& Source, bool isAngled) {
> +    // Do any filename replacements before anything else
> +    llvm::StringMap<std::pair<StringRef,bool>>::const_iterator iter =

Please capitalize IsAngled and Iter, and write "> >" not ">>". I don't like
repeating this type here (and especially not missing out the
llvm::BumpPtrAllocator argument); consider adding a typedef for your
container type.

> +      IncludeAliasMap.find(Source);
> +    if (iter != IncludeAliasMap.end()) {
> +      // If the angling matches, then we've found a replacement
> +      if (iter->second.second == isAngled) {
> +        return iter->second.first;
> +      }

Prevailing llvm/clang style is to omit the braces around a simple
single-statement body such as this.

> +    }
> +    return Source;
> +  }
> +
>    /// \brief Set the path to the module cache.
>    void setModuleCachePath(StringRef CachePath) {
>      ModuleCachePath = CachePath;
> Index: include/clang/Lex/Preprocessor.h
> ===================================================================
> --- include/clang/Lex/Preprocessor.h (revision 150897)
> +++ include/clang/Lex/Preprocessor.h (working copy)
> @@ -1262,6 +1262,7 @@
>    void HandlePragmaMessage(Token &MessageTok);
>    void HandlePragmaPushMacro(Token &Tok);
>    void HandlePragmaPopMacro(Token &Tok);
> +  void HandlePragmaIncludeAlias(Token &Tok);
>    IdentifierInfo *ParsePragmaPushOrPopMacro(Token &Tok);
>
>    // Return true and store the first token only if any CommentHandler
> Index: lib/Lex/PPDirectives.cpp
> ===================================================================
> --- lib/Lex/PPDirectives.cpp (revision 150897)
> +++ lib/Lex/PPDirectives.cpp (working copy)
> @@ -1297,6 +1297,9 @@
>      PragmaARCCFCodeAuditedLoc = SourceLocation();
>    }
>
> +  // Map the filename
> +  Filename = HeaderInfo.MapHeader(Filename, isAngled);
> +
>    // Search include directories.
>    const DirectoryLookup *CurDir;
>    SmallString<1024> SearchPath;
> Index: lib/Lex/Pragma.cpp
> ===================================================================
> --- lib/Lex/Pragma.cpp (revision 150897)
> +++ lib/Lex/Pragma.cpp (working copy)
> @@ -663,6 +663,101 @@
>    }
>  }
>
> +void Preprocessor::HandlePragmaIncludeAlias(Token& Tok) {
> +  // We will either get a quoted filename or a bracketed filename, and
we
> +  // have to track which we got.  The first filename is the source name,
> +  // and the second name is the mapped filename.  If the first is quoted,
> +  // the second must be as well (cannot mix and match quotes and
brackets).
> +  SourceLocation Loc = Tok.getLocation();
> +
> +  // Get the open paren
> +  Lex(Tok);
> +  if (Tok.isNot(tok::l_paren)) {
> +    Diag(Tok, diag::warn_pragma_include_alias_expected) << "(";
> +    return;
> +  }
> +
> +  // We expect either a quoted string literal, or a bracketed name
> +  Token SourceFilenameTok;
> +  CurPPLexer->LexIncludeFilename(SourceFilenameTok);
> +  if (SourceFilenameTok.is(tok::eod)) {
> +    // The diagnostic has already been handled
> +    return;
> +  }
> +
> +  StringRef SourceFileName;
> +  SmallString<128> FileNameBuffer;
> +  if (SourceFilenameTok.is(tok::string_literal) ||
> +      SourceFilenameTok.is(tok::angle_string_literal)) {
> +    SourceFileName = getSpelling(SourceFilenameTok, FileNameBuffer);
> +  } else if (SourceFilenameTok.is(tok::less)) {
> +    // This could be a path instead of just a name
> +    FileNameBuffer.push_back('<');
> +    SourceLocation End;
> +    if (ConcatenateIncludeName(FileNameBuffer, End))
> +      return; // Diagnostic already emitted
> +    SourceFileName = FileNameBuffer.str();
> +  } else {
> +    Diag(Tok, diag::warn_pragma_include_alias_expected) << "include
filename";

English phrases shouldn't be used as diagnostic arguments, since they won't
be translatable (if we ever choose to translate the diagnostics). Please
use a different diagnostic for this; perhaps diag::err_pp_expects_filename?

Also, I don't see a test for this error.

> +    return;
> +  }
> +  FileNameBuffer.clear();
> +
> +  // Now we expect a comma, followed by another include name
> +  Lex(Tok);
> +  if (Tok.isNot(tok::comma)) {
> +    Diag(Tok, diag::warn_pragma_include_alias_expected) << ",";
> +    return;
> +  }
> +
> +  Token ReplaceFilenameTok;
> +  CurPPLexer->LexIncludeFilename(ReplaceFilenameTok);
> +  if (ReplaceFilenameTok.is(tok::eod)) {
> +    // The diagnostic has already been handled
> +    return;
> +  }
> +
> +  StringRef ReplaceFileName;
> +  if (ReplaceFilenameTok.is(tok::string_literal) ||
> +      ReplaceFilenameTok.is(tok::angle_string_literal)) {
> +    ReplaceFileName = getSpelling(ReplaceFilenameTok, FileNameBuffer);
> +  } else if (ReplaceFilenameTok.is(tok::less)) {
> +    // This could be a path instead of just a name
> +    FileNameBuffer.push_back('<');
> +    SourceLocation End;
> +    if (ConcatenateIncludeName(FileNameBuffer, End))
> +      return; // Diagnostic already emitted
> +    ReplaceFileName = FileNameBuffer.str();
> +  } else {
> +    Diag(Tok, diag::warn_pragma_include_alias_expected) << "include
filename";
> +    return;
> +  }

Could this repeated code be factored out? (I imagine we have largely the
same pattern in some other places too.)

> +
> +  // Finally, we expect the closing paren
> +  Lex(Tok);
> +  if (Tok.isNot(tok::r_paren)) {
> +    Diag(Tok, diag::warn_pragma_include_alias_expected) << ")";
> +    return;
> +  }
> +
> +  // Now that we have the source and target filenames, we need to make
sure
> +  // they're both of the same type (angled vs non-angled)
> +  bool SourceIsAngled =
> +    GetIncludeFilenameSpelling(SourceFilenameTok.getLocation(),
> +                                SourceFileName);
> +  bool ReplaceIsAngled =
> +    GetIncludeFilenameSpelling(ReplaceFilenameTok.getLocation(),
> +                                ReplaceFileName);
> +  if (SourceIsAngled != ReplaceIsAngled) {
> +    Diag(Loc, diag::warn_pragma_include_alias_mismatch);
> +    return;
> +  }
> +
> +  // Now we can let the include handler know about this mapping
> +  getHeaderSearchInfo().AddHeaderMapping(SourceFileName,
ReplaceFileName,

ReplaceFileName here refers to a string which expires at the end of this
function, and it's being saved away by AddHeaderMapping for later use.

> +                                          SourceIsAngled);
> +}
> +
>  /// AddPragmaHandler - Add the specified pragma handler to the
preprocessor.
>  /// If 'Namespace' is non-null, then it is a token required to exist on
the
>  /// pragma line before the pragma string starts, e.g. "STDC" or "GCC".
> @@ -943,6 +1038,15 @@
>    }
>  };
>
> +/// PragmaIncludeAliasHandler - "#pragma include_alias("...")".
> +struct PragmaIncludeAliasHandler : public PragmaHandler {
> +  PragmaIncludeAliasHandler() : PragmaHandler("include_alias") {}
> +  virtual void HandlePragma(Preprocessor &PP, PragmaIntroducerKind
Introducer,
> +                            Token &IncludeAliasTok) {
> +      PP.HandlePragmaIncludeAlias(IncludeAliasTok);
> +  }
> +};
> +
>  /// PragmaMessageHandler - "#pragma message("...")".
>  struct PragmaMessageHandler : public PragmaHandler {
>    PragmaMessageHandler() : PragmaHandler("message") {}
> @@ -1095,5 +1199,6 @@
>    // MS extensions.
>    if (Features.MicrosoftExt) {
>      AddPragmaHandler(new PragmaCommentHandler());
> +    AddPragmaHandler(new PragmaIncludeAliasHandler());
>    }
>  }
> Index: test/Preprocessor/pragma_microsoft.c
> ===================================================================
> --- test/Preprocessor/pragma_microsoft.c (revision 150897)
> +++ test/Preprocessor/pragma_microsoft.c (working copy)
> @@ -38,3 +38,20 @@
>    // this warning should go away.
>    MACRO_WITH__PRAGMA // expected-warning {{expression result unused}}
>  }
> +
> +
> +// This should include macro_arg_directive even though the include
> +// is looking for test.h  This allows us to assign to "n"
> +#pragma include_alias("test.h", "macro_arg_directive.h" )
> +#include "test.h"
> +void test( void ) {
> +  n = 12;
> +}
> +
> +#pragma include_alias("foo.h", <bar.h>) // expected-warning {{pragma
include_alias requires matching include directives (e.g
include_alias("foo.h", "bar.h") or include_alias(<foo.h>, <bar.h>))}}
> +#pragma include_alias("test.h") // expected-warning {{pragma
include_alias expected ','}}
> +
> +// Make sure that the names match exactly for a replacement, including
path information.  If
> +// this were to fail, we would get a file not found error
> +#pragma include_alias(".\pp-record.h", "does_not_exist.h")
> +#include "pp-record.h"
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120229/0bd57a89/attachment.html>


More information about the cfe-commits mailing list