From 5d0e60b8bc0288f23b455f433801f1f0803783b6 Mon Sep 17 00:00:00 2001 From: Jakob Petsovits Date: Wed, 9 Mar 2016 18:31:12 -0500 Subject: [PATCH] Fix shadowing of C++ type names by similar method names. This can be triggered by the following kinds of methods: ``` Conflict = interface +c { } conflict_user = interface +c { # Invalid method: # std::shared_ptr Conflict(); # The template argument refers to the method name, not the type. Conflict(): Conflict; # Invalid method: # void conflict_arg(const std::set>& cs); # The other method name still shadows the 'Conflict' type. conflict_arg(cs: set): bool; } ``` (This is more of an issue when the target C++ naming convention uses lower-case names for both types and methods, as is common in e.g. Boost or C++ standard library names.) Both are fixed by using the type's fully qualified name if the scope contains conflicting names/symbols. --- src/source/CppGenerator.scala | 5 ++-- src/source/CppMarshal.scala | 52 +++++++++++++++++++++++++++-------- 2 files changed, 43 insertions(+), 14 deletions(-) diff --git a/src/source/CppGenerator.scala b/src/source/CppGenerator.scala index f7e472da7..119636fd7 100644 --- a/src/source/CppGenerator.scala +++ b/src/source/CppGenerator.scala @@ -273,6 +273,7 @@ class CppGenerator(spec: Spec) extends Generator(spec) { }) val self = marshal.typename(ident, i) + val methodNamesInScope = i.methods.map(m => idCpp.method(m.ident)) writeHppFile(ident, origin, refs.hpp, refs.hppFwds, w => { writeDoc(w, doc) @@ -287,8 +288,8 @@ class CppGenerator(spec: Spec) extends Generator(spec) { for (m <- i.methods) { w.wl writeDoc(w, m.doc) - val ret = marshal.returnType(m.ret) - val params = m.params.map(p => marshal.paramType(p.ty) + " " + idCpp.local(p.ident)) + val ret = marshal.returnType(m.ret, methodNamesInScope) + val params = m.params.map(p => marshal.paramType(p.ty, methodNamesInScope) + " " + idCpp.local(p.ident)) if (m.static) { w.wl(s"static $ret ${idCpp.method(m.ident)}${params.mkString("(", ", ", ")")};") } else { diff --git a/src/source/CppMarshal.scala b/src/source/CppMarshal.scala index 76a0b9b9d..6304c47ca 100644 --- a/src/source/CppMarshal.scala +++ b/src/source/CppMarshal.scala @@ -5,27 +5,42 @@ import djinni.generatorTools._ import djinni.meta._ class CppMarshal(spec: Spec) extends Marshal(spec) { + // The scopeSymbols parameter accepted by many of these functions describes a Seq of + // symbols/names that are declared in the current scope. The TypeRef or MExpr expression + // will be fully qualified if it clashes with any of these symbols, even if full qualification + // has not been requested. - override def typename(tm: MExpr): String = toCppType(tm, None) + override def typename(tm: MExpr): String = toCppType(tm, None, Seq()) + def typename(tm: MExpr, scopeSymbols: Seq[String]): String = toCppType(tm, None, scopeSymbols) + def typename(ty: TypeRef, scopeSymbols: Seq[String]): String = typename(ty.resolved, scopeSymbols) def typename(name: String, ty: TypeDef): String = ty match { case e: Enum => idCpp.enumType(name) case i: Interface => idCpp.ty(name) case r: Record => idCpp.ty(name) } - override def fqTypename(tm: MExpr): String = toCppType(tm, Some(spec.cppNamespace)) + override def fqTypename(tm: MExpr): String = toCppType(tm, Some(spec.cppNamespace), Seq()) def fqTypename(name: String, ty: TypeDef): String = ty match { case e: Enum => withNs(Some(spec.cppNamespace), idCpp.enumType(name)) case i: Interface => withNs(Some(spec.cppNamespace), idCpp.ty(name)) case r: Record => withNs(Some(spec.cppNamespace), idCpp.ty(name)) } + def paramType(tm: MExpr, scopeSymbols: Seq[String]): String = toCppParamType(tm, None, scopeSymbols) + def paramType(ty: TypeRef, scopeSymbols: Seq[String]): String = paramType(ty.resolved, scopeSymbols) override def paramType(tm: MExpr): String = toCppParamType(tm) override def fqParamType(tm: MExpr): String = toCppParamType(tm, Some(spec.cppNamespace)) + def returnType(ret: Option[TypeRef], scopeSymbols: Seq[String]): String = { + ret.fold("void")(toCppType(_, None, scopeSymbols)) + } override def returnType(ret: Option[TypeRef]): String = ret.fold("void")(toCppType(_, None)) - override def fqReturnType(ret: Option[TypeRef]): String = ret.fold("void")(toCppType(_, Some(spec.cppNamespace))) + override def fqReturnType(ret: Option[TypeRef]): String = { + ret.fold("void")(toCppType(_, Some(spec.cppNamespace))) + } + def fieldType(tm: MExpr, scopeSymbols: Seq[String]): String = typename(tm, scopeSymbols) + def fieldType(ty: TypeRef, scopeSymbols: Seq[String]): String = fieldType(ty.resolved, scopeSymbols) override def fieldType(tm: MExpr): String = typename(tm) override def fqFieldType(tm: MExpr): String = fqTypename(tm) @@ -113,8 +128,21 @@ class CppMarshal(spec: Spec) extends Marshal(spec) { def include(ident: String): String = q(spec.cppIncludePrefix + spec.cppFileIdentStyle(ident) + "." + spec.cppHeaderExt) - private def toCppType(ty: TypeRef, namespace: Option[String] = None): String = toCppType(ty.resolved, namespace) - private def toCppType(tm: MExpr, namespace: Option[String]): String = { + private def toCppType(ty: TypeRef, namespace: Option[String] = None, scopeSymbols: Seq[String] = Seq()): String = + toCppType(ty.resolved, namespace, scopeSymbols) + + private def toCppType(tm: MExpr, namespace: Option[String], scopeSymbols: Seq[String]): String = { + def withNamespace(name: String): String = { + // If an unqualified symbol needs to have its namespace added, this code assumes that the + // namespace is the one that's defined for generated types (spec.cppNamespace). + // This seems like a safe assumption for the C++ generator as it doesn't make much use of + // other global names, but might need to be refined to cover other types in the future. + val ns = namespace match { + case Some(ns) => Some(ns) + case None => if (scopeSymbols.contains(name)) Some(spec.cppNamespace) else None + } + withNs(ns, name) + } def base(m: Meta): String = m match { case p: MPrimitive => p.cName case MString => "std::string" @@ -126,9 +154,9 @@ class CppMarshal(spec: Spec) extends Marshal(spec) { case MMap => "std::unordered_map" case d: MDef => d.defType match { - case DEnum => withNs(namespace, idCpp.enumType(d.name)) - case DRecord => withNs(namespace, idCpp.ty(d.name)) - case DInterface => s"std::shared_ptr<${withNs(namespace, idCpp.ty(d.name))}>" + case DEnum => withNamespace(idCpp.enumType(d.name)) + case DRecord => withNamespace(idCpp.ty(d.name)) + case DInterface => s"std::shared_ptr<${withNamespace(idCpp.ty(d.name))}>" } case e: MExtern => e.defType match { case DInterface => s"std::shared_ptr<${e.cpp.typename}>" @@ -145,14 +173,14 @@ class CppMarshal(spec: Spec) extends Marshal(spec) { tm.base match { case d: MDef => d.defType match { - case DInterface => s"${nnType}<${withNs(namespace, idCpp.ty(d.name))}>" + case DInterface => s"${nnType}<${withNamespace(idCpp.ty(d.name))}>" case _ => base(tm.base) + args } case MOptional => tm.args.head.base match { case d: MDef => d.defType match { - case DInterface => s"std::shared_ptr<${withNs(namespace, idCpp.ty(d.name))}>" + case DInterface => s"std::shared_ptr<${withNamespace(idCpp.ty(d.name))}>" case _ => base(tm.base) + args } case _ => base(tm.base) + args @@ -195,8 +223,8 @@ class CppMarshal(spec: Spec) extends Marshal(spec) { } // this can be used in c++ generation to know whether a const& should be applied to the parameter or not - private def toCppParamType(tm: MExpr, namespace: Option[String] = None): String = { - val cppType = toCppType(tm, namespace) + private def toCppParamType(tm: MExpr, namespace: Option[String] = None, scopeSymbols: Seq[String] = Seq()): String = { + val cppType = toCppType(tm, namespace, scopeSymbols) val refType = "const " + cppType + " &" val valueType = cppType if(byValue(tm)) valueType else refType