SourceLoc use in command line processing (#1848)

* #include an absolute path didn't work - because paths were taken to always be relative.

* Added SourceLoc handling for command line parsing.

* Fix typo in debug.

* Fix issue around the DiagnosticSink used in options parsing not having a writer available - by having DiagnosticSink parenting.

* Small rename for clarity.

Co-authored-by: T. Foley <tfoleyNV@users.noreply.github.com>

6 files changed, 344 insertions, 36 deletions

diff --git a/source/compiler-core/slang-command-line-args.cpp b/source/compiler-core/slang-command-line-args.cpp
new file mode 100644
index 000000000..da262e9bb
--- /dev/null
+++ b/source/compiler-core/slang-command-line-args.cpp
@@ -0,0 +1,95 @@
+#include "slang-command-line-args.h"
+
+#include "../core/slang-process-util.h"
+#include "../core/slang-string-escape-util.h"
+
+#include "slang-core-diagnostics.h"
+
+namespace Slang {
+
+void CommandLineArgs::setArgs(const char*const* args, size_t argCount)
+{
+    m_args.clear();
+
+    const SourceLoc startLoc = m_sourceManager->getNextRangeStart();
+
+    StringBuilder buf;
+   
+    auto escapeHandler = ProcessUtil::getEscapeHandler();
+
+    for (size_t i = 0; i < argCount; ++i)
+    {
+        const Index offset = buf.getLength();
+
+        const char* srcArg = args[i];
+
+        Arg dstArg;
+        dstArg.loc = startLoc + offset;
+        dstArg.value = srcArg;
+
+        m_args.add(dstArg);
+
+        // Write the string escaped if necessary
+        StringEscapeUtil::appendMaybeQuoted(escapeHandler, dstArg.value.getUnownedSlice(), buf);
+
+        // Put a space between the args
+        buf << " ";
+    }
+
+    SourceFile* sourceFile = m_sourceManager->createSourceFileWithString(PathInfo::makeUnknown(), buf.ProduceString());
+    m_sourceView = m_sourceManager->createSourceView(sourceFile, nullptr, SourceLoc::fromRaw(0));
+
+    SLANG_ASSERT(m_sourceView->getRange().begin == startLoc);
+}
+
+/* !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
+
+                         CommandLineReader
+
+!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! */
+
+String CommandLineReader::getPreviousValue() const
+{
+    SLANG_ASSERT(m_index > 0);
+    if (m_index > 0)
+    {
+        const auto& prevArg = (*m_args)[m_index - 1];
+        return prevArg.value;
+    }
+    else
+    {
+        return String();
+    }
+}
+
+SlangResult CommandLineReader::expectArg(String& outArg)
+{
+    if (hasArg())
+    {
+        outArg = m_args->m_args[m_index++].value;
+        return SLANG_OK;
+    }
+    else
+    {
+        m_sink->diagnose(peekLoc(), MiscDiagnostics::expectedArgumentForOption, getPreviousValue());
+        return SLANG_FAIL;
+    }
+}
+
+SlangResult CommandLineReader::expectArg(CommandLineArg& outArg)
+{
+    if (hasArg())
+    {
+        outArg = peekArg();
+        advance();
+        return SLANG_OK;
+    }
+    else
+    {
+        m_sink->diagnose(peekLoc(), MiscDiagnostics::expectedArgumentForOption, getPreviousValue());
+        return SLANG_FAIL;
+    }
+}
+
+
+} // namespace Slang
diff --git a/source/compiler-core/slang-command-line-args.h b/source/compiler-core/slang-command-line-args.h
new file mode 100644
index 000000000..6dea7408c
--- /dev/null
+++ b/source/compiler-core/slang-command-line-args.h
@@ -0,0 +1,95 @@
+#ifndef SLANG_COMMAND_LINE_ARGS_H
+#define SLANG_COMMAND_LINE_ARGS_H
+
+// This file defines the `Name` type, used to represent
+// the name of types, variables, etc. in the AST.
+
+#include "../core/slang-basic.h"
+
+#include "slang-source-loc.h"
+#include "slang-diagnostic-sink.h"
+
+namespace Slang {
+
+struct CommandLineArg
+{
+    String value;           ///< The value of the arg
+    SourceLoc loc;          ///< The location of the arg
+};
+
+struct CommandLineArgs
+{
+    typedef CommandLineArg Arg;
+
+    SLANG_FORCE_INLINE Index getArgCount() const { return m_args.getCount(); }
+    const Arg& operator[](Index i) const { return m_args[i]; }
+
+    const Arg* begin() const { return m_args.begin(); }
+    const Arg* end() const { return m_args.end(); }
+
+        /// NOTE! Should NOT include the executable name
+    void setArgs(const char*const* args, size_t argCount);
+
+        /// Ctor with a source manager
+    CommandLineArgs(SourceManager* manager):
+        m_sourceManager(manager),
+        m_sourceView(nullptr)
+    {
+    }
+
+    String m_executablePath;                ///< Can be optionally be set
+
+    List<Arg> m_args;                       ///< The args
+    SourceManager* m_sourceManager;         ///< The source manager and associated diagnostics sink
+    SourceView* m_sourceView;        ///< contains the command line as source
+};
+
+struct CommandLineReader
+{
+        /// Peek the current location
+    SourceLoc peekLoc() const { return m_index < m_args->getArgCount() ? (*m_args)[m_index].loc : SourceLoc(); }
+        /// Peek the current arg
+    const CommandLineArg& peekArg() const { SLANG_ASSERT(hasArg()); return (*m_args)[m_index]; }
+
+        /// Peek the string value at that position
+    const String& peekValue() const { SLANG_ASSERT(hasArg()); return (*m_args)[m_index].value; }
+
+        /// Get the arg and advance
+    CommandLineArg getArgAndAdvance() { CommandLineArg arg(peekArg()); advance(); return arg; }
+
+    const String& getValueAndAdvance() { const String& value = peekValue(); advance(); return value; }
+
+        /// True if at end
+    bool atEnd() const { return m_index >= m_args->getArgCount(); }
+        /// True if has a current arg
+    bool hasArg() const { return !atEnd(); }
+
+        /// Advance to next arg
+    void advance() { SLANG_ASSERT(m_index < m_args->getArgCount()); m_index++; }
+        /// Removes arg at current position
+    void removeArg() { SLANG_ASSERT(hasArg()); m_args->m_args.removeAt(m_index); }
+
+        /// Get the value from the arg previous to the current position. Will assert if there isn't one.
+    String getPreviousValue() const; 
+
+        /// If there is an arg outArg is set and advanced
+        /// Note, this *assumes* the previous arg is the option that initated this
+    SlangResult expectArg(String& outArg);
+    SlangResult expectArg(CommandLineArg& outArg);
+
+        /// Set up reader with args
+    CommandLineReader(CommandLineArgs* args, DiagnosticSink* sink):
+        m_args(args),
+        m_index(0),
+        m_sink(sink)
+    {
+    }
+
+    DiagnosticSink* m_sink;
+    CommandLineArgs* m_args;
+    Index m_index;
+};
+
+} // namespace Slang
+
+#endif
diff --git a/source/compiler-core/slang-diagnostic-sink.cpp b/source/compiler-core/slang-diagnostic-sink.cpp
index 727c322a5..0ad16b2b4 100644
--- a/source/compiler-core/slang-diagnostic-sink.cpp
+++ b/source/compiler-core/slang-diagnostic-sink.cpp
@@ -131,12 +131,15 @@ static void formatDiagnosticMessage(StringBuilder& sb, char const* format, int a
     }
 }
 
-static void formatDiagnostic(const HumaneSourceLoc& humaneLoc, Diagnostic const& diagnostic, StringBuilder& outBuilder)
+static void formatDiagnostic(const HumaneSourceLoc& humaneLoc, Diagnostic const& diagnostic, DiagnosticSink::Flags flags, StringBuilder& outBuilder)
 {
-    outBuilder << humaneLoc.pathInfo.foundPath;
-    outBuilder << "(";
-    outBuilder << Int32(humaneLoc.line);
-    outBuilder << "): ";
+    if (flags & DiagnosticSink::Flag::HumaneLoc)
+    {
+        outBuilder << humaneLoc.pathInfo.foundPath;
+        outBuilder << "(";
+        outBuilder << Int32(humaneLoc.line);
+        outBuilder << "): ";
+    }
 
     outBuilder << getSeverityName(diagnostic.severity);
 
@@ -236,7 +239,15 @@ static UnownedStringSlice _extractLineContainingPosition(const UnownedStringSlic
     return UnownedStringSlice(start, end);
 }
 
-static void _sourceLocationNoteDiagnostic(SourceView* sourceView, SourceLoc sourceLoc, DiagnosticSink::SourceLocationLexer lexer, StringBuilder& sb)
+static void _reduceLength(Index startIndex, StringBuilder& ioBuf)
+{
+    StringBuilder buf;
+    buf << "...";
+    buf.append(ioBuf.getUnownedSlice().tail(startIndex));
+    ioBuf = buf;
+}
+
+static void _sourceLocationNoteDiagnostic(DiagnosticSink* sink, SourceView* sourceView, SourceLoc sourceLoc, StringBuilder& sb)
 {
     SourceFile* sourceFile = sourceView->getSourceFile();
     if (!sourceFile)
@@ -266,7 +277,7 @@ static void _sourceLocationNoteDiagnostic(SourceView* sourceView, SourceLoc sour
     // TODO(JS): The tab size should ideally be configurable from command line.
     // For now just go with 4.
     const Index tabSize = 4;
-    
+
     StringBuilder sourceLine;
     StringBuilder caretLine;
 
@@ -282,10 +293,13 @@ static void _sourceLocationNoteDiagnostic(SourceView* sourceView, SourceLoc sour
         const Index length = caretLine.getLength();
         caretLine.Clear();
         caretLine.appendRepeatedChar(' ', length);
-        
+
+        Index caretIndex = caretLine.getLength();
+
         // Add caret
         caretLine << "^";
 
+        auto lexer = sink->getSourceLocationLexer();
         if (lexer)
         {
             UnownedStringSlice token = lexer(UnownedStringSlice(pos, line.end()));
@@ -295,6 +309,28 @@ static void _sourceLocationNoteDiagnostic(SourceView* sourceView, SourceLoc sour
                 caretLine.appendRepeatedChar('~', token.getLength() - 1);
             }
         }
+
+        const Index maxLength = sink->getSourceLineMaxLength();
+        if (maxLength > 0)
+        {
+            Index endIndex = lexer ? caretLine.getLength() : (caretIndex + (maxLength / 4));
+
+            if (endIndex > maxLength)
+            {
+                Index startIndex = endIndex - (maxLength - 3);
+
+                _reduceLength(startIndex, sourceLine);
+                _reduceLength(startIndex, caretLine);
+            }
+
+            if (sourceLine.getLength() > maxLength)
+            {
+                StringBuilder buf;
+                buf.append(sourceLine.getUnownedSlice().head(maxLength - 3));
+                buf << "...";
+                sourceLine = buf;
+            }
+        }
     }
 
     // We could have handling here for if the line is too long, that we surround the important section
@@ -322,7 +358,7 @@ static void formatDiagnostic(
         {
             humaneLoc = sourceView->getHumaneLoc(sourceLoc);
         }
-        formatDiagnostic(humaneLoc, diagnostic, sb);
+        formatDiagnostic(humaneLoc, diagnostic, sink->getFlags(), sb);
 
         {
             SourceView* currentView = sourceView;
@@ -353,7 +389,7 @@ static void formatDiagnostic(
                 HumaneSourceLoc pasteHumaneLoc = initiatingView->getHumaneLoc(sourceView->getInitiatingSourceLoc());
 
                 // Okay we should output where the token paste took place
-                formatDiagnostic(pasteHumaneLoc, initiationDiagnostic, sb);
+                formatDiagnostic(pasteHumaneLoc, initiationDiagnostic, sink->getFlags(), sb);
 
                 // Make the initiatingView the current view
                 currentView = initiatingView;
@@ -365,7 +401,7 @@ static void formatDiagnostic(
     // of the other main severity types, and so the information should already be output on the initial line
     if (sourceView && sink->isFlagSet(DiagnosticSink::Flag::SourceLocationLine) && diagnostic.severity != Severity::Note)
     {
-       _sourceLocationNoteDiagnostic(sourceView, sourceLoc, sink->getSourceLocationLexer(), sb);
+        _sourceLocationNoteDiagnostic(sink, sourceView, sourceLoc, sb);
     }
 
     if (sourceView && sink->isFlagSet(DiagnosticSink::Flag::VerbosePath))
@@ -380,7 +416,7 @@ static void formatDiagnostic(
             actualHumaneLoc.line != humaneLoc.line ||
             actualHumaneLoc.column != humaneLoc.column)
         { 
-            formatDiagnostic(actualHumaneLoc, diagnostic, sb);
+            formatDiagnostic(actualHumaneLoc, diagnostic, sink->getFlags(), sb);
         }
     }
 }
@@ -390,10 +426,11 @@ void DiagnosticSink::init(SourceManager* sourceManager, SourceLocationLexer sour
     m_errorCount = 0;
     m_internalErrorLocsNoted = 0;
 
-    m_flags = 0;
-
     m_sourceManager = sourceManager;
     m_sourceLocationLexer = sourceLocationLexer;
+    m_sourceLineMaxLength = 0;
+
+    m_flags = Flag::HumaneLoc;
 
     // If we have a source location lexer, we'll by default enable source location output
     if (sourceLocationLexer)
@@ -402,45 +439,96 @@ void DiagnosticSink::init(SourceManager* sourceManager, SourceLocationLexer sour
     }
 }
 
-void DiagnosticSink::diagnoseImpl(SourceLoc const& pos, DiagnosticInfo const& info, int argCount, DiagnosticArg const* const* args)
+void DiagnosticSink::noteInternalErrorLoc(SourceLoc const& loc)
 {
-    StringBuilder sb;
-    formatDiagnosticMessage(sb, info.messageFormat, argCount, args);
+    // Don't consider invalid source locations.
+    if (!loc.isValid())
+        return;
 
-    Diagnostic diagnostic;
-    diagnostic.ErrorID = info.id;
-    diagnostic.Message = sb.ProduceString();
-    diagnostic.loc = pos;
-    diagnostic.severity = info.severity;
+    if (m_parentSink)
+    {
+        m_parentSink->noteInternalErrorLoc(loc);
+    }
 
-    if (diagnostic.severity >= Severity::Error)
+    // If this is the first source location being noted,
+    // then emit a message to help the user isolate what
+    // code might have confused the compiler.
+    if (m_internalErrorLocsNoted == 0)
     {
-        m_errorCount++;
+        diagnose(loc, MiscDiagnostics::noteLocationOfInternalError);
     }
+    m_internalErrorLocsNoted++;
+}
 
-    // Did the client supply a callback for us to use?
-    if( writer )
+SlangResult DiagnosticSink::getBlobIfNeeded(ISlangBlob** outBlob)
+{
+    // If the client doesn't want an output blob, there is nothing to do.
+    //
+    if (!outBlob) return SLANG_OK;
+
+    // For outputBuffer to be valid and hold diagnostics, writer must not be set
+    SLANG_ASSERT(writer == nullptr);
+
+    // If there were no errors, and there was no diagnostic output, there is nothing to do.
+    if (getErrorCount() == 0 && outputBuffer.getLength() == 0)
     {
-        // If so, pass the error string along to them
-        StringBuilder messageBuilder;
-        formatDiagnostic(this, diagnostic, messageBuilder);
+        return SLANG_OK;
+    }
+
+    Slang::ComPtr<ISlangBlob> blob = Slang::StringUtil::createStringBlob(outputBuffer);
+    *outBlob = blob.detach();
+
+    return SLANG_OK;
+}
+
+void DiagnosticSink::diagnoseImpl(DiagnosticInfo const& info, const UnownedStringSlice& formattedMessage)
+{
+    if (info.severity >= Severity::Error)
+    {
+        m_errorCount++;
+    }
 
-        writer->write(messageBuilder.getBuffer(), messageBuilder.getLength());
+    if (writer)
+    {
+        writer->write(formattedMessage.begin(), formattedMessage.getLength());
     }
     else
     {
-        // If the user doesn't have a callback, then just
-        // collect our diagnostic messages into a buffer
-        formatDiagnostic(this, diagnostic, outputBuffer);
+        outputBuffer.append(formattedMessage);
     }
 
-    if (diagnostic.severity >= Severity::Fatal)
+    if (m_parentSink)
+    {
+        m_parentSink->diagnoseImpl(info, formattedMessage);
+    }
+
+    if (info.severity >= Severity::Fatal)
     {
         // TODO: figure out a better policy for aborting compilation
         throw AbortCompilationException();
     }
 }
 
+void DiagnosticSink::diagnoseImpl(SourceLoc const& pos, DiagnosticInfo const& info, int argCount, DiagnosticArg const* const* args)
+{
+    StringBuilder messageBuilder;
+    {
+        StringBuilder sb;
+        formatDiagnosticMessage(sb, info.messageFormat, argCount, args);
+
+        Diagnostic diagnostic;
+        diagnostic.ErrorID = info.id;
+        diagnostic.Message = sb.ProduceString();
+        diagnostic.loc = pos;
+        diagnostic.severity = info.severity;
+
+        // If so, pass the error string along to them
+        formatDiagnostic(this, diagnostic, messageBuilder);
+    }
+
+    diagnoseImpl(info, messageBuilder.getUnownedSlice());
+}
+
 void DiagnosticSink::diagnoseRaw(
     Severity    severity,
     char const* message)
@@ -470,6 +558,11 @@ void DiagnosticSink::diagnoseRaw(
         outputBuffer.append(message);
     }
 
+    if (m_parentSink)
+    {
+        m_parentSink->diagnoseRaw(severity, message);
+    }
+
     if (severity >= Severity::Fatal)
     {
         // TODO: figure out a better policy for aborting compilation
diff --git a/source/compiler-core/slang-diagnostic-sink.h b/source/compiler-core/slang-diagnostic-sink.h
index 347ff761c..09502e48a 100644
--- a/source/compiler-core/slang-diagnostic-sink.h
+++ b/source/compiler-core/slang-diagnostic-sink.h
@@ -138,8 +138,9 @@ public:
     {
         enum Enum: Flags
         {
-            VerbosePath = 0x1,                  ///< Will display a more verbose path (if available) - such as a canonical or absolute path
-            SourceLocationLine = 0x2,           ///< If set will display the location line if source is available
+            VerbosePath         = 0x1,           ///< Will display a more verbose path (if available) - such as a canonical or absolute path
+            SourceLocationLine  = 0x2,           ///< If set will display the location line if source is available
+            HumaneLoc           = 0x4,           ///< If set will display humane locs (filename/line number) information
         };
     };
 
@@ -203,6 +204,8 @@ public:
         /// Set the source manager used for lookup of source locs
     void setSourceManager(SourceManager* inSourceManager) { m_sourceManager = inSourceManager; }
 
+        /// Set the flags
+    void setFlags(Flags flags) { m_flags = flags; } 
         /// Get the flags
     Flags getFlags() const { return m_flags; }
         /// Set a flag
@@ -217,6 +220,14 @@ public:
         /// character caret at location
     SourceLocationLexer getSourceLocationLexer() const { return m_sourceLocationLexer; }
 
+        /// Set the maximum length (in chars) of a source line displayed. Set to 0 for no limit
+    void setSourceLineMaxLength(Index length) { m_sourceLineMaxLength = length; }
+    Index getSourceLineMaxLength() const { return m_sourceLineMaxLength; }
+
+        /// The parent sink is another sink that will receive diagnostics from this sink.
+    void setParentSink(DiagnosticSink* parentSink) { m_parentSink = parentSink; }
+    DiagnosticSink* getParentSink() const { return m_parentSink; }
+
         /// Initialize state. 
     void init(SourceManager* sourceManager, SourceLocationLexer sourceLocationLexer);
 
@@ -238,10 +249,17 @@ public:
 
 protected:
     void diagnoseImpl(SourceLoc const& pos, DiagnosticInfo const& info, int argCount, DiagnosticArg const* const* args);
+    void diagnoseImpl(DiagnosticInfo const& info, const UnownedStringSlice& formattedMessage);
+
+        /// If set all diagnostics (as formatted by *this* sink, will be routed to the parent).
+    DiagnosticSink* m_parentSink = nullptr;
 
     int m_errorCount = 0;
     int m_internalErrorLocsNoted = 0;
 
+    /// If 0, then there is no limit, otherwise max amount of chars of the source line location
+    Index m_sourceLineMaxLength = 0;            
+
     Flags m_flags = 0;
 
     // The source manager to use when mapping source locations to file+line info
diff --git a/source/compiler-core/slang-misc-diagnostic-defs.h b/source/compiler-core/slang-misc-diagnostic-defs.h
index a1fd01475..df31b9f01 100644
--- a/source/compiler-core/slang-misc-diagnostic-defs.h
+++ b/source/compiler-core/slang-misc-diagnostic-defs.h
@@ -24,4 +24,8 @@
 
 DIAGNOSTIC(-1, Note, seeTokenPasteLocation, "see token pasted location")
 
+DIAGNOSTIC(21, Error, expectedArgumentForOption, "expected an argument for command-line option '$0'")
+
+DIAGNOSTIC(99999, Note, noteLocationOfInternalError, "an internal error threw an exception while working on code near this location")
+
 #undef DIAGNOSTIC
diff --git a/source/compiler-core/slang-source-loc.h b/source/compiler-core/slang-source-loc.h
index 54811918f..5093e8de0 100644
--- a/source/compiler-core/slang-source-loc.h
+++ b/source/compiler-core/slang-source-loc.h
@@ -379,6 +379,9 @@ struct SourceManager
         /// Allocate a range of SourceLoc locations, these can be used to identify a specific location in the source
     SourceRange allocateSourceRange(UInt size);
 
+        /// Returns the loc for start of next allocation
+    SourceLoc getNextRangeStart() const { return m_nextLoc; }
+
         /// Create a SourceFile defined with the specified path, and content held within a blob
     SourceFile* createSourceFileWithSize(const PathInfo& pathInfo, size_t contentSize);
     SourceFile* createSourceFileWithString(const PathInfo& pathInfo, const String& contents);