From c10c7a39a6a245bb327b86f95c3f68691840448e Mon Sep 17 00:00:00 2001 From: Nikos Gorogiannis Date: Wed, 19 Feb 2020 12:47:28 -0800 Subject: [PATCH] [java] use a package/classname record for java classes instead of string Summary: Use a record of package, class name to store (qualified) Java class names. This saves the round trip of concatenating then splitting again, etc, as well as saves some memory in the type environment as now the package paths can be shared across classes of the same package (about 10% in tests). Also remove some unfortunate APIs. Reviewed By: jvillard Differential Revision: D19969325 fbshipit-source-id: f7b7f5a55 --- infer/src/IR/JavaClassName.ml | 33 ++++++++++++++++--- infer/src/IR/JavaClassName.mli | 4 +++ infer/src/IR/Typ.ml | 28 +++++++++++----- infer/src/IR/Typ.mli | 3 -- infer/src/cost/costDomain.ml | 2 +- infer/src/nullsafe/NullabilitySuggest.ml | 2 +- .../build_systems/racerd_dedup/issues.exp | 4 +-- .../codetoanalyze/java/racerd/issues.exp | 2 +- 8 files changed, 58 insertions(+), 20 deletions(-) diff --git a/infer/src/IR/JavaClassName.ml b/infer/src/IR/JavaClassName.ml index 7cd97de55..830a534f1 100644 --- a/infer/src/IR/JavaClassName.ml +++ b/infer/src/IR/JavaClassName.ml @@ -6,11 +6,36 @@ *) open! IStd +module F = Format +module L = Logging -type t = string [@@deriving compare] +(** invariant: if [package = Some str] then [not (String.equal str "")] *) +type t = {classname: string; package: string option} [@@deriving compare] -let from_string str = str +let from_string str = + match String.rsplit2 str ~on:'.' with + | None -> + {classname= str; package= None} + | Some ("", _) -> + L.die InternalError "Empty package path in Java qualified classname.@." + | Some (pkg, classname) -> + {classname; package= Some pkg} -let to_string str = str -let pp = String.pp +let to_string = function + | {classname; package= None} -> + classname + | {classname; package= Some pkg} -> + String.concat ~sep:"." [pkg; classname] + + +let pp fmt = function + | {classname; package= None} -> + F.pp_print_string fmt classname + | {classname; package= Some pkg} -> + F.fprintf fmt "%s.%s" pkg classname + + +let package {package} = package + +let classname {classname} = classname diff --git a/infer/src/IR/JavaClassName.mli b/infer/src/IR/JavaClassName.mli index 148a31856..46c75deac 100644 --- a/infer/src/IR/JavaClassName.mli +++ b/infer/src/IR/JavaClassName.mli @@ -14,3 +14,7 @@ val from_string : string -> t val to_string : t -> string val pp : Format.formatter -> t -> unit + +val package : t -> string option + +val classname : t -> string diff --git a/infer/src/IR/Typ.ml b/infer/src/IR/Typ.ml index ce477cf67..9aed01c8d 100644 --- a/infer/src/IR/Typ.ml +++ b/infer/src/IR/Typ.ml @@ -467,6 +467,12 @@ module Name = struct {type_name= package_classname; package= None} + let of_java_class_name java_class_name = + let package = JavaClassName.package java_class_name in + let type_name = JavaClassName.classname java_class_name in + make ?package type_name + + let package {package} = package let type_name {type_name} = type_name @@ -498,10 +504,19 @@ module Name = struct let java_lang_string = from_string "java.lang.String" - let split_typename typename = Split.of_string (name typename) + let get_java_class_name_exn typename = + match typename with + | JavaClass java_class_name -> + java_class_name + | _ -> + L.die InternalError "Tried to split a non-java class name into a java split type@." + + + let split_typename typename = Split.of_java_class_name (get_java_class_name_exn typename) let is_anonymous_inner_class_name class_name = - let class_name_no_package = Split.type_name (split_typename class_name) in + let java_class_name = get_java_class_name_exn class_name in + let class_name_no_package = JavaClassName.classname java_class_name in match String.rsplit2 class_name_no_package ~on:'$' with | Some (_, s) -> let is_int = @@ -515,12 +530,9 @@ module Name = struct false - let is_external_classname name_string = - let {Split.package} = Split.of_string name_string in - Option.exists ~f:Config.java_package_is_external package - - - let is_external t = is_external_classname (name t) + let is_external t = + get_java_class_name_exn t |> JavaClassName.package + |> Option.exists ~f:Config.java_package_is_external end module Cpp = struct diff --git a/infer/src/IR/Typ.mli b/infer/src/IR/Typ.mli index 6b9916fe3..7d8f45e8a 100644 --- a/infer/src/IR/Typ.mli +++ b/infer/src/IR/Typ.mli @@ -242,9 +242,6 @@ module Name : sig val is_class : t -> bool (** [is_class name] holds if [name] names a Java class *) - val is_external_classname : string -> bool - (** return true if the string is in the .inferconfig list of external classes *) - val is_external : t -> bool (** return true if the typename is in the .inferconfig list of external classes *) diff --git a/infer/src/cost/costDomain.ml b/infer/src/cost/costDomain.ml index fc4c96d03..2a616830f 100644 --- a/infer/src/cost/costDomain.ml +++ b/infer/src/cost/costDomain.ml @@ -15,7 +15,7 @@ module BasicCost = struct (* NOTE: Increment the version number if you changed the [t] type. This is for avoiding demarshalling failure of cost analysis results in running infer-reportdiff. *) - let version = 3 + let version = 4 end (** diff --git a/infer/src/nullsafe/NullabilitySuggest.ml b/infer/src/nullsafe/NullabilitySuggest.ml index fd90b8896..e2fb64a5b 100644 --- a/infer/src/nullsafe/NullabilitySuggest.ml +++ b/infer/src/nullsafe/NullabilitySuggest.ml @@ -174,7 +174,7 @@ let pretty_field_name proc_data field_name = let is_outside_codebase proc_name field_name = match proc_name with | Procname.Java _ -> - Typ.Name.Java.is_external_classname (Typ.Name.name (Fieldname.get_class_name field_name)) + Typ.Name.Java.is_external (Fieldname.get_class_name field_name) | _ -> false diff --git a/infer/tests/build_systems/racerd_dedup/issues.exp b/infer/tests/build_systems/racerd_dedup/issues.exp index 1cb70980a..98f0c81d1 100644 --- a/infer/tests/build_systems/racerd_dedup/issues.exp +++ b/infer/tests/build_systems/racerd_dedup/issues.exp @@ -2,7 +2,7 @@ DeDup.java, build_systems.threadsafety.DeDup.colocated_read_write():void, 63, TH DeDup.java, build_systems.threadsafety.DeDup.separate_write_to_colocated_read():void, 68, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [access to `this.colocated_read`] DeDup.java, build_systems.threadsafety.DeDup.twoWritesOneInCaller():void, 50, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [call to void DeDup.writeToField(),access to `this.field`] DeDup.java, build_systems.threadsafety.DeDup.two_fields():void, 20, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [call to void DeDup.foo(),access to `this.fielda`] -DeDup.java, build_systems.threadsafety.DeDup.two_reads():void, 37, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [,access to `this.field`,,access to `this.field`] +DeDup.java, build_systems.threadsafety.DeDup.two_reads():void, 37, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [,access to `this.field`,,call to void DeDup.writeToField(),access to `this.field`] DeDup.java, build_systems.threadsafety.DeDup.two_writes():void, 30, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [access to `this.field`] DeDup.java, build_systems.threadsafety.DeDup.write_read():void, 44, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [access to `this.field`] -DeDup.java, build_systems.threadsafety.DeDup.write_read():void, 45, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [,access to `this.field`,,access to `this.field`] +DeDup.java, build_systems.threadsafety.DeDup.write_read():void, 45, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [,access to `this.field`,,call to void DeDup.writeToField(),access to `this.field`] diff --git a/infer/tests/codetoanalyze/java/racerd/issues.exp b/infer/tests/codetoanalyze/java/racerd/issues.exp index 82bf133e4..729213174 100644 --- a/infer/tests/codetoanalyze/java/racerd/issues.exp +++ b/infer/tests/codetoanalyze/java/racerd/issues.exp @@ -86,7 +86,7 @@ codetoanalyze/java/racerd/Ownership.java, codetoanalyze.java.checkers.Ownership. codetoanalyze/java/racerd/Ownership.java, codetoanalyze.java.checkers.Ownership.writeToNotOwnedInCalleeBad1(codetoanalyze.java.checkers.Obj):void, 157, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [call to void Ownership.writeToFormal(Obj),access to `o.f`] codetoanalyze/java/racerd/Ownership.java, codetoanalyze.java.checkers.Ownership.writeToNotOwnedInCalleeBad2():void, 162, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [call to void Ownership.writeToFormal(Obj),access to `o.f`] codetoanalyze/java/racerd/Ownership.java, codetoanalyze.java.checkers.Ownership.writeToNotOwnedInCalleeBad3(codetoanalyze.java.checkers.Obj):void, 166, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [call to void Ownership.callWriteToFormal(Obj),call to void Ownership.writeToFormal(Obj),access to `o.f`] -codetoanalyze/java/racerd/Ownership.java, codetoanalyze.java.checkers.Ownership.writeToOwnedInCalleeOk2():void, 183, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [,access to `this.field`,,call to void Ownership.setField(Obj),access to `this.field`] +codetoanalyze/java/racerd/Ownership.java, codetoanalyze.java.checkers.Ownership.writeToOwnedInCalleeOk2():void, 183, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [,access to `this.field`,,access to `this.field`] codetoanalyze/java/racerd/RaceWithMainThread.java, codetoanalyze.java.checkers.RaceWithMainThread.conditional2_bad(boolean):void, 130, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [access to `this.ff`] codetoanalyze/java/racerd/RaceWithMainThread.java, codetoanalyze.java.checkers.RaceWithMainThread.conditionalMainThreadWriteBad():void, 219, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [call to void RaceWithMainThread.conditionalMainThreadWrite2(boolean),access to `this.mOnlyWrittenOnMain`] codetoanalyze/java/racerd/RaceWithMainThread.java, codetoanalyze.java.checkers.RaceWithMainThread.conditional_isMainThread_ElseBranch_Bad():void, 152, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [access to `this.ff`]