tmf: Remove function name from ITmfCallsite
authorAlexandre Montplaisir <alexmonthy@efficios.com>
Mon, 18 Jul 2016 21:17:24 +0000 (17:17 -0400)
committerAlexandre Montplaisir <alexmonthy@efficios.com>
Wed, 27 Jul 2016 18:19:22 +0000 (14:19 -0400)
It just so happens today that the UST Debug-info analysis makes
use of addr2line to retrieve the source code location AND the
function names, but those two are completely separate concepts.

Since "function name" is not relevant for source lookup, it
should be removed from the concept of what is a call site.
ISymbolProvider should be the sole provider of function names,
especially now that it has been decoupled from ITmfCallsite.

While at it, add a new version of #getLineNumber() which returns
a "@Nullable Long", since it's very possible for callsites to
have a file name but no line number available.

Change-Id: I9b7407876166d9cc1e174804145937388306137d
Signed-off-by: Alexandre Montplaisir <alexmonthy@efficios.com>
Reviewed-on: https://git.eclipse.org/r/77554
Reviewed-by: Hudson CI
Reviewed-by: Patrick Tasse <patrick.tasse@gmail.com>
Tested-by: Patrick Tasse <patrick.tasse@gmail.com>
14 files changed:
lttng/org.eclipse.tracecompass.lttng2.ust.core/.settings/.api_filters [new file with mode: 0644]
lttng/org.eclipse.tracecompass.lttng2.ust.core/META-INF/MANIFEST.MF
lttng/org.eclipse.tracecompass.lttng2.ust.core/src/org/eclipse/tracecompass/internal/lttng2/ust/core/analysis/debuginfo/FileOffsetMapper.java
lttng/org.eclipse.tracecompass.lttng2.ust.core/src/org/eclipse/tracecompass/lttng2/ust/core/analysis/debuginfo/FunctionLocation.java
lttng/org.eclipse.tracecompass.lttng2.ust.core/src/org/eclipse/tracecompass/lttng2/ust/core/analysis/debuginfo/SourceCallsite.java
lttng/org.eclipse.tracecompass.lttng2.ust.core/src/org/eclipse/tracecompass/lttng2/ust/core/analysis/debuginfo/UstDebugInfoFunctionAspect.java
lttng/org.eclipse.tracecompass.lttng2.ust.core/src/org/eclipse/tracecompass/lttng2/ust/core/analysis/debuginfo/UstDebugInfoSourceAspect.java
lttng/org.eclipse.tracecompass.lttng2.ust.ui/src/org/eclipse/tracecompass/internal/lttng2/ust/ui/analysis/debuginfo/UstDebugInfoSymbolProvider.java
tmf/org.eclipse.tracecompass.tmf.core.tests/src/org/eclipse/tracecompass/tmf/core/tests/event/lookup/TmfCallsiteTest.java
tmf/org.eclipse.tracecompass.tmf.core.tests/stubs/org/eclipse/tracecompass/tmf/tests/stubs/trace/text/SyslogEvent.java
tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/event/lookup/ITmfCallsite.java
tmf/org.eclipse.tracecompass.tmf.core/src/org/eclipse/tracecompass/tmf/core/event/lookup/TmfCallsite.java
tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/viewers/events/TmfEventPropertySource.java
tmf/org.eclipse.tracecompass.tmf.ui/src/org/eclipse/tracecompass/tmf/ui/viewers/events/TmfEventsTable.java

diff --git a/lttng/org.eclipse.tracecompass.lttng2.ust.core/.settings/.api_filters b/lttng/org.eclipse.tracecompass.lttng2.ust.core/.settings/.api_filters
new file mode 100644 (file)
index 0000000..8b558d4
--- /dev/null
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<component id="org.eclipse.tracecompass.lttng2.ust.core" version="2">
+    <resource path="src/org/eclipse/tracecompass/lttng2/ust/core/analysis/debuginfo/UstDebugInfoSourceAspect.java" type="org.eclipse.tracecompass.lttng2.ust.core.analysis.debuginfo.UstDebugInfoSourceAspect">
+        <filter comment="Change return type from SourceCallsite to TmfCallsite, which are effectively identical." id="338792546">
+            <message_arguments>
+                <message_argument value="org.eclipse.tracecompass.lttng2.ust.core.analysis.debuginfo.UstDebugInfoSourceAspect"/>
+                <message_argument value="resolve(ITmfEvent)"/>
+            </message_arguments>
+        </filter>
+    </resource>
+</component>
index 668093b6486ec8ad1929f4b9e4b82a2cb184b3da..8947b4dae3270b9322b0d51d477c0d7ae7ea50d4 100644 (file)
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %Bundle-Name
 Bundle-Vendor: %Bundle-Vendor
-Bundle-Version: 2.0.0.qualifier
+Bundle-Version: 2.1.0.qualifier
 Bundle-Localization: plugin
 Bundle-SymbolicName: org.eclipse.tracecompass.lttng2.ust.core;singleton:=true
 Bundle-Activator: org.eclipse.tracecompass.internal.lttng2.ust.core.Activator
index d426042769be636d9e14de3b6730c56c6350f356..7717955aef228758a21cc671fc288788b5ab01af 100644 (file)
@@ -17,20 +17,22 @@ import java.io.IOException;
 import java.io.InputStreamReader;
 import java.nio.file.Files;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
 
+import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.jdt.annotation.Nullable;
 import org.eclipse.tracecompass.common.core.log.TraceCompassLog;
-import org.eclipse.tracecompass.lttng2.ust.core.analysis.debuginfo.SourceCallsite;
 import org.eclipse.tracecompass.tmf.core.event.lookup.TmfCallsite;
 
 import com.google.common.base.Objects;
 import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
+import com.google.common.collect.Iterables;
 
 /**
  * Utility class to get file name, function/symbol name and line number from a
@@ -96,32 +98,41 @@ public final class FileOffsetMapper {
         }
     }
 
+
     /**
-     * Cache of all calls to 'addr2line', so that we can avoid recalling the
-     * external process repeatedly.
+     * Generate the callsite from a given binary file and address offset.
      *
-     * It is static, meaning one cache for the whole application, since the
-     * symbols in a file on disk are independent from the trace referring to it.
+     * Due to function inlining, it is possible for one offset to actually have
+     * multiple call sites. We will return the most precise one (inner-most) we
+     * have available.
+     *
+     * @param file
+     *            The binary file to look at
+     * @param buildId
+     *            The expected buildId of the binary file (is not verified at
+     *            the moment)
+     * @param offset
+     *            The memory offset in the file
+     * @return The corresponding call site
      */
-    private static final LoadingCache<FileOffset, @Nullable Iterable<SourceCallsite>> CALLSITE_CACHE;
-    static {
-        CALLSITE_CACHE = checkNotNull(CacheBuilder.newBuilder()
-            .maximumSize(CACHE_SIZE)
-            .build(new CacheLoader<FileOffset, @Nullable Iterable<SourceCallsite>>() {
-                @Override
-                public @Nullable Iterable<SourceCallsite> load(FileOffset fo) {
-                    LOGGER.fine(() -> "[FileOffsetMapper:CacheMiss] file/offset=" + fo.toString()); //$NON-NLS-1$
-                    return getCallsiteFromOffsetWithAddr2line(fo);
-                }
-            }));
+    public static @Nullable TmfCallsite getCallsiteFromOffset(File file, @Nullable String buildId, long offset) {
+       Iterable<Addr2lineInfo> output = getAddr2lineInfo(file, buildId, offset);
+       if (output == null || Iterables.isEmpty(output)) {
+           return null;
+       }
+       Addr2lineInfo info = Iterables.getLast(output);
+       String sourceFile = info.fSourceFileName;
+       Long sourceLine = info.fSourceLineNumber;
+
+       if (sourceFile == null) {
+           /* Not enough information to provide a callsite */
+           return null;
+       }
+       return new TmfCallsite(sourceFile, sourceLine);
     }
 
     /**
-     * Generate the callsites from a given binary file and address offset.
-     *
-     * Due to function inlining, it is possible for one offset to actually have
-     * multiple call sites. This is why we can return more than one callsite per
-     * call.
+     * Get the function/symbol name corresponding to binary file and offset.
      *
      * @param file
      *            The binary file to look at
@@ -130,11 +141,72 @@ public final class FileOffsetMapper {
      *            the moment)
      * @param offset
      *            The memory offset in the file
-     * @return The list of callsites corresponding to the offset, reported from
-     *         the "highest" inlining location, down to the initial definition.
+     * @return The corresponding function/symbol name
+     */
+    public static @Nullable String getFunctionNameFromOffset(File file, @Nullable String buildId, long offset) {
+        /*
+         * TODO We are currently also using 'addr2line' to resolve function
+         * names, which requires the binary's DWARF information to be available.
+         * A better approach would be to use the binary's symbol table (if it is
+         * not stripped), since this is usually more readily available than
+         * DWARF.
+         */
+        Iterable<Addr2lineInfo> output = getAddr2lineInfo(file, buildId, offset);
+        if (output == null || Iterables.isEmpty(output)) {
+            return null;
+        }
+        Addr2lineInfo info = Iterables.getLast(output);
+        return info.fFunctionName;
+    }
+
+    // ------------------------------------------------------------------------
+    // Utility methods making use of 'addr2line'
+    // ------------------------------------------------------------------------
+
+    /**
+     * Cache of all calls to 'addr2line', so that we can avoid recalling the
+     * external process repeatedly.
+     *
+     * It is static, meaning one cache for the whole application, since the
+     * symbols in a file on disk are independent from the trace referring to it.
      */
-    public static @Nullable Iterable<SourceCallsite> getCallsiteFromOffset(File file, @Nullable String buildId, long offset) {
-        LOGGER.finer(() -> String.format("[FileOffsetMapper:Request] file=%s, buildId=%s, offset=0x%h", //$NON-NLS-1$
+    private static final LoadingCache<FileOffset, @NonNull Iterable<Addr2lineInfo>> ADDR2LINE_INFO_CACHE;
+    static {
+        ADDR2LINE_INFO_CACHE = checkNotNull(CacheBuilder.newBuilder()
+            .maximumSize(CACHE_SIZE)
+            .build(new CacheLoader<FileOffset, @NonNull Iterable<Addr2lineInfo>>() {
+                @Override
+                public @NonNull Iterable<Addr2lineInfo> load(FileOffset fo) {
+                    LOGGER.fine(() -> "[FileOffsetMapper:CacheMiss] file/offset=" + fo.toString()); //$NON-NLS-1$
+                    return callAddr2line(fo);
+                }
+            }));
+    }
+
+    private static class Addr2lineInfo {
+
+        private final @Nullable String fSourceFileName;
+        private final @Nullable Long fSourceLineNumber;
+        private final @Nullable String fFunctionName;
+
+        public Addr2lineInfo(@Nullable String sourceFileName,  @Nullable String functionName, @Nullable Long sourceLineNumber) {
+            fSourceFileName = sourceFileName;
+            fSourceLineNumber = sourceLineNumber;
+            fFunctionName = functionName;
+        }
+
+        @Override
+        public String toString() {
+            return Objects.toStringHelper(this)
+                    .add("fSourceFileName", fSourceFileName) //$NON-NLS-1$
+                    .add("fSourceLineNumber", fSourceLineNumber) //$NON-NLS-1$
+                    .add("fFunctionName", fFunctionName) //$NON-NLS-1$
+                    .toString();
+        }
+    }
+
+    private static @Nullable Iterable<Addr2lineInfo> getAddr2lineInfo(File file, @Nullable String buildId, long offset) {
+        LOGGER.finer(() -> String.format("[FileOffsetMapper:Addr2lineRequest] file=%s, buildId=%s, offset=0x%h", //$NON-NLS-1$
                 file.toString(), buildId, offset));
 
         if (!Files.exists((file.toPath()))) {
@@ -145,16 +217,16 @@ public final class FileOffsetMapper {
         // the file we are attempting to open.
         FileOffset fo = new FileOffset(checkNotNull(file.toString()), buildId, offset);
 
-        Iterable<SourceCallsite> callsites = CALLSITE_CACHE.getUnchecked(fo);
+        @Nullable Iterable<Addr2lineInfo> callsites = ADDR2LINE_INFO_CACHE.getUnchecked(fo);
         LOGGER.finer(() -> String.format("[FileOffsetMapper:RequestComplete] callsites=%s", callsites)); //$NON-NLS-1$
         return callsites;
     }
 
-    private static @Nullable Iterable<SourceCallsite> getCallsiteFromOffsetWithAddr2line(FileOffset fo) {
+    private static Iterable<Addr2lineInfo> callAddr2line(FileOffset fo) {
         String filePath = fo.fFilePath;
         long offset = fo.fOffset;
 
-        List<SourceCallsite> callsites = new LinkedList<>();
+        List<Addr2lineInfo> callsites = new LinkedList<>();
 
         // FIXME Could eventually use CDT's Addr2line class once it implements --inlines
         List<String> output = getOutputFromCommand(Arrays.asList(
@@ -162,7 +234,7 @@ public final class FileOffsetMapper {
 
         if (output == null) {
             /* Command returned an error */
-            return null;
+            return Collections.EMPTY_SET;
         }
 
         /*
@@ -190,7 +262,7 @@ public final class FileOffsetMapper {
                 }
                 try {
                     long lineNumber = Long.parseLong(elems[1]);
-                    callsites.add(new SourceCallsite(fileName, currentFunctionName, lineNumber));
+                    callsites.add(new Addr2lineInfo(fileName, currentFunctionName, lineNumber));
 
                 } catch (NumberFormatException e) {
                     /*
index ce13611afa3aaa3ab304d383c9a73f7d4189f3eb..ebe9640e678fb24c98931e4b7976cad8421a890c 100644 (file)
@@ -39,6 +39,26 @@ public class FunctionLocation {
         fOffset = offsetInFunction;
     }
 
+    /**
+     * Get the function name.
+     *
+     * @return The function name
+     * @since 2.1
+     */
+    public String getFunctionName() {
+        return fFunctionName;
+    }
+
+    /**
+     * Get the offset *within this function* represented by this location.
+     *
+     * @return The offset of this location, or 'null' if unavailable
+     * @since 2.1
+     */
+    public @Nullable Long getOffsetInFunction() {
+        return fOffset;
+    }
+
     @Override
     public String toString() {
         Long offset = fOffset;
index 22a25bc2d6982531c12c1879f9134fa734d6bf0d..76446ac4cfb3c14967b70f7715a5e2c00f77d164 100644 (file)
@@ -19,7 +19,9 @@ import org.eclipse.tracecompass.tmf.core.event.lookup.TmfCallsite;
  *
  * @author Alexandre Montplaisir
  * @since 2.0
+ * @deprecated No need for this anymore, use {@link TmfCallsite} directly.
  */
+@Deprecated
 public class SourceCallsite extends TmfCallsite {
 
     /**
@@ -33,7 +35,7 @@ public class SourceCallsite extends TmfCallsite {
      *            Line number
      */
     public SourceCallsite(String fileName, @Nullable String functionName, long lineNumber) {
-        super(fileName, functionName, lineNumber);
+        super(fileName, lineNumber);
     }
 
     @Override
index 6c73d0072a88ff990b548e5deb698fa093eb14cb..56759f09d3ab5b761cdb61dbcb447f4f9eb43d81 100644 (file)
@@ -11,7 +11,10 @@ package org.eclipse.tracecompass.lttng2.ust.core.analysis.debuginfo;
 
 import static org.eclipse.tracecompass.common.core.NonNullUtils.nullToEmptyString;
 
+import java.io.File;
+
 import org.eclipse.jdt.annotation.Nullable;
+import org.eclipse.tracecompass.internal.lttng2.ust.core.analysis.debuginfo.FileOffsetMapper;
 import org.eclipse.tracecompass.tmf.core.event.ITmfEvent;
 import org.eclipse.tracecompass.tmf.core.event.aspect.ITmfEventAspect;
 
@@ -40,17 +43,35 @@ public class UstDebugInfoFunctionAspect implements ITmfEventAspect<FunctionLocat
 
     @Override
     public @Nullable FunctionLocation resolve(ITmfEvent event) {
-        SourceCallsite sc = UstDebugInfoSourceAspect.INSTANCE.resolve(event);
-        if (sc == null) {
+        /* Resolve the binary callsite first. */
+        BinaryCallsite bc = UstDebugInfoBinaryAspect.INSTANCE.resolve(event);
+        if (bc == null) {
             return null;
         }
 
-        String functionName = sc.getFunctionName();
+        return getFunctionFromBinaryLocation(bc);
+    }
+
+    /**
+     * Get a function location starting directly from a binary callsite, instead
+     * of from a trace event.
+     *
+     * @param bc
+     *            The binary callsite, representing a binary and offset within
+     *            this binary
+     * @return The corresponding function location
+     * @since 2.1
+     */
+    public static @Nullable FunctionLocation getFunctionFromBinaryLocation(BinaryCallsite bc) {
+        String functionName = FileOffsetMapper.getFunctionNameFromOffset(
+                new File(bc.getBinaryFilePath()),
+                bc.getBuildId(),
+                bc.getOffset());
         if (functionName == null) {
             return null;
         }
 
-        /* We do not track the offset in the function at this time */
+        /* We do not track the offset inside the function at this time */
         return new FunctionLocation(functionName, null);
     }
 
index 98c0fd4a48046bf7a6d7756aa270cf17c9daefc4..0bc78f0237ea08fb08a3be84f21212e5b3195421 100644 (file)
@@ -20,8 +20,6 @@ import org.eclipse.tracecompass.tmf.core.event.ITmfEvent;
 import org.eclipse.tracecompass.tmf.core.event.aspect.ITmfEventAspect;
 import org.eclipse.tracecompass.tmf.core.event.lookup.TmfCallsite;
 
-import com.google.common.collect.Iterables;
-
 /**
  * Event aspect of UST traces to generate a {@link TmfCallsite} using the debug
  * info analysis and the IP (instruction pointer) context.
@@ -29,7 +27,7 @@ import com.google.common.collect.Iterables;
  * @author Alexandre Montplaisir
  * @since 2.0
  */
-public class UstDebugInfoSourceAspect implements ITmfEventAspect<SourceCallsite> {
+public class UstDebugInfoSourceAspect implements ITmfEventAspect<TmfCallsite> {
 
     /** Singleton instance */
     public static final UstDebugInfoSourceAspect INSTANCE = new UstDebugInfoSourceAspect();
@@ -46,8 +44,11 @@ public class UstDebugInfoSourceAspect implements ITmfEventAspect<SourceCallsite>
         return nullToEmptyString(Messages.UstDebugInfoAnalysis_SourceAspectHelpText);
     }
 
+    /**
+     * @since 2.1
+     */
     @Override
-    public @Nullable SourceCallsite resolve(ITmfEvent event) {
+    public @Nullable TmfCallsite resolve(ITmfEvent event) {
         /* This aspect only supports UST traces */
         if (!(event.getTrace() instanceof LttngUstTrace)) {
             return null;
@@ -63,7 +64,25 @@ public class UstDebugInfoSourceAspect implements ITmfEventAspect<SourceCallsite>
             return null;
         }
 
-        return getSourceCallsite(trace, bc);
+        TmfCallsite callsite = FileOffsetMapper.getCallsiteFromOffset(
+                new File(bc.getBinaryFilePath()),
+                bc.getBuildId(),
+                bc.getOffset());
+        if (callsite == null) {
+            return null;
+        }
+
+        /*
+         * Apply the path prefix again, this time on the path given from
+         * addr2line. If applicable.
+         */
+        String pathPrefix = trace.getSymbolProviderConfig().getActualRootDirPath();
+        if (pathPrefix.isEmpty()) {
+            return callsite;
+        }
+
+        String fullFileName = (pathPrefix + callsite.getFileName());
+        return new TmfCallsite(fullFileName, callsite.getLineNo());
     }
 
     /**
@@ -76,22 +95,23 @@ public class UstDebugInfoSourceAspect implements ITmfEventAspect<SourceCallsite>
      *            The binary callsite
      * @return The source callsite, which sould include file name, function name
      *         and line number
+     * @since 2.0
+     * @deprecated Should not be needed anymore, call aspects's resolve() method
+     *             directly. The SourceAspect does not include the function name
+     *             anymore.
      */
+    @Deprecated
     public static @Nullable SourceCallsite getSourceCallsite(LttngUstTrace trace, BinaryCallsite bc) {
-        Iterable<SourceCallsite> callsites = FileOffsetMapper.getCallsiteFromOffset(
+        TmfCallsite callsite = FileOffsetMapper.getCallsiteFromOffset(
                 new File(bc.getBinaryFilePath()),
                 bc.getBuildId(),
                 bc.getOffset());
-
-        if (callsites == null || Iterables.isEmpty(callsites)) {
+        if (callsite == null) {
             return null;
         }
-        /*
-         * TMF only supports the notion of one callsite per event at the moment.
-         * We will take the "deepest" one in the stack, which should refer to
-         * the initial, non-inlined location.
-         */
-        SourceCallsite callsite = Iterables.getLast(callsites);
+
+        Long callsiteLineNo = callsite.getLineNo();
+        long lineNo = (callsiteLineNo == null ? -1 : callsiteLineNo.longValue());
 
         /*
          * Apply the path prefix again, this time on the path given from
@@ -99,10 +119,10 @@ public class UstDebugInfoSourceAspect implements ITmfEventAspect<SourceCallsite>
          */
         String pathPrefix = trace.getSymbolProviderConfig().getActualRootDirPath();
         if (pathPrefix.isEmpty()) {
-            return callsite;
+            return new SourceCallsite(callsite.getFileName(), null, lineNo);
         }
 
         String fullFileName = (pathPrefix + callsite.getFileName());
-        return new SourceCallsite(fullFileName, callsite.getFunctionName(), callsite.getLineNumber());
+        return new SourceCallsite(fullFileName, null, lineNo);
     }
 }
index c6670d8aa97883d72e69a8d2080aaf47be24ef0f..9f1671ced9c4888c24cacc569475c1571f30cc26 100644 (file)
@@ -12,8 +12,10 @@ package org.eclipse.tracecompass.internal.lttng2.ust.ui.analysis.debuginfo;
 import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.jdt.annotation.Nullable;
 import org.eclipse.tracecompass.lttng2.ust.core.analysis.debuginfo.BinaryCallsite;
+import org.eclipse.tracecompass.lttng2.ust.core.analysis.debuginfo.FunctionLocation;
 import org.eclipse.tracecompass.lttng2.ust.core.analysis.debuginfo.UstDebugInfoAnalysisModule;
 import org.eclipse.tracecompass.lttng2.ust.core.analysis.debuginfo.UstDebugInfoBinaryAspect;
+import org.eclipse.tracecompass.lttng2.ust.core.analysis.debuginfo.UstDebugInfoFunctionAspect;
 import org.eclipse.tracecompass.lttng2.ust.core.analysis.debuginfo.UstDebugInfoSourceAspect;
 import org.eclipse.tracecompass.lttng2.ust.core.trace.LttngUstTrace;
 import org.eclipse.tracecompass.tmf.core.event.lookup.TmfCallsite;
@@ -62,8 +64,8 @@ public class UstDebugInfoSymbolProvider extends DefaultSymbolProvider {
             return null;
         }
 
-        TmfCallsite callsite = UstDebugInfoSourceAspect.getSourceCallsite(getTrace(), bc);
-        return (callsite == null ? null : callsite.getFunctionName());
+        FunctionLocation loc = UstDebugInfoFunctionAspect.getFunctionFromBinaryLocation(bc);
+        return (loc == null ? null : loc.getFunctionName());
     }
 
     @Deprecated
index 644e3757f86630d038def2a82f794820571a55d7..39289c37d23a8d1ad1356d3c457271f7cd848f5e 100644 (file)
@@ -16,6 +16,7 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
+import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.tracecompass.tmf.core.event.lookup.ITmfCallsite;
 import org.eclipse.tracecompass.tmf.core.event.lookup.TmfCallsite;
 import org.junit.Test;
@@ -29,26 +30,23 @@ public class TmfCallsiteTest {
     // ------------------------------------------------------------------------
     // Variables
     // ------------------------------------------------------------------------
-    private final static String fFileName1 = "filename1";
-    private final static String fFunctionName1 = "func1";
-    private final static long fLine1 = 10;
 
-    private final static String fFileName2 = "filename2";
-    private final static String fFunctionName2 = "func2";
-    private final static long fLine2 = 25;
+    private final static @NonNull String fFileName1 = "filename1";
+    private final static Long fLine1 = 10L;
 
-    private final static String fFileName3 = "filename3";
-    private final static String fFunctionName3 = null;
-    private final static long fLine3 = 123;
+    private final static @NonNull String fFileName2 = "filename2";
+    private final static Long fLine2 = 25L;
 
-    private final static String fFileName4 = "filename1";
-    private final static String fFunctionName4 = "func1";
-    private final static long fLine4 = 11;
+    private final static @NonNull String fFileName3 = "filename3";
+    private final static Long fLine3 = 123L;
 
-    private final static ITmfCallsite fCallsite1 = new TmfCallsite(fFileName1, fFunctionName1, fLine1);
-    private final static ITmfCallsite fCallsite2 = new TmfCallsite(fFileName2, fFunctionName2, fLine2);
-    private final static ITmfCallsite fCallsite3 = new TmfCallsite(fFileName3, fFunctionName3, fLine3);
-    private final static ITmfCallsite fCallsite4 = new TmfCallsite(fFileName4, fFunctionName4, fLine4);
+    private final static @NonNull String fFileName4 = "filename1";
+    private final static Long fLine4 = 11L;
+
+    private final static @NonNull ITmfCallsite fCallsite1 = new TmfCallsite(fFileName1, fLine1);
+    private final static @NonNull ITmfCallsite fCallsite2 = new TmfCallsite(fFileName2, fLine2);
+    private final static @NonNull ITmfCallsite fCallsite3 = new TmfCallsite(fFileName3, fLine3);
+    private final static @NonNull ITmfCallsite fCallsite4 = new TmfCallsite(fFileName4, fLine4);
 
     // ------------------------------------------------------------------------
     // Constructors
@@ -57,8 +55,7 @@ public class TmfCallsiteTest {
     @Test
     public void testDefaultConstructor() {
         assertEquals(fFileName1, fCallsite1.getFileName());
-        assertEquals(fFunctionName1, fCallsite1.getFunctionName());
-        assertEquals(fLine1, fCallsite1.getLineNumber());
+        assertEquals(fLine1, fCallsite1.getLineNo());
     }
 
     @Test
@@ -66,18 +63,7 @@ public class TmfCallsiteTest {
         TmfCallsite copy = new TmfCallsite(fCallsite1);
 
         assertEquals(fFileName1, copy.getFileName());
-        assertEquals(fFunctionName1, copy.getFunctionName());
-        assertEquals(fLine1, copy.getLineNumber());
-    }
-
-    @Test(expected = IllegalArgumentException.class)
-    public void testCallsiteCopy2() {
-        new TmfCallsite(null);
-    }
-
-    @Test(expected = IllegalArgumentException.class)
-    public void testCallsiteFileNull() {
-        new TmfCallsite(null, fFunctionName1, fLine1);
+        assertEquals(fLine1, copy.getLineNo());
     }
 
     // ------------------------------------------------------------------------
@@ -145,21 +131,14 @@ public class TmfCallsiteTest {
         assertFalse("equals", fCallsite1.equals(fCallsite1.getFileName()));
     }
 
-    @Test
-    public void testNullElements() {
-        ITmfCallsite callsite = new TmfCallsite(fFileName1, null, fLine1);
-        assertFalse("equals", fCallsite1.equals(callsite));
-        assertFalse("equals", callsite.equals(fCallsite1));
-    }
-
     // ------------------------------------------------------------------------
     // toString
     // ------------------------------------------------------------------------
 
     @Test
     public void testToString() {
-        assertEquals("toString", "filename1:10 func1()", fCallsite1.toString());
-        assertEquals("toString", "filename2:25 func2()", fCallsite2.toString());
+        assertEquals("toString", "filename1:10", fCallsite1.toString());
+        assertEquals("toString", "filename2:25", fCallsite2.toString());
         assertEquals("toString", "filename3:123", fCallsite3.toString());
     }
 }
index 0822458b1edc5b117271182da1642e2cdc01a600..1d9c1f0833936e44fd5784920cd8cc03b8c9c304 100644 (file)
@@ -131,7 +131,7 @@ public class SyslogEvent extends TextTraceEvent implements ITmfCollapsibleEvent,
             } catch (NumberFormatException e) {
                 // ignore
             }
-            return new TmfCallsite((String) getContent().getField(Field.FILE).getValue(), null, lineNo);
+            return new TmfCallsite((String) getContent().getField(Field.FILE).getValue(), lineNo);
         }
         return null;
     }
index 9ebd74ec47977537892a51855f40e7987e0141ce..146b87772a4627ea005fb89f74b1eb6e3ff2e959 100644 (file)
@@ -40,13 +40,28 @@ public interface ITmfCallsite {
      * Returns the function name of the call site.
      *
      * @return the function name or null
+     * @deprecated Should not be part of this interface anymore.
      */
+    @Deprecated
     @Nullable String getFunctionName();
 
     /**
      * Returns the line number of the call site.
      *
      * @return the line number
+     * @deprecated Use {@link #getLineNo()} instead, which can return null.
      */
+    @Deprecated
     long getLineNumber();
+
+    /**
+     * Returns the line number of the call site.
+     *
+     * @return The line number, or 'null' if unavailable
+     * @since 2.1
+     */
+    default @Nullable Long getLineNo() {
+        /* TODO Change to abstract method once getLineNumber() is removed */
+        return getLineNumber();
+    }
 }
index 8e81eaa82d589997d2f980362f7eaa96157ec02f..818a581a747df37816d43eebe47ed41ac6a1bd58 100644 (file)
 
 package org.eclipse.tracecompass.tmf.core.event.lookup;
 
+import static org.eclipse.tracecompass.common.core.NonNullUtils.checkNotNull;
+
 import java.util.Objects;
 
 import org.eclipse.jdt.annotation.NonNull;
+import org.eclipse.jdt.annotation.Nullable;
 
 /**
  * TMF call site information for source code lookup.
@@ -30,11 +33,8 @@ public class TmfCallsite implements ITmfCallsite {
     /** The file name string. */
     private final @NonNull String fFileName;
 
-    /** The function name. */
-    private final String fFunctionName;
-
     /** The line number. */
-    private final long fLineNumber;
+    private final @Nullable Long fLineNumber;
 
     // ------------------------------------------------------------------------
     // Constructors
@@ -49,13 +49,24 @@ public class TmfCallsite implements ITmfCallsite {
      *            - a function name
      * @param lineNumber
      *            - a line number
+     * @deprecated Use {@link #TmfCallsite(String, Long)} instead.
      */
+    @Deprecated
     public TmfCallsite(String fileName, String functionName, long lineNumber) {
-        if (fileName == null) {
-            throw new IllegalArgumentException();
-        }
+        this(checkNotNull(fileName), lineNumber);
+    }
+
+    /**
+     * Constructor
+     *
+     * @param fileName
+     *            The source file's name
+     * @param lineNumber
+     *            The line number in the source file
+     * @since 2.1
+     */
+    public TmfCallsite(@NonNull String fileName, @Nullable Long lineNumber) {
         fFileName = fileName;
-        fFunctionName = functionName;
         fLineNumber = lineNumber;
     }
 
@@ -65,13 +76,9 @@ public class TmfCallsite implements ITmfCallsite {
      * @param other
      *            - An other call site implementation
      */
-    public TmfCallsite(ITmfCallsite other) {
-        if (other == null) {
-            throw new IllegalArgumentException();
-        }
+    public TmfCallsite(@NonNull ITmfCallsite other) {
         fFileName = other.getFileName();
-        fFunctionName = other.getFunctionName();
-        fLineNumber = other.getLineNumber();
+        fLineNumber = other.getLineNo();
     }
 
     // ------------------------------------------------------------------------
@@ -83,13 +90,25 @@ public class TmfCallsite implements ITmfCallsite {
         return fFileName;
     }
 
+    @Deprecated
     @Override
     public String getFunctionName() {
-        return fFunctionName;
+        return ""; //$NON-NLS-1$
     }
 
+    @Deprecated
     @Override
     public long getLineNumber() {
+        Long lineNumber = fLineNumber;
+        return (lineNumber == null ? -1 : lineNumber.longValue());
+    }
+
+
+    /**
+     * @since 2.1
+     */
+    @Override
+    public @Nullable Long getLineNo() {
         return fLineNumber;
     }
 
@@ -99,13 +118,7 @@ public class TmfCallsite implements ITmfCallsite {
 
     @Override
     public int hashCode() {
-        final int prime = 31;
-        int result = 1;
-        // fFileName cannot be null!
-        result = prime * result + fFileName.hashCode();
-        result = prime * result + ((fFunctionName == null) ? 0 : fFunctionName.hashCode());
-        result = prime * result + (int) (fLineNumber ^ (fLineNumber >>> 32));
-        return result;
+        return Objects.hash(fFileName, fLineNumber);
     }
 
     @Override
@@ -126,9 +139,6 @@ public class TmfCallsite implements ITmfCallsite {
             return false;
         }
 
-        if (!Objects.equals(fFunctionName, other.fFunctionName)) {
-            return false;
-        }
         if (fLineNumber != other.fLineNumber) {
             return false;
         }
@@ -137,13 +147,11 @@ public class TmfCallsite implements ITmfCallsite {
 
     @Override
     public String toString() {
+        Long lineNumber = fLineNumber;
+
         StringBuilder builder = new StringBuilder();
         builder.append(fFileName).append(':');
-        builder.append(Long.toString(fLineNumber));
-        if (fFunctionName != null) {
-            builder.append(' ');
-            builder.append(fFunctionName).append("()"); //$NON-NLS-1$
-        }
+        builder.append(lineNumber == null ? "??" : Long.toString(lineNumber)); //$NON-NLS-1$
         return builder.toString();
     }
 }
index adbc9e8565db69ee5abd4501acf289a97236f7c4..ef02db0252029900e5532a64fb690c2e1868df8a 100644 (file)
@@ -13,6 +13,8 @@
 
 package org.eclipse.tracecompass.tmf.ui.viewers.events;
 
+import static org.eclipse.tracecompass.common.core.NonNullUtils.nullToEmptyString;
+
 import java.util.ArrayList;
 import java.util.List;
 
@@ -150,11 +152,9 @@ public class TmfEventPropertySource implements IPropertySource {
     private static class SourceLookupPropertySource implements IPropertySource {
 
         private static final String ID_FILE_NAME = "callsite_file"; //$NON-NLS-1$
-        private static final String ID_FUNCTION_NAME = "callsite_function"; //$NON-NLS-1$
         private static final String ID_LINE_NUMBER = "callsite_line"; //$NON-NLS-1$
 
         private static final String NAME_FILE_NAME = "File"; //$NON-NLS-1$
-        private static final String NAME_FUNCTION_NAME = "Function"; //$NON-NLS-1$
         private static final String NAME_LINE_NUMBER = "Line"; //$NON-NLS-1$
 
         private final ITmfSourceLookup fSourceLookup;
@@ -179,10 +179,6 @@ public class TmfEventPropertySource implements IPropertySource {
             if (cs != null) {
                 descriptors.add(new ReadOnlyTextPropertyDescriptor(ID_FILE_NAME, NAME_FILE_NAME));
                 descriptors.add(new ReadOnlyTextPropertyDescriptor(ID_LINE_NUMBER, NAME_LINE_NUMBER));
-                // only display function if available
-                if (cs.getFunctionName() != null) {
-                    descriptors.add(new ReadOnlyTextPropertyDescriptor(ID_FUNCTION_NAME, NAME_FUNCTION_NAME));
-                }
             }
             return descriptors.toArray(new IPropertyDescriptor[0]);
         }
@@ -205,10 +201,8 @@ public class TmfEventPropertySource implements IPropertySource {
             switch ((String) id) {
             case ID_FILE_NAME:
                 return cs.getFileName();
-            case ID_FUNCTION_NAME:
-                return cs.getFunctionName();
             case ID_LINE_NUMBER:
-                return String.valueOf(cs.getLineNumber());
+                return nullToEmptyString(cs.getLineNo());
             default:
                 return null;
             }
index b2af93c7d8e841ed8edaa448864a8f3fd36807b9..f0d39861f385bf5797b060aeee8ef9497c358db8 100644 (file)
@@ -1100,6 +1100,11 @@ public class TmfEventsTable extends TmfComponent implements IGotoMarker, IColorS
                 if (cs == null) {
                     return;
                 }
+                Long lineNo = cs.getLineNo();
+                if (lineNo == null) {
+                    /* Not enough information to provide a full callsite */
+                    return;
+                }
 
                 String fileName = cs.getFileName();
                 final String trimmedPath = fileName.replaceAll("\\.\\./", EMPTY_STRING); //$NON-NLS-1$
@@ -1121,7 +1126,7 @@ public class TmfEventsTable extends TmfComponent implements IGotoMarker, IColorS
                              * the line number, then seek there.
                              */
                             ITextEditor textEditor = (ITextEditor) editor;
-                            int lineNumber = Long.valueOf(cs.getLineNumber()).intValue();
+                            int lineNumber = lineNo.intValue();
                             IDocument document = textEditor.getDocumentProvider().getDocument(textEditor.getEditorInput());
 
                             IRegion region = document.getLineInformation(lineNumber - 1);
@@ -1170,7 +1175,7 @@ public class TmfEventsTable extends TmfComponent implements IGotoMarker, IColorS
                         }
                         if (file != null) {
                             marker = file.createMarker(IMarker.MARKER);
-                            marker.setAttribute(IMarker.LINE_NUMBER, Long.valueOf(cs.getLineNumber()).intValue());
+                            marker.setAttribute(IMarker.LINE_NUMBER, lineNo.intValue());
                             IDE.openEditor(PlatformUI.getWorkbench().getActiveWorkbenchWindow().getActivePage(), marker);
                             marker.delete();
                         } else if (files.isEmpty()) {
This page took 0.037953 seconds and 5 git commands to generate.