Add parsing for extern declarations + lexing for strings

[bloody76]

This change is 

[bloody76]

Review status: 0 of 15 files reviewed at latest revision, 1 unresolved discussion.

---

src/parser/parser.cc, line 3 at r1 (raw file):

#include "parser/parser.h"

#include <iostream>

remove

---

Comments from Reviewable

[bloody76]

Review status: 0 of 15 files reviewed at latest revision, 2 unresolved discussions.

---

src/parser/parser.h, line 75 at r1 (raw file):

  parse_function_arguments_declaration();

  ErrorOrPtr<ast::Declaration> parse_extern_declaration();

doc

---

Comments from Reviewable

[coveralls]

Coverage decreased (-0.5%) to 98.832% when pulling 1ad665210b4429dadccb3d2921cd0baf197c361b on add_extern into 1dad241d4f7395507bece2fd9f82f71d5bc3279d on master.

[coveralls]

Coverage decreased (-0.5%) to 98.832% when pulling 46ff5a11e7e8bf76ecfb89de1674684cc54624b5 on add_extern into 1dad241d4f7395507bece2fd9f82f71d5bc3279d on master.

[coveralls]

Coverage decreased (-0.5%) to 98.832% when pulling a49fbb5bf817e1d0a0bb73ba68a035cf8c5ece1e on add_extern into 1dad241d4f7395507bece2fd9f82f71d5bc3279d on master.

[nitnelave]

Reviewed 13 of 15 files at r1, 2 of 2 files at r2. Review status: all files reviewed at latest revision, 7 unresolved discussions, some commit checks failed.

---

src/ast/declaration.h, line 15 at r2 (raw file):

 public:
  Declaration(lexer::Range location, NodeType node_type, Identifier id,
              Option<Type> type, CallingConvention calling_convention)

Calling convention in a declaration? For variables? Why not in the functiondeclaration node?

---

src/lexer/lexer.cc, line 320 at r2 (raw file):

/// Reads until valid double quote.
ErrorOr<Token, LexError> Lexer::read_string(const Location& beginning) {

This is rather crude, but I suppose it'll do for now

---

src/parser/parser.cc, line 207 at r2 (raw file):

template <class Declaration>
Parser::ErrorOrPtr<Declaration> Parser::parse_variable_declaration(
    ast::CallingConvention calling_convention) {

again, calling convention for a variable?

---

src/pretty_printer/function_declaration.cc, line 27 at r2 (raw file):

  out_ << ')';
  if (node->type().is_ok())
    out_ << " : " << node->type().value_or_die().to_string();

Print the extern declaration?

---

test/resources/lexer/strings/unimplemented.gh, line 1 at r2 (raw file):

val a = "test";

Got any better test than that?

---

Comments from Reviewable

[bloody76]

Reviewed 2 of 15 files at r1, 1 of 2 files at r2. Review status: all files reviewed at latest revision, 7 unresolved discussions, some commit checks failed.

---

src/ast/declaration.h, line 15 at r2 (raw file):

Previously, nitnelave (nitnelave) wrote…

Calling convention in a declaration? For variables? Why not in the functiondeclaration node?

I can move it there indeed, I just wonder if there could be any convention for variables

---

src/lexer/lexer.cc, line 320 at r2 (raw file):

Previously, nitnelave (nitnelave) wrote…

This is rather crude, but I suppose it'll do for now

you mean the algorithm ? It is indeed very straightforward, there surely are cases not handled for the moment

---

src/parser/parser.cc, line 207 at r2 (raw file):

Previously, nitnelave (nitnelave) wrote…

again, calling convention for a variable?

will move

---

test/resources/lexer/strings/unimplemented.gh, line 1 at r2 (raw file):

Previously, nitnelave (nitnelave) wrote…

Got any better test than that?

I will add more, anyway I am decreasing the test coverage for now

---

Comments from Reviewable

[bloody76]

Review status: all files reviewed at latest revision, 7 unresolved discussions, some commit checks failed.

---

src/ast/declaration.h, line 15 at r2 (raw file):

Previously, bloody76 (Bd76) wrote…

I can move it there indeed, I just wonder if there could be any convention for variables

Maybe it would be nice to have an ExternVariableDeclaration, what do you think ?

---

Comments from Reviewable

[bloody76]

Review status: all files reviewed at latest revision, 7 unresolved discussions, some commit checks failed.

---

src/ast/declaration.h, line 15 at r2 (raw file):

Previously, bloody76 (Bd76) wrote…

Maybe it would be nice to have an ExternVariableDeclaration, what do you think ?

we could also imagine the same for FunctionDeclaration, the ExternDeclaration would inherit from their Local/Normal versions

---

Comments from Reviewable

[coveralls]

Coverage decreased (-0.4%) to 98.847% when pulling bf6bd100fee98e7d2293763a2ff4df2058edbc48 on add_extern into 1dad241d4f7395507bece2fd9f82f71d5bc3279d on master.

[bloody76]

Review status: 6 of 22 files reviewed at latest revision, 7 unresolved discussions.

---

src/pretty_printer/function_declaration.cc, line 27 at r2 (raw file):

Previously, nitnelave (nitnelave) wrote…

Print the extern declaration?

Done.

---

Comments from Reviewable

[coveralls]

Coverage decreased (-0.5%) to 98.784% when pulling 7223230c54d225c9591293ae48d5e507bebdb44e on add_extern into 1dad241d4f7395507bece2fd9f82f71d5bc3279d on master.

[nitnelave]

Reviewed 7 of 19 files at r3. Review status: 13 of 22 files reviewed at latest revision, 10 unresolved discussions.

---

src/ast/ast.h, line 70 at r3 (raw file):

class FunctionCall;
class FunctionDeclaration;
class ExternFunctionDeclaration;

not a big fan of the extra class. And you're making a concrete class inherit from a concrete class.

---

src/ast/declaration.h, line 15 at r2 (raw file):

Previously, bloody76 (Bd76) wrote…

we could also imagine the same for FunctionDeclaration, the ExternDeclaration would inherit from their Local/Normal versions

you can't call a variable, so I don't think there are calling conventions for variables (feel free to prove me wrong, but I don't remember that from the LLVM manual). I would move the calling convention down to the function. And actually, it's not exactly the calling convention that we mean. More like... a compatibility mode? The calling convention is a consequence of the compatibility mode, but it's not a direct and reciprocate one. For instance, the "fast" calling convention can be used inside a module, but maybe we need the "C" convention for module interfaces (we'll have to see that). Anyway, rambling to say that extern "C" is not a calling convention, but a compatibility mode, and as such it can apply to variables (maybe).

---

src/ast/extern_function_declaration.h, line 16 at r3 (raw file):

namespace ast {

enum CallingConvention { NORMAL = 0, C = 1 };

enum class

---

src/lexer/lexer.cc, line 320 at r2 (raw file):

Previously, bloody76 (Bd76) wrote…

you mean the algorithm ? It is indeed very straightforward, there surely are cases not handled for the moment

to list a few: the escaping only creates backslashes (i.e. "\n" is parsed as a single backslash), we're not handling unicode, and there's probably many other cases we don't handle :)

---

src/parser/parser.cc, line 440 at r3 (raw file):

  ASSERT_TOKEN(TokenType::EXTERN);

  if (current_token().type() == TokenType::VAL ||

we should maybe extract this condition in a function

---

src/parser/parser.cc, line 450 at r3 (raw file):

  if (current_token().type() == TokenType::VAL ||
      current_token().type() == TokenType::MUT) {
    return ParseError("Extern variable doesn't need a calling convention",

I'm not sure I understand what you mean...

---

Comments from Reviewable

[bloody76]

Review status: 13 of 22 files reviewed at latest revision, 9 unresolved discussions.

---

src/ast/ast.h, line 70 at r3 (raw file):

Previously, nitnelave (nitnelave) wrote…

not a big fan of the extra class. And you're making a concrete class inherit from a concrete class.

it's handier in the visitors I would say, cleaner maybe I don't know. But I can get back to a normal version, it would make sense also

---

src/ast/declaration.h, line 15 at r2 (raw file):

Previously, nitnelave (nitnelave) wrote…

you can't _call_ a variable, so I don't think there are calling conventions for variables (feel free to prove me wrong, but I don't remember that from the LLVM manual). I would move the calling convention down to the function. And actually, it's not exactly the calling convention that we mean. More like... a compatibility mode? The calling convention is a consequence of the compatibility mode, but it's not a direct and reciprocate one. For instance, the "fast" calling convention can be used inside a module, but maybe we need the "C" convention for module interfaces (we'll have to see that). Anyway, rambling to say that `extern "C"` is not a calling convention, but a compatibility mode, and as such it can apply to variables (maybe).

I don't think it can apply to variables indeedl. For the calling convention vs compatilibty mode... we can call it compatbility mode if you want

---

src/parser/parser.cc, line 440 at r3 (raw file):

Previously, nitnelave (nitnelave) wrote…

we should maybe extract this condition in a function

yeah I thought about it also, will do

---

src/parser/parser.cc, line 450 at r3 (raw file):

Previously, nitnelave (nitnelave) wrote…

I'm not sure I understand what you mean...

if someone is writing extern "C" val, it will be wrong and the message would be clearer IMO if we say that it doesn't need to "C" thing. We can call that compatbility mode if you want :D

---

Comments from Reviewable

[nitnelave]

Review status: 13 of 22 files reviewed at latest revision, 9 unresolved discussions.

---

src/ast/ast.h, line 70 at r3 (raw file):

Previously, bloody76 (Bd76) wrote…

it's handier in the visitors I would say, cleaner maybe I don't know. But I can get back to a normal version, it would make sense also

I'd rather have a normal version. It's more a property of the class than its nature, I think (although that's debatable).

---

src/ast/declaration.h, line 15 at r2 (raw file):

Previously, bloody76 (Bd76) wrote…

I don't think it can apply to variables indeedl. For the calling convention vs compatilibty mode... we can call it compatbility mode if you want

Well, for instance, maybe we would have different global variable initialization in C++ compatibility mode or something. So I can see a compatibility mode on global variables, but still not a calling convention :)

---

src/parser/parser.cc, line 450 at r3 (raw file):

Previously, bloody76 (Bd76) wrote…

if someone is writing `extern "C" val `, it will be wrong and the message would be clearer IMO if we say that it doesn't need to "C" thing. We can call that compatbility mode if you want :D

Well, it depends if we do global variable compatibility mode... but for now it's not planned, so let's keep something like what you wrote.

---

Comments from Reviewable