I'm working on a patch series to extend clang to parse GLSL, and I
came up with these generic cleanups along the way -- the GLSL patches
depend on the last three, at least, going in first.

(There was a patch #1, but I dropped it and didn't rename the files.)
====
2. refactor -ccc-gcc-name code

Put the logic for deciding the default name for gcc/g++
in the only place that actually cares about it.

This also pushes an ifdef out of the generic driver code
to a little further down, when the target is actually known.
Hopefully it can be changed into just a runtime check
in the future.
====
3. refactor flags for TokenKinds.def

Make KEYALL a combination of all other flags instead
of its own separate flag. Also rewrite the enum
definitions in hex instead of decimal.
====
4. complete TokenKinds.def documentation

It was missing a description of BOOLSUPPORT.
====
5. generalize number parsing for directives other than #line

This makes way for sharing code with GLSL's #version directive.
====

Okay to commit?
From ccec40295b509000086e08ddb1cb39836fbbb103 Mon Sep 17 00:00:00 2001
From: nobled <[email protected]>
Date: Wed, 6 Apr 2011 21:22:44 +0000
Subject: [PATCH 2/7] refactor -ccc-gcc-name code

Put the logic for deciding the default name for gcc/g++
in the only place that actually cares about it.

This also pushes an ifdef out of the generic driver code
to a little further down, when the target is actually known.
Hopefully it can be changed into just a runtime check
in the future.
---
 include/clang/Driver/Driver.h |    4 ++--
 lib/Driver/Driver.cpp         |   16 +---------------
 lib/Driver/Tools.cpp          |   24 +++++++++++++++++++++++-
 3 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/include/clang/Driver/Driver.h b/include/clang/Driver/Driver.h
index 92497ea..0f8a86b 100644
--- a/include/clang/Driver/Driver.h
+++ b/include/clang/Driver/Driver.h
@@ -127,7 +127,7 @@ public:
   unsigned CCPrintHeaders : 1;
 
 private:
-  /// Name to use when calling the generic gcc.
+  /// Name to use when invoking gcc/g++.
   std::string CCCGenericGCCName;
 
   /// Whether to check that input files exist when constructing compilation
@@ -175,7 +175,7 @@ public:
   /// @name Accessors
   /// @{
 
-  /// Name to use when calling the generic gcc.
+  /// Name to use when invoking gcc/g++.
   const std::string &getCCCGenericGCCName() const { return CCCGenericGCCName; }
 
 
diff --git a/lib/Driver/Driver.cpp b/lib/Driver/Driver.cpp
index 9d02dee..4595e5b 100644
--- a/lib/Driver/Driver.cpp
+++ b/lib/Driver/Driver.cpp
@@ -43,13 +43,6 @@
 
 #include <map>
 
-#ifdef __CYGWIN__
-#include <cygwin/version.h>
-#if defined(CYGWIN_VERSION_DLL_MAJOR) && CYGWIN_VERSION_DLL_MAJOR<1007
-#define IS_CYGWIN15 1
-#endif
-#endif
-
 using namespace clang::driver;
 using namespace clang;
 
@@ -65,7 +58,7 @@ Driver::Driver(llvm::StringRef _ClangExecutable,
     Host(0),
     CCPrintOptionsFilename(0), CCPrintHeadersFilename(0), CCCIsCXX(false),
     CCCIsCPP(false),CCCEcho(false), CCCPrintBindings(false),
-    CCPrintOptions(false), CCPrintHeaders(false), CCCGenericGCCName("gcc"),
+    CCPrintOptions(false), CCPrintHeaders(false), CCCGenericGCCName(""),
     CheckInputsExist(true), CCCUseClang(true), CCCUseClangCXX(true),
     CCCUseClangCPP(true), CCCUsePCH(true), SuppressMissingInputWarning(false) {
   if (IsProduction) {
@@ -236,13 +229,6 @@ Compilation *Driver::BuildCompilation(llvm::ArrayRef<const char *> ArgList) {
   CCCPrintActions = Args->hasArg(options::OPT_ccc_print_phases);
   CCCPrintBindings = Args->hasArg(options::OPT_ccc_print_bindings);
   CCCIsCXX = Args->hasArg(options::OPT_ccc_cxx) || CCCIsCXX;
-  if (CCCIsCXX) {
-#ifdef IS_CYGWIN15
-    CCCGenericGCCName = "g++-4";
-#else
-    CCCGenericGCCName = "g++";
-#endif
-  }
   CCCEcho = Args->hasArg(options::OPT_ccc_echo);
   if (const Arg *A = Args->getLastArg(options::OPT_ccc_gcc_name))
     CCCGenericGCCName = A->getValue(*Args);
diff --git a/lib/Driver/Tools.cpp b/lib/Driver/Tools.cpp
index e722822..09ab4a7 100644
--- a/lib/Driver/Tools.cpp
+++ b/lib/Driver/Tools.cpp
@@ -34,6 +34,13 @@
 #include "InputInfo.h"
 #include "ToolChains.h"
 
+#ifdef __CYGWIN__
+#include <cygwin/version.h>
+#if defined(CYGWIN_VERSION_DLL_MAJOR) && CYGWIN_VERSION_DLL_MAJOR<1007
+#define IS_CYGWIN15 1
+#endif
+#endif
+
 using namespace clang::driver;
 using namespace clang::driver::tools;
 
@@ -2052,7 +2059,22 @@ void gcc::Common::ConstructJob(Compilation &C, const JobAction &JA,
     }
   }
 
-  const char *GCCName = getToolChain().getDriver().getCCCGenericGCCName().c_str();
+  const std::string customGCCName = D.getCCCGenericGCCName();
+  const char *GCCName;
+  if (!customGCCName.empty())
+    GCCName = customGCCName.c_str();
+  else if (D.CCCIsCXX) {
+#ifdef IS_CYGWIN15
+    // FIXME: Detect the version of Cygwin at runtime?
+    if (getToolChain().getTriple().getOS() == llvm::Triple::Cygwin)
+      GCCName = "g++-4";
+    else
+#else
+    GCCName = "g++";
+#endif
+  } else
+    GCCName = "gcc";
+
   const char *Exec =
     Args.MakeArgString(getToolChain().GetProgramPath(GCCName));
   C.addCommand(new Command(JA, *this, Exec, CmdArgs));
-- 
1.7.0.4

From 546b6eb814b2acc75ee1f41c46fdf72f6a89af71 Mon Sep 17 00:00:00 2001
From: nobled <[email protected]>
Date: Thu, 7 Apr 2011 12:09:18 +0000
Subject: [PATCH 3/7] refactor flags for TokenKinds.def

Make KEYALL a combination of all other flags instead
of its own separate flag. Also rewrite the enum
definitions in hex instead of decimal.
---
 lib/Basic/IdentifierTable.cpp |   24 ++++++++++++------------
 1 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/lib/Basic/IdentifierTable.cpp b/lib/Basic/IdentifierTable.cpp
index 7647f07..cfe8d27 100644
--- a/lib/Basic/IdentifierTable.cpp
+++ b/lib/Basic/IdentifierTable.cpp
@@ -81,17 +81,17 @@ IdentifierTable::IdentifierTable(const LangOptions &LangOpts,
 // Constants for TokenKinds.def
 namespace {
   enum {
-    KEYALL = 1,
-    KEYC99 = 2,
-    KEYCXX = 4,
-    KEYCXX0X = 8,
-    KEYGNU = 16,
-    KEYMS = 32,
-    BOOLSUPPORT = 64,
-    KEYALTIVEC = 128,
-    KEYNOCXX = 256,
-    KEYBORLAND = 512,
-    KEYOPENCL = 1024
+    KEYC99 = 0x1,
+    KEYCXX = 0x2,
+    KEYCXX0X = 0x4,
+    KEYGNU = 0x8,
+    KEYMS = 0x10,
+    BOOLSUPPORT = 0x20,
+    KEYALTIVEC = 0x40,
+    KEYNOCXX = 0x80,
+    KEYBORLAND = 0x100,
+    KEYOPENCL = 0x200,
+    KEYALL = 0x3ff
   };
 }
 
@@ -107,7 +107,7 @@ static void AddKeyword(llvm::StringRef Keyword,
                        tok::TokenKind TokenCode, unsigned Flags,
                        const LangOptions &LangOpts, IdentifierTable &Table) {
   unsigned AddResult = 0;
-  if (Flags & KEYALL) AddResult = 2;
+  if (Flags == KEYALL) AddResult = 2;
   else if (LangOpts.CPlusPlus && (Flags & KEYCXX)) AddResult = 2;
   else if (LangOpts.CPlusPlus0x && (Flags & KEYCXX0X)) AddResult = 2;
   else if (LangOpts.C99 && (Flags & KEYC99)) AddResult = 2;
-- 
1.7.0.4

From 64edf75876ea61e879eda4be78e9eb9e07adf1ec Mon Sep 17 00:00:00 2001
From: nobled <[email protected]>
Date: Thu, 7 Apr 2011 12:42:01 +0000
Subject: [PATCH 4/7] complete TokenKinds.def documentation

It was missing a description of BOOLSUPPORT.
---
 include/clang/Basic/TokenKinds.def |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/clang/Basic/TokenKinds.def b/include/clang/Basic/TokenKinds.def
index 4d75f6b..ccc8ac7 100644
--- a/include/clang/Basic/TokenKinds.def
+++ b/include/clang/Basic/TokenKinds.def
@@ -196,6 +196,7 @@ PUNCTUATOR(greatergreatergreater, ">>>")
 //   KEYOPENCL  - This is a keyword in OpenCL
 //   KEYALTIVEC - This is a keyword in AltiVec
 //   KEYBORLAND - This is a keyword if Borland extensions are enabled
+//   BOOLSUPPORT - This is a keyword if 'bool' is a built-in type
 //
 KEYWORD(auto                        , KEYALL)
 KEYWORD(break                       , KEYALL)
-- 
1.7.0.4

From 68ced29efde87fa63c8e8edc3c241c6ad3cbdcc5 Mon Sep 17 00:00:00 2001
From: nobled <[email protected]>
Date: Thu, 7 Apr 2011 18:01:47 +0000
Subject: [PATCH 5/7] generalize number parsing for directives other than #line

This makes way for sharing code with GLSL's #version directive.
---
 include/clang/Basic/DiagnosticLexKinds.td |    8 +++---
 lib/Lex/PPDirectives.cpp                  |   31 ++++++++++++++++------------
 2 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/include/clang/Basic/DiagnosticLexKinds.td b/include/clang/Basic/DiagnosticLexKinds.td
index d1a5309..73d3408 100644
--- a/include/clang/Basic/DiagnosticLexKinds.td
+++ b/include/clang/Basic/DiagnosticLexKinds.td
@@ -299,10 +299,10 @@ def err_pp_line_requires_integer : Error<
   "#line directive requires a positive integer argument">;
 def err_pp_line_invalid_filename : Error<
   "invalid filename for #line directive">;
-def warn_pp_line_decimal : Warning<
-  "#line directive interprets number as decimal, not octal">;
-def err_pp_line_digit_sequence : Error<
-  "#line directive requires a simple digit sequence">;
+def warn_pp_decimal : Warning<
+  "#%0 directive interprets number as decimal, not octal">;
+def err_pp_digit_sequence : Error<
+  "#%0 directive requires a simple digit sequence">;
 def err_pp_linemarker_requires_integer : Error<
   "line marker directive requires a positive integer argument">;
 def err_pp_linemarker_invalid_filename : Error<
diff --git a/lib/Lex/PPDirectives.cpp b/lib/Lex/PPDirectives.cpp
index bfb6641..7f599e7 100644
--- a/lib/Lex/PPDirectives.cpp
+++ b/lib/Lex/PPDirectives.cpp
@@ -154,7 +154,7 @@ void Preprocessor::ReadMacroName(Token &MacroNameTok, char isDefineUndef) {
 void Preprocessor::CheckEndOfDirective(const char *DirType, bool EnableMacros) {
   Token Tmp;
   // Lex unexpanded tokens for most directives: macros might expand to zero
-  // tokens, causing us to miss diagnosing invalid lines.  Some directives (like
+  // tokens, causing us to mis-diagnose invalid lines.  Some directives (like
   // #line) allow empty macros.
   if (EnableMacros)
     Lex(Tmp);
@@ -683,10 +683,10 @@ TryAgain:
   // Okay, we're done parsing the directive.
 }
 
-/// GetLineValue - Convert a numeric token into an unsigned value, emitting
+/// GetDecimalValue - Convert a numeric token into an unsigned value, emitting
 /// Diagnostic DiagID if it is invalid, and returning the value in Val.
-static bool GetLineValue(Token &DigitTok, unsigned &Val,
-                         unsigned DiagID, Preprocessor &PP) {
+static bool GetDecimalValue(Preprocessor &PP, Token &DigitTok, unsigned &Val,
+                            const char *DirType, unsigned DiagID) {
   if (DigitTok.isNot(tok::numeric_constant)) {
     PP.Diag(DigitTok, DiagID);
 
@@ -710,7 +710,7 @@ static bool GetLineValue(Token &DigitTok, unsigned &Val,
   for (unsigned i = 0; i != ActualLength; ++i) {
     if (!isdigit(DigitTokBegin[i])) {
       PP.Diag(PP.AdvanceToTokenCharacter(DigitTok.getLocation(), i),
-              diag::err_pp_line_digit_sequence);
+              diag::err_pp_digit_sequence) << DirType;
       PP.DiscardUntilEndOfDirective();
       return true;
     }
@@ -732,7 +732,7 @@ static bool GetLineValue(Token &DigitTok, unsigned &Val,
   }
 
   if (DigitTokBegin[0] == '0')
-    PP.Diag(DigitTok.getLocation(), diag::warn_pp_line_decimal);
+    PP.Diag(DigitTok.getLocation(), diag::warn_pp_decimal) << DirType;
 
   return false;
 }
@@ -749,7 +749,8 @@ void Preprocessor::HandleLineDirective(Token &Tok) {
 
   // Validate the number and convert it to an unsigned.
   unsigned LineNo;
-  if (GetLineValue(DigitTok, LineNo, diag::err_pp_line_requires_integer,*this))
+  if (GetDecimalValue( *this, DigitTok, LineNo, "line",
+                      diag::err_pp_line_requires_integer))
     return;
 
   // Enforce C99 6.10.4p3: "The digit sequence shall not specify ... a
@@ -805,7 +806,8 @@ static bool ReadLineMarkerFlags(bool &IsFileEntry, bool &IsFileExit,
   Token FlagTok;
   PP.Lex(FlagTok);
   if (FlagTok.is(tok::eod)) return false;
-  if (GetLineValue(FlagTok, FlagVal, diag::err_pp_linemarker_invalid_flag, PP))
+  if (GetDecimalValue(PP, FlagTok, FlagVal, "line",
+                      diag::err_pp_linemarker_invalid_flag))
     return true;
 
   if (FlagVal == 1) {
@@ -813,7 +815,8 @@ static bool ReadLineMarkerFlags(bool &IsFileEntry, bool &IsFileExit,
 
     PP.Lex(FlagTok);
     if (FlagTok.is(tok::eod)) return false;
-    if (GetLineValue(FlagTok, FlagVal, diag::err_pp_linemarker_invalid_flag,PP))
+    if (GetDecimalValue(PP, FlagTok, FlagVal, "line",
+                        diag::err_pp_linemarker_invalid_flag))
       return true;
   } else if (FlagVal == 2) {
     IsFileExit = true;
@@ -839,7 +842,8 @@ static bool ReadLineMarkerFlags(bool &IsFileEntry, bool &IsFileExit,
 
     PP.Lex(FlagTok);
     if (FlagTok.is(tok::eod)) return false;
-    if (GetLineValue(FlagTok, FlagVal, diag::err_pp_linemarker_invalid_flag,PP))
+    if (GetDecimalValue(PP, FlagTok, FlagVal, "line",
+                        diag::err_pp_linemarker_invalid_flag))
       return true;
   }
 
@@ -854,7 +858,8 @@ static bool ReadLineMarkerFlags(bool &IsFileEntry, bool &IsFileExit,
 
   PP.Lex(FlagTok);
   if (FlagTok.is(tok::eod)) return false;
-  if (GetLineValue(FlagTok, FlagVal, diag::err_pp_linemarker_invalid_flag, PP))
+  if (GetDecimalValue(PP, FlagTok, FlagVal, "line",
+                      diag::err_pp_linemarker_invalid_flag))
     return true;
 
   // We must have 4 if there is yet another flag.
@@ -886,8 +891,8 @@ void Preprocessor::HandleDigitDirective(Token &DigitTok) {
   // Validate the number and convert it to an unsigned.  GNU does not have a
   // line # limit other than it fit in 32-bits.
   unsigned LineNo;
-  if (GetLineValue(DigitTok, LineNo, diag::err_pp_linemarker_requires_integer,
-                   *this))
+  if (GetDecimalValue(*this, DigitTok, LineNo, "line",
+                      diag::err_pp_linemarker_requires_integer))
     return;
 
   Token StrTok;
-- 
1.7.0.4

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to