From dafe651ecf21f2dce7f156179af785adca08ced0 Mon Sep 17 00:00:00 2001 From: jsmall-nvidia Date: Wed, 28 Sep 2022 13:30:37 -0400 Subject: Improvements around diagnostic controls (#2414) * #include an absolute path didn't work - because paths were taken to always be relative. * Test for disabling warnings. * Output diagnostic if argument parsing fails in render test. * More improvements around disabling diagnostics. * Add support for re enabling a warning. * Add warning controls to help text. * Tidy up around NameConventionUtil. * Make NameConvention an enum. * Handle leading underscores. * Update comment, and remove intial handling of _ prefix. --- source/compiler-core/slang-diagnostic-sink.cpp | 93 +++++++++---- source/compiler-core/slang-diagnostic-sink.h | 39 +++--- .../compiler-core/slang-name-convention-util.cpp | 152 ++++++++++++++++----- source/compiler-core/slang-name-convention-util.h | 71 ++++++++-- source/slang/slang-diagnostics.cpp | 7 +- source/slang/slang-diagnostics.h | 1 + source/slang/slang-options.cpp | 123 ++++++++++++++--- tests/compute/disable-warning.slang | 17 +++ tests/compute/disable-warning.slang.expected.txt | 4 + tools/render-test/slang-support.cpp | 13 +- tools/slang-cpp-extractor/cpp-extractor-main.cpp | 4 +- tools/slang-cpp-extractor/node-tree.cpp | 2 +- 12 files changed, 411 insertions(+), 115 deletions(-) create mode 100644 tests/compute/disable-warning.slang create mode 100644 tests/compute/disable-warning.slang.expected.txt diff --git a/source/compiler-core/slang-diagnostic-sink.cpp b/source/compiler-core/slang-diagnostic-sink.cpp index 85c7792c0..5e58098b0 100644 --- a/source/compiler-core/slang-diagnostic-sink.cpp +++ b/source/compiler-core/slang-diagnostic-sink.cpp @@ -667,45 +667,88 @@ void DiagnosticSink::diagnoseRaw( } } -/* !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! DiagnosticLookup !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! */ - -void DiagnosticsLookup::_add(const char* name, Index index) +void DiagnosticSink::overrideDiagnosticSeverity(int diagnosticId, Severity overrideSeverity, const DiagnosticInfo* info) { - UnownedStringSlice nameSlice(name); - m_map.Add(nameSlice, index); - - // Add a dashed version (KababCase) + if (info) { - m_work.Clear(); + SLANG_ASSERT(info->id == diagnosticId); - NameConventionUtil::convert(NameConvention::Camel, nameSlice, CharCase::Lower, NameConvention::Kabab, m_work); + // If the override is the same as the default, we can just remove the override + if (info->severity == overrideSeverity) + { + m_severityOverrides.Remove(diagnosticId); + return; + } + } - UnownedStringSlice dashSlice(m_arena.allocateString(m_work.getBuffer(), m_work.getLength()), m_work.getLength()); + // Set the override + m_severityOverrides[diagnosticId] = overrideSeverity; +} - m_map.AddIfNotExists(dashSlice, index); - } +/* !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! DiagnosticLookup !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! */ + +Index DiagnosticsLookup::_findDiagnosticIndexByExactName(const UnownedStringSlice& slice) const +{ + const Index* indexPtr = m_nameMap.TryGetValue(slice); + return indexPtr ? *indexPtr : -1; +} + +void DiagnosticsLookup::_addName(const char* name, Index diagnosticIndex) +{ + UnownedStringSlice nameSlice(name); + m_nameMap.Add(nameSlice, diagnosticIndex); } + void DiagnosticsLookup::addAlias(const char* name, const char* diagnosticName) { - const Index index = _findDiagnosticIndex(UnownedStringSlice(diagnosticName)); + const Index index = _findDiagnosticIndexByExactName(UnownedStringSlice(diagnosticName)); SLANG_ASSERT(index >= 0); if (index >= 0) { - _add(name, index); + _addName(name, index); } } +const DiagnosticInfo* DiagnosticsLookup::getDiagnosticById(Int id) const +{ + const auto indexPtr = m_idMap.TryGetValue(id); + return indexPtr ? m_diagnostics[*indexPtr] : nullptr; +} + +const DiagnosticInfo* DiagnosticsLookup::findDiagnosticByExactName(const UnownedStringSlice& slice) const +{ + const Index* indexPtr = m_nameMap.TryGetValue(slice); + return indexPtr ? m_diagnostics[*indexPtr] : nullptr; +} + +const DiagnosticInfo* DiagnosticsLookup::findDiagnosticByName(const UnownedStringSlice& slice) const +{ + const auto convention = NameConventionUtil::inferConventionFromText(slice); + switch (convention) + { + case NameConvention::Invalid: return nullptr; + case NameConvention::LowerCamel: return findDiagnosticByExactName(slice); + default: break; + } + + StringBuilder buf; + NameConventionUtil::convert(getNameStyle(convention), slice, NameConvention::LowerCamel, buf); + + return findDiagnosticByExactName(buf.getUnownedSlice()); +} + Index DiagnosticsLookup::add(const DiagnosticInfo* info) { // Check it's not already added SLANG_ASSERT(m_diagnostics.indexOf(info) < 0); - const Index index = m_diagnostics.getCount(); - - _add(info->name, index); - + const Index diagnosticIndex = m_diagnostics.getCount(); m_diagnostics.add(info); - return index; + + _addName(info->name, diagnosticIndex); + m_idMap.AddIfNotExists(info->id, diagnosticIndex); + + return diagnosticIndex; } void DiagnosticsLookup::add(const DiagnosticInfo*const* infos, Index infosCount) @@ -724,21 +767,13 @@ DiagnosticsLookup::DiagnosticsLookup(): DiagnosticsLookup::DiagnosticsLookup(const DiagnosticInfo*const* diagnostics, Index diagnosticsCount) : m_arena(kArenaInitialSize) { - m_diagnostics.addRange(diagnostics, diagnosticsCount); - // TODO: We should eventually have a more formal system for associating individual // diagnostics, or groups of diagnostics, with user-exposed names for use when // enabling/disabling warnings (or turning warnings into errors, etc.). // - // For now we build a map from diagnostic name to it's entry. Two entries are typically - // added - the 'original name' as associated with the diagnostic in lowerCamel, and - // a dashified version. + // For now we build a map from diagnostic name to it's entry. - for (Index i = 0; i < diagnosticsCount; ++i) - { - const DiagnosticInfo* diagnostic = diagnostics[i]; - _add(diagnostic->name, i); - } + add(diagnostics, diagnosticsCount); } } // namespace Slang diff --git a/source/compiler-core/slang-diagnostic-sink.h b/source/compiler-core/slang-diagnostic-sink.h index c4d2e07d3..466a68827 100644 --- a/source/compiler-core/slang-diagnostic-sink.h +++ b/source/compiler-core/slang-diagnostic-sink.h @@ -227,10 +227,8 @@ public: bool isFlagSet(Flag::Enum flag) { return (m_flags & Flags(flag)) != 0; } /// Sets an override on the severity of a specific diagnostic message (by numeric identifier) - void overrideDiagnosticSeverity(int messageID, Severity overrideSeverity) - { - m_severityOverrides[messageID] = overrideSeverity; - } + /// info can be set to nullptr if only to override + void overrideDiagnosticSeverity(int diagnosticId, Severity overrideSeverity, const DiagnosticInfo* info = nullptr); /// Get the (optional) diagnostic sink lexer. This is used to /// improve quality of highlighting a locations token. If not set, will just have a single @@ -322,37 +320,46 @@ class DiagnosticsLookup : public RefObject public: static const Index kArenaInitialSize = 2048; - const DiagnosticInfo* findDiagostic(const UnownedStringSlice& slice) const - { - const Index* indexPtr = m_map.TryGetValue(slice); - return indexPtr ? m_diagnostics[*indexPtr] : nullptr; - } - Index _findDiagnosticIndex(const UnownedStringSlice& slice) const - { - const Index* indexPtr = m_map.TryGetValue(slice); - return indexPtr ? *indexPtr : 0; - } + /// Will take into account the slice name could be using different conventions + const DiagnosticInfo* findDiagnosticByName(const UnownedStringSlice& slice) const; + /// The name must be as defined in the diagnostics exactly, typically lower camel + const DiagnosticInfo* findDiagnosticByExactName(const UnownedStringSlice& slice) const; + + /// Get a diagnostic by it's id. + /// NOTE! That it is possible for multiple diagnostics to have the same id. This will return + /// the first added + const DiagnosticInfo* getDiagnosticById(Int id) const; /// info must stay in scope Index add(const DiagnosticInfo* info); + /// Infos referenced must remain in scope void add(const DiagnosticInfo*const* infos, Index infosCount); + /// NOTE! Name must stay in scope as long as the diagnostics lookup. + /// If not possible add it to the arena to keep in scope. void addAlias(const char* name, const char* diagnosticName); /// Get the diagnostics held in this lookup const List& getDiagnostics() const { return m_diagnostics; } + /// Get the associated arena + MemoryArena& getArena() { return m_arena; } + /// NOTE! diagnostics must stay in scope for lifetime of lookup DiagnosticsLookup(const DiagnosticInfo*const* diagnostics, Index diagnosticsCount); DiagnosticsLookup(); protected: - void _add(const char* name, Index index); + void _addName(const char* name, Index diagnosticIndex); + + Index _findDiagnosticIndexByExactName(const UnownedStringSlice& slice) const; List m_diagnostics; StringBuilder m_work; - Dictionary m_map; + Dictionary m_nameMap; + Dictionary m_idMap; + MemoryArena m_arena; }; diff --git a/source/compiler-core/slang-name-convention-util.cpp b/source/compiler-core/slang-name-convention-util.cpp index 930e00e18..c3d08326c 100644 --- a/source/compiler-core/slang-name-convention-util.cpp +++ b/source/compiler-core/slang-name-convention-util.cpp @@ -7,35 +7,110 @@ namespace Slang { -/* static */NameConvention NameConventionUtil::getConvention(const UnownedStringSlice& slice) +/* static */NameConvention NameConventionUtil::inferConventionFromText(const UnownedStringSlice& slice) { + // If no chars, or first char isn't alpha we don't know what it is + if (slice.getLength() <= 0 || !CharUtil::isAlpha(slice[0])) + { + return NameConvention::Invalid; + } + + typedef int Flags; + struct Flag + { + enum Enum : Flags + { + Underscore = 0x1, + Dash = 0x2, + Upper = 0x4, + Lower = 0x8, + }; + }; + + Flags flags = 0; + for (const char c : slice) { switch (c) { - case '-': return NameConvention::Kabab; - case '_': return NameConvention::Snake; + case '-': flags |= Flag::Dash; break; + case '_': flags |= Flag::Underscore; break; + default: + { + if (CharUtil::isLower(c)) + { + flags |= Flag::Lower; + } + else if (CharUtil::isUpper(c)) + { + flags |= Flag::Upper; + } + else if (CharUtil::isDigit(c)) + { + // Is okay as long as not first char (which we already tested is alpha) + } + else + { + // Don't know what this style is + return NameConvention::Invalid; + } + } + } + } + + // Use flags to determine what convention is used + switch (flags) + { + // We'll assume it's lower camel. + case Flag::Lower: return NameConvention::LowerCamel; + // We'll assume it's upper snake. It almost certainly isn't camel, and snake is more usual + // than kabab. + case Flag::Upper: return NameConvention::UpperSnake; + case Flag::Upper | Flag::Lower: + { + // Looks like camel, choose the right case based on first char + return CharUtil::isUpper(slice[0]) ? NameConvention::UpperCamel : NameConvention::LowerCamel; + } + case Flag::Lower | Flag::Dash: return NameConvention::LowerKabab; + case Flag::Upper | Flag::Dash: return NameConvention::UpperKabab; + case Flag::Lower | Flag::Underscore: return NameConvention::LowerSnake; + case Flag::Upper | Flag::Underscore: return NameConvention::UpperSnake; + default: break; + } + + // Don't know what this style is + return NameConvention::Invalid; +} + +/* static */NameStyle NameConventionUtil::inferStyleFromText(const UnownedStringSlice& slice) +{ + for (const char c : slice) + { + switch (c) + { + case '-': return NameStyle::Kabab; + case '_': return NameStyle::Snake; default: break; } } - return NameConvention::Camel; + return NameStyle::Camel; } -/* static */void NameConventionUtil::split(NameConvention convention, const UnownedStringSlice& slice, List& out) +/* static */void NameConventionUtil::split(NameStyle style, const UnownedStringSlice& slice, List& out) { - switch (convention) + switch (style) { - case NameConvention::Kabab: + case NameStyle::Kabab: { StringUtil::split(slice, '-', out); break; } - case NameConvention::Snake: + case NameStyle::Snake: { StringUtil::split(slice, '_', out); break; } - case NameConvention::Camel: + case NameStyle::Camel: { typedef CharUtil::Flags CharFlags; typedef CharUtil::Flag CharFlag; @@ -87,15 +162,20 @@ namespace Slang } break; } + case NameStyle::Unknown: + { + out.add(slice); + break; + } } } void NameConventionUtil::split(const UnownedStringSlice& slice, List& out) { - split(getConvention(slice), slice, out); + split(inferStyleFromText(slice), slice, out); } -/* static */void NameConventionUtil::join(const UnownedStringSlice* slices, Index slicesCount, CharCase charCase, char joinChar, StringBuilder& out) +/* static */void NameConventionUtil::join(const UnownedStringSlice* slices, Index slicesCount, NameConvention convention, char joinChar, StringBuilder& out) { if (slicesCount <= 0) { @@ -111,6 +191,8 @@ void NameConventionUtil::split(const UnownedStringSlice& slice, List 0 && !(i == 0 && charCase == CharCase::Lower)) + if (count > 0 && !(i == 0 && isLower(convention))) { // Capitalize first letter of each word, unless on first word and 'lower' dst[j] = CharUtil::toUpper(src[j]); @@ -189,24 +268,27 @@ void NameConventionUtil::split(const UnownedStringSlice& slice, List slices; - split(fromConvention, slice, slices); + split(fromStyle, slice, slices); // Join the slices in the toConvention - join(slices.getBuffer(), slices.getCount(), charCase, toConvention, out); + join(slices.getBuffer(), slices.getCount(), toConvention, out); } -/* static */void NameConventionUtil::convert(const UnownedStringSlice& slice, CharCase charCase, NameConvention toConvention, StringBuilder& out) +/* static */void NameConventionUtil::convert(const UnownedStringSlice& slice, NameConvention toConvention, StringBuilder& out) { - convert(getConvention(slice), slice, charCase, toConvention, out); + convert(inferStyleFromText(slice), slice, toConvention, out); } } diff --git a/source/compiler-core/slang-name-convention-util.h b/source/compiler-core/slang-name-convention-util.h index d84ffd3b7..95f4f63aa 100644 --- a/source/compiler-core/slang-name-convention-util.h +++ b/source/compiler-core/slang-name-convention-util.h @@ -7,19 +7,52 @@ namespace Slang { -enum class NameConvention +typedef uint8_t NameConventionBackingType; + +enum class NameStyle : NameConventionBackingType +{ + Unknown, /// Unknown style + Kabab, /// Words are separated with -. WORDS-ARE-SEPARATED, words-are-separted + Snake, /// Words are separated with _. WORDS_ARE_SEPARATED, words_are_separated + Camel, /// Words start with a capital. (Upper will make first words character capitalized, aka PascalCase) +}; + +struct NameConventionFlag +{ + enum Enum : NameConventionBackingType + { + UpperCase = 0x80, + }; +}; + +struct NameConventionMask { - Kabab, /// Words are separated with -. WORDS-ARE-SEPARATED - Snake, /// Words are separated with _. WORDS_ARE_SEPARATED - Camel, /// Words start with a capital. (Upper will make first words character capitalized, aka PascalCase) + enum Enum : NameConventionBackingType + { + Style = 0x7, + }; }; -enum class CharCase +enum class NameConvention : NameConventionBackingType { - Upper, - Lower, + Invalid = NameConventionBackingType(NameStyle::Unknown), + + LowerKabab = NameConventionBackingType(NameStyle::Kabab), + LowerSnake = NameConventionBackingType(NameStyle::Snake), + LowerCamel = NameConventionBackingType(NameStyle::Camel), + + UpperKabab = NameConventionBackingType(NameStyle::Kabab) | NameConventionFlag::UpperCase, + UpperSnake = NameConventionBackingType(NameStyle::Snake) | NameConventionFlag::UpperCase, + UpperCamel = NameConventionBackingType(NameStyle::Camel) | NameConventionFlag::UpperCase, }; +SLANG_FORCE_INLINE NameConvention makeUpper(NameStyle style) { return NameConvention(NameConventionBackingType(style) | NameConventionFlag::UpperCase); } +SLANG_FORCE_INLINE NameConvention makeLower(NameStyle style) { return NameConvention(style); } + +SLANG_FORCE_INLINE bool isUpper(NameConvention convention) { return (NameConventionBackingType(convention) & NameConventionFlag::UpperCase) != 0; } +SLANG_FORCE_INLINE bool isLower(NameConvention convention) { return (NameConventionBackingType(convention) & NameConventionFlag::UpperCase) == 0; } +SLANG_FORCE_INLINE NameStyle getNameStyle(NameConvention convention) { return NameStyle(NameConventionBackingType(convention) & NameConventionMask::Style); } + /* This utility is to enable easy conversion and interpretation of names that use standard conventions, typically in programming languages. The conventions are largely how to represent multiple words together. @@ -32,23 +65,33 @@ struct NameConventionUtil { /// Given a slice tries to determine the convention used. /// If no separators are found, will assume Camel - static NameConvention getConvention(const UnownedStringSlice& slice); + /// Doesn't exhaustively test the string slice, or determine invalid scenarios + /// Use 'getConvention' to get error checking + static NameStyle inferStyleFromText(const UnownedStringSlice& slice); + + /// Gets the naming convention based on the slice. + /// Will return invalid convention if cannot be determined. + /// + /// TODO(JS): + /// Does not handle leading `_` styles: "_a" and "_1" will be invalid. + /// We may want to make it do so, but requires changes in infer, split and join. + static NameConvention inferConventionFromText(const UnownedStringSlice& slice); /// Given a slice and a naming convention, split into it's constituent parts. If convention isn't specified, will infer from slice using getConvention. - static void split(NameConvention convention, const UnownedStringSlice& slice, List& out); + static void split(NameStyle nameStyle, const UnownedStringSlice& slice, List& out); static void split(const UnownedStringSlice& slice, List& out); /// Given slices, join together with the specified convention into out - static void join(const UnownedStringSlice* slices, Index slicesCount, CharCase charCase, NameConvention convention, StringBuilder& out); + static void join(const UnownedStringSlice* slices, Index slicesCount, NameConvention convention, StringBuilder& out); /// Join with a join char, and potentially changing case of input slices - static void join(const UnownedStringSlice* slices, Index slicesCount, CharCase charCase, char joinChar, StringBuilder& out); + static void join(const UnownedStringSlice* slices, Index slicesCount, NameConvention convention, char joinChar, StringBuilder& out); /// Convert from one convention to another. If fromConvention isn't specified, will infer from slice using getConvention. - static void convert(NameConvention fromConvention, const UnownedStringSlice& slice, CharCase charCase, NameConvention toConvention, StringBuilder& out); - static void convert(const UnownedStringSlice& slice, CharCase charCase, NameConvention toConvention, StringBuilder& out); + static void convert(NameStyle fromStyle, const UnownedStringSlice& slice, NameConvention toConvention, StringBuilder& out); + static void convert(const UnownedStringSlice& slice, NameConvention toConvention, StringBuilder& out); }; } -#endif // SLANG_CORE_NAME_CONVENTION_UTIL_H +#endif // SLANG_COMPILER_CORE_NAME_CONVENTION_UTIL_H diff --git a/source/slang/slang-diagnostics.cpp b/source/slang/slang-diagnostics.cpp index 4a831aacf..08ab829b9 100644 --- a/source/slang/slang-diagnostics.cpp +++ b/source/slang/slang-diagnostics.cpp @@ -52,7 +52,12 @@ static DiagnosticsLookup* _getDiagnosticLookupSingleton() DiagnosticInfo const* findDiagnosticByName(UnownedStringSlice const& name) { - return _getDiagnosticLookupSingleton()->findDiagostic(name); + return _getDiagnosticLookupSingleton()->findDiagnosticByName(name); +} + +const DiagnosticsLookup* getDiagnosticsLookup() +{ + return _getDiagnosticLookupSingleton(); } } // namespace Slang diff --git a/source/slang/slang-diagnostics.h b/source/slang/slang-diagnostics.h index 05411a90a..917187007 100644 --- a/source/slang/slang-diagnostics.h +++ b/source/slang/slang-diagnostics.h @@ -13,6 +13,7 @@ namespace Slang { DiagnosticInfo const* findDiagnosticByName(UnownedStringSlice const& name); + const DiagnosticsLookup* getDiagnosticsLookup(); namespace Diagnostics { diff --git a/source/slang/slang-options.cpp b/source/slang/slang-options.cpp index a572ab375..58039bb7c 100644 --- a/source/slang/slang-options.cpp +++ b/source/slang/slang-options.cpp @@ -15,6 +15,8 @@ #include "../compiler-core/slang-artifact-impl.h" #include "../compiler-core/slang-artifact-representation-impl.h" +#include "../compiler-core/slang-name-convention-util.h" + #include "slang-repro.h" #include "slang-serialize-ir.h" @@ -26,6 +28,8 @@ #include "../compiler-core/slang-artifact-desc-util.h" #include "../compiler-core/slang-core-diagnostics.h" +#include "../core/slang-char-util.h" + #include namespace Slang { @@ -557,6 +561,9 @@ struct OptionsParser " -warnings-as-errors all: Treat all warnings as errors.\n" " -warnings-as-errors [,...]: Treat specific warning ids as errors.\n" " -warnings-disable [,...]: Disable specific warning ids.\n" + " -W: Enable a warning with the specified id.\n" + " -Wno-: Disable a warning with the specified id.\n" + " -dump-warning-diagnostics: Dump to output list of warning diagnostic numeric and name ids.\n" " --: Treat the rest of the command line as input files.\n" "\n" "Target code generation options:\n" @@ -711,20 +718,90 @@ struct OptionsParser #undef EXECUTABLE_EXTENSION } - SlangResult overrideDiagnosticSeverity(String const& identifierList, Severity overrideSeverity) + // Pass Severity::Disabled to allow any original severity + SlangResult _overrideDiagnostics(const UnownedStringSlice& identifierList, Severity originalSeverity, Severity overrideSeverity, DiagnosticSink* sink) { List slices; - StringUtil::split(identifierList.getUnownedSlice(), ',', slices); - Index sliceCount = slices.getCount(); + StringUtil::split(identifierList, ',', slices); + + for (const auto& slice : slices) + { + SLANG_RETURN_ON_FAIL(_overrideDiagnostic(slice, originalSeverity, overrideSeverity, sink)); + } + return SLANG_OK; + } + + // Pass Severity::Disabled to allow any original severity + SlangResult _overrideDiagnostic(const UnownedStringSlice& identifier, Severity originalSeverity, Severity overrideSeverity, DiagnosticSink* sink) + { + auto diagnosticsLookup = getDiagnosticsLookup(); + + const DiagnosticInfo* diagnostic = nullptr; + Int diagnosticId = -1; + + // If it starts with a digit we assume it a number + if (identifier.getLength() > 0 && (CharUtil::isDigit(identifier[0]) || identifier[0] == '-')) + { + if (SLANG_FAILED(StringUtil::parseInt(identifier, diagnosticId))) + { + sink->diagnose(SourceLoc(), Diagnostics::unknownDiagnosticName, identifier); + return SLANG_FAIL; + } + + // If we use numbers, we don't worry if we can't find a diagnostic + // and silently ignore. This was the previous behavior, and perhaps + // provides a way to safely disable warnings, without worrying about + // the version of the compiler. + diagnostic = diagnosticsLookup->getDiagnosticById(diagnosticId); + } + else + { + diagnostic = diagnosticsLookup->findDiagnosticByName(identifier); + if (!diagnostic) + { + sink->diagnose(SourceLoc(), Diagnostics::unknownDiagnosticName, identifier); + return SLANG_FAIL; + } + diagnosticId = diagnostic->id; + } + + // If we are only allowing certain original severities check it's the right type + if (diagnostic && originalSeverity != Severity::Disable && diagnostic->severity != originalSeverity) + { + // Strictly speaking the diagnostic name is known, but it's not the right severity + // to be converted from, so it is an 'unknown name' in the context of severity... + // Or perhaps we want another diagnostic + sink->diagnose(SourceLoc(), Diagnostics::unknownDiagnosticName, identifier); + return SLANG_FAIL; + } - for (Index i = 0; i < sliceCount; ++i) + // Override the diagnostic severity in the sink + requestImpl->getSink()->overrideDiagnosticSeverity(int(diagnosticId), overrideSeverity, diagnostic); + + return SLANG_OK; + } + + SlangResult _dumpDiagnostics(Severity originalSeverity, DiagnosticSink* sink) + { + // Get the diagnostics and dump them + auto diagnosticsLookup = getDiagnosticsLookup(); + + StringBuilder buf; + + for (const auto& diagnostic : diagnosticsLookup->getDiagnostics()) { - UnownedStringSlice warningIdentifier = slices[i]; + if (originalSeverity != Severity::Disable && + diagnostic->severity != originalSeverity) + { + continue; + } - Int warningIndex = -1; - SLANG_RETURN_ON_FAIL(StringUtil::parseInt(warningIdentifier, warningIndex)); + buf.Clear(); - requestImpl->getSink()->overrideDiagnosticSeverity(int(warningIndex), overrideSeverity); + buf << diagnostic->id << " : "; + NameConventionUtil::convert(NameStyle::Camel, UnownedStringSlice(diagnostic->name), NameConvention::LowerKabab, buf); + buf << "\n"; + sink->diagnoseRaw(Severity::Note, buf.getUnownedSlice()); } return SLANG_OK; @@ -1038,31 +1115,45 @@ struct OptionsParser { requestImpl->getSink()->setFlag(DiagnosticSink::Flag::VerbosePath); } + else if (argValue == "-dump-warning-diagnostics") + { + _dumpDiagnostics(Severity::Warning, sink); + } else if (argValue == "-warnings-as-errors") { CommandLineArg operand; SLANG_RETURN_ON_FAIL(reader.expectArg(operand)); if (operand.value == "all") + { + // TODO(JS): + // Perhaps there needs to be a way to disable this selectively. requestImpl->getSink()->setFlag(DiagnosticSink::Flag::TreatWarningsAsErrors); + } else { - if (SLANG_FAILED(overrideDiagnosticSeverity(operand.value, Severity::Error))) - { - sink->diagnose(operand.loc, MiscDiagnostics::invalidArgumentForOption, "-warnings-as-errors"); - return SLANG_FAIL; - } + SLANG_RETURN_ON_FAIL(_overrideDiagnostics(operand.value.getUnownedSlice(), Severity::Warning, Severity::Error, sink)); } } else if (argValue == "-warnings-disable") { CommandLineArg operand; SLANG_RETURN_ON_FAIL(reader.expectArg(operand)); + SLANG_RETURN_ON_FAIL(_overrideDiagnostics(operand.value.getUnownedSlice(), Severity::Warning, Severity::Disable, sink)); + } + else if (argValue.startsWith(toSlice("-W"))) + { + auto name = argValue.getUnownedSlice().tail(2); - if (SLANG_FAILED(overrideDiagnosticSeverity(operand.value, Severity::Disable))) + // If prefixed with 'no-', disable the warning + if (name.startsWith(toSlice("no-"))) { - sink->diagnose(operand.loc, MiscDiagnostics::invalidArgumentForOption, "-warnings-disable"); - return SLANG_FAIL; + SLANG_RETURN_ON_FAIL(_overrideDiagnostic(name.tail(3), Severity::Warning, Severity::Disable, sink)); + } + else + { + // Enable the warning + SLANG_RETURN_ON_FAIL(_overrideDiagnostic(name, Severity::Warning, Severity::Warning, sink)); } } else if (argValue == "-verify-debug-serial-ir") diff --git a/tests/compute/disable-warning.slang b/tests/compute/disable-warning.slang new file mode 100644 index 000000000..2fd2bde61 --- /dev/null +++ b/tests/compute/disable-warning.slang @@ -0,0 +1,17 @@ +//TEST(smoke,compute):COMPARE_COMPUTE: -shaderobj -Xslang... -Wno-unrecommended-implicit-conversion -X. +//TEST(smoke,compute):COMPARE_COMPUTE:-cpu -shaderobj -Xslang... -Wno-unrecommended-implicit-conversion -X. + +// Test to check warning disable + +//TEST_INPUT:ubuffer(data=[0 0 0 0], stride=4):out,name=outputBuffer +RWStructuredBuffer outputBuffer; + +[numthreads(4, 1, 1)] +void computeMain(uint3 dispatchThreadID : SV_DispatchThreadID) +{ + uint v = dispatchThreadID.x; + // This coercion loses data and would normally produce a warning + int v1 = v; + + outputBuffer[dispatchThreadID.x] = v1; +} \ No newline at end of file diff --git a/tests/compute/disable-warning.slang.expected.txt b/tests/compute/disable-warning.slang.expected.txt new file mode 100644 index 000000000..bc856dafa --- /dev/null +++ b/tests/compute/disable-warning.slang.expected.txt @@ -0,0 +1,4 @@ +0 +1 +2 +3 diff --git a/tools/render-test/slang-support.cpp b/tools/render-test/slang-support.cpp index ed7bb752e..5b2d69bf4 100644 --- a/tools/render-test/slang-support.cpp +++ b/tools/render-test/slang-support.cpp @@ -85,9 +85,20 @@ void ShaderCompilerUtil::Output::reset() args.add(arg.value.getBuffer()); } + // If there are additional args parse them if (args.getCount()) { - SLANG_RETURN_ON_FAIL(spProcessCommandLineArguments(slangRequest, args.getBuffer(), int(args.getCount()))); + const auto res = slangRequest->processCommandLineArguments(args.getBuffer(), int(args.getCount())); + + // If there is a parse failure and diagnostic, output it + if (SLANG_FAILED(res)) + { + if (auto diagnostics = slangRequest->getDiagnosticOutput()) + { + fprintf(stderr, "%s", diagnostics); + } + return res; + } } } diff --git a/tools/slang-cpp-extractor/cpp-extractor-main.cpp b/tools/slang-cpp-extractor/cpp-extractor-main.cpp index 6fbba20c3..c8a4d80f5 100644 --- a/tools/slang-cpp-extractor/cpp-extractor-main.cpp +++ b/tools/slang-cpp-extractor/cpp-extractor-main.cpp @@ -246,7 +246,7 @@ SlangResult App::execute(const Options& options) { StringBuilder buf; // Let's guess a 'fileMark' (it becomes part of the filename) based on the macro name. Use lower kabab. - NameConventionUtil::join(slices.getBuffer(), slices.getCount(), CharCase::Lower, NameConvention::Kabab, buf); + NameConventionUtil::join(slices.getBuffer(), slices.getCount(), NameConvention::LowerKabab, buf); typeSet->m_fileMark = buf.ProduceString(); } @@ -254,7 +254,7 @@ SlangResult App::execute(const Options& options) { // Let's guess a typename if not set -> go with upper camel StringBuilder buf; - NameConventionUtil::join(slices.getBuffer(), slices.getCount(), CharCase::Upper, NameConvention::Camel, buf); + NameConventionUtil::join(slices.getBuffer(), slices.getCount(), NameConvention::UpperCamel, buf); typeSet->m_typeName = buf.ProduceString(); } } diff --git a/tools/slang-cpp-extractor/node-tree.cpp b/tools/slang-cpp-extractor/node-tree.cpp index f21d912a7..546a54c27 100644 --- a/tools/slang-cpp-extractor/node-tree.cpp +++ b/tools/slang-cpp-extractor/node-tree.cpp @@ -79,7 +79,7 @@ SourceOrigin* NodeTree::addSourceOrigin(SourceFile* sourceFile, const Options& o slice = slice.trim('-'); StringBuilder out; - NameConventionUtil::convert(slice, CharCase::Upper, NameConvention::Snake, out); + NameConventionUtil::convert(slice, NameConvention::UpperSnake, out); return out; } -- cgit v1.2.3