Skip to content

Commit dafe651

Browse files
authored
Improvements around diagnostic controls (shader-slang#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.
1 parent e449446 commit dafe651

12 files changed

+411
-115
lines changed

source/compiler-core/slang-diagnostic-sink.cpp

+64-29
Original file line numberDiff line numberDiff line change
@@ -667,45 +667,88 @@ void DiagnosticSink::diagnoseRaw(
667667
}
668668
}
669669

670-
/* !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! DiagnosticLookup !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! */
671-
672-
void DiagnosticsLookup::_add(const char* name, Index index)
670+
void DiagnosticSink::overrideDiagnosticSeverity(int diagnosticId, Severity overrideSeverity, const DiagnosticInfo* info)
673671
{
674-
UnownedStringSlice nameSlice(name);
675-
m_map.Add(nameSlice, index);
676-
677-
// Add a dashed version (KababCase)
672+
if (info)
678673
{
679-
m_work.Clear();
674+
SLANG_ASSERT(info->id == diagnosticId);
680675

681-
NameConventionUtil::convert(NameConvention::Camel, nameSlice, CharCase::Lower, NameConvention::Kabab, m_work);
676+
// If the override is the same as the default, we can just remove the override
677+
if (info->severity == overrideSeverity)
678+
{
679+
m_severityOverrides.Remove(diagnosticId);
680+
return;
681+
}
682+
}
682683

683-
UnownedStringSlice dashSlice(m_arena.allocateString(m_work.getBuffer(), m_work.getLength()), m_work.getLength());
684+
// Set the override
685+
m_severityOverrides[diagnosticId] = overrideSeverity;
686+
}
684687

685-
m_map.AddIfNotExists(dashSlice, index);
686-
}
688+
/* !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! DiagnosticLookup !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! */
689+
690+
Index DiagnosticsLookup::_findDiagnosticIndexByExactName(const UnownedStringSlice& slice) const
691+
{
692+
const Index* indexPtr = m_nameMap.TryGetValue(slice);
693+
return indexPtr ? *indexPtr : -1;
694+
}
695+
696+
void DiagnosticsLookup::_addName(const char* name, Index diagnosticIndex)
697+
{
698+
UnownedStringSlice nameSlice(name);
699+
m_nameMap.Add(nameSlice, diagnosticIndex);
687700
}
701+
688702
void DiagnosticsLookup::addAlias(const char* name, const char* diagnosticName)
689703
{
690-
const Index index = _findDiagnosticIndex(UnownedStringSlice(diagnosticName));
704+
const Index index = _findDiagnosticIndexByExactName(UnownedStringSlice(diagnosticName));
691705
SLANG_ASSERT(index >= 0);
692706
if (index >= 0)
693707
{
694-
_add(name, index);
708+
_addName(name, index);
695709
}
696710
}
697711

712+
const DiagnosticInfo* DiagnosticsLookup::getDiagnosticById(Int id) const
713+
{
714+
const auto indexPtr = m_idMap.TryGetValue(id);
715+
return indexPtr ? m_diagnostics[*indexPtr] : nullptr;
716+
}
717+
718+
const DiagnosticInfo* DiagnosticsLookup::findDiagnosticByExactName(const UnownedStringSlice& slice) const
719+
{
720+
const Index* indexPtr = m_nameMap.TryGetValue(slice);
721+
return indexPtr ? m_diagnostics[*indexPtr] : nullptr;
722+
}
723+
724+
const DiagnosticInfo* DiagnosticsLookup::findDiagnosticByName(const UnownedStringSlice& slice) const
725+
{
726+
const auto convention = NameConventionUtil::inferConventionFromText(slice);
727+
switch (convention)
728+
{
729+
case NameConvention::Invalid: return nullptr;
730+
case NameConvention::LowerCamel: return findDiagnosticByExactName(slice);
731+
default: break;
732+
}
733+
734+
StringBuilder buf;
735+
NameConventionUtil::convert(getNameStyle(convention), slice, NameConvention::LowerCamel, buf);
736+
737+
return findDiagnosticByExactName(buf.getUnownedSlice());
738+
}
739+
698740
Index DiagnosticsLookup::add(const DiagnosticInfo* info)
699741
{
700742
// Check it's not already added
701743
SLANG_ASSERT(m_diagnostics.indexOf(info) < 0);
702744

703-
const Index index = m_diagnostics.getCount();
704-
705-
_add(info->name, index);
706-
745+
const Index diagnosticIndex = m_diagnostics.getCount();
707746
m_diagnostics.add(info);
708-
return index;
747+
748+
_addName(info->name, diagnosticIndex);
749+
m_idMap.AddIfNotExists(info->id, diagnosticIndex);
750+
751+
return diagnosticIndex;
709752
}
710753

711754
void DiagnosticsLookup::add(const DiagnosticInfo*const* infos, Index infosCount)
@@ -724,21 +767,13 @@ DiagnosticsLookup::DiagnosticsLookup():
724767
DiagnosticsLookup::DiagnosticsLookup(const DiagnosticInfo*const* diagnostics, Index diagnosticsCount) :
725768
m_arena(kArenaInitialSize)
726769
{
727-
m_diagnostics.addRange(diagnostics, diagnosticsCount);
728-
729770
// TODO: We should eventually have a more formal system for associating individual
730771
// diagnostics, or groups of diagnostics, with user-exposed names for use when
731772
// enabling/disabling warnings (or turning warnings into errors, etc.).
732773
//
733-
// For now we build a map from diagnostic name to it's entry. Two entries are typically
734-
// added - the 'original name' as associated with the diagnostic in lowerCamel, and
735-
// a dashified version.
774+
// For now we build a map from diagnostic name to it's entry.
736775

737-
for (Index i = 0; i < diagnosticsCount; ++i)
738-
{
739-
const DiagnosticInfo* diagnostic = diagnostics[i];
740-
_add(diagnostic->name, i);
741-
}
776+
add(diagnostics, diagnosticsCount);
742777
}
743778

744779
} // namespace Slang

source/compiler-core/slang-diagnostic-sink.h

+23-16
Original file line numberDiff line numberDiff line change
@@ -227,10 +227,8 @@ class DiagnosticSink
227227
bool isFlagSet(Flag::Enum flag) { return (m_flags & Flags(flag)) != 0; }
228228

229229
/// Sets an override on the severity of a specific diagnostic message (by numeric identifier)
230-
void overrideDiagnosticSeverity(int messageID, Severity overrideSeverity)
231-
{
232-
m_severityOverrides[messageID] = overrideSeverity;
233-
}
230+
/// info can be set to nullptr if only to override
231+
void overrideDiagnosticSeverity(int diagnosticId, Severity overrideSeverity, const DiagnosticInfo* info = nullptr);
234232

235233
/// Get the (optional) diagnostic sink lexer. This is used to
236234
/// improve quality of highlighting a locations token. If not set, will just have a single
@@ -322,37 +320,46 @@ class DiagnosticsLookup : public RefObject
322320
public:
323321
static const Index kArenaInitialSize = 2048;
324322

325-
const DiagnosticInfo* findDiagostic(const UnownedStringSlice& slice) const
326-
{
327-
const Index* indexPtr = m_map.TryGetValue(slice);
328-
return indexPtr ? m_diagnostics[*indexPtr] : nullptr;
329-
}
330-
Index _findDiagnosticIndex(const UnownedStringSlice& slice) const
331-
{
332-
const Index* indexPtr = m_map.TryGetValue(slice);
333-
return indexPtr ? *indexPtr : 0;
334-
}
323+
/// Will take into account the slice name could be using different conventions
324+
const DiagnosticInfo* findDiagnosticByName(const UnownedStringSlice& slice) const;
325+
/// The name must be as defined in the diagnostics exactly, typically lower camel
326+
const DiagnosticInfo* findDiagnosticByExactName(const UnownedStringSlice& slice) const;
327+
328+
/// Get a diagnostic by it's id.
329+
/// NOTE! That it is possible for multiple diagnostics to have the same id. This will return
330+
/// the first added
331+
const DiagnosticInfo* getDiagnosticById(Int id) const;
335332

336333
/// info must stay in scope
337334
Index add(const DiagnosticInfo* info);
335+
/// Infos referenced must remain in scope
338336
void add(const DiagnosticInfo*const* infos, Index infosCount);
339337

338+
/// NOTE! Name must stay in scope as long as the diagnostics lookup.
339+
/// If not possible add it to the arena to keep in scope.
340340
void addAlias(const char* name, const char* diagnosticName);
341341

342342
/// Get the diagnostics held in this lookup
343343
const List<const DiagnosticInfo*>& getDiagnostics() const { return m_diagnostics; }
344344

345+
/// Get the associated arena
346+
MemoryArena& getArena() { return m_arena; }
347+
345348
/// NOTE! diagnostics must stay in scope for lifetime of lookup
346349
DiagnosticsLookup(const DiagnosticInfo*const* diagnostics, Index diagnosticsCount);
347350
DiagnosticsLookup();
348351

349352
protected:
350-
void _add(const char* name, Index index);
353+
void _addName(const char* name, Index diagnosticIndex);
354+
355+
Index _findDiagnosticIndexByExactName(const UnownedStringSlice& slice) const;
351356

352357
List<const DiagnosticInfo*> m_diagnostics;
353358

354359
StringBuilder m_work;
355-
Dictionary<UnownedStringSlice, Index> m_map;
360+
Dictionary<UnownedStringSlice, Index> m_nameMap;
361+
Dictionary<Int, Index> m_idMap;
362+
356363
MemoryArena m_arena;
357364
};
358365

0 commit comments

Comments
 (0)