jklaehn created this revision.

In `ASTUnit::LoadFromASTFile`, the preprocesor object is set up using
default-constructed `LangOptions` (which only later get populated).
Then, in the constructor of `IdentifierTable`, these default-constructed
`LangOptions` were used in the call to `AddKeywords`, leading to wrong
initialization of the identifier table.

This change defers adding the keywords to the identifier table until
after the language options have been loaded from the AST file.

Prior to this change the included test would fail due to the `class`
token being reported as an identifier (since the C++11 `enum class`
construct is not present when using the default language options).


https://reviews.llvm.org/D35181

Files:
  include/clang/Basic/IdentifierTable.h
  include/clang/Lex/Preprocessor.h
  lib/Basic/IdentifierTable.cpp
  lib/Frontend/ASTUnit.cpp
  lib/Lex/Preprocessor.cpp
  unittests/libclang/LibclangTest.cpp

Index: unittests/libclang/LibclangTest.cpp
===================================================================
--- unittests/libclang/LibclangTest.cpp
+++ unittests/libclang/LibclangTest.cpp
@@ -572,3 +572,67 @@
   EXPECT_EQ(0U, clang_getNumDiagnostics(ClangTU));
   DisplayDiagnostics();
 }
+
+class LibclangSerializationTest : public LibclangParseTest {
+public:
+  bool SaveAndLoadTU(const std::string &Filename) {
+    unsigned options = clang_defaultSaveOptions(ClangTU);
+    if (clang_saveTranslationUnit(ClangTU, Filename.c_str(), options) !=
+        CXSaveError_None) {
+      DEBUG(llvm::dbgs() << "Saving failed\n");
+      return false;
+    }
+
+    clang_disposeTranslationUnit(ClangTU);
+
+    ClangTU = clang_createTranslationUnit(Index, Filename.c_str());
+
+    if (!ClangTU) {
+      DEBUG(llvm::dbgs() << "Loading failed\n");
+      return false;
+    }
+
+    return true;
+  }
+};
+
+TEST_F(LibclangSerializationTest, TokenKindsAreCorrectAfterLoading) {
+  // Ensure that "class" is recognized as a keyword token after serializing
+  // and reloading the AST, as it is not a keyword for the default LangOptions.
+  std::string HeaderName = "test.h";
+  WriteFile(HeaderName, "enum class Something {};");
+
+  const char *Argv[] = {"-xc++-header", "-std=c++11"};
+
+  ClangTU = clang_parseTranslationUnit(Index, HeaderName.c_str(), Argv,
+                                       sizeof(Argv) / sizeof(Argv[0]), nullptr,
+                                       0, TUFlags);
+
+  auto CheckTokenKinds = [=]() {
+    CXSourceRange Range =
+        clang_getCursorExtent(clang_getTranslationUnitCursor(ClangTU));
+
+    CXToken *Tokens;
+    unsigned int NumTokens;
+    clang_tokenize(ClangTU, Range, &Tokens, &NumTokens);
+
+    ASSERT_EQ(6u, NumTokens);
+    EXPECT_EQ(CXToken_Keyword, clang_getTokenKind(Tokens[0]));
+    EXPECT_EQ(CXToken_Keyword, clang_getTokenKind(Tokens[1]));
+    EXPECT_EQ(CXToken_Identifier, clang_getTokenKind(Tokens[2]));
+    EXPECT_EQ(CXToken_Punctuation, clang_getTokenKind(Tokens[3]));
+    EXPECT_EQ(CXToken_Punctuation, clang_getTokenKind(Tokens[4]));
+    EXPECT_EQ(CXToken_Punctuation, clang_getTokenKind(Tokens[5]));
+
+    clang_disposeTokens(ClangTU, Tokens, NumTokens);
+  };
+
+  CheckTokenKinds();
+
+  std::string ASTName = "test.ast";
+  WriteFile(ASTName, "");
+
+  ASSERT_TRUE(SaveAndLoadTU(ASTName));
+
+  CheckTokenKinds();
+}
Index: lib/Lex/Preprocessor.cpp
===================================================================
--- lib/Lex/Preprocessor.cpp
+++ lib/Lex/Preprocessor.cpp
@@ -73,12 +73,14 @@
                            SourceManager &SM, MemoryBufferCache &PCMCache,
                            HeaderSearch &Headers, ModuleLoader &TheModuleLoader,
                            IdentifierInfoLookup *IILookup, bool OwnsHeaders,
-                           TranslationUnitKind TUKind)
+                           TranslationUnitKind TUKind,
+                           bool DeferKeywordAddition)
     : PPOpts(std::move(PPOpts)), Diags(&diags), LangOpts(opts), Target(nullptr),
       AuxTarget(nullptr), FileMgr(Headers.getFileMgr()), SourceMgr(SM),
       PCMCache(PCMCache), ScratchBuf(new ScratchBuffer(SourceMgr)),
       HeaderInfo(Headers), TheModuleLoader(TheModuleLoader),
-      ExternalSource(nullptr), Identifiers(opts, IILookup),
+      ExternalSource(nullptr),
+      Identifiers(opts, IILookup, DeferKeywordAddition),
       PragmaHandlers(new PragmaNamespace(StringRef())),
       IncrementalProcessing(false), TUKind(TUKind), CodeComplete(nullptr),
       CodeCompletionFile(nullptr), CodeCompletionOffset(0),
Index: lib/Frontend/ASTUnit.cpp
===================================================================
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -536,6 +536,10 @@
     // Initialize the preprocessor.
     PP.Initialize(*Target);
 
+    // Populate the identifier table with info about keywords for the current
+    // language.
+    PP.getIdentifierTable().AddKeywords(LangOpt);
+
     if (!Context)
       return;
 
@@ -718,11 +722,13 @@
   HeaderSearch &HeaderInfo = *AST->HeaderInfo;
   unsigned Counter;
 
+  // As the language options have not been loaded yet, adding keywords to the
+  // identifier table is deferred and will be initiated by ASTInfoCollector.
   AST->PP = std::make_shared<Preprocessor>(
       AST->PPOpts, AST->getDiagnostics(), *AST->LangOpts,
       AST->getSourceManager(), *AST->PCMCache, HeaderInfo, AST->ModuleLoader,
-      /*IILookup=*/nullptr,
-      /*OwnsHeaderSearch=*/false);
+      /*IILookup=*/nullptr, /*OwnsHeaderSearch=*/false, TU_Complete,
+      /*DeferKeywordAddition=*/true);
   Preprocessor &PP = *AST->PP;
 
   if (ToLoad >= LoadASTOnly)
Index: lib/Basic/IdentifierTable.cpp
===================================================================
--- lib/Basic/IdentifierTable.cpp
+++ lib/Basic/IdentifierTable.cpp
@@ -73,17 +73,15 @@
 }
 
 IdentifierTable::IdentifierTable(const LangOptions &LangOpts,
-                                 IdentifierInfoLookup* externalLookup)
-  : HashTable(8192), // Start with space for 8K identifiers.
-    ExternalLookup(externalLookup) {
+                                 IdentifierInfoLookup *externalLookup,
+                                 bool DeferKeywordAddition)
+    : HashTable(8192), // Start with space for 8K identifiers.
+      ExternalLookup(externalLookup) {
 
   // Populate the identifier table with info about keywords for the current
   // language.
-  AddKeywords(LangOpts);
-      
-
-  // Add the '_experimental_modules_import' contextual keyword.
-  get("import").setModulesImport(true);
+  if (!DeferKeywordAddition)
+    AddKeywords(LangOpts);
 }
 
 //===----------------------------------------------------------------------===//
@@ -227,6 +225,9 @@
 
   if (LangOpts.DeclSpecKeyword)
     AddKeyword("__declspec", tok::kw___declspec, KEYALL, LangOpts, *this);
+
+  // Add the '_experimental_modules_import' contextual keyword.
+  get("import").setModulesImport(true);
 }
 
 /// \brief Checks if the specified token kind represents a keyword in the
Index: include/clang/Lex/Preprocessor.h
===================================================================
--- include/clang/Lex/Preprocessor.h
+++ include/clang/Lex/Preprocessor.h
@@ -691,7 +691,8 @@
                HeaderSearch &Headers, ModuleLoader &TheModuleLoader,
                IdentifierInfoLookup *IILookup = nullptr,
                bool OwnsHeaderSearch = false,
-               TranslationUnitKind TUKind = TU_Complete);
+               TranslationUnitKind TUKind = TU_Complete,
+               bool DeferKeywordAddition = false);
 
   ~Preprocessor();
 
Index: include/clang/Basic/IdentifierTable.h
===================================================================
--- include/clang/Basic/IdentifierTable.h
+++ include/clang/Basic/IdentifierTable.h
@@ -477,9 +477,11 @@
 
 public:
   /// \brief Create the identifier table, populating it with info about the
-  /// language keywords for the language specified by \p LangOpts.
+  /// language keywords for the language specified by \p LangOpts if
+  /// \p DeferKeywordAddition is not set.
   IdentifierTable(const LangOptions &LangOpts,
-                  IdentifierInfoLookup* externalLookup = nullptr);
+                  IdentifierInfoLookup *externalLookup = nullptr,
+                  bool DeferKeywordAddition = false);
 
   /// \brief Set the external identifier lookup mechanism.
   void setExternalIdentifierLookup(IdentifierInfoLookup *IILookup) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to