From 34a1ff5226a526cc17c5baecd63637f69c324fc7 Mon Sep 17 00:00:00 2001 From: jsmall-nvidia Date: Sat, 22 May 2021 16:03:30 -0400 Subject: Improvements in -X support (#1852) * #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. * WIP extracting command line args for downstream tools. * Unit tests/bug fixes around extracting args. * Use DownstreamArgs in the EndToEndCompileRequest * Passing downstream compiler options downstream. * Fix issue with endToEndReq being nullptr. * Fix issue with diagnostics number change. * Small improvements to how the source line is displayed if it's too long. Default to 120, as suggested in previous review. * Make render test use x-args parsing and CommandArgReader. * Added missing diagnostics. * More DownstreamArgs to linkage so can be seen by 'components'. Added dxc-x-arg test. * Used combination of name and args instead of two Lists, which whilst equivalent was perhaps a little confusing. * Added documentation for -X support. * Added test for x-args parsing diagnostic. Improved diagnostic with list of known names. * Fix issues from merge. * Fix lookup for -matrix-layout-column-major in render test. * Remove commented out line. --- source/compiler-core/slang-command-line-args.cpp | 44 +++++++++++++++++------ source/compiler-core/slang-command-line-args.h | 21 ++++++++--- source/compiler-core/slang-misc-diagnostic-defs.h | 2 +- 3 files changed, 50 insertions(+), 17 deletions(-) (limited to 'source/compiler-core') diff --git a/source/compiler-core/slang-command-line-args.cpp b/source/compiler-core/slang-command-line-args.cpp index 1d452bb10..d7a053b00 100644 --- a/source/compiler-core/slang-command-line-args.cpp +++ b/source/compiler-core/slang-command-line-args.cpp @@ -63,7 +63,6 @@ bool CommandLineArgs::hasArgs(const char*const* args, Index count) const return true; } - /* !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! CommandLineReader @@ -121,14 +120,11 @@ SlangResult CommandLineReader::expectArg(CommandLineArg& outArg) Index DownstreamArgs::addName(const String& name) { - Index index = m_names.indexOf(name); + Index index = findName(name); if (index < 0) { - index = m_names.getCount(); - m_names.add(name); - - CommandLineArgs args(m_context); - m_args.add(args); + index = m_entries.getCount(); + m_entries.add(Entry{name, CommandLineArgs(m_context) }); } return index; } @@ -137,7 +133,10 @@ Index DownstreamArgs::_findOrAddName(SourceLoc loc, const UnownedStringSlice& na { if (name.getLength() <= 0) { - sink->diagnose(loc, MiscDiagnostics::downstreamToolNameNotDefined); + if (sink) + { + sink->diagnose(loc, MiscDiagnostics::downstreamToolNameNotDefined); + } return -1; } @@ -152,15 +151,38 @@ Index DownstreamArgs::_findOrAddName(SourceLoc loc, const UnownedStringSlice& na return index; } - sink->diagnose(loc, MiscDiagnostics::downstreamNameNotKnown); + if (sink) + { + StringBuilder names; + + names << "[ "; + for (Index i = 0; i < m_entries.getCount(); ++i) + { + if (i) + { + names << ", "; + } + names << m_entries[i].name; + } + names << " ]"; + + sink->diagnose(loc, MiscDiagnostics::downstreamNameNotKnown, names); + } return -1; } CommandLineArgs& DownstreamArgs::getArgsByName(char* name) { - Index index = findName(name); + const Index index = findName(name); + SLANG_ASSERT(index >= 0); + return m_entries[index].args; +} + +const CommandLineArgs& DownstreamArgs::getArgsByName(char* name) const +{ + const Index index = findName(name); SLANG_ASSERT(index >= 0); - return m_args[index]; + return m_entries[index].args; } SlangResult DownstreamArgs::stripDownstreamArgs(CommandLineArgs& ioArgs, Flags flags, DiagnosticSink* sink) diff --git a/source/compiler-core/slang-command-line-args.h b/source/compiler-core/slang-command-line-args.h index 7500c1a91..c18996005 100644 --- a/source/compiler-core/slang-command-line-args.h +++ b/source/compiler-core/slang-command-line-args.h @@ -17,6 +17,9 @@ struct CommandLineArg SourceLoc loc; ///< The location of the arg }; +/* This type ends up being really just a container for the sourceManager that has the CommandLine specific SourceLocs. +That it would perhaps be better to just have SourceManager derive from RefObject, and then we could remove this +type. */ class CommandLineContext : public RefObject { public: @@ -27,6 +30,8 @@ public: { m_sourceManager.initialize(nullptr, fileSystemExt); // Make range start from high value, so can be differentiated from other uses + // That this doesn't not assume exclusive use of this range - just that in normal use scenarios + // there is no confusion, and using the wrong source manager, will typically report nothing is found. m_sourceManager.allocateSourceRange(~(~SourceLoc::RawValue(0) >> 1)); } @@ -128,15 +133,22 @@ struct DownstreamArgs }; }; + struct Entry + { + String name; ///< The name of the 'tool' that these args are associated with + CommandLineArgs args; ///< The args to be passed to the tool + }; + /// Add a name, returns the index Index addName(const String& name); /// Find the index of a name. Returns < 0 if not found. - Index findName(const String& name) const { return m_names.indexOf(name); } + Index findName(const String& name) const { return m_entries.findFirstIndex([&](const Entry& entry) -> bool { return entry.name == name; }); } /// Get the args at the nameIndex - CommandLineArgs& getArgsAt(Index nameIndex) { return m_args[nameIndex]; } + CommandLineArgs& getArgsAt(Index nameIndex) { return m_entries[nameIndex].args; } /// Get args by name - will assert if name isn't found CommandLineArgs& getArgsByName(char* name); + const CommandLineArgs& getArgsByName(char* name) const; /// Looks for '-X' expressions, removing them from ioArgs and putting in appropriate args SlangResult stripDownstreamArgs(CommandLineArgs& ioArgs, Flags flags, DiagnosticSink* sink); @@ -155,10 +167,9 @@ struct DownstreamArgs protected: Index _findOrAddName(SourceLoc loc, const UnownedStringSlice& name, Flags flags, DiagnosticSink* sink); - List m_names; - List m_args; + List m_entries; ///< All of the entries - RefPtr m_context; + RefPtr m_context; ///< The context that is being used (primarily for loc tracking) across all entries/args }; diff --git a/source/compiler-core/slang-misc-diagnostic-defs.h b/source/compiler-core/slang-misc-diagnostic-defs.h index e5c7c335e..aa87f02f9 100644 --- a/source/compiler-core/slang-misc-diagnostic-defs.h +++ b/source/compiler-core/slang-misc-diagnostic-defs.h @@ -24,7 +24,7 @@ DIAGNOSTIC(-1, Note, seeTokenPasteLocation, "see token pasted location") -DIAGNOSTIC(100000, Error, downstreamNameNotKnown, "downstream tool name not known") +DIAGNOSTIC(100000, Error, downstreamNameNotKnown, "downstream tool name not known, allowed names are $0") DIAGNOSTIC(100001, Error, expectedArgumentForOption, "expected an argument for command-line option '$0'") DIAGNOSTIC(100002, Error, unbalancedDownstreamArguments, "unbalanced downstream arguments") DIAGNOSTIC(100003, Error, closeOfUnopenDownstreamArgs, "close of an unopen downstream argument scope") -- cgit v1.2.3