From 95dabd900e6fcb2314208bbc2a2f3c2c2395a68c Mon Sep 17 00:00:00 2001 From: Jozufozu Date: Wed, 14 Jul 2021 15:20:49 -0700 Subject: [PATCH] Better errors still - Actually can print something that underlines a span - Resolve imports and use the resolutions during building - Doesn't actually make sense to have #checkErrors - Simplify some regexes - Parse structs --- .../flywheel/backend/ShaderSources.java | 2 + .../flywheel/backend/pipeline/Includer.java | 13 ++-- .../flywheel/backend/pipeline/SourceFile.java | 70 +++++++++++++++++-- .../backend/pipeline/error/ErrorBuilder.java | 58 ++++++++++++++- .../backend/pipeline/error/ErrorReporter.java | 11 ++- .../pipeline/parse/AbstractShaderElement.java | 2 - .../backend/pipeline/parse/Include.java | 28 +++++--- .../pipeline/parse/ShaderFunction.java | 5 -- .../backend/pipeline/parse/ShaderStruct.java | 15 ++-- .../backend/pipeline/parse/StructField.java | 19 ++--- .../backend/pipeline/parse/Variable.java | 5 -- .../flywheel/backend/pipeline/span/Span.java | 19 +++++ .../jozufozu/flywheel/util/StringUtil.java | 12 ++++ 13 files changed, 197 insertions(+), 62 deletions(-) create mode 100644 src/main/java/com/jozufozu/flywheel/util/StringUtil.java diff --git a/src/main/java/com/jozufozu/flywheel/backend/ShaderSources.java b/src/main/java/com/jozufozu/flywheel/backend/ShaderSources.java index d21dc3b3c..df35aa754 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/ShaderSources.java +++ b/src/main/java/com/jozufozu/flywheel/backend/ShaderSources.java @@ -89,6 +89,8 @@ public class ShaderSources implements ISelectiveResourceReloadListener { loadProgramSpecs(manager); loadShaderSources(manager); + shaderSources.values().forEach(SourceFile::resolveIncludes); + WorldShaderPipeline pl = new WorldShaderPipeline<>(this); SourceFile source = source(new ResourceLocation(Flywheel.ID, "model.glsl")); diff --git a/src/main/java/com/jozufozu/flywheel/backend/pipeline/Includer.java b/src/main/java/com/jozufozu/flywheel/backend/pipeline/Includer.java index 022854218..3da3da6a0 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/pipeline/Includer.java +++ b/src/main/java/com/jozufozu/flywheel/backend/pipeline/Includer.java @@ -15,28 +15,25 @@ import net.minecraft.util.ResourceLocation; public class Includer { public static List recurseIncludes(SourceFile from) { - ShaderSources sources = from.getParent(); - Set seen = new HashSet<>(); seen.add(from.name); List out = new ArrayList<>(); - process(sources, seen, out, from); + process(seen, out, from); return out; } - private static void process(ShaderSources sources, Set seen, List out, SourceFile source) { + private static void process(Set seen, List out, SourceFile source) { ImmutableList includes = source.getIncludes(); for (Include include : includes) { - if (seen.add(include.getFile())) { - SourceFile file = sources.source(include.getFile()); - - process(sources, seen, out, file); + SourceFile file = include.getTarget(); + if (file != null && seen.add(file.name)) { + process(seen, out, file); out.add(file); } diff --git a/src/main/java/com/jozufozu/flywheel/backend/pipeline/SourceFile.java b/src/main/java/com/jozufozu/flywheel/backend/pipeline/SourceFile.java index 4c6ad0c42..0ccec9693 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/pipeline/SourceFile.java +++ b/src/main/java/com/jozufozu/flywheel/backend/pipeline/SourceFile.java @@ -10,20 +10,23 @@ import java.util.regex.Pattern; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.jozufozu.flywheel.backend.ShaderSources; +import com.jozufozu.flywheel.backend.pipeline.error.ErrorReporter; +import com.jozufozu.flywheel.backend.pipeline.parse.AbstractShaderElement; import com.jozufozu.flywheel.backend.pipeline.parse.Include; import com.jozufozu.flywheel.backend.pipeline.parse.ShaderFunction; +import com.jozufozu.flywheel.backend.pipeline.parse.ShaderStruct; import com.jozufozu.flywheel.backend.pipeline.span.CharPos; import com.jozufozu.flywheel.backend.pipeline.span.ErrorSpan; import com.jozufozu.flywheel.backend.pipeline.span.Span; import com.jozufozu.flywheel.backend.pipeline.span.StringSpan; +import com.jozufozu.flywheel.util.StringUtil; import it.unimi.dsi.fastutil.ints.IntArrayList; import it.unimi.dsi.fastutil.ints.IntList; import net.minecraft.util.ResourceLocation; public class SourceFile { - // #use "valid_namespace:valid/path_to_file.glsl" - private static final Pattern includePattern = Pattern.compile("#use \"(\\w+:[\\w./]+)\""); + private static final Pattern includePattern = Pattern.compile("#use \"(.*)\""); private static final Pattern newLine = Pattern.compile("(\\r\\n|\\r|\\n)"); @@ -34,11 +37,13 @@ public class SourceFile { private final ShaderSources parent; private final String source; + private final ImmutableList lines; private final IntList lineStarts; // Function name -> Function object private final ImmutableMap functions; + private final ImmutableMap structs; // Includes ordered as defined in the source private final ImmutableList includes; @@ -51,10 +56,12 @@ public class SourceFile { this.name = name; this.source = source; - this.lineStarts = getLinePositions(); + this.lineStarts = createLineStarts(); + this.lines = createLineList(lineStarts); - this.functions = parseFunctions(); this.includes = parseIncludes(); + this.functions = parseFunctions(); + this.structs = parseStructs(); } public String getSource() { @@ -73,6 +80,12 @@ public class SourceFile { return includes; } + public void resolveIncludes() { + for (Include include : includes) { + include.resolve(); + } + } + public CharPos getCharPos(int charPos) { int lineNo = 0; for (; lineNo < lineStarts.size(); lineNo++) { @@ -83,7 +96,9 @@ public class SourceFile { } } - int lineStart = lineStarts.getInt(lineNo - 1); + lineNo -= 1; + + int lineStart = lineStarts.getInt(lineNo); return new CharPos(charPos, lineNo, charPos - lineStart); } @@ -95,7 +110,7 @@ public class SourceFile { .append(name) .append("':\n"); int i = 1; - for (String s : source.split("\n")) { + for (String s : lines) { builder.append(String.format("%1$4s: ", i++)) .append(s) .append('\n'); @@ -129,7 +144,7 @@ public class SourceFile { /** * Scan the source for line breaks, recording the position of the first character of each line. */ - private IntList getLinePositions() { + private IntList createLineStarts() { IntList l = new IntArrayList(); l.add(0); // first line is always at position 0 @@ -141,6 +156,19 @@ public class SourceFile { return l; } + private ImmutableList createLineList(IntList lines) { + ImmutableList.Builder builder = ImmutableList.builder(); + + for (int i = 1; i < lines.size(); i++) { + int start = lines.getInt(i - 1); + int end = lines.getInt(i); + + builder.add(StringUtil.trimEnd(source.substring(start, end))); + } + + return builder.build(); + } + /** * Scan the source for function definitions and "parse" them into objects that contain properties of the function. */ @@ -175,6 +203,26 @@ public class SourceFile { return ImmutableMap.copyOf(functions); } + /** + * Scan the source for function definitions and "parse" them into objects that contain properties of the function. + */ + private ImmutableMap parseStructs() { + Matcher matcher = ShaderStruct.struct.matcher(source); + + ImmutableMap.Builder functions = ImmutableMap.builder(); + while (matcher.find()) { + Span self = Span.fromMatcher(this, matcher); + Span name = Span.fromMatcher(this, matcher, 1); + Span body = Span.fromMatcher(this, matcher, 2); + + ShaderStruct shaderStruct = new ShaderStruct(self, name, body); + + functions.put(body.get(), shaderStruct); + } + + return functions.build(); + } + /** * Scan the source for #use "..." directives. * Records the contents of the directive into an {@link Include} object, and marks the directive for elision. @@ -214,4 +262,12 @@ public class SourceFile { return -1; } + + public int getLineCount() { + return lines.size(); + } + + public CharSequence getLine(int lineNo) { + return lines.get(lineNo); + } } diff --git a/src/main/java/com/jozufozu/flywheel/backend/pipeline/error/ErrorBuilder.java b/src/main/java/com/jozufozu/flywheel/backend/pipeline/error/ErrorBuilder.java index 5f706a962..72115c9c6 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/pipeline/error/ErrorBuilder.java +++ b/src/main/java/com/jozufozu/flywheel/backend/pipeline/error/ErrorBuilder.java @@ -1,10 +1,11 @@ package com.jozufozu.flywheel.backend.pipeline.error; import com.jozufozu.flywheel.backend.pipeline.SourceFile; +import com.jozufozu.flywheel.backend.pipeline.span.Span; public class ErrorBuilder { - private StringBuilder internal; + private final StringBuilder internal = new StringBuilder(); public ErrorBuilder header(CharSequence msg) { internal.append("error: ") @@ -18,7 +19,15 @@ public class ErrorBuilder { return endLine(); } - public ErrorBuilder line(int no, CharSequence content) { + public ErrorBuilder numberedLine(int no, CharSequence content) { + return line(String.valueOf(no), content); + } + + public ErrorBuilder line(CharSequence leftColumn, CharSequence rightColumn) { + + internal.append(leftColumn) + .append(" | ") + .append(rightColumn); return endLine(); } @@ -27,4 +36,49 @@ public class ErrorBuilder { internal.append('\n'); return this; } + + public ErrorBuilder pointAt(Span span, int ctxLines) { + SourceFile file = span.getSourceFile(); + + if (span.lines() == 1) { + + int spanLine = span.firstLine(); + + int firstLine = Math.max(0, spanLine - ctxLines); + int lastLine = Math.min(file.getLineCount(), spanLine + ctxLines); + + + int firstCol = span.getStart().getCol(); + int lastCol = span.getEnd().getCol(); + + for (int i = firstLine; i <= lastLine; i++) { + CharSequence line = file.getLine(i); + + numberedLine(i + 1, line); + + if (i == spanLine) { + line(" ", generateUnderline(firstCol, lastCol)); + } + } + } + + return this; + } + + public CharSequence build() { + return internal; + } + + public CharSequence generateUnderline(int firstCol, int lastCol) { + StringBuilder line = new StringBuilder(lastCol); + for (int i = 0; i < firstCol; i++) { + line.append(' '); + } + + for (int i = firstCol; i < lastCol; i++) { + line.append('^'); + } + + return line; + } } diff --git a/src/main/java/com/jozufozu/flywheel/backend/pipeline/error/ErrorReporter.java b/src/main/java/com/jozufozu/flywheel/backend/pipeline/error/ErrorReporter.java index cdbda6169..d124a009a 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/pipeline/error/ErrorReporter.java +++ b/src/main/java/com/jozufozu/flywheel/backend/pipeline/error/ErrorReporter.java @@ -1,16 +1,21 @@ package com.jozufozu.flywheel.backend.pipeline.error; +import com.jozufozu.flywheel.backend.Backend; import com.jozufozu.flywheel.backend.pipeline.SourceFile; import com.jozufozu.flywheel.backend.pipeline.span.Span; public class ErrorReporter { - - public String generateSpanError(Span span, String message) { + public static void generateSpanError(Span span, String message) { SourceFile file = span.getSourceFile(); + ErrorBuilder builder = new ErrorBuilder(); + CharSequence error = builder.header(message) + .errorIn(file) + .pointAt(span, 2) + .build(); - return ""; + Backend.log.info(error); } } diff --git a/src/main/java/com/jozufozu/flywheel/backend/pipeline/parse/AbstractShaderElement.java b/src/main/java/com/jozufozu/flywheel/backend/pipeline/parse/AbstractShaderElement.java index 6f73370f5..e7a623158 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/pipeline/parse/AbstractShaderElement.java +++ b/src/main/java/com/jozufozu/flywheel/backend/pipeline/parse/AbstractShaderElement.java @@ -1,6 +1,5 @@ package com.jozufozu.flywheel.backend.pipeline.parse; -import com.jozufozu.flywheel.backend.pipeline.error.ErrorReporter; import com.jozufozu.flywheel.backend.pipeline.span.Span; public abstract class AbstractShaderElement { @@ -11,5 +10,4 @@ public abstract class AbstractShaderElement { this.self = self; } - public abstract void checkErrors(ErrorReporter e); } diff --git a/src/main/java/com/jozufozu/flywheel/backend/pipeline/parse/Include.java b/src/main/java/com/jozufozu/flywheel/backend/pipeline/parse/Include.java index 9d1f38504..497b99240 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/pipeline/parse/Include.java +++ b/src/main/java/com/jozufozu/flywheel/backend/pipeline/parse/Include.java @@ -2,6 +2,8 @@ package com.jozufozu.flywheel.backend.pipeline.parse; import java.util.Optional; +import javax.annotation.Nullable; + import com.jozufozu.flywheel.backend.ShaderSources; import com.jozufozu.flywheel.backend.pipeline.error.ErrorReporter; import com.jozufozu.flywheel.backend.pipeline.SourceFile; @@ -14,37 +16,43 @@ public class Include extends AbstractShaderElement { private final ShaderSources sources; private Span file; + private ResourceLocation fileLoc; private SourceFile resolution; + public Include(ShaderSources sources, Span self, Span file) { super(self); this.sources = sources; this.file = file; + + try { + fileLoc = new ResourceLocation(file.get()); + } catch (RuntimeException error) { + ErrorReporter.generateSpanError(file, "malformed source name"); + } } public boolean isResolved() { return resolution != null; } - public Optional getTarget() { - return Optional.ofNullable(resolution); + @Nullable + public SourceFile getTarget() { + return resolution; } public ResourceLocation getFile() { - return new ResourceLocation(file.get()); + return fileLoc; } - @Override - public void checkErrors(ErrorReporter e) { + public void resolve() { - String name = file.get(); + if (fileLoc == null) return; try { - ResourceLocation loc = new ResourceLocation(name); - resolution = sources.source(loc); + resolution = sources.source(fileLoc); } catch (RuntimeException error) { - + ErrorReporter.generateSpanError(file, "could not find source"); } - } } diff --git a/src/main/java/com/jozufozu/flywheel/backend/pipeline/parse/ShaderFunction.java b/src/main/java/com/jozufozu/flywheel/backend/pipeline/parse/ShaderFunction.java index ab2a8176a..4c4276e55 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/pipeline/parse/ShaderFunction.java +++ b/src/main/java/com/jozufozu/flywheel/backend/pipeline/parse/ShaderFunction.java @@ -61,9 +61,4 @@ public class ShaderFunction extends AbstractShaderElement { return type + " " + name + "(" + p + ")"; } - - @Override - public void checkErrors(ErrorReporter e) { - - } } diff --git a/src/main/java/com/jozufozu/flywheel/backend/pipeline/parse/ShaderStruct.java b/src/main/java/com/jozufozu/flywheel/backend/pipeline/parse/ShaderStruct.java index 01820d8b2..42c8a6570 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/pipeline/parse/ShaderStruct.java +++ b/src/main/java/com/jozufozu/flywheel/backend/pipeline/parse/ShaderStruct.java @@ -1,5 +1,6 @@ package com.jozufozu.flywheel.backend.pipeline.parse; +import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -12,8 +13,8 @@ import com.jozufozu.flywheel.backend.pipeline.span.Span; public class ShaderStruct extends AbstractShaderElement { - // https://regexr.com/5t207 - public static final Pattern struct = Pattern.compile("struct\\s+([\\w\\d]*)\\s*\\{([\\w\\d \\t#\\[\\](),;\\n]*)}\\s*;"); + // https://regexr.com/61rpe + public static final Pattern struct = Pattern.compile("struct\\s+([\\w\\d]*)\\s*\\{([\\w\\d\\s,;]*)}\\s*;\\s"); public final Span name; public final Span body; @@ -26,18 +27,16 @@ public class ShaderStruct extends AbstractShaderElement { this.name = name; this.body = body; this.fields = parseFields(); + this.fields2Types = createTypeLookup(); + } + private ImmutableMap createTypeLookup() { ImmutableMap.Builder lookup = ImmutableMap.builder(); for (StructField field : fields) { lookup.put(field.name.get(), field.type); } - this.fields2Types = lookup.build(); - } - - @Override - public void checkErrors(ErrorReporter e) { - + return lookup.build(); } private ImmutableList parseFields() { diff --git a/src/main/java/com/jozufozu/flywheel/backend/pipeline/parse/StructField.java b/src/main/java/com/jozufozu/flywheel/backend/pipeline/parse/StructField.java index bf61c8eb1..391592fc7 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/pipeline/parse/StructField.java +++ b/src/main/java/com/jozufozu/flywheel/backend/pipeline/parse/StructField.java @@ -8,30 +8,25 @@ import com.jozufozu.flywheel.backend.pipeline.span.Span; public class StructField extends AbstractShaderElement { public static final Pattern fieldPattern = Pattern.compile("(\\S+)\\s*(\\S+);"); - public Span name; public Span type; + public Span name; - public StructField(Span self, Span name, Span type) { + public StructField(Span self, Span type, Span name) { super(self); - this.name = name; this.type = type; - } - - public Span getName() { - return name; + this.name = name; } public Span getType() { return type; } + public Span getName() { + return name; + } + @Override public String toString() { return "TaggedField{" + "name='" + name + '\'' + ", type='" + type + '\'' + '}'; } - - @Override - public void checkErrors(ErrorReporter e) { - - } } diff --git a/src/main/java/com/jozufozu/flywheel/backend/pipeline/parse/Variable.java b/src/main/java/com/jozufozu/flywheel/backend/pipeline/parse/Variable.java index 1b0c5226f..b081499ee 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/pipeline/parse/Variable.java +++ b/src/main/java/com/jozufozu/flywheel/backend/pipeline/parse/Variable.java @@ -21,9 +21,4 @@ public class Variable extends AbstractShaderElement { public Span getName() { return name; } - - @Override - public void checkErrors(ErrorReporter e) { - - } } diff --git a/src/main/java/com/jozufozu/flywheel/backend/pipeline/span/Span.java b/src/main/java/com/jozufozu/flywheel/backend/pipeline/span/Span.java index e635b1c98..f8355c16f 100644 --- a/src/main/java/com/jozufozu/flywheel/backend/pipeline/span/Span.java +++ b/src/main/java/com/jozufozu/flywheel/backend/pipeline/span/Span.java @@ -30,6 +30,14 @@ public abstract class Span implements CharSequence { return in; } + public CharPos getStart() { + return start; + } + + public CharPos getEnd() { + return end; + } + /** * @return the string index at the (inclusive) beginning of this code segment. */ @@ -51,6 +59,17 @@ public abstract class Span implements CharSequence { return start == end; } + /** + * @return how many lines this span spans. + */ + public int lines() { + return end.getLine() - start.getLine() + 1; + } + + public int firstLine() { + return start.getLine(); + } + /** * Get a span referring to a code segment inside this code segment. */ diff --git a/src/main/java/com/jozufozu/flywheel/util/StringUtil.java b/src/main/java/com/jozufozu/flywheel/util/StringUtil.java new file mode 100644 index 000000000..e451a8d43 --- /dev/null +++ b/src/main/java/com/jozufozu/flywheel/util/StringUtil.java @@ -0,0 +1,12 @@ +package com.jozufozu.flywheel.util; + +public class StringUtil { + public static String trimEnd(String value) { + int len = value.length(); + int st = 0; + while ((st < len) && Character.isWhitespace(value.charAt(len - 1))) { + len--; + } + return value.substring(0, len); + } +}