Skip to content

Commit f2b20fd

Browse files
bgamarimartijnbastiaan
authored andcommitted
prettyBlackBox: Correctly escape [ and ]
The way that way that blackboxes of function arguments to higher order blackboxes are handled is somewhat weird. For historical reasons, they are parsed, pretty-printed, later re-parsed again. However, the pretty-printer failed to correctly re-escape previously escaped square brackets, resulting in the generation of invalid HDL, as seen in #2809. Fixes #2809. Fixes #2988.
1 parent 85c07cf commit f2b20fd

File tree

6 files changed

+36
-2
lines changed

6 files changed

+36
-2
lines changed
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
FIXED: Verilog and System Verilog code gen bug for `map head` [#2809](https://github.com/clash-lang/clash-compiler/issues/2809)
2+
FIXED: Error parsing blackbox: `Clash.Sized.Vector.head` [#2988](https://github.com/clash-lang/clash-compiler/issues/2988)

clash-lib/src/Clash/Netlist/BlackBox/Parser.hs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,8 @@ pElemE = pTagE
150150

151151
-- | Parse SigD
152152
pSigD :: Parser [Element]
153-
pSigD = some (pTagE <|> (Text (pack "[") <$ (pack <$> string "[\\"))
154-
<|> (Text (pack "]") <$ (pack <$> string "\\]"))
153+
pSigD = some (pTagE <|> (EscapedSymbol SquareBracketOpen <$ string "[\\")
154+
<|> (EscapedSymbol SquareBracketClose <$ string "\\]")
155155
<|> (Text <$> (pack <$> some (satisfyRange '\000' '\90')))
156156
<|> (Text <$> (pack <$> some (satisfyRange '\94' '\125'))))
157157

clash-lib/src/Clash/Netlist/BlackBox/Types.hs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ module Clash.Netlist.BlackBox.Types
1919
, BlackBoxTemplate
2020
, TemplateKind (..)
2121
, Element(..)
22+
, EscapedSymbol(..)
2223
, Decl(..)
2324
, HdlSyn(..)
2425
, RenderVoid(..)
@@ -211,6 +212,12 @@ data Element
211212
| CtxName
212213
-- ^ The "context name", name set by `Clash.Magic.setName`, defaults to the
213214
-- name of the closest binder
215+
| EscapedSymbol EscapedSymbol
216+
-- ^ Used for "[\" and "\]", they'll be rendered as "[" and "]",
217+
-- but pretty printed as "[\" and "\]".
218+
deriving (Show, Generic, NFData, Binary, Eq, Hashable)
219+
220+
data EscapedSymbol = SquareBracketOpen | SquareBracketClose
214221
deriving (Show, Generic, NFData, Binary, Eq, Hashable)
215222

216223
-- | Component instantiation hole. First argument indicates which function argument

clash-lib/src/Clash/Netlist/BlackBox/Util.hs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ inputHole = \case
140140
DevNull _ -> Nothing
141141
SigD _ nM -> nM
142142
CtxName -> Nothing
143+
EscapedSymbol _ -> Nothing
143144

144145
-- | Determine if the number of normal\/literal\/function inputs of a blackbox
145146
-- context at least matches the number of argument that is expected by the
@@ -923,6 +924,9 @@ renderTag b CtxName = case bbCtxName b of
923924
-> return (Id.toLazyText t)
924925
_ -> error "internal error"
925926

927+
renderTag _ (EscapedSymbol sym) = case sym of
928+
SquareBracketOpen -> return "["
929+
SquareBracketClose -> return "]"
926930

927931
renderTag _ e = do e' <- getAp (prettyElem e)
928932
error $ $(curLoc) ++ "Unable to evaluate: " ++ show e'
@@ -1127,6 +1131,9 @@ prettyElem (Template bbname source) = do
11271131
<> brackets (string $ Text.concat bbname')
11281132
<> brackets (string $ Text.concat source'))
11291133
prettyElem CtxName = return "~CTXNAME"
1134+
prettyElem (EscapedSymbol sym) = case sym of
1135+
SquareBracketOpen -> return "[\\"
1136+
SquareBracketClose -> return "\\]"
11301137

11311138
-- | Recursively walk @Element@, applying @f@ to each element in the tree.
11321139
walkElement
@@ -1197,6 +1204,7 @@ walkElement f el = maybeToList (f el) ++ walked
11971204
Repeat es1 es2 ->
11981205
concatMap go es1 ++ concatMap go es2
11991206
CtxName -> []
1207+
EscapedSymbol _ -> []
12001208

12011209
-- | Determine variables used in an expression. Used for VHDL sensitivity list.
12021210
-- Also see: https://github.com/clash-lang/clash-compiler/issues/365
@@ -1285,6 +1293,7 @@ getUsedArguments (N.BBTemplate t) = nub (concatMap (walkElement matchArg) t)
12851293
TypM _ -> Nothing
12861294
Vars _ -> Nothing
12871295
CtxName -> Nothing
1296+
EscapedSymbol _ -> Nothing
12881297

12891298
onBlackBox
12901299
:: (BlackBoxTemplate -> r)

tests/Main.hs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -668,6 +668,7 @@ runClashTest = defaultMain
668668
, runTest "T2845" def{hdlSim=[],hdlTargets=[Verilog]}
669669
, runTest "T2904" def
670670
, runTest "T2966" def{hdlSim=[],hdlTargets=[Verilog]}
671+
, runTest "T2988" def{hdlSim=[]}
671672
] <>
672673
if compiledWith == Cabal then
673674
-- This tests fails without environment files present, which are only

tests/shouldwork/Issues/T2988.hs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
{-# LANGUAGE DataKinds #-}
2+
{-# LANGUAGE CPP #-}
3+
4+
module T2988 where
5+
6+
import Clash.Prelude
7+
import Prelude ()
8+
9+
topEntity :: Signal System (Vec 4 (Vec 4 (Unsigned 32))) -> Signal System (Vec 4 (Unsigned 32))
10+
topEntity = f
11+
{-# CLASH_OPAQUE topEntity #-}
12+
13+
f :: Signal System (Vec n (Vec 4 (Unsigned 32))) -> Signal System (Vec n (Unsigned 32))
14+
f x = fmap (fmap head) x
15+
{-# CLASH_OPAQUE f #-}

0 commit comments

Comments
 (0)