From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp2.migadu.com ([2001:41d0:700:3204::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms5.migadu.com with LMTPS id oCGmGy9H4mUc+gAAbAwnHQ (envelope-from ) for ; Fri, 01 Mar 2024 22:22:55 +0100 Received: from aspmx1.migadu.com ([2001:41d0:303:e16b::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp2.migadu.com with LMTPS id WH8YFS9H4mWgSQAAe85BDQ (envelope-from ) for ; Fri, 01 Mar 2024 22:22:55 +0100 X-Envelope-To: patches@johnnyrichard.com Authentication-Results: aspmx1.migadu.com; dkim=pass header.d=lists.sr.ht header.s=20240113 header.b=VzhhkbqC; dkim=pass header.d=johnnyrichard.com header.s=key1 header.b=rlSw4iPs; dmarc=pass (policy=quarantine) header.from=johnnyrichard.com; spf=pass (aspmx1.migadu.com: domain of lists@sr.ht designates 46.23.81.152 as permitted sender) smtp.mailfrom=lists@sr.ht ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=johnnyrichard.com; s=key1; t=1709328175; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id: list-unsubscribe:list-subscribe:list-post:dkim-signature; bh=aHeYUNnR9tLvoguitVIDybh5qh6lHV/LqRLRKI+Ejw0=; b=X4Qc2Mg49vndjcEIVH2zH27vGpVSrbfgZfnebp8MS4SoWmP6JL3lNcG6s4K5nD71RNUoYu CHM3IT/nRxaTyGq+o1mFfclVkwsyr8DnLZPg/2+hIVfiJZQArAzW/gLiFsR5cmhxNcyobp wX8lXi2WLQXVIlTRKCkg1w+Xshjqm6pjIGtChsS0GyNdknt2nYcAk+Y7NOwZxkiRz31JWq 7zYBAhANzeivp9t0Vh8BPylC6gt+4njdOfhN9rSfWZwlIfr0M5+1rPTNeeCiFoPXZ/o6wR MN3s5HqdJmzfzvqORGTAhx++AL2/AcrLKu52Vb1qaG8FIVYW59akwD/0WrEZoQ== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=pass header.d=lists.sr.ht header.s=20240113 header.b=VzhhkbqC; dkim=pass header.d=johnnyrichard.com header.s=key1 header.b=rlSw4iPs; dmarc=pass (policy=quarantine) header.from=johnnyrichard.com; spf=pass (aspmx1.migadu.com: domain of lists@sr.ht designates 46.23.81.152 as permitted sender) smtp.mailfrom=lists@sr.ht ARC-Seal: i=1; s=key1; d=johnnyrichard.com; t=1709328175; a=rsa-sha256; cv=none; b=gUlSvjhFQmb951gpWgPrQrCu5QnWbQosoX0DDvWiSlpmi0emzRKIcVoVrOxmFR8wAtrb/9 B/JxXhaXEs/L/IFa5QhTrIu7MXsstfWGTezqecKr5pn2ullga8nstbf6UpEHSUoeiygIzg 51Z6WwEH/kO0ClmiypZEzyRil5teNhxg4d+g/fDnj4VC+q7wEOh5GAmguvBBdlgFP1/Nf6 PGtT+xhr/5r0uWN8VxuDj7r4el+0YhqX+bZEUF4e1tkBIbYrOjV4FPIqjQMmkEovF9bsb1 xJz21bpg5O59yhpq+1e1lUkgf4kW+1A4KDPi+pDchFkh48r+BnaZpNTwXaHTqg== Received: from mail-a.sr.ht (mail-a.sr.ht [46.23.81.152]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id F132B45EB5 for ; Fri, 1 Mar 2024 22:22:51 +0100 (CET) DKIM-Signature: a=rsa-sha256; bh=5NfsLU1LVZhUdAvTBu56/m04PJPyu5fiwUSseK9EJRQ=; c=simple/simple; d=lists.sr.ht; h=Date:From:To:Cc:Subject:References:In-Reply-To:List-Unsubscribe:List-Subscribe:List-Archive:List-Post:List-ID; q=dns/txt; s=20240113; t=1709328171; v=1; b=VzhhkbqCZ3O8GwX/d9L21EjG2wtvyl4cmFLZUIXkBwhiuYxIiv3YkQEx5LIusVR9cN/g843J g0XpspAZataVI/sNclS+8UoI64uRi6F4NRHX9yX/Fh1XpNgm2U0kaelf0x1HoC3rsIVAL/kFQCT XgVh0hD5G9u+risr9+OwlrM69NLNsxeRZqeo0nOFPr3zXau+98hMcLbACMTHyI3X1ybqWuteJlQ T6KLZJECi4i6pMtPnJ0Lz+CgEPTPqaKjBINSD6CC1Rn5wPv8u0Ka87ODb9lQ5zrLZOHdC2uWwRa gc8kyQn5hWgZrFMxyUpRjK6JM7lpFbXNb37t1hdtPfaAQ== Received: from lists.sr.ht (unknown [46.23.81.154]) by mail-a.sr.ht (Postfix) with ESMTPSA id 480F8203FA for ; Fri, 1 Mar 2024 21:22:51 +0000 (UTC) Received: from out-187.mta0.migadu.com (out-187.mta0.migadu.com [91.218.175.187]) by mail-a.sr.ht (Postfix) with ESMTPS id 9E534203F3 for <~johnnyrichard/olang-devel@lists.sr.ht>; Fri, 1 Mar 2024 21:22:50 +0000 (UTC) Date: Fri, 1 Mar 2024 23:23:01 +0100 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=johnnyrichard.com; s=key1; t=1709328169; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=aHeYUNnR9tLvoguitVIDybh5qh6lHV/LqRLRKI+Ejw0=; b=rlSw4iPs00826iTDLiRrg4Xqa6SfIw4nv/TSMBBX00M7fkS2WgkISk6SCJ8ro/8jb++NKp VVw1XyZrybtolJN4LFcBBhqfBEqJJ448Toi27qrk6dBE54WwbX6bTpW9c5eA/lYvu4hOqZ smwoJzumRbg79KLhBFuXA5MI3FNTJh3TpLpi04+f7OO4TF5KLGF+nvcsbp3r9l1R0sNx+g jxi2s1pIQoqluJQsLgBxHqltkKrNHSvjRzYsol6/IusOhWvPUziUG3u/Zoypd/I5BC0OYe aOfjFOQV9SW+sY/o5S5KqvIlA56bER4lJUGmuvHxBei19yuOG8SsrGmNe65mBg== X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Johnny Richard To: Carlos Maniero Cc: ~johnnyrichard/olang-devel@lists.sr.ht Subject: Re: [PATCH olang v1 4/4] parser: create simplified parser for tiny AST Message-ID: References: <20240228190956.78191-1-johnny@johnnyrichard.com> <20240228190956.78191-5-johnny@johnnyrichard.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: List-Unsubscribe: List-Subscribe: List-Archive: Archived-At: List-Post: List-ID: ~johnnyrichard/olang-devel <~johnnyrichard/olang-devel.lists.sr.ht> Sender: ~johnnyrichard/olang-devel <~johnnyrichard/olang-devel@lists.sr.ht> X-Migadu-Flow: FLOW_IN X-Migadu-Country: NL X-Migadu-Spam-Score: -10.02 X-Spam-Score: -10.02 X-Migadu-Queue-Id: F132B45EB5 X-Migadu-Scanner: mx13.migadu.com X-TUID: d7hV0eppZl0h On Fri, Mar 01, 2024 at 12:34:59AM -0300, Carlos Maniero wrote: > > +ast_node_t * > > +ast_make_node_fn_def(arena_t *arena, string_view_t identifier, type_t return_type, ast_node_t *block); > > + > > +ast_node_t * > > +ast_make_node_literal_u32(arena_t *arena, uint32_t value); > > + > > +ast_node_t * > > +ast_make_node_return_stmt(arena_t *arena); > > + > > +ast_node_t * > > +ast_make_node_block(arena_t *arena); > > s/ast_make/ast_new just to keep consistency with other functions that > allocate memory. Sure! > > +void > > +lexer_print_token_highlight(lexer_t *lexer, token_t *token, FILE *stream) > > +{ > > + size_t offset = token->location.bol; > > + char *str = lexer->source.chars + offset; > > + > > + size_t i = 0; > > + while ((i + offset) < lexer->source.size && str[i] != '\n' && str[i] != 0) { > > + ++i; > > + } > > + string_view_t line = { .chars = str, .size = i }; > > + fprintf(stream, "" SV_FMT "\n", SV_ARG(line)); > > + fprintf(stream, "%*s\n", (int)(token->location.offset - token->location.bol + 1), "^"); > > +} > > 1. It bothers me a little that the lexer is performing IO operations. > IMO, the parser should be responsible for error handling. But I can live > with this if you think this is the right place for this function. After analise the code and think about the future typechecker implementation I kind agree with you, I will change in the next patch. > 2. An alternative is making the lexer returning the line *string_view* > and make the parser do the rest. Great, I will do that. > 3. nitpick: Isn't *(i + offset) < lexer->source.size* and *str[i] != 0* > redundant? The last char will (or at least should) always be a NULL > terminator. I will keep the check, I don't see a problem on keeping it. This will make the code more resilient in case of bugs. > 4. I spent some time trying to understand what this function was > supposed to do. I believe that having a function that just returns > the token's line could help (2). But an alternative here is using the > *string_view* struct instead of having *str* and *i*. Something like > this: > > void > lexer_print_token_highlight(lexer_t *lexer, token_t *token, FILE *stream) > { > size_t offset = token->location.bol; > string_view_t line = { .chars = lexer->source.chars + offset, .size = 0 }; > > while (line.chars[i] != '\n' && line.chars[i] != 0) { Ops, you forgot to remove the **i** here. :x > ++line.size; > } > > fprintf(stream, "" SV_FMT "\n", SV_ARG(line)); > fprintf(stream, "%*s\n", (int)(token->location.offset - token->location.bol + 1), "^"); > } Yeah, I will do this change as well, thanks. > > + if (!skip_expected_token(parser, TOKEN_COLON)) > > + return NULL; > > + > > + skip_line_feeds(parser->lexer); > > + > > + type_t fn_return_type; > > + if (!parser_parse_type(parser, &fn_return_type)) { > > + return NULL; > > + } > > nitpick: I’ve spotted some inconsistency with the use of brackets in > if statements. Just throwing it out there, but I believe we should be > using brackets in all if statements. However, I’m totally fine with > removing them in guard clauses, as long as we maintain a consistent > style. Let's be consistent and keep **{** and **}** on every flow control statements. Changed. > This is great work man! I'm excited of how close we are now from start > the back-end. Thanks for reviewing it. I will make almost every change you requested on a new patch revision.