[PATCH] D142323: [Kaleidoscope] Update code snippets in text to match full code listings

[Dhruv Chawla via Phabricator via llvm-commits]

0xdc03 added inline comments.


================
Comment at: llvm/docs/tutorial/MyFirstLanguageFrontend/LangImpl04.rst:300
 
+    static ExitOnError ExitOnErr;
+    ...
----------------
xgupta wrote:
> we may remove this line.
I thought it would be more useful to know what the type of `ExitOnErr` was, as this variable has never been referenced before, and it would help not having to scroll to the very end for that


================
Comment at: llvm/docs/tutorial/MyFirstLanguageFrontend/LangImpl07.rst:583
+        // Note: If LLVM is built without RTTI (the default), a static_cast
+        //       must be used here.
         VariableExprAST *LHSE = dynamic_cast<VariableExprAST*>(LHS.get());
----------------
xgupta wrote:
> Not sure about this, why not use static_cast then by default? @kazu might better review this.
Yeah, I was a little confused at this point so I decided to stick with what the tutorial had before and mention how the code listing did it


================
Comment at: llvm/examples/Kaleidoscope/Chapter9/toy.cpp:803
     // Make an anonymous proto.
-    auto Proto = std::make_unique<PrototypeAST>(FnLoc, "__anon_expr",
+    auto Proto = std::make_unique<PrototypeAST>(FnLoc, "main",
                                                  std::vector<std::string>());
----------------
xgupta wrote:
>   // Make an anonymous proto.
> This comment is better suited for  __anon_expr. we should keep it.
Oh, I completely missed this comment! This was also one of the points where I wasn't sure what to do. Given that the start of the chapter explicitly says

> First we make our anonymous function that contains our top level statement be our “main”:

under section 9.3, I feel it'd be better to update the comment to say something like `// Make the top level expression be our "main" function`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142323/new/

https://reviews.llvm.org/D142323