<br><br><div class="gmail_quote">On Sat, Dec 22, 2012 at 11:48 PM, João Matos <span dir="ltr"><<a href="mailto:ripzonetriton@gmail.com" target="_blank">ripzonetriton@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><div>+class MSPropertyDecl : public DeclaratorDecl {</div><div>+public:</div><div>+  MSPropertyDecl(Kind DK, DeclContext *DC, SourceLocation L,</div><div>+                 DeclarationName N, QualType T, TypeSourceInfo *TInfo,</div>


<div>+                 SourceLocation StartL, llvm::StringRef Getter,</div><div>+                 llvm::StringRef Setter):</div><div>+    DeclaratorDecl(DK, DC, L, N, T, TInfo, StartL), GetterName(Getter),</div><div>+    SetterName(Setter) {}</div>


<div>+  </div><div>+  static bool classof(const Decl *D) { return D->getKind() == MSProperty; }</div><div>+</div><div>+  bool hasGetter() { return !GetterName.empty(); }</div><div>+  llvm::StringRef getGetterName() { return GetterName; }</div>


<div>+  bool hasSetter() { return !SetterName.empty(); }</div><div>+  llvm::StringRef getSetterName() { return SetterName; }</div><div>+</div><div>+private:</div><div>+  llvm::StringRef GetterName, SetterName;</div><div>

+};</div>
</div><div><br></div>Why are you storing the getter and setter as StringRef in MSPropertyDecl?<div><br></div><div>I think it would make more sense to lookup them up under Sema and store the AST node directly. Also the private members should be at the top of the class to be consistent with the rest of the AST code conventions.</div>
</blockquote><div><br></div><div>Getter/Setter can be defined after property. So when we're parsing property, getter/setter may not be available yet.</div><div>However, we can cache the search result into MSPropertyDecl to speedup following search.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<div><br></div><div><div>+void ASTStmtReader::VisitMSPropertyRefExpr(MSPropertyRefExpr *E) {</div><div>+  llvm_unreachable("Shouldn't get here");</div><div>+}</div></div><div><br></div><div><div>+void ASTStmtWriter::VisitMSPropertyRefExpr(MSPropertyRefExpr *E) {</div>


<div>+  llvm_unreachable("Shouldn't get here");</div><div>+}</div></div><div style="text-align:right"><br></div><div>These should be implemented.</div><div><br></div><div>The parsing code seems to do more string comparisons than are necessary but since this feature is not that common it might be fine, I'll leave that for other people to comment.</div>
<span class="HOEnZb"><font color="#888888">

<div><br></div><div><div>-- <br>João Matos
</div></div>
</font></span></blockquote></div><br><br clear="all"><div><br></div>-- <br>Best Regards, Tong Shen (沈彤)