Skip to content

Commit

Permalink
Use Error Prone's @lazyinit annotation as a signal to upgrade referen…
Browse files Browse the repository at this point in the history
…ce type fields to volatile semantics to avoid unsafe racy inits.

	Change on 2016/11/24 by kstanger <kstanger@google.com>

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=140129608
  • Loading branch information
kstanger authored and Keith Stanger committed Dec 1, 2016
1 parent e51ebde commit 10811b0
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 12 deletions.
Expand Up @@ -46,7 +46,6 @@
import javax.lang.model.element.ElementKind;
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.element.Modifier;
import javax.lang.model.element.Name;
import javax.lang.model.element.NestingKind;
import javax.lang.model.element.PackageElement;
import javax.lang.model.element.TypeElement;
Expand Down Expand Up @@ -77,6 +76,8 @@ public final class ElementUtil {
private static final Set<Modifier> VISIBILITY_MODIFIERS = EnumSet.of(
Modifier.PUBLIC, Modifier.PROTECTED, Modifier.PRIVATE);

private static final String LAZY_INIT = "com.google.errorprone.annotations.concurrent.LazyInit";

private final Elements javacElements;

private static final Map<Integer, Set<Modifier>> modifierSets = new HashMap<>();
Expand Down Expand Up @@ -121,7 +122,11 @@ public static boolean isPrivate(Element element) {
}

public static boolean isVolatile(VariableElement element) {
return hasModifier(element, Modifier.VOLATILE);
return hasModifier(element, Modifier.VOLATILE)
// Upgrade reference type fields marked with error prone's LazyInit because this indicates
// an intentional racy init.
|| (!element.asType().getKind().isPrimitive()
&& hasQualifiedNamedAnnotation(element, LAZY_INIT));
}

public static boolean isTopLevel(TypeElement type) {
Expand Down Expand Up @@ -603,13 +608,7 @@ public static boolean isRuntimeAnnotation(Element e) {
}

public static AnnotationMirror getAnnotation(Element element, Class<?> annotationClass) {
for (AnnotationMirror annotation : element.getAnnotationMirrors()) {
String className = TypeUtil.getQualifiedName(annotation.getAnnotationType());
if (className.equals(annotationClass.getName())) {
return annotation;
}
}
return null;
return getQualifiedNamedAnnotation(element, annotationClass.getName());
}

public static boolean hasAnnotation(Element element, Class<?> annotationClass) {
Expand All @@ -619,16 +618,28 @@ public static boolean hasAnnotation(Element element, Class<?> annotationClass) {
/**
* Less strict version of the above where we don't care about the annotation's package.
*/
public static boolean hasNamedAnnotation(AnnotatedConstruct ac, String annotationName) {
public static boolean hasNamedAnnotation(AnnotatedConstruct ac, String name) {
for (AnnotationMirror annotation : ac.getAnnotationMirrors()) {
Name annotationTypeName = annotation.getAnnotationType().asElement().getSimpleName();
if (annotationTypeName.toString().equals(annotationName)) {
if (getName(annotation.getAnnotationType().asElement()).equals(name)) {
return true;
}
}
return false;
}

public static boolean hasQualifiedNamedAnnotation(Element element, String name) {
return getQualifiedNamedAnnotation(element, name) != null;
}

public static AnnotationMirror getQualifiedNamedAnnotation(Element element, String name) {
for (AnnotationMirror annotation : element.getAnnotationMirrors()) {
if (getQualifiedName((TypeElement) annotation.getAnnotationType().asElement()).equals(name)) {
return annotation;
}
}
return null;
}

/**
* Return true if a binding has a named "Nullable" annotation. Package names aren't
* checked because different nullable annotations are defined by several different
Expand Down
Expand Up @@ -201,4 +201,19 @@ public void testRetainedLocalRef() throws IOException {
"return [((id<JavaUtilComparator>) nil_chk(((Test_Thing *) nil_chk(thing))->comp_)) "
+ "compareWithId:s1 withId:s2] == 0;");
}

public void testLazyInitFields() throws IOException {
addSourcesToSourcepaths();
addSourceFile("package com.google.errorprone.annotations.concurrent;"
+ "public @interface LazyInit {}",
"com/google/errorprone/annotations/concurrent/LazyInit.java");
String translation = translateSourceFile(
"import com.google.errorprone.annotations.concurrent.LazyInit;"
+ "class Test { @LazyInit String lazyStr; @LazyInit static String lazyStaticStr; }",
"Test", "Test.h");
assertTranslation(translation, "volatile_id lazyStr_;");
assertTranslatedLines(translation,
"FOUNDATION_EXPORT volatile_id Test_lazyStaticStr;",
"J2OBJC_STATIC_FIELD_OBJ_VOLATILE(Test, lazyStaticStr, NSString *)");
}
}

0 comments on commit 10811b0

Please sign in to comment.